[PATCH] hw/ufs: Fix buffer overflow bug

2024-04-21 Thread Jeuk Kim
It fixes the buffer overflow vulnerability in the ufs device.
The bug was detected by sanitizers.

You can reproduce it by:

cat << EOF |\
qemu-system-x86_64 \
-display none -machine accel=qtest -m 512M -M q35 -nodefaults -drive \
file=null-co://,if=none,id=disk0 -device ufs,id=ufs_bus -device \
ufs-lu,drive=disk0,bus=ufs_bus -qtest stdio
outl 0xcf8 0x8810
outl 0xcfc 0xe000
outl 0xcf8 0x8804
outw 0xcfc 0x06
write 0xe058 0x1 0xa7
write 0xa 0x1 0x50
EOF

Resolves: #2299
Fixes: 329f16624499 ("hw/ufs: Support for Query Transfer Requests")
Reported-by: Zheyu Ma 
Signed-off-by: Jeuk Kim 
---
 hw/ufs/ufs.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index eccdb852a0..bac78a32bb 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -126,6 +126,10 @@ static MemTxResult ufs_dma_read_req_upiu(UfsRequest *req)
 copy_size = sizeof(UtpUpiuHeader) + UFS_TRANSACTION_SPECIFIC_FIELD_SIZE +
 data_segment_length;
 
+if (copy_size > sizeof(req->req_upiu)) {
+copy_size = sizeof(req->req_upiu);
+}
+
 ret = ufs_addr_read(u, req_upiu_base_addr, >req_upiu, copy_size);
 if (ret) {
 trace_ufs_err_dma_read_req_upiu(req->slot, req_upiu_base_addr);
@@ -225,6 +229,10 @@ static MemTxResult ufs_dma_write_rsp_upiu(UfsRequest *req)
 copy_size = rsp_upiu_byte_len;
 }
 
+if (copy_size > sizeof(req->rsp_upiu)) {
+copy_size = sizeof(req->rsp_upiu);
+}
+
 ret = ufs_addr_write(u, rsp_upiu_base_addr, >rsp_upiu, copy_size);
 if (ret) {
 trace_ufs_err_dma_write_rsp_upiu(req->slot, rsp_upiu_base_addr);
-- 
2.34.1




Re: [PATCH v4] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI

2024-04-21 Thread Alistair Francis
On Sat, Apr 13, 2024 at 9:26 PM Alexei Filippov
 wrote:
>
> kvm_riscv_handle_sbi() may return not supported return code to not trigger
> qemu abort with vendor-specific sbi.
>
> Added SBI related return code's defines.
>
> Signed-off-by: Alexei Filippov 
> Fixes: 4eb47125 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")
> ---
>
> Changes since v3:
> -Clear Reviewed-by tags
>  target/riscv/kvm/kvm-cpu.c | 13 +
>  target/riscv/sbi_ecall_interface.h | 12 
>  2 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 6a6c6cae80..844942d9ba 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1392,7 +1392,6 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs)
>
>  static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run)
>  {
> -int ret = 0;
>  unsigned char ch;
>  switch (run->riscv_sbi.extension_id) {
>  case SBI_EXT_0_1_CONSOLE_PUTCHAR:
> @@ -1400,22 +1399,20 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct 
> kvm_run *run)
>  qemu_chr_fe_write(serial_hd(0)->be, , sizeof(ch));
>  break;
>  case SBI_EXT_0_1_CONSOLE_GETCHAR:
> -ret = qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch));
> -if (ret == sizeof(ch)) {
> +if (qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch)) == 
> sizeof(ch)) {
>  run->riscv_sbi.ret[0] = ch;
>  } else {
> -run->riscv_sbi.ret[0] = -1;
> +run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;

I'm not sure I follow. This seems like a failure but we report success
to the caller of this function?

Can you expand the commit message to explain why we want this change

Alistair

>  }
> -ret = 0;
>  break;
>  default:
>  qemu_log_mask(LOG_UNIMP,
> -  "%s: un-handled SBI EXIT, specific reasons is %lu\n",
> +  "%s: Unhandled SBI exit with extension-id %lu\n",
>__func__, run->riscv_sbi.extension_id);
> -ret = -1;
> +run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
>  break;
>  }
> -return ret;
> +return 0;
>  }
>
>  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> diff --git a/target/riscv/sbi_ecall_interface.h 
> b/target/riscv/sbi_ecall_interface.h
> index 43899d08f6..a2e21d9b8c 100644
> --- a/target/riscv/sbi_ecall_interface.h
> +++ b/target/riscv/sbi_ecall_interface.h
> @@ -69,4 +69,16 @@
>  #define SBI_EXT_VENDOR_END  0x09FF
>  /* clang-format on */
>
> +/* SBI return error codes */
> +#define SBI_SUCCESS  0
> +#define SBI_ERR_FAILURE -1
> +#define SBI_ERR_NOT_SUPPORTED   -2
> +#define SBI_ERR_INVALID_PARAM   -3
> +#define SBI_ERR_DENIED  -4
> +#define SBI_ERR_INVALID_ADDRESS -5
> +#define SBI_ERR_ALREADY_AVAILABLE   -6
> +#define SBI_ERR_ALREADY_STARTED -7
> +#define SBI_ERR_ALREADY_STOPPED -8
> +#define SBI_ERR_NO_SHMEM-9
> +
>  #endif
> --
> 2.34.1
>
>



Re: [PATCH v3] riscv: thead: Add th.sxstatus CSR emulation

2024-04-21 Thread Alistair Francis
On Thu, Apr 18, 2024 at 8:55 AM Christoph Müllner
 wrote:
>
> The th.sxstatus CSR can be used to identify available custom extension
> on T-Head CPUs. The CSR is documented here:
>   
> https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadsxstatus.adoc
>
> An important property of this patch is, that the th.sxstatus MAEE field
> is not set (indicating that XTheadMae is not available).
> XTheadMae is a memory attribute extension (similar to Svpbmt) which is
> implemented in many T-Head CPUs (C906, C910, etc.) and utilizes bits
> in PTEs that are marked as reserved. QEMU maintainers prefer to not
> implement XTheadMae, so we need give kernels a mechanism to identify
> if XTheadMae is available in a system or not. And this patch introduces
> this mechanism in QEMU in a way that's compatible with real HW
> (i.e., probing the th.sxstatus.MAEE bit).
>
> Further context can be found on the list:
> https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00775.html
>
> Reviewed-by: LIU Zhiwei 
> Signed-off-by: Christoph Müllner 
> ---
>  target/riscv/cpu.c   |  1 +
>  target/riscv/cpu.h   |  4 +++
>  target/riscv/csr.c   |  2 +-
>  target/riscv/meson.build |  1 +
>  target/riscv/th_csr.c| 68 
>  5 files changed, 75 insertions(+), 1 deletion(-)
>  create mode 100644 target/riscv/th_csr.c
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 36e3e5fdaf..b82ba95ae6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -545,6 +545,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
>  cpu->cfg.mvendorid = THEAD_VENDOR_ID;
>  #ifndef CONFIG_USER_ONLY
>  set_satp_mode_max_supported(cpu, VM_1_10_SV39);
> +th_register_custom_csrs(cpu);
>  #endif
>
>  /* inherited from parent obj via riscv_cpu_init() */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3b1a02b944..fd9424c8e9 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -818,10 +818,14 @@ extern const bool valid_vm_1_10_32[], 
> valid_vm_1_10_64[];
>
>  void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
>  void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
> +RISCVException smode(CPURISCVState *env, int csrno);

I don't think we should make this public. Especially not the name `smode()`.

One option is to rename this, something like `riscv_csr_check_smode()` maybe?

The better option is probably just to copy it to the vendor
implementation as it's a pretty simple function.

Alistair

>
>  void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
>
>  uint8_t satp_mode_max_from_map(uint32_t map);
>  const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
>
> +/* Implemented in th_csr.c */
> +void th_register_custom_csrs(RISCVCPU *cpu);
> +
>  #endif /* RISCV_CPU_H */
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 726096444f..503eeb5f24 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -260,7 +260,7 @@ static RISCVException aia_any32(CPURISCVState *env, int 
> csrno)
>  return any32(env, csrno);
>  }
>
> -static RISCVException smode(CPURISCVState *env, int csrno)
> +RISCVException smode(CPURISCVState *env, int csrno)
>  {
>  if (riscv_has_ext(env, RVS)) {
>  return RISCV_EXCP_NONE;
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index a5e0734e7f..a4bd61e52a 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -33,6 +33,7 @@ riscv_system_ss.add(files(
>'monitor.c',
>'machine.c',
>'pmu.c',
> +  'th_csr.c',
>'time_helper.c',
>'riscv-qmp-cmds.c',
>  ))
> diff --git a/target/riscv/th_csr.c b/target/riscv/th_csr.c
> new file mode 100644
> index 00..44e28a9298
> --- /dev/null
> +++ b/target/riscv/th_csr.c
> @@ -0,0 +1,68 @@
> +/*
> + * T-Head-specific CSRs.
> + *
> + * Copyright (c) 2024 VRULL GmbH
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "cpu_vendorid.h"
> +
> +#define CSR_TH_SXSTATUS 0x5c0
> +
> +/* TH_SXSTATUS bits */
> +#define TH_SXSTATUS_UCMEBIT(16)
> +#define TH_SXSTATUS_MAEEBIT(21)
> +#define TH_SXSTATUS_THEADISAEE  BIT(22)
> +
> +typedef struct {
> +int csrno;
> +int (*insertion_test)(RISCVCPU *cpu);
> +riscv_csr_operations csr_ops;
> +} riscv_csr;
> +
> +static int 

Re: [RFC PATCH 1/3] target/riscv: change RISCV_EXCP_SEMIHOST exception number

2024-04-21 Thread Alistair Francis
On Thu, Apr 18, 2024 at 11:40 PM Clément Léger  wrote:
>
> The double trap specification defines the double trap exception number
> to be 16 which is actually used by the internal semihosting one. Change
> it to some other value.
>
> Signed-off-by: Clément Léger 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu_bits.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index fc2068ee4d..9ade72ff31 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -670,7 +670,7 @@ typedef enum RISCVException {
>  RISCV_EXCP_INST_PAGE_FAULT = 0xc, /* since: priv-1.10.0 */
>  RISCV_EXCP_LOAD_PAGE_FAULT = 0xd, /* since: priv-1.10.0 */
>  RISCV_EXCP_STORE_PAGE_FAULT = 0xf, /* since: priv-1.10.0 */
> -RISCV_EXCP_SEMIHOST = 0x10,
> +RISCV_EXCP_SEMIHOST = 0x11,
>  RISCV_EXCP_INST_GUEST_PAGE_FAULT = 0x14,
>  RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT = 0x15,
>  RISCV_EXCP_VIRT_INSTRUCTION_FAULT = 0x16,
> --
> 2.43.0
>
>



Re: [PATCH v13 00/24] target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI

2024-04-21 Thread Jinjie Ruan via



On 2024/4/19 21:41, Peter Maydell wrote:
> On Sun, 7 Apr 2024 at 09:19, Jinjie Ruan  wrote:
>>
>> This patch set implements FEAT_NMI and FEAT_GICv3_NMI for ARMv8. These
>> introduce support for a new category of interrupts in the architecture
>> which we can use to provide NMI like functionality.
> 
> I had one last loose end I wanted to tidy up, and I got round
> to working through reading the spec about it today. This is
> the question of what the "is NMI enabled?" test should be
> in the code in arm_gicv3_cpuif.c.
> 
> The spec wording isn't always super clear, but there are several
> things here:
> 
>  * FEAT_NMI : the changes to the CPU proper which implement
>superpriority for IRQ and FIQ, PSTATE.ALLINT, etc etc.
>  * FEAT_GICv3_NMI : the changes to the CPU interface for
>GICv3 NMI handling. Any CPU with FEAT_NMI and FEAT_GICv3
>must have this.
>  * NMI support in the IRI (Interrupt Routing Infrastructure,
>i.e. all the bits of the GIC that aren't the cpuif; the
>distributor and redistributors). Table 3-1 in the GIC spec
>says that you can have an IRI without NMI support connected
>to a CPU which does have NMI support. This is what the ID
>register bit GICD_TYPER.NMI reports.

That is right. It seems reasonable for me.

> 
> At the moment this patchset conflates FEAT_GICv3_NMI and
> the NMI support in the IRI. The effect of this is that we
> allow a machine model to create a CPU with FEAT_NMI but
> without FEAT_GICv3_NMI in the cpuif, and we don't allow
> a setup where the CPU and cpuif have NMI support but the
> IRI does not. (This will actually happen with this patchset
> with the sbsa-ref machine and -cpu max, because we haven't
> (yet) made sbsa-ref enable NMI in the GIC device when the
> CPU has NMI support.)
> 
> For a Linux guest this doesn't make much difference, because
> Linux will only enable NMI support if it finds it in both
> the IRI and the CPU, but I think it would be better to
> get the enable-tests right as these can be awkward to change
> after the fact in a backwards-compatible way.
> 
> I think this is easy to fix -- we can add a new bool field
> GICv3CPUState::nmi_support which we initialize in
> gicv3_init_cpuif() if the CPU has FEAT_NMI, and make the
> checks in arm_gicv3_cpuif.c check cs->nmi_support instead
> of cs->gic->nmi_support. That looks like this squashed into
> patch 18:
> 
> diff --git a/include/hw/intc/arm_gicv3_common.h
> b/include/hw/intc/arm_gicv3_common.h
> index 88533749ebb..cd09bee3bc4 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -225,6 +225,13 @@ struct GICv3CPUState {
> 
>  /* This is temporary working state, to avoid a malloc in gicv3_update() 
> */
>  bool seenbetter;
> +
> +/*
> + * Whether the CPU interface has NMI support (FEAT_GICv3_NMI). The
> + * CPU interface may support NMIs even when the GIC proper (what the
> + * spec calls the IRI; the redistributors and distributor) does not.
> + */
> +bool nmi_support;
>  };
> 
>  /*
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index 2457b7bca23..715909d0f7d 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -21,6 +21,7 @@
>  #include "hw/irq.h"
>  #include "cpu.h"
>  #include "target/arm/cpregs.h"
> +#include "target/arm/cpu-features.h"
>  #include "sysemu/tcg.h"
>  #include "sysemu/qtest.h"
> 
> @@ -839,7 +840,7 @@ static int icc_highest_active_prio(GICv3CPUState *cs)
>   */
>  int i;
> 
> -if (cs->gic->nmi_support) {
> +if (cs->nmi_support) {
>  /*
>   * If an NMI is active this takes precedence over anything else
>   * for priority purposes; the NMI bit is only in the AP1R0 bit.
> @@ -1285,7 +1286,7 @@ static void icc_drop_prio(GICv3CPUState *cs, int grp)
>  continue;
>  }
> 
> -if (i == 0 && cs->gic->nmi_support && (*papr & ICC_AP1R_EL1_NMI)) {
> +if (i == 0 && cs->nmi_support && (*papr & ICC_AP1R_EL1_NMI)) {
>  *papr &= (~ICC_AP1R_EL1_NMI);
>  break;
>  }
> @@ -1324,7 +1325,7 @@ static int icc_highest_active_group(GICv3CPUState *cs)
>   */
>  int i;
> 
> -if (cs->gic->nmi_support) {
> +if (cs->nmi_support) {
>  if (cs->icc_apr[GICV3_G1][0] & ICC_AP1R_EL1_NMI) {
>  return GICV3_G1;
>  }
> @@ -1787,7 +1788,7 @@ static void icc_ap_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
>  return;
>  }
> 
> -if (cs->gic->nmi_support) {
> +if (cs->nmi_support) {
>  cs->icc_apr[grp][regno] = value & (0xU | ICC_AP1R_EL1_NMI);
>  } else {
>  cs->icc_apr[grp][regno] = value & 0xU;
> @@ -1901,7 +1902,7 @@ static uint64_t icc_rpr_read(CPUARMState *env,
> const ARMCPRegInfo *ri)
>  }
>  }
> 
> -if (cs->gic->nmi_support) {
> +if (cs->nmi_support) {
>  /* NMI info is reported in the high bits of RPR */
>  if 

[PATCH] hw/virtio: Fix obtain the buffer id from the last descriptor

2024-04-21 Thread Wafer
The virtio-1.3 specification
 writes:
2.8.6 Next Flag: Descriptor Chaining
  Buffer ID is included in the last descriptor in the list.

If the feature (_F_INDIRECT_DESC) has been negotiated, install only
one descriptor in the virtqueue.
Therefor the buffer id should be obtained from the first descriptor.

In descriptor chaining scenarios, the buffer id should be obtained
from the last descriptor.

Fixes: 86044b24e8 ("virtio: basic packed virtqueue support")

Signed-off-by: Wafer 
---
 hw/virtio/virtio.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 871674f9be..f65d4b4161 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1739,6 +1739,11 @@ static void *virtqueue_packed_pop(VirtQueue *vq, size_t 
sz)
 goto err_undo_map;
 }
 
+if (desc_cache != _desc_cache) {
+/* Buffer ID is included in the last descriptor in the list. */
+id = desc.id;
+}
+
 rc = virtqueue_packed_read_next_desc(vq, , desc_cache, max, ,
  desc_cache ==
  _desc_cache);
-- 
2.27.0




Re: [PATCH 24/24] exec: Remove unnecessary inclusions of 'hw/core/cpu.h'

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

When "hw/core/cpu.h" is not required, remove it.

Signed-off-by: Philippe Mathieu-Daudé
---
  include/exec/cpu-all.h| 1 -
  include/exec/cpu-defs.h   | 1 -
  include/hw/boards.h   | 1 -
  include/hw/ppc/openpic.h  | 1 -
  include/sysemu/hw_accel.h | 1 -
  5 files changed, 5 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 23/24] exec: Remove 'disas/disas.h' from 'exec/log.h'

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

"exec/log.h" doesn't require "disas/disas.h". Remove it,
including it in the sources when required.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/log.h   | 1 -
  target/avr/translate.c   | 1 +
  target/hexagon/translate.c   | 1 +
  target/loongarch/tcg/translate.c | 1 +
  target/rx/translate.c| 1 +
  tcg/tcg.c| 1 +
  6 files changed, 5 insertions(+), 1 deletion(-)


Most of these additions are obviated by
https://patchew.org/QEMU/20240405102459.462551-1-richard.hender...@linaro.org/


r~



Re: [PATCH] target/riscv: Use get_address() to get address with Zicbom extensions

2024-04-21 Thread Daniel Henrique Barboza




On 4/19/24 08:05, Philippe Mathieu-Daudé wrote:

We need to use get_address() to get an address from cpu_gpr[],
since $zero is "special" (NULL).

Fixes: e05da09b7c ("target/riscv: implement Zicbom extension")
Reported-by: Zhiwei Jiang (姜智伟) 
Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Daniel Henrique Barboza 


  target/riscv/insn_trans/trans_rvzicbo.c.inc | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvzicbo.c.inc 
b/target/riscv/insn_trans/trans_rvzicbo.c.inc
index d5d7095903..6f6b29598d 100644
--- a/target/riscv/insn_trans/trans_rvzicbo.c.inc
+++ b/target/riscv/insn_trans/trans_rvzicbo.c.inc
@@ -31,27 +31,27 @@
  static bool trans_cbo_clean(DisasContext *ctx, arg_cbo_clean *a)
  {
  REQUIRE_ZICBOM(ctx);
-gen_helper_cbo_clean_flush(tcg_env, cpu_gpr[a->rs1]);
+gen_helper_cbo_clean_flush(tcg_env, get_address(ctx, a->rs1, 0));
  return true;
  }
  
  static bool trans_cbo_flush(DisasContext *ctx, arg_cbo_flush *a)

  {
  REQUIRE_ZICBOM(ctx);
-gen_helper_cbo_clean_flush(tcg_env, cpu_gpr[a->rs1]);
+gen_helper_cbo_clean_flush(tcg_env, get_address(ctx, a->rs1, 0));
  return true;
  }
  
  static bool trans_cbo_inval(DisasContext *ctx, arg_cbo_inval *a)

  {
  REQUIRE_ZICBOM(ctx);
-gen_helper_cbo_inval(tcg_env, cpu_gpr[a->rs1]);
+gen_helper_cbo_inval(tcg_env, get_address(ctx, a->rs1, 0));
  return true;
  }
  
  static bool trans_cbo_zero(DisasContext *ctx, arg_cbo_zero *a)

  {
  REQUIRE_ZICBOZ(ctx);
-gen_helper_cbo_zero(tcg_env, cpu_gpr[a->rs1]);
+gen_helper_cbo_zero(tcg_env, get_address(ctx, a->rs1, 0));
  return true;
  }




Re: [PATCH 21/24] plugins: Un-inline qemu_plugin_disable_mem_helpers()

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

"qemu/plugin.h" only include the huge "hw/core/cpu.h"
because qemu_plugin_disable_mem_helpers() accesses
CPUState::plugin_mem_cbs. In order to avoid including
it, un-inline qemu_plugin_disable_mem_helpers().

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/qemu/plugin.h | 6 +-
  plugins/core.c| 5 +
  2 files changed, 6 insertions(+), 5 deletions(-)



I don't see how this helps.
All of the places that use qemu_plugin_disable_mem_helpers also have cpu.h.


r~



Re: [PATCH 19/24] gdbstub: Avoid including 'cpu.h' in 'gdbstub/helpers.h'

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

We only need the "exec/tswap.h" and "cpu-param.h" headers.
Only include "cpu.h" in the target gdbstub.c source files.

Signed-off-by: Philippe Mathieu-Daudé
---
  include/gdbstub/helpers.h | 3 ++-
  target/avr/gdbstub.c  | 1 +
  target/tricore/gdbstub.c  | 1 +
  3 files changed, 4 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 18/24] plugins: Include missing 'qemu/bitmap.h' header

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

"qemu/plugin.h" uses DECLARE_BITMAP(), which is
declared in "qemu/bitmap.h".

Signed-off-by: Philippe Mathieu-Daudé
---
  include/qemu/plugin.h | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 17/24] exec: Include missing 'qemu/log-for-trace.h' header in 'exec/log.h'

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

"exec/log.h" accesses the qemu_loglevel variable,
which is declared in "qemu/log-for-trace.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/log.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/include/exec/log.h b/include/exec/log.h
index 4a7375a45f..e0ff778a10 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -2,6 +2,7 @@
  #define QEMU_EXEC_LOG_H
  
  #include "qemu/log.h"

+#include "qemu/log-for-trace.h"
  #include "hw/core/cpu.h"
  #include "disas/disas.h"
  


I disagree: qemu/log.h is the main file; log-for-trace.h was split out for other usage. 
That shouldn't mean that log-for-trace.h needs to be spread around.



r~



Re: [PATCH 13/24] target/sparc: Replace abi_ulong by uint32_t for TARGET_ABI32

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

We have abi_ulong == uint32_t for the 32-bit ABI.
Use the generic type to avoid to depend on the
"exec/user/abitypes.h" header.

Signed-off-by: Philippe Mathieu-Daudé
---
  target/sparc/gdbstub.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 12/24] exec: Move CPUTLBEntry helpers to cputlb.c

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

The following CPUTLBEntry helpers are only used in accel/tcg/cputlb.c:
   - tlb_index()
   - tlb_entry()
   - tlb_read_idx()
   - tlb_addr_write()

Move them to this file, allowing to remove the huge "cpu.h" header
inclusion from "exec/cpu_ldst.h".

Signed-off-by: Philippe Mathieu-Daudé
---
  include/exec/cpu_ldst.h | 55 -
  accel/tcg/cputlb.c  | 51 ++
  2 files changed, 51 insertions(+), 55 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 10/24] exec: Reduce tlb_set_dirty() declaration scope

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

tlb_set_dirty() is only used in accel/tcg/cputlb.c,
where it is defined. Declare it statically, removing
the stub.

Signed-off-by: Philippe Mathieu-Daudé
---
  include/exec/exec-all.h | 1 -
  accel/stubs/tcg-stub.c  | 4 
  accel/tcg/cputlb.c  | 2 +-
  3 files changed, 1 insertion(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 08/24] physmem: Move TCG CPU IOTLB methods around

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

The next commit will restrict TCG specific code in physmem.c
using some #ifdef'ry. In order to keep it simple, move
iotlb_to_section() and memory_region_section_get_iotlb()
around close together.

Signed-off-by: Philippe Mathieu-Daudé
---
  system/physmem.c | 50 
  1 file changed, 25 insertions(+), 25 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 07/24] exec: Un-inline tlb_vaddr_to_host() and declare it in 'exec/cputlb.h'

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

Declare tlb_vaddr_to_host() in "exec/cputlb.h" with the CPU TLB
API. Un-inline the user emulation definition to avoid including
"exec/cpu_ldst.h" (which declares g2h) in "exec/cputlb.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/cpu_ldst.h  | 24 
  include/exec/cputlb.h| 18 ++
  accel/tcg/user-exec.c|  7 +++
  target/arm/tcg/helper-a64.c  |  1 +
  target/riscv/vector_helper.c |  1 +
  target/sparc/mmu_helper.c|  1 +
  6 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 63186b07e4..7032949dba 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -418,28 +418,4 @@ static inline int cpu_ldsw_code(CPUArchState *env, abi_ptr 
addr)
  return (int16_t)cpu_lduw_code(env, addr);
  }
  
