[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 4/4] x86/pv: Expose RDTSCP to PV guests



>>> On 15.11.18 at 22:47, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.