Re: [PATCH 1/1] spapr.c: remove 'ibm,chip-id' from DT

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

On Thu, Mar 11, 2021 at 03:22:39PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 3/11/21 1:29 PM, Greg Kurz wrote:
> > On Thu, 11 Mar 2021 12:15:57 -0300
> > Daniel Henrique Barboza <danielhb413@gmail.com> wrote:
> > 
> > > The attribute 'ibm,chip-id' does not exist in PAPR. This alone would be
> > > enough reason to remove it from the spapr DT, but before doing that,
> > > let's give a brief context on how and why it was introduced.
> > > 
> > > 'ibm,chip-id' was added in the spapr DT by commit 10582ff83279. This
> > > commit references kernel commit 256f2d4b463d ("powerpc: Use ibm, chip-id
> > > property to compute cpu_core_mask if available"). In this kernel commit,
> > > the 'ibm,chip-id' DT attribute is being used to calculate the
> > > cpu_core_mask in traverse_siblings_chip_id(). This function still need
> > > to consider 'ibm,chip-id' not being available as well to avoid breaking
> > > older guests.
> > > 
> > > Later on, kernel commit df52f6714071 ("powerpc/smp: Rework CPU topology
> > > construction") removed traverse_siblings_chip_id() and its callers,
> > > making the CPU topology calculation independent of the 'ibm,chip-id'
> > > attribute. Today, the kernel uses 'ibm,chip-id' for PowerNV related code
> > > only - the pseries kernel does not rely on it.
> > > 
> > 
> > Thanks for the archaeology ! So this means that the pseries kernel used
> > to rely on it up to kernel v4.14, right ?
> 
> 
> The pseries kernel up to 4.14 used to consider the existence of 'ibm,chip-id',
> but it also had an error path for its absence as well.
> 
> > 
> > > All that said, since it's not in PAPR and the pseries kernel does not
> > > rely on it, let's remove ibm,chip-id from the DT.
> > > 
> > 
> > Even if it isn't part of PAPR, this is something that we've been
> > exposing to guests with existing machine types and someone could
> > have found a use for it or still using a pre-4.14 kernel, e.g.
> > debian stretch still relies on 4.9.
> 
> I understand that it's generally not cool to change guest visible information.
> 
> If we want to be on the safe, I can send a v2 where this change if effective only
> on pseries-6.0.0 and newer.

Yes, we should do that.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]