| Message ID | A1484485FD99714DB2AB2C5EF81E7AC2AA746862@ORSMSX116.amr.corp.intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Add basic Slim Bootloader payload support | expand |
On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden <aiden.park@intel.com> wrote: > > Slim Bootloader provides TSC clock information in its performance > info hob. For now, TSC clock information is only used for timer driver > from the performance info hob. > - Get TSC frequency from performance info hob > - Set tsc_base and clock_rate for timer driver So why do we need this at all? We have a common TSC driver that works for all x86. > +#define LOADER_PERFORMANCE_INFO_GUID \ > + { \ > + 0x868204be, 0x23d0, 0x4ff9, \ > + { 0xac, 0x34, 0xb9, 0x95, 0xac, 0x04, 0xb1, 0xb9 } \ > + } Use EFI_GUID(), please. > +struct performance_info { Same, you better to make less generic names for data structs. > +} __packed; Same question. If it's aligned naturally by 32-bit boundaries, Why do you need __packed?
On Mon, Jul 22, 2019 at 11:41 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden <aiden.park@intel.com> wrote: > > > > Slim Bootloader provides TSC clock information in its performance > > info hob. For now, TSC clock information is only used for timer driver > > from the performance info hob. > > - Get TSC frequency from performance info hob > > - Set tsc_base and clock_rate for timer driver > > So why do we need this at all? We have a common TSC driver that works > for all x86. > Ah, yes! Thanks for the review. The U-Boot native TSC driver should support this out of the box. Aiden, could you please add a TSC DT node in the slimbootloader.dts file, and have a try? If the native U-Boot TSC driver does not work on your platform, I think you need send a patch to update the TSC driver directly. > > +#define LOADER_PERFORMANCE_INFO_GUID \ > > + { \ > > + 0x868204be, 0x23d0, 0x4ff9, \ > > + { 0xac, 0x34, 0xb9, 0x95, 0xac, 0x04, 0xb1, 0xb9 } \ > > + } > > Use EFI_GUID(), please. > > > +struct performance_info { > > Same, you better to make less generic names for data structs. > > > +} __packed; > Regards, Bin
Hi Andy, > -----Original Message----- > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] > Sent: Monday, July 22, 2019 8:42 AM > To: Park, Aiden <aiden.park@intel.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Simon Glass > <sjg@chromium.org>; Bin Meng <bmeng.cn@gmail.com> > Subject: Re: [PATCH v5 5/8] x86: slimbootloader: Set TSC information for timer > driver > > On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden <aiden.park@intel.com> wrote: > > > > Slim Bootloader provides TSC clock information in its performance info > > hob. For now, TSC clock information is only used for timer driver from > > the performance info hob. > > - Get TSC frequency from performance info hob > > - Set tsc_base and clock_rate for timer driver > > So why do we need this at all? We have a common TSC driver that works for all > x86. > This series also uses TSC driver, but giving these values will bypass TSC calibration in the driver. Slim Bootloader already has CPU specific TSC value and provides the TSC value in its performance data HOB. Therefore, U-Boot TSC driver does not have to re-calibrate TSC. > > +#define LOADER_PERFORMANCE_INFO_GUID \ > > + { \ > > + 0x868204be, 0x23d0, 0x4ff9, \ > > + { 0xac, 0x34, 0xb9, 0x95, 0xac, 0x04, 0xb1, 0xb9 } \ > > + } > > Use EFI_GUID(), please. > Let me use EFI_GUID. > > +struct performance_info { > > Same, you better to make less generic names for data structs. > Let me use particular one. Thanks. > > +} __packed; > > Same question. If it's aligned naturally by 32-bit boundaries, Why do you need > __packed? > Let me remote __packed. Thanks. > -- > 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..2153d2ac66 100644 --- a/arch/x86/cpu/slimbootloader/slimbootloader.c +++ b/arch/x86/cpu/slimbootloader/slimbootloader.c @@ -4,9 +4,43 @@ */ #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. + */ +static void tsc_init(void) +{ + struct performance_info *data = NULL; + const struct efi_guid guid = LOADER_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 = (struct performance_info *) + 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 5a0994544a..e0edbdddf5 100644 --- a/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h +++ b/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h @@ -27,6 +27,15 @@ { 0xbb, 0x98, 0x95, 0x8d, 0x62, 0xde, 0x87, 0xf1 } \ } +/** + * A GUID to get boot performance info hob which is provided by Slim Bootloader + */ +#define LOADER_PERFORMANCE_INFO_GUID \ + { \ + 0x868204be, 0x23d0, 0x4ff9, \ + { 0xac, 0x34, 0xb9, 0x95, 0xac, 0x04, 0xb1, 0xb9 } \ + } + /** * A single entry of memory map information * @@ -71,6 +80,27 @@ struct serial_port_info { u32 rsvd1; } __packed; +/** + * 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 performance_info { + u8 rev; + u8 rsvd[3]; + u16 count; + u16 flags; + u32 frequency; + u64 timestamp[0]; +} __packed; + /** * This includes all memory map entries which is sorted based on physical start * address, from low to high, and carved out reserved, acpi nvs, acpi reclaim