[PATCH] pciinit: use the 32-bit MMIO address space preferred

[Hulang]

Currently, pci region on busses[0] may be migrate to
64-bit mmio space. this will cause a mistake to read/write
device config space

example:
A modern virtio device map to 64-bit mmio space will set
mode to VP_ACCESS_PCICFG, But the real device cap is
VIRTIO_PCI_CAP_COMMON_CFG, we can not access the cap
rightly.

If this is a virtio blk/scsi device as system disk, VM
will not be booted on the device.

A simple solution is make device use the 32-bit address
space as much as possible.

This patch changes the placement of the PCI bars.

[1] commit 0e21548b15e2 ("virtio: pci cfg access")

Signed-off-by: hulang 
---
src/fw/pciinit.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index b3e359d7..2e9544dc 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -1100,8 +1100,12 @@ static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r)
{
     struct hlist_node *n;
     struct pci_region_entry *entry;
+    u64 r_end = r->base + pci_region_sum(r)
+
     hlist_for_each_entry_safe(entry, n, &r->list, node) {
         u64 addr = r->base;
+        if (addr + entry->size >= r_end)
+            continue;
         r->base += entry->size;
         if (entry->bar == -1)
             // Update bus base address if entry is a bridge region
@@ -1114,6 +1118,8 @@ static void pci_region_map_entries(struct pci_bus *busses, struct pci_region *r)
 static void pci_bios_map_devices(struct pci_bus *busses)
{
+    int bus;
+
     if (pci_bios_init_root_regions_io(busses))
         panic("PCI: out of I/O address space\n");
@@ -1122,6 +1128,15 @@ static void pci_bios_map_devices(struct pci_bus *busses)
         struct pci_region r64_mem, r64_pref;
         r64_mem.list.first = NULL;
         r64_pref.list.first = NULL;
+
+        // try map pci region to 32bit
+        for (bus = 0; bus<=MaxPCIBus; bus++) {
+            int type;
+            for (type = 0; type < PCI_REGION_TYPE_COUNT; type++)
+                pci_region_map_entries(busses, &busses[bus].r[type]);
+        }
+
+        // map remaining pci region to 64bit
         pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],
                                          &r64_mem);
         pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_PREFMEM],
@@ -1166,7 +1181,6 @@ static void pci_bios_map_devices(struct pci_bus *busses)
         pcimem64_start = 0;
     }
     // Map regions on each device.
-    int bus;
     for (bus = 0; bus<=MaxPCIBus; bus++) {
         int type;
         for (type = 0; type < PCI_REGION_TYPE_COUNT; type++)
--
2.33.0

Best Regards！
胡 浪 Hu Lang
华为云计算公司 德科 软件开发工程师
Espace：h30028282
Mobile：[phone]
E-mail： [e-mail]
www.fescoadecco.comhttp://www.fescoadecco.com/
办公地址：杭州市滨江区江淑路华为研究所Z6-4-B06R-087S

[Gerd Hoffmann]

On Thu, Dec 26, 2024 at 08:04:55AM +0000, Hulang via SeaBIOS wrote:
> Currently, pci region on busses[0] may be migrate to
> 64-bit mmio space. this will cause a mistake to read/write
> device config space
>
> example:
> A modern virtio device map to 64-bit mmio space will set
> mode to VP_ACCESS_PCICFG, But the real device cap is
> VIRTIO_PCI_CAP_COMMON_CFG, we can not access the cap
> rightly.

the mmio bars (including virtio config space) of virtio-pci devices
can be accessed via pci config space.  See VIRTIO_PCI_CAP_PCI_CFG
documentation in the virtio spec.  So seabios can drive these devices
even if they are mapped above 4G.

What is the host device implementation?  Probably not qemu?
> A simple solution is make device use the 32-bit address
> space as much as possible.

An easier way to do that is to skip the devices in question
in pci_region_migrate_64bit_entries().  There already is one
line for usb host adapters which makes sure xhci mmio bars
are mapped below 4G.  But as explained above this should not
be needed for virtio-pci devices.

take care,
  Gerd