linux-kernel - Re: [PATCH 2/2] iommu/arm-smmu-v3: Report stalled S2 events

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: <e5a8e78e-2459-453a-b3b7-e1ed2ca4addc@arm.com>
Date: Tue, 13 Aug 2024 12:57:02 +0100
From: Robin Murphy <robin.murphy@....com>
To: Mostafa Saleh <smostafa@...gle.com>, linux-kernel@...r.kernel.org,
 iommu@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 will@...nel.org, joro@...tes.org
Cc: jgg@...pe.ca, nicolinc@...dia.com, mshavit@...gle.com
Subject: Re: [PATCH 2/2] iommu/arm-smmu-v3: Report stalled S2 events

On 12/08/2024 9:52 pm, Mostafa Saleh wrote:
> Previously, S2 stall was disabled and in case there was an event it
> wouldn't be reported on the assumption that it's always pinned  by VFIO.
> 
> However, now since we can enable stall, devices that use S2 outside
> VFIO should be able to report the stalls similar to S1.
> 
> Also, to keep the old behaviour were S2 events from nested domains were
> not reported as they are pinned (from VFIO) add a new flag to track this.
> 
> Signed-off-by: Mostafa Saleh <smostafa@...gle.com>
> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 18 +++++++++++++-----
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  2 ++
>   2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 8d573d9ca93c..ffa865529d73 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1733,6 +1733,7 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
>   	u32 sid = FIELD_GET(EVTQ_0_SID, evt[0]);
>   	struct iopf_fault fault_evt = { };
>   	struct iommu_fault *flt = &fault_evt.fault;
> +	struct arm_smmu_domain *smmu_domain;
>   
>   	switch (FIELD_GET(EVTQ_0_ID, evt[0])) {
>   	case EVT_ID_TRANSLATION_FAULT:
> @@ -1744,10 +1745,6 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
>   		return -EOPNOTSUPP;
>   	}
>   
> -	/* Stage-2 is always pinned at the moment */
> -	if (evt[1] & EVTQ_1_S2)
> -		return -EFAULT;
> -
>   	if (!(evt[1] & EVTQ_1_STALL))
>   		return -EOPNOTSUPP;
>   
> @@ -1782,6 +1779,15 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
>   		goto out_unlock;
>   	}
>   
> +	/* It is guaranteed that smmu_domain exists as EVTQ_1_STALL is checked. */
> +	smmu_domain = to_smmu_domain(iommu_get_domain_for_dev(master->dev));
> +
> +	/* nesting domain is always pinned at the moment */
> +	if (smmu_domain->enable_nesting) {

Ugh, has the whole enable_nesting method still not gone away already?

However, at least for now, isn't this functionally equivalent to just 
testing !(smmu->features & ARM_SMMU_FEAT_TRANS_S1) anyway? We still 
won't be able to differentiate a nominally-pinned non-nested VFIO domain 
from a nominally-stallable non-VFIO domain on S2-only hardware.

Thanks,
Robin.

> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
>   	iommu_report_device_fault(master->dev, &fault_evt);
>   out_unlock:
>   	mutex_unlock(&smmu->streams_mutex);
> @@ -3373,8 +3379,10 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
>   	mutex_lock(&smmu_domain->init_mutex);
>   	if (smmu_domain->smmu)
>   		ret = -EPERM;
> -	else
> +	else {
>   		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
> +		smmu_domain->enable_nesting = true;
> +	}
>   	mutex_unlock(&smmu_domain->init_mutex);
>   
>   	return ret;
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 0dc7ad43c64c..f66efeec2cf8 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -745,6 +745,8 @@ struct arm_smmu_domain {
>   	spinlock_t			devices_lock;
>   
>   	struct mmu_notifier		mmu_notifier;
> +
> +	bool				enable_nesting;
>   };
>   
>   /* The following are exposed for testing purposes. */