[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

On 20 March 2015 at 13:25, Will Deacon <will.deacon at arm.com> wrote:
> On Fri, Mar 20, 2015 at 11:45:17AM +0000, Ard Biesheuvel wrote:
>> On 20 March 2015 at 12:41, Mark Rutland <mark.rutland at arm.com> wrote:
>> >> >> +     if (boot_args[1] || boot_args[2] || boot_args[3]) {
>> >> >> +             pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
>> >> >> +                     boot_args[1], boot_args[2], boot_args[3]);
>> >> >> +             pr_err("WARNING: your bootloader may fail to load newer kernels\n");
>> >> >
>> >> > If we ever decide to use x1-x3 for something, and try to boot an older
>> >> > kernel, that warning is going to be a bit misleading. That could matter
>> >> > for VMs where we're going to see old kernel images for a long time.
>> >> >
>> >> > I would like the warning to mention that could be the case.
>> >> >
>> >> > It would also be nice if the message were consistently spaced regardless
>> >> > of the values of x1-x3, so we should zero-pad them (and as that takes a
>> >> > resonable amount of space, let's give them a line each).
>> >> >
>> >> > So could we change the warning to be something like:
>> >> >
>> >> >         pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"
>> >> >                "\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"
>> >> >                "This indicates a broken bootloader or old kernel\n",
>> >> >                boot_args[1], boot_args[2], boot_args[3]);
>> >> >
>> >>
>> >> OK, I have applied this change.
>> >>
>> >> But I would like to note that we should probably only extend the boot
>> >> protocol in a way that would not trigger this on older kernels in the
>> >> first place.
>> >> I.e., assign a bit in the flags field in the header, which indicates
>> >> whether some boot protocol enhancement is supported by the kernel
>> >> being loaded, and only allow x1/x2/x3 to be non-zero if said
>> >> enhancement defines that.
>> >
>> > Good point.
>> >
>> > Given that, if you want to restore your original last line, that would
>> > be fine with me (and my Ack still applies).
>> >
>>
>> I think it's fine to leave it as is
>
> Yup, and this is sitting pretty on the arm64 devel branch.
>
> Ard: I also pushed a kvm-bounce-page branch for you. Next step would be to
> merge everything into for-next/core and put your VA changes on top of that.
>
> I'd appreciate a sanity check of the current branch first, though!
>

OK, I pulled devel and put the KVM and VA branches on top, and
everything builds and runs fine afaict.

I also checked that Arnd's dotconfig from hell does not have the false
negative on the HYP text size assert.

"""
0x00000001                ASSERT ((((__hyp_idmap_text_start & 0xfff) +
__hyp_idmap_size) <= 0x1000), , HYP init code too big or misaligned)
"""

(Note that this .config does not even produce a bootable zImage.)

You will get a conflict in head.S when you merge the core VA range patch.
Nothing major but I'm happy to resend the patch.