[PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Manish Jaggi]

Xen currently does not have PCI support for ARM builds. This patch set
makes the code compilable for ARM PCI and adds places-holder code
which would be replaced with PCI pass-through support patch series.

Re-factor MSI Handling
-------------
There is a some x86 specific code which is found in common code:
xen/drivers/passthrough/pci.c which needs to be re factored.

MSI/X are configured and handled by dom0 or domU code on ARM64 and is not
required to be part of common code. However there are functions which are
used as part of common code and calls to these functions cannot be easily
re factored like pci_cleanup_msi.

xen/drivers/passthrough/<arch>/pci.c files handle these functions.

Add ARM PCI Support
---------------
a) Place holder functions are added for pci_conf_read/write calls.
b) Macros dev_is_pci, pci_to_dev are implemented in
drivers/passthrough/pci/arm code

Manish Jaggi (2):
   xen/x86: Patch re-factors MSI/X config code from
     drivers/passthrough/pci.c to x86 specific
   xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code

  xen/arch/arm/Makefile                |    1 +
  xen/arch/arm/pci.c                   |   60 ++++++++++++++++++
  xen/drivers/passthrough/arm/Makefile |    1 +
  xen/drivers/passthrough/arm/pci.c    |   88 ++++++++++++++++++++++++++
  xen/drivers/passthrough/arm/smmu.c   |    1 -
  xen/drivers/passthrough/pci.c        |  111 +++-----------------------------
  xen/drivers/passthrough/x86/Makefile |    1 +
  xen/drivers/passthrough/x86/pci.c    |  115 ++++++++++++++++++++++++++++++++++
  xen/include/asm-arm/device.h         |   33 +++++++---
  xen/include/asm-arm/domain.h         |    3 +
  xen/include/asm-arm/pci.h            |    7 ++-
  xen/include/asm-x86/msi.h            |    1 -
  xen/include/xen/pci.h                |   20 +++++-
  13 files changed, 323 insertions(+), 119 deletions(-)
  create mode 100644 xen/arch/arm/pci.c
  create mode 100644 xen/drivers/passthrough/arm/pci.c
  create mode 100644 xen/drivers/passthrough/x86/pci.c

-- 
1.7.9.5

Re: [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Julien Grall]

Hi Manish,

On 13/04/15 08:37, Manish Jaggi wrote:
> Xen currently does not have PCI support for ARM builds. This patch set
> makes the code compilable for ARM PCI and adds places-holder code
> which would be replaced with PCI pass-through support patch series.

May I ask why you did send directly all the code to support PCI on ARM?

Without the rest it's hard to tell whether these patches make sense or not.

> Re-factor MSI Handling
> -------------
> There is a some x86 specific code which is found in common code:
> xen/drivers/passthrough/pci.c which needs to be re factored.
> 
> MSI/X are configured and handled by dom0 or domU code on ARM64 and is not
> required to be part of common code. However there are functions which are
> used as part of common code and calls to these functions cannot be easily
> re factored like pci_cleanup_msi.

On x86, the hypervisor is taking care of MSI (enabling/disabling in the
config space) as long as doing sanity check on MSI used by a given domain.

How do you plan to handle it on ARM? IHMO, the MSI code is very useful
and would also help GIC MSI implement (ITS + V2M).

FWIW, I've pointed out the same issue on the ITS series a couple of
weeks ago.

Regards,

-- 
Julien Grall

Re: [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Julien Grall]

BTW, your series is not threaded.

I've already said it on another series. Please look at [1] to see how to
send correctly your series.

[1]
http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Sending_the_patches_to_the_list

Regards,

On 13/04/15 11:19, Julien Grall wrote:
> Hi Manish,
> 
> On 13/04/15 08:37, Manish Jaggi wrote:
>> Xen currently does not have PCI support for ARM builds. This patch set
>> makes the code compilable for ARM PCI and adds places-holder code
>> which would be replaced with PCI pass-through support patch series.
> 
> May I ask why you did send directly all the code to support PCI on ARM?
> 
> Without the rest it's hard to tell whether these patches make sense or not.
> 
>> Re-factor MSI Handling
>> -------------
>> There is a some x86 specific code which is found in common code:
>> xen/drivers/passthrough/pci.c which needs to be re factored.
>>
>> MSI/X are configured and handled by dom0 or domU code on ARM64 and is not
>> required to be part of common code. However there are functions which are
>> used as part of common code and calls to these functions cannot be easily
>> re factored like pci_cleanup_msi.
> 
> On x86, the hypervisor is taking care of MSI (enabling/disabling in the
> config space) as long as doing sanity check on MSI used by a given domain.
> 
> How do you plan to handle it on ARM? IHMO, the MSI code is very useful
> and would also help GIC MSI implement (ITS + V2M).
> 
> FWIW, I've pointed out the same issue on the ITS series a couple of
> weeks ago.
> 
> Regards,
> 


