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

[Huacai Chen]

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

Huacai

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