2023-10-12 05:17:10

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 0/8] RISC-V SBI debug console extension support

The SBI v2.0 specification is now frozen. The SBI v2.0 specification defines
SBI debug console (DBCN) extension which replaces the legacy SBI v0.1
functions sbi_console_putchar() and sbi_console_getchar().
(Refer v2.0-rc5 at https://github.com/riscv-non-isa/riscv-sbi-doc/releases)

This series adds support for SBI debug console (DBCN) extension in KVM RISC-V
and Linux RISC-V.

To try these patches with KVM RISC-V, use KVMTOOL from riscv_sbi_dbcn_v1
branch at: https://github.com/avpatel/kvmtool.git

These patches can also be found in the riscv_sbi_dbcn_v2 branch at:
https://github.com/avpatel/linux.git

Changes since v1:
- Remove use of #ifdef from PATCH4 and PATCH5 of the v1 series
- Improved commit description of PATCH3 in v1 series
- Introduced new PATCH3 in this series to allow some SBI extensions
(such as SBI DBCN) do to disabled by default so that older KVM user space
work fine and newer KVM user space have to explicitly opt-in for emulating
SBI DBCN.
- Introduced new PATCH5 in this series which adds inline version of
sbi_console_getchar() and sbi_console_putchar() for the case where
CONFIG_RISCV_SBI_V01 is disabled.

Anup Patel (7):
RISC-V: Add defines for SBI debug console extension
RISC-V: KVM: Change the SBI specification version to v2.0
RISC-V: KVM: Allow some SBI extensions to be disabled by default
RISC-V: KVM: Forward SBI DBCN extension to user-space
RISC-V: Add inline version of sbi_console_putchar/getchar() functions
tty/serial: Add RISC-V SBI debug console based earlycon
RISC-V: Enable SBI based earlycon support

Atish Patra (1):
tty: Add SBI debug console support to HVC SBI driver

arch/riscv/configs/defconfig | 1 +
arch/riscv/configs/rv32_defconfig | 1 +
arch/riscv/include/asm/kvm_vcpu_sbi.h | 7 ++-
arch/riscv/include/asm/sbi.h | 12 ++++
arch/riscv/include/uapi/asm/kvm.h | 1 +
arch/riscv/kvm/vcpu.c | 6 ++
arch/riscv/kvm/vcpu_sbi.c | 49 +++++++++-------
arch/riscv/kvm/vcpu_sbi_replace.c | 32 +++++++++++
drivers/tty/hvc/Kconfig | 2 +-
drivers/tty/hvc/hvc_riscv_sbi.c | 76 ++++++++++++++++++++++---
drivers/tty/serial/Kconfig | 2 +-
drivers/tty/serial/earlycon-riscv-sbi.c | 32 +++++++++--
12 files changed, 188 insertions(+), 33 deletions(-)

--
2.34.1


2023-10-12 05:17:17

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 1/8] RISC-V: Add defines for SBI debug console extension

We add SBI debug console extension related defines/enum to the
asm/sbi.h header.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/sbi.h | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 5b4a1bf5f439..12dfda6bb924 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_DBCN = 0x4442434E,

/* Experimentals extensions must lie within this range */
SBI_EXT_EXPERIMENTAL_START = 0x08000000,
@@ -236,6 +237,12 @@ enum sbi_pmu_ctr_type {
/* Flags defined for counter stop function */
#define SBI_PMU_STOP_FLAG_RESET (1 << 0)

+enum sbi_ext_dbcn_fid {
+ SBI_EXT_DBCN_CONSOLE_WRITE = 0,
+ SBI_EXT_DBCN_CONSOLE_READ = 1,
+ SBI_EXT_DBCN_CONSOLE_WRITE_BYTE = 2,
+};
+
#define SBI_SPEC_VERSION_DEFAULT 0x1
#define SBI_SPEC_VERSION_MAJOR_SHIFT 24
#define SBI_SPEC_VERSION_MAJOR_MASK 0x7f
--
2.34.1

2023-10-12 05:17:18

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 7/8] tty: Add SBI debug console support to HVC SBI driver

From: Atish Patra <[email protected]>

RISC-V SBI specification supports advanced debug console
support via SBI DBCN extension.

Extend the HVC SBI driver to support it.

Signed-off-by: Atish Patra <[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
drivers/tty/hvc/Kconfig | 2 +-
drivers/tty/hvc/hvc_riscv_sbi.c | 76 ++++++++++++++++++++++++++++++---
2 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index 4f9264d005c0..6e05c5c7bca1 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -108,7 +108,7 @@ config HVC_DCC_SERIALIZE_SMP

config HVC_RISCV_SBI
bool "RISC-V SBI console support"
- depends on RISCV_SBI_V01
+ depends on RISCV_SBI
select HVC_DRIVER
help
This enables support for console output via RISC-V SBI calls, which
diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
index 31f53fa77e4a..da318d7f55c5 100644
--- a/drivers/tty/hvc/hvc_riscv_sbi.c
+++ b/drivers/tty/hvc/hvc_riscv_sbi.c
@@ -39,21 +39,83 @@ static int hvc_sbi_tty_get(uint32_t vtermno, char *buf, int count)
return i;
}

-static const struct hv_ops hvc_sbi_ops = {
+static const struct hv_ops hvc_sbi_v01_ops = {
.get_chars = hvc_sbi_tty_get,
.put_chars = hvc_sbi_tty_put,
};

-static int __init hvc_sbi_init(void)
+static int hvc_sbi_dbcn_tty_put(uint32_t vtermno, const char *buf, int count)
{
- return PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_ops, 16));
+ phys_addr_t pa;
+ struct sbiret ret;
+
+ if (is_vmalloc_addr(buf))
+ pa = page_to_phys(vmalloc_to_page(buf)) + offset_in_page(buf);
+ else
+ pa = __pa(buf);
+
+ if (IS_ENABLED(CONFIG_32BIT))
+ ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_WRITE,
+ count, lower_32_bits(pa), upper_32_bits(pa),
+ 0, 0, 0);
+ else
+ ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_WRITE,
+ count, pa, 0, 0, 0, 0);
+ if (ret.error)
+ return 0;
+
+ return count;
}
-device_initcall(hvc_sbi_init);

-static int __init hvc_sbi_console_init(void)
+static int hvc_sbi_dbcn_tty_get(uint32_t vtermno, char *buf, int count)
{
- hvc_instantiate(0, 0, &hvc_sbi_ops);
+ phys_addr_t pa;
+ struct sbiret ret;
+
+ if (is_vmalloc_addr(buf))
+ pa = page_to_phys(vmalloc_to_page(buf)) + offset_in_page(buf);
+ else
+ pa = __pa(buf);
+
+ if (IS_ENABLED(CONFIG_32BIT))
+ ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_READ,
+ count, lower_32_bits(pa), upper_32_bits(pa),
+ 0, 0, 0);
+ else
+ ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_READ,
+ count, pa, 0, 0, 0, 0);
+ if (ret.error)
+ return 0;
+
+ return ret.value;
+}
+
+static const struct hv_ops hvc_sbi_dbcn_ops = {
+ .put_chars = hvc_sbi_dbcn_tty_put,
+ .get_chars = hvc_sbi_dbcn_tty_get,
+};
+
+static int __init hvc_sbi_init(void)
+{
+ int err;
+
+ if ((sbi_spec_version >= sbi_mk_version(2, 0)) &&
+ (sbi_probe_extension(SBI_EXT_DBCN) > 0)) {
+ err = PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_dbcn_ops, 16));
+ if (err)
+ return err;
+ hvc_instantiate(0, 0, &hvc_sbi_dbcn_ops);
+ } else {
+ if (IS_ENABLED(CONFIG_RISCV_SBI_V01)) {
+ err = PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_v01_ops, 16));
+ if (err)
+ return err;
+ hvc_instantiate(0, 0, &hvc_sbi_v01_ops);
+ } else {
+ return -ENODEV;
+ }
+ }

return 0;
}
-console_initcall(hvc_sbi_console_init);
+device_initcall(hvc_sbi_init);
--
2.34.1

