Re: [Xen-devel] [PATCH 04/11] x86: Fix calculation of %dr6/7 reserved bits

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jun 06, 2018 at 04:59:24PM +0100, Andrew Cooper wrote:
> On 06/06/18 16:49, Roger Pau Monné wrote:
> > On Mon, Jun 04, 2018 at 02:59:08PM +0100, Andrew Cooper wrote:
> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >> index 10415e6..7fddae1 100644
> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -977,6 +977,7 @@ unsigned long hvm_cr4_guest_valid_bits(const struct 
> >> domain *d, bool restore)
> >>  
> >>  static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> >>  {
> >> +    const struct cpuid_policy *cp = d->arch.cpuid;
> >>      int vcpuid;
> >>      struct vcpu *v;
> >>      struct hvm_hw_cpu ctxt;
> >> @@ -1154,8 +1155,8 @@ static int hvm_load_cpu_ctxt(struct domain *d, 
> >> hvm_domain_context_t *h)
> >>      v->arch.debugreg[1] = ctxt.dr1;
> >>      v->arch.debugreg[2] = ctxt.dr2;
> >>      v->arch.debugreg[3] = ctxt.dr3;
> >> -    v->arch.debugreg[6] = ctxt.dr6;
> >> -    v->arch.debugreg[7] = ctxt.dr7;
> >> +    v->arch.debugreg[6] = adjust_dr6_rsvd(ctxt.dr6, cp->feat.rtm);
> >> +    v->arch.debugreg[7] = adjust_dr7_rsvd(ctxt.dr7, cp->feat.rtm);
> > I don't know much, but if the value in dr6/dr7 is changed from the one
> > in the provided context, won't the guest experience issues?
> >
> > Won't it be better to plain refuse to load a context that has
> > unsupported DR6/7 values?
> 
> adjust_dr{6,7}_rsvd() match the adjustments the processor does when
> loading values.
> 
> For backwards compatibility, the processor accepts any bit pattern for
> the bottom 32 bits, then ignores the bits it doesn't care about.  When
> reading the register back, the unimplemented bits appear as their
> defaults, which aren't all 0.
> 
> In the case of hvm_load_cpu_ctxt(), we'd ideally strictly limit it to
> architectural expectation, but there will be a good chunk of VMs out
> there which haven't ever touched %dr6/%dr7 in their lifetime, and will
> therefore have 0's in the migration record at this point.

Right, I guess that's the best approach.

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

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