* [PATCH v3 1/4] x86/pvh: Set 32b PVH guest mode in XEN_DOMCTL_set_address_size
2015-07-10 22:20 [PATCH v3 0/4] 32-bit domU PVH support Boris Ostrovsky
@ 2015-07-10 22:20 ` Boris Ostrovsky
2015-07-23 13:59 ` Jan Beulich
2015-07-10 22:20 ` [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode Boris Ostrovsky
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Boris Ostrovsky @ 2015-07-10 22:20 UTC (permalink / raw)
To: xen-devel
Cc: elena.ufimtseva, wei.liu2, ian.campbell, andrew.cooper3,
stefano.stabellini, ian.jackson, jbeulich, boris.ostrovsky,
roger.pau
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v3:
* Updated switch_native() not to call release_compat_l4() on PVH guests
* hvm_set_mode() handles both 8 and 4 modes and returns
-EOPNOTSUPP otherwise. Similar chages to vmx_set_mode()
xen/arch/x86/domain.c | 27 ++++++++++++++++-----------
xen/arch/x86/hvm/hvm.c | 24 +++++++++++++++++++++++-
xen/arch/x86/hvm/vmx/vmcs.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 19 +++++++++++++++++++
xen/include/asm-x86/hvm/hvm.h | 2 ++
5 files changed, 61 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 956ac70..9c29ef2 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -366,7 +366,11 @@ int switch_native(struct domain *d)
for_each_vcpu( d, v )
{
free_compat_arg_xlat(v);
- release_compat_l4(v);
+
+ if ( !is_pvh_domain(d) )
+ release_compat_l4(v);
+ else
+ hvm_set_mode(v, 8);
}
return 0;
@@ -377,25 +381,26 @@ int switch_compat(struct domain *d)
struct vcpu *v;
int rc;
- if ( is_pvh_domain(d) )
- {
- printk(XENLOG_G_INFO
- "Xen currently does not support 32bit PVH guests\n");
- return -EINVAL;
- }
-
if ( !may_switch_mode(d) )
return -EACCES;
if ( is_pv_32bit_domain(d) )
return 0;
- d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
+ d->arch.has_32bit_shinfo = 1;
+ if ( is_pv_domain(d) )
+ d->arch.is_32bit_pv = 1;
for_each_vcpu( d, v )
{
rc = setup_compat_arg_xlat(v);
if ( !rc )
- rc = setup_compat_l4(v);
+ {
+ if ( !is_pvh_domain(d) )
+ rc = setup_compat_l4(v);
+ else
+ rc = hvm_set_mode(v, 4);
+ }
+
if ( rc )
goto undo_and_fail;
}
@@ -410,7 +415,7 @@ int switch_compat(struct domain *d)
{
free_compat_arg_xlat(v);
- if ( !pagetable_is_null(v->arch.guest_table) )
+ if ( !is_pvh_domain(d) && !pagetable_is_null(v->arch.guest_table) )
release_compat_l4(v);
}
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ebcf7a9..6f247a0 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2373,7 +2373,6 @@ int hvm_vcpu_initialise(struct vcpu *v)
if ( is_pvh_domain(d) )
{
- v->arch.hvm_vcpu.hcall_64bit = 1; /* PVH 32bitfixme. */
/* This is for hvm_long_mode_enabled(v). */
v->arch.hvm_vcpu.guest_efer = EFER_LMA | EFER_LME;
return 0;
@@ -6463,6 +6462,29 @@ void hvm_toggle_singlestep(struct vcpu *v)
v->arch.hvm_vcpu.single_step = !v->arch.hvm_vcpu.single_step;
}
+int hvm_set_mode(struct vcpu *v, int mode)
+{
+
+ switch ( mode )
+ {
+ case 4:
+ v->arch.hvm_vcpu.guest_efer &= ~(EFER_LMA | EFER_LME);
+ break;
+ case 8:
+ v->arch.hvm_vcpu.guest_efer |= (EFER_LMA | EFER_LME);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ hvm_update_guest_efer(v);
+
+ if ( hvm_funcs.set_mode )
+ return hvm_funcs.set_mode(v, mode);
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 4c5ceb5..55ab7ad 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1125,7 +1125,7 @@ static int construct_vmcs(struct vcpu *v)
__vmwrite(GUEST_FS_AR_BYTES, 0xc093);
__vmwrite(GUEST_GS_AR_BYTES, 0xc093);
if ( is_pvh_domain(d) )
- /* CS.L == 1, exec, read/write, accessed. PVH 32bitfixme. */
+ /* CS.L == 1, exec, read/write, accessed. */
__vmwrite(GUEST_CS_AR_BYTES, 0xa09b);
else
__vmwrite(GUEST_CS_AR_BYTES, 0xc09b); /* exec/read, accessed */
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index bc3212f..888dea2 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1770,6 +1770,24 @@ static bool_t vmx_is_singlestep_supported(void)
return cpu_has_monitor_trap_flag;
}
+static int vmx_set_mode(struct vcpu *v, int mode)
+{
+ unsigned long attr;
+
+ if ( !is_pvh_vcpu(v) )
+ return 0;
+
+ ASSERT((mode == 4) || (mode == 8));
+
+ attr = (mode == 4) ? 0xc09b : 0xa09b;
+
+ vmx_vmcs_enter(v);
+ __vmwrite(GUEST_CS_AR_BYTES, attr);
+ vmx_vmcs_exit(v);
+
+ return 0;
+}
+
static struct hvm_function_table __initdata vmx_function_table = {
.name = "VMX",
.cpu_up_prepare = vmx_cpu_up_prepare,
@@ -1828,6 +1846,7 @@ static struct hvm_function_table __initdata vmx_function_table = {
.hypervisor_cpuid_leaf = vmx_hypervisor_cpuid_leaf,
.enable_msr_exit_interception = vmx_enable_msr_exit_interception,
.is_singlestep_supported = vmx_is_singlestep_supported,
+ .set_mode = vmx_set_mode,
};
const struct hvm_function_table * __init start_vmx(void)
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 35f1300..c35dd19 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -203,6 +203,7 @@ struct hvm_function_table {
void (*enable_msr_exit_interception)(struct domain *d);
bool_t (*is_singlestep_supported)(void);
+ int (*set_mode)(struct vcpu *v, int mode);
};
extern struct hvm_function_table hvm_funcs;
@@ -237,6 +238,7 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc);
u64 hvm_get_guest_tsc_fixed(struct vcpu *v, u64 at_tsc);
#define hvm_get_guest_tsc(v) hvm_get_guest_tsc_fixed(v, 0)
+int hvm_set_mode(struct vcpu *v, int mode);
void hvm_init_guest_time(struct domain *d);
void hvm_set_guest_time(struct vcpu *v, u64 guest_time);
u64 hvm_get_guest_time_fixed(struct vcpu *v, u64 at_tsc);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-07-10 22:20 [PATCH v3 0/4] 32-bit domU PVH support Boris Ostrovsky
2015-07-10 22:20 ` [PATCH v3 1/4] x86/pvh: Set 32b PVH guest mode in XEN_DOMCTL_set_address_size Boris Ostrovsky
@ 2015-07-10 22:20 ` Boris Ostrovsky
2015-07-23 14:07 ` Jan Beulich
2015-07-10 22:20 ` [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b PVH guests Boris Ostrovsky
2015-07-10 22:20 ` [PATCH v3 4/4] libxc/x86/pvh: Allow creation of " Boris Ostrovsky
3 siblings, 1 reply; 17+ messages in thread
From: Boris Ostrovsky @ 2015-07-10 22:20 UTC (permalink / raw)
To: xen-devel
Cc: elena.ufimtseva, wei.liu2, ian.campbell, andrew.cooper3,
stefano.stabellini, ian.jackson, jbeulich, boris.ostrovsky,
roger.pau
Add is_pvh_32bit_domain() macro and use it alongside is_pv_32bit_domain() where
necessary.
Since PVH guests cannot change execution mode, has_32bit_shinfo is a good
indicator of whether the guest is PVH and 32-bit.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v3:
* Defined is_pvh_32bit_domain()
* Dropped unnecessary changes in hypercall_create_continuation() and
do_domctl() (XEN_DOMCTL_set/getvcpucontext)
xen/arch/x86/domain.c | 6 +++---
xen/arch/x86/domctl.c | 5 +++--
xen/include/asm-x86/domain.h | 1 +
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 9c29ef2..c743362 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -358,7 +358,7 @@ int switch_native(struct domain *d)
if ( !may_switch_mode(d) )
return -EACCES;
- if ( !is_pv_32bit_domain(d) )
+ if ( !is_pv_32bit_domain(d) && !is_pvh_32bit_domain(d) )
return 0;
d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
@@ -383,7 +383,7 @@ int switch_compat(struct domain *d)
if ( !may_switch_mode(d) )
return -EACCES;
- if ( is_pv_32bit_domain(d) )
+ if ( is_pv_32bit_domain(d) || is_pvh_32bit_domain(d) )
return 0;
d->arch.has_32bit_shinfo = 1;
@@ -771,7 +771,7 @@ int arch_set_info_guest(
/* 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. */
- compat = is_pv_32bit_domain(d);
+ compat = is_pv_32bit_domain(d) || is_pvh_32bit_domain(d);
#define c(fld) (compat ? (c.cmp->fld) : (c.nat->fld))
flags = c(flags);
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index b5047db..63cca52 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -349,7 +349,8 @@ long arch_do_domctl(
case XEN_DOMCTL_get_address_size:
domctl->u.address_size.size =
- is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG;
+ (is_pv_32bit_domain(d) || is_pvh_32bit_domain(d)) ?
+ 32 : BITS_PER_LONG;
copyback = 1;
break;
@@ -1203,7 +1204,7 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
{
unsigned int i;
const struct domain *d = v->domain;
- bool_t compat = is_pv_32bit_domain(d);
+ bool_t compat = is_pv_32bit_domain(d) || is_pvh_32bit_domain(d);
#define c(fld) (!compat ? (c.nat->fld) : (c.cmp->fld))
if ( !is_pv_domain(d) )
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 201436d..263c022 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -14,6 +14,7 @@
#define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo)
#define is_pv_32bit_domain(d) ((d)->arch.is_32bit_pv)
#define is_pv_32bit_vcpu(v) (is_pv_32bit_domain((v)->domain))
+#define is_pvh_32bit_domain(d) (is_pvh_domain(d) && has_32bit_shinfo(d))
#define is_hvm_pv_evtchn_domain(d) (has_hvm_container_domain(d) && \
d->arch.hvm_domain.irq.callback_via_type == HVMIRQ_callback_vector)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-07-10 22:20 ` [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode Boris Ostrovsky
@ 2015-07-23 14:07 ` Jan Beulich
2015-07-23 14:13 ` Ian Campbell
2015-07-24 17:54 ` Boris Ostrovsky
0 siblings, 2 replies; 17+ messages in thread
From: Jan Beulich @ 2015-07-23 14:07 UTC (permalink / raw)
To: roger.pau, Boris Ostrovsky
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel
>>> On 11.07.15 at 00:20, <boris.ostrovsky@oracle.com> wrote:
> Add is_pvh_32bit_domain() macro and use it alongside is_pv_32bit_domain()
> where necessary.
>
> Since PVH guests cannot change execution mode, has_32bit_shinfo is a good
> indicator of whether the guest is PVH and 32-bit.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Relative to what is in the tree right now this is fine, but ...
> @@ -771,7 +771,7 @@ int arch_set_info_guest(
>
> /* 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. */
> - compat = is_pv_32bit_domain(d);
> + compat = is_pv_32bit_domain(d) || is_pvh_32bit_domain(d);
... won't this and ...
> @@ -1203,7 +1204,7 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
> {
> unsigned int i;
> const struct domain *d = v->domain;
> - bool_t compat = is_pv_32bit_domain(d);
> + bool_t compat = is_pv_32bit_domain(d) || is_pvh_32bit_domain(d);
... this get in the way of what we called "no-pm" on yesterday's call?
I would assume that for the transitional period both ought to be able
to co-exist.
Plus - is this in line with what the tools are doing? Aren't they
assuming !PV <=> native format context? I.e. don't you need
to treat differently v->domain == current->domain and its
opposite? Roger btw. raised a similar question on IRC earlier
today...
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-07-23 14:07 ` Jan Beulich
@ 2015-07-23 14:13 ` Ian Campbell
2015-07-23 14:23 ` Jan Beulich
2015-07-24 17:54 ` Boris Ostrovsky
1 sibling, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2015-07-23 14:13 UTC (permalink / raw)
To: Jan Beulich, roger.pau, Boris Ostrovsky
Cc: elena.ufimtseva, wei.liu2, stefano.stabellini, andrew.cooper3,
ian.jackson, xen-devel
On Thu, 2015-07-23 at 08:07 -0600, Jan Beulich wrote:
> what we called "no-pm" on yesterday's call?
FYI it was "no-dm" (no device model).
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-07-23 14:13 ` Ian Campbell
@ 2015-07-23 14:23 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2015-07-23 14:23 UTC (permalink / raw)
To: Ian Campbell
Cc: elena.ufimtseva, wei.liu2, stefano.stabellini, andrew.cooper3,
ian.jackson, xen-devel, Boris Ostrovsky, roger.pau
>>> On 23.07.15 at 16:13, <ian.campbell@citrix.com> wrote:
> On Thu, 2015-07-23 at 08:07 -0600, Jan Beulich wrote:
>> what we called "no-pm" on yesterday's call?
>
> FYI it was "no-dm" (no device model).
Of course it was - fingers typing disconnected from brain, I guess.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-07-23 14:07 ` Jan Beulich
2015-07-23 14:13 ` Ian Campbell
@ 2015-07-24 17:54 ` Boris Ostrovsky
2015-08-11 9:19 ` Jan Beulich
1 sibling, 1 reply; 17+ messages in thread
From: Boris Ostrovsky @ 2015-07-24 17:54 UTC (permalink / raw)
To: Jan Beulich, roger.pau
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel
On 07/23/2015 10:07 AM, Jan Beulich wrote:
>>>> On 11.07.15 at 00:20, <boris.ostrovsky@oracle.com> wrote:
>> Add is_pvh_32bit_domain() macro and use it alongside is_pv_32bit_domain()
>> where necessary.
>>
>> Since PVH guests cannot change execution mode, has_32bit_shinfo is a good
>> indicator of whether the guest is PVH and 32-bit.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Relative to what is in the tree right now this is fine, but ...
>
>> @@ -771,7 +771,7 @@ int arch_set_info_guest(
>>
>> /* 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. */
>> - compat = is_pv_32bit_domain(d);
>> + compat = is_pv_32bit_domain(d) || is_pvh_32bit_domain(d);
> ... won't this and ...
>
>> @@ -1203,7 +1204,7 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
>> {
>> unsigned int i;
>> const struct domain *d = v->domain;
>> - bool_t compat = is_pv_32bit_domain(d);
>> + bool_t compat = is_pv_32bit_domain(d) || is_pvh_32bit_domain(d);
> ... this get in the way of what we called "no-pm" on yesterday's call?
I made no effort to make this workable for "no-dm" case. This will
probably need to be adjusted when that code gets in. IIRC this new code
has a flag (or something along those lines) that will allow us to
distinguish PVH-"classic" vs. "no-dm".
> I would assume that for the transitional period both ought to be able
> to co-exist.
Yes, but I suspect this will not be the only place where we will need to
have different code paths for the two approaches. hvm_set_cr0(), for
example, doesn't allow PVH guests to switch modes.
> Plus - is this in line with what the tools are doing? Aren't they
> assuming !PV <=> native format context? I.e. don't you need
> to treat differently v->domain == current->domain and its
> opposite? Roger btw. raised a similar question on IRC earlier
> today...
Not sure I understand this. You mean for copying 64-bit guest's info
into 32-bit dom0?
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-07-24 17:54 ` Boris Ostrovsky
@ 2015-08-11 9:19 ` Jan Beulich
2015-08-11 17:21 ` Boris Ostrovsky
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2015-08-11 9:19 UTC (permalink / raw)
To: roger.pau, Boris Ostrovsky
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel
>>> On 24.07.15 at 19:54, <boris.ostrovsky@oracle.com> wrote:
> On 07/23/2015 10:07 AM, Jan Beulich wrote:
>> Plus - is this in line with what the tools are doing? Aren't they
>> assuming !PV <=> native format context? I.e. don't you need
>> to treat differently v->domain == current->domain and its
>> opposite? Roger btw. raised a similar question on IRC earlier
>> today...
>
> Not sure I understand this. You mean for copying 64-bit guest's info
> into 32-bit dom0?
Basically yes - tool stack and guest invocations may need to
behave differently.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-08-11 9:19 ` Jan Beulich
@ 2015-08-11 17:21 ` Boris Ostrovsky
2015-08-12 6:23 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Boris Ostrovsky @ 2015-08-11 17:21 UTC (permalink / raw)
To: Jan Beulich, roger.pau
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel
On 08/11/2015 05:19 AM, Jan Beulich wrote:
>>>> On 24.07.15 at 19:54, <boris.ostrovsky@oracle.com> wrote:
>> On 07/23/2015 10:07 AM, Jan Beulich wrote:
>>> Plus - is this in line with what the tools are doing? Aren't they
>>> assuming !PV <=> native format context? I.e. don't you need
>>> to treat differently v->domain == current->domain and its
>>> opposite? Roger btw. raised a similar question on IRC earlier
>>> today...
>> Not sure I understand this. You mean for copying 64-bit guest's info
>> into 32-bit dom0?
> Basically yes - tool stack and guest invocations may need to
> behave differently.
This being PVH-"classic" it follows exactly the PV path (both in tools
and the hypervisor). Wouldn't PV be broken then as well?
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-08-11 17:21 ` Boris Ostrovsky
@ 2015-08-12 6:23 ` Jan Beulich
2015-08-12 15:02 ` Boris Ostrovsky
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2015-08-12 6:23 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, roger.pau
>>> On 11.08.15 at 19:21, <boris.ostrovsky@oracle.com> wrote:
> On 08/11/2015 05:19 AM, Jan Beulich wrote:
>>>>> On 24.07.15 at 19:54, <boris.ostrovsky@oracle.com> wrote:
>>> On 07/23/2015 10:07 AM, Jan Beulich wrote:
>>>> Plus - is this in line with what the tools are doing? Aren't they
>>>> assuming !PV <=> native format context? I.e. don't you need
>>>> to treat differently v->domain == current->domain and its
>>>> opposite? Roger btw. raised a similar question on IRC earlier
>>>> today...
>>> Not sure I understand this. You mean for copying 64-bit guest's info
>>> into 32-bit dom0?
>> Basically yes - tool stack and guest invocations may need to
>> behave differently.
>
> This being PVH-"classic" it follows exactly the PV path (both in tools
> and the hypervisor). Wouldn't PV be broken then as well?
Note that I raised a question originally (still seen above) instead
of asking for a specific change. In the end all I'm asking for is that
you make changes in the hypervisor in a way compaible with tools
expectations, or adjust the tools accordingly. And of course you
should keep in mind what "no-dm" will want (i.e. perhaps sync with
Roger), such that we don't end up with guest exposed interface
behavior not suitable for the long term targets we have.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode
2015-08-12 6:23 ` Jan Beulich
@ 2015-08-12 15:02 ` Boris Ostrovsky
0 siblings, 0 replies; 17+ messages in thread
From: Boris Ostrovsky @ 2015-08-12 15:02 UTC (permalink / raw)
To: Jan Beulich
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, roger.pau
On 08/12/2015 02:23 AM, Jan Beulich wrote:
>>>> On 11.08.15 at 19:21, <boris.ostrovsky@oracle.com> wrote:
>> On 08/11/2015 05:19 AM, Jan Beulich wrote:
>>>>>> On 24.07.15 at 19:54, <boris.ostrovsky@oracle.com> wrote:
>>>> On 07/23/2015 10:07 AM, Jan Beulich wrote:
>>>>> Plus - is this in line with what the tools are doing? Aren't they
>>>>> assuming !PV <=> native format context? I.e. don't you need
>>>>> to treat differently v->domain == current->domain and its
>>>>> opposite? Roger btw. raised a similar question on IRC earlier
>>>>> today...
>>>> Not sure I understand this. You mean for copying 64-bit guest's info
>>>> into 32-bit dom0?
>>> Basically yes - tool stack and guest invocations may need to
>>> behave differently.
>> This being PVH-"classic" it follows exactly the PV path (both in tools
>> and the hypervisor). Wouldn't PV be broken then as well?
> Note that I raised a question originally (still seen above) instead
> of asking for a specific change. In the end all I'm asking for is that
> you make changes in the hypervisor in a way compaible with tools
> expectations, or adjust the tools accordingly.
So the tools have been adjusted (in the 32-bit path) to make PVH behave
similarly to PV. This is obviously not what we need to move forward with
no-dm PVH but this whole series is really tries to bring 32-bit PVH to
parity with (existing) 64-bit PVH.
> And of course you
> should keep in mind what "no-dm" will want (i.e. perhaps sync with
> Roger), such that we don't end up with guest exposed interface
> behavior not suitable for the long term targets we have.
I suspect the notion of is_pvh_32bit_domain() (which is what this patch
adds) will be irrelevant in no-dm world.
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b PVH guests
2015-07-10 22:20 [PATCH v3 0/4] 32-bit domU PVH support Boris Ostrovsky
2015-07-10 22:20 ` [PATCH v3 1/4] x86/pvh: Set 32b PVH guest mode in XEN_DOMCTL_set_address_size Boris Ostrovsky
2015-07-10 22:20 ` [PATCH v3 2/4] x86/compat: Test both PV and PVH guests for compat mode Boris Ostrovsky
@ 2015-07-10 22:20 ` Boris Ostrovsky
2015-07-23 14:21 ` Jan Beulich
2015-07-10 22:20 ` [PATCH v3 4/4] libxc/x86/pvh: Allow creation of " Boris Ostrovsky
3 siblings, 1 reply; 17+ messages in thread
From: Boris Ostrovsky @ 2015-07-10 22:20 UTC (permalink / raw)
To: xen-devel
Cc: elena.ufimtseva, wei.liu2, ian.campbell, andrew.cooper3,
stefano.stabellini, ian.jackson, jbeulich, boris.ostrovsky,
roger.pau
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v3:
* Defined compat_mmuext_op(). (XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t)
is not defined in header files so I used 'void' type. I am not convinced
this is the best solution)
xen/arch/x86/hvm/hvm.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 6f247a0..a4e7185 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4931,7 +4931,6 @@ static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = {
[ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
};
-/* PVH 32bitfixme. */
static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
HYPERCALL(platform_op),
HYPERCALL(memory_op),
@@ -4951,6 +4950,29 @@ static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
[ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
};
+extern int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) cmp_uops,
+ unsigned int count,
+ XEN_GUEST_HANDLE_PARAM(uint) pdone,
+ unsigned int foreigndom);
+static hvm_hypercall_t *const pvh_hypercall32_table[NR_hypercalls] = {
+ HYPERCALL(platform_op),
+ COMPAT_CALL(memory_op),
+ HYPERCALL(xen_version),
+ HYPERCALL(console_io),
+ [ __HYPERVISOR_grant_table_op ] =
+ (hvm_hypercall_t *)hvm_grant_table_op_compat32,
+ COMPAT_CALL(vcpu_op),
+ COMPAT_CALL(mmuext_op),
+ HYPERCALL(xsm_op),
+ COMPAT_CALL(sched_op),
+ HYPERCALL(event_channel_op),
+ [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
+ HYPERCALL(hvm_op),
+ HYPERCALL(sysctl),
+ HYPERCALL(domctl),
+ [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
+};
+
extern const uint8_t hypercall_args_table[], compat_hypercall_args_table[];
int hvm_do_hypercall(struct cpu_user_regs *regs)
@@ -4981,7 +5003,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
return viridian_hypercall(regs);
if ( (eax >= NR_hypercalls) ||
- (is_pvh_domain(currd) ? !pvh_hypercall64_table[eax]
+ (is_pvh_domain(currd) ? !pvh_hypercall32_table[eax]
: !hvm_hypercall32_table[eax]) )
{
regs->eax = -ENOSYS;
@@ -5037,8 +5059,6 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
}
#endif
}
- else if ( unlikely(is_pvh_domain(currd)) )
- regs->_eax = -ENOSYS; /* PVH 32bitfixme. */
else
{
unsigned int ebx = regs->_ebx;
@@ -5064,7 +5084,10 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
}
#endif
- regs->_eax = hvm_hypercall32_table[eax](ebx, ecx, edx, esi, edi, ebp);
+ regs->_eax = (is_pvh_vcpu(curr)
+ ? pvh_hypercall32_table
+ : hvm_hypercall32_table)[eax](ebx, ecx, edx,
+ esi, edi, ebp);
#ifndef NDEBUG
if ( !curr->arch.hvm_vcpu.hcall_preempted )
--
1.8.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b PVH guests
2015-07-10 22:20 ` [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b PVH guests Boris Ostrovsky
@ 2015-07-23 14:21 ` Jan Beulich
2015-07-24 18:35 ` Boris Ostrovsky
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2015-07-23 14:21 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, roger.pau
>>> On 11.07.15 at 00:20, <boris.ostrovsky@oracle.com> wrote:
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> Changes in v3:
> * Defined compat_mmuext_op(). (XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t)
> is not defined in header files so I used 'void' type.
How is it not? It's in compat/xen.h (which is a generated header).
> @@ -4951,6 +4950,29 @@ static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
> [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
> };
>
> +extern int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) cmp_uops,
> + unsigned int count,
> + XEN_GUEST_HANDLE_PARAM(uint) pdone,
> + unsigned int foreigndom);
> +static hvm_hypercall_t *const pvh_hypercall32_table[NR_hypercalls] = {
> + HYPERCALL(platform_op),
> + COMPAT_CALL(memory_op),
> + HYPERCALL(xen_version),
> + HYPERCALL(console_io),
> + [ __HYPERVISOR_grant_table_op ] =
> + (hvm_hypercall_t *)hvm_grant_table_op_compat32,
> + COMPAT_CALL(vcpu_op),
> + COMPAT_CALL(mmuext_op),
> + HYPERCALL(xsm_op),
> + COMPAT_CALL(sched_op),
> + HYPERCALL(event_channel_op),
> + [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
> + HYPERCALL(hvm_op),
> + HYPERCALL(sysctl),
> + HYPERCALL(domctl),
> + [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
Looks like you didn't fully sync with staging - did you forget that it
was you who added xenpmu_op to the 64-bit counterpart?
Without that ...
> @@ -4981,7 +5003,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
> return viridian_hypercall(regs);
>
> if ( (eax >= NR_hypercalls) ||
> - (is_pvh_domain(currd) ? !pvh_hypercall64_table[eax]
> + (is_pvh_domain(currd) ? !pvh_hypercall32_table[eax]
> : !hvm_hypercall32_table[eax]) )
... this will break (as we're assuming 32- and 64-bit tables to be fully
in sync here; there's still the pending work item of constructing these
tables so that this has a better chance of not getting broken).
Also, since you're touching this here anyway, could I ask you to
adjust this along the lines of the change you do further down,
i.e. to become
!(is_pvh_domain(currd) ? pvh_hypercall32_table
: hvm_hypercall32_table)[eax] )
Thanks, Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b PVH guests
2015-07-23 14:21 ` Jan Beulich
@ 2015-07-24 18:35 ` Boris Ostrovsky
2015-08-11 9:32 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Boris Ostrovsky @ 2015-07-24 18:35 UTC (permalink / raw)
To: Jan Beulich
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, roger.pau
On 07/23/2015 10:21 AM, Jan Beulich wrote:
>>>> On 11.07.15 at 00:20, <boris.ostrovsky@oracle.com> wrote:
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>> Changes in v3:
>> * Defined compat_mmuext_op(). (XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t)
>> is not defined in header files so I used 'void' type.
> How is it not? It's in compat/xen.h (which is a generated header).
compat/xen.h has DEFINE_COMPAT_HANDLE(mmuext_op_compat_t) (which is
__compat_handle_mmuext_op_compat_t).
We need XEN_GUEST_HANDLE(mmuext_op_compat_t), which is
__guest_handle_mmuext_op_compat_t. And I wasn't sure it's worth
explicitly adding it to a header file (like I think what we do for
vcpu_runstate_info_compat_t in sched.h);
>
>> @@ -4951,6 +4950,29 @@ static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
>> [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
>> };
>>
>> +extern int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) cmp_uops,
>> + unsigned int count,
>> + XEN_GUEST_HANDLE_PARAM(uint) pdone,
>> + unsigned int foreigndom);
>> +static hvm_hypercall_t *const pvh_hypercall32_table[NR_hypercalls] = {
>> + HYPERCALL(platform_op),
>> + COMPAT_CALL(memory_op),
>> + HYPERCALL(xen_version),
>> + HYPERCALL(console_io),
>> + [ __HYPERVISOR_grant_table_op ] =
>> + (hvm_hypercall_t *)hvm_grant_table_op_compat32,
>> + COMPAT_CALL(vcpu_op),
>> + COMPAT_CALL(mmuext_op),
>> + HYPERCALL(xsm_op),
>> + COMPAT_CALL(sched_op),
>> + HYPERCALL(event_channel_op),
>> + [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
>> + HYPERCALL(hvm_op),
>> + HYPERCALL(sysctl),
>> + HYPERCALL(domctl),
>> + [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
> Looks like you didn't fully sync with staging - did you forget that it
> was you who added xenpmu_op to the 64-bit counterpart?
I think I posted this before VPMU got committed. But yes...
> Without that ...
>
>> @@ -4981,7 +5003,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>> return viridian_hypercall(regs);
>>
>> if ( (eax >= NR_hypercalls) ||
>> - (is_pvh_domain(currd) ? !pvh_hypercall64_table[eax]
>> + (is_pvh_domain(currd) ? !pvh_hypercall32_table[eax]
>> : !hvm_hypercall32_table[eax]) )
> ... this will break (as we're assuming 32- and 64-bit tables to be fully
> in sync here; there's still the pending work item of constructing these
> tables so that this has a better chance of not getting broken).
So you prefer to have full check --- explicitly for both 32- and 64-bit,
right?
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b PVH guests
2015-07-24 18:35 ` Boris Ostrovsky
@ 2015-08-11 9:32 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2015-08-11 9:32 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: elena.ufimtseva, wei.liu2, ian.campbell, stefano.stabellini,
andrew.cooper3, ian.jackson, xen-devel, roger.pau
>>> On 24.07.15 at 20:35, <boris.ostrovsky@oracle.com> wrote:
> On 07/23/2015 10:21 AM, Jan Beulich wrote:
>>>>> On 11.07.15 at 00:20, <boris.ostrovsky@oracle.com> wrote:
>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> ---
>>> Changes in v3:
>>> * Defined compat_mmuext_op(). (XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t)
>>> is not defined in header files so I used 'void' type.
>> How is it not? It's in compat/xen.h (which is a generated header).
>
> compat/xen.h has DEFINE_COMPAT_HANDLE(mmuext_op_compat_t) (which is
> __compat_handle_mmuext_op_compat_t).
>
> We need XEN_GUEST_HANDLE(mmuext_op_compat_t), which is
> __guest_handle_mmuext_op_compat_t. And I wasn't sure it's worth
> explicitly adding it to a header file (like I think what we do for
> vcpu_runstate_info_compat_t in sched.h);
Hmm, indeed all other compat_..._op()-s use void handles (albeit in
most if not all of the cases their native counterparts do too). So I
guess using void here is fine then, or using COMPAT_HANDLE()
instead. It's not really relevant anyway since COMPAT_CALL()
casts the function pointer to the intended type anyway.
>>> @@ -4981,7 +5003,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>>> return viridian_hypercall(regs);
>>>
>>> if ( (eax >= NR_hypercalls) ||
>>> - (is_pvh_domain(currd) ? !pvh_hypercall64_table[eax]
>>> + (is_pvh_domain(currd) ? !pvh_hypercall32_table[eax]
>>> : !hvm_hypercall32_table[eax]) )
>> ... this will break (as we're assuming 32- and 64-bit tables to be fully
>> in sync here; there's still the pending work item of constructing these
>> tables so that this has a better chance of not getting broken).
>
> So you prefer to have full check --- explicitly for both 32- and 64-bit,
> right?
No. Just adding the missing operation to the table will deal with it.
I wouldn't like to see more conditionals to be added to this code
path when we can avoid doing so. What we could do is add a
respective ASSERT() to the 64-bit path, albeit the NULL deref
would be observable as a fault without the ASSERT() too (and
adding one wouldn't help release builds [and their security]).
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 4/4] libxc/x86/pvh: Allow creation of 32b PVH guests
2015-07-10 22:20 [PATCH v3 0/4] 32-bit domU PVH support Boris Ostrovsky
` (2 preceding siblings ...)
2015-07-10 22:20 ` [PATCH v3 3/4] x86/pvh: Handle hypercalls for 32b PVH guests Boris Ostrovsky
@ 2015-07-10 22:20 ` Boris Ostrovsky
3 siblings, 0 replies; 17+ messages in thread
From: Boris Ostrovsky @ 2015-07-10 22:20 UTC (permalink / raw)
To: xen-devel
Cc: elena.ufimtseva, wei.liu2, ian.campbell, andrew.cooper3,
stefano.stabellini, ian.jackson, jbeulich, boris.ostrovsky,
roger.pau
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxc/xc_dom_x86.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 6a04399..d0723f1 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -301,7 +301,8 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
pgpfn = (addr - dom->parms.virt_base) >> PAGE_SHIFT_X86;
l1tab[l1off] =
pfn_to_paddr(xc_dom_p2m_guest(dom, pgpfn)) | L1_PROT;
- if ( (addr >= dom->pgtables_seg.vstart) &&
+ if ( (!dom->pvh_enabled) &&
+ (addr >= dom->pgtables_seg.vstart) &&
(addr < dom->pgtables_seg.vend) )
l1tab[l1off] &= ~_PAGE_RW; /* page tables are r/o */
@@ -591,22 +592,9 @@ static int vcpu_x86_32(struct xc_dom_image *dom, void *ptr)
DOMPRINTF_CALLED(dom->xch);
- if ( dom->pvh_enabled )
- {
- xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
- "%s: PVH not supported for 32bit guests.", __FUNCTION__);
- return -1;
- }
-
/* clear everything */
memset(ctxt, 0, sizeof(*ctxt));
- ctxt->user_regs.ds = FLAT_KERNEL_DS_X86_32;
- ctxt->user_regs.es = FLAT_KERNEL_DS_X86_32;
- ctxt->user_regs.fs = FLAT_KERNEL_DS_X86_32;
- ctxt->user_regs.gs = FLAT_KERNEL_DS_X86_32;
- ctxt->user_regs.ss = FLAT_KERNEL_SS_X86_32;
- ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_32;
ctxt->user_regs.eip = dom->parms.virt_entry;
ctxt->user_regs.esp =
dom->parms.virt_base + (dom->bootstack_pfn + 1) * PAGE_SIZE_X86;
@@ -614,9 +602,6 @@ static int vcpu_x86_32(struct xc_dom_image *dom, void *ptr)
dom->parms.virt_base + (dom->start_info_pfn) * PAGE_SIZE_X86;
ctxt->user_regs.eflags = 1 << 9; /* Interrupt Enable */
- ctxt->kernel_ss = ctxt->user_regs.ss;
- ctxt->kernel_sp = ctxt->user_regs.esp;
-
ctxt->flags = VGCF_in_kernel_X86_32 | VGCF_online_X86_32;
if ( dom->parms.pae == 2 /* extended_cr3 */ ||
dom->parms.pae == 3 /* bimodal */ )
@@ -627,6 +612,19 @@ static int vcpu_x86_32(struct xc_dom_image *dom, void *ptr)
DOMPRINTF("%s: cr3: pfn 0x%" PRIpfn " mfn 0x%" PRIpfn "",
__FUNCTION__, dom->pgtables_seg.pfn, cr3_pfn);
+ if ( dom->pvh_enabled )
+ return 0;
+
+ ctxt->user_regs.ds = FLAT_KERNEL_DS_X86_32;
+ ctxt->user_regs.es = FLAT_KERNEL_DS_X86_32;
+ ctxt->user_regs.fs = FLAT_KERNEL_DS_X86_32;
+ ctxt->user_regs.gs = FLAT_KERNEL_DS_X86_32;
+ ctxt->user_regs.ss = FLAT_KERNEL_SS_X86_32;
+ ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_32;
+
+ ctxt->kernel_ss = ctxt->user_regs.ss;
+ ctxt->kernel_sp = ctxt->user_regs.esp;
+
return 0;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread