Hi Shameer,
On 3/9/26 11:48 AM, Shameer Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 09 March 2026 09:18
>> To: Shameer Kolothum Thodi <[email protected]>; qemu-
>> [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; Nicolin
>> Chen <[email protected]>; Nathan Chen <[email protected]>; Matt
>> Ochs <[email protected]>; Jiandi An <[email protected]>; Jason Gunthorpe
>> <[email protected]>; [email protected];
>> [email protected]; [email protected]; Krishnakant Jaju
>> <[email protected]>; [email protected]
>> Subject: Re: [PATCH v3 06/32] hw/arm/tegra241-cmdqv: Add Tegra241
>> CMDQV ops backend stub
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Shameer,
>>
>> On 2/26/26 11:50 AM, Shameer Kolothum wrote:
>>> Introduce a Tegra241 CMDQV backend that plugs into the SMMUv3
>>> accelerated CMDQV ops interface.
>>>
>>> This patch wires up the Tegra241 CMDQV backend and provides a stub
>>> implementation for CMDQV probe, initialization, vIOMMU allocation and
>>> reset handling.
>>>
>>> Functional CMDQV support is added in follow-up patches.
>>>
>>> Signed-off-by: Shameer Kolothum <[email protected]>
>>> ---
>>> hw/arm/tegra241-cmdqv.h | 15 ++++++++++
>>> hw/arm/tegra241-cmdqv-stubs.c | 18 +++++++++++
>>> hw/arm/tegra241-cmdqv.c | 56
>> +++++++++++++++++++++++++++++++++++
>>> hw/arm/Kconfig | 5 ++++
>>> hw/arm/meson.build | 2 ++
>>> 5 files changed, 96 insertions(+)
>>> create mode 100644 hw/arm/tegra241-cmdqv.h create mode 100644
>>> hw/arm/tegra241-cmdqv-stubs.c create mode 100644
>>> hw/arm/tegra241-cmdqv.c
>>>
>>> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h new
>>> file mode 100644 index 0000000000..07e10e86ee
>>> --- /dev/null
>>> +++ b/hw/arm/tegra241-cmdqv.h
>>> @@ -0,0 +1,15 @@
>>> +/*
>>> + * Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All
>>> +rights reserved
>>> + * NVIDIA Tegra241 CMDQ-Virtualiisation extension for SMMUv3
>>> + *
>>> + * Written by Nicolin Chen, Shameer Kolothum
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later */
>>> +
>>> +#ifndef HW_ARM_TEGRA241_CMDQV_H
>>> +#define HW_ARM_TEGRA241_CMDQV_H
>>> +
>>> +const SMMUv3AccelCmdqvOps *tegra241_cmdqv_get_ops(void);
>>> +
>>> +#endif /* HW_ARM_TEGRA241_CMDQV_H */
>>> diff --git a/hw/arm/tegra241-cmdqv-stubs.c
>>> b/hw/arm/tegra241-cmdqv-stubs.c new file mode 100644 index
>>> 0000000000..eedc7bfdcd
>>> --- /dev/null
>>> +++ b/hw/arm/tegra241-cmdqv-stubs.c
>>> @@ -0,0 +1,18 @@
>>> +/*
>>> + * Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All
>>> +rights reserved
>>> + *
>>> + * Stubs for Tegra241 CMDQ-Virtualiisation extension for SMMUv3
>> ^typo
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/arm/smmuv3.h"
>>> +#include "smmuv3-accel.h"
>>> +#include "hw/arm/tegra241-cmdqv.h"
>>> +
>>> +const SMMUv3AccelCmdqvOps *tegra241_cmdqv_get_ops(void) {
>>> + return NULL;
>>> +}
>>> +
>>> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c new
>>> file mode 100644 index 0000000000..ad5a0d4611
>>> --- /dev/null
>>> +++ b/hw/arm/tegra241-cmdqv.c
>>> @@ -0,0 +1,56 @@
>>> +/*
>>> + * Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All
>>> +rights reserved
>>> + * NVIDIA Tegra241 CMDQ-Virtualization extension for SMMUv3
>>> + *
>>> + * Written by Nicolin Chen, Shameer Kolothum
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later */
>>> +
>>> +#include "qemu/osdep.h"
>>> +
>>> +#include "hw/arm/smmuv3.h"
>>> +#include "smmuv3-accel.h"
>>> +#include "tegra241-cmdqv.h"
>>> +
>>> +static void tegra241_cmdqv_free_viommu(SMMUv3State *s) { }
>>> +
>>> +static bool
>>> +tegra241_cmdqv_alloc_viommu(SMMUv3State *s,
>> HostIOMMUDeviceIOMMUFD *idev,
>>> + uint32_t *out_viommu_id, Error **errp) {
>>> + error_setg(errp, "NVIDIA Tegra241 CMDQV is unsupported");
>>> + return false;
>>> +}
>>> +
>>> +static void tegra241_cmdqv_reset(SMMUv3State *s) { }
>>> +
>>> +static bool tegra241_cmdqv_init(SMMUv3State *s, Error **errp) {
>>> + error_setg(errp, "NVIDIA Tegra241 CMDQV is unsupported");
>>> + return false;
>>> +}
>>> +
>>> +static bool tegra241_cmdqv_probe(SMMUv3State *s,
>> HostIOMMUDeviceIOMMUFD *idev,
>>> + Error **errp) {
>>> + error_setg(errp, "NVIDIA Tegra241 CMDQV is unsupported");
>>> + return false;
>>> +}
>>> +
>>> +static const SMMUv3AccelCmdqvOps tegra241_cmdqv_ops = {
>>> + .probe = tegra241_cmdqv_probe,
>>> + .init = tegra241_cmdqv_init,
>>> + .alloc_viommu = tegra241_cmdqv_alloc_viommu,
>>> + .free_viommu = tegra241_cmdqv_free_viommu,
>>> + .reset = tegra241_cmdqv_reset,
>> I think I would create a no_cmdqv_ops to convert the existing code without
>> any cmdqv support to the same api. This would avoid the repeating:
>>
>> if (cmdqv_ops && cmdqv_ops-><func>) check in subsequent patches.
> I guess you are suggesting having something like,
>
> static const SMMUv3AccelCmdqvOps no_cmdqv_ops = {
> .probe = no_cmdqv_probe,
> .alloc_viommu = no_cmdqv_alloc_viommu,
> ...
> };
>
> static bool no_cmdqv_probe(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> Error **errp)
> {
> return true;
> }
>
> static bool no_cmdqv_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> uint32_t *id,
> Error **errp)
> {
> return false; /* We have to call the default vIOMMU alloc*/
> }
>
> And then call them directly. I will take a look at that approach.
Well forget it, I don't think it is worth the pain.
>
>> Also this looks a bit strange to have different signatures for the functions
>> used
>> with both vcmdq on and off
> Is the concern here is mainly around alloc_viommu( that is the one which
> require fallback
> to default vIOMMU alloc)?
>
> static bool
> smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> Error **errp)
>
> and
>
> bool (*alloc_viommu)(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> uint32_t *out_viommu_id,
> Error **errp);
>
> Making those consistent would likely require some refactoring of the
> existing path. I will have a play with that and see.
Same here. That's OK for now.
Thanks
Eric
>
> Thanks,
> Shameer
>
>> Eric
>>
>>
>>> +};
>>> +
>>> +const SMMUv3AccelCmdqvOps *tegra241_cmdqv_get_ops(void) {
>>> + return &tegra241_cmdqv_ops;
>>> +}
>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
>>> c66c452737..3305c6e76e 100644
>>> --- a/hw/arm/Kconfig
>>> +++ b/hw/arm/Kconfig
>>> @@ -626,6 +626,10 @@ config FSL_IMX8MP_EVK
>>> depends on TCG
>>> select FSL_IMX8MP
>>>
>>> +config TEGRA241_CMDQV
>>> + bool
>>> + depends on ARM_SMMUV3_ACCEL
>>> +
>>> config ARM_SMMUV3_ACCEL
>>> bool
>>> depends on ARM_SMMUV3
>>> @@ -633,6 +637,7 @@ config ARM_SMMUV3_ACCEL config
>> ARM_SMMUV3
>>> bool
>>> select ARM_SMMUV3_ACCEL if IOMMUFD
>>> + imply TEGRA241_CMDQV
>>>
>>> config FSL_IMX6UL
>>> bool
>>> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
>>> index 8f834c32b1..fc83b635e7 100644
>>> --- a/hw/arm/meson.build
>>> +++ b/hw/arm/meson.build
>>> @@ -88,6 +88,8 @@ arm_common_ss.add(when:
>> 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
>>> arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
>> files('smmuv3.c'))
>>> arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true:
>> files('smmuv3-accel.c'))
>>> stub_ss.add(files('smmuv3-accel-stubs.c'))
>>> +arm_common_ss.add(when: 'CONFIG_TEGRA241_CMDQV', if_true:
>> files('tegra241-cmdqv.c'))
>>> +stub_ss.add(files('tegra241-cmdqv-stubs.c'))
>>> arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-
>> imx6ul.c', 'mcimx6ul-evk.c'))
>>> arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true:
>> files('nrf51_soc.c'))
>>> arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
