date:20220709

Re: [PATCH] target/riscv: fix right shifts shamt value for rv128c

2022-07-09 Thread Frédéric Pétrot


Le 09/07/2022 à 10:52, Weiwei Li a écrit :


在 2022/7/8 下午11:00, Frédéric Pétrot 写道:

For rv128c right shifts, the 6-bit shamt is sign extended to 7 bits.

Signed-off-by: Frédéric Pétrot
---
  target/riscv/insn16.decode |  7 ---
  disas/riscv.c  | 27 +--
  target/riscv/translate.c   | 12 +++-
  3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 02c8f61b48..ea3c5a0411 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -31,7 +31,8 @@
  %imm_cb12:s1 5:2 2:1 10:2 3:2 !function=ex_shift_1
  %imm_cj12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1
  
-%shimm_6bit   12:1 2:5   !function=ex_rvc_shifti

+%shlimm_6bit   12:1 2:5  !function=ex_rvc_shiftli
+%shrimm_6bit   12:1 2:5  !function=ex_rvc_shiftri
  %uimm_6bit_lq 2:4 12:1 6:1   !function=ex_shift_4
  %uimm_6bit_ld 2:3 12:1 5:2   !function=ex_shift_3
  %uimm_6bit_lw 2:2 12:1 4:3   !function=ex_shift_2


It's better to maintain the alignment here.


  Ooups! I'll do that.




@@ -82,9 +83,9 @@
  @c_addi16sp ... .  . . ..  imm=%imm_addi16sp rs1=2 rd=2
  
  @c_shift... . .. ... . .. \

- rd=%rs1_3 rs1=%rs1_3 shamt=%shimm_6bit
+ rd=%rs1_3 rs1=%rs1_3 shamt=%shrimm_6bit
  @c_shift2   ... . .. ... . .. \
- rd=%rd rs1=%rd shamt=%shimm_6bit
+ rd=%rd rs1=%rd shamt=%shlimm_6bit
  
  @c_andi ... . .. ... . ..  imm=%imm_ci rs1=%rs1_3 rd=%rs1_3
  
diff --git a/disas/riscv.c b/disas/riscv.c

index 7af6afc8fa..489c2ae5e8 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -2402,10 +2402,25 @@ static int32_t operand_sbimm12(rv_inst inst)
  ((inst << 56) >> 63) << 11;
  }
  
-static uint32_t operand_cimmsh6(rv_inst inst)

+static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
  {
-return ((inst << 51) >> 63) << 5 |
+int imm = ((inst << 51) >> 63) << 5 |
  (inst << 57) >> 59;
+if (isa == rv128) {
+imm = imm ? imm : 64;
+}
+return imm;
+}
+
+static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
+{
+int imm = ((inst << 51) >> 63) << 5 |
+(inst << 57) >> 59;
+if (isa == rv128) {
+imm = imm | (imm & 32) << 1;
+imm = imm ? imm : 64;
+}
+return imm;
  }
  
  static int32_t operand_cimmi(rv_inst inst)

@@ -2529,7 +2544,7 @@ static uint32_t operand_rnum(rv_inst inst)
  
  /* decode operands */
  
-static void decode_inst_operands(rv_decode *dec)

+static void decode_inst_operands(rv_decode *dec, rv_isa isa)
  {
  rv_inst inst = dec->inst;
  dec->codec = opcode_data[dec->op].codec;
@@ -2652,7 +2667,7 @@ static void decode_inst_operands(rv_decode *dec)
  case rv_codec_cb_sh6:
  dec->rd = dec->rs1 = operand_crs1rdq(inst) + 8;
  dec->rs2 = rv_ireg_zero;
-dec->imm = operand_cimmsh6(inst);
+dec->imm = operand_cimmshr6(inst, isa);
  break;
  case rv_codec_ci:
  dec->rd = dec->rs1 = operand_crs1rd(inst);
@@ -2667,7 +2682,7 @@ static void decode_inst_operands(rv_decode *dec)
  case rv_codec_ci_sh6:
  dec->rd = dec->rs1 = operand_crs1rd(inst);
  dec->rs2 = rv_ireg_zero;
-dec->imm = operand_cimmsh6(inst);
+dec->imm = operand_cimmshl6(inst, isa);
  break;
  case rv_codec_ci_16sp:
  dec->rd = rv_ireg_sp;
@@ -3193,7 +3208,7 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, 
uint64_t pc, rv_inst inst)
  dec.pc = pc;
  dec.inst = inst;
  decode_inst_opcode(, isa);
-decode_inst_operands();
+decode_inst_operands(, isa);
  decode_inst_decompress(, isa);
  decode_inst_lift_pseudo();
  format_inst(buf, buflen, 16, );
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 63b04e8a94..af3a2cd68c 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -705,12 +705,22 @@ static int ex_rvc_register(DisasContext *ctx, int reg)
  return 8 + reg;
  }
  
-static int ex_rvc_shifti(DisasContext *ctx, int imm)

+static int ex_rvc_shiftli(DisasContext *ctx, int imm)
  {
  /* For RV128 a shamt of 0 means a shift by 64. */
  return imm ? imm : 64;
  }
  
+static int ex_rvc_shiftri(DisasContext *ctx, int imm)

+{
+/*
+ * For RV128 a shamt of 0 means a shift by 64, furthermore, for right
+ * shifts, the shamt is sign-extended.
+ */
+imm = imm | (imm & 32) << 1;
+return imm ? imm : 64;
+}
+


This calculation didn't add limitation for only working in RV128, and may 
trigger fault in RV32/RV64,


such as following check in gen_shift_imm_fn:

if (a->shamt >= max_len) {
return false;
}


  Indeed! Thanks for pointing that mistake out.
  Also 0 becoming 64 is a rv128 only thing, and a shamt of zero should be a
  hint in rv64/rv32, so I believe the immediat should not be changed.

Re: [PATCH v6 03/10] i386/pc: pass pci_hole64_size to pc_memory_init()

2022-07-09 Thread B




Am 1. Juli 2022 16:10:07 UTC schrieb Joao Martins :
>Use the pre-initialized pci-host qdev and fetch the
>pci-hole64-size into pc_memory_init() newly added argument.
>piix needs a bit of care given all the !pci_enabled()
>and that the pci_hole64_size is private to i440fx.

It exposes this value as the property PCI_HOST_PROP_PCI_HOLE64_SIZE. Reusing it 
allows for not touching i440fx in this patch at all.

For code symmetry reasons the analogue property could be used for Q35 as well.

Best regards,
Bernhard

>
>This is in preparation to determine that host-phys-bits are
>enough and for pci-hole64-size to be considered to relocate
>ram-above-4g to be at 1T (on AMD platforms).
>
>Signed-off-by: Joao Martins 
>Reviewed-by: Igor Mammedov 
>---
> hw/i386/pc.c | 3 ++-
> hw/i386/pc_piix.c| 5 -
> hw/i386/pc_q35.c | 8 +++-
> hw/pci-host/i440fx.c | 7 +++
> include/hw/i386/pc.h | 3 ++-
> include/hw/pci-host/i440fx.h | 1 +
> 6 files changed, 23 insertions(+), 4 deletions(-)
>
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index a9d1bf95649a..1bb89a9c17ec 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -817,7 +817,8 @@ void xen_load_linux(PCMachineState *pcms)
> void pc_memory_init(PCMachineState *pcms,
> MemoryRegion *system_memory,
> MemoryRegion *rom_memory,
>-MemoryRegion **ram_memory)
>+MemoryRegion **ram_memory,
>+uint64_t pci_hole64_size)
> {
> int linux_boot, i;
> MemoryRegion *option_rom_mr;
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index 6186a1473755..f3c726e42400 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -91,6 +91,7 @@ static void pc_init1(MachineState *machine,
> MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> ram_addr_t lowmem;
>+uint64_t hole64_size;
> DeviceState *i440fx_host;
> 
> /*
>@@ -166,10 +167,12 @@ static void pc_init1(MachineState *machine,
> memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> rom_memory = pci_memory;
> i440fx_host = qdev_new(host_type);
>+hole64_size = i440fx_pci_hole64_size(i440fx_host);
> } else {
> pci_memory = NULL;
> rom_memory = system_memory;
> i440fx_host = NULL;
>+hole64_size = 0;
> }
> 
> pc_guest_info_init(pcms);
>@@ -186,7 +189,7 @@ static void pc_init1(MachineState *machine,
> /* allocate ram and load rom/bios */
> if (!xen_enabled()) {
> pc_memory_init(pcms, system_memory,
>-   rom_memory, _memory);
>+   rom_memory, _memory, hole64_size);
> } else {
> pc_system_flash_cleanup_unused(pcms);
> if (machine->kernel_filename != NULL) {
>diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>index 46ea89e564de..5a4a737fe203 100644
>--- a/hw/i386/pc_q35.c
>+++ b/hw/i386/pc_q35.c
>@@ -138,6 +138,7 @@ static void pc_q35_init(MachineState *machine)
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> bool acpi_pcihp;
> bool keep_pci_slot_hpc;
>+uint64_t pci_hole64_size = 0;
> 
> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
>  * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
>@@ -206,8 +207,13 @@ static void pc_q35_init(MachineState *machine)
> /* create pci host bus */
> q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
> 
>+if (pcmc->pci_enabled) {
>+pci_hole64_size = q35_host->mch.pci_hole64_size;
>+}
>+
> /* allocate ram and load rom/bios */
>-pc_memory_init(pcms, get_system_memory(), rom_memory, _memory);
>+pc_memory_init(pcms, get_system_memory(), rom_memory, _memory,
>+   pci_hole64_size);
> 
> object_property_add_child(qdev_get_machine(), "q35", OBJECT(q35_host));
> object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
>diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
>index d5426ef4a53c..15680da7d709 100644
>--- a/hw/pci-host/i440fx.c
>+++ b/hw/pci-host/i440fx.c
>@@ -237,6 +237,13 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
> }
> }
> 
>+uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
>+{
>+I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
>+
>+return i440fx->pci_hole64_size;
>+}
>+
> PCIBus *i440fx_init(const char *pci_type,
> DeviceState *dev,
> MemoryRegion *address_space_mem,
>diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>index b7735dccfc81..568c226d3034 100644
>--- a/include/hw/i386/pc.h
>+++ b/include/hw/i386/pc.h
>@@ -159,7 +159,8 @@ void xen_load_linux(PCMachineState *pcms);
> void pc_memory_init(PCMachineState *pcms,
> MemoryRegion *system_memory,
> MemoryRegion *rom_memory,
>-MemoryRegion **ram_memory);
>+MemoryRegion **ram_memory,
>+

Re: [PATCH v8 00/12] qtests: Check accelerator available at runtime via QMP 'query-accels'

2022-07-09 Thread Antonio Huete Jimenez


Second attempt of trying to resurrect this thread :-)

May anybody please let us know what else is missing in this series of 
patches? I'm trying to add a 'query-nvmm' qmp command but got redirected 
to this thread.


On 4/7/22 21:27, Antonio Huete Jimenez wrote:
Are all these changes OK or is there anything else missing? I was 
thinking in adding a QMP 'query-nvmm' command but I got pointed to this 
thread as a better alternative to having a per-accelerator query command.

Re: [PATCH 2/2] target/riscv: Auto set elen from vector extension by default

2022-07-09 Thread Weiwei Li




在 2022/7/9 下午4:11, Weiwei Li 写道:

在 2022/7/8 下午3:39, Kito Cheng 写道:

Default ELEN is setting to 64 for now, which is incorrect setting for
Zve32*, and spec has mention minimum VLEN and supported EEW in chapter
"Zve*: Vector Extensions for Embedded Processors" is 32 for Zve32.

ELEN actaully could be derived from which extensions are enabled,
so this patch set elen to 0 as auto detect, and keep the capability to
let user could configure that.

Signed-off-by: Kito Cheng 
---
  target/riscv/cpu.c | 16 +---
  1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 487d0faa63..c1b96da7da 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -751,13 +751,22 @@ static void riscv_cpu_realize(DeviceState *dev, 
Error **errp)

  "Vector extension ELEN must be power of 2");
  return;
  }
-    if (cpu->cfg.elen > 64 || cpu->cfg.vlen < 8) {
+    if (cpu->cfg.elen == 0) {
+  if (cpu->cfg.ext_zve32f) {
+    cpu->cfg.elen = 32;
+  }
+  if (cpu->cfg.ext_zve64f || cpu->cfg.ext_v) {
+    cpu->cfg.elen = 64;
+  }


This code is in the "if(cpu->cfg.ext_v){...}",  so 
"cpu->cfg.ext_zve64f || cpu->cfg.ext_v" will always be true.


It can use "else ... " directly.


Sorry, misunderstood the logic. It seems that we should set 
cpu->cfg.elen=64 directly here.


Regards,

Weiwei Li


+    }
+    if (cpu->cfg.elen != 0 && (cpu->cfg.elen > 64 ||
+   cpu->cfg.elen < 8)) {
  error_setg(errp,
  "Vector extension implementation only 
supports ELEN "

  "in the range [8, 64]");
  return;
  }
-    if (cpu->cfg.vlen < cpu->cfg.elen) {
+    if (cpu->cfg.elen != 0 && cpu->cfg.vlen < cpu->cfg.elen) {


when cfg.elen is set to zero, it  will be changed to the auto detect 
value(32/64)  before this two check.


So this two modifications seem unnecessary.

Regards,

Weiwei Li


  error_setg(errp,
  "Vector extension VLEN must be greater than 
or equal "

  "to ELEN");
@@ -901,7 +910,8 @@ static Property riscv_cpu_extensions[] = {
  DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
  DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
  DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
-    DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+    /* elen = 0 means set from v or zve* extension */
+    DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 0),
    DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
  DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),

Re: [PATCH 1/2] target/riscv: Lower bound of VLEN is 32, and check VLEN >= ELEN

2022-07-09 Thread Weiwei Li




在 2022/7/8 下午3:39, Kito Cheng 写道:

According RVV spec 1.0, the minmal requirement of VLEN is great than or
equal to ELEN, and minmal possible ELEN is 32, and also spec has mention
`Minimum VLEN` for zve32* is 32, so the lower bound of VLEN is 32 I
think.

[1] 
https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#2-implementation-defined-constant-parameters
[2] 
https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#182-zve-vector-extensions-for-embedded-processors

Signed-off-by: Kito Cheng 
---
  target/riscv/cpu.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1bb3973806..487d0faa63 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -740,10 +740,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  "Vector extension VLEN must be power of 2");
  return;
  }
-if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
+if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 32) {
  error_setg(errp,
  "Vector extension implementation only supports VLEN "
-"in the range [128, %d]", RV_VLEN_MAX);
+"in the range [32, %d]", RV_VLEN_MAX);
  return;
  }


The check for "VLEN in the range [128, RV_VLEN_MAX]" seems right here, 
since this check is for


V vector extension in current implementation and  "The V vector 
extension requires Zvl128b"(in section 18.3).


Regards,

Weiwei Li


  if (!is_power_of_2(cpu->cfg.elen)) {
@@ -757,6 +757,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  "in the range [8, 64]");
  return;
  }
+if (cpu->cfg.vlen < cpu->cfg.elen) {
+error_setg(errp,
+"Vector extension VLEN must be greater than or equal "
+"to ELEN");
+return;
+}
  if (cpu->cfg.vext_spec) {
  if (!g_strcmp0(cpu->cfg.vext_spec, "v1.0")) {
  vext_version = VEXT_VERSION_1_00_0;

Re: [PATCH 2/2] target/riscv: Implement dump content of vector register

2022-07-09 Thread Weiwei Li




在 2022/7/8 下午4:57, Kito Cheng 写道:

Implement -d cpu,vu to dump content of vector register.

Signed-off-by: Kito Cheng 
---
  target/riscv/cpu.c | 31 +++
  1 file changed, 31 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c1b96da7da..97b289d277 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -72,6 +72,15 @@ const char * const riscv_fpr_regnames[] = {
"f30/ft10", "f31/ft11"
  };
  
+const char * const riscv_vr_regnames[] = {

+  "v0",  "v1",  "v2",  "v3",  "v4",  "v5",
+  "v6",  "v7",  "v8",  "v9",  "v10", "v11",
+  "v12", "v13", "v14", "v15", "v16", "v17",
+  "v18", "v19", "v20", "v21", "v22", "v23",
+  "v24", "v25", "v26", "v27", "v28", "v29",
+  "v30", "v31"
+};
+
  static const char * const riscv_excp_names[] = {
  "misaligned_fetch",
  "fault_fetch",
@@ -375,6 +384,28 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, 
int flags)
  }
  }
  }
