linux-kernel - Re: [PATCH] iommu/arm-smmu-v3-iommufd: Allow attaching nested domain for GBPA cases

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aQi8TivdgmtAyb7v@google.com>
Date: Mon, 3 Nov 2025 14:29:34 +0000
From: Pranjal Shrivastava <praan@...gle.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: jgg@...dia.com, will@...nel.org, robin.murphy@....com, joro@...tes.org,
	kevin.tian@...el.com, linux-arm-kernel@...ts.infradead.org,
	iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
	skolothumtho@...dia.com
Subject: Re: [PATCH] iommu/arm-smmu-v3-iommufd: Allow attaching nested domain
 for GBPA cases

On Thu, Oct 23, 2025 at 09:05:51PM -0700, Nicolin Chen wrote:
> A vDEVICE has been a hard requirement for attaching a nested domain to the
> device. This makes sense when installing a guest STE, since a vSID must be
> present and given to the kernel during the vDEVICE allocation.
> 
> But, when CR0.SMMUEN is disabled, VM doesn't really need a vSID to program
> the vSMMU behavior as GBPA will take effect, in which case the vSTE in the
> nested domain could have carried the bypass or abort configuration in GBPA
> register. Thus, having such a hard requirement doesn't work well for GBPA.
> 
> Add an additional condition in arm_smmu_attach_prepare_vmaster(), to allow
> a bypass or abort vSTE working for a GBPA setup. And do not forget to skip
> vsid=0 when reporting vevents, since the guest SMMU in this setup will not
> report event anyway.
> 
> Update the uAPI doc accordingly.
> 
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c    | 14 ++++++++++++--
>  include/uapi/linux/iommufd.h                       |  7 +++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 

Overall, the approach seems fine as it adds value since we can't have
vSMMUs with ABORT / BYPASS config with the current code.

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> index 8cd8929bbfdf8..7d13b9f55512e 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> @@ -99,15 +99,22 @@ static void arm_smmu_make_nested_domain_ste(
>  int arm_smmu_attach_prepare_vmaster(struct arm_smmu_attach_state *state,
>  				    struct arm_smmu_nested_domain *nested_domain)
>  {
> +	unsigned int cfg =
> +		FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(nested_domain->ste[0]));
>  	struct arm_smmu_vmaster *vmaster;
> -	unsigned long vsid;
> +	unsigned long vsid = 0;

I'm a little confused here, can we not have a vDEVICE allocated with
vSID = 0 ?

IIRC, vsid is given the value of vdev->virt_id, whereas vdev->virt_id is
allocated in iommufd_vdevice_alloc_ioctl() using the user-provided
cmd->virt_id.

Thus, should we mention this in the uAPI that vdev->virt_id 0 is reserved?

Because otherwise, a VMM may actually allocate a vDEVICE with vSID = 0
(which iommufd currently allows) and we'll start dropping its events
due to the if (!vmaster->vsid) check below. This would be a functional 
regression for such a VMM.

Perhaps a separate bool has_vdevice flag in struct arm_smmu_vmaster
would be clearer and avoid this ambiguity, allowing vsid = 0 to be a
valid ID for an allocated vdevice when user-space explicitly requests it?

>  	int ret;
>  
>  	iommu_group_mutex_assert(state->master->dev);
>  
>  	ret = iommufd_viommu_get_vdev_id(&nested_domain->vsmmu->core,
>  					 state->master->dev, &vsid);
> -	if (ret)
> +	/*
> +	 * Attaching to a translate nested domain must allocate a vDEVICE prior,
> +	 * as CD/ATS invalidations and vevents require a vSID to work properly.
> +	 * A bypass/abort domain is allowed to attach with vsid=0 for GBPA case.
> +	 */
> +	if (ret && cfg == STRTAB_STE_0_CFG_S1_TRANS)
>  		return ret;
>  
>  	vmaster = kzalloc(sizeof(*vmaster), GFP_KERNEL);
> @@ -460,6 +467,9 @@ int arm_vmaster_report_event(struct arm_smmu_vmaster *vmaster, u64 *evt)
>  
>  	lockdep_assert_held(&vmaster->vsmmu->smmu->streams_mutex);
>  
> +	if (!vmaster->vsid)
> +		return -ENOENT;
> +
>  	vevt.evt[0] = cpu_to_le64((evt[0] & ~EVTQ_0_SID) |
>  				  FIELD_PREP(EVTQ_0_SID, vmaster->vsid));
>  	for (i = 1; i < EVTQ_ENT_DWORDS; i++)
> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index c218c89e0e2eb..a2527425f398b 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -450,6 +450,13 @@ struct iommu_hwpt_vtd_s1 {
>   * nested domain will translate the same as the nesting parent. The S1 will
>   * install a Context Descriptor Table pointing at userspace memory translated
>   * by the nesting parent.
> + *
> + * Notes
> + * - when Cfg=translate, a vdevice must be allocated prior to attaching to the
> + *   allocated nested domain, as CD/ATS invalidations and vevents need a vSID.
> + * - when Cfg=bypass/abort, vdevice is not required to attach to the allocated
> + *   nested domain. This particularly works for a GBPA case, when CR0.SMMUEN=0
> + *   in the guest VM.
>   */
>  struct iommu_hwpt_arm_smmuv3 {
>  	__aligned_le64 ste[2];
> -- 
> 2.43.0
> 

Thanks,
Praan