Re: [PATCH 2/2] iommu/amd-vi: do not zero IOMMU MMIO region

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/05/2026 8:37 am, Roger Pau Monne wrote:
> Attempting to memset the whole IOMMU MMIO region to zero is dangerous to
> say the least.  We don't know what registers might be there, neither what
> values might be safe for those registers.

Minor grammar.  "there, nor which values".

> On a forthcoming platform doing
> the zeroing of the MMIO region can put the IOMMU in a broken state,

"does put"

> which is not recovered by the IOMMU initialization procedure in Xen.

"recoverable".

> diff --git a/xen/drivers/passthrough/amd/iommu_init.c 
> b/xen/drivers/passthrough/amd/iommu_init.c
> index 76ae78e5ea53..8bf5ca4de18f 100644
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1367,11 +1355,14 @@ static int __init amd_iommu_prepare_one(struct 
> amd_iommu *iommu)
>  {
>      int rc = alloc_ivrs_mappings(iommu->sbdf.seg);
>  
> -    if ( !rc )
> -        rc = map_iommu_mmio_region(iommu);
>      if ( rc )
>          return rc;
>  
> +    iommu->mmio_base = ioremap(iommu->mmio_base_phys,
> +                               IOMMU_MMIO_REGION_LENGTH);
> +    if ( !iommu->mmio_base )
> +        return -ENOMEM;
> +
>      get_iommu_features(iommu);
>  
>      /*
> @@ -1381,6 +1372,11 @@ static int __init amd_iommu_prepare_one(struct 
> amd_iommu *iommu)
>      if ( amd_iommu_max_paging_mode < amd_iommu_min_paging_mode )
>          return -ERANGE;
>  
> +    /* Read current control register and forcefully disable the IOMMU. */
> +    iommu->ctrl.raw = readq(iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET);
> +    disable_iommu(iommu, true);
> +    iommu->ctrl.raw = 0;
> +
>      return 0;
>  } 

These two things are unrelated at want splitting into separate patches
at a minimum.  The removal of memset() critically needs backporting.

As for disabling the IOMMU, I'm not certain it's wise.

Linux can already "bring up" an already-live IOMMU and Xen needs to gain
this ability in due course.  This is mainly for supporting PreBoot DMA
Protection, but also for things like the kexec environment.

~Andrew