[PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Eric Auger]

Instead of using a compat in the mach-virt machine to force
PPI off for all virt machines (PPI not supported by the
tpm-tis-device device), let's simply change the default value
in the sysbus device.

Best Regards

Eric

Eric Auger (2):
  tpm: tpm-tis-device: set PPI to false by default
  hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off

 hw/arm/virt.c           | 5 -----
 hw/tpm/tpm_tis_sysbus.c | 2 +-
 2 files changed, 1 insertion(+), 6 deletions(-)

-- 
2.20.1

[PATCH 1/2] tpm: tpm-tis-device: set PPI to false by default

[Eric Auger]

The tpm-tis-device device does not support PPI. Let's
change the default value for the corresponding property
instead of tricking this latter in the mach-virt machine.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/tpm/tpm_tis_sysbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index 18c02aed67..eced1fc843 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -91,7 +91,7 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
 static Property tpm_tis_sysbus_properties[] = {
     DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ),
     DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
-    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, true),
+    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.20.1

Re: [PATCH 1/2] tpm: tpm-tis-device: set PPI to false by default

[Cornelia Huck]

On Mon, 27 Apr 2020 16:31:44 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> The tpm-tis-device device does not support PPI. Let's
> change the default value for the corresponding property
> instead of tricking this latter in the mach-virt machine.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/tpm/tpm_tis_sysbus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index 18c02aed67..eced1fc843 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -91,7 +91,7 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
>  static Property tpm_tis_sysbus_properties[] = {
>      DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ),
>      DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
> -    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, true),
> +    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  

This looks like a better place to do this than in the virt compat
machines, and should get us the same result, leaving compatibility
intact.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 1/2] tpm: tpm-tis-device: set PPI to false by default

[Stefan Berger]

On 4/28/20 6:34 AM, Cornelia Huck wrote:
> On Mon, 27 Apr 2020 16:31:44 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> The tpm-tis-device device does not support PPI. Let's
>> change the default value for the corresponding property
>> instead of tricking this latter in the mach-virt machine.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>   hw/tpm/tpm_tis_sysbus.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
>> index 18c02aed67..eced1fc843 100644
>> --- a/hw/tpm/tpm_tis_sysbus.c
>> +++ b/hw/tpm/tpm_tis_sysbus.c
>> @@ -91,7 +91,7 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
>>   static Property tpm_tis_sysbus_properties[] = {
>>       DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ),
>>       DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
>> -    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, true),
>> +    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>   
> This looks like a better place to do this than in the virt compat
> machines, and should get us the same result, leaving compatibility
> intact.
>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

[PATCH 2/2] hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off

[Eric Auger]

Now that the tpm-tis-device device PPI property is off by default,
we can remove the compat used for the same goal.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/virt.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7dc96abf72..2a68306f28 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2311,11 +2311,6 @@ type_init(machvirt_machine_init);
 
 static void virt_machine_5_0_options(MachineClass *mc)
 {
-    static GlobalProperty compat[] = {
-        { TYPE_TPM_TIS_SYSBUS, "ppi", "false" },
-    };
-
-    compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
 }
 DEFINE_VIRT_MACHINE_AS_LATEST(5, 0)
 
-- 
2.20.1

Re: [PATCH 2/2] hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off

[Cornelia Huck]

On Mon, 27 Apr 2020 16:31:45 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> Now that the tpm-tis-device device PPI property is off by default,
> we can remove the compat used for the same goal.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/arm/virt.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7dc96abf72..2a68306f28 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2311,11 +2311,6 @@ type_init(machvirt_machine_init);
>  
>  static void virt_machine_5_0_options(MachineClass *mc)
>  {
> -    static GlobalProperty compat[] = {
> -        { TYPE_TPM_TIS_SYSBUS, "ppi", "false" },
> -    };
> -
> -    compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
>  }
>  DEFINE_VIRT_MACHINE_AS_LATEST(5, 0)
>  

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 2/2] hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off

[Stefan Berger]

On 4/28/20 6:36 AM, Cornelia Huck wrote:
> On Mon, 27 Apr 2020 16:31:45 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Now that the tpm-tis-device device PPI property is off by default,
>> we can remove the compat used for the same goal.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>   hw/arm/virt.c | 5 -----
>>   1 file changed, 5 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 7dc96abf72..2a68306f28 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2311,11 +2311,6 @@ type_init(machvirt_machine_init);
>>   
>>   static void virt_machine_5_0_options(MachineClass *mc)
>>   {
>> -    static GlobalProperty compat[] = {
>> -        { TYPE_TPM_TIS_SYSBUS, "ppi", "false" },
>> -    };
>> -
>> -    compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
>>   }
>>   DEFINE_VIRT_MACHINE_AS_LATEST(5, 0)
>>   
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Stefan Berger]

On 4/27/20 10:31 AM, Eric Auger wrote:
> Instead of using a compat in the mach-virt machine to force
> PPI off for all virt machines (PPI not supported by the
> tpm-tis-device device), let's simply change the default value
> in the sysbus device.

There is no change in behavior on any arm machine due to this patch, 
right? So backporting would not be necessary?


    Stefan

Re: [PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Auger Eric]

Hi Stefan,

On 4/27/20 10:21 PM, Stefan Berger wrote:
> On 4/27/20 10:31 AM, Eric Auger wrote:
>> Instead of using a compat in the mach-virt machine to force
>> PPI off for all virt machines (PPI not supported by the
>> tpm-tis-device device), let's simply change the default value
>> in the sysbus device.
> 
> There is no change in behavior on any arm machine due to this patch,
> right? So backporting would not be necessary?

Indeed, there is no functional change as we keep PPI off. My
understanding is we can safely remove the compat mechanism. I tested
migration between virt machine with compat to virt machine without
compat and conversely (without tpm-tis-device though) and it passed.

Thanks

Eric
> 
> 
>    Stefan
> 
> 

Re: [PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Cornelia Huck]

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

> Instead of using a compat in the mach-virt machine to force
> PPI off for all virt machines (PPI not supported by the
> tpm-tis-device device), let's simply change the default value
> in the sysbus device.
> 
> Best Regards
> 
> Eric
> 
> Eric Auger (2):
>   tpm: tpm-tis-device: set PPI to false by default
>   hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off
> 
>  hw/arm/virt.c           | 5 -----
>  hw/tpm/tpm_tis_sysbus.c | 2 +-
>  2 files changed, 1 insertion(+), 6 deletions(-)
> 

I think we can apply the compat machines patch on top of these two
patches.

Q: Who will queue this and the machine types patch? It feels a bit
weird taking arm patches through the s390 tree :)

Re: [PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Stefan Berger]

On 4/28/20 6:38 AM, Cornelia Huck wrote:
> On Mon, 27 Apr 2020 16:31:43 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Instead of using a compat in the mach-virt machine to force
>> PPI off for all virt machines (PPI not supported by the
>> tpm-tis-device device), let's simply change the default value
>> in the sysbus device.
>>
>> Best Regards
>>
>> Eric
>>
>> Eric Auger (2):
>>    tpm: tpm-tis-device: set PPI to false by default
>>    hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off
>>
>>   hw/arm/virt.c           | 5 -----
>>   hw/tpm/tpm_tis_sysbus.c | 2 +-
>>   2 files changed, 1 insertion(+), 6 deletions(-)
>>
> I think we can apply the compat machines patch on top of these two
> patches.
>
> Q: Who will queue this and the machine types patch? It feels a bit
> weird taking arm patches through the s390 tree :)
>
I can queue them and would send the PR soon. I am also fine with someone 
else doing it.

Re: [PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Cornelia Huck]

On Tue, 28 Apr 2020 16:13:05 -0400
Stefan Berger <stefanb@linux.ibm.com> wrote:

