On Thu, Apr 4, 2013 at 3:47 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > On 04/04/13 00:15, Christoffer Dall wrote: >> On Wed, Apr 03, 2013 at 11:00:39AM +0100, Marc Zyngier wrote: >>> On 03/04/13 10:50, Will Deacon wrote: >>>> On Tue, Apr 02, 2013 at 02:25:12PM +0100, Marc Zyngier wrote: >>>>> We're about to move to a init procedure where we rely on the >>>>> fact that the init code fits in a single page. Make sure we >>>>> align the idmap text on a page boundary, and that the code is >>>>> not bigger than a single page. >>>>> >>>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> >>>>> --- >>>>> arch/arm/kernel/vmlinux.lds.S | 2 +- >>>>> arch/arm/kvm/init.S | 7 +++++++ >>>>> 2 files changed, 8 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S >>>>> index b571484..d9dd265 100644 >>>>> --- a/arch/arm/kernel/vmlinux.lds.S >>>>> +++ b/arch/arm/kernel/vmlinux.lds.S >>>>> @@ -20,7 +20,7 @@ >>>>> VMLINUX_SYMBOL(__idmap_text_start) = .; \ >>>>> *(.idmap.text) \ >>>>> VMLINUX_SYMBOL(__idmap_text_end) = .; \ >>>>> - ALIGN_FUNCTION(); \ >>>>> + . = ALIGN(PAGE_SIZE); \ >>>>> VMLINUX_SYMBOL(__hyp_idmap_text_start) = .; \ >>>>> *(.hyp.idmap.text) \ >>>>> VMLINUX_SYMBOL(__hyp_idmap_text_end) = .; >>>>> diff --git a/arch/arm/kvm/init.S b/arch/arm/kvm/init.S >>>>> index 9f37a79..35a463f 100644 >>>>> --- a/arch/arm/kvm/init.S >>>>> +++ b/arch/arm/kvm/init.S >>>>> @@ -111,4 +111,11 @@ __do_hyp_init: >>>>> .globl __kvm_hyp_init_end >>>>> __kvm_hyp_init_end: >>>>> >>>>> + /* >>>>> + * The above code *must* fit in a single page for the trampoline >>>>> + * madness to work. Whoever decides to change it must make sure >>>>> + * we map the right amount of memory for the trampoline to work. >>>>> + * The line below ensures any breakage will get noticed. >> >> This comment is a little funny. What's this about changing it? And why >> madness? OK, it's a little mad. >> >> I think the nice thing to explain here is why .org would complain - I >> had to look in the AS reference to figure out that it will complain if >> the location pointer is forwarded outside the section - if that is the >> right reason? >> >>>>> + */ >>>>> + .org __kvm_hyp_init + PAGE_SIZE >>>>> .popsection >>>> >>>> What effect does this have on the size of the kernel image? I'd expect the >>>> idmap code to be pretty small, so aligning to a page might be overkill a lot >>>> of the time. >>> >>> So we're indeed wasting memory between the kernel and the HYP idmaps. >>> The reason for this is to ensure that the init code is not spread across >>> two pages, which would make the trampoline harder to allocate. >>> >>> If there's a way to ensure that the init.S code will fit a single page, >>> then we can remove this ALIGN(PAGE_SIZE). >> >> Just allocate a temporary page and copy the bit of code into there? It >> should be easy enough to make sure it's location independent. We can >> even give that page back later if we want and re-do the trick when a CPU >> gets hot-plugged if we want. No? > > I'd really like to avoid freeing stuff, because it gives us even more > chances to screw our TLBs (we'd have to invalidate on unmap, and deal > with potential races between CPUs coming up). > > We could allocate this page *if* the HYP init code happens to cross a > page boundary, and only then. I'll have a look at implementing this as > it should solve Will's issue (which is even worse on arm64 with 64k pages). > The freeing stuff was a minor point, that I don't care much about, but reducing the kernel size is a valid concern I guess. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
