Re: [Qemu-devel] [PATCH qemu] vfio/spapr: Allow fallback to SPAPR TCE IOMMU v1

[Alexey Kardashevskiy]

On 30/11/17 02:26, Alex Williamson wrote:
> On Wed, 22 Nov 2017 16:15:52 +1100
> Alexey Kardashevskiy <address@hidden> wrote:
> 
>> The vfio_iommu_spapr_tce driver always advertises v1 and v2 IOMMU support,
>> however PR KVM (a special version of KVM designed to work in
>> a paravirtualized system; these days used for nested virtualizaion) only
>> supports the "pseries" platform which does not support v2. Since there is
>> no way to choose the IOMMU version in QEMU, it fails to start.
> 
> Seems like the bug is then in advertising v2 support when it doesn't
> exist. 


The way to distinguish v1 IOMMUs from v2 IOMMU in the powerpc code is: get
an IOMMU group ops pointer and see if DMA window callbacks are defined. So
at least in theory it is still possible to have both types of IOMMUs and
VFIO_CHECK_EXTENSION is called before any group is attached to a container.

Well, I could walk through all IOMMU groups in the system (possibly
hundreds), work out the maximum common set of IOMMU types and only
advertise those. Doing fallback to v1 seems nicer, no?



> Isn't that a kernel bug?  Otherwise I think this is a long
> standing bug, since QEMU-2.7 and probably not 2.11 material this late
> into the freeze.  A "Fixes" tag would probably also be appropriate,
> identifying the commit where this was introduced (318f67ce1371) if it
> is a QEMU bug.  Thanks,
> 
> Alex
> 
>> This adds a fallback to the v1 IOMMU if v2 cannot be used.
>>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>> ---
>>  hw/vfio/common.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 7b2924c..cd81cc9 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -1040,6 +1040,11 @@ static int vfio_connect_container(VFIOGroup *group, 
>> AddressSpace *as,
>>              v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
>>          ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>>          if (ret) {
>> +            container->iommu_type = VFIO_SPAPR_TCE_IOMMU;
>> +            v2 = false;
>> +            ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>> +        }
>> +        if (ret) {
>>              error_setg_errno(errp, errno, "failed to set iommu for 
>> container");
>>              ret = -errno;
>>              goto free_container_exit;
> 


-- 
Alexey