Re: [PATCH v1 03/14] s390x/s390-virtio-hcall: prepare for more diag500 hypercalls

[David Hildenbrand]

On 17.09.24 12:45, David Hildenbrand wrote:
> On 12.09.24 15:22, Nina Schoetterl-Glausch wrote:
> > On Tue, 2024-09-10 at 19:57 +0200, David Hildenbrand wrote:
> > > Let's generalize, abstracting the virtio bits. diag500 is now a generic
> > > hypercall to handle QEMU/KVM specific things. Explicitly specify all
> > > already defined subcodes, including legacy ones (so we know what we can
> > > use for new hypercalls).
> > >
> > > We'll rename the files separately, so git properly detects the rename.
> > >
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> > > ---
> > >    hw/s390x/s390-virtio-hcall.c   |  8 ++++----
> > >    hw/s390x/s390-virtio-hcall.h   | 11 ++++++-----
> > >    target/s390x/kvm/kvm.c         | 10 ++--------
> > >    target/s390x/tcg/misc_helper.c |  4 ++--
> > >    4 files changed, 14 insertions(+), 19 deletions(-)
> > [...]
> > > diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> > > index 94181d9281..ac292b184a 100644
> > > --- a/target/s390x/kvm/kvm.c
> > > +++ b/target/s390x/kvm/kvm.c
> > > @@ -1493,16 +1493,10 @@ static int handle_e3(S390CPU *cpu, struct kvm_run *run, 
> > > uint8_t ipbl)
> > >    
> > >    static int handle_hypercall(S390CPU *cpu, struct kvm_run *run)
> >
> > Might as well make the return type void then.
>
> Agreed.
>
> > More importantly, are you changing the behavior?
> > If I'm reading the code right, previously handle_instruction would inject an
> > additional PGM_OPERATION due to the negative return value, which does seem off 
> > to me.
> > If so, IMO this change should go into a separate patch.
>
> You are right, agreed.

Ah, reading again, we have a "return 0;" after the 
"kvm_s390_program_interrupt", so it should work as expected.

--
Cheers,

David / dhildenb