Re: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global CMDQV registers

2026-03-11 Thread Eric Auger



On 3/10/26 12:37 PM, Shameer Kolothum Thodi wrote:
> Hi Eric,
>
>> -Original Message-
>> From: Eric Auger 
>> Sent: 09 March 2026 16:33
>> To: Shameer Kolothum Thodi ; qemu-
>> [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; Nicolin
>> Chen ; Nathan Chen ; Matt
>> Ochs ; Jiandi An ; Jason Gunthorpe
>> ; [email protected];
>> [email protected]; [email protected]; Krishnakant Jaju
>> ; [email protected]
>> Subject: Re: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global
>> CMDQV registers
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Shameer,
>>
>> On 2/26/26 11:50 AM, Shameer Kolothum wrote:
>>> From: Nicolin Chen 
>>>
>>> Tegra241 CMDQV defines a set of global control and status registers
>>> used to configure virtual command queue allocation and interrupt
>>> behavior.
>>>
>>> Add read/write emulation for the global CMDQV register page
>>> (offset 0x0), backed by a simple register cache. This includes
>>> CONFIG, PARAM, STATUS, VI error and interrupt maps, CMDQ allocation
>>> map and the VINTF0 related registers defined in the global CMDQV
>>> register space.
>>>
>>> Signed-off-by: Nicolin Chen 
>>> Signed-off-by: Shameer Kolothum 
>>> ---
>>>  hw/arm/tegra241-cmdqv.h | 108
>> +++
>>>  hw/arm/tegra241-cmdqv.c | 121
>> +++-
>>>  2 files changed, 228 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
>>> index 46aa9e8a9f..50bcecee9d 100644
>>> --- a/hw/arm/tegra241-cmdqv.h
>>> +++ b/hw/arm/tegra241-cmdqv.h
>>> @@ -10,6 +10,9 @@
>>>  #ifndef HW_ARM_TEGRA241_CMDQV_H
>>>  #define HW_ARM_TEGRA241_CMDQV_H
>>>
>>> +#include "hw/core/registerfields.h"
>>> +#include "smmuv3-accel.h"
>>> +
>>>  #define TEGRA241_CMDQV_VERSION 1
>>>  #define TEGRA241_CMDQV_NUM_CMDQ_LOG2   1
>>>  #define TEGRA241_CMDQV_MAX_CMDQ(1U <<
>> TEGRA241_CMDQV_NUM_CMDQ_LOG2)
>>> @@ -31,8 +34,113 @@ typedef struct Tegra241CMDQV {
>>>  SMMUv3AccelState *s_accel;
>>>  MemoryRegion mmio_cmdqv;
>>>  qemu_irq irq;
>>> +
>>> +/* Register Cache */
>>> +uint32_t config;
>>> +uint32_t param;
>>> +uint32_t status;
>>> +uint32_t vi_err_map[2];
>>> +uint32_t vi_int_mask[2];
>>> +uint32_t cmdq_err_map[4];
>>> +uint32_t cmdq_alloc_map[TEGRA241_CMDQV_MAX_CMDQ];
>>> +uint32_t vintf_config;
>>> +uint32_t vintf_status;
>>> +uint32_t vintf_sid_match[16];
>>> +uint32_t vintf_sid_replace[16];
>>> +uint32_t vintf_cmdq_err_map[4];
>>>  } Tegra241CMDQV;
>>>
>>> +/* Global CMDQV MMIO registers (offset 0x0) */
>>> +REG32(CONFIG, 0x0)
>>> +FIELD(CONFIG, CMDQV_EN, 0, 1)
>>> +FIELD(CONFIG, CMDQV_PER_CMD_OFFSET, 1, 3)
>>> +FIELD(CONFIG, CMDQ_MAX_CLK_BATCH, 4, 8)
>>> +FIELD(CONFIG, CMDQ_MAX_CMD_BATCH, 12, 8)
>>> +FIELD(CONFIG, CONS_DRAM_EN, 20, 1)
>>> +
>>> +REG32(PARAM, 0x4)
>>> +FIELD(PARAM, CMDQV_VER, 0, 4)
>>> +FIELD(PARAM, CMDQV_NUM_CMDQ_LOG2, 4, 4)
>>> +FIELD(PARAM, CMDQV_NUM_VM_LOG2, 8, 4)
>> this is called CMDQV_NUM_VI_LOG2 in the spec I have access to
>> VI = virtual interface
>> I guess there is 1-1 mapping vetween VI and VM but I would keep spec
>> naming
>>> +FIELD(PARAM, CMDQV_NUM_SID_PER_VM_LOG2, 12, 4)
>> same here s/VM/VI
>>> +
>>> +REG32(STATUS, 0x8)
>>> +FIELD(STATUS, CMDQV_ENABLED, 0, 1)
>>> +
>> I would add "SMMU_CMDQV_VI_ERR_MAP_0/1 definitions"
>>> +#define A_VI_ERR_MAP 0x14
>> _0?
>>> +#define A_VI_ERR_MAP_1 0x18
>>> +#define V_VI_ERR_MAP_NO_ERROR (0)
>>> +#define V_VI_ERR_MAP_ERROR (1)
>>> +
>> same SMMU_CMDQV_VI_ING_MASK0/1
>>> +#define A_VI_INT_MASK 0x1c
>>> +#define A_VI_INT_MASK_1 0x20
>>> +#define V_VI_INT_MASK_NOT_MASKED (0)
>>> +#define V_VI_INT_MASK_MASKED (1)
>>> +
>> SMMU_CMDQV_CMDQ_ERR_MAP0-3
>>> +#define A_CMDQ_ERR_MAP 0x24
>>> +#define A_CMDQ_ERR_MAP_1 0x28
>>> +#define A_CMDQ_ERR_MAP_2 0x2c
>>> +#define A_CMDQ_ERR_MAP_3 0x

Re: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global CMDQV registers

2026-03-11 Thread Eric Auger
Hi Shameer,

On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> From: Nicolin Chen 
>
> Tegra241 CMDQV defines a set of global control and status registers
> used to configure virtual command queue allocation and interrupt
> behavior.
>
> Add read/write emulation for the global CMDQV register page
> (offset 0x0), backed by a simple register cache. This includes
> CONFIG, PARAM, STATUS, VI error and interrupt maps, CMDQ allocation
> map and the VINTF0 related registers defined in the global CMDQV
> register space.
This is called CMDQ-V Config within [CMDQV_BASE, CMDQV-CMDQ_BASE] in the
spec.

Precise we only support a single VINTF0 and VINTF1-63 are not supported.
>
> Signed-off-by: Nicolin Chen 
> Signed-off-by: Shameer Kolothum 
> ---
>  hw/arm/tegra241-cmdqv.h | 108 +++
>  hw/arm/tegra241-cmdqv.c | 121 +++-
>  2 files changed, 228 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> index 46aa9e8a9f..50bcecee9d 100644
> --- a/hw/arm/tegra241-cmdqv.h
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -10,6 +10,9 @@
>  #ifndef HW_ARM_TEGRA241_CMDQV_H
>  #define HW_ARM_TEGRA241_CMDQV_H
>  
> +#include "hw/core/registerfields.h"
> +#include "smmuv3-accel.h"
> +
>  #define TEGRA241_CMDQV_VERSION 1
>  #define TEGRA241_CMDQV_NUM_CMDQ_LOG2   1
>  #define TEGRA241_CMDQV_MAX_CMDQ(1U << 
> TEGRA241_CMDQV_NUM_CMDQ_LOG2)
> @@ -31,8 +34,113 @@ typedef struct Tegra241CMDQV {
>  SMMUv3AccelState *s_accel;
>  MemoryRegion mmio_cmdqv;
>  qemu_irq irq;
> +
> +/* Register Cache */
> +uint32_t config;
> +uint32_t param;
> +uint32_t status;
> +uint32_t vi_err_map[2];
> +uint32_t vi_int_mask[2];
> +uint32_t cmdq_err_map[4];
> +uint32_t cmdq_alloc_map[TEGRA241_CMDQV_MAX_CMDQ];
> +uint32_t vintf_config;
> +uint32_t vintf_status;
> +uint32_t vintf_sid_match[16];
> +uint32_t vintf_sid_replace[16];
> +uint32_t vintf_cmdq_err_map[4];
>  } Tegra241CMDQV;
>  
> +/* Global CMDQV MMIO registers (offset 0x0) */
> +REG32(CONFIG, 0x0)
> +FIELD(CONFIG, CMDQV_EN, 0, 1)
> +FIELD(CONFIG, CMDQV_PER_CMD_OFFSET, 1, 3)
> +FIELD(CONFIG, CMDQ_MAX_CLK_BATCH, 4, 8)
> +FIELD(CONFIG, CMDQ_MAX_CMD_BATCH, 12, 8)
> +FIELD(CONFIG, CONS_DRAM_EN, 20, 1)
> +
> +REG32(PARAM, 0x4)
> +FIELD(PARAM, CMDQV_VER, 0, 4)
> +FIELD(PARAM, CMDQV_NUM_CMDQ_LOG2, 4, 4)
> +FIELD(PARAM, CMDQV_NUM_VM_LOG2, 8, 4)
> +FIELD(PARAM, CMDQV_NUM_SID_PER_VM_LOG2, 12, 4)
> +
> +REG32(STATUS, 0x8)
> +FIELD(STATUS, CMDQV_ENABLED, 0, 1)
> +
> +#define A_VI_ERR_MAP 0x14
> +#define A_VI_ERR_MAP_1 0x18
> +#define V_VI_ERR_MAP_NO_ERROR (0)
> +#define V_VI_ERR_MAP_ERROR (1)
> +
> +#define A_VI_INT_MASK 0x1c
> +#define A_VI_INT_MASK_1 0x20
> +#define V_VI_INT_MASK_NOT_MASKED (0)
> +#define V_VI_INT_MASK_MASKED (1)
> +
> +#define A_CMDQ_ERR_MAP 0x24
> +#define A_CMDQ_ERR_MAP_1 0x28
> +#define A_CMDQ_ERR_MAP_2 0x2c
> +#define A_CMDQ_ERR_MAP_3 0x30
> +
> +/* i = [0, 1] */
> +#define A_CMDQ_ALLOC_MAP_(i) \
> +REG32(CMDQ_ALLOC_MAP_##i, 0x200 + i * 4) \
> +FIELD(CMDQ_ALLOC_MAP_##i, ALLOC, 0, 1)   \
> +FIELD(CMDQ_ALLOC_MAP_##i, LVCMDQ, 1, 7)  \
> +FIELD(CMDQ_ALLOC_MAP_##i, VIRT_INTF_INDX, 15, 6)
> +
> +A_CMDQ_ALLOC_MAP_(0)
> +A_CMDQ_ALLOC_MAP_(1)
> +
> +
> +/* i = [0, 0] */
> +#define A_VINTFi_CONFIG(i)   \
> +REG32(VINTF##i##_CONFIG, 0x1000 + i * 0x100) \
> +FIELD(VINTF##i##_CONFIG, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_CONFIG, VMID, 1, 16)\
> +FIELD(VINTF##i##_CONFIG, HYP_OWN, 17, 1)
> +
> +A_VINTFi_CONFIG(0)
> +
> +#define A_VINTFi_STATUS(i)   \
> +REG32(VINTF##i##_STATUS, 0x1004 + i * 0x100) \
> +FIELD(VINTF##i##_STATUS, ENABLE_OK, 0, 1)\
> +FIELD(VINTF##i##_STATUS, STATUS, 1, 3)   \
> +FIELD(VINTF##i##_STATUS, VI_NUM_LVCMDQ, 16, 8)
> +
> +A_VINTFi_STATUS(0)
> +
> +#define V_VINTF_STATUS_NO_ERROR (0 << 1)
> +#define V_VINTF_STATUS_VCMDQ_EROR (1 << 1)
> +
> +/* i = [0, 0], j = [0, 15] */
> +#define A_VINTFi_SID_MATCH_(i, j)   \
> +REG32(VINTF##i##_SID_MATCH_##j, 0x1040 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_SID_MATCH_##j, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_SID_MATCH_##j, VIRT_SID, 1, 20)
> +
> +A_VINTFi_SID_MATCH_(0, 0)
> +/* Omitting [0][1~14] as not being directly called */
> +A_VINTFi_SID_MATCH_(0, 15)
> +
> +/* i = [0, 0], j = [0, 15] */
> +#define A_VINTFi_SID_REPLACE_(i, j)   \
> +REG32(VINTF##i##_SID_REPLACE_##j, 0x1080 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_SID_REPLACE_##j, PHYS_SID, 0, 19)
> +
> +A_VINTFi_SID_REPLACE_(0, 0)
> +/* Omitting [0][1~14] as not being directly called */
> +A_VINTFi_SID_REPLACE_(0, 15)
> +
> +/* i = [0, 0], j = [0, 3] */
> +#define A_VINTFi_LVCMDQ_ERR_MAP_(i, j)   \
> +REG32(VINTF##i##_LVCMDQ_ERR_MAP_##j, 0x10c0 + j * 4 + i *

RE: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global CMDQV registers

2026-03-10 Thread Shameer Kolothum Thodi
Hi Eric,

> -Original Message-
> From: Eric Auger 
> Sent: 09 March 2026 16:33
> To: Shameer Kolothum Thodi ; qemu-
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; Nicolin
> Chen ; Nathan Chen ; Matt
> Ochs ; Jiandi An ; Jason Gunthorpe
> ; [email protected];
> [email protected]; [email protected]; Krishnakant Jaju
> ; [email protected]
> Subject: Re: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global
> CMDQV registers
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Shameer,
> 
> On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> > From: Nicolin Chen 
> >
> > Tegra241 CMDQV defines a set of global control and status registers
> > used to configure virtual command queue allocation and interrupt
> > behavior.
> >
> > Add read/write emulation for the global CMDQV register page
> > (offset 0x0), backed by a simple register cache. This includes
> > CONFIG, PARAM, STATUS, VI error and interrupt maps, CMDQ allocation
> > map and the VINTF0 related registers defined in the global CMDQV
> > register space.
> >
> > Signed-off-by: Nicolin Chen 
> > Signed-off-by: Shameer Kolothum 
> > ---
> >  hw/arm/tegra241-cmdqv.h | 108
> +++
> >  hw/arm/tegra241-cmdqv.c | 121
> +++-
> >  2 files changed, 228 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> > index 46aa9e8a9f..50bcecee9d 100644
> > --- a/hw/arm/tegra241-cmdqv.h
> > +++ b/hw/arm/tegra241-cmdqv.h
> > @@ -10,6 +10,9 @@
> >  #ifndef HW_ARM_TEGRA241_CMDQV_H
> >  #define HW_ARM_TEGRA241_CMDQV_H
> >
> > +#include "hw/core/registerfields.h"
> > +#include "smmuv3-accel.h"
> > +
> >  #define TEGRA241_CMDQV_VERSION 1
> >  #define TEGRA241_CMDQV_NUM_CMDQ_LOG2   1
> >  #define TEGRA241_CMDQV_MAX_CMDQ(1U <<
> TEGRA241_CMDQV_NUM_CMDQ_LOG2)
> > @@ -31,8 +34,113 @@ typedef struct Tegra241CMDQV {
> >  SMMUv3AccelState *s_accel;
> >  MemoryRegion mmio_cmdqv;
> >  qemu_irq irq;
> > +
> > +/* Register Cache */
> > +uint32_t config;
> > +uint32_t param;
> > +uint32_t status;
> > +uint32_t vi_err_map[2];
> > +uint32_t vi_int_mask[2];
> > +uint32_t cmdq_err_map[4];
> > +uint32_t cmdq_alloc_map[TEGRA241_CMDQV_MAX_CMDQ];
> > +uint32_t vintf_config;
> > +uint32_t vintf_status;
> > +uint32_t vintf_sid_match[16];
> > +uint32_t vintf_sid_replace[16];
> > +uint32_t vintf_cmdq_err_map[4];
> >  } Tegra241CMDQV;
> >
> > +/* Global CMDQV MMIO registers (offset 0x0) */
> > +REG32(CONFIG, 0x0)
> > +FIELD(CONFIG, CMDQV_EN, 0, 1)
> > +FIELD(CONFIG, CMDQV_PER_CMD_OFFSET, 1, 3)
> > +FIELD(CONFIG, CMDQ_MAX_CLK_BATCH, 4, 8)
> > +FIELD(CONFIG, CMDQ_MAX_CMD_BATCH, 12, 8)
> > +FIELD(CONFIG, CONS_DRAM_EN, 20, 1)
> > +
> > +REG32(PARAM, 0x4)
> > +FIELD(PARAM, CMDQV_VER, 0, 4)
> > +FIELD(PARAM, CMDQV_NUM_CMDQ_LOG2, 4, 4)
> > +FIELD(PARAM, CMDQV_NUM_VM_LOG2, 8, 4)
> this is called CMDQV_NUM_VI_LOG2 in the spec I have access to
> VI = virtual interface
> I guess there is 1-1 mapping vetween VI and VM but I would keep spec
> naming
> > +FIELD(PARAM, CMDQV_NUM_SID_PER_VM_LOG2, 12, 4)
> same here s/VM/VI
> > +
> > +REG32(STATUS, 0x8)
> > +FIELD(STATUS, CMDQV_ENABLED, 0, 1)
> > +
> I would add "SMMU_CMDQV_VI_ERR_MAP_0/1 definitions"
> > +#define A_VI_ERR_MAP 0x14
> _0?
> > +#define A_VI_ERR_MAP_1 0x18
> > +#define V_VI_ERR_MAP_NO_ERROR (0)
> > +#define V_VI_ERR_MAP_ERROR (1)
> > +
> same SMMU_CMDQV_VI_ING_MASK0/1
> > +#define A_VI_INT_MASK 0x1c
> > +#define A_VI_INT_MASK_1 0x20
> > +#define V_VI_INT_MASK_NOT_MASKED (0)
> > +#define V_VI_INT_MASK_MASKED (1)
> > +
> SMMU_CMDQV_CMDQ_ERR_MAP0-3
> > +#define A_CMDQ_ERR_MAP 0x24
> > +#define A_CMDQ_ERR_MAP_1 0x28
> > +#define A_CMDQ_ERR_MAP_2 0x2c
> > +#define A_CMDQ_ERR_MAP_3 0x30
> > +
> > +/* i = [0, 1] */
> 
> > +#define A_CMDQ_ALLOC_MAP_(i)
> why A_? Since you rely on the REG macros, A_ should prefix the address
> offset
> Can't we call that SMMU_CMDQV_CMDQ_ALLOC_MAP_() as it is refered to in
> the spec?

I will incorporate all suggestions related to naming etc and will make it same 
as
used in the spec.

> >   \
> > +REG

Re: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global CMDQV registers

2026-03-09 Thread Eric Auger



On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> From: Nicolin Chen 
>
> Tegra241 CMDQV defines a set of global control and status registers
> used to configure virtual command queue allocation and interrupt
> behavior.
>
> Add read/write emulation for the global CMDQV register page
> (offset 0x0), backed by a simple register cache. This includes
> CONFIG, PARAM, STATUS, VI error and interrupt maps, CMDQ allocation
> map and the VINTF0 related registers defined in the global CMDQV
> register space.
>
> Signed-off-by: Nicolin Chen 
> Signed-off-by: Shameer Kolothum 
> ---
>  hw/arm/tegra241-cmdqv.h | 108 +++
>  hw/arm/tegra241-cmdqv.c | 121 +++-
>  2 files changed, 228 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> index 46aa9e8a9f..50bcecee9d 100644
> --- a/hw/arm/tegra241-cmdqv.h
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -10,6 +10,9 @@
>  #ifndef HW_ARM_TEGRA241_CMDQV_H
>  #define HW_ARM_TEGRA241_CMDQV_H
>  
> +#include "hw/core/registerfields.h"
> +#include "smmuv3-accel.h"
> +
>  #define TEGRA241_CMDQV_VERSION 1
>  #define TEGRA241_CMDQV_NUM_CMDQ_LOG2   1
>  #define TEGRA241_CMDQV_MAX_CMDQ(1U << 
> TEGRA241_CMDQV_NUM_CMDQ_LOG2)
> @@ -31,8 +34,113 @@ typedef struct Tegra241CMDQV {
>  SMMUv3AccelState *s_accel;
>  MemoryRegion mmio_cmdqv;
>  qemu_irq irq;
> +
> +/* Register Cache */
> +uint32_t config;
> +uint32_t param;
> +uint32_t status;
> +uint32_t vi_err_map[2];
> +uint32_t vi_int_mask[2];
> +uint32_t cmdq_err_map[4];
> +uint32_t cmdq_alloc_map[TEGRA241_CMDQV_MAX_CMDQ];
> +uint32_t vintf_config;
> +uint32_t vintf_status;
> +uint32_t vintf_sid_match[16];
> +uint32_t vintf_sid_replace[16];
> +uint32_t vintf_cmdq_err_map[4];
>  } Tegra241CMDQV;
>  
> +/* Global CMDQV MMIO registers (offset 0x0) */
> +REG32(CONFIG, 0x0)
> +FIELD(CONFIG, CMDQV_EN, 0, 1)
> +FIELD(CONFIG, CMDQV_PER_CMD_OFFSET, 1, 3)
> +FIELD(CONFIG, CMDQ_MAX_CLK_BATCH, 4, 8)
> +FIELD(CONFIG, CMDQ_MAX_CMD_BATCH, 12, 8)
> +FIELD(CONFIG, CONS_DRAM_EN, 20, 1)
> +
> +REG32(PARAM, 0x4)
> +FIELD(PARAM, CMDQV_VER, 0, 4)
> +FIELD(PARAM, CMDQV_NUM_CMDQ_LOG2, 4, 4)
> +FIELD(PARAM, CMDQV_NUM_VM_LOG2, 8, 4)
> +FIELD(PARAM, CMDQV_NUM_SID_PER_VM_LOG2, 12, 4)
> +
> +REG32(STATUS, 0x8)
> +FIELD(STATUS, CMDQV_ENABLED, 0, 1)
> +
> +#define A_VI_ERR_MAP 0x14
> +#define A_VI_ERR_MAP_1 0x18
> +#define V_VI_ERR_MAP_NO_ERROR (0)
> +#define V_VI_ERR_MAP_ERROR (1)
> +
> +#define A_VI_INT_MASK 0x1c
> +#define A_VI_INT_MASK_1 0x20
> +#define V_VI_INT_MASK_NOT_MASKED (0)
> +#define V_VI_INT_MASK_MASKED (1)
> +
> +#define A_CMDQ_ERR_MAP 0x24
> +#define A_CMDQ_ERR_MAP_1 0x28
> +#define A_CMDQ_ERR_MAP_2 0x2c
> +#define A_CMDQ_ERR_MAP_3 0x30
> +
> +/* i = [0, 1] */
> +#define A_CMDQ_ALLOC_MAP_(i) \
> +REG32(CMDQ_ALLOC_MAP_##i, 0x200 + i * 4) \
> +FIELD(CMDQ_ALLOC_MAP_##i, ALLOC, 0, 1)   \
> +FIELD(CMDQ_ALLOC_MAP_##i, LVCMDQ, 1, 7)  \
> +FIELD(CMDQ_ALLOC_MAP_##i, VIRT_INTF_INDX, 15, 6)
> +
> +A_CMDQ_ALLOC_MAP_(0)
> +A_CMDQ_ALLOC_MAP_(1)
> +
> +
> +/* i = [0, 0] */
> +#define A_VINTFi_CONFIG(i)   \
> +REG32(VINTF##i##_CONFIG, 0x1000 + i * 0x100) \
> +FIELD(VINTF##i##_CONFIG, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_CONFIG, VMID, 1, 16)\
> +FIELD(VINTF##i##_CONFIG, HYP_OWN, 17, 1)
> +
> +A_VINTFi_CONFIG(0)
> +
> +#define A_VINTFi_STATUS(i)   \
> +REG32(VINTF##i##_STATUS, 0x1004 + i * 0x100) \
> +FIELD(VINTF##i##_STATUS, ENABLE_OK, 0, 1)\
> +FIELD(VINTF##i##_STATUS, STATUS, 1, 3)   \
> +FIELD(VINTF##i##_STATUS, VI_NUM_LVCMDQ, 16, 8)
> +
> +A_VINTFi_STATUS(0)
> +
> +#define V_VINTF_STATUS_NO_ERROR (0 << 1)
> +#define V_VINTF_STATUS_VCMDQ_EROR (1 << 1)
> +
> +/* i = [0, 0], j = [0, 15] */
> +#define A_VINTFi_SID_MATCH_(i, j)   \
> +REG32(VINTF##i##_SID_MATCH_##j, 0x1040 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_SID_MATCH_##j, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_SID_MATCH_##j, VIRT_SID, 1, 20)
> +
> +A_VINTFi_SID_MATCH_(0, 0)
> +/* Omitting [0][1~14] as not being directly called */
> +A_VINTFi_SID_MATCH_(0, 15)
> +
> +/* i = [0, 0], j = [0, 15] */
> +#define A_VINTFi_SID_REPLACE_(i, j)   \
> +REG32(VINTF##i##_SID_REPLACE_##j, 0x1080 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_SID_REPLACE_##j, PHYS_SID, 0, 19)
> +
> +A_VINTFi_SID_REPLACE_(0, 0)
> +/* Omitting [0][1~14] as not being directly called */
> +A_VINTFi_SID_REPLACE_(0, 15)
> +
> +/* i = [0, 0], j = [0, 3] */
> +#define A_VINTFi_LVCMDQ_ERR_MAP_(i, j)   \
> +REG32(VINTF##i##_LVCMDQ_ERR_MAP_##j, 0x10c0 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_LVCMDQ_ERR_MAP_##j, LVCMDQ_ERR_MAP, 0, 32)
> +
> +A_VINTFi_LVCMDQ_ERR_MAP_(0, 0)
> +/* Omitting [0][1~2] as not being directly ca

Re: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global CMDQV registers

2026-03-09 Thread Eric Auger



On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> From: Nicolin Chen 
>
> Tegra241 CMDQV defines a set of global control and status registers
> used to configure virtual command queue allocation and interrupt
> behavior.
>
> Add read/write emulation for the global CMDQV register page
In the spec this is named CMDQ-V Config. This 64kB page contains CMDQ-V
Config + VI-n Configs

Eric
> (offset 0x0), backed by a simple register cache. This includes
> CONFIG, PARAM, STATUS, VI error and interrupt maps, CMDQ allocation
> map and the VINTF0 related registers defined in the global CMDQV
> register space.
>
> Signed-off-by: Nicolin Chen 
> Signed-off-by: Shameer Kolothum 
> ---
>  hw/arm/tegra241-cmdqv.h | 108 +++
>  hw/arm/tegra241-cmdqv.c | 121 +++-
>  2 files changed, 228 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> index 46aa9e8a9f..50bcecee9d 100644
> --- a/hw/arm/tegra241-cmdqv.h
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -10,6 +10,9 @@
>  #ifndef HW_ARM_TEGRA241_CMDQV_H
>  #define HW_ARM_TEGRA241_CMDQV_H
>  
> +#include "hw/core/registerfields.h"
> +#include "smmuv3-accel.h"
> +
>  #define TEGRA241_CMDQV_VERSION 1
>  #define TEGRA241_CMDQV_NUM_CMDQ_LOG2   1
>  #define TEGRA241_CMDQV_MAX_CMDQ(1U << 
> TEGRA241_CMDQV_NUM_CMDQ_LOG2)
> @@ -31,8 +34,113 @@ typedef struct Tegra241CMDQV {
>  SMMUv3AccelState *s_accel;
>  MemoryRegion mmio_cmdqv;
>  qemu_irq irq;
> +
> +/* Register Cache */
> +uint32_t config;
> +uint32_t param;
> +uint32_t status;
> +uint32_t vi_err_map[2];
> +uint32_t vi_int_mask[2];
> +uint32_t cmdq_err_map[4];
> +uint32_t cmdq_alloc_map[TEGRA241_CMDQV_MAX_CMDQ];
> +uint32_t vintf_config;
> +uint32_t vintf_status;
> +uint32_t vintf_sid_match[16];
> +uint32_t vintf_sid_replace[16];
> +uint32_t vintf_cmdq_err_map[4];
>  } Tegra241CMDQV;
>  
> +/* Global CMDQV MMIO registers (offset 0x0) */
> +REG32(CONFIG, 0x0)
> +FIELD(CONFIG, CMDQV_EN, 0, 1)
> +FIELD(CONFIG, CMDQV_PER_CMD_OFFSET, 1, 3)
> +FIELD(CONFIG, CMDQ_MAX_CLK_BATCH, 4, 8)
> +FIELD(CONFIG, CMDQ_MAX_CMD_BATCH, 12, 8)
> +FIELD(CONFIG, CONS_DRAM_EN, 20, 1)
> +
> +REG32(PARAM, 0x4)
> +FIELD(PARAM, CMDQV_VER, 0, 4)
> +FIELD(PARAM, CMDQV_NUM_CMDQ_LOG2, 4, 4)
> +FIELD(PARAM, CMDQV_NUM_VM_LOG2, 8, 4)
> +FIELD(PARAM, CMDQV_NUM_SID_PER_VM_LOG2, 12, 4)
> +
> +REG32(STATUS, 0x8)
> +FIELD(STATUS, CMDQV_ENABLED, 0, 1)
> +
> +#define A_VI_ERR_MAP 0x14
> +#define A_VI_ERR_MAP_1 0x18
> +#define V_VI_ERR_MAP_NO_ERROR (0)
> +#define V_VI_ERR_MAP_ERROR (1)
> +
> +#define A_VI_INT_MASK 0x1c
> +#define A_VI_INT_MASK_1 0x20
> +#define V_VI_INT_MASK_NOT_MASKED (0)
> +#define V_VI_INT_MASK_MASKED (1)
> +
> +#define A_CMDQ_ERR_MAP 0x24
> +#define A_CMDQ_ERR_MAP_1 0x28
> +#define A_CMDQ_ERR_MAP_2 0x2c
> +#define A_CMDQ_ERR_MAP_3 0x30
> +
> +/* i = [0, 1] */
> +#define A_CMDQ_ALLOC_MAP_(i) \
> +REG32(CMDQ_ALLOC_MAP_##i, 0x200 + i * 4) \
> +FIELD(CMDQ_ALLOC_MAP_##i, ALLOC, 0, 1)   \
> +FIELD(CMDQ_ALLOC_MAP_##i, LVCMDQ, 1, 7)  \
> +FIELD(CMDQ_ALLOC_MAP_##i, VIRT_INTF_INDX, 15, 6)
> +
> +A_CMDQ_ALLOC_MAP_(0)
> +A_CMDQ_ALLOC_MAP_(1)
> +
> +
> +/* i = [0, 0] */
> +#define A_VINTFi_CONFIG(i)   \
> +REG32(VINTF##i##_CONFIG, 0x1000 + i * 0x100) \
> +FIELD(VINTF##i##_CONFIG, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_CONFIG, VMID, 1, 16)\
> +FIELD(VINTF##i##_CONFIG, HYP_OWN, 17, 1)
> +
> +A_VINTFi_CONFIG(0)
> +
> +#define A_VINTFi_STATUS(i)   \
> +REG32(VINTF##i##_STATUS, 0x1004 + i * 0x100) \
> +FIELD(VINTF##i##_STATUS, ENABLE_OK, 0, 1)\
> +FIELD(VINTF##i##_STATUS, STATUS, 1, 3)   \
> +FIELD(VINTF##i##_STATUS, VI_NUM_LVCMDQ, 16, 8)
> +
> +A_VINTFi_STATUS(0)
> +
> +#define V_VINTF_STATUS_NO_ERROR (0 << 1)
> +#define V_VINTF_STATUS_VCMDQ_EROR (1 << 1)
> +
> +/* i = [0, 0], j = [0, 15] */
> +#define A_VINTFi_SID_MATCH_(i, j)   \
> +REG32(VINTF##i##_SID_MATCH_##j, 0x1040 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_SID_MATCH_##j, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_SID_MATCH_##j, VIRT_SID, 1, 20)
> +
> +A_VINTFi_SID_MATCH_(0, 0)
> +/* Omitting [0][1~14] as not being directly called */
> +A_VINTFi_SID_MATCH_(0, 15)
> +
> +/* i = [0, 0], j = [0, 15] */
> +#define A_VINTFi_SID_REPLACE_(i, j)   \
> +REG32(VINTF##i##_SID_REPLACE_##j, 0x1080 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_SID_REPLACE_##j, PHYS_SID, 0, 19)
> +
> +A_VINTFi_SID_REPLACE_(0, 0)
> +/* Omitting [0][1~14] as not being directly called */
> +A_VINTFi_SID_REPLACE_(0, 15)
> +
> +/* i = [0, 0], j = [0, 3] */
> +#define A_VINTFi_LVCMDQ_ERR_MAP_(i, j)   \
> +REG32(VINTF##i##_LVCMDQ_ERR_MAP_##j, 0x10c0 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_LVCMDQ_ERR_MAP_##j, LVCMDQ_ER

Re: [PATCH v3 14/32] hw/arm/tegra241-cmdqv: Emulate global CMDQV registers

2026-03-09 Thread Eric Auger
Hi Shameer,

On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> From: Nicolin Chen 
>
> Tegra241 CMDQV defines a set of global control and status registers
> used to configure virtual command queue allocation and interrupt
> behavior.
>
> Add read/write emulation for the global CMDQV register page
> (offset 0x0), backed by a simple register cache. This includes
> CONFIG, PARAM, STATUS, VI error and interrupt maps, CMDQ allocation
> map and the VINTF0 related registers defined in the global CMDQV
> register space.
>
> Signed-off-by: Nicolin Chen 
> Signed-off-by: Shameer Kolothum 
> ---
>  hw/arm/tegra241-cmdqv.h | 108 +++
>  hw/arm/tegra241-cmdqv.c | 121 +++-
>  2 files changed, 228 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> index 46aa9e8a9f..50bcecee9d 100644
> --- a/hw/arm/tegra241-cmdqv.h
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -10,6 +10,9 @@
>  #ifndef HW_ARM_TEGRA241_CMDQV_H
>  #define HW_ARM_TEGRA241_CMDQV_H
>  
> +#include "hw/core/registerfields.h"
> +#include "smmuv3-accel.h"
> +
>  #define TEGRA241_CMDQV_VERSION 1
>  #define TEGRA241_CMDQV_NUM_CMDQ_LOG2   1
>  #define TEGRA241_CMDQV_MAX_CMDQ(1U << 
> TEGRA241_CMDQV_NUM_CMDQ_LOG2)
> @@ -31,8 +34,113 @@ typedef struct Tegra241CMDQV {
>  SMMUv3AccelState *s_accel;
>  MemoryRegion mmio_cmdqv;
>  qemu_irq irq;
> +
> +/* Register Cache */
> +uint32_t config;
> +uint32_t param;
> +uint32_t status;
> +uint32_t vi_err_map[2];
> +uint32_t vi_int_mask[2];
> +uint32_t cmdq_err_map[4];
> +uint32_t cmdq_alloc_map[TEGRA241_CMDQV_MAX_CMDQ];
> +uint32_t vintf_config;
> +uint32_t vintf_status;
> +uint32_t vintf_sid_match[16];
> +uint32_t vintf_sid_replace[16];
> +uint32_t vintf_cmdq_err_map[4];
>  } Tegra241CMDQV;
>  
> +/* Global CMDQV MMIO registers (offset 0x0) */
> +REG32(CONFIG, 0x0)
> +FIELD(CONFIG, CMDQV_EN, 0, 1)
> +FIELD(CONFIG, CMDQV_PER_CMD_OFFSET, 1, 3)
> +FIELD(CONFIG, CMDQ_MAX_CLK_BATCH, 4, 8)
> +FIELD(CONFIG, CMDQ_MAX_CMD_BATCH, 12, 8)
> +FIELD(CONFIG, CONS_DRAM_EN, 20, 1)
> +
> +REG32(PARAM, 0x4)
> +FIELD(PARAM, CMDQV_VER, 0, 4)
> +FIELD(PARAM, CMDQV_NUM_CMDQ_LOG2, 4, 4)
> +FIELD(PARAM, CMDQV_NUM_VM_LOG2, 8, 4)
this is called CMDQV_NUM_VI_LOG2 in the spec I have access to
VI = virtual interface
I guess there is 1-1 mapping vetween VI and VM but I would keep spec naming
> +FIELD(PARAM, CMDQV_NUM_SID_PER_VM_LOG2, 12, 4)
same here s/VM/VI
> +
> +REG32(STATUS, 0x8)
> +FIELD(STATUS, CMDQV_ENABLED, 0, 1)
> +
I would add "SMMU_CMDQV_VI_ERR_MAP_0/1 definitions"
> +#define A_VI_ERR_MAP 0x14
_0?
> +#define A_VI_ERR_MAP_1 0x18
> +#define V_VI_ERR_MAP_NO_ERROR (0)
> +#define V_VI_ERR_MAP_ERROR (1)
> +
same SMMU_CMDQV_VI_ING_MASK0/1
> +#define A_VI_INT_MASK 0x1c
> +#define A_VI_INT_MASK_1 0x20
> +#define V_VI_INT_MASK_NOT_MASKED (0)
> +#define V_VI_INT_MASK_MASKED (1)
> +
SMMU_CMDQV_CMDQ_ERR_MAP0-3
> +#define A_CMDQ_ERR_MAP 0x24
> +#define A_CMDQ_ERR_MAP_1 0x28
> +#define A_CMDQ_ERR_MAP_2 0x2c
> +#define A_CMDQ_ERR_MAP_3 0x30
> +
> +/* i = [0, 1] */

> +#define A_CMDQ_ALLOC_MAP_(i)   
why A_? Since you rely on the REG macros, A_ should prefix the address
offset
Can't we call that SMMU_CMDQV_CMDQ_ALLOC_MAP_() as it is refered to in
the spec?
>   \
> +REG32(CMDQ_ALLOC_MAP_##i, 0x200 + i * 4) \
> +FIELD(CMDQ_ALLOC_MAP_##i, ALLOC, 0, 1)   \
> +FIELD(CMDQ_ALLOC_MAP_##i, LVCMDQ, 1, 7)  \
> +FIELD(CMDQ_ALLOC_MAP_##i, VIRT_INTF_INDX, 15, 6)
> +
Please explain why we only expose 2 of those regs among 128?
> +A_CMDQ_ALLOC_MAP_(0)
> +A_CMDQ_ALLOC_MAP_(1)
> +
> +
> +/* i = [0, 0] */
> +#define A_VINTFi_CONFIG(i) 
again why don't we use the spec terminology
SMMU_CMDQV_VINTF0_CONFIG_0
>   \
> +REG32(VINTF##i##_CONFIG, 0x1000 + i * 0x100) \
> +FIELD(VINTF##i##_CONFIG, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_CONFIG, VMID, 1, 16)\
> +FIELD(VINTF##i##_CONFIG, HYP_OWN, 17, 1)
> +
> +A_VINTFi_CONFIG(0)
> +
> +#define A_VINTFi_STATUS(i)   \
> +REG32(VINTF##i##_STATUS, 0x1004 + i * 0x100) \
> +FIELD(VINTF##i##_STATUS, ENABLE_OK, 0, 1)\
> +FIELD(VINTF##i##_STATUS, STATUS, 1, 3)   \
> +FIELD(VINTF##i##_STATUS, VI_NUM_LVCMDQ, 16, 8)
> +
> +A_VINTFi_STATUS(0)
> +
> +#define V_VINTF_STATUS_NO_ERROR (0 << 1)
> +#define V_VINTF_STATUS_VCMDQ_EROR (1 << 1)
Nit: the spec also contains the typo but I would rather fix it in the code

Explicitly state we only expose 1 VINTF
> +
> +/* i = [0, 0], j = [0, 15] */
does that mean that we can have a max 16 SIDs per VM?
> +#define A_VINTFi_SID_MATCH_(i, j) 
if matched vi, I would prefer vi
>   \
> +REG32(VINTF##i##_SID_MATCH_##j, 0x1040 + j * 4 + i * 0x100) \
> +FIELD(VINTF##i##_SID_MATCH_##j, ENABLE, 0, 1)   \
> +FIELD(VINTF##i##_SID_MATCH_##j, VIRT_SID, 1, 20)
> +
> +A_VI