On Tue, Mar 11, 2025 at 2:08 PM Will Deacon <w...@kernel.org> wrote:
>
> On Tue, Mar 04, 2025 at 11:56:48AM -0500, Connor Abbott wrote:
> > In some cases drm/msm has to resume a stalled transaction directly in
> > its fault handler. Experimentally this doesn't work on SMMU500 if the
> > fault hasn't already been acknowledged by clearing FSR. Rather than
> > trying to clear FSR in msm's fault handler and implementing a
> > tricky handshake to avoid accidentally clearing FSR twice, we want to
> > clear FSR before calling the fault handlers, but this means that the
> > contents of registers can change underneath us in the fault handler and
> > msm currently uses a private function to read the register contents for
> > its own purposes in its fault handler, such as using the
> > implementation-defined FSYNR1 to determine which block caused the fault.
> > Fix this by making msm use the register values already read by arm-smmu
> > itself before clearing FSR rather than messing around with reading
> > registers directly.
> >
> > Signed-off-by: Connor Abbott <cwabbo...@gmail.com>
> > ---
> > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 19 +++++++++----------
> > drivers/iommu/arm/arm-smmu/arm-smmu.c | 14 +++++++-------
> > drivers/iommu/arm/arm-smmu/arm-smmu.h | 21 +++++++++++----------
> > 3 files changed, 27 insertions(+), 27 deletions(-)
>
> [...]
>
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > index
> > d3bc77dcd4d40f25bc70f3289616fb866649b022..411d807e0a7033833716635efb3968a0bd3ff237
> > 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > @@ -373,6 +373,16 @@ enum arm_smmu_domain_stage {
> > ARM_SMMU_DOMAIN_NESTED,
> > };
> >
> > +struct arm_smmu_context_fault_info {
> > + unsigned long iova;
> > + u64 ttbr0;
> > + u32 fsr;
> > + u32 fsynr0;
> > + u32 fsynr1;
> > + u32 cbfrsynra;
> > + u32 contextidr;
> > +};
> > +
> > struct arm_smmu_domain {
> > struct arm_smmu_device *smmu;
> > struct io_pgtable_ops *pgtbl_ops;
> > @@ -380,6 +390,7 @@ struct arm_smmu_domain {
> > const struct iommu_flush_ops *flush_ops;
> > struct arm_smmu_cfg cfg;
> > enum arm_smmu_domain_stage stage;
> > + struct arm_smmu_context_fault_info cfi;
>
> Does this mean we have to serialise all faults for a given domain? That
> can't be right...
>
> Will
They are already serialized? There's only one of each register per
context bank, so you can only have one context fault at a time per
context bank, and AFAIK a context bank is 1:1 with a domain. Also this
struct is only written and then read inside the context bank's
interrupt handler, and you can only have one interrupt at a time, no?
Connor
