Re: [Xen-devel] [PATCH 6/6] x86: introduce dr_mask_idx() helper function...

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 08 March 2019 17:10
> To: Jan Beulich <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; 
> xen-devel <xen-
> devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH 6/6] x86: introduce dr_mask_idx() helper function...
> 
> On 08/03/2019 16:58, Jan Beulich wrote:
> >>>> On 07.01.19 at 13:02, <paul.durrant@xxxxxxxxxx> wrote:
> >> @@ -202,13 +201,10 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, 
> >> uint64_t *val)
> >>           */
> >>  #ifdef CONFIG_HVM
> >>          if ( v == current && is_hvm_domain(d) && 
> >> v->arch.hvm.flag_dr_dirty )
> >> -            rdmsrl(msr, *val);
> >> -        else
> >> +            rdmsrl(msr, msrs->dr_mask[dr_mask_idx(msr)]);
> >>  #endif
> >> -            *val = msrs->dr_mask[
> >> -                array_index_nospec((msr == MSR_AMD64_DR0_ADDRESS_MASK)
> >> -                                   ? 0 : (msr - 
> >> MSR_AMD64_DR1_ADDRESS_MASK + 1),
> >> -                                   ARRAY_SIZE(msrs->dr_mask))];
> >> +
> >> +        *val = msrs->dr_mask[dr_mask_idx(msr)];
> >>          break;
> > While I don't really mind this behavioral change (of updating *msrs),
> > I'd like to get Andrew's opinion on this from a conceptual pov.
> 
> So, considering...
> 
> >
> >> @@ -317,6 +318,26 @@ struct vcpu_msrs
> >>      } xss;
> >>  };
> >>
> >> +static inline unsigned int dr_mask_idx(uint32_t msr)
> >> +{
> >> +    switch (msr)
> > Missing blanks immediately inside the parentheses.
> >
> >> +    {
> >> +    default:
> >> +        ASSERT_UNREACHABLE();
> >> +        /* Fallthrough */
> >> +    case MSR_AMD64_DR0_ADDRESS_MASK:
> >> +        return 0;
> 
> ... this reintroduces a half-Spectre-v1 gadget, I'm -1 for the change.
> 
> I don't anticipate this translation being needed anywhere else, which is
> why I didn't introduce a helper the first time around.
> 
> I'd just drop the patch and leave the code as it is.

Ok, I'll drop it.

  Paul

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