qemu-devel
[Top][All Lists]
Advanced

[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: Marcel Apfelbaum
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 16:00:41 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

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

On Wed, 2 Dec 2015 11:51:46 +0200, address@hidden 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

[Prev in Thread] Current Thread [Next in Thread]