Re: [XEN PATCH v5 1/3] x86/iommu: Disable IOMMU if cx16 isn't supported
On 18.04.2024 13:57, Teddy Astie wrote:
> --- 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;
> +}
One additional nit: Here you neatly have just AMD-Vi: as a prefix for the
log message. Why ...
> @@ -2630,6 +2598,15 @@ static int __init cf_check vtd_setup(void)
> int ret;
> bool reg_inval_supported = true;
>
> +if ( unlikely(!cpu_has_cx16) )
> +{
> +printk(XENLOG_ERR VTDPREFIX
> + "IOMMU: CPU doesn't support CMPXCHG16B, disabling\n");
> +
> +ret = -ENODEV;
> +goto error;
> +}
... both VTDPREFIX and "IOMMU:" here?
Jan
Re: [XEN PATCH v5 1/3] x86/iommu: Disable IOMMU if cx16 isn't supported
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
