On 12/02/2015 03:30 PM, Shmulik Ladkani wrote:
Hi,

On Wed, 2 Dec 2015 11:51:46 +0200, mar...@redhat.com wrote:
2. We still have pci_is_express returning true, this is error prone because
one can use this function assuming the device is express. Maybe we should
call it "can_be_express" ?

If you think this is good enough, you can simply do the same:
    - Instead of replacing the realize method, just advertise it with
      "is_express" (meaning it can be express)
    - Leave all the conditions as they were in prev patch.
As a result, the pci config space will have the right length.

Oh but we can't do so, as the change of config space size is guest
visible and breaks migration; it must depend on your x-pcie-disable
flag :)

Indeed, we need at least to condition it on  x-pcie-disable.


As I can't decide what's better, I'm following your initial suggestion
and submit for maintainers to review.

Sure, and thanks for the patience to get to the bottom of it.


However, do note that there are few more evidence that 'pci_is_express'
is true while not necessarily placed on a pcie bus:

and this is scary ... we really should call it "pci_can_be_express"

- pcie_endpoint_cap_init:
   it tests for 'pci_bus_is_express' although 'dev' is guaranteed to be
   'pci_is_express' (assertion in pcie_cap_init)
- 058fdcf 'xhci: add endpoint cap on express bus only'


Thanks,
Marcel


Thanks,
Shmulik



Reply via email to