All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/4] add support for loading ACPI tables from QEMU
@ 2016-01-15  3:12 Miao Yan
  2016-01-15  3:12 ` [U-Boot] [PATCH 1/4] x86: qemu: re-structure qemu_fwcfg_list_firmware() Miao Yan
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Miao Yan @ 2016-01-15  3:12 UTC (permalink / raw)
  To: u-boot

Currently, if CONFIG_GENERATE_ACPI_TABLE is defined, U-Boot will generate ACPI
tables itlself, this patchset adds the ability to load the ACPI tables generated
by QEMU.

Miao Yan (4):
  x86: qemu: re-structure qemu_fwcfg_list_firmware()
  x86: qemu: setup PM IO base for ACPI in southbridge
  x86: qemu: add the ability to load and link ACPI tables from QEMU
  x86: qemu: loading ACPI table from QEMU

 arch/x86/cpu/qemu/fw_cfg.c              | 274 ++++++++++++++++++++++++++++++--
 arch/x86/cpu/qemu/qemu.c                |  39 +++++
 arch/x86/include/asm/arch-qemu/device.h |   8 +
 arch/x86/include/asm/fw_cfg.h           |  78 +++++++++
 arch/x86/lib/tables.c                   |   5 +-
 5 files changed, 389 insertions(+), 15 deletions(-)

-- 
1.9.1

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

* [U-Boot] [PATCH 1/4] x86: qemu: re-structure qemu_fwcfg_list_firmware()
  2016-01-15  3:12 [U-Boot] [PATCH 0/4] add support for loading ACPI tables from QEMU Miao Yan
@ 2016-01-15  3:12 ` Miao Yan
  2016-01-16 13:23   ` Bin Meng
  2016-01-15  3:12 ` [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge Miao Yan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Miao Yan @ 2016-01-15  3:12 UTC (permalink / raw)
  To: u-boot

Re-write the logic in qemu_fwcfg_list_firmware(), add a function
qemu_cfg_read_firmware_list() to handle reading firmware list.

Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
 arch/x86/cpu/qemu/fw_cfg.c    | 60 +++++++++++++++++++++++++++++++++----------
 arch/x86/include/asm/fw_cfg.h |  8 ++++++
 2 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c
index 0599214..b22026c 100644
--- a/arch/x86/cpu/qemu/fw_cfg.c
+++ b/arch/x86/cpu/qemu/fw_cfg.c
@@ -10,10 +10,13 @@
 #include <malloc.h>
 #include <asm/io.h>
 #include <asm/fw_cfg.h>
+#include <linux/list.h>
 
 static bool fwcfg_present;
 static bool fwcfg_dma_present;
 
+static LIST_HEAD(fw_list);
+
 /* Read configuration item using fw_cfg PIO interface */
 static void qemu_fwcfg_read_entry_pio(uint16_t entry,
 		uint32_t size, void *address)
@@ -162,29 +165,58 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
 	return 0;
 }
 
-static int qemu_fwcfg_list_firmware(void)
+static int qemu_fwcfg_read_firmware_list(void)
 {
 	int i;
 	uint32_t count;
-	struct fw_cfg_files *files;
+	struct fw_file *file;
+	struct list_head *entry;
+
+	/* don't read it twice */
+	if (!list_empty(&fw_list))
+		return 0;
 
 	qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count);
 	if (!count)
 		return 0;
 
 	count = be32_to_cpu(count);
-	files = malloc(count * sizeof(struct fw_cfg_file));
-	if (!files)
-		return -ENOMEM;
-
-	files->count = count;
-	qemu_fwcfg_read_entry(FW_CFG_INVALID,
-			      count * sizeof(struct fw_cfg_file),
-			      files->files);
-
-	for (i = 0; i < files->count; i++)
-		printf("%-56s\n", files->files[i].name);
-	free(files);
+	for (i = 0; i < count; i++) {
+		file = malloc(sizeof(*file));
+		if (!file) {
+			printf("error: allocating resource\n");
+			goto err;
+		}
+		qemu_fwcfg_read_entry(FW_CFG_INVALID,
+				      sizeof(struct fw_cfg_file), &file->cfg);
+		file->addr = 0;
+		list_add_tail(&file->list, &fw_list);
+	}
+	return 0;
+
+err:
+	list_for_each(entry, &fw_list) {
+		file = list_entry(entry, struct fw_file, list);
+		free(file);
+	}
+	return -ENOMEM;
+}
+
+static int qemu_fwcfg_list_firmware(void)
+{
+	int ret;
+	struct list_head *entry;
+	struct fw_file *file;
+
+	/* make sure fw_list is loaded */
+	ret = qemu_fwcfg_read_firmware_list();
+	if (ret)
+		return ret;
+
+	list_for_each(entry, &fw_list) {
+		file = list_entry(entry, struct fw_file, list);
+		printf("%-56s\n", file->cfg.name);
+	}
 	return 0;
 }
 
diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h
index fb110fa..285d805 100644
--- a/arch/x86/include/asm/fw_cfg.h
+++ b/arch/x86/include/asm/fw_cfg.h
@@ -12,6 +12,8 @@
 #define FW_DMA_PORT_LOW	0x514
 #define FW_DMA_PORT_HIGH	0x518
 
