Re: [PATCH v4 19/31] hw/arm/tegra241-cmdqv: Allocate HW VCMDQs on base register programming

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, May 05, 2026 at 07:26:29AM -0700, Shameer Kolothum Thodi wrote:
> > > +static void tegra241_cmdqv_free_all_vcmdq(Tegra241CMDQV *cmdqv)
> > > +{
> > > +    /* Free in reverse order to avoid "resource busy" error */
> > can you provide additional details about the above problematic. Is it
> > documented in the spec?
> 
> See p.176:
> 
> Deallocate a VCMDQ from a Virtual Interface 
>     Logical CMDQ being deallocated for a Guest must be in decreasing order
>     starting from the highest numbered LVCMDQ.

Maybe add "Spec demands" to things like this.

> > > +    for (int i = (TEGRA241_CMDQV_MAX_CMDQ - 1); i >= 0; i--) {
> > > +        tegra241_cmdqv_free_vcmdq(cmdqv, i);
> > > +    }
> > > +}
> > > +
> > > +static bool tegra241_cmdqv_setup_vcmdq(Tegra241CMDQV *cmdqv, int
> > index,
> > > +                                       Error **errp)
> > > +{
> > > +    SMMUv3AccelState *accel = cmdqv->s_accel;
> > > +    uint64_t base_mask = (uint64_t)R_VCMDQ0_BASE_L_ADDR_MASK |
> > > +                         (uint64_t)R_VCMDQ0_BASE_H_ADDR_MASK << 32;
> > > +    uint64_t addr = cmdqv->vcmdq_base[index] & base_mask;
> > > +    uint64_t log2 = cmdqv->vcmdq_base[index] &
> > R_VCMDQ0_BASE_L_LOG2SIZE_MASK;
> > > +    uint64_t size = 1ULL << (log2 + 4);
> > > +    IOMMUFDViommu *viommu = accel->viommu;
> > > +    IOMMUFDHWqueue *hw_queue;
> > > +    uint32_t hw_queue_id;
> > > +
> > > +    /* Ignore any invalid address. This may come as part of reset etc. */
> > > +    if (!address_space_is_ram(&address_space_memory, addr) ||
> > > +        !address_space_is_ram(&address_space_memory, addr + size - 1)) {
> > this check looks a little bit risky, no? Don't we have a better way to
> > test the address has been properly set?
> 
> I think eventually kernel will handle any attempt to use an invalid address
> through IOMMU_HW_QUEUE_ALLOC IOCTL:
>      iommufd_hw_queue_alloc_phys()/iommufd_access_pin_pages() etc.
> 
> Any attempt to pass an invalid address will return error, I think.
> 
> @Nicolin, is that a safe assumption to make?

Kernel validates if the gPA range is in the stage-2 page table and
physically contiguous. It might be nicer if QEMU has a way to make
sure that is the range is in RAM.

Nicolin