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

[Julien Grall <julien.grall@xxxxxxxxxx>]

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
> > > > 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.
> > > 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
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel