* [RESEND PATCH v2 0/2] Enable Odroid-XU3/4 to use Energy Model and Energy Aware Scheduler
From: Lukasz Luba @ 2020-02-20 9:56 UTC (permalink / raw)
To: linux-kernel, kgene, krzk, linux-arm-kernel, linux-samsung-soc,
devicetree, linux-pm
Cc: myungjoo.ham, kyungmin.park, cw00.choi, robh+dt, mark.rutland,
b.zolnierkie, lukasz.luba, dietmar.eggemann
Hi all,
This is just a resend, now with proper v2 in the patches subject.
The Odroid-XU4/3 is a decent and easy accessible ARM big.LITTLE platform,
which might be used for research and development.
This small patch set provides possibility to run Energy Aware Scheduler (EAS)
on Odroid-XU4/3 and experiment with it.
The patch 1/2 provides 'dynamic-power-coefficient' in CPU DT nodes, which is
then used by the Energy Model (EM).
The patch 2/2 enables SCHED_MC (which adds another level in scheduling domains)
and enables EM making EAS possible to run (when schedutil is set as a CPUFreq
governor).
1. Test results
Two types of different tests have been executed. The first is energy test
case showing impact on energy consumption of this patch set. It is using a
synthetic set of tasks (rt-app based). The second is the performance test
case which is using hackbench (less time to complete is better).
In both tests schedutil has been used as cpufreq governor. In all tests
PROVE_LOCKING has not been compiled into the kernels.
1.1 Energy test case
10 iterations of 24 periodic rt-app tasks (16ms period, 10% duty-cycle)
with energy measurement. The cpufreq governor - schedutil. Unit is Joules.
The energy is calculated based on hwmon0 and hwmon3 power1_input.
The goal is to save energy, lower is better.
+-----------+-----------------+------------------------+
| | Without patches | With patches |
+-----------+--------+--------+----------------+-------+
| benchmark | Mean | RSD* | Mean | RSD* |
+-----------+--------+--------+----------------+-------+
| 24 rt-app | 21.56 | 1.37% | 19.85 (-9.2%) | 0.92% |
| tasks | | | | |
+-----------+--------+--------+----------------+-------+
1.2 Performance test case
10 consecutive iterations of hackbench (hackbench -l 500 -s 4096),
no delay between two successive executions.
The cpufreq governor - schedutil. Units in seconds.
The goal is to see not regression, lower completion time is better.
+-----------+-----------------+------------------------+
| | Without patches | With patches |
+-----------+--------+--------+----------------+-------+
| benchmark | Mean | RSD* | Mean | RSD* |
+-----------+--------+--------+----------------+-------+
| hackbench | 8.15 | 2.86% | 7.95 (-2.5%) | 0.60% |
+-----------+--------+--------+----------------+-------+
*RSD: Relative Standard Deviation (std dev / mean)
Changes:
v2:
- changed dynamic power coeffcient to 90 for A7, which prevents odd
behaviour for some low utilisation and at low OPPs;
now, the power ratio is ~3x between big an LITTLE core;
it's better aligned with [1]; probably due to measurement noise
at lower OPPs the values obtained from hwmon0|3 were different
from reality; some synthetic workloads showed this differences
- cleaned commit messages (no measurements in commit message)
- merged configs into one patch and re-ordered patches
- provided energy measurmements in the cover letter
- measurements focused on comparing similar setup - with schedutil governor,
to compare apples with apples
The v1 can be found in [2].
The patch set is on top of Krzysztof's tree, branch 'next/dt' [3] and has
been tested on Odroid-XU3 rev0.2 20140529.
Regards,
Lukasz Luba
[1] https://www.cl.cam.ac.uk/~rdm34/big.LITTLE.pdf
[2] https://lore.kernel.org/linux-arm-kernel/20200127215453.15144-1-lukasz.luba@arm.com/T/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git/log/?h=next/dt
Lukasz Luba (2):
ARM: dts: exynos: Add dynamic-power-coefficient to Exynos5422 CPUs
ARM: exynos_defconfig: Enable SCHED_MC and ENERGY_MODEL
arch/arm/boot/dts/exynos5422-cpus.dtsi | 8 ++++++++
arch/arm/configs/exynos_defconfig | 2 ++
2 files changed, 10 insertions(+)
--
2.17.1
^ permalink raw reply
* [RESEND PATCH v2 1/2] ARM: dts: exynos: Add dynamic-power-coefficient to Exynos5422 CPUs
From: Lukasz Luba @ 2020-02-20 9:56 UTC (permalink / raw)
To: linux-kernel, kgene, krzk, linux-arm-kernel, linux-samsung-soc,
devicetree, linux-pm
Cc: myungjoo.ham, kyungmin.park, cw00.choi, robh+dt, mark.rutland,
b.zolnierkie, lukasz.luba, dietmar.eggemann
In-Reply-To: <20200220095636.29469-1-lukasz.luba@arm.com>
To use Energy Aware Scheduler (EAS) the Energy Model (EM) should be
registered for CPUs. Add dynamic-power-coefficient into CPU nodes which
let CPUFreq subsystem register the EM structures. This will increase
energy efficiency of big.LITTLE platforms.
The 'dynamic-power-coefficient' values have been obtained experimenting
with different workloads. The power measurements taken from big CPU
Cluster and LITTLE CPU Cluster has been compared with official documents
and synthetic workloads estimations. The effective power ratio between
Cortex-A7 and Cortex-A15 CPUs (~3x) is also aligned with documentation.
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
arch/arm/boot/dts/exynos5422-cpus.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5422-cpus.dtsi b/arch/arm/boot/dts/exynos5422-cpus.dtsi
index 1b8605cf2407..4b641b9b8179 100644
--- a/arch/arm/boot/dts/exynos5422-cpus.dtsi
+++ b/arch/arm/boot/dts/exynos5422-cpus.dtsi
@@ -31,6 +31,7 @@
operating-points-v2 = <&cluster_a7_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <539>;
+ dynamic-power-coefficient = <90>;
};
cpu1: cpu@101 {
@@ -43,6 +44,7 @@
operating-points-v2 = <&cluster_a7_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <539>;
+ dynamic-power-coefficient = <90>;
};
cpu2: cpu@102 {
@@ -55,6 +57,7 @@
operating-points-v2 = <&cluster_a7_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <539>;
+ dynamic-power-coefficient = <90>;
};
cpu3: cpu@103 {
@@ -67,6 +70,7 @@
operating-points-v2 = <&cluster_a7_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <539>;
+ dynamic-power-coefficient = <90>;
};
cpu4: cpu@0 {
@@ -79,6 +83,7 @@
operating-points-v2 = <&cluster_a15_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <1024>;
+ dynamic-power-coefficient = <310>;
};
cpu5: cpu@1 {
@@ -91,6 +96,7 @@
operating-points-v2 = <&cluster_a15_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <1024>;
+ dynamic-power-coefficient = <310>;
};
cpu6: cpu@2 {
@@ -103,6 +109,7 @@
operating-points-v2 = <&cluster_a15_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <1024>;
+ dynamic-power-coefficient = <310>;
};
cpu7: cpu@3 {
@@ -115,6 +122,7 @@
operating-points-v2 = <&cluster_a15_opp_table>;
#cooling-cells = <2>; /* min followed by max */
capacity-dmips-mhz = <1024>;
+ dynamic-power-coefficient = <310>;
};
};
};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 20/21] btrfs: skip LOOP_NO_EMPTY_SIZE if not clustered allocation
From: Naohiro Aota @ 2020-02-20 9:56 UTC (permalink / raw)
To: Josef Bacik
Cc: linux-btrfs, David Sterba, Chris Mason, Nikolay Borisov,
Damien Le Moal, Johannes Thumshirn, Hannes Reinecke, Anand Jain,
linux-fsdevel
In-Reply-To: <b8908ae0-9e4d-5086-0d4c-768d45215695@toxicpanda.com>
On Thu, Feb 13, 2020 at 02:55:30PM -0500, Josef Bacik wrote:
>On 2/12/20 2:20 AM, Naohiro Aota wrote:
>>LOOP_NO_EMPTY_SIZE is solely dedicated for clustered allocation. So,
>>we can skip this stage and go to LOOP_GIVEUP stage to indicate we gave
>>up the allocation. This commit also moves the scope of the "clustered"
>>variable.
>>
>>Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>>---
>> fs/btrfs/extent-tree.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>>diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>>index 8f0d489f76fa..3ab0d2f5d718 100644
>>--- a/fs/btrfs/extent-tree.c
>>+++ b/fs/btrfs/extent-tree.c
>>@@ -3373,6 +3373,7 @@ enum btrfs_loop_type {
>> LOOP_CACHING_WAIT,
>> LOOP_ALLOC_CHUNK,
>> LOOP_NO_EMPTY_SIZE,
>>+ LOOP_GIVEUP,
>
>Why do we need a new loop definition here? Can we just return ENOSPC
>and be done? You don't appear to use it anywhere, so it doesn't seem
>like it's needed. Thanks,
>
>Josef
This is for other allocation policy to skip unnecessary loop stages
(e.g. LOOP_NO_EMPTY_SIZE) from an earlier stage. For example, zoned
allocation policy can implement the code below in
chunk_allocation_failed() to skip the following stages.
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 4badfae0c932..0a18c09b078b 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3775,6 +3854,10 @@ static int chunk_allocation_failed(struct find_free_extent_ctl *ffe_ctl)
*/
ffe_ctl->loop = LOOP_NO_EMPTY_SIZE;
return 0;
+ case BTRFS_EXTENT_ALLOC_ZONED:
+ /* give up here */
+ ffe_ctl->loop = LOOP_GIVEUP;
+ return -ENOSPC;
default:
BUG();
}
But, I can keep this LOOP_GIVEUP introduction patch later with this
zoned allocator ones.
Thanks,
^ permalink raw reply related
* Re: [net-next 06/13] ice: Report correct DCB mode
From: Sergei Shtylyov @ 2020-02-20 9:55 UTC (permalink / raw)
To: Jeff Kirsher, davem
Cc: Avinash Dayanand, netdev, nhorman, sassmann, Scott Register,
Andrew Bowers
In-Reply-To: <20200219220652.2255171-7-jeffrey.t.kirsher@intel.com>
On 20.02.2020 1:06, Jeff Kirsher wrote:
> From: Avinash Dayanand <avinash.dayanand@intel.com>
>
> Add code to detect if DCB is in IEEE or CEE mode. Without this the code
> will always report as IEEE mode which is incorrect and confuses the
> user.
>
> Signed-off-by: Avinash Dayanand <avinash.dayanand@intel.com>
> Signed-off-by: Scott Register <scottx.register@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 27 +++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> index 1c118e7bab88..16656b6c3d09 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> @@ -62,6 +62,26 @@ u8 ice_dcb_get_ena_tc(struct ice_dcbx_cfg *dcbcfg)
> return ena_tc;
> }
>
> +/**
> + * ice_dcb_get_mode - gets the DCB mode
> + * @port_info: pointer to port info structure
> + * @host: if set it's HOST if not it's MANAGED
> + */
> +static u8 ice_dcb_get_mode(struct ice_port_info *port_info, bool host)
> +{
> + u8 mode;
> +
> + if (host)
> + mode = DCB_CAP_DCBX_HOST;
> + else
> + mode = DCB_CAP_DCBX_LLD_MANAGED;
> +
> + if (port_info->local_dcbx_cfg.dcbx_mode & ICE_DCBX_MODE_CEE)
> + return (mode | DCB_CAP_DCBX_VER_CEE);
> + else
> + return (mode | DCB_CAP_DCBX_VER_IEEE);
() not really needed here and above.
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v8 00/12] add support for Clang's Shadow Call Stack
From: Marc Zyngier @ 2020-02-20 9:55 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: James Morse, Sami Tolvanen, Will Deacon, Catalin Marinas,
Steven Rostedt, Masami Hiramatsu, Mark Rutland, Dave Martin,
Kees Cook, Laura Abbott, Nick Desaulniers, Jann Horn,
Miguel Ojeda, Masahiro Yamada, clang-built-linux,
Kernel Hardening, linux-arm-kernel, Linux Kernel Mailing List
In-Reply-To: <CAKv+Gu8gHcYW_5G5pfS=yVA7J5JPq0tWqYRcVBAxS0ZYjw9jPw@mail.gmail.com>
On 2020-02-19 18:53, Ard Biesheuvel wrote:
> On Wed, 19 Feb 2020 at 19:38, James Morse <james.morse@arm.com> wrote:
>>
>> Hi Sami,
>>
>> (CC: +Marc)
>>
>> On 19/02/2020 00:08, Sami Tolvanen wrote:
>> > This patch series adds support for Clang's Shadow Call Stack
>> > (SCS) mitigation, which uses a separately allocated shadow stack
>> > to protect against return address overwrites.
>>
>> I took this for a spin on some real hardware. cpu-idle, kexec
>> hibernate etc all work
>> great... but starting a KVM guest causes the CPU to get stuck in EL2.
>>
>> With CONFIG_SHADOW_CALL_STACK disabled, this doesn't happen ... so its
>> something about the
>> feature being enabled.
>>
>>
>> I'm using clang-9 from debian bullseye/sid. (I tried to build tip of
>> tree ... that doesn't
>> go so well on arm64)
>>
>> KVM takes an instruction abort from EL2 to EL2, because some of the
>> code it runs is not
>> mapped at EL2:
>>
>> | ffffa00011588308 <__kvm_tlb_flush_local_vmid>:
>> | ffffa00011588308: d10103ff sub sp, sp, #0x40
>> | ffffa0001158830c: f90013f3 str x19, [sp, #32]
>> | ffffa00011588310: a9037bfd stp x29, x30, [sp, #48]
>> | ffffa00011588314: 9100c3fd add x29, sp, #0x30
>> | ffffa00011588318: 97ae18bf bl ffffa0001010e614
>> <__kern_hyp_va>
>>
>> INSTRUCTION ABORT!
>>
>> | ffffa0001158831c: f9400000 ldr x0, [x0]
>> | ffffa00011588320: 97ae18bd bl ffffa0001010e614
>> <__kern_hyp_va>
>> | ffffa00011588324: aa0003f3 mov x19, x0
>> | ffffa00011588328: 97ae18c1 bl ffffa0001010e62c
>> <has_vhe>
>>
>>
>> __kern_hyp_va() is static-inline which is patched wherever it appears
>> at boot with the EL2
>> ASLR values, it converts a kernel linear-map address to its EL2 KVM
>> alias:
>>
>> | ffffa0001010dc5c <__kern_hyp_va>:
>> | ffffa0001010dc5c: 92400000 and x0, x0, #0x1
>> | ffffa0001010dc60: 93c00400 ror x0, x0, #1
>> | ffffa0001010dc64: 91000000 add x0, x0, #0x0
>> | ffffa0001010dc68: 91400000 add x0, x0, #0x0, lsl
>> #12
>> | ffffa0001010dc6c: 93c0fc00 ror x0, x0, #63
>> | ffffa0001010dc70: d65f03c0 ret
>>
>>
>> The problem here is where __kern_hyp_va() is. Its outside the
>> __hyp_text section:
>> | morse@eglon:~/kernel/linux-pigs$ nm -s vmlinux | grep hyp_text
>> | ffffa0001158b800 T __hyp_text_end
>> | ffffa000115838a0 T __hyp_text_start
>>
>>
>> If I disable CONFIG_SHADOW_CALL_STACK in Kconfig, I get:
>> | ffffa00011527fe0 <__kvm_tlb_flush_local_vmid>:
>> | ffffa00011527fe0: d100c3ff sub sp, sp, #0x30
>> | ffffa00011527fe4: a9027bfd stp x29, x30, [sp, #32]
>> | ffffa00011527fe8: 910083fd add x29, sp, #0x20
>> | ffffa00011527fec: 92400000 and x0, x0, #0x1
>> | ffffa00011527ff0: 93c00400 ror x0, x0, #1
>> | ffffa00011527ff4: 91000000 add x0, x0, #0x0
>> | ffffa00011527ff8: 91400000 add x0, x0, #0x0, lsl
>> #12
>> | ffffa00011527ffc: 93c0fc00 ror x0, x0, #63
>> | ffffa00011528000: f9400000 ldr x0, [x0]
>> | ffffa00011528004: 910023e1 add x1, sp, #0x8
>> | ffffa00011528008: 92400000 and x0, x0, #0x1
>> | ffffa0001152800c: 93c00400 ror x0, x0, #1
>> | ffffa00011528010: 91000000 add x0, x0, #0x0
>> | ffffa00011528014: 91400000 add x0, x0, #0x0, lsl
>> #12
>> | ffffa00011528018: 93c0fc00 ror x0, x0, #63
>> | ffffa0001152801c: 97ffff78 bl ffffa00011527dfc
>> <__tlb_switch_>
>> | ffffa00011528020: d508871f tlbi vmalle1
>> | ffffa00011528024: d503201f nop
>>
>>
>> This looks like reserving x18 is causing Clang to not-inline the
>> __kern_hyp_va() calls,
>> losing the vitally important section information. (I can see why the
>> compiler thinks this
>> is fair)
>>
>> Is this a known, er, thing, with clang-9?
>>
>> From eyeballing the disassembly __always_inline on __kern_hyp_va() is
>> enough of a hint to
>> stop this, ... with this configuration of clang-9. But KVM still
>> doesn't work, so it isn't
>> the only inlining decision KVM relies on that is changed by SCS.
>>
>> I suspect repainting all KVM's 'inline' with __always_inline will fix
>> it. (yuck!) I'll try
>> tomorrow.
>>
>
> If we are relying on the inlining for correctness, these should have
> been __always_inline to begin with, and yuckness aside, I don't think
> there's anything wrong with that.
Agreed. Not having __always_inline is definitely an oversight, and we
should fix it ASAP (hell knows what another compiler could produce...).
And the whole EL2 aliasing is utter yuck already, this isn't going to
make things much worse...
I can queue something today for __kern_hyp_va(), but I'd like to make
sure there isn't other silly mistakes like this one somewhere...
>> I don't think keeping the compiler-flags as they are today for KVM is
>> the right thing to
>> do, it could lead to x18 getting corrupted with the shared vhe/non-vhe
>> code. Splitting
>> that code up would lead to duplication.
>>
>> (hopefully objtool will be able to catch these at build time)
>>
>
> I don't see why we should selectively en/disable the reservation of
> x18 (as I argued in the context of the EFI libstub patch as well).
> Just reserving it everywhere shouldn't hurt performance, and removes
> the need to prove that we reserved it in all the right places.
I'd certainly like to keep things simple if we can.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v8 00/12] add support for Clang's Shadow Call Stack
From: Marc Zyngier @ 2020-02-20 9:55 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Mark Rutland, Kees Cook, Jann Horn, Masahiro Yamada,
Catalin Marinas, Kernel Hardening, Nick Desaulniers,
Linux Kernel Mailing List, Steven Rostedt, Miguel Ojeda,
James Morse, Masami Hiramatsu, Sami Tolvanen, clang-built-linux,
Laura Abbott, Will Deacon, Dave Martin, linux-arm-kernel
In-Reply-To: <CAKv+Gu8gHcYW_5G5pfS=yVA7J5JPq0tWqYRcVBAxS0ZYjw9jPw@mail.gmail.com>
On 2020-02-19 18:53, Ard Biesheuvel wrote:
> On Wed, 19 Feb 2020 at 19:38, James Morse <james.morse@arm.com> wrote:
>>
>> Hi Sami,
>>
>> (CC: +Marc)
>>
>> On 19/02/2020 00:08, Sami Tolvanen wrote:
>> > This patch series adds support for Clang's Shadow Call Stack
>> > (SCS) mitigation, which uses a separately allocated shadow stack
>> > to protect against return address overwrites.
>>
>> I took this for a spin on some real hardware. cpu-idle, kexec
>> hibernate etc all work
>> great... but starting a KVM guest causes the CPU to get stuck in EL2.
>>
>> With CONFIG_SHADOW_CALL_STACK disabled, this doesn't happen ... so its
>> something about the
>> feature being enabled.
>>
>>
>> I'm using clang-9 from debian bullseye/sid. (I tried to build tip of
>> tree ... that doesn't
>> go so well on arm64)
>>
>> KVM takes an instruction abort from EL2 to EL2, because some of the
>> code it runs is not
>> mapped at EL2:
>>
>> | ffffa00011588308 <__kvm_tlb_flush_local_vmid>:
>> | ffffa00011588308: d10103ff sub sp, sp, #0x40
>> | ffffa0001158830c: f90013f3 str x19, [sp, #32]
>> | ffffa00011588310: a9037bfd stp x29, x30, [sp, #48]
>> | ffffa00011588314: 9100c3fd add x29, sp, #0x30
>> | ffffa00011588318: 97ae18bf bl ffffa0001010e614
>> <__kern_hyp_va>
>>
>> INSTRUCTION ABORT!
>>
>> | ffffa0001158831c: f9400000 ldr x0, [x0]
>> | ffffa00011588320: 97ae18bd bl ffffa0001010e614
>> <__kern_hyp_va>
>> | ffffa00011588324: aa0003f3 mov x19, x0
>> | ffffa00011588328: 97ae18c1 bl ffffa0001010e62c
>> <has_vhe>
>>
>>
>> __kern_hyp_va() is static-inline which is patched wherever it appears
>> at boot with the EL2
>> ASLR values, it converts a kernel linear-map address to its EL2 KVM
>> alias:
>>
>> | ffffa0001010dc5c <__kern_hyp_va>:
>> | ffffa0001010dc5c: 92400000 and x0, x0, #0x1
>> | ffffa0001010dc60: 93c00400 ror x0, x0, #1
>> | ffffa0001010dc64: 91000000 add x0, x0, #0x0
>> | ffffa0001010dc68: 91400000 add x0, x0, #0x0, lsl
>> #12
>> | ffffa0001010dc6c: 93c0fc00 ror x0, x0, #63
>> | ffffa0001010dc70: d65f03c0 ret
>>
>>
>> The problem here is where __kern_hyp_va() is. Its outside the
>> __hyp_text section:
>> | morse@eglon:~/kernel/linux-pigs$ nm -s vmlinux | grep hyp_text
>> | ffffa0001158b800 T __hyp_text_end
>> | ffffa000115838a0 T __hyp_text_start
>>
>>
>> If I disable CONFIG_SHADOW_CALL_STACK in Kconfig, I get:
>> | ffffa00011527fe0 <__kvm_tlb_flush_local_vmid>:
>> | ffffa00011527fe0: d100c3ff sub sp, sp, #0x30
>> | ffffa00011527fe4: a9027bfd stp x29, x30, [sp, #32]
>> | ffffa00011527fe8: 910083fd add x29, sp, #0x20
>> | ffffa00011527fec: 92400000 and x0, x0, #0x1
>> | ffffa00011527ff0: 93c00400 ror x0, x0, #1
>> | ffffa00011527ff4: 91000000 add x0, x0, #0x0
>> | ffffa00011527ff8: 91400000 add x0, x0, #0x0, lsl
>> #12
>> | ffffa00011527ffc: 93c0fc00 ror x0, x0, #63
>> | ffffa00011528000: f9400000 ldr x0, [x0]
>> | ffffa00011528004: 910023e1 add x1, sp, #0x8
>> | ffffa00011528008: 92400000 and x0, x0, #0x1
>> | ffffa0001152800c: 93c00400 ror x0, x0, #1
>> | ffffa00011528010: 91000000 add x0, x0, #0x0
>> | ffffa00011528014: 91400000 add x0, x0, #0x0, lsl
>> #12
>> | ffffa00011528018: 93c0fc00 ror x0, x0, #63
>> | ffffa0001152801c: 97ffff78 bl ffffa00011527dfc
>> <__tlb_switch_>
>> | ffffa00011528020: d508871f tlbi vmalle1
>> | ffffa00011528024: d503201f nop
>>
>>
>> This looks like reserving x18 is causing Clang to not-inline the
>> __kern_hyp_va() calls,
>> losing the vitally important section information. (I can see why the
>> compiler thinks this
>> is fair)
>>
>> Is this a known, er, thing, with clang-9?
>>
>> From eyeballing the disassembly __always_inline on __kern_hyp_va() is
>> enough of a hint to
>> stop this, ... with this configuration of clang-9. But KVM still
>> doesn't work, so it isn't
>> the only inlining decision KVM relies on that is changed by SCS.
>>
>> I suspect repainting all KVM's 'inline' with __always_inline will fix
>> it. (yuck!) I'll try
>> tomorrow.
>>
>
> If we are relying on the inlining for correctness, these should have
> been __always_inline to begin with, and yuckness aside, I don't think
> there's anything wrong with that.
Agreed. Not having __always_inline is definitely an oversight, and we
should fix it ASAP (hell knows what another compiler could produce...).
And the whole EL2 aliasing is utter yuck already, this isn't going to
make things much worse...
I can queue something today for __kern_hyp_va(), but I'd like to make
sure there isn't other silly mistakes like this one somewhere...
>> I don't think keeping the compiler-flags as they are today for KVM is
>> the right thing to
>> do, it could lead to x18 getting corrupted with the shared vhe/non-vhe
>> code. Splitting
>> that code up would lead to duplication.
>>
>> (hopefully objtool will be able to catch these at build time)
>>
>
> I don't see why we should selectively en/disable the reservation of
> x18 (as I argued in the context of the EFI libstub patch as well).
> Just reserving it everywhere shouldn't hurt performance, and removes
> the need to prove that we reserved it in all the right places.
I'd certainly like to keep things simple if we can.
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
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 8/8] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events
From: maddy @ 2020-02-20 9:53 UTC (permalink / raw)
To: Kajol Jain, acme, linuxppc-dev
Cc: ravi.bangoria, maddy, jmario, mpetlan, peterz, gregkh,
linux-kernel, alexander.shishkin, linux-perf-users, ak, yao.jin,
anju, jolsa, namhyung, mingo, kan.liang
In-Reply-To: <20200214110335.31483-9-kjain@linux.ibm.com>
On 2/14/20 4:33 PM, Kajol Jain wrote:
> The hv_24×7 feature in IBM® POWER9™ processor-based servers provide the
> facility to continuously collect large numbers of hardware performance
> metrics efficiently and accurately.
> This patch adds hv_24x7 json metric file for different Socket/chip
> resources.
>
> Result:
>
> power9 platform:
>
> command:# ./perf stat --metric-only -M Memory_RD_BW_Chip -C 0
> -I 1000 sleep 1
>
> time MB Memory_RD_BW_Chip_0 MB Memory_RD_BW_Chip_1 MB
> 1.000192635 0.4 0.0
> 1.001695883 0.0 0.0
>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> .../arch/powerpc/power9/hv_24x7_metrics.json | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
>
> diff --git a/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json b/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
> new file mode 100644
> index 000000000000..ac38f5540ac6
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
Better to have it as nest_metrics.json instead. Rest looks fine
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> @@ -0,0 +1,19 @@
> +[
> + {
> + "MetricExpr": "(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT23\\,chip\\=?@)",
> + "MetricName": "Memory_RD_BW_Chip",
> + "MetricGroup": "Memory_BW",
> + "ScaleUnit": "1.6e-2MB"
> + },
> + {
> + "MetricExpr": "(hv_24x7@PM_MCS01_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_WR_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT23\\,chip\\=?@ )",
> + "MetricName": "Memory_WR_BW_Chip",
> + "MetricGroup": "Memory_BW",
> + "ScaleUnit": "1.6e-2MB"
> + },
> + {
> + "MetricExpr": "(hv_24x7@PM_PB_CYC\\,chip\\=?@ )",
> + "MetricName": "PowerBUS_Frequency",
> + "ScaleUnit": "2.5e-7GHz"
> + }
> +]
^ permalink raw reply
* Re: [PATCH] Documentation/admin-guide/acpi: fix fan_performance_states.rst warnings
From: Rafael J. Wysocki @ 2020-02-20 9:53 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-doc@vger.kernel.org, ACPI Devel Mailing List,
Srinivas Pandruvada, Rafael J. Wysocki, Jonathan Corbet
In-Reply-To: <98d72d3b-c9eb-36bc-1d68-2c3020bdf9c5@infradead.org>
On Mon, Feb 17, 2020 at 5:42 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix Sphinx format warnings in fan_performace_states.rst
> by adding indentation.
>
> Documentation/admin-guide/acpi/fan_performance_states.rst:21: WARNING: Literal block ends without a blank line; unexpected unindent.
> Documentation/admin-guide/acpi/fan_performance_states.rst:41: WARNING: Literal block expected; none found.
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> Documentation/admin-guide/acpi/fan_performance_states.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- lnx-56-rc2.orig/Documentation/admin-guide/acpi/fan_performance_states.rst
> +++ lnx-56-rc2/Documentation/admin-guide/acpi/fan_performance_states.rst
> @@ -18,7 +18,7 @@ may look as follows::
>
> $ ls -l /sys/bus/acpi/devices/INT3404:00/
> total 0
> -...
> + ...
> -r--r--r-- 1 root root 4096 Dec 13 20:38 state0
> -r--r--r-- 1 root root 4096 Dec 13 20:38 state1
> -r--r--r-- 1 root root 4096 Dec 13 20:38 state10
> @@ -38,7 +38,7 @@ where each of the "state*" files represe
> and contains a colon-separated list of 5 integer numbers (fields) with the
> following interpretation::
>
> -control_percent:trip_point_index:speed_rpm:noise_level_mdb:power_mw
> + control_percent:trip_point_index:speed_rpm:noise_level_mdb:power_mw
>
> * ``control_percent``: The percent value to be used to set the fan speed to a
> specific level using the _FSL object (0-100).
Applied as a fix for 5.6, thanks!
^ permalink raw reply
* Re: [PATCH 8/8] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events
From: maddy @ 2020-02-20 9:53 UTC (permalink / raw)
To: Kajol Jain, acme, linuxppc-dev
Cc: linux-kernel, linux-perf-users, anju, maddy, ravi.bangoria,
peterz, yao.jin, ak, jolsa, kan.liang, jmario, alexander.shishkin,
mingo, paulus, namhyung, mpetlan, gregkh, benh
In-Reply-To: <20200214110335.31483-9-kjain@linux.ibm.com>
On 2/14/20 4:33 PM, Kajol Jain wrote:
> The hv_24×7 feature in IBM® POWER9™ processor-based servers provide the
> facility to continuously collect large numbers of hardware performance
> metrics efficiently and accurately.
> This patch adds hv_24x7 json metric file for different Socket/chip
> resources.
>
> Result:
>
> power9 platform:
>
> command:# ./perf stat --metric-only -M Memory_RD_BW_Chip -C 0
> -I 1000 sleep 1
>
> time MB Memory_RD_BW_Chip_0 MB Memory_RD_BW_Chip_1 MB
> 1.000192635 0.4 0.0
> 1.001695883 0.0 0.0
>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> .../arch/powerpc/power9/hv_24x7_metrics.json | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
>
> diff --git a/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json b/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
> new file mode 100644
> index 000000000000..ac38f5540ac6
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/powerpc/power9/hv_24x7_metrics.json
Better to have it as nest_metrics.json instead. Rest looks fine
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> @@ -0,0 +1,19 @@
> +[
> + {
> + "MetricExpr": "(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT23\\,chip\\=?@)",
> + "MetricName": "Memory_RD_BW_Chip",
> + "MetricGroup": "Memory_BW",
> + "ScaleUnit": "1.6e-2MB"
> + },
> + {
> + "MetricExpr": "(hv_24x7@PM_MCS01_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_WR_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT23\\,chip\\=?@ )",
> + "MetricName": "Memory_WR_BW_Chip",
> + "MetricGroup": "Memory_BW",
> + "ScaleUnit": "1.6e-2MB"
> + },
> + {
> + "MetricExpr": "(hv_24x7@PM_PB_CYC\\,chip\\=?@ )",
> + "MetricName": "PowerBUS_Frequency",
> + "ScaleUnit": "2.5e-7GHz"
> + }
> +]
^ permalink raw reply
* Re: [PATCH v4] tools/perf/metricgroup: Fix printing event names of metric group with multiple events incase of overlapping events
From: kajoljain @ 2020-02-20 9:53 UTC (permalink / raw)
To: Joakim Zhang, acme@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Jiri Olsa, Alexander Shishkin, Andi Kleen, Kan Liang,
Peter Zijlstra, Jin Yao, Madhavan Srinivasan, Anju T Sudhakar,
Ravi Bangoria
In-Reply-To: <DB7PR04MB46186AB5557F4D04FD5C4FEAE6160@DB7PR04MB4618.eurprd04.prod.outlook.com>
On 2/17/20 8:41 AM, Joakim Zhang wrote:
>
>> -----Original Message-----
>> From: linux-perf-users-owner@vger.kernel.org
>> <linux-perf-users-owner@vger.kernel.org> On Behalf Of Kajol Jain
>> Sent: 2020年2月12日 13:41
>> To: acme@kernel.org
>> Cc: linux-kernel@vger.kernel.org; linux-perf-users@vger.kernel.org;
>> kjain@linux.ibm.com; Jiri Olsa <jolsa@kernel.org>; Alexander Shishkin
>> <alexander.shishkin@linux.intel.com>; Andi Kleen <ak@linux.intel.com>; Kan
>> Liang <kan.liang@linux.intel.com>; Peter Zijlstra <peterz@infradead.org>; Jin
>> Yao <yao.jin@linux.intel.com>; Madhavan Srinivasan
>> <maddy@linux.vnet.ibm.com>; Anju T Sudhakar <anju@linux.vnet.ibm.com>;
>> Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> Subject: [PATCH v4] tools/perf/metricgroup: Fix printing event names of metric
>> group with multiple events incase of overlapping events
>>
>> Commit f01642e4912b ("perf metricgroup: Support multiple events for
>> metricgroup") introduced support for multiple events in a metric group. But
>> with the current upstream, metric events names are not printed properly
>> incase we try to run multiple metric groups with overlapping event.
>>
>> With current upstream version, incase of overlapping metric events issue is, we
>> always start our comparision logic from start.
>> So, the events which already matched with some metric group also take part in
>> comparision logic. Because of that when we have overlapping events, we end
>> up matching current metric group event with already matched one.
>>
>> For example, in skylake machine we have metric event CoreIPC and
>> Instructions. Both of them need 'inst_retired.any' event value.
>> As events in Instructions is subset of events in CoreIPC, they endup in pointing
>> to same 'inst_retired.any' value.
>>
>> In skylake platform:
>>
>> command:# ./perf stat -M CoreIPC,Instructions -C 0 sleep 1
>>
>> Performance counter stats for 'CPU(s) 0':
>>
>> 1,254,992,790 inst_retired.any # 1254992790.0
>> Instructions
>> # 1.3
>> CoreIPC
>> 977,172,805 cycles
>> 1,254,992,756 inst_retired.any
>>
>> 1.000802596 seconds time elapsed
>>
>> command:# sudo ./perf stat -M UPI,IPC sleep 1
>>
>> Performance counter stats for 'sleep 1':
>>
>> 948,650 uops_retired.retire_slots
>> 866,182 inst_retired.any # 0.7 IPC
>> 866,182 inst_retired.any
>> 1,175,671 cpu_clk_unhalted.thread
>>
>> Patch fixes the issue by adding a new bool pointer 'evlist_used' to keep track of
>> events which already matched with some group by setting it true.
>> So, we skip all used events in list when we start comparision logic.
>> Patch also make some changes in comparision logic, incase we get a match
>> miss, we discard the whole match and start again with first event id in metric
>> event.
>>
>> With this patch:
>> In skylake platform:
>>
>> command:# ./perf stat -M CoreIPC,Instructions -C 0 sleep 1
>>
>> Performance counter stats for 'CPU(s) 0':
>>
>> 3,348,415 inst_retired.any # 0.3 CoreIPC
>> 11,779,026 cycles
>> 3,348,381 inst_retired.any # 3348381.0
>> Instructions
>>
>> 1.001649056 seconds time elapsed
>>
>> command:# ./perf stat -M UPI,IPC sleep 1
>>
>> Performance counter stats for 'sleep 1':
>>
>> 1,023,148 uops_retired.retire_slots # 1.1 UPI
>> 924,976 inst_retired.any
>> 924,976 inst_retired.any # 0.6 IPC
>> 1,489,414 cpu_clk_unhalted.thread
>>
>> 1.003064672 seconds time elapsed
>>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Andi Kleen <ak@linux.intel.com>
>> Cc: Kan Liang <kan.liang@linux.intel.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Jin Yao <yao.jin@linux.intel.com>
>> Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>> Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>> tools/perf/util/metricgroup.c | 50 ++++++++++++++++++++++-------------
>> 1 file changed, 31 insertions(+), 19 deletions(-)
>
> Hi Kajol,
>
> I am not sure if it is good to ask a question here :-)
>
> I encountered a perf metricgroup issue, the result is incorrect when the metric includes more than 2 events.
>
> git log --oneline tools/perf/util/metricgroup.c
> 3635b27cc058 perf metricgroup: Fix printing event names of metric group with multiple events
> f01642e4912b perf metricgroup: Support multiple events for metricgroup
> 287f2649f791 perf metricgroup: Scale the metric result
>
> I did a simple test, below is the JSON file and result.
> [
> {
> "PublicDescription": "Calculate DDR0 bus actual utilization which vary from DDR0 controller clock frequency",
> "BriefDescription": "imx8qm: ddr0 bus actual utilization",
> "MetricName": "imx8qm-ddr0-bus-util",
> "MetricExpr": "( imx8_ddr0\\/read\\-cycles\\/ + imx8_ddr0\\/write\\-cycles\\/ )",
> "MetricGroup": "i.MX8QM_DDR0_BUS_UTIL"
> }
> ]
> ./perf stat -I 1000 -M imx8qm-ddr0-bus-util
> # time counts unit events
> 1.000104250 16720 imx8_ddr0/read-cycles/ # 22921.0 imx8qm-ddr0-bus-util
> 1.000104250 6201 imx8_ddr0/write-cycles/
> 2.000525625 8316 imx8_ddr0/read-cycles/ # 12785.5 imx8qm-ddr0-bus-util
> 2.000525625 2738 imx8_ddr0/write-cycles/
> 3.000819125 1056 imx8_ddr0/read-cycles/ # 4136.7 imx8qm-ddr0-bus-util
> 3.000819125 303 imx8_ddr0/write-cycles/
> 4.001103750 6260 imx8_ddr0/read-cycles/ # 9149.8 imx8qm-ddr0-bus-util
> 4.001103750 2317 imx8_ddr0/write-cycles/
> 5.001392750 2084 imx8_ddr0/read-cycles/ # 4516.0 imx8qm-ddr0-bus-util
> 5.001392750 601 imx8_ddr0/write-cycles/
>
> You can see that only the first result is correct, could this be reproduced at you side?
Hi Joakim,
Will try to look into it from my side.
Thanks,
Kajol
>
> Thanks a lot!
>
> Best Regards,
> Joakim Zhang
>
^ permalink raw reply
* next/master boot: 104 boots: 17 failed, 86 passed with 1 untried/unknown (next-20200220)
From: kernelci.org bot @ 2020-02-20 9:53 UTC (permalink / raw)
To: linux-next
next/master boot: 104 boots: 17 failed, 86 passed with 1 untried/unknown (next-20200220)
Full Boot Summary: https://kernelci.org/boot/all/job/next/branch/master/kernel/next-20200220/
Full Build Summary: https://kernelci.org/build/next/branch/master/kernel/next-20200220/
Tree: next
Branch: master
Git Describe: next-20200220
Git Commit: f4aba10148cd290bbbf4d0efae0e9789a13c2778
Git URL: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
Tested: 49 unique boards, 11 SoC families, 8 builds out of 41
Boot Regressions Detected:
arm:
sama5_defconfig:
gcc-8:
at91-sama5d4_xplained:
lab-baylibre: failing since 2 days (last pass: next-20200214 - first fail: next-20200217)
sunxi_defconfig:
gcc-8:
sun7i-a20-cubieboard2:
lab-clabbe: failing since 5 days (last pass: next-20200212 - first fail: next-20200214)
lab-baylibre: failing since 5 days (last pass: next-20200212 - first fail: next-20200214)
sun7i-a20-olinuxino-lime2:
lab-baylibre: failing since 5 days (last pass: next-20200212 - first fail: next-20200214)
sun8i-a33-olinuxino:
lab-clabbe: failing since 5 days (last pass: next-20200212 - first fail: next-20200214)
sun8i-a83t-bananapi-m3:
lab-clabbe: failing since 5 days (last pass: next-20200211 - first fail: next-20200214)
sun8i-h2-plus-libretech-all-h3-cc:
lab-baylibre: failing since 5 days (last pass: next-20200212 - first fail: next-20200214)
sun8i-h2-plus-orangepi-r1:
lab-baylibre: failing since 5 days (last pass: next-20200207 - first fail: next-20200214)
sun8i-h2-plus-orangepi-zero:
lab-baylibre: failing since 5 days (last pass: next-20200212 - first fail: next-20200214)
sun8i-h3-orangepi-pc:
lab-clabbe: failing since 5 days (last pass: next-20200212 - first fail: next-20200214)
arm64:
defconfig:
gcc-8:
rk3399-gru-kevin:
lab-collabora: failing since 5 days (last pass: next-20200213 - first fail: next-20200214)
defconfig+CONFIG_RANDOMIZE_BASE=y:
gcc-8:
meson-gxl-s805x-libretech-ac:
lab-baylibre: new failure (last pass: next-20200219)
rk3399-gru-kevin:
lab-collabora: new failure (last pass: next-20200213)
sun50i-h6-pine-h64-model-b:
lab-baylibre: new failure (last pass: next-20200219)
Boot Failures Detected:
arm:
sunxi_defconfig:
gcc-8:
sun7i-a20-cubieboard2: 2 failed labs
sun7i-a20-olinuxino-lime2: 1 failed lab
sun8i-a33-olinuxino: 1 failed lab
sun8i-a83t-bananapi-m3: 1 failed lab
sun8i-h2-plus-libretech-all-h3-cc: 1 failed lab
sun8i-h2-plus-orangepi-r1: 1 failed lab
sun8i-h2-plus-orangepi-zero: 1 failed lab
sun8i-h3-libretech-all-h3-cc: 1 failed lab
sun8i-h3-orangepi-pc: 1 failed lab
sama5_defconfig:
gcc-8:
at91-sama5d4_xplained: 1 failed lab
arm64:
defconfig:
gcc-8:
rk3399-gru-kevin: 1 failed lab
rk3399-puma-haikou: 1 failed lab
defconfig+CONFIG_RANDOMIZE_BASE=y:
gcc-8:
meson-gxl-s805x-libretech-ac: 1 failed lab
rk3399-gru-kevin: 1 failed lab
rk3399-puma-haikou: 1 failed lab
---
For more info write to <info@kernelci.org>
^ permalink raw reply
* Re: vc4 on rpi3 A+
From: Sergey Suloev @ 2020-02-20 9:52 UTC (permalink / raw)
To: Nicolas Saenz Julienne, linux-rpi-kernel
Cc: Stefan Wahren, Noralf Trønnes, bcm-kernel-feedback-list,
linux-arm-kernel
In-Reply-To: <4850fa1006d5a329c2e6d4e61ec6fb9420125414.camel@suse.de>
Hi, Nicolas,
On 2/20/20 12:25 PM, Nicolas Saenz Julienne wrote:
> Hi Sergey,
>
> On Thu, 2020-02-20 at 11:21 +0300, Sergey Suloev wrote:
>> Hello, guys,
>>
>> could anyone clarify the status of vc4 drm support on RPI 3A+ ?
> I don't have one so I can't really tell for that specific board, but I'm going
> to try to reproduce it on a rpi3b.
>
>> I tried to build kernel 5.5 and 5.6-rc2 in 32bit and aarch64
>> configurations with VC4 turned ON but both unsuccessful - vc4 drm driver
>> is listed in memory but not working and not producing any typical DRM
>> log output.
> AFAIK there is a known issue in 5.6-rc2, which has already been addressed[1].
> Note that the driver fails on probe so there is some amount of DRM output.
>
> I tried to reproduce your issue with v5.5, but vc4 seems to probe alright
> (rpi3b+aarch64+defconfig):
>
> [ 15.443047] vc4_hdmi 3f902000.hdmi: vc4-hdmi-hifi <-> 3f902000.hdmi mapping ok
> [ 15.452864] vc4_hdmi 3f902000.hdmi: ASoC: no DMI vendor name!
> [ 15.459836] vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops [vc4])
> [ 15.467062] vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [vc4])
> [ 15.478722] vc4-drm soc:gpu: bound 3f004000.txp (ops vc4_txp_ops [vc4])
> [ 15.485749] vc4-drm soc:gpu: bound 3f400000.hvs (ops vc4_hvs_ops [vc4])
> [ 15.499009] vc4-drm soc:gpu: bound 3f206000.pixelvalve (ops vc4_crtc_ops [vc4])
> [ 15.526217] vc4-drm soc:gpu: bound 3f207000.pixelvalve (ops vc4_crtc_ops [vc4])
> [ 15.542257] vc4-drm soc:gpu: bound 3f807000.pixelvalve (ops vc4_crtc_ops [vc4])
> [ 15.560113] vc4-drm soc:gpu: bound 3fc00000.v3d (ops vc4_v3d_ops [vc4])
> [ 15.574684] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [ 15.587375] [drm] Driver supports precise vblank timestamp query.
> [ 15.606831] [drm] Initialized vc4 0.0.0 20140616 for soc:gpu on minor 0
> [ 15.617505] Console: switching to colour frame buffer device 90x30
> [ 15.627858] vc4-drm soc:gpu: fb0: vc4drmfb frame buffer device
>
> Could it be that you forgot to update the device tree or kernel modules?
>
> Regards,
> Nicolas
>
> [1] https://lkml.kernel.org/lkml/20200219102526.692126-1-jbrunet@baylibre.com/T/
>
the patch [1] has already been included into 5.6-rc2, so the issue with
Rpi 3A+ isn't related to that.
[1]
https://lkml.kernel.org/lkml/20200219102526.692126-1-jbrunet@baylibre.com/T/
Thank you
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [Xen-devel] dom-u stuck in ---sr- state after reboot
From: msd+xen-devel @ 2020-02-20 9:52 UTC (permalink / raw)
To: xen-devel
Hi everyone,
Since I use Xen 4.11 on Debian 10, sometimes (about one in twenty times)
after a dom-u reboot, it stays stuck with the state '---sr-'.
This had never happened for me with Xen 4.4 on Debian 8.
I have to manually destroy and reboot the dom-u stuck domain.
I can't see any error in logs.
Do you know why and how I can avoid it ?
Best regards,
Guillaume
I try to ask on this mailing list because I got no response here :
https://markmail.org/message/n4mjhvxalc2gwf67
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* [PATCH v3] dm: uclass: don't assign aliased seq numbers
From: Michael Walle @ 2020-02-20 9:52 UTC (permalink / raw)
To: u-boot
In-Reply-To: <0329abd9-16bf-5c71-dc90-ccc05b79b7cf@xilinx.com>
Hi Michal,
Am 2020-02-20 09:30, schrieb Michal Simek:
> On 03. 02. 20 18:11, Michael Walle wrote:
>> If there are aliases for an uclass, set the base for the "dynamically"
>> allocated numbers next to the highest alias.
>>
>> Please note, that this might lead to holes in the sequences, depending
>> on the device tree. For example if there is only an alias "ethernet1",
>> the next device seq number would be 2.
>>
>> In particular this fixes a problem with boards which are using
>> ethernet
>> aliases but also might have network add-in cards like the E1000. If
>> the
>> board is started with the add-in card and depending on the order of
>> the
>> drivers, the E1000 might occupy the first ethernet device and mess up
>> all the hardware addresses, because the devices are now shifted by
>> one.
>>
>> Also adapt the test cases to the new handling and add test cases
>> checking the holes in the seq numbers.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> Reviewed-by: Alex Marginean <alexandru.marginean@nxp.com>
>> Tested-by: Alex Marginean <alexandru.marginean@nxp.com>
>> Acked-by: Vladimir Oltean <olteanv@gmail.com>
>> ---
>>
>> Please note that I've kept the R-b, T-b, and A-b tags although they
>> were
>> for an older version. They only affects the drivers/core/uclass.c not
>> the
>> test/dm/ part. OTOH none of the actual implementation has changed.
>>
>> I couldn't split the patch, otherwise the tests would fail.
>>
>> As a side effect, this should also make the following commits
>> superfluous:
>> - 7f3289bf6d ("dm: device: Request next sequence number")
>> - 61607225d1 ("i2c: Fill req_seq in i2c_post_bind()")
>> Although I don't understand the root cause of the said problem.
>>
>> Thomas, Michal, could you please test this and then I'd add a second
>> patch removing the old code.
>>
>> changes since v2:
>> - adapt/new test cases, thanks Simon
>>
>> changes since v1:
>> - move notice about superfluous commits from commit message to this
>> section.
>> - fix the comment style
>>
>> arch/sandbox/dts/test.dts | 4 ++--
>> drivers/core/uclass.c | 21 +++++++++++++++------
>> include/configs/sandbox.h | 6 +++---
>> test/dm/eth.c | 14 +++++++-------
>> test/dm/test-fdt.c | 22 +++++++++++++++++-----
>> 5 files changed, 44 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
>> index e529c54d8d..d448892a65 100644
>> --- a/arch/sandbox/dts/test.dts
>> +++ b/arch/sandbox/dts/test.dts
>> @@ -19,8 +19,8 @@
>> pci0 = &pci0;
>> pci1 = &pci1;
>> pci2 = &pci2;
>> - remoteproc1 = &rproc_1;
>> - remoteproc2 = &rproc_2;
>> + remoteproc0 = &rproc_1;
>> + remoteproc1 = &rproc_2;
>> rtc0 = &rtc_0;
>> rtc1 = &rtc_1;
>> spi0 = "/spi at 0";
>> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
>> index c520ef113a..3c216221e0 100644
>> --- a/drivers/core/uclass.c
>> +++ b/drivers/core/uclass.c
>> @@ -675,13 +675,14 @@ int uclass_unbind_device(struct udevice *dev)
>>
>> int uclass_resolve_seq(struct udevice *dev)
>> {
>> + struct uclass *uc = dev->uclass;
>> + struct uclass_driver *uc_drv = uc->uc_drv;
>> struct udevice *dup;
>> - int seq;
>> + int seq = 0;
>> int ret;
>>
>> assert(dev->seq == -1);
>> - ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id,
>> dev->req_seq,
>> - false, &dup);
>> + ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false,
>> &dup);
>> if (!ret) {
>> dm_warn("Device '%s': seq %d is in use by '%s'\n",
>> dev->name, dev->req_seq, dup->name);
>> @@ -693,9 +694,17 @@ int uclass_resolve_seq(struct udevice *dev)
>> return ret;
>> }
>>
>> - for (seq = 0; seq < DM_MAX_SEQ; seq++) {
>> - ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, seq,
>> - false, &dup);
>> + if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
>> + (uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
>> + /*
>> + * dev_read_alias_highest_id() will return -1 if there no
>> + * alias. Thus we can always add one.
>> + */
>> + seq = dev_read_alias_highest_id(uc_drv->name) + 1;
>> + }
>> +
>> + for (; seq < DM_MAX_SEQ; seq++) {
>> + ret = uclass_find_device_by_seq(uc_drv->id, seq, false, &dup);
>> if (ret == -ENODEV)
>> break;
>> if (ret)
>> diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
>> index 1c13055cdc..b02c362fed 100644
>> --- a/include/configs/sandbox.h
>> +++ b/include/configs/sandbox.h
>> @@ -97,9 +97,9 @@
>> #endif
>>
>> #define SANDBOX_ETH_SETTINGS "ethaddr=00:00:11:22:33:44\0" \
>> - "eth1addr=00:00:11:22:33:45\0" \
>> - "eth3addr=00:00:11:22:33:46\0" \
>> - "eth5addr=00:00:11:22:33:47\0" \
>> + "eth3addr=00:00:11:22:33:45\0" \
>> + "eth5addr=00:00:11:22:33:46\0" \
>> + "eth6addr=00:00:11:22:33:47\0" \
>> "ipaddr=1.2.3.4\0"
>>
>> #define MEM_LAYOUT_ENV_SETTINGS \
>> diff --git a/test/dm/eth.c b/test/dm/eth.c
>> index ad5354b4bf..75315a0c6d 100644
>> --- a/test/dm/eth.c
>> +++ b/test/dm/eth.c
>> @@ -47,7 +47,7 @@ static int dm_test_eth_alias(struct unit_test_state
>> *uts)
>> ut_assertok(net_loop(PING));
>> ut_asserteq_str("eth at 10002000", env_get("ethact"));
>>
>> - env_set("ethact", "eth1");
>> + env_set("ethact", "eth6");
>> ut_assertok(net_loop(PING));
>> ut_asserteq_str("eth at 10004000", env_get("ethact"));
>>
>> @@ -104,7 +104,7 @@ static int dm_test_eth_act(struct unit_test_state
>> *uts)
>> const char *ethname[DM_TEST_ETH_NUM] = {"eth at 10002000",
>> "eth at 10003000",
>> "sbe5", "eth at 10004000"};
>> const char *addrname[DM_TEST_ETH_NUM] = {"ethaddr", "eth5addr",
>> - "eth3addr", "eth1addr"};
>> + "eth3addr", "eth6addr"};
>> char ethaddr[DM_TEST_ETH_NUM][18];
>> int i;
>>
>> @@ -187,15 +187,15 @@ static int dm_test_eth_rotate(struct
>> unit_test_state *uts)
>>
>> /* Invalidate eth1's MAC address */
>> memset(ethaddr, '\0', sizeof(ethaddr));
>> - strncpy(ethaddr, env_get("eth1addr"), 17);
>> - /* Must disable access protection for eth1addr before clearing */
>> - env_set(".flags", "eth1addr");
>> - env_set("eth1addr", NULL);
>> + strncpy(ethaddr, env_get("eth6addr"), 17);
>> + /* Must disable access protection for eth6addr before clearing */
>> + env_set(".flags", "eth6addr");
>> + env_set("eth6addr", NULL);
>>
>> retval = _dm_test_eth_rotate1(uts);
>>
>> /* Restore the env */
>> - env_set("eth1addr", ethaddr);
>> + env_set("eth6addr", ethaddr);
>> env_set("ethrotate", NULL);
>>
>> if (!retval) {
>> diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
>> index d59c449ce0..a4e5c64a1f 100644
>> --- a/test/dm/test-fdt.c
>> +++ b/test/dm/test-fdt.c
>> @@ -359,20 +359,32 @@ static int dm_test_fdt_uclass_seq(struct
>> unit_test_state *uts)
>> ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 2, &dev));
>> ut_asserteq_str("d-test", dev->name);
>>
>> - /* d-test actually gets 0 */
>> - ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 0, &dev));
>> + /*
>> + * d-test actually gets 9, because thats the next free one after the
>> + * aliases.
>> + */
>> + ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 9, &dev));
>> ut_asserteq_str("d-test", dev->name);
>>
>> - /* initially no one wants seq 1 */
>> - ut_asserteq(-ENODEV, uclass_get_device_by_seq(UCLASS_TEST_FDT, 1,
>> + /* initially no one wants seq 10 */
>> + ut_asserteq(-ENODEV, uclass_get_device_by_seq(UCLASS_TEST_FDT, 10,
>> &dev));
>> ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
>> ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 4, &dev));
>>
>> /* But now that it is probed, we can find it */
>> - ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 1, &dev));
>> + ut_assertok(uclass_get_device_by_seq(UCLASS_TEST_FDT, 10, &dev));
>> ut_asserteq_str("f-test", dev->name);
>>
>> + /*
>> + * And we should still have holes in our sequence numbers, that is 2
>> + * and 4 should not be used.
>> + */
>> + ut_asserteq(-ENODEV, uclass_find_device_by_seq(UCLASS_TEST_FDT, 2,
>> + true, &dev));
>> + ut_asserteq(-ENODEV, uclass_find_device_by_seq(UCLASS_TEST_FDT, 4,
>> + true, &dev));
>> +
>> return 0;
>> }
>> DM_TEST(dm_test_fdt_uclass_seq, DM_TESTF_SCAN_PDATA |
>> DM_TESTF_SCAN_FDT);
>>
>
> Tested-by: Michal Simek <michal.simek@xilinx.com>
Thanks!
Just to be sure, did you test this patch or did you also revert
61607225d1 ("i2c: Fill req_seq in i2c_post_bind()")
before testing?
-michael
^ permalink raw reply
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
From: Vitaly Mireyno @ 2020-02-20 9:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-comment@lists.oasis-open.org, jasowang@redhat.com
>-----Original Message-----
>From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-open.org> On Behalf Of
>Michael S. Tsirkin
>Sent: Thursday, 20 February, 2020 10:12
>To: Vitaly Mireyno <vmireyno@marvell.com>
>Cc: virtio-comment@lists.oasis-open.org; jasowang@redhat.com
>Subject: Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
>
>----------------------------------------------------------------------
>On Thu, Oct 24, 2019 at 03:24:43PM +0000, Vitaly Mireyno wrote:
>> Some devices benefit from the knowledge of the exact header length for TSO processing.
>> Add a feature bit for a driver that is capable of providing the correct header length for TSO packets.
>>
>> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
>
>So I looked at implementing this and maybe this was not such a good idea after all.
>
>How would we implement this in Linux?
>Current code just puts skb_headlen there which is # of bytes in the linear buffer.
>Which I guess it often the header, but not at all always.
>
>Should this have been limited to TSO packets?
>
>I also could not figure out how this is useful for the host.
>Could you enlighten me pls?
As see it, header length is essential for TSO, and maybe not so useful for non-TSO.
To calculate the header length, I suppose a Linux driver could do something like:
skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data
>
>> ---
>> content.tex | 49 +++++++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 39 insertions(+), 10 deletions(-)
>>
>> diff --git a/content.tex b/content.tex index 679391e..dac6921 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -2811,6 +2811,9 @@ \subsection{Feature bits}\label{sec:Device Types
>> / Network Device / Feature bits \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address
>through control
>> channel.
>>
>> +\item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>> + value.
>> +
>> \item[VIRTIO_NET_F_RSC_EXT(61)] Device can process duplicated ACKs
>> and report number of coalesced segments and duplicated ACKs
>>
>> @@ -2840,6 +2843,7 @@ \subsubsection{Feature bit
>> requirements}\label{sec:Device Types / Network Device \item[VIRTIO_NET_F_MQ] Requires
>VIRTIO_NET_F_CTRL_VQ.
>> \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>> \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
>> +\item[VIRTIO_NET_F_GUEST_HDRLEN] Requires VIRTIO_NET_F_HOST_TSO4 or
>VIRTIO_NET_F_HOST_TSO6 or VIRTIO_NET_F_HOST_UFO.
>> \end{description}
>>
>> \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
>> / Network Device / Feature bits / Legacy Interface: Feature bits} @@ -3095,12 +3099,21 @@
>\subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
>> into smaller packets. The other gso fields are set:
>>
>> \begin{itemize}
>> - \item \field{hdr_len} is a hint to the device as to how much of the
>> header
>> + \item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
>> + \field{hdr_len} indicates the header length that needs to be replicated
>> + for each packet. It's a number of bytes from beginning of the packet
>> + to beginning of the transport payload.
>> + Otherwise, if the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
>> + \field{hdr_len} is a hint to the device as to how much of the
>> + header
>> needs to be kept to copy into each packet, usually set to the
>> length of the headers, including the transport
>> header\footnote{Due to various bugs in implementations, this field is not useful as a guarantee of
>the transport header size.
>> }.
>>
>> + \begin{note}
>> + Some devices benefit from the knowledge of the exact header length, for TSO processing.
>> + \end{note}
>> +
>> \item \field{gso_size} is the maximum size of each packet beyond that
>> header (ie. MSS).
>>
>> @@ -3173,9 +3186,17 @@ \subsubsection{Packet
>> Transmission}\label{sec:Device Types / Network Device / De desired MSS.
>>
>> If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have -been
>> negotiated, the driver SHOULD set \field{hdr_len} to a value -not less
>> than the length of the headers, including the transport -header.
>> +been negotiated:
>> +\begin{itemize}
>> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
>> + the driver MUST set \field{hdr_len} to a value equal to the length
>> + of the headers, including the transport header.
>> +
>> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
>> + the driver SHOULD set \field{hdr_len} to a value
>> + not less than the length of the headers, including the transport
>> + header.
>> +\end{itemize}
>>
>> The driver MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID and
>> VIRTIO_NET_HDR_F_RSC_INFO bits in \field{flags}.
>> @@ -3187,12 +3208,20 @@ \subsubsection{Packet
>> Transmission}\label{sec:Device Types / Network Device / De device MUST NOT use the
>\field{csum_start} and \field{csum_offset}.
>>
>> If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have -been
>> negotiated, the device MAY use \field{hdr_len} only as a hint about
>> the -transport header size.
>> -The device MUST NOT rely on \field{hdr_len} to be correct.
>> -\begin{note}
>> -This is due to various bugs in implementations.
>> -\end{note}
>> +been negotiated:
>> +\begin{itemize}
>> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
>> + the device MAY use \field{hdr_len} as the transport header size.
>> +
>> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
>> + the device MAY use \field{hdr_len} only as a hint about the
>> + transport header size.
>> + The device MUST NOT rely on \field{hdr_len} to be correct.
>> +
>> + \begin{note}
>> + This is due to various bugs in implementations.
>> + \end{note}
>> +\end{itemize}
>>
>> If VIRTIO_NET_HDR_F_NEEDS_CSUM is not set, the device MUST NOT rely
>> on the packet checksum being correct.
>> --
>>
>>
>> This publicly archived list offers a means to provide input to the
>> OASIS Virtual I/O Device (VIRTIO) TC.
>>
>> In order to verify user consent to the Feedback License terms and to
>> minimize spam in the list archive, subscription is required before
>> posting.
>>
>> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
>> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
>> List help: virtio-comment-help@lists.oasis-open.org
>> List archive:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.oasis-2Dope
>> n.org_archives_virtio-2Dcomment_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=l
>> DHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-
>> mREeI_KYFcLA9Idzwo&s=nlr22vvxamnoP6GTBunPiIQXatfR1jEpMVWo3QWjgkU&e=
>> Feedback License:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_who_ipr_feedback-5Flicense.pdf&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r
>> =lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915En
>> u-mREeI_KYFcLA9Idzwo&s=fLR52kyo8ob2sRKD-YqfBkcFpc4N8CjVyoGB8l9jiDY&e=
>> List Guidelines:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_policies-2Dguidelines_mailing-2Dlists&d=DwIFAg&c=nKjWec2b6R0mOyPaz
>> 7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kL
>> fj915Enu-mREeI_KYFcLA9Idzwo&s=T4qEUr9UYy6-wRGkVEQCwR6bmlDDnJHJMSR3cDx2
>> jow&e=
>> Committee:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_committees_virtio_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ
>> 3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFc
>> LA9Idzwo&s=FvN-HcjlfOBLxKBa3aCs7Q8oLFjlwi9b0VEfabOQ-yc&e=
>> Join OASIS:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_join_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdce
>> vq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=x-
>> xmnzrTYOlr2UQIHWAbcuM9xWt0IXCPnEFjNNdC2sA&e=
>
>
>This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC.
>
>In order to verify user consent to the Feedback License terms and to minimize spam in the list archive,
>subscription is required before posting.
>
>Subscribe: virtio-comment-subscribe@lists.oasis-open.org
>Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
>List help: virtio-comment-help@lists.oasis-open.org
>List archive: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.oasis-
>2Dopen.org_archives_virtio-
>2Dcomment_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-
>mREeI_KYFcLA9Idzwo&s=nlr22vvxamnoP6GTBunPiIQXatfR1jEpMVWo3QWjgkU&e=
>Feedback License: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_who_ipr_feedback-
>5Flicense.pdf&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=fLR52kyo8ob2sRKD-
>YqfBkcFpc4N8CjVyoGB8l9jiDY&e=
>List Guidelines: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_policies-2Dguidelines_mailing-
>2Dlists&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7
>xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=T4qEUr9UYy6-
>wRGkVEQCwR6bmlDDnJHJMSR3cDx2jow&e=
>Committee: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_committees_virtio_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgF
>Rdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=FvN-
>HcjlfOBLxKBa3aCs7Q8oLFjlwi9b0VEfabOQ-yc&e=
>Join OASIS: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_join_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQ
>Aw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=x-
>xmnzrTYOlr2UQIHWAbcuM9xWt0IXCPnEFjNNdC2sA&e=
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
^ permalink raw reply
* [PATCH] ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close()
From: Kai Vehmanen @ 2020-02-20 9:49 UTC (permalink / raw)
To: broonie, alsa-devel, kuninori.morimoto.gx, digetx
Cc: pierre-louis.bossart, kai.vehmanen, ranjani.sridharan
ASoC component open/close and snd_soc_component_module_get/put are called
independently for each component-substream pair, so the logic added in
commit dd03907bf129 ("ASoC: soc-pcm: call snd_soc_component_open/close()
once") was not sufficient and led to PCM playback and module unload errors.
Implement handling of failures directly in soc_pcm_components_open(),
so that any successfully opened components are closed upon error with
other components. This allows to clean up error handling in
soc_pcm_open() without adding more state tracking.
Fixes: dd03907bf129 ("ASoC: soc-pcm: call snd_soc_component_open/close() once")
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
include/sound/soc-component.h | 7 ++-----
sound/soc/soc-component.c | 35 +++++++----------------------------
sound/soc/soc-pcm.c | 27 +++++++++++++++++++++------
3 files changed, 30 insertions(+), 39 deletions(-)
diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 1866ecc8e94b..154d02fbbfed 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -147,6 +147,8 @@ struct snd_soc_component {
unsigned int active;
+ unsigned int suspended:1; /* is in suspend PM state */
+
struct list_head list;
struct list_head card_aux_list; /* for auxiliary bound components */
struct list_head card_list;
@@ -180,11 +182,6 @@ struct snd_soc_component {
struct dentry *debugfs_root;
const char *debugfs_prefix;
#endif
-
- /* bit field */
- unsigned int suspended:1; /* is in suspend PM state */
- unsigned int opened:1;
- unsigned int module:1;
};
#define for_each_component_dais(component, dai)\
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index ee00c09df5e7..14e175cdeeb8 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -297,55 +297,34 @@ EXPORT_SYMBOL_GPL(snd_soc_component_set_jack);
int snd_soc_component_module_get(struct snd_soc_component *component,
int upon_open)
{
- if (component->module)
- return 0;
-
if (component->driver->module_get_upon_open == !!upon_open &&
!try_module_get(component->dev->driver->owner))
return -ENODEV;
- component->module = 1;
-
return 0;
}
void snd_soc_component_module_put(struct snd_soc_component *component,
int upon_open)
{
- if (component->module &&
- component->driver->module_get_upon_open == !!upon_open)
+ if (component->driver->module_get_upon_open == !!upon_open)
module_put(component->dev->driver->owner);
-
- component->module = 0;
}
int snd_soc_component_open(struct snd_soc_component *component,
struct snd_pcm_substream *substream)
{
- int ret = 0;
-
- if (!component->opened &&
- component->driver->open)
- ret = component->driver->open(component, substream);
-
- if (ret == 0)
- component->opened = 1;
-
- return ret;
+ if (component->driver->open)
+ return component->driver->open(component, substream);
+ return 0;
}
int snd_soc_component_close(struct snd_soc_component *component,
struct snd_pcm_substream *substream)
{
- int ret = 0;
-
- if (component->opened &&
- component->driver->close)
- ret = component->driver->close(component, substream);
-
- component->opened = 0;
-
- return ret;
+ if (component->driver->close)
+ return component->driver->close(component, substream);
+ return 0;
}
int snd_soc_component_prepare(struct snd_soc_component *component,
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 63f67eb7c077..9e761f932887 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -469,28 +469,43 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
static int soc_pcm_components_open(struct snd_pcm_substream *substream)
{
struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_component *last = NULL;
struct snd_soc_component *component;
int i, ret = 0;
for_each_rtd_components(rtd, i, component) {
+ last = component;
+
ret = snd_soc_component_module_get_when_open(component);
if (ret < 0) {
dev_err(component->dev,
"ASoC: can't get module %s\n",
component->name);
- return ret;
+ break;
}
ret = snd_soc_component_open(component, substream);
if (ret < 0) {
+ snd_soc_component_module_put_when_close(component);
dev_err(component->dev,
"ASoC: can't open component %s: %d\n",
component->name, ret);
- return ret;
+ break;
}
}
- return 0;
+ if (ret < 0) {
+ /* rollback on error */
+ for_each_rtd_components(rtd, i, component) {
+ if (component == last)
+ break;
+
+ snd_soc_component_close(component, substream);
+ snd_soc_component_module_put_when_close(component);
+ }
+ }
+
+ return ret;
}
static int soc_pcm_components_close(struct snd_pcm_substream *substream)
@@ -585,7 +600,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
if (ret < 0) {
pr_err("ASoC: %s startup failed: %d\n",
rtd->dai_link->name, ret);
- goto component_err;
+ goto rtd_startup_err;
}
/* startup the audio subsystem */
@@ -681,9 +696,9 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
snd_soc_dai_shutdown(cpu_dai, substream);
soc_rtd_shutdown(rtd, substream);
-component_err:
+rtd_startup_err:
soc_pcm_components_close(substream);
-
+component_err:
mutex_unlock(&rtd->card->pcm_mutex);
for_each_rtd_components(rtd, i, component) {
--
2.17.1
^ permalink raw reply related
* t3513-revert-submodule.sh (3513.8) fails sometimes
From: Alex Riesen @ 2020-02-20 9:50 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2809 bytes --]
Hi,
the test fails for some reason, relatively rarely (20 per 100
invocations). I could not find the why, so I'm just posting the logs
below with the full log attached.
The test has been failing here for at least a year (the logs below done with
v2.25.1).
I have a config.mak:
USE_LIBPCRE2=YesPlease
OPENSSL_SHA256=YesPlease
CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
CFLAGS += -fstack-protector-strong
CFLAGS += -fpie
LDFLAGS += -z relro -z now
LDFLAGS += -pie
There local modifications, but none in the core core (small customizations in gitk).
The file system where the test is run on is an ext4 volume (an old, but still
usable ssd). The kernel is an v5.5.3. Debian without systemd. Not a container.
Not a VM.
expecting success of 3513.8 'git_revert: replace submodule containing a .git directory with a directory must fail':
prolog &&
reset_work_tree_to add_sub1 &&
(
cd submodule_update &&
git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
replace_gitfile_with_git_dir sub1 &&
test_must_fail $command replace_sub1_with_directory &&
test_superproject_content origin/add_sub1 &&
test_git_directory_is_unchanged sub1 &&
test_submodule_content sub1 origin/add_sub1
)
Cloning into 'submodule_update'...
done.
Branch 'add_sub1' set up to track remote branch 'add_sub1' from 'origin'.
Switched to a new branch 'add_sub1'
Submodule 'sub1' (/home/ari/compile/git/t/trash directory.t3513-revert-submodule/submodule_update_sub1) registered for path 'sub1'
Cloning into '/home/ari/compile/git/t/trash directory.t3513-revert-submodule/submodule_update/sub1'...
done.
Submodule path 'sub1': checked out '4a3c63c700bd465527a865db70efb925e380182c'
Branch 'replace_sub1_with_directory' set up to track remote branch 'replace_sub1_with_directory' from 'origin'.
error: The following untracked working tree files would be overwritten by checkout:
sub1/file1
sub1/file2
Please move or remove them before you switch branches.
Aborting
'actual' is not empty, it contains:
:100644 100644 587be6b4c3f93f93c489c0111bba5596147a26cb 0000000000000000000000000000000000000000 M file1
:100644 100644 975fbec8256d3e8a3797e7a3611380f27c49f4ac 0000000000000000000000000000000000000000 M file2
not ok 8 - git_revert: replace submodule containing a .git directory with a directory must fail
#
# prolog &&
# reset_work_tree_to add_sub1 &&
# (
# cd submodule_update &&
# git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
# replace_gitfile_with_git_dir sub1 &&
# test_must_fail $command replace_sub1_with_directory &&
# test_superproject_content origin/add_sub1 &&
# test_git_directory_is_unchanged sub1 &&
# test_submodule_content sub1 origin/add_sub1
# )
#
Regards,
Alex
[-- Attachment #2: t3513-revert-submodule.log.xz --]
[-- Type: application/x-xz, Size: 4708 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] crypto: hisilicon/sec2 - Add pbuffer mode for SEC driver
From: John Garry @ 2020-02-20 9:50 UTC (permalink / raw)
To: Zaibo Xu, herbert, davem
Cc: qianweili, tanghui20, forest.zhouchang, linuxarm, zhangwei375,
shenyang39, yekai13, linux-crypto
In-Reply-To: <1582189495-38051-5-git-send-email-xuzaibo@huawei.com>
On 20/02/2020 09:04, Zaibo Xu wrote:
> From: liulongfang <liulongfang@huawei.com>
>
> In the scenario of SMMU translation, the SEC performance of short messages
> (<512Bytes) cannot meet our expectations. To avoid this, we reserve the
> plat buffer (PBUF) memory for small packets when creating TFM.
>
I haven't gone through the code here, but why not use this method also
for non-translated? What are the pros and cons?
The commit message is very light on details.
Thanks
john
^ permalink raw reply
* Re: [PATCH 2/3] btrfs: export btrfs_uuid_scan_kthread
From: Johannes Thumshirn @ 2020-02-20 9:49 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs@vger.kernel.org
In-Reply-To: <20200218145609.13427-3-nborisov@suse.com>
I'd squash this into the next patch
^ permalink raw reply
* Re: Questions about logic_pio
From: Wei Xu @ 2020-02-20 9:48 UTC (permalink / raw)
To: Jiaxun Yang, yuanzhichang, john.garry, gabrielepaoloni, bhelgaas,
andyshevchenko
Cc: linux-kernel
In-Reply-To: <1705dbe62ce.10ae800394772.9222265269135747883@flygoat.com>
Hi Jiaxun,
On 2020/2/19 21:58, Jiaxun Yang wrote:
> Hi there,
>
> Logic PIO gives us a way to make indirect PIO access, however,
> the way it handles direct (MMIO) I/O access confused me.
>
> I was trying to create a PCI controller Driver and noticed that I/O range parsed
> from DeviceTree will be added to the Logic PIO range by logic_pio_register_range.
> And than PCI subsystem will use the ioport obtained from `logic_pio_trans_cpuaddr`
> to allocate resources for the host bridge. In my case, the range added to the logic pio
> was set as hw_start 0x4000, size 0x4000. Later, `logic_pio_trans_cpuaddr` called
> by `pci_address_to_pio` gives a ioport of 0x0, which is totally wrong.
>
> After dig into logic pio logic, I found that logic pio is trying to "allocate" an io_start
> for MMIO ranges, the allocation starts from 0x0. And later the io_start is used to calculate
> cpu_address. In my opinion, for direct MMIO access, logic_pio address should always
> equal to hw address, because there is no way to translate address from logic pio address
> to actual hw address in {in,out}{b,sb,w,sb,l,sl} operations.
>
> How this mechanism intends to work? What is the reason that we are trying to
> allocate a io_start for MMIO rather than take their hw_start ioport directly?
>
> Thanks.
Corrected John's mail address.
Maybe he can help.
Best Regards,
Wei
>
> --
> Jiaxun Yang
>
>
>
> .
>
^ permalink raw reply
* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Jiri Slaby @ 2020-02-20 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Richard Henderson, Ivan Kokshaysky,
Matt Turner, Alexey Brodkin, Vineet Gupta, James E.J. Bottomley,
Helge Deller, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Yoshinori Sato, Rich Felker, Dave Airlie,
David Airlie, Daniel Vetter, Ben Skeggs, Mauro Carvalho Chehab,
Nick Kossifidis, Luis Chamberlain, Kalle Valo, David S. Miller,
Dave Jiang, Jon Mason, Allen Hubbe, Michael S. Tsirkin,
Jason Wang, Arnd Bergmann, Geert Uytterhoeven, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-parisc, linuxppc-dev, linux-sh, dri-devel, nouveau,
linux-media, linux-wireless, netdev, linux-ntb, virtualization,
linux-arch
In-Reply-To: <20200219175007.13627-10-krzk@kernel.org>
On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> The ioreadX() helpers have inconsistent interface. On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> index 2c9cec8b53d9..8bd01df369fb 100644
> --- a/drivers/net/wireless/ath/ath5k/ahb.c
> +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
> if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> /* Enable WMAC AHB arbitration */
> - reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> + reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
While I understand why the parameter of ioread32 should be const, I
don't see a reason for these casts on the users' side. What does it
bring except longer code to read?
thanks,
--
js
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
^ permalink raw reply
* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Jiri Slaby @ 2020-02-20 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Richard Henderson, Ivan Kokshaysky,
Matt Turner, Alexey Brodkin, Vineet Gupta, James E.J. Bottomley,
Helge Deller, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Yoshinori Sato, Rich Felker, Dave Airlie,
David Airlie, Daniel Vetter, Ben Skeggs, Mauro Carvalho Chehab,
Nick Kossifidis, Luis Chamberlain, Kalle Valo, David S. Miller,
Dave Jiang, Jon Mason, Allen Hubbe, Michael S. Tsirkin,
Jason Wang, Arnd Bergmann, Geert Uytterhoeven, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-parisc, linuxppc-dev, linux-sh, dri-devel, nouveau,
linux-media, linux-wireless, netdev, linux-ntb, virtualization,
linux-arch
In-Reply-To: <20200219175007.13627-10-krzk@kernel.org>
On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> The ioreadX() helpers have inconsistent interface. On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> index 2c9cec8b53d9..8bd01df369fb 100644
> --- a/drivers/net/wireless/ath/ath5k/ahb.c
> +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
> if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> /* Enable WMAC AHB arbitration */
> - reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> + reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
While I understand why the parameter of ioread32 should be const, I
don't see a reason for these casts on the users' side. What does it
bring except longer code to read?
thanks,
--
js
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Jiri Slaby @ 2020-02-20 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Richard Henderson, Ivan Kokshaysky,
Matt Turner, Alexey Brodkin, Vineet Gupta, James E.J. Bottomley,
Helge Deller, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Yoshinori Sato, Rich Felker, Dave Airlie,
David Airlie, Daniel Vetter, Ben Skeggs, Mauro Carvalho Chehab,
Nick Kossifidis, Luis Chamberlain, Kalle Valo, David S. Miller,
Dave Jiang, Jon Mason, Allen Hubbe, Michael S. Tsirkin,
Jason Wang, Arnd Bergmann, Geert Uytterhoeven, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-parisc, linuxppc-dev, linux-sh, dri-devel, nouveau,
linux-media, linux-wireless, netdev, linux-ntb, virtualization,
linux-arch
In-Reply-To: <20200219175007.13627-10-krzk@kernel.org>
On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> The ioreadX() helpers have inconsistent interface. On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> index 2c9cec8b53d9..8bd01df369fb 100644
> --- a/drivers/net/wireless/ath/ath5k/ahb.c
> +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
> if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> /* Enable WMAC AHB arbitration */
> - reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> + reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
While I understand why the parameter of ioread32 should be const, I
don't see a reason for these casts on the users' side. What does it
bring except longer code to read?
thanks,
--
js
^ permalink raw reply
* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Jiri Slaby @ 2020-02-20 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Richard Henderson, Ivan Kokshaysky,
Matt Turner, Alexey Brodkin, Vineet Gupta, James E.J. Bottomley,
Helge Deller, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Yoshinori Sato, Rich Felker, Dave Airlie,
David Airlie, Daniel Vetter, Ben Skeggs, Mauro Carvalho Chehab,
Nick Kossifidis, Lui
In-Reply-To: <20200219175007.13627-10-krzk@kernel.org>
On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> The ioreadX() helpers have inconsistent interface. On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> index 2c9cec8b53d9..8bd01df369fb 100644
> --- a/drivers/net/wireless/ath/ath5k/ahb.c
> +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
> if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> /* Enable WMAC AHB arbitration */
> - reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> + reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
While I understand why the parameter of ioread32 should be const, I
don't see a reason for these casts on the users' side. What does it
bring except longer code to read?
thanks,
--
js
^ permalink raw reply
* Re: [RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)
From: Jiri Slaby @ 2020-02-20 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Richard Henderson, Ivan Kokshaysky,
Matt Turner, Alexey Brodkin, Vineet Gupta, James E.J. Bottomley,
Helge Deller, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Yoshinori Sato, Rich Felker, Dave Airlie,
David Airlie, Daniel Vetter, Ben Skeggs, Mauro Carvalho Chehab,
Nick Kossifidis, Luis Chamberlain, Kalle Valo, David S. Miller,
Dave Jiang
In-Reply-To: <20200219175007.13627-10-krzk@kernel.org>
On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> The ioreadX() helpers have inconsistent interface. On some architectures
> void *__iomem address argument is a pointer to const, on some not.
>
> Implementations of ioreadX() do not modify the memory under the address
> so they can be converted to a "const" version for const-safety and
> consistency among architectures.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> index 2c9cec8b53d9..8bd01df369fb 100644
> --- a/drivers/net/wireless/ath/ath5k/ahb.c
> +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
> if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> /* Enable WMAC AHB arbitration */
> - reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> + reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
While I understand why the parameter of ioread32 should be const, I
don't see a reason for these casts on the users' side. What does it
bring except longer code to read?
thanks,
--
js
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.