[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 2/4] xen: do not return -EEXIST if iommu_add_dt_device is called twice

Hi Jan,

On 23/07/2021 14:02, Jan Beulich wrote:
On 23.07.2021 11:28, Julien Grall wrote:
Hi Jan,

On 23/07/2021 07:31, Jan Beulich wrote:
On 23.07.2021 01:36, Stefano Stabellini wrote:
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -140,8 +140,13 @@ int iommu_add_dt_device(struct dt_device_node *np)
       if ( !ops )
           return -EINVAL;
+ /*
+     * Some Device Trees may expose both legacy SMMU and generic
+     * IOMMU bindings together. If both are present, the device
+     * can be already added.
+     */
       if ( dev_iommu_fwspec_get(dev) )
-        return -EEXIST;
+        return 0;

Since the xen: prefix in the subject made me go look (I wouldn't have
if it had been e.g. dt: ), I may as well ask: Since previously there
was concern about bogus duplicate entries, does this concern go away
no altogether?

The check wasn't originally added because of legacy vs generic binding.

It was added because in some circumstances iommu_add_dt_device() could
genuinely be called twice (for instance if the device is re-assigned).
This was returning -EEXIST rather than 0 so the caller can decide
whether it is normal that the device is already added.

Okay. If that distinction is of no interest anymore, then I can see
this wanting dropping.

Calling iommu_add_dt_device() twice doesn't hurt but after patch #1
(this patch should really be first), dev_iommu_fwspec_get() will return
a non-NULL pointer as the legacy devices are added when the IOMMU is probed.

It's one thing for there to be a legacy and a generic
binding, but another if you found two legacy or two generic ones, I
would think.

I am not quite too sure what you mean by "two legacy" and "two generic".
Can you clarify it?

Well, I'm having trouble describing it in different terms. I mean
two entries of the same kind (both legacy or both generic) referring
to the same device, thus leading to the function recognizing the 2nd > time 
round that the device is already there.

I think you are misunderstanding the purpose of this function. It is called when we discover a new device rather than discovering a new entry in the IOMMU. The function will then sort out what to do for the device.

The legacy binding is somewhat specific because it bypass the function as the discovering is done per IOMMU rather than per device.


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.