Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

PATCH In review Manish Jaggi 7 participants 8 messages Apr 13, 2015 – Apr 16, 2015

Share & tools

Manish Jaggi

author

2015-04-13 07:37 UTC #0 lore raw

cover Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI

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

Signature · 1 line

1.7.9.5

Julien Grall

2015-04-13 10:19 UTC #1 lore raw

Hi Manish,

Quoted 3 lines · 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.

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.

Quoted 9 lines

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,

Signature · 1 line

Julien Grall

Julien Grall

2015-04-13 10:21 UTC #2 lore raw

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,

Quoted 32 lines · Julien Grall

Hi Manish,
Quoted 3 lines · 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.
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.
Quoted 9 lines
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,

Signature · 1 line

Julien Grall

Jaggi, Manish

2015-04-14 01:12 UTC #3 lore raw

In reply to Julien Grall

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

Hi Manish,

Quoted 3 lines · 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.

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.

Quoted 9 lines

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

Jan Beulich

2015-04-14 09:07 UTC #6 lore raw

In reply to Manish Jaggi

Quoted 10 lines

Quoted 1 line
Quoted 1 line · On 13.04.15 at 09:37, <[email protected]> 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.

Quoted 4 lines

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

Stefano Stabellini

2015-04-14 09:34 UTC #4 lore raw

In reply to Jaggi, Manish

Quoted 14 lines · Manish

Hi Julien,
From: Julien Grall <[email protected]>
Sent: Monday, April 13, 2015 3:49 PM
To: Jaggi, Manish; xen Devel; Stefano Stabellini; Julien Grall; Ian Campbell; Kumar, Vijaya; [email protected]
Subject: Re: [Xen-devel] [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI
Hi Manish,
Quoted 3 lines · 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.

Quoted 7 lines

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.

Ian Campbell

2015-04-16 15:27 UTC #7 lore raw

In reply to Manish Jaggi

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.

Quoted 45 lines

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

Ian Campbell

2015-04-16 15:28 UTC #5 lore raw

In reply to Stefano Stabellini

Quoted 21 lines · Stefano Stabellini

Quoted 20 lines · Manish
Hi Julien,
From: Julien Grall <[email protected]>
Sent: Monday, April 13, 2015 3:49 PM
To: Jaggi, Manish; xen Devel; Stefano Stabellini; Julien Grall; Ian Campbell; Kumar, Vijaya; [email protected]
Subject: Re: [Xen-devel] [PATCH 0/2] Re-factoring passthrough/pci.c and adding place-holder code for ARM/PCI
Hi Manish,
Quoted 3 lines · 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.
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.

Quoted 8 lines

Quoted 1 line
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.