On 22/06/12 22:34, Christoffer Dall wrote:
> On Mon, May 14, 2012 at 9:06 AM, Marc Zyngier <marc.zyngier at arm.com> wrote:
>> Instead of hardcoding the VGIC's base addresses and interrupt,
>> obtain the values from the device tree. Nicer, safer.
>>
>> What remains to be fixed is the mapping into the guest. This
>> should be provided by the platform emulation code.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
>> ---
>> arch/arm/kvm/vgic.c | 62 +++++++++++++++++++++++++++++++++++++--------------
>> 1 files changed, 45 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
>> index b5e6082..f26b091 100644
>> --- a/arch/arm/kvm/vgic.c
>> +++ b/arch/arm/kvm/vgic.c
>> @@ -20,24 +20,23 @@
>> #include <linux/kvm_host.h>
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>>
>> #include <asm/hardware/gic.h>
>> #include <asm/kvm_arm.h>
>> #include <asm/kvm_mmu.h>
>>
>> -/* Temporary hacks, need to probe DT instead */
>> +/* Temporary hacks, need to be provided by userspace emulation */
>> #define VGIC_DIST_BASE 0x2c001000
>> #define VGIC_DIST_SIZE 0x1000
>> #define VGIC_CPU_BASE 0x2c002000
>> #define VGIC_CPU_SIZE 0x2000
>> -#define VGIC_VCTRL_BASE 0x2c004000
>> -#define VGIC_VCTRL_SIZE 0x2000
>> -#define VGIC_VCPU_BASE 0x2c006000
>> -#define VGIC_VCPU_SIZE 0x2000
>> -#define VGIC_MAINT_IRQ 25
>>
>> static struct kvm_vcpu __percpu **vgic_vcpus;
>> static void __iomem *vgic_vctrl_base;
>> +static struct device_node *vgic_node;
>>
>> #define ACCESS_READ_VALUE (1 << 0)
>> #define ACCESS_READ_RAZ (0 << 0)
>> @@ -778,6 +777,12 @@ void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
>> int kvm_vgic_hyp_init(void)
>> {
>> int ret;
>> + unsigned int irq;
>> + struct resource vctrl_res;
>> +
>> + vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic");
>> + if (!vgic_node)
>> + return -ENODEV;
>
> this means that if you configure VGIC support but run the kernel on a
> processor without vgic support, the kernel and KVM will just fail,
> right?
>
> a print and return 0 instead...
>
> or catch -ENODEV in the caller...
Good point. I'll rework this.
>>
>> vgic_vcpus = alloc_percpu(struct kvm_vcpu *);
>> if (!vgic_vcpus) {
>> @@ -785,34 +790,47 @@ int kvm_vgic_hyp_init(void)
>> return -ENOMEM;
>> }
>>
>> - ret = request_percpu_irq(VGIC_MAINT_IRQ, kvm_vgic_maintainance_handler,
>> + irq = irq_of_parse_and_map(vgic_node, 0);
>> + if (!irq) {
>> + ret = -ENXIO;
>> + goto out_free_vcpus;
>> + }
>> +
>> + ret = request_percpu_irq(irq, kvm_vgic_maintainance_handler,
>> "vgic", vgic_vcpus);
>> if (ret) {
>> - kvm_err("Cannot register interrupt %d\n", VGIC_MAINT_IRQ);
>> + kvm_err("Cannot register interrupt %d\n", irq);
>> goto out_free_vcpus;
>> }
>>
>> - vgic_vctrl_base = ioremap(VGIC_VCTRL_BASE, VGIC_VCTRL_SIZE);
>> + ret = of_address_to_resource(vgic_node, 2, &vctrl_res);
>> + if (ret) {
>> + kvm_err("Cannot obtain VCTRL resource\n");
>> + goto out_free_irq;
>> + }
>> +
>> + vgic_vctrl_base = of_iomap(vgic_node, 2);
>> if (!vgic_vctrl_base) {
>> kvm_err("Cannot ioremap VCTRL\n");
>> + ret = -ENOMEM;
>
> ah, you fixed the previous one here :)
>
>> goto out_free_irq;
>> }
>>
>> - ret = create_hyp_io_mappings(kvm_hyp_pgd,
>> - vgic_vctrl_base,
>> - vgic_vctrl_base + VGIC_VCTRL_SIZE,
>> - VGIC_VCTRL_BASE);
>> + ret = create_hyp_io_mappings(vgic_vctrl_base,
>> + vgic_vctrl_base + resource_size(&vctrl_res),
>> + vctrl_res.start);
>
> I guess this change should have been applied further down the patch
> series, but no problem, the VGIC support is going to be flattened
> anyhow.
This patch is intended to be merged with the previous ones. But I'd like
to keep the VGIC series mostly as it is (unflattened). Nobody wants to
review a single 2000+ line patch.
M.
--
Jazz is not dead. It just smells funny...
