From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33708) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsmcL-0006LH-N0 for qemu-devel@nongnu.org; Mon, 16 Dec 2013 23:50:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VsmcG-0006Hh-F9 for qemu-devel@nongnu.org; Mon, 16 Dec 2013 23:50:49 -0500 Received: from mail-pb0-f42.google.com ([209.85.160.42]:37482) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsmcG-0006HQ-6q for qemu-devel@nongnu.org; Mon, 16 Dec 2013 23:50:44 -0500 Received: by mail-pb0-f42.google.com with SMTP id uo5so6452358pbc.15 for ; Mon, 16 Dec 2013 20:50:42 -0800 (PST) Date: Mon, 16 Dec 2013 20:50:41 -0800 From: Christoffer Dall Message-ID: <20131217045041.GJ5711@cbox> References: <1385645602-18662-1-git-send-email-peter.maydell@linaro.org> <1385645602-18662-7-git-send-email-peter.maydell@linaro.org> <20131216234033.GF5711@cbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 6/7] hw/arm/boot: Add boot support for AArch64 processor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Patch Tracking , QEMU Developers , "kvmarm@lists.cs.columbia.edu" On Tue, Dec 17, 2013 at 12:25:43AM +0000, Peter Maydell wrote: > On 16 December 2013 23:40, Christoffer Dall wrote: > > On Thu, Nov 28, 2013 at 01:33:21PM +0000, Peter Maydell wrote: > >> From: "Mian M. Hamayun" > >> > >> This commit adds support for booting a single AArch64 CPU by setting > >> appropriate registers. The bootloader includes placehoders for Board-ID > >> that are used to implement uniform indexing across different bootloaders. > >> > >> Signed-off-by: Mian M. Hamayun > >> [PMM: > >> * updated to use ARMInsnFixup style bootloader fragments > >> * dropped virt.c additions > >> * use runtime checks for "is this an AArch64 core" rather than ifdefs > >> * drop some unnecessary setting of registers in reset hook > >> ] > >> Signed-off-by: Peter Maydell > >> --- > >> hw/arm/boot.c | 40 +++++++++++++++++++++++++++++++++++----- > >> 1 file changed, 35 insertions(+), 5 deletions(-) > >> > >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c > >> index 77d29a8..b6b04b7 100644 > >> --- a/hw/arm/boot.c > >> +++ b/hw/arm/boot.c > >> @@ -19,6 +19,8 @@ > >> > >> #define KERNEL_ARGS_ADDR 0x100 > >> #define KERNEL_LOAD_ADDR 0x00010000 > >> +/* The AArch64 kernel boot protocol specifies a different preferred address */ > >> +#define KERNEL64_LOAD_ADDR 0x00080000 > > > > I assume you referring to Documentation/arm/Booting and > > Documentation/arm64/booting.txt here? Perhaps it would be nicer to > > refer to that and state how we reach the address for the two archs > > instead of having the aarch64 specific note here, e.g. "The kernel > > recommends booting at offset 0x80000 from system RAM" or something like > > that... > > Yeah, we could put the references to the document names in. > I don't see the point repeating the 0x80000 figure in the comment though. > I didn't mean it that literal, just that now it's sort of weird that there's no comment for 32-bit, but something for the aarch64, which sort of suggests that any braindead monkey of course knows the 32-bit details:) But it's also something that can be fixed later if it creates a rebasing headache for you at this point. > >> > >> typedef enum { > >> FIXUP_NONE = 0, /* do nothing */ > >> @@ -37,6 +39,20 @@ typedef struct ARMInsnFixup { > >> FixupType fixup; > >> } ARMInsnFixup; > >> > >> +static const ARMInsnFixup bootloader_aarch64[] = { > >> + { 0x580000c0 }, /* ldr x0, 18 ; Load the lower 32-bits of DTB */ > > > > so by 18 you mean the label 0x18 assuming the instruction above has the > > label 0 or something like that? Is this an accepted/familiar notation > > or shoudl we do something like the arm32 bootloaders and "define a > > label in the comments"...? > > referring down to a label or something would be better. This is probably > just a copy of output from a disassembler of a binary blob with assumed > offset zero. > Yeah, I actually dumped your binary values and did a raw disassembly to check that you weren't lying in the comments, and my disassembler gave me the hex versions so I sort of figured, but that could be made easier for the readers who don't want to do that or possess an intimate knowledge of the aarch64 opcodes in their head. -Christoffer