Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210226140305.26356-1-nsaenzjulienne@suse.de>
 <20210226140305.26356-10-nsaenzjulienne@suse.de> <3a4cf13f-c098-9ff3-6c0e-2c94daae452b@arm.com>
In-Reply-To: <3a4cf13f-c098-9ff3-6c0e-2c94daae452b@arm.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Wed, 3 Mar 2021 09:44:07 +0100
Message-ID: <CAK8P3a1px7aC92p+rZOYRV2npbHeMPP94fQz==UopHUrApE5Jw@mail.gmail.com>
Subject: Re: [RFC 09/13] iommu/arm-smmu: Make use of dev_64bit_mmio_supported()
To:     Robin Murphy <robin.murphy@arm.com>
Cc:     Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        DTML <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Christoph Hellwig <hch@infradead.org>,
        Neil Armstrong <narmstrong@baylibre.com>,
        David Woodhouse <dwmw2@infradead.org>,
        Russell King - ARM Linux <linux@armlinux.org.uk>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Mar 2, 2021 at 12:07 PM Robin Murphy <robin.murphy@arm.com> wrote:
> On 2021-02-26 14:03, Nicolas Saenz Julienne wrote:

> > index d2a2d1bc58ba..997d13a21717 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > @@ -477,15 +477,20 @@ static inline void arm_smmu_writel(struct arm_smmu_device *smmu, int page,
> >   {
> >       if (smmu->impl && unlikely(smmu->impl->write_reg))
> >               smmu->impl->write_reg(smmu, page, offset, val);
> > -     else
> > +     else if (dev_64bit_mmio_supported(smmu->dev))
> >               writel_relaxed(val, arm_smmu_page(smmu, page) + offset);
> > +     else
> > +             hi_lo_writeq_relaxed(val, arm_smmu_page(smmu, page) + offset);
>
> As Arnd pointed out, this is in completely the wrong place. Also, in
> general it doesn't work if the implementation already needs a hook to
> filter or override register accesses for any other reason. TBH I'm not
> convinced that this isn't *more* of a mess than handling it on a
> SoC-specific basis...

I think the main problem for handling it in a SoC specific way is that there is
no device-independent way to do a 64-bit store as two 32-bit stores:

- some devices need hi_lo_writeq_relaxed(), others need lo_hi_writeq_relaxed(),
  and some absolutely require 64-bit stores and cannot work at all behind a
  broken PCI bus.

- if the driver requires the store to be atomic, it needs to use a spinlock
  around the two writel(), but if the register is on a PCI bus or mapped
  with page attributes that allow posted writes (like arm64 ioremap), then
  you may need to read back the register before spin_unlock to serialize
  them properly. However, reading back an mmio register is slow and can
  have side-effects, so you can't put that in driver-independent code either.

        Arnd