Re: [PATCH v5 3/6] LoongArch: KVM: Use existing macro about interrupt bit mask

[Huacai Chen]

On Wed, Jun 10, 2026 at 5:01 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
>
>
>
> On 2026/6/6 下午6:03, Huacai Chen wrote:
> > On Thu, Jun 4, 2026 at 9:51 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> >>
> >>
> >>
> >> On 2026/6/3 下午10:23, Huacai Chen wrote:
> >>> On Tue, Jun 2, 2026 at 4:13 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2026/6/2 下午1:01, Huacai Chen wrote:
> >>>>> On Tue, Jun 2, 2026 at 9:39 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 2026/6/1 下午9:50, Huacai Chen wrote:
> >>>>>>> Hi, Bibo,
> >>>>>>>
> >>>>>>> On Tue, May 26, 2026 at 8:53 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> >>>>>>>>
> >>>>>>>> With interrupt post, register CSR_GINTC and CSR_ESTAT is used, and
> >>>>>>>> CSR_ESTAT is used for percpu interrupt injection and CSR_GINTC is for
> >>>>>>>> external hardware interrupt injection.
> >>>>>>>>
> >>>>>>>> Here use existing macro about interrupt bit of register CSR_GINTC and
> >>>>>>>> CSR_ESTAT, rather than hard coded constant value.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
> >>>>>>>> ---
> >>>>>>>>      arch/loongarch/include/asm/kvm_vcpu.h | 43 ++++++++++++++++++---------
> >>>>>>>>      1 file changed, 29 insertions(+), 14 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/loongarch/include/asm/kvm_vcpu.h b/arch/loongarch/include/asm/kvm_vcpu.h
> >>>>>>>> index 3784ab4ccdb5..efe26b04b35f 100644
> >>>>>>>> --- a/arch/loongarch/include/asm/kvm_vcpu.h
> >>>>>>>> +++ b/arch/loongarch/include/asm/kvm_vcpu.h
> >>>>>>>> @@ -10,22 +10,37 @@
> >>>>>>>>      #include <asm/loongarch.h>
> >>>>>>>>
> >>>>>>>>      /* Controlled by 0x5 guest estat */
> >>>>>>>> -#define CPU_SIP0                       (_ULCAST_(1))
> >>>>>>>> -#define CPU_SIP1                       (_ULCAST_(1) << 1)
> >>>>>>>> -#define CPU_PMU                                (_ULCAST_(1) << 10)
> >>>>>>>> -#define CPU_TIMER                      (_ULCAST_(1) << 11)
> >>>>>>>> -#define CPU_IPI                                (_ULCAST_(1) << 12)
> >>>>>>>> -#define CPU_AVEC                        (_ULCAST_(1) << 14)
> >>>>>>>> +#define CPU_SIP0                       BIT(INT_SWI0)
> >>>>>>>> +#define CPU_SIP1                       BIT(INT_SWI1)
> >>>>>>>> +#define CPU_HWI0                       BIT(INT_HWI0)
> >>>>>>>> +#define CPU_HWI1                       BIT(INT_HWI1)
> >>>>>>>> +#define CPU_HWI2                       BIT(INT_HWI2)
> >>>>>>>> +#define CPU_HWI3                       BIT(INT_HWI3)
> >>>>>>>> +#define CPU_HWI4                       BIT(INT_HWI4)
> >>>>>>>> +#define CPU_HWI5                       BIT(INT_HWI5)
> >>>>>>>> +#define CPU_HWI6                       BIT(INT_HWI6)
> >>>>>>>> +#define CPU_HWI7                       BIT(INT_HWI7)
> >>>>>>>> +#define CPU_PMU                                BIT(INT_PCOV)
> >>>>>>>> +#define CPU_TIMER                      BIT(INT_TI)
> >>>>>>>> +#define CPU_IPI                                BIT(INT_IPI)
> >>>>>>>> +#define CPU_AVEC                       BIT(INT_AVEC)
> >>>>>>>> +#define KVM_ESTAT_IRQ_MASK             (CPU_SIP0 | CPU_SIP1 | CPU_PMU | CPU_TIMER \
> >>>>>>>> +                                               | CPU_IPI | CPU_AVEC)
> >>>>>>>> +#define KVM_ESTAT_HWI_MASK             (CPU_HWI0 | CPU_HWI1 | CPU_HWI2 | CPU_HWI3 \
> >>>>>>>> +                                               | CPU_HWI4 | CPU_HWI5 | CPU_HWI6 | CPU_HWI7)
> >>>>>>>>
> >>>>>>>>      /* Controlled by 0x52 guest exception VIP aligned to estat bit 5~12 */
> >>>>>>>> -#define CPU_IP0                                (_ULCAST_(1))
> >>>>>>>> -#define CPU_IP1                                (_ULCAST_(1) << 1)
> >>>>>>>> -#define CPU_IP2                                (_ULCAST_(1) << 2)
> >>>>>>>> -#define CPU_IP3                                (_ULCAST_(1) << 3)
> >>>>>>>> -#define CPU_IP4                                (_ULCAST_(1) << 4)
> >>>>>>>> -#define CPU_IP5                                (_ULCAST_(1) << 5)
> >>>>>>>> -#define CPU_IP6                                (_ULCAST_(1) << 6)
> >>>>>>>> -#define CPU_IP7                                (_ULCAST_(1) << 7)
> >>>>>>>> +#define GINTC_VIP_DELTA                        (INT_HWI0 - CSR_GINTC_VIP_SHIFT)
> >>>>>>> Don't use too many wrappers, just use INT_HWI0 instead of
> >>>>>>> GINTC_VIP_DELTA in this series is perfect. In other places INT_SWI0 is
> >>>>>>> also used for shifts.
> >>>>>>
> >>>>>> The above macro definition is a little complicated, I do not know how to
> >>>>>> optimize it. do you mean remove GINTC_VIP_DELTA just use 2 such as:
> >>>>>> #define CPU_IP0                       BIT(INT_HWI0 - 2)
> >>>>> CSR_GINTC_VIP_SHIFT is 0 and GINTC_VIP_DELTA is equal to INT_HWI0. I
> >>>>> mean remove the GINTC_VIP_DELTA definition and use INT_HWI0 instead.
> >>>> For macro definition such CPU_IP0, GINTC_VIP_DELTA can be removed. And
> >>>> define it as follows directly.
> >>>> #define CPU_IP0                       BIT(0)
> >>>>
> >>>> However GINTC_VIP_DELTA is still useful, because bitmap with HWI0-HWI7
> >>>> in estat register starts from INT_HWI0, however with register gintc it
> >>>> starts from CSR_GINTC_VIP_SHIFT.
> >>>>
> >>>> For example with _kvm_getcsr() API, it is to get interrupt status. Here
> >>>> is the example code.
> >>>> gintc = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_GINTC)& KVM_GINTC_IRQ_MASK;
> >>>> estat = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_ESTAT) &~KVM_ESTAT_HWI_MASK;
> >>>> *val = estat | (gintc << GINTC_VIP_DELTA);
> >>>>
> >>>> Remove GINTC_VIP_DELTA and use INT_HWI0 directly, it will be
> >>>> *val = estat | (gintc << INT_HWI0);
> >>>>
> >>>> I do not know what is the meaning about "gintc << INT_HWI0", especially
> >>>> if the value of CSR_GINTC_VIP_SHIFT is no zero.
> >>> In your Patch-2 there is "clear_csr_estat(1 << INT_SWI0)", INT_SW0 and
> >>> INT_HW0 are good friends. Why "1 << INT_SWI0" is acceptable but
> >>> "gintc << INT_HWI0" is not?
> >>
> >> The bit definition INT_SWI0 is for ESTAT register, is that right?
> >>
> >> The bitmap mask KVM_GINTC_IRQ_MASK is for GINTC register, to convert to
> >> bitmap mask of ESTAT,  should not be (gintc >> CSR_GINTC_VIP_SHIFT) <<
> >> INT_HWI0? why CSR_GINTC_VIP_SHIFT is discarded here, just because it is
> >> zero?
> > Yes, this is a reason,  but if you insist on, the macro can be
> > defined, but the name can be changed.
> yes, I want to use one macro describe the delta value.
>
> >
> > In theory, it is VIP_DELTA_BETWEEN_ESTAT_AND_GINTC, but we can just
> > use VIP_DELTA for short because CPU_IP0, CPU_IP1 .... also don't have
> > a "GINTC_" prefix.
> yes, prefix GINTC_ should be removed. VIP_DELTA_BETWEEN_ESTAT_AND_GINTC
> is a little long, how about KVM_VIP_DELTA or others for short.
This is a definition for hardware, not for KVM, so I prefer VIP_DELTA,
but KVM_VIP_DELTA is also OK.

>
> >
> > And KVM_ESTAT_IRQ_MASK is not suitable. In theory SWI and HWI are both
> > IRQ, so you cannot use KVM_ESTAT_IRQ_MASK to stand for the rest IRQ
> > bits of KVM_ESTAT_HWI_MASK.
> >
> > Yes, KVM_ESTAT_SWI_MASK is also not suitable because TI, IPI are
> > "hardware interrupts" indeed, but KVM_ESTAT_SWI_MASK is still better
> > than KVM_ESTAT_IRQ_MASK.
> >
> > A even better distinction is using "internal interrupts" and "external
> > interrupts" to describe, so KVM_ESTAT_INTI_MASK and
> > KVM_ESTAT_EXTI_MASK can be considered.
> KVM_ESTAT_EXTI_MASK is ok for me. With KVM_ESTAT_INTI_MASK, how about
> KVM_ESTAT_PPI_MASK or KVM_ESTAT_CPU_MASK to emphasize percpu interrupt?
> I am not name specialist, just a suggestion, or other names will be better.
PPI is hard to understand, CPU loses the "interrupt" information, so I
think KVM_ESTAT_INTI_MASK is better.

Though INT in INTI may be misunderstood, but it is defined near EXTI
so I think we needn't worry.


Huacai

>
> Regards
> Bibo Mao
> >
> >
> > Huacai
> >
> >>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo Mao
> >>>>>
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>>
> >>>>>> or something like this:
> >>>>>> #define CPU_IP0                       (CPU_HWI0 >> GINTC_VIP_DELTA)
> >>>>>>
> >>>>>> Regards
> >>>>>> Bibo Mao
> >>>>>>>
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>>> +#define CPU_IP0                                BIT(INT_HWI0 - GINTC_VIP_DELTA)
> >>>>>>>> +#define CPU_IP1                                BIT(INT_HWI1 - GINTC_VIP_DELTA)
> >>>>>>>> +#define CPU_IP2                                BIT(INT_HWI2 - GINTC_VIP_DELTA)
> >>>>>>>> +#define CPU_IP3                                BIT(INT_HWI3 - GINTC_VIP_DELTA)
> >>>>>>>> +#define CPU_IP4                                BIT(INT_HWI4 - GINTC_VIP_DELTA)
> >>>>>>>> +#define CPU_IP5                                BIT(INT_HWI5 - GINTC_VIP_DELTA)
> >>>>>>>> +#define CPU_IP6                                BIT(INT_HWI6 - GINTC_VIP_DELTA)
> >>>>>>>> +#define CPU_IP7                                BIT(INT_HWI7 - GINTC_VIP_DELTA)
> >>>>>>>> +#define KVM_GINTC_IRQ_MASK             (CPU_IP0 | CPU_IP1 | CPU_IP2 | CPU_IP3 \
> >>>>>>>> +                                               | CPU_IP4 | CPU_IP5 | CPU_IP6 | CPU_IP7)
> >>>>>>>>
> >>>>>>>>      #define MNSEC_PER_SEC                  (NSEC_PER_SEC >> 20)
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> 2.39.3
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
>
>