2022-05-28 19:39:25

by Atish Patra

[permalink] [raw]
Subject: [PATCH v4 0/4] Add Sstc extension support

This series implements Sstc extension support which was ratified recently.
Before the Sstc extension, an SBI call is necessary to generate timer
interrupts as only M-mode have access to the timecompare registers. Thus,
there is significant latency to generate timer interrupts at kernel.
For virtualized enviornments, its even worse as the KVM handles the SBI call
and uses a software timer to emulate the timecomapre register.

Sstc extension solves both these problems by defining a stimecmp/vstimecmp
at supervisor (host/guest) level. It allows kernel to program a timer and
recieve interrupt without supervisor execution enviornment (M-mode/HS mode)
intervention.

KVM directly updates the vstimecmp as well if the guest kernel invokes the SBI
call instead of updating stimecmp directly. This is required because KVM will
enable sstc extension if the hardware supports it unless the VMM explicitly
disables it for that guest. The hardware is expected to compare the
vstimecmp at every cycle if sstc is enabled and any stale value in vstimecmp
will lead to spurious timer interrupts. This also helps maintaining the
backward compatibility with older kernels.

Similary, the M-mode firmware(OpenSBI) uses stimecmp for older kernel
without sstc support as STIP bit in mip is read only for hardware with sstc.

The PATCH 1 & 2 enables the basic infrastructure around Sstc extension while
PATCH 3 lets kernel use the Sstc extension if it is available in hardware.
PATCH 4 implements the Sstc extension in KVM.

This series has been tested on Qemu(RV32 & RV64) with additional patches in
Qemu[2]. This series can also be found at [3].

Changes from v3->v4:
1. Rebased on 5.18-rc6
2. Unified vstimemp & next_cycles.
3. Addressed comments in PATCH 3 & 4.

Changes from v2->v3:
1. Dropped unrelated KVM fixes from this series.
2. Rebased on 5.18-rc3.

Changes from v1->v2:
1. Separate the static key from kvm usage
2. Makde the sstc specific static key local to the driver/clocksource
3. Moved the vstimecmp update code to the vcpu_timer
4. Used function pointers instead of static key to invoke vstimecmp vs
hrtimer at the run time. This will help in future for migration of vms
from/to sstc enabled hardware to non-sstc enabled hardware.
5. Unified the vstimer & timer to 1 timer as only one of them will be used
at runtime.

[1] https://drive.google.com/file/d/1m84Re2yK8m_vbW7TspvevCDR82MOBaSX/view
[2] https://github.com/atishp04/qemu/tree/sstc_v3
[3] https://github.com/atishp04/linux/tree/sstc_v4

Atish Patra (4):
RISC-V: Add SSTC extension CSR details
RISC-V: Enable sstc extension parsing from DT
RISC-V: Prefer sstc extension if available
RISC-V: KVM: Support sstc extension

arch/riscv/include/asm/csr.h | 11 ++
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/include/asm/kvm_vcpu_timer.h | 8 +-
arch/riscv/include/uapi/asm/kvm.h | 1 +
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 1 +
arch/riscv/kvm/main.c | 12 +-
arch/riscv/kvm/vcpu.c | 5 +-
arch/riscv/kvm/vcpu_timer.c | 144 +++++++++++++++++++++++-
drivers/clocksource/timer-riscv.c | 24 +++-
10 files changed, 198 insertions(+), 10 deletions(-)

--
2.25.1



2022-05-28 19:44:25

by Atish Patra

[permalink] [raw]
Subject: [PATCH v4 2/4] RISC-V: Enable sstc extension parsing from DT

The ISA extension framework now allows parsing any multi-letter
ISA extension.

Enable that for sstc extension.

Reviewed-by: Anup Patel <[email protected]>
Signed-off-by: Atish Patra <[email protected]>
---
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 1 +
3 files changed, 3 insertions(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 0734e42f74f2..25915eb60d61 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -52,6 +52,7 @@ extern unsigned long elf_hwcap;
*/
enum riscv_isa_ext_id {
RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
+ RISCV_ISA_EXT_SSTC,
RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
};

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ccb617791e56..ca0e4c0db17e 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -88,6 +88,7 @@ int riscv_of_parent_hartid(struct device_node *node)
*/
static struct riscv_isa_ext_data isa_ext_arr[] = {
__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
+ __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
__RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
};

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1b2d42d7f589..a214537c22f1 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -192,6 +192,7 @@ void __init riscv_fill_hwcap(void)
set_bit(*ext - 'a', this_isa);
} else {
SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
+ SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
}
#undef SET_ISA_EXT_MAP
}
--
2.25.1


2022-05-28 20:01:05

by Atish Patra

[permalink] [raw]
Subject: [PATCH v4 3/4] RISC-V: Prefer sstc extension if available

RISC-V ISA has sstc extension which allows updating the next clock event
via a CSR (stimecmp) instead of an SBI call. This should happen dynamically
if sstc extension is available. Otherwise, it will fallback to SBI call
to maintain backward compatibility.

Signed-off-by: Atish Patra <[email protected]>
---
drivers/clocksource/timer-riscv.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
index 1767f8bf2013..881d9335c92d 100644
--- a/drivers/clocksource/timer-riscv.c
+++ b/drivers/clocksource/timer-riscv.c
@@ -7,6 +7,9 @@
* either be read from the "time" and "timeh" CSRs, and can use the SBI to
* setup events, or directly accessed using MMIO registers.
*/
+
+#define pr_fmt(fmt) "timer: " fmt
+
#include <linux/clocksource.h>
#include <linux/clockchips.h>
#include <linux/cpu.h>
@@ -23,11 +26,24 @@
#include <asm/sbi.h>
#include <asm/timex.h>

+static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available);
+
static int riscv_clock_next_event(unsigned long delta,
struct clock_event_device *ce)
{
+ u64 next_tval = get_cycles64() + delta;
+
csr_set(CSR_IE, IE_TIE);
- sbi_set_timer(get_cycles64() + delta);
+ if (static_branch_likely(&riscv_sstc_available)) {
+#if defined(CONFIG_32BIT)
+ csr_write(CSR_STIMECMP, next_tval & 0xFFFFFFFF);
+ csr_write(CSR_STIMECMPH, next_tval >> 32);
+#else
+ csr_write(CSR_STIMECMP, next_tval);
+#endif
+ } else
+ sbi_set_timer(next_tval);
+
return 0;
}

@@ -165,6 +181,12 @@ static int __init riscv_timer_init_dt(struct device_node *n)
if (error)
pr_err("cpu hp setup state failed for RISCV timer [%d]\n",
error);
+
+ if (riscv_isa_extension_available(NULL, SSTC)) {
+ pr_info("Timer interrupt in S-mode is available via sstc extension\n");
+ static_branch_enable(&riscv_sstc_available);
+ }
+
return error;
}

--
2.25.1


2022-06-10 05:08:02

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] RISC-V: Prefer sstc extension if available

On Fri, May 27, 2022 at 9:59 AM Atish Patra <[email protected]> wrote:
>
> RISC-V ISA has sstc extension which allows updating the next clock event
> via a CSR (stimecmp) instead of an SBI call. This should happen dynamically
> if sstc extension is available. Otherwise, it will fallback to SBI call
> to maintain backward compatibility.
>
> Signed-off-by: Atish Patra <[email protected]>
> ---
> drivers/clocksource/timer-riscv.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 1767f8bf2013..881d9335c92d 100644
> --- a/drivers/clocksource/timer-riscv.c
> +++ b/drivers/clocksource/timer-riscv.c
> @@ -7,6 +7,9 @@
> * either be read from the "time" and "timeh" CSRs, and can use the SBI to
> * setup events, or directly accessed using MMIO registers.
> */
> +
> +#define pr_fmt(fmt) "timer: " fmt

The "timer: " prefix is too generic. I suggest to use "riscv-timer: "
as a prefix.

> +
> #include <linux/clocksource.h>
> #include <linux/clockchips.h>
> #include <linux/cpu.h>
> @@ -23,11 +26,24 @@
> #include <asm/sbi.h>
> #include <asm/timex.h>
>
> +static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available);
> +
> static int riscv_clock_next_event(unsigned long delta,
> struct clock_event_device *ce)
> {
> + u64 next_tval = get_cycles64() + delta;
> +
> csr_set(CSR_IE, IE_TIE);
> - sbi_set_timer(get_cycles64() + delta);
> + if (static_branch_likely(&riscv_sstc_available)) {
> +#if defined(CONFIG_32BIT)
> + csr_write(CSR_STIMECMP, next_tval & 0xFFFFFFFF);
> + csr_write(CSR_STIMECMPH, next_tval >> 32);
> +#else
> + csr_write(CSR_STIMECMP, next_tval);
> +#endif
> + } else
> + sbi_set_timer(next_tval);
> +
> return 0;
> }
>
> @@ -165,6 +181,12 @@ static int __init riscv_timer_init_dt(struct device_node *n)
> if (error)
> pr_err("cpu hp setup state failed for RISCV timer [%d]\n",
> error);
> +
> + if (riscv_isa_extension_available(NULL, SSTC)) {
> + pr_info("Timer interrupt in S-mode is available via sstc extension\n");
> + static_branch_enable(&riscv_sstc_available);
> + }
> +
> return error;
> }
>
> --
> 2.25.1
>

Apart from the minor comment above, this looks good to me.

Reviewed-by: Anup Patel <[email protected]>

Regards,
Anup

2022-06-10 05:55:45

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] Add Sstc extension support

On Fri, May 27, 2022 at 9:59 AM Atish Patra <[email protected]> wrote:
>
> This series implements Sstc extension support which was ratified recently.
> Before the Sstc extension, an SBI call is necessary to generate timer
> interrupts as only M-mode have access to the timecompare registers. Thus,
> there is significant latency to generate timer interrupts at kernel.
> For virtualized enviornments, its even worse as the KVM handles the SBI call
> and uses a software timer to emulate the timecomapre register.
>
> Sstc extension solves both these problems by defining a stimecmp/vstimecmp
> at supervisor (host/guest) level. It allows kernel to program a timer and
> recieve interrupt without supervisor execution enviornment (M-mode/HS mode)
> intervention.
>
> KVM directly updates the vstimecmp as well if the guest kernel invokes the SBI
> call instead of updating stimecmp directly. This is required because KVM will
> enable sstc extension if the hardware supports it unless the VMM explicitly
> disables it for that guest. The hardware is expected to compare the
> vstimecmp at every cycle if sstc is enabled and any stale value in vstimecmp
> will lead to spurious timer interrupts. This also helps maintaining the
> backward compatibility with older kernels.
>
> Similary, the M-mode firmware(OpenSBI) uses stimecmp for older kernel
> without sstc support as STIP bit in mip is read only for hardware with sstc.
>
> The PATCH 1 & 2 enables the basic infrastructure around Sstc extension while
> PATCH 3 lets kernel use the Sstc extension if it is available in hardware.
> PATCH 4 implements the Sstc extension in KVM.
>
> This series has been tested on Qemu(RV32 & RV64) with additional patches in
> Qemu[2]. This series can also be found at [3].
>
> Changes from v3->v4:
> 1. Rebased on 5.18-rc6
> 2. Unified vstimemp & next_cycles.
> 3. Addressed comments in PATCH 3 & 4.
>
> Changes from v2->v3:
> 1. Dropped unrelated KVM fixes from this series.
> 2. Rebased on 5.18-rc3.
>
> Changes from v1->v2:
> 1. Separate the static key from kvm usage
> 2. Makde the sstc specific static key local to the driver/clocksource
> 3. Moved the vstimecmp update code to the vcpu_timer
> 4. Used function pointers instead of static key to invoke vstimecmp vs
> hrtimer at the run time. This will help in future for migration of vms
> from/to sstc enabled hardware to non-sstc enabled hardware.
> 5. Unified the vstimer & timer to 1 timer as only one of them will be used
> at runtime.
>
> [1] https://drive.google.com/file/d/1m84Re2yK8m_vbW7TspvevCDR82MOBaSX/view
> [2] https://github.com/atishp04/qemu/tree/sstc_v3
> [3] https://github.com/atishp04/linux/tree/sstc_v4
>
> Atish Patra (4):
> RISC-V: Add SSTC extension CSR details
> RISC-V: Enable sstc extension parsing from DT
> RISC-V: Prefer sstc extension if available
> RISC-V: KVM: Support sstc extension

