On 2023/10/12 21:39, Jason Gunthorpe wrote:
On Thu, Oct 12, 2023 at 05:11:09PM +0800, Yi Liu wrote:
On 2023/10/11 00:58, Jason Gunthorpe wrote:
On Thu, Sep 21, 2023 at 12:51:22AM -0700, Yi Liu wrote:
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 660dc1931dc9..12e12e5563e6 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -14,6 +14,7 @@
#include <linux/err.h>
#include <linux/of.h>
#include <uapi/linux/iommu.h>
+#include <uapi/linux/iommufd.h>
Oh we should definately avoid doing that!
Maybe this is a good moment to start a new header file exclusively for
iommu drivers and core subsystem to include?
include/linux/iommu-driver.h
?
Put iommu_copy_user_data() and struct iommu_user_data in there
Avoid this include in this file.
sure. btw. seems all the user of this API and structure are in the
drivers/iommu directory. can we just putting them in
drivers/iommu/iommu-priv.h?
iommu-priv.h should be private to the core iommu code, and we sort of
extended it to iommufd as well.
iommu-driver.h would be "private" to the core and all the drivers
only.
As include ../.. is often frown on at large scale it is probably
better to be in include/linux
got it.
Just one concern. There are other paths (like cache_invalidate of
this series and Nic's set_dev_data) uses this struct as well. I'm
a bit worrying if it is good to put type here as type is meaningful
for the domain_alloc_user path.
There is always a type though? I haven't got that far in the series
yet to see..
not really. Below the users of the struct iommu_user_data in my current
iommufd_nesting branch. Only the domain_alloc_user op has type as there
can be multiple vendor specific alloc data types. Basically, I'm ok to
make the change you suggested, just not sure if it is good to add type
as it is only needed by one path.
/* Domain allocation and freeing by the iommu driver */
struct iommu_domain *(*domain_alloc)(unsigned iommu_domain_type);
struct iommu_domain *(*domain_alloc_user)(struct device *dev, u32 flags,
enum iommu_hwpt_type hwpt_type,
struct iommu_domain *parent,
const struct iommu_user_data *user_data);
int (*set_dev_user_data)(struct device *dev,
const struct iommu_user_data *user_data);
void (*unset_dev_user_data)(struct device *dev);
int (*cache_invalidate_user)(struct iommu_domain *domain,
struct iommu_user_data_array *array,
u32 *error_code);
above code snippet is from below file:
https://github.com/yiliu1765/iommufd/blob/iommufd_nesting/include/linux/iommu.h
--
Regards,
Yi Liu
