On Mon, Sep 29, 2025 at 02:36:29PM +0100, Shameer Kolothum wrote:
> Provide a helper and use that to issue the invalidation cmd to host SMMUv3.
> We only issue one cmd at a time for now.
>
> Support for batching of commands will be added later after analysing the
> impact.
>
> Signed-off-by: Shameer Kolothum <[email protected]>
> ---
> hw/arm/smmuv3-accel.c | 38 ++++++++++++++++++++++++++++++++++++++
> hw/arm/smmuv3-accel.h | 8 ++++++++
> hw/arm/smmuv3.c | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 76 insertions(+)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index f4e01fba6d..9ad8595ce2 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -218,6 +218,44 @@ bool smmuv3_accel_install_nested_ste_range(SMMUv3State
> *s, SMMUSIDRange *range,
> return true;
> }
>
> +/*
> + * This issues the invalidation cmd to the host SMMUv3.
> + * Note: sdev can be NULL for certain invalidation commands
> + * e.g., SMMU_CMD_TLBI_NH_ASID, SMMU_CMD_TLBI_NH_VA etc.
* Note a TLBI command is passed in with a NULL sdev.
> + */
> +bool smmuv3_accel_issue_inv_cmd(SMMUv3State *bs, void *cmd, SMMUDevice *sdev,
> + Error **errp)
> +{
> + SMMUv3State *s = ARM_SMMUV3(bs);
> + SMMUv3AccelState *s_accel = s->s_accel;
> + IOMMUFDViommu *viommu_core;
> + uint32_t entry_num = 1;
> +
> + if (!s->accel || !s_accel->viommu) {
if (!accel || !s_accel->viommu) {
> + /*
> + * We may end up here for any emulated PCI bridge or root port type
> devices.
> + * However, passing invalidation commands with sid (eg: CFGI_CD) to host
> + * SMMUv3 only matters for vfio-pci endpoint devices. Hence check that if
> + * sdev is valid.
> + */
I think we should use "allowed" over "matters".
> + if (sdev) {
> + SMMUv3AccelDevice *accel_dev = container_of(sdev, SMMUv3AccelDevice,
> + sdev);
> + if (!accel_dev->vdev) {
> + return true;
> + }
> + }
And we could simplify with:
/*
* Only a device associated with the vIOMMU (by having a valid vdev) is
* allowed to flush its device cache
*/
if (sdev && !container_of(sdev, SMMUv3AccelDevice, sdev)->vdev) {
return true;
}
> @@ -1440,18 +1452,36 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> QEMU_FALLTHROUGH;
> }
> case SMMU_CMD_TLBI_NSNH_ALL:
> + {
> + Error *local_err = NULL;
> +
> trace_smmuv3_cmdq_tlbi_nsnh();
> smmu_inv_notifiers_all(&s->smmu_state);
> smmu_iotlb_inv_all(bs);
> + if (!smmuv3_accel_issue_inv_cmd(s, &cmd, NULL, &local_err)) {
> + error_report_err(local_err);
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> break;
> + }
local_err is not used but only only printed. It'd be cleaner to
move it inside smmuv3_accel_issue_inv_cmd(). So, you would not
need to add "{}" nor have an "Error *" parameter in the helper.
Only cmd_error should stay.
With these,
Reviewed-by: Nicolin Chen <[email protected]>
