[PATCH stable-5.7] KVM: arm64: Synchronize sysreg state on injecting an AArch32 exception

[PATCH stable-5.7] KVM: arm64: Synchronize sysreg state on injecting an AArch32 exception

[Marc Zyngier]

commit 0370964dd3ff7d3d406f292cb443a927952cbd05 upstream

On a VHE system, the EL1 state is left in the CPU most of the time,
and only syncronized back to memory when vcpu_put() is called (most
of the time on preemption).

Which means that when injecting an exception, we'd better have a way
to either:
(1) write directly to the EL1 sysregs
(2) synchronize the state back to memory, and do the changes there

For an AArch64, we already do (1), so we are safe. Unfortunately,
doing the same thing for AArch32 would be pretty invasive. Instead,
we can easily implement (2) by calling the put/load architectural
backends, and keep preemption disabled. We can then reload the
state back into EL1.

Cc: stable@vger.kernel.org
Reported-by: James Morse <james.morse@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 virt/kvm/arm/aarch32.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/virt/kvm/arm/aarch32.c b/virt/kvm/arm/aarch32.c
index 0a356aa91aa1..40a62a99fbf8 100644
--- a/virt/kvm/arm/aarch32.c
+++ b/virt/kvm/arm/aarch32.c
@@ -33,6 +33,26 @@ static const u8 return_offsets[8][2] = {
 	[7] = { 4, 4 },		/* FIQ, unused */
 };
 
+static bool pre_fault_synchronize(struct kvm_vcpu *vcpu)
+{
+	preempt_disable();
+	if (vcpu->arch.sysregs_loaded_on_cpu) {
+		kvm_arch_vcpu_put(vcpu);
+		return true;
+	}
+
+	preempt_enable();
+	return false;
+}
+
+static void post_fault_synchronize(struct kvm_vcpu *vcpu, bool loaded)
+{
+	if (loaded) {
+		kvm_arch_vcpu_load(vcpu, smp_processor_id());
+		preempt_enable();
+	}
+}
+
 /*
  * When an exception is taken, most CPSR fields are left unchanged in the
  * handler. However, some are explicitly overridden (e.g. M[4:0]).
@@ -155,7 +175,10 @@ static void prepare_fault32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
 
 void kvm_inject_undef32(struct kvm_vcpu *vcpu)
 {
+	bool loaded = pre_fault_synchronize(vcpu);
+
 	prepare_fault32(vcpu, PSR_AA32_MODE_UND, 4);
+	post_fault_synchronize(vcpu, loaded);
 }
 
 /*
@@ -168,6 +191,9 @@ static void inject_abt32(struct kvm_vcpu *vcpu, bool is_pabt,
 	u32 vect_offset;
 	u32 *far, *fsr;
 	bool is_lpae;
+	bool loaded;
+
+	loaded = pre_fault_synchronize(vcpu);
 
 	if (is_pabt) {
 		vect_offset = 12;
@@ -191,6 +217,8 @@ static void inject_abt32(struct kvm_vcpu *vcpu, bool is_pabt,
 		/* no need to shuffle FS[4] into DFSR[10] as its 0 */
 		*fsr = DFSR_FSC_EXTABT_nLPAE;
 	}
+
+	post_fault_synchronize(vcpu, loaded);
 }
 
 void kvm_inject_dabt32(struct kvm_vcpu *vcpu, unsigned long addr)
-- 
2.27.0

Re: [PATCH stable-5.7] KVM: arm64: Synchronize sysreg state on injecting an AArch32 exception

[Greg KH]

On Tue, Jun 16, 2020 at 01:52:00PM +0100, Marc Zyngier wrote:
> commit 0370964dd3ff7d3d406f292cb443a927952cbd05 upstream
> 
> On a VHE system, the EL1 state is left in the CPU most of the time,
> and only syncronized back to memory when vcpu_put() is called (most
> of the time on preemption).
> 
> Which means that when injecting an exception, we'd better have a way
> to either:
> (1) write directly to the EL1 sysregs
> (2) synchronize the state back to memory, and do the changes there
> 
> For an AArch64, we already do (1), so we are safe. Unfortunately,
> doing the same thing for AArch32 would be pretty invasive. Instead,
> we can easily implement (2) by calling the put/load architectural
> backends, and keep preemption disabled. We can then reload the
> state back into EL1.
> 
> Cc: stable@vger.kernel.org
> Reported-by: James Morse <james.morse@arm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  virt/kvm/arm/aarch32.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)

Thanks for this, and the other backport.  Queued up.

greg k-h

Re: [PATCH stable-5.7] KVM: arm64: Synchronize sysreg state on injecting an AArch32 exception

[Marc Zyngier]

Hi Greg,

On 2020-06-16 14:09, Greg KH wrote:
> On Tue, Jun 16, 2020 at 01:52:00PM +0100, Marc Zyngier wrote:
>> commit 0370964dd3ff7d3d406f292cb443a927952cbd05 upstream
>> 
>> On a VHE system, the EL1 state is left in the CPU most of the time,
>> and only syncronized back to memory when vcpu_put() is called (most
>> of the time on preemption).
>> 
>> Which means that when injecting an exception, we'd better have a way
>> to either:
>> (1) write directly to the EL1 sysregs
>> (2) synchronize the state back to memory, and do the changes there
>> 
>> For an AArch64, we already do (1), so we are safe. Unfortunately,
>> doing the same thing for AArch32 would be pretty invasive. Instead,
>> we can easily implement (2) by calling the put/load architectural
>> backends, and keep preemption disabled. We can then reload the
>> state back into EL1.
>> 
>> Cc: stable@vger.kernel.org
>> Reported-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>  virt/kvm/arm/aarch32.c | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
> 
> Thanks for this, and the other backport.  Queued up.

You seem to have queued the same patches for 5.4 and 5.6.
This will break 32bit ARM (the patch applies nicely, but it will blow up 
at compile time).

I'll have the corresponding backports later today, once I've finished 
testing them.

Thanks,

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

Re: [PATCH stable-5.7] KVM: arm64: Synchronize sysreg state on injecting an AArch32 exception

[Marc Zyngier]

On 2020-06-16 14:19, Marc Zyngier wrote:
> Hi Greg,
> 
> On 2020-06-16 14:09, Greg KH wrote:
>> On Tue, Jun 16, 2020 at 01:52:00PM +0100, Marc Zyngier wrote:
>>> commit 0370964dd3ff7d3d406f292cb443a927952cbd05 upstream
>>> 
>>> On a VHE system, the EL1 state is left in the CPU most of the time,
>>> and only syncronized back to memory when vcpu_put() is called (most
>>> of the time on preemption).
>>> 
>>> Which means that when injecting an exception, we'd better have a way
>>> to either:
>>> (1) write directly to the EL1 sysregs
>>> (2) synchronize the state back to memory, and do the changes there
>>> 
>>> For an AArch64, we already do (1), so we are safe. Unfortunately,
>>> doing the same thing for AArch32 would be pretty invasive. Instead,
>>> we can easily implement (2) by calling the put/load architectural
>>> backends, and keep preemption disabled. We can then reload the
>>> state back into EL1.
>>> 
>>> Cc: stable@vger.kernel.org
>>> Reported-by: James Morse <james.morse@arm.com>
>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>> ---
>>>  virt/kvm/arm/aarch32.c | 28 ++++++++++++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>> 
>> Thanks for this, and the other backport.  Queued up.
> 
> You seem to have queued the same patches for 5.4 and 5.6.

Huh, and 4.19 as well. Gahh...

> This will break 32bit ARM (the patch applies nicely, but it will blow
> up at compile time).
> 
> I'll have the corresponding backports later today, once I've finished
> testing them.
> 
> Thanks,
> 
>         M.

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

Re: [PATCH stable-5.7] KVM: arm64: Synchronize sysreg state on injecting an AArch32 exception

[Greg KH]

On Tue, Jun 16, 2020 at 02:21:11PM +0100, Marc Zyngier wrote:
> On 2020-06-16 14:19, Marc Zyngier wrote:
> > Hi Greg,
> > 
> > On 2020-06-16 14:09, Greg KH wrote:
> > > On Tue, Jun 16, 2020 at 01:52:00PM +0100, Marc Zyngier wrote:
> > > > commit 0370964dd3ff7d3d406f292cb443a927952cbd05 upstream
> > > > 
> > > > On a VHE system, the EL1 state is left in the CPU most of the time,
> > > > and only syncronized back to memory when vcpu_put() is called (most
> > > > of the time on preemption).
> > > > 
> > > > Which means that when injecting an exception, we'd better have a way
> > > > to either:
> > > > (1) write directly to the EL1 sysregs
> > > > (2) synchronize the state back to memory, and do the changes there
> > > > 
> > > > For an AArch64, we already do (1), so we are safe. Unfortunately,
> > > > doing the same thing for AArch32 would be pretty invasive. Instead,
> > > > we can easily implement (2) by calling the put/load architectural
> > > > backends, and keep preemption disabled. We can then reload the
> > > > state back into EL1.
> > > > 
> > > > Cc: stable@vger.kernel.org
> > > > Reported-by: James Morse <james.morse@arm.com>
> > > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > > ---
> > > >  virt/kvm/arm/aarch32.c | 28 ++++++++++++++++++++++++++++
> > > >  1 file changed, 28 insertions(+)
> > > 
> > > Thanks for this, and the other backport.  Queued up.
> > 
> > You seem to have queued the same patches for 5.4 and 5.6.
> 
> Huh, and 4.19 as well. Gahh...

Oops, sorry, my fault.

I'll go drop them now, thanks.

greg k-h