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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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?