netdev - Re: [PATCH 04/11] of: address: Preserve the flags portion on 1:1 dma-ranges mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240903110953.2b1f55b6@bootlin.com>
Date: Tue, 3 Sep 2024 11:09:53 +0200
From: Herve Codina <herve.codina@...tlin.com>
To: Rob Herring <robh@...nel.org>
Cc: Andrea della Porta <andrea.porta@...e.com>, Michael Turquette
 <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Florian
 Fainelli <florian.fainelli@...adcom.com>, Broadcom internal kernel review
 list <bcm-kernel-feedback-list@...adcom.com>, Linus Walleij
 <linus.walleij@...aro.org>, Catalin Marinas <catalin.marinas@....com>, Will
 Deacon <will@...nel.org>, Derek Kiernan <derek.kiernan@....com>, Dragan
 Cvetic <dragan.cvetic@....com>, Arnd Bergmann <arnd@...db.de>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Nicolas Ferre
 <nicolas.ferre@...rochip.com>, Claudiu Beznea <claudiu.beznea@...on.dev>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
 <pabeni@...hat.com>, Saravana Kannan <saravanak@...gle.com>, Bjorn Helgaas
 <bhelgaas@...gle.com>, linux-clk@...r.kernel.org,
 devicetree@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-gpio@...r.kernel.org, netdev@...r.kernel.org,
 linux-pci@...r.kernel.org, linux-arch@...r.kernel.org, Lee Jones
 <lee@...nel.org>, Andrew Lunn <andrew@...n.ch>, Stefan Wahren
 <wahrenst@....net>, Luca Ceresoli <luca.ceresoli@...tlin.com>, Thomas
 Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 04/11] of: address: Preserve the flags portion on 1:1
 dma-ranges mapping

Hi,

On Fri, 30 Aug 2024 14:37:54 -0500
Rob Herring <robh@...nel.org> wrote:

...

> > this view is much like Bootlin's approach, also my pci-ep-bus node now would look
> > like this:
> >  ...
> >  pci-ep-bus@0 {
> >         ranges = <0xc0 0x40000000
> >                   0x01 0x00 0x00000000
> >                   0x00 0x00400000>;
> >         ...
> >  };
> >
> > and also the correct unit address here is 0 again, since the parent address in
> > ranges is 0x01 0x00 0x00000000 (0x01 is the flags and in this case represent
> > BAR1, I assume that for the unit address I should use only the address part that
> > is 0, right?).  
> 
> No, it should be 1 for BAR1. It's 1 node per BAR.

It should be 1 node per BAR but in some cases it is not.

Indeed, in the LAN966x case, the pci-ep-bus need to have access to several
BARs and we have:
	...
	pci-ep-bus@0 {
		compatible = "simple-bus";
		#address-cells = <1>;
		#size-cells = <1>;

		/*
		 * map @0xe2000000 (32MB) to BAR0 (CPU)
		 * map @0xe0000000 (16MB) to BAR1 (AMBA)
		 */
		ranges = <0xe2000000 0x00 0x00 0x00 0x2000000
		          0xe0000000 0x01 0x00 0x00 0x1000000>;
	...

Some devices under this bus need to use both BARs and use two regs values
in their reg properties to access BAR0 and BAR1.

> > > > > The assumption so far with all of this is that you have some specific
> > > > > PCI device (and therefore a driver). The simple-buses under it are
> > > > > defined per BAR. Not really certain if that makes sense in all cases,
> > > > > but since the address assignment is dynamic, it may have to. I'm also
> > > > > not completely convinced we should reuse 'simple-bus' here or define
> > > > > something specific like 'pci-bar-bus' or something.  
> > > >
> > > > Good point. Labeling a new bus for this kind of 'appliance' could be
> > > > beneficial to unify the dt overlay approach, and I guess it could be
> > > > adopted by the aforementioned Bootlin's Microchip patchset too.
> > > > However, since the difference with simple-bus would be basically non
> > > > existent, I believe that this could be done in a future patch due to
> > > > the fact that the dtbo is contained into the driver itself, so we do
> > > > not suffer from the proliferation that happens when dtb are managed
> > > > outside.  
> > >
> > > It's an ABI, so we really need to decide first.  
> >
> > Okay. How should we proceed?  
> 
> I think simple-bus where you have it is fine. It is really 1 level up
> that needs to be specified. Basically something that's referenced from
> the specific PCI device's schema (e.g. the RP1 schema (which you are
> missing)).
> 
> That schema needs to roughly look like this:
> 
> properties:
>   "#address-cells":
>     const: 3
>   "#size-cells":
>     const: 2
>   ranges:
>     minItems: 1
>     maxItems: 6
>     items:
>       additionalItems: true
>       items:
>         - maximum: 5  # The BAR number
>         - const: 0
>         - const: 0
>         - # TODO: valid PCI memory flags
> 
> patternProperties:
>   "^bar-bus@[0-5]$":
>     type: object
>     additionalProperties: true
>     properties:
>       compatible:
>         const: simple-bus
>       ranges: true
> 

IMHO, the node should not have 'bar' in the name.
In the LAN966x PCI use case, multiple BARs have to be accessed by devices
under this simple-bus. That's why I choose pci-ep-bus for this node name.

Best regards,
Hervé