Re: [XEN PATCH] x86/emul: Fix misaligned IO breakpoint behaviour in PV guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.08.2024 17:36, Matthew Barnes wrote:
> When hardware breakpoints are configured on misaligned IO ports, the
> hardware will mask the addresses based on the breakpoint width during
> comparison.
> 
> For PV guests, misaligned IO breakpoints do not behave the same way, and
> therefore yield different results.
> 
> This patch tweaks the emulation of IO breakpoints for PV guests such
> that they reproduce the same behaviour as hardware.
> 
> Signed-off-by: Matthew Barnes <matthew.barnes@xxxxxxxxx>

No Fixes: tag?

> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -324,7 +324,7 @@ static unsigned int check_guest_io_breakpoint(struct vcpu 
> *v,
>                                                unsigned int len)
>  {
>      unsigned int width, i, match = 0;
> -    unsigned long start;
> +    unsigned long start, debug_mask;
>  
>      if ( !v->arch.pv.dr7_emul || !(v->arch.pv.ctrlreg[4] & X86_CR4_DE) )
>          return 0;
> @@ -346,7 +346,9 @@ static unsigned int check_guest_io_breakpoint(struct vcpu 
> *v,
>          case DR_LEN_8: width = 8; break;
>          }
>  
> -        if ( (start < (port + len)) && ((start + width) > port) )
> +        debug_mask = (~(width - 1u));

Maybe simply alter "start" directly:

        start &= ~(width - 1UL);

I've switched to making the resulting mask an unsigned long quantity at the
same time, as I don't think we ought to masking off the top 32 bits.

Then ...

> +        if ( ((start & debug_mask) < (port + len)) && (((start & debug_mask) 
> + width) > port) )

... I also won't need to complain that this line now is too long.

Jan