Re: [PATCH v5 1/8] KVM: arm64: Pass struct kvm to per-EC handlers

[Fuad Tabba <tabba@google.com>]

Hi Marc,

> > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > index 0397606c0951..7cbff0ee59a5 100644
> > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > @@ -163,7 +163,7 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu)
> >   * If FP/SIMD is not implemented, handle the trap and inject an undefined
> >   * instruction exception to the guest. Similarly for trapped SVE accesses.
> >   */
> > -static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
> > +static inline bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
>
> No, please don't do that. We already have function pointers for each
> of these, so by doing that you are forcing the compiler to emit the
> code *twice*.
>
> Instead, call into the relevant EC handler by using the base array
> that already does the non-protected handling.
>
...
> > -static const exit_handler_fn *kvm_get_exit_handler_array(void);
> > +const exit_handler_fn *kvm_get_exit_handler_array(struct kvm *kvm);
>
> Why? What breaks if when this is static? There really shouldn't be
> anything else referencing this array.

For the two points above, the reason I did that is because later
patches call these functions from the newly added
arch/arm64/kvm/hyp/nvhe/sys_regs.c. That said, I think that the code
that calls them more naturally belongs in
arch/arm64/kvm/hyp/nvhe/switch.c instead.

I'll fix that, rebase on 5.15-rc2, and respin.

Thanks,
/fuad


> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm