Re: [PATCH 3/7] hw/riscv: implement CBQRI capacity controller

[Drew Fustini <fustini@kernel.org>]

On Thu, Nov 20, 2025 at 08:47:44AM -0300, Daniel Henrique Barboza wrote:
> [ ...]
> 
> > +
> > +static void riscv_cbqri_cc_write(void *opaque, hwaddr addr,
> > +                                 uint64_t value, unsigned size)
> > +{
> > +    RiscvCbqriCapacityState *cc = opaque;
> > +
> > +    assert((addr % 8) == 0);
> > +    assert(size == 8);
> 
> So here and in the read callback (riscv_cbqri_cc_read) you're doing asserts for
> size == 8, while your memoryops has:
> 
> static const MemoryRegionOps riscv_cbqri_cc_ops = {
>     .read = riscv_cbqri_cc_read,
>     .write = riscv_cbqri_cc_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,
> };
> 
> 
> You can get rid of assert(size == 8) in both callbacks by setting
> min_access_size = 8.

Thanks for the review. 

I think that the assert in riscv_cbqri_cc_write maybe incorrect.

The CBQRI spec states that the registers:
 - start at an 8-byte aligned physical address. 
 - can accessed by using naturally aligned 4-byte or 8-byte accesses
 - 4-byte access to a register must be single-copy atomic
 - It is UNSPECIFIED whether 8-byte access must be single-copy atomic
 - 8-byte access can appear internally to the CBQRI implementation as
   if two separate 4-byte accesses are performed.

The spec further notes that:

"The CBQRI registers are defined so that software can perform two
individual 4 byte accesses, or hardware can perform two independent 4
byte transactions resulting from an 8 byte access, to the high and low
halves of the register as long as the register semantics, with regards
to side-effects, are respected between the two software accesses, or two
hardware transactions, respectively."

Based on the above, I believe .valid.min_access_size does need to stay
at 4 bytes. The Qemu documentation states that otherwise "accesses
outside this range will have device and bus specific behaviour (ignored,
or machine check)".

I am confused whether ".impl.min_access_size = 8" is correct. The Qemu
documentation states that "other access sizes will be emulated using the
ones available. For example a 4-byte write will be emulated using four
1-byte writes, if .impl.max_access_size = 1."

Radim asked if 32-bit (4 byte) access would be supported, but I am
confused how other access sizes are emulated.

Do I need to add code to the read and write hooks for riscv_cbqri_cc_ops
and riscv_cbqri_bc_ops?

Or is there some aspect of MemoryRegionOps that can handle the emulation
of other sizes automatically?

Thanks,
Drew