Re: [PATCH 09/12] KVM: PPC: Book3S HV nestedv2: Do not call H_COPY_TOFROM_GUEST

From: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
To: Vaibhav Jain <vaibhav@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
Cc: mikey@neuling.org, sbhat@linux.ibm.com,
	amachhiw@linux.vnet.ibm.com, gautam@linux.ibm.com,
	Nicholas Piggin <npiggin@gmail.com>,
	David.Laight@ACULAB.COM, kconsul@linux.vnet.ibm.com,
	Jordan Niethe <jniethe5@gmail.com>,
	Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
Subject: Re: [PATCH 09/12] KVM: PPC: Book3S HV nestedv2: Do not call H_COPY_TOFROM_GUEST
Date: Mon, 18 Dec 2023 12:59:27 +0530	[thread overview]
Message-ID: <875y0wrmso.fsf@kernel.org> (raw)
In-Reply-To: <87zfy89enk.fsf@vajain21.in.ibm.com>

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> Hi Aneesh,
>
> "Aneesh Kumar K.V" <aneesh.kumar@kernel.org> writes:
>
> <snip>
>>> Yes, Agreed and thats a nice suggestion. However ATM the hypervisor 
>>> supporting Nestedv2 doesnt have support for this hcall. In future
>>> once we have support for this hcall for nestedv2 from the hypervisor
>>> we can replace this branch with a firmware_has_feature() test.
>>> 
>>
>> What I am suggesting is we convert that conditional to firmware_has_feature so that
>> later when hypervisor supports this hcall all older kernel can make
>> use of the copy_tofrom_guest without any code change.
>
> AFAIK for firmware_has_feature to work we either need:
> - A way to call this hcall with some invalid args. However lpid/pid for
> guest arent allocated during boot.
>
> - A way for hypervisor to advertise support for this hcall before the L1
> kernel boots.
>
> ATM L0 dosent support for any of these two ways. I can do a follow up
> patch later when we have a clarity on how we want to advertise support
> for this hcall. For now current kernel supporting nestedv2 wont be
> using this hcall assuming its not supported. Future kernels can use one
> of the two ways above to set the firmware_has_feature flag to take
> advantage of this hcall.
>

We can use the second option and have L0 publish the firmware feature
when it adds the new hcall. The good part about this is that all
existing L1 kernels will automatically use the new hcall. Something
like.

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 69ae9cf57d50..0ef97b56f999 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -57,6 +57,7 @@
 #define FW_FEATURE_ENERGY_SCALE_INFO ASM_CONST(0x0000040000000000)
 #define FW_FEATURE_WATCHDOG	ASM_CONST(0x0000080000000000)
 #define FW_FEATURE_PLPKS	ASM_CONST(0x0000100000000000)
+#define FW_FEATURE_H_COPY_TOFROM_GUEST	ASM_CONST(0x0000200000000000)
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 282d1b54b073..8fc598b4767a 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -39,6 +39,9 @@ unsigned long __kvmhv_copy_tofrom_guest_radix(int lpid, int pid,
 	unsigned long quadrant, ret = n;
 	bool is_load = !!to;
 
+	if (!firmware_has_feature(FW_FEATURE_H_COPY_TOFROM_GUEST))
+		return H_UNSUPPORTED;
+
 	/* Can't access quadrants 1 or 2 in non-HV mode, call the HV to do it */
 	if (kvmhv_on_pseries())
 		return plpar_hcall_norets(H_COPY_TOFROM_GUEST, lpid, pid, eaddr,
diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
index 18447e5fa17d..d49b5c52e7b8 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -69,6 +69,8 @@ hypertas_fw_features_table[] = {
 	{FW_FEATURE_ENERGY_SCALE_INFO,	"hcall-energy-scale-info"},
 	{FW_FEATURE_WATCHDOG,		"hcall-watchdog"},
 	{FW_FEATURE_PLPKS,		"hcall-pks"},
+	{FW_FEATURE_H_COPY_TOFROM_GUEST,
+					"hcall-h-copy_tofrom-guest"},
 };
 
 /* Build up the firmware features bitmask using the contents of

