Re: [PATCH v19 070/130] KVM: TDX: TDP MMU TDX support

Edgecombe, Rick P
2024-05-31T22:17:04Z

On Wed, 2024-05-29 at 15:56 +0800, Chen Yu wrote:
> > > You don't like IS_ENABLED(CONFIG_HYPERV) || IS_ENABLED(CONFIG_TDX_HOST) in
> > > many
> > > places?  Then, we can do something like the followings.  Although It would
> > > be
> > > a bit ugly than the commit of 0277022a77a5, it's better to keep the
> > > intention
> > > of it.
> > >
> >
> > Ah, we have already __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS_RANGE. We can use it.
>
> Yes, and there is also __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS so we can use
> ifdef __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS_RANGE when needed.

In the current dev branch we already have:
#if IS_ENABLED(CONFIG_HYPERV) || IS_ENABLED(CONFIG_INTEL_TDX_HOST)
int (*flush_remote_tlbs)(struct kvm *kvm);
#endif

It was apparently reported by Vijay Dhanraj also.

0277022a77a5 has:
Declare the kvm_x86_ops hooks used to wire up paravirt TLB flushes when
running under Hyper-V if and only if CONFIG_HYPERV!=n. Wrapping yet more
code with IS_ENABLED(CONFIG_HYPERV) eliminates a handful of conditional
branches, and makes it super obvious why the hooks *might* be valid.

So the bug should already be fixed, but if anyone has a strong style opinion
please send a fixup. None of the options jump out to me as obviously better.