PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

[Pavel Fedin]

 Hello! I have retested this on 4.3rc3, everything is fine. 2/2 has been already picked up, so this
is the only bit left.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of
> Pavel Fedin
> Sent: Friday, September 04, 2015 4:30 PM
> To: linux-arm-kernel at lists.infradead.org
> Cc: Marc Zyngier; Russell King; trivial at kernel.org; Will Deacon
> Subject: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine
> 
> msi.h is added because it is required by linux/msi.h if
> CONFIG_GENERIC_MSI_IRQ_DOMAIN is defined. It gets enabled by the following
> KConfig dependency chain:
> ARM_GIC_V2M => PCI_MSI_IRQ_DOMAIN => GENERIC_MSI_IRQ_DOMAIN
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  arch/arm/Kconfig            | 1 +
>  arch/arm/include/asm/Kbuild | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 45df48b..1091025 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -815,6 +815,7 @@ config ARCH_VIRT
>  	bool "Dummy Virtual Machine" if ARCH_MULTI_V7
>  	select ARM_AMBA
>  	select ARM_GIC
> +	select ARM_GIC_V2M if PCI_MSI
>  	select ARM_PSCI
>  	select HAVE_ARM_ARCH_TIMER
> 
> diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
> index 3c4596d..01806f5 100644
> --- a/arch/arm/include/asm/Kbuild
> +++ b/arch/arm/include/asm/Kbuild
> @@ -14,6 +14,7 @@ generic-y += local.h
>  generic-y += local64.h
>  generic-y += mcs_spinlock.h
>  generic-y += msgbuf.h
> +generic-y += msi.h
>  generic-y += param.h
>  generic-y += parport.h
>  generic-y += poll.h
> --
> 2.4.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

[Russell King - ARM Linux]

On Wed, Oct 07, 2015 at 11:45:06AM +0300, Pavel Fedin wrote:
>  Hello! I have retested this on 4.3rc3, everything is fine. 2/2 has
> been already picked up, so this is the only bit left.

Sorry, I've no idea if this is the right thing to do.

I know we have MSI-using platforms working on 32-bit ARM at the moment,
which brings up the question about msi.h in your patch, and what are
the implications for the existing MSI using platforms of having
PCI_MSI_IRQ_DOMAIN enabled in a multiplatform kernel.

I don't know enough to be able to merge this patch, and there's not
enough information in the commit message either.  Plus, with the lack
of acks from people in ARM, I'm very reluctant to take this (I don't
know whether the original patch got any, it's fallen out of my mailbox.)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

[Pavel Fedin]

 Hello!

> I don't know enough to be able to merge this patch, and there's not
> enough information in the commit message either.

 What kind of information would you need in the message?

>  Plus, with the lack
> of acks from people in ARM, I'm very reluctant to take this

 cc'ing to Marc and Peter Maydell (the main person behind qemu).

> (I don't
> know whether the original patch got any, it's fallen out of my mailbox.)

 2/2 from this series has been ACKed and picked up by Marc Zynger. He told me that 1/2 is your area
of competence.
 Just in case, original series is here: http://www.spinics.net/lists/arm-kernel/msg442981.html

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

[Russell King - ARM Linux]

On Wed, Oct 07, 2015 at 02:02:23PM +0300, Pavel Fedin wrote:
>  2/2 from this series has been ACKed and picked up by Marc Zynger. He
> told me that 1/2 is your area of competence.

... which is kind of funny, because all of ARM Ltd's platforms that
have PCIe on the board, and thus might have MSI, the PCIe is non-
functional.  So, until SolidRun sent me an Armada 388 platform a few
weeks back, I had zero MSI platforms, and zero MSI knowledge.

Just because code falls into a particular area of the tree does not
mean that the person who's looking after that area knows everything
inside out - especially when it comes to architecture maintanence
and the code in that subtree covers virtually every technology out
there, and the maintainer is kept functional hardware-starved [*].

Now, the Armada 388 doesn't use the GIC for MSI, but uses its own
interrupt controller.  I haven't yet worked out which bits of the
kernel are being used to provide this functionality yet, but I know
it works.  What I don't want to do is to merge your patch and then
have my one and only MSI platform break because of it, so what I'm
looking for is an assurance that this has had some exposure on
non-GIC MSI platforms, or has been reviewed with this in mind.

We're having far too many regressions of the kind of "let's get X
working on platform Y which then causes platform Z to break".


* - I'm not saying that I want more hardware; I've been out of physical
space for new hardware for some time, and the few extra platforms I've
received recently are having to be stacked on top of each other - which
isn't good when they're bare boards.  It would be nice, however, if the
hardware I did have was fully functional!

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

[Pavel Fedin]

 Hello!

> I know we have MSI-using platforms working on 32-bit ARM at the moment,
> which brings up the question about msi.h in your patch, and what are
> the implications for the existing MSI using platforms of having
> PCI_MSI_IRQ_DOMAIN enabled in a multiplatform kernel.

 I have just tested, what happens if i disable PCI_MSI_IRQ_DOMAIN. No, it's not going to work,
irq-gic-v2m.c requires MSI domain API. Looks like old MSI drivers do not use IRQ domains and just
transparently handle translation of MSIs to SPI IDs.
 At this point unfortunately i cannot do anything else, because i don't have any of those platforms,
so cannot test. Added also Christoffer to cc. Hopefully somebody in ARM or Linaro gets interested.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

[Marc Zyngier]

On 07/10/15 12:29, Russell King - ARM Linux wrote:
> On Wed, Oct 07, 2015 at 02:02:23PM +0300, Pavel Fedin wrote:
>>  2/2 from this series has been ACKed and picked up by Marc Zynger. He
>> told me that 1/2 is your area of competence.
> 
> ... which is kind of funny, because all of ARM Ltd's platforms that
> have PCIe on the board, and thus might have MSI, the PCIe is non-
> functional.  So, until SolidRun sent me an Armada 388 platform a few
> weeks back, I had zero MSI platforms, and zero MSI knowledge.
> 
> Just because code falls into a particular area of the tree does not
> mean that the person who's looking after that area knows everything
> inside out - especially when it comes to architecture maintanence
> and the code in that subtree covers virtually every technology out
> there, and the maintainer is kept functional hardware-starved [*].
> 
> Now, the Armada 388 doesn't use the GIC for MSI, but uses its own
> interrupt controller.  I haven't yet worked out which bits of the
> kernel are being used to provide this functionality yet, but I know
> it works.  What I don't want to do is to merge your patch and then
> have my one and only MSI platform break because of it, so what I'm
> looking for is an assurance that this has had some exposure on
> non-GIC MSI platforms, or has been reviewed with this in mind.
> 
> We're having far too many regressions of the kind of "let's get X
> working on platform Y which then causes platform Z to break".
> 
> 
> * - I'm not saying that I want more hardware; I've been out of physical
> space for new hardware for some time, and the few extra platforms I've
> received recently are having to be stacked on top of each other - which
> isn't good when they're bare boards.  It would be nice, however, if the
> hardware I did have was fully functional!
> 

The generic MSI domain (which is what asm-generic/msi.h is about -
nothing GIC specific about it) is entirely isolated from the rest of the
MSI infrastructure used on 32bit ARM, and is very unlikely to introduce
any regression.

If there is any, I will fix it, as I am (slowly) converting existing MSI
controllers to the new infrastructure. As for GICv2m on 32bit, this is
purely a virtualization-specific thing. No need for actual HW, as a
recent version of QEMU should do the trick (I don't think anyone has
built a 32bit system with GICv2m).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

PING: [PATCH v4 TRIVIAL 1/2] ARM: Enable GICv2m on 32-bit virt machine

[Russell King - ARM Linux]

On Wed, Oct 07, 2015 at 01:43:30PM +0100, Marc Zyngier wrote:
> On 07/10/15 12:29, Russell King - ARM Linux wrote:
> > On Wed, Oct 07, 2015 at 02:02:23PM +0300, Pavel Fedin wrote:
> >>  2/2 from this series has been ACKed and picked up by Marc Zynger. He
> >> told me that 1/2 is your area of competence.
> > 
> > ... which is kind of funny, because all of ARM Ltd's platforms that
> > have PCIe on the board, and thus might have MSI, the PCIe is non-
> > functional.  So, until SolidRun sent me an Armada 388 platform a few
> > weeks back, I had zero MSI platforms, and zero MSI knowledge.
> > 
> > Just because code falls into a particular area of the tree does not
> > mean that the person who's looking after that area knows everything
> > inside out - especially when it comes to architecture maintanence
> > and the code in that subtree covers virtually every technology out
> > there, and the maintainer is kept functional hardware-starved [*].
> > 
> > Now, the Armada 388 doesn't use the GIC for MSI, but uses its own
> > interrupt controller.  I haven't yet worked out which bits of the
> > kernel are being used to provide this functionality yet, but I know
> > it works.  What I don't want to do is to merge your patch and then
> > have my one and only MSI platform break because of it, so what I'm
> > looking for is an assurance that this has had some exposure on
> > non-GIC MSI platforms, or has been reviewed with this in mind.
> > 
> > We're having far too many regressions of the kind of "let's get X
> > working on platform Y which then causes platform Z to break".
> > 
> > 
> > * - I'm not saying that I want more hardware; I've been out of physical
> > space for new hardware for some time, and the few extra platforms I've
> > received recently are having to be stacked on top of each other - which
> > isn't good when they're bare boards.  It would be nice, however, if the
> > hardware I did have was fully functional!
> > 
> 
> The generic MSI domain (which is what asm-generic/msi.h is about -
> nothing GIC specific about it) is entirely isolated from the rest of the
> MSI infrastructure used on 32bit ARM, and is very unlikely to introduce
> any regression.

That's good to hear.  Okay, let's get this patch in the patch system then,
and I'll see whether it has any effect on Armada 388.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.