Re: [PATCH v6 5/6] LoongArch: KVM: Add valid bit check when set ESTAT CSR register

[Bibo Mao]

On 2026/6/12 下午4:39, Huacai Chen wrote:
> On Thu, Jun 11, 2026 at 9:01 PM Huacai Chen <chenhuacai@xxxxxxxxxx> wrote:
> > On Thu, Jun 11, 2026 at 8:55 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > On 2026/6/11 下午6:10, Huacai Chen wrote:
> > > > Hi, Bibo,
> > > >
> > > > On Thu, Jun 11, 2026 at 2:58 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > > > > When set ESTAT CSR register in function _kvm_setcsr(), valid bit check
> > > > > is added here. Also interrupt CPU_AVEC is checked by msgint feature.
> > > > >
> > > > > Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
> > > > > ---
> > > > >    arch/loongarch/kvm/vcpu.c | 25 ++++++++++++++++---------
> > > > >    1 file changed, 16 insertions(+), 9 deletions(-)
> > > > >
> > > > > diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> > > > > index e986146d2272..4f67eefbd4f1 100644
> > > > > --- a/arch/loongarch/kvm/vcpu.c
> > > > > +++ b/arch/loongarch/kvm/vcpu.c
> > > > > @@ -602,7 +602,7 @@ struct kvm_vcpu *kvm_get_vcpu_by_cpuid(struct kvm *kvm, int cpuid)
> > > > >
> > > > >    static int _kvm_getcsr(struct kvm_vcpu *vcpu, unsigned int id, u64 *val)
> > > > >    {
> > > > > -       unsigned long gintc;
> > > > > +       unsigned long gintc, estat;
> > > > >           struct loongarch_csrs *csr = vcpu->arch.csr;
> > > > >
> > > > >           if (get_gcsr_flag(id) & INVALID_GCSR)
> > > > > @@ -621,8 +621,9 @@ static int _kvm_getcsr(struct kvm_vcpu *vcpu, unsigned int id, u64 *val)
> > > > >                   preempt_enable();
> > > > >
> > > > >                   /* ESTAT IP0~IP7 get from GINTC */
> > > > > -               gintc = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_GINTC) & 0xff;
> > > > > -               *val = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_ESTAT) | (gintc << 2);
> > > > > +               gintc = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_GINTC) & KVM_GINTC_IRQ_MASK;
> > > > > +               estat = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_ESTAT) & ~KVM_ESTAT_EXTI_MASK;
> > > > > +               *val = estat | (gintc << VIP_DELTA);
> > > > >                   return 0;
> > > > >           }
> > > > >
> > > > > @@ -637,7 +638,8 @@ static int _kvm_getcsr(struct kvm_vcpu *vcpu, unsigned int id, u64 *val)
> > > > >
> > > > >    static int _kvm_setcsr(struct kvm_vcpu *vcpu, unsigned int id, u64 val)
> > > > >    {
> > > > > -       int ret = 0, gintc;
> > > > > +       int ret = 0;
> > > > > +       unsigned long gintc, estat;
> > > > >           struct loongarch_csrs *csr = vcpu->arch.csr;
> > > > >
> > > > >           if (get_gcsr_flag(id) & INVALID_GCSR)
> > > > > @@ -648,11 +650,16 @@ static int _kvm_setcsr(struct kvm_vcpu *vcpu, unsigned int id, u64 val)
> > > > >
> > > > >           if (id == LOONGARCH_CSR_ESTAT) {
> > > > >                   /* ESTAT IP0~IP7 inject through GINTC */
> > > > > -               gintc = (val >> 2) & 0xff;
> > > > > -               kvm_set_sw_gcsr(csr, LOONGARCH_CSR_GINTC, gintc);
> > > > > -
> > > > > -               gintc = val & ~(0xffUL << 2);
> > > > > -               kvm_set_sw_gcsr(csr, LOONGARCH_CSR_ESTAT, gintc);
> > > > > +               gintc = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_GINTC) & ~KVM_GINTC_IRQ_MASK;
> > > > > +               gintc |= (val >> VIP_DELTA) & KVM_GINTC_IRQ_MASK;
> > > > > +               kvm_write_sw_gcsr(csr, LOONGARCH_CSR_GINTC, gintc);
> > > > I think read & write can be combined, which means it can be simplified as:
> > > >                   gintc = (val >> VIP_DELTA) & KVM_GINTC_IRQ_MASK;
> > > >                   kvm_set_sw_gcsr(csr, LOONGARCH_CSR_GINTC, gintc);
> > > >
> > > > > +
> > > > > +               /* only set valid ESTAT bits */
> > > > > +               estat = val & ~KVM_ESTAT_EXTI_MASK;
> > > > > +               estat &= CSR_ESTAT_IS | CSR_ESTAT_EXC | CSR_ESTAT_ESUBCODE;
> > > > > +               if (!kvm_guest_has_msgint(&vcpu->arch))
> > > > > +                       estat &= ~CPU_AVEC;
> > > > > +               kvm_write_sw_gcsr(csr, LOONGARCH_CSR_ESTAT, estat);
> > > > I think write csr may lose bits from the original value, so
> > > >                   kvm_write_sw_gcsr(csr, LOONGARCH_CSR_ESTAT, estat)
> > > > should be
> > > >                   kvm_set_sw_gcsr(csr, LOONGARCH_CSR_ESTAT, estat)
> > > >
> > > > If you agree, I will change it when I apply because all others look good to me.
> > > At the beginning, it is kvm_set_sw_gcsr() with CSR_GINTC/CSR_ESTAT,
> > > later I change it with kvm_write_sw_gcsr(). With kvm_set_sw_gcsr() API,
> > > it is only set new interrupt, however does not clear the interrupt. So I
> > > change it with kvm_write_sw_gcsr() which will overwrite the interrupt bits.
> > >
> > > At the same time, bit field CSR_ESTAT_EXC | CSR_ESTAT_ESUBCODE is not
> > > suitable for bit set, overwrite its value is more suitable.
> > OK, then I will keep using kvm_write_sw_gcsr(), and please update the
> > selftest series if needed.
> Now all KVM patches are applied, you can double check it here.
> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-kvm
I test it on my 3C6000 Dual-way machine, it works well. Also there is no 
problem by reviewing code.

Thanks for your efforts.

Bibo Mao
> Huacai
>
> > Huacai
> >
> > > Regards
> > > Bibo Mao
> > >
> > > > Huacai
> > > >
> > > > >                   return ret;
> > > > >           }
> > > > > --
> > > > > 2.39.3