Re: [Xen-devel] [PATCH 07/11] x86/emul: Add pending_dbg field to x86_event

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.06.18 at 15:59, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -82,9 +83,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 TRAP_debug:
> +        curr->arch.dr6 |= event->pending_dbg;

Considering what you've been telling me over and over, shouldn't
you mask out the low four bits here before ORing in new state?

> +        /* Fallthrough */
> +
> +    default:
>          trace_pv_trap(vector, regs->rip, use_error_code, error_code);

I also wonder whether tracing wouldn't benefit from being informed
about pending_dbg here instead of the error code. Not something
for this patch (or series) of course, just as a remark.

Since the equivalent HVM code looks to be missing, I take it that's
going to be addressed in later patches of the series?

> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -88,7 +88,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 TRAP_page_fault h/w exception */
> +    union {
> +        unsigned long cr2;         /* #PF */
> +        unsigned long pending_dbg; /* #DB (new DR6 bits, positive polarity) 
> */
> +    };
>  };

Seeing that this is the only x86_emulate* change, I don't suppose
you fancy making the emulator correctly raise X86_DR6_BD at the
same time?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel