trulyspinach / SMCAMDProcessor

Power management, monitoring and VirtualSMC plugin for AMD processors
BSD 3-Clause "New" or "Revised" License
1.03k stars 87 forks source link

fix #issues 163: Not Publishing CPU Temperature Readings in the version after 0.6.4 #167

Closed ChgygLin closed 10 months ago

ChgygLin commented 1 year ago

We can get the SMCKey info from the command "smcread -s". (smcread came from VirtualSMC)

The version 0.6.4: It's OK! [TC0E] type [sp78] 73703738 len [ 2] attr [80] -> 3610 [TC0F] type [sp78] 73703738 len [ 2] attr [80] -> 3610 [TC0J] type [sp78] 73703738 len [ 2] attr [80] -> 0000 [TC0P] type [sp78] 73703738 len [ 2] attr [80] -> 3610 [TC0T] type [sp78] 73703738 len [ 2] attr [80] -> 3610 [TC0p] type [sp78] 73703738 len [ 2] attr [80] -> 3610 [PCPR] type [?] 00000000 len [ 0] attr [00] -> NOT READABLE, code 89 [PSTR] type [?] 00000000 len [ 0] attr [00] -> NOT READABLE, code 89

After the version 0.6.4: It's not OK! [E0CT] type [87ps] 38377073 len [ 2] attr [80] -> 3460 [F0CT] type [87ps] 38377073 len [ 2] attr [80] -> 3460 [J0CT] type [87ps] 38377073 len [ 2] attr [80] -> 0000 [P0CT] type [87ps] 38377073 len [ 2] attr [80] -> 3460 [T0CT] type [87ps] 38377073 len [ 2] attr [80] -> 3460 [p0CT] type [87ps] 38377073 len [ 2] attr [80] -> 3460 [RPCP] type [?] 00000000 len [ 0] attr [00] -> NOT READABLE, code 89 [RTSP] type [?] 00000000 len [ 0] attr [00] -> NOT READABLE, code 89

The iStat read the SMCKey value to show sensor info. The Key "TC0E" means CPU Temperature. But now we can found the SMCKey "TC0E" was becoming "E0CT" ! They are in reverse order, So iStat cannot show the CPU Temperature again.

The "TC0E" was generated by the macro SMC_MAKE_IDENTIFIER. The order was controlled by the "EFIAPI" in SMC_MAKE_IDENTIFIER.

In the Lilu new version, there was a MacKernelSDK, and add the define "EFIAPI". MacKernelSDK/Headers/pexpert/i386/efi.h:78:#define EFIAPI

So we must undef the EFIAPI after EFIAPI was defined && before the using by SMC_MAKE_IDENTIFIER.

trulyspinach commented 11 months ago

Hi ChgygLin,

Thanks a lot for the fix and I am sorry for not being able to get back to you earlier.

I would be grateful If anyone can verify this fix.

ChgygLin commented 11 months ago

Hi ChgygLin,

Thanks a lot for the fix and I am sorry for not being able to get back to you earlier.

I would be grateful If anyone can verify this fix.

Hi,trulyspinach, Thanks for your replay and maybe I make a mistake. I only want the commit 079cf0c! And the commit 3aec304 is needless which is convenient for me to compile. I will commit another PR.

ChgygLin commented 11 months ago

I have reset the commit 3aec304, It turns out that pr commits have automatically synchronized.

Now everything is OK.

trulyspinach commented 10 months ago

Close and reopen to trigger CD.

trulyspinach commented 10 months ago

Hi @ChgygLin,

After you resetting the commits now it appears that this branch only have one line of different. I believe some changes were missing?

ChgygLin commented 10 months ago

Hi @ChgygLin,

After you resetting the commits now it appears that this branch only have one line of different. I believe some changes were missing?

Hi. @trulyspinach, It's a little complicated. I buil the project using Xcode 14 in macOS Ventura, and found some compatibility problems.

  1. One kernel header changed: <Library/LegacyIOService.h> has replaced by <IOKit/IOService.h>. i modified the include file.
  2. Also I found at the moment we must use MacKernelSDK instead of Lilu.kext, and i modified the library configuration.
  3. The MacKernelSDK defined a marco "EFIAPI" which will influence SMC_MAKE_IDENTIFIER. And we use SMC_MAKE_IDENTIFIER to generate the CPU Temperature SMCKey "TC0E" which will be used by VituralSMC to update the CPU Temperature. When the marco "EFIAPI" exists, the SMCKey "TC0E" become "E0CT". So the commit 3aec304 fixed this.

I'm not sure if the problem 1/2 commit is needed in older Xocde and macOS version, so i reset the commit which fixed the problem 1/2, and keep the commit 3aec304 which fixed the problem 3.

Welcome to discuss ^_^

trulyspinach commented 10 months ago

Hi @ChgygLin,

Thank for your reply, that's indeed interesting. I will refer this issue here #163 so it's easier to get around. I still can't test this myself unfortunately, hopefully someone will try out the automated build and let us know!

I've also switched most of the include to MacKernelSDK instead of Lilu in my recent commits.

ChgygLin commented 10 months ago

Hi @trulyspinach , The marco SMC_MAKE_IDENTIFIER is defined in the <VirtualSMCSDK/AppleSmc.h>. I wonder if it's needed to add the marco "EFIAPI" after <VirtualSMCSDK/AppleSmc.h>.

SMCAMDProcessor/SMCAMDProcessor.hpp

undef EFIAPI

#include <VirtualSMCSDK/kern_vsmcapi.hpp>
#include <VirtualSMCSDK/AppleSmc.h>
#define EFIAPI

Maybe it Avoid potential additional effects? Welcome your suggestions. ^_^

trulyspinach commented 10 months ago

Hi @trulyspinach , The marco SMC_MAKE_IDENTIFIER is defined in the <VirtualSMCSDK/AppleSmc.h>. I wonder if it's needed to add the marco "EFIAPI" after <VirtualSMCSDK/AppleSmc.h>.

SMCAMDProcessor/SMCAMDProcessor.hpp #undef EFIAPI #include <VirtualSMCSDK/kern_vsmcapi.hpp> #include <VirtualSMCSDK/AppleSmc.h> #define EFIAPI

Maybe it Avoid potential additional effects? Welcome your suggestions. ^_^

Hi @ChgygLin, yes, I believe that will be better.

ChgygLin commented 10 months ago

Hi @trulyspinach , The marco SMC_MAKE_IDENTIFIER is defined in the <VirtualSMCSDK/AppleSmc.h>. I wonder if it's needed to add the marco "EFIAPI" after <VirtualSMCSDK/AppleSmc.h>.

SMCAMDProcessor/SMCAMDProcessor.hpp #undef EFIAPI #include <VirtualSMCSDK/kern_vsmcapi.hpp> #include <VirtualSMCSDK/AppleSmc.h> #define EFIAPI

Maybe it Avoid potential additional effects? Welcome your suggestions. ^_^

Hi @ChgygLin, yes, I believe that will be better.

Thanks! ^_^