Re: [PATCH v3] iommu/tegra241-cmdqv: Reject a VCMDQ base above the 48-bit hardware limit

On 2026-08-06 1:47 pm, Will Deacon wrote:
> On Wed, Jul 29, 2026 at 03:03:29PM -0700, Nicolin Chen wrote:
>> tegra241_vcmdq_alloc_smmu_cmdq() allocates the VCMDQ buffer via the common
>> arm_smmu_init_one_queue(), which uses smmu->dev and its coherent DMA mask
>> of DMA_BIT_MASK(smmu->oas). An SMMUv3 queue base holds a 52-bit address in
>> Q_BASE_ADDR_MASK, but the VCMDQ_BASE register holds only 48 (VCMDQ_ADDR),
>> so the masked write "q_base = base_dma & VCMDQ_ADDR" silently drops bits 48
>> and up. The HW would then fetch its commands from a wrong memory location,
>> which likely raises VCMDQ errors.
>>
>> Real hardware always pairs an SMMU OAS with a matching VCMDQ address field,
>> so this cannot happen; it takes a VMM that gives a guest a mismatched OAS.
>> Nor can the guest rely on the VMM to catch it: an SMMU CMDQ base keeps bits
>> 48 and up in Q_BASE_ADDR_MASK, so a trapped CMDQ_BASE write still carries
>> them, while a VCMDQ base goes straight to the hardware and loses them.
>>
>> Reject a base_dma with bits set outside VCMDQ_ADDR, the way the user-VCMDQ
>> path already does for its base_addr_pa, failing the queue init rather than
>> silently truncating the base. Use dev_warn_once() rather than WARN_ON(), as
>> the OAS is VMM-controlled and a WARN_ON() would let it panic a guest booted
>> with panic_on_warn.
>>
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Nicolin Chen <[email protected]>
>> ---
> 
> This still just sounds like something that a VMM shouldn't do?

Yeah, if a VMM is going out of its way to emulate obviously-broken 
hardware, then in my opinion we should just give it what it wants and 
let it deal with the consequences of its brokenness.

But if we really do want to bother sanity-checking emulations, then why 
allow them to still get away with being broken in the case that we do 
happen to allocate queue memory at a sufficiently low PA anyway? Why not 
actually validate smmu->oas itself (plus anything else relevant) in 
__tegra241_cmdqv_probe()?

Thanks,
Robin.