Re: [PATCH 6/7] x86: Extend x86_event with a pending_dbg field

[Jan Beulich]

On 15.09.2023 22:36, Andrew Cooper wrote:
> ... using the Intel VMCS PENDING_DBG semantics, and sharing storage with cr2.
> 
> This requires working around anonymous union bugs in obsolete versions of GCC,
> which in turn needs to drop unnecessary const qualifiers.
> 
> Also introduce a pv_inject_DB() wrapper use this field nicely.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 

Re: [PATCH 6/7] x86: Extend x86_event with a pending_dbg field

[Jinoh Kang]

On 9/16/23 05:36, Andrew Cooper wrote:
> ... using the Intel VMCS PENDING_DBG semantics, and sharing storage with cr2.
> 
> This requires working around anonymous union bugs in obsolete versions of GCC,
> which in turn needs to drop unnecessary const qualifiers.

I'd write this as a inline comment as long as it doesn't make the code
too verbose.

Please disregard this if it doesn't match our coding style.

> 
> Also introduce a pv_inject_DB() wrapper use this field nicely.

Minor nit: Also introduce a pv_inject_DB() wrapper to* use this field nicely.

> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Roger Pau Monné 
> CC: Wei Liu 
> CC: Jinoh Kang 
> 
> v2:
>  * Split out of prior patch.
> ---
>  xen/arch/x86/include/asm/domain.h  | 18 --
>  xen/arch/x86/include/asm/hvm/hvm.h |  3 ++-
>  xen/arch/x86/x86_emulate/x86_emulate.h |  5 -
>  3 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/domain.h 
> b/xen/arch/x86/include/asm/domain.h
> index c2d9fc333be5..fd1f306222be 100644
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -729,15 +729,29 @@ static inline void pv_inject_hw_exception(unsigned int 
> vector, int errcode)
>  pv_inject_event();
>  }
>  
> +static inline void pv_inject_DB(unsigned long pending_dbg)
> +{
> +struct x86_event event = {
> +.vector  = X86_EXC_DB,
> +.type= X86_EVENTTYPE_HW_EXCEPTION,
> +.error_code  = X86_EVENT_NO_EC,
> +};
> +
> +event.pending_dbg = pending_dbg;
> +
> +pv_inject_event();
> +}
> +
>  static inline void pv_inject_page_fault(int errcode, unsigned long cr2)
>  {
> -const struct x86_event event = {
> +struct x86_event event = {
>  .vector = X86_EXC_PF,
>  .type = X86_EVENTTYPE_HW_EXCEPTION,
>  .error_code = errcode,
> -.cr2 = cr2,
>  };
>  
> +event.cr2 = cr2;
> +
>  pv_inject_event();
>  }
>  
> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
> b/xen/arch/x86/include/asm/hvm/hvm.h
> index 6d53713fc3a9..ea966f4429f9 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -532,9 +532,10 @@ static inline void hvm_inject_page_fault(int errcode, 
> unsigned long cr2)
>  .vector = X86_EXC_PF,
>  .type = X86_EVENTTYPE_HW_EXCEPTION,
>  .error_code = errcode,
> -.cr2 = cr2,
>  };
>  
> +event.cr2 = cr2;
> +
>  hvm_inject_event();
>  }
>  
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h 
> b/xen/arch/x86/x86_emulate/x86_emulate.h
> index fbc023c37e34..e567a9b635d9 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -78,7 +78,10 @@ struct x86_event {
>  uint8_t   type; /* X86_EVENTTYPE_* */
>  uint8_t   insn_len; /* Instruction length */
>  int32_t   error_code;   /* X86_EVENT_NO_EC if n/a */
> -unsigned long cr2;  /* Only for X86_EXC_PF h/w exception */
> +union {
> +unsigned long cr2; /* #PF */
> +unsigned long pending_dbg; /* #DB (new DR6 bits, positive polarity) 
> */
> +};
>  };
>  
>  /*

-- 
Sincerely,
Jinoh Kang

[PATCH 6/7] x86: Extend x86_event with a pending_dbg field

[Andrew Cooper]

... using the Intel VMCS PENDING_DBG semantics, and sharing storage with cr2.

This requires working around anonymous union bugs in obsolete versions of GCC,
which in turn needs to drop unnecessary const qualifiers.

Also introduce a pv_inject_DB() wrapper use this field nicely.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Jinoh Kang 

v2:
 * Split out of prior patch.
---
 xen/arch/x86/include/asm/domain.h  | 18 --
 xen/arch/x86/include/asm/hvm/hvm.h |  3 ++-
 xen/arch/x86/x86_emulate/x86_emulate.h |  5 -
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/include/asm/domain.h 
b/xen/arch/x86/include/asm/domain.h
index c2d9fc333be5..fd1f306222be 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -729,15 +729,29 @@ static inline void pv_inject_hw_exception(unsigned int 
vector, int errcode)
 pv_inject_event();
 }
 
+static inline void pv_inject_DB(unsigned long pending_dbg)
+{
+struct x86_event event = {
+.vector  = X86_EXC_DB,
+.type= X86_EVENTTYPE_HW_EXCEPTION,
+.error_code  = X86_EVENT_NO_EC,
+};
+
+event.pending_dbg = pending_dbg;
+
+pv_inject_event();
+}
+
 static inline void pv_inject_page_fault(int errcode, unsigned long cr2)
 {
-const struct x86_event event = {
+struct x86_event event = {
 .vector = X86_EXC_PF,
 .type = X86_EVENTTYPE_HW_EXCEPTION,
 .error_code = errcode,
-.cr2 = cr2,
 };
 
+event.cr2 = cr2;
+
 pv_inject_event();
 }
 
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
b/xen/arch/x86/include/asm/hvm/hvm.h
index 6d53713fc3a9..ea966f4429f9 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -532,9 +532,10 @@ static inline void hvm_inject_page_fault(int errcode, 
unsigned long cr2)
 .vector = X86_EXC_PF,
 .type = X86_EVENTTYPE_HW_EXCEPTION,
 .error_code = errcode,
-.cr2 = cr2,
 };
 
+event.cr2 = cr2;
+
 hvm_inject_event();
 }
 
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h 
b/xen/arch/x86/x86_emulate/x86_emulate.h
index fbc023c37e34..e567a9b635d9 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -78,7 +78,10 @@ struct x86_event {
 uint8_t   type; /* X86_EVENTTYPE_* */
 uint8_t   insn_len; /* Instruction length */
 int32_t   error_code;   /* X86_EVENT_NO_EC if n/a */
-unsigned long cr2;  /* Only for X86_EXC_PF h/w exception */
+union {
+unsigned long cr2; /* #PF */
+unsigned long pending_dbg; /* #DB (new DR6 bits, positive polarity) */
+};
 };
 
 /*
-- 
2.30.2