+if (flags & CPU_DUMP_VU) {
+int vlen = cpu->cfg.vlen;
+int n_chunk = vlen / 64;
+if (vlen == 32) {
+for (i = 0; i < 32; i++) {
+qemu_fprintf(f, "0x%08" PRIx64 "\n", env->vreg[i]);
+}


Seems forget to dump the riscv_vr_regnames here.

Just another question: whether is it necessary to dump the vector 
related csrs too?


Regards,

Weiwei Li


+} else {
+for (i = 0; i < 32; i++) {
+qemu_fprintf(f, " %-8s ",
+ riscv_vr_regnames[i]);
+
+int vec_reg_offset = i * vlen / 64;
+qemu_fprintf(f, "0x");
+for (int j = n_chunk - 1; j >= 0; --j) {
+qemu_fprintf(f, "%016" PRIx64,
+ env->vreg[vec_reg_offset + j]);
+}
+qemu_fprintf(f, "\n");
+}
+}
+}
  }
  
  static void riscv_cpu_set_pc(CPUState *cs, vaddr value)

Re: [PATCH] target/riscv: fix right shifts shamt value for rv128c

2022-07-09 Thread Weiwei Li



在 2022/7/8 下午11:00, Frédéric Pétrot 写道:

For rv128c right shifts, the 6-bit shamt is sign extended to 7 bits.

Signed-off-by: Frédéric Pétrot 
---
  target/riscv/insn16.decode |  7 ---
  disas/riscv.c  | 27 +--
  target/riscv/translate.c   | 12 +++-
  3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 02c8f61b48..ea3c5a0411 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -31,7 +31,8 @@
  %imm_cb12:s1 5:2 2:1 10:2 3:2 !function=ex_shift_1
  %imm_cj12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1
  
-%shimm_6bit   12:1 2:5   !function=ex_rvc_shifti

+%shlimm_6bit   12:1 2:5  !function=ex_rvc_shiftli
+%shrimm_6bit   12:1 2:5  !function=ex_rvc_shiftri
  %uimm_6bit_lq 2:4 12:1 6:1   !function=ex_shift_4
  %uimm_6bit_ld 2:3 12:1 5:2   !function=ex_shift_3
  %uimm_6bit_lw 2:2 12:1 4:3   !function=ex_shift_2


It's better to maintain the alignment here.


@@ -82,9 +83,9 @@
  @c_addi16sp ... .  . . ..  imm=%imm_addi16sp rs1=2 rd=2
  
  @c_shift... . .. ... . .. \

- rd=%rs1_3 rs1=%rs1_3 shamt=%shimm_6bit
+ rd=%rs1_3 rs1=%rs1_3 shamt=%shrimm_6bit
  @c_shift2   ... . .. ... . .. \
- rd=%rd rs1=%rd shamt=%shimm_6bit
+ rd=%rd rs1=%rd shamt=%shlimm_6bit
  
  @c_andi ... . .. ... . ..  imm=%imm_ci rs1=%rs1_3 rd=%rs1_3
  
diff --git a/disas/riscv.c b/disas/riscv.c

index 7af6afc8fa..489c2ae5e8 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -2402,10 +2402,25 @@ static int32_t operand_sbimm12(rv_inst inst)
  ((inst << 56) >> 63) << 11;
  }
  
-static uint32_t operand_cimmsh6(rv_inst inst)

+static uint32_t operand_cimmshl6(rv_inst inst, rv_isa isa)
  {
-return ((inst << 51) >> 63) << 5 |
+int imm = ((inst << 51) >> 63) << 5 |
  (inst << 57) >> 59;
+if (isa == rv128) {
+imm = imm ? imm : 64;
+}
+return imm;
+}
+
+static uint32_t operand_cimmshr6(rv_inst inst, rv_isa isa)
+{
+int imm = ((inst << 51) >> 63) << 5 |
+(inst << 57) >> 59;
+if (isa == rv128) {
+imm = imm | (imm & 32) << 1;
+imm = imm ? imm : 64;
+}
+return imm;
  }
  
  static int32_t operand_cimmi(rv_inst inst)

@@ -2529,7 +2544,7 @@ static uint32_t operand_rnum(rv_inst inst)
  
  /* decode operands */
  
-static void decode_inst_operands(rv_decode *dec)

+static void decode_inst_operands(rv_decode *dec, rv_isa isa)
  {
  rv_inst inst = dec->inst;
  dec->codec = opcode_data[dec->op].codec;
@@ -2652,7 +2667,7 @@ static void decode_inst_operands(rv_decode *dec)
  case rv_codec_cb_sh6:
  dec->rd = dec->rs1 = operand_crs1rdq(inst) + 8;
  dec->rs2 = rv_ireg_zero;
-dec->imm = operand_cimmsh6(inst);
+dec->imm = operand_cimmshr6(inst, isa);
  break;
  case rv_codec_ci:
  dec->rd = dec->rs1 = operand_crs1rd(inst);
@@ -2667,7 +2682,7 @@ static void decode_inst_operands(rv_decode *dec)
  case rv_codec_ci_sh6:
  dec->rd = dec->rs1 = operand_crs1rd(inst);
  dec->rs2 = rv_ireg_zero;
-dec->imm = operand_cimmsh6(inst);
+dec->imm = operand_cimmshl6(inst, isa);
  break;
  case rv_codec_ci_16sp:
  dec->rd = rv_ireg_sp;
@@ -3193,7 +3208,7 @@ disasm_inst(char *buf, size_t buflen, rv_isa isa, 
uint64_t pc, rv_inst inst)
  dec.pc = pc;
  dec.inst = inst;
  decode_inst_opcode(, isa);
-decode_inst_operands();
+decode_inst_operands(, isa);
  decode_inst_decompress(, isa);
  decode_inst_lift_pseudo();
  format_inst(buf, buflen, 16, );
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 63b04e8a94..af3a2cd68c 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -705,12 +705,22 @@ static int ex_rvc_register(DisasContext *ctx, int reg)
  return 8 + reg;
  }
  
-static int ex_rvc_shifti(DisasContext *ctx, int imm)

+static int ex_rvc_shiftli(DisasContext *ctx, int imm)
  {
  /* For RV128 a shamt of 0 means a shift by 64. */
  return imm ? imm : 64;
  }
  
+static int ex_rvc_shiftri(DisasContext *ctx, int imm)

+{
+/*
+ * For RV128 a shamt of 0 means a shift by 64, furthermore, for right
+ * shifts, the shamt is sign-extended.
+ */
+imm = imm | (imm & 32) << 1;
+return imm ? imm : 64;
+}
+


This calculation didn't add limitation for only working in RV128, and 
may trigger fault in RV32/RV64,


such as following check in gen_shift_imm_fn:

if (a->shamt >= max_len) {
return false;
}
Regards,
Weiwei Li

  /* Include the auto-generated decoder for 32 bit insn */
  #include "decode-insn32.c.inc"

Re: [PATCH 1/2] target/riscv: Lower bound of VLEN is 32, and check VLEN >= ELEN

2022-07-09 Thread Weiwei Li




在 2022/7/8 下午3:39, Kito Cheng 写道:

According RVV spec 1.0, the minmal requirement of VLEN is great than or
equal to ELEN, and minmal possible ELEN is 32, and also spec has mention
`Minimum VLEN` for zve32* is 32, so the lower bound of VLEN is 32 I
think.


Sorry. I have a question about how to decide the minmal possible ELEN to 
be 32?


In current implementation, elen should be in the range [8, 64](and there 
seems to be a typo in the check for this).


If the minmal possible ELEN is 32, maybe we need change the check to 
"elen >= 32" too.


Regards,

Weiwei Li


[1] 
https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#2-implementation-defined-constant-parameters
[2] 
https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#182-zve-vector-extensions-for-embedded-processors

Signed-off-by: Kito Cheng 
---
  target/riscv/cpu.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1bb3973806..487d0faa63 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -740,10 +740,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  "Vector extension VLEN must be power of 2");
  return;
  }
-if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
+if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 32) {
  error_setg(errp,
  "Vector extension implementation only supports VLEN "
-"in the range [128, %d]", RV_VLEN_MAX);
+"in the range [32, %d]", RV_VLEN_MAX);
  return;
  }
  if (!is_power_of_2(cpu->cfg.elen)) {
@@ -757,6 +757,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  "in the range [8, 64]");
  return;
  }
+if (cpu->cfg.vlen < cpu->cfg.elen) {
+error_setg(errp,
+"Vector extension VLEN must be greater than or equal "
+"to ELEN");
+return;
+}
  if (cpu->cfg.vext_spec) {
  if (!g_strcmp0(cpu->cfg.vext_spec, "v1.0")) {
  vext_version = VEXT_VERSION_1_00_0;

Re: [PATCH 2/2] target/riscv: Auto set elen from vector extension by default

2022-07-09 Thread Weiwei Li


在 2022/7/8 下午3:39, Kito Cheng 写道:

Default ELEN is setting to 64 for now, which is incorrect setting for
Zve32*, and spec has mention minimum VLEN and supported EEW in chapter
"Zve*: Vector Extensions for Embedded Processors" is 32 for Zve32.

ELEN actaully could be derived from which extensions are enabled,
so this patch set elen to 0 as auto detect, and keep the capability to
let user could configure that.

Signed-off-by: Kito Cheng 
---
  target/riscv/cpu.c | 16 +---
  1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 487d0faa63..c1b96da7da 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -751,13 +751,22 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  "Vector extension ELEN must be power of 2");
  return;
  }
-if (cpu->cfg.elen > 64 || cpu->cfg.vlen < 8) {
+if (cpu->cfg.elen == 0) {
+  if (cpu->cfg.ext_zve32f) {
+cpu->cfg.elen = 32;
+  }
+  if (cpu->cfg.ext_zve64f || cpu->cfg.ext_v) {
+cpu->cfg.elen = 64;
+  }


This code is in the "if(cpu->cfg.ext_v){...}",  so "cpu->cfg.ext_zve64f 
|| cpu->cfg.ext_v" will always be true.


It can use "else ... " directly.

+}
+if (cpu->cfg.elen != 0 && (cpu->cfg.elen > 64 ||
+   cpu->cfg.elen < 8)) {
  error_setg(errp,
  "Vector extension implementation only supports ELEN "
  "in the range [8, 64]");
  return;
  }
-if (cpu->cfg.vlen < cpu->cfg.elen) {
+if (cpu->cfg.elen != 0 && cpu->cfg.vlen < cpu->cfg.elen) {


when cfg.elen is set to zero, it  will be changed to the auto detect 
value(32/64)  before this two check.


So this two modifications seem unnecessary.

Regards,

Weiwei Li


  error_setg(errp,
  "Vector extension VLEN must be greater than or equal "
  "to ELEN");
@@ -901,7 +910,8 @@ static Property riscv_cpu_extensions[] = {
  DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
  DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
  DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
-DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+/* elen = 0 means set from v or zve* extension */
+DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 0),
  
  DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),

  DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),

Re: [PATCH] target/riscv: fix right shifts shamt value for rv128c

Re: [PATCH v6 03/10] i386/pc: pass pci_hole64_size to pc_memory_init()

Re: [PATCH v8 00/12] qtests: Check accelerator available at runtime via QMP 'query-accels'

Re: [PATCH 2/2] target/riscv: Auto set elen from vector extension by default

Re: [PATCH 1/2] target/riscv: Lower bound of VLEN is 32, and check VLEN >= ELEN

Re: [PATCH 2/2] target/riscv: Implement dump content of vector register

Re: [PATCH] target/riscv: fix right shifts shamt value for rv128c

Re: [PATCH 1/2] target/riscv: Lower bound of VLEN is 32, and check VLEN >= ELEN

Re: [PATCH 2/2] target/riscv: Auto set elen from vector extension by default

9 matches

Site Navigation

Mail list logo

Footer information