-/**

- * tlb_vaddr_to_host:
- * @env: CPUArchState
- * @addr: guest virtual address to look up
- * @access_type: 0 for read, 1 for write, 2 for execute
- * @mmu_idx: MMU index to use for lookup
- *
- * Look up the specified guest virtual index in the TCG softmmu TLB.
- * If we can translate a host virtual address suitable for direct RAM
- * access, without causing a guest exception, then return it.
- * Otherwise (TLB entry is for an I/O access, guest software
- * TLB fill required, etc) return NULL.
- */
-#ifdef CONFIG_USER_ONLY
-static inline void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
-  MMUAccessType access_type, int mmu_idx)
-{
-return g2h(env_cpu(env), addr);
-}
-#else
-void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
-MMUAccessType access_type, int mmu_idx);
-#endif
-
  #endif /* CPU_LDST_H */
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index ef18642a32..173eb98b9a 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -20,10 +20,28 @@
  #ifndef CPUTLB_H
  #define CPUTLB_H
  
+#include "exec/abi_ptr.h"

  #include "exec/cpu-common.h"
+#include "exec/mmu-access-type.h"
  
  #ifdef CONFIG_TCG
  
+/**

+ * tlb_vaddr_to_host:
+ * @env: CPUArchState
+ * @addr: guest virtual address to look up
+ * @access_type: 0 for read, 1 for write, 2 for execute
+ * @mmu_idx: MMU index to use for lookup
+ *
+ * Look up the specified guest virtual index in the TCG softmmu TLB.
+ * If we can translate a host virtual address suitable for direct RAM
+ * access, without causing a guest exception, then return it.
+ * Otherwise (TLB entry is for an I/O access, guest software
+ * TLB fill required, etc) return NULL.
+ */
+void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
+MMUAccessType access_type, int mmu_idx);
+


