> -----Original Message-----
> From: Andrew Cooper
> Sent: 08 March 2019 17:10
> To: Jan Beulich <jbeul...@suse.com>; Paul Durrant <paul.durr...@citrix.com>
> Cc: Roger Pau Monne <roger....@citrix.com>; Wei Liu <wei.l...@citrix.com>;
> xen-devel <xen-
> de...@lists.xenproject.org>
> 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.durr...@citrix.com> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
