Re: [RFC PATCH] PCI: dwc: add support for Allwinner SoCs' PCIe controller

From: Maxime Ripard <maxime@cerno.tech>
To: Icenowy Zheng <icenowy@aosc.io>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Andrew Murray <amurray@thegoodpenguin.co.uk>,
	Bjorn Helgaas <bhelgaas@google.com>, Chen-Yu Tsai <wens@csie.org>,
	Rob Herring <robh@kernel.org>,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-sunxi@googlegroups.com
Subject: Re: [RFC PATCH] PCI: dwc: add support for Allwinner SoCs' PCIe controller
Date: Mon, 6 Apr 2020 10:27:32 +0200	[thread overview]
Message-ID: <20200406082732.nt5d7puwn65j4nvl@gilmour.lan> (raw)
In-Reply-To: <20200402160549.296203-1-icenowy@aosc.io>

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

Hi,

On Fri, Apr 03, 2020 at 12:05:49AM +0800, Icenowy Zheng wrote:
> The Allwinner H6 SoC uses DesignWare's PCIe controller to provide a PCIe
> host.
>
> However, on Allwinner H6, the PCIe host has bad MMIO, which needs to be
> workarounded. A workaround with the EL2 hypervisor functionality of ARM
> Cortex cores is now available, which wraps MMIO operations.
>
> This patch is going to add a driver for the DWC PCIe controller
> available in Allwinner SoCs, either the H6 one when wrapped by the
> hypervisor (so that the driver can consider it as an ordinary PCIe
> controller) or further not buggy ones.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> There's no device tree binding patch available, because I still have
> questions on the device tree compatible string. I want to use it to
> describe that this driver doesn't support the "native Allwinner H6 PCIe
> controller", but a wrapped version with my hypervisor.
>
> I think supporting a "para-physical" device is some new thing, so this
> patch is RFC.
>
> My hypervisor is at [1], and some basic usage documentation is at [2].
>
> [1] https://github.com/Icenowy/aw-el2-barebone
> [2] https://forum.armbian.com/topic/13529-a-try-on-utilizing-h6-pcie-with-virtualization/

I'm a bit concerned to throw yet another mandatory, difficult to
update, component in the already quite long boot chain.

Getting fixes deployed in ATF or U-Boot is already pretty long, having
another component in there will just make it worse, and it's another
hard to debug component that we throw into the mix.

And this prevents any use of virtualisation on the platform.

I haven't found an explanation on what that hypervisor is doing
exactly, but from a look at it it seems that it will trap all the
accesses to the PCIe memory region to emulate a regular space on top
of the restricted one we have?

If so, can't we do that from the kernel directly by using a memory
region that always fault with a fault handler like Framebuffer's
deferred_io is doing (drivers/video/fbdev/core/fb_defio.c) ?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]