From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LUGRl-000309-MK for qemu-devel@nongnu.org; Tue, 03 Feb 2009 03:15:53 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LUGRj-0002zt-Br for qemu-devel@nongnu.org; Tue, 03 Feb 2009 03:15:52 -0500 Received: from [199.232.76.173] (port=38031 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LUGRj-0002zq-50 for qemu-devel@nongnu.org; Tue, 03 Feb 2009 03:15:51 -0500 Received: from mx20.gnu.org ([199.232.41.8]:20749) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LUGRi-0006lm-O1 for qemu-devel@nongnu.org; Tue, 03 Feb 2009 03:15:50 -0500 Received: from mr01.hansenet.de ([213.191.74.10]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LUGRh-000337-S2 for qemu-devel@nongnu.org; Tue, 03 Feb 2009 03:15:50 -0500 Message-ID: <4987FD29.3040404@exactcode.de> Date: Tue, 03 Feb 2009 09:15:37 +0100 From: Rene Rebe MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Add multi-boot kernel loading support References: <49873680.2040603@exactcode.de> <498776EC.1050208@codemonkey.ws> In-Reply-To: <498776EC.1050208@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: Quoted-Printable Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Alexander Graf Anthony Liguori wrote: ... >> + memcpy(p, gdt, sizeof(gdt)); >> + p+=3Dsizeof(gdt); >> + *pgdt++ =3D gdtr; >> + *pgdt++ =3D gdtr >> 8; >> + *pgdt++ =3D gdtr >> 16; >> + *pgdt++ =3D gdtr >> 24; >> + } >=20 > It was already pushing the limit to embed the linux loader. This is a=20 > bit too much. Perhaps we should split it to a separate .S file? I=20 > guess we could apply it as is but I'd like to see this code (along with= =20 > the Linux code) moved to another file at least. How do you want to assemble the .S file on non x86 builds, or just ship a pre-built object like the BIOS? >> + fprintf(stderr, "qemu: multiboot loader code is %d bytes=20 >> long.\n", (int)(p-rom)); >> + >> + /* sign rom */ >> + sum =3D 0; >> + for (i =3D 0; i < (sizeof(rom) - 1); i++) >> + sum +=3D rom[i]; >> + rom[sizeof(rom) - 1] =3D -sum; >> + >> + memcpy(option_rom, rom, sizeof(rom)); >=20 > This too could be common code. Ok. + } else { >> + /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */ >> + uint32_t mh_header_addr =3D ldl_p(header+i+12); >> + mh_load_addr =3D ldl_p(header+i+16); >=20 > Mixing variable definitions and code. Yuck. >> + fprintf(stderr, "multiboot: mh_header_addr =3D %#x\n",=20 >> mh_header_addr); >> + fprintf(stderr, "multiboot: mh_load_addr =3D %#x\n",=20 >> mh_load_addr); >> + fprintf(stderr, "multiboot: mh_load_end_addr =3D %#x\n",=20 >> mh_load_end_addr); >> + fprintf(stderr, "multiboot: mh_bss_end_addr =3D %#x\n",=20 >> mh_bss_end_addr); >=20 > It's a bit chatty by default. Yes, indeed - though quite handy for a yet new feature. I could just drop those (maybe after testing booting some other multi-boot kernel as well, ...). > Weird indent. >=20 > What all uses multiboot at this point? I know Xen does. Maybe other homebrew desktop kernels. I remeber there also went support into NetBSD some time ago. > Some additional documentation would be nice too. Greetings, Rene --=20 Ren=E9 Rebe - ExactCODE GmbH - Europe, Germany, Berlin http://exactcode.de | http://t2-project.org | http://rene.rebe.name