Re: [PATCH v4 14/14] vtd: use a bit field for dma_pte

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.08.2020 15:42, Paul Durrant wrote:
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1772,13 +1772,14 @@ static int __must_check intel_iommu_map_page(struct 
> domain *d, dfn_t dfn,
>      old = *pte;
>  
>      dma_set_pte_addr(new, mfn_to_maddr(mfn));
> -    dma_set_pte_prot(new,
> -                     ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
> -                     ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
> +    if ( flags & IOMMUF_readable )
> +        dma_set_pte_readable(new);
> +    if ( flags & IOMMUF_writable )
> +        dma_set_pte_writable(new);
>  
>      /* Set the SNP on leaf page table if Snoop Control available */
>      if ( iommu_snoop )
> -        dma_set_pte_snp(new);
> +        dma_set_pte_snoop(new);

Perhaps simply use an initializer:

    new = (struct dma_ptr){
            .r = flags & IOMMUF_readable,
            .w = flags & IOMMUF_writable,
            .snp = iommu_snoop,
            .addr = mfn_x(mfn),
        };

? This also points out that the "addr" field isn't really an address,
and hence may want renaming.

Again comments on the two earlier patch apply here respectively (or
else part of the suggestion above isn't going to work as is).

Jan