+#include <linux/list.h>
+
 enum qemu_fwcfg_items {
 	FW_CFG_SIGNATURE	= 0x00,
 	FW_CFG_ID		= 0x01,
@@ -67,6 +69,12 @@ struct fw_cfg_file {
 	char name[FW_CFG_MAX_FILE_PATH];
 };
 
+struct fw_file {
+	struct fw_cfg_file cfg;
+	unsigned long addr;
+	struct list_head list;
+};
+
 struct fw_cfg_files {
 	__be32 count;
 	struct fw_cfg_file files[];
-- 
1.9.1

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

* [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge
  2016-01-15  3:12 [U-Boot] [PATCH 0/4] add support for loading ACPI tables from QEMU Miao Yan
  2016-01-15  3:12 ` [U-Boot] [PATCH 1/4] x86: qemu: re-structure qemu_fwcfg_list_firmware() Miao Yan
@ 2016-01-15  3:12 ` Miao Yan
  2016-01-16 13:23   ` Bin Meng
  2016-01-15  3:12 ` [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU Miao Yan
  2016-01-15  3:12 ` [U-Boot] [PATCH 4/4] x86: qemu: loading ACPI table " Miao Yan
  3 siblings, 1 reply; 16+ messages in thread
From: Miao Yan @ 2016-01-15  3:12 UTC (permalink / raw)
  To: u-boot

Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)

Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
 arch/x86/cpu/qemu/qemu.c                | 39 +++++++++++++++++++++++++++++++++
 arch/x86/include/asm/arch-qemu/device.h |  8 +++++++
 2 files changed, 47 insertions(+)

diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
index 46111c9..e7d8a6c 100644
--- a/arch/x86/cpu/qemu/qemu.c
+++ b/arch/x86/cpu/qemu/qemu.c
@@ -15,6 +15,41 @@
 
 static bool i440fx;
 
+static void enable_pm_piix(void)
+{
+	u8 en;
+	u16 device, cmd;
+
+	device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID);
+	if (device != PCI_DEVICE_ID_INTEL_82371AB_3)
+		return;
+
+	/* Set the PM I/O base. */
+	x86_pci_write_config32(PIIX_PM, PMBA, DEFAULT_PMBASE | 1);
+
+	/* Enable access to the PM I/O space. */
+	cmd = x86_pci_read_config16(PIIX_PM, PCI_COMMAND);
+	cmd |= PCI_COMMAND_IO;
+	x86_pci_write_config16(PIIX_PM, PCI_COMMAND, cmd);
+
+	/* PM I/O Space Enable (PMIOSE). */
+	en = x86_pci_read_config8(PIIX_PM, PMREGMISC);
+	en |= PMIOSE;
+	x86_pci_write_config8(PIIX_PM, PMREGMISC, en);
+}
+
+static void enable_pm_ich9(void)
+{
+	u16 device;
+
+	device = x86_pci_read_config16(ICH9_PM, PCI_DEVICE_ID);
+	if (device != PCI_DEVICE_ID_INTEL_ICH9_8)
+		return;
+
+	/* Set the PM I/O base. */
+	x86_pci_write_config32(ICH9_PM, PMBA, DEFAULT_PMBASE | 1);
+}
+
 static void qemu_chipset_init(void)
 {
 	u16 device, xbcs;
@@ -53,10 +88,14 @@ static void qemu_chipset_init(void)
 		xbcs = x86_pci_read_config16(PIIX_ISA, XBCS);
 		xbcs |= APIC_EN;
 		x86_pci_write_config16(PIIX_ISA, XBCS, xbcs);
+
+		enable_pm_piix();
 	} else {
 		/* Configure PCIe ECAM base address */
 		x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR,
 				       CONFIG_PCIE_ECAM_BASE | BAR_EN);
+
+		enable_pm_ich9();
 	}
 
 	qemu_fwcfg_init();
diff --git a/arch/x86/include/asm/arch-qemu/device.h b/arch/x86/include/asm/arch-qemu/device.h
index 75a435e..2e11100 100644
--- a/arch/x86/include/asm/arch-qemu/device.h
+++ b/arch/x86/include/asm/arch-qemu/device.h
@@ -13,9 +13,17 @@
 #define PIIX_ISA	PCI_BDF(0, 1, 0)
 #define PIIX_IDE	PCI_BDF(0, 1, 1)
 #define PIIX_USB	PCI_BDF(0, 1, 2)
+#define PIIX_PM	PCI_BDF(0, 1, 3)
+#define ICH9_PM	PCI_BDF(0, 0x1f, 0)
 #define I440FX_VGA	PCI_BDF(0, 2, 0)
 
 #define QEMU_Q35	PCI_BDF(0, 0, 0)
 #define Q35_VGA		PCI_BDF(0, 1, 0)
 
+#define PMBA		0x40
+#define DEFAULT_PMBASE	0xe400
+#define PM_IO_BASE	DEFAULT_PMBASE
+#define PMREGMISC	0x80
+#define PMIOSE		(1 << 0)
+
 #endif /* _QEMU_DEVICE_H_ */
-- 
1.9.1

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

* [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU
  2016-01-15  3:12 [U-Boot] [PATCH 0/4] add support for loading ACPI tables from QEMU Miao Yan
  2016-01-15  3:12 ` [U-Boot] [PATCH 1/4] x86: qemu: re-structure qemu_fwcfg_list_firmware() Miao Yan
  2016-01-15  3:12 ` [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge Miao Yan
@ 2016-01-15  3:12 ` Miao Yan
  2016-01-16 13:24   ` Bin Meng
  2016-01-15  3:12 ` [U-Boot] [PATCH 4/4] x86: qemu: loading ACPI table " Miao Yan
  3 siblings, 1 reply; 16+ messages in thread
From: Miao Yan @ 2016-01-15  3:12 UTC (permalink / raw)
  To: u-boot

This patch adds the ability to load and link ACPI tables provided by QEMU.
QEMU tells guests how to load and patch ACPI tables through its fw_cfg
interface, by adding a firmware file 'etc/table-loader'. Guests are
supposed to parse this file and execute corresponding QEMU commands.

Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
 arch/x86/cpu/qemu/fw_cfg.c    | 214 ++++++++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/fw_cfg.h |  70 ++++++++++++++
 2 files changed, 284 insertions(+)

diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c
index b22026c..755676c 100644
--- a/arch/x86/cpu/qemu/fw_cfg.c
+++ b/arch/x86/cpu/qemu/fw_cfg.c
@@ -10,7 +10,10 @@
 #include <malloc.h>
 #include <asm/io.h>
 #include <asm/fw_cfg.h>
+#include <asm/tables.h>
+#include <asm/e820.h>
 #include <linux/list.h>
+#include <memalign.h>
 
 static bool fwcfg_present;
 static bool fwcfg_dma_present;
@@ -202,6 +205,217 @@ err:
 	return -ENOMEM;
 }
 
+static struct fw_file *qemu_fwcfg_find_file(const char *name)
+{
+	struct list_head *entry;
+	struct fw_file *file;
+
+	list_for_each(entry, &fw_list) {
+		file = list_entry(entry, struct fw_file, list);
+		if (!strcmp(file->cfg.name, name))
+			return file;
+	}
+
+	return NULL;
+}
+
+static int bios_linker_allocate(struct bios_linker_entry *entry,
+			   unsigned long *addr)
+{
+	uint32_t size, align;
+	struct fw_file *file;
+	unsigned long aligned_addr;
+
+	align = le32_to_cpu(entry->alloc.align);
+	/* align must be power of 2 */
+	if (align & (align - 1)) {
+		printf("error: wrong alignment %u\n", align);
+		return -EINVAL;
+	}
+
+	file = qemu_fwcfg_find_file(entry->alloc.file);
+	if (!file) {
+		printf("error: can't find file %s\n", entry->alloc.file);
+		return -ENOENT;
+	}
+
+	size = be32_to_cpu(file->cfg.size);
+
+	/*
+	 * ZONE_HIGH means we need to allocate from high memory, since
+	 * malloc space is already at the end of RAM, so we directly use it.
+	 * If allocation zone is ZONE_FSEG, then we use the 'addr' passed
+	 * in which is low memory
+	 */
+	if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH) {
+		aligned_addr = (unsigned long)memalign(align, size);
+	} else if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG) {
+		aligned_addr = ALIGN(*addr, align);
+	} else {
+		printf("error: invalid allocation zone\n");
+		return -EINVAL;
+	}
+
+	debug("bios_linker_allocate: allocate file %s, size %u, zone %d, align %u, addr 0x%lx\n",
+	      file->cfg.name, size, entry->alloc.zone, align, aligned_addr);
+
+	qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
+			      size, (void *)aligned_addr);
+	file->addr = aligned_addr;
+
+	/* adjust address for low memory allocation */
+	if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG)
+		*addr = (aligned_addr + size);
+
+	return 0;
+}
+
+static int bios_linker_add_pointer(struct bios_linker_entry *entry)
+{
+	struct fw_file *dest, *src;
+	uint32_t offset = le32_to_cpu(entry->pointer.offset);
+	uint64_t pointer = 0;
+
+	dest = qemu_fwcfg_find_file(entry->pointer.dest_file);
+	if (!dest || !dest->addr)
+		return -ENOENT;
+	src = qemu_fwcfg_find_file(entry->pointer.src_file);
+	if (!src || !src->addr)
+		return -ENOENT;
+
+
+	memcpy(&pointer, (char *)dest->addr + offset, entry->pointer.size);
+	pointer	= le64_to_cpu(pointer);
+
+
+	debug("bios_linker_add_pointer: dest->addr 0x%lx, src->addr 0x%lx, offset 0x%x size %u, 0x%llx\n",
+	      dest->addr, src->addr, offset, entry->pointer.size, pointer);
+
+	pointer += (unsigned long)src->addr;
+	pointer	= cpu_to_le64(pointer);
+	memcpy((char *)dest->addr + offset, &pointer, entry->pointer.size);
+
+	return 0;
+}
+
+static int bios_linker_add_checksum(struct bios_linker_entry *entry)
+{
+	struct fw_file *file;
+	uint8_t *data, cksum = 0;
+	uint8_t *cksum_start;
+
+	file = qemu_fwcfg_find_file(entry->cksum.file);
+	if (!file || !file->addr)
+		return -ENOENT;
+
+	data = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.offset));
+	cksum_start = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.start));
+	cksum = table_compute_checksum(cksum_start,
+				       le32_to_cpu(entry->cksum.length));
+	*data = cksum;
+
+	return 0;
+}
+
+unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
+{
+	entries[0].addr = 0;
+	entries[0].size = ISA_START_ADDRESS;
+	entries[0].type = E820_RAM;
+
+	entries[1].addr = ISA_START_ADDRESS;
+	entries[1].size = ISA_END_ADDRESS - ISA_START_ADDRESS;
+	entries[1].type = E820_RESERVED;
+
+	/*
+	 * since we use memalign(malloc) to allocate high memory for
+	 * storing ACPI tables, we need to reserve them in e820 tables,
+	 * otherwise kernel will reclaim them and data will be corrupted
+	 */
+	entries[2].addr = ISA_END_ADDRESS;
+	entries[2].size = gd->relocaddr - TOTAL_MALLOC_LEN - ISA_END_ADDRESS;
+	entries[2].type = E820_RAM;
+
+	/* for simplicity, reserve entire malloc space */
+	entries[3].addr = gd->relocaddr - TOTAL_MALLOC_LEN;
+	entries[3].size = TOTAL_MALLOC_LEN;
+	entries[3].type = E820_RESERVED;
+
+	entries[4].addr = gd->relocaddr;
+	entries[4].size = gd->ram_size - gd->relocaddr;
+	entries[4].type = E820_RESERVED;
+
+	entries[5].addr = CONFIG_PCIE_ECAM_BASE;
+	entries[5].size = CONFIG_PCIE_ECAM_SIZE;
+	entries[5].type = E820_RESERVED;
+
+	return 6;
+}
+
+/* This function loads and patches ACPI tables provided by QEMU */
+unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr)
+{
+	int i, ret = 0;
+	struct fw_file *file;
+	struct bios_linker_entry *table_loader;
+	struct bios_linker_entry *entry;
+	uint32_t size;
+
+	/* make sure fw_list is loaded */
+	ret = qemu_fwcfg_read_firmware_list();
+	if (ret) {
+		printf("error: can't read firmware file list\n");
+		return addr;
+	}
+
+	file = qemu_fwcfg_find_file("etc/table-loader");
+	if (!file) {
+		printf("error: can't find etc/table-loader\n");
+		return addr;
+	}
+
+	size = be32_to_cpu(file->cfg.size);
+	if ((size % sizeof(*entry)) != 0) {
+		printf("error: table loader maybe corrupted\n");
+		return addr;
+	}
+
+	table_loader = malloc(size);
+	if (!table_loader) {
+		printf("error: no memory for table-loader\n");
+		return addr;
+	}
+
+	qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
+			      size, table_loader);
+
+	for (i = 0; i < (size / sizeof(*entry)); i++) {
+		entry = table_loader + i;
+		switch (le32_to_cpu(entry->command)) {
+		case BIOS_LINKER_LOADER_COMMAND_ALLOCATE:
+			ret = bios_linker_allocate(entry, &addr);
+			if (ret)
+				goto err;
+			break;
+		case BIOS_LINKER_LOADER_COMMAND_ADD_POINTER:
+			ret = bios_linker_add_pointer(entry);
+			if (ret)
+				goto err;
+			break;
+		case BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM:
+			ret = bios_linker_add_checksum(entry);
+			if (ret)
+				goto err;
+			break;
+		default:
+			break;
+		}
+	}
+err:
+	free(table_loader);
+	return addr;
+}
+
 static int qemu_fwcfg_list_firmware(void)
 {
 	int ret;
diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h
index 285d805..ad58205 100644
--- a/arch/x86/include/asm/fw_cfg.h
+++ b/arch/x86/include/asm/fw_cfg.h
@@ -47,11 +47,23 @@ enum qemu_fwcfg_items {
 	FW_CFG_INVALID		= 0xffff,
 };
 
+enum {
+	BIOS_LINKER_LOADER_COMMAND_ALLOCATE	= 0x1,
+	BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
+	BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
+};
+
+enum {
+	BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1,
+	BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
+};
+
 #define FW_CFG_FILE_SLOTS	0x10
 #define FW_CFG_MAX_ENTRY	(FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS)
 #define FW_CFG_ENTRY_MASK	 ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
 
 #define FW_CFG_MAX_FILE_PATH	56
+#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH
 
 #define QEMU_FW_CFG_SIGNATURE	(('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
 
@@ -69,6 +81,7 @@ struct fw_cfg_file {
 	char name[FW_CFG_MAX_FILE_PATH];
 };
 
+
 struct fw_file {
 	struct fw_cfg_file cfg;
 	unsigned long addr;
@@ -86,6 +99,55 @@ struct fw_cfg_dma_access {
 	__be64 address;
 };
 
+struct bios_linker_entry {
+	__le32 command;
+	union {
+		/*
+		 * COMMAND_ALLOCATE - allocate a table from @alloc.file
+		 * subject to @alloc.align alignment (must be power of 2)
+		 * and @alloc.zone (can be HIGH or FSEG) requirements.
+		 *
+		 * Must appear exactly once for each file, and before
+		 * this file is referenced by any other command.
+		 */
+		struct {
+			char file[BIOS_LINKER_LOADER_FILESZ];
+			__le32 align;
+			uint8_t zone;
+		} alloc;
+
+		/*
+		 * COMMAND_ADD_POINTER - patch the table (originating from
+		 * @dest_file) at @pointer.offset, by adding a pointer to the
+		 * table originating from @src_file. 1,2,4 or 8 byte unsigned
+		 * addition is used depending on @pointer.size.
+		 */
+		struct {
+			char dest_file[BIOS_LINKER_LOADER_FILESZ];
+			char src_file[BIOS_LINKER_LOADER_FILESZ];
+			__le32 offset;
+			uint8_t size;
+		} pointer;
+
+		/*
+		 * COMMAND_ADD_CHECKSUM - calculate checksum of the range
+		 * specified by @cksum_start and @cksum_length fields,
+		 * and then add the value@@cksum.offset.
+		 * Checksum simply sums -X for each byte X in the range
+		 * using 8-bit math.
+		 */
+		struct {
+			char file[BIOS_LINKER_LOADER_FILESZ];
+			__le32 offset;
+			__le32 start;
+			__le32 length;
+		} cksum;
+
+		/* padding */
+		char pad[124];
+	};
+} __packed;
+
 /**
  * Initialize QEMU fw_cfg interface
  */
@@ -98,4 +160,12 @@ void qemu_fwcfg_init(void);
  */
 int qemu_fwcfg_online_cpus(void);
 
+/**
+ * Load ACPI tables from QEMU
+ *
+ * @addr:	address to load
+ * @return:	next address
+ */
+unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr);
+
 #endif
-- 
1.9.1

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

* [U-Boot]  [PATCH 4/4] x86: qemu: loading ACPI table from QEMU
  2016-01-15  3:12 [U-Boot] [PATCH 0/4] add support for loading ACPI tables from QEMU Miao Yan
                   ` (2 preceding siblings ...)
  2016-01-15  3:12 ` [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU Miao Yan
@ 2016-01-15  3:12 ` Miao Yan
  2016-01-16 13:24   ` Bin Meng
  3 siblings, 1 reply; 16+ messages in thread
From: Miao Yan @ 2016-01-15  3:12 UTC (permalink / raw)
  To: u-boot

If CONFIG_GENERATE_ACPI_TABLE is not defined, then use ACPI table created
by QEMU.

Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
 arch/x86/lib/tables.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index 14b15cf..1671385 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -10,6 +10,7 @@
 #include <asm/smbios.h>
 #include <asm/tables.h>
 #include <asm/acpi_table.h>
+#include <asm/fw_cfg.h>
 
 u8 table_compute_checksum(void *v, int len)
 {
@@ -55,8 +56,10 @@ void write_tables(void)
 #endif
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 	rom_table_end = write_acpi_tables(rom_table_end);
-	rom_table_end = ALIGN(rom_table_end, 1024);
+#else
+	rom_table_end = qemu_fwcfg_write_acpi_tables(rom_table_end);
 #endif
+	rom_table_end = ALIGN(rom_table_end, 1024);
 #ifdef CONFIG_GENERATE_SMBIOS_TABLE
 	rom_table_end = write_smbios_table(rom_table_end);
 	rom_table_end = ALIGN(rom_table_end, 1024);
-- 
1.9.1

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

* [U-Boot] [PATCH 1/4] x86: qemu: re-structure qemu_fwcfg_list_firmware()
  2016-01-15  3:12 ` [U-Boot] [PATCH 1/4] x86: qemu: re-structure qemu_fwcfg_list_firmware() Miao Yan
@ 2016-01-16 13:23   ` Bin Meng
  0 siblings, 0 replies; 16+ messages in thread
From: Bin Meng @ 2016-01-16 13:23 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> Re-write the logic in qemu_fwcfg_list_firmware(), add a function
> qemu_cfg_read_firmware_list() to handle reading firmware list.

qemu_fwcfg_read_firmware_list()

>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
>  arch/x86/cpu/qemu/fw_cfg.c    | 60 +++++++++++++++++++++++++++++++++----------
>  arch/x86/include/asm/fw_cfg.h |  8 ++++++
>  2 files changed, 54 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c
> index 0599214..b22026c 100644
> --- a/arch/x86/cpu/qemu/fw_cfg.c
> +++ b/arch/x86/cpu/qemu/fw_cfg.c
> @@ -10,10 +10,13 @@
>  #include <malloc.h>
>  #include <asm/io.h>
>  #include <asm/fw_cfg.h>
> +#include <linux/list.h>
>
>  static bool fwcfg_present;
>  static bool fwcfg_dma_present;
>
> +static LIST_HEAD(fw_list);
> +
>  /* Read configuration item using fw_cfg PIO interface */
>  static void qemu_fwcfg_read_entry_pio(uint16_t entry,
>                 uint32_t size, void *address)
> @@ -162,29 +165,58 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
>         return 0;
>  }
>
> -static int qemu_fwcfg_list_firmware(void)
> +static int qemu_fwcfg_read_firmware_list(void)
>  {
>         int i;
>         uint32_t count;
> -       struct fw_cfg_files *files;
> +       struct fw_file *file;
> +       struct list_head *entry;
> +
> +       /* don't read it twice */
> +       if (!list_empty(&fw_list))
> +               return 0;
>
>         qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count);
>         if (!count)
>                 return 0;
>
>         count = be32_to_cpu(count);
> -       files = malloc(count * sizeof(struct fw_cfg_file));
> -       if (!files)
> -               return -ENOMEM;
> -
> -       files->count = count;
> -       qemu_fwcfg_read_entry(FW_CFG_INVALID,
> -                             count * sizeof(struct fw_cfg_file),
> -                             files->files);
> -
> -       for (i = 0; i < files->count; i++)
> -               printf("%-56s\n", files->files[i].name);
> -       free(files);
> +       for (i = 0; i < count; i++) {
> +               file = malloc(sizeof(*file));
> +               if (!file) {
> +                       printf("error: allocating resource\n");
> +                       goto err;
> +               }
> +               qemu_fwcfg_read_entry(FW_CFG_INVALID,
> +                                     sizeof(struct fw_cfg_file), &file->cfg);
> +               file->addr = 0;
> +               list_add_tail(&file->list, &fw_list);
> +       }

nits: leave one blank line before return

> +       return 0;
> +
> +err:
> +       list_for_each(entry, &fw_list) {
> +               file = list_entry(entry, struct fw_file, list);
> +               free(file);
> +       }

nits: leave one blank line before return

> +       return -ENOMEM;
> +}
> +
> +static int qemu_fwcfg_list_firmware(void)
> +{
> +       int ret;
> +       struct list_head *entry;
> +       struct fw_file *file;
> +
> +       /* make sure fw_list is loaded */
> +       ret = qemu_fwcfg_read_firmware_list();
> +       if (ret)
> +               return ret;
> +
> +       list_for_each(entry, &fw_list) {
> +               file = list_entry(entry, struct fw_file, list);
> +               printf("%-56s\n", file->cfg.name);
> +       }

nits: leave one blank line before return

>         return 0;
>  }
>
> diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h
> index fb110fa..285d805 100644
> --- a/arch/x86/include/asm/fw_cfg.h
> +++ b/arch/x86/include/asm/fw_cfg.h
> @@ -12,6 +12,8 @@
>  #define FW_DMA_PORT_LOW        0x514
>  #define FW_DMA_PORT_HIGH       0x518
>
> +#include <linux/list.h>
> +
>  enum qemu_fwcfg_items {
>         FW_CFG_SIGNATURE        = 0x00,
>         FW_CFG_ID               = 0x01,
> @@ -67,6 +69,12 @@ struct fw_cfg_file {
>         char name[FW_CFG_MAX_FILE_PATH];
>  };
>
> +struct fw_file {
> +       struct fw_cfg_file cfg;
> +       unsigned long addr;
> +       struct list_head list;
> +};

Can you please put a comment block above to describe what is each member for?

> +
>  struct fw_cfg_files {

Looks that this struct fw_cfg_files is no longer used by any codes.
Please remove this.

>         __be32 count;
>         struct fw_cfg_file files[];
> --

Regards,
Bin

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

* [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge
  2016-01-15  3:12 ` [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge Miao Yan
@ 2016-01-16 13:23   ` Bin Meng
  2016-01-19  2:46     ` Miao Yan
  0 siblings, 1 reply; 16+ messages in thread
From: Bin Meng @ 2016-01-16 13:23 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
>  arch/x86/cpu/qemu/qemu.c                | 39 +++++++++++++++++++++++++++++++++
>  arch/x86/include/asm/arch-qemu/device.h |  8 +++++++
>  2 files changed, 47 insertions(+)
>
> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
> index 46111c9..e7d8a6c 100644
> --- a/arch/x86/cpu/qemu/qemu.c
> +++ b/arch/x86/cpu/qemu/qemu.c
> @@ -15,6 +15,41 @@
>
>  static bool i440fx;
>
> +static void enable_pm_piix(void)
> +{
> +       u8 en;
> +       u16 device, cmd;
> +
> +       device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID);
> +       if (device != PCI_DEVICE_ID_INTEL_82371AB_3)
> +               return;

Guess the check is already covered in qemu_chipset_init().

> +
> +       /* Set the PM I/O base. */

nits: please remove the ending period. Please fix this globally in this file.

> +       x86_pci_write_config32(PIIX_PM, PMBA, DEFAULT_PMBASE | 1);
> +
> +       /* Enable access to the PM I/O space. */
> +       cmd = x86_pci_read_config16(PIIX_PM, PCI_COMMAND);
> +       cmd |= PCI_COMMAND_IO;
> +       x86_pci_write_config16(PIIX_PM, PCI_COMMAND, cmd);
> +
> +       /* PM I/O Space Enable (PMIOSE). */
> +       en = x86_pci_read_config8(PIIX_PM, PMREGMISC);
> +       en |= PMIOSE;
> +       x86_pci_write_config8(PIIX_PM, PMREGMISC, en);
> +}
> +
> +static void enable_pm_ich9(void)
> +{
> +       u16 device;
> +
> +       device = x86_pci_read_config16(ICH9_PM, PCI_DEVICE_ID);
> +       if (device != PCI_DEVICE_ID_INTEL_ICH9_8)
> +               return;

Guess the check is already covered in qemu_chipset_init().

> +
> +       /* Set the PM I/O base. */
> +       x86_pci_write_config32(ICH9_PM, PMBA, DEFAULT_PMBASE | 1);
> +}
> +
>  static void qemu_chipset_init(void)
>  {
>         u16 device, xbcs;
> @@ -53,10 +88,14 @@ static void qemu_chipset_init(void)
>                 xbcs = x86_pci_read_config16(PIIX_ISA, XBCS);
>                 xbcs |= APIC_EN;
>                 x86_pci_write_config16(PIIX_ISA, XBCS, xbcs);
> +
> +               enable_pm_piix();
>         } else {
>                 /* Configure PCIe ECAM base address */
>                 x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR,
>                                        CONFIG_PCIE_ECAM_BASE | BAR_EN);
> +
> +               enable_pm_ich9();
>         }
>
>         qemu_fwcfg_init();
> diff --git a/arch/x86/include/asm/arch-qemu/device.h b/arch/x86/include/asm/arch-qemu/device.h
> index 75a435e..2e11100 100644
> --- a/arch/x86/include/asm/arch-qemu/device.h
> +++ b/arch/x86/include/asm/arch-qemu/device.h
> @@ -13,9 +13,17 @@
>  #define PIIX_ISA       PCI_BDF(0, 1, 0)
>  #define PIIX_IDE       PCI_BDF(0, 1, 1)
>  #define PIIX_USB       PCI_BDF(0, 1, 2)
> +#define PIIX_PM        PCI_BDF(0, 1, 3)
> +#define ICH9_PM        PCI_BDF(0, 0x1f, 0)
>  #define I440FX_VGA     PCI_BDF(0, 2, 0)
>
>  #define QEMU_Q35       PCI_BDF(0, 0, 0)
>  #define Q35_VGA                PCI_BDF(0, 1, 0)
>
> +#define PMBA           0x40
> +#define DEFAULT_PMBASE 0xe400

See arch/x86/cpu/quark/Kconfig we have ACPI_PM1_BASE already. Maybe we
need consolidate this to introduce a similar one for QEMU.

> +#define PM_IO_BASE     DEFAULT_PMBASE

PM_IO_BASE is not referenced anywhere.

> +#define PMREGMISC      0x80
> +#define PMIOSE         (1 << 0)
> +

Please move these register defines to include/asm/arch-qemu/qemu.h

>  #endif /* _QEMU_DEVICE_H_ */
> --

Regards,
Bin

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

* [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU
  2016-01-15  3:12 ` [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU Miao Yan
@ 2016-01-16 13:24   ` Bin Meng
  2016-01-19  2:39     ` Miao Yan
  0 siblings, 1 reply; 16+ messages in thread
From: Bin Meng @ 2016-01-16 13:24 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> This patch adds the ability to load and link ACPI tables provided by QEMU.
> QEMU tells guests how to load and patch ACPI tables through its fw_cfg
> interface, by adding a firmware file 'etc/table-loader'. Guests are
> supposed to parse this file and execute corresponding QEMU commands.
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
>  arch/x86/cpu/qemu/fw_cfg.c    | 214 ++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/include/asm/fw_cfg.h |  70 ++++++++++++++
>  2 files changed, 284 insertions(+)
>
> diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c
> index b22026c..755676c 100644
> --- a/arch/x86/cpu/qemu/fw_cfg.c
> +++ b/arch/x86/cpu/qemu/fw_cfg.c
> @@ -10,7 +10,10 @@
>  #include <malloc.h>
>  #include <asm/io.h>
>  #include <asm/fw_cfg.h>
> +#include <asm/tables.h>
> +#include <asm/e820.h>
>  #include <linux/list.h>
> +#include <memalign.h>
>
>  static bool fwcfg_present;
>  static bool fwcfg_dma_present;
> @@ -202,6 +205,217 @@ err:
>         return -ENOMEM;
>  }
>
> +static struct fw_file *qemu_fwcfg_find_file(const char *name)
> +{
> +       struct list_head *entry;
> +       struct fw_file *file;
> +
> +       list_for_each(entry, &fw_list) {
> +               file = list_entry(entry, struct fw_file, list);
> +               if (!strcmp(file->cfg.name, name))
> +                       return file;
> +       }
> +
> +       return NULL;
> +}
> +
> +static int bios_linker_allocate(struct bios_linker_entry *entry,
> +                          unsigned long *addr)

Please add a comment block for what this function is doing, its
parameters, return value, etc. Please do the same for the other 2
functions below.

> +{
> +       uint32_t size, align;
> +       struct fw_file *file;
> +       unsigned long aligned_addr;
> +
> +       align = le32_to_cpu(entry->alloc.align);
> +       /* align must be power of 2 */
> +       if (align & (align - 1)) {
> +               printf("error: wrong alignment %u\n", align);
> +               return -EINVAL;
> +       }
> +
> +       file = qemu_fwcfg_find_file(entry->alloc.file);
> +       if (!file) {
> +               printf("error: can't find file %s\n", entry->alloc.file);
> +               return -ENOENT;
> +       }
> +
> +       size = be32_to_cpu(file->cfg.size);
> +
> +       /*
> +        * ZONE_HIGH means we need to allocate from high memory, since
> +        * malloc space is already at the end of RAM, so we directly use it.
> +        * If allocation zone is ZONE_FSEG, then we use the 'addr' passed
> +        * in which is low memory
> +        */
> +       if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH) {
> +               aligned_addr = (unsigned long)memalign(align, size);
> +       } else if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG) {
> +               aligned_addr = ALIGN(*addr, align);
> +       } else {
> +               printf("error: invalid allocation zone\n");
> +               return -EINVAL;
> +       }
> +
> +       debug("bios_linker_allocate: allocate file %s, size %u, zone %d, align %u, addr 0x%lx\n",
> +             file->cfg.name, size, entry->alloc.zone, align, aligned_addr);
> +
> +       qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
> +                             size, (void *)aligned_addr);
> +       file->addr = aligned_addr;
> +
> +       /* adjust address for low memory allocation */
> +       if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG)
> +               *addr = (aligned_addr + size);
> +
> +       return 0;
> +}
> +
> +static int bios_linker_add_pointer(struct bios_linker_entry *entry)
> +{
> +       struct fw_file *dest, *src;
> +       uint32_t offset = le32_to_cpu(entry->pointer.offset);
> +       uint64_t pointer = 0;
> +
> +       dest = qemu_fwcfg_find_file(entry->pointer.dest_file);
> +       if (!dest || !dest->addr)
> +               return -ENOENT;
> +       src = qemu_fwcfg_find_file(entry->pointer.src_file);
> +       if (!src || !src->addr)
> +               return -ENOENT;
> +

Remove one blank line here.

> +
> +       memcpy(&pointer, (char *)dest->addr + offset, entry->pointer.size);
> +       pointer = le64_to_cpu(pointer);
> +

Remove one blank line here.

> +
> +       debug("bios_linker_add_pointer: dest->addr 0x%lx, src->addr 0x%lx, offset 0x%x size %u, 0x%llx\n",
> +             dest->addr, src->addr, offset, entry->pointer.size, pointer);
> +
> +       pointer += (unsigned long)src->addr;
> +       pointer = cpu_to_le64(pointer);
> +       memcpy((char *)dest->addr + offset, &pointer, entry->pointer.size);
> +
> +       return 0;
> +}
> +
> +static int bios_linker_add_checksum(struct bios_linker_entry *entry)
> +{
> +       struct fw_file *file;
> +       uint8_t *data, cksum = 0;
> +       uint8_t *cksum_start;
> +
> +       file = qemu_fwcfg_find_file(entry->cksum.file);
> +       if (!file || !file->addr)
> +               return -ENOENT;
> +
> +       data = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.offset));
> +       cksum_start = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.start));
> +       cksum = table_compute_checksum(cksum_start,
> +                                      le32_to_cpu(entry->cksum.length));
> +       *data = cksum;
> +
> +       return 0;
> +}
> +
> +unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
> +{
> +       entries[0].addr = 0;
> +       entries[0].size = ISA_START_ADDRESS;
> +       entries[0].type = E820_RAM;
> +
> +       entries[1].addr = ISA_START_ADDRESS;
> +       entries[1].size = ISA_END_ADDRESS - ISA_START_ADDRESS;
> +       entries[1].type = E820_RESERVED;
> +
> +       /*
> +        * since we use memalign(malloc) to allocate high memory for
> +        * storing ACPI tables, we need to reserve them in e820 tables,
> +        * otherwise kernel will reclaim them and data will be corrupted
> +        */
> +       entries[2].addr = ISA_END_ADDRESS;
> +       entries[2].size = gd->relocaddr - TOTAL_MALLOC_LEN - ISA_END_ADDRESS;
> +       entries[2].type = E820_RAM;
> +
> +       /* for simplicity, reserve entire malloc space */
> +       entries[3].addr = gd->relocaddr - TOTAL_MALLOC_LEN;
> +       entries[3].size = TOTAL_MALLOC_LEN;
> +       entries[3].type = E820_RESERVED;
> +
> +       entries[4].addr = gd->relocaddr;
> +       entries[4].size = gd->ram_size - gd->relocaddr;
> +       entries[4].type = E820_RESERVED;
> +
> +       entries[5].addr = CONFIG_PCIE_ECAM_BASE;
> +       entries[5].size = CONFIG_PCIE_ECAM_SIZE;
> +       entries[5].type = E820_RESERVED;
> +
> +       return 6;
> +}
> +
> +/* This function loads and patches ACPI tables provided by QEMU */
> +unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr)
> +{
> +       int i, ret = 0;
> +       struct fw_file *file;
> +       struct bios_linker_entry *table_loader;
> +       struct bios_linker_entry *entry;
> +       uint32_t size;
> +
> +       /* make sure fw_list is loaded */
> +       ret = qemu_fwcfg_read_firmware_list();
> +       if (ret) {
> +               printf("error: can't read firmware file list\n");
> +               return addr;
> +       }
> +
> +       file = qemu_fwcfg_find_file("etc/table-loader");
> +       if (!file) {
> +               printf("error: can't find etc/table-loader\n");
> +               return addr;
> +       }
> +
> +       size = be32_to_cpu(file->cfg.size);
> +       if ((size % sizeof(*entry)) != 0) {
> +               printf("error: table loader maybe corrupted\n");

table-loader may be

> +               return addr;
> +       }
> +
> +       table_loader = malloc(size);
> +       if (!table_loader) {
> +               printf("error: no memory for table-loader\n");
> +               return addr;
> +       }
> +
> +       qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
> +                             size, table_loader);
> +
> +       for (i = 0; i < (size / sizeof(*entry)); i++) {
> +               entry = table_loader + i;
> +               switch (le32_to_cpu(entry->command)) {
> +               case BIOS_LINKER_LOADER_COMMAND_ALLOCATE:
> +                       ret = bios_linker_allocate(entry, &addr);
> +                       if (ret)
> +                               goto err;
> +                       break;
> +               case BIOS_LINKER_LOADER_COMMAND_ADD_POINTER:
> +                       ret = bios_linker_add_pointer(entry);
> +                       if (ret)
> +                               goto err;
> +                       break;
> +               case BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM:
> +                       ret = bios_linker_add_checksum(entry);
> +                       if (ret)
> +                               goto err;
> +                       break;
> +               default:
> +                       break;
> +               }
> +       }
> +err:
> +       free(table_loader);
> +       return addr;
> +}
> +
>  static int qemu_fwcfg_list_firmware(void)
>  {
>         int ret;
> diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h
> index 285d805..ad58205 100644
> --- a/arch/x86/include/asm/fw_cfg.h
> +++ b/arch/x86/include/asm/fw_cfg.h
> @@ -47,11 +47,23 @@ enum qemu_fwcfg_items {
>         FW_CFG_INVALID          = 0xffff,
>  };
>
> +enum {
> +       BIOS_LINKER_LOADER_COMMAND_ALLOCATE     = 0x1,
> +       BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
> +       BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
> +};
> +
> +enum {
> +       BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1,
> +       BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
> +};
> +
>  #define FW_CFG_FILE_SLOTS      0x10
>  #define FW_CFG_MAX_ENTRY       (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS)
>  #define FW_CFG_ENTRY_MASK       ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
>
>  #define FW_CFG_MAX_FILE_PATH   56
> +#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH

Use FW_CFG_MAX_FILE_PATH directly?

>
>  #define QEMU_FW_CFG_SIGNATURE  (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
>
> @@ -69,6 +81,7 @@ struct fw_cfg_file {
>         char name[FW_CFG_MAX_FILE_PATH];
>  };
>
> +
>  struct fw_file {
>         struct fw_cfg_file cfg;
>         unsigned long addr;
> @@ -86,6 +99,55 @@ struct fw_cfg_dma_access {
>         __be64 address;
>  };
>
> +struct bios_linker_entry {
> +       __le32 command;
> +       union {
> +               /*
> +                * COMMAND_ALLOCATE - allocate a table from @alloc.file
> +                * subject to @alloc.align alignment (must be power of 2)
> +                * and @alloc.zone (can be HIGH or FSEG) requirements.
> +                *
> +                * Must appear exactly once for each file, and before
> +                * this file is referenced by any other command.
> +                */
> +               struct {
> +                       char file[BIOS_LINKER_LOADER_FILESZ];

Use FW_CFG_MAX_FILE_PATH directly?

> +                       __le32 align;
> +                       uint8_t zone;
> +               } alloc;
> +
> +               /*
> +                * COMMAND_ADD_POINTER - patch the table (originating from
> +                * @dest_file) at @pointer.offset, by adding a pointer to the
> +                * table originating from @src_file. 1,2,4 or 8 byte unsigned
> +                * addition is used depending on @pointer.size.
> +                */
> +               struct {
> +                       char dest_file[BIOS_LINKER_LOADER_FILESZ];
> +                       char src_file[BIOS_LINKER_LOADER_FILESZ];
> +                       __le32 offset;
> +                       uint8_t size;
> +               } pointer;
> +
> +               /*
> +                * COMMAND_ADD_CHECKSUM - calculate checksum of the range
> +                * specified by @cksum_start and @cksum_length fields,
> +                * and then add the value at @cksum.offset.
> +                * Checksum simply sums -X for each byte X in the range
> +                * using 8-bit math.
> +                */
> +               struct {
> +                       char file[BIOS_LINKER_LOADER_FILESZ];
> +                       __le32 offset;
> +                       __le32 start;
> +                       __le32 length;
> +               } cksum;
> +
> +               /* padding */
> +               char pad[124];
> +       };
> +} __packed;
> +
>  /**
>   * Initialize QEMU fw_cfg interface
>   */
> @@ -98,4 +160,12 @@ void qemu_fwcfg_init(void);
>   */
>  int qemu_fwcfg_online_cpus(void);
>
> +/**
> + * Load ACPI tables from QEMU
> + *
> + * @addr:      address to load
> + * @return:    next address
> + */
> +unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr);
> +
>  #endif
> --

Regards,
Bin

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

* [U-Boot] [PATCH 4/4] x86: qemu: loading ACPI table from QEMU
  2016-01-15  3:12 ` [U-Boot] [PATCH 4/4] x86: qemu: loading ACPI table " Miao Yan
@ 2016-01-16 13:24   ` Bin Meng
  2016-01-19  2:48     ` Miao Yan
  0 siblings, 1 reply; 16+ messages in thread
From: Bin Meng @ 2016-01-16 13:24 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> If CONFIG_GENERATE_ACPI_TABLE is not defined, then use ACPI table created
> by QEMU.
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
>  arch/x86/lib/tables.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
> index 14b15cf..1671385 100644
> --- a/arch/x86/lib/tables.c
> +++ b/arch/x86/lib/tables.c
> @@ -10,6 +10,7 @@
>  #include <asm/smbios.h>
>  #include <asm/tables.h>
>  #include <asm/acpi_table.h>
> +#include <asm/fw_cfg.h>
>
>  u8 table_compute_checksum(void *v, int len)
>  {
> @@ -55,8 +56,10 @@ void write_tables(void)
>  #endif
>  #ifdef CONFIG_GENERATE_ACPI_TABLE
>         rom_table_end = write_acpi_tables(rom_table_end);
> -       rom_table_end = ALIGN(rom_table_end, 1024);
> +#else
> +       rom_table_end = qemu_fwcfg_write_acpi_tables(rom_table_end);

This breaks other x86 boards.Can we hide this changes in acpi_table.c
with proper #ifdef and some comments?

For QEMU ACPI, how about:

- GENERATE_ACPI_TABLE is the overall switch to turn on ACPI table by U-Boot
- Introduce another Kconfig option for QEMU to generate ACPI tables
from firmware interface. If this is turned on, the original method
provided in acpi_tables.c will not be used.

>  #endif
> +       rom_table_end = ALIGN(rom_table_end, 1024);
>  #ifdef CONFIG_GENERATE_SMBIOS_TABLE
>         rom_table_end = write_smbios_table(rom_table_end);
>         rom_table_end = ALIGN(rom_table_end, 1024);
> --

Regards,
Bin

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

* [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU
  2016-01-16 13:24   ` Bin Meng
@ 2016-01-19  2:39     ` Miao Yan
  2016-01-19  9:25       ` Bin Meng
  0 siblings, 1 reply; 16+ messages in thread
From: Miao Yan @ 2016-01-19  2:39 UTC (permalink / raw)
  To: u-boot

Hi Bin,

2016-01-16 21:24 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
> Hi Miao,
>
> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>> This patch adds the ability to load and link ACPI tables provided by QEMU.
>> QEMU tells guests how to load and patch ACPI tables through its fw_cfg
>> interface, by adding a firmware file 'etc/table-loader'. Guests are
>> supposed to parse this file and execute corresponding QEMU commands.
>>
>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>> ---
>>  arch/x86/cpu/qemu/fw_cfg.c    | 214 ++++++++++++++++++++++++++++++++++++++++++
>>  arch/x86/include/asm/fw_cfg.h |  70 ++++++++++++++
>>  2 files changed, 284 insertions(+)
>>
>> diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c
>> index b22026c..755676c 100644
>> --- a/arch/x86/cpu/qemu/fw_cfg.c
>> +++ b/arch/x86/cpu/qemu/fw_cfg.c
>> @@ -10,7 +10,10 @@
>>  #include <malloc.h>
>>  #include <asm/io.h>
>>  #include <asm/fw_cfg.h>
>> +#include <asm/tables.h>
>> +#include <asm/e820.h>
>>  #include <linux/list.h>
>> +#include <memalign.h>
>>
>>  static bool fwcfg_present;
>>  static bool fwcfg_dma_present;
>> @@ -202,6 +205,217 @@ err:
>>         return -ENOMEM;
>>  }
>>
>> +static struct fw_file *qemu_fwcfg_find_file(const char *name)
>> +{
>> +       struct list_head *entry;
>> +       struct fw_file *file;
>> +
>> +       list_for_each(entry, &fw_list) {
>> +               file = list_entry(entry, struct fw_file, list);
>> +               if (!strcmp(file->cfg.name, name))
>> +                       return file;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>> +static int bios_linker_allocate(struct bios_linker_entry *entry,
>> +                          unsigned long *addr)
>
> Please add a comment block for what this function is doing, its
> parameters, return value, etc. Please do the same for the other 2
> functions below.
>
>> +{
>> +       uint32_t size, align;
>> +       struct fw_file *file;
>> +       unsigned long aligned_addr;
>> +
>> +       align = le32_to_cpu(entry->alloc.align);
>> +       /* align must be power of 2 */
>> +       if (align & (align - 1)) {
>> +               printf("error: wrong alignment %u\n", align);
>> +               return -EINVAL;
>> +       }
>> +
>> +       file = qemu_fwcfg_find_file(entry->alloc.file);
>> +       if (!file) {
>> +               printf("error: can't find file %s\n", entry->alloc.file);
>> +               return -ENOENT;
>> +       }
>> +
>> +       size = be32_to_cpu(file->cfg.size);
>> +
>> +       /*
>> +        * ZONE_HIGH means we need to allocate from high memory, since
>> +        * malloc space is already at the end of RAM, so we directly use it.
>> +        * If allocation zone is ZONE_FSEG, then we use the 'addr' passed
>> +        * in which is low memory
>> +        */
>> +       if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH) {
>> +               aligned_addr = (unsigned long)memalign(align, size);
>> +       } else if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG) {
>> +               aligned_addr = ALIGN(*addr, align);
>> +       } else {
>> +               printf("error: invalid allocation zone\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       debug("bios_linker_allocate: allocate file %s, size %u, zone %d, align %u, addr 0x%lx\n",
>> +             file->cfg.name, size, entry->alloc.zone, align, aligned_addr);
>> +
>> +       qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
>> +                             size, (void *)aligned_addr);
>> +       file->addr = aligned_addr;
>> +
>> +       /* adjust address for low memory allocation */
>> +       if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG)
>> +               *addr = (aligned_addr + size);
>> +
>> +       return 0;
>> +}
>> +
>> +static int bios_linker_add_pointer(struct bios_linker_entry *entry)
>> +{
>> +       struct fw_file *dest, *src;
>> +       uint32_t offset = le32_to_cpu(entry->pointer.offset);
>> +       uint64_t pointer = 0;
>> +
>> +       dest = qemu_fwcfg_find_file(entry->pointer.dest_file);
>> +       if (!dest || !dest->addr)
>> +               return -ENOENT;
>> +       src = qemu_fwcfg_find_file(entry->pointer.src_file);
>> +       if (!src || !src->addr)
>> +               return -ENOENT;
>> +
>
> Remove one blank line here.
>
>> +
>> +       memcpy(&pointer, (char *)dest->addr + offset, entry->pointer.size);
>> +       pointer = le64_to_cpu(pointer);
>> +
>
> Remove one blank line here.
>
>> +
>> +       debug("bios_linker_add_pointer: dest->addr 0x%lx, src->addr 0x%lx, offset 0x%x size %u, 0x%llx\n",
>> +             dest->addr, src->addr, offset, entry->pointer.size, pointer);
>> +
>> +       pointer += (unsigned long)src->addr;
>> +       pointer = cpu_to_le64(pointer);
>> +       memcpy((char *)dest->addr + offset, &pointer, entry->pointer.size);
>> +
>> +       return 0;
>> +}
>> +
>> +static int bios_linker_add_checksum(struct bios_linker_entry *entry)
>> +{
>> +       struct fw_file *file;
>> +       uint8_t *data, cksum = 0;
>> +       uint8_t *cksum_start;
>> +
>> +       file = qemu_fwcfg_find_file(entry->cksum.file);
>> +       if (!file || !file->addr)
>> +               return -ENOENT;
>> +
>> +       data = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.offset));
>> +       cksum_start = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.start));
>> +       cksum = table_compute_checksum(cksum_start,
>> +                                      le32_to_cpu(entry->cksum.length));
>> +       *data = cksum;
>> +
>> +       return 0;
>> +}
>> +
>> +unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
>> +{
>> +       entries[0].addr = 0;
>> +       entries[0].size = ISA_START_ADDRESS;
>> +       entries[0].type = E820_RAM;
>> +
>> +       entries[1].addr = ISA_START_ADDRESS;
>> +       entries[1].size = ISA_END_ADDRESS - ISA_START_ADDRESS;
>> +       entries[1].type = E820_RESERVED;
>> +
>> +       /*
>> +        * since we use memalign(malloc) to allocate high memory for
>> +        * storing ACPI tables, we need to reserve them in e820 tables,
>> +        * otherwise kernel will reclaim them and data will be corrupted
>> +        */
>> +       entries[2].addr = ISA_END_ADDRESS;
>> +       entries[2].size = gd->relocaddr - TOTAL_MALLOC_LEN - ISA_END_ADDRESS;
>> +       entries[2].type = E820_RAM;
>> +
>> +       /* for simplicity, reserve entire malloc space */
>> +       entries[3].addr = gd->relocaddr - TOTAL_MALLOC_LEN;
>> +       entries[3].size = TOTAL_MALLOC_LEN;
>> +       entries[3].type = E820_RESERVED;
>> +
>> +       entries[4].addr = gd->relocaddr;
>> +       entries[4].size = gd->ram_size - gd->relocaddr;
>> +       entries[4].type = E820_RESERVED;
>> +
>> +       entries[5].addr = CONFIG_PCIE_ECAM_BASE;
>> +       entries[5].size = CONFIG_PCIE_ECAM_SIZE;
>> +       entries[5].type = E820_RESERVED;
>> +
>> +       return 6;
>> +}
>> +
>> +/* This function loads and patches ACPI tables provided by QEMU */
>> +unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr)
>> +{
>> +       int i, ret = 0;
>> +       struct fw_file *file;
>> +       struct bios_linker_entry *table_loader;
>> +       struct bios_linker_entry *entry;
>> +       uint32_t size;
>> +
>> +       /* make sure fw_list is loaded */
>> +       ret = qemu_fwcfg_read_firmware_list();
>> +       if (ret) {
>> +               printf("error: can't read firmware file list\n");
>> +               return addr;
>> +       }
>> +
>> +       file = qemu_fwcfg_find_file("etc/table-loader");
>> +       if (!file) {
>> +               printf("error: can't find etc/table-loader\n");
>> +               return addr;
>> +       }
>> +
>> +       size = be32_to_cpu(file->cfg.size);
>> +       if ((size % sizeof(*entry)) != 0) {
>> +               printf("error: table loader maybe corrupted\n");
>
> table-loader may be
>
>> +               return addr;
>> +       }
>> +
>> +       table_loader = malloc(size);
>> +       if (!table_loader) {
>> +               printf("error: no memory for table-loader\n");
>> +               return addr;
>> +       }
>> +
>> +       qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
>> +                             size, table_loader);
>> +
>> +       for (i = 0; i < (size / sizeof(*entry)); i++) {
>> +               entry = table_loader + i;
>> +               switch (le32_to_cpu(entry->command)) {
>> +               case BIOS_LINKER_LOADER_COMMAND_ALLOCATE:
>> +                       ret = bios_linker_allocate(entry, &addr);
>> +                       if (ret)
>> +                               goto err;
>> +                       break;
>> +               case BIOS_LINKER_LOADER_COMMAND_ADD_POINTER:
>> +                       ret = bios_linker_add_pointer(entry);
>> +                       if (ret)
>> +                               goto err;
>> +                       break;
>> +               case BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM:
>> +                       ret = bios_linker_add_checksum(entry);
>> +                       if (ret)
>> +                               goto err;
>> +                       break;
>> +               default:
>> +                       break;
>> +               }
>> +       }
>> +err:
>> +       free(table_loader);
>> +       return addr;
>> +}
>> +
>>  static int qemu_fwcfg_list_firmware(void)
>>  {
>>         int ret;
>> diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h
>> index 285d805..ad58205 100644
>> --- a/arch/x86/include/asm/fw_cfg.h
>> +++ b/arch/x86/include/asm/fw_cfg.h
>> @@ -47,11 +47,23 @@ enum qemu_fwcfg_items {
>>         FW_CFG_INVALID          = 0xffff,
>>  };
>>
>> +enum {
>> +       BIOS_LINKER_LOADER_COMMAND_ALLOCATE     = 0x1,
>> +       BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
>> +       BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
>> +};
>> +
>> +enum {
>> +       BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1,
>> +       BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
>> +};
>> +
>>  #define FW_CFG_FILE_SLOTS      0x10
>>  #define FW_CFG_MAX_ENTRY       (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS)
>>  #define FW_CFG_ENTRY_MASK       ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
>>
>>  #define FW_CFG_MAX_FILE_PATH   56
>> +#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH
>
> Use FW_CFG_MAX_FILE_PATH directly?

Those codes are derived from QEMU, with some modifications to make
them compatible with U-Boot source. So I think maybe we should keep it
in sync with QEMU.


>
>>
>>  #define QEMU_FW_CFG_SIGNATURE  (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
>>
>> @@ -69,6 +81,7 @@ struct fw_cfg_file {
>>         char name[FW_CFG_MAX_FILE_PATH];
>>  };
>>
>> +
>>  struct fw_file {
>>         struct fw_cfg_file cfg;
>>         unsigned long addr;
>> @@ -86,6 +99,55 @@ struct fw_cfg_dma_access {
>>         __be64 address;
>>  };
>>
>> +struct bios_linker_entry {
>> +       __le32 command;
>> +       union {
>> +               /*
>> +                * COMMAND_ALLOCATE - allocate a table from @alloc.file
>> +                * subject to @alloc.align alignment (must be power of 2)
>> +                * and @alloc.zone (can be HIGH or FSEG) requirements.
>> +                *
>> +                * Must appear exactly once for each file, and before
>> +                * this file is referenced by any other command.
>> +                */
>> +               struct {
>> +                       char file[BIOS_LINKER_LOADER_FILESZ];
>
> Use FW_CFG_MAX_FILE_PATH directly?
>
>> +                       __le32 align;
>> +                       uint8_t zone;
>> +               } alloc;
>> +
>> +               /*
>> +                * COMMAND_ADD_POINTER - patch the table (originating from
>> +                * @dest_file) at @pointer.offset, by adding a pointer to the
>> +                * table originating from @src_file. 1,2,4 or 8 byte unsigned
>> +                * addition is used depending on @pointer.size.
>> +                */
>> +               struct {
>> +                       char dest_file[BIOS_LINKER_LOADER_FILESZ];
>> +                       char src_file[BIOS_LINKER_LOADER_FILESZ];
>> +                       __le32 offset;
>> +                       uint8_t size;
>> +               } pointer;
>> +
>> +               /*
>> +                * COMMAND_ADD_CHECKSUM - calculate checksum of the range
>> +                * specified by @cksum_start and @cksum_length fields,
>> +                * and then add the value at @cksum.offset.
>> +                * Checksum simply sums -X for each byte X in the range
>> +                * using 8-bit math.
>> +                */
>> +               struct {
>> +                       char file[BIOS_LINKER_LOADER_FILESZ];
>> +                       __le32 offset;
>> +                       __le32 start;
>> +                       __le32 length;
>> +               } cksum;
>> +
>> +               /* padding */
>> +               char pad[124];
>> +       };
>> +} __packed;
>> +
>>  /**
>>   * Initialize QEMU fw_cfg interface
>>   */
>> @@ -98,4 +160,12 @@ void qemu_fwcfg_init(void);
>>   */
>>  int qemu_fwcfg_online_cpus(void);
>>
>> +/**
>> + * Load ACPI tables from QEMU
>> + *
>> + * @addr:      address to load
>> + * @return:    next address
>> + */
>> +unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr);
>> +
>>  #endif
>> --
>
> Regards,
> Bin

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

* [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge
  2016-01-16 13:23   ` Bin Meng
@ 2016-01-19  2:46     ` Miao Yan
  2016-01-19  9:25       ` Bin Meng
  0 siblings, 1 reply; 16+ messages in thread
From: Miao Yan @ 2016-01-19  2:46 UTC (permalink / raw)
  To: u-boot

Hi Bin,

2016-01-16 21:23 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
> Hi Miao,
>
> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>> Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)
>>
>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>> ---
>>  arch/x86/cpu/qemu/qemu.c                | 39 +++++++++++++++++++++++++++++++++
>>  arch/x86/include/asm/arch-qemu/device.h |  8 +++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
>> index 46111c9..e7d8a6c 100644
>> --- a/arch/x86/cpu/qemu/qemu.c
>> +++ b/arch/x86/cpu/qemu/qemu.c
>> @@ -15,6 +15,41 @@
>>
>>  static bool i440fx;
>>
>> +static void enable_pm_piix(void)
>> +{
>> +       u8 en;
>> +       u16 device, cmd;
>> +
>> +       device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID);
>> +       if (device != PCI_DEVICE_ID_INTEL_82371AB_3)
>> +               return;
>
> Guess the check is already covered in qemu_chipset_init().


Do you mean this check ?

    device = x86_pci_read_config16(PCI_BDF(0, 0, 0), PCI_DEVICE_ID);
    i440fx = (device == PCI_DEVICE_ID_INTEL_82441);

So is it guaranteed that PIIX_PM is always on that BDF ?

IMO, we are operating on another chipset, and we better make
sure it's the one we expect, besides, an extra check won't do any harm.


>
>> +
>> +       /* Set the PM I/O base. */
>
> nits: please remove the ending period. Please fix this globally in this file.
>
>> +       x86_pci_write_config32(PIIX_PM, PMBA, DEFAULT_PMBASE | 1);
>> +
>> +       /* Enable access to the PM I/O space. */
>> +       cmd = x86_pci_read_config16(PIIX_PM, PCI_COMMAND);
>> +       cmd |= PCI_COMMAND_IO;
>> +       x86_pci_write_config16(PIIX_PM, PCI_COMMAND, cmd);
>> +
>> +       /* PM I/O Space Enable (PMIOSE). */
>> +       en = x86_pci_read_config8(PIIX_PM, PMREGMISC);
>> +       en |= PMIOSE;
>> +       x86_pci_write_config8(PIIX_PM, PMREGMISC, en);
>> +}
>> +
>> +static void enable_pm_ich9(void)
>> +{
>> +       u16 device;
>> +
>> +       device = x86_pci_read_config16(ICH9_PM, PCI_DEVICE_ID);
>> +       if (device != PCI_DEVICE_ID_INTEL_ICH9_8)
>> +               return;
>
> Guess the check is already covered in qemu_chipset_init().
>
>> +
>> +       /* Set the PM I/O base. */
>> +       x86_pci_write_config32(ICH9_PM, PMBA, DEFAULT_PMBASE | 1);
>> +}
>> +
>>  static void qemu_chipset_init(void)
>>  {
>>         u16 device, xbcs;
>> @@ -53,10 +88,14 @@ static void qemu_chipset_init(void)
>>                 xbcs = x86_pci_read_config16(PIIX_ISA, XBCS);
>>                 xbcs |= APIC_EN;
>>                 x86_pci_write_config16(PIIX_ISA, XBCS, xbcs);
>> +
>> +               enable_pm_piix();
>>         } else {
>>                 /* Configure PCIe ECAM base address */
>>                 x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR,
>>                                        CONFIG_PCIE_ECAM_BASE | BAR_EN);
>> +
>> +               enable_pm_ich9();
>>         }
>>
>>         qemu_fwcfg_init();
>> diff --git a/arch/x86/include/asm/arch-qemu/device.h b/arch/x86/include/asm/arch-qemu/device.h
>> index 75a435e..2e11100 100644
>> --- a/arch/x86/include/asm/arch-qemu/device.h
>> +++ b/arch/x86/include/asm/arch-qemu/device.h
>> @@ -13,9 +13,17 @@
>>  #define PIIX_ISA       PCI_BDF(0, 1, 0)
>>  #define PIIX_IDE       PCI_BDF(0, 1, 1)
>>  #define PIIX_USB       PCI_BDF(0, 1, 2)
>> +#define PIIX_PM        PCI_BDF(0, 1, 3)
>> +#define ICH9_PM        PCI_BDF(0, 0x1f, 0)
>>  #define I440FX_VGA     PCI_BDF(0, 2, 0)
>>
>>  #define QEMU_Q35       PCI_BDF(0, 0, 0)
>>  #define Q35_VGA                PCI_BDF(0, 1, 0)
>>
>> +#define PMBA           0x40
>> +#define DEFAULT_PMBASE 0xe400
>
> See arch/x86/cpu/quark/Kconfig we have ACPI_PM1_BASE already. Maybe we
> need consolidate this to introduce a similar one for QEMU.

OK, will fix this.

>
>> +#define PM_IO_BASE     DEFAULT_PMBASE
>
> PM_IO_BASE is not referenced anywhere.
>
>> +#define PMREGMISC      0x80
>> +#define PMIOSE         (1 << 0)
>> +
>
> Please move these register defines to include/asm/arch-qemu/qemu.h

OK, will fix this.

>
>>  #endif /* _QEMU_DEVICE_H_ */
>> --
>
> Regards,
> Bin

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

* [U-Boot] [PATCH 4/4] x86: qemu: loading ACPI table from QEMU
  2016-01-16 13:24   ` Bin Meng
@ 2016-01-19  2:48     ` Miao Yan
  0 siblings, 0 replies; 16+ messages in thread
From: Miao Yan @ 2016-01-19  2:48 UTC (permalink / raw)
  To: u-boot

Hi Bin,

2016-01-16 21:24 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
> Hi Miao,
>
> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>> If CONFIG_GENERATE_ACPI_TABLE is not defined, then use ACPI table created
>> by QEMU.
>>
>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>> ---
>>  arch/x86/lib/tables.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
>> index 14b15cf..1671385 100644
>> --- a/arch/x86/lib/tables.c
>> +++ b/arch/x86/lib/tables.c
>> @@ -10,6 +10,7 @@
>>  #include <asm/smbios.h>
>>  #include <asm/tables.h>
>>  #include <asm/acpi_table.h>
>> +#include <asm/fw_cfg.h>
>>
>>  u8 table_compute_checksum(void *v, int len)
>>  {
>> @@ -55,8 +56,10 @@ void write_tables(void)
>>  #endif
>>  #ifdef CONFIG_GENERATE_ACPI_TABLE
>>         rom_table_end = write_acpi_tables(rom_table_end);
>> -       rom_table_end = ALIGN(rom_table_end, 1024);
>> +#else
>> +       rom_table_end = qemu_fwcfg_write_acpi_tables(rom_table_end);
>
> This breaks other x86 boards.Can we hide this changes in acpi_table.c
> with proper #ifdef and some comments?
>
> For QEMU ACPI, how about:
>
> - GENERATE_ACPI_TABLE is the overall switch to turn on ACPI table by U-Boot
> - Introduce another Kconfig option for QEMU to generate ACPI tables
> from firmware interface. If this is turned on, the original method
> provided in acpi_tables.c will not be used.

Sounds OK, I'll drop this one. Thanks.

>
>>  #endif
>> +       rom_table_end = ALIGN(rom_table_end, 1024);
>>  #ifdef CONFIG_GENERATE_SMBIOS_TABLE
>>         rom_table_end = write_smbios_table(rom_table_end);
>>         rom_table_end = ALIGN(rom_table_end, 1024);
>> --
>
> Regards,
> Bin

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

* [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge
  2016-01-19  2:46     ` Miao Yan
@ 2016-01-19  9:25       ` Bin Meng
  2016-01-20  1:58         ` Miao Yan
  0 siblings, 1 reply; 16+ messages in thread
From: Bin Meng @ 2016-01-19  9:25 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Tue, Jan 19, 2016 at 10:46 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> Hi Bin,
>
> 2016-01-16 21:23 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
>> Hi Miao,
>>
>> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>>> Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)
>>>
>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>> ---
>>>  arch/x86/cpu/qemu/qemu.c                | 39 +++++++++++++++++++++++++++++++++
>>>  arch/x86/include/asm/arch-qemu/device.h |  8 +++++++
>>>  2 files changed, 47 insertions(+)
>>>
>>> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
>>> index 46111c9..e7d8a6c 100644
>>> --- a/arch/x86/cpu/qemu/qemu.c
>>> +++ b/arch/x86/cpu/qemu/qemu.c
>>> @@ -15,6 +15,41 @@
>>>
>>>  static bool i440fx;
>>>
>>> +static void enable_pm_piix(void)
>>> +{
>>> +       u8 en;
>>> +       u16 device, cmd;
>>> +
>>> +       device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID);
>>> +       if (device != PCI_DEVICE_ID_INTEL_82371AB_3)
>>> +               return;
>>
>> Guess the check is already covered in qemu_chipset_init().
>
>
> Do you mean this check ?
>
>     device = x86_pci_read_config16(PCI_BDF(0, 0, 0), PCI_DEVICE_ID);
>     i440fx = (device == PCI_DEVICE_ID_INTEL_82441);
>
> So is it guaranteed that PIIX_PM is always on that BDF ?

I believe so. If you look at the codes in qemu.c, the variable "static
bool i440fx" is used to distinguish QEMU machine pc and q35. It does
not check whether the chipset is i440fx, or PIIX which is the chipset
connected to i440fx.

>
> IMO, we are operating on another chipset, and we better make
> sure it's the one we expect, besides, an extra check won't do any harm.
>

Yes, that makes sense. So if we go with your way, maybe we need expand
"static bool i440fx" to multiple variables and use correct variable to
check? But this looks a bit complex than a single variable.

>
>>
>>> +
>>> +       /* Set the PM I/O base. */
>>
>> nits: please remove the ending period. Please fix this globally in this file.
>>
>>> +       x86_pci_write_config32(PIIX_PM, PMBA, DEFAULT_PMBASE | 1);
>>> +
>>> +       /* Enable access to the PM I/O space. */
>>> +       cmd = x86_pci_read_config16(PIIX_PM, PCI_COMMAND);
>>> +       cmd |= PCI_COMMAND_IO;
>>> +       x86_pci_write_config16(PIIX_PM, PCI_COMMAND, cmd);
>>> +
>>> +       /* PM I/O Space Enable (PMIOSE). */
>>> +       en = x86_pci_read_config8(PIIX_PM, PMREGMISC);
>>> +       en |= PMIOSE;
>>> +       x86_pci_write_config8(PIIX_PM, PMREGMISC, en);
>>> +}
>>> +
>>> +static void enable_pm_ich9(void)
>>> +{
>>> +       u16 device;
>>> +
>>> +       device = x86_pci_read_config16(ICH9_PM, PCI_DEVICE_ID);
>>> +       if (device != PCI_DEVICE_ID_INTEL_ICH9_8)
>>> +               return;
>>
>> Guess the check is already covered in qemu_chipset_init().
>>
>>> +
>>> +       /* Set the PM I/O base. */
>>> +       x86_pci_write_config32(ICH9_PM, PMBA, DEFAULT_PMBASE | 1);
>>> +}
>>> +
>>>  static void qemu_chipset_init(void)
>>>  {
>>>         u16 device, xbcs;
>>> @@ -53,10 +88,14 @@ static void qemu_chipset_init(void)
>>>                 xbcs = x86_pci_read_config16(PIIX_ISA, XBCS);
>>>                 xbcs |= APIC_EN;
>>>                 x86_pci_write_config16(PIIX_ISA, XBCS, xbcs);
>>> +
>>> +               enable_pm_piix();
>>>         } else {
>>>                 /* Configure PCIe ECAM base address */
>>>                 x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR,
>>>                                        CONFIG_PCIE_ECAM_BASE | BAR_EN);
>>> +
>>> +               enable_pm_ich9();
>>>         }
>>>
>>>         qemu_fwcfg_init();
>>> diff --git a/arch/x86/include/asm/arch-qemu/device.h b/arch/x86/include/asm/arch-qemu/device.h
>>> index 75a435e..2e11100 100644
>>> --- a/arch/x86/include/asm/arch-qemu/device.h
>>> +++ b/arch/x86/include/asm/arch-qemu/device.h
>>> @@ -13,9 +13,17 @@
>>>  #define PIIX_ISA       PCI_BDF(0, 1, 0)
>>>  #define PIIX_IDE       PCI_BDF(0, 1, 1)
>>>  #define PIIX_USB       PCI_BDF(0, 1, 2)
>>> +#define PIIX_PM        PCI_BDF(0, 1, 3)
>>> +#define ICH9_PM        PCI_BDF(0, 0x1f, 0)
>>>  #define I440FX_VGA     PCI_BDF(0, 2, 0)
>>>
>>>  #define QEMU_Q35       PCI_BDF(0, 0, 0)
>>>  #define Q35_VGA                PCI_BDF(0, 1, 0)
>>>
>>> +#define PMBA           0x40
>>> +#define DEFAULT_PMBASE 0xe400
>>
>> See arch/x86/cpu/quark/Kconfig we have ACPI_PM1_BASE already. Maybe we
>> need consolidate this to introduce a similar one for QEMU.
>
> OK, will fix this.
>
>>
>>> +#define PM_IO_BASE     DEFAULT_PMBASE
>>
>> PM_IO_BASE is not referenced anywhere.
>>
>>> +#define PMREGMISC      0x80
>>> +#define PMIOSE         (1 << 0)
>>> +
>>
>> Please move these register defines to include/asm/arch-qemu/qemu.h
>
> OK, will fix this.
>
>>
>>>  #endif /* _QEMU_DEVICE_H_ */
>>> --
>>

