> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 06 August 2020 13:54
> To: Paul Durrant <p...@xen.org>
> Cc: xen-devel@lists.xenproject.org; Paul Durrant <pdurr...@amazon.com>; Kevin
> Tian
> <kevin.t...@intel.com>
> Subject: Re: [PATCH v4 14/14] vtd: use a bit field for dma_pte
>
> 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.
If I am getting rid of macros then this makes more sense.
Paul
>
> 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
