Skip to contentSkip to conversation

kvm: arm/arm64 : fix vm's hanging at startup time

PATCH In review Peng Hao 6 participants 23 messages Nov 15, 2018 – Nov 24, 2018 1 message not in the archive KVM arm64

lore
Share & tools
lore

Peng Hao

author
#0 lore raw

PATCH 1/1 kvm: arm/arm64 : fix vm's hanging at startup time

When virtual machine starts, hang up. The kernel version of guest is 4.16. Host support vgic_v3.
It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.
group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
 virt/kvm/arm/vgic/vgic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +1 -1
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 	if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
 		irq->line_level = false;
 
-	if (irq->group)
+	if (model == KVM_DEV_TYPE_ARM_VGIC_V3)
 		val |= ICH_LR_GROUP;
 
 	val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;
Signature · 1 line
1.8.3.1
#1 lore raw

Hi Peng,

Quoted 2 lines · Peng Hao

When virtual machine starts, hang up. The kernel version of guest is 4.16. Host support vgic_v3.

I don't understand the issue here. the vgic_irq->group field does not exist in 4.16 (neither in the first nor the last version). Nor does the line you are modifying.

Both exist in mainline, but what you are mentioning shouldn't be an issue in mainline since vgic_irq->group gets initialized in kvm_vgic_vcpu_init for SGIs and PPIs (i.e. intid < 32) which includes your interrupt.

Cheers,

Quoted 28 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.
group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +1 -1
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
  	if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
  		irq->line_level = false;
 
-	if (irq->group)
+	if (model == KVM_DEV_TYPE_ARM_VGIC_V3)
  		val |= ICH_LR_GROUP;
 
  	val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;
Signature · 1 line
Julien Thierry
#2 lore raw
Quoted 9 lines

Hi Peng,

Quoted 2 lines · Peng Hao

When virtual machine starts, hang up. The kernel version of guest is 4.16. Host support vgic_v3.

I don't understand the issue here. the vgic_irq->group field does not exist in 4.16 (neither in the first nor the last version). Nor does the line you are modifying.

Guest's kernel version is 4.16 and Host's kernel version is mainline. vgic_irq is a structure of Host's kvm.

Quoted 5 lines

Both exist in mainline, but what you are mentioning shouldn't be an issue in mainline since vgic_irq->group gets initialized in kvm_vgic_vcpu_init for SGIs and PPIs (i.e. intid < 32) which includes your interrupt.

Initialization sequence of several components is like this;
1. kvm_vgic_vcpu_init ---- here, dist is not initialized at this time.

                                             and the value of irq->group depends on
                                              dist->vgic_model. but now dist->vgic_model = 0
                                               and irq->group=0.

2. vgic_create ----- assign to dist->vgic_model.
3. kvm_vgic_dist_init

thanks.

Quoted 29 lines

Cheers,

Quoted 26 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.
group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +1 -1
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
      if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
          irq->line_level = false;
 
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)
          val |= ICH_LR_GROUP;
 
Quoted 5 lines

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 204

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +0 -0
      val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;

--
Julien Thierry

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#3 lore raw
Quoted 16 lines · On 15/11/18 10:22, [email protected] wrote:
Quoted 9 lines

Hi Peng,

Quoted 2 lines · Peng Hao

When virtual machine starts, hang up. The kernel version of guest is 4.16. Host support vgic_v3.

I don't understand the issue here. the vgic_irq->group field does not exist in 4.16 (neither in the first nor the last version). Nor does the line you are modifying.

Guest's kernel version is 4.16 and Host's kernel version is mainline. vgic_irq is a structure of Host's kvm.

Quoted 5 lines

Both exist in mainline, but what you are mentioning shouldn't be an issue in mainline since vgic_irq->group gets initialized in kvm_vgic_vcpu_init for SGIs and PPIs (i.e. intid < 32) which includes your interrupt.

Quoted 8 lines

Initialization sequence of several components is like this;
1. kvm_vgic_vcpu_init ---- here, dist is not initialized at this time.

                                              and the value of irq->group depends on
                                               dist->vgic_model. but now dist->vgic_model = 0
                                                and irq->group=0.

2. vgic_create ----- assign to dist->vgic_model.
3. kvm_vgic_dist_init

I see, thanks for explaining.

This means the initialization in kvm_vgic_vcpu_init depends on whether userland creates the GIC device first or the vcpus first...

I don't think the issue lies in vgic_v3_populate_lr, but in kvm_vgic_vcpu_init which clearly expects dist->vgic_model to be initialized in two places. However, the comment at the top of vgic-init.c states:

* CPU Interface:

*

* - kvm_vgic_vcpu_init(): initialization of static data that

* doesn't depend on any sizing information or emulation type. No

* allocation is allowed there.

So I don't think we're supposed to check for dist->vgic_model here. Those two checks should probably be moved elsewhere (maybe in kvm_vgic_vcpu_enable?) once the gic type has been set.

Thanks,

Quoted 35 lines

thanks.

Quoted 34 lines

Cheers,

Quoted 20 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.
group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
   virt/kvm/arm/vgic/vgic-v3.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 204

Quoted 8 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
       if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
           irq->line_level = false;
 
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)
           val |= ICH_LR_GROUP;
 
       val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;

--
Julien Thierry

Signature · 1 line
Julien Thierry
#4 lore raw
In reply to Peng Hao

On Thu, 15 Nov 2018 23:14:08 +0800
Peng Hao <[email protected]> wrote:

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
 virt/kvm/arm/vgic/vgic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 201

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-	if (irq->group)
+	if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Cheers,
Andre.

Quoted 3 lines
virt/kvm/arm/vgic/vgic-v3.c +0 -0
 		val |= ICH_LR_GROUP;
 
 	val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;
#5 lore raw
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 25 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
 virt/kvm/arm/vgic/vgic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 201

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Quoted 4 lines

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Quoted 6 lines

Cheers,
Andre.

Quoted 3 lines
         val |= ICH_LR_GROUP;

     val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#6 lore raw
Quoted 47 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 25 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 201

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Quoted 7 lines
Quoted 4 lines

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Signature · 1 line
Julien Thierry
#7 lore raw
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 201

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 7 lines
Quoted 3 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 3 lines

--
Julien Thierry

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#8 lore raw
Quoted 59 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 201

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

Thanks,
Mark.

#9 lore raw
Quoted 67 lines · Mark Rutland
Quoted 59 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, peng.hao2 at zte.com.cn wrote:
Quoted 52 lines · On 16/11/18 00:23, peng.hao2 at zte.com.cn wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 201

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I doubt it. The EDK2 bug resulted in no interrupt being delivered due to the group being set to 1, and the interrupt being acknowledged through the Group0 accessor. Also, this was a 32bit special, and 64bit EDK2 was just fine.

What Peng describes here is the guest writing the group as 0, which doesn't make much sense (I certainly don't see EDK2 doing that).

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#10 lore raw
In reply to Mark Rutland
Quoted 67 lines · Mark Rutland
Quoted 59 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 201

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#11 lore raw
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I will update the UEFI version in recent days and test it later. Thanks.

Quoted 5 lines

Thanks,

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

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#12 lore raw
In reply to Marc Zyngier
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest. Thanks.

Quoted 5 lines

Thanks,

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

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#13 lore raw

Hi,

Quoted 90 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

Quoted 8 lines
[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest.

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +10 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 			irq->config = VGIC_CONFIG_LEVEL;
 		}
 
-		/*
-		 * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-		 * so we always know the emulation type at this point as it's
-		 * either explicitly configured as GICv3, or explicitly
-		 * configured as GICv2, or not configured yet which also
-		 * implies GICv2.
-		 */
 		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
 			irq->group = 1;
 		else
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
 	if (ret)
 		goto out;
 
+	/* Initialize groups on CPUs created before the VGIC type was known */
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+			irq->group = 1;
+		}
+	}
+
 	if (vgic_has_its(kvm)) {
 		ret = vgic_v4_init(kvm);
 		if (ret)

Thanks,

Christoffer

#14 lore raw
Quoted 131 lines · Christoffer Dall

Hi,

Quoted 90 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
   virt/kvm/arm/vgic/vgic-v3.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

Quoted 8 lines
[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest.

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +3 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
  			irq->config = VGIC_CONFIG_LEVEL;
  		}
 
-		/*
-		 * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-		 * so we always know the emulation type at this point as it's
-		 * either explicitly configured as GICv3, or explicitly
-		 * configured as GICv2, or not configured yet which also
-		 * implies GICv2.
-		 */
  		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
  			irq->group = 1;
  		else
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
  	if (ret)
  		goto out;
 
+	/* Initialize groups on CPUs created before the VGIC type was known */
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;

virt/kvm/arm/vgic/vgic-init.c int vgic_init(struct kvm *kvm) · line 306

Quoted 10 lines
virt/kvm/arm/vgic/vgic-init.c +7 -0
+
+		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+			irq->group = 1;
+		}
+	}
+
  	if (vgic_has_its(kvm)) {
  		ret = vgic_v4_init(kvm);
  		if (ret)

If the value of dist->vgic_model is not properly initialized at the time we call kvm_vgic_vcpu_init is call, won't we still overwrite the irq->group when we get there? (I still haven't seen why we could consider dist->vgic_model is initialized at that point).

Shouldn't we do the irq->group initialization somewhere in kvm_arch_vcpu_ioctl_vcpu_init? (with some vgic_* function to encapsulate it of course). At that point I believe we know that dist->vgic_model is initialized.

Cheers,

Signature · 1 line
Julien Thierry
#15 lore raw
Quoted 138 lines · Julien Thierry
Quoted 135 lines · Christoffer Dall

Hi,

Quoted 90 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

Quoted 8 lines
[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest.

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +10 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 			irq->config = VGIC_CONFIG_LEVEL;
 		}
-		/*
-		 * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-		 * so we always know the emulation type at this point as it's
-		 * either explicitly configured as GICv3, or explicitly
-		 * configured as GICv2, or not configured yet which also
-		 * implies GICv2.
-		 */
 		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
 			irq->group = 1;
 		else
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
 	if (ret)
 		goto out;
+	/* Initialize groups on CPUs created before the VGIC type was known */
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+			irq->group = 1;
+		}
+	}
+
 	if (vgic_has_its(kvm)) {
Quoted 8 lines

virt/kvm/arm/vgic/vgic-init.c int vgic_init(struct kvm *kvm)

Quoted 4 lines
virt/kvm/arm/vgic/vgic-init.c +0 -0
 		ret = vgic_v4_init(kvm);
 		if (ret)
 

If the value of dist->vgic_model is not properly initialized at the time we call kvm_vgic_vcpu_init is call, won't we still overwrite the irq->group when we get there?

I don't understand this question. When we get where?

Quoted 2 lines

(I still haven't seen why we could consider
dist->vgic_model is initialized at that point).

Because there is no enforced ordering between creating VCPUs and creating the VGIC.

Quoted 6 lines

Shouldn't we do the irq->group initialization somewhere in kvm_arch_vcpu_ioctl_vcpu_init? (with some vgic_* function to encapsulate it of course). At that point I believe we know that dist->vgic_model is initialized.

See above. AFAICT we don't have a single point at which we can do everything because creation of the two components can be interleaved.

We could hook into kvm_vcpu_first_run_init(), but then userspace can observe uninitialized values if it looks at the GIC state prior to running the VCPUs, which is also not great.

In other words, I think the problem is that you can do:

  create_vcpu(0);
  create_vgic(v3);
  create_vcpu(2);

Now you'll have vcpu0 have its private IRQs set to group 0, and you'll have vcpu1 have its private IRQs set to group 1 (prior to my patch).

Am I missing something?

Thanks,

Christoffer

#16 lore raw
Quoted 148 lines · Christoffer Dall
Quoted 138 lines · Julien Thierry
Quoted 135 lines · Christoffer Dall

Hi,

Quoted 90 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
   virt/kvm/arm/vgic/vgic-v3.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

Quoted 8 lines
[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest.

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +10 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
  			irq->config = VGIC_CONFIG_LEVEL;
  		}
-		/*
-		 * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-		 * so we always know the emulation type at this point as it's
-		 * either explicitly configured as GICv3, or explicitly
-		 * configured as GICv2, or not configured yet which also
-		 * implies GICv2.
-		 */
  		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
  			irq->group = 1;
  		else
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
  	if (ret)
  		goto out;
+	/* Initialize groups on CPUs created before the VGIC type was known */
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+			irq->group = 1;
+		}
+	}
+
  	if (vgic_has_its(kvm)) {
Quoted 8 lines

virt/kvm/arm/vgic/vgic-init.c int vgic_init(struct kvm *kvm)

Quoted 4 lines
virt/kvm/arm/vgic/vgic-init.c +0 -0
  		ret = vgic_v4_init(kvm);
  		if (ret)
 

If the value of dist->vgic_model is not properly initialized at the time we call kvm_vgic_vcpu_init is call, won't we still overwrite the irq->group when we get there?

Quoted 1 line

I don't understand this question. When we get where?

Sorry, I meant when we get to the irq->group initialization in kvm_vgic_vcpu_init.

Quoted 6 lines
Quoted 3 lines

(I still haven't seen why we could consider
dist->vgic_model is initialized at that point).

Because there is no enforced ordering between creating VCPUs and creating the VGIC.

Hmmm, I think that's what I am saying, so we shouldn't look at the value of vgic_dist->vgic_model since it could be uninitialized (or 0).

Quoted 21 lines
Quoted 7 lines

Shouldn't we do the irq->group initialization somewhere in kvm_arch_vcpu_ioctl_vcpu_init? (with some vgic_* function to encapsulate it of course). At that point I believe we know that dist->vgic_model is initialized.

See above. AFAICT we don't have a single point at which we can do everything because creation of the two components can be interleaved.

We could hook into kvm_vcpu_first_run_init(), but then userspace can observe uninitialized values if it looks at the GIC state prior to running the VCPUs, which is also not great.

In other words, I think the problem is that you can do:

   create_vcpu(0);
   create_vgic(v3);
   create_vcpu(2);
Quoted 6 lines

Now you'll have vcpu0 have its private IRQs set to group 0, and you'll have vcpu1 have its private IRQs set to group 1 (prior to my patch).

Am I missing something?

No, I just got confused between create_vgic and vgic_init. So the place looks fine. Sorry for the confusion.

The fact that we still check dist->vgic_model in kvm_vgic_vcpu_init (twice actually), however seems dodgy to me since it might not have been initialized.

Thanks,

Signature · 1 line
Julien Thierry
#17 lore raw
Quoted 194 lines · Julien Thierry
Quoted 148 lines · Christoffer Dall
Quoted 138 lines · Julien Thierry
Quoted 135 lines · Christoffer Dall

Hi,

Quoted 90 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

Quoted 8 lines
[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest.

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +10 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 			irq->config = VGIC_CONFIG_LEVEL;
 		}
-		/*
-		 * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-		 * so we always know the emulation type at this point as it's
-		 * either explicitly configured as GICv3, or explicitly
-		 * configured as GICv2, or not configured yet which also
-		 * implies GICv2.
-		 */
 		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
 			irq->group = 1;
 		else
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
 	if (ret)
 		goto out;
+	/* Initialize groups on CPUs created before the VGIC type was known */
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+		for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+			struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+			irq->group = 1;
+		}
+	}
+
 	if (vgic_has_its(kvm)) {
Quoted 8 lines

virt/kvm/arm/vgic/vgic-init.c int vgic_init(struct kvm *kvm)

Quoted 4 lines
virt/kvm/arm/vgic/vgic-init.c +0 -0
 		ret = vgic_v4_init(kvm);
 		if (ret)
 

If the value of dist->vgic_model is not properly initialized at the time we call kvm_vgic_vcpu_init is call, won't we still overwrite the irq->group when we get there?

Quoted 1 line

I don't understand this question. When we get where?

Sorry, I meant when we get to the irq->group initialization in kvm_vgic_vcpu_init.

Quoted 6 lines
Quoted 3 lines

(I still haven't seen why we could consider
dist->vgic_model is initialized at that point).

Because there is no enforced ordering between creating VCPUs and creating the VGIC.

Hmmm, I think that's what I am saying, so we shouldn't look at the value of vgic_dist->vgic_model since it could be uninitialized (or 0).

Quoted 21 lines
Quoted 7 lines

Shouldn't we do the irq->group initialization somewhere in kvm_arch_vcpu_ioctl_vcpu_init? (with some vgic_* function to encapsulate it of course). At that point I believe we know that dist->vgic_model is initialized.

See above. AFAICT we don't have a single point at which we can do everything because creation of the two components can be interleaved.

We could hook into kvm_vcpu_first_run_init(), but then userspace can observe uninitialized values if it looks at the GIC state prior to running the VCPUs, which is also not great.

In other words, I think the problem is that you can do:

  create_vcpu(0);
  create_vgic(v3);
  create_vcpu(2);
Quoted 6 lines

Now you'll have vcpu0 have its private IRQs set to group 0, and you'll have vcpu1 have its private IRQs set to group 1 (prior to my patch).

Am I missing something?

Quoted 7 lines

No, I just got confused between create_vgic and vgic_init. So the place looks fine. Sorry for the confusion.

The fact that we still check dist->vgic_model in kvm_vgic_vcpu_init (twice actually), however seems dodgy to me since it might not have been initialized.

It might not, but it might also be. In which case you want to initialize the CPU to the right values. So you have:

1. VCPUs created before the VGIC are initialized according to the model version when the VGIC is created.

2. VCPUs created after the VGIC is initialized are initialized according to the model as they are created.

The alternative is to have a sync-point after the two above, but I haven't been able to find a good API-backwards way of doing that.

Thanks,

Christoffer

#18 lore raw
In reply to Christoffer Dall
Quoted 128 lines

Hi,

Quoted 90 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through >vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

Quoted 8 lines
[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg >timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest.

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +0 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 irq->config = VGIC_CONFIG_LEVEL;
 }
-        /*
-         * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-         * so we always know the emulation type at this point as it's
-         * either explicitly configured as GICv3, or explicitly
-         * configured as GICv2, or not configured yet which also
-         * implies GICv2.
-         */
 if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
 irq->group = 1;

else

virt/kvm/arm/vgic/vgic-init.c +4 -0
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
 if (ret)
 goto out;
+    /* Initialize groups on CPUs created before the VGIC type was known */
+    kvm_for_each_vcpu(i, vcpu, kvm) {
+        struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+

virt/kvm/arm/vgic/vgic-init.c int vgic_init(struct kvm *kvm) · line 306

Quoted 10 lines
virt/kvm/arm/vgic/vgic-init.c +6 -0
+        for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+            struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+            irq->group = 1;
+        }
+    }
+
 if (vgic_has_its(kvm)) {
 ret = vgic_v4_init(kvm);
 if (ret)
 

Sorry for the late reply.
I test the patch and it works. But I think there is randomness here. 1.vgic_init: set intid/27 group=1 2.vgic_mmio_write_group:## intid/27 group=0
3.!!! receive a vtimer irq ----- it may exist.
4.vgic_mmio_write_group:## intid/27 group=1

I made an attempt before:
1. kvm_vgic_vcpu_init

     according to if  kvm_vgic_global_state.type == VGIC_V3, set group=1

2.vgic_mmio_write_group:## intid/27 group=0
3.!!! receive a vtimer irq -----I really receive a vtimer irq, then the guest hangs.

thanks.

Quoted 4 lines

Thanks,

Christoffer

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#19 lore raw
Quoted 136 lines · On Fri, Nov 23, 2018 at 02:01:56PM +0800, [email protected] wrote:
Quoted 136 lines

Hi,

Quoted 90 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 80 lines · Mark Rutland
Quoted 67 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 59 lines
Quoted 52 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 51 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 29 lines
Quoted 18 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c

Quoted 2 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

This is not the right fix, not only because it basically reverts the GICv3 part of 87322099052 (KVM: arm/arm64: vgic: Signal IRQs using their configured group).

Can you try to work out why kvm_vgic_vcpu_init() is apparently called before dist->vgic_model is set, also what value it has?
If I understand the code correctly, that shouldn't happen for a GICv3.

Quoted 6 lines
Quoted 2 lines

Even if the value of group is correctly assigned in kvm_vgic_vcpu_init, the group is then written 0 through >vgic_mmio_write_group.
If the interrupt comes at this time, the interrupt injection fails.

Does that mean that the guest is configuring its interrupts as Group0? That sounds wrong, Linux should configure all it's interrupts as non-secure group1.

Quoted 6 lines

no, I think that uefi dose this, not linux.
1. kvm_vgic_vcpu_init
2. vgic_create
3. kvm_vgic_dist_init 4.vgic_mmio_write_group: uefi as guest, write group=0 5.vgic_mmio_write_group: linux as guest, write group=1

Quoted 7 lines

Is this the same issue fixed by EDK2 commit:

66127011a544b90e ("ArmPkg/ArmGicDxe ARM: fix encoding for GICv3 interrupt acknowledge")

... where EDK2 would try to use IAR0 rather than IAR1?

The commit messages notes this lead to a boot-time hang.

I managed to trigger an issue with a really old EFI implementation that doesn't configure its interrupts as Group1, and yet tries to ACK its interrupts using the Group1 accessor. Guess what? It is not going to work.

Quoted 9 lines

Commit c7fefb690661f2e38afcb8200bd318ecf38ab961 in the edk2 tree seems to be the fix (I only assume it does, I haven't actually checked). A recent build, as found in Debian Buster, works perfectly (tested with both QEMU v2.12 and tip of tree).

Now, I really don't get what you're saying about Linux not getting interrupts. How do you get to booting Linux if EFI is not making any forward progress? Are you trying them independently?

I start linux with bypassing uefi, the print info is the same.

Quoted 8 lines
[507107.748908]  vgic_mmio_write_group:## intid/27 group=0
[507107.752185]  vgic_mmio_write_group:## intid/27 group=0
[507107.899566]  vgic_mmio_write_group:## intid/27 group=1
[507107.907370]  vgic_mmio_write_group:## intid/27 group=1

the command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -kernel /home/kernelboot/vmlinuz-4.16.0+ -initrd /home/kernelboot/initramfs-4.16.0+.img -append root=/dev/mapper/cla-root ro crashkernel=auto rd.lvm.lv=cla/root rd.lvm.lv=cla/swap.UTF-8 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg >timestamp=on

This is strange. That's probably the Linux 4.16 kernel setting group to 0, and I'll continue to track in guest.

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +0 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 irq->config = VGIC_CONFIG_LEVEL;
 }
-        /*
-         * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-         * so we always know the emulation type at this point as it's
-         * either explicitly configured as GICv3, or explicitly
-         * configured as GICv2, or not configured yet which also
-         * implies GICv2.
-         */
 if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
 irq->group = 1;

else

virt/kvm/arm/vgic/vgic-init.c +10 -0
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
 if (ret)
 goto out;
+    /* Initialize groups on CPUs created before the VGIC type was known */
+    kvm_for_each_vcpu(i, vcpu, kvm) {
+        struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+        for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+            struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+            irq->group = 1;
+        }
+    }
+
 if (vgic_has_its(kvm)) {
 ret = vgic_v4_init(kvm);
Quoted 8 lines

virt/kvm/arm/vgic/vgic-init.c int vgic_init(struct kvm *kvm) · line 293

Quoted 2 lines
virt/kvm/arm/vgic/vgic-init.c +0 -0
 if (ret)
 

Sorry for the late reply.
I test the patch and it works. But I think there is randomness here. 1.vgic_init: set intid/27 group=1 2.vgic_mmio_write_group:## intid/27 group=0
3.!!! receive a vtimer irq ----- it may exist.
4.vgic_mmio_write_group:## intid/27 group=1

Are these writes from userspace or from the guest? (I assume they're from the guest.)

Quoted 2 lines

I made an attempt before:

Was this previous experiment using my patch or without my patch?

Quoted 5 lines

1. kvm_vgic_vcpu_init

     according to if  kvm_vgic_global_state.type == VGIC_V3, set group=1

2.vgic_mmio_write_group:## intid/27 group=0
3.!!! receive a vtimer irq -----I really receive a vtimer irq, then the guest hangs.

If the guest you're using is flipping the group of the interrupt and yet enabling the device and interrupt while flipping the group, then it's not expected to work and it's a guest bug.

My hunch here tells me that my patch is what we need for KVM, and either

(1) You no longer have a problem but are tracing some guest initialization behavior which is fine, or

(2) You still have a problem because you also have some broken guest code (guest UEFI or guest Linux) that also needs fixing.

If you can demonstrate that you still have a problem, with my path applied to KVM, and with a recent guest Linux and guest UEFI, then I'll be happy to have another look. In that case, could you please provide the exact versions of your guest Linux and UEFI, as well as the QEMU version and command line.

Thanks,

Christoffer

#20 lore raw

PATCH 1/1 kvm: arm/arm64 : fix vm's hanging at startup time

Quoted 48 lines
Quoted 48 lines
Quoted 48 lines

Hi,

Quoted 45 lines · On Wed, Nov 21, 2018 at 04:56:54PM +0800, [email protected] wrote:
Quoted 45 lines · Mark Rutland
Quoted 44 lines · On Sat, Nov 17, 2018 at 10:58:37AM +0800, [email protected] wrote:
Quoted 43 lines
Quoted 43 lines · On 16/11/18 00:23, [email protected] wrote:
Quoted 42 lines
Quoted 11 lines

Hi,

Quoted 1 line

When virtual machine starts, hang up.

I take it you mean the *guest* hangs? Because it doesn't get a timer interrupt?

Quoted 2 lines

The kernel version of guest
is 4.16. Host support vgic_v3.

Your host kernel is something recent, I guess?

Quoted 8 lines
Quoted 4 lines

It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.

Mmh, that shouldn't happen with (v)GICv3. Do you use QEMU (which version?) or some other userland tool?

QEMU emulator version 3.0.50 .

Quoted 20 lines
Quoted 20 lines

group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
  virt/kvm/arm/vgic/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu,

struct vgic_irq *irq, int lr) if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) irq->line_level = false;

virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)

[...]

Quoted 66 lines
Quoted 34 lines
Quoted 34 lines

Could you try the following patch:

virt/kvm/arm/vgic/vgic-init.c +0 -7
index c0c0b88af1d5..6fa858c7a5a6 100644
@@ -231,13 +231,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 irq->config = VGIC_CONFIG_LEVEL;
 }
-        /*
-         * GICv3 can only be created via the KVM_DEVICE_CREATE API and
-         * so we always know the emulation type at this point as it's
-         * either explicitly configured as GICv3, or explicitly
-         * configured as GICv2, or not configured yet which also
-         * implies GICv2.
-         */
 if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
 irq->group = 1;

else

virt/kvm/arm/vgic/vgic-init.c +10 -0
@@ -298,6 +291,16 @@ int vgic_init(struct kvm *kvm)
 if (ret)
 goto out;
+    /* Initialize groups on CPUs created before the VGIC type was known */
+    kvm_for_each_vcpu(i, vcpu, kvm) {
+        struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+        for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
+            struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
+            irq->group = 1;
+        }
+    }
+
 if (vgic_has_its(kvm)) {
 ret = vgic_v4_init(kvm);
Quoted 9 lines

virt/kvm/arm/vgic/vgic-init.c int vgic_init(struct kvm *kvm) · line 293

Quoted 2 lines
virt/kvm/arm/vgic/vgic-init.c +0 -0
 if (ret)
 

Sorry for the late reply.
I test the patch and it works. But I think there is randomness here. 1.vgic_init: set intid/27 group=1 2.vgic_mmio_write_group:## intid/27 group=0
3.!!! receive a vtimer irq ----- it may exist.
4.vgic_mmio_write_group:## intid/27 group=1

Are these writes from userspace or from the guest? (I assume they're from the guest.)

Quoted 10 lines

I made an attempt before:

Was this previous experiment using my patch or without my patch?

1. kvm_vgic_vcpu_init

     according to if  kvm_vgic_global_state.type == VGIC_V3, set group=1

2.vgic_mmio_write_group:## intid/27 group=0
3.!!! receive a vtimer irq -----I really receive a vtimer irq, then the guest hangs.

If the guest you're using is flipping the group of the interrupt and yet enabling the device and interrupt while flipping the group, then it's not expected to work and it's a guest bug.

My hunch here tells me that my patch is what we need for KVM, and either

(1) You no longer have a problem but are tracing some guest initialization behavior which is fine, or

Quoted 10 lines

(2) You still have a problem because you also have some broken guest code (guest UEFI or guest Linux) that also needs fixing.

If you can demonstrate that you still have a problem, with my path applied to KVM, and with a recent guest Linux and guest UEFI, then I'll be happy to have another look. In that case, could you please provide the exact versions of your guest Linux and UEFI, as well as the QEMU version and command line.

I'm sorry I make a mistake.I updated UEFI but never had time to test it. Yesterday I merged your patch
to test, but forgot that I had updated UEFI.
Here are the corrected test results :
-----------------------------------------------------------------------------------------

|_new uefi + your patch_____________|______guest start normally__|
|_old uefi + your patch______________|______guest hangs________|
|_bypass uefi _(with/without your patch)|______guest start normally__|
|_new uefi + without your patch ______|______guest start normally_  |
|_old uefi + without your patch_ ______| _____ guest hangs________|

So the problem seems to be in UEFI. new uefi can be sure there is no a irq between the group=0 and group=1 from vgic_mmio_write_group.

guest kernel :4.16.0 (commit c18bb396d3d261ebbb4efbc05129c5d354c541e4)
the version of guest old uefi corresponds to qemu 3.0.50.
the version of new uefi is the latest edk2.
I use libvirt to start guest, and the qemu command line is like this:
/home/qemu-patch/linshi/qemu/aarch64-softmmu/qemu-system-aarch64 -name guest=centos74-ph-1,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-1-centos74-ph-1/master-key.aes -machine virt-3.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 -cpu host -drive file=/usr/share/AAVMF/AAVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on -drive file=/var/lib/libvirt/qemu/nvram/centos74-ph-1_VARS.fd,if=pflash,format=raw,unit=1 -m 4096 -realtime mlock=off -smp 4,maxcpus=8,sockets=8,cores=1,threads=1 -uuid ff5ecf4e-1234-425f-948d-8d91135e109c -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-1-centos74-ph-1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot strict=on -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 -device pcie-root-port,port=0x10,chassis=3,id=pci.3,bus=pcie.0,multifunction=on,addr=0x2 -device pcie-root-port,port=0x11,chassis=4,id=pci.4,bus=pcie.0,addr=0x2.0x1 -device usb-ehci,id=usb,bus=pci.2,addr=0x1 -device piix3-usb-uhci,id=usb1,bus=pci.2,addr=0x2 -device virtio-scsi-device,id=scsi0 -device virtio-serial-device,id=virtio-serial0 -drive file=/home/centos74-ph/boot.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -drive file=/home/centos74-ph/data.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-1 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 -drive file=/home/CentOS-7-aarch64-Everything.iso,format=raw,if=none,id=drive-scsi0-0-0-2,readonly=on -device scsi-cd,bus=scsi0.0,channel=0,scsi-id=0,lun=2,drive=drive-scsi0-0-0-2,id=scsi0-0-0-2,bootindex=2 -netdev tap,fd=26,id=hostnet0,vhost=on,vhostfd=28 -device virtio-net-device,netdev=hostnet0,id=net0,mac=aa:35:97:5b:33:ac -add-fd set=2,fd=30 -chardev pty,id=charserial0,logfile=/dev/fdset/2,logappend=on -serial chardev:charserial0 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/centos74.agent,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 -device usb-tablet,id=input0,bus=usb.0,port=1 -device usb-kbd,id=input1,bus=usb.0,port=2 -vnc 0.0.0.0:0 -k en-us -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.3,addr=0x0 -device pvpanic-mmio -msg timestamp=on

Thanks.

Quoted 3 lines

Thanks,

Christoffer

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#21 lore raw
In reply to Peng Hao
Quoted 22 lines · Peng Hao

When virtual machine starts, hang up. The kernel version of guest is 4.16. Host support vgic_v3.
It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.
group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
 virt/kvm/arm/vgic/vgic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 204

Quoted 8 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 	if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
 		irq->line_level = false;
 
-	if (irq->group)
+	if (model == KVM_DEV_TYPE_ARM_VGIC_V3)
 		val |= ICH_LR_GROUP;
 
 	val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;

This patch makes exactly zero sense. The group is a property of the interrupt, and has nothing to do with the model used for the emulation

Which version of the kernel is this against? What userspace are you using? How can I reproduce this issue?

Thanks,

	M.
Signature · 1 line
Jazz is not dead, it just smell funny.
#22 lore raw
Quoted 25 lines
Quoted 25 lines

When virtual machine starts, hang up. The kernel version of guest is 4.16. Host support vgic_v3.
It was mainly due to the incorrect vgic_irq's(intid=27) group value during injection interruption. when kvm_vgic_vcpu_init is called, dist is not initialized at this time. Unable to get vgic V3 or V2 correctly, so group is not set.
group is setted to 1 when vgic_mmio_write_group is invoked at some time.
when irq->group=0 (intid=27), No ICH_LR_GROUP flag was set and interrupt injection failed.

---
 virt/kvm/arm/vgic/vgic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic/vgic-v3.c +0 -0
index 9c0dd23..d101000 100644
@@ -198,7 +198,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
     if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
         irq->line_level = false;
 
Quoted 9 lines

virt/kvm/arm/vgic/vgic-v3.c void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) · line 204

Quoted 5 lines
virt/kvm/arm/vgic/vgic-v3.c +1 -1
-    if (irq->group)
+    if (model == KVM_DEV_TYPE_ARM_VGIC_V3)
         val |= ICH_LR_GROUP;
 
     val |= (u64)irq->priority << ICH_LR_PRIORITY_SHIFT;

This patch makes exactly zero sense. The group is a property of the interrupt, and has nothing to do with the model used for the emulation

in kvm_vgic_vcpu_init, the value of group is dependent on vgic_model.

Quoted 2 lines

Which version of the kernel is this against? What userspace are you using? How can I reproduce this issue?

QEMU emulator version 3.0.50 .
host kernel: 4.19.0-rc4+
guest kernel :4.16.0
when starting the virtual machine, it will happen.

Thanks.

Quoted 7 lines

Thanks,

M.

--
Jazz is not dead, it just smell funny.

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm