Hi,
On Tue, 1 Dec 2015 18:36:39 +0200 Marcel Apfelbaum <mar...@redhat.com> wrote:
> > + if (pci_is_express(pci_dev) && pci_bus_is_express(pci_dev->bus) &&
> > + !pci_bus_is_root(pci_dev->bus)) {
> > int pos;
>
> Here you should check only for 'pci_is_express(pci_dev)' .
[snip]
> > +static void virtio_pci_dc_realize(DeviceState *qdev, Error **errp)
> > +{
> > + VirtioPCIClass *vpciklass = VIRTIO_PCI_GET_CLASS(qdev);
> > + VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev);
> > + PCIDevice *pci_dev = &proxy->pci_dev;
> > +
> > + if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) &&
> > + !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN)) {
> > + pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>
> And here you should also check:
> pci_bus_is_express(pci_dev->bus) && !pci_bus_is_root(pci_dev->bus))
>
> The reason is the device becomes express only if *all* the conditions
> are met.
I'm ok with either approaches.
However it seems common practice to set QEMU_PCI_CAP_EXPRESS
unconditionally for PCIE devices.
The few existing PCIE devices do so by assigning their
PCIDeviceClass.is_express to 1 within their 'class_init', regardless the
properties of the bus their on.
(e.g. xhci_class_init, megasas_class_init, vfio_pci_dev_class_init,
nvme_class_init, and more)
Some devices later call pcie_endpoint_cap_init conditionally.
(e.g. usb_xhci_realize).
Can you please examine this and let me know the preferred approach?
> > + DeviceRealize saved_dc_realize;
>
> I would change the name to parent_realize :)
Sure.
