[PATCH 15/15] arm/arm64: KVM: Check that IDMAP doesn't intersect with VA range

[Christoffer Dall]

On Thu, Jun 30, 2016 at 01:51:00PM +0100, Marc Zyngier wrote:
> On 28/06/16 23:01, Christoffer Dall wrote:
> > On Tue, Jun 07, 2016 at 11:58:35AM +0100, Marc Zyngier wrote:
> >> This is more of a safety measure than anything else: If we end-up
> >> with an idmap page that intersect with the range picked for the
> >> the HYP VA space, abort the KVM setup, as it is unsafe to go
> >> further.
> >>
> >> I cannot imagine it happening on 64bit (we have a mechanism to
> >> work around it), but could potentially occur on a 32bit system with
> >> the kernel loaded high enough in memory so that in conflicts with
> >> the kernel VA.
> > 
> > ah, you had a patch for this...
> > 
> > does this even work for enabling the MMU during kernel boot or how do
> > they deal with it?
> 
> As I said in a reply to an earlier patch, this must already taken care
> of by the bootloader, making sure that the kernel physical memory does
> not alias with the VAs. Pretty scary.
> 
> > 
> >>
> >> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
> >> ---
> >>  arch/arm/kvm/mmu.c | 15 +++++++++++++++
> >>  1 file changed, 15 insertions(+)
> >>
> >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> >> index 46b8604..819517d 100644
> >> --- a/arch/arm/kvm/mmu.c
> >> +++ b/arch/arm/kvm/mmu.c
> >> @@ -1708,6 +1708,21 @@ int kvm_mmu_init(void)
> >>  	 */
> >>  	BUG_ON((hyp_idmap_start ^ (hyp_idmap_end - 1)) & PAGE_MASK);
> >>  
> >> +	kvm_info("IDMAP page: %lx\n", hyp_idmap_start);
> >> +	kvm_info("HYP VA range: %lx:%lx\n",
> >> +		 KERN_TO_HYP(PAGE_OFFSET), KERN_TO_HYP(~0UL));
> >> +
> >> +	if (hyp_idmap_start >= KERN_TO_HYP(PAGE_OFFSET) &&
> >> +	    hyp_idmap_start <  KERN_TO_HYP(~0UL)) {
> > 
> > why is the second part of this clause necessary?
> 
> We want to check that our clash avoiding mechanism works.
> 
> Since we're translating the kernel VA downwards (by clearing the top
> bits), we can definitely end-up in a situation where the idmap is above
> the translated "top of the kernel" (that's the "low mask" option). So it
> is definitely worth checking that we really don't get any aliasing. This
> has been quite useful when debugging this code.
> 
Right, I thought about this only in the context of 32-bit and got
confused.

Thanks,
-Christoffer