linux-kernel - Re: [PATCH v3 1/2] PCI: don't rely on of_platform

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: <20240827140942.mujjwea34iscqzmx@thinkpad>
Date: Tue, 27 Aug 2024 19:39:42 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Krishna chaitanya chundru <quic_krichai@...cinc.com>,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v3 1/2] PCI: don't rely on of_platform_depopulate() for
 reused OF-nodes

On Tue, Aug 27, 2024 at 02:25:58PM +0200, Bartosz Golaszewski wrote:
> On Tue, Aug 27, 2024 at 10:40 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@...aro.org> wrote:
> >
> > + Rob
> >
> > On Fri, Aug 23, 2024 at 11:33:22AM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> > >
> > > of_platform_depopulate() doesn't play nice with reused OF nodes - it
> > > ignores the ones that are not marked explicitly as populated and it may
> > > happen that the PCI device goes away before the platform device in which
> > > case the PCI core clears the OF_POPULATED bit. We need to
> > > unconditionally unregister the platform devices for child nodes when
> > > stopping the PCI device.
> > >
> >
> > It sounds like the fix is in of_platform_depopulate() itself and this patch
> > works around the API issue in PCI driver.
> >
> > Rob, is that correct?
> >
> > - Mani
> 
> of_platform_depopulate() has more issues than just that. For one: it's
> asymmetric to of_platform_populate() as it takes a struct device as
> argument and not a device node. This causes issues for users like TI
> aemif that call of_platform_populate() on nodes without the compatible
> property that are never consumed by any device. AFAIK there's
> currently no way to depopulate them.
> 

Oouch!

> In this particular case I think that the OF_POPULATED bit should not
> be set when the PCI device is created but only when the platform
> device is.
> 
> However I'm afraid to change the semantics of of_platform_depopulate()
> et al for all users so I'm more inclined to have this fix in v6.11 to
> avoid releasing non functional code (pwrctl devices not being removed)
> and then possibly introduce a new variant of of_platform_depopulate()
> that would work slightly differently.
> 

Ok, sounds like a plan. Since Rob is also in favor of this patch, it is good to
get this series merged for 6.11.

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>

- Mani

-- 
மணிவண்ணன் சதாசிவம்