* [PATCH v9 09/16] arm64/sve: Move read_zcr_features() out of cpufeature.h
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
Having read_zcr_features() inline in cpufeature.h results in that
header requiring #includes which make it hard to include
<asm/fpsimd.h> elsewhere without triggering header inclusion
cycles.
This is not a hot-path function and arguably should not be in
cpufeature.h in the first place, so this patch moves it to
fpsimd.c, compiled conditionally if CONFIG_ARM64_SVE=y.
This allows some SVE-related #includes to be dropped from
cpufeature.h, which will ease future maintenance.
A couple of missing #includes of <asm/fpsimd.h> are exposed by this
change under arch/arm64/. This patch adds the missing #includes as
necessary.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 29 -----------------------------
arch/arm64/include/asm/fpsimd.h | 2 ++
arch/arm64/include/asm/processor.h | 1 +
arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++++++++++++++++
arch/arm64/kernel/ptrace.c | 1 +
5 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 09b0f2a..0a6b713 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -11,9 +11,7 @@
#include <asm/cpucaps.h>
#include <asm/cputype.h>
-#include <asm/fpsimd.h>
#include <asm/hwcap.h>
-#include <asm/sigcontext.h>
#include <asm/sysreg.h>
/*
@@ -510,33 +508,6 @@ static inline bool system_supports_sve(void)
cpus_have_const_cap(ARM64_SVE);
}
-/*
- * Read the pseudo-ZCR used by cpufeatures to identify the supported SVE
- * vector length.
- *
- * Use only if SVE is present.
- * This function clobbers the SVE vector length.
- */
-static inline u64 read_zcr_features(void)
-{
- u64 zcr;
- unsigned int vq_max;
-
- /*
- * Set the maximum possible VL, and write zeroes to all other
- * bits to see if they stick.
- */
- sve_kernel_enable(NULL);
- write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL1);
-
- zcr = read_sysreg_s(SYS_ZCR_EL1);
- zcr &= ~(u64)ZCR_ELx_LEN_MASK; /* find sticky 1s outside LEN field */
- vq_max = sve_vq_from_vl(sve_get_vl());
- zcr |= vq_max - 1; /* set LEN field to maximum effective value */
-
- return zcr;
-}
-
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index 3e00f70..fb60b22 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -69,6 +69,8 @@ extern unsigned int sve_get_vl(void);
struct arm64_cpu_capabilities;
extern void sve_kernel_enable(const struct arm64_cpu_capabilities *__unused);
+extern u64 read_zcr_features(void);
+
extern int __ro_after_init sve_max_vl;
#ifdef CONFIG_ARM64_SVE
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 7675989..f902b6d 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -40,6 +40,7 @@
#include <asm/alternative.h>
#include <asm/cpufeature.h>
+#include <asm/fpsimd.h>
#include <asm/hw_breakpoint.h>
#include <asm/lse.h>
#include <asm/pgtable-hwdef.h>
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 051b455..5c33667 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -37,6 +37,7 @@
#include <linux/sched/task_stack.h>
#include <linux/signal.h>
#include <linux/slab.h>
+#include <linux/stddef.h>
#include <linux/sysctl.h>
#include <asm/esr.h>
@@ -754,6 +755,33 @@ void sve_kernel_enable(const struct arm64_cpu_capabilities *__always_unused p)
isb();
}
+/*
+ * Read the pseudo-ZCR used by cpufeatures to identify the supported SVE
+ * vector length.
+ *
+ * Use only if SVE is present.
+ * This function clobbers the SVE vector length.
+ */
+u64 read_zcr_features(void)
+{
+ u64 zcr;
+ unsigned int vq_max;
+
+ /*
+ * Set the maximum possible VL, and write zeroes to all other
+ * bits to see if they stick.
+ */
+ sve_kernel_enable(NULL);
+ write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL1);
+
+ zcr = read_sysreg_s(SYS_ZCR_EL1);
+ zcr &= ~(u64)ZCR_ELx_LEN_MASK; /* find sticky 1s outside LEN field */
+ vq_max = sve_vq_from_vl(sve_get_vl());
+ zcr |= vq_max - 1; /* set LEN field to maximum effective value */
+
+ return zcr;
+}
+
void __init sve_setup(void)
{
u64 zcr;
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 7ff81fe..78889c4 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -44,6 +44,7 @@
#include <asm/compat.h>
#include <asm/cpufeature.h>
#include <asm/debug-monitors.h>
+#include <asm/fpsimd.h>
#include <asm/pgtable.h>
#include <asm/stacktrace.h>
#include <asm/syscall.h>
--
2.1.4
^ permalink raw reply related
* [PATCH v9 10/16] arm64/sve: Switch sve_pffr() argument from task to thread
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
sve_pffr(), which is used to derive the base address used for
low-level SVE save/restore routines, currently takes the relevant
task_struct as an argument.
The only accessed fields are actually part of thread_struct, so
this patch changes the argument type accordingly. This is done in
preparation for moving this function to a header, where we do not
want to have to include <linux/sched.h> due to the consequent
circular #include problems.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kernel/fpsimd.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 5c33667..a456d6f 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -44,6 +44,7 @@
#include <asm/fpsimd.h>
#include <asm/cpufeature.h>
#include <asm/cputype.h>
+#include <asm/processor.h>
#include <asm/simd.h>
#include <asm/sigcontext.h>
#include <asm/sysreg.h>
@@ -167,10 +168,9 @@ static size_t sve_ffr_offset(int vl)
return SVE_SIG_FFR_OFFSET(sve_vq_from_vl(vl)) - SVE_SIG_REGS_OFFSET;
}
-static void *sve_pffr(struct task_struct *task)
+static void *sve_pffr(struct thread_struct *thread)
{
- return (char *)task->thread.sve_state +
- sve_ffr_offset(task->thread.sve_vl);
+ return (char *)thread->sve_state + sve_ffr_offset(thread->sve_vl);
}
static void change_cpacr(u64 val, u64 mask)
@@ -253,7 +253,7 @@ static void task_fpsimd_load(void)
WARN_ON(!in_softirq() && !irqs_disabled());
if (system_supports_sve() && test_thread_flag(TIF_SVE))
- sve_load_state(sve_pffr(current),
+ sve_load_state(sve_pffr(¤t->thread),
¤t->thread.uw.fpsimd_state.fpsr,
sve_vq_from_vl(current->thread.sve_vl) - 1);
else
@@ -284,7 +284,7 @@ void fpsimd_save(void)
return;
}
- sve_save_state(sve_pffr(current), &st->fpsr);
+ sve_save_state(sve_pffr(¤t->thread), &st->fpsr);
} else
fpsimd_save_state(st);
}
--
2.1.4
^ permalink raw reply related
* [PATCH v9 11/16] arm64/sve: Move sve_pffr() to fpsimd.h and make inline
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
In order to make sve_save_state()/sve_load_state() more easily
reusable and to get rid of a potential branch on context switch
critical paths, this patch makes sve_pffr() inline and moves it to
fpsimd.h.
<asm/processor.h> must be included in fpsimd.h in order to make
this work, and this creates an #include cycle that is tricky to
avoid without modifying core code, due to the way the PR_SVE_*()
prctl helpers are included in the core prctl implementation.
Instead of breaking the cycle, this patch defers inclusion of
<asm/fpsimd.h> in <asm/processor.h> until the point where it is
actually needed: i.e., immediately before the prctl definitions.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/include/asm/fpsimd.h | 13 +++++++++++++
arch/arm64/include/asm/processor.h | 3 ++-
arch/arm64/kernel/fpsimd.c | 12 ------------
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index fb60b22..fa92747 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -18,6 +18,8 @@
#include <asm/ptrace.h>
#include <asm/errno.h>
+#include <asm/processor.h>
+#include <asm/sigcontext.h>
#ifndef __ASSEMBLY__
@@ -61,6 +63,17 @@ extern void sve_flush_cpu_state(void);
/* Maximum VL that SVE VL-agnostic software can transparently support */
#define SVE_VL_ARCH_MAX 0x100
+/* Offset of FFR in the SVE register dump */
+static inline size_t sve_ffr_offset(int vl)
+{
+ return SVE_SIG_FFR_OFFSET(sve_vq_from_vl(vl)) - SVE_SIG_REGS_OFFSET;
+}
+
+static inline void *sve_pffr(struct thread_struct *thread)
+{
+ return (char *)thread->sve_state + sve_ffr_offset(thread->sve_vl);
+}
+
extern void sve_save_state(void *state, u32 *pfpsr);
extern void sve_load_state(void const *state, u32 const *pfpsr,
unsigned long vq_minus_1);
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index f902b6d..ebaadb1 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -40,7 +40,6 @@
#include <asm/alternative.h>
#include <asm/cpufeature.h>
-#include <asm/fpsimd.h>
#include <asm/hw_breakpoint.h>
#include <asm/lse.h>
#include <asm/pgtable-hwdef.h>
@@ -245,6 +244,8 @@ void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused);
void cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused);
void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused);
+#include <asm/fpsimd.h>
+
/* Userspace interface for PR_SVE_{SET,GET}_VL prctl()s: */
#define SVE_SET_VL(arg) sve_set_current_vl(arg)
#define SVE_GET_VL() sve_get_current_vl()
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index a456d6f..bf7ce9b 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -161,18 +161,6 @@ static void sve_free(struct task_struct *task)
__sve_free(task);
}
-
-/* Offset of FFR in the SVE register dump */
-static size_t sve_ffr_offset(int vl)
-{
- return SVE_SIG_FFR_OFFSET(sve_vq_from_vl(vl)) - SVE_SIG_REGS_OFFSET;
-}
-
-static void *sve_pffr(struct thread_struct *thread)
-{
- return (char *)thread->sve_state + sve_ffr_offset(thread->sve_vl);
-}
-
static void change_cpacr(u64 val, u64 mask)
{
u64 cpacr = read_sysreg(CPACR_EL1);
--
2.1.4
^ permalink raw reply related
* [PATCH v9 12/16] KVM: arm64: Save host SVE context as appropriate
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
This patch adds SVE context saving to the hyp FPSIMD context switch
path. This means that it is no longer necessary to save the host
SVE state in advance of entering the guest, when in use.
In order to avoid adding pointless complexity to the code, VHE is
assumed if SVE is in use. VHE is an architectural prerequisite for
SVE, so there is no good reason to turn CONFIG_ARM64_VHE off in
kernels that support both SVE and KVM.
Historically, software models exist that can expose the
architecturally invalid configuration of SVE without VHE, so if
this situation is detected at kvm_init() time then KVM will be
disabled.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
* Stripped the following tags, reviewers please reconfirm:
Formerly-Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Formerly-Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Formerly-Acked-by: Catalin Marinas <catalin.marinas@arm.com>
(Creation of a new file for this one change may be deemed undesirable,
but there didn't seem to be a correct place to put it. There may also
be a way around the circular include problem that I missed.)
Changes since v8:
* Add kvm_arch_check_supported() hook, and move arm64-specific check
for SVE-implies-VHE into arch/arm64/.
Due to circular header dependency problems, it is difficult to get
the prototype for kvm_pr_*() functions in <asm/kvm_host.h>, so this
patch puts arm64's kvm_arch_check_supported() hook out of line.
This is not a hot function.
---
arch/arm/include/asm/kvm_host.h | 1 +
arch/arm64/Kconfig | 7 +++++++
arch/arm64/include/asm/kvm_host.h | 1 +
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/fpsimd.c | 1 -
arch/arm64/kvm/hyp/switch.c | 20 +++++++++++++++++++-
arch/arm64/kvm/init.c | 33 +++++++++++++++++++++++++++++++++
virt/kvm/arm/arm.c | 6 ++++++
8 files changed, 68 insertions(+), 3 deletions(-)
create mode 100644 arch/arm64/kvm/init.c
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index ac870b2..e627ef8 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -280,6 +280,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+static inline int kvm_arch_check_supported(void) { return 0; }
static inline void kvm_arch_hardware_unsetup(void) {}
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index eb2cf49..b0d3820 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1130,6 +1130,7 @@ endmenu
config ARM64_SVE
bool "ARM Scalable Vector Extension support"
default y
+ depends on !KVM || ARM64_VHE
help
The Scalable Vector Extension (SVE) is an extension to the AArch64
execution state which complements and extends the SIMD functionality
@@ -1155,6 +1156,12 @@ config ARM64_SVE
booting the kernel. If unsure and you are not observing these
symptoms, you should assume that it is safe to say Y.
+ CPUs that support SVE are architecturally required to support the
+ Virtualization Host Extensions (VHE), so the kernel makes no
+ provision for supporting SVE alongside KVM without VHE enabled.
+ Thus, you will need to enable CONFIG_ARM64_VHE if you want to support
+ KVM in the same kernel image.
+
config ARM64_MODULE_PLTS
bool
select HAVE_MOD_ARCH_SPECIFIC
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index b3fe730..80f3985 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -405,6 +405,7 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
kvm_call_hyp(__kvm_set_tpidr_el2, tpidr_el2);
}
+int kvm_arch_check_supported(void);
static inline void kvm_arch_hardware_unsetup(void) {}
static inline void kvm_arch_sync_events(struct kvm *kvm) {}
static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 0f2a135..5e66afe 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -17,7 +17,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
-kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
+kvm-$(CONFIG_KVM_ARM_HOST) += init.o hyp.o hyp-init.o handle_exit.o
kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index d9b5f73..52185ec 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -59,7 +59,6 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
*/
void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
{
- BUG_ON(system_supports_sve());
BUG_ON(!current->mm);
vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 118f300..a6a8c7d 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -21,6 +21,7 @@
#include <kvm/arm_psci.h>
+#include <asm/cpufeature.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_emulate.h>
#include <asm/kvm_host.h>
@@ -28,6 +29,7 @@
#include <asm/kvm_mmu.h>
#include <asm/fpsimd.h>
#include <asm/debug-monitors.h>
+#include <asm/processor.h>
#include <asm/thread_info.h>
/* Check whether the FP regs were dirtied while in the host-side run loop: */
@@ -329,6 +331,8 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
struct kvm_vcpu *vcpu)
{
+ struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
+
if (has_vhe())
write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
cpacr_el1);
@@ -339,7 +343,21 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
isb();
if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
- __fpsimd_save_state(vcpu->arch.host_fpsimd_state);
+ /*
+ * In the SVE case, VHE is assumed: it is enforced by
+ * Kconfig and kvm_arch_init().
+ */
+ if (system_supports_sve() &&
+ (vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE)) {
+ struct thread_struct *thread = container_of(
+ host_fpsimd,
+ struct thread_struct, uw.fpsimd_state);
+
+ sve_save_state(sve_pffr(thread), &host_fpsimd->fpsr);
+ } else {
+ __fpsimd_save_state(host_fpsimd);
+ }
+
vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
}
diff --git a/arch/arm64/kvm/init.c b/arch/arm64/kvm/init.c
new file mode 100644
index 0000000..3b6e730
--- /dev/null
+++ b/arch/arm64/kvm/init.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * arch/arm64/kvm/init.c: KVM initialisation support
+ *
+ * Copyright 2018 Arm Limited
+ * Author: Dave Martin <Dave.Martin@arm.com>
+ */
+#include <linux/errno.h>
+#include <linux/kvm_host.h>
+#include <asm/cpufeature.h>
+#include <asm/kvm_host.h>
+
+/* Additional arch-dependent checks for KVM usability */
+int kvm_arch_check_supported(void)
+{
+ /*
+ * VHE is a prerequisite for SVE in the Arm architecture, and
+ * Kconfig ensures that if system_supports_sve() here then
+ * CONFIG_ARM64_VHE is enabled, so if VHE support wasn't already
+ * detected and enabled, the CPU is architecturally
+ * noncompliant.
+ *
+ * Just in case this mismatch is seen, detect it, warn and give
+ * up. Supporting this forbidden configuration in Hyp would be
+ * pointless.
+ */
+ if (system_supports_sve() && !has_vhe()) {
+ kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
+ return -ENODEV;
+ }
+
+ return 0;
+}
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index bee226c..8518df0 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -16,6 +16,7 @@
* Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
+#include <linux/bug.h>
#include <linux/cpu_pm.h>
#include <linux/errno.h>
#include <linux/err.h>
@@ -41,6 +42,7 @@
#include <asm/mman.h>
#include <asm/tlbflush.h>
#include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
#include <asm/virt.h>
#include <asm/kvm_arm.h>
#include <asm/kvm_asm.h>
@@ -1574,6 +1576,10 @@ int kvm_arch_init(void *opaque)
return -ENODEV;
}
+ err = kvm_arch_check_supported();
+ if (err)
+ return err;
+
for_each_online_cpu(cpu) {
smp_call_function_single(cpu, check_kvm_target_cpu, &ret, 1);
if (ret < 0) {
--
2.1.4
^ permalink raw reply related
* [PATCH v9 13/16] KVM: arm64: Remove eager host SVE state saving
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
Now that the host SVE context can be saved on demand from Hyp,
there is no longer any need to save this state in advance before
entering the guest.
This patch removes the relevant call to
kvm_fpsimd_flush_cpu_state().
Since the problem that function was intended to solve now no longer
exists, the function and its dependencies are also deleted.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm/include/asm/kvm_host.h | 3 ---
arch/arm64/include/asm/kvm_host.h | 10 ----------
arch/arm64/kernel/fpsimd.c | 21 ---------------------
virt/kvm/arm/arm.c | 3 ---
4 files changed, 37 deletions(-)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index e627ef8..fa0d9e1 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -312,9 +312,6 @@ static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
-/* All host FP/SIMD state is restored on guest exit, so nothing to save: */
-static inline void kvm_fpsimd_flush_cpu_state(void) {}
-
static inline void kvm_arm_vhe_guest_enter(void) {}
static inline void kvm_arm_vhe_guest_exit(void) {}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 80f3985..556067c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -445,16 +445,6 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
}
#endif
-/*
- * All host FP/SIMD state is restored on guest exit, so nothing needs
- * doing here except in the SVE case:
-*/
-static inline void kvm_fpsimd_flush_cpu_state(void)
-{
- if (system_supports_sve())
- sve_flush_cpu_state();
-}
-
static inline void kvm_arm_vhe_guest_enter(void)
{
local_daif_mask();
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index bf7ce9b..b0d29b7 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -120,7 +120,6 @@
*/
struct fpsimd_last_state_struct {
struct user_fpsimd_state *st;
- bool sve_in_use;
};
static DEFINE_PER_CPU(struct fpsimd_last_state_struct, fpsimd_last_state);
@@ -1013,7 +1012,6 @@ void fpsimd_bind_task_to_cpu(void)
this_cpu_ptr(&fpsimd_last_state);
last->st = ¤t->thread.uw.fpsimd_state;
- last->sve_in_use = test_thread_flag(TIF_SVE);
current->thread.fpsimd_cpu = smp_processor_id();
if (system_supports_sve()) {
@@ -1035,7 +1033,6 @@ void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st)
WARN_ON(!in_softirq() && !irqs_disabled());
last->st = st;
- last->sve_in_use = false;
}
/*
@@ -1095,24 +1092,6 @@ void fpsimd_flush_cpu_state(void)
__this_cpu_write(fpsimd_last_state.st, NULL);
}
-/*
- * Invalidate any task SVE state currently held in this CPU's regs.
- *
- * This is used to prevent the kernel from trying to reuse SVE register data
- * that is detroyed by KVM guest enter/exit. This function should go away when
- * KVM SVE support is implemented. Don't use it for anything else.
- */
-#ifdef CONFIG_ARM64_SVE
-void sve_flush_cpu_state(void)
-{
- struct fpsimd_last_state_struct const *last =
- this_cpu_ptr(&fpsimd_last_state);
-
- if (last->st && last->sve_in_use)
- fpsimd_flush_cpu_state();
-}
-#endif /* CONFIG_ARM64_SVE */
-
#ifdef CONFIG_KERNEL_MODE_NEON
DEFINE_PER_CPU(bool, kernel_neon_busy);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 8518df0..16e852e 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -682,9 +682,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
*/
preempt_disable();
- /* Flush FP/SIMD state that can't survive guest entry/exit */
- kvm_fpsimd_flush_cpu_state();
-
kvm_pmu_flush_hwstate(vcpu);
local_irq_disable();
--
2.1.4
^ permalink raw reply related
* [PATCH v9 14/16] KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit()
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
In fixup_guest_exit(), there are a couple of cases where after
checking what the exit code was, we assign it explicitly with the
value it already had.
Assuming this is not indicative of a bug, these assignments are not
needed.
This patch removes the redundant assignments, and simplifies some
if-nesting that becomes trivial as a result.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kvm/hyp/switch.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index a6a8c7d..18d0faa 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -403,12 +403,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
if (valid) {
int ret = __vgic_v2_perform_cpuif_access(vcpu);
- if (ret == 1) {
- if (__skip_instr(vcpu))
- return true;
- else
- *exit_code = ARM_EXCEPTION_TRAP;
- }
+ if (ret == 1 && __skip_instr(vcpu))
+ return true;
if (ret == -1) {
/* Promote an illegal access to an
@@ -430,12 +426,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
int ret = __vgic_v3_perform_cpuif_access(vcpu);
- if (ret == 1) {
- if (__skip_instr(vcpu))
- return true;
- else
- *exit_code = ARM_EXCEPTION_TRAP;
- }
+ if (ret == 1 && __skip_instr(vcpu))
+ return true;
}
/* Return to the host kernel and handle the exit */
--
2.1.4
^ permalink raw reply related
* [PATCH v9 15/16] KVM: arm64: Fold redundant exit code checks out of fixup_guest_exit()
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
The entire tail of fixup_guest_exit() is contained in if statements
of the form if (x && *exit_code == ARM_EXCEPTION_TRAP). As a result,
we can check just once and bail out of the function early, allowing
the remaining if conditions to be simplified.
The only awkward case is where *exit_code is changed to
ARM_EXCEPTION_EL1_SERROR in the case of an illegal GICv2 CPU
interface access: in that case, the GICv3 trap handling code is
skipped using a goto. This avoids pointlessly evaluating the
static branch check for the GICv3 case, even though we can't have
vgic_v2_cpuif_trap and vgic_v3_cpuif_trap true simultaneously
unless we have a GICv3 and GICv2 on the host: that sounds stupid,
but I haven't satisfied myself that it can't happen.
No functional change.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kvm/hyp/switch.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 18d0faa..4fbee95 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -387,11 +387,13 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
* same PC once the SError has been injected, and replay the
* trapping instruction.
*/
- if (*exit_code == ARM_EXCEPTION_TRAP && !__populate_fault_info(vcpu))
+ if (*exit_code != ARM_EXCEPTION_TRAP)
+ goto exit;
+
+ if (!__populate_fault_info(vcpu))
return true;
- if (static_branch_unlikely(&vgic_v2_cpuif_trap) &&
- *exit_code == ARM_EXCEPTION_TRAP) {
+ if (static_branch_unlikely(&vgic_v2_cpuif_trap)) {
bool valid;
valid = kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_DABT_LOW &&
@@ -417,11 +419,12 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
*vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
*exit_code = ARM_EXCEPTION_EL1_SERROR;
}
+
+ goto exit;
}
}
if (static_branch_unlikely(&vgic_v3_cpuif_trap) &&
- *exit_code == ARM_EXCEPTION_TRAP &&
(kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 ||
kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
int ret = __vgic_v3_perform_cpuif_access(vcpu);
@@ -430,6 +433,7 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
return true;
}
+exit:
/* Return to the host kernel and handle the exit */
return false;
}
--
2.1.4
^ permalink raw reply related
* [PATCH v9 16/16] KVM: arm64: Invoke FPSIMD context switch trap from C
From: Dave Martin @ 2018-05-21 14:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-1-git-send-email-Dave.Martin@arm.com>
The conversion of the FPSIMD context switch trap code to C has added
some overhead to calling it, due to the need to save registers that
the procedure call standard defines as caller-saved.
So, perhaps it is no longer worth invoking this trap handler quite
so early.
Instead, we can invoke it from fixup_guest_exit(), with little
likelihood of increasing the overhead much further.
As a convenience, this patch gives __hyp_switch_fpsimd() the same
return semantics fixup_guest_exit(). For now there is no
possibility of a spurious FPSIMD trap, so the function always
returns true, but this allows it to be tail-called with a single
return statement.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
---
arch/arm64/kvm/hyp/entry.S | 30 ------------------------------
arch/arm64/kvm/hyp/hyp-entry.S | 19 -------------------
arch/arm64/kvm/hyp/switch.c | 15 +++++++++++++--
3 files changed, 13 insertions(+), 51 deletions(-)
diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 40f349b..fad1e16 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -166,33 +166,3 @@ abort_guest_exit_end:
orr x0, x0, x5
1: ret
ENDPROC(__guest_exit)
-
-ENTRY(__fpsimd_guest_restore)
- // x0: esr
- // x1: vcpu
- // x2-x29,lr: vcpu regs
- // vcpu x0-x1 on the stack
- stp x2, x3, [sp, #-144]!
- stp x4, x5, [sp, #16]
- stp x6, x7, [sp, #32]
- stp x8, x9, [sp, #48]
- stp x10, x11, [sp, #64]
- stp x12, x13, [sp, #80]
- stp x14, x15, [sp, #96]
- stp x16, x17, [sp, #112]
- stp x18, lr, [sp, #128]
-
- bl __hyp_switch_fpsimd
-
- ldp x4, x5, [sp, #16]
- ldp x6, x7, [sp, #32]
- ldp x8, x9, [sp, #48]
- ldp x10, x11, [sp, #64]
- ldp x12, x13, [sp, #80]
- ldp x14, x15, [sp, #96]
- ldp x16, x17, [sp, #112]
- ldp x18, lr, [sp, #128]
- ldp x0, x1, [sp, #144]
- ldp x2, x3, [sp], #160
- eret
-ENDPROC(__fpsimd_guest_restore)
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index bffece2..753b9d2 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -113,25 +113,6 @@ el1_hvc_guest:
el1_trap:
get_vcpu_ptr x1, x0
-
- mrs x0, esr_el2
- lsr x0, x0, #ESR_ELx_EC_SHIFT
- /*
- * x0: ESR_EC
- * x1: vcpu pointer
- */
-
- /*
- * We trap the first access to the FP/SIMD to save the host context
- * and restore the guest context lazily.
- * If FP/SIMD is not implemented, handle the trap and inject an
- * undefined instruction exception to the guest.
- */
-alternative_if_not ARM64_HAS_NO_FPSIMD
- cmp x0, #ESR_ELx_EC_FP_ASIMD
- b.eq __fpsimd_guest_restore
-alternative_else_nop_endif
-
mov x0, #ARM_EXCEPTION_TRAP
b __guest_exit
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 4fbee95..2d45bd7 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -328,8 +328,7 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
}
}
-void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
- struct kvm_vcpu *vcpu)
+static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
{
struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
@@ -369,6 +368,8 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
fpexc32_el2);
vcpu->arch.flags |= KVM_ARM64_FP_ENABLED;
+
+ return true;
}
/*
@@ -390,6 +391,16 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
if (*exit_code != ARM_EXCEPTION_TRAP)
goto exit;
+ /*
+ * We trap the first access to the FP/SIMD to save the host context
+ * and restore the guest context lazily.
+ * If FP/SIMD is not implemented, handle the trap and inject an
+ * undefined instruction exception to the guest.
+ */
+ if (system_supports_fpsimd() &&
+ kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_FP_ASIMD)
+ return __hyp_switch_fpsimd(vcpu);
+
if (!__populate_fault_info(vcpu))
return true;
--
2.1.4
^ permalink raw reply related
* [PATCH v9 12/16] KVM: arm64: Save host SVE context as appropriate
From: Marc Zyngier @ 2018-05-21 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526912237-25308-13-git-send-email-Dave.Martin@arm.com>
On 21/05/18 15:17, Dave Martin wrote:
> This patch adds SVE context saving to the hyp FPSIMD context switch
> path. This means that it is no longer necessary to save the host
> SVE state in advance of entering the guest, when in use.
>
> In order to avoid adding pointless complexity to the code, VHE is
> assumed if SVE is in use. VHE is an architectural prerequisite for
> SVE, so there is no good reason to turn CONFIG_ARM64_VHE off in
> kernels that support both SVE and KVM.
>
> Historically, software models exist that can expose the
> architecturally invalid configuration of SVE without VHE, so if
> this situation is detected at kvm_init() time then KVM will be
> disabled.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
>
> ---
>
> * Stripped the following tags, reviewers please reconfirm:
>
> Formerly-Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
> Formerly-Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Formerly-Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> (Creation of a new file for this one change may be deemed undesirable,
> but there didn't seem to be a correct place to put it. There may also
> be a way around the circular include problem that I missed.)
>
> Changes since v8:
>
> * Add kvm_arch_check_supported() hook, and move arm64-specific check
> for SVE-implies-VHE into arch/arm64/.
>
> Due to circular header dependency problems, it is difficult to get
> the prototype for kvm_pr_*() functions in <asm/kvm_host.h>, so this
> patch puts arm64's kvm_arch_check_supported() hook out of line.
> This is not a hot function.
> ---
> arch/arm/include/asm/kvm_host.h | 1 +
> arch/arm64/Kconfig | 7 +++++++
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/Makefile | 2 +-
> arch/arm64/kvm/fpsimd.c | 1 -
> arch/arm64/kvm/hyp/switch.c | 20 +++++++++++++++++++-
> arch/arm64/kvm/init.c | 33 +++++++++++++++++++++++++++++++++
> virt/kvm/arm/arm.c | 6 ++++++
> 8 files changed, 68 insertions(+), 3 deletions(-)
> create mode 100644 arch/arm64/kvm/init.c
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index ac870b2..e627ef8 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -280,6 +280,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>
> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>
> +static inline int kvm_arch_check_supported(void) { return 0; }
> static inline void kvm_arch_hardware_unsetup(void) {}
> static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index eb2cf49..b0d3820 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1130,6 +1130,7 @@ endmenu
> config ARM64_SVE
> bool "ARM Scalable Vector Extension support"
> default y
> + depends on !KVM || ARM64_VHE
> help
> The Scalable Vector Extension (SVE) is an extension to the AArch64
> execution state which complements and extends the SIMD functionality
> @@ -1155,6 +1156,12 @@ config ARM64_SVE
> booting the kernel. If unsure and you are not observing these
> symptoms, you should assume that it is safe to say Y.
>
> + CPUs that support SVE are architecturally required to support the
> + Virtualization Host Extensions (VHE), so the kernel makes no
> + provision for supporting SVE alongside KVM without VHE enabled.
> + Thus, you will need to enable CONFIG_ARM64_VHE if you want to support
> + KVM in the same kernel image.
> +
> config ARM64_MODULE_PLTS
> bool
> select HAVE_MOD_ARCH_SPECIFIC
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index b3fe730..80f3985 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -405,6 +405,7 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
> kvm_call_hyp(__kvm_set_tpidr_el2, tpidr_el2);
> }
>
> +int kvm_arch_check_supported(void);
> static inline void kvm_arch_hardware_unsetup(void) {}
> static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 0f2a135..5e66afe 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -17,7 +17,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
>
> kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
> -kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += init.o hyp.o hyp-init.o handle_exit.o
> kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
> kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index d9b5f73..52185ec 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -59,7 +59,6 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
> */
> void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> {
> - BUG_ON(system_supports_sve());
> BUG_ON(!current->mm);
>
> vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 118f300..a6a8c7d 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -21,6 +21,7 @@
>
> #include <kvm/arm_psci.h>
>
> +#include <asm/cpufeature.h>
> #include <asm/kvm_asm.h>
> #include <asm/kvm_emulate.h>
> #include <asm/kvm_host.h>
> @@ -28,6 +29,7 @@
> #include <asm/kvm_mmu.h>
> #include <asm/fpsimd.h>
> #include <asm/debug-monitors.h>
> +#include <asm/processor.h>
> #include <asm/thread_info.h>
>
> /* Check whether the FP regs were dirtied while in the host-side run loop: */
> @@ -329,6 +331,8 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
> void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
> struct kvm_vcpu *vcpu)
> {
> + struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
> +
> if (has_vhe())
> write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
> cpacr_el1);
> @@ -339,7 +343,21 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
> isb();
>
> if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
> - __fpsimd_save_state(vcpu->arch.host_fpsimd_state);
> + /*
> + * In the SVE case, VHE is assumed: it is enforced by
> + * Kconfig and kvm_arch_init().
> + */
> + if (system_supports_sve() &&
> + (vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE)) {
> + struct thread_struct *thread = container_of(
> + host_fpsimd,
> + struct thread_struct, uw.fpsimd_state);
> +
> + sve_save_state(sve_pffr(thread), &host_fpsimd->fpsr);
> + } else {
> + __fpsimd_save_state(host_fpsimd);
> + }
> +
> vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
> }
>
> diff --git a/arch/arm64/kvm/init.c b/arch/arm64/kvm/init.c
> new file mode 100644
> index 0000000..3b6e730
> --- /dev/null
> +++ b/arch/arm64/kvm/init.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * arch/arm64/kvm/init.c: KVM initialisation support
> + *
> + * Copyright 2018 Arm Limited
> + * Author: Dave Martin <Dave.Martin@arm.com>
> + */
> +#include <linux/errno.h>
> +#include <linux/kvm_host.h>
> +#include <asm/cpufeature.h>
> +#include <asm/kvm_host.h>
> +
> +/* Additional arch-dependent checks for KVM usability */
> +int kvm_arch_check_supported(void)
> +{
> + /*
> + * VHE is a prerequisite for SVE in the Arm architecture, and
> + * Kconfig ensures that if system_supports_sve() here then
> + * CONFIG_ARM64_VHE is enabled, so if VHE support wasn't already
> + * detected and enabled, the CPU is architecturally
> + * noncompliant.
> + *
> + * Just in case this mismatch is seen, detect it, warn and give
> + * up. Supporting this forbidden configuration in Hyp would be
> + * pointless.
> + */
> + if (system_supports_sve() && !has_vhe()) {
> + kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
[entering bikeshedding territory]
I'm not exactly keen on this. This feels overkill, and a departure from
what we've been doing so far.
Why don't you simply have a helper in kvm_host.h that does:
static inline bool kvm_arm_check_sve_valid(void)
{
return !system_supports_sve() || has_vhe();
}
(and a canonical "return true;" implementation for 32bit)
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index bee226c..8518df0 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -16,6 +16,7 @@
> * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> */
>
> +#include <linux/bug.h>
> #include <linux/cpu_pm.h>
> #include <linux/errno.h>
> #include <linux/err.h>
> @@ -41,6 +42,7 @@
> #include <asm/mman.h>
> #include <asm/tlbflush.h>
> #include <asm/cacheflush.h>
> +#include <asm/cpufeature.h>
> #include <asm/virt.h>
> #include <asm/kvm_arm.h>
> #include <asm/kvm_asm.h>
> @@ -1574,6 +1576,10 @@ int kvm_arch_init(void *opaque)
> return -ENODEV;
> }
>
> + err = kvm_arch_check_supported();
> + if (err)
> + return err;
> +
And turn this into:
if (!kvm_arm_check_sve_valid()) {
kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
return -EINVAL;
}
It has the advantage of being obvious of what we check for, and doesn't
add any extra file.
Thoughts?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 6/6] arm64: perf: Add support for chaining counters
From: Suzuki K Poulose @ 2018-05-21 14:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f7ffdcfb-5a32-9a55-32a5-4f8770e23aa0@arm.com>
On 21/05/18 15:00, Robin Murphy wrote:
> On 21/05/18 14:42, Suzuki K Poulose wrote:
>> On 18/05/18 16:57, Suzuki K Poulose wrote:
>>> Hi Robin,
>>>
>>> On 18/05/18 14:49, Robin Murphy wrote:
>>>> On 18/05/18 11:22, Suzuki K Poulose wrote:
>>>>> Add support for chained event counters. PMUv3 allows chaining
>>>>> a pair of adjacent PMU counters (with the lower counter number
>>>>> being always "even"). The low counter is programmed to count
>>>>> the event of interest and the high counter(odd numbered) is
>>>>> programmed with a special event code (0x1e - Chain). Thus
>>>>> we need special allocation schemes to make the full use of
>>>>> available counters. So, we allocate the counters from either
>>>>> ends. i.e, chained counters are allocated from the lower
>>>>> end in pairs of two and the normal counters are allocated
>>>>> from the higher number. Also makes necessary changes to
>>>>> handle the chained events as a single event with 2 counters.
>>>>>
>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>
>>>
>>>>> ? /*
>>>>> @@ -845,8 +1016,14 @@ static int __armv8_pmuv3_map_event(struct perf_event *event,
>>>>> ???????????????????????? &armv8_pmuv3_perf_cache_map,
>>>>> ???????????????????????? ARMV8_PMU_EVTYPE_EVENT);
>>>>> -??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES)
>>>>> +??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
>>>>> +??????? /* Prevent chaining for cycle counter */
>>>>
>>>> Why? Sure, we want to avoid executing the chaining logic if we're scheduling a cycles event in the dedicated counter (which is perhaps what the comment above wanted to say), but if one ends up allocated into a regular counter (e.g. if the user asks for multiple cycle counts with different filters), then I don't see any reason to forbid that being chained.
>>>
>>> Ah, I didn't think about that case. I was under the assumption that the
>>> cycles are *only* placed on the cycle counter. I will take care of that.
>>> Thanks for the review.
>>
>> Robin, Mark, Will
>>
>> One potential problem I see with allowing chaining of the cycle counter
>> *and* the promotion of cycle event to 64bit by default is when the user
>> may actually be able to count 1 less event (due to the promotion of
>> cycle event to 64bit and thus forcing to use chain, if the cycle counter
>> is unavailable).
>
> Right, I didn't mean to imply we should inject the "chain" attr automatically for all cycles events, just that we shouldn't be rejecting it if the user does explicitly set it (but then just ignore it if using the dedicated counter).
Right, I was not talking about the automatic "chain" for cycles events. The
problem is we don't know if we would get the "cycle" counter for the given
event until it is "added", at which point we have already decided
whether the event is 32bit or 64bit. So, we cannot really delay the decision
until that. Thats where this comes up. Given a cycle event (without an explicit
chain request), do we treat it as a 64bit event or not ? If we do, we could
1) get the Cycle counter, all is fine.
2) If not, fallback to Chaining. The user looses a counter.
>
>> So one option is to drop automatic promotion of the cycle counter to
>> 64bit and do it only when it is requested by the user and use either the
>> Cycle counter (preferred) or fall back to chaining. That way, the user
>> has the control over the number of events he can count using the given
>> set of counters.
>
> Naively, there doesn't seem to be any inherent harm in always using 64-bit arithmetic for the dedicated counter, but it would mean that with multiple (non-chained) cycles events, some would be taking an interrupt every few seconds while one would effectively never overflow. I guess the question is whether that matters or not.
>
The problem is we can't have a mask per counter as we don't know where
the event would be placed until it is added. Or we should delay the
period initialisation/update to post get_event_idx().
Suzuki
^ permalink raw reply
* [PATCH 2/2] rtc: st-lpc: add range
From: kbuild test robot @ 2018-05-21 14:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180520123337.14856-2-alexandre.belloni@bootlin.com>
Hi Alexandre,
I love your patch! Yet something to improve:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.17-rc6 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Alexandre-Belloni/rtc-st-lpc-fix-possible-race-condition/20180521-192317
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All error/warnings (new ones prefixed by >>):
drivers/rtc/rtc-st-lpc.c: In function 'st_rtc_probe':
>> drivers/rtc/rtc-st-lpc.c:261:5: error: 'struct st_rtc' has no member named 'range_max'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~
In file included from arch/arm/include/asm/div64.h:127:0,
from include/linux/kernel.h:173,
from include/linux/clk.h:16,
from drivers/rtc/rtc-st-lpc.c:17:
>> include/asm-generic/div64.h:226:7: error: lvalue required as left operand of assignment
(n) >>= ilog2(__base); \
^
>> drivers/rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
>> include/asm-generic/div64.h:230:31: warning: large integer implicitly truncated to unsigned type [-Woverflow]
uint32_t __res_lo, __n_lo = (n); \
^
>> drivers/rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
include/asm-generic/div64.h:231:7: error: lvalue required as left operand of assignment
(n) = __div64_const32(n, __base); \
^
>> drivers/rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
include/asm-generic/div64.h:233:14: warning: large integer implicitly truncated to unsigned type [-Woverflow]
__res_lo = (n); \
^
>> drivers/rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
include/asm-generic/div64.h:237:7: error: lvalue required as left operand of assignment
(n) = (uint32_t)(n) / __base; \
^
>> drivers/rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
>> include/asm-generic/div64.h:239:22: error: lvalue required as unary '&' operand
__rem = __div64_32(&(n), __base); \
^
>> drivers/rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
--
drivers//rtc/rtc-st-lpc.c: In function 'st_rtc_probe':
drivers//rtc/rtc-st-lpc.c:261:5: error: 'struct st_rtc' has no member named 'range_max'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~
In file included from arch/arm/include/asm/div64.h:127:0,
from include/linux/kernel.h:173,
from include/linux/clk.h:16,
from drivers//rtc/rtc-st-lpc.c:17:
>> include/asm-generic/div64.h:226:7: error: lvalue required as left operand of assignment
(n) >>= ilog2(__base); \
^
drivers//rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
>> include/asm-generic/div64.h:230:31: warning: large integer implicitly truncated to unsigned type [-Woverflow]
uint32_t __res_lo, __n_lo = (n); \
^
drivers//rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
include/asm-generic/div64.h:231:7: error: lvalue required as left operand of assignment
(n) = __div64_const32(n, __base); \
^
drivers//rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
include/asm-generic/div64.h:233:14: warning: large integer implicitly truncated to unsigned type [-Woverflow]
__res_lo = (n); \
^
drivers//rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
include/asm-generic/div64.h:237:7: error: lvalue required as left operand of assignment
(n) = (uint32_t)(n) / __base; \
^
drivers//rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
>> include/asm-generic/div64.h:239:22: error: lvalue required as unary '&' operand
__rem = __div64_32(&(n), __base); \
^
drivers//rtc/rtc-st-lpc.c:261:19: note: in expansion of macro 'do_div'
rtc->range_max = do_div(U64_MAX, rtc->clkrate);
^~~~~~
vim +261 drivers/rtc/rtc-st-lpc.c
192
193 static int st_rtc_probe(struct platform_device *pdev)
194 {
195 struct device_node *np = pdev->dev.of_node;
196 struct st_rtc *rtc;
197 struct resource *res;
198 uint32_t mode;
199 int ret = 0;
200
201 ret = of_property_read_u32(np, "st,lpc-mode", &mode);
202 if (ret) {
203 dev_err(&pdev->dev, "An LPC mode must be provided\n");
204 return -EINVAL;
205 }
206
207 /* LPC can either run as a Clocksource or in RTC or WDT mode */
208 if (mode != ST_LPC_MODE_RTC)
209 return -ENODEV;
210
211 rtc = devm_kzalloc(&pdev->dev, sizeof(struct st_rtc), GFP_KERNEL);
212 if (!rtc)
213 return -ENOMEM;
214
215 rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
216 if (IS_ERR(rtc->rtc_dev))
217 return PTR_ERR(rtc->rtc_dev);
218
219 spin_lock_init(&rtc->lock);
220
221 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
222 rtc->ioaddr = devm_ioremap_resource(&pdev->dev, res);
223 if (IS_ERR(rtc->ioaddr))
224 return PTR_ERR(rtc->ioaddr);
225
226 rtc->irq = irq_of_parse_and_map(np, 0);
227 if (!rtc->irq) {
228 dev_err(&pdev->dev, "IRQ missing or invalid\n");
229 return -EINVAL;
230 }
231
232 ret = devm_request_irq(&pdev->dev, rtc->irq, st_rtc_handler, 0,
233 pdev->name, rtc);
234 if (ret) {
235 dev_err(&pdev->dev, "Failed to request irq %i\n", rtc->irq);
236 return ret;
237 }
238
239 enable_irq_wake(rtc->irq);
240 disable_irq(rtc->irq);
241
242 rtc->clk = clk_get(&pdev->dev, NULL);
243 if (IS_ERR(rtc->clk)) {
244 dev_err(&pdev->dev, "Unable to request clock\n");
245 return PTR_ERR(rtc->clk);
246 }
247
248 clk_prepare_enable(rtc->clk);
249
250 rtc->clkrate = clk_get_rate(rtc->clk);
251 if (!rtc->clkrate) {
252 dev_err(&pdev->dev, "Unable to fetch clock rate\n");
253 return -EINVAL;
254 }
255
256 device_set_wakeup_capable(&pdev->dev, 1);
257
258 platform_set_drvdata(pdev, rtc);
259
260 rtc->rtc_dev->ops = &st_rtc_ops;
> 261 rtc->range_max = do_div(U64_MAX, rtc->clkrate);
262
263 ret = rtc_register_device(rtc->rtc_dev);
264 if (ret) {
265 clk_disable_unprepare(rtc->clk);
266 return ret;
267 }
268
269 return 0;
270 }
271
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 43343 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180521/7bf5deb2/attachment-0001.gz>
^ permalink raw reply
* [PATCH v2 02/40] iommu/sva: Bind process address spaces to devices
From: Jean-Philippe Brucker @ 2018-05-21 14:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517141058.00001c76@huawei.com>
On 17/05/18 14:10, Jonathan Cameron wrote:
> On Fri, 11 May 2018 20:06:03 +0100
> Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:
>
>> Add bind() and unbind() operations to the IOMMU API. Bind() returns a
>> PASID that drivers can program in hardware, to let their devices access an
>> mm. This patch only adds skeletons for the device driver API, most of the
>> implementation is still missing.
>>
>> IOMMU groups with more than one device aren't supported for SVA at the
>> moment. There may be P2P traffic between devices within a group, which
>> cannot be seen by an IOMMU (note that supporting PASID doesn't add any
>> form of isolation with regard to P2P). Supporting groups would require
>> calling bind() for all bound processes every time a device is added to a
>> group, to perform sanity checks (e.g. ensure that new devices support
>> PASIDs at least as big as those already allocated in the group).
>
> Is it worth adding an explicit comment on this reasoning (or a minimal subset
> of it) at the check for the number of devices in the group?
> It's well laid out here, but might not be so obvious if someone is reading
> the code in the future.
Sure, I'll add something
Thanks,
Jean
^ permalink raw reply
* [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Jean-Philippe Brucker @ 2018-05-21 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517150516.000067ca@huawei.com>
On 17/05/18 15:25, Jonathan Cameron wrote:
>> +static struct io_mm *
>> +io_mm_alloc(struct iommu_domain *domain, struct device *dev,
>> + struct mm_struct *mm, unsigned long flags)
>> +{
>> + int ret;
>> + int pasid;
>> + struct io_mm *io_mm;
>> + struct iommu_sva_param *param = dev->iommu_param->sva_param;
>> +
>> + if (!domain->ops->mm_alloc || !domain->ops->mm_free)
>> + return ERR_PTR(-ENODEV);
>> +
>> + io_mm = domain->ops->mm_alloc(domain, mm, flags);
>> + if (IS_ERR(io_mm))
>> + return io_mm;
>> + if (!io_mm)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + /*
>> + * The mm must not be freed until after the driver frees the io_mm
>> + * (which may involve unpinning the CPU ASID for instance, requiring a
>> + * valid mm struct.)
>> + */
>> + mmgrab(mm);
>> +
>> + io_mm->flags = flags;
>> + io_mm->mm = mm;
>> + io_mm->release = domain->ops->mm_free;
>> + INIT_LIST_HEAD(&io_mm->devices);
>> +
>> + idr_preload(GFP_KERNEL);
>> + spin_lock(&iommu_sva_lock);
>> + pasid = idr_alloc(&iommu_pasid_idr, io_mm, param->min_pasid,
>> + param->max_pasid + 1, GFP_ATOMIC);
>
> I'd expect the IDR cleanup to be in io_mm_free as that would 'match'
> against io_mm_alloc but it's in io_mm_release just before the io_mm_free
> call, perhaps move it or am I missing something?
>
> Hmm. This is reworked in patch 5 to use call rcu to do the free. I suppose
> we may be burning an idr entry if we take a while to get round to the
> free.. If so a comment to explain this would be great.
Ok, I'll see if I can come up with some comments for both patch 3 and 5.
>> + io_mm->pasid = pasid;
>> + spin_unlock(&iommu_sva_lock);
>> + idr_preload_end();
>> +
>> + if (pasid < 0) {
>> + ret = pasid;
>> + goto err_free_mm;
>> + }
>> +
>> + /* TODO: keep track of mm. For the moment, abort. */
>
> From later patches, I can now see why we didn't init the kref
> here, but perhaps a comment would make that clear rather than
> people checking it is correctly used throughout? Actually just grab
> the comment from patch 5 and put it in this one and that will
> do the job nicely.
Ok
>> + ret = -ENOSYS;
>> + spin_lock(&iommu_sva_lock);
>> + idr_remove(&iommu_pasid_idr, io_mm->pasid);
>> + spin_unlock(&iommu_sva_lock);
>> +
>> +err_free_mm:
>> + domain->ops->mm_free(io_mm);
>
> Really minor, but you now have io_mm->release set so to keep
> this obviously the same as the io_mm_free path, perhaps call
> that rather than mm_free directly.
Yes, makes sense
>> +static void io_mm_detach_locked(struct iommu_bond *bond)
>> +{
>> + struct iommu_bond *tmp;
>> + bool detach_domain = true;
>> + struct iommu_domain *domain = bond->domain;
>> +
>> + list_for_each_entry(tmp, &domain->mm_list, domain_head) {
>> + if (tmp->io_mm == bond->io_mm && tmp->dev != bond->dev) {
>> + detach_domain = false;
>> + break;
>> + }
>> + }
>> +
>> + domain->ops->mm_detach(domain, bond->dev, bond->io_mm, detach_domain);
>> +
>
> I can't see an immediate reason to have a different order in her to the reverse of
> the attach above. So I think you should be detaching after the list_del calls.
> If there is a reason, can we have a comment so I don't ask on v10.
I don't see a reason either right now, I'll see if it can be moved
>
>> + list_del(&bond->mm_head);
>> + list_del(&bond->domain_head);
>> + list_del(&bond->dev_head);
>> + io_mm_put_locked(bond->io_mm);
>> + /* If an io_mm already exists, use it */
>> + spin_lock(&iommu_sva_lock);
>> + idr_for_each_entry(&iommu_pasid_idr, io_mm, i) {
>> + if (io_mm->mm == mm && io_mm_get_locked(io_mm)) {
>> + /* ... Unless it's already bound to this device */
>> + list_for_each_entry(tmp, &io_mm->devices, mm_head) {
>> + if (tmp->dev == dev) {
>> + bond = tmp;
>
> Using bond for this is clear in a sense, but can we not just use ret
> so it is obvious here that we are going to return -EEXIST?
> At first glance I thought you were going to carry on with this bond
> and couldn't work out why it would ever make sense to have two bonds
> between a device an an io_mm (which it doesn't!)
Yes, using ret is nicer
Thanks,
Jean
^ permalink raw reply
* [PATCH v2 05/40] iommu/sva: Track mm changes with an MMU notifier
From: Jean-Philippe Brucker @ 2018-05-21 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517152514.00004247@huawei.com>
On 17/05/18 15:25, Jonathan Cameron wrote:
>> + * already have been removed from the list. Check is someone is
>
> Check if someone...
Ok
Thanks,
Jean
^ permalink raw reply
* [PATCH v4 0/3] arm64: dts: Draak: Enable video inputs and VIN4
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
this series enables HDMI, CVBS and VIN4 for R8A77995 Draak board.
Compared to v3, the analog video decoder ADV7180 is now described in bindings
with the compatible string "adi,adv7180cp" and its port nodes definition
has been res-structured according to the chip bindings as reported by
Laurent on v3.
Bindings have been copi^W^Winspired from Gen2 Gose board, the only Gen2 board
using the "adv7180cp" compatible string.
Example of how to switch to HDMI input is still available here:
git://jmondi.org/linux d3/media-master/salvator-x-dts_csi2/d3-hdmi
And the series still depends on
[PATCH v3 0/9] rcar-vin: Add support for parallel input on Gen3
Thanks
j
Jacopo Mondi (3):
dt-bindings: media: rcar-vin: Add R8A77995 support
arm64: dts: renesas: draak: Describe CVBS input
arm64: dts: renesas: draak: Describe HDMI input
v3 -> v4:
- Use 'adi,adv7180cp' in [3/3] as detailed in the commit message
- Add Laurent's R-b tag to [2/3]
v2 -> v3:
- Add comment to CVBS and HDMI inputs
- Add missing Signed-off-by to [2/3]
- Add Niklas' tag
.../devicetree/bindings/media/rcar_vin.txt | 1 +
arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 117 +++++++++++++++++++++
2 files changed, 118 insertions(+)
--
2.7.4
^ permalink raw reply
* [PATCH v4 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526913942-15426-1-git-send-email-jacopo+renesas@jmondi.org>
Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
rcar-vin driver.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..5c6f2a7 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -22,6 +22,7 @@ on Gen3 platforms to a CSI-2 receiver.
- "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,vin-r8a7796" for the R8A7796 device
- "renesas,vin-r8a77970" for the R8A77970 device
+ - "renesas,vin-r8a77995" for the R8A77995 device
- "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
device.
--
2.7.4
^ permalink raw reply related
* [PATCH v4 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526913942-15426-1-git-send-email-jacopo+renesas@jmondi.org>
Describe CVBS video input through analog video decoder ADV7180
connected to video input interface VIN4.
The video input signal path is shared with HDMI video input, and
selected by on-board switches SW-53 and SW-54 with CVBS input selected
by the default switches configuration.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
v3 -> v4:
- Use 'adi,adv7180cp' compatible string in place of just 'adi,adv7180'
as suggested by Laurent.
- Re-structure CVBS input ports definition according to adv7180cp
bindings: add port at 0 for composite input, add port at 3 for parallel
video output.
- Change node label to 'composite-in' as in Gose board bindings.
v2 -> v3:
- Add comment to describe the shared input video path.
- Add my SoB and Niklas' R-b tags.
---
arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 9d73de8..ad59032 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -59,6 +59,16 @@
};
};
+ composite-in {
+ compatible = "composite-video-connector";
+
+ port {
+ composite_con_in: endpoint {
+ remote-endpoint = <&adv7180_in>;
+ };
+ };
+ };
+
memory at 48000000 {
device_type = "memory";
/* first 128MB is reserved for secure area. */
@@ -142,6 +152,11 @@
groups = "usb0";
function = "usb0";
};
+
+ vin4_pins_cvbs: vin4 {
+ groups = "vin4_data8", "vin4_sync", "vin4_clk";
+ function = "vin4";
+ };
};
&i2c0 {
@@ -154,6 +169,39 @@
reg = <0x50>;
pagesize = <8>;
};
+
+ composite-in at 20 {
+ compatible = "adi,adv7180cp";
+ reg = <0x20>;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+ adv7180_in: endpoint {
+ remote-endpoint = <&composite_con_in>;
+ };
+ };
+
+ port at 3 {
+ reg = <3>;
+
+ /*
+ * The VIN4 video input path is shared between
+ * CVBS and HDMI inputs through SW[49-53]
+ * switches.
+ *
+ * CVBS is the default selection, link it to
+ * VIN4 here.
+ */
+ adv7180_out: endpoint {
+ remote-endpoint = <&vin4_in>;
+ };
+ };
+ };
+ };
};
&i2c1 {
@@ -246,3 +294,23 @@
timeout-sec = <60>;
status = "okay";
};
+
+&vin4 {
+ pinctrl-0 = <&vin4_pins_cvbs>;
+ pinctrl-names = "default";
+
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+
+ vin4_in: endpoint {
+ remote-endpoint = <&adv7180_out>;
+ };
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v4 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526913942-15426-1-git-send-email-jacopo+renesas@jmondi.org>
Describe HDMI input connector and ADV7612 HDMI decoder installed on
R-Car Gen3 Draak board.
The video signal routing to the HDMI decoder to the video input interface
VIN4 is multiplexed with CVBS input path, and enabled/disabled through
on-board switches SW-49, SW-50, SW-51 and SW-52.
As the default board switches configuration connects CVBS input to VIN4,
leave the HDMI decoder unconnected in DTS.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v3 -> v4:
- Add Laurent's R-b tag.
v2 -> v3:
- Add comment on HDMI output port about the shared CVBS/HDMI video path.
- Add Niklas' R-b tag.
---
arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 49 ++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index ad59032..1a474f94 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -69,6 +69,17 @@
};
};
+ hdmi-in {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_con_in: endpoint {
+ remote-endpoint = <&adv7612_in>;
+ };
+ };
+ };
+
memory at 48000000 {
device_type = "memory";
/* first 128MB is reserved for secure area. */
@@ -201,6 +212,44 @@
};
};
};
+
+ };
+
+ hdmi-decoder at 4c {
+ compatible = "adi,adv7612";
+ reg = <0x4c>;
+ default-input = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port at 0 {
+ reg = <0>;
+
+ adv7612_in: endpoint {
+ remote-endpoint = <&hdmi_con_in>;
+ };
+ };
+
+ port at 2 {
+ reg = <2>;
+
+ /*
+ * The VIN4 video input path is shared between
+ * CVBS and HDMI inputs through SW[49-53]
+ * switches.
+ *
+ * CVBS is the default selection, leave HDMI
+ * not connected here.
+ */
+ adv7612_out: endpoint {
+ pclk-sample = <0>;
+ hsync-active = <0>;
+ vsync-active = <0>;
+ };
+ };
+ };
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH v2 07/40] iommu: Add a page fault handler
From: Jean-Philippe Brucker @ 2018-05-21 14:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517162555.00002bd3@huawei.com>
On 17/05/18 16:25, Jonathan Cameron wrote:
> On Fri, 11 May 2018 20:06:08 +0100
> Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:
>
>> Some systems allow devices to handle I/O Page Faults in the core mm. For
>> example systems implementing the PCI PRI extension or Arm SMMU stall
>> model. Infrastructure for reporting these recoverable page faults was
>> recently added to the IOMMU core for SVA virtualisation. Add a page fault
>> handler for host SVA.
>>
>> IOMMU driver can now instantiate several fault workqueues and link them to
>> IOPF-capable devices. Drivers can choose between a single global
>> workqueue, one per IOMMU device, one per low-level fault queue, one per
>> domain, etc.
>>
>> When it receives a fault event, supposedly in an IRQ handler, the IOMMU
>> driver reports the fault using iommu_report_device_fault(), which calls
>> the registered handler. The page fault handler then calls the mm fault
>> handler, and reports either success or failure with iommu_page_response().
>> When the handler succeeded, the IOMMU retries the access.
>>
>> The iopf_param pointer could be embedded into iommu_fault_param. But
>> putting iopf_param into the iommu_param structure allows us not to care
>> about ordering between calls to iopf_queue_add_device() and
>> iommu_register_device_fault_handler().
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>
> Hi Jean-Phillipe,
>
> One question below on how we would end up with partial faults left when
> doing the queue remove. Code looks fine, but I'm not seeing how that
> would happen without buggy hardware... + we presumably have to rely on
> the hardware timing out on that request or it's dead anyway...
>> +/**
>> + * iopf_queue_remove_device - Remove producer from fault queue
>> + * @dev: device to remove
>> + *
>> + * Caller makes sure that no more fault is reported for this device, and no more
>> + * flush is scheduled for this device.
>> + *
>> + * Note: safe to call unconditionally on a cleanup path, even if the device
>> + * isn't registered to any IOPF queue.
>> + *
>> + * Return 0 if the device was attached to the IOPF queue
>> + */
>> +int iopf_queue_remove_device(struct device *dev)
>> +{
>> + struct iopf_context *fault, *next;
>> + struct iopf_device_param *iopf_param;
>> + struct iommu_param *param = dev->iommu_param;
>> +
>> + if (!param)
>> + return -EINVAL;
>> +
>> + mutex_lock(¶m->lock);
>> + iopf_param = param->iopf_param;
>> + if (iopf_param) {
>> + refcount_dec(&iopf_param->queue->refs);
>> + param->iopf_param = NULL;
>> + }
>> + mutex_unlock(¶m->lock);
>> + if (!iopf_param)
>> + return -EINVAL;
>> +
>> + list_for_each_entry_safe(fault, next, &iopf_param->partial, head)
>> + kfree(fault);
>> +
>
> Why would we end up here with partials still in the list? Buggy hardware?
Buggy hardware is one possibility. There also is the nasty case of PRI
queue overflow. If the PRI queue is full, then the SMMU discards new
Page Requests from the device. It may discard a 'last' PR of a group
that is already in iopf_param->partial. This group will never be freed
until this cleanup.
The spec dismisses PRIq overflows because the OS is supposed to allocate
PRI credits to devices such that they can't send more than the PRI queue
size. This isn't possible in Linux because we don't know exactly how
many PRI-capable devices will share a queue (the upper limit is 2**32,
and devices may be hotplugged well after we allocated the PRI queue). So
PRIq overflow is possible.
Ideally when detecting a PRIq overflow we need to immediately remove all
partial faults of all devices sharing this queue. Since I can't easily
test that case (my device doesn't do PRGs) and it's complicated code, I
left it as TODO in patch 39, and this is our only chance to free
orphaned page requests.
>> +void iopf_queue_free(struct iopf_queue *queue)
>> +{
>> +
>
> Nitpick : Blank line here doesn't add anything.
Ok
Thanks,
Jean
^ permalink raw reply
* [PATCH v9 12/16] KVM: arm64: Save host SVE context as appropriate
From: Dave Martin @ 2018-05-21 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <eac5821b-c5e0-90c9-cdd6-73643662d68d@arm.com>
On Mon, May 21, 2018 at 03:40:02PM +0100, Marc Zyngier wrote:
> On 21/05/18 15:17, Dave Martin wrote:
> > This patch adds SVE context saving to the hyp FPSIMD context switch
> > path. This means that it is no longer necessary to save the host
> > SVE state in advance of entering the guest, when in use.
> >
> > In order to avoid adding pointless complexity to the code, VHE is
> > assumed if SVE is in use. VHE is an architectural prerequisite for
> > SVE, so there is no good reason to turn CONFIG_ARM64_VHE off in
> > kernels that support both SVE and KVM.
> >
> > Historically, software models exist that can expose the
> > architecturally invalid configuration of SVE without VHE, so if
> > this situation is detected at kvm_init() time then KVM will be
> > disabled.
> >
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> >
> > ---
> >
> > * Stripped the following tags, reviewers please reconfirm:
> >
> > Formerly-Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
> > Formerly-Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > Formerly-Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> > (Creation of a new file for this one change may be deemed undesirable,
> > but there didn't seem to be a correct place to put it. There may also
> > be a way around the circular include problem that I missed.)
> >
> > Changes since v8:
> >
> > * Add kvm_arch_check_supported() hook, and move arm64-specific check
> > for SVE-implies-VHE into arch/arm64/.
> >
> > Due to circular header dependency problems, it is difficult to get
> > the prototype for kvm_pr_*() functions in <asm/kvm_host.h>, so this
> > patch puts arm64's kvm_arch_check_supported() hook out of line.
> > This is not a hot function.
> > ---
> > arch/arm/include/asm/kvm_host.h | 1 +
> > arch/arm64/Kconfig | 7 +++++++
> > arch/arm64/include/asm/kvm_host.h | 1 +
> > arch/arm64/kvm/Makefile | 2 +-
> > arch/arm64/kvm/fpsimd.c | 1 -
> > arch/arm64/kvm/hyp/switch.c | 20 +++++++++++++++++++-
> > arch/arm64/kvm/init.c | 33 +++++++++++++++++++++++++++++++++
> > virt/kvm/arm/arm.c | 6 ++++++
> > 8 files changed, 68 insertions(+), 3 deletions(-)
> > create mode 100644 arch/arm64/kvm/init.c
> >
[...]
> > diff --git a/arch/arm64/kvm/init.c b/arch/arm64/kvm/init.c
> > new file mode 100644
> > index 0000000..3b6e730
> > --- /dev/null
> > +++ b/arch/arm64/kvm/init.c
> > @@ -0,0 +1,33 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * arch/arm64/kvm/init.c: KVM initialisation support
> > + *
> > + * Copyright 2018 Arm Limited
> > + * Author: Dave Martin <Dave.Martin@arm.com>
> > + */
> > +#include <linux/errno.h>
> > +#include <linux/kvm_host.h>
> > +#include <asm/cpufeature.h>
> > +#include <asm/kvm_host.h>
> > +
> > +/* Additional arch-dependent checks for KVM usability */
> > +int kvm_arch_check_supported(void)
> > +{
> > + /*
> > + * VHE is a prerequisite for SVE in the Arm architecture, and
> > + * Kconfig ensures that if system_supports_sve() here then
> > + * CONFIG_ARM64_VHE is enabled, so if VHE support wasn't already
> > + * detected and enabled, the CPU is architecturally
> > + * noncompliant.
> > + *
> > + * Just in case this mismatch is seen, detect it, warn and give
> > + * up. Supporting this forbidden configuration in Hyp would be
> > + * pointless.
> > + */
> > + if (system_supports_sve() && !has_vhe()) {
> > + kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
>
> [entering bikeshedding territory]
>
> I'm not exactly keen on this. This feels overkill, and a departure from
> what we've been doing so far.
>
> Why don't you simply have a helper in kvm_host.h that does:
>
> static inline bool kvm_arm_check_sve_valid(void)
> {
> return !system_supports_sve() || has_vhe();
> }
>
> (and a canonical "return true;" implementation for 32bit)
>
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index bee226c..8518df0 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
[...]
> > @@ -1574,6 +1576,10 @@ int kvm_arch_init(void *opaque)
> > return -ENODEV;
> > }
> >
> > + err = kvm_arch_check_supported();
> > + if (err)
> > + return err;
> > +
>
> And turn this into:
>
> if (!kvm_arm_check_sve_valid()) {
> kvm_pr_unimpl("SVE system without VHE unsupported. Broken cpu?");
> return -EINVAL;
> }
>
> It has the advantage of being obvious of what we check for, and doesn't
> add any extra file.
Making the call special-purpose makes it more natural to pull the printk
out of the call, solving the header issues.
This would be a bit gross in core code, but arm.c is supported to be
Arm-specific, so if you prefer this approach it doesn't look too bad
here.
I'm happy to rework along these lines.
Cheers
---Dave
^ permalink raw reply
* [PATCH v2 07/40] iommu: Add a page fault handler
From: Jean-Philippe Brucker @ 2018-05-21 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518110434.150a0e64@jacob-builder>
On 18/05/18 19:04, Jacob Pan wrote:
>> +struct iopf_context {
>> + struct device *dev;
>> + struct iommu_fault_event evt;
>> + struct list_head head;
>> +};
>> +
>> +struct iopf_group {
>> + struct iopf_context last_fault;
>> + struct list_head faults;
>> + struct work_struct work;
>> +};
>> +
> All the page requests in the group should belong to the same device,
> perhaps struct device tracking should be in iopf_group instead of
> iopf_context?
Right, this is a leftover from when we kept a global list of partial
faults. Since the list is now per-device, I can move the dev pointer (I
think I should also rename iopf_context to iopf_fault, if that's all right)
>> +static int iopf_complete(struct device *dev, struct
>> iommu_fault_event *evt,
>> + enum page_response_code status)
>> +{
>> + struct page_response_msg resp = {
>> + .addr = evt->addr,
>> + .pasid = evt->pasid,
>> + .pasid_present = evt->pasid_valid,
>> + .page_req_group_id = evt->page_req_group_id,
>> + .private_data = evt->iommu_private,
>> + .resp_code = status,
>> + };
>> +
>> + return iommu_page_response(dev, &resp);
>> +}
>> +
>> +static enum page_response_code
>> +iopf_handle_single(struct iopf_context *fault)
>> +{
>> + /* TODO */
>> + return -ENODEV;
>> +}
>> +
>> +static void iopf_handle_group(struct work_struct *work)
>> +{
>> + struct iopf_group *group;
>> + struct iopf_context *fault, *next;
>> + enum page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
>> +
>> + group = container_of(work, struct iopf_group, work);
>> +
>> + list_for_each_entry_safe(fault, next, &group->faults, head) {
>> + struct iommu_fault_event *evt = &fault->evt;
>> + /*
>> + * Errors are sticky: don't handle subsequent faults
>> in the
>> + * group if there is an error.
>> + */
> There might be performance benefit for certain device to continue in
> spite of error in that the ATS retry may have less fault. Perhaps a
> policy decision for later enhancement.
Yes I think we should leave it to future work. My current reasoning is
that subsequent requests are more likely to fail as well and reporting
the error is more urgent, but we need real workloads to confirm this.
>> + if (status == IOMMU_PAGE_RESP_SUCCESS)
>> + status = iopf_handle_single(fault);
>> +
>> + if (!evt->last_req)
>> + kfree(fault);
>> + }
>> +
>> + iopf_complete(group->last_fault.dev, &group->last_fault.evt,
>> status);
>> + kfree(group);
>> +}
>> +
>> +/**
>> + * iommu_queue_iopf - IO Page Fault handler
>> + * @evt: fault event
>> + * @cookie: struct device, passed to
>> iommu_register_device_fault_handler.
>> + *
>> + * Add a fault to the device workqueue, to be handled by mm.
>> + */
>> +int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie)
>> +{
>> + struct iopf_group *group;
>> + struct iopf_context *fault, *next;
>> + struct iopf_device_param *iopf_param;
>> +
>> + struct device *dev = cookie;
>> + struct iommu_param *param = dev->iommu_param;
>> +
>> + if (WARN_ON(!mutex_is_locked(¶m->lock)))
>> + return -EINVAL;
>> +
>> + if (evt->type != IOMMU_FAULT_PAGE_REQ)
>> + /* Not a recoverable page fault */
>> + return IOMMU_PAGE_RESP_CONTINUE;
>> +
>> + /*
>> + * As long as we're holding param->lock, the queue can't be
>> unlinked
>> + * from the device and therefore cannot disappear.
>> + */
>> + iopf_param = param->iopf_param;
>> + if (!iopf_param)
>> + return -ENODEV;
>> +
>> + if (!evt->last_req) {
>> + fault = kzalloc(sizeof(*fault), GFP_KERNEL);
>> + if (!fault)
>> + return -ENOMEM;
>> +
>> + fault->evt = *evt;
>> + fault->dev = dev;
>> +
>> + /* Non-last request of a group. Postpone until the
>> last one */
>> + list_add(&fault->head, &iopf_param->partial);
>> +
>> + return IOMMU_PAGE_RESP_HANDLED;
>> + }
>> +
>> + group = kzalloc(sizeof(*group), GFP_KERNEL);
>> + if (!group)
>> + return -ENOMEM;
>> +
>> + group->last_fault.evt = *evt;
>> + group->last_fault.dev = dev;
>> + INIT_LIST_HEAD(&group->faults);
>> + list_add(&group->last_fault.head, &group->faults);
>> + INIT_WORK(&group->work, iopf_handle_group);
>> +
>> + /* See if we have partial faults for this group */
>> + list_for_each_entry_safe(fault, next, &iopf_param->partial,
>> head) {
>> + if (fault->evt.page_req_group_id ==
>> evt->page_req_group_id)
> If all 9 bit group index are used, there can be lots of PRGs. For
> future enhancement, maybe we can have per group partial list ready to
> go when LPIG comes in? I will be less searching.
Yes, allocating the PRG from the start could be more efficient. I think
it's slightly more complicated code so I'd rather see performance
numbers before implementing it
>> + /* Insert *before* the last fault */
>> + list_move(&fault->head, &group->faults);
>> + }
>> +
> If you already sorted the group list with last fault at the end, why do
> you need a separate entry to track the last fault?
Not sure I understand your question, sorry. Do you mean why the
iopf_group.last_fault? Just to avoid one more kzalloc.
>> +
>> + queue->flush(queue->flush_arg, dev);
>> +
>> + /*
>> + * No need to clear the partial list. All PRGs containing
>> the PASID that
>> + * needs to be decommissioned are whole (the device driver
>> made sure of
>> + * it before this function was called). They have been
>> submitted to the
>> + * queue by the above flush().
>> + */
> So you are saying device driver need to make sure LPIG PRQ is submitted
> in the flush call above such that partial list is cleared?
Not exactly, it's the IOMMU driver that makes sure all LPIG in its
queues are submitted by the above flush call. In more details the flow is:
* Either device driver calls unbind()/sva_device_shutdown(), or the
process exits.
* If the device driver called, then it already told the device to stop
using the PASID. Otherwise we use the mm_exit() callback to tell the
device driver to stop using the PASID.
* In either case, when receiving a stop request from the driver, the
device sends the LPIGs to the IOMMU queue.
* Then, the flush call above ensures that the IOMMU reports the LPIG
with iommu_report_device_fault.
* While submitting all LPIGs for this PASID to the work queue,
ipof_queue_fault also picked up all partial faults, so the partial list
is clean.
Maybe I should improve this comment?
> what if
> there are device failures where device needs to reset (either whole
> function level or PASID level), should there still be a need to clear
> the partial list for all associated PASID/group?
I guess the simplest way out, when resetting the device, is for the
device driver to call iommu_sva_device_shutdown(). Both the IOMMU
driver's sva_device_shutdown() and remove_device() ops should call
iopf_queue_remove_device(), which clears the partial list.
Thanks,
Jean
^ permalink raw reply
* [PATCH 1/2] drm/fourcc: add a 10bits fully packed variant of NV12
From: Ville Syrjälä @ 2018-05-21 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180520171705.29690-2-ayaka@soulik.info>
On Mon, May 21, 2018 at 01:17:04AM +0800, Randy Li wrote:
> This pixel format is a fully packed and 10bits variant of NV12.
> A luma pixel would take 10bits in memory, without any
> filled bits between pixels in a stride. The color gamut
> follows the BT.2020 standard.
>
> Signed-off-by: Randy Li <ayaka@soulik.info>
> ---
> drivers/gpu/drm/drm_fourcc.c | 1 +
> include/uapi/drm/drm_fourcc.h | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 5ca6395cd4d3..1f43967c4013 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -173,6 +173,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
> { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
> + { .format = DRM_FORMAT_NV12_10LE40, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
> };
>
> unsigned int i;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index e04613d30a13..8eabf01e966f 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -140,6 +140,9 @@ extern "C" {
> #define DRM_FORMAT_NV61 fourcc_code('N', 'V', '6', '1') /* 2x1 subsampled Cb:Cr plane */
> #define DRM_FORMAT_NV24 fourcc_code('N', 'V', '2', '4') /* non-subsampled Cr:Cb plane */
> #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
> +/* A fully packed variant of NV12_10LE32 */
What does "fully packed" mean? NV12_10LE32 doesn't even exist so
referring to it makes no sense.
Please try to provide an unambiguous description of new formats like we
have for everything else.
> +#define DRM_FORMAT_NV12_10LE40 fourcc_code('R', 'K', '2', '0') /* 2x2 subsampled Cr:Cb plane */
> +
>
> /*
> * 3 plane YCbCr
> --
> 2.14.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrj?l?
Intel
^ permalink raw reply
* [PATCH v2 17/40] iommu/arm-smmu-v3: Link domains and devices
From: Jean-Philippe Brucker @ 2018-05-21 14:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517170748.00004927@huawei.com>
On 17/05/18 17:07, Jonathan Cameron wrote:
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -595,6 +595,11 @@ struct arm_smmu_device {
>> struct arm_smmu_master_data {
>> struct arm_smmu_device *smmu;
>> struct arm_smmu_strtab_ent ste;
>> +
>> + struct arm_smmu_domain *domain;
>> + struct list_head list; /* domain->devices */
>
> More meaningful name perhaps to avoid the need for the comment?
Sure
Thanks,
Jean
^ permalink raw reply
* [PATCH v4 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: Laurent Pinchart @ 2018-05-21 14:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526913942-15426-3-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thank you for the patch.
On Monday, 21 May 2018 17:45:41 EEST Jacopo Mondi wrote:
> Describe CVBS video input through analog video decoder ADV7180
> connected to video input interface VIN4.
>
> The video input signal path is shared with HDMI video input, and
> selected by on-board switches SW-53 and SW-54 with CVBS input selected
> by the default switches configuration.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> v3 -> v4:
> - Use 'adi,adv7180cp' compatible string in place of just 'adi,adv7180'
> as suggested by Laurent.
> - Re-structure CVBS input ports definition according to adv7180cp
> bindings: add port at 0 for composite input, add port at 3 for parallel
> video output.
> - Change node label to 'composite-in' as in Gose board bindings.
>
> v2 -> v3:
> - Add comment to describe the shared input video path.
> - Add my SoB and Niklas' R-b tags.
> ---
> arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68
> ++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 9d73de8..ad59032
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -59,6 +59,16 @@
> };
> };
>
> + composite-in {
> + compatible = "composite-video-connector";
> +
> + port {
> + composite_con_in: endpoint {
> + remote-endpoint = <&adv7180_in>;
> + };
> + };
> + };
> +
> memory at 48000000 {
> device_type = "memory";
> /* first 128MB is reserved for secure area. */
> @@ -142,6 +152,11 @@
> groups = "usb0";
> function = "usb0";
> };
> +
> + vin4_pins_cvbs: vin4 {
> + groups = "vin4_data8", "vin4_sync", "vin4_clk";
> + function = "vin4";
> + };
> };
>
> &i2c0 {
> @@ -154,6 +169,39 @@
> reg = <0x50>;
> pagesize = <8>;
> };
> +
> + composite-in at 20 {
> + compatible = "adi,adv7180cp";
> + reg = <0x20>;
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> + adv7180_in: endpoint {
> + remote-endpoint = <&composite_con_in>;
> + };
> + };
> +
> + port at 3 {
> + reg = <3>;
> +
> + /*
> + * The VIN4 video input path is shared between
> + * CVBS and HDMI inputs through SW[49-53]
> + * switches.
> + *
> + * CVBS is the default selection, link it to
> + * VIN4 here.
> + */
> + adv7180_out: endpoint {
> + remote-endpoint = <&vin4_in>;
> + };
> + };
> + };
> + };
> };
>
> &i2c1 {
> @@ -246,3 +294,23 @@
> timeout-sec = <60>;
> status = "okay";
> };
> +
> +&vin4 {
> + pinctrl-0 = <&vin4_pins_cvbs>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> +
> + vin4_in: endpoint {
> + remote-endpoint = <&adv7180_out>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH V7] ARM: dts: da850-evm: Enable LCD and Backlight
From: Sekhar Nori @ 2018-05-21 14:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518203335.13878-1-aford173@gmail.com>
On Saturday 19 May 2018 02:03 AM, Adam Ford wrote:
> When using the board files the LCD works, but not with the DT.
> This adds enables the original da850-evm to work with the same
> LCD in device tree mode.
>
> The EVM has a gpio for the regulator and a PWM for dimming the
> backlight. The LCD and the vpif display pins are mutually
> exclusive, so if using the LCD, do not load the vpif driver.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index 0e82bb988fde..a76c2ddfd23e 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -27,6 +27,60 @@
> spi0 = &spi1;
> };
>
> + backlight: backlight-pwm {
> + pinctrl-names = "default";
> + pinctrl-0 = <&ecap2_pins>;
> + power-supply = <&backlight_lcd>;
> + compatible = "pwm-backlight";
> + pwms = <&ecap2 0 50000 0>;
It will be nice to add a comment here: "The PWM here corresponds to
production hardware. The schematic needs to be 1015171 (15 March 2010),
Rev A or newer."
> + brightness-levels = <0 10 20 30 40 50 60 70 80 90 99>;
> + default-brightness-level = <7>;
> + };
> +
> + panel {
> + compatible = "ti,tilcdc,panel";
> + pinctrl-names = "default";
> + pinctrl-0 = <&lcd_pins>;
> + /*
> + * The vpif and the LCD are mutually exclusive.
> + * To enable VPIF, change the status below to 'disabled' then
> + * then change the status of the vpif below to 'okay'
> + */
This results in checkpatch warning:
[PATCH V7] ARM: dts: da850-evm: Enable LCD and Backlight.eml:153: WARNING: Block comments should align the * on each line
[PATCH V7] ARM: dts: da850-evm: Enable LCD and Backlight.eml:239: WARNING: Block comments should align the * on each line
> &vpif {
> pinctrl-names = "default";
> pinctrl-0 = <&vpif_capture_pins>, <&vpif_display_pins>;
> - status = "okay";
> + /*
> + * The vpif and the LCD are mutually exclusive.
> + * To enable VPIF, disable the ti,tilcdc,panel then
> + * changed the status below to 'okay'
> + */
> + status = "disabled";
Are you able to see VPIF is disabled after this? Trying your patch, I
still see VPIF probing[1]. Also, only VPIF display has a conflict with
LCD, correct (capture should be fine)?
Thanks,
Sekhar
[1]
[ 34.739314] adv7343 0-002a: chip found @ 0x54 (DaVinci I2C adapter)
[ 34.881422] vpif_display vpif_display: Pixel details: Width = 720,Height = 480
[ 34.881506] vpif_display vpif_display: channel=5bf2d970,channel->video_dev=5bf2d970
[ 34.883374] vpif_display vpif_display: Pixel details: Width = 720,Height = 480
[ 34.883450] vpif_display vpif_display: channel=16041996,channel->video_dev=16041996
[ 35.370777] tvp514x 0-005d: tvp514x 0-005d decoder driver registered !!
[ 35.470088] vpif_capture vpif_capture: registered sub device tvp514x-0
[ 35.827492] tvp514x 0-005c: tvp514x 0-005c decoder driver registered !!
[ 35.866031] vpif_capture vpif_capture: registered sub device tvp514x-1
[ 35.953752] vpif_capture vpif_capture: VPIF capture driver initialized
# ls /dev/video
video0 video1 video2 video3
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox