Re: [PATCH] hw: add compat machines for 5.1

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 27 Apr 2020 09:43:33 +0200
Auger Eric <eric.auger@redhat.com> wrote:

> Hi Connie,
> 
> On 4/24/20 11:03 AM, Cornelia Huck wrote:
> > Add 5.1 machine types for arm/i440fx/q35/s390x/spapr.
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> > 
> > Still keeping the default cpu model version on x86 at v1.
> > 
> > I'll queue this to my s390-next branch, as I'm planning to send a pull
> > req as soon as 5.0 is out; if someone else wants to queue this, we'll
> > figure it out :)
> > 
> > ---
> >  hw/arm/virt.c              |  9 ++++++++-
> >  hw/core/machine.c          |  3 +++
> >  hw/i386/pc.c               |  3 +++
> >  hw/i386/pc_piix.c          | 14 +++++++++++++-
> >  hw/i386/pc_q35.c           | 13 ++++++++++++-
> >  hw/ppc/spapr.c             | 15 +++++++++++++--
> >  hw/s390x/s390-virtio-ccw.c | 14 +++++++++++++-
> >  include/hw/boards.h        |  3 +++
> >  include/hw/i386/pc.h       |  3 +++
> >  9 files changed, 71 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 7dc96abf72cf..5e84c09402dd 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -2309,15 +2309,22 @@ static void machvirt_machine_init(void)
> >  }
> >  type_init(machvirt_machine_init);
> >  
> > +static void virt_machine_5_1_options(MachineClass *mc)
> > +{
> > +}
> > +DEFINE_VIRT_MACHINE_AS_LATEST(5, 1)
> > +
> >  static void virt_machine_5_0_options(MachineClass *mc)
> >  {
> >      static GlobalProperty compat[] = {
> >          { TYPE_TPM_TIS_SYSBUS, "ppi", "false" },
> >      };
> >  
> > +    virt_machine_5_1_options(mc);
> > +    compat_props_add(mc->compat_props, hw_compat_5_0, hw_compat_5_0_len);
> >      compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));  
> As spotted by patchew we need { TYPE_TPM_TIS_SYSBUS, "ppi", "false" } to
> be applied for all machine types 5_0 onwards as our tpm-tis-device does
> not support PPI. So I guess we need to move compat[] declaration outside
> of virt_machine_5_0_options and call
> "compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));"
> for each  virt_machine_5_*_options? Or is a misuse of compats?

Ah, that was the error (I could not really make sense of it).

So, if I understand it correctly, ppi needs to be disabled for all virt
machines. I think the correct way to do this would be to add the
"compat" prop in virt_machine_class_init() (even if it is not strictly
compat, as it always needs to be disabled). It can be removed there and
added to a compat machine should support be added in the future.

Or does anyone have a better idea?