From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39130) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QyIxH-0005Vj-BR for qemu-devel@nongnu.org; Tue, 30 Aug 2011 03:41:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QyIxF-0002b2-Ut for qemu-devel@nongnu.org; Tue, 30 Aug 2011 03:41:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:24528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QyIxF-0002av-Lj for qemu-devel@nongnu.org; Tue, 30 Aug 2011 03:41:53 -0400 Message-ID: <4E5C94EE.2000705@redhat.com> Date: Tue, 30 Aug 2011 09:44:46 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1314564200-6872-1-git-send-email-weil@mail.berlios.de> <4E5B1D42.1010605@mail.berlios.de> <4E5BEEBE.6070500@mail.berlios.de> In-Reply-To: <4E5BEEBE.6070500@mail.berlios.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/7] Fix packing for MinGW with -mms-bitfields List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: TeLeMan , QEMU Developers , Alexander Graf , Blue Swirl , Isaku Yamahata , Jan Kiszka , Gerd Hoffmann Am 29.08.2011 21:55, schrieb Stefan Weil: > Am 29.08.2011 10:34, schrieb TeLeMan: >> On Mon, Aug 29, 2011 at 13:01, Stefan Weil wrote: >>> Am 28.08.2011 23:43, schrieb Blue Swirl: >>>> >>>> On Sun, Aug 28, 2011 at 8:43 PM, Stefan Weil >>>> wrote: >>>>> >>>>> These patches fix the packing of structures which were affected by >>>>> the new compiler attribute -mms-bitfields (which is needed for >>>>> glib-2.0). >>>>> >>>>> I compiled qemu.exe with and without -mms-bitfields and compared >>>>> the resulting struct alignment using pahole and codiff. >>>> >>>> If a structure is only used internally by QEMU (not used in network, >>>> disk or guest interfaces), changes in padding don't matter. In fact, >>>> in those cases it may be better to remove the packing, because then >>>> the fields may be naturally aligned and that gives better performance >>>> on most architectures. Could you please check if this is the case for >>>> any of the structs? >>> >>> I did this already, but also forward your question to the maintainers. >>> Here is my result: >>> >>> [PATCH 2/7] block/vvfat: Fix packing for w32: needs packing (disk) >>> [PATCH 3/7] acpi: Fix packing for w32: needs packing (bios interface) >>> [PATCH 4/7] hpet: Fix packing for w32: needs packing (bios interface) >>> [PATCH 5/7] usb: Fix packing for w32: needs packing (usb interface) >>> [PATCH 6/7] virtio: Fix packing for w32: needs packing? (guest >>> interface?) >>> [PATCH 7/7] slirp: Fix packing for w32: needs packing (network interface) >>> >>> All those struct statements need the pack attribute (otherwise the code >>> would have to be rewritten which is of course always possible). >> gesn_cdb in atapi.c, VMDK4Header in vmdk.c and many structures in >> bt.h need be fixed too. > > Oops, you are right. Obviously I missed all anonymous structs: > codiff simply ignores them, and pahole must be called with > flags -a -A to show them. Who invented packing of structs? > > Comparing the output of pahole -a -A is less elegant than using > codiff, but shows the structs which you mentioned. > > I suggest to apply my patch series first because it fixes > the most important bugs in networking. The remaining > bugs are in code which is used less often. They will be > fixed by a second patch series which replaces all remaining > packed attributes. Shouldn't we have a look at every packed structure instead of just fixing what we notice as broken in the x86 emulator binary with one given configuration? I think if there is a QEMU_PACKED, we should use it consistently, or is there a reason not to do so? Kevin