On 24.01.2025 13:01, Roger Pau Monne wrote:
> From: Teddy Astie <teddy.as...@vates.tech>
>
> 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.
Nit: There's nothing being removed here, so I expect the 2nd half of the
sentence wants dropping.
Jan
> If the local APICs support x2APIC mode the IOMMU support for interrupt
> remapping will be checked earlier using a specific helper. If no support
> for CX16 is detected by that earlier hook disable the IOMMU at that point
> and prevent further poking for CX16 later in the boot process, which would
> also fail.
>
> There's a possible corner case when running virtualized, and the underlying
> hypervisor exposing an IOMMU but no CMPXCHG16B support. In which case
> ignoring the IOMMU is fine, albeit the most natural would be for the
> underlying hypervisor to also expose CMPXCHG16B support if an IOMMU is
> available to the VM.
>
> Note this change only introduces the checks, but doesn't remove the now
> stale checks for CX16 support sprinkled in the IOMMU code. Further changes
> will take care of that.
>
> Suggested-by: Andrew Cooper <andrew.coop...@citrix.com>
> Signed-off-by: Teddy Astie <teddy.as...@vates.tech>
> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> ---
> Changes since pickup:
> - Unify all CX16 checks into a single patch.
> ---
> xen/drivers/passthrough/amd/iommu_intr.c | 13 +++++++++++++
> xen/drivers/passthrough/amd/pci_amd_iommu.c | 6 ++++++
> xen/drivers/passthrough/vtd/intremap.c | 13 +++++++++++++
> xen/drivers/passthrough/vtd/iommu.c | 7 +++++++
> 4 files changed, 39 insertions(+)
>
> diff --git a/xen/drivers/passthrough/amd/iommu_intr.c
> b/xen/drivers/passthrough/amd/iommu_intr.c
> index 7fc796dec25b..f07fd9e3d970 100644
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -649,6 +649,19 @@ bool __init cf_check iov_supports_xt(void)
> if ( !iommu_enable || !iommu_intremap )
> return false;
>
> + if ( unlikely(!cpu_has_cx16) )
> + {
> + AMD_IOMMU_ERROR("no CMPXCHG16B support, disabling IOMMU\n");
> + /*
> + * Disable IOMMU support at once: there's no reason to check for CX16
> + * yet again when attempting to initialize IOMMU DMA remapping
> + * functionality or interrupt remapping without x2APIC support.
> + */
> + iommu_enable = false;
> + iommu_intremap = iommu_intremap_off;
> + return false;
> + }
> +
> if ( amd_iommu_prepare(true) )
> return false;
>
> diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> index 73dcc4a2dd0c..f96f59440bcc 100644
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -309,6 +309,12 @@ static int __init cf_check iov_detect(void)
> if ( !iommu_enable && !iommu_intremap )
> return 0;
>
> + if ( unlikely(!cpu_has_cx16) )
> + {
> + AMD_IOMMU_ERROR("no CMPXCHG16B support, disabling IOMMU\n");
> + return -ENODEV;
> + }
> +
> if ( (init_done ? amd_iommu_init_late()
> : amd_iommu_init(false)) != 0 )
> {
> diff --git a/xen/drivers/passthrough/vtd/intremap.c
> b/xen/drivers/passthrough/vtd/intremap.c
> index c504852eb818..233db5cb64de 100644
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -150,6 +150,19 @@ bool __init cf_check intel_iommu_supports_eim(void)
> if ( !iommu_qinval || !iommu_intremap || list_empty(&acpi_drhd_units) )
> return false;
>
> + if ( unlikely(!cpu_has_cx16) )
> + {
> + printk(XENLOG_ERR VTDPREFIX "no CMPXCHG16B support, disabling
> IOMMU\n");
> + /*
> + * Disable IOMMU support at once: there's no reason to check for CX16
> + * yet again when attempting to initialize IOMMU DMA remapping
> + * functionality or interrupt remapping without x2APIC support.
> + */
> + iommu_enable = false;
> + iommu_intremap = iommu_intremap_off;
> + return false;
> + }
> +
> /* We MUST have a DRHD unit for each IOAPIC. */
> for ( apic = 0; apic < nr_ioapics; apic++ )
> if ( !ioapic_to_drhd(IO_APIC_ID(apic)) )
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 27a4d1640189..3daedc4f5593 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2633,6 +2633,13 @@ static int __init cf_check vtd_setup(void)
> int ret;
> bool reg_inval_supported = true;
>
> + if ( unlikely(!cpu_has_cx16) )
> + {
> + printk(XENLOG_ERR VTDPREFIX "no CMPXCHG16B support, disabling
> IOMMU\n");
> + ret = -ENODEV;
> + goto error;
> + }
> +
> if ( list_empty(&acpi_drhd_units) )
> {
> ret = -ENODEV;
