On 08/08/2013 04:04 PM, Andreas Färber wrote:
> Am 08.08.2013 09:26, schrieb Prerna Saxena:
>>
>> From: Prerna Saxena <pre...@linux.vnet.ibm.com>
>> Date: Thu, 8 Aug 2013 06:38:03 +0530
>> Subject: [PATCH 2/2] Enhance CPU nodes of device tree to be PAPR compliant.
>>
>> This is based on patch from Andreas which enables the default CPU with KVM
>> to show up as "-cpu <type>", such as "POWER7_V2.3@0"
>>
>> While this is definitely, more descriptive, PAPR mandates the device tree CPU
>> node names to be of the form : "PowerPC,<name>" where <name> should not have
>> underscores.
>> Hence replacing the CPU model (which has underscores) with CPU alias.
>>
>> With this patch, the CPU nodes of device tree show up as :
>> /proc/device-tree/cpus/PowerPC,POWER7@0/...
>> /proc/device-tree/cpus/PowerPC,POWER7@4/...
>>
>> Signed-off-by: Prerna Saxena <pre...@linux.vnet.ibm.com>
>
> Not yet happy...
:(
>
>> ---
>> hw/ppc/spapr.c | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 59e2fea..8efd84e 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -43,6 +43,7 @@
>> #include "hw/pci-host/spapr.h"
>> #include "hw/ppc/xics.h"
>> #include "hw/pci/msi.h"
>> +#include "cpu-models.h"
>>
>> #include "hw/pci/pci.h"
>>
>> @@ -80,6 +81,8 @@
>>
>> #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift))
>>
>> +#define PPC_DEVTREE_STR "PowerPC,"
>> +
>> sPAPREnvironment *spapr;
>>
>> int spapr_allocate_irq(int hint, bool lsi)
>> @@ -322,9 +325,16 @@ static void *spapr_create_fdt_skel(const char
>> *cpu_model,
>> _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
>> _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
>>
>> - modelname = g_strdup(cpu_model);
>> + /*
>> + * PAPR convention mandates that
>> + * Device tree nodes must be named as:
>> + * PowerPC,CPU-NAME@...
>> + * Also, CPU-NAME must not have underscores.(hence use of CPU-ALIAS)
>> + */
>> +
>> + modelname = g_strdup_printf(PPC_DEVTREE_STR "%s", cpu_model);
>>
>> - for (i = 0; i < strlen(modelname); i++) {
>> + for (i = strlen(PPC_DEVTREE_STR); i < strlen(modelname); i++) {
>> modelname[i] = toupper(modelname[i]);
>> }
>>
>
> One of your colleagues had brought up that "PowerPC," prefix were not
> mandatory - is it *required* by the PAPR spec now, or is it just that
> the IBM CPUs used with PAPR happen to have such a name?
I dont know what context lead to this observation.
However, PAPR mentions the following nomenclature guideline:
"The value of this property shall be of the form: “PowerPC,<name>”,
where <name> is the name of the processor chip which may be displayed to
the user. <name> shall not contain underscores."
I think this name guideline will hold good for all PAPR compliant
processors.
>
>> @@ -1315,6 +1325,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>
>> cpu_model = g_strndup(parent_name,
>> strlen(parent_name) - strlen("-" TYPE_POWERPC_CPU));
>> +
>> + for (i = 0; ppc_cpu_aliases[i].model != NULL; i++) {
>> + if (strcmp(ppc_cpu_aliases[i].model, cpu_model) == 0) {
>> + g_free(cpu_model);
>> + cpu_model = g_strndup(ppc_cpu_aliases[i].alias,
>> + strlen(ppc_cpu_aliases[i].alias));
>> + }
>> + }
>> }
>>
>> /* Prepare the device tree */
>
> This is still fixing up the name in the wrong place: -cpu POWER7_v2.3
> will not get fixed, only -cpu host or KVM's default.
>
> The solution I had discussed with Alex is the following: When devices
> need to expose their name to firmware in a special way, we have the
> DeviceClass::fw_name field. All we have to do is assign it and use it
> instead of cpu_model if non-NULL, just like we assign DeviceClass::desc.
> The way to do it would be to extend the family of POWERPC_DEF* macros to
> specify the additional field on the relevant CPU models.
>
Would this be the same use-case as reflected by: ppc_cpu_aliases.alias ?
If so, do we really need a separate field to convey the same information ?
> Therefore my above question: Would it be sufficient to explicitly name
> POWER7_v2.3 PowerPC,POWER7 etc. and to drop the upper-casing?
> Or would we also need to name a CPU such as MPC8572E (random Freescale
> CPU where I don't know the expected fw_name and that is unlikely to
> occur/work in sPAPR) "PowerPC,MPC8572E" if someone specified it with
> -cpu MPC8572E?
>
If this is not a PAPR-compliant CPU, I dont think the PAPR naming
convention is of any good.
I havent worked with non-PAPR cpus. Is the device tree for such CPUs
generated by routines in hw/ppc/spapr.c ? Or do they have custom
routines to generate appropriate device tree nodes ?
Regards,
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
