[PATCH 0/2] add guest hint for memory layout, take two
Gerd Hoffmann (2): update pci_pad_mem64 handling add fwcfg override for pci_pad_mem64 src/util.h | 1 + src/fw/paravirt.c | 13 +++++++++++++ src/fw/pciinit.c | 11 ++++++++--- 3 files changed, 22 insertions(+), 3 deletions(-) -- 2.47.0
Add a new possible state: '-1' means 'use default'. In that case seabios continue to use the current heuristic: In case memory above 4G is present enable 64-bit guest friendly configuration. This allows forcing the one or the other behavior by setting the pci_pad_mem64 variable beforehand (which is done by another patch). Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/fw/pciinit.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index b3e359d7fe62..df63acc19e24 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -55,7 +55,7 @@ u64 pcimem64_end = BUILD_PCIMEM64_END; // Resource allocation limits static u64 pci_io_low_end = 0xa000; static u64 pci_mem64_top = 0; -static u32 pci_pad_mem64 = 0; +static u32 pci_pad_mem64 = -1; struct pci_region_entry { struct pci_device *dev; @@ -1202,8 +1202,13 @@ pci_setup(void) } } - if (CPUPhysBits >= 36 && CPULongMode && RamSizeOver4G) - pci_pad_mem64 = 1; + if (pci_pad_mem64 == -1) + // when not set (via fw_cfg) enable in case memory over 4G is present + pci_pad_mem64 = RamSizeOver4G ? 1 : 0; + + if (!CPULongMode) + // force off for 32-bit CPUs + pci_pad_mem64 = 0; dprintf(1, "=== PCI bus & bridge init ===\n"); if (pci_probe_host() != 0) { -- 2.47.0
Allow setting pci_pad_mem64 via fw_cfg. Usage: qemu -fw_cfg name=opt/org.seabios/pci64,string={yes,no} Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/util.h | 1 + src/fw/paravirt.c | 13 +++++++++++++ src/fw/pciinit.c | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/util.h b/src/util.h index aff8e888955e..03dcafb83794 100644 --- a/src/util.h +++ b/src/util.h @@ -135,6 +135,7 @@ void multiboot_init(void); extern u64 pcimem_start, pcimem_end; extern u64 pcimem64_start, pcimem64_end; extern const u8 pci_irqs[4]; +extern u32 pci_pad_mem64; void pci_setup(void); void pci_resume(void); diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index e5d4eca0cb5a..6428a70361c4 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -718,6 +718,19 @@ void qemu_cfg_init(void) dprintf(1, "Moving pm_base to 0x%x\n", acpi_pm_base); } + // 64-bit pci mmio window + char *hint = romfile_loadfile("opt/org.seabios/pci64", NULL); + if (hint) { + if (0 == strcmp(hint, "0") || + 0 == strcmp(hint, "no") || + 0 == strcmp(hint, "off")) + pci_pad_mem64 = 0; + if (0 == strcmp(hint, "1") || + 0 == strcmp(hint, "yes") || + 0 == strcmp(hint, "on")) + pci_pad_mem64 = 1; + } + // serial console u16 nogfx = 0; qemu_cfg_read_entry(&nogfx, QEMU_CFG_NOGRAPHIC, sizeof(nogfx)); diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index df63acc19e24..5b4cf3bd5598 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -55,7 +55,7 @@ u64 pcimem64_end = BUILD_PCIMEM64_END; // Resource allocation limits static u64 pci_io_low_end = 0xa000; static u64 pci_mem64_top = 0; -static u32 pci_pad_mem64 = -1; +u32 pci_pad_mem64 = -1; struct pci_region_entry { struct pci_device *dev; -- 2.47.0
On Thu, Nov 21, 2024 at 05:07:50PM +0100, Gerd Hoffmann wrote:
Allow setting pci_pad_mem64 via fw_cfg. Usage: qemu -fw_cfg name=opt/org.seabios/pci64,string={yes,no}
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/util.h | 1 + src/fw/paravirt.c | 13 +++++++++++++ src/fw/pciinit.c | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/util.h b/src/util.h index aff8e888955e..03dcafb83794 100644 --- a/src/util.h +++ b/src/util.h @@ -135,6 +135,7 @@ void multiboot_init(void); extern u64 pcimem_start, pcimem_end; extern u64 pcimem64_start, pcimem64_end; extern const u8 pci_irqs[4]; +extern u32 pci_pad_mem64; void pci_setup(void); void pci_resume(void);
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index e5d4eca0cb5a..6428a70361c4 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -718,6 +718,19 @@ void qemu_cfg_init(void) dprintf(1, "Moving pm_base to 0x%x\n", acpi_pm_base); }
+ // 64-bit pci mmio window + char *hint = romfile_loadfile("opt/org.seabios/pci64", NULL); + if (hint) { + if (0 == strcmp(hint, "0") || + 0 == strcmp(hint, "no") || + 0 == strcmp(hint, "off")) + pci_pad_mem64 = 0; + if (0 == strcmp(hint, "1") || + 0 == strcmp(hint, "yes") || + 0 == strcmp(hint, "on")) + pci_pad_mem64 = 1; + }
Thanks. In general it seems fine to me. For what it is worth, the "filename" and format seems a little odd to me. We've not used the "opt/org.seabios/" scheme prior. We can adopt that scheme going forward I guess. The "pci64" seems very generic to me - if I understand the capability, we're padding space between 64bit memory regions for busses, not enabling 64bit pci. Alas, I'm not sure I have a good suggestion for an alternative. If we want to support human readable values (eg, "on") perhaps it would be good to add a romfile_readbool() helper to romfile.c (or similar). Also, any reason to put this in paravirt.c instead of directly in pciinit.c? Thanks again, -Kevin
For what it is worth, the "filename" and format seems a little odd to me. We've not used the "opt/org.seabios/" scheme prior. We can adopt that scheme going forward I guess.
In qemu places fw_cfg files for all firmwares in 'etc/'. fw_cfg files which do not come from qemu but are just passed through because someone adds them to the qemu command line should go into 'opt/<reverse-dns>/<name>'. So edk2 uses 'opt/org.tianocore/...' for most options (there are some in 'opt/ovmf/...' for historical reasons). For seabios-specific options 'opt/org.seabios/...' would be the correct choice.
The "pci64" seems very generic to me - if I understand the capability, we're padding space between 64bit memory regions for busses, not enabling 64bit pci.
It does both.
If we want to support human readable values (eg, "on") perhaps it would be good to add a romfile_readbool() helper to romfile.c (or similar).
Makes sense indeed.
Also, any reason to put this in paravirt.c instead of directly in pciinit.c?
Looked reasonable to me because all fw_cfg access is there, but placing it directly in pciinit.c should work too. take care, Gerd
participants (2)
-
Gerd Hoffmann -
Kevin O'Connor