Re: [PATCH] iommu/vtd: fix address translation for superpages

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, May 09, 2023 at 06:06:45PM +0200, Jan Beulich wrote:
> On 09.05.2023 12:41, Roger Pau Monne wrote:
> > When translating an address that falls inside of a superpage in the
> > IOMMU page tables the fetching of the PTE physical address field
> > wasn't using dma_pte_addr(), which caused the returned data to be
> > corrupt as it would contain bits not related to the address field.
> 
> I'm afraid I don't understand:
> 
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -359,16 +359,18 @@ static uint64_t addr_to_dma_page_maddr(struct domain 
> > *domain, daddr_t addr,
> >  
> >              if ( !alloc )
> >              {
> > -                pte_maddr = 0;
> >                  if ( !dma_pte_present(*pte) )
> > +                {
> > +                    pte_maddr = 0;
> >                      break;
> > +                }
> >  
> >                  /*
> >                   * When the leaf entry was requested, pass back the full 
> > PTE,
> >                   * with the address adjusted to account for the residual of
> >                   * the walk.
> >                   */
> > -                pte_maddr = pte->val +
> > +                pte_maddr +=
> >                      (addr & ((1UL << level_to_offset_bits(level)) - 1) &
> >                       PAGE_MASK);
> 
> With this change you're now violating what the comment says (plus what
> the comment ahead of the function says). And it says what it says for
> a reason - see intel_iommu_lookup_page(), which I think your change is
> breaking.

Hm, but the code in intel_iommu_lookup_page() is now wrong as it takes
the bits in DMA_PTE_CONTIG_MASK as part of the physical address when
doing the conversion to mfn?  maddr_to_mfn() doesn't perform a any
masking to remove the bits above PADDR_BITS.

Thanks, Roger.