follow-up to guest debug support patches

follow-up to guest debug support patches

[Kip Macy]

If the archive is any indication, the patches were inlined and truncated.
The two patches can be found here:
http://www.fsmware.com/xenofreebsd/gdb/050311/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

RE: follow-up to guest debug support patches

[Ian Pratt]

Kip,
Here's a compendium of collectd comments on the patch. Shouldn't take
long to either address them or explain them.

Best,
Ian


It should at some point be extended to allow multiple exec_domains
like gdb supports multiple threads in a single process.

patch1:
It seems to be missing a way to undo the foreign mappings on exit?

The memcpy copying the user_ctxt is now after if gets used (addtl.
VMX checks)  Looks like he wasn't too careful when forward
porting -- scary :-(  Maybe the VMX check should just look at the
context passed in.

Not sure why he's skipping setting DONEFPUINIT, kernel mode and clearing
IOPL bits though.

Doesn't the change in traps.c break in-guest debugger support?  It seems
to always pause the domain if it is in kernel, an in-guest debugger
which has set a breakpoint in the kernel will never get the int3.  I
think
this needs some kind of flag to enable/disable this behaviour.

patch2:
adresses point2 above

Now same concern applies to do_debug as for int3

I think the order in arch_final_setup_guest should be:

- check cs/ss in passed in cpu_ctxt (before doing anything)
- update DONEFPUINIT and TF_kernel_mode flags
- copy user_ctxt
- copy fpu ctxt
- clear iopl
- exit if updating (EDF_DONEINIT)



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click

Re: follow-up to guest debug support patches

[Keir Fraser]

On 12 Mar 2005, at 16:02, Ian Pratt wrote:

> Kip,
> Here's a compendium of collectd comments on the patch. Shouldn't take
> long to either address them or explain them.

Also, please merge the two patches together and sync them with head of 
tree.

  -- Keir



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: follow-up to guest debug support patches

[Kip Macy]

Thanks for the prompt response.
 
> It should at some point be extended to allow multiple exec_domains
> like gdb supports multiple threads in a single process.

As I pointed out in my original e-mail the absence of SMP support is temporary.
I'll add x86_64 support as soon as I have something to play with.

> patch1:
> It seems to be missing a way to undo the foreign mappings on exit?

Foreign mappings aren't automatically undone when the process exits
and the reference count goes down? If not, that seems like a major
problem - what if the program crashes?
 
> The memcpy copying the user_ctxt is now after if gets used (addtl.
> VMX checks)  Looks like he wasn't too careful when forward
> porting -- scary :-(  Maybe the VMX check should just look at the
> context passed in.
Fixed in patch 2.

> Not sure why he's skipping setting DONEFPUINIT, kernel mode and clearing
> IOPL bits though.

This is unchanged. If it looks weird it is an artifact of how the diff
is generated.
Why would I want to set DONEFPUINIT and clear IOPL more than once?
I've probably mis-parsed the sentence.

    if (!(c->flags & ECF_VMX_GUEST)) 
        if ( ((ed->arch.user_ctxt.cs & 3) == 0) ||
             ((ed->arch.user_ctxt.ss & 3) == 0) )
                return -EINVAL;

    if (test_bit(EDF_DONEINIT, &ed->ed_flags))
        return 0;

    clear_bit(EDF_DONEFPUINIT, &ed->ed_flags);
    if ( c->flags & ECF_I387_VALID )
        set_bit(EDF_DONEFPUINIT, &ed->ed_flags)
 
> Doesn't the change in traps.c break in-guest debugger support?  It seems
> to always pause the domain if it is in kernel, an in-guest debugger
> which has set a breakpoint in the kernel will never get the int3.  I
> think
> this needs some kind of flag to enable/disable this behaviour.

I thought about that, but is there an In-guest kernel debugger that
can set breakpoints? All I know of are stubs that require a serial
port. Does NetBSD's in-kernel debugger allow one to set breakpoints
and continue? If so I can just put the debugger option back in
Rules.mk.

> 
> patch2:
> adresses point2 above
> 
> Now same concern applies to do_debug as for int3
> 
> I think the order in arch_final_setup_guest should be:
> 
> - check cs/ss in passed in cpu_ctxt (before doing anything)
> - update DONEFPUINIT and TF_kernel_mode flags
> - copy user_ctxt
> - copy fpu ctxt
> - clear iopl
> - exit if updating (EDF_DONEINIT)

OK, I'll do that. I'll pull a new tree so I can unify the patches as
requested by Keir.

       -Kip


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: follow-up to guest debug support patches

[Christian Limpach]

On Sat, Mar 12, 2005 at 10:40:02AM -0800, Kip Macy wrote:
> > patch1:
> > It seems to be missing a way to undo the foreign mappings on exit?
> 
> Foreign mappings aren't automatically undone when the process exits
> and the reference count goes down? If not, that seems like a major
> problem - what if the program crashes?

They should get undone automatically, but if the domain crashes, it
might not get cleaned up until the debugging process exits -- which
might very well be what you want.

> > Not sure why he's skipping setting DONEFPUINIT, kernel mode and clearing
> > IOPL bits though.
> 
> This is unchanged. If it looks weird it is an artifact of how the diff
> is generated.
> Why would I want to set DONEFPUINIT and clear IOPL more than once?
> I've probably mis-parsed the sentence.

The code tries to bring the EDF_DONEFPUINIT flag in the exec domain
structure and the ECF_I387_VALID flags in sync with the newly passed
in full execution context in sync, i.e. EDF_DONEFPUINIT needs to be
set if and only if ECF_I387_VALID is set.  For IOPL, you copy the passed
in user_ctxt into the exec domain structure, the user_ctxt includes
eflags which needs to be cleared if the domain is not privileged.

> > Doesn't the change in traps.c break in-guest debugger support?  It seems
> > to always pause the domain if it is in kernel, an in-guest debugger
> > which has set a breakpoint in the kernel will never get the int3.  I
> > think
> > this needs some kind of flag to enable/disable this behaviour.
> 
> I thought about that, but is there an In-guest kernel debugger that
> can set breakpoints? All I know of are stubs that require a serial
> port. Does NetBSD's in-kernel debugger allow one to set breakpoints
> and continue? If so I can just put the debugger option back in
> Rules.mk.

Yes, the NetBSD in-kernel debugger allows you to set breakpoints
in the kernel.  The FreeBSD one should as well?  I'd prefer if
it was possible to set an EDF flag on the domain from dom0 which
controls the behaviour.  I guess you want the pausing behaviour all
the time such that if a domain causes a fault, it is paused and then
you can attach the debugger.  Could you change it such that the
new EDF flag gets set in setdomain (if the corresponding new ECF
flags is set, very similar to the FPU/I387 flag above) and then
make it a compile time option in the domain builder in libxc?
That way we can at some later point extend the tools to let you
specify the behaviour in the config file and/or have a tool/option
to change it for a running domain.  Of course, if you're brave and
highly motivated, you could also extend the tools ;-)

> OK, I'll do that. I'll pull a new tree so I can unify the patches as
> requested by Keir.

Thanks!  I think this is an excellent feature to have!

    christian



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: follow-up to guest debug support patches

[Kip Macy]

> They should get undone automatically, but if the domain crashes, it
> might not get cleaned up until the debugging process exits -- which
> might very well be what you want.

Yes it is.

> Yes, the NetBSD in-kernel debugger allows you to set breakpoints
> in the kernel.  The FreeBSD one should as well?  I'd prefer if

Probably. I find DDB so crufty that I've only ever used it after a crash.

> it was possible to set an EDF flag on the domain from dom0 which
> controls the behaviour.  I guess you want the pausing behaviour all
> the time such that if a domain causes a fault, it is paused and then

I had thought about this, but it seemed clumsy. For some reason it
hadn't occurred to me to make it a guest boot-time option. Thanks.

> highly motivated, you could also extend the tools ;-)

Given the time I'll do that.

> Thanks!  I think this is an excellent feature to have!

And thank you for the excellent comments and suggestions. 

                 -Kip


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click