Re: [PATCH v4 0/7] Add support for QCOM IOMMU v2 and 500

From: AngeloGioacchino Del Regno <kholk11@gmail.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: MSM <linux-arm-msm@vger.kernel.org>,
	iommu@lists.linux-foundation.org, marijns95@gmail.com,
	agross@kernel.org, Rob Clark <robdclark@gmail.com>,
	joro@8bytes.org
Subject: Re: [PATCH v4 0/7] Add support for QCOM IOMMU v2 and 500
Date: Sat, 5 Oct 2019 11:32:35 +0200	[thread overview]
Message-ID: <CAK7fi1ZkjV6vt=OeRYz2JGC9v4n4eb5Rupqc0TWQmnM1UdJ-mg@mail.gmail.com> (raw)
In-Reply-To: <20191005045626.GE6390@tuxbook-pro>

Il giorno sab 5 ott 2019 alle ore 06:56 Bjorn Andersson
<bjorn.andersson@linaro.org> ha scritto:
>
> On Tue 01 Oct 15:01 PDT 2019, kholk11@gmail.com wrote:
>
> > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> >
> > Some Qualcomm Family-B SoCs have got a different version of the QCOM
> > IOMMU, specifically v2 and 500, which perfectly adhere to the current
> > qcom_iommu driver, but need some variations due to slightly different
> > hypervisor behavior.
> >
>
> Do you think it's out of the question to get the arm-smmu driver to play
> nice with this platform?
>
>
> If not, would it be possible to change the DT binding so that we specify
> the SID and then read the SMR and S2CR registers to figure out the
> associated context bank?
>
> Regards,
> Bjorn
>
> > The personal aim is to upstream MSM8956 as much as possible.
> >
> > This code has been tested on two Sony phones featuring the Qualcomm
> > MSM8956 SoC.
> >
> > Changes in v2:
> > - Fixed optional properties placement in documentation
> >
> > Changes in v3:
> > - Rebased onto linux-next 01/10/2019
> > - Added missing SCM commit (required by the AArch64 PT switch support)
> >
> > Changes in v4:
> > - Removed rej files from the SCM patch (I'm truly sorry for the noise...)
> >
> > Angelo G. Del Regno (1):
> >   firmware: qcom: scm: Add function to set IOMMU pagetable addressing
> >
> > AngeloGioacchino Del Regno (6):
> >   iommu/qcom: Use the asid read from device-tree if specified
> >   iommu/qcom: Write TCR before TTBRs to fix ASID access behavior
> >   iommu/qcom: Properly reset the IOMMU context
> >   iommu/qcom: Add support for AArch64 IOMMU pagetables
> >   iommu/qcom: Index contexts by asid number to allow asid 0
> >   iommu/qcom: Add support for QCIOMMUv2 and QCIOMMU-500 secured contexts
> >
> >  .../devicetree/bindings/iommu/qcom,iommu.txt  |   5 +
> >  drivers/firmware/qcom_scm-32.c                |   6 +
> >  drivers/firmware/qcom_scm-64.c                |  15 ++
> >  drivers/firmware/qcom_scm.c                   |   7 +
> >  drivers/firmware/qcom_scm.h                   |   4 +
> >  drivers/iommu/qcom_iommu.c                    | 134 ++++++++++++++----
> >  include/linux/qcom_scm.h                      |   2 +
> >  7 files changed, 145 insertions(+), 28 deletions(-)
> >
> > --
> > 2.21.0
> >

In reality, when I started the IOMMU integration for this SoC, the
arm-smmu didn't even
have the new arm-smmu-impl stuff....
I tried multiple times to get the arm-smmu driver to play nice with
this IOMMU, but it's
really too much work to do there, (even with the new arm-smmu-impl
stuff) as we would
have to make a lot of changes in that driver just to support
this thing which, yes - it's standard-ish - but no, due to the
firmware configuration that
happens on this kind of platforms (the entire family, 8917, 8953,
8956, 8976 and others)
there is a lil percent of the arm-smmu code that would apply.

Shorter said, since it would be a complete mess to integrate the
support there, IMHO
it's really not a good idea.
In my trials for that I've ended up changing like 50% of the arm-smmu driver.

After all, the qcom_iommu driver is there to get IOMMUs with this kind
of firmware
configuration working and, even if it was originally done for
QCIOMMUv1, as I have
also explained in one of the patches here, 98-99% of the reasons why we have a
separate driver called qcom_iommu are applying to the implementation
that I've done.