linux-kernel - Re: [PATCH] KVM: arm64: nv: Fix MI line level calculation in vgic_v3_nested_update

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: <w6yfzeynleghp7sxwscbrq6ko4244c5iphtqw6e4ar5dndqs3m@37jouplavkpw>
Date: Thu, 26 Jun 2025 13:57:17 +0800
From: Wei-Lin Chang <r09922117@...e.ntu.edu.tw>
To: Marc Zyngier <maz@...nel.org>
Cc: linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, Oliver Upton <oliver.upton@...ux.dev>, 
	Joey Gouly <joey.gouly@....com>, Suzuki K Poulose <suzuki.poulose@....com>, 
	Zenghui Yu <yuzenghui@...wei.com>, Catalin Marinas <catalin.marinas@....com>, 
	Will Deacon <will@...nel.org>, Jintack Lim <jintack@...columbia.edu>, 
	Christoffer Dall <christoffer.dall@....com>
Subject: Re: [PATCH] KVM: arm64: nv: Fix MI line level calculation in
 vgic_v3_nested_update_mi()

On Wed, Jun 25, 2025 at 05:45:06PM +0100, Marc Zyngier wrote:
> On Wed, 25 Jun 2025 09:47:09 +0100,
> Wei-Lin Chang <r09922117@...e.ntu.edu.tw> wrote:
> > 
> > The state of the vcpu's MI line should be asserted when its
> > ICH_HCR_EL2.En is set and ICH_MISR_EL2 is non-zero. Using bitwise AND
> > (&=) directly for this calculation will not give us the correct result
> > when the LSB of the vcpu's ICH_MISR_EL2 isn't set. Correct this by first
> > adjusting the return value of vgic_v3_get_misr() into 1 if it is
> > non-zero.
> > 
> > Signed-off-by: Wei-Lin Chang <r09922117@...e.ntu.edu.tw>
> > ---
> >  arch/arm64/kvm/vgic/vgic-v3-nested.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3-nested.c b/arch/arm64/kvm/vgic/vgic-v3-nested.c
> > index 4f6954c30674..ebffad632fd2 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3-nested.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3-nested.c
> > @@ -400,7 +400,7 @@ void vgic_v3_nested_update_mi(struct kvm_vcpu *vcpu)
> >  
> >  	level  = __vcpu_sys_reg(vcpu, ICH_HCR_EL2) & ICH_HCR_EL2_En;
> >  	if (level)
> > -		level &= vgic_v3_get_misr(vcpu);
> > +		level &= !!vgic_v3_get_misr(vcpu);
> >  	kvm_vgic_inject_irq(vcpu->kvm, vcpu,
> >  			    vcpu->kvm->arch.vgic.mi_intid, level, vcpu);
> >  }
> 
> Very well spotted, once more. Where were you when I posted all these
> patches? ;-)

Thanks ;)

> 
> We could make it even clearer with this:
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-v3-nested.c b/arch/arm64/kvm/vgic/vgic-v3-nested.c
> index a50fb7e6841f7..679aafe77de2e 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3-nested.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3-nested.c
> @@ -401,9 +401,7 @@ void vgic_v3_nested_update_mi(struct kvm_vcpu *vcpu)
>  {
>  	bool level;
>  
> -	level  = __vcpu_sys_reg(vcpu, ICH_HCR_EL2) & ICH_HCR_EL2_En;
> -	if (level)
> -		level &= vgic_v3_get_misr(vcpu);
> +	level = (__vcpu_sys_reg(vcpu, ICH_HCR_EL2) & ICH_HCR_EL2_En) && vgic_v3_get_misr(vcpu);
>  	kvm_vgic_inject_irq(vcpu->kvm, vcpu,
>  			    vcpu->kvm->arch.vgic.mi_intid, level, vcpu);
>  }
> 
> If you're OK with it, I'll use this, keeping your authorship of
> course.

Yes this is more straightforward, with this the commit message should be
rephrased too. I'm ok with keeping my authorship but really feel free to
adjust if you think some other credit is more appropriate.
I'm just glad that I'm able to help a little :)

Thanks,
Wei-Lin Chang

> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.