On Fri, Sep 15, 2023 at 2:23 PM Leonardo Bras <leobras@xxxxxxxxxx> wrote:
>
> On Sun, Sep 10, 2023 at 04:29:07AM -0400, guoren@xxxxxxxxxx wrote:
> > From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> >
> > Implement pv_kick with SBI implementation, and add SBI_EXT_PVLOCK
> > extension detection.
> >
> > Signed-off-by: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Guo Ren <guoren@xxxxxxxxxx>
> > ---
> > arch/riscv/include/asm/sbi.h | 6 ++++++
> > arch/riscv/kernel/qspinlock_paravirt.c | 7 ++++++-
> > 2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> > index e0233b3d7a5f..3533f8d4f3e2 100644
> > --- a/arch/riscv/include/asm/sbi.h
> > +++ b/arch/riscv/include/asm/sbi.h
> > @@ -30,6 +30,7 @@ enum sbi_ext_id {
> > SBI_EXT_HSM = 0x48534D,
> > SBI_EXT_SRST = 0x53525354,
> > SBI_EXT_PMU = 0x504D55,
> > + SBI_EXT_PVLOCK = 0xAB0401,
> >
> > /* Experimentals extensions must lie within this range */
> > SBI_EXT_EXPERIMENTAL_START = 0x08000000,
> > @@ -243,6 +244,11 @@ enum sbi_pmu_ctr_type {
> > /* Flags defined for counter stop function */
> > #define SBI_PMU_STOP_FLAG_RESET (1 << 0)
> >
> > +/* SBI PVLOCK (kick cpu out of wfi) */
> > +enum sbi_ext_pvlock_fid {
> > + SBI_EXT_PVLOCK_KICK_CPU = 0,
> > +};
> > +
> > #define SBI_SPEC_VERSION_DEFAULT 0x1
> > #define SBI_SPEC_VERSION_MAJOR_SHIFT 24
> > #define SBI_SPEC_VERSION_MAJOR_MASK 0x7f
> > diff --git a/arch/riscv/kernel/qspinlock_paravirt.c b/arch/riscv/kernel/qspinlock_paravirt.c
> > index a0ad4657f437..571626f350be 100644
> > --- a/arch/riscv/kernel/qspinlock_paravirt.c
> > +++ b/arch/riscv/kernel/qspinlock_paravirt.c
> > @@ -11,6 +11,8 @@
> >
> > void pv_kick(int cpu)
> > {
> > + sbi_ecall(SBI_EXT_PVLOCK, SBI_EXT_PVLOCK_KICK_CPU,
> > + cpuid_to_hartid_map(cpu), 0, 0, 0, 0, 0);
> > return;
> > }
> >
> > @@ -25,7 +27,7 @@ void pv_wait(u8 *ptr, u8 val)
> > if (READ_ONCE(*ptr) != val)
> > goto out;
> >
> > - /* wait_for_interrupt(); */
> > + wait_for_interrupt();
> > out:
> > local_irq_restore(flags);
> > }
> > @@ -62,6 +64,9 @@ void __init pv_qspinlock_init(void)
> > if(sbi_get_firmware_id() != SBI_EXT_BASE_IMPL_ID_KVM)
> > return;
> >
> > + if (!sbi_probe_extension(SBI_EXT_PVLOCK))
> > + return;
> > +
> > pr_info("PV qspinlocks enabled\n");
> > __pv_init_lock_hash();
> >
> > --
> > 2.36.1
> >
>
> IIUC this PVLOCK extension is now a requirement to use pv_qspinlock(), and
> it allows a cpu to use an instruction to wait for interrupt in pv_wait(),
> and kicks it out of this wait using a new sbi_ecall() on pv_kick().
Yes.
>
> Overall it LGTM, but would be nice to have the reference doc in the commit
> msg. I end up inferring some of the inner workings by your implementation,
> which is not ideal for reviewing.
I would improve the commit msg in the next version of patch.
>
> If understanding above is right,
> Reviewed-by: Leonardo Bras <leobras@xxxxxxxxxx>
>
> Thanks!
> Leo
>
--
Best Regards
Guo Ren
