[PATCH] viommu: return error code for unknown probe type

[PATCH] viommu: return error code for unknown probe type

[flyingpenghao]

From: Peng Hao <flyingpeng@tencent.com>

If the probe types processed by the loop are all unknown,
no error will be returned.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 drivers/iommu/virtio-iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 04048f64a2c0..80352505b762 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -555,6 +555,7 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
 			break;
 		default:
 			dev_err(dev, "unknown viommu prop 0x%x\n", type);
+			ret = -EINVAL;
 		}
 
 		if (ret)
-- 
2.31.1

Re: [PATCH] viommu: return error code for unknown probe type

[Jean-Philippe Brucker]

Hello,

On Mon, Apr 15, 2024 at 04:18:23PM +0800, flyingpenghao@gmail.com wrote:
> From: Peng Hao <flyingpeng@tencent.com>
> 
> If the probe types processed by the loop are all unknown,
> no error will be returned.
> 
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> ---
>  drivers/iommu/virtio-iommu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 04048f64a2c0..80352505b762 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -555,6 +555,7 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
>  			break;
>  		default:
>  			dev_err(dev, "unknown viommu prop 0x%x\n", type);
> +			ret = -EINVAL;

The intent here is only to warn (and it should probably be replaced with
dev_warn()). I don't think unrecognized properties should cause the
endpoint probe to fail, they just indicate that the device supports newer
features.

The virtio-iommu specification states this in 5.13.6.7.1 Driver
Requirements: PROBE request: 
"If the driver doesn’t recognize the type of a property, it SHOULD ignore
 the property."

Ideally, new PROBE properties are introduced alongside a virtio feature
bit, and the device doesn't present them in the PROBE buffer if that
feature isn't negotiated during device initialization. However we don't
yet have an example of such extension. Did you encounter this problem on
existing systems?

Thanks,
Jean

>  		}
>  
>  		if (ret)
> -- 
> 2.31.1
>