[PATCH] powerpc/imc-pmu: Fix IMC PMU code of using mutex in IRQs disabled section
Current imc-pmu code triggers a WARNING with CONFIG_DEBUG_ATOMIC_SLEEP and
CONFIG_PROVE_LOCKING enabled, while running a thread_imc event.
Command to trigger the warning:
[command]# perf stat -e thread_imc/CPM_CS_FROM_L4_MEM_X_DPTEG/ sleep 5
Performance counter stats for 'sleep 5':
0 thread_imc/CPM_CS_FROM_L4_MEM_X_DPTEG/
5.002117947 seconds time elapsed
0.000131000 seconds user
0.001063000 seconds sys
Below is snippet of the warning in dmesg:
[ 196.520838] BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:580
[ 196.521126] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 2869,
name: perf-exec
[ 196.521136] preempt_count: 2, expected: 0
[ 196.521143] 4 locks held by perf-exec/2869:
[ 196.521151] #0: c0004325c540 (>cred_guard_mutex){+.+.}-{3:3}, at:
bprm_execve+0x64/0xa90
[ 196.521173] #1: c0004325c5d8 (>exec_update_lock){}-{3:3}, at:
begin_new_exec+0x460/0xef0
[ 196.521192] #2: c003fa99d4e0 (_lock){-...}-{2:2}, at:
perf_event_exec+0x290/0x510
[ 196.521212] #3: c00017ab8418 (>lock){}-{2:2}, at:
perf_event_exec+0x29c/0x510
[ 196.521230] irq event stamp: 4806
[ 196.521237] hardirqs last enabled at (4805): []
_raw_spin_unlock_irqrestore+0x94/0xd0
[ 196.521248] hardirqs last disabled at (4806): []
perf_event_exec+0x394/0x510
[ 196.521260] softirqs last enabled at (0): []
copy_process+0xc34/0x1ff0
[ 196.521270] softirqs last disabled at (0): [<>] 0x0
[ 196.521281] CPU: 36 PID: 2869 Comm: perf-exec Not tainted
6.2.0-rc2-00011-g1247637727f2 #61
[ 196.521291] Hardware name: 8375-42A POWER9 0x4e1202 opal:v7.0-16-g9b85f7d961
PowerNV
[ 196.521300] Call Trace:
[ 196.521306] [c0004d213560] [c0f20c74] dump_stack_lvl+0x98/0xe0
(unreliable)
[ 196.521321] [c0004d2135a0] [c0194968] __might_resched+0x2f8/0x310
[ 196.521332] [c0004d213620] [c0f5a5dc] __mutex_lock+0x6c/0x13f0
[ 196.521344] [c0004d213740] [c0127a44]
thread_imc_event_add+0xf4/0x1b0
[ 196.521357] [c0004d2137c0] [c03ec930] event_sched_in+0xe0/0x210
[ 196.521369] [c0004d213810] [c03ed940] merge_sched_in+0x1f0/0x600
[ 196.521380] [c0004d213860] [c03ee00c]
visit_groups_merge.isra.92.constprop.166+0x2bc/0x6c0
[ 196.521393] [c0004d213920] [c03ee4dc]
ctx_flexible_sched_in+0xcc/0x140
[ 196.521405] [c0004d213980] [c03ee75c] ctx_sched_in+0x20c/0x2a0
[ 196.521416] [c0004d2139f0] [c03ee974] ctx_resched+0x104/0x1c0
[ 196.521427] [c0004d213a50] [c03fadf0] perf_event_exec+0x340/0x510
[ 196.521440] [c0004d213ac0] [c054a570] begin_new_exec+0x730/0xef0
[ 196.521451] [c0004d213b70] [c05dd7f8]
load_elf_binary+0x3f8/0x1e10
---
[ 196.522551] do not call blocking ops when !TASK_RUNNING; state=2001 set at
[] do_nanosleep+0x60/0x1a0
[ 196.522566] WARNING: CPU: 36 PID: 2869 at kernel/sched/core.c:9912
__might_sleep+0x9c/0xb0
[ 196.522575] Modules linked in: kvm_hv kvm vmx_crypto leds_powernv
powernv_op_panel led_class gf128mul crc32c_vpmsum fuse autofs4
[ 196.522594] CPU: 36 PID: 2869 Comm: sleep Tainted: GW
6.2.0-rc2-00011-g1247637727f2 #61
[ 196.522602] Hardware name: 8375-42A POWER9 0x4e1202 opal:v7.0-16-g9b85f7d961
PowerNV
[ 196.522609] NIP: c0194a1c LR: c0194a18 CTR: c0a78670
[ 196.522616] REGS: c0004d2134e0 TRAP: 0700 Tainted: GW
(6.2.0-rc2-00011-g1247637727f2)
[ 196.522624] MSR: 90021033 CR: 48002824
XER:
[ 196.522639] CFAR: c013fb64 IRQMASK: 1
[ 196.522639] GPR00: c0194a18 c0004d213780 c13c1700
006b
[ 196.522639] GPR04: fffe c0004d213540 c0004d213538
0027
---
The above warning triggered because current imc-pmu code uses mutex lock in
interrupt disabled sections. The function mutex_lock internally calls
"__might_resched" function, which will check if irq is disabled or not and
incase irq is disabled, it will trigger the warning. Patch fix this issue
by changing the mutex lock to spinlock.
Fixes: 8f95faaac56c ("powerpc/powernv: Detect and create IMC device")
Suggested-by: Michael Ellerman
Signed-off-by: Kajol Jain
---
arch/powerpc/include/asm/imc-pmu.h | 2 +-
arch/powerpc/perf/imc-pmu.c| 109 +++--
2 files changed, 56 insertions(+), 55 deletions(-)
diff --git a/arch/powerpc/include/asm/imc-pmu.h
b/arch/powerpc/include/asm/imc-pmu.h
index 4f897993b710..699a88584ae1 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -137,7 +137,7 @@ struct imc_pmu {
* are inited.
*/
struct imc_pmu_ref {
- struct mutex lock;
+ spinlock_t lock;
unsigned int id;
int refc;
};
diff --git a/arch/powerpc/perf/imc-pmu.c
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Thu, 2023-01-05 at 19:15 +1100, Andrew Donnellan wrote: > On Fri, 2022-12-30 at 15:20 +1100, Russell Currey wrote: > > The pseries platform can support dynamic secure boot (i.e. secure > > boot > > using user-defined keys) using variables contained with the PowerVM > > LPAR > > Platform KeyStore (PLPKS). Using the powerpc secvar API, expose > > the > > relevant variables for pseries dynamic secure boot through the > > existing > > secvar filesystem layout. > > > > The relevant variables for dynamic secure boot are signed in the > > keystore, and can only be modified using the H_PKS_SIGNED_UPDATE > > hcall. > > Object labels in the keystore are encoded using ucs2 format. With > > our > > fixed variable names we don't have to care about encoding outside > > of > > the > > necessary byte padding. > > > > When a user writes to a variable, the first 8 bytes of data must > > contain > > the signed update flags as defined by the hypervisor. > > > > When a user reads a variable, the first 4 bytes of data contain the > > policies defined for the object. > > > > Limitations exist due to the underlying implementation of sysfs > > binary > > attributes, as is the case for the OPAL secvar implementation - > > partial writes are unsupported and writes cannot be larger than > > PAGE_SIZE. > > The PAGE_SIZE limit, in practice, will be a major limitation with 4K > pages (we expect several of the variables to regularly be larger than > 4K) but won't be much of an issue for 64K (that's all the storage > space > the hypervisor will give you anyway). > > In a previous internal version, we printed a message when PAGE_SIZE < > plpks_get_maxobjectsize(), might be worth still doing that? Yeah, we should do that in the secvar code. The same limitation applies on the powernv side as well. > > > > > Co-developed-by: Nayna Jain > > Signed-off-by: Nayna Jain > > Co-developed-by: Andrew Donnellan > > Signed-off-by: Andrew Donnellan > > Signed-off-by: Russell Currey > > Some minor comments for v3 on a patch which already carries my > signoff... > > > --- > > v2: Remove unnecessary config vars from sysfs and document the > > others, > > thanks to review from Greg. If we end up needing to expose > > more, > > we > > can add them later and update the docs. > > > > Use sysfs_emit() instead of sprintf(), thanks to Greg. > > > > Change the size of the sysfs binary attributes to include the > > 8- > > byte > > flags header, preventing truncation of large writes. > > > > Documentation/ABI/testing/sysfs-secvar | 67 - > > arch/powerpc/platforms/pseries/Kconfig | 13 + > > arch/powerpc/platforms/pseries/Makefile | 4 +- > > arch/powerpc/platforms/pseries/plpks-secvar.c | 245 > > ++ > > 4 files changed, 326 insertions(+), 3 deletions(-) > > create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c > > > > diff --git a/Documentation/ABI/testing/sysfs-secvar > > b/Documentation/ABI/testing/sysfs-secvar > > index feebb8c57294..466a8cb92b92 100644 > > --- a/Documentation/ABI/testing/sysfs-secvar > > +++ b/Documentation/ABI/testing/sysfs-secvar > > @@ -34,7 +34,7 @@ Description: An integer representation of the > > size > > of the content of the > > > > What: /sys/firmware/secvar/vars//data > > Date: August 2019 > > -Contact: Nayna Jain h > > +Contact: Nayna Jain > > Description: A read-only file containing the value of the > > variable. The size > > of the file represents the maximum size of the > > variable data. > > > > @@ -44,3 +44,68 @@ Contact: Nayna Jain > > Description: A write-only file that is used to submit the new > > value for the > > variable. The size of the file represents the > > maximum > > size of > > the variable data that can be written. > > + > > +What: /sys/firmware/secvar/config > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: This optional directory contains read-only config > > attributes as > > + defined by the secure variable implementation. All > > data is in > > + ASCII format. The directory is only created if the > > backing > > + implementation provides variables to populate it, > > which at > > + present is only PLPKS on the pseries platform. > > + > > +What: /sys/firmware/secvar/config/version > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: RO file, only present if the secvar implementation > > is > > PLPKS. > > + > > + Contains the config version as reported by the > > hypervisor in > > + ASCII decimal format. > > + > > +What: /sys/firmware/secvar/config/max_object_size > > +Date: December 2022 > > +Contact: Nayna Jain > > +Description: RO file, only present if the secvar implementation > > is > > PLPKS. > > + > > +
Re: [PATCH v2 6/7] powerpc/secvar: Extend sysfs to include config vars
On Fri, 2023-01-06 at 15:15 +1100, Michael Ellerman wrote:
> Russell Currey writes:
> > The forthcoming pseries consumer of the secvar API wants to expose
> > a
> > number of config variables. Allowing secvar implementations to
> > provide
> > their own sysfs attributes makes it easy for consumers to expose
> > what
> > they need to.
> >
> > This is not being used by the OPAL secvar implementation at
> > present, and
> > the config directory will not be created if no attributes are set.
>
> Would it be slightly cleaner if the attributes were just a member of
> secvar_operations?
>
> That would avoid the need for some of the separate handling of the
> ops
> and the attributes.
>
> I know "ops" implies it's only methods, but I think that's not a big
> problem. The power_pmu struct does something similar, where it
> combines
> ops and attributes.
Yeah that does seem easier, thanks for the suggestion.
>
> cheers
>
> > diff --git a/arch/powerpc/include/asm/secvar.h
> > b/arch/powerpc/include/asm/secvar.h
> > index 92d2c051918b..250e7066b6da 100644
> > --- a/arch/powerpc/include/asm/secvar.h
> > +++ b/arch/powerpc/include/asm/secvar.h
> > @@ -10,6 +10,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > extern const struct secvar_operations *secvar_ops;
> >
> > @@ -27,10 +28,12 @@ struct secvar_operations {
> > #ifdef CONFIG_PPC_SECURE_BOOT
> >
> > extern void set_secvar_ops(const struct secvar_operations *ops);
> > +extern void set_secvar_config_attrs(const struct attribute
> > **attrs);
> >
> > #else
> >
> > static inline void set_secvar_ops(const struct secvar_operations
> > *ops) { }
> > +static inline void set_secvar_config_attrs(const struct attribute
> > **attrs) { }
> >
> > #endif
> >
> > diff --git a/arch/powerpc/kernel/secvar-sysfs.c
> > b/arch/powerpc/kernel/secvar-sysfs.c
> > index aa1daec480e1..ad1e1d72d2ae 100644
> > --- a/arch/powerpc/kernel/secvar-sysfs.c
> > +++ b/arch/powerpc/kernel/secvar-sysfs.c
> > @@ -15,9 +15,17 @@
> >
> > #define NAME_MAX_SIZE 1024
> >
> > +const struct attribute **secvar_config_attrs __ro_after_init =
> > NULL;
> > +
> > static struct kobject *secvar_kobj;
> > static struct kset *secvar_kset;
> >
> > +void set_secvar_config_attrs(const struct attribute **attrs)
> > +{
> > + WARN_ON_ONCE(secvar_config_attrs);
> > + secvar_config_attrs = attrs;
> > +}
> > +
> > static ssize_t format_show(struct kobject *kobj, struct
> > kobj_attribute *attr,
> > char *buf)
> > {
> > @@ -134,6 +142,16 @@ static int update_kobj_size(void)
> > return 0;
> > }
> >
> > +static int secvar_sysfs_config(struct kobject *kobj)
> > +{
> > + struct attribute_group config_group = {
> > + .name = "config",
> > + .attrs = (struct attribute **)secvar_config_attrs,
> > + };
> > +
> > + return sysfs_create_group(kobj, _group);
> > +}
> > +
> > static int secvar_sysfs_load(void)
> > {
> > char *name;
> > @@ -196,26 +214,38 @@ static int secvar_sysfs_init(void)
> >
> > rc = sysfs_create_file(secvar_kobj, _attr.attr);
> > if (rc) {
> > - kobject_put(secvar_kobj);
> > - return -ENOMEM;
> > + pr_err("secvar: Failed to create format object\n");
> > + rc = -ENOMEM;
> > + goto err;
> > }
> >
> > secvar_kset = kset_create_and_add("vars", NULL,
> > secvar_kobj);
> > if (!secvar_kset) {
> > pr_err("secvar: sysfs kobject registration
> > failed.\n");
> > - kobject_put(secvar_kobj);
> > - return -ENOMEM;
> > + rc = -ENOMEM;
> > + goto err;
> > }
> >
> > rc = update_kobj_size();
> > if (rc) {
> > pr_err("Cannot read the size of the attribute\n");
> > - return rc;
> > + goto err;
> > + }
> > +
> > + if (secvar_config_attrs) {
> > + rc = secvar_sysfs_config(secvar_kobj);
> > + if (rc) {
> > + pr_err("secvar: Failed to create config
> > directory\n");
> > + goto err;
> > + }
> > }
> >
> > secvar_sysfs_load();
> >
> > return 0;
> > +err:
> > + kobject_put(secvar_kobj);
> > + return rc;
> > }
> >
> > late_initcall(secvar_sysfs_init);
> > --
> > 2.38.1
Re: [PATCH v2 6/7] powerpc/secvar: Extend sysfs to include config vars
On Thu, 2023-01-05 at 18:28 +1100, Andrew Donnellan wrote:
> On Fri, 2022-12-30 at 15:20 +1100, Russell Currey wrote:
> > The forthcoming pseries consumer of the secvar API wants to expose
> > a
> > number of config variables. Allowing secvar implementations to
> > provide
> > their own sysfs attributes makes it easy for consumers to expose
> > what
> > they need to.
> >
> > This is not being used by the OPAL secvar implementation at
> > present,
> > and
> > the config directory will not be created if no attributes are set.
> >
> > Signed-off-by: Russell Currey
>
> Minor comments below, but regardless:
>
> Reviewed-by: Andrew Donnellan
>
> > ---
> > I played around with adding an API call to facilitate a more
> > generic
> > key/value interface for config variables and it seemed like
> > unnecessary
> > complexity. I think this is cleaner. If there was ever a secvar
> > interface other than sysfs we'd have to rework it, though.
>
> I concur, this can be dealt with if/when the secvar interface is
> exposed by some other means than sysfs.
>
> >
> > arch/powerpc/include/asm/secvar.h | 3 +++
> > arch/powerpc/kernel/secvar-sysfs.c | 40
> > ++--
> > --
> > 2 files changed, 38 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/secvar.h
> > b/arch/powerpc/include/asm/secvar.h
> > index 92d2c051918b..250e7066b6da 100644
> > --- a/arch/powerpc/include/asm/secvar.h
> > +++ b/arch/powerpc/include/asm/secvar.h
> > @@ -10,6 +10,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > extern const struct secvar_operations *secvar_ops;
> >
> > @@ -27,10 +28,12 @@ struct secvar_operations {
> > #ifdef CONFIG_PPC_SECURE_BOOT
> >
> > extern void set_secvar_ops(const struct secvar_operations *ops);
> > +extern void set_secvar_config_attrs(const struct attribute
> > **attrs);
> >
> > #else
> >
> > static inline void set_secvar_ops(const struct secvar_operations
> > *ops) { }
> > +static inline void set_secvar_config_attrs(const struct attribute
> > **attrs) { }
> >
> > #endif
> >
> > diff --git a/arch/powerpc/kernel/secvar-sysfs.c
> > b/arch/powerpc/kernel/secvar-sysfs.c
> > index aa1daec480e1..ad1e1d72d2ae 100644
> > --- a/arch/powerpc/kernel/secvar-sysfs.c
> > +++ b/arch/powerpc/kernel/secvar-sysfs.c
> > @@ -15,9 +15,17 @@
> >
> > #define NAME_MAX_SIZE 1024
> >
> > +const struct attribute **secvar_config_attrs __ro_after_init =
> > NULL;
> > +
> > static struct kobject *secvar_kobj;
> > static struct kset *secvar_kset;
> >
> > +void set_secvar_config_attrs(const struct attribute **attrs)
> > +{
> > + WARN_ON_ONCE(secvar_config_attrs);
> > + secvar_config_attrs = attrs;
> > +}
> > +
> > static ssize_t format_show(struct kobject *kobj, struct
> > kobj_attribute *attr,
> > char *buf)
> > {
> > @@ -134,6 +142,16 @@ static int update_kobj_size(void)
> > return 0;
> > }
> >
> > +static int secvar_sysfs_config(struct kobject *kobj)
> > +{
> > + struct attribute_group config_group = {
> > + .name = "config",
> > + .attrs = (struct attribute **)secvar_config_attrs,
> > + };
>
> I was slightly concerned that you're putting this on the stack, but
> it
> doesn't appear that sysfs_create_group() keeps any references to the
> group around after it creates all the files, so I think this is fine.
>
> > +
> > + return sysfs_create_group(kobj, _group);
> > +}
> > +
> > static int secvar_sysfs_load(void)
> > {
> > char *name;
> > @@ -196,26 +214,38 @@ static int secvar_sysfs_init(void)
> >
> > rc = sysfs_create_file(secvar_kobj, _attr.attr);
> > if (rc) {
> > - kobject_put(secvar_kobj);
> > - return -ENOMEM;
> > + pr_err("secvar: Failed to create format object\n");
>
> This file defines pr_fmt, so the secvar: prefix here can go away,
> though I notice that is the case for all the existing prints in this
> function too.
Yeah we should fix that for all of them, good catch.
>
> > + rc = -ENOMEM;
> > + goto err;
> > }
> >
> > secvar_kset = kset_create_and_add("vars", NULL,
> > secvar_kobj);
> > if (!secvar_kset) {
> > pr_err("secvar: sysfs kobject registration
> > failed.\n");
> > - kobject_put(secvar_kobj);
> > - return -ENOMEM;
> > + rc = -ENOMEM;
> > + goto err;
> > }
> >
> > rc = update_kobj_size();
> > if (rc) {
> > pr_err("Cannot read the size of the attribute\n");
> > - return rc;
> > + goto err;
> > + }
> > +
> > + if (secvar_config_attrs) {
> > + rc = secvar_sysfs_config(secvar_kobj);
> > + if (rc) {
> > + pr_err("secvar: Failed to create config
> > directory\n");
>
> Same comment as above
>
> > +
Re: [PATCH v2 6/7] powerpc/secvar: Extend sysfs to include config vars
Russell Currey writes:
> The forthcoming pseries consumer of the secvar API wants to expose a
> number of config variables. Allowing secvar implementations to provide
> their own sysfs attributes makes it easy for consumers to expose what
> they need to.
>
> This is not being used by the OPAL secvar implementation at present, and
> the config directory will not be created if no attributes are set.
Would it be slightly cleaner if the attributes were just a member of
secvar_operations?
That would avoid the need for some of the separate handling of the ops
and the attributes.
I know "ops" implies it's only methods, but I think that's not a big
problem. The power_pmu struct does something similar, where it combines
ops and attributes.
cheers
> diff --git a/arch/powerpc/include/asm/secvar.h
> b/arch/powerpc/include/asm/secvar.h
> index 92d2c051918b..250e7066b6da 100644
> --- a/arch/powerpc/include/asm/secvar.h
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -10,6 +10,7 @@
>
> #include
> #include
> +#include
>
> extern const struct secvar_operations *secvar_ops;
>
> @@ -27,10 +28,12 @@ struct secvar_operations {
> #ifdef CONFIG_PPC_SECURE_BOOT
>
> extern void set_secvar_ops(const struct secvar_operations *ops);
> +extern void set_secvar_config_attrs(const struct attribute **attrs);
>
> #else
>
> static inline void set_secvar_ops(const struct secvar_operations *ops) { }
> +static inline void set_secvar_config_attrs(const struct attribute **attrs) {
> }
>
> #endif
>
> diff --git a/arch/powerpc/kernel/secvar-sysfs.c
> b/arch/powerpc/kernel/secvar-sysfs.c
> index aa1daec480e1..ad1e1d72d2ae 100644
> --- a/arch/powerpc/kernel/secvar-sysfs.c
> +++ b/arch/powerpc/kernel/secvar-sysfs.c
> @@ -15,9 +15,17 @@
>
> #define NAME_MAX_SIZE 1024
>
> +const struct attribute **secvar_config_attrs __ro_after_init = NULL;
> +
> static struct kobject *secvar_kobj;
> static struct kset *secvar_kset;
>
> +void set_secvar_config_attrs(const struct attribute **attrs)
> +{
> + WARN_ON_ONCE(secvar_config_attrs);
> + secvar_config_attrs = attrs;
> +}
> +
> static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr,
> char *buf)
> {
> @@ -134,6 +142,16 @@ static int update_kobj_size(void)
> return 0;
> }
>
> +static int secvar_sysfs_config(struct kobject *kobj)
> +{
> + struct attribute_group config_group = {
> + .name = "config",
> + .attrs = (struct attribute **)secvar_config_attrs,
> + };
> +
> + return sysfs_create_group(kobj, _group);
> +}
> +
> static int secvar_sysfs_load(void)
> {
> char *name;
> @@ -196,26 +214,38 @@ static int secvar_sysfs_init(void)
>
> rc = sysfs_create_file(secvar_kobj, _attr.attr);
> if (rc) {
> - kobject_put(secvar_kobj);
> - return -ENOMEM;
> + pr_err("secvar: Failed to create format object\n");
> + rc = -ENOMEM;
> + goto err;
> }
>
> secvar_kset = kset_create_and_add("vars", NULL, secvar_kobj);
> if (!secvar_kset) {
> pr_err("secvar: sysfs kobject registration failed.\n");
> - kobject_put(secvar_kobj);
> - return -ENOMEM;
> + rc = -ENOMEM;
> + goto err;
> }
>
> rc = update_kobj_size();
> if (rc) {
> pr_err("Cannot read the size of the attribute\n");
> - return rc;
> + goto err;
> + }
> +
> + if (secvar_config_attrs) {
> + rc = secvar_sysfs_config(secvar_kobj);
> + if (rc) {
> + pr_err("secvar: Failed to create config directory\n");
> + goto err;
> + }
> }
>
> secvar_sysfs_load();
>
> return 0;
> +err:
> + kobject_put(secvar_kobj);
> + return rc;
> }
>
> late_initcall(secvar_sysfs_init);
> --
> 2.38.1
Re: [PATCH] ASoC: fsl_micfil: Correct the number of steps on SX controls
On Wed, Jan 4, 2023 at 10:58 AM Chancel Liu wrote:
> The parameter "max" of SOC_SINGLE_SX_TLV() means the number of steps
> rather than maximum value. This patch corrects the minimum value to -8
> and the number of steps to 15.
>
> Signed-off-by: Chancel Liu
>
Acked-by: Shengjiu Wang
Best regards
Wang Shengjiu
> ---
> sound/soc/fsl/fsl_micfil.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
> index d7e6fc996166..d4d3bc7ca87c 100644
> --- a/sound/soc/fsl/fsl_micfil.c
> +++ b/sound/soc/fsl/fsl_micfil.c
> @@ -325,21 +325,21 @@ static int hwvad_detected(struct snd_kcontrol
> *kcontrol,
>
> static const struct snd_kcontrol_new fsl_micfil_snd_controls[] = {
> SOC_SINGLE_SX_TLV("CH0 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(0), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(0), 0x8, 0xF, gain_tlv),
> SOC_SINGLE_SX_TLV("CH1 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(1), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(1), 0x8, 0xF, gain_tlv),
> SOC_SINGLE_SX_TLV("CH2 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(2), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(2), 0x8, 0xF, gain_tlv),
> SOC_SINGLE_SX_TLV("CH3 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(3), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(3), 0x8, 0xF, gain_tlv),
> SOC_SINGLE_SX_TLV("CH4 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(4), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(4), 0x8, 0xF, gain_tlv),
> SOC_SINGLE_SX_TLV("CH5 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(5), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(5), 0x8, 0xF, gain_tlv),
> SOC_SINGLE_SX_TLV("CH6 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(6), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(6), 0x8, 0xF, gain_tlv),
> SOC_SINGLE_SX_TLV("CH7 Volume", REG_MICFIL_OUT_CTRL,
> - MICFIL_OUTGAIN_CHX_SHIFT(7), 0xF, 0x7, gain_tlv),
> + MICFIL_OUTGAIN_CHX_SHIFT(7), 0x8, 0xF, gain_tlv),
> SOC_ENUM_EXT("MICFIL Quality Select",
> fsl_micfil_quality_enum,
> micfil_quality_get, micfil_quality_set),
> --
> 2.25.1
>
>
Re: [PATCH v7 2/2] arm64: support batched/deferred tlb shootdown during page reclamation
On Thu, Nov 17, 2022 at 04:26:48PM +0800, Yicong Yang wrote:
> It is tested on 4,8,128 CPU platforms and shows to be beneficial on
> large systems but may not have improvement on small systems like on
> a 4 CPU platform. So make ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH depends
> on CONFIG_EXPERT for this stage and make this disabled on systems
> with less than 8 CPUs. User can modify this threshold according to
> their own platforms by CONFIG_NR_CPUS_FOR_BATCHED_TLB.
What's the overhead of such batching on systems with 4 or fewer CPUs? If
it isn't noticeable, I'd rather have it always on than some number
chosen on whichever SoC you tested.
Another option would be to make this a sysctl tunable.
> .../features/vm/TLB/arch-support.txt | 2 +-
> arch/arm64/Kconfig| 6 +++
> arch/arm64/include/asm/tlbbatch.h | 12 +
> arch/arm64/include/asm/tlbflush.h | 52 ++-
> arch/x86/include/asm/tlbflush.h | 5 +-
> include/linux/mm_types_task.h | 4 +-
> mm/rmap.c | 10 ++--
Please keep any function prototype changes in a preparatory patch so
that the arm64 one only introduces the arch specific changes. Easier to
review.
> +static inline bool arch_tlbbatch_should_defer(struct mm_struct *mm)
> +{
> + /*
> + * TLB batched flush is proved to be beneficial for systems with large
> + * number of CPUs, especially system with more than 8 CPUs. TLB shutdown
> + * is cheap on small systems which may not need this feature. So use
> + * a threshold for enabling this to avoid potential side effects on
> + * these platforms.
> + */
> + if (num_online_cpus() < CONFIG_ARM64_NR_CPUS_FOR_BATCHED_TLB)
> + return false;
The x86 implementation tracks the cpumask of where a task has run. We
don't have such tracking on arm64 and I don't think it matters. As
noticed/described in this series, the bottleneck is the actual DSB
synchronisation (which sends a DVM Sync message to all the other CPUs
and waits for a DVM Complete response). So I think it makes sense not to
bother with an mm_cpumask(). What this patch aims to optimise is
actually the number of DSBs issued on an SMP system by
ptep_clear_flush().
The DVM is not an architected concept (well, it's part of AMBA AXI). I'd
be curious to know how such patch behaves on Apple's M1/M2 hardware. My
preference would be to have this always on for num_online_cpus() > 1 if
there's no overhead.
--
Catalin
Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg
On Thu, Dec 29, 2022 at 10:54:50AM +0100, Andrzej Hajda wrote: > Forgive me late response - Holidays, > > On 22.12.2022 18:21, Andrew Morton wrote: > > On Thu, 22 Dec 2022 12:46:16 +0100 Andrzej Hajda > > wrote: > > > > > Hi all, > > > > > > I hope there will be place for such tiny helper in kernel. > > > Quick cocci analyze shows there is probably few thousands places > > > where it could be useful. > > So to clarify, the intent here is a simple readability cleanup for > > existing open-coded exchange operations. > > And replace private helpers with common one, see the last patch - the > ultimate goal > would be to replace all occurrences of fetch_and_zero with __xchg. > > > The intent is *not* to > > identify existing xchg() sites which are unnecessarily atomic and to > > optimize them by using the non-atomic version. > > > > Have you considered the latter? > > If you mean some way of (semi-)automatic detection of such cases, then no. > Anyway this could be quite interesting challenge. My take is that unless there is very clear demand for this macro from outside of i915, it's not worth it. All that fetch_and_zero zero achieved is make i915 code a lot more confusing to read for people who don't know this thing. And it replaces 2 entirely standard lines of 0, every often clearing pointers in data structures where you really want the verbosity to have a reminder and thinking about the locking. Plus it smells way too much like the cmpxchg family of atomic functions, addig further to the locking confuion. Imo the right approach is to just open code this macro in i915 and then drop it. Again, unless enough people outside of i915 really really want this, and want to lift this to a kernel idiom. -Daniel > > > > > > I am not sure who is good person to review/ack such patches, > > I can take 'em. > > > > > so I've used my intuition to construct to/cc lists, sorry for mistakes. > > > This is the 2nd approach of the same idea, with comments addressed[0]. > > > > > > The helper is tiny and there are advices we can leave without it, so > > > I want to present few arguments why it would be good to have it: > > > > > > 1. Code readability/simplification/number of lines: > > > > > > Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c: > > > - previous_min_rate = evport->qos.min_rate; > > > - evport->qos.min_rate = min_rate; > > > + previous_min_rate = __xchg(evport->qos.min_rate, min_rate); > > > > > > For sure the code is more compact, and IMHO more readable. > > > > > > 2. Presence of similar helpers in other somehow related languages/libs: > > > > > > a) Rust[1]: 'replace' from std::mem module, there is also 'take' > > > helper (__xchg(, 0)), which is the same as private helper in > > > i915 - fetch_and_zero, see latest patch. > > > b) C++ [2]: 'exchange' from utility header. > > > > > > If the idea is OK there are still 2 qestions to answer: > > > > > > 1. Name of the helper, __xchg follows kernel conventions, > > > but for me Rust names are also OK. > > I like replace(), or, shockingly, exchange(). > > > > But... Can we simply make swap() return the previous value? > > > > previous_min_rate = swap(>qos.min_rate, min_rate); > > As Alexander already pointed out, swap requires 'references' to two > variables, > in contrast to xchg which requires reference to variable and value. > So we cannot use swap for cases: > old_value = __xchg(, new_value); > > Regards > Andrzej > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 2/3] powerpc/vmlinux.lds: Don't discard .rela* for relocatable builds
Relocatable kernels must not discard relocations, they need to be
processed at runtime. As such they are included for CONFIG_RELOCATABLE
builds in the powerpc linker script (line 340).
However they are also unconditionally discarded later in the
script (line 414). Previously that worked because the earlier inclusion
superseded the discard.
However commit 99cb0d917ffa ("arch: fix broken BuildID for arm64 and
riscv") introduced an earlier use of DISCARD as part of the RO_DATA
macro (line 137). With binutils < 2.36 that causes the DISCARD
directives later in the script to be applied earlier, causing .rela* to
actually be discarded at link time, leading to build warnings and a
kernel that doesn't boot:
ld: warning: discarding dynamic section .rela.init.rodata
Fix it by conditionally discarding .rela* only when CONFIG_RELOCATABLE
is disabled.
Fixes: 99cb0d917ffa ("arch: fix broken BuildID for arm64 and riscv")
Signed-off-by: Michael Ellerman
---
arch/powerpc/kernel/vmlinux.lds.S | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S
b/arch/powerpc/kernel/vmlinux.lds.S
index c5ea7d03d539..a4c6efadc90c 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -411,9 +411,12 @@ SECTIONS
DISCARDS
/DISCARD/ : {
*(*.EMB.apuinfo)
- *(.glink .iplt .plt .rela* .comment)
+ *(.glink .iplt .plt .comment)
*(.gnu.version*)
*(.gnu.attributes)
*(.eh_frame)
+#ifndef CONFIG_RELOCATABLE
+ *(.rela*)
+#endif
}
}
--
2.39.0
[PATCH 1/3] powerpc/vmlinux.lds: Define RUNTIME_DISCARD_EXIT
The powerpc linker script explicitly includes .exit.text, because
otherwise the link fails due to references from __bug_table and
__ex_table. The code is freed (discarded) at runtime along with
.init.text and data.
That has worked in the past despite powerpc not defining
RUNTIME_DISCARD_EXIT because DISCARDS appears late in the powerpc linker
script (line 410), and the explicit inclusion of .exit.text
earlier (line 280) supersedes the discard.
However commit 99cb0d917ffa ("arch: fix broken BuildID for arm64 and
riscv") introduced an earlier use of DISCARD as part of the RO_DATA
macro (line 136). With binutils < 2.36 that causes the DISCARD
directives later in the script to be applied earlier [1], causing
.exit.text to actually be discarded at link time, leading to build
errors:
'.exit.text' referenced in section '__bug_table' of crypto/algboss.o: defined
in
discarded section '.exit.text' of crypto/algboss.o
'.exit.text' referenced in section '__ex_table' of drivers/nvdimm/core.o:
defined in
discarded section '.exit.text' of drivers/nvdimm/core.o
Fix it by defining RUNTIME_DISCARD_EXIT, which causes the generic
DISCARDS macro to not include .exit.text at all.
1: https://lore.kernel.org/lkml/87fscp2v7k@igel.home/
Fixes: 99cb0d917ffa ("arch: fix broken BuildID for arm64 and riscv")
Signed-off-by: Michael Ellerman
---
arch/powerpc/kernel/vmlinux.lds.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S
b/arch/powerpc/kernel/vmlinux.lds.S
index 8c3862b4c259..c5ea7d03d539 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -8,6 +8,7 @@
#define BSS_FIRST_SECTIONS *(.bss.prominit)
#define EMITS_PT_NOTE
#define RO_EXCEPTION_TABLE_ALIGN 0
+#define RUNTIME_DISCARD_EXIT
#define SOFT_MASK_TABLE(align) \
. = ALIGN(align); \
--
2.39.0
[PATCH 3/3] powerpc/vmlinux.lds: Don't discard .comment
Although the powerpc linker script mentions .comment in the DISCARD
section, that has never actually caused it to be discarded, because the
earlier ELF_DETAILS macro (previously STABS_DEBUG) explicitly includes
.comment.
However commit 99cb0d917ffa ("arch: fix broken BuildID for arm64 and
riscv") introduced an earlier use of DISCARD as part of the RO_DATA
macro. With binutils < 2.36 that causes the DISCARD directives later in
the script to be applied earlier, causing .comment to actually be
discarded.
It's confusing to explicitly include and discard .comment, and even more
so if the behaviour depends on the toolchain version. So don't discard
.comment in order to maintain the existing behaviour in all cases.
Fixes: 83a092cf95f2 ("powerpc: Link warning for orphan sections")
Signed-off-by: Michael Ellerman
---
arch/powerpc/kernel/vmlinux.lds.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S
b/arch/powerpc/kernel/vmlinux.lds.S
index a4c6efadc90c..958e77a24f85 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -411,7 +411,7 @@ SECTIONS
DISCARDS
/DISCARD/ : {
*(*.EMB.apuinfo)
- *(.glink .iplt .plt .comment)
+ *(.glink .iplt .plt)
*(.gnu.version*)
*(.gnu.attributes)
*(.eh_frame)
--
2.39.0
Re: [PATCH 2/2] perf test bpf: Skip test if kernel-debuginfo is not present
Em Thu, Jan 05, 2023 at 05:47:42PM +0530, Athira Rajeev escreveu:
> Perf BPF filter test fails in environment where "kernel-debuginfo"
> is not installed.
I'll apply this to perf/core, for the next merge window, as its more an
improvement than a fix, i.e. we know why it fails, we're just improving
the user reporting to make that clear at first sight.
- Arnaldo
> Test failure logs:
> <<>>
> 42: BPF filter:
> 42.1: Basic BPF filtering : Ok
> 42.2: BPF pinning : Ok
> 42.3: BPF prologue generation : FAILED!
> <<>>
>
> Enabling verbose option provided debug logs, which says debuginfo
> needs to be installed. Snippet of verbose logs:
>
> <<>>
> 42.3: BPF prologue generation :
> --- start ---
> test child forked, pid 28218
> <<>>
> Rebuild with CONFIG_DEBUG_INFO=y, or install an appropriate debuginfo
> package.
> bpf_probe: failed to convert perf probe events
> Failed to add events selected by BPF
> test child finished with -1
> end
> BPF filter subtest 3: FAILED!
> <<>>
>
> Here subtest, "BPF prologue generation" failed and
> logs shows debuginfo is needed. After installing
> kernel-debuginfo package, testcase passes.
>
> Subtest "BPF prologue generation" failed because, the "do_test"
> function returns "TEST_FAIL" without checking the error type
> returned by "parse_events_load_bpf_obj" function.
> Function parse_events_load_bpf_obj can also return error of type
> "-ENODATA" incase kernel-debuginfo package is not installed. Fix this
> by adding check for -ENODATA error.
>
> Test result after the patch changes:
>
> Test failure logs:
> <<>>
> 42: BPF filter :
> 42.1: Basic BPF filtering : Ok
> 42.2: BPF pinning : Ok
> 42.3: BPF prologue generation : Skip (clang/debuginfo isn't
> installed or environment missing BPF support)
>
> Fixes: ba1fae431e74 ("perf test: Add 'perf test BPF'")
> Signed-off-by: Athira Rajeev
> ---
> Note: This is dependent on patch 1:
> tools/perf: Update the exit error codes in function
> try_to_find_probe_trace_event
>
> tools/perf/tests/bpf.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> index 17c023823713..6a4235a9cf57 100644
> --- a/tools/perf/tests/bpf.c
> +++ b/tools/perf/tests/bpf.c
> @@ -126,6 +126,10 @@ static int do_test(struct bpf_object *obj, int
> (*func)(void),
>
> err = parse_events_load_bpf_obj(_state, _state.list, obj,
> NULL);
> parse_events_error__exit(_error);
> + if (err == -ENODATA) {
> + pr_debug("Failed to add events selected by BPF, debuginfo
> package not installed\n");
> + return TEST_SKIP;
> + }
> if (err || list_empty(_state.list)) {
> pr_debug("Failed to add events selected by BPF\n");
> return TEST_FAIL;
> @@ -368,7 +372,7 @@ static struct test_case bpf_tests[] = {
> "clang isn't installed or environment missing BPF
> support"),
> #ifdef HAVE_BPF_PROLOGUE
> TEST_CASE_REASON("BPF prologue generation", bpf_prologue_test,
> - "clang isn't installed or environment missing BPF
> support"),
> + "clang/debuginfo isn't installed or environment missing
> BPF support"),
> #else
> TEST_CASE_REASON("BPF prologue generation", bpf_prologue_test, "not
> compiled in"),
> #endif
> --
> 2.31.1
--
- Arnaldo
Re: [PATCH] tools/perf: Fix bpf-script-test-prologue test compile issue with clang
Em Thu, Jan 05, 2023 at 05:34:36PM +0530, Athira Rajeev escreveu:
> While running perf test for bpf, observed that "BPF prologue
> generation" test case fails to compile with clang. Logs below
> from powerpc:
>
> :33:2: error: use of undeclared identifier 'fmode_t'
> fmode_t f_mode = (fmode_t)_f_mode;
> ^
> :37:6: error: use of undeclared identifier 'f_mode'; did you mean
> '_f_mode'?
> if (f_mode & FMODE_WRITE)
> ^~
> _f_mode
> :30:60: note: '_f_mode' declared here
> int bpf_func__null_lseek(void *ctx, int err, unsigned long _f_mode,
>^
> 2 errors generated.
Thanks for fixing this, I noticed the problem but didn't got around to
investigate it.
Tested and applied.
- Arnaldo
> The test code tests/bpf-script-test-prologue.c uses fmode_t.
> And the error above is for "fmode_t" which is defined in
> include/linux/types.h as part of kernel build directory:
> "/lib/modules//build" that comes from
> kernel devel [ soft link to /usr/src/ ].
> Clang picks this header file from "-working-directory" build
> option that specifies this build folder.
>
> But the 'commit 14e4b9f4289a ("perf trace: Raw augmented
> syscalls fix libbpf 1.0+ compatibility")', changed the
> include directory to use: "/usr/include". Post this change,
> types.h from /usr/include/ is getting picked upwhich doesn’t
> contain definition of "fmode_t" and hence fails to compile.
>
> Compilation command before this commit:
> /usr/bin/clang -D__KERNEL__ -D__NR_CPUS__=72 -DLINUX_VERSION_CODE=0x50e00 -xc
> -I/root/lib/perf/include/bpf -nostdinc -I./arch/powerpc/include
> -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi
> -I./arch/powerpc/include/generated/uapi -I./include/uapi
> -I./include/generated/uapi -include ./include/linux/compiler-version.h
> -include ./include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign
> -working-directory /lib/modules//build -c - -target bpf -g -O2 -o -
>
> Compilation command after this commit:
> /usr/bin/clang -D__KERNEL__ -D__NR_CPUS__=72 -DLINUX_VERSION_CODE=0x50e00 -xc
> -I/usr/include/ -nostdinc -I./arch/powerpc/include
> -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi
> -I./arch/powerpc/include/generated/uapi -I./include/uapi
> -I./include/generated/uapi -include ./include/linux/compiler-version.h
> -include ./include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign
> -working-directory /lib/modules//build -c - -target bpf -g -O2 -o -
>
> The difference is addition of -I/usr/include/ in the first line
> which is causing the error. Fix this by adding typedef for "fmode_t"
> in the testcase to solve the compile issue.
>
> Fixes: 14e4b9f4289a ("perf trace: Raw augmented syscalls fix libbpf 1.0+
> compatibility")
> Signed-off-by: Athira Rajeev
> ---
> tools/perf/tests/bpf-script-test-prologue.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/tests/bpf-script-test-prologue.c
> b/tools/perf/tests/bpf-script-test-prologue.c
> index bd83d364cf30..91778b5c6125 100644
> --- a/tools/perf/tests/bpf-script-test-prologue.c
> +++ b/tools/perf/tests/bpf-script-test-prologue.c
> @@ -20,6 +20,8 @@
> # undef if
> #endif
>
> +typedef unsigned int __bitwise fmode_t;
> +
> #define FMODE_READ 0x1
> #define FMODE_WRITE 0x2
>
> --
> 2.31.1
--
- Arnaldo
[PATCH 2/2] perf test bpf: Skip test if kernel-debuginfo is not present
Perf BPF filter test fails in environment where "kernel-debuginfo"
is not installed.
Test failure logs:
<<>>
42: BPF filter:
42.1: Basic BPF filtering : Ok
42.2: BPF pinning : Ok
42.3: BPF prologue generation : FAILED!
<<>>
Enabling verbose option provided debug logs, which says debuginfo
needs to be installed. Snippet of verbose logs:
<<>>
42.3: BPF prologue generation :
--- start ---
test child forked, pid 28218
<<>>
Rebuild with CONFIG_DEBUG_INFO=y, or install an appropriate debuginfo
package.
bpf_probe: failed to convert perf probe events
Failed to add events selected by BPF
test child finished with -1
end
BPF filter subtest 3: FAILED!
<<>>
Here subtest, "BPF prologue generation" failed and
logs shows debuginfo is needed. After installing
kernel-debuginfo package, testcase passes.
Subtest "BPF prologue generation" failed because, the "do_test"
function returns "TEST_FAIL" without checking the error type
returned by "parse_events_load_bpf_obj" function.
Function parse_events_load_bpf_obj can also return error of type
"-ENODATA" incase kernel-debuginfo package is not installed. Fix this
by adding check for -ENODATA error.
Test result after the patch changes:
Test failure logs:
<<>>
42: BPF filter :
42.1: Basic BPF filtering : Ok
42.2: BPF pinning : Ok
42.3: BPF prologue generation : Skip (clang/debuginfo isn't
installed or environment missing BPF support)
Fixes: ba1fae431e74 ("perf test: Add 'perf test BPF'")
Signed-off-by: Athira Rajeev
---
Note: This is dependent on patch 1:
tools/perf: Update the exit error codes in function
try_to_find_probe_trace_event
tools/perf/tests/bpf.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 17c023823713..6a4235a9cf57 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -126,6 +126,10 @@ static int do_test(struct bpf_object *obj, int
(*func)(void),
err = parse_events_load_bpf_obj(_state, _state.list, obj,
NULL);
parse_events_error__exit(_error);
+ if (err == -ENODATA) {
+ pr_debug("Failed to add events selected by BPF, debuginfo
package not installed\n");
+ return TEST_SKIP;
+ }
if (err || list_empty(_state.list)) {
pr_debug("Failed to add events selected by BPF\n");
return TEST_FAIL;
@@ -368,7 +372,7 @@ static struct test_case bpf_tests[] = {
"clang isn't installed or environment missing BPF
support"),
#ifdef HAVE_BPF_PROLOGUE
TEST_CASE_REASON("BPF prologue generation", bpf_prologue_test,
- "clang isn't installed or environment missing BPF
support"),
+ "clang/debuginfo isn't installed or environment missing
BPF support"),
#else
TEST_CASE_REASON("BPF prologue generation", bpf_prologue_test, "not
compiled in"),
#endif
--
2.31.1
[PATCH 1/2] tools/perf: Update the exit error codes in function try_to_find_probe_trace_event
The function "try_to_find_probe_trace_events" uses return
error code as ENOENT in two places. First place is after
"open_debuginfo" when opening debuginfo fails and secondly,
after when not finding the probe point. This function is
invoked during bpf load and there are other exit points in
this code path which returns ENOENT. This makes it difficult
to understand the exact reason for exit.
Patches changes the exit code from ENOENT to:
- ENODATA when it fails to find debuginfo
- ENODEV when it fails to find probe point
Signed-off-by: Athira Rajeev
---
tools/perf/util/probe-event.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0c24bc7afbca..881d94f65a6b 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -917,7 +917,7 @@ static int try_to_find_probe_trace_events(struct
perf_probe_event *pev,
dinfo = open_debuginfo(pev->target, pev->nsi, !need_dwarf);
if (!dinfo) {
if (need_dwarf)
- return -ENOENT;
+ return -ENODATA;
pr_debug("Could not open debuginfo. Try to use symbols.\n");
return 0;
}
@@ -956,7 +956,7 @@ static int try_to_find_probe_trace_events(struct
perf_probe_event *pev,
if (ntevs == 0) { /* No error but failed to find probe point. */
pr_warning("Probe point '%s' not found.\n",
synthesize_perf_probe_point(>point));
- return -ENOENT;
+ return -ENODEV;
} else if (ntevs < 0) {
/* Error path : ntevs < 0 */
pr_debug("An error occurred in debuginfo analysis (%d).\n",
ntevs);
--
2.31.1
[PATCH] tools/perf: Fix bpf-script-test-prologue test compile issue with clang
While running perf test for bpf, observed that "BPF prologue
generation" test case fails to compile with clang. Logs below
from powerpc:
:33:2: error: use of undeclared identifier 'fmode_t'
fmode_t f_mode = (fmode_t)_f_mode;
^
:37:6: error: use of undeclared identifier 'f_mode'; did you mean
'_f_mode'?
if (f_mode & FMODE_WRITE)
^~
_f_mode
:30:60: note: '_f_mode' declared here
int bpf_func__null_lseek(void *ctx, int err, unsigned long _f_mode,
^
2 errors generated.
The test code tests/bpf-script-test-prologue.c uses fmode_t.
And the error above is for "fmode_t" which is defined in
include/linux/types.h as part of kernel build directory:
"/lib/modules//build" that comes from
kernel devel [ soft link to /usr/src/ ].
Clang picks this header file from "-working-directory" build
option that specifies this build folder.
But the 'commit 14e4b9f4289a ("perf trace: Raw augmented
syscalls fix libbpf 1.0+ compatibility")', changed the
include directory to use: "/usr/include". Post this change,
types.h from /usr/include/ is getting picked upwhich doesn’t
contain definition of "fmode_t" and hence fails to compile.
Compilation command before this commit:
/usr/bin/clang -D__KERNEL__ -D__NR_CPUS__=72 -DLINUX_VERSION_CODE=0x50e00 -xc
-I/root/lib/perf/include/bpf -nostdinc -I./arch/powerpc/include
-I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi
-I./arch/powerpc/include/generated/uapi -I./include/uapi
-I./include/generated/uapi -include ./include/linux/compiler-version.h -include
./include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign
-working-directory /lib/modules//build -c - -target bpf -g -O2 -o -
Compilation command after this commit:
/usr/bin/clang -D__KERNEL__ -D__NR_CPUS__=72 -DLINUX_VERSION_CODE=0x50e00 -xc
-I/usr/include/ -nostdinc -I./arch/powerpc/include
-I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi
-I./arch/powerpc/include/generated/uapi -I./include/uapi
-I./include/generated/uapi -include ./include/linux/compiler-version.h -include
./include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign
-working-directory /lib/modules//build -c - -target bpf -g -O2 -o -
The difference is addition of -I/usr/include/ in the first line
which is causing the error. Fix this by adding typedef for "fmode_t"
in the testcase to solve the compile issue.
Fixes: 14e4b9f4289a ("perf trace: Raw augmented syscalls fix libbpf 1.0+
compatibility")
Signed-off-by: Athira Rajeev
---
tools/perf/tests/bpf-script-test-prologue.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/tests/bpf-script-test-prologue.c
b/tools/perf/tests/bpf-script-test-prologue.c
index bd83d364cf30..91778b5c6125 100644
--- a/tools/perf/tests/bpf-script-test-prologue.c
+++ b/tools/perf/tests/bpf-script-test-prologue.c
@@ -20,6 +20,8 @@
# undef if
#endif
+typedef unsigned int __bitwise fmode_t;
+
#define FMODE_READ 0x1
#define FMODE_WRITE0x2
--
2.31.1
[PATCH v4] arch: rename all internal names __xchg to __arch_xchg
__xchg will be used for non-atomic xchg macro.
Signed-off-by: Andrzej Hajda
Reviewed-by: Arnd Bergmann
---
v2: squashed all arch patches into one
v3: fixed alpha/xchg_local, thx to l...@intel.com
v4: adjusted indentation (Heiko)
---
arch/alpha/include/asm/cmpxchg.h | 10 +-
arch/arc/include/asm/cmpxchg.h | 4 ++--
arch/arm/include/asm/cmpxchg.h | 7 ---
arch/arm64/include/asm/cmpxchg.h | 7 +++
arch/hexagon/include/asm/cmpxchg.h | 10 +-
arch/ia64/include/asm/cmpxchg.h | 2 +-
arch/ia64/include/uapi/asm/cmpxchg.h | 4 ++--
arch/loongarch/include/asm/cmpxchg.h | 4 ++--
arch/m68k/include/asm/cmpxchg.h | 6 +++---
arch/mips/include/asm/cmpxchg.h | 4 ++--
arch/openrisc/include/asm/cmpxchg.h | 10 +-
arch/parisc/include/asm/cmpxchg.h| 4 ++--
arch/powerpc/include/asm/cmpxchg.h | 4 ++--
arch/riscv/include/asm/atomic.h | 2 +-
arch/riscv/include/asm/cmpxchg.h | 4 ++--
arch/s390/include/asm/cmpxchg.h | 8
arch/sh/include/asm/cmpxchg.h| 4 ++--
arch/sparc/include/asm/cmpxchg_32.h | 4 ++--
arch/sparc/include/asm/cmpxchg_64.h | 6 +++---
arch/xtensa/include/asm/cmpxchg.h| 4 ++--
20 files changed, 54 insertions(+), 54 deletions(-)
diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 6e0a850aa9d38c..91d4a4d9258cd2 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -6,15 +6,15 @@
* Atomic exchange routines.
*/
-#define xchg(type, args...)__xchg ## type ## _local(args)
+#define xchg(type, args...)__arch_xchg ## type ##
_local(args)
#define cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
#include
#define xchg_local(ptr, x) \
({ \
__typeof__(*(ptr)) _x_ = (x); \
- (__typeof__(*(ptr))) __xchg_local((ptr), (unsigned long)_x_,\
- sizeof(*(ptr))); \
+ (__typeof__(*(ptr))) __arch_xchg_local((ptr), (unsigned long)_x_,\
+ sizeof(*(ptr))); \
})
#define arch_cmpxchg_local(ptr, o, n) \
@@ -34,7 +34,7 @@
#undef xchg
#undef cmpxchg
-#define xchg(type, args...)__xchg ##type(args)
+#define xchg(type, args...)__arch_xchg ##type(args)
#define cmpxchg(type, args...) __cmpxchg ##type(args)
#include
@@ -48,7 +48,7 @@
__typeof__(*(ptr)) _x_ = (x); \
smp_mb(); \
__ret = (__typeof__(*(ptr)))\
- __xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
+ __arch_xchg((ptr), (unsigned long)_x_, sizeof(*(ptr))); \
smp_mb(); \
__ret; \
})
diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
index c5b544a5fe8106..e138fde067dea5 100644
--- a/arch/arc/include/asm/cmpxchg.h
+++ b/arch/arc/include/asm/cmpxchg.h
@@ -85,7 +85,7 @@
*/
#ifdef CONFIG_ARC_HAS_LLSC
-#define __xchg(ptr, val) \
+#define __arch_xchg(ptr, val) \
({ \
__asm__ __volatile__( \
" ex %0, [%1]\n" /* set new value */ \
@@ -102,7 +102,7 @@
\
switch(sizeof(*(_p_))) {\
case 4: \
- _val_ = __xchg(_p_, _val_); \
+ _val_ = __arch_xchg(_p_, _val_);\
break; \
default:\
BUILD_BUG();\
diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index 4dfe538dfc689b..44667bdb4707a9 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -25,7 +25,8 @@
#define swp_is_buggy
#endif
-static inline unsigned long __xchg(unsigned long x, volatile void *ptr, int
size)
+static inline unsigned long
+__arch_xchg(unsigned long x, volatile void *ptr, int size)
{
extern void __bad_xchg(volatile void *, int);
unsigned long ret;
@@ -115,8 +116,8 @@ static inline unsigned long __xchg(unsigned
Re: [PATCH 07/10] tty: Convert ->dtr_rts() to take bool argument
On Thu, 5 Jan 2023, Jiri Slaby wrote:
> On 04. 01. 23, 16:15, Ilpo Järvinen wrote:
> > Convert the raise/on parameter in ->dtr_rts() to bool through the
> > callchain. The parameter is used like bool. In USB serial, there
> > remains a few implicit bool -> larger type conversions because some
> > devices use u8 in their control messages.
>
> Reviewed-by: Jiri Slaby
>
> > Signed-off-by: Ilpo Järvinen
> > ---
> ...
> > --- a/drivers/char/pcmcia/synclink_cs.c
> > +++ b/drivers/char/pcmcia/synclink_cs.c
> > @@ -378,7 +378,7 @@ static void async_mode(MGSLPC_INFO *info);
> > static void tx_timeout(struct timer_list *t);
> > static bool carrier_raised(struct tty_port *port);
> > -static void dtr_rts(struct tty_port *port, int onoff);
> > +static void dtr_rts(struct tty_port *port, bool onoff);
>
> Not anything for this patch, but having this dubbed "onoff" instead of "on"
> makes it really confusing.
>
> > --- a/drivers/mmc/core/sdio_uart.c
> > +++ b/drivers/mmc/core/sdio_uart.c
> > @@ -548,14 +548,14 @@ static bool uart_carrier_raised(struct tty_port
> > *tport)
> >*adjusted during an open, close and hangup.
> >*/
> > -static void uart_dtr_rts(struct tty_port *tport, int onoff)
> > +static void uart_dtr_rts(struct tty_port *tport, bool onoff)
> > {
> > struct sdio_uart_port *port =
> > container_of(tport, struct sdio_uart_port, port);
> > int ret = sdio_uart_claim_func(port);
> > if (ret)
> > return;
> > - if (onoff == 0)
> > + if (!onoff)
> > sdio_uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
> > else
> > sdio_uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
>
> Especially here. What does "!onoff" mean? If it were:
>
> if (on)
> sdio_uart_set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
> else
> sdio_uart_clear_mctrl(port, TIOCM_DTR | TIOCM_RTS);
>
> it would be a lot more clear.
>
> > --- a/drivers/tty/amiserial.c
> > +++ b/drivers/tty/amiserial.c
> > @@ -1459,7 +1459,7 @@ static bool amiga_carrier_raised(struct tty_port
> > *port)
> > return !(ciab.pra & SER_DCD);
> > }
> > -static void amiga_dtr_rts(struct tty_port *port, int raise)
> > +static void amiga_dtr_rts(struct tty_port *port, bool raise)
>
> Or "raise". That makes sense too and we call it as such in
> tty_port_operations:
>
> > --- a/include/linux/tty_port.h
> > +++ b/include/linux/tty_port.h
> ...
> > @@ -32,7 +32,7 @@ struct tty_struct;
> >*/
> > struct tty_port_operations {
> > bool (*carrier_raised)(struct tty_port *port);
> > - void (*dtr_rts)(struct tty_port *port, int raise);
> > + void (*dtr_rts)(struct tty_port *port, bool raise);
> > void (*shutdown)(struct tty_port *port);
> > int (*activate)(struct tty_port *port, struct tty_struct *tty);
> > void (*destruct)(struct tty_port *port);
>
> Care to fix that up too?
Sure. I noticed they were inconsistent but it didn't feel like changing
the name "while at it" would be good as this is long already. I think I'll
make another patch out of the name changes.
--
i.
Re: [PATCH v2 7/7] powerpc/pseries: Implement secvars for dynamic secure boot
On Fri, 2022-12-30 at 15:20 +1100, Russell Currey wrote: > The pseries platform can support dynamic secure boot (i.e. secure > boot > using user-defined keys) using variables contained with the PowerVM > LPAR > Platform KeyStore (PLPKS). Using the powerpc secvar API, expose the > relevant variables for pseries dynamic secure boot through the > existing > secvar filesystem layout. > > The relevant variables for dynamic secure boot are signed in the > keystore, and can only be modified using the H_PKS_SIGNED_UPDATE > hcall. > Object labels in the keystore are encoded using ucs2 format. With > our > fixed variable names we don't have to care about encoding outside of > the > necessary byte padding. > > When a user writes to a variable, the first 8 bytes of data must > contain > the signed update flags as defined by the hypervisor. > > When a user reads a variable, the first 4 bytes of data contain the > policies defined for the object. > > Limitations exist due to the underlying implementation of sysfs > binary > attributes, as is the case for the OPAL secvar implementation - > partial writes are unsupported and writes cannot be larger than > PAGE_SIZE. The PAGE_SIZE limit, in practice, will be a major limitation with 4K pages (we expect several of the variables to regularly be larger than 4K) but won't be much of an issue for 64K (that's all the storage space the hypervisor will give you anyway). In a previous internal version, we printed a message when PAGE_SIZE < plpks_get_maxobjectsize(), might be worth still doing that? > > Co-developed-by: Nayna Jain > Signed-off-by: Nayna Jain > Co-developed-by: Andrew Donnellan > Signed-off-by: Andrew Donnellan > Signed-off-by: Russell Currey Some minor comments for v3 on a patch which already carries my signoff... > --- > v2: Remove unnecessary config vars from sysfs and document the > others, > thanks to review from Greg. If we end up needing to expose more, > we > can add them later and update the docs. > > Use sysfs_emit() instead of sprintf(), thanks to Greg. > > Change the size of the sysfs binary attributes to include the 8- > byte > flags header, preventing truncation of large writes. > > Documentation/ABI/testing/sysfs-secvar | 67 - > arch/powerpc/platforms/pseries/Kconfig | 13 + > arch/powerpc/platforms/pseries/Makefile | 4 +- > arch/powerpc/platforms/pseries/plpks-secvar.c | 245 > ++ > 4 files changed, 326 insertions(+), 3 deletions(-) > create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c > > diff --git a/Documentation/ABI/testing/sysfs-secvar > b/Documentation/ABI/testing/sysfs-secvar > index feebb8c57294..466a8cb92b92 100644 > --- a/Documentation/ABI/testing/sysfs-secvar > +++ b/Documentation/ABI/testing/sysfs-secvar > @@ -34,7 +34,7 @@ Description: An integer representation of the size > of the content of the > > What: /sys/firmware/secvar/vars//data > Date: August 2019 > -Contact: Nayna Jain h > +Contact: Nayna Jain > Description: A read-only file containing the value of the > variable. The size > of the file represents the maximum size of the > variable data. > > @@ -44,3 +44,68 @@ Contact: Nayna Jain > Description: A write-only file that is used to submit the new > value for the > variable. The size of the file represents the maximum > size of > the variable data that can be written. > + > +What: /sys/firmware/secvar/config > +Date: December 2022 > +Contact: Nayna Jain > +Description: This optional directory contains read-only config > attributes as > + defined by the secure variable implementation. All > data is in > + ASCII format. The directory is only created if the > backing > + implementation provides variables to populate it, > which at > + present is only PLPKS on the pseries platform. > + > +What: /sys/firmware/secvar/config/version > +Date: December 2022 > +Contact: Nayna Jain > +Description: RO file, only present if the secvar implementation is > PLPKS. > + > + Contains the config version as reported by the > hypervisor in > + ASCII decimal format. > + > +What: /sys/firmware/secvar/config/max_object_size > +Date: December 2022 > +Contact: Nayna Jain > +Description: RO file, only present if the secvar implementation is > PLPKS. > + > + Contains the maximum allowed size of objects in the > keystore > + in bytes, represented in ASCII decimal format. > + > + This is not necessarily the same as the max size that > can be > + written to an update file as writes can contain more > than > + object data, you should use the size of the update > file for > + that purpose. > + > +What:
Re: [PATCH 00/14] Remove clang's -Qunused-arguments from KBUILD_CPPFLAGS
On Wed, Jan 04, 2023 at 12:54:18PM -0700, Nathan Chancellor wrote: > Hi all, ... > This series has seen my personal test framework, which tests several different > configurations and architectures, with LLVM tip of tree (16.0.0). I have done > defconfig, allmodconfig, and allnoconfig builds for arm, arm64, i386, mips, > powerpc, riscv, s390, and x86_64 with GCC 12.2.0 as well but I am hoping the > rest of the test infrastructure will catch any lurking problems. > > I would like this series to stay together so that there is no opportunity for > breakage so please consider giving acks so that this can be carried via the > kbuild tree. ... > s390/vdso: Drop unused '-s' flag from KBUILD_AFLAGS_64 > s390/vdso: Drop '-shared' from KBUILD_CFLAGS_64 > s390/purgatory: Remove unused '-MD' and unnecessary '-c' flags ... > arch/s390/kernel/vdso64/Makefile| 4 +-- > arch/s390/purgatory/Makefile| 2 +- For the s390 bits: Acked-by: Heiko Carstens
