Re: [Qemu-devel] [PATCH RFC] hw/virtio: Add PCIe capability to virtio devices

[Marcel Apfelbaum]

On Monday, October 12, 2015, Michael S. Tsirkin <address@hidden> wrote:
> On Mon, Oct 12, 2015 at 06:17:54PM +0300, Marcel Apfelbaum wrote:
> > The virtio devices are converted to PCI-Express
> > if they are plugged into a PCI-Express bus and
> > the 'modern' protocol is enabled.
> >
> > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > ---
> >
> > This is an RFC because all it does it adds the PCIe capability and nothing more.
>
> Express capability is easy.
> But if you go over express space you will see that a bunch of
> other capabilities are required, such as PM capability etc.
> These might need more work.

Sure, I'll look on the PCIe spec for the minimum requirements.

 

> > I post it early so I can get feedbacks on what is the best way to continue it.
> >
> > Any comments would be appreciated,
> > Thanks,
> > Marcel
> >
> >  hw/virtio/virtio-pci.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 6703806..f7c93d9 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -1591,6 +1591,17 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
> >
> >      address_space_init(&proxy->modern_as, &proxy->modern_cfg, "virtio-pci-cfg-as");
> >
> > +    if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN)
> > +        && pci_bus_is_express(pci_dev->bus)) {
>
> One point: we probably want to avoid doing this for integrated
> devices on root bus. Does pci_bus_is_express return true there?

Hmm, I'll check, but I think it does. Is this a must? 

 

> > +        int pos = pci_add_capability(pci_dev, PCI_CAP_ID_EXP, 0,
> > +                                     PCI_EXP_VER2_SIZEOF);
> > +
> > +        if (pos > 0) {
>
> We probably want to assert on pos < 0 instead.
> That implies a code bug.

I was wondering what to do here, thanks for the idea!

Thanks,

Marcel

 

> > +            pci_dev->exp.exp_cap = pos;
> > +            pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> > +        }
> > +    }
> > +
> >      virtio_pci_bus_new(&proxy->bus, sizeof(proxy->bus), proxy);
> >      if (k->realize) {
> >          k->realize(proxy, errp);
> > --
> > 2.1.0