Re: [PATCH 03/13] powerpc/rtas: avoid device tree lookups in rtas_os_term()

From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Nathan Lynch" <nathanl@linux.ibm.com>, <linuxppc-dev@lists.ozlabs.org>
Cc: tyreld@linux.ibm.com, nnac123@linux.ibm.com,
	ldufour@linux.ibm.com, ajd@linux.ibm.com
Subject: Re: [PATCH 03/13] powerpc/rtas: avoid device tree lookups in rtas_os_term()
Date: Tue, 29 Nov 2022 16:45:32 +1000	[thread overview]
Message-ID: <COOKSD5PYV9N.3AHJPTBH1S081@bobo> (raw)
In-Reply-To: <875yez9bq5.fsf@linux.ibm.com>

On Tue Nov 29, 2022 at 4:26 AM AEST, Nathan Lynch wrote:
> "Nicholas Piggin" <npiggin@gmail.com> writes:
> > On Sat Nov 19, 2022 at 1:07 AM AEST, Nathan Lynch wrote:
> >> rtas_os_term() is called during panic. Its behavior depends on a
> >> couple of conditions in the /rtas node of the device tree, the
> >> traversal of which entails locking and local IRQ state changes. If the
> >> kernel panics while devtree_lock is held, rtas_os_term() as currently
> >> written could hang.
> >
> > Nice.
> >
> >>
> >> Instead of discovering the relevant characteristics at panic time,
> >> cache them in file-static variables at boot. Note the lookup for
> >> "ibm,extended-os-term" is converted to of_property_read_bool() since
> >> it is a boolean property, not a RTAS function token.
> >
> > Small nit, but you could do that at the query site unless you
> > were going to start using ibm,os-term without the extended
> > capability.
>
> I'm unsure that this is what you're suggesting, but we don't want to use
> of_property_read_bool() in this context either, because it has the same
> undesirable qualities as rtas_token().

I mean rtas_initialize() could do

	if (of_property_read_bool(rtas.dev, "ibm,extended-os-term"))
		ibm_os_term_token = rtas_token("ibm,os-term");

Thanks,
Nick