On Wednesday 08 April 2015 02:53 PM, Stefano Stabellini wrote:
On Wed, 8 Apr 2015, Manish Jaggi wrote:
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 ?
It doesn't look like we need such a big change, just few more pci_to_dev
and dev_is_pci calls under drivers/passthrough.
Certainly better than duplicating assign_device and reassign_device.


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.
Code duplication is harmful and it is not that minimal. In any case it
is good to have a common arch-independent infrastructure. However if you
don't like 6c5d3075d97ebe26661df063ee95b14168ad10f7, that's fine, you
can come up with a generalization that works better.

It is working now, I added a few changes in pci_dev->arch and device and implemented pci_to_dev for arm. For a short term this is ok, I have a generalization of individual device.h to include/xen/device.h. Will send that after merging of PCI passthrough patches.
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.gr...@linaro.org>
      Acked-by: Jan Beulich <jbeul...@suse.com>
      Acked-by: Stefano Stabellini <stefano.stabell...@eu.citrix.com>
      CC: Keir Fraser <k...@xen.org>
      CC: Andrew Cooper <andrew.coop...@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

Reply via email to