Re: [Qemu-devel] [RFCv2 10/12] pseries: Move CPU compatibility property to machine

[Greg Kurz <groug@kaod.org>]

Hi David,

I've started to go through the testing you suggest in the cover letter. QEMU
crashes when trying to use the deprecated "compat" CPU property.

On Wed, 16 Nov 2016 09:17:53 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Server class POWER CPUs have a "compat" property, which is used to set the
> backwards compatibility mode for the processor.  However, this only makes
> sense for machine types which don't give the guest access to hypervisor
> privilege - otherwise the compatibility level is under the guest's control.
> 
> To reflect this, this removes the CPU 'compat' property and instead
> creates a 'max-cpu-compat' property on the pseries machine.  Strictly
> speaking this breaks compatibility, but AFAIK the 'compat' option was
> never (directly) used with -device or device_add.
> 
> The option was used with -cpu.  So, to maintain compatibility, this patch
> adds a hack to the cpu option parsing to strip out any compat options
> supplied with -cpu and set them on the machine property instead of the new
> removed cpu property.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
[...]
> +static void getset_compat_deprecated(Object *obj, Visitor *v, const char *name,
> +                                     void *opaque, Error **errp)
>  {
> -    Error *error = NULL;
> -    char *value = NULL;
> -    Property *prop = opaque;
> -    uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
> -
> -    visit_type_str(v, name, &value, &error);
> -    if (error) {
> -        error_propagate(errp, error);
> -        return;
> -    }
> -
> -    if (strcmp(value, "power6") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_05;
> -    } else if (strcmp(value, "power7") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_06;
> -    } else if (strcmp(value, "power8") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_07;
> -    } else {
> -        error_setg(errp, "Invalid compatibility mode \"%s\"", value);
> -    }
> -
> -    g_free(value);
> +    error_report("CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead");
> +    visit_type_null(v, name, errp);

The string input and output visitors don't support explicit null yet. I have
a tentative patch for that.

Cheers.

--
Greg

>  }
>  
> -static PropertyInfo powerpc_compat_propinfo = {
> +static PropertyInfo ppc_compat_deprecated_propinfo = {
>      .name = "str",
> -    .description = "compatibility mode, power6/power7/power8",
> -    .get = powerpc_get_compat,
> -    .set = powerpc_set_compat,
> +    .description = "compatibility mode (deprecated)",
> +    .get = getset_compat_deprecated,
> +    .set = getset_compat_deprecated,
>  };
> -
> -#define DEFINE_PROP_POWERPC_COMPAT(_n, _s, _f) \
> -    DEFINE_PROP(_n, _s, _f, powerpc_compat_propinfo, uint32_t)
> -
>  static Property powerpc_servercpu_properties[] = {
> -    DEFINE_PROP_POWERPC_COMPAT("compat", PowerPCCPU, max_compat),
> +    {
> +        .name = "compat",
> +        .info = &ppc_compat_deprecated_propinfo,
> +    },
>      DEFINE_PROP_END_OF_LIST(),
>  };
>