Hi Eric, On 12/09/17 18:13, Auger Eric wrote: > 2.6.7 > - As I am currently integrating v0.4 in QEMU here are some other comments: > At the moment struct virtio_iommu_req_probe flags is missing in your > header. As such I understood the ACK protocol was not implemented by the > driver in your branch. Uh indeed. And yet I could swear I've written that code... somewhere. I will add it to the batch of v0.5 changes, it shouldn't be too invasive. > - VIRTIO_IOMMU_PROBE_PROPERTY_TYPE_MASK is VIRTIO_IOMMU_T_MASK in your > header too. Yes, keeping VIRTIO_IOMMU_PROBE_PROPERTY_TYPE_MASK is probably best (though it is a mouthful). > 2.6.8.2: > - I am really confused about what the device should report as resv > regions depending on the PE nature (VFIO or not VFIO) > > In other iommu drivers, the resv regions are populated by the iommu > driver through its get_resv_regions callback. They are usually composed > of an iommu specific MSI region (mapped or bypassed) and non IOMMU > specific (device specific) reserved regions: > iommu_dma_get_resv_regions(). In the case of virtio-iommu driver, those > are the guest reserved regions. > > First in the current virtio-iommu driver I don't see the > iommu_dma_get_resv_regions call. Do you agree that the virtio-iommu > driver should compute the non IOMMU specific MSI regions. ie. this is > not the responsability of the virtio-iommu device. For SW_MSI, certainly. The driver allocates a fixed IOVA region for mapping the MSI doorbell. But the driver has to know whether the doorbell region is translated or bypassed. > Then why is it more the job of the device to return the guest iommu > specific region rather than the driver itself? The MSI region is architectural on x86 IOMMUs, but implementation-defined on virtio-iommu. It depends which platform the host is emulating. In Linux, x86 IOMMU drivers register the bypass region because there always is an IOAPIC on the other end, with a fixed MSI address. But virtio-iommu may be either behind a GIC, an APIC or some other IRQ chip. The driver *could* go over all the irqchips/platforms it knows and try to guess if there is a fixed doorbell or if it needs to reserve an IOVA for them, but it would look horrible. I much prefer having a well-defined way of doing this, so a description from the device. > Then I understand this is the responsability of the virtio-iommu device > to gather information about the host resv regions in case of VFIO EP. > Typically the host PCIe host bridge windows cannot be used for IOVA. > Also the host MSI reserved IOVA window cannot be used. Do you agree. Yes, all regions reported in sysfs reserved_regions in the host would be reported as RESV_T_RESERVED by virtio-iommu. > I really think the spec should clarify what exact resv regions the > device should return in case of VFIO device and non VFIO device. Agreed. I will add something about RESV_T_RESERVED with the PCI bridge example in Implementation Notes. Do you think the MSI examples at the end need improvement as well? I can try to explain that RESV_MSI regions in virtio-iommu are only those of the emulated platform, not the HW or SW MSI regions from the host. Thanks, Jean
