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

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> > >      Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >      Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > >      CC: Keir Fraser <keir@xxxxxxx>
> > >      CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > >
> > > ----
> > >
> > > 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel