Skip to contentSkip to conversation

KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

PATCH New Andre Przywara 7 participants 19 messages Aug 14, 2013 – Aug 15, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

---
 arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm/kvm/handle_exit.c +20 -0
index df4c82d..2cbe6a0 100644
@@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	return 1;
 }
 
+/*
+ * OMAP4 and Highbank machines do a SMC call to handle the L2 cache
+ * controller. They put 0x102 in r12 to request this functionality.
+ * This is not needed on A15s, so we can safely ignore it in KVM guests.
+ */
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)
+{
+	unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+	if (fn_nr == 0x102) {
+		kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+		return 1;
+	}
+
+	return 0;
+}
+
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
+	if (kvm_ignore_l2x0_call(vcpu))
+		return 1;
+
 	kvm_inject_undefined(vcpu);
 	return 1;
 }
Signature · 1 line
1.7.12.1
#1 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 9 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

Hold on.

Are you trying to run A9 guests on KVM? Sorry, but that's not a supported mode of operation just yet.

So, until we have a proper framework to deal with multiple CPUs, the only valid configuration is A15-on-A15.

Quoted 20 lines
---
 arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm/kvm/handle_exit.c +4 -0
index df4c82d..2cbe6a0 100644
@@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu,
 struct kvm_run *run)
 	return 1;
 }
 
+/*
+ * OMAP4 and Highbank machines do a SMC call to handle the L2 cache
+ * controller. They put 0x102 in r12 to request this functionality.
+ * This is not needed on A15s, so we can safely ignore it in KVM 

guests.
+ */

arch/arm/kvm/handle_exit.c +1 -0
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 68

Quoted 10 lines
arch/arm/kvm/handle_exit.c +10 -0
+{
+	unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+	if (fn_nr == 0x102) {
+		kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+		return 1;
+	}
+
+	return 0;
+}

And what if I run mach-foo which uses r12 to request bar services from secure mode? Is it safe to ignore it? We need something much better than just testing random registers to guess what the guest wants.

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 77

Quoted 8 lines

static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run) {

arch/arm/kvm/handle_exit.c +3 -0
+	if (kvm_ignore_l2x0_call(vcpu))
+		return 1;
+
 	kvm_inject_undefined(vcpu);
 	return 1;
 }
         M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
#2 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

Quoted 6 lines
Quoted 1 line

Hold on.

Are you trying to run A9 guests on KVM? Sorry, but that's not a supported mode of operation just yet.

No, I don't. I just run guests with kernels that would support A9 also. If you select Highbank in your .config, you will get CACHE_L2X0 and the kernel will do SMCs - regardless of the CPU you are running on. Those SMCs are ignored by the firmware on Midway.

I agree that the proper solution would be to detect at run-time in the (guest) kernel whether you actually need the PL310 handling, but for the time being and to support older kernels we will need this fix.

For me this fixes "qemu -machine midway --enable-kvm".

Regards,
Andre.

Quoted 50 lines

So, until we have a proper framework to deal with multiple CPUs, the only valid configuration is A15-on-A15.

Quoted 20 lines
---
  arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
  1 file changed, 20 insertions(+)
arch/arm/kvm/handle_exit.c +4 -0
index df4c82d..2cbe6a0 100644
@@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu,
 struct kvm_run *run)
  	return 1;
  }
 
+/*
+ * OMAP4 and Highbank machines do a SMC call to handle the L2 cache
+ * controller. They put 0x102 in r12 to request this functionality.
+ * This is not needed on A15s, so we can safely ignore it in KVM

guests.
+ */

arch/arm/kvm/handle_exit.c +1 -0
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 68

Quoted 10 lines
arch/arm/kvm/handle_exit.c +10 -0
+{
+	unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+	if (fn_nr == 0x102) {
+		kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+		return 1;
+	}
+
+	return 0;
+}

And what if I run mach-foo which uses r12 to request bar services from secure mode? Is it safe to ignore it? We need something much better than just testing random registers to guess what the guest wants.

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 77

Quoted 8 lines
  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
  {
arch/arm/kvm/handle_exit.c +3 -0
+	if (kvm_ignore_l2x0_call(vcpu))
+		return 1;
+
  	kvm_inject_undefined(vcpu);
  	return 1;
  }
          M.
#3 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 24 lines · Andre Przywara
Quoted 11 lines · Marc Zyngier
Quoted 10 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore,
so it is safe to simply ignore these calls and proceed with the next
instruction.

Quoted 6 lines
Quoted 1 line

Hold on.

Are you trying to run A9 guests on KVM? Sorry, but that's not a supported mode of operation just yet.

No, I don't. I just run guests with kernels that would support A9 also. If you select Highbank in your .config, you will get CACHE_L2X0 and the kernel will do SMCs - regardless of the CPU you are running on. Those SMCs are ignored by the firmware on Midway.

Quoted 6 lines

I agree that the proper solution would be to detect at run-time in the (guest) kernel whether you actually need the PL310 handling, but for the time being and to support older kernels we will need this fix.

For me this fixes "qemu -machine midway --enable-kvm".

I understand that this fixes an issue, but I'd rather see either the guest kernel being fixed, or some decent framework for SMC handling in KVM (potentially leaving it to platform emulation to handle it).

Testing random registers won't cut it, I'm afraid.

         M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
#4 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

In reply to Andre Przywara
Quoted 27 lines · Andre Przywara
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

Quoted 6 lines
Quoted 1 line

Hold on.

Are you trying to run A9 guests on KVM? Sorry, but that's not a supported mode of operation just yet.

No, I don't. I just run guests with kernels that would support A9 also. If you select Highbank in your .config, you will get CACHE_L2X0 and the kernel will do SMCs - regardless of the CPU you are running on. Those SMCs are ignored by the firmware on Midway.

I agree that the proper solution would be to detect at run-time in the (guest) kernel whether you actually need the PL310 handling, but for the time being and to support older kernels we will need this fix.

Quoted 55 lines

For me this fixes "qemu -machine midway --enable-kvm".

Regards,
Andre.

Quoted 50 lines

So, until we have a proper framework to deal with multiple CPUs, the only valid configuration is A15-on-A15.

Quoted 20 lines
---
 arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm/kvm/handle_exit.c +4 -0
index df4c82d..2cbe6a0 100644
@@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu,
 struct kvm_run *run)
 	return 1;
 }
 
+/*
+ * OMAP4 and Highbank machines do a SMC call to handle the L2 cache
+ * controller. They put 0x102 in r12 to request this functionality.
+ * This is not needed on A15s, so we can safely ignore it in KVM

guests.
+ */

arch/arm/kvm/handle_exit.c +1 -0
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 68

Quoted 10 lines
arch/arm/kvm/handle_exit.c +10 -0
+{
+	unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+	if (fn_nr == 0x102) {
+		kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+		return 1;
+	}
+
+	return 0;
+}

And what if I run mach-foo which uses r12 to request bar services from secure mode? Is it safe to ignore it? We need something much better than just testing random registers to guess what the guest wants.

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 77

Quoted 8 lines

static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run) {

arch/arm/kvm/handle_exit.c +3 -0
+	if (kvm_ignore_l2x0_call(vcpu))
+		return 1;
+
 	kvm_inject_undefined(vcpu);
 	return 1;
 }
         M.

Now, suppose a second board family has a similar problem, with a different SMC ABI, which doesn't use r12.

If the kernel might believe it is running on that board, it may make SMC calls, but r12 may contain garbage. Sometimes it will contain 0x102 and hit your code.

Because this is a board-specific emulation issue, not virtualisation, it seems wrong to put knowledge about every platform's random firmware into KVM.

I see two solutions:

1) Describe the presence/absence of the firmware in the DT.

2) Provide a framework which allows qemu to emulate the needed parts of the the firmware (i.e., allowing SMCs to be trapped back to qemu)

(1) is the best option for any situation where we don't have legacy to support (i.e., we're not trying to run old kernels which don't know about the DT binding)

(2) allows for the most authentic simulation in KVM. It's also the
only way to be backwards compatible with older kernels that don't understand the added DT bindings.

We would also want a way to turn the PSCI implementation in the kernel off: that's valid for the kvmtool case, because it is part of the canonical paravirtualised environment. But it's not valid for the qemu full emulation case where we probably want to punt HVC/SMC calls back to qemu for emulation, or otherwise fault/ignore them (depending on whether the board in question has the Security/Virtualisation extensions, and on what the firmware is supposed to be present).

Cheers
---Dave

#5 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 106 lines · Dave Martin
Quoted 27 lines · Andre Przywara
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

Quoted 6 lines
Quoted 1 line

Hold on.

Are you trying to run A9 guests on KVM? Sorry, but that's not a supported mode of operation just yet.

No, I don't. I just run guests with kernels that would support A9 also. If you select Highbank in your .config, you will get CACHE_L2X0 and the kernel will do SMCs - regardless of the CPU you are running on. Those SMCs are ignored by the firmware on Midway.

I agree that the proper solution would be to detect at run-time in the (guest) kernel whether you actually need the PL310 handling, but for the time being and to support older kernels we will need this fix.

Quoted 55 lines

For me this fixes "qemu -machine midway --enable-kvm".

Regards,
Andre.

Quoted 50 lines

So, until we have a proper framework to deal with multiple CPUs, the only valid configuration is A15-on-A15.

Quoted 20 lines
---
 arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/arm/kvm/handle_exit.c +4 -0
index df4c82d..2cbe6a0 100644
@@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu,
 struct kvm_run *run)
 	return 1;
 }
 
+/*
+ * OMAP4 and Highbank machines do a SMC call to handle the L2 cache
+ * controller. They put 0x102 in r12 to request this functionality.
+ * This is not needed on A15s, so we can safely ignore it in KVM

guests.
+ */

arch/arm/kvm/handle_exit.c +1 -0
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 68

Quoted 10 lines
arch/arm/kvm/handle_exit.c +10 -0
+{
+	unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+	if (fn_nr == 0x102) {
+		kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+		return 1;
+	}
+
+	return 0;
+}

And what if I run mach-foo which uses r12 to request bar services from secure mode? Is it safe to ignore it? We need something much better than just testing random registers to guess what the guest wants.

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 77

Quoted 8 lines

static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run) {

arch/arm/kvm/handle_exit.c +3 -0
+	if (kvm_ignore_l2x0_call(vcpu))
+		return 1;
+
 	kvm_inject_undefined(vcpu);
 	return 1;
 }
         M.

Now, suppose a second board family has a similar problem, with a different SMC ABI, which doesn't use r12.

If the kernel might believe it is running on that board, it may make SMC calls, but r12 may contain garbage. Sometimes it will contain 0x102 and hit your code.

Because this is a board-specific emulation issue, not virtualisation, it seems wrong to put knowledge about every platform's random firmware into KVM.

I see two solutions:

1) Describe the presence/absence of the firmware in the DT.

2) Provide a framework which allows qemu to emulate the needed parts of the the firmware (i.e., allowing SMCs to be trapped back to qemu)

Quoted 7 lines

(1) is the best option for any situation where we don't have legacy to support (i.e., we're not trying to run old kernels which don't know about the DT binding)

(2) allows for the most authentic simulation in KVM. It's also the
only way to be backwards compatible with older kernels that don't understand the added DT bindings.

The question really is if we need legacy support for kernels. I suspect that this use case will arise (we are already hearing some chatter about this from the networking space) and therefore we will most likely end up with some combination of (1) and (2).

Quoted 10 lines

We would also want a way to turn the PSCI implementation in the kernel off: that's valid for the kvmtool case, because it is part of the canonical paravirtualised environment. But it's not valid for the qemu full emulation case where we probably want to punt HVC/SMC calls back to qemu for emulation, or otherwise fault/ignore them (depending on whether the board in question has the Security/Virtualisation extensions, and on what the firmware is supposed to be present).

I don't understand this paragraph, sorry.

-Christoffer

#6 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

In reply to Marc Zyngier
Quoted 17 lines · Marc Zyngier

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 68

Quoted 11 lines · Andre Przywara
arch/arm/kvm/handle_exit.c +11 -0
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)
+{
+     unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+     if (fn_nr == 0x102) {
+             kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+             return 1;
+     }
+
+     return 0;
+}

And what if I run mach-foo which uses r12 to request bar services from secure mode? Is it safe to ignore it? We need something much better than just testing random registers to guess what the guest wants.

Definitely. This needs to be addressed via the kernel providing some mechanism so that userspace and/or a KVM-specific bit of 'firmware' running in the guest VM can handle the SMC calls the guest tries to make, because it's totally board specific.

-- PMM

