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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> @@ -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;
> +
> +    case MSR_AMD64_DR1_ADDRESS_MASK ... MSR_AMD64_DR3_ADDRESS_MASK:
> +    {
> +        struct vcpu_msrs msrs; /* only used for ARRAY_SIZE() */

I don't think you need this - you can use e.g.
ARRAY_SIZE(current->arch.vcpu.msrs.dr_mask), can't you?

Jan



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