[PATCH v4 2/4] RISC-V: KVM: Detect and expose supported HGATP G-stage modes

[fangyu.yu at linux.alibaba.com]

>> >> From: Fangyu Yu <fangyu.yu at linux.alibaba.com>
>> >>
>> >> 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 at linux.alibaba.com>
>> >> ---
>> >> diff --git a/arch/riscv/include/asm/kvm_gstage.h b/arch/riscv/include/asm/kvm_gstage.h
>> >> @@ -75,4 +76,40 @@ void kvm_riscv_gstage_wp_range(struct kvm_gstage *gstage, gpa_t start, gpa_t end
>> >> +enum kvm_riscv_hgatp_mode_bit {
>> >> +	HGATP_MODE_SV39X4_BIT = 0,
>> >> +	HGATP_MODE_SV48X4_BIT = 1,
>> >> +	HGATP_MODE_SV57X4_BIT = 2,
>> >
>> >I think it's a bit awkward to pass 9 when selecting the hgatp mode, but
>> >then look for bit 0 when detecting it...
>> >Why not to use the RVI defined values for this UABI as well?
>> >
>> >There are only 16 possible hgatp.mode values, so we're fine storing them
>> >in a bitmap even on RV32.
>>
>> I think this is a good point.
>>
>> Using logical bits 0/1/2 is indeed less intuitive than testing
>> BIT(HGATP_MODE_SV39X4) when userspace passes the architectural HGATP.MODE
>> encoding.
>>
>> However, if we use “HGATP.MODE encoding as bit index”, we need to export
>> those encodings to userspace. Today HGATP_MODE_* are not part of the
>> UAPI, so userspace would need to hardcode magic numbers.
>>
>> So if we go with this approach, I’ll add UAPI definitions for the HGATP
>> mode encodings (e.g. #define KVM_RISCV_HGATP_MODE_SV39X4_BIT  8, etc.) and
>> then define the returned bitmask as BIT(mode).
>
>The best part of Radim's suggestion is that there is no need to add the
>bits to UAPI. We can write in the documentation for the capability that
>the mode values match the spec. kvm userspace can then just look at the
>spec to determine those values and create its own defines (which QEMU,
>for example, has certainly already done).

Makes sense, thanks.

If we use the architectural HGATP.MODE encoding as the bit index, we can
indeed avoid adding any extra *_BIT or mode constants to the UAPI.

Not sure why my replies didn’t go through yesterday.

Thanks for the review. I’ll incorporate this feedback as well as your
other suggestions and address them in the next revision of the series.
>
>Thanks,
>drew

Thanks,
Fangyu