[PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[Yu Zhang]

KVM already has a 'GPA_INVALID' defined as (~(gpa_t)0) in kvm_types.h,
and it is used by ARM code. We do not need another definition of
'INVALID_GPA' for X86 specifically.

Instead of using the common 'GPA_INVALID' for X86, replace it with
'INVALID_GPA', and change the users of 'GPA_INVALID' so that the diff
can be smaller. Also because the name 'INVALID_GPA' tells the user we
are using an invalid GPA, while the name 'GPA_INVALID' is emphasizing
the GPA is an invalid one.

No functional change intended.

Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Sean Christopherson <seanjc@google.com>
---
V5:
- No need to add the 'INVALID_GFN' as 'KVM_XEN_INVALID_GPA' and 
'KVM_XEN_INVALID_GFN' were added.
V4:
- Put the addition of 'INVALID_GFN' into a seperate patch.
V3:
- Added 'INVALID_GFN' and use it.
v2:
- Renamed 'GPA_INVALID' to 'INVALID_GPA' and modify _those_ users. 
v1:
https://lore.kernel.org/lkml/20221209023622.274715-1-yu.c.zhang@linux.intel.com/
---
 arch/arm64/include/asm/kvm_host.h | 4 ++--
 arch/arm64/kvm/hypercalls.c       | 2 +-
 arch/arm64/kvm/pvtime.c           | 8 ++++----
 arch/x86/include/asm/kvm_host.h   | 2 --
 include/linux/kvm_types.h         | 2 +-
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 35a159d131b5..37766e57c8f2 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -916,12 +916,12 @@ void kvm_arm_vmid_clear_active(void);
 
 static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
 {
-	vcpu_arch->steal.base = GPA_INVALID;
+	vcpu_arch->steal.base = INVALID_GPA;
 }
 
 static inline bool kvm_arm_is_pvtime_enabled(struct kvm_vcpu_arch *vcpu_arch)
 {
-	return (vcpu_arch->steal.base != GPA_INVALID);
+	return (vcpu_arch->steal.base != INVALID_GPA);
 }
 
 void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index c9f401fa01a9..64c086c02c60 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -198,7 +198,7 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 		break;
 	case ARM_SMCCC_HV_PV_TIME_ST:
 		gpa = kvm_init_stolen_time(vcpu);
-		if (gpa != GPA_INVALID)
+		if (gpa != INVALID_GPA)
 			val[0] = gpa;
 		break;
 	case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
diff --git a/arch/arm64/kvm/pvtime.c b/arch/arm64/kvm/pvtime.c
index 78a09f7a6637..4ceabaa4c30b 100644
--- a/arch/arm64/kvm/pvtime.c
+++ b/arch/arm64/kvm/pvtime.c
@@ -19,7 +19,7 @@ void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
 	u64 steal = 0;
 	int idx;
 
-	if (base == GPA_INVALID)
+	if (base == INVALID_GPA)
 		return;
 
 	idx = srcu_read_lock(&kvm->srcu);
@@ -40,7 +40,7 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)
 	switch (feature) {
 	case ARM_SMCCC_HV_PV_TIME_FEATURES:
 	case ARM_SMCCC_HV_PV_TIME_ST:
-		if (vcpu->arch.steal.base != GPA_INVALID)
+		if (vcpu->arch.steal.base != INVALID_GPA)
 			val = SMCCC_RET_SUCCESS;
 		break;
 	}
@@ -54,7 +54,7 @@ gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu)
 	struct kvm *kvm = vcpu->kvm;
 	u64 base = vcpu->arch.steal.base;
 
-	if (base == GPA_INVALID)
+	if (base == INVALID_GPA)
 		return base;
 
 	/*
@@ -89,7 +89,7 @@ int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu,
 		return -EFAULT;
 	if (!IS_ALIGNED(ipa, 64))
 		return -EINVAL;
-	if (vcpu->arch.steal.base != GPA_INVALID)
+	if (vcpu->arch.steal.base != INVALID_GPA)
 		return -EEXIST;
 
 	/* Check the address is in a valid memslot */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index c70690b2c82d..f18ab36ad684 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -134,8 +134,6 @@
 #define INVALID_PAGE (~(hpa_t)0)
 #define VALID_PAGE(x) ((x) != INVALID_PAGE)
 
-#define INVALID_GPA (~(gpa_t)0)
-
 /* KVM Hugepage definitions for x86 */
 #define KVM_MAX_HUGEPAGE_LEVEL	PG_LEVEL_1G
 #define KVM_NR_PAGE_SIZES	(KVM_MAX_HUGEPAGE_LEVEL - PG_LEVEL_4K + 1)
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 76de36e56cdf..2728d49bbdf6 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -40,7 +40,7 @@ typedef unsigned long  gva_t;
 typedef u64            gpa_t;
 typedef u64            gfn_t;
 
-#define GPA_INVALID	(~(gpa_t)0)
+#define INVALID_GPA	(~(gpa_t)0)
 
 typedef unsigned long  hva_t;
 typedef u64            hpa_t;
-- 
2.25.1

Re: [PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[Sean Christopherson]

On Thu, Jan 05, 2023, Yu Zhang wrote:
> KVM already has a 'GPA_INVALID' defined as (~(gpa_t)0) in kvm_types.h,
> and it is used by ARM code. We do not need another definition of
> 'INVALID_GPA' for X86 specifically.
> 
> Instead of using the common 'GPA_INVALID' for X86, replace it with
> 'INVALID_GPA', and change the users of 'GPA_INVALID' so that the diff
> can be smaller. Also because the name 'INVALID_GPA' tells the user we
> are using an invalid GPA, while the name 'GPA_INVALID' is emphasizing
> the GPA is an invalid one.
> 
> No functional change intended.
> 
> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> Reviewed-by: Paul Durrant <paul@xen.org>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
> ---

Marc and/or Oliver,

Do you want to grab this since most of the changes are to arm64?  I'll happily
take it through x86, but generating a conflict in arm64 seems infinitely more likely.

Re: [PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[Yu Zhang]

On Thu, Jan 19, 2023 at 12:53:36AM +0000, Sean Christopherson wrote:
> On Thu, Jan 05, 2023, Yu Zhang wrote:
> > KVM already has a 'GPA_INVALID' defined as (~(gpa_t)0) in kvm_types.h,
> > and it is used by ARM code. We do not need another definition of
> > 'INVALID_GPA' for X86 specifically.
> > 
> > Instead of using the common 'GPA_INVALID' for X86, replace it with
> > 'INVALID_GPA', and change the users of 'GPA_INVALID' so that the diff
> > can be smaller. Also because the name 'INVALID_GPA' tells the user we
> > are using an invalid GPA, while the name 'GPA_INVALID' is emphasizing
> > the GPA is an invalid one.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> > Reviewed-by: Paul Durrant <paul@xen.org>
> > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > ---
> 
> Marc and/or Oliver,
> 
> Do you want to grab this since most of the changes are to arm64?  I'll happily
> take it through x86, but generating a conflict in arm64 seems infinitely more likely.
> 
Thank you, Sean! 

This patch was based on KVM's next branch - fc471e831016c ("Merge branch 'kvm-late-6.1'
into HEAD"). Tested by cross-building arm64. 

Do you know if KVM arm use a seperate branch(or repo)? Thanks!

B.R.
Yu

Re: [PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[Oliver Upton]

On Thu, Jan 19, 2023 at 03:25:56PM +0800, Yu Zhang wrote:
> On Thu, Jan 19, 2023 at 12:53:36AM +0000, Sean Christopherson wrote:
> > On Thu, Jan 05, 2023, Yu Zhang wrote:
> > > KVM already has a 'GPA_INVALID' defined as (~(gpa_t)0) in kvm_types.h,
> > > and it is used by ARM code. We do not need another definition of
> > > 'INVALID_GPA' for X86 specifically.
> > > 
> > > Instead of using the common 'GPA_INVALID' for X86, replace it with
> > > 'INVALID_GPA', and change the users of 'GPA_INVALID' so that the diff
> > > can be smaller. Also because the name 'INVALID_GPA' tells the user we
> > > are using an invalid GPA, while the name 'GPA_INVALID' is emphasizing
> > > the GPA is an invalid one.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> > > Reviewed-by: Paul Durrant <paul@xen.org>
> > > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > 
> > Marc and/or Oliver,
> > 
> > Do you want to grab this since most of the changes are to arm64?  I'll happily
> > take it through x86, but generating a conflict in arm64 seems infinitely more likely.
> > 
> Thank you, Sean! 
> 
> This patch was based on KVM's next branch - fc471e831016c ("Merge branch 'kvm-late-6.1'
> into HEAD"). Tested by cross-building arm64. 
> 
> Do you know if KVM arm use a seperate branch(or repo)? Thanks!

Yes, you can find us over here:

  https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git/

I'll grab this after I wrap up testing what I have so far.

--
Thanks,
Oliver

Re: [PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[Marc Zyngier]

On Thu, 19 Jan 2023 00:53:36 +0000,
Sean Christopherson <seanjc@google.com> wrote:
> 
> On Thu, Jan 05, 2023, Yu Zhang wrote:
> > KVM already has a 'GPA_INVALID' defined as (~(gpa_t)0) in kvm_types.h,
> > and it is used by ARM code. We do not need another definition of
> > 'INVALID_GPA' for X86 specifically.
> > 
> > Instead of using the common 'GPA_INVALID' for X86, replace it with
> > 'INVALID_GPA', and change the users of 'GPA_INVALID' so that the diff
> > can be smaller. Also because the name 'INVALID_GPA' tells the user we
> > are using an invalid GPA, while the name 'GPA_INVALID' is emphasizing
> > the GPA is an invalid one.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> > Reviewed-by: Paul Durrant <paul@xen.org>
> > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > ---
> 
> Marc and/or Oliver,
> 
> Do you want to grab this since most of the changes are to arm64?
> I'll happily take it through x86, but generating a conflict in arm64
> seems infinitely more likely.
> 

Do you mind acking it then, at least for the x86 part?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[Marc Zyngier]

On Thu, 19 Jan 2023 09:54:50 +0000,
Marc Zyngier <maz@kernel.org> wrote:
> 
> On Thu, 19 Jan 2023 00:53:36 +0000,
> Sean Christopherson <seanjc@google.com> wrote:
> > 
> > On Thu, Jan 05, 2023, Yu Zhang wrote:
> > > KVM already has a 'GPA_INVALID' defined as (~(gpa_t)0) in kvm_types.h,
> > > and it is used by ARM code. We do not need another definition of
> > > 'INVALID_GPA' for X86 specifically.
> > > 
> > > Instead of using the common 'GPA_INVALID' for X86, replace it with
> > > 'INVALID_GPA', and change the users of 'GPA_INVALID' so that the diff
> > > can be smaller. Also because the name 'INVALID_GPA' tells the user we
> > > are using an invalid GPA, while the name 'GPA_INVALID' is emphasizing
> > > the GPA is an invalid one.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> > > Reviewed-by: Paul Durrant <paul@xen.org>
> > > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > 
> > Marc and/or Oliver,
> > 
> > Do you want to grab this since most of the changes are to arm64?
> > I'll happily take it through x86, but generating a conflict in arm64
> > seems infinitely more likely.
> > 
> 
> Do you mind acking it then, at least for the x86 part?

Duh, ignore me, I haven't had enough coffee yet... I'll coordinate
with Oliver on how to queue this.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v5] KVM: MMU: Make the definition of 'INVALID_GPA' common

[Oliver Upton]

On Thu, 5 Jan 2023 21:01:27 +0800, Yu Zhang wrote:
> KVM already has a 'GPA_INVALID' defined as (~(gpa_t)0) in kvm_types.h,
> and it is used by ARM code. We do not need another definition of
> 'INVALID_GPA' for X86 specifically.
> 
> Instead of using the common 'GPA_INVALID' for X86, replace it with
> 'INVALID_GPA', and change the users of 'GPA_INVALID' so that the diff
> can be smaller. Also because the name 'INVALID_GPA' tells the user we
> are using an invalid GPA, while the name 'GPA_INVALID' is emphasizing
> the GPA is an invalid one.
> 
> [...]

Applied to kvmarm/next, thanks!

[1/1] KVM: MMU: Make the definition of 'INVALID_GPA' common
      https://git.kernel.org/kvmarm/kvmarm/c/cecafc0a830f

--
Best,
Oliver