Why have you chosen cputlb.h, when the other probe functions are in exec-all.h?

Alternately, we only have two users remaining, which could be migrated to the newer 
interfaces...



r~



Re: [PATCH 06/24] exec: Have guest_addr_valid() methods take abi_ptr/size_t arguments

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

abi_ulong is target specific, replace by abi_ptr which isn't.
Use size_t for the @len type.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/cpu_ldst.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index f3c2a3ca74..63186b07e4 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -92,12 +92,12 @@ static inline void *g2h(CPUState *cs, abi_ptr x)
  return g2h_untagged(cpu_untagged_addr(cs, x));
  }
  
-static inline bool guest_addr_valid_untagged(abi_ulong x)

+static inline bool guest_addr_valid_untagged(abi_ptr x)
  {
  return x <= GUEST_ADDR_MAX;
  }
  
-static inline bool guest_range_valid_untagged(abi_ulong start, abi_ulong len)

+static inline bool guest_range_valid_untagged(abi_ptr start, size_t len)


No, this needs to remain a large type for 64-bit guest on 32-bit host.
(When will that ever die...)


r~



Re: [PATCH 05/24] exec: Restrict 'cpu_ldst.h' to TCG accelerator

2024-04-21 Thread Richard Henderson

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:

"exec/cpu_ldst.h" is specific to TCG, do not allow its
inclusion from other accelerators.

Signed-off-by: Philippe Mathieu-Daudé
---
  include/exec/cpu_ldst.h | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



[PATCH] hw/misc : Correct 5 spaces indents in stm32l4x5_exti

2024-04-21 Thread Inès Varhol
Signed-off-by: Inès Varhol 
---
 hw/misc/stm32l4x5_exti.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c
index 9fd859160d..5c55ee4268 100644
--- a/hw/misc/stm32l4x5_exti.c
+++ b/hw/misc/stm32l4x5_exti.c
@@ -59,22 +59,22 @@ static const uint32_t exti_romask[EXTI_NUM_REGISTER] = {
 
 static unsigned regbank_index_by_irq(unsigned irq)
 {
- return irq >= EXTI_MAX_IRQ_PER_BANK ? 1 : 0;
+return irq >= EXTI_MAX_IRQ_PER_BANK ? 1 : 0;
 }
 
 static unsigned regbank_index_by_addr(hwaddr addr)
 {
- return addr >= EXTI_IMR2 ? 1 : 0;
+return addr >= EXTI_IMR2 ? 1 : 0;
 }
 
 static unsigned valid_mask(unsigned bank)
 {
- return MAKE_64BIT_MASK(0, irqs_per_bank[bank]);
+return MAKE_64BIT_MASK(0, irqs_per_bank[bank]);
 }
 
 static unsigned configurable_mask(unsigned bank)
 {
- return valid_mask(bank) & ~exti_romask[bank];
+return valid_mask(bank) & ~exti_romask[bank];
 }
 
 static void stm32l4x5_exti_reset_hold(Object *obj)
-- 
2.43.2




[PATCH v5 5/5] tests/qtest : Add testcase for DM163

2024-04-21 Thread Inès Varhol
`test_dm163_bank()`
Checks that the pin "sout" of the DM163 led driver outputs the values
received on pin "sin" with the expected latency (depending on the bank).

`test_dm163_gpio_connection()`
Check that changes to relevant STM32L4x5 GPIO pins are propagated to the
DM163 device.

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
Acked-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
---
 tests/qtest/dm163-test.c | 194 +++
 tests/qtest/meson.build  |   2 +
 2 files changed, 196 insertions(+)
 create mode 100644 tests/qtest/dm163-test.c

diff --git a/tests/qtest/dm163-test.c b/tests/qtest/dm163-test.c
new file mode 100644
index 00..3161c9208d
--- /dev/null
+++ b/tests/qtest/dm163-test.c
@@ -0,0 +1,194 @@
+/*
+ * QTest testcase for DM163
+ *
+ * Copyright (C) 2024 Samuel Tardieu 
+ * Copyright (C) 2024 Arnaud Minier 
+ * Copyright (C) 2024 Inès Varhol 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+enum DM163_INPUTS {
+SIN = 8,
+DCK = 9,
+RST_B = 10,
+LAT_B = 11,
+SELBK = 12,
+EN_B = 13
+};
+
+#define DEVICE_NAME "/machine/dm163"
+#define GPIO_OUT(name, value) qtest_set_irq_in(qts, DEVICE_NAME, NULL, name,   
\
+   value)
+#define GPIO_PULSE(name)   
\
+  do { 
\
+GPIO_OUT(name, 1); 
\
+GPIO_OUT(name, 0); 
\
+  } while (0)
+
+
+static void rise_gpio_pin_dck(QTestState *qts)
+{
+/* Configure output mode for pin PB1 */
+qtest_writel(qts, 0x48000400, 0xFEB7);
+/* Write 1 in ODR for PB1 */
+qtest_writel(qts, 0x48000414, 0x0002);
+}
+
+static void lower_gpio_pin_dck(QTestState *qts)
+{
+/* Configure output mode for pin PB1 */
+qtest_writel(qts, 0x48000400, 0xFEB7);
+/* Write 0 in ODR for PB1 */
+qtest_writel(qts, 0x48000414, 0x);
+}
+
+static void rise_gpio_pin_selbk(QTestState *qts)
+{
+/* Configure output mode for pin PC5 */
+qtest_writel(qts, 0x48000800, 0xF7FF);
+/* Write 1 in ODR for PC5 */
+qtest_writel(qts, 0x48000814, 0x0020);
+}
+
+static void lower_gpio_pin_selbk(QTestState *qts)
+{
+/* Configure output mode for pin PC5 */
+qtest_writel(qts, 0x48000800, 0xF7FF);
+/* Write 0 in ODR for PC5 */
+qtest_writel(qts, 0x48000814, 0x);
+}
+
+static void rise_gpio_pin_lat_b(QTestState *qts)
+{
+/* Configure output mode for pin PC4 */
+qtest_writel(qts, 0x48000800, 0xFDFF);
+/* Write 1 in ODR for PC4 */
+qtest_writel(qts, 0x48000814, 0x0010);
+}
+
+static void lower_gpio_pin_lat_b(QTestState *qts)
+{
+/* Configure output mode for pin PC4 */
+qtest_writel(qts, 0x48000800, 0xFDFF);
+/* Write 0 in ODR for PC4 */
+qtest_writel(qts, 0x48000814, 0x);
+}
+
+static void rise_gpio_pin_rst_b(QTestState *qts)
+{
+/* Configure output mode for pin PC3 */
+qtest_writel(qts, 0x48000800, 0xFF7F);
+/* Write 1 in ODR for PC3 */
+qtest_writel(qts, 0x48000814, 0x0008);
+}
+
+static void lower_gpio_pin_rst_b(QTestState *qts)
+{
+/* Configure output mode for pin PC3 */
+qtest_writel(qts, 0x48000800, 0xFF7F);
+/* Write 0 in ODR for PC3 */
+qtest_writel(qts, 0x48000814, 0x);
+}
+
+static void rise_gpio_pin_sin(QTestState *qts)
+{
+/* Configure output mode for pin PA4 */
+qtest_writel(qts, 0x4800, 0xFDFF);
+/* Write 1 in ODR for PA4 */
+qtest_writel(qts, 0x4814, 0x0010);
+}
+
+static void lower_gpio_pin_sin(QTestState *qts)
+{
+/* Configure output mode for pin PA4 */
+qtest_writel(qts, 0x4800, 0xFDFF);
+/* Write 0 in ODR for PA4 */
+qtest_writel(qts, 0x4814, 0x);
+}
+
+static void test_dm163_bank(const void *opaque)
+{
+const unsigned bank = (uintptr_t) opaque;
+const int width = bank ? 192 : 144;
+
+QTestState *qts = qtest_initf("-M b-l475e-iot01a");
+qtest_irq_intercept_out_named(qts, DEVICE_NAME, "sout");
+GPIO_OUT(RST_B, 1);
+GPIO_OUT(EN_B, 0);
+GPIO_OUT(DCK, 0);
+GPIO_OUT(SELBK, bank);
+GPIO_OUT(LAT_B, 1);
+
+/* Fill bank with zeroes */
+GPIO_OUT(SIN, 0);
+for (int i = 0; i < width; i++) {
+GPIO_PULSE(DCK);
+}
+/* Fill bank with ones, check that we get the previous zeroes */
+GPIO_OUT(SIN, 1);
+for (int i = 0; i < width; i++) {
+GPIO_PULSE(DCK);
+g_assert(!qtest_get_irq(qts, 0));
+}
+
+/* Pulse one more bit in the bank, check that we get a one */
+GPIO_PULSE(DCK);
+g_assert(qtest_get_irq(qts, 0));
+
+qtest_quit(qts);
+}
+
+static void test_dm163_gpio_connection(void)
+{
+QTestState *qts = qtest_init("-M b-l475e-iot01a");
+

[PATCH v5 1/5] hw/display : Add device DM163

2024-04-21 Thread Inès Varhol
This device implements the IM120417002 colors shield v1.1 for Arduino
(which relies on the DM163 8x3-channel led driving logic) and features
a simple display of an 8x8 RGB matrix. The columns of the matrix are
driven by the DM163 and the rows are driven externally.

Acked-by: Alistair Francis 
Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
---
 docs/system/arm/b-l475e-iot01a.rst |   3 +-
 include/hw/display/dm163.h |  59 +
 hw/display/dm163.c | 334 +
 hw/display/Kconfig |   3 +
 hw/display/meson.build |   1 +
 hw/display/trace-events|  14 ++
 6 files changed, 413 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/display/dm163.h
 create mode 100644 hw/display/dm163.c

diff --git a/docs/system/arm/b-l475e-iot01a.rst 
b/docs/system/arm/b-l475e-iot01a.rst
index 0afef8e4f4..91de5e82fc 100644
--- a/docs/system/arm/b-l475e-iot01a.rst
+++ b/docs/system/arm/b-l475e-iot01a.rst
@@ -12,13 +12,14 @@ USART, I2C, SPI, CAN and USB OTG, as well as a variety of 
sensors.
 Supported devices
 "
 
-Currently B-L475E-IOT01A machine's only supports the following devices:
+Currently B-L475E-IOT01A machines support the following devices:
 
 - Cortex-M4F based STM32L4x5 SoC
 - STM32L4x5 EXTI (Extended interrupts and events controller)
 - STM32L4x5 SYSCFG (System configuration controller)
 - STM32L4x5 RCC (Reset and clock control)
 - STM32L4x5 GPIOs (General-purpose I/Os)
+- optional 8x8 led display (based on DM163 driver)
 
 Missing devices
 """
diff --git a/include/hw/display/dm163.h b/include/hw/display/dm163.h
new file mode 100644
index 00..4377f77bb7
--- /dev/null
+++ b/include/hw/display/dm163.h
@@ -0,0 +1,59 @@
+/*
+ * QEMU DM163 8x3-channel constant current led driver
+ * driving columns of associated 8x8 RGB matrix.
+ *
+ * Copyright (C) 2024 Samuel Tardieu 
+ * Copyright (C) 2024 Arnaud Minier 
+ * Copyright (C) 2024 Inès Varhol 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_DISPLAY_DM163_H
+#define HW_DISPLAY_DM163_H
+
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_DM163 "dm163"
+OBJECT_DECLARE_SIMPLE_TYPE(DM163State, DM163);
+
+#define RGB_MATRIX_NUM_ROWS 8
+#define RGB_MATRIX_NUM_COLS 8
+#define DM163_NUM_LEDS (RGB_MATRIX_NUM_COLS * 3)
+/* The last row is filled with 0 (turned off row) */
+#define COLOR_BUFFER_SIZE (RGB_MATRIX_NUM_ROWS + 1)
+
+typedef struct DM163State {
+DeviceState parent_obj;
+
+/* DM163 driver */
+uint64_t bank0_shift_register[3];
+uint64_t bank1_shift_register[3];
+uint16_t latched_outputs[DM163_NUM_LEDS];
+uint16_t outputs[DM163_NUM_LEDS];
+qemu_irq sout;
+
+uint8_t sin;
+uint8_t dck;
+uint8_t rst_b;
+uint8_t lat_b;
+uint8_t selbk;
+uint8_t en_b;
+
+/* IM120417002 colors shield */
+uint8_t activated_rows;
+
+/* 8x8 RGB matrix */
+QemuConsole *console;
+uint8_t redraw;
+/* Rows currently being displayed on the matrix. */
+/* The last row is filled with 0 (turned off row) */
+uint32_t buffer[COLOR_BUFFER_SIZE][RGB_MATRIX_NUM_COLS];
+uint8_t last_buffer_idx;
+uint8_t buffer_idx_of_row[RGB_MATRIX_NUM_ROWS];
+/* Used to simulate retinal persistence of rows */
+uint8_t row_persistence_delay[RGB_MATRIX_NUM_ROWS];
+} DM163State;
+
+#endif /* HW_DISPLAY_DM163_H */
diff --git a/hw/display/dm163.c b/hw/display/dm163.c
new file mode 100644
index 00..9079e6604e
--- /dev/null
+++ b/hw/display/dm163.c
@@ -0,0 +1,334 @@
+/*
+ * QEMU DM163 8x3-channel constant current led driver
+ * driving columns of associated 8x8 RGB matrix.
+ *
+ * Copyright (C) 2024 Samuel Tardieu 
+ * Copyright (C) 2024 Arnaud Minier 
+ * Copyright (C) 2024 Inès Varhol 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * The reference used for the DM163 is the following :
+ * http://www.siti.com.tw/product/spec/LED/DM163.pdf
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/display/dm163.h"
+#include "ui/console.h"
+#include "trace.h"
+
+#define LED_SQUARE_SIZE 100
+/* Number of frames a row stays visible after being turned off. */
+#define ROW_PERSISTENCE 3
+#define TURNED_OFF_ROW (COLOR_BUFFER_SIZE - 1)
+
+static const VMStateDescription vmstate_dm163 = {
+.name = TYPE_DM163,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT64_ARRAY(bank0_shift_register, DM163State, 3),
+VMSTATE_UINT64_ARRAY(bank1_shift_register, DM163State, 3),
+VMSTATE_UINT16_ARRAY(latched_outputs, DM163State, DM163_NUM_LEDS),
+VMSTATE_UINT16_ARRAY(outputs, DM163State, DM163_NUM_LEDS),
+VMSTATE_UINT8(dck, DM163State),
+VMSTATE_UINT8(en_b, DM163State),
+VMSTATE_UINT8(lat_b, DM163State),
+VMSTATE_UINT8(rst_b, DM163State),
+

[PATCH v5 0/5] Add device DM163 (led driver, matrix colors shield & display)

2024-04-21 Thread Inès Varhol
This device implements the IM120417002 colors shield v1.1 for Arduino
(which relies on the DM163 8x3-channel led driving logic) and features
a simple display of an 8x8 RGB matrix. This color shield can be plugged
on the Arduino board (or the B-L475E-IOT01A board) to drive an 8x8
RGB led matrix. This RGB led matrix takes advantage of retinal persistance
to seemingly display different colors in each row.

Hello,

I'm planning to add a more thorough test of the display functionality
in a later patch, based on the avocado test  `machine_m68k_nextcube.py`.

Thank you for the reviews.

Changes from v4 :
dm163
- redefine `DM163_NUM_LEDS` for clarity
- change definition of `COLOR_BUFFER_SIZE`
- rename `age_of_row` to `row_persistance_delay`
- remove unnecessary QOM cast macro in GPIO handlers
- remove unnecessary inline of `dm163_bank0` and `dm163_bank1`
- replace occurrences of number 8 by the right define macro
- use unsigned type to print GPIO input `new_stateq`
STM32L4x5 qtests
- add comment to explain GPIO forwarding to SoC
B-L475E-IOT01A
- correct formatting
- use unsigned for gpio pins' indices
DM163 qtest
- use an enum for dm163 inputs
- inline ['dm163-test'] in meson.build (I don't plan on adding qtests)
OTHER
- update copyrights to 2023-2024

Changes from v3 (review of the 1st commit by Peter Maydell) :
- dm163.c : instead of redrawing the entire console each frame,
only redraw the rows that changed using a new variable `redraw`
- reset all the fields in `dm163_reset_hold`
- correcting typos : persistance -> persistence
- b-l475e-iot01a.rst : correcting typo

Changes from v2 : Corrected typo in the Based-on message id

Changes from v1 :
- moving the DM163 from the SoC to the B-L475E-IOT01A machine
(changing config files and tests accordingly)
- restricting DM163 test to ARM & DM163 availability
- using `object_class_by_name()` to check for DM163 presence at run-time
- exporting SYSCFG inputs to the SoC (and adapting tests accordingly)

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 

Inès Varhol (5):
  hw/display : Add device DM163
  hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC
  hw/arm : Create Bl475eMachineState
  hw/arm : Connect DM163 to B-L475E-IOT01A
  tests/qtest : Add testcase for DM163

 docs/system/arm/b-l475e-iot01a.rst  |   3 +-
 include/hw/display/dm163.h  |  59 +
 hw/arm/b-l475e-iot01a.c | 105 +++--
 hw/arm/stm32l4x5_soc.c  |   6 +-
 hw/display/dm163.c  | 334 
 tests/qtest/dm163-test.c| 194 
 tests/qtest/stm32l4x5_gpio-test.c   |  13 +-
 tests/qtest/stm32l4x5_syscfg-test.c |  17 +-
 hw/arm/Kconfig  |   1 +
 hw/display/Kconfig  |   3 +
 hw/display/meson.build  |   1 +
 hw/display/trace-events |  14 ++
 tests/qtest/meson.build |   2 +
 13 files changed, 721 insertions(+), 31 deletions(-)
 create mode 100644 include/hw/display/dm163.h
 create mode 100644 hw/display/dm163.c
 create mode 100644 tests/qtest/dm163-test.c

-- 
2.43.2




[PATCH v5 2/5] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC

2024-04-21 Thread Inès Varhol
Exposing SYSCFG inputs to the SoC is practical in order to wire the SoC
to the optional DM163 display from the board code (GPIOs outputs need
to be connected to both SYSCFG inputs and DM163 inputs).

STM32L4x5 SYSCFG in-irq interception needed to be changed accordingly.

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/arm/stm32l4x5_soc.c  |  6 --
 tests/qtest/stm32l4x5_gpio-test.c   | 13 -
 tests/qtest/stm32l4x5_syscfg-test.c | 17 ++---
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 40e294f838..0332b67701 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -1,8 +1,8 @@
 /*
  * STM32L4x5 SoC family
  *
- * Copyright (c) 2023 Arnaud Minier 
- * Copyright (c) 2023 Inès Varhol 
+ * Copyright (c) 2023-2024 Arnaud Minier 
+ * Copyright (c) 2023-2024 Inès Varhol 
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  *
@@ -221,6 +221,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
Error **errp)
 }
 }
 
+qdev_pass_gpios(DEVICE(>syscfg), dev_soc, NULL);
+
 /* EXTI device */
 busdev = SYS_BUS_DEVICE(>exti);
 if (!sysbus_realize(busdev, errp)) {
diff --git a/tests/qtest/stm32l4x5_gpio-test.c 
b/tests/qtest/stm32l4x5_gpio-test.c
index 0f6bda54d3..72a7823406 100644
--- a/tests/qtest/stm32l4x5_gpio-test.c
+++ b/tests/qtest/stm32l4x5_gpio-test.c
@@ -43,6 +43,9 @@
 #define OTYPER_PUSH_PULL 0
 #define OTYPER_OPEN_DRAIN 1
 
+/* SoC forwards GPIOs to SysCfg */
+#define SYSCFG "/machine/soc"
+
 const uint32_t moder_reset[NUM_GPIOS] = {
 0xABFF,
 0xFEBF,
@@ -284,7 +287,7 @@ static void test_gpio_output_mode(const void *data)
 uint32_t gpio = test_gpio_addr(data);
 unsigned int gpio_id = get_gpio_id(gpio);
 
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
 
 /* Set a bit in ODR and check nothing happens */
 gpio_set_bit(gpio, ODR, pin, 1);
@@ -319,7 +322,7 @@ static void test_gpio_input_mode(const void *data)
 uint32_t gpio = test_gpio_addr(data);
 unsigned int gpio_id = get_gpio_id(gpio);
 
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
 
 /* Configure a line as input, raise it, and check that the pin is high */
 gpio_set_2bits(gpio, MODER, pin, MODER_INPUT);
@@ -348,7 +351,7 @@ static void test_pull_up_pull_down(const void *data)
 uint32_t gpio = test_gpio_addr(data);
 unsigned int gpio_id = get_gpio_id(gpio);
 
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
 
 /* Configure a line as input with pull-up, check the line is set high */
 gpio_set_2bits(gpio, MODER, pin, MODER_INPUT);
@@ -378,7 +381,7 @@ static void test_push_pull(const void *data)
 uint32_t gpio = test_gpio_addr(data);
 uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
 
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
 
 /* Setting a line high externally, configuring it in push-pull output */
 /* And checking the pin was disconnected */
@@ -425,7 +428,7 @@ static void test_open_drain(const void *data)
 uint32_t gpio = test_gpio_addr(data);
 uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
 
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
 
 /* Setting a line high externally, configuring it in open-drain output */
 /* And checking the pin was disconnected */
diff --git a/tests/qtest/stm32l4x5_syscfg-test.c 
b/tests/qtest/stm32l4x5_syscfg-test.c
index ed4801798d..733b42df55 100644
--- a/tests/qtest/stm32l4x5_syscfg-test.c
+++ b/tests/qtest/stm32l4x5_syscfg-test.c
@@ -1,8 +1,8 @@
 /*
  * QTest testcase for STM32L4x5_SYSCFG
  *
- * Copyright (c) 2023 Arnaud Minier 
- * Copyright (c) 2023 Inès Varhol 
+ * Copyright (c) 2024 Arnaud Minier 
+ * Copyright (c) 2024 Inès Varhol 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -25,6 +25,10 @@
 #define SYSCFG_SWPR2 0x28
 #define INVALID_ADDR 0x2C
 
+/* SoC forwards GPIOs to SysCfg */
+#define SYSCFG "/machine/soc"
+#define EXTI "/machine/soc/exti"
+
 static void syscfg_writel(unsigned int offset, uint32_t value)
 {
 writel(SYSCFG_BASE_ADDR + offset, value);
@@ -37,8 +41,7 @@ static uint32_t syscfg_readl(unsigned int offset)
 
 static void syscfg_set_irq(int num, int level)
 {
-   qtest_set_irq_in(global_qtest, "/machine/soc/syscfg",
-NULL, num, level);
+   qtest_set_irq_in(global_qtest, SYSCFG, NULL, num, level);
 }
 
 static void system_reset(void)
@@ -197,7 +200,7 @@ static void test_interrupt(void)
  * Test that GPIO rising lines result in an irq
   

[PATCH v5 3/5] hw/arm : Create Bl475eMachineState

2024-04-21 Thread Inès Varhol
Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/arm/b-l475e-iot01a.c | 46 -
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/hw/arm/b-l475e-iot01a.c b/hw/arm/b-l475e-iot01a.c
index d862aa43fc..970c637ce6 100644
--- a/hw/arm/b-l475e-iot01a.c
+++ b/hw/arm/b-l475e-iot01a.c
@@ -2,8 +2,8 @@
  * B-L475E-IOT01A Discovery Kit machine
  * (B-L475E-IOT01A IoT Node)
  *
- * Copyright (c) 2023 Arnaud Minier 
- * Copyright (c) 2023 Inès Varhol 
+ * Copyright (c) 2023-2024 Arnaud Minier 
+ * Copyright (c) 2023-2024 Inès Varhol 
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  *
@@ -32,33 +32,51 @@
 
 /* B-L475E-IOT01A implementation is derived from netduinoplus2 */
 
-static void b_l475e_iot01a_init(MachineState *machine)
+#define TYPE_B_L475E_IOT01A MACHINE_TYPE_NAME("b-l475e-iot01a")
+OBJECT_DECLARE_SIMPLE_TYPE(Bl475eMachineState, B_L475E_IOT01A)
+
+typedef struct Bl475eMachineState {
+MachineState parent_obj;
+
+Stm32l4x5SocState soc;
+} Bl475eMachineState;
+
+static void bl475e_init(MachineState *machine)
 {
+Bl475eMachineState *s = B_L475E_IOT01A(machine);
 const Stm32l4x5SocClass *sc;
-DeviceState *dev;
 
-dev = qdev_new(TYPE_STM32L4X5XG_SOC);
-object_property_add_child(OBJECT(machine), "soc", OBJECT(dev));
-sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+object_initialize_child(OBJECT(machine), "soc", >soc,
+TYPE_STM32L4X5XG_SOC);
+sysbus_realize(SYS_BUS_DEVICE(>soc), _fatal);
 
-sc = STM32L4X5_SOC_GET_CLASS(dev);
-armv7m_load_kernel(ARM_CPU(first_cpu),
-   machine->kernel_filename,
-   0, sc->flash_size);
+sc = STM32L4X5_SOC_GET_CLASS(>soc);
+armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0,
+   sc->flash_size);
 }
 
-static void b_l475e_iot01a_machine_init(MachineClass *mc)
+static void bl475e_machine_init(ObjectClass *oc, void *data)
 {
+MachineClass *mc = MACHINE_CLASS(oc);
 static const char *machine_valid_cpu_types[] = {
 ARM_CPU_TYPE_NAME("cortex-m4"),
 NULL
 };
 mc->desc = "B-L475E-IOT01A Discovery Kit (Cortex-M4)";
-mc->init = b_l475e_iot01a_init;
+mc->init = bl475e_init;
 mc->valid_cpu_types = machine_valid_cpu_types;
 
 /* SRAM pre-allocated as part of the SoC instantiation */
 mc->default_ram_size = 0;
 }
 
-DEFINE_MACHINE("b-l475e-iot01a", b_l475e_iot01a_machine_init)
+static const TypeInfo bl475e_machine_type[] = {
+{
+.name   = TYPE_B_L475E_IOT01A,
+.parent = TYPE_MACHINE,
+.instance_size  = sizeof(Bl475eMachineState),
+.class_init = bl475e_machine_init,
+}
+};
+
+DEFINE_TYPES(bl475e_machine_type)
-- 
2.43.2




[PATCH v5 4/5] hw/arm : Connect DM163 to B-L475E-IOT01A

2024-04-21 Thread Inès Varhol
Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/arm/b-l475e-iot01a.c | 59 +++--
 hw/arm/Kconfig  |  1 +
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/hw/arm/b-l475e-iot01a.c b/hw/arm/b-l475e-iot01a.c
index 970c637ce6..5002a40f06 100644
--- a/hw/arm/b-l475e-iot01a.c
+++ b/hw/arm/b-l475e-iot01a.c
@@ -27,10 +27,37 @@
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
 #include "qemu/error-report.h"
-#include "hw/arm/stm32l4x5_soc.h"
 #include "hw/arm/boot.h"
+#include "hw/core/split-irq.h"
+#include "hw/arm/stm32l4x5_soc.h"
+#include "hw/gpio/stm32l4x5_gpio.h"
+#include "hw/display/dm163.h"
+
+/* B-L475E-IOT01A implementation is inspired from netduinoplus2 and arduino */
 
-/* B-L475E-IOT01A implementation is derived from netduinoplus2 */
+/*
+ * There are actually 14 input pins in the DM163 device.
+ * Here the DM163 input pin EN isn't connected to the STM32L4x5
+ * GPIOs as the IM120417002 colors shield doesn't actually use
+ * this pin to drive the RGB matrix.
+ */
+#define NUM_DM163_INPUTS 13
+
+static const unsigned dm163_input[NUM_DM163_INPUTS] = {
+1 * GPIO_NUM_PINS + 2,  /* ROW0  PB2   */
+0 * GPIO_NUM_PINS + 15, /* ROW1  PA15  */
+0 * GPIO_NUM_PINS + 2,  /* ROW2  PA2   */
+0 * GPIO_NUM_PINS + 7,  /* ROW3  PA7   */
+0 * GPIO_NUM_PINS + 6,  /* ROW4  PA6   */
+0 * GPIO_NUM_PINS + 5,  /* ROW5  PA5   */
+1 * GPIO_NUM_PINS + 0,  /* ROW6  PB0   */
+0 * GPIO_NUM_PINS + 3,  /* ROW7  PA3   */
+0 * GPIO_NUM_PINS + 4,  /* SIN (SDA) PA4   */
+1 * GPIO_NUM_PINS + 1,  /* DCK (SCK) PB1   */
+2 * GPIO_NUM_PINS + 3,  /* RST_B (RST) PC3 */
+2 * GPIO_NUM_PINS + 4,  /* LAT_B (LAT) PC4 */
+2 * GPIO_NUM_PINS + 5,  /* SELBK (SB)  PC5 */
+};
 
 #define TYPE_B_L475E_IOT01A MACHINE_TYPE_NAME("b-l475e-iot01a")
 OBJECT_DECLARE_SIMPLE_TYPE(Bl475eMachineState, B_L475E_IOT01A)
@@ -39,12 +66,16 @@ typedef struct Bl475eMachineState {
 MachineState parent_obj;
 
 Stm32l4x5SocState soc;
+SplitIRQ gpio_splitters[NUM_DM163_INPUTS];
+DM163State dm163;
 } Bl475eMachineState;
 
 static void bl475e_init(MachineState *machine)
 {
 Bl475eMachineState *s = B_L475E_IOT01A(machine);
 const Stm32l4x5SocClass *sc;
+DeviceState *dev, *gpio_out_splitter;
+unsigned gpio, pin;
 
 object_initialize_child(OBJECT(machine), "soc", >soc,
 TYPE_STM32L4X5XG_SOC);
@@ -53,6 +84,30 @@ static void bl475e_init(MachineState *machine)
 sc = STM32L4X5_SOC_GET_CLASS(>soc);
 armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0,
sc->flash_size);
+
+if (object_class_by_name(TYPE_DM163)) {
+object_initialize_child(OBJECT(machine), "dm163",
+>dm163, TYPE_DM163);
+dev = DEVICE(>dm163);
+qdev_realize(dev, NULL, _abort);
+
+for (unsigned i = 0; i < NUM_DM163_INPUTS; i++) {
+object_initialize_child(OBJECT(machine), "gpio-out-splitters[*]",
+>gpio_splitters[i], TYPE_SPLIT_IRQ);
+gpio_out_splitter = DEVICE(>gpio_splitters[i]);
+qdev_prop_set_uint32(gpio_out_splitter, "num-lines", 2);
+qdev_realize(gpio_out_splitter, NULL, _fatal);
+
+qdev_connect_gpio_out(gpio_out_splitter, 0,
+qdev_get_gpio_in(DEVICE(>soc), dm163_input[i]));
+qdev_connect_gpio_out(gpio_out_splitter, 1,
+qdev_get_gpio_in(dev, i));
+gpio = dm163_input[i] / GPIO_NUM_PINS;
+pin = dm163_input[i] % GPIO_NUM_PINS;
+qdev_connect_gpio_out(DEVICE(>soc.gpio[gpio]), pin,
+qdev_get_gpio_in(DEVICE(gpio_out_splitter), 0));
+}
+}
 }
 
 static void bl475e_machine_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 893a7bff66..8431384402 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -468,6 +468,7 @@ config B_L475E_IOT01A
 default y
 depends on TCG && ARM
 select STM32L4X5_SOC
+imply DM163
 
 config STM32L4X5_SOC
 bool
-- 
2.43.2




Re: [PATCH v3 2/2] cxl/core: add poison creation event handler

2024-04-21 Thread kernel test robot
Hi Shiyang,

kernel test robot noticed the following build errors:

[auto build test ERROR on cxl/next]
[also build test ERROR on linus/master v6.9-rc4 next-20240419]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Shiyang-Ruan/cxl-core-correct-length-of-DPA-field-masks/20240417-155443
base:   https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next
patch link:
https://lore.kernel.org/r/20240417075053.3273543-3-ruansy.fnst%40fujitsu.com
patch subject: [PATCH v3 2/2] cxl/core: add poison creation event handler
config: csky-randconfig-002-20240421 
(https://download.01.org/0day-ci/archive/20240421/202404212044.usxtgrtl-...@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240421/202404212044.usxtgrtl-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202404212044.usxtgrtl-...@intel.com/

All errors (new ones prefixed by >>):

   `.exit.text' referenced in section `__jump_table' of fs/ceph/super.o: 
defined in discarded section `.exit.text' of fs/ceph/super.o
   `.exit.text' referenced in section `__jump_table' of fs/ceph/super.o: 
defined in discarded section `.exit.text' of fs/ceph/super.o
   csky-linux-ld: drivers/cxl/core/mbox.o: in function `__cxl_report_poison':
   mbox.c:(.text+0xa12): undefined reference to `cxl_trace_hpa'
>> csky-linux-ld: mbox.c:(.text+0xa44): undefined reference to `cxl_trace_hpa'
   `.exit.text' referenced in section `__jump_table' of 
drivers/target/target_core_configfs.o: defined in discarded section 
`.exit.text' of drivers/target/target_core_configfs.o
   `.exit.text' referenced in section `__jump_table' of 
drivers/target/target_core_configfs.o: defined in discarded section 
`.exit.text' of drivers/target/target_core_configfs.o
   `.exit.text' referenced in section `__jump_table' of net/ceph/ceph_common.o: 
defined in discarded section `.exit.text' of net/ceph/ceph_common.o
   `.exit.text' referenced in section `__jump_table' of net/ceph/ceph_common.o: 
defined in discarded section `.exit.text' of net/ceph/ceph_common.o

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki



Re: Qemu for TC377

2024-04-21 Thread Bastian Koppelmann
Hi Sameer,

On Tue, Apr 16, 2024 at 02:26:10PM -0400, Sameer Kalliadan Poyil wrote:
> Hi Bastian,
>
> Thanks for the information. I thought that I can do some prototyping before 
> the
> HW arrives. :)
>
>  Yes I am interested for your bare metal program boot_to_main run it on TSIM. 
>  
> Is Infineon TSIM free? I searched it and I didn't find any download link. 
> Could
> you please give a link for that if it is from Infineon?

I usually get it from the free entry toolchain [1]

>
> s it(TSIM)  trace32 simulator ? https://repo.lauterbach.com/download_demo.html
> ?
>
> This page https://wiki.qemu.org/Documentation/Platforms/TriCore shows SCU is
> under development.

I should change that on the wiki. I was experimenting with a QEMU model for the
SCU when I was still in University, but nothing usable resulted from that. Now
my time for such developments is unfortunately limited :(.

>
> Could you let me know who is developing it ? is  it possible to take an
> existing SCU and modify according to AURIX data sheet? I see that UART is
> possible to for Tricore like the one developed for ARM versatile platform
>
> Here is the link 
> https://mail.gnu.org/archive/html/qemu-devel/2016-10/msg04514.html

Sure, you can add a model of the Aurix UART in QEMU. It's "just" a matter of
putting in the time to implement its registers and functionality.

>
> I have aurix development trial version and able to compile a UART project 
> using
> Tasking compiler and tried to run it on qemu, but I don't see any logs in the
> qemu terminal as you said there is no peripherals implemented
>
> qemu-system-tricore -machine KIT_AURIX_TC277_TRB -cpu tc27x -m 6M -nographic
> -kernel ASCLIN_Shell_UART_1_KIT_TC277_TFT.elf  -serial stdio -append "console=
> ttyAMA0 console=ttyS0"

I usually add '-d exec,cpu,nochain -D /tmp/exec.log -accel 
tcg,one-insn-per-tb=on'
to get an execution trace to see if the binary is executing.

You can also try attaching gdb by adding '-s -S' to the CLI. And then run in
tricore-gdb 'target remote localhost:1234' see [2]

>
>
> Also do you know if there is a virtual UART framework to communicate between
> two Qemu instances or two TSIM instances running similar OS or different OS? I
> need to do prototype testing RPMSg communication between  MCU and SOC using
> external physical UART/SPI which can be tested using vritual UART using two
> qemu instances. 

No, I don't know of something like this.

Cheers,
Bastian

[1] https://free-entry-toolchain.hightec-rt.com/
[2] https://www.qemu.org/docs/master/system/gdb.html



Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT

2024-04-21 Thread Dorjoy Chowdhury
On Sun, Apr 21, 2024 at 11:40 AM Richard Henderson
 wrote:
>
> On 4/19/24 11:31, Dorjoy Chowdhury wrote:
> > +
> > +/*
> > + * Instantiate a temporary CPU object to build mp_affinity
> > + * of the possible CPUs.
> > + */
> > +cpuobj = object_new(ms->cpu_type);
> > +armcpu = ARM_CPU(cpuobj);
> > +
> >   for (n = 0; n < ms->possible_cpus->len; n++) {
> >   ms->possible_cpus->cpus[n].type = ms->cpu_type;
> >   ms->possible_cpus->cpus[n].arch_id =
> > -sbsa_ref_cpu_mp_affinity(sms, n);
> > +sbsa_ref_cpu_mp_affinity(armcpu, n);
> >   ms->possible_cpus->cpus[n].props.has_thread_id = true;
> >   ms->possible_cpus->cpus[n].props.thread_id = n;
> >   }
> > +
> > +object_unref(cpuobj);
>
> Why is this instantiation necessary?
>

The instantiation is necessary (like the one in hw/arm/virt.c in
virt_possible_cpu_arch_ids) because the
"sbsa_ref_possible_cpu_arch_ids" function is called (via
possible_cpu_arch_ids) before the CPUs are created in the
"sbsa_ref_init" function. There was some related discussion here
(qemu-devel archive URL):
https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg02074.html . I
picked this up from the same thing done in hw/arm/virt.c in the
"machvirt_init" function in the "if (!vms->memap) {" block.

> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1314,8 +1314,18 @@ static void arm_cpu_dump_state(CPUState *cs, FILE 
> > *f, int flags)
> >   }
> >   }
> >
> > -uint64_t arm_build_mp_affinity(int idx, uint8_t clustersz)
> > +uint64_t arm_build_mp_affinity(ARMCPU *cpu, int idx, uint8_t clustersz)
> >   {
> > +if (cpu->has_smt) {
> > +/*
> > + * Right now, the ARM CPUs with SMT supported by QEMU only have
> > + * one thread per core. So Aff0 is always 0.
> > + */
>
> Well, this isn't true.
>
>  -smp 
> [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets]
> 
> [,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]
>
> I would expect all of Aff[0-3] to be settable with the proper topology 
> parameters.
>

Sorry, maybe I misunderstood. From the gitlab bug ticket (URL:
https://gitlab.com/qemu-project/qemu/-/issues/1608) and the discussion
in qemu-devel (URL:
https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg01964.html) I
looked at the technical reference manuals of the MPIDR register of the
a-55, a-76, a-710, neoverse-v1, neoverse-n1, neoverse-n2 CPUs and all
of them had MT=1 and one thread per core so Aff0 is always 0.

I don't know what Aff3 should be set to. I see this comment in the
target/arm/cpu.c "arm_cpu_realizefn" function
   /* This cpu-id-to-MPIDR affinity is used only for TCG; KVM will
override it.│
 * We don't support setting cluster ID ([16..23]) (known as Aff2
 │
 * in later ARM ARM versions), or any of the higher affinity level
fields,  │
 * so these bits always RAZ.
 │
 */
Right now we don't seem to set Aff3[39:32] at all in the mp_affinity
property. Let me know what should be the expected behavior here as I
don't have enough knowledge here. I will try to fix it. Thanks!

Regards,
Dorjoy