On 30.11.2021 12:56, Roger Pau Monné wrote:
> 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?
That wouldn't match its new purpose either. It can return an address
_or_ a full PTE, as per - as said - the comment being added at the
top of the function.
Jan
