[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC 06/11] fwnode xen spacific changes
On 06/03/18 10:27, Manish Jaggi wrote:
On 01/19/2018 12:21 AM, Julien Grall wrote:
Hi Manish,
Please use scripts/get_maintainers.pl to CC relevant maintainers. I
have done it for you this time.
Title: s/spacific/specific/
On 02/01/18 09:28, manish.jaggi@xxxxxxxxxx wrote:
From: Manish Jaggi <manish.jaggi@xxxxxxxxxx>
Merge few more changes from linux kernel code (v4.14) into iommu.c
Modify code specifc to xen.
I appreciate you pick-up the series from Sameer. I would also have
appreciated if you have addressed my remarks from there.
Sameer explain why he imported fwnode. This has been dropped here. Also,
I think you probably want a bit more context in the commit message
about implement fwnode.h in common code.
Within this series, fwnode seems to only be used by Arm. So what would
be the advantage to get that in xen/? Is it going to be used by x86 or
taken advantage in common code?
Do you want patch code (iommu.h iommu.c) to be moved to xen/arch/arm
In patch 4, i have created a new file xen/include/xen/fwnode.h
Should I move it to asm-arm ?
Please read my e-mail and answer the question. If you answer by no, then
move it to asm-arm. If you answer by yes, then write the rationale in
the commit message.
It looks like to me we might only need that in Arm, but I wanted your
input as the author of the patch. You likely have a reason to put this
code here, am I right?
Cheers,
Signed-off-by: Manish Jaggi <manish.jaggi@xxxxxxxxxx>
---
xen/drivers/passthrough/iommu.c | 75
+++++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/device.h | 11 ++++--
xen/include/xen/iommu.h | 22 ++++++++++++
3 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/xen/drivers/passthrough/iommu.c
b/xen/drivers/passthrough/iommu.c
index 1aecf7cf34..408f44106d 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -13,6 +13,7 @@
*/
#include <xen/sched.h>
+#include <xen/fwnode.h>
#include <xen/iommu.h>
#include <xen/paging.h>
#include <xen/guest_access.h>
@@ -507,6 +508,80 @@ static void iommu_dump_p2m_table(unsigned char key)
}
}
+/**
+ * fwnode_handle_put - Drop reference to a device node
+ * @fwnode: Pointer to the device node to drop the reference to.
+ *
+ * This has to be used when terminating device_for_each_child_node()
iteration
+ * with break or return to prevent stale device node references from
being left
+ * behind.
+ */
+void fwnode_handle_put(struct fwnode_handle *fwnode)
+{
+ fwnode_call_void_op(fwnode, put);
This file is following Xen coding style. And therefore you should use
Xen coding.
+}
+
+const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle
*fwnode)
+{
+ return iommu_get_ops();
+}
+
+int iommu_fwspec_init(struct device *dev, struct fwnode_handle
*iommu_fwnode,
+ const struct iommu_ops *ops)
+{
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+
+ if (fwspec)
+ return ops == fwspec->ops ? 0 : -EINVAL;
+
+ fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
You define kzalloc in a later patch and hence break the build. *All*
the patches should build one by one to help bisectability.
But given the side of the code and the fact you are going to fix the
coding style. It might be easier to use Xen name here.
+ if (!fwspec)
+ return -ENOMEM;
+#if 0
+ of_node_get(to_of_node(iommu_fwnode));
+#endif
+ fwspec->iommu_fwnode = iommu_fwnode; > + fwspec->ops = ops;
+ dev->iommu_fwspec = fwspec;
+ return 0;
+}
+
+void iommu_fwspec_free(struct device *dev)
+{
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+
+ if (fwspec) {
+ fwnode_handle_put(fwspec->iommu_fwnode);
+ kfree(fwspec);
+ dev->iommu_fwspec = NULL;
+ }
+}
+
+int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
+{
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ size_t size;
+ int i;
+
+ if (!fwspec)
+ return -EINVAL;
+
+ size = offsetof(struct iommu_fwspec, ids[fwspec->num_ids +
num_ids]);
+ if (size > sizeof(*fwspec)) {
+ //TBD: fwspec = krealloc(dev->iommu_fwspec, size,
GFP_KERNEL);
Hmmm?
+ if (!fwspec)
+ return -ENOMEM;
+
+ dev->iommu_fwspec = fwspec;
+ }
+
+ for (i = 0; i < num_ids; i++)
+ fwspec->ids[fwspec->num_ids + i] = ids[i];
+
+ fwspec->num_ids += num_ids;
+ return 0;
+
+}
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
index 6734ae8efd..f78482ca0c 100644
--- a/xen/include/asm-arm/device.h
+++ b/xen/include/asm-arm/device.h
@@ -6,6 +6,8 @@
enum device_type
{
DEV_DT,
+ DEV_ACPI,
You don't use DEV_ACPI in this patch. So why is there?
+ DEV_PCI,
};
struct dev_archdata {
@@ -18,8 +20,13 @@ struct device
enum device_type type;
#ifdef CONFIG_HAS_DEVICE_TREE
struct dt_device_node *of_node; /* Used by drivers imported
from Linux */
As said on Sameer's patches, I was expecting a todo in the code after
the discussion about leave of_node here.
I might have missed your comment on sameers patch, could you please restate
+#endif
+#ifdef CONFIG_ACPI
+ void *acpi_node;
You don't use acpi_node in this patch. So why is it there?
#endif
struct dev_archdata archdata;
+ struct fwnode_handle *fwnode; /* firmware device node */
Ditto.
+ struct iommu_fwspec *iommu_fwspec;
};
typedef struct device device_t;
@@ -27,8 +34,8 @@ typedef struct device device_t;
#include <xen/device_tree.h>
/* TODO: Correctly implement dev_is_pci when PCI is supported on
ARM */
-#define dev_is_pci(dev) ((void)(dev), 0)
-#define dev_is_dt(dev) ((dev->type == DEV_DT)
+#define dev_is_pci(dev) (dev->type == DEV_PCI)
+#define dev_is_dt(dev) (dev->type == DEV_DT)
Those two changes does not belong to this patch. It is likely 2
separate patches:
1# fixing dev_is_dt because of the missing parenthese
2# implementing dev_is_dt
enum device_class
{
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 33c8b221dc..56b169bae9 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -208,4 +208,26 @@ DECLARE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
extern struct spinlock iommu_pt_cleanup_lock;
extern struct page_list_head iommu_pt_cleanup_list;
+/**
+ * struct iommu_fwspec - per-device IOMMU instance data
+ * @ops: ops for this device's IOMMU
+ * @iommu_fwnode: firmware handle for this device's IOMMU
+ * @iommu_priv: IOMMU driver private data for this device
+ * @num_ids: number of associated device IDs
+ * @ids: IDs which this device may present to the IOMMU
+ */
+struct iommu_fwspec {
+ const struct iommu_ops *ops;
+ struct fwnode_handle *iommu_fwnode;
+ void *iommu_priv;
+ unsigned int num_ids;
+ u32 ids[1];
+};
+
+int iommu_fwspec_init(struct device *dev, struct fwnode_handle
*iommu_fwnode,
+ const struct iommu_ops *ops);
+void iommu_fwspec_free(struct device *dev);
+int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
+const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle
*fwnode);
+
#endif /* _IOMMU_H_ */
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
