On 08/03/2019 16:58, Jan Beulich wrote:
>>>> On 07.01.19 at 13:02, <[email protected]> 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.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
