Re: [PATCH v2 6/7] libqos: make the virtio-pci BAR index configurable

[Thomas Huth]

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 
> ---
>  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 

Re: [PATCH v2 6/7] libqos: make the virtio-pci BAR index configurable

[Sergio Lopez]

Stefan Hajnoczi  writes:

> On Fri, Oct 11, 2019 at 02:06:01PM +0200, Sergio Lopez wrote:
>> 
>> Stefan Hajnoczi  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 
>> > ---
>> >  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 = _pci_legacy;
>> >  dev->msix_ops = _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 

> Stefan



signature.asc
Description: PGP signature

Re: [PATCH v2 6/7] libqos: make the virtio-pci BAR index configurable

[Stefan Hajnoczi]

On Fri, Oct 11, 2019 at 02:06:01PM +0200, Sergio Lopez wrote:
> 
> Stefan Hajnoczi  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 
> > ---
> >  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 = _pci_legacy;
> >  dev->msix_ops = _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


signature.asc
Description: PGP signature

Re: [PATCH v2 6/7] libqos: make the virtio-pci BAR index configurable

[Sergio Lopez]

Stefan Hajnoczi  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 
> ---
>  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 = _pci_legacy;
>  dev->msix_ops = _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.


signature.asc
Description: PGP signature