Re: [PATCH 5/5] x86: Rework MSR_TSC_AUX handling from scratch.

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Feb 20, 2018 at 05:42:06PM +0000, Andrew Cooper wrote:
> On 20/02/18 17:03, Wei Liu wrote:
> > On Tue, Feb 20, 2018 at 11:58:43AM +0000, Andrew Cooper wrote:
> >> There are many problems with MSR_TSC_AUX handling.
> >>
> >> To being with, the RDPID instruction reads MSR_TSC_AUX, but it is only the
> >      ^
> >      begin
> >
> >> RDTSCP feature which enumerates the MSR.  Therefore, RDPID functionally
> >> depends on RDTSCP.
> >>
> >> For PV guests, we hide RDTSCP but advertise RDPID.  We also silently drop
> >> writes to MSR_TSC_AUX, which is very antisocial.  Therefore, enable RDTSCP for
> >> PV guests, which in turn allows RDPID to work.
> >>
> >> To support RDTSCP properly for PV guests, the MSR_TSC_AUX handling is moved
> >> into the generic MSR policy infrastructure, and becomes common.  One
> >> improvement is that we will now reject invalid values, rather than silently
> >> truncating an accepting them.  This also causes the emulator to reject RDTSCP
> >              ^
> >              and
> >
> >> for guests without the features.
> >>
> >>  
> > [...]
> >> @@ -1284,7 +1285,18 @@ long arch_do_domctl(
> >>                  for ( j = 0; j < ARRAY_SIZE(msrs_to_send); ++j )
> >>                  {
> >>                      uint64_t val;
> >> -                    int rc = guest_rdmsr(v, msrs_to_send[j], &val);
> >> +                    int rc;
> >> +
> >> +                    /*
> >> +                     * Skip MSR_TSC_AUX if using TSC_MODE_PVRDTSCP.  In this
> >> +                     * case, the MSR is read-only, and should be rejected if
> >> +                     * seen on the restore side.
> >> +                     */
> >> +                    if ( msrs_to_send[j] == MSR_TSC_AUX &&
> >> +                         d->arch.tsc_mode == TSC_MODE_PVRDTSCP )
> >> +                        continue;
> > Shouldn't we increment incarnation and send it over to the remote end?
> > Or send the original value and let the remote increments it?
> 
> incarnation, and its increments, is handled in tsc_set_info(), which is
> keyed off the TSC_INFO record in the migration stream.  That side of
> things "already works" (FSVO "works").
> 

OK. That's fine then.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel