[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [BUGFIX][PATCH v2 2/5] dbg_rw_guest_mem: need to call put_gfn in error path.
- To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Don Slutz <dslutz@xxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx
- From: Don Slutz <dslutz@xxxxxxxxxxx>
- Date: Tue, 07 Jan 2014 20:06:30 -0500
- Cc: Keir Fraser <keir@xxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
- Delivery-date: Wed, 08 Jan 2014 01:06:44 +0000
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
On 01/07/14 19:55, Andrew Cooper wrote:
On 08/01/2014 00:25, Don Slutz wrote:
Using a 1G hvm domU (in grub) and gdbsx:
(gdb) set arch i8086
warning: A handler for the OS ABI "GNU/Linux" is not built into this
configuration
of GDB. Attempting to continue with the default i8086 settings.
The target architecture is assumed to be i8086
(gdb) target remote localhost:9999
Remote debugging using localhost:9999
Remote debugging from host 127.0.0.1
0x0000d475 in ?? ()
(gdb) x/1xh 0x6ae9168b
Will reproduce this bug.
With a debug=y build you will get:
Assertion '!preempt_count()' failed at preempt.c:37
For a debug=n build you will get a dom0 VCPU hung (at some point) in:
[ffff82c4c0126eec] _write_lock+0x3c/0x50
ffff82c4c01e43a0 __get_gfn_type_access+0x150/0x230
ffff82c4c0158885 dbg_rw_mem+0x115/0x360
ffff82c4c0158fc8 arch_do_domctl+0x4b8/0x22f0
ffff82c4c01709ed get_page+0x2d/0x100
ffff82c4c01031aa do_domctl+0x2ba/0x11e0
ffff82c4c0179662 do_mmuext_op+0x8d2/0x1b20
ffff82c4c0183598 __update_vcpu_system_time+0x288/0x340
ffff82c4c015c719 continue_nonidle_domain+0x9/0x30
ffff82c4c012938b add_entry+0x4b/0xb0
ffff82c4c02223f9 syscall_enter+0xa9/0xae
And gdb output:
(gdb) x/1xh 0x6ae9168b
0x6ae9168b: 0x3024
(gdb) x/1xh 0x6ae9168b
0x6ae9168b: Ignoring packet error, continuing...
Reply contains invalid hex digit 116
The 1st one worked because the p2m.lock is recursive and the PCPU
had not yet changed.
crash reports (for example):
crash> mm_rwlock_t 0xffff83083f913010
struct mm_rwlock_t {
lock = {
raw = {
lock = 2147483647
},
debug = {<No data fields>}
},
unlock_level = 0,
recurse_count = 1,
locker = 1,
locker_function = 0xffff82c4c022c640 <__func__.13514> "__get_gfn_type_access"
}
Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
Technically this should include by Signed-off-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> tag (being the author of the code) as well
as your own (being the discoverer of the bug and author of the commit
message), but I notice I accidentally omitted it from the original email
thread, so my apologies.
I was not sure if I should have added it without you adding it... So I went
without.
It should probably also include your Tested-by: tag
That is fine with me. Should I make a v3 of just this with both tags?
-Don Slutz
Ian (with RM hat on):
This is a hypervisor reference counting error on a toolstack hypercall
path. Irrespective of any of the other patches in this series, I think
this should be included ASAP (although probably subject to review from a
third person), which will fix the hypervisor crashes from gdbsx usage.
~Andrew
---
xen/arch/x86/debug.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index a67a192..ba6a64d 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -63,10 +63,17 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
if ( p2m_is_readonly(gfntype) && toaddr )
{
DBGP2("kdb:p2m_is_readonly: gfntype:%x\n", gfntype);
- return INVALID_MFN;
+ mfn = INVALID_MFN;
}
DBGP2("X: vaddr:%lx domid:%d mfn:%lx\n", vaddr, dp->domain_id, mfn);
+
+ if ( mfn == INVALID_MFN )
+ {
+ put_gfn(dp, *gfn);
+ *gfn = INVALID_GFN;
+ }
+
return mfn;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|