On Tue, Dec 17, 2013 at 12:25:43AM +0000, Peter Maydell wrote:
> On 16 December 2013 23:40, Christoffer Dall <christoffer.dall@xxxxxxxxxx> wrote:
> > On Thu, Nov 28, 2013 at 01:33:21PM +0000, Peter Maydell wrote:
> >> From: "Mian M. Hamayun" <m.hamayun@xxxxxxxxxxxxxxxxxxxxxx>
> >>
> >> 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 <m.hamayun@xxxxxxxxxxxxxxxxxxxxxx>
> >> [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 <peter.maydell@xxxxxxxxxx>
> >> ---
> >> 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
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
