Re: [edk2-devel] [Patch V3] EmulatorPkg: don't display the cpu current speed
Thanks for the reply. I understand now that this is not a very proper change. I will talk to the bug reporter in Bugzilla to see if we can keep the current status. Best Regards, Zhiguang Liu >-Original Message- >From: Justen, Jordan L >Sent: Thursday, June 13, 2019 3:47 PM >To: Liu, Zhiguang ; devel@edk2.groups.io >Cc: Andrew Fish ; Ni, Ray >Subject: RE: [Patch V3] EmulatorPkg: don't display the cpu current speed > >On 2019-06-11 22:42:13, Liu, Zhiguang wrote: >> Thanks for the comments. >> I will change the commit message in the next version. >> But I don't think this is a issue worth making a major change. >> Given that the change is consistent with NT32, will you agree with this >change? > >Hmm. You are right that NT32 also does this. I don't agree enough to give you >a Reviewed-by, but if Andrew or Ray give a Reviewed-by, then I'm ok for us to >take the patch. > >-Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42400): https://edk2.groups.io/g/devel/message/42400 Mute This Topic: https://groups.io/mt/32013654/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V3] EmulatorPkg: don't display the cpu current speed
On 2019-06-11 22:42:13, Liu, Zhiguang wrote: > Thanks for the comments. > I will change the commit message in the next version. > But I don't think this is a issue worth making a major change. > Given that the change is consistent with NT32, will you agree with this > change? Hmm. You are right that NT32 also does this. I don't agree enough to give you a Reviewed-by, but if Andrew or Ray give a Reviewed-by, then I'm ok for us to take the patch. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42336): https://edk2.groups.io/g/devel/message/42336 Mute This Topic: https://groups.io/mt/32013654/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V3] EmulatorPkg: don't display the cpu current speed
Thanks for the comments. I will change the commit message in the next version. But I don't think this is a issue worth making a major change. Given that the change is consistent with NT32, will you agree with this change? Best Regards, Zhiguang Liu >-Original Message- >From: Justen, Jordan L >Sent: Tuesday, June 11, 2019 3:56 PM >To: Liu, Zhiguang ; devel@edk2.groups.io >Cc: Andrew Fish ; Ni, Ray >Subject: Re: [Patch V3] EmulatorPkg: don't display the cpu current speed > >On 2019-06-11 00:32:27, Zhiguang Liu wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1686 >> >> V3: I hope that changing the status of the mCpuSmbiosType4 >> wouldn't affect other features except showing CPU speed. >> The value is zero in NT32Pkg. >> >> Cc: Jordan Justen >> Cc: Andrew Fish >> Cc: Ray Ni >> Signed-off-by: Zhiguang Liu >> --- >> EmulatorPkg/CpuRuntimeDxe/Cpu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/EmulatorPkg/CpuRuntimeDxe/Cpu.c >> b/EmulatorPkg/CpuRuntimeDxe/Cpu.c index 00e93016af..a5e19b4181 >100644 >> --- a/EmulatorPkg/CpuRuntimeDxe/Cpu.c >> +++ b/EmulatorPkg/CpuRuntimeDxe/Cpu.c >> @@ -104,7 +104,7 @@ SMBIOS_TABLE_TYPE4 mCpuSmbiosType4 = { >>0, // ExternalClock; >>0, // MaxSpeed; >>0, // CurrentSpeed; >> - 0x41, // Status; >> + 0, // Status; > >It looks like bit 6 means the process is populated, and bits[2:0]==1 means the >CPU is enabled. > >So, it looks like this change will make SMBIOS indicate the the processor >socket is not populated, and bit2[2:0]==0 means that the CPU status is >unknown. > >I think the commit message for this patch should have been: > >=== > >EmulatorPkg: Change SMBIOS processor to unpopulated > >This change updates the SMBIOS processor information to indicate that the >processor is not populated, and that it's status is unknown. > >With this change the processor speed will not be shown in setup. > >Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1686 > >=== > >But, I'm not sure I agree we should make this change to fix this bug. >I'm not particularly concerned with this bug, but I wonder if perhaps the >MdeModulePkg should just suppress the item if the speed is 0. > >Or, alternately, perhaps we can investigate some methods to attempt to >determine the processor speed. I guess for all OS's, it might be difficult, but >we probably could support finding the processor speed under the most >common environments. > >-Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42256): https://edk2.groups.io/g/devel/message/42256 Mute This Topic: https://groups.io/mt/32013654/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V3] EmulatorPkg: don't display the cpu current speed
On 2019-06-11 00:32:27, Zhiguang Liu wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1686 > > V3: I hope that changing the status of the mCpuSmbiosType4 > wouldn't affect other features except showing CPU speed. > The value is zero in NT32Pkg. > > Cc: Jordan Justen > Cc: Andrew Fish > Cc: Ray Ni > Signed-off-by: Zhiguang Liu > --- > EmulatorPkg/CpuRuntimeDxe/Cpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/EmulatorPkg/CpuRuntimeDxe/Cpu.c b/EmulatorPkg/CpuRuntimeDxe/Cpu.c > index 00e93016af..a5e19b4181 100644 > --- a/EmulatorPkg/CpuRuntimeDxe/Cpu.c > +++ b/EmulatorPkg/CpuRuntimeDxe/Cpu.c > @@ -104,7 +104,7 @@ SMBIOS_TABLE_TYPE4 mCpuSmbiosType4 = { >0, // ExternalClock; >0, // MaxSpeed; >0, // CurrentSpeed; > - 0x41, // Status; > + 0, // Status; It looks like bit 6 means the process is populated, and bits[2:0]==1 means the CPU is enabled. So, it looks like this change will make SMBIOS indicate the the processor socket is not populated, and bit2[2:0]==0 means that the CPU status is unknown. I think the commit message for this patch should have been: === EmulatorPkg: Change SMBIOS processor to unpopulated This change updates the SMBIOS processor information to indicate that the processor is not populated, and that it's status is unknown. With this change the processor speed will not be shown in setup. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1686 === But, I'm not sure I agree we should make this change to fix this bug. I'm not particularly concerned with this bug, but I wonder if perhaps the MdeModulePkg should just suppress the item if the speed is 0. Or, alternately, perhaps we can investigate some methods to attempt to determine the processor speed. I guess for all OS's, it might be difficult, but we probably could support finding the processor speed under the most common environments. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42184): https://edk2.groups.io/g/devel/message/42184 Mute This Topic: https://groups.io/mt/32013654/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V3] EmulatorPkg: don't display the cpu current speed
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1686 V3: I hope that changing the status of the mCpuSmbiosType4 wouldn't affect other features except showing CPU speed. The value is zero in NT32Pkg. Cc: Jordan Justen Cc: Andrew Fish Cc: Ray Ni Signed-off-by: Zhiguang Liu --- EmulatorPkg/CpuRuntimeDxe/Cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/EmulatorPkg/CpuRuntimeDxe/Cpu.c b/EmulatorPkg/CpuRuntimeDxe/Cpu.c index 00e93016af..a5e19b4181 100644 --- a/EmulatorPkg/CpuRuntimeDxe/Cpu.c +++ b/EmulatorPkg/CpuRuntimeDxe/Cpu.c @@ -104,7 +104,7 @@ SMBIOS_TABLE_TYPE4 mCpuSmbiosType4 = { 0, // ExternalClock; 0, // MaxSpeed; 0, // CurrentSpeed; - 0x41, // Status; + 0, // Status; ProcessorUpgradeOther, // ProcessorUpgrade; ///< The enumeration value from PROCESSOR_UPGRADE. 0, // L1CacheHandle; 0, // L2CacheHandle; -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42181): https://edk2.groups.io/g/devel/message/42181 Mute This Topic: https://groups.io/mt/32013654/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-