From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Kciqf-0004qP-Ne for qemu-devel@nongnu.org; Mon, 08 Sep 2008 11:40:17 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Kciqf-0004ps-0R for qemu-devel@nongnu.org; Mon, 08 Sep 2008 11:40:17 -0400 Received: from [199.232.76.173] (port=58449 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Kciqe-0004pm-NA for qemu-devel@nongnu.org; Mon, 08 Sep 2008 11:40:16 -0400 Received: from mx2.redhat.com ([66.187.237.31]:35425) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Kciqd-0001gD-Dm for qemu-devel@nongnu.org; Mon, 08 Sep 2008 11:40:16 -0400 Date: Mon, 8 Sep 2008 12:38:01 -0300 From: Glauber Costa Message-ID: <20080908153801.GA3724@poweredge.glommer> References: <1220303503-19413-1-git-send-email-glommer@redhat.com> <48BCFBB1.4090109@qumranet.com> <5d6222a80809020407l68f7ab87i6d2520c57c7ddeb3@mail.gmail.com> <48BD59D7.7000702@qumranet.com> <20080903192700.GA8000@poweredge.glommer> <48C377BF.1020700@qumranet.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Dxnq1zWXvFF0Q93v" Content-Disposition: inline In-Reply-To: <48C377BF.1020700@qumranet.com> Subject: [Qemu-devel] Re: [PATCH] Fix up pxe boot Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: aliguori@us.ibm.com, kvm@vger.kernel.org, apevec@redhat.com, Glauber Costa , qemu-devel@nongnu.org, chrisw@sous-sol.org, Eduardo Habkost --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Sep 07, 2008 at 09:42:07AM +0300, Avi Kivity wrote: > Glauber Costa wrote: >> After a second look, here's what it seems to me: >> >> It's not in a generic place, such as ldl, because in general, we may want to grab >> a 32-bit value from a 64-bit address. This is perfectly valid. >> >> It's a specifity that the pop instruction, when not in long mode (manual says that in 64-bit mode >> no 32-bit operand is valid, but then again, qemu should use the POPQ macro), that ssp:sp may overflow, >> but we don't want it. >> >> It would be possible to do something more generic if we had a segment_to_linear() function, that returned >> the linear address, but we don't. >> >> Does it make more sense to you? >> > > Yes. > > I guess tcg code is mostly safe since it generates 32-bit additions for > segment bases, so this is limited to the places you identified. And a > helper to add segment bases would be helpful. > > -- > error compiling committee.c: too many arguments to function > what do you think of the attached version? --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="0001-Fix-up-pxe-boot.patch" >>From e185d17904febce8b9fe0b0d403c0ee9df92ca38 Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Mon, 1 Sep 2008 17:49:23 -0300 Subject: [PATCH] Fix up pxe boot As discussed in http://lists.gnu.org/archive/html/qemu-devel/2008-08/msg00667.html, current pxe boot is broken for some use cases. The problem goes away if we reduce the number of allowed bits in the address space to 32 (which has the side effect of reducing guest max mem size to 4Gb). After digging for a while, it turns out that it happens because pxelinux tries to access address 0x10009e9a6, which does not fit a 32-bit address. A closer look, however, reveals this access is totally valid: It's just 0x9e9a6 with an add carry. To avoid this, this patch casts the address passed to the POPL macro to a 32-bit value. This is also done, although just theorectically, for PUSHL too. Signed-off-by: Glauber Costa Reported-by: Chris Lalancette CC: Eduardo Habkost --- target-i386/op_helper.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c index 0b5fdc0..4c3ee06 100644 --- a/target-i386/op_helper.c +++ b/target-i386/op_helper.c @@ -590,6 +590,10 @@ do {\ #define SET_ESP(val, sp_mask) ESP = (ESP & ~(sp_mask)) | ((val) & (sp_mask)) #endif +/* in 64-bit machines, this can overflow. So this segment addition macro + * can be used to trim the value to 32-bit whenever needed */ +#define SEG_ADDL(ssp, sp, sp_mask) ((uint32_t)((ssp) + (sp & (sp_mask)))) + /* XXX: add a is_user flag to have proper security support */ #define PUSHW(ssp, sp, sp_mask, val)\ {\ @@ -600,7 +604,7 @@ do {\ #define PUSHL(ssp, sp, sp_mask, val)\ {\ sp -= 4;\ - stl_kernel((ssp) + (sp & (sp_mask)), (val));\ + stl_kernel(SEG_ADDL(ssp, sp, sp_mask), (uint32_t)(val));\ } #define POPW(ssp, sp, sp_mask, val)\ @@ -611,7 +615,7 @@ do {\ #define POPL(ssp, sp, sp_mask, val)\ {\ - val = (uint32_t)ldl_kernel((ssp) + (sp & (sp_mask)));\ + val = (uint32_t)ldl_kernel(SEG_ADDL(ssp, sp, sp_mask));\ sp += 4;\ } -- 1.5.5.1 --Dxnq1zWXvFF0Q93v--