-- 
Julien Grall

Re: [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Jaggi, Manish]

Hi Julien,
________________________________________
From: Julien Grall <julien.grall.oss@gmail.com>
Sent: Monday, April 13, 2015 3:49 PM
To: Jaggi, Manish; xen Devel; Stefano Stabellini; Julien Grall; Ian Campbell; Kumar, Vijaya; Prasun.kapoor@cavium.com
Subject: Re: [Xen-devel] [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

Hi Manish,

On 13/04/15 08:37, Manish Jaggi wrote:
> Xen currently does not have PCI support for ARM builds. This patch set
> makes the code compilable for ARM PCI and adds places-holder code
> which would be replaced with PCI pass-through support patch series.

May I ask why you did send directly all the code to support PCI on ARM?

Without the rest it's hard to tell whether these patches make sense or not.
[Manish] As I have mentioned these two patches are only for refactoring msi specific  functions to x86 files and providing the constructs that make PCI ARM code compilable.

The code for PCI ARM is very basic in the patch and PCI passthrough code patches will follow in next series. Please let me know which code does not makes sense and I will modify it.

> Re-factor MSI Handling
> -------------
> There is a some x86 specific code which is found in common code:
> xen/drivers/passthrough/pci.c which needs to be re factored.
>
> MSI/X are configured and handled by dom0 or domU code on ARM64 and is not
> required to be part of common code. However there are functions which are
> used as part of common code and calls to these functions cannot be easily
> re factored like pci_cleanup_msi.

On x86, the hypervisor is taking care of MSI (enabling/disabling in the
config space) as long as doing sanity check on MSI used by a given domain.

How do you plan to handle it on ARM? IHMO, the MSI code is very useful
and would also help GIC MSI implement (ITS + V2M).

[manish] As we had discussed in Linaro Connect and over the mailing list MSIs (LPIs in ARMv8) are handled directly on ARM by the guest and hypervisor is only doing injection of the LPIs using GIVv3 emulation. 

FWIW, I've pointed out the same issue on the ITS series a couple of
weeks ago.

Regards,

--
Julien Grall

Re: [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Jan Beulich]

>>> On 13.04.15 at 09:37, <mjaggi@caviumnetworks.com> wrote:
> Xen currently does not have PCI support for ARM builds. This patch set
> makes the code compilable for ARM PCI and adds places-holder code
> which would be replaced with PCI pass-through support patch series.
> 
> Re-factor MSI Handling
> -------------
> There is a some x86 specific code which is found in common code:
> xen/drivers/passthrough/pci.c which needs to be re factored.
> 
> MSI/X are configured and handled by dom0 or domU code on ARM64 and is not
> required to be part of common code. However there are functions which are
> used as part of common code and calls to these functions cannot be easily
> re factored like pci_cleanup_msi.

I can only second Julien's reply here: Without explanation (here,
not by reference to some past discussion) how you envision MSI
to work securely when you leave control of it to DomU-s there's
very little point in separating out MSI from PCI code.

Also please make sure you Cc all involved maintainers especially
on non- trivial and potentially controversial patches like these.

Jan

Re: [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Stefano Stabellini]

On Tue, 14 Apr 2015, Jaggi, Manish wrote:
> Hi Julien,
>
> From: Julien Grall <julien.grall.oss@gmail.com>
> Sent: Monday, April 13, 2015 3:49 PM
> To: Jaggi, Manish; xen Devel; Stefano Stabellini; Julien Grall; Ian Campbell; Kumar, Vijaya; Prasun.kapoor@cavium.com
> Subject: Re: [Xen-devel] [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI
> 
> Hi Manish,
> 
> On 13/04/15 08:37, Manish Jaggi wrote:
> > Xen currently does not have PCI support for ARM builds. This patch set
> > makes the code compilable for ARM PCI and adds places-holder code
> > which would be replaced with PCI pass-through support patch series.
> 
> May I ask why you did send directly all the code to support PCI on ARM?
> 
> Without the rest it's hard to tell whether these patches make sense or not.
>
> [Manish] As I have mentioned these two patches are only for refactoring msi specific  functions to x86 files and providing the constructs that make PCI ARM code compilable.
> 
> The code for PCI ARM is very basic in the patch and PCI passthrough code patches will follow in next series. Please let me know which code does not makes sense and I will modify it.

On the face if it, it makes sense, and I understand that you are doing
this to just build Xen on ARM with HAS_PCI.

However it is hard to understand the full picture, how all the pieces
fit together, without the rest. I think it would make sense to place
these two patches at the beginning of your longer series.

Re: [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Ian Campbell]

On Mon, 2015-04-13 at 13:07 +0530, Manish Jaggi wrote:

Please use git send-email (or an equivalent manual procedure) such that
the two patches appear in the same threads and not as two more
independent threads.

On this occasion I found I only had 0/2 and 2/2 filed away in my queue,
so I had to waste time going to locate the other bit, this makes me
grumpy right before I'm about to look at your code.

Secondly since this touches more than just arm specific code you need to
CC all of the relevant maintainers. Using ./script/get_maintainer.pl can
help with this.

> Xen currently does not have PCI support for ARM builds. This patch set
> makes the code compilable for ARM PCI and adds places-holder code
> which would be replaced with PCI pass-through support patch series.
> 
> Re-factor MSI Handling
> -------------
> There is a some x86 specific code which is found in common code:
> xen/drivers/passthrough/pci.c which needs to be re factored.
> 
> MSI/X are configured and handled by dom0 or domU code on ARM64 and is not
> required to be part of common code. However there are functions which are
> used as part of common code and calls to these functions cannot be easily
> re factored like pci_cleanup_msi.
> 
> xen/drivers/passthrough/<arch>/pci.c files handle these functions.
> 
> Add ARM PCI Support
> ---------------
> a) Place holder functions are added for pci_conf_read/write calls.
> b) Macros dev_is_pci, pci_to_dev are implemented in
> drivers/passthrough/pci/arm code
> 
> Manish Jaggi (2):
>    xen/x86: Patch re-factors MSI/X config code from
>      drivers/passthrough/pci.c to x86 specific
>    xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code
> 
>   xen/arch/arm/Makefile                |    1 +
>   xen/arch/arm/pci.c                   |   60 ++++++++++++++++++
>   xen/drivers/passthrough/arm/Makefile |    1 +
>   xen/drivers/passthrough/arm/pci.c    |   88 ++++++++++++++++++++++++++
>   xen/drivers/passthrough/arm/smmu.c   |    1 -
>   xen/drivers/passthrough/pci.c        |  111 +++-----------------------------
>   xen/drivers/passthrough/x86/Makefile |    1 +
>   xen/drivers/passthrough/x86/pci.c    |  115 ++++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/device.h         |   33 +++++++---
>   xen/include/asm-arm/domain.h         |    3 +
>   xen/include/asm-arm/pci.h            |    7 ++-
>   xen/include/asm-x86/msi.h            |    1 -
>   xen/include/xen/pci.h                |   20 +++++-
>   13 files changed, 323 insertions(+), 119 deletions(-)
>   create mode 100644 xen/arch/arm/pci.c
>   create mode 100644 xen/drivers/passthrough/arm/pci.c
>   create mode 100644 xen/drivers/passthrough/x86/pci.c
> 

Re: [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

[Ian Campbell]

On Tue, 2015-04-14 at 10:34 +0100, Stefano Stabellini wrote:
> On Tue, 14 Apr 2015, Jaggi, Manish wrote:
> > Hi Julien,
> >
> > From: Julien Grall <julien.grall.oss@gmail.com>
> > Sent: Monday, April 13, 2015 3:49 PM
> > To: Jaggi, Manish; xen Devel; Stefano Stabellini; Julien Grall; Ian Campbell; Kumar, Vijaya; Prasun.kapoor@cavium.com
> > Subject: Re: [Xen-devel] [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI
> > 
> > Hi Manish,
> > 
> > On 13/04/15 08:37, Manish Jaggi wrote:
> > > Xen currently does not have PCI support for ARM builds. This patch set
> > > makes the code compilable for ARM PCI and adds places-holder code
> > > which would be replaced with PCI pass-through support patch series.
> > 
> > May I ask why you did send directly all the code to support PCI on ARM?
> > 
> > Without the rest it's hard to tell whether these patches make sense or not.
> >
> > [Manish] As I have mentioned these two patches are only for refactoring msi specific  functions to x86 files and providing the constructs that make PCI ARM code compilable.
> > 
> > The code for PCI ARM is very basic in the patch and PCI passthrough code patches will follow in next series. Please let me know which code does not makes sense and I will modify it.
> 
> On the face if it, it makes sense, and I understand that you are doing
> this to just build Xen on ARM with HAS_PCI.
> 
> However it is hard to understand the full picture, how all the pieces
> fit together, without the rest. I think it would make sense to place
> these two patches at the beginning of your longer series.

I agree.