On Tue, Feb 20, 2018 at 11:58:41AM +0000, Andrew Cooper wrote: > Having pv_soft_rdtsc() emulate all parts of an rdtscp is awkward, and gets in > the way of some intended cleanup. > > * Drop the rdtscp parameter and always make the caller responsible for ecx > updates when appropriate. > * Switch the function from being void, and return the main timestamp in the > return value. > > The regs parameter is still needed, but only for the stats collection, once > again bringing into question their utility. The parameter can however switch > to being const. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > --- > CC: Jan Beulich <jbeul...@suse.com> > CC: Wei Liu <wei.l...@citrix.com> > CC: Roger Pau Monné <roger....@citrix.com> > CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> > --- > xen/arch/x86/pv/emul-inv-op.c | 7 ++++++- > xen/arch/x86/pv/emul-priv-op.c | 12 ++++++++---- > xen/arch/x86/time.c | 8 ++------ > xen/include/asm-x86/time.h | 2 +- > 4 files changed, 17 insertions(+), 12 deletions(-) > > diff --git a/xen/arch/x86/pv/emul-inv-op.c b/xen/arch/x86/pv/emul-inv-op.c > index f894417..b1916b4 100644 > --- a/xen/arch/x86/pv/emul-inv-op.c > +++ b/xen/arch/x86/pv/emul-inv-op.c > @@ -46,6 +46,7 @@ static int emulate_invalid_rdtscp(struct cpu_user_regs > *regs) > char opcode[3]; > unsigned long eip, rc; > struct vcpu *v = current; > + struct domain *currd = v->domain; const?
> > eip = regs->rip; > if ( (rc = copy_from_user(opcode, (char *)eip, sizeof(opcode))) != 0 ) > @@ -56,7 +57,11 @@ static int emulate_invalid_rdtscp(struct cpu_user_regs > *regs) > if ( memcmp(opcode, "\xf\x1\xf9", sizeof(opcode)) ) > return 0; > eip += sizeof(opcode); > - pv_soft_rdtsc(v, regs, 1); > + > + msr_split(regs, pv_soft_rdtsc(v, regs)); > + regs->rcx = ((currd->arch.tsc_mode == TSC_MODE_PVRDTSCP) > + ? currd->arch.incarnation : 0); In the previous patch you use the same expression without any parentheses. I think I prefer the version without parentheses. I also wonder whether it would make sense to turn this into a macro (seeing it being used in the previous patch and also further below). With those fixed (if applicable): Reviewed-by: Roger Pau Monné <roger....@citrix.com> Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel