Re: [Xen-devel] [PATCH] AMD/IOMMU: correct handling when XT's prereq features are unavailable

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.02.2020 16:25, Andrew Cooper wrote:
> On 27/02/2020 14:34, Jan Beulich wrote:
>> --- a/xen/drivers/passthrough/amd/iommu_init.c
>> +++ b/xen/drivers/passthrough/amd/iommu_init.c
>> @@ -1364,6 +1364,7 @@ static int __init amd_iommu_prepare_one(
>>  int __init amd_iommu_prepare(bool xt)
>>  {
>>      struct amd_iommu *iommu;
>> +    bool no_xt = false;
> 
> I think the logic would be easier to follow if this was has_xt, with
> inverted meaning.

I'm not fully convinced it'll make it easier, but I've switched
things around.

>  However...
> 
>>      int rc = -ENODEV;
>>  
>>      BUG_ON( !iommu_found() );
>> @@ -1400,9 +1401,8 @@ int __init amd_iommu_prepare(bool xt)
>>          if ( rc )
>>              goto error_out;
>>  
>> -        rc = -ENODEV;
>> -        if ( xt && (!iommu->features.flds.ga_sup || 
>> !iommu->features.flds.xt_sup) )
>> -            goto error_out;
>> +        if ( !iommu->features.flds.ga_sup || !iommu->features.flds.xt_sup )
>> +            no_xt = true;
>>      }
>>  
>>      for_each_amd_iommu ( iommu )
> 
> ... the contents of this loop depends on the early exit path you've just
> deleted.
> 
> In the case of x2apic not being available, we'll still set {ga,xt}_en to
> the caller requested value.

Oh, indeed (and Roger, thank you too for noticing this). In fact
it explains a hang later during boot that I did observe, and that
I meant to look into later. That said, interrupts for Dom0 still
don't seem to work quite right in this (partly broken) mode even
with this fixed (there are several "No irq handler for vector"
messages, and at the very least the disk doesn't work); I'll see
to find time to look into that as well, but I'm pretty convinced
it's an independent issue. (Seeing that interrupt remapping gets
enabled this way, and x2APIC is pre-enabled, I'm suspecting this
is another case where we need to force physical mode.)

Let me blame this on the cold I'm fighting, which isn't quite bad
enough to stay home, but which also isn't helpful. v2 coming ...

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel