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

[Michael S. Tsirkin]

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.

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

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


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