Re: [PATCH v3 4/4] wdt_z069: Add support for MEN 16z069 Watchdog

[Alistair Francis]

On Tue, Apr 11, 2023 at 3:50 AM Johannes Thumshirn  wrote:
>
> Add 16z069 Watchdog over MEN Chameleon BUS emulation.
>
> Signed-off-by: Johannes Thumshirn 
> ---
>  hw/watchdog/Kconfig |   5 +
>  hw/watchdog/meson.build |   1 +
>  hw/watchdog/wdt_z069.c  | 218 
>  3 files changed, 224 insertions(+)
>  create mode 100644 hw/watchdog/wdt_z069.c
>
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> index 66e1d029e3..a3f1196f66 100644
> --- a/hw/watchdog/Kconfig
> +++ b/hw/watchdog/Kconfig
> @@ -20,3 +20,8 @@ config WDT_IMX2
>
>  config WDT_SBSA
>  bool
> +
> +config WDT_Z069
> +bool
> +default y if MCB
> +depends on MCB
> diff --git a/hw/watchdog/meson.build b/hw/watchdog/meson.build
> index 8974b5cf4c..7bc353774e 100644
> --- a/hw/watchdog/meson.build
> +++ b/hw/watchdog/meson.build
> @@ -6,4 +6,5 @@ softmmu_ss.add(when: 'CONFIG_WDT_DIAG288', if_true: 
> files('wdt_diag288.c'))
>  softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('wdt_aspeed.c'))
>  softmmu_ss.add(when: 'CONFIG_WDT_IMX2', if_true: files('wdt_imx2.c'))
>  softmmu_ss.add(when: 'CONFIG_WDT_SBSA', if_true: files('sbsa_gwdt.c'))
> +softmmu_ss.add(when: 'CONFIG_WDT_Z069', if_true: files('wdt_z069.c'))
>  specific_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr_watchdog.c'))
> diff --git a/hw/watchdog/wdt_z069.c b/hw/watchdog/wdt_z069.c
> new file mode 100644
> index 00..b20ceab532
> --- /dev/null
> +++ b/hw/watchdog/wdt_z069.c
> @@ -0,0 +1,218 @@
> +/*
> + * QEMU MEN 16z069 Watchdog over MCB emulation
> + *
> + * Copyright (C) 2023 Johannes Thumshirn 
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qemu/timer.h"
> +#include "sysemu/watchdog.h"
> +#include "hw/mcb/mcb.h"
> +#include "migration/vmstate.h"
> +#include "hw/qdev-properties.h"
> +
> +/* #define Z069_DEBUG 1 */
> +
> +#ifdef Z069_DEBUG
> +#define z069_debug(fmt, ...)\
> +fprintf(stderr, "wdt_z069: %s: "fmt, __func__, ##__VA_ARGS__)
> +#else
> +#define z069_debug(fmt, ...)
> +#endif

Same comment from the previous versions about using traces instead of
macro prints

Alistair

> +
> +#define MEN_Z069_WTR 0x10
> +#define MEN_Z069_WTR_WDEN BIT(15)
> +#define MEN_Z069_WTR_WDET_MASK  0x7fff
> +#define MEN_Z069_WVR 0x14
> +
> +#define CLK_500(x) ((x) * 2) /* 500Hz in ms */
> +
> +typedef struct {
> +/*< private >*/
> +MCBDevice dev;
> +
> +/*< public >*/
> +QEMUTimer *timer;
> +
> +bool enabled;
> +unsigned int timeout;
> +
> +MemoryRegion mmio;
> +
> +/* Registers */
> +uint16_t wtr;
> +uint16_t wvr;
> +} MENZ069State;
> +
> +static void men_z069_wdt_enable(MENZ069State *s)
> +{
> +z069_debug("next timeout will fire in +%dms\n", s->timeout);
> +timer_mod(s->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->timeout);
> +}
> +
> +static void men_z069_wdt_disable(MENZ069State *s)
> +{
> +timer_del(s->timer);
> +}
> +
> +static uint64_t men_z069_wdt_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +MENZ069State *s = opaque;
> +uint64_t ret;
> +
> +switch (addr) {
> +case MEN_Z069_WTR:
> +ret = s->wtr;
> +break;
> +case MEN_Z069_WVR:
> +ret = s->wvr;
> +break;
> +default:
> +ret = 0UL;
> +break;
> +}
> +
> +z069_debug("returning: 0x%"PRIx64" @ 0x%lx\n", ret, addr);
> +return ret;
> +}
> +
> +static void men_z069_wdt_write(void *opaque, hwaddr addr, uint64_t v,
> +   unsigned size)
> +{
> +MENZ069State *s = opaque;
> +bool old_ena = s->enabled;
> +uint16_t val = v & 0x;
> +uint16_t tout;
> +
> +z069_debug("got: 0x%"PRIx64" @ 0x%lx\n", v, addr);
> +
> +switch (addr) {
> +case MEN_Z069_WTR:
> +s->wtr = val;
> +tout = val & MEN_Z069_WTR_WDET_MASK;
> +s->timeout = CLK_500(tout);
> +s->enabled = val & MEN_Z069_WTR_WDEN;
> +z069_debug("new timeout: %u (0x%x) %u\n", tout, tout, s->timeout);
> +
> +if (old_ena && !s->enabled) {
> +men_z069_wdt_disable(s);
> +} else if (!old_ena && s->enabled) {
> +men_z069_wdt_enable(s);
> +}
> +
> +break;
> +case MEN_Z069_WVR:
> +/* The watchdog trigger value toggles between 0x and 0x */
> +if (val == (s->wvr ^ 0x)) {
> +s->wvr = val;
> +z069_debug("watchdog triggered, next timeout will fire in 
> +%dms\n",
> +   s->timeout);
> +timer_mod(s->timer,
> +  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + s->timeout);
> +}
> +break;
> +default:
> +break;
> +}
> +return;
> +}
> +
> +static const MemoryRegionOps men_z069_io_ops = {
> +.read = men_z069_wdt_read,
> +.write 

Re: [PATCH v2] target/riscv: Restore the predicate() NULL check behavior

[Alistair Francis]

On Mon, Apr 17, 2023 at 2:32 PM Bin Meng  wrote:
>
> When reading a non-existent CSR QEMU should raise illegal instruction
> exception, but currently it just exits due to the g_assert() check.
>
> This actually reverts commit 0ee342256af9205e7388efdf193a6d8f1ba1a617.
> Some comments are also added to indicate that predicate() must be
> provided for an implemented CSR.
>
> Reported-by: Fei Wu 
> Signed-off-by: Bin Meng 
> Reviewed-by: Daniel Henrique Barboza 
> Reviewed-by: Weiwei Li 
> Reviewed-by: Alistair Francis 
> Reviewed-by: LIU Zhiwei 

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> ---
>
> Changes in v2:
> - rebase on top of Alistair's riscv-to-apply.next tree
>
>  target/riscv/csr.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index f4d2dcfdc8..7000eb3350 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3817,6 +3817,11 @@ static inline RISCVException 
> riscv_csrrw_check(CPURISCVState *env,
>  return RISCV_EXCP_ILLEGAL_INST;
>  }
>
> +/* ensure CSR is implemented by checking predicate */
> +if (!csr_ops[csrno].predicate) {
> +return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
>  /* privileged spec version check */
>  if (env->priv_ver < csr_min_priv) {
>  return RISCV_EXCP_ILLEGAL_INST;
> @@ -3834,7 +3839,6 @@ static inline RISCVException 
> riscv_csrrw_check(CPURISCVState *env,
>   * illegal instruction exception should be triggered instead of virtual
>   * instruction exception. Hence this comes after the read / write check.
>   */
> -g_assert(csr_ops[csrno].predicate != NULL);
>  RISCVException ret = csr_ops[csrno].predicate(env, csrno);
>  if (ret != RISCV_EXCP_NONE) {
>  return ret;
> @@ -4023,7 +4027,10 @@ static RISCVException write_jvt(CPURISCVState *env, 
> int csrno,
>  return RISCV_EXCP_NONE;
>  }
>
> -/* Control and Status Register function table */
> +/*
> + * Control and Status Register function table
> + * riscv_csr_operations::predicate() must be provided for an implemented CSR
> + */
>  riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>  /* User Floating-Point CSRs */
>  [CSR_FFLAGS]   = { "fflags",   fs, read_fflags,  write_fflags },
> --
> 2.25.1
>
>

Re: [PATCH v2] target/riscv: Fix Guest Physical Address Translation

[Alistair Francis]

On Sat, Apr 8, 2023 at 1:34 AM Irina Ryapolova
 wrote:
>
> Before changing the flow check for sv39/48/57.
>
> According to specification (for Supervisor mode):
> Sv39 implementations support a 39-bit virtual address space, divided into 4 
> KiB pages.
> Instruction fetch addresses and load and store effective addresses, which are 
> 64 bits,
> must have bits 63–39 all equal to bit 38, or else a page-fault exception will 
> occur.
> Likewise for Sv48 and Sv57.
>
> So the high bits are equal to bit 38 for sv39.
>
> According to specification (for Hypervisor mode):
> For Sv39x4, address bits of the guest physical address 63:41 must all be 
> zeros, or else a
> guest-page-fault exception occurs.
>
> Likewise for Sv48x4 and Sv57x4.
> For Sv48x4 address bits 63:50 must all be zeros, or else a guest-page-fault 
> exception occurs.
> For Sv57x4 address bits 63:59 must all be zeros, or else a guest-page-fault 
> exception occurs.
>
> For example we are trying to access address 0x__ff01_ with only 
> G-translation enabled.
> So expected behavior is to generate exception. But qemu doesn't generate such 
> exception.
>
> For the old check, we get
> va_bits == 41, mask == (1 << 24) - 1, masked_msbs == (0x__ff01_ 
> >> 40) & mask == mask.
> Accordingly, the condition masked_msbs != 0 && masked_msbs != mask is not 
> fulfilled
> and the check passes.
>
> Signed-off-by: Irina Ryapolova 

Do you mind rebasing this patch on
https://github.com/alistair23/qemu/tree/riscv-to-apply.next and
sending a v3?

Alistair

> ---
> Changes for v2:
>   -Add more detailed commit message
> ---
>  target/riscv/cpu_helper.c | 25 -
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..27289f2305 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -863,17 +863,24 @@ static int get_physical_address(CPURISCVState *env, 
> hwaddr *physical,
>
>  CPUState *cs = env_cpu(env);
>  int va_bits = PGSHIFT + levels * ptidxbits + widened;
> -target_ulong mask, masked_msbs;
>
> -if (TARGET_LONG_BITS > (va_bits - 1)) {
> -mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
> -} else {
> -mask = 0;
> -}
> -masked_msbs = (addr >> (va_bits - 1)) & mask;
> +if (first_stage == true) {
> +target_ulong mask, masked_msbs;
> +
> +if (TARGET_LONG_BITS > (va_bits - 1)) {
> +mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
> +} else {
> +mask = 0;
> +}
> +masked_msbs = (addr >> (va_bits - 1)) & mask;
>
> -if (masked_msbs != 0 && masked_msbs != mask) {
> -return TRANSLATE_FAIL;
> +if (masked_msbs != 0 && masked_msbs != mask) {
> +return TRANSLATE_FAIL;
> +}
> +} else {
> +if (vm != VM_1_10_SV32 && addr >> va_bits != 0) {
> +return TRANSLATE_FAIL;
> +}
>  }
>
>  int ptshift = (levels - 1) * ptidxbits;
> --
> 2.25.1
>
>

[PATCH v2] target/riscv: Restore the predicate() NULL check behavior

[Bin Meng]

When reading a non-existent CSR QEMU should raise illegal instruction
exception, but currently it just exits due to the g_assert() check.

This actually reverts commit 0ee342256af9205e7388efdf193a6d8f1ba1a617.
Some comments are also added to indicate that predicate() must be
provided for an implemented CSR.

Reported-by: Fei Wu 
Signed-off-by: Bin Meng 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Weiwei Li 
Reviewed-by: Alistair Francis 
Reviewed-by: LIU Zhiwei 

---

Changes in v2:
- rebase on top of Alistair's riscv-to-apply.next tree

 target/riscv/csr.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index f4d2dcfdc8..7000eb3350 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3817,6 +3817,11 @@ static inline RISCVException 
riscv_csrrw_check(CPURISCVState *env,
 return RISCV_EXCP_ILLEGAL_INST;
 }
 
+/* ensure CSR is implemented by checking predicate */
+if (!csr_ops[csrno].predicate) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
 /* privileged spec version check */
 if (env->priv_ver < csr_min_priv) {
 return RISCV_EXCP_ILLEGAL_INST;
@@ -3834,7 +3839,6 @@ static inline RISCVException 
riscv_csrrw_check(CPURISCVState *env,
  * illegal instruction exception should be triggered instead of virtual
  * instruction exception. Hence this comes after the read / write check.
  */
-g_assert(csr_ops[csrno].predicate != NULL);
 RISCVException ret = csr_ops[csrno].predicate(env, csrno);
 if (ret != RISCV_EXCP_NONE) {
 return ret;
@@ -4023,7 +4027,10 @@ static RISCVException write_jvt(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
-/* Control and Status Register function table */
+/*
+ * Control and Status Register function table
+ * riscv_csr_operations::predicate() must be provided for an implemented CSR
+ */
 riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 /* User Floating-Point CSRs */
 [CSR_FFLAGS]   = { "fflags",   fs, read_fflags,  write_fflags },
-- 
2.25.1

Re: [PATCH 00/12] virtio: add vhost-user-generic and reduce copy and paste

[Viresh Kumar]

On 14-04-23, 17:04, Alex Bennée wrote:
>  hw/virtio/vhost-user-device-pci.c  |  71 +
>  hw/virtio/vhost-user-device.c  | 359 ++
>  hw/virtio/vhost-user-fs.c  |   4 +-
>  hw/virtio/vhost-user-gpio.c| 405 +
>  hw/virtio/vhost-user-rng.c | 264 +---

I wonder why isn't i2c removed as well ?

-- 
viresh

Re: [PATCH v2] target/riscv: Fix Guest Physical Address Translation

[Alistair Francis]

On Sat, Apr 8, 2023 at 1:34 AM Irina Ryapolova
 wrote:
>
> Before changing the flow check for sv39/48/57.
>
> According to specification (for Supervisor mode):
> Sv39 implementations support a 39-bit virtual address space, divided into 4 
> KiB pages.
> Instruction fetch addresses and load and store effective addresses, which are 
> 64 bits,
> must have bits 63–39 all equal to bit 38, or else a page-fault exception will 
> occur.
> Likewise for Sv48 and Sv57.
>
> So the high bits are equal to bit 38 for sv39.
>
> According to specification (for Hypervisor mode):
> For Sv39x4, address bits of the guest physical address 63:41 must all be 
> zeros, or else a
> guest-page-fault exception occurs.
>
> Likewise for Sv48x4 and Sv57x4.
> For Sv48x4 address bits 63:50 must all be zeros, or else a guest-page-fault 
> exception occurs.
> For Sv57x4 address bits 63:59 must all be zeros, or else a guest-page-fault 
> exception occurs.
>
> For example we are trying to access address 0x__ff01_ with only 
> G-translation enabled.
> So expected behavior is to generate exception. But qemu doesn't generate such 
> exception.
>
> For the old check, we get
> va_bits == 41, mask == (1 << 24) - 1, masked_msbs == (0x__ff01_ 
> >> 40) & mask == mask.
> Accordingly, the condition masked_msbs != 0 && masked_msbs != mask is not 
> fulfilled
> and the check passes.
>
> Signed-off-by: Irina Ryapolova 

Reviewed-by: Alistair Francis 

Alistair

> ---
> Changes for v2:
>   -Add more detailed commit message
> ---
>  target/riscv/cpu_helper.c | 25 -
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..27289f2305 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -863,17 +863,24 @@ static int get_physical_address(CPURISCVState *env, 
> hwaddr *physical,
>
>  CPUState *cs = env_cpu(env);
>  int va_bits = PGSHIFT + levels * ptidxbits + widened;
> -target_ulong mask, masked_msbs;
>
> -if (TARGET_LONG_BITS > (va_bits - 1)) {
> -mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
> -} else {
> -mask = 0;
> -}
> -masked_msbs = (addr >> (va_bits - 1)) & mask;
> +if (first_stage == true) {
> +target_ulong mask, masked_msbs;
> +
> +if (TARGET_LONG_BITS > (va_bits - 1)) {
> +mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
> +} else {
> +mask = 0;
> +}
> +masked_msbs = (addr >> (va_bits - 1)) & mask;
>
> -if (masked_msbs != 0 && masked_msbs != mask) {
> -return TRANSLATE_FAIL;
> +if (masked_msbs != 0 && masked_msbs != mask) {
> +return TRANSLATE_FAIL;
> +}
> +} else {
> +if (vm != VM_1_10_SV32 && addr >> va_bits != 0) {
> +return TRANSLATE_FAIL;
> +}
>  }
>
>  int ptshift = (levels - 1) * ptidxbits;
> --
> 2.25.1
>
>

Re: [PATCH] riscv: Raise an exception if pte reserved bits are not cleared

[Alistair Francis]

On Wed, Apr 12, 2023 at 7:18 PM Alexandre Ghiti  wrote:
>
> As per the specification, in 64-bit, if any of the pte reserved bits 60-54
> is set, an exception should be triggered (see 4.4.1, "Addressing and Memory
> Protection"), so implement this behaviour in the address translation process.
>
> Reported-by: Andrea Parri 
> Signed-off-by: Alexandre Ghiti 
> ---
>  target/riscv/cpu_bits.h   | 1 +
>  target/riscv/cpu_helper.c | 5 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index fca7ef0cef..8d9ba2ce11 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -640,6 +640,7 @@ typedef enum {
>  #define PTE_SOFT0x300 /* Reserved for Software */
>  #define PTE_PBMT0x6000ULL /* Page-based memory types 
> */
>  #define PTE_N   0x8000ULL /* NAPOT translation */
> +#define PTE_RESERVED0x1FC0ULL /* Reserved bits */
>  #define PTE_ATTR(PTE_N | PTE_PBMT) /* All attributes bits */
>
>  /* Page table PPN shift amount */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..39c323a865 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -936,6 +936,11 @@ restart:
>  return TRANSLATE_FAIL;
>  }
>
> +/* PTE reserved bits must be cleared otherwise an exception is 
> raised */
> +if (riscv_cpu_mxl(env) == MXL_RV64 && (pte & PTE_RESERVED)) {
> +return TRANSLATE_FAIL;
> +}

Isn't this caught by our existing check?

if ((pte & ~(target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT) {
return TRANSLATE_FAIL;
}

Alistair

> +
>  bool pbmte = env->menvcfg & MENVCFG_PBMTE;
>  bool hade = env->menvcfg & MENVCFG_HADE;
>
> --
> 2.37.2
>
>

Re: [PATCH v3 2/4] Add MEN Chameleon Bus via PCI carrier

[Alistair Francis]

On Tue, Apr 11, 2023 at 3:51 AM Johannes Thumshirn  wrote:
>
> Add PCI based MEN Chameleon Bus carrier emulation.
>
> Signed-off-by: Johannes Thumshirn 

Acked-by: Alistair Francis 

Alistair

> ---
>  hw/mcb/Kconfig  |   6 +
>  hw/mcb/mcb-pci.c| 297 
>  hw/mcb/meson.build  |   1 +
>  hw/mcb/trace-events |   3 +
>  hw/mcb/trace.h  |   1 +
>  meson.build |   1 +
>  6 files changed, 309 insertions(+)
>  create mode 100644 hw/mcb/mcb-pci.c
>  create mode 100644 hw/mcb/trace-events
>  create mode 100644 hw/mcb/trace.h
>
> diff --git a/hw/mcb/Kconfig b/hw/mcb/Kconfig
> index 36a7a583a8..7deb96c2fe 100644
> --- a/hw/mcb/Kconfig
> +++ b/hw/mcb/Kconfig
> @@ -1,2 +1,8 @@
>  config MCB
>  bool
> +
> +config MCB_PCI
> +bool
> +default y if PCI_DEVICES
> +depends on PCI
> +select MCB
> diff --git a/hw/mcb/mcb-pci.c b/hw/mcb/mcb-pci.c
> new file mode 100644
> index 00..516f133c2e
> --- /dev/null
> +++ b/hw/mcb/mcb-pci.c
> @@ -0,0 +1,297 @@
> +/*
> + * QEMU MEN Chameleon Bus emulation
> + *
> + * Copyright (C) 2023 Johannes Thumshirn 
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/mcb/mcb.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_device.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "trace.h"
> +
> +typedef struct {
> +uint8_t revision;
> +char model;
> +uint8_t minor;
> +uint8_t bus_type;
> +uint16_t magic;
> +uint16_t reserved;
> +/* This one has no '\0' at the end!!! */
> +char filename[12];
> +} ChameleonFPGAHeader;
> +#define CHAMELEON_BUS_TYPE_WISHBONE 0
> +#define CHAMELEONV2_MAGIC 0xabce
> +
> +typedef struct {
> +PCIDevice dev;
> +MCBus bus;
> +MemoryRegion ctbl;
> +uint16_t status;
> +uint8_t int_set;
> +ChameleonFPGAHeader *header;
> +
> +uint8_t minor;
> +uint8_t rev;
> +uint8_t model;
> +} MPCIState;
> +
> +#define TYPE_MCB_PCI "mcb-pci"
> +
> +#define MPCI(obj)   \
> +OBJECT_CHECK(MPCIState, (obj), TYPE_MCB_PCI)
> +
> +#define CHAMELEON_TABLE_SIZE 0x200
> +#define N_MODULES 32
> +
> +#define PCI_VENDOR_ID_MEN 0x1a88
> +#define PCI_DEVICE_ID_MEN_MCBPCI 0x4d45
> +
> +static uint32_t read_header(MPCIState *s, hwaddr addr)
> +{
> +uint32_t ret = 0;
> +ChameleonFPGAHeader *header = s->header;
> +
> +switch (addr >> 2) {
> +case 0:
> +ret |= header->revision;
> +ret |= header->model << 8;
> +ret |= header->minor << 16;
> +ret |= header->bus_type << 24;
> +break;
> +case 1:
> +ret |= header->magic;
> +ret |= header->reserved << 16;
> +break;
> +case 2:
> +memcpy(, header->filename, sizeof(uint32_t));
> +break;
> +case 3:
> +memcpy(, header->filename + sizeof(uint32_t),
> +   sizeof(uint32_t));
> +break;
> +case 4:
> +memcpy(, header->filename + 2 * sizeof(uint32_t),
> +   sizeof(uint32_t));
> +}
> +
> +return ret;
> +}
> +
> +static uint32_t read_gdd(MCBDevice *mdev, int reg)
> +{
> +ChameleonDeviceDescriptor *gdd;
> +uint32_t ret = 0;
> +
> +gdd = mdev->gdd;
> +
> +switch (reg) {
> +case 0:
> +ret = gdd->reg1;
> +break;
> +case 1:
> +ret = gdd->reg2;
> +break;
> +case 2:
> +ret = gdd->offset;
> +break;
> +case 3:
> +ret = gdd->size;
> +break;
> +}
> +
> +return ret;
> +}
> +
> +static uint64_t mpci_chamtbl_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +MPCIState *s = opaque;
> +MCBus *bus = >bus;
> +MCBDevice *mdev;
> +
> +trace_mpci_chamtbl_read(addr, size);
> +
> +if (addr < sizeof(ChameleonFPGAHeader)) {
> +return le32_to_cpu(read_header(s, addr));
> +} else if (addr >= sizeof(ChameleonFPGAHeader) &&
> +   addr < CHAMELEON_TABLE_SIZE) {
> +/* Handle read on chameleon table */
> +BusChild *kid;
> +DeviceState *qdev;
> +int slot;
> +int offset;
> +int i;
> +
> +offset = addr - sizeof(ChameleonFPGAHeader);
> +slot = offset / sizeof(ChameleonDeviceDescriptor);
> +
> +kid = QTAILQ_FIRST((bus)->children);
> +for (i = 0; i < slot; i++) {
> +kid = QTAILQ_NEXT(kid, sibling);
> +if (!kid) { /* Last element */
> +return ~0U;
> +}
> +}
> +qdev = kid->child;
> +mdev = MCB_DEVICE(qdev);
> +offset -= slot * 16;
> +
> +return le32_to_cpu(read_gdd(mdev, offset / 4));
> +}
> +
> +return 0;
> +}
> +
> +static void mpci_chamtbl_write(void *opaque, hwaddr addr, uint64_t val,
> +   unsigned size)
> +{
> +
> +  

Re: [PATCH v3 0/3] target/riscv: implement query-cpu-definitions

[Alistair Francis]

On Wed, Apr 12, 2023 at 4:36 AM Daniel Henrique Barboza
 wrote:
>
> Hi,
>
> In this v3 I removed patches 3 and 4 of v2.
>
> Patch 3 now implements a new type that the generic CPUs (any, rv32,
> rv64, x-rv128) were converted to. This type will be used by
> query-cpu-definitions to determine if a given cpu is static or not based
> on its type. This approach was suggested by Richard Henderson in the v2
> review.
>
> Patches are based on top of Alistair's riscv-to-apply.next.
>
> Changes from v2:
> - old patches 3 and 4: removed
> - patch 3:
>   - add TYPE_RISCV_DYNAMIC_CPU
>   - use this type to set 'q_static' in riscv_cpu_add_definition()
> - v2 link: https://lists.gnu.org/archive/html/qemu-devel/2023-04/msg01310.html
>
> Daniel Henrique Barboza (3):
>   target/riscv: add CPU QOM header
>   target/riscv: add query-cpy-definitions support
>   target/riscv: add TYPE_RISCV_DYNAMIC_CPU

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  qapi/machine-target.json  |  6 ++-
>  target/riscv/cpu-qom.h| 70 +++
>  target/riscv/cpu.c| 20 --
>  target/riscv/cpu.h| 46 +--
>  target/riscv/meson.build  |  3 +-
>  target/riscv/riscv-qmp-cmds.c | 57 
>  6 files changed, 150 insertions(+), 52 deletions(-)
>  create mode 100644 target/riscv/cpu-qom.h
>  create mode 100644 target/riscv/riscv-qmp-cmds.c
>
> --
> 2.39.2
>
>

Re: [PATCH v3 3/3] target/riscv: add TYPE_RISCV_DYNAMIC_CPU

[Alistair Francis]

On Wed, Apr 12, 2023 at 4:36 AM Daniel Henrique Barboza
 wrote:
>
> This new abstract type will be used to differentiate between static and
> non-static CPUs in query-cpu-definitions.
>
> All generic CPUs were changed to be of this type. Named CPUs are kept as
> TYPE_RISCV_CPU and will still be considered static.
>
> This is the output of query-cpu-definitions after this change for the
> riscv64 target:
>
> $ ./build/qemu-system-riscv64 -S -M virt -display none -qmp stdio
> {"QMP": {"version": (...)}
> {"execute": "qmp_capabilities", "arguments": {"enable": ["oob"]}}
> {"return": {}}
> {"execute": "query-cpu-definitions"}
> {"return": [
> {"name": "rv64", "typename": "rv64-riscv-cpu", "static": false, "deprecated": 
> false},
> {"name": "sifive-e51", "typename": "sifive-e51-riscv-cpu", "static": true, 
> "deprecated": false},
> {"name": "any", "typename": "any-riscv-cpu", "static": false, "deprecated": 
> false},
> {"name": "x-rv128", "typename": "x-rv128-riscv-cpu", "static": false, 
> "deprecated": false},
> {"name": "shakti-c", "typename": "shakti-c-riscv-cpu", "static": true, 
> "deprecated": false},
> {"name": "thead-c906", "typename": "thead-c906-riscv-cpu", "static": true, 
> "deprecated": false},
> {"name": "sifive-u54", "typename": "sifive-u54-riscv-cpu", "static": true, 
> "deprecated": false}
> ]}
>
> Suggested-by: Richard Henderson 
> Signed-off-by: Daniel Henrique Barboza 

Acked-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu-qom.h|  2 +-
>  target/riscv/cpu.c| 20 
>  target/riscv/riscv-qmp-cmds.c |  4 
>  3 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
> index b9318e0783..b29090ad86 100644
> --- a/target/riscv/cpu-qom.h
> +++ b/target/riscv/cpu-qom.h
> @@ -23,6 +23,7 @@
>  #include "qom/object.h"
>
>  #define TYPE_RISCV_CPU "riscv-cpu"
> +#define TYPE_RISCV_DYNAMIC_CPU "riscv-dynamic-cpu"
>
>  #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
>  #define RISCV_CPU_TYPE_NAME(name) (name RISCV_CPU_TYPE_SUFFIX)
> @@ -66,5 +67,4 @@ struct RISCVCPUClass {
>  DeviceRealize parent_realize;
>  ResettablePhases parent_phases;
>  };
> -
>  #endif /* RISCV_CPU_QOM_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index fab38859ec..56f2b345cf 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1788,6 +1788,13 @@ void riscv_cpu_list(void)
>  .instance_init = initfn\
>  }
>
> +#define DEFINE_DYNAMIC_CPU(type_name, initfn) \
> +{ \
> +.name = type_name,\
> +.parent = TYPE_RISCV_DYNAMIC_CPU, \
> +.instance_init = initfn   \
> +}
> +
>  static const TypeInfo riscv_cpu_type_infos[] = {
>  {
>  .name = TYPE_RISCV_CPU,
> @@ -1799,23 +1806,28 @@ static const TypeInfo riscv_cpu_type_infos[] = {
>  .class_size = sizeof(RISCVCPUClass),
>  .class_init = riscv_cpu_class_init,
>  },
> -DEFINE_CPU(TYPE_RISCV_CPU_ANY,  riscv_any_cpu_init),
> +{
> +.name = TYPE_RISCV_DYNAMIC_CPU,
> +.parent = TYPE_RISCV_CPU,
> +.abstract = true,
> +},
> +DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY,  riscv_any_cpu_init),
>  #if defined(CONFIG_KVM)
>  DEFINE_CPU(TYPE_RISCV_CPU_HOST, riscv_host_cpu_init),
>  #endif
>  #if defined(TARGET_RISCV32)
> -DEFINE_CPU(TYPE_RISCV_CPU_BASE32,   rv32_base_cpu_init),
> +DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE32,   rv32_base_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_IBEX, rv32_ibex_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,   rv32_sifive_e_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34,   rv32_imafcu_nommu_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,   rv32_sifive_u_cpu_init),
>  #elif defined(TARGET_RISCV64)
> -DEFINE_CPU(TYPE_RISCV_CPU_BASE64,   rv64_base_cpu_init),
> +DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE64,   rv64_base_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,   rv64_sifive_e_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   rv64_sifive_u_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SHAKTI_C, rv64_sifive_u_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906,   rv64_thead_c906_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_BASE128,  rv128_base_cpu_init),
> +DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE128,  rv128_base_cpu_init),
>  #endif
>  };
>
> diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
> index 128677add9..5ecff1afb3 100644
> --- a/target/riscv/riscv-qmp-cmds.c
> +++ b/target/riscv/riscv-qmp-cmds.c
> @@ -33,11 +33,15 @@ static void riscv_cpu_add_definition(gpointer data, 
> gpointer user_data)
>  CpuDefinitionInfoList **cpu_list = user_data;
>  CpuDefinitionInfo *info = g_malloc0(sizeof(*info));
>  const char *typename = 

Re: [PATCH v3 2/3] target/riscv: add query-cpy-definitions support

[Alistair Francis]

On Wed, Apr 12, 2023 at 4:37 AM Daniel Henrique Barboza
 wrote:
>
> This command is used by tooling like libvirt to retrieve a list of
> supported CPUs. Each entry returns a CpuDefinitionInfo object that
> contains more information about each CPU.
>
> This initial support includes only the name of the CPU and its typename.
> Here's what the command produces for the riscv64 target:
>
> $ ./build/qemu-system-riscv64 -S -M virt -display none -qmp stdio
> {"QMP": {"version": (...)}
> {"execute": "qmp_capabilities", "arguments": {"enable": ["oob"]}}
> {"return": {}}
> {"execute": "query-cpu-definitions"}
> {"return": [
> {"name": "rv64", "typename": "rv64-riscv-cpu", "static": false, "deprecated": 
> false},
> {"name": "sifive-e51", "typename": "sifive-e51-riscv-cpu", "static": false, 
> "deprecated": false},
> {"name": "any", "typename": "any-riscv-cpu", "static": false, "deprecated": 
> false},
> {"name": "x-rv128", "typename": "x-rv128-riscv-cpu", "static": false, 
> "deprecated": false},
> {"name": "shakti-c", "typename": "shakti-c-riscv-cpu", "static": false, 
> "deprecated": false},
> {"name": "thead-c906", "typename": "thead-c906-riscv-cpu", "static": false, 
> "deprecated": false},
> {"name": "sifive-u54", "typename": "sifive-u54-riscv-cpu", "static": false, 
> "deprecated": false}]
> }
>
> Next patch will introduce a way to tell whether a given CPU is static or
> not.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Richard Henderson 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  qapi/machine-target.json  |  6 ++--
>  target/riscv/meson.build  |  3 +-
>  target/riscv/riscv-qmp-cmds.c | 53 +++
>  3 files changed, 59 insertions(+), 3 deletions(-)
>  create mode 100644 target/riscv/riscv-qmp-cmds.c
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 2e267fa458..f3a3de6648 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -324,7 +324,8 @@
> 'TARGET_I386',
> 'TARGET_S390X',
> 'TARGET_MIPS',
> -   'TARGET_LOONGARCH64' ] } }
> +   'TARGET_LOONGARCH64',
> +   'TARGET_RISCV' ] } }
>
>  ##
>  # @query-cpu-definitions:
> @@ -341,4 +342,5 @@
> 'TARGET_I386',
> 'TARGET_S390X',
> 'TARGET_MIPS',
> -   'TARGET_LOONGARCH64' ] } }
> +   'TARGET_LOONGARCH64',
> +   'TARGET_RISCV' ] } }
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index 5b7f813a3e..e1ff6d9b95 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -32,7 +32,8 @@ riscv_softmmu_ss.add(files(
>'monitor.c',
>'machine.c',
>'pmu.c',
> -  'time_helper.c'
> +  'time_helper.c',
> +  'riscv-qmp-cmds.c',
>  ))
>
>  target_arch += {'riscv': riscv_ss}
> diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
> new file mode 100644
> index 00..128677add9
> --- /dev/null
> +++ b/target/riscv/riscv-qmp-cmds.c
> @@ -0,0 +1,53 @@
> +/*
> + * QEMU CPU QMP commands for RISC-V
> + *
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/qapi-commands-machine-target.h"
> +#include "cpu-qom.h"
> +
> +static void riscv_cpu_add_definition(gpointer data, gpointer user_data)
> +{
> +ObjectClass *oc = data;
> +CpuDefinitionInfoList **cpu_list = user_data;
> +CpuDefinitionInfo *info = g_malloc0(sizeof(*info));
> +const char *typename = object_class_get_name(oc);
> +
> +info->name = g_strndup(typename,
> +   strlen(typename) - strlen("-" TYPE_RISCV_CPU));
> +info->q_typename = g_strdup(typename);
> +
> +QAPI_LIST_PREPEND(*cpu_list, info);
> +}
> +
> 

Re: [PATCH v3 1/3] target/riscv: add CPU QOM header

[Alistair Francis]

On Wed, Apr 12, 2023 at 4:36 AM Daniel Henrique Barboza
 wrote:
>
> QMP CPU commands are usually implemented by a separated file,
> -qmp-cmds.c, to allow them to be build only for softmmu targets.
> This file uses a CPU QOM header with basic QOM declarations for the
> arch.
>
> We'll introduce query-cpu-definitions for RISC-V CPUs in the next patch,
> but first we need a cpu-qom.h header with the definitions of
> TYPE_RISCV_CPU and RISCVCPUClass declarations. These were moved from
> cpu.h to the new file, and cpu.h now includes "cpu-qom.h".
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Richard Henderson 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu-qom.h | 70 ++
>  target/riscv/cpu.h | 46 +--
>  2 files changed, 71 insertions(+), 45 deletions(-)
>  create mode 100644 target/riscv/cpu-qom.h
>
> diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
> new file mode 100644
> index 00..b9318e0783
> --- /dev/null
> +++ b/target/riscv/cpu-qom.h
> @@ -0,0 +1,70 @@
> +/*
> + * QEMU RISC-V CPU QOM header
> + *
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + *
> + * 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 .
> + */
> +
> +#ifndef RISCV_CPU_QOM_H
> +#define RISCV_CPU_QOM_H
> +
> +#include "hw/core/cpu.h"
> +#include "qom/object.h"
> +
> +#define TYPE_RISCV_CPU "riscv-cpu"
> +
> +#define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
> +#define RISCV_CPU_TYPE_NAME(name) (name RISCV_CPU_TYPE_SUFFIX)
> +#define CPU_RESOLVING_TYPE TYPE_RISCV_CPU
> +
> +#define TYPE_RISCV_CPU_ANY  RISCV_CPU_TYPE_NAME("any")
> +#define TYPE_RISCV_CPU_BASE32   RISCV_CPU_TYPE_NAME("rv32")
> +#define TYPE_RISCV_CPU_BASE64   RISCV_CPU_TYPE_NAME("rv64")
> +#define TYPE_RISCV_CPU_BASE128  RISCV_CPU_TYPE_NAME("x-rv128")
> +#define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
> +#define TYPE_RISCV_CPU_SHAKTI_C RISCV_CPU_TYPE_NAME("shakti-c")
> +#define TYPE_RISCV_CPU_SIFIVE_E31   RISCV_CPU_TYPE_NAME("sifive-e31")
> +#define TYPE_RISCV_CPU_SIFIVE_E34   RISCV_CPU_TYPE_NAME("sifive-e34")
> +#define TYPE_RISCV_CPU_SIFIVE_E51   RISCV_CPU_TYPE_NAME("sifive-e51")
> +#define TYPE_RISCV_CPU_SIFIVE_U34   RISCV_CPU_TYPE_NAME("sifive-u34")
> +#define TYPE_RISCV_CPU_SIFIVE_U54   RISCV_CPU_TYPE_NAME("sifive-u54")
> +#define TYPE_RISCV_CPU_THEAD_C906   RISCV_CPU_TYPE_NAME("thead-c906")
> +#define TYPE_RISCV_CPU_HOST RISCV_CPU_TYPE_NAME("host")
> +
> +#if defined(TARGET_RISCV32)
> +# define TYPE_RISCV_CPU_BASETYPE_RISCV_CPU_BASE32
> +#elif defined(TARGET_RISCV64)
> +# define TYPE_RISCV_CPU_BASETYPE_RISCV_CPU_BASE64
> +#endif
> +
> +typedef struct CPUArchState CPURISCVState;
> +
> +OBJECT_DECLARE_CPU_TYPE(RISCVCPU, RISCVCPUClass, RISCV_CPU)
> +
> +/**
> + * RISCVCPUClass:
> + * @parent_realize: The parent class' realize handler.
> + * @parent_phases: The parent class' reset phase handlers.
> + *
> + * A RISCV CPU model.
> + */
> +struct RISCVCPUClass {
> +/*< private >*/
> +CPUClass parent_class;
> +/*< public >*/
> +DeviceRealize parent_realize;
> +ResettablePhases parent_phases;
> +};
> +
> +#endif /* RISCV_CPU_QOM_H */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 86e08d10da..fa2655306d 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -28,6 +28,7 @@
>  #include "qemu/int128.h"
>  #include "cpu_bits.h"
>  #include "qapi/qapi-types-common.h"
> +#include "cpu-qom.h"
>
>  #define TCG_GUEST_DEFAULT_MO 0
>
> @@ -37,32 +38,6 @@
>   */
>  #define TARGET_INSN_START_EXTRA_WORDS 1
>
> -#define TYPE_RISCV_CPU "riscv-cpu"
> -
> -#define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU
> -#define RISCV_CPU_TYPE_NAME(name) (name RISCV_CPU_TYPE_SUFFIX)
> -#define CPU_RESOLVING_TYPE TYPE_RISCV_CPU
> -
> -#define TYPE_RISCV_CPU_ANY  RISCV_CPU_TYPE_NAME("any")
> -#define TYPE_RISCV_CPU_BASE32   RISCV_CPU_TYPE_NAME("rv32")
> -#define TYPE_RISCV_CPU_BASE64   RISCV_CPU_TYPE_NAME("rv64")
> -#define TYPE_RISCV_CPU_BASE128  RISCV_CPU_TYPE_NAME("x-rv128")
> -#define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
> -#define TYPE_RISCV_CPU_SHAKTI_C RISCV_CPU_TYPE_NAME("shakti-c")
> -#define TYPE_RISCV_CPU_SIFIVE_E31   RISCV_CPU_TYPE_NAME("sifive-e31")
> 

Re: [PATCH v2] target/riscv: Update check for Zca/Zcf/Zcd

[Alistair Francis]

On Fri, Apr 14, 2023 at 12:13 AM Weiwei Li  wrote:
>
> Even though Zca/Zcf/Zcd can be included by C/F/D, however, their priv
> version is higher than the priv version of C/F/D. So if we use check
> for them instead of check for C/F/D totally, it will trigger new
> problem when we try to disable the extensions based on the configured
> priv version.
>
> Signed-off-by: Weiwei Li 
> Signed-off-by: Junqiang Wang 

Reviewed-by: Alistair Francis 

Alistair

> ---
> v2:
> * Fix code style errors
>
>  target/riscv/insn_trans/trans_rvd.c.inc | 12 +++-
>  target/riscv/insn_trans/trans_rvf.c.inc | 14 --
>  target/riscv/insn_trans/trans_rvi.c.inc |  5 +++--
>  target/riscv/translate.c|  5 +++--
>  4 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc 
> b/target/riscv/insn_trans/trans_rvd.c.inc
> index 2c51e01c40..6bdb55ef43 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -31,9 +31,11 @@
>  } \
>  } while (0)
>
> -#define REQUIRE_ZCD(ctx) do { \
> -if (!ctx->cfg_ptr->ext_zcd) {  \
> -return false; \
> +#define REQUIRE_ZCD_OR_DC(ctx) do { \
> +if (!ctx->cfg_ptr->ext_zcd) { \
> +if (!has_ext(ctx, RVD) || !has_ext(ctx, RVC)) { \
> +return false; \
> +} \
>  } \
>  } while (0)
>
> @@ -67,13 +69,13 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
>
>  static bool trans_c_fld(DisasContext *ctx, arg_fld *a)
>  {
> -REQUIRE_ZCD(ctx);
> +REQUIRE_ZCD_OR_DC(ctx);
>  return trans_fld(ctx, a);
>  }
>
>  static bool trans_c_fsd(DisasContext *ctx, arg_fsd *a)
>  {
> -REQUIRE_ZCD(ctx);
> +REQUIRE_ZCD_OR_DC(ctx);
>  return trans_fsd(ctx, a);
>  }
>
> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc 
> b/target/riscv/insn_trans/trans_rvf.c.inc
> index 9e9fa2087a..593855e73a 100644
> --- a/target/riscv/insn_trans/trans_rvf.c.inc
> +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> @@ -30,10 +30,12 @@
>  } \
>  } while (0)
>
> -#define REQUIRE_ZCF(ctx) do {  \
> -if (!ctx->cfg_ptr->ext_zcf) {  \
> -return false;  \
> -}  \
> +#define REQUIRE_ZCF_OR_FC(ctx) do { \
> +if (!ctx->cfg_ptr->ext_zcf) {   \
> +if (!has_ext(ctx, RVF) || !has_ext(ctx, RVC)) { \
> +return false;   \
> +}   \
> +}   \
>  } while (0)
>
>  static bool trans_flw(DisasContext *ctx, arg_flw *a)
> @@ -69,13 +71,13 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
>
>  static bool trans_c_flw(DisasContext *ctx, arg_flw *a)
>  {
> -REQUIRE_ZCF(ctx);
> +REQUIRE_ZCF_OR_FC(ctx);
>  return trans_flw(ctx, a);
>  }
>
>  static bool trans_c_fsw(DisasContext *ctx, arg_fsw *a)
>  {
> -REQUIRE_ZCF(ctx);
> +REQUIRE_ZCF_OR_FC(ctx);
>  return trans_fsw(ctx, a);
>  }
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index c70c495fc5..e33f63bea1 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
>  tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>
>  gen_set_pc(ctx, cpu_pc);
> -if (!ctx->cfg_ptr->ext_zca) {
> +if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
>  TCGv t0 = tcg_temp_new();
>
>  misaligned = gen_new_label();
> @@ -169,7 +169,8 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, 
> TCGCond cond)
>
>  gen_set_label(l); /* branch taken */
>
> -if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
> +if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca &&
> +((ctx->base.pc_next + a->imm) & 0x3)) {
>  /* misaligned */
>  gen_exception_inst_addr_mis(ctx);
>  } else {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index d0094922b6..661e29ab39 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -551,7 +551,7 @@ static void gen_jal(DisasContext *ctx, int rd, 
> target_ulong imm)
>
>  /* check misaligned: */
>  next_pc = ctx->base.pc_next + imm;
> -if (!ctx->cfg_ptr->ext_zca) {
> +if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
>  if ((next_pc & 0x3) != 0) {
>  gen_exception_inst_addr_mis(ctx);
>  return;
> @@ -1137,7 +1137,8 @@ static void decode_opc(CPURISCVState *env, DisasContext 
> *ctx, uint16_t opcode)
>   * The Zca extension is added as way to refer to instructions in the 
> C
>   * extension that do not include the floating-point loads and stores
>   */
> -if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) 

Re: [PATCH v12 02/10] target/riscv: add support for Zca extension

[Alistair Francis]

On Thu, Apr 13, 2023 at 3:24 AM Daniel Henrique Barboza
 wrote:
>
>
>
> On 4/12/23 08:35, Weiwei Li wrote:
> >
> > On 2023/4/12 18:55, Alistair Francis wrote:
> >> On Wed, Apr 12, 2023 at 12:55 PM Weiwei Li  wrote:
> >>>
> >>> On 2023/4/12 10:12, Alistair Francis wrote:
>  On Fri, Apr 7, 2023 at 6:23 AM Daniel Henrique Barboza
>   wrote:
> > Hi,
> >
> > This patch is going to break the sifive_u boot if I rebase
> >
> > "[PATCH v6 0/9] target/riscv: rework CPU extensions validation"
> >
> > on top of it, as it is the case today with the current 
> > riscv-to-apply.next.
> >
> > The reason is that the priv spec version for Zca is marked as 1_12_0, 
> > and
> > the priv spec version for both sifive CPUs is 1_10_0, and both are 
> > enabling
> > RVC.
> >
> > This patch from that series above:
> >
> > "[PATCH v6 5/9] target/riscv/cpu.c: add priv_spec validate/disable_exts 
> > helpers"
> >
> > Makes the disabling of the extension based on priv version to happen 
> > *after* we
> > do all the validations, instead of before as we're doing today. Zca 
> > (and Zcd) will
> > be manually enabled just to be disabled shortly after by the priv spec 
> > code. And
> > this will happen:
> >
> > qemu-system-riscv64: warning: disabling zca extension for hart 
> > 0x because privilege spec version does not match
> > qemu-system-riscv64: warning: disabling zca extension for hart 
> > 0x0001 because privilege spec version does not match
> > qemu-system-riscv64: warning: disabling zcd extension for hart 
> > 0x0001 because privilege spec version does not match
> > (--- hangs ---)
> >
> > This means that the assumption made in this patch - that Zca implies 
> > RVC - is no
> > longer valid, and all these translations won't work.
>  Thanks for catching and reporting this
> 
> > Some possible solutions:
> >
> > - Do not use Zca as a synonym for RVC, i.e. drop this patch. We would 
> > need to convert
> > all Zca checks to RVC checks in all translation code.
>  This to me seems like the best solution
> >>> I had also implemented a patch for this solution. I'll sent it later.
> >> Thanks!
> >>
> >>> However, this will introduce additional check. i.e. check both Zca and C
> >>> or , both Zcf/d and CF/CD.
> >> Is there a large performance penalty for that?
> >
> > May not very large.  Just one or two additional check in instruction 
> > translation. You can see the patch at:
> >
> > https://lore.kernel.org/qemu-devel/20230412030648.80470-1-liwei...@iscas.ac.cn/

This is my prefered way. I think it's pretty simple and explicitly
follows the spec.

> >
> >>
> >>> I think this is not very necessary. Implcitly-enabled extensions is
> >>> often the subsets of existed extension.
> >> Yeah, I see what you are saying. It just becomes difficult to manage
> >> though. It all worked fine until there are conflicts between the priv
> >> specs.
> >>
> >>> So enabling them will introduce no additional function to the cpus.
> >>>
> >>> We can just make them invisible to user(mask them in the isa string)
> >>> instead of disabling them  to be compatible with lower priv version.
> >> I'm open to other options, but masking them like this seems like more
> >> work and also seems confusing. The extension will end up enabled in
> >> QEMU and potentially visible to external tools, but then just not
> >> exposed to the guest. It seems prone to future bugs.
> >
> > Yeah. they may be visible to external tools if they read cfg directly.
> >
> > Another way is to introduce another parameter instead of cfg.ext_zca to 
> > indicate whether Zca/Zcf/Zcd
> >
> > instructions are enabled. This is be done by patchset:
> >
> > https://lore.kernel.org/qemu-devel/20230410033526.31708-1-liwei...@iscas.ac.cn/

I don't prefer this option, but if others feel strongly I'm not
completely opposed to it.

> >
> > All of the three ways are acceptable to me.
>
> Earlier today I grabbed two Weiwei Li patches (isa_string changes and 
> Zca/Zcf/Zcd
> changes) in the "rework CPU extensions validation" series, but after reading 
> these
> last messages I guess I jumped the gun.

My preference would be the "target/riscv: Update check for
Zca/zcf/zcd" patch, which I think is what you picked. So that seems
like the way to go

>
> Alistair, I'm considering drop the patch that disables extensions via 
> priv_spec later
> during realize() (the one I mentioned in my first message) from that series 
> until we
> figure the best way of dealing with priv spec and Z-extensions being used as 
> MISA bits
> and so on. We can merge those cleanups and write_misa() changes that are 
> already reviewed
> in the meantime. Are you ok with that?

That's also fine

Alistair

>
>
> Thanks,
>
>
> Daniel
>
>
> >
> > Regards,
> >
> > Weiwei Li
> >
> >
> >> Alistair
> >>
> >>> 

Re: [PATCH] hw/intc/riscv_aplic: Zero init APLIC internal state

[Alistair Francis]

On Thu, Apr 13, 2023 at 11:35 PM Ivan Klokov  wrote:
>
> Since g_new is used to initialize the RISCVAPLICState->state structure,
> in some case we get behavior that is not as expected. This patch
> changes this to g_new0, which allows to initialize the APLIC in the correct 
> state.
>
> Signed-off-by: Ivan Klokov 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/intc/riscv_aplic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index cfd007e629..71591d44bf 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -803,7 +803,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error 
> **errp)
>
>  aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
>  aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
> -aplic->state = g_new(uint32_t, aplic->num_irqs);
> +aplic->state = g_new0(uint32_t, aplic->num_irqs);
>  aplic->target = g_new0(uint32_t, aplic->num_irqs);
>  if (!aplic->msimode) {
>  for (i = 0; i < aplic->num_irqs; i++) {
> --
> 2.34.1
>
>

Re: [PATCH] hw/intc/riscv_aplic: Zero init APLIC internal state

[Alistair Francis]

On Thu, Apr 13, 2023 at 11:35 PM Ivan Klokov  wrote:
>
> Since g_new is used to initialize the RISCVAPLICState->state structure,
> in some case we get behavior that is not as expected. This patch
> changes this to g_new0, which allows to initialize the APLIC in the correct 
> state.
>
> Signed-off-by: Ivan Klokov 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/intc/riscv_aplic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index cfd007e629..71591d44bf 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -803,7 +803,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error 
> **errp)
>
>  aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
>  aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
> -aplic->state = g_new(uint32_t, aplic->num_irqs);
> +aplic->state = g_new0(uint32_t, aplic->num_irqs);
>  aplic->target = g_new0(uint32_t, aplic->num_irqs);
>  if (!aplic->msimode) {
>  for (i = 0; i < aplic->num_irqs; i++) {
> --
> 2.34.1
>
>

Re: [PATCH v7 00/25] target/riscv: MSTATUS_SUM + cleanups

[Alistair Francis]

On Wed, Apr 12, 2023 at 9:43 PM Richard Henderson
 wrote:
>
> v6: 20230325105429.1142530-1-richard.hender...@linaro.org
>
> Changes for v7:
>   * Rebase on Alistair's riscv-to-apply.next.
>   * Replace priv_level() with ctx->priv in trans_xthead.c.inc (Zhiwei).
>
>
> r~
>
>
> Fei Wu (2):
>   target/riscv: Separate priv from mmu_idx
>   target/riscv: Reduce overhead of MSTATUS_SUM change
>
> LIU Zhiwei (4):
>   target/riscv: Extract virt enabled state from tb flags
>   target/riscv: Add a general status enum for extensions
>   target/riscv: Encode the FS and VS on a normal way for tb flags
>   target/riscv: Add a tb flags field for vstart
>
> Richard Henderson (19):
>   target/riscv: Remove mstatus_hs_{fs, vs} from tb_flags
>   accel/tcg: Add cpu_ld*_code_mmu
>   target/riscv: Use cpu_ld*_code_mmu for HLVX
>   target/riscv: Handle HLV, HSV via helpers
>   target/riscv: Rename MMU_HYP_ACCESS_BIT to MMU_2STAGE_BIT
>   target/riscv: Introduce mmuidx_sum
>   target/riscv: Introduce mmuidx_priv
>   target/riscv: Introduce mmuidx_2stage
>   target/riscv: Move hstatus.spvp check to check_access_hlsv
>   target/riscv: Set MMU_2STAGE_BIT in riscv_cpu_mmu_index
>   target/riscv: Check SUM in the correct register
>   target/riscv: Hoist second stage mode change to callers
>   target/riscv: Hoist pbmte and hade out of the level loop
>   target/riscv: Move leaf pte processing out of level loop
>   target/riscv: Suppress pte update with is_debug
>   target/riscv: Don't modify SUM with is_debug
>   target/riscv: Merge checks for reserved pte flags
>   target/riscv: Reorg access check in get_physical_address
>   target/riscv: Reorg sum check in get_physical_address

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  include/exec/cpu_ldst.h   |   9 +
>  target/riscv/cpu.h|  47 +-
>  target/riscv/cpu_bits.h   |  12 +-
>  target/riscv/helper.h |  12 +-
>  target/riscv/internals.h  |  35 ++
>  accel/tcg/cputlb.c|  48 +++
>  accel/tcg/user-exec.c |  58 +++
>  target/riscv/cpu.c|   2 +-
>  target/riscv/cpu_helper.c | 403 +-
>  target/riscv/csr.c|  21 +-
>  target/riscv/op_helper.c  | 113 -
>  target/riscv/translate.c  |  70 +--
>  .../riscv/insn_trans/trans_privileged.c.inc   |   2 +-
>  target/riscv/insn_trans/trans_rvf.c.inc   |   2 +-
>  target/riscv/insn_trans/trans_rvh.c.inc   | 137 +++---
>  target/riscv/insn_trans/trans_rvv.c.inc   |  22 +-
>  target/riscv/insn_trans/trans_xthead.c.inc|  14 +-
>  17 files changed, 591 insertions(+), 416 deletions(-)
>
> --
> 2.34.1
>

[RFC PATCH 2/8] hw/riscv: define capabilities of CBQRI controllers

[Drew Fustini]

From: Nicolas Pitre 

Define structs to represent the hardware capabilities of capacity and
bandwidth controllers according to the RISC-V Capacity and Bandwidth QoS
Register Interface (CBQRI).

Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf
Signed-off-by: Nicolas Pitre 
Signed-off-by: Drew Fustini 
---
 include/hw/riscv/cbqri.h | 81 
 1 file changed, 81 insertions(+)
 create mode 100644 include/hw/riscv/cbqri.h

diff --git a/include/hw/riscv/cbqri.h b/include/hw/riscv/cbqri.h
new file mode 100644
index ..b43a3572dd7e
--- /dev/null
+++ b/include/hw/riscv/cbqri.h
@@ -0,0 +1,81 @@
+/*
+ * RISC-V Capacity and Bandwidth QoS Register Interface
+ * URL: https://github.com/riscv-non-isa/riscv-cbqri
+ *
+ * Copyright (c) 2023 BayLibre SAS
+ *
+ * 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 .
+ */
+
+#ifndef HW_RISCV_CBQRI_H
+#define HW_RISCV_CBQRI_H
+
+#include "qemu/typedefs.h"
+
+#define RISCV_CBQRI_VERSION_MAJOR   0
+#define RISCV_CBQRI_VERSION_MINOR   1
+
+/* Capacity Controller hardware capabilities */
+typedef struct RiscvCbqriCapacityCaps {
+uint16_t nb_mcids;
+uint16_t nb_rcids;
+
+uint16_t ncblks;
+
+bool supports_at_data:1;
+bool supports_at_code:1;
+
+bool supports_alloc_op_config_limit:1;
+bool supports_alloc_op_read_limit:1;
+bool supports_alloc_op_flush_rcid:1;
+
+bool supports_mon_op_config_event:1;
+bool supports_mon_op_read_counter:1;
+
+bool supports_mon_evt_id_none:1;
+bool supports_mon_evt_id_occupancy:1;
+} RiscvCbqriCapacityCaps;
+
+/* Bandwidth Controller hardware capabilities */
+typedef struct RiscvCbqriBandwidthCaps {
+uint16_t nb_mcids;
+uint16_t nb_rcids;
+
+uint16_t nbwblks;
+uint16_t mrbwb;
+
+bool supports_at_data:1;
+bool supports_at_code:1;
+
+bool supports_alloc_op_config_limit:1;
+bool supports_alloc_op_read_limit:1;
+
+bool supports_mon_op_config_event:1;
+bool supports_mon_op_read_counter:1;
+
+bool supports_mon_evt_id_none:1;
+bool supports_mon_evt_id_rdwr_count:1;
+bool supports_mon_evt_id_rdonly_count:1;
+bool supports_mon_evt_id_wronly_count:1;
+} RiscvCbqriBandwidthCaps;
+
+DeviceState *riscv_cbqri_cc_create(hwaddr addr,
+   const RiscvCbqriCapacityCaps *caps,
+   const char *instance_name);
+DeviceState *riscv_cbqri_bc_create(hwaddr addr,
+   const RiscvCbqriBandwidthCaps *caps,
+   const char *instance_name);
+
+void example_soc_cbqri_init(void);
+
+#endif
-- 
2.34.1

[RFC PATCH 0/8] riscv: implement Ssqosid extension and CBQRI controllers

[Drew Fustini]

This RFC series implements the Ssqosid extension and the sqoscfg CSR as
defined in the RISC-V Capacity and Bandwidth Controller QoS Register
Interface (CBQRI) specification [1]. Quality of Service (QoS) in this
context is concerned with shared resources on an SoC such as cache
capacity and memory bandwidth.

These patches are available as a public git branch [2].

sqoscfg CSR
---
The sqoscfg CSR provides a mechanism by which a software workload (e.g.
a process or a set of processes) can be associated with a resource
control ID (RCID) and a monitoring counter ID (MCID) that accompanies
each request made by the hart to shared resources like cache.

CBQRI defines operations to configure resource usage limits, in the form
of capacity or bandwidth, for an RCID. CBQRI also defines operations to
configure counters to track the resource utilization per MCID.

CBQRI controllers
-
This series also implements an CBQRI capacity controller and an CBQRI
bandwidth controller as well as an example SoC that instantiates the
controllers with a specific configuration:

  - L2 cache controllers
- Resource type: Capacity
- Number of capacity blocks (NCBLKS): 12
- In the context of a set-associative cache, the number of
  capacity blocks can be thought of as the number of ways
- Number of access types: 2 (code and data)
- Usage monitoring not supported
- Capacity allocation operations: CONFIG_LIMIT, READ_LIMIT

  - Last-level cache (LLC) controller
- Resource type: Capacity
- Number of capacity blocks (NCBLKS): 16
- Number of access types: 2 (code and data)
- Usage monitoring operations: CONFIG_EVENT, READ_COUNTER
- Event IDs supported: None, Occupancy
- Capacity allocation ops: CONFIG_LIMIT, READ_LIMIT, FLUSH_RCID

  - Memory controllers
- Resource type: Bandwidth
- Number of bandwidth blocks (NBWBLKS): 1024
   - Bandwidth blocks do not have a unit but instead represent a
 portion of the total bandwidth resource. For NWBLKS of 1024,
 each block represents about 0.1% of the bandwidth resource.
- Maximum reserved bandwidth blocks (MRBWB): 819 [80% of NBWBLKS]
- Number of access types: 1 (no code/data differentiation)
- Usage monitoring operations: CONFIG_EVENT, READ_COUNTER
- Event IDs supported: None, Total read/write byte count, Total
   read byte count, Total write byte count
- Bandwidth allocation operations: CONFIG_LIMIT, READ_LIMIT

The memory map for this example SoC:

  Base addr  Size
  0x482  4KB  Cluster 0 L2 cache controller
  0x4821000  4KB  Cluster 1 L2 cache controller
  0x4828000  4KB  Memory controller 0
  0x4829000  4KB  Memory controller 1
  0x482A000  4KB  Memory controller 2
  0x482B000  4KB  Shared LLC cache controller

This configuration is meant to provide a "concrete" example for software
(like Linux) to test against. It represents just one of many possible
ways for hardware to implement the CBQRI spec.

In addition, please note that this RFC series only implements the
register interface that CBQRI specifies. It does not attempt to emulate
the performance impact of configuring limits on shared resources like
cache and memory bandwidth. Similarly, the code does not attempt to
emulate cache and memory bandwidth utilization, like what would be
observed on a real hardware system implementing CBQRI.

Status of CBQRI
---
The CBQRI spec is still in a draft state and is undergoing review [3].
It is possible there will be changes to the Ssqosid extension and the
CBQRI spec. For example, the sqoscfg CSR address is not yet finalized.

The goal of this Qemu patch series, along with complimentary Linux patch
series, is to satisfy the software proof of concept requirement for
CBQRI to be frozen.

Note: CBQRI was previously known as CMQRI and the github repo with the
spec has not yet been renamed [4].

Future work
---
Currently the configuration of the CBQRI controller parameters is done
directly in cbqri_example_soc.c but this requires a user to edit the
code for a different configuration. In the future, the configuration of
CBQRI controller parameters will be done as command line arguments.

In addition, the device tree generation will be expanded to include
CBQRI-related properties. Currently, these are added to the dumped dtb
that Linux consumes for the purposes of testing this RFC.

For those interested in the software using this Qemu implementation,
I will follow-up with a link to the Linux patches once posted.

[1] https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf
[2] https://gitlab.baylibre.com/baylibre/qemu/-/tree/riscv-cbqri-rfc
[3] https://lists.riscv.org/g/tech-cmqri/message/38
[4] https://lists.riscv.org/g/tech-cmqri/message/41

Kornel Dulęba (1):
  riscv: implement Ssqosid extension and sqoscfg CSR

Nicolas Pitre (7):
  hw/riscv: define capabilities of CBQRI controllers
  hw/riscv: implement CBQRI capacity 

[RFC PATCH 7/8] hw/riscv: meson: add CBQRI options to the build

[Drew Fustini]

From: Nicolas Pitre 

Build the CBQRI controllers and CBQRI example configuration when enabled
by Kconfig.

Signed-off-by: Nicolas Pitre 
Signed-off-by: Drew Fustini 
---
 hw/riscv/meson.build | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build
index 2f7ee81be3ca..cec6ecdcf5de 100644
--- a/hw/riscv/meson.build
+++ b/hw/riscv/meson.build
@@ -11,4 +11,8 @@ riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c'))
 riscv_ss.add(when: 'CONFIG_MICROCHIP_PFSOC', if_true: 
files('microchip_pfsoc.c'))
 riscv_ss.add(when: 'CONFIG_ACPI', if_true: files('virt-acpi-build.c'))
 
+riscv_ss.add(when: 'CONFIG_RISCV_CBQRI',
+  if_true: files('cbqri_capacity.c', 'cbqri_bandwidth.c'))
+riscv_ss.add(when: 'CONFIG_CBQRI_EXAMPLE_SOC', if_true: 
files('cbqri_example_soc.c'))
+
 hw_arch += {'riscv': riscv_ss}
-- 
2.34.1

[RFC PATCH 5/8] hw/riscv: instantiate CBQRI controllers for an example SoC

[Drew Fustini]

From: Nicolas Pitre 

Instantiate a hypothetical CBQRI configuration for testing purposes with
these properties:

  - L2 cache controllers
- Resource type: Capacity
- NCBLKS: 12
- Number of access types: 2 (code and data)
- Usage monitoring not supported
- Capacity allocation operations: CONFIG_LIMIT, READ_LIMIT

  - Last-level cache (LLC) controller
- Resource type: Capacity
- NCBLKS: 16
- Number of access types: 2 (code and data)
- Usage monitoring operations: CONFIG_EVENT, READ_COUNTER
- Event IDs supported: None, Occupancy
- Capacity allocation ops: CONFIG_LIMIT, READ_LIMIT, FLUSH_RCID

  - Memory controllers
- Resource type: Bandwidth
- NBWBLKS: 1024
- MRBWB: 819 (80% of NBWBLKS)
- Number of access types: 1 (no code/data differentiation)
- Usage monitoring operations: CONFIG_EVENT, READ_COUNTER
- Event IDs supported: None, Total read/write byte count, Total
   read byte count, Total write byte count
- Bandwidth allocation operations: CONFIG_LIMIT, READ_LIMIT

The memory map for the CBQRI controllers in this example SoC:

  Base addr  Size
  0x482  4KB  Cluster 0 L2 cache controller
  0x4821000  4KB  Cluster 1 L2 cache controller
  0x4828000  4KB  Memory controller 0
  0x4829000  4KB  Memory controller 1
  0x482A000  4KB  Memory controller 2
  0x482B000  4KB  Shared LLC cache controller

Signed-off-by: Nicolas Pitre 
Signed-off-by: Drew Fustini 
---
Note: this solution is not flexible enough for upstream inclusion.
Future work will allow CBQRI controllers to be configured by command
line options, and the controller will only be instantiated if the user
specifies a valid configuration.

 hw/riscv/cbqri_example_soc.c | 124 +++
 1 file changed, 124 insertions(+)
 create mode 100644 hw/riscv/cbqri_example_soc.c

diff --git a/hw/riscv/cbqri_example_soc.c b/hw/riscv/cbqri_example_soc.c
new file mode 100644
index ..91240cdd105e
--- /dev/null
+++ b/hw/riscv/cbqri_example_soc.c
@@ -0,0 +1,124 @@
+/*
+ * RISC-V Capacity and Bandwidth QoS Register Interface
+ * URL: https://github.com/riscv-non-isa/riscv-cbqri
+ *
+ * Copyright (c) 2023 BayLibre SAS
+ *
+ * This file contains an hypothetical CBQRI configuration instantiation
+ * for testing purposes. This ought to become configurable from the command
+ * line eventually.
+ *
+ * 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 "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/sysbus.h"
+#include "target/riscv/cpu.h"
+#include "hw/riscv/cbqri.h"
+
+/*
+ * Example hardware:
+ *
+ * - Global
+ *   - Number of RCIDs - 64
+ *   - Number of MCIDs - 256
+ * - L2 cache
+ *   - NCBLKS - 12
+ *   - Number of access types - 2 (code and data)
+ *   - Usage monitoring not supported
+ *   - Capacity allocation operations - CONFIG_LIMIT, READ_LIMIT
+ * - LLC
+ *   - NCBLKS - 16
+ *   - Number of access types - 2 (code and data)
+ *   - Usage monitoring operations - CONFIG_EVENT, READ_COUNTER
+ *   - Event IDs supported - None, Occupancy
+ *   - Capacity allocation operations - CONFIG_LIMIT, READ_LIMIT, FLUSH_RCID
+ * - Memory controllers
+ *   - NBWBLKS - 1024
+ *   - MRBWB - 80 (80%)
+ *   - Usage monitoring operations - CONFIG_EVENT, READ_COUNTER
+ *   - Event IDs supported - None, Total read/write byte count,
+ *   - total read byte count, total write byte count
+ *   - Bandwidth allocation operations - CONFIG_LIMIT, READ_LIMIT
+ *   - Number of access types - 1 (no code/data differentiation)
+ *
+ * 0x0482  Cluster 0 L2 cache controller
+ * 0x04821000  Cluster 1 L2 cache controller
+ * 0x0482B000  Shared LLC controller
+ * 0x04828000  Memory controller 0
+ * 0x04829000  Memory controller 1
+ * 0x0482A000  Memory controller 2
+ */
+
+#define CBQRI_NB_MCIDS  256
+#define CBQRI_NB_RCIDS  64
+
+static const RiscvCbqriCapacityCaps example_soc_L2_cluster = {
+.nb_mcids = CBQRI_NB_MCIDS,
+.nb_rcids = CBQRI_NB_RCIDS,
+.ncblks = 12,
+.supports_at_data = true,
+.supports_at_code = true,
+.supports_alloc_op_config_limit = true,
+.supports_alloc_op_read_limit = true,
+};
+
+static const RiscvCbqriCapacityCaps example_soc_LLC = {
+.nb_mcids = CBQRI_NB_MCIDS,
+.nb_rcids = CBQRI_NB_RCIDS,
+.ncblks = 16,
+.supports_at_data = true,
+

[RFC PATCH 1/8] riscv: implement Ssqosid extension and sqoscfg CSR

[Drew Fustini]

From: Kornel Dulęba 

Implement the sqoscfg CSR defined by the Ssqosid ISA extension
(Supervisor-mode Quality of Service ID). The CSR contains two fields:

  - Resource Control ID (RCID) used determine resource allocation
  - Monitoring Counter ID (MCID) used to track resource usage

The CSR is defined for S-mode but accessing it when V=1 shall cause a
virtual instruction exception. Implement this behavior by calling the
hmode predicate.

Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf
Signed-off-by: Kornel Dulęba 
[dfustini: rebase on v8.0.0-rc4, reword commit message]
Signed-off-by: Drew Fustini 
---
Note: the Ssqosid extension and CBQRI spec are still in a draft state.
The CSR address of sqoscfg is not final.

 disas/riscv.c   |  1 +
 target/riscv/cpu.c  |  2 ++
 target/riscv/cpu.h  |  3 +++
 target/riscv/cpu_bits.h |  5 +
 target/riscv/csr.c  | 34 ++
 5 files changed, 45 insertions(+)

diff --git a/disas/riscv.c b/disas/riscv.c
index d6b0fbe5e877..94336f54637b 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -2100,6 +2100,7 @@ static const char *csr_name(int csrno)
 case 0x0143: return "stval";
 case 0x0144: return "sip";
 case 0x0180: return "satp";
+case 0x0181: return "sqoscfg";
 case 0x0200: return "hstatus";
 case 0x0202: return "hedeleg";
 case 0x0203: return "hideleg";
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1e97473af27b..fb3f8c43a32d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -114,6 +114,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(smaia, true, PRIV_VERSION_1_12_0, ext_smaia),
 ISA_EXT_DATA_ENTRY(ssaia, true, PRIV_VERSION_1_12_0, ext_ssaia),
 ISA_EXT_DATA_ENTRY(sscofpmf, true, PRIV_VERSION_1_12_0, ext_sscofpmf),
+ISA_EXT_DATA_ENTRY(ssqosid, true, PRIV_VERSION_1_12_0, ext_ssqosid),
 ISA_EXT_DATA_ENTRY(sstc, true, PRIV_VERSION_1_12_0, ext_sstc),
 ISA_EXT_DATA_ENTRY(svadu, true, PRIV_VERSION_1_12_0, ext_svadu),
 ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
@@ -1397,6 +1398,7 @@ static Property riscv_cpu_extensions[] = {
 
 DEFINE_PROP_BOOL("svadu", RISCVCPU, cfg.ext_svadu, true),
 
+DEFINE_PROP_BOOL("ssqosid", RISCVCPU, cfg.ext_ssqosid, true),
 DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
 DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
 DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a57..ffc1b5009d15 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -222,6 +222,8 @@ struct CPUArchState {
 target_ulong mcause;
 target_ulong mtval;  /* since: priv-1.10.0 */
 
+target_ulong sqoscfg;
+
 /* Machine and Supervisor interrupt priorities */
 uint8_t miprio[64];
 uint8_t siprio[64];
@@ -454,6 +456,7 @@ struct RISCVCPUConfig {
 bool ext_icboz;
 bool ext_zicond;
 bool ext_zihintpause;
+bool ext_ssqosid;
 bool ext_smstateen;
 bool ext_sstc;
 bool ext_svadu;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fca7ef0cef91..d11a3928735e 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -217,6 +217,7 @@
 /* Supervisor Protection and Translation */
 #define CSR_SPTBR   0x180
 #define CSR_SATP0x180
+#define CSR_SQOSCFG 0x181
 
 /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
 #define CSR_SISELECT0x150
@@ -898,4 +899,8 @@ typedef enum RISCVException {
 #define MHPMEVENT_IDX_MASK 0xF
 #define MHPMEVENT_SSCOF_RESVD  16
 
+/* SQOSCFG BITS (QOSID) */
+#define SQOSCFG_RCID  0x0FFF
+#define SQOSCFG_MCID  0x0FFF
+
 #endif
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b63a..5769b3545704 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2700,6 +2700,37 @@ static RISCVException write_satp(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
+static RISCVException check_sqoscfg(CPURISCVState *env, int csrno)
+{
+RISCVCPU *cpu = env_archcpu(env);
+
+if (!cpu->cfg.ext_ssqosid) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+/*
+ * Even though this is an S-mode CSR the spec says that we need to throw
+ * and virt instruction fault if a guest tries to access it.
+ */
+return hmode(env, csrno);
+}
+
+static RISCVException read_sqoscfg(CPURISCVState *env, int csrno,
+target_ulong *val)
+{
+*val = env->sqoscfg;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_sqoscfg(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+env->sqoscfg = val & (SQOSCFG_RCID | SQOSCFG_MCID);
+return RISCV_EXCP_NONE;
+}
+
+
+
 static int read_vstopi(CPURISCVState *env, int csrno, target_ulong 

[RFC PATCH 8/8] hw/riscv: virt: initialize the CBQRI example SoC

[Drew Fustini]

From: Nicolas Pitre 

Initialize an example SoC that instantiates CBQRI capacity and bandwidth
controllers with specific parameters for testing purposes.

Signed-off-by: Nicolas Pitre 
Signed-off-by: Drew Fustini 
---
Note: this solution is not flexible enough for upstream inclusion.
Future work will allow CBQRI controllers to be configured by command
line options, and the controller will only be instantiated if the user
specifies a valid configuration.

 hw/riscv/virt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4e3efbee16f0..38edc4b91c93 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -34,6 +34,7 @@
 #include "hw/riscv/riscv_hart.h"
 #include "hw/riscv/virt.h"
 #include "hw/riscv/boot.h"
+#include "hw/riscv/cbqri.h"
 #include "hw/riscv/numa.h"
 #include "hw/intc/riscv_aclint.h"
 #include "hw/intc/riscv_aplic.h"
@@ -1519,6 +1520,8 @@ static void virt_machine_init(MachineState *machine)
 }
 virt_flash_map(s, system_memory);
 
+example_soc_cbqri_init();
+
 /* load/create device tree */
 if (machine->dtb) {
 machine->fdt = load_device_tree(machine->dtb, >fdt_size);
-- 
2.34.1

[RFC PATCH 3/8] hw/riscv: implement CBQRI capacity controller

[Drew Fustini]

From: Nicolas Pitre 

Implement a capacity controller according to the Capacity and Bandwidth
QoS Register Interface (CBQRI) which supports these capabilities:

  - Number of access types: 2 (code and data)
  - Usage monitoring operations: CONFIG_EVENT, READ_COUNTER
  - Event IDs supported: None, Occupancy
  - Capacity allocation ops: CONFIG_LIMIT, READ_LIMIT, FLUSH_RCID

Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf
Signed-off-by: Nicolas Pitre 
Signed-off-by: Drew Fustini 
---
 hw/riscv/cbqri_capacity.c | 532 ++
 1 file changed, 532 insertions(+)
 create mode 100644 hw/riscv/cbqri_capacity.c

diff --git a/hw/riscv/cbqri_capacity.c b/hw/riscv/cbqri_capacity.c
new file mode 100644
index ..a9f65c2ba25f
--- /dev/null
+++ b/hw/riscv/cbqri_capacity.c
@@ -0,0 +1,532 @@
+/*
+ * RISC-V Capacity and Bandwidth QoS Register Interface
+ * URL: https://github.com/riscv-non-isa/riscv-cbqri
+ *
+ * Copyright (c) 2023 BayLibre SAS
+ *
+ * This file contains the Capacity-controller QoS Register Interface.
+ *
+ * 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 "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/bitmap.h"
+#include "hw/sysbus.h"
+#include "target/riscv/cpu.h"
+#include "hw/riscv/cbqri.h"
+
+/* Encodings of `AT` field */
+enum {
+CC_AT_DATA = 0,
+CC_AT_CODE = 1,
+};
+
+/* Capabilities */
+REG64(CC_CAPABILITIES, 0);
+FIELD(CC_CAPABILITIES, VER, 0, 8);
+FIELD(CC_CAPABILITIES, VER_MINOR, 0, 4);
+FIELD(CC_CAPABILITIES, VER_MAJOR, 4, 4);
+FIELD(CC_CAPABILITIES, NCBLKS, 8, 16);
+FIELD(CC_CAPABILITIES, FRCID, 24, 1);
+
+/* Usage monitoring control */
+REG64(CC_MON_CTL, 8);
+FIELD(CC_MON_CTL, OP, 0, 5);
+FIELD(CC_MON_CTL, AT, 5, 3);
+FIELD(CC_MON_CTL, MCID, 8, 12);
+FIELD(CC_MON_CTL, EVT_ID, 20, 8);
+FIELD(CC_MON_CTL, ATV, 28, 1);
+FIELD(CC_MON_CTL, STATUS, 32, 7);
+FIELD(CC_MON_CTL, BUSY, 39, 1);
+
+/* Usage monitoring operations */
+enum {
+CC_MON_OP_CONFIG_EVENT = 1,
+CC_MON_OP_READ_COUNTER = 2,
+};
+
+/* Usage monitoring event ID */
+enum {
+CC_EVT_ID_None = 0,
+CC_EVT_ID_Occupancy = 1,
+};
+
+/* CC_MON_CTL.STATUS field encodings */
+enum {
+CC_MON_CTL_STATUS_SUCCESS = 1,
+CC_MON_CTL_STATUS_INVAL_OP = 2,
+CC_MON_CTL_STATUS_INVAL_MCID = 3,
+CC_MON_CTL_STATUS_INVAL_EVT_ID = 4,
+CC_MON_CTL_STATUS_INVAL_AT = 5,
+};
+
+/* Monitoring counter value */
+REG64(CC_MON_CTR_VAL, 16);
+FIELD(CC_MON_CTR_VAL, CTR, 0, 63);
+FIELD(CC_MON_CTR_VAL, INVALID, 63, 1);
+
+/* Capacity allocation control */
+REG64(CC_ALLOC_CTL, 24);
+FIELD(CC_ALLOC_CTL, OP, 0, 5);
+FIELD(CC_ALLOC_CTL, AT, 5, 3);
+FIELD(CC_ALLOC_CTL, RCID, 8, 12);
+FIELD(CC_ALLOC_CTL, STATUS, 32, 7);
+FIELD(CC_ALLOC_CTL, BUSY, 39, 1);
+
+/* Capacity allocation operations */
+enum {
+CC_ALLOC_OP_CONFIG_LIMIT = 1,
+CC_ALLOC_OP_READ_LIMIT = 2,
+CC_ALLOC_OP_FLUSH_RCID = 3,
+};
+
+/* CC_ALLOC_CTL.STATUS field encodings */
+enum {
+CC_ALLOC_STATUS_SUCCESS = 1,
+CC_ALLOC_STATUS_INVAL_OP = 2,
+CC_ALLOC_STATUS_INVAL_RCID = 3,
+CC_ALLOC_STATUS_INVAL_AT = 4,
+CC_ALLOC_STATUS_INVAL_BLKMASK = 5,
+};
+
+REG64(CC_BLOCK_MASK, 32);
+
+
+typedef struct MonitorCounter {
+uint64_t ctr_val;
+int at;
+int evt_id;
+bool active;
+} MonitorCounter;
+
+typedef struct RiscvCbqriCapacityState {
+SysBusDevice parent_obj;
+MemoryRegion mmio;
+const char *name;
+const RiscvCbqriCapacityCaps *caps;
+
+/* cached value of some registers */
+uint64_t cc_mon_ctl;
+uint64_t cc_mon_ctr_val;
+uint64_t cc_alloc_ctl;
+
+/* monitoring counters */
+MonitorCounter *mon_counters;
+
+/* allocation blockmasks (1st one is the CC_BLOCK_MASK register) */
+uint64_t *alloc_blockmasks;
+} RiscvCbqriCapacityState;
+
+#define TYPE_RISCV_CBQRI_CC "riscv.cbqri.capacity"
+
+#define RISCV_CBQRI_CC(obj) \
+OBJECT_CHECK(RiscvCbqriCapacityState, (obj), TYPE_RISCV_CBQRI_CC)
+
+static uint64_t *get_blockmask_location(RiscvCbqriCapacityState *cc,
+uint32_t rcid, uint32_t at)
+{
+/*
+ * All blockmasks are contiguous to simplify allocation.
+ * The first one is used to hold the CC_BLOCK_MASK register content,
+ * followed by respective blockmasks for each AT per RCID.
+ * Each blockmask is made of one or 

[RFC PATCH 6/8] hw/riscv: Kconfig: add CBQRI options

[Drew Fustini]

From: Nicolas Pitre 

Add Kconfig options for CBQRI and an example instantiation of capacity
and bandwidth controllers.

Signed-off-by: Nicolas Pitre 
Signed-off-by: Drew Fustini 
---
 hw/riscv/Kconfig | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 6528ebfa3a3b..04de0273888b 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -45,6 +45,14 @@ config RISCV_VIRT
 select FW_CFG_DMA
 select PLATFORM_BUS
 select ACPI
+select CBQRI_EXAMPLE_SOC
+
+config RISCV_CBQRI
+ bool
+
+config CBQRI_EXAMPLE_SOC
+ bool
+ select RISCV_CBQRI
 
 config SHAKTI_C
 bool
-- 
2.34.1

[RFC PATCH 4/8] hw/riscv: implement CBQRI memory controller

[Drew Fustini]

From: Nicolas Pitre 

Implement a bandwidth controller according to the Capacity and Bandwidth
QoS Register Interface (CBQRI) which supports these capabilities:

- Number of access types: 1 (no code/data differentiation)
- Usage monitoring operations: CONFIG_EVENT, READ_COUNTER
- Event IDs supported: None, Total read/write byte count, Total
   read byte count, Total write byte count
- Bandwidth allocation operations: CONFIG_LIMIT, READ_LIMIT

Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf
Signed-off-by: Nicolas Pitre 
Signed-off-by: Drew Fustini 
---
 hw/riscv/cbqri_bandwidth.c | 511 +
 1 file changed, 511 insertions(+)
 create mode 100644 hw/riscv/cbqri_bandwidth.c

diff --git a/hw/riscv/cbqri_bandwidth.c b/hw/riscv/cbqri_bandwidth.c
new file mode 100644
index ..da33c34fada4
--- /dev/null
+++ b/hw/riscv/cbqri_bandwidth.c
@@ -0,0 +1,511 @@
+/*
+ * RISC-V Capacity and Bandwidth QoS Register Interface
+ * URL: https://github.com/riscv-non-isa/riscv-cbqri
+ *
+ * Copyright (c) 2023 BayLibre SAS
+ *
+ * This file contains the Bandwidth-controller QoS Register Interface.
+ *
+ * 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 "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/sysbus.h"
+#include "target/riscv/cpu.h"
+#include "hw/riscv/cbqri.h"
+
+
+/* Encodings of `AT` field */
+enum {
+BC_AT_DATA = 0,
+BC_AT_CODE = 1,
+};
+
+/* Capabilities */
+REG64(BC_CAPABILITIES, 0);
+FIELD(BC_CAPABILITIES, VER, 0, 8);
+FIELD(BC_CAPABILITIES, VER_MINOR, 0, 4);
+FIELD(BC_CAPABILITIES, VER_MAJOR, 4, 4);
+FIELD(BC_CAPABILITIES, NBWBLKS, 8, 16);
+FIELD(BC_CAPABILITIES, MRBWB, 32, 16);
+
+/* Usage monitoring control */
+REG64(BC_MON_CTL, 8);
+FIELD(BC_MON_CTL, OP, 0, 5);
+FIELD(BC_MON_CTL, AT, 5, 3);
+FIELD(BC_MON_CTL, MCID, 8, 12);
+FIELD(BC_MON_CTL, EVT_ID, 20, 8);
+FIELD(BC_MON_CTL, ATV, 28, 1);
+FIELD(BC_MON_CTL, STATUS, 32, 7);
+FIELD(BC_MON_CTL, BUSY, 39, 1);
+
+/* Usage monitoring operations */
+enum {
+BC_MON_OP_CONFIG_EVENT = 1,
+BC_MON_OP_READ_COUNTER = 2,
+};
+
+/* Bandwidth monitoring event ID */
+enum {
+BC_EVT_ID_None = 0,
+BC_EVT_ID_RDWR_count = 1,
+BC_EVT_ID_RDONLY_count = 2,
+BC_EVT_ID_WRONLY_count = 3,
+};
+
+/* BC_MON_CTL.STATUS field encodings */
+enum {
+BC_MON_CTL_STATUS_SUCCESS = 1,
+BC_MON_CTL_STATUS_INVAL_OP = 2,
+BC_MON_CTL_STATUS_INVAL_MCID = 3,
+BC_MON_CTL_STATUS_INVAL_EVT_ID = 4,
+BC_MON_CTL_STATUS_INVAL_AT = 5,
+};
+
+/* Monitoring counter value */
+REG64(BC_MON_CTR_VAL, 16);
+FIELD(BC_MON_CTR_VAL, CTR, 0, 62);
+FIELD(BC_MON_CTR_VAL, INVALID, 62, 1);
+FIELD(BC_MON_CTR_VAL, OVF, 63, 1);
+
+/* Bandwidth Allocation control */
+REG64(BC_ALLOC_CTL, 24);
+FIELD(BC_ALLOC_CTL, OP, 0, 5);
+FIELD(BC_ALLOC_CTL, AT, 5, 3);
+FIELD(BC_ALLOC_CTL, RCID, 8, 12);
+FIELD(BC_ALLOC_CTL, STATUS, 32, 7);
+FIELD(BC_ALLOC_CTL, BUSY, 39, 1);
+
+/* Bandwidth allocation operations */
+enum {
+BC_ALLOC_OP_CONFIG_LIMIT = 1,
+BC_ALLOC_OP_READ_LIMIT = 2,
+};
+
+/* BC_ALLOC_CTL.STATUS field encodings */
+enum {
+BC_ALLOC_STATUS_SUCCESS = 1,
+BC_ALLOC_STATUS_INVAL_OP = 2,
+BC_ALLOC_STATUS_INVAL_RCID = 3,
+BC_ALLOC_STATUS_INVAL_AT = 4,
+BC_ALLOC_STATUS_INVAL_BLKS = 5,
+};
+
+/* Bandwidth allocation */
+REG64(BC_BW_ALLOC, 32);
+FIELD(BC_BW_ALLOC, Rbwb, 0, 16);
+FIELD(BC_BW_ALLOC, Mweight, 20, 8);
+FIELD(BC_BW_ALLOC, sharedAT, 28, 3);
+FIELD(BC_BW_ALLOC, useShared, 31, 1);
+
+
+typedef struct MonitorCounter {
+uint64_t ctr_val;
+int at;
+int evt_id;
+bool active;
+} MonitorCounter;
+
+typedef struct BandwidthAllocation {
+uint32_t Rbwb:16;
+uint32_t Mweight:8;
+uint32_t sharedAT:3;
+bool useShared:1;
+} BandwidthAllocation;
+
+typedef struct RiscvCbqriBandwidthState {
+SysBusDevice parent_obj;
+MemoryRegion mmio;
+const char *name;
+const RiscvCbqriBandwidthCaps *caps;
+
+/* cached value of some registers */
+uint64_t bc_mon_ctl;
+uint64_t bc_mon_ctr_val;
+uint64_t bc_alloc_ctl;
+uint64_t bc_bw_alloc;
+
+MonitorCounter *mon_counters;
+BandwidthAllocation *bw_allocations;
+} RiscvCbqriBandwidthState;
+
+#define TYPE_RISCV_CBQRI_BC "riscv.cbqri.bandwidth"
+
+#define RISCV_CBQRI_BC(obj) \
+

[PATCH 0/1] hw/ide/core.c: fix handling of unsupported commands

[Mateusz Albecki]

The patch was developed during the debug for the UEFI SCT issue reported in
https://bugzilla.tianocore.org/show_bug.cgi?id=4016. After fixing the issue
in EDK2 the test was still failing on qemu since qemu wouldn't return abort and
instead we would get a command timeout. Additionally qemu also has another bug 
in that
it reports support for storage security commands which it doesn't support(will 
file a bug).

Tests:
- tested against UEFI SCT test mentioned in bugzilla and it is passing.
- able to read/write files on ahci controller from UEFI.

Mateusz Albecki (1):
  hw/ide/core.c: fix handling of unsupported commands

 hw/ide/core.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.40.0

[PATCH 1/1] hw/ide/core.c: fix handling of unsupported commands

[Mateusz Albecki]

From: Mateusz Albecki 

Current code will not call ide_cmd_done when aborting the unsupported
command which will lead to the command timeout on the driver side instead
of getting a D2H FIS with ABRT indication. This can lead to problems on the
driver side as the spec mandates that device should return a D2H FIS with
ABRT bit set in ERR register(from SATA 3.1 section 16.3.3.8.6)

Signed-off-by: Mateusz Albecki 
---
 hw/ide/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 45d14a25e9..d7027bbd4d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2146,6 +2146,7 @@ void ide_bus_exec_cmd(IDEBus *bus, uint32_t val)
 
 if (!ide_cmd_permitted(s, val)) {
 ide_abort_command(s);
+ide_cmd_done(s);
 ide_bus_set_irq(s->bus);
 return;
 }
-- 
2.40.0

[PATCH] linux-user: Don't require PROT_READ for mincore

[Thomas Weißschuh]

The kernel does not require PROT_READ for addresses passed to mincore.
For example the fincore(1) tool from util-linux uses PROT_NONE and
currently does not work under qemu-user.

Example (with fincore(1) from util-linux 2.38):

$ fincore /proc/self/exe
RES PAGES  SIZE FILE
24K 6 22.1K /proc/self/exe

$ qemu-x86_64 /usr/bin/fincore /proc/self/exe
fincore: failed to do mincore: /proc/self/exe: Cannot allocate memory

With this patch:

$ ./build/qemu-x86_64 /usr/bin/fincore /proc/self/exe
RES PAGES  SIZE FILE
24K 6 22.1K /proc/self/exe

Signed-off-by: Thomas Weißschuh 
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 69f740ff98c8..0ebcbf3d7f90 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11897,7 +11897,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
 #ifdef TARGET_NR_mincore
 case TARGET_NR_mincore:
 {
-void *a = lock_user(VERIFY_READ, arg1, arg2, 0);
+void *a = lock_user(0, arg1, arg2, 0);
 if (!a) {
 return -TARGET_ENOMEM;
 }

base-commit: 7dbd6f8a27e30fe14adb3d5869097cddf24038d6
-- 
2.40.0

RE: [PATCH] replication: compile out some staff when replication is not configured

[Zhang, Chen]

> -Original Message-
> From: Vladimir Sementsov-Ogievskiy 
> Sent: Friday, April 14, 2023 5:51 PM
> To: Zhang, Chen ; qemu-devel@nongnu.org
> Cc: qemu-bl...@nongnu.org; pbonz...@redhat.com; arm...@redhat.com;
> ebl...@redhat.com; jasow...@redhat.com; dgilb...@redhat.com;
> quint...@redhat.com; hre...@redhat.com; kw...@redhat.com; Zhang,
> Hailiang ; lizhij...@fujitsu.com;
> wencongya...@huawei.com; xiechanglon...@gmail.com; den-
> plotni...@yandex-team.ru
> Subject: Re: [PATCH] replication: compile out some staff when replication is
> not configured
> 
> On 14.04.23 04:24, Zhang, Chen wrote:
> >> So, if I want to have an option to disable all COLO modules, do you
> >> mean it should be additional --disable-colo option? Or better keep
> >> one option -- disable-replication (and, maybe just rename to to --disable-
> colo)?
> > I think keep the option --disable-replication is enough.
> > Generally speaking, these three modules do not belong to COLO, It has
> been decoupled at the time of design.
> > Instead, COLO is formed when these three modules are used in
> combination.
> 
> But it's not enough to me, I want to have a possibility to not build the
> subsystem I don't need.

As I said, COLO not a specific subsystem, It is a usage of three general 
subsystems.
Let's back to this patch, it try to not build block replication when not 
configured. It's OK.
Although COLO may be the only user of replication,  but can't assume all the 
COLO used subsystem
not needed, even have a --disable-colo. For example in this patch disabled the 
net/filter-mirror/redirector
Qemu network filter is a general framework with many submodules: 
filter-buffer/replay/mirror/rewriter.
Logically speaking, It is completely irrelevant with COLO.

Thanks
Chen


> 
> --
> Best regards,
> Vladimir

RE: [PATCH 30/40] igb: Implement igb-specific oversize check

[Sriram Yagnaraman]

> -Original Message-
> From: Akihiko Odaki 
> Sent: Friday, 14 April 2023 13:37
> Cc: Sriram Yagnaraman ; Jason Wang
> ; Dmitry Fleytman ;
> Michael S. Tsirkin ; Alex Bennée ;
> Philippe Mathieu-Daudé ; Thomas Huth
> ; Wainer dos Santos Moschetta
> ; Beraldo Leal ; Cleber Rosa
> ; Laurent Vivier ; Paolo Bonzini
> ; qemu-devel@nongnu.org; Akihiko Odaki
> 
> Subject: [PATCH 30/40] igb: Implement igb-specific oversize check
> 
> igb has a configurable size limit for LPE, and uses different limits 
> depending on
> whether the packet is treated as a VLAN packet.
> 
> Signed-off-by: Akihiko Odaki 
> ---
>  hw/net/igb_core.c | 41 +++--
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> 2013a9a53d..569897fb99 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -954,16 +954,21 @@ igb_rx_l4_cso_enabled(IGBCore *core)
>  return !!(core->mac[RXCSUM] & E1000_RXCSUM_TUOFLD);  }
> 
> -static bool

The convention in seems to be to declare return value in first line and then 
the function name in the next line. 

> -igb_rx_is_oversized(IGBCore *core, uint16_t qn, size_t size)
> +static bool igb_rx_is_oversized(IGBCore *core, const struct eth_header *ehdr,
> +size_t size, bool lpe, uint16_t rlpml)
>  {
> -uint16_t pool = qn % IGB_NUM_VM_POOLS;
> -bool lpe = !!(core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE);
> -int max_ethernet_lpe_size =
> -core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK;
> -int max_ethernet_vlan_size = 1522;
> +size += 4;

Is the above 4 CRC bytes?

> +
> +if (lpe) {
> +return size > rlpml;
> +}
> +
> +if (e1000x_is_vlan_packet(ehdr, core->mac[VET] & 0x) &&
> +e1000x_vlan_rx_filter_enabled(core->mac)) {
> +return size > 1522;
> +}

Should a check for 1526 bytes if extended VLAN is present be added?
Maybe in "igb: Strip the second VLAN tag for extended VLAN"?

> 
> -return size > (lpe ? max_ethernet_lpe_size : max_ethernet_vlan_size);
> +return size > 1518;
>  }
> 
>  static uint16_t igb_receive_assign(IGBCore *core, const L2Header *l2_header,
> @@ -976,6 +981,8 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const L2Header *l2_header,
>  uint16_t queues = 0;
>  uint16_t oversized = 0;
>  uint16_t vid = be16_to_cpu(l2_header->vlan[0].h_tci) & VLAN_VID_MASK;
> +bool lpe;
> +uint16_t rlpml;
>  int i;
> 
>  memset(rss_info, 0, sizeof(E1000E_RSSInfo)); @@ -984,6 +991,14 @@
> static uint16_t igb_receive_assign(IGBCore *core, const L2Header *l2_header,
>  *external_tx = true;
>  }
> 
> +lpe = !!(core->mac[RCTL] & E1000_RCTL_LPE);
> +rlpml = core->mac[RLPML];
> +if (!(core->mac[RCTL] & E1000_RCTL_SBP) &&
> +igb_rx_is_oversized(core, ehdr, size, lpe, rlpml)) {
> +trace_e1000x_rx_oversized(size);
> +return queues;
> +}
> +
>  if (e1000x_is_vlan_packet(ehdr, core->mac[VET] & 0x) &&
>  !e1000x_rx_vlan_filter(core->mac, PKT_GET_VLAN_HDR(ehdr))) {
>  return queues;
> @@ -1067,7 +1082,10 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const L2Header *l2_header,
>  queues &= core->mac[VFRE];
>  if (queues) {
>  for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> -if ((queues & BIT(i)) && igb_rx_is_oversized(core, i, size)) 
> {
> +lpe = !!(core->mac[VMOLR0 + i] & E1000_VMOLR_LPE);
> +rlpml = core->mac[VMOLR0 + i] & E1000_VMOLR_RLPML_MASK;
> +if ((queues & BIT(i)) &&
> +igb_rx_is_oversized(core, ehdr, size, lpe, rlpml))
> + {
>  oversized |= BIT(i);
>  }
>  }
> @@ -1609,11 +1627,6 @@ igb_receive_internal(IGBCore *core, const struct
> iovec *iov, int iovcnt,
>  iov_to_buf(iov, iovcnt, iov_ofs, _buf, 
> sizeof(min_buf.l2_header));
>  }
> 
> -/* Discard oversized packets if !LPE and !SBP. */
> -if (e1000x_is_oversized(core->mac, size)) {
> -return orig_size;
> -}
> -
>  net_rx_pkt_set_packet_type(core->rx_pkt,
> get_eth_packet_type(_buf.l2_header.eth));
>  net_rx_pkt_set_protocols(core->rx_pkt, iov, iovcnt, iov_ofs);
> --
> 2.40.0