Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] regulator: mt6360: Add support for MT6360 regulator
From: Mark Brown @ 2020-06-04 13:39 UTC (permalink / raw)
  To: Gene Chen
  Cc: gene_chen, lgirdwood, linux-kernel, cy_huang, benjamin.chao,
	linux-mediatek, matthias.bgg, Wilma.Wu, linux-arm-kernel,
	shufan_lee
In-Reply-To: <1591254387-10304-1-git-send-email-gene.chen.richtek@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2567 bytes --]

On Thu, Jun 04, 2020 at 03:06:27PM +0800, Gene Chen wrote:

This looks nice and simple, a few fairly small comments below but high
level it's basically fine.

> --- /dev/null
> +++ b/drivers/regulator/mt6360-regulator.c
> @@ -0,0 +1,571 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 MediaTek Inc.

Please make the entire comment a C++ one so things look more
intentional.

> +	for (i = 0; i < devdata->num_irq_descs; i++) {
> +		irq_desc = devdata->irq_descs + i;
> +		if (unlikely(!irq_desc->name))
> +			continue;

Do we really need an unlikely here?  This shouldn't be a hot path.

> +static int mt6360_regulator_set_mode(
> +				  struct regulator_dev *rdev, unsigned int mode)
> +{

> +	switch (1 << (ffs(mode) - 1)) {
> +	case REGULATOR_MODE_NORMAL:

I don't understand why this isn't just a straight switch on mode?

> +static unsigned int mt6360_regulator_get_mode(struct regulator_dev *rdev)
> +{
> +	const struct mt6360_regulator_desc *desc =
> +			       (const struct mt6360_regulator_desc *)rdev->desc;
> +	int shift = ffs(desc->mode_get_mask) - 1, ret;
> +	unsigned int val = 0;
> +
> +	default:
> +		ret = 0;
> +	}

If we can't parse a valid value from the hardware then that's an error.

> +static int mt6360_regulator_reg_write(void *context,
> +				      unsigned int reg, unsigned int val)
> +{
> +	struct mt6360_regulator_data *mrd = context;
> +	u8 chunk[4] = {0};
> +
> +	/* chunk 0 ->i2c addr, 1 -> reg_addr, 2 -> reg_val 3-> crc8 */
> +	chunk[0] = (mrd->i2c->addr & 0x7f) << 1;
> +	chunk[1] = reg & 0x3f;
> +	chunk[2] = (u8)val;
> +	chunk[3] = crc8(mrd->crc8_table, chunk, 3, 0);
> +	/* also dummy one byte */
> +	return i2c_smbus_write_i2c_block_data(mrd->i2c, chunk[1], 3, chunk + 2);
> +}

Oh, wow - that's a fun I/O interface!

> +static const struct of_device_id __maybe_unused mt6360_regulator_of_id[] = {
> +	{
> +		.compatible = "mediatek,mt6360_pmic",
> +		.data = (void *)&mt6360_pmic_devdata,
> +	},
> +	{
> +		.compatible = "mediatek,mt6360_ldo",
> +		.data = (void *)&mt6360_ldo_devdata,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mt6360_regulator_of_id);

I don't see any DT bindings documentation for this, documentation is
required for all new bindings.

> +	mrd->regmap = devm_regmap_init(&(mrd->i2c->dev),
> +				       NULL, mrd, devdata->regmap_config);
> +	if (IS_ERR(mrd->regmap)) {
> +		dev_err(&pdev->dev, "Failed to register regmap\n");
> +		return PTR_ERR(mrd->regmap);
> +	}

This looks like a MFD so it's surprising to see us defining a regmap at
this level.  Why are we doing this?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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 0/3] firmware: Add support for PSA FF-A interface
From: Will Deacon @ 2020-06-04 13:37 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: devicetree, qwandor, Marc Zyngier, linux-kernel, linux-arm-kernel,
	ardb, tabba
In-Reply-To: <20200601094512.50509-1-sudeep.holla@arm.com>

Hi Sudeep, [+Fuad, Andrew and Ard]

(To other interested readers: if you haven't seen it, the FF-A spec is here:
 https://static.docs.arm.com/den0077/a/DEN0077A_PSA_Firmware_Framework_Arm_v8-A_1.0_EAC.pdf
 since this discussion makes no sense without that, and a tiny bit of sense
 with it. It used to be called "SPCI" but it was recently renamed.)

On Mon, Jun 01, 2020 at 10:45:09AM +0100, Sudeep Holla wrote:
> Sorry for posting in the middle of merge window and I must have done
> this last week itself. This is not the driver I had thought about posting
> last week. After I started cleaning up and looking at Will's KVM prototype[1]
> for PSA FF-A (previously known as SPCI),

Yes, I need to do the Big Rename at some point. Joy.

> I got more doubts on alignment and dropped huge chunk of interface APIs in
> the driver in order to keep it simple, and get aligned more with that
> prototype and avoid scanning lots of code unnecessary.

You also dropped most of the code, so this doesn't really do anything in
its current form ;)

> Here are few things to clarify:
> 
> 1. DT bindings
> ---------------
> 	I was initially against adding bindings for Tx/Rx buffers for
> 	partitions. As per the spec, an endpoint could allocate the
> 	buffer pair and use the FFA_RXTX_MAP interface to map it with the
> 	Hypervisor(KVM here). However looking at the prototype and also
> 	I remember you mentioning that it is not possible to manage buffers
> 	in that way. Please confirm if you plan to add the buffer details
> 	fetcthing them through ioctls in KVM and adding them to VM DT nodes
> 	in KVM userspace. I will update the bindings accordingly.

I think it's useful to have a mode of operation where the hypervisor
allocates the RX/TX buffers and advertises them in the DT. However, we
can always add this later, so there's no need to have it in the binding
from the start. Best start as simple as possible, I reckon.

Setting the static RX/TX buffer allocation aside, why is a DT node needed
at all for the case where Linux is running purely as an FF-A client? I
thought everything should be discoverable via FFA_VERSION, FFA_FEATURES,
FFA_PARTITION_INFO_GET and FFA_ID_GET? That should mean we can get away
without a binding at all for the client case.

> 2. Driver
> ---------
> a. Support for multiple partitions in a VM
> ------------------------------------------
> 	I am not sure if there is need for supporting multiple partitions
> 	within a VM. It should be possible to do so as I expect to create
> 	device for each partition entry under arm-psa-ffa devicetree node.
> 	However, I don't want to assume something that will never be a
> 	usecase. However I don't think this will change must of the
> 	abstraction as we need to keep the interface API implementation
> 	separate to support different partitions on various platforms.

I think Ard has a case for something like this, where a VM actually consists
of multiple partitions so that S-EL0 services can be provided from NS-EL0.
However, he probably wants that for a dynamically created VM, so we'd
need a way to instantiate an FFA namespace for the VM. Maybe that can be
done entirely in userspace by the VMM...

> b. SMCCC interface
> ------------------
> 	This is something I messed up completely while trying to add
> 	support for SMCCC v1.2. It now supports x0-x17 as parameter
> 	registers(input) and return registers(output). I started simple
> 	with x0-x7 as both input and output as PSA FF-A needs that at
> 	most. But extending to x0-x17 then became with messy in my
> 	implementation. That's the reason I dropped it completely
> 	here and thought of checking it first.
> 
> 	Do we need to extend the optimisations that were done to handle
> 	ARCH_WORKAROUND_{1,2}. Or should be just use a version with x0-x7
> 	as both input and ouput. Hyper-V guys need full x0-x17 support.
> 
> 	I need some guidance as what is the approach preferred ?

I think we can start off with x0-x7 and extend if later if we need to.

> 3. Partitions
> -------------
> 	I am not sure if we have a full define partition that we plan to
> 	push upstream. Without one, we can have a sample/example partition
> 	to test all the interface APIs, but is that fine with respect to
> 	what we want upstream ? Any other thoughts that helps to test the
> 	driver ?

I think that's the best you can do for now. We can probably help with
testing as our stuff gets off the ground.

> Sorry for long email and too many questions, but I thought it is easier
> this way to begin with than throwing huge code implementing loads of APIs
> with no users(expect example partition) especially that I am posting this
> during merge window.

No problem. Maybe it would help if I described roughly what we were thinking
of doing for KVM (this is open for discussion, of course):

 1. Describe KVM-managed partitions in the DT, along the lines of [1]
 2. Expose each partition as a file to userspace. E.g.:

    /dev/spci/:

	self
	e3a48fa5-dc54-4a8b-898b-bdc4dfeeb7b8
	49f65057-d002-4ae2-b4ee-d31c7940a13d

    Here, self would be a symlink to the host uuid. The host uuid file
    would implement FFA_MEM operations using an ioctl(), so you could,
    for example, share a user buffer with multiple partitions by issuing
    a MEM_SHARE ioctl() on self, passing the fds for the borrower partitions
    as arguments. Messaging would be implemented as ioctl()s on the
    partition uuid files themselves.

 3. We'll need some (all?) of these patches to unmap memory from the host
    when necessary:

    https://lwn.net/Articles/821215/

    (for nVHE, we'll have a stage-2 for the host so we can unmap there as
    well)

For communicating with partitions that are not managed by KVM (e.g. trusted
applications), it's not clear to me how much of that will be handled in
kernel or user. I think it would still be worth exposing the partitions as
files, but perhaps having them root only or just returning -EPERM for the
ioctl() if a kernel driver has claimed the partition as its own? Ideally,
FF-A would allow us to transition some of the Trusted OS interfacing code
out to userspace, but I don't know how realistic that is.

Anyway, to enable this, I think we need a clear separation in the kernel
between the FF-A code and the users: KVM will want to expose things as
above, but if drivers need to use this stuff as well then they can plug in
as additional users and we don't have to worry about tripping over the
RX/TX buffers etc.

What do you think, and do you reckon you can spin a cut-down driver that
implements the common part of the logic (since I know you've written much
of this code already)?

Cheers,

Will

[1] https://android-kvm.googlesource.com/linux/+/8632a5723ef106017c4ab57e95d9ce7630d35522%5E%21/#F0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 3/3] KVM: arm64: Enforce PtrAuth being disabled if not advertized
From: Marc Zyngier @ 2020-06-04 13:33 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel
  Cc: Mark Rutland, kernel-team, Suzuki K Poulose, Catalin Marinas,
	James Morse, Will Deacon, Julien Thierry
In-Reply-To: <20200604133354.1279412-1-maz@kernel.org>

Even if we don't expose PtrAuth to a guest, the guest can still
write to its SCTIRLE_1 register and set the En{I,D}{A,B} bits
and execute PtrAuth instructions from the NOP space. This has
the effect of trapping to EL2, and we currently inject an UNDEF.
This is definitely the wrong thing to do, as the architecture says
that these instructions should behave as NOPs.

Instead, we can simply reset the offending SCTLR_EL1 bits to
zero, and resume the guest. It can still observe the SCTLR bits
being set and then being cleared by magic, but that's much better
than delivering an unexpected extension.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/handle_exit.c | 12 ------------
 arch/arm64/kvm/hyp/switch.c  | 18 ++++++++++++++++--
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 5a02d4c90559..98d8adf6f865 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -162,17 +162,6 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	return 1;
 }
 
-/*
- * Guest usage of a ptrauth instruction (which the guest EL1 did not turn into
- * a NOP). If we get here, it is that we didn't fixup ptrauth on exit, and all
- * that we can do is give the guest an UNDEF.
- */
-static int kvm_handle_ptrauth(struct kvm_vcpu *vcpu, struct kvm_run *run)
-{
-	kvm_inject_undefined(vcpu);
-	return 1;
-}
-
 static exit_handle_fn arm_exit_handlers[] = {
 	[0 ... ESR_ELx_EC_MAX]	= kvm_handle_unknown_ec,
 	[ESR_ELx_EC_WFx]	= kvm_handle_wfx,
@@ -195,7 +184,6 @@ static exit_handle_fn arm_exit_handlers[] = {
 	[ESR_ELx_EC_BKPT32]	= kvm_handle_guest_debug,
 	[ESR_ELx_EC_BRK64]	= kvm_handle_guest_debug,
 	[ESR_ELx_EC_FP_ASIMD]	= handle_no_fpsimd,
-	[ESR_ELx_EC_PAC]	= kvm_handle_ptrauth,
 };
 
 static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 2a50b3771c3b..fc09c3dfa466 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -503,8 +503,22 @@ static bool __hyp_text __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
 	struct kvm_cpu_context *ctxt;
 	u64 val;
 
-	if (!vcpu_has_ptrauth(vcpu))
-		return false;
+	if (!vcpu_has_ptrauth(vcpu)) {
+		if (ec != ESR_ELx_EC_PAC)
+			return false;
+
+		/*
+		 * Interesting situation: the guest has enabled PtrAuth,
+		 * despite KVM not advertising it. Fix SCTLR_El1 on behalf
+		 * of the guest (the bits should behave as RES0 anyway).
+		 */
+		val = read_sysreg_el1(SYS_SCTLR);
+		val &= ~(SCTLR_ELx_ENIA | SCTLR_ELx_ENIB |
+			 SCTLR_ELx_ENDA | SCTLR_ELx_ENDB);
+		write_sysreg_el1(val, SYS_SCTLR);
+
+		return true;
+	}
 
 	switch (ec) {
 	case ESR_ELx_EC_PAC:
-- 
2.26.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 2/3] KVM: arm64: Handle PtrAuth traps early
From: Marc Zyngier @ 2020-06-04 13:33 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel
  Cc: Mark Rutland, kernel-team, Suzuki K Poulose, Catalin Marinas,
	James Morse, Will Deacon, Julien Thierry
In-Reply-To: <20200604133354.1279412-1-maz@kernel.org>

The current way we deal with PtrAuth is a bit heavy handed:

- We forcefully save the host's keys on each vcpu_load()
- Handling the PtrAuth trap forces us to go all the way back
  to the exit handling code to just set the HCR bits

Overall, this is pretty heavy handed. A better approach would be
to handle it the same way we deal with the FPSIMD registers:

- On vcpu_load() disable PtrAuth for the guest
- On first use, save the host's keys, enable PtrAuth in the
  guest

Crutially, this can happen as a fixup, which is done very early
on exit. We can then reenter the guest immediately without
leaving the hypervisor role.

Another thing is that it simplify the rest of the host handling:
exiting all the way to the host means that the only possible
outcome for this trap is to inject an UNDEF.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/arm.c         | 17 +----------
 arch/arm64/kvm/handle_exit.c | 17 ++---------
 arch/arm64/kvm/hyp/switch.c  | 59 ++++++++++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c    | 13 +++-----
 4 files changed, 68 insertions(+), 38 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 152049c5055d..14b747266607 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -337,12 +337,6 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
 	preempt_enable();
 }
 
-#define __ptrauth_save_key(regs, key)						\
-({										\
-	regs[key ## KEYLO_EL1] = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);	\
-	regs[key ## KEYHI_EL1] = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);	\
-})
-
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
 	int *last_ran;
@@ -376,17 +370,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	else
 		vcpu_set_wfx_traps(vcpu);
 
-	if (vcpu_has_ptrauth(vcpu)) {
-		struct kvm_cpu_context *ctxt = vcpu->arch.host_cpu_context;
-
-		__ptrauth_save_key(ctxt->sys_regs, APIA);
-		__ptrauth_save_key(ctxt->sys_regs, APIB);
-		__ptrauth_save_key(ctxt->sys_regs, APDA);
-		__ptrauth_save_key(ctxt->sys_regs, APDB);
-		__ptrauth_save_key(ctxt->sys_regs, APGA);
-
+	if (vcpu_has_ptrauth(vcpu))
 		vcpu_ptrauth_disable(vcpu);
-	}
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 065251efa2e6..5a02d4c90559 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -162,25 +162,14 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	return 1;
 }
 
-/*
- * Handle the guest trying to use a ptrauth instruction, or trying to access a
- * ptrauth register.
- */
-void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu)
-{
-	if (vcpu_has_ptrauth(vcpu))
-		vcpu_ptrauth_enable(vcpu);
-	else
-		kvm_inject_undefined(vcpu);
-}
-
 /*
  * Guest usage of a ptrauth instruction (which the guest EL1 did not turn into
- * a NOP).
+ * a NOP). If we get here, it is that we didn't fixup ptrauth on exit, and all
+ * that we can do is give the guest an UNDEF.
  */
 static int kvm_handle_ptrauth(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	kvm_arm_vcpu_ptrauth_trap(vcpu);
+	kvm_inject_undefined(vcpu);
 	return 1;
 }
 
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index c07a45643cd4..2a50b3771c3b 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -490,6 +490,62 @@ static bool __hyp_text handle_tx2_tvm(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+#define __ptrauth_save_key(regs, key)						\
+({										\
+	regs[key ## KEYLO_EL1] = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);	\
+	regs[key ## KEYHI_EL1] = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);	\
+})
+
+static bool __hyp_text __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
+{
+	u32 sysreg = esr_sys64_to_sysreg(kvm_vcpu_get_hsr(vcpu));
+	u32 ec = kvm_vcpu_trap_get_class(vcpu);
+	struct kvm_cpu_context *ctxt;
+	u64 val;
+
+	if (!vcpu_has_ptrauth(vcpu))
+		return false;
+
+	switch (ec) {
+	case ESR_ELx_EC_PAC:
+		break;
+	case ESR_ELx_EC_SYS64:
+		switch (sysreg) {
+		case SYS_APIAKEYLO_EL1:
+		case SYS_APIAKEYHI_EL1:
+		case SYS_APIBKEYLO_EL1:
+		case SYS_APIBKEYHI_EL1:
+		case SYS_APDAKEYLO_EL1:
+		case SYS_APDAKEYHI_EL1:
+		case SYS_APDBKEYLO_EL1:
+		case SYS_APDBKEYHI_EL1:
+		case SYS_APGAKEYLO_EL1:
+		case SYS_APGAKEYHI_EL1:
+			break;
+		default:
+			return false;
+		}
+		break;
+	default:
+		return false;
+	}
+
+	ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+	__ptrauth_save_key(ctxt->sys_regs, APIA);
+	__ptrauth_save_key(ctxt->sys_regs, APIB);
+	__ptrauth_save_key(ctxt->sys_regs, APDA);
+	__ptrauth_save_key(ctxt->sys_regs, APDB);
+	__ptrauth_save_key(ctxt->sys_regs, APGA);
+
+	vcpu_ptrauth_enable(vcpu);
+
+	val = read_sysreg(hcr_el2);
+	val |= (HCR_API | HCR_APK);
+	write_sysreg(val, hcr_el2);
+
+	return true;
+}
+
 /*
  * Return true when we were able to fixup the guest exit and should return to
  * the guest, false when we should restore the host state and return to the
@@ -524,6 +580,9 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 	if (__hyp_handle_fpsimd(vcpu))
 		return true;
 
+	if (__hyp_handle_ptrauth(vcpu))
+		return true;
+
 	if (!__populate_fault_info(vcpu))
 		return true;
 
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index ad1d57501d6d..564995084cf8 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1034,16 +1034,13 @@ static bool trap_ptrauth(struct kvm_vcpu *vcpu,
 			 struct sys_reg_params *p,
 			 const struct sys_reg_desc *rd)
 {
-	kvm_arm_vcpu_ptrauth_trap(vcpu);
-
 	/*
-	 * Return false for both cases as we never skip the trapped
-	 * instruction:
-	 *
-	 * - Either we re-execute the same key register access instruction
-	 *   after enabling ptrauth.
-	 * - Or an UNDEF is injected as ptrauth is not supported/enabled.
+	 * If we land here, that is because we didn't fixup the access on exit
+	 * by allowing the PtrAuth sysregs. The only way this happens is when
+	 * the guest does not have PtrAuth support enabled.
 	 */
+	kvm_inject_undefined(vcpu);
+
 	return false;
 }
 
-- 
2.26.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 1/3] KVM: arm64: Save the host's PtrAuth keys in non-preemptible context
From: Marc Zyngier @ 2020-06-04 13:33 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel
  Cc: Mark Rutland, kernel-team, Suzuki K Poulose, Catalin Marinas,
	stable, James Morse, Will Deacon, Julien Thierry
In-Reply-To: <20200604133354.1279412-1-maz@kernel.org>

When using the PtrAuth feature in a guest, we need to save the host's
keys before allowing the guest to program them. For that, we dump
them in a per-CPU data structure (the so called host context).

But both call sites that do this are in preemptible context,
which may end up in disaster should the vcpu thread get preempted
before reentering the guest.

Instead, save the keys eagerly on each vcpu_load(). This has an
increased overhead, but is at least safe.

Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_emulate.h |  6 ------
 arch/arm64/kvm/arm.c                 | 18 +++++++++++++++++-
 arch/arm64/kvm/handle_exit.c         | 19 ++-----------------
 3 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index a30b4eec7cb4..977843e4d5fb 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -112,12 +112,6 @@ static inline void vcpu_ptrauth_disable(struct kvm_vcpu *vcpu)
 	vcpu->arch.hcr_el2 &= ~(HCR_API | HCR_APK);
 }
 
-static inline void vcpu_ptrauth_setup_lazy(struct kvm_vcpu *vcpu)
-{
-	if (vcpu_has_ptrauth(vcpu))
-		vcpu_ptrauth_disable(vcpu);
-}
-
 static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
 {
 	return vcpu->arch.vsesr_el2;
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index d6988401c22a..152049c5055d 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -337,6 +337,12 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
 	preempt_enable();
 }
 
+#define __ptrauth_save_key(regs, key)						\
+({										\
+	regs[key ## KEYLO_EL1] = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);	\
+	regs[key ## KEYHI_EL1] = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);	\
+})
+
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
 	int *last_ran;
@@ -370,7 +376,17 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	else
 		vcpu_set_wfx_traps(vcpu);
 
-	vcpu_ptrauth_setup_lazy(vcpu);
+	if (vcpu_has_ptrauth(vcpu)) {
+		struct kvm_cpu_context *ctxt = vcpu->arch.host_cpu_context;
+
+		__ptrauth_save_key(ctxt->sys_regs, APIA);
+		__ptrauth_save_key(ctxt->sys_regs, APIB);
+		__ptrauth_save_key(ctxt->sys_regs, APDA);
+		__ptrauth_save_key(ctxt->sys_regs, APDB);
+		__ptrauth_save_key(ctxt->sys_regs, APGA);
+
+		vcpu_ptrauth_disable(vcpu);
+	}
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index eb194696ef62..065251efa2e6 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -162,31 +162,16 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	return 1;
 }
 
-#define __ptrauth_save_key(regs, key)						\
-({										\
-	regs[key ## KEYLO_EL1] = read_sysreg_s(SYS_ ## key ## KEYLO_EL1);	\
-	regs[key ## KEYHI_EL1] = read_sysreg_s(SYS_ ## key ## KEYHI_EL1);	\
-})
-
 /*
  * Handle the guest trying to use a ptrauth instruction, or trying to access a
  * ptrauth register.
  */
 void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu)
 {
-	struct kvm_cpu_context *ctxt;
-
-	if (vcpu_has_ptrauth(vcpu)) {
+	if (vcpu_has_ptrauth(vcpu))
 		vcpu_ptrauth_enable(vcpu);
-		ctxt = vcpu->arch.host_cpu_context;
-		__ptrauth_save_key(ctxt->sys_regs, APIA);
-		__ptrauth_save_key(ctxt->sys_regs, APIB);
-		__ptrauth_save_key(ctxt->sys_regs, APDA);
-		__ptrauth_save_key(ctxt->sys_regs, APDB);
-		__ptrauth_save_key(ctxt->sys_regs, APGA);
-	} else {
+	else
 		kvm_inject_undefined(vcpu);
-	}
 }
 
 /*
-- 
2.26.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 0/3] kvm: arm64: Pointer Authentication handling fixes
From: Marc Zyngier @ 2020-06-04 13:33 UTC (permalink / raw)
  To: kvm, kvmarm, linux-arm-kernel
  Cc: Mark Rutland, kernel-team, Suzuki K Poulose, Catalin Marinas,
	James Morse, Will Deacon, Julien Thierry

I recently discovered that the Pointer Authentication (PtrAuth)
handling code in KVM is busted, and has been for a while. The main
issue is that the we save the host's keys from a preemptible
context. Things will go wrong at some point.

In order to address this, the first patch move the saving of the
host's keys to vcpu_load(). It is done eagerly, which is a bore, but
is at least safe. This is definitely stable material.

The following two patches are adding an optimisation and a fix for a
corner case: we handle key saving and HCR massaging as a fixup, much
like the FPSIMD code. This subsequently allows us to deal with the
ugly case of a guest enabling PtrAuth despite it not being advertised,
resulting in PAC instructions UNDEF'ing while they should be NOPs.

This has been very lightly tested on a model.

Marc Zyngier (3):
  KVM: arm64: Save the host's PtrAuth keys in non-preemptible context
  KVM: arm64: Handle PtrAuth traps early
  KVM: arm64: Enforce PtrAuth being disabled if not advertized

 arch/arm64/include/asm/kvm_emulate.h |  6 ---
 arch/arm64/kvm/arm.c                 |  3 +-
 arch/arm64/kvm/handle_exit.c         | 38 ---------------
 arch/arm64/kvm/hyp/switch.c          | 73 ++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c            | 13 ++---
 5 files changed, 80 insertions(+), 53 deletions(-)

-- 
2.26.2


_______________________________________________
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 0/2] Introduce PCI_FIXUP_IOMMU
From: Zhangfei Gao @ 2020-06-04 13:33 UTC (permalink / raw)
  To: Bjorn Helgaas, Joerg Roedel
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
	linux-pci, Greg Kroah-Hartman, Hanjun Guo, Rafael J. Wysocki,
	linux-kernel, iommu, linux-acpi, Wangzhou, linux-crypto,
	Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012, linux-arm-kernel,
	Len Brown
In-Reply-To: <20200601174104.GA734973@bjorn-Precision-5520>



On 2020/6/2 上午1:41, Bjorn Helgaas wrote:
> On Thu, May 28, 2020 at 09:33:44AM +0200, Joerg Roedel wrote:
>> On Wed, May 27, 2020 at 01:18:42PM -0500, Bjorn Helgaas wrote:
>>> Is this slowdown significant?  We already iterate over every device
>>> when applying PCI_FIXUP_FINAL quirks, so if we used the existing
>>> PCI_FIXUP_FINAL, we wouldn't be adding a new loop.  We would only be
>>> adding two more iterations to the loop in pci_do_fixups() that tries
>>> to match quirks against the current device.  I doubt that would be a
>>> measurable slowdown.
>> I don't know how significant it is, but I remember people complaining
>> about adding new PCI quirks because it takes too long for them to run
>> them all. That was in the discussion about the quirk disabling ATS on
>> AMD Stoney systems.
>>
>> So it probably depends on how many PCI devices are in the system whether
>> it causes any measureable slowdown.
> I found this [1] from Paul Menzel, which was a slowdown caused by
> quirk_usb_early_handoff().  I think the real problem is individual
> quirks that take a long time.
>
> The PCI_FIXUP_IOMMU things we're talking about should be fast, and of
> course, they're only run for matching devices anyway.  So I'd rather
> keep them as PCI_FIXUP_FINAL than add a whole new phase.
>
Thanks Bjorn for taking time for this.
If so, it would be much simpler.

+++ b/drivers/iommu/iommu.c
@@ -2418,6 +2418,10 @@ int iommu_fwspec_init(struct device *dev, struct 
fwnode_handle *iommu_fwnode,
         fwspec->iommu_fwnode = iommu_fwnode;
         fwspec->ops = ops;
         dev_iommu_fwspec_set(dev, fwspec);
+
+       if (dev_is_pci(dev))
+               pci_fixup_device(pci_fixup_final, to_pci_dev(dev));
+

Then pci_fixup_final will be called twice, the first in pci_bus_add_device.
Here in iommu_fwspec_init is the second time, specifically for iommu_fwspec.
Will send this when 5.8-rc1 is open.

Thanks

_______________________________________________
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] arm: dts: vexpress: Move mcc node back into motherboard node
From: Sudeep Holla @ 2020-06-04 13:13 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Rob Herring, Lorenzo Pieralisi, devicetree, Liviu Dudau,
	linux-arm-kernel, Sudeep Holla, Guenter Roeck
In-Reply-To: <159126075826.16785.4183160239670270692.b4-ty@arm.com>

On Thu, Jun 04, 2020 at 09:56:31AM +0100, Sudeep Holla wrote:
> On Wed, 3 Jun 2020 17:22:37 +0100, Andre Przywara wrote:
> > Commit 	d9258898ad49 ("arm64: dts: arm: vexpress: Move fixed devices
> > out of bus node") moved the "mcc" DT node into the root node, because
> > it does not have any children using "reg" properties, so does violate
> > some dtc checks about "simple-bus" nodes.
> > However this broke the vexpress config-bus code, which walks up the
> > device tree to find the first node with an "arm,vexpress,site" property.
> > This gave the wrong result (matching the root node instead of the
> > motherboard node), so broke the clocks and some other devices for
> > VExpress boards.
> > 
> > [...]
> 
> Applied to sudeep.holla/linux (for-next/juno), thanks!
> 
> [1/1] arm: dts: vexpress: Move mcc node back into motherboard node
>       https://git.kernel.org/sudeep.holla/c/8a8cd9a910

Had to fix the 'Fixes' tag based on the report from linux-next, so updated to:

https://git.kernel.org/sudeep.holla/c/38ac46002d1d

--
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

* [PATCH] ARM: socfpga: add missing put_device() call in socfpga_setup_ocram_self_refresh()
From: yu kuai @ 2020-06-04 13:10 UTC (permalink / raw)
  To: dinguyen, linux, khilman, atull; +Cc: yukuai3, linux-kernel, linux-arm-kernel

if of_find_device_by_node() succeed, socfpga_setup_ocram_self_refresh()
doesn't have a corresponding put_device(). Thus add a jump target to fix
the exception handling for this function implementation.

Fixes: 44fd8c7d4005 ("ARM: socfpga: support suspend to ram")
Signed-off-by: yu kuai <yukuai3@huawei.com>
---
 arch/arm/mach-socfpga/pm.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-socfpga/pm.c b/arch/arm/mach-socfpga/pm.c
index 6ed887cf8dc9..7267f5da15a4 100644
--- a/arch/arm/mach-socfpga/pm.c
+++ b/arch/arm/mach-socfpga/pm.c
@@ -49,14 +49,14 @@ static int socfpga_setup_ocram_self_refresh(void)
 	if (!ocram_pool) {
 		pr_warn("%s: ocram pool unavailable!\n", __func__);
 		ret = -ENODEV;
-		goto put_node;
+		goto put_device;
 	}
 
 	ocram_base = gen_pool_alloc(ocram_pool, socfpga_sdram_self_refresh_sz);
 	if (!ocram_base) {
 		pr_warn("%s: unable to alloc ocram!\n", __func__);
 		ret = -ENOMEM;
-		goto put_node;
+		goto put_device;
 	}
 
 	ocram_pbase = gen_pool_virt_to_phys(ocram_pool, ocram_base);
@@ -67,7 +67,7 @@ static int socfpga_setup_ocram_self_refresh(void)
 	if (!suspend_ocram_base) {
 		pr_warn("%s: __arm_ioremap_exec failed!\n", __func__);
 		ret = -ENOMEM;
-		goto put_node;
+		goto put_device;
 	}
 
 	/* Copy the code that puts DDR in self refresh to ocram */
@@ -80,7 +80,12 @@ static int socfpga_setup_ocram_self_refresh(void)
 	     "could not copy function to ocram");
 	if (!socfpga_sdram_self_refresh_in_ocram)
 		ret = -EFAULT;
+
+	if (!ret)
+		goto put_node;
 
+put_device:
+	put_device(&pdev->dev);
 put_node:
 	of_node_put(np);
 
-- 
2.25.4


_______________________________________________
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] ARM: imx6: add missing put_device() call in imx6q_suspend_init()
From: yu kuai @ 2020-06-04 12:54 UTC (permalink / raw)
  To: linux, shawnguo, s.hauer, kernel, festevam, linux-imx,
	Anson.Huang
  Cc: yukuai3, linux-kernel, linux-arm-kernel, yi.zhang

if of_find_device_by_node() succeed, imx6q_suspend_init() doesn't have a
corresponding put_device(). Thus add a jump target to fix the exception
handling for this function implementation.

Signed-off-by: yu kuai <yukuai3@huawei.com>
---
 arch/arm/mach-imx/pm-imx6.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index dd34dff13762..40c74b4c4d73 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -493,14 +493,14 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
 	if (!ocram_pool) {
 		pr_warn("%s: ocram pool unavailable!\n", __func__);
 		ret = -ENODEV;
-		goto put_node;
+		goto put_device;
 	}
 
 	ocram_base = gen_pool_alloc(ocram_pool, MX6Q_SUSPEND_OCRAM_SIZE);
 	if (!ocram_base) {
 		pr_warn("%s: unable to alloc ocram!\n", __func__);
 		ret = -ENOMEM;
-		goto put_node;
+		goto put_device;
 	}
 
 	ocram_pbase = gen_pool_virt_to_phys(ocram_pool, ocram_base);
@@ -523,7 +523,7 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
 	ret = imx6_pm_get_base(&pm_info->mmdc_base, socdata->mmdc_compat);
 	if (ret) {
 		pr_warn("%s: failed to get mmdc base %d!\n", __func__, ret);
-		goto put_node;
+		goto put_device;
 	}
 
 	ret = imx6_pm_get_base(&pm_info->src_base, socdata->src_compat);
@@ -570,7 +570,7 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
 		&imx6_suspend,
 		MX6Q_SUSPEND_OCRAM_SIZE - sizeof(*pm_info));
 
-	goto put_node;
+	goto put_device;
 
 pl310_cache_map_failed:
 	iounmap(pm_info->gpc_base.vbase);
@@ -580,6 +580,8 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
 	iounmap(pm_info->src_base.vbase);
 src_map_failed:
 	iounmap(pm_info->mmdc_base.vbase);
+put_device:
+	put_device(&pdev->dev);
 put_node:
 	of_node_put(node);
 
-- 
2.25.4


_______________________________________________
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] ARM: imx5: add missing put_device() call in imx_suspend_alloc_ocram()
From: yu kuai @ 2020-06-04 12:42 UTC (permalink / raw)
  To: shawnguo, s.hauer, kernel, festevam, linux-imx, mfuzzey
  Cc: yukuai3, linux-kernel, linux-arm-kernel, yi.zhang

if of_find_device_by_node() succeed, imx_suspend_alloc_ocram() doesn't
have a corresponding put_device(). Thus add a jump target to fix the
exception handling for this function implementation.

Fixes: 1579c7b9fe01 ("ARM: imx53: Set DDR pins to high impedance when in suspend to RAM.")
Signed-off-by: yu kuai <yukuai3@huawei.com>
---
 arch/arm/mach-imx/pm-imx5.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/pm-imx5.c b/arch/arm/mach-imx/pm-imx5.c
index f057df813f83..e9962b48e30c 100644
--- a/arch/arm/mach-imx/pm-imx5.c
+++ b/arch/arm/mach-imx/pm-imx5.c
@@ -295,14 +295,14 @@ static int __init imx_suspend_alloc_ocram(
 	if (!ocram_pool) {
 		pr_warn("%s: ocram pool unavailable!\n", __func__);
 		ret = -ENODEV;
-		goto put_node;
+		goto put_device;
 	}
 
 	ocram_base = gen_pool_alloc(ocram_pool, size);
 	if (!ocram_base) {
 		pr_warn("%s: unable to alloc ocram!\n", __func__);
 		ret = -ENOMEM;
-		goto put_node;
+		goto put_device;
 	}
 
 	phys = gen_pool_virt_to_phys(ocram_pool, ocram_base);
@@ -312,6 +312,8 @@ static int __init imx_suspend_alloc_ocram(
 	if (virt_out)
 		*virt_out = virt;
 
+put_device:
+	put_device(&pdev->dev);
 put_node:
 	of_node_put(node);
 
-- 
2.25.4


_______________________________________________
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 v3 2/3] media: stm32-dcmi: Set minimum cpufreq requirement
From: Benjamin Gaignard @ 2020-06-04 12:39 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue
  Cc: Benjamin Gaignard, rjw, linux-kernel, linux-stm32,
	valentin.schneider, linux-arm-kernel, linux-media
In-Reply-To: <20200604123932.20512-1-benjamin.gaignard@st.com>

Before start streaming set cpufreq minimum frequency requirement.
The cpufreq governor will adapt the frequencies and we will have
no latency for handling interrupts.
The frequency requirement is retrieved from the device-tree node.

While streaming be notified if the IRQ affinity change thanks to 
irq_affinity_notify callback.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 3:
- add a cpumask field to track boosted CPUs
- add irq_affinity_notify callback
- protect cpumask field with a mutex 

 drivers/media/platform/stm32/stm32-dcmi.c | 187 ++++++++++++++++++++++++++++--
 1 file changed, 179 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index b8931490b83b..fb6ab09eaff0 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -13,10 +13,13 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/cpufreq.h>
+#include <linux/cpumask.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -99,6 +102,8 @@ enum state {
 
 #define OVERRUN_ERROR_THRESHOLD	3
 
+static DEFINE_PER_CPU(struct freq_qos_request, qos_req);
+
 struct dcmi_graph_entity {
 	struct v4l2_async_subdev asd;
 
@@ -133,6 +138,7 @@ struct stm32_dcmi {
 	struct resource			*res;
 	struct reset_control		*rstc;
 	int				sequence;
+	int				irq;
 	struct list_head		buffers;
 	struct dcmi_buf			*active;
 
@@ -173,6 +179,11 @@ struct stm32_dcmi {
 	struct media_device		mdev;
 	struct media_pad		vid_cap_pad;
 	struct media_pipeline		pipeline;
+
+	u32				min_frequency;
+	cpumask_var_t			boosted;
+	struct irq_affinity_notify	notify;
+	struct mutex			freq_lock;
 };
 
 static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
@@ -722,6 +733,156 @@ static void dcmi_pipeline_stop(struct stm32_dcmi *dcmi)
 	dcmi_pipeline_s_stream(dcmi, 0);
 }
 
+static void dcmi_get_min_frequency(struct stm32_dcmi *dcmi)
+{
+	struct device_node *np = dcmi->mdev.dev->of_node;
+
+	dcmi->min_frequency = FREQ_QOS_MIN_DEFAULT_VALUE;
+
+	of_property_read_u32(np, "st,stm32-dcmi-min-frequency",
+			     &dcmi->min_frequency);
+}
+
+static void dcmi_irq_notifier_notify(struct irq_affinity_notify *notify,
+				     const cpumask_t *mask)
+{
+	struct stm32_dcmi *dcmi = container_of(notify,
+					       struct stm32_dcmi,
+					       notify);
+	struct cpufreq_policy *p;
+	int cpu;
+
+	mutex_lock(&dcmi->freq_lock);
+	/*
+	 * For all boosted CPUs check if it is still the case
+	 * if not remove the request
+	 */
+	for_each_cpu(cpu, dcmi->boosted) {
+		if (cpumask_test_cpu(cpu, mask))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_remove_request(&per_cpu(qos_req, cpu));
+		cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
+
+		cpufreq_cpu_put(p);
+	}
+
+	/*
+	 * For CPUs in the mask check if they are boosted if not add
+	 * a request
+	 */
+	for_each_cpu(cpu, mask) {
+		if (cpumask_test_cpu(cpu, dcmi->boosted))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
+				     FREQ_QOS_MIN, dcmi->min_frequency);
+		cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
+		cpufreq_cpu_put(p);
+	}
+
+	mutex_unlock(&dcmi->freq_lock);
+}
+
+static void dcmi_irq_notifier_release(struct kref *ref)
+{
+	/*
+	 * This is required by affinity notifier. We don't have anything to
+	 * free here.
+	 */
+}
+
+static void dcmi_get_cpu_policy(struct stm32_dcmi *dcmi)
+{
+	struct cpufreq_policy *p;
+	int cpu;
+
+	if (!alloc_cpumask_var(&dcmi->boosted, GFP_KERNEL))
+		return;
+
+	mutex_lock(&dcmi->freq_lock);
+
+	for_each_cpu(cpu, irq_get_affinity_mask(dcmi->irq)) {
+		if (cpumask_test_cpu(cpu, dcmi->boosted))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_add_request(&p->constraints, &per_cpu(qos_req, cpu),
+				     FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
+
+		cpumask_or(dcmi->boosted, dcmi->boosted, p->cpus);
+
+		cpufreq_cpu_put(p);
+	}
+
+	mutex_unlock(&dcmi->freq_lock);
+}
+
+static void dcmi_put_cpu_policy(struct stm32_dcmi *dcmi)
+{
+	struct cpufreq_policy *p;
+	int cpu;
+
+	mutex_lock(&dcmi->freq_lock);
+
+	for_each_cpu(cpu, irq_get_affinity_mask(dcmi->irq)) {
+		if (!cpumask_test_cpu(cpu, dcmi->boosted))
+			continue;
+
+		p = cpufreq_cpu_get(cpu);
+		if (!p)
+			continue;
+
+		freq_qos_remove_request(&per_cpu(qos_req, cpu));
+		cpumask_andnot(dcmi->boosted, dcmi->boosted, p->cpus);
+
+		cpufreq_cpu_put(p);
+	}
+
+	free_cpumask_var(dcmi->boosted);
+
+	mutex_unlock(&dcmi->freq_lock);
+}
+
+static void dcmi_set_min_frequency(struct stm32_dcmi *dcmi, s32 freq)
+{
+	struct irq_affinity_notify *notify = &dcmi->notify;
+	int cpu;
+
+	mutex_lock(&dcmi->freq_lock);
+
+	for_each_cpu(cpu, irq_get_affinity_mask(dcmi->irq)) {
+		if (!cpumask_test_cpu(cpu, dcmi->boosted))
+			continue;
+
+		if (!freq_qos_request_active(&per_cpu(qos_req, cpu)))
+			continue;
+
+		freq_qos_update_request(&per_cpu(qos_req, cpu), freq);
+	}
+
+	mutex_unlock(&dcmi->freq_lock);
+
+	if (freq != FREQ_QOS_MIN_DEFAULT_VALUE) {
+		notify->notify = dcmi_irq_notifier_notify;
+		notify->release = dcmi_irq_notifier_release;
+		irq_set_affinity_notifier(dcmi->irq, notify);
+	} else {
+		irq_set_affinity_notifier(dcmi->irq, NULL);
+	}
+}
+
 static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
 	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
@@ -736,11 +897,13 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 		goto err_release_buffers;
 	}
 
+	dcmi_set_min_frequency(dcmi, dcmi->min_frequency);
+
 	ret = media_pipeline_start(&dcmi->vdev->entity, &dcmi->pipeline);
 	if (ret < 0) {
 		dev_err(dcmi->dev, "%s: Failed to start streaming, media pipeline start error (%d)\n",
 			__func__, ret);
-		goto err_pm_put;
+		goto err_drop_qos;
 	}
 
 	ret = dcmi_pipeline_start(dcmi);
@@ -835,7 +998,8 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
 err_media_pipeline_stop:
 	media_pipeline_stop(&dcmi->vdev->entity);
 
-err_pm_put:
+err_drop_qos:
+	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
 	pm_runtime_put(dcmi->dev);
 
 err_release_buffers:
@@ -863,6 +1027,8 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
 
 	media_pipeline_stop(&dcmi->vdev->entity);
 
+	dcmi_set_min_frequency(dcmi, FREQ_QOS_MIN_DEFAULT_VALUE);
+
 	spin_lock_irq(&dcmi->irqlock);
 
 	/* Disable interruptions */
@@ -1838,7 +2004,6 @@ static int dcmi_probe(struct platform_device *pdev)
 	struct vb2_queue *q;
 	struct dma_chan *chan;
 	struct clk *mclk;
-	int irq;
 	int ret = 0;
 
 	match = of_match_device(of_match_ptr(stm32_dcmi_of_match), &pdev->dev);
@@ -1879,9 +2044,9 @@ static int dcmi_probe(struct platform_device *pdev)
 	dcmi->bus.bus_width = ep.bus.parallel.bus_width;
 	dcmi->bus.data_shift = ep.bus.parallel.data_shift;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return irq ? irq : -ENXIO;
+	dcmi->irq = platform_get_irq(pdev, 0);
+	if (dcmi->irq <= 0)
+		return dcmi->irq ? dcmi->irq : -ENXIO;
 
 	dcmi->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!dcmi->res) {
@@ -1895,11 +2060,12 @@ static int dcmi_probe(struct platform_device *pdev)
 		return PTR_ERR(dcmi->regs);
 	}
 
-	ret = devm_request_threaded_irq(&pdev->dev, irq, dcmi_irq_callback,
+	ret = devm_request_threaded_irq(&pdev->dev, dcmi->irq,
+					dcmi_irq_callback,
 					dcmi_irq_thread, IRQF_ONESHOT,
 					dev_name(&pdev->dev), dcmi);
 	if (ret) {
-		dev_err(&pdev->dev, "Unable to request irq %d\n", irq);
+		dev_err(&pdev->dev, "Unable to request irq %d\n", dcmi->irq);
 		return ret;
 	}
 
@@ -2022,6 +2188,9 @@ static int dcmi_probe(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "Probe done\n");
 
+	dcmi_get_min_frequency(dcmi);
+	dcmi_get_cpu_policy(dcmi);
+
 	platform_set_drvdata(pdev, dcmi);
 
 	pm_runtime_enable(&pdev->dev);
@@ -2049,6 +2218,8 @@ static int dcmi_remove(struct platform_device *pdev)
 
 	pm_runtime_disable(&pdev->dev);
 
+	dcmi_put_cpu_policy(dcmi);
+
 	v4l2_async_notifier_unregister(&dcmi->notifier);
 	v4l2_async_notifier_cleanup(&dcmi->notifier);
 	media_entity_cleanup(&dcmi->vdev->entity);
-- 
2.15.0


_______________________________________________
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 v3 3/3] ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x
From: Benjamin Gaignard @ 2020-06-04 12:39 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue
  Cc: Benjamin Gaignard, rjw, linux-kernel, linux-stm32,
	valentin.schneider, linux-arm-kernel, linux-media
In-Reply-To: <20200604123932.20512-1-benjamin.gaignard@st.com>

Make sure that CPUs will at least run at 650Mhz when streaming
sensor frames.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 arch/arm/boot/dts/stm32mp151.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
index 3ea05ba48215..f6d7bf4f8231 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1091,6 +1091,7 @@
 			clock-names = "mclk";
 			dmas = <&dmamux1 75 0x400 0x0d>;
 			dma-names = "tx";
+			st,stm32-dcmi-min-frequency = <650000>;
 			status = "disabled";
 		};
 
-- 
2.15.0


_______________________________________________
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 v3 0/3] DCMI set minimum cpufreq requirement
From: Benjamin Gaignard @ 2020-06-04 12:39 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue
  Cc: Benjamin Gaignard, rjw, linux-kernel, linux-stm32,
	valentin.schneider, linux-arm-kernel, linux-media

This series allow to STM32 camera interface (DCMI) to require a minimum
frequency to the CPUs before start streaming frames from the sensor.
The minimum frequency requirement is provided in the devide-tree node.

Setting a minimum frequency for the CPUs is needed to ensure a quick handling
of the interrupts between two sensor frames and avoid dropping half of them.

version 3:
- add a cpumask field to track boosted CPUs
- add irq_affinity_notify callback
- protect cpumask field with a mutex 