> On 4/28/20 6:38 AM, Cornelia Huck wrote:
> > On Mon, 27 Apr 2020 16:31:43 +0200
> > Eric Auger <eric.auger@redhat.com> wrote:
> >  
> >> Instead of using a compat in the mach-virt machine to force
> >> PPI off for all virt machines (PPI not supported by the
> >> tpm-tis-device device), let's simply change the default value
> >> in the sysbus device.
> >>
> >> Best Regards
> >>
> >> Eric
> >>
> >> Eric Auger (2):
> >>    tpm: tpm-tis-device: set PPI to false by default
> >>    hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off
> >>
> >>   hw/arm/virt.c           | 5 -----
> >>   hw/tpm/tpm_tis_sysbus.c | 2 +-
> >>   2 files changed, 1 insertion(+), 6 deletions(-)
> >>  
> > I think we can apply the compat machines patch on top of these two
> > patches.
> >
> > Q: Who will queue this and the machine types patch? It feels a bit
> > weird taking arm patches through the s390 tree :)
> >  
> I can queue them and would send the PR soon. I am also fine with someone 
> else doing it.

Would be great if you could queue these together with
https://patchew.org/QEMU/20200424090314.544-1-cohuck@redhat.com/
(hopefully should still apply cleanly, let me know if a respin is
needed).

Re: [PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Stefan Berger]

On 4/29/20 2:11 AM, Cornelia Huck wrote:
> On Tue, 28 Apr 2020 16:13:05 -0400
> Stefan Berger <stefanb@linux.ibm.com> wrote:
>
>> On 4/28/20 6:38 AM, Cornelia Huck wrote:
>>> On Mon, 27 Apr 2020 16:31:43 +0200
>>> Eric Auger <eric.auger@redhat.com> wrote:
>>>   
>>>> Instead of using a compat in the mach-virt machine to force
>>>> PPI off for all virt machines (PPI not supported by the
>>>> tpm-tis-device device), let's simply change the default value
>>>> in the sysbus device.
>>>>
>>>> Best Regards
>>>>
>>>> Eric
>>>>
>>>> Eric Auger (2):
>>>>     tpm: tpm-tis-device: set PPI to false by default
>>>>     hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off
>>>>
>>>>    hw/arm/virt.c           | 5 -----
>>>>    hw/tpm/tpm_tis_sysbus.c | 2 +-
>>>>    2 files changed, 1 insertion(+), 6 deletions(-)
>>>>   
>>> I think we can apply the compat machines patch on top of these two
>>> patches.
>>>
>>> Q: Who will queue this and the machine types patch? It feels a bit
>>> weird taking arm patches through the s390 tree :)
>>>   
>> I can queue them and would send the PR soon. I am also fine with someone
>> else doing it.
> Would be great if you could queue these together with
> https://patchew.org/QEMU/20200424090314.544-1-cohuck@redhat.com/
> (hopefully should still apply cleanly, let me know if a respin is
> needed).
>
It's probably better to respin:

 > patches apply id:20200424090314.544-1-cohuck@redhat.com
Applying: hw: add compat machines for 5.1
Using index info to reconstruct a base tree...
M    hw/arm/virt.c
Falling back to patching base and 3-way merge...
Auto-merging hw/arm/virt.c
CONFLICT (content): Merge conflict in hw/arm/virt.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 hw: add compat machines for 5.1
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [PATCH 0/2] virt: Set tpm-tis-device ppi property to off by default

[Peter Maydell]

On Mon, 27 Apr 2020 at 15:32, Eric Auger <eric.auger@redhat.com> wrote:
>
> Instead of using a compat in the mach-virt machine to force
> PPI off for all virt machines (PPI not supported by the
> tpm-tis-device device), let's simply change the default value
> in the sysbus device.
>
> Best Regards
>
> Eric
>
> Eric Auger (2):
>   tpm: tpm-tis-device: set PPI to false by default
>   hw/arm/virt: Remove the compat forcing tpm-tis-device PPI to off
>
>  hw/arm/virt.c           | 5 -----
>  hw/tpm/tpm_tis_sysbus.c | 2 +-
>  2 files changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Stefan, feel free to take these through your tree if you
think that makes sense.

thanks
-- PMM