On Tue, Mar 2, 2021 at 12:07 PM Robin Murphy <robin.murphy@xxxxxxx> 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