On Mon, 27 Apr 2020 11:58:28 +0200
Auger Eric <eric.au...@redhat.com> wrote:
> Hi Connie,
>
> On 4/27/20 11:06 AM, Cornelia Huck wrote:
> > On Mon, 27 Apr 2020 09:43:33 +0200
> > Auger Eric <eric.au...@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 <coh...@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.
> ppi property of tpm-tis-sysbus device needs to be disabled for all virt
> machines whose version >= 5.0. tpm-tis-sysbus device is supported from
> 5.0 onwards.
It would probably be ok to set for all versions; if the device is not
available, a compat entry should not hurt.
>
> Looking again at the code, I should rather set the default value of the
> ppi property to false in tpm_tis_sysbus_properties. Before we shared the
> same device with x86, hence the compat but now we have a separate one. I
> guess that should work directly without the pain of that compat :-( Let
> me check ...
Yes, that seems like a better solution.
>
> Thanks
>
> Eric
> 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?
> >
> >
