* [PATCH v2 01/16] x86: make mm.c build with !CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-26 15:57 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 02/16] x86: put some code in arch_set_info_guest under CONFIG_PV Wei Liu
` (14 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Start by putting hypercall handlers which are supposed to be PV only
under CONFIG_PV. Shuffle some code around to avoid introducing
excessive numbers of CONFIG_PV.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: rebase
---
xen/arch/x86/hvm/hypercall.c | 2 +-
xen/arch/x86/mm.c | 168 +++++++++++++++++++--------------
xen/arch/x86/x86_64/compat/mm.c | 2 +-
3 files changed, 102 insertions(+), 70 deletions(-)
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 56713d1..19d1263 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -135,7 +135,9 @@ static const hypercall_table_t hvm_hypercall_table[] = {
HYPERCALL(tmem_op),
#endif
COMPAT_CALL(platform_op),
+#ifdef CONFIG_PV
COMPAT_CALL(mmuext_op),
+#endif
HYPERCALL(xenpmu_op),
COMPAT_CALL(dm_op),
HYPERCALL(arch_1)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index c53bc86..703f330 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,38 +640,11 @@ 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);
-static int get_page_and_type_from_mfn(
- mfn_t mfn, unsigned long type, struct domain *d,
- int partial, int preemptible)
-{
- struct page_info *page = mfn_to_page(mfn);
- int rc;
-
- if ( likely(partial >= 0) &&
- unlikely(!get_page_from_mfn(mfn, d)) )
- return -EINVAL;
-
- rc = _get_page_type(page, type, preemptible);
-
- if ( unlikely(rc) && partial >= 0 &&
- (!preemptible || page != current->arch.old_guest_table) )
- put_page(page);
-
- return rc;
-}
-
-static void put_data_page(struct page_info *page, bool 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)
@@ -1128,6 +1102,27 @@ get_page_from_l1e(
return -EBUSY;
}
+#ifdef CONFIG_PV
+static int get_page_and_type_from_mfn(
+ mfn_t mfn, unsigned long type, struct domain *d,
+ int partial, int preemptible)
+{
+ struct page_info *page = mfn_to_page(mfn);
+ int rc;
+
+ if ( likely(partial >= 0) &&
+ unlikely(!get_page_from_mfn(mfn, d)) )
+ return -EINVAL;
+
+ rc = _get_page_type(page, type, preemptible);
+
+ if ( unlikely(rc) && partial >= 0 &&
+ (!preemptible || page != current->arch.old_guest_table) )
+ put_page(page);
+
+ return rc;
+}
+
define_get_linear_pagetable(l2);
static int
get_page_from_l2e(
@@ -1195,6 +1190,7 @@ get_page_from_l4e(
return rc;
}
+#endif /* CONFIG_PV */
static int _put_page_type(struct page_info *page, bool preemptible,
struct page_info *ptpg);
@@ -1275,6 +1271,14 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner)
}
}
+#ifdef CONFIG_PV
+static void put_data_page(struct page_info *page, bool writeable)
+{
+ if ( writeable )
+ put_page_and_type(page);
+ else
+ put_page(page);
+}
/*
* NB. Virtual address 'l2e' maps to a machine address within frame 'pfn'.
@@ -1621,6 +1625,7 @@ void init_xen_pae_l2_slots(l2_pgentry_t *l2t, const struct domain *d)
l2_table_offset(HIRO_COMPAT_MPT_VIRT_START)],
COMPAT_L2_PAGETABLE_XEN_SLOTS(d) * sizeof(*l2t));
}
+#endif /* CONFIG_PV */
/*
* Fill an L4 with Xen entries.
@@ -1728,6 +1733,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);
@@ -1918,6 +1924,7 @@ static int free_l4_table(struct page_info *page)
return rc;
}
+#endif /* CONFIG_PV */
#ifndef NDEBUG
/*
@@ -2002,6 +2009,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.
@@ -2313,6 +2321,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)
{
@@ -2420,6 +2429,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;
@@ -2489,12 +2499,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;
@@ -2543,6 +2558,10 @@ int free_page_type(struct page_info *page, unsigned long type,
}
return rc;
+#else
+ ASSERT_UNREACHABLE();
+ return -EINVAL;
+#endif
}
@@ -2933,6 +2952,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;
@@ -3031,48 +3051,10 @@ int new_guest_cr3(mfn_t mfn)
}
return rc;
-}
-
-static struct domain *get_pg_owner(domid_t domid)
-{
- struct domain *pg_owner = NULL, *curr = current->domain;
-
- if ( likely(domid == DOMID_SELF) )
- {
- pg_owner = rcu_lock_current_domain();
- goto out;
- }
-
- if ( unlikely(domid == curr->domain_id) )
- {
- gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
- goto out;
- }
-
- switch ( domid )
- {
- case DOMID_IO:
- pg_owner = rcu_lock_domain(dom_io);
- break;
- case DOMID_XEN:
- pg_owner = rcu_lock_domain(dom_xen);
- break;
- default:
- if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
- {
- gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
- break;
- }
- break;
- }
-
- out:
- return pg_owner;
-}
-
-static void put_pg_owner(struct domain *pg_owner)
-{
- rcu_unlock_domain(pg_owner);
+#else
+ ASSERT_UNREACHABLE();
+ return -EINVAL;
+#endif
}
static inline int vcpumask_to_pcpumask(
@@ -3117,6 +3099,49 @@ static inline int vcpumask_to_pcpumask(
}
}
+#ifdef CONFIG_PV
+static struct domain *get_pg_owner(domid_t domid)
+{
+ struct domain *pg_owner = NULL, *curr = current->domain;
+
+ if ( likely(domid == DOMID_SELF) )
+ {
+ pg_owner = rcu_lock_current_domain();
+ goto out;
+ }
+
+ if ( unlikely(domid == curr->domain_id) )
+ {
+ gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
+ goto out;
+ }
+
+ switch ( domid )
+ {
+ case DOMID_IO:
+ pg_owner = rcu_lock_domain(dom_io);
+ break;
+ case DOMID_XEN:
+ pg_owner = rcu_lock_domain(dom_xen);
+ break;
+ default:
+ if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
+ {
+ gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
+ break;
+ }
+ break;
+ }
+
+ out:
+ return pg_owner;
+}
+
+static void put_pg_owner(struct domain *pg_owner)
+{
+ rcu_unlock_domain(pg_owner);
+}
+
long do_mmuext_op(
XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
unsigned int count,
@@ -3973,6 +3998,7 @@ long do_mmu_update(
return rc;
}
+#endif /* CONFIG_PV */
int donate_page(
struct domain *d, struct page_info *page, unsigned int memflags)
@@ -4080,6 +4106,7 @@ 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)
{
@@ -4242,6 +4269,7 @@ 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);
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index 02bc75b..32410ed 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -163,6 +163,7 @@ int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
return rc;
}
+#ifdef CONFIG_PV
DEFINE_XEN_GUEST_HANDLE(mmuext_op_compat_t);
int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) arg,
@@ -313,6 +314,7 @@ int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) arg,
return rc;
}
+#endif /* CONFIG_PV */
/*
* Local variables:
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v2 02/16] x86: put some code in arch_set_info_guest under CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
2018-10-19 14:28 ` [PATCH v2 01/16] x86: make mm.c build with !CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-26 15:58 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 03/16] x86: make traps.c build with !CONFIG_PV Wei Liu
` (13 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
This function is called by both PV and HVM. Unfortunately the code is
very convoluted. We can reason that code between the call to
hvm_set_info_guest and out label is PV only. Put that portion under
CONFIG_PV.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/domain.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 9371efc..c931377 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -777,11 +777,15 @@ int arch_set_info_guest(
struct vcpu *v, vcpu_guest_context_u c)
{
struct domain *d = v->domain;
+ unsigned int i;
+ unsigned long flags;
+ bool compat;
+#ifdef CONFIG_PV
unsigned long cr3_gfn;
struct page_info *cr3_page;
- unsigned long flags, cr4;
- unsigned int i;
- int rc = 0, compat;
+ unsigned long cr4;
+ int rc = 0;
+#endif
/* The context is a compat-mode one if the target domain is compat-mode;
* we expect the tools to DTRT even in compat-mode callers. */
@@ -875,6 +879,7 @@ int arch_set_info_guest(
goto out;
}
+#ifdef CONFIG_PV
/* IOPL privileges are virtualised. */
v->arch.pv.iopl = v->arch.user_regs.eflags & X86_EFLAGS_IOPL;
v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
@@ -1140,6 +1145,7 @@ int arch_set_info_guest(
paging_update_paging_modes(v);
update_cr3(v);
+#endif /* CONFIG_PV */
out:
if ( flags & VGCF_online )
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v2 03/16] x86: make traps.c build with !CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
2018-10-19 14:28 ` [PATCH v2 01/16] x86: make mm.c build with !CONFIG_PV Wei Liu
2018-10-19 14:28 ` [PATCH v2 02/16] x86: put some code in arch_set_info_guest under CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-26 16:02 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 04/16] x86: make construct_dom0 " Wei Liu
` (12 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Provide a stub for pv_inject_event, put code that accesses PV fields
and GDT / LDT fault handling code under CONFIG_PV.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: reduce the amount of ifdefs
---
xen/arch/x86/traps.c | 23 +++++++++++++++++++++++
xen/include/asm-x86/domain.h | 7 +++++++
2 files changed, 30 insertions(+)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 3988753..002e98f 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1115,6 +1115,7 @@ static void reserved_bit_page_fault(unsigned long addr,
show_execution_state(regs);
}
+#ifdef CONFIG_PV
static int handle_ldt_mapping_fault(unsigned int offset,
struct cpu_user_regs *regs)
{
@@ -1185,6 +1186,7 @@ static int handle_gdt_ldt_mapping_fault(unsigned long offset,
return EXCRET_fault_fixed;
}
+#endif
#define IN_HYPERVISOR_RANGE(va) \
(((va) >= HYPERVISOR_VIRT_START) && ((va) < HYPERVISOR_VIRT_END))
@@ -1337,8 +1339,15 @@ static int fixup_page_fault(unsigned long addr, struct cpu_user_regs *regs)
{
if ( !(regs->error_code & (PFEC_user_mode | PFEC_reserved_bit)) &&
(addr >= GDT_LDT_VIRT_START) && (addr < GDT_LDT_VIRT_END) )
+ {
+#ifdef CONFIG_PV
return handle_gdt_ldt_mapping_fault(
addr - GDT_LDT_VIRT_START, regs);
+#else
+ ASSERT_UNREACHABLE();
+ return 0;
+#endif
+ }
return 0;
}
@@ -1494,7 +1503,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 +1517,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.
*
@@ -1557,6 +1569,7 @@ void do_general_protection(struct cpu_user_regs *regs)
/* Pass on GPF as is. */
pv_inject_hw_exception(TRAP_gp_fault, regs->error_code);
return;
+#endif
gp_in_kernel:
@@ -1744,7 +1757,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 +1777,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 +1787,9 @@ void do_device_not_available(struct cpu_user_regs *regs)
}
else
TRACE_0D(TRC_PV_MATH_STATE_RESTORE);
+#else
+ ASSERT_UNREACHABLE();
+#endif
return;
}
@@ -2069,13 +2088,16 @@ void activate_debugregs(const struct vcpu *curr)
if ( boot_cpu_has(X86_FEATURE_DBEXT) )
{
+#ifdef CONFIG_PV
wrmsrl(MSR_AMD64_DR0_ADDRESS_MASK, curr->arch.pv.dr_mask[0]);
wrmsrl(MSR_AMD64_DR1_ADDRESS_MASK, curr->arch.pv.dr_mask[1]);
wrmsrl(MSR_AMD64_DR2_ADDRESS_MASK, curr->arch.pv.dr_mask[2]);
wrmsrl(MSR_AMD64_DR3_ADDRESS_MASK, curr->arch.pv.dr_mask[3]);
+#endif
}
}
+#ifdef CONFIG_PV
/*
* Used by hypercalls and the emulator.
* -ENODEV => #UD
@@ -2190,6 +2212,7 @@ long set_debugreg(struct vcpu *v, unsigned int reg, unsigned long value)
v->arch.debugreg[reg] = value;
return 0;
}
+#endif /* CONFIG_PV */
void asm_domain_crash_synchronous(unsigned long addr)
{
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index e7b8227..b94b2c8 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -672,7 +672,14 @@ static inline void free_vcpu_guest_context(struct vcpu_guest_context *vgc)
struct vcpu_hvm_context;
int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context *ctx);
+#ifdef CONFIG_PV
void pv_inject_event(const struct x86_event *event);
+#else
+static inline void pv_inject_event(const struct x86_event *event)
+{
+ ASSERT_UNREACHABLE();
+}
+#endif
static inline void pv_inject_hw_exception(unsigned int vector, int errcode)
{
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 03/16] x86: make traps.c build with !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 03/16] x86: make traps.c build with !CONFIG_PV Wei Liu
@ 2018-10-26 16:02 ` Jan Beulich
2018-10-30 17:48 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-26 16:02 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> @@ -1337,8 +1339,15 @@ static int fixup_page_fault(unsigned long addr, struct cpu_user_regs *regs)
> {
> if ( !(regs->error_code & (PFEC_user_mode | PFEC_reserved_bit)) &&
> (addr >= GDT_LDT_VIRT_START) && (addr < GDT_LDT_VIRT_END) )
> + {
> +#ifdef CONFIG_PV
> return handle_gdt_ldt_mapping_fault(
> addr - GDT_LDT_VIRT_START, regs);
> +#else
> + ASSERT_UNREACHABLE();
I'm not convinced: A buggy access to this range should lead to
an unhandled #PF, not an assertion failure.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 03/16] x86: make traps.c build with !CONFIG_PV
2018-10-26 16:02 ` Jan Beulich
@ 2018-10-30 17:48 ` Wei Liu
0 siblings, 0 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-30 17:48 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Fri, Oct 26, 2018 at 10:02:19AM -0600, Jan Beulich wrote:
> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> > @@ -1337,8 +1339,15 @@ static int fixup_page_fault(unsigned long addr, struct cpu_user_regs *regs)
> > {
> > if ( !(regs->error_code & (PFEC_user_mode | PFEC_reserved_bit)) &&
> > (addr >= GDT_LDT_VIRT_START) && (addr < GDT_LDT_VIRT_END) )
> > + {
> > +#ifdef CONFIG_PV
> > return handle_gdt_ldt_mapping_fault(
> > addr - GDT_LDT_VIRT_START, regs);
> > +#else
> > + ASSERT_UNREACHABLE();
>
> I'm not convinced: A buggy access to this range should lead to
> an unhandled #PF, not an assertion failure.
OK. In that case ASSERT_UNREACHABLE is not needed here. Since this whole
snippet ends up returning 0 anyway I will put the inner if statement
under CONFIG_PV.
Wei.
>
> Jan
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 04/16] x86: make construct_dom0 build with !CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (2 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 03/16] x86: make traps.c build with !CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:04 ` Andrew Cooper
2018-10-19 14:28 ` [PATCH v2 05/16] x86/pv: make guest_io_{read, write} local functions Wei Liu
` (11 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: panic instead.
---
xen/arch/x86/dom0_build.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index dcd7afb..0f0977d 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -510,8 +510,13 @@ 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
+ panic("Cannot construct Dom0. No guest interface available.");
+
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
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 04/16] x86: make construct_dom0 build with !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 04/16] x86: make construct_dom0 " Wei Liu
@ 2018-10-19 15:04 ` Andrew Cooper
0 siblings, 0 replies; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 15:04 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Jan Beulich
On 19/10/18 15:28, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2: panic instead.
> ---
> xen/arch/x86/dom0_build.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index dcd7afb..0f0977d 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -510,8 +510,13 @@ 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
> + panic("Cannot construct Dom0. No guest interface available.");
\n and you can drop the final full stop.
With that, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> +
> if ( rc )
> return rc;
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 05/16] x86/pv: make guest_io_{read, write} local functions
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (3 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 04/16] x86: make construct_dom0 " Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:10 ` Andrew Cooper
2018-10-19 14:28 ` [PATCH v2 06/16] x86/amd: call post outb hook for both PV and HVM Wei Liu
` (10 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/pv/emul-priv-op.c | 8 ++++----
xen/include/asm-x86/traps.h | 5 -----
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 6422f91..b85c65f 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -214,8 +214,8 @@ static bool pci_cfg_ok(struct domain *currd, unsigned int start,
pci_conf_write_intercept(0, machine_bdf, start, size, write) >= 0;
}
-uint32_t guest_io_read(unsigned int port, unsigned int bytes,
- struct domain *currd)
+static uint32_t guest_io_read(unsigned int port, unsigned int bytes,
+ struct domain *currd)
{
uint32_t data = 0;
unsigned int shift = 0;
@@ -343,8 +343,8 @@ static int read_io(unsigned int port, unsigned int bytes,
return X86EMUL_OKAY;
}
-void guest_io_write(unsigned int port, unsigned int bytes, uint32_t data,
- struct domain *currd)
+static void guest_io_write(unsigned int port, unsigned int bytes,
+ uint32_t data, struct domain *currd)
{
if ( admin_io_okay(port, bytes, currd) )
{
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index bed2529..b88f2a4 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -21,11 +21,6 @@
void async_exception_cleanup(struct vcpu *);
-uint32_t guest_io_read(unsigned int port, unsigned int bytes,
- struct domain *);
-void guest_io_write(unsigned int port, unsigned int bytes, uint32_t data,
- struct domain *);
-
const char *trapstr(unsigned int trapnr);
#endif /* ASM_TRAP_H */
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v2 06/16] x86/amd: call post outb hook for both PV and HVM
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (4 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 05/16] x86/pv: make guest_io_{read, write} local functions Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:55 ` Wei Liu
2018-10-29 14:00 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV Wei Liu
` (9 subsequent siblings)
15 siblings, 2 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Wei Liu, Jan Beulich
The issue described in 10d03342912 is applicable to both PV and HVM
domains that have access to that SMI CMD port.
Move the hook to AMD code, remove its pv_ prefix and call it in both
PV and HVM code.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: rewritten
---
xen/arch/x86/cpu/amd.c | 4 +++-
xen/arch/x86/hvm/io.c | 2 ++
xen/arch/x86/pv/emul-priv-op.c | 10 ++++------
xen/include/asm-x86/io.h | 2 +-
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c394c1c..9ab5e84 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -44,6 +44,8 @@ integer_param("cpuid_mask_thermal_ecx", opt_cpuid_mask_thermal_ecx);
s8 __read_mostly opt_allow_unsafe;
boolean_param("allow_unsafe", opt_allow_unsafe);
+void (*post_outb_hook)(unsigned int port, uint8_t value);
+
static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
unsigned int *hi)
{
@@ -628,7 +630,7 @@ static void init_amd(struct cpuinfo_x86 *c)
case 0xf ... 0x17:
disable_c1e(NULL);
if (acpi_smi_cmd && (acpi_enable_value | acpi_disable_value))
- pv_post_outb_hook = check_disable_c1e;
+ post_outb_hook = check_disable_c1e;
break;
}
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index a5b0a23..896874d 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -234,6 +234,8 @@ static int g2m_portio_write(const struct hvm_io_handler *handler,
{
case 1:
outb(data, mport);
+ if ( post_outb_hook )
+ post_outb_hook(mport, (uint8_t)data);
break;
case 2:
outw(data, mport);
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index b85c65f..e99cbcc 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -59,8 +59,6 @@ struct priv_op_ctxt {
void host_to_guest_gpr_switch(struct cpu_user_regs *);
unsigned long guest_to_host_gpr_switch(unsigned long);
-void (*pv_post_outb_hook)(unsigned int port, u8 value);
-
typedef void io_emul_stub_t(struct cpu_user_regs *);
static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt *ctxt, u8 opcode,
@@ -352,8 +350,8 @@ static void guest_io_write(unsigned int port, unsigned int bytes,
{
case 1:
outb((uint8_t)data, port);
- if ( pv_post_outb_hook )
- pv_post_outb_hook(port, (uint8_t)data);
+ if ( post_outb_hook )
+ post_outb_hook(port, (uint8_t)data);
break;
case 2:
outw((uint16_t)data, port);
@@ -433,8 +431,8 @@ static int write_io(unsigned int port, unsigned int bytes,
io_emul_stub_setup(poc, ctxt->opcode, port, bytes);
io_emul(ctxt->regs);
- if ( (bytes == 1) && pv_post_outb_hook )
- pv_post_outb_hook(port, val);
+ if ( (bytes == 1) && post_outb_hook )
+ post_outb_hook(port, val);
return X86EMUL_DONE;
}
diff --git a/xen/include/asm-x86/io.h b/xen/include/asm-x86/io.h
index 4d2064e..0326e9d 100644
--- a/xen/include/asm-x86/io.h
+++ b/xen/include/asm-x86/io.h
@@ -48,7 +48,7 @@ __OUT(b,"b",char)
__OUT(w,"w",short)
__OUT(l,,int)
-extern void (*pv_post_outb_hook)(unsigned int port, u8 value);
+extern void (*post_outb_hook)(unsigned int port, u8 value);
/* Function pointer used to handle platform specific I/O port emulation. */
#define IOEMUL_QUIRK_STUB_BYTES 10
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 06/16] x86/amd: call post outb hook for both PV and HVM
2018-10-19 14:28 ` [PATCH v2 06/16] x86/amd: call post outb hook for both PV and HVM Wei Liu
@ 2018-10-19 15:55 ` Wei Liu
2018-10-29 14:00 ` Jan Beulich
1 sibling, 0 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-19 15:55 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Wei Liu, Jan Beulich
On Fri, Oct 19, 2018 at 03:28:32PM +0100, Wei Liu wrote:
> The issue described in 10d03342912 is applicable to both PV and HVM
> domains that have access to that SMI CMD port.
>
> Move the hook to AMD code, remove its pv_ prefix and call it in both
> PV and HVM code.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Ignore this patch. Roger has posted another patch to fix this.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 06/16] x86/amd: call post outb hook for both PV and HVM
2018-10-19 14:28 ` [PATCH v2 06/16] x86/amd: call post outb hook for both PV and HVM Wei Liu
2018-10-19 15:55 ` Wei Liu
@ 2018-10-29 14:00 ` Jan Beulich
1 sibling, 0 replies; 61+ messages in thread
From: Jan Beulich @ 2018-10-29 14:00 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, Paul Durrant, xen-devel
>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -234,6 +234,8 @@ static int g2m_portio_write(const struct hvm_io_handler *handler,
> {
> case 1:
> outb(data, mport);
> + if ( post_outb_hook )
> + post_outb_hook(mport, (uint8_t)data);
Pointless cast. I notice ...
> @@ -352,8 +350,8 @@ static void guest_io_write(unsigned int port, unsigned int bytes,
> {
> case 1:
> outb((uint8_t)data, port);
> - if ( pv_post_outb_hook )
> - pv_post_outb_hook(port, (uint8_t)data);
> + if ( post_outb_hook )
> + post_outb_hook(port, (uint8_t)data);
... one has been present here (which should be dropped instead)
and ...
> @@ -433,8 +431,8 @@ static int write_io(unsigned int port, unsigned int bytes,
> io_emul_stub_setup(poc, ctxt->opcode, port, bytes);
>
> io_emul(ctxt->regs);
> - if ( (bytes == 1) && pv_post_outb_hook )
> - pv_post_outb_hook(port, val);
> + if ( (bytes == 1) && post_outb_hook )
> + post_outb_hook(port, val);
... none has been present here.
However, looking at Roger's approach in "amd/pvh: enable ACPI C1E
disable quirk on PVH Dom0" I wonder if execution would ever make it
to g2m_portio_write() for Dom0 - g2m_ioport_list gets added to from
a domctl only afaics.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (5 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 06/16] x86/amd: call post outb hook for both PV and HVM Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:18 ` Andrew Cooper
2018-10-29 14:28 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 08/16] x86: connect guest creation with CONFIG_PV Wei Liu
` (8 subsequent siblings)
15 siblings, 2 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
They are useful to PV only.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/domctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 115ddf6..8ec0b9b 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -608,6 +608,7 @@ long arch_do_domctl(
copyback = true;
break;
+#ifdef CONFIG_PV
case XEN_DOMCTL_set_address_size:
if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
@@ -623,6 +624,7 @@ long arch_do_domctl(
BITS_PER_LONG;
copyback = true;
break;
+#endif
case XEN_DOMCTL_set_machine_address_size:
if ( d->tot_pages > 0 )
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV Wei Liu
@ 2018-10-19 15:18 ` Andrew Cooper
2018-10-30 21:08 ` Wei Liu
2018-10-29 14:28 ` Jan Beulich
1 sibling, 1 reply; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 15:18 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Jan Beulich
On 19/10/18 15:28, Wei Liu wrote:
> They are useful to PV only.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
How easy is this to turn into a domaincreate flag? It is yet another
singleton operation which complicated Xen's init/cleanup path having to
cope with it being enabled after the fact.
(This is a slightly loaded question. I have a suspicion that the answer
is "rather complicated to make the associated libxl changes", as you
need to know the bitness of the PV kernel before calling domaincreate.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-19 15:18 ` Andrew Cooper
@ 2018-10-30 21:08 ` Wei Liu
0 siblings, 0 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-30 21:08 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Wei Liu, Jan Beulich
On Fri, Oct 19, 2018 at 04:18:21PM +0100, Andrew Cooper wrote:
> On 19/10/18 15:28, Wei Liu wrote:
> > They are useful to PV only.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>
> How easy is this to turn into a domaincreate flag? It is yet another
> singleton operation which complicated Xen's init/cleanup path having to
> cope with it being enabled after the fact.
>
> (This is a slightly loaded question. I have a suspicion that the answer
> is "rather complicated to make the associated libxl changes", as you
> need to know the bitness of the PV kernel before calling domaincreate.
It appears that by the time the set hypercall is called toolstack should
have already known the bitness of the kernel, so it is possible to turn
the set hypercall into a flag (but still non-trivial). Getting rid of
the get hypercall is more complicated because the bitness is not
recorded anywhere in userspace. Parsing the kernel again is not an
option.
Wei.
>
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV Wei Liu
2018-10-19 15:18 ` Andrew Cooper
@ 2018-10-29 14:28 ` Jan Beulich
2018-10-30 20:50 ` Wei Liu
1 sibling, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-29 14:28 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> They are useful to PV only.
Considering there was no is_{hvm,pv}_...() check so far, I think
you need to extend this a little plus ...
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -608,6 +608,7 @@ long arch_do_domctl(
> copyback = true;
> break;
>
> +#ifdef CONFIG_PV
> case XEN_DOMCTL_set_address_size:
> if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
> ((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
> @@ -623,6 +624,7 @@ long arch_do_domctl(
>
> BITS_PER_LONG;
> copyback = true;
> break;
> +#endif
... add such a check so that similar behavior will result with PV
enabled and disabled (error codes may differ, but success vs
error ought to match).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-29 14:28 ` Jan Beulich
@ 2018-10-30 20:50 ` Wei Liu
2018-10-31 9:00 ` Jan Beulich
0 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-30 20:50 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Mon, Oct 29, 2018 at 08:28:07AM -0600, Jan Beulich wrote:
> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> > They are useful to PV only.
>
> Considering there was no is_{hvm,pv}_...() check so far, I think
> you need to extend this a little plus ...
I thought the code was self-explanatory enough.
Anyway, I will update the commit message to the following:
Going through the toolstack code, this set of hypercalls is only called
for PV guests on x86.
>
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -608,6 +608,7 @@ long arch_do_domctl(
> > copyback = true;
> > break;
> >
> > +#ifdef CONFIG_PV
> > case XEN_DOMCTL_set_address_size:
> > if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
> > ((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
> > @@ -623,6 +624,7 @@ long arch_do_domctl(
> >
> > BITS_PER_LONG;
> > copyback = true;
> > break;
> > +#endif
>
> ... add such a check so that similar behavior will result with PV
> enabled and disabled (error codes may differ, but success vs
> error ought to match).
I don't follow. Do you mean adding is_pv_domain check somewhere?
Wei.
>
> Jan
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-30 20:50 ` Wei Liu
@ 2018-10-31 9:00 ` Jan Beulich
2018-10-31 9:33 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-31 9:00 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 30.10.18 at 21:50, <wei.liu2@citrix.com> wrote:
> On Mon, Oct 29, 2018 at 08:28:07AM -0600, Jan Beulich wrote:
>> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
>> > --- a/xen/arch/x86/domctl.c
>> > +++ b/xen/arch/x86/domctl.c
>> > @@ -608,6 +608,7 @@ long arch_do_domctl(
>> > copyback = true;
>> > break;
>> >
>> > +#ifdef CONFIG_PV
>> > case XEN_DOMCTL_set_address_size:
>> > if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
>> > ((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
>> > @@ -623,6 +624,7 @@ long arch_do_domctl(
>> >
>> > BITS_PER_LONG;
>> > copyback = true;
>> > break;
>> > +#endif
>>
>> ... add such a check so that similar behavior will result with PV
>> enabled and disabled (error codes may differ, but success vs
>> error ought to match).
>
> I don't follow. Do you mean adding is_pv_domain check somewhere?
Yes. Otherwise behavior differs between a PV and a !PV hypervisor.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-31 9:00 ` Jan Beulich
@ 2018-10-31 9:33 ` Wei Liu
2018-10-31 9:47 ` Jan Beulich
0 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-31 9:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Wed, Oct 31, 2018 at 03:00:17AM -0600, Jan Beulich wrote:
> >>> On 30.10.18 at 21:50, <wei.liu2@citrix.com> wrote:
> > On Mon, Oct 29, 2018 at 08:28:07AM -0600, Jan Beulich wrote:
> >> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> >> > --- a/xen/arch/x86/domctl.c
> >> > +++ b/xen/arch/x86/domctl.c
> >> > @@ -608,6 +608,7 @@ long arch_do_domctl(
> >> > copyback = true;
> >> > break;
> >> >
> >> > +#ifdef CONFIG_PV
> >> > case XEN_DOMCTL_set_address_size:
> >> > if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
> >> > ((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
> >> > @@ -623,6 +624,7 @@ long arch_do_domctl(
> >> >
> >> > BITS_PER_LONG;
> >> > copyback = true;
> >> > break;
> >> > +#endif
> >>
> >> ... add such a check so that similar behavior will result with PV
> >> enabled and disabled (error codes may differ, but success vs
> >> error ought to match).
> >
> > I don't follow. Do you mean adding is_pv_domain check somewhere?
>
> Yes. Otherwise behavior differs between a PV and a !PV hypervisor.
I'm still at a loss to figure out what "similar behavior" you want. When
you say behaviour differs, do you mean the lack of copyback in my patch?
Why would that be relevant to a ENOSYS hypercall?
Wei.
>
> Jan
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-31 9:33 ` Wei Liu
@ 2018-10-31 9:47 ` Jan Beulich
2018-10-31 9:54 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-31 9:47 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 31.10.18 at 10:33, <wei.liu2@citrix.com> wrote:
> On Wed, Oct 31, 2018 at 03:00:17AM -0600, Jan Beulich wrote:
>> >>> On 30.10.18 at 21:50, <wei.liu2@citrix.com> wrote:
>> > On Mon, Oct 29, 2018 at 08:28:07AM -0600, Jan Beulich wrote:
>> >> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
>> >> > --- a/xen/arch/x86/domctl.c
>> >> > +++ b/xen/arch/x86/domctl.c
>> >> > @@ -608,6 +608,7 @@ long arch_do_domctl(
>> >> > copyback = true;
>> >> > break;
>> >> >
>> >> > +#ifdef CONFIG_PV
>> >> > case XEN_DOMCTL_set_address_size:
>> >> > if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
>> >> > ((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
>> >> > @@ -623,6 +624,7 @@ long arch_do_domctl(
>> >> >
>> >> > BITS_PER_LONG;
>> >> > copyback = true;
>> >> > break;
>> >> > +#endif
>> >>
>> >> ... add such a check so that similar behavior will result with PV
>> >> enabled and disabled (error codes may differ, but success vs
>> >> error ought to match).
>> >
>> > I don't follow. Do you mean adding is_pv_domain check somewhere?
>>
>> Yes. Otherwise behavior differs between a PV and a !PV hypervisor.
>
> I'm still at a loss to figure out what "similar behavior" you want. When
> you say behaviour differs, do you mean the lack of copyback in my patch?
> Why would that be relevant to a ENOSYS hypercall?
With !PV you'd get -ENOSYS (which is wrong in its own right, but
that a different topic). With PV you'd not get any error under at
least some conditions even for a HVM domain (if you don't make it
into switch_compat()), afaict.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-31 9:47 ` Jan Beulich
@ 2018-10-31 9:54 ` Wei Liu
2018-10-31 10:12 ` Jan Beulich
0 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-31 9:54 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Wed, Oct 31, 2018 at 03:47:25AM -0600, Jan Beulich wrote:
> >>> On 31.10.18 at 10:33, <wei.liu2@citrix.com> wrote:
> > On Wed, Oct 31, 2018 at 03:00:17AM -0600, Jan Beulich wrote:
> >> >>> On 30.10.18 at 21:50, <wei.liu2@citrix.com> wrote:
> >> > On Mon, Oct 29, 2018 at 08:28:07AM -0600, Jan Beulich wrote:
> >> >> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> >> >> > --- a/xen/arch/x86/domctl.c
> >> >> > +++ b/xen/arch/x86/domctl.c
> >> >> > @@ -608,6 +608,7 @@ long arch_do_domctl(
> >> >> > copyback = true;
> >> >> > break;
> >> >> >
> >> >> > +#ifdef CONFIG_PV
> >> >> > case XEN_DOMCTL_set_address_size:
> >> >> > if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
> >> >> > ((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
> >> >> > @@ -623,6 +624,7 @@ long arch_do_domctl(
> >> >> >
> >> >> > BITS_PER_LONG;
> >> >> > copyback = true;
> >> >> > break;
> >> >> > +#endif
> >> >>
> >> >> ... add such a check so that similar behavior will result with PV
> >> >> enabled and disabled (error codes may differ, but success vs
> >> >> error ought to match).
> >> >
> >> > I don't follow. Do you mean adding is_pv_domain check somewhere?
> >>
> >> Yes. Otherwise behavior differs between a PV and a !PV hypervisor.
> >
> > I'm still at a loss to figure out what "similar behavior" you want. When
> > you say behaviour differs, do you mean the lack of copyback in my patch?
> > Why would that be relevant to a ENOSYS hypercall?
>
> With !PV you'd get -ENOSYS (which is wrong in its own right, but
> that a different topic). With PV you'd not get any error under at
> least some conditions even for a HVM domain (if you don't make it
> into switch_compat()), afaict.
Right, so you actually wanted me to fix two issues.
One is to change it to not return -ENOSYS. I think you would want
-EOPNOTSUPP instead?
The other is to tighten the hypercalls to be PV only. I think for HVM
guests they should return -EINVAL.
So instead of putting the pair under CONFIG_PV, I should be adding some
checks to them individually to do what we agree on later. Does this make
sense?
Wei.
>
> Jan
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV
2018-10-31 9:54 ` Wei Liu
@ 2018-10-31 10:12 ` Jan Beulich
0 siblings, 0 replies; 61+ messages in thread
From: Jan Beulich @ 2018-10-31 10:12 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 31.10.18 at 10:54, <wei.liu2@citrix.com> wrote:
> On Wed, Oct 31, 2018 at 03:47:25AM -0600, Jan Beulich wrote:
>> >>> On 31.10.18 at 10:33, <wei.liu2@citrix.com> wrote:
>> > On Wed, Oct 31, 2018 at 03:00:17AM -0600, Jan Beulich wrote:
>> >> >>> On 30.10.18 at 21:50, <wei.liu2@citrix.com> wrote:
>> >> > On Mon, Oct 29, 2018 at 08:28:07AM -0600, Jan Beulich wrote:
>> >> >> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
>> >> >> > --- a/xen/arch/x86/domctl.c
>> >> >> > +++ b/xen/arch/x86/domctl.c
>> >> >> > @@ -608,6 +608,7 @@ long arch_do_domctl(
>> >> >> > copyback = true;
>> >> >> > break;
>> >> >> >
>> >> >> > +#ifdef CONFIG_PV
>> >> >> > case XEN_DOMCTL_set_address_size:
>> >> >> > if ( ((domctl->u.address_size.size == 64) && !d->arch.is_32bit_pv) ||
>> >> >> > ((domctl->u.address_size.size == 32) && d->arch.is_32bit_pv) )
>> >> >> > @@ -623,6 +624,7 @@ long arch_do_domctl(
>> >> >> >
>> >> >> > BITS_PER_LONG;
>> >> >> > copyback = true;
>> >> >> > break;
>> >> >> > +#endif
>> >> >>
>> >> >> ... add such a check so that similar behavior will result with PV
>> >> >> enabled and disabled (error codes may differ, but success vs
>> >> >> error ought to match).
>> >> >
>> >> > I don't follow. Do you mean adding is_pv_domain check somewhere?
>> >>
>> >> Yes. Otherwise behavior differs between a PV and a !PV hypervisor.
>> >
>> > I'm still at a loss to figure out what "similar behavior" you want. When
>> > you say behaviour differs, do you mean the lack of copyback in my patch?
>> > Why would that be relevant to a ENOSYS hypercall?
>>
>> With !PV you'd get -ENOSYS (which is wrong in its own right, but
>> that a different topic). With PV you'd not get any error under at
>> least some conditions even for a HVM domain (if you don't make it
>> into switch_compat()), afaict.
>
> Right, so you actually wanted me to fix two issues.
>
> One is to change it to not return -ENOSYS. I think you would want
> -EOPNOTSUPP instead?
We've had that discussion in the past, and there was no consensus
reached as to changing pre-existing (but wrong) error codes. So no,
this wasn't part of what I've been asking for.
> The other is to tighten the hypercalls to be PV only. I think for HVM
> guests they should return -EINVAL.
Or indeed -EOPNOTSUPP, seeing that Arm does support them
despite not supporting PV guests.
> So instead of putting the pair under CONFIG_PV, I should be adding some
> checks to them individually to do what we agree on later. Does this make
> sense?
Yes.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 08/16] x86: connect guest creation with CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (6 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 07/16] x86: put XEN_DOMCTL_{set, get}_address_size under CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:36 ` Andrew Cooper
2018-10-19 14:28 ` [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV Wei Liu
` (7 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Jan Beulich
This is a bit more complicated than the HVM case because system
domains have PV guest type. Leave them like that.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2:
1. Remove useless ASSERTs.
2. Rewrite comment.
---
xen/common/domain.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 65151e2..fc09088 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -322,17 +322,36 @@ 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 )
{
+ 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
+ {
+ /*
+ * 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.
+ */
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
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 08/16] x86: connect guest creation with CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 08/16] x86: connect guest creation with CONFIG_PV Wei Liu
@ 2018-10-19 15:36 ` Andrew Cooper
0 siblings, 0 replies; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 15:36 UTC (permalink / raw)
To: Wei Liu, xen-devel
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
Tim Deegan, Ian Jackson, Julien Grall, Jan Beulich
On 19/10/18 15:28, Wei Liu wrote:
> This is a bit more complicated than the HVM case because system
> domains have PV guest type. Leave them like that.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2:
> 1. Remove useless ASSERTs.
> 2. Rewrite comment.
> ---
> xen/common/domain.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 65151e2..fc09088 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -322,17 +322,36 @@ 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 )
> {
> + if ( config->flags & XEN_DOMCTL_CDF_hvm_guest)
Space at the end.
> + {
> #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;
Indentation to match the HVM case?
Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (7 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 08/16] x86: connect guest creation with CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:59 ` Andrew Cooper
2018-10-29 14:33 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 10/16] x86: don't setup legacy syscall vector " Wei Liu
` (6 subsequent siblings)
15 siblings, 2 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/x86_64/traps.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 27154f2..2b7ec7d 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -267,6 +267,7 @@ void do_double_fault(struct cpu_user_regs *regs)
panic("DOUBLE FAULT -- system shutdown\n");
}
+#ifdef CONFIG_PV
static unsigned int write_stub_trampoline(
unsigned char *stub, unsigned long stub_va,
unsigned long stack_bottom, unsigned long target_va)
@@ -296,6 +297,7 @@ static unsigned int write_stub_trampoline(
/* Round up to a multiple of 16 bytes. */
return 32;
}
+#endif
DEFINE_PER_CPU(struct stubs, stubs);
void lstar_enter(void);
@@ -303,14 +305,17 @@ void cstar_enter(void);
void subarch_percpu_traps_init(void)
{
+#ifdef CONFIG_PV
unsigned long stack_bottom = get_stack_bottom();
unsigned long stub_va = this_cpu(stubs.addr);
unsigned char *stub_page;
unsigned int offset;
+#endif
/* IST_MAX IST pages + at least 1 guard page + primary stack. */
BUILD_BUG_ON((IST_MAX + 1) * PAGE_SIZE + PRIMARY_STACK_SIZE > STACK_SIZE);
+#ifdef CONFIG_PV
stub_page = map_domain_page(_mfn(this_cpu(stubs.mfn)));
/*
@@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
/* Common SYSCALL parameters. */
wrmsrl(MSR_STAR, XEN_MSR_STAR);
wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
+#endif
}
void hypercall_page_initialise(struct domain *d, void *hypercall_page)
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV Wei Liu
@ 2018-10-19 15:59 ` Andrew Cooper
2018-10-29 14:37 ` Jan Beulich
2018-10-29 14:33 ` Jan Beulich
1 sibling, 1 reply; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 15:59 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Jan Beulich
On 19/10/18 15:28, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2: new
> ---
> xen/arch/x86/x86_64/traps.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 27154f2..2b7ec7d 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -267,6 +267,7 @@ void do_double_fault(struct cpu_user_regs *regs)
> panic("DOUBLE FAULT -- system shutdown\n");
> }
>
> +#ifdef CONFIG_PV
> static unsigned int write_stub_trampoline(
> unsigned char *stub, unsigned long stub_va,
> unsigned long stack_bottom, unsigned long target_va)
> @@ -296,6 +297,7 @@ static unsigned int write_stub_trampoline(
> /* Round up to a multiple of 16 bytes. */
> return 32;
> }
> +#endif
>
> DEFINE_PER_CPU(struct stubs, stubs);
> void lstar_enter(void);
> @@ -303,14 +305,17 @@ void cstar_enter(void);
>
> void subarch_percpu_traps_init(void)
> {
> +#ifdef CONFIG_PV
> unsigned long stack_bottom = get_stack_bottom();
> unsigned long stub_va = this_cpu(stubs.addr);
> unsigned char *stub_page;
> unsigned int offset;
> +#endif
>
> /* IST_MAX IST pages + at least 1 guard page + primary stack. */
> BUILD_BUG_ON((IST_MAX + 1) * PAGE_SIZE + PRIMARY_STACK_SIZE > STACK_SIZE);
>
> +#ifdef CONFIG_PV
> stub_page = map_domain_page(_mfn(this_cpu(stubs.mfn)));
>
> /*
> @@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
> /* Common SYSCALL parameters. */
> wrmsrl(MSR_STAR, XEN_MSR_STAR);
> wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
> +#endif
It would be a wise precaution to initialise these MSRs to 0 in the !PV
case, so we don't retain stale values.
~Andrew
> }
>
> void hypercall_page_initialise(struct domain *d, void *hypercall_page)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-10-19 15:59 ` Andrew Cooper
@ 2018-10-29 14:37 ` Jan Beulich
2018-10-30 18:08 ` Andrew Cooper
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-29 14:37 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu; +Cc: xen-devel
>>> On 19.10.18 at 17:59, <andrew.cooper3@citrix.com> wrote:
> On 19/10/18 15:28, Wei Liu wrote:
>> @@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
>> /* Common SYSCALL parameters. */
>> wrmsrl(MSR_STAR, XEN_MSR_STAR);
>> wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
>> +#endif
>
> It would be a wise precaution to initialise these MSRs to 0 in the !PV
> case, so we don't retain stale values.
If anything, EFER.SCE needs to be kept clear, as that's what
controls whether SYSCALL would raise #GP(0). But without a
PV domain around, nothing can access the host values of
these MSRs in the first place, so instead we could simplify
some context switching by never restoring host values, and
only ever loading guest ones. Except that, of course, VMLOAD
is an all-or-nothing insn, and we need to use to get TR loaded.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-10-29 14:37 ` Jan Beulich
@ 2018-10-30 18:08 ` Andrew Cooper
2018-10-31 8:58 ` Jan Beulich
0 siblings, 1 reply; 61+ messages in thread
From: Andrew Cooper @ 2018-10-30 18:08 UTC (permalink / raw)
To: Jan Beulich, Wei Liu; +Cc: xen-devel
On 29/10/18 14:37, Jan Beulich wrote:
>>>> On 19.10.18 at 17:59, <andrew.cooper3@citrix.com> wrote:
>> On 19/10/18 15:28, Wei Liu wrote:
>>> @@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
>>> /* Common SYSCALL parameters. */
>>> wrmsrl(MSR_STAR, XEN_MSR_STAR);
>>> wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
>>> +#endif
>> It would be a wise precaution to initialise these MSRs to 0 in the !PV
>> case, so we don't retain stale values.
> If anything, EFER.SCE needs to be kept clear, as that's what
> controls whether SYSCALL would raise #GP(0).
I toyed with suggesting this, but I'm not entirely certain.
SVM unilaterally switches EFER between host and guest context, so will
preserve whatever value Xen had at VMRUN time.
Gen 2 VT-x has host/guest load/save support, so can be configured to
exit in whichever configuration we would like.
Gen 1 VT-x uses MSR load-save lists, with an optimisation in the case
that guest == host. By clearing SCE in Xen context, we miss the
optimisation in the common case for 64bit guests.
> But without a
> PV domain around, nothing can access the host values of
> these MSRs in the first place, so instead we could simplify
> some context switching by never restoring host values, and
> only ever loading guest ones. Except that, of course, VMLOAD
> is an all-or-nothing insn, and we need to use to get TR loaded.
The VMLOAD path is a bit of a special case, in that we need to do it,
and its rather faster than the other available options. Conditionally
feeding zeros into this would be fine.
That said, overall, we may want to leave some poisoned values around.
In the case that SCE is enabled and we do hit a spurious SYSCALL/SYSRET
instruction, it would be better to definitely crash.
(I'm sorry if this doesn't look like a helpful email)
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-10-30 18:08 ` Andrew Cooper
@ 2018-10-31 8:58 ` Jan Beulich
2018-11-02 12:57 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-31 8:58 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Wei Liu
>>> On 30.10.18 at 19:08, <andrew.cooper3@citrix.com> wrote:
> On 29/10/18 14:37, Jan Beulich wrote:
>>>>> On 19.10.18 at 17:59, <andrew.cooper3@citrix.com> wrote:
>>> On 19/10/18 15:28, Wei Liu wrote:
>>>> @@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
>>>> /* Common SYSCALL parameters. */
>>>> wrmsrl(MSR_STAR, XEN_MSR_STAR);
>>>> wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
>>>> +#endif
>>> It would be a wise precaution to initialise these MSRs to 0 in the !PV
>>> case, so we don't retain stale values.
>> If anything, EFER.SCE needs to be kept clear, as that's what
>> controls whether SYSCALL would raise #GP(0).
>
> I toyed with suggesting this, but I'm not entirely certain.
>
> SVM unilaterally switches EFER between host and guest context, so will
> preserve whatever value Xen had at VMRUN time.
>
> Gen 2 VT-x has host/guest load/save support, so can be configured to
> exit in whichever configuration we would like.
>
> Gen 1 VT-x uses MSR load-save lists, with an optimisation in the case
> that guest == host. By clearing SCE in Xen context, we miss the
> optimisation in the common case for 64bit guests.
>
>> But without a
>> PV domain around, nothing can access the host values of
>> these MSRs in the first place, so instead we could simplify
>> some context switching by never restoring host values, and
>> only ever loading guest ones. Except that, of course, VMLOAD
>> is an all-or-nothing insn, and we need to use to get TR loaded.
>
> The VMLOAD path is a bit of a special case, in that we need to do it,
> and its rather faster than the other available options. Conditionally
> feeding zeros into this would be fine.
>
> That said, overall, we may want to leave some poisoned values around.
> In the case that SCE is enabled and we do hit a spurious SYSCALL/SYSRET
> instruction, it would be better to definitely crash.
I'd be fine with poisoned (but not zero) values, if indeed we mean
to allow for a hypervisor crash in that case (which ought to be
fine, since we're talking about unreachable code anyway). Ideally
"poisoned" would be "non-canonical", but the MSRs don't allow for
non-canonical addresses to be loaded into them, so we'd need to
think of different poisoning values.
Trapping a spurious SYSRET seems impossible though, as EFER.SCE
is the only attribute we control there.
For SYSENTER/SYSEXIT, storing zeros is of course going to be
fine (yielding #GP(0) on both insns).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-10-31 8:58 ` Jan Beulich
@ 2018-11-02 12:57 ` Wei Liu
2018-11-02 13:03 ` Jan Beulich
0 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-11-02 12:57 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Wed, Oct 31, 2018 at 02:58:59AM -0600, Jan Beulich wrote:
> >>> On 30.10.18 at 19:08, <andrew.cooper3@citrix.com> wrote:
> > On 29/10/18 14:37, Jan Beulich wrote:
> >>>>> On 19.10.18 at 17:59, <andrew.cooper3@citrix.com> wrote:
> >>> On 19/10/18 15:28, Wei Liu wrote:
> >>>> @@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
> >>>> /* Common SYSCALL parameters. */
> >>>> wrmsrl(MSR_STAR, XEN_MSR_STAR);
> >>>> wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
> >>>> +#endif
> >>> It would be a wise precaution to initialise these MSRs to 0 in the !PV
> >>> case, so we don't retain stale values.
> >> If anything, EFER.SCE needs to be kept clear, as that's what
> >> controls whether SYSCALL would raise #GP(0).
> >
> > I toyed with suggesting this, but I'm not entirely certain.
> >
> > SVM unilaterally switches EFER between host and guest context, so will
> > preserve whatever value Xen had at VMRUN time.
> >
> > Gen 2 VT-x has host/guest load/save support, so can be configured to
> > exit in whichever configuration we would like.
> >
> > Gen 1 VT-x uses MSR load-save lists, with an optimisation in the case
> > that guest == host. By clearing SCE in Xen context, we miss the
> > optimisation in the common case for 64bit guests.
> >
> >> But without a
> >> PV domain around, nothing can access the host values of
> >> these MSRs in the first place, so instead we could simplify
> >> some context switching by never restoring host values, and
> >> only ever loading guest ones. Except that, of course, VMLOAD
> >> is an all-or-nothing insn, and we need to use to get TR loaded.
> >
> > The VMLOAD path is a bit of a special case, in that we need to do it,
> > and its rather faster than the other available options. Conditionally
> > feeding zeros into this would be fine.
> >
> > That said, overall, we may want to leave some poisoned values around.
> > In the case that SCE is enabled and we do hit a spurious SYSCALL/SYSRET
> > instruction, it would be better to definitely crash.
>
> I'd be fine with poisoned (but not zero) values, if indeed we mean
> to allow for a hypervisor crash in that case (which ought to be
> fine, since we're talking about unreachable code anyway). Ideally
> "poisoned" would be "non-canonical", but the MSRs don't allow for
> non-canonical addresses to be loaded into them, so we'd need to
> think of different poisoning values.
How about putting in a function which calls panic? That seems to be the
least intrusive option?
Wei.
>
> Trapping a spurious SYSRET seems impossible though, as EFER.SCE
> is the only attribute we control there.
>
> For SYSENTER/SYSEXIT, storing zeros is of course going to be
> fine (yielding #GP(0) on both insns).
>
> Jan
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-11-02 12:57 ` Wei Liu
@ 2018-11-02 13:03 ` Jan Beulich
0 siblings, 0 replies; 61+ messages in thread
From: Jan Beulich @ 2018-11-02 13:03 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 02.11.18 at 13:57, <wei.liu2@citrix.com> wrote:
> On Wed, Oct 31, 2018 at 02:58:59AM -0600, Jan Beulich wrote:
>> >>> On 30.10.18 at 19:08, <andrew.cooper3@citrix.com> wrote:
>> > On 29/10/18 14:37, Jan Beulich wrote:
>> >>>>> On 19.10.18 at 17:59, <andrew.cooper3@citrix.com> wrote:
>> >>> On 19/10/18 15:28, Wei Liu wrote:
>> >>>> @@ -347,6 +352,7 @@ void subarch_percpu_traps_init(void)
>> >>>> /* Common SYSCALL parameters. */
>> >>>> wrmsrl(MSR_STAR, XEN_MSR_STAR);
>> >>>> wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
>> >>>> +#endif
>> >>> It would be a wise precaution to initialise these MSRs to 0 in the !PV
>> >>> case, so we don't retain stale values.
>> >> If anything, EFER.SCE needs to be kept clear, as that's what
>> >> controls whether SYSCALL would raise #GP(0).
>> >
>> > I toyed with suggesting this, but I'm not entirely certain.
>> >
>> > SVM unilaterally switches EFER between host and guest context, so will
>> > preserve whatever value Xen had at VMRUN time.
>> >
>> > Gen 2 VT-x has host/guest load/save support, so can be configured to
>> > exit in whichever configuration we would like.
>> >
>> > Gen 1 VT-x uses MSR load-save lists, with an optimisation in the case
>> > that guest == host. By clearing SCE in Xen context, we miss the
>> > optimisation in the common case for 64bit guests.
>> >
>> >> But without a
>> >> PV domain around, nothing can access the host values of
>> >> these MSRs in the first place, so instead we could simplify
>> >> some context switching by never restoring host values, and
>> >> only ever loading guest ones. Except that, of course, VMLOAD
>> >> is an all-or-nothing insn, and we need to use to get TR loaded.
>> >
>> > The VMLOAD path is a bit of a special case, in that we need to do it,
>> > and its rather faster than the other available options. Conditionally
>> > feeding zeros into this would be fine.
>> >
>> > That said, overall, we may want to leave some poisoned values around.
>> > In the case that SCE is enabled and we do hit a spurious SYSCALL/SYSRET
>> > instruction, it would be better to definitely crash.
>>
>> I'd be fine with poisoned (but not zero) values, if indeed we mean
>> to allow for a hypervisor crash in that case (which ought to be
>> fine, since we're talking about unreachable code anyway). Ideally
>> "poisoned" would be "non-canonical", but the MSRs don't allow for
>> non-canonical addresses to be loaded into them, so we'd need to
>> think of different poisoning values.
>
> How about putting in a function which calls panic? That seems to be the
> least intrusive option?
Fine with me.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV Wei Liu
2018-10-19 15:59 ` Andrew Cooper
@ 2018-10-29 14:33 ` Jan Beulich
1 sibling, 0 replies; 61+ messages in thread
From: Jan Beulich @ 2018-10-29 14:33 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> @@ -303,14 +305,17 @@ void cstar_enter(void);
>
> void subarch_percpu_traps_init(void)
> {
> +#ifdef CONFIG_PV
> unsigned long stack_bottom = get_stack_bottom();
> unsigned long stub_va = this_cpu(stubs.addr);
> unsigned char *stub_page;
> unsigned int offset;
> +#endif
>
> /* IST_MAX IST pages + at least 1 guard page + primary stack. */
> BUILD_BUG_ON((IST_MAX + 1) * PAGE_SIZE + PRIMARY_STACK_SIZE > STACK_SIZE);
By moving this to the end of the function, one less #if/#endif
pair would be needed.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 10/16] x86: don't setup legacy syscall vector when !CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (8 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 09/16] x86: don't setup PV hypercall stubs and entries when !CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 16:09 ` Andrew Cooper
2018-10-19 14:28 ` [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs " Wei Liu
` (5 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Put the code in smpboot.c under CONFIG_PV. Not that we don't need to
set up a stub here because entry.S already does that.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/smpboot.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 35abfc4..43deb82 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1094,12 +1094,14 @@ void __init smp_prepare_cpus(void)
{
get_cpu_info()->pv_cr3 = 0;
+#ifdef CONFIG_PV
/*
* All entry points which may need to switch page tables have to start
* with interrupts off. Re-write what pv_trap_init() has put there.
*/
_set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_irq_gate, 3,
&int80_direct_trap);
+#endif
}
set_nr_sockets();
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 10/16] x86: don't setup legacy syscall vector when !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 10/16] x86: don't setup legacy syscall vector " Wei Liu
@ 2018-10-19 16:09 ` Andrew Cooper
2018-10-22 11:12 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 16:09 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Jan Beulich
On 19/10/18 15:28, Wei Liu wrote:
> Put the code in smpboot.c under CONFIG_PV. Not that we don't need to
> set up a stub here because entry.S already does that.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
The stub isn't the purpose of the code. This code is to switch between
SYS_DESC_trap_gate and SYS_DESC_irq_gate depending on whether we're
intending to use XPTI. All other details in the IDT entry are already
configured correctly for int80_direct_trap.
The patch is fine, but with an tweaked commit message, Reviewed-by:
Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 10/16] x86: don't setup legacy syscall vector when !CONFIG_PV
2018-10-19 16:09 ` Andrew Cooper
@ 2018-10-22 11:12 ` Wei Liu
2018-10-22 11:21 ` Andrew Cooper
0 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-22 11:12 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Wei Liu, Jan Beulich
On Fri, Oct 19, 2018 at 05:09:43PM +0100, Andrew Cooper wrote:
> On 19/10/18 15:28, Wei Liu wrote:
> > Put the code in smpboot.c under CONFIG_PV. Not that we don't need to
> > set up a stub here because entry.S already does that.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>
> The stub isn't the purpose of the code. This code is to switch between
> SYS_DESC_trap_gate and SYS_DESC_irq_gate depending on whether we're
> intending to use XPTI. All other details in the IDT entry are already
> configured correctly for int80_direct_trap.
>
> The patch is fine, but with an tweaked commit message, Reviewed-by:
> Andrew Cooper <andrew.cooper3@citrix.com>
I have updated the commit message to the following:
The code snippet is to switch between SYS_DECS_trap_gate and
SYS_DESC_irq_gate depending on whether XPTI is used. When PV is disabled
there is no need to switch.
Does that look OK to you?
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 10/16] x86: don't setup legacy syscall vector when !CONFIG_PV
2018-10-22 11:12 ` Wei Liu
@ 2018-10-22 11:21 ` Andrew Cooper
0 siblings, 0 replies; 61+ messages in thread
From: Andrew Cooper @ 2018-10-22 11:21 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Jan Beulich
On 22/10/18 12:12, Wei Liu wrote:
> On Fri, Oct 19, 2018 at 05:09:43PM +0100, Andrew Cooper wrote:
>> On 19/10/18 15:28, Wei Liu wrote:
>>> Put the code in smpboot.c under CONFIG_PV. Not that we don't need to
>>> set up a stub here because entry.S already does that.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> The stub isn't the purpose of the code. This code is to switch between
>> SYS_DESC_trap_gate and SYS_DESC_irq_gate depending on whether we're
>> intending to use XPTI. All other details in the IDT entry are already
>> configured correctly for int80_direct_trap.
>>
>> The patch is fine, but with an tweaked commit message, Reviewed-by:
>> Andrew Cooper <andrew.cooper3@citrix.com>
> I have updated the commit message to the following:
>
> The code snippet is to switch between SYS_DECS_trap_gate and
> SYS_DESC_irq_gate depending on whether XPTI is used. When PV is disabled
> there is no need to switch.
>
> Does that look OK to you?
Yup.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs when !CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (9 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 10/16] x86: don't setup legacy syscall vector " Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:42 ` Andrew Cooper
2018-11-02 14:05 ` Wei Liu
2018-10-19 14:28 ` [PATCH v2 12/16] x86: stub out PV only code in do_debug Wei Liu
` (4 subsequent siblings)
15 siblings, 2 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Kevin Tian, Wei Liu, Jan Beulich, Jun Nakajima
The symbol will not be available when PV is disabled.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/hvm/vmx/vmcs.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index d9747b4..282677a 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1159,9 +1159,16 @@ static int construct_vmcs(struct vcpu *v)
__vmwrite(HOST_CS_SELECTOR, __HYPERVISOR_CS);
__vmwrite(HOST_RIP, (unsigned long)vmx_asm_vmexit_handler);
+#ifdef CONFIG_PV
/* Host SYSENTER CS:RIP. */
__vmwrite(HOST_SYSENTER_CS, __HYPERVISOR_CS);
__vmwrite(HOST_SYSENTER_EIP, (unsigned long)sysenter_entry);
+#else
+ /*
+ * Should something be put here for debugging purpose? We never
+ * set it up in the first place.
+ */
+#endif
/* MSR intercepts. */
__vmwrite(VM_EXIT_MSR_LOAD_COUNT, 0);
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs when !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs " Wei Liu
@ 2018-10-19 15:42 ` Andrew Cooper
2018-10-19 16:00 ` Wei Liu
2018-11-02 14:05 ` Wei Liu
1 sibling, 1 reply; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 15:42 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Kevin Tian, Jan Beulich, Jun Nakajima
On 19/10/18 15:28, Wei Liu wrote:
> The symbol will not be available when PV is disabled.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2: new
> ---
> xen/arch/x86/hvm/vmx/vmcs.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index d9747b4..282677a 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1159,9 +1159,16 @@ static int construct_vmcs(struct vcpu *v)
> __vmwrite(HOST_CS_SELECTOR, __HYPERVISOR_CS);
> __vmwrite(HOST_RIP, (unsigned long)vmx_asm_vmexit_handler);
>
> +#ifdef CONFIG_PV
> /* Host SYSENTER CS:RIP. */
> __vmwrite(HOST_SYSENTER_CS, __HYPERVISOR_CS);
> __vmwrite(HOST_SYSENTER_EIP, (unsigned long)sysenter_entry);
> +#else
> + /*
> + * Should something be put here for debugging purpose? We never
> + * set it up in the first place.
With MSR_SYSENTER_CS set to 0, SYSENTER/SYSEXIT instructions
automatically #GP[0].
I don't think we need anything here in the !PV case.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs when !CONFIG_PV
2018-10-19 15:42 ` Andrew Cooper
@ 2018-10-19 16:00 ` Wei Liu
2018-10-19 16:16 ` Andrew Cooper
0 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 16:00 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Wei Liu, Jan Beulich, Jun Nakajima
On Fri, Oct 19, 2018 at 04:42:24PM +0100, Andrew Cooper wrote:
> On 19/10/18 15:28, Wei Liu wrote:
> > The symbol will not be available when PV is disabled.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > v2: new
> > ---
> > xen/arch/x86/hvm/vmx/vmcs.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> > index d9747b4..282677a 100644
> > --- a/xen/arch/x86/hvm/vmx/vmcs.c
> > +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> > @@ -1159,9 +1159,16 @@ static int construct_vmcs(struct vcpu *v)
> > __vmwrite(HOST_CS_SELECTOR, __HYPERVISOR_CS);
> > __vmwrite(HOST_RIP, (unsigned long)vmx_asm_vmexit_handler);
> >
> > +#ifdef CONFIG_PV
> > /* Host SYSENTER CS:RIP. */
> > __vmwrite(HOST_SYSENTER_CS, __HYPERVISOR_CS);
> > __vmwrite(HOST_SYSENTER_EIP, (unsigned long)sysenter_entry);
> > +#else
> > + /*
> > + * Should something be put here for debugging purpose? We never
> > + * set it up in the first place.
>
> With MSR_SYSENTER_CS set to 0, SYSENTER/SYSEXIT instructions
> automatically #GP[0].
OK. So we should initialise MSR_SYSENTER_CS to 0 in a previous patch.
Wei.
>
> I don't think we need anything here in the !PV case.
>
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs when !CONFIG_PV
2018-10-19 16:00 ` Wei Liu
@ 2018-10-19 16:16 ` Andrew Cooper
0 siblings, 0 replies; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 16:16 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Kevin Tian, Jan Beulich, Jun Nakajima
On 19/10/18 17:00, Wei Liu wrote:
> On Fri, Oct 19, 2018 at 04:42:24PM +0100, Andrew Cooper wrote:
>> On 19/10/18 15:28, Wei Liu wrote:
>>> The symbol will not be available when PV is disabled.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>> v2: new
>>> ---
>>> xen/arch/x86/hvm/vmx/vmcs.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
>>> index d9747b4..282677a 100644
>>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>>> @@ -1159,9 +1159,16 @@ static int construct_vmcs(struct vcpu *v)
>>> __vmwrite(HOST_CS_SELECTOR, __HYPERVISOR_CS);
>>> __vmwrite(HOST_RIP, (unsigned long)vmx_asm_vmexit_handler);
>>>
>>> +#ifdef CONFIG_PV
>>> /* Host SYSENTER CS:RIP. */
>>> __vmwrite(HOST_SYSENTER_CS, __HYPERVISOR_CS);
>>> __vmwrite(HOST_SYSENTER_EIP, (unsigned long)sysenter_entry);
>>> +#else
>>> + /*
>>> + * Should something be put here for debugging purpose? We never
>>> + * set it up in the first place.
>> With MSR_SYSENTER_CS set to 0, SYSENTER/SYSEXIT instructions
>> automatically #GP[0].
> OK. So we should initialise MSR_SYSENTER_CS to 0 in a previous patch.
Yes, but not for this reason. vmexit on Intel systems unilaterally
loads the two HOST SYSENTER MSRs, so will clobber whatever was
previously there. (There isn't even an option to turn this off to speed
up vmentry/vmexit).
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs when !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs " Wei Liu
2018-10-19 15:42 ` Andrew Cooper
@ 2018-11-02 14:05 ` Wei Liu
2018-11-02 14:32 ` Jan Beulich
1 sibling, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-11-02 14:05 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Kevin Tian, Wei Liu, Jan Beulich, Jun Nakajima
On Fri, Oct 19, 2018 at 03:28:37PM +0100, Wei Liu wrote:
> The symbol will not be available when PV is disabled.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
In light of the discussion of the previous patch, a stub for
sysenter_entry will be provided, so 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] 61+ messages in thread
* Re: [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs when !CONFIG_PV
2018-11-02 14:05 ` Wei Liu
@ 2018-11-02 14:32 ` Jan Beulich
2018-11-02 14:33 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-11-02 14:32 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, xen-devel
>>> On 02.11.18 at 15:05, <wei.liu2@citrix.com> wrote:
> On Fri, Oct 19, 2018 at 03:28:37PM +0100, Wei Liu wrote:
>> The symbol will not be available when PV is disabled.
>>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>
> In light of the discussion of the previous patch, a stub for
> sysenter_entry will be provided, so this patch can be dropped.
As said - SYSENTER is fine without stub, due to its different
faulting conditions compared to SYSCALL. Just zero the
respective MSR(s).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs when !CONFIG_PV
2018-11-02 14:32 ` Jan Beulich
@ 2018-11-02 14:33 ` Wei Liu
0 siblings, 0 replies; 61+ messages in thread
From: Wei Liu @ 2018-11-02 14:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Kevin Tian, Wei Liu, Jun Nakajima, xen-devel
On Fri, Nov 02, 2018 at 08:32:11AM -0600, Jan Beulich wrote:
> >>> On 02.11.18 at 15:05, <wei.liu2@citrix.com> wrote:
> > On Fri, Oct 19, 2018 at 03:28:37PM +0100, Wei Liu wrote:
> >> The symbol will not be available when PV is disabled.
> >>
> >> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >
> > In light of the discussion of the previous patch, a stub for
> > sysenter_entry will be provided, so this patch can be dropped.
>
> As said - SYSENTER is fine without stub, due to its different
> faulting conditions compared to SYSCALL. Just zero the
> respective MSR(s).
OK.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 12/16] x86: stub out PV only code in do_debug
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (10 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 11/16] x86: don't set sysenter_entry in vmcs " Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:43 ` Andrew Cooper
2018-10-19 14:28 ` [PATCH v2 13/16] x86: rearrange x86_64/entry.S Wei Liu
` (3 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
When PV is disabled those symbols won't be available. It is impossible
for Xen to hit #DB there.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/traps.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 002e98f..7cca072 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1841,6 +1841,7 @@ void do_debug(struct cpu_user_regs *regs)
if ( regs->eflags & X86_EFLAGS_TF )
{
+#ifdef CONFIG_PV
/* In SYSENTER entry path we can't zap TF until EFLAGS is saved. */
if ( (regs->rip >= (unsigned long)sysenter_entry) &&
(regs->rip <= (unsigned long)sysenter_eflags_saved) )
@@ -1849,6 +1850,7 @@ void do_debug(struct cpu_user_regs *regs)
regs->eflags &= ~X86_EFLAGS_TF;
return;
}
+#endif
if ( !debugger_trap_fatal(TRAP_debug, regs) )
{
WARN();
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v2 13/16] x86: rearrange x86_64/entry.S
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (11 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 12/16] x86: stub out PV only code in do_debug Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-29 14:57 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV Wei Liu
` (2 subsequent siblings)
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Split the file into two halves. The first half pertains to PV guest
code while the second half is mostly used by the hypervisor itself to
handle interrupts and exceptions.
No functional change intended.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new, requested by Andrew
---
xen/arch/x86/x86_64/entry.S | 94 ++++++++++++++++++++------------------
1 file changed, 51 insertions(+), 43 deletions(-)
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 48cb96c..319f923 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -121,16 +121,6 @@ process_trap:
call create_bounce_frame
jmp test_all_events
-/* No special register assumptions. */
-ENTRY(ret_from_intr)
- GET_CURRENT(bx)
- testb $3, UREGS_cs(%rsp)
- jz restore_all_xen
- movq VCPU_domain(%rbx), %rax
- cmpb $0, DOMAIN_is_32bit_pv(%rax)
- je test_all_events
- jmp compat_test_all_events
-
.section .text.entry, "ax", @progbits
/* %rbx: struct vcpu, interrupts disabled */
@@ -211,26 +201,6 @@ iret_exit_to_guest:
.Lft0: iretq
_ASM_PRE_EXTABLE(.Lft0, handle_exception)
- ALIGN
-/* No special register assumptions. */
-restore_all_xen:
- /*
- * Check whether we need to switch to the per-CPU page tables, in
- * case we return to late PV exit code (from an NMI or #MC).
- */
- GET_STACK_END(bx)
- cmpb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx)
-UNLIKELY_START(ne, exit_cr3)
- mov STACK_CPUINFO_FIELD(pv_cr3)(%rbx), %rax
- mov %rax, %cr3
-UNLIKELY_END(exit_cr3)
-
- /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_XEN_IST /* Req: %rbx=end, Clob: acd */
-
- RESTORE_ALL adj=8
- iretq
-
/*
* When entering SYSCALL from kernel mode:
* %rax = hypercall vector
@@ -420,6 +390,21 @@ int80_slow_path:
GET_STACK_END(14)
jmp handle_exception_saved
+self_ipi_restore_all_guest:
+ GET_CURRENT(bx)
+ /* Send an IPI to ourselves to cover for the lack of event checking. */
+ movl VCPU_processor(%rbx),%eax
+ shll $IRQSTAT_shift,%eax
+ leaq irq_stat+IRQSTAT_softirq_pending(%rip),%rcx
+ cmpl $0,(%rcx,%rax,1)
+ je 1f
+ movl $EVENT_CHECK_VECTOR,%edi
+ call send_IPI_self
+1: movq VCPU_domain(%rbx),%rax
+ cmpb $0,DOMAIN_is_32bit_pv(%rax)
+ je restore_all_guest
+ jmp compat_restore_all_guest
+
/* create_bounce_frame & helpers don't need to be in .text.entry */
.text
@@ -553,8 +538,43 @@ ENTRY(dom_crash_sync_extable)
jmp asm_domain_crash_synchronous /* Does not return */
.popsection
+/* --- CODE BELOW THIS LINE (MOSTLY) NOT GUEST RELATED --- */
+
+ .text
+
+ ALIGN
+/* No special register assumptions. */
+ENTRY(ret_from_intr)
+ GET_CURRENT(bx)
+ testb $3, UREGS_cs(%rsp)
+ jz restore_all_xen
+ movq VCPU_domain(%rbx), %rax
+ cmpb $0, DOMAIN_is_32bit_pv(%rax)
+ je test_all_events
+ jmp compat_test_all_events
+
.section .text.entry, "ax", @progbits
+ ALIGN
+/* No special register assumptions. */
+restore_all_xen:
+ /*
+ * Check whether we need to switch to the per-CPU page tables, in
+ * case we return to late PV exit code (from an NMI or #MC).
+ */
+ GET_STACK_END(bx)
+ cmpb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx)
+UNLIKELY_START(ne, exit_cr3)
+ mov STACK_CPUINFO_FIELD(pv_cr3)(%rbx), %rax
+ mov %rax, %cr3
+UNLIKELY_END(exit_cr3)
+
+ /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
+ SPEC_CTRL_EXIT_TO_XEN_IST /* Req: %rbx=end, Clob: acd */
+
+ RESTORE_ALL adj=8
+ iretq
+
ENTRY(common_interrupt)
SAVE_ALL CLAC
@@ -845,19 +865,7 @@ handle_ist_exception:
/* We want to get straight to the IRET on the NMI exit path. */
testb $3,UREGS_cs(%rsp)
jz restore_all_xen
- GET_CURRENT(bx)
- /* Send an IPI to ourselves to cover for the lack of event checking. */
- movl VCPU_processor(%rbx),%eax
- shll $IRQSTAT_shift,%eax
- leaq irq_stat+IRQSTAT_softirq_pending(%rip),%rcx
- cmpl $0,(%rcx,%rax,1)
- je 1f
- movl $EVENT_CHECK_VECTOR,%edi
- call send_IPI_self
-1: movq VCPU_domain(%rbx),%rax
- cmpb $0,DOMAIN_is_32bit_pv(%rax)
- je restore_all_guest
- jmp compat_restore_all_guest
+ jmp self_ipi_restore_all_guest
ENTRY(machine_check)
pushq $0
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 13/16] x86: rearrange x86_64/entry.S
2018-10-19 14:28 ` [PATCH v2 13/16] x86: rearrange x86_64/entry.S Wei Liu
@ 2018-10-29 14:57 ` Jan Beulich
2018-11-02 12:54 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-29 14:57 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> @@ -845,19 +865,7 @@ handle_ist_exception:
> /* We want to get straight to the IRET on the NMI exit path. */
> testb $3,UREGS_cs(%rsp)
> jz restore_all_xen
> - GET_CURRENT(bx)
> - /* Send an IPI to ourselves to cover for the lack of event checking. */
> - movl VCPU_processor(%rbx),%eax
> - shll $IRQSTAT_shift,%eax
> - leaq irq_stat+IRQSTAT_softirq_pending(%rip),%rcx
> - cmpl $0,(%rcx,%rax,1)
> - je 1f
> - movl $EVENT_CHECK_VECTOR,%edi
> - call send_IPI_self
> -1: movq VCPU_domain(%rbx),%rax
> - cmpb $0,DOMAIN_is_32bit_pv(%rax)
> - je restore_all_guest
> - jmp compat_restore_all_guest
> + jmp self_ipi_restore_all_guest
I'm having some trouble understanding why you move this code:
Without CONFIG_PV it is unreachable. I'd prefer if it stayed in
place and simply got an #ifdef around it. The one alternative
option I could see is to move the restore_all_xen code block
right here, so the JMP you put in could become JNZ.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 13/16] x86: rearrange x86_64/entry.S
2018-10-29 14:57 ` Jan Beulich
@ 2018-11-02 12:54 ` Wei Liu
0 siblings, 0 replies; 61+ messages in thread
From: Wei Liu @ 2018-11-02 12:54 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Mon, Oct 29, 2018 at 08:57:19AM -0600, Jan Beulich wrote:
> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> > @@ -845,19 +865,7 @@ handle_ist_exception:
> > /* We want to get straight to the IRET on the NMI exit path. */
> > testb $3,UREGS_cs(%rsp)
> > jz restore_all_xen
> > - GET_CURRENT(bx)
> > - /* Send an IPI to ourselves to cover for the lack of event checking. */
> > - movl VCPU_processor(%rbx),%eax
> > - shll $IRQSTAT_shift,%eax
> > - leaq irq_stat+IRQSTAT_softirq_pending(%rip),%rcx
> > - cmpl $0,(%rcx,%rax,1)
> > - je 1f
> > - movl $EVENT_CHECK_VECTOR,%edi
> > - call send_IPI_self
> > -1: movq VCPU_domain(%rbx),%rax
> > - cmpb $0,DOMAIN_is_32bit_pv(%rax)
> > - je restore_all_guest
> > - jmp compat_restore_all_guest
> > + jmp self_ipi_restore_all_guest
>
> I'm having some trouble understanding why you move this code:
> Without CONFIG_PV it is unreachable.
I thought this snippet was self-contained enough to move to the first
half of the file. But ...
> I'd prefer if it stayed in
> place and simply got an #ifdef around it.
... I'm fine with this too.
Wei.
> The one alternative
> option I could see is to move the restore_all_xen code block
> right here, so the JMP you put in could become JNZ.
>
> Jan
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (12 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 13/16] x86: rearrange x86_64/entry.S Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-29 15:03 ` Jan Beulich
2018-10-19 14:28 ` [PATCH v2 15/16] x86: expose CONFIG_PV Wei Liu
2018-10-19 14:28 ` [PATCH v2 16/16] x86: update help string for CONFIG_HVM Wei Liu
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Skip building x86_64/compat/entry.S and put CONFIG_PV in
x86_64/entry.S.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/x86_64/Makefile | 2 +-
xen/arch/x86/x86_64/entry.S | 30 ++++++++++++++++++++++++++++--
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile
index f336a6a..4bfa148 100644
--- a/xen/arch/x86/x86_64/Makefile
+++ b/xen/arch/x86/x86_64/Makefile
@@ -1,4 +1,4 @@
-subdir-y += compat
+subdir-$(CONFIG_PV) += compat
obj-bin-y += entry.o
obj-y += traps.o
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 319f923..16cd911 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -15,6 +15,7 @@
#include <public/xen.h>
#include <irq_vectors.h>
+#ifdef CONFIG_PV
/* %rbx: struct vcpu */
ENTRY(switch_to_kernel)
leaq VCPU_trap_bounce(%rbx),%rdx
@@ -537,6 +538,7 @@ ENTRY(dom_crash_sync_extable)
xorl %edi,%edi
jmp asm_domain_crash_synchronous /* Does not return */
.popsection
+#endif /* CONFIG_PV */
/* --- CODE BELOW THIS LINE (MOSTLY) NOT GUEST RELATED --- */
@@ -548,10 +550,14 @@ ENTRY(ret_from_intr)
GET_CURRENT(bx)
testb $3, UREGS_cs(%rsp)
jz restore_all_xen
+#ifdef CONFIG_PV
movq VCPU_domain(%rbx), %rax
cmpb $0, DOMAIN_is_32bit_pv(%rax)
je test_all_events
jmp compat_test_all_events
+#else
+ BUG
+#endif
.section .text.entry, "ax", @progbits
@@ -596,8 +602,9 @@ ENTRY(common_interrupt)
cmovnz %r12, %r15
cmovnz %r12d, %ebx
.Lintr_cr3_okay:
-
+#ifdef CONFIG_PV
CR4_PV32_RESTORE
+#endif
movq %rsp,%rdi
callq do_IRQ
mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
@@ -634,12 +641,15 @@ handle_exception_saved:
testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp)
jz exception_with_ints_disabled
+#ifdef CONFIG_PV
ALTERNATIVE_2 "jmp .Lcr4_pv32_done", \
__stringify(mov VCPU_domain(%rbx), %rax), X86_FEATURE_XEN_SMEP, \
__stringify(mov VCPU_domain(%rbx), %rax), X86_FEATURE_XEN_SMAP
+#endif
testb $3,UREGS_cs(%rsp)
jz .Lcr4_pv32_done
+#ifdef CONFIG_PV
cmpb $0,DOMAIN_is_32bit_pv(%rax)
je .Lcr4_pv32_done
call cr4_pv32_restore
@@ -672,6 +682,9 @@ handle_exception_saved:
xor UREGS_error_code(%rsp),%eax
test $~(PFEC_write_access|PFEC_insn_fetch),%eax
jz compat_test_all_events
+#else
+ BUG
+#endif /* CONFIG_PV */
.Lcr4_pv32_done:
sti
1: movq %rsp,%rdi
@@ -684,10 +697,14 @@ handle_exception_saved:
mov %r13b, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
testb $3,UREGS_cs(%rsp)
jz restore_all_xen
+#ifdef CONFIG_PV
movq VCPU_domain(%rbx),%rax
cmpb $0, DOMAIN_is_32bit_pv(%rax)
jne compat_test_all_events
jmp test_all_events
+#else
+ BUG
+#endif
/* No special register assumptions. */
exception_with_ints_disabled:
@@ -837,10 +854,12 @@ handle_ist_exception:
/* %r12 is still zero at this point. */
mov %r12, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
.List_cr3_okay:
-
+#ifdef CONFIG_PV
CR4_PV32_RESTORE
+#endif
testb $3,UREGS_cs(%rsp)
jz 1f
+#ifdef CONFIG_PV
/*
* Interrupted guest context. Clear the restore value for xen_cr3
* and copy the context to stack bottom.
@@ -852,6 +871,9 @@ handle_ist_exception:
movl $UREGS_kernel_sizeof/8,%ecx
movq %rdi,%rsp
rep movsq
+#else
+ BUG
+#endif
1: movq %rsp,%rdi
movzbl UREGS_entry_vector(%rsp),%eax
leaq exception_table(%rip),%rdx
@@ -865,7 +887,11 @@ handle_ist_exception:
/* We want to get straight to the IRET on the NMI exit path. */
testb $3,UREGS_cs(%rsp)
jz restore_all_xen
+#ifdef CONFIG_PV
jmp self_ipi_restore_all_guest
+#else
+ BUG
+#endif
ENTRY(machine_check)
pushq $0
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV Wei Liu
@ 2018-10-29 15:03 ` Jan Beulich
2018-11-02 13:08 ` Wei Liu
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-10-29 15:03 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> @@ -548,10 +550,14 @@ ENTRY(ret_from_intr)
> GET_CURRENT(bx)
> testb $3, UREGS_cs(%rsp)
> jz restore_all_xen
> +#ifdef CONFIG_PV
> movq VCPU_domain(%rbx), %rax
> cmpb $0, DOMAIN_is_32bit_pv(%rax)
> je test_all_events
> jmp compat_test_all_events
> +#else
> + BUG
> +#endif
Hmm, not sure here (and elsewhere): Another option is to
streamline execution by replacing the conditional branch with an
unconditional one in the !PV case. Andrew, do you have any
thoughts either way?
> @@ -596,8 +602,9 @@ ENTRY(common_interrupt)
> cmovnz %r12, %r15
> cmovnz %r12d, %ebx
> .Lintr_cr3_okay:
> -
> +#ifdef CONFIG_PV
> CR4_PV32_RESTORE
> +#endif
Couldn't you instead make the macro expand to nothing?
Also please don't remove the blank line.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV
2018-10-29 15:03 ` Jan Beulich
@ 2018-11-02 13:08 ` Wei Liu
2018-11-02 13:45 ` Jan Beulich
0 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-11-02 13:08 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel
On Mon, Oct 29, 2018 at 09:03:18AM -0600, Jan Beulich wrote:
> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> > @@ -548,10 +550,14 @@ ENTRY(ret_from_intr)
> > GET_CURRENT(bx)
> > testb $3, UREGS_cs(%rsp)
> > jz restore_all_xen
> > +#ifdef CONFIG_PV
> > movq VCPU_domain(%rbx), %rax
> > cmpb $0, DOMAIN_is_32bit_pv(%rax)
> > je test_all_events
> > jmp compat_test_all_events
> > +#else
> > + BUG
> > +#endif
>
> Hmm, not sure here (and elsewhere): Another option is to
> streamline execution by replacing the conditional branch with an
> unconditional one in the !PV case. Andrew, do you have any
> thoughts either way?
My original thought was to catch potential issues in Xen code which
messes up with the permission level. Using unconditional jump is fine
by me, too. But in that case I will seek to at least add an assertion
for debug build.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV
2018-11-02 13:08 ` Wei Liu
@ 2018-11-02 13:45 ` Jan Beulich
2018-11-02 14:14 ` Andrew Cooper
0 siblings, 1 reply; 61+ messages in thread
From: Jan Beulich @ 2018-11-02 13:45 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 02.11.18 at 14:08, <wei.liu2@citrix.com> wrote:
> On Mon, Oct 29, 2018 at 09:03:18AM -0600, Jan Beulich wrote:
>> >>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
>> > @@ -548,10 +550,14 @@ ENTRY(ret_from_intr)
>> > GET_CURRENT(bx)
>> > testb $3, UREGS_cs(%rsp)
>> > jz restore_all_xen
>> > +#ifdef CONFIG_PV
>> > movq VCPU_domain(%rbx), %rax
>> > cmpb $0, DOMAIN_is_32bit_pv(%rax)
>> > je test_all_events
>> > jmp compat_test_all_events
>> > +#else
>> > + BUG
>> > +#endif
>>
>> Hmm, not sure here (and elsewhere): Another option is to
>> streamline execution by replacing the conditional branch with an
>> unconditional one in the !PV case. Andrew, do you have any
>> thoughts either way?
>
> My original thought was to catch potential issues in Xen code which
> messes up with the permission level. Using unconditional jump is fine
> by me, too. But in that case I will seek to at least add an assertion
> for debug build.
Assertion additions are definitely fine with me.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV
2018-11-02 13:45 ` Jan Beulich
@ 2018-11-02 14:14 ` Andrew Cooper
0 siblings, 0 replies; 61+ messages in thread
From: Andrew Cooper @ 2018-11-02 14:14 UTC (permalink / raw)
To: Jan Beulich, Wei Liu; +Cc: xen-devel
On 02/11/18 13:45, Jan Beulich wrote:
>>>> On 02.11.18 at 14:08, <wei.liu2@citrix.com> wrote:
>> On Mon, Oct 29, 2018 at 09:03:18AM -0600, Jan Beulich wrote:
>>>>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
>>>> @@ -548,10 +550,14 @@ ENTRY(ret_from_intr)
>>>> GET_CURRENT(bx)
>>>> testb $3, UREGS_cs(%rsp)
>>>> jz restore_all_xen
>>>> +#ifdef CONFIG_PV
>>>> movq VCPU_domain(%rbx), %rax
>>>> cmpb $0, DOMAIN_is_32bit_pv(%rax)
>>>> je test_all_events
>>>> jmp compat_test_all_events
>>>> +#else
>>>> + BUG
>>>> +#endif
>>> Hmm, not sure here (and elsewhere): Another option is to
>>> streamline execution by replacing the conditional branch with an
>>> unconditional one in the !PV case. Andrew, do you have any
>>> thoughts either way?
>> My original thought was to catch potential issues in Xen code which
>> messes up with the permission level. Using unconditional jump is fine
>> by me, too. But in that case I will seek to at least add an assertion
>> for debug build.
> Assertion additions are definitely fine with me.
Yeah - I'd also err on the side of an assertion and an unconditional jump.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 15/16] x86: expose CONFIG_PV
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (13 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 14/16] x86: make entry point code build when !CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:51 ` Andrew Cooper
2018-10-19 14:28 ` [PATCH v2 16/16] x86: update help string for CONFIG_HVM Wei Liu
15 siblings, 1 reply; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: guest -> domain
---
xen/arch/x86/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 548cbf9..c7e97e2 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 domains 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 domains.
+
+ 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
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 15/16] x86: expose CONFIG_PV
2018-10-19 14:28 ` [PATCH v2 15/16] x86: expose CONFIG_PV Wei Liu
@ 2018-10-19 15:51 ` Andrew Cooper
0 siblings, 0 replies; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 15:51 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Jan Beulich
On 19/10/18 15:28, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2: guest -> domain
> ---
> xen/arch/x86/Kconfig | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 548cbf9..c7e97e2 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 domains which don't require hardware
> + support like Intel's VT-x or AMD's SVM.
Sorry to nitpick, but I'd suggest wording this as:
Interfaces to support PV domains. These require guest kernel support to
run as a PV guest, but don't require any specific hardware support.
Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> +
> + This option is needed if you want to run PV domains.
> +
> + If unsure, say Y.
>
> config PV_LINEAR_PT
> bool "Support for PV linear pagetables"
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v2 16/16] x86: update help string for CONFIG_HVM
2018-10-19 14:28 [PATCH v2 00/16] Make CONFIG_PV work on x86 Wei Liu
` (14 preceding siblings ...)
2018-10-19 14:28 ` [PATCH v2 15/16] x86: expose CONFIG_PV Wei Liu
@ 2018-10-19 14:28 ` Wei Liu
2018-10-19 15:55 ` Andrew Cooper
2018-10-29 14:14 ` Jan Beulich
15 siblings, 2 replies; 61+ messages in thread
From: Wei Liu @ 2018-10-19 14:28 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Change "guest" to "domain" where appropriate because "guest" doesn't
include Domain 0.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
---
xen/arch/x86/Kconfig | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index c7e97e2..b4bd83e 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -70,12 +70,12 @@ config HVM
def_bool !PV_SHIM_EXCLUSIVE
prompt "HVM support"
---help---
- Interfaces to support HVM guests which require hardware
+ Interfaces to support HVM domains 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.
+ doesn't distinguish HVM or PVH. PVH is only a concept for
+ end users.
- This option is needed if you want to run HVM or PVH guests.
+ This option is needed if you want to run HVM or PVH domains.
If unsure, say Y.
--
git-series 0.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v2 16/16] x86: update help string for CONFIG_HVM
2018-10-19 14:28 ` [PATCH v2 16/16] x86: update help string for CONFIG_HVM Wei Liu
@ 2018-10-19 15:55 ` Andrew Cooper
2018-10-29 14:14 ` Jan Beulich
1 sibling, 0 replies; 61+ messages in thread
From: Andrew Cooper @ 2018-10-19 15:55 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Jan Beulich
On 19/10/18 15:28, Wei Liu wrote:
> Change "guest" to "domain" where appropriate because "guest" doesn't
> include Domain 0.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2: new
> ---
> xen/arch/x86/Kconfig | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index c7e97e2..b4bd83e 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -70,12 +70,12 @@ config HVM
> def_bool !PV_SHIM_EXCLUSIVE
> prompt "HVM support"
> ---help---
> - Interfaces to support HVM guests which require hardware
> + Interfaces to support HVM domains 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.
> + doesn't distinguish HVM or PVH. PVH is only a concept for
> + end users.
Matching the PV suggestion, how about:
Interfaces to support HVM domains. HVM domains requires hardware
virtualisation extensions (e.g. Intel VT-x, AMD SVM), but can boot
guests which have no specific Xen knowledge.
?
~Andrew
>
> - This option is needed if you want to run HVM or PVH guests.
> + This option is needed if you want to run HVM or PVH domains.
>
> If unsure, say Y.
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v2 16/16] x86: update help string for CONFIG_HVM
2018-10-19 14:28 ` [PATCH v2 16/16] x86: update help string for CONFIG_HVM Wei Liu
2018-10-19 15:55 ` Andrew Cooper
@ 2018-10-29 14:14 ` Jan Beulich
1 sibling, 0 replies; 61+ messages in thread
From: Jan Beulich @ 2018-10-29 14:14 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, xen-devel
>>> On 19.10.18 at 16:28, <wei.liu2@citrix.com> wrote:
> Change "guest" to "domain" where appropriate because "guest" doesn't
> include Domain 0.
>
> 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] 61+ messages in thread