Re: Issue With Patch Compilation Fails ( xen/arm: Introduce a generic way to describe device) with HAS_PCI and HAS_PASSTHROUGH.

[Manish Jaggi <mjaggi@caviumnetworks.com>]

On Wednesday 08 April 2015 11:05 AM, Manish Jaggi wrote:
>
> On Tuesday 07 April 2015 10:13 PM, Stefano Stabellini wrote:
>> On Tue, 7 Apr 2015, Jaggi, Manish wrote:
>>> Hi Julien,
>>>
>>>
>>> Following patch generated compiler error when HAS_PCI adn 
>>> HAS_PASSTHROUGH enabled.
>>> Please advice how to fix this issue, or you can revert this patch.
>>> Should I add a device structure in pci_dev or there is another way.
>> Hello Manish,
>>
>> we have never really built Xen on ARM with HAS_PCI=y so it is normal
>> that it won't compile out of the box, it is not just a problem caused by
>> the commit below.
> The problem with the patch is it introduces two different structures 
> for device for x86 and arm. While x86 device = pci_dev, for ARM there 
> is a proper device structure and != pci_dev.
> So the compilation failure is by design.
>>   I imagine that you'll need to do more than setting
>> HAS_PCI to y in order to get PCI and PCI passthrough working properly
>> with Xen on ARM.  Feel free to go ahead and propose any changes
>> necessary.
> ok
The source of the problem is reusing code of two functions
- reassign_device
- assign_device
Earlier code has dt_ variants of these two functions.

Now the point is simple, should there be redundancy of two functions OR
change a lot of code in common file drivers/passthrough/pci.c and add 
pci_to_dev macros in all platform_ops calls ?

There are lot of issues with pci_to_dev approach
a) iommu_ops callbacks have a pci_dev parameter in x86 but have a device 
parameter in arm (smmu.c)
b) hack is done to make device as pci_dev and that is not a good way of 
doing.

I prefer having minimal/some redundancy of two functions rather than 
changing a lot of code.
So IMHO revert this patch.

>>
>> Cheers,
>>
>> Stefano
>>
>>
>>>      xen/arm: Introduce a generic way to describe device
>>>           Currently, Xen is supporting PCI and Platform device 
>>> (based on Device Tree).
>>>          While Xen only supports Platform device on ARM, Xen will 
>>> gain support of
>>>      PCI soon.
>>>           Some drivers, such as IOMMU drivers, may handle PCI and 
>>> platform device in
>>>      the same way. Only few lines of code differs.
>>>           Rather than requesting to provide 2 set of functions (one 
>>> for PCI and
>>>      one for platform device), introduce a generic structure 
>>> "device" which
>>>      is embedded in each specialized device.
>>>           As x86 only supports PCI, introduce a new type device_t 
>>> which will be an
>>>      alias to pci_dev for this architecture. It will avoid to add a 
>>> new field
>>>      for this place.
>>>           Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>>      Acked-by: Jan Beulich <jbeulich@suse.com>
>>>      Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>      CC: Keir Fraser <keir@xen.org>
>>>      CC: Andrew Cooper <andrew.cooper3@citrix.com>
>>>
>>> ----
>>>
>>> Compilation error
>>> pci.c: In function ‘iommu_add_device’:
>>> pci.c:1263:5: error: implicit declaration of function ‘pci_to_dev’ 
>>> [-Werror=implicit-function-declaration]
>>> pci.c:1263:5: error: nested extern declaration of ‘pci_to_dev’ 
>>> [-Werror=nested-externs]
>>> pci.c:1263:5: error: passing argument 2 of 
>>> ‘hd->platform_ops->add_device’ makes pointer from integer without a 
>>> cast [-Werror]
>>> pci.c:1263:5: note: expected ‘struct device_t *’ but argument is of 
>>> type ‘int’
>>> pci.c:1272:9: error: passing argument 2 of 
>>> ‘hd->platform_ops->add_device’ makes pointer from integer without a 
>>> cast [-Werror]
>>> pci.c:1272:9: note: expected ‘struct device_t *’ but argument is of 
>>> type ‘int’
>>>
>>>
>>> Regards,
>>> Manish Jaggi
>


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