[v7,02/12] iommu/arm-smmu-v3: Remove unrecoverable faults reporting

Message ID 20231115030226.16700-3-baolu.lu@linux.intel.com
State New
Headers
Series iommu: Prepare to deliver page faults to user space |

Commit Message

Baolu Lu Nov. 15, 2023, 3:02 a.m. UTC
  No device driver registers fault handler to handle the reported
unrecoveraable faults. Remove it to avoid dead code.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 46 ++++++---------------
 1 file changed, 13 insertions(+), 33 deletions(-)
  

Comments

Jason Gunthorpe Dec. 1, 2023, 3:42 p.m. UTC | #1
On Wed, Nov 15, 2023 at 11:02:16AM +0800, Lu Baolu wrote:
> No device driver registers fault handler to handle the reported
> unrecoveraable faults. Remove it to avoid dead code.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 46 ++++++---------------
>  1 file changed, 13 insertions(+), 33 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

If we do bring this back it will be in some form where the opaque
driver event information is delivered to userspace to forward to the
VM.

Jason
  
Yi Liu Dec. 4, 2023, 10:54 a.m. UTC | #2
On 2023/11/15 11:02, Lu Baolu wrote:
> No device driver registers fault handler to handle the reported
> unrecoveraable faults. Remove it to avoid dead code.

I noticed only ARM code is removed. So intel iommu driver does not have
code that tries to report unrecoveraable faults?

> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 46 ++++++---------------
>   1 file changed, 13 insertions(+), 33 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 7445454c2af2..505400538a2e 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1463,7 +1463,6 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
>   static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
>   {
>   	int ret;
> -	u32 reason;
>   	u32 perm = 0;
>   	struct arm_smmu_master *master;
>   	bool ssid_valid = evt[0] & EVTQ_0_SSV;
> @@ -1473,16 +1472,9 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
>   
>   	switch (FIELD_GET(EVTQ_0_ID, evt[0])) {
>   	case EVT_ID_TRANSLATION_FAULT:
> -		reason = IOMMU_FAULT_REASON_PTE_FETCH;
> -		break;
>   	case EVT_ID_ADDR_SIZE_FAULT:
> -		reason = IOMMU_FAULT_REASON_OOR_ADDRESS;
> -		break;
>   	case EVT_ID_ACCESS_FAULT:
> -		reason = IOMMU_FAULT_REASON_ACCESS;
> -		break;
>   	case EVT_ID_PERMISSION_FAULT:
> -		reason = IOMMU_FAULT_REASON_PERMISSION;
>   		break;
>   	default:
>   		return -EOPNOTSUPP;
> @@ -1492,6 +1484,9 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
>   	if (evt[1] & EVTQ_1_S2)
>   		return -EFAULT;
>   
> +	if (!(evt[1] & EVTQ_1_STALL))
> +		return -EOPNOTSUPP;
> +
>   	if (evt[1] & EVTQ_1_RnW)
>   		perm |= IOMMU_FAULT_PERM_READ;
>   	else
> @@ -1503,32 +1498,17 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
>   	if (evt[1] & EVTQ_1_PnU)
>   		perm |= IOMMU_FAULT_PERM_PRIV;
>   
> -	if (evt[1] & EVTQ_1_STALL) {
> -		flt->type = IOMMU_FAULT_PAGE_REQ;
> -		flt->prm = (struct iommu_fault_page_request) {
> -			.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
> -			.grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
> -			.perm = perm,
> -			.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
> -		};
> +	flt->type = IOMMU_FAULT_PAGE_REQ;
> +	flt->prm = (struct iommu_fault_page_request) {
> +		.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
> +		.grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
> +		.perm = perm,
> +		.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
> +	};
>   
> -		if (ssid_valid) {
> -			flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> -			flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
> -		}
> -	} else {
> -		flt->type = IOMMU_FAULT_DMA_UNRECOV;
> -		flt->event = (struct iommu_fault_unrecoverable) {
> -			.reason = reason,
> -			.flags = IOMMU_FAULT_UNRECOV_ADDR_VALID,
> -			.perm = perm,
> -			.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
> -		};
> -
> -		if (ssid_valid) {
> -			flt->event.flags |= IOMMU_FAULT_UNRECOV_PASID_VALID;
> -			flt->event.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
> -		}
> +	if (ssid_valid) {
> +		flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> +		flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
>   	}
>   
>   	mutex_lock(&smmu->streams_mutex);
  
Baolu Lu Dec. 5, 2023, 11:48 a.m. UTC | #3
On 2023/12/4 18:54, Yi Liu wrote:
> On 2023/11/15 11:02, Lu Baolu wrote:
>> No device driver registers fault handler to handle the reported
>> unrecoveraable faults. Remove it to avoid dead code.
> 
> I noticed only ARM code is removed. So intel iommu driver does not have
> code that tries to report unrecoveraable faults?

Yes.

Best regards,
baolu
  

Patch

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 7445454c2af2..505400538a2e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1463,7 +1463,6 @@  arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
 static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
 {
 	int ret;
-	u32 reason;
 	u32 perm = 0;
 	struct arm_smmu_master *master;
 	bool ssid_valid = evt[0] & EVTQ_0_SSV;
@@ -1473,16 +1472,9 @@  static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
 
 	switch (FIELD_GET(EVTQ_0_ID, evt[0])) {
 	case EVT_ID_TRANSLATION_FAULT:
-		reason = IOMMU_FAULT_REASON_PTE_FETCH;
-		break;
 	case EVT_ID_ADDR_SIZE_FAULT:
-		reason = IOMMU_FAULT_REASON_OOR_ADDRESS;
-		break;
 	case EVT_ID_ACCESS_FAULT:
-		reason = IOMMU_FAULT_REASON_ACCESS;
-		break;
 	case EVT_ID_PERMISSION_FAULT:
-		reason = IOMMU_FAULT_REASON_PERMISSION;
 		break;
 	default:
 		return -EOPNOTSUPP;
@@ -1492,6 +1484,9 @@  static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
 	if (evt[1] & EVTQ_1_S2)
 		return -EFAULT;
 
+	if (!(evt[1] & EVTQ_1_STALL))
+		return -EOPNOTSUPP;
+
 	if (evt[1] & EVTQ_1_RnW)
 		perm |= IOMMU_FAULT_PERM_READ;
 	else
@@ -1503,32 +1498,17 @@  static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
 	if (evt[1] & EVTQ_1_PnU)
 		perm |= IOMMU_FAULT_PERM_PRIV;
 
-	if (evt[1] & EVTQ_1_STALL) {
-		flt->type = IOMMU_FAULT_PAGE_REQ;
-		flt->prm = (struct iommu_fault_page_request) {
-			.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
-			.grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
-			.perm = perm,
-			.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
-		};
+	flt->type = IOMMU_FAULT_PAGE_REQ;
+	flt->prm = (struct iommu_fault_page_request) {
+		.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
+		.grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
+		.perm = perm,
+		.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
+	};
 
-		if (ssid_valid) {
-			flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
-			flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
-		}
-	} else {
-		flt->type = IOMMU_FAULT_DMA_UNRECOV;
-		flt->event = (struct iommu_fault_unrecoverable) {
-			.reason = reason,
-			.flags = IOMMU_FAULT_UNRECOV_ADDR_VALID,
-			.perm = perm,
-			.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
-		};
-
-		if (ssid_valid) {
-			flt->event.flags |= IOMMU_FAULT_UNRECOV_PASID_VALID;
-			flt->event.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
-		}
+	if (ssid_valid) {
+		flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
+		flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
 	}
 
 	mutex_lock(&smmu->streams_mutex);