Re: [PATCH v2 05/10] xen/arm: smmuv3: Remove the page 1 fixup

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Stefano,

> On 3 Sep 2022, at 12:17 am, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> On Fri, 2 Sep 2022, Rahul Singh wrote:
>> From: Robin Murphy <robin.murphy@xxxxxxx>
>> 
>> Backport Linux commit 86d2d9214880. This is the clean backport without
>> any changes.
>> 
>> Since we now keep track of page 1 via a separate pointer that
>> already encapsulates aliasing to page 0 as necessary, we can remove
>> the clunky fixup routine and simply use the relevant bases directly.
>> The current architecture spec (IHI0070D.a) defines
>> SMMU_{EVENTQ,PRIQ}_{PROD,CONS} as offsets relative to page 1, so the
>> cleanup represents a little bit of convergence as well as just
>> lines of code saved.
>> 
>> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
>> Signed-off-by: Will Deacon <will@xxxxxxxxxx>
>> Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
>> 86d2d9214880
>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>> ---
>> Changes in v2:
>> - fix commit msg.
>> ---
>> xen/drivers/passthrough/arm/smmu-v3.c | 42 ++++++++++-----------------
>> xen/drivers/passthrough/arm/smmu-v3.h |  8 ++---
>> 2 files changed, 20 insertions(+), 30 deletions(-)
>> 
>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
>> b/xen/drivers/passthrough/arm/smmu-v3.c
>> index 85ad066266..f5485a8a1c 100644
>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>> @@ -235,15 +235,6 @@ static struct arm_smmu_option_prop arm_smmu_options[] = 
>> {
>>      { 0, NULL},
>> };
>> 
>> -static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset,
>> -                                             struct arm_smmu_device *smmu)
>> -{
>> -    if (offset > SZ_64K)
>> -            return smmu->page1 + offset - SZ_64K;
>> -
>> -    return smmu->base + offset;
>> -}
>> -
>> static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>> {
>>      return container_of(dom, struct arm_smmu_domain, domain);
>> @@ -1578,6 +1569,7 @@ static int arm_smmu_dt_xlate(struct device *dev,
>> /* Probing and initialisation functions */
>> static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
>>                                 struct arm_smmu_queue *q,
>> +                               void __iomem *page,
>>                                 unsigned long prod_off,
>>                                 unsigned long cons_off,
>>                                 size_t dwords, const char *name)
>> @@ -1606,8 +1598,8 @@ static int arm_smmu_init_one_queue(struct 
>> arm_smmu_device *smmu,
>>                       1 << q->llq.max_n_shift, name);
>>      }
>> 
>> -    q->prod_reg     = arm_smmu_page1_fixup(prod_off, smmu);
>> -    q->cons_reg     = arm_smmu_page1_fixup(cons_off, smmu);
>> +    q->prod_reg     = page + prod_off;
>> +    q->cons_reg     = page + prod_off;
> 
> In the original patch it was:
> 
> -     q->prod_reg     = arm_smmu_page1_fixup(prod_off, smmu);
> -     q->cons_reg     = arm_smmu_page1_fixup(cons_off, smmu);
> +     q->prod_reg     = page + prod_off;
> +     q->cons_reg     = page + cons_off;
> 
> Specifically the second line seems to be wrong here?

Yes you are right I somehow miss that . I will fix that.

Regards.
Rahul