From: "Guo Ren (Alibaba DAMO Academy)" <guoren@kernel.org>
urrent kvm_riscv_gstage_mode_detect() assumes H-extension must
have HGATP_MODE_SV39X4/SV32X4 at least, but the spec allows
H-extension with HGATP_MODE_BARE alone. The KVM depends on
!HGATP_MODE_BARE at least, so enhance the gstage-mode-detect
to block HGATP_MODE_BARE.
Move gstage-mode-check closer to gstage-mode-detect to prevent
unnecessary init.
Reviewed-by: Troy Mitchell <troy.mitchell@linux.dev>
Reviewed-by: Nutty Liu <nutty.liu@hotmail.com>
Signed-off-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
---
arch/riscv/kvm/gstage.c | 27 ++++++++++++++++++++++++---
arch/riscv/kvm/main.c | 35 +++++++++++++++++------------------
2 files changed, 41 insertions(+), 21 deletions(-)
diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
index 24c270d6d0e2..b67d60d722c2 100644
--- a/arch/riscv/kvm/gstage.c
+++ b/arch/riscv/kvm/gstage.c
@@ -321,7 +321,7 @@ void __init kvm_riscv_gstage_mode_detect(void)
if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV57X4) {
kvm_riscv_gstage_mode = HGATP_MODE_SV57X4;
kvm_riscv_gstage_pgd_levels = 5;
- goto skip_sv48x4_test;
+ goto done;
}
/* Try Sv48x4 G-stage mode */
@@ -329,10 +329,31 @@ void __init kvm_riscv_gstage_mode_detect(void)
if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV48X4) {
kvm_riscv_gstage_mode = HGATP_MODE_SV48X4;
kvm_riscv_gstage_pgd_levels = 4;
+ goto done;
}
-skip_sv48x4_test:
+ /* Try Sv39x4 G-stage mode */
+ csr_write(CSR_HGATP, HGATP_MODE_SV39X4 << HGATP_MODE_SHIFT);
+ if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV39X4) {
+ kvm_riscv_gstage_mode = HGATP_MODE_SV39X4;
+ kvm_riscv_gstage_pgd_levels = 3;
+ goto done;
+ }
+#else /* CONFIG_32BIT */
+ /* Try Sv32x4 G-stage mode */
+ csr_write(CSR_HGATP, HGATP_MODE_SV32X4 << HGATP_MODE_SHIFT);
+ if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV32X4) {
+ kvm_riscv_gstage_mode = HGATP_MODE_SV32X4;
+ kvm_riscv_gstage_pgd_levels = 2;
+ goto done;
+ }
+#endif
+
+ /* KVM depends on !HGATP_MODE_OFF */
+ kvm_riscv_gstage_mode = HGATP_MODE_OFF;
+ kvm_riscv_gstage_pgd_levels = 0;
+
+done:
csr_write(CSR_HGATP, 0);
kvm_riscv_local_hfence_gvma_all();
-#endif
}
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index 67c876de74ef..8ee7aaa74ddc 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -93,6 +93,23 @@ static int __init riscv_kvm_init(void)
return rc;
kvm_riscv_gstage_mode_detect();
+ switch (kvm_riscv_gstage_mode) {
+ case HGATP_MODE_SV32X4:
+ str = "Sv32x4";
+ break;
+ case HGATP_MODE_SV39X4:
+ str = "Sv39x4";
+ break;
+ case HGATP_MODE_SV48X4:
+ str = "Sv48x4";
+ break;
+ case HGATP_MODE_SV57X4:
+ str = "Sv57x4";
+ break;
+ default:
+ return -ENODEV;
+ }
+ kvm_info("using %s G-stage page table format\n", str);
kvm_riscv_gstage_vmid_detect();
@@ -135,24 +152,6 @@ static int __init riscv_kvm_init(void)
(rc) ? slist : "no features");
}
- switch (kvm_riscv_gstage_mode) {
- case HGATP_MODE_SV32X4:
- str = "Sv32x4";
- break;
- case HGATP_MODE_SV39X4:
- str = "Sv39x4";
- break;
- case HGATP_MODE_SV48X4:
- str = "Sv48x4";
- break;
- case HGATP_MODE_SV57X4:
- str = "Sv57x4";
- break;
- default:
- return -ENODEV;
- }
- kvm_info("using %s G-stage page table format\n", str);
-
kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());
if (kvm_riscv_aia_available())
--
2.40.1On Thu, Aug 21, 2025 at 7:56 PM <guoren@kernel.org> wrote:
>
> From: "Guo Ren (Alibaba DAMO Academy)" <guoren@kernel.org>
>
> urrent kvm_riscv_gstage_mode_detect() assumes H-extension must
s/urrent/Current/
> have HGATP_MODE_SV39X4/SV32X4 at least, but the spec allows
> H-extension with HGATP_MODE_BARE alone. The KVM depends on
> !HGATP_MODE_BARE at least, so enhance the gstage-mode-detect
> to block HGATP_MODE_BARE.
>
> Move gstage-mode-check closer to gstage-mode-detect to prevent
> unnecessary init.
>
> Reviewed-by: Troy Mitchell <troy.mitchell@linux.dev>
> Reviewed-by: Nutty Liu <nutty.liu@hotmail.com>
> Signed-off-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
> ---
> arch/riscv/kvm/gstage.c | 27 ++++++++++++++++++++++++---
> arch/riscv/kvm/main.c | 35 +++++++++++++++++------------------
> 2 files changed, 41 insertions(+), 21 deletions(-)
>
> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index 24c270d6d0e2..b67d60d722c2 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c
> @@ -321,7 +321,7 @@ void __init kvm_riscv_gstage_mode_detect(void)
> if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV57X4) {
> kvm_riscv_gstage_mode = HGATP_MODE_SV57X4;
> kvm_riscv_gstage_pgd_levels = 5;
> - goto skip_sv48x4_test;
> + goto done;
> }
>
> /* Try Sv48x4 G-stage mode */
> @@ -329,10 +329,31 @@ void __init kvm_riscv_gstage_mode_detect(void)
> if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV48X4) {
> kvm_riscv_gstage_mode = HGATP_MODE_SV48X4;
> kvm_riscv_gstage_pgd_levels = 4;
> + goto done;
> }
> -skip_sv48x4_test:
>
> + /* Try Sv39x4 G-stage mode */
> + csr_write(CSR_HGATP, HGATP_MODE_SV39X4 << HGATP_MODE_SHIFT);
> + if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV39X4) {
> + kvm_riscv_gstage_mode = HGATP_MODE_SV39X4;
> + kvm_riscv_gstage_pgd_levels = 3;
> + goto done;
> + }
> +#else /* CONFIG_32BIT */
> + /* Try Sv32x4 G-stage mode */
> + csr_write(CSR_HGATP, HGATP_MODE_SV32X4 << HGATP_MODE_SHIFT);
> + if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV32X4) {
> + kvm_riscv_gstage_mode = HGATP_MODE_SV32X4;
> + kvm_riscv_gstage_pgd_levels = 2;
> + goto done;
> + }
> +#endif
> +
> + /* KVM depends on !HGATP_MODE_OFF */
> + kvm_riscv_gstage_mode = HGATP_MODE_OFF;
> + kvm_riscv_gstage_pgd_levels = 0;
> +
> +done:
> csr_write(CSR_HGATP, 0);
> kvm_riscv_local_hfence_gvma_all();
> -#endif
> }
> diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
> index 67c876de74ef..8ee7aaa74ddc 100644
> --- a/arch/riscv/kvm/main.c
> +++ b/arch/riscv/kvm/main.c
> @@ -93,6 +93,23 @@ static int __init riscv_kvm_init(void)
> return rc;
>
> kvm_riscv_gstage_mode_detect();
> + switch (kvm_riscv_gstage_mode) {
> + case HGATP_MODE_SV32X4:
> + str = "Sv32x4";
> + break;
> + case HGATP_MODE_SV39X4:
> + str = "Sv39x4";
> + break;
> + case HGATP_MODE_SV48X4:
> + str = "Sv48x4";
> + break;
> + case HGATP_MODE_SV57X4:
> + str = "Sv57x4";
> + break;
> + default:
Need kvm_riscv_nacl_exit() here.
> + return -ENODEV;
> + }
> + kvm_info("using %s G-stage page table format\n", str);
Moving the kvm_info() over here now prints G-stage mode
before announcing availablity of h-extension which looks odd.
It's better to keep kvm_info() in the same location and only
move the switch-case.
>
> kvm_riscv_gstage_vmid_detect();
>
> @@ -135,24 +152,6 @@ static int __init riscv_kvm_init(void)
> (rc) ? slist : "no features");
> }
>
> - switch (kvm_riscv_gstage_mode) {
> - case HGATP_MODE_SV32X4:
> - str = "Sv32x4";
> - break;
> - case HGATP_MODE_SV39X4:
> - str = "Sv39x4";
> - break;
> - case HGATP_MODE_SV48X4:
> - str = "Sv48x4";
> - break;
> - case HGATP_MODE_SV57X4:
> - str = "Sv57x4";
> - break;
> - default:
> - return -ENODEV;
> - }
> - kvm_info("using %s G-stage page table format\n", str);
> -
> kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());
>
> if (kvm_riscv_aia_available())
> --
> 2.40.1
>
Otherwise, this looks good to me.
I will take care of minor comments mentioned above at the
time of merging this series.
Regards,
Anup
On Fri, Sep 5, 2025 at 2:51 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Thu, Aug 21, 2025 at 7:56 PM <guoren@kernel.org> wrote:
> >
> > From: "Guo Ren (Alibaba DAMO Academy)" <guoren@kernel.org>
> >
> > urrent kvm_riscv_gstage_mode_detect() assumes H-extension must
>
> s/urrent/Current/
Oh, my fault about copy & paste.
>
> > have HGATP_MODE_SV39X4/SV32X4 at least, but the spec allows
> > H-extension with HGATP_MODE_BARE alone. The KVM depends on
> > !HGATP_MODE_BARE at least, so enhance the gstage-mode-detect
> > to block HGATP_MODE_BARE.
> >
> > Move gstage-mode-check closer to gstage-mode-detect to prevent
> > unnecessary init.
> >
> > Reviewed-by: Troy Mitchell <troy.mitchell@linux.dev>
> > Reviewed-by: Nutty Liu <nutty.liu@hotmail.com>
> > Signed-off-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
> > ---
> > arch/riscv/kvm/gstage.c | 27 ++++++++++++++++++++++++---
> > arch/riscv/kvm/main.c | 35 +++++++++++++++++------------------
> > 2 files changed, 41 insertions(+), 21 deletions(-)
> >
> > diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> > index 24c270d6d0e2..b67d60d722c2 100644
> > --- a/arch/riscv/kvm/gstage.c
> > +++ b/arch/riscv/kvm/gstage.c
> > @@ -321,7 +321,7 @@ void __init kvm_riscv_gstage_mode_detect(void)
> > if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV57X4) {
> > kvm_riscv_gstage_mode = HGATP_MODE_SV57X4;
> > kvm_riscv_gstage_pgd_levels = 5;
> > - goto skip_sv48x4_test;
> > + goto done;
> > }
> >
> > /* Try Sv48x4 G-stage mode */
> > @@ -329,10 +329,31 @@ void __init kvm_riscv_gstage_mode_detect(void)
> > if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV48X4) {
> > kvm_riscv_gstage_mode = HGATP_MODE_SV48X4;
> > kvm_riscv_gstage_pgd_levels = 4;
> > + goto done;
> > }
> > -skip_sv48x4_test:
> >
> > + /* Try Sv39x4 G-stage mode */
> > + csr_write(CSR_HGATP, HGATP_MODE_SV39X4 << HGATP_MODE_SHIFT);
> > + if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV39X4) {
> > + kvm_riscv_gstage_mode = HGATP_MODE_SV39X4;
> > + kvm_riscv_gstage_pgd_levels = 3;
> > + goto done;
> > + }
> > +#else /* CONFIG_32BIT */
> > + /* Try Sv32x4 G-stage mode */
> > + csr_write(CSR_HGATP, HGATP_MODE_SV32X4 << HGATP_MODE_SHIFT);
> > + if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV32X4) {
> > + kvm_riscv_gstage_mode = HGATP_MODE_SV32X4;
> > + kvm_riscv_gstage_pgd_levels = 2;
> > + goto done;
> > + }
> > +#endif
> > +
> > + /* KVM depends on !HGATP_MODE_OFF */
> > + kvm_riscv_gstage_mode = HGATP_MODE_OFF;
> > + kvm_riscv_gstage_pgd_levels = 0;
> > +
> > +done:
> > csr_write(CSR_HGATP, 0);
> > kvm_riscv_local_hfence_gvma_all();
> > -#endif
> > }
> > diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
> > index 67c876de74ef..8ee7aaa74ddc 100644
> > --- a/arch/riscv/kvm/main.c
> > +++ b/arch/riscv/kvm/main.c
> > @@ -93,6 +93,23 @@ static int __init riscv_kvm_init(void)
> > return rc;
> >
> > kvm_riscv_gstage_mode_detect();
> > + switch (kvm_riscv_gstage_mode) {
> > + case HGATP_MODE_SV32X4:
> > + str = "Sv32x4";
> > + break;
> > + case HGATP_MODE_SV39X4:
> > + str = "Sv39x4";
> > + break;
> > + case HGATP_MODE_SV48X4:
> > + str = "Sv48x4";
> > + break;
> > + case HGATP_MODE_SV57X4:
> > + str = "Sv57x4";
> > + break;
> > + default:
>
> Need kvm_riscv_nacl_exit() here.
Yes, it's another legacy problem, which lacks:
kvm_riscv_aia_exit();
kvm_riscv_nacl_exit();
After we move it up, it still needs:
kvm_riscv_nacl_exit();
I'm okay with it being fixed in this patch.
>
> > + return -ENODEV;
> > + }
> > + kvm_info("using %s G-stage page table format\n", str);
>
> Moving the kvm_info() over here now prints G-stage mode
> before announcing availablity of h-extension which looks odd.
> It's better to keep kvm_info() in the same location and only
> move the switch-case.
okay.
>
> >
> > kvm_riscv_gstage_vmid_detect();
> >
> > @@ -135,24 +152,6 @@ static int __init riscv_kvm_init(void)
> > (rc) ? slist : "no features");
> > }
> >
> > - switch (kvm_riscv_gstage_mode) {
> > - case HGATP_MODE_SV32X4:
> > - str = "Sv32x4";
> > - break;
> > - case HGATP_MODE_SV39X4:
> > - str = "Sv39x4";
> > - break;
> > - case HGATP_MODE_SV48X4:
> > - str = "Sv48x4";
> > - break;
> > - case HGATP_MODE_SV57X4:
> > - str = "Sv57x4";
> > - break;
> > - default:
> > - return -ENODEV;
> > - }
> > - kvm_info("using %s G-stage page table format\n", str);
> > -
> > kvm_info("VMID %ld bits available\n", kvm_riscv_gstage_vmid_bits());
> >
> > if (kvm_riscv_aia_available())
> > --
> > 2.40.1
> >
>
> Otherwise, this looks good to me.
>
> I will take care of minor comments mentioned above at the
> time of merging this series.
Thx for taking care. Nice!
--
Best Regards
Guo Ren
© 2016 - 2025 Red Hat, Inc.