On Tue, Dec 01, 2015 at 11:49:31AM -0800, Sukadev Bhattiprolu wrote:
> David Gibson [da...@gibson.dropbear.id.au] wrote:
> | > @@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU
> *cpu,
> | > target_ulong ret = RTAS_OUT_SUCCESS;
> | >
> | > switch (parameter) {
> | > + case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
> | > + struct sPAPRRTASModuleInfo modinfo;
> | > + int i, size = sizeof(modinfo), offset = 0;
> | > +
> | > + memset(&modinfo, 0, size);
> | > + if (kvmppc_rtas_get_module_info(&modinfo)) {
> | > + ret = RTAS_OUT_HW_ERROR;
> | > + break;
> | > + }
> | > +
> | > + stw_be_phys(&address_space_memory, buffer+offset, size);
> |
> | You're still advertising the full structure size to the guest, even
> | though it may be only partially populated.
> |
> | That will probably work in practice, but I think we should be
> | PAPRishly correct and only output the size that we actually use here.
>
> Ok. Will have kvmppc_rtas_get_module_info() take/update a size parameter
> and use that here.
>
> |
>
> <snip>
>
> | > +/* Each core in the system is represented by a directory with the prefix
> | > + * 'PowerPC,POWER' in directory /proc/device-tree/cpus/. Process that
> | > + * directory and count the number of cores in the system.
> | > + *
> | > + * Return 0 if one or more cores are found. Return -1 otherwise.
> | > + */
> | > +static int kvmppc_count_cores_dt(int *num_cores)
> | > +{
> | > + int rc;
> | > + glob_t dtglob;
> | > + const char *cpus_pattern = "/proc/device-tree/cpus/PowerPC,POWER*";
> |
> | Under KVM PR, this could still be too specific to IBM machines. I
> | think it's probably safer to just use /proc/device-tree/cpus/*, I
> | don't *think* we get anything under /cpus that isn't a cpu node.
>
> Well, on my Tuleta system (3.18.22-355.el7_1.pkvm3_1_0.3700.3.ppc64le)
> I see several l2-cache, l3-cache entries as well as some properties
> (like phandle, #size-cells) besides the PowerPC,POWER* entries.
>
> $ cd /proc/device-tree/cpus
>
> $ lsprop l3-cache@30000020/device_type
> l3-cache@30000020/device_type
> "cache"
>
> $ lsprop l2-cache@200008f0/device_type
> l2-cache@200008f0/device_type
> "cache"
>
> $ lsprop PowerPC,POWER8@860/device_type
> PowerPC,POWER8@860/device_type
> "cpu"
Ah.. right, guess I was wrong.
> Should we walk the /proc/device-tree/cpus/ tree and count only dirs with
> device-type "cpu" (rather than relying on the pattern PowerPC,POWER*)?
Yes, I think you'll have to.
> |
> | In a number of ways I'd actually prefer to move to /cpus/cpu@NNN in
> | general, since that follows the OF generic names recommendation we
> | follow for most other nodes.
>
> Do you mean rename '/proc/device-tree/cpus/PowerPC,POWER8@NNN' to
> /proc/device-tree/cpus/cpu@NNN?
Yes. This is a firmware matter, so it's not something that can simply
be changed everywhere, but it's the approach that I'd prefer to
encourage for people making future machines and firmwares.
--
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
signature.asc
Description: PGP signature
