* [PATCH v2 5/5] KVM: arm64: Exclude FP ownership from kvm_vcpu_arch
From: Marc Zyngier @ 2024-03-22 17:09 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu,
James Clark, Anshuman Khandual, Mark Brown, Dongli Zhang
In-Reply-To: <20240322170945.3292593-1-maz@kernel.org>
In retrospect, it is fairly obvious that the FP state ownership
is only meaningful for a given CPU, and that locating this
information in the vcpu was just a mistake.
Move the ownership tracking into the host data structure, and
rename it from fp_state to fp_owner, which is a better description
(name suggested by Mark Brown).
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/kvm_emulate.h | 4 ++--
arch/arm64/include/asm/kvm_host.h | 14 +++++++-------
arch/arm64/kvm/arm.c | 6 ------
arch/arm64/kvm/fpsimd.c | 10 +++++-----
arch/arm64/kvm/hyp/include/hyp/switch.h | 6 +++---
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 2 --
arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
arch/arm64/kvm/hyp/vhe/switch.c | 2 +-
8 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index debc3753d2ef..b17f2269fb81 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -594,7 +594,7 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN);
if (!vcpu_has_sve(vcpu) ||
- (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED))
+ (*host_data_ptr(fp_owner) != FP_STATE_GUEST_OWNED))
val |= CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN;
if (cpus_have_final_cap(ARM64_SME))
val |= CPACR_EL1_SMEN_EL1EN | CPACR_EL1_SMEN_EL0EN;
@@ -602,7 +602,7 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu)
val = CPTR_NVHE_EL2_RES1;
if (vcpu_has_sve(vcpu) &&
- (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED))
+ (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED))
val |= CPTR_EL2_TZ;
if (cpus_have_final_cap(ARM64_SME))
val &= ~CPTR_EL2_TSM;
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 838cdee2ecf7..951303e976de 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -545,6 +545,13 @@ struct kvm_host_data {
struct kvm_cpu_context host_ctxt;
struct user_fpsimd_state *fpsimd_state; /* hyp VA */
+ /* Ownership of the FP regs */
+ enum {
+ FP_STATE_FREE,
+ FP_STATE_HOST_OWNED,
+ FP_STATE_GUEST_OWNED,
+ } fp_owner;
+
/*
* host_debug_state contains the host registers which are
* saved and restored during world switches.
@@ -621,13 +628,6 @@ struct kvm_vcpu_arch {
/* Exception Information */
struct kvm_vcpu_fault_info fault;
- /* Ownership of the FP regs */
- enum {
- FP_STATE_FREE,
- FP_STATE_HOST_OWNED,
- FP_STATE_GUEST_OWNED,
- } fp_state;
-
/* Configuration flags, set once and for all before the vcpu can run */
u8 cflags;
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index a24287c3ba99..66d8112da268 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -378,12 +378,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO;
- /*
- * Default value for the FP state, will be overloaded at load
- * time if we support FP (pretty likely)
- */
- vcpu->arch.fp_state = FP_STATE_FREE;
-
/* Set up the timer */
kvm_timer_vcpu_init(vcpu);
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index e6bd99358615..98f8b9272e24 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -84,7 +84,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
* guest in kvm_arch_vcpu_ctxflush_fp() and override this to
* FP_STATE_FREE if the flag set.
*/
- vcpu->arch.fp_state = FP_STATE_HOST_OWNED;
+ *host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
*host_data_ptr(fpsimd_state) = kern_hyp_va(¤t->thread.uw.fpsimd_state);
vcpu_clear_flag(vcpu, HOST_SVE_ENABLED);
@@ -109,7 +109,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
* been saved, this is very unlikely to happen.
*/
if (read_sysreg_s(SYS_SVCR) & (SVCR_SM_MASK | SVCR_ZA_MASK)) {
- vcpu->arch.fp_state = FP_STATE_FREE;
+ *host_data_ptr(fp_owner) = FP_STATE_FREE;
fpsimd_save_and_flush_cpu_state();
}
}
@@ -125,7 +125,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
void kvm_arch_vcpu_ctxflush_fp(struct kvm_vcpu *vcpu)
{
if (test_thread_flag(TIF_FOREIGN_FPSTATE))
- vcpu->arch.fp_state = FP_STATE_FREE;
+ *host_data_ptr(fp_owner) = FP_STATE_FREE;
}
/*
@@ -141,7 +141,7 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
WARN_ON_ONCE(!irqs_disabled());
- if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) {
+ if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED) {
/*
* Currently we do not support SME guests so SVCR is
@@ -194,7 +194,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
isb();
}
- if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) {
+ if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED) {
if (vcpu_has_sve(vcpu)) {
__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 6def6ad8dd48..2629420d0659 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -42,7 +42,7 @@ extern struct kvm_exception_table_entry __stop___kvm_ex_table;
/* Check whether the FP regs are owned by the guest */
static inline bool guest_owns_fp_regs(struct kvm_vcpu *vcpu)
{
- return vcpu->arch.fp_state == FP_STATE_GUEST_OWNED;
+ return *host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED;
}
/* Save the 32-bit only FPSIMD system register state */
@@ -376,7 +376,7 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
isb();
/* Write out the host state if it's in the registers */
- if (vcpu->arch.fp_state == FP_STATE_HOST_OWNED)
+ if (*host_data_ptr(fp_owner) == FP_STATE_HOST_OWNED)
__fpsimd_save_state(*host_data_ptr(fpsimd_state));
/* Restore the guest state */
@@ -389,7 +389,7 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
if (!(read_sysreg(hcr_el2) & HCR_RW))
write_sysreg(__vcpu_sys_reg(vcpu, FPEXC32_EL2), fpexc32_el2);
- vcpu->arch.fp_state = FP_STATE_GUEST_OWNED;
+ *host_data_ptr(fp_owner) = FP_STATE_GUEST_OWNED;
return true;
}
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index c5f625dc1f07..26561c562f7a 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -39,7 +39,6 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
hyp_vcpu->vcpu.arch.cptr_el2 = host_vcpu->arch.cptr_el2;
hyp_vcpu->vcpu.arch.iflags = host_vcpu->arch.iflags;
- hyp_vcpu->vcpu.arch.fp_state = host_vcpu->arch.fp_state;
hyp_vcpu->vcpu.arch.debug_ptr = kern_hyp_va(host_vcpu->arch.debug_ptr);
@@ -63,7 +62,6 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
host_vcpu->arch.fault = hyp_vcpu->vcpu.arch.fault;
host_vcpu->arch.iflags = hyp_vcpu->vcpu.arch.iflags;
- host_vcpu->arch.fp_state = hyp_vcpu->vcpu.arch.fp_state;
host_cpu_if->vgic_hcr = hyp_cpu_if->vgic_hcr;
for (i = 0; i < hyp_cpu_if->used_lrs; ++i)
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 544a419b9a39..1f82d531a494 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -337,7 +337,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
__sysreg_restore_state_nvhe(host_ctxt);
- if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)
+ if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED)
__fpsimd_save_fpexc32(vcpu);
__debug_switch_to_host(vcpu);
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 14b7a6bc5909..b92f9fe2d50e 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -258,7 +258,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
sysreg_restore_host_state_vhe(host_ctxt);
- if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)
+ if (*host_data_ptr(fp_owner) == FP_STATE_GUEST_OWNED)
__fpsimd_save_fpexc32(vcpu);
__debug_switch_to_host(vcpu);
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 3/5] KVM: arm64: Exclude mdcr_el2_host from kvm_vcpu_arch
From: Marc Zyngier @ 2024-03-22 17:09 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu,
James Clark, Anshuman Khandual, Mark Brown, Dongli Zhang
In-Reply-To: <20240322170945.3292593-1-maz@kernel.org>
As for the rest of the host debug state, the host copy of mdcr_el2
has little to do in the vcpu, and is better placed in the host_data
structure.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/kvm_host.h | 5 ++---
arch/arm64/kvm/hyp/include/hyp/switch.h | 4 ++--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 8c149e4ae99d..590e8767b720 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -555,6 +555,8 @@ struct kvm_host_data {
u64 pmscr_el1;
/* Self-hosted trace */
u64 trfcr_el1;
+ /* Values of trap registers for the host before guest entry. */
+ u64 mdcr_el2;
} host_debug_state;
};
@@ -615,9 +617,6 @@ struct kvm_vcpu_arch {
u64 mdcr_el2;
u64 cptr_el2;
- /* Values of trap registers for the host before guest entry. */
- u64 mdcr_el2_host;
-
/* Exception Information */
struct kvm_vcpu_fault_info fault;
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index ae198b84ca01..7d7de0245ed0 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -232,7 +232,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
vcpu_set_flag(vcpu, PMUSERENR_ON_CPU);
}
- vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2);
+ *host_data_ptr(host_debug_state.mdcr_el2) = read_sysreg(mdcr_el2);
write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
if (cpus_have_final_cap(ARM64_HAS_HCX)) {
@@ -254,7 +254,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
{
- write_sysreg(vcpu->arch.mdcr_el2_host, mdcr_el2);
+ write_sysreg(*host_data_ptr(host_debug_state.mdcr_el2), mdcr_el2);
write_sysreg(0, hstr_el2);
if (kvm_arm_support_pmu_v3()) {
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/5] KVM: arm64: Move host-specific data out of kvm_vcpu_arch
From: Marc Zyngier @ 2024-03-22 17:09 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu,
James Clark, Anshuman Khandual, Mark Brown, Dongli Zhang
This is the second take on this series aiming at reducing the abuse of
the kvm_vcpu_arch structure, and moving things info the per-CPU
context.
* From v1 [1]:
- Fixed the per-CPU accessor outside of the hypervisor code (the
protected case is... interesting)
- Spelling fixes
- Collected RBs
- Rebased on kvmarm-6.9
[1] https://lore.kernel.org/r/20240302111935.129994-1-maz@kernel.org
Marc Zyngier (5):
KVM: arm64: Add accessor for per-CPU state
KVM: arm64: Exclude host_debug_data from vcpu_arch
KVM: arm64: Exclude mdcr_el2_host from kvm_vcpu_arch
KVM: arm64: Exclude host_fpsimd_state pointer from kvm_vcpu_arch
KVM: arm64: Exclude FP ownership from kvm_vcpu_arch
arch/arm64/include/asm/kvm_emulate.h | 4 +-
arch/arm64/include/asm/kvm_host.h | 89 ++++++++++++++++-------
arch/arm64/kvm/arm.c | 8 +-
arch/arm64/kvm/fpsimd.c | 13 ++--
arch/arm64/kvm/hyp/include/hyp/debug-sr.h | 8 +-
arch/arm64/kvm/hyp/include/hyp/switch.h | 20 ++---
arch/arm64/kvm/hyp/nvhe/debug-sr.c | 8 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 -
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 2 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 3 +-
arch/arm64/kvm/hyp/nvhe/switch.c | 6 +-
arch/arm64/kvm/hyp/vhe/switch.c | 6 +-
arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 4 +-
arch/arm64/kvm/pmu.c | 2 +-
14 files changed, 102 insertions(+), 74 deletions(-)
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/5] KVM: arm64: Add accessor for per-CPU state
From: Marc Zyngier @ 2024-03-22 17:09 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu,
James Clark, Anshuman Khandual, Mark Brown, Dongli Zhang
In-Reply-To: <20240322170945.3292593-1-maz@kernel.org>
In order to facilitate the introduction of new per-CPU state,
add a new host_data_ptr() helped that hides some of the per-CPU
verbosity, and make it easier to move that state around in the
future.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/kvm_host.h | 37 +++++++++++++++++++++++
arch/arm64/kvm/arm.c | 2 +-
arch/arm64/kvm/hyp/include/hyp/debug-sr.h | 4 +--
arch/arm64/kvm/hyp/include/hyp/switch.h | 8 ++---
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 2 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 3 +-
arch/arm64/kvm/hyp/nvhe/switch.c | 4 +--
arch/arm64/kvm/hyp/vhe/switch.c | 4 +--
arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 4 +--
arch/arm64/kvm/pmu.c | 2 +-
10 files changed, 53 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 6883963bbc3a..ca6ef663950d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -530,6 +530,17 @@ struct kvm_cpu_context {
u64 *vncr_array;
};
+/*
+ * This structure is instantiated on a per-CPU basis, and contains
+ * data that is:
+ *
+ * - tied to a single physical CPU, and
+ * - either have a lifetime that does not extend past vcpu_put()
+ * - or is an invariant for the lifetime of the system
+ *
+ * Use host_data_ptr(field) as a way to access a pointer to such a
+ * field.
+ */
struct kvm_host_data {
struct kvm_cpu_context host_ctxt;
};
@@ -1167,6 +1178,32 @@ struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
+/*
+ * How we access per-CPU host data depends on the where we access it from,
+ * and the mode we're in:
+ *
+ * - VHE and nVHE hypervisor bits use their locally defined instance
+ *
+ * - the rest of the kernel use either the VHE or nVHE one, depending on
+ * the mode we're running in.
+ *
+ * Unless we're in protected mode, fully deprivileged, and the nVHE
+ * per-CPU stuff is exclusively accessible to the protected EL2 code.
+ * In this case, the EL1 code uses the *VHE* data as its private state
+ * (which makes sense in a way as there shouldn't be any shared state
+ * between the host and the hypervisor).
+ *
+ * Yes, this is all totally trivial. Shoot me now.
+ */
+#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
+#define host_data_ptr(f) (&this_cpu_ptr(&kvm_host_data)->f)
+#else
+#define host_data_ptr(f) \
+ (static_branch_unlikely(&kvm_protected_mode_initialized) ? \
+ &this_cpu_ptr(&kvm_host_data)->f : \
+ &this_cpu_ptr_hyp_sym(kvm_host_data)->f)
+#endif
+
static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
{
/* The host's MPIDR is immutable, so let's set it up at boot time */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3dee5490eea9..a24287c3ba99 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1971,7 +1971,7 @@ static void cpu_set_hyp_vector(void)
static void cpu_hyp_init_context(void)
{
- kvm_init_host_cpu_context(&this_cpu_ptr_hyp_sym(kvm_host_data)->host_ctxt);
+ kvm_init_host_cpu_context(host_data_ptr(host_ctxt));
if (!is_kernel_in_hyp_mode())
cpu_init_hyp_mode();
diff --git a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
index 961bbef104a6..eec0f8ccda56 100644
--- a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
@@ -135,7 +135,7 @@ static inline void __debug_switch_to_guest_common(struct kvm_vcpu *vcpu)
if (!vcpu_get_flag(vcpu, DEBUG_DIRTY))
return;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
guest_ctxt = &vcpu->arch.ctxt;
host_dbg = &vcpu->arch.host_debug_state.regs;
guest_dbg = kern_hyp_va(vcpu->arch.debug_ptr);
@@ -154,7 +154,7 @@ static inline void __debug_switch_to_host_common(struct kvm_vcpu *vcpu)
if (!vcpu_get_flag(vcpu, DEBUG_DIRTY))
return;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
guest_ctxt = &vcpu->arch.ctxt;
host_dbg = &vcpu->arch.host_debug_state.regs;
guest_dbg = kern_hyp_va(vcpu->arch.debug_ptr);
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index e3fcf8c4d5b4..ae198b84ca01 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -155,7 +155,7 @@ static inline bool cpu_has_amu(void)
static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
{
- struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ struct kvm_cpu_context *hctxt = host_data_ptr(host_ctxt);
struct kvm *kvm = kern_hyp_va(vcpu->kvm);
CHECK_FGT_MASKS(HFGRTR_EL2);
@@ -191,7 +191,7 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu)
{
- struct kvm_cpu_context *hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ struct kvm_cpu_context *hctxt = host_data_ptr(host_ctxt);
struct kvm *kvm = kern_hyp_va(vcpu->kvm);
if (!cpus_have_final_cap(ARM64_HAS_FGT))
@@ -226,7 +226,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
write_sysreg(0, pmselr_el0);
- hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ hctxt = host_data_ptr(host_ctxt);
ctxt_sys_reg(hctxt, PMUSERENR_EL0) = read_sysreg(pmuserenr_el0);
write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
vcpu_set_flag(vcpu, PMUSERENR_ON_CPU);
@@ -260,7 +260,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
if (kvm_arm_support_pmu_v3()) {
struct kvm_cpu_context *hctxt;
- hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ hctxt = host_data_ptr(host_ctxt);
write_sysreg(ctxt_sys_reg(hctxt, PMUSERENR_EL0), pmuserenr_el0);
vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU);
}
diff --git a/arch/arm64/kvm/hyp/nvhe/psci-relay.c b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
index d57bcb6ab94d..dfe8fe0f7eaf 100644
--- a/arch/arm64/kvm/hyp/nvhe/psci-relay.c
+++ b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
@@ -205,7 +205,7 @@ asmlinkage void __noreturn __kvm_host_psci_cpu_entry(bool is_cpu_on)
struct psci_boot_args *boot_args;
struct kvm_cpu_context *host_ctxt;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
if (is_cpu_on)
boot_args = this_cpu_ptr(&cpu_on_args);
diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c
index bc58d1b515af..ae00dfa80801 100644
--- a/arch/arm64/kvm/hyp/nvhe/setup.c
+++ b/arch/arm64/kvm/hyp/nvhe/setup.c
@@ -257,8 +257,7 @@ static int fix_hyp_pgtable_refcnt(void)
void __noreturn __pkvm_init_finalise(void)
{
- struct kvm_host_data *host_data = this_cpu_ptr(&kvm_host_data);
- struct kvm_cpu_context *host_ctxt = &host_data->host_ctxt;
+ struct kvm_cpu_context *host_ctxt = host_data_ptr(host_ctxt);
unsigned long nr_pages, reserved_pages, pfn;
int ret;
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index c50f8459e4fc..544a419b9a39 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -264,7 +264,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
pmr_sync();
}
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
host_ctxt->__hyp_running_vcpu = vcpu;
guest_ctxt = &vcpu->arch.ctxt;
@@ -367,7 +367,7 @@ asmlinkage void __noreturn hyp_panic(void)
struct kvm_cpu_context *host_ctxt;
struct kvm_vcpu *vcpu;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
vcpu = host_ctxt->__hyp_running_vcpu;
if (vcpu) {
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 1581df6aec87..14b7a6bc5909 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -221,7 +221,7 @@ static int __kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
struct kvm_cpu_context *guest_ctxt;
u64 exit_code;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
host_ctxt->__hyp_running_vcpu = vcpu;
guest_ctxt = &vcpu->arch.ctxt;
@@ -306,7 +306,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64 par)
struct kvm_cpu_context *host_ctxt;
struct kvm_vcpu *vcpu;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
vcpu = host_ctxt->__hyp_running_vcpu;
__deactivate_traps(vcpu);
diff --git a/arch/arm64/kvm/hyp/vhe/sysreg-sr.c b/arch/arm64/kvm/hyp/vhe/sysreg-sr.c
index a8b9ea496706..e12bd7d6d2dc 100644
--- a/arch/arm64/kvm/hyp/vhe/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/vhe/sysreg-sr.c
@@ -67,7 +67,7 @@ void __vcpu_load_switch_sysregs(struct kvm_vcpu *vcpu)
struct kvm_cpu_context *guest_ctxt = &vcpu->arch.ctxt;
struct kvm_cpu_context *host_ctxt;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
__sysreg_save_user_state(host_ctxt);
/*
@@ -110,7 +110,7 @@ void __vcpu_put_switch_sysregs(struct kvm_vcpu *vcpu)
struct kvm_cpu_context *guest_ctxt = &vcpu->arch.ctxt;
struct kvm_cpu_context *host_ctxt;
- host_ctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ host_ctxt = host_data_ptr(host_ctxt);
__sysreg_save_el1_state(guest_ctxt);
__sysreg_save_user_state(guest_ctxt);
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index a243934c5568..329819806096 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -232,7 +232,7 @@ bool kvm_set_pmuserenr(u64 val)
if (!vcpu || !vcpu_get_flag(vcpu, PMUSERENR_ON_CPU))
return false;
- hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+ hctxt = host_data_ptr(host_ctxt);
ctxt_sys_reg(hctxt, PMUSERENR_EL0) = val;
return true;
}
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 4/5] KVM: arm64: Exclude host_fpsimd_state pointer from kvm_vcpu_arch
From: Marc Zyngier @ 2024-03-22 17:09 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu,
James Clark, Anshuman Khandual, Mark Brown, Dongli Zhang
In-Reply-To: <20240322170945.3292593-1-maz@kernel.org>
As the name of the field indicates, host_fpsimd_state is strictly
a host piece of data, and we reset this pointer on each PID change.
So let's move it where it belongs, and set it at load-time. Although
this is slightly more often, it is a well defined life-cycle which
matches other pieces of data.
Reviewed-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/kvm_host.h | 2 +-
arch/arm64/kvm/fpsimd.c | 3 +--
arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 1 -
4 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 590e8767b720..838cdee2ecf7 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -543,6 +543,7 @@ struct kvm_cpu_context {
*/
struct kvm_host_data {
struct kvm_cpu_context host_ctxt;
+ struct user_fpsimd_state *fpsimd_state; /* hyp VA */
/*
* host_debug_state contains the host registers which are
@@ -661,7 +662,6 @@ struct kvm_vcpu_arch {
struct kvm_guest_debug_arch vcpu_debug_state;
struct kvm_guest_debug_arch external_debug_state;
- struct user_fpsimd_state *host_fpsimd_state; /* hyp VA */
struct task_struct *parent_task;
/* VGIC state */
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index 571cf6eef1e1..e6bd99358615 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -49,8 +49,6 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
if (ret)
return ret;
- vcpu->arch.host_fpsimd_state = kern_hyp_va(fpsimd);
-
/*
* We need to keep current's task_struct pinned until its data has been
* unshared with the hypervisor to make sure it is not re-used by the
@@ -87,6 +85,7 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
* FP_STATE_FREE if the flag set.
*/
vcpu->arch.fp_state = FP_STATE_HOST_OWNED;
+ *host_data_ptr(fpsimd_state) = kern_hyp_va(¤t->thread.uw.fpsimd_state);
vcpu_clear_flag(vcpu, HOST_SVE_ENABLED);
if (read_sysreg(cpacr_el1) & CPACR_EL1_ZEN_EL0EN)
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 7d7de0245ed0..6def6ad8dd48 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -377,7 +377,7 @@ static bool kvm_hyp_handle_fpsimd(struct kvm_vcpu *vcpu, u64 *exit_code)
/* Write out the host state if it's in the registers */
if (vcpu->arch.fp_state == FP_STATE_HOST_OWNED)
- __fpsimd_save_state(vcpu->arch.host_fpsimd_state);
+ __fpsimd_save_state(*host_data_ptr(fpsimd_state));
/* Restore the guest state */
if (sve_guest)
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 2385fd03ed87..c5f625dc1f07 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -42,7 +42,6 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
hyp_vcpu->vcpu.arch.fp_state = host_vcpu->arch.fp_state;
hyp_vcpu->vcpu.arch.debug_ptr = kern_hyp_va(host_vcpu->arch.debug_ptr);
- hyp_vcpu->vcpu.arch.host_fpsimd_state = host_vcpu->arch.host_fpsimd_state;
hyp_vcpu->vcpu.arch.vsesr_el2 = host_vcpu->arch.vsesr_el2;
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/5] KVM: arm64: Exclude host_debug_data from vcpu_arch
From: Marc Zyngier @ 2024-03-22 17:09 UTC (permalink / raw)
To: kvmarm, linux-arm-kernel, kvm
Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu,
James Clark, Anshuman Khandual, Mark Brown, Dongli Zhang
In-Reply-To: <20240322170945.3292593-1-maz@kernel.org>
Keeping host_debug_state on a per-vcpu basis is completely
pointless. The lifetime of this data is only that of the inner
run-loop, which means it is never accessed outside of the core
EL2 code.
Move the structure into kvm_host_data, and save over 500 bytes
per vcpu.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/kvm_host.h | 31 +++++++++++++----------
arch/arm64/kvm/hyp/include/hyp/debug-sr.h | 4 +--
arch/arm64/kvm/hyp/nvhe/debug-sr.c | 8 +++---
3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ca6ef663950d..8c149e4ae99d 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -543,6 +543,19 @@ struct kvm_cpu_context {
*/
struct kvm_host_data {
struct kvm_cpu_context host_ctxt;
+
+ /*
+ * host_debug_state contains the host registers which are
+ * saved and restored during world switches.
+ */
+ struct {
+ /* {Break,watch}point registers */
+ struct kvm_guest_debug_arch regs;
+ /* Statistical profiling extension */
+ u64 pmscr_el1;
+ /* Self-hosted trace */
+ u64 trfcr_el1;
+ } host_debug_state;
};
struct kvm_host_psci_config {
@@ -637,11 +650,10 @@ struct kvm_vcpu_arch {
* We maintain more than a single set of debug registers to support
* debugging the guest from the host and to maintain separate host and
* guest state during world switches. vcpu_debug_state are the debug
- * registers of the vcpu as the guest sees them. host_debug_state are
- * the host registers which are saved and restored during
- * world switches. external_debug_state contains the debug
- * values we want to debug the guest. This is set via the
- * KVM_SET_GUEST_DEBUG ioctl.
+ * registers of the vcpu as the guest sees them.
+ *
+ * external_debug_state contains the debug values we want to debug the
+ * guest. This is set via the KVM_SET_GUEST_DEBUG ioctl.
*
* debug_ptr points to the set of debug registers that should be loaded
* onto the hardware when running the guest.
@@ -653,15 +665,6 @@ struct kvm_vcpu_arch {
struct user_fpsimd_state *host_fpsimd_state; /* hyp VA */
struct task_struct *parent_task;
- struct {
- /* {Break,watch}point registers */
- struct kvm_guest_debug_arch regs;
- /* Statistical profiling extension */
- u64 pmscr_el1;
- /* Self-hosted trace */
- u64 trfcr_el1;
- } host_debug_state;
-
/* VGIC state */
struct vgic_cpu vgic_cpu;
struct arch_timer_cpu timer_cpu;
diff --git a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
index eec0f8ccda56..d00093699aaf 100644
--- a/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/debug-sr.h
@@ -137,7 +137,7 @@ static inline void __debug_switch_to_guest_common(struct kvm_vcpu *vcpu)
host_ctxt = host_data_ptr(host_ctxt);
guest_ctxt = &vcpu->arch.ctxt;
- host_dbg = &vcpu->arch.host_debug_state.regs;
+ host_dbg = host_data_ptr(host_debug_state.regs);
guest_dbg = kern_hyp_va(vcpu->arch.debug_ptr);
__debug_save_state(host_dbg, host_ctxt);
@@ -156,7 +156,7 @@ static inline void __debug_switch_to_host_common(struct kvm_vcpu *vcpu)
host_ctxt = host_data_ptr(host_ctxt);
guest_ctxt = &vcpu->arch.ctxt;
- host_dbg = &vcpu->arch.host_debug_state.regs;
+ host_dbg = host_data_ptr(host_debug_state.regs);
guest_dbg = kern_hyp_va(vcpu->arch.debug_ptr);
__debug_save_state(guest_dbg, guest_ctxt);
diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
index 7746ea507b6f..53efda0235cf 100644
--- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
+++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
@@ -83,10 +83,10 @@ void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
{
/* Disable and flush SPE data generation */
if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_SPE))
- __debug_save_spe(&vcpu->arch.host_debug_state.pmscr_el1);
+ __debug_save_spe(host_data_ptr(host_debug_state.pmscr_el1));
/* Disable and flush Self-Hosted Trace generation */
if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
- __debug_save_trace(&vcpu->arch.host_debug_state.trfcr_el1);
+ __debug_save_trace(host_data_ptr(host_debug_state.trfcr_el1));
}
void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
@@ -97,9 +97,9 @@ void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
{
if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_SPE))
- __debug_restore_spe(vcpu->arch.host_debug_state.pmscr_el1);
+ __debug_restore_spe(*host_data_ptr(host_debug_state.pmscr_el1));
if (vcpu_get_flag(vcpu, DEBUG_STATE_SAVE_TRBE))
- __debug_restore_trace(vcpu->arch.host_debug_state.trfcr_el1);
+ __debug_restore_trace(*host_data_ptr(host_debug_state.trfcr_el1));
}
void __debug_switch_to_host(struct kvm_vcpu *vcpu)
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
From: Sudeep Holla @ 2024-03-22 17:08 UTC (permalink / raw)
To: David Woodhouse
Cc: Marc Zyngier, linux-arm-kernel, kvm, Paolo Bonzini, Sudeep Holla,
Jonathan Corbet, Oliver Upton, James Morse, Suzuki K Poulose,
Zenghui Yu, Catalin Marinas, Will Deacon, Mark Rutland,
Lorenzo Pieralisi, Rafael J. Wysocki, Len Brown, Pavel Machek,
Mostafa Saleh, Jean-Philippe Brucker, linux-doc, linux-kernel,
kvmarm, linux-pm
In-Reply-To: <12bc0c787fc20e1a3f5dc2588a2712d996ac6d38.camel@infradead.org>
On Fri, Mar 22, 2024 at 04:55:04PM +0000, David Woodhouse wrote:
> On Fri, 2024-03-22 at 16:37 +0000, Marc Zyngier wrote:
> >
> > I agree that nothing really breaks, but I also hold the view that
> > broken firmware implementations should be given the finger, specially
> > given that you have done this work *ahead* of the spec. I would really
> > like this to fail immediately on these and not even try to suspend.
> >
> > With that in mind, if doesn't really matter whether HIBERNATE_OFF is
> > mandatory or not. We really should check for it and pretend it doesn't
> > exist if the correct flag isn't set.
>
> Ack.
>
> I'll rename that variable to 'psci_system_off2_hibernate_supported' then.
>
> static void __init psci_init_system_off2(void)
> {
> int ret;
>
> ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> if (ret < 0)
> return;
>
> if (ret & (1 << PSCI_1_3_HIBERNATE_TYPE_OFF))
> psci_system_off2_hibernate_supported = true;
>
Ah OK, you have already agreed to do this, please ignore my response then.
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
From: Sudeep Holla @ 2024-03-22 17:05 UTC (permalink / raw)
To: Marc Zyngier
Cc: David Woodhouse, linux-arm-kernel, kvm, Sudeep Holla,
Paolo Bonzini, Jonathan Corbet, Oliver Upton, James Morse,
Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
Mark Rutland, Lorenzo Pieralisi, Rafael J. Wysocki, Len Brown,
Pavel Machek, Mostafa Saleh, Jean-Philippe Brucker, linux-doc,
linux-kernel, kvmarm, linux-pm
In-Reply-To: <86edc2z0hs.wl-maz@kernel.org>
On Fri, Mar 22, 2024 at 04:37:19PM +0000, Marc Zyngier wrote:
> On Fri, 22 Mar 2024 16:12:44 +0000,
> David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > On Fri, 2024-03-22 at 16:02 +0000, Marc Zyngier wrote:
> > > On Tue, 19 Mar 2024 12:59:06 +0000,
> > > David Woodhouse <dwmw2@infradead.org> wrote:
> > >
> > > [...]
> > >
> > > > +static void __init psci_init_system_off2(void)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> > > > +
> > > > + if (ret != PSCI_RET_NOT_SUPPORTED)
> > > > + psci_system_off2_supported = true;
> > >
> > > It'd be worth considering the (slightly broken) case where SYSTEM_OFF2
> > > is supported, but HIBERNATE_OFF is not set in the response, as the
> > > spec doesn't say that this bit is mandatory (it seems legal to
> > > implement SYSTEM_OFF2 without any hibernate type, making it similar to
> > > SYSTEM_OFF).
> >
> > Such is not my understanding. If SYSTEM_OFF2 is supported, then
> > HIBERNATE_OFF *is* mandatory.
> >
> > The next update to the spec is turning the PSCI_FEATURES response into
> > a *bitmap* of the available features, and I believe it will mandate
> > that bit zero is set.
Correct, but we add a extra check as well to be sure even if it is mandated
unless the spec relaxes in a way that psci_features(SYSTEM_OFF2) need not
return the mandatory types in the bitmask which I doubt.
Something like:
if (ret != PSCI_RET_NOT_SUPPORTED &&
(ret & BIT(PSCI_1_3_HIBERNATE_TYPE_OFF)))
psci_system_off2_supported = true;
This will ensure the firmware will not randomly set bit[0]=0 if in the
future it support some newer types as well.
I understand the kernel is not conformance test for the spec but in
practice especially for such features and PSCI spec in particular, kernel
has become defacto conformance for firmware developers which is sad.
It some feature works in the kernel, the firmware is assumed to be
conformant to the spec w.r.t the feature.
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler
From: Johan Hovold @ 2024-03-22 17:00 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Taniya Das,
Johan Hovold, linux-arm-msm, linux-clk, devicetree, linux-kernel,
linux-pm, linux-arm-kernel, Ulf Hansson, Rob Clark, Abhinav Kumar,
Dmitry Baryshkov
In-Reply-To: <20240125-sa8295p-gpu-v4-2-7011c2a63037@quicinc.com>
On Thu, Jan 25, 2024 at 01:05:08PM -0800, Bjorn Andersson wrote:
> The GX GDSC is modelled to aid the GMU in powering down the GPU in the
> event that the GPU crashes, so that it can be restarted again. But in
> the event that the power-domain is supplied through a dedicated
> regulator (in contrast to being a subdomin of another power-domain),
> something needs to turn that regulator on, both to make sure things are
> powered and to match the operation in gdsc_disable().
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
> drivers/clk/qcom/gdsc.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 5358e28122ab..e7a4068b9f39 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -557,7 +557,15 @@ void gdsc_unregister(struct gdsc_desc *desc)
> */
> int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
> {
> - /* Do nothing but give genpd the impression that we were successful */
> - return 0;
> + struct gdsc *sc = domain_to_gdsc(domain);
> + int ret = 0;
> +
> + /* Enable the parent supply, when controlled through the regulator framework. */
> + if (sc->rsupply)
> + ret = regulator_enable(sc->rsupply);
> +
> + /* Do nothing with the GDSC itself */
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
This patch (and series) is now in mainline as commit 9187ebb954ab ("clk:
qcom: gdsc: Enable supply reglator in GPU GX handler") and appears to be
involved in triggering the below lockdep splat on boot of the Lenovo
ThinkPad X13s.
Adding Ulf and the MSM DRM devs as well in case I blamed the wrong
change here.
Johan
[ 5.849106] ======================================================
[ 5.849111] WARNING: possible circular locking dependency detected
[ 5.849115] 6.8.0 #111 Not tainted
[ 5.849119] ------------------------------------------------------
[ 5.849123] kworker/u32:2/66 is trying to acquire lock:
[ 5.849128] ffffaffb0ca9b4f0 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent+0x54/0x27c
[ 5.849148]
but task is already holding lock:
[ 5.849152] ffffaffad8f33a00 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x18/0x24
[ 5.849165]
which lock already depends on the new lock.
[ 5.849170]
the existing dependency chain (in reverse order) is:
[ 5.849175]
-> #3 (&genpd->mlock){+.+.}-{3:3}:
[ 5.849182] lock_acquire+0x68/0x84
[ 5.849190] __mutex_lock+0xa0/0x840
[ 5.849197] mutex_lock_nested+0x24/0x30
[ 5.849201] genpd_lock_mtx+0x18/0x24
[ 5.849206] genpd_runtime_resume+0x104/0x2f4
[ 5.849211] __rpm_callback+0x48/0x1a8
[ 5.849218] rpm_callback+0x68/0x74
[ 5.849223] rpm_resume+0x444/0x638
[ 5.849228] __pm_runtime_resume+0x5c/0xbc
[ 5.849233] device_link_add+0x680/0x6e8
[ 5.849239] arm_smmu_probe_device+0x2a4/0x3e4
[ 5.849245] __iommu_probe_device+0x108/0x430
[ 5.849250] iommu_probe_device+0x3c/0x80
[ 5.849255] of_iommu_configure+0x170/0x25c
[ 5.849260] of_dma_configure_id+0x10c/0x340
[ 5.849265] platform_dma_configure+0x78/0xbc
[ 5.849270] really_probe+0x74/0x388
[ 5.849276] __driver_probe_device+0x7c/0x160
[ 5.849281] driver_probe_device+0x40/0x114
[ 5.849287] __driver_attach+0xfc/0x208
[ 5.849292] bus_for_each_dev+0x74/0xd0
[ 5.849296] driver_attach+0x24/0x30
[ 5.849301] bus_add_driver+0x110/0x214
[ 5.849306] driver_register+0x60/0x128
[ 5.849312] __platform_driver_register+0x28/0x34
[ 5.849316] panel_edp_init+0x20/0x1000 [panel_edp]
[ 5.849329] panel_edp_init+0xc8/0x1000 [panel_edp]
[ 5.849337] do_one_initcall+0x74/0x344
[ 5.849342] do_init_module+0x5c/0x1f8
[ 5.849347] load_module+0x1c9c/0x1e18
[ 5.849351] init_module_from_file+0x84/0xc0
[ 5.849356] idempotent_init_module+0x180/0x250
[ 5.849360] __arm64_sys_finit_module+0x68/0xcc
[ 5.849365] invoke_syscall+0x48/0x114
[ 5.849370] el0_svc_common.constprop.0+0xc0/0xe0
[ 5.849376] do_el0_svc+0x1c/0x28
[ 5.849381] el0_svc+0x48/0x114
[ 5.849386] el0t_64_sync_handler+0xc0/0xc4
[ 5.849391] el0t_64_sync+0x190/0x194
[ 5.849395]
-> #2 (dpm_list_mtx){+.+.}-{3:3}:
[ 5.849403] lock_acquire+0x68/0x84
[ 5.849408] __mutex_lock+0xa0/0x840
[ 5.849412] mutex_lock_nested+0x24/0x30
[ 5.849416] device_pm_lock+0x1c/0x28
[ 5.849422] device_link_add+0x274/0x6e8
[ 5.849426] fw_devlink_create_devlink+0x118/0x2fc
[ 5.849431] __fw_devlink_link_to_consumers.isra.0+0x50/0x104
[ 5.849437] device_add+0x744/0x7b0
[ 5.849441] of_device_add+0x44/0x60
[ 5.849446] of_platform_device_create_pdata+0x98/0x140
[ 5.849451] of_platform_bus_create+0x1b4/0x4b4
[ 5.849455] of_platform_populate+0x58/0x150
[ 5.849459] of_platform_default_populate_init+0xd8/0xf0
[ 5.849466] do_one_initcall+0x74/0x344
[ 5.849470] kernel_init_freeable+0x244/0x350
[ 5.849476] kernel_init+0x20/0x1d8
[ 5.849480] ret_from_fork+0x10/0x20
[ 5.849484]
-> #1 (device_links_lock){+.+.}-{3:3}:
[ 5.849492] lock_acquire+0x68/0x84
[ 5.849497] __mutex_lock+0xa0/0x840
[ 5.849501] mutex_lock_nested+0x24/0x30
[ 5.849505] device_link_remove+0x38/0x94
[ 5.849509] _regulator_put.part.0+0x168/0x190
[ 5.849515] regulator_bulk_free+0x64/0x90
[ 5.849521] devm_regulator_bulk_release+0x1c/0x28
[ 5.849526] release_nodes+0x5c/0x90
[ 5.849532] devres_release_group+0xc8/0x134
[ 5.849537] i2c_device_probe+0x138/0x2e8 [i2c_core]
[ 5.849549] really_probe+0xc0/0x388
[ 5.849554] __driver_probe_device+0x7c/0x160
[ 5.849559] driver_probe_device+0x40/0x114
[ 5.849565] __device_attach_driver+0xbc/0x158
[ 5.849571] bus_for_each_drv+0x84/0xe0
[ 5.849576] __device_attach_async_helper+0xb0/0x10c
[ 5.849582] async_run_entry_fn+0x34/0x14c
[ 5.849588] process_one_work+0x220/0x634
[ 5.849595] worker_thread+0x268/0x3a8
[ 5.849599] kthread+0x124/0x128
[ 5.849603] ret_from_fork+0x10/0x20
[ 5.849608]
-> #0 (regulator_list_mutex){+.+.}-{3:3}:
[ 5.849615] __lock_acquire+0x130c/0x2064
[ 5.849621] lock_acquire.part.0+0xc8/0x20c
[ 5.849626] lock_acquire+0x68/0x84
[ 5.849631] __mutex_lock+0xa0/0x840
[ 5.849636] mutex_lock_nested+0x24/0x30
[ 5.849640] regulator_lock_dependent+0x54/0x27c
[ 5.849646] regulator_enable+0x34/0xd0
[ 5.849651] gdsc_gx_do_nothing_enable+0x18/0x2c
[ 5.849659] _genpd_power_on+0x94/0x17c
[ 5.849664] genpd_power_on.part.0+0xa4/0x1a8
[ 5.849669] genpd_runtime_resume+0x120/0x2f4
[ 5.849673] __rpm_callback+0x48/0x1a8
[ 5.849678] rpm_callback+0x68/0x74
[ 5.849683] rpm_resume+0x584/0x638
[ 5.849688] __pm_runtime_resume+0x5c/0xbc
[ 5.849693] a6xx_gmu_resume+0x70/0xcf8 [msm]
[ 5.849722] a6xx_gmu_pm_resume+0x3c/0x284 [msm]
[ 5.849745] adreno_runtime_resume+0x28/0x34 [msm]
[ 5.849769] pm_generic_runtime_resume+0x2c/0x44
[ 5.849774] __rpm_callback+0x48/0x1a8
[ 5.849779] rpm_callback+0x68/0x74
[ 5.849783] rpm_resume+0x444/0x638
[ 5.849788] __pm_runtime_resume+0x5c/0xbc
[ 5.849793] adreno_load_gpu+0x78/0x238 [msm]
[ 5.849816] msm_open+0x114/0x128 [msm]
[ 5.849843] drm_file_alloc+0x184/0x2a8 [drm]
[ 5.849876] drm_client_init+0x7c/0x10c [drm]
[ 5.849900] msm_fbdev_setup+0x84/0x150 [msm]
[ 5.849929] msm_drm_bind+0x264/0x420 [msm]
[ 5.849950] try_to_bring_up_aggregate_device+0x1ec/0x2f4
[ 5.849962] __component_add+0xa8/0x194
[ 5.849965] component_add+0x14/0x20
[ 5.849967] dp_display_probe_tail+0x4c/0xac [msm]
[ 5.850000] dp_auxbus_done_probe+0x14/0x20 [msm]
[ 5.850023] dp_aux_ep_probe+0x4c/0xf0 [drm_dp_aux_bus]
[ 5.850030] really_probe+0xc0/0x388
[ 5.850034] __driver_probe_device+0x7c/0x160
[ 5.850038] driver_probe_device+0x40/0x114
[ 5.850042] __device_attach_driver+0xbc/0x158
[ 5.850046] bus_for_each_drv+0x84/0xe0
[ 5.850050] __device_attach+0xa8/0x1d4
[ 5.850053] device_initial_probe+0x14/0x20
[ 5.850058] bus_probe_device+0xb0/0xb4
[ 5.850061] device_add+0x5b8/0x7b0
[ 5.850065] device_register+0x20/0x30
[ 5.850068] of_dp_aux_populate_bus+0xcc/0x1a0 [drm_dp_aux_bus]
[ 5.850074] devm_of_dp_aux_populate_bus+0x18/0x80 [drm_dp_aux_bus]
[ 5.850080] dp_display_probe+0x2a4/0x454 [msm]
[ 5.850098] platform_probe+0x68/0xd8
[ 5.850102] really_probe+0xc0/0x388
[ 5.850106] __driver_probe_device+0x7c/0x160
[ 5.850109] driver_probe_device+0x40/0x114
[ 5.850113] __device_attach_driver+0xbc/0x158
[ 5.850117] bus_for_each_drv+0x84/0xe0
[ 5.850121] __device_attach+0xa8/0x1d4
[ 5.850125] device_initial_probe+0x14/0x20
[ 5.850129] bus_probe_device+0xb0/0xb4
[ 5.850133] deferred_probe_work_func+0xa0/0xf4
[ 5.850137] process_one_work+0x220/0x634
[ 5.850141] worker_thread+0x268/0x3a8
[ 5.850144] kthread+0x124/0x128
[ 5.850147] ret_from_fork+0x10/0x20
[ 5.850151]
other info that might help us debug this:
[ 5.850154] Chain exists of:
regulator_list_mutex --> dpm_list_mtx --> &genpd->mlock
[ 5.850163] Possible unsafe locking scenario:
[ 5.850166] CPU0 CPU1
[ 5.850168] ---- ----
[ 5.850171] lock(&genpd->mlock);
[ 5.850174] lock(dpm_list_mtx);
[ 5.850178] lock(&genpd->mlock);
[ 5.850182] lock(regulator_list_mutex);
[ 5.850185]
*** DEADLOCK ***
[ 5.850188] 8 locks held by kworker/u32:2/66:
[ 5.850191] #0: ffff2be180020948 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x1a0/0x634
[ 5.850199] #1: ffff800081033de0 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x1c8/0x634
[ 5.850207] #2: ffff2be1849e38f8 (&dev->mutex){....}-{3:3}, at: __device_attach+0x38/0x1d4
[ 5.850215] #3: ffff2be183c550e8 (&dev->mutex){....}-{3:3}, at: __device_attach+0x38/0x1d4
[ 5.850223] #4: ffffaffb0caa59e8 (component_mutex){+.+.}-{3:3}, at: __component_add+0x60/0x194
[ 5.850231] #5: ffffaffad96ee178 (init_lock){+.+.}-{3:3}, at: msm_open+0x38/0x128 [msm]
[ 5.850252] #6: ffff2be18d6536d8 (&a6xx_gpu->gmu.lock){+.+.}-{3:3}, at: a6xx_gmu_pm_resume+0x34/0x284 [msm]
[ 5.850274] #7: ffffaffad8f33a00 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x18/0x24
[ 5.850282]
stack backtrace:
[ 5.850286] CPU: 6 PID: 66 Comm: kworker/u32:2 Not tainted 6.8.0 #111
[ 5.850291] Hardware name: LENOVO 21BYZ9SRUS/21BYZ9SRUS, BIOS N3HET87W (1.59 ) 12/05/2023
[ 5.850296] Workqueue: events_unbound deferred_probe_work_func
[ 5.850301] Call trace:
[ 5.850303] dump_backtrace+0x9c/0x11c
[ 5.850307] show_stack+0x18/0x24
[ 5.850312] dump_stack_lvl+0x90/0xd0
[ 5.850316] dump_stack+0x18/0x24
[ 5.850320] print_circular_bug+0x290/0x370
[ 5.850324] check_noncircular+0x15c/0x170
[ 5.850327] __lock_acquire+0x130c/0x2064
[ 5.850331] lock_acquire.part.0+0xc8/0x20c
[ 5.850335] lock_acquire+0x68/0x84
[ 5.850339] __mutex_lock+0xa0/0x840
[ 5.850342] mutex_lock_nested+0x24/0x30
[ 5.850345] regulator_lock_dependent+0x54/0x27c
[ 5.850349] regulator_enable+0x34/0xd0
[ 5.850352] gdsc_gx_do_nothing_enable+0x18/0x2c
[ 5.850356] _genpd_power_on+0x94/0x17c
[ 5.850360] genpd_power_on.part.0+0xa4/0x1a8
[ 5.850363] genpd_runtime_resume+0x120/0x2f4
[ 5.850366] __rpm_callback+0x48/0x1a8
[ 5.850370] rpm_callback+0x68/0x74
[ 5.850374] rpm_resume+0x584/0x638
[ 5.850377] __pm_runtime_resume+0x5c/0xbc
[ 5.850381] a6xx_gmu_resume+0x70/0xcf8 [msm]
[ 5.850398] a6xx_gmu_pm_resume+0x3c/0x284 [msm]
[ 5.850416] adreno_runtime_resume+0x28/0x34 [msm]
[ 5.850433] pm_generic_runtime_resume+0x2c/0x44
[ 5.850437] __rpm_callback+0x48/0x1a8
[ 5.850440] rpm_callback+0x68/0x74
[ 5.850443] rpm_resume+0x444/0x638
[ 5.850447] __pm_runtime_resume+0x5c/0xbc
[ 5.850450] adreno_load_gpu+0x78/0x238 [msm]
[ 5.850467] msm_open+0x114/0x128 [msm]
[ 5.850485] drm_file_alloc+0x184/0x2a8 [drm]
[ 5.850504] drm_client_init+0x7c/0x10c [drm]
[ 5.850522] msm_fbdev_setup+0x84/0x150 [msm]
[ 5.850540] msm_drm_bind+0x264/0x420 [msm]
[ 5.850557] try_to_bring_up_aggregate_device+0x1ec/0x2f4
[ 5.850562] __component_add+0xa8/0x194
[ 5.850566] component_add+0x14/0x20
[ 5.850570] dp_display_probe_tail+0x4c/0xac [msm]
[ 5.850587] dp_auxbus_done_probe+0x14/0x20 [msm]
[ 5.850605] dp_aux_ep_probe+0x4c/0xf0 [drm_dp_aux_bus]
[ 5.850611] really_probe+0xc0/0x388
[ 5.850615] __driver_probe_device+0x7c/0x160
[ 5.850619] driver_probe_device+0x40/0x114
[ 5.850624] __device_attach_driver+0xbc/0x158
[ 5.850628] bus_for_each_drv+0x84/0xe0
[ 5.850631] __device_attach+0xa8/0x1d4
[ 5.850635] device_initial_probe+0x14/0x20
[ 5.850639] bus_probe_device+0xb0/0xb4
[ 5.850642] device_add+0x5b8/0x7b0
[ 5.850645] device_register+0x20/0x30
[ 5.850648] of_dp_aux_populate_bus+0xcc/0x1a0 [drm_dp_aux_bus]
[ 5.850654] devm_of_dp_aux_populate_bus+0x18/0x80 [drm_dp_aux_bus]
[ 5.850660] dp_display_probe+0x2a4/0x454 [msm]
[ 5.850678] platform_probe+0x68/0xd8
[ 5.850681] really_probe+0xc0/0x388
[ 5.850684] __driver_probe_device+0x7c/0x160
[ 5.850688] driver_probe_device+0x40/0x114
[ 5.850692] __device_attach_driver+0xbc/0x158
[ 5.850696] bus_for_each_drv+0x84/0xe0
[ 5.850699] __device_attach+0xa8/0x1d4
[ 5.850703] device_initial_probe+0x14/0x20
[ 5.850707] bus_probe_device+0xb0/0xb4
[ 5.850710] deferred_probe_work_func+0xa0/0xf4
[ 5.850714] process_one_work+0x220/0x634
[ 5.850717] worker_thread+0x268/0x3a8
[ 5.850720] kthread+0x124/0x128
[ 5.850723] ret_from_fork+0x10/0x20
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 64/64] i2c: reword i2c_algorithm in drivers according to newest specification
From: Andy Shevchenko @ 2024-03-22 17:00 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c, Andi Shyti, Krzysztof Kozlowski,
linux-arm-kernel, chrome-platform, linux-samsung-soc, imx,
linux-mips, linux-amlogic, linux-mediatek, openbmc, linux-omap,
linuxppc-dev, asahi, linux-arm-msm, linux-renesas-soc,
linux-stm32, linux-tegra, virtualization
In-Reply-To: <Zf22dmwBpN7Ctk3v@shikoro>
On Fri, Mar 22, 2024 at 05:48:54PM +0100, Wolfram Sang wrote:
>
> > > static const struct i2c_algorithm at91_twi_algorithm = {
> > > - .master_xfer = at91_twi_xfer,
> > > + .xfer = at91_twi_xfer,
> >
> > Seems you made this by a script, can you check the indentations afterwards?
>
> Yes, I noticed as well. But other (not converted) drivers have issues
> there as well, so this will be a seperate patch.
The problem is that you add to a technical debt. We don't want that.
If you have not introduced a new indentation issue, it obviously is
not needed to be fixed in a separate patch. So, please consider this.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
From: David Woodhouse @ 2024-03-22 16:55 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-arm-kernel, kvm, Paolo Bonzini, Jonathan Corbet,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Len Brown, Pavel Machek, Mostafa Saleh,
Jean-Philippe Brucker, linux-doc, linux-kernel, kvmarm, linux-pm
In-Reply-To: <86edc2z0hs.wl-maz@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 874 bytes --]
On Fri, 2024-03-22 at 16:37 +0000, Marc Zyngier wrote:
>
> I agree that nothing really breaks, but I also hold the view that
> broken firmware implementations should be given the finger, specially
> given that you have done this work *ahead* of the spec. I would really
> like this to fail immediately on these and not even try to suspend.
>
> With that in mind, if doesn't really matter whether HIBERNATE_OFF is
> mandatory or not. We really should check for it and pretend it doesn't
> exist if the correct flag isn't set.
Ack.
I'll rename that variable to 'psci_system_off2_hibernate_supported' then.
static void __init psci_init_system_off2(void)
{
int ret;
ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
if (ret < 0)
return;
if (ret & (1 << PSCI_1_3_HIBERNATE_TYPE_OFF))
psci_system_off2_hibernate_supported = true;
}
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 64/64] i2c: reword i2c_algorithm in drivers according to newest specification
From: Wolfram Sang @ 2024-03-22 16:48 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-i2c, Andi Shyti, Krzysztof Kozlowski, linux-arm-kernel,
chrome-platform, linux-samsung-soc, imx, linux-mips,
linux-amlogic, linux-mediatek, openbmc, linux-omap, linuxppc-dev,
asahi, linux-arm-msm, linux-renesas-soc, linux-stm32, linux-tegra,
virtualization
In-Reply-To: <Zf2tSLJzujUHbJjp@smile.fi.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 351 bytes --]
> > static const struct i2c_algorithm at91_twi_algorithm = {
> > - .master_xfer = at91_twi_xfer,
> > + .xfer = at91_twi_xfer,
>
> Seems you made this by a script, can you check the indentations afterwards?
Yes, I noticed as well. But other (not converted) drivers have issues
there as well, so this will be a seperate patch.
Thanks!
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/1] arm64: dts: imx8-ss-conn: fix usdhc wrong lpcg clock order
From: Frank Li @ 2024-03-22 16:47 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Dong Aisheng,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
The actual clock show wrong frequency:
echo on >/sys/devices/platform/bus\@5b000000/5b010000.mmc/power/control
cat /sys/kernel/debug/mmc0/ios
clock: 200000000 Hz
actual clock: 166000000 Hz
^^^^^^^^^
.....
According to
sdhc0_lpcg: clock-controller@5b200000 {
compatible = "fsl,imx8qxp-lpcg";
reg = <0x5b200000 0x10000>;
#clock-cells = <1>;
clocks = <&clk IMX_SC_R_SDHC_0 IMX_SC_PM_CLK_PER>,
<&conn_ipg_clk>, <&conn_axi_clk>;
clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>,
<IMX_LPCG_CLK_5>;
clock-output-names = "sdhc0_lpcg_per_clk",
"sdhc0_lpcg_ipg_clk",
"sdhc0_lpcg_ahb_clk";
power-domains = <&pd IMX_SC_R_SDHC_0>;
}
"per_clk" should be IMX_LPCG_CLK_0 instead of IMX_LPCG_CLK_5.
After correct clocks order:
echo on >/sys/devices/platform/bus\@5b000000/5b010000.mmc/power/control
cat /sys/kernel/debug/mmc0/ios
clock: 200000000 Hz
actual clock: 198000000 Hz
^^^^^^^^
...
Fixes: 16c4ea7501b1 ("arm64: dts: imx8: switch to new lpcg clock binding")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi
index 3c42240e78e24..af2259e997967 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi
@@ -67,8 +67,8 @@ usdhc1: mmc@5b010000 {
interrupts = <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x5b010000 0x10000>;
clocks = <&sdhc0_lpcg IMX_LPCG_CLK_4>,
- <&sdhc0_lpcg IMX_LPCG_CLK_0>,
- <&sdhc0_lpcg IMX_LPCG_CLK_5>;
+ <&sdhc0_lpcg IMX_LPCG_CLK_5>,
+ <&sdhc0_lpcg IMX_LPCG_CLK_0>;
clock-names = "ipg", "ahb", "per";
power-domains = <&pd IMX_SC_R_SDHC_0>;
status = "disabled";
@@ -78,8 +78,8 @@ usdhc2: mmc@5b020000 {
interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x5b020000 0x10000>;
clocks = <&sdhc1_lpcg IMX_LPCG_CLK_4>,
- <&sdhc1_lpcg IMX_LPCG_CLK_0>,
- <&sdhc1_lpcg IMX_LPCG_CLK_5>;
+ <&sdhc1_lpcg IMX_LPCG_CLK_5>,
+ <&sdhc1_lpcg IMX_LPCG_CLK_0>;
clock-names = "ipg", "ahb", "per";
power-domains = <&pd IMX_SC_R_SDHC_1>;
fsl,tuning-start-tap = <20>;
@@ -91,8 +91,8 @@ usdhc3: mmc@5b030000 {
interrupts = <GIC_SPI 234 IRQ_TYPE_LEVEL_HIGH>;
reg = <0x5b030000 0x10000>;
clocks = <&sdhc2_lpcg IMX_LPCG_CLK_4>,
- <&sdhc2_lpcg IMX_LPCG_CLK_0>,
- <&sdhc2_lpcg IMX_LPCG_CLK_5>;
+ <&sdhc2_lpcg IMX_LPCG_CLK_5>,
+ <&sdhc2_lpcg IMX_LPCG_CLK_0>;
clock-names = "ipg", "ahb", "per";
power-domains = <&pd IMX_SC_R_SDHC_2>;
status = "disabled";
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
From: Marc Zyngier @ 2024-03-22 16:37 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-arm-kernel, kvm, Paolo Bonzini, Jonathan Corbet,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Len Brown, Pavel Machek, Mostafa Saleh,
Jean-Philippe Brucker, linux-doc, linux-kernel, kvmarm, linux-pm
In-Reply-To: <9efb39597fa7b36b6c4202ab73fae6610194e45e.camel@infradead.org>
On Fri, 22 Mar 2024 16:12:44 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Fri, 2024-03-22 at 16:02 +0000, Marc Zyngier wrote:
> > On Tue, 19 Mar 2024 12:59:06 +0000,
> > David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > [...]
> >
> > > +static void __init psci_init_system_off2(void)
> > > +{
> > > + int ret;
> > > +
> > > + ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> > > +
> > > + if (ret != PSCI_RET_NOT_SUPPORTED)
> > > + psci_system_off2_supported = true;
> >
> > It'd be worth considering the (slightly broken) case where SYSTEM_OFF2
> > is supported, but HIBERNATE_OFF is not set in the response, as the
> > spec doesn't say that this bit is mandatory (it seems legal to
> > implement SYSTEM_OFF2 without any hibernate type, making it similar to
> > SYSTEM_OFF).
>
> Such is not my understanding. If SYSTEM_OFF2 is supported, then
> HIBERNATE_OFF *is* mandatory.
>
> The next update to the spec is turning the PSCI_FEATURES response into
> a *bitmap* of the available features, and I believe it will mandate
> that bit zero is set.
The bitmap is already present in the current Alpha spec:
<quote>
5.16.2 Implementation responsibilities
[...]
Bits[31] Reserved, must be zero.
Bits[30:0] Hibernate types supported.
- 0x0 - HIBERNATE_OFF
All other values are reserved for future use.
</quote>
and doesn't say (yet) that HIBERNATE_OFF is mandatory. Furthermore,
<quote>
5.11.2 Caller responsibilities
The calling OS uses the PSCI_FEATURES API, with the SYSTEM_OFF2
function ID, to discover whether the function is present:
- If the function is implemented, PSCI_FEATURES returns the hibernate
types supported.
- If the function is not implemented, PSCI_FEATURES returns
NOT_SUPPORTED.
</quote>
which doesn't say anything about which hibernate type must be
implemented. Which makes sense, as I expect it to, in the fine ARM
tradition, grow things such as "HIBERNATE_WITH_ROT13_ENCRYPTION" and
even "HIBERNATE_WITH_ERRATA_XYZ", because firmware is where people
dump their crap. And we will need some special handling for these
tasty variants.
> And if for whatever reason that SYSTEM_OFF2/HIBERNATE_OFF call
> *doesn't* work, Linux will end up doing a 'real' poweroff, first
> through EFI and then finally as a last resort with a PSCI SYSTEM_OFF.
> So it would be OK to have false positives in the detection.
I agree that nothing really breaks, but I also hold the view that
broken firmware implementations should be given the finger, specially
given that you have done this work *ahead* of the spec. I would really
like this to fail immediately on these and not even try to suspend.
With that in mind, if doesn't really matter whether HIBERNATE_OFF is
mandatory or not. We really should check for it and pretend it doesn't
exist if the correct flag isn't set.
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 0/1] phy: freescale: imx8m-pcie: facing pcie link-up instability
From: Fabio Estevam @ 2024-03-22 16:29 UTC (permalink / raw)
To: Marcel Ziswiler
Cc: linux-phy, linux-imx, Lucas Stach, linux-arm-kernel, kernel,
Richard Zhu, linux-kernel, Marcel Ziswiler, Heiko Stuebner,
Kishon Vijay Abraham I, Marc Kleine-Budde, Rob Herring,
Sascha Hauer, Shawn Guo, Tim Harvey, Vinod Koul, Yang Li, imx
In-Reply-To: <20240322130646.1016630-1-marcel@ziswiler.com>
Hi Marcel,
On Fri, Mar 22, 2024 at 10:07 AM Marcel Ziswiler <marcel@ziswiler.com> wrote:
>
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
>
> In our automated testing setup, we use Delock Mini-PCIe SATA cards [1].
> While this setup has proven very stable overall we noticed upstream on
> the i.MX8M Mini fails quite regularly (about 50/50) to bring up the PCIe
> link while with NXP's downstream BSP 5.15.71_2.2.2 it always works. As
> that old downstream stuff was quite different, I first also tried NXP's
> latest downstream BSP 6.1.55_2.2.0 which from a PCIe point of view is
> fairly vanilla, however, also there the PCIe link-up was not stable.
> Comparing and debugging I noticed that upstream explicitly configures
> the AUX_PLL_REFCLK_SEL to I_PLL_REFCLK_FROM_SYSPLL while working
> downstream [2] leaving it at reset defaults of AUX_IN (PLL clock).
> Unfortunately, the TRM does not mention any further details about this
> register (both for the i.MX 8M Mini as well as the Plus). Maybe somebody
> from NXP could further comment on this?
>
> BTW: On the i.MX 8M Plus we have not seen any issues with PCIe with the
> exact same setup which is why I left it unchanged.
>
> [1] https://www.delock.com/produkt/95233/merkmale.html
> [2] https://github.com/nxp-imx/linux-imx/blob/lf-5.15.71-2.2.0/drivers/pci/controller/dwc/pci-imx6.c#L1548
The detailed information above is important. It should be in the
commit log of the patch IMHO.
Thanks for the fix.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 5/8] drm: zynqmp_dp: Don't retrain the link in our IRQ
From: Sean Anderson @ 2024-03-22 16:18 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Michal Simek, David Airlie, linux-kernel, Daniel Vetter,
linux-arm-kernel, Laurent Pinchart, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, dri-devel
In-Reply-To: <d6a8bc5c-aed9-4ef4-adb2-dc171106b44b@ideasonboard.com>
On 3/22/24 01:32, Tomi Valkeinen wrote:
> On 21/03/2024 21:17, Sean Anderson wrote:
>> On 3/21/24 15:08, Tomi Valkeinen wrote:
>>> On 21/03/2024 20:01, Sean Anderson wrote:
>>>> On 3/21/24 13:25, Tomi Valkeinen wrote:
>>>>> On 21/03/2024 17:52, Sean Anderson wrote:
>>>>>> On 3/20/24 02:53, Tomi Valkeinen wrote:
>>>>>>> On 20/03/2024 00:51, Sean Anderson wrote:
>>>>>>> Do we need to handle interrupts while either delayed work is being done?
>>>>>>
>>>>>> Probably not.
>>>>>>
>>>>>>> If we do need a delayed work, would just one work be enough which
>>>>>>> handles both HPD_EVENT and HPD_IRQ, instead of two?
>>>>>>
>>>>>> Maybe, but then we need to determine which pending events we need to
>>>>>> handle. I think since we have only two events it will be easier to just
>>>>>> have separate workqueues.
>>>>>
>>>>> The less concurrency, the better...Which is why it would be nice to do it all in the threaded irq.
>>>>
>>>> Yeah, but we can use a mutex for this which means there is not too much
>>>> interesting going on.
>>>
>>> Ok. Yep, if we get (hopefully) a single mutex with clearly defined fields that it protects, I'm ok with workqueues.
>>>
>>> I'd still prefer just one workqueue, though...
>>
>> Yeah, but then we need a spinlock or something to tell the workqueue what it should do.
>
> Yep. We could also always look at the HPD (if we drop the big sleeps) in the wq, and have a flag for the HPD IRQ, which would reduce the state to a single bit.
How about something like
zynqmp_dp_irq_handler(...)
{
/* Read status and handle underflow/overflow/vblank */
status &= ZYNQMP_DP_INT_HPD_EVENT | ZYNQMP_DP_INT_HPD_IRQ;
if (status) {
atomic_or(status, &dp->status);
return IRQ_WAKE_THREAD;
}
return IRQ_HANDLED;
}
zynqmp_dp_thread_handler(...)
{
status = atomic_xchg(&dp->status, 0);
/* process HPD stuff */
}
which gets rid of the workqueue too.
--Sean
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 2/5] KVM: arm64: Add support for PSCI v1.2 and v1.3
From: David Woodhouse @ 2024-03-22 16:14 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-arm-kernel, kvm, Paolo Bonzini, Jonathan Corbet,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Len Brown, Pavel Machek, Mostafa Saleh,
Jean-Philippe Brucker, linux-doc, linux-kernel, kvmarm, linux-pm
In-Reply-To: <86il1ez1zn.wl-maz@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 523 bytes --]
On Fri, 2024-03-22 at 16:05 +0000, Marc Zyngier wrote:
>
> Consider making the visibility of v1.2/1.3 to userspace and guest the
> last patch in the series, so that there is no transient support for
> some oddball PSCI version with no feature (keeps bisection clean).
Ack. I think I can just reorder the patches and that will Just Work,
and the check for 'minor >= 3' will be tautologically false until the
later patch which adds v1.3 support for real. Will do that and test
that it does indeed build that way.
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 5/5] arm64: Use SYSTEM_OFF2 PSCI call to power off for hibernate
From: David Woodhouse @ 2024-03-22 16:12 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-arm-kernel, kvm, Paolo Bonzini, Jonathan Corbet,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Len Brown, Pavel Machek, Mostafa Saleh,
Jean-Philippe Brucker, linux-doc, linux-kernel, kvmarm, linux-pm
In-Reply-To: <86jzluz24b.wl-maz@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 1297 bytes --]
On Fri, 2024-03-22 at 16:02 +0000, Marc Zyngier wrote:
> On Tue, 19 Mar 2024 12:59:06 +0000,
> David Woodhouse <dwmw2@infradead.org> wrote:
>
> [...]
>
> > +static void __init psci_init_system_off2(void)
> > +{
> > + int ret;
> > +
> > + ret = psci_features(PSCI_FN_NATIVE(1_3, SYSTEM_OFF2));
> > +
> > + if (ret != PSCI_RET_NOT_SUPPORTED)
> > + psci_system_off2_supported = true;
>
> It'd be worth considering the (slightly broken) case where SYSTEM_OFF2
> is supported, but HIBERNATE_OFF is not set in the response, as the
> spec doesn't say that this bit is mandatory (it seems legal to
> implement SYSTEM_OFF2 without any hibernate type, making it similar to
> SYSTEM_OFF).
Such is not my understanding. If SYSTEM_OFF2 is supported, then
HIBERNATE_OFF *is* mandatory.
The next update to the spec is turning the PSCI_FEATURES response into
a *bitmap* of the available features, and I believe it will mandate
that bit zero is set.
And if for whatever reason that SYSTEM_OFF2/HIBERNATE_OFF call
*doesn't* work, Linux will end up doing a 'real' poweroff, first
through EFI and then finally as a last resort with a PSCI SYSTEM_OFF.
So it would be OK to have false positives in the detection.
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 00/12] mm/gup: Unify hugetlb, part 2
From: Jason Gunthorpe @ 2024-03-22 16:10 UTC (permalink / raw)
To: peterx
Cc: linux-mm, linux-kernel, linuxppc-dev, Michael Ellerman,
Christophe Leroy, Matthew Wilcox, Rik van Riel, Lorenzo Stoakes,
Axel Rasmussen, Yang Shi, John Hubbard, linux-arm-kernel,
Kirill A . Shutemov, Andrew Jones, Vlastimil Babka, Mike Rapoport,
Andrew Morton, Muchun Song, Christoph Hellwig, linux-riscv,
James Houghton, David Hildenbrand, Andrea Arcangeli,
Aneesh Kumar K . V, Mike Kravetz
In-Reply-To: <20240321220802.679544-1-peterx@redhat.com>
On Thu, Mar 21, 2024 at 06:07:50PM -0400, peterx@redhat.com wrote:
> From: Peter Xu <peterx@redhat.com>
>
> v3:
> - Rebased to latest mm-unstalbe (a824831a082f, of March 21th)
> - Dropped patch to introduce pmd_thp_or_huge(), replace such uses (and also
> pXd_huge() users) with pXd_leaf() [Jason]
> - Add a comment for CONFIG_PGTABLE_HAS_HUGE_LEAVES [Jason]
> - Use IS_ENABLED() in follow_huge_pud() [Jason]
> - Remove redundant none pud check in follow_pud_mask() [Jason]
>
> rfc: https://lore.kernel.org/r/20231116012908.392077-1-peterx@redhat.com
> v1: https://lore.kernel.org/r/20231219075538.414708-1-peterx@redhat.com
> v2: https://lore.kernel.org/r/20240103091423.400294-1-peterx@redhat.com
>
> The series removes the hugetlb slow gup path after a previous refactor work
> [1], so that slow gup now uses the exact same path to process all kinds of
> memory including hugetlb.
>
> For the long term, we may want to remove most, if not all, call sites of
> huge_pte_offset(). It'll be ideal if that API can be completely dropped
> from arch hugetlb API. This series is one small step towards merging
> hugetlb specific codes into generic mm paths. From that POV, this series
> removes one reference to huge_pte_offset() out of many others.
This remark would be a little easier to understand if you refer to
hugetlb_walk() not huge_pte_offset() - the latter is only used to
implement hugetlb_walk() and isn't removed by this series, while a
single hugetlb_walk() was removed.
Regardless, I think the point is spot on, the direction should be to
make the page table reading generic with minimal/no interaction with
hugetlbfs in the generic code.
After this series I would suggest doing the pagewalk.c stuff as it is
very parallel to GUP slow (indeed it would be amazing to figure out a
way to make GUP slow and pagewalk.c use the same code without a
performance cost)
Some of the other core mm callers don't look too bad either, getting
to the point where hugetlb_walk() is internal to hugetlb.c would be a
nice step that looks reasonable size.
> One goal of such a route is that we can reconsider merging hugetlb features
> like High Granularity Mapping (HGM). It was not accepted in the past
> because it may add lots of hugetlb specific codes and make the mm code even
> harder to maintain. With a merged codeset, features like HGM can hopefully
> share some code with THP, legacy (PMD+) or modern (continuous PTEs).
Yeah, if all the special hugetlb stuff is using generic arch code and
generic page walkers (maybe with that special vma locking hook) it is
much easier to swallow than adding yet another special class of code
to all the page walkers.
> To make it work, the generic slow gup code will need to at least understand
> hugepd, which is already done like so in fast-gup. Due to the specialty of
> hugepd to be software-only solution (no hardware recognizes the hugepd
> format, so it's purely artificial structures), there's chance we can merge
> some or all hugepd formats with cont_pte in the future. That question is
> yet unsettled from Power side to have an acknowledgement.
At a minimum, I think we have a concurrence that the reading of the
hugepd entries should be fine through the standard contig pte APIs and
so all the page walkers doing read side operations could stop having
special hugepd code. It is just an implementation of contig pte with
the new property that the size of a PTE can be larger than a PMD
entry.
If, or how much, the hugepd write side remains special is the open
question, I think.
> this series, I kept the hugepd handling because we may still need to do so
> before getting a clearer picture of the future of hugepd. The other reason
> is simply that we did it already for fast-gup and most codes are still
> around to be reused. It'll make more sense to keep slow/fast gup behave
> the same before a decision is made to remove hugepd.
Yeah, I think that is right for this series. Lets get this done and
then try to remove hugepd read side.
Thanks,
Jason
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 64/64] i2c: reword i2c_algorithm in drivers according to newest specification
From: Andy Shevchenko @ 2024-03-22 16:09 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, Andi Shyti, Krzysztof Kozlowski, linux-arm-kernel,
chrome-platform, linux-samsung-soc, imx, linux-mips,
linux-amlogic, linux-mediatek, openbmc, linux-omap, linuxppc-dev,
asahi, linux-arm-msm, linux-renesas-soc, linux-stm32, linux-tegra,
virtualization
In-Reply-To: <20240322132619.6389-65-wsa+renesas@sang-engineering.com>
On Fri, Mar 22, 2024 at 02:25:57PM +0100, Wolfram Sang wrote:
> Match the wording in i2c_algorithm in I2C drivers wrt. the newest I2C
> v7, SMBus 3.2, I3C specifications and replace "master/slave" with more
> appropriate terms. For some drivers, this means no more conversions are
> needed. For the others more work needs to be done but this will be
> performed incrementally along with API changes/improvements. All these
> changes here are simple search/replace results.
...
> static const struct i2c_algorithm at91_twi_algorithm = {
> - .master_xfer = at91_twi_xfer,
> + .xfer = at91_twi_xfer,
Seems you made this by a script, can you check the indentations afterwards?
> .functionality = at91_twi_func,
> };
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 0/5] Add PSCI v1.3 SYSTEM_OFF2 support for hibernation
From: Marc Zyngier @ 2024-03-22 16:09 UTC (permalink / raw)
To: David Woodhouse
Cc: Oliver Upton, linux-arm-kernel, kvm, Paolo Bonzini,
Jonathan Corbet, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Len Brown, Pavel Machek, Mostafa Saleh,
Jean-Philippe Brucker, linux-doc, linux-kernel, kvmarm, linux-pm
In-Reply-To: <248ec30e7b5c8a42d184f029c1cc9b664656b356.camel@infradead.org>
On Fri, 22 Mar 2024 10:17:58 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
>
> [1 <text/plain; UTF-8 (quoted-printable)>]
> On Tue, 2024-03-19 at 12:41 -0700, Oliver Upton wrote:
> > On Tue, Mar 19, 2024 at 05:14:42PM +0000, David Woodhouse wrote:
> > > On Tue, 2024-03-19 at 08:27 -0700, Oliver Upton wrote:
> > > > If we're going down the route of having this PSCI call live in KVM, it
> > > > really deserves a test. I think you can just pile on the existing
> > > > psci_test selftest.
> > >
> > > Added to
> > > https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/psci-hibernate
> > > for next time.
> > >
> > > From 8c72a78e6179bc8970edc66a85ab6bee26f581fb Mon Sep 17 00:00:00 2001
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > Date: Tue, 19 Mar 2024 17:07:46 +0000
> > > Subject: [PATCH 4/8] KVM: selftests: Add test for PSCI SYSTEM_OFF2
> > >
> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> >
> > Looks good, thanks!
>
> Thanks.
>
> Marc, I think I've also addressed your feedback? Is there anything else
> to do other than wait for the spec to be published?
Other than the couple of minor nits I mentioned in replies to the
individual patches, this looks good to me.
> Shall I post a v4 with PSCI v1.3 as default and the self-test? Would
> you apply that into a branch ready for merging when the spec is ready,
> or should I just wait and repost it all then?
I think this can wait for the final spec. I assume that you are
directly tracking this anyway, so we don't need to poll for the spec
update.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 12/12] mm/gup: Handle hugetlb in the generic follow_page_mask code
From: Jason Gunthorpe @ 2024-03-22 16:08 UTC (permalink / raw)
To: Peter Xu
Cc: linux-mm, linux-kernel, linuxppc-dev, Michael Ellerman,
Christophe Leroy, Matthew Wilcox, Rik van Riel, Lorenzo Stoakes,
Axel Rasmussen, Yang Shi, John Hubbard, linux-arm-kernel,
Kirill A . Shutemov, Andrew Jones, Vlastimil Babka, Mike Rapoport,
Andrew Morton, Muchun Song, Christoph Hellwig, linux-riscv,
James Houghton, David Hildenbrand, Andrea Arcangeli,
Aneesh Kumar K . V, Mike Kravetz
In-Reply-To: <Zf2p38Pb51T3e9uB@x1n>
On Fri, Mar 22, 2024 at 11:55:11AM -0400, Peter Xu wrote:
> Jason,
>
> On Fri, Mar 22, 2024 at 10:30:12AM -0300, Jason Gunthorpe wrote:
> > On Thu, Mar 21, 2024 at 06:08:02PM -0400, peterx@redhat.com wrote:
> >
> > > A quick performance test on an aarch64 VM on M1 chip shows 15% degrade over
> > > a tight loop of slow gup after the path switched. That shouldn't be a
> > > problem because slow-gup should not be a hot path for GUP in general: when
> > > page is commonly present, fast-gup will already succeed, while when the
> > > page is indeed missing and require a follow up page fault, the slow gup
> > > degrade will probably buried in the fault paths anyway. It also explains
> > > why slow gup for THP used to be very slow before 57edfcfd3419 ("mm/gup:
> > > accelerate thp gup even for "pages != NULL"") lands, the latter not part of
> > > a performance analysis but a side benefit. If the performance will be a
> > > concern, we can consider handle CONT_PTE in follow_page().
> >
> > I think this is probably fine for the moment, at least for this
> > series, as CONT_PTE is still very new.
> >
> > But it will need to be optimized. "slow" GUP is the only GUP that is
> > used by FOLL_LONGTERM and it still needs to be optimized because you
> > can't assume a FOLL_LONGTERM user will be hitting the really slow
> > fault path. There are enough important cases where it is just reading
> > already populted page tables, and these days, often with large folios.
>
> Ah, I thought FOLL_LONGTERM should work in most cases for fast-gup,
> especially for hugetlb, but maybe I missed something?
Ah, no this is my bad memory, there was a time where that was true,
but it is not the case now. Oh, it is a really bad memory because it
seems I removed parts of it :)
> I do see that devmap skips fast-gup for LONGTERM, we also have that
> writeback issue but none of those that I can find applies to
> hugetlb. This might be a problem indeed if we have hugetlb cont_pte
> pages that will constantly fallback to slow gup.
Right, DAX would be the main use case I can think of. Today the
intersection of DAX and contig PTE is non-existant so lets not worry.
> OTOH, I also agree with you that such batching would be nice to have for
> slow-gup, likely devmap or many fs (exclude shmem/hugetlb) file mappings
> can at least benefit from it due to above. But then that'll be a more
> generic issue to solve, IOW, we still don't do that for !hugetlb cont_pte
> large folios, before or after this series.
Right, improving contig pte is going to be a process and eventually it
will make sense to optimize this regardless of hugetlbfs
Jason
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: Why is the ARM SMMU v1/v2 put into bypass mode on kexec?
From: Will Deacon @ 2024-03-22 16:06 UTC (permalink / raw)
To: Tyler Hicks
Cc: Robin Murphy, Jason Gunthorpe, Jerry Snitselaar, linux-arm-kernel,
iommu, linux-kernel, Dexuan Cui, Easwar Hariharan
In-Reply-To: <ZfnkEqglNPRzH3Zk@sequoia>
On Tue, Mar 19, 2024 at 02:14:26PM -0500, Tyler Hicks wrote:
> On 2024-03-19 15:47:56, Will Deacon wrote:
> > On Tue, Mar 19, 2024 at 12:57:52PM +0000, Robin Murphy wrote:
> > > Beyond properly quiescing and resetting the system back to a boot-time
> > > state, the outgoing kernel in a kexec can only really do things which affect
> > > itself. Sure, we *could* configure the SMMU to block all traffic and disable
> > > the interrupt to avoid getting stuck in a storm of faults on the way out,
> > > but what does that mean for the incoming kexec payload? That it can have the
> > > pleasure of discovering the SMMU, innocently enabling the interrupt and
> > > getting stuck in an unexpected storm of faults. Or perhaps just resetting
> > > the SMMU into a disabled state and thus still unwittingly allowing its
> > > memory to be corrupted by the previous kernel not supporting kexec properly.
> >
> > Right, it's hard to win if DMA-active devices weren't quiesced properly
> > by the outgoing kernel. Either the SMMU was left in abort (leading to the
> > problems you list above) or the SMMU is left in bypass (leading to possible
> > data corruption). Which is better?
>
> My thoughts are that a loud and obvious failure (via unidentified stream
> fault messages and/or a possible interrupt storm preventing the new
> kernel from booting) is favorable to silent and subtle data corruption
> of the target kernel.
Looking at the SMMUv3 spec, the architecture does actually allow hardware
to reset into an aborting state:
[GBPA.ABORT]
| Note: An implementation can reset this field to 1, in order to
| implement a default deny policy on reset.
so perhaps it's not that unreasonable. I just dread the flood of emails
I'll get because the SMMU driver is noisy due to missing ->shutdown()
callbacks elsewhere :/
> > The best solution is obviously to implement those missing ->shutdown()
> > callbacks.
>
> Completely agree here but it can be difficult to even identify that a
> missing ->shutdown hook is the root cause without code changes to put
> the SMMU into abort mode and sleep for a bit in the SMMU's ->shutdown
> hook.
Perhaps that's the thing to tackle first, then? If we make it easier for
folks to diagnose and fix the missing ->shutdown() callbacks, then going
into abort is much more reasonable,
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 3/5] KVM: arm64: Add PSCI v1.3 SYSTEM_OFF2 function for hibernation
From: Marc Zyngier @ 2024-03-22 16:06 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-arm-kernel, kvm, Paolo Bonzini, Jonathan Corbet,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Len Brown, Pavel Machek, David Woodhouse,
Mostafa Saleh, Jean-Philippe Brucker, linux-doc, linux-kernel,
kvmarm, linux-pm
In-Reply-To: <20240319130957.1050637-4-dwmw2@infradead.org>
On Tue, 19 Mar 2024 12:59:04 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The PSCI v1.3 specification (alpha) adds support for a SYSTEM_OFF2 function
> which is analogous to ACPI S4 state. This will allow hosting environments
> to determine that a guest is hibernated rather than just powered off, and
> ensure that they preserve the virtual environment appropriately to allow
> the guest to resume safely (or bump the hardware_signature in the FACS to
> trigger a clean reboot instead).
>
> The beta version will be changed to say that PSCI_FEATURES returns a bit
> mask of the supported hibernate types, which is implemented here.
>
> Although this new feature is inflicted unconditionally on unexpecting
> userspace, it ought to be mostly OK because it still results in the same
> KVM_SYSTEM_EVENT_SHUTDOWN event, just with a new flag which hopefully
> won't cause userspace to get unhappy.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> Documentation/virt/kvm/api.rst | 11 +++++++++
> arch/arm64/include/uapi/asm/kvm.h | 6 +++++
> arch/arm64/kvm/psci.c | 37 +++++++++++++++++++++++++++++++
> 3 files changed, 54 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 0b5a33ee71ee..ba4ddb13e253 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6761,6 +6761,10 @@ the first `ndata` items (possibly zero) of the data array are valid.
> the guest issued a SYSTEM_RESET2 call according to v1.1 of the PSCI
> specification.
>
> + - for arm64, data[0] is set to KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2
> + if the guest issued a SYSTEM_OFF2 call according to v1.3 of the PSCI
> + specification.
> +
> - for RISC-V, data[0] is set to the value of the second argument of the
> ``sbi_system_reset`` call.
>
> @@ -6794,6 +6798,13 @@ either:
> - Deny the guest request to suspend the VM. See ARM DEN0022D.b 5.19.2
> "Caller responsibilities" for possible return values.
>
> +Hibernation using the PSCI SYSTEM_OFF2 call is enabled when PSCI v1.3
> +is enabled. If a guest invokes the PSCI SYSTEM_OFF2 function, KVM will
> +exit to userspace with the KVM_SYSTEM_EVENT_SHUTDOWN event type and with
> +data[0] set to KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2. The only
> +supported hibernate type for the SYSTEM_OFF2 function is HIBERNATE_OFF
> +0x0).
> +
> ::
>
> /* KVM_EXIT_IOAPIC_EOI */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 964df31da975..66736ff04011 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -484,6 +484,12 @@ enum {
> */
> #define KVM_SYSTEM_EVENT_RESET_FLAG_PSCI_RESET2 (1ULL << 0)
>
> +/*
> + * Shutdown caused by a PSCI v1.3 SYSTEM_OFF2 call.
> + * Valid only when the system event has a type of KVM_SYSTEM_EVENT_SHUTDOWN.
> + */
> +#define KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2 (1ULL << 0)
> +
> /* run->fail_entry.hardware_entry_failure_reason codes. */
> #define KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED (1ULL << 0)
>
> diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
> index f689ef3f2f10..7acf07900c08 100644
> --- a/arch/arm64/kvm/psci.c
> +++ b/arch/arm64/kvm/psci.c
> @@ -194,6 +194,12 @@ static void kvm_psci_system_off(struct kvm_vcpu *vcpu)
> kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN, 0);
> }
>
> +static void kvm_psci_system_off2(struct kvm_vcpu *vcpu)
> +{
> + kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN,
> + KVM_SYSTEM_EVENT_SHUTDOWN_FLAG_PSCI_OFF2);
> +}
> +
> static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
> {
> kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET, 0);
> @@ -353,6 +359,11 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
> if (test_bit(KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED, &kvm->arch.flags))
> val = 0;
> break;
> + case PSCI_1_3_FN_SYSTEM_OFF2:
> + case PSCI_1_3_FN64_SYSTEM_OFF2:
> + if (minor >= 3)
> + val = 1UL << PSCI_1_3_HIBERNATE_TYPE_OFF;
> + break;
> case PSCI_1_1_FN_SYSTEM_RESET2:
> case PSCI_1_1_FN64_SYSTEM_RESET2:
nit: please keep the switch ordered by version number.
> if (minor >= 1)
> @@ -374,6 +385,32 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
> return 0;
> }
> break;
> + case PSCI_1_3_FN_SYSTEM_OFF2:
> + kvm_psci_narrow_to_32bit(vcpu);
> + fallthrough;
> + case PSCI_1_3_FN64_SYSTEM_OFF2:
> + if (minor < 3)
> + break;
> +
> + arg = smccc_get_arg1(vcpu);
> + if (arg != PSCI_1_3_HIBERNATE_TYPE_OFF) {
> + val = PSCI_RET_INVALID_PARAMS;
> + break;
> + }
> + kvm_psci_system_off2(vcpu);
> + /*
> + * We shouldn't be going back to guest VCPU after
> + * receiving SYSTEM_OFF2 request.
> + *
> + * If user space accidentally/deliberately resumes
> + * guest VCPU after SYSTEM_OFF2 request then guest
> + * VCPU should see internal failure from PSCI return
> + * value. To achieve this, we preload r0 (or x0) with
> + * PSCI return value INTERNAL_FAILURE.
> + */
> + val = PSCI_RET_INTERNAL_FAILURE;
> + ret = 0;
> + break;
> case PSCI_1_1_FN_SYSTEM_RESET2:
> kvm_psci_narrow_to_32bit(vcpu);
> fallthrough;
Same thing here.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 2/5] KVM: arm64: Add support for PSCI v1.2 and v1.3
From: Marc Zyngier @ 2024-03-22 16:05 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-arm-kernel, kvm, Paolo Bonzini, Jonathan Corbet,
Oliver Upton, James Morse, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Mark Rutland, Lorenzo Pieralisi,
Rafael J. Wysocki, Len Brown, Pavel Machek, David Woodhouse,
Mostafa Saleh, Jean-Philippe Brucker, linux-doc, linux-kernel,
kvmarm, linux-pm
In-Reply-To: <20240319130957.1050637-3-dwmw2@infradead.org>
On Tue, 19 Mar 2024 12:59:03 +0000,
David Woodhouse <dwmw2@infradead.org> wrote:
>
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Since the v1.3 specification is still in Alpha, only default to v1.2
> unless userspace explicitly requests v1.3 for now.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> arch/arm64/kvm/hypercalls.c | 2 ++
> arch/arm64/kvm/psci.c | 6 +++++-
> include/kvm/arm_psci.h | 4 +++-
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 5763d979d8ca..9c6267ca2b82 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -575,6 +575,8 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> case KVM_ARM_PSCI_0_2:
> case KVM_ARM_PSCI_1_0:
> case KVM_ARM_PSCI_1_1:
> + case KVM_ARM_PSCI_1_2:
> + case KVM_ARM_PSCI_1_3:
> if (!wants_02)
> return -EINVAL;
> vcpu->kvm->arch.psci_version = val;
> diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
> index 1f69b667332b..f689ef3f2f10 100644
> --- a/arch/arm64/kvm/psci.c
> +++ b/arch/arm64/kvm/psci.c
> @@ -322,7 +322,7 @@ static int kvm_psci_1_x_call(struct kvm_vcpu *vcpu, u32 minor)
>
> switch(psci_fn) {
> case PSCI_0_2_FN_PSCI_VERSION:
> - val = minor == 0 ? KVM_ARM_PSCI_1_0 : KVM_ARM_PSCI_1_1;
> + val = PSCI_VERSION(1, minor);
> break;
> case PSCI_1_0_FN_PSCI_FEATURES:
> arg = smccc_get_arg1(vcpu);
> @@ -449,6 +449,10 @@ int kvm_psci_call(struct kvm_vcpu *vcpu)
> }
>
> switch (version) {
> + case KVM_ARM_PSCI_1_3:
> + return kvm_psci_1_x_call(vcpu, 3);
> + case KVM_ARM_PSCI_1_2:
> + return kvm_psci_1_x_call(vcpu, 2);
> case KVM_ARM_PSCI_1_1:
> return kvm_psci_1_x_call(vcpu, 1);
> case KVM_ARM_PSCI_1_0:
> diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
> index e8fb624013d1..ebd7d9a12790 100644
> --- a/include/kvm/arm_psci.h
> +++ b/include/kvm/arm_psci.h
> @@ -14,8 +14,10 @@
> #define KVM_ARM_PSCI_0_2 PSCI_VERSION(0, 2)
> #define KVM_ARM_PSCI_1_0 PSCI_VERSION(1, 0)
> #define KVM_ARM_PSCI_1_1 PSCI_VERSION(1, 1)
> +#define KVM_ARM_PSCI_1_2 PSCI_VERSION(1, 2)
> +#define KVM_ARM_PSCI_1_3 PSCI_VERSION(1, 3)
>
> -#define KVM_ARM_PSCI_LATEST KVM_ARM_PSCI_1_1
> +#define KVM_ARM_PSCI_LATEST KVM_ARM_PSCI_1_2 /* v1.3 is still Alpha */
>
> static inline int kvm_psci_version(struct kvm_vcpu *vcpu)
> {
Consider making the visibility of v1.2/1.3 to userspace and guest the
last patch in the series, so that there is no transient support for
some oddball PSCI version with no feature (keeps bisection clean).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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