On Tue, May 02, 2017 at 04:24:55PM +0200, Greg Kurz wrote:
> On Thu, 27 Apr 2017 17:28:41 +1000
> David Gibson <da...@gibson.dropbear.id.au> wrote:
[snip]
> > @@ -45,18 +48,21 @@ static const CompatInfo compat_table[] = {
> > .max_threads = 2,
> > },
> > { /* POWER7, ISA2.06 */
> > + .name = "power7",
> > .pvr = CPU_POWERPC_LOGICAL_2_06,
> > .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
> > PCR_TM_DIS,
> > .pcr_level = PCR_COMPAT_2_06,
> > .max_threads = 4,
> > },
> > {
> > + .name = "power7+",
> > .pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
> > .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
> > PCR_TM_DIS,
> > .pcr_level = PCR_COMPAT_2_06,
> > .max_threads = 4,
> > },
> > { /* POWER8, ISA2.07 */
> > + .name = "power8",
> > .pvr = CPU_POWERPC_LOGICAL_2_07,
> > .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07,
> > .pcr_level = PCR_COMPAT_2_07,
>
> And now we also have POWER9 in the list, so:
>
> .max_threads = 8,
> },
> { /* POWER9, ISA3.00 */
> + .name = "power9",
> .pvr = CPU_POWERPC_LOGICAL_3_00,
> .pcr = PCR_COMPAT_3_00,
> .pcr_level = PCR_COMPAT_3_00,
Updated for the next spin.
> > @@ -189,3 +195,62 @@ int ppc_compat_max_threads(PowerPCCPU *cpu)
> >
> > return n_threads;
> > }
> > +
> > +void ppc_compat_prop_get(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + uint32_t compat_pvr = *((uint32_t *)opaque);
> > + const char *value;
> > +
> > + if (!compat_pvr) {
> > + value = "";
> > + } else {
> > + const CompatInfo *compat = compat_by_pvr(compat_pvr);
> > +
> > + g_assert(compat);
> > +
> > + value = compat->name;
> > + }
> > +
> > + visit_type_str(v, name, (char **)&value, errp);
> > +}
> > +
> > +void ppc_compat_prop_set(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + Error *error = NULL;
> > + char *value;
> > + uint32_t compat_pvr;
> > +
> > + visit_type_str(v, name, &value, &error);
> > + if (error) {
> > + error_propagate(errp, error);
> > + return;
> > + }
> > +
> > + if (strcmp(value, "") == 0) {
> > + compat_pvr = 0;
>
> The current implementation in powerpc_get_compat() considers "" to be an
> invalid compatibility mode. Is there a reason to behave differently with
> max-cpu-compat ?
Hrm. Symmetry, really. In ppc_compat_prop_get() we represent no
compatibility mode set as an empty string. Setting the same value
back should have the corresponding effect.
[snip]
> > +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);
> > }
> >
>
> As suggested in another mail, maybe NULL should be passed instead of errp if
> we really want to implement the "has no effect". Otherwise, it has the effect
> of terminating QEMU when passing a compat prop that isn't "".
Good point. I'll change that.
--
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
