Re: [RFC PATCH v3 4/7] bus/cdx: add cdx-MSI domain with gic-its domain as parent

From: Marc Zyngier <maz@kernel.org>
To: "Radovanovic, Aleksandar" <aleksandar.radovanovic@amd.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>,
	"Gupta, Nipun" <Nipun.Gupta@amd.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski+dt@linaro.org"
	<krzysztof.kozlowski+dt@linaro.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"Gupta, Puneet (DCG-ENG)" <puneet.gupta@amd.com>,
	"song.bao.hua@hisilicon.com" <song.bao.hua@hisilicon.com>,
	"mchehab+huawei@kernel.org" <mchehab+huawei@kernel.org>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	"jeffrey.l.hugo@gmail.com" <jeffrey.l.hugo@gmail.com>,
	"saravanak@google.com" <saravanak@google.com>,
	"Michael.Srba@seznam.cz" <Michael.Srba@seznam.cz>,
	"mani@kernel.org" <mani@kernel.org>,
	"yishaih@nvidia.com" <yishaih@nvidia.com>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"will@kernel.org" <will@kernel.org>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"masahiroy@kernel.org" <masahiroy@kernel.org>,
	"ndesaulniers@google.com" <ndesaulniers@google.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kbuild@vger.kernel.org" <linux-kbuild@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"okaya@kernel.org" <okaya@kernel.org>,
	"Anand, Harpreet" <harpreet.anand@amd.com>,
	"Agarwal, Nikhil" <nikhil.agarwal@amd.com>,
	"Simek,\ Michal" <michal.simek@amd.com>,
	"git (AMD-Xilinx)" <git@amd.com>
Subject: Re: [RFC PATCH v3 4/7] bus/cdx: add cdx-MSI domain with gic-its domain as parent
Date: Thu, 08 Sep 2022 09:08:00 +0100	[thread overview]
Message-ID: <87edwmuw4f.wl-maz@kernel.org> (raw)
In-Reply-To: <MN2PR12MB43581495197F603E901BBACA89419@MN2PR12MB4358.namprd12.prod.outlook.com>

On Wed, 07 Sep 2022 14:18:52 +0100,
"Radovanovic, Aleksandar" <aleksandar.radovanovic@amd.com> wrote:
> >
> > > As Marc mentions, CDX
> > > MSI writes are downstream of the SMMU and, if SMMU does not provide
> > > identity mapping for GITS_TRANSLATER, then we have a problem and may
> > > need to allow the OS to write the address part. However, even if we
> > > did, the CDX hardware is limited in that it can only take one
> > > GITS_TRANSLATER register target address per system, not per CDX
> > > device, nor per MSI vector.
> > 
> > If the MSI generation is downstream of the SMMU, why should the SMMU
> > provide a 1:1 mapping for GITS_TRANSLATER? I don't think it should provide a
> > mapping at all in this case. But it looks like I don't really understand how
> > these things are placed relative to each other... :-/
> > 
> 
> Apologies, I got my streams confused. It is _upstream_ of the SMMU,
> it does go through SMMU mapping.

OK, so you definitely need a mapping, but it cannot be a translation,
and it needs to be in all the possible address spaces. OMG.

> > > As for the data part (EventID in GIC parlance), this is always going
> > > to be the CDX device-relative vector number - I believe this can't be
> > > changed, it is a hardware limitation (but I need to double-check).
> > > That should be OK, though, as I believe this is exactly what Linux
> > > would write anyway, as each CDX device should be in its own IRQ domain
> > > (i.e. have its own ITS device table).
> > 
> > But that's really the worse part. You have hardcoded what is the
> > *current* Linux behaviour. Things change. And baking SW behaviour into a
> > piece of HW looks incredibly shortsighted...
> 
> For posterity, I'm not an RTL designer/architect, so share your
> sentiment to a certain extent. That said, I expect the decision was
> not based on Linux or any other SW behaviour, but because it is the
> most straightforward and least expensive way to do it. Having an
> EventID register for each and every MSI source just so you can
> program it in any random order costs flops and all the associated
> complexity of extra RTL logic (think timing closure, etc.), so
> trade-offs are made. The fact that it matches current Linux
> behaviour means we just got lucky...

Yeah, but that's not the only problem: there is no guarantee that we
have enough LPIs to allocate for the device, so we'll perform a
partial allocation (8 instead of 32 LPIs, for example).

How do you tell the device which events are valid? As far as I can
see, you can't, and I guess it will fire on all of them, resulting in
interrupts being dropped on the floor.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel