CozmoNate / HWSensors

HWSensors is a software bundle that includes drivers and applications that allow you to access information from hardware sensors available on your Mac
660 stars 133 forks source link

NVRAM saving compatibility with Chameleon #94

Open RehabMan opened 11 years ago

RehabMan commented 11 years ago

Here's a few other changes I had to make for Chameleon compatibility.

First of all, the waitForMatchingService. Currently you have a 10 sec timeout for this. On Chameleon, even though you will end up having nvram saving disabled, this wait adds 10 sec to boot time. On my system that triples the boot time since my system boots in about 5 seconds (so it goes from 5 -> 15 sec). This boot penalty is there even if you use the -fakesmc-ignore-nvram boot flag.

Our versions have diverged a bit, but you could implement the following:

   OSDictionary *matching = 0;
    if (smcDevice->savingKeysToNVRAM() && (matching = serviceMatching("IODTNVRAM"))) {
        if (IODTNVRAM *nvram = OSDynamicCast(IODTNVRAM, waitForMatchingService(matching, 1000000000ULL * 10))) {

Where:

public:
    bool                savingKeysToNVRAM() { return !ignoreNVRAM; }

The alternative (and probably cleaner) would be to move the code from FakeSMC that loads keys from nvram into a public member function in FakeSMCDevice (loadKeysFromNVRAM), since it has access to the ignoreNVRAM private member variable. I would have done it that way, but I didn't want to cause more diffs than necessary.

But I think you get the idea: Don't execute the waitForMatchingService if ignoreNVRAM is true.

RehabMan commented 11 years ago

BTW, I have nvram loading/saving working with Chameleon. There are more changes required for that (exception list mentioned on the other related issue)... mainly loading keys from /chosen/nvram instead of waiting for IODTNVRAM. But also changing the format of the way keys are stored in nvram... in saveKeyToNVRAM:

        snprintf(name, 32, "%s-%s-%s", kFakeSMCKeyPropertyPrefix, key->getKey(), key->getType());

Using dot/colon syntax was causing the data to be stored as separate registry entries in /chosen/nvram...

I'm not sure what you're proposal is for Chameleon users right now. Whether you want to do nvram saving or not for Chameleon. Feel free to borrow as necessary from my fork.

Currently I have enabled nvram saving only for Chameleon if you use boot flag -fakesmc-force-nvram. Once I test more, I'll probably remove that flag and go just with what you have (default on but with -fakesmc-ignore-nvram override).

RehabMan commented 11 years ago

I found that firmware vendor in IODeviceTree:/efi is stored in property 'firmware-vendor' not 'fw-vendor' as the code in FakeSMC.cpp expects. Is this another difference in Chameleon vs. Clover or is there a mistake in FakeSMC.cpp.

I changed my copy as follows:

localhost:kozlek.git Admin$ git diff
diff --git a/FakeSMC/FakeSMC.cpp b/FakeSMC/FakeSMC.cpp
index 8c1ca6c..b4b5576 100755
--- a/FakeSMC/FakeSMC.cpp
+++ b/FakeSMC/FakeSMC.cpp
@@ -60,7 +60,7 @@ bool FakeSMC::init(OSDictionary *dictionary)
     }

     if (IORegistryEntry *efi = IORegistryEntry::fromPath("/efi", gIODTPlane)) {
-        if (OSData *vendor = OSDynamicCast(OSData, efi->getProperty(kFakeSMCFirmwareVendor))) {
+        if (OSData *vendor = OSDynamicCast(OSData, efi->getProperty(kEFIFirmwareVendor))) {
             OSData *buffer = OSData::withCapacity(128);
             const unsigned char* data = static_cast<const unsigned char*>(vendor->getBytesNoCopy());

diff --git a/Shared/FakeSMCDefinitions.h b/Shared/FakeSMCDefinitions.h
index f544df0..0d4afd1 100755
--- a/Shared/FakeSMCDefinitions.h
+++ b/Shared/FakeSMCDefinitions.h
@@ -195,6 +195,7 @@
 #define kFakeSMCSetValueCallback                "kFakeSMCSetValueCallback"

 // NVRAM
+#define kEFIFirmwareVendor                      "firmware-vendor"
 #define kFakeSMCFirmwareVendor                  "fw-vendor"
 #define kFakeSMCKeyPropertyPrefix               "fakesmc-key"
RehabMan commented 11 years ago

Just checked the ioreg of your machine... It is 'firmware-vendor' there too, not 'fw-vendor'... so not a difference between Chameleon/Clover, just a bug in FakeSMC.cpp as noted above.

CozmoNate commented 11 years ago

It was my mistake, should be 'firmware-vendor', as it was before