Re: [Xen-devel] [PATCH] x86/debug: Avoid crashing in early boot because of debugger_trap_entry()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.08.16 at 11:20, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 04/08/16 08:05, Jan Beulich wrote:
>>>>> On 03.08.16 at 19:08, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> debugger_trap_entry() is not safe to use during early boot, as it follows
>>> current before it is necesserily safe to do so.  Futhermore it does this
>>> unconditionally, despite most callsites turning into no-ops because of the
>>> vector test.
>>>
>>> Inline debugger_trap_entry() into the two callers where doesn't become a
>>> no-op,
>> Implied from that you delete all other invocations. While from a
>> (current) functionality pov that's no functional change, to me these
>> invocations have always served as an indication that some action
>> _could_ be taken there as well. I'm not sure it's a good idea to
>> remove that.
> 
> I considered that point specifically, but I disagree.
> 
> The current callsites require debugger_trap_entry() to double up all the
> safety checks the main, which isn't reasonable IMO.
> 
> This code hasn't changed in years and don't think it is likely to change
> in the forseeable future.  If people did want something a little like
> how it currently is, the current code is not a good starting point.

If I had ever gotten to port my Linux kernel debugger to Xen, I
would most likely have customized debugger_trap_entry(), and
the deletion of its invocations would then have broken everything.

>>> @@ -3814,6 +3825,13 @@ void do_debug(struct cpu_user_regs *regs)
>>>      /* Save debug status register where guest OS can peek at it */
>>>      v->arch.debugreg[6] = read_debugreg(6);
>>>  
>>> +    if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
>>> +    {
>>> +        v->arch.gdbsx_vcpu_event = TRAP_debug;
>> Note how in the original code this write didn't happen. I can't tell why,
>> but it's a behavioral change which certainly would need explaining
> 
> This was mentioned in the commit message.

Well, before writing the reply I did read it a second time. Now I've
read a third and fourth time, and still can't spot any mention of this.
I'm sorry if I'm being dense...

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel