Re: [edk2-devel] Determining TSC frequency programmatically

2019-08-16 Thread Vitaly Cheptsov via Groups.Io
Andrew,

Thanks for the reminder for CpuDxe.

I think we saw CpuDxe calculation some time ago, and it was most likely a 
reason we went with a very similar approach to determine the TSC frequency as a 
fallback mechanism. It, however, is in fact prone to issues, putting aside the 
non-guaranteed period and CPU frequency match. Basically, there are two 
problems:

1. The code implemented in C is subject to minor time drifts, which results in 
slightly different results across the reboots. The calculation based on CPUID 
15H is more accurate.
2. The lack of TPL changes during frequency measurement results in EFI_EVENTs 
potentially interrupting the process and thus making the resulting value very 
inaccurate. Since the value is cached there is not always a possibility to 
avoid it.

EmulatorPkg is a bit of a special case, which we think we know about :), 
actually thanks for fixing it up on macOS, this is appreciated!

All in all, I would say that there really needs to exist a decent library, 
which most likely CpuDxe should make a use of as well.

Best wishes,
Vitaly

> 16 авг. 2019 г., в 21:35, Andrew Fish  написал(а):
> 
> Vitaly,
> 
> As Mike mentioned platforms can know more info about how they are constructed 
> thus you may not want to have a lot of generic discovery code floating about 
> if you don't really need it. 
> 
> One option could be to pass up the TSC Frequency/Period via some EFI 
> mechanism so generic code can be told by platform specific code. 
> 
> The PI spec already has an abstraction for a CPU based timer that is 
> architecture neutral. The CPU Architectural Protocol has a GetTimerValue() 
> member function. 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/Cpu.h#L220
>  
> 
> 
> For X86 it returns TSC
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/CpuDxe/CpuDxe.c#L289 
> 
> 
> EFI Systems are not required to implement PI so we usually don't encourage 
> generic EFI code to go after PI APIs. 
> 
> I'd also point out that using TSC can break in things like the EmulatorPkg as 
> you end up running in ring 0 and TSC access is blocked. 
> https://github.com/tianocore/edk2/blob/master/EmulatorPkg/CpuRuntimeDxe/Cpu.c#L352
>  
> 
> https://github.com/tianocore/edk2/blob/master/EmulatorPkg/Unix/Host/EmuThunk.c#L250
>  
> 
> 
> 
> I would point that a library that did TSC frequency discovery would likely be 
> useful for the UefiCpuPkg CpuDxe driver. 
> 
> Thanks,
> 
> Andrew Fish
> 
>> On Aug 15, 2019, at 2:10 PM, Vitaly Cheptsov via Groups.Io 
>> mailto:vit9696=protonmail@groups.io>> 
>> wrote:
>> 
>> Hello,
>> 
>> I initially raised this question in a new TimerLib patch[1], but as the 
>> discussion was getting more distracted, I decided to create a separate 
>> thread in hopes new people could join.
>> 
>> The issue is that our UEFI bootloader needs to obtain TSC frequency to pass 
>> it to our specialised operating system that uses TSC for scheduling on x86.
>> 
>> For a while we went with ACPI power management timer to measure the 
>> frequency, but as modern Intel CPUs support CPUID 15H leaf 
>> (CPUID_TIME_STAMP_COUNTER) we try to use where possible for better accuracy. 
>> The issue with this CPUID leaf is that the crystal clock frequency returned 
>> in ECX register is optional and therefore can be 0. Intel SDM suggests to 
>> use a static value in this case[2], but it is completely opaque on how to 
>> match the running CPU with its static value from SDM.
>> 
>> Initially we went with CPUID model checking, but this failed badly for Xeon 
>> Scalable and Xeon W, as they share the CPUID (06_55H) but have different 
>> crystal clock frequencies (25 MHz vs 24 MHz accordingly). Donald Kuo gave a 
>> good hint in the previous thread that client CPUs usually get 24 MHz crystal 
>> clock, server CPUs have 25, and Atoms have 19.2. This, however, does not 
>> make the situation easier as we do not see a way to determine CPU vertical 
>> segment without e.g. parsing the CPUID brand string.
>> 
>> Apparently, we are not alone, and different open-source operating systems 
>> have different workarounds to this issue. For example, Linux kernel went 
>> with using marketing frequency from CPUID 16H leaf 
>> (CPUID_PROCESSOR_FREQUENCY)[3], and BSD flavours fallback to older methods 
>> when neither crystal clock frequency can be obtained through CPUID 15H, nor 
>> unambiguous CPUID models exist to be able to use static values.
>> 
>> Another issue we see with EDK II TimerLib implementations for x86 is that 
>> they are very model specific. As Michael Kinney said, the situation is not a 
>> problem when you use TimerLib for BSP 

Re: [edk2-devel] Determining TSC frequency programmatically

2019-08-16 Thread Andrew Fish via Groups.Io
Vitaly,

As Mike mentioned platforms can know more info about how they are constructed 
thus you may not want to have a lot of generic discovery code floating about if 
you don't really need it. 

One option could be to pass up the TSC Frequency/Period via some EFI mechanism 
so generic code can be told by platform specific code. 

The PI spec already has an abstraction for a CPU based timer that is 
architecture neutral. The CPU Architectural Protocol has a GetTimerValue() 
member function. 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/Cpu.h#L220

For X86 it returns TSC
https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/CpuDxe/CpuDxe.c#L289

EFI Systems are not required to implement PI so we usually don't encourage 
generic EFI code to go after PI APIs. 

I'd also point out that using TSC can break in things like the EmulatorPkg as 
you end up running in ring 0 and TSC access is blocked. 
https://github.com/tianocore/edk2/blob/master/EmulatorPkg/CpuRuntimeDxe/Cpu.c#L352
https://github.com/tianocore/edk2/blob/master/EmulatorPkg/Unix/Host/EmuThunk.c#L250


I would point that a library that did TSC frequency discovery would likely be 
useful for the UefiCpuPkg CpuDxe driver. 

Thanks,

Andrew Fish

> On Aug 15, 2019, at 2:10 PM, Vitaly Cheptsov via Groups.Io 
>  wrote:
> 
> Hello,
> 
> I initially raised this question in a new TimerLib patch[1], but as the 
> discussion was getting more distracted, I decided to create a separate thread 
> in hopes new people could join.
> 
> The issue is that our UEFI bootloader needs to obtain TSC frequency to pass 
> it to our specialised operating system that uses TSC for scheduling on x86.
> 
> For a while we went with ACPI power management timer to measure the 
> frequency, but as modern Intel CPUs support CPUID 15H leaf 
> (CPUID_TIME_STAMP_COUNTER) we try to use where possible for better accuracy. 
> The issue with this CPUID leaf is that the crystal clock frequency returned 
> in ECX register is optional and therefore can be 0. Intel SDM suggests to use 
> a static value in this case[2], but it is completely opaque on how to match 
> the running CPU with its static value from SDM.
> 
> Initially we went with CPUID model checking, but this failed badly for Xeon 
> Scalable and Xeon W, as they share the CPUID (06_55H) but have different 
> crystal clock frequencies (25 MHz vs 24 MHz accordingly). Donald Kuo gave a 
> good hint in the previous thread that client CPUs usually get 24 MHz crystal 
> clock, server CPUs have 25, and Atoms have 19.2. This, however, does not make 
> the situation easier as we do not see a way to determine CPU vertical segment 
> without e.g. parsing the CPUID brand string.
> 
> Apparently, we are not alone, and different open-source operating systems 
> have different workarounds to this issue. For example, Linux kernel went with 
> using marketing frequency from CPUID 16H leaf (CPUID_PROCESSOR_FREQUENCY)[3], 
> and BSD flavours fallback to older methods when neither crystal clock 
> frequency can be obtained through CPUID 15H, nor unambiguous CPUID models 
> exist to be able to use static values.
> 
> Another issue we see with EDK II TimerLib implementations for x86 is that 
> they are very model specific. As Michael Kinney said, the situation is not a 
> problem when you use TimerLib for BSP bringup, as you already know the CPU 
> family you target, however, it is not great for other uses, although they may 
> be discouraged. In our opinion, regardless of the use, this approach has a 
> number of design issues.
> 
> All modern Intel x86 CPUs have virtual TSC counter that has fixed frequency. 
> In fact, this is true for most, if not all, modern x86 CPUs by other vendors 
> as well. This makes us believe that EDK II should have generic TscTimerLib 
> for x86, which will work anywhere when given valid TSC frequency, and 
> TscFrequencyLib, which would determine TSC frequency in a vendor-specific 
> method. Ideally there exists GenericTscFrequencyLib that can do this for a 
> wide majority of CPUs through different methods at runtime, including CPUID 
> 15H, ACPI power management, vendor-specific extensions, etc.
> 
> To summarise:
> - We need to know how to match current running CPU with its crystal clock 
> frequency when CPUID 15H reports 0 ECX.
> - We would like to suggest to split TSC-based TimerLib in two libraries: 
> generic with timer implementation and platform-specific with TSC frequency 
> discovery.
> - We wonder whether a generic vendor-supported TSC frequency discovery 
> library working on a wide range of CPUs is feasible to have in EDK II 
> mainline.
> 
> Best regards,
> Vitaly
> 
> [1] 
> https://edk2.groups.io/g/devel/topic/patch_ueficpupkg_adding_a/32839184?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,32839184
>  
> 
> [PATCH] UefiCpuPkg: Adding a new TSC 

