Re: [PATCH] target/riscv/kvm: implement SBI debug console (DBCN) calls

2024-04-28 Thread Alistair Francis
On Fri, Apr 26, 2024 at 1:51 AM Daniel Henrique Barboza
 wrote:
>
> SBI defines a Debug Console extension "DBCN" that will, in time, replace
> the legacy console putchar and getchar SBI extensions.
>
> The appeal of the DBCN extension is that it allows multiple bytes to be
> read/written in the SBI console in a single SBI call.
>
> As far as KVM goes, the DBCN calls are forwarded by an in-kernel KVM
> module to userspace. But this will only happens if the KVM module
> actually supports this SBI extension and we activate it.
>
> We'll check for DBCN support during init time, checking if get-reg-list
> is advertising KVM_RISCV_SBI_EXT_DBCN. In that case, we'll enable it via
> kvm_set_one_reg() during kvm_arch_init_vcpu().
>
> Finally, change kvm_riscv_handle_sbi() to handle the incoming calls for
> SBI_EXT_DBCN, reading and writing as required.
>
> A simple KVM guest with 'earlycon=sbi', running in an emulated RISC-V
> host, takes around 20 seconds to boot without using DBCN. With this
> patch we're taking around 14 seconds to boot due to the speed-up in the
> terminal output.  There's no change in boot time if the guest isn't
> using earlycon.
>
> Signed-off-by: Daniel Henrique Barboza 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/kvm/kvm-cpu.c | 111 +
>  target/riscv/sbi_ecall_interface.h |  17 +
>  2 files changed, 128 insertions(+)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 03e3fee607..54a9ab9fd7 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -409,6 +409,12 @@ static KVMCPUConfig kvm_v_vlenb = {
> KVM_REG_RISCV_VECTOR_CSR_REG(vlenb)
>  };
>
> +static KVMCPUConfig kvm_sbi_dbcn = {
> +.name = "sbi_dbcn",
> +.kvm_reg_id = KVM_REG_RISCV | KVM_REG_SIZE_U64 |
> +  KVM_REG_RISCV_SBI_EXT | KVM_RISCV_SBI_EXT_DBCN
> +};
> +
>  static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
>  {
>  CPURISCVState *env = >env;
> @@ -1041,6 +1047,20 @@ static int uint64_cmp(const void *a, const void *b)
>  return 0;
>  }
>
> +static void kvm_riscv_check_sbi_dbcn_support(RISCVCPU *cpu,
> + KVMScratchCPU *kvmcpu,
> + struct kvm_reg_list *reglist)
> +{
> +struct kvm_reg_list *reg_search;
> +
> +reg_search = bsearch(_sbi_dbcn.kvm_reg_id, reglist->reg, reglist->n,
> + sizeof(uint64_t), uint64_cmp);
> +
> +if (reg_search) {
> +kvm_sbi_dbcn.supported = true;
> +}
> +}
> +
>  static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
>   struct kvm_reg_list *reglist)
>  {
> @@ -1146,6 +1166,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
> KVMScratchCPU *kvmcpu)
>  if (riscv_has_ext(>env, RVV)) {
>  kvm_riscv_read_vlenb(cpu, kvmcpu, reglist);
>  }
> +
> +kvm_riscv_check_sbi_dbcn_support(cpu, kvmcpu, reglist);
>  }
>
>  static void riscv_init_kvm_registers(Object *cpu_obj)
> @@ -1320,6 +1342,17 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, 
> CPUState *cs)
>  return ret;
>  }
>
> +static int kvm_vcpu_enable_sbi_dbcn(RISCVCPU *cpu, CPUState *cs)
> +{
> +target_ulong reg = 1;
> +
> +if (!kvm_sbi_dbcn.supported) {
> +return 0;
> +}
> +
> +return kvm_set_one_reg(cs, kvm_sbi_dbcn.kvm_reg_id, );
> +}
> +
>  int kvm_arch_init_vcpu(CPUState *cs)
>  {
>  int ret = 0;
> @@ -1337,6 +1370,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  kvm_riscv_update_cpu_misa_ext(cpu, cs);
>  kvm_riscv_update_cpu_cfg_isa_ext(cpu, cs);
>
> +ret = kvm_vcpu_enable_sbi_dbcn(cpu, cs);
> +
>  return ret;
>  }
>
> @@ -1394,6 +1429,79 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs)
>  return true;
>  }
>
> +static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)
> +{
> +g_autofree uint8_t *buf = NULL;
> +RISCVCPU *cpu = RISCV_CPU(cs);
> +target_ulong num_bytes;
> +uint64_t addr;
> +unsigned char ch;
> +int ret;
> +
> +switch (run->riscv_sbi.function_id) {
> +case SBI_EXT_DBCN_CONSOLE_READ:
> +case SBI_EXT_DBCN_CONSOLE_WRITE:
> +num_bytes = run->riscv_sbi.args[0];
> +
> +if (num_bytes == 0) {
> +run->riscv_sbi.ret[0] = SBI_SUCCESS;
> +run->riscv_sbi.ret[1] = 0;
> +break;
> +}
> +
> +addr = run->riscv_sbi.args[1];
> +
> +/*
> + * Handle the case where a 32 bit CPU is running in a
> + * 64 bit addressing env.
> + */
> +if (riscv_cpu_mxl(>env) == MXL_RV32) {
> +addr |= (uint64_t)run->riscv_sbi.args[2] << 32;
> +}
> +
> +buf = g_malloc0(num_bytes);
> +
> +if (run->riscv_sbi.function_id == SBI_EXT_DBCN_CONSOLE_READ) {
> +ret = qemu_chr_fe_read_all(serial_hd(0)->be, buf, num_bytes);
> + 

Re: [PATCH] target/riscv/kvm: implement SBI debug console (DBCN) calls

2024-04-25 Thread Andrew Jones
On Thu, Apr 25, 2024 at 12:50:12PM GMT, Daniel Henrique Barboza wrote:
> SBI defines a Debug Console extension "DBCN" that will, in time, replace
> the legacy console putchar and getchar SBI extensions.
> 
> The appeal of the DBCN extension is that it allows multiple bytes to be
> read/written in the SBI console in a single SBI call.
> 
> As far as KVM goes, the DBCN calls are forwarded by an in-kernel KVM
> module to userspace. But this will only happens if the KVM module
> actually supports this SBI extension and we activate it.
> 
> We'll check for DBCN support during init time, checking if get-reg-list
> is advertising KVM_RISCV_SBI_EXT_DBCN. In that case, we'll enable it via
> kvm_set_one_reg() during kvm_arch_init_vcpu().
> 
> Finally, change kvm_riscv_handle_sbi() to handle the incoming calls for
> SBI_EXT_DBCN, reading and writing as required.
> 
> A simple KVM guest with 'earlycon=sbi', running in an emulated RISC-V
> host, takes around 20 seconds to boot without using DBCN. With this
> patch we're taking around 14 seconds to boot due to the speed-up in the
> terminal output.  There's no change in boot time if the guest isn't
> using earlycon.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  target/riscv/kvm/kvm-cpu.c | 111 +
>  target/riscv/sbi_ecall_interface.h |  17 +
>  2 files changed, 128 insertions(+)
>

Reviewed-by: Andrew Jones 



[PATCH] target/riscv/kvm: implement SBI debug console (DBCN) calls

2024-04-25 Thread Daniel Henrique Barboza
SBI defines a Debug Console extension "DBCN" that will, in time, replace
the legacy console putchar and getchar SBI extensions.

The appeal of the DBCN extension is that it allows multiple bytes to be
read/written in the SBI console in a single SBI call.

As far as KVM goes, the DBCN calls are forwarded by an in-kernel KVM
module to userspace. But this will only happens if the KVM module
actually supports this SBI extension and we activate it.

We'll check for DBCN support during init time, checking if get-reg-list
is advertising KVM_RISCV_SBI_EXT_DBCN. In that case, we'll enable it via
kvm_set_one_reg() during kvm_arch_init_vcpu().

Finally, change kvm_riscv_handle_sbi() to handle the incoming calls for
SBI_EXT_DBCN, reading and writing as required.

A simple KVM guest with 'earlycon=sbi', running in an emulated RISC-V
host, takes around 20 seconds to boot without using DBCN. With this
patch we're taking around 14 seconds to boot due to the speed-up in the
terminal output.  There's no change in boot time if the guest isn't
using earlycon.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/kvm/kvm-cpu.c | 111 +
 target/riscv/sbi_ecall_interface.h |  17 +
 2 files changed, 128 insertions(+)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 03e3fee607..54a9ab9fd7 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -409,6 +409,12 @@ static KVMCPUConfig kvm_v_vlenb = {
KVM_REG_RISCV_VECTOR_CSR_REG(vlenb)
 };
 
+static KVMCPUConfig kvm_sbi_dbcn = {
+.name = "sbi_dbcn",
+.kvm_reg_id = KVM_REG_RISCV | KVM_REG_SIZE_U64 |
+  KVM_REG_RISCV_SBI_EXT | KVM_RISCV_SBI_EXT_DBCN
+};
+
 static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
 {
 CPURISCVState *env = >env;
@@ -1041,6 +1047,20 @@ static int uint64_cmp(const void *a, const void *b)
 return 0;
 }
 
+static void kvm_riscv_check_sbi_dbcn_support(RISCVCPU *cpu,
+ KVMScratchCPU *kvmcpu,
+ struct kvm_reg_list *reglist)
+{
+struct kvm_reg_list *reg_search;
+
+reg_search = bsearch(_sbi_dbcn.kvm_reg_id, reglist->reg, reglist->n,
+ sizeof(uint64_t), uint64_cmp);
+
+if (reg_search) {
+kvm_sbi_dbcn.supported = true;
+}
+}
+
 static void kvm_riscv_read_vlenb(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
  struct kvm_reg_list *reglist)
 {
@@ -1146,6 +1166,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
KVMScratchCPU *kvmcpu)
 if (riscv_has_ext(>env, RVV)) {
 kvm_riscv_read_vlenb(cpu, kvmcpu, reglist);
 }
+
+kvm_riscv_check_sbi_dbcn_support(cpu, kvmcpu, reglist);
 }
 
 static void riscv_init_kvm_registers(Object *cpu_obj)
@@ -1320,6 +1342,17 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, 
CPUState *cs)
 return ret;
 }
 
+static int kvm_vcpu_enable_sbi_dbcn(RISCVCPU *cpu, CPUState *cs)
+{
+target_ulong reg = 1;
+
+if (!kvm_sbi_dbcn.supported) {
+return 0;
+}
+
+return kvm_set_one_reg(cs, kvm_sbi_dbcn.kvm_reg_id, );
+}
+
 int kvm_arch_init_vcpu(CPUState *cs)
 {
 int ret = 0;
@@ -1337,6 +1370,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
 kvm_riscv_update_cpu_misa_ext(cpu, cs);
 kvm_riscv_update_cpu_cfg_isa_ext(cpu, cs);
 
+ret = kvm_vcpu_enable_sbi_dbcn(cpu, cs);
+
 return ret;
 }
 
@@ -1394,6 +1429,79 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs)
 return true;
 }
 
+static void kvm_riscv_handle_sbi_dbcn(CPUState *cs, struct kvm_run *run)
+{
+g_autofree uint8_t *buf = NULL;
+RISCVCPU *cpu = RISCV_CPU(cs);
+target_ulong num_bytes;
+uint64_t addr;
+unsigned char ch;
+int ret;
+
+switch (run->riscv_sbi.function_id) {
+case SBI_EXT_DBCN_CONSOLE_READ:
+case SBI_EXT_DBCN_CONSOLE_WRITE:
+num_bytes = run->riscv_sbi.args[0];
+
+if (num_bytes == 0) {
+run->riscv_sbi.ret[0] = SBI_SUCCESS;
+run->riscv_sbi.ret[1] = 0;
+break;
+}
+
+addr = run->riscv_sbi.args[1];
+
+/*
+ * Handle the case where a 32 bit CPU is running in a
+ * 64 bit addressing env.
+ */
+if (riscv_cpu_mxl(>env) == MXL_RV32) {
+addr |= (uint64_t)run->riscv_sbi.args[2] << 32;
+}
+
+buf = g_malloc0(num_bytes);
+
+if (run->riscv_sbi.function_id == SBI_EXT_DBCN_CONSOLE_READ) {
+ret = qemu_chr_fe_read_all(serial_hd(0)->be, buf, num_bytes);
+if (ret < 0) {
+error_report("SBI_EXT_DBCN_CONSOLE_READ: error when "
+ "reading chardev");
+exit(1);
+}
+
+cpu_physical_memory_write(addr, buf, ret);
+} else {
+cpu_physical_memory_read(addr, buf, num_bytes);
+
+ret = qemu_chr_fe_write_all(serial_hd(0)->be,