linux-kernel - Re: Re: [PATCH v3 1/2] RISC-V: KVM: Support runtime configuration for per-VM's HGATP mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20260130132458.16367-1-fangyu.yu@linux.alibaba.com>
Date: Fri, 30 Jan 2026 21:24:58 +0800
From: fangyu.yu@...ux.alibaba.com
To: radim.krcmar@....qualcomm.com
Cc: ajones@...tanamicro.com,
	alex@...ti.fr,
	andrew.jones@....qualcomm.com,
	anup@...infault.org,
	aou@...s.berkeley.edu,
	atish.patra@...ux.dev,
	corbet@....net,
	fangyu.yu@...ux.alibaba.com,
	guoren@...nel.org,
	kvm-riscv@...ts.infradead.org,
	kvm@...r.kernel.org,
	linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-riscv@...ts.infradead.org,
	palmer@...belt.com,
	pbonzini@...hat.com,
	pjw@...nel.org
Subject: Re: Re: [PATCH v3 1/2] RISC-V: KVM: Support runtime configuration for per-VM's HGATP mode 

>> From: Fangyu Yu <fangyu.yu@...ux.alibaba.com>
>>
>> Introduces one per-VM architecture-specific fields to support runtime
>> configuration of the G-stage page table format:
>>
>> - kvm->arch.kvm_riscv_gstage_pgd_levels: the corresponding number of page
>>   table levels for the selected mode.
>>
>> These fields replace the previous global variables
>> kvm_riscv_gstage_mode and kvm_riscv_gstage_pgd_levels, enabling different
>> virtual machines to independently select their G-stage page table format
>> instead of being forced to share the maximum mode detected by the kernel
>> at boot time.
>>
>> Signed-off-by: Fangyu Yu <fangyu.yu@...ux.alibaba.com>
>> ---
>> diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
>> @@ -87,6 +87,22 @@ struct kvm_vcpu_stat {
>>  struct kvm_arch_memory_slot {
>>  };
>>  
>> +static inline unsigned long kvm_riscv_gstage_mode(unsigned long pgd_levels)
>> +{
>> +	switch (pgd_levels) {
>> +	case 2:
>> +		return HGATP_MODE_SV32X4;
>> +	case 3:
>> +		return HGATP_MODE_SV39X4;
>> +	case 4:
>> +		return HGATP_MODE_SV48X4;
>> +	case 5:
>> +		return HGATP_MODE_SV57X4;
>> +	default:
>> +		return HGATP_MODE_OFF;
>
>I think default should be an internal error.
>We can do "case 0: return HGATP_MODE_OFF;", or just error it too since
>KVM shouldn't ever ask for mode without protection anyway.

Good point. Returning HGATP_MODE_OFF in the default case would hide an
internal bug (unexpected pgd_levels). I’ll treat it as an internal error
instead, Something like:
    default:
        WARN_ON_ONCE(1);
        return HGATP_MODE_OFF;
    }

>> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
>> @@ -319,41 +321,48 @@ void __init kvm_riscv_gstage_mode_detect(void)
>> +unsigned long kvm_riscv_gstage_gpa_bits(struct kvm_arch *ka)
>> +{
>> +	return (HGATP_PAGE_SHIFT +
>> +		ka->kvm_riscv_gstage_pgd_levels * kvm_riscv_gstage_index_bits +
>> +		kvm_riscv_gstage_pgd_xbits);
>> +}
>> +
>> +gpa_t kvm_riscv_gstage_gpa_size(struct kvm_arch *ka)
>> +{
>> +	return BIT_ULL(kvm_riscv_gstage_gpa_bits(ka));
>> +}
>
>Please define these two functions as static inline in the header files.
>They used to be just macros there, so it'd be safer not put LTO into the
>equation.

Agreed.

>> diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
>> @@ -105,17 +105,17 @@ static int __init riscv_kvm_init(void)
>>  		return rc;
>>  
>>  	kvm_riscv_gstage_mode_detect();
>> -	switch (kvm_riscv_gstage_mode) {
>> -	case HGATP_MODE_SV32X4:
>> +	switch (kvm_riscv_gstage_max_pgd_levels) {
>> +	case 2:
>>  		str = "Sv32x4";
>>  		break;
>> -	case HGATP_MODE_SV39X4:
>> +	case 3:
>>  		str = "Sv39x4";
>>  		break;
>> -	case HGATP_MODE_SV48X4:
>> +	case 4:
>>  		str = "Sv48x4";
>>  		break;
>> -	case HGATP_MODE_SV57X4:
>> +	case 5:
>>  		str = "Sv57x4";
>>  		break;
>>  	default:
>> @@ -164,7 +164,7 @@ static int __init riscv_kvm_init(void)
>>  			 (rc) ? slist : "no features");
>>  	}
>>  
>> -	kvm_info("using %s G-stage page table format\n", str);
>> +	kvm_info("Max G-stage page table format %s\n", str);
>
>Fun fact: the ISA doesn't define the same hierarchy for hgatp modes as
>it does for satp modes, so we could have just Sv57x4 and nothing below.
>
>We could do just with a code comment that we're assuming vendors will do
>better, but I'd rather not introduce more assumptions...
>I think the easiest would be to kvm_riscv_gstage_mode_detect() levels in
>reverse and stop on the first one that is not supported.
>(I'll reply with a patch later.)

Please refer to the discussion here:
https://github.com/riscv/riscv-isa-manual/issues/2208
If Sv57x4 is implemented, then Sv48x4 and Sv39x4 must also be implemented.

>Thanks.

Thanks,
Fangyu