| Message ID | A1484485FD99714DB2AB2C5EF81E7AC2AA75839D@ORSMSX116.amr.corp.intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Add basic Slim Bootloader payload support | expand |
On Fri, Jul 26, 2019 at 10:00 AM Park, Aiden <aiden.park@intel.com> wrote: > > Slim Bootloader already calibrated TSC and provides it to U-Boot. > Therefore, U-Boot does not have to re-calibrate TSC. > Configuring tsc_base and clock_rate makes x86 tsc_timer driver > bypass TSC calibration and use the provided TSC frequency. > - Get TSC frequency from performance info hob > - Set tsc_base and clock_rate for tsc_timer driver I'm still not convinced to have this. As kernel followed by U-Boot we thrust hardware more than something else. One more layer in between is usually an additional point to be error-prone. So, if something we may get directly from hardware, I consider better to get it from there. Of course, data structures can be left for sake of self-documentation.
Hi Andy, > -----Original Message----- > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] > Sent: Friday, July 26, 2019 7:37 PM > To: Park, Aiden <aiden.park@intel.com> > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u- > boot@lists.denx.de>; Simon Glass <sjg@chromium.org> > Subject: Re: [PATCH v6 5/8] x86: slimbootloader: Set TSC information for > tsc_timer > > On Fri, Jul 26, 2019 at 10:00 AM Park, Aiden <aiden.park@intel.com> wrote: > > > > Slim Bootloader already calibrated TSC and provides it to U-Boot. > > Therefore, U-Boot does not have to re-calibrate TSC. > > Configuring tsc_base and clock_rate makes x86 tsc_timer driver bypass > > TSC calibration and use the provided TSC frequency. > > - Get TSC frequency from performance info hob > > - Set tsc_base and clock_rate for tsc_timer driver > > I'm still not convinced to have this. As kernel followed by U-Boot we thrust > hardware more than something else. > One more layer in between is usually an additional point to be error-prone. > > So, if something we may get directly from hardware, I consider better to get it > from there. > > Of course, data structures can be left for sake of self-documentation. > Basically, Providing known information to a payload is Slim Bootloader architecture concept. Slim Bootloader does initialize chipset and hardware as much as it can, and provides useful information to a payload in HOBs to minimize hardware re-initialization in a payload, make it generic and make it light-weight. TSC frequency is specific to CPU or chipset, and the way to get exact TSC frequency also varies depending on CPU. ex) checking cpuid, platform info MSR, perf MSR for LFM/HFM/Turbo Mode, or FSB. All these consideration has already done in Slim Bootloader on each CPUs. I think that U-Boot tsc_timer also considers this case, and that's why tsc_timer skips calibration in its probing time if tsc_base and clock_rate are already configured. Kernel uses many timers like TSC, APIC timer, 8254 timer, HPET and so on. I also understand kernel trusts hardware and re-calibrate them because all these timers may not be properly configured in boot firmware stage. But, HOB information including TSC must be trusted between Slim Bootloader and its payload(U-Boot) in Slim Bootloader architecture. > -- > With Best Regards, > Andy Shevchenko Best Regards, Aiden
On Fri, Jul 26, 2019 at 3:50 PM Park, Aiden <aiden.park@intel.com> wrote: > > On Fri, Jul 26, 2019 at 10:00 AM Park, Aiden <aiden.park@intel.com> wrote: > > > > > > Slim Bootloader already calibrated TSC and provides it to U-Boot. > > > Therefore, U-Boot does not have to re-calibrate TSC. > > > Configuring tsc_base and clock_rate makes x86 tsc_timer driver bypass > > > TSC calibration and use the provided TSC frequency. > > > - Get TSC frequency from performance info hob > > > - Set tsc_base and clock_rate for tsc_timer driver > > > > I'm still not convinced to have this. As kernel followed by U-Boot we thrust > > hardware more than something else. > > One more layer in between is usually an additional point to be error-prone. > > > > So, if something we may get directly from hardware, I consider better to get it > > from there. > > > > Of course, data structures can be left for sake of self-documentation. > > > Basically, Providing known information to a payload is Slim Bootloader > architecture concept. Slim Bootloader does initialize chipset and hardware > as much as it can, and provides useful information to a payload in HOBs > to minimize hardware re-initialization in a payload, make it generic > and make it light-weight. > > TSC frequency is specific to CPU or chipset, and the way to get exact TSC > frequency also varies depending on CPU. > ex) checking cpuid, platform info MSR, perf MSR for LFM/HFM/Turbo Mode, or FSB. > All these consideration has already done in Slim Bootloader on each CPUs. > > I think that U-Boot tsc_timer also considers this case, and that's why > tsc_timer skips calibration in its probing time if tsc_base and clock_rate are > already configured. > > Kernel uses many timers like TSC, APIC timer, 8254 timer, HPET and so on. > I also understand kernel trusts hardware and re-calibrate them because all > these timers may not be properly configured in boot firmware stage. > But, HOB information including TSC must be trusted between Slim Bootloader > and its payload(U-Boot) in Slim Bootloader architecture. Okay, I didn't remember if this is described in documentation. If it's not, please, add a section and put there somelike you expleined to me here.
Hi Andy, > -----Original Message----- > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] > Sent: Saturday, July 27, 2019 6:06 AM > To: Park, Aiden <aiden.park@intel.com> > Cc: Bin Meng <bmeng.cn@gmail.com>; U-Boot Mailing List <u- > boot@lists.denx.de>; Simon Glass <sjg@chromium.org> > Subject: Re: [PATCH v6 5/8] x86: slimbootloader: Set TSC information for > tsc_timer > > On Fri, Jul 26, 2019 at 3:50 PM Park, Aiden <aiden.park@intel.com> wrote: > > > On Fri, Jul 26, 2019 at 10:00 AM Park, Aiden <aiden.park@intel.com> wrote: > > > > > > > > Slim Bootloader already calibrated TSC and provides it to U-Boot. > > > > Therefore, U-Boot does not have to re-calibrate TSC. > > > > Configuring tsc_base and clock_rate makes x86 tsc_timer driver > > > > bypass TSC calibration and use the provided TSC frequency. > > > > - Get TSC frequency from performance info hob > > > > - Set tsc_base and clock_rate for tsc_timer driver > > > > > > I'm still not convinced to have this. As kernel followed by U-Boot > > > we thrust hardware more than something else. > > > One more layer in between is usually an additional point to be error-prone. > > > > > > So, if something we may get directly from hardware, I consider > > > better to get it from there. > > > > > > Of course, data structures can be left for sake of self-documentation. > > > > > Basically, Providing known information to a payload is Slim Bootloader > > architecture concept. Slim Bootloader does initialize chipset and > > hardware as much as it can, and provides useful information to a > > payload in HOBs to minimize hardware re-initialization in a payload, > > make it generic and make it light-weight. > > > > TSC frequency is specific to CPU or chipset, and the way to get exact > > TSC frequency also varies depending on CPU. > > ex) checking cpuid, platform info MSR, perf MSR for LFM/HFM/Turbo Mode, > or FSB. > > All these consideration has already done in Slim Bootloader on each CPUs. > > > > I think that U-Boot tsc_timer also considers this case, and that's why > > tsc_timer skips calibration in its probing time if tsc_base and > > clock_rate are already configured. > > > > Kernel uses many timers like TSC, APIC timer, 8254 timer, HPET and so on. > > I also understand kernel trusts hardware and re-calibrate them because > > all these timers may not be properly configured in boot firmware stage. > > But, HOB information including TSC must be trusted between Slim > > Bootloader and its payload(U-Boot) in Slim Bootloader architecture. > > Okay, I didn't remember if this is described in documentation. > If it's not, please, add a section and put there somelike you expleined to me here. > Thanks for your understanding. Let me add more description in documentation. > -- > With Best Regards, > Andy Shevchenko Best Regards, Aiden
diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c b/arch/x86/cpu/slimbootloader/slimbootloader.c index 9f3a61ec61..e6b174ca88 100644 --- a/arch/x86/cpu/slimbootloader/slimbootloader.c +++ b/arch/x86/cpu/slimbootloader/slimbootloader.c @@ -4,9 +4,46 @@ */ #include <common.h> +#include <asm/arch/slimbootloader.h> + +DECLARE_GLOBAL_DATA_PTR; + +/** + * This sets tsc_base and clock_rate for early_timer and tsc_timer. + * The performance info guid hob has all performance timestamp data, but + * the only tsc frequency info is used for the timer driver for now. + * + * Slim Bootloader already calibrated TSC and provides it to U-Boot. + * Therefore, U-Boot does not have to re-calibrate TSC. + * Configuring tsc_base and clock_rate here makes x86 tsc_timer driver + * bypass TSC calibration and use the provided TSC frequency. + */ +static void tsc_init(void) +{ + struct sbl_performance_info *data; + const efi_guid_t guid = SBL_PERFORMANCE_INFO_GUID; + + if (!gd->arch.hob_list) + panic("hob list not found!"); + + gd->arch.tsc_base = rdtsc(); + debug("tsc_base=0x%llx\n", gd->arch.tsc_base); + + data = hob_get_guid_hob_data(gd->arch.hob_list, NULL, &guid); + if (!data) { + debug("performance info hob not found\n"); + return; + } + + /* frequency is in KHz, so to Hz */ + gd->arch.clock_rate = data->frequency * 1000; + debug("freq=0x%lx\n", gd->arch.clock_rate); +} int arch_cpu_init(void) { + tsc_init(); + return x86_cpu_init_f(); } diff --git a/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h b/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h index 174a341d32..05dd1b2b44 100644 --- a/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h +++ b/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h @@ -23,6 +23,13 @@ EFI_GUID(0x6c6872fe, 0x56a9, 0x4403, \ 0xbb, 0x98, 0x95, 0x8d, 0x62, 0xde, 0x87, 0xf1) +/** + * A GUID to get boot performance info hob which is provided by Slim Bootloader + */ +#define SBL_PERFORMANCE_INFO_GUID \ + EFI_GUID(0x868204be, 0x23d0, 0x4ff9, \ + 0xac, 0x34, 0xb9, 0x95, 0xac, 0x04, 0xb1, 0xb9) + /** * A single entry of memory map information * @@ -84,4 +91,25 @@ struct sbl_serial_port_info { u32 rsvd1; }; +/** + * This includes timestamp data which has been collected in Slim Bootloader + * stages from the reset vector. In addition, this has TSC frequency in KHz to + * calculate each timestamp. + * + * @rev : revision of performance_info structure. currently 1. + * @rsvd : padding for alignment + * @count : the number of collected timestamp data + * @flags : only used in Slim Bootloader + * @frequency: tsc frequency in KHz + * @timestamp: the array of timestamp data which has 64-bit tsc value + */ +struct sbl_performance_info { + u8 rev; + u8 rsvd[3]; + u16 count; + u16 flags; + u32 frequency; + u64 timestamp[0]; +}; + #endif /* __SLIMBOOTLOADER_ARCH_H__ */