[PATCH 3/3] arm64: Add software workaround for Falkor erratum 1041

2017-11-02 Thread Shanker Donthineni
The ARM architecture defines the memory locations that are permitted
to be accessed as the result of a speculative instruction fetch from
an exception level for which all stages of translation are disabled.
Specifically, the core is permitted to speculatively fetch from the
4KB region containing the current program counter and next 4KB.

When translation is changed from enabled to disabled for the running
exception level (SCTLR_ELn[M] changed from a value of 1 to 0), the
Falkor core may errantly speculatively access memory locations outside
of the 4KB region permitted by the architecture. The errant memory
access may lead to one of the following unexpected behaviors.

1) A System Error Interrupt (SEI) being raised by the Falkor core due
   to the errant memory access attempting to access a region of memory
   that is protected by a slave-side memory protection unit.
2) Unpredictable device behavior due to a speculative read from device
   memory. This behavior may only occur if the instruction cache is
   disabled prior to or coincident with translation being changed from
   enabled to disabled.

To avoid the errant behavior, software must execute an ISB immediately
prior to executing the MSR that will change SCTLR_ELn[M] from 1 to 0.

Signed-off-by: Shanker Donthineni 
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 10 ++
 arch/arm64/include/asm/assembler.h | 17 +
 arch/arm64/include/asm/cpucaps.h   |  3 ++-
 arch/arm64/kernel/cpu_errata.c | 16 
 arch/arm64/kernel/efi-entry.S  |  4 ++--
 arch/arm64/kernel/head.S   |  4 ++--
 7 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index 66e8ce1..704770c0 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -74,3 +74,4 @@ stable kernels.
 | Qualcomm Tech. | Falkor v1   | E1003   | 
QCOM_FALKOR_ERRATUM_1003|
 | Qualcomm Tech. | Falkor v1   | E1009   | 
QCOM_FALKOR_ERRATUM_1009|
 | Qualcomm Tech. | QDF2400 ITS | E0065   | 
QCOM_QDF2400_ERRATUM_0065   |
+| Qualcomm Tech. | Falkor v{1,2}   | E1041   | 
QCOM_FALKOR_ERRATUM_1041|
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0df64a6..7e933fb 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -539,6 +539,16 @@ config QCOM_QDF2400_ERRATUM_0065
 
  If unsure, say Y.
 
+config QCOM_FALKOR_ERRATUM_1041
+   bool "Falkor E1041: Speculative instruction fetches might cause errant 
memory access"
+   default y
+   help
+ Falkor CPU may speculatively fetch instructions from an improper
+ memory location when MMU translation is changed from SCTLR_ELn[M]=1
+ to SCTLR_ELn[M]=0. Prefix an ISB instruction to fix the problem.
+
+ If unsure, say Y.
+
 endmenu
 
 
diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index b6dfb4f..4c91efb 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Enable and disable interrupts.
@@ -514,6 +515,22 @@
  *   reg: the value to be written.
  */
.macro  write_sctlr, eln, reg
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1041
+alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1041
+   tbnz\reg, #0, 8000f  // enable MMU?
+   isb
+8000:
+alternative_else_nop_endif
+#endif
+   msr sctlr_\eln, \reg
+   .endm
+
+   .macro  early_write_sctlr, eln, reg
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1041
+   tbnz\reg, #0, 8000f  // enable MMU?
+   isb
+8000:
+#endif
msr sctlr_\eln, \reg
.endm
 
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 8da6216..7f7a59d 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -40,7 +40,8 @@
 #define ARM64_WORKAROUND_85892119
 #define ARM64_WORKAROUND_CAVIUM_30115  20
 #define ARM64_HAS_DCPOP21
+#define ARM64_WORKAROUND_QCOM_FALKOR_E1041 22
 
-#define ARM64_NCAPS22
+#define ARM64_NCAPS23
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 0e27f86..27f9a45 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -179,6 +179,22 @@ static int cpu_enable_trap_ctr_access(void *__unused)
   MIDR_CPU_VAR_REV(0, 0)),
},
 #endif
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1041
+   {
+   .desc = "Qualcomm Technologies Falkor erratum 1041",
+   .capability = ARM64_WORKAROUND_QCOM_FALKOR_E1041,
+   MIDR_RANGE(MIDR_QCOM_FALKOR_V1,
+   MIDR_CPU_VAR

[PATCH 0/3] Implement a software workaround for Falkor erratum 1041

2017-11-02 Thread Shanker Donthineni
On Falkor CPU, we’ve discovered a hardware issue which might lead to a
kernel crash or the unexpected behavior. The Falkor core may errantly
access memory locations on speculative instruction fetches. This may
happen whenever MMU translation state, SCTLR_ELn[M] bit is being changed
from enabled to disabled for the currently running exception level. To
prevent the errant hardware behavior, software must execute an ISB
immediately prior to executing the MSR that changes SCTLR_ELn[M] from a
value of 1 to 0. To simplify the complexity of a workaround, this patch
series issues an ISB whenever SCTLR_ELn[M] is changed to 0 to fix the
Falkor erratum 1041.

Patch1:
  - CPUTYPE definitions for Falkor CPU.

Patch2:
  - Define two ASM helper macros to read/write SCTLR_ELn register.

Patch3:
  - Actual workaround changes for erratum E1041.

Shanker Donthineni (3):
  arm64: Define cputype macros for Falkor CPU
  arm64: Prepare SCTLR_ELn accesses to handle Falkor erratum 1041
  arm64: Add software workaround for Falkor erratum 1041

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 10 ++
 arch/arm64/include/asm/assembler.h | 35 ++
 arch/arm64/include/asm/cpucaps.h   |  3 ++-
 arch/arm64/include/asm/cputype.h   |  2 ++
 arch/arm64/kernel/cpu-reset.S  |  4 ++--
 arch/arm64/kernel/cpu_errata.c | 16 
 arch/arm64/kernel/efi-entry.S  |  8 
 arch/arm64/kernel/head.S   | 18 -
 arch/arm64/kernel/relocate_kernel.S|  4 ++--
 arch/arm64/kvm/hyp-init.S  |  6 +++---
 arch/arm64/mm/proc.S   |  6 +++---
 12 files changed, 89 insertions(+), 24 deletions(-)

-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, 
Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH 1/3] arm64: Define cputype macros for Falkor CPU

2017-11-02 Thread Shanker Donthineni
Add cputype definition macros for Qualcomm Datacenter Technologies
Falkor CPU in cputype.h. It's unfortunate that the first revision
of the Falkor CPU used the wrong part number 0x800, got fixed in v2
chip with part number 0xC00, and would be used the same value for
future revisions.

Signed-off-by: Shanker Donthineni 
Signed-off-by: Neil Leeder 
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 235e77d..cbf08d7 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -91,6 +91,7 @@
 #define BRCM_CPU_PART_VULCAN   0x516
 
 #define QCOM_CPU_PART_FALKOR_V10x800
+#define QCOM_CPU_PART_FALKOR   0xC00
 
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, 
ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, 
ARM_CPU_PART_CORTEX_A57)
@@ -99,6 +100,7 @@
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, 
CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, 
CAVIUM_CPU_PART_THUNDERX_83XX)
 #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, 
QCOM_CPU_PART_FALKOR_V1)
+#define MIDR_QCOM_FALKOR MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR)
 
 #ifndef __ASSEMBLY__
 
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, 
Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH 2/3] arm64: Prepare SCTLR_ELn accesses to handle Falkor erratum 1041

2017-11-02 Thread Shanker Donthineni
This patch introduces two helper macros read_sctlr and write_sctlr
to access system register SCTLR_ELn. Replace all MSR/MRS references
to sctlr_el1{el2} with macros.

This should cause no behavioral change.

Signed-off-by: Shanker Donthineni 
---
 arch/arm64/include/asm/assembler.h  | 18 ++
 arch/arm64/kernel/cpu-reset.S   |  4 ++--
 arch/arm64/kernel/efi-entry.S   |  8 
 arch/arm64/kernel/head.S| 18 +-
 arch/arm64/kernel/relocate_kernel.S |  4 ++--
 arch/arm64/kvm/hyp-init.S   |  6 +++---
 arch/arm64/mm/proc.S|  6 +++---
 7 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index d58a625..b6dfb4f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -499,4 +499,22 @@
 #endif
.endm
 
+/**
+ * Read value of the system control register SCTLR_ELn.
+ *   eln: which system control register.
+ *   reg: contents of the SCTLR_ELn.
+ */
+   .macro  read_sctlr, eln, reg
+   mrs \reg, sctlr_\eln
+   .endm
+
+/**
+ * Write the value to the system control register SCTLR_ELn.
+ *   eln: which system control register.
+ *   reg: the value to be written.
+ */
+   .macro  write_sctlr, eln, reg
+   msr sctlr_\eln, \reg
+   .endm
+
 #endif /* __ASM_ASSEMBLER_H */
diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
index 65f42d2..9224abd 100644
--- a/arch/arm64/kernel/cpu-reset.S
+++ b/arch/arm64/kernel/cpu-reset.S
@@ -34,10 +34,10 @@
  */
 ENTRY(__cpu_soft_restart)
/* Clear sctlr_el1 flags. */
-   mrs x12, sctlr_el1
+   read_sctlr el1, x12
ldr x13, =SCTLR_ELx_FLAGS
bic x12, x12, x13
-   msr sctlr_el1, x12
+   write_sctlr el1, x12
isb
 
cbz x0, 1f  // el2_switch?
diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
index 4e6ad35..acae627 100644
--- a/arch/arm64/kernel/efi-entry.S
+++ b/arch/arm64/kernel/efi-entry.S
@@ -93,17 +93,17 @@ ENTRY(entry)
mrs x0, CurrentEL
cmp x0, #CurrentEL_EL2
b.ne1f
-   mrs x0, sctlr_el2
+   read_sctlr el2, x0
bic x0, x0, #1 << 0 // clear SCTLR.M
bic x0, x0, #1 << 2 // clear SCTLR.C
-   msr sctlr_el2, x0
+   write_sctlr el2, x0
isb
b   2f
 1:
-   mrs x0, sctlr_el1
+   read_sctlr el1, x0
bic x0, x0, #1 << 0 // clear SCTLR.M
bic x0, x0, #1 << 2 // clear SCTLR.C
-   msr sctlr_el1, x0
+   write_sctlr el1, x0
isb
 2:
/* Jump to kernel entry point */
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 0b243ec..b8d5b73 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -388,18 +388,18 @@ ENTRY(el2_setup)
mrs x0, CurrentEL
cmp x0, #CurrentEL_EL2
b.eq1f
-   mrs x0, sctlr_el1
+   read_sctlr el1, x0
 CPU_BE(orr x0, x0, #(3 << 24)  )   // Set the EE and E0E 
bits for EL1
 CPU_LE(bic x0, x0, #(3 << 24)  )   // Clear the EE and E0E 
bits for EL1
-   msr sctlr_el1, x0
+   write_sctlr el1, x0
mov w0, #BOOT_CPU_MODE_EL1  // This cpu booted in EL1
isb
ret
 
-1: mrs x0, sctlr_el2
+1: read_sctlr el2, x0
 CPU_BE(orr x0, x0, #(1 << 25)  )   // Set the EE bit for 
EL2
 CPU_LE(bic x0, x0, #(1 << 25)  )   // Clear the EE bit for 
EL2
-   msr sctlr_el2, x0
+   write_sctlr el2, x0
 
 #ifdef CONFIG_ARM64_VHE
/*
@@ -511,7 +511,7 @@ install_el2_stub:
mov x0, #0x0800 // Set/clear RES{1,0} bits
 CPU_BE(movkx0, #0x33d0, lsl #16)   // Set EE and E0E on BE 
systems
 CPU_LE(movkx0, #0x30d0, lsl #16)   // Clear EE and E0E on 
LE systems
-   msr sctlr_el1, x0
+   write_sctlr el1, x0
 
/* Coprocessor traps. */
mov x0, #0x33ff
@@ -664,7 +664,7 @@ ENTRY(__enable_mmu)
msr ttbr0_el1, x1   // load TTBR0
msr ttbr1_el1, x2   // load TTBR1
isb
-   msr sctlr_el1, x0
+   write_sctlr el1, x0
isb
/*
 * Invalidate the local I-cache so that any instructions fetched
@@ -716,7 +716,7 @@ ENDPROC(__relocate_kernel)
 __primary_switch:
 #ifdef CONFIG_RANDOMIZE_BASE
mov x19, x0 // preserve new SCTLR_EL1 value
-   mrs x20, sctlr_el1  // preserve old SCTLR_EL1 value
+   read_sctlr el1, x20 // preserve old SCTLR_EL1 value
 #endif
 
bl  __enable_mmu
@@ -732,14 +732,14 @@ __primary_switch:
 * to take into account by discarding the current kernel mapping and
 

Re: [PATCH v5 04/30] arm64: KVM: Hide unsupported AArch64 CPU features from guests

2017-11-02 Thread Christoffer Dall
On Thu, Nov 02, 2017 at 11:01:37AM +, Dave Martin wrote:
> On Thu, Nov 02, 2017 at 09:15:57AM +0100, Christoffer Dall wrote:
> > On Wed, Nov 01, 2017 at 10:26:03AM +, Dave Martin wrote:
> > > On Wed, Nov 01, 2017 at 05:47:29AM +0100, Christoffer Dall wrote:
> > > > On Tue, Oct 31, 2017 at 03:50:56PM +, Dave Martin wrote:
> > > > > Currently, a guest kernel sees the true CPU feature registers
> > > > > (ID_*_EL1) when it reads them using MRS instructions.  This means
> > > > > that the guest may observe features that are present in the
> > > > > hardware but the host doesn't understand or doesn't provide support
> > > > > for.  A guest may legimitately try to use such a feature as per the
> > > > > architecture, but use of the feature may trap instead of working
> > > > > normally, triggering undef injection into the guest.
> > > > > 
> > > > > This is not a problem for the host, but the guest may go wrong when
> > > > > running on newer hardware than the host knows about.
> > > > > 
> > > > > This patch hides from guest VMs any AArch64-specific CPU features
> > > > > that the host doesn't support, by exposing to the guest the
> > > > > sanitised versions of the registers computed by the cpufeatures
> > > > > framework, instead of the true hardware registers.  To achieve
> > > > > this, HCR_EL2.TID3 is now set for AArch64 guests, and emulation
> > > > > code is added to KVM to report the sanitised versions of the
> > > > > affected registers in response to MRS and register reads from
> > > > > userspace.
> > > > > 
> > > > > The affected registers are removed from invariant_sys_regs[] (since
> > > > > the invariant_sys_regs handling is no longer quite correct for
> > > > > them) and added to sys_reg_desgs[], with appropriate access(),
> > > > > get_user() and set_user() methods.  No runtime vcpu storage is
> > > > > allocated for the registers: instead, they are read on demand from
> > > > > the cpufeatures framework.  This may need modification in the
> > > > > future if there is a need for userspace to customise the features
> > > > > visible to the guest.
> > > > > 
> > > > > Attempts by userspace to write the registers are handled similarly
> > > > > to the current invariant_sys_regs handling: writes are permitted,
> > > > > but only if they don't attempt to change the value.  This is
> > > > > sufficient to support VM snapshot/restore from userspace.
> > > > > 
> > > > > Because of the additional registers, restoring a VM on an older
> > > > > kernel may not work unless userspace knows how to handle the extra
> > > > > VM registers exposed to the KVM user ABI by this patch.
> > > > > 
> > > > > Under the principle of least damage, this patch makes no attempt to
> > > > > handle any of the other registers currently in
> > > > > invariant_sys_regs[], or to emulate registers for AArch32: however,
> > > > > these could be handled in a similar way in future, as necessary.
> > > > > 
> > > > > Signed-off-by: Dave Martin 
> > > > > Reviewed-by: Marc Zyngier 
> > > > > Acked-by: Catalin Marinas 
> > > > > Cc: Christoffer Dall 
> > > > > ---
> > > > >  arch/arm64/include/asm/sysreg.h |   3 +
> > > > >  arch/arm64/kvm/hyp/switch.c |   6 +
> > > > >  arch/arm64/kvm/sys_regs.c   | 282 
> > > > > +---
> > > > >  3 files changed, 246 insertions(+), 45 deletions(-)
> > > > > 
> > > > > diff --git a/arch/arm64/include/asm/sysreg.h 
> > > > > b/arch/arm64/include/asm/sysreg.h
> > > > > index 4dceb12..609d59af 100644
> > > > > --- a/arch/arm64/include/asm/sysreg.h
> > > > > +++ b/arch/arm64/include/asm/sysreg.h
> > > > > @@ -149,6 +149,9 @@
> > > > >  #define SYS_ID_AA64DFR0_EL1  sys_reg(3, 0, 0, 5, 0)
> > > > >  #define SYS_ID_AA64DFR1_EL1  sys_reg(3, 0, 0, 5, 1)
> > > > >  
> > > > > +#define SYS_ID_AA64AFR0_EL1  sys_reg(3, 0, 0, 5, 4)
> > > > > +#define SYS_ID_AA64AFR1_EL1  sys_reg(3, 0, 0, 5, 5)
> > > > > +
> > > > >  #define SYS_ID_AA64ISAR0_EL1 sys_reg(3, 0, 0, 6, 0)
> > > > >  #define SYS_ID_AA64ISAR1_EL1 sys_reg(3, 0, 0, 6, 1)
> > > > >  
> > > > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > > > index 945e79c..35a90b8 100644
> > > > > --- a/arch/arm64/kvm/hyp/switch.c
> > > > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > > > @@ -81,11 +81,17 @@ static void __hyp_text __activate_traps(struct 
> > > > > kvm_vcpu *vcpu)
> > > > >* it will cause an exception.
> > > > >*/
> > > > >   val = vcpu->arch.hcr_el2;
> > > > > +
> > > > >   if (!(val & HCR_RW) && system_supports_fpsimd()) {
> > > > >   write_sysreg(1 << 30, fpexc32_el2);
> > > > >   isb();
> > > > >   }
> > > > > +
> > > > > + if (val & HCR_RW) /* for AArch64 only: */
> > > > > + val |= HCR_TID3; /* TID3: trap feature register 
> > > > > accesses */
> > > > > +
> > > > 
> > > > I still think we should set this in vcpu_reset_hcr() and do it once
> > > > instead 

Re: [PULL 0/8] KVM/ARM Fixes for v4.14

2017-11-02 Thread Paolo Bonzini
On 30/10/2017 03:55, Christoffer Dall wrote:
> Hi Paolo and Radim,
> 
> Here are the fixes we've been trying to get ready for v4.14 for a while;
> sorry about sending these so late.
> 
> We're fixing:
> 
>  - a number of issues with saving/restoring the ITS
>  - a bug in KVM/ARM when branch profiling is enabled in Hyp mode
>  - an emulation bug for 32-bit guests when injecting aborts
>  - a failure to check if a kmalloc succeeds in the ITS emulation
> 
> The following changes since commit 8a5776a5f49812d29fe4b2d0a2d71675c3facf3f:
> 
>   Linux 4.14-rc4 (2017-10-08 20:53:29 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git 
> tags/kvm-arm-fixes-for-v4.14
> 
> for you to fetch changes up to c2385eaa6c5a87cdc4e04ed589ae103ca3297c84:
> 
>   KVM: arm/arm64: vgic-its: Check GITS_BASER Valid bit before saving tables 
> (2017-10-29 03:25:06 +0100)
> 
> 
> Thanks,
> -Christoffer
> 
> Christoffer Dall (1):
>   KVM: arm64: its: Fix missing dynamic allocation check in scan_its_table
> 
> Dongjiu Geng (1):
>   arm/arm64: KVM: set right LR register value for 32 bit guest when 
> inject abort
> 
> Eric Auger (3):
>   KVM: arm/arm64: vgic-its: Fix vgic_its_restore_collection_table 
> returned value
>   KVM: arm/arm64: vgic-its: Check CBASER/BASER validity before enabling 
> the ITS
>   KVM: arm/arm64: vgic-its: Check GITS_BASER Valid bit before saving 
> tables
> 
> Julien Thierry (2):
>   arm/arm64: kvm: Move initialization completion message
>   arm/arm64: kvm: Disable branch profiling in HYP code
> 
> wanghaibin (1):
>   KVM: arm/arm64: vgic-its: Fix return value for device table restore
> 
>  arch/arm/kvm/emulate.c|  6 ++--
>  arch/arm/kvm/hyp/Makefile |  2 +-
>  arch/arm64/kvm/hyp/Makefile   |  2 +-
>  arch/arm64/kvm/inject_fault.c | 16 +-
>  virt/kvm/arm/arm.c| 31 +-
>  virt/kvm/arm/vgic/vgic-its.c  | 73 
> ---
>  6 files changed, 81 insertions(+), 49 deletions(-)
> 


Pulled, thanks.

Paolo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v5 00/30] ARM Scalable Vector Extension (SVE)

2017-11-02 Thread Dave P Martin
On Thu, Nov 02, 2017 at 04:32:49PM +, Will Deacon wrote:
> On Tue, Oct 31, 2017 at 03:50:52PM +, Dave Martin wrote:
> > This series implements Linux kernel support for the ARM Scalable Vector
> > Extension (SVE). [1]  It supersedes the previous v3: see [3] for link
> > and full cover letter.
> >
> > This is a minor update to v4, but does contain a couple of important
> > fixes.
>
> I've pushed a version of this series out to the arm64 devel branch, with
> a view to putting it into next.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git devel
>
> Please kick the tyres and let me know of any issues ASAP.

No obvious problems so far.

Cheers
---Dave
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v5 00/30] ARM Scalable Vector Extension (SVE)

2017-11-02 Thread Will Deacon
On Tue, Oct 31, 2017 at 03:50:52PM +, Dave Martin wrote:
> This series implements Linux kernel support for the ARM Scalable Vector
> Extension (SVE). [1]  It supersedes the previous v3: see [3] for link
> and full cover letter.
> 
> This is a minor update to v4, but does contain a couple of important
> fixes.

I've pushed a version of this series out to the arm64 devel branch, with
a view to putting it into next.

git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git devel

Please kick the tyres and let me know of any issues ASAP.

Will
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 6/9] KVM: arm/arm64: Limit icache invalidation to prefetch aborts

2017-11-02 Thread Andrew Jones
On Thu, Nov 02, 2017 at 10:36:35AM +, Marc Zyngier wrote:
> On Wed, Nov 01 2017 at 11:17:27 am GMT, Andrew Jones  
> wrote:
> > On Mon, Oct 23, 2017 at 05:11:19PM +0100, Marc Zyngier wrote:
> >> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> >> index 2174244f6317..0417c8e2a81c 100644
> >> --- a/virt/kvm/arm/mmu.c
> >> +++ b/virt/kvm/arm/mmu.c
> >> @@ -1292,7 +1292,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
> >> phys_addr_t fault_ipa,
> >>  unsigned long fault_status)
> >>  {
> >>int ret;
> >> -  bool write_fault, writable, hugetlb = false, force_pte = false;
> >> +  bool write_fault, exec_fault, writable, hugetlb = false, force_pte = 
> >> false;
> >>unsigned long mmu_seq;
> >>gfn_t gfn = fault_ipa >> PAGE_SHIFT;
> >>struct kvm *kvm = vcpu->kvm;
> >> @@ -1304,7 +1304,10 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
> >> phys_addr_t fault_ipa,
> >>unsigned long flags = 0;
> >>  
> >>write_fault = kvm_is_write_fault(vcpu);
> >> -  if (fault_status == FSC_PERM && !write_fault) {
> >> +  exec_fault = kvm_vcpu_trap_is_iabt(vcpu);
> >> +  VM_BUG_ON(write_fault && exec_fault);
> >
> > This VM_BUG_ON can never fire as long as kvm_is_write_fault() is
> > defined as
> >
> >  {
> >if (kvm_vcpu_trap_is_iabt(vcpu))
> >return false;
> >return kvm_vcpu_dabt_iswrite(vcpu);
> >  }
> 
> That's indeed what I expect. But given that the code now relies on this
> property, I chose to make it explicit.
> 
> Or are you seeing a better way of making this an invariant?
>

No, I wish I did, because then I wouldn't have to apologize for the
noise :-) The VM_BUG_ON() does indeed improve the code by documenting/
enforcing the requirement.

Thanks,
drew
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 20/21] KVM: arm64: Take any host SError before entering the guest

2017-11-02 Thread James Morse
Hi Christoffer,

On 01/11/17 04:55, Christoffer Dall wrote:
> On Tue, Oct 31, 2017 at 11:43:42AM +, James Morse wrote:
>> On 31/10/17 06:23, Christoffer Dall wrote:
>>> On Thu, Oct 19, 2017 at 03:58:06PM +0100, James Morse wrote:
 On VHE systems KVM masks SError before switching the VBAR value. Any
 host RAS error that the CPU knew about before world-switch may become
 pending as an SError during world-switch, and only be taken once we enter
 the guest.

 Until KVM can take RAS SErrors during world switch, add an ESB to
 force any RAS errors to be synchronised and taken on the host before
 we enter world switch.

 RAS errors that become pending during world switch are still taken
 once we enter the guest.
>>
 diff --git a/arch/arm64/include/asm/kvm_host.h 
 b/arch/arm64/include/asm/kvm_host.h
 index cf5d78ba14b5..5dc6f2877762 100644
 --- a/arch/arm64/include/asm/kvm_host.h
 +++ b/arch/arm64/include/asm/kvm_host.h
 @@ -392,6 +392,7 @@ static inline void __cpu_init_stage2(void)
  
  static inline void kvm_arm_vhe_guest_enter(void)
  {
 +  esb();
>>
>>> I don't fully appreciate what the point of this is?
>>>
>>> As I understand it, our fundamental goal here is to try to distinguish
>>> between errors happening on the host or in the guest.
>>
>> Not just host/guest, but also those we can and can't handle.
>>
>> KVM can't currently take an SError during world switch, so a RAS error that 
>> the
>> CPU was hoping to defer may spread from the host into KVM's
>> no-SError:world-switch code. If this happens it will (almost certainly) have 
>> to
>> be re-classified as uncontainable.
>>
>> There is also a firmware-first angle here: NOTIFY_SEI can't be delivered if 
>> the
>> normal world has SError masked, so any error that spreads past this point
>> becomes a reboot-by-firmware instead of an OS notification and almost-helpful
>> error message.
>>
>>
>>> If that's correct, then why don't we do it at the last possible moment
>>> when we still have a scratch register left, in the world switch code
>>> itself, and in the case abort the guest entry and report back a "host
>>> SError" return code.
>>
>> We have IESB to run the error-barrier as we enter the guest. This would make 
>> any
>> host error pending as an SError, and we would exit the guest immediately. 
>> But if
>> there was an RAS error during world switch, by this point its likely to be
>> classified as uncontainable.
>>
>> This esb() is trying to keep this window of code as small as possible, to 
>> just
>> errors that occur during world switch.
>>
>> With your vcpu load/save this window becomes a lot smaller, it may be 
>> possible
>> to get a VHE-host's arch-code SError handler to take errors from EL2, in 
>> which
>> case this barrier can disappear.
>> (note to self: guest may still own the debug hardware)
>>
> 
> ok, thanks for your detailed explanation.  I didn't consider that the
> classification of a RAS error as containable vs. non-containable
> depended on where we take the exception.

Will makes the point over on patch 11 that until we have different handling for
these different classifications of error, there isn't much point doing this now.
(i.e. we treat an error generated here, or when we enter the guest in the same 
way).

I was trying to keep my eye on what we need for kernel-first support, so we
don't have to change the code twice, we just expand the error handling to do 
better.

I'll drop this patch for now, it will come back if/when we get kernel-first
support for RAS.


What about firmware-first? Firmware can always take these errors when the normal
world is running. Dropping the barrier means its up to the CPU when any error
gets reported, if firmware has to use NOTIFY_SEI it will have to do a reboot if
the error occurs during world-switch (as SError is masked). If an error spreads
over this boundary, that's just tough-luck, the kernel would have panic'd 
anyway.


Sorry for the noise,


Thanks,

James

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 11/21] arm64: cpufeature: Detect CPU RAS Extentions

2017-11-02 Thread James Morse
Hi Will,

On 31/10/17 13:14, Will Deacon wrote:
> On Thu, Oct 19, 2017 at 03:57:57PM +0100, James Morse wrote:
>> From: Xie XiuQi 
>>
>> ARM's v8.2 Extentions add support for Reliability, Availability and
>> Serviceability (RAS). On CPUs with these extensions system software
>> can use additional barriers to isolate errors and determine if faults
>> are pending.
>>
>> Add cpufeature detection and a barrier in the context-switch code.
>> There is no need to use alternatives for this as CPUs that don't
>> support this feature will treat the instruction as a nop.
>>
>> Platform level RAS support may require additional firmware support.

>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index cd52d365d1f0..0fc017b55cb1 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -125,6 +125,7 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = {
>>  };
>>  
>>  static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
>> +ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 
>> ID_AA64PFR0_RAS_SHIFT, 4, 0),

> We probably want FTR_LOWER_SAFE here now, right? (we changed the other
> fields in for-next/core).

Ah, yes.
(Looks like some copy-and-paste)


>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index 2dc0f8482210..5e5d2f0a1d0a 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -365,6 +365,9 @@ __notrace_funcgraph struct task_struct 
>> *__switch_to(struct task_struct *prev,
>>   */
>>  dsb(ish);
>>  
>> +/* Deliver any pending SError from prev */
>> +esb();

> I'm assuming this is going to be expensive.

I'm hoping not, but without numbers to prove otherwise...


> What if we moved it to switch_mm
> instead. Do we actually need thread granularity for error isolation?

(after a verbal discussion with Will:)

This would be needed to blame the correct thread, but until we have kernel-first
handling this is moot as do_serror() will panic() regardless.

So, lets drop the esb() here and decide what to do if/when we get kernel-first
handling. If that only acts on groups of threads, then switch_mm is a better
place for it.

In the meantime if we see RAS SError panic()s we should remember it may have
just switched task, which in practice will probably be obvious from the stack 
trace.

There is no firmware-first angle here as SError is unmasked either side of this,
unlike in the KVM example.

I'll apply the same logic to the KVM version in patch 20...



Thanks,

James



___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 12/21] arm64: kernel: Survive corrected RAS errors notified by SError

2017-11-02 Thread James Morse
Hi Will,

On 31/10/17 13:50, Will Deacon wrote:
> On Thu, Oct 19, 2017 at 03:57:58PM +0100, James Morse wrote:
>> Prior to v8.2, SError is an uncontainable fatal exception. The v8.2 RAS
>> extensions use SError to notify software about RAS errors, these can be
>> contained by the ESB instruction.
>>
>> An ACPI system with firmware-first may use SError as its 'SEI'
>> notification. Future patches may add code to 'claim' this SError as a
>> notification.
>>
>> Other systems can distinguish these RAS errors from the SError ESR and
>> use the AET bits and additional data from RAS-Error registers to handle
>> the error. Future patches may add this kernel-first handling.
>>
>> Without support for either of these we will panic(), even if we received
>> a corrected error. Add code to decode the severity of RAS errors. We can
>> safely ignore contained errors where the CPU can continue to make
>> progress. For all other errors we continue to panic().

>> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
>> index 66ed8b6b9976..8ea52f15bf1c 100644
>> --- a/arch/arm64/include/asm/esr.h
>> +++ b/arch/arm64/include/asm/esr.h
>> @@ -85,6 +85,15 @@
>>  #define ESR_ELx_WNR_SHIFT   (6)
>>  #define ESR_ELx_WNR (UL(1) << ESR_ELx_WNR_SHIFT)
>>  
>> +/* Asynchronous Error Type */
>> +#define ESR_ELx_AET (UL(0x7) << 10)

> Can you add a #define for the AET shift in the Srror ISS, please? (we have
> other blocks in this file for different abort types). e.g.
> 
> /* ISS fields definitions for SError interrupts */
> #define ESR_ELx_AER_SHIFT 10
> 
> then use it below.

Yes,  I should have done that..


>> +#define ESR_ELx_AET_UC  (UL(0) << 10)   /* Uncontainable */
>> +#define ESR_ELx_AET_UEU (UL(1) << 10)   /* Uncorrected 
>> Unrecoverable */
>> +#define ESR_ELx_AET_UEO (UL(2) << 10)   /* Uncorrected 
>> Restartable */
>> +#define ESR_ELx_AET_UER (UL(3) << 10)   /* Uncorrected 
>> Recoverable */
>> +#define ESR_ELx_AET_CE  (UL(6) << 10)   /* Corrected */
>> +
>>  /* Shared ISS field definitions for Data/Instruction aborts */
>>  #define ESR_ELx_SET_SHIFT   (11)
>>  #define ESR_ELx_SET_MASK(UL(3) << ESR_ELx_SET_SHIFT)
>> @@ -99,6 +108,7 @@
>>  #define ESR_ELx_FSC (0x3F)
>>  #define ESR_ELx_FSC_TYPE(0x3C)
>>  #define ESR_ELx_FSC_EXTABT  (0x10)
>> +#define ESR_ELx_FSC_SERROR  (0x11)
>>  #define ESR_ELx_FSC_ACCESS  (0x08)
>>  #define ESR_ELx_FSC_FAULT   (0x04)

>>  #define ESR_ELx_FSC_PERM(0x0C)
>> diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
>> index d131501c6222..8d2a1fff5c6b 100644
>> --- a/arch/arm64/include/asm/traps.h
>> +++ b/arch/arm64/include/asm/traps.h
>> @@ -19,6 +19,7 @@
>>  #define __ASM_TRAP_H
>>  
>>  #include 
>> +#include 
>>  #include 
>>  
>>  struct pt_regs;
>> @@ -58,4 +59,39 @@ static inline int in_entry_text(unsigned long ptr)
>>  return ptr >= (unsigned long)&__entry_text_start &&
>> ptr < (unsigned long)&__entry_text_end;
>>  }
>> +
>> +static inline bool arm64_is_ras_serror(u32 esr)
>> +{
>> +bool impdef = esr & ESR_ELx_ISV; /* aka IDS */
> 
> I think you should add an IDS field along with the AET one I suggested.

Sure,

>> +
>> +if (cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
>> +return !impdef;
>> +
>> +return false;
>> +}
>> +
>> +/* Return the AET bits of an SError ESR, or 0/uncontainable/uncategorized */
>> +static inline u32 arm64_ras_serror_get_severity(u32 esr)
>> +{
>> +u32 aet = esr & ESR_ELx_AET;
>> +
>> +if (!arm64_is_ras_serror(esr)) {
>> +/* Not a RAS error, we can't interpret the ESR */
>> +return 0;
>> +}
>> +
>> +/*
>> + * AET is RES0 if 'the value returned in the DFSC field is not
>> + * [ESR_ELx_FSC_SERROR]'
>> + */
>> +if ((esr & ESR_ELx_FSC) != ESR_ELx_FSC_SERROR) {
>> +/* No severity information */
>> +return 0;
>> +}

> Hmm, this means we can't distinguish impdef or RES0 encodings from
> uncontainable errors. Is that desirable?

We panic for for both impdef and uncontainable ESR values, so the difference
doesn't matter. I'll remove the 'is_ras_serror()' in here and make it the
callers problem to check...


RES0 encodings?
If this is an imp-def 'all zeros', those should all be matched as impdef by
arm64_is_ras_serror().
Otherwise its a RAS encoding with {I,D}FSC bits that indicate we can't know the
severity.
The ARM-ARM calls these 'uncategorized'. Yes I'm treating them as uncontained,
(on aarch32 these share an encoding). I'll add a comment to call it out.


> Also, could we end up in a situation where some CPUs support RAS and some
> don't, 

Ooer, differing CPU support. I hadn't considered that... wouldn't cpufeature
declare such a system insane?


> so arm64_is_ras_serror returns false yet a correctable error is
> reported by one the CPUs and we treat it as uncontainable?

Makeing the HAS_RAS tests use this_cpu

Re: [PATCH v4 09/21] KVM: arm/arm64: mask/unmask daif around VHE guests

2017-11-02 Thread James Morse
Hi Christoffer,

On 30/10/17 07:40, Christoffer Dall wrote:
> On Thu, Oct 19, 2017 at 03:57:55PM +0100, James Morse wrote:
>> Non-VHE systems take an exception to EL2 in order to world-switch into the
>> guest. When returning from the guest KVM implicitly restores the DAIF
>> flags when it returns to the kernel at EL1.
>>
>> With VHE none of this exception-level jumping happens, so KVMs
>> world-switch code is exposed to the host kernel's DAIF values, and KVM
>> spills the guest-exit DAIF values back into the host kernel.
>> On entry to a guest we have Debug and SError exceptions unmasked, KVM
>> has switched VBAR but isn't prepared to handle these. On guest exit
>> Debug exceptions are left disabled once we return to the host and will
>> stay this way until we enter user space.
>>
>> Add a helper to mask/unmask DAIF around VHE guests. The unmask can only
>> happen after the hosts VBAR value has been synchronised by the isb in
>> __vhe_hyp_call (via kvm_call_hyp()). Masking could be as late as
>> setting KVMs VBAR value, but is kept here for symmetry.

> Reviewed-by: Christoffer Dall 

Thanks!


>> ---
>> Give me a kick if you want this reworked as a fix (which will then
>> conflict with this series), or a backportable version.
> 
> I don't know of any real-world issues where some more graceful handling
> of SErrors would make sense on older kernels, so I'm fine with just
> merging this together with this series.

What about debug?
> On guest exit Debug exceptions are left disabled once we return to the host
> and will stay this way until we enter user space.

Today VHE:KVM causes the kernel to run with SError unmasked and debug disabled
until the next return to user-space, whereas previously the kernel expected
SError to be masked and debug enabled.


(Reposting just the SError rework without this patch changes the kernel to
expect SError to be unmasked, which isn't making this any worse.)


Thanks,

James
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 9/9] arm64: entry.S: move SError handling into a C function for future expansion

2017-11-02 Thread James Morse
From: Xie XiuQi 

Today SError is taken using the inv_entry macro that ends up in
bad_mode.

SError can be used by the RAS Extensions to notify either the OS or
firmware of CPU problems, some of which may have been corrected.

To allow this handling to be added, add a do_serror() C function
that just panic()s. Add the entry.S boiler plate to save/restore the
CPU registers and unmask debug exceptions. Future patches may change
do_serror() to return if the SError Interrupt was notification of a
corrected error.

Signed-off-by: Xie XiuQi 
Signed-off-by: Wang Xiongfeng 
[Split out of a bigger patch, added compat path, renamed, enabled debug
 exceptions]
Signed-off-by: James Morse 
Reviewed-by: Catalin Marinas 
---
 arch/arm64/Kconfig|  2 +-
 arch/arm64/kernel/entry.S | 36 +---
 arch/arm64/kernel/traps.c | 13 +
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0df64a6a56d4..70dfe4e9ccc5 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -98,7 +98,7 @@ config ARM64
select HAVE_IRQ_TIME_ACCOUNTING
select HAVE_MEMBLOCK
select HAVE_MEMBLOCK_NODE_MAP if NUMA
-   select HAVE_NMI if ACPI_APEI_SEA
+   select HAVE_NMI
select HAVE_PATA_PLATFORM
select HAVE_PERF_EVENTS
select HAVE_PERF_REGS
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index df085ec003b0..e147c1d00b41 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -375,18 +375,18 @@ ENTRY(vectors)
kernel_ventry   el1_sync// Synchronous EL1h
kernel_ventry   el1_irq // IRQ EL1h
kernel_ventry   el1_fiq_invalid // FIQ EL1h
-   kernel_ventry   el1_error_invalid   // Error EL1h
+   kernel_ventry   el1_error   // Error EL1h
 
kernel_ventry   el0_sync// Synchronous 64-bit 
EL0
kernel_ventry   el0_irq // IRQ 64-bit EL0
kernel_ventry   el0_fiq_invalid // FIQ 64-bit EL0
-   kernel_ventry   el0_error_invalid   // Error 64-bit EL0
+   kernel_ventry   el0_error   // Error 64-bit EL0
 
 #ifdef CONFIG_COMPAT
kernel_ventry   el0_sync_compat // Synchronous 32-bit 
EL0
kernel_ventry   el0_irq_compat  // IRQ 32-bit EL0
kernel_ventry   el0_fiq_invalid_compat  // FIQ 32-bit EL0
-   kernel_ventry   el0_error_invalid_compat// Error 32-bit EL0
+   kernel_ventry   el0_error_compat// Error 32-bit EL0
 #else
kernel_ventry   el0_sync_invalid// Synchronous 32-bit 
EL0
kernel_ventry   el0_irq_invalid // IRQ 32-bit EL0
@@ -455,10 +455,6 @@ ENDPROC(el0_error_invalid)
 el0_fiq_invalid_compat:
inv_entry 0, BAD_FIQ, 32
 ENDPROC(el0_fiq_invalid_compat)
-
-el0_error_invalid_compat:
-   inv_entry 0, BAD_ERROR, 32
-ENDPROC(el0_error_invalid_compat)
 #endif
 
 el1_sync_invalid:
@@ -663,6 +659,10 @@ el0_svc_compat:
 el0_irq_compat:
kernel_entry 0, 32
b   el0_irq_naked
+
+el0_error_compat:
+   kernel_entry 0, 32
+   b   el0_error_naked
 #endif
 
 el0_da:
@@ -780,6 +780,28 @@ el0_irq_naked:
b   ret_to_user
 ENDPROC(el0_irq)
 
+el1_error:
+   kernel_entry 1
+   mrs x1, esr_el1
+   enable_dbg
+   mov x0, sp
+   bl  do_serror
+   kernel_exit 1
+ENDPROC(el1_error)
+
+el0_error:
+   kernel_entry 0
+el0_error_naked:
+   mrs x1, esr_el1
+   enable_dbg
+   mov x0, sp
+   bl  do_serror
+   enable_daif
+   ct_user_exit
+   b   ret_to_user
+ENDPROC(el0_error)
+
+
 /*
  * This is the fast syscall return path.  We do as little as possible here,
  * and this includes saving x0 back into the kernel stack.
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1808be65d22f..773aae69c376 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -709,6 +709,19 @@ asmlinkage void handle_bad_stack(struct pt_regs *regs)
 }
 #endif
 
+asmlinkage void do_serror(struct pt_regs *regs, unsigned int esr)
+{
+   nmi_enter();
+
+   console_verbose();
+
+   pr_crit("SError Interrupt on CPU%d, code 0x%08x -- %s\n",
+   smp_processor_id(), esr, esr_get_class_string(esr));
+   __show_regs(regs);
+
+   panic("Asynchronous SError Interrupt");
+}
+
 void __pte_error(const char *file, int line, unsigned long val)
 {
pr_err("%s:%d: bad pte %016lx.\n", file, line, val);
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 7/9] arm64: entry.S convert el0_sync

2017-11-02 Thread James Morse
el0_sync also unmasks exceptions on a case-by-case basis, debug exceptions
are enabled, unless this was a debug exception. Irqs are unmasked for
some exception types but not for others.

el0_dbg should run with everything masked to prevent us taking a debug
exception from do_debug_exception. For the other cases we can unmask
everything. This changes the behaviour of fpsimd_{acc,exc} and el0_inv
which previously ran with irqs masked.

This patch removed the last user of enable_dbg_and_irq, remove it.

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 
---
 arch/arm64/include/asm/assembler.h |  9 -
 arch/arm64/kernel/entry.S  | 24 ++--
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index abb5abd61ddb..c2a37e2f733c 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -96,15 +96,6 @@
 9990:
.endm
 
-/*
- * Enable both debug exceptions and interrupts. This is likely to be
- * faster than two daifclr operations, since writes to this register
- * are self-synchronising.
- */
-   .macro  enable_dbg_and_irq
-   msr daifclr, #(8 | 2)
-   .endm
-
 /*
  * SMP data memory barrier
  */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index bd54115972a4..f7dfe5d2b1fb 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -670,8 +670,7 @@ el0_da:
 * Data abort handling
 */
mrs x26, far_el1
-   // enable interrupts before calling the main handler
-   enable_dbg_and_irq
+   enable_daif
ct_user_exit
clear_address_tag x0, x26
mov x1, x25
@@ -683,8 +682,7 @@ el0_ia:
 * Instruction abort handling
 */
mrs x26, far_el1
-   // enable interrupts before calling the main handler
-   enable_dbg_and_irq
+   enable_daif
ct_user_exit
mov x0, x26
mov x1, x25
@@ -695,7 +693,7 @@ el0_fpsimd_acc:
/*
 * Floating Point or Advanced SIMD access
 */
-   enable_dbg
+   enable_daif
ct_user_exit
mov x0, x25
mov x1, sp
@@ -705,7 +703,7 @@ el0_fpsimd_exc:
/*
 * Floating Point or Advanced SIMD exception
 */
-   enable_dbg
+   enable_daif
ct_user_exit
mov x0, x25
mov x1, sp
@@ -716,8 +714,7 @@ el0_sp_pc:
 * Stack or PC alignment exception handling
 */
mrs x26, far_el1
-   // enable interrupts before calling the main handler
-   enable_dbg_and_irq
+   enable_daif
ct_user_exit
mov x0, x26
mov x1, x25
@@ -728,8 +725,7 @@ el0_undef:
/*
 * Undefined instruction
 */
-   // enable interrupts before calling the main handler
-   enable_dbg_and_irq
+   enable_daif
ct_user_exit
mov x0, sp
bl  do_undefinstr
@@ -738,7 +734,7 @@ el0_sys:
/*
 * System instructions, for trapped cache maintenance instructions
 */
-   enable_dbg_and_irq
+   enable_daif
ct_user_exit
mov x0, x25
mov x1, sp
@@ -753,11 +749,11 @@ el0_dbg:
mov x1, x25
mov x2, sp
bl  do_debug_exception
-   enable_dbg
+   enable_daif
ct_user_exit
b   ret_to_user
 el0_inv:
-   enable_dbg
+   enable_daif
ct_user_exit
mov x0, sp
mov x1, #BAD_SYNC
@@ -836,7 +832,7 @@ el0_svc:
mov wsc_nr, #__NR_syscalls
 el0_svc_naked: // compat entry point
stp x0, xscno, [sp, #S_ORIG_X0] // save the original x0 and 
syscall number
-   enable_dbg_and_irq
+   enable_daif
ct_user_exit 1
 
ldr x16, [tsk, #TSK_TI_FLAGS]   // check for syscall hooks
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 8/9] arm64: entry.S: convert elX_irq

2017-11-02 Thread James Morse
Following our 'dai' order, irqs should be processed with debug and
serror exceptions unmasked.

Add a helper to unmask these two, (and fiq for good measure).

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 

==
Changes since v3:
* Added comment against enable_da_f
---
 arch/arm64/include/asm/assembler.h | 5 +
 arch/arm64/kernel/entry.S  | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index c2a37e2f733c..e4ac505b7b3d 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -54,6 +54,11 @@
msr daif, \tmp
.endm
 
+   /* IRQ is the lowest priority flag, unconditionally unmask the rest. */
+   .macro enable_da_f
+   msr daifclr, #(8 | 4 | 1)
+   .endm
+
 /*
  * Enable and disable interrupts.
  */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index f7dfe5d2b1fb..df085ec003b0 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -557,7 +557,7 @@ ENDPROC(el1_sync)
.align  6
 el1_irq:
kernel_entry 1
-   enable_dbg
+   enable_da_f
 #ifdef CONFIG_TRACE_IRQFLAGS
bl  trace_hardirqs_off
 #endif
@@ -766,7 +766,7 @@ ENDPROC(el0_sync)
 el0_irq:
kernel_entry 0
 el0_irq_naked:
-   enable_dbg
+   enable_da_f
 #ifdef CONFIG_TRACE_IRQFLAGS
bl  trace_hardirqs_off
 #endif
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 6/9] arm64: entry.S: convert el1_sync

2017-11-02 Thread James Morse
el1_sync unmasks exceptions on a case-by-case basis, debug exceptions
are unmasked, unless this was a debug exception. IRQs are unmasked
for instruction and data aborts only if the interupted context had
irqs unmasked.

Following our 'dai' order, el1_dbg should run with everything masked.
For the other cases we can inherit whatever we interrupted.

Add a macro inherit_daif to set daif based on the interrupted pstate.

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 
---
 arch/arm64/include/asm/assembler.h |  6 ++
 arch/arm64/kernel/entry.S  | 12 
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index 1b0972046a72..abb5abd61ddb 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -48,6 +48,12 @@
msr daif, \flags
.endm
 
+   /* Only on aarch64 pstate, PSR_D_BIT is different for aarch32 */
+   .macro  inherit_daif, pstate:req, tmp:req
+   and \tmp, \pstate, #(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
+   msr daif, \tmp
+   .endm
+
 /*
  * Enable and disable interrupts.
  */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index f7d7bf9d76e7..bd54115972a4 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -510,11 +510,7 @@ el1_da:
 * Data abort handling
 */
mrs x3, far_el1
-   enable_dbg
-   // re-enable interrupts if they were enabled in the aborted context
-   tbnzx23, #7, 1f // PSR_I_BIT
-   enable_irq
-1:
+   inherit_daifpstate=x23, tmp=x2
clear_address_tag x0, x3
mov x2, sp  // struct pt_regs
bl  do_mem_abort
@@ -525,7 +521,7 @@ el1_sp_pc:
 * Stack or PC alignment exception handling
 */
mrs x0, far_el1
-   enable_dbg
+   inherit_daifpstate=x23, tmp=x2
mov x2, sp
bl  do_sp_pc_abort
ASM_BUG()
@@ -533,7 +529,7 @@ el1_undef:
/*
 * Undefined instruction
 */
-   enable_dbg
+   inherit_daifpstate=x23, tmp=x2
mov x0, sp
bl  do_undefinstr
ASM_BUG()
@@ -550,7 +546,7 @@ el1_dbg:
kernel_exit 1
 el1_inv:
// TODO: add support for undefined instructions in kernel mode
-   enable_dbg
+   inherit_daifpstate=x23, tmp=x2
mov x0, sp
mov x2, x1
mov x1, #BAD_SYNC
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 4/9] arm64: Mask all exceptions during kernel_exit

2017-11-02 Thread James Morse
To take RAS Exceptions as quickly as possible we need to keep SError
unmasked as much as possible. We need to mask it during kernel_exit
as taking an error from this code will overwrite the exception-registers.

Adding a naked 'disable_daif' to kernel_exit causes a performance problem
for micro-benchmarks that do no real work, (e.g. calling getpid() in a
loop). This is because the ret_to_user loop has already masked IRQs so
that the TIF_WORK_MASK thread flags can't change underneath it, adding
disable_daif is an additional self-synchronising operation.

In the future, the RAS APEI code may need to modify the TIF_WORK_MASK
flags from an SError, in which case the ret_to_user loop must mask SError
while it examines the flags.

Disable all exceptions for return to EL1. For return to EL0 get the
ret_to_user loop to leave all exceptions masked once it has done its
work, this avoids an extra pstate-write.

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 

==
Changes since v3:
* swapped verb/daif word-order.
---
 arch/arm64/kernel/entry.S  | 10 +-
 arch/arm64/kernel/signal.c |  8 ++--
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index e1c59d4008a8..f7d7bf9d76e7 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -221,6 +221,8 @@ alternative_else_nop_endif
 
.macro  kernel_exit, el
.if \el != 0
+   disable_daif
+
/* Restore the task's original addr_limit. */
ldr x20, [sp, #S_ORIG_ADDR_LIMIT]
str x20, [tsk, #TSK_TI_ADDR_LIMIT]
@@ -517,8 +519,6 @@ el1_da:
mov x2, sp  // struct pt_regs
bl  do_mem_abort
 
-   // disable interrupts before pulling preserved data off the stack
-   disable_irq
kernel_exit 1
 el1_sp_pc:
/*
@@ -793,7 +793,7 @@ ENDPROC(el0_irq)
  * and this includes saving x0 back into the kernel stack.
  */
 ret_fast_syscall:
-   disable_irq // disable interrupts
+   disable_daif
str x0, [sp, #S_X0] // returned x0
ldr x1, [tsk, #TSK_TI_FLAGS]// re-check for syscall tracing
and x2, x1, #_TIF_SYSCALL_WORK
@@ -803,7 +803,7 @@ ret_fast_syscall:
enable_step_tsk x1, x2
kernel_exit 0
 ret_fast_syscall_trace:
-   enable_irq  // enable interrupts
+   enable_daif
b   __sys_trace_return_skipped  // we already saved x0
 
 /*
@@ -821,7 +821,7 @@ work_pending:
  * "slow" syscall return path.
  */
 ret_to_user:
-   disable_irq // disable interrupts
+   disable_daif
ldr x1, [tsk, #TSK_TI_FLAGS]
and x2, x1, #_TIF_WORK_MASK
cbnzx2, work_pending
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 0bdc96c61bc0..8e6500c9471b 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -756,9 +757,12 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
addr_limit_user_check();
 
if (thread_flags & _TIF_NEED_RESCHED) {
+   /* Unmask Debug and SError for the next task */
+   local_daif_restore(DAIF_PROCCTX_NOIRQ);
+
schedule();
} else {
-   local_irq_enable();
+   local_daif_restore(DAIF_PROCCTX);
 
if (thread_flags & _TIF_UPROBE)
uprobe_notify_resume(regs);
@@ -775,7 +779,7 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
fpsimd_restore_current_state();
}
 
-   local_irq_disable();
+   local_daif_mask();
thread_flags = READ_ONCE(current_thread_info()->flags);
} while (thread_flags & _TIF_WORK_MASK);
 }
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 3/9] arm64: Move the async/fiq helpers to explicitly set process context flags

2017-11-02 Thread James Morse
Remove the local_{async,fiq}_{en,dis}able macros as they don't respect
our newly defined order and are only used to set the flags for process
context when we bring CPUs online.

Add a helper to do this. The IRQ flag varies as we want it masked on
the boot CPU until we are ready to handle interrupts.
The boot CPU unmasks SError during early boot once it can print an error
message. If we can print an error message about SError, we can do the
same for FIQ. Debug exceptions are already enabled by __cpu_setup(),
which has also configured MDSCR_EL1 to disable MDE and KDE.

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 

==
Changes since v3:
 * swapped verb/daif word-order.
---
 arch/arm64/include/asm/daifflags.h | 3 +++
 arch/arm64/include/asm/irqflags.h  | 6 --
 arch/arm64/kernel/setup.c  | 8 +---
 arch/arm64/kernel/smp.c| 3 +--
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/daifflags.h 
b/arch/arm64/include/asm/daifflags.h
index 55e2598a8c4c..22e4c83de5a5 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -18,6 +18,9 @@
 
 #include 
 
+#define DAIF_PROCCTX   0
+#define DAIF_PROCCTX_NOIRQ PSR_I_BIT
+
 /* mask/save/unmask/restore all exceptions, including interrupts. */
 static inline void local_daif_mask(void)
 {
diff --git a/arch/arm64/include/asm/irqflags.h 
b/arch/arm64/include/asm/irqflags.h
index 9ecdca7011f0..24692edf1a69 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -66,12 +66,6 @@ static inline void arch_local_irq_disable(void)
: "memory");
 }
 
-#define local_fiq_enable() asm("msrdaifclr, #1" : : : "memory")
-#define local_fiq_disable()asm("msrdaifset, #1" : : : "memory")
-
-#define local_async_enable()   asm("msrdaifclr, #4" : : : "memory")
-#define local_async_disable()  asm("msrdaifset, #4" : : : "memory")
-
 /*
  * Save the current interrupt enable state.
  */
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index d4b740538ad5..ad285f024934 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -262,10 +263,11 @@ void __init setup_arch(char **cmdline_p)
parse_early_param();
 
/*
-*  Unmask asynchronous aborts after bringing up possible earlycon.
-* (Report possible System Errors once we can report this occurred)
+* Unmask asynchronous aborts and fiq after bringing up possible
+* earlycon. (Report possible System Errors once we can report this
+* occurred).
 */
-   local_async_enable();
+   local_daif_restore(DAIF_PROCCTX_NOIRQ);
 
/*
 * TTBR0 is only used for the identity mapping at this stage. Make it
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 5a407eba01f7..d92e03faa51a 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -272,8 +272,7 @@ asmlinkage void secondary_start_kernel(void)
set_cpu_online(cpu, true);
complete(&cpu_running);
 
-   local_irq_enable();
-   local_async_enable();
+   local_daif_restore(DAIF_PROCCTX);
 
/*
 * OK, it's off to the idle thread for us
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 2/9] arm64: introduce an order for exceptions

2017-11-02 Thread James Morse
Currently SError is always masked in the kernel. To support RAS exceptions
using SError on hardware with the v8.2 RAS Extensions we need to unmask
SError as much as possible.

Let's define an order for masking and unmasking exceptions. 'dai' is
memorable and effectively what we have today.

Disabling debug exceptions should cause all other exceptions to be masked.
Masking SError should mask irq, but not disable debug exceptions.
Masking irqs has no side effects for other flags. Keeping to this order
makes it easier for entry.S to know which exceptions should be unmasked.

FIQ is never expected, but we mask it when we mask debug exceptions, and
unmask it at all other times.

Given masking debug exceptions masks everything, we don't need macros
to save/restore that bit independently. Remove them and switch the last
caller over to use the daif calls.

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 

==
Changes since v3:
 * Use save_daif version, swapped verb/daif word-order.
 * tweak last sentence of commit message.
---
 arch/arm64/include/asm/irqflags.h  | 34 +-
 arch/arm64/kernel/debug-monitors.c |  5 +++--
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/include/asm/irqflags.h 
b/arch/arm64/include/asm/irqflags.h
index 8c581281fa12..9ecdca7011f0 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -20,6 +20,19 @@
 
 #include 
 
+/*
+ * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
+ * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
+ * order:
+ * Masking debug exceptions causes all other exceptions to be masked too/
+ * Masking SError masks irq, but not debug exceptions. Masking irqs has no
+ * side effects for other flags. Keeping to this order makes it easier for
+ * entry.S to know which exceptions should be unmasked.
+ *
+ * FIQ is never expected, but we mask it when we disable debug exceptions, and
+ * unmask it at all other times.
+ */
+
 /*
  * CPU interrupt mask handling.
  */
@@ -89,26 +102,5 @@ static inline int arch_irqs_disabled_flags(unsigned long 
flags)
 {
return flags & PSR_I_BIT;
 }
-
-/*
- * save and restore debug state
- */
-#define local_dbg_save(flags)  \
-   do {\
-   typecheck(unsigned long, flags);\
-   asm volatile(   \
-   "mrs%0, daif// local_dbg_save\n"\
-   "msrdaifset, #8"\
-   : "=r" (flags) : : "memory");   \
-   } while (0)
-
-#define local_dbg_restore(flags)   \
-   do {\
-   typecheck(unsigned long, flags);\
-   asm volatile(   \
-   "msrdaif, %0// local_dbg_restore\n" \
-   : : "r" (flags) : "memory");\
-   } while (0)
-
 #endif
 #endif
diff --git a/arch/arm64/kernel/debug-monitors.c 
b/arch/arm64/kernel/debug-monitors.c
index c7ef99904934..a88b6ccebbb4 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -30,6 +30,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -46,9 +47,9 @@ u8 debug_monitors_arch(void)
 static void mdscr_write(u32 mdscr)
 {
unsigned long flags;
-   local_dbg_save(flags);
+   flags = local_daif_save();
write_sysreg(mdscr, mdscr_el1);
-   local_dbg_restore(flags);
+   local_daif_restore(flags);
 }
 NOKPROBE_SYMBOL(mdscr_write);
 
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 5/9] arm64: entry.S: Remove disable_dbg

2017-11-02 Thread James Morse
enable_step_tsk is the only user of disable_dbg, which doesn't respect
our 'dai' order for exception masking. enable_step_tsk may enable
single-step, so previously needed to mask debug exceptions to prevent us
from single-stepping kernel_exit. enable_step_tsk is called at the end
of the ret_to_user loop, which has already masked all exceptions so this
is no longer needed.

Remove disable_dbg, add a comment that enable_step_tsk's caller should
have masked debug.

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 
---
 arch/arm64/include/asm/assembler.h | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index 114e741ca873..1b0972046a72 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -68,13 +68,6 @@
msr daif, \flags
.endm
 
-/*
- * Enable and disable debug exceptions.
- */
-   .macro  disable_dbg
-   msr daifset, #8
-   .endm
-
.macro  enable_dbg
msr daifclr, #8
.endm
@@ -88,9 +81,9 @@
 9990:
.endm
 
+   /* call with daif masked */
.macro  enable_step_tsk, flgs, tmp
tbz \flgs, #TIF_SINGLESTEP, 9990f
-   disable_dbg
mrs \tmp, mdscr_el1
orr \tmp, \tmp, #1
msr mdscr_el1, \tmp
-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[RESEND PATCH v4 1/9] arm64: explicitly mask all exceptions

2017-11-02 Thread James Morse
There are a few places where we want to mask all exceptions. Today we
do this in a piecemeal fashion, typically we expect the caller to
have masked irqs and the arch code masks debug exceptions, ignoring
serror which is probably masked.

Make it clear that 'mask all exceptions' is the intention by adding
helpers to do exactly that.

This will let us unmask SError without having to add 'oh and SError'
to these paths.

Signed-off-by: James Morse 
Reviewed-by: Julien Thierry 
Reviewed-by: Catalin Marinas 

==
Remove the disable IRQs comment above cpu_die(): we return from idle via
cpu_resume. This comment is confusing once the local_irq_disable() call
is changed.

Changes since v3:
 * Split local_mask_daif into a save and mask versions,
 * swapped verb/daif word-order.
 * Removed {asm,linux} includes - one will always include the other
---
 arch/arm64/include/asm/assembler.h | 17 ++
 arch/arm64/include/asm/daifflags.h | 69 ++
 arch/arm64/kernel/hibernate.c  |  5 +--
 arch/arm64/kernel/machine_kexec.c  |  4 +--
 arch/arm64/kernel/smp.c|  9 ++---
 arch/arm64/kernel/suspend.c|  7 ++--
 arch/arm64/kernel/traps.c  |  3 +-
 arch/arm64/mm/proc.S   |  9 +++--
 8 files changed, 104 insertions(+), 19 deletions(-)
 create mode 100644 arch/arm64/include/asm/daifflags.h

diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index d58a6253c6ab..114e741ca873 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -31,6 +31,23 @@
 #include 
 #include 
 
+   .macro save_and_disable_daif, flags
+   mrs \flags, daif
+   msr daifset, #0xf
+   .endm
+
+   .macro disable_daif
+   msr daifset, #0xf
+   .endm
+
+   .macro enable_daif
+   msr daifclr, #0xf
+   .endm
+
+   .macro  restore_daif, flags:req
+   msr daif, \flags
+   .endm
+
 /*
  * Enable and disable interrupts.
  */
diff --git a/arch/arm64/include/asm/daifflags.h 
b/arch/arm64/include/asm/daifflags.h
new file mode 100644
index ..55e2598a8c4c
--- /dev/null
+++ b/arch/arm64/include/asm/daifflags.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2017 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+#ifndef __ASM_DAIFFLAGS_H
+#define __ASM_DAIFFLAGS_H
+
+#include 
+
+/* mask/save/unmask/restore all exceptions, including interrupts. */
+static inline void local_daif_mask(void)
+{
+   asm volatile(
+   "msrdaifset, #0xf   // local_daif_mask\n"
+   :
+   :
+   : "memory");
+   trace_hardirqs_off();
+}
+
+static inline unsigned long local_daif_save(void)
+{
+   unsigned long flags;
+
+   asm volatile(
+   "mrs%0, daif// local_daif_save\n"
+   : "=r" (flags)
+   :
+   : "memory");
+   local_daif_mask();
+
+   return flags;
+}
+
+static inline void local_daif_unmask(void)
+{
+   trace_hardirqs_on();
+   asm volatile(
+   "msrdaifclr, #0xf   // local_daif_unmask"
+   :
+   :
+   : "memory");
+}
+
+static inline void local_daif_restore(unsigned long flags)
+{
+   if (!arch_irqs_disabled_flags(flags))
+   trace_hardirqs_on();
+   asm volatile(
+   "msrdaif, %0// local_daif_restore"
+   :
+   : "r" (flags)
+   : "memory");
+   if (arch_irqs_disabled_flags(flags))
+   trace_hardirqs_off();
+}
+
+#endif
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 095d3c170f5d..3009b8b80f08 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -285,7 +286,7 @@ int swsusp_arch_suspend(void)
return -EBUSY;
}
 
-   local_dbg_save(flags);
+   flags = local_daif_save();
 
if (__cpu_suspend_enter(&state)) {
/* make the crash dump kernel image visible/saveable */
@@ -315,7 +316,7 @@ int swsusp_arch_suspend(void)
__cpu_suspend_exit();
}
 
-   local_dbg_restore(flags);
+   local_daif_restore(flags);
 
return ret;
 }
diff --git a/arch/arm64/kernel/machine_kexec.c 
b/a

[RESEND PATCH v4 0/9] SError rework (- RAS & IESB for firmware first support)

2017-11-02 Thread James Morse
Hello,

This is repost of the SError Rework bits of the bigger series [0].
None of these patches have changed since v3.

Thanks,

James


[0] https://www.spinics.net/lists/arm-kernel/msg612870.html


James Morse (8):
  arm64: explicitly mask all exceptions
  arm64: introduce an order for exceptions
  arm64: Move the async/fiq helpers to explicitly set process context
flags
  arm64: Mask all exceptions during kernel_exit
  arm64: entry.S: Remove disable_dbg
  arm64: entry.S: convert el1_sync
  arm64: entry.S convert el0_sync
  arm64: entry.S: convert elX_irq

Xie XiuQi (1):
  arm64: entry.S: move SError handling into a C function for future
expansion

 arch/arm64/Kconfig |  2 +-
 arch/arm64/include/asm/assembler.h | 46 
 arch/arm64/include/asm/daifflags.h | 72 +++
 arch/arm64/include/asm/irqflags.h  | 40 ++
 arch/arm64/kernel/debug-monitors.c |  5 ++-
 arch/arm64/kernel/entry.S  | 86 ++
 arch/arm64/kernel/hibernate.c  |  5 ++-
 arch/arm64/kernel/machine_kexec.c  |  4 +-
 arch/arm64/kernel/setup.c  |  8 ++--
 arch/arm64/kernel/signal.c |  8 +++-
 arch/arm64/kernel/smp.c| 12 ++
 arch/arm64/kernel/suspend.c|  7 ++--
 arch/arm64/kernel/traps.c  | 16 ++-
 arch/arm64/mm/proc.S   |  9 ++--
 14 files changed, 211 insertions(+), 109 deletions(-)
 create mode 100644 arch/arm64/include/asm/daifflags.h

-- 
2.15.0.rc2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v5 04/30] arm64: KVM: Hide unsupported AArch64 CPU features from guests

2017-11-02 Thread Dave Martin
On Thu, Nov 02, 2017 at 09:15:57AM +0100, Christoffer Dall wrote:
> On Wed, Nov 01, 2017 at 10:26:03AM +, Dave Martin wrote:
> > On Wed, Nov 01, 2017 at 05:47:29AM +0100, Christoffer Dall wrote:
> > > On Tue, Oct 31, 2017 at 03:50:56PM +, Dave Martin wrote:
> > > > Currently, a guest kernel sees the true CPU feature registers
> > > > (ID_*_EL1) when it reads them using MRS instructions.  This means
> > > > that the guest may observe features that are present in the
> > > > hardware but the host doesn't understand or doesn't provide support
> > > > for.  A guest may legimitately try to use such a feature as per the
> > > > architecture, but use of the feature may trap instead of working
> > > > normally, triggering undef injection into the guest.
> > > > 
> > > > This is not a problem for the host, but the guest may go wrong when
> > > > running on newer hardware than the host knows about.
> > > > 
> > > > This patch hides from guest VMs any AArch64-specific CPU features
> > > > that the host doesn't support, by exposing to the guest the
> > > > sanitised versions of the registers computed by the cpufeatures
> > > > framework, instead of the true hardware registers.  To achieve
> > > > this, HCR_EL2.TID3 is now set for AArch64 guests, and emulation
> > > > code is added to KVM to report the sanitised versions of the
> > > > affected registers in response to MRS and register reads from
> > > > userspace.
> > > > 
> > > > The affected registers are removed from invariant_sys_regs[] (since
> > > > the invariant_sys_regs handling is no longer quite correct for
> > > > them) and added to sys_reg_desgs[], with appropriate access(),
> > > > get_user() and set_user() methods.  No runtime vcpu storage is
> > > > allocated for the registers: instead, they are read on demand from
> > > > the cpufeatures framework.  This may need modification in the
> > > > future if there is a need for userspace to customise the features
> > > > visible to the guest.
> > > > 
> > > > Attempts by userspace to write the registers are handled similarly
> > > > to the current invariant_sys_regs handling: writes are permitted,
> > > > but only if they don't attempt to change the value.  This is
> > > > sufficient to support VM snapshot/restore from userspace.
> > > > 
> > > > Because of the additional registers, restoring a VM on an older
> > > > kernel may not work unless userspace knows how to handle the extra
> > > > VM registers exposed to the KVM user ABI by this patch.
> > > > 
> > > > Under the principle of least damage, this patch makes no attempt to
> > > > handle any of the other registers currently in
> > > > invariant_sys_regs[], or to emulate registers for AArch32: however,
> > > > these could be handled in a similar way in future, as necessary.
> > > > 
> > > > Signed-off-by: Dave Martin 
> > > > Reviewed-by: Marc Zyngier 
> > > > Acked-by: Catalin Marinas 
> > > > Cc: Christoffer Dall 
> > > > ---
> > > >  arch/arm64/include/asm/sysreg.h |   3 +
> > > >  arch/arm64/kvm/hyp/switch.c |   6 +
> > > >  arch/arm64/kvm/sys_regs.c   | 282 
> > > > +---
> > > >  3 files changed, 246 insertions(+), 45 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/include/asm/sysreg.h 
> > > > b/arch/arm64/include/asm/sysreg.h
> > > > index 4dceb12..609d59af 100644
> > > > --- a/arch/arm64/include/asm/sysreg.h
> > > > +++ b/arch/arm64/include/asm/sysreg.h
> > > > @@ -149,6 +149,9 @@
> > > >  #define SYS_ID_AA64DFR0_EL1sys_reg(3, 0, 0, 5, 0)
> > > >  #define SYS_ID_AA64DFR1_EL1sys_reg(3, 0, 0, 5, 1)
> > > >  
> > > > +#define SYS_ID_AA64AFR0_EL1sys_reg(3, 0, 0, 5, 4)
> > > > +#define SYS_ID_AA64AFR1_EL1sys_reg(3, 0, 0, 5, 5)
> > > > +
> > > >  #define SYS_ID_AA64ISAR0_EL1   sys_reg(3, 0, 0, 6, 0)
> > > >  #define SYS_ID_AA64ISAR1_EL1   sys_reg(3, 0, 0, 6, 1)
> > > >  
> > > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > > index 945e79c..35a90b8 100644
> > > > --- a/arch/arm64/kvm/hyp/switch.c
> > > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > > @@ -81,11 +81,17 @@ static void __hyp_text __activate_traps(struct 
> > > > kvm_vcpu *vcpu)
> > > >  * it will cause an exception.
> > > >  */
> > > > val = vcpu->arch.hcr_el2;
> > > > +
> > > > if (!(val & HCR_RW) && system_supports_fpsimd()) {
> > > > write_sysreg(1 << 30, fpexc32_el2);
> > > > isb();
> > > > }
> > > > +
> > > > +   if (val & HCR_RW) /* for AArch64 only: */
> > > > +   val |= HCR_TID3; /* TID3: trap feature register 
> > > > accesses */
> > > > +
> > > 
> > > I still think we should set this in vcpu_reset_hcr() and do it once
> > > instead of adding code in every iteration of the critical path.
> > 
> > Dang, I somehow missed this in the last round.
> > 
> > How about:
> > 
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> > b/arch/arm64/include/asm/

Re: [PATCH v3 6/9] KVM: arm/arm64: Limit icache invalidation to prefetch aborts

2017-11-02 Thread Marc Zyngier
On Wed, Nov 01 2017 at 11:17:27 am GMT, Andrew Jones  wrote:
> On Mon, Oct 23, 2017 at 05:11:19PM +0100, Marc Zyngier wrote:
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index 2174244f6317..0417c8e2a81c 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -1292,7 +1292,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
>> phys_addr_t fault_ipa,
>>unsigned long fault_status)
>>  {
>>  int ret;
>> -bool write_fault, writable, hugetlb = false, force_pte = false;
>> +bool write_fault, exec_fault, writable, hugetlb = false, force_pte = 
>> false;
>>  unsigned long mmu_seq;
>>  gfn_t gfn = fault_ipa >> PAGE_SHIFT;
>>  struct kvm *kvm = vcpu->kvm;
>> @@ -1304,7 +1304,10 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
>> phys_addr_t fault_ipa,
>>  unsigned long flags = 0;
>>  
>>  write_fault = kvm_is_write_fault(vcpu);
>> -if (fault_status == FSC_PERM && !write_fault) {
>> +exec_fault = kvm_vcpu_trap_is_iabt(vcpu);
>> +VM_BUG_ON(write_fault && exec_fault);
>
> This VM_BUG_ON can never fire as long as kvm_is_write_fault() is
> defined as
>
>  {
>if (kvm_vcpu_trap_is_iabt(vcpu))
>return false;
>return kvm_vcpu_dabt_iswrite(vcpu);
>  }

That's indeed what I expect. But given that the code now relies on this
property, I chose to make it explicit.

Or are you seeing a better way of making this an invariant?

Thanks,

M.
-- 
Jazz is not dead, it just smell funny.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v5 04/30] arm64: KVM: Hide unsupported AArch64 CPU features from guests

2017-11-02 Thread Dave Martin
On Thu, Nov 02, 2017 at 09:15:57AM +0100, Christoffer Dall wrote:
> On Wed, Nov 01, 2017 at 10:26:03AM +, Dave Martin wrote:
> > On Wed, Nov 01, 2017 at 05:47:29AM +0100, Christoffer Dall wrote:
> > > On Tue, Oct 31, 2017 at 03:50:56PM +, Dave Martin wrote:
> > > > Currently, a guest kernel sees the true CPU feature registers
> > > > (ID_*_EL1) when it reads them using MRS instructions.  This means
> > > > that the guest may observe features that are present in the
> > > > hardware but the host doesn't understand or doesn't provide support
> > > > for.  A guest may legimitately try to use such a feature as per the
> > > > architecture, but use of the feature may trap instead of working
> > > > normally, triggering undef injection into the guest.
> > > > 
> > > > This is not a problem for the host, but the guest may go wrong when
> > > > running on newer hardware than the host knows about.
> > > > 
> > > > This patch hides from guest VMs any AArch64-specific CPU features
> > > > that the host doesn't support, by exposing to the guest the
> > > > sanitised versions of the registers computed by the cpufeatures
> > > > framework, instead of the true hardware registers.  To achieve
> > > > this, HCR_EL2.TID3 is now set for AArch64 guests, and emulation
> > > > code is added to KVM to report the sanitised versions of the
> > > > affected registers in response to MRS and register reads from
> > > > userspace.
> > > > 
> > > > The affected registers are removed from invariant_sys_regs[] (since
> > > > the invariant_sys_regs handling is no longer quite correct for
> > > > them) and added to sys_reg_desgs[], with appropriate access(),
> > > > get_user() and set_user() methods.  No runtime vcpu storage is
> > > > allocated for the registers: instead, they are read on demand from
> > > > the cpufeatures framework.  This may need modification in the
> > > > future if there is a need for userspace to customise the features
> > > > visible to the guest.
> > > > 
> > > > Attempts by userspace to write the registers are handled similarly
> > > > to the current invariant_sys_regs handling: writes are permitted,
> > > > but only if they don't attempt to change the value.  This is
> > > > sufficient to support VM snapshot/restore from userspace.
> > > > 
> > > > Because of the additional registers, restoring a VM on an older
> > > > kernel may not work unless userspace knows how to handle the extra
> > > > VM registers exposed to the KVM user ABI by this patch.
> > > > 
> > > > Under the principle of least damage, this patch makes no attempt to
> > > > handle any of the other registers currently in
> > > > invariant_sys_regs[], or to emulate registers for AArch32: however,
> > > > these could be handled in a similar way in future, as necessary.
> > > > 
> > > > Signed-off-by: Dave Martin 
> > > > Reviewed-by: Marc Zyngier 
> > > > Acked-by: Catalin Marinas 
> > > > Cc: Christoffer Dall 
> > > > ---
> > > >  arch/arm64/include/asm/sysreg.h |   3 +
> > > >  arch/arm64/kvm/hyp/switch.c |   6 +
> > > >  arch/arm64/kvm/sys_regs.c   | 282 
> > > > +---
> > > >  3 files changed, 246 insertions(+), 45 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/include/asm/sysreg.h 
> > > > b/arch/arm64/include/asm/sysreg.h
> > > > index 4dceb12..609d59af 100644
> > > > --- a/arch/arm64/include/asm/sysreg.h
> > > > +++ b/arch/arm64/include/asm/sysreg.h
> > > > @@ -149,6 +149,9 @@
> > > >  #define SYS_ID_AA64DFR0_EL1sys_reg(3, 0, 0, 5, 0)
> > > >  #define SYS_ID_AA64DFR1_EL1sys_reg(3, 0, 0, 5, 1)
> > > >  
> > > > +#define SYS_ID_AA64AFR0_EL1sys_reg(3, 0, 0, 5, 4)
> > > > +#define SYS_ID_AA64AFR1_EL1sys_reg(3, 0, 0, 5, 5)
> > > > +
> > > >  #define SYS_ID_AA64ISAR0_EL1   sys_reg(3, 0, 0, 6, 0)
> > > >  #define SYS_ID_AA64ISAR1_EL1   sys_reg(3, 0, 0, 6, 1)
> > > >  
> > > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > > index 945e79c..35a90b8 100644
> > > > --- a/arch/arm64/kvm/hyp/switch.c
> > > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > > @@ -81,11 +81,17 @@ static void __hyp_text __activate_traps(struct 
> > > > kvm_vcpu *vcpu)
> > > >  * it will cause an exception.
> > > >  */
> > > > val = vcpu->arch.hcr_el2;
> > > > +
> > > > if (!(val & HCR_RW) && system_supports_fpsimd()) {
> > > > write_sysreg(1 << 30, fpexc32_el2);
> > > > isb();
> > > > }
> > > > +
> > > > +   if (val & HCR_RW) /* for AArch64 only: */
> > > > +   val |= HCR_TID3; /* TID3: trap feature register 
> > > > accesses */
> > > > +
> > > 
> > > I still think we should set this in vcpu_reset_hcr() and do it once
> > > instead of adding code in every iteration of the critical path.
> > 
> > Dang, I somehow missed this in the last round.
> > 
> > How about:
> > 
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> > b/arch/arm64/include/asm/

Re: [PATCH v1 1/3] arm64: add a macro for SError synchronization

2017-11-02 Thread gengdongjiu

On 2017/11/1 22:16, Mark Rutland wrote:
>> it will report Error.
> Alternatives cannot be nested. You need to define a cap like:
> 
>   ARM64_HAS_RAS_NOT_IESB
> 
> ... which is set when ARM64_HAS_RAS_EXTN && !ARM64_HAS_IESB.
> 
> Then you can do:
> 
> alternative_if ARM64_HAS_RAS_NOT_IESB
>   esb
> alternative_else_nop_endif
> 
> See ARM64_ALT_PAN_NOT_UAO for an example.
> 
> That said, as Robin points out we may not even need the alternative.

Ok, got it. thank you very much for your point and opinion


> 
> Thanks,
> Mark.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v5 04/30] arm64: KVM: Hide unsupported AArch64 CPU features from guests

2017-11-02 Thread Christoffer Dall
On Wed, Nov 01, 2017 at 10:26:03AM +, Dave Martin wrote:
> On Wed, Nov 01, 2017 at 05:47:29AM +0100, Christoffer Dall wrote:
> > On Tue, Oct 31, 2017 at 03:50:56PM +, Dave Martin wrote:
> > > Currently, a guest kernel sees the true CPU feature registers
> > > (ID_*_EL1) when it reads them using MRS instructions.  This means
> > > that the guest may observe features that are present in the
> > > hardware but the host doesn't understand or doesn't provide support
> > > for.  A guest may legimitately try to use such a feature as per the
> > > architecture, but use of the feature may trap instead of working
> > > normally, triggering undef injection into the guest.
> > > 
> > > This is not a problem for the host, but the guest may go wrong when
> > > running on newer hardware than the host knows about.
> > > 
> > > This patch hides from guest VMs any AArch64-specific CPU features
> > > that the host doesn't support, by exposing to the guest the
> > > sanitised versions of the registers computed by the cpufeatures
> > > framework, instead of the true hardware registers.  To achieve
> > > this, HCR_EL2.TID3 is now set for AArch64 guests, and emulation
> > > code is added to KVM to report the sanitised versions of the
> > > affected registers in response to MRS and register reads from
> > > userspace.
> > > 
> > > The affected registers are removed from invariant_sys_regs[] (since
> > > the invariant_sys_regs handling is no longer quite correct for
> > > them) and added to sys_reg_desgs[], with appropriate access(),
> > > get_user() and set_user() methods.  No runtime vcpu storage is
> > > allocated for the registers: instead, they are read on demand from
> > > the cpufeatures framework.  This may need modification in the
> > > future if there is a need for userspace to customise the features
> > > visible to the guest.
> > > 
> > > Attempts by userspace to write the registers are handled similarly
> > > to the current invariant_sys_regs handling: writes are permitted,
> > > but only if they don't attempt to change the value.  This is
> > > sufficient to support VM snapshot/restore from userspace.
> > > 
> > > Because of the additional registers, restoring a VM on an older
> > > kernel may not work unless userspace knows how to handle the extra
> > > VM registers exposed to the KVM user ABI by this patch.
> > > 
> > > Under the principle of least damage, this patch makes no attempt to
> > > handle any of the other registers currently in
> > > invariant_sys_regs[], or to emulate registers for AArch32: however,
> > > these could be handled in a similar way in future, as necessary.
> > > 
> > > Signed-off-by: Dave Martin 
> > > Reviewed-by: Marc Zyngier 
> > > Acked-by: Catalin Marinas 
> > > Cc: Christoffer Dall 
> > > ---
> > >  arch/arm64/include/asm/sysreg.h |   3 +
> > >  arch/arm64/kvm/hyp/switch.c |   6 +
> > >  arch/arm64/kvm/sys_regs.c   | 282 
> > > +---
> > >  3 files changed, 246 insertions(+), 45 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/sysreg.h 
> > > b/arch/arm64/include/asm/sysreg.h
> > > index 4dceb12..609d59af 100644
> > > --- a/arch/arm64/include/asm/sysreg.h
> > > +++ b/arch/arm64/include/asm/sysreg.h
> > > @@ -149,6 +149,9 @@
> > >  #define SYS_ID_AA64DFR0_EL1  sys_reg(3, 0, 0, 5, 0)
> > >  #define SYS_ID_AA64DFR1_EL1  sys_reg(3, 0, 0, 5, 1)
> > >  
> > > +#define SYS_ID_AA64AFR0_EL1  sys_reg(3, 0, 0, 5, 4)
> > > +#define SYS_ID_AA64AFR1_EL1  sys_reg(3, 0, 0, 5, 5)
> > > +
> > >  #define SYS_ID_AA64ISAR0_EL1 sys_reg(3, 0, 0, 6, 0)
> > >  #define SYS_ID_AA64ISAR1_EL1 sys_reg(3, 0, 0, 6, 1)
> > >  
> > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > > index 945e79c..35a90b8 100644
> > > --- a/arch/arm64/kvm/hyp/switch.c
> > > +++ b/arch/arm64/kvm/hyp/switch.c
> > > @@ -81,11 +81,17 @@ static void __hyp_text __activate_traps(struct 
> > > kvm_vcpu *vcpu)
> > >* it will cause an exception.
> > >*/
> > >   val = vcpu->arch.hcr_el2;
> > > +
> > >   if (!(val & HCR_RW) && system_supports_fpsimd()) {
> > >   write_sysreg(1 << 30, fpexc32_el2);
> > >   isb();
> > >   }
> > > +
> > > + if (val & HCR_RW) /* for AArch64 only: */
> > > + val |= HCR_TID3; /* TID3: trap feature register accesses */
> > > +
> > 
> > I still think we should set this in vcpu_reset_hcr() and do it once
> > instead of adding code in every iteration of the critical path.
> 
> Dang, I somehow missed this in the last round.
> 
> How about:
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h 
> b/arch/arm64/include/asm/kvm_emulate.h
> index e5df3fc..c87be0d 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -49,6 +49,14 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
>   vcpu->arch.hcr_el2 |= HCR_E2H;
>   if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.

Re: [PATCH v4 00/21] SError rework + RAS&IESB for firmware first support

2017-11-02 Thread Christoffer Dall
On Wed, Nov 01, 2017 at 03:23:50PM +, James Morse wrote:
> Hi guys,
> 
> On 31/10/17 10:08, Will Deacon wrote:
> > On Tue, Oct 31, 2017 at 07:35:35AM +0100, Christoffer Dall wrote:
> >> On Thu, Oct 19, 2017 at 03:57:46PM +0100, James Morse wrote:
> >>> The aim of this series is to enable IESB and add ESB-instructions to let 
> >>> us
> >>> kick any pending RAS errors into firmware to be handled by firmware-first.
> >>>
> >>> Not all systems will have this firmware, so these RAS errors will become
> >>> pending SErrors. We should take these as quickly as possible and avoid
> >>> panic()ing for errors where we could have continued.
> >>>
> >>> This first part of this series reworks the DAIF masking so that SError is
> >>> unmasked unless we are handling a debug exception.
> >>>
> >>> The last part provides the same minimal handling for SError that interrupt
> >>> KVM. KVM is currently unable to handle SErrors during world-switch, unless
> >>> they occur during a magic single-instruction window, it hyp-panics. I 
> >>> suspect
> >>> this will be easier to fix once the VHE world-switch is further optimised.
> >>>
> >>> KVMs kvm_inject_vabt() needs updating for v8.2 as now we can specify an 
> >>> ESR,
> >>> and all-zeros has a RAS meaning.
> >>>
> >>> KVM's existing 'impdef SError to the guest' behaviour probably needs 
> >>> revisiting.
> >>> These are errors where we don't know what they mean, they may not be
> >>> synchronised by ESB. Today we blame the guest.
> >>> My half-baked suggestion would be to make a virtual SError pending, but 
> >>> then
> >>> exit to user-space to give Qemu the change to quit (for virtual machines 
> >>> that
> >>> don't generate SError), pend an SError with a new Qemu-specific ESR, or 
> >>> blindly
> >>> continue and take KVMs default all-zeros impdef ESR.
> >>
> >> The KVM side of this series is looking pretty good.
> >>
> >> What are the merge plans for this?  I am fine if you will take this via
> >> the arm64 tree with our acks from the KVM side.  Alternatively, I
> >> suppose you can apply all the arm64 patches and provide us with a stable
> >> branch for that?
> > 
> > I'll take a look this afternoon, but we haven't had a linux next release
> > since the 18th so I'm starting to get nervous about conflicts if I end up
> > pulling in new trees now.
> 
> Will's 'what about mixed RAS support' comment will take me a while to get to
> fix, and I don't think I can test that before the end of the week.
> 
> Unless there is an rc8+linux-next I think this is too late, but I will split 
> off
> and repost the SError_rework bits as that seems uncontentious...
> 
> 

It is indeed cutting it a bit close.  We'll have the same challenge of
either going via arm64 or using a stable branch we merge into the KVM
side for the next merge window as well.  I prefer the latter, since
there's going to be some conflicts with my optimization series which I
hope to get in for v4.16.

Thanks,
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm