Re: [Xen-devel] [RFC PATCH v1 0/7] IPMMU-VMSA support on ARM

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 8 Aug 2017, Julien Grall wrote:
> On 01/08/17 18:13, Oleksandr Tyshchenko wrote:
> > Hi, Julien
> > 
> > On Tue, Aug 1, 2017 at 3:27 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> > > On 26/07/17 16:09, Oleksandr Tyshchenko wrote:
> > > > 
> > > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > > 
> > > > Hi, all.
> > > 
> > > 
> > > Hi,
> > > 
> > > Please CC maintainers and any relevant person on the cover letter. This is
> > > quite useful to have in the inbox.
> > Yes. I CCed guys who, I think, are/were involved in IPMMU-VMSA
> > development from Linux side +
> > IOMMU maintainers (mostly ARM). Sorry, if I missed someone or mistakenly
> > added.
> > 
> > > 
> > > > The purpose of this patch series is to add IPMMU-VMSA support to Xen on
> > > > ARM.
> > > > It is VMSA-compatible IOMMU that integrated in the newest Renesas R-Car
> > > > Gen3 SoCs (ARM).
> > > > And this IOMMU can't share the page table with the CPU since it doesn't
> > > > use the same page-table format
> > > > as the CPU on ARM therefore I name it "Non-shared" IOMMU.
> > > > This all means that current patch series must be based on "Non-shared"
> > > > IOMMU support [1]
> > > > for the IPMMU-VMSA to be functional inside Xen.
> > > > 
> > > > The IPMMU-VMSA driver as well as the ARM LPAE allocator were directly
> > > > ported from BSP for Linux the vendor provides:
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git
> > > > rcar-3.5.3
> > > 
> > > 
> > > I think this is probably a good starting point to discuss about IOMMU
> > > support in Xen. I skimmed through the patches and saw the words "rfc" and
> > > "ported from BSP".
> > Well, at the time of porting IPMMU-VMSA driver, BSP [1] had more
> > complete support than mainline [2]
> > and seems to have at the moment.
> > For example, mainline driver still has single IPMMU context while BSP
> > driver can have up to 8 contexts,
> > the maximum uTLBs mainline driver can handle is 32, but for BSP driver
> > this value was increased to 48, etc.
> > But, I see attempts to get all required support in [3]. So, when this
> > support reaches upsteam, I hope,
> > it won't be a big problem to rebase on mainline driver if we decide to
> > align with it.
> 
> My main concern here is this driver haven't had a thorough review by the Linux
> community.
> 
> When we ported the SMMUv{1,2} driver we knew the Linux community was happy
> with it and hence adapting for Xen was not a big deal. There are only few
> limited changes in the code from Linux.
> 
> Looking at patch #2, #6, #7, the changes don't seem limited in the code from
> Linux + it is a driver from a BSP. The code really needs to be very close to
> make the port from Linux really worth it.
> 
> Stefano do you have any opinion?

Yes, the fact that a driver is already in Linux gives us a guarantee
that it had been reviewed before. This driver doesn't come with that
guarantee, that means that one of us would have to review it. Otherwise,
we could take the contribution and mark it as "unsupported/experimental"
like we do for many new features until we think it is stable enough.

Regardless, I think you are right that there is no gain in trying to
make this patch series "compatible" with the original Linux driver,
because the driver isn't in Linux. We might as well take a clean
contribution to Xen without any #if 0.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel