[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capabi
From: |
Shmulik Ladkani |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method |
Date: |
Wed, 2 Dec 2015 10:01:09 +0200 |
Thanks Marcel,
On Tue, 1 Dec 2015 22:46:33 +0200, address@hidden wrote:
> >> 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?
>
> Yes, I saw that..., as always not a walk in the park.
>
> - So we have "is_express = true" <=> QEMU_PCI_CAP_EXPRESS on <=> "config size
> = PCIe"
> - Not related to the above (!!), if (some condition) => add PCIe express
> capability
> (megasas is the exception)
>
> Let's take "usb_xhci":
> - If we put it under a PCI bus it will not be an express device, but
> it will have a "big" config space. Also pci_is_express(dev) will still
> return true!
> - This is probably a bug. (or I am missing something)
I actually assumed this is the right behavior.
A device class reports whether its instances *could* be pcie by arming
its PCIDeviceClass.is_express.
As such, the "big" config space is allocated for the instance. This is
harmless.
Such a device may (or may not) be connected to a pcie bus, and only if
so, we report it is a pcie endpoint.
Also, pcie_add_capability is allowed on that device, in order to setup
whatever capabilities on its pcie config space (even if finally not on a
pcie bus).
Moreover, VMSTATE_PCIE_DEVICE (which uses vmstate_pcie_device rather
than vmstate_pci_device) can be used for that device's
VMStateDescription fields *without* worrying whether the actual config
space is "big" or "small".
Otherwise one should examine whether vmstate_pcie_device or
vmstate_pci_device need to be used. Seems tedious.
This is the reasoning I can think of, why assigning QEMU_PCI_CAP_EXPRESS
and reporting pcie_endpoint_cap_init are not tightly coupled.
Indeed, no strict solution here, both approaches seem reasoanble (and
both are used!).
WDYT? Is my above interpretation makes sense?
Regards,
Shmulik
- [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Shmulik Ladkani, 2015/12/01
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Marcel Apfelbaum, 2015/12/01
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Shmulik Ladkani, 2015/12/01
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Marcel Apfelbaum, 2015/12/01
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method,
Shmulik Ladkani <=
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Marcel Apfelbaum, 2015/12/02
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Shmulik Ladkani, 2015/12/02
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Marcel Apfelbaum, 2015/12/02
- Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method, Shmulik Ladkani, 2015/12/02