From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bharata B Rao Date: Thu, 08 Jul 2021 04:41:08 +0000 Subject: Re: [PATCH v8 3/6] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE Message-Id: List-Id: References: <20210621085003.904767-1-bharata@linux.ibm.com> <20210621085003.904767-4-bharata@linux.ibm.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: David Gibson Cc: farosas@linux.ibm.com, aneesh.kumar@linux.ibm.com, npiggin@gmail.com, kvm-ppc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org On Thu, Jul 08, 2021 at 01:58:04PM +1000, David Gibson wrote: > On Tue, Jul 06, 2021 at 10:56:32AM +0530, Bharata B Rao wrote: > > On Mon, Jul 05, 2021 at 02:42:33PM +1000, David Gibson wrote: > > > On Mon, Jun 21, 2021 at 02:20:00PM +0530, Bharata B Rao wrote: > > > > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h > > > > index 4bc45d3ed8b0..b44f291fc909 100644 > > > > --- a/arch/powerpc/include/asm/mmu_context.h > > > > +++ b/arch/powerpc/include/asm/mmu_context.h > > > > @@ -124,8 +124,17 @@ static inline bool need_extra_context(struct mm_struct *mm, unsigned long ea) > > > > > > > > #if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_RADIX_MMU) > > > > extern void radix_kvm_prefetch_workaround(struct mm_struct *mm); > > > > +void do_h_rpt_invalidate_prt(unsigned long pid, unsigned long lpid, > > > > + unsigned long type, unsigned long pg_sizes, > > > > + unsigned long start, unsigned long end); > > > > #else > > > > static inline void radix_kvm_prefetch_workaround(struct mm_struct *mm) { } > > > > +static inline void do_h_rpt_invalidate_prt(unsigned long pid, > > > > + unsigned long lpid, > > > > + unsigned long type, > > > > + unsigned long pg_sizes, > > > > + unsigned long start, > > > > + unsigned long end) { } > > > > > > Since the only plausible caller is in KVM HV code, why do you need the > > > #else clause. > > > > The call to the above routine is prevented for non-radix guests > > in KVM HV code at runtime using kvm_is_radix() check and not by > > CONFIG_PPC_RADIX_MMU. Hence the #else version would be needed. > > kvm_is_radix() should evaluate to false at compile time if > !defined(CONFIG_PPC_RADIX_MMU), in which case, no you shouldn't need > the else version. At least in the latest mainline, I don't see the definition of kvm_is_radix() being conditional to CONFIG_PPC_RADIX_MMU. Anyway this is what I see in practice if the #else version is removed and CONFIG_PPC_RADIX_MMU is turned off: arch/powerpc/kvm/book3s_hv.c: In function ‘kvmppc_h_rpt_invalidate’: arch/powerpc/kvm/book3s_hv.c:983:2: error: implicit declaration of function ‘do_h_rpt_invalidate_prt’; did you mean ‘do_h_rpt_invalidate_pat’? [-Werror=implicit-function-declaration] 983 | do_h_rpt_invalidate_prt(id, vcpu->kvm->arch.lpid, | ^~~~~~~~~~~~~~~~~~~~~~~ | do_h_rpt_invalidate_pat Regards, Bharata.