From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC PATCH 1/3] ARM, arm64: kvm: get rid of the bounce page
Date: Thu, 26 Feb 2015 18:24:40 +0000 [thread overview]
Message-ID: <54EF64E8.4090605@arm.com> (raw)
In-Reply-To: <CAKv+Gu_HuVuh7VE8shn4WnLfq0ankvkt2uSP2eG_2cczRAfApw@mail.gmail.com>
On 26/02/15 17:31, Ard Biesheuvel wrote:
> On 26 February 2015 at 16:10, Marc Zyngier <marc.zyngier@arm•com> wrote:
>> On 26/02/15 15:29, Ard Biesheuvel wrote:
>>> The HYP init bounce page is a runtime construct that ensures that the
>>> HYP init code does not cross a page boundary. However, this is something
>>> we can do perfectly well at build time, by aligning the code appropriately.
>>>
>>> For arm64, we just align to 4 KB, and enforce that the code size is less
>>> than 4 KB, regardless of the chosen page size.
>>>
>>> For ARM, the whole code is less than 256 bytes, so we tweak the linker
>>> script to align at a power of 2 upper bound of the code size
>>>
>>> Note that this also fixes a benign off-by-one error in the original bounce
>>> page code, where a bounce page would be allocated unnecessarily if the code
>>> was exactly 1 page in size.
>>
>> I really like this simplification. Can you please check that it still
>> work on 32bit with this patch from Arnd?
>>
>> https://www.mail-archive.com/kvm at vger.kernel.org/msg112364.html
>>
>
> Yes, it does.
>
> Note that the kernel's RODATA permissions shouldn't affect whether
> this code is executable or not in HYP mode, so I think this code
> belongs in RODATA in the 1st place.
Yup. Maybe we should do the same for arm64, shouldn't we?
>> Another question below:
>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro•org>
>>> ---
>>> arch/arm/kernel/vmlinux.lds.S | 12 +++++++++---
>>> arch/arm/kvm/init.S | 11 +++++++++++
>>> arch/arm/kvm/mmu.c | 42 +++++------------------------------------
>>> arch/arm64/kernel/vmlinux.lds.S | 18 ++++++++++++------
>>> 4 files changed, 37 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>>> index b31aa73e8076..8179d3903dee 100644
>>> --- a/arch/arm/kernel/vmlinux.lds.S
>>> +++ b/arch/arm/kernel/vmlinux.lds.S
>>> @@ -23,7 +23,7 @@
>>> VMLINUX_SYMBOL(__idmap_text_start) = .; \
>>> *(.idmap.text) \
>>> VMLINUX_SYMBOL(__idmap_text_end) = .; \
>>> - . = ALIGN(32); \
>>> + . = ALIGN(1 << __hyp_idmap_align_order); \
>>> VMLINUX_SYMBOL(__hyp_idmap_text_start) = .; \
>>> *(.hyp.idmap.text) \
>>> VMLINUX_SYMBOL(__hyp_idmap_text_end) = .;
>>> @@ -346,8 +346,14 @@ SECTIONS
>>> */
>>> ASSERT((__proc_info_end - __proc_info_begin), "missing CPU support")
>>> ASSERT((__arch_info_end - __arch_info_begin), "no machine record defined")
>>> +
>>> /*
>>> - * The HYP init code can't be more than a page long.
>>> + * The HYP init code can't be more than a page long,
>>> + * and should not cross a page boundary.
>>> * The above comment applies as well.
>>> */
>>> -ASSERT(((__hyp_idmap_text_end - __hyp_idmap_text_start) <= PAGE_SIZE), "HYP init code too big")
>>> +ASSERT(((__hyp_idmap_text_end - 1) & PAGE_MASK) -
>>> + (__hyp_idmap_text_start & PAGE_MASK) == 0,
>>> + "HYP init code too big or unaligned")
>>> +ASSERT(__hyp_idmap_size <= (1 << __hyp_idmap_align_order),
>>> + "__hyp_idmap_size should be <= (1 << __hyp_idmap_align_order)")
>>> diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S
>>> index 3988e72d16ff..7a279bc8e0e1 100644
>>> --- a/arch/arm/kvm/init.S
>>> +++ b/arch/arm/kvm/init.S
>>> @@ -157,3 +157,14 @@ target: @ We're now in the trampoline code, switch page tables
>>> __kvm_hyp_init_end:
>>>
>>> .popsection
>>> +
>>> + /*
>>> + * When making changes to this file, make sure that the value of
>>> + * __hyp_idmap_align_order is updated if the size of the code ends up
>>> + * exceeding (1 << __hyp_idmap_align_order). This helps ensure that the
>>> + * code never crosses a page boundary, without wasting too much memory
>>> + * on aligning to PAGE_SIZE.
>>> + */
>>> + .global __hyp_idmap_size, __hyp_idmap_align_order
>>> + .set __hyp_idmap_size, __kvm_hyp_init_end - __kvm_hyp_init
>>> + .set __hyp_idmap_align_order, 8
>>
>> Is there a way to generate this __hyp_idmap_align_order automatically?
>> We're already pretty close to this 8 bit limit...
>>
>
> This seems to work:
>
> #define HYP_IDMAP_ALIGN \
> __hyp_idmap_size <= 0x100 ? 0x100 : \
> __hyp_idmap_size <= 0x200 ? 0x200 : \
> __hyp_idmap_size <= 0x400 ? 0x400 : \
> __hyp_idmap_size <= 0x800 ? 0x800 : 0x1000
>
> and
>
> . = ALIGN(HYP_IDMAP_ALIGN); \
>
> and we are limited at 1 page anyway.
Ah, excellent.
> Should I respin and include the move to RODATA at the same time?
> Or would you like me to rebase onto Arnd's patch?
[adding Arnd on CC]
Rebasing on top of Arnd's patch seems fair, as he came up with the idea
the first place.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2015-02-26 18:24 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-26 15:29 [RFC PATCH 0/3] arm64: KVM: use increased VA range if needed Ard Biesheuvel
2015-02-26 15:29 ` [RFC PATCH 1/3] ARM, arm64: kvm: get rid of the bounce page Ard Biesheuvel
2015-02-26 16:10 ` Marc Zyngier
2015-02-26 17:31 ` Ard Biesheuvel
2015-02-26 18:24 ` Marc Zyngier [this message]
2015-02-26 18:41 ` Ard Biesheuvel
2015-02-27 8:19 ` Ard Biesheuvel
2015-02-26 15:29 ` [RFC PATCH 2/3] arm64: make ID map shareable with EL2 Ard Biesheuvel
2015-02-26 15:29 ` [RFC PATCH 3/3] arm64: KVM: use ID map with increased VA range if required Ard Biesheuvel
2015-02-26 16:11 ` Catalin Marinas
2015-02-26 16:29 ` Catalin Marinas
2015-02-26 16:56 ` Ard Biesheuvel
2015-02-26 18:07 ` Catalin Marinas
2015-02-26 18:06 ` Marc Zyngier
2015-02-26 18:17 ` Ard Biesheuvel
2015-02-26 18:51 ` Marc Zyngier
2015-02-26 19:04 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54EF64E8.4090605@arm.com \
--to=marc.zyngier@arm$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox