Hi Shameer,
On 5/19/26 12:36 PM, Shameer Kolothum wrote:
> From: Nicolin Chen <[email protected]>
>
> This is the write side counterpart of the VCMDQ read emulation. Add
> write handling for both the direct VCMDQ aperture and the VINTF
> logical aperture using the same index decoding and VINTF-to-VCMDQ
> translation logic as the read path.
>
> VINTF aperture writes are translated to their direct-aperture
> equivalent and update the same cached state. Page 1 registers
> (BASE, CONS_INDX_BASE) always update the cache. Once
> IOMMU_HW_QUEUE_ALLOC and viommu_mmap are wired up in a subsequent
> patch, Page 0 register writes will be forwarded to the hardware-
> backed mmap'd page.
>
> Ignore VCMDQ BASE writes if the VCMDQ is already enabled.
>
> Signed-off-by: Nicolin Chen <[email protected]>
> Co-developed-by: Shameer Kolothum <[email protected]>
> Signed-off-by: Shameer Kolothum <[email protected]>
> ---
> hw/arm/tegra241-cmdqv.c | 164 +++++++++++++++++++++++++++++++++++++++-
> hw/arm/trace-events | 2 +
> 2 files changed, 164 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> index f859126ad6..5d2d9d2e26 100644
> --- a/hw/arm/tegra241-cmdqv.c
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -16,6 +16,16 @@
> #include "tegra241-cmdqv.h"
> #include "trace.h"
>
> +/*
> + * Returns true if the per-VCMDQ CMDQ_EN_OK bit is set.
> + */
> +static bool tegra241_vcmdq_enabled(Tegra241CMDQV *cmdqv, int index)
> +{
> + uint32_t status = cmdqv->vcmdq_status[index];
> +
> + return status & R_VCMDQ0_STATUS_CMDQ_EN_OK_MASK;
> +}
> +
> /*
> * Read a VCMDQ Page 0 register (control/status) using VCMDQ0_* offsets.
> *
> @@ -116,6 +126,108 @@ static uint64_t
> tegra241_cmdqv_config_vintf_read(Tegra241CMDQV *cmdqv,
> }
> }
>
> +/*
> + * Write a VCMDQ Page 0 register (control/status) using VCMDQ0_* offsets.
> + *
> + * The caller normalizes the MMIO offset such that @offset0 always refers
> + * to a VCMDQ0_* register, while @index selects the VCMDQ instance.
> + *
> + * Page 0 registers are all 32-bit; this helper is only called for 4-byte
> + * writes.
> + */
> +static void tegra241_cmdqv_write_vcmdq_page0(Tegra241CMDQV *cmdqv,
> + hwaddr offset0, int index,
> + uint32_t value)
> +{
> + switch (offset0) {
> + case A_VCMDQ0_CONS_INDX:
> + cmdqv->vcmdq_cons_indx[index] = value;
> + break;
> + case A_VCMDQ0_PROD_INDX:
> + cmdqv->vcmdq_prod_indx[index] = value;
so there is no cmd consumption triggered by an increment of the producer
index as opposed to std smmu emulation. This shall at least be explained
in the commit msg and maybe in the code, + the justification. It is a
bit strange to emulate the MMIO accesses while not doing anything
functional out of them.
> + break;
> + case A_VCMDQ0_CONFIG:
> + if (value & R_VCMDQ0_CONFIG_CMDQ_EN_MASK) {
> + cmdqv->vcmdq_status[index] |= R_VCMDQ0_STATUS_CMDQ_EN_OK_MASK;
> + } else {
> + cmdqv->vcmdq_status[index] &= ~R_VCMDQ0_STATUS_CMDQ_EN_OK_MASK;
> + }
> + cmdqv->vcmdq_config[index] = value;
> + break;
> + case A_VCMDQ0_GERRORN:
> + cmdqv->vcmdq_gerrorn[index] = value;
shouldn't it do something functional besides storing the value
on std SMMU we resume consuming cmds. interaction with interrupts,
CONS_INDX?
> + break;
> + default:
> + qemu_log_mask(LOG_UNIMP,
> + "%s unhandled write access at 0x%" PRIx64 "\n",
> + __func__, offset0);
> + }
> + trace_tegra241_cmdqv_write_vcmdq_page0(index, offset0, value);
> +}
> +
> +/*
> + * Write a VCMDQ Page 1 register (base / DRAM address) - 4-byte access.
> + */
> +static void tegra241_cmdqv_write_vcmdq_page1(Tegra241CMDQV *cmdqv,
> + hwaddr offset0, int index,
> + uint32_t value)
> +{
> + switch (offset0) {
> + case A_VCMDQ0_BASE_L:
> + if (tegra241_vcmdq_enabled(cmdqv, index)) {
> + return;
> + }
> + cmdqv->vcmdq_base[index] =
> + deposit64(cmdqv->vcmdq_base[index], 0, 32, value);
> + break;
> + case A_VCMDQ0_BASE_H:
> + if (tegra241_vcmdq_enabled(cmdqv, index)) {
where is it documented in the spec? I don't see this restriction in the
reg doc. Then "Enabling the virtual CMDQ" paragraph describes a std
setup but does not say - afaics- that it should be ignored -
worth commenting
> + return;
> + }
> + cmdqv->vcmdq_base[index] =
> + deposit64(cmdqv->vcmdq_base[index], 32, 32, value);
> + break;
> + case A_VCMDQ0_CONS_INDX_BASE_DRAM_L:
> + cmdqv->vcmdq_cons_indx_base[index] =
> + deposit64(cmdqv->vcmdq_cons_indx_base[index], 0, 32, value);
> + break;
> + case A_VCMDQ0_CONS_INDX_BASE_DRAM_H:
> + cmdqv->vcmdq_cons_indx_base[index] =
> + deposit64(cmdqv->vcmdq_cons_indx_base[index], 32, 32, value);
> + break;
> + default:
> + qemu_log_mask(LOG_UNIMP,
> + "%s unhandled write access at 0x%" PRIx64 "\n",
> + __func__, offset0);
> + }
> + trace_tegra241_cmdqv_write_vcmdq_page1(index, offset0, value);
> +}
> +
> +/*
> + * Write a VCMDQ Page 1 register - 8-byte access at BASE_L or DRAM_L.
> + */
> +static void tegra241_cmdqv_write_vcmdq_page1_64(Tegra241CMDQV *cmdqv,
> + hwaddr offset0, int index,
> + uint64_t value)
> +{
> + switch (offset0) {
> + case A_VCMDQ0_BASE_L:
> + if (tegra241_vcmdq_enabled(cmdqv, index)) {
> + return;
> + }
> + cmdqv->vcmdq_base[index] = value;
> + break;
> + case A_VCMDQ0_CONS_INDX_BASE_DRAM_L:
> + cmdqv->vcmdq_cons_indx_base[index] = value;
> + break;
> + default:
> + qemu_log_mask(LOG_UNIMP,
> + "%s unhandled 64-bit write at 0x%" PRIx64 "\n",
> + __func__, offset0);
> + }
> + trace_tegra241_cmdqv_write_vcmdq_page1(index, offset0, value);
> +}
> +
> static void tegra241_cmdqv_config_vintf_write(Tegra241CMDQV *cmdqv,
> hwaddr offset, uint64_t value)
> {
> @@ -235,6 +347,8 @@ out:
> static void tegra241_cmdqv_writel_mmio(Tegra241CMDQV *cmdqv, hwaddr offset,
> uint32_t value)
> {
> + int index;
> +
> switch (offset) {
> case A_CONFIG:
> cmdqv->config = value;
> @@ -253,6 +367,33 @@ static void tegra241_cmdqv_writel_mmio(Tegra241CMDQV
> *cmdqv, hwaddr offset,
> case A_VINTF0_CONFIG ... A_VINTF0_LVCMDQ_ERR_MAP_3:
> tegra241_cmdqv_config_vintf_write(cmdqv, offset, value);
> break;
> + case A_VI_VCMDQ0_CONS_INDX ... A_VI_VCMDQ1_GERRORN:
> + /*
> + * VINTF Page0 registers are hardware aliases of VCMDQ Page0
> registers.
> + * Translate the VINTF aperture offset to its VCMDQ Page0 equivalent
> + * and fall through to the Page0 dispatch below.
> + */
> + offset -= CMDQV_VINTF_PAGE0_BASE - CMDQV_VCMDQ_PAGE0_BASE;
> + QEMU_FALLTHROUGH;
> + case A_VCMDQ0_CONS_INDX ... A_VCMDQ1_GERRORN:
> + /*
> + * Decode a per-VCMDQ Page 0 access. Each VCMDQ occupies a
> + * CMDQV_VCMDQ_STRIDE-byte window; extract the index and normalize
> + * to the VCMDQ0_* offset before calling the Page 0 helper.
> + */
> + index = (offset - CMDQV_VCMDQ_PAGE0_BASE) / CMDQV_VCMDQ_STRIDE;
> + tegra241_cmdqv_write_vcmdq_page0(cmdqv,
> + offset - index * CMDQV_VCMDQ_STRIDE, index, value);
> + break;
> + case A_VI_VCMDQ0_BASE_L ... A_VI_VCMDQ1_CONS_INDX_BASE_DRAM_H:
> + /* Same VINTF-to-VCMDQ translation as VINTF Page0 case above. */
> + offset -= CMDQV_VINTF_PAGE1_BASE - CMDQV_VCMDQ_PAGE1_BASE;
> + QEMU_FALLTHROUGH;
> + case A_VCMDQ0_BASE_L ... A_VCMDQ1_CONS_INDX_BASE_DRAM_H:
> + index = (offset - CMDQV_VCMDQ_PAGE1_BASE) / CMDQV_VCMDQ_STRIDE;
> + tegra241_cmdqv_write_vcmdq_page1(cmdqv,
> + offset - index * CMDQV_VCMDQ_STRIDE, index, value);
> + break;
> default:
> qemu_log_mask(LOG_UNIMP, "%s unhandled write access at 0x%" PRIx64
> "\n",
> __func__, offset);
> @@ -260,14 +401,33 @@ static void tegra241_cmdqv_writel_mmio(Tegra241CMDQV
> *cmdqv, hwaddr offset,
> }
>
> /*
> - * 8-byte MMIO write handler.
> + * 8-byte MMIO write handler. Only Page 1 BASE / CONS_INDX_BASE_DRAM accept
> + * full 64-bit writes; other offsets are write-ignored.
> */
> static void tegra241_cmdqv_writell_mmio(Tegra241CMDQV *cmdqv, hwaddr offset,
> uint64_t value)
> {
> - qemu_log_mask(LOG_UNIMP,
> + int index;
> +
> + switch (offset) {
> + case A_VI_VCMDQ0_BASE_L ... A_VI_VCMDQ1_CONS_INDX_BASE_DRAM_H:
> + /*
> + * VINTF Page1 registers are hardware aliases of VCMDQ Page1
> registers.
> + * Translate the VINTF aperture offset to its VCMDQ Page1 equivalent
> + * and fall through to the Page1 dispatch below.
> + */
> + offset -= CMDQV_VINTF_PAGE1_BASE - CMDQV_VCMDQ_PAGE1_BASE;
> + QEMU_FALLTHROUGH;
> + case A_VCMDQ0_BASE_L ... A_VCMDQ1_CONS_INDX_BASE_DRAM_H:
> + index = (offset - CMDQV_VCMDQ_PAGE1_BASE) / CMDQV_VCMDQ_STRIDE;
> + tegra241_cmdqv_write_vcmdq_page1_64(cmdqv,
> + offset - index * CMDQV_VCMDQ_STRIDE, index, value);
> + break;
> + default:
> + qemu_log_mask(LOG_UNIMP,
> "%s unhandled 64-bit write at 0x%" PRIx64 " (WI)\n",
> __func__, offset);
> + }
> }
>
> static void tegra241_cmdqv_write_mmio(void *opaque, hwaddr offset,
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index 8c34a04b24..c4262bb2be 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -77,6 +77,8 @@ tegra241_cmdqv_read_mmio(uint64_t offset, uint64_t val,
> unsigned size) "offset:
> tegra241_cmdqv_write_mmio(uint64_t offset, uint64_t val, unsigned size)
> "offset: 0x%"PRIx64" val: 0x%"PRIx64" size: 0x%x"
> tegra241_cmdqv_read_vcmdq_page0(int index, uint64_t offset0, uint64_t val)
> "vcmdq[%d] page0 offset0: 0x%"PRIx64" val: 0x%"PRIx64
> tegra241_cmdqv_read_vcmdq_page1(int index, uint64_t offset0, uint64_t val)
> "vcmdq[%d] page1 offset0: 0x%"PRIx64" val: 0x%"PRIx64
> +tegra241_cmdqv_write_vcmdq_page0(int index, uint64_t offset0, uint64_t val)
> "vcmdq[%d] page0 offset0: 0x%"PRIx64" val: 0x%"PRIx64
> +tegra241_cmdqv_write_vcmdq_page1(int index, uint64_t offset0, uint64_t val)
> "vcmdq[%d] page1 offset0: 0x%"PRIx64" val: 0x%"PRIx64
>
> # strongarm.c
> strongarm_uart_update_parameters(const char *label, int speed, char parity,
> int data_bits, int stop_bits) "%s speed=%d parity=%c data=%d stop=%d"
Thanks
Eric
