diff mbox series

[4/4] be less conservative with the 64bit pci io window

Message ID 20221121103213.1675568-5-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series misc tweaks for kvm and the 64bit pci window | expand

Commit Message

Gerd Hoffmann Nov. 21, 2022, 10:32 a.m. UTC 
Current seabios code will only enable and use the 64bit pci io window in
case it runs out of space in the 32bit pci mmio window below 4G.

This patch will also enable the 64bit pci io window when
  (a) RAM above 4G is present, and
  (b) the physical address space size is known, and
  (c) seabios is running on a 64bit capable processor.

This operates with the assumption that guests which are ok with memory
above 4G most likely can handle mmio above 4G too.

In case the 64bit pci io window is enabled also assign more memory to
prefetchable pci bridge windows (scale with address space).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 src/fw/pciinit.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Kevin O'Connor Nov. 22, 2022, 6:43 p.m. UTC | #1 
On Mon, Nov 21, 2022 at 11:32:13AM +0100, Gerd Hoffmann wrote:
> Current seabios code will only enable and use the 64bit pci io window in
> case it runs out of space in the 32bit pci mmio window below 4G.
> 
> This patch will also enable the 64bit pci io window when
>   (a) RAM above 4G is present, and
>   (b) the physical address space size is known, and
>   (c) seabios is running on a 64bit capable processor.
> 
> This operates with the assumption that guests which are ok with memory
> above 4G most likely can handle mmio above 4G too.

Thanks.  In general, the series looks good to me.  Can you elaborate
on the background to this change though?  It sounds like there is a
(small) risk of a regression, so I think it would be good to have a
high level understanding of what is driving this memory reorg.

Cheers,
-Kevin


> 
> In case the 64bit pci io window is enabled also assign more memory to
> prefetchable pci bridge windows (scale with address space).
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  src/fw/pciinit.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index ad6def93633b..3e9636b139a4 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -51,6 +51,7 @@ u64 pcimem_end     = BUILD_PCIMEM_END;
>  u64 pcimem64_start = BUILD_PCIMEM64_START;
>  u64 pcimem64_end   = BUILD_PCIMEM64_END;
>  u64 pci_io_low_end = 0xa000;
> +u32 pci_use_64bit  = 0;
>  
>  struct pci_region_entry {
>      struct pci_device *dev;
> @@ -920,6 +921,8 @@ static int pci_bios_check_devices(struct pci_bus *busses)
>          for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
>              u64 align = (type == PCI_REGION_TYPE_IO) ?
>                  PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
> +            if (pci_use_64bit && (type == PCI_REGION_TYPE_PREFMEM))
> +                align = (u64)1 << (PhysBits - 11);
>              if (!pci_bridge_has_region(s->bus_dev, type))
>                  continue;
>              u64 size = 0;
> @@ -1108,7 +1111,7 @@ static void pci_bios_map_devices(struct pci_bus *busses)
>          panic("PCI: out of I/O address space\n");
>  
>      dprintf(1, "PCI: 32: %016llx - %016llx\n", pcimem_start, pcimem_end);
> -    if (pci_bios_init_root_regions_mem(busses)) {
> +    if (pci_use_64bit || pci_bios_init_root_regions_mem(busses)) {
>          struct pci_region r64_mem, r64_pref;
>          r64_mem.list.first = NULL;
>          r64_pref.list.first = NULL;
> @@ -1174,6 +1177,9 @@ pci_setup(void)
>  
>      dprintf(3, "pci setup\n");
>  
> +    if (PhysBits >= 36 && LongMode && RamSizeOver4G)
> +        pci_use_64bit = 1;
> +
>      dprintf(1, "=== PCI bus & bridge init ===\n");
>      if (pci_probe_host() != 0) {
>          return;
> -- 
> 2.38.1
> 
>
Gerd Hoffmann Nov. 23, 2022, 10:25 a.m. UTC | #2 
On Tue, Nov 22, 2022 at 01:43:16PM -0500, Kevin O'Connor wrote:
> On Mon, Nov 21, 2022 at 11:32:13AM +0100, Gerd Hoffmann wrote:
> > Current seabios code will only enable and use the 64bit pci io window in
> > case it runs out of space in the 32bit pci mmio window below 4G.
> > 
> > This patch will also enable the 64bit pci io window when
> >   (a) RAM above 4G is present, and
> >   (b) the physical address space size is known, and
> >   (c) seabios is running on a 64bit capable processor.
> > 
> > This operates with the assumption that guests which are ok with memory
> > above 4G most likely can handle mmio above 4G too.
> 
> Thanks.  In general, the series looks good to me.  Can you elaborate
> on the background to this change though?  It sounds like there is a
> (small) risk of a regression, so I think it would be good to have a
> high level understanding of what is driving this memory reorg.

Well, the idea is to adapt to the world moving forward.  Running a
64-bit capable OS is standard these days, and the resources needed
by devices (especially GPUs) are becoming larger and larger.

Yes, there is the risk that (old) guests are unhappy with their
PCI bars suddenly being mapped above 4G.  Can happen only in case
seabios handles pci initialization (i.e. when running on qemu,
otherwise coreboot initializes the pci bars).  I hope the memory
check handles the 'old guest' case: when the guest can't handle
addresses above 4G it is unlikely that qemu is configured to have
memory mapped above 4G ...

take care,
  Gerd
Igor Mammedov Dec. 8, 2022, 4:01 p.m. UTC | #3 
On Wed, 23 Nov 2022 11:25:08 +0100
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Tue, Nov 22, 2022 at 01:43:16PM -0500, Kevin O'Connor wrote:
> > On Mon, Nov 21, 2022 at 11:32:13AM +0100, Gerd Hoffmann wrote:  
> > > Current seabios code will only enable and use the 64bit pci io window in
> > > case it runs out of space in the 32bit pci mmio window below 4G.
> > > 
> > > This patch will also enable the 64bit pci io window when
> > >   (a) RAM above 4G is present, and
> > >   (b) the physical address space size is known, and
> > >   (c) seabios is running on a 64bit capable processor.
> > > 
> > > This operates with the assumption that guests which are ok with memory
> > > above 4G most likely can handle mmio above 4G too.  
> > 
> > Thanks.  In general, the series looks good to me.  Can you elaborate
> > on the background to this change though?  It sounds like there is a
> > (small) risk of a regression, so I think it would be good to have a
> > high level understanding of what is driving this memory reorg.  
> 
> Well, the idea is to adapt to the world moving forward.  Running a
> 64-bit capable OS is standard these days, and the resources needed
> by devices (especially GPUs) are becoming larger and larger.
> 
> Yes, there is the risk that (old) guests are unhappy with their
> PCI bars suddenly being mapped above 4G.  Can happen only in case
> seabios handles pci initialization (i.e. when running on qemu,
> otherwise coreboot initializes the pci bars).  I hope the memory
> check handles the 'old guest' case: when the guest can't handle
> addresses above 4G it is unlikely that qemu is configured to have
> memory mapped above 4G ...

does it break 32-bit PAE enabled guests
(which can have more then 4Gb RAM configured)?

> 
> take care,
>   Gerd
> 
> _______________________________________________
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-leave@seabios.org
>
Gerd Hoffmann Dec. 9, 2022, 11:51 a.m. UTC | #4 
Hi,

> > Well, the idea is to adapt to the world moving forward.  Running a
> > 64-bit capable OS is standard these days, and the resources needed
> > by devices (especially GPUs) are becoming larger and larger.
> > 
> > Yes, there is the risk that (old) guests are unhappy with their
> > PCI bars suddenly being mapped above 4G.  Can happen only in case
> > seabios handles pci initialization (i.e. when running on qemu,
> > otherwise coreboot initializes the pci bars).  I hope the memory
> > check handles the 'old guest' case: when the guest can't handle
> > addresses above 4G it is unlikely that qemu is configured to have
> > memory mapped above 4G ...
> 
> does it break 32-bit PAE enabled guests
> (which can have more then 4Gb RAM configured)?

Well, depends on the guest OS I guess.  Modern linux copes just fine,
either uses PAE paging to access the PCI bars (seen with debian, works
even in case the PCI bars are above the 64G limit of 32bit processors),
or or it remaps the bars to places below 4G (seen with alpine which
compiles i386 kernels with PAE=n).

take care,
  Gerd
diff mbox series

Patch

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index ad6def93633b..3e9636b139a4 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -51,6 +51,7 @@  u64 pcimem_end     = BUILD_PCIMEM_END;
 u64 pcimem64_start = BUILD_PCIMEM64_START;
 u64 pcimem64_end   = BUILD_PCIMEM64_END;
 u64 pci_io_low_end = 0xa000;
+u32 pci_use_64bit  = 0;
 
 struct pci_region_entry {
     struct pci_device *dev;
@@ -920,6 +921,8 @@  static int pci_bios_check_devices(struct pci_bus *busses)
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
             u64 align = (type == PCI_REGION_TYPE_IO) ?
                 PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
+            if (pci_use_64bit && (type == PCI_REGION_TYPE_PREFMEM))
+                align = (u64)1 << (PhysBits - 11);
             if (!pci_bridge_has_region(s->bus_dev, type))
                 continue;
             u64 size = 0;
@@ -1108,7 +1111,7 @@  static void pci_bios_map_devices(struct pci_bus *busses)
         panic("PCI: out of I/O address space\n");
 
     dprintf(1, "PCI: 32: %016llx - %016llx\n", pcimem_start, pcimem_end);
-    if (pci_bios_init_root_regions_mem(busses)) {
+    if (pci_use_64bit || pci_bios_init_root_regions_mem(busses)) {
         struct pci_region r64_mem, r64_pref;
         r64_mem.list.first = NULL;
         r64_pref.list.first = NULL;
@@ -1174,6 +1177,9 @@  pci_setup(void)
 
     dprintf(3, "pci setup\n");
 
+    if (PhysBits >= 36 && LongMode && RamSizeOver4G)
+        pci_use_64bit = 1;
+
     dprintf(1, "=== PCI bus & bridge init ===\n");
     if (pci_probe_host() != 0) {
         return;