Re: [Qemu-devel] [PATCH 18/19] target-ppc: Enhance the CPU node labels for the guest device tree for pseries.

[David Gibson]

On Sat, Jul 06, 2013 at 11:54:15PM +1000, Alexey Kardashevskiy wrote:
> From: Prerna Saxena <address@hidden>
> 
> In absence of a -CPU parameter in the qemu command line, the nodes of
> KVM-enabled guest device tree look like this :
> 
> /proc/device-tree/cpus/address@hidden/...
> /proc/device-tree/cpus/address@hidden/...
> 
> This patch replaces this obscure 'HOST' label with a more descriptive label.
> This is gathered by first identifying the PVR of the host, and then 
> determining
> the host CPU alias which corresponds to the model indicated by this PVR.
> 
> Sample Final outcome for an KVM-enabled pseries guest running on POWER7:
> /proc/device-tree/cpus/PowerPC,address@hidden/...
> /proc/device-tree/cpus/PowerPC,address@hidden/...
> 
> This also helps userspace tools like ppc64_cpu, which expect the device tree
> to be in this format in the guest.
> 
> Signed-off-by: Prerna Saxena <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
>  hw/ppc/spapr.c              | 17 ++++++++++++++---
>  target-ppc/cpu-qom.h        |  1 +
>  target-ppc/translate_init.c | 28 ++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 26dd3f7..5ecd81b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -80,6 +80,7 @@
>  
>  #define HTAB_SIZE(spapr)        (1ULL << ((spapr)->htab_shift))
>  
> +#define PPC_DEVTREE_STR         "PowerPC,"

I thought under PowerVM, modern CPUs showed up as simply
e.g. "address@hidden" not "PowerPC,address@hidden".  Have I misremembered?


>  sPAPREnvironment *spapr;
>  
>  int spapr_allocate_irq(int hint, bool lsi)
> @@ -296,9 +297,12 @@ 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);
> +    /* device tree nodes must look like this :
> +     * PowerPC,address@hidden
> +     */
> +    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]);
>      }
>  
> @@ -1112,7 +1116,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>      MemoryRegion *sysmem = get_system_memory();
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      hwaddr rma_alloc_size;
> -    uint32_t initrd_base = 0;
> +    uint32_t initrd_base = 0, pvr = 0;
>      long kernel_size = 0, initrd_size = 0;
>      long load_limit, rtas_limit, fw_size;
>      char *filename;
> @@ -1342,6 +1346,13 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>      register_savevm_live(NULL, "spapr/htab", -1, 1,
>                           &savevm_htab_handlers, spapr);
>  
> +    /* Ensure that cpu_model is correctly reflected for a KVM guest */
> +    if (kvm_enabled() && !strcmp(cpu_model, "host")) {
> +        asm ("mfpvr %0"
> +            : "=r"(pvr));
> +        cpu_model = ppc_cpu_alias_by_pvr(pvr);

This needs to be protected by an ifdef CONFIG_KVM or similar.  If the
compiler optimization level is turned down, so that it doesn't
recognize that the kvm_enabled() is always false, then this could
attempt to compile the ppc asm instructions on an x86 (or whatever)
host.

-- 
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

pgpQam_tNpPUF.pgp
Description: PGP signature