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)?
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@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:
pgp5TNX8VlMAV.pgp
Description: OpenPGP digital signature
