Re: [PATCH v3 12/13] xen/iommu: smmu: Add Xen specific code to be able to use the driver

[Julien Grall <julien.grall@linaro.org>]

Hi Stefano,

On 09/02/15 15:40, Julien Grall wrote:
>>
>>>   static void arm_smmu_init_context_bank(struct arm_smmu_domain
>>> *smmu_domain)
>>>   {
>>> @@ -757,6 +996,7 @@ static void arm_smmu_init_context_bank(struct
>>> arm_smmu_domain *smmu_domain)
>>>       struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>>>       struct arm_smmu_device *smmu = smmu_domain->smmu;
>>>       void __iomem *cb_base, *gr0_base, *gr1_base;
>>> +    paddr_t p2maddr;
>>>
>>>       gr0_base = ARM_SMMU_GR0(smmu);
>>>       gr1_base = ARM_SMMU_GR1(smmu);
>>> @@ -840,11 +1080,16 @@ static void arm_smmu_init_context_bank(struct
>>> arm_smmu_domain *smmu_domain)
>>>       }
>>>
>>>       /* TTBR0 */
>>> -    arm_smmu_flush_pgtable(smmu, cfg->pgd,
>>> -                   PTRS_PER_PGD * sizeof(pgd_t));
>>> -    reg = __pa(cfg->pgd);
>>> +    /* Xen: The page table is shared with the P2M code */
>>> +    ASSERT(smmu_domain->cfg.domain != NULL);
>>> +    p2maddr = page_to_maddr(smmu_domain->cfg.domain->arch.p2m.root);
>>> +
>>> +    dev_notice(smmu->dev, "d%u: p2maddr 0x%"PRIpaddr"\n",
>>> +           smmu_domain->cfg.domain->domain_id, p2maddr);
>>> +
>>> +    reg = (p2maddr & ((1ULL << 32) - 1));
>>>       writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
>>> -    reg = (phys_addr_t)__pa(cfg->pgd) >> 32;
>>> +    reg = (p2maddr >> 32);
>>>       if (stage1)
>>>           reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT;
>>>       writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI);
>>
>> Not sure how we could get rid of this change.
>> Maybe we could keep pgd in arm_smmu_cfg and make it point to
>> smmu_domain->cfg.domain->arch.p2m.root?
> 
> Maybe, I will look at it.

I though more about it. __pa(...) requires a virtual address on
parameter. As the page allocated for the P2M is part of the domheap,
there is no permanent mapping.

That would require to map the p2m or override __pa.

IHMO, both of those case a less clear than the current code. So I will
keep the current solution.

Regards,

-- 
Julien Grall