Re: Re: [PATCH v5 2/3] RISC-V: KVM: Detect and expose supported HGATP G-stage modes

[fangyu . yu]

>> From: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
>>
>> Extend kvm_riscv_gstage_mode_detect() to probe all HGATP.MODE values
>> supported by the host and record them in a bitmask. Keep tracking the
>> maximum supported G-stage page table level for existing internal users.
>>
>> Also provide lightweight helpers to retrieve the supported-mode bitmask
>> and validate a requested HGATP.MODE against it.
>>
>> Signed-off-by: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
>> ---
>>  arch/riscv/include/asm/kvm_gstage.h | 11 ++++++++
>>  arch/riscv/kvm/gstage.c             | 43 +++++++++++++++--------------
>>  2 files changed, 34 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/kvm_gstage.h b/arch/riscv/include/asm/kvm_gstage.h
>> index b12605fbca44..76c37b5dc02d 100644
>> --- a/arch/riscv/include/asm/kvm_gstage.h
>> +++ b/arch/riscv/include/asm/kvm_gstage.h
>> @@ -30,6 +30,7 @@ struct kvm_gstage_mapping {
>>  #endif
>>
>>  extern unsigned long kvm_riscv_gstage_max_pgd_levels;
>> +extern u32 kvm_riscv_gstage_mode_mask;
>
>s/u32/unsigned long/
>s/kvm_riscv_gstage_mode_mask/kvm_riscv_gstage_supported_mode_mask/
>

Ack, will switch the type to unsigned long and rename it to
kvm_riscv_gstage_supported_mode_mask in the next revision.

>>
>>  #define kvm_riscv_gstage_pgd_xbits     2
>>  #define kvm_riscv_gstage_pgd_size      (1UL << (HGATP_PAGE_SHIFT + kvm_riscv_gstage_pgd_xbits))
>> @@ -75,4 +76,14 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
>>
>>  void kvm_riscv_gstage_mode_detect(void);
>>
>> +static inline u32 kvm_riscv_get_hgatp_mode_mask(void)
>> +{
>> +       return kvm_riscv_gstage_mode_mask;
>> +}
>> +
>> +static inline bool kvm_riscv_hgatp_mode_is_valid(unsigned long mode)
>> +{
>> +       return kvm_riscv_gstage_mode_mask & BIT(mode);
>> +}
>> +
>>  #endif
>> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
>> index 2d0045f502d1..328d4138f162 100644
>> --- a/arch/riscv/kvm/gstage.c
>> +++ b/arch/riscv/kvm/gstage.c
>> @@ -16,6 +16,8 @@ unsigned long kvm_riscv_gstage_max_pgd_levels __ro_after_init = 3;
>>  #else
>>  unsigned long kvm_riscv_gstage_max_pgd_levels __ro_after_init = 2;
>>  #endif
>> +/* Bitmask of supported HGATP.MODE encodings (BIT(HGATP_MODE_*)). */
>> +u32 kvm_riscv_gstage_mode_mask __ro_after_init;
>>
>>  #define gstage_pte_leaf(__ptep)        \
>>         (pte_val(*(__ptep)) & (_PAGE_READ | _PAGE_WRITE | _PAGE_EXEC))
>> @@ -315,42 +317,43 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
>>         }
>>  }
>>
>> +static bool __init kvm_riscv_hgatp_mode_supported(unsigned long mode)
>> +{
>> +       csr_write(CSR_HGATP, mode << HGATP_MODE_SHIFT);
>> +       return ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == mode);
>> +}
>> +
>>  void __init kvm_riscv_gstage_mode_detect(void)
>>  {
>> +       kvm_riscv_gstage_mode_mask = 0;
>> +       kvm_riscv_gstage_max_pgd_levels = 0;
>> +
>>  #ifdef CONFIG_64BIT
>> -       /* Try Sv57x4 G-stage mode */
>> -       csr_write(CSR_HGATP, HGATP_MODE_SV57X4 << HGATP_MODE_SHIFT);
>> -       if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV57X4) {
>> -               kvm_riscv_gstage_max_pgd_levels = 5;
>> -               goto done;
>> +       /* Try Sv39x4 G-stage mode */
>> +       if (kvm_riscv_hgatp_mode_supported(HGATP_MODE_SV39X4)) {
>> +               kvm_riscv_gstage_mode_mask |= BIT(HGATP_MODE_SV39X4);
>> +               kvm_riscv_gstage_max_pgd_levels = 3;
>>         }
>>
>>         /* Try Sv48x4 G-stage mode */
>> -       csr_write(CSR_HGATP, HGATP_MODE_SV48X4 << HGATP_MODE_SHIFT);
>> -       if ((csr_read(CSR_HGATP) >> HGATP_MODE_SHIFT) == HGATP_MODE_SV48X4) {
>> +       if (kvm_riscv_hgatp_mode_supported(HGATP_MODE_SV48X4)) {
>> +               kvm_riscv_gstage_mode_mask |= BIT(HGATP_MODE_SV48X4);
>>                 kvm_riscv_gstage_max_pgd_levels = 4;
>> -               goto done;
>>         }
>>
>> -       /* 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_max_pgd_levels = 3;
>> -               goto done;
>> +       /* Try Sv57x4 G-stage mode */
>> +       if (kvm_riscv_hgatp_mode_supported(HGATP_MODE_SV57X4)) {
>> +               kvm_riscv_gstage_mode_mask |= BIT(HGATP_MODE_SV57X4);
>> +               kvm_riscv_gstage_max_pgd_levels = 5;
>>         }
>>  #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) {
>> +       if (kvm_riscv_hgatp_mode_supported(HGATP_MODE_SV32X4)) {
>> +               kvm_riscv_gstage_mode_mask |= BIT(HGATP_MODE_SV32X4);
>>                 kvm_riscv_gstage_max_pgd_levels = 2;
>> -               goto done;
>>         }
>>  #endif
>>
>> -       /* KVM depends on !HGATP_MODE_OFF */
>> -       kvm_riscv_gstage_max_pgd_levels = 0;
>> -
>> -done:
>
>Here are some statements from RISC-V privilege specification:
>"Implementations that support Sv48 must also support Sv39."
>"Implementations that support Sv57 must also support Sv48."
>"The conversion of an Sv32x4, Sv39x4, Sv48x4, or Sv57x4 guest physical
>address is accomplished with the
>same algorithm used for Sv32, Sv39, Sv48, or Sv57, as presented in
>Section 12.3.2, except that:"
>"hgatp substitutes for the usual satp;"
>
>Based on above it is a waste to try each and every mode.
>For example: if mode Sv48x4 is supported then Sv39x4 is also supported.
>

Radmi and I discussed this topic before; please refer to the following link:
https://lore.kernel.org/linux-riscv/20260131061238.52708-1-fangyu.yu@xxxxxxxxxxxxxxxxx/

>Regards,
>Anup

Thanks,
Fangyu