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

[Huacai Chen]

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.

Huacai

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