Re: [PATCH 4/7] hw/riscv: implement CBQRI bandwidth controller

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

On 11/19/25 9:42 PM, Drew Fustini wrote:
> From: Nicolas Pitre <npitre@baylibre.com>
> 
> Implement a bandwidth 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, 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-cbqri/blob/main/riscv-cbqri.pdf
> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> [fustini: add fields introduced in the ratified spec: rpfx and p]
> Signed-off-by: Drew Fustini <fustini@kernel.org>
> ---
>   MAINTAINERS                |   1 +
>   hw/riscv/cbqri_bandwidth.c | 613 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 614 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 48cca4ac8702..16500c4e8a83 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -362,6 +362,7 @@ M: Nicolas Pitre <npitre@baylibre.com>
>   M: Drew Fustini <fustini@kernel.org>
>   L: qemu-riscv@nongnu.org
>   S: Supported
> +F: hw/riscv/cbqri_bandwidth.c
>   F: hw/riscv/cbqri_capacity.c
>   F: include/hw/riscv/cbqri.h
>   
> diff --git a/hw/riscv/cbqri_bandwidth.c b/hw/riscv/cbqri_bandwidth.c
> new file mode 100644
> index 000000000000..40357a3219d8
> --- /dev/null
> +++ b/hw/riscv/cbqri_bandwidth.c
> @@ -0,0 +1,613 @@
> +/*
> + * RISC-V Capacity and Bandwidth QoS Register Interface
> + * URL: https://github.com/riscv-non-isa/riscv-cbqri/releases/tag/v1.0
> + *
> + * Copyright (c) 2023 BayLibre SAS
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * 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 <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/sysbus.h"
> +#include "target/riscv/cpu.h"
> +#include "hw/riscv/cbqri.h"
> +

[...]

> +static void riscv_cbqri_bc_write(void *opaque, hwaddr addr,
> +                                 uint64_t value, unsigned size)
> +{
> +    RiscvCbqriBandwidthState *bc = opaque;
> +
> +    assert((addr % 8) == 0);
> +    assert(size == 8);

Same comment I made on patch 3: both callbacks are asserting if size == 8. This can be
avoided by setting the minimal access size in riscv_cbqri_bc_ops to 8.


Thanks,

Daniel

> +
> +    switch (addr) {
> +    case A_BC_CAPABILITIES:
> +        /* read-only register */
> +        break;
> +    case A_BC_MON_CTL:
> +        riscv_cbqri_bc_write_mon_ctl(bc, value);
> +        break;
> +    case A_BC_MON_CTR_VAL:
> +        /* read-only register */
> +        break;
> +    case A_BC_ALLOC_CTL:
> +        riscv_cbqri_bc_write_alloc_ctl(bc, value);
> +        break;
> +    case A_BC_BW_ALLOC:
> +        riscv_cbqri_bc_write_bw_alloc(bc, value);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: out of bounds (addr=0x%x)",
> +                      __func__, (uint32_t)addr);
> +    }
> +}
> +
> +static uint64_t riscv_cbqri_bc_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    RiscvCbqriBandwidthState *bc = opaque;
> +    uint64_t value = 0;
> +
> +    assert((addr % 8) == 0);
> +    assert(size == 8);
> +
> +    switch (addr) {
> +    case A_BC_CAPABILITIES:
> +        value = FIELD_DP64(value, BC_CAPABILITIES, VER_MAJOR,
> +                           RISCV_CBQRI_VERSION_MAJOR);
> +        value = FIELD_DP64(value, BC_CAPABILITIES, VER_MINOR,
> +                           RISCV_CBQRI_VERSION_MINOR);
> +        value = FIELD_DP64(value, BC_CAPABILITIES, RPFX,
> +                           bc->rpfx);
> +        value = FIELD_DP64(value, BC_CAPABILITIES, P,
> +                           bc->p);
> +        value = FIELD_DP64(value, BC_CAPABILITIES, NBWBLKS,
> +                           bc->nbwblks);
> +        value = FIELD_DP64(value, BC_CAPABILITIES, MRBWB,
> +                           bc->mrbwb);
> +        break;
> +    case A_BC_MON_CTL:
> +        value = bc->bc_mon_ctl;
> +        break;
> +    case A_BC_MON_CTR_VAL:
> +        value = bc->bc_mon_ctr_val;
> +        break;
> +    case A_BC_ALLOC_CTL:
> +        value = bc->bc_alloc_ctl;
> +        break;
> +    case A_BC_BW_ALLOC:
> +        BandwidthAllocation *bc_bw_alloc = &bc->bw_allocations[0];
> +        value = FIELD_DP64(value, BC_BW_ALLOC, Rbwb, bc_bw_alloc->Rbwb);
> +        value = FIELD_DP64(value, BC_BW_ALLOC, Mweight, bc_bw_alloc->Mweight);
> +        value = FIELD_DP64(value, BC_BW_ALLOC, sharedAT, bc_bw_alloc->sharedAT);
> +        value = FIELD_DP64(value, BC_BW_ALLOC, useShared,
> +                           bc_bw_alloc->useShared);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: out of bounds (addr=0x%x)",
> +                      __func__, (uint32_t)addr);
> +    }
> +
> +    return value;
> +}
> +
> +static const MemoryRegionOps riscv_cbqri_bc_ops = {
> +    .read = riscv_cbqri_bc_read,
> +    .write = riscv_cbqri_bc_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +};
> +
> +static void riscv_cbqri_bc_realize(DeviceState *dev, Error **errp)
> +{
> +    RiscvCbqriBandwidthState *bc = RISCV_CBQRI_BC(dev);
> +
> +    if (!bc->mmio_base) {
> +        error_setg(errp, "mmio_base property not set");
> +        return;
> +    }
> +
> +    assert(bc->mon_counters == NULL);
> +    bc->mon_counters = g_new0(MonitorCounter, bc->nb_mcids);
> +
> +    assert(bc->bw_allocations == NULL);
> +    BandwidthAllocation *bw_alloc_end = get_bw_alloc(bc, bc->nb_rcids, 0);
> +    unsigned int bw_alloc_size = bw_alloc_end - bc->bw_allocations;
> +    bc->bw_allocations = g_new0(BandwidthAllocation, bw_alloc_size);
> +
> +    memory_region_init_io(&bc->mmio, OBJECT(dev), &riscv_cbqri_bc_ops,
> +                          bc, TYPE_RISCV_CBQRI_BC".mmio", 4 * 1024);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &bc->mmio);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, bc->mmio_base);
> +}
> +
> +static void riscv_cbqri_bc_reset(DeviceState *dev)
> +{
> +    RiscvCbqriBandwidthState *bc = RISCV_CBQRI_BC(dev);
> +
> +    bc->bc_mon_ctl = 0;
> +    bc->bc_alloc_ctl = 0;
> +}
> +
> +static Property riscv_cbqri_bc_properties[] = {
> +    DEFINE_PROP_UINT64("mmio_base", RiscvCbqriBandwidthState, mmio_base, 0),
> +    DEFINE_PROP_STRING("target", RiscvCbqriBandwidthState, target),
> +
> +    DEFINE_PROP_UINT16("max_mcids", RiscvCbqriBandwidthState, nb_mcids, 256),
> +    DEFINE_PROP_UINT16("max_rcids", RiscvCbqriBandwidthState, nb_rcids, 64),
> +    DEFINE_PROP_UINT16("nbwblks", RiscvCbqriBandwidthState, nbwblks, 1024),
> +    DEFINE_PROP_UINT16("mrbwb", RiscvCbqriBandwidthState, mrbwb, 819),
> +
> +    DEFINE_PROP_BOOL("rpfx", RiscvCbqriBandwidthState, rpfx, true),
> +    DEFINE_PROP_UINT8("p", RiscvCbqriBandwidthState, p, 4),
> +
> +    DEFINE_PROP_BOOL("at_data", RiscvCbqriBandwidthState,
> +                     supports_at_data, true),
> +    DEFINE_PROP_BOOL("at_code", RiscvCbqriBandwidthState,
> +                     supports_at_code, true),
> +
> +    DEFINE_PROP_BOOL("alloc_op_config_limit", RiscvCbqriBandwidthState,
> +                     supports_alloc_op_config_limit, true),
> +    DEFINE_PROP_BOOL("alloc_op_read_limit", RiscvCbqriBandwidthState,
> +                     supports_alloc_op_read_limit, true),
> +
> +    DEFINE_PROP_BOOL("mon_op_config_event", RiscvCbqriBandwidthState,
> +                     supports_mon_op_config_event, true),
> +    DEFINE_PROP_BOOL("mon_op_read_counter", RiscvCbqriBandwidthState,
> +                     supports_mon_op_read_counter, true),
> +
> +    DEFINE_PROP_BOOL("mon_evt_id_none", RiscvCbqriBandwidthState,
> +                     supports_mon_evt_id_none, true),
> +    DEFINE_PROP_BOOL("mon_evt_id_rdwr_count", RiscvCbqriBandwidthState,
> +                     supports_mon_evt_id_rdwr_count, true),
> +    DEFINE_PROP_BOOL("mon_evt_id_rdonly_count", RiscvCbqriBandwidthState,
> +                     supports_mon_evt_id_rdonly_count, true),
> +    DEFINE_PROP_BOOL("mon_evt_id_wronly_count", RiscvCbqriBandwidthState,
> +                     supports_mon_evt_id_wronly_count, true),
> +};
> +
> +static void riscv_cbqri_bc_class_init(ObjectClass *klass, const void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    dc->realize = riscv_cbqri_bc_realize;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    dc->desc = "RISC-V CBQRI Bandwidth Controller";
> +    device_class_set_props(dc, riscv_cbqri_bc_properties);
> +    dc->legacy_reset = riscv_cbqri_bc_reset;
> +    dc->user_creatable = true;
> +}
> +
> +static const TypeInfo riscv_cbqri_bc_info = {
> +    .name          = TYPE_RISCV_CBQRI_BC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(RiscvCbqriBandwidthState),
> +    .class_init    = riscv_cbqri_bc_class_init,
> +};
> +
> +static void riscv_cbqri_bc_register_types(void)
> +{
> +    type_register_static(&riscv_cbqri_bc_info);
> +}
> +
> +DeviceState *riscv_cbqri_bc_create(hwaddr addr,
> +                                   const RiscvCbqriBandwidthCaps *caps,
> +                                   const char *target_name)
> +{
> +    DeviceState *dev = qdev_new(TYPE_RISCV_CBQRI_BC);
> +
> +    qdev_prop_set_uint64(dev, "mmio_base", addr);
> +    qdev_prop_set_string(dev, "target", target_name);
> +    qdev_prop_set_uint16(dev, "max_mcids", caps->nb_mcids);
> +    qdev_prop_set_uint16(dev, "max_rcids", caps->nb_rcids);
> +    qdev_prop_set_uint16(dev, "nbwblks", caps->nbwblks);
> +    qdev_prop_set_bit(dev, "rpfx", caps->rpfx);
> +    qdev_prop_set_uint8(dev, "p", caps->p);
> +
> +    qdev_prop_set_bit(dev, "at_data",
> +                      caps->supports_at_data);
> +    qdev_prop_set_bit(dev, "at_code",
> +                      caps->supports_at_code);
> +    qdev_prop_set_bit(dev, "alloc_op_config_limit",
> +                      caps->supports_alloc_op_config_limit);
> +    qdev_prop_set_bit(dev, "alloc_op_read_limit",
> +                      caps->supports_alloc_op_read_limit);
> +    qdev_prop_set_bit(dev, "mon_op_config_event",
> +                      caps->supports_mon_op_config_event);
> +    qdev_prop_set_bit(dev, "mon_op_read_counter",
> +                      caps->supports_mon_op_read_counter);
> +    qdev_prop_set_bit(dev, "mon_evt_id_none",
> +                      caps->supports_mon_evt_id_none);
> +    qdev_prop_set_bit(dev, "mon_evt_id_rdwr_count",
> +                      caps->supports_mon_evt_id_rdwr_count);
> +    qdev_prop_set_bit(dev, "mon_evt_id_rdonly_count",
> +                      caps->supports_mon_evt_id_rdonly_count);
> +    qdev_prop_set_bit(dev, "mon_evt_id_wronly_count",
> +                      caps->supports_mon_evt_id_wronly_count);
> +
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> +
> +    return dev;
> +}
> +
> +type_init(riscv_cbqri_bc_register_types)
>