Re: [RFT PATCH v3 12/27] of/address: Add infrastructure to declare MMIO as non-posted

[Hector Martin]

On 11/03/2021 18.12, Arnd Bergmann wrote:
> On Wed, Mar 10, 2021 at 6:01 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> > On Wed, Mar 10, 2021 at 1:27 AM Hector Martin <marcan@xxxxxxxxx> wrote:
> > > On 10/03/2021 07.06, Rob Herring wrote:
> > > > > My main concern here is that this creates an inconsistency in the device
> > > > > tree representation that only works because PCI drivers happen not to
> > > > > use these code paths. Logically, having "nonposted-mmio" above the PCI
> > > > > controller would imply that it applies to that bus too. Sure, it doesn't
> > > > > matter for Linux since it is ignored, but this creates an implicit
> > > > > exception that PCI buses always use posted modes.
> > > >
> > > > We could be stricter that "nonposted-mmio" must be in the immediate
> > > > parent. That's kind of in line with how addressing already works.
> > > > Every level has to have 'ranges' to be an MMIO address, and the
> > > > address cell size is set by the immediate parent.
> > > >
> > > > > Then if a device comes along that due to some twisted fabric logic needs
> > > > > nonposted nGnRnE mappings for PCIe (even though the actual PCIe ops will
> > > > > end up posted at the bus anyway)... how do we represent that? Declare
> > > > > that another "nonposted-mmio" on the PCIe bus means "no, really, use
> > > > > nonposted mmio for this"?
> > > >
> > > > If we're strict, yes. The PCI host bridge would have to have "nonposted-mmio".
> > >
> > > Works for me; then let's just make it non-recursive.
> > >
> > > Do you think we can get rid of the Apple-only optimization if we do
> > > this? It would mean only looking at the parent during address
> > > resolution, not recursing all the way to the top, so presumably the
> > > performance impact would be quite minimal.
>
> Works for me.

Incidentally, even though it would now be unused, I'd like to keep the 
apple,arm-platform compatible at this point; we've already been pretty 
close to a use case for it, and I don't want to have to fall back to a 
list of SoC compatibles if we ever need another quirk for all Apple ARM 
SoCs (or break backwards compat). It doesn't really hurt to have it in 
the binding and devicetrees, right?

> > Yeah, that should be fine. I'd keep an IS_ENABLED() config check
> > though. Then I'll also know if anyone else needs this.
>
> Ok, makes sense.
>
> Conceptually, I'd like to then see a check that verifies that the
> property is only set for nodes whose parent also has it set, since
> that is how AXI defines it: A bus can wait for the ack from its
> child node, or it can acknowledge the write to its parent early.
> However, this breaks down as soon as a bus does the early ack:
> all its children by definition use posted writes (as seen by the
> CPU), even if they wait for stores that come from other masters.
>
> Does this make sense to you?

Makes sense. This shouldn't really be something the kernel concerns 
itself with at runtime, just something for the dts linting, right?

I assume this isn't representable in json-schema, so it would presumably 
need some ad-hoc validation code.

--
Hector Martin (marcan@xxxxxxxxx)
Public Key: https://mrcn.st/pub