Woller, Thomas
2006-May-09 18:24 UTC
[Xen-devel] [PATCH][SPT][DISCUSS] BUG() in shadow.h delete_shadow_status() with HVM guest
We are hitting the BUG() at the end of delete_shadow_status() in
xen/include/asm-x86/shadow.h with a PAE enabled 32bit unmodified windows
guest, on both SVM and VMX, with a 64bit hypervisor. After a PAE 32bit
winxpsp2 or win2003 guest, boots, and then has been properly shutdown -
performing a "xm destroy/xm shutdown" results in this BUG().
We clearly don''t understand the intricacies of the SPT code, but our
best guess seems to be that the BUG() is not valid in the failing case.
The theory is that since a single PDPT page can have multiple PDPs, then
maybe this function has already been called for this particular gmfn.
Took the same philosophy as with free_shadow_page() in shadow_public.c
line 766ish.
We are definitely unsure as to the viability of this fix as a final
solution, but the below fix alleviates this BUG() on both SVM and VMX
boxes. If anyone knowledgable in the SPT area could take a look at the
patch, we''d appreciate your thoughts.
thanks
Tom
# HG changeset patch
# User twoller@xen-trw2.amd.com
# Node ID c8e01ad41814e923c70e97877a22ae6ffeacb30a
# Parent 6e35b42994944e82550da99d03bcf9676b4ddec2
Fix a problem when destroying windows domains that are PAE enabled.
diff -r 6e35b4299494 -r c8e01ad41814 xen/include/asm-x86/shadow.h
--- a/xen/include/asm-x86/shadow.h Mon May 8 17:44:46 2006
+++ b/xen/include/asm-x86/shadow.h Tue May 9 15:40:35 2006
@@ -1352,6 +1352,17 @@
}
while ( x != NULL );
+#if CONFIG_PAGING_LEVELS == 4
+ /*
+ * Since a single PDPT page can have multiple PDPs, it''s possible
+ * that it has been already called for this gmfn.
+ */
+ if ( stype == PGT_l4_shadow && (!mfn_is_page_table(gmfn)) )
+ {
+ goto found;
+ }
+#endif
+
/* If we got here, it wasn''t in the list! */
BUG();
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Nakajima, Jun
2006-May-09 20:43 UTC
RE: [Xen-devel] [PATCH][SPT][DISCUSS] BUG() in shadow.h delete_shadow_status() with HVM guest
Tom, hi Thanks for the debugging. Woller, Thomas wrote:> We are hitting the BUG() at the end of delete_shadow_status() in > xen/include/asm-x86/shadow.h with a PAE enabled 32bit unmodified > windows guest, on both SVM and VMX, with a 64bit hypervisor. After a > PAE 32bit winxpsp2 or win2003 guest, boots, and then has been > properly shutdown - performing a "xm destroy/xm shutdown" results in > this BUG(). > > We clearly don''t understand the intricacies of the SPT code, but our > best guess seems to be that the BUG() is not valid in the failing > case. > > The theory is that since a single PDPT page can have multiple PDPs, > then maybe this function has already been called for this particular > gmfn. Took the same philosophy as with free_shadow_page() in > shadow_public.c line 766ish.I think this is a bit different because the hash key has the index of the PDP for PAE guests. I guess somehow tlbflush_timestamp has been modified. Can you try this patch? diff -r 1e3977e029fd xen/arch/x86/shadow.c --- a/xen/arch/x86/shadow.c Mon May 8 18:21:41 2006 +++ b/xen/arch/x86/shadow.c Tue May 9 13:20:33 2006 @@ -3467,7 +3467,9 @@ } else { printk("For non HVM shadow, create_l1_shadow:%d\n", create_l2_shadow); } - shadow_update_min_max(l4e_get_pfn(sl4e), l3_table_offset(va)); + + if ( v->domain->arch.ops->guest_paging_levels == PAGING_L4 ) + shadow_update_min_max(l4e_get_pfn(sl4e), l3_table_offset(va)); } But the second check !mfn_is_page_table(gmfn) is interesting. So I guess the gmfn is shadowed as a different level, i.e. a linear page table is used.> > We are definitely unsure as to the viability of this fix as a final > solution, but the below fix alleviates this BUG() on both SVM and VMX > boxes. If anyone knowledgable in the SPT area could take a look at > the patch, we''d appreciate your thoughts. > thanks > Tom > > > # HG changeset patch > # User twoller@xen-trw2.amd.com > # Node ID c8e01ad41814e923c70e97877a22ae6ffeacb30a > # Parent 6e35b42994944e82550da99d03bcf9676b4ddec2 > Fix a problem when destroying windows domains that are PAE enabled. > > diff -r 6e35b4299494 -r c8e01ad41814 xen/include/asm-x86/shadow.h > --- a/xen/include/asm-x86/shadow.h Mon May 8 17:44:46 2006 > +++ b/xen/include/asm-x86/shadow.h Tue May 9 15:40:35 2006 > @@ -1352,6 +1352,17 @@ > } > while ( x != NULL ); > > +#if CONFIG_PAGING_LEVELS == 4 > + /* > + * Since a single PDPT page can have multiple PDPs, it''s possible > + * that it has been already called for this gmfn. > + */ > + if ( stype == PGT_l4_shadow && (!mfn_is_page_table(gmfn)) ) > + { > + goto found; > + } > +#endif > + > /* If we got here, it wasn''t in the list! */ > BUG(); >Jun --- Intel Open Source Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Woller, Thomas
2006-May-09 23:06 UTC
RE: [Xen-devel] [PATCH][SPT][DISCUSS] BUG() in shadow.h delete_shadow_status() with HVM guest
> I think this is a bit different because the hash key has the > index of the PDP for PAE guests. I guess somehow > tlbflush_timestamp has been modified. Can you try this patch?Thanks for the reply and the fix - your patch was successful on both SVM and VMX boxes. I tested 32bit PAE win2003 server SE on SVM, and 32bit PAE Winxpsp2 on VMX. Both did not hit the BUG() in shadow.h. We definitely don''t have much priority with PAE here, might be prudent to let this patch sit with your more extensive PAE testing, including 32bit hv, etc. We''ll use your patch internally for a while, and indicate if we see an adverse side-affects. So, unless you indicate otherwise, I''ll defer to you to push up when you feel it''s a solid fix. thanks Tom> diff -r 1e3977e029fd xen/arch/x86/shadow.c > --- a/xen/arch/x86/shadow.c Mon May 8 18:21:41 2006 > +++ b/xen/arch/x86/shadow.c Tue May 9 13:20:33 2006 > @@ -3467,7 +3467,9 @@ > } else { > printk("For non HVM shadow, > create_l1_shadow:%d\n", create_l2_shadow); > } > - shadow_update_min_max(l4e_get_pfn(sl4e), > l3_table_offset(va)); > + > + if ( v->domain->arch.ops->guest_paging_levels == PAGING_L4 ) > + shadow_update_min_max(l4e_get_pfn(sl4e), > l3_table_offset(va)); >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Woller, Thomas
2006-Jun-01 21:08 UTC
RE: [Xen-devel] [PATCH][SPT][DISCUSS] BUG() in shadow.h delete_shadow_status() with HVM guest
Keir, we have been using this patch since May 8th on our internal trees,
and haven''t seen any negative consequences. Jun''s patch below
fixes a
problem with a hang when performing an "xm destroy" on a windows
guest.
We would like to see it go into the xen-unstable.hg tree, and
3.0-testing if you feel comfortable with it.
Thanks
Tom
Signed-off-by: Tom Woller <thomas.woller@amd.com>
diff -r 1e3977e029fd xen/arch/x86/shadow.c
--- a/xen/arch/x86/shadow.c Mon May 8 18:21:41 2006
+++ b/xen/arch/x86/shadow.c Tue May 9 13:20:33 2006
@@ -3467,7 +3467,9 @@
} else {
printk("For non HVM shadow, create_l1_shadow:%d\n",
create_l2_shadow);
}
- shadow_update_min_max(l4e_get_pfn(sl4e), l3_table_offset(va));
+
+ if ( v->domain->arch.ops->guest_paging_levels == PAGING_L4 )
+ shadow_update_min_max(l4e_get_pfn(sl4e),
l3_table_offset(va));
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Woller, Thomas
> Sent: Tuesday, May 09, 2006 6:07 PM
> To: Nakajima, Jun; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] [PATCH][SPT][DISCUSS] BUG() in
> shadow.h delete_shadow_status() with HVM guest
>
> > I think this is a bit different because the hash key has
> the index of
> > the PDP for PAE guests. I guess somehow tlbflush_timestamp has been
> > modified. Can you try this patch?
>
> Thanks for the reply and the fix - your patch was successful
> on both SVM and VMX boxes. I tested 32bit PAE win2003 server
> SE on SVM, and 32bit PAE Winxpsp2 on VMX. Both did not hit
> the BUG() in shadow.h.
>
> We definitely don''t have much priority with PAE here, might
> be prudent to let this patch sit with your more extensive PAE
> testing, including 32bit hv, etc. We''ll use your patch
> internally for a while, and indicate if we see an adverse
> side-affects.
>
> So, unless you indicate otherwise, I''ll defer to you to push
> up when you feel it''s a solid fix.
> thanks
> Tom
>
>
> > diff -r 1e3977e029fd xen/arch/x86/shadow.c
> > --- a/xen/arch/x86/shadow.c Mon May 8 18:21:41 2006
> > +++ b/xen/arch/x86/shadow.c Tue May 9 13:20:33 2006
> > @@ -3467,7 +3467,9 @@
> > } else {
> > printk("For non HVM shadow,
create_l1_shadow:%d\n",
> > create_l2_shadow);
> > }
> > - shadow_update_min_max(l4e_get_pfn(sl4e),
> > l3_table_offset(va));
> > +
> > + if ( v->domain->arch.ops->guest_paging_levels ==
> PAGING_L4 )
> > + shadow_update_min_max(l4e_get_pfn(sl4e),
> > l3_table_offset(va));
> >
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Nakajima, Jun
2006-Jun-02 00:58 UTC
RE: [Xen-devel] [PATCH][SPT][DISCUSS] BUG() in shadow.h delete_shadow_status() with HVM guest
The patch below is included in our next patch (3-on-3). It''s okay to
_prefetch_ that part.
Jun
---
Intel Open Source Technology Center
-----Original Message-----
From: Woller, Thomas [mailto:thomas.woller@amd.com]
Sent: Thursday, June 01, 2006 2:09 PM
To: Woller, Thomas; Nakajima, Jun; xen-devel@lists.xensource.com
Subject: RE: [Xen-devel] [PATCH][SPT][DISCUSS] BUG() in shadow.h
delete_shadow_status() with HVM guest
Keir, we have been using this patch since May 8th on our internal trees,
and haven''t seen any negative consequences. Jun''s patch below
fixes a
problem with a hang when performing an "xm destroy" on a windows
guest.
We would like to see it go into the xen-unstable.hg tree, and
3.0-testing if you feel comfortable with it.
Thanks
Tom
Signed-off-by: Tom Woller <thomas.woller@amd.com>
diff -r 1e3977e029fd xen/arch/x86/shadow.c
--- a/xen/arch/x86/shadow.c Mon May 8 18:21:41 2006
+++ b/xen/arch/x86/shadow.c Tue May 9 13:20:33 2006
@@ -3467,7 +3467,9 @@
} else {
printk("For non HVM shadow, create_l1_shadow:%d\n",
create_l2_shadow);
}
- shadow_update_min_max(l4e_get_pfn(sl4e), l3_table_offset(va));
+
+ if ( v->domain->arch.ops->guest_paging_levels == PAGING_L4 )
+ shadow_update_min_max(l4e_get_pfn(sl4e),
l3_table_offset(va));
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Woller, Thomas
> Sent: Tuesday, May 09, 2006 6:07 PM
> To: Nakajima, Jun; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] [PATCH][SPT][DISCUSS] BUG() in
> shadow.h delete_shadow_status() with HVM guest
>
> > I think this is a bit different because the hash key has
> the index of
> > the PDP for PAE guests. I guess somehow tlbflush_timestamp has been
> > modified. Can you try this patch?
>
> Thanks for the reply and the fix - your patch was successful
> on both SVM and VMX boxes. I tested 32bit PAE win2003 server
> SE on SVM, and 32bit PAE Winxpsp2 on VMX. Both did not hit
> the BUG() in shadow.h.
>
> We definitely don''t have much priority with PAE here, might
> be prudent to let this patch sit with your more extensive PAE
> testing, including 32bit hv, etc. We''ll use your patch
> internally for a while, and indicate if we see an adverse
> side-affects.
>
> So, unless you indicate otherwise, I''ll defer to you to push
> up when you feel it''s a solid fix.
> thanks
> Tom
>
>
> > diff -r 1e3977e029fd xen/arch/x86/shadow.c
> > --- a/xen/arch/x86/shadow.c Mon May 8 18:21:41 2006
> > +++ b/xen/arch/x86/shadow.c Tue May 9 13:20:33 2006
> > @@ -3467,7 +3467,9 @@
> > } else {
> > printk("For non HVM shadow,
create_l1_shadow:%d\n",
> > create_l2_shadow);
> > }
> > - shadow_update_min_max(l4e_get_pfn(sl4e),
> > l3_table_offset(va));
> > +
> > + if ( v->domain->arch.ops->guest_paging_levels ==
> PAGING_L4 )
> > + shadow_update_min_max(l4e_get_pfn(sl4e),
> > l3_table_offset(va));
> >
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel