diff mbox series

[RFC,36/37] KVM: s390: protvirt: Support cmd 5 operation state

Message ID 20191024114059.102802-37-frankja@linux.ibm.com
State New, archived
Headers show
Series KVM: s390: Add support for protected VMs | expand

Commit Message

Janosch Frank Oct. 24, 2019, 11:40 a.m. UTC
Code 5 for the set cpu state UV call tells the UV to load a PSW from
the SE header (first IPL) or from guest location 0x0 (diag 308 subcode
0/1). Also it sets the cpu into operating state afterwards, so we can
start it.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/include/asm/uv.h | 1 +
 arch/s390/kvm/kvm-s390.c   | 4 ++++
 include/uapi/linux/kvm.h   | 1 +
 3 files changed, 6 insertions(+)

Comments

Thomas Huth Nov. 15, 2019, 11:25 a.m. UTC | #1
On 24/10/2019 13.40, Janosch Frank wrote:
> Code 5 for the set cpu state UV call tells the UV to load a PSW from
> the SE header (first IPL) or from guest location 0x0 (diag 308 subcode
> 0/1). Also it sets the cpu into operating state afterwards, so we can
> start it.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/include/asm/uv.h | 1 +
>  arch/s390/kvm/kvm-s390.c   | 4 ++++
>  include/uapi/linux/kvm.h   | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 33b52ba306af..8d10ae731458 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -163,6 +163,7 @@ struct uv_cb_unp {
>  #define PV_CPU_STATE_OPR	1
>  #define PV_CPU_STATE_STP	2
>  #define PV_CPU_STATE_CHKSTP	3
> +#define PV_CPU_STATE_OPR_LOAD	5
>  
>  struct uv_cb_cpu_set_state {
>  	struct uv_cb_header header;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index cc5feb67f145..5cc9108c94e4 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4652,6 +4652,10 @@ static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
>  		r = kvm_s390_pv_destroy_cpu(vcpu);
>  		break;
>  	}
> +	case KVM_PV_VCPU_SET_IPL_PSW: {
> +		r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_OPR_LOAD);
> +		break;
> +	}

Nit: No need for the curly braces here.

 Thomas
Cornelia Huck Nov. 18, 2019, 5:38 p.m. UTC | #2
On Thu, 24 Oct 2019 07:40:58 -0400
Janosch Frank <frankja@linux.ibm.com> wrote:

> Code 5 for the set cpu state UV call tells the UV to load a PSW from
> the SE header (first IPL) or from guest location 0x0 (diag 308 subcode
> 0/1). Also it sets the cpu into operating state afterwards, so we can
> start it.

I'm a bit confused by the patch description: Does this mean that the UV
does the transition to operating state? Does the hypervisor get a
notification for that?

> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/include/asm/uv.h | 1 +
>  arch/s390/kvm/kvm-s390.c   | 4 ++++
>  include/uapi/linux/kvm.h   | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 33b52ba306af..8d10ae731458 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -163,6 +163,7 @@ struct uv_cb_unp {
>  #define PV_CPU_STATE_OPR	1
>  #define PV_CPU_STATE_STP	2
>  #define PV_CPU_STATE_CHKSTP	3
> +#define PV_CPU_STATE_OPR_LOAD	5
>  
>  struct uv_cb_cpu_set_state {
>  	struct uv_cb_header header;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index cc5feb67f145..5cc9108c94e4 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4652,6 +4652,10 @@ static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
>  		r = kvm_s390_pv_destroy_cpu(vcpu);
>  		break;
>  	}
> +	case KVM_PV_VCPU_SET_IPL_PSW: {
> +		r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_OPR_LOAD);
> +		break;
> +	}
>  	default:
>  		r = -ENOTTY;
>  	}
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 2846ed5e5dd9..973007d27d55 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1483,6 +1483,7 @@ enum pv_cmd_id {
>  	KVM_PV_VM_UNSHARE,
>  	KVM_PV_VCPU_CREATE,
>  	KVM_PV_VCPU_DESTROY,
> +	KVM_PV_VCPU_SET_IPL_PSW,
>  };
>  
>  struct kvm_pv_cmd {
Janosch Frank Nov. 19, 2019, 8:13 a.m. UTC | #3
On 11/18/19 6:38 PM, Cornelia Huck wrote:
> On Thu, 24 Oct 2019 07:40:58 -0400
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Code 5 for the set cpu state UV call tells the UV to load a PSW from
>> the SE header (first IPL) or from guest location 0x0 (diag 308 subcode
>> 0/1). Also it sets the cpu into operating state afterwards, so we can
>> start it.
> 
> I'm a bit confused by the patch description: Does this mean that the UV
> does the transition to operating state? Does the hypervisor get a
> notification for that?

CMD 5 is defined as "load psw and set to operating".
Currently QEMU will still go out and do a "set to operating" after the
cmd 5 because our current infrastructure does it and it's basically a
nop, so I didn't want to put in the effort to remove it.

> 
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  arch/s390/include/asm/uv.h | 1 +
>>  arch/s390/kvm/kvm-s390.c   | 4 ++++
>>  include/uapi/linux/kvm.h   | 1 +
>>  3 files changed, 6 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
>> index 33b52ba306af..8d10ae731458 100644
>> --- a/arch/s390/include/asm/uv.h
>> +++ b/arch/s390/include/asm/uv.h
>> @@ -163,6 +163,7 @@ struct uv_cb_unp {
>>  #define PV_CPU_STATE_OPR	1
>>  #define PV_CPU_STATE_STP	2
>>  #define PV_CPU_STATE_CHKSTP	3
>> +#define PV_CPU_STATE_OPR_LOAD	5
>>  
>>  struct uv_cb_cpu_set_state {
>>  	struct uv_cb_header header;
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index cc5feb67f145..5cc9108c94e4 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -4652,6 +4652,10 @@ static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
>>  		r = kvm_s390_pv_destroy_cpu(vcpu);
>>  		break;
>>  	}
>> +	case KVM_PV_VCPU_SET_IPL_PSW: {
>> +		r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_OPR_LOAD);
>> +		break;
>> +	}
>>  	default:
>>  		r = -ENOTTY;
>>  	}
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 2846ed5e5dd9..973007d27d55 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1483,6 +1483,7 @@ enum pv_cmd_id {
>>  	KVM_PV_VM_UNSHARE,
>>  	KVM_PV_VCPU_CREATE,
>>  	KVM_PV_VCPU_DESTROY,
>> +	KVM_PV_VCPU_SET_IPL_PSW,
>>  };
>>  
>>  struct kvm_pv_cmd {
>
Cornelia Huck Nov. 19, 2019, 10:23 a.m. UTC | #4
On Tue, 19 Nov 2019 09:13:11 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 11/18/19 6:38 PM, Cornelia Huck wrote:
> > On Thu, 24 Oct 2019 07:40:58 -0400
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> Code 5 for the set cpu state UV call tells the UV to load a PSW from
> >> the SE header (first IPL) or from guest location 0x0 (diag 308 subcode
> >> 0/1). Also it sets the cpu into operating state afterwards, so we can
> >> start it.  
> > 
> > I'm a bit confused by the patch description: Does this mean that the UV
> > does the transition to operating state? Does the hypervisor get a
> > notification for that?  
> 
> CMD 5 is defined as "load psw and set to operating".
> Currently QEMU will still go out and do a "set to operating" after the
> cmd 5 because our current infrastructure does it and it's basically a
> nop, so I didn't want to put in the effort to remove it.

So, the "it" setting the cpu into operating state is QEMU, via the
mpstate interface, which triggers that call? Or is that implicit, but
it does not hurt to do it again (which would make more sense to me)?

Assuming the latter, what about the following description:

"KVM: s390: protvirt: support setting cpu state 5

Setting code 5 ("load psw and set to operating") in the set cpu state
UV call tells the UV to load a PSW either from the SE header (first
IPL) or from guest location 0x0 (diag 308 subcode 0/1). Subsequently,
the cpu is set into operating state by the UV.

Note that we can still instruct the UV to set the cpu into operating
state explicitly afterwards."

> 
> >   
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>  arch/s390/include/asm/uv.h | 1 +
> >>  arch/s390/kvm/kvm-s390.c   | 4 ++++
> >>  include/uapi/linux/kvm.h   | 1 +
> >>  3 files changed, 6 insertions(+)
> >>
> >> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> >> index 33b52ba306af..8d10ae731458 100644
> >> --- a/arch/s390/include/asm/uv.h
> >> +++ b/arch/s390/include/asm/uv.h
> >> @@ -163,6 +163,7 @@ struct uv_cb_unp {
> >>  #define PV_CPU_STATE_OPR	1
> >>  #define PV_CPU_STATE_STP	2
> >>  #define PV_CPU_STATE_CHKSTP	3
> >> +#define PV_CPU_STATE_OPR_LOAD	5
> >>  
> >>  struct uv_cb_cpu_set_state {
> >>  	struct uv_cb_header header;
> >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> >> index cc5feb67f145..5cc9108c94e4 100644
> >> --- a/arch/s390/kvm/kvm-s390.c
> >> +++ b/arch/s390/kvm/kvm-s390.c
> >> @@ -4652,6 +4652,10 @@ static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
> >>  		r = kvm_s390_pv_destroy_cpu(vcpu);
> >>  		break;
> >>  	}
> >> +	case KVM_PV_VCPU_SET_IPL_PSW: {
> >> +		r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_OPR_LOAD);

Also maybe add a comment here that setting into oper state (again) can
be done separately?

> >> +		break;
> >> +	}
> >>  	default:
> >>  		r = -ENOTTY;
> >>  	}
> >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> >> index 2846ed5e5dd9..973007d27d55 100644
> >> --- a/include/uapi/linux/kvm.h
> >> +++ b/include/uapi/linux/kvm.h
> >> @@ -1483,6 +1483,7 @@ enum pv_cmd_id {
> >>  	KVM_PV_VM_UNSHARE,
> >>  	KVM_PV_VCPU_CREATE,
> >>  	KVM_PV_VCPU_DESTROY,
> >> +	KVM_PV_VCPU_SET_IPL_PSW,
> >>  };
> >>  
> >>  struct kvm_pv_cmd {  
> >   
> 
>
Janosch Frank Nov. 19, 2019, 11:40 a.m. UTC | #5
On 11/19/19 11:23 AM, Cornelia Huck wrote:
> On Tue, 19 Nov 2019 09:13:11 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 11/18/19 6:38 PM, Cornelia Huck wrote:
>>> On Thu, 24 Oct 2019 07:40:58 -0400
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>   
>>>> Code 5 for the set cpu state UV call tells the UV to load a PSW from
>>>> the SE header (first IPL) or from guest location 0x0 (diag 308 subcode
>>>> 0/1). Also it sets the cpu into operating state afterwards, so we can
>>>> start it.  
>>>
>>> I'm a bit confused by the patch description: Does this mean that the UV
>>> does the transition to operating state? Does the hypervisor get a
>>> notification for that?  
>>
>> CMD 5 is defined as "load psw and set to operating".
>> Currently QEMU will still go out and do a "set to operating" after the
>> cmd 5 because our current infrastructure does it and it's basically a
>> nop, so I didn't want to put in the effort to remove it.
> 
> So, the "it" setting the cpu into operating state is QEMU, via the
> mpstate interface, which triggers that call? Or is that implicit, but
> it does not hurt to do it again (which would make more sense to me)?

Qemu sets operating state via mpstate.
I could fence that via env->pv checks but that would also mean more code
and setting operating twice is just a nop.

> 
> Assuming the latter, what about the following description:
> 
> "KVM: s390: protvirt: support setting cpu state 5
> 
> Setting code 5 ("load psw and set to operating") in the set cpu state
> UV call tells the UV to load a PSW either from the SE header (first
> IPL) or from guest location 0x0 (diag 308 subcode 0/1). Subsequently,
> the cpu is set into operating state by the UV.
> 
> Note that we can still instruct the UV to set the cpu into operating
> state explicitly afterwards."

Sounds good

> 
>>
>>>   
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>>  arch/s390/include/asm/uv.h | 1 +
>>>>  arch/s390/kvm/kvm-s390.c   | 4 ++++
>>>>  include/uapi/linux/kvm.h   | 1 +
>>>>  3 files changed, 6 insertions(+)
>>>>
>>>> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
>>>> index 33b52ba306af..8d10ae731458 100644
>>>> --- a/arch/s390/include/asm/uv.h
>>>> +++ b/arch/s390/include/asm/uv.h
>>>> @@ -163,6 +163,7 @@ struct uv_cb_unp {
>>>>  #define PV_CPU_STATE_OPR	1
>>>>  #define PV_CPU_STATE_STP	2
>>>>  #define PV_CPU_STATE_CHKSTP	3
>>>> +#define PV_CPU_STATE_OPR_LOAD	5
>>>>  
>>>>  struct uv_cb_cpu_set_state {
>>>>  	struct uv_cb_header header;
>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>> index cc5feb67f145..5cc9108c94e4 100644
>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>> @@ -4652,6 +4652,10 @@ static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
>>>>  		r = kvm_s390_pv_destroy_cpu(vcpu);
>>>>  		break;
>>>>  	}
>>>> +	case KVM_PV_VCPU_SET_IPL_PSW: {
>>>> +		r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_OPR_LOAD);
> 
> Also maybe add a comment here that setting into oper state (again) can
> be done separately?
> 
>>>> +		break;
>>>> +	}
>>>>  	default:
>>>>  		r = -ENOTTY;
>>>>  	}
>>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>>> index 2846ed5e5dd9..973007d27d55 100644
>>>> --- a/include/uapi/linux/kvm.h
>>>> +++ b/include/uapi/linux/kvm.h
>>>> @@ -1483,6 +1483,7 @@ enum pv_cmd_id {
>>>>  	KVM_PV_VM_UNSHARE,
>>>>  	KVM_PV_VCPU_CREATE,
>>>>  	KVM_PV_VCPU_DESTROY,
>>>> +	KVM_PV_VCPU_SET_IPL_PSW,
>>>>  };
>>>>  
>>>>  struct kvm_pv_cmd {  
>>>   
>>
>>
>
diff mbox series

Patch

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 33b52ba306af..8d10ae731458 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -163,6 +163,7 @@  struct uv_cb_unp {
 #define PV_CPU_STATE_OPR	1
 #define PV_CPU_STATE_STP	2
 #define PV_CPU_STATE_CHKSTP	3
+#define PV_CPU_STATE_OPR_LOAD	5
 
 struct uv_cb_cpu_set_state {
 	struct uv_cb_header header;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index cc5feb67f145..5cc9108c94e4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4652,6 +4652,10 @@  static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
 		r = kvm_s390_pv_destroy_cpu(vcpu);
 		break;
 	}
+	case KVM_PV_VCPU_SET_IPL_PSW: {
+		r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_OPR_LOAD);
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 2846ed5e5dd9..973007d27d55 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1483,6 +1483,7 @@  enum pv_cmd_id {
 	KVM_PV_VM_UNSHARE,
 	KVM_PV_VCPU_CREATE,
 	KVM_PV_VCPU_DESTROY,
+	KVM_PV_VCPU_SET_IPL_PSW,
 };
 
 struct kvm_pv_cmd {