Hi Eric,

On 2026/8/28 18:09, Eric Auger wrote:

On 8/13/26 6:25 PM, Tao Tang wrote:
Add access control for command queue and event queue related registers
to ensure they can only be modified under proper conditions.
please explain what is wrong first
OK.
For command queue (CMDQ):
- smmu_cmdq_disabled_stable(): checks CMDQ bit in CR0/CR0ACK
always assume CR0=CR0ACK?
I'll drop the redundant CR0ACK checks.
- smmu_cmdq_base_writable(): checks IDR1.QUEUES_PRESET==0 and CMDQ disabled

For event queue (EVTQ):
- smmu_eventq_disabled_stable(): checks EVTQ bit in CR0/CR0ACK
- smmu_eventq_base_writable(): checks IDR1.QUEUES_PRESET==0 and EVTQ disabled
...
- smmu_irq_cfg_present(): checks SMMU_IDR0.MSI for the Non-secure
   EVENTQ_IRQ_CFG register set and SMMU_S_IDR0.MSI for the Secure set
can't you simply assume MSI is not supported?

Sure. EVENTQ_IRQ_CFGx should be RES0 because of unsupported MSI as discussed for patch #16.

Additionally, QUEUES_PRESET checks will be dropped following the TABLES_PRESET simplification in patch #17.

- smmu_eventq_irq_cfg_writable(): additionally checks EVENTQ_IRQEN

Absent EVENTQ_IRQ_CFG registers read as RES0. Present registers retain
their programmed values for readback. The helper remains keyed by
SEC_SID so future Realm support can select SMMU_R_IDR0.MSI.

Additionally, mask reserved bits on writes using SMMU_QUEUE_BASE_RESERVED
for queue base registers and SMMU_EVENTQ_IRQ_CFG0_RESERVED for
EVENTQ_IRQ_CFG0.

Fixes: fae4be38b35d ("hw/arm/smmuv3: Implement MMIO write operations")
Signed-off-by: Tao Tang <[email protected]>
Reviewed-by: Pierrick Bouvier <[email protected]>
---
  hw/arm/smmuv3.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++--
210 changes lines -> Can't you split into 2a ll the more so the handling
is different from CMDQ and EVENTQ

I'll split it into 2 patches and describe these issues first in the commit messages.


Thanks,

Tao


Reply via email to