[edk2-devel] Determining TSC frequency programmatically

2019-08-15 Thread Vitaly Cheptsov via Groups.Io
Hello,

I initially raised this question in a new TimerLib patch[1], but as the 
discussion was getting more distracted, I decided to create a separate thread 
in hopes new people could join.

The issue is that our UEFI bootloader needs to obtain TSC frequency to pass it 
to our specialised operating system that uses TSC for scheduling on x86.

For a while we went with ACPI power management timer to measure the frequency, 
but as modern Intel CPUs support CPUID 15H leaf (CPUID_TIME_STAMP_COUNTER) we 
try to use where possible for better accuracy. The issue with this CPUID leaf 
is that the crystal clock frequency returned in ECX register is optional and 
therefore can be 0. Intel SDM suggests to use a static value in this case[2], 
but it is completely opaque on how to match the running CPU with its static 
value from SDM.

Initially we went with CPUID model checking, but this failed badly for Xeon 
Scalable and Xeon W, as they share the CPUID (06_55H) but have different 
crystal clock frequencies (25 MHz vs 24 MHz accordingly). Donald Kuo gave a 
good hint in the previous thread that client CPUs usually get 24 MHz crystal 
clock, server CPUs have 25, and Atoms have 19.2. This, however, does not make 
the situation easier as we do not see a way to determine CPU vertical segment 
without e.g. parsing the CPUID brand string.

Apparently, we are not alone, and different open-source operating systems have 
different workarounds to this issue. For example, Linux kernel went with using 
marketing frequency from CPUID 16H leaf (CPUID_PROCESSOR_FREQUENCY)[3], and BSD 
flavours fallback to older methods when neither crystal clock frequency can be 
obtained through CPUID 15H, nor unambiguous CPUID models exist to be able to 
use static values.

Another issue we see with EDK II TimerLib implementations for x86 is that they 
are very model specific. As Michael Kinney said, the situation is not a problem 
when you use TimerLib for BSP bringup, as you already know the CPU family you 
target, however, it is not great for other uses, although they may be 
discouraged. In our opinion, regardless of the use, this approach has a number 
of design issues.

All modern Intel x86 CPUs have virtual TSC counter that has fixed frequency. In 
fact, this is true for most, if not all, modern x86 CPUs by other vendors as 
well. This makes us believe that EDK II should have generic TscTimerLib for 
x86, which will work anywhere when given valid TSC frequency, and 
TscFrequencyLib, which would determine TSC frequency in a vendor-specific 
method. Ideally there exists GenericTscFrequencyLib that can do this for a wide 
majority of CPUs through different methods at runtime, including CPUID 15H, 
ACPI power management, vendor-specific extensions, etc.

To summarise:
- We need to know how to match current running CPU with its crystal clock 
frequency when CPUID 15H reports 0 ECX.
- We would like to suggest to split TSC-based TimerLib in two libraries: 
generic with timer implementation and platform-specific with TSC frequency 
discovery.
- We wonder whether a generic vendor-supported TSC frequency discovery library 
working on a wide range of CPUs is feasible to have in EDK II mainline.

Best regards,
Vitaly

[1] 
https://edk2.groups.io/g/devel/topic/patch_ueficpupkg_adding_a/32839184?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,32839184
 

[PATCH] UefiCpuPkg: Adding a new TSC library by using CPUID(0x15) TSC leaf
[2] 18.7.3 Determining the Processor Base Frequency
Table 18-75. Nominal Core Crystal Clock Frequency
[3] https://lore.kernel.org/lkml/20190509055417.13152-1-dr...@endlessm.com/ 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#45750): https://edk2.groups.io/g/devel/message/45750
Mute This Topic: https://groups.io/mt/32891598/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



signature.asc
Description: OpenPGP digital signature