Re: [RFC PATCH 06/11] iommu: arm-smmu: Remove Calxeda secure mode quirk

On Fri, 28 Feb 2020 13:56:46 +0000
Will Deacon [off-list ref] wrote:

On Fri, Feb 28, 2020 at 01:42:54PM +0000, Andre Przywara wrote:

quoted

On Fri, 28 Feb 2020 10:50:25 +0000
Will Deacon [off-list ref] wrote:  

quoted

On Fri, Feb 28, 2020 at 10:25:56AM +0000, Andre Przywara wrote:  

quoted

quoted

On Tue, Feb 25, 2020 at 04:01:54PM -0600, Rob Herring wrote:    

quoted

Seems we're leaving the platform support for now, but I think we never
actually enabled SMMU support. It's not in the dts either in mainline
nor the version I have which should be close to what shipped in
firmware. So as long as Andre agrees, this one is good to apply.      

Andre? Can I queue this one for 5.7, please?    

I was wondering how much of a pain it is to keep it in? AFAICS there are
other users of the "impl" indirection. If those goes away, I would be
happy to let Calxeda go.    

The impl stuff is new, so we'll keep it around. The concern is more about
testing (see below).
  

quoted

But Eric had the magic DT nodes to get the SMMU working, and I used that
before, with updating the DT either on flash or dynamically via U-Boot.    

What did you actually use the SMMU for, though? The
'arm_iommu_create_mapping()' interface isn't widely used and, given that
highbank doesn't support KVM, the use-cases for VFIO are pretty limited
too.  

AFAIK Highbank doesn't have the SMMU, probably mostly for that reason.
I have a DT snippet for Midway, and that puts the MMIO base at ~36GB, which is not possible on Highbank.
So I think that the quirk is really meant and needed for Midway.  

Sorry, but I don't follow your reasoning here. The MMIO base has nothing
to do with the quirk,

It hasn't, but Highbank has no LPAE, so couldn't possible have a device at such an address. And this is the only MMIO address I know of.

although doing some digging it looks like your
conclusion about this applying to Midway (ecx-2000?) is correct:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/226095.html

Right, thanks for that find. Yes, Midway is the codename for the ECX-2000 SoC product.

Cheers,
Andre
 

quoted

quoted

quoted

So I don't know exactly *how* desperate you are with removing this, or if
there are other reasons than "negative diffstat", but if possible I would
like to keep it in.    

It's more that we *do* make quite a lot of changes to the arm-smmu driver
and it's never tested with this quirk. If you're stepping up to run smmu
tests on my queue for each release on highbank, then great, but otherwise
I'd rather not carry the code for fun. The change in diffstat is minimal
(we're going to need to hooks for nvidia, who broke things in a different
way).  

I am about to set up some more sophisticated testing, and will include
some SMMU bits in it.  

Yes, please.

Will