| Message ID | 20191011085611.4194-7-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libqos: add VIRTIO PCI 1.0 support | expand |
Stefan Hajnoczi <stefanha@redhat.com> writes: > The Legacy virtio-pci interface always uses BAR 0. VIRTIO 1.0 may need > to use a different BAR index, so make it configurable. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > tests/libqos/virtio-pci.h | 2 ++ > tests/libqos/virtio-pci.c | 3 ++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h > index b620c30451..f2d53aa377 100644 > --- a/tests/libqos/virtio-pci.h > +++ b/tests/libqos/virtio-pci.h > @@ -25,6 +25,8 @@ typedef struct QVirtioPCIDevice { > uint16_t config_msix_entry; > uint64_t config_msix_addr; > uint32_t config_msix_data; > + > + uint8_t bar_idx; > } QVirtioPCIDevice; > > struct QVirtioPCIMSIXOps { > diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > index 3fb4af4016..efd8caee18 100644 > --- a/tests/libqos/virtio-pci.c > +++ b/tests/libqos/virtio-pci.c > @@ -300,7 +300,7 @@ static const QVirtioPCIMSIXOps qvirtio_pci_msix_ops_legacy = { > void qvirtio_pci_device_enable(QVirtioPCIDevice *d) > { > qpci_device_enable(d->pdev); > - d->bar = qpci_iomap(d->pdev, 0, NULL); > + d->bar = qpci_iomap(d->pdev, d->bar_idx, NULL); > } > > void qvirtio_pci_device_disable(QVirtioPCIDevice *d) > @@ -389,6 +389,7 @@ void qvirtio_pci_start_hw(QOSGraphObject *obj) > static void qvirtio_pci_init_legacy(QVirtioPCIDevice *dev) > { > dev->vdev.device_type = qpci_config_readw(dev->pdev, PCI_SUBSYSTEM_ID); > + dev->bar_idx = 0; > dev->vdev.bus = &qvirtio_pci_legacy; > dev->msix_ops = &qvirtio_pci_msix_ops_legacy; > dev->vdev.big_endian = qtest_big_endian(dev->pdev->bus->qts); qpci_iomap() is also called with a hardcoded bar at virtio-blk-test.c:test_nonexistent_virtqueue(). Should it be fixed here or in a future patch? Sergio.
On Fri, Oct 11, 2019 at 02:06:01PM +0200, Sergio Lopez wrote: > > Stefan Hajnoczi <stefanha@redhat.com> writes: > > > The Legacy virtio-pci interface always uses BAR 0. VIRTIO 1.0 may need > > to use a different BAR index, so make it configurable. > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > tests/libqos/virtio-pci.h | 2 ++ > > tests/libqos/virtio-pci.c | 3 ++- > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h > > index b620c30451..f2d53aa377 100644 > > --- a/tests/libqos/virtio-pci.h > > +++ b/tests/libqos/virtio-pci.h > > @@ -25,6 +25,8 @@ typedef struct QVirtioPCIDevice { > > uint16_t config_msix_entry; > > uint64_t config_msix_addr; > > uint32_t config_msix_data; > > + > > + uint8_t bar_idx; > > } QVirtioPCIDevice; > > > > struct QVirtioPCIMSIXOps { > > diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > > index 3fb4af4016..efd8caee18 100644 > > --- a/tests/libqos/virtio-pci.c > > +++ b/tests/libqos/virtio-pci.c > > @@ -300,7 +300,7 @@ static const QVirtioPCIMSIXOps qvirtio_pci_msix_ops_legacy = { > > void qvirtio_pci_device_enable(QVirtioPCIDevice *d) > > { > > qpci_device_enable(d->pdev); > > - d->bar = qpci_iomap(d->pdev, 0, NULL); > > + d->bar = qpci_iomap(d->pdev, d->bar_idx, NULL); > > } > > > > void qvirtio_pci_device_disable(QVirtioPCIDevice *d) > > @@ -389,6 +389,7 @@ void qvirtio_pci_start_hw(QOSGraphObject *obj) > > static void qvirtio_pci_init_legacy(QVirtioPCIDevice *dev) > > { > > dev->vdev.device_type = qpci_config_readw(dev->pdev, PCI_SUBSYSTEM_ID); > > + dev->bar_idx = 0; > > dev->vdev.bus = &qvirtio_pci_legacy; > > dev->msix_ops = &qvirtio_pci_msix_ops_legacy; > > dev->vdev.big_endian = qtest_big_endian(dev->pdev->bus->qts); > > qpci_iomap() is also called with a hardcoded bar at > virtio-blk-test.c:test_nonexistent_virtqueue(). Should it be fixed here > or in a future patch? That virtio-blk-pci-specific continues to work because the virtio-blk-pci device is a Transitional device that still supports Legacy mode in BAR 0. Also, that test doesn't use the libqos virtio API so there is no conflict between it and this patch series. Stefan
Stefan Hajnoczi <stefanha@redhat.com> writes: > On Fri, Oct 11, 2019 at 02:06:01PM +0200, Sergio Lopez wrote: >> >> Stefan Hajnoczi <stefanha@redhat.com> writes: >> >> > The Legacy virtio-pci interface always uses BAR 0. VIRTIO 1.0 may need >> > to use a different BAR index, so make it configurable. >> > >> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >> > --- >> > tests/libqos/virtio-pci.h | 2 ++ >> > tests/libqos/virtio-pci.c | 3 ++- >> > 2 files changed, 4 insertions(+), 1 deletion(-) >> > >> > diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h >> > index b620c30451..f2d53aa377 100644 >> > --- a/tests/libqos/virtio-pci.h >> > +++ b/tests/libqos/virtio-pci.h >> > @@ -25,6 +25,8 @@ typedef struct QVirtioPCIDevice { >> > uint16_t config_msix_entry; >> > uint64_t config_msix_addr; >> > uint32_t config_msix_data; >> > + >> > + uint8_t bar_idx; >> > } QVirtioPCIDevice; >> > >> > struct QVirtioPCIMSIXOps { >> > diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c >> > index 3fb4af4016..efd8caee18 100644 >> > --- a/tests/libqos/virtio-pci.c >> > +++ b/tests/libqos/virtio-pci.c >> > @@ -300,7 +300,7 @@ static const QVirtioPCIMSIXOps qvirtio_pci_msix_ops_legacy = { >> > void qvirtio_pci_device_enable(QVirtioPCIDevice *d) >> > { >> > qpci_device_enable(d->pdev); >> > - d->bar = qpci_iomap(d->pdev, 0, NULL); >> > + d->bar = qpci_iomap(d->pdev, d->bar_idx, NULL); >> > } >> > >> > void qvirtio_pci_device_disable(QVirtioPCIDevice *d) >> > @@ -389,6 +389,7 @@ void qvirtio_pci_start_hw(QOSGraphObject *obj) >> > static void qvirtio_pci_init_legacy(QVirtioPCIDevice *dev) >> > { >> > dev->vdev.device_type = qpci_config_readw(dev->pdev, PCI_SUBSYSTEM_ID); >> > + dev->bar_idx = 0; >> > dev->vdev.bus = &qvirtio_pci_legacy; >> > dev->msix_ops = &qvirtio_pci_msix_ops_legacy; >> > dev->vdev.big_endian = qtest_big_endian(dev->pdev->bus->qts); >> >> qpci_iomap() is also called with a hardcoded bar at >> virtio-blk-test.c:test_nonexistent_virtqueue(). Should it be fixed here >> or in a future patch? > > That virtio-blk-pci-specific continues to work because the > virtio-blk-pci device is a Transitional device that still supports > Legacy mode in BAR 0. > > Also, that test doesn't use the libqos virtio API so there is no > conflict between it and this patch series. OK, in that case: Reviewed-by: Sergio Lopez <slp@redhat.com> > Stefan
On 11/10/2019 10.56, Stefan Hajnoczi wrote: > The Legacy virtio-pci interface always uses BAR 0. VIRTIO 1.0 may need > to use a different BAR index, so make it configurable. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > tests/libqos/virtio-pci.h | 2 ++ > tests/libqos/virtio-pci.c | 3 ++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h > index b620c30451..f2d53aa377 100644 > --- a/tests/libqos/virtio-pci.h > +++ b/tests/libqos/virtio-pci.h > @@ -25,6 +25,8 @@ typedef struct QVirtioPCIDevice { > uint16_t config_msix_entry; > uint64_t config_msix_addr; > uint32_t config_msix_data; > + > + uint8_t bar_idx; I think I'd rather make that an "int" instead of "uint8_t" ... but that's just my personal taste, so anyway: Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h index b620c30451..f2d53aa377 100644 --- a/tests/libqos/virtio-pci.h +++ b/tests/libqos/virtio-pci.h @@ -25,6 +25,8 @@ typedef struct QVirtioPCIDevice { uint16_t config_msix_entry; uint64_t config_msix_addr; uint32_t config_msix_data; + + uint8_t bar_idx; } QVirtioPCIDevice; struct QVirtioPCIMSIXOps { diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c index 3fb4af4016..efd8caee18 100644 --- a/tests/libqos/virtio-pci.c +++ b/tests/libqos/virtio-pci.c @@ -300,7 +300,7 @@ static const QVirtioPCIMSIXOps qvirtio_pci_msix_ops_legacy = { void qvirtio_pci_device_enable(QVirtioPCIDevice *d) { qpci_device_enable(d->pdev); - d->bar = qpci_iomap(d->pdev, 0, NULL); + d->bar = qpci_iomap(d->pdev, d->bar_idx, NULL); } void qvirtio_pci_device_disable(QVirtioPCIDevice *d) @@ -389,6 +389,7 @@ void qvirtio_pci_start_hw(QOSGraphObject *obj) static void qvirtio_pci_init_legacy(QVirtioPCIDevice *dev) { dev->vdev.device_type = qpci_config_readw(dev->pdev, PCI_SUBSYSTEM_ID); + dev->bar_idx = 0; dev->vdev.bus = &qvirtio_pci_legacy; dev->msix_ops = &qvirtio_pci_msix_ops_legacy; dev->vdev.big_endian = qtest_big_endian(dev->pdev->bus->qts);
The Legacy virtio-pci interface always uses BAR 0. VIRTIO 1.0 may need to use a different BAR index, so make it configurable. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/libqos/virtio-pci.h | 2 ++ tests/libqos/virtio-pci.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-)