[PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[Pavel Fedin]

Some hardware (like Raspberry Pi 2) is capable of running KVM, however lacks
functional vGIC registers. This series introduces software vGIC emulation for
such machines, allowing to fully use virtualization capabilities

Pavel Fedin (3):
  KVM: arm: Add basic infrastructure for software vGIC emulation
  KVM: arm: Introduce software emulation of vGICv2 CPU interface
  KVM: arm: Enable vGICv2 software emulation

 include/kvm/arm_vgic.h      |   4 ++
 virt/kvm/arm/vgic-v2-emul.c |  60 +++++++++++++++--
 virt/kvm/arm/vgic-v2.c      |  29 ++++----
 virt/kvm/arm/vgic.c         | 159 +++++++++++++++++++++++++++++++++-----------
 virt/kvm/arm/vgic.h         |   3 +
 5 files changed, 196 insertions(+), 59 deletions(-)

-- 
2.1.4

[PATCH 1/3] KVM: arm: Add basic infrastructure for software vGIC emulation

[Pavel Fedin]

Two main functions (vgic_get_pending_irq() and vgic_clear_pending_irq())
are responsible for ACK and EOI respectively. Maintenance code path
refactored in order to accommodate these changes. The main problem is that
__kvm_vgic_sync_hwstate() is called before I/O emulation takes place,
therefore vgic_process_maintenance() cannot be reused.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 include/kvm/arm_vgic.h |   3 +
 virt/kvm/arm/vgic.c    | 149 ++++++++++++++++++++++++++++++++++++++-----------
 virt/kvm/arm/vgic.h    |   3 +
 3 files changed, 121 insertions(+), 34 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 133ea00..31cda96 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -160,6 +160,9 @@ struct vgic_dist {
 	bool			in_kernel;
 	bool			ready;
 
+	/* Use software emulation of CPU interface */
+	bool			sw_cpuif;
+
 	/* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */
 	u32			vgic_model;
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 78fb820..aec6063 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1104,6 +1104,8 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
 static void vgic_queue_irq_to_lr(struct kvm_vcpu *vcpu, int irq,
 				 int lr_nr, struct vgic_lr vlr)
 {
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+
 	if (vgic_irq_is_active(vcpu, irq)) {
 		vlr.state |= LR_STATE_ACTIVE;
 		kvm_debug("Set active, clear distributor: 0x%x\n", vlr.state);
@@ -1119,6 +1121,9 @@ static void vgic_queue_irq_to_lr(struct kvm_vcpu *vcpu, int irq,
 
 	vgic_set_lr(vcpu, lr_nr, vlr);
 	vgic_sync_lr_elrsr(vcpu, lr_nr, vlr);
+
+	if (dist->sw_cpuif && (vlr.state & LR_STATE_PENDING))
+		vcpu_set_hcr(vcpu, vcpu_get_hcr(vcpu) | HCR_VI);
 }
 
 /*
@@ -1171,6 +1176,115 @@ bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
 	return true;
 }
 
+int vgic_get_pending_irq(struct kvm_vcpu *vcpu)
+{
+	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+	struct vgic_lr vlr;
+	int lr;
+
+	for_each_set_bit(lr, vgic_cpu->lr_used, vgic->nr_lr) {
+		vlr = vgic_get_lr(vcpu, lr);
+
+		if (vlr.state & LR_STATE_PENDING) {
+			vlr.state &= ~LR_STATE_PENDING;
+			vlr.state |= LR_STATE_ACTIVE;
+			vgic_set_lr(vcpu, lr, vlr);
+
+			return vlr.irq;
+		}
+	}
+	vcpu_set_hcr(vcpu, vcpu_get_hcr(vcpu) & ~HCR_VI);
+	return 1023;
+}
+
+static int vgic_eoi_level_irq(struct kvm_vcpu *vcpu, int irq)
+{
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+	int level_pending;
+
+	vgic_irq_clear_queued(vcpu, irq);
+
+	/*
+	 * If the IRQ was EOIed it was also ACKed and we we
+	 * therefore assume we can clear the soft pending
+	 * state (should it had been set) for this interrupt.
+	 *
+	 * Note: if the IRQ soft pending state was set after
+	 * the IRQ was acked, it actually shouldn't be
+	 * cleared, but we have no way of knowing that unless
+	 * we start trapping ACKs when the soft-pending state
+	 * is set.
+	 */
+	vgic_dist_irq_clear_soft_pend(vcpu, irq);
+
+	/*
+	 * kvm_notify_acked_irq calls kvm_set_irq()
+	 * to reset the IRQ level. Need to release the
+	 * lock for kvm_set_irq to grab it.
+	 */
+	spin_unlock(&dist->lock);
+
+	kvm_notify_acked_irq(vcpu->kvm, 0, irq - VGIC_NR_PRIVATE_IRQS);
+	spin_lock(&dist->lock);
+
+	/* Any additional pending interrupt? */
+	level_pending = vgic_dist_irq_get_level(vcpu, irq);
+	if (level_pending) {
+		vgic_cpu_irq_set(vcpu, irq);
+	} else {
+		vgic_dist_irq_clear_pending(vcpu, irq);
+		vgic_cpu_irq_clear(vcpu, irq);
+	}
+
+	return level_pending;
+}
+
+void vgic_clear_pending_irq(struct kvm_vcpu *vcpu, int irq)
+{
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+	int level_pending = 0;
+	struct vgic_lr vlr;
+	int lr;
+
+	if (unlikely(irq >= dist->nr_irqs)) {
+		kvm_info("EOI on invalid IRQ %d from guest\n", irq);
+		return;
+	}
+
+	lr = vgic_cpu->vgic_irq_lr_map[irq];
+	if (unlikely(lr == LR_EMPTY)) {
+		kvm_info("EOI on non-pending IRQ %d from guest\n", irq);
+		return;
+	}
+
+	vlr = vgic_get_lr(vcpu, lr);
+
+	if (vlr.state & LR_EOI_INT)
+		level_pending = vgic_eoi_level_irq(vcpu, vlr.irq);
+
+	vlr.state = 0;
+	vgic_set_lr(vcpu, lr, vlr);
+	vgic_sync_lr_elrsr(vcpu, lr, vlr);
+
+	clear_bit(lr, vgic_cpu->lr_used);
+	vgic_cpu->vgic_irq_lr_map[vlr.irq] = LR_EMPTY;
+
+	/*
+	 * The following is the same as end of __kvm_vgic_sync_hwstate,
+	 * but with conditions inverted for performance optimization.
+	 */
+	if (!level_pending) {
+		u64 elrsr = vgic_get_elrsr(vcpu);
+		unsigned long *elrsr_ptr = u64_to_bitmask(&elrsr);
+		int pending = find_first_zero_bit(elrsr_ptr, vgic->nr_lr);
+
+		if (pending >= vgic->nr_lr)
+			return;
+	}
+	set_bit(vcpu->vcpu_id, dist->irq_pending_on_cpu);
+}
+
 static bool vgic_queue_hwirq(struct kvm_vcpu *vcpu, int irq)
 {
 	if (!vgic_can_sample_irq(vcpu, irq))
@@ -1261,7 +1375,6 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
 	u32 status = vgic_get_interrupt_status(vcpu);
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 	bool level_pending = false;
-	struct kvm *kvm = vcpu->kvm;
 
 	kvm_debug("STATUS = %08x\n", status);
 
@@ -1279,43 +1392,11 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
 			WARN_ON(vgic_irq_is_edge(vcpu, vlr.irq));
 
 			spin_lock(&dist->lock);
-			vgic_irq_clear_queued(vcpu, vlr.irq);
 			WARN_ON(vlr.state & LR_STATE_MASK);
 			vlr.state = 0;
 			vgic_set_lr(vcpu, lr, vlr);
 
-			/*
-			 * If the IRQ was EOIed it was also ACKed and we we
-			 * therefore assume we can clear the soft pending
-			 * state (should it had been set) for this interrupt.
-			 *
-			 * Note: if the IRQ soft pending state was set after
-			 * the IRQ was acked, it actually shouldn't be
-			 * cleared, but we have no way of knowing that unless
-			 * we start trapping ACKs when the soft-pending state
-			 * is set.
-			 */
-			vgic_dist_irq_clear_soft_pend(vcpu, vlr.irq);
-
-			/*
-			 * kvm_notify_acked_irq calls kvm_set_irq()
-			 * to reset the IRQ level. Need to release the
-			 * lock for kvm_set_irq to grab it.
-			 */
-			spin_unlock(&dist->lock);
-
-			kvm_notify_acked_irq(kvm, 0,
-					     vlr.irq - VGIC_NR_PRIVATE_IRQS);
-			spin_lock(&dist->lock);
-
-			/* Any additional pending interrupt? */
-			if (vgic_dist_irq_get_level(vcpu, vlr.irq)) {
-				vgic_cpu_irq_set(vcpu, vlr.irq);
-				level_pending = true;
-			} else {
-				vgic_dist_irq_clear_pending(vcpu, vlr.irq);
-				vgic_cpu_irq_clear(vcpu, vlr.irq);
-			}
+			level_pending |= vgic_eoi_level_irq(vcpu, vlr.irq);
 
 			spin_unlock(&dist->lock);
 
diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
index 0df74cb..75ecfef 100644
--- a/virt/kvm/arm/vgic.h
+++ b/virt/kvm/arm/vgic.h
@@ -59,6 +59,9 @@ void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
 bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq);
 void vgic_unqueue_irqs(struct kvm_vcpu *vcpu);
 
+int vgic_get_pending_irq(struct kvm_vcpu *vcpu);
+void vgic_clear_pending_irq(struct kvm_vcpu *vcpu, int irq);
+
 struct kvm_exit_mmio {
 	phys_addr_t	phys_addr;
 	void		*data;
-- 
2.1.4

[PATCH 2/3] KVM: arm: Introduce software emulation of vGICv2 CPU interface

[Pavel Fedin]

The emulation code is activated by setting params->vcpu_base to zero

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 include/kvm/arm_vgic.h      |  1 +
 virt/kvm/arm/vgic-v2-emul.c | 60 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 31cda96..fa533e7 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -254,6 +254,7 @@ struct vgic_dist {
 
 	struct vgic_vm_ops	vm_ops;
 	struct vgic_io_device	dist_iodev;
+	struct vgic_io_device	cpu_iodev;
 	struct vgic_io_device	*redist_iodevs;
 };
 
diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index 1390797..c2e1274 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -404,6 +404,8 @@ static const struct vgic_io_range vgic_dist_ranges[] = {
 	{}
 };
 
+static const struct vgic_io_range vgic_cpu_ranges[];
+
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 {
 	struct kvm *kvm = vcpu->kvm;
@@ -520,12 +522,20 @@ static int vgic_v2_map_resources(struct kvm *kvm,
 		goto out_unregister;
 	}
 
-	ret = kvm_phys_addr_ioremap(kvm, dist->vgic_cpu_base,
-				    params->vcpu_base, KVM_VGIC_V2_CPU_SIZE,
-				    true);
-	if (ret) {
-		kvm_err("Unable to remap VGIC CPU to VCPU\n");
-		goto out_unregister;
+	if (params->vcpu_base) {
+		ret = kvm_phys_addr_ioremap(kvm, dist->vgic_cpu_base,
+					    params->vcpu_base,
+					    KVM_VGIC_V2_CPU_SIZE, true);
+		if (ret) {
+			kvm_err("Unable to remap VGIC CPU to VCPU\n");
+			goto out_unregister;
+		}
+	} else {
+		vgic_register_kvm_io_dev(kvm, dist->vgic_cpu_base,
+					 KVM_VGIC_V2_CPU_SIZE,
+					 vgic_cpu_ranges, -1,
+					 &dist->cpu_iodev);
+		dist->sw_cpuif = true;
 	}
 
 	dist->ready = true;
@@ -611,6 +621,34 @@ static bool handle_cpu_mmio_misc(struct kvm_vcpu *vcpu,
 	return updated;
 }
 
+static bool handle_mmio_intack(struct kvm_vcpu *vcpu,
+			       struct kvm_exit_mmio *mmio, phys_addr_t offset)
+{
+	int pending;
+
+	if (mmio->is_write)
+		return false;
+
+	pending = vgic_get_pending_irq(vcpu);
+	mmio_data_write(mmio, ~0, pending);
+
+	return true;
+}
+
+static bool handle_mmio_eoi(struct kvm_vcpu *vcpu,
+			    struct kvm_exit_mmio *mmio, phys_addr_t offset)
+{
+	u32 reg;
+
+	if (!mmio->is_write)
+		return false;
+
+	reg = mmio_data_read(mmio, ~0);
+	vgic_clear_pending_irq(vcpu, reg);
+
+	return false;
+}
+
 static bool handle_mmio_abpr(struct kvm_vcpu *vcpu,
 			     struct kvm_exit_mmio *mmio, phys_addr_t offset)
 {
@@ -645,6 +683,16 @@ static const struct vgic_io_range vgic_cpu_ranges[] = {
 		.handle_mmio	= handle_cpu_mmio_misc,
 	},
 	{
+		.base		= GIC_CPU_INTACK,
+		.len		= 4,
+		.handle_mmio	= handle_mmio_intack,
+	},
+	{
+		.base		= GIC_CPU_EOI,
+		.len		= 4,
+		.handle_mmio	= handle_mmio_eoi,
+	},
+	{
 		.base		= GIC_CPU_ALIAS_BINPOINT,
 		.len		= 4,
 		.handle_mmio	= handle_mmio_abpr,
-- 
2.1.4

[PATCH 3/3] KVM: arm: Enable vGICv2 software emulation

[Pavel Fedin]

The emulation code is automatically enabled when one of vGIC resources
is missing from the device tree

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 virt/kvm/arm/vgic-v2.c | 29 ++++++++++++++---------------
 virt/kvm/arm/vgic.c    | 10 ++++++----
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
index f9b9c7c..b371a59 100644
--- a/virt/kvm/arm/vgic-v2.c
+++ b/virt/kvm/arm/vgic-v2.c
@@ -180,22 +180,27 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		  const struct vgic_ops **ops,
 		  const struct vgic_params **params)
 {
-	int ret;
+	int ret = 0;
 	struct resource vctrl_res;
 	struct resource vcpu_res;
 	struct vgic_params *vgic = &vgic_v2_params;
 
+	vgic->nr_lr = VGIC_V2_MAX_LRS;
+
 	vgic->maint_irq = irq_of_parse_and_map(vgic_node, 0);
 	if (!vgic->maint_irq) {
 		kvm_err("error getting vgic maintenance irq from DT\n");
-		ret = -ENXIO;
-		goto out;
+		goto sw_emul;
 	}
 
-	ret = of_address_to_resource(vgic_node, 2, &vctrl_res);
-	if (ret) {
+	if (of_address_to_resource(vgic_node, 2, &vctrl_res)) {
 		kvm_err("Cannot obtain GICH resource\n");
-		goto out;
+		goto sw_emul;
+	}
+
+	if (of_address_to_resource(vgic_node, 3, &vcpu_res)) {
+		kvm_err("Cannot obtain GICV resource\n");
+		goto sw_emul;
 	}
 
 	vgic->vctrl_base = of_iomap(vgic_node, 2);
@@ -216,12 +221,6 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		goto out_unmap;
 	}
 
-	if (of_address_to_resource(vgic_node, 3, &vcpu_res)) {
-		kvm_err("Cannot obtain GICV resource\n");
-		ret = -ENXIO;
-		goto out_unmap;
-	}
-
 	if (!PAGE_ALIGNED(vcpu_res.start)) {
 		kvm_err("GICV physical address 0x%llx not page aligned\n",
 			(unsigned long long)vcpu_res.start);
@@ -237,13 +236,13 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		goto out_unmap;
 	}
 
-	vgic->can_emulate_gicv2 = true;
-	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
-
 	vgic->vcpu_base = vcpu_res.start;
 
 	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
 		 vctrl_res.start, vgic->maint_irq);
+sw_emul:
+	vgic->can_emulate_gicv2 = true;
+	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
 
 	vgic->type = VGIC_V2;
 	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index aec6063..66e0cae 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2196,11 +2196,13 @@ int kvm_vgic_hyp_init(void)
 	if (ret)
 		return ret;
 
-	ret = request_percpu_irq(vgic->maint_irq, vgic_maintenance_handler,
+	if (vgic->maint_irq) {
+		ret = request_percpu_irq(vgic->maint_irq, vgic_maintenance_handler,
 				 "vgic", kvm_get_running_vcpus());
-	if (ret) {
-		kvm_err("Cannot register interrupt %d\n", vgic->maint_irq);
-		return ret;
+		if (ret) {
+			kvm_err("Cannot register interrupt %d\n", vgic->maint_irq);
+			return ret;
+		}
 	}
 
 	ret = __register_cpu_notifier(&vgic_cpu_nb);
-- 
2.1.4

Re: [PATCH 3/3] KVM: arm: Enable vGICv2 software emulation

[Marc Zyngier]

On 29/06/15 10:53, Pavel Fedin wrote:
> The emulation code is automatically enabled when one of vGIC resources
> is missing from the device tree

Hold on a second. In your cover letter, your saying "RPi-2". The RPi-2
doesn't have a GIC at all, so I'd really like to know *how* you end-up
in the GICv2 probe function?

Thanks,

	M.

> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  virt/kvm/arm/vgic-v2.c | 29 ++++++++++++++---------------
>  virt/kvm/arm/vgic.c    | 10 ++++++----
>  2 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
> index f9b9c7c..b371a59 100644
> --- a/virt/kvm/arm/vgic-v2.c
> +++ b/virt/kvm/arm/vgic-v2.c
> @@ -180,22 +180,27 @@ int vgic_v2_probe(struct device_node *vgic_node,
>  		  const struct vgic_ops **ops,
>  		  const struct vgic_params **params)
>  {
> -	int ret;
> +	int ret = 0;
>  	struct resource vctrl_res;
>  	struct resource vcpu_res;
>  	struct vgic_params *vgic = &vgic_v2_params;
>  
> +	vgic->nr_lr = VGIC_V2_MAX_LRS;
> +
>  	vgic->maint_irq = irq_of_parse_and_map(vgic_node, 0);
>  	if (!vgic->maint_irq) {
>  		kvm_err("error getting vgic maintenance irq from DT\n");
> -		ret = -ENXIO;
> -		goto out;
> +		goto sw_emul;
>  	}
>  
> -	ret = of_address_to_resource(vgic_node, 2, &vctrl_res);
> -	if (ret) {
> +	if (of_address_to_resource(vgic_node, 2, &vctrl_res)) {
>  		kvm_err("Cannot obtain GICH resource\n");
> -		goto out;
> +		goto sw_emul;
> +	}
> +
> +	if (of_address_to_resource(vgic_node, 3, &vcpu_res)) {
> +		kvm_err("Cannot obtain GICV resource\n");
> +		goto sw_emul;
>  	}
>  
>  	vgic->vctrl_base = of_iomap(vgic_node, 2);
> @@ -216,12 +221,6 @@ int vgic_v2_probe(struct device_node *vgic_node,
>  		goto out_unmap;
>  	}
>  
> -	if (of_address_to_resource(vgic_node, 3, &vcpu_res)) {
> -		kvm_err("Cannot obtain GICV resource\n");
> -		ret = -ENXIO;
> -		goto out_unmap;
> -	}
> -
>  	if (!PAGE_ALIGNED(vcpu_res.start)) {
>  		kvm_err("GICV physical address 0x%llx not page aligned\n",
>  			(unsigned long long)vcpu_res.start);
> @@ -237,13 +236,13 @@ int vgic_v2_probe(struct device_node *vgic_node,
>  		goto out_unmap;
>  	}
>  
> -	vgic->can_emulate_gicv2 = true;
> -	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
> -
>  	vgic->vcpu_base = vcpu_res.start;
>  
>  	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
>  		 vctrl_res.start, vgic->maint_irq);
> +sw_emul:
> +	vgic->can_emulate_gicv2 = true;
> +	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
>  
>  	vgic->type = VGIC_V2;
>  	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index aec6063..66e0cae 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2196,11 +2196,13 @@ int kvm_vgic_hyp_init(void)
>  	if (ret)
>  		return ret;
>  
> -	ret = request_percpu_irq(vgic->maint_irq, vgic_maintenance_handler,
> +	if (vgic->maint_irq) {
> +		ret = request_percpu_irq(vgic->maint_irq, vgic_maintenance_handler,
>  				 "vgic", kvm_get_running_vcpus());
> -	if (ret) {
> -		kvm_err("Cannot register interrupt %d\n", vgic->maint_irq);
> -		return ret;
> +		if (ret) {
> +			kvm_err("Cannot register interrupt %d\n", vgic->maint_irq);
> +			return ret;
> +		}
>  	}
>  
>  	ret = __register_cpu_notifier(&vgic_cpu_nb);
> 


-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 3/3] KVM: arm: Enable vGICv2 software emulation

[Pavel Fedin]

 Hello!

> Hold on a second. In your cover letter, your saying "RPi-2". The RPi-2
> doesn't have a GIC at all, so I'd really like to know *how* you end-up
> in the GICv2 probe function?

 I'm not on RPi-2. I am on some Samsung's proprietary hardware which suffers from this problem.
Actually it has "weird GIC", similar to Exynos-4 (every CPU accesses GIC at its own base), thus
hardware vGIC is unusable.
 I learned about RPi-2 when i was discussing some related things earlier with Andre over private
mail and we suggested that it could be useful there.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [PATCH 3/3] KVM: arm: Enable vGICv2 software emulation

[Marc Zyngier]

On 29/06/15 13:37, Pavel Fedin wrote:
>  Hello!
> 
>> Hold on a second. In your cover letter, your saying "RPi-2". The RPi-2
>> doesn't have a GIC at all, so I'd really like to know *how* you end-up
>> in the GICv2 probe function?
> 
> I'm not on RPi-2. I am on some Samsung's proprietary hardware which
> suffers from this problem. Actually it has "weird GIC", similar to
> Exynos-4 (every CPU accesses GIC at its own base), thus hardware vGIC
> is unusable.

Well, I'm sorry you have to work with such broken designs, but this is
simply not suitable for KVM. The Franken-GIC has caused us a lot of pain
in the past, and papering over it only gives people an incentive to push
for more broken HW.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[Marc Zyngier]

Hi Pavel,

On 29/06/15 10:53, Pavel Fedin wrote:
> Some hardware (like Raspberry Pi 2) is capable of running KVM, however lacks
> functional vGIC registers. This series introduces software vGIC emulation for
> such machines, allowing to fully use virtualization capabilities

I'm really not keen on any of this. The RPi-2 is mostly unsupported in
mainline, and not only lacks the vGIC, but doesn't have a GIC at all.
This would break ongoing developments (inability to properly support
EOImode=1, to preserve the active state for shared devices like timers).

As far as I'm concerned, this hardware is not architecturally compliant
and the solution is to use userspace emulation (QEMU provides a GIC
model). Taking these patches would keep us stuck in the past.

I'm sure whoever maintains the RPi kernel will be glad to take these
patches on top of the pile of stuff they keep out of tree.

Thanks,

	M.

> Pavel Fedin (3):
>   KVM: arm: Add basic infrastructure for software vGIC emulation
>   KVM: arm: Introduce software emulation of vGICv2 CPU interface
>   KVM: arm: Enable vGICv2 software emulation
> 
>  include/kvm/arm_vgic.h      |   4 ++
>  virt/kvm/arm/vgic-v2-emul.c |  60 +++++++++++++++--
>  virt/kvm/arm/vgic-v2.c      |  29 ++++----
>  virt/kvm/arm/vgic.c         | 159 +++++++++++++++++++++++++++++++++-----------
>  virt/kvm/arm/vgic.h         |   3 +
>  5 files changed, 196 insertions(+), 59 deletions(-)
> 


-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[Christoffer Dall]

Hi Pavel,

[Please cc the kvm/arm list for such patches according to the
MAINTAINERS file in the future]

On Mon, Jun 29, 2015 at 12:53:46PM +0300, Pavel Fedin wrote:
> Some hardware (like Raspberry Pi 2) is capable of running KVM, however lacks
> functional vGIC registers. This series introduces software vGIC emulation for
> such machines, allowing to fully use virtualization capabilities

Is this rather esoteric use case really worth the extra code in the
kernel?  I really feel that pure emulation should happen in userspace
unless there's a very good reason for doing it in the kernel, such as a
clearly measureable difference in performance, etc.

I would much rather see a version of this where a userspace provided GIC
works with the in-kernel arch timers support.

-Christoffer

Re: [PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[Andre Przywara]

Hi,

On 29/06/15 13:52, Christoffer Dall wrote:
> Hi Pavel,
> 
> [Please cc the kvm/arm list for such patches according to the
> MAINTAINERS file in the future]
> 
> On Mon, Jun 29, 2015 at 12:53:46PM +0300, Pavel Fedin wrote:
>> Some hardware (like Raspberry Pi 2) is capable of running KVM, however lacks
>> functional vGIC registers. This series introduces software vGIC emulation for
>> such machines, allowing to fully use virtualization capabilities
> 
> Is this rather esoteric use case really worth the extra code in the
> kernel? 

I wonder if these patches would pave the way to support running GICv2
guests on GICv3s without compat support? Admittedly not a really
compelling use case either, but at least worth discussing, I think.

Also if this will make the hack needed to enable KVM on RPi2 smaller,
I'd rather embrace this one than letting any random hacks appear on that
RPi kernel tree (patches which I have seen already on some other repo).
If I get this correctly, there are some efforts currently to get closer
to mainline with the RPi tree.

Pavel, is this "broken" GIC you are talking about going to appear in a
publicly available SoC? If yes, you could either state this right now or
send it later once you can talk publicly.

Marc, Christoffer:
So is this GICv2 CPU interface emulation totally out of question for us
or is it worth at least commenting on the patches?

Cheers,
Andre.

> I really feel that pure emulation should happen in userspace
> unless there's a very good reason for doing it in the kernel, such as a
> clearly measureable difference in performance, etc.
> 
> I would much rather see a version of this where a userspace provided GIC
> works with the in-kernel arch timers support.
> 
> -Christoffer
> 

Re: [PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[Christoffer Dall]

On Mon, Jun 29, 2015 at 4:11 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> On 29/06/15 13:52, Christoffer Dall wrote:
>> Hi Pavel,
>>
>> [Please cc the kvm/arm list for such patches according to the
>> MAINTAINERS file in the future]
>>
>> On Mon, Jun 29, 2015 at 12:53:46PM +0300, Pavel Fedin wrote:
>>> Some hardware (like Raspberry Pi 2) is capable of running KVM, however lacks
>>> functional vGIC registers. This series introduces software vGIC emulation for
>>> such machines, allowing to fully use virtualization capabilities
>>
>> Is this rather esoteric use case really worth the extra code in the
>> kernel?
>
> I wonder if these patches would pave the way to support running GICv2
> guests on GICv3s without compat support? Admittedly not a really
> compelling use case either, but at least worth discussing, I think.
>
> Also if this will make the hack needed to enable KVM on RPi2 smaller,
> I'd rather embrace this one than letting any random hacks appear on that
> RPi kernel tree (patches which I have seen already on some other repo).
> If I get this correctly, there are some efforts currently to get closer
> to mainline with the RPi tree.
>
> Pavel, is this "broken" GIC you are talking about going to appear in a
> publicly available SoC? If yes, you could either state this right now or
> send it later once you can talk publicly.
>
> Marc, Christoffer:
> So is this GICv2 CPU interface emulation totally out of question for us
> or is it worth at least commenting on the patches?
>
Well, I certainly have a lot of seemingly higher-priority things to
look at currently.

I'd like some numbers for the performance benefit of putting this in
the kernel before I entertain the idea.

-Christoffer

Re: [PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[Marc Zyngier]

On 29/06/15 15:11, Andre Przywara wrote:
> Hi,
> 
> On 29/06/15 13:52, Christoffer Dall wrote:
>> Hi Pavel,
>>
>> [Please cc the kvm/arm list for such patches according to the
>> MAINTAINERS file in the future]
>>
>> On Mon, Jun 29, 2015 at 12:53:46PM +0300, Pavel Fedin wrote:
>>> Some hardware (like Raspberry Pi 2) is capable of running KVM, however lacks
>>> functional vGIC registers. This series introduces software vGIC emulation for
>>> such machines, allowing to fully use virtualization capabilities
>>
>> Is this rather esoteric use case really worth the extra code in the
>> kernel? 
> 
> I wonder if these patches would pave the way to support running GICv2
> guests on GICv3s without compat support? Admittedly not a really
> compelling use case either, but at least worth discussing, I think.

Let's face it: arm64 has no legacy to support. So if you're on a pure
GICv3 system, you run a GICv3 guest (oddly enough, pure GICv3 systems
are also pure AArch64 systems - see a pattern?). We've made sure the
software was available in a timely manner.

> Also if this will make the hack needed to enable KVM on RPi2 smaller,
> I'd rather embrace this one than letting any random hacks appear on that
> RPi kernel tree (patches which I have seen already on some other repo).
> If I get this correctly, there are some efforts currently to get closer
> to mainline with the RPi tree.

Whatever the RPi people do in their tree is their problem. I don't care.
I'm interested in supporting *compliant hardware*, and not doing a quick
hack on the side.

Even if RPi-2 was fully supported in mainline, this code would actively
prevent us from supporting proper timer deactivation, for example.

> Pavel, is this "broken" GIC you are talking about going to appear in a
> publicly available SoC? If yes, you could either state this right now or
> send it later once you can talk publicly.
> 
> Marc, Christoffer:
> So is this GICv2 CPU interface emulation totally out of question for us
> or is it worth at least commenting on the patches?

As long as this code is there to support a platform that doesn't exist
in a mainline tree, I'm not interested. We have much bigger fish to fry,
and supporting what is effectively a broken platform is not exactly high
on the agenda.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 0/3] KVM: arm: Implement software vGICv2 emulation

[Pavel Fedin]

 Hello!

> I wonder if these patches would pave the way to support running GICv2
> guests on GICv3s without compat support?

 Yes, it does. I haven't implemented GICv3 bit for this time, but - yes. And it will be a very small
addition.

> Pavel, is this "broken" GIC you are talking about going to appear in a
> publicly available SoC?

 In kernel documentation it's known as "FrankenGIC".
 First of all, it's Exynos 4 (however i don't know whether it implements virtualization extensions).
 Second, it is present in many consumer electronics products (does this count as "publicly
available"? But you can learn this fact from Samsung's opensource releases). I use one of these
boards for my tests because we have lots of them here :)
 Third, this code doesn't really depend on hardware GIC, and can be reused on RPi-2 with a little
more hacking.
 Fourth, there is at least of one ARM64 machine with broken vGICv3. And you can buy it. I have it
here. And it would also benefit from this code (yes, with GICv3 CPU interface emulation added, which
becomes trivial).

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia