All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 00/10] x86 passthrough code cleanup
@ 2018-02-21 21:46 Wei Liu
  2018-02-21 21:46 ` [PATCH RFC 01/10] passthrough: rearrange x86 code Wei Liu
                   ` (11 more replies)
  0 siblings, 12 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Hi all

At some point I would like to make CONFIG_HVM and CONFIG_PV work. The
passthrough code is one of the road blocks for that work.

A short discussion on #xendevel made me think that having host side code
regardless of if HVM (the primary user) is configured is desirable because PV
guests may still have limited use of the hardware, hence this series.

What I want is to have clear hierarchy of the code and split the host side
and the guest side code, and start to use CONFIG_HVM where applicable. Luckily
the amount of work seemed to be smaller than I had expected.

RFC because there are a few open questions. Please see individual patches.

Wei.

Wei Liu (10):
  passthrough: rearrange x86 code
  passthrough: split out x86 PCI code to x86/pci.c
  x86/passthrough: io.c is used for HVM only
  x86/passthrough: arch_pci_clean_irqs is HVM only
  x86/passthrough: move hvm_dpci_isairq_eoi
  passthrough/amd: remove guest iommu support
  passthrough/amd: split out hvm code from iommu_map.c
  passthrough/amd: make clear_iommu_pte_present static
  passthrough/intel: put some code under CONFIG_HVM
  x86: check hvm domain before calling pt_irq_destroy_bind

 MAINTAINERS                                        |   8 +-
 xen/arch/x86/domctl.c                              |   4 +
 xen/drivers/passthrough/Makefile                   |   3 -
 xen/drivers/passthrough/amd/iommu_guest.c          | 927 ---------------------
 xen/drivers/passthrough/pci.c                      |  51 +-
 xen/drivers/passthrough/x86/Makefile               |   5 +
 xen/drivers/passthrough/{ => x86}/amd/Makefile     |   2 +-
 xen/drivers/passthrough/x86/amd/hvm.c              | 108 +++
 xen/drivers/passthrough/x86/amd/iommu.h            |  32 +
 xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c |   0
 xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c  |   2 +-
 .../passthrough/{ => x86}/amd/iommu_detect.c       |   0
 xen/drivers/passthrough/{ => x86}/amd/iommu_init.c |  21 +-
 xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c |   0
 xen/drivers/passthrough/{ => x86}/amd/iommu_map.c  | 107 +--
 .../passthrough/{ => x86}/amd/pci_amd_iommu.c      |   2 +-
 xen/drivers/passthrough/{ => x86}/io.c             |  45 +
 xen/drivers/passthrough/x86/pci.c                  |  74 ++
 xen/drivers/passthrough/{ => x86}/vtd/Makefile     |   0
 xen/drivers/passthrough/{ => x86}/vtd/dmar.c       |   0
 xen/drivers/passthrough/{ => x86}/vtd/dmar.h       |   0
 xen/drivers/passthrough/{ => x86}/vtd/extern.h     |   0
 xen/drivers/passthrough/{ => x86}/vtd/intremap.c   |   0
 xen/drivers/passthrough/{ => x86}/vtd/iommu.c      |  17 +-
 xen/drivers/passthrough/{ => x86}/vtd/iommu.h      |   0
 xen/drivers/passthrough/{ => x86}/vtd/qinval.c     |   2 +-
 xen/drivers/passthrough/{ => x86}/vtd/quirks.c     |   0
 xen/drivers/passthrough/{ => x86}/vtd/utils.c      |   0
 xen/drivers/passthrough/{ => x86}/vtd/vtd.h        |   0
 xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile |   0
 xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c    |   2 +-
 xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c    |  45 -
 xen/include/asm-x86/amd-iommu.h                    |  51 --
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h      |   8 -
 xen/include/asm-x86/iommu.h                        |   1 -
 xen/include/xen/iommu.h                            |   1 -
 xen/include/xen/pci.h                              |   2 +
 37 files changed, 310 insertions(+), 1210 deletions(-)
 delete mode 100644 xen/drivers/passthrough/amd/iommu_guest.c
 rename xen/drivers/passthrough/{ => x86}/amd/Makefile (86%)
 create mode 100644 xen/drivers/passthrough/x86/amd/hvm.c
 create mode 100644 xen/drivers/passthrough/x86/amd/iommu.h
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c (99%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_detect.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_init.c (99%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_map.c (86%)
 rename xen/drivers/passthrough/{ => x86}/amd/pci_amd_iommu.c (99%)
 rename xen/drivers/passthrough/{ => x86}/io.c (96%)
 create mode 100644 xen/drivers/passthrough/x86/pci.c
 rename xen/drivers/passthrough/{ => x86}/vtd/Makefile (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/dmar.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/dmar.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/extern.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/intremap.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/iommu.c (99%)
 rename xen/drivers/passthrough/{ => x86}/vtd/iommu.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/qinval.c (99%)
 rename xen/drivers/passthrough/{ => x86}/vtd/quirks.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/utils.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/vtd.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c (99%)
 rename xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c (72%)

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH RFC 01/10] passthrough: rearrange x86 code
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-04-23 15:27   ` Jan Beulich
  2018-02-21 21:46 ` [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c Wei Liu
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Kevin Tian, Wei Liu, Suravee Suthikulpanit, Jan Beulich

Clean up the hierarchy of the directory: put vtd, amd and io.c under
x86. Adjust makefile and MAINTAINERS.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
 MAINTAINERS                                           | 8 ++++----
 xen/drivers/passthrough/Makefile                      | 3 ---
 xen/drivers/passthrough/x86/Makefile                  | 4 ++++
 xen/drivers/passthrough/{ => x86}/amd/Makefile        | 0
 xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c    | 0
 xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c     | 2 +-
 xen/drivers/passthrough/{ => x86}/amd/iommu_detect.c  | 0
 xen/drivers/passthrough/{ => x86}/amd/iommu_guest.c   | 0
 xen/drivers/passthrough/{ => x86}/amd/iommu_init.c    | 0
 xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c    | 0
 xen/drivers/passthrough/{ => x86}/amd/iommu_map.c     | 2 +-
 xen/drivers/passthrough/{ => x86}/amd/pci_amd_iommu.c | 2 +-
 xen/drivers/passthrough/{ => x86}/io.c                | 0
 xen/drivers/passthrough/{ => x86}/vtd/Makefile        | 0
 xen/drivers/passthrough/{ => x86}/vtd/dmar.c          | 0
 xen/drivers/passthrough/{ => x86}/vtd/dmar.h          | 0
 xen/drivers/passthrough/{ => x86}/vtd/extern.h        | 0
 xen/drivers/passthrough/{ => x86}/vtd/intremap.c      | 0
 xen/drivers/passthrough/{ => x86}/vtd/iommu.c         | 2 +-
 xen/drivers/passthrough/{ => x86}/vtd/iommu.h         | 0
 xen/drivers/passthrough/{ => x86}/vtd/qinval.c        | 2 +-
 xen/drivers/passthrough/{ => x86}/vtd/quirks.c        | 0
 xen/drivers/passthrough/{ => x86}/vtd/utils.c         | 0
 xen/drivers/passthrough/{ => x86}/vtd/vtd.h           | 0
 xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile    | 0
 xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c       | 2 +-
 xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c       | 0
 27 files changed, 14 insertions(+), 13 deletions(-)
 rename xen/drivers/passthrough/{ => x86}/amd/Makefile (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c (99%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_detect.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_guest.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_init.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c (100%)
 rename xen/drivers/passthrough/{ => x86}/amd/iommu_map.c (99%)
 rename xen/drivers/passthrough/{ => x86}/amd/pci_amd_iommu.c (99%)
 rename xen/drivers/passthrough/{ => x86}/io.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/Makefile (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/dmar.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/dmar.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/extern.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/intremap.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/iommu.c (99%)
 rename xen/drivers/passthrough/{ => x86}/vtd/iommu.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/qinval.c (99%)
 rename xen/drivers/passthrough/{ => x86}/vtd/quirks.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/utils.c (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/vtd.h (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile (100%)
 rename xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c (99%)
 rename xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index e4070ffb80..6500ba2ac2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -143,7 +143,7 @@ F:	tools/libacpi/
 AMD IOMMU
 M:	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
 S:	Maintained
-F:	xen/drivers/passthrough/amd/
+F:	xen/drivers/passthrough/x86/amd/
 
 AMD SVM
 M:	Boris Ostrovsky <boris.ostrovsky@oracle.com>
@@ -222,7 +222,7 @@ F:	xen/include/asm-x86/tboot.h
 INTEL(R) VT FOR DIRECTED I/O (VT-D)
 M:	Kevin Tian <kevin.tian@intel.com>
 S:	Supported
-F:	xen/drivers/passthrough/vtd/
+F:	xen/drivers/passthrough/x86/vtd/
 
 INTEL(R) VT FOR X86 (VT-X)
 M:	Jun Nakajima <jun.nakajima@intel.com>
@@ -237,10 +237,10 @@ IOMMU VENDOR INDEPENDENT CODE
 M:	Jan Beulich <jbeulich@suse.com>
 S:	Supported
 F:	xen/drivers/passthrough/
-X:	xen/drivers/passthrough/amd/
 X:	xen/drivers/passthrough/arm/
-X:	xen/drivers/passthrough/vtd/
 X:	xen/drivers/passthrough/device_tree.c
+X:	xen/drivers/passthrough/x86/amd/
+X:	xen/drivers/passthrough/x86/vtd/
 F:	xen/include/xen/iommu.h
 
 KCONFIG
diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index 6087333a34..4b698bd566 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -1,9 +1,6 @@
-subdir-$(CONFIG_X86) += vtd
-subdir-$(CONFIG_X86) += amd
 subdir-$(CONFIG_X86) += x86
 subdir-$(CONFIG_ARM) += arm
 
 obj-y += iommu.o
-obj-$(CONFIG_X86) += io.o
 obj-$(CONFIG_HAS_PCI) += pci.o
 obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
diff --git a/xen/drivers/passthrough/x86/Makefile b/xen/drivers/passthrough/x86/Makefile
index a70cf9460d..06971707f8 100644
--- a/xen/drivers/passthrough/x86/Makefile
+++ b/xen/drivers/passthrough/x86/Makefile
@@ -1,2 +1,6 @@
+subdir-y += vtd
+subdir-y += amd
+
 obj-y += ats.o
+obj-y += io.o
 obj-y += iommu.o
diff --git a/xen/drivers/passthrough/amd/Makefile b/xen/drivers/passthrough/x86/amd/Makefile
similarity index 100%
rename from xen/drivers/passthrough/amd/Makefile
rename to xen/drivers/passthrough/x86/amd/Makefile
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/x86/amd/iommu_acpi.c
similarity index 100%
rename from xen/drivers/passthrough/amd/iommu_acpi.c
rename to xen/drivers/passthrough/x86/amd/iommu_acpi.c
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/x86/amd/iommu_cmd.c
similarity index 99%
rename from xen/drivers/passthrough/amd/iommu_cmd.c
rename to xen/drivers/passthrough/x86/amd/iommu_cmd.c
index 08247fa354..a2948fca46 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/x86/amd/iommu_cmd.c
@@ -20,7 +20,7 @@
 #include <xen/sched.h>
 #include <asm/amd-iommu.h>
 #include <asm/hvm/svm/amd-iommu-proto.h>
-#include "../ats.h"
+#include "../../ats.h"
 
 static int queue_iommu_command(struct amd_iommu *iommu, u32 cmd[])
 {
diff --git a/xen/drivers/passthrough/amd/iommu_detect.c b/xen/drivers/passthrough/x86/amd/iommu_detect.c
similarity index 100%
rename from xen/drivers/passthrough/amd/iommu_detect.c
rename to xen/drivers/passthrough/x86/amd/iommu_detect.c
diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/x86/amd/iommu_guest.c
similarity index 100%
rename from xen/drivers/passthrough/amd/iommu_guest.c
rename to xen/drivers/passthrough/x86/amd/iommu_guest.c
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/x86/amd/iommu_init.c
similarity index 100%
rename from xen/drivers/passthrough/amd/iommu_init.c
rename to xen/drivers/passthrough/x86/amd/iommu_init.c
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/x86/amd/iommu_intr.c
similarity index 100%
rename from xen/drivers/passthrough/amd/iommu_intr.c
rename to xen/drivers/passthrough/x86/amd/iommu_intr.c
diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/x86/amd/iommu_map.c
similarity index 99%
rename from xen/drivers/passthrough/amd/iommu_map.c
rename to xen/drivers/passthrough/x86/amd/iommu_map.c
index fd2327d3e5..0f9bd538af 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/x86/amd/iommu_map.c
@@ -22,7 +22,7 @@
 #include <asm/p2m.h>
 #include <asm/amd-iommu.h>
 #include <asm/hvm/svm/amd-iommu-proto.h>
-#include "../ats.h"
+#include "../../ats.h"
 #include <xen/pci.h>
 
 /* Given pfn and page table level, return pde index */
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/x86/amd/pci_amd_iommu.c
similarity index 99%
rename from xen/drivers/passthrough/amd/pci_amd_iommu.c
rename to xen/drivers/passthrough/x86/amd/pci_amd_iommu.c
index 12d2695b89..385d595bc0 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/x86/amd/pci_amd_iommu.c
@@ -25,7 +25,7 @@
 #include <xen/softirq.h>
 #include <asm/amd-iommu.h>
 #include <asm/hvm/svm/amd-iommu-proto.h>
-#include "../ats.h"
+#include "../../ats.h"
 
 static bool_t __read_mostly init_done;
 
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/x86/io.c
similarity index 100%
rename from xen/drivers/passthrough/io.c
rename to xen/drivers/passthrough/x86/io.c
diff --git a/xen/drivers/passthrough/vtd/Makefile b/xen/drivers/passthrough/x86/vtd/Makefile
similarity index 100%
rename from xen/drivers/passthrough/vtd/Makefile
rename to xen/drivers/passthrough/x86/vtd/Makefile
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/x86/vtd/dmar.c
similarity index 100%
rename from xen/drivers/passthrough/vtd/dmar.c
rename to xen/drivers/passthrough/x86/vtd/dmar.c
diff --git a/xen/drivers/passthrough/vtd/dmar.h b/xen/drivers/passthrough/x86/vtd/dmar.h
similarity index 100%
rename from xen/drivers/passthrough/vtd/dmar.h
rename to xen/drivers/passthrough/x86/vtd/dmar.h
diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/x86/vtd/extern.h
similarity index 100%
rename from xen/drivers/passthrough/vtd/extern.h
rename to xen/drivers/passthrough/x86/vtd/extern.h
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/x86/vtd/intremap.c
similarity index 100%
rename from xen/drivers/passthrough/vtd/intremap.c
rename to xen/drivers/passthrough/x86/vtd/intremap.c
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/x86/vtd/iommu.c
similarity index 99%
rename from xen/drivers/passthrough/vtd/iommu.c
rename to xen/drivers/passthrough/x86/vtd/iommu.c
index daaed0abbd..1d161fe149 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/x86/vtd/iommu.c
@@ -39,7 +39,7 @@
 #include "dmar.h"
 #include "extern.h"
 #include "vtd.h"
-#include "../ats.h"
+#include "../../ats.h"
 
 struct mapped_rmrr {
     struct list_head list;
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/x86/vtd/iommu.h
similarity index 100%
rename from xen/drivers/passthrough/vtd/iommu.h
rename to xen/drivers/passthrough/x86/vtd/iommu.h
diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/x86/vtd/qinval.c
similarity index 99%
rename from xen/drivers/passthrough/vtd/qinval.c
rename to xen/drivers/passthrough/x86/vtd/qinval.c
index e95dc54a8d..d2dace638a 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/x86/vtd/qinval.c
@@ -27,7 +27,7 @@
 #include "dmar.h"
 #include "vtd.h"
 #include "extern.h"
-#include "../ats.h"
+#include "../../ats.h"
 
 #define VTD_QI_TIMEOUT	1
 
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/x86/vtd/quirks.c
similarity index 100%
rename from xen/drivers/passthrough/vtd/quirks.c
rename to xen/drivers/passthrough/x86/vtd/quirks.c
diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/x86/vtd/utils.c
similarity index 100%
rename from xen/drivers/passthrough/vtd/utils.c
rename to xen/drivers/passthrough/x86/vtd/utils.c
diff --git a/xen/drivers/passthrough/vtd/vtd.h b/xen/drivers/passthrough/x86/vtd/vtd.h
similarity index 100%
rename from xen/drivers/passthrough/vtd/vtd.h
rename to xen/drivers/passthrough/x86/vtd/vtd.h
diff --git a/xen/drivers/passthrough/vtd/x86/Makefile b/xen/drivers/passthrough/x86/vtd/x86/Makefile
similarity index 100%
rename from xen/drivers/passthrough/vtd/x86/Makefile
rename to xen/drivers/passthrough/x86/vtd/x86/Makefile
diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/x86/vtd/x86/ats.c
similarity index 99%
rename from xen/drivers/passthrough/vtd/x86/ats.c
rename to xen/drivers/passthrough/x86/vtd/x86/ats.c
index 1a3adb4acb..4332819136 100644
--- a/xen/drivers/passthrough/vtd/x86/ats.c
+++ b/xen/drivers/passthrough/x86/vtd/x86/ats.c
@@ -26,7 +26,7 @@
 #include "../dmar.h"
 #include "../vtd.h"
 #include "../extern.h"
-#include "../../ats.h"
+#include "../../../ats.h"
 
 static LIST_HEAD(ats_dev_drhd_units);
 
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/x86/vtd/x86/vtd.c
similarity index 100%
rename from xen/drivers/passthrough/vtd/x86/vtd.c
rename to xen/drivers/passthrough/x86/vtd/x86/vtd.c
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
  2018-02-21 21:46 ` [PATCH RFC 01/10] passthrough: rearrange x86 code Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-02-26 10:57   ` Julien Grall
  2018-04-23 15:34   ` Jan Beulich
  2018-02-21 21:46 ` [PATCH RFC 03/10] x86/passthrough: io.c is used for HVM only Wei Liu
                   ` (9 subsequent siblings)
  11 siblings, 2 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich

Move the functions that reference x86 hvm data structures to its own
file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.

There is still one location in that file which references
arch.hvm_domain, but it is fine because ARM guest is HVM.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

ARM doesn't select HAS_PCI, that's why ARM build is not broken by
this. AIUI ARM will select HAS_PCI at some point, hence I only move
the x86 bits.
---
 xen/drivers/passthrough/pci.c        | 51 +----------------------------
 xen/drivers/passthrough/x86/Makefile |  1 +
 xen/drivers/passthrough/x86/pci.c    | 62 ++++++++++++++++++++++++++++++++++++
 xen/include/xen/pci.h                |  2 ++
 4 files changed, 66 insertions(+), 50 deletions(-)
 create mode 100644 xen/drivers/passthrough/x86/pci.c

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 2b976ade62..bf83d07279 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -22,7 +22,6 @@
 #include <xen/iommu.h>
 #include <xen/irq.h>
 #include <xen/vm_event.h>
-#include <asm/hvm/irq.h>
 #include <xen/delay.h>
 #include <xen/keyhandler.h>
 #include <xen/event.h>
@@ -798,54 +797,6 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
     return ret;
 }
 
-static int pci_clean_dpci_irq(struct domain *d,
-                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
-{
-    struct dev_intx_gsi_link *digl, *tmp;
-
-    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
-
-    if ( pt_irq_need_timer(pirq_dpci->flags) )
-        kill_timer(&pirq_dpci->timer);
-
-    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
-    {
-        list_del(&digl->list);
-        xfree(digl);
-    }
-
-    return pt_pirq_softirq_active(pirq_dpci) ? -ERESTART : 0;
-}
-
-static int pci_clean_dpci_irqs(struct domain *d)
-{
-    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
-
-    if ( !iommu_enabled )
-        return 0;
-
-    if ( !is_hvm_domain(d) )
-        return 0;
-
-    spin_lock(&d->event_lock);
-    hvm_irq_dpci = domain_get_irq_dpci(d);
-    if ( hvm_irq_dpci != NULL )
-    {
-        int ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
-
-        if ( ret )
-        {
-            spin_unlock(&d->event_lock);
-            return ret;
-        }
-
-        hvm_domain_irq(d)->dpci = NULL;
-        free_hvm_irq_dpci(hvm_irq_dpci);
-    }
-    spin_unlock(&d->event_lock);
-    return 0;
-}
-
 int pci_release_devices(struct domain *d)
 {
     struct pci_dev *pdev;
@@ -853,7 +804,7 @@ int pci_release_devices(struct domain *d)
     int ret;
 
     pcidevs_lock();
-    ret = pci_clean_dpci_irqs(d);
+    ret = arch_pci_clean_irqs(d);
     if ( ret )
     {
         pcidevs_unlock();
diff --git a/xen/drivers/passthrough/x86/Makefile b/xen/drivers/passthrough/x86/Makefile
index 06971707f8..0a21b60b5a 100644
--- a/xen/drivers/passthrough/x86/Makefile
+++ b/xen/drivers/passthrough/x86/Makefile
@@ -4,3 +4,4 @@ subdir-y += amd
 obj-y += ats.o
 obj-y += io.o
 obj-y += iommu.o
+obj-y += pci.o
diff --git a/xen/drivers/passthrough/x86/pci.c b/xen/drivers/passthrough/x86/pci.c
new file mode 100644
index 0000000000..e0a7e473b1
--- /dev/null
+++ b/xen/drivers/passthrough/x86/pci.c
@@ -0,0 +1,62 @@
+#include <xen/irq.h>
+#include <xen/iommu.h>
+#include <xen/sched.h>
+
+#include <asm/hvm/irq.h>
+
+static int pci_clean_dpci_irq(struct domain *d,
+                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
+{
+    struct dev_intx_gsi_link *digl, *tmp;
+
+    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
+
+    if ( pt_irq_need_timer(pirq_dpci->flags) )
+        kill_timer(&pirq_dpci->timer);
+
+    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
+    {
+        list_del(&digl->list);
+        xfree(digl);
+    }
+
+    return pt_pirq_softirq_active(pirq_dpci) ? -ERESTART : 0;
+}
+
+int arch_pci_clean_irqs(struct domain *d)
+{
+    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
+
+    if ( !iommu_enabled )
+        return 0;
+
+    if ( !is_hvm_domain(d) )
+        return 0;
+
+    spin_lock(&d->event_lock);
+    hvm_irq_dpci = domain_get_irq_dpci(d);
+    if ( hvm_irq_dpci != NULL )
+    {
+        int ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
+
+        if ( ret )
+        {
+            spin_unlock(&d->event_lock);
+            return ret;
+        }
+
+        hvm_domain_irq(d)->dpci = NULL;
+        free_hvm_irq_dpci(hvm_irq_dpci);
+    }
+    spin_unlock(&d->event_lock);
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index dd5ec43a70..2d3bdf386f 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -153,6 +153,8 @@ struct pci_dev *pci_get_pdev_by_domain(const struct domain *, int seg,
                                        int bus, int devfn);
 void pci_check_disable_device(u16 seg, u8 bus, u8 devfn);
 
+int arch_pci_clean_irqs(struct domain *d);
+
 uint8_t pci_conf_read8(
     unsigned int seg, unsigned int bus, unsigned int dev, unsigned int func,
     unsigned int reg);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 03/10] x86/passthrough: io.c is used for HVM only
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
  2018-02-21 21:46 ` [PATCH RFC 01/10] passthrough: rearrange x86 code Wei Liu
  2018-02-21 21:46 ` [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-04-23 15:37   ` Jan Beulich
  2018-02-21 21:46 ` [PATCH RFC 04/10] x86/passthrough: arch_pci_clean_irqs is " Wei Liu
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
This file has a few functions that are called in other places. We need
to provide stubs for them at some point. Currently the declarations
are in different places. What is the preferred name / location for the
stubs?
---
 xen/drivers/passthrough/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/x86/Makefile b/xen/drivers/passthrough/x86/Makefile
index 0a21b60b5a..02659bd9f5 100644
--- a/xen/drivers/passthrough/x86/Makefile
+++ b/xen/drivers/passthrough/x86/Makefile
@@ -2,6 +2,6 @@ subdir-y += vtd
 subdir-y += amd
 
 obj-y += ats.o
-obj-y += io.o
+obj-$(CONFIG_HVM) += io.o
 obj-y += iommu.o
 obj-y += pci.o
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 04/10] x86/passthrough: arch_pci_clean_irqs is HVM only
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (2 preceding siblings ...)
  2018-02-21 21:46 ` [PATCH RFC 03/10] x86/passthrough: io.c is used for HVM only Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-02-21 21:46 ` [PATCH RFC 05/10] x86/passthrough: move hvm_dpci_isairq_eoi Wei Liu
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Provide a !CONFIG_HVM stub in preparation for future usage.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/passthrough/x86/pci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/xen/drivers/passthrough/x86/pci.c b/xen/drivers/passthrough/x86/pci.c
index e0a7e473b1..cdba905466 100644
--- a/xen/drivers/passthrough/x86/pci.c
+++ b/xen/drivers/passthrough/x86/pci.c
@@ -4,6 +4,8 @@
 
 #include <asm/hvm/irq.h>
 
+#ifdef CONFIG_HVM
+
 static int pci_clean_dpci_irq(struct domain *d,
                               struct hvm_pirq_dpci *pirq_dpci, void *arg)
 {
@@ -52,6 +54,16 @@ int arch_pci_clean_irqs(struct domain *d)
     return 0;
 }
 
+#else
+
+int arch_pci_clean_irqs(struct domain *d)
+{
+    ASSERT(!is_hvm_domain(d));
+    return 0;
+}
+
+#endif
+
 /*
  * Local variables:
  * mode: C
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 05/10] x86/passthrough: move hvm_dpci_isairq_eoi
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (3 preceding siblings ...)
  2018-02-21 21:46 ` [PATCH RFC 04/10] x86/passthrough: arch_pci_clean_irqs is " Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-04-23 15:39   ` Jan Beulich
  2018-02-21 21:46 ` [PATCH RFC 06/10] passthrough/amd: remove guest iommu support Wei Liu
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Wei Liu, Jan Beulich

This function is not Intel specific. Move it to io.c along side its
sole user. Remove declaration in iommu.h.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Kevin Tian <kevin.tian@intel.com>
---
 xen/drivers/passthrough/x86/io.c          | 45 +++++++++++++++++++++++++++++++
 xen/drivers/passthrough/x86/vtd/x86/vtd.c | 45 -------------------------------
 xen/include/xen/iommu.h                   |  1 -
 3 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/xen/drivers/passthrough/x86/io.c b/xen/drivers/passthrough/x86/io.c
index 8f16e6c0a5..6a7c6415dc 100644
--- a/xen/drivers/passthrough/x86/io.c
+++ b/xen/drivers/passthrough/x86/io.c
@@ -51,6 +51,51 @@ enum {
     STATE_RUN
 };
 
+static int _hvm_dpci_isairq_eoi(struct domain *d,
+                                struct hvm_pirq_dpci *pirq_dpci, void *arg)
+{
+    struct hvm_irq *hvm_irq = hvm_domain_irq(d);
+    unsigned int isairq = (long)arg;
+    const struct dev_intx_gsi_link *digl;
+
+    list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
+    {
+        unsigned int link = hvm_pci_intx_link(digl->device, digl->intx);
+
+        if ( hvm_irq->pci_link.route[link] == isairq )
+        {
+            hvm_pci_intx_deassert(d, digl->device, digl->intx);
+            if ( --pirq_dpci->pending == 0 )
+            {
+                stop_timer(&pirq_dpci->timer);
+                pirq_guest_eoi(dpci_pirq(pirq_dpci));
+            }
+        }
+    }
+
+    return 0;
+}
+
+static void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
+{
+    struct hvm_irq_dpci *dpci = NULL;
+
+    ASSERT(isairq < NR_ISAIRQS);
+    if ( !iommu_enabled)
+        return;
+
+    spin_lock(&d->event_lock);
+
+    dpci = domain_get_irq_dpci(d);
+
+    if ( dpci && test_bit(isairq, dpci->isairq_map) )
+    {
+        /* Multiple mirq may be mapped to one isa irq */
+        pt_pirq_iterate(d, _hvm_dpci_isairq_eoi, (void *)(long)isairq);
+    }
+    spin_unlock(&d->event_lock);
+}
+
 /*
  * This can be called multiple times, but the softirq is only raised once.
  * That is until the STATE_SCHED state has been cleared. The state can be
diff --git a/xen/drivers/passthrough/x86/vtd/x86/vtd.c b/xen/drivers/passthrough/x86/vtd/x86/vtd.c
index 88a60b3307..c7823be4e8 100644
--- a/xen/drivers/passthrough/x86/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/x86/vtd/x86/vtd.c
@@ -63,51 +63,6 @@ void flush_all_cache()
     wbinvd();
 }
 
-static int _hvm_dpci_isairq_eoi(struct domain *d,
-                                struct hvm_pirq_dpci *pirq_dpci, void *arg)
-{
-    struct hvm_irq *hvm_irq = hvm_domain_irq(d);
-    unsigned int isairq = (long)arg;
-    const struct dev_intx_gsi_link *digl;
-
-    list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
-    {
-        unsigned int link = hvm_pci_intx_link(digl->device, digl->intx);
-
-        if ( hvm_irq->pci_link.route[link] == isairq )
-        {
-            hvm_pci_intx_deassert(d, digl->device, digl->intx);
-            if ( --pirq_dpci->pending == 0 )
-            {
-                stop_timer(&pirq_dpci->timer);
-                pirq_guest_eoi(dpci_pirq(pirq_dpci));
-            }
-        }
-    }
-
-    return 0;
-}
-
-void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
-{
-    struct hvm_irq_dpci *dpci = NULL;
-
-    ASSERT(isairq < NR_ISAIRQS);
-    if ( !iommu_enabled)
-        return;
-
-    spin_lock(&d->event_lock);
-
-    dpci = domain_get_irq_dpci(d);
-
-    if ( dpci && test_bit(isairq, dpci->isairq_map) )
-    {
-        /* Multiple mirq may be mapped to one isa irq */
-        pt_pirq_iterate(d, _hvm_dpci_isairq_eoi, (void *)(long)isairq);
-    }
-    spin_unlock(&d->event_lock);
-}
-
 void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
 {
     unsigned long i, j, tmp, top;
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 33c8b221dc..32674e6e59 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -99,7 +99,6 @@ int hvm_do_IRQ_dpci(struct domain *, struct pirq *);
 int pt_irq_create_bind(struct domain *, const struct xen_domctl_bind_pt_irq *);
 int pt_irq_destroy_bind(struct domain *, const struct xen_domctl_bind_pt_irq *);
 
-void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq);
 struct hvm_irq_dpci *domain_get_irq_dpci(const struct domain *);
 void free_hvm_irq_dpci(struct hvm_irq_dpci *dpci);
 bool_t pt_irq_need_timer(uint32_t flags);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 06/10] passthrough/amd: remove guest iommu support
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (4 preceding siblings ...)
  2018-02-21 21:46 ` [PATCH RFC 05/10] x86/passthrough: move hvm_dpci_isairq_eoi Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-05-02 17:10   ` Suravee Suthikulpanit
  2018-02-21 21:46 ` [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c Wei Liu
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Suravee Suthikulpanit, Jan Beulich

It is never used and it is getting in the way of cleaning up.

The only callsite of guest_iommu_add_ppr_log has no effect because
guest iommu is not initialised.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 xen/drivers/passthrough/x86/amd/Makefile      |   1 -
 xen/drivers/passthrough/x86/amd/iommu_guest.c | 927 --------------------------
 xen/drivers/passthrough/x86/amd/iommu_init.c  |  21 +-
 xen/include/asm-x86/amd-iommu.h               |  51 --
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |   8 -
 xen/include/asm-x86/iommu.h                   |   1 -
 6 files changed, 6 insertions(+), 1003 deletions(-)
 delete mode 100644 xen/drivers/passthrough/x86/amd/iommu_guest.c

diff --git a/xen/drivers/passthrough/x86/amd/Makefile b/xen/drivers/passthrough/x86/amd/Makefile
index 95c04ed237..415146fcdb 100644
--- a/xen/drivers/passthrough/x86/amd/Makefile
+++ b/xen/drivers/passthrough/x86/amd/Makefile
@@ -5,4 +5,3 @@ obj-y += pci_amd_iommu.o
 obj-bin-y += iommu_acpi.init.o
 obj-y += iommu_intr.o
 obj-y += iommu_cmd.o
-obj-y += iommu_guest.o
diff --git a/xen/drivers/passthrough/x86/amd/iommu_guest.c b/xen/drivers/passthrough/x86/amd/iommu_guest.c
deleted file mode 100644
index 96175bb9ac..0000000000
--- a/xen/drivers/passthrough/x86/amd/iommu_guest.c
+++ /dev/null
@@ -1,927 +0,0 @@
-/*
- * Copyright (C) 2011 Advanced Micro Devices, Inc.
- * Author: Wei Wang <wei.wang2@amd.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <xen/sched.h>
-#include <asm/p2m.h>
-#include <asm/amd-iommu.h>
-#include <asm/hvm/svm/amd-iommu-proto.h>
-
-
-#define IOMMU_MMIO_SIZE                         0x8000
-#define IOMMU_MMIO_PAGE_NR                      0x8
-#define RING_BF_LENGTH_MASK                     0x0F000000
-#define RING_BF_LENGTH_SHIFT                    24
-
-#define PASMAX_9_bit                            0x8
-#define GUEST_CR3_1_LEVEL                       0x0
-#define GUEST_ADDRESS_SIZE_6_LEVEL              0x2
-#define HOST_ADDRESS_SIZE_6_LEVEL               0x2
-
-#define guest_iommu_set_status(iommu, bit) \
-        iommu_set_bit(&((iommu)->reg_status.lo), bit)
-
-#define guest_iommu_clear_status(iommu, bit) \
-        iommu_clear_bit(&((iommu)->reg_status.lo), bit)
-
-#define reg_to_u64(reg) (((uint64_t)reg.hi << 32) | reg.lo )
-#define u64_to_reg(reg, val) \
-    do \
-    { \
-        (reg)->lo = (u32)(val); \
-        (reg)->hi = (val) >> 32; \
-    } while (0)
-
-static unsigned int machine_bdf(struct domain *d, uint16_t guest_bdf)
-{
-    return guest_bdf;
-}
-
-static uint16_t guest_bdf(struct domain *d, uint16_t machine_bdf)
-{
-    return machine_bdf;
-}
-
-static inline struct guest_iommu *domain_iommu(struct domain *d)
-{
-    return dom_iommu(d)->arch.g_iommu;
-}
-
-static inline struct guest_iommu *vcpu_iommu(struct vcpu *v)
-{
-    return dom_iommu(v->domain)->arch.g_iommu;
-}
-
-static void guest_iommu_enable(struct guest_iommu *iommu)
-{
-    iommu->enabled = 1;
-}
-
-static void guest_iommu_disable(struct guest_iommu *iommu)
-{
-    iommu->enabled = 0;
-}
-
-static uint64_t get_guest_cr3_from_dte(dev_entry_t *dte)
-{
-    uint64_t gcr3_1, gcr3_2, gcr3_3;
-
-    gcr3_1 = get_field_from_reg_u32(dte->data[1],
-                                    IOMMU_DEV_TABLE_GCR3_1_MASK,
-                                    IOMMU_DEV_TABLE_GCR3_1_SHIFT);
-    gcr3_2 = get_field_from_reg_u32(dte->data[2],
-                                    IOMMU_DEV_TABLE_GCR3_2_MASK,
-                                    IOMMU_DEV_TABLE_GCR3_2_SHIFT);
-    gcr3_3 = get_field_from_reg_u32(dte->data[3],
-                                    IOMMU_DEV_TABLE_GCR3_3_MASK,
-                                    IOMMU_DEV_TABLE_GCR3_3_SHIFT);
-
-    return ((gcr3_3 << 31) | (gcr3_2 << 15 ) | (gcr3_1 << 12)) >> PAGE_SHIFT;
-}
-
-static uint16_t get_domid_from_dte(dev_entry_t *dte)
-{
-    return get_field_from_reg_u32(dte->data[2], IOMMU_DEV_TABLE_DOMAIN_ID_MASK,
-                                  IOMMU_DEV_TABLE_DOMAIN_ID_SHIFT);
-}
-
-static uint16_t get_glx_from_dte(dev_entry_t *dte)
-{
-    return get_field_from_reg_u32(dte->data[1], IOMMU_DEV_TABLE_GLX_MASK,
-                                  IOMMU_DEV_TABLE_GLX_SHIFT);
-}
-
-static uint16_t get_gv_from_dte(dev_entry_t *dte)
-{
-    return get_field_from_reg_u32(dte->data[1],IOMMU_DEV_TABLE_GV_MASK,
-                                  IOMMU_DEV_TABLE_GV_SHIFT);
-}
-
-static unsigned int host_domid(struct domain *d, uint64_t g_domid)
-{
-    /* Only support one PPR device in guest for now */
-    return d->domain_id;
-}
-
-static unsigned long get_gfn_from_base_reg(uint64_t base_raw)
-{
-    base_raw &= PADDR_MASK;
-    ASSERT ( base_raw != 0 );
-    return base_raw >> PAGE_SHIFT;
-}
-
-static void guest_iommu_deliver_msi(struct domain *d)
-{
-    uint8_t vector, dest, dest_mode, delivery_mode, trig_mode;
-    struct guest_iommu *iommu = domain_iommu(d);
-
-    vector = iommu->msi.vector;
-    dest = iommu->msi.dest;
-    dest_mode = iommu->msi.dest_mode;
-    delivery_mode = iommu->msi.delivery_mode;
-    trig_mode = iommu->msi.trig_mode;
-
-    vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode);
-}
-
-static unsigned long guest_iommu_get_table_mfn(struct domain *d,
-                                               uint64_t base_raw,
-                                               unsigned int entry_size,
-                                               unsigned int pos)
-{
-    unsigned long idx, gfn, mfn;
-    p2m_type_t p2mt;
-
-    gfn = get_gfn_from_base_reg(base_raw);
-    idx = (pos * entry_size) >> PAGE_SHIFT;
-
-    mfn = mfn_x(get_gfn(d, gfn + idx, &p2mt));
-    put_gfn(d, gfn);
-
-    return mfn;
-}
-
-static void guest_iommu_enable_dev_table(struct guest_iommu *iommu)
-{
-    uint32_t length_raw = get_field_from_reg_u32(iommu->dev_table.reg_base.lo,
-                                                 IOMMU_DEV_TABLE_SIZE_MASK,
-                                                 IOMMU_DEV_TABLE_SIZE_SHIFT);
-    iommu->dev_table.size = (length_raw + 1) * PAGE_SIZE;
-}
-
-static void guest_iommu_enable_ring_buffer(struct guest_iommu *iommu,
-                                           struct guest_buffer *buffer,
-                                           uint32_t entry_size)
-{
-    uint32_t length_raw = get_field_from_reg_u32(buffer->reg_base.hi,
-                                                 RING_BF_LENGTH_MASK,
-                                                 RING_BF_LENGTH_SHIFT);
-    buffer->entries = 1 << length_raw;
-}
-
-void guest_iommu_add_ppr_log(struct domain *d, u32 entry[])
-{
-    uint16_t gdev_id;
-    unsigned long mfn, tail, head;
-    ppr_entry_t *log, *log_base;
-    struct guest_iommu *iommu;
-
-    if ( !is_hvm_domain(d) )
-        return;
-
-    iommu = domain_iommu(d);
-    if ( !iommu )
-        return;
-
-    tail = iommu_get_rb_pointer(iommu->ppr_log.reg_tail.lo);
-    head = iommu_get_rb_pointer(iommu->ppr_log.reg_head.lo);
-
-    if ( tail >= iommu->ppr_log.entries || head >= iommu->ppr_log.entries )
-    {
-        AMD_IOMMU_DEBUG("Error: guest iommu ppr log overflows\n");
-        guest_iommu_disable(iommu);
-        return;
-    }
-
-    mfn = guest_iommu_get_table_mfn(d, reg_to_u64(iommu->ppr_log.reg_base),
-                                    sizeof(ppr_entry_t), tail);
-    ASSERT(mfn_valid(_mfn(mfn)));
-
-    log_base = map_domain_page(_mfn(mfn));
-    log = log_base + tail % (PAGE_SIZE / sizeof(ppr_entry_t));
-
-    /* Convert physical device id back into virtual device id */
-    gdev_id = guest_bdf(d, iommu_get_devid_from_cmd(entry[0]));
-    iommu_set_devid_to_cmd(&entry[0], gdev_id);
-
-    memcpy(log, entry, sizeof(ppr_entry_t));
-
-    /* Now shift ppr log tail pointer */
-    if ( ++tail >= iommu->ppr_log.entries )
-    {
-        tail = 0;
-        guest_iommu_set_status(iommu, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT);
-    }
-    iommu_set_rb_pointer(&iommu->ppr_log.reg_tail.lo, tail);
-    unmap_domain_page(log_base);
-
-    guest_iommu_deliver_msi(d);
-}
-
-void guest_iommu_add_event_log(struct domain *d, u32 entry[])
-{
-    uint16_t dev_id;
-    unsigned long mfn, tail, head;
-    event_entry_t *log, *log_base;
-    struct guest_iommu *iommu;
-
-    if ( !is_hvm_domain(d) )
-        return;
-
-    iommu = domain_iommu(d);
-    if ( !iommu )
-        return;
-
-    tail = iommu_get_rb_pointer(iommu->event_log.reg_tail.lo);
-    head = iommu_get_rb_pointer(iommu->event_log.reg_head.lo);
-
-    if ( tail >= iommu->event_log.entries || head >= iommu->event_log.entries )
-    {
-        AMD_IOMMU_DEBUG("Error: guest iommu event overflows\n");
-        guest_iommu_disable(iommu);
-        return;
-    }
-
-    mfn = guest_iommu_get_table_mfn(d, reg_to_u64(iommu->event_log.reg_base),
-                                    sizeof(event_entry_t), tail);
-    ASSERT(mfn_valid(_mfn(mfn)));
-
-    log_base = map_domain_page(_mfn(mfn));
-    log = log_base + tail % (PAGE_SIZE / sizeof(event_entry_t));
-
-    /* re-write physical device id into virtual device id */
-    dev_id = guest_bdf(d, iommu_get_devid_from_cmd(entry[0]));
-    iommu_set_devid_to_cmd(&entry[0], dev_id);
-    memcpy(log, entry, sizeof(event_entry_t));
-
-    /* Now shift event log tail pointer */
-    if ( ++tail >= iommu->event_log.entries )
-    {
-        tail = 0;
-        guest_iommu_set_status(iommu, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT);
-    }
-
-    iommu_set_rb_pointer(&iommu->event_log.reg_tail.lo, tail);
-    unmap_domain_page(log_base);
-
-    guest_iommu_deliver_msi(d);
-}
-
-static int do_complete_ppr_request(struct domain *d, cmd_entry_t *cmd)
-{
-    uint16_t dev_id;
-    struct amd_iommu *iommu;
-
-    dev_id = machine_bdf(d, iommu_get_devid_from_cmd(cmd->data[0]));
-    iommu = find_iommu_for_device(0, dev_id);
-
-    if ( !iommu )
-    {
-        AMD_IOMMU_DEBUG("%s: Fail to find iommu for bdf %x\n",
-                        __func__, dev_id);
-        return -ENODEV;
-    }
-
-    /* replace virtual device id into physical */
-    iommu_set_devid_to_cmd(&cmd->data[0], dev_id);
-    amd_iommu_send_guest_cmd(iommu, cmd->data);
-
-    return 0;
-}
-
-static int do_invalidate_pages(struct domain *d, cmd_entry_t *cmd)
-{
-    uint16_t gdom_id, hdom_id;
-    struct amd_iommu *iommu = NULL;
-
-    gdom_id = get_field_from_reg_u32(cmd->data[1],
-                                    IOMMU_INV_IOMMU_PAGES_DOMAIN_ID_MASK,
-                                    IOMMU_INV_IOMMU_PAGES_DOMAIN_ID_SHIFT);
-
-    hdom_id = host_domid(d, gdom_id);
-    set_field_in_reg_u32(hdom_id, cmd->data[1],
-                         IOMMU_INV_IOMMU_PAGES_DOMAIN_ID_MASK,
-                         IOMMU_INV_IOMMU_PAGES_DOMAIN_ID_SHIFT, &cmd->data[1]);
-
-    for_each_amd_iommu ( iommu )
-        amd_iommu_send_guest_cmd(iommu, cmd->data);
-
-    return 0;
-}
-
-static int do_invalidate_all(struct domain *d, cmd_entry_t *cmd)
-{
-    struct amd_iommu *iommu = NULL;
-
-    for_each_amd_iommu ( iommu )
-        amd_iommu_flush_all_pages(d);
-
-    return 0;
-}
-
-static int do_invalidate_iotlb_pages(struct domain *d, cmd_entry_t *cmd)
-{
-    struct amd_iommu *iommu;
-    uint16_t dev_id;
-
-    dev_id = machine_bdf(d, iommu_get_devid_from_cmd(cmd->data[0]));
-
-    iommu = find_iommu_for_device(0, dev_id);
-    if ( !iommu )
-    {
-        AMD_IOMMU_DEBUG("%s: Fail to find iommu for bdf %x\n",
-                         __func__, dev_id);
-        return -ENODEV;
-    }
-
-    iommu_set_devid_to_cmd(&cmd->data[0], dev_id);
-    amd_iommu_send_guest_cmd(iommu, cmd->data);
-
-    return 0;
-}
-
-static int do_completion_wait(struct domain *d, cmd_entry_t *cmd)
-{
-    bool_t com_wait_int_en, com_wait_int, i, s;
-    struct guest_iommu *iommu;
-    unsigned long gfn;
-    p2m_type_t p2mt;
-
-    iommu = domain_iommu(d);
-
-    i = iommu_get_bit(cmd->data[0], IOMMU_COMP_WAIT_I_FLAG_SHIFT);
-    s = iommu_get_bit(cmd->data[0], IOMMU_COMP_WAIT_S_FLAG_SHIFT);
-
-    if ( i )
-        guest_iommu_set_status(iommu, IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
-
-    if ( s )
-    {
-        uint64_t gaddr_lo, gaddr_hi, gaddr_64, data;
-        void *vaddr;
-
-        data = (uint64_t)cmd->data[3] << 32 | cmd->data[2];
-        gaddr_lo = get_field_from_reg_u32(cmd->data[0],
-                                          IOMMU_COMP_WAIT_ADDR_LOW_MASK,
-                                          IOMMU_COMP_WAIT_ADDR_LOW_SHIFT);
-        gaddr_hi = get_field_from_reg_u32(cmd->data[1],
-                                          IOMMU_COMP_WAIT_ADDR_HIGH_MASK,
-                                          IOMMU_COMP_WAIT_ADDR_HIGH_SHIFT);
-
-        gaddr_64 = (gaddr_hi << 32) | (gaddr_lo << 3);
-
-        gfn = gaddr_64 >> PAGE_SHIFT;
-        vaddr = map_domain_page(get_gfn(d, gfn ,&p2mt));
-        put_gfn(d, gfn);
-
-        write_u64_atomic((uint64_t *)(vaddr + (gaddr_64 & (PAGE_SIZE-1))),
-                         data);
-        unmap_domain_page(vaddr);
-    }
-
-    com_wait_int_en = iommu_get_bit(iommu->reg_ctrl.lo,
-                                    IOMMU_CONTROL_COMP_WAIT_INT_SHIFT);
-    com_wait_int = iommu_get_bit(iommu->reg_status.lo,
-                                 IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
-
-    if ( com_wait_int_en && com_wait_int )
-        guest_iommu_deliver_msi(d);
-
-    return 0;
-}
-
-static int do_invalidate_dte(struct domain *d, cmd_entry_t *cmd)
-{
-    uint16_t gbdf, mbdf, req_id, gdom_id, hdom_id;
-    dev_entry_t *gdte, *mdte, *dte_base;
-    struct amd_iommu *iommu = NULL;
-    struct guest_iommu *g_iommu;
-    uint64_t gcr3_gfn, gcr3_mfn;
-    uint8_t glx, gv;
-    unsigned long dte_mfn, flags;
-    p2m_type_t p2mt;
-
-    g_iommu = domain_iommu(d);
-    gbdf = iommu_get_devid_from_cmd(cmd->data[0]);
-    mbdf = machine_bdf(d, gbdf);
-
-    /* Guest can only update DTEs for its passthru devices */
-    if ( mbdf == 0 || gbdf == 0 )
-        return 0;
-
-    /* Sometimes guest invalidates devices from non-exists dtes */
-    if ( (gbdf * sizeof(dev_entry_t)) > g_iommu->dev_table.size )
-        return 0;
-
-    dte_mfn = guest_iommu_get_table_mfn(d,
-                                        reg_to_u64(g_iommu->dev_table.reg_base),
-                                        sizeof(dev_entry_t), gbdf);
-    ASSERT(mfn_valid(_mfn(dte_mfn)));
-
-    /* Read guest dte information */
-    dte_base = map_domain_page(_mfn(dte_mfn));
-
-    gdte = dte_base + gbdf % (PAGE_SIZE / sizeof(dev_entry_t));
-
-    gdom_id  = get_domid_from_dte(gdte);
-    gcr3_gfn = get_guest_cr3_from_dte(gdte);
-    glx      = get_glx_from_dte(gdte);
-    gv       = get_gv_from_dte(gdte);
-
-    unmap_domain_page(dte_base);
-
-    /* Do not update host dte before gcr3 has been set */
-    if ( gcr3_gfn == 0 )
-        return 0;
-
-    gcr3_mfn = mfn_x(get_gfn(d, gcr3_gfn, &p2mt));
-    put_gfn(d, gcr3_gfn);
-
-    ASSERT(mfn_valid(_mfn(gcr3_mfn)));
-
-    iommu = find_iommu_for_device(0, mbdf);
-    if ( !iommu )
-    {
-        AMD_IOMMU_DEBUG("%s: Fail to find iommu for bdf %x!\n",
-                        __func__, mbdf);
-        return -ENODEV;
-    }
-
-    /* Setup host device entry */
-    hdom_id = host_domid(d, gdom_id);
-    req_id = get_dma_requestor_id(iommu->seg, mbdf);
-    mdte = iommu->dev_table.buffer + (req_id * sizeof(dev_entry_t));
-
-    spin_lock_irqsave(&iommu->lock, flags);
-    iommu_dte_set_guest_cr3((u32 *)mdte, hdom_id,
-                            gcr3_mfn << PAGE_SHIFT, gv, glx);
-
-    amd_iommu_flush_device(iommu, req_id);
-    spin_unlock_irqrestore(&iommu->lock, flags);
-
-    return 0;
-}
-
-static void guest_iommu_process_command(unsigned long _d)
-{
-    unsigned long opcode, tail, head, entries_per_page, cmd_mfn;
-    cmd_entry_t *cmd, *cmd_base;
-    struct domain *d = (struct domain *)_d;
-    struct guest_iommu *iommu;
-
-    iommu = domain_iommu(d);
-
-    if ( !iommu->enabled )
-        return;
-
-    head = iommu_get_rb_pointer(iommu->cmd_buffer.reg_head.lo);
-    tail = iommu_get_rb_pointer(iommu->cmd_buffer.reg_tail.lo);
-
-    /* Tail pointer is rolled over by guest driver, value outside
-     * cmd_buffer_entries cause iommu disabled
-     */
-
-    if ( tail >= iommu->cmd_buffer.entries ||
-         head >= iommu->cmd_buffer.entries )
-    {
-        AMD_IOMMU_DEBUG("Error: guest iommu cmd buffer overflows\n");
-        guest_iommu_disable(iommu);
-        return;
-    }
-
-    entries_per_page = PAGE_SIZE / sizeof(cmd_entry_t);
-
-    while ( head != tail )
-    {
-        int ret = 0;
-
-        cmd_mfn = guest_iommu_get_table_mfn(d,
-                                            reg_to_u64(iommu->cmd_buffer.reg_base),
-                                            sizeof(cmd_entry_t), head);
-        ASSERT(mfn_valid(_mfn(cmd_mfn)));
-
-        cmd_base = map_domain_page(_mfn(cmd_mfn));
-        cmd = cmd_base + head % entries_per_page;
-
-        opcode = get_field_from_reg_u32(cmd->data[1],
-                                        IOMMU_CMD_OPCODE_MASK,
-                                        IOMMU_CMD_OPCODE_SHIFT);
-        switch ( opcode )
-        {
-        case IOMMU_CMD_COMPLETION_WAIT:
-            ret = do_completion_wait(d, cmd);
-            break;
-        case IOMMU_CMD_INVALIDATE_DEVTAB_ENTRY:
-            ret = do_invalidate_dte(d, cmd);
-            break;
-        case IOMMU_CMD_INVALIDATE_IOMMU_PAGES:
-            ret = do_invalidate_pages(d, cmd);
-            break;
-        case IOMMU_CMD_INVALIDATE_IOTLB_PAGES:
-            ret = do_invalidate_iotlb_pages(d, cmd);
-            break;
-        case IOMMU_CMD_INVALIDATE_INT_TABLE:
-            break;
-        case IOMMU_CMD_COMPLETE_PPR_REQUEST:
-            ret = do_complete_ppr_request(d, cmd);
-            break;
-        case IOMMU_CMD_INVALIDATE_IOMMU_ALL:
-            ret = do_invalidate_all(d, cmd);
-            break;
-        default:
-            AMD_IOMMU_DEBUG("CMD: Unknown command cmd_type = %lx "
-                            "head = %ld\n", opcode, head);
-            break;
-        }
-
-        unmap_domain_page(cmd_base);
-        if ( ++head >= iommu->cmd_buffer.entries )
-            head = 0;
-        if ( ret )
-            guest_iommu_disable(iommu);
-    }
-
-    /* Now shift cmd buffer head pointer */
-    iommu_set_rb_pointer(&iommu->cmd_buffer.reg_head.lo, head);
-    return;
-}
-
-static int guest_iommu_write_ctrl(struct guest_iommu *iommu, uint64_t newctrl)
-{
-    bool_t cmd_en, event_en, iommu_en, ppr_en, ppr_log_en;
-    bool_t cmd_en_old, event_en_old, iommu_en_old;
-    bool_t cmd_run;
-
-    iommu_en = iommu_get_bit(newctrl,
-                             IOMMU_CONTROL_TRANSLATION_ENABLE_SHIFT);
-    iommu_en_old = iommu_get_bit(iommu->reg_ctrl.lo,
-                                 IOMMU_CONTROL_TRANSLATION_ENABLE_SHIFT);
-
-    cmd_en = iommu_get_bit(newctrl,
-                           IOMMU_CONTROL_COMMAND_BUFFER_ENABLE_SHIFT);
-    cmd_en_old = iommu_get_bit(iommu->reg_ctrl.lo,
-                               IOMMU_CONTROL_COMMAND_BUFFER_ENABLE_SHIFT);
-    cmd_run = iommu_get_bit(iommu->reg_status.lo,
-                            IOMMU_STATUS_CMD_BUFFER_RUN_SHIFT);
-    event_en = iommu_get_bit(newctrl,
-                             IOMMU_CONTROL_EVENT_LOG_ENABLE_SHIFT);
-    event_en_old = iommu_get_bit(iommu->reg_ctrl.lo,
-                                 IOMMU_CONTROL_EVENT_LOG_ENABLE_SHIFT);
-
-    ppr_en = iommu_get_bit(newctrl,
-                           IOMMU_CONTROL_PPR_ENABLE_SHIFT);
-    ppr_log_en = iommu_get_bit(newctrl,
-                               IOMMU_CONTROL_PPR_LOG_ENABLE_SHIFT);
-
-    if ( iommu_en )
-    {
-        guest_iommu_enable(iommu);
-        guest_iommu_enable_dev_table(iommu);
-    }
-
-    if ( iommu_en && cmd_en )
-    {
-        guest_iommu_enable_ring_buffer(iommu, &iommu->cmd_buffer,
-                                       sizeof(cmd_entry_t));
-        /* Enable iommu command processing */
-        tasklet_schedule(&iommu->cmd_buffer_tasklet);
-    }
-
-    if ( iommu_en && event_en )
-    {
-        guest_iommu_enable_ring_buffer(iommu, &iommu->event_log,
-                                       sizeof(event_entry_t));
-        guest_iommu_set_status(iommu, IOMMU_STATUS_EVENT_LOG_RUN_SHIFT);
-        guest_iommu_clear_status(iommu, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT);
-    }
-
-    if ( iommu_en && ppr_en && ppr_log_en )
-    {
-        guest_iommu_enable_ring_buffer(iommu, &iommu->ppr_log,
-                                       sizeof(ppr_entry_t));
-        guest_iommu_set_status(iommu, IOMMU_STATUS_PPR_LOG_RUN_SHIFT);
-        guest_iommu_clear_status(iommu, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT);
-    }
-
-    if ( iommu_en && cmd_en_old && !cmd_en )
-    {
-        /* Disable iommu command processing */
-        tasklet_kill(&iommu->cmd_buffer_tasklet);
-    }
-
-    if ( event_en_old && !event_en )
-        guest_iommu_clear_status(iommu, IOMMU_STATUS_EVENT_LOG_RUN_SHIFT);
-
-    if ( iommu_en_old && !iommu_en )
-        guest_iommu_disable(iommu);
-
-    u64_to_reg(&iommu->reg_ctrl, newctrl);
-    return 0;
-}
-
-static uint64_t iommu_mmio_read64(struct guest_iommu *iommu,
-                                  unsigned long offset)
-{
-    uint64_t val;
-
-    switch ( offset )
-    {
-    case IOMMU_DEV_TABLE_BASE_LOW_OFFSET:
-        val = reg_to_u64(iommu->dev_table.reg_base);
-        break;
-    case IOMMU_CMD_BUFFER_BASE_LOW_OFFSET:
-        val = reg_to_u64(iommu->cmd_buffer.reg_base);
-        break;
-    case IOMMU_EVENT_LOG_BASE_LOW_OFFSET:
-        val = reg_to_u64(iommu->event_log.reg_base);
-        break;
-    case IOMMU_PPR_LOG_BASE_LOW_OFFSET:
-        val = reg_to_u64(iommu->ppr_log.reg_base);
-        break;
-    case IOMMU_CMD_BUFFER_HEAD_OFFSET:
-        val = reg_to_u64(iommu->cmd_buffer.reg_head);
-        break;
-    case IOMMU_CMD_BUFFER_TAIL_OFFSET:
-        val = reg_to_u64(iommu->cmd_buffer.reg_tail);
-        break;
-    case IOMMU_EVENT_LOG_HEAD_OFFSET:
-        val = reg_to_u64(iommu->event_log.reg_head);
-        break;
-    case IOMMU_EVENT_LOG_TAIL_OFFSET:
-        val = reg_to_u64(iommu->event_log.reg_tail);
-        break;
-    case IOMMU_PPR_LOG_HEAD_OFFSET:
-        val = reg_to_u64(iommu->ppr_log.reg_head);
-        break;
-    case IOMMU_PPR_LOG_TAIL_OFFSET:
-        val = reg_to_u64(iommu->ppr_log.reg_tail);
-        break;
-    case IOMMU_CONTROL_MMIO_OFFSET:
-        val = reg_to_u64(iommu->reg_ctrl);
-        break;
-    case IOMMU_STATUS_MMIO_OFFSET:
-        val = reg_to_u64(iommu->reg_status);
-        break;
-    case IOMMU_EXT_FEATURE_MMIO_OFFSET:
-        val = reg_to_u64(iommu->reg_ext_feature);
-        break;
-
-    default:
-        AMD_IOMMU_DEBUG("Guest reads unknown mmio offset = %lx\n", offset);
-        val = 0;
-        break;
-    }
-
-    return val;
-}
-
-static int guest_iommu_mmio_read(struct vcpu *v, unsigned long addr,
-                                 unsigned int len, unsigned long *pval)
-{
-    struct guest_iommu *iommu = vcpu_iommu(v);
-    unsigned long offset;
-    uint64_t val;
-    uint32_t mmio, shift;
-    uint64_t mask = 0;
-
-    offset = addr - iommu->mmio_base;
-
-    if ( unlikely((offset & (len - 1 )) || (len > 8)) )
-    {
-        AMD_IOMMU_DEBUG("iommu mmio read access is not aligned:"
-                        " offset = %lx, len = %x\n", offset, len);
-        return X86EMUL_UNHANDLEABLE;
-    }
-
-    mask = (len == 8) ? ~0ULL : (1ULL << (len * 8)) - 1;
-    shift = (offset & 7u) * 8;
-
-    /* mmio access is always aligned on 8-byte boundary */
-    mmio = offset & (~7u);
-
-    spin_lock(&iommu->lock);
-    val = iommu_mmio_read64(iommu, mmio);
-    spin_unlock(&iommu->lock);
-
-    *pval = (val >> shift ) & mask;
-
-    return X86EMUL_OKAY;
-}
-
-static void guest_iommu_mmio_write64(struct guest_iommu *iommu,
-                                    unsigned long offset, uint64_t val)
-{
-    switch ( offset )
-    {
-    case IOMMU_DEV_TABLE_BASE_LOW_OFFSET:
-        u64_to_reg(&iommu->dev_table.reg_base, val);
-        break;
-    case IOMMU_CMD_BUFFER_BASE_LOW_OFFSET:
-        u64_to_reg(&iommu->cmd_buffer.reg_base, val);
-        break;
-    case IOMMU_EVENT_LOG_BASE_LOW_OFFSET:
-        u64_to_reg(&iommu->event_log.reg_base, val);
-        break;
-    case IOMMU_PPR_LOG_BASE_LOW_OFFSET:
-        u64_to_reg(&iommu->ppr_log.reg_base, val);
-        break;
-    case IOMMU_CONTROL_MMIO_OFFSET:
-        guest_iommu_write_ctrl(iommu, val);
-        break;
-    case IOMMU_CMD_BUFFER_HEAD_OFFSET:
-        u64_to_reg(&iommu->cmd_buffer.reg_head, val);
-        break;
-    case IOMMU_CMD_BUFFER_TAIL_OFFSET:
-        u64_to_reg(&iommu->cmd_buffer.reg_tail, val);
-        tasklet_schedule(&iommu->cmd_buffer_tasklet);
-        break;
-    case IOMMU_EVENT_LOG_HEAD_OFFSET:
-        u64_to_reg(&iommu->event_log.reg_head, val);
-        break;
-    case IOMMU_EVENT_LOG_TAIL_OFFSET:
-        u64_to_reg(&iommu->event_log.reg_tail, val);
-        break;
-    case IOMMU_PPR_LOG_HEAD_OFFSET:
-        u64_to_reg(&iommu->ppr_log.reg_head, val);
-        break;
-    case IOMMU_PPR_LOG_TAIL_OFFSET:
-        u64_to_reg(&iommu->ppr_log.reg_tail, val);
-        break;
-    case IOMMU_STATUS_MMIO_OFFSET:
-        val &= IOMMU_STATUS_EVENT_OVERFLOW_MASK |
-               IOMMU_STATUS_EVENT_LOG_INT_MASK |
-               IOMMU_STATUS_COMP_WAIT_INT_MASK |
-               IOMMU_STATUS_PPR_LOG_OVERFLOW_MASK |
-               IOMMU_STATUS_PPR_LOG_INT_MASK |
-               IOMMU_STATUS_GAPIC_LOG_OVERFLOW_MASK |
-               IOMMU_STATUS_GAPIC_LOG_INT_MASK;
-        u64_to_reg(&iommu->reg_status, reg_to_u64(iommu->reg_status) & ~val);
-        break;
-
-    default:
-        AMD_IOMMU_DEBUG("guest writes unknown mmio offset = %lx,"
-                        " val = %" PRIx64 "\n", offset, val);
-        break;
-    }
-}
-
-static int guest_iommu_mmio_write(struct vcpu *v, unsigned long addr,
-                                  unsigned int len, unsigned long val)
-{
-    struct guest_iommu *iommu = vcpu_iommu(v);
-    unsigned long offset;
-    uint64_t reg_old, mmio;
-    uint32_t shift;
-    uint64_t mask = 0;
-
-    offset = addr - iommu->mmio_base;
-
-    if ( unlikely((offset & (len - 1)) || (len > 8)) )
-    {
-        AMD_IOMMU_DEBUG("iommu mmio write access is not aligned:"
-                        " offset = %lx, len = %x\n", offset, len);
-        return X86EMUL_UNHANDLEABLE;
-    }
-
-    mask = (len == 8) ? ~0ULL : (1ULL << (len * 8)) - 1;
-    shift = (offset & 7) * 8;
-
-    /* mmio access is always aligned on 8-byte boundary */
-    mmio = offset & ~7;
-
-    spin_lock(&iommu->lock);
-
-    reg_old = iommu_mmio_read64(iommu, mmio);
-    reg_old &= ~(mask << shift);
-    val = reg_old | ((val & mask) << shift);
-    guest_iommu_mmio_write64(iommu, mmio, val);
-
-    spin_unlock(&iommu->lock);
-
-    return X86EMUL_OKAY;
-}
-
-int guest_iommu_set_base(struct domain *d, uint64_t base)
-{
-    p2m_type_t t;
-    struct guest_iommu *iommu = domain_iommu(d);
-
-    if ( !iommu )
-        return -EACCES;
-
-    iommu->mmio_base = base;
-    base >>= PAGE_SHIFT;
-
-    for ( int i = 0; i < IOMMU_MMIO_PAGE_NR; i++ )
-    {
-        unsigned long gfn = base + i;
-
-        get_gfn_query(d, gfn, &t);
-        p2m_change_type_one(d, gfn, t, p2m_mmio_dm);
-        put_gfn(d, gfn);
-    }
-
-    return 0;
-}
-
-/* Initialize mmio read only bits */
-static void guest_iommu_reg_init(struct guest_iommu *iommu)
-{
-    uint32_t lower, upper;
-
-    lower = upper = 0;
-    /* Support prefetch */
-    iommu_set_bit(&lower,IOMMU_EXT_FEATURE_PREFSUP_SHIFT);
-    /* Support PPR log */
-    iommu_set_bit(&lower,IOMMU_EXT_FEATURE_PPRSUP_SHIFT);
-    /* Support guest translation */
-    iommu_set_bit(&lower,IOMMU_EXT_FEATURE_GTSUP_SHIFT);
-    /* Support invalidate all command */
-    iommu_set_bit(&lower,IOMMU_EXT_FEATURE_IASUP_SHIFT);
-
-    /* Host translation size has 6 levels */
-    set_field_in_reg_u32(HOST_ADDRESS_SIZE_6_LEVEL, lower,
-                         IOMMU_EXT_FEATURE_HATS_MASK,
-                         IOMMU_EXT_FEATURE_HATS_SHIFT,
-                         &lower);
-    /* Guest translation size has 6 levels */
-    set_field_in_reg_u32(GUEST_ADDRESS_SIZE_6_LEVEL, lower,
-                         IOMMU_EXT_FEATURE_GATS_MASK,
-                         IOMMU_EXT_FEATURE_GATS_SHIFT,
-                         &lower);
-    /* Single level gCR3 */
-    set_field_in_reg_u32(GUEST_CR3_1_LEVEL, lower,
-                         IOMMU_EXT_FEATURE_GLXSUP_MASK,
-                         IOMMU_EXT_FEATURE_GLXSUP_SHIFT, &lower);
-    /* 9 bit PASID */
-    set_field_in_reg_u32(PASMAX_9_bit, upper,
-                         IOMMU_EXT_FEATURE_PASMAX_MASK,
-                         IOMMU_EXT_FEATURE_PASMAX_SHIFT, &upper);
-
-    iommu->reg_ext_feature.lo = lower;
-    iommu->reg_ext_feature.hi = upper;
-}
-
-static int guest_iommu_mmio_range(struct vcpu *v, unsigned long addr)
-{
-    struct guest_iommu *iommu = vcpu_iommu(v);
-
-    return iommu && addr >= iommu->mmio_base &&
-           addr < iommu->mmio_base + IOMMU_MMIO_SIZE;
-}
-
-static const struct hvm_mmio_ops iommu_mmio_ops = {
-    .check = guest_iommu_mmio_range,
-    .read = guest_iommu_mmio_read,
-    .write = guest_iommu_mmio_write
-};
-
-/* Domain specific initialization */
-int guest_iommu_init(struct domain* d)
-{
-    struct guest_iommu *iommu;
-    struct domain_iommu *hd = dom_iommu(d);
-
-    if ( !is_hvm_domain(d) || !iommu_enabled || !iommuv2_enabled ||
-         !has_viommu(d) )
-        return 0;
-
-    iommu = xzalloc(struct guest_iommu);
-    if ( !iommu )
-    {
-        AMD_IOMMU_DEBUG("Error allocating guest iommu structure.\n");
-        return 1;
-    }
-
-    guest_iommu_reg_init(iommu);
-    iommu->mmio_base = ~0ULL;
-    iommu->domain = d;
-    hd->arch.g_iommu = iommu;
-
-    tasklet_init(&iommu->cmd_buffer_tasklet,
-                 guest_iommu_process_command, (unsigned long)d);
-
-    spin_lock_init(&iommu->lock);
-
-    register_mmio_handler(d, &iommu_mmio_ops);
-
-    return 0;
-}
-
-void guest_iommu_destroy(struct domain *d)
-{
-    struct guest_iommu *iommu;
-
-    iommu = domain_iommu(d);
-    if ( !iommu )
-        return;
-
-    tasklet_kill(&iommu->cmd_buffer_tasklet);
-    xfree(iommu);
-
-    dom_iommu(d)->arch.g_iommu = NULL;
-}
diff --git a/xen/drivers/passthrough/x86/amd/iommu_init.c b/xen/drivers/passthrough/x86/amd/iommu_init.c
index 474992a75a..c0742d1cee 100644
--- a/xen/drivers/passthrough/x86/amd/iommu_init.c
+++ b/xen/drivers/passthrough/x86/amd/iommu_init.c
@@ -638,12 +638,15 @@ static void iommu_check_event_log(struct amd_iommu *iommu)
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
+/*
+ * XXX Originally this function calls (dead) guest_iommu code. It does
+ * nothing other than zero'ing the entry (and handling erratum). This
+ * is what is looks like when all dead code is deleted.
+ */
 void parse_ppr_log_entry(struct amd_iommu *iommu, u32 entry[])
 {
 
-    u16 device_id;
-    u8 bus, devfn, code;
-    struct pci_dev *pdev;
+    u8 code;
     int count = 0;
 
     code = get_field_from_reg_u32(entry[1], IOMMU_PPR_LOG_CODE_MASK,
@@ -668,18 +671,6 @@ void parse_ppr_log_entry(struct amd_iommu *iommu, u32 entry[])
                                       IOMMU_PPR_LOG_CODE_SHIFT);
     }
 
-    /* here device_id is physical value */
-    device_id = iommu_get_devid_from_cmd(entry[0]);
-    bus = PCI_BUS(device_id);
-    devfn = PCI_DEVFN2(device_id);
-
-    pcidevs_lock();
-    pdev = pci_get_real_pdev(iommu->seg, bus, devfn);
-    pcidevs_unlock();
-
-    if ( pdev )
-        guest_iommu_add_ppr_log(pdev->domain, entry);
-
     memset(entry, 0, IOMMU_PPR_LOG_ENTRY_SIZE);
 }
 
diff --git a/xen/include/asm-x86/amd-iommu.h b/xen/include/asm-x86/amd-iommu.h
index 02715b482b..102518a864 100644
--- a/xen/include/asm-x86/amd-iommu.h
+++ b/xen/include/asm-x86/amd-iommu.h
@@ -134,57 +134,6 @@ struct ivrs_mappings *get_ivrs_mappings(u16 seg);
 int iterate_ivrs_mappings(int (*)(u16 seg, struct ivrs_mappings *));
 int iterate_ivrs_entries(int (*)(u16 seg, struct ivrs_mappings *));
 
-/* iommu tables in guest space */
-struct mmio_reg {
-    uint32_t    lo;
-    uint32_t    hi;
-};
-
-struct guest_dev_table {
-    struct mmio_reg         reg_base;
-    uint32_t                size;
-};
-
-struct guest_buffer {
-    struct mmio_reg         reg_base;
-    struct mmio_reg         reg_tail;
-    struct mmio_reg         reg_head;
-    uint32_t                entries;
-};
-
-struct guest_iommu_msi {
-    uint8_t                 vector;
-    uint8_t                 dest;
-    uint8_t                 dest_mode;
-    uint8_t                 delivery_mode;
-    uint8_t                 trig_mode;
-};
-
-/* virtual IOMMU structure */
-struct guest_iommu {
-
-    struct domain          *domain;
-    spinlock_t              lock;
-    bool_t                  enabled;
-
-    struct guest_dev_table  dev_table;
-    struct guest_buffer     cmd_buffer;
-    struct guest_buffer     event_log;
-    struct guest_buffer     ppr_log;
-
-    struct tasklet          cmd_buffer_tasklet;
-
-    uint64_t                mmio_base;             /* MMIO base address */
-
-    /* MMIO regs */
-    struct mmio_reg         reg_ctrl;              /* MMIO offset 0018h */
-    struct mmio_reg         reg_status;            /* MMIO offset 2020h */
-    struct mmio_reg         reg_ext_feature;       /* MMIO offset 0030h */
-
-    /* guest interrupt settings */
-    struct guest_iommu_msi  msi;
-};
-
 extern bool_t iommuv2_enabled;
 
 #endif /* _ASM_X86_64_AMD_IOMMU_H */
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index 99bc21c7b3..9163dc9035 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -130,14 +130,6 @@ void amd_iommu_resume(void);
 int __must_check amd_iommu_suspend(void);
 void amd_iommu_crash_shutdown(void);
 
-/* guest iommu support */
-void amd_iommu_send_guest_cmd(struct amd_iommu *iommu, u32 cmd[]);
-void guest_iommu_add_ppr_log(struct domain *d, u32 entry[]);
-void guest_iommu_add_event_log(struct domain *d, u32 entry[]);
-int guest_iommu_init(struct domain* d);
-void guest_iommu_destroy(struct domain *d);
-int guest_iommu_set_base(struct domain *d, uint64_t base);
-
 static inline u32 get_field_from_reg_u32(u32 reg_value, u32 mask, u32 shift)
 {
     u32 field;
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 14ad0489a6..8cb7bec26b 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -41,7 +41,6 @@ struct arch_iommu
     /* amd iommu support */
     int paging_mode;
     struct page_info *root_table;
-    struct guest_iommu *g_iommu;
 };
 
 extern const struct iommu_ops intel_iommu_ops;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (5 preceding siblings ...)
  2018-02-21 21:46 ` [PATCH RFC 06/10] passthrough/amd: remove guest iommu support Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-04-23 15:43   ` Jan Beulich
  2018-05-02 17:32   ` Suravee Suthikulpanit
  2018-02-21 21:46 ` [PATCH RFC 08/10] passthrough/amd: make clear_iommu_pte_present static Wei Liu
                   ` (4 subsequent siblings)
  11 siblings, 2 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Suravee Suthikulpanit, Jan Beulich

Move and rename update_paging_mode. Create a local header file for
this and other functions that need exporting.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 xen/drivers/passthrough/x86/amd/Makefile    |   1 +
 xen/drivers/passthrough/x86/amd/hvm.c       | 108 ++++++++++++++++++++++++++++
 xen/drivers/passthrough/x86/amd/iommu.h     |  32 +++++++++
 xen/drivers/passthrough/x86/amd/iommu_map.c | 103 ++------------------------
 4 files changed, 148 insertions(+), 96 deletions(-)
 create mode 100644 xen/drivers/passthrough/x86/amd/hvm.c
 create mode 100644 xen/drivers/passthrough/x86/amd/iommu.h

diff --git a/xen/drivers/passthrough/x86/amd/Makefile b/xen/drivers/passthrough/x86/amd/Makefile
index 415146fcdb..ed28fdb660 100644
--- a/xen/drivers/passthrough/x86/amd/Makefile
+++ b/xen/drivers/passthrough/x86/amd/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_HVM) += hvm.o
 obj-bin-y += iommu_detect.init.o
 obj-y += iommu_init.o
 obj-y += iommu_map.o
diff --git a/xen/drivers/passthrough/x86/amd/hvm.c b/xen/drivers/passthrough/x86/amd/hvm.c
new file mode 100644
index 0000000000..35c76c2545
--- /dev/null
+++ b/xen/drivers/passthrough/x86/amd/hvm.c
@@ -0,0 +1,108 @@
+#include <xen/sched.h>
+#include <asm/amd-iommu.h>
+#include <asm/hvm/svm/amd-iommu-proto.h>
+#include "../../ats.h"
+#include <xen/pci.h>
+
+#include "iommu.h"
+
+int hvm_update_paging_mode(struct domain *d, unsigned long gfn)
+{
+    u16 bdf;
+    void *device_entry;
+    unsigned int req_id, level, offset;
+    unsigned long flags;
+    struct pci_dev *pdev;
+    struct amd_iommu *iommu = NULL;
+    struct page_info *new_root = NULL;
+    struct page_info *old_root = NULL;
+    void *new_root_vaddr;
+    unsigned long old_root_mfn;
+    struct domain_iommu *hd = dom_iommu(d);
+
+    if ( gfn == gfn_x(INVALID_GFN) )
+        return -EADDRNOTAVAIL;
+    ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
+
+    level = hd->arch.paging_mode;
+    old_root = hd->arch.root_table;
+    offset = gfn >> (PTE_PER_TABLE_SHIFT * (level - 1));
+
+    ASSERT(spin_is_locked(&hd->arch.mapping_lock) && is_hvm_domain(d));
+
+    while ( offset >= PTE_PER_TABLE_SIZE )
+    {
+        /* Allocate and install a new root table.
+         * Only upper I/O page table grows, no need to fix next level bits */
+        new_root = alloc_amd_iommu_pgtable();
+        if ( new_root == NULL )
+        {
+            AMD_IOMMU_DEBUG("%s Cannot allocate I/O page table\n",
+                            __func__);
+            return -ENOMEM;
+        }
+
+        new_root_vaddr = __map_domain_page(new_root);
+        old_root_mfn = page_to_mfn(old_root);
+        set_iommu_pde_present(new_root_vaddr, old_root_mfn, level,
+                              !!IOMMUF_writable, !!IOMMUF_readable);
+        level++;
+        old_root = new_root;
+        offset >>= PTE_PER_TABLE_SHIFT;
+        unmap_domain_page(new_root_vaddr);
+    }
+
+    if ( new_root != NULL )
+    {
+        hd->arch.paging_mode = level;
+        hd->arch.root_table = new_root;
+
+        if ( !pcidevs_locked() )
+            AMD_IOMMU_DEBUG("%s Try to access pdev_list "
+                            "without aquiring pcidevs_lock.\n", __func__);
+
+        /* Update device table entries using new root table and paging mode */
+        for_each_pdev( d, pdev )
+        {
+            bdf = PCI_BDF2(pdev->bus, pdev->devfn);
+            iommu = find_iommu_for_device(pdev->seg, bdf);
+            if ( !iommu )
+            {
+                AMD_IOMMU_DEBUG("%s Fail to find iommu.\n", __func__);
+                return -ENODEV;
+            }
+
+            spin_lock_irqsave(&iommu->lock, flags);
+            do {
+                req_id = get_dma_requestor_id(pdev->seg, bdf);
+                device_entry = iommu->dev_table.buffer +
+                               (req_id * IOMMU_DEV_TABLE_ENTRY_SIZE);
+
+                /* valid = 0 only works for dom0 passthrough mode */
+                amd_iommu_set_root_page_table((u32 *)device_entry,
+                                              page_to_maddr(hd->arch.root_table),
+                                              d->domain_id,
+                                              hd->arch.paging_mode, 1);
+
+                amd_iommu_flush_device(iommu, req_id);
+                bdf += pdev->phantom_stride;
+            } while ( PCI_DEVFN2(bdf) != pdev->devfn &&
+                      PCI_SLOT(bdf) == PCI_SLOT(pdev->devfn) );
+            spin_unlock_irqrestore(&iommu->lock, flags);
+        }
+
+        /* For safety, invalidate all entries */
+        amd_iommu_flush_all_pages(d);
+    }
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/passthrough/x86/amd/iommu.h b/xen/drivers/passthrough/x86/amd/iommu.h
new file mode 100644
index 0000000000..bb31de61ae
--- /dev/null
+++ b/xen/drivers/passthrough/x86/amd/iommu.h
@@ -0,0 +1,32 @@
+#ifndef _X86_AMD_IOMMU_H_
+#define _X86_AMD_IOMMU_H_
+
+bool_t set_iommu_pde_present(u32 *pde, unsigned long next_mfn,
+                             unsigned int next_level,
+                             bool_t iw, bool_t ir);
+
+#ifdef CONFIG_HVM
+
+int hvm_update_paging_mode(struct domain *d, unsigned long gfn);
+
+#else
+
+static inline int hvm_update_paging_mode(struct domain *d, unsigned long gfn)
+{
+    ASSERT_UNREACHABLE();
+    return -EINVAL;
+}
+
+#endif
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/passthrough/x86/amd/iommu_map.c b/xen/drivers/passthrough/x86/amd/iommu_map.c
index 0f9bd538af..e51a5c5ffe 100644
--- a/xen/drivers/passthrough/x86/amd/iommu_map.c
+++ b/xen/drivers/passthrough/x86/amd/iommu_map.c
@@ -25,6 +25,8 @@
 #include "../../ats.h"
 #include <xen/pci.h>
 
+#include "iommu.h"
+
 /* Given pfn and page table level, return pde index */
 static unsigned int pfn_to_pde_idx(unsigned long pfn, unsigned int level)
 {
@@ -45,9 +47,9 @@ void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long gfn)
     unmap_domain_page(table);
 }
 
-static bool_t set_iommu_pde_present(u32 *pde, unsigned long next_mfn, 
-                                    unsigned int next_level,
-                                    bool_t iw, bool_t ir)
+bool_t set_iommu_pde_present(u32 *pde, unsigned long next_mfn,
+			     unsigned int next_level,
+			     bool_t iw, bool_t ir)
 {
     u64 addr_lo, addr_hi, maddr_old, maddr_next;
     u32 entry;
@@ -540,97 +542,6 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
     return 0;
 }
 
-static int update_paging_mode(struct domain *d, unsigned long gfn)
-{
-    u16 bdf;
-    void *device_entry;
-    unsigned int req_id, level, offset;
-    unsigned long flags;
-    struct pci_dev *pdev;
-    struct amd_iommu *iommu = NULL;
-    struct page_info *new_root = NULL;
-    struct page_info *old_root = NULL;
-    void *new_root_vaddr;
-    unsigned long old_root_mfn;
-    struct domain_iommu *hd = dom_iommu(d);
-
-    if ( gfn == gfn_x(INVALID_GFN) )
-        return -EADDRNOTAVAIL;
-    ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
-
-    level = hd->arch.paging_mode;
-    old_root = hd->arch.root_table;
-    offset = gfn >> (PTE_PER_TABLE_SHIFT * (level - 1));
-
-    ASSERT(spin_is_locked(&hd->arch.mapping_lock) && is_hvm_domain(d));
-
-    while ( offset >= PTE_PER_TABLE_SIZE )
-    {
-        /* Allocate and install a new root table.
-         * Only upper I/O page table grows, no need to fix next level bits */
-        new_root = alloc_amd_iommu_pgtable();
-        if ( new_root == NULL )
-        {
-            AMD_IOMMU_DEBUG("%s Cannot allocate I/O page table\n",
-                            __func__);
-            return -ENOMEM;
-        }
-
-        new_root_vaddr = __map_domain_page(new_root);
-        old_root_mfn = page_to_mfn(old_root);
-        set_iommu_pde_present(new_root_vaddr, old_root_mfn, level,
-                              !!IOMMUF_writable, !!IOMMUF_readable);
-        level++;
-        old_root = new_root;
-        offset >>= PTE_PER_TABLE_SHIFT;
-        unmap_domain_page(new_root_vaddr);
-    }
-
-    if ( new_root != NULL )
-    {
-        hd->arch.paging_mode = level;
-        hd->arch.root_table = new_root;
-
-        if ( !pcidevs_locked() )
-            AMD_IOMMU_DEBUG("%s Try to access pdev_list "
-                            "without aquiring pcidevs_lock.\n", __func__);
-
-        /* Update device table entries using new root table and paging mode */
-        for_each_pdev( d, pdev )
-        {
-            bdf = PCI_BDF2(pdev->bus, pdev->devfn);
-            iommu = find_iommu_for_device(pdev->seg, bdf);
-            if ( !iommu )
-            {
-                AMD_IOMMU_DEBUG("%s Fail to find iommu.\n", __func__);
-                return -ENODEV;
-            }
-
-            spin_lock_irqsave(&iommu->lock, flags);
-            do {
-                req_id = get_dma_requestor_id(pdev->seg, bdf);
-                device_entry = iommu->dev_table.buffer +
-                               (req_id * IOMMU_DEV_TABLE_ENTRY_SIZE);
-
-                /* valid = 0 only works for dom0 passthrough mode */
-                amd_iommu_set_root_page_table((u32 *)device_entry,
-                                              page_to_maddr(hd->arch.root_table),
-                                              d->domain_id,
-                                              hd->arch.paging_mode, 1);
-
-                amd_iommu_flush_device(iommu, req_id);
-                bdf += pdev->phantom_stride;
-            } while ( PCI_DEVFN2(bdf) != pdev->devfn &&
-                      PCI_SLOT(bdf) == PCI_SLOT(pdev->devfn) );
-            spin_unlock_irqrestore(&iommu->lock, flags);
-        }
-
-        /* For safety, invalidate all entries */
-        amd_iommu_flush_all_pages(d);
-    }
-    return 0;
-}
-
 int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
                        unsigned int flags)
 {
@@ -660,7 +571,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
      * we might need a deeper page table for lager gfn now */
     if ( is_hvm_domain(d) )
     {
-        if ( update_paging_mode(d, gfn) )
+        if ( hvm_update_paging_mode(d, gfn) )
         {
             spin_unlock(&hd->arch.mapping_lock);
             AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
@@ -742,7 +653,7 @@ int amd_iommu_unmap_page(struct domain *d, unsigned long gfn)
      * we might need a deeper page table for lager gfn now */
     if ( is_hvm_domain(d) )
     {
-        int rc = update_paging_mode(d, gfn);
+        int rc = hvm_update_paging_mode(d, gfn);
 
         if ( rc )
         {
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 08/10] passthrough/amd: make clear_iommu_pte_present static
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (6 preceding siblings ...)
  2018-02-21 21:46 ` [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c Wei Liu
@ 2018-02-21 21:46 ` Wei Liu
  2018-02-21 21:47 ` [PATCH RFC 09/10] passthrough/intel: put some code under CONFIG_HVM Wei Liu
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:46 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Suravee Suthikulpanit, Jan Beulich

There is only one user in the same file.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 xen/drivers/passthrough/x86/amd/iommu_map.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/x86/amd/iommu_map.c b/xen/drivers/passthrough/x86/amd/iommu_map.c
index e51a5c5ffe..8568819eac 100644
--- a/xen/drivers/passthrough/x86/amd/iommu_map.c
+++ b/xen/drivers/passthrough/x86/amd/iommu_map.c
@@ -37,7 +37,7 @@ static unsigned int pfn_to_pde_idx(unsigned long pfn, unsigned int level)
     return idx;
 }
 
-void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long gfn)
+static void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long gfn)
 {
     u64 *table, *pte;
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 09/10] passthrough/intel: put some code under CONFIG_HVM
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (7 preceding siblings ...)
  2018-02-21 21:46 ` [PATCH RFC 08/10] passthrough/amd: make clear_iommu_pte_present static Wei Liu
@ 2018-02-21 21:47 ` Wei Liu
  2018-04-23 15:47   ` Jan Beulich
  2018-02-21 21:47 ` [PATCH RFC 10/10] x86: check hvm domain before calling pt_irq_destroy_bind Wei Liu
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:47 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Wei Liu, Jan Beulich

The code which references code under arch/x86/hvm/ is surrounded by
CONFIG_HVM now.

The basic idea is that in !CONFIG_HVM && CONFIG_PV case we still want
to retain the host side IOMMU code so that PV guests can use it if
necessary.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Kevin Tian <kevin.tian@intel.com>
---
 xen/drivers/passthrough/x86/vtd/iommu.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/x86/vtd/iommu.c b/xen/drivers/passthrough/x86/vtd/iommu.c
index 1d161fe149..c27e63dfc1 100644
--- a/xen/drivers/passthrough/x86/vtd/iommu.c
+++ b/xen/drivers/passthrough/x86/vtd/iommu.c
@@ -32,7 +32,6 @@
 #include <xen/keyhandler.h>
 #include <asm/msi.h>
 #include <asm/irq.h>
-#include <asm/hvm/vmx/vmx.h>
 #include <asm/p2m.h>
 #include <mach_apic.h>
 #include "iommu.h"
@@ -41,6 +40,10 @@
 #include "vtd.h"
 #include "../../ats.h"
 
+#ifdef CONFIG_HVM
+#include <asm/hvm/vmx/vmx.h>
+#endif
+
 struct mapped_rmrr {
     struct list_head list;
     u64 base, end;
@@ -1873,6 +1876,7 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
     return rc;
 }
 
+#ifdef CONFIG_HVM
 static int __init vtd_ept_page_compatible(struct iommu *iommu)
 {
     u64 ept_cap, vtd_cap = iommu->cap;
@@ -1885,6 +1889,7 @@ static int __init vtd_ept_page_compatible(struct iommu *iommu)
     return (ept_has_2mb(ept_cap) && opt_hap_2mb) == cap_sps_2mb(vtd_cap) &&
            (ept_has_1gb(ept_cap) && opt_hap_1gb) == cap_sps_1gb(vtd_cap);
 }
+#endif
 
 /*
  * set VT-d page table directory to EPT table if allowed
@@ -2280,7 +2285,9 @@ int __init intel_vtd_setup(void)
         if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
             iommu_intpost = 0;
 
+#ifdef CONFIG_HVM
         if ( !vtd_ept_page_compatible(iommu) )
+#endif
             iommu_hap_pt_share = 0;
 
         ret = iommu_set_interrupt(drhd);
@@ -2376,14 +2383,18 @@ static int reassign_device_ownership(
     if ( ret )
         return ret;
 
+#ifdef CONFIG_HVM
     if ( !has_arch_pdevs(target) )
         vmx_pi_hooks_assign(target);
+#endif
 
     ret = domain_context_mapping(target, devfn, pdev);
     if ( ret )
     {
+#ifdef CONFIG_HVM
         if ( !has_arch_pdevs(target) )
             vmx_pi_hooks_deassign(target);
+#endif
 
         return ret;
     }
@@ -2394,8 +2405,10 @@ static int reassign_device_ownership(
         pdev->domain = target;
     }
 
+#ifdef CONFIG_HVM
     if ( !has_arch_pdevs(source) )
         vmx_pi_hooks_deassign(source);
+#endif
 
     return ret;
 }
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH RFC 10/10] x86: check hvm domain before calling pt_irq_destroy_bind
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (8 preceding siblings ...)
  2018-02-21 21:47 ` [PATCH RFC 09/10] passthrough/intel: put some code under CONFIG_HVM Wei Liu
@ 2018-02-21 21:47 ` Wei Liu
  2018-02-23  5:12 ` [PATCH RFC 00/10] x86 passthrough code cleanup Tian, Kevin
  2018-03-08 12:18 ` Wei Liu
  11 siblings, 0 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-21 21:47 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

... just like its counter part because that function is HVM only.

There is no risk of corruption because pt_irq_destroy_bind already has
proper check in place, but it would be nice to be more explicit before
calling the said function.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/domctl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 8fbbf3aeb3..07b7d41b66 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -707,6 +707,10 @@ long arch_do_domctl(
         struct xen_domctl_bind_pt_irq *bind = &domctl->u.bind_pt_irq;
         int irq = domain_pirq_to_irq(d, bind->machine_irq);
 
+        ret = -EINVAL;
+        if ( !is_hvm_domain(d) )
+            break;
+
         ret = -EPERM;
         if ( irq <= 0 || !irq_access_permitted(currd, irq) )
             break;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (9 preceding siblings ...)
  2018-02-21 21:47 ` [PATCH RFC 10/10] x86: check hvm domain before calling pt_irq_destroy_bind Wei Liu
@ 2018-02-23  5:12 ` Tian, Kevin
  2018-02-23 16:08   ` Wei Liu
  2018-02-24  4:39   ` Doug Goldstein
  2018-03-08 12:18 ` Wei Liu
  11 siblings, 2 replies; 31+ messages in thread
From: Tian, Kevin @ 2018-02-23  5:12 UTC (permalink / raw)
  To: Wei Liu, Xen-devel; +Cc: Andrew Cooper, Jan Beulich

> From: Wei Liu
> Sent: Thursday, February 22, 2018 5:47 AM
> 
> Hi all
> 
> At some point I would like to make CONFIG_HVM and CONFIG_PV work.
> The
> passthrough code is one of the road blocks for that work.

Can you elaborate the motivation of this change? why does someone
want to disable HVM or PV logic completely from hypervisor?

> 
> A short discussion on #xendevel made me think that having host side code
> regardless of if HVM (the primary user) is configured is desirable because
> PV
> guests may still have limited use of the hardware, hence this series.
> 
> What I want is to have clear hierarchy of the code and split the host side
> and the guest side code, and start to use CONFIG_HVM where applicable.
> Luckily
> the amount of work seemed to be smaller than I had expected.
> 
> RFC because there are a few open questions. Please see individual patches.
> 
> Wei.
> 
> Wei Liu (10):
>   passthrough: rearrange x86 code
>   passthrough: split out x86 PCI code to x86/pci.c
>   x86/passthrough: io.c is used for HVM only
>   x86/passthrough: arch_pci_clean_irqs is HVM only
>   x86/passthrough: move hvm_dpci_isairq_eoi
>   passthrough/amd: remove guest iommu support
>   passthrough/amd: split out hvm code from iommu_map.c
>   passthrough/amd: make clear_iommu_pte_present static
>   passthrough/intel: put some code under CONFIG_HVM
>   x86: check hvm domain before calling pt_irq_destroy_bind
> 
>  MAINTAINERS                                        |   8 +-
>  xen/arch/x86/domctl.c                              |   4 +
>  xen/drivers/passthrough/Makefile                   |   3 -
>  xen/drivers/passthrough/amd/iommu_guest.c          | 927 ---------------------
>  xen/drivers/passthrough/pci.c                      |  51 +-
>  xen/drivers/passthrough/x86/Makefile               |   5 +
>  xen/drivers/passthrough/{ => x86}/amd/Makefile     |   2 +-
>  xen/drivers/passthrough/x86/amd/hvm.c              | 108 +++
>  xen/drivers/passthrough/x86/amd/iommu.h            |  32 +
>  xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c |   0
>  xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c  |   2 +-
>  .../passthrough/{ => x86}/amd/iommu_detect.c       |   0
>  xen/drivers/passthrough/{ => x86}/amd/iommu_init.c |  21 +-
>  xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c |   0
>  xen/drivers/passthrough/{ => x86}/amd/iommu_map.c  | 107 +--
>  .../passthrough/{ => x86}/amd/pci_amd_iommu.c      |   2 +-
>  xen/drivers/passthrough/{ => x86}/io.c             |  45 +
>  xen/drivers/passthrough/x86/pci.c                  |  74 ++
>  xen/drivers/passthrough/{ => x86}/vtd/Makefile     |   0
>  xen/drivers/passthrough/{ => x86}/vtd/dmar.c       |   0
>  xen/drivers/passthrough/{ => x86}/vtd/dmar.h       |   0
>  xen/drivers/passthrough/{ => x86}/vtd/extern.h     |   0
>  xen/drivers/passthrough/{ => x86}/vtd/intremap.c   |   0
>  xen/drivers/passthrough/{ => x86}/vtd/iommu.c      |  17 +-
>  xen/drivers/passthrough/{ => x86}/vtd/iommu.h      |   0
>  xen/drivers/passthrough/{ => x86}/vtd/qinval.c     |   2 +-
>  xen/drivers/passthrough/{ => x86}/vtd/quirks.c     |   0
>  xen/drivers/passthrough/{ => x86}/vtd/utils.c      |   0
>  xen/drivers/passthrough/{ => x86}/vtd/vtd.h        |   0
>  xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile |   0
>  xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c    |   2 +-
>  xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c    |  45 -
>  xen/include/asm-x86/amd-iommu.h                    |  51 --
>  xen/include/asm-x86/hvm/svm/amd-iommu-proto.h      |   8 -
>  xen/include/asm-x86/iommu.h                        |   1 -
>  xen/include/xen/iommu.h                            |   1 -
>  xen/include/xen/pci.h                              |   2 +
>  37 files changed, 310 insertions(+), 1210 deletions(-)
>  delete mode 100644 xen/drivers/passthrough/amd/iommu_guest.c
>  rename xen/drivers/passthrough/{ => x86}/amd/Makefile (86%)
>  create mode 100644 xen/drivers/passthrough/x86/amd/hvm.c
>  create mode 100644 xen/drivers/passthrough/x86/amd/iommu.h
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_detect.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_init.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_map.c (86%)
>  rename xen/drivers/passthrough/{ => x86}/amd/pci_amd_iommu.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/io.c (96%)
>  create mode 100644 xen/drivers/passthrough/x86/pci.c
>  rename xen/drivers/passthrough/{ => x86}/vtd/Makefile (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/dmar.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/dmar.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/extern.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/intremap.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/iommu.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/iommu.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/qinval.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/quirks.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/utils.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/vtd.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c (72%)
> 
> --
> 2.11.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-23  5:12 ` [PATCH RFC 00/10] x86 passthrough code cleanup Tian, Kevin
@ 2018-02-23 16:08   ` Wei Liu
  2018-02-24  3:23     ` Tian, Kevin
  2018-02-24  4:39   ` Doug Goldstein
  1 sibling, 1 reply; 31+ messages in thread
From: Wei Liu @ 2018-02-23 16:08 UTC (permalink / raw)
  To: Tian, Kevin; +Cc: Xen-devel, Wei Liu, Jan Beulich, Andrew Cooper

On Fri, Feb 23, 2018 at 05:12:05AM +0000, Tian, Kevin wrote:
> > From: Wei Liu
> > Sent: Thursday, February 22, 2018 5:47 AM
> > 
> > Hi all
> > 
> > At some point I would like to make CONFIG_HVM and CONFIG_PV work.
> > The
> > passthrough code is one of the road blocks for that work.
> 
> Can you elaborate the motivation of this change? why does someone
> want to disable HVM or PV logic completely from hypervisor?
> 

At some point in the future, we would like to utilise as many hardware
features as possible and have an HVM / PVH only setup. At that point PV
code will be necessarily and should be preferably compiled out to reduce
code size and attack surface.

Having PV compiled out also enable Xen to reclaim some address space
from the PV ABI.

But, we understand that PV is here to stay for at least a while and
could be useful for some other niche use cases, so upstream have
developed a PV-in-PVH shim to continue to support PV guests. The shim is
actually yet another configuration of Xen running as a PVH guest but
exposes PV ABI to PV guests. We want to disable HVM code in that case,
again, to reduce code size and attach surface.

There is also the long term benefit to make Xen more maintainable and
approachable in the future.

See
https://www.slideshare.net/xen_com_mgr/xpdds17-keynote-towards-a-configurable-and-slimmer-x86-hypervisor-wei-liu-citrix

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-23 16:08   ` Wei Liu
@ 2018-02-24  3:23     ` Tian, Kevin
  2018-02-26  8:20       ` Jan Beulich
  2018-02-26 12:45       ` Wei Liu
  0 siblings, 2 replies; 31+ messages in thread
From: Tian, Kevin @ 2018-02-24  3:23 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel, Jan Beulich, Andrew Cooper

> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: Saturday, February 24, 2018 12:08 AM
> 
> On Fri, Feb 23, 2018 at 05:12:05AM +0000, Tian, Kevin wrote:
> > > From: Wei Liu
> > > Sent: Thursday, February 22, 2018 5:47 AM
> > >
> > > Hi all
> > >
> > > At some point I would like to make CONFIG_HVM and CONFIG_PV work.
> > > The
> > > passthrough code is one of the road blocks for that work.
> >
> > Can you elaborate the motivation of this change? why does someone
> > want to disable HVM or PV logic completely from hypervisor?
> >
> 
> At some point in the future, we would like to utilise as many hardware
> features as possible and have an HVM / PVH only setup. At that point PV
> code will be necessarily and should be preferably compiled out to reduce
> code size and attack surface.
> 
> Having PV compiled out also enable Xen to reclaim some address space
> from the PV ABI.
> 
> But, we understand that PV is here to stay for at least a while and
> could be useful for some other niche use cases, so upstream have
> developed a PV-in-PVH shim to continue to support PV guests. The shim is
> actually yet another configuration of Xen running as a PVH guest but
> exposes PV ABI to PV guests. We want to disable HVM code in that case,
> again, to reduce code size and attach surface.

since it's only temporary, do we really care about code size or attacking
surface in such case? or is it worthy of the effort regarding to the gain
which you anticipate?

I sort of buy-in CONFIG_PV, but not sure whether CONFIG_HVM is
required.

> 
> There is also the long term benefit to make Xen more maintainable and
> approachable in the future.
> 
> See
> https://www.slideshare.net/xen_com_mgr/xpdds17-keynote-towards-a-
> configurable-and-slimmer-x86-hypervisor-wei-liu-citrix
> 
> Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-23  5:12 ` [PATCH RFC 00/10] x86 passthrough code cleanup Tian, Kevin
  2018-02-23 16:08   ` Wei Liu
@ 2018-02-24  4:39   ` Doug Goldstein
  2018-02-26  0:47     ` Marek Marczykowski-Górecki
  1 sibling, 1 reply; 31+ messages in thread
From: Doug Goldstein @ 2018-02-24  4:39 UTC (permalink / raw)
  To: Tian, Kevin, Wei Liu, Xen-devel
  Cc: Andrew Cooper, Marek Marczykowski-Górecki, Jan Beulich


[-- Attachment #1.1.1: Type: text/plain, Size: 1541 bytes --]

On 2/22/18 11:12 PM, Tian, Kevin wrote:
>> From: Wei Liu
>> Sent: Thursday, February 22, 2018 5:47 AM
>>
>> Hi all
>>
>> At some point I would like to make CONFIG_HVM and CONFIG_PV work.
>> The
>> passthrough code is one of the road blocks for that work.
> 
> Can you elaborate the motivation of this change? why does someone
> want to disable HVM or PV logic completely from hypervisor?

I can say I recall advocating for this at Xen Summit in Cambridge. I
believe I talked about it in Toronto as well. There are a number of
users of Xen that would certainly want to ship without all the code
associated with PV compiled in. Given the nature of design "compromises"
in many parts of x86 systems there is certainly a non-zero sum of people
that would likely utilize the ability to remove code that doesn't need
to be there. I think every individual on this list who has been involved
in the security has been in a room of @intel.com folks has seen features
vs security win out many times.

I don't think its a hard stretch of the imagination to see people
disabling PV in data centers running newer workloads on PVH and HVM
only. I can see the real question being why HVM? That I would say lies
with the direction of discretionary access controls in Xen vs mandatory
access controls. To solve for the lack of functionality we've grown
things like "dmops" and I could certainly see a product like Qubes
running only PVH domains in the future.

Since I picked on Qubes I've CC'd Marek.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-24  4:39   ` Doug Goldstein
@ 2018-02-26  0:47     ` Marek Marczykowski-Górecki
  2018-02-26 12:49       ` Wei Liu
  0 siblings, 1 reply; 31+ messages in thread
From: Marek Marczykowski-Górecki @ 2018-02-26  0:47 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: Xen-devel, Tian, Kevin, Wei Liu, Jan Beulich, Andrew Cooper


[-- Attachment #1.1: Type: text/plain, Size: 2243 bytes --]

On Fri, Feb 23, 2018 at 10:39:20PM -0600, Doug Goldstein wrote:
> On 2/22/18 11:12 PM, Tian, Kevin wrote:
> >> From: Wei Liu
> >> Sent: Thursday, February 22, 2018 5:47 AM
> >>
> >> Hi all
> >>
> >> At some point I would like to make CONFIG_HVM and CONFIG_PV work.
> >> The
> >> passthrough code is one of the road blocks for that work.
> > 
> > Can you elaborate the motivation of this change? why does someone
> > want to disable HVM or PV logic completely from hypervisor?
> 
> I can say I recall advocating for this at Xen Summit in Cambridge. I
> believe I talked about it in Toronto as well. There are a number of
> users of Xen that would certainly want to ship without all the code
> associated with PV compiled in. Given the nature of design "compromises"
> in many parts of x86 systems there is certainly a non-zero sum of people
> that would likely utilize the ability to remove code that doesn't need
> to be there. I think every individual on this list who has been involved
> in the security has been in a room of @intel.com folks has seen features
> vs security win out many times.
> 
> I don't think its a hard stretch of the imagination to see people
> disabling PV in data centers running newer workloads on PVH and HVM
> only.

Yes, definitely disabling PV will be useful. Right after being able to
use PCI passthrough with PVH.

> I can see the real question being why HVM? That I would say lies
> with the direction of discretionary access controls in Xen vs mandatory
> access controls. To solve for the lack of functionality we've grown
> things like "dmops" and I could certainly see a product like Qubes
> running only PVH domains in the future.
> 
> Since I picked on Qubes I've CC'd Marek.

So, is it going to be an option to have CONFIG_HVM=n and CONFIG_PVH=y at
the same time? While currently we do support Windows, so need
CONFIG_HVM=y, but I can see in some future/alternative version we could
have even that disabled. For example right now we do have
CONFIG_SHADOW_PAGING disabled.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-24  3:23     ` Tian, Kevin
@ 2018-02-26  8:20       ` Jan Beulich
  2018-02-26 12:45       ` Wei Liu
  1 sibling, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2018-02-26  8:20 UTC (permalink / raw)
  To: Wei Liu, Kevin Tian; +Cc: Andrew Cooper, Xen-devel

>>> On 24.02.18 at 04:23, <kevin.tian@intel.com> wrote:
> I sort of buy-in CONFIG_PV, but not sure whether CONFIG_HVM is
> required.

I think the shim is where CONFIG_HVM would want to be turned off.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c
  2018-02-21 21:46 ` [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c Wei Liu
@ 2018-02-26 10:57   ` Julien Grall
  2018-04-23 15:34   ` Jan Beulich
  1 sibling, 0 replies; 31+ messages in thread
From: Julien Grall @ 2018-02-26 10:57 UTC (permalink / raw)
  To: Wei Liu, Xen-devel; +Cc: Andrew Cooper, Stefano Stabellini, Jan Beulich

Hi,

On 21/02/18 21:46, Wei Liu wrote:
> Move the functions that reference x86 hvm data structures to its own
> file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.

NIT: Double space.

> 
> There is still one location in that file which references
> arch.hvm_domain, but it is fine because ARM guest is HVM.

I guess you mean hvm_domain.mem_sharing_enabled? If so, we don't support 
memory sharing on ARM. This would need to be stub.

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> ARM doesn't select HAS_PCI, that's why ARM build is not broken by
> this. AIUI ARM will select HAS_PCI at some point, hence I only move
> the x86 bits.

Potentially there are more x86-ism in that code such as the way to 
handle MSI. Anyway, this patch makes sense from an Arm perspective:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-24  3:23     ` Tian, Kevin
  2018-02-26  8:20       ` Jan Beulich
@ 2018-02-26 12:45       ` Wei Liu
  1 sibling, 0 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-26 12:45 UTC (permalink / raw)
  To: Tian, Kevin; +Cc: Xen-devel, Wei Liu, Jan Beulich, Andrew Cooper

On Sat, Feb 24, 2018 at 03:23:48AM +0000, Tian, Kevin wrote:
> > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > Sent: Saturday, February 24, 2018 12:08 AM
> > 
> > On Fri, Feb 23, 2018 at 05:12:05AM +0000, Tian, Kevin wrote:
> > > > From: Wei Liu
> > > > Sent: Thursday, February 22, 2018 5:47 AM
> > > >
> > > > Hi all
> > > >
> > > > At some point I would like to make CONFIG_HVM and CONFIG_PV work.
> > > > The
> > > > passthrough code is one of the road blocks for that work.
> > >
> > > Can you elaborate the motivation of this change? why does someone
> > > want to disable HVM or PV logic completely from hypervisor?
> > >
> > 
> > At some point in the future, we would like to utilise as many hardware
> > features as possible and have an HVM / PVH only setup. At that point PV
> > code will be necessarily and should be preferably compiled out to reduce
> > code size and attack surface.
> > 
> > Having PV compiled out also enable Xen to reclaim some address space
> > from the PV ABI.
> > 
> > But, we understand that PV is here to stay for at least a while and
> > could be useful for some other niche use cases, so upstream have
> > developed a PV-in-PVH shim to continue to support PV guests. The shim is
> > actually yet another configuration of Xen running as a PVH guest but
> > exposes PV ABI to PV guests. We want to disable HVM code in that case,
> > again, to reduce code size and attach surface.
> 
> since it's only temporary, do we really care about code size or attacking
> surface in such case? or is it worthy of the effort regarding to the gain
> which you anticipate?
> 

It depends. We don't really know how long the shim is going to live. If
it is going to be useful for the next 5 to 10 years after we decide to
go HVM / PVH only (which I think that's a reasonable assumption for
enterprise usage), it is worthy of the effort.

> I sort of buy-in CONFIG_PV, but not sure whether CONFIG_HVM is
> required.
> 

I kinda agree: CONFIG_PV is more valuable than CONFIG_HVM. But I have
already done the heavy-lifting to distinguish PV and HVM code so having
CONFIG_HVM work can be seen as a side effect of that.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-26  0:47     ` Marek Marczykowski-Górecki
@ 2018-02-26 12:49       ` Wei Liu
  0 siblings, 0 replies; 31+ messages in thread
From: Wei Liu @ 2018-02-26 12:49 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Tian, Kevin, Wei Liu, Andrew Cooper, Doug Goldstein, Jan Beulich,
	Xen-devel

On Mon, Feb 26, 2018 at 01:47:38AM +0100, Marek Marczykowski-Górecki wrote:
> On Fri, Feb 23, 2018 at 10:39:20PM -0600, Doug Goldstein wrote:
> > On 2/22/18 11:12 PM, Tian, Kevin wrote:
> > >> From: Wei Liu
> > >> Sent: Thursday, February 22, 2018 5:47 AM
> > >>
> > >> Hi all
> > >>
> > >> At some point I would like to make CONFIG_HVM and CONFIG_PV work.
> > >> The
> > >> passthrough code is one of the road blocks for that work.
> > > 
> > > Can you elaborate the motivation of this change? why does someone
> > > want to disable HVM or PV logic completely from hypervisor?
> > 
> > I can say I recall advocating for this at Xen Summit in Cambridge. I
> > believe I talked about it in Toronto as well. There are a number of
> > users of Xen that would certainly want to ship without all the code
> > associated with PV compiled in. Given the nature of design "compromises"
> > in many parts of x86 systems there is certainly a non-zero sum of people
> > that would likely utilize the ability to remove code that doesn't need
> > to be there. I think every individual on this list who has been involved
> > in the security has been in a room of @intel.com folks has seen features
> > vs security win out many times.
> > 
> > I don't think its a hard stretch of the imagination to see people
> > disabling PV in data centers running newer workloads on PVH and HVM
> > only.
> 
> Yes, definitely disabling PV will be useful. Right after being able to
> use PCI passthrough with PVH.
> 
> > I can see the real question being why HVM? That I would say lies
> > with the direction of discretionary access controls in Xen vs mandatory
> > access controls. To solve for the lack of functionality we've grown
> > things like "dmops" and I could certainly see a product like Qubes
> > running only PVH domains in the future.
> > 
> > Since I picked on Qubes I've CC'd Marek.
> 
> So, is it going to be an option to have CONFIG_HVM=n and CONFIG_PVH=y at
> the same time? While currently we do support Windows, so need
> CONFIG_HVM=y, but I can see in some future/alternative version we could
> have even that disabled. For example right now we do have
> CONFIG_SHADOW_PAGING disabled.
> 

Hypervisor doesn't distinguish HVM and PVH at this point. More work is
needed there. But I expect the debate of what each option covers will
take longer than actually writing the code.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
                   ` (10 preceding siblings ...)
  2018-02-23  5:12 ` [PATCH RFC 00/10] x86 passthrough code cleanup Tian, Kevin
@ 2018-03-08 12:18 ` Wei Liu
  2018-03-08 12:37   ` Jan Beulich
  11 siblings, 1 reply; 31+ messages in thread
From: Wei Liu @ 2018-03-08 12:18 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Ping?

Any more comment on this?


On Wed, Feb 21, 2018 at 09:46:51PM +0000, Wei Liu wrote:
> Hi all
> 
> At some point I would like to make CONFIG_HVM and CONFIG_PV work. The
> passthrough code is one of the road blocks for that work.
> 
> A short discussion on #xendevel made me think that having host side code
> regardless of if HVM (the primary user) is configured is desirable because PV
> guests may still have limited use of the hardware, hence this series.
> 
> What I want is to have clear hierarchy of the code and split the host side
> and the guest side code, and start to use CONFIG_HVM where applicable. Luckily
> the amount of work seemed to be smaller than I had expected.
> 
> RFC because there are a few open questions. Please see individual patches.
> 
> Wei.
> 
> Wei Liu (10):
>   passthrough: rearrange x86 code
>   passthrough: split out x86 PCI code to x86/pci.c
>   x86/passthrough: io.c is used for HVM only
>   x86/passthrough: arch_pci_clean_irqs is HVM only
>   x86/passthrough: move hvm_dpci_isairq_eoi
>   passthrough/amd: remove guest iommu support
>   passthrough/amd: split out hvm code from iommu_map.c
>   passthrough/amd: make clear_iommu_pte_present static
>   passthrough/intel: put some code under CONFIG_HVM
>   x86: check hvm domain before calling pt_irq_destroy_bind
> 
>  MAINTAINERS                                        |   8 +-
>  xen/arch/x86/domctl.c                              |   4 +
>  xen/drivers/passthrough/Makefile                   |   3 -
>  xen/drivers/passthrough/amd/iommu_guest.c          | 927 ---------------------
>  xen/drivers/passthrough/pci.c                      |  51 +-
>  xen/drivers/passthrough/x86/Makefile               |   5 +
>  xen/drivers/passthrough/{ => x86}/amd/Makefile     |   2 +-
>  xen/drivers/passthrough/x86/amd/hvm.c              | 108 +++
>  xen/drivers/passthrough/x86/amd/iommu.h            |  32 +
>  xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c |   0
>  xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c  |   2 +-
>  .../passthrough/{ => x86}/amd/iommu_detect.c       |   0
>  xen/drivers/passthrough/{ => x86}/amd/iommu_init.c |  21 +-
>  xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c |   0
>  xen/drivers/passthrough/{ => x86}/amd/iommu_map.c  | 107 +--
>  .../passthrough/{ => x86}/amd/pci_amd_iommu.c      |   2 +-
>  xen/drivers/passthrough/{ => x86}/io.c             |  45 +
>  xen/drivers/passthrough/x86/pci.c                  |  74 ++
>  xen/drivers/passthrough/{ => x86}/vtd/Makefile     |   0
>  xen/drivers/passthrough/{ => x86}/vtd/dmar.c       |   0
>  xen/drivers/passthrough/{ => x86}/vtd/dmar.h       |   0
>  xen/drivers/passthrough/{ => x86}/vtd/extern.h     |   0
>  xen/drivers/passthrough/{ => x86}/vtd/intremap.c   |   0
>  xen/drivers/passthrough/{ => x86}/vtd/iommu.c      |  17 +-
>  xen/drivers/passthrough/{ => x86}/vtd/iommu.h      |   0
>  xen/drivers/passthrough/{ => x86}/vtd/qinval.c     |   2 +-
>  xen/drivers/passthrough/{ => x86}/vtd/quirks.c     |   0
>  xen/drivers/passthrough/{ => x86}/vtd/utils.c      |   0
>  xen/drivers/passthrough/{ => x86}/vtd/vtd.h        |   0
>  xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile |   0
>  xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c    |   2 +-
>  xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c    |  45 -
>  xen/include/asm-x86/amd-iommu.h                    |  51 --
>  xen/include/asm-x86/hvm/svm/amd-iommu-proto.h      |   8 -
>  xen/include/asm-x86/iommu.h                        |   1 -
>  xen/include/xen/iommu.h                            |   1 -
>  xen/include/xen/pci.h                              |   2 +
>  37 files changed, 310 insertions(+), 1210 deletions(-)
>  delete mode 100644 xen/drivers/passthrough/amd/iommu_guest.c
>  rename xen/drivers/passthrough/{ => x86}/amd/Makefile (86%)
>  create mode 100644 xen/drivers/passthrough/x86/amd/hvm.c
>  create mode 100644 xen/drivers/passthrough/x86/amd/iommu.h
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_acpi.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_cmd.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_detect.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_init.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_intr.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/amd/iommu_map.c (86%)
>  rename xen/drivers/passthrough/{ => x86}/amd/pci_amd_iommu.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/io.c (96%)
>  create mode 100644 xen/drivers/passthrough/x86/pci.c
>  rename xen/drivers/passthrough/{ => x86}/vtd/Makefile (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/dmar.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/dmar.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/extern.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/intremap.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/iommu.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/iommu.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/qinval.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/quirks.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/utils.c (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/vtd.h (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/x86/Makefile (100%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/x86/ats.c (99%)
>  rename xen/drivers/passthrough/{ => x86}/vtd/x86/vtd.c (72%)
> 
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 00/10] x86 passthrough code cleanup
  2018-03-08 12:18 ` Wei Liu
@ 2018-03-08 12:37   ` Jan Beulich
  0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2018-03-08 12:37 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Xen-devel

>>> On 08.03.18 at 13:18, <wei.liu2@citrix.com> wrote:
> Ping?

And I'm sorry again, even less so for any RFCs (there are about 100
older ones as well that want looking at).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 01/10] passthrough: rearrange x86 code
  2018-02-21 21:46 ` [PATCH RFC 01/10] passthrough: rearrange x86 code Wei Liu
@ 2018-04-23 15:27   ` Jan Beulich
  0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2018-04-23 15:27 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Kevin Tian, Suravee Suthikulpanit, xen-devel

>>> On 21.02.18 at 22:46, <wei.liu2@citrix.com> wrote:
> Clean up the hierarchy of the directory: put vtd, amd and io.c under
> x86. Adjust makefile and MAINTAINERS.

Considering the history in particular of VT-d, I'm not convinced of this move:
x86 and ia64 did share the bulk of the VT-d code back when ia64 was still in
the tree. Arguably the AMD IOMMU is less likely to be leveraged for some
non-x86 architecture, but I think it would better stay the way it is as well.

As to io.c - it's not currently needed by ARM, but there doesn't look to be
that many x86-specific code in there. I would prefer if the ARM folks could
judge whether that's code they're never going to use, in which case moving
it to x86/ would be fine with me.

What I don't understand here at all is how this movement of code relates
to the PV/HVM splitting.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c
  2018-02-21 21:46 ` [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c Wei Liu
  2018-02-26 10:57   ` Julien Grall
@ 2018-04-23 15:34   ` Jan Beulich
  2018-04-24  9:08     ` Julien Grall
  1 sibling, 1 reply; 31+ messages in thread
From: Jan Beulich @ 2018-04-23 15:34 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel

>>> On 21.02.18 at 22:46, <wei.liu2@citrix.com> wrote:
> Move the functions that reference x86 hvm data structures to its own
> file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.
> 
> There is still one location in that file which references
> arch.hvm_domain, but it is fine because ARM guest is HVM.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> ARM doesn't select HAS_PCI, that's why ARM build is not broken by
> this. AIUI ARM will select HAS_PCI at some point, hence I only move
> the x86 bits.

The fact that this change doesn't actually affect ARM is again an
indication that the decision whether the code you move here is
x86-specific depends on what parts of PCI pass-through ARM is
actually going to want to (re-)use. I'd again prefer to take that
decision either when ARM actually implements that, or based on
the ARM folks clearly foreseeing that this code is not coming
close to anything they may want use.

> @@ -853,7 +804,7 @@ int pci_release_devices(struct domain *d)
>      int ret;
>  
>      pcidevs_lock();
> -    ret = pci_clean_dpci_irqs(d);
> +    ret = arch_pci_clean_irqs(d);

One note regarding the naming: I'd prefer if "dpci" remained part of
the name, unless you mean to get rid of it altogether from the code
base. That'll help (a little) with visually separating code pieces
belonging to the host side of PCI from those belonging to the
passed through side of it (granted - they aren't entirely independent
anyway).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 03/10] x86/passthrough: io.c is used for HVM only
  2018-02-21 21:46 ` [PATCH RFC 03/10] x86/passthrough: io.c is used for HVM only Wei Liu
@ 2018-04-23 15:37   ` Jan Beulich
  0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2018-04-23 15:37 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel

>>> On 21.02.18 at 22:46, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> This file has a few functions that are called in other places. We need
> to provide stubs for them at some point. Currently the declarations
> are in different places. What is the preferred name / location for the
> stubs?

Perhaps dpci.h, and maybe this file should also be named dpci.c? I've
never understood on what grounds its current name was chosen.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 05/10] x86/passthrough: move hvm_dpci_isairq_eoi
  2018-02-21 21:46 ` [PATCH RFC 05/10] x86/passthrough: move hvm_dpci_isairq_eoi Wei Liu
@ 2018-04-23 15:39   ` Jan Beulich
  0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2018-04-23 15:39 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Kevin Tian, xen-devel

>>> On 21.02.18 at 22:46, <wei.liu2@citrix.com> wrote:
> This function is not Intel specific. Move it to io.c along side its
> sole user. Remove declaration in iommu.h.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
(with whatever path name adjustments are necessary depending on the
disposition of patches earlier in the series)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c
  2018-02-21 21:46 ` [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c Wei Liu
@ 2018-04-23 15:43   ` Jan Beulich
  2018-05-02 17:32   ` Suravee Suthikulpanit
  1 sibling, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2018-04-23 15:43 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Suravee Suthikulpanit, xen-devel

>>> On 21.02.18 at 22:46, <wei.liu2@citrix.com> wrote:
> Move and rename update_paging_mode. Create a local header file for
> this and other functions that need exporting.

Functions you move into global name space need suitable prefixes - neither
hvm_update_paging_mode() nor set_iommu_pde_present() clarify that this
is AMD IOMMU specific code.

Also please don't have bool_t or things like u32 in new code you add (here:
the set_iommu_pde_present() prototype).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 09/10] passthrough/intel: put some code under CONFIG_HVM
  2018-02-21 21:47 ` [PATCH RFC 09/10] passthrough/intel: put some code under CONFIG_HVM Wei Liu
@ 2018-04-23 15:47   ` Jan Beulich
  0 siblings, 0 replies; 31+ messages in thread
From: Jan Beulich @ 2018-04-23 15:47 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Kevin Tian, xen-devel

>>> On 21.02.18 at 22:47, <wei.liu2@citrix.com> wrote:
> @@ -1873,6 +1876,7 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
>      return rc;
>  }
>  
> +#ifdef CONFIG_HVM
>  static int __init vtd_ept_page_compatible(struct iommu *iommu)
>  {
>      u64 ept_cap, vtd_cap = iommu->cap;
> @@ -1885,6 +1889,7 @@ static int __init vtd_ept_page_compatible(struct iommu *iommu)
>      return (ept_has_2mb(ept_cap) && opt_hap_2mb) == cap_sps_2mb(vtd_cap) &&
>             (ept_has_1gb(ept_cap) && opt_hap_1gb) == cap_sps_1gb(vtd_cap);
>  }
> +#endif

I think the #ifdef here would better move into the function, such that ...

> @@ -2280,7 +2285,9 @@ int __init intel_vtd_setup(void)
>          if ( !cap_intr_post(iommu->cap) || !cpu_has_cx16 )
>              iommu_intpost = 0;
>  
> +#ifdef CONFIG_HVM
>          if ( !vtd_ept_page_compatible(iommu) )
> +#endif
>              iommu_hap_pt_share = 0;

... this one becomes unnecessary. Furthermore iommu_hap_pt_share is a
variable that's meaningful for HVM only, so another alternative would be to
widen the #ifdef here.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c
  2018-04-23 15:34   ` Jan Beulich
@ 2018-04-24  9:08     ` Julien Grall
  0 siblings, 0 replies; 31+ messages in thread
From: Julien Grall @ 2018-04-24  9:08 UTC (permalink / raw)
  To: Jan Beulich, Wei Liu; +Cc: Andrew Cooper, Stefano Stabellini, xen-devel

Hi,

On 04/23/2018 04:34 PM, Jan Beulich wrote:
>>>> On 21.02.18 at 22:46, <wei.liu2@citrix.com> wrote:
>> Move the functions that reference x86 hvm data structures to its own
>> file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.
>>
>> There is still one location in that file which references
>> arch.hvm_domain, but it is fine because ARM guest is HVM.
>>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> ---
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Julien Grall <julien.grall@arm.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> ARM doesn't select HAS_PCI, that's why ARM build is not broken by
>> this. AIUI ARM will select HAS_PCI at some point, hence I only move
>> the x86 bits.
> 
> The fact that this change doesn't actually affect ARM is again an
> indication that the decision whether the code you move here is
> x86-specific depends on what parts of PCI pass-through ARM is
> actually going to want to (re-)use. I'd again prefer to take that
> decision either when ARM actually implements that, or based on
> the ARM folks clearly foreseeing that this code is not coming
> close to anything they may want use.

I can't see any use of that code on Arm. There are potential to move 
x86-ism outside, but this patch is already a good start.

I already gave my acked-by on a separate e-mail.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 06/10] passthrough/amd: remove guest iommu support
  2018-02-21 21:46 ` [PATCH RFC 06/10] passthrough/amd: remove guest iommu support Wei Liu
@ 2018-05-02 17:10   ` Suravee Suthikulpanit
  0 siblings, 0 replies; 31+ messages in thread
From: Suravee Suthikulpanit @ 2018-05-02 17:10 UTC (permalink / raw)
  To: Wei Liu, Xen-devel; +Cc: Andrew Cooper, Jan Beulich

Wei,

On 2/21/18 3:46 PM, Wei Liu wrote:
> It is never used and it is getting in the way of cleaning up.
> 
> The only callsite of guest_iommu_add_ppr_log has no effect because
> guest iommu is not initialised.
> 
> Signed-off-by: Wei Liu<wei.liu2@citrix.com>
> ---
> Cc: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
> ---
>   xen/drivers/passthrough/x86/amd/Makefile      |   1 -
>   xen/drivers/passthrough/x86/amd/iommu_guest.c | 927 --------------------------
>   xen/drivers/passthrough/x86/amd/iommu_init.c  |  21 +-
>   xen/include/asm-x86/amd-iommu.h               |  51 --
>   xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |   8 -
>   xen/include/asm-x86/iommu.h                   |   1 -
>   6 files changed, 6 insertions(+), 1003 deletions(-)
>   delete mode 100644 xen/drivers/passthrough/x86/amd/iommu_guest.c

Would it be too much trouble if we keep this code for now. There is a plan to support
IOMMU in guest in the near future. This could be clean up at that point if no longer
needed.

Thanks,
Suravee

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c
  2018-02-21 21:46 ` [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c Wei Liu
  2018-04-23 15:43   ` Jan Beulich
@ 2018-05-02 17:32   ` Suravee Suthikulpanit
  1 sibling, 0 replies; 31+ messages in thread
From: Suravee Suthikulpanit @ 2018-05-02 17:32 UTC (permalink / raw)
  To: Wei Liu, Xen-devel; +Cc: Andrew Cooper, Jan Beulich

Wei,

On 2/21/18 3:46 PM, Wei Liu wrote:
> Move and rename update_paging_mode. Create a local header file for
> this and other functions that need exporting.
> 
> Signed-off-by: Wei Liu<wei.liu2@citrix.com>
> ---
> Cc: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
> ---
>   xen/drivers/passthrough/x86/amd/Makefile    |   1 +
>   xen/drivers/passthrough/x86/amd/hvm.c       | 108 ++++++++++++++++++++++++++++
>   xen/drivers/passthrough/x86/amd/iommu.h     |  32 +++++++++
>   xen/drivers/passthrough/x86/amd/iommu_map.c | 103 ++------------------------
>   4 files changed, 148 insertions(+), 96 deletions(-)
>   create mode 100644 xen/drivers/passthrough/x86/amd/hvm.c
>   create mode 100644 xen/drivers/passthrough/x86/amd/iommu.h

I am still not sure why the update_paging_mode need to be moved out of iommu_map.c.
Could you please elaborate?

Thanks,
Suravee

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2018-05-02 17:32 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-21 21:46 [PATCH RFC 00/10] x86 passthrough code cleanup Wei Liu
2018-02-21 21:46 ` [PATCH RFC 01/10] passthrough: rearrange x86 code Wei Liu
2018-04-23 15:27   ` Jan Beulich
2018-02-21 21:46 ` [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c Wei Liu
2018-02-26 10:57   ` Julien Grall
2018-04-23 15:34   ` Jan Beulich
2018-04-24  9:08     ` Julien Grall
2018-02-21 21:46 ` [PATCH RFC 03/10] x86/passthrough: io.c is used for HVM only Wei Liu
2018-04-23 15:37   ` Jan Beulich
2018-02-21 21:46 ` [PATCH RFC 04/10] x86/passthrough: arch_pci_clean_irqs is " Wei Liu
2018-02-21 21:46 ` [PATCH RFC 05/10] x86/passthrough: move hvm_dpci_isairq_eoi Wei Liu
2018-04-23 15:39   ` Jan Beulich
2018-02-21 21:46 ` [PATCH RFC 06/10] passthrough/amd: remove guest iommu support Wei Liu
2018-05-02 17:10   ` Suravee Suthikulpanit
2018-02-21 21:46 ` [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c Wei Liu
2018-04-23 15:43   ` Jan Beulich
2018-05-02 17:32   ` Suravee Suthikulpanit
2018-02-21 21:46 ` [PATCH RFC 08/10] passthrough/amd: make clear_iommu_pte_present static Wei Liu
2018-02-21 21:47 ` [PATCH RFC 09/10] passthrough/intel: put some code under CONFIG_HVM Wei Liu
2018-04-23 15:47   ` Jan Beulich
2018-02-21 21:47 ` [PATCH RFC 10/10] x86: check hvm domain before calling pt_irq_destroy_bind Wei Liu
2018-02-23  5:12 ` [PATCH RFC 00/10] x86 passthrough code cleanup Tian, Kevin
2018-02-23 16:08   ` Wei Liu
2018-02-24  3:23     ` Tian, Kevin
2018-02-26  8:20       ` Jan Beulich
2018-02-26 12:45       ` Wei Liu
2018-02-24  4:39   ` Doug Goldstein
2018-02-26  0:47     ` Marek Marczykowski-Górecki
2018-02-26 12:49       ` Wei Liu
2018-03-08 12:18 ` Wei Liu
2018-03-08 12:37   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.