Re: [Qemu-devel] [PATCH v2 1/1] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[Nishanth Aravamudan]

On 11.11.2015 [11:17:58 +1100], David Gibson wrote:
> On Mon, Nov 09, 2015 at 08:22:32PM -0800, Sukadev Bhattiprolu wrote:
> > David Gibson address@hidden wrote:
> > | On Wed, Nov 04, 2015 at 03:06:05PM -0800, Sukadev Bhattiprolu wrote:
> > | > Implement RTAS_SYSPARM_PROCESSOR_MODULE_INFO parameter to 
> > rtas_get_sysparm()
> > | > call in qemu. This call returns the processor module (socket), chip and 
> > core
> > | > information as specified in section 7.3.16.18 of PAPR v2.7.
> > | 
> > | PAPR v2.7 isn't available publically.  For upstream patches, please
> > | reference LoPAPR instead (where it's section 7.3.16.17 AFAICT).
> > 
> > Ok.
> > 
> > | 
> > | > We walk the /proc/device-tree to determine the number of chips, cores 
> > and
> > | > modules in the _host_ system and return this info to the guest 
> > application
> > | > that makes the rtas_get_sysparm() call.
> > | > 
> > | > We currently hard code the number of module_types to 1, but we should 
> > determine
> > | > that dynamically somehow later.
> > | > 
> > | > Thanks to input from Nishanth Aravamudan and Alexey Kardashevsk.
> > | > 
> > | > Signed-off-by: Sukadev Bhattiprolu <address@hidden>
> > | 
> > | This isn't ready to go yet - you need to put some more consideration
> > | into the uncommon cases: PR KVM, TCG and non-Power hosts.
> > 
> > Ok. Is there a we can make this code applicable only a Powerpc host?
> > (would moving this code to target-ppc/kvm.c do that?)
> 
> Yes, moving it to target-ppc/kvm.c would mostly do that.  You'd need
> some logic to make sure it fails gracefully in other cases, of course.
> 
> [snip]
> > | >      switch (parameter) {
> > | > +    case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
> > | > +        int i;
> > | > +        int offset = 0;
> > | > +        int size;
> > | > +        struct rtas_module_info modinfo;
> > | > +
> > | > +        if (rtas_get_module_info(&modinfo)) {
> > | > +            break;
> > | > +        }
> > | 
> > | So, you handle the variable size of this structure before sending it
> > | to the guest, but you don't handle it in allocation of the structure
> > | right here.  You'll get away with that because for now you only ever
> > | have one entry in the sockets array, but it's a bit icky.
> > 
> > Can we assume that the size is static for now...
> > | 
> > | > +
> > | > +        size = sizeof(modinfo);
> > | > +        size += (modinfo.module_types - 1) * sizeof(struct 
> > rtas_socket_info);
> > | 
> > | More seriously, this calculation will break horribly if you change the
> > | size of the array in struct rtas_module_info.
> > 
> > and just set 'size' to sizeof(modinfo)?.
> 
> For purposes of allocation you could just use a fixed size.  But the
> guest might get confused by additional data beyond the declared size,
> so you do need to get the value correct that you send back to the guest.
> 
> [snip]
> > | > +/*
> > | > + * Each module's (aka socket's) id is contained in the 
> > 'ibm,hw-module-id'
> > | > + * file in the "xscom" directory (/proc/device-tree/xscom*). Similarly 
> > each
> > | > + * chip's id is contained in the 'ibm,chip-id' file in the xscom 
> > directory.
> > | > + *
> > | > + * A module can contain more than one chip and a chip can contain more
> > | > + * than one core. So there are likely to be duplicates in the module
> > | > + * and chip identifiers (i.e more than one xscom directory can contain
> > | > + * the same module/chip id).
> > | > + *
> > | > + * Search the xscom directories and count the number of _UNIQUE_ module
> > | > + * and chip identifiers in the system.
> > | 
> > | There's no direct way to go from a core
> > | (i.e. /proc/device-tree/cpus/address@hidden) to the corresponding chip 
> > and/or
> > | module?
> > 
> > Yes, it would logical to find the chip and module from the core :-)
> > 
> > While 'ibm,chip-id' is in the core dir (/proc/device-tree/cpus/PowerPC,*/), 
> > the 'ibm,hw-module-id' is not there (on my Tuleta system). Maybe the
> > 'ibm,hw-module-id' will be added in the future?
> 
> Hm, I see.  Is there any device node that represents the "chip"?
> 
> > I am using the xscom node to be consistent in counting chips and modules.
> 
> The trouble with xscom is that it's extremely specific to the way the
> current IBM servers present things.  It won't work on other types of
> host machine (which could happen with PR KVM), and could even break if
> IBM changes the way it organizes the SCOMs in a future machine.
> 
> Working from the nodes in /cpus still has some dependencies on IBM
> specific properties, but it's at least partially based on OF
> standards.
> 
> There's also another possible approach here, though I don't know if it
> will work.  Instead of looking directly in the device tree, try to get
> the information from lscpu, or libosinfo.  That would at least give
> you some hope of providing meaningful information on other host types.

Heh, the issue that is underlying all of this, is that `lscpu` itself is
quite wrong.

On PAPR-compliant hypervisors (well, PowerVM, at least), the only
supported means of determining the underlying hardware CPU information
(which is what licensing models want in the end), is to use this RTAS
call in an LPAR. `lscpu` is explicitly incorrect in these environments
(it's values are "derived" from sysfs and some are adjusted to ensure
the division of values works out).

So, we are trying to at least resolve what PowerKVM guest can see by
supporting this RTAS call there. We should report *something* to the
guest, if possible, and we can adjust what is reported to the guests as
we go, from the host perspective.

I haven't followed along too closely in this thread, but woudl it be
reasonable to only report this RTAS call as being supported under KVM?
How are other RTAS calls dealt with for PR and non-IBM models currently?

-Nish