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 <da...@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. 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. I'm also wondering if the injection of PGM_SPECIFICATION should just go into handle_diag_500 and handle_hypercall should just be inlined. > { > - CPUS390XState *env = &cpu->env; > - int ret; > - > - ret = s390_virtio_hypercall(env); > - if (ret == -EINVAL) { > + if (handle_diag_500(&cpu->env)) { > kvm_s390_program_interrupt(cpu, PGM_SPECIFICATION); > - return 0; > } > - > - return ret; > + return 0; > }