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
658 stars 133 forks source link

New nvram code in FakeSMC::start causing kernel backtrace, hang on boot #90

Open RehabMan opened 11 years ago

RehabMan commented 11 years ago

This is on my desktop system Intel DH67GD/Core i7-2600k.

I have commented out the nvram loading code for now in my version:

    if (smcDevice->init(provider, OSDynamicCast(OSDictionary, getProperty("Configuration")))) {

        smcDevice->registerService();
        // Loading keys from NVRAM
        OSDictionary *matching = serviceMatching("IODTNVRAM");

#if 0
        if (IORegistryEntry* nvram = waitForMatchingService(matching)) {
            if (OSData *keys = OSDynamicCast(OSData, nvram->getProperty(kFakeSMCPropertyKeys))) {
                if (keys->getLength()) {
                    int count = 0;
                    unsigned int offset = 0;

                    do {
                        char name[5]; memcpy(name, keys->getBytesNoCopy(offset, 4), 4); name[4] = '\0'; offset += 4;
                        char type[5]; memcpy(type, keys->getBytesNoCopy(offset, 4), 4); type[4] = '\0'; offset += 4;
                        unsigned char size = 0; memcpy(&size, keys->getBytesNoCopy(offset, 1), 1); offset++;
                        const void *value = keys->getBytesNoCopy(offset, size); offset += size;

                        HWSensorsInfoLog("adding key %s from NVRAM", name);
                        if (smcDevice->addKeyWithValue(name, type, size, value)) {
                            HWSensorsInfoLog("key %s added from NVRAM", name);
                            count++;
                        }

                    } while (offset + 4 < keys->getLength());

                    HWSensorsInfoLog("%d key%s added from NVRAM", count, count == 1 ? "" : "s");
                }
            }
        }
#endif

        OSSafeRelease(matching);
    }
    else {
        HWSensorsInfoLog("failed to initialize SMC device");
        return false;
    }

If I move the #if 0 just one line down to allow the waitForMatchingService(matching) to execute, it causes kernel backtrace on boot (using "-v") and system does not boot.

So... waiting for this IODTNVRAM service is causing a problem.

CozmoNate commented 11 years ago

But you are using IORecursiveLock, change it to IOLock also

RehabMan commented 11 years ago

Doesn't matter. RecursiveLock is safer.

CozmoNate commented 11 years ago

But it's don't locks nothing if called from the same thread!

CozmoNate commented 11 years ago

Your suggested code doesn't works with Clover... Need to continue investigation

RehabMan commented 11 years ago

re: But it's don't locks nothing if called from the same thread!

An IORecursiveLock can be used anywhere an IOLock is being used without exception. The only time IORecursiveLockLock does nothing is when the lock has already been acquired by the same thread. In that case, there is no need to acquire the lock as it is already acquired. And even then... it does more than nothing because it increments the lock count such that when the IORecursiveLockUnlock is called, the lock is still held unless the count goes to zero. It allows you to have constructs like this, which are fairly common:

void MethodA()
{
    IORecursiveLockLock(lock);

    // manipulate data structures with exclusive access
    // and...

    MethodB(); // call to subroutine

    // perhaps more work on data structures

    IOResursiveLockUnlock(lock);
}

void MethodB()
{
     // grab lock because perhaps MethodB is called from other places than MethodA...
     IORecursiveLockLock(lock);

     // do work on sensitive structures
     //

     // release lock (note will not release if called from MethodA above)
     IORecursiveLockUnlock(lock);
}     

If the above was to be coded using an IOLock instead, the IOLockLock in MethodB would deadlock (on its own thread) when MethodB is called from MethodA because there is no lock count associated with an IOLock.

re: Your suggested code doesn't works with Clover... Need to continue investigation

Which version? Timer version? Exception list version? Or latest version bypassing AppleNVRAM::setProperty?

Either way, we can make it run-time conditional:

In initAndStart:

        OSString *vendor = OSDynamicCast(OSString, provider->getProperty(kFakeSMCFirmwareVendor));
        runningClover = (vendor && vendor->isEqualTo("CLOVER"));

And...

       if (runningClover)
           nvram->setProperty(tempName, OSData::withBytes(key->getValue(), key->getSize()));
        else
           nvram->IORegistryEntry::setProperty(tempName, OSData::withBytes(key->getValue(), key->getSize()));
CozmoNate commented 11 years ago

Ok, as it expected to work : you write to options using ioregistryentry method. Setproperty is overriden by nvram driver. So it writes your data to nvram and to options. But if you only write to options, that doesn't work with clover. You know, Clover firmware is efi, chameleon is only emulation. I'll send you my ioreg later today so you can see the difference. The first thing, on clover nvram driver is AppleEFINVRAM, on chameleon driver is AppleNVRAM.

CozmoNate commented 11 years ago

I think we should check nvram driver's class. a And do not stick with specific firmware name, if it will be Clover or Chaneleon

RehabMan commented 11 years ago

It would be great if you could send me the ioreg running via Clover. I'd like to see the differences...

Someday, when I have some spare time, I'll try Clover again and see if I can get it to work. I tried about 6 months ago and it was a disaster, so I gave up. But maybe things have improved enough for a retry...

RehabMan commented 11 years ago

FYI: Latest commit fb7b91953a54757e9ae93aef079a72fe90e1531e

FakeSMCDevice::loadKeysFromNVRAM is defined but never called.

CozmoNate commented 11 years ago

Yes, have no much time now.

That's my IOReg:

Kozlek's Mac.zip (432 KB) https://mega.co.nz/#!awJljKwK!UFitYC0dTzm4SLhSxHuG3y3W3P1SiDJpoQ2d8-J1CXY

felixbuenemann commented 11 years ago

Have the fixes been integrated/released yet? I'm still seeing this backtrace w/Chameleon 2.2-r2265:

HWSensors v5.3.877 Copyright 2013 netkas, slice, usr-sse2, kozlek, navi, THe KiNG, RehabMan. All rights reserved.
waitForSystemMapper
Backtrace 0xffffff80006b7239 0xffffff80006b6d1d 0xffffff7f81a2da18 0xffffff7f81a2cfd6 0xffffff8000692180 0xffffff800068e119 0xffffff8000693093
      Kernel Extensions in backtrace:
         org.netkas.driver.FakeSMC(877.0)[791F19DA-E7E8-3A96-890E-C893BA6C331F]@0xffffff7f81a27000->0xffffff7f81a3cfff
            dependency: com.apple.iokit.IOACPIFamily(1.4)[045D5D6F-AD1E-36DB-A249-A346E2B48E54]@0xffffff7f8161b000
[ PCI configuration begin ]
FakeSMCDevice: 13 preconfigured keys added
SMC: successfully initialized
tdack commented 11 years ago

Happening here with Chameleon 2.2-r2266 (Mavericks retail)

HWSensors v5.3.901 Copyright 2013 netkas, slice, usr-sse2, kozlek, navi, THe KiNG, RehabMan. All rights reserved.
waitForSystemMapper
Backtrace 0xffffff80006b7239 0xffffff80006b6d1d 0xffffff7f81a2ca18 0xffffff7f81a2bfd6 0xffffff8000692180 0xffffff800068e119 0xffffff8000693093
      Kernel Extensions in backtrace:
         org.netkas.driver.FakeSMC(901.0)[54B24D5C-CCCB-3578-871D-CD4D006D7B52]@0xffffff7f81a26000->0xffffff7f81a3bfff
            dependency: com.apple.iokit.IOACPIFamily(1.4)[045D5D6F-AD1E-36DB-A249-A346E2B48E54]@0xffffff7f8168f000
[ PCI configuration begin ]
FakeSMCDevice: 13 preconfigured keys added
SMC: successfully initialized
RehabMan commented 11 years ago

@felixbuenemann @tdack

Those are a separate issue. See issue #115. Currently, the NVRAM saving/loading is disabled by default.