| Message ID | 1422016183-15968-1-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 23, 2015 at 01:29:43PM +0100, Gerd Hoffmann wrote: > Only for legacy-free virtio devices, to avoid unpleasent > surprises with old drivers. > > mtree snippet: > > 00000000fea00000-00000000fea7ffff (prio 1, RW): virtio-pci > 00000000fea00000-00000000fea00fff (prio 0, RW): virtio-pci-common > 00000000fea01000-00000000fea01fff (prio 0, RW): virtio-pci-isr > 00000000fea02000-00000000fea02fff (prio 0, RW): virtio-pci-device > 00000000fea03000-00000000fea42fff (prio 0, RW): virtio-pci-notify > 00000000fea7e000-00000000fea7e02f (prio 0, RW): msix-table > 00000000fea7f000-00000000fea7f007 (prio 0, RW): msix-pba > > Applies on top of mst's virtio-1.0 branch, plus my three virtio-pci > patches posted to the list yesterday. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Hmm what's the point here? I wanted to make the modern BAR prefetcheable, so it can be a full 64-bit one, this is impossible with the MSI-X BAR. This requires keeping it separate though. Again, if we start running of BARs we'll consider our options then, these things are easy to change. > --- > hw/virtio/virtio-pci.c | 39 +++++++++++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 12 deletions(-) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index ba1405f..934ca7a 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1235,7 +1235,7 @@ static void virtio_pci_device_plugged(DeviceState *d) > bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); > int modern_mem_bar; > uint8_t *config; > - uint32_t size; > + uint32_t modern_mem_size, legacy_io_size; > > config = proxy->pci_dev.config; > if (proxy->class_code) { > @@ -1339,9 +1339,10 @@ static void virtio_pci_device_plugged(DeviceState *d) > virtio_pci_add_mem_cap(proxy, ¬ify.cap); > > virtio_add_feature(&proxy->host_features, VIRTIO_F_VERSION_1); > + modern_mem_size = 2 * QEMU_VIRTIO_PCI_QUEUE_MEM_MULT * > + VIRTIO_PCI_QUEUE_MAX; > memory_region_init(&proxy->modern_bar, OBJECT(proxy), "virtio-pci", > - 2 * QEMU_VIRTIO_PCI_QUEUE_MEM_MULT * > - VIRTIO_PCI_QUEUE_MAX); > + modern_mem_size); > memory_region_init_io(&proxy->common, OBJECT(proxy), > &common_ops, > proxy, > @@ -1369,25 +1370,39 @@ static void virtio_pci_device_plugged(DeviceState *d) > &proxy->modern_bar); > } > > - if (proxy->nvectors && > - msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) { > - error_report("unable to init msix vectors to %" PRIu32, > - proxy->nvectors); > - proxy->nvectors = 0; > + if (proxy->nvectors) { > + if (modern && modern_mem_bar == 0) { > + if (msix_init(&proxy->pci_dev, proxy->nvectors, > + &proxy->modern_bar, modern_mem_bar, > + modern_mem_size - 0x2000, > + &proxy->modern_bar, modern_mem_bar, > + modern_mem_size - 0x1000, > + 0x90)) { > + error_report("unable to init msix vectors to %" PRIu32, > + proxy->nvectors); > + proxy->nvectors = 0; > + } > + } else { > + if (msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) { > + error_report("unable to init msix vectors to %" PRIu32, > + proxy->nvectors); > + proxy->nvectors = 0; > + } > + } > } > > proxy->pci_dev.config_write = virtio_write_config; > > if (legacy) { > - size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) > + legacy_io_size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) > + virtio_bus_get_vdev_config_len(bus); > - if (size & (size - 1)) { > - size = 1 << qemu_fls(size); > + if (legacy_io_size & (legacy_io_size - 1)) { > + legacy_io_size = 1 << qemu_fls(legacy_io_size); > } > > memory_region_init_io(&proxy->bar, OBJECT(proxy), > &virtio_pci_config_ops, > - proxy, "virtio-pci", size); > + proxy, "virtio-pci", legacy_io_size); > > pci_register_bar(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO, > &proxy->bar); > -- > 1.8.3.1
Hi, > I wanted to make the modern BAR prefetcheable, so it can > be a full 64-bit one, It's not the case right now though, the two bars have identical attributes, and there is plenty of unused space in the modern virtio bar ... > this is impossible with the MSI-X > BAR. What exactly is impossible? msi-x being in a 64bit bar? msi-x being in a prefetchable bar? Also why the bar needs to be prefetchable to be 64bit bit? xhci looks this way right now: 00:0f.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 03) (prog-if 30 [XHCI]) Subsystem: Red Hat, Inc QEMU Virtual Machine Physical Slot: 15 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 10 Region 0: Memory at febd0000 (64-bit, non-prefetchable) [size=16K] Capabilities: [90] MSI-X: Enable+ Count=16 Masked- Vector table: BAR=0 offset=00003000 PBA: BAR=0 offset=00003800 Capabilities: [70] MSI: Enable- Count=1/16 Maskable- 64bit+ Address: 0000000000000000 Data: 0000 Kernel driver in use: xhci_hcd Which works just fine even with msi-x in a 64-bit (non-prefetchable) bar, but maybe just by pure luck? Also seabios wouldn't map the bar above 4G. cheers, Gerd
On Fri, Jan 23, 2015 at 02:53:27PM +0100, Gerd Hoffmann wrote: > Hi, > > > I wanted to make the modern BAR prefetcheable, so it can > > be a full 64-bit one, > > It's not the case right now though, the two bars have identical > attributes, and there is plenty of unused space in the modern virtio > bar ... It's just POC, I'll change it. > > this is impossible with the MSI-X > > BAR. > > What exactly is impossible? msi-x being in a 64bit bar? msi-x being in > a prefetchable bar? This. > Also why the bar needs to be prefetchable to be > 64bit bit? It doesn't but bridges only have 32 bit windows for non prefetcheable bars. So even if you try to make it 64 bit, only the low 32 bit are used. > xhci looks this way right now: > > 00:0f.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host > Controller (rev 03) (prog-if 30 [XHCI]) > Subsystem: Red Hat, Inc QEMU Virtual Machine > Physical Slot: 15 > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- > Stepping- SERR+ FastB2B- DisINTx+ > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- > <MAbort- >SERR- <PERR- INTx- > Latency: 0, Cache Line Size: 64 bytes > Interrupt: pin A routed to IRQ 10 > Region 0: Memory at febd0000 (64-bit, non-prefetchable) [size=16K] > Capabilities: [90] MSI-X: Enable+ Count=16 Masked- > Vector table: BAR=0 offset=00003000 > PBA: BAR=0 offset=00003800 > Capabilities: [70] MSI: Enable- Count=1/16 Maskable- 64bit+ > Address: 0000000000000000 Data: 0000 > Kernel driver in use: xhci_hcd > > Which works just fine even with msi-x in a 64-bit (non-prefetchable) > bar, but maybe just by pure luck? xhci is integrated in a root complex so it can work there. > Also seabios wouldn't map the bar > above 4G. > > cheers, > Gerd We can teach it to do that, and even if not, OS can do it on hotplug.
Hi, > > Also seabios wouldn't map the bar > > above 4G. > We can teach it to do that, and even if not, OS can do it on hotplug. Just to clarify: seabios can do it in the general case, it just wouldn't do it for xhci specifically because otherwise the seabios xhci driver wouldn't be able to reach the registers any more ... For non-transitional virtio-{blk,scsi,net} devices seabios (and ipxe) would have a simliar problem btw: When mapping the bars above 4G it can't handle the devices any more. But we'll probably stick to transitional by default for a while (where seabios can just use the legacy registers in io address space), and there also is ovmf coming which runs in 64bit mode so mapping the bars high isn't a problem there. cheers, Gerd
On Fri, Jan 23, 2015 at 03:48:28PM +0100, Gerd Hoffmann wrote: > Hi, > > > > Also seabios wouldn't map the bar > > > above 4G. > > > We can teach it to do that, and even if not, OS can do it on hotplug. > > Just to clarify: seabios can do it in the general case, it just wouldn't > do it for xhci specifically because otherwise the seabios xhci driver > wouldn't be able to reach the registers any more ... > > For non-transitional virtio-{blk,scsi,net} devices seabios (and ipxe) > would have a simliar problem btw: When mapping the bars above 4G it > can't handle the devices any more. You can always use the config cap for this. I didn't implement it yet, but I will. > But we'll probably stick to transitional by default for a while (where > seabios can just use the legacy registers in io address space), io *is* very crowded. I think we should try and code up modern drivers when we have the time. > and > there also is ovmf coming which runs in 64bit mode so mapping the bars > high isn't a problem there. > > cheers, > Gerd >
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ba1405f..934ca7a 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1235,7 +1235,7 @@ static void virtio_pci_device_plugged(DeviceState *d) bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); int modern_mem_bar; uint8_t *config; - uint32_t size; + uint32_t modern_mem_size, legacy_io_size; config = proxy->pci_dev.config; if (proxy->class_code) { @@ -1339,9 +1339,10 @@ static void virtio_pci_device_plugged(DeviceState *d) virtio_pci_add_mem_cap(proxy, ¬ify.cap); virtio_add_feature(&proxy->host_features, VIRTIO_F_VERSION_1); + modern_mem_size = 2 * QEMU_VIRTIO_PCI_QUEUE_MEM_MULT * + VIRTIO_PCI_QUEUE_MAX; memory_region_init(&proxy->modern_bar, OBJECT(proxy), "virtio-pci", - 2 * QEMU_VIRTIO_PCI_QUEUE_MEM_MULT * - VIRTIO_PCI_QUEUE_MAX); + modern_mem_size); memory_region_init_io(&proxy->common, OBJECT(proxy), &common_ops, proxy, @@ -1369,25 +1370,39 @@ static void virtio_pci_device_plugged(DeviceState *d) &proxy->modern_bar); } - if (proxy->nvectors && - msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) { - error_report("unable to init msix vectors to %" PRIu32, - proxy->nvectors); - proxy->nvectors = 0; + if (proxy->nvectors) { + if (modern && modern_mem_bar == 0) { + if (msix_init(&proxy->pci_dev, proxy->nvectors, + &proxy->modern_bar, modern_mem_bar, + modern_mem_size - 0x2000, + &proxy->modern_bar, modern_mem_bar, + modern_mem_size - 0x1000, + 0x90)) { + error_report("unable to init msix vectors to %" PRIu32, + proxy->nvectors); + proxy->nvectors = 0; + } + } else { + if (msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) { + error_report("unable to init msix vectors to %" PRIu32, + proxy->nvectors); + proxy->nvectors = 0; + } + } } proxy->pci_dev.config_write = virtio_write_config; if (legacy) { - size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) + legacy_io_size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) + virtio_bus_get_vdev_config_len(bus); - if (size & (size - 1)) { - size = 1 << qemu_fls(size); + if (legacy_io_size & (legacy_io_size - 1)) { + legacy_io_size = 1 << qemu_fls(legacy_io_size); } memory_region_init_io(&proxy->bar, OBJECT(proxy), &virtio_pci_config_ops, - proxy, "virtio-pci", size); + proxy, "virtio-pci", legacy_io_size); pci_register_bar(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &proxy->bar);
Only for legacy-free virtio devices, to avoid unpleasent surprises with old drivers. mtree snippet: 00000000fea00000-00000000fea7ffff (prio 1, RW): virtio-pci 00000000fea00000-00000000fea00fff (prio 0, RW): virtio-pci-common 00000000fea01000-00000000fea01fff (prio 0, RW): virtio-pci-isr 00000000fea02000-00000000fea02fff (prio 0, RW): virtio-pci-device 00000000fea03000-00000000fea42fff (prio 0, RW): virtio-pci-notify 00000000fea7e000-00000000fea7e02f (prio 0, RW): msix-table 00000000fea7f000-00000000fea7f007 (prio 0, RW): msix-pba Applies on top of mst's virtio-1.0 branch, plus my three virtio-pci patches posted to the list yesterday. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/virtio/virtio-pci.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-)