Re: [RESEND PATCH 1/5] vtd: Ensure root entry is updated consistently

On 29.07.2026 12:05, Teddy Astie wrote:
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -281,13 +281,13 @@ void free_pgtable_maddr(u64 maddr)
>  /* context entry handling */
>  static u64 bus_to_context_maddr(struct vtd_iommu *iommu, u8 bus)
>  {
> -    struct root_entry *root, *root_entries;
> +    struct root_entry root, *root_entries;
>      u64 maddr;
>  
>      ASSERT(spin_is_locked(&iommu->lock));
>      root_entries = (struct root_entry *)map_vtd_domain_page(iommu->root_maddr);
> -    root = &root_entries[bus];
> -    if ( !root_present(*root) )
> +    root.val = ACCESS_ONCE(root_entries[bus].val);

I'm pretty concerned about this: You're reading only half of the entry here,
and you're writing only half of it further down. The other half is reserved
right now, but there's not even a comment being added to this effect. (Yet
even with a comment, I'd still be concerned, just not as much.)

> @@ -295,11 +295,12 @@ static u64 bus_to_context_maddr(struct vtd_iommu *iommu, u8 bus)
>              unmap_vtd_domain_page(root_entries);
>              return 0;
>          }
> -        set_root_value(*root, maddr);
> -        set_root_present(*root);
> -        iommu_sync_cache(root, sizeof(struct root_entry));
> +        set_root_value(root, maddr);
> +        set_root_present(root);
> +        ACCESS_ONCE(root_entries[bus].val) = root.val;
> +        iommu_sync_cache(&root_entries[bus], sizeof(struct root_entry));

sizeof(<expression>) please in favor of sizeof(<type>), whenever possible.

>      }
> -    maddr = (u64) get_context_addr(*root);
> +    maddr = (u64) get_context_addr(root);

While there, drop the pointless casts, thus getting rid of two style issues
as well?

Jan