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

[Philippe Mathieu-Daudé]

On 11/12/23 15:34, Peter Maydell wrote:
> 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.

Thank you.