Please don't forget to CC kvm-riscv mailing list for KVM RISC-V patches.

We have a patchwork setup for KVM RISC-V will also miss a series if
patches are not CCed.

Regards,
Anup

>
> arch/riscv/include/asm/csr.h | 11 ++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/asm/kvm_vcpu_timer.h | 8 +-
> arch/riscv/include/uapi/asm/kvm.h | 1 +
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 1 +
> arch/riscv/kvm/main.c | 12 +-
> arch/riscv/kvm/vcpu.c | 5 +-
> arch/riscv/kvm/vcpu_timer.c | 144 +++++++++++++++++++++++-
> drivers/clocksource/timer-riscv.c | 24 +++-
> 10 files changed, 198 insertions(+), 10 deletions(-)
>
> --
> 2.25.1
>

2022-06-14 18:27:24

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] RISC-V: Prefer sstc extension if available

On Thu, Jun 9, 2022 at 9:41 PM Anup Patel <[email protected]> wrote:
>
> On Fri, May 27, 2022 at 9:59 AM Atish Patra <[email protected]> wrote:
> >
> > RISC-V ISA has sstc extension which allows updating the next clock event
> > via a CSR (stimecmp) instead of an SBI call. This should happen dynamically
> > if sstc extension is available. Otherwise, it will fallback to SBI call
> > to maintain backward compatibility.
> >
> > Signed-off-by: Atish Patra <[email protected]>
> > ---
> > drivers/clocksource/timer-riscv.c | 24 +++++++++++++++++++++++-
> > 1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index 1767f8bf2013..881d9335c92d 100644
> > --- a/drivers/clocksource/timer-riscv.c
> > +++ b/drivers/clocksource/timer-riscv.c
> > @@ -7,6 +7,9 @@
> > * either be read from the "time" and "timeh" CSRs, and can use the SBI to
> > * setup events, or directly accessed using MMIO registers.
> > */
> > +
> > +#define pr_fmt(fmt) "timer: " fmt
>
> The "timer: " prefix is too generic. I suggest to use "riscv-timer: "
> as a prefix.
>

Sured. Fixed in the next version.

> > +
> > #include <linux/clocksource.h>
> > #include <linux/clockchips.h>
> > #include <linux/cpu.h>
> > @@ -23,11 +26,24 @@
> > #include <asm/sbi.h>
> > #include <asm/timex.h>
> >
> > +static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available);
> > +
> > static int riscv_clock_next_event(unsigned long delta,
> > struct clock_event_device *ce)
> > {
> > + u64 next_tval = get_cycles64() + delta;
> > +
> > csr_set(CSR_IE, IE_TIE);
> > - sbi_set_timer(get_cycles64() + delta);
> > + if (static_branch_likely(&riscv_sstc_available)) {
> > +#if defined(CONFIG_32BIT)
> > + csr_write(CSR_STIMECMP, next_tval & 0xFFFFFFFF);
> > + csr_write(CSR_STIMECMPH, next_tval >> 32);
> > +#else
> > + csr_write(CSR_STIMECMP, next_tval);
> > +#endif
> > + } else
> > + sbi_set_timer(next_tval);
> > +
> > return 0;
> > }
> >
> > @@ -165,6 +181,12 @@ static int __init riscv_timer_init_dt(struct device_node *n)
> > if (error)
> > pr_err("cpu hp setup state failed for RISCV timer [%d]\n",
> > error);
> > +
> > + if (riscv_isa_extension_available(NULL, SSTC)) {
> > + pr_info("Timer interrupt in S-mode is available via sstc extension\n");
> > + static_branch_enable(&riscv_sstc_available);
> > + }
> > +
> > return error;
> > }
> >
> > --
> > 2.25.1
> >
>
> Apart from the minor comment above, this looks good to me.
>
> Reviewed-by: Anup Patel <[email protected]>
>
> Regards,
> Anup



--
Regards,
Atish

2022-06-14 20:22:07

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] Add Sstc extension support

On Thu, Jun 9, 2022 at 9:53 PM Anup Patel <[email protected]> wrote:
>
> On Fri, May 27, 2022 at 9:59 AM Atish Patra <[email protected]> wrote:
> >
> > This series implements Sstc extension support which was ratified recently.
> > Before the Sstc extension, an SBI call is necessary to generate timer
> > interrupts as only M-mode have access to the timecompare registers. Thus,
> > there is significant latency to generate timer interrupts at kernel.
> > For virtualized enviornments, its even worse as the KVM handles the SBI call
> > and uses a software timer to emulate the timecomapre register.
> >
> > Sstc extension solves both these problems by defining a stimecmp/vstimecmp
> > at supervisor (host/guest) level. It allows kernel to program a timer and
> > recieve interrupt without supervisor execution enviornment (M-mode/HS mode)
> > intervention.
> >
> > KVM directly updates the vstimecmp as well if the guest kernel invokes the SBI
> > call instead of updating stimecmp directly. This is required because KVM will
> > enable sstc extension if the hardware supports it unless the VMM explicitly
> > disables it for that guest. The hardware is expected to compare the
> > vstimecmp at every cycle if sstc is enabled and any stale value in vstimecmp
> > will lead to spurious timer interrupts. This also helps maintaining the
> > backward compatibility with older kernels.
> >
> > Similary, the M-mode firmware(OpenSBI) uses stimecmp for older kernel
> > without sstc support as STIP bit in mip is read only for hardware with sstc.
> >
> > The PATCH 1 & 2 enables the basic infrastructure around Sstc extension while
> > PATCH 3 lets kernel use the Sstc extension if it is available in hardware.
> > PATCH 4 implements the Sstc extension in KVM.
> >
> > This series has been tested on Qemu(RV32 & RV64) with additional patches in
> > Qemu[2]. This series can also be found at [3].
> >
> > Changes from v3->v4:
> > 1. Rebased on 5.18-rc6
> > 2. Unified vstimemp & next_cycles.
> > 3. Addressed comments in PATCH 3 & 4.
> >
> > Changes from v2->v3:
> > 1. Dropped unrelated KVM fixes from this series.
> > 2. Rebased on 5.18-rc3.
> >
> > Changes from v1->v2:
> > 1. Separate the static key from kvm usage
> > 2. Makde the sstc specific static key local to the driver/clocksource
> > 3. Moved the vstimecmp update code to the vcpu_timer
> > 4. Used function pointers instead of static key to invoke vstimecmp vs
> > hrtimer at the run time. This will help in future for migration of vms
> > from/to sstc enabled hardware to non-sstc enabled hardware.
> > 5. Unified the vstimer & timer to 1 timer as only one of them will be used
> > at runtime.
> >
> > [1] https://drive.google.com/file/d/1m84Re2yK8m_vbW7TspvevCDR82MOBaSX/view
> > [2] https://github.com/atishp04/qemu/tree/sstc_v3
> > [3] https://github.com/atishp04/linux/tree/sstc_v4
> >
> > Atish Patra (4):
> > RISC-V: Add SSTC extension CSR details
> > RISC-V: Enable sstc extension parsing from DT
> > RISC-V: Prefer sstc extension if available
> > RISC-V: KVM: Support sstc extension
>
> Please don't forget to CC kvm-riscv mailing list for KVM RISC-V patches.
>

Sorry. My scripts did not pick up kvm-riscv for some reason.
Fixed it.

> We have a patchwork setup for KVM RISC-V will also miss a series if
> patches are not CCed.
>
> Regards,
> Anup
>
> >
> > arch/riscv/include/asm/csr.h | 11 ++
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/kvm_vcpu_timer.h | 8 +-
> > arch/riscv/include/uapi/asm/kvm.h | 1 +
> > arch/riscv/kernel/cpu.c | 1 +
> > arch/riscv/kernel/cpufeature.c | 1 +
> > arch/riscv/kvm/main.c | 12 +-
> > arch/riscv/kvm/vcpu.c | 5 +-
> > arch/riscv/kvm/vcpu_timer.c | 144 +++++++++++++++++++++++-
> > drivers/clocksource/timer-riscv.c | 24 +++-
> > 10 files changed, 198 insertions(+), 10 deletions(-)
> >
> > --
> > 2.25.1
> >



--
Regards,
Atish