[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 17:17, Jan Beulich wrote:
Julien Grall <julien.grall@xxxxxxxxxx> 12/17/14 2:04 PM >>>
On 17/12/14 10:46, Jan Beulich wrote:
On 17.12.14 at 11:30, <julien.grall@xxxxxxxxxx> wrote:
Having a generic way to describe device will really help ARM code (see

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

Not really, if e.g. "device" was simply an alias of "pci_dev" on x86.

How many pci_dev instance you could have on a platform? 1000? Though it
might be a high value but that mean we use 2k more of RAM.

Sure the total amount isn't big. But these days everyone thinks that way, and
data size gets grown without much consideration. And you shouldn't just think
about RAM cache utilization.

I will go ahead with the aliasing.

It doesn't seem to bad for the benefit to have a clear code.

Aliasing device and pci_dev for x86 would yield similar clarity afaict.

To be sure, by aliasing you mean creating a typedef?

For x86:
typedef struct pci_dev device_t;

And for ARM:
typedef struct device device_t;

+#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.

I'm afraid that won't help, as you still need to specify a type as
2nd argument to container_of(), and that type can't be both
const and non-const at the same time, i.e. you can't easily
inherit the const-ness of the passed in pointer.

I agree that we will drop the const-ness. But is it really an issue?

We won't have many place where we don't want to modify the pci_dev.

Did you check (including places where const could be added)? But at least
you didn't have to drop and const-s, so I'm not heavily objecting the change
you propose.

The only usage will be in the IOMMU code where most of the time we require a non const version.

At least on the SMMU driver, we have to store data per-device. This is to know what is the SMMU master for this device.

Julien Grall

Xen-devel mailing list



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