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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/19/19 11:23 AM, Cornelia Huck wrote:
> On Tue, 19 Nov 2019 09:13:11 +0100
> Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
> 
>> On 11/18/19 6:38 PM, Cornelia Huck wrote:
>>> On Thu, 24 Oct 2019 07:40:58 -0400
>>> Janosch Frank <frankja@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>>>> ---
>>>>  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 {  
>>>   
>>
>>
> 


Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux