Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-10 Thread Baicar, Tyler

Hello James,


On 3/8/2017 9:09 AM, James Morse wrote:

On 07/03/17 17:58, Baicar, Tyler wrote:

On 3/7/2017 4:48 AM, James Morse wrote:

On 06/03/17 20:45, Tyler Baicar wrote:

Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..f3608c9 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
struct kvm_run *run)
 /* Check the stage-2 fault is trans. fault or write fault */
   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-fault_status != FSC_ACCESS) {
+
+/* The host kernel will handle the synchronous external abort. There
+ * is no need to pass the error into the guest.
+ */
+if (is_abort_synchronous(fault_status)) {
+if(handle_guest_sea((unsigned long)fault_ipa,
+kvm_vcpu_get_hsr(vcpu))) {

... Looking further up in this function:

 is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
 if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
 kvm_inject_vabt(vcpu);
 return 1;
 }

... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).

We need to do the handle_guest_sea() before this code.

kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.

I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.

Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.

Yes, I'll move the handle_guest_sea() call above this. My testing didn't call
into that if statement for some reason...it made it to the handle_guest_sea()
call successfully.

I guess you're not using data aborts for testing then.



If there is no work for the GHES code to do it will return and could still make
the kvm_inject_vabt() call. It will also return and do that same thing if the
error was not fatal in GHES...would that be an issue?

We might inject a superfluous SError Interrupt in that case.

For memory errors we may do the whole unmapping and signalling thing to handle
the fault. For recoverable faults, QEMU can generate its own CPER records for
the guest and do the work to notify the guest. Everything looks fine until the
guest gets the extra SError interrupt.

If there is firmware-first RAS data, we should skip the injected SError
Interrupt as the host's RAS code should choose what to do. If this Synchronous
External Abort was nothing to do with RAS, we should inject the SError interrupt
as it's the existing behaviour, and not all platforms will have this Synchronous
External Abort mechanism.

x86's APEI NMI needs to know if the NMI was due to RAS, so we can probably
borrow the same trick. ghes_notify_nmi() calls ghes_read_estatus() for each
struct ghes. If they all return an error, there was no work to do.


(I assume firmware will only generate one of these at a time, so there is no
risk of one list-walker processing two entries, then the second finding nothing
to do?)

Okay, I will add in this functionality to avoid injecting an SEI if 
there is actually a GHES error populated.


Firmware that supports the new specs should only generate one of these 
at a time, it will wait for the ack from kernel before sending a second 
error (patch 1 of this series).


Thanks,
Tyler




+kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx
ESR_EL2=%#lx\n",
+kvm_vcpu_trap_get_class(vcpu),
+(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+(unsigned long)kvm_vcpu_get_hsr(vcpu));
+return -EFAULT;
+}
+} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+   fault_status != FSC_ACCESS) {
   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
   kvm_vcpu_trap_get_class(vcpu),
   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.



Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-10 Thread Baicar, Tyler

Hello James,


On 3/8/2017 9:09 AM, James Morse wrote:

On 07/03/17 17:58, Baicar, Tyler wrote:

On 3/7/2017 4:48 AM, James Morse wrote:

On 06/03/17 20:45, Tyler Baicar wrote:

Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..f3608c9 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
struct kvm_run *run)
 /* Check the stage-2 fault is trans. fault or write fault */
   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-fault_status != FSC_ACCESS) {
+
+/* The host kernel will handle the synchronous external abort. There
+ * is no need to pass the error into the guest.
+ */
+if (is_abort_synchronous(fault_status)) {
+if(handle_guest_sea((unsigned long)fault_ipa,
+kvm_vcpu_get_hsr(vcpu))) {

... Looking further up in this function:

 is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
 if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
 kvm_inject_vabt(vcpu);
 return 1;
 }

... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).

We need to do the handle_guest_sea() before this code.

kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.

I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.

Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.

Yes, I'll move the handle_guest_sea() call above this. My testing didn't call
into that if statement for some reason...it made it to the handle_guest_sea()
call successfully.

I guess you're not using data aborts for testing then.



If there is no work for the GHES code to do it will return and could still make
the kvm_inject_vabt() call. It will also return and do that same thing if the
error was not fatal in GHES...would that be an issue?

We might inject a superfluous SError Interrupt in that case.

For memory errors we may do the whole unmapping and signalling thing to handle
the fault. For recoverable faults, QEMU can generate its own CPER records for
the guest and do the work to notify the guest. Everything looks fine until the
guest gets the extra SError interrupt.

If there is firmware-first RAS data, we should skip the injected SError
Interrupt as the host's RAS code should choose what to do. If this Synchronous
External Abort was nothing to do with RAS, we should inject the SError interrupt
as it's the existing behaviour, and not all platforms will have this Synchronous
External Abort mechanism.

x86's APEI NMI needs to know if the NMI was due to RAS, so we can probably
borrow the same trick. ghes_notify_nmi() calls ghes_read_estatus() for each
struct ghes. If they all return an error, there was no work to do.


(I assume firmware will only generate one of these at a time, so there is no
risk of one list-walker processing two entries, then the second finding nothing
to do?)

Okay, I will add in this functionality to avoid injecting an SEI if 
there is actually a GHES error populated.


Firmware that supports the new specs should only generate one of these 
at a time, it will wait for the ack from kernel before sending a second 
error (patch 1 of this series).


Thanks,
Tyler




+kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx
ESR_EL2=%#lx\n",
+kvm_vcpu_trap_get_class(vcpu),
+(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+(unsigned long)kvm_vcpu_get_hsr(vcpu));
+return -EFAULT;
+}
+} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+   fault_status != FSC_ACCESS) {
   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
   kvm_vcpu_trap_get_class(vcpu),
   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.



Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-08 Thread James Morse
On 07/03/17 17:58, Baicar, Tyler wrote:
> On 3/7/2017 4:48 AM, James Morse wrote:
>> On 06/03/17 20:45, Tyler Baicar wrote:
>>> Currently external aborts are unsupported by the guest abort
>>> handling. Add handling for SEAs so that the host kernel reports
>>> SEAs which occur in the guest kernel.

>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>>> index a5265ed..f3608c9 100644
>>> --- a/arch/arm/kvm/mmu.c
>>> +++ b/arch/arm/kvm/mmu.c
>>> @@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
>>> struct kvm_run *run)
>>> /* Check the stage-2 fault is trans. fault or write fault */
>>>   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
>>> -if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>>> -fault_status != FSC_ACCESS) {
>>> +
>>> +/* The host kernel will handle the synchronous external abort. There
>>> + * is no need to pass the error into the guest.
>>> + */
>>> +if (is_abort_synchronous(fault_status)) {
>>> +if(handle_guest_sea((unsigned long)fault_ipa,
>>> +kvm_vcpu_get_hsr(vcpu))) {
>>
>> ... Looking further up in this function:
>>> is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
>>> if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
>>> kvm_inject_vabt(vcpu);
>>> return 1;
>>> }
>> ... so external data aborts will have already been 'claimed' by kvm and dealt
>> with, and we already have a helper for spotting external aborts. (sorry I 
>> didn't
>> spot it earlier).
>>
>> We need to do the handle_guest_sea() before this code.
>>
>> kvm_inject_vabt() makes an SError interrupt pending for the guest. This 
>> makes a
>> synchronous error asynchronous as the guest may have SError interrupts 
>> masked.
>>
>> I guess this was the best that could be done at the time of (4055710baca8
>> "arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), 
>> but
>> in the light of this firmware-first handling, I'm not sure its the right 
>> thing
>> to do.
>>
>> Is it possible for handle_guest_sea() to return whether it actually found any
>> work to do? If there was none I think we should keep this kvm_inject_vabt() 
>> as
>> it is the existing behaviour.

> Yes, I'll move the handle_guest_sea() call above this. My testing didn't call
> into that if statement for some reason...it made it to the handle_guest_sea()
> call successfully.

I guess you're not using data aborts for testing then.


> If there is no work for the GHES code to do it will return and could still 
> make
> the kvm_inject_vabt() call. It will also return and do that same thing if the
> error was not fatal in GHES...would that be an issue?

We might inject a superfluous SError Interrupt in that case.

For memory errors we may do the whole unmapping and signalling thing to handle
the fault. For recoverable faults, QEMU can generate its own CPER records for
the guest and do the work to notify the guest. Everything looks fine until the
guest gets the extra SError interrupt.

If there is firmware-first RAS data, we should skip the injected SError
Interrupt as the host's RAS code should choose what to do. If this Synchronous
External Abort was nothing to do with RAS, we should inject the SError interrupt
as it's the existing behaviour, and not all platforms will have this Synchronous
External Abort mechanism.

x86's APEI NMI needs to know if the NMI was due to RAS, so we can probably
borrow the same trick. ghes_notify_nmi() calls ghes_read_estatus() for each
struct ghes. If they all return an error, there was no work to do.


(I assume firmware will only generate one of these at a time, so there is no
risk of one list-walker processing two entries, then the second finding nothing
to do?)


Thanks,

James


>>> +kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx
>>> ESR_EL2=%#lx\n",
>>> +kvm_vcpu_trap_get_class(vcpu),
>>> +(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>>> +(unsigned long)kvm_vcpu_get_hsr(vcpu));
>>> +return -EFAULT;
>>> +}
>>> +} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>>> +   fault_status != FSC_ACCESS) {
>>>   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>>>   kvm_vcpu_trap_get_class(vcpu),
>>>   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>>



Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-08 Thread James Morse
On 07/03/17 17:58, Baicar, Tyler wrote:
> On 3/7/2017 4:48 AM, James Morse wrote:
>> On 06/03/17 20:45, Tyler Baicar wrote:
>>> Currently external aborts are unsupported by the guest abort
>>> handling. Add handling for SEAs so that the host kernel reports
>>> SEAs which occur in the guest kernel.

>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>>> index a5265ed..f3608c9 100644
>>> --- a/arch/arm/kvm/mmu.c
>>> +++ b/arch/arm/kvm/mmu.c
>>> @@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu,
>>> struct kvm_run *run)
>>> /* Check the stage-2 fault is trans. fault or write fault */
>>>   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
>>> -if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>>> -fault_status != FSC_ACCESS) {
>>> +
>>> +/* The host kernel will handle the synchronous external abort. There
>>> + * is no need to pass the error into the guest.
>>> + */
>>> +if (is_abort_synchronous(fault_status)) {
>>> +if(handle_guest_sea((unsigned long)fault_ipa,
>>> +kvm_vcpu_get_hsr(vcpu))) {
>>
>> ... Looking further up in this function:
>>> is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
>>> if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
>>> kvm_inject_vabt(vcpu);
>>> return 1;
>>> }
>> ... so external data aborts will have already been 'claimed' by kvm and dealt
>> with, and we already have a helper for spotting external aborts. (sorry I 
>> didn't
>> spot it earlier).
>>
>> We need to do the handle_guest_sea() before this code.
>>
>> kvm_inject_vabt() makes an SError interrupt pending for the guest. This 
>> makes a
>> synchronous error asynchronous as the guest may have SError interrupts 
>> masked.
>>
>> I guess this was the best that could be done at the time of (4055710baca8
>> "arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), 
>> but
>> in the light of this firmware-first handling, I'm not sure its the right 
>> thing
>> to do.
>>
>> Is it possible for handle_guest_sea() to return whether it actually found any
>> work to do? If there was none I think we should keep this kvm_inject_vabt() 
>> as
>> it is the existing behaviour.

> Yes, I'll move the handle_guest_sea() call above this. My testing didn't call
> into that if statement for some reason...it made it to the handle_guest_sea()
> call successfully.

I guess you're not using data aborts for testing then.


> If there is no work for the GHES code to do it will return and could still 
> make
> the kvm_inject_vabt() call. It will also return and do that same thing if the
> error was not fatal in GHES...would that be an issue?

We might inject a superfluous SError Interrupt in that case.

For memory errors we may do the whole unmapping and signalling thing to handle
the fault. For recoverable faults, QEMU can generate its own CPER records for
the guest and do the work to notify the guest. Everything looks fine until the
guest gets the extra SError interrupt.

If there is firmware-first RAS data, we should skip the injected SError
Interrupt as the host's RAS code should choose what to do. If this Synchronous
External Abort was nothing to do with RAS, we should inject the SError interrupt
as it's the existing behaviour, and not all platforms will have this Synchronous
External Abort mechanism.

x86's APEI NMI needs to know if the NMI was due to RAS, so we can probably
borrow the same trick. ghes_notify_nmi() calls ghes_read_estatus() for each
struct ghes. If they all return an error, there was no work to do.


(I assume firmware will only generate one of these at a time, so there is no
risk of one list-walker processing two entries, then the second finding nothing
to do?)


Thanks,

James


>>> +kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx
>>> ESR_EL2=%#lx\n",
>>> +kvm_vcpu_trap_get_class(vcpu),
>>> +(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>>> +(unsigned long)kvm_vcpu_get_hsr(vcpu));
>>> +return -EFAULT;
>>> +}
>>> +} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>>> +   fault_status != FSC_ACCESS) {
>>>   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>>>   kvm_vcpu_trap_get_class(vcpu),
>>>   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>>



Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-07 Thread Baicar, Tyler

Hello James,


On 3/7/2017 4:48 AM, James Morse wrote:

On 06/03/17 20:45, Tyler Baicar wrote:

Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.
diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index e22089f..a1a3dff 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -187,6 +187,16 @@
  #define FSC_FAULT (0x04)
  #define FSC_ACCESS(0x08)
  #define FSC_PERM  (0x0c)
+#define FSC_SEA(0x10)
+#define FSC_SEA_TTW0   (0x14)
+#define FSC_SEA_TTW1   (0x15)
+#define FSC_SEA_TTW2   (0x16)
+#define FSC_SEA_TTW3   (0x17)
+#define FSC_SECC   (0x18)
+#define FSC_SECC_TTW0  (0x1c)

aarch32 doesn't have either of these 'TW0' values, it's an unused encoding.
(However ...)


+#define FSC_SECC_TTW1  (0x1d)
+#define FSC_SECC_TTW2  (0x1e)
+#define FSC_SECC_TTW3  (0x1f)
  
  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */

  #define HPFAR_MASK(~0xf)
  #endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..f3608c9 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
  
  	/* Check the stage-2 fault is trans. fault or write fault */

fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-   if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-   fault_status != FSC_ACCESS) {
+
+   /* The host kernel will handle the synchronous external abort. There
+* is no need to pass the error into the guest.
+*/
+   if (is_abort_synchronous(fault_status)) {
+   if(handle_guest_sea((unsigned long)fault_ipa,
+   kvm_vcpu_get_hsr(vcpu))) {


... Looking further up in this function:

is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
kvm_inject_vabt(vcpu);
return 1;
}

... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).

We need to do the handle_guest_sea() before this code.

kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.

I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.

Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.
Yes, I'll move the handle_guest_sea() call above this. My testing didn't 
call into that if statement for some reason...it made it to the 
handle_guest_sea() call successfully.


If there is no work for the GHES code to do it will return and could 
still make the kvm_inject_vabt() call. It will also return and do that 
same thing if the error was not fatal in GHES...would that be an issue?



+   kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx 
ESR_EL2=%#lx\n",
+   kvm_vcpu_trap_get_class(vcpu),
+   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+   (unsigned long)kvm_vcpu_get_hsr(vcpu));
+   return -EFAULT;
+   }
+   } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+  fault_status != FSC_ACCESS) {
kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
kvm_vcpu_trap_get_class(vcpu),
(unsigned long)kvm_vcpu_trap_get_fault(vcpu),



diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b2d57fc..31c5171 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
  }
  
  /*

+ * Handle Synchronous External Aborts that occur in a guest kernel.
+ */
+int handle_guest_sea(unsigned long addr, unsigned int esr)
+{
+   /*
+* synchronize_rcu() will wait for nmi_exit(), so no need to
+* rcu_read_lock().
+*/

This comment has a life of its own! Given we don't always call ghes_notify_sea()
when we interrupted un-interruptable code its not always true. I think the
rcu_read_{,un}lock() should go against the list walk (so it looks like the
examples), and ditch the comment!
:) Okay I will move the rcu calls into ghes_notify_sea() and remove the 
comments.


Thanks,
Tyler



+   if(IS_ENABLED(ACPI_APEI_SEA)) {
+   rcu_read_lock();
+   ghes_notify_sea();

Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-07 Thread Baicar, Tyler

Hello James,


On 3/7/2017 4:48 AM, James Morse wrote:

On 06/03/17 20:45, Tyler Baicar wrote:

Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.
diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index e22089f..a1a3dff 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -187,6 +187,16 @@
  #define FSC_FAULT (0x04)
  #define FSC_ACCESS(0x08)
  #define FSC_PERM  (0x0c)
+#define FSC_SEA(0x10)
+#define FSC_SEA_TTW0   (0x14)
+#define FSC_SEA_TTW1   (0x15)
+#define FSC_SEA_TTW2   (0x16)
+#define FSC_SEA_TTW3   (0x17)
+#define FSC_SECC   (0x18)
+#define FSC_SECC_TTW0  (0x1c)

aarch32 doesn't have either of these 'TW0' values, it's an unused encoding.
(However ...)


+#define FSC_SECC_TTW1  (0x1d)
+#define FSC_SECC_TTW2  (0x1e)
+#define FSC_SECC_TTW3  (0x1f)
  
  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */

  #define HPFAR_MASK(~0xf)
  #endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..f3608c9 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
  
  	/* Check the stage-2 fault is trans. fault or write fault */

fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-   if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-   fault_status != FSC_ACCESS) {
+
+   /* The host kernel will handle the synchronous external abort. There
+* is no need to pass the error into the guest.
+*/
+   if (is_abort_synchronous(fault_status)) {
+   if(handle_guest_sea((unsigned long)fault_ipa,
+   kvm_vcpu_get_hsr(vcpu))) {


... Looking further up in this function:

is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
kvm_inject_vabt(vcpu);
return 1;
}

... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).

We need to do the handle_guest_sea() before this code.

kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.

I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.

Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.
Yes, I'll move the handle_guest_sea() call above this. My testing didn't 
call into that if statement for some reason...it made it to the 
handle_guest_sea() call successfully.


If there is no work for the GHES code to do it will return and could 
still make the kvm_inject_vabt() call. It will also return and do that 
same thing if the error was not fatal in GHES...would that be an issue?



+   kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx 
ESR_EL2=%#lx\n",
+   kvm_vcpu_trap_get_class(vcpu),
+   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+   (unsigned long)kvm_vcpu_get_hsr(vcpu));
+   return -EFAULT;
+   }
+   } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+  fault_status != FSC_ACCESS) {
kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
kvm_vcpu_trap_get_class(vcpu),
(unsigned long)kvm_vcpu_trap_get_fault(vcpu),



diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b2d57fc..31c5171 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
  }
  
  /*

+ * Handle Synchronous External Aborts that occur in a guest kernel.
+ */
+int handle_guest_sea(unsigned long addr, unsigned int esr)
+{
+   /*
+* synchronize_rcu() will wait for nmi_exit(), so no need to
+* rcu_read_lock().
+*/

This comment has a life of its own! Given we don't always call ghes_notify_sea()
when we interrupted un-interruptable code its not always true. I think the
rcu_read_{,un}lock() should go against the list walk (so it looks like the
examples), and ditch the comment!
:) Okay I will move the rcu calls into ghes_notify_sea() and remove the 
comments.


Thanks,
Tyler



+   if(IS_ENABLED(ACPI_APEI_SEA)) {
+   rcu_read_lock();
+   ghes_notify_sea();

Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-07 Thread James Morse
Hi Tyler,

On 06/03/17 20:45, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.

> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..a1a3dff 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,16 @@
>  #define FSC_FAULT(0x04)
>  #define FSC_ACCESS   (0x08)
>  #define FSC_PERM (0x0c)
> +#define FSC_SEA  (0x10)
> +#define FSC_SEA_TTW0 (0x14)
> +#define FSC_SEA_TTW1 (0x15)
> +#define FSC_SEA_TTW2 (0x16)
> +#define FSC_SEA_TTW3 (0x17)
> +#define FSC_SECC (0x18)
> +#define FSC_SECC_TTW0(0x1c)

aarch32 doesn't have either of these 'TW0' values, it's an unused encoding.
(However ...)

> +#define FSC_SECC_TTW1(0x1d)
> +#define FSC_SECC_TTW2(0x1e)
> +#define FSC_SECC_TTW3(0x1f)
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK   (~0xf)

>  #endif /* __ASM_ARM_SYSTEM_MISC_H */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..f3608c9 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, 
> struct kvm_run *run)
>  
>   /* Check the stage-2 fault is trans. fault or write fault */
>   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> - if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> - fault_status != FSC_ACCESS) {
> +
> + /* The host kernel will handle the synchronous external abort. There
> +  * is no need to pass the error into the guest.
> +  */
> + if (is_abort_synchronous(fault_status)) {
> + if(handle_guest_sea((unsigned long)fault_ipa,
> + kvm_vcpu_get_hsr(vcpu))) {


... Looking further up in this function:
>   is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
>   if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
>   kvm_inject_vabt(vcpu);
>   return 1;
>   }

... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).

We need to do the handle_guest_sea() before this code.

kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.

I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.

Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.


> + kvm_err("Failed to handle guest SEA, FSC: EC=%#x 
> xFSC=%#lx ESR_EL2=%#lx\n",
> + kvm_vcpu_trap_get_class(vcpu),
> + (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> + (unsigned long)kvm_vcpu_get_hsr(vcpu));
> + return -EFAULT;
> + }
> + } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +fault_status != FSC_ACCESS) {
>   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>   kvm_vcpu_trap_get_class(vcpu),
>   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),


> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..31c5171 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>  }
>  
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> + /*
> +  * synchronize_rcu() will wait for nmi_exit(), so no need to
> +  * rcu_read_lock().
> +  */

This comment has a life of its own! Given we don't always call ghes_notify_sea()
when we interrupted un-interruptable code its not always true. I think the
rcu_read_{,un}lock() should go against the list walk (so it looks like the
examples), and ditch the comment!


> + if(IS_ENABLED(ACPI_APEI_SEA)) {
> + rcu_read_lock();
> + ghes_notify_sea();
> + rcu_read_unlock();
> + }
> +
> + return 0;
> +}
> +
> +/*
>   * Dispatch a data abort to the relevant handler.
>   */
>  asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int 
> esr,
> 


Thanks,

James



Re: [PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-07 Thread James Morse
Hi Tyler,

On 06/03/17 20:45, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.

> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..a1a3dff 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,16 @@
>  #define FSC_FAULT(0x04)
>  #define FSC_ACCESS   (0x08)
>  #define FSC_PERM (0x0c)
> +#define FSC_SEA  (0x10)
> +#define FSC_SEA_TTW0 (0x14)
> +#define FSC_SEA_TTW1 (0x15)
> +#define FSC_SEA_TTW2 (0x16)
> +#define FSC_SEA_TTW3 (0x17)
> +#define FSC_SECC (0x18)
> +#define FSC_SECC_TTW0(0x1c)

aarch32 doesn't have either of these 'TW0' values, it's an unused encoding.
(However ...)

> +#define FSC_SECC_TTW1(0x1d)
> +#define FSC_SECC_TTW2(0x1e)
> +#define FSC_SECC_TTW3(0x1f)
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK   (~0xf)

>  #endif /* __ASM_ARM_SYSTEM_MISC_H */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..f3608c9 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, 
> struct kvm_run *run)
>  
>   /* Check the stage-2 fault is trans. fault or write fault */
>   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> - if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> - fault_status != FSC_ACCESS) {
> +
> + /* The host kernel will handle the synchronous external abort. There
> +  * is no need to pass the error into the guest.
> +  */
> + if (is_abort_synchronous(fault_status)) {
> + if(handle_guest_sea((unsigned long)fault_ipa,
> + kvm_vcpu_get_hsr(vcpu))) {


... Looking further up in this function:
>   is_iabt = kvm_vcpu_trap_is_iabt(vcpu);
>   if (unlikely(!is_iabt && kvm_vcpu_dabt_isextabt(vcpu))) {
>   kvm_inject_vabt(vcpu);
>   return 1;
>   }

... so external data aborts will have already been 'claimed' by kvm and dealt
with, and we already have a helper for spotting external aborts. (sorry I didn't
spot it earlier).

We need to do the handle_guest_sea() before this code.

kvm_inject_vabt() makes an SError interrupt pending for the guest. This makes a
synchronous error asynchronous as the guest may have SError interrupts masked.

I guess this was the best that could be done at the time of (4055710baca8
"arm/arm64: KVM: Inject virtual abort when guest exits on external abort"), but
in the light of this firmware-first handling, I'm not sure its the right thing
to do.

Is it possible for handle_guest_sea() to return whether it actually found any
work to do? If there was none I think we should keep this kvm_inject_vabt() as
it is the existing behaviour.


> + kvm_err("Failed to handle guest SEA, FSC: EC=%#x 
> xFSC=%#lx ESR_EL2=%#lx\n",
> + kvm_vcpu_trap_get_class(vcpu),
> + (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> + (unsigned long)kvm_vcpu_get_hsr(vcpu));
> + return -EFAULT;
> + }
> + } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +fault_status != FSC_ACCESS) {
>   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>   kvm_vcpu_trap_get_class(vcpu),
>   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),


> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..31c5171 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>  }
>  
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> + /*
> +  * synchronize_rcu() will wait for nmi_exit(), so no need to
> +  * rcu_read_lock().
> +  */

This comment has a life of its own! Given we don't always call ghes_notify_sea()
when we interrupted un-interruptable code its not always true. I think the
rcu_read_{,un}lock() should go against the list walk (so it looks like the
examples), and ditch the comment!


> + if(IS_ENABLED(ACPI_APEI_SEA)) {
> + rcu_read_lock();
> + ghes_notify_sea();
> + rcu_read_unlock();
> + }
> +
> + return 0;
> +}
> +
> +/*
>   * Dispatch a data abort to the relevant handler.
>   */
>  asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int 
> esr,
> 


Thanks,

James



[PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-06 Thread Tyler Baicar
Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.

Signed-off-by: Tyler Baicar 
---
 arch/arm/include/asm/kvm_arm.h   | 10 ++
 arch/arm/include/asm/system_misc.h   |  5 +
 arch/arm/kvm/mmu.c   | 36 ++--
 arch/arm64/include/asm/kvm_arm.h | 10 ++
 arch/arm64/include/asm/system_misc.h |  2 ++
 arch/arm64/mm/fault.c| 18 ++
 6 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index e22089f..a1a3dff 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -187,6 +187,16 @@
 #define FSC_FAULT  (0x04)
 #define FSC_ACCESS (0x08)
 #define FSC_PERM   (0x0c)
+#define FSC_SEA(0x10)
+#define FSC_SEA_TTW0   (0x14)
+#define FSC_SEA_TTW1   (0x15)
+#define FSC_SEA_TTW2   (0x16)
+#define FSC_SEA_TTW3   (0x17)
+#define FSC_SECC   (0x18)
+#define FSC_SECC_TTW0  (0x1c)
+#define FSC_SECC_TTW1  (0x1d)
+#define FSC_SECC_TTW2  (0x1e)
+#define FSC_SECC_TTW3  (0x1f)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK (~0xf)
diff --git a/arch/arm/include/asm/system_misc.h 
b/arch/arm/include/asm/system_misc.h
index a3d61ad..ea45d94 100644
--- a/arch/arm/include/asm/system_misc.h
+++ b/arch/arm/include/asm/system_misc.h
@@ -24,4 +24,9 @@
 
 #endif /* !__ASSEMBLY__ */
 
+static inline int handle_guest_sea(unsigned long addr, unsigned int esr)
+{
+   return -1;
+}
+
 #endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..f3608c9 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "trace.h"
 
@@ -1409,6 +1410,24 @@ static void handle_access_fault(struct kvm_vcpu *vcpu, 
phys_addr_t fault_ipa)
kvm_set_pfn_accessed(pfn);
 }
 
+static bool is_abort_synchronous(unsigned long fault_status) {
+   switch (fault_status) {
+   case FSC_SEA:
+   case FSC_SEA_TTW0:
+   case FSC_SEA_TTW1:
+   case FSC_SEA_TTW2:
+   case FSC_SEA_TTW3:
+   case FSC_SECC:
+   case FSC_SECC_TTW0:
+   case FSC_SECC_TTW1:
+   case FSC_SECC_TTW2:
+   case FSC_SECC_TTW3:
+   return true;
+   default:
+   return false;
+   }
+}
+
 /**
  * kvm_handle_guest_abort - handles all 2nd stage aborts
  * @vcpu:  the VCPU pointer
@@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
 
/* Check the stage-2 fault is trans. fault or write fault */
fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-   if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-   fault_status != FSC_ACCESS) {
+
+   /* The host kernel will handle the synchronous external abort. There
+* is no need to pass the error into the guest.
+*/
+   if (is_abort_synchronous(fault_status)) {
+   if(handle_guest_sea((unsigned long)fault_ipa,
+   kvm_vcpu_get_hsr(vcpu))) {
+   kvm_err("Failed to handle guest SEA, FSC: EC=%#x 
xFSC=%#lx ESR_EL2=%#lx\n",
+   kvm_vcpu_trap_get_class(vcpu),
+   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+   (unsigned long)kvm_vcpu_get_hsr(vcpu));
+   return -EFAULT;
+   }
+   } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+  fault_status != FSC_ACCESS) {
kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
kvm_vcpu_trap_get_class(vcpu),
(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 2a2752b..dcacdbb 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -201,6 +201,16 @@
 #define FSC_FAULT  ESR_ELx_FSC_FAULT
 #define FSC_ACCESS ESR_ELx_FSC_ACCESS
 #define FSC_PERM   ESR_ELx_FSC_PERM
+#define FSC_SEAESR_ELx_FSC_EXTABT
+#define FSC_SEA_TTW0   (0x14)
+#define FSC_SEA_TTW1   (0x15)
+#define FSC_SEA_TTW2   (0x16)
+#define FSC_SEA_TTW3   (0x17)
+#define FSC_SECC   (0x18)
+#define FSC_SECC_TTW0  (0x1c)
+#define FSC_SECC_TTW1  (0x1d)
+#define FSC_SECC_TTW2  (0x1e)
+#define FSC_SECC_TTW3  (0x1f)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK (~UL(0xf))
diff --git a/arch/arm64/include/asm/system_misc.h 
b/arch/arm64/include/asm/system_misc.h
index bc81243..5b2cecd1 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -58,4 +58,6 @@ void hook_debug_fault_code(int 

[PATCH V12 10/10] arm/arm64: KVM: add guest SEA support

2017-03-06 Thread Tyler Baicar
Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.

Signed-off-by: Tyler Baicar 
---
 arch/arm/include/asm/kvm_arm.h   | 10 ++
 arch/arm/include/asm/system_misc.h   |  5 +
 arch/arm/kvm/mmu.c   | 36 ++--
 arch/arm64/include/asm/kvm_arm.h | 10 ++
 arch/arm64/include/asm/system_misc.h |  2 ++
 arch/arm64/mm/fault.c| 18 ++
 6 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index e22089f..a1a3dff 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -187,6 +187,16 @@
 #define FSC_FAULT  (0x04)
 #define FSC_ACCESS (0x08)
 #define FSC_PERM   (0x0c)
+#define FSC_SEA(0x10)
+#define FSC_SEA_TTW0   (0x14)
+#define FSC_SEA_TTW1   (0x15)
+#define FSC_SEA_TTW2   (0x16)
+#define FSC_SEA_TTW3   (0x17)
+#define FSC_SECC   (0x18)
+#define FSC_SECC_TTW0  (0x1c)
+#define FSC_SECC_TTW1  (0x1d)
+#define FSC_SECC_TTW2  (0x1e)
+#define FSC_SECC_TTW3  (0x1f)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK (~0xf)
diff --git a/arch/arm/include/asm/system_misc.h 
b/arch/arm/include/asm/system_misc.h
index a3d61ad..ea45d94 100644
--- a/arch/arm/include/asm/system_misc.h
+++ b/arch/arm/include/asm/system_misc.h
@@ -24,4 +24,9 @@
 
 #endif /* !__ASSEMBLY__ */
 
+static inline int handle_guest_sea(unsigned long addr, unsigned int esr)
+{
+   return -1;
+}
+
 #endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..f3608c9 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "trace.h"
 
@@ -1409,6 +1410,24 @@ static void handle_access_fault(struct kvm_vcpu *vcpu, 
phys_addr_t fault_ipa)
kvm_set_pfn_accessed(pfn);
 }
 
+static bool is_abort_synchronous(unsigned long fault_status) {
+   switch (fault_status) {
+   case FSC_SEA:
+   case FSC_SEA_TTW0:
+   case FSC_SEA_TTW1:
+   case FSC_SEA_TTW2:
+   case FSC_SEA_TTW3:
+   case FSC_SECC:
+   case FSC_SECC_TTW0:
+   case FSC_SECC_TTW1:
+   case FSC_SECC_TTW2:
+   case FSC_SECC_TTW3:
+   return true;
+   default:
+   return false;
+   }
+}
+
 /**
  * kvm_handle_guest_abort - handles all 2nd stage aborts
  * @vcpu:  the VCPU pointer
@@ -1444,8 +1463,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct 
kvm_run *run)
 
/* Check the stage-2 fault is trans. fault or write fault */
fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-   if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-   fault_status != FSC_ACCESS) {
+
+   /* The host kernel will handle the synchronous external abort. There
+* is no need to pass the error into the guest.
+*/
+   if (is_abort_synchronous(fault_status)) {
+   if(handle_guest_sea((unsigned long)fault_ipa,
+   kvm_vcpu_get_hsr(vcpu))) {
+   kvm_err("Failed to handle guest SEA, FSC: EC=%#x 
xFSC=%#lx ESR_EL2=%#lx\n",
+   kvm_vcpu_trap_get_class(vcpu),
+   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+   (unsigned long)kvm_vcpu_get_hsr(vcpu));
+   return -EFAULT;
+   }
+   } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+  fault_status != FSC_ACCESS) {
kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
kvm_vcpu_trap_get_class(vcpu),
(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 2a2752b..dcacdbb 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -201,6 +201,16 @@
 #define FSC_FAULT  ESR_ELx_FSC_FAULT
 #define FSC_ACCESS ESR_ELx_FSC_ACCESS
 #define FSC_PERM   ESR_ELx_FSC_PERM
+#define FSC_SEAESR_ELx_FSC_EXTABT
+#define FSC_SEA_TTW0   (0x14)
+#define FSC_SEA_TTW1   (0x15)
+#define FSC_SEA_TTW2   (0x16)
+#define FSC_SEA_TTW3   (0x17)
+#define FSC_SECC   (0x18)
+#define FSC_SECC_TTW0  (0x1c)
+#define FSC_SECC_TTW1  (0x1d)
+#define FSC_SECC_TTW2  (0x1e)
+#define FSC_SECC_TTW3  (0x1f)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK (~UL(0xf))
diff --git a/arch/arm64/include/asm/system_misc.h 
b/arch/arm64/include/asm/system_misc.h
index bc81243..5b2cecd1 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -58,4 +58,6 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned