Hi Joerg,

Joerg Roedel <[email protected]> wrote @ Mon, 4 Feb 2013 20:53:32 +0100:

> >  static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
> >  {
> > -   BUG_ON(offs < 0x10);
> > -   if (offs < 0x3c)
> > -           return readl(smmu->regs[0] + offs - 0x10);
> > -   BUG_ON(offs < 0x1f0);
> > -   if (offs < 0x200)
> > -           return readl(smmu->regs[1] + offs - 0x1f0);
> > -   BUG_ON(offs < 0x228);
> > -   if (offs < 0x284)
> > -           return readl(smmu->regs[2] + offs - 0x228);
> > +   int i;
> > +
> > +   for (i = 0; i < smmu->nregs; i++) {
> > +           void __iomem *addr = smmu->regbase + offs;
> > +
> > +           BUG_ON(addr < smmu->regs[i]);
> > +           if (addr <= smmu->rege[i])
> > +                   return readl(addr);
> > +   }
> 
> This loop is purely for checking offset to be valid. And this loop is
> repeated in the smmu_write() function. I queued a patch on-top to make
> this more clear. Please double-check:

Actually I did the similar thing in the first version of this patch(*1)

+static inline void smmu_check_reg_range(size_t offs)
+{
+       int i;
+
+       for (i = 0; i < smmu->nregs; i++) {
+           BUG_ON(offs < smmu->regs[i] - smmu->regbase);
+                       if (offs <= smmu->rege[i] - smmu->regbase)
+                                break;
+        }
+}

*1:
http://lists.linuxfoundation.org/pipermail/iommu/2013-January/005072.html

Then, Stehpen pointed out about this check function(*2).

"... here, you'd be doing the loop every access anyway, so you may as
well not calculate regbase at all, move the body of
smmu_check_reg_range() into smmu_read()/smmu_write(), and do the access
inside the if statement inside the loop, with the per-range mapping."

*2:
http://lists.linuxfoundation.org/pipermail/iommu/2013-January/005074.html

I might not get Stehpen's point in the latest patch(?), though....
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to