All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/13] x86: Various minor enhancements for coreboot
@ 2023-02-20 19:49 Simon Glass
  2023-02-20 19:49 ` [PATCH 01/13] mtrr: Don't show an invalid CPU number Simon Glass
                   ` (13 more replies)
  0 siblings, 14 replies; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Bin Meng, Simon Glass, AKASHI Takahiro, Andrew Scull,
	Heinrich Schuchardt, Ilias Apalodimas, Jason Liu, John Keeping,
	Marek Vasut, Masahisa Kojima, Michal Suchanek, Pali Rohár,
	Pierre-Clément Tosi, Rasmus Villemoes, Stefan Roese

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(-)

-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 01/13] mtrr: Don't show an invalid CPU number
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  6:23   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 02/13] x86: Adjust search range for sysinfo table Simon Glass
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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,
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 02/13] x86: Adjust search range for sysinfo table
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
  2023-02-20 19:49 ` [PATCH 01/13] mtrr: Don't show an invalid CPU number Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  6:30   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 03/13] input: Only reset the keyboard when running bare metal Simon Glass
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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);
 
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 03/13] input: Only reset the keyboard when running bare metal
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
  2023-02-20 19:49 ` [PATCH 01/13] mtrr: Don't show an invalid CPU number Simon Glass
  2023-02-20 19:49 ` [PATCH 02/13] x86: Adjust search range for sysinfo table Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  6:32   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 04/13] x86: coreboot: Allow ACPI tables to be recorded Simon Glass
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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)
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 04/13] x86: coreboot: Allow ACPI tables to be recorded
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (2 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 03/13] input: Only reset the keyboard when running bare metal Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-02-20 19:49 ` [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables Simon Glass
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Bin Meng, Simon Glass, AKASHI Takahiro, Heinrich Schuchardt,
	Ilias Apalodimas, Jason Liu, John Keeping, Masahisa Kojima,
	Pali Rohár, Rasmus Villemoes, Stefan Roese

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
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (3 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 04/13] x86: coreboot: Allow ACPI tables to be recorded Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  7:44   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 06/13] x86: Allow locating UARTs by device ID Simon Glass
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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,
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 06/13] x86: Allow locating UARTs by device ID
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (4 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  7:56   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 07/13] pci: coreboot: Don't read regions when booting Simon Glass
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass, Pali Rohár, Stefan Roese

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
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 07/13] pci: coreboot: Don't read regions when booting
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (5 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 06/13] x86: Allow locating UARTs by device ID Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  7:58   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 08/13] usb: Quieten a debug message Simon Glass
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Bin Meng, Simon Glass, Andrew Scull, Michal Suchanek,
	Pali Rohár, Pierre-Clément Tosi, Stefan Roese

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__);
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 08/13] usb: Quieten a debug message
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (6 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 07/13] pci: coreboot: Don't read regions when booting Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-02-20 20:01   ` Mark Kettenis
  2023-02-20 19:49 ` [PATCH 09/13] x86: coreboot: Use a memory-mapped UART Simon Glass
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass, Marek Vasut

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;
 	}
 
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 09/13] x86: coreboot: Use a memory-mapped UART
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (7 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 08/13] usb: Quieten a debug message Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  8:08   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 10/13] x86: coreboot: Document how to enable the debug UART Simon Glass
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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 */
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 10/13] x86: coreboot: Document how to enable the debug UART
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (8 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 09/13] x86: coreboot: Use a memory-mapped UART Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  8:14   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 11/13] x86: coreboot: Scan PCI after relocation Simon Glass
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 11/13] x86: coreboot: Scan PCI after relocation
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (9 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 10/13] x86: coreboot: Document how to enable the debug UART Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  8:15   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 12/13] x86: coreboot: Log function names and line numbers Simon Glass
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 12/13] x86: coreboot: Log function names and line numbers
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (10 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 11/13] x86: coreboot: Scan PCI after relocation Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  8:18   ` Bin Meng
  2023-02-20 19:49 ` [PATCH 13/13] x86: coreboot: Show unimplemented sysinfo tags Simon Glass
  2023-02-21  2:02 ` [PATCH 00/13] x86: Various minor enhancements for coreboot Jason Liu
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 13/13] x86: coreboot: Show unimplemented sysinfo tags
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (11 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 12/13] x86: coreboot: Log function names and line numbers Simon Glass
@ 2023-02-20 19:49 ` Simon Glass
  2023-03-20  8:19   ` Bin Meng
  2023-02-21  2:02 ` [PATCH 00/13] x86: Various minor enhancements for coreboot Jason Liu
  13 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-02-20 19:49 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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,
-- 
2.39.2.637.g21b0678d19-goog


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH 08/13] usb: Quieten a debug message
  2023-02-20 19:49 ` [PATCH 08/13] usb: Quieten a debug message Simon Glass
@ 2023-02-20 20:01   ` Mark Kettenis
  2023-02-20 23:45     ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Mark Kettenis @ 2023-02-20 20:01 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, bmeng.cn, sjg, marex

> 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);
>  		return -EINVAL;
>  	}
>  
> -- 
> 2.39.2.637.g21b0678d19-goog
> 
> 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 08/13] usb: Quieten a debug message
  2023-02-20 20:01   ` Mark Kettenis
@ 2023-02-20 23:45     ` Simon Glass
  0 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2023-02-20 23:45 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: u-boot, bmeng.cn, marex

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
> >
> >

^ permalink raw reply	[flat|nested] 41+ messages in thread

* RE: [PATCH 00/13] x86: Various minor enhancements for coreboot
  2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
                   ` (12 preceding siblings ...)
  2023-02-20 19:49 ` [PATCH 13/13] x86: coreboot: Show unimplemented sysinfo tags Simon Glass
@ 2023-02-21  2:02 ` Jason Liu
  2023-03-18 20:20   ` Simon Glass
  13 siblings, 1 reply; 41+ messages in thread
From: Jason Liu @ 2023-02-21  2:02 UTC (permalink / raw)
  To: Simon Glass, U-Boot Mailing List
  Cc: Bin Meng, AKASHI Takahiro, Andrew Scull, Heinrich Schuchardt,
	Ilias Apalodimas, John Keeping, Marek Vasut, Masahisa Kojima,
	Michal Suchanek, Pali Rohár, Pierre-Clément Tosi,
	Rasmus Villemoes, Stefan Roese

[-- Attachment #1: Type: text/plain, Size: 3121 bytes --]



> -----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
<jason.hui.liu@nxp.com>;
> 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


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9729 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 00/13] x86: Various minor enhancements for coreboot
  2023-02-21  2:02 ` [PATCH 00/13] x86: Various minor enhancements for coreboot Jason Liu
@ 2023-03-18 20:20   ` Simon Glass
  0 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2023-03-18 20:20 UTC (permalink / raw)
  To: Jason Liu
  Cc: U-Boot Mailing List, Bin Meng, AKASHI Takahiro, Andrew Scull,
	Heinrich Schuchardt, Ilias Apalodimas, John Keeping, Marek Vasut,
	Masahisa Kojima, Michal Suchanek, Pali Rohár,
	Pierre-Clément Tosi, Rasmus Villemoes, Stefan Roese

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
> <jason.hui.liu@nxp.com>;
> > 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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/13] mtrr: Don't show an invalid CPU number
  2023-02-20 19:49 ` [PATCH 01/13] mtrr: Don't show an invalid CPU number Simon Glass
@ 2023-03-20  6:23   ` Bin Meng
  0 siblings, 0 replies; 41+ messages in thread
From: Bin Meng @ 2023-03-20  6:23 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 02/13] x86: Adjust search range for sysinfo table
  2023-02-20 19:49 ` [PATCH 02/13] x86: Adjust search range for sysinfo table Simon Glass
@ 2023-03-20  6:30   ` Bin Meng
  0 siblings, 0 replies; 41+ messages in thread
From: Bin Meng @ 2023-03-20  6:30 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 03/13] input: Only reset the keyboard when running bare metal
  2023-02-20 19:49 ` [PATCH 03/13] input: Only reset the keyboard when running bare metal Simon Glass
