Hi,
The q35 C code bits for review. The confusing mcfg acpi table code is largely rewritten, otherwise q35 initialization is almost unmodified.
DSDT isn't tackled yet.
cheers, Gerd
Gerd Hoffmann (1): pci/acpi: add mcfg table for mmconfig
Isaku Yamahata (1): seabios: add q35 initialization functions.
src/acpi.c | 54 ++++++++++++++++++++++++++++++- src/acpi.h | 17 ++++++++++ src/dev-q35.h | 46 +++++++++++++++++++++++++++ src/pci.h | 1 + src/pciinit.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/shadow.c | 13 ++++++++ src/smm.c | 37 ++++++++++++++++++++++ 7 files changed, 262 insertions(+), 3 deletions(-) create mode 100644 src/dev-q35.h
From: Isaku Yamahata yamahata@valinux.co.jp
add q35 initialization functions.
[jbaron@redhat.com: restructured to current seabios base, updated pci base to 0xb0000000]
Signed-off-by: Isaku Yamahata yamahata@valinux.co.jp Signed-off-by: Jason Baron jbaron@redhat.com --- src/acpi.c | 32 +++++++++++++++++++- src/dev-q35.h | 46 +++++++++++++++++++++++++++++ src/pciinit.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/shadow.c | 13 ++++++++ src/smm.c | 37 ++++++++++++++++++++++++ 5 files changed, 213 insertions(+), 3 deletions(-) create mode 100644 src/dev-q35.h
diff --git a/src/acpi.c b/src/acpi.c index 1fc170f..72c8fe8 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -13,6 +13,7 @@ #include "pci_regs.h" // PCI_INTERRUPT_LINE #include "ioport.h" // inl #include "paravirt.h" // qemu_cfg_irq0_override +#include "dev-q35.h" // qemu_cfg_irq0_override
/****************************************************/ /* ACPI tables init */ @@ -260,11 +261,38 @@ static void piix4_fadt_init(struct pci_device *pci, void *arg) (1 << 15)); }
+/* PCI_VENDOR_ID_INTEL && PCI_DEVICE_ID_INTEL_ICH9_LPC */ +void ich9_lpc_fadt_init(struct pci_device *dev, void *arg) +{ + struct fadt_descriptor_rev1 *fadt = arg; + + fadt->model = 1; + fadt->reserved1 = 0; + fadt->sci_int = cpu_to_le16(9); + fadt->smi_cmd = cpu_to_le32(PORT_SMI_CMD); + fadt->acpi_enable = ICH9_ACPI_ENABLE; + fadt->acpi_disable = ICH9_ACPI_DISABLE; + fadt->pm1a_evt_blk = cpu_to_le32(PORT_ACPI_PM_BASE); + fadt->pm1a_cnt_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x04); + fadt->pm_tmr_blk = cpu_to_le32(PORT_ACPI_PM_BASE + 0x08); + fadt->gpe0_blk = cpu_to_le32(PORT_ACPI_PM_BASE + ICH9_PMIO_GPE0_STS); + fadt->pm1_evt_len = 4; + fadt->pm1_cnt_len = 2; + fadt->pm_tmr_len = 4; + fadt->gpe0_blk_len = ICH9_PMIO_GPE0_BLK_LEN; + fadt->plvl2_lat = cpu_to_le16(0xfff); // C2 state not supported + fadt->plvl3_lat = cpu_to_le16(0xfff); // C3 state not supported + /* WBINVD + PROC_C1 + SLP_BUTTON + FIX_RTC + RTC_S4 */ + fadt->flags = cpu_to_le32((1 << 0) | (1 << 2) | (1 << 5) | (1 << 6) | + (1 << 7)); +} + static const struct pci_device_id fadt_init_tbl[] = { /* PIIX4 Power Management device (for ACPI) */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3, piix4_fadt_init), - + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_LPC, + ich9_lpc_fadt_init), PCI_DEVICE_END };
@@ -739,7 +767,7 @@ build_srat(void) static const struct pci_device_id acpi_find_tbl[] = { /* PIIX4 Power Management device. */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3, NULL), - + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_LPC, NULL), PCI_DEVICE_END, };
diff --git a/src/dev-q35.h b/src/dev-q35.h new file mode 100644 index 0000000..6ae039f --- /dev/null +++ b/src/dev-q35.h @@ -0,0 +1,46 @@ +#ifndef __DEV_Q35_H +#define __DEV_Q35_H + +#include "types.h" // u16 + +#define PCI_DEVICE_ID_INTEL_Q35_MCH 0x29c0 +#define Q35_HOST_BRIDGE_PAM0 0x90 +#define Q35_HOST_BRIDGE_SMRAM 0x9d +#define Q35_HOST_BRIDGE_PCIEXBAR 0x60 +#define Q35_HOST_BRIDGE_PCIEXBAR_SIZE (256 * 1024 * 1024) +#define Q35_HOST_BRIDGE_PCIEXBAR_ADDR 0xb0000000 +#define Q35_HOST_BRIDGE_PCIEXBAREN ((u64)1) +#define Q35_HOST_PCIE_PCI_SEGMENT 0 +#define Q35_HOST_PCIE_START_BUS_NUMBER 0 +#define Q35_HOST_PCIE_END_BUS_NUMBER 255 + +#define PCI_DEVICE_ID_INTEL_ICH9_LPC 0x2918 +#define ICH9_LPC_PMBASE 0x40 +#define ICH9_LPC_PMBASE_RTE 0x1 + +#define ICH9_LPC_ACPI_CTRL 0x44 +#define ICH9_LPC_ACPI_CTRL_ACPI_EN 0x80 +#define ICH9_LPC_PIRQA_ROUT 0x60 +#define ICH9_LPC_PIRQE_ROUT 0x68 +#define ICH9_LPC_PIRQ_ROUT_IRQEN 0x80 +#define ICH9_LPC_PORT_ELCR1 0x4d0 +#define ICH9_LPC_PORT_ELCR2 0x4d1 +#define PCI_DEVICE_ID_INTEL_ICH9_SMBUS 0x2930 +#define ICH9_SMB_SMB_BASE 0x20 +#define ICH9_SMB_HOSTC 0x40 +#define ICH9_SMB_HOSTC_HST_EN 0x01 + +#define ICH9_ACPI_ENABLE 0x2 +#define ICH9_ACPI_DISABLE 0x3 + +/* ICH9 LPC PM I/O registers are 128 ports and 128-aligned */ +#define ICH9_PMIO_GPE0_STS 0x20 +#define ICH9_PMIO_GPE0_BLK_LEN 0x10 +#define ICH9_PMIO_SMI_EN 0x30 +#define ICH9_PMIO_SMI_EN_APMC_EN (1 << 5) + +/* FADT ACPI_ENABLE/ACPI_DISABLE */ +#define ICH9_APM_ACPI_ENABLE 0x2 +#define ICH9_APM_ACPI_DISABLE 0x3 + +#endif // dev-q35.h diff --git a/src/pciinit.c b/src/pciinit.c index 6176e40..091d0e6 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -12,6 +12,8 @@ #include "ioport.h" // PORT_ATA1_CMD_BASE #include "config.h" // CONFIG_* #include "xen.h" // usingXen +#include "memmap.h" // add_e820 +#include "dev-q35.h"
#define PCI_DEVICE_MEM_MIN 0x1000 #define PCI_BRIDGE_IO_MIN 0x1000 @@ -121,12 +123,44 @@ static void piix_isa_bridge_init(struct pci_device *pci, void *arg) dprintf(1, "PIIX3/PIIX4 init: elcr=%02x %02x\n", elcr[0], elcr[1]); }
+/* ICH9 LPC PCI to ISA bridge */ +/* PCI_VENDOR_ID_INTEL && PCI_DEVICE_ID_INTEL_ICH9_LPC */ +void mch_isa_bridge_init(struct pci_device *dev, void *arg) +{ + u16 bdf = dev->bdf; + int i, irq; + u8 elcr[2]; + + elcr[0] = 0x00; + elcr[1] = 0x00; + + for (i = 0; i < 4; i++) { + irq = pci_irqs[i]; + /* set to trigger level */ + elcr[irq >> 3] |= (1 << (irq & 7)); + + /* activate irq remapping in LPC */ + + /* PIRQ[A-D] routing */ + pci_config_writeb(bdf, ICH9_LPC_PIRQA_ROUT + i, + irq | ICH9_LPC_PIRQ_ROUT_IRQEN); + /* PIRQ[E-H] routing */ + pci_config_writeb(bdf, ICH9_LPC_PIRQE_ROUT + i, + irq | ICH9_LPC_PIRQ_ROUT_IRQEN); + } + outb(elcr[0], ICH9_LPC_PORT_ELCR1); + outb(elcr[1], ICH9_LPC_PORT_ELCR2); + dprintf(1, "Q35 LPC init: elcr=%02x %02x\n", elcr[0], elcr[1]); +} + static const struct pci_device_id pci_isa_bridge_tbl[] = { /* PIIX3/PIIX4 PCI to ISA bridge */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371SB_0, piix_isa_bridge_init), PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_0, piix_isa_bridge_init), + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_LPC, + mch_isa_bridge_init),
PCI_DEVICE_END }; @@ -200,11 +234,42 @@ static void piix4_pm_init(struct pci_device *pci, void *arg) pmtimer_init(PORT_ACPI_PM_BASE + 0x08, PM_TIMER_FREQUENCY / 1000); }
+/* ICH9 LPC Power Management device (for ACPI) */ +/* PCI_VENDOR_ID_INTEL && PCI_DEVICE_ID_INTEL_ICH9_LPC */ +void ich9_lpc_pm_init(struct pci_device *dev, void *arg) +{ + u16 bdf = dev->bdf; + /* pm io base */ + pci_config_writel(bdf, ICH9_LPC_PMBASE, + PORT_ACPI_PM_BASE | ICH9_LPC_PMBASE_RTE); + + /* acpi enable, SCI: IRQ9 000b = irq9*/ + pci_config_writeb(bdf, ICH9_LPC_ACPI_CTRL, ICH9_LPC_ACPI_CTRL_ACPI_EN); + + pmtimer_init(PORT_ACPI_PM_BASE + 0x08, PM_TIMER_FREQUENCY / 1000); +} + +/* ICH9 SMBUS */ +/* PCI_VENDOR_ID_INTEL && PCI_DEVICE_ID_INTEL_ICH9_SMBUS */ +void ich9_smbus_init(struct pci_device *dev, void *arg) +{ + u16 bdf = dev->bdf; + /* map smbus into io space */ + pci_config_writel(bdf, ICH9_SMB_SMB_BASE, + PORT_SMB_BASE | PCI_BASE_ADDRESS_SPACE_IO); + + /* enable SMBus */ + pci_config_writeb(bdf, ICH9_SMB_HOSTC, ICH9_SMB_HOSTC_HST_EN); +} + static const struct pci_device_id pci_device_tbl[] = { /* PIIX4 Power Management device (for ACPI) */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3, piix4_pm_init), - + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_LPC, + ich9_lpc_pm_init), + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_SMBUS, + ich9_smbus_init), PCI_DEVICE_END, };
@@ -608,9 +673,30 @@ void i440fx_mem_addr_init(struct pci_device *dev, void *arg) mtrr_base = pcimem_start; }
+void mch_mem_addr_init(struct pci_device *dev, void *arg) +{ + u64 addr = Q35_HOST_BRIDGE_PCIEXBAR_ADDR; + u32 size = Q35_HOST_BRIDGE_PCIEXBAR_SIZE; + + /* setup mmconfig */ + u16 bdf = dev->bdf; + u32 upper = addr >> 32; + u32 lower = (addr & 0xffffffff) | Q35_HOST_BRIDGE_PCIEXBAREN; + pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR, 0); + pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR + 4, upper); + pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR, lower); + add_e820(addr, size, E820_RESERVED); + + /* setup pci i/o window (above mmconfig) */ + pcimem_start = addr + size; + mtrr_base = addr + size; +} + static const struct pci_device_id pci_mem_addr_tbl[] = { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441, i440fx_mem_addr_init), + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q35_MCH, + mch_mem_addr_init), PCI_DEVICE_END, };
diff --git a/src/shadow.c b/src/shadow.c index 11c4d5e..a2195da 100644 --- a/src/shadow.c +++ b/src/shadow.c @@ -11,6 +11,7 @@ #include "pci_ids.h" // PCI_VENDOR_ID_INTEL #include "pci_regs.h" // PCI_VENDOR_ID #include "xen.h" // usingXen +#include "dev-q35.h" // PCI_VENDOR_ID_INTEL
// On the emulators, the bios at 0xf0000 is also at 0xffff0000 #define BIOS_SRC_OFFSET 0xfff00000 @@ -101,9 +102,16 @@ static void i440fx_bios_make_readonly(struct pci_device *pci, void *arg) make_bios_readonly_intel(pci->bdf, I440FX_PAM0); }
+void mch_bios_make_readonly(struct pci_device *pci, void *arg) +{ + make_bios_readonly_intel(pci->bdf, Q35_HOST_BRIDGE_PAM0); +} + static const struct pci_device_id dram_controller_make_readonly_tbl[] = { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82441, i440fx_bios_make_readonly), + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q35_MCH, + mch_bios_make_readonly), PCI_DEVICE_END };
@@ -127,6 +135,11 @@ make_bios_writable(void) make_bios_writable_intel(bdf, I440FX_PAM0); return; } + if (vendor == PCI_VENDOR_ID_INTEL + && device == PCI_DEVICE_ID_INTEL_Q35_MCH) { + make_bios_writable_intel(bdf, Q35_HOST_BRIDGE_PAM0); + return; + } } dprintf(1, "Unable to unlock ram - bridge not found\n"); } diff --git a/src/smm.c b/src/smm.c index d0d1476..7977ac7 100644 --- a/src/smm.c +++ b/src/smm.c @@ -11,6 +11,7 @@ #include "ioport.h" // outb #include "pci_ids.h" // PCI_VENDOR_ID_INTEL #include "xen.h" // usingXen +#include "dev-q35.h"
ASM32FLAT( ".global smm_relocation_start\n" @@ -137,9 +138,45 @@ static void piix4_apmc_smm_init(struct pci_device *pci, void *arg) pci_config_writeb(i440_pci->bdf, I440FX_SMRAM, 0x02 | 0x08); }
+/* PCI_VENDOR_ID_INTEL && PCI_DEVICE_ID_INTEL_ICH9_LPC */ +void ich9_lpc_apmc_smm_init(struct pci_device *dev, void *arg) +{ + struct pci_device *mch_dev; + int mch_bdf; + + // This code is hardcoded for Q35 Power Management device. + mch_dev = pci_find_device(PCI_VENDOR_ID_INTEL, + PCI_DEVICE_ID_INTEL_Q35_MCH); + mch_bdf = mch_dev->bdf; + + if (mch_bdf < 0) + return; + + /* check if SMM init is already done */ + u32 value = inl(PORT_ACPI_PM_BASE + ICH9_PMIO_SMI_EN); + if (value & ICH9_PMIO_SMI_EN_APMC_EN) + return; + + /* enable the SMM memory window */ + pci_config_writeb(mch_bdf, Q35_HOST_BRIDGE_SMRAM, 0x02 | 0x48); + + smm_save_and_copy(); + + /* enable SMI generation when writing to the APMC register */ + outl(value | ICH9_PMIO_SMI_EN_APMC_EN, + PORT_ACPI_PM_BASE + ICH9_PMIO_SMI_EN); + + smm_relocate_and_restore(); + + /* close the SMM memory window and enable normal SMM */ + pci_config_writeb(mch_bdf, Q35_HOST_BRIDGE_SMRAM, 0x02 | 0x08); +} + static const struct pci_device_id smm_init_tbl[] = { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3, piix4_apmc_smm_init), + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH9_LPC, + ich9_lpc_apmc_smm_init),
PCI_DEVICE_END, };
On Tue, Nov 20, 2012 at 05:55:50PM +0100, Gerd Hoffmann wrote:
From: Isaku Yamahata yamahata@valinux.co.jp
add q35 initialization functions.
Thanks - looks okay to me. However, I did find it odd that pci_bios_init_devices() had 3 separate pci device tables - I sent a separate patch to the list about that. However, that patch does impact your patch.
-Kevin
On 11/21/12 02:58, Kevin O'Connor wrote:
On Tue, Nov 20, 2012 at 05:55:50PM +0100, Gerd Hoffmann wrote:
From: Isaku Yamahata yamahata@valinux.co.jp
add q35 initialization functions.
Thanks - looks okay to me. However, I did find it odd that pci_bios_init_devices() had 3 separate pci device tables - I sent a separate patch to the list about that.
I assumed there are initialization order issues, but never actually checked. Looking at your patch the separate tables seem to be pretty pointless indeed.
However, that patch does impact your patch.
No problem, I'll go rebase once the patches are in master.
cheers, Gerd
On Wed, Nov 21, 2012 at 08:34:17AM +0100, Gerd Hoffmann wrote:
On 11/21/12 02:58, Kevin O'Connor wrote:
On Tue, Nov 20, 2012 at 05:55:50PM +0100, Gerd Hoffmann wrote:
From: Isaku Yamahata yamahata@valinux.co.jp
add q35 initialization functions.
Thanks - looks okay to me. However, I did find it odd that pci_bios_init_devices() had 3 separate pci device tables - I sent a separate patch to the list about that.
I assumed there are initialization order issues, but never actually checked. Looking at your patch the separate tables seem to be pretty pointless indeed.
However, that patch does impact your patch.
No problem, I'll go rebase once the patches are in master.
Okay - I pushed my patch to master.
-Kevin
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/acpi.c | 22 ++++++++++++++++++++++ src/acpi.h | 17 +++++++++++++++++ src/pci.h | 1 + src/pciinit.c | 9 +++++++++ 4 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/src/acpi.c b/src/acpi.c index 72c8fe8..456f8f7 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -764,6 +764,27 @@ build_srat(void) return srat; }
+static void * +build_mcfg(void) +{ + struct acpi_table_mcfg *mcfg; + + if (!mmconfig.address) + return NULL; + + int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]); + mcfg = malloc_high(len); + if (!mcfg) { + warn_noalloc(); + return NULL; + } + memset(mcfg, 0, len); + mcfg->allocation[0] = mmconfig; + + build_header((void *)mcfg, MCFG_SIGNATURE, len, 1); + return mcfg; +} + static const struct pci_device_id acpi_find_tbl[] = { /* PIIX4 Power Management device. */ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371AB_3, NULL), @@ -804,6 +825,7 @@ acpi_bios_init(void) ACPI_INIT_TABLE(build_madt()); ACPI_INIT_TABLE(build_hpet()); ACPI_INIT_TABLE(build_srat()); + ACPI_INIT_TABLE(build_mcfg());
u16 i, external_tables = qemu_cfg_acpi_additional_tables();
diff --git a/src/acpi.h b/src/acpi.h index cb21561..715d19d 100644 --- a/src/acpi.h +++ b/src/acpi.h @@ -107,4 +107,21 @@ struct bfld { u64 p1l; /* pci window 1 (above 4g) - length */ } PACKED;
+/* PCI fw r3.0 MCFG table. */ +/* Subtable */ +struct acpi_mcfg_allocation { + u64 address; /* Base address, processor-relative */ + u16 pci_segment; /* PCI segment group number */ + u8 start_bus_number; /* Starting PCI Bus number */ + u8 end_bus_number; /* Final PCI Bus number */ + u32 reserved; +} PACKED; + +#define MCFG_SIGNATURE 0x4746434d // MCFG +struct acpi_table_mcfg { + ACPI_TABLE_HEADER_DEF; + u8 reserved[8]; + struct acpi_mcfg_allocation allocation[0]; +} PACKED; + #endif // acpi.h diff --git a/src/pci.h b/src/pci.h index 104638d..04f79f2 100644 --- a/src/pci.h +++ b/src/pci.h @@ -59,6 +59,7 @@ struct pci_device { extern u64 mtrr_base; extern u64 pcimem_start, pcimem_end; extern u64 pcimem64_start, pcimem64_end; +extern struct acpi_mcfg_allocation mmconfig; extern struct pci_device *PCIDevices; extern int MaxPCIBus; int pci_probe_host(void); diff --git a/src/pciinit.c b/src/pciinit.c index 091d0e6..3bdeeca 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -13,6 +13,7 @@ #include "config.h" // CONFIG_* #include "xen.h" // usingXen #include "memmap.h" // add_e820 +#include "acpi.h" // acpi_table_mcfg #include "dev-q35.h"
#define PCI_DEVICE_MEM_MIN 0x1000 @@ -39,6 +40,8 @@ u64 pcimem_end = BUILD_PCIMEM_END; u64 pcimem64_start = BUILD_PCIMEM64_START; u64 pcimem64_end = BUILD_PCIMEM64_END;
+struct acpi_mcfg_allocation mmconfig; + struct pci_region_entry { struct pci_device *dev; int bar; @@ -687,6 +690,12 @@ void mch_mem_addr_init(struct pci_device *dev, void *arg) pci_config_writel(bdf, Q35_HOST_BRIDGE_PCIEXBAR, lower); add_e820(addr, size, E820_RESERVED);
+ /* for build_mcfg */ + mmconfig.address = addr; + mmconfig.pci_segment = Q35_HOST_PCIE_PCI_SEGMENT; + mmconfig.start_bus_number = Q35_HOST_PCIE_START_BUS_NUMBER; + mmconfig.end_bus_number = Q35_HOST_PCIE_END_BUS_NUMBER; + /* setup pci i/o window (above mmconfig) */ pcimem_start = addr + size; mtrr_base = addr + size;
On Tue, Nov 20, 2012 at 05:55:51PM +0100, Gerd Hoffmann wrote: [...]
+static void * +build_mcfg(void) +{
- struct acpi_table_mcfg *mcfg;
- if (!mmconfig.address)
return NULL;- int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]);
- mcfg = malloc_high(len);
- if (!mcfg) {
warn_noalloc();return NULL;- }
- memset(mcfg, 0, len);
- mcfg->allocation[0] = mmconfig;
Given all the funky things SeaBIOS does with 16/32bit mode string copies, I'd prefer to use explicit memcpy() calls instead of using implicit struct copies (though I agree it's not likely to be an issue in this particular call site).
That said, though, I don't see a gain in having the acpi code communicate with the pciinit code. Both sets of code are effectively qemu specific, build_mcfg() is only going to get called on q35, and thus the mmconfig struct will always have hard coded values. Might as well have build_mcfg detect q35 and just put the hard-coded values in directly. Let me know if I've missed something.
-Kevin
Hi,
That said, though, I don't see a gain in having the acpi code communicate with the pciinit code. Both sets of code are effectively qemu specific, build_mcfg() is only going to get called on q35, and thus the mmconfig struct will always have hard coded values. Might as well have build_mcfg detect q35 and just put the hard-coded values in directly. Let me know if I've missed something.
/me was thinking about making the 32bit pci window dynamic on q35 too, so the mmconfig base address wouldn't be fixed any more. Didn't do it in the end because the window is larger on q35 anyway (qemu wouldn't map ram above 0xb0000000). So, yes, we can just hard-code stuff.
cheers, Gerd
On Tue, Nov 20, 2012 at 05:55:49PM +0100, Gerd Hoffmann wrote:
Hi,
The q35 C code bits for review. The confusing mcfg acpi table code is largely rewritten, otherwise q35 initialization is almost unmodified.
DSDT isn't tackled yet.
Hi Gerd,
Thanks for posting these bits. I've sanity tested these bits along with the previous q35 'preparatory' bits, and with the q35 DSDT table you posted. Seems to work fine.
I'm still intending for the qemu bits to land in 1.3, so it would be great if the seabios bits were included as well (once they are ready). Thanks again for you help here!
-Jason
-
Gerd Hoffmann -
Jason Baron -
Kevin O'Connor