1. Patchew
  2. QEMU
  3. [PATCH v2 00/13] s390x: Protected Virtualization support
  4. View patch

[PATCH v2 05/13] s390x: protvirt: Add pv state to cpu env

Janosch Frank posted 13 patches 6 years, 2 months ago
There is a newer version of this series
[PATCH v2 05/13] s390x: protvirt: Add pv state to cpu env
Posted by Janosch Frank 6 years, 2 months ago 
We need to know if we run in pv state or not when emulating
instructions.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 2 ++
 target/s390x/cpu.h         | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index e2a302398d..6fcd695b81 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -357,6 +357,7 @@ static void s390_machine_reset(MachineState *machine)
                 s390_pv_vcpu_destroy(t);
             }
             s390_pv_vm_destroy();
+            env->pv = false;
         }
 
         qemu_devices_reset();
@@ -406,6 +407,7 @@ static void s390_machine_reset(MachineState *machine)
         s390_ipl_pv_unpack();
         /* Verify integrity */
         s390_pv_verify();
+        env->pv = true;
         s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
         break;
     default:
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index d2af13b345..43e6c286d2 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -116,6 +116,7 @@ struct CPUS390XState {
 
     /* Fields up to this point are cleared by a CPU reset */
     struct {} end_reset_fields;
+    bool pv; /* protected virtualization */
 
 #if !defined(CONFIG_USER_ONLY)
     uint32_t core_id; /* PoP "CPU address", same as cpu_index */
-- 
2.20.1
Re: [PATCH v2 05/13] s390x: protvirt: Add pv state to cpu env
Posted by David Hildenbrand 6 years, 2 months ago 
On 29.11.19 10:48, Janosch Frank wrote:
> We need to know if we run in pv state or not when emulating
> instructions.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 2 ++
>  target/s390x/cpu.h         | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index e2a302398d..6fcd695b81 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -357,6 +357,7 @@ static void s390_machine_reset(MachineState *machine)
>                  s390_pv_vcpu_destroy(t);
>              }
>              s390_pv_vm_destroy();
> +            env->pv = false;
>          }
>  
>          qemu_devices_reset();
> @@ -406,6 +407,7 @@ static void s390_machine_reset(MachineState *machine)
>          s390_ipl_pv_unpack();
>          /* Verify integrity */
>          s390_pv_verify();
> +        env->pv = true;
>          s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>          break;
>      default:
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index d2af13b345..43e6c286d2 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -116,6 +116,7 @@ struct CPUS390XState {
>  
>      /* Fields up to this point are cleared by a CPU reset */
>      struct {} end_reset_fields;
> +    bool pv; /* protected virtualization */

so ... the preceding patches fail to compile?

Please properly squash that ...

-- 
Thanks,

David / dhildenb
Re: [PATCH v2 05/13] s390x: protvirt: Add pv state to cpu env
Posted by Janosch Frank 6 years, 2 months ago 
On 11/29/19 11:30 AM, David Hildenbrand wrote:
> On 29.11.19 10:48, Janosch Frank wrote:
>> We need to know if we run in pv state or not when emulating
>> instructions.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 2 ++
>>  target/s390x/cpu.h         | 1 +
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index e2a302398d..6fcd695b81 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -357,6 +357,7 @@ static void s390_machine_reset(MachineState *machine)
>>                  s390_pv_vcpu_destroy(t);
>>              }
>>              s390_pv_vm_destroy();
>> +            env->pv = false;
>>          }
>>  
>>          qemu_devices_reset();
>> @@ -406,6 +407,7 @@ static void s390_machine_reset(MachineState *machine)
>>          s390_ipl_pv_unpack();
>>          /* Verify integrity */
>>          s390_pv_verify();
>> +        env->pv = true;
>>          s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>>          break;
>>      default:
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index d2af13b345..43e6c286d2 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -116,6 +116,7 @@ struct CPUS390XState {
>>  
>>      /* Fields up to this point are cleared by a CPU reset */
>>      struct {} end_reset_fields;
>> +    bool pv; /* protected virtualization */
> 
> so ... the preceding patches fail to compile?
> 
> Please properly squash that ...
> 

Sure, will do.
Re: [PATCH v2 05/13] s390x: protvirt: Add pv state to cpu env
Posted by Janosch Frank 6 years, 2 months ago 
On 11/29/19 11:30 AM, David Hildenbrand wrote:
> On 29.11.19 10:48, Janosch Frank wrote:
>> We need to know if we run in pv state or not when emulating
>> instructions.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 2 ++
>>  target/s390x/cpu.h         | 1 +
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index e2a302398d..6fcd695b81 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -357,6 +357,7 @@ static void s390_machine_reset(MachineState *machine)
>>                  s390_pv_vcpu_destroy(t);
>>              }
>>              s390_pv_vm_destroy();
>> +            env->pv = false;
>>          }
>>  
>>          qemu_devices_reset();
>> @@ -406,6 +407,7 @@ static void s390_machine_reset(MachineState *machine)
>>          s390_ipl_pv_unpack();
>>          /* Verify integrity */
>>          s390_pv_verify();
>> +        env->pv = true;
>>          s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>>          break;
>>      default:
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index d2af13b345..43e6c286d2 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -116,6 +116,7 @@ struct CPUS390XState {
>>  
>>      /* Fields up to this point are cleared by a CPU reset */
>>      struct {} end_reset_fields;
>> +    bool pv; /* protected virtualization */
> 
> so ... the preceding patches fail to compile?
> 
> Please properly squash that ...
> 

As it turns out, this patch doesn't really support multiple cpus, since
we only mark the ipl cpu as protected. We also need to mark all others
(we can do that in the create cpu call) and have a machine flag, so we
also mark hotplug cpus.

I need to think about that a bit more.
Re: [PATCH v2 05/13] s390x: protvirt: Add pv state to cpu env
Posted by David Hildenbrand 6 years, 2 months ago 
On 06.12.19 10:50, Janosch Frank wrote:
> On 11/29/19 11:30 AM, David Hildenbrand wrote:
>> On 29.11.19 10:48, Janosch Frank wrote:
>>> We need to know if we run in pv state or not when emulating
>>> instructions.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>  hw/s390x/s390-virtio-ccw.c | 2 ++
>>>  target/s390x/cpu.h         | 1 +
>>>  2 files changed, 3 insertions(+)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index e2a302398d..6fcd695b81 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -357,6 +357,7 @@ static void s390_machine_reset(MachineState *machine)
>>>                  s390_pv_vcpu_destroy(t);
>>>              }
>>>              s390_pv_vm_destroy();
>>> +            env->pv = false;
>>>          }
>>>  
>>>          qemu_devices_reset();
>>> @@ -406,6 +407,7 @@ static void s390_machine_reset(MachineState *machine)
>>>          s390_ipl_pv_unpack();
>>>          /* Verify integrity */
>>>          s390_pv_verify();
>>> +        env->pv = true;
>>>          s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>>>          break;
>>>      default:
>>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>>> index d2af13b345..43e6c286d2 100644
>>> --- a/target/s390x/cpu.h
>>> +++ b/target/s390x/cpu.h
>>> @@ -116,6 +116,7 @@ struct CPUS390XState {
>>>  
>>>      /* Fields up to this point are cleared by a CPU reset */
>>>      struct {} end_reset_fields;
>>> +    bool pv; /* protected virtualization */
>>
>> so ... the preceding patches fail to compile?
>>
>> Please properly squash that ...
>>
> 
> As it turns out, this patch doesn't really support multiple cpus, since
> we only mark the ipl cpu as protected. We also need to mark all others
> (we can do that in the create cpu call) and have a machine flag, so we
> also mark hotplug cpus.

Setting a machine state flag when enabling/disabling prot feels right.
But not sure about the performance implications when always having to
look up the machine state ... maybe you have to cache it in all CPUs, or
somewhere else ("global variable").

> 
> I need to think about that a bit more.
> 


-- 
Thanks,

David / dhildenb

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.