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