Benjamin Gaignard (3):
  dt-bindings: media: stm32-dcmi: Add DCMI min frequency property
  media: stm32-dcmi: Set minimum cpufreq requirement
  ARM: dts: stm32: Set DCMI frequency requirement for stm32mp15x

 .../devicetree/bindings/media/st,stm32-dcmi.yaml   |   8 +
 arch/arm/boot/dts/stm32mp151.dtsi                  |   1 +
 drivers/media/platform/stm32/stm32-dcmi.c          | 187 ++++++++++++++++++++-
 3 files changed, 188 insertions(+), 8 deletions(-)

-- 
2.15.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v3 1/3] dt-bindings: media: stm32-dcmi: Add DCMI min frequency property
From: Benjamin Gaignard @ 2020-06-04 12:39 UTC (permalink / raw)
  To: hugues.fruchet, mchehab, mcoquelin.stm32, alexandre.torgue
  Cc: Benjamin Gaignard, rjw, linux-kernel, linux-stm32,
	valentin.schneider, linux-arm-kernel, linux-media
In-Reply-To: <20200604123932.20512-1-benjamin.gaignard@st.com>

Document st,stm32-dcmi-min-frequency property which is used to
request CPUs minimum frequency when streaming frames.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
index 3fe778cb5cc3..05ca85a2411a 100644
--- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
+++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml
@@ -44,6 +44,13 @@ properties:
       bindings defined in
       Documentation/devicetree/bindings/media/video-interfaces.txt.
 
+  st,stm32-dcmi-min-frequency:
+    description: DCMI minimum CPUs frequency requirement (in KHz).
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - minimum: 0
+      - default: 0
+
 required:
   - compatible
   - reg
@@ -71,6 +78,7 @@ examples:
         clock-names = "mclk";
         dmas = <&dmamux1 75 0x400 0x0d>;
         dma-names = "tx";
+        st,stm32-dcmi-min-frequency = <650000>;
 
         port {
              dcmi_0: endpoint {
-- 
2.15.0


_______________________________________________
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: [PATCH V2 2/3] firmware: imx: add resource management api
From: Peng Fan @ 2020-06-04 12:37 UTC (permalink / raw)
  To: Aisheng Dong, shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: festevam@gmail.com, Joakim Zhang, linux@rempel-privat.de,
	linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
	Leonard Crestez, Daniel Baluta, linux-kernel@vger.kernel.org,
	dl-linux-imx
In-Reply-To: <AM6PR04MB4966B2E779E2B7718A1D99A580890@AM6PR04MB4966.eurprd04.prod.outlook.com>


> Subject: RE: [PATCH V2 2/3] firmware: imx: add resource management api
> 
[...]

> > +/*
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Copyright 2017-2020 NXP
> > + *
> > + * Header file containing the public API for the System Controller
> > +(SC)
> > + * Power Management (PM) function. This includes functions for power
> > +state
> > + * control, clock control, reset control, and wake-up event control.
> 
> Fix the code comments.

Oops, forgot this comment in v1. Will fix in v3.

Thanks,
Peng.

> Otherwise:
> Dong Aisheng <aisheng.dong@nxp.com>
> 
> Regards
> Aisheng
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] ARM: at91: pm: add missing put_device() call in at91_pm_sram_init()
From: yu kuai @ 2020-06-04 12:33 UTC (permalink / raw)
  To: linux, nicolas.ferre, alexandre.belloni, ludovic.desroches
  Cc: yukuai3, linux-kernel, linux-arm-kernel, yi.zhang

if of_find_device_by_node() succeed, at91_pm_sram_init() doesn't have
a corresponding put_device(). Thus add a jump target to fix the exception
handling for this function implementation.

Fixes: d2e467905596 ("ARM: at91: pm: use the mmio-sram pool to access SRAM")
Signed-off-by: yu kuai <yukuai3@huawei.com>
---
 arch/arm/mach-at91/pm.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 074bde64064e..2aab043441e8 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -592,13 +592,13 @@ static void __init at91_pm_sram_init(void)
 	sram_pool = gen_pool_get(&pdev->dev, NULL);
 	if (!sram_pool) {
 		pr_warn("%s: sram pool unavailable!\n", __func__);
-		return;
+		goto out_put_device;
 	}
 
 	sram_base = gen_pool_alloc(sram_pool, at91_pm_suspend_in_sram_sz);
 	if (!sram_base) {
 		pr_warn("%s: unable to alloc sram!\n", __func__);
-		return;
+		goto out_put_device;
 	}
 
 	sram_pbase = gen_pool_virt_to_phys(sram_pool, sram_base);
@@ -606,12 +606,17 @@ static void __init at91_pm_sram_init(void)
 					at91_pm_suspend_in_sram_sz, false);
 	if (!at91_suspend_sram_fn) {
 		pr_warn("SRAM: Could not map\n");
-		return;
+		goto out_put_device;
 	}
 
 	/* Copy the pm suspend handler to SRAM */
 	at91_suspend_sram_fn = fncpy(at91_suspend_sram_fn,
 			&at91_pm_suspend_in_sram, at91_pm_suspend_in_sram_sz);
+	return;
+
+out_put_device:
+	put_device(&pdev->dev);
+	return;
 }
 
 static bool __init at91_is_pm_mode_active(int pm_mode)
-- 
2.25.4


_______________________________________________
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: [PATCH 3/3] spi: bcm2835: Enable shared interrupt support
From: Mark Brown @ 2020-06-04 12:32 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Scott Branden, lukas, Ray Jui, linux-kernel,
	open list:SPI SUBSYSTEM, Rob Herring,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Martin Sperl, Nicolas Saenz Julienne
In-Reply-To: <20200604034655.15930-4-f.fainelli@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1763 bytes --]

On Wed, Jun 03, 2020 at 08:46:55PM -0700, Florian Fainelli wrote:
> The SPI controller found in the BCM2711 and BCM7211 SoCs is instantiated
> 5 times, with all instances sharing the same interrupt line. We
> specifically match the two compatible strings here to determine whether
> it is necessary to request the interrupt with the IRQF_SHARED flag and
> to use an appropriate interrupt handler capable of returning IRQ_NONE.

> For the BCM2835 case which is deemed performance critical, there is no
> overhead since a dedicated handler that does not assume sharing is used.

This feels hacky - it's essentially using the compatible string to set a
boolean flag which isn't really about the IP but rather the platform
integration.  It might cause problems if we do end up having to quirk
this version of the IP for some other reason.  I'm also looking at the
code and wondering if the overhead of checking to see if the interrupt
is flagged is really that severe, it's just a check to see if a bit is
set in a register which we already read so should be a couple of
instructions (which disassembly seems to confirm).  It *is* overhead so
there's some value in it, I'm just surprised that it's such a hot path
especially with a reasonably deep FIFO like this device has.

I guess ideally genirq would provide a way to figure out if an interrupt
is actually shared in the present system, and better yet we'd have a way
for drivers to say they aren't using the interrupt ATM, but that might
be more effort than it's really worth.  If this is needed and there's no
better way of figuring out if the interrupt is really shared then I'd
suggest a boolean flag rather than a compatible string, it's still a
hack but it's less likely to store up trouble for the future.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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] pinctrl: pxa: pxa2xx: Remove 'pxa2xx_pinctrl_exit()' which is unused and broken
From: Dan Carpenter @ 2020-06-04 12:30 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Linus Walleij, kernel-janitors, linux-kernel@vger.kernel.org,
	Haojian Zhuang, open list:GPIO SUBSYSTEM, Christophe JAILLET,
	Linux ARM, Joe Perches, Robert Jarzmik, Daniel Mack
In-Reply-To: <alpine.DEB.2.21.2006041328570.2577@hadrien>

On Thu, Jun 04, 2020 at 01:42:12PM +0200, Julia Lawall wrote:
> OK, I recall a discussion with Dan where he suggested that some things
> that were not actually bug fixes could also merit a Fixes tag.  But it's
> probably better if he weighs in directly.

I generally think Fixes should only be used for "real bug" fixes.

The one exception is when I'm reviewing a patch that fixes an "unused
assignment" static checker warning is that I know which commit
introduced the warning.  I don't have strong feelings if it's in the
Fixes tag or if it's just mentioned in the commit message.  But it
helps me when I'm reviewing if I don't have to grovel through git
history myself.  Also I think that it's always a good exercise for
people fixing patches to look at how the bug was introduced.

regards,
dan carpenter

_______________________________________________
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 0/5 v2] KASan for ARM
From: Ard Biesheuvel @ 2020-06-04 12:10 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Rob Herring, Florian Fainelli, Abbott Liu, Russell King,
	Andrey Ryabinin, Linux ARM
In-Reply-To: <CAMj1kXF=SOan4i9T3U2guGPaNQxeZ-mUOp7fW7j7FtFCY2ZH1g@mail.gmail.com>

On Thu, 4 Jun 2020 at 13:26, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 4 Jun 2020 at 11:24, Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Wed, Jun 3, 2020 at 10:45 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Mon, Jun 1, 2020 at 6:37 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > >
> > > > This branch got me a bit further,
> > >
> > > Thanks, at least we get improvements. :)
> > >
> > > > but still failed to fully initialize
> > > > (see attached kasan.log), on another platform with a slightly different
> > > > memory map, I ended up getting a different error (kasan2.log).
> > >
> > > I have this error too on a Qualcomm board, it is what I report
> > > in the cover letter, that if I load the kernel into 0x40200000
> > > this happens but when I load it into 0x50000000 it does not
> > > happen.
> >
> > So this is what happens to me, even after I try to de-instrument
> > the DT parsing code (maybe I do it all wrong...)
> > This is done with that patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/commit/?h=kasan-apq8060-test&id=1cd83357f3c35b037400f6ec2547eeff074c578c
> >
> > If I boot from physical memory at 0x40200000
> > fastboot --base 40200000 --cmdline "console=ttyMSM0,115200,n8" boot zImage
> >
> > kasan: populating shadow for b7040000, b75c0000
> > kasan: populating shadow for b8000000, bb000000
> > kasan: populating shadow for b6e00000, b7000000
> > kasan: Kernel address sanitizer initialized
> > 8<--- cut here ---
> > Unable to handle kernel paging request at virtual address c30050b0
> > pgd = (ptrval)
> > [c30050b0] *pgd=00000000c
> > Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> > Modules linked in:c
> > CPU: 0 PID: 0 Comm: swapper Not tainted 5.7.0-00011-g1cd83357f3c3 #34
> > Hardware name: Generic DT based system
> > PC is at fdt_check_header+0x0/0x168
> > LR is at __unflatten_device_tree+0x6c/0x338
> > pc : [<c08e6968>]    lr : [<c0d698a8>]    psr: 60000093
> > sp : c1e03db8  ip : cffffee0  fp : fffff000
> > r10: 00000000  r9 : c2646000  r8 : 00000000
> > r7 : c30050b0  r6 : c192e9e4  r5 : c19492e8  r4 : c21d7448
> > r3 : 00000000  r2 : c2646000  r1 : 00000000  r0 : c30050b0
> > (...)
> > [<c08e6968>] (fdt_check_header) from [<c192e9e4>]
> > (early_init_dt_alloc_memory_arch+0x0/0x64)
> > [<c192e9e4>] (early_init_dt_alloc_memory_arch) from [<c1930264>]
> > (unflatten_device_tree+0x34/0x44)
> > [<c1930264>] (unflatten_device_tree) from [<c1905794>] (setup_arch+0xac4/0xde8)
> > [<c1905794>] (setup_arch) from [<c1900b98>] (start_kernel+0xd8/0x634)
> > [<c1900b98>] (start_kernel) from [<00000000>] (0x0)
> > Code: e3a00020 e12fff1e e3a0001c e12fff1e (e5901000)
> > random: get_random_bytes called from print_oops_end_marker+0x38/0x50
> > with crng_init=0
> > ---[ end trace 0000000000000000 ]---
> > Kernel panic - not syncing: Attempted to kill the idle task!
> > ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---
> >
>
> I don't think we ever check whether the ATAGS/DTB pointer points into
> memory that is described to the kernel as unreserved lowmem. We simply
> call phys_to_virt() on it [in setup_machine_fdt()], and assume that by
> the time we call unflatten_device_tree(), the same virtual address
> still points to the DT contents.
>
> On arm64, we have a special fixmap slot for the DT, so there this
> doesn't happen.
>
> Could you try whether the following makes the issues go away?
>
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index d8e18cdd96d3..b00867a026ed 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -1076,6 +1076,7 @@ void __init hyp_mode_check(void)
>  void __init setup_arch(char **cmdline_p)
>  {
>         const struct machine_desc *mdesc;
> +       void *fdt;
>
>         setup_processor();
>         mdesc = setup_machine_fdt(__atags_pointer);
> @@ -1135,6 +1136,8 @@ void __init setup_arch(char **cmdline_p)
>         if (mdesc->restart)
>                 arm_pm_restart = mdesc->restart;
>
> +       fdt = memremap(__atags_pointer, SZ_1M, MEMREMAP_WB);
> +       early_init_dt_verify(fdt);
>         unflatten_device_tree();
>
>         arm_dt_init_cpu_maps();
>
>
> (Not saying this is the right fix, just testing a hypothesis)
>

OK, so the bad news is that this doesn't help, but the good news is
that that means I have a reproducer :-)

_______________________________________________
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] arm64: Enable perf events based hard lockup detector
From: Sumit Garg @ 2020-06-04 12:09 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: mark.rutland, Sumit Garg, daniel.thompson, peterz,
	catalin.marinas, jolsa, alexandru.elisei, dianders, acme,
	alexander.shishkin, mingo, namhyung, tglx, will,
	julien.thierry.kdev

With the recent feature added to enable perf events to use pseudo NMIs
as interrupts on platforms which support GICv3 or later, its now been
possible to enable hard lockup detector (or NMI watchdog) on arm64
platforms. So enable corresponding support.

One thing to note here is that normally lockup detector is initialized
just after the early initcalls but PMU on arm64 comes up much later as
device_initcall(). So we need to re-initialize lockup detection once
PMU has been initialized.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---

This patch is dependent on perf NMI patch-set [1].

[1] https://patchwork.kernel.org/cover/11047407/

Changes since RFC:
- Rebased on top of Alex's WIP-pmu-nmi branch.
- Add comment for safe max. CPU frequency.
- Misc. cleanup.

 arch/arm64/Kconfig             |  2 ++
 arch/arm64/kernel/perf_event.c | 41 +++++++++++++++++++++++++++++++++++++++--
 drivers/perf/arm_pmu.c         |  9 +++++++++
 include/linux/perf/arm_pmu.h   |  2 ++
 4 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 40fb05d..36f75c2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -160,6 +160,8 @@ config ARM64
 	select HAVE_NMI
 	select HAVE_PATA_PLATFORM
 	select HAVE_PERF_EVENTS
+	select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI
+	select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_REGS_AND_STACK_ACCESS_API
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index e109aa5..a37f018 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -20,6 +20,8 @@
 #include <linux/perf/arm_pmu.h>
 #include <linux/platform_device.h>
 #include <linux/smp.h>
+#include <linux/nmi.h>
+#include <linux/cpufreq.h>
 
 /* ARMv8 Cortex-A53 specific event types. */
 #define ARMV8_A53_PERFCTR_PREF_LINEFILL				0xC2
@@ -1191,10 +1193,21 @@ static struct platform_driver armv8_pmu_driver = {
 
 static int __init armv8_pmu_driver_init(void)
 {
+	int ret;
+
 	if (acpi_disabled)
-		return platform_driver_register(&armv8_pmu_driver);
+		ret = platform_driver_register(&armv8_pmu_driver);
 	else
-		return arm_pmu_acpi_probe(armv8_pmuv3_init);
+		ret = arm_pmu_acpi_probe(armv8_pmuv3_init);
+
+	/*
+	 * Try to re-initialize lockup detector after PMU init in
+	 * case PMU events are triggered via NMIs.
+	 */
+	if (arm_pmu_irq_is_nmi())
+		lockup_detector_init();
+
+	return ret;
 }
 device_initcall(armv8_pmu_driver_init)
 
@@ -1226,3 +1239,27 @@ void arch_perf_update_userpage(struct perf_event *event,
 	userpg->time_shift = (u16)shift;
 	userpg->time_offset = -now;
 }
+
+#ifdef CONFIG_HARDLOCKUP_DETECTOR_PERF
+/*
+ * Safe maximum CPU frequency in case a particular platform doesn't implement
+ * cpufreq driver. Although, architecture doesn't put any restrictions on
+ * maximum frequency but 5 GHz seems to be safe maximum given the available
+ * Arm CPUs in the market which are clocked much less than 5 GHz. On the other
+ * hand, we can't make it much higher as it would lead to a large hard-lockup
+ * detection timeout on parts which are running slower (eg. 1GHz on
+ * Developerbox) and doesn't possess a cpufreq driver.
+ */
+#define SAFE_MAX_CPU_FREQ	5000000000UL // 5 GHz
+u64 hw_nmi_get_sample_period(int watchdog_thresh)
+{
+	unsigned int cpu = smp_processor_id();
+	unsigned int max_cpu_freq;
+
+	max_cpu_freq = cpufreq_get_hw_max_freq(cpu);
+	if (max_cpu_freq)
+		return (u64)max_cpu_freq * 1000 * watchdog_thresh;
+	else
+		return (u64)SAFE_MAX_CPU_FREQ * watchdog_thresh;
+}
+#endif
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index f96cfc4..6c25c0d1 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -718,6 +718,15 @@ static int armpmu_get_cpu_irq(struct arm_pmu *pmu, int cpu)
 	return per_cpu(hw_events->irq, cpu);
 }
 
+bool arm_pmu_irq_is_nmi(void)
+{
+	const struct pmu_irq_ops *irq_ops;
+
+	irq_ops = per_cpu(cpu_irq_ops, smp_processor_id());
+
+	return irq_ops == &pmunmi_ops || irq_ops == &percpu_pmunmi_ops;
+}
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index d9b8b76..a71f029 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -155,6 +155,8 @@ int arm_pmu_acpi_probe(armpmu_init_fn init_fn);
 static inline int arm_pmu_acpi_probe(armpmu_init_fn init_fn) { return 0; }
 #endif
 
+bool arm_pmu_irq_is_nmi(void);
+
 /* Internal functions only for core arm_pmu code */
 struct arm_pmu *armpmu_alloc(void);
 struct arm_pmu *armpmu_alloc_atomic(void);
-- 
2.7.4


_______________________________________________
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: [PATCH] leds: mt6360: Add LED driver for MT6360
From: Pavel Machek @ 2020-06-04 12:06 UTC (permalink / raw)
  To: Gene Chen
  Cc: gene_chen, linux-kernel, cy_huang, benjamin.chao, linux-mediatek,
	jacek.anaszewski, linux-leds, matthias.bgg, shufan_lee, Wilma.Wu,
	linux-arm-kernel, dmurphy
In-Reply-To: <1591252004-12681-1-git-send-email-gene.chen.richtek@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 601 bytes --]

