[PATCH 00/13] x86: Various minor enhancements for coreboot
This series includes some patches generated while getting U-Boot to boot more nicely on Brya, an Adler Lake Chromebook.
This includes: - show the ACPI tables with 'acpi list' - get the UART to work even if coreboot doesn't enable it - show unimplemented sysinfo tags - fix for keyboard not working - fix for trying to set up PCI regions when the info is not available - fix for looking at inaccessible memory to find the sysinfo table
Simon Glass (13): mtrr: Don't show an invalid CPU number x86: Adjust search range for sysinfo table input: Only reset the keyboard when running bare metal x86: coreboot: Allow ACPI tables to be recorded x86: coreboot: Collect the address of the ACPI tables x86: Allow locating UARTs by device ID pci: coreboot: Don't read regions when booting usb: Quieten a debug message x86: coreboot: Use a memory-mapped UART x86: coreboot: Document how to enable the debug UART x86: coreboot: Scan PCI after relocation x86: coreboot: Log function names and line numbers x86: coreboot: Show unimplemented sysinfo tags
arch/x86/cpu/cpu.c | 2 +- arch/x86/dts/coreboot.dts | 4 ++ arch/x86/include/asm/cb_sysinfo.h | 8 +++ arch/x86/include/asm/coreboot_tables.h | 2 + arch/x86/lib/coreboot/cb_sysinfo.c | 13 +++++ cmd/Kconfig | 3 +- cmd/acpi.c | 4 ++ cmd/x86/cbsysinfo.c | 9 ++++ cmd/x86/mtrr.c | 3 +- configs/coreboot_defconfig | 5 ++ doc/board/coreboot/coreboot.rst | 29 +++++++++++ drivers/input/i8042.c | 13 +++-- drivers/pci/pci-uclass.c | 4 ++ drivers/serial/serial_coreboot.c | 69 +++++++++++++++++++++++--- drivers/usb/host/xhci.c | 4 +- include/asm-generic/global_data.h | 4 +- include/configs/coreboot.h | 2 + include/pci_ids.h | 3 ++ 18 files changed, 161 insertions(+), 20 deletions(-)
When U-Boot did not do the MP init, we don't get an actual CPU number here. Skip printing it in that case.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/x86/mtrr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c index b213a942fde..95916933e9a 100644 --- a/cmd/x86/mtrr.c +++ b/cmd/x86/mtrr.c @@ -145,7 +145,8 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int argc, for (; i >= 0; i = mp_next_cpu(cpu_select, i)) { if (!first) printf("\n"); - printf("CPU %d:\n", i); + if (i < MP_SELECT_ALL) + printf("CPU %d:\n", i); ret = do_mtrr_list(reg_count, i); if (ret) { printf("Failed to read CPU %d (err=%d)\n", i,
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
When U-Boot did not do the MP init, we don't get an actual CPU number here. Skip printing it in that case.
Signed-off-by: Simon Glass sjg@chromium.org
cmd/x86/mtrr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c index b213a942fde..95916933e9a 100644 --- a/cmd/x86/mtrr.c +++ b/cmd/x86/mtrr.c @@ -145,7 +145,8 @@ static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int argc, for (; i >= 0; i = mp_next_cpu(cpu_select, i)) { if (!first) printf("\n");
printf("CPU %d:\n", i);
if (i < MP_SELECT_ALL)printf("CPU %d:\n", i); ret = do_mtrr_list(reg_count, i); if (ret) { printf("Failed to read CPU %d (err=%d)\n", i,
The CPU number <i> should not be printed here if (i >= MP_SELECT_ALL)
Regards, Bin
Avoid searching starting at 0 since this memory may not be available and the table cannot be there anyway. Start at 0x400 instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index 6fe6eaf6c84..3394e5b523c 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -352,7 +352,7 @@ long locate_coreboot_table(void) long addr;
/* We look for LBIO in the first 4K of RAM and again at 960KB */ - addr = detect_coreboot_table_at(0x0, 0x1000); + addr = detect_coreboot_table_at(0x400, 0xc00); if (addr < 0) addr = detect_coreboot_table_at(0xf0000, 0x1000);
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Avoid searching starting at 0 since this memory may not be available
Please describe in more detail why memory address 0 is not available?
and the table cannot be there anyway. Start at 0x400 instead.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index 6fe6eaf6c84..3394e5b523c 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -352,7 +352,7 @@ long locate_coreboot_table(void) long addr;
/* We look for LBIO in the first 4K of RAM and again at 960KB */
And update the comment here for the memory address 0 too.
addr = detect_coreboot_table_at(0x0, 0x1000);
addr = detect_coreboot_table_at(0x400, 0xc00); if (addr < 0) addr = detect_coreboot_table_at(0xf0000, 0x1000);--
Regards, Bin
If U-Boot is not the first-stage bootloader we should not init the keyboard, since it has already been done. Check for this.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/input/i8042.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/input/i8042.c b/drivers/input/i8042.c index 3563dc98838..e2189726674 100644 --- a/drivers/input/i8042.c +++ b/drivers/input/i8042.c @@ -6,11 +6,14 @@
/* i8042.c - Intel 8042 keyboard driver routines */
+#define LOG_CATEGORY UCLASS_KEYBOARD + #include <common.h> #include <dm.h> #include <env.h> #include <errno.h> #include <i8042.h> +#include <init.h> #include <input.h> #include <keyboard.h> #include <log.h> @@ -132,10 +135,12 @@ static int kbd_reset(int quirk) goto err;
/* keyboard reset */ - if (kbd_write(I8042_DATA_REG, CMD_RESET_KBD) || - kbd_read(I8042_DATA_REG) != KBD_ACK || - kbd_read(I8042_DATA_REG) != KBD_POR) - goto err; + if (ll_boot_init()) { + if (kbd_write(I8042_DATA_REG, CMD_RESET_KBD) || + kbd_read(I8042_DATA_REG) != KBD_ACK || + kbd_read(I8042_DATA_REG) != KBD_POR) + goto err; + }
if (kbd_write(I8042_DATA_REG, CMD_DRAIN_OUTPUT) || kbd_read(I8042_DATA_REG) != KBD_ACK)
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
If U-Boot is not the first-stage bootloader we should not init the keyboard, since it has already been done. Check for this.
But re-init does no harm, right?
Signed-off-by: Simon Glass sjg@chromium.org
drivers/input/i8042.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
Regards, Bin
Hi Bin,
On Mon, 20 Mar 2023 at 19:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
If U-Boot is not the first-stage bootloader we should not init the keyboard, since it has already been done. Check for this.
But re-init does no harm, right?
Actually it causes the keyboard to fail on Brya (Felwinter), a Chromebook. It could be due to it having a numeric keypad.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/input/i8042.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
Regards, Simon
Hi Simon,
On Tue, Mar 21, 2023 at 2:40 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 20 Mar 2023 at 19:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
If U-Boot is not the first-stage bootloader we should not init the keyboard, since it has already been done. Check for this.
But re-init does no harm, right?
Actually it causes the keyboard to fail on Brya (Felwinter), a Chromebook. It could be due to it having a numeric keypad.
I assume Linux kernel will do the keyboard re-init no matter what bootloader does. So does Linux kernel fail to re-init the keyboard? If no, I guess we could improve the U-Boot keyboard driver somehow?
Hi Bin,
On Tue, 21 Mar 2023 at 14:25, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Mar 21, 2023 at 2:40 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 20 Mar 2023 at 19:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
If U-Boot is not the first-stage bootloader we should not init the keyboard, since it has already been done. Check for this.
But re-init does no harm, right?
Actually it causes the keyboard to fail on Brya (Felwinter), a Chromebook. It could be due to it having a numeric keypad.
I assume Linux kernel will do the keyboard re-init no matter what bootloader does. So does Linux kernel fail to re-init the keyboard? If no, I guess we could improve the U-Boot keyboard driver somehow?
I found another way, to flush the buffer first.
Regards, Simon
Hi Bin,
On Thu, 23 Mar 2023 at 11:55, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Tue, 21 Mar 2023 at 14:25, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Mar 21, 2023 at 2:40 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 20 Mar 2023 at 19:32, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
If U-Boot is not the first-stage bootloader we should not init the keyboard, since it has already been done. Check for this.
But re-init does no harm, right?
Actually it causes the keyboard to fail on Brya (Felwinter), a Chromebook. It could be due to it having a numeric keypad.
I assume Linux kernel will do the keyboard re-init no matter what bootloader does. So does Linux kernel fail to re-init the keyboard? If no, I guess we could improve the U-Boot keyboard driver somehow?
I found another way, to flush the buffer first.
Hmm no that does not always work, depending on what has happened before in coreboot.
Also it doesn't look like Linux always resets. Please see here:
https://github.com/torvalds/linux/blob/master/drivers/input/serio/i8042.c#L5...
Regards, Simon
At present any ACPI tables created by prior-stage firmware are ignored. It is useful to be able to view these in U-Boot.
Add the acpi command for coreboot and use that to allow recording the ACPI-table start.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/Kconfig | 3 +-- cmd/acpi.c | 4 ++++ include/asm-generic/global_data.h | 4 ++-- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index b2d75987170..97c8610e461 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -109,8 +109,7 @@ menu "Info commands"
config CMD_ACPI bool "acpi" - depends on ACPIGEN - default y + default y if TARGET_COREBOOT || ACPIGEN help List and dump ACPI tables. ACPI (Advanced Configuration and Power Interface) is used mostly on x86 for providing information to the diff --git a/cmd/acpi.c b/cmd/acpi.c index d0fc062ef8c..991b5235e28 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -162,6 +162,10 @@ static int do_acpi_items(struct cmd_tbl *cmdtp, int flag, int argc, bool dump_contents;
dump_contents = argc >= 2 && !strcmp("-d", argv[1]); + if (!IS_ENABLED(CONFIG_ACPIGEN)) { + printf("Not supported (enable ACPIGEN)\n"); + return CMD_RET_FAILURE; + } acpi_dump_items(dump_contents ? ACPI_DUMP_CONTENTS : ACPI_DUMP_LIST);
return 0; diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index da17ac8cbc8..2e3feb5a8c0 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -457,7 +457,7 @@ struct global_data { */ fdt_addr_t translation_offset; #endif -#ifdef CONFIG_GENERATE_ACPI_TABLE +#if defined(CONFIG_GENERATE_ACPI_TABLE) || defined(CONFIG_CMD_ACPI) /** * @acpi_ctx: ACPI context pointer */ @@ -536,7 +536,7 @@ static_assert(sizeof(struct global_data) == GD_SIZE); #define gd_dm_priv_base() NULL #endif
-#ifdef CONFIG_GENERATE_ACPI_TABLE +#if defined(CONFIG_GENERATE_ACPI_TABLE) || defined(CONFIG_CMD_ACPI) #define gd_acpi_ctx() gd->acpi_ctx #define gd_acpi_start() gd->acpi_start #define gd_set_acpi_start(addr) gd->acpi_start = addr
Pick this up from the sysinfo tables and display it with the cbsysinfo command. This allows the 'acpi list' command to work when booting from coreboot.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/include/asm/cb_sysinfo.h | 2 ++ arch/x86/include/asm/coreboot_tables.h | 2 ++ arch/x86/lib/coreboot/cb_sysinfo.c | 11 +++++++++++ cmd/x86/cbsysinfo.c | 1 + 4 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 0201ac6b03a..6b266149cf6 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -133,6 +133,7 @@ * @mtc_size: Size of MTC region * @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning * not used + * @rsdp: Pointer to ACPI RSDP table */ struct sysinfo_t { unsigned int cpu_khz; @@ -211,6 +212,7 @@ struct sysinfo_t { u64 mtc_start; u32 mtc_size; void *chromeos_vpd; + void *rsdp; };
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index f131de56a40..2d6f3db3a5f 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -422,6 +422,8 @@ struct cb_tsc_info { #define CB_TAG_SERIALNO 0x002a #define CB_MAX_SERIALNO_LENGTH 32
+#define CB_TAG_ACPI_RSDP 0x0043 + #define CB_TAG_CMOS_OPTION_TABLE 0x00c8
struct cb_cmos_option_table { diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index 748fa4ee53b..a11a2587f66 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -264,6 +264,13 @@ static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info) info->mrc_cache = map_sysmem(cbmem->cbmem_tab, 0); }
+static void cb_parse_acpi_rsdp(void *ptr, struct sysinfo_t *info) +{ + struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr; + + info->rsdp = map_sysmem(cbmem->cbmem_tab, 0); +} + __weak void cb_parse_unhandled(u32 tag, unsigned char *ptr) { } @@ -428,6 +435,9 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) case CB_TAG_MRC_CACHE: cb_parse_mrc_cache(rec, info); break; + case CB_TAG_ACPI_RSDP: + cb_parse_acpi_rsdp(rec, info); + break; default: cb_parse_unhandled(rec->tag, ptr); break; @@ -454,6 +464,7 @@ int get_coreboot_info(struct sysinfo_t *info) if (!ret) return -ENOENT; gd->arch.coreboot_table = addr; + gd_set_acpi_start(map_to_sysmem(info->rsdp)); gd->flags |= GD_FLG_SKIP_LL_INIT;
return 0; diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 34fdaf5b1b1..07570b00c9a 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -363,6 +363,7 @@ static void show_table(struct sysinfo_t *info, bool verbose) print_hex("MTC size", info->mtc_size);
print_ptr("Chrome OS VPD", info->chromeos_vpd); + print_ptr("RSDP", info->rsdp); }
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Pick this up from the sysinfo tables and display it with the cbsysinfo command. This allows the 'acpi list' command to work when booting from coreboot.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 2 ++ arch/x86/include/asm/coreboot_tables.h | 2 ++ arch/x86/lib/coreboot/cb_sysinfo.c | 11 +++++++++++ cmd/x86/cbsysinfo.c | 1 + 4 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 0201ac6b03a..6b266149cf6 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -133,6 +133,7 @@
- @mtc_size: Size of MTC region
- @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning
not used
*/
- @rsdp: Pointer to ACPI RSDP table
struct sysinfo_t { unsigned int cpu_khz; @@ -211,6 +212,7 @@ struct sysinfo_t { u64 mtc_start; u32 mtc_size; void *chromeos_vpd;
void *rsdp;};
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index f131de56a40..2d6f3db3a5f 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -422,6 +422,8 @@ struct cb_tsc_info { #define CB_TAG_SERIALNO 0x002a #define CB_MAX_SERIALNO_LENGTH 32
+#define CB_TAG_ACPI_RSDP 0x0043
This is using space but should be tab.
#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
struct cb_cmos_option_table { diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index 748fa4ee53b..a11a2587f66 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -264,6 +264,13 @@ static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info) info->mrc_cache = map_sysmem(cbmem->cbmem_tab, 0); }
+static void cb_parse_acpi_rsdp(void *ptr, struct sysinfo_t *info) +{
struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;info->rsdp = map_sysmem(cbmem->cbmem_tab, 0);+}
__weak void cb_parse_unhandled(u32 tag, unsigned char *ptr) { } @@ -428,6 +435,9 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) case CB_TAG_MRC_CACHE: cb_parse_mrc_cache(rec, info); break;
case CB_TAG_ACPI_RSDP:cb_parse_acpi_rsdp(rec, info);break; default: cb_parse_unhandled(rec->tag, ptr); break;@@ -454,6 +464,7 @@ int get_coreboot_info(struct sysinfo_t *info) if (!ret) return -ENOENT; gd->arch.coreboot_table = addr;
gd_set_acpi_start(map_to_sysmem(info->rsdp)); gd->flags |= GD_FLG_SKIP_LL_INIT; return 0;diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 34fdaf5b1b1..07570b00c9a 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -363,6 +363,7 @@ static void show_table(struct sysinfo_t *info, bool verbose) print_hex("MTC size", info->mtc_size);
print_ptr("Chrome OS VPD", info->chromeos_vpd);
print_ptr("RSDP", info->rsdp);}
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
I tested this patch on top of coreboot with U-Boot as a payload running on QEMU i440fx, but U-Boot does not list acpi tables.
=> acpi list No ACPI tables present
Regards, Bin
Hi Bin,
On Mon, 20 Mar 2023 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Pick this up from the sysinfo tables and display it with the cbsysinfo command. This allows the 'acpi list' command to work when booting from coreboot.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 2 ++ arch/x86/include/asm/coreboot_tables.h | 2 ++ arch/x86/lib/coreboot/cb_sysinfo.c | 11 +++++++++++ cmd/x86/cbsysinfo.c | 1 + 4 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 0201ac6b03a..6b266149cf6 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -133,6 +133,7 @@
- @mtc_size: Size of MTC region
- @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning
not used
*/
- @rsdp: Pointer to ACPI RSDP table
struct sysinfo_t { unsigned int cpu_khz; @@ -211,6 +212,7 @@ struct sysinfo_t { u64 mtc_start; u32 mtc_size; void *chromeos_vpd;
void *rsdp;};
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index f131de56a40..2d6f3db3a5f 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -422,6 +422,8 @@ struct cb_tsc_info { #define CB_TAG_SERIALNO 0x002a #define CB_MAX_SERIALNO_LENGTH 32
+#define CB_TAG_ACPI_RSDP 0x0043
This is using space but should be tab.
#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
struct cb_cmos_option_table { diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index 748fa4ee53b..a11a2587f66 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -264,6 +264,13 @@ static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info) info->mrc_cache = map_sysmem(cbmem->cbmem_tab, 0); }
+static void cb_parse_acpi_rsdp(void *ptr, struct sysinfo_t *info) +{
struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;info->rsdp = map_sysmem(cbmem->cbmem_tab, 0);+}
__weak void cb_parse_unhandled(u32 tag, unsigned char *ptr) { } @@ -428,6 +435,9 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) case CB_TAG_MRC_CACHE: cb_parse_mrc_cache(rec, info); break;
case CB_TAG_ACPI_RSDP:cb_parse_acpi_rsdp(rec, info);break; default: cb_parse_unhandled(rec->tag, ptr); break;@@ -454,6 +464,7 @@ int get_coreboot_info(struct sysinfo_t *info) if (!ret) return -ENOENT; gd->arch.coreboot_table = addr;
gd_set_acpi_start(map_to_sysmem(info->rsdp)); gd->flags |= GD_FLG_SKIP_LL_INIT; return 0;
Regards, Bin
diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 34fdaf5b1b1..07570b00c9a 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -363,6 +363,7 @@ static void show_table(struct sysinfo_t *info, bool verbose) print_hex("MTC size", info->mtc_size);
print_ptr("Chrome OS VPD", info->chromeos_vpd);
print_ptr("RSDP", info->rsdp);}
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
I tested this patch on top of coreboot with U-Boot as a payload running on QEMU i440fx, but U-Boot does not list acpi tables.
=> acpi list No ACPI tables present
Can you try 'cbsysinfo' to see what it is passing in as the RSDP?
Regards, Simon
Hi Simon,
On Fri, Mar 24, 2023 at 1:55 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 20 Mar 2023 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Pick this up from the sysinfo tables and display it with the cbsysinfo command. This allows the 'acpi list' command to work when booting from coreboot.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 2 ++ arch/x86/include/asm/coreboot_tables.h | 2 ++ arch/x86/lib/coreboot/cb_sysinfo.c | 11 +++++++++++ cmd/x86/cbsysinfo.c | 1 + 4 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 0201ac6b03a..6b266149cf6 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -133,6 +133,7 @@
- @mtc_size: Size of MTC region
- @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning
not used
*/
- @rsdp: Pointer to ACPI RSDP table
struct sysinfo_t { unsigned int cpu_khz; @@ -211,6 +212,7 @@ struct sysinfo_t { u64 mtc_start; u32 mtc_size; void *chromeos_vpd;
void *rsdp;};
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index f131de56a40..2d6f3db3a5f 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -422,6 +422,8 @@ struct cb_tsc_info { #define CB_TAG_SERIALNO 0x002a #define CB_MAX_SERIALNO_LENGTH 32
+#define CB_TAG_ACPI_RSDP 0x0043
This is using space but should be tab.
#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
struct cb_cmos_option_table { diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index 748fa4ee53b..a11a2587f66 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -264,6 +264,13 @@ static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info) info->mrc_cache = map_sysmem(cbmem->cbmem_tab, 0); }
+static void cb_parse_acpi_rsdp(void *ptr, struct sysinfo_t *info) +{
struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;info->rsdp = map_sysmem(cbmem->cbmem_tab, 0);+}
__weak void cb_parse_unhandled(u32 tag, unsigned char *ptr) { } @@ -428,6 +435,9 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) case CB_TAG_MRC_CACHE: cb_parse_mrc_cache(rec, info); break;
case CB_TAG_ACPI_RSDP:cb_parse_acpi_rsdp(rec, info);break; default: cb_parse_unhandled(rec->tag, ptr); break;@@ -454,6 +464,7 @@ int get_coreboot_info(struct sysinfo_t *info) if (!ret) return -ENOENT; gd->arch.coreboot_table = addr;
gd_set_acpi_start(map_to_sysmem(info->rsdp)); gd->flags |= GD_FLG_SKIP_LL_INIT; return 0;Regards, Bin
diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 34fdaf5b1b1..07570b00c9a 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -363,6 +363,7 @@ static void show_table(struct sysinfo_t *info, bool verbose) print_hex("MTC size", info->mtc_size);
print_ptr("Chrome OS VPD", info->chromeos_vpd);
print_ptr("RSDP", info->rsdp);}
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
I tested this patch on top of coreboot with U-Boot as a payload running on QEMU i440fx, but U-Boot does not list acpi tables.
=> acpi list No ACPI tables present
Can you try 'cbsysinfo' to see what it is passing in as the RSDP?
'cbsysinfo' shows RSDP is 0.
=> cbsysinfo Coreboot table at 500, decoded to 06f558a0, forwarded to 07f99000 ... RSDP : 00000000 Unimpl. : 10 37 40 => acpi list No ACPI tables present
coreboot boot log says:
[DEBUG] Writing coreboot table at 0x07f99000 [DEBUG] 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES [DEBUG] 1. 0000000000001000-000000000009ffff: RAM [DEBUG] 2. 00000000000a0000-00000000000fffff: RESERVED [DEBUG] 3. 0000000000100000-0000000007f6bfff: RAM [DEBUG] 4. 0000000007f6c000-0000000007fa1fff: CONFIGURATION TABLES [DEBUG] 5. 0000000007fa2000-0000000007fcffff: RAMSTAGE [DEBUG] 6. 0000000007fd0000-0000000007ffffff: CONFIGURATION TABLES [DEBUG] 7. 00000000fec00000-00000000fec00fff: RESERVED [DEBUG] 8. 00000000ff800000-00000000ffffffff: RESERVED [DEBUG] FMAP: area COREBOOT found @ 200 (4193792 bytes) [DEBUG] Wrote coreboot table at: 0x07f99000, 0x2e8 bytes, checksum cb3f [DEBUG] coreboot table: 768 bytes. [DEBUG] IMD ROOT 0. 0x07fff000 0x00001000 [DEBUG] IMD SMALL 1. 0x07ffe000 0x00001000 [DEBUG] CONSOLE 2. 0x07fde000 0x00020000 [DEBUG] TIME STAMP 3. 0x07fdd000 0x00000910 [DEBUG] AFTER CAR 4. 0x07fd0000 0x0000d000 [DEBUG] RAMSTAGE 5. 0x07fa1000 0x0002f000 [DEBUG] COREBOOT 6. 0x07f99000 0x00008000 [DEBUG] IRQ TABLE 7. 0x07f98000 0x00001000 [DEBUG] ACPI 8. 0x07f74000 0x00024000 [DEBUG] SMBIOS 9. 0x07f6c000 0x00008000
Regards, Bin
Hi Bin,
On Sat, 6 May 2023 at 00:05, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Mar 24, 2023 at 1:55 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 20 Mar 2023 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Pick this up from the sysinfo tables and display it with the cbsysinfo command. This allows the 'acpi list' command to work when booting from coreboot.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 2 ++ arch/x86/include/asm/coreboot_tables.h | 2 ++ arch/x86/lib/coreboot/cb_sysinfo.c | 11 +++++++++++ cmd/x86/cbsysinfo.c | 1 + 4 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 0201ac6b03a..6b266149cf6 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -133,6 +133,7 @@
- @mtc_size: Size of MTC region
- @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning
not used
*/
- @rsdp: Pointer to ACPI RSDP table
struct sysinfo_t { unsigned int cpu_khz; @@ -211,6 +212,7 @@ struct sysinfo_t { u64 mtc_start; u32 mtc_size; void *chromeos_vpd;
void *rsdp;};
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index f131de56a40..2d6f3db3a5f 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -422,6 +422,8 @@ struct cb_tsc_info { #define CB_TAG_SERIALNO 0x002a #define CB_MAX_SERIALNO_LENGTH 32
+#define CB_TAG_ACPI_RSDP 0x0043
This is using space but should be tab.
#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
struct cb_cmos_option_table { diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index 748fa4ee53b..a11a2587f66 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -264,6 +264,13 @@ static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info) info->mrc_cache = map_sysmem(cbmem->cbmem_tab, 0); }
+static void cb_parse_acpi_rsdp(void *ptr, struct sysinfo_t *info) +{
struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;info->rsdp = map_sysmem(cbmem->cbmem_tab, 0);+}
__weak void cb_parse_unhandled(u32 tag, unsigned char *ptr) { } @@ -428,6 +435,9 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) case CB_TAG_MRC_CACHE: cb_parse_mrc_cache(rec, info); break;
case CB_TAG_ACPI_RSDP:cb_parse_acpi_rsdp(rec, info);break; default: cb_parse_unhandled(rec->tag, ptr); break;@@ -454,6 +464,7 @@ int get_coreboot_info(struct sysinfo_t *info) if (!ret) return -ENOENT; gd->arch.coreboot_table = addr;
gd_set_acpi_start(map_to_sysmem(info->rsdp)); gd->flags |= GD_FLG_SKIP_LL_INIT; return 0;Regards, Bin
diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 34fdaf5b1b1..07570b00c9a 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -363,6 +363,7 @@ static void show_table(struct sysinfo_t *info, bool verbose) print_hex("MTC size", info->mtc_size);
print_ptr("Chrome OS VPD", info->chromeos_vpd);
print_ptr("RSDP", info->rsdp);}
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
I tested this patch on top of coreboot with U-Boot as a payload running on QEMU i440fx, but U-Boot does not list acpi tables.
=> acpi list No ACPI tables present
Can you try 'cbsysinfo' to see what it is passing in as the RSDP?
'cbsysinfo' shows RSDP is 0.
=> cbsysinfo Coreboot table at 500, decoded to 06f558a0, forwarded to 07f99000 ... RSDP : 00000000 Unimpl. : 10 37 40 => acpi list No ACPI tables present
coreboot boot log says:
[DEBUG] Writing coreboot table at 0x07f99000 [DEBUG] 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES [DEBUG] 1. 0000000000001000-000000000009ffff: RAM [DEBUG] 2. 00000000000a0000-00000000000fffff: RESERVED [DEBUG] 3. 0000000000100000-0000000007f6bfff: RAM [DEBUG] 4. 0000000007f6c000-0000000007fa1fff: CONFIGURATION TABLES [DEBUG] 5. 0000000007fa2000-0000000007fcffff: RAMSTAGE [DEBUG] 6. 0000000007fd0000-0000000007ffffff: CONFIGURATION TABLES [DEBUG] 7. 00000000fec00000-00000000fec00fff: RESERVED [DEBUG] 8. 00000000ff800000-00000000ffffffff: RESERVED [DEBUG] FMAP: area COREBOOT found @ 200 (4193792 bytes) [DEBUG] Wrote coreboot table at: 0x07f99000, 0x2e8 bytes, checksum cb3f [DEBUG] coreboot table: 768 bytes. [DEBUG] IMD ROOT 0. 0x07fff000 0x00001000 [DEBUG] IMD SMALL 1. 0x07ffe000 0x00001000 [DEBUG] CONSOLE 2. 0x07fde000 0x00020000 [DEBUG] TIME STAMP 3. 0x07fdd000 0x00000910 [DEBUG] AFTER CAR 4. 0x07fd0000 0x0000d000 [DEBUG] RAMSTAGE 5. 0x07fa1000 0x0002f000 [DEBUG] COREBOOT 6. 0x07f99000 0x00008000 [DEBUG] IRQ TABLE 7. 0x07f98000 0x00001000 [DEBUG] ACPI 8. 0x07f74000 0x00024000 [DEBUG] SMBIOS 9. 0x07f6c000 0x00008000
Can you check why coreboot is not passing it through? It should provide the RSDP in LB_TAG_ACPI_RSDP
Regards, Simon
Hi Simon,
On Tue, May 9, 2023 at 5:23 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sat, 6 May 2023 at 00:05, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Mar 24, 2023 at 1:55 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 20 Mar 2023 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Pick this up from the sysinfo tables and display it with the cbsysinfo command. This allows the 'acpi list' command to work when booting from coreboot.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 2 ++ arch/x86/include/asm/coreboot_tables.h | 2 ++ arch/x86/lib/coreboot/cb_sysinfo.c | 11 +++++++++++ cmd/x86/cbsysinfo.c | 1 + 4 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 0201ac6b03a..6b266149cf6 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -133,6 +133,7 @@
- @mtc_size: Size of MTC region
- @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning
not used
*/
- @rsdp: Pointer to ACPI RSDP table
struct sysinfo_t { unsigned int cpu_khz; @@ -211,6 +212,7 @@ struct sysinfo_t { u64 mtc_start; u32 mtc_size; void *chromeos_vpd;
void *rsdp;};
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index f131de56a40..2d6f3db3a5f 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -422,6 +422,8 @@ struct cb_tsc_info { #define CB_TAG_SERIALNO 0x002a #define CB_MAX_SERIALNO_LENGTH 32
+#define CB_TAG_ACPI_RSDP 0x0043
This is using space but should be tab.
#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
struct cb_cmos_option_table { diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index 748fa4ee53b..a11a2587f66 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -264,6 +264,13 @@ static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info) info->mrc_cache = map_sysmem(cbmem->cbmem_tab, 0); }
+static void cb_parse_acpi_rsdp(void *ptr, struct sysinfo_t *info) +{
struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;info->rsdp = map_sysmem(cbmem->cbmem_tab, 0);+}
__weak void cb_parse_unhandled(u32 tag, unsigned char *ptr) { } @@ -428,6 +435,9 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) case CB_TAG_MRC_CACHE: cb_parse_mrc_cache(rec, info); break;
case CB_TAG_ACPI_RSDP:cb_parse_acpi_rsdp(rec, info);break; default: cb_parse_unhandled(rec->tag, ptr); break;@@ -454,6 +464,7 @@ int get_coreboot_info(struct sysinfo_t *info) if (!ret) return -ENOENT; gd->arch.coreboot_table = addr;
gd_set_acpi_start(map_to_sysmem(info->rsdp)); gd->flags |= GD_FLG_SKIP_LL_INIT; return 0;Regards, Bin
diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 34fdaf5b1b1..07570b00c9a 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -363,6 +363,7 @@ static void show_table(struct sysinfo_t *info, bool verbose) print_hex("MTC size", info->mtc_size);
print_ptr("Chrome OS VPD", info->chromeos_vpd);
print_ptr("RSDP", info->rsdp);}
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
I tested this patch on top of coreboot with U-Boot as a payload running on QEMU i440fx, but U-Boot does not list acpi tables.
=> acpi list No ACPI tables present
Can you try 'cbsysinfo' to see what it is passing in as the RSDP?
'cbsysinfo' shows RSDP is 0.
=> cbsysinfo Coreboot table at 500, decoded to 06f558a0, forwarded to 07f99000 ... RSDP : 00000000 Unimpl. : 10 37 40 => acpi list No ACPI tables present
coreboot boot log says:
[DEBUG] Writing coreboot table at 0x07f99000 [DEBUG] 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES [DEBUG] 1. 0000000000001000-000000000009ffff: RAM [DEBUG] 2. 00000000000a0000-00000000000fffff: RESERVED [DEBUG] 3. 0000000000100000-0000000007f6bfff: RAM [DEBUG] 4. 0000000007f6c000-0000000007fa1fff: CONFIGURATION TABLES [DEBUG] 5. 0000000007fa2000-0000000007fcffff: RAMSTAGE [DEBUG] 6. 0000000007fd0000-0000000007ffffff: CONFIGURATION TABLES [DEBUG] 7. 00000000fec00000-00000000fec00fff: RESERVED [DEBUG] 8. 00000000ff800000-00000000ffffffff: RESERVED [DEBUG] FMAP: area COREBOOT found @ 200 (4193792 bytes) [DEBUG] Wrote coreboot table at: 0x07f99000, 0x2e8 bytes, checksum cb3f [DEBUG] coreboot table: 768 bytes. [DEBUG] IMD ROOT 0. 0x07fff000 0x00001000 [DEBUG] IMD SMALL 1. 0x07ffe000 0x00001000 [DEBUG] CONSOLE 2. 0x07fde000 0x00020000 [DEBUG] TIME STAMP 3. 0x07fdd000 0x00000910 [DEBUG] AFTER CAR 4. 0x07fd0000 0x0000d000 [DEBUG] RAMSTAGE 5. 0x07fa1000 0x0002f000 [DEBUG] COREBOOT 6. 0x07f99000 0x00008000 [DEBUG] IRQ TABLE 7. 0x07f98000 0x00001000 [DEBUG] ACPI 8. 0x07f74000 0x00024000 [DEBUG] SMBIOS 9. 0x07f6c000 0x00008000
Can you check why coreboot is not passing it through? It should provide the RSDP in LB_TAG_ACPI_RSDP
Yes, I identified a bug in coreboot's write_acpi_tables() for QEMU. Will send a patch to coreboot community then.
Regards, Bin
Hi Bin,
On Mon, 8 May 2023 at 22:17, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, May 9, 2023 at 5:23 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sat, 6 May 2023 at 00:05, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Mar 24, 2023 at 1:55 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 20 Mar 2023 at 20:44, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Pick this up from the sysinfo tables and display it with the cbsysinfo command. This allows the 'acpi list' command to work when booting from coreboot.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 2 ++ arch/x86/include/asm/coreboot_tables.h | 2 ++ arch/x86/lib/coreboot/cb_sysinfo.c | 11 +++++++++++ cmd/x86/cbsysinfo.c | 1 + 4 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 0201ac6b03a..6b266149cf6 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -133,6 +133,7 @@
- @mtc_size: Size of MTC region
- @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning
not used
*/
- @rsdp: Pointer to ACPI RSDP table
struct sysinfo_t { unsigned int cpu_khz; @@ -211,6 +212,7 @@ struct sysinfo_t { u64 mtc_start; u32 mtc_size; void *chromeos_vpd;
void *rsdp;};
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index f131de56a40..2d6f3db3a5f 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -422,6 +422,8 @@ struct cb_tsc_info { #define CB_TAG_SERIALNO 0x002a #define CB_MAX_SERIALNO_LENGTH 32
+#define CB_TAG_ACPI_RSDP 0x0043
This is using space but should be tab.
#define CB_TAG_CMOS_OPTION_TABLE 0x00c8
struct cb_cmos_option_table { diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index 748fa4ee53b..a11a2587f66 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -264,6 +264,13 @@ static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info) info->mrc_cache = map_sysmem(cbmem->cbmem_tab, 0); }
+static void cb_parse_acpi_rsdp(void *ptr, struct sysinfo_t *info) +{
struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;info->rsdp = map_sysmem(cbmem->cbmem_tab, 0);+}
__weak void cb_parse_unhandled(u32 tag, unsigned char *ptr) { } @@ -428,6 +435,9 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) case CB_TAG_MRC_CACHE: cb_parse_mrc_cache(rec, info); break;
case CB_TAG_ACPI_RSDP:cb_parse_acpi_rsdp(rec, info);break; default: cb_parse_unhandled(rec->tag, ptr); break;@@ -454,6 +464,7 @@ int get_coreboot_info(struct sysinfo_t *info) if (!ret) return -ENOENT; gd->arch.coreboot_table = addr;
gd_set_acpi_start(map_to_sysmem(info->rsdp)); gd->flags |= GD_FLG_SKIP_LL_INIT; return 0;Regards, Bin
diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 34fdaf5b1b1..07570b00c9a 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -363,6 +363,7 @@ static void show_table(struct sysinfo_t *info, bool verbose) print_hex("MTC size", info->mtc_size);
print_ptr("Chrome OS VPD", info->chromeos_vpd);
print_ptr("RSDP", info->rsdp);}
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
I tested this patch on top of coreboot with U-Boot as a payload running on QEMU i440fx, but U-Boot does not list acpi tables.
=> acpi list No ACPI tables present
Can you try 'cbsysinfo' to see what it is passing in as the RSDP?
'cbsysinfo' shows RSDP is 0.
=> cbsysinfo Coreboot table at 500, decoded to 06f558a0, forwarded to 07f99000 ... RSDP : 00000000 Unimpl. : 10 37 40 => acpi list No ACPI tables present
coreboot boot log says:
[DEBUG] Writing coreboot table at 0x07f99000 [DEBUG] 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES [DEBUG] 1. 0000000000001000-000000000009ffff: RAM [DEBUG] 2. 00000000000a0000-00000000000fffff: RESERVED [DEBUG] 3. 0000000000100000-0000000007f6bfff: RAM [DEBUG] 4. 0000000007f6c000-0000000007fa1fff: CONFIGURATION TABLES [DEBUG] 5. 0000000007fa2000-0000000007fcffff: RAMSTAGE [DEBUG] 6. 0000000007fd0000-0000000007ffffff: CONFIGURATION TABLES [DEBUG] 7. 00000000fec00000-00000000fec00fff: RESERVED [DEBUG] 8. 00000000ff800000-00000000ffffffff: RESERVED [DEBUG] FMAP: area COREBOOT found @ 200 (4193792 bytes) [DEBUG] Wrote coreboot table at: 0x07f99000, 0x2e8 bytes, checksum cb3f [DEBUG] coreboot table: 768 bytes. [DEBUG] IMD ROOT 0. 0x07fff000 0x00001000 [DEBUG] IMD SMALL 1. 0x07ffe000 0x00001000 [DEBUG] CONSOLE 2. 0x07fde000 0x00020000 [DEBUG] TIME STAMP 3. 0x07fdd000 0x00000910 [DEBUG] AFTER CAR 4. 0x07fd0000 0x0000d000 [DEBUG] RAMSTAGE 5. 0x07fa1000 0x0002f000 [DEBUG] COREBOOT 6. 0x07f99000 0x00008000 [DEBUG] IRQ TABLE 7. 0x07f98000 0x00001000 [DEBUG] ACPI 8. 0x07f74000 0x00024000 [DEBUG] SMBIOS 9. 0x07f6c000 0x00008000
Can you check why coreboot is not passing it through? It should provide the RSDP in LB_TAG_ACPI_RSDP
Yes, I identified a bug in coreboot's write_acpi_tables() for QEMU. Will send a patch to coreboot community then.
OK, great, thanks. QEMU is quite different from real boards in how it handles the tables.
Regards, SImon
When coreboot does not pass a UART in its sysinfo struct, there is no easy way to find it out. Add a way to specify known UARTs so we can find them without needing help from coreboot.
Since coreboot does not actually init the serial device when serial is disabled, it is not possible to make it add this information to the sysinfo table.
Also, we cannot use the class information, since we don't know which UART is being used. For example, with Alder Lake there are two:
00.16.00 0x8086 0x51e0 Simple comm. controller 0x80 00.1e.00 0x8086 0x51a8 Simple comm. controller 0x80
In our case the second one is the right one, but thre is no way to distinguish it from the first one without using the device ID.
If we have Adler Lake hardware which uses a different UART, we could perhaps look at the ACPI tables, or the machine information passed in the SMBIOS tables.
This was discussed previously before: [1]
[1] https://patchwork.ozlabs.org/project/uboot/patch/ 20210407163159.3.I967ea8c85e009f870c7aa944372d32c990f1b14a@changeid/
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/dts/coreboot.dts | 4 ++ drivers/serial/serial_coreboot.c | 69 ++++++++++++++++++++++++++++---- include/pci_ids.h | 3 ++ 3 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/arch/x86/dts/coreboot.dts b/arch/x86/dts/coreboot.dts index d21978d6e09..3cd23b797ab 100644 --- a/arch/x86/dts/coreboot.dts +++ b/arch/x86/dts/coreboot.dts @@ -14,6 +14,8 @@ /include/ "rtc.dtsi"
#include "tsc_timer.dtsi" +#include <dt-bindings/pci/pci.h> +#include <pci_ids.h>
/ { model = "coreboot x86 payload"; @@ -34,6 +36,8 @@ pci { compatible = "pci-x86"; u-boot,dm-pre-reloc; + u-boot,pci-pre-reloc = < + PCI_VENDEV(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ADP_P_UART0) >; };
serial: serial { diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c index de09c8681f5..4d960bbe04d 100644 --- a/drivers/serial/serial_coreboot.c +++ b/drivers/serial/serial_coreboot.c @@ -11,19 +11,72 @@ #include <serial.h> #include <asm/cb_sysinfo.h>
+static const struct pci_device_id ids[] = { + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ADP_P_UART0) }, + {}, +}; + +/* + * Coreboot only sets up the UART if it uses it and doesn't bother to put the + * details in sysinfo if it doesn't. Try to guess in that case, using devices + * we know about + * + * @plat: Platform data to fill in + * @return 0 if found, -ve if no UART was found + */ +static int guess_uart(struct ns16550_plat *plat) +{ + struct udevice *bus, *dev; + ulong addr; + int index; + int ret; + + ret = uclass_first_device_err(UCLASS_PCI, &bus); + if (ret) + return ret; + index = 0; + ret = pci_bus_find_devices(bus, ids, &index, &dev); + if (ret) + return ret; + addr = dm_pci_read_bar32(dev, 0); + plat->base = addr; + plat->reg_shift = 2; + plat->reg_width = 4; + plat->clock = 1843200; + plat->fcr = UART_FCR_DEFVAL; + plat->flags = 0; + + return 0; +} + static int coreboot_of_to_plat(struct udevice *dev) { struct ns16550_plat *plat = dev_get_plat(dev); struct cb_serial *cb_info = lib_sysinfo.serial;
- plat->base = cb_info->baseaddr; - plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0; - plat->reg_width = cb_info->regwidth; - plat->clock = cb_info->input_hertz; - plat->fcr = UART_FCR_DEFVAL; - plat->flags = 0; - if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED) - plat->flags |= NS16550_FLAG_IO; + if (cb_info) { + plat->base = cb_info->baseaddr; + plat->reg_shift = cb_info->regwidth == 4 ? 2 : 0; + plat->reg_width = cb_info->regwidth; + plat->clock = cb_info->input_hertz; + plat->fcr = UART_FCR_DEFVAL; + plat->flags = 0; + if (cb_info->type == CB_SERIAL_TYPE_IO_MAPPED) + plat->flags |= NS16550_FLAG_IO; + } else if (CONFIG_IS_ENABLED(PCI)) { + int ret; + + ret = guess_uart(plat); + if (ret) { + /* + * Returning an error will cause U-Boot to complain that + * there is no UART, which may panic. So stay silent and + * pray that the video console will work. + */ + log_debug("Cannot detect UART\n"); + } + }
return 0; } diff --git a/include/pci_ids.h b/include/pci_ids.h index 88b0a640458..5ae1b9b7fb6 100644 --- a/include/pci_ids.h +++ b/include/pci_ids.h @@ -2992,6 +2992,9 @@ #define PCI_DEVICE_ID_INTEL_UNC_R3QPI1 0x3c45 #define PCI_DEVICE_ID_INTEL_JAKETOWN_UBOX 0x3ce0 #define PCI_DEVICE_ID_INTEL_IOAT_SNB 0x402f +#define PCI_DEVICE_ID_INTEL_ADP_P_UART0 0x51a8 +#define PCI_DEVICE_ID_INTEL_ADP_P_UART1 0x51a9 +#define PCI_DEVICE_ID_INTEL_APL_UART2 0x5ac0 #define PCI_DEVICE_ID_INTEL_5100_16 0x65f0 #define PCI_DEVICE_ID_INTEL_5100_19 0x65f3 #define PCI_DEVICE_ID_INTEL_5100_21 0x65f5
+Andy
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
When coreboot does not pass a UART in its sysinfo struct, there is no easy way to find it out. Add a way to specify known UARTs so we can find them without needing help from coreboot.
Since coreboot does not actually init the serial device when serial is disabled, it is not possible to make it add this information to the sysinfo table.
Also, we cannot use the class information, since we don't know which UART is being used. For example, with Alder Lake there are two:
00.16.00 0x8086 0x51e0 Simple comm. controller 0x80 00.1e.00 0x8086 0x51a8 Simple comm. controller 0x80
In our case the second one is the right one, but thre is no way to distinguish it from the first one without using the device ID.
If we have Adler Lake hardware which uses a different UART, we could perhaps look at the ACPI tables, or the machine information passed in the SMBIOS tables.
This was discussed previously before: [1]
[1] https://patchwork.ozlabs.org/project/uboot/patch/ 20210407163159.3.I967ea8c85e009f870c7aa944372d32c990f1b14a@changeid/
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/dts/coreboot.dts | 4 ++ drivers/serial/serial_coreboot.c | 69 ++++++++++++++++++++++++++++---- include/pci_ids.h | 3 ++ 3 files changed, 68 insertions(+), 8 deletions(-)
Last time we discussed this, both Andy and I thought this was a hack. I cited Andy's point below:
"What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this."
So maybe somehow we should parse the SPCR ACPI table instead?
Regards, Bin
Hi Bin,
On Mon, 20 Mar 2023 at 20:56, Bin Meng bmeng.cn@gmail.com wrote:
+Andy
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
When coreboot does not pass a UART in its sysinfo struct, there is no easy way to find it out. Add a way to specify known UARTs so we can find them without needing help from coreboot.
Since coreboot does not actually init the serial device when serial is disabled, it is not possible to make it add this information to the sysinfo table.
Also, we cannot use the class information, since we don't know which UART is being used. For example, with Alder Lake there are two:
00.16.00 0x8086 0x51e0 Simple comm. controller 0x80 00.1e.00 0x8086 0x51a8 Simple comm. controller 0x80
In our case the second one is the right one, but thre is no way to distinguish it from the first one without using the device ID.
If we have Adler Lake hardware which uses a different UART, we could perhaps look at the ACPI tables, or the machine information passed in the SMBIOS tables.
This was discussed previously before: [1]
[1] https://patchwork.ozlabs.org/project/uboot/patch/ 20210407163159.3.I967ea8c85e009f870c7aa944372d32c990f1b14a@changeid/
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/dts/coreboot.dts | 4 ++ drivers/serial/serial_coreboot.c | 69 ++++++++++++++++++++++++++++---- include/pci_ids.h | 3 ++ 3 files changed, 68 insertions(+), 8 deletions(-)
Last time we discussed this, both Andy and I thought this was a hack. I cited Andy's point below:
"What coreboot should do is either provide serial information or SPCR ACPI table. Otherwise if it does not provide it, I think it's on purpose, and we have to respect this."
We cannot change what coreboot does. I have written up a design for it but it seems unlikely that anything will happen there in the short term, perhaps ever. I will keep pushing.
In the meantime U-Boot cannot be used as a coreboot payload in this (common) situation. So we do need a solution.
So maybe somehow we should parse the SPCR ACPI table instead?
I don't see that table present, but there is DBG2 so I will take a look.
Regards, Simon
When U-Boot is the second-stage bootloader, PCI is already set up. We cannot read the regions from the device tree. There is no point anyway, since PCI devices have already been allocated according to the regions and it is not safe for U-Boot to make any changes.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/pci/pci-uclass.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 9343cfc62a9..8d27e40338c 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -973,6 +973,10 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node, int len; int i;
+ /* handle booting from coreboot, etc. */ + if (!ll_boot_init()) + return 0; + prop = ofnode_get_property(node, "ranges", &len); if (!prop) { debug("%s: Cannot decode regions\n", __func__);
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
When U-Boot is the second-stage bootloader, PCI is already set up. We cannot read the regions from the device tree. There is no point anyway, since PCI devices have already been allocated according to the regions and it is not safe for U-Boot to make any changes.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/pci/pci-uclass.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
When U-Boot is the second-stage bootloader, PCI is already set up. We cannot read the regions from the device tree. There is no point anyway, since PCI devices have already been allocated according to the regions and it is not safe for U-Boot to make any changes.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/pci/pci-uclass.c | 4 ++++ 1 file changed, 4 insertions(+)
Tested-by: Christian Gmeiner christian.gmeiner@gmail.com
This comes up repeatedly on Intel ADL. Use a debug message instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/usb/host/xhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dbeb88afe37..bd7e88b1769 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -885,8 +885,8 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
if ((req->requesttype & USB_RT_PORT) && le16_to_cpu(req->index) > max_ports) { - printf("The request port(%d) exceeds maximum port number\n", - le16_to_cpu(req->index) - 1); + log_debug("The request port(%d) exceeds maximum port number\n", + le16_to_cpu(req->index) - 1); return -EINVAL; }
From: Simon Glass sjg@chromium.org Date: Mon, 20 Feb 2023 12:49:22 -0700
This comes up repeatedly on Intel ADL. Use a debug message instead.
commit e330c8b83e87 fixed the (likely) root cause for this error message. Are you still seeing this message with that commit present?
Signed-off-by: Simon Glass sjg@chromium.org
drivers/usb/host/xhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dbeb88afe37..bd7e88b1769 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -885,8 +885,8 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
if ((req->requesttype & USB_RT_PORT) && le16_to_cpu(req->index) > max_ports) {
printf("The request port(%d) exceeds maximum port number\n",le16_to_cpu(req->index) - 1);
log_debug("The request port(%d) exceeds maximum port number\n",le16_to_cpu(req->index) - 1);-- 2.39.2.637.g21b0678d19-goog
Hi Mark,
On Mon, 20 Feb 2023 at 13:01, Mark Kettenis mark.kettenis@xs4all.nl wrote:
From: Simon Glass sjg@chromium.org Date: Mon, 20 Feb 2023 12:49:22 -0700
This comes up repeatedly on Intel ADL. Use a debug message instead.
commit e330c8b83e87 fixed the (likely) root cause for this error message. Are you still seeing this message with that commit present?
Yes it does, thank you! Will drop this patch.
Regards, Simon
Signed-off-by: Simon Glass sjg@chromium.org
drivers/usb/host/xhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dbeb88afe37..bd7e88b1769 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -885,8 +885,8 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
if ((req->requesttype & USB_RT_PORT) && le16_to_cpu(req->index) > max_ports) {
printf("The request port(%d) exceeds maximum port number\n",le16_to_cpu(req->index) - 1);
log_debug("The request port(%d) exceeds maximum port number\n",le16_to_cpu(req->index) - 1); return -EINVAL; }-- 2.39.2.637.g21b0678d19-goog
This is much more common on modern hardware, so default to using it.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/configs/coreboot.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/coreboot.h b/include/configs/coreboot.h index f73004386fd..2775f8d76ce 100644 --- a/include/configs/coreboot.h +++ b/include/configs/coreboot.h @@ -19,6 +19,8 @@ "stdout=serial,vidconsole\0" \ "stderr=serial,vidconsole\0"
+#undef CONFIG_SYS_NS16550_PORT_MAPPED + /* ATA/IDE support */
#endif /* __CONFIG_H */
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
This is much more common on modern hardware, so default to using it.
Signed-off-by: Simon Glass sjg@chromium.org
include/configs/coreboot.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/coreboot.h b/include/configs/coreboot.h index f73004386fd..2775f8d76ce 100644 --- a/include/configs/coreboot.h +++ b/include/configs/coreboot.h @@ -19,6 +19,8 @@ "stdout=serial,vidconsole\0" \ "stderr=serial,vidconsole\0"
+#undef CONFIG_SYS_NS16550_PORT_MAPPED
This macro is only meaningful for non-DM boards which is not the case for x86.
/* ATA/IDE support */
#endif /* __CONFIG_H */
Regards, Bin
On Mon, Mar 20, 2023 at 4:08 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
This is much more common on modern hardware, so default to using it.
Signed-off-by: Simon Glass sjg@chromium.org
include/configs/coreboot.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/coreboot.h b/include/configs/coreboot.h index f73004386fd..2775f8d76ce 100644 --- a/include/configs/coreboot.h +++ b/include/configs/coreboot.h @@ -19,6 +19,8 @@ "stdout=serial,vidconsole\0" \ "stderr=serial,vidconsole\0"
+#undef CONFIG_SYS_NS16550_PORT_MAPPED
This macro is only meaningful for non-DM boards which is not the case for x86.
for non-DM boards, or for boards that don't enable CONFIG_NS16550_DYNAMIC
Regards, Bin
This is not obvious so add a little note about how it works.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/board/coreboot/coreboot.rst | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/doc/board/coreboot/coreboot.rst b/doc/board/coreboot/coreboot.rst index 4a5f101cad2..0fe95af56d2 100644 --- a/doc/board/coreboot/coreboot.rst +++ b/doc/board/coreboot/coreboot.rst @@ -71,3 +71,32 @@ Memory map (typically redirects to 7ab10030 or similar) 500 Location of coreboot sysinfo table, used during startup ========== ================================================================== + + +Debug UART +---------- + +It is possible to enable the debug UART with coreboot. To do this, use the +info from the cbsysinfo command to locate the UART base. For example:: + + => cbsysinfo + ... + Serial I/O port: 00000000 + base : 00000000 + pointer : 767b51bc + type : 2 + base : fe03e000 + baud : 0d115200 + regwidth : 4 + input_hz : 0d1843200 + PCI addr : 00000010 + ... + +Here you can see that the UART base is fe03e000, regwidth is 4 (1 << 2) and the +input clock is 1843200. So you can add the following CONFIG options:: + + CONFIG_DEBUG_UART=y + CONFIG_DEBUG_UART_BASE=fe03e000 + CONFIG_DEBUG_UART_CLOCK=1843200 + CONFIG_DEBUG_UART_SHIFT=2 + CONFIG_DEBUG_UART_ANNOUNCE=y
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
This is not obvious so add a little note about how it works.
Signed-off-by: Simon Glass sjg@chromium.org
doc/board/coreboot/coreboot.rst | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Enable this so that PCI devices can be used correctly without needing to do a manual scan.
Signed-off-by: Simon Glass sjg@chromium.org ---
configs/coreboot_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/coreboot_defconfig b/configs/coreboot_defconfig index d8c5be66ad7..1c5e7fc717d 100644 --- a/configs/coreboot_defconfig +++ b/configs/coreboot_defconfig @@ -18,6 +18,7 @@ CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y +CONFIG_PCI_INIT_R=y CONFIG_HUSH_PARSER=y CONFIG_SYS_PBSIZE=532 CONFIG_CMD_IDE=y
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Enable this so that PCI devices can be used correctly without needing to do a manual scan.
Signed-off-by: Simon Glass sjg@chromium.org
configs/coreboot_defconfig | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Turn these options on to make it easier to debug things.
Also enable dhrystone so we can get some measure of performance.
Signed-off-by: Simon Glass sjg@chromium.org ---
configs/coreboot_defconfig | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/configs/coreboot_defconfig b/configs/coreboot_defconfig index 1c5e7fc717d..e6def760ada 100644 --- a/configs/coreboot_defconfig +++ b/configs/coreboot_defconfig @@ -16,6 +16,9 @@ CONFIG_USE_BOOTCOMMAND=y CONFIG_BOOTCOMMAND="ext2load scsi 0:3 01000000 /boot/vmlinuz; zboot 01000000" CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_SYS_CONSOLE_INFO_QUIET=y +CONFIG_LOG=y +CONFIG_LOGF_LINE=y +CONFIG_LOGF_FUNC=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y CONFIG_PCI_INIT_R=y @@ -59,5 +62,6 @@ CONFIG_SYS_64BIT_LBA=y CONFIG_SOUND=y CONFIG_SOUND_I8254=y CONFIG_CONSOLE_SCROLL_LINES=5 +CONFIG_CMD_DHRYSTONE=y # CONFIG_GZIP is not set CONFIG_SMBIOS_PARSER=y
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Turn these options on to make it easier to debug things.
Also enable dhrystone so we can get some measure of performance.
Signed-off-by: Simon Glass sjg@chromium.org
configs/coreboot_defconfig | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Sometimes coreboot adds new tags that U-Boot does not know about. These are silently ignored, but it is useful to at least know what we are missing.
Add a way to collect this information. For Brya it shows:
Unimpl. 38 41 37 34 42 40
These are:
LB_TAG_PLATFORM_BLOB_VERSION LB_TAG_ACPI_CNVS LB_TAG_FMAP LB_TAG_VBOOT_WORKBUF LB_TAG_TYPE_C_INFO LB_TAG_BOARD_CONFIG
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/include/asm/cb_sysinfo.h | 6 ++++++ arch/x86/lib/coreboot/cb_sysinfo.c | 2 ++ cmd/x86/cbsysinfo.c | 8 ++++++++ 3 files changed, 16 insertions(+)
diff --git a/arch/x86/include/asm/cb_sysinfo.h b/arch/x86/include/asm/cb_sysinfo.h index 6b266149cf6..2c78b22d0d2 100644 --- a/arch/x86/include/asm/cb_sysinfo.h +++ b/arch/x86/include/asm/cb_sysinfo.h @@ -16,6 +16,8 @@ #define SYSINFO_MAX_GPIOS 8 /* Up to 10 MAC addresses */ #define SYSINFO_MAX_MACS 10 +/* Track the first 32 unimplemented tags */ +#define SYSINFO_MAX_UNIMPL 32
/** * struct sysinfo_t - Information passed to U-Boot from coreboot @@ -134,6 +136,8 @@ * @chromeos_vpd: Chromium OS Vital Product Data region, typically NULL, meaning * not used * @rsdp: Pointer to ACPI RSDP table + * @unimpl_count: Number of entries in unimpl_map[] + * @unimpl: List of unimplemented IDs (bottom 8 bits only) */ struct sysinfo_t { unsigned int cpu_khz; @@ -213,6 +217,8 @@ struct sysinfo_t { u32 mtc_size; void *chromeos_vpd; void *rsdp; + u32 unimpl_count; + u8 unimpl[SYSINFO_MAX_UNIMPL]; };
extern struct sysinfo_t lib_sysinfo; diff --git a/arch/x86/lib/coreboot/cb_sysinfo.c b/arch/x86/lib/coreboot/cb_sysinfo.c index a11a2587f66..42cc3a128d9 100644 --- a/arch/x86/lib/coreboot/cb_sysinfo.c +++ b/arch/x86/lib/coreboot/cb_sysinfo.c @@ -439,6 +439,8 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info) cb_parse_acpi_rsdp(rec, info); break; default: + if (info->unimpl_count < SYSINFO_MAX_UNIMPL) + info->unimpl[info->unimpl_count++] = rec->tag; cb_parse_unhandled(rec->tag, ptr); break; } diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c index 07570b00c9a..2b8d3b0a435 100644 --- a/cmd/x86/cbsysinfo.c +++ b/cmd/x86/cbsysinfo.c @@ -364,6 +364,14 @@ static void show_table(struct sysinfo_t *info, bool verbose)
print_ptr("Chrome OS VPD", info->chromeos_vpd); print_ptr("RSDP", info->rsdp); + printf("%-12s: ", "Unimpl."); + if (info->unimpl_count) { + for (i = 0; i < info->unimpl_count; i++) + printf("%02x ", info->unimpl[i]); + printf("\n"); + } else { + printf("(none)\n"); + } }
static int do_cbsysinfo(struct cmd_tbl *cmdtp, int flag, int argc,
On Tue, Feb 21, 2023 at 3:49 AM Simon Glass sjg@chromium.org wrote:
Sometimes coreboot adds new tags that U-Boot does not know about. These are silently ignored, but it is useful to at least know what we are missing.
Add a way to collect this information. For Brya it shows:
Unimpl. 38 41 37 34 42 40
These are:
LB_TAG_PLATFORM_BLOB_VERSION LB_TAG_ACPI_CNVS LB_TAG_FMAP LB_TAG_VBOOT_WORKBUF LB_TAG_TYPE_C_INFO LB_TAG_BOARD_CONFIG
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/include/asm/cb_sysinfo.h | 6 ++++++ arch/x86/lib/coreboot/cb_sysinfo.c | 2 ++ cmd/x86/cbsysinfo.c | 8 ++++++++ 3 files changed, 16 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: 2023年2月21日 3:49 To: U-Boot Mailing List u-boot@lists.denx.de Cc: Bin Meng bmeng.cn@gmail.com; Simon Glass sjg@chromium.org; AKASHI Takahiro takahiro.akashi@linaro.org; Andrew Scull ascull@google.com; Heinrich Schuchardt xypron.glpk@gmx.de; Ilias Apalodimas ilias.apalodimas@linaro.org; Jason Liu
John Keeping john@metanate.com; Marek Vasut marex@denx.de; Masahisa Kojima masahisa.kojima@linaro.org; Michal Suchanek msuchanek@suse.de; Pali Rohár pali@kernel.org; Pierre-Clément Tosi ptosi@google.com; Rasmus Villemoes rasmus.villemoes@prevas.dk; Stefan Roese sr@denx.de Subject: [PATCH 00/13] x86: Various minor enhancements for coreboot
This series includes some patches generated while getting U-Boot to boot
more
nicely on Brya, an Adler Lake Chromebook.
This includes:
- show the ACPI tables with 'acpi list'
- get the UART to work even if coreboot doesn't enable it
- show unimplemented sysinfo tags
- fix for keyboard not working
- fix for trying to set up PCI regions when the info is not available
- fix for looking at inaccessible memory to find the sysinfo table
Simon Glass (13): mtrr: Don't show an invalid CPU number x86: Adjust search range for sysinfo table input: Only reset the keyboard when running bare metal x86: coreboot: Allow ACPI tables to be recorded x86: coreboot: Collect the address of the ACPI tables x86: Allow locating UARTs by device ID pci: coreboot: Don't read regions when booting usb: Quieten a debug message x86: coreboot: Use a memory-mapped UART x86: coreboot: Document how to enable the debug UART x86: coreboot: Scan PCI after relocation x86: coreboot: Log function names and line numbers x86: coreboot: Show unimplemented sysinfo tags
This patch-set looks fine to me, thus,
Reviewed-by: Jason Liu jason.hui.liu@nxp.com
arch/x86/cpu/cpu.c | 2 +- arch/x86/dts/coreboot.dts | 4 ++ arch/x86/include/asm/cb_sysinfo.h | 8 +++ arch/x86/include/asm/coreboot_tables.h | 2 + arch/x86/lib/coreboot/cb_sysinfo.c | 13 +++++ cmd/Kconfig | 3 +- cmd/acpi.c | 4 ++ cmd/x86/cbsysinfo.c | 9 ++++ cmd/x86/mtrr.c | 3 +- configs/coreboot_defconfig | 5 ++ doc/board/coreboot/coreboot.rst | 29 +++++++++++ drivers/input/i8042.c | 13 +++-- drivers/pci/pci-uclass.c | 4 ++ drivers/serial/serial_coreboot.c | 69 +++++++++++++++++++++++--- drivers/usb/host/xhci.c | 4 +- include/asm-generic/global_data.h | 4 +- include/configs/coreboot.h | 2 + include/pci_ids.h | 3 ++ 18 files changed, 161 insertions(+), 20 deletions(-)
-- 2.39.2.637.g21b0678d19-goog
Hi Bin,
On Mon, 20 Feb 2023 at 19:02, Jason Liu jason.hui.liu@nxp.com wrote:
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: 2023年2月21日 3:49 To: U-Boot Mailing List u-boot@lists.denx.de Cc: Bin Meng bmeng.cn@gmail.com; Simon Glass sjg@chromium.org; AKASHI Takahiro takahiro.akashi@linaro.org; Andrew Scull ascull@google.com; Heinrich Schuchardt xypron.glpk@gmx.de; Ilias Apalodimas ilias.apalodimas@linaro.org; Jason Liu
John Keeping john@metanate.com; Marek Vasut marex@denx.de; Masahisa Kojima masahisa.kojima@linaro.org; Michal Suchanek msuchanek@suse.de; Pali Rohár pali@kernel.org; Pierre-Clément Tosi ptosi@google.com; Rasmus Villemoes rasmus.villemoes@prevas.dk; Stefan Roese sr@denx.de Subject: [PATCH 00/13] x86: Various minor enhancements for coreboot
This series includes some patches generated while getting U-Boot to boot
more
nicely on Brya, an Adler Lake Chromebook.
This includes:
- show the ACPI tables with 'acpi list'
- get the UART to work even if coreboot doesn't enable it
- show unimplemented sysinfo tags
- fix for keyboard not working
- fix for trying to set up PCI regions when the info is not available
- fix for looking at inaccessible memory to find the sysinfo table
Simon Glass (13): mtrr: Don't show an invalid CPU number x86: Adjust search range for sysinfo table input: Only reset the keyboard when running bare metal x86: coreboot: Allow ACPI tables to be recorded x86: coreboot: Collect the address of the ACPI tables x86: Allow locating UARTs by device ID pci: coreboot: Don't read regions when booting usb: Quieten a debug message x86: coreboot: Use a memory-mapped UART x86: coreboot: Document how to enable the debug UART x86: coreboot: Scan PCI after relocation x86: coreboot: Log function names and line numbers x86: coreboot: Show unimplemented sysinfo tags
This patch-set looks fine to me, thus,
Reviewed-by: Jason Liu jason.hui.liu@nxp.com
Any thoughts on this series please?
We need to drop the usb one as the bug has been fixed.
Regards, Simon
participants (5)
-
Bin Meng -
Christian Gmeiner -
Jason Liu -
Mark Kettenis -
Simon Glass