diff mbox series

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

Message ID 88d0c78e7fecac79d4ef958962c59836c966cc91.1722871218.git.matthew.barnes@cloud.com (mailing list archive)
State Superseded
Headers show
Series [XEN] x86/emul: Fix misaligned IO breakpoint behaviour in PV guests | expand

Commit Message

Matthew Barnes Aug. 5, 2024, 3:36 p.m. UTC
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@cloud.com>
---
 xen/arch/x86/pv/emul-priv-op.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Jan Beulich Aug. 5, 2024, 4:08 p.m. UTC | #1
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@cloud.com>

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
diff mbox series

Patch

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index f101510a1bab..02901a379b2a 100644
--- 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));
+
+        if ( ((start & debug_mask) < (port + len)) && (((start & debug_mask) + width) > port) )
             match |= 1u << i;
     }