All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/16] Make CONFIG_HVM work
@ 2018-09-04 16:15 Wei Liu
  2018-09-04 16:15 ` [PATCH v3 01/16] x86: change name of parameter for various invlpg functions Wei Liu
                   ` (15 more replies)
  0 siblings, 16 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu

This series goes through x86 code to make CONFIG_HVM work.

With this series, it is possible to build Xen with PV support only.

Running `xl info` on a host with PV only Xen:

root@lcy2-dt108:~# xl info
host                   : lcy2-dt108
release                : 4.17.0-0.bpo.1-amd64
version                : #1 SMP Debian 4.17.8-1~bpo9+1 (2018-07-23)
machine                : x86_64
nr_cpus                : 8
max_cpu_id             : 7
nr_nodes               : 1
cores_per_socket       : 4
threads_per_core       : 2
cpu_mhz                : 3504.241
hw_caps                : bfebfbff:77faf3ff:2c100800:00000121:0000000f:009c6fbf:00000000:00000100
virt_caps              : directio
total_memory           : 32589
free_memory            : 1033
sharing_freed_memory   : 0
sharing_used_memory    : 0
outstanding_claims     : 0
free_cpus              : 0
xen_major              : 4
xen_minor              : 12
xen_extra              : -unstable
xen_version            : 4.12-unstable
xen_caps               : xen-3.0-x86_64 xen-3.0-x86_32p
xen_scheduler          : credit
xen_pagesize           : 4096
platform_params        : virt_start=0xffff800000000000
xen_changeset          : Fri Aug 24 21:01:40 2018 +0100 git:5036cad847
xen_commandline        : placeholder loglvl=all guest_loglvl=all com2=115200,8n1 ucode=scan console=com2,vga console_to_ring sync_console hvm_fep
cc_compiler            : gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
cc_compile_by          : wei
cc_compile_domain      : uk.xensource.com
cc_compile_date        : Tue Sep  4 16:51:04 BST 2018
build_id               : a3ba77ce4d5117ef0176df6c0e336bacf6c39437

The major goal at the moment is to get something that works first,
then refine code structure later.  Currently CONFIG_HVM is littered in
individual files. In the future some of the code could / should be
moved to files under hvm/ for cleaner split.

I ran some basic PV / PVSHIM VM life cycle tests and XTF PV tests, all
worked.

Baseline:
$ ls -l xen # default build, non-debug
-rwxrwxr-x 1 wei wei 2379388 Aug 17 15:39 xen

$ ls -l xen # PV only, non-debug
-rwxrwxr-x 1 wei wei 1912380 Sep  4 17:00 xen

The PV only Xen is ~19.6% smaller in size.

Wei.

Wei Liu (16):
  x86: change name of parameter for various invlpg functions
  x86: introduce and use a set of internal emulation flags
  x86: XENMEM_resource_ioreq_server is HVM only
  x86: monitor.o is currently HVM only
  x86: PIT emulation is common to both PV and HVM
  libxl: don't set PoD target for PV guests
  x86/p2m/pod: make it build with !CONFIG_HVM
  x86/hvm: rearrange content of hvm.h
  x86: provide stubs, declarations and macros in hvm.h
  x86/mm: put nested p2m code under CONFIG_HVM
  x86/mm: put HVM only code under CONFIG_HVM
  x86/mm: put paging_update_nestedmode under CONFIG_HVM
  xen: connect guest creation with CONFIG_{HVM,PV}
  x86: expose CONFIG_HVM
  x86/pvshim: disable HVM for PV shim
  xen: decouple HVM and IOMMU capabilities

 tools/firmware/xen-dir/shim.config         |   2 +-
 tools/libxl/libxl.c                        |   5 +-
 tools/libxl/libxl.h                        |   6 +
 tools/libxl/libxl_mem.c                    |  22 +--
 tools/libxl/libxl_types.idl                |   1 +
 tools/xl/xl_info.c                         |   5 +-
 xen/arch/x86/Kconfig                       |  11 ++
 xen/arch/x86/Makefile                      |   3 +-
 xen/arch/x86/domain.c                      |  19 ++-
 xen/arch/x86/{hvm/i8254.c => emul-i8254.c} |  18 +-
 xen/arch/x86/hvm/Makefile                  |   1 -
 xen/arch/x86/hvm/ioreq.c                   |   5 +-
 xen/arch/x86/hvm/svm/svm.c                 |  14 +-
 xen/arch/x86/hvm/vmx/vmx.c                 |  12 +-
 xen/arch/x86/mm.c                          |  17 +-
 xen/arch/x86/mm/Makefile                   |  11 +-
 xen/arch/x86/mm/hap/hap.c                  |   2 +-
 xen/arch/x86/mm/mem_access.c               |  18 +-
 xen/arch/x86/mm/mem_sharing.c              |   2 +
 xen/arch/x86/mm/p2m-pt.c                   |   4 +
 xen/arch/x86/mm/p2m.c                      |  53 ++++--
 xen/arch/x86/mm/paging.c                   |   2 +
 xen/arch/x86/mm/shadow/multi.c             |  14 +-
 xen/arch/x86/mm/shadow/none.c              |   2 +-
 xen/arch/x86/sysctl.c                      |   2 +-
 xen/common/domain.c                        |  15 ++
 xen/common/memory.c                        |   3 +-
 xen/common/vm_event.c                      |   6 +
 xen/include/asm-x86/altp2m.h               |  13 +-
 xen/include/asm-x86/domain.h               |  59 +++++--
 xen/include/asm-x86/hvm/domain.h           |   4 +
 xen/include/asm-x86/hvm/hvm.h              | 262 ++++++++++++++++++++---------
 xen/include/asm-x86/hvm/svm/asid.h         |   4 +-
 xen/include/asm-x86/hvm/svm/svm.h          |   4 +-
 xen/include/asm-x86/monitor.h              |  13 ++
 xen/include/asm-x86/p2m.h                  |  47 +++++-
 xen/include/asm-x86/paging.h               |   3 +-
 xen/include/public/sysctl.h                |   8 +-
 38 files changed, 504 insertions(+), 188 deletions(-)
 rename xen/arch/x86/{hvm/i8254.c => emul-i8254.c} (97%)

-- 
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] 57+ messages in thread

* [PATCH v3 01/16] x86: change name of parameter for various invlpg functions
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-06 11:12   ` George Dunlap
  2018-09-13 16:11   ` George Dunlap
  2018-09-04 16:15 ` [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags Wei Liu
                   ` (14 subsequent siblings)
  15 siblings, 2 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit, Jun Nakajima,
	Andrew Cooper, Tim Deegan, George Dunlap, Jan Beulich,
	Boris Ostrovsky, Brian Woods

They all incorrectly named a parameter virtual address while it should
have been linear address.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Acked-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 xen/arch/x86/hvm/svm/svm.c         | 14 +++++++-------
 xen/arch/x86/hvm/vmx/vmx.c         | 12 ++++++------
 xen/arch/x86/mm.c                  | 10 +++++-----
 xen/arch/x86/mm/hap/hap.c          |  2 +-
 xen/arch/x86/mm/shadow/multi.c     | 14 +++++++-------
 xen/arch/x86/mm/shadow/none.c      |  2 +-
 xen/include/asm-x86/hvm/hvm.h      |  6 +++---
 xen/include/asm-x86/hvm/svm/asid.h |  4 ++--
 xen/include/asm-x86/hvm/svm/svm.h  |  4 ++--
 xen/include/asm-x86/paging.h       |  3 ++-
 10 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 0b06e2ff11..34d55b4938 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2488,18 +2488,18 @@ static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs)
 }
 
 static void svm_invlpga_intercept(
-    struct vcpu *v, unsigned long vaddr, uint32_t asid)
+    struct vcpu *v, unsigned long linear, uint32_t asid)
 {
-    svm_invlpga(vaddr,
+    svm_invlpga(linear,
                 (asid == 0)
                 ? v->arch.hvm.n1asid.asid
                 : vcpu_nestedhvm(v).nv_n2asid.asid);
 }
 
-static void svm_invlpg_intercept(unsigned long vaddr)
+static void svm_invlpg_intercept(unsigned long linear)
 {
-    HVMTRACE_LONG_2D(INVLPG, 0, TRC_PAR_LONG(vaddr));
-    paging_invlpg(current, vaddr);
+    HVMTRACE_LONG_2D(INVLPG, 0, TRC_PAR_LONG(linear));
+    paging_invlpg(current, linear);
 }
 
 static bool is_invlpg(const struct x86_emulate_state *state,
@@ -2512,9 +2512,9 @@ static bool is_invlpg(const struct x86_emulate_state *state,
            (ext & 7) == 7;
 }
 
-static void svm_invlpg(struct vcpu *v, unsigned long vaddr)
+static void svm_invlpg(struct vcpu *v, unsigned long linear)
 {
-    svm_asid_g_invlpg(v, vaddr);
+    svm_asid_g_invlpg(v, linear);
 }
 
 static bool svm_get_pending_event(struct vcpu *v, struct x86_event *info)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e926b0b28e..b2e1a28038 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -75,7 +75,7 @@ static void vmx_wbinvd_intercept(void);
 static void vmx_fpu_dirty_intercept(void);
 static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content);
 static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content);
-static void vmx_invlpg(struct vcpu *v, unsigned long vaddr);
+static void vmx_invlpg(struct vcpu *v, unsigned long linear);
 
 /* Values for domain's ->arch.hvm_domain.pi_ops.flags. */
 #define PI_CSW_FROM (1u << 0)
@@ -2595,16 +2595,16 @@ static void vmx_dr_access(unsigned long exit_qualification,
     vmx_update_cpu_exec_control(v);
 }
 
-static void vmx_invlpg_intercept(unsigned long vaddr)
+static void vmx_invlpg_intercept(unsigned long linear)
 {
-    HVMTRACE_LONG_2D(INVLPG, /*invlpga=*/ 0, TRC_PAR_LONG(vaddr));
-    paging_invlpg(current, vaddr);
+    HVMTRACE_LONG_2D(INVLPG, /*invlpga=*/ 0, TRC_PAR_LONG(linear));
+    paging_invlpg(current, linear);
 }
 
-static void vmx_invlpg(struct vcpu *v, unsigned long vaddr)
+static void vmx_invlpg(struct vcpu *v, unsigned long linear)
 {
     if ( cpu_has_vmx_vpid )
-        vpid_sync_vcpu_gva(v, vaddr);
+        vpid_sync_vcpu_gva(v, linear);
 }
 
 static int vmx_vmfunc_intercept(struct cpu_user_regs *regs)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 84979f28d5..409814ce0a 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5793,19 +5793,19 @@ const unsigned long *__init get_platform_badpages(unsigned int *array_size)
     return bad_pages;
 }
 
-void paging_invlpg(struct vcpu *v, unsigned long va)
+void paging_invlpg(struct vcpu *v, unsigned long linear)
 {
-    if ( !is_canonical_address(va) )
+    if ( !is_canonical_address(linear) )
         return;
 
     if ( paging_mode_enabled(v->domain) &&
-         !paging_get_hostmode(v)->invlpg(v, va) )
+         !paging_get_hostmode(v)->invlpg(v, linear) )
         return;
 
     if ( is_pv_vcpu(v) )
-        flush_tlb_one_local(va);
+        flush_tlb_one_local(linear);
     else
-        hvm_invlpg(v, va);
+        hvm_invlpg(v, linear);
 }
 
 /* Build a 32bit PSE page table using 4MB pages. */
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index c53d76cf69..3d651b94c3 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -650,7 +650,7 @@ static int hap_page_fault(struct vcpu *v, unsigned long va,
  * should not be intercepting it.  However, we need to correctly handle
  * getting here from instruction emulation.
  */
-static bool_t hap_invlpg(struct vcpu *v, unsigned long va)
+static bool_t hap_invlpg(struct vcpu *v, unsigned long linear)
 {
     /*
      * Emulate INVLPGA:
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 7bb6f47155..bba573ae87 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3549,7 +3549,7 @@ propagate:
  * instruction should be issued on the hardware, or false if it's safe not
  * to do so.
  */
-static bool sh_invlpg(struct vcpu *v, unsigned long va)
+static bool sh_invlpg(struct vcpu *v, unsigned long linear)
 {
     mfn_t sl1mfn;
     shadow_l2e_t sl2e;
@@ -3572,14 +3572,14 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
     {
         shadow_l3e_t sl3e;
         if ( !(shadow_l4e_get_flags(
-                   sh_linear_l4_table(v)[shadow_l4_linear_offset(va)])
+                   sh_linear_l4_table(v)[shadow_l4_linear_offset(linear)])
                & _PAGE_PRESENT) )
             return false;
         /* This must still be a copy-from-user because we don't have the
          * paging lock, and the higher-level shadows might disappear
          * under our feet. */
         if ( __copy_from_user(&sl3e, (sh_linear_l3_table(v)
-                                      + shadow_l3_linear_offset(va)),
+                                      + shadow_l3_linear_offset(linear)),
                               sizeof (sl3e)) != 0 )
         {
             perfc_incr(shadow_invlpg_fault);
@@ -3589,7 +3589,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
             return false;
     }
 #else /* SHADOW_PAGING_LEVELS == 3 */
-    if ( !(l3e_get_flags(v->arch.paging.shadow.l3table[shadow_l3_linear_offset(va)])
+    if ( !(l3e_get_flags(v->arch.paging.shadow.l3table[shadow_l3_linear_offset(linear)])
            & _PAGE_PRESENT) )
         // no need to flush anything if there's no SL2...
         return false;
@@ -3598,7 +3598,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
     /* This must still be a copy-from-user because we don't have the shadow
      * lock, and the higher-level shadows might disappear under our feet. */
     if ( __copy_from_user(&sl2e,
-                          sh_linear_l2_table(v) + shadow_l2_linear_offset(va),
+                          sh_linear_l2_table(v) + shadow_l2_linear_offset(linear),
                           sizeof (sl2e)) != 0 )
     {
         perfc_incr(shadow_invlpg_fault);
@@ -3642,7 +3642,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
              * feet. */
             if ( __copy_from_user(&sl2e,
                                   sh_linear_l2_table(v)
-                                  + shadow_l2_linear_offset(va),
+                                  + shadow_l2_linear_offset(linear),
                                   sizeof (sl2e)) != 0 )
             {
                 perfc_incr(shadow_invlpg_fault);
@@ -3664,7 +3664,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
                         && page_is_out_of_sync(pg) ) )
             {
                 shadow_l1e_t *sl1;
-                sl1 = sh_linear_l1_table(v) + shadow_l1_linear_offset(va);
+                sl1 = sh_linear_l1_table(v) + shadow_l1_linear_offset(linear);
                 /* Remove the shadow entry that maps this VA */
                 (void) shadow_set_l1e(d, sl1, shadow_l1e_empty(),
                                       p2m_invalid, sl1mfn);
diff --git a/xen/arch/x86/mm/shadow/none.c b/xen/arch/x86/mm/shadow/none.c
index a8c9604cdf..4de645a433 100644
--- a/xen/arch/x86/mm/shadow/none.c
+++ b/xen/arch/x86/mm/shadow/none.c
@@ -37,7 +37,7 @@ static int _page_fault(struct vcpu *v, unsigned long va,
     return 0;
 }
 
-static bool _invlpg(struct vcpu *v, unsigned long va)
+static bool _invlpg(struct vcpu *v, unsigned long linear)
 {
     ASSERT_UNREACHABLE();
     return true;
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 132e62b4f6..6b0e088750 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -160,7 +160,7 @@ struct hvm_function_table {
 
     int  (*event_pending)(struct vcpu *v);
     bool (*get_pending_event)(struct vcpu *v, struct x86_event *info);
-    void (*invlpg)(struct vcpu *v, unsigned long vaddr);
+    void (*invlpg)(struct vcpu *v, unsigned long linear);
 
     int  (*cpu_up_prepare)(unsigned int cpu);
     void (*cpu_dead)(unsigned int cpu);
@@ -454,9 +454,9 @@ static inline int hvm_event_pending(struct vcpu *v)
     return hvm_funcs.event_pending(v);
 }
 
-static inline void hvm_invlpg(struct vcpu *v, unsigned long va)
+static inline void hvm_invlpg(struct vcpu *v, unsigned long linear)
 {
-    hvm_funcs.invlpg(v, va);
+    hvm_funcs.invlpg(v, linear);
 }
 
 /* These bits in CR4 are owned by the host. */
diff --git a/xen/include/asm-x86/hvm/svm/asid.h b/xen/include/asm-x86/hvm/svm/asid.h
index 60cbb7b881..0e5ec3ab78 100644
--- a/xen/include/asm-x86/hvm/svm/asid.h
+++ b/xen/include/asm-x86/hvm/svm/asid.h
@@ -25,11 +25,11 @@
 void svm_asid_init(const struct cpuinfo_x86 *c);
 void svm_asid_handle_vmrun(void);
 
-static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_vaddr)
+static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_linear)
 {
 #if 0
     /* Optimization? */
-    svm_invlpga(g_vaddr, v->arch.hvm.svm.vmcb->guest_asid);
+    svm_invlpga(g_linear, v->arch.hvm.svm.vmcb->guest_asid);
 #endif
 
     /* Safe fallback. Take a new ASID. */
diff --git a/xen/include/asm-x86/hvm/svm/svm.h b/xen/include/asm-x86/hvm/svm/svm.h
index 4e5e142910..8166046a6d 100644
--- a/xen/include/asm-x86/hvm/svm/svm.h
+++ b/xen/include/asm-x86/hvm/svm/svm.h
@@ -40,13 +40,13 @@ static inline void svm_vmsave_pa(paddr_t vmcb)
         : : "a" (vmcb) : "memory" );
 }
 
-static inline void svm_invlpga(unsigned long vaddr, uint32_t asid)
+static inline void svm_invlpga(unsigned long linear, uint32_t asid)
 {
     asm volatile (
         ".byte 0x0f,0x01,0xdf"
         : /* output */
         : /* input */
-        "a" (vaddr), "c" (asid));
+        "a" (linear), "c" (asid));
 }
 
 unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index f440e3e53c..b51e1709d3 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -110,7 +110,8 @@ struct shadow_paging_mode {
 struct paging_mode {
     int           (*page_fault            )(struct vcpu *v, unsigned long va,
                                             struct cpu_user_regs *regs);
-    bool          (*invlpg                )(struct vcpu *v, unsigned long va);
+    bool          (*invlpg                )(struct vcpu *v,
+                                            unsigned long linear);
     unsigned long (*gva_to_gfn            )(struct vcpu *v,
                                             struct p2m_domain *p2m,
                                             unsigned long va,
-- 
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] 57+ messages in thread

* [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
  2018-09-04 16:15 ` [PATCH v3 01/16] x86: change name of parameter for various invlpg functions Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-06 13:27   ` Jan Beulich
  2018-09-04 16:15 ` [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only Wei Liu
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Use these flags in has_* tests and emulation_flags_ok.

Not using raw flags directly enabling DCE to kick in for has_* tests
while at the same time making sure emulation_flags_ok won't go out of
sync.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: new
---
 xen/arch/x86/domain.c        | 13 ++++++----
 xen/include/asm-x86/domain.h | 57 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index cd1419e740..313ebb3221 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -403,19 +403,22 @@ void vcpu_destroy(struct vcpu *v)
 
 static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
 {
+#ifdef CONFIG_HVM
+    /* This doesn't catch !CONFIG_HVM case but it is better than nothing */
+    BUILD_BUG_ON(X86_EMU_ALL != XEN_X86_EMU_ALL);
+#endif
 
     if ( is_hvm_domain(d) )
     {
         if ( is_hardware_domain(d) &&
-             emflags != (XEN_X86_EMU_VPCI | XEN_X86_EMU_LAPIC |
-                         XEN_X86_EMU_IOAPIC) )
+             emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
             return false;
         if ( !is_hardware_domain(d) &&
-             emflags != (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI) &&
-             emflags != XEN_X86_EMU_LAPIC )
+             emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
+             emflags != X86_EMU_LAPIC )
             return false;
     }
-    else if ( emflags != 0 && emflags != XEN_X86_EMU_PIT )
+    else if ( emflags != 0 && emflags != X86_EMU_PIT )
     {
         /* PV or classic PVH. */
         return false;
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index c7cdf974bf..4da4353de7 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -440,18 +440,51 @@ struct arch_domain
     uint32_t emulation_flags;
 } __cacheline_aligned;
 
-#define has_vlapic(d)      (!!((d)->arch.emulation_flags & XEN_X86_EMU_LAPIC))
-#define has_vhpet(d)       (!!((d)->arch.emulation_flags & XEN_X86_EMU_HPET))
-#define has_vpm(d)         (!!((d)->arch.emulation_flags & XEN_X86_EMU_PM))
-#define has_vrtc(d)        (!!((d)->arch.emulation_flags & XEN_X86_EMU_RTC))
-#define has_vioapic(d)     (!!((d)->arch.emulation_flags & XEN_X86_EMU_IOAPIC))
-#define has_vpic(d)        (!!((d)->arch.emulation_flags & XEN_X86_EMU_PIC))
-#define has_vvga(d)        (!!((d)->arch.emulation_flags & XEN_X86_EMU_VGA))
-#define has_viommu(d)      (!!((d)->arch.emulation_flags & XEN_X86_EMU_IOMMU))
-#define has_vpit(d)        (!!((d)->arch.emulation_flags & XEN_X86_EMU_PIT))
-#define has_pirq(d)        (!!((d)->arch.emulation_flags & \
-                            XEN_X86_EMU_USE_PIRQ))
-#define has_vpci(d)        (!!((d)->arch.emulation_flags & XEN_X86_EMU_VPCI))
+#ifdef CONFIG_HVM
+#define X86_EMU_LAPIC    XEN_X86_EMU_LAPIC
+#define X86_EMU_HPET     XEN_X86_EMU_HPET
+#define X86_EMU_PM       XEN_X86_EMU_PM
+#define X86_EMU_RTC      XEN_X86_EMU_RTC
+#define X86_EMU_IOAPIC   XEN_X86_EMU_IOAPIC
+#define X86_EMU_PIC      XEN_X86_EMU_PIC
+#define X86_EMU_VGA      XEN_X86_EMU_VGA
+#define X86_EMU_IOMMU    XEN_X86_EMU_IOMMU
+#define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
+#define X86_EMU_VPCI     XEN_X86_EMU_VPCI
+#else
+#define X86_EMU_LAPIC    0
+#define X86_EMU_HPET     0
+#define X86_EMU_PM       0
+#define X86_EMU_RTC      0
+#define X86_EMU_IOAPIC   0
+#define X86_EMU_PIC      0
+#define X86_EMU_VGA      0
+#define X86_EMU_IOMMU    0
+#define X86_EMU_USE_PIRQ 0
+#define X86_EMU_VPCI     0
+#endif
+
+#define X86_EMU_PIT     XEN_X86_EMU_PIT
+
+/* This must match XEN_X86_EMU_ALL in xen.h */
+#define X86_EMU_ALL             (X86_EMU_LAPIC | X86_EMU_HPET |         \
+                                 X86_EMU_PM | X86_EMU_RTC |             \
+                                 X86_EMU_IOAPIC | X86_EMU_PIC |         \
+                                 X86_EMU_VGA | X86_EMU_IOMMU |          \
+                                 X86_EMU_PIT | X86_EMU_USE_PIRQ |       \
+                                 X86_EMU_VPCI)
+
+#define has_vlapic(d)      (!!((d)->arch.emulation_flags & X86_EMU_LAPIC))
+#define has_vhpet(d)       (!!((d)->arch.emulation_flags & X86_EMU_HPET))
+#define has_vpm(d)         (!!((d)->arch.emulation_flags & X86_EMU_PM))
+#define has_vrtc(d)        (!!((d)->arch.emulation_flags & X86_EMU_RTC))
+#define has_vioapic(d)     (!!((d)->arch.emulation_flags & X86_EMU_IOAPIC))
+#define has_vpic(d)        (!!((d)->arch.emulation_flags & X86_EMU_PIC))
+#define has_vvga(d)        (!!((d)->arch.emulation_flags & X86_EMU_VGA))
+#define has_viommu(d)      (!!((d)->arch.emulation_flags & X86_EMU_IOMMU))
+#define has_vpit(d)        (!!((d)->arch.emulation_flags & X86_EMU_PIT))
+#define has_pirq(d)        (!!((d)->arch.emulation_flags & X86_EMU_USE_PIRQ))
+#define has_vpci(d)        (!!((d)->arch.emulation_flags & X86_EMU_VPCI))
 
 #define has_arch_pdevs(d)    (!list_empty(&(d)->arch.pdev_list))
 
-- 
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] 57+ messages in thread

* [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
  2018-09-04 16:15 ` [PATCH v3 01/16] x86: change name of parameter for various invlpg functions Wei Liu
  2018-09-04 16:15 ` [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-04 16:24   ` Paul Durrant
  2018-09-04 16:42   ` Wei Liu
  2018-09-04 16:15 ` [PATCH v3 04/16] x86: monitor.o is currently " Wei Liu
                   ` (12 subsequent siblings)
  15 siblings, 2 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Wei Liu, Jan Beulich

Put the entire case branch under CONFIG_HVM.

Nonetheless check HVM before trying to get ioreq server.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3:
1. put an assert in hvm_get_ioreq_server_frame
2. remove redundant assignment of rc

v2: put entire case branch under CONFIG_HVM
---
 xen/arch/x86/hvm/ioreq.c | 5 ++---
 xen/arch/x86/mm.c        | 5 +++++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 138ed697cd..fecca58a3d 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -961,12 +961,11 @@ int hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
     struct hvm_ioreq_server *s;
     int rc;
 
+    ASSERT(is_hvm_domain(d));
+
     if ( id == DEFAULT_IOSERVID )
         return -EOPNOTSUPP;
 
-    if ( !is_hvm_domain(d) )
-        return -EINVAL;
-
     spin_lock_recursive(&d->arch.hvm.ioreq_server.lock);
 
     s = get_ioreq_server(d, id);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 409814ce0a..baea2f5e63 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4381,12 +4381,16 @@ int arch_acquire_resource(struct domain *d, unsigned int type,
 
     switch ( type )
     {
+#ifdef CONFIG_HVM
     case XENMEM_resource_ioreq_server:
     {
         ioservid_t ioservid = id;
         unsigned int i;
 
         rc = -EINVAL;
+        if ( !is_hvm_domain(d) )
+            break;
+
         if ( id != (unsigned int)ioservid )
             break;
 
@@ -4409,6 +4413,7 @@ int arch_acquire_resource(struct domain *d, unsigned int type,
         *flags |= XENMEM_rsrc_acq_caller_owned;
         break;
     }
+#endif
 
     default:
         rc = -EOPNOTSUPP;
-- 
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] 57+ messages in thread

* [PATCH v3 04/16] x86: monitor.o is currently HVM only
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (2 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-04 16:35   ` Razvan Cojocaru
  2018-09-04 16:15 ` [PATCH v3 05/16] x86: PIT emulation is common to both PV and HVM Wei Liu
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Tamas K Lengyel, Wei Liu, Razvan Cojocaru, Jan Beulich

There has been plan to make PV work, but it is not yet there.  Provide
stubs to make it build with !CONFIG_HVM.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3:
1. return EOPNOUTSUPP instead of 0
2. fix style issue
---
 xen/arch/x86/Makefile         |  2 +-
 xen/include/asm-x86/monitor.h | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 261c598ff3..2f2ad3adfd 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -45,7 +45,7 @@ obj-y += microcode_amd.o
 obj-y += microcode_intel.o
 obj-y += microcode.o
 obj-y += mm.o x86_64/mm.o
-obj-y += monitor.o
+obj-$(CONFIG_HVM) += monitor.o
 obj-y += mpparse.o
 obj-y += nmi.o
 obj-y += numa.o
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 49889033f4..e02484f56b 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -99,10 +99,23 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
 int arch_monitor_domctl_event(struct domain *d,
                               struct xen_domctl_monitor_op *mop);
 
+#ifdef CONFIG_HVM
+
 int arch_monitor_init_domain(struct domain *d);
 
 void arch_monitor_cleanup_domain(struct domain *d);
 
+#else
+
+static inline int arch_monitor_init_domain(struct domain *d)
+{
+    return -EOPNOTSUPP;
+}
+
+static inline void arch_monitor_cleanup_domain(struct domain *d) {}
+
+#endif
+
 bool monitored_msr(const struct domain *d, u32 msr);
 bool monitored_msr_onchangeonly(const struct domain *d, u32 msr);
 
-- 
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] 57+ messages in thread

* [PATCH v3 05/16] x86: PIT emulation is common to both PV and HVM
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (3 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 04/16] x86: monitor.o is currently " Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-06 14:26   ` Jan Beulich
  2018-09-04 16:15 ` [PATCH v3 06/16] libxl: don't set PoD target for PV guests Wei Liu
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Move the file to x86 common code and change its name to emul-i8254.c.

Put HVM only code under CONFIG_HVM or is_hvm_domain.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: remove some CONFIG_HVMs, rely on DCE
v2: move the whole file.
---
 xen/arch/x86/Makefile                      |  1 +
 xen/arch/x86/{hvm/i8254.c => emul-i8254.c} | 18 +++++++++++++-----
 xen/arch/x86/hvm/Makefile                  |  1 -
 3 files changed, 14 insertions(+), 6 deletions(-)
 rename xen/arch/x86/{hvm/i8254.c => emul-i8254.c} (97%)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 2f2ad3adfd..162b0b94c0 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -27,6 +27,7 @@ obj-y += domain.o
 obj-bin-y += dom0_build.init.o
 obj-y += domain_page.o
 obj-y += e820.o
+obj-y += emul-i8254.o
 obj-y += extable.o
 obj-y += flushtlb.o
 obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
diff --git a/xen/arch/x86/hvm/i8254.c b/xen/arch/x86/emul-i8254.c
similarity index 97%
rename from xen/arch/x86/hvm/i8254.c
rename to xen/arch/x86/emul-i8254.c
index b8ec56f8d3..7f1ded2623 100644
--- a/xen/arch/x86/hvm/i8254.c
+++ b/xen/arch/x86/emul-i8254.c
@@ -379,6 +379,7 @@ static uint32_t pit_ioport_read(struct PITState *pit, uint32_t addr)
     return ret;
 }
 
+#ifdef CONFIG_HVM
 void pit_stop_channel0_irq(PITState *pit)
 {
     if ( !has_vpit(current->domain) )
@@ -438,6 +439,7 @@ static int pit_load(struct domain *d, hvm_domain_context_t *h)
 }
 
 HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, pit_load, 1, HVMSR_PER_DOM);
+#endif
 
 void pit_reset(struct domain *d)
 {
@@ -448,9 +450,12 @@ void pit_reset(struct domain *d)
     if ( !has_vpit(d) )
         return;
 
-    TRACE_0D(TRC_HVM_EMUL_PIT_STOP_TIMER);
-    destroy_periodic_time(&pit->pt0);
-    pit->pt0.source = PTSRC_isa;
+    if ( is_hvm_domain(d) )
+    {
+        TRACE_0D(TRC_HVM_EMUL_PIT_STOP_TIMER);
+        destroy_periodic_time(&pit->pt0);
+        pit->pt0.source = PTSRC_isa;
+    }
 
     spin_lock(&pit->lock);
 
@@ -490,8 +495,11 @@ void pit_deinit(struct domain *d)
     if ( !has_vpit(d) )
         return;
 
-    TRACE_0D(TRC_HVM_EMUL_PIT_STOP_TIMER);
-    destroy_periodic_time(&pit->pt0);
+    if ( is_hvm_domain(d) )
+    {
+        TRACE_0D(TRC_HVM_EMUL_PIT_STOP_TIMER);
+        destroy_periodic_time(&pit->pt0);
+    }
 }
 
 /* the intercept action for PIT DM retval:0--not handled; 1--handled */  
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 5bd38f633f..5e04bc1429 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -10,7 +10,6 @@ obj-y += grant_table.o
 obj-y += hpet.o
 obj-y += hvm.o
 obj-y += hypercall.o
-obj-y += i8254.o
 obj-y += intercept.o
 obj-y += io.o
 obj-y += ioreq.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] 57+ messages in thread

* [PATCH v3 06/16] libxl: don't set PoD target for PV guests
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (4 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 05/16] x86: PIT emulation is common to both PV and HVM Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-07 13:44   ` Ian Jackson
  2018-09-04 16:15 ` [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM Wei Liu
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu

Previously PoD target was unconditionally set for both PV and HVM
guests, but in fact PoD has always been an HVM (now PVH as well) only
feature.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_mem.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
index e551e09fed..448a2af8fd 100644
--- a/tools/libxl/libxl_mem.c
+++ b/tools/libxl/libxl_mem.c
@@ -298,16 +298,18 @@ retry_transaction:
         }
     }
 
-    r = xc_domain_set_pod_target(ctx->xch, domid,
-            (new_target_memkb + size) / 4, NULL, NULL, NULL);
-    if (r != 0) {
-        LOGED(ERROR, domid,
-              "xc_domain_set_pod_target memkb=%"PRIu64" failed rc=%d\n",
-              (new_target_memkb + size) / 4,
-              r);
-        abort_transaction = 1;
-        rc = ERROR_FAIL;
-        goto out;
+    if (d_config.c_info.type != LIBXL_DOMAIN_TYPE_PV) {
+        r = xc_domain_set_pod_target(ctx->xch, domid,
+                (new_target_memkb + size) / 4, NULL, NULL, NULL);
+        if (r != 0) {
+            LOGED(ERROR, domid,
+                  "xc_domain_set_pod_target memkb=%"PRIu64" failed rc=%d\n",
+                  (new_target_memkb + size) / 4,
+                  r);
+            abort_transaction = 1;
+            rc = ERROR_FAIL;
+            goto out;
+        }
     }
 
     libxl__xs_printf(gc, t, GCSPRINTF("%s/memory/target", dompath),
-- 
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] 57+ messages in thread

* [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (5 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 06/16] libxl: don't set PoD target for PV guests Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-04 17:08   ` Razvan Cojocaru
                     ` (2 more replies)
  2018-09-04 16:15 ` [PATCH v3 08/16] x86/hvm: rearrange content of hvm.h Wei Liu
                   ` (8 subsequent siblings)
  15 siblings, 3 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Razvan Cojocaru, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
	Tamas K Lengyel, Jan Beulich

Populate-on-demand is HVM only.

Provide a bunch of stubs for common p2m code and guard one invocation
of guest_physmap_mark_populate_on_demand with is_hvm_domain.

Put relevant fields in p2m_domain and code which touches those fields
under CONFIG_HVM.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: Put pod related fields and code under CONFIG_HVM.

Note, this should be applied after the toolstack side change.
---
 xen/arch/x86/mm.c         |  2 ++
 xen/arch/x86/mm/p2m-pt.c  |  4 ++++
 xen/arch/x86/mm/p2m.c     | 22 ++++++++++++++++++++--
 xen/common/memory.c       |  3 ++-
 xen/common/vm_event.c     |  4 ++++
 xen/include/asm-x86/p2m.h | 38 ++++++++++++++++++++++++++++++++++----
 6 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index baea2f5e63..54023e0c69 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4588,6 +4588,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         return 0;
     }
 
+#ifdef CONFIG_HVM
     case XENMEM_set_pod_target:
     case XENMEM_get_pod_target:
     {
@@ -4644,6 +4645,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         rcu_unlock_domain(d);
         return rc;
     }
+#endif
 
     default:
         return subarch_memory_op(cmd, arg);
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index b8c5d2ed26..74884ea063 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -974,7 +974,9 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
     unsigned long mfn, gfn, m2pfn;
 
     ASSERT(p2m_locked_by_me(p2m));
+#ifdef CONFIG_HVM
     ASSERT(pod_locked_by_me(p2m));
+#endif
 
     /* Audit part one: walk the domain's p2m table, checking the entries. */
     if ( pagetable_get_pfn(p2m_get_pagetable(p2m)) != 0 )
@@ -1105,6 +1107,7 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
         unmap_domain_page(l4e);
     }
 
+#ifdef CONFIG_HVM
     if ( entry_count != p2m->pod.entry_count )
     {
         printk("%s: refcounted entry count %ld, audit count %lu!\n",
@@ -1113,6 +1116,7 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
                entry_count);
         BUG();
     }
+#endif
 
     return pmbad;
 }
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 6020553c17..79d0e7203a 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
 /* Init the datastructures for later use by the p2m code */
 static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
 {
-    unsigned int i;
     int ret = 0;
+#ifdef CONFIG_HVM
+    unsigned int i;
+#endif
 
     mm_rwlock_init(&p2m->lock);
-    mm_lock_init(&p2m->pod.lock);
     INIT_LIST_HEAD(&p2m->np2m_list);
     INIT_PAGE_LIST_HEAD(&p2m->pages);
+
+#ifdef CONFIG_HVM
+    mm_lock_init(&p2m->pod.lock);
     INIT_PAGE_LIST_HEAD(&p2m->pod.super);
     INIT_PAGE_LIST_HEAD(&p2m->pod.single);
+#endif
 
     p2m->domain = d;
     p2m->default_access = p2m_access_rwx;
@@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
     p2m->np2m_base = P2M_BASE_EADDR;
     p2m->np2m_generation = 0;
 
+#ifdef CONFIG_HVM
     for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
         p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
+#endif
 
     if ( hap_enabled(d) && cpu_has_vmx )
         ret = ept_p2m_init(p2m);
@@ -917,6 +924,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
                  gfn_x(gfn), mfn_x(mfn));
         rc = p2m_set_entry(p2m, gfn, INVALID_MFN, page_order,
                            p2m_invalid, p2m->default_access);
+#ifdef CONFIG_HVM
         if ( rc == 0 )
         {
             pod_lock(p2m);
@@ -924,6 +932,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
             BUG_ON(p2m->pod.entry_count < 0);
             pod_unlock(p2m);
         }
+#endif
     }
 
 out:
@@ -1114,6 +1123,7 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn_l,
     if ( rc )
         gdprintk(XENLOG_ERR, "p2m_set_entry: %#lx:%u -> %d (0x%"PRI_mfn")\n",
                  gfn_l, order, rc, mfn_x(mfn));
+#ifdef CONFIG_HVM
     else if ( p2m_is_pod(ot) )
     {
         pod_lock(p2m);
@@ -1121,6 +1131,7 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn_l,
         BUG_ON(p2m->pod.entry_count < 0);
         pod_unlock(p2m);
     }
+#endif
     gfn_unlock(p2m, gfn, order);
 
     return rc;
@@ -1743,9 +1754,11 @@ p2m_flush_table_locked(struct p2m_domain *p2m)
      * when discarding them.
      */
     ASSERT(!p2m_is_hostp2m(p2m));
+#ifdef CONFIG_HVM
     /* Nested p2m's do not do pod, hence the asserts (and no pod lock)*/
     ASSERT(page_list_empty(&p2m->pod.super));
     ASSERT(page_list_empty(&p2m->pod.single));
+#endif
 
     /* No need to flush if it's already empty */
     if ( p2m_is_nestedp2m(p2m) && p2m->np2m_base == P2M_BASE_EADDR )
@@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
     P2M_PRINTK("p2m audit starts\n");
 
     p2m_lock(p2m);
+
+#ifdef CONFIG_HVM
     pod_lock(p2m);
+#endif
 
     if (p2m->audit_p2m)
         pmbad = p2m->audit_p2m(p2m);
@@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
     }
     spin_unlock(&d->page_alloc_lock);
 
+#ifdef CONFIG_HVM
     pod_unlock(p2m);
+#endif
     p2m_unlock(p2m);
  
     P2M_PRINTK("p2m audit complete\n");
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 996f94b103..5c71ce13ce 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -210,7 +210,8 @@ static void populate_physmap(struct memop_args *a)
             if ( d == curr_d )
                 goto out;
 
-            if ( guest_physmap_mark_populate_on_demand(d, gpfn,
+            if ( is_hvm_domain(d) &&
+                 guest_physmap_mark_populate_on_demand(d, gpfn,
                                                        a->extent_order) < 0 )
                 goto out;
         }
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 4793aacc35..a3bbfc9474 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
         {
         case XEN_VM_EVENT_ENABLE:
         {
+#ifdef CONFIG_HVM
             struct p2m_domain *p2m = p2m_get_hostp2m(d);
+#endif
 
             rc = -EOPNOTSUPP;
             /* hvm fixme: p2m_is_foreign types need addressing */
@@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
             if ( unlikely(need_iommu(d)) )
                 break;
 
+#ifdef CONFIG_HVM
             rc = -EXDEV;
             /* Disallow paging in a PoD guest */
             if ( p2m->pod.entry_count )
                 break;
+#endif
 
             /* domain_pause() not required here, see XSA-99 */
             rc = vm_event_enable(d, vec, &d->vm_event_paging, _VPF_mem_paging,
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index d4b3cfcb6e..3785598f54 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -306,6 +306,7 @@ struct p2m_domain {
      * to resume the search */
     unsigned long next_shared_gfn_to_relinquish;
 
+#ifdef CONFIG_HVM
     /* Populate-on-demand variables
      * All variables are protected with the pod lock. We cannot rely on
      * the p2m lock if it's turned into a fine-grained lock.
@@ -337,6 +338,8 @@ struct p2m_domain {
         mm_lock_t        lock;         /* Locking of private pod structs,   *
                                         * not relying on the p2m lock.      */
     } pod;
+#endif
+
     union {
         struct ept_data ept;
         /* NPT-equivalent structure could be added here. */
@@ -646,6 +649,12 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
 /* Dump PoD information about the domain */
 void p2m_pod_dump_data(struct domain *d);
 
+#ifdef CONFIG_HVM
+
+/* Called by p2m code when demand-populating a PoD page */
+bool
+p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn, unsigned int order);
+
 /* Move all pages from the populate-on-demand cache to the domain page_list
  * (usually in preparation for domain destruction) */
 int p2m_pod_empty_cache(struct domain *d);
@@ -662,6 +671,31 @@ p2m_pod_offline_or_broken_hit(struct page_info *p);
 void
 p2m_pod_offline_or_broken_replace(struct page_info *p);
 
+#else
+
+static inline bool
+p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn, unsigned int order)
+{
+    return false;
+}
+
+static inline int p2m_pod_empty_cache(struct domain *d)
+{
+    return 0;
+}
+
+static inline int p2m_pod_offline_or_broken_hit(struct page_info *p)
+{
+    return 0;
+}
+
+static inline void p2m_pod_offline_or_broken_replace(struct page_info *p)
+{
+    ASSERT_UNREACHABLE();
+}
+
+#endif
+
 
 /*
  * Paging to disk and page-sharing
@@ -730,10 +764,6 @@ extern void audit_p2m(struct domain *d,
 #define P2M_DEBUG(f, a...) do { (void)(f); } while(0)
 #endif
 
-/* Called by p2m code when demand-populating a PoD page */
-bool
-p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn, unsigned int order);
-
 /*
  * Functions specific to the p2m-pt implementation
  */
-- 
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] 57+ messages in thread

* [PATCH v3 08/16] x86/hvm: rearrange content of hvm.h
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (6 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-07  6:52   ` Jan Beulich
  2018-09-04 16:15 ` [PATCH v3 09/16] x86: provide stubs, declarations and macros in hvm.h Wei Liu
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Move enum and function declarations to first half of the file.

Static inline functions and macros, which reference HVM specific
fields directly are grouped together in second half of the file.

The movement is needed because in a later patch the second half is
going to be enclosed in CONFIG_HVM.

Pure code movement. No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/asm-x86/hvm/hvm.h | 159 +++++++++++++++++++++---------------------
 1 file changed, 80 insertions(+), 79 deletions(-)

diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 6b0e088750..0c321409ee 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -257,16 +257,6 @@ void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
 int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat);
 
 u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
-#define hvm_get_guest_tsc(v) hvm_get_guest_tsc_fixed(v, 0)
-
-#define hvm_tsc_scaling_supported \
-    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
-
-#define hvm_default_tsc_scaling_ratio \
-    (1ULL << hvm_funcs.tsc_scaling.ratio_frac_bits)
-
-#define hvm_tsc_scaling_ratio(d) \
-    ((d)->arch.hvm.tsc_scaling_ratio)
 
 u64 hvm_scale_tsc(const struct domain *d, u64 tsc);
 u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz);
@@ -274,7 +264,6 @@ u64 hvm_get_tsc_scaling_ratio(u32 gtsc_khz);
 void hvm_init_guest_time(struct domain *d);
 void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
 uint64_t hvm_get_guest_time_fixed(const struct vcpu *v, uint64_t at_tsc);
-#define hvm_get_guest_time(v) hvm_get_guest_time_fixed(v, 0)
 
 int vmsi_deliver(
     struct domain *d, int vector,
@@ -284,6 +273,86 @@ struct hvm_pirq_dpci;
 void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *);
 int hvm_girq_dest_2_vcpu_id(struct domain *d, uint8_t dest, uint8_t dest_mode);
 
+enum hvm_intblk
+hvm_interrupt_blocked(struct vcpu *v, struct hvm_intack intack);
+
+void hvm_hypercall_page_initialise(struct domain *d, void *hypercall_page);
+
+void hvm_get_segment_register(struct vcpu *v, enum x86_segment seg,
+                              struct segment_register *reg);
+void hvm_set_segment_register(struct vcpu *v, enum x86_segment seg,
+                              struct segment_register *reg);
+
+bool hvm_set_guest_bndcfgs(struct vcpu *v, u64 val);
+
+bool hvm_check_cpuid_faulting(struct vcpu *v);
+void hvm_migrate_timers(struct vcpu *v);
+void hvm_do_resume(struct vcpu *v);
+void hvm_migrate_pirqs(struct vcpu *v);
+
+void hvm_inject_event(const struct x86_event *event);
+
+int hvm_event_needs_reinjection(uint8_t type, uint8_t vector);
+
+uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2);
+
+void hvm_set_rdtsc_exiting(struct domain *d, bool_t enable);
+
+enum hvm_task_switch_reason { TSW_jmp, TSW_iret, TSW_call_or_int };
+void hvm_task_switch(
+    uint16_t tss_sel, enum hvm_task_switch_reason taskswitch_reason,
+    int32_t errcode);
+
+enum hvm_access_type {
+    hvm_access_insn_fetch,
+    hvm_access_none,
+    hvm_access_read,
+    hvm_access_write
+};
+bool_t hvm_virtual_to_linear_addr(
+    enum x86_segment seg,
+    const struct segment_register *reg,
+    unsigned long offset,
+    unsigned int bytes,
+    enum hvm_access_type access_type,
+    const struct segment_register *active_cs,
+    unsigned long *linear_addr);
+
+void *hvm_map_guest_frame_rw(unsigned long gfn, bool_t permanent,
+                             bool_t *writable);
+void *hvm_map_guest_frame_ro(unsigned long gfn, bool_t permanent);
+void hvm_unmap_guest_frame(void *p, bool_t permanent);
+void hvm_mapped_guest_frames_mark_dirty(struct domain *);
+
+int hvm_debug_op(struct vcpu *v, int32_t op);
+
+/* Caller should pause vcpu before calling this function */
+void hvm_toggle_singlestep(struct vcpu *v);
+
+int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
+                              struct npfec npfec);
+
+int hvm_x2apic_msr_read(struct vcpu *v, unsigned int msr, uint64_t *msr_content);
+int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t msr_content);
+
+/* Check CR4/EFER values */
+const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
+                           signed int cr0_pg);
+unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore);
+
+#define hvm_get_guest_tsc(v) hvm_get_guest_tsc_fixed(v, 0)
+
+#define hvm_tsc_scaling_supported \
+    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
+
+#define hvm_default_tsc_scaling_ratio \
+    (1ULL << hvm_funcs.tsc_scaling.ratio_frac_bits)
+
+#define hvm_tsc_scaling_ratio(d) \
+    ((d)->arch.hvm.tsc_scaling_ratio)
+
+#define hvm_get_guest_time(v) hvm_get_guest_time_fixed(v, 0)
+
 #define hvm_paging_enabled(v) \
     (!!((v)->arch.hvm.guest_cr[0] & X86_CR0_PG))
 #define hvm_wp_enabled(v) \
@@ -307,9 +376,6 @@ int hvm_girq_dest_2_vcpu_id(struct domain *d, uint8_t dest, uint8_t dest_mode);
 
 #define hvm_long_mode_active(v) (!!((v)->arch.hvm.guest_efer & EFER_LMA))
 
-enum hvm_intblk
-hvm_interrupt_blocked(struct vcpu *v, struct hvm_intack intack);
-
 static inline int
 hvm_guest_x86_mode(struct vcpu *v)
 {
@@ -363,20 +429,12 @@ static inline void hvm_flush_guest_tlbs(void)
         hvm_asid_flush_core();
 }
 
-void hvm_hypercall_page_initialise(struct domain *d,
-                                   void *hypercall_page);
-
 static inline unsigned int
 hvm_get_cpl(struct vcpu *v)
 {
     return hvm_funcs.get_cpl(v);
 }
 
-void hvm_get_segment_register(struct vcpu *v, enum x86_segment seg,
-                              struct segment_register *reg);
-void hvm_set_segment_register(struct vcpu *v, enum x86_segment seg,
-                              struct segment_register *reg);
-
 static inline unsigned long hvm_get_shadow_gs_base(struct vcpu *v)
 {
     return hvm_funcs.get_shadow_gs_base(v);
@@ -388,8 +446,6 @@ static inline bool hvm_get_guest_bndcfgs(struct vcpu *v, u64 *val)
            hvm_funcs.get_guest_bndcfgs(v, val);
 }
 
-bool hvm_set_guest_bndcfgs(struct vcpu *v, u64 val);
-
 #define has_hvm_params(d) \
     ((d)->arch.hvm.params != NULL)
 
@@ -405,13 +461,6 @@ bool hvm_set_guest_bndcfgs(struct vcpu *v, u64 val);
 #define has_viridian_apic_assist(d) \
     (is_viridian_domain(d) && (viridian_feature_mask(d) & HVMPV_apic_assist))
 
-bool hvm_check_cpuid_faulting(struct vcpu *v);
-void hvm_migrate_timers(struct vcpu *v);
-void hvm_do_resume(struct vcpu *v);
-void hvm_migrate_pirqs(struct vcpu *v);
-
-void hvm_inject_event(const struct x86_event *event);
-
 static inline void hvm_inject_exception(
     unsigned int vector, unsigned int type,
     unsigned int insn_len, int error_code)
@@ -468,12 +517,6 @@ static inline void hvm_invlpg(struct vcpu *v, unsigned long linear)
                        (1U << TRAP_alignment_check) | \
                        (1U << TRAP_machine_check))
 
-int hvm_event_needs_reinjection(uint8_t type, uint8_t vector);
-
-uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2);
-
-void hvm_set_rdtsc_exiting(struct domain *d, bool_t enable);
-
 static inline int hvm_cpu_up(void)
 {
     return (hvm_funcs.cpu_up ? hvm_funcs.cpu_up() : 0);
@@ -490,43 +533,12 @@ static inline unsigned int hvm_get_insn_bytes(struct vcpu *v, uint8_t *buf)
     return (hvm_funcs.get_insn_bytes ? hvm_funcs.get_insn_bytes(v, buf) : 0);
 }
 
-enum hvm_task_switch_reason { TSW_jmp, TSW_iret, TSW_call_or_int };
-void hvm_task_switch(
-    uint16_t tss_sel, enum hvm_task_switch_reason taskswitch_reason,
-    int32_t errcode);
-
-enum hvm_access_type {
-    hvm_access_insn_fetch,
-    hvm_access_none,
-    hvm_access_read,
-    hvm_access_write
-};
-bool_t hvm_virtual_to_linear_addr(
-    enum x86_segment seg,
-    const struct segment_register *reg,
-    unsigned long offset,
-    unsigned int bytes,
-    enum hvm_access_type access_type,
-    const struct segment_register *active_cs,
-    unsigned long *linear_addr);
-
-void *hvm_map_guest_frame_rw(unsigned long gfn, bool_t permanent,
-                             bool_t *writable);
-void *hvm_map_guest_frame_ro(unsigned long gfn, bool_t permanent);
-void hvm_unmap_guest_frame(void *p, bool_t permanent);
-void hvm_mapped_guest_frames_mark_dirty(struct domain *);
-
 static inline void hvm_set_info_guest(struct vcpu *v)
 {
     if ( hvm_funcs.set_info_guest )
         return hvm_funcs.set_info_guest(v);
 }
 
-int hvm_debug_op(struct vcpu *v, int32_t op);
-
-/* Caller should pause vcpu before calling this function */
-void hvm_toggle_singlestep(struct vcpu *v);
-
 static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
 {
 #ifndef NDEBUG
@@ -542,18 +554,12 @@ static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
 #endif
 }
 
-int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
-                              struct npfec npfec);
-
 #define hvm_msr_tsc_aux(v) ({                                               \
     struct domain *__d = (v)->domain;                                       \
     (__d->arch.tsc_mode == TSC_MODE_PVRDTSCP)                               \
         ? (u32)__d->arch.incarnation : (u32)(v)->arch.hvm.msr_tsc_aux;      \
 })
 
-int hvm_x2apic_msr_read(struct vcpu *v, unsigned int msr, uint64_t *msr_content);
-int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t msr_content);
-
 /*
  * Nested HVM
  */
@@ -657,11 +663,6 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
     return false;
 }
 
-/* Check CR4/EFER values */
-const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
-                           signed int cr0_pg);
-unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore);
-
 /*
  * This must be defined as a macro instead of an inline function,
  * because it uses 'struct vcpu' and 'struct domain' which have
-- 
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] 57+ messages in thread

* [PATCH v3 09/16] x86: provide stubs, declarations and macros in hvm.h
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (7 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 08/16] x86/hvm: rearrange content of hvm.h Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-07  7:02   ` Jan Beulich
  2018-09-04 16:15 ` [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM Wei Liu
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Make sure hvm_enabled evaluate to false then provide necessary things
to make xen build when !CONFIG_HVM.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: rewritten
---
 xen/include/asm-x86/hvm/hvm.h | 97 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 0c321409ee..28fd483a04 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -340,6 +340,9 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
                            signed int cr0_pg);
 unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore);
 
+
+#ifdef CONFIG_HVM
+
 #define hvm_get_guest_tsc(v) hvm_get_guest_tsc_fixed(v, 0)
 
 #define hvm_tsc_scaling_supported \
@@ -675,6 +678,100 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
         d_->arch.hvm.pi_ops.vcpu_block(v_);                     \
 })
 
+#else  /* CONFIG_HVM */
+
+#define hvm_enabled false
+
+/*
+ * List of inline functions above, of which only declarations are
+ * needed because DCE will kick in.
+ */
+int hvm_guest_x86_mode(struct vcpu *v);
+unsigned long hvm_get_shadow_gs_base(struct vcpu *v);
+void hvm_set_info_guest(struct vcpu *v);
+void hvm_cpuid_policy_changed(struct vcpu *v);
+void hvm_set_tsc_offset(struct vcpu *v, uint64_t offset, uint64_t at_tsc);
+
+static inline bool hvm_is_singlestep_supported(void)
+{
+    return false;
+}
+
+static inline bool hvm_hap_supported(void)
+{
+    return false;
+}
+
+static inline bool nhvm_vmcx_hap_enabled(struct vcpu *v)
+{
+    ASSERT_UNREACHABLE();
+    return false;
+}
+
+static inline int hvm_cpu_up(void)
+{
+    return 0;
+}
+
+static inline void hvm_cpu_down(void) {}
+
+static inline void hvm_flush_guest_tlbs(void) {}
+
+static inline void hvm_update_host_cr3(struct vcpu *v)
+{
+    ASSERT_UNREACHABLE();
+}
+
+static inline void hvm_update_guest_cr3(struct vcpu *v, bool noflush)
+{
+    ASSERT_UNREACHABLE();
+}
+
+static inline unsigned int hvm_get_cpl(struct vcpu *v)
+{
+    ASSERT_UNREACHABLE();
+    return -1;
+}
+
+static inline int hvm_event_pending(struct vcpu *v)
+{
+    return 0;
+}
+
+static inline void hvm_inject_hw_exception(unsigned int vector, int errcode)
+{
+    ASSERT_UNREACHABLE();
+}
+
+static inline void hvm_invlpg(struct vcpu *v, unsigned long linear)
+{
+    ASSERT_UNREACHABLE();
+}
+
+#define is_viridian_domain(d) ({(void)(d); false;})
+#define has_viridian_time_ref_count(d) ({(void)(d); false;})
+#define hvm_long_mode_active(v) ({(void)(v); false;})
+#define hvm_pae_enabled(v) ({(void)(v); false;})
+#define hvm_get_guest_time(v) ({(void)(v); 0;})
+
+#define hvm_tsc_scaling_supported false
+#define hap_has_1gb false
+#define hap_has_2mb false
+
+
+#define hvm_paging_enabled(v) ({(void)(v); false;})
+#define hvm_wp_enabled(v) ({(void)(v); false;})
+#define hvm_pcid_enabled(v) ({(void)(v); false;})
+#define hvm_pae_enabled(v) ({(void)(v); false;})
+#define hvm_smep_enabled(v) ({(void)(v); false;})
+#define hvm_smap_enabled(v) ({(void)(v); false;})
+#define hvm_nx_enabled(v) ({(void)(v); false;})
+#define hvm_pku_enabled(v) ({(void)(v); false;})
+
+#define arch_vcpu_block(v) ((void)(v))
+
+#endif  /* CONFIG_HVM */
+
 #endif /* __ASM_X86_HVM_HVM_H__ */
 
 /*
-- 
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] 57+ messages in thread

* [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (8 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 09/16] x86: provide stubs, declarations and macros in hvm.h Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-06 16:20   ` George Dunlap
  2018-09-07  7:06   ` Jan Beulich
  2018-09-04 16:15 ` [PATCH v3 11/16] x86/mm: put HVM only " Wei Liu
                   ` (5 subsequent siblings)
  15 siblings, 2 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Wei Liu, Jan Beulich

These functions are only useful for nested hvm, which isn't enabled
when CONFIG_HVM is false.

Enclose relevant code and fields in CONFIG_HVM. Guard np2m_schedule
with nestedhvm_enabled.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/domain.c        |  6 ++++--
 xen/arch/x86/mm/p2m.c        | 18 ++++++++++++++----
 xen/include/asm-x86/domain.h |  2 ++
 xen/include/asm-x86/p2m.h    |  2 ++
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 313ebb3221..7c945a2428 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1691,7 +1691,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
     {
         _update_runstate_area(prev);
         vpmu_switch_from(prev);
-        np2m_schedule(NP2M_SCHEDLE_OUT);
+        if ( nestedhvm_enabled(prevd) )
+            np2m_schedule(NP2M_SCHEDLE_OUT);
     }
 
     if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
@@ -1758,7 +1759,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
 
         /* Must be done with interrupts enabled */
         vpmu_switch_to(next);
-        np2m_schedule(NP2M_SCHEDLE_IN);
+        if ( nestedhvm_enabled(nextd) )
+            np2m_schedule(NP2M_SCHEDLE_IN);
     }
 
     /* Ensure that the vcpu has an up-to-date time base. */
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 79d0e7203a..7a12cd37e8 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -58,22 +58,22 @@ static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
 #endif
 
     mm_rwlock_init(&p2m->lock);
-    INIT_LIST_HEAD(&p2m->np2m_list);
     INIT_PAGE_LIST_HEAD(&p2m->pages);
 
 #ifdef CONFIG_HVM
     mm_lock_init(&p2m->pod.lock);
     INIT_PAGE_LIST_HEAD(&p2m->pod.super);
     INIT_PAGE_LIST_HEAD(&p2m->pod.single);
+    INIT_LIST_HEAD(&p2m->np2m_list);
+
+    p2m->np2m_base = P2M_BASE_EADDR;
+    p2m->np2m_generation = 0;
 #endif
 
     p2m->domain = d;
     p2m->default_access = p2m_access_rwx;
     p2m->p2m_class = p2m_host;
 
-    p2m->np2m_base = P2M_BASE_EADDR;
-    p2m->np2m_generation = 0;
-
 #ifdef CONFIG_HVM
     for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
         p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
@@ -149,6 +149,7 @@ static void p2m_teardown_hostp2m(struct domain *d)
     }
 }
 
+#ifdef CONFIG_HVM
 static void p2m_teardown_nestedp2m(struct domain *d)
 {
     unsigned int i;
@@ -186,6 +187,7 @@ static int p2m_init_nestedp2m(struct domain *d)
 
     return 0;
 }
+#endif
 
 static void p2m_teardown_altp2m(struct domain *d)
 {
@@ -233,6 +235,7 @@ int p2m_init(struct domain *d)
     if ( rc )
         return rc;
 
+#ifdef CONFIG_HVM
     /* Must initialise nestedp2m unconditionally
      * since nestedhvm_enabled(d) returns false here.
      * (p2m_init runs too early for HVM_PARAM_* options) */
@@ -242,12 +245,15 @@ int p2m_init(struct domain *d)
         p2m_teardown_hostp2m(d);
         return rc;
     }
+#endif
 
     rc = p2m_init_altp2m(d);
     if ( rc )
     {
         p2m_teardown_hostp2m(d);
+#ifdef CONFIG_HVM
         p2m_teardown_nestedp2m(d);
+#endif
     }
 
     return rc;
@@ -699,7 +705,9 @@ void p2m_final_teardown(struct domain *d)
      * we initialise them unconditionally.
      */
     p2m_teardown_altp2m(d);
+#ifdef CONFIG_HVM
     p2m_teardown_nestedp2m(d);
+#endif
 
     /* Iterate over all p2m tables per domain */
     p2m_teardown_hostp2m(d);
@@ -1725,6 +1733,7 @@ void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
         p2m_switch_vcpu_altp2m_by_id(v, idx);
 }
 
+#ifdef CONFIG_HVM
 static struct p2m_domain *
 p2m_getlru_nestedp2m(struct domain *d, struct p2m_domain *p2m)
 {
@@ -1982,6 +1991,7 @@ void np2m_schedule(int dir)
         p2m_unlock(p2m);
     }
 }
+#endif
 
 unsigned long paging_gva_to_gfn(struct vcpu *v,
                                 unsigned long va,
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 4da4353de7..b46cfb0ce4 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -333,9 +333,11 @@ struct arch_domain
         void (*tail)(struct vcpu *);
     } *ctxt_switch;
 
+#ifdef CONFIG_HVM
     /* nestedhvm: translate l2 guest physical to host physical */
     struct p2m_domain *nested_p2m[MAX_NESTEDP2M];
     mm_lock_t nested_p2m_lock;
+#endif
 
     /* altp2m: allow multiple copies of host p2m */
     bool_t altp2m_active;
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 3785598f54..20cf3f1a25 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -204,6 +204,7 @@ struct p2m_domain {
 
     p2m_class_t       p2m_class; /* host/nested/alternate */
 
+#ifdef CONFIG_HVM
     /* Nested p2ms only: nested p2m base value that this p2m shadows.
      * This can be cleared to P2M_BASE_EADDR under the per-p2m lock but
      * needs both the per-p2m lock and the per-domain nestedp2m lock
@@ -216,6 +217,7 @@ struct p2m_domain {
      * The host p2m hasolds the head of the list and the np2ms are 
      * threaded on in LRU order. */
     struct list_head   np2m_list;
+#endif
 
     /* Host p2m: Log-dirty ranges registered for the domain. */
     struct rangeset   *logdirty_ranges;
-- 
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] 57+ messages in thread

* [PATCH v3 11/16] x86/mm: put HVM only code under CONFIG_HVM
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (9 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-04 17:10   ` Razvan Cojocaru
                     ` (2 more replies)
  2018-09-04 16:15 ` [PATCH v3 12/16] x86/mm: put paging_update_nestedmode " Wei Liu
                   ` (4 subsequent siblings)
  15 siblings, 3 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Tamas K Lengyel, Wei Liu, Razvan Cojocaru, George Dunlap,
	Andrew Cooper, Jan Beulich

Going through the code, HAP, EPT, PoD and ALTP2M depend on HVM code.
Put these components under CONFIG_HVM. This further requires putting
one of the vm event under CONFIG_HVM.

Altp2m requires a bit more attention because its code is embedded in
generic x86 p2m code.

Also make hap_enabled evaluate to false when !CONFIG_HVM. Make sure it
evaluate its parameter to avoid unused variable warnings in its users.

Also sort items in Makefile while at it.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/mm/Makefile         | 11 ++++++-----
 xen/arch/x86/mm/mem_access.c     | 18 +++++++++++++++++-
 xen/arch/x86/mm/mem_sharing.c    |  2 ++
 xen/arch/x86/mm/p2m.c            | 23 ++++++++++++-----------
 xen/common/vm_event.c            |  2 ++
 xen/include/asm-x86/altp2m.h     | 13 ++++++++++++-
 xen/include/asm-x86/domain.h     |  2 +-
 xen/include/asm-x86/hvm/domain.h |  4 ++++
 xen/include/asm-x86/p2m.h        |  7 ++++++-
 9 files changed, 62 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index 3017119813..9cbb2cfcde 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -1,15 +1,16 @@
 subdir-y += shadow
-subdir-y += hap
+subdir-$(CONFIG_HVM) += hap
 
-obj-y += paging.o
-obj-y += p2m.o p2m-pt.o p2m-ept.o p2m-pod.o
-obj-y += altp2m.o
+obj-$(CONFIG_HVM) += altp2m.o
 obj-y += guest_walk_2.o
 obj-y += guest_walk_3.o
 obj-y += guest_walk_4.o
+obj-y += mem_access.o
 obj-y += mem_paging.o
 obj-y += mem_sharing.o
-obj-y += mem_access.o
+obj-y += p2m.o p2m-pt.o
+obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o
+obj-y += paging.o
 
 guest_walk_%.o: guest_walk.c Makefile
 	$(CC) $(CFLAGS) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index a8b3e99ec4..84879b7faf 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -246,7 +246,6 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
     /* Return whether vCPU pause is required (aka. sync event) */
     return (p2ma != p2m_access_n2rwx);
 }
-#endif
 
 int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
                               struct p2m_domain *ap2m, p2m_access_t a,
@@ -288,6 +287,7 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
     return ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a,
                            current->domain != d);
 }
+#endif
 
 static int set_mem_access(struct domain *d, struct p2m_domain *p2m,
                           struct p2m_domain *ap2m, p2m_access_t a,
@@ -295,6 +295,7 @@ static int set_mem_access(struct domain *d, struct p2m_domain *p2m,
 {
     int rc = 0;
 
+#ifdef CONFIG_HVM
     if ( ap2m )
     {
         rc = p2m_set_altp2m_mem_access(d, p2m, ap2m, a, gfn);
@@ -303,6 +304,9 @@ static int set_mem_access(struct domain *d, struct p2m_domain *p2m,
             rc = 0;
     }
     else
+#else
+    ASSERT(!ap2m);
+#endif
     {
         mfn_t mfn;
         p2m_access_t _a;
@@ -364,6 +368,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
     long rc = 0;
 
     /* altp2m view 0 is treated as the hostp2m */
+#ifdef CONFIG_HVM
     if ( altp2m_idx )
     {
         if ( altp2m_idx >= MAX_ALTP2M ||
@@ -372,6 +377,9 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
 
         ap2m = d->arch.altp2m_p2m[altp2m_idx];
     }
+#else
+    ASSERT(!altp2m_idx);
+#endif
 
     if ( !xenmem_access_to_p2m_access(p2m, access, &a) )
         return -EINVAL;
@@ -419,6 +427,7 @@ long p2m_set_mem_access_multi(struct domain *d,
     long rc = 0;
 
     /* altp2m view 0 is treated as the hostp2m */
+#ifdef CONFIG_HVM
     if ( altp2m_idx )
     {
         if ( altp2m_idx >= MAX_ALTP2M ||
@@ -427,6 +436,9 @@ long p2m_set_mem_access_multi(struct domain *d,
 
         ap2m = d->arch.altp2m_p2m[altp2m_idx];
     }
+#else
+    ASSERT(!altp2m_idx);
+#endif
 
     p2m_lock(p2m);
     if ( ap2m )
@@ -480,12 +492,15 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access)
 
 void arch_p2m_set_access_required(struct domain *d, bool access_required)
 {
+#ifdef CONFIG_HVM
     unsigned int i;
+#endif
 
     ASSERT(atomic_read(&d->pause_count));
 
     p2m_get_hostp2m(d)->access_required = access_required;
 
+#ifdef CONFIG_HVM
     if ( !altp2m_active(d) )
         return;
 
@@ -496,6 +511,7 @@ void arch_p2m_set_access_required(struct domain *d, bool access_required)
         if ( p2m )
             p2m->access_required = access_required;
     }
+#endif
 }
 
 /*
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index d04f9c79b3..349e6fd2cf 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -802,6 +802,7 @@ static int nominate_page(struct domain *d, gfn_t gfn,
     if ( !p2m_is_sharable(p2mt) )
         goto out;
 
+#ifdef CONFIG_HVM
     /* Check if there are mem_access/remapped altp2m entries for this page */
     if ( altp2m_active(d) )
     {
@@ -829,6 +830,7 @@ static int nominate_page(struct domain *d, gfn_t gfn,
 
         altp2m_list_unlock(d);
     }
+#endif
 
     /* Try to convert the mfn to the sharable type */
     page = mfn_to_page(mfn);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 7a12cd37e8..027202f39e 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -187,7 +187,6 @@ static int p2m_init_nestedp2m(struct domain *d)
 
     return 0;
 }
-#endif
 
 static void p2m_teardown_altp2m(struct domain *d)
 {
@@ -226,6 +225,7 @@ static int p2m_init_altp2m(struct domain *d)
 
     return 0;
 }
+#endif
 
 int p2m_init(struct domain *d)
 {
@@ -245,16 +245,14 @@ int p2m_init(struct domain *d)
         p2m_teardown_hostp2m(d);
         return rc;
     }
-#endif
 
     rc = p2m_init_altp2m(d);
     if ( rc )
     {
         p2m_teardown_hostp2m(d);
-#ifdef CONFIG_HVM
         p2m_teardown_nestedp2m(d);
-#endif
     }
+#endif
 
     return rc;
 }
@@ -700,12 +698,12 @@ void p2m_teardown(struct p2m_domain *p2m)
 
 void p2m_final_teardown(struct domain *d)
 {
+#ifdef CONFIG_HVM
     /*
      * We must teardown both of them unconditionally because
      * we initialise them unconditionally.
      */
     p2m_teardown_altp2m(d);
-#ifdef CONFIG_HVM
     p2m_teardown_nestedp2m(d);
 #endif
 
@@ -1727,12 +1725,6 @@ void p2m_mem_paging_resume(struct domain *d, vm_event_response_t *rsp)
     }
 }
 
-void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
-{
-    if ( altp2m_active(v->domain) )
-        p2m_switch_vcpu_altp2m_by_id(v, idx);
-}
-
 #ifdef CONFIG_HVM
 static struct p2m_domain *
 p2m_getlru_nestedp2m(struct domain *d, struct p2m_domain *p2m)
@@ -2182,6 +2174,14 @@ int unmap_mmio_regions(struct domain *d,
     return i == nr ? 0 : i ?: ret;
 }
 
+#ifdef CONFIG_HVM
+
+void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
+{
+    if ( altp2m_active(v->domain) )
+        p2m_switch_vcpu_altp2m_by_id(v, idx);
+}
+
 bool_t p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
 {
     struct domain *d = v->domain;
@@ -2559,6 +2559,7 @@ int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
 
     return ret;
 }
+#endif /* CONFIG_HVM */
 
 /*** Audit ***/
 
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index a3bbfc9474..12293c1588 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -429,9 +429,11 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
              */
             vm_event_toggle_singlestep(d, v, &rsp);
 
+#ifdef CONFIG_HVM
             /* Check for altp2m switch */
             if ( rsp.flags & VM_EVENT_FLAG_ALTERNATE_P2M )
                 p2m_altp2m_check(v, rsp.altp2m_idx);
+#endif
 
             if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
                 vm_event_set_registers(v, &rsp);
diff --git a/xen/include/asm-x86/altp2m.h b/xen/include/asm-x86/altp2m.h
index 64c761873e..41fdd828a2 100644
--- a/xen/include/asm-x86/altp2m.h
+++ b/xen/include/asm-x86/altp2m.h
@@ -18,12 +18,14 @@
 #ifndef __ASM_X86_ALTP2M_H
 #define __ASM_X86_ALTP2M_H
 
+#ifdef CONFIG_HVM
+
 #include <xen/types.h>
 #include <xen/sched.h>         /* for struct vcpu, struct domain */
 #include <asm/hvm/vcpu.h>      /* for vcpu_altp2m */
 
 /* Alternate p2m HVM on/off per domain */
-static inline bool_t altp2m_active(const struct domain *d)
+static inline bool altp2m_active(const struct domain *d)
 {
     return d->arch.altp2m_active;
 }
@@ -37,5 +39,14 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
 {
     return vcpu_altp2m(v).p2midx;
 }
+#else
+
+static inline bool altp2m_active(const struct domain *d)
+{
+    return false;
+}
+
+uint16_t altp2m_vcpu_idx(const struct vcpu *v);
+#endif
 
 #endif /* __ASM_X86_ALTP2M_H */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index b46cfb0ce4..cb0721e9d5 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -337,13 +337,13 @@ struct arch_domain
     /* nestedhvm: translate l2 guest physical to host physical */
     struct p2m_domain *nested_p2m[MAX_NESTEDP2M];
     mm_lock_t nested_p2m_lock;
-#endif
 
     /* altp2m: allow multiple copies of host p2m */
     bool_t altp2m_active;
     struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
     mm_lock_t altp2m_list_lock;
     uint64_t *altp2m_eptp;
+#endif
 
     /* NB. protected by d->event_lock and by irq_desc[irq].lock */
     struct radix_tree_root irq_pirq;
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 7388cd895e..6b81f10074 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -195,7 +195,11 @@ struct hvm_domain {
     };
 };
 
+#ifdef CONFIG_HVM
 #define hap_enabled(d)  ((d)->arch.hvm.hap_enabled)
+#else
+#define hap_enabled(d)  ({(void)(d); false;})
+#endif
 
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 20cf3f1a25..5b5faf37f0 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -231,8 +231,10 @@ struct p2m_domain {
      * host p2m's lock. */
     int                defer_nested_flush;
 
+#ifdef CONFIG_HVM
     /* Alternate p2m: count of vcpu's currently using this p2m. */
     atomic_t           active_vcpus;
+#endif
 
     /* Pages used to construct the p2m */
     struct page_list_head pages;
@@ -823,7 +825,7 @@ void nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
 /*
  * Alternate p2m: shadow p2m tables used for alternate memory views
  */
-
+#ifdef CONFIG_HVM
 /* get current alternate p2m table */
 static inline struct p2m_domain *p2m_get_altp2m(struct vcpu *v)
 {
@@ -870,6 +872,9 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
 int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
                                 mfn_t mfn, unsigned int page_order,
                                 p2m_type_t p2mt, p2m_access_t p2ma);
+#else
+struct p2m_domain *p2m_get_altp2m(struct vcpu *v);
+#endif
 
 /*
  * p2m type to IOMMU 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] 57+ messages in thread

* [PATCH v3 12/16] x86/mm: put paging_update_nestedmode under CONFIG_HVM
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (10 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 11/16] x86/mm: put HVM only " Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-13 16:39   ` George Dunlap
  2018-09-04 16:15 ` [PATCH v3 13/16] xen: connect guest creation with CONFIG_{HVM, PV} Wei Liu
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Wei Liu, Jan Beulich

Nested HVM is not enabled when !CONFIG_HVM.

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

diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index dcee496eb0..7f460bd321 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -919,6 +919,7 @@ const struct paging_mode *paging_get_mode(struct vcpu *v)
     return paging_get_nestedmode(v);
 }
 
+#ifdef CONFIG_HVM
 void paging_update_nestedmode(struct vcpu *v)
 {
     ASSERT(nestedhvm_enabled(v->domain));
@@ -930,6 +931,7 @@ void paging_update_nestedmode(struct vcpu *v)
         v->arch.paging.nestedmode = NULL;
     hvm_asid_flush_vcpu(v);
 }
+#endif
 
 void paging_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
                             l1_pgentry_t *p, l1_pgentry_t new,
-- 
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] 57+ messages in thread

* [PATCH v3 13/16] xen: connect guest creation with CONFIG_{HVM, PV}
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (11 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 12/16] x86/mm: put paging_update_nestedmode " Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-04 16:15 ` [PATCH v3 14/16] x86: expose CONFIG_HVM Wei Liu
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/common/domain.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 78c450e4b4..1f95bd169c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -318,9 +318,24 @@ struct domain *domain_create(domid_t domid,
 
     /* Sort out our idea of is_{pv,hvm}_domain(). */
     if ( config && (config->flags & XEN_DOMCTL_CDF_hvm_guest) )
+    {
+#ifdef CONFIG_HVM
         d->guest_type = guest_type_hvm;
+#else
+        err = -EINVAL;
+        goto fail;
+#endif
+    }
     else
+    {
+#ifdef CONFIG_PV
         d->guest_type = guest_type_pv;
+#else
+        err = -EINVAL;
+        goto fail;
+#endif
+    }
+
 
     TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
 
-- 
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] 57+ messages in thread

* [PATCH v3 14/16] x86: expose CONFIG_HVM
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (12 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 13/16] xen: connect guest creation with CONFIG_{HVM, PV} Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-07  7:15   ` Jan Beulich
  2018-09-04 16:15 ` [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim Wei Liu
  2018-09-04 16:15 ` [PATCH v3 16/16] xen: decouple HVM and IOMMU capabilities Wei Liu
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: longer text
v2: use tab to indent

Haven't added a dependency on PV_SHIM_EXCLUSIVE because agreement is
not yet reached.
---
 xen/arch/x86/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index ae1b707c19..c160fd65f6 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -60,6 +60,17 @@ config PV_LINEAR_PT
 
 config HVM
 	def_bool y
+	prompt "HVM support"
+	---help---
+	  Interfaces to support HVM guests which require hardware
+	  support like Intel's VT-x or AMD's SVM. Note the hypervisor
+	  doesn't distinguish HVM or PVH guest types. PVH guest type
+	  is only a concept for end users.
+
+	  This option is needed if you want to run HVM or PVH guests.
+
+	  If unsure, say Y.
+
 
 config SHADOW_PAGING
         bool "Shadow Paging"
-- 
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] 57+ messages in thread

* [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (13 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 14/16] x86: expose CONFIG_HVM Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-07  7:18   ` Jan Beulich
  2018-09-04 16:15 ` [PATCH v3 16/16] xen: decouple HVM and IOMMU capabilities Wei Liu
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/firmware/xen-dir/shim.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/firmware/xen-dir/shim.config b/tools/firmware/xen-dir/shim.config
index 21d7075bb4..de53dfe376 100644
--- a/tools/firmware/xen-dir/shim.config
+++ b/tools/firmware/xen-dir/shim.config
@@ -12,7 +12,7 @@ CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
 CONFIG_NR_CPUS=32
 CONFIG_PV=y
 CONFIG_PV_LINEAR_PT=y
-CONFIG_HVM=y
+# CONFIG_HVM is not set
 # CONFIG_SHADOW_PAGING is not set
 # CONFIG_BIGMEM is not set
 # CONFIG_HVM_FEP is not set
-- 
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] 57+ messages in thread

* [PATCH v3 16/16] xen: decouple HVM and IOMMU capabilities
  2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
                   ` (14 preceding siblings ...)
  2018-09-04 16:15 ` [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim Wei Liu
@ 2018-09-04 16:15 ` Wei Liu
  2018-09-13 15:52   ` Ian Jackson
  15 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich

HVM and IOMMU are two distinct hardware features, yet they were
bundled together in sysctl and xl's output.

Decouple them on sysctl level. On toolstack level we still need to
maintain a sensible semantics for `xl info`. Massage the information
according to the following table:

pv      hvm     iommu           flags in xl info
0       0       0               n/a
0       0       1               n/a
0       1       0               hvm
0       1       1               hvm hvm_directio
1       0       0               NIL
1       0       1               directio
1       1       0               hvm
1       1       1               hvm hvm_directio directio

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 tools/libxl/libxl.c         | 5 +++--
 tools/libxl/libxl.h         | 6 ++++++
 tools/libxl/libxl_types.idl | 1 +
 tools/xl/xl_info.c          | 5 +++--
 xen/arch/x86/sysctl.c       | 2 +-
 xen/include/public/sysctl.h | 8 ++++----
 6 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b41ade9fda..a0d9f2bfe7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -396,8 +396,9 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
     memcpy(physinfo->hw_cap,xcphysinfo.hw_cap, sizeof(physinfo->hw_cap));
 
     physinfo->cap_hvm = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hvm);
-    physinfo->cap_hvm_directio =
-        !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hvm_directio);
+    physinfo->cap_directio =
+        !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_directio);
+    physinfo->cap_hvm_directio = physinfo->cap_hvm && physinfo->cap_directio;
 
     GC_FREE;
     return 0;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index ae2d63df0c..2cfc1b08ad 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -67,6 +67,12 @@
  * the same $(XEN_VERSION) (e.g. throughout a major release).
  */
 
+/* LIBXL_HAVE_PHYSINFO_CAP_DIRECTIO
+ *
+ * If this is defined, libxl_physinfo has a "cap_directio" field.
+ */
+#define LIBXL_HAVE_PHYSINFO_CAP_DIRECTIO 1
+
 /* LIBXL_HAVE_CONSOLE_NOTIFY_FD
  *
  * If this is defined, libxl_console_exec and
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 4a385801ba..2cceb8c057 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -1014,6 +1014,7 @@ libxl_physinfo = Struct("physinfo", [
 
     ("cap_hvm", bool),
     ("cap_hvm_directio", bool),
+    ("cap_directio", bool),
     ], dir=DIR_OUT)
 
 libxl_connectorinfo = Struct("connectorinfo", [
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 6c8be26119..93e2c5fa7d 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -210,9 +210,10 @@ static void output_physinfo(void)
          info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
         );
 
-    maybe_printf("virt_caps              :%s%s\n",
+    maybe_printf("virt_caps              :%s%s%s\n",
          info.cap_hvm ? " hvm" : "",
-         info.cap_hvm_directio ? " hvm_directio" : ""
+         info.cap_hvm_directio ? " hvm_directio" : "",
+         info.cap_directio ? " directio" : ""
         );
 
     vinfo = libxl_get_version_info(ctx);
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index e704ed7f1c..456dc58d8f 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -93,7 +93,7 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
     if ( hvm_enabled )
         pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
     if ( iommu_enabled )
-        pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm_directio;
+        pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
 }
 
 long arch_do_sysctl(
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 839c1b9f25..8cd0a9cb0d 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
 #include "physdev.h"
 #include "tmem.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x00000011
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000012
 
 /*
  * Read console content from Xen buffer ring.
@@ -85,9 +85,9 @@ struct xen_sysctl_tbuf_op {
  /* (x86) The platform supports HVM guests. */
 #define _XEN_SYSCTL_PHYSCAP_hvm          0
 #define XEN_SYSCTL_PHYSCAP_hvm           (1u<<_XEN_SYSCTL_PHYSCAP_hvm)
- /* (x86) The platform supports HVM-guest direct access to I/O devices. */
-#define _XEN_SYSCTL_PHYSCAP_hvm_directio 1
-#define XEN_SYSCTL_PHYSCAP_hvm_directio  (1u<<_XEN_SYSCTL_PHYSCAP_hvm_directio)
+ /* (x86) The platform supports direct access to I/O devices with IOMMU. */
+#define _XEN_SYSCTL_PHYSCAP_directio 1
+#define XEN_SYSCTL_PHYSCAP_directio  (1u<<_XEN_SYSCTL_PHYSCAP_directio)
 struct xen_sysctl_physinfo {
     uint32_t threads_per_core;
     uint32_t cores_per_socket;
-- 
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] 57+ messages in thread

* Re: [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only
  2018-09-04 16:15 ` [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only Wei Liu
@ 2018-09-04 16:24   ` Paul Durrant
  2018-09-04 16:42   ` Wei Liu
  1 sibling, 0 replies; 57+ messages in thread
From: Paul Durrant @ 2018-09-04 16:24 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 04 September 2018 17:15
> To: xen-devel@lists.xenproject.org
> Cc: Wei Liu <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>;
> Jan Beulich <jbeulich@suse.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>
> Subject: [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM
> only
> 
> Put the entire case branch under CONFIG_HVM.
> 
> Nonetheless check HVM before trying to get ioreq server.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> v3:
> 1. put an assert in hvm_get_ioreq_server_frame
> 2. remove redundant assignment of rc
> 
> v2: put entire case branch under CONFIG_HVM
> ---
>  xen/arch/x86/hvm/ioreq.c | 5 ++---
>  xen/arch/x86/mm.c        | 5 +++++
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 138ed697cd..fecca58a3d 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -961,12 +961,11 @@ int hvm_get_ioreq_server_frame(struct domain
> *d, ioservid_t id,
>      struct hvm_ioreq_server *s;
>      int rc;
> 
> +    ASSERT(is_hvm_domain(d));
> +
>      if ( id == DEFAULT_IOSERVID )
>          return -EOPNOTSUPP;
> 
> -    if ( !is_hvm_domain(d) )
> -        return -EINVAL;
> -
>      spin_lock_recursive(&d->arch.hvm.ioreq_server.lock);
> 
>      s = get_ioreq_server(d, id);
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 409814ce0a..baea2f5e63 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4381,12 +4381,16 @@ int arch_acquire_resource(struct domain *d,
> unsigned int type,
> 
>      switch ( type )
>      {
> +#ifdef CONFIG_HVM
>      case XENMEM_resource_ioreq_server:
>      {
>          ioservid_t ioservid = id;
>          unsigned int i;
> 
>          rc = -EINVAL;
> +        if ( !is_hvm_domain(d) )
> +            break;
> +
>          if ( id != (unsigned int)ioservid )
>              break;
> 
> @@ -4409,6 +4413,7 @@ int arch_acquire_resource(struct domain *d,
> unsigned int type,
>          *flags |= XENMEM_rsrc_acq_caller_owned;
>          break;
>      }
> +#endif
> 
>      default:
>          rc = -EOPNOTSUPP;
> --
> 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] 57+ messages in thread

* Re: [PATCH v3 04/16] x86: monitor.o is currently HVM only
  2018-09-04 16:15 ` [PATCH v3 04/16] x86: monitor.o is currently " Wei Liu
@ 2018-09-04 16:35   ` Razvan Cojocaru
  0 siblings, 0 replies; 57+ messages in thread
From: Razvan Cojocaru @ 2018-09-04 16:35 UTC (permalink / raw)
  To: Wei Liu, xen-devel; +Cc: Andrew Cooper, Tamas K Lengyel, Jan Beulich

On 9/4/18 7:15 PM, Wei Liu wrote:
> There has been plan to make PV work, but it is not yet there.  Provide
> stubs to make it build with !CONFIG_HVM.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>


Thanks,
Razvan

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

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

* Re: [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only
  2018-09-04 16:15 ` [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only Wei Liu
  2018-09-04 16:24   ` Paul Durrant
@ 2018-09-04 16:42   ` Wei Liu
  2018-09-06 13:29     ` Jan Beulich
  1 sibling, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-04 16:42 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Wei Liu, Jan Beulich

On Tue, Sep 04, 2018 at 05:15:20PM +0100, Wei Liu wrote:
> Put the entire case branch under CONFIG_HVM.
> 
> Nonetheless check HVM before trying to get ioreq server.

The wording here is confusing. It should have been written as:

Lift the check from hvm_get_ioreq_server_frame into its caller.

Wei.

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

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

* Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM Wei Liu
@ 2018-09-04 17:08   ` Razvan Cojocaru
  2018-09-04 17:10     ` Razvan Cojocaru
  2018-09-04 17:24   ` Julien Grall
  2018-09-06 15:05   ` Jan Beulich
  2 siblings, 1 reply; 57+ messages in thread
From: Razvan Cojocaru @ 2018-09-04 17:08 UTC (permalink / raw)
  To: Wei Liu, xen-devel
  Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Tamas K Lengyel, Jan Beulich

On 9/4/18 7:15 PM, Wei Liu wrote:
> Populate-on-demand is HVM only.
> 
> Provide a bunch of stubs for common p2m code and guard one invocation
> of guest_physmap_mark_populate_on_demand with is_hvm_domain.
> 
> Put relevant fields in p2m_domain and code which touches those fields
> under CONFIG_HVM.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>


Thanks,
Razvan

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

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

* Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-04 17:08   ` Razvan Cojocaru
@ 2018-09-04 17:10     ` Razvan Cojocaru
  0 siblings, 0 replies; 57+ messages in thread
From: Razvan Cojocaru @ 2018-09-04 17:10 UTC (permalink / raw)
  To: Wei Liu, xen-devel
  Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Tamas K Lengyel, Jan Beulich

On 9/4/18 8:08 PM, Razvan Cojocaru wrote:
> On 9/4/18 7:15 PM, Wei Liu wrote:
>> Populate-on-demand is HVM only.
>>
>> Provide a bunch of stubs for common p2m code and guard one invocation
>> of guest_physmap_mark_populate_on_demand with is_hvm_domain.
>>
>> Put relevant fields in p2m_domain and code which touches those fields
>> under CONFIG_HVM.
>>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

Sorry, obviously meant Acked-by.


Thanks,
Razvan

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

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

* Re: [PATCH v3 11/16] x86/mm: put HVM only code under CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 11/16] x86/mm: put HVM only " Wei Liu
@ 2018-09-04 17:10   ` Razvan Cojocaru
  2018-09-07  7:12   ` Jan Beulich
  2018-09-07 21:27   ` Tamas K Lengyel
  2 siblings, 0 replies; 57+ messages in thread
From: Razvan Cojocaru @ 2018-09-04 17:10 UTC (permalink / raw)
  To: Wei Liu, xen-devel
  Cc: George Dunlap, Andrew Cooper, Tamas K Lengyel, Jan Beulich

On 9/4/18 7:15 PM, Wei Liu wrote:
> Going through the code, HAP, EPT, PoD and ALTP2M depend on HVM code.
> Put these components under CONFIG_HVM. This further requires putting
> one of the vm event under CONFIG_HVM.
> 
> Altp2m requires a bit more attention because its code is embedded in
> generic x86 p2m code.
> 
> Also make hap_enabled evaluate to false when !CONFIG_HVM. Make sure it
> evaluate its parameter to avoid unused variable warnings in its users.
> 
> Also sort items in Makefile while at it.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>


Thanks,
Razvan

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

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

* Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM Wei Liu
  2018-09-04 17:08   ` Razvan Cojocaru
@ 2018-09-04 17:24   ` Julien Grall
  2018-09-06 10:57     ` Wei Liu
  2018-09-06 15:05   ` Jan Beulich
  2 siblings, 1 reply; 57+ messages in thread
From: Julien Grall @ 2018-09-04 17:24 UTC (permalink / raw)
  To: Wei Liu, xen-devel
  Cc: Stefano Stabellini, Razvan Cojocaru, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Tamas K Lengyel,
	Jan Beulich

Hi Wei,

On 09/04/2018 05:15 PM, Wei Liu wrote:
> Populate-on-demand is HVM only.
> 
> Provide a bunch of stubs for common p2m code and guard one invocation
> of guest_physmap_mark_populate_on_demand with is_hvm_domain.
> 
> Put relevant fields in p2m_domain and code which touches those fields
> under CONFIG_HVM.

Arm does not have any POD support. Would it be worth to introduce a 
CONFIG_HAS_POD to avoid dummy function on Arm?

Cheers,

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: Put pod related fields and code under CONFIG_HVM.
> 
> Note, this should be applied after the toolstack side change.
> ---
>   xen/arch/x86/mm.c         |  2 ++
>   xen/arch/x86/mm/p2m-pt.c  |  4 ++++
>   xen/arch/x86/mm/p2m.c     | 22 ++++++++++++++++++++--
>   xen/common/memory.c       |  3 ++-
>   xen/common/vm_event.c     |  4 ++++
>   xen/include/asm-x86/p2m.h | 38 ++++++++++++++++++++++++++++++++++----
>   6 files changed, 66 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index baea2f5e63..54023e0c69 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4588,6 +4588,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>           return 0;
>       }
>   
> +#ifdef CONFIG_HVM
>       case XENMEM_set_pod_target:
>       case XENMEM_get_pod_target:
>       {
> @@ -4644,6 +4645,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>           rcu_unlock_domain(d);
>           return rc;
>       }
> +#endif
>   
>       default:
>           return subarch_memory_op(cmd, arg);
> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
> index b8c5d2ed26..74884ea063 100644
> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -974,7 +974,9 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>       unsigned long mfn, gfn, m2pfn;
>   
>       ASSERT(p2m_locked_by_me(p2m));
> +#ifdef CONFIG_HVM
>       ASSERT(pod_locked_by_me(p2m));
> +#endif
>   
>       /* Audit part one: walk the domain's p2m table, checking the entries. */
>       if ( pagetable_get_pfn(p2m_get_pagetable(p2m)) != 0 )
> @@ -1105,6 +1107,7 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>           unmap_domain_page(l4e);
>       }
>   
> +#ifdef CONFIG_HVM
>       if ( entry_count != p2m->pod.entry_count )
>       {
>           printk("%s: refcounted entry count %ld, audit count %lu!\n",
> @@ -1113,6 +1116,7 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>                  entry_count);
>           BUG();
>       }
> +#endif
>   
>       return pmbad;
>   }
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 6020553c17..79d0e7203a 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>   /* Init the datastructures for later use by the p2m code */
>   static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>   {
> -    unsigned int i;
>       int ret = 0;
> +#ifdef CONFIG_HVM
> +    unsigned int i;
> +#endif
>   
>       mm_rwlock_init(&p2m->lock);
> -    mm_lock_init(&p2m->pod.lock);
>       INIT_LIST_HEAD(&p2m->np2m_list);
>       INIT_PAGE_LIST_HEAD(&p2m->pages);
> +
> +#ifdef CONFIG_HVM
> +    mm_lock_init(&p2m->pod.lock);
>       INIT_PAGE_LIST_HEAD(&p2m->pod.super);
>       INIT_PAGE_LIST_HEAD(&p2m->pod.single);
> +#endif
>   
>       p2m->domain = d;
>       p2m->default_access = p2m_access_rwx;
> @@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>       p2m->np2m_base = P2M_BASE_EADDR;
>       p2m->np2m_generation = 0;
>   
> +#ifdef CONFIG_HVM
>       for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
>           p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
> +#endif
>   
>       if ( hap_enabled(d) && cpu_has_vmx )
>           ret = ept_p2m_init(p2m);
> @@ -917,6 +924,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
>                    gfn_x(gfn), mfn_x(mfn));
>           rc = p2m_set_entry(p2m, gfn, INVALID_MFN, page_order,
>                              p2m_invalid, p2m->default_access);
> +#ifdef CONFIG_HVM
>           if ( rc == 0 )
>           {
>               pod_lock(p2m);
> @@ -924,6 +932,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
>               BUG_ON(p2m->pod.entry_count < 0);
>               pod_unlock(p2m);
>           }
> +#endif
>       }
>   
>   out:
> @@ -1114,6 +1123,7 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn_l,
>       if ( rc )
>           gdprintk(XENLOG_ERR, "p2m_set_entry: %#lx:%u -> %d (0x%"PRI_mfn")\n",
>                    gfn_l, order, rc, mfn_x(mfn));
> +#ifdef CONFIG_HVM
>       else if ( p2m_is_pod(ot) )
>       {
>           pod_lock(p2m);
> @@ -1121,6 +1131,7 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn_l,
>           BUG_ON(p2m->pod.entry_count < 0);
>           pod_unlock(p2m);
>       }
> +#endif
>       gfn_unlock(p2m, gfn, order);
>   
>       return rc;
> @@ -1743,9 +1754,11 @@ p2m_flush_table_locked(struct p2m_domain *p2m)
>        * when discarding them.
>        */
>       ASSERT(!p2m_is_hostp2m(p2m));
> +#ifdef CONFIG_HVM
>       /* Nested p2m's do not do pod, hence the asserts (and no pod lock)*/
>       ASSERT(page_list_empty(&p2m->pod.super));
>       ASSERT(page_list_empty(&p2m->pod.single));
> +#endif
>   
>       /* No need to flush if it's already empty */
>       if ( p2m_is_nestedp2m(p2m) && p2m->np2m_base == P2M_BASE_EADDR )
> @@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
>       P2M_PRINTK("p2m audit starts\n");
>   
>       p2m_lock(p2m);
> +
> +#ifdef CONFIG_HVM
>       pod_lock(p2m);
> +#endif
>   
>       if (p2m->audit_p2m)
>           pmbad = p2m->audit_p2m(p2m);
> @@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
>       }
>       spin_unlock(&d->page_alloc_lock);
>   
> +#ifdef CONFIG_HVM
>       pod_unlock(p2m);
> +#endif
>       p2m_unlock(p2m);
>    
>       P2M_PRINTK("p2m audit complete\n");
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index 996f94b103..5c71ce13ce 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -210,7 +210,8 @@ static void populate_physmap(struct memop_args *a)
>               if ( d == curr_d )
>                   goto out;
>   
> -            if ( guest_physmap_mark_populate_on_demand(d, gpfn,
> +            if ( is_hvm_domain(d) &&
> +                 guest_physmap_mark_populate_on_demand(d, gpfn,
>                                                          a->extent_order) < 0 )
>                   goto out;
>           }
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 4793aacc35..a3bbfc9474 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
>           {
>           case XEN_VM_EVENT_ENABLE:
>           {
> +#ifdef CONFIG_HVM
>               struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +#endif
>   
>               rc = -EOPNOTSUPP;
>               /* hvm fixme: p2m_is_foreign types need addressing */
> @@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
>               if ( unlikely(need_iommu(d)) )
>                   break;
>   
> +#ifdef CONFIG_HVM
>               rc = -EXDEV;
>               /* Disallow paging in a PoD guest */
>               if ( p2m->pod.entry_count )
>                   break;
> +#endif
>   
>               /* domain_pause() not required here, see XSA-99 */
>               rc = vm_event_enable(d, vec, &d->vm_event_paging, _VPF_mem_paging,
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index d4b3cfcb6e..3785598f54 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -306,6 +306,7 @@ struct p2m_domain {
>        * to resume the search */
>       unsigned long next_shared_gfn_to_relinquish;
>   
> +#ifdef CONFIG_HVM
>       /* Populate-on-demand variables
>        * All variables are protected with the pod lock. We cannot rely on
>        * the p2m lock if it's turned into a fine-grained lock.
> @@ -337,6 +338,8 @@ struct p2m_domain {
>           mm_lock_t        lock;         /* Locking of private pod structs,   *
>                                           * not relying on the p2m lock.      */
>       } pod;
> +#endif
> +
>       union {
>           struct ept_data ept;
>           /* NPT-equivalent structure could be added here. */
> @@ -646,6 +649,12 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>   /* Dump PoD information about the domain */
>   void p2m_pod_dump_data(struct domain *d);
>   
> +#ifdef CONFIG_HVM
> +
> +/* Called by p2m code when demand-populating a PoD page */
> +bool
> +p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn, unsigned int order);
> +
>   /* Move all pages from the populate-on-demand cache to the domain page_list
>    * (usually in preparation for domain destruction) */
>   int p2m_pod_empty_cache(struct domain *d);
> @@ -662,6 +671,31 @@ p2m_pod_offline_or_broken_hit(struct page_info *p);
>   void
>   p2m_pod_offline_or_broken_replace(struct page_info *p);
>   
> +#else
> +
> +static inline bool
> +p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn, unsigned int order)
> +{
> +    return false;
> +}
> +
> +static inline int p2m_pod_empty_cache(struct domain *d)
> +{
> +    return 0;
> +}
> +
> +static inline int p2m_pod_offline_or_broken_hit(struct page_info *p)
> +{
> +    return 0;
> +}
> +
> +static inline void p2m_pod_offline_or_broken_replace(struct page_info *p)
> +{
> +    ASSERT_UNREACHABLE();
> +}
> +
> +#endif
> +
>   
>   /*
>    * Paging to disk and page-sharing
> @@ -730,10 +764,6 @@ extern void audit_p2m(struct domain *d,
>   #define P2M_DEBUG(f, a...) do { (void)(f); } while(0)
>   #endif
>   
> -/* Called by p2m code when demand-populating a PoD page */
> -bool
> -p2m_pod_demand_populate(struct p2m_domain *p2m, gfn_t gfn, unsigned int order);
> -
>   /*
>    * Functions specific to the p2m-pt implementation
>    */
> 

-- 
Julien Grall

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

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

* Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-04 17:24   ` Julien Grall
@ 2018-09-06 10:57     ` Wei Liu
  2018-09-06 15:30       ` George Dunlap
  0 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-06 10:57 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Razvan Cojocaru, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Tamas K Lengyel,
	Jan Beulich, xen-devel

On Tue, Sep 04, 2018 at 06:24:28PM +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 09/04/2018 05:15 PM, Wei Liu wrote:
> > Populate-on-demand is HVM only.
> > 
> > Provide a bunch of stubs for common p2m code and guard one invocation
> > of guest_physmap_mark_populate_on_demand with is_hvm_domain.
> > 
> > Put relevant fields in p2m_domain and code which touches those fields
> > under CONFIG_HVM.
> 
> Arm does not have any POD support. Would it be worth to introduce a
> CONFIG_HAS_POD to avoid dummy function on Arm?

That sounds fine. This will have the effect of not compiling PoD on Arm
at all, which is good.

I will wait for more reviews before reworking this patch.

Wei.

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

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

* Re: [PATCH v3 01/16] x86: change name of parameter for various invlpg functions
  2018-09-04 16:15 ` [PATCH v3 01/16] x86: change name of parameter for various invlpg functions Wei Liu
@ 2018-09-06 11:12   ` George Dunlap
  2018-09-13 16:11   ` George Dunlap
  1 sibling, 0 replies; 57+ messages in thread
From: George Dunlap @ 2018-09-06 11:12 UTC (permalink / raw)
  To: Wei Liu, xen-devel
  Cc: Kevin Tian, Jan Beulich, George Dunlap, Andrew Cooper,
	Tim Deegan, Jun Nakajima, Boris Ostrovsky, Brian Woods,
	Suravee Suthikulpanit

On 09/04/2018 05:15 PM, Wei Liu wrote:
> They all incorrectly named a parameter virtual address while it should
> have been linear address.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Acked-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Acked-by: George Dunlap <george.dunlap@citrix.com>

> ---
>  xen/arch/x86/hvm/svm/svm.c         | 14 +++++++-------
>  xen/arch/x86/hvm/vmx/vmx.c         | 12 ++++++------
>  xen/arch/x86/mm.c                  | 10 +++++-----
>  xen/arch/x86/mm/hap/hap.c          |  2 +-
>  xen/arch/x86/mm/shadow/multi.c     | 14 +++++++-------
>  xen/arch/x86/mm/shadow/none.c      |  2 +-
>  xen/include/asm-x86/hvm/hvm.h      |  6 +++---
>  xen/include/asm-x86/hvm/svm/asid.h |  4 ++--
>  xen/include/asm-x86/hvm/svm/svm.h  |  4 ++--
>  xen/include/asm-x86/paging.h       |  3 ++-
>  10 files changed, 36 insertions(+), 35 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 0b06e2ff11..34d55b4938 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2488,18 +2488,18 @@ static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs)
>  }
>  
>  static void svm_invlpga_intercept(
> -    struct vcpu *v, unsigned long vaddr, uint32_t asid)
> +    struct vcpu *v, unsigned long linear, uint32_t asid)
>  {
> -    svm_invlpga(vaddr,
> +    svm_invlpga(linear,
>                  (asid == 0)
>                  ? v->arch.hvm.n1asid.asid
>                  : vcpu_nestedhvm(v).nv_n2asid.asid);
>  }
>  
> -static void svm_invlpg_intercept(unsigned long vaddr)
> +static void svm_invlpg_intercept(unsigned long linear)
>  {
> -    HVMTRACE_LONG_2D(INVLPG, 0, TRC_PAR_LONG(vaddr));
> -    paging_invlpg(current, vaddr);
> +    HVMTRACE_LONG_2D(INVLPG, 0, TRC_PAR_LONG(linear));
> +    paging_invlpg(current, linear);
>  }
>  
>  static bool is_invlpg(const struct x86_emulate_state *state,
> @@ -2512,9 +2512,9 @@ static bool is_invlpg(const struct x86_emulate_state *state,
>             (ext & 7) == 7;
>  }
>  
> -static void svm_invlpg(struct vcpu *v, unsigned long vaddr)
> +static void svm_invlpg(struct vcpu *v, unsigned long linear)
>  {
> -    svm_asid_g_invlpg(v, vaddr);
> +    svm_asid_g_invlpg(v, linear);
>  }
>  
>  static bool svm_get_pending_event(struct vcpu *v, struct x86_event *info)
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index e926b0b28e..b2e1a28038 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -75,7 +75,7 @@ static void vmx_wbinvd_intercept(void);
>  static void vmx_fpu_dirty_intercept(void);
>  static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content);
>  static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content);
> -static void vmx_invlpg(struct vcpu *v, unsigned long vaddr);
> +static void vmx_invlpg(struct vcpu *v, unsigned long linear);
>  
>  /* Values for domain's ->arch.hvm_domain.pi_ops.flags. */
>  #define PI_CSW_FROM (1u << 0)
> @@ -2595,16 +2595,16 @@ static void vmx_dr_access(unsigned long exit_qualification,
>      vmx_update_cpu_exec_control(v);
>  }
>  
> -static void vmx_invlpg_intercept(unsigned long vaddr)
> +static void vmx_invlpg_intercept(unsigned long linear)
>  {
> -    HVMTRACE_LONG_2D(INVLPG, /*invlpga=*/ 0, TRC_PAR_LONG(vaddr));
> -    paging_invlpg(current, vaddr);
> +    HVMTRACE_LONG_2D(INVLPG, /*invlpga=*/ 0, TRC_PAR_LONG(linear));
> +    paging_invlpg(current, linear);
>  }
>  
> -static void vmx_invlpg(struct vcpu *v, unsigned long vaddr)
> +static void vmx_invlpg(struct vcpu *v, unsigned long linear)
>  {
>      if ( cpu_has_vmx_vpid )
> -        vpid_sync_vcpu_gva(v, vaddr);
> +        vpid_sync_vcpu_gva(v, linear);
>  }
>  
>  static int vmx_vmfunc_intercept(struct cpu_user_regs *regs)
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 84979f28d5..409814ce0a 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5793,19 +5793,19 @@ const unsigned long *__init get_platform_badpages(unsigned int *array_size)
>      return bad_pages;
>  }
>  
> -void paging_invlpg(struct vcpu *v, unsigned long va)
> +void paging_invlpg(struct vcpu *v, unsigned long linear)
>  {
> -    if ( !is_canonical_address(va) )
> +    if ( !is_canonical_address(linear) )
>          return;
>  
>      if ( paging_mode_enabled(v->domain) &&
> -         !paging_get_hostmode(v)->invlpg(v, va) )
> +         !paging_get_hostmode(v)->invlpg(v, linear) )
>          return;
>  
>      if ( is_pv_vcpu(v) )
> -        flush_tlb_one_local(va);
> +        flush_tlb_one_local(linear);
>      else
> -        hvm_invlpg(v, va);
> +        hvm_invlpg(v, linear);
>  }
>  
>  /* Build a 32bit PSE page table using 4MB pages. */
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index c53d76cf69..3d651b94c3 100644
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -650,7 +650,7 @@ static int hap_page_fault(struct vcpu *v, unsigned long va,
>   * should not be intercepting it.  However, we need to correctly handle
>   * getting here from instruction emulation.
>   */
> -static bool_t hap_invlpg(struct vcpu *v, unsigned long va)
> +static bool_t hap_invlpg(struct vcpu *v, unsigned long linear)
>  {
>      /*
>       * Emulate INVLPGA:
> diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
> index 7bb6f47155..bba573ae87 100644
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -3549,7 +3549,7 @@ propagate:
>   * instruction should be issued on the hardware, or false if it's safe not
>   * to do so.
>   */
> -static bool sh_invlpg(struct vcpu *v, unsigned long va)
> +static bool sh_invlpg(struct vcpu *v, unsigned long linear)
>  {
>      mfn_t sl1mfn;
>      shadow_l2e_t sl2e;
> @@ -3572,14 +3572,14 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
>      {
>          shadow_l3e_t sl3e;
>          if ( !(shadow_l4e_get_flags(
> -                   sh_linear_l4_table(v)[shadow_l4_linear_offset(va)])
> +                   sh_linear_l4_table(v)[shadow_l4_linear_offset(linear)])
>                 & _PAGE_PRESENT) )
>              return false;
>          /* This must still be a copy-from-user because we don't have the
>           * paging lock, and the higher-level shadows might disappear
>           * under our feet. */
>          if ( __copy_from_user(&sl3e, (sh_linear_l3_table(v)
> -                                      + shadow_l3_linear_offset(va)),
> +                                      + shadow_l3_linear_offset(linear)),
>                                sizeof (sl3e)) != 0 )
>          {
>              perfc_incr(shadow_invlpg_fault);
> @@ -3589,7 +3589,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
>              return false;
>      }
>  #else /* SHADOW_PAGING_LEVELS == 3 */
> -    if ( !(l3e_get_flags(v->arch.paging.shadow.l3table[shadow_l3_linear_offset(va)])
> +    if ( !(l3e_get_flags(v->arch.paging.shadow.l3table[shadow_l3_linear_offset(linear)])
>             & _PAGE_PRESENT) )
>          // no need to flush anything if there's no SL2...
>          return false;
> @@ -3598,7 +3598,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
>      /* This must still be a copy-from-user because we don't have the shadow
>       * lock, and the higher-level shadows might disappear under our feet. */
>      if ( __copy_from_user(&sl2e,
> -                          sh_linear_l2_table(v) + shadow_l2_linear_offset(va),
> +                          sh_linear_l2_table(v) + shadow_l2_linear_offset(linear),
>                            sizeof (sl2e)) != 0 )
>      {
>          perfc_incr(shadow_invlpg_fault);
> @@ -3642,7 +3642,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
>               * feet. */
>              if ( __copy_from_user(&sl2e,
>                                    sh_linear_l2_table(v)
> -                                  + shadow_l2_linear_offset(va),
> +                                  + shadow_l2_linear_offset(linear),
>                                    sizeof (sl2e)) != 0 )
>              {
>                  perfc_incr(shadow_invlpg_fault);
> @@ -3664,7 +3664,7 @@ static bool sh_invlpg(struct vcpu *v, unsigned long va)
>                          && page_is_out_of_sync(pg) ) )
>              {
>                  shadow_l1e_t *sl1;
> -                sl1 = sh_linear_l1_table(v) + shadow_l1_linear_offset(va);
> +                sl1 = sh_linear_l1_table(v) + shadow_l1_linear_offset(linear);
>                  /* Remove the shadow entry that maps this VA */
>                  (void) shadow_set_l1e(d, sl1, shadow_l1e_empty(),
>                                        p2m_invalid, sl1mfn);
> diff --git a/xen/arch/x86/mm/shadow/none.c b/xen/arch/x86/mm/shadow/none.c
> index a8c9604cdf..4de645a433 100644
> --- a/xen/arch/x86/mm/shadow/none.c
> +++ b/xen/arch/x86/mm/shadow/none.c
> @@ -37,7 +37,7 @@ static int _page_fault(struct vcpu *v, unsigned long va,
>      return 0;
>  }
>  
> -static bool _invlpg(struct vcpu *v, unsigned long va)
> +static bool _invlpg(struct vcpu *v, unsigned long linear)
>  {
>      ASSERT_UNREACHABLE();
>      return true;
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 132e62b4f6..6b0e088750 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -160,7 +160,7 @@ struct hvm_function_table {
>  
>      int  (*event_pending)(struct vcpu *v);
>      bool (*get_pending_event)(struct vcpu *v, struct x86_event *info);
> -    void (*invlpg)(struct vcpu *v, unsigned long vaddr);
> +    void (*invlpg)(struct vcpu *v, unsigned long linear);
>  
>      int  (*cpu_up_prepare)(unsigned int cpu);
>      void (*cpu_dead)(unsigned int cpu);
> @@ -454,9 +454,9 @@ static inline int hvm_event_pending(struct vcpu *v)
>      return hvm_funcs.event_pending(v);
>  }
>  
> -static inline void hvm_invlpg(struct vcpu *v, unsigned long va)
> +static inline void hvm_invlpg(struct vcpu *v, unsigned long linear)
>  {
> -    hvm_funcs.invlpg(v, va);
> +    hvm_funcs.invlpg(v, linear);
>  }
>  
>  /* These bits in CR4 are owned by the host. */
> diff --git a/xen/include/asm-x86/hvm/svm/asid.h b/xen/include/asm-x86/hvm/svm/asid.h
> index 60cbb7b881..0e5ec3ab78 100644
> --- a/xen/include/asm-x86/hvm/svm/asid.h
> +++ b/xen/include/asm-x86/hvm/svm/asid.h
> @@ -25,11 +25,11 @@
>  void svm_asid_init(const struct cpuinfo_x86 *c);
>  void svm_asid_handle_vmrun(void);
>  
> -static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_vaddr)
> +static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_linear)
>  {
>  #if 0
>      /* Optimization? */
> -    svm_invlpga(g_vaddr, v->arch.hvm.svm.vmcb->guest_asid);
> +    svm_invlpga(g_linear, v->arch.hvm.svm.vmcb->guest_asid);
>  #endif
>  
>      /* Safe fallback. Take a new ASID. */
> diff --git a/xen/include/asm-x86/hvm/svm/svm.h b/xen/include/asm-x86/hvm/svm/svm.h
> index 4e5e142910..8166046a6d 100644
> --- a/xen/include/asm-x86/hvm/svm/svm.h
> +++ b/xen/include/asm-x86/hvm/svm/svm.h
> @@ -40,13 +40,13 @@ static inline void svm_vmsave_pa(paddr_t vmcb)
>          : : "a" (vmcb) : "memory" );
>  }
>  
> -static inline void svm_invlpga(unsigned long vaddr, uint32_t asid)
> +static inline void svm_invlpga(unsigned long linear, uint32_t asid)
>  {
>      asm volatile (
>          ".byte 0x0f,0x01,0xdf"
>          : /* output */
>          : /* input */
> -        "a" (vaddr), "c" (asid));
> +        "a" (linear), "c" (asid));
>  }
>  
>  unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
> diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
> index f440e3e53c..b51e1709d3 100644
> --- a/xen/include/asm-x86/paging.h
> +++ b/xen/include/asm-x86/paging.h
> @@ -110,7 +110,8 @@ struct shadow_paging_mode {
>  struct paging_mode {
>      int           (*page_fault            )(struct vcpu *v, unsigned long va,
>                                              struct cpu_user_regs *regs);
> -    bool          (*invlpg                )(struct vcpu *v, unsigned long va);
> +    bool          (*invlpg                )(struct vcpu *v,
> +                                            unsigned long linear);
>      unsigned long (*gva_to_gfn            )(struct vcpu *v,
>                                              struct p2m_domain *p2m,
>                                              unsigned long va,
> 


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

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

* Re: [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags
  2018-09-04 16:15 ` [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags Wei Liu
@ 2018-09-06 13:27   ` Jan Beulich
  2018-09-06 13:47     ` Wei Liu
  0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2018-09-06 13:27 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> Use these flags in has_* tests and emulation_flags_ok.
> 
> Not using raw flags directly enabling DCE to kick in for has_* tests
> while at the same time making sure emulation_flags_ok won't go out of
> sync.

This is rather hard to read (for me at least). Perhaps s/enabling/enables/
and split sentences after the first line, or at least put a comma
somewhere?

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



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

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

* Re: [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only
  2018-09-04 16:42   ` Wei Liu
@ 2018-09-06 13:29     ` Jan Beulich
  0 siblings, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2018-09-06 13:29 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Paul Durrant, xen-devel

>>> On 04.09.18 at 18:42, <wei.liu2@citrix.com> wrote:
> On Tue, Sep 04, 2018 at 05:15:20PM +0100, Wei Liu wrote:
>> Put the entire case branch under CONFIG_HVM.
>> 
>> Nonetheless check HVM before trying to get ioreq server.
> 
> The wording here is confusing. It should have been written as:
> 
> Lift the check from hvm_get_ioreq_server_frame into its caller.

And then
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan



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

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

* Re: [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags
  2018-09-06 13:27   ` Jan Beulich
@ 2018-09-06 13:47     ` Wei Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-06 13:47 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel

On Thu, Sep 06, 2018 at 07:27:47AM -0600, Jan Beulich wrote:
> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> > Use these flags in has_* tests and emulation_flags_ok.
> > 
> > Not using raw flags directly enabling DCE to kick in for has_* tests
> > while at the same time making sure emulation_flags_ok won't go out of
> > sync.
> 
> This is rather hard to read (for me at least). Perhaps s/enabling/enables/
> and split sentences after the first line, or at least put a comma
> somewhere?

There were some grammar errors. The commit message has been updated to:

 Not using raw flags directly enables DCE to kick in for has_* tests,
 while at the same time makes sure emulation_flags_ok won't go out of
 sync.

> 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

Wei.

> 
> 

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

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

* Re: [PATCH v3 05/16] x86: PIT emulation is common to both PV and HVM
  2018-09-04 16:15 ` [PATCH v3 05/16] x86: PIT emulation is common to both PV and HVM Wei Liu
@ 2018-09-06 14:26   ` Jan Beulich
  0 siblings, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2018-09-06 14:26 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> Move the file to x86 common code and change its name to emul-i8254.c.
> 
> Put HVM only code under CONFIG_HVM or is_hvm_domain.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

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

* Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM Wei Liu
  2018-09-04 17:08   ` Razvan Cojocaru
  2018-09-04 17:24   ` Julien Grall
@ 2018-09-06 15:05   ` Jan Beulich
  2018-09-06 16:06     ` George Dunlap
  2 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2018-09-06 15:05 UTC (permalink / raw)
  To: Wei Liu
  Cc: Stefano Stabellini, Razvan Cojocaru, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
	Tamas K Lengyel, xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>  /* Init the datastructures for later use by the p2m code */
>  static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>  {
> -    unsigned int i;
>      int ret = 0;
> +#ifdef CONFIG_HVM
> +    unsigned int i;
> +#endif
>  
>      mm_rwlock_init(&p2m->lock);
> -    mm_lock_init(&p2m->pod.lock);
>      INIT_LIST_HEAD(&p2m->np2m_list);
>      INIT_PAGE_LIST_HEAD(&p2m->pages);
> +
> +#ifdef CONFIG_HVM
> +    mm_lock_init(&p2m->pod.lock);
>      INIT_PAGE_LIST_HEAD(&p2m->pod.super);
>      INIT_PAGE_LIST_HEAD(&p2m->pod.single);
> +#endif
>  
>      p2m->domain = d;
>      p2m->default_access = p2m_access_rwx;
> @@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>      p2m->np2m_base = P2M_BASE_EADDR;
>      p2m->np2m_generation = 0;
>  
> +#ifdef CONFIG_HVM
>      for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
>          p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
> +#endif

I wonder whether all of this wouldn't better go into p2m_pod_init()
(or alike), to limit/avoid the #ifdef-ary here.

> @@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
>      P2M_PRINTK("p2m audit starts\n");
>  
>      p2m_lock(p2m);
> +
> +#ifdef CONFIG_HVM
>      pod_lock(p2m);
> +#endif
>  
>      if (p2m->audit_p2m)
>          pmbad = p2m->audit_p2m(p2m);
> @@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
>      }
>      spin_unlock(&d->page_alloc_lock);
>  
> +#ifdef CONFIG_HVM
>      pod_unlock(p2m);
> +#endif

Perhaps better make them inline stubs? Otoh - isn't the whole
function useful for HVM only?

> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct 
> xen_domctl_vm_event_op *vec,
>          {
>          case XEN_VM_EVENT_ENABLE:
>          {
> +#ifdef CONFIG_HVM
>              struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +#endif
>  
>              rc = -EOPNOTSUPP;
>              /* hvm fixme: p2m_is_foreign types need addressing */
> @@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
>              if ( unlikely(need_iommu(d)) )
>                  break;
>  
> +#ifdef CONFIG_HVM
>              rc = -EXDEV;
>              /* Disallow paging in a PoD guest */
>              if ( p2m->pod.entry_count )
>                  break;
> +#endif

Perhaps simply ditch the local variable?

Jan



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

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

* Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-06 10:57     ` Wei Liu
@ 2018-09-06 15:30       ` George Dunlap
  0 siblings, 0 replies; 57+ messages in thread
From: George Dunlap @ 2018-09-06 15:30 UTC (permalink / raw)
  To: Wei Liu, Julien Grall
  Cc: Stefano Stabellini, Razvan Cojocaru, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Tamas K Lengyel,
	Jan Beulich, xen-devel

On 09/06/2018 11:57 AM, Wei Liu wrote:
> On Tue, Sep 04, 2018 at 06:24:28PM +0100, Julien Grall wrote:
>> Hi Wei,
>>
>> On 09/04/2018 05:15 PM, Wei Liu wrote:
>>> Populate-on-demand is HVM only.
>>>
>>> Provide a bunch of stubs for common p2m code and guard one invocation
>>> of guest_physmap_mark_populate_on_demand with is_hvm_domain.
>>>
>>> Put relevant fields in p2m_domain and code which touches those fields
>>> under CONFIG_HVM.
>>
>> Arm does not have any POD support. Would it be worth to introduce a
>> CONFIG_HAS_POD to avoid dummy function on Arm?
> 
> That sounds fine. This will have the effect of not compiling PoD on Arm
> at all, which is good.

Another advantage of CONFIG_HAS_POD is that it would make it easier at
some later point to compile out, if we decided that was worth doing -- I
was going to suggest it just so as not to lose the "information" already
contained in this patch as where all the PoD-related code was.

 -George

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

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

* Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
  2018-09-06 15:05   ` Jan Beulich
@ 2018-09-06 16:06     ` George Dunlap
  0 siblings, 0 replies; 57+ messages in thread
From: George Dunlap @ 2018-09-06 16:06 UTC (permalink / raw)
  To: Jan Beulich, Wei Liu
  Cc: Stefano Stabellini, Razvan Cojocaru, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
	Tamas K Lengyel, xen-devel

On 09/06/2018 04:05 PM, Jan Beulich wrote:
>>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>>  /* Init the datastructures for later use by the p2m code */
>>  static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>>  {
>> -    unsigned int i;
>>      int ret = 0;
>> +#ifdef CONFIG_HVM
>> +    unsigned int i;
>> +#endif
>>  
>>      mm_rwlock_init(&p2m->lock);
>> -    mm_lock_init(&p2m->pod.lock);
>>      INIT_LIST_HEAD(&p2m->np2m_list);
>>      INIT_PAGE_LIST_HEAD(&p2m->pages);
>> +
>> +#ifdef CONFIG_HVM
>> +    mm_lock_init(&p2m->pod.lock);
>>      INIT_PAGE_LIST_HEAD(&p2m->pod.super);
>>      INIT_PAGE_LIST_HEAD(&p2m->pod.single);
>> +#endif
>>  
>>      p2m->domain = d;
>>      p2m->default_access = p2m_access_rwx;
>> @@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>>      p2m->np2m_base = P2M_BASE_EADDR;
>>      p2m->np2m_generation = 0;
>>  
>> +#ifdef CONFIG_HVM
>>      for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
>>          p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
>> +#endif
> 
> I wonder whether all of this wouldn't better go into p2m_pod_init()
> (or alike), to limit/avoid the #ifdef-ary here.

The cover letter for this series did say the initial goal was just to
make it build; and cleaning up could be done later.  Adding
p2m_pod_init() would be better long term, I think, but I'm also happy
with leaving if #ifdef's for now and cleaning them up later.

> 
>> @@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
>>      P2M_PRINTK("p2m audit starts\n");
>>  
>>      p2m_lock(p2m);
>> +
>> +#ifdef CONFIG_HVM
>>      pod_lock(p2m);
>> +#endif
>>  
>>      if (p2m->audit_p2m)
>>          pmbad = p2m->audit_p2m(p2m);
>> @@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
>>      }
>>      spin_unlock(&d->page_alloc_lock);
>>  
>> +#ifdef CONFIG_HVM
>>      pod_unlock(p2m);
>> +#endif
> 
> Perhaps better make them inline stubs? Otoh - isn't the whole
> function useful for HVM only?

Oh yes -- now that we've gotten rid of full translate guests for PV, I
guess so.

> 
>> --- a/xen/common/vm_event.c
>> +++ b/xen/common/vm_event.c
>> @@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct 
>> xen_domctl_vm_event_op *vec,
>>          {
>>          case XEN_VM_EVENT_ENABLE:
>>          {
>> +#ifdef CONFIG_HVM
>>              struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> +#endif
>>  
>>              rc = -EOPNOTSUPP;
>>              /* hvm fixme: p2m_is_foreign types need addressing */
>> @@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
>>              if ( unlikely(need_iommu(d)) )
>>                  break;
>>  
>> +#ifdef CONFIG_HVM
>>              rc = -EXDEV;
>>              /* Disallow paging in a PoD guest */
>>              if ( p2m->pod.entry_count )
>>                  break;
>> +#endif
> 
> Perhaps simply ditch the local variable?

That seems like a good idea.

Everything else looks good to me.

 -George

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

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

* Re: [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM Wei Liu
@ 2018-09-06 16:20   ` George Dunlap
  2018-09-13 15:46     ` Wei Liu
  2018-09-07  7:06   ` Jan Beulich
  1 sibling, 1 reply; 57+ messages in thread
From: George Dunlap @ 2018-09-06 16:20 UTC (permalink / raw)
  To: Wei Liu, xen-devel; +Cc: George Dunlap, Andrew Cooper, Jan Beulich

On 09/04/2018 05:15 PM, Wei Liu wrote:
> These functions are only useful for nested hvm, which isn't enabled
> when CONFIG_HVM is false.
> 
> Enclose relevant code and fields in CONFIG_HVM. Guard np2m_schedule
> with nestedhvm_enabled.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  xen/arch/x86/domain.c        |  6 ++++--
>  xen/arch/x86/mm/p2m.c        | 18 ++++++++++++++----
>  xen/include/asm-x86/domain.h |  2 ++
>  xen/include/asm-x86/p2m.h    |  2 ++
>  4 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 313ebb3221..7c945a2428 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1691,7 +1691,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>      {
>          _update_runstate_area(prev);
>          vpmu_switch_from(prev);
> -        np2m_schedule(NP2M_SCHEDLE_OUT);
> +        if ( nestedhvm_enabled(prevd) )
> +            np2m_schedule(NP2M_SCHEDLE_OUT);
>      }
>  
>      if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
> @@ -1758,7 +1759,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>  
>          /* Must be done with interrupts enabled */
>          vpmu_switch_to(next);
> -        np2m_schedule(NP2M_SCHEDLE_IN);
> +        if ( nestedhvm_enabled(nextd) )
> +            np2m_schedule(NP2M_SCHEDLE_IN);

There's already a nestedhvm_enabled() check first thing in
np2m_schedule().  How does adding this check help the CONFIG_HVM cause?

And why not #ifdef out this call, as well as the np2m_schedule()
functions entirely?

Everything else looks good.

 -George

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

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

* Re: [PATCH v3 08/16] x86/hvm: rearrange content of hvm.h
  2018-09-04 16:15 ` [PATCH v3 08/16] x86/hvm: rearrange content of hvm.h Wei Liu
@ 2018-09-07  6:52   ` Jan Beulich
  0 siblings, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2018-09-07  6:52 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> Move enum and function declarations to first half of the file.
> 
> Static inline functions and macros, which reference HVM specific
> fields directly are grouped together in second half of the file.
> 
> The movement is needed because in a later patch the second half is
> going to be enclosed in CONFIG_HVM.
> 
> Pure code movement. No functional change.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

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

* Re: [PATCH v3 09/16] x86: provide stubs, declarations and macros in hvm.h
  2018-09-04 16:15 ` [PATCH v3 09/16] x86: provide stubs, declarations and macros in hvm.h Wei Liu
@ 2018-09-07  7:02   ` Jan Beulich
  2018-09-13 15:31     ` Wei Liu
  0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2018-09-07  7:02 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> @@ -675,6 +678,100 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
>          d_->arch.hvm.pi_ops.vcpu_block(v_);                     \
>  })
>  
> +#else  /* CONFIG_HVM */
> +
> +#define hvm_enabled false
> +
> +/*
> + * List of inline functions above, of which only declarations are
> + * needed because DCE will kick in.
> + */

With this comment I think ...

> +int hvm_guest_x86_mode(struct vcpu *v);
> +unsigned long hvm_get_shadow_gs_base(struct vcpu *v);
> +void hvm_set_info_guest(struct vcpu *v);
> +void hvm_cpuid_policy_changed(struct vcpu *v);
> +void hvm_set_tsc_offset(struct vcpu *v, uint64_t offset, uint64_t at_tsc);
> +
> +static inline bool hvm_is_singlestep_supported(void)

... there should be another comment above here to sort of
terminate that first comment's effect.

> +static inline int hvm_cpu_up(void)
> +{
> +    return 0;
> +}
> +
> +static inline void hvm_cpu_down(void) {}
> +
> +static inline void hvm_flush_guest_tlbs(void) {}
> +
> +static inline void hvm_update_host_cr3(struct vcpu *v)
> +{
> +    ASSERT_UNREACHABLE();
> +}

Here and below - why ASSERT_UNREACHABLE() instead of the declaration
only approach above? (If it really needs to be this way, I think it would
help if the patch description said why.)

> +static inline int hvm_event_pending(struct vcpu *v)
> +{
> +    return 0;
> +}

Would there be an issue if you made this return bool and take pointer
to const right away, even without touching the "full" function? Perhaps
the const part would apply to other stubs here as well.

> +#define is_viridian_domain(d) ({(void)(d); false;})
> +#define has_viridian_time_ref_count(d) ({(void)(d); false;})
> +#define hvm_long_mode_active(v) ({(void)(v); false;})
> +#define hvm_pae_enabled(v) ({(void)(v); false;})
> +#define hvm_get_guest_time(v) ({(void)(v); 0;})

Perhaps simply without the need to use a gcc extension

#define is_viridian_domain(d) ((void)(d), false)

etc? Otherwise please add blanks inside the figure braces.

> +#define hvm_paging_enabled(v) ({(void)(v); false;})
> +#define hvm_wp_enabled(v) ({(void)(v); false;})
> +#define hvm_pcid_enabled(v) ({(void)(v); false;})
> +#define hvm_pae_enabled(v) ({(void)(v); false;})
> +#define hvm_smep_enabled(v) ({(void)(v); false;})
> +#define hvm_smap_enabled(v) ({(void)(v); false;})
> +#define hvm_nx_enabled(v) ({(void)(v); false;})
> +#define hvm_pku_enabled(v) ({(void)(v); false;})

Same here.

Jan



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

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

* Re: [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM Wei Liu
  2018-09-06 16:20   ` George Dunlap
@ 2018-09-07  7:06   ` Jan Beulich
  2018-09-13 15:07     ` Wei Liu
  1 sibling, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2018-09-07  7:06 UTC (permalink / raw)
  To: Wei Liu, George Dunlap; +Cc: Andrew Cooper, xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> @@ -149,6 +149,7 @@ static void p2m_teardown_hostp2m(struct domain *d)
>      }
>  }
>  
> +#ifdef CONFIG_HVM
>  static void p2m_teardown_nestedp2m(struct domain *d)
>  {
>      unsigned int i;
> @@ -186,6 +187,7 @@ static int p2m_init_nestedp2m(struct domain *d)
>  
>      return 0;
>  }
> +#endif

With the goal of limited code churn I think these would better be put
around the entire body of the function. That way the ones below
enclosing the function calls can go away.

> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -333,9 +333,11 @@ struct arch_domain
>          void (*tail)(struct vcpu *);
>      } *ctxt_switch;
>  
> +#ifdef CONFIG_HVM
>      /* nestedhvm: translate l2 guest physical to host physical */
>      struct p2m_domain *nested_p2m[MAX_NESTEDP2M];
>      mm_lock_t nested_p2m_lock;
> +#endif

Not something to be done here, just a general remark / question (perhaps
also more to George than you): Why do we have part of things here ...

> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -204,6 +204,7 @@ struct p2m_domain {
>  
>      p2m_class_t       p2m_class; /* host/nested/alternate */
>  
> +#ifdef CONFIG_HVM
>      /* Nested p2ms only: nested p2m base value that this p2m shadows.
>       * This can be cleared to P2M_BASE_EADDR under the per-p2m lock but
>       * needs both the per-p2m lock and the per-domain nestedp2m lock
> @@ -216,6 +217,7 @@ struct p2m_domain {
>       * The host p2m hasolds the head of the list and the np2ms are 
>       * threaded on in LRU order. */
>      struct list_head   np2m_list;
> +#endif

... and another part here?

Jan



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

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

* Re: [PATCH v3 11/16] x86/mm: put HVM only code under CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 11/16] x86/mm: put HVM only " Wei Liu
  2018-09-04 17:10   ` Razvan Cojocaru
@ 2018-09-07  7:12   ` Jan Beulich
  2018-09-07 21:27   ` Tamas K Lengyel
  2 siblings, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2018-09-07  7:12 UTC (permalink / raw)
  To: Wei Liu
  Cc: George Dunlap, Andrew Cooper, Tamas K Lengyel, Razvan Cojocaru,
	xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/mm/Makefile
> +++ b/xen/arch/x86/mm/Makefile
> @@ -1,15 +1,16 @@
>  subdir-y += shadow
> -subdir-y += hap
> +subdir-$(CONFIG_HVM) += hap
>  
> -obj-y += paging.o
> -obj-y += p2m.o p2m-pt.o p2m-ept.o p2m-pod.o
> -obj-y += altp2m.o
> +obj-$(CONFIG_HVM) += altp2m.o
>  obj-y += guest_walk_2.o
>  obj-y += guest_walk_3.o
>  obj-y += guest_walk_4.o
> +obj-y += mem_access.o

Despite it currently being always-on on x86, perhaps use
obj-$(CONFIG_MEM_ACCESS) as you touch it anyway?

With or without this
Acked-by: Jan Beulich <jbeulich@suse.com>
for the tiny bit of this patch where it's applicable.

Jan



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

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

* Re: [PATCH v3 14/16] x86: expose CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 14/16] x86: expose CONFIG_HVM Wei Liu
@ 2018-09-07  7:15   ` Jan Beulich
  2018-09-13 16:01     ` Wei Liu
  0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2018-09-07  7:15 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: longer text
> v2: use tab to indent
> 
> Haven't added a dependency on PV_SHIM_EXCLUSIVE because agreement is
> not yet reached.

Hmm, but then I would have expected you to at least do the minimal
agreed upon change (modifying that other option's default). Beyond
that I'm afraid we're moving towards a dead end here.

Jan



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

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

* Re: [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim
  2018-09-04 16:15 ` [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim Wei Liu
@ 2018-09-07  7:18   ` Jan Beulich
  2018-09-07  7:46     ` Wei Liu
  0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2018-09-07  7:18 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, Ian Jackson

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I think this should be merged into the previous patch, no matter what
the final resolution there is, as even with the change of default the
adjustment here should happen. Otoh Roger's "pvshim: introduce a PV
shim defconfig" should eliminate the need for this change altogether.
I notice (only now) that his patch appears to simply be waiting for an
x86 maintainer ack, so I guess I'll just go ahead and commit it.

Jan


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

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

* Re: [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim
  2018-09-07  7:18   ` Jan Beulich
@ 2018-09-07  7:46     ` Wei Liu
  2018-09-07  7:48       ` Wei Liu
  0 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-07  7:46 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Ian Jackson, Wei Liu, xen-devel

On Fri, Sep 07, 2018 at 01:18:52AM -0600, Jan Beulich wrote:
> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> I think this should be merged into the previous patch, no matter what
> the final resolution there is, as even with the change of default the
> adjustment here should happen. Otoh Roger's "pvshim: introduce a PV
> shim defconfig" should eliminate the need for this change altogether.

I think I will still need to patch that new file at some point because
it has CONFIG_PV=y, but that can be done easily.  I will fold that into
the previous patch in next iteration.

> I notice (only now) that his patch appears to simply be waiting for an
> x86 maintainer ack, so I guess I'll just go ahead and commit it.

Yeah, please commit that patch.  I will rebase my code.

Wei.

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

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

* Re: [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim
  2018-09-07  7:46     ` Wei Liu
@ 2018-09-07  7:48       ` Wei Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-07  7:48 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Ian Jackson, Wei Liu, xen-devel

On Fri, Sep 07, 2018 at 08:46:34AM +0100, Wei Liu wrote:
> On Fri, Sep 07, 2018 at 01:18:52AM -0600, Jan Beulich wrote:
> > >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> > 
> > I think this should be merged into the previous patch, no matter what
> > the final resolution there is, as even with the change of default the
> > adjustment here should happen. Otoh Roger's "pvshim: introduce a PV
> > shim defconfig" should eliminate the need for this change altogether.
> 
> I think I will still need to patch that new file at some point because
> it has CONFIG_PV=y, but that can be done easily.  I will fold that into
> the previous patch in next iteration.

My bad. Ignore this. Not enough caffeine in the morning.

This patch can be dropped.

Wei.

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

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

* Re: [PATCH v3 06/16] libxl: don't set PoD target for PV guests
  2018-09-04 16:15 ` [PATCH v3 06/16] libxl: don't set PoD target for PV guests Wei Liu
@ 2018-09-07 13:44   ` Ian Jackson
  0 siblings, 0 replies; 57+ messages in thread
From: Ian Jackson @ 2018-09-07 13:44 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel

Wei Liu writes ("[PATCH v3 06/16] libxl: don't set PoD target for PV guests"):
> Previously PoD target was unconditionally set for both PV and HVM
> guests, but in fact PoD has always been an HVM (now PVH as well) only
> feature.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

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

* Re: [PATCH v3 11/16] x86/mm: put HVM only code under CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 11/16] x86/mm: put HVM only " Wei Liu
  2018-09-04 17:10   ` Razvan Cojocaru
  2018-09-07  7:12   ` Jan Beulich
@ 2018-09-07 21:27   ` Tamas K Lengyel
  2018-09-13 15:43     ` Wei Liu
  2 siblings, 1 reply; 57+ messages in thread
From: Tamas K Lengyel @ 2018-09-07 21:27 UTC (permalink / raw)
  To: Wei Liu
  Cc: George Dunlap, Xen-devel, Razvan Cojocaru, Jan Beulich, Andrew Cooper


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

On Tue, Sep 4, 2018, 10:29 AM Wei Liu <wei.liu2@citrix.com> wrote:

> Going through the code, HAP, EPT, PoD and ALTP2M depend on HVM code.
> Put these components under CONFIG_HVM. This further requires putting
> one of the vm event under CONFIG_HVM.
>
> Altp2m requires a bit more attention because its code is embedded in
> generic x86 p2m code.
>
> Also make hap_enabled evaluate to false when !CONFIG_HVM. Make sure it
> evaluate its parameter to avoid unused variable warnings in its users.
>
> Also sort items in Makefile while at it.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  xen/arch/x86/mm/Makefile         | 11 ++++++-----
>  xen/arch/x86/mm/mem_access.c     | 18 +++++++++++++++++-
>  xen/arch/x86/mm/mem_sharing.c    |  2 ++
>  xen/arch/x86/mm/p2m.c            | 23 ++++++++++++-----------
>  xen/common/vm_event.c            |  2 ++
>

As before, please only apply CONFIG_HVM ifdefs to x86 specific parts. We
have an altp2m implementation for ARM that is planned to be posted again to
the mailinglist in the near future.

Thanks,
Tamas

[-- Attachment #1.2: Type: text/html, Size: 1602 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] 57+ messages in thread

* Re: [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
  2018-09-07  7:06   ` Jan Beulich
@ 2018-09-13 15:07     ` Wei Liu
  2018-09-14  8:01       ` Jan Beulich
  0 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-13 15:07 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, Andrew Cooper, Wei Liu, xen-devel

On Fri, Sep 07, 2018 at 01:06:38AM -0600, Jan Beulich wrote:
> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> > @@ -149,6 +149,7 @@ static void p2m_teardown_hostp2m(struct domain *d)
> >      }
> >  }
> >  
> > +#ifdef CONFIG_HVM
> >  static void p2m_teardown_nestedp2m(struct domain *d)
> >  {
> >      unsigned int i;
> > @@ -186,6 +187,7 @@ static int p2m_init_nestedp2m(struct domain *d)
> >  
> >      return 0;
> >  }
> > +#endif
> 
> With the goal of limited code churn I think these would better be put
> around the entire body of the function. That way the ones below
> enclosing the function calls can go away.

Later the ifdefs here and some other places will be expand to cover
altp2m as well. If we enclose only the body here, we will need to do the
same things for altp2m functions.

The end result is we will actually have more or less the
same amount whether we change to that method or not. But with this
method we will have less code churn when moving those functions out to
another file.

> 
> > --- a/xen/include/asm-x86/domain.h
> > +++ b/xen/include/asm-x86/domain.h
> > @@ -333,9 +333,11 @@ struct arch_domain
> >          void (*tail)(struct vcpu *);
> >      } *ctxt_switch;
> >  
> > +#ifdef CONFIG_HVM
> >      /* nestedhvm: translate l2 guest physical to host physical */
> >      struct p2m_domain *nested_p2m[MAX_NESTEDP2M];
> >      mm_lock_t nested_p2m_lock;
> > +#endif
> 
> Not something to be done here, just a general remark / question (perhaps
> also more to George than you): Why do we have part of things here ...
> 
> > --- a/xen/include/asm-x86/p2m.h
> > +++ b/xen/include/asm-x86/p2m.h
> > @@ -204,6 +204,7 @@ struct p2m_domain {
> >  
> >      p2m_class_t       p2m_class; /* host/nested/alternate */
> >  
> > +#ifdef CONFIG_HVM
> >      /* Nested p2ms only: nested p2m base value that this p2m shadows.
> >       * This can be cleared to P2M_BASE_EADDR under the per-p2m lock but
> >       * needs both the per-p2m lock and the per-domain nestedp2m lock
> > @@ -216,6 +217,7 @@ struct p2m_domain {
> >       * The host p2m hasolds the head of the list and the np2ms are 
> >       * threaded on in LRU order. */
> >      struct list_head   np2m_list;
> > +#endif
> 
> ... and another part here?

I also found them a bit strange to be placed in two different
structures. I will leave this question to George.

Wei.

> 
> Jan
> 
> 

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

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

* Re: [PATCH v3 09/16] x86: provide stubs, declarations and macros in hvm.h
  2018-09-07  7:02   ` Jan Beulich
@ 2018-09-13 15:31     ` Wei Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-13 15:31 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel

On Fri, Sep 07, 2018 at 01:02:02AM -0600, Jan Beulich wrote:
> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> > @@ -675,6 +678,100 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v)
> >          d_->arch.hvm.pi_ops.vcpu_block(v_);                     \
> >  })
> >  
> > +#else  /* CONFIG_HVM */
> > +
> > +#define hvm_enabled false
> > +
> > +/*
> > + * List of inline functions above, of which only declarations are
> > + * needed because DCE will kick in.
> > + */
> 
> With this comment I think ...
> 
> > +int hvm_guest_x86_mode(struct vcpu *v);
> > +unsigned long hvm_get_shadow_gs_base(struct vcpu *v);
> > +void hvm_set_info_guest(struct vcpu *v);
> > +void hvm_cpuid_policy_changed(struct vcpu *v);
> > +void hvm_set_tsc_offset(struct vcpu *v, uint64_t offset, uint64_t at_tsc);
> > +
> > +static inline bool hvm_is_singlestep_supported(void)
> 
> ... there should be another comment above here to sort of
> terminate that first comment's effect.

OK.

> 
> > +static inline int hvm_cpu_up(void)
> > +{
> > +    return 0;
> > +}
> > +
> > +static inline void hvm_cpu_down(void) {}
> > +
> > +static inline void hvm_flush_guest_tlbs(void) {}
> > +
> > +static inline void hvm_update_host_cr3(struct vcpu *v)
> > +{
> > +    ASSERT_UNREACHABLE();
> > +}
> 
> Here and below - why ASSERT_UNREACHABLE() instead of the declaration
> only approach above? (If it really needs to be this way, I think it would
> help if the patch description said why.)
> 

Shadow code has some code paths which are HVM only but haven't been
cleaned up. I will add a comment to that effect.


> > +static inline int hvm_event_pending(struct vcpu *v)
> > +{
> > +    return 0;
> > +}
> 
> Would there be an issue if you made this return bool and take pointer
> to const right away, even without touching the "full" function? Perhaps
> the const part would apply to other stubs here as well.

I wanted to keep make them have the same prototype. I'm happy to make
the changes here.

> 
> > +#define is_viridian_domain(d) ({(void)(d); false;})
> > +#define has_viridian_time_ref_count(d) ({(void)(d); false;})
> > +#define hvm_long_mode_active(v) ({(void)(v); false;})
> > +#define hvm_pae_enabled(v) ({(void)(v); false;})
> > +#define hvm_get_guest_time(v) ({(void)(v); 0;})
> 
> Perhaps simply without the need to use a gcc extension
> 
> #define is_viridian_domain(d) ((void)(d), false)

Done (for this and others).

Wei.

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

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

* Re: [PATCH v3 11/16] x86/mm: put HVM only code under CONFIG_HVM
  2018-09-07 21:27   ` Tamas K Lengyel
@ 2018-09-13 15:43     ` Wei Liu
  2018-09-13 16:31       ` Tamas K Lengyel
  0 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-13 15:43 UTC (permalink / raw)
  To: Tamas K Lengyel
  Cc: Wei Liu, Razvan Cojocaru, George Dunlap, Andrew Cooper,
	Jan Beulich, Xen-devel

On Fri, Sep 07, 2018 at 03:27:37PM -0600, Tamas K Lengyel wrote:
> On Tue, Sep 4, 2018, 10:29 AM Wei Liu <wei.liu2@citrix.com> wrote:
> 
> > Going through the code, HAP, EPT, PoD and ALTP2M depend on HVM code.
> > Put these components under CONFIG_HVM. This further requires putting
> > one of the vm event under CONFIG_HVM.
> >
> > Altp2m requires a bit more attention because its code is embedded in
> > generic x86 p2m code.
> >
> > Also make hap_enabled evaluate to false when !CONFIG_HVM. Make sure it
> > evaluate its parameter to avoid unused variable warnings in its users.
> >
> > Also sort items in Makefile while at it.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  xen/arch/x86/mm/Makefile         | 11 ++++++-----
> >  xen/arch/x86/mm/mem_access.c     | 18 +++++++++++++++++-
> >  xen/arch/x86/mm/mem_sharing.c    |  2 ++
> >  xen/arch/x86/mm/p2m.c            | 23 ++++++++++++-----------
> >  xen/common/vm_event.c            |  2 ++
> >
> 
> As before, please only apply CONFIG_HVM ifdefs to x86 specific parts. We
> have an altp2m implementation for ARM that is planned to be posted again to
> the mailinglist in the near future.

I take that you are specifically talking about common/vm_event.c because
that's the only common code here in the list of file.

I have switched to provide a stub for p2m_altp2m_check in x86 header
instead.

Wei.

> 
> Thanks,
> Tamas

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

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

* Re: [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
  2018-09-06 16:20   ` George Dunlap
@ 2018-09-13 15:46     ` Wei Liu
  2018-09-13 16:01       ` George Dunlap
  0 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-13 15:46 UTC (permalink / raw)
  To: George Dunlap
  Cc: George Dunlap, xen-devel, Wei Liu, Jan Beulich, Andrew Cooper

On Thu, Sep 06, 2018 at 05:20:53PM +0100, George Dunlap wrote:
> On 09/04/2018 05:15 PM, Wei Liu wrote:
> > These functions are only useful for nested hvm, which isn't enabled
> > when CONFIG_HVM is false.
> > 
> > Enclose relevant code and fields in CONFIG_HVM. Guard np2m_schedule
> > with nestedhvm_enabled.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  xen/arch/x86/domain.c        |  6 ++++--
> >  xen/arch/x86/mm/p2m.c        | 18 ++++++++++++++----
> >  xen/include/asm-x86/domain.h |  2 ++
> >  xen/include/asm-x86/p2m.h    |  2 ++
> >  4 files changed, 22 insertions(+), 6 deletions(-)
> > 
> > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> > index 313ebb3221..7c945a2428 100644
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -1691,7 +1691,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
> >      {
> >          _update_runstate_area(prev);
> >          vpmu_switch_from(prev);
> > -        np2m_schedule(NP2M_SCHEDLE_OUT);
> > +        if ( nestedhvm_enabled(prevd) )
> > +            np2m_schedule(NP2M_SCHEDLE_OUT);
> >      }
> >  
> >      if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
> > @@ -1758,7 +1759,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
> >  
> >          /* Must be done with interrupts enabled */
> >          vpmu_switch_to(next);
> > -        np2m_schedule(NP2M_SCHEDLE_IN);
> > +        if ( nestedhvm_enabled(nextd) )
> > +            np2m_schedule(NP2M_SCHEDLE_IN);
> 
> There's already a nestedhvm_enabled() check first thing in
> np2m_schedule().  How does adding this check help the CONFIG_HVM cause?

np2m_schedule will be gone entirely when !HVM.  Add nestedhvm_enabled
check here, which always evaluates to false when !HVM, makes compiler
able to eliminate the call to np2m_schedule when linking.

> 
> And why not #ifdef out this call, as well as the np2m_schedule()
> functions entirely?

The end result is the same. I'm happy to use either.

Wei.

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

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

* Re: [PATCH v3 16/16] xen: decouple HVM and IOMMU capabilities
  2018-09-04 16:15 ` [PATCH v3 16/16] xen: decouple HVM and IOMMU capabilities Wei Liu
@ 2018-09-13 15:52   ` Ian Jackson
  0 siblings, 0 replies; 57+ messages in thread
From: Ian Jackson @ 2018-09-13 15:52 UTC (permalink / raw)
  To: Wei Liu
  Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
	Andrew Cooper, Tim Deegan, Julien Grall, Jan Beulich, xen-devel

Wei Liu writes ("[PATCH v3 16/16] xen: decouple HVM and IOMMU capabilities"):
> HVM and IOMMU are two distinct hardware features, yet they were
> bundled together in sysctl and xl's output.
> 
> Decouple them on sysctl level. On toolstack level we still need to
> maintain a sensible semantics for `xl info`. Massage the information
> according to the following table:
> 
> pv      hvm     iommu           flags in xl info
> 0       0       0               n/a
> 0       0       1               n/a
> 0       1       0               hvm
> 0       1       1               hvm hvm_directio
> 1       0       0               NIL
> 1       0       1               directio
> 1       1       0               hvm
> 1       1       1               hvm hvm_directio directio

LGTM.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

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

* Re: [PATCH v3 14/16] x86: expose CONFIG_HVM
  2018-09-07  7:15   ` Jan Beulich
@ 2018-09-13 16:01     ` Wei Liu
  2018-09-14  8:07       ` Jan Beulich
  0 siblings, 1 reply; 57+ messages in thread
From: Wei Liu @ 2018-09-13 16:01 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel

On Fri, Sep 07, 2018 at 01:15:05AM -0600, Jan Beulich wrote:
> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > v3: longer text
> > v2: use tab to indent
> > 
> > Haven't added a dependency on PV_SHIM_EXCLUSIVE because agreement is
> > not yet reached.
> 
> Hmm, but then I would have expected you to at least do the minimal
> agreed upon change (modifying that other option's default). Beyond
> that I'm afraid we're moving towards a dead end here.
> 

I extended the help text to address Andrew's comment. Other than that, I
didn't see an agreement on "other option's default". What did I miss?

Wei.

> Jan
> 
> 

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

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

* Re: [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
  2018-09-13 15:46     ` Wei Liu
@ 2018-09-13 16:01       ` George Dunlap
  0 siblings, 0 replies; 57+ messages in thread
From: George Dunlap @ 2018-09-13 16:01 UTC (permalink / raw)
  To: Wei Liu; +Cc: George Dunlap, xen-devel, Jan Beulich, Andrew Cooper

On 09/13/2018 04:46 PM, Wei Liu wrote:
> On Thu, Sep 06, 2018 at 05:20:53PM +0100, George Dunlap wrote:
>> On 09/04/2018 05:15 PM, Wei Liu wrote:
>>> These functions are only useful for nested hvm, which isn't enabled
>>> when CONFIG_HVM is false.
>>>
>>> Enclose relevant code and fields in CONFIG_HVM. Guard np2m_schedule
>>> with nestedhvm_enabled.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>>  xen/arch/x86/domain.c        |  6 ++++--
>>>  xen/arch/x86/mm/p2m.c        | 18 ++++++++++++++----
>>>  xen/include/asm-x86/domain.h |  2 ++
>>>  xen/include/asm-x86/p2m.h    |  2 ++
>>>  4 files changed, 22 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>>> index 313ebb3221..7c945a2428 100644
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1691,7 +1691,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>>>      {
>>>          _update_runstate_area(prev);
>>>          vpmu_switch_from(prev);
>>> -        np2m_schedule(NP2M_SCHEDLE_OUT);
>>> +        if ( nestedhvm_enabled(prevd) )
>>> +            np2m_schedule(NP2M_SCHEDLE_OUT);
>>>      }
>>>  
>>>      if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
>>> @@ -1758,7 +1759,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>>>  
>>>          /* Must be done with interrupts enabled */
>>>          vpmu_switch_to(next);
>>> -        np2m_schedule(NP2M_SCHEDLE_IN);
>>> +        if ( nestedhvm_enabled(nextd) )
>>> +            np2m_schedule(NP2M_SCHEDLE_IN);
>>
>> There's already a nestedhvm_enabled() check first thing in
>> np2m_schedule().  How does adding this check help the CONFIG_HVM cause?
> 
> np2m_schedule will be gone entirely when !HVM.  Add nestedhvm_enabled
> check here, which always evaluates to false when !HVM, makes compiler
> able to eliminate the call to np2m_schedule when linking.
> 
>>
>> And why not #ifdef out this call, as well as the np2m_schedule()
>> functions entirely?
> 
> The end result is the same. I'm happy to use either.

The end result may usually be the same in the case of !CONFIG_HVM, but
in the case of CONFIG_HVM, you'll have added a completely redundant
conditional.  More importantly,  if you use #ifdef, then it will be
immediately obvious to anyone reading the code that np2m_schedule isn't
called if !CONFIG_HVM.

Unless there are better reasons I'm not aware of, I'd prefer #ifdef here.

Thanks,
 -George

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

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

* Re: [PATCH v3 01/16] x86: change name of parameter for various invlpg functions
  2018-09-04 16:15 ` [PATCH v3 01/16] x86: change name of parameter for various invlpg functions Wei Liu
  2018-09-06 11:12   ` George Dunlap
@ 2018-09-13 16:11   ` George Dunlap
  1 sibling, 0 replies; 57+ messages in thread
From: George Dunlap @ 2018-09-13 16:11 UTC (permalink / raw)
  To: Wei Liu, xen-devel
  Cc: Kevin Tian, Jan Beulich, George Dunlap, Andrew Cooper,
	Tim Deegan, Jun Nakajima, Boris Ostrovsky, Brian Woods,
	Suravee Suthikulpanit

On 09/04/2018 05:15 PM, Wei Liu wrote:
> They all incorrectly named a parameter virtual address while it should
> have been linear address.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Acked-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Acked-by: George Dunlap <george.dunlap@citrix.com>

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

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

* Re: [PATCH v3 11/16] x86/mm: put HVM only code under CONFIG_HVM
  2018-09-13 15:43     ` Wei Liu
@ 2018-09-13 16:31       ` Tamas K Lengyel
  0 siblings, 0 replies; 57+ messages in thread
From: Tamas K Lengyel @ 2018-09-13 16:31 UTC (permalink / raw)
  To: Wei Liu
  Cc: George Dunlap, Xen-devel, Razvan Cojocaru, Jan Beulich, Andrew Cooper

On Thu, Sep 13, 2018 at 9:43 AM Wei Liu <wei.liu2@citrix.com> wrote:
>
> On Fri, Sep 07, 2018 at 03:27:37PM -0600, Tamas K Lengyel wrote:
> > On Tue, Sep 4, 2018, 10:29 AM Wei Liu <wei.liu2@citrix.com> wrote:
> >
> > > Going through the code, HAP, EPT, PoD and ALTP2M depend on HVM code.
> > > Put these components under CONFIG_HVM. This further requires putting
> > > one of the vm event under CONFIG_HVM.
> > >
> > > Altp2m requires a bit more attention because its code is embedded in
> > > generic x86 p2m code.
> > >
> > > Also make hap_enabled evaluate to false when !CONFIG_HVM. Make sure it
> > > evaluate its parameter to avoid unused variable warnings in its users.
> > >
> > > Also sort items in Makefile while at it.
> > >
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > ---
> > >  xen/arch/x86/mm/Makefile         | 11 ++++++-----
> > >  xen/arch/x86/mm/mem_access.c     | 18 +++++++++++++++++-
> > >  xen/arch/x86/mm/mem_sharing.c    |  2 ++
> > >  xen/arch/x86/mm/p2m.c            | 23 ++++++++++++-----------
> > >  xen/common/vm_event.c            |  2 ++
> > >
> >
> > As before, please only apply CONFIG_HVM ifdefs to x86 specific parts. We
> > have an altp2m implementation for ARM that is planned to be posted again to
> > the mailinglist in the near future.
>
> I take that you are specifically talking about common/vm_event.c because
> that's the only common code here in the list of file.

Yes, exactly.

> I have switched to provide a stub for p2m_altp2m_check in x86 header
> instead.

Thanks!
Tamas

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

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

* Re: [PATCH v3 12/16] x86/mm: put paging_update_nestedmode under CONFIG_HVM
  2018-09-04 16:15 ` [PATCH v3 12/16] x86/mm: put paging_update_nestedmode " Wei Liu
@ 2018-09-13 16:39   ` George Dunlap
  0 siblings, 0 replies; 57+ messages in thread
From: George Dunlap @ 2018-09-13 16:39 UTC (permalink / raw)
  To: Wei Liu, xen-devel; +Cc: George Dunlap, Andrew Cooper, Jan Beulich

On 09/04/2018 05:15 PM, Wei Liu wrote:
> Nested HVM is not enabled when !CONFIG_HVM.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: George Dunlap <george.dunlap@citrix.com>

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

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

* Re: [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM
  2018-09-13 15:07     ` Wei Liu
@ 2018-09-14  8:01       ` Jan Beulich
  0 siblings, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2018-09-14  8:01 UTC (permalink / raw)
  To: Wei Liu; +Cc: George Dunlap, Andrew Cooper, xen-devel

>>> On 13.09.18 at 17:07, <wei.liu2@citrix.com> wrote:
> On Fri, Sep 07, 2018 at 01:06:38AM -0600, Jan Beulich wrote:
>> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
>> > @@ -149,6 +149,7 @@ static void p2m_teardown_hostp2m(struct domain *d)
>> >      }
>> >  }
>> >  
>> > +#ifdef CONFIG_HVM
>> >  static void p2m_teardown_nestedp2m(struct domain *d)
>> >  {
>> >      unsigned int i;
>> > @@ -186,6 +187,7 @@ static int p2m_init_nestedp2m(struct domain *d)
>> >  
>> >      return 0;
>> >  }
>> > +#endif
>> 
>> With the goal of limited code churn I think these would better be put
>> around the entire body of the function. That way the ones below
>> enclosing the function calls can go away.
> 
> Later the ifdefs here and some other places will be expand to cover
> altp2m as well. If we enclose only the body here, we will need to do the
> same things for altp2m functions.
> 
> The end result is we will actually have more or less the
> same amount whether we change to that method or not.

But (to me at least) it's a difference whether there are many small
cope (covering a single line, i.e. a call site here) #ifdef-s, or the
same amount of ones covering while function bodies. The latter
imo is better readable overall.

Jan



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

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

* Re: [PATCH v3 14/16] x86: expose CONFIG_HVM
  2018-09-13 16:01     ` Wei Liu
@ 2018-09-14  8:07       ` Jan Beulich
  2018-09-14 10:36         ` Wei Liu
  0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2018-09-14  8:07 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel

>>> On 13.09.18 at 18:01, <wei.liu2@citrix.com> wrote:
> On Fri, Sep 07, 2018 at 01:15:05AM -0600, Jan Beulich wrote:
>> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
>> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> > ---
>> > v3: longer text
>> > v2: use tab to indent
>> > 
>> > Haven't added a dependency on PV_SHIM_EXCLUSIVE because agreement is
>> > not yet reached.
>> 
>> Hmm, but then I would have expected you to at least do the minimal
>> agreed upon change (modifying that other option's default). Beyond
>> that I'm afraid we're moving towards a dead end here.
>> 
> 
> I extended the help text to address Andrew's comment. Other than that, I
> didn't see an agreement on "other option's default". What did I miss?

I don't think there was disagreement on this minimal change (in fact
iirc Andrew explicitly agreed to it [1]). What we can't seem to agree
on is whether to add a "depends on !PV_SHIM_EXCLUSIVE" to HVM.

Jan

[1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg02408.html



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

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

* Re: [PATCH v3 14/16] x86: expose CONFIG_HVM
  2018-09-14  8:07       ` Jan Beulich
@ 2018-09-14 10:36         ` Wei Liu
  0 siblings, 0 replies; 57+ messages in thread
From: Wei Liu @ 2018-09-14 10:36 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel

On Fri, Sep 14, 2018 at 02:07:42AM -0600, Jan Beulich wrote:
> >>> On 13.09.18 at 18:01, <wei.liu2@citrix.com> wrote:
> > On Fri, Sep 07, 2018 at 01:15:05AM -0600, Jan Beulich wrote:
> >> >>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >> > ---
> >> > v3: longer text
> >> > v2: use tab to indent
> >> > 
> >> > Haven't added a dependency on PV_SHIM_EXCLUSIVE because agreement is
> >> > not yet reached.
> >> 
> >> Hmm, but then I would have expected you to at least do the minimal
> >> agreed upon change (modifying that other option's default). Beyond
> >> that I'm afraid we're moving towards a dead end here.
> >> 
> > 
> > I extended the help text to address Andrew's comment. Other than that, I
> > didn't see an agreement on "other option's default". What did I miss?
> 
> I don't think there was disagreement on this minimal change (in fact
> iirc Andrew explicitly agreed to it [1]). What we can't seem to agree
> on is whether to add a "depends on !PV_SHIM_EXCLUSIVE" to HVM.
> 
> Jan
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg02408.html
> 

Oh, then that's more or less what I said in v4 patch. I proposed

   def_bool y if !PV_SHIM_EXCLUSIVE

in the that patch as a compromise.

Wei.

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

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

end of thread, other threads:[~2018-09-14 10:36 UTC | newest]

Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-04 16:15 [PATCH v3 00/16] Make CONFIG_HVM work Wei Liu
2018-09-04 16:15 ` [PATCH v3 01/16] x86: change name of parameter for various invlpg functions Wei Liu
2018-09-06 11:12   ` George Dunlap
2018-09-13 16:11   ` George Dunlap
2018-09-04 16:15 ` [PATCH v3 02/16] x86: introduce and use a set of internal emulation flags Wei Liu
2018-09-06 13:27   ` Jan Beulich
2018-09-06 13:47     ` Wei Liu
2018-09-04 16:15 ` [PATCH v3 03/16] x86: XENMEM_resource_ioreq_server is HVM only Wei Liu
2018-09-04 16:24   ` Paul Durrant
2018-09-04 16:42   ` Wei Liu
2018-09-06 13:29     ` Jan Beulich
2018-09-04 16:15 ` [PATCH v3 04/16] x86: monitor.o is currently " Wei Liu
2018-09-04 16:35   ` Razvan Cojocaru
2018-09-04 16:15 ` [PATCH v3 05/16] x86: PIT emulation is common to both PV and HVM Wei Liu
2018-09-06 14:26   ` Jan Beulich
2018-09-04 16:15 ` [PATCH v3 06/16] libxl: don't set PoD target for PV guests Wei Liu
2018-09-07 13:44   ` Ian Jackson
2018-09-04 16:15 ` [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM Wei Liu
2018-09-04 17:08   ` Razvan Cojocaru
2018-09-04 17:10     ` Razvan Cojocaru
2018-09-04 17:24   ` Julien Grall
2018-09-06 10:57     ` Wei Liu
2018-09-06 15:30       ` George Dunlap
2018-09-06 15:05   ` Jan Beulich
2018-09-06 16:06     ` George Dunlap
2018-09-04 16:15 ` [PATCH v3 08/16] x86/hvm: rearrange content of hvm.h Wei Liu
2018-09-07  6:52   ` Jan Beulich
2018-09-04 16:15 ` [PATCH v3 09/16] x86: provide stubs, declarations and macros in hvm.h Wei Liu
2018-09-07  7:02   ` Jan Beulich
2018-09-13 15:31     ` Wei Liu
2018-09-04 16:15 ` [PATCH v3 10/16] x86/mm: put nested p2m code under CONFIG_HVM Wei Liu
2018-09-06 16:20   ` George Dunlap
2018-09-13 15:46     ` Wei Liu
2018-09-13 16:01       ` George Dunlap
2018-09-07  7:06   ` Jan Beulich
2018-09-13 15:07     ` Wei Liu
2018-09-14  8:01       ` Jan Beulich
2018-09-04 16:15 ` [PATCH v3 11/16] x86/mm: put HVM only " Wei Liu
2018-09-04 17:10   ` Razvan Cojocaru
2018-09-07  7:12   ` Jan Beulich
2018-09-07 21:27   ` Tamas K Lengyel
2018-09-13 15:43     ` Wei Liu
2018-09-13 16:31       ` Tamas K Lengyel
2018-09-04 16:15 ` [PATCH v3 12/16] x86/mm: put paging_update_nestedmode " Wei Liu
2018-09-13 16:39   ` George Dunlap
2018-09-04 16:15 ` [PATCH v3 13/16] xen: connect guest creation with CONFIG_{HVM, PV} Wei Liu
2018-09-04 16:15 ` [PATCH v3 14/16] x86: expose CONFIG_HVM Wei Liu
2018-09-07  7:15   ` Jan Beulich
2018-09-13 16:01     ` Wei Liu
2018-09-14  8:07       ` Jan Beulich
2018-09-14 10:36         ` Wei Liu
2018-09-04 16:15 ` [PATCH v3 15/16] x86/pvshim: disable HVM for PV shim Wei Liu
2018-09-07  7:18   ` Jan Beulich
2018-09-07  7:46     ` Wei Liu
2018-09-07  7:48       ` Wei Liu
2018-09-04 16:15 ` [PATCH v3 16/16] xen: decouple HVM and IOMMU capabilities Wei Liu
2018-09-13 15:52   ` Ian Jackson

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.