2023-10-12 05:17:19

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 2/8] RISC-V: KVM: Change the SBI specification version to v2.0

We will be implementing SBI DBCN extension for KVM RISC-V so let
us change the KVM RISC-V SBI specification version to v2.0.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/kvm_vcpu_sbi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index cdcf0ff07be7..8d6d4dce8a5e 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -11,7 +11,7 @@

#define KVM_SBI_IMPID 3

-#define KVM_SBI_VERSION_MAJOR 1
+#define KVM_SBI_VERSION_MAJOR 2
#define KVM_SBI_VERSION_MINOR 0

enum kvm_riscv_sbi_ext_status {
--
2.34.1

2023-10-12 05:17:22

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 4/8] RISC-V: KVM: Forward SBI DBCN extension to user-space

The frozen SBI v2.0 specification defines the SBI debug console
(DBCN) extension which replaces the legacy SBI v0.1 console
functions namely sbi_console_getchar() and sbi_console_putchar().

The SBI DBCN extension needs to be emulated in the KVM user-space
(i.e. QEMU-KVM or KVMTOOL) so we forward SBI DBCN calls from KVM
guest to the KVM user-space which can then redirect the console
input/output to wherever it wants (e.g. telnet, file, stdio, etc).

The SBI debug console is simply a early console available to KVM
guest for early prints and it does not intend to replace the proper
console devices such as 8250, VirtIO console, etc.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 +
arch/riscv/include/uapi/asm/kvm.h | 1 +
arch/riscv/kvm/vcpu_sbi.c | 4 ++++
arch/riscv/kvm/vcpu_sbi_replace.c | 32 +++++++++++++++++++++++++++
4 files changed, 38 insertions(+)

diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index c02bda5559d7..6a453f7f8b56 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -73,6 +73,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi;
extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence;
extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn;
extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;

diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index 917d8cc2489e..60d3b21dead7 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -156,6 +156,7 @@ enum KVM_RISCV_SBI_EXT_ID {
KVM_RISCV_SBI_EXT_PMU,
KVM_RISCV_SBI_EXT_EXPERIMENTAL,
KVM_RISCV_SBI_EXT_VENDOR,
+ KVM_RISCV_SBI_EXT_DBCN,
KVM_RISCV_SBI_EXT_MAX,
};

diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index 1b1cee86efda..bb76c3cf633f 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -66,6 +66,10 @@ static const struct kvm_riscv_sbi_extension_entry sbi_ext[] = {
.ext_idx = KVM_RISCV_SBI_EXT_PMU,
.ext_ptr = &vcpu_sbi_ext_pmu,
},
+ {
+ .ext_idx = KVM_RISCV_SBI_EXT_DBCN,
+ .ext_ptr = &vcpu_sbi_ext_dbcn,
+ },
{
.ext_idx = KVM_RISCV_SBI_EXT_EXPERIMENTAL,
.ext_ptr = &vcpu_sbi_ext_experimental,
diff --git a/arch/riscv/kvm/vcpu_sbi_replace.c b/arch/riscv/kvm/vcpu_sbi_replace.c
index 7c4d5d38a339..23b57c931b15 100644
--- a/arch/riscv/kvm/vcpu_sbi_replace.c
+++ b/arch/riscv/kvm/vcpu_sbi_replace.c
@@ -175,3 +175,35 @@ const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst = {
.extid_end = SBI_EXT_SRST,
.handler = kvm_sbi_ext_srst_handler,
};
+
+static int kvm_sbi_ext_dbcn_handler(struct kvm_vcpu *vcpu,
+ struct kvm_run *run,
+ struct kvm_vcpu_sbi_return *retdata)
+{
+ struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
+ unsigned long funcid = cp->a6;
+
+ switch (funcid) {
+ case SBI_EXT_DBCN_CONSOLE_WRITE:
+ case SBI_EXT_DBCN_CONSOLE_READ:
+ case SBI_EXT_DBCN_CONSOLE_WRITE_BYTE:
+ /*
+ * The SBI debug console functions are unconditionally
+ * forwarded to the userspace.
+ */
+ kvm_riscv_vcpu_sbi_forward(vcpu, run);
+ retdata->uexit = true;
+ break;
+ default:
+ retdata->err_val = SBI_ERR_NOT_SUPPORTED;
+ }
+
+ return 0;
+}
+
+const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn = {
+ .extid_start = SBI_EXT_DBCN,
+ .extid_end = SBI_EXT_DBCN,
+ .default_unavail = true,
+ .handler = kvm_sbi_ext_dbcn_handler,
+};
--
2.34.1

2023-10-12 05:17:26

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 8/8] RISC-V: Enable SBI based earlycon support

Let us enable SBI based earlycon support in defconfigs for both RV32
and RV64 so that "earlycon=sbi" can be used again.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/configs/defconfig | 1 +
arch/riscv/configs/rv32_defconfig | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index ab86ec3b9eab..f82700da0056 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -132,6 +132,7 @@ CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_8250_DW=y
CONFIG_SERIAL_OF_PLATFORM=y
CONFIG_SERIAL_SH_SCI=y
+CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
CONFIG_VIRTIO_CONSOLE=y
CONFIG_HW_RANDOM=y
CONFIG_HW_RANDOM_VIRTIO=y
diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
index 89b601e253a6..5721af39afd1 100644
--- a/arch/riscv/configs/rv32_defconfig
+++ b/arch/riscv/configs/rv32_defconfig
@@ -66,6 +66,7 @@ CONFIG_INPUT_MOUSEDEV=y
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
CONFIG_VIRTIO_CONSOLE=y
CONFIG_HW_RANDOM=y
CONFIG_HW_RANDOM_VIRTIO=y
--
2.34.1

2023-10-12 05:17:37

by Anup Patel

[permalink] [raw]
Subject: [PATCH v2 5/8] RISC-V: Add inline version of sbi_console_putchar/getchar() functions

The functions sbi_console_putchar() and sbi_console_getchar() are
not defined when CONFIG_RISCV_SBI_V01 is disabled so let us add
inline version of these functions to avoid "#ifdef" on user side.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/sbi.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 12dfda6bb924..cbcefa344417 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -271,8 +271,13 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
unsigned long arg3, unsigned long arg4,
unsigned long arg5);

+#ifdef CONFIG_RISCV_SBI_V01
void sbi_console_putchar(int ch);
int sbi_console_getchar(void);
+#else
+static inline void sbi_console_putchar(int ch) { }
+static inline int sbi_console_getchar(void) { return -1; }
+#endif
long sbi_get_mvendorid(void);
long sbi_get_marchid(void);
long sbi_get_mimpid(void);
--
2.34.1

2023-10-12 11:39:07

by Björn Töpel

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] tty: Add SBI debug console support to HVC SBI driver

Anup Patel <[email protected]> writes:

> From: Atish Patra <[email protected]>
>
> RISC-V SBI specification supports advanced debug console
> support via SBI DBCN extension.
>
> Extend the HVC SBI driver to support it.
>
> Signed-off-by: Atish Patra <[email protected]>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> drivers/tty/hvc/Kconfig | 2 +-
> drivers/tty/hvc/hvc_riscv_sbi.c | 76 ++++++++++++++++++++++++++++++---
> 2 files changed, 70 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
> index 4f9264d005c0..6e05c5c7bca1 100644
> --- a/drivers/tty/hvc/Kconfig
> +++ b/drivers/tty/hvc/Kconfig
> @@ -108,7 +108,7 @@ config HVC_DCC_SERIALIZE_SMP
>
> config HVC_RISCV_SBI
> bool "RISC-V SBI console support"
> - depends on RISCV_SBI_V01
> + depends on RISCV_SBI
> select HVC_DRIVER
> help
> This enables support for console output via RISC-V SBI calls, which
> diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
> index 31f53fa77e4a..da318d7f55c5 100644
> --- a/drivers/tty/hvc/hvc_riscv_sbi.c
> +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
> @@ -39,21 +39,83 @@ static int hvc_sbi_tty_get(uint32_t vtermno, char *buf, int count)
> return i;
> }
>
> -static const struct hv_ops hvc_sbi_ops = {
> +static const struct hv_ops hvc_sbi_v01_ops = {
> .get_chars = hvc_sbi_tty_get,
> .put_chars = hvc_sbi_tty_put,
> };
>
> -static int __init hvc_sbi_init(void)
> +static int hvc_sbi_dbcn_tty_put(uint32_t vtermno, const char *buf, int count)
> {
> - return PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_ops, 16));
> + phys_addr_t pa;
> + struct sbiret ret;
> +
> + if (is_vmalloc_addr(buf))
> + pa = page_to_phys(vmalloc_to_page(buf)) + offset_in_page(buf);

What is assumed from buf here? If buf is crossing a page, you need to
adjust the count, no?

> + else
> + pa = __pa(buf);
> +
> + if (IS_ENABLED(CONFIG_32BIT))
> + ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_WRITE,
> + count, lower_32_bits(pa), upper_32_bits(pa),
> + 0, 0, 0);
> + else
> + ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_WRITE,
> + count, pa, 0, 0, 0, 0);
> + if (ret.error)
> + return 0;
> +
> + return count;
> }
> -device_initcall(hvc_sbi_init);
>
> -static int __init hvc_sbi_console_init(void)
> +static int hvc_sbi_dbcn_tty_get(uint32_t vtermno, char *buf, int count)
> {
> - hvc_instantiate(0, 0, &hvc_sbi_ops);
> + phys_addr_t pa;
> + struct sbiret ret;
> +
> + if (is_vmalloc_addr(buf))
> + pa = page_to_phys(vmalloc_to_page(buf)) + offset_in_page(buf);

And definitely adjust count here, if we're crossing a page!


Björn

2023-10-13 15:43:10

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] tty: Add SBI debug console support to HVC SBI driver

On Thu, Oct 12, 2023 at 5:08 PM Björn Töpel <[email protected]> wrote:
>
> Anup Patel <[email protected]> writes:
>
> > From: Atish Patra <[email protected]>
> >
> > RISC-V SBI specification supports advanced debug console
> > support via SBI DBCN extension.
> >
> > Extend the HVC SBI driver to support it.
> >
> > Signed-off-by: Atish Patra <[email protected]>
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > drivers/tty/hvc/Kconfig | 2 +-
> > drivers/tty/hvc/hvc_riscv_sbi.c | 76 ++++++++++++++++++++++++++++++---
> > 2 files changed, 70 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
> > index 4f9264d005c0..6e05c5c7bca1 100644
> > --- a/drivers/tty/hvc/Kconfig
> > +++ b/drivers/tty/hvc/Kconfig
> > @@ -108,7 +108,7 @@ config HVC_DCC_SERIALIZE_SMP
> >
> > config HVC_RISCV_SBI
> > bool "RISC-V SBI console support"
> > - depends on RISCV_SBI_V01
> > + depends on RISCV_SBI
> > select HVC_DRIVER
> > help
> > This enables support for console output via RISC-V SBI calls, which
> > diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
> > index 31f53fa77e4a..da318d7f55c5 100644
> > --- a/drivers/tty/hvc/hvc_riscv_sbi.c
> > +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
> > @@ -39,21 +39,83 @@ static int hvc_sbi_tty_get(uint32_t vtermno, char *buf, int count)
> > return i;
> > }
> >
> > -static const struct hv_ops hvc_sbi_ops = {
> > +static const struct hv_ops hvc_sbi_v01_ops = {
> > .get_chars = hvc_sbi_tty_get,
> > .put_chars = hvc_sbi_tty_put,
> > };
> >
> > -static int __init hvc_sbi_init(void)
> > +static int hvc_sbi_dbcn_tty_put(uint32_t vtermno, const char *buf, int count)
> > {
> > - return PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_ops, 16));
> > + phys_addr_t pa;
> > + struct sbiret ret;
> > +
> > + if (is_vmalloc_addr(buf))
> > + pa = page_to_phys(vmalloc_to_page(buf)) + offset_in_page(buf);
>
> What is assumed from buf here? If buf is crossing a page, you need to
> adjust the count, no?

I never saw a page crossing buffer but I will certainly address this
in the next revision.

>
> > + else
> > + pa = __pa(buf);
> > +
> > + if (IS_ENABLED(CONFIG_32BIT))
> > + ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_WRITE,
> > + count, lower_32_bits(pa), upper_32_bits(pa),
> > + 0, 0, 0);
> > + else
> > + ret = sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_WRITE,
> > + count, pa, 0, 0, 0, 0);
> > + if (ret.error)
> > + return 0;
> > +
> > + return count;
> > }
> > -device_initcall(hvc_sbi_init);
> >
> > -static int __init hvc_sbi_console_init(void)
> > +static int hvc_sbi_dbcn_tty_get(uint32_t vtermno, char *buf, int count)
> > {
> > - hvc_instantiate(0, 0, &hvc_sbi_ops);
> > + phys_addr_t pa;
> > + struct sbiret ret;
> > +
> > + if (is_vmalloc_addr(buf))
> > + pa = page_to_phys(vmalloc_to_page(buf)) + offset_in_page(buf);
>
> And definitely adjust count here, if we're crossing a page!

Sure, I will update here as well.

Thanks,
Anup

2023-10-19 07:46:12

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 1/8] RISC-V: Add defines for SBI debug console extension

On Thu, Oct 12, 2023 at 10:45:02AM +0530, Anup Patel wrote:
> We add SBI debug console extension related defines/enum to the
> asm/sbi.h header.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/sbi.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 5b4a1bf5f439..12dfda6bb924 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_DBCN = 0x4442434E,
>
> /* Experimentals extensions must lie within this range */
> SBI_EXT_EXPERIMENTAL_START = 0x08000000,
> @@ -236,6 +237,12 @@ enum sbi_pmu_ctr_type {
> /* Flags defined for counter stop function */
> #define SBI_PMU_STOP_FLAG_RESET (1 << 0)
>
> +enum sbi_ext_dbcn_fid {
> + SBI_EXT_DBCN_CONSOLE_WRITE = 0,
> + SBI_EXT_DBCN_CONSOLE_READ = 1,
> + SBI_EXT_DBCN_CONSOLE_WRITE_BYTE = 2,
> +};
> +
> #define SBI_SPEC_VERSION_DEFAULT 0x1
> #define SBI_SPEC_VERSION_MAJOR_SHIFT 24
> #define SBI_SPEC_VERSION_MAJOR_MASK 0x7f
> --
> 2.34.1
>

Reviewed-by: Andrew Jones <[email protected]>

2023-10-19 07:46:47

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 2/8] RISC-V: KVM: Change the SBI specification version to v2.0

On Thu, Oct 12, 2023 at 10:45:03AM +0530, Anup Patel wrote:
> We will be implementing SBI DBCN extension for KVM RISC-V so let
> us change the KVM RISC-V SBI specification version to v2.0.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/kvm_vcpu_sbi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index cdcf0ff07be7..8d6d4dce8a5e 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -11,7 +11,7 @@
>
> #define KVM_SBI_IMPID 3
>
> -#define KVM_SBI_VERSION_MAJOR 1
> +#define KVM_SBI_VERSION_MAJOR 2
> #define KVM_SBI_VERSION_MINOR 0
>
> enum kvm_riscv_sbi_ext_status {
> --
> 2.34.1
>

Reviewed-by: Andrew Jones <[email protected]>

2023-10-19 08:02:15

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 4/8] RISC-V: KVM: Forward SBI DBCN extension to user-space

On Thu, Oct 12, 2023 at 10:45:05AM +0530, Anup Patel wrote:
> The frozen SBI v2.0 specification defines the SBI debug console
> (DBCN) extension which replaces the legacy SBI v0.1 console
> functions namely sbi_console_getchar() and sbi_console_putchar().
>
> The SBI DBCN extension needs to be emulated in the KVM user-space
> (i.e. QEMU-KVM or KVMTOOL) so we forward SBI DBCN calls from KVM
> guest to the KVM user-space which can then redirect the console
> input/output to wherever it wants (e.g. telnet, file, stdio, etc).
>
> The SBI debug console is simply a early console available to KVM
> guest for early prints and it does not intend to replace the proper
> console devices such as 8250, VirtIO console, etc.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 +
> arch/riscv/include/uapi/asm/kvm.h | 1 +
> arch/riscv/kvm/vcpu_sbi.c | 4 ++++
> arch/riscv/kvm/vcpu_sbi_replace.c | 32 +++++++++++++++++++++++++++
> 4 files changed, 38 insertions(+)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index c02bda5559d7..6a453f7f8b56 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -73,6 +73,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;
>
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 917d8cc2489e..60d3b21dead7 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -156,6 +156,7 @@ enum KVM_RISCV_SBI_EXT_ID {
> KVM_RISCV_SBI_EXT_PMU,
> KVM_RISCV_SBI_EXT_EXPERIMENTAL,
> KVM_RISCV_SBI_EXT_VENDOR,
> + KVM_RISCV_SBI_EXT_DBCN,
> KVM_RISCV_SBI_EXT_MAX,
> };
>
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index 1b1cee86efda..bb76c3cf633f 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -66,6 +66,10 @@ static const struct kvm_riscv_sbi_extension_entry sbi_ext[] = {
> .ext_idx = KVM_RISCV_SBI_EXT_PMU,
> .ext_ptr = &vcpu_sbi_ext_pmu,
> },
> + {
> + .ext_idx = KVM_RISCV_SBI_EXT_DBCN,
> + .ext_ptr = &vcpu_sbi_ext_dbcn,
> + },
> {
> .ext_idx = KVM_RISCV_SBI_EXT_EXPERIMENTAL,
> .ext_ptr = &vcpu_sbi_ext_experimental,
> diff --git a/arch/riscv/kvm/vcpu_sbi_replace.c b/arch/riscv/kvm/vcpu_sbi_replace.c
> index 7c4d5d38a339..23b57c931b15 100644
> --- a/arch/riscv/kvm/vcpu_sbi_replace.c
> +++ b/arch/riscv/kvm/vcpu_sbi_replace.c
> @@ -175,3 +175,35 @@ const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst = {
> .extid_end = SBI_EXT_SRST,
> .handler = kvm_sbi_ext_srst_handler,
> };
> +
> +static int kvm_sbi_ext_dbcn_handler(struct kvm_vcpu *vcpu,
> + struct kvm_run *run,
> + struct kvm_vcpu_sbi_return *retdata)
> +{
> + struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
> + unsigned long funcid = cp->a6;
> +
> + switch (funcid) {
> + case SBI_EXT_DBCN_CONSOLE_WRITE:
> + case SBI_EXT_DBCN_CONSOLE_READ:
> + case SBI_EXT_DBCN_CONSOLE_WRITE_BYTE:
> + /*
> + * The SBI debug console functions are unconditionally
> + * forwarded to the userspace.
> + */
> + kvm_riscv_vcpu_sbi_forward(vcpu, run);
> + retdata->uexit = true;
> + break;
> + default:
> + retdata->err_val = SBI_ERR_NOT_SUPPORTED;
> + }
> +
> + return 0;
> +}
> +
> +const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn = {
> + .extid_start = SBI_EXT_DBCN,
> + .extid_end = SBI_EXT_DBCN,
> + .default_unavail = true,
> + .handler = kvm_sbi_ext_dbcn_handler,
> +};
> --
> 2.34.1
>

Reviewed-by: Andrew Jones <[email protected]>

2023-10-19 08:04:05

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] RISC-V: Add inline version of sbi_console_putchar/getchar() functions

On Thu, Oct 12, 2023 at 10:45:06AM +0530, Anup Patel wrote:
> The functions sbi_console_putchar() and sbi_console_getchar() are
> not defined when CONFIG_RISCV_SBI_V01 is disabled so let us add
> inline version of these functions to avoid "#ifdef" on user side.

nit: Here and in the summary I'd probably use the word "stub" instead of
"inline version".

>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/sbi.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 12dfda6bb924..cbcefa344417 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -271,8 +271,13 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
> unsigned long arg3, unsigned long arg4,
> unsigned long arg5);
>
> +#ifdef CONFIG_RISCV_SBI_V01
> void sbi_console_putchar(int ch);
> int sbi_console_getchar(void);
> +#else
> +static inline void sbi_console_putchar(int ch) { }
> +static inline int sbi_console_getchar(void) { return -1; }
> +#endif
> long sbi_get_mvendorid(void);
> long sbi_get_marchid(void);
> long sbi_get_mimpid(void);
> --
> 2.34.1
>

Otherwise,

Reviewed-by: Andrew Jones <[email protected]>

2023-10-19 08:47:00

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] RISC-V: Enable SBI based earlycon support

On Thu, Oct 12, 2023 at 10:45:09AM +0530, Anup Patel wrote:
> Let us enable SBI based earlycon support in defconfigs for both RV32
> and RV64 so that "earlycon=sbi" can be used again.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/configs/defconfig | 1 +
> arch/riscv/configs/rv32_defconfig | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index ab86ec3b9eab..f82700da0056 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -132,6 +132,7 @@ CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_8250_DW=y
> CONFIG_SERIAL_OF_PLATFORM=y
> CONFIG_SERIAL_SH_SCI=y
> +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
> CONFIG_VIRTIO_CONSOLE=y
> CONFIG_HW_RANDOM=y
> CONFIG_HW_RANDOM_VIRTIO=y
> diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
> index 89b601e253a6..5721af39afd1 100644
> --- a/arch/riscv/configs/rv32_defconfig
> +++ b/arch/riscv/configs/rv32_defconfig
> @@ -66,6 +66,7 @@ CONFIG_INPUT_MOUSEDEV=y
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_OF_PLATFORM=y
> +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
> CONFIG_VIRTIO_CONSOLE=y
> CONFIG_HW_RANDOM=y
> CONFIG_HW_RANDOM_VIRTIO=y
> --
> 2.34.1
>

Reviewed-by: Andrew Jones <[email protected]>

2023-10-19 09:17:54

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v2 4/8] RISC-V: KVM: Forward SBI DBCN extension to user-space

On Thu, Oct 12, 2023 at 10:45:05AM +0530, Anup Patel wrote:
> The frozen SBI v2.0 specification defines the SBI debug console
> (DBCN) extension which replaces the legacy SBI v0.1 console
> functions namely sbi_console_getchar() and sbi_console_putchar().
>
> The SBI DBCN extension needs to be emulated in the KVM user-space
> (i.e. QEMU-KVM or KVMTOOL) so we forward SBI DBCN calls from KVM
> guest to the KVM user-space which can then redirect the console
> input/output to wherever it wants (e.g. telnet, file, stdio, etc).
>
> The SBI debug console is simply a early console available to KVM
> guest for early prints and it does not intend to replace the proper
> console devices such as 8250, VirtIO console, etc.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 +
> arch/riscv/include/uapi/asm/kvm.h | 1 +
> arch/riscv/kvm/vcpu_sbi.c | 4 ++++
> arch/riscv/kvm/vcpu_sbi_replace.c | 32 +++++++++++++++++++++++++++
> 4 files changed, 38 insertions(+)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index c02bda5559d7..6a453f7f8b56 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -73,6 +73,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
> extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;
>
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 917d8cc2489e..60d3b21dead7 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -156,6 +156,7 @@ enum KVM_RISCV_SBI_EXT_ID {
> KVM_RISCV_SBI_EXT_PMU,
> KVM_RISCV_SBI_EXT_EXPERIMENTAL,
> KVM_RISCV_SBI_EXT_VENDOR,
> + KVM_RISCV_SBI_EXT_DBCN,

We should add this new register to the get-reg-list kselftest, i.e.

diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
index 234006d035c9..4a0f8a8cfbf8 100644
--- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
+++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
@@ -565,6 +565,7 @@ static __u64 base_regs[] = {
KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SRST,
KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_HSM,
KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_PMU,
+ KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN,
KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL,
KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR,
KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_MULTI_EN | 0,

Thanks,
drew