#7 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 21 lines · Peter Maydell
Quoted 11 lines · Marc Zyngier
Quoted 9 lines · Andre Przywara
arch/arm/kvm/handle_exit.c +9 -0
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)
+{
+     unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+     if (fn_nr == 0x102) {
+             kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+             return 1;
+     }
+
Quoted 8 lines

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 68

Quoted 2 lines
arch/arm/kvm/handle_exit.c +2 -0
+     return 0;
+}

And what if I run mach-foo which uses r12 to request bar services from
secure mode? Is it safe to ignore it? We need something much better than
just testing random registers to guess what the guest wants.

Quoted 5 lines

Definitely. This needs to be addressed via the kernel providing some mechanism so that userspace and/or a KVM-specific bit of 'firmware' running in the guest VM can handle the SMC calls the guest tries to make, because it's totally board specific.

Right. We're in violent agreement here.

What I can imagine is some kind of feature bit that would cause an exit all the way to userspace, letting QEMU handle the call.

That would be simple enough to implement, I believe. At least on the kernel side.

         M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
#8 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 28 lines · Marc Zyngier
Quoted 21 lines · Peter Maydell
Quoted 11 lines · Marc Zyngier
Quoted 9 lines · Andre Przywara
arch/arm/kvm/handle_exit.c +9 -0
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)
+{
+     unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+     if (fn_nr == 0x102) {
+             kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+             return 1;
+     }
+
Quoted 8 lines

arch/arm/kvm/handle_exit.c static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 68

Quoted 2 lines
arch/arm/kvm/handle_exit.c +2 -0
+     return 0;
+}

And what if I run mach-foo which uses r12 to request bar services from
secure mode? Is it safe to ignore it? We need something much better than
just testing random registers to guess what the guest wants.

Quoted 5 lines

Definitely. This needs to be addressed via the kernel providing some mechanism so that userspace and/or a KVM-specific bit of 'firmware' running in the guest VM can handle the SMC calls the guest tries to make, because it's totally board specific.

Quoted 8 lines

Right. We're in violent agreement here.

What I can imagine is some kind of feature bit that would cause an exit all the way to userspace, letting QEMU handle the call.

That would be simple enough to implement, I believe. At least on the kernel side.

How would we distinguish between a PSCI call that the kernel should support and a call to secure firmware that needs to be forwarded to QEMU? Is this simply a binary config at VM creation time?

-Christoffer

#9 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 3 lines · Christoffer Dall

How would we distinguish between a PSCI call that the kernel should support and a call to secure firmware that needs to be forwarded to QEMU? Is this simply a binary config at VM creation time?

Kernel PSCI is always HVC (right?) so you could just say that HVC is the kernel's business and SMC is the guest firmware's.

If we make the kernel just restart the guest inside its firmware blob without reflecting the SMC out to userspace are we going to regret it later?

-- PMM

#10 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 9 lines · Peter Maydell
Quoted 3 lines · Christoffer Dall

How would we distinguish between a PSCI call that the kernel should support and a call to secure firmware that needs to be forwarded to QEMU? Is this simply a binary config at VM creation time?

Kernel PSCI is always HVC (right?) so you could just say that HVC is the kernel's business and SMC is the guest firmware's.

As I understand it, current implementations rely on info from the DT and guests are therefore told only to use HVCs, but the spec allows both an HVC and an SMC as the conduit (unless I read this wrong), so I think it's quite possible that we'll end up supporting something that needs make PSCI calls via SMC. On the other hand, if QEMU can make the distinction and do everything that the kernel would otherwise be able to do to handle the PSCI, then we can still just let QEMU handle the whole thing.

(feel free to replace QEMU with "user space" in the above)

Quoted 4 lines

If we make the kernel just restart the guest inside its firmware blob without reflecting the SMC out to userspace are we going to regret it later?

Are you suggesting that we'd load the secure firmware inside the guest in a separate address space somehow and just let it execute the binary? That won't work without considerable emulation efforts in the kernel to support the privileged operations right? What if the secure firmware does something SoC-specific that KVM will never know about, but QEMU would, then there's still the need for some 'backdoor' out to QEMU.

Did I misunderstand your point here?

I would imagine that at most QEMU can tell KVM to set SMC calls to exactly one of these modes:
1) Handle SMCs as undefined
2) Handle SMCs as PSCI
3) Forward all SMCs to me

And that would more or less be the end of it as far as KVM is involved...

-Christoffer

#11 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 11 lines · Christoffer Dall
Quoted 3 lines · Peter Maydell

If we make the kernel just restart the guest inside its firmware blob without reflecting the SMC out to userspace are we going to regret it later?

Are you suggesting that we'd load the secure firmware inside the guest in a separate address space somehow and just let it execute the binary? That won't work without considerable emulation efforts in the kernel to support the privileged operations right? What if the secure firmware does something SoC-specific that KVM will never know about, but QEMU would, then there's still the need for some 'backdoor' out to QEMU.

No, the suggestion is that the 'firmware' blob is a specifically written thing to work with QEMU/KVM, so it supports the right entry points but is written to work within a slightly odd environment where:
* the kernel supports an emulated MVBAR
* SMC causes us to redirect the guest so it enters as per the MVBAR, but in EL1, not EL3
* the "firmware" blob does whatever is necessary before returning from the SMC

There is obviously no insulation between the guest kernel and the firmware blob in this scenario, but if we're not actually trying to emulate secure mode, just deal somehow with a handful of API calls, that should be fine.

(This is an idea I've floated before, based partly on what the current QEMU OMAP3 emulation does. The advantage from my point of view is that it keeps the details of what the SMC entrypoints are supposed to do out of QEMU and in the board-specific blob.)

-- PMM

#12 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 13 lines · Peter Maydell
Quoted 11 lines · Christoffer Dall
Quoted 3 lines · Peter Maydell

If we make the kernel just restart the guest inside its firmware blob without reflecting the SMC out to userspace are we going to regret it later?

Are you suggesting that we'd load the secure firmware inside the guest in a separate address space somehow and just let it execute the binary? That won't work without considerable emulation efforts in the kernel to support the privileged operations right? What if the secure firmware does something SoC-specific that KVM will never know about, but QEMU would, then there's still the need for some 'backdoor' out to QEMU.

Quoted 9 lines

No, the suggestion is that the 'firmware' blob is a specifically written thing to work with QEMU/KVM, so it supports the right entry points but is written to work within a slightly odd environment where:
* the kernel supports an emulated MVBAR
* SMC causes us to redirect the guest so it enters as per the MVBAR, but in EL1, not EL3
* the "firmware" blob does whatever is necessary before returning from the SMC

ok, I see.

Quoted 5 lines

There is obviously no insulation between the guest kernel and the firmware blob in this scenario, but if we're not actually trying to emulate secure mode, just deal somehow with a handful of API calls, that should be fine.

Well, we could load the firmware in memory that we only ever map in Stage-2 mappings when we execute the special firmware blob, which would at least prevent the guest kernel from mocking with the firmware code. Sort of like an emulated secure physical memory region.

Quoted 6 lines

(This is an idea I've floated before, based partly on what the current QEMU OMAP3 emulation does. The advantage from my point of view is that it keeps the details of what the SMC entrypoints are supposed to do out of QEMU and in the board-specific blob.)

The clear advantage is that it keeps the code out of QEMU. The downside is a potentially more complicated development environment (you're sort writing a small OS here, and you can't really reuse existing secure firmwares because the environment is special, right?) where having the emulation simply integrated in QEMU makes it a nice debuggable piece of user space code.

Hmmm....

-Christoffer

#13 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

In reply to Andre Przywara
Quoted 4 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

Quoted 8 lines

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

---
 arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

At least for highbank, we can fix this in the kernel:

(quoted diff) +0 -0

b/arch/arm/mach-highbank/highbank.c
index 1894dcf..b5d0375 100644

arch/arm/mach-highbank/highbank.c +4 -2
@@ -77,8 +77,10 @@ static void __init highbank_init_irq(void)
 {
        irqchip_init();
 
-       if (of_find_compatible_node(NULL, NULL, "arm,cortex-a9"))
-               highbank_scu_map_io();
+       if (!of_find_compatible_node(NULL, NULL, "arm,cortex-a9"))
+               return;
+
+       highbank_scu_map_io();
 
 #ifdef CONFIG_CACHE_L2X0
        /* Enable PL310 L2 Cache controller */
Quoted 42 lines
arch/arm/kvm/handle_exit.c +20 -1
index df4c82d..2cbe6a0 100644
@@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
        return 1;
 }
 
+/*
+ * OMAP4 and Highbank machines do a SMC call to handle the L2 cache
+ * controller. They put 0x102 in r12 to request this functionality.
+ * This is not needed on A15s, so we can safely ignore it in KVM guests.
+ */
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)
+{
+       unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+       if (fn_nr == 0x102) {
+               kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+               return 1;
+       }
+
+       return 0;
+}
+
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
+       if (kvm_ignore_l2x0_call(vcpu))
+               return 1;
+
        kvm_inject_undefined(vcpu);
        return 1;
 }
--

1.7.12.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#14 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 10 lines · Rob Herring
Quoted 8 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

Quoted 6 lines
Quoted 4 lines
---
  arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
  1 file changed, 20 insertions(+)

At least for highbank, we can fix this in the kernel:

Yes, and we should do. But that won't fix older guest kernels, say Ubuntu 12.10 or the like. And I think this is a use case for virtualization, so we need both, guest and host fix.

Regards,
Andre

Quoted 62 lines
arch/arm/mach-highbank/highbank.c +4 -2
index 1894dcf..b5d0375 100644
@@ -77,8 +77,10 @@ static void __init highbank_init_irq(void)
  {
         irqchip_init();
 
-       if (of_find_compatible_node(NULL, NULL, "arm,cortex-a9"))
-               highbank_scu_map_io();
+       if (!of_find_compatible_node(NULL, NULL, "arm,cortex-a9"))
+               return;
+
+       highbank_scu_map_io();
 
  #ifdef CONFIG_CACHE_L2X0
         /* Enable PL310 L2 Cache controller */
Quoted 43 lines
arch/arm/kvm/handle_exit.c +20 -1
index df4c82d..2cbe6a0 100644
@@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
         return 1;
  }
 
+/*
+ * OMAP4 and Highbank machines do a SMC call to handle the L2 cache
+ * controller. They put 0x102 in r12 to request this functionality.
+ * This is not needed on A15s, so we can safely ignore it in KVM guests.
+ */
+static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu)
+{
+       unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0);
+
+       if (fn_nr == 0x102) {
+               kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
+               return 1;
+       }
+
+       return 0;
+}
+
  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
  {
+       if (kvm_ignore_l2x0_call(vcpu))
+               return 1;
+
         kvm_inject_undefined(vcpu);
         return 1;
  }
--

1.7.12.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#15 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 16 lines · Andre Przywara
Quoted 15 lines · Rob Herring
Quoted 12 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

---
 arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
Quoted 6 lines
Quoted 1 line

At least for highbank, we can fix this in the kernel:

Yes, and we should do. But that won't fix older guest kernels, say Ubuntu 12.10 or the like. And I think this is a use case for virtualization, so we need both, guest and host fix.

Agreed, but we need a more generic solution for the secure call handling. I've created a backlog item in Linaro's JIRA (CARD-801) for this work, let's see how quickly we can get it approved and put on the roadmap.

-Christoffer

#16 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 19 lines · Christoffer Dall
Quoted 18 lines · Andre Przywara
Quoted 16 lines · Rob Herring
Quoted 4 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

Quoted 8 lines

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

---
  arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
  1 file changed, 20 insertions(+)

At least for highbank, we can fix this in the kernel:

Quoted 8 lines
Quoted 4 lines

Yes, and we should do. But that won't fix older guest kernels, say Ubuntu 12.10 or the like. And I think this is a use case for virtualization, so we need both, guest and host fix.

Agreed, but we need a more generic solution for the secure call handling. I've created a backlog item in Linaro's JIRA (CARD-801) for this work, let's see how quickly we can get it approved and put on the roadmap.

So I did some research already, I am not sure we can wait until Jira is ready ;-)
I'd opt for something like this:
1. Allow userland to let the kernel ignore all smc's. That's low overhead, easy to implement and would cover Highbank and Broadcom, which do only L2 cache controller handling via smc.
2. Think about how to handle TI Keystone and Qualcomm MSM, which do secondary cores bringup via smc's. Do we need to support this or can we demand PSCI support? If I got this correctly, a PSCI node in the DTB overrides any platform smp_ops, so injecting PSCI should avoid those smc's on those two platforms.
3. Agree on whether we support PSCI via smc. I think we abandoned this with 24a7f67 (ARM: KVM: Don't handle PSCI calls via SMC), so do we really want to re-introduce it?
4. Dig through all this OMAP smc code to decide what we really want to emulate and whether we need to: Maybe we can safely ignore this since it is for OMAP4 with A9s or lower only.
If there is a need to emulate, fold this into one ioctl which also enables the ignore-all case.

I am not sure whether it is a wise decision to pull _all_ SMC handling unconditionally into userland, since that would separate the source of the SMCs (the kernel) and their emulation.

Regards,
Andre.

#17 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

Quoted 24 lines · Andre Przywara
Quoted 23 lines · Christoffer Dall
Quoted 22 lines · Andre Przywara
Quoted 10 lines · Rob Herring
Quoted 8 lines · Andre Przywara

Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) will trigger SMCs to handle the L2 cache controller (PL310). This will currently inject #UNDEFs and eventually stop the guest.

We don't need explicit L2 cache controller handling on A15s anymore, so it is safe to simply ignore these calls and proceed with the next instruction.

Quoted 6 lines
Quoted 4 lines
---
 arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

At least for highbank, we can fix this in the kernel:

Yes, and we should do. But that won't fix older guest kernels, say Ubuntu 12.10 or the like. And I think this is a use case for virtualization, so we need both, guest and host fix.

Quoted 7 lines
Quoted 4 lines

Agreed, but we need a more generic solution for the secure call handling. I've created a backlog item in Linaro's JIRA (CARD-801) for this work, let's see how quickly we can get it approved and put on the roadmap.

So I did some research already, I am not sure we can wait until Jira is ready ;-)

This is not a quick-and-dirty sort of fix, but something we need to fix properly.

Quoted 1 line

I'd opt for something like this:

I'm confused: are you giving us a list of choices or are you listing the work items that you think we need to do?

Quoted 3 lines

1. Allow userland to let the kernel ignore all smc's. That's low overhead, easy to implement and would cover Highbank and Broadcom, which do only L2 cache controller handling via smc.

Hmmm, it's low overhead, but we still need to come up with an API for this (perhaps it's a vcpu feature), but might be abused. Again, I think this would be solved in a more generic approach - see Peter's suggestion on the discussion of the patch.

Quoted 5 lines

2. Think about how to handle TI Keystone and Qualcomm MSM, which do secondary cores bringup via smc's. Do we need to support this or can we demand PSCI support? If I got this correctly, a PSCI node in the DTB overrides any platform smp_ops, so injecting PSCI should avoid those smc's on those two platforms.

Yes, and this is only relevant if you want to run those kinds of guests. If the dominating use case is mach-virt, isn't all of this sort of moot?

Quoted 3 lines

3. Agree on whether we support PSCI via smc. I think we abandoned this with 24a7f67 (ARM: KVM: Don't handle PSCI calls via SMC), so do we really want to re-introduce it?

I don't see any reason to introduce that right now, but I certainly don't want to build an infrastructure around SMC handling that makes it hard or impossible to handle guests that do PSCI calls.

Quoted 6 lines

4. Dig through all this OMAP smc code to decide what we really want to emulate and whether we need to: Maybe we can safely ignore this since it is for OMAP4 with A9s or lower only.
If there is a need to emulate, fold this into one ioctl which also enables the ignore-all case.

I don't want to dig through any OMAP smc code, no. Why are we talking about supporting these arcane guest kernels? Is this not only a problem for a few recent kernels that have multiplatform support and some other combination of configs enabled and disregard info in the DT?

Quoted 4 lines

I am not sure whether it is a wise decision to pull _all_ SMC handling unconditionally into userland, since that would separate the source of the SMCs (the kernel) and their emulation.

Hmmm... The source of the SMCs is the guest software, and the only reasons to keep it in the host kernel would be performance or the requirement to perform privileged operation on the host cpu to emulate the smc. The latter may be the case, but I want to get a clearer picture of which cases we're trying to support here exactly, and why.

-Christoffer

#18 lore raw

PATCH 1/1 KVM: ARM: ignore guest L2 cache control SMCs on Highbank and OMAP

In reply to Christoffer Dall
Quoted 4 lines · Christoffer Dall

Agreed, but we need a more generic solution for the secure call handling. I've created a backlog item in Linaro's JIRA (CARD-801) for this work, let's see how quickly we can get it approved and put on the roadmap.

FYI, here's my sketch from last year of what I had in mind:
http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg03012.html

-- PMM