On Mon, Sep 27, 2021 at 11:04:26AM +0200, Jan Beulich wrote:
> On 24.09.2021 16:45, Roger Pau Monné wrote:
> > On Fri, Sep 24, 2021 at 11:42:13AM +0200, Jan Beulich wrote:
> >> - parent = (struct dma_pte
> >> *)map_vtd_domain_page(hd->arch.vtd.pgd_maddr);
> >> - while ( level > 1 )
> >> + pte_maddr = hd->arch.vtd.pgd_maddr;
> >> + parent = map_vtd_domain_page(pte_maddr);
> >> + while ( level > target )
> >> {
> >> offset = address_level_offset(addr, level);
> >> pte = &parent[offset];
> >>
> >> pte_maddr = dma_pte_addr(*pte);
> >> - if ( !pte_maddr )
> >> + if ( !dma_pte_present(*pte) || (level > 1 &&
> >> dma_pte_superpage(*pte)) )
> >> {
> >> struct page_info *pg;
> >> + /*
> >> + * Higher level tables always set r/w, last level page table
> >> + * controls read/write.
> >> + */
> >> + struct dma_pte new_pte = { DMA_PTE_PROT };
> >>
> >> if ( !alloc )
> >> - break;
> >> + {
> >> + pte_maddr = 0;
> >> + if ( !dma_pte_present(*pte) )
> >> + 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 +
> >
> > Wouldn't it be better to use dma_pte_addr(*pte) rather than accessing
> > pte->val, and then you could drop the PAGE_MASK?
> >
> > Or is the addr parameter not guaranteed to be page aligned?
>
> addr is page aligned, but may not be superpage aligned. Yet that's not
> the point here. As per the comment at the top of the function (and as
> per the needs of intel_iommu_lookup_page()) we want to return a proper
> (even if fake) PTE here, i.e. in particular including the access
> control bits. Is "full" in the comment not sufficient to express this?
I see. I guess I got confused by the function name. It would be better
called addr_to_dma_pte?
Thanks, Roger.
