From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [Qemu-devel] Re: [PATCH] pc: e820 qemu_cfg tables need to be packed Date: Thu, 14 Oct 2010 23:19:09 +0200 Message-ID: <201010142319.09412.arnd@arndb.de> References: <20101014183249.23510.29196.stgit@s20.home> <201010142220.26329.arnd@arndb.de> <1287089944.2987.33.camel@x201> Mime-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, Anthony Liguori , Jes Sorensen , kvm@vger.kernel.org To: Alex Williamson Return-path: Received: from moutng.kundenserver.de ([212.227.17.10]:53390 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755927Ab0JNVTZ (ORCPT ); Thu, 14 Oct 2010 17:19:25 -0400 In-Reply-To: <1287089944.2987.33.camel@x201> Sender: kvm-owner@vger.kernel.org List-ID: On Thursday 14 October 2010 22:59:04 Alex Williamson wrote: > The structs in question only contain 4 & 8 byte elements, so there > shouldn't be any change on x86-32 using one-byte aligned packing. I'm talking about the alignment of the structure, not the members within the structure. The data structure should be compatible, but not accesses to it. > AFAIK, e820 is x86-only, so we don't need to worry about breaking anyone > else. You can use qemu to emulate an x86 pc on anything... > Performance isn't much of a consideration for this type of > interface since it's only used pre-boot. In fact, the channel between > qemu and the bios is only one byte wide, so wider alignment can cost > extra emulated I/O accesses. Right, the data gets passed as bytes, so it hardly matters in the end. Still the e820_add_entry assigns data to the struct members, which it either does using byte accesses and shifts or a multiple 32 bit assignment. Just because using a one byte alignment technically results in correct output doesn't make it the right solution. I don't care about the few cycles of execution time or the few bytes you waste in this particular case, but you are setting a wrong example by using smaller alignment than necessary. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=47171 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P6VDE-0006jk-5H for qemu-devel@nongnu.org; Thu, 14 Oct 2010 17:19:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P6VDB-0007Xw-I0 for qemu-devel@nongnu.org; Thu, 14 Oct 2010 17:19:42 -0400 Received: from moutng.kundenserver.de ([212.227.17.10]:64677) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P6VDB-0007Xs-6Z for qemu-devel@nongnu.org; Thu, 14 Oct 2010 17:19:41 -0400 From: Arnd Bergmann Subject: Re: [Qemu-devel] Re: [PATCH] pc: e820 qemu_cfg tables need to be packed Date: Thu, 14 Oct 2010 23:19:09 +0200 References: <20101014183249.23510.29196.stgit@s20.home> <201010142220.26329.arnd@arndb.de> <1287089944.2987.33.camel@x201> In-Reply-To: <1287089944.2987.33.camel@x201> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201010142319.09412.arnd@arndb.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: Jes Sorensen , qemu-devel@nongnu.org, kvm@vger.kernel.org On Thursday 14 October 2010 22:59:04 Alex Williamson wrote: > The structs in question only contain 4 & 8 byte elements, so there > shouldn't be any change on x86-32 using one-byte aligned packing. I'm talking about the alignment of the structure, not the members within the structure. The data structure should be compatible, but not accesses to it. > AFAIK, e820 is x86-only, so we don't need to worry about breaking anyone > else. You can use qemu to emulate an x86 pc on anything... > Performance isn't much of a consideration for this type of > interface since it's only used pre-boot. In fact, the channel between > qemu and the bios is only one byte wide, so wider alignment can cost > extra emulated I/O accesses. Right, the data gets passed as bytes, so it hardly matters in the end. Still the e820_add_entry assigns data to the struct members, which it either does using byte accesses and shifts or a multiple 32 bit assignment. Just because using a one byte alignment technically results in correct output doesn't make it the right solution. I don't care about the few cycles of execution time or the few bytes you waste in this particular case, but you are setting a wrong example by using smaller alignment than necessary. Arnd