Re: [PATCH v2 1/8] x86: Fix calculation of %dr6/7 reserved bits

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/08/2023 4:25 pm, Jinoh Kang wrote:
> - Define X86_DR{6,7}_* constants in x86-defns.h instead of open-coding
>   naked numbers (thanks Jan)

Jan - stop insisting of other people things I've already rejected,
particularly on my patches.

> diff --git a/xen/arch/x86/include/asm/debugreg.h 
> b/xen/arch/x86/include/asm/debugreg.h
> index 86aa6d7143..74344555d2 100644
> --- a/xen/arch/x86/include/asm/debugreg.h
> +++ b/xen/arch/x86/include/asm/debugreg.h
> @@ -80,4 +78,20 @@
>  long set_debugreg(struct vcpu *, unsigned int reg, unsigned long value);
>  void activate_debugregs(const struct vcpu *);
>  
> +static inline unsigned long adjust_dr6_rsvd(unsigned long dr6, bool rtm)
> +{
> +    dr6 |= X86_DR6_MBS_BASE | (rtm ? 0 : X86_DR6_MBS_NO_RTM);
> +    dr6 &= ~X86_DR6_MBZ;
> +
> +    return dr6;
> +}
> +
> +static inline unsigned long adjust_dr7_rsvd(unsigned long dr7, bool rtm)
> +{
> +    dr7 |= X86_DR7_MBS;
> +    dr7 &= ~(X86_DR7_MBZ_BASE | (rtm ? 0 : X86_DR7_MBZ_NO_RTM));
> +
> +    return dr7;
> +}

Jinoh, for your benefit, the reason it was the way it was is because of
how the processor manuals describe this logic.  Not this, which is
borderline illegible with double negations all over the place.


However, in the time since I wrote this patch, more inverted bits have
appeared that need accounting for, and this is no longer the best interface.

I'll adjust the patch because it's unfair for you to be caught in the
middle of an an existing fight over code comprehensibility.

~Andrew