Hi,
I'm sure I had sent this before, but appearently totally forgot about it. Just tested booting with virtio bars mapped above 4G, didn't work. Investigated, found this bitroting in a local branch. Undusted it, rebased to latest master with some fixups, and here it is ...
Gerd Hoffmann (2): virtio: uninline _vp_{read,write} virtio: pci cfg access
src/hw/virtio-pci.c | 254 ++++++++++++++++++++++++++++++++++++++++++++++++---- src/hw/virtio-pci.h | 94 ++++--------------- 2 files changed, 252 insertions(+), 96 deletions(-)
Next patch makes it larger, and I don't think it makes sense to continue inlining it. Uninline and move from header to c file.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/hw/virtio-pci.h | 80 ++--------------------------------------------------- 2 files changed, 81 insertions(+), 78 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index ce15672..f37f71d 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -25,6 +25,85 @@ #include "virtio-pci.h" #include "virtio-ring.h"
+u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) +{ + u64 var; + + if (cap->is_io) { + u32 addr = cap->ioaddr + offset; + switch (size) { + case 8: + var = inl(addr); + var |= (u64)inl(addr+4) << 32; + break; + case 4: + var = inl(addr); + break; + case 2: + var = inw(addr); + break; + case 1: + var = inb(addr); + break; + default: + var = 0; + } + } else { + void *addr = cap->memaddr + offset; + switch (size) { + case 8: + var = readl(addr); + var |= (u64)readl(addr+4) << 32; + break; + case 4: + var = readl(addr); + break; + case 2: + var = readw(addr); + break; + case 1: + var = readb(addr); + break; + default: + var = 0; + } + } + dprintf(9, "vp read %x (%d) -> 0x%llx\n", cap->ioaddr + offset, size, var); + return var; +} + +void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) +{ + dprintf(9, "vp write %x (%d) <- 0x%llx\n", cap->ioaddr + offset, size, var); + if (cap->is_io) { + u32 addr = cap->ioaddr + offset; + switch (size) { + case 4: + outl(var, addr); + break; + case 2: + outw(var, addr); + break; + case 1: + outb(var, addr); + break; + } + } else { + void *addr = cap->memaddr + offset; + switch (size) { + case 4: + writel(addr, var); + break; + case 2: + writew(addr, var); + break; + case 1: + writeb(addr, var); + break; + } + } +} + u64 vp_get_features(struct vp_device *vp) { u32 f0, f1; diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index ff8454e..70b40f7 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -101,84 +101,8 @@ struct vp_device { u8 use_modern; };
-static inline u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) -{ - u64 var; - - if (cap->is_io) { - u32 addr = cap->ioaddr + offset; - switch (size) { - case 8: - var = inl(addr); - var |= (u64)inl(addr+4) << 32; - break; - case 4: - var = inl(addr); - break; - case 2: - var = inw(addr); - break; - case 1: - var = inb(addr); - break; - default: - var = 0; - } - } else { - void *addr = cap->memaddr + offset; - switch (size) { - case 8: - var = readl(addr); - var |= (u64)readl(addr+4) << 32; - break; - case 4: - var = readl(addr); - break; - case 2: - var = readw(addr); - break; - case 1: - var = readb(addr); - break; - default: - var = 0; - } - } - dprintf(9, "vp read %x (%d) -> 0x%llx\n", cap->ioaddr + offset, size, var); - return var; -} - -static inline void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) -{ - dprintf(9, "vp write %x (%d) <- 0x%llx\n", cap->ioaddr + offset, size, var); - if (cap->is_io) { - u32 addr = cap->ioaddr + offset; - switch (size) { - case 4: - outl(var, addr); - break; - case 2: - outw(var, addr); - break; - case 1: - outb(var, addr); - break; - } - } else { - void *addr = cap->memaddr + offset; - switch (size) { - case 4: - writel(addr, var); - break; - case 2: - writew(addr, var); - break; - case 1: - writeb(addr, var); - break; - } - } -} +u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size); +void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var);
#define vp_read(_cap, _struct, _field) \ _vp_read(_cap, offsetof(_struct, _field), \
virtio regions can also be accessed using a window in pci cfg space. Add support for it. Enable it in case the virtio regions are mapped high (above 4g), so direct mmio access doesn't work for us even in 32bit mode.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 193 +++++++++++++++++++++++++++++++++++++++++++++------- src/hw/virtio-pci.h | 14 +++- 2 files changed, 180 insertions(+), 27 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index f37f71d..8efc826 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -27,9 +27,11 @@
u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) { - u64 var; + u64 var = 0;
- if (cap->is_io) { + switch (cap->mode) { + case VP_ACCESS_IO: + { u32 addr = cap->ioaddr + offset; switch (size) { case 8: @@ -45,10 +47,12 @@ u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) case 1: var = inb(addr); break; - default: - var = 0; } - } else { + break; + } + + case VP_ACCESS_MMIO: + { void *addr = cap->memaddr + offset; switch (size) { case 8: @@ -64,9 +68,46 @@ u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) case 1: var = readb(addr); break; - default: - var = 0; } + break; + } + + case VP_ACCESS_PCICFG: + { + u32 addr = cap->baroff + offset; + pci_config_writeb(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, cap.bar), + cap->bar); + pci_config_writel(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, cap.offset), + addr); + pci_config_writel(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, cap.length), + (size > 4) ? 4 : size); + switch (size) { + case 8: + var = pci_config_readl(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data)); + pci_config_writel(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, cap.offset), + addr + 4); + var |= (u64)pci_config_readl(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data)) << 32; + break; + case 4: + var = pci_config_readl(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data)); + break; + case 2: + var = pci_config_readw(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data)); + break; + case 1: + var = pci_config_readb(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data)); + break; + } + } } dprintf(9, "vp read %x (%d) -> 0x%llx\n", cap->ioaddr + offset, size, var); return var; @@ -75,7 +116,10 @@ u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) { dprintf(9, "vp write %x (%d) <- 0x%llx\n", cap->ioaddr + offset, size, var); - if (cap->is_io) { + + switch (cap->mode) { + case VP_ACCESS_IO: + { u32 addr = cap->ioaddr + offset; switch (size) { case 4: @@ -88,7 +132,11 @@ void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) outb(var, addr); break; } - } else { + break; + } + + case VP_ACCESS_MMIO: + { void *addr = cap->memaddr + offset; switch (size) { case 4: @@ -101,6 +149,39 @@ void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) writeb(addr, var); break; } + break; + } + + case VP_ACCESS_PCICFG: + { + u32 addr = cap->baroff + offset; + pci_config_writeb(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, cap.bar), + cap->bar); + pci_config_writel(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, cap.offset), + addr); + pci_config_writel(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, cap.length), + size); + switch (size) { + case 4: + pci_config_writel(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data), + var); + break; + case 2: + pci_config_writew(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data), + var); + break; + case 1: + pci_config_writeb(cap->bdf, cap->cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data), + var); + break; + } + } } }
@@ -181,10 +262,26 @@ void vp_notify(struct vp_device *vp, struct vring_virtqueue *vq) { if (vp->use_modern) { u32 offset = vq->queue_notify_off * vp->notify_off_multiplier; - if (vp->notify.is_io) { + switch (vp->notify.mode) { + case VP_ACCESS_IO: outw(vq->queue_index, vp->notify.ioaddr + offset); - } else { + break; + case VP_ACCESS_MMIO: writew(vp->notify.memaddr + offset, vq->queue_index); + break; + case VP_ACCESS_PCICFG: + pci_config_writeb(vp->notify.bdf, vp->notify.cfg + + offsetof(struct virtio_pci_cfg_cap, cap.bar), + vp->notify.bar); + pci_config_writel(vp->notify.bdf, vp->notify.cfg + + offsetof(struct virtio_pci_cfg_cap, cap.offset), + vp->notify.baroff + offset); + pci_config_writel(vp->notify.bdf, vp->notify.cfg + + offsetof(struct virtio_pci_cfg_cap, cap.length), + 2); + pci_config_writew(vp->notify.bdf, vp->notify.cfg + + offsetof(struct virtio_pci_cfg_cap, pci_cfg_data), + vq->queue_index); } dprintf(9, "vp notify %x (%d) -- 0x%x\n", vp->notify.ioaddr, 2, vq->queue_index); @@ -286,7 +383,9 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) { u8 cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, 0); struct vp_cap *vp_cap; - u32 offset, mul; + const char *mode; + u32 offset, base, mul; + u64 addr; u8 type;
memset(vp, 0, sizeof(*vp)); @@ -308,6 +407,20 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) case VIRTIO_PCI_CAP_DEVICE_CFG: vp_cap = &vp->device; break; + case VIRTIO_PCI_CAP_PCI_CFG: + vp->common.cfg = cap; + vp->common.bdf = pci->bdf; + vp->notify.cfg = cap; + vp->notify.bdf = pci->bdf; + vp->isr.cfg = cap; + vp->isr.bdf = pci->bdf; + vp->device.cfg = cap; + vp->device.bdf = pci->bdf; + vp_cap = NULL; + dprintf(1, "pci dev %x:%x virtio cap at 0x%x type %d [pci cfg access]\n", + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + cap, type); + break; default: vp_cap = NULL; break; @@ -318,24 +431,52 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) offsetof(struct virtio_pci_cap, bar)); offset = pci_config_readl(pci->bdf, cap + offsetof(struct virtio_pci_cap, offset)); - u32 bar = PCI_BASE_ADDRESS_0 + 4 * vp_cap->bar; - if (pci_config_readl(pci->bdf, bar) & PCI_BASE_ADDRESS_SPACE_IO) { - vp_cap->is_io = 1; - u32 addr = pci_enable_iobar(pci, bar); - if (!addr) - return; - vp_cap->ioaddr = addr + offset; + base = PCI_BASE_ADDRESS_0 + 4 * vp_cap->bar; + addr = pci_config_readl(pci->bdf, base); + if (addr & PCI_BASE_ADDRESS_SPACE_IO) { + addr &= PCI_BASE_ADDRESS_IO_MASK; + vp_cap->mode = VP_ACCESS_IO; + } else if ((addr & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == + PCI_BASE_ADDRESS_MEM_TYPE_64) { + addr &= PCI_BASE_ADDRESS_MEM_MASK; + addr |= (u64)pci_config_readl(pci->bdf, base + 4) << 32; + vp_cap->mode = (addr > 0xffffffffll) ? + VP_ACCESS_PCICFG : VP_ACCESS_MMIO; } else { - vp_cap->is_io = 0; - void *addr = pci_enable_membar(pci, bar); + addr &= PCI_BASE_ADDRESS_MEM_MASK; + vp_cap->mode = VP_ACCESS_MMIO; + } + switch (vp_cap->mode) { + case VP_ACCESS_IO: + { + u32 addr = pci_enable_iobar(pci, vp_cap->bar); + if (!addr) + return; + vp_cap->ioaddr = addr + offset; + mode = "io"; + break; + } + case VP_ACCESS_MMIO: + { + void *addr = pci_enable_membar(pci, vp_cap->bar); if (!addr) return; vp_cap->memaddr = addr + offset; + mode = "mmio"; + break; } - dprintf(3, "pci dev %pP virtio cap at 0x%x type %d " - "bar %d at 0x%08x off +0x%04x [%s]\n", - pci, vp_cap->cap, type, vp_cap->bar, vp_cap->ioaddr, offset, - vp_cap->is_io ? "io" : "mmio"); + case VP_ACCESS_PCICFG: + mode = "pcicfg"; + vp_cap->baroff = offset; + break; + default: + mode = "Huh?"; + break; + } + dprintf(1, "pci dev %x:%x virtio cap at 0x%x type %d " + "bar %d at 0x%08llx off +0x%04x [%s]\n", + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + vp_cap->cap, type, vp_cap->bar, addr, offset, mode); }
cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, cap); @@ -350,7 +491,7 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) vp->legacy.ioaddr = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0); if (!vp->legacy.ioaddr) return; - vp->legacy.is_io = 1; + vp->legacy.mode = VP_ACCESS_IO; }
vp_reset(vp); diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 70b40f7..492e5c7 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -54,6 +54,11 @@ struct virtio_pci_notify_cap { u32 notify_off_multiplier; /* Multiplier for queue_notify_off. */ };
+struct virtio_pci_cfg_cap { + struct virtio_pci_cap cap; + u8 pci_cfg_data[4]; /* Data for BAR access. */ +}; + typedef struct virtio_pci_common_cfg { /* About the whole device. */ u32 device_feature_select; /* read-write */ @@ -85,14 +90,21 @@ typedef struct virtio_pci_isr {
/* --- driver structs ----------------------------------------------- */
+#define VP_ACCESS_IO 1 +#define VP_ACCESS_MMIO 2 +#define VP_ACCESS_PCICFG 3 + struct vp_cap { union { void *memaddr; u32 ioaddr; + u32 baroff; }; + u16 bdf; u8 cap; + u8 cfg; u8 bar; - u8 is_io; + u8 mode; };
struct vp_device {
On Fri, Jun 17, 2016 at 01:09:05PM +0200, Gerd Hoffmann wrote:
Hi,
I'm sure I had sent this before, but appearently totally forgot about it. Just tested booting with virtio bars mapped above 4G, didn't work. Investigated, found this bitroting in a local branch. Undusted it, rebased to latest master with some fixups, and here it is ...
Looks good to me.
-Kevin
On Fr, 2016-06-17 at 11:31 -0400, Kevin O'Connor wrote:
On Fri, Jun 17, 2016 at 01:09:05PM +0200, Gerd Hoffmann wrote:
Hi,
I'm sure I had sent this before, but appearently totally forgot about it. Just tested booting with virtio bars mapped above 4G, didn't work. Investigated, found this bitroting in a local branch. Undusted it, rebased to latest master with some fixups, and here it is ...
Looks good to me.
Patches committed now.
cheers, Gerd
-
Gerd Hoffmann -
Kevin O'Connor