Regards,
Bin

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

* [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU
  2016-01-19  2:39     ` Miao Yan
@ 2016-01-19  9:25       ` Bin Meng
  0 siblings, 0 replies; 16+ messages in thread
From: Bin Meng @ 2016-01-19  9:25 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Tue, Jan 19, 2016 at 10:39 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> Hi Bin,
>
> 2016-01-16 21:24 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
>> Hi Miao,
>>
>> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>>> This patch adds the ability to load and link ACPI tables provided by QEMU.
>>> QEMU tells guests how to load and patch ACPI tables through its fw_cfg
>>> interface, by adding a firmware file 'etc/table-loader'. Guests are
>>> supposed to parse this file and execute corresponding QEMU commands.
>>>
>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>> ---
>>>  arch/x86/cpu/qemu/fw_cfg.c    | 214 ++++++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/include/asm/fw_cfg.h |  70 ++++++++++++++
>>>  2 files changed, 284 insertions(+)
>>>
>>> diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c
>>> index b22026c..755676c 100644
>>> --- a/arch/x86/cpu/qemu/fw_cfg.c
>>> +++ b/arch/x86/cpu/qemu/fw_cfg.c
>>> @@ -10,7 +10,10 @@
>>>  #include <malloc.h>
>>>  #include <asm/io.h>
>>>  #include <asm/fw_cfg.h>
>>> +#include <asm/tables.h>
>>> +#include <asm/e820.h>
>>>  #include <linux/list.h>
>>> +#include <memalign.h>
>>>
>>>  static bool fwcfg_present;
>>>  static bool fwcfg_dma_present;
>>> @@ -202,6 +205,217 @@ err:
>>>         return -ENOMEM;
>>>  }
>>>
>>> +static struct fw_file *qemu_fwcfg_find_file(const char *name)
>>> +{
>>> +       struct list_head *entry;
>>> +       struct fw_file *file;
>>> +
>>> +       list_for_each(entry, &fw_list) {
>>> +               file = list_entry(entry, struct fw_file, list);
>>> +               if (!strcmp(file->cfg.name, name))
>>> +                       return file;
>>> +       }
>>> +
>>> +       return NULL;
>>> +}
>>> +
>>> +static int bios_linker_allocate(struct bios_linker_entry *entry,
>>> +                          unsigned long *addr)
>>
>> Please add a comment block for what this function is doing, its
>> parameters, return value, etc. Please do the same for the other 2
>> functions below.
>>
>>> +{
>>> +       uint32_t size, align;
>>> +       struct fw_file *file;
>>> +       unsigned long aligned_addr;
>>> +
>>> +       align = le32_to_cpu(entry->alloc.align);
>>> +       /* align must be power of 2 */
>>> +       if (align & (align - 1)) {
>>> +               printf("error: wrong alignment %u\n", align);
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       file = qemu_fwcfg_find_file(entry->alloc.file);
>>> +       if (!file) {
>>> +               printf("error: can't find file %s\n", entry->alloc.file);
>>> +               return -ENOENT;
>>> +       }
>>> +
>>> +       size = be32_to_cpu(file->cfg.size);
>>> +
>>> +       /*
>>> +        * ZONE_HIGH means we need to allocate from high memory, since
>>> +        * malloc space is already at the end of RAM, so we directly use it.
>>> +        * If allocation zone is ZONE_FSEG, then we use the 'addr' passed
>>> +        * in which is low memory
>>> +        */
>>> +       if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH) {
>>> +               aligned_addr = (unsigned long)memalign(align, size);
>>> +       } else if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG) {
>>> +               aligned_addr = ALIGN(*addr, align);
>>> +       } else {
>>> +               printf("error: invalid allocation zone\n");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       debug("bios_linker_allocate: allocate file %s, size %u, zone %d, align %u, addr 0x%lx\n",
>>> +             file->cfg.name, size, entry->alloc.zone, align, aligned_addr);
>>> +
>>> +       qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
>>> +                             size, (void *)aligned_addr);
>>> +       file->addr = aligned_addr;
>>> +
>>> +       /* adjust address for low memory allocation */
>>> +       if (entry->alloc.zone == BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG)
>>> +               *addr = (aligned_addr + size);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int bios_linker_add_pointer(struct bios_linker_entry *entry)
>>> +{
>>> +       struct fw_file *dest, *src;
>>> +       uint32_t offset = le32_to_cpu(entry->pointer.offset);
>>> +       uint64_t pointer = 0;
>>> +
>>> +       dest = qemu_fwcfg_find_file(entry->pointer.dest_file);
>>> +       if (!dest || !dest->addr)
>>> +               return -ENOENT;
>>> +       src = qemu_fwcfg_find_file(entry->pointer.src_file);
>>> +       if (!src || !src->addr)
>>> +               return -ENOENT;
>>> +
>>
>> Remove one blank line here.
>>
>>> +
>>> +       memcpy(&pointer, (char *)dest->addr + offset, entry->pointer.size);
>>> +       pointer = le64_to_cpu(pointer);
>>> +
>>
>> Remove one blank line here.
>>
>>> +
>>> +       debug("bios_linker_add_pointer: dest->addr 0x%lx, src->addr 0x%lx, offset 0x%x size %u, 0x%llx\n",
>>> +             dest->addr, src->addr, offset, entry->pointer.size, pointer);
>>> +
>>> +       pointer += (unsigned long)src->addr;
>>> +       pointer = cpu_to_le64(pointer);
>>> +       memcpy((char *)dest->addr + offset, &pointer, entry->pointer.size);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int bios_linker_add_checksum(struct bios_linker_entry *entry)
>>> +{
>>> +       struct fw_file *file;
>>> +       uint8_t *data, cksum = 0;
>>> +       uint8_t *cksum_start;
>>> +
>>> +       file = qemu_fwcfg_find_file(entry->cksum.file);
>>> +       if (!file || !file->addr)
>>> +               return -ENOENT;
>>> +
>>> +       data = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.offset));
>>> +       cksum_start = (uint8_t *)(file->addr + le32_to_cpu(entry->cksum.start));
>>> +       cksum = table_compute_checksum(cksum_start,
>>> +                                      le32_to_cpu(entry->cksum.length));
>>> +       *data = cksum;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
>>> +{
>>> +       entries[0].addr = 0;
>>> +       entries[0].size = ISA_START_ADDRESS;
>>> +       entries[0].type = E820_RAM;
>>> +
>>> +       entries[1].addr = ISA_START_ADDRESS;
>>> +       entries[1].size = ISA_END_ADDRESS - ISA_START_ADDRESS;
>>> +       entries[1].type = E820_RESERVED;
>>> +
>>> +       /*
>>> +        * since we use memalign(malloc) to allocate high memory for
>>> +        * storing ACPI tables, we need to reserve them in e820 tables,
>>> +        * otherwise kernel will reclaim them and data will be corrupted
>>> +        */
>>> +       entries[2].addr = ISA_END_ADDRESS;
>>> +       entries[2].size = gd->relocaddr - TOTAL_MALLOC_LEN - ISA_END_ADDRESS;
>>> +       entries[2].type = E820_RAM;
>>> +
>>> +       /* for simplicity, reserve entire malloc space */
>>> +       entries[3].addr = gd->relocaddr - TOTAL_MALLOC_LEN;
>>> +       entries[3].size = TOTAL_MALLOC_LEN;
>>> +       entries[3].type = E820_RESERVED;
>>> +
>>> +       entries[4].addr = gd->relocaddr;
>>> +       entries[4].size = gd->ram_size - gd->relocaddr;
>>> +       entries[4].type = E820_RESERVED;
>>> +
>>> +       entries[5].addr = CONFIG_PCIE_ECAM_BASE;
>>> +       entries[5].size = CONFIG_PCIE_ECAM_SIZE;
>>> +       entries[5].type = E820_RESERVED;
>>> +
>>> +       return 6;
>>> +}
>>> +
>>> +/* This function loads and patches ACPI tables provided by QEMU */
>>> +unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr)
>>> +{
>>> +       int i, ret = 0;
>>> +       struct fw_file *file;
>>> +       struct bios_linker_entry *table_loader;
>>> +       struct bios_linker_entry *entry;
>>> +       uint32_t size;
>>> +
>>> +       /* make sure fw_list is loaded */
>>> +       ret = qemu_fwcfg_read_firmware_list();
>>> +       if (ret) {
>>> +               printf("error: can't read firmware file list\n");
>>> +               return addr;
>>> +       }
>>> +
>>> +       file = qemu_fwcfg_find_file("etc/table-loader");
>>> +       if (!file) {
>>> +               printf("error: can't find etc/table-loader\n");
>>> +               return addr;
>>> +       }
>>> +
>>> +       size = be32_to_cpu(file->cfg.size);
>>> +       if ((size % sizeof(*entry)) != 0) {
>>> +               printf("error: table loader maybe corrupted\n");
>>
>> table-loader may be
>>
>>> +               return addr;
>>> +       }
>>> +
>>> +       table_loader = malloc(size);
>>> +       if (!table_loader) {
>>> +               printf("error: no memory for table-loader\n");
>>> +               return addr;
>>> +       }
>>> +
>>> +       qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
>>> +                             size, table_loader);
>>> +
>>> +       for (i = 0; i < (size / sizeof(*entry)); i++) {
>>> +               entry = table_loader + i;
>>> +               switch (le32_to_cpu(entry->command)) {
>>> +               case BIOS_LINKER_LOADER_COMMAND_ALLOCATE:
>>> +                       ret = bios_linker_allocate(entry, &addr);
>>> +                       if (ret)
>>> +                               goto err;
>>> +                       break;
>>> +               case BIOS_LINKER_LOADER_COMMAND_ADD_POINTER:
>>> +                       ret = bios_linker_add_pointer(entry);
>>> +                       if (ret)
>>> +                               goto err;
>>> +                       break;
>>> +               case BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM:
>>> +                       ret = bios_linker_add_checksum(entry);
>>> +                       if (ret)
>>> +                               goto err;
>>> +                       break;
>>> +               default:
>>> +                       break;
>>> +               }
>>> +       }
>>> +err:
>>> +       free(table_loader);
>>> +       return addr;
>>> +}
>>> +
>>>  static int qemu_fwcfg_list_firmware(void)
>>>  {
>>>         int ret;
>>> diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h
>>> index 285d805..ad58205 100644
>>> --- a/arch/x86/include/asm/fw_cfg.h
>>> +++ b/arch/x86/include/asm/fw_cfg.h
>>> @@ -47,11 +47,23 @@ enum qemu_fwcfg_items {
>>>         FW_CFG_INVALID          = 0xffff,
>>>  };
>>>
>>> +enum {
>>> +       BIOS_LINKER_LOADER_COMMAND_ALLOCATE     = 0x1,
>>> +       BIOS_LINKER_LOADER_COMMAND_ADD_POINTER  = 0x2,
>>> +       BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3,
>>> +};
>>> +
>>> +enum {
>>> +       BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1,
>>> +       BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2,
>>> +};
>>> +
>>>  #define FW_CFG_FILE_SLOTS      0x10
>>>  #define FW_CFG_MAX_ENTRY       (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS)
>>>  #define FW_CFG_ENTRY_MASK       ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
>>>
>>>  #define FW_CFG_MAX_FILE_PATH   56
>>> +#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH
>>
>> Use FW_CFG_MAX_FILE_PATH directly?
>
> Those codes are derived from QEMU, with some modifications to make
> them compatible with U-Boot source. So I think maybe we should keep it
> in sync with QEMU.
>
>

OK, makes sense.

>>
>>>
>>>  #define QEMU_FW_CFG_SIGNATURE  (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
>>>
>>> @@ -69,6 +81,7 @@ struct fw_cfg_file {
>>>         char name[FW_CFG_MAX_FILE_PATH];
>>>  };
>>>
>>> +
>>>  struct fw_file {
>>>         struct fw_cfg_file cfg;
>>>         unsigned long addr;
>>> @@ -86,6 +99,55 @@ struct fw_cfg_dma_access {
>>>         __be64 address;
>>>  };
>>>
>>> +struct bios_linker_entry {
>>> +       __le32 command;
>>> +       union {
>>> +               /*
>>> +                * COMMAND_ALLOCATE - allocate a table from @alloc.file
>>> +                * subject to @alloc.align alignment (must be power of 2)
>>> +                * and @alloc.zone (can be HIGH or FSEG) requirements.
>>> +                *
>>> +                * Must appear exactly once for each file, and before
>>> +                * this file is referenced by any other command.
>>> +                */
>>> +               struct {
>>> +                       char file[BIOS_LINKER_LOADER_FILESZ];
>>
>> Use FW_CFG_MAX_FILE_PATH directly?
>>
>>> +                       __le32 align;
>>> +                       uint8_t zone;
>>> +               } alloc;
>>> +
>>> +               /*
>>> +                * COMMAND_ADD_POINTER - patch the table (originating from
>>> +                * @dest_file) at @pointer.offset, by adding a pointer to the
>>> +                * table originating from @src_file. 1,2,4 or 8 byte unsigned
>>> +                * addition is used depending on @pointer.size.
>>> +                */
>>> +               struct {
>>> +                       char dest_file[BIOS_LINKER_LOADER_FILESZ];
>>> +                       char src_file[BIOS_LINKER_LOADER_FILESZ];
>>> +                       __le32 offset;
>>> +                       uint8_t size;
>>> +               } pointer;
>>> +
>>> +               /*
>>> +                * COMMAND_ADD_CHECKSUM - calculate checksum of the range
>>> +                * specified by @cksum_start and @cksum_length fields,
>>> +                * and then add the value at @cksum.offset.
>>> +                * Checksum simply sums -X for each byte X in the range
>>> +                * using 8-bit math.
>>> +                */
>>> +               struct {
>>> +                       char file[BIOS_LINKER_LOADER_FILESZ];
>>> +                       __le32 offset;
>>> +                       __le32 start;
>>> +                       __le32 length;
>>> +               } cksum;
>>> +
>>> +               /* padding */
>>> +               char pad[124];
>>> +       };
>>> +} __packed;
>>> +
>>>  /**
>>>   * Initialize QEMU fw_cfg interface
>>>   */
>>> @@ -98,4 +160,12 @@ void qemu_fwcfg_init(void);
>>>   */
>>>  int qemu_fwcfg_online_cpus(void);
>>>
>>> +/**
>>> + * Load ACPI tables from QEMU
>>> + *
>>> + * @addr:      address to load
>>> + * @return:    next address
>>> + */
>>> +unsigned long qemu_fwcfg_write_acpi_tables(unsigned long addr);
>>> +
>>>  #endif
>>> --

Regards,
Bin

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

* [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge
  2016-01-19  9:25       ` Bin Meng
@ 2016-01-20  1:58         ` Miao Yan
  2016-01-20  2:13           ` Bin Meng
  0 siblings, 1 reply; 16+ messages in thread
From: Miao Yan @ 2016-01-20  1:58 UTC (permalink / raw)
  To: u-boot

Hi Bin,

2016-01-19 17:25 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
> Hi Miao,
>
> On Tue, Jan 19, 2016 at 10:46 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>> Hi Bin,
>>
>> 2016-01-16 21:23 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
>>> Hi Miao,
>>>
>>> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>>>> Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)
>>>>
>>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>>> ---
>>>>  arch/x86/cpu/qemu/qemu.c                | 39 +++++++++++++++++++++++++++++++++
>>>>  arch/x86/include/asm/arch-qemu/device.h |  8 +++++++
>>>>  2 files changed, 47 insertions(+)
>>>>
>>>> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
>>>> index 46111c9..e7d8a6c 100644
>>>> --- a/arch/x86/cpu/qemu/qemu.c
>>>> +++ b/arch/x86/cpu/qemu/qemu.c
>>>> @@ -15,6 +15,41 @@
>>>>
>>>>  static bool i440fx;
>>>>
>>>> +static void enable_pm_piix(void)
>>>> +{
>>>> +       u8 en;
>>>> +       u16 device, cmd;
>>>> +
>>>> +       device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID);
>>>> +       if (device != PCI_DEVICE_ID_INTEL_82371AB_3)
>>>> +               return;
>>>
>>> Guess the check is already covered in qemu_chipset_init().
>>
>>
>> Do you mean this check ?
>>
>>     device = x86_pci_read_config16(PCI_BDF(0, 0, 0), PCI_DEVICE_ID);
>>     i440fx = (device == PCI_DEVICE_ID_INTEL_82441);
>>
>> So is it guaranteed that PIIX_PM is always on that BDF ?
>
> I believe so. If you look at the codes in qemu.c, the variable "static
> bool i440fx" is used to distinguish QEMU machine pc and q35. It does
> not check whether the chipset is i440fx, or PIIX which is the chipset
> connected to i440fx.
>
>>
>> IMO, we are operating on another chipset, and we better make
>> sure it's the one we expect, besides, an extra check won't do any harm.
>>
>
> Yes, that makes sense. So if we go with your way, maybe we need expand
> "static bool i440fx" to multiple variables and use correct variable to
> check? But this looks a bit complex than a single variable.
>

Yes, that's a little bit complex and not necessary if their PCI
addresses are fixed . And I don't think we should do it in this
patchset.

So how do you suggest we do this ? Either I remove the two checks to
make it aligned with the rest or create a separate patch to do the
checks ?



>>
>>>
>>>> +
>>>> +       /* Set the PM I/O base. */
>>>
>>> nits: please remove the ending period. Please fix this globally in this file.
>>>
>>>> +       x86_pci_write_config32(PIIX_PM, PMBA, DEFAULT_PMBASE | 1);
>>>> +
>>>> +       /* Enable access to the PM I/O space. */
>>>> +       cmd = x86_pci_read_config16(PIIX_PM, PCI_COMMAND);
>>>> +       cmd |= PCI_COMMAND_IO;
>>>> +       x86_pci_write_config16(PIIX_PM, PCI_COMMAND, cmd);
>>>> +
>>>> +       /* PM I/O Space Enable (PMIOSE). */
>>>> +       en = x86_pci_read_config8(PIIX_PM, PMREGMISC);
>>>> +       en |= PMIOSE;
>>>> +       x86_pci_write_config8(PIIX_PM, PMREGMISC, en);
>>>> +}
>>>> +
>>>> +static void enable_pm_ich9(void)
>>>> +{
>>>> +       u16 device;
>>>> +
>>>> +       device = x86_pci_read_config16(ICH9_PM, PCI_DEVICE_ID);
>>>> +       if (device != PCI_DEVICE_ID_INTEL_ICH9_8)
>>>> +               return;
>>>
>>> Guess the check is already covered in qemu_chipset_init().
>>>
>>>> +
>>>> +       /* Set the PM I/O base. */
>>>> +       x86_pci_write_config32(ICH9_PM, PMBA, DEFAULT_PMBASE | 1);
>>>> +}
>>>> +
>>>>  static void qemu_chipset_init(void)
>>>>  {
>>>>         u16 device, xbcs;
>>>> @@ -53,10 +88,14 @@ static void qemu_chipset_init(void)
>>>>                 xbcs = x86_pci_read_config16(PIIX_ISA, XBCS);
>>>>                 xbcs |= APIC_EN;
>>>>                 x86_pci_write_config16(PIIX_ISA, XBCS, xbcs);
>>>> +
>>>> +               enable_pm_piix();
>>>>         } else {
>>>>                 /* Configure PCIe ECAM base address */
>>>>                 x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR,
>>>>                                        CONFIG_PCIE_ECAM_BASE | BAR_EN);
>>>> +
>>>> +               enable_pm_ich9();
>>>>         }
>>>>
>>>>         qemu_fwcfg_init();
>>>> diff --git a/arch/x86/include/asm/arch-qemu/device.h b/arch/x86/include/asm/arch-qemu/device.h
>>>> index 75a435e..2e11100 100644
>>>> --- a/arch/x86/include/asm/arch-qemu/device.h
>>>> +++ b/arch/x86/include/asm/arch-qemu/device.h
>>>> @@ -13,9 +13,17 @@
>>>>  #define PIIX_ISA       PCI_BDF(0, 1, 0)
>>>>  #define PIIX_IDE       PCI_BDF(0, 1, 1)
>>>>  #define PIIX_USB       PCI_BDF(0, 1, 2)
>>>> +#define PIIX_PM        PCI_BDF(0, 1, 3)
>>>> +#define ICH9_PM        PCI_BDF(0, 0x1f, 0)
>>>>  #define I440FX_VGA     PCI_BDF(0, 2, 0)
>>>>
>>>>  #define QEMU_Q35       PCI_BDF(0, 0, 0)
>>>>  #define Q35_VGA                PCI_BDF(0, 1, 0)
>>>>
>>>> +#define PMBA           0x40
>>>> +#define DEFAULT_PMBASE 0xe400
>>>
>>> See arch/x86/cpu/quark/Kconfig we have ACPI_PM1_BASE already. Maybe we
>>> need consolidate this to introduce a similar one for QEMU.
>>
>> OK, will fix this.
>>
>>>
>>>> +#define PM_IO_BASE     DEFAULT_PMBASE
>>>
>>> PM_IO_BASE is not referenced anywhere.
>>>
>>>> +#define PMREGMISC      0x80
>>>> +#define PMIOSE         (1 << 0)
>>>> +
>>>
>>> Please move these register defines to include/asm/arch-qemu/qemu.h
>>
>> OK, will fix this.
>>
>>>
>>>>  #endif /* _QEMU_DEVICE_H_ */
>>>> --
>>>
>
> Regards,
> Bin

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

* [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge
  2016-01-20  1:58         ` Miao Yan
@ 2016-01-20  2:13           ` Bin Meng
  0 siblings, 0 replies; 16+ messages in thread
From: Bin Meng @ 2016-01-20  2:13 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Wed, Jan 20, 2016 at 9:58 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> Hi Bin,
>
> 2016-01-19 17:25 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
>> Hi Miao,
>>
>> On Tue, Jan 19, 2016 at 10:46 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>>> Hi Bin,
>>>
>>> 2016-01-16 21:23 GMT+08:00 Bin Meng <bmeng.cn@gmail.com>:
>>>> Hi Miao,
>>>>
>>>> On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
>>>>> Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)
>>>>>
>>>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>>>> ---
>>>>>  arch/x86/cpu/qemu/qemu.c                | 39 +++++++++++++++++++++++++++++++++
>>>>>  arch/x86/include/asm/arch-qemu/device.h |  8 +++++++
>>>>>  2 files changed, 47 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
>>>>> index 46111c9..e7d8a6c 100644
>>>>> --- a/arch/x86/cpu/qemu/qemu.c
>>>>> +++ b/arch/x86/cpu/qemu/qemu.c
>>>>> @@ -15,6 +15,41 @@
>>>>>
>>>>>  static bool i440fx;
>>>>>
>>>>> +static void enable_pm_piix(void)
>>>>> +{
>>>>> +       u8 en;
>>>>> +       u16 device, cmd;
>>>>> +
>>>>> +       device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID);
>>>>> +       if (device != PCI_DEVICE_ID_INTEL_82371AB_3)
>>>>> +               return;
>>>>
>>>> Guess the check is already covered in qemu_chipset_init().
>>>
>>>
>>> Do you mean this check ?
>>>
>>>     device = x86_pci_read_config16(PCI_BDF(0, 0, 0), PCI_DEVICE_ID);
>>>     i440fx = (device == PCI_DEVICE_ID_INTEL_82441);
>>>
>>> So is it guaranteed that PIIX_PM is always on that BDF ?
>>
>> I believe so. If you look at the codes in qemu.c, the variable "static
>> bool i440fx" is used to distinguish QEMU machine pc and q35. It does
>> not check whether the chipset is i440fx, or PIIX which is the chipset
>> connected to i440fx.
>>
>>>
>>> IMO, we are operating on another chipset, and we better make
>>> sure it's the one we expect, besides, an extra check won't do any harm.
>>>
>>
>> Yes, that makes sense. So if we go with your way, maybe we need expand
>> "static bool i440fx" to multiple variables and use correct variable to
>> check? But this looks a bit complex than a single variable.
>>
>
> Yes, that's a little bit complex and not necessary if their PCI
> addresses are fixed . And I don't think we should do it in this
> patchset.
>
> So how do you suggest we do this ? Either I remove the two checks to
> make it aligned with the rest or create a separate patch to do the
> checks ?
>

I suggest we do it in existing way (single variable).

[snip]

Regards,
Bin

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

end of thread, other threads:[~2016-01-20  2:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-15  3:12 [U-Boot] [PATCH 0/4] add support for loading ACPI tables from QEMU Miao Yan
2016-01-15  3:12 ` [U-Boot] [PATCH 1/4] x86: qemu: re-structure qemu_fwcfg_list_firmware() Miao Yan
2016-01-16 13:23   ` Bin Meng
2016-01-15  3:12 ` [U-Boot] [PATCH 2/4] x86: qemu: setup PM IO base for ACPI in southbridge Miao Yan
2016-01-16 13:23   ` Bin Meng
2016-01-19  2:46     ` Miao Yan
2016-01-19  9:25       ` Bin Meng
2016-01-20  1:58         ` Miao Yan
2016-01-20  2:13           ` Bin Meng
2016-01-15  3:12 ` [U-Boot] [PATCH 3/4] x86: qemu: add the ability to load and link ACPI tables from QEMU Miao Yan
2016-01-16 13:24   ` Bin Meng
2016-01-19  2:39     ` Miao Yan
2016-01-19  9:25       ` Bin Meng
2016-01-15  3:12 ` [U-Boot] [PATCH 4/4] x86: qemu: loading ACPI table " Miao Yan
2016-01-16 13:24   ` Bin Meng
2016-01-19  2:48     ` Miao Yan

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.