linux-kernel - Re: [PATCH 00/16] IOMMU driver for Kirin 960/970

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200817124617.303bb4a9@coco.lan>
Date:   Mon, 17 Aug 2020 12:46:17 +0200
From:   Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Christoph Hellwig <hch@...radead.org>, devel@...verdev.osuosl.org,
        devicetree@...r.kernel.org, Joerg Roedel <jroedel@...e.de>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Joerg Roedel <joro@...tes.org>, linuxarm@...wei.com,
        Wei Xu <xuwei5@...ilicon.com>, linux-kernel@...r.kernel.org,
        iommu@...ts.linux-foundation.org, Rob Herring <robh+dt@...nel.org>,
        John Stultz <john.stultz@...aro.org>,
        Chenfeng <puck.chen@...ilicon.com>, mauro.chehab@...wei.com,
        Suzhuangluan <suzhuangluan@...ilicon.com>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 00/16] IOMMU driver for Kirin 960/970

Em Mon, 17 Aug 2020 11:37:03 +0200
Greg Kroah-Hartman <gregkh@...uxfoundation.org> escreveu:

> On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Christoph,
> > 
> > Em Mon, 17 Aug 2020 09:21:06 +0100
> > Christoph Hellwig <hch@...radead.org> escreveu:
> >   
> > > On Mon, Aug 17, 2020 at 09:49:59AM +0200, Mauro Carvalho Chehab wrote:  
> > > > Add a driver for the Kirin 960/970 iommu.
> > > > 
> > > > As on the past series, this starts from the original 4.9 driver from
> > > > the 96boards tree:
> > > > 
> > > > 	https://github.com/96boards-hikey/linux/tree/hikey970-v4.9
> > > > 
> > > > The remaining patches add SPDX headers and make it build and run with
> > > > the upstream Kernel.    
> > > 
> > > Please don't add iommu drivers to staging, and just work with the
> > > maintainers to properly clean it up.  
> > 
> > I need to start from the original patch in order to preserve its
> > authorship.
> > 
> > My plan is to work with the iommu subsystem maintainers after
> > have this (and another pending patch series for DRM) merged.
> >   
> > > I also don't think adding a totally out of date not compiling version
> > > is a good idea.  Please do a proper rollup, and if required (probably
> > > not in this case), split it into useful chunks.  
> > 
> > This series make this driver working as expected.
> > 
> > I mean, while patch 01/16 is against Kernel 4.9, the other patches
> > on this series ports it to upstream, cleans up the driver and
> > address several issues on it.
> > 
> > This specific IOMMU seems to be an specific part of the SoC dedicated for 
> > the display engine and by the encoding/decoding images via the ISP. 
> > With this series, this driver builds and runs as expected, providing
> > IOMMU support needed by the upcoming KMS/DRM driver.
> > 
> > The only issue on it (as far as I can tell) is that the DT bindings
> > require some work, as, instead of using dma-ranges, the DRM driver binds
> > into it with:
> > 
> > 	smmu_lpae {
> >                  compatible = "hisilicon,smmu-lpae";
> >          };
> > 
> >          dpe: dpe@...00000 {
> >                  compatible = "hisilicon,kirin970-dpe";
> > ...
> >                  iommu_info {
> >                          start-addr = <0x8000>;
> >                          size = <0xbfff8000>;
> >                  };
> >          };
> > 
> > In order to properly address it, the best would be to also have the
> > DRM driver merged upstream, as it relies on it. So, a change in DT will 
> > also mean a change at the way the DRM uses it.
> > 
> > The DRM itself should go via staging, as it has some bugs that I'd
> > like to fix before moving it to drivers/gpu/drm. Those are more
> > tricky to solve, as they seem to require using different settings for 
> > some hardware registers, and the downstream driver also have the same 
> > issues. Fixing them will likely require some time.  
> 
> DRM drivers can't go through staging unless you get the DRM developers
> to agree with it, and last I heard, they were strongly against it.


Ok, I'll ping them. There's already a thread I opened at the DRM devel
ML about this driver. IMHO, there's nothing on this specific driver
that would prevent having it on staging for a while, until the two
or three remaining bugs gets fixed.

On the other hand, the code already follows the current DRM policies,
as far as I can tell. The bugs are related to some specific register
settings that would need to be better tuned (and maybe some delay
when waiting for EDID data at the already-existing adv7535 driver).
Maybe it would be preferred to have it at drivers/gpu even with
such bugs.

> It's _always_ faster to just do the work out-of-tree for a week or so
> and then merge it correctly to the proper part of the kernel tree.  I'd
> recommend doing that here for the iommu driver, as well as the DRM
> driver.

It is OK for me to working for a week or so, until the iommu people
become happy with that. 

The main reason of submitting via staging is that I need to preserve
the patch that added this driver as-is, in order to preserve its
SoB and not causing legal issues.

It it is OK for iommu to accept a submission like that, I can
re-submit it, doing the changes at drivers/iommu.

If not, besides this series, the only alternative I can think of is to 
merge it first at the staging, with the incremental changes, and with
a final patch moving the code out of staging.

Thanks,
Mauro