Re: [PATCH 20/21] target/arm/kvm: Unexport and tidy kvm_arm_sync_mpstate_to_{kvm, qemu}

[Peter Maydell]

On Fri, 24 Nov 2023 at 12:06, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 23/11/23 05:42, Richard Henderson wrote:
> > Drop fprintfs and actually use the return values in the callers.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   target/arm/kvm_arm.h | 20 --------------------
> >   target/arm/kvm.c     | 23 ++++++-----------------
> >   2 files changed, 6 insertions(+), 37 deletions(-)
>
>
> >   /*
> >    * Sync the KVM MP_STATE into QEMU
> >    */
> > -int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
> > +static int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
> >   {
> >       if (cap_has_mp_state) {
> >           struct kvm_mp_state mp_state;
> >           int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
> >           if (ret) {
> > -            fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
> > -                    __func__, ret, strerror(-ret));
> > -            abort();
>
> I suppose if this abort() had fired, we'd have reworked that code...
> Maybe mention its removal? Otherwise,

Well, it's a "KVM has failed in a way that's fatal for the VM"
kind of error. It's OK to drop the abort() here because since
7191f24c7fcf we will catch error returns from these arch-specific
functions in the accel/kvm generic code. When this was written
before that commit then if we didn't detect and print something
here we'd just have silently dropped the error, I think.

I added a brief note to the commit message to that effect.

-- PMM