@ 2023-03-20  6:32   ` Bin Meng
  2023-03-20 18:40     ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-03-20  6:32 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables
  2023-02-20 19:49 ` [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables Simon Glass
@ 2023-03-20  7:44   ` Bin Meng
  2023-03-23 17:55     ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-03-20  7:44 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 06/13] x86: Allow locating UARTs by device ID
  2023-02-20 19:49 ` [PATCH 06/13] x86: Allow locating UARTs by device ID Simon Glass
@ 2023-03-20  7:56   ` Bin Meng
  2023-03-23 18:28     ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-03-20  7:56 UTC (permalink / raw)
  To: Simon Glass, Andy Shevchenko
  Cc: U-Boot Mailing List, Pali Rohár, Stefan Roese

+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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 07/13] pci: coreboot: Don't read regions when booting
  2023-02-20 19:49 ` [PATCH 07/13] pci: coreboot: Don't read regions when booting Simon Glass
@ 2023-03-20  7:58   ` Bin Meng
  2023-03-22 15:54     ` Christian Gmeiner
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-03-20  7:58 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Andrew Scull, Michal Suchanek,
	Pali Rohár, Pierre-Clément Tosi, Stefan Roese

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>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 09/13] x86: coreboot: Use a memory-mapped UART
  2023-02-20 19:49 ` [PATCH 09/13] x86: coreboot: Use a memory-mapped UART Simon Glass
@ 2023-03-20  8:08   ` Bin Meng
  2023-03-20  8:10     ` Bin Meng
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-03-20  8:08 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 09/13] x86: coreboot: Use a memory-mapped UART
  2023-03-20  8:08   ` Bin Meng
@ 2023-03-20  8:10     ` Bin Meng
  0 siblings, 0 replies; 41+ messages in thread
From: Bin Meng @ 2023-03-20  8:10 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 10/13] x86: coreboot: Document how to enable the debug UART
  2023-02-20 19:49 ` [PATCH 10/13] x86: coreboot: Document how to enable the debug UART Simon Glass
@ 2023-03-20  8:14   ` Bin Meng
  0 siblings, 0 replies; 41+ messages in thread
From: Bin Meng @ 2023-03-20  8:14 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 11/13] x86: coreboot: Scan PCI after relocation
  2023-02-20 19:49 ` [PATCH 11/13] x86: coreboot: Scan PCI after relocation Simon Glass
@ 2023-03-20  8:15   ` Bin Meng
  0 siblings, 0 replies; 41+ messages in thread
From: Bin Meng @ 2023-03-20  8:15 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 12/13] x86: coreboot: Log function names and line numbers
  2023-02-20 19:49 ` [PATCH 12/13] x86: coreboot: Log function names and line numbers Simon Glass
@ 2023-03-20  8:18   ` Bin Meng
  0 siblings, 0 replies; 41+ messages in thread
From: Bin Meng @ 2023-03-20  8:18 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 13/13] x86: coreboot: Show unimplemented sysinfo tags
  2023-02-20 19:49 ` [PATCH 13/13] x86: coreboot: Show unimplemented sysinfo tags Simon Glass
@ 2023-03-20  8:19   ` Bin Meng
  0 siblings, 0 replies; 41+ messages in thread
From: Bin Meng @ 2023-03-20  8:19 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 03/13] input: Only reset the keyboard when running bare metal
  2023-03-20  6:32   ` Bin Meng
@ 2023-03-20 18:40     ` Simon Glass
  2023-03-21  1:24       ` Bin Meng
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-03-20 18:40 UTC (permalink / raw)
  To: Bin Meng; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 03/13] input: Only reset the keyboard when running bare metal
  2023-03-20 18:40     ` Simon Glass
@ 2023-03-21  1:24       ` Bin Meng
  2023-03-23 17:55         ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-03-21  1:24 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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?

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 07/13] pci: coreboot: Don't read regions when booting
  2023-03-20  7:58   ` Bin Meng
@ 2023-03-22 15:54     ` Christian Gmeiner
  0 siblings, 0 replies; 41+ messages in thread
From: Christian Gmeiner @ 2023-03-22 15:54 UTC (permalink / raw)
  To: Bin Meng
  Cc: Simon Glass, U-Boot Mailing List, Andrew Scull, Michal Suchanek,
	Pali Rohár, Pierre-Clément Tosi, Stefan Roese

> > 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>

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables
  2023-03-20  7:44   ` Bin Meng
@ 2023-03-23 17:55     ` Simon Glass
  2023-05-06  6:04       ` Bin Meng
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-03-23 17:55 UTC (permalink / raw)
  To: Bin Meng; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 03/13] input: Only reset the keyboard when running bare metal
  2023-03-21  1:24       ` Bin Meng
@ 2023-03-23 17:55         ` Simon Glass
  2023-04-19  1:46           ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-03-23 17:55 UTC (permalink / raw)
  To: Bin Meng; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 06/13] x86: Allow locating UARTs by device ID
  2023-03-20  7:56   ` Bin Meng
@ 2023-03-23 18:28     ` Simon Glass
  0 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2023-03-23 18:28 UTC (permalink / raw)
  To: Bin Meng
  Cc: Andy Shevchenko, U-Boot Mailing List, Pali Rohár, Stefan Roese

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 03/13] input: Only reset the keyboard when running bare metal
  2023-03-23 17:55         ` Simon Glass
@ 2023-04-19  1:46           ` Simon Glass
  0 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2023-04-19  1:46 UTC (permalink / raw)
  To: Bin Meng; +Cc: U-Boot Mailing List

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#L58

Regards,
Simon

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables
  2023-03-23 17:55     ` Simon Glass
@ 2023-05-06  6:04       ` Bin Meng
  2023-05-08 21:23         ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-05-06  6:04 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables
  2023-05-06  6:04       ` Bin Meng
@ 2023-05-08 21:23         ` Simon Glass
  2023-05-09  4:17           ` Bin Meng
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2023-05-08 21:23 UTC (permalink / raw)
  To: Bin Meng; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables
  2023-05-08 21:23         ` Simon Glass
@ 2023-05-09  4:17           ` Bin Meng
  2023-05-09 21:09             ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Bin Meng @ 2023-05-09  4:17 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables
  2023-05-09  4:17           ` Bin Meng
@ 2023-05-09 21:09             ` Simon Glass
  0 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2023-05-09 21:09 UTC (permalink / raw)
  To: Bin Meng; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2023-05-09 21:09 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-20 19:49 [PATCH 00/13] x86: Various minor enhancements for coreboot Simon Glass
2023-02-20 19:49 ` [PATCH 01/13] mtrr: Don't show an invalid CPU number Simon Glass
2023-03-20  6:23   ` Bin Meng
2023-02-20 19:49 ` [PATCH 02/13] x86: Adjust search range for sysinfo table Simon Glass
2023-03-20  6:30   ` Bin Meng
2023-02-20 19:49 ` [PATCH 03/13] input: Only reset the keyboard when running bare metal Simon Glass
2023-03-20  6:32   ` Bin Meng
2023-03-20 18:40     ` Simon Glass
2023-03-21  1:24       ` Bin Meng
2023-03-23 17:55         ` Simon Glass
2023-04-19  1:46           ` Simon Glass
2023-02-20 19:49 ` [PATCH 04/13] x86: coreboot: Allow ACPI tables to be recorded Simon Glass
2023-02-20 19:49 ` [PATCH 05/13] x86: coreboot: Collect the address of the ACPI tables Simon Glass
2023-03-20  7:44   ` Bin Meng
2023-03-23 17:55     ` Simon Glass
2023-05-06  6:04       ` Bin Meng
2023-05-08 21:23         ` Simon Glass
2023-05-09  4:17           ` Bin Meng
2023-05-09 21:09             ` Simon Glass
2023-02-20 19:49 ` [PATCH 06/13] x86: Allow locating UARTs by device ID Simon Glass
2023-03-20  7:56   ` Bin Meng
2023-03-23 18:28     ` Simon Glass
2023-02-20 19:49 ` [PATCH 07/13] pci: coreboot: Don't read regions when booting Simon Glass
2023-03-20  7:58   ` Bin Meng
2023-03-22 15:54     ` Christian Gmeiner
2023-02-20 19:49 ` [PATCH 08/13] usb: Quieten a debug message Simon Glass
2023-02-20 20:01   ` Mark Kettenis
2023-02-20 23:45     ` Simon Glass
2023-02-20 19:49 ` [PATCH 09/13] x86: coreboot: Use a memory-mapped UART Simon Glass
2023-03-20  8:08   ` Bin Meng
2023-03-20  8:10     ` Bin Meng
2023-02-20 19:49 ` [PATCH 10/13] x86: coreboot: Document how to enable the debug UART Simon Glass
2023-03-20  8:14   ` Bin Meng
2023-02-20 19:49 ` [PATCH 11/13] x86: coreboot: Scan PCI after relocation Simon Glass
2023-03-20  8:15   ` Bin Meng
2023-02-20 19:49 ` [PATCH 12/13] x86: coreboot: Log function names and line numbers Simon Glass
2023-03-20  8:18   ` Bin Meng
2023-02-20 19:49 ` [PATCH 13/13] x86: coreboot: Show unimplemented sysinfo tags Simon Glass
2023-03-20  8:19   ` Bin Meng
2023-02-21  2:02 ` [PATCH 00/13] x86: Various minor enhancements for coreboot Jason Liu
2023-03-18 20:20   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.