On Monday, October 12, 2015, Michael S. Tsirkin <m...@redhat.com> 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 <mar...@redhat.com <javascript:;>>
> > ---
> >
> > 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
>
>
