| 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: <c5sk7h5r5fozjdyguamekenlzd7pzbtvdh2boq3arynxifba22@ii4ikea2tuyq>
Date: Fri, 16 Jan 2026 14:12:06 -0600
From: Andrew Jones <andrew.jones@....qualcomm.com>
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, guoren@...nel.org,
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
On Thu, Jan 08, 2026 at 09:48:55PM +0800, 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) {
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@....qualcomm.com>
Thanks,
drew
Powered by blists - more mailing lists