2023-01-12 14:06:38

by Anup Patel

Subject: [PATCH 3/7] RISC-V: KVM: Drop the _MASK suffix from hgatp.VMID mask defines

The hgatp.VMID mask defines are used before shifting when extracting
VMID value from hgatp CSR value so based on the convention followed
in the other parts of asm/csr.h, the hgatp.VMID mask defines should
not have a _MASK suffix.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/csr.h | 8 ++++----
arch/riscv/kvm/mmu.c | 3 +--
arch/riscv/kvm/vmid.c | 4 ++--
3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index d608dac4b19f..36d580528f90 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -131,12 +131,12 @@

#define HGATP32_MODE_SHIFT 31
#define HGATP32_VMID_SHIFT 22
-#define HGATP32_VMID_MASK _AC(0x1FC00000, UL)
+#define HGATP32_VMID _AC(0x1FC00000, UL)
#define HGATP32_PPN _AC(0x003FFFFF, UL)

#define HGATP64_MODE_SHIFT 60
#define HGATP64_VMID_SHIFT 44
-#define HGATP64_VMID_MASK _AC(0x03FFF00000000000, UL)
+#define HGATP64_VMID _AC(0x03FFF00000000000, UL)
#define HGATP64_PPN _AC(0x00000FFFFFFFFFFF, UL)

#define HGATP_PAGE_SHIFT 12
@@ -144,12 +144,12 @@
#ifdef CONFIG_64BIT
#define HGATP_PPN HGATP64_PPN
#define HGATP_VMID_SHIFT HGATP64_VMID_SHIFT
-#define HGATP_VMID_MASK HGATP64_VMID_MASK
+#define HGATP_VMID HGATP64_VMID
#define HGATP_MODE_SHIFT HGATP64_MODE_SHIFT
#else
#define HGATP_PPN HGATP32_PPN
#define HGATP_VMID_SHIFT HGATP32_VMID_SHIFT
-#define HGATP_VMID_MASK HGATP32_VMID_MASK
+#define HGATP_VMID HGATP32_VMID
#define HGATP_MODE_SHIFT HGATP32_MODE_SHIFT
#endif

diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index 34b57e0be2ef..034746638fa6 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -748,8 +748,7 @@ void kvm_riscv_gstage_update_hgatp(struct kvm_vcpu *vcpu)
unsigned long hgatp = gstage_mode;
struct kvm_arch *k = &vcpu->kvm->arch;

- hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) &
- HGATP_VMID_MASK;
+ hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
hgatp |= (k->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;

csr_write(CSR_HGATP, hgatp);
diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
index 6cd93995fb65..6f4d4979a759 100644
--- a/arch/riscv/kvm/vmid.c
+++ b/arch/riscv/kvm/vmid.c
@@ -26,9 +26,9 @@ void kvm_riscv_gstage_vmid_detect(void)

/* Figure-out number of VMID bits in HW */
old = csr_read(CSR_HGATP);
- csr_write(CSR_HGATP, old | HGATP_VMID_MASK);
+ csr_write(CSR_HGATP, old | HGATP_VMID);
vmid_bits = csr_read(CSR_HGATP);
- vmid_bits = (vmid_bits & HGATP_VMID_MASK) >> HGATP_VMID_SHIFT;
+ vmid_bits = (vmid_bits & HGATP_VMID) >> HGATP_VMID_SHIFT;
vmid_bits = fls_long(vmid_bits);
csr_write(CSR_HGATP, old);

--
2.34.1

2023-01-26 16:12:19

by Andrew Jones

Subject: Re: [PATCH 3/7] RISC-V: KVM: Drop the _MASK suffix from hgatp.VMID mask defines

On Thu, Jan 12, 2023 at 07:33:00PM +0530, Anup Patel wrote:
> The hgatp.VMID mask defines are used before shifting when extracting
> VMID value from hgatp CSR value so based on the convention followed
> in the other parts of asm/csr.h, the hgatp.VMID mask defines should
> not have a _MASK suffix.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/csr.h | 8 ++++----
> arch/riscv/kvm/mmu.c | 3 +--
> arch/riscv/kvm/vmid.c | 4 ++--
> 3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index d608dac4b19f..36d580528f90 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -131,12 +131,12 @@
>
> #define HGATP32_MODE_SHIFT 31
> #define HGATP32_VMID_SHIFT 22
> -#define HGATP32_VMID_MASK _AC(0x1FC00000, UL)
> +#define HGATP32_VMID _AC(0x1FC00000, UL)
> #define HGATP32_PPN _AC(0x003FFFFF, UL)
>
> #define HGATP64_MODE_SHIFT 60
> #define HGATP64_VMID_SHIFT 44
> -#define HGATP64_VMID_MASK _AC(0x03FFF00000000000, UL)
> +#define HGATP64_VMID _AC(0x03FFF00000000000, UL)
> #define HGATP64_PPN _AC(0x00000FFFFFFFFFFF, UL)
>
> #define HGATP_PAGE_SHIFT 12
> @@ -144,12 +144,12 @@
> #ifdef CONFIG_64BIT
> #define HGATP_PPN HGATP64_PPN
> #define HGATP_VMID_SHIFT HGATP64_VMID_SHIFT
> -#define HGATP_VMID_MASK HGATP64_VMID_MASK
> +#define HGATP_VMID HGATP64_VMID
> #define HGATP_MODE_SHIFT HGATP64_MODE_SHIFT
> #else
> #define HGATP_PPN HGATP32_PPN
> #define HGATP_VMID_SHIFT HGATP32_VMID_SHIFT
> -#define HGATP_VMID_MASK HGATP32_VMID_MASK
> +#define HGATP_VMID HGATP32_VMID
> #define HGATP_MODE_SHIFT HGATP32_MODE_SHIFT
> #endif
>
> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 34b57e0be2ef..034746638fa6 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -748,8 +748,7 @@ void kvm_riscv_gstage_update_hgatp(struct kvm_vcpu *vcpu)
> unsigned long hgatp = gstage_mode;
> struct kvm_arch *k = &vcpu->kvm->arch;
>
> - hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) &
> - HGATP_VMID_MASK;
> + hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
> hgatp |= (k->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;
>
> csr_write(CSR_HGATP, hgatp);
> diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
> index 6cd93995fb65..6f4d4979a759 100644
> --- a/arch/riscv/kvm/vmid.c
> +++ b/arch/riscv/kvm/vmid.c
> @@ -26,9 +26,9 @@ void kvm_riscv_gstage_vmid_detect(void)
>
> /* Figure-out number of VMID bits in HW */
> old = csr_read(CSR_HGATP);
> - csr_write(CSR_HGATP, old | HGATP_VMID_MASK);
> + csr_write(CSR_HGATP, old | HGATP_VMID);
> vmid_bits = csr_read(CSR_HGATP);
> - vmid_bits = (vmid_bits & HGATP_VMID_MASK) >> HGATP_VMID_SHIFT;
> + vmid_bits = (vmid_bits & HGATP_VMID) >> HGATP_VMID_SHIFT;
> vmid_bits = fls_long(vmid_bits);
> csr_write(CSR_HGATP, old);
>
> --
> 2.34.1
>

Could switch to GENMASK too at the same time :-)

Anyway,

Reviewed-by: Andrew Jones <[email protected]>

Thanks,
drew

2023-01-27 13:53:32

by Anup Patel

Subject: Re: [PATCH 3/7] RISC-V: KVM: Drop the _MASK suffix from hgatp.VMID mask defines

On Thu, Jan 26, 2023 at 9:41 PM Andrew Jones <[email protected]> wrote:
>
> On Thu, Jan 12, 2023 at 07:33:00PM +0530, Anup Patel wrote:
> > The hgatp.VMID mask defines are used before shifting when extracting
> > VMID value from hgatp CSR value so based on the convention followed
> > in the other parts of asm/csr.h, the hgatp.VMID mask defines should
> > not have a _MASK suffix.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/include/asm/csr.h | 8 ++++----
> > arch/riscv/kvm/mmu.c | 3 +--
> > arch/riscv/kvm/vmid.c | 4 ++--
> > 3 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> > index d608dac4b19f..36d580528f90 100644
> > --- a/arch/riscv/include/asm/csr.h
> > +++ b/arch/riscv/include/asm/csr.h
> > @@ -131,12 +131,12 @@
> >
> > #define HGATP32_MODE_SHIFT 31
> > #define HGATP32_VMID_SHIFT 22
> > -#define HGATP32_VMID_MASK _AC(0x1FC00000, UL)
> > +#define HGATP32_VMID _AC(0x1FC00000, UL)
> > #define HGATP32_PPN _AC(0x003FFFFF, UL)
> >
> > #define HGATP64_MODE_SHIFT 60
> > #define HGATP64_VMID_SHIFT 44
> > -#define HGATP64_VMID_MASK _AC(0x03FFF00000000000, UL)
> > +#define HGATP64_VMID _AC(0x03FFF00000000000, UL)
> > #define HGATP64_PPN _AC(0x00000FFFFFFFFFFF, UL)
> >
> > #define HGATP_PAGE_SHIFT 12
> > @@ -144,12 +144,12 @@
> > #ifdef CONFIG_64BIT
> > #define HGATP_PPN HGATP64_PPN
> > #define HGATP_VMID_SHIFT HGATP64_VMID_SHIFT
> > -#define HGATP_VMID_MASK HGATP64_VMID_MASK
> > +#define HGATP_VMID HGATP64_VMID
> > #define HGATP_MODE_SHIFT HGATP64_MODE_SHIFT
> > #else
> > #define HGATP_PPN HGATP32_PPN
> > #define HGATP_VMID_SHIFT HGATP32_VMID_SHIFT
> > -#define HGATP_VMID_MASK HGATP32_VMID_MASK
> > +#define HGATP_VMID HGATP32_VMID
> > #define HGATP_MODE_SHIFT HGATP32_MODE_SHIFT
> > #endif
> >
> > diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> > index 34b57e0be2ef..034746638fa6 100644
> > --- a/arch/riscv/kvm/mmu.c
> > +++ b/arch/riscv/kvm/mmu.c
> > @@ -748,8 +748,7 @@ void kvm_riscv_gstage_update_hgatp(struct kvm_vcpu *vcpu)
> > unsigned long hgatp = gstage_mode;
> > struct kvm_arch *k = &vcpu->kvm->arch;
> >
> > - hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) &
> > - HGATP_VMID_MASK;
> > + hgatp |= (READ_ONCE(k->vmid.vmid) << HGATP_VMID_SHIFT) & HGATP_VMID;
> > hgatp |= (k->pgd_phys >> PAGE_SHIFT) & HGATP_PPN;
> >
> > csr_write(CSR_HGATP, hgatp);
> > diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
> > index 6cd93995fb65..6f4d4979a759 100644
> > --- a/arch/riscv/kvm/vmid.c
> > +++ b/arch/riscv/kvm/vmid.c
> > @@ -26,9 +26,9 @@ void kvm_riscv_gstage_vmid_detect(void)
> >
> > /* Figure-out number of VMID bits in HW */
> > old = csr_read(CSR_HGATP);
> > - csr_write(CSR_HGATP, old | HGATP_VMID_MASK);
> > + csr_write(CSR_HGATP, old | HGATP_VMID);
> > vmid_bits = csr_read(CSR_HGATP);
> > - vmid_bits = (vmid_bits & HGATP_VMID_MASK) >> HGATP_VMID_SHIFT;
> > + vmid_bits = (vmid_bits & HGATP_VMID) >> HGATP_VMID_SHIFT;
> > vmid_bits = fls_long(vmid_bits);
> > csr_write(CSR_HGATP, old);
> >
> > --
> > 2.34.1
> >
>
> Could switch to GENMASK too at the same time :-)

Okay, I will update.

>
> Anyway,
>
> Reviewed-by: Andrew Jones <[email protected]>
>

Thanks,
Anup