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

[Radovanovic, Aleksandar]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Marc Zyngier <maz at kernel.org>
> Sent: 07 September 2022 13:33
> To: Radovanovic, Aleksandar <aleksandar.radovanovic at amd.com>
> Cc: Jason Gunthorpe <jgg at nvidia.com>; Gupta, Nipun
> <Nipun.Gupta at amd.com>; robh+dt at kernel.org;
> krzysztof.kozlowski+dt at linaro.org; gregkh at linuxfoundation.org;
> rafael at kernel.org; eric.auger at redhat.com; alex.williamson at redhat.com;
> cohuck at redhat.com; Gupta, Puneet (DCG-ENG)
> <puneet.gupta at amd.com>; song.bao.hua at hisilicon.com;
> mchehab+huawei at kernel.org; f.fainelli at gmail.com;
> jeffrey.l.hugo at gmail.com; saravanak at google.com;
> Michael.Srba at seznam.cz; mani at kernel.org; yishaih at nvidia.com;
> robin.murphy at arm.com; will at kernel.org; joro at 8bytes.org;
> masahiroy at kernel.org; ndesaulniers at google.com; linux-arm-
> kernel at lists.infradead.org; linux-kbuild at vger.kernel.org; linux-
> kernel at vger.kernel.org; devicetree at vger.kernel.org; kvm at vger.kernel.org;
> okaya at kernel.org; Anand, Harpreet <harpreet.anand at amd.com>; Agarwal,
> Nikhil <nikhil.agarwal at amd.com>; Simek, Michal <michal.simek at amd.com>;
> git (AMD-Xilinx) <git at amd.com>
> Subject: Re: [RFC PATCH v3 4/7] bus/cdx: add cdx-MSI domain with gic-its
> domain as parent
> 
> [CAUTION: External Email]
> 
> > 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.

> >
> > 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... 

Anyway, I'm straying off topic here, I'll check with the system architects if there's anything that can be done here. But I'm not feeling hopeful.

Aleksandar