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

Re: [Xen-devel] [PATCH for 4.6 07/13] xen: Introduce a generic way to describe device



Hi Jan,

On 17/12/2014 10:16, Jan Beulich wrote:
On 16.12.14 at 21:08, <julien.grall@xxxxxxxxxx> wrote:
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -2,6 +2,7 @@ obj-y += bitmap.o
  obj-y += core_parking.o
  obj-y += cpu.o
  obj-y += cpupool.o
+obj-y += device.o

Shouldn't this instead be two lines, one using HAS_PCI and the second
HAS_DEVICE_TREE?

When ARM will gain PCI will, it will fail to compile because device.o is included twice.


@@ -75,8 +76,19 @@ struct pci_dev {
  #define PT_FAULT_THRESHOLD 10
      } fault;
      u64 vf_rlen[6];
+
+    struct device dev;
  };

I'm not convinced yet that growing this structure (of which we have
quite many instances on some systems) is really worth it, in particular
on x86 where we (so far) only have one device type anyway.

Actually this will growing by only sizeof (enum type) on x86.

Having a generic way to describe device will really help ARM code (see IOMMU).

If we don't have a such thing, we may need to duplicate quite a lots of code. Which will make hard to maintain.

+#define pci_to_dev(pcidev)  (&(pcidev)->dev)
+
+static inline struct pci_dev *dev_to_pci(struct device *dev)
+{
+    ASSERT(dev->type == DEV_PCI);
+
+    return container_of(dev, struct pci_dev, dev);
+}

While the former is const-correct, I dislike the inability of passing
pointers to const into helper functions like the latter. I can't think
of a good solution other than introducing a second const variant
of it, but I suppose we should try to find alternatives before
adding such a construct that moves us in a direction opposite to
getting our code more const-correct.

Oh right. I didn't though about that case. I will turn this inline function into a macro.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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