Re: [PATCH 2/4] KVM: x86/mmu: Remove redundant is_tdp_mmu_enabled check

[David Matlack <dmatlack@google.com>]

On Fri, Jun 18, 2021 at 12:42:56PM +0200, Paolo Bonzini wrote:
> On 18/06/21 09:17, kernel test robot wrote:
> > Hi David,
> > 
> > Thank you for the patch! Yet something to improve:
> > 
> > [auto build test ERROR on kvm/queue]
> > [also build test ERROR on vhost/linux-next v5.13-rc6]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> > 
> > url:https://github.com/0day-ci/linux/commits/David-Matlack/KVM-x86-mmu-Remove-redundant-is_tdp_mmu_root-check/20210618-082018
> > base:https://git.kernel.org/pub/scm/virt/kvm/kvm.git  queue
> > config: i386-randconfig-a016-20210618 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> > reproduce (this is a W=1 build):
> >          #https://github.com/0day-ci/linux/commit/6ab060f3cf9061da492b1eb89808eb2da5406781
> >          git remote add linux-reviewhttps://github.com/0day-ci/linux
> >          git fetch --no-tags linux-review David-Matlack/KVM-x86-mmu-Remove-redundant-is_tdp_mmu_root-check/20210618-082018
> >          git checkout 6ab060f3cf9061da492b1eb89808eb2da5406781
> >          # save the attached .config to linux build tree
> >          make W=1 ARCH=i386
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot<lkp@intel.com>
> > 
> > All errors (new ones prefixed by >>):
> > 
> >     ld: arch/x86/kvm/mmu/mmu.o: in function `get_mmio_spte':
> > > > arch/x86/kvm/mmu/mmu.c:3612: undefined reference to `kvm_tdp_mmu_get_walk'
> >     ld: arch/x86/kvm/mmu/mmu.o: in function `direct_page_fault':
> > > > arch/x86/kvm/mmu/mmu.c:3830: undefined reference to `kvm_tdp_mmu_map'
> 
> Turns out sometimes is_tdp_mmu_root is not inlined after this patch.
> Fixed thusly:

Thanks for the fix. I guess after I removed the is_tdp_mmu_enabled()
check the compiler couldn't determine what is_tdp_mmu_root() would
return on 32-bit builds anymore.

Pretty nice of the compiler to throw out kvm_tdp_mmu_get_walk() and
kvm_tdp_mmu_map() once it knows they are not reachable so we don't have
to implement stubs for those functions that BUG_ON and pray they never
get called!

> 
> --------- 8< -----------
> Subject: [PATCH] KVM: x86: Stub out is_tdp_mmu_root on 32-bit hosts
> 
> If is_tdp_mmu_root is not inlined, the elimination of TDP MMU calls as dead
> code might not work out.  To avoid this, explicitly declare the stubbed
> is_tdp_mmu_root on 32-bit hosts.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.h b/arch/x86/kvm/mmu/tdp_mmu.h
> index fabfea947e46..f6e0667cf4b6 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.h
> +++ b/arch/x86/kvm/mmu/tdp_mmu.h
> @@ -85,12 +85,6 @@ bool kvm_mmu_init_tdp_mmu(struct kvm *kvm);
>  void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm);
>  static inline bool is_tdp_mmu_enabled(struct kvm *kvm) { return kvm->arch.tdp_mmu_enabled; }
>  static inline bool is_tdp_mmu_page(struct kvm_mmu_page *sp) { return sp->tdp_mmu_page; }
> -#else
> -static inline bool kvm_mmu_init_tdp_mmu(struct kvm *kvm) { return false; }
> -static inline void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm) {}
> -static inline bool is_tdp_mmu_enabled(struct kvm *kvm) { return false; }
> -static inline bool is_tdp_mmu_page(struct kvm_mmu_page *sp) { return false; }
> -#endif
>  static inline bool is_tdp_mmu_root(hpa_t hpa)
>  {
> @@ -105,5 +99,12 @@ static inline bool is_tdp_mmu_root(hpa_t hpa)
>         return is_tdp_mmu_page(sp) && sp->root_count;
>  }
> +#else
> +static inline bool kvm_mmu_init_tdp_mmu(struct kvm *kvm) { return false; }
> +static inline void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm) {}
> +static inline bool is_tdp_mmu_enabled(struct kvm *kvm) { return false; }
> +static inline bool is_tdp_mmu_page(struct kvm_mmu_page *sp) { return false; }
> +static inline bool is_tdp_mmu_root(hpa_t hpa) { return false; }
> +#endif
>  #endif /* __KVM_X86_MMU_TDP_MMU_H */
>