Re: [PATCH 2/5] vtd: Ensure context entry is cleared properly

On 29.07.2026 11:59, Teddy Astie wrote:
> When removing a context entry for a device, the present bit needs to
> be cleared first, then we can clear the rest of the field. In the current
> logic, the compiler is allowed to perform optimizations in a way where high
> is cleared before the present bit (which is in low part) is, leading to a
> window where the context entry is invalid and would make the IOMMU fault
> (as address width would be set to a reserved value).
> 
> Fix the logic by ensuring we clear the low part first (which also clears
> the present bit) then the high part afterward.
> 
> Fixes: cada0c18f8d1 ("vtd: Move dom0 RMRR check to intel_iommu_remove_device()")
> Reported-by: Teddy Astie <[email protected]>
> Signed-off-by: Andrew Cooper <[email protected]>
> Signed-off-by: Teddy Astie <[email protected]>
> ---
>  xen/drivers/passthrough/vtd/iommu.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index c314ce1db8..1005e200c1 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1889,8 +1889,14 @@ int domain_context_unmap_one(
>  
>      iommu_domid = context_domain_id(*context);
>  
> -    context_clear_present(*context);
> -    context_clear_entry(*context);
> +    /*
> +     * Clear the context entry.
> +     *
> +     * As this is performed with two stores, ensure lo (containing the present
> +     * bit) is cleared first.
> +     */
> +    ACCESS_ONCE(context->lo) = 0;
> +    ACCESS_ONCE(context->hi) = 0;

Implying the placement of the present bit is again something I'm a little uneasy
with.

With the uses of context_clear_{present,entry}() dropped, the macros are unused.
I think they would better be dropped right away, to prevent misguided use
elsewhere.

Jan