diff mbox series

[V11,13/17] RISC-V: paravirt: pvqspinlock: Add SBI implementation

Message ID 20230910082911.3378782-14-guoren@kernel.org (mailing list archive)
State Changes Requested
Headers show
Series riscv: Add Native/Paravirt qspinlock support | expand

Checks

Context Check Description
conchuod/tree_selection fail Failed to apply to next/pending-fixes, riscv/for-next or riscv/master

Commit Message

Guo Ren Sept. 10, 2023, 8:29 a.m. UTC 
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(-)

Comments

Leonardo Bras Sept. 15, 2023, 6:23 a.m. UTC | #1 
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
Guo Ren Sept. 17, 2023, 3:06 p.m. UTC | #2 
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
>
Leonardo Bras Sept. 19, 2023, 5:45 a.m. UTC | #3 
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 mbox series

Patch

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();