| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJF2gTQj-mtbg8Q9fN6=EprfV0XaXy-X=auP9iwrF-nLAJ4Ovg@mail.gmail.com>
Date: Sun, 18 Jan 2026 21:54:52 +0800
From: Guo Ren <guoren@...nel.org>
To: fangyu.yu@...ux.alibaba.com
Cc: tjeznach@...osinc.com, joro@...tes.org, will@...nel.org,
robin.murphy@....com, pjw@...nel.org, palmer@...belt.com,
aou@...s.berkeley.edu, alex@...ti.fr, ajones@...tanamicro.com,
iommu@...ts.linux.dev, linux-riscv@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iommu/riscv: Add IOTINVAL after updating DDT/PDT entries
Hi fangyu,
On Thu, Jan 8, 2026 at 9:49 PM <fangyu.yu@...ux.alibaba.com> wrote:
>
> From: Fangyu Yu <fangyu.yu@...ux.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@...ux.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
Powered by blists - more mailing lists