Re: [XEN PATCH v5 1/3] x86/iommu: Disable IOMMU if cx16 isn't supported

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.01.2025 12:09, Roger Pau Monné wrote:
> On Wed, Apr 24, 2024 at 04:27:10PM +0200, Jan Beulich wrote:
>> On 18.04.2024 13:57, Teddy Astie wrote:
>>> All hardware with VT-d/AMD-Vi has CMPXCHG16B support. Check this at
>>> initialisation time, and remove the effectively-dead logic for the
>>> non-cx16 case.
>>
>> As before: What about Xen itself running virtualized, and the underlying
>> hypervisor surfacing an IOMMU but not CX16? It may be okay to ignore the
>> IOMMU in such an event, but by not mentioning the case you give the
>> appearance of not having considered it at all.
>>
>>> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
>>> @@ -305,6 +305,12 @@ static int __init cf_check iov_detect(void)
>>>      if ( !iommu_enable && !iommu_intremap )
>>>          return 0;
>>>  
>>> +    if ( unlikely(!cpu_has_cx16) )
>>> +    {
>>> +        printk("AMD-Vi: CPU doesn't support CMPXCHG16B, disabling\n");
>>> +        return -ENODEV;
>>> +    }
>>> +
>>>      if ( (init_done ? amd_iommu_init_late()
>>>                      : amd_iommu_init(false)) != 0 )
>>>      {
>>
>> I did previously point out (and that's even visible in patch context here)
>> that the per-vendor .setup() hooks aren't necessarily the first thing to
>> run. Please can you make sure you address (verbally or by code) prior to
>> submitting new versions?
> 
> I've re-visiting this as part of my other IOMMU IRTE atomic update
> fix.
> 
> Would you prefer the check for CX16 be in the common x86
> iommu_hardware_setup()?  That would be kind of layering violation, as
> in principle a further IOMMU implementation on x86 might not require
> CX16.  However I find it very unlikely, and hence I would be fine in
> placing the check in iommu_hardware_setup() if you prefer it there.

No. The check needs replicating into the other hook that may run first,
->supports_x2apic(). And the ->enable_x2apic() hooks may want to gain
respective assertions then.

Jan