* [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
@ 2026-01-08 13:48 fangyu.yu
2026-01-16 20:12 ` Andrew Jones
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: fangyu.yu @ 2026-01-08 13:48 UTC (permalink / raw)
To: tjeznach, joro, will, robin.murphy, pjw, palmer, aou, alex
Cc: guoren, ajones, iommu, linux-riscv, linux-kernel, Fangyu Yu
From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
Add riscv_iommu_iodir_iotinval() to perform required TLB and context cache
invalidations after updating DDT or PDT entries, as mandated by the RISC-V
IOMMU specification (Section 6.3.1 and 6.3.2).
Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
---
drivers/iommu/riscv/iommu.c | 85 +++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
index d9429097a2b5..2900170133fc 100644
--- a/drivers/iommu/riscv/iommu.c
+++ b/drivers/iommu/riscv/iommu.c
@@ -996,7 +996,82 @@ static void riscv_iommu_iotlb_inval(struct riscv_iommu_domain *domain,
}
#define RISCV_IOMMU_FSC_BARE 0
+/*
+ * This function sends IOTINVAL commands as required by the RISC-V
+ * IOMMU specification (Section 6.3.1 and 6.3.2 in 1.0 spec version)
+ * after modifying DDT or PDT entries
+ */
+static void riscv_iommu_iodir_iotinval(struct riscv_iommu_device *iommu,
+ bool inval_pdt, unsigned long iohgatp,
+ struct riscv_iommu_dc *dc, struct riscv_iommu_pc *pc)
+{
+ struct riscv_iommu_command cmd;
+ if (FIELD_GET(RISCV_IOMMU_DC_IOHGATP_MODE, iohgatp) ==
+ RISCV_IOMMU_DC_IOHGATP_MODE_BARE) {
+ if (inval_pdt) {
+ /*
+ * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
+ * PSCID=PC.PSCID
+ */
+ riscv_iommu_cmd_inval_vma(&cmd);
+ riscv_iommu_cmd_inval_set_pscid(&cmd,
+ FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
+ } else {
+ if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) || (
+ FIELD_GET(RISCV_IOMMU_DC_FSC_MODE, dc->fsc) ==
+ RISCV_IOMMU_DC_FSC_MODE_BARE)) {
+ /* IOTINVAL.VMA with GV=AV=PSCV=0 */
+ riscv_iommu_cmd_inval_vma(&cmd);
+ } else {
+ /*
+ * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
+ * PSCID=DC.ta.PSCID
+ */
+ riscv_iommu_cmd_inval_vma(&cmd);
+ riscv_iommu_cmd_inval_set_pscid(&cmd,
+ FIELD_GET(RISCV_IOMMU_DC_TA_PSCID, dc->ta));
+ }
+ }
+ } else {
+ if (inval_pdt) {
+ /*
+ * IOTINVAL.VMA with GV=1, AV=0, and PSCV=1, and
+ * GSCID=DC.iohgatp.GSCID, PSCID=PC.PSCID
+ */
+ riscv_iommu_cmd_inval_vma(&cmd);
+ riscv_iommu_cmd_inval_set_gscid(&cmd,
+ FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
+ riscv_iommu_cmd_inval_set_pscid(&cmd,
+ FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
+ } else {
+ /*
+ * IOTINVAL.VMA with GV=1,AV=PSCV=0,and
+ * GSCID=DC.iohgatp.GSCID
+ */
+ riscv_iommu_cmd_inval_vma(&cmd);
+ riscv_iommu_cmd_inval_set_gscid(&cmd,
+ FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
+
+ /*
+ * IOTINVAL.GVMA with GV=1,AV=0,and
+ * GSCID=DC.iohgatp.GSCID
+ */
+ /*
+ * For now, the Second-Stage feature have not yet been merged, so
+ * let's comment out the code first.
+ */
+#if 0
+ riscv_iommu_cmd_send(iommu, &cmd);
+ memset(&cmd, 0, sizeof(cmd));
+ riscv_iommu_cmd_inval_gvma(&cmd);
+ riscv_iommu_cmd_inval_set_gscid(&cmd,
+ FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
+#endif
+ }
+ }
+ riscv_iommu_cmd_send(iommu, &cmd);
+}
/*
* Update IODIR for the device.
*
@@ -1031,6 +1106,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
riscv_iommu_cmd_iodir_inval_ddt(&cmd);
riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
riscv_iommu_cmd_send(iommu, &cmd);
+ /*
+ * For now, the SVA and PASID features have not yet been merged, the
+ * default configuration is inval_pdt=false and pc=NULL.
+ */
+ riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
sync_required = true;
}
@@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
/* Invalidate device context after update */
riscv_iommu_cmd_iodir_inval_ddt(&cmd);
riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
+ /*
+ * For now, the SVA and PASID features have not yet been merged, the
+ * default configuration is inval_pdt=false and pc=NULL.
+ */
+ riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
riscv_iommu_cmd_send(iommu, &cmd);
}
--
2.50.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
2026-01-08 13:48 [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries fangyu.yu
@ 2026-01-16 20:12 ` Andrew Jones
2026-01-16 21:23 ` Andrew Jones
2026-01-17 4:31 ` fangyu.yu
2026-01-18 13:48 ` Guo Ren
2026-01-18 13:54 ` Guo Ren
2 siblings, 2 replies; 8+ messages in thread
From: Andrew Jones @ 2026-01-16 20:12 UTC (permalink / raw)
To: fangyu.yu
Cc: tjeznach, joro, will, robin.murphy, pjw, palmer, aou, alex,
guoren, ajones, iommu, linux-riscv, linux-kernel
On Thu, Jan 08, 2026 at 09:48:55PM +0800, fangyu.yu@linux.alibaba.com wrote:
> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>
> Add riscv_iommu_iodir_iotinval() to perform required TLB and context cache
> invalidations after updating DDT or PDT entries, as mandated by the RISC-V
> IOMMU specification (Section 6.3.1 and 6.3.2).
>
> Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
> ---
> drivers/iommu/riscv/iommu.c | 85 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index d9429097a2b5..2900170133fc 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c
> @@ -996,7 +996,82 @@ static void riscv_iommu_iotlb_inval(struct riscv_iommu_domain *domain,
> }
>
> #define RISCV_IOMMU_FSC_BARE 0
> +/*
> + * This function sends IOTINVAL commands as required by the RISC-V
> + * IOMMU specification (Section 6.3.1 and 6.3.2 in 1.0 spec version)
> + * after modifying DDT or PDT entries
> + */
> +static void riscv_iommu_iodir_iotinval(struct riscv_iommu_device *iommu,
> + bool inval_pdt, unsigned long iohgatp,
> + struct riscv_iommu_dc *dc, struct riscv_iommu_pc *pc)
> +{
> + struct riscv_iommu_command cmd;
>
> + if (FIELD_GET(RISCV_IOMMU_DC_IOHGATP_MODE, iohgatp) ==
> + RISCV_IOMMU_DC_IOHGATP_MODE_BARE) {
nit: This condition can stick out. We have 100 chars.
> + if (inval_pdt) {
> + /*
> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
> + * PSCID=PC.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
> + } else {
> + if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) || (
> + FIELD_GET(RISCV_IOMMU_DC_FSC_MODE, dc->fsc) ==
> + RISCV_IOMMU_DC_FSC_MODE_BARE)) {
nit: formatting
if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) ||
(FIELD_GET(RISCV_IOMMU_DC_FSC_MODE, dc->fsc) ==
RISCV_IOMMU_DC_FSC_MODE_BARE)) {
> + /* IOTINVAL.VMA with GV=AV=PSCV=0 */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + } else {
> + /*
> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
> + * PSCID=DC.ta.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_TA_PSCID, dc->ta));
> + }
> + }
> + } else {
> + if (inval_pdt) {
> + /*
> + * IOTINVAL.VMA with GV=1, AV=0, and PSCV=1, and
> + * GSCID=DC.iohgatp.GSCID, PSCID=PC.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
> + } else {
> + /*
> + * IOTINVAL.VMA with GV=1,AV=PSCV=0,and
> + * GSCID=DC.iohgatp.GSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
> +
> + /*
> + * IOTINVAL.GVMA with GV=1,AV=0,and
> + * GSCID=DC.iohgatp.GSCID
> + */
> + /*
> + * For now, the Second-Stage feature have not yet been merged, so
> + * let's comment out the code first.
> + */
> +#if 0
> + riscv_iommu_cmd_send(iommu, &cmd);
> + memset(&cmd, 0, sizeof(cmd));
The memset isn't necessary since riscv_iommu_cmd_inval_gvma(), which
doesn't yet exist, will overwrite dword0 and zero out dword1.
> + riscv_iommu_cmd_inval_gvma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
> +#endif
> + }
> + }
> + riscv_iommu_cmd_send(iommu, &cmd);
> +}
> /*
> * Update IODIR for the device.
> *
> @@ -1031,6 +1106,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
> riscv_iommu_cmd_send(iommu, &cmd);
> + /*
> + * For now, the SVA and PASID features have not yet been merged, the
> + * default configuration is inval_pdt=false and pc=NULL.
> + */
> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
> sync_required = true;
> }
>
> @@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
> /* Invalidate device context after update */
> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
> + /*
> + * For now, the SVA and PASID features have not yet been merged, the
> + * default configuration is inval_pdt=false and pc=NULL.
> + */
> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
> riscv_iommu_cmd_send(iommu, &cmd);
> }
>
> --
> 2.50.1
>
A faithful implementation of the 6.3.1 and 6.3.2 guidelines for what the
code currently supports. I presume this is fixing an issue? If so, can you
point that out in the commit message?
Otherwise,
Reviewed-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
2026-01-16 20:12 ` Andrew Jones
@ 2026-01-16 21:23 ` Andrew Jones
2026-01-17 4:31 ` fangyu.yu
2026-01-17 4:31 ` fangyu.yu
1 sibling, 1 reply; 8+ messages in thread
From: Andrew Jones @ 2026-01-16 21:23 UTC (permalink / raw)
To: fangyu.yu
Cc: tjeznach, joro, will, robin.murphy, pjw, palmer, aou, alex,
guoren, ajones, iommu, linux-riscv, linux-kernel
On Fri, Jan 16, 2026 at 02:12:06PM -0600, Andrew Jones wrote:
...
> > @@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
> > /* Invalidate device context after update */
> > riscv_iommu_cmd_iodir_inval_ddt(&cmd);
> > riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
> > + /*
> > + * For now, the SVA and PASID features have not yet been merged, the
> > + * default configuration is inval_pdt=false and pc=NULL.
> > + */
> > + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
Shouldn't this iotinval come after the cmd-send below?
> > riscv_iommu_cmd_send(iommu, &cmd);
> > }
> >
> > --
> > 2.50.1
> >
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
2026-01-16 20:12 ` Andrew Jones
2026-01-16 21:23 ` Andrew Jones
@ 2026-01-17 4:31 ` fangyu.yu
1 sibling, 0 replies; 8+ messages in thread
From: fangyu.yu @ 2026-01-17 4:31 UTC (permalink / raw)
To: andrew.jones
Cc: ajones, alex, aou, fangyu.yu, guoren, iommu, joro, linux-kernel,
linux-riscv, palmer, pjw, robin.murphy, tjeznach, will
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 5328 bytes --]
>> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>>
>> Add riscv_iommu_iodir_iotinval() to perform required TLB and context cache
>> invalidations after updating DDT or PDT entries, as mandated by the RISC-V
>> IOMMU specification (Section 6.3.1 and 6.3.2).
>>
>> Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>> ---
>> drivers/iommu/riscv/iommu.c | 85 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 85 insertions(+)
>>
>> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
>> index d9429097a2b5..2900170133fc 100644
>> --- a/drivers/iommu/riscv/iommu.c
>> +++ b/drivers/iommu/riscv/iommu.c
>> @@ -996,7 +996,82 @@ static void riscv_iommu_iotlb_inval(struct riscv_iommu_domain *domain,
>> }
>>
>> #define RISCV_IOMMU_FSC_BARE 0
>> +/*
>> + * This function sends IOTINVAL commands as required by the RISC-V
>> + * IOMMU specification (Section 6.3.1 and 6.3.2 in 1.0 spec version)
>> + * after modifying DDT or PDT entries
>> + */
>> +static void riscv_iommu_iodir_iotinval(struct riscv_iommu_device *iommu,
>> + bool inval_pdt, unsigned long iohgatp,
>> + struct riscv_iommu_dc *dc, struct riscv_iommu_pc *pc)
>> +{
>> + struct riscv_iommu_command cmd;
>>
>> + if (FIELD_GET(RISCV_IOMMU_DC_IOHGATP_MODE, iohgatp) ==
>> + RISCV_IOMMU_DC_IOHGATP_MODE_BARE) {
>
>nit: This condition can stick out. We have 100 chars.
>
Ack.
>> + if (inval_pdt) {
>> + /*
>> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
>> + * PSCID=PC.PSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_pscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
>> + } else {
>> + if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) || (
>> + FIELD_GET(RISCV_IOMMU_DC_FSC_MODE, dc->fsc) ==
>> + RISCV_IOMMU_DC_FSC_MODE_BARE)) {
>
>nit: formatting
Ack.
> if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) ||
> (FIELD_GET(RISCV_IOMMU_DC_FSC_MODE, dc->fsc) ==
> RISCV_IOMMU_DC_FSC_MODE_BARE)) {
>
>> + /* IOTINVAL.VMA with GV=AV=PSCV=0 */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + } else {
>> + /*
>> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
>> + * PSCID=DC.ta.PSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_pscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_TA_PSCID, dc->ta));
>> + }
>> + }
>> + } else {
>> + if (inval_pdt) {
>> + /*
>> + * IOTINVAL.VMA with GV=1, AV=0, and PSCV=1, and
>> + * GSCID=DC.iohgatp.GSCID, PSCID=PC.PSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_gscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
>> + riscv_iommu_cmd_inval_set_pscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
>> + } else {
>> + /*
>> + * IOTINVAL.VMA with GV=1,AV=PSCV=0,and
>> + * GSCID=DC.iohgatp.GSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_gscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
>> +
>> + /*
>> + * IOTINVAL.GVMA with GV=1,AV=0,and
>> + * GSCID=DC.iohgatp.GSCID
>> + */
>> + /*
>> + * For now, the Second-Stage feature have not yet been merged, so
>> + * let's comment out the code first.
>> + */
>> +#if 0
>> + riscv_iommu_cmd_send(iommu, &cmd);
>> + memset(&cmd, 0, sizeof(cmd));
>
>The memset isn't necessary since riscv_iommu_cmd_inval_gvma(), which
>doesn't yet exist, will overwrite dword0 and zero out dword1.
Agreed, I’ll drop memset in v2.
>
>> + riscv_iommu_cmd_inval_gvma(&cmd);
>> + riscv_iommu_cmd_inval_set_gscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
>> +#endif
>> + }
>> + }
>> + riscv_iommu_cmd_send(iommu, &cmd);
>> +}
>> /*
>> * Update IODIR for the device.
>> *
>> @@ -1031,6 +1106,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
>> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
>> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
>> riscv_iommu_cmd_send(iommu, &cmd);
>> + /*
>> + * For now, the SVA and PASID features have not yet been merged, the
>> + * default configuration is inval_pdt=false and pc=NULL.
>> + */
>> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
>> sync_required = true;
>> }
>>
>> @@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
>> /* Invalidate device context after update */
>> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
>> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
>> + /*
>> + * For now, the SVA and PASID features have not yet been merged, the
>> + * default configuration is inval_pdt=false and pc=NULL.
>> + */
>> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
>> riscv_iommu_cmd_send(iommu, &cmd);
>> }
>>
>> --
>> 2.50.1
>>
>
>A faithful implementation of the 6.3.1 and 6.3.2 guidelines for what the
>code currently supports. I presume this is fixing an issue? If so, can you
>point that out in the commit message?
Yes, this is fixing a functional issue: when software changes a leaf-level
DDT or PDT entry we weren't issuing the required IOTINVAL.
I'll update the commit message in v2.
>
>Otherwise,
>
>Reviewed-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
>
>Thanks,
>drew
>
Thanks,
Fangyu
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
2026-01-16 21:23 ` Andrew Jones
@ 2026-01-17 4:31 ` fangyu.yu
0 siblings, 0 replies; 8+ messages in thread
From: fangyu.yu @ 2026-01-17 4:31 UTC (permalink / raw)
To: andrew.jones
Cc: ajones, alex, aou, fangyu.yu, guoren, iommu, joro, linux-kernel,
linux-riscv, palmer, pjw, robin.murphy, tjeznach, will
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 796 bytes --]
>On Fri, Jan 16, 2026 at 02:12:06PM -0600, Andrew Jones wrote:
>...
>> > @@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
>> > /* Invalidate device context after update */
>> > riscv_iommu_cmd_iodir_inval_ddt(&cmd);
>> > riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
>> > + /*
>> > + * For now, the SVA and PASID features have not yet been merged, the
>> > + * default configuration is inval_pdt=false and pc=NULL.
>> > + */
>> > + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
>
>Shouldn't this iotinval come after the cmd-send below?
Yes, that's a typo on my side.
I’ll fix in v2.
>
>> > riscv_iommu_cmd_send(iommu, &cmd);
>> > }
>> >
>> > --
>> > 2.50.1
>> >
>
>Thanks,
>drew
>
Thanks,
Fangyu
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
2026-01-08 13:48 [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries fangyu.yu
2026-01-16 20:12 ` Andrew Jones
@ 2026-01-18 13:48 ` Guo Ren
2026-01-18 13:54 ` Guo Ren
2 siblings, 0 replies; 8+ messages in thread
From: Guo Ren @ 2026-01-18 13:48 UTC (permalink / raw)
To: fangyu.yu
Cc: tjeznach, joro, will, robin.murphy, pjw, palmer, aou, alex,
ajones, iommu, linux-riscv, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 6724 bytes --]
On Thu, Jan 8, 2026 at 9:49 PM <fangyu.yu@linux.alibaba.com> wrote:
> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>
> Add riscv_iommu_iodir_iotinval() to perform required TLB and context cache
> invalidations after updating DDT or PDT entries, as mandated by the RISC-V
> IOMMU specification (Section 6.3.1 and 6.3.2).
>
> Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
> ---
> drivers/iommu/riscv/iommu.c | 85 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index d9429097a2b5..2900170133fc 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c
> @@ -996,7 +996,82 @@ static void riscv_iommu_iotlb_inval(struct
> riscv_iommu_domain *domain,
> }
>
> #define RISCV_IOMMU_FSC_BARE 0
> +/*
> + * This function sends IOTINVAL commands as required by the RISC-V
> + * IOMMU specification (Section 6.3.1 and 6.3.2 in 1.0 spec version)
> + * after modifying DDT or PDT entries
> + */
> +static void riscv_iommu_iodir_iotinval(struct riscv_iommu_device *iommu,
> + bool inval_pdt, unsigned long iohgatp,
> + struct riscv_iommu_dc *dc, struct
> riscv_iommu_pc *pc)
> +{
> + struct riscv_iommu_command cmd;
>
> + if (FIELD_GET(RISCV_IOMMU_DC_IOHGATP_MODE, iohgatp) ==
> + RISCV_IOMMU_DC_IOHGATP_MODE_BARE) {
> + if (inval_pdt) {
> + /*
> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
> + * PSCID=PC.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID,
> pc->ta));
> + } else {
> + if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) || (
> + FIELD_GET(RISCV_IOMMU_DC_FSC_MODE,
> dc->fsc) ==
> + RISCV_IOMMU_DC_FSC_MODE_BARE)) {
> + /* IOTINVAL.VMA with GV=AV=PSCV=0 */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + } else {
> + /*
> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1,
> and
> + * PSCID=DC.ta.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_TA_PSCID,
> dc->ta));
> + }
> + }
> + } else {
> + if (inval_pdt) {
> + /*
> + * IOTINVAL.VMA with GV=1, AV=0, and PSCV=1, and
> + * GSCID=DC.iohgatp.GSCID, PSCID=PC.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
+ riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID,
> iohgatp));
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID,
> pc->ta));
>
The riscv_iommu_cmd_inval_set_pscid() and riscv_iommu_cmd_inval_vma() could
be moved out to prevent duplicate code.
> + } else {
> + /*
> + * IOTINVAL.VMA with GV=1,AV=PSCV=0,and
> + * GSCID=DC.iohgatp.GSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID,
> iohgatp));
> +
> + /*
> + * IOTINVAL.GVMA with GV=1,AV=0,and
> + * GSCID=DC.iohgatp.GSCID
> + */
> + /*
> + * For now, the Second-Stage feature have not yet
> been merged, so
> + * let's comment out the code first.
> + */
> +#if 0
+ riscv_iommu_cmd_send(iommu, &cmd);
> + memset(&cmd, 0, sizeof(cmd));
> + riscv_iommu_cmd_inval_gvma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID,
> iohgatp));
> +#endif
>
All the above should be removed from the patch; we don't need draft code.
+ }
> + }
> + riscv_iommu_cmd_send(iommu, &cmd);
> +}
> /*
> * Update IODIR for the device.
> *
> @@ -1031,6 +1106,11 @@ static void riscv_iommu_iodir_update(struct
> riscv_iommu_device *iommu,
> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
> riscv_iommu_cmd_send(iommu, &cmd);
> + /*
> + * For now, the SVA and PASID features have not yet been
> merged, the
> + * default configuration is inval_pdt=false and pc=NULL.
> + */
> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc,
> NULL);
>
The riscv_iommu_iodir_iotinval() is the same level as
riscv_iommu_iodir_update(). Could we move it out and put it after
riscv_iommu_iodir_update()?
sync_required = true;
> }
>
> @@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct
> riscv_iommu_device *iommu,
>
Why do you have two riscv_iommu_iodir_update() function definitions?
> /* Invalidate device context after update */
> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
> + /*
> + * For now, the SVA and PASID features have not yet been
> merged, the
> + * default configuration is inval_pdt=false and pc=NULL.
> + */
> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc,
> NULL);
> riscv_iommu_cmd_send(iommu, &cmd);
>
If IOTLB invalidation occurs before DDT_CACHE invalidation, the IOTLB may
use DDT_CACHE's stall info, which may cause IOTLB invalidation to fail.
> }
>
> --
> 2.50.1
>
>
--
Best Regards
Guo Ren
[-- Attachment #1.2: Type: text/html, Size: 9623 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
2026-01-08 13:48 [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries fangyu.yu
2026-01-16 20:12 ` Andrew Jones
2026-01-18 13:48 ` Guo Ren
@ 2026-01-18 13:54 ` Guo Ren
2026-01-18 14:33 ` fangyu.yu
2 siblings, 1 reply; 8+ messages in thread
From: Guo Ren @ 2026-01-18 13:54 UTC (permalink / raw)
To: fangyu.yu
Cc: tjeznach, joro, will, robin.murphy, pjw, palmer, aou, alex,
ajones, iommu, linux-riscv, linux-kernel
Hi fangyu,
On Thu, Jan 8, 2026 at 9:49 PM <fangyu.yu@linux.alibaba.com> wrote:
>
> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>
> Add riscv_iommu_iodir_iotinval() to perform required TLB and context cache
> invalidations after updating DDT or PDT entries, as mandated by the RISC-V
> IOMMU specification (Section 6.3.1 and 6.3.2).
>
> Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
> ---
> drivers/iommu/riscv/iommu.c | 85 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 85 insertions(+)
>
> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index d9429097a2b5..2900170133fc 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c
> @@ -996,7 +996,82 @@ static void riscv_iommu_iotlb_inval(struct riscv_iommu_domain *domain,
> }
>
> #define RISCV_IOMMU_FSC_BARE 0
> +/*
> + * This function sends IOTINVAL commands as required by the RISC-V
> + * IOMMU specification (Section 6.3.1 and 6.3.2 in 1.0 spec version)
> + * after modifying DDT or PDT entries
> + */
> +static void riscv_iommu_iodir_iotinval(struct riscv_iommu_device *iommu,
> + bool inval_pdt, unsigned long iohgatp,
> + struct riscv_iommu_dc *dc, struct riscv_iommu_pc *pc)
> +{
> + struct riscv_iommu_command cmd;
>
> + if (FIELD_GET(RISCV_IOMMU_DC_IOHGATP_MODE, iohgatp) ==
> + RISCV_IOMMU_DC_IOHGATP_MODE_BARE) {
> + if (inval_pdt) {
> + /*
> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
> + * PSCID=PC.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
> + } else {
> + if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) || (
> + FIELD_GET(RISCV_IOMMU_DC_FSC_MODE, dc->fsc) ==
> + RISCV_IOMMU_DC_FSC_MODE_BARE)) {
> + /* IOTINVAL.VMA with GV=AV=PSCV=0 */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + } else {
> + /*
> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
> + * PSCID=DC.ta.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_TA_PSCID, dc->ta));
> + }
> + }
> + } else {
> + if (inval_pdt) {
> + /*
> + * IOTINVAL.VMA with GV=1, AV=0, and PSCV=1, and
> + * GSCID=DC.iohgatp.GSCID, PSCID=PC.PSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
> + riscv_iommu_cmd_inval_set_pscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
The riscv_iommu_cmd_inval_vma() and riscv_iommu_cmd_inval_set_pscid()
could be moved out to prevent duplicate code.
> + } else {
> + /*
> + * IOTINVAL.VMA with GV=1,AV=PSCV=0,and
> + * GSCID=DC.iohgatp.GSCID
> + */
> + riscv_iommu_cmd_inval_vma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
> +
> + /*
> + * IOTINVAL.GVMA with GV=1,AV=0,and
> + * GSCID=DC.iohgatp.GSCID
> + */
> + /*
> + * For now, the Second-Stage feature have not yet been merged, so
> + * let's comment out the code first.
> + */
> +#if 0
> + riscv_iommu_cmd_send(iommu, &cmd);
> + memset(&cmd, 0, sizeof(cmd));
> + riscv_iommu_cmd_inval_gvma(&cmd);
> + riscv_iommu_cmd_inval_set_gscid(&cmd,
> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
> +#endif
All the above should be removed from the patch; we don't need draft code.
> + }
> + }
> + riscv_iommu_cmd_send(iommu, &cmd);
> +}
> /*
> * Update IODIR for the device.
> *
> @@ -1031,6 +1106,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
> riscv_iommu_cmd_send(iommu, &cmd);
> + /*
> + * For now, the SVA and PASID features have not yet been merged, the
> + * default configuration is inval_pdt=false and pc=NULL.
> + */
> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
The riscv_iommu_iodir_iotinval() is the same level as
riscv_iommu_iodir_update(). Could we move it out and put it after
riscv_iommu_iodir_update()?
> sync_required = true;
> }
>
> @@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
Why do you have two riscv_iommu_iodir_update() function definitions?
> /* Invalidate device context after update */
> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
> + /*
> + * For now, the SVA and PASID features have not yet been merged, the
> + * default configuration is inval_pdt=false and pc=NULL.
> + */
> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
If IOTLB invalidation occurs before DDT_CACHE invalidation, the IOTLB
may use DDT_CACHE's stall info, which may cause IOTLB invalidation to
fail.
> riscv_iommu_cmd_send(iommu, &cmd);
> }
>
> --
> 2.50.1
>
--
Best Regards
Guo Ren
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
2026-01-18 13:54 ` Guo Ren
@ 2026-01-18 14:33 ` fangyu.yu
0 siblings, 0 replies; 8+ messages in thread
From: fangyu.yu @ 2026-01-18 14:33 UTC (permalink / raw)
To: guoren
Cc: ajones, alex, aou, fangyu.yu, iommu, joro, linux-kernel,
linux-riscv, palmer, pjw, robin.murphy, tjeznach, will
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 7532 bytes --]
>Hi fangyu,
>
>On Thu, Jan 8, 2026 at 9:49 PM <fangyu.yu@linux.alibaba.com> wrote:
>>
>> From: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>>
>> Add riscv_iommu_iodir_iotinval() to perform required TLB and context cache
>> invalidations after updating DDT or PDT entries, as mandated by the RISC-V
>> IOMMU specification (Section 6.3.1 and 6.3.2).
>>
>> Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com>
>> ---
>> drivers/iommu/riscv/iommu.c | 85 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 85 insertions(+)
>>
>> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
>> index d9429097a2b5..2900170133fc 100644
>> --- a/drivers/iommu/riscv/iommu.c
>> +++ b/drivers/iommu/riscv/iommu.c
>> @@ -996,7 +996,82 @@ static void riscv_iommu_iotlb_inval(struct riscv_iommu_domain *domain,
>> }
>>
>> #define RISCV_IOMMU_FSC_BARE 0
>> +/*
>> + * This function sends IOTINVAL commands as required by the RISC-V
>> + * IOMMU specification (Section 6.3.1 and 6.3.2 in 1.0 spec version)
>> + * after modifying DDT or PDT entries
>> + */
>> +static void riscv_iommu_iodir_iotinval(struct riscv_iommu_device *iommu,
>> + bool inval_pdt, unsigned long iohgatp,
>> + struct riscv_iommu_dc *dc, struct riscv_iommu_pc *pc)
>> +{
>> + struct riscv_iommu_command cmd;
>>
>> + if (FIELD_GET(RISCV_IOMMU_DC_IOHGATP_MODE, iohgatp) ==
>> + RISCV_IOMMU_DC_IOHGATP_MODE_BARE) {
>> + if (inval_pdt) {
>> + /*
>> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
>> + * PSCID=PC.PSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_pscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
>> + } else {
>> + if (FIELD_GET(RISCV_IOMMU_DC_TC_PDTV, dc->tc) || (
>> + FIELD_GET(RISCV_IOMMU_DC_FSC_MODE, dc->fsc) ==
>> + RISCV_IOMMU_DC_FSC_MODE_BARE)) {
>> + /* IOTINVAL.VMA with GV=AV=PSCV=0 */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + } else {
>> + /*
>> + * IOTINVAL.VMA with GV=AV=0, and PSCV=1, and
>> + * PSCID=DC.ta.PSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_pscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_TA_PSCID, dc->ta));
>> + }
>> + }
>> + } else {
>> + if (inval_pdt) {
>> + /*
>> + * IOTINVAL.VMA with GV=1, AV=0, and PSCV=1, and
>> + * GSCID=DC.iohgatp.GSCID, PSCID=PC.PSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_gscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
>> + riscv_iommu_cmd_inval_set_pscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_PC_TA_PSCID, pc->ta));
>The riscv_iommu_cmd_inval_vma() and riscv_iommu_cmd_inval_set_pscid()
>could be moved out to prevent duplicate code.
Thanks, Agreed on reducing duplication.
I'll move riscv_iommu_cmd_inval_vma() to a common path (build the VMA command
once), and only conditionally call riscv_iommu_cmd_inval_set_pscid() when
PSCV=1 is required by the spec.
>
>> + } else {
>> + /*
>> + * IOTINVAL.VMA with GV=1,AV=PSCV=0,and
>> + * GSCID=DC.iohgatp.GSCID
>> + */
>> + riscv_iommu_cmd_inval_vma(&cmd);
>> + riscv_iommu_cmd_inval_set_gscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
>> +
>> + /*
>> + * IOTINVAL.GVMA with GV=1,AV=0,and
>> + * GSCID=DC.iohgatp.GSCID
>> + */
>> + /*
>> + * For now, the Second-Stage feature have not yet been merged, so
>> + * let's comment out the code first.
>> + */
>> +#if 0
>> + riscv_iommu_cmd_send(iommu, &cmd);
>> + memset(&cmd, 0, sizeof(cmd));
>> + riscv_iommu_cmd_inval_gvma(&cmd);
>> + riscv_iommu_cmd_inval_set_gscid(&cmd,
>> + FIELD_GET(RISCV_IOMMU_DC_IOHGATP_GSCID, iohgatp));
>> +#endif
>All the above should be removed from the patch; we don't need draft code.
Agreed, I will drop the entire #if 0 draft block and add a TODO here.
>
>> + }
>> + }
>> + riscv_iommu_cmd_send(iommu, &cmd);
>> +}
>> /*
>> * Update IODIR for the device.
>> *
>> @@ -1031,6 +1106,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
>> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
>> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
>> riscv_iommu_cmd_send(iommu, &cmd);
>> + /*
>> + * For now, the SVA and PASID features have not yet been merged, the
>> + * default configuration is inval_pdt=false and pc=NULL.
>> + */
>> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
>The riscv_iommu_iodir_iotinval() is the same level as
>riscv_iommu_iodir_update(). Could we move it out and put it after
>riscv_iommu_iodir_update()?
In riscv_iommu_iodir_update() we first clear the valid bit in dc->tc, an
IOTINVAL is required immediately after modifying the DDT/PDT entry to make
the change effective.
So riscv_iommu_iodir_iotinval() is paired with the DDT/PDT update sequence at the
same level as riscv_iommu_cmd_iodir_inval_ddt(), and keeping it adjacent preserves
the required ordering and avoids a window where stale cached context could be used.
>
>> sync_required = true;
>> }
>>
>> @@ -1055,6 +1135,11 @@ static void riscv_iommu_iodir_update(struct riscv_iommu_device *iommu,
>Why do you have two riscv_iommu_iodir_update() function definitions?
>
>> /* Invalidate device context after update */
>> riscv_iommu_cmd_iodir_inval_ddt(&cmd);
>> riscv_iommu_cmd_iodir_set_did(&cmd, fwspec->ids[i]);
>> + /*
>> + * For now, the SVA and PASID features have not yet been merged, the
>> + * default configuration is inval_pdt=false and pc=NULL.
>> + */
>> + riscv_iommu_iodir_iotinval(iommu, false, dc->iohgatp, dc, NULL);
>If IOTLB invalidation occurs before DDT_CACHE invalidation, the IOTLB
>may use DDT_CACHE's stall info, which may cause IOTLB invalidation to
>fail.
You're right. I placed riscv_iommu_iodir_iotinval() before riscv_iommu_cmd_send()
by mistake, I’ll fix the ordering in v2.
>
>> riscv_iommu_cmd_send(iommu, &cmd);
>> }
>>
>> --
>> 2.50.1
>>
>
>--
>Best Regards
> Guo Ren
Thanks,
Fangyu
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-01-27 19:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-08 13:48 [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries fangyu.yu
2026-01-16 20:12 ` Andrew Jones
2026-01-16 21:23 ` Andrew Jones
2026-01-17 4:31 ` fangyu.yu
2026-01-17 4:31 ` fangyu.yu
2026-01-18 13:48 ` Guo Ren
2026-01-18 13:54 ` Guo Ren
2026-01-18 14:33 ` fangyu.yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox