From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53436) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsiUD-0002UH-LN for qemu-devel@nongnu.org; Mon, 16 Dec 2013 19:26:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VsiU9-0005Yw-AV for qemu-devel@nongnu.org; Mon, 16 Dec 2013 19:26:09 -0500 Received: from mail-pb0-f52.google.com ([209.85.160.52]:40519) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsiU9-0005Yr-4D for qemu-devel@nongnu.org; Mon, 16 Dec 2013 19:26:05 -0500 Received: by mail-pb0-f52.google.com with SMTP id uo5so6223076pbc.11 for ; Mon, 16 Dec 2013 16:26:04 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20131216234033.GF5711@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> From: Peter Maydell Date: Tue, 17 Dec 2013 00:25:43 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 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: Christoffer Dall Cc: Patch Tracking , QEMU Developers , "kvmarm@lists.cs.columbia.edu" 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. >> >> 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. thanks -- PMM