On 24.08.2023 17:26, Jinoh Kang wrote:
> @@ -62,9 +63,16 @@ void pv_inject_event(const struct x86_event *event)
> error_code |= PFEC_user_mode;
>
> trace_pv_page_fault(event->cr2, error_code);
> - }
> - else
> + break;
> +
> + case X86_EXC_DB:
> + curr->arch.dr6 |= event->pending_dbg;
> + /* Fallthrough */
I guess I have another question here, perhaps more to Andrew: How come
this is just an OR? Not only with some of the bits having inverted sense
and earlier logic being ...
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1989,17 +1989,17 @@ void do_debug(struct cpu_user_regs *regs)
> return;
> }
>
> - /* Save debug status register where guest OS can peek at it */
> - v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
> - v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
... an OR and an AND, but also with ...
> --- 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)
> */
... the comment here saying "positive polarity", which I understand
to mean that inverted bits need inverting by the consumer of this
field. If this is solely because none of the inverted bits are
supported for PV, then I guess this wants a comment at the use site
(not the least because it would need adjusting as soon as one such
would become supported).
Jan
