Re: virtio-iommu issue with VFIO device downstream to a PCIe-to-PCI bridge: VFIO devices are not assigned any iommu group

[Eric Auger <eauger@redhat.com>]

Hi,

On 1/18/23 19:28, Alex Williamson wrote:
> On Wed, 18 Jan 2023 18:03:13 +0000
> Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> 
>> On Fri, Jan 13, 2023 at 10:57:00AM -0700, Alex Williamson wrote:
>>> On Fri, 13 Jan 2023 12:39:18 +0000
>>> Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
>>>   
>>>> Hi,
>>>>
>>>> On Mon, Jan 09, 2023 at 10:11:19PM +0100, Eric Auger wrote:  
>>>>>> Jean, do you have any idea about how to fix that? Do you think we have a
>>>>>> trouble in the acpi/viot setup or virtio-iommu probe sequence. It looks
>>>>>> like virtio probe and attach commands are called too early, before the
>>>>>> bus is actually correctly numbered.    
>>>>>
>>>>> So after further investigations looks this is not a problem of bus
>>>>> number, which is good at the time of the virtio cmd calls but rather a
>>>>> problem related to the devfn (0 was used when creating the IOMMU MR)
>>>>> whereas the virtio-iommu cmds looks for the non aliased devfn. With that
>>>>> fixed, the probe and attach at least succeeds. The device still does not
>>>>> work for me but I will continue my investigations and send a tentative fix.    
>>>>
>>>> If I remember correctly VIOT can deal with bus numbers because bridges are
>>>> assigned a range by QEMU, but I haven't tested that in detail, and I don't
>>>> know how it holds with conventional PCI bridges.  
>>>
>>> In my reading of the virtio-iommu spec,  
>>
>> Hm, is that the virtio-iommu spec or ACPI VIOT/device tree spec?
>> The virtio-iommu spec shouldn't refer to PCI buses at the moment. The
>> intent is that for PCI, the "endpoint ID" passed in an ATTACH request
>> corresponds to PCI segment and RID of PCI devices at the time of the
>> request (so after the OS renumbered the buses). If you found something in
>> the spec that contradicts this, it should be fixed. Note that "endpoint"
>> is a misnomer, it can refer to PCI bridges as well, anything that can
>> issue DMA transactions.
> 
> Sorry, the ACPI spec defining the VIOT table[1]:
> 
> 	Each node identifies one or more devices using either their PCI
> 	Handle or their base MMIO (Memory-Mapped I/O) address. A PCI
> 	Handle is a PCI Segment number and a BDF (Bus-Device-Function)
> 	with the following layout:
> 
> 	* Bits 15:8 Bus Number
> 
> 	* Bits 7:3 Device Number
> 
> 	* Bits 2:0 Function Number
> 
> 	This identifier corresponds to the one observed by the
> 	operating system when parsing the PCI configuration space for
> 	the first time after boot.
> 
>>> I noted that it specifies the
>>> bus numbers *at the time of OS handoff*, so it essentially washes its
>>> hands of the OS renumbering buses while leaving subtle dependencies on
>>> initial numbering in the guest and QEMU implementations.  
>>
>> Yes we needed to describe in the firmware tables (device-tree and ACPI
>> VIOT) which devices the IOMMU manages. And at the time we generate the
>> tables, if we want to refer to PCI devices behind bridges, we can either
>> use catch-all ranges for any possible bus numbers they will get, or
>> initialize bus numbers in bridges and pass those to the OS.
>>
>> But that's only to communicate the IOMMU topology to the OS, because we
>> couldn't come up with anything better. After it sets up PCI the OS should
>> be able to use its own configuration of the PCI topology in virtio-iommu
>> requests.
> 
> The VT-d spec[2](8.3.1) has a more elegant solution using a path
> described in a device scope, based on a root bus number (not
> susceptible to OS renumbering) and a sequence of devfns to uniquely
> describe a hierarchy or endpoint, invariant of OS bus renumbering.
> Thanks,

Independently on the potential issue raised by Alex about later bus
renumbering, I observe that the VIOT content, in my case, is correct and
properly advertises the translation of the RIDs of all my devices. So
the iommu group topology issue I have on guest is not due to the VIOT
ACPI table content.

Eric
> 
> Alex
> 
> [1]https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#virtual-i-o-translation-viot-table-header
> [2]https://cdrdv2-public.intel.com/671081/vt-directed-io-spec.pdf
>