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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.12.14 at 11:30, <julien.grall@xxxxxxxxxx> wrote:
> 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.

Not necessarily: If we don't do this already, we should eliminate
duplicates from $(obj-y) just like Linux does.

>>> @@ -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.

No, by 8 bytes (due to padding).

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

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

Jan


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