* [PATCH 1/4] x86: Begin to remove TSC mode PVRDTSCP
2018-11-15 21:47 [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Andrew Cooper
@ 2018-11-15 21:47 ` Andrew Cooper
2018-11-19 15:25 ` Jan Beulich
2018-11-15 21:47 ` [PATCH 2/4] x86/pv: Remove deferred RDTSC{, P} handling in pv_emulate_privileged_op() Andrew Cooper
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2018-11-15 21:47 UTC (permalink / raw)
To: Xen-devel
Cc: Wei Liu, Konrad Rzeszutek Wilk, Andrew Cooper, Jan Beulich,
Boris Ostrovsky, Roger Pau Monné
For more historical context, see
c/s c17b36d5dc792cfdf59b6de0213b168bec0af8e8
c/s 04656384a1b9714e43db850c51431008e23450d8
PVRDTSCP was an attempt to provide Xen-aware userspace with a stable monotonic
clock, and enough information for said userspace to cope with frequency
changes across migrate. However, the PVRDTSCP infrastructure has resulted in
very tangled code, and non-architectural behaviour even in non-PVRDTSCP cases.
Seeing as the functionality has been replaced entirely by improvements in PV
clocks (including being plumbed into the VDSO nowadays), or alternatively by
hardware TSC scaling features, and no-one is aware of any remaining users of
this mode, take the opportunity to remove it.
For now, introduce an upper range check on the toolstack setting to
XEN_DOMCTL_settscinfo, and exclude TSC_MODE_PVRDTSCP from selection.
(Arguably, its a bug that this hypercall previously accepted any value and
turned into a nop). This will catch and cleanly reject attempts to migrate in
a VM configured to use PVRDTSCP, rather than letting it run and have the wrong
timing mode.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/arch/x86/domctl.c | 3 ++-
xen/arch/x86/time.c | 35 -----------------------------------
xen/include/asm-x86/time.h | 5 +----
3 files changed, 3 insertions(+), 40 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 175a0c9..97ea5d8 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -970,7 +970,8 @@ long arch_do_domctl(
break;
case XEN_DOMCTL_settscinfo:
- if ( d == currd ) /* no domain_pause() */
+ if ( d == currd || /* no domain_pause() */
+ domctl->u.tsc_info.tsc_mode > TSC_MODE_NEVER_EMULATE )
ret = -EINVAL;
else
{
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 24d4c27..3f095a2 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2165,21 +2165,6 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
*elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns);
*gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : cpu_khz;
break;
- case TSC_MODE_PVRDTSCP:
- if ( d->arch.vtsc )
- {
- *elapsed_nsec = get_s_time() - d->arch.vtsc_offset;
- *gtsc_khz = cpu_khz;
- }
- else
- {
- tsc = rdtsc();
- *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
- d->arch.vtsc_offset;
- *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz
- : 0 /* ignored by tsc_set_info */;
- }
- break;
}
if ( (int64_t)*elapsed_nsec < 0 )
@@ -2208,8 +2193,6 @@ void tsc_set_info(struct domain *d,
switch ( d->arch.tsc_mode = tsc_mode )
{
- bool enable_tsc_scaling;
-
case TSC_MODE_DEFAULT:
case TSC_MODE_ALWAYS_EMULATE:
d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
@@ -2235,24 +2218,6 @@ void tsc_set_info(struct domain *d,
d->arch.vtsc = 1;
d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
break;
- case TSC_MODE_PVRDTSCP:
- d->arch.vtsc = !boot_cpu_has(X86_FEATURE_RDTSCP) ||
- !host_tsc_is_safe();
- enable_tsc_scaling = is_hvm_domain(d) && !d->arch.vtsc &&
- hvm_get_tsc_scaling_ratio(gtsc_khz ?: cpu_khz);
- d->arch.tsc_khz = (enable_tsc_scaling && gtsc_khz) ? gtsc_khz : cpu_khz;
- set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000 );
- d->arch.ns_to_vtsc = scale_reciprocal(d->arch.vtsc_to_ns);
- if ( d->arch.vtsc )
- d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
- else {
- /* when using native TSC, offset is nsec relative to power-on
- * of physical machine */
- d->arch.vtsc_offset = scale_delta(rdtsc(),
- &this_cpu(cpu_time).tsc_scale) -
- elapsed_nsec;
- }
- break;
}
d->arch.incarnation = incarnation + 1;
if ( is_hvm_domain(d) )
diff --git a/xen/include/asm-x86/time.h b/xen/include/asm-x86/time.h
index ce96ec9..070cdef 100644
--- a/xen/include/asm-x86/time.h
+++ b/xen/include/asm-x86/time.h
@@ -12,10 +12,7 @@
* 2 = guest rdtsc always executed natively (no monotonicity/frequency
* guarantees); guest rdtscp emulated at native frequency if
* unsupported by h/w, else executed natively
- * 3 = same as 2, except xen manages TSC_AUX register so guest can
- * determine when a restore/migration has occurred and assumes
- * guest obtains/uses pvclock-like mechanism to adjust for
- * monotonicity and frequency changes
+ * 3 = Removed, was PVRDTSCP.
*/
#define TSC_MODE_DEFAULT 0
#define TSC_MODE_ALWAYS_EMULATE 1
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/4] x86: Begin to remove TSC mode PVRDTSCP
2018-11-15 21:47 ` [PATCH 1/4] x86: Begin to remove TSC mode PVRDTSCP Andrew Cooper
@ 2018-11-19 15:25 ` Jan Beulich
2018-11-20 11:14 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2018-11-19 15:25 UTC (permalink / raw)
To: Andrew Cooper
Cc: Boris Ostrovsky, Xen-devel, Wei Liu, Konrad Rzeszutek Wilk,
Roger Pau Monne
>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
> For more historical context, see
> c/s c17b36d5dc792cfdf59b6de0213b168bec0af8e8
> c/s 04656384a1b9714e43db850c51431008e23450d8
>
> PVRDTSCP was an attempt to provide Xen-aware userspace with a stable monotonic
> clock, and enough information for said userspace to cope with frequency
> changes across migrate. However, the PVRDTSCP infrastructure has resulted in
> very tangled code, and non-architectural behaviour even in non-PVRDTSCP
> cases.
>
> Seeing as the functionality has been replaced entirely by improvements in PV
> clocks (including being plumbed into the VDSO nowadays), or alternatively by
> hardware TSC scaling features, and no-one is aware of any remaining users of
> this mode, take the opportunity to remove it.
With "remaining" suitably replaced to express that none are known to
ever have existed in the wild ...
> For now, introduce an upper range check on the toolstack setting to
> XEN_DOMCTL_settscinfo, and exclude TSC_MODE_PVRDTSCP from selection.
> (Arguably, its a bug that this hypercall previously accepted any value and
> turned into a nop). This will catch and cleanly reject attempts to migrate in
> a VM configured to use PVRDTSCP, rather than letting it run and have the wrong
> timing mode.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
albeit I think this ...
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -970,7 +970,8 @@ long arch_do_domctl(
> break;
>
> case XEN_DOMCTL_settscinfo:
> - if ( d == currd ) /* no domain_pause() */
> + if ( d == currd || /* no domain_pause() */
> + domctl->u.tsc_info.tsc_mode > TSC_MODE_NEVER_EMULATE )
... would better be left to tsc_set_info().
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] x86: Begin to remove TSC mode PVRDTSCP
2018-11-19 15:25 ` Jan Beulich
@ 2018-11-20 11:14 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2018-11-20 11:14 UTC (permalink / raw)
To: Jan Beulich
Cc: Boris Ostrovsky, Xen-devel, Wei Liu, Konrad Rzeszutek Wilk,
Roger Pau Monne
On 19/11/2018 15:25, Jan Beulich wrote:
>>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
>> For more historical context, see
>> c/s c17b36d5dc792cfdf59b6de0213b168bec0af8e8
>> c/s 04656384a1b9714e43db850c51431008e23450d8
>>
>> PVRDTSCP was an attempt to provide Xen-aware userspace with a stable monotonic
>> clock, and enough information for said userspace to cope with frequency
>> changes across migrate. However, the PVRDTSCP infrastructure has resulted in
>> very tangled code, and non-architectural behaviour even in non-PVRDTSCP
>> cases.
>>
>> Seeing as the functionality has been replaced entirely by improvements in PV
>> clocks (including being plumbed into the VDSO nowadays), or alternatively by
>> hardware TSC scaling features, and no-one is aware of any remaining users of
>> this mode, take the opportunity to remove it.
> With "remaining" suitably replaced to express that none are known to
> ever have existed in the wild ...
>
>> For now, introduce an upper range check on the toolstack setting to
>> XEN_DOMCTL_settscinfo, and exclude TSC_MODE_PVRDTSCP from selection.
>> (Arguably, its a bug that this hypercall previously accepted any value and
>> turned into a nop). This will catch and cleanly reject attempts to migrate in
>> a VM configured to use PVRDTSCP, rather than letting it run and have the wrong
>> timing mode.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> albeit I think this ...
>
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -970,7 +970,8 @@ long arch_do_domctl(
>> break;
>>
>> case XEN_DOMCTL_settscinfo:
>> - if ( d == currd ) /* no domain_pause() */
>> + if ( d == currd || /* no domain_pause() */
>> + domctl->u.tsc_info.tsc_mode > TSC_MODE_NEVER_EMULATE )
> ... would better be left to tsc_set_info().
I think I'll do a separate prerequisite patch which causes
tsc_set_info() not to be void.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] x86/pv: Remove deferred RDTSC{, P} handling in pv_emulate_privileged_op()
2018-11-15 21:47 [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Andrew Cooper
2018-11-15 21:47 ` [PATCH 1/4] x86: Begin to remove TSC mode PVRDTSCP Andrew Cooper
@ 2018-11-15 21:47 ` Andrew Cooper
2018-11-20 8:40 ` Jan Beulich
2018-11-15 21:47 ` [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests Andrew Cooper
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2018-11-15 21:47 UTC (permalink / raw)
To: Xen-devel
Cc: Wei Liu, Konrad Rzeszutek Wilk, Andrew Cooper, Jan Beulich,
Boris Ostrovsky, Roger Pau Monné
As noted in c/s 4999bf3e8b "x86/PV: use generic emulator for privileged
instruction handling", these hoops are jumped through to retain the older
behaviour, along with a note suggesting that we should reconsider things.
It does not matter exactly when pv_soft_rdtsc() is called, as Xen's behaviour
is an opaque atomic action from the guests point of view. Furthermore, even
with PVRDTSCP mode, the TSC_AUX value constant while the domain is executing.
Drop all the deferral logic, and leave TSC_AUX uniformly at 0 as PVRDTSCP mode
is being removed. Later changes will make this behave architecturally.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/arch/x86/pv/emul-priv-op.c | 30 ++----------------------------
1 file changed, 2 insertions(+), 28 deletions(-)
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 83441b6..3641d31 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -51,9 +51,6 @@ struct priv_op_ctxt {
} cs;
char *io_emul_stub;
unsigned int bpmatch;
- unsigned int tsc;
-#define TSC_BASE 1
-#define TSC_AUX 2
};
/* I/O emulation support. Helper routines for, and type of, the stack stub. */
@@ -810,7 +807,6 @@ static inline bool is_cpufreq_controller(const struct domain *d)
static int read_msr(unsigned int reg, uint64_t *val,
struct x86_emulate_ctxt *ctxt)
{
- struct priv_op_ctxt *poc = container_of(ctxt, struct priv_op_ctxt, ctxt);
const struct vcpu *curr = current;
const struct domain *currd = curr->domain;
bool vpmu_msr = false;
@@ -847,19 +843,11 @@ static int read_msr(unsigned int reg, uint64_t *val,
*val = curr->arch.pv.gs_base_user;
return X86EMUL_OKAY;
- /*
- * In order to fully retain original behavior, defer calling
- * pv_soft_rdtsc() until after emulation. This may want/need to be
- * reconsidered.
- */
case MSR_IA32_TSC:
- poc->tsc |= TSC_BASE;
- goto normal;
+ *val = currd->arch.vtsc ? pv_soft_rdtsc(curr, ctxt->regs) : rdtsc();
+ return X86EMUL_OKAY;
case MSR_TSC_AUX:
- poc->tsc |= TSC_AUX;
- if ( cpu_has_rdtscp )
- goto normal;
*val = 0;
return X86EMUL_OKAY;
@@ -1341,20 +1329,6 @@ int pv_emulate_privileged_op(struct cpu_user_regs *regs)
switch ( rc )
{
case X86EMUL_OKAY:
- if ( ctxt.tsc & TSC_BASE )
- {
- if ( currd->arch.vtsc || (ctxt.tsc & TSC_AUX) )
- {
- msr_split(regs, pv_soft_rdtsc(curr, regs));
-
- if ( ctxt.tsc & TSC_AUX )
- regs->rcx = (currd->arch.tsc_mode == TSC_MODE_PVRDTSCP
- ? currd->arch.incarnation : 0);
- }
- else
- msr_split(regs, rdtsc());
- }
-
if ( ctxt.ctxt.retire.singlestep )
ctxt.bpmatch |= DR_STEP;
if ( ctxt.bpmatch )
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] x86/pv: Remove deferred RDTSC{, P} handling in pv_emulate_privileged_op()
2018-11-15 21:47 ` [PATCH 2/4] x86/pv: Remove deferred RDTSC{, P} handling in pv_emulate_privileged_op() Andrew Cooper
@ 2018-11-20 8:40 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2018-11-20 8:40 UTC (permalink / raw)
To: Andrew Cooper
Cc: Boris Ostrovsky, Xen-devel, Wei Liu, Konrad Rzeszutek Wilk,
Roger Pau Monne
>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
> As noted in c/s 4999bf3e8b "x86/PV: use generic emulator for privileged
> instruction handling", these hoops are jumped through to retain the older
> behaviour, along with a note suggesting that we should reconsider things.
>
> It does not matter exactly when pv_soft_rdtsc() is called, as Xen's behaviour
> is an opaque atomic action from the guests point of view. Furthermore, even
> with PVRDTSCP mode, the TSC_AUX value constant while the domain is
> executing.
Not exactly: I've tried to reconstruct the reasons for the comment you
refer to, and I think it was twofold: For one I wanted to keep the actual
RDTSC as close to the point where the guest actually gets to use the
value as possible. And then from an abstract pov the split between two
MSR accesses means that either can fail, yet pv_soft_rdtsc() back at
the time wrote to the guest RCX directly. That second reason is gone
as of 5b04262079 ("x86/time: Rework pv_soft_rdtsc() to aid further
cleanup"), and the first is presumably not overly relevant, so ...
> Drop all the deferral logic, and leave TSC_AUX uniformly at 0 as PVRDTSCP mode
> is being removed. Later changes will make this behave architecturally.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests
2018-11-15 21:47 [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Andrew Cooper
2018-11-15 21:47 ` [PATCH 1/4] x86: Begin to remove TSC mode PVRDTSCP Andrew Cooper
2018-11-15 21:47 ` [PATCH 2/4] x86/pv: Remove deferred RDTSC{, P} handling in pv_emulate_privileged_op() Andrew Cooper
@ 2018-11-15 21:47 ` Andrew Cooper
2018-11-20 10:58 ` Jan Beulich
2018-11-26 15:28 ` Woods, Brian
2018-11-15 21:47 ` [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests Andrew Cooper
2018-11-16 10:11 ` [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Jan Beulich
4 siblings, 2 replies; 17+ messages in thread
From: Andrew Cooper @ 2018-11-15 21:47 UTC (permalink / raw)
To: Xen-devel
Cc: Kevin Tian, Wei Liu, Jan Beulich, Konrad Rzeszutek Wilk,
Andrew Cooper, Jun Nakajima, Boris Ostrovsky, Brian Woods,
Suravee Suthikulpanit, Roger Pau Monné
With PVRDTSCP mode removed, handling of MSR_TSC_AUX can move into the common
code. Move its storage into struct vcpu_msrs (dropping the HVM-specific
msr_tsc_aux), and add an RDPID feature check as this bit also enumerates the
presence of the MSR.
Drop hvm_msr_tsc_aux() entirely, and use v->arch.msrs->tsc_aux directly.
Update hvm_load_cpu_ctxt() to check that the incoming ctxt.msr_tsc_aux isn't
out of range. In practice, no previous version of Xen ever wrote an
out-of-range value. Add MSR_TSC_AUX to the list of MSRs migrated for PV
guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
CC: Brian Woods <brian.woods@amd.com>
---
xen/arch/x86/domain.c | 3 +--
xen/arch/x86/domctl.c | 2 ++
xen/arch/x86/hvm/hvm.c | 18 +++++-------------
xen/arch/x86/hvm/svm/svm.c | 4 ++--
xen/arch/x86/hvm/vmx/vmx.c | 4 ++--
xen/arch/x86/msr.c | 18 ++++++++++++++++++
xen/arch/x86/pv/emul-priv-op.c | 4 ----
xen/include/asm-x86/hvm/hvm.h | 6 ------
xen/include/asm-x86/hvm/vcpu.h | 1 -
xen/include/asm-x86/msr.h | 8 ++++++++
10 files changed, 38 insertions(+), 30 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 295b10c..2067a0c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1593,8 +1593,7 @@ void paravirt_ctxt_switch_to(struct vcpu *v)
activate_debugregs(v);
if ( cpu_has_rdtscp )
- wrmsr_tsc_aux(v->domain->arch.tsc_mode == TSC_MODE_PVRDTSCP
- ? v->domain->arch.incarnation : 0);
+ wrmsr_tsc_aux(v->arch.msrs->tsc_aux);
}
/* Update per-VCPU guest runstate shared memory area (if registered). */
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 97ea5d8..b8d0796 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1275,6 +1275,7 @@ long arch_do_domctl(
static const uint32_t msrs_to_send[] = {
MSR_SPEC_CTRL,
MSR_INTEL_MISC_FEATURES_ENABLES,
+ MSR_TSC_AUX,
};
uint32_t nr_msrs = ARRAY_SIZE(msrs_to_send);
@@ -1399,6 +1400,7 @@ long arch_do_domctl(
{
case MSR_SPEC_CTRL:
case MSR_INTEL_MISC_FEATURES_ENABLES:
+ case MSR_TSC_AUX:
if ( guest_wrmsr(v, msr.index, msr.value) != X86EMUL_OKAY )
break;
continue;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0bc676c..1e4fc7d 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -774,7 +774,7 @@ static int hvm_save_cpu_ctxt(struct vcpu *v, hvm_domain_context_t *h)
struct segment_register seg;
struct hvm_hw_cpu ctxt = {
.tsc = hvm_get_guest_tsc_fixed(v, v->domain->arch.hvm.sync_tsc),
- .msr_tsc_aux = hvm_msr_tsc_aux(v),
+ .msr_tsc_aux = v->arch.msrs->tsc_aux,
.rax = v->arch.user_regs.rax,
.rbx = v->arch.user_regs.rbx,
.rcx = v->arch.user_regs.rcx,
@@ -1040,7 +1040,10 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
if ( hvm_funcs.tsc_scaling.setup )
hvm_funcs.tsc_scaling.setup(v);
- v->arch.hvm.msr_tsc_aux = ctxt.msr_tsc_aux;
+ if ( ctxt.msr_tsc_aux != (uint32_t)ctxt.msr_tsc_aux )
+ return -EINVAL;
+
+ v->arch.msrs->tsc_aux = ctxt.msr_tsc_aux;
hvm_set_guest_tsc_fixed(v, ctxt.tsc, d->arch.hvm.sync_tsc);
@@ -3400,10 +3403,6 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
*msr_content = v->arch.hvm.msr_tsc_adjust;
break;
- case MSR_TSC_AUX:
- *msr_content = hvm_msr_tsc_aux(v);
- break;
-
case MSR_IA32_APICBASE:
*msr_content = vcpu_vlapic(v)->hw.apic_base_msr;
break;
@@ -3556,13 +3555,6 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t msr_content,
hvm_set_guest_tsc_adjust(v, msr_content);
break;
- case MSR_TSC_AUX:
- v->arch.hvm.msr_tsc_aux = (uint32_t)msr_content;
- if ( cpu_has_rdtscp
- && (v->domain->arch.tsc_mode != TSC_MODE_PVRDTSCP) )
- wrmsr_tsc_aux(msr_content);
- break;
-
case MSR_IA32_APICBASE:
if ( !vlapic_msr_set(vcpu_vlapic(v), msr_content) )
goto gp_fault;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 396ee4a..e42e152 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1136,7 +1136,7 @@ static void svm_ctxt_switch_to(struct vcpu *v)
svm_tsc_ratio_load(v);
if ( cpu_has_rdtscp )
- wrmsr_tsc_aux(hvm_msr_tsc_aux(v));
+ wrmsr_tsc_aux(v->arch.msrs->tsc_aux);
}
static void noreturn svm_do_resume(struct vcpu *v)
@@ -3063,7 +3063,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
break;
case VMEXIT_RDTSCP:
- regs->rcx = hvm_msr_tsc_aux(v);
+ regs->rcx = v->arch.msrs->tsc_aux;
/* fall through */
case VMEXIT_RDTSC:
svm_vmexit_do_rdtsc(regs);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 365eeb2..ea9694a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -512,7 +512,7 @@ static void vmx_restore_guest_msrs(struct vcpu *v)
wrmsrl(MSR_SYSCALL_MASK, v->arch.hvm.vmx.sfmask);
if ( cpu_has_rdtscp )
- wrmsr_tsc_aux(hvm_msr_tsc_aux(v));
+ wrmsr_tsc_aux(v->arch.msrs->tsc_aux);
}
void vmx_update_cpu_exec_control(struct vcpu *v)
@@ -3956,7 +3956,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
vmx_invlpg_intercept(exit_qualification);
break;
case EXIT_REASON_RDTSCP:
- regs->rcx = hvm_msr_tsc_aux(v);
+ regs->rcx = v->arch.msrs->tsc_aux;
/* fall through */
case EXIT_REASON_RDTSC:
update_guest_eip(); /* Safe: RDTSC, RDTSCP */
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index c9e87b1..ba1ce29 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -159,6 +159,13 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
ret = guest_rdmsr_xen(v, msr, val);
break;
+ case MSR_TSC_AUX:
+ if ( !cp->extd.rdtscp && !cp->feat.rdpid )
+ goto gp_fault;
+
+ *val = msrs->tsc_aux;
+ break;
+
default:
return X86EMUL_UNHANDLEABLE;
}
@@ -285,6 +292,17 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
ret = guest_wrmsr_xen(v, msr, val);
break;
+ case MSR_TSC_AUX:
+ if ( !cp->extd.rdtscp && !cp->feat.rdpid )
+ goto gp_fault;
+ if ( val != (uint32_t)val )
+ goto gp_fault;
+
+ msrs->tsc_aux = val;
+ if ( v == curr )
+ wrmsr_tsc_aux(val);
+ break;
+
default:
return X86EMUL_UNHANDLEABLE;
}
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 3641d31..7c2b635 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -847,10 +847,6 @@ static int read_msr(unsigned int reg, uint64_t *val,
*val = currd->arch.vtsc ? pv_soft_rdtsc(curr, ctxt->regs) : rdtsc();
return X86EMUL_OKAY;
- case MSR_TSC_AUX:
- *val = 0;
- return X86EMUL_OKAY;
-
case MSR_EFER:
/* Hide unknown bits, and unconditionally hide SVME from guests. */
*val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME;
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 3d3250d..3a92bb3 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -563,12 +563,6 @@ static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
#endif
}
-#define hvm_msr_tsc_aux(v) ({ \
- struct domain *__d = (v)->domain; \
- (__d->arch.tsc_mode == TSC_MODE_PVRDTSCP) \
- ? (u32)__d->arch.incarnation : (u32)(v)->arch.hvm.msr_tsc_aux; \
-})
-
/*
* Nested HVM
*/
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index c663155..1d2f407 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -171,7 +171,6 @@ struct hvm_vcpu {
struct hvm_vcpu_asid n1asid;
- u32 msr_tsc_aux;
u64 msr_tsc_adjust;
u64 msr_xss;
diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
index c1cb38f..9d0d52b 100644
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -289,6 +289,14 @@ struct vcpu_msrs
} misc_features_enables;
/*
+ * 0xc0000103 - MSR_TSC_AUX
+ *
+ * Value is guest chosen, and eagerly loaded in guest context. The value
+ * is accessible to userspace with the RDTSCP and RDPID instructions.
+ */
+ uint32_t tsc_aux;
+
+ /*
* 0xc00110{27,19-1b} MSR_AMD64_DR{0-3}_ADDRESS_MASK
* TODO: Not yet handled by guest_{rd,wr}msr() infrastructure.
*/
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests
2018-11-15 21:47 ` [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests Andrew Cooper
@ 2018-11-20 10:58 ` Jan Beulich
2018-11-26 15:28 ` Woods, Brian
1 sibling, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2018-11-20 10:58 UTC (permalink / raw)
To: Andrew Cooper
Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit,
Konrad Rzeszutek Wilk, Xen-devel, Jun Nakajima, Boris Ostrovsky,
Brian Woods, Roger Pau Monne
>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
> @@ -1040,7 +1040,10 @@ static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> if ( hvm_funcs.tsc_scaling.setup )
> hvm_funcs.tsc_scaling.setup(v);
>
> - v->arch.hvm.msr_tsc_aux = ctxt.msr_tsc_aux;
> + if ( ctxt.msr_tsc_aux != (uint32_t)ctxt.msr_tsc_aux )
> + return -EINVAL;
> +
> + v->arch.msrs->tsc_aux = ctxt.msr_tsc_aux;
The check (but not the setting of the value) wants to move up,
next to the other error returns. We should at least try to avoid
(where possible) failures getting reported after part of the state
was already modified.
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -512,7 +512,7 @@ static void vmx_restore_guest_msrs(struct vcpu *v)
> wrmsrl(MSR_SYSCALL_MASK, v->arch.hvm.vmx.sfmask);
>
> if ( cpu_has_rdtscp )
> - wrmsr_tsc_aux(hvm_msr_tsc_aux(v));
> + wrmsr_tsc_aux(v->arch.msrs->tsc_aux);
Any reason you don't also add an RDPID feature check here?
With the first issue taken care of and the second at least
explained,
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests
2018-11-15 21:47 ` [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests Andrew Cooper
2018-11-20 10:58 ` Jan Beulich
@ 2018-11-26 15:28 ` Woods, Brian
1 sibling, 0 replies; 17+ messages in thread
From: Woods, Brian @ 2018-11-26 15:28 UTC (permalink / raw)
To: Andrew Cooper
Cc: Kevin Tian, Wei Liu, Jan Beulich, Konrad Rzeszutek Wilk,
Xen-devel, Jun Nakajima, Boris Ostrovsky, Woods, Brian,
Suthikulpanit, Suravee, Roger Pau Monné
On Thu, Nov 15, 2018 at 09:47:17PM +0000, Andy Cooper wrote:
> With PVRDTSCP mode removed, handling of MSR_TSC_AUX can move into the common
> code. Move its storage into struct vcpu_msrs (dropping the HVM-specific
> msr_tsc_aux), and add an RDPID feature check as this bit also enumerates the
> presence of the MSR.
>
> Drop hvm_msr_tsc_aux() entirely, and use v->arch.msrs->tsc_aux directly.
> Update hvm_load_cpu_ctxt() to check that the incoming ctxt.msr_tsc_aux isn't
> out of range. In practice, no previous version of Xen ever wrote an
> out-of-range value. Add MSR_TSC_AUX to the list of MSRs migrated for PV
> guests.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Brian Woods <brian.woods@amd.com>
--
Brian Woods
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests
2018-11-15 21:47 [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Andrew Cooper
` (2 preceding siblings ...)
2018-11-15 21:47 ` [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests Andrew Cooper
@ 2018-11-15 21:47 ` Andrew Cooper
2018-11-20 11:06 ` Jan Beulich
2018-11-16 10:11 ` [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Jan Beulich
4 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2018-11-15 21:47 UTC (permalink / raw)
To: Xen-devel
Cc: Wei Liu, Konrad Rzeszutek Wilk, Andrew Cooper, Jan Beulich,
Boris Ostrovsky, Roger Pau Monné
The final remnanat of PVRDTSCP is that we would emulate RDTSCP even on
hardware which lacked the instruction. RDTSCP is available on almost all
64-bit x86 hardware.
Remove this emulation, drop the TSC_MODE_PVRDTSCP constant, and allow RDTSCP
in a PV guest's CPUID policy.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/arch/x86/pv/emul-inv-op.c | 27 +--------------------------
xen/include/asm-x86/time.h | 1 -
xen/include/public/arch-x86/cpufeatureset.h | 2 +-
3 files changed, 2 insertions(+), 28 deletions(-)
diff --git a/xen/arch/x86/pv/emul-inv-op.c b/xen/arch/x86/pv/emul-inv-op.c
index 56f5a45..91d0579 100644
--- a/xen/arch/x86/pv/emul-inv-op.c
+++ b/xen/arch/x86/pv/emul-inv-op.c
@@ -41,31 +41,6 @@
#include "emulate.h"
-static int emulate_invalid_rdtscp(struct cpu_user_regs *regs)
-{
- char opcode[3];
- unsigned long eip, rc;
- struct vcpu *v = current;
- const struct domain *currd = v->domain;
-
- eip = regs->rip;
- if ( (rc = copy_from_user(opcode, (char *)eip, sizeof(opcode))) != 0 )
- {
- pv_inject_page_fault(0, eip + sizeof(opcode) - rc);
- return EXCRET_fault_fixed;
- }
- if ( memcmp(opcode, "\xf\x1\xf9", sizeof(opcode)) )
- return 0;
- eip += sizeof(opcode);
-
- msr_split(regs, pv_soft_rdtsc(v, regs));
- regs->rcx = (currd->arch.tsc_mode == TSC_MODE_PVRDTSCP
- ? currd->arch.incarnation : 0);
-
- pv_emul_instruction_done(regs, eip);
- return EXCRET_fault_fixed;
-}
-
static int emulate_forced_invalid_op(struct cpu_user_regs *regs)
{
char sig[5], instr[2];
@@ -121,7 +96,7 @@ static int emulate_forced_invalid_op(struct cpu_user_regs *regs)
bool pv_emulate_invalid_op(struct cpu_user_regs *regs)
{
- return !emulate_invalid_rdtscp(regs) && !emulate_forced_invalid_op(regs);
+ return !emulate_forced_invalid_op(regs);
}
/*
diff --git a/xen/include/asm-x86/time.h b/xen/include/asm-x86/time.h
index 070cdef..d95814e 100644
--- a/xen/include/asm-x86/time.h
+++ b/xen/include/asm-x86/time.h
@@ -17,7 +17,6 @@
#define TSC_MODE_DEFAULT 0
#define TSC_MODE_ALWAYS_EMULATE 1
#define TSC_MODE_NEVER_EMULATE 2
-#define TSC_MODE_PVRDTSCP 3
typedef u64 cycles_t;
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 6c82816..fbc68fa 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -156,7 +156,7 @@ XEN_CPUFEATURE(NX, 2*32+20) /*A Execute Disable */
XEN_CPUFEATURE(MMXEXT, 2*32+22) /*A AMD MMX extensions */
XEN_CPUFEATURE(FFXSR, 2*32+25) /*A FFXSR instruction optimizations */
XEN_CPUFEATURE(PAGE1GB, 2*32+26) /*H 1Gb large page support */
-XEN_CPUFEATURE(RDTSCP, 2*32+27) /*S RDTSCP */
+XEN_CPUFEATURE(RDTSCP, 2*32+27) /*A RDTSCP */
XEN_CPUFEATURE(LM, 2*32+29) /*A Long Mode (x86-64) */
XEN_CPUFEATURE(3DNOWEXT, 2*32+30) /*A AMD 3DNow! extensions */
XEN_CPUFEATURE(3DNOW, 2*32+31) /*A 3DNow! */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests
2018-11-15 21:47 ` [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests Andrew Cooper
@ 2018-11-20 11:06 ` Jan Beulich
2018-11-20 11:29 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2018-11-20 11:06 UTC (permalink / raw)
To: Andrew Cooper
Cc: Boris Ostrovsky, Xen-devel, Wei Liu, Konrad Rzeszutek Wilk,
Roger Pau Monne
>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
> The final remnanat of PVRDTSCP is that we would emulate RDTSCP even on
> hardware which lacked the instruction. RDTSCP is available on almost all
> 64-bit x86 hardware.
>
> Remove this emulation, drop the TSC_MODE_PVRDTSCP constant, and allow RDTSCP
> in a PV guest's CPUID policy.
Why would we not want to emulate the insn when unavailable, when
it's generally useful to guests? IOW rather than removing the code,
why don't you simply correct ...
> -static int emulate_invalid_rdtscp(struct cpu_user_regs *regs)
> -{
> - char opcode[3];
> - unsigned long eip, rc;
> - struct vcpu *v = current;
> - const struct domain *currd = v->domain;
> -
> - eip = regs->rip;
> - if ( (rc = copy_from_user(opcode, (char *)eip, sizeof(opcode))) != 0 )
> - {
> - pv_inject_page_fault(0, eip + sizeof(opcode) - rc);
> - return EXCRET_fault_fixed;
> - }
> - if ( memcmp(opcode, "\xf\x1\xf9", sizeof(opcode)) )
> - return 0;
> - eip += sizeof(opcode);
> -
> - msr_split(regs, pv_soft_rdtsc(v, regs));
> - regs->rcx = (currd->arch.tsc_mode == TSC_MODE_PVRDTSCP
> - ? currd->arch.incarnation : 0);
... this to use what the prior patch has arranged for? The only
reason I can see why this could be pointless is if guests only ever
used this on performance critical paths. But afaict that's not the
case here.
We may not be at the point yet where we can announce to guests
a feature not available in hardware via the (not so) new CPUID
infrastructure, but keeping this code here would make this an easy
change down the road.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests
2018-11-20 11:06 ` Jan Beulich
@ 2018-11-20 11:29 ` Andrew Cooper
2018-11-20 15:14 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2018-11-20 11:29 UTC (permalink / raw)
To: Jan Beulich
Cc: Boris Ostrovsky, Xen-devel, Wei Liu, Konrad Rzeszutek Wilk,
Roger Pau Monne
On 20/11/2018 11:06, Jan Beulich wrote:
>>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
>> The final remnanat of PVRDTSCP is that we would emulate RDTSCP even on
>> hardware which lacked the instruction. RDTSCP is available on almost all
>> 64-bit x86 hardware.
>>
>> Remove this emulation, drop the TSC_MODE_PVRDTSCP constant, and allow RDTSCP
>> in a PV guest's CPUID policy.
> Why would we not want to emulate the insn when unavailable, when
> it's generally useful to guests?
For exactly the same kind of safety reasons as for why we don't tolerate
doing this in HVM guests in general.
As it stands, it is an unnecessary attack surface, and if we were to
re-introduce the functionality (not that I can see a valid reason to),
it should use x86_emulate() rather than opencoding the logic.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests
2018-11-20 11:29 ` Andrew Cooper
@ 2018-11-20 15:14 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2018-11-20 15:14 UTC (permalink / raw)
To: Andrew Cooper
Cc: Boris Ostrovsky, Xen-devel, Wei Liu, Konrad Rzeszutek Wilk,
Roger Pau Monne
>>> On 20.11.18 at 12:29, <andrew.cooper3@citrix.com> wrote:
> On 20/11/2018 11:06, Jan Beulich wrote:
>>>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
>>> The final remnanat of PVRDTSCP is that we would emulate RDTSCP even on
>>> hardware which lacked the instruction. RDTSCP is available on almost all
>>> 64-bit x86 hardware.
>>>
>>> Remove this emulation, drop the TSC_MODE_PVRDTSCP constant, and allow RDTSCP
>>> in a PV guest's CPUID policy.
>> Why would we not want to emulate the insn when unavailable, when
>> it's generally useful to guests?
>
> For exactly the same kind of safety reasons as for why we don't tolerate
> doing this in HVM guests in general.
But imo we should start doing so for certain features. In particular
I think we had agreed to try to support UMIP even on incapable
hardware. You had merely asked to postpone my patch to this
effect until more of the CPUID infrastructure was in place (which
afaict it still isn't).
> As it stands, it is an unnecessary attack surface, and if we were to
> re-introduce the functionality (not that I can see a valid reason to),
> it should use x86_emulate() rather than opencoding the logic.
How much larger is the attack surface of emulate_invalid_rdtscp()
compared to emulate_forced_invalid_op()?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests
2018-11-15 21:47 [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Andrew Cooper
` (3 preceding siblings ...)
2018-11-15 21:47 ` [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests Andrew Cooper
@ 2018-11-16 10:11 ` Jan Beulich
2018-11-16 17:43 ` Andrew Cooper
4 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2018-11-16 10:11 UTC (permalink / raw)
To: Andrew Cooper
Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit,
Konrad Rzeszutek Wilk, Xen-devel, Jun Nakajima, Boris Ostrovsky,
Brian Woods, Roger Pau Monne
>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
> Boris has confirmed that noone appears to be using PVRDTSCP any more, and in
> the decade since it was introduced, guest kernel / hardware support has
> provided a better alternative.
Doesn't removal of functionality require knowing that it was never used
at all, rather than just knowing that nothing uses it anymore? What if
some old guest somewhere relies on it?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests
2018-11-16 10:11 ` [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests Jan Beulich
@ 2018-11-16 17:43 ` Andrew Cooper
2018-11-16 19:55 ` Konrad Rzeszutek Wilk
2018-11-19 8:29 ` Jan Beulich
0 siblings, 2 replies; 17+ messages in thread
From: Andrew Cooper @ 2018-11-16 17:43 UTC (permalink / raw)
To: Jan Beulich
Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit,
Konrad Rzeszutek Wilk, Xen-devel, Jun Nakajima, Boris Ostrovsky,
Brian Woods, Roger Pau Monne
On 16/11/2018 10:11, Jan Beulich wrote:
>>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
>> Boris has confirmed that noone appears to be using PVRDTSCP any more, and in
>> the decade since it was introduced, guest kernel / hardware support has
>> provided a better alternative.
> Doesn't removal of functionality require knowing that it was never used
> at all, rather than just knowing that nothing uses it anymore? What if
> some old guest somewhere relies on it?
Its an all-or-nothing feature. The entirety of your VM userspace need
to support it, or timing will go wrong on migrate.
We already established that it appears to be a vestigial Oracle-ism for
which no consumer side code ever appeared, and that isn't used.
What is unacceptable is PVRDTSCP's implementation causing breakages in
architectural behaviour for non-PVRDTSCP configurations, and one way or
another, this needs fixing.
Please can we make a decision, because I don't have to time (or indeed,
the want) to and fix this a 3rd different way if that's going to run
into a similar reaction.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests
2018-11-16 17:43 ` Andrew Cooper
@ 2018-11-16 19:55 ` Konrad Rzeszutek Wilk
2018-11-19 8:29 ` Jan Beulich
1 sibling, 0 replies; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2018-11-16 19:55 UTC (permalink / raw)
To: Andrew Cooper
Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit, Xen-devel,
Jan Beulich, Jun Nakajima, Boris Ostrovsky, Brian Woods,
Roger Pau Monne
On Fri, Nov 16, 2018 at 05:43:42PM +0000, Andrew Cooper wrote:
> On 16/11/2018 10:11, Jan Beulich wrote:
> >>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
> >> Boris has confirmed that noone appears to be using PVRDTSCP any more, and in
> >> the decade since it was introduced, guest kernel / hardware support has
> >> provided a better alternative.
> > Doesn't removal of functionality require knowing that it was never used
> > at all, rather than just knowing that nothing uses it anymore? What if
> > some old guest somewhere relies on it?
We have verified that it was never used.
>
> Its an all-or-nothing feature. The entirety of your VM userspace need
> to support it, or timing will go wrong on migrate.
>
> We already established that it appears to be a vestigial Oracle-ism for
> which no consumer side code ever appeared, and that isn't used.
>
> What is unacceptable is PVRDTSCP's implementation causing breakages in
> architectural behaviour for non-PVRDTSCP configurations, and one way or
> another, this needs fixing.
>
> Please can we make a decision, because I don't have to time (or indeed,
> the want) to and fix this a 3rd different way if that's going to run
> into a similar reaction.
>
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] x86: Drop PVRDTSCP and fix MSR_TSC_AUX handling for guests
2018-11-16 17:43 ` Andrew Cooper
2018-11-16 19:55 ` Konrad Rzeszutek Wilk
@ 2018-11-19 8:29 ` Jan Beulich
1 sibling, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2018-11-19 8:29 UTC (permalink / raw)
To: Andrew Cooper
Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit,
Konrad Rzeszutek Wilk, Xen-devel, Jun Nakajima, Boris Ostrovsky,
Brian Woods, Roger Pau Monne
>>> On 16.11.18 at 18:43, <andrew.cooper3@citrix.com> wrote:
> On 16/11/2018 10:11, Jan Beulich wrote:
>>>>> On 15.11.18 at 22:47, <andrew.cooper3@citrix.com> wrote:
>>> Boris has confirmed that noone appears to be using PVRDTSCP any more, and in
>>> the decade since it was introduced, guest kernel / hardware support has
>>> provided a better alternative.
>> Doesn't removal of functionality require knowing that it was never used
>> at all, rather than just knowing that nothing uses it anymore? What if
>> some old guest somewhere relies on it?
>
> Its an all-or-nothing feature. The entirety of your VM userspace need
> to support it, or timing will go wrong on migrate.
>
> We already established that it appears to be a vestigial Oracle-ism for
> which no consumer side code ever appeared, and that isn't used.
>
> What is unacceptable is PVRDTSCP's implementation causing breakages in
> architectural behaviour for non-PVRDTSCP configurations, and one way or
> another, this needs fixing.
>
> Please can we make a decision, because I don't have to time (or indeed,
> the want) to and fix this a 3rd different way if that's going to run
> into a similar reaction.
With Konrad's statement I'm fine with the removal. I'll get to look
at the individual patches.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread