Re: [PATCH RFC 2/5] s390x: implement diag260

[Cornelia Huck]

On Fri, 10 Jul 2020 10:41:33 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 10.07.20 10:32, David Hildenbrand wrote:
> > On 09.07.20 12:37, Cornelia Huck wrote:  
> >> On Wed,  8 Jul 2020 20:51:32 +0200
> >> David Hildenbrand <david@redhat.com> wrote:

> >>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> >>> index 58dbc023eb..d7274eb320 100644
> >>> --- a/target/s390x/misc_helper.c
> >>> +++ b/target/s390x/misc_helper.c
> >>> @@ -116,6 +116,12 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, 
> >>> uint32_t r3, uint32_t num)
> >>>      uint64_t r;
> >>>  
> >>>      switch (num) {
> >>> +    case 0x260:
> >>> +        qemu_mutex_lock_iothread();
> >>> +        handle_diag_260(env, r1, r3, GETPC());
> >>> +        qemu_mutex_unlock_iothread();
> >>> +        r = 0;
> >>> +        break;
> >>>      case 0x500:
> >>>          /* KVM hypercall */
> >>>          qemu_mutex_lock_iothread();  
> >>
> >> Looking at the doc referenced above, it seems that we treat every diag
> >> call as privileged under tcg; but it seems that 0x44 isn't? (Unrelated
> >> to your patch; maybe I'm misreading.)  
> > 
> > That's also a BUG in kvm then?
> > 
> > int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
> > {
> > ...
> >     if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)
> >             return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP);
> > ...
> > }
> >   
> 
> But OTOH, it does not sound sane if user space can bypass the OS to
> yield the CPU ... so this might just be a wrong documentation. All DIAGs
> should be privileged IIRC.

Maybe not all of them, but the diag 0x44 case is indeed odd. No idea
what is documented for its use on LPAR (I don't think that document is
public.)