From mboxrd@z Thu Jan  1 00:00:00 1970
From: Cyril Chemparathy <cyril@ti.com>
Subject: Re: [PATCH v4 05/14] KVM: ARM: Hypervisor inititalization
Date: Mon, 19 Nov 2012 10:27:05 -0500
Message-ID: <50AA4FC9.8080208@ti.com>
References: <20121110154203.2836.46686.stgit@chazy-air> <20121110154245.2836.17259.stgit@chazy-air> <20121119145100.GX3205@mudshark.cambridge.arm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="ISO-8859-1"; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Christoffer Dall <c.dall@virtualopensystems.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	Marcelo Tosatti <mtosatti@redhat.com>
To: Will Deacon <will.deacon@arm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from arroyo.ext.ti.com ([192.94.94.40]:50534 "EHLO arroyo.ext.ti.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752735Ab2KSP1X (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 19 Nov 2012 10:27:23 -0500
In-Reply-To: <20121119145100.GX3205@mudshark.cambridge.arm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 11/19/2012 09:51 AM, Will Deacon wrote:
>
> Typo in subject, use one of initiali[sz]ation instead.
>
> On Sat, Nov 10, 2012 at 03:42:45PM +0000, Christoffer Dall wrote:
>> Sets up KVM code to handle all exceptions taken to Hyp mode.
>>
>> When the kernel is booted in Hyp mode, calling "hvc #0xff" with r0 pointing to
>> the new vectors, the HVBAR is changed to the the vector pointers.  This allows
>> subsystems (like KVM here) to execute code in Hyp-mode with the MMU disabled.
>>
>> We initialize other Hyp-mode registers and enables the MMU for Hyp-mode from
>> the id-mapped hyp initialization code. Afterwards, the HVBAR is changed to
>> point to KVM Hyp vectors used to catch guest faults and to switch to Hyp mode
>> to perform a world-switch into a KVM guest.
>>
>> If the KVM module is unloaded we call "hvc #0xff" once more to disable the MMU
>> in Hyp mode again and install a vector handler to change the HVBAR for a
>> subsequent reload of KVM or another hypervisor.
>
> 0xff might be a bit too simple. I notice Xen use 0xEA1, which is
> probably less likely to conflict with anything else. We should probably
> also put these numbers in the same header file so that any conflicts
> become immediately apparent.
>
>>
>> Also provides memory mapping code to map required code pages, data structures,
>> and I/O regions  accessed in Hyp mode at the same virtual address as the host
>> kernel virtual addresses, but which conforms to the architectural requirements
>> for translations in Hyp mode. This interface is added in arch/arm/kvm/arm_mmu.c
>> and comprises:
>>   - create_hyp_mappings(from, to);
>>   - create_hyp_io_mappings(from, to, phys_addr);
>>   - free_hyp_pmds();
>>
>> Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>
[...]
>> +static int init_hyp_mode(void)
>> +{
>> +       phys_addr_t init_phys_addr;
>> +       int cpu;
>> +       int err = 0;
>> +
>> +       /*
>> +        * Allocate Hyp PGD and setup Hyp identity mapping
>> +        */
>> +       err = kvm_mmu_init();
>> +       if (err)
>> +               return err;
>> +
>> +       /*
>> +        * It is probably enough to obtain the default on one
>> +        * CPU. It's unlikely to be different on the others.
>> +        */
>> +       hyp_default_vectors = __hyp_get_vectors();
>> +
>> +       /*
>> +        * Allocate stack pages for Hypervisor-mode
>> +        */
>> +       for_each_possible_cpu(cpu) {
>> +               unsigned long stack_page;
>> +
>> +               stack_page = __get_free_page(GFP_KERNEL);
>> +               if (!stack_page) {
>> +                       err = -ENOMEM;
>> +                       goto out_free_stack_pages;
>> +               }
>> +
>> +               per_cpu(kvm_arm_hyp_stack_page, cpu) = stack_page;
>> +       }
>> +
>> +       /*
>> +        * Execute the init code on each CPU.
>> +        *
>> +        * Note: The stack is not mapped yet, so don't do anything else than
>> +        * initializing the hypervisor mode on each CPU using a local stack
>> +        * space for temporary storage.
>> +        */
>> +       init_phys_addr = virt_to_phys(__kvm_hyp_init);
>> +       for_each_online_cpu(cpu) {
>> +               smp_call_function_single(cpu, cpu_init_hyp_mode,
>> +                                        (void *)(long)init_phys_addr, 1);
>> +       }
>
> Hmm, this will probably go wrong for platforms like keystone, where
> everything is above 4GB in physical memory. Actually, I'm not sure on
> the status of the patches so you could check with Cyril [CC'd].
>

Thanks for the heads up.

Looks like __kvm_hyp_init is idmap'ed.  This should be ok on keystone, 
but we'll need to replace the virt_to_phys() with a virt_to_idmap() when 
we get to this.

Thanks
-- Cyril.