[PATCH] RISC-V: KVM: Fix comments in kvm_riscv_vcpu_isa_disable_allowed

[PATCH] RISC-V: KVM: Fix comments in kvm_riscv_vcpu_isa_disable_allowed

[Chao Du]

The comments for EXT_SVADE are opposite with the codes. Fix it to avoid
confusion.

Signed-off-by: Chao Du <duchao@eswincomputing.com>
---
 arch/riscv/kvm/vcpu_onereg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index f6d27b59c641..6df41794e346 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -203,7 +203,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
 	case KVM_RISCV_ISA_EXT_SVADE:
 		/*
 		 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
-		 * Svade is not allowed to disable when the platform use Svade.
+		 * Svade is allowed to disable when the platform use Svade.
 		 */
 		return arch_has_hw_pte_young();
 	default:
-- 
2.34.1


-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH] RISC-V: KVM: Fix comments in kvm_riscv_vcpu_isa_disable_allowed

[Andrew Jones]

On Thu, Feb 20, 2025 at 07:49:05AM +0000, Chao Du wrote:
> The comments for EXT_SVADE are opposite with the codes. Fix it to avoid
> confusion.
> 
> Signed-off-by: Chao Du <duchao@eswincomputing.com>
> ---
>  arch/riscv/kvm/vcpu_onereg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index f6d27b59c641..6df41794e346 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c
> @@ -203,7 +203,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
>  	case KVM_RISCV_ISA_EXT_SVADE:
>  		/*
>  		 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
> -		 * Svade is not allowed to disable when the platform use Svade.
> +		 * Svade is allowed to disable when the platform use Svade.
>  		 */

It was correct (but confusing) before this change. When
arch_has_hw_pte_young() returns true, that means we can use
SVADU (which is !SVADE). If we don't have SVADU, then we must
be using SVADE, and therefore can't disable it.

How about

/*
 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
 * Svade can't be disabled unless we support Svadu.
 */

Thanks,
drew

>  		return arch_has_hw_pte_young();
>  	default:
> -- 
> 2.34.1
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [PATCH] RISC-V: KVM: Fix comments in kvm_riscv_vcpu_isa_disable_allowed

[Chao Du]

On 2025-02-20 23:54, Andrew Jones <ajones@ventanamicro.com> wrote:
> On Thu, Feb 20, 2025 at 07:49:05AM +0000, Chao Du wrote:
> > The comments for EXT_SVADE are opposite with the codes. Fix it to avoid
> > confusion.
> > 
> > Signed-off-by: Chao Du <duchao@eswincomputing.com>
> > ---
> >  arch/riscv/kvm/vcpu_onereg.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> > index f6d27b59c641..6df41794e346 100644
> > --- a/arch/riscv/kvm/vcpu_onereg.c
> > +++ b/arch/riscv/kvm/vcpu_onereg.c
> > @@ -203,7 +203,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
> >  	case KVM_RISCV_ISA_EXT_SVADE:
> >  		/*
> >  		 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
> > -		 * Svade is not allowed to disable when the platform use Svade.
> > +		 * Svade is allowed to disable when the platform use Svade.
> >  		 */
> 
> It was correct (but confusing) before this change. When
> arch_has_hw_pte_young() returns true, that means we can use
> SVADU (which is !SVADE). If we don't have SVADU, then we must
> be using SVADE, and therefore can't disable it.
> 

Thanks for the clarification. 
I had some misunderstandings about the relation between SVADU
and SVADE.

> How about
> 
> /*
>  * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
>  * Svade can't be disabled unless we support Svadu.
>  */

Yeah, that's better.

Thanks,
Chao

> 
> Thanks,
> drew
> 
> >  		return arch_has_hw_pte_young();
> >  	default:
> > -- 
> > 2.34.1
> > 
> > 
> > -- 
> > kvm-riscv mailing list
> > kvm-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kvm-riscv
-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv