Puthiyaparambil, Aravindh
2005-Oct-05 18:27 UTC
[Xen-devel] [PATCH] "lock cmpxch8b" and split locks
I have been looking at the following bug http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=252 That investigation is still underway. While looking at this problem I found that in "lock cmpxch8b &page->count_info" occurring in "get_page( )", "&page->count_info" is on a misaligned address. Furthermore the address of "page" is not modulo 64. So the "lock cmpxch8b" can happen across cache lines and lead to a split lock. On systems which have multiple buses, the atomicity of such a statement is not assured and can cause synchronization problems. I have attached a patch which solves this issue. Does anyone know if there are other places where the "lock" prefix is used with a cache misaligned address? Signed off by Aravindh Puthiyaparambil <aravindh.puthiyaparambil@unisys.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> I have been looking at the following bug > http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=252 > That investigation is still underway. > > While looking at this problem I found that in "lock cmpxch8b > &page->count_info" occurring in "get_page( )", > "&page->count_info" is on a misaligned address. Furthermore > the address of "page" is not modulo 64. So the "lock > cmpxch8b" can happen across cache lines and lead to a split > lock. On systems which have multiple buses, the atomicity of > such a statement is not assured and can cause synchronization > problems. > > I have attached a patch which solves this issue.We can''t just increase the size of pfn_info structure -- it has implications for the amount of VM space that Xen uses. This will require more thought. Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 5 Oct 2005, at 19:27, Puthiyaparambil, Aravindh wrote:> Does anyone know if there are other places where the "lock" prefix is > used with a cache misaligned address?x86 systems are supposed to guarantee that LOCKed instructions access their memory operand atomically, regardless of alignment (Vol 3 of the Intel reference manual). Your systems break this application-visible guarantee? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 5 Oct 2005, at 22:46, Keir Fraser wrote:>> Does anyone know if there are other places where the "lock" prefix is >> used with a cache misaligned address? > > x86 systems are supposed to guarantee that LOCKed instructions access > their memory operand atomically, regardless of alignment (Vol 3 of the > Intel reference manual). Your systems break this application-visible > guarantee?Also, the patch is way bigger and more invasive than it needs to be. There should be no need to make pfn_info bigger than it is. It''s currently a multiple of 8 bytes (e.g., 24 bytes on 32-bit) which is sufficient to avoid cache-line crossing of aligned 8-byte quantities. What if we just move ''tlbflush_timestamp'' to the end of the structure? A one-line fix? :-) -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Puthiyaparambil, Aravindh
2005-Oct-05 22:16 UTC
RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks
Which looks something like my attachment? :-) Signed off by: Aravindh Puthiyaparambil <aravindh.puthiyaparambil@unisys.com>> -----Original Message----- > From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk] > Sent: Wednesday, October 05, 2005 6:03 PM > To: Keir Fraser > Cc: Koren, Bradley J; xen-devel@lists.xensource.com; Puthiyaparambil, > Aravindh; Vessey, Bruce A; Subrahmanian, Raj > Subject: Re: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks > > > On 5 Oct 2005, at 22:46, Keir Fraser wrote: > > >> Does anyone know if there are other places where the "lock" prefixis> >> used with a cache misaligned address? > > > > x86 systems are supposed to guarantee that LOCKed instructionsaccess> > their memory operand atomically, regardless of alignment (Vol 3 ofthe> > Intel reference manual). Your systems break this application-visible > > guarantee? > > Also, the patch is way bigger and more invasive than it needs to be. > There should be no need to make pfn_info bigger than it is. It''s > currently a multiple of 8 bytes (e.g., 24 bytes on 32-bit) which is > sufficient to avoid cache-line crossing of aligned 8-byte quantities. > > What if we just move ''tlbflush_timestamp'' to the end of the structure? > A one-line fix? :-) > > -- Keir_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 5 Oct 2005, at 23:16, Puthiyaparambil, Aravindh wrote:> Which looks something like my attachment? :-) > > Signed off by: Aravindh Puthiyaparambil > <aravindh.puthiyaparambil@unisys.com>Turns out it breaks x86/64. I checked in a fixed up version. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Puthiyaparambil, Aravindh
2005-Oct-05 22:51 UTC
RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks
I just realized that moving ''tlbflush_timestamp'' to the end of the structure will make us trip up on cmpxchg(&page->u.inuse.type_info, x, nx) in put_page_type(). I think we have to come up with an alternative solution. Aravindh> -----Original Message----- > From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk] > Sent: Wednesday, October 05, 2005 6:40 PM > To: Puthiyaparambil, Aravindh > Cc: Vessey, Bruce A; xen-devel@lists.xensource.com; Koren, Bradley J; > Subrahmanian, Raj > Subject: Re: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks > > > On 5 Oct 2005, at 23:16, Puthiyaparambil, Aravindh wrote: > > > Which looks something like my attachment? :-) > > > > Signed off by: Aravindh Puthiyaparambil > > <aravindh.puthiyaparambil@unisys.com> > > Turns out it breaks x86/64. I checked in a fixed up version. > > -- Keir_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Puthiyaparambil, Aravindh
2005-Oct-06 04:53 UTC
RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks
Keir, Sorry about breaking x86_32. The fixed up patch takes care of the misalignment. Thanks, Aravindh PS: Please ignore my message below.> -----Original Message----- > From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel- > bounces@lists.xensource.com] On Behalf Of Puthiyaparambil, Aravindh > Sent: Wednesday, October 05, 2005 6:51 PM > To: Keir Fraser > Cc: Koren, Bradley J; xen-devel@lists.xensource.com; Vessey, Bruce A; > Subrahmanian, Raj > Subject: RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks > > I just realized that moving ''tlbflush_timestamp'' to the end of the > structure will make us trip up on cmpxchg(&page->u.inuse.type_info, x, > nx) in put_page_type(). > > I think we have to come up with an alternative solution. > > Aravindh > > > -----Original Message----- > > From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk] > > Sent: Wednesday, October 05, 2005 6:40 PM > > To: Puthiyaparambil, Aravindh > > Cc: Vessey, Bruce A; xen-devel@lists.xensource.com; Koren, BradleyJ;> > Subrahmanian, Raj > > Subject: Re: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks > > > > > > On 5 Oct 2005, at 23:16, Puthiyaparambil, Aravindh wrote: > > > > > Which looks something like my attachment? :-) > > > > > > Signed off by: Aravindh Puthiyaparambil > > > <aravindh.puthiyaparambil@unisys.com> > > > > Turns out it breaks x86/64. I checked in a fixed up version. > > > > -- Keir > > > _______________________________________________ > 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
Puthiyaparambil, Aravindh
2005-Oct-06 21:00 UTC
RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks
Keir, I spoke to our hardware engineers about this. They pointed me at Section 7.1.1 of Volume 3 of the Intel Software Developers Manual. "Accesses to cacheable memory that are split across bus widths, cache lines, and page boundaries are not guaranteed to be atomic by the Pentium 4, Intel Xeon, P6 family, Pentium, and Intel486 processors. The Pentium 4, Intel Xeon, and P6 family processors provide bus control signals that permit external memory subsystems to make split accesses atomic; however, on aligned data accesses will seriously impact the performance of the processor and should be avoided." I hope this gives you a better picture of the situation. Aravindh> -----Original Message----- > From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk] > Sent: Wednesday, October 05, 2005 5:47 PM > To: Puthiyaparambil, Aravindh > Cc: Subrahmanian, Raj; Vessey, Bruce A; xen-devel@lists.xensource.com; > Koren, Bradley J > Subject: Re: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks > > > On 5 Oct 2005, at 19:27, Puthiyaparambil, Aravindh wrote: > > > Does anyone know if there are other places where the "lock" prefixis> > used with a cache misaligned address? > > x86 systems are supposed to guarantee that LOCKed instructions access > their memory operand atomically, regardless of alignment (Vol 3 of the > Intel reference manual). Your systems break this application-visible > guarantee? > > -- Keir_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Nice try, but the first sentence of your quote applies only to ordinary (non-LOCKed) memory accesses. Section 7.1.2.2 states that "The integrity of a bus lock is not affected by the alignment of the memory field. The LOCK semantics are followed for as many bus cycles as necessary to update the entire operand." I''m sure you get away with this in practise. 64-bit quantities are the only simple type that does not get naturally aligned in x86 C ABI. cmpxchg8b is a pretty rare instruction and most users would be very careful to ensure correct alignment in the cases it is used. Luckily it was easy for us to make the necessary changes too. -- Keir On 6 Oct 2005, at 22:00, Puthiyaparambil, Aravindh wrote:> I spoke to our hardware engineers about this. They pointed me at > Section > 7.1.1 of Volume 3 of the Intel Software Developers Manual. > > "Accesses to cacheable memory that are split across bus widths, cache > lines, and page boundaries are not guaranteed to be atomic by the > Pentium 4, Intel Xeon, P6 family, Pentium, and Intel486 processors. The > Pentium 4, Intel Xeon, and P6 family processors provide bus control > signals that permit external memory subsystems to make split accesses > atomic; however, on aligned data accesses will seriously impact the > performance of the processor and should be avoided." > > I hope this gives you a better picture of the situation._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Nakajima, Jun
2005-Oct-07 19:21 UTC
RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks
Keir Fraser wrote:> Nice try, but the first sentence of your quote applies only to > ordinary (non-LOCKed) memory accesses. Section 7.1.2.2 states that > "The integrity of a bus lock is not affected by the alignment of the > memory field. The LOCK semantics are followed for as many bus cycles > as necessary to update the entire operand." > > I''m sure you get away with this in practise. 64-bit quantities are the > only simple type that does not get naturally aligned in x86 C ABI. > cmpxchg8b is a pretty rare instruction and most users would be very > careful to ensure correct alignment in the cases it is used. Luckily > it was easy for us to make the necessary changes too.But we don''t want to see unexpected #AC in ring0. Can check the bit 4 (Split-Lock Disable) and 8 (Suppress Lock Enable) of IA32_MISC_ENABLE MSR (0x1a0)? You may have set the bit 4. You want to set the bit 8, not bit 4.> > -- Keir > > On 6 Oct 2005, at 22:00, Puthiyaparambil, Aravindh wrote: > >> I spoke to our hardware engineers about this. They pointed me at >> Section >> 7.1.1 of Volume 3 of the Intel Software Developers Manual. >> >> "Accesses to cacheable memory that are split across bus widths, cache >> lines, and page boundaries are not guaranteed to be atomic by the >> Pentium 4, Intel Xeon, P6 family, Pentium, and Intel486 processors. >> The Pentium 4, Intel Xeon, and P6 family processors provide bus >> control signals that permit external memory subsystems to make split >> accesses atomic; however, on aligned data accesses will seriously >> impact the performance of the processor and should be avoided." >> >> I hope this gives you a better picture of the situation. > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-develJun --- Intel Open Source Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Puthiyaparambil, Aravindh
2005-Oct-11 14:13 UTC
RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks
Jun, I ended up writing a module which checked the IA32_MISC_ENABLE MSR on my system. I found that it was being set incorrectly for some logical processors. Bringing up a DomU (prior to fixing up the pfn_info structure) on one of these processors was the reason why the alignment check was being thrown. This was on a test system which was running a beta BIOS. So thank you for helping us find this bug. I owe you a beer at the next Xen summit. :-) Aravindh> -----Original Message----- > From: Nakajima, Jun [mailto:jun.nakajima@intel.com] > Sent: Friday, October 07, 2005 3:21 PM > To: Keir Fraser; Puthiyaparambil, Aravindh > Cc: Koren, Bradley J; xen-devel@lists.xensource.com; Subrahmanian,Raj;> Vessey, Bruce A > Subject: RE: [Xen-devel] [PATCH] "lock cmpxch8b" and split locks > > Keir Fraser wrote: > > Nice try, but the first sentence of your quote applies only to > > ordinary (non-LOCKed) memory accesses. Section 7.1.2.2 states that > > "The integrity of a bus lock is not affected by the alignment of the > > memory field. The LOCK semantics are followed for as many bus cycles > > as necessary to update the entire operand." > > > > I''m sure you get away with this in practise. 64-bit quantities arethe> > only simple type that does not get naturally aligned in x86 C ABI. > > cmpxchg8b is a pretty rare instruction and most users would be very > > careful to ensure correct alignment in the cases it is used. Luckily > > it was easy for us to make the necessary changes too. > > But we don''t want to see unexpected #AC in ring0. Can check the bit 4 > (Split-Lock Disable) and 8 (Suppress Lock Enable) of IA32_MISC_ENABLE > MSR (0x1a0)? You may have set the bit 4. You want to set the bit 8,not> bit 4._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel