| Message ID | 20230910082911.3378782-14-guoren@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Add Native/Paravirt qspinlock support | expand |
On Sun, Sep 10, 2023 at 04:29:07AM -0400, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > Implement pv_kick with SBI implementation, and add SBI_EXT_PVLOCK > extension detection. > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@kernel.org> > --- > 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(). 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. If understanding above is right, Reviewed-by: Leonardo Bras <leobras@redhat.com> Thanks! Leo
On Fri, Sep 15, 2023 at 2:23 PM Leonardo Bras <leobras@redhat.com> wrote: > > On Sun, Sep 10, 2023 at 04:29:07AM -0400, guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > Implement pv_kick with SBI implementation, and add SBI_EXT_PVLOCK > > extension detection. > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > Signed-off-by: Guo Ren <guoren@kernel.org> > > --- > > 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@redhat.com> > > Thanks! > Leo >
On Sun, Sep 17, 2023 at 11:06:48PM +0800, Guo Ren wrote: > On Fri, Sep 15, 2023 at 2:23 PM Leonardo Bras <leobras@redhat.com> wrote: > > > > On Sun, Sep 10, 2023 at 04:29:07AM -0400, guoren@kernel.org wrote: > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > > > Implement pv_kick with SBI implementation, and add SBI_EXT_PVLOCK > > > extension detection. > > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > > Signed-off-by: Guo Ren <guoren@kernel.org> > > > --- > > > 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. Thx! Leo > > > > > If understanding above is right, > > Reviewed-by: Leonardo Bras <leobras@redhat.com> > > > > Thanks! > > Leo > > > > > -- > Best Regards > Guo Ren >
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();