On Mon, May 14, 2012 at 9:05 AM, Marc Zyngier <marc.zyngier at arm.com> wrote:
> Add the init code for the hypervisor, the virtual machine, and
> the virtual CPUs.
>
> An interrupt handler is also wired to allow the VGIC maintainance
> interrupts, used in case of underflow.
>
> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
> ---
> ?arch/arm/include/asm/kvm_vgic.h | ? ?3 +
> ?arch/arm/kvm/arm.c ? ? ? ? ? ? ?| ? ?4 +
> ?arch/arm/kvm/vgic.c ? ? ? ? ? ? | ?120 +++++++++++++++++++++++++++++++++++++++
> ?3 files changed, 127 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
> index e293f60..354e528 100644
> --- a/arch/arm/include/asm/kvm_vgic.h
> +++ b/arch/arm/include/asm/kvm_vgic.h
> @@ -138,6 +138,9 @@ struct kvm_vcpu;
> ?struct kvm_run;
>
> ?#ifdef CONFIG_KVM_ARM_VGIC
> +int kvm_vgic_hyp_init(void);
> +int kvm_vgic_init(struct kvm *kvm);
> +void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu);
> ?void kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu);
> ?void kvm_vgic_sync_from_cpu(struct kvm_vcpu *vcpu);
> ?int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, const struct kvm_irq_level *irq);
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index c03e450..7cd8190 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -618,6 +618,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
> ? ? ? ?void __user *argp = (void __user *)arg;
>
> ? ? ? ?switch (ioctl) {
> +#ifdef CONFIG_KVM_ARM_VGIC
> + ? ? ? case KVM_CREATE_IRQCHIP:
> + ? ? ? ? ? ? ? return kvm_vgic_init(kvm);
> +#endif
> ? ? ? ?case KVM_IRQ_LINE: {
> ? ? ? ? ? ? ? ?struct kvm_irq_level irq_event;
>
> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
> index a48b436..b5e6082 100644
> --- a/arch/arm/kvm/vgic.c
> +++ b/arch/arm/kvm/vgic.c
> @@ -21,9 +21,23 @@
> ?#include <linux/interrupt.h>
> ?#include <linux/io.h>
>
> +#include <asm/hardware/gic.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_mmu.h>
> +
> ?/* Temporary hacks, need to probe DT instead */
> ?#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;
perhaps a comment as to what this is...
> +static void __iomem *vgic_vctrl_base;
I assume this is the common base address with cpuid redirection -
perhaps a comment...
>
> ?#define ACCESS_READ_VALUE ? ? ?(1 << 0)
> ?#define ACCESS_READ_RAZ ? ? ? ? ? ? ? ?(0 << 0)
> @@ -718,3 +732,109 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, const struct kvm_irq_level *
>
> ? ? ? ?return 0;
> ?}
> +
> +static irqreturn_t kvm_vgic_maintainance_handler(int irq, void *data)
> +{
> + ? ? ? struct kvm_vcpu *vcpu = *(struct kvm_vcpu **)data;
> +
> + ? ? ? WARN(!vcpu,
> + ? ? ? ? ? ?"VGIC interrupt on CPU %d with no vcpu\n", smp_processor_id());
> + ? ? ? /*
> + ? ? ? ?* Not much to do, as we handle everything on world switch.
> + ? ? ? ?* It should be possible to do things lazily though, and move
> + ? ? ? ?* away from world-switch.
> + ? ? ? ?*/
> + ? ? ? return IRQ_HANDLED;
> +}
> +
> +void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
> +{
> + ? ? ? struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> + ? ? ? struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
> +
interesting newline
> + ? ? ? int i;
> +
> + ? ? ? if (!irqchip_in_kernel(vcpu->kvm))
> + ? ? ? ? ? ? ? return;
> +
> + ? ? ? spin_lock_init(&vgic_cpu->lock);
> + ? ? ? for (i = 0; i < VGIC_NR_IRQS; i++) {
> + ? ? ? ? ? ? ? if (i < 16)
> + ? ? ? ? ? ? ? ? ? ? ? vgic_bitmap_set_irq_val(&dist->irq_enabled,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? vcpu->vcpu_id, i, 1);
why only these 16 (reset: implementation defined?)
> + ? ? ? ? ? ? ? vgic_cpu->vgic_irq_lr_map[i] = 0xff;
> + ? ? ? }
> +
> + ? ? ? BUG_ON(!vcpu->kvm->arch.vgic.vctrl_base);
> + ? ? ? vgic_cpu->nr_lr = (readl_relaxed(vcpu->kvm->arch.vgic.vctrl_base + GICH_VTR) & 0x1f) + 1;
please fix the 80 char lines here:
vtr = readl_relaxed(...)
vgic_cpu->nr_lr = (vtr & 0x1f) + 1;
> + ? ? ? for (i = 0; i < vgic_cpu->nr_lr; i++)
> + ? ? ? ? ? ? ? vgic_cpu->vgic_lr_irq_map[i] = VGIC_NR_IRQS;
> +
> + ? ? ? vgic_cpu->vgic_mcr = readl_relaxed(vcpu->kvm->arch.vgic.vctrl_base + GICH_VMCR);
same as above
> + ? ? ? vgic_cpu->vgic_mcr |= 0xf << 28; /* Priority */
> + ? ? ? vgic_cpu->vgic_hcr |= VGIC_HCR_EN; /* Get the show on the road... */
> +}
> +
> +int kvm_vgic_hyp_init(void)
> +{
> + ? ? ? int ret;
> +
> + ? ? ? vgic_vcpus = alloc_percpu(struct kvm_vcpu *);
> + ? ? ? if (!vgic_vcpus) {
> + ? ? ? ? ? ? ? kvm_err("Cannot allocate vgic_vcpus\n");
> + ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
> +
> + ? ? ? ret = request_percpu_irq(VGIC_MAINT_IRQ, kvm_vgic_maintainance_handler,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"vgic", vgic_vcpus);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? kvm_err("Cannot register interrupt %d\n", VGIC_MAINT_IRQ);
> + ? ? ? ? ? ? ? goto out_free_vcpus;
> + ? ? ? }
> +
> + ? ? ? vgic_vctrl_base = ioremap(VGIC_VCTRL_BASE, VGIC_VCTRL_SIZE);
> + ? ? ? if (!vgic_vctrl_base) {
> + ? ? ? ? ? ? ? kvm_err("Cannot ioremap VCTRL\n");
> + ? ? ? ? ? ? ? goto out_free_irq;
need to set ret = -EFAULT / -ENOMEM here
> + ? ? ? }
> +
> + ? ? ? ret = create_hyp_io_mappings(kvm_hyp_pgd,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?vgic_vctrl_base,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?vgic_vctrl_base + VGIC_VCTRL_SIZE,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?VGIC_VCTRL_BASE);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? kvm_err("Cannot map VCTRL into hyp\n");
> + ? ? ? ? ? ? ? goto out_unmap;
> + ? ? ? }
> +
> + ? ? ? return 0;
> +
> +out_unmap:
> + ? ? ? iounmap(vgic_vctrl_base);
> +out_free_irq:
> + ? ? ? free_percpu_irq(VGIC_MAINT_IRQ, vgic_vcpus);
> +out_free_vcpus:
> + ? ? ? free_percpu(vgic_vcpus);
> +
> + ? ? ? return ret;
> +}
> +
> +int kvm_vgic_init(struct kvm *kvm)
> +{
> + ? ? ? int ret = -EEXIST;
> +
> + ? ? ? mutex_lock(&kvm->lock);
> + ? ? ? if (atomic_read(&kvm->online_vcpus))
> + ? ? ? ? ? ? ? goto out;
is -EEXIST the proper error code here? Wouldn't say -EBUSY be more
correct, and then do:
if (kvm->arch.vgic.vctrl_base) {
ret = -EBUSY;
goto out;
}
> +
> + ? ? ? spin_lock_init(&kvm->arch.vgic.lock);
> + ? ? ? kvm->arch.vgic.vctrl_base = vgic_vctrl_base;
> +
> + ? ? ? ret = kvm_phys_addr_ioremap(kvm, VGIC_CPU_BASE,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? VGIC_VCPU_BASE, VGIC_CPU_SIZE);
> + ? ? ? if (ret)
> + ? ? ? ? ? ? ? kvm_err("Unable to remap VGIC CPU to VCPU\n");
> +out:
> + ? ? ? mutex_unlock(&kvm->lock);
> + ? ? ? return ret;
> +}
-Christoffer
