Hi Eric,
> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: 27 October 2025 10:14
> To: Shameer Kolothum <[email protected]>; qemu-
> [email protected]; [email protected]
> Cc: [email protected]; Jason Gunthorpe <[email protected]>; Nicolin
> Chen <[email protected]>; [email protected]; [email protected];
> Nathan Chen <[email protected]>; Matt Ochs <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v4 13/27] hw/arm/smmuv3-accel: Add support to issue
> invalidation cmd to host
>
> External email: Use caution opening links or attachments
>
>
> Hi Shameer,
>
> On 9/29/25 3:36 PM, 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.
> > + */
> > +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) {
> Can you explain/document why !s_accel->viommu is handled as no error?
!s_accel->viommu basically means there are no vfio-pci devices with iommufd
backend. I will add a comment.
> > + return true;
> > + }
> > +
> > + /*
> > + * 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.
> Only propagate errors to host if the SDEV matches a VFIO devices?
The vdev is only allocated for a valid idev device. The below !accel_dev->vdev
checks that.
> > + */
> > + if (sdev) {
> > + SMMUv3AccelDevice *accel_dev = container_of(sdev,
> SMMUv3AccelDevice,
> > + sdev);
> > + if (!accel_dev->vdev) {
> > + return true;
> > + }
> > + }
> > +
> > + viommu_core = &s_accel->viommu->core;
>
> > + return iommufd_backend_invalidate_cache(
> > + viommu_core->iommufd, viommu_core->viommu_id,
> > + IOMMU_VIOMMU_INVALIDATE_DATA_ARM_SMMUV3,
> > + sizeof(Cmd), &entry_num, cmd, errp);
> what shall we do if iommufd_backend_invalidate_cache() succeeds with
> output entry_num is different from onput one? In current case we have
> entry_num = 1 so maybe this is not possible but we might leave a comment at
> least?
Ok. I will add that to comment.
> > +}
> > +
> > static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs,
> SMMUPciBus *sbus,
> > PCIBus *bus, int
> > devfn) { diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> > index 6242614c00..3bdba47616 100644
> > --- a/hw/arm/smmuv3-accel.h
> > +++ b/hw/arm/smmuv3-accel.h
> > @@ -46,6 +46,8 @@ bool smmuv3_accel_install_nested_ste(SMMUv3State
> *s, SMMUDevice *sdev, int sid,
> > Error **errp); bool
> > smmuv3_accel_install_nested_ste_range(SMMUv3State *s,
> SMMUSIDRange *range,
> > Error **errp);
> > +bool smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd,
> SMMUDevice *sdev,
> > + Error **errp);
> > #else
> > static inline void smmuv3_accel_init(SMMUv3State *s) { @@ -62,6
> > +64,12 @@ smmuv3_accel_install_nested_ste_range(SMMUv3State *s,
> > SMMUSIDRange *range, {
> > return true;
> > }
> > +static inline bool
> > +smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice
> *sdev,
> > + Error **errp) {
> > + return true;
> > +}
> > #endif
> >
> > #endif /* HW_ARM_SMMUV3_ACCEL_H */
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index
> > 1fd8aaa0c7..3963bdc87f 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -1381,6 +1381,7 @@ static int
> smmuv3_cmdq_consume(SMMUv3State *s)
> > {
> > uint32_t sid = CMD_SID(&cmd);
> > SMMUDevice *sdev = smmu_find_sdev(bs, sid);
> > + Error *local_err = NULL;
> >
> > if (CMD_SSEC(&cmd)) {
> > cmd_error = SMMU_CERROR_ILL; @@ -1393,11 +1394,17 @@
> > static int smmuv3_cmdq_consume(SMMUv3State *s)
> >
> > trace_smmuv3_cmdq_cfgi_cd(sid);
> > smmuv3_flush_config(sdev);
> > + if (!smmuv3_accel_issue_inv_cmd(s, &cmd, sdev, &local_err)) {
> > + error_report_err(local_err);
> > + cmd_error = SMMU_CERROR_ILL;
> > + break;
> > + }
> > break;
> > }
> > case SMMU_CMD_TLBI_NH_ASID:
> > {
> > int asid = CMD_ASID(&cmd);
> > + Error *local_err = NULL;
> > int vmid = -1;
> >
> > if (!STAGE1_SUPPORTED(s)) { @@ -1416,6 +1423,11 @@ static
> > int smmuv3_cmdq_consume(SMMUv3State *s)
> > trace_smmuv3_cmdq_tlbi_nh_asid(asid);
> > smmu_inv_notifiers_all(&s->smmu_state);
> > smmu_iotlb_inv_asid_vmid(bs, asid, vmid);
> > + if (!smmuv3_accel_issue_inv_cmd(s, &cmd, NULL, &local_err)) {
> > + error_report_err(local_err);
> > + cmd_error = SMMU_CERROR_ILL;
> > + break;
> > + }
> > break;
> > }
> > case SMMU_CMD_TLBI_NH_ALL:
> do we check somewhere that accel mode only applies to stage=1?
See patch #7. We mandate stage-1 for accel=on case.
Thanks,
Shameer
