[PATCH 00/17] Hi all

[PATCH 00/17] Hi all

[Wei Liu]

This series makes CONFIG_PV work.

Booting a hypervisor with PVH Dom0 works. It also creates / destroys HVM guests
just fine.

Due to issues in PVH implementation, XTF tests cause hypervisor to crash (seen
on staging as well).

root@lcy2-dt108:~# xl info
host                   : lcy2-dt108
release                : 4.18.0-0.bpo.1-amd64
version                : #1 SMP Debian 4.18.6-1~bpo9+1 (2018-09-13)
machine                : x86_64
nr_cpus                : 8
max_cpu_id             : 7
nr_nodes               : 1
cores_per_socket       : 4
threads_per_core       : 2
cpu_mhz                : 3504.048
hw_caps                : bfebfbff:77faf3ff:2c100800:00000121:0000000f:009c6fbf:00000000:00000100
virt_caps              : hvm hvm_directio directio
total_memory           : 32589
free_memory            : 28096
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               : hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64
xen_scheduler          : credit
xen_pagesize           : 4096
platform_params        : virt_start=0xffff800000000000
xen_changeset          : Thu Oct 4 13:33:09 2018 +0100 git:c04e54f5e5
xen_commandline        : placeholder loglvl=all guest_loglvl=all com2=115200,8n1 ucode=scan console=com2, vga console_to_ring sync_console hvm_fep dom0_mem=4096M dom0=pvh noreboot
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        : Thu Oct  4 13:33:27 BST 2018
build_id               : 46468926f8c4d81959d5921df3a923dccdf953f9
xend_config_format     : 4

Some numbers about size of different builds with this series:

Non-debug builds.

No HVM, no PV:
-rwxrwxr-x 1 wei wei 1814076 Oct  4 16:17 xen

No PV:
-rwxrwxr-x 1 wei wei 2272892 Oct  4 16:19 xen

No HVM:
-rwxrwxr-x 1 wei wei 1916476 Oct  4 16:22 xen

Full:
-rwxrwxr-x 1 wei wei 2383484 Oct  4 16:20 xen

Wei.

Wei Liu (17):
  x86/shadow: put PV L1TF functions under CONFIG_PV
  x86/mm: make mm.c build with !CONFIG_PV
  x86: turn is_pv_{,32bit_}{domain,vcpu} into inline functions
  x86: introduce is_pv_64bit_{vcpu,domain}
  x86: make x86_64/traps.c build with !CONFIG_PV
  x86: provide stub for arch_do_multicall_call
  x86/traps: put PV code handlers under CONFIG_PV
  x86: make construct_dom0 build with !CONFIG_PV
  x86/amd: put setting pv_post_outb_hook under CONFIG_PV
  x86: provide stub for switch_compat
  x86: provide stubs for entry point
  x86: connect guest creation with CONFIG_PV
  x86: don't create Dom0 if neither PV nor HVM is available
  x86: don't report PV support when !CONFIG_PV
  x86: expose CONFIG_PV
  x86: introduce new defconfigs for PV and HVM
  automation: build Xen with CONFIG_PV and CONFIG_HVM explicitly set

 automation/scripts/build                 | 18 ++++++++-
 xen/arch/x86/Kconfig                     |  8 ++++-
 xen/arch/x86/configs/hvm_only_defconfig  |  2 +-
 xen/arch/x86/configs/no_hvm_pv_defconfig |  2 +-
 xen/arch/x86/configs/pv_only_defconfig   |  2 +-
 xen/arch/x86/cpu/amd.c                   |  4 ++-
 xen/arch/x86/dom0_build.c                | 12 ++++-
 xen/arch/x86/hypercall.c                 |  9 ++++-
 xen/arch/x86/mm.c                        | 55 ++++++++++++++++++++-----
 xen/arch/x86/setup.c                     | 21 ++++++++--
 xen/arch/x86/traps.c                     | 38 +++++++++++++++++-
 xen/arch/x86/x86_64/Makefile             |  5 ++-
 xen/arch/x86/x86_64/no-pv.c              | 41 +++++++++++++++++++-
 xen/arch/x86/x86_64/traps.c              |  6 ++-
 xen/common/domain.c                      | 21 +++++++++-
 xen/include/asm-x86/domain.h             |  2 +-
 xen/include/asm-x86/hypercall.h          |  3 +-
 xen/include/asm-x86/shadow.h             |  4 ++-
 xen/include/xen/compat.h                 |  9 ++++-
 xen/include/xen/sched.h                  | 31 +++++++++++++-
 20 files changed, 269 insertions(+), 24 deletions(-)
 create mode 100644 xen/arch/x86/configs/hvm_only_defconfig
 create mode 100644 xen/arch/x86/configs/no_hvm_pv_defconfig
 create mode 100644 xen/arch/x86/configs/pv_only_defconfig
 create mode 100644 xen/arch/x86/x86_64/no-pv.c

base-commit: 74584a367051bc0d6f4b96fd360fa7bc6538fc39
-- 
git-series 0.9.1

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

[PATCH 01/17] x86/shadow: put PV L1TF functions under CONFIG_PV

[Wei Liu]

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

diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index b3ebe56..7d88637 100644
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -163,6 +163,8 @@ static inline bool is_l1tf_safe_maddr(intpte_t pte)
     return maddr == 0 || maddr >= l1tf_safe_maddr;
 }
 
+#ifdef CONFIG_PV
+
 static inline bool pv_l1tf_check_pte(struct domain *d, unsigned int level,
                                      intpte_t pte)
 {
@@ -241,6 +243,8 @@ static inline void pv_l1tf_domain_destroy(struct domain *d)
 #endif
 }
 
+#endif /* CONFIG_PV */
+
 /* Remove all shadows of the guest mfn. */
 static inline void shadow_remove_all_shadows(struct domain *d, mfn_t gmfn)
 {
-- 
git-series 0.9.1

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

[PATCH 02/17] x86/mm: make mm.c build with !CONFIG_PV

[Wei Liu]

Put PV only code under CONFIG_PV.

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

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 02abd06..f211383 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -625,6 +625,7 @@ const char __section(".bss.page_aligned.const") __aligned(PAGE_SIZE)
     zero_page[PAGE_SIZE];
 
 
+#ifdef CONFIG_PV
 static int alloc_segdesc_page(struct page_info *page)
 {
     const struct domain *owner = page_get_owner(page);
@@ -639,6 +640,7 @@ static int alloc_segdesc_page(struct page_info *page)
 
     return i == 512 ? 0 : -EINVAL;
 }
+#endif
 
 static int _get_page_type(struct page_info *page, unsigned long type,
                           bool preemptible);
@@ -663,15 +665,6 @@ static int get_page_and_type_from_mfn(
     return rc;
 }
 
-static void put_data_page(
-    struct page_info *page, int writeable)
-{
-    if ( writeable )
-        put_page_and_type(page);
-    else
-        put_page(page);
-}
-
 #ifdef CONFIG_PV_LINEAR_PT
 
 static bool inc_linear_entries(struct page_info *pg)
@@ -1129,6 +1122,8 @@ get_page_from_l1e(
     return -EBUSY;
 }
 
+#ifdef CONFIG_PV
+
 define_get_linear_pagetable(l2);
 static int
 get_page_from_l2e(
@@ -1197,6 +1192,8 @@ get_page_from_l4e(
     return rc;
 }
 
+#endif /* CONFIG_PV */
+
 static int _put_page_type(struct page_info *page, bool preemptible,
                           struct page_info *ptpg);
 
@@ -1277,6 +1274,8 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner)
 }
 
 
+#ifdef CONFIG_PV
+
 /*
  * NB. Virtual address 'l2e' maps to a machine address within frame 'pfn'.
  * Note also that this automatically deals correctly with linear p.t.'s.
@@ -1306,6 +1305,15 @@ static int put_page_from_l2e(l2_pgentry_t l2e, unsigned long pfn)
     return 0;
 }
 
+static void put_data_page(
+    struct page_info *page, int writeable)
+{
+    if ( writeable )
+        put_page_and_type(page);
+    else
+        put_page(page);
+}
+
 static int put_page_from_l3e(l3_pgentry_t l3e, unsigned long pfn,
                              int partial, bool defer)
 {
@@ -1620,6 +1628,8 @@ void init_xen_pae_l2_slots(l2_pgentry_t *l2t, const struct domain *d)
            COMPAT_L2_PAGETABLE_XEN_SLOTS(d) * sizeof(*l2t));
 }
 
+#endif /* CONFIG_PV */
+
 /*
  * Fill an L4 with Xen entries.
  *
@@ -1726,6 +1736,7 @@ void zap_ro_mpt(mfn_t mfn)
     unmap_domain_page(l4tab);
 }
 
+#ifdef CONFIG_PV
 static int alloc_l4_table(struct page_info *page)
 {
     struct domain *d = page_get_owner(page);
@@ -1916,6 +1927,7 @@ static int free_l4_table(struct page_info *page)
 
     return rc;
 }
+#endif
 
 #ifndef NDEBUG
 /*
@@ -2000,6 +2012,7 @@ void page_unlock(struct page_info *page)
     current_locked_page_set(NULL);
 }
 
+#ifdef CONFIG_PV
 /*
  * PTE flags that a guest may change without re-validating the PTE.
  * All other bits affect translation, caching, or Xen's safety.
@@ -2311,6 +2324,7 @@ static int mod_l4_entry(l4_pgentry_t *pl4e,
     put_page_from_l4e(ol4e, pfn, 0, 1);
     return rc;
 }
+#endif /* CONFIG_PV */
 
 static int cleanup_page_cacheattr(struct page_info *page)
 {
@@ -2418,6 +2432,7 @@ static void get_page_light(struct page_info *page)
 static int alloc_page_type(struct page_info *page, unsigned long type,
                            int preemptible)
 {
+#ifdef CONFIG_PV
     struct domain *owner = page_get_owner(page);
     int rc;
 
@@ -2487,12 +2502,17 @@ static int alloc_page_type(struct page_info *page, unsigned long type,
     }
 
     return rc;
+#else
+    ASSERT_UNREACHABLE();
+    return -EINVAL;
+#endif
 }
 
 
 int free_page_type(struct page_info *page, unsigned long type,
                    int preemptible)
 {
+#ifdef CONFIG_PV
     struct domain *owner = page_get_owner(page);
     unsigned long gmfn;
     int rc;
@@ -2541,6 +2561,10 @@ int free_page_type(struct page_info *page, unsigned long type,
     }
 
     return rc;
+#else
+    ASSERT_UNREACHABLE();
+    return -EINVAL;
+#endif
 }
 
 
@@ -2931,6 +2955,7 @@ int vcpu_destroy_pagetables(struct vcpu *v)
 
 int new_guest_cr3(mfn_t mfn)
 {
+#ifdef CONFIG_PV
     struct vcpu *curr = current;
     struct domain *d = curr->domain;
     int rc;
@@ -3029,6 +3054,10 @@ int new_guest_cr3(mfn_t mfn)
     }
 
     return rc;
+#else
+    ASSERT_UNREACHABLE();
+    return -EINVAL;
+#endif
 }
 
 static struct domain *get_pg_owner(domid_t domid)
@@ -3643,6 +3672,8 @@ long do_mmuext_op(
     return rc;
 }
 
+#ifdef CONFIG_PV
+
 long do_mmu_update(
     XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
     unsigned int count,
@@ -3972,6 +4003,8 @@ long do_mmu_update(
     return rc;
 }
 
+#endif
+
 int donate_page(
     struct domain *d, struct page_info *page, unsigned int memflags)
 {
@@ -4078,6 +4111,8 @@ int steal_page(
     return -EINVAL;
 }
 
+#ifdef CONFIG_PV
+
 static int __do_update_va_mapping(
     unsigned long va, u64 val64, unsigned long flags, struct domain *pg_owner)
 {
@@ -4241,6 +4276,8 @@ int compat_update_va_mapping_otherdomain(unsigned int va,
     return rc;
 }
 
+#endif /* CONFIG_PV */
+
 typedef struct e820entry e820entry_t;
 DEFINE_XEN_GUEST_HANDLE(e820entry_t);
 
-- 
git-series 0.9.1

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

[PATCH 03/17] x86: turn is_pv_{, 32bit_}{domain, vcpu} into inline functions

[Wei Liu]

And make them work with CONFIG_PV.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/asm-x86/domain.h |  2 --
 xen/include/xen/sched.h      | 21 +++++++++++++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index cb0721e..e7b8227 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -13,8 +13,6 @@
 #include <public/hvm/hvm_info_table.h>
 
 #define has_32bit_shinfo(d)    ((d)->arch.has_32bit_shinfo)
-#define is_pv_32bit_domain(d)  ((d)->arch.is_32bit_pv)
-#define is_pv_32bit_vcpu(v)    (is_pv_32bit_domain((v)->domain))
 
 #define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \
         (d)->arch.hvm.irq->callback_via_type == HVMIRQ_callback_vector)
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 0ba80cb..b32ad57 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -877,8 +877,25 @@ void watchdog_domain_destroy(struct domain *d);
 
 #define VM_ASSIST(d, t) (test_bit(VMASST_TYPE_ ## t, &(d)->vm_assist))
 
-#define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
-#define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
+static inline bool is_pv_domain(const struct domain *d)
+{
+    return IS_ENABLED(CONFIG_PV) ? d->guest_type == guest_type_pv : false;
+}
+
+static inline bool is_pv_vcpu(const struct vcpu *v)
+{
+    return is_pv_domain(v->domain);
+}
+
+static inline bool is_pv_32bit_domain(const struct domain *d)
+{
+    return is_pv_domain(d) && d->arch.is_32bit_pv;
+}
+
+static inline bool is_pv_32bit_vcpu(const struct vcpu *v)
+{
+    return is_pv_32bit_domain(v->domain);
+}
 
 static inline bool is_hvm_domain(const struct domain *d)
 {
-- 
git-series 0.9.1

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

[PATCH 04/17] x86: introduce is_pv_64bit_{vcpu, domain}

[Wei Liu]

This is useful to rewrite the following pattern (v is PV vcpu)

   if ( is_pv_32bit_vcpu(v) )
       do_foo;
   else
       do_bar;

to

   if ( is_pv_32bit_vcpu(v) )
       do_foo;
   else if ( is_pv_64bit_vcpu(v) )
       do_bar;
   else
       ASSERT_UNREACHABLE;
.

Previously it is not possible to rely on DCE to eliminate the do_bar
part. It becomes possible with the new code structure.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/xen/sched.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b32ad57..46c22f1 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -897,6 +897,16 @@ static inline bool is_pv_32bit_vcpu(const struct vcpu *v)
     return is_pv_32bit_domain(v->domain);
 }
 
+static inline bool is_pv_64bit_domain(const struct domain *d)
+{
+    return is_pv_domain(d) && !d->arch.is_32bit_pv;
+}
+
+static inline bool is_pv_64bit_vcpu(const struct vcpu *v)
+{
+    return is_pv_64bit_domain(v->domain);
+}
+
 static inline bool is_hvm_domain(const struct domain *d)
 {
     return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : false;
-- 
git-series 0.9.1

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

[PATCH 05/17] x86: make x86_64/traps.c build with !CONFIG_PV

[Wei Liu]

Provide declarations for hypercall_page_initialise_ring*_kernel, make
sure DCE work as expected.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/x86_64/traps.c     | 6 ++++--
 xen/include/asm-x86/hypercall.h | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index afe539f..27154f2 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -354,10 +354,12 @@ void hypercall_page_initialise(struct domain *d, void *hypercall_page)
     memset(hypercall_page, 0xCC, PAGE_SIZE);
     if ( is_hvm_domain(d) )
         hvm_hypercall_page_initialise(d, hypercall_page);
-    else if ( !is_pv_32bit_domain(d) )
+    else if ( is_pv_64bit_domain(d) )
         hypercall_page_initialise_ring3_kernel(hypercall_page);
-    else
+    else if ( is_pv_32bit_domain(d) )
         hypercall_page_initialise_ring1_kernel(hypercall_page);
+    else
+        ASSERT_UNREACHABLE();
 }
 
 /*
diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h
index da38b79..7f302ec 100644
--- a/xen/include/asm-x86/hypercall.h
+++ b/xen/include/asm-x86/hypercall.h
@@ -28,9 +28,10 @@ extern const hypercall_args_t hypercall_args_table[NR_hypercalls];
 #ifdef CONFIG_PV
 extern const hypercall_table_t pv_hypercall_table[];
 void pv_hypercall(struct cpu_user_regs *regs);
+#endif
+
 void hypercall_page_initialise_ring3_kernel(void *hypercall_page);
 void hypercall_page_initialise_ring1_kernel(void *hypercall_page);
-#endif
 
 /*
  * Both do_mmuext_op() and do_mmu_update():
-- 
git-series 0.9.1

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

[PATCH 06/17] x86: provide stub for arch_do_multicall_call

[Wei Liu]

This hypercall is PV only on x86. Provide a stub for it when
!CONFIG_PV.

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

diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 74bde5e..a2f4797 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -248,6 +248,15 @@ int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
     return rc;
 }
 
+#ifndef CONFIG_PV
+/* Stub for arch_do_multicall_call */
+enum mc_disposition arch_do_multicall_call(struct mc_state *mc)
+{
+    return mc_exit;
+}
+
+#endif
+
 /*
  * Local variables:
  * mode: C
-- 
git-series 0.9.1

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

[PATCH 07/17] x86/traps: put PV code handlers under CONFIG_PV

[Wei Liu]

The trap handlers in hypervisor need to forward events to PV guests if
necessary. If there is no PV guest relevant code should be excluded.

Put code under CONFIG_PV and add ASSERT_UNREACHABLE.

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

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 3988753..6c2729c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -743,9 +743,13 @@ static void do_trap(struct cpu_user_regs *regs)
 
     if ( guest_mode(regs) )
     {
+#ifdef CONFIG_PV
         pv_inject_hw_exception(trapnr,
                                (TRAP_HAVE_EC & (1u << trapnr))
                                ? regs->error_code : X86_EVENT_NO_EC);
+#else
+        ASSERT_UNREACHABLE();
+#endif
         return;
     }
 
@@ -974,8 +978,12 @@ void do_invalid_op(struct cpu_user_regs *regs)
 
     if ( likely(guest_mode(regs)) )
     {
+#ifdef CONFIG_PV
         if ( pv_emulate_invalid_op(regs) )
             pv_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC);
+#else
+        ASSERT_UNREACHABLE();
+#endif
         return;
     }
 
@@ -1103,7 +1111,11 @@ void do_int3(struct cpu_user_regs *regs)
         return;
     }
 
+#ifdef CONFIG_PV
     pv_inject_hw_exception(TRAP_int3, X86_EVENT_NO_EC);
+#else
+    ASSERT_UNREACHABLE();
+#endif
 }
 
 static void reserved_bit_page_fault(unsigned long addr,
@@ -1140,6 +1152,7 @@ static int handle_ldt_mapping_fault(unsigned int offset,
         if ( !guest_mode(regs) )
             return 0;
 
+#ifdef CONFIG_PV
         /* Access would have become non-canonical? Pass #GP[sel] back. */
         if ( unlikely(!is_canonical_address(curr->arch.pv.ldt_base + offset)) )
         {
@@ -1151,6 +1164,9 @@ static int handle_ldt_mapping_fault(unsigned int offset,
             /* else pass the #PF back, with adjusted %cr2. */
             pv_inject_page_fault(regs->error_code,
                                  curr->arch.pv.ldt_base + offset);
+#else
+        ASSERT_UNREACHABLE();
+#endif
     }
 
     return EXCRET_fault_fixed;
@@ -1457,7 +1473,11 @@ void do_page_fault(struct cpu_user_regs *regs)
     if ( unlikely(regs->error_code & PFEC_reserved_bit) )
         reserved_bit_page_fault(addr, regs);
 
+#ifdef CONFIG_PV
     pv_inject_page_fault(regs->error_code, addr);
+#else
+    ASSERT_UNREACHABLE();
+#endif
 }
 
 /*
@@ -1494,7 +1514,9 @@ void __init do_early_page_fault(struct cpu_user_regs *regs)
 
 void do_general_protection(struct cpu_user_regs *regs)
 {
+#ifdef CONFIG_PV
     struct vcpu *v = current;
+#endif
     unsigned long fixup;
 
     if ( debugger_trap_entry(TRAP_gp_fault, regs) )
@@ -1506,6 +1528,7 @@ void do_general_protection(struct cpu_user_regs *regs)
     if ( !guest_mode(regs) )
         goto gp_in_kernel;
 
+#ifdef CONFIG_PV
     /*
      * Cunning trick to allow arbitrary "INT n" handling.
      *
@@ -1556,6 +1579,9 @@ void do_general_protection(struct cpu_user_regs *regs)
 
     /* Pass on GPF as is. */
     pv_inject_hw_exception(TRAP_gp_fault, regs->error_code);
+#else
+    ASSERT_UNREACHABLE();
+#endif
     return;
 
  gp_in_kernel:
@@ -1744,7 +1770,9 @@ void unset_nmi_callback(void)
 
 void do_device_not_available(struct cpu_user_regs *regs)
 {
+#ifdef CONFIG_PV
     struct vcpu *curr = current;
+#endif
 
     if ( !guest_mode(regs) )
     {
@@ -1762,6 +1790,7 @@ void do_device_not_available(struct cpu_user_regs *regs)
         return;
     }
 
+#ifdef CONFIG_PV
     vcpu_restore_fpu_lazy(curr);
 
     if ( curr->arch.pv.ctrlreg[0] & X86_CR0_TS )
@@ -1771,6 +1800,9 @@ void do_device_not_available(struct cpu_user_regs *regs)
     }
     else
         TRACE_0D(TRC_PV_MATH_STATE_RESTORE);
+#else
+    ASSERT_UNREACHABLE();
+#endif
 
     return;
 }
@@ -1778,7 +1810,9 @@ void do_device_not_available(struct cpu_user_regs *regs)
 void do_debug(struct cpu_user_regs *regs)
 {
     unsigned long dr6;
+#ifdef CONFIG_PV
     struct vcpu *v = current;
+#endif
 
     /* Stash dr6 as early as possible. */
     dr6 = read_debugreg(6);
@@ -1888,11 +1922,15 @@ void do_debug(struct cpu_user_regs *regs)
         return;
     }
 
+#ifdef CONFIG_PV
     /* Save debug status register where guest OS can peek at it */
     v->arch.debugreg[6] |= (dr6 & ~X86_DR6_DEFAULT);
     v->arch.debugreg[6] &= (dr6 | ~X86_DR6_DEFAULT);
 
     pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+#else
+    ASSERT_UNREACHABLE();
+#endif
 }
 
 static void __init noinline __set_intr_gate(unsigned int n,
-- 
git-series 0.9.1

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

[PATCH 08/17] x86: make construct_dom0 build with !CONFIG_PV

[Wei Liu]

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

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 86eb7db..4577111 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -509,8 +509,16 @@ int __init construct_dom0(struct domain *d, const module_t *image,
     }
 #endif
 
-    rc = (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
-         (d, image, image_headroom, initrd, cmdline);
+    if ( is_hvm_domain(d) )
+        rc = dom0_construct_pvh(d, image, image_headroom, initrd, cmdline);
+    else if ( is_pv_domain(d) )
+        rc = dom0_construct_pv(d, image, image_headroom, initrd, cmdline);
+    else
+    {
+        ASSERT_UNREACHABLE();
+        rc = -EINVAL;
+    }
+
     if ( rc )
         return rc;
 
-- 
git-series 0.9.1

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

[PATCH 09/17] x86/amd: put setting pv_post_outb_hook under CONFIG_PV

[Wei Liu]

It is used by PV code only.

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

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c394c1c..5e2112e 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -443,12 +443,14 @@ static void disable_c1e(void *unused)
 		       smp_processor_id(), msr_content);
 }
 
+#ifdef CONFIG_PV
 static void check_disable_c1e(unsigned int port, u8 value)
 {
 	/* C1E is sometimes enabled during entry to ACPI mode. */
 	if ((port == acpi_smi_cmd) && (value == acpi_enable_value))
 		on_each_cpu(disable_c1e, NULL, 1);
 }
+#endif
 
 /*
  * BIOS is expected to clear MtrrFixDramModEn bit. According to AMD BKDG : 
@@ -627,8 +629,10 @@ static void init_amd(struct cpuinfo_x86 *c)
 	{
 	case 0xf ... 0x17:
 		disable_c1e(NULL);
+#ifdef CONFIG_PV
 		if (acpi_smi_cmd && (acpi_enable_value | acpi_disable_value))
 			pv_post_outb_hook = check_disable_c1e;
+#endif
 		break;
 	}
 
-- 
git-series 0.9.1

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

[PATCH 10/17] x86: provide stub for switch_compat

[Wei Liu]

DCE couldn't have helped here because the invocation of switch_compat
also depends on external input.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/xen/compat.h |  9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/xen/include/xen/compat.h b/xen/include/xen/compat.h
index 895e2ff..6f72353 100644
--- a/xen/include/xen/compat.h
+++ b/xen/include/xen/compat.h
@@ -228,7 +228,16 @@ struct vcpu_runstate_info;
 void xlat_vcpu_runstate_info(struct vcpu_runstate_info *);
 
 struct domain;
+
+#ifdef CONFIG_PV
 int switch_compat(struct domain *);
+#else
+# include <xen/errno.h>
+static inline int switch_compat(struct domain *d)
+{
+    return -EINVAL;
+}
+#endif
 
 #else
 
-- 
git-series 0.9.1

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

[PATCH 11/17] x86: provide stubs for entry point

[Wei Liu]

Instead of trying to split up entry.S and compat/entry.S, simply
provide some stubs for them.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/x86_64/Makefile |  5 +++++-
 xen/arch/x86/x86_64/no-pv.c  | 41 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+)
 create mode 100644 xen/arch/x86/x86_64/no-pv.c

diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile
index f336a6a..6cf38a0 100644
--- a/xen/arch/x86/x86_64/Makefile
+++ b/xen/arch/x86/x86_64/Makefile
@@ -8,6 +8,11 @@ obj-y += acpi_mmcfg.o
 obj-y += mmconf-fam10h.o
 obj-y += mmconfig_64.o
 obj-y += mmconfig-shared.o
+
+ifneq ($(CONFIG_PV),y)
+obj-y += no-pv.o
+endif
+
 obj-y += domain.o
 obj-y += cpu_idle.o
 obj-y += cpufreq.o
diff --git a/xen/arch/x86/x86_64/no-pv.c b/xen/arch/x86/x86_64/no-pv.c
new file mode 100644
index 0000000..b8dec9a
--- /dev/null
+++ b/xen/arch/x86/x86_64/no-pv.c
@@ -0,0 +1,41 @@
+#include <xen/lib.h>
+
+struct cpu_user_regs;
+struct vcpu;
+
+void do_entry_int82(struct cpu_user_regs *regs)
+{
+    ASSERT_UNREACHABLE();
+}
+
+bool set_guest_machinecheck_trapbounce(void)
+{
+    ASSERT_UNREACHABLE();
+    return false;
+}
+
+bool set_guest_nmi_trapbounce(void)
+{
+    ASSERT_UNREACHABLE();
+    return false;
+}
+
+void pv_hypercall(struct cpu_user_regs *regs)
+{
+    ASSERT_UNREACHABLE();
+}
+
+void toggle_guest_mode(struct vcpu *v)
+{
+    ASSERT_UNREACHABLE();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
git-series 0.9.1

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

[PATCH 12/17] x86: connect guest creation with CONFIG_PV

[Wei Liu]

This is a bit more complicated than the HVM case because system
domains have PV guest type.

Instead of inventing a new guest type, we still set system domains'
type to PV. This shouldn't really matter in practice.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/domain.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 65151e2..8a4698f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -322,17 +322,34 @@ 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) )
+    if ( config )
     {
+        ASSERT(!is_system_domain(d));
+        if ( config->flags & XEN_DOMCTL_CDF_hvm_guest)
+        {
 #ifdef CONFIG_HVM
-        d->guest_type = guest_type_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
+        }
     }
     else
+    {
+        /* The type of system domain shouldn't matter. */
+        ASSERT(is_system_domain(d));
         d->guest_type = guest_type_pv;
+    }
 
     TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
 
-- 
git-series 0.9.1

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

[PATCH 13/17] x86: don't create Dom0 if neither PV nor HVM is available

[Wei Liu]

This will give us a Xen setting in idle loop. This doesn't have
practical use except for debugging purpose.

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

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 6a44902..6ac5d32 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -114,11 +114,13 @@ unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
 #define SMEP_HVM_ONLY (-1)
 static s8 __initdata opt_smep = 1;
 
+#if defined(CONFIG_PV) || defined(CONFIG_HVM)
 /*
  * Initial domain place holder. Needs to be global so it can be created in
  * __start_xen and unpaused in init_done.
  */
 static struct domain *__initdata dom0;
+#endif
 
 static int __init parse_smep_param(const char *s)
 {
@@ -594,7 +596,9 @@ static void noinline init_done(void)
 
     system_state = SYS_STATE_active;
 
+#if defined(CONFIG_PV) || defined(CONFIG_HVM)
     domain_unpause_by_systemcontroller(dom0);
+#endif
 
     /* MUST be done prior to removing .init data. */
     unregister_init_virtual_region();
@@ -1706,10 +1710,12 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     }
     dom0_cfg.max_vcpus = dom0_max_vcpus();
 
+#if defined(CONFIG_PV) || defined(CONFIG_HVM)
     /* Create initial domain 0. */
     dom0 = domain_create(get_initial_domain_id(), &dom0_cfg, !pv_shim);
     if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
         panic("Error creating domain 0\n");
+#endif
 
     /* Grab the DOM0 command line. */
     cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
@@ -1765,6 +1771,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
            cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
            cpu_has_nx ? "" : "not ");
 
+#if defined(CONFIG_PV) || defined(CONFIG_HVM)
     /*
      * We're going to setup domain0 using the module(s) that we stashed safely
      * above our heap. The second module, if present, is an initrd ramdisk.
@@ -1779,6 +1786,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         write_cr4(read_cr4() | X86_CR4_SMAP);
         cr4_pv32_mask |= X86_CR4_SMAP;
     }
+#endif
 
     heap_init_late();
 
@@ -1793,7 +1801,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     dmi_end_boot();
 
+#if defined(CONFIG_PV) || defined(CONFIG_HVM)
     setup_io_bitmap(dom0);
+#endif
 
     if ( bsp_delay_spec_ctrl )
     {
-- 
git-series 0.9.1

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

[PATCH 14/17] x86: don't report PV support when !CONFIG_PV

[Wei Liu]

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

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 6ac5d32..293c9aa 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1827,10 +1827,13 @@ void arch_get_xen_caps(xen_capabilities_info_t *info)
 
     (*info)[0] = '\0';
 
-    snprintf(s, sizeof(s), "xen-%d.%d-x86_64 ", major, minor);
-    safe_strcat(*info, s);
-    snprintf(s, sizeof(s), "xen-%d.%d-x86_32p ", major, minor);
-    safe_strcat(*info, s);
+    if ( IS_ENABLED(CONFIG_PV) )
+    {
+        snprintf(s, sizeof(s), "xen-%d.%d-x86_64 ", major, minor);
+        safe_strcat(*info, s);
+        snprintf(s, sizeof(s), "xen-%d.%d-x86_32p ", major, minor);
+        safe_strcat(*info, s);
+    }
     if ( hvm_enabled )
     {
         snprintf(s, sizeof(s), "hvm-%d.%d-x86_32 ", major, minor);
-- 
git-series 0.9.1

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

[PATCH 15/17] x86: expose CONFIG_PV

[Wei Liu]

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

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 548cbf9..955443f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -37,6 +37,14 @@ source "arch/Kconfig"
 
 config PV
 	def_bool y
+	prompt "PV support"
+	---help---
+	  Interfaces to support PV guests which don't require hardware
+	  support like Intel's VT-x or AMD's SVM.
+
+	  This option is needed if you want to run PV guests.
+
+	  If unsure, say Y.
 
 config PV_LINEAR_PT
        bool "Support for PV linear pagetables"
-- 
git-series 0.9.1

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

[PATCH 16/17] x86: introduce new defconfigs for PV and HVM

[Wei Liu]

They will be used by build tests.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/configs/hvm_only_defconfig  | 2 ++
 xen/arch/x86/configs/no_hvm_pv_defconfig | 2 ++
 xen/arch/x86/configs/pv_only_defconfig   | 2 ++
 3 files changed, 6 insertions(+)
 create mode 100644 xen/arch/x86/configs/hvm_only_defconfig
 create mode 100644 xen/arch/x86/configs/no_hvm_pv_defconfig
 create mode 100644 xen/arch/x86/configs/pv_only_defconfig

diff --git a/xen/arch/x86/configs/hvm_only_defconfig b/xen/arch/x86/configs/hvm_only_defconfig
new file mode 100644
index 0000000..e82cc04
--- /dev/null
+++ b/xen/arch/x86/configs/hvm_only_defconfig
@@ -0,0 +1,2 @@
+CONFIG_HVM=y
+# CONFIG_PV is not set
diff --git a/xen/arch/x86/configs/no_hvm_pv_defconfig b/xen/arch/x86/configs/no_hvm_pv_defconfig
new file mode 100644
index 0000000..ed853cd
--- /dev/null
+++ b/xen/arch/x86/configs/no_hvm_pv_defconfig
@@ -0,0 +1,2 @@
+# CONFIG_HVM is not set
+# CONFIG_PV is not set
diff --git a/xen/arch/x86/configs/pv_only_defconfig b/xen/arch/x86/configs/pv_only_defconfig
new file mode 100644
index 0000000..aca77b6
--- /dev/null
+++ b/xen/arch/x86/configs/pv_only_defconfig
@@ -0,0 +1,2 @@
+CONFIG_PV=y
+# CONFIG_HVM is not set
-- 
git-series 0.9.1

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

[PATCH 17/17] automation: build Xen with CONFIG_PV and CONFIG_HVM explicitly set

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 automation/scripts/build | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/automation/scripts/build b/automation/scripts/build
index c463b06..8af3fab 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -31,3 +31,21 @@ fi
 ./configure "${cfgargs[@]}"
 
 make -j$(nproc) dist
+
+# build PV only, HVM only and bare bone Xen on x86
+if [[ "${XEN_TARGET_ARCH}" == "x86_64" ]]; then
+    # PV only
+    rm -f xen/.config
+    make -C xen KBUILD_DEFCONFIG=pv_only_defconfig XEN_CONFIG_EXPERT=y defconfig
+    make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y
+
+    # HVM only
+    rm -f xen/.config
+    make -C xen KBUILD_DEFCONFIG=hvm_only_defconfig XEN_CONFIG_EXPERT=y defconfig
+    make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y
+
+    # Bare bone
+    rm -f xen/.config
+    make -C xen KBUILD_DEFCONFIG=no_hvm_pv_defconfig XEN_CONFIG_EXPERT=y defconfig
+    make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y
+fi
-- 
git-series 0.9.1

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

Re: [PATCH 17/17] automation: build Xen with CONFIG_PV and CONFIG_HVM explicitly set

[Doug Goldstein]

On Thu, Oct 04, 2018 at 04:43:36PM +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Looks good.

Acked-by: Doug Goldstein <cardoe@cardoe.com>

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

Re: [PATCH 01/17] x86/shadow: put PV L1TF functions under CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

If applicable (not sure whether Tim would have to ack it instead):
Acked-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH 02/17] x86/mm: make mm.c build with !CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> @@ -1277,6 +1274,8 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner)
>  }
>  
>  
> +#ifdef CONFIG_PV
> +
>  /*

Could you please avoid inserting yet another blank line here,
and instead make use of the existing so far one too many?

Jan



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

Re: [PATCH 03/17] x86: turn is_pv_{, 32bit_}{domain, vcpu} into inline functions

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -877,8 +877,25 @@ void watchdog_domain_destroy(struct domain *d);
>  
>  #define VM_ASSIST(d, t) (test_bit(VMASST_TYPE_ ## t, &(d)->vm_assist))
>  
> -#define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
> -#define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
> +static inline bool is_pv_domain(const struct domain *d)
> +{
> +    return IS_ENABLED(CONFIG_PV) ? d->guest_type == guest_type_pv : false;
> +}
> +
> +static inline bool is_pv_vcpu(const struct vcpu *v)
> +{
> +    return is_pv_domain(v->domain);
> +}
> +
> +static inline bool is_pv_32bit_domain(const struct domain *d)
> +{
> +    return is_pv_domain(d) && d->arch.is_32bit_pv;
> +}
> +
> +static inline bool is_pv_32bit_vcpu(const struct vcpu *v)
> +{
> +    return is_pv_32bit_domain(v->domain);
> +}

Afaict this breaks the Arm build, i.e. I think you need e.g.
#ifdef CONFIG_COMPAT around the last two. (I can see why,
being inline functions now, they can't remain in the arch-specific
header.) With this
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

Re: [PATCH 04/17] x86: introduce is_pv_64bit_{vcpu, domain}

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> This is useful to rewrite the following pattern (v is PV vcpu)
> 
>    if ( is_pv_32bit_vcpu(v) )
>        do_foo;
>    else
>        do_bar;
> 
> to
> 
>    if ( is_pv_32bit_vcpu(v) )
>        do_foo;
>    else if ( is_pv_64bit_vcpu(v) )
>        do_bar;
>    else
>        ASSERT_UNREACHABLE;
> .
> 
> Previously it is not possible to rely on DCE to eliminate the do_bar
> part. It becomes possible with the new code structure.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Provided the additions go inside the #ifdef-s mentioned for patch 3
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan



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

Re: [PATCH 05/17] x86: make x86_64/traps.c build with !CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> Provide declarations for hypercall_page_initialise_ring*_kernel, make
> sure DCE work as expected.
> 
> 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

Re: [PATCH 06/17] x86: provide stub for arch_do_multicall_call

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/hypercall.c
> +++ b/xen/arch/x86/hypercall.c
> @@ -248,6 +248,15 @@ int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
>      return rc;
>  }
>  
> +#ifndef CONFIG_PV
> +/* Stub for arch_do_multicall_call */
> +enum mc_disposition arch_do_multicall_call(struct mc_state *mc)
> +{
> +    return mc_exit;
> +}
> +
> +#endif

Please drop the blank line above here. For the moment I'm fine, so
Acked-by: Jan Beulich <jbeulich@suse.com>
but I really think HVM should gain multicall support, seeing that ARM
has this as well. At that point, the #ifdef above will start to look a
little funny.

Jan



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

Re: [PATCH 07/17] x86/traps: put PV code handlers under CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> The trap handlers in hypervisor need to forward events to PV guests if
> necessary. If there is no PV guest relevant code should be excluded.
> 
> Put code under CONFIG_PV and add ASSERT_UNREACHABLE.

-ETOOMANYIFDEFS

What's wrong with an inline stub for pv_inject_event()? This won't
eliminate all #ifdef-s, but quite a few of them afaics.

> @@ -1140,6 +1152,7 @@ static int handle_ldt_mapping_fault(unsigned int offset,
>          if ( !guest_mode(regs) )
>              return 0;
>  
> +#ifdef CONFIG_PV
>          /* Access would have become non-canonical? Pass #GP[sel] back. */
>          if ( unlikely(!is_canonical_address(curr->arch.pv.ldt_base + offset)) )
>          {
> @@ -1151,6 +1164,9 @@ static int handle_ldt_mapping_fault(unsigned int offset,
>              /* else pass the #PF back, with adjusted %cr2. */
>              pv_inject_page_fault(regs->error_code,
>                                   curr->arch.pv.ldt_base + offset);
> +#else
> +        ASSERT_UNREACHABLE();
> +#endif

I think here it should be the call site to gain #ifdef (around the
entire if()), and the function as a whole should then be put in
another #ifdef, or be moved to pv/traps.c.

> @@ -1494,7 +1514,9 @@ void __init do_early_page_fault(struct cpu_user_regs *regs)
>  
>  void do_general_protection(struct cpu_user_regs *regs)
>  {
> +#ifdef CONFIG_PV
>      struct vcpu *v = current;
> +#endif

I think this would better be resolved by moving the declaration into
the block which has multiple references, and use plain "current" in
the subsequent "else if()". But it's a matter of taste, I agree.

Jan



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

Re: [PATCH 08/17] x86: make construct_dom0 build with !CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -509,8 +509,16 @@ int __init construct_dom0(struct domain *d, const module_t *image,
>      }
>  #endif
>  
> -    rc = (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
> -         (d, image, image_headroom, initrd, cmdline);
> +    if ( is_hvm_domain(d) )
> +        rc = dom0_construct_pvh(d, image, image_headroom, initrd, cmdline);
> +    else if ( is_pv_domain(d) )
> +        rc = dom0_construct_pv(d, image, image_headroom, initrd, cmdline);
> +    else
> +    {
> +        ASSERT_UNREACHABLE();
> +        rc = -EINVAL;
> +    }

Depending on what the plans are wrt simultaneous PV=n and HVM=n,
this may better need to be panic(). The assertion is certainly not valid
in that case - it is very much expected to get there in such a case. It
is only valid if the Kconfig change doesn't allow for that combination.
In any event I see that patch 13 doesn't change the code above
again.

Jan



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

Re: [PATCH 09/17] x86/amd: put setting pv_post_outb_hook under CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> It is used by PV code only.

And wrongly so - the same is needed for a PVH Dom0 afaict.

Jan



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

Re: [PATCH 10/17] x86: provide stub for switch_compat

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> --- a/xen/include/xen/compat.h
> +++ b/xen/include/xen/compat.h
> @@ -228,7 +228,16 @@ struct vcpu_runstate_info;
>  void xlat_vcpu_runstate_info(struct vcpu_runstate_info *);
>  
>  struct domain;
> +
> +#ifdef CONFIG_PV
>  int switch_compat(struct domain *);
> +#else
> +# include <xen/errno.h>
> +static inline int switch_compat(struct domain *d)
> +{
> +    return -EINVAL;
> +}
> +#endif

I see two options here: Either we go with th above, but with -EACCES
as the return value (to match the current one for HVM domains), or
the #ifdef goes around the case block in domctl.c. Personally I'd prefer
the latter.

Jan



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

Re: [PATCH 10/17] x86: provide stub for switch_compat

[Wei Liu]

On Fri, Oct 12, 2018 at 08:33:48AM -0600, Jan Beulich wrote:
> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> > --- a/xen/include/xen/compat.h
> > +++ b/xen/include/xen/compat.h
> > @@ -228,7 +228,16 @@ struct vcpu_runstate_info;
> >  void xlat_vcpu_runstate_info(struct vcpu_runstate_info *);
> >  
> >  struct domain;
> > +
> > +#ifdef CONFIG_PV
> >  int switch_compat(struct domain *);
> > +#else
> > +# include <xen/errno.h>
> > +static inline int switch_compat(struct domain *d)
> > +{
> > +    return -EINVAL;
> > +}
> > +#endif
> 
> I see two options here: Either we go with th above, but with -EACCES
> as the return value (to match the current one for HVM domains), or
> the #ifdef goes around the case block in domctl.c. Personally I'd prefer
> the latter.

This makes me chuckle because originally I opted for what you want here
but decided to switch to using stub before posting.

I will do that in the next version of this series.

Wei.

> 
> Jan
> 
> 

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

Re: [PATCH 11/17] x86: provide stubs for entry point

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> Instead of trying to split up entry.S and compat/entry.S, simply
> provide some stubs for them.

I'm not convinced; I'd much rather see the call sites recognizably
compiled out with !PV. I'm curious what Andrew thinks here.

Jan



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

Re: [PATCH 12/17] x86: connect guest creation with CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -322,17 +322,34 @@ 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) )
> +    if ( config )
>      {
> +        ASSERT(!is_system_domain(d));

This and the other ASSERT() are redundant with what's earlier in
the function. Do we really need them?

> +        if ( config->flags & XEN_DOMCTL_CDF_hvm_guest)
> +        {
>  #ifdef CONFIG_HVM
> -        d->guest_type = guest_type_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
> +        }
>      }
>      else
> +    {
> +        /* The type of system domain shouldn't matter. */
> +        ASSERT(is_system_domain(d));
>          d->guest_type = guest_type_pv;
> +    }

I'm afraid this comment may cause ambiguity. I think we had (and
perhaps still have) a number of places where we assume that in
particular the idle domain is a PV one. So I'd like to ask to eithr
extend the comment to explain reality, or to drop it.

Jan



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

Re: [PATCH 13/17] x86: don't create Dom0 if neither PV nor HVM is available

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> This will give us a Xen setting in idle loop. This doesn't have
> practical use except for debugging purpose.

Hmm, that's an acceptable alternative to the panic() variant, but
I'd still prefer that one. Again - let's see if Andrew has an opinion
either way.

Jan



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

Re: [PATCH 14/17] x86: don't report PV support when !CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> 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

Re: [PATCH 15/17] x86: expose CONFIG_PV

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -37,6 +37,14 @@ source "arch/Kconfig"
>  
>  config PV
>  	def_bool y
> +	prompt "PV support"
> +	---help---
> +	  Interfaces to support PV guests which don't require hardware
> +	  support like Intel's VT-x or AMD's SVM.
> +
> +	  This option is needed if you want to run PV guests.

We've had a discussion with (iirc) George and perhaps a few others
about "guest" vs "domain", and I've learned that generally the
former is meant to exclude Dom0 while the latter would include it.
In that light either s/guest/domain/ for the help text, or please add
explicit mention of Dom0 there (as PV is still the prevailing kind of
Dom0, and you wouldn't want to trip people into disabling this just
because they only run HVM guests).

Jan



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

Re: [PATCH 16/17] x86: introduce new defconfigs for PV and HVM

[Jan Beulich]

>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> They will be used by build tests.

And is it difficult for the build tests to set these up themselves?
I don't really like such additions, in particular the neither-PV-nor-
HVM one (which is otherwise completely useless). At the very
least that one should gain a comment saying it exists for build
testing only.

Jan



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

Re: [PATCH 13/17] x86: don't create Dom0 if neither PV nor HVM is available

[Andrew Cooper]

On 12/10/18 16:12, Jan Beulich wrote:
>>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
>> This will give us a Xen setting in idle loop. This doesn't have
>> practical use except for debugging purpose.
> Hmm, that's an acceptable alternative to the panic() variant, but
> I'd still prefer that one. Again - let's see if Andrew has an opinion
> either way.

I think that, for the purpose of keeping our interfaces clean, being
able to compile without PV and without HVM is a useful property
(especially as randconfig should hit it one in every 4 cases).

I've specifically needed to have no dom0 for certain debugging in the
past.  (The HPET series to fix the broken MSI handler, which I realise
still hasn't made its way upstream yet.)

I'm less certain however if implementing it like this is wise.  I
implemented it with a "nodom0" command line parameter, and left the rest
of the functionality intact.

In particular, one fuzzing idea I have is to have a tiny AFL stub on the
end of the serial port, at which point, having no dom0 but PV and HVM
fully compiled in would be the ideal position.

I'm sorry if this isn't necessarily a very helpful answer.  I'd be
tempted to drop this patch for now, and let the first person with a
concrete usecase to decide exactly how a no-dom0 system would look.

~Andrew

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

Re: [PATCH 16/17] x86: introduce new defconfigs for PV and HVM

[Wei Liu]

On Fri, Oct 12, 2018 at 09:19:45AM -0600, Jan Beulich wrote:
> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> > They will be used by build tests.
> 
> And is it difficult for the build tests to set these up themselves?

I specifically said "build tests" but not "CI systems".

It wouldn't be very difficult for a CI system to generate these files,
but my idea is that providing these in tree can make things more
convenient for humans -- developers and maintainers.

Suppose you want a contributor to test their changes, instead of writing
"first please generate the following files, and then ...", you just give
them a rune / some runes to reference these files directly. There
doesn't need explaining and no ambiguity will arise. And if there is
disagreement on what work or what doesn't, it would be easy to
reproduce.

To me this is a clear win: a small investment that comes with long term
benefit.

> I don't really like such additions, in particular the neither-PV-nor-
> HVM one (which is otherwise completely useless). At the very
> least that one should gain a comment saying it exists for build
> testing only.
> 

Sure, I can do that.

Wei.

> Jan
> 
> 

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

Re: [PATCH 16/17] x86: introduce new defconfigs for PV and HVM

[Jan Beulich]

>>> On 12.10.18 at 17:34, <wei.liu2@citrix.com> wrote:
> On Fri, Oct 12, 2018 at 09:19:45AM -0600, Jan Beulich wrote:
>> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
>> > They will be used by build tests.
>> 
>> And is it difficult for the build tests to set these up themselves?
> 
> I specifically said "build tests" but not "CI systems".
> 
> It wouldn't be very difficult for a CI system to generate these files,
> but my idea is that providing these in tree can make things more
> convenient for humans -- developers and maintainers.
> 
> Suppose you want a contributor to test their changes, instead of writing
> "first please generate the following files, and then ...", you just give
> them a rune / some runes to reference these files directly. There
> doesn't need explaining and no ambiguity will arise. And if there is
> disagreement on what work or what doesn't, it would be easy to
> reproduce.
> 
> To me this is a clear win: a small investment that comes with long term
> benefit.

Depends: At least for now I don't see it as an important goal to
avoid breaking the no-PV and no-HVM case (breaking the individual
ones is another thing). Furthermore, saying to someone "Set
HVM=n in your .config" is not a big deal at all.

What I want to avoid is that, once we gain further non-expert
options, (almost) all possible combinations of options end up as
defconfig-s. We don't have no-shadow and bigmem defconfigs,
yet I think we (should) care about not breaking those builds (I
routinely check them every once in a while).

Jan



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

Re: [PATCH 13/17] x86: don't create Dom0 if neither PV nor HVM is available

[Wei Liu]

On Fri, Oct 12, 2018 at 04:28:21PM +0100, Andrew Cooper wrote:
> On 12/10/18 16:12, Jan Beulich wrote:
> >>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> >> This will give us a Xen setting in idle loop. This doesn't have
> >> practical use except for debugging purpose.
> > Hmm, that's an acceptable alternative to the panic() variant, but
> > I'd still prefer that one. Again - let's see if Andrew has an opinion
> > either way.
> 
> I think that, for the purpose of keeping our interfaces clean, being
> able to compile without PV and without HVM is a useful property
> (especially as randconfig should hit it one in every 4 cases).
> 
> I've specifically needed to have no dom0 for certain debugging in the
> past.  (The HPET series to fix the broken MSI handler, which I realise
> still hasn't made its way upstream yet.)
> 
> I'm less certain however if implementing it like this is wise.  I
> implemented it with a "nodom0" command line parameter, and left the rest
> of the functionality intact.
> 
> In particular, one fuzzing idea I have is to have a tiny AFL stub on the
> end of the serial port, at which point, having no dom0 but PV and HVM
> fully compiled in would be the ideal position.
> 
> I'm sorry if this isn't necessarily a very helpful answer.  I'd be
> tempted to drop this patch for now, and let the first person with a
> concrete usecase to decide exactly how a no-dom0 system would look.

Sure. I don't mind dropping this patch.

Wei.

> 
> ~Andrew

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

Re: [PATCH 16/17] x86: introduce new defconfigs for PV and HVM

[Wei Liu]

On Fri, Oct 12, 2018 at 09:43:43AM -0600, Jan Beulich wrote:
> >>> On 12.10.18 at 17:34, <wei.liu2@citrix.com> wrote:
> > On Fri, Oct 12, 2018 at 09:19:45AM -0600, Jan Beulich wrote:
> >> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> >> > They will be used by build tests.
> >> 
> >> And is it difficult for the build tests to set these up themselves?
> > 
> > I specifically said "build tests" but not "CI systems".
> > 
> > It wouldn't be very difficult for a CI system to generate these files,
> > but my idea is that providing these in tree can make things more
> > convenient for humans -- developers and maintainers.
> > 
> > Suppose you want a contributor to test their changes, instead of writing
> > "first please generate the following files, and then ...", you just give
> > them a rune / some runes to reference these files directly. There
> > doesn't need explaining and no ambiguity will arise. And if there is
> > disagreement on what work or what doesn't, it would be easy to
> > reproduce.
> > 
> > To me this is a clear win: a small investment that comes with long term
> > benefit.
> 
> Depends: At least for now I don't see it as an important goal to
> avoid breaking the no-PV and no-HVM case (breaking the individual
> ones is another thing). Furthermore, saying to someone "Set
> HVM=n in your .config" is not a big deal at all.
> 
> What I want to avoid is that, once we gain further non-expert
> options, (almost) all possible combinations of options end up as
> defconfig-s. We don't have no-shadow and bigmem defconfigs,
> yet I think we (should) care about not breaking those builds (I
> routinely check them every once in a while).

I think I will just move these two configs somewhere else, say, under
automation directory, since they will be used there. We can also add the
configs you care about there.

This satisfies my requirement as well as yours.

Wei.

> 
> Jan
> 
> 

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

Re: [PATCH 11/17] x86: provide stubs for entry point

[Wei Liu]

On Fri, Oct 12, 2018 at 08:56:22AM -0600, Jan Beulich wrote:
> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> > Instead of trying to split up entry.S and compat/entry.S, simply
> > provide some stubs for them.
> 
> I'm not convinced; I'd much rather see the call sites recognizably
> compiled out with !PV. I'm curious what Andrew thinks here.
> 

Andrew?

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

Re: [PATCH 09/17] x86/amd: put setting pv_post_outb_hook under CONFIG_PV

[Roger Pau Monné]

On Fri, Oct 12, 2018 at 08:14:36AM -0600, Jan Beulich wrote:
> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> > It is used by PV code only.
> 
> And wrongly so - the same is needed for a PVH Dom0 afaict.

Yes, looking at the models affected by this issue according to
init_amd it seems to cover the full AMD line (from family 0xf to
0x17). I don't seem to find a reference in the code to the Errata, do
you know if there's some formal description of it?

Thanks, Roger.

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

Re: [PATCH 11/17] x86: provide stubs for entry point

[Andrew Cooper]

On 15/10/18 11:40, Wei Liu wrote:
> On Fri, Oct 12, 2018 at 08:56:22AM -0600, Jan Beulich wrote:
>>>>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
>>> Instead of trying to split up entry.S and compat/entry.S, simply
>>> provide some stubs for them.
>> I'm not convinced; I'd much rather see the call sites recognizably
>> compiled out with !PV. I'm curious what Andrew thinks here.
>>
> Andrew?

It would be nice to see if we can cleanly compile out bits of entry.S,
rather than stubbing out at the C level.

A few well-placed #ifdef CONFIG_PV might go a long way, but you
definitely don't want to try splitting fragments out into a separate file.

If however that proves to be very tangled, this patch is ok in the interim.

~Andrew

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

Re: [PATCH 12/17] x86: connect guest creation with CONFIG_PV

[Wei Liu]

On Fri, Oct 12, 2018 at 09:09:28AM -0600, Jan Beulich wrote:
> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -322,17 +322,34 @@ 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) )
> > +    if ( config )
> >      {
> > +        ASSERT(!is_system_domain(d));
> 
> This and the other ASSERT() are redundant with what's earlier in
> the function. Do we really need them?

No, not really.

> 
> > +        if ( config->flags & XEN_DOMCTL_CDF_hvm_guest)
> > +        {
> >  #ifdef CONFIG_HVM
> > -        d->guest_type = guest_type_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
> > +        }
> >      }
> >      else
> > +    {
> > +        /* The type of system domain shouldn't matter. */
> > +        ASSERT(is_system_domain(d));
> >          d->guest_type = guest_type_pv;
> > +    }
> 
> I'm afraid this comment may cause ambiguity. I think we had (and
> perhaps still have) a number of places where we assume that in
> particular the idle domain is a PV one. So I'd like to ask to eithr
> extend the comment to explain reality, or to drop it.

Yes the idle domain uses PV context switch routines, but that's the
extend of its PV-ness, which has nothing to do with guest interfaces.
The system still functions even if I change the type of system domains
to guest_type_foobar AFAICT.

But I think at least the idle domain should be called out explicitly. I
will change the comment to say:

   At least the idle domain should be treated as PV domain because it
   uses PV context switch functions. To err on the safe side, leave all
   system domains to be guest_type_pv.

Wei.

> 
> Jan
> 
> 

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

Re: [PATCH 15/17] x86: expose CONFIG_PV

[Wei Liu]

On Fri, Oct 12, 2018 at 09:16:42AM -0600, Jan Beulich wrote:
> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> > --- a/xen/arch/x86/Kconfig
> > +++ b/xen/arch/x86/Kconfig
> > @@ -37,6 +37,14 @@ source "arch/Kconfig"
> >  
> >  config PV
> >  	def_bool y
> > +	prompt "PV support"
> > +	---help---
> > +	  Interfaces to support PV guests which don't require hardware
> > +	  support like Intel's VT-x or AMD's SVM.
> > +
> > +	  This option is needed if you want to run PV guests.
> 
> We've had a discussion with (iirc) George and perhaps a few others
> about "guest" vs "domain", and I've learned that generally the
> former is meant to exclude Dom0 while the latter would include it.
> In that light either s/guest/domain/ for the help text, or please add
> explicit mention of Dom0 there (as PV is still the prevailing kind of
> Dom0, and you wouldn't want to trip people into disabling this just
> because they only run HVM guests).

I will change "guest" to "domain". On the same note the text for HVM
should be changed, too.

Wei.

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

Re: [PATCH 07/17] x86/traps: put PV code handlers under CONFIG_PV

[Wei Liu]

On Fri, Oct 12, 2018 at 08:08:19AM -0600, Jan Beulich wrote:
> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> > The trap handlers in hypervisor need to forward events to PV guests if
> > necessary. If there is no PV guest relevant code should be excluded.
> > 
> > Put code under CONFIG_PV and add ASSERT_UNREACHABLE.
> 
> -ETOOMANYIFDEFS
> 
> What's wrong with an inline stub for pv_inject_event()? This won't
> eliminate all #ifdef-s, but quite a few of them afaics.

Nothing wrong with that. I have provided a stub in my next version.

> 
> > @@ -1140,6 +1152,7 @@ static int handle_ldt_mapping_fault(unsigned int offset,
> >          if ( !guest_mode(regs) )
> >              return 0;
> >  
> > +#ifdef CONFIG_PV
> >          /* Access would have become non-canonical? Pass #GP[sel] back. */
> >          if ( unlikely(!is_canonical_address(curr->arch.pv.ldt_base + offset)) )
> >          {
> > @@ -1151,6 +1164,9 @@ static int handle_ldt_mapping_fault(unsigned int offset,
> >              /* else pass the #PF back, with adjusted %cr2. */
> >              pv_inject_page_fault(regs->error_code,
> >                                   curr->arch.pv.ldt_base + offset);
> > +#else
> > +        ASSERT_UNREACHABLE();
> > +#endif
> 
> I think here it should be the call site to gain #ifdef (around the
> entire if()), and the function as a whole should then be put in
> another #ifdef, or be moved to pv/traps.c.
> 

Done. Relevant code and call site are now put under CONFIG_PV. The
movement will be done later.

> > @@ -1494,7 +1514,9 @@ void __init do_early_page_fault(struct cpu_user_regs *regs)
> >  
> >  void do_general_protection(struct cpu_user_regs *regs)
> >  {
> > +#ifdef CONFIG_PV
> >      struct vcpu *v = current;
> > +#endif
> 
> I think this would better be resolved by moving the declaration into
> the block which has multiple references, and use plain "current" in
> the subsequent "else if()". But it's a matter of taste, I agree.

To me calling current more than necessary is bad because the resulting
machine code can end up calculating the address of cpu_info multiple
times.

Wei.

> 
> Jan
> 
> 

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

Re: [PATCH 09/17] x86/amd: put setting pv_post_outb_hook under CONFIG_PV

[Jan Beulich]

>>> On 15.10.18 at 13:19, <roger.pau@citrix.com> wrote:
> On Fri, Oct 12, 2018 at 08:14:36AM -0600, Jan Beulich wrote:
>> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
>> > It is used by PV code only.
>> 
>> And wrongly so - the same is needed for a PVH Dom0 afaict.
> 
> Yes, looking at the models affected by this issue according to
> init_amd it seems to cover the full AMD line (from family 0xf to
> 0x17). I don't seem to find a reference in the code to the Errata, do
> you know if there's some formal description of it?

I don't recall this having been considered an erratum.

Checking just the Fam10 and Fam15 BKDGs, I'm getting the
impression that Fam15 does not have the issue worked around
here. It might further be that not even all Fam10 steppings are
affected. Brian - any chance you could clarify this for us?

Independent of the range of systems affected I still think that
PVH Dom0 behavior here ought to be the same as PV Dom0's.

Jan



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

Re: [PATCH 09/17] x86/amd: put setting pv_post_outb_hook under CONFIG_PV

[Woods, Brian]

On Thu, Oct 25, 2018 at 03:22:17AM -0600, Jan Beulich wrote:
> >>> On 15.10.18 at 13:19, <roger.pau@citrix.com> wrote:
> > On Fri, Oct 12, 2018 at 08:14:36AM -0600, Jan Beulich wrote:
> >> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> >> > It is used by PV code only.
> >> 
> >> And wrongly so - the same is needed for a PVH Dom0 afaict.
> > 
> > Yes, looking at the models affected by this issue according to
> > init_amd it seems to cover the full AMD line (from family 0xf to
> > 0x17). I don't seem to find a reference in the code to the Errata, do
> > you know if there's some formal description of it?
> 
> I don't recall this having been considered an erratum.
> 
> Checking just the Fam10 and Fam15 BKDGs, I'm getting the
> impression that Fam15 does not have the issue worked around
> here. It might further be that not even all Fam10 steppings are
> affected. Brian - any chance you could clarify this for us?
> 
> Independent of the range of systems affected I still think that
> PVH Dom0 behavior here ought to be the same as PV Dom0's.
> 
> Jan
> 

Just an FYI, I'm looking into it.  I might require to an email to an
engineer who's always extremely busy, so it might take a little while.

-- 
Brian Woods

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

Re: [PATCH 09/17] x86/amd: put setting pv_post_outb_hook under CONFIG_PV

[Roger Pau Monné]

On Thu, Oct 25, 2018 at 03:22:17AM -0600, Jan Beulich wrote:
> >>> On 15.10.18 at 13:19, <roger.pau@citrix.com> wrote:
> > On Fri, Oct 12, 2018 at 08:14:36AM -0600, Jan Beulich wrote:
> >> >>> On 04.10.18 at 17:43, <wei.liu2@citrix.com> wrote:
> >> > It is used by PV code only.
> >> 
> >> And wrongly so - the same is needed for a PVH Dom0 afaict.
> > 
> > Yes, looking at the models affected by this issue according to
> > init_amd it seems to cover the full AMD line (from family 0xf to
> > 0x17). I don't seem to find a reference in the code to the Errata, do
> > you know if there's some formal description of it?
> 
> I don't recall this having been considered an erratum.
> 
> Checking just the Fam10 and Fam15 BKDGs, I'm getting the
> impression that Fam15 does not have the issue worked around
> here. It might further be that not even all Fam10 steppings are
> affected. Brian - any chance you could clarify this for us?
> 
> Independent of the range of systems affected I still think that
> PVH Dom0 behavior here ought to be the same as PV Dom0's.

Posted a series before going on PTO that contains a patch to add this
behavior to PVH Dom0:

https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01558.html

Regardless of the clarification on the affected models the patch
shares the detection logic between PV and PVH, so it can be easily
modified afterwards to narrow the scope of the workaround if required.

Thanks, Roger.

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