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?
Jan