Hi!

> Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode,
> and 3-channel RGB LED support Register/Flash/Breath Mode
> 
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> base-commit: 098c4adf249c198519a4abebe482b1e6b8c50e47

Does this need device tree binding, too?

> +++ b/drivers/leds/leds-mt6360.c
> @@ -0,0 +1,1061 @@
> +// SPDX-License-Identifier: GPL-2.0

Could we get GPL 2.0 or later?

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 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] pinctrl: pxa: pxa2xx: Remove 'pxa2xx_pinctrl_exit()' which is unused and broken
From: Julia Lawall @ 2020-06-04 11:42 UTC (permalink / raw)
  To: Joe Perches
  Cc: Linux ARM, Linus Walleij, kernel-janitors,
	linux-kernel@vger.kernel.org, Haojian Zhuang, Julia Lawall,
	open list:GPIO SUBSYSTEM, Christophe JAILLET, Dan Carpenter,
	Robert Jarzmik, Daniel Mack
In-Reply-To: <10e54ee84bd44171ef329bed9e7e6a946bae61ba.camel@perches.com>



On Thu, 4 Jun 2020, Joe Perches wrote:

> On Thu, 2020-06-04 at 12:33 +0200, Julia Lawall wrote:
> >
> > On Thu, 4 Jun 2020, Joe Perches wrote:
> >
> > > On Thu, 2020-06-04 at 11:52 +0200, Julia Lawall wrote:
> > > > Should Fixes also be used when the change will make it hard to port other
> > > > fixes over it?
> > >
> > > If it's a logic defect or regression that's being fixed,
> > > shouldn't the logic defect or regression be fixed as
> > > reasonably soon as possible?
> >
> > Sure, but I recall seeing some patches that mentioned that the problem had
> > existed since the beginning of git.  Of course, it should be rare.
>
> git history goes back 15 years already.
> There are scant few bugs that old.
>
> There is a tree with even older history that Rob Landley
> still has here: https://landley.net/kdocs/fullhist/
>
> It does make git blame research a bit easier for those
> rare and extremely old defects.
>
> > > The nature of the fix should ideally be optimal for
> > > backporting, but I believe that should not stop any
> > > consideration for the standalone fix itself.
> >
> > I'm not sure to follow this.
>
> I think it comes down to defects in current need to be
> fixed.  Describing
> the base commit that is being fixed
> is useful for backporting.
>
> I believe it's not reasonable to ask the author of a
> fix to research how it could or should be backported.
>
> > Sometimes non-bug fixes that block
> > backporting a bug fix have to be backported as well.  So the fixes would
> > again highlight the range of versions affected by the issue.
>
> Sure, but the non-bug fixes that may also need backporting
> to enable easy backports of the actual fix should not be
> described in the Fixes: <commit> as those are  generally
> easily researched from a command like:
>
> $ git log <commit>.. <files in fix>
>
> by whoever needs to backport.

OK, I recall a discussion with Dan where he suggested that some things
that were not actually bug fixes could also merit a Fixes tag.  But it's
probably better if he weighs in directly.

It would be unfortunate if someone doesn't submit a fix because they can't
figure out how to make the Fixes tag properly, though.

For example, when there is a lot of concurrency involved, some of the bugs
reported by syzkaller can be hard to fully understand.  In the case of a
NULL pointer dereference can a patch only be submitted if it is known
where the NULL comes from, and at what time the reason for producing the
NULL was introduced into the kernel?  Adding a NULL test without fully
understanding how the NULL can arise could reasonably be seen as papering
over a real problem.  But sometimes it could be better to paper over the
problem than incur the problem in a critical situation.

But I agree that these are corner cases.  Hopefully if such a NULL test
was submitted with an explanation on why the submitter was not able to
find the source of the problem and why the problem is important, then the
maintainer could provide some guidance that would resolve the situation.

julia

_______________________________________________
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 1/3] dma-direct: provide the ability to reserve per-numa CMA
From: Dan Carpenter @ 2020-06-04 11:36 UTC (permalink / raw)
  To: kbuild, Barry Song, hch, m.szyprowski, robin.murphy,
	catalin.marinas
  Cc: kbuild-all, lkp, john.garry, linux-kernel, linuxarm, iommu,
	Jonathan.Cameron, linux-arm-kernel, Dan Carpenter
In-Reply-To: <20200603024231.61748-2-song.bao.hua@hisilicon.com>

[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]

Hi Barry,

url:    https://github.com/0day-ci/linux/commits/Barry-Song/support-per-numa-CMA-for-ARM-server/20200603-104821
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: x86_64-randconfig-m001-20200603 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
kernel/dma/contiguous.c:274 dma_alloc_contiguous() warn: variable dereferenced before check 'dev' (see line 272)

# https://github.com/0day-ci/linux/commit/adb919e972c1cac3d8b11905d5258d23d3aac6a4
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout adb919e972c1cac3d8b11905d5258d23d3aac6a4
vim +/dev +274 kernel/dma/contiguous.c

b1d2dc009dece4 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  267  struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
b1d2dc009dece4 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  268  {
90ae409f9eb3bc kernel/dma/contiguous.c       Christoph Hellwig 2019-08-20  269  	size_t count = size >> PAGE_SHIFT;
b1d2dc009dece4 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  270  	struct page *page = NULL;
bd2e75633c8012 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  271  	struct cma *cma = NULL;
adb919e972c1ca kernel/dma/contiguous.c       Barry Song        2020-06-03 @272  	int nid = dev_to_node(dev);
                                                                                                              ^^^
Dereferenced inside function.

bd2e75633c8012 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  273  
bd2e75633c8012 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23 @274  	if (dev && dev->cma_area)
                                                                                            ^^^
Too late.

bd2e75633c8012 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  275  		cma = dev->cma_area;
adb919e972c1ca kernel/dma/contiguous.c       Barry Song        2020-06-03  276  	else if ((nid != NUMA_NO_NODE) && dma_contiguous_pernuma_area[nid]
adb919e972c1ca kernel/dma/contiguous.c       Barry Song        2020-06-03  277  		&& !(gfp & (GFP_DMA | GFP_DMA32)))
adb919e972c1ca kernel/dma/contiguous.c       Barry Song        2020-06-03  278  		cma = dma_contiguous_pernuma_area[nid];
bd2e75633c8012 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  279  	else if (count > 1)
bd2e75633c8012 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  280  		cma = dma_contiguous_default_area;
b1d2dc009dece4 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  281  
b1d2dc009dece4 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  282  	/* CMA can be used only in the context which permits sleeping */
b1d2dc009dece4 kernel/dma/contiguous.c       Nicolin Chen      2019-05-23  283  	if (cma && gfpflags_allow_blocking(gfp)) {

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32337 bytes --]

[-- Attachment #3: 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] iommu/mediatek: Use totalram_pages to setup enable_4GB
From: David Hildenbrand @ 2020-06-04 11:32 UTC (permalink / raw)
  To: Miles Chen
  Cc: wsd_upstream, Joerg Roedel, linux-kernel, Chao Hao, iommu,
	linux-mediatek, Yong Wu, Matthias Brugger, yingjoe.chen,
	linux-arm-kernel
In-Reply-To: <1591264174.12661.17.camel@mtkswgap22>

On 04.06.20 11:49, Miles Chen wrote:
> On Thu, 2020-06-04 at 10:25 +0200, David Hildenbrand wrote:
>> On 04.06.20 10:01, Miles Chen wrote:
>>> To build this driver as a kernel module, we cannot use
>>> the unexported symbol "max_pfn" to setup enable_4GB.
>>>
>>> Use totalram_pages() instead to setup enable_4GB.
>>>
>>> Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
>>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Chao Hao <chao.hao@mediatek.com>
>>> ---
>>>  drivers/iommu/mtk_iommu.c | 5 ++---
>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 5f4d6df59cf6..c2798a6e0e38 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -3,7 +3,6 @@
>>>   * Copyright (c) 2015-2016 MediaTek Inc.
>>>   * Author: Yong Wu <yong.wu@mediatek.com>
>>>   */
>>> -#include <linux/memblock.h>
>>>  #include <linux/bug.h>
>>>  #include <linux/clk.h>
>>>  #include <linux/component.h>
>>> @@ -626,8 +625,8 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>  		return -ENOMEM;
>>>  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>>>  
>>> -	/* Whether the current dram is over 4GB */
>>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
>>> +	/* Whether the current dram is over 4GB, note: DRAM start at 1GB  */
>>> +	data->enable_4GB = !!(totalram_pages() > ((SZ_2G + SZ_1G) >> PAGE_SHIFT));
>>
>> A similar thing seems to be done by
>> drivers/media/platform/mtk-vpu/mtk_vpu.c:
>> 	vpu->enable_4GB = !!(totalram_pages() > (SZ_2G >> PAGE_SHIFT));
>>
>> I do wonder if some weird memory hotplug setups might give you false
>> negatives.
>>
>> E.g., start a VM with 1GB and hotplug 1GB - it will be hotplugged on
>> x86-64 above 4GB, turning max_pfn into 5GB. totalram_pages() should
>> return something < 2GB.
>>
>> Same can happen when you have a VM and use ballooning to fake-unplug
>> memory, making totalram_pages() return something < 4GB, but leaving
>> usable pfns >= 4GB
> 
> Yes. Yingjoe also told me that this patch is not correct.
> 
> Thanks for pointing this out. totalram_pages() does not work 
> for some cases:
> 

Just a thought: If memory hotplug is applicable as well, you might
either want to always assume data->enable_4GB, or handle memory hotplug
events from the memory notifier, when new memory gets onlined (not sure
how tricky that is).


-- 
Thanks,

David / dhildenb


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox