Re: [PATCH v3 25/34] misc: Add Keem Bay VPU manager
On 1/29/21 6:20 PM, mgr...@linux.intel.com wrote: > diff --git a/drivers/misc/vpumgr/Kconfig b/drivers/misc/vpumgr/Kconfig > new file mode 100644 > index ..bb82ff83afd3 > --- /dev/null > +++ b/drivers/misc/vpumgr/Kconfig > @@ -0,0 +1,9 @@ > +config VPUMGR > + tristate "VPU Manager" > + depends on ARM64 && XLINK_CORE > + help > + VPUMGR manages life-cycle of VPU related resources which were > + dynamically allocated on demands of user-space application End above sentence with a period ('.'). Hm, same as I said the previous round of patches (also v3). > + > + Select y or m if you have a processor including the Intel > + Vision Processor (VPU) on it. -- ~Randy
Re: [External] [PATCH] misc: pvpanic: sysfs_emit uses should have a newline
On 1/30/21 3:08 AM, Joe Perches wrote: Add newline terminations to the sysfs_emit uses added by -next commit 8d6da6575ffe ("misc: pvpanic: introduce events device attribue") Signed-off-by: Joe Perches --- drivers/misc/pvpanic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c index b1e4922a7fda..9f350e05ef68 100644 --- a/drivers/misc/pvpanic.c +++ b/drivers/misc/pvpanic.c @@ -25,13 +25,13 @@ static unsigned int events; static ssize_t capability_show(struct device *dev, struct device_attribute *attr, char *buf) { - return sysfs_emit(buf, "%x", capability); + return sysfs_emit(buf, "%x\n", capability); } static DEVICE_ATTR_RO(capability); static ssize_t events_show(struct device *dev, struct device_attribute *attr, char *buf) { - return sysfs_emit(buf, "%x", events); + return sysfs_emit(buf, "%x\n", events); } static ssize_t events_store(struct device *dev, struct device_attribute *attr, Hi, Greg is the maintainer of this driver. -- zhenwei pi
Re: linux-next: build warning after merge of the drm tree
Hi all, On Fri, 22 Jan 2021 11:59:18 +1100 Stephen Rothwell wrote: > > After merging the drm tree, today's linux-next build (x86_64 allmodconfig) > produced this warning: > > WARNING: unmet direct dependencies detected for DRM_I915_WERROR > Depends on [n]: HAS_IOMEM [=y] && DRM_I915 [=m] && EXPERT [=y] && > !COMPILE_TEST [=y] > Selected by [m]: > - DRM_I915_DEBUG [=y] && HAS_IOMEM [=y] && EXPERT [=y] && DRM_I915 [=m] > > WARNING: unmet direct dependencies detected for DRM_I915_WERROR > Depends on [n]: HAS_IOMEM [=y] && DRM_I915 [=m] && EXPERT [=y] && > !COMPILE_TEST [=y] > Selected by [m]: > - DRM_I915_DEBUG [=y] && HAS_IOMEM [=y] && EXPERT [=y] && DRM_I915 [=m] > > WARNING: unmet direct dependencies detected for DRM_I915_WERROR > Depends on [n]: HAS_IOMEM [=y] && DRM_I915 [=m] && EXPERT [=y] && > !COMPILE_TEST [=y] > Selected by [m]: > - DRM_I915_DEBUG [=y] && HAS_IOMEM [=y] && EXPERT [=y] && DRM_I915 [=m] > > Maybe introduced by commit > > 4f86975f539d ("drm/i915: Add DEBUG_GEM to the recommended CI config") I am still getting this warning. -- Cheers, Stephen Rothwell pgpl0M4oDY8u6.pgp Description: OpenPGP digital signature
Re: [PATCH 2/3] usb: xhci-mtk: fix UAS issue by XHCI_BROKEN_STREAMS quirk
On Sun, 2021-01-31 at 15:13 +0100, Matthias Brugger wrote: > > On 24/12/2020 08:18, Chunfeng Yun wrote: > > On Wed, 2020-12-16 at 19:43 -0800, Rosen Penev wrote: > >> On Wed, Dec 16, 2020 at 6:29 PM Chunfeng Yun > >> wrote: > >>> > >>> On Wed, 2020-12-16 at 20:28 +0800, Nicolas Boichat wrote: > On Wed, Dec 16, 2020 at 7:53 PM Chunfeng Yun > wrote: > [...] > > mtk->lpm_support = of_property_read_bool(node, > > "usb3-lpm-capable"); > > + mtk->broken_streams = > > + of_property_read_bool(node, > > "mediatek,broken_streams_quirk"); > > Would it be better to add a data field to struct of_device_id > mtk_xhci_of_match, and enable this quirk on mediatek,mt8173-xhci only? > >>> This is the common issue for all SoCs (before 2016.06) with 0.96 xHCI > >>> when the controller don't support bulk stream. If enable this quirk only > >>> for mt8173, then for other SoCs, the compatible need include > >>> "mediatek,mt8173-xhci" in dts, this may be not flexible for some cases, > >>> e.g. a new SoC has the broken stream as mt8173, but also has another > >>> different quirk, the way you suggested will not handle it. > >>> And I plan to remove "mediatek,mt8173-xhci" in mtk_xhci_of_match after > >>> converting the binding to YMAL. > >> I'm guessing this also applies to mt7621? > > Yes, mt7621 doesn't support bulk stream > > > > Then please provide patches to the DTSI for all SoCs that have this problem. > Either as a follow-up or as part of this series, if you need to resubmit. Ok, I'll send new version, and also try other way to fix it without add property in DTS, thanks > > Regards, > Matthias
[PATCH] lib/cmdline: remove an unneeded local variable in next_arg()
The local variable 'next' is unneeded because you can simply advance the existing pointer 'args'. Signed-off-by: Masahiro Yamada --- lib/cmdline.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index b390dd03363b..f9844ea417c0 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -215,7 +215,6 @@ char *next_arg(char *args, char **param, char **val) { unsigned int i, equals = 0; int in_quote = 0, quoted = 0; - char *next; if (*args == '"') { args++; @@ -253,10 +252,10 @@ char *next_arg(char *args, char **param, char **val) if (args[i]) { args[i] = '\0'; - next = args + i + 1; + args += i + 1; } else - next = args + i; + args += i; /* Chew up trailing spaces. */ - return skip_spaces(next); + return skip_spaces(args); } -- 2.27.0
Re: [PATCH] cxl: Simplify bool conversion
On 29/1/21 7:25 pm, Yang Li wrote: Fix the following coccicheck warning: ./drivers/misc/cxl/sysfs.c:181:48-53: WARNING: conversion to bool not needed here Reported-by: Abaci Robot Signed-off-by: Yang Li Reviewed-by: Andrew Donnellan Thanks! --- drivers/misc/cxl/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c index d97a243..c173a5e 100644 --- a/drivers/misc/cxl/sysfs.c +++ b/drivers/misc/cxl/sysfs.c @@ -178,7 +178,7 @@ static ssize_t perst_reloads_same_image_store(struct device *device, if ((rc != 1) || !(val == 1 || val == 0)) return -EINVAL; - adapter->perst_same_image = (val == 1 ? true : false); + adapter->perst_same_image = (val == 1); return count; } -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH net-next 9/9] net: ipa: don't disable NAPI in suspend
On Sun, Jan 31, 2021 at 10:32 AM Alex Elder wrote: > > On 1/31/21 8:52 AM, Willem de Bruijn wrote: > > On Sat, Jan 30, 2021 at 11:29 PM Alex Elder wrote: > >> > >> On 1/30/21 9:25 AM, Willem de Bruijn wrote: > >>> On Fri, Jan 29, 2021 at 3:29 PM Alex Elder wrote: > > The channel stop and suspend paths both call __gsi_channel_stop(), > which quiesces channel activity, disables NAPI, and (on other than > SDM845) stops the channel. Similarly, the start and resume paths > share __gsi_channel_start(), which starts the channel and re-enables > NAPI again. > > Disabling NAPI should be done when stopping a channel, but this > should *not* be done when suspending. It's not necessary in the > suspend path anyway, because the stopped channel (or suspended > endpoint on SDM845) will not cause interrupts to schedule NAPI, > and gsi_channel_trans_quiesce() won't return until there are no > more transactions to process in the NAPI polling loop. > >>> > >>> But why is it incorrect to do so? > >> > >> Maybe it's not; I also thought it was fine before, but... > >> > >> Someone at Qualcomm asked me why I thought NAPI needed > >> to be disabled on suspend. My response was basically > >> that it was a lightweight operation, and it shouldn't > >> really be a problem to do so. > >> > >> Then, when I posted two patches last month, Jakub's > >> response told me he didn't understand why I was doing > >> what I was doing, and I stepped back to reconsider > >> the details of what was happening at suspend time. > >> > >> https://lore.kernel.org/netdev/20210107183803.47308...@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/ > >> > >> Four things were happening to suspend a channel: > >> quiesce activity; disable interrupt; disable NAPI; > >> and stop the channel. It occurred to me that a > >> stopped channel would not generate interrupts, so if > >> the channel was stopped earlier there would be no need > >> to disable the interrupt. Similarly there would be > >> (essentially) no need to disable NAPI once a channel > >> was stopped. > >> > >> Underlying all of this is that I started chasing a > >> hang that was occurring on suspend over a month ago. > >> It was hard to reproduce (hundreds or thousands of > >> suspend/resume cycles without hitting it), and one > >> of the few times I actually hit the problem it was > >> stuck in napi_disable(), apparently waiting for > >> NAPI_STATE_SCHED to get cleared by napi_complete(). > > > > This is important information. > > > > What exactly do you mean by hang? > > Yes it's important! Unfortunately I was not able to > gather details about the problem in all the cases where > it occurred. But in at least one case I *did* confirm > it was in the situation described above. > > What I mean by "hang" is that the system simply stopped > on its way down, and the IPA ->suspend callback never > completed (stuck in napi_disable). So I expect that > the SCHED flag was never going to get cleared (because > of a race, presumably). > > >> My best guess about how this could occur was if there > >> were a race of some kind between the interrupt handler > >> (scheduling NAPI) and the poll function (completing > >> it). I found a number of problems while looking > >> at this, and in the past few weeks I've posted some > >> fixes to improve things. Still, even with some of > >> these fixes in place we have seen a hang (but now > >> even more rarely). > >> > >> So this grand rework of suspending/stopping channels > >> is an attempt to resolve this hang on suspend. > > > > Do you have any data that this patchset resolves the issue, or is it > > too hard to reproduce to say anything? > > The data I have is that I have been running for weeks > with tens of thousands of iterations with this patch > (and the rest of them) without any hang. Unfortunately > that doesn't guarantee anything. I contemplated trying > to "catch" the problem and report that it *would have* > occurred had the fix not been in place, but I haven't > tried that (in part because it might not be easy). > > So... Too hard to reproduce, but I have evidence that > my testing so far has never reproduced the hang. > > >> The channel is now stopped early, and once stopped, > >> everything that completed prior to the channel being > >> stopped is polled before considering the suspend > >> function done. > > > > Does the call to gsi_channel_trans_quiesce before > > gsi_channel_stop_retry leave a race where new transactions may occur > > until state GSI_CHANNEL_STATE_STOPPED is reached? Asking without truly > > knowing the details of this device. > > It should not. For TX endpoints that have a net device, new > requests will have been stopped earlier by netif_stop_queue() > (in ipa_modem_suspend()). For RX endpoints, receive buffers > are replenished to the hardware, but we stop that earlier > as well, in ipa_endpoint_suspend_one(). So the quiesce call > is meant to figure out
Re: [PATCH] scripts: switch some more scripts explicitly to Python 3
On Mon, Feb 01, 2021 at 10:08:18AM +0900, Masahiro Yamada wrote: > For the same reason as commit 51839e29cb59 ("scripts: switch explicitly > to Python 3"), switch some more scripts, which I tested and confirmed > working on Python 3. > > Signed-off-by: Masahiro Yamada Acked-by: Nathan Chancellor > --- > > scripts/clang-tools/gen_compile_commands.py | 2 +- > scripts/clang-tools/run-clang-tools.py | 2 +- > scripts/spdxcheck.py| 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/scripts/clang-tools/gen_compile_commands.py > b/scripts/clang-tools/gen_compile_commands.py > index 19963708bcf8..8ddb5d099029 100755 > --- a/scripts/clang-tools/gen_compile_commands.py > +++ b/scripts/clang-tools/gen_compile_commands.py > @@ -1,4 +1,4 @@ > -#!/usr/bin/env python > +#!/usr/bin/env python3 > # SPDX-License-Identifier: GPL-2.0 > # > # Copyright (C) Google LLC, 2018 > diff --git a/scripts/clang-tools/run-clang-tools.py > b/scripts/clang-tools/run-clang-tools.py > index fa7655c7cec0..f754415af398 100755 > --- a/scripts/clang-tools/run-clang-tools.py > +++ b/scripts/clang-tools/run-clang-tools.py > @@ -1,4 +1,4 @@ > -#!/usr/bin/env python > +#!/usr/bin/env python3 > # SPDX-License-Identifier: GPL-2.0 > # > # Copyright (C) Google LLC, 2020 > diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py > index bc87200f9c7c..cbdb5c83c08f 100755 > --- a/scripts/spdxcheck.py > +++ b/scripts/spdxcheck.py > @@ -1,4 +1,4 @@ > -#!/usr/bin/env python > +#!/usr/bin/env python3 > # SPDX-License-Identifier: GPL-2.0 > # Copyright Thomas Gleixner > > -- > 2.27.0 >
linux-next: manual merge of the drm tree with Linus' tree
Hi all, Today's linux-next merge of the drm tree got a conflict in: drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h between commit: a119f87b86bc ("Revert "drm/amdgpu/swsmu: drop set_fan_speed_percent (v2)"") from Linus' tree and commit: d8a0b8dd690b ("drm/amd/pm: add pptable_funcs documentation (v3)") from the drm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h index 0d797fa9f5cc,a087e00382e6.. --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h @@@ -551,39 -924,199 +924,201 @@@ struct pptable_funcs int (*display_clock_voltage_request)(struct smu_context *smu, struct pp_display_clock_request *clock_req); + + /** +* @get_fan_control_mode: Get the current fan control mode. +*/ uint32_t (*get_fan_control_mode)(struct smu_context *smu); + + /** +* @set_fan_control_mode: Set the fan control mode. +*/ int (*set_fan_control_mode)(struct smu_context *smu, uint32_t mode); + + int (*set_fan_speed_percent)(struct smu_context *smu, uint32_t speed); ++ + /** +* @set_fan_speed_rpm: Set a static fan speed in RPM. +*/ int (*set_fan_speed_rpm)(struct smu_context *smu, uint32_t speed); + + /** +* @set_xgmi_pstate: Set inter-chip global memory interconnect pstate. +* : Pstate to set. D0 if Nonzero, D3 otherwise. +*/ int (*set_xgmi_pstate)(struct smu_context *smu, uint32_t pstate); + + /** +* @gfx_off_control: Enable/disable graphics engine poweroff. +*/ int (*gfx_off_control)(struct smu_context *smu, bool enable); + + + /** +* @get_gfx_off_status: Get graphics engine poweroff status. +* +* Return: +* 0 - GFXOFF(default). +* 1 - Transition out of GFX State. +* 2 - Not in GFXOFF. +* 3 - Transition into GFXOFF. +*/ uint32_t (*get_gfx_off_status)(struct smu_context *smu); + + /** +* @register_irq_handler: Register interupt request handlers. +*/ int (*register_irq_handler)(struct smu_context *smu); + + /** +* @set_azalia_d3_pme: Wake the audio decode engine from d3 sleep. +*/ int (*set_azalia_d3_pme)(struct smu_context *smu); + + /** +* @get_max_sustainable_clocks_by_dc: Get a copy of the max sustainable +*clock speeds table. +* +* Provides a way for the display component (DC) to get the max +* sustainable clocks from the SMU. +*/ int (*get_max_sustainable_clocks_by_dc)(struct smu_context *smu, struct pp_smu_nv_clock_table *max_clocks); + + /** +* @baco_is_support: Check if GPU supports BACO (Bus Active, Chip Off). +*/ bool (*baco_is_support)(struct smu_context *smu); + + /** +* @baco_get_state: Get the current BACO state. +* +* Return: Current BACO state. +*/ enum smu_baco_state (*baco_get_state)(struct smu_context *smu); + + /** +* @baco_set_state: Enter/exit BACO. +*/ int (*baco_set_state)(struct smu_context *smu, enum smu_baco_state state); + + /** +* @baco_enter: Enter BACO. +*/ int (*baco_enter)(struct smu_context *smu); + + /** +* @baco_exit: Exit Baco. +*/ int (*baco_exit)(struct smu_context *smu); + + /** +* @mode1_reset_is_support: Check if GPU supports mode1 reset. +*/ bool (*mode1_reset_is_support)(struct smu_context *smu); + + /** +* @mode1_reset: Perform mode1 reset. +* +* Complete GPU reset. +*/ int (*mode1_reset)(struct smu_context *smu); + + /** +* @mode2_reset: Perform mode2 reset. +* +* Mode2 reset generally does not reset as many IPs as mode1 reset. The +* IPs reset varies by asic. +*/ int (*mode2_reset)(struct smu_context *smu); + + /** +* @get_dpm_ultimate_freq: Get the hard frequency range of a clock +* domain in MHz. +*/ int (*get_dpm_ultimate_freq)(struct smu_context *smu, enum smu_clk_type clk_type, uint32_t *min, uint32_t *max); + + /** +* @set_soft_freq_limited_range: Set the soft frequency range of a clock +* domain in MHz. +
Re: [PATCH v1] ARM: imx: avic: Convert to using IRQCHIP_DECLARE
Hi Saravana, On Sun, Jan 31, 2021 at 5:56 PM Saravana Kannan wrote: > +static int __init imx_avic_init(struct device_node *node, > + struct device_node *parent) > +{ > + void __iomem *avic_base; > + > + avic_base = of_iomap(node, 0); > + BUG_ON(!avic_base); > + mxc_init_irq(avic_base); > + return 0; > +} > + > +IRQCHIP_DECLARE(imx_avic, "fsl,imx31-avic", imx_avic_init); Shouldn't the compatible be "fsl,avic" instead?
Re: [PATCH] powerpc: fix AKEBONO build failures
Randy Dunlap writes: > On 1/21/21 5:14 PM, Michael Ellerman wrote: >> Randy Dunlap writes: >>> On 1/20/21 1:29 PM, Yury Norov wrote: Hi all, I found the power pc build broken on today's linux-next (647060f3b592). >>> >>> Darn, I was building linux-5.11-rc4. >>> >>> I'll try linux-next after I send this. >>> >>> --- >>> From: Randy Dunlap >>> >>> Fulfill AKEBONO Kconfig requirements. >>> >>> Fixes these Kconfig warnings (and more) and fixes the subsequent >>> build errors: >>> >>> WARNING: unmet direct dependencies detected for NETDEVICES >>>Depends on [n]: NET [=n] >>>Selected by [y]: >>>- AKEBONO [=y] && PPC_47x [=y] >>> >>> WARNING: unmet direct dependencies detected for MMC_SDHCI >>>Depends on [n]: MMC [=n] && HAS_DMA [=y] >>>Selected by [y]: >>>- AKEBONO [=y] && PPC_47x [=y] >>> >>> Signed-off-by: Randy Dunlap >>> Cc: Michael Ellerman >>> Cc: Benjamin Herrenschmidt >>> Cc: Paul Mackerras >>> Cc: linuxppc-...@lists.ozlabs.org >>> Cc: Yury Norov >>> --- >>> arch/powerpc/platforms/44x/Kconfig |2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> --- lnx-511-rc4.orig/arch/powerpc/platforms/44x/Kconfig >>> +++ lnx-511-rc4/arch/powerpc/platforms/44x/Kconfig >>> @@ -206,6 +206,7 @@ config AKEBONO >>> select PPC4xx_HSTA_MSI >>> select I2C >>> select I2C_IBM_IIC >>> + select NET >>> select NETDEVICES >>> select ETHERNET >>> select NET_VENDOR_IBM >> >> I think the problem here is too much use of select, for things that >> should instead be in the defconfig. >> >> The patch below results in the same result for make >> 44x/akebono_defconfig. Does it fix the original issue? > > Hi Michael, > Sorry for the delay. > > Changing the akebono_defconfig doesn't cause the missing symbols > to be set -- the defconfig is not being used here. Yep, but that's OK. None of those selected symbols are hard dependencies of AKEBONO, they're just things you probably want in your kernel to actually boot on an akebono board. > I guess that if you have users who set CONFIG_AKEBONO and expect > it to build cleanly, you will need something like my patch or the > patch that Florian just posted. It will build cleanly, it just won't necessarily boot on a real board. Users who enable AKEBONO manually need to know what they're doing, or they should just use the defconfig. > Changing the akebono_defconfig also would not help 'make randconfig' > builds to build cleanly if they had happened to enable AKEBONO. Changing the defconfig doesn't help randconfig, but dropping the selects does. Anyway I'll send a proper version of my patch, which I'm pretty confident will fix all the issues. cheers
Re: [PATCH] hugetlbfs: show pagesize in unit of GB if possible
Hi: On 2021/1/31 6:07, David Rientjes wrote: > On Sat, 30 Jan 2021, Miaohe Lin wrote: > >> Hugepage size in unit of GB is supported. We could show pagesize in unit of >> GB to make it more friendly to read. Also rework the calculation code of >> page size unit to make it more readable. >> >> Signed-off-by: Miaohe Lin >> --- >> fs/hugetlbfs/inode.c | 12 >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c >> index 3a08fbae3b53..40a9795f250a 100644 >> --- a/fs/hugetlbfs/inode.c >> +++ b/fs/hugetlbfs/inode.c >> @@ -1014,11 +1014,15 @@ static int hugetlbfs_show_options(struct seq_file >> *m, struct dentry *root) >> if (sbinfo->max_inodes != -1) >> seq_printf(m, ",nr_inodes=%lu", sbinfo->max_inodes); >> >> -hpage_size /= 1024; >> -mod = 'K'; >> -if (hpage_size >= 1024) { >> -hpage_size /= 1024; >> +if (hpage_size >= SZ_1G) { >> +hpage_size /= SZ_1G; >> +mod = 'G'; >> +} else if (hpage_size >= SZ_1M) { >> +hpage_size /= SZ_1M; >> mod = 'M'; >> +} else { >> +hpage_size /= SZ_1K; >> +mod = 'K'; >> } >> seq_printf(m, ",pagesize=%lu%c", hpage_size, mod); >> if (spool) { > > NACK, this can break existing userspace parsing. > . > I see. I should take care of this. Many thanks.
Re: [PATCH v3 bpf-next 1/4] bpf: enable task local storage for tracing programs
On Thu, Jan 28, 2021 at 1:20 AM Song Liu wrote: > > To access per-task data, BPF programs usually creates a hash table with > pid as the key. This is not ideal because: > 1. The user need to estimate the proper size of the hash table, which may > be inaccurate; > 2. Big hash tables are slow; > 3. To clean up the data properly during task terminations, the user need > to write extra logic. > > Task local storage overcomes these issues and offers a better option for > these per-task data. Task local storage is only available to BPF_LSM. Now > enable it for tracing programs. > > Unlike LSM progreams, tracing programs can be called in IRQ contexts. nit: typo *programs > Helpers that accesses task local storage are updated to use nit: Helpers that access.. > raw_spin_lock_irqsave() instead of raw_spin_lock_bh(). > > Tracing programs can attach to functions on the task free path, e.g. > exit_creds(). To avoid allocating task local storage after > bpf_task_storage_free(). bpf_task_storage_get() is updated to not allocate > new storage when the task is not refcounted (task->usage == 0). > > Signed-off-by: Song Liu Acked-by: KP Singh Thanks for adding better commit descriptions :) I think checking the usage before adding storage should work for the task exit path (I could not think of cases where it would break). Would also be nice to check with Martin and Hao about this.
Re: [PATCH v5] tracepoint: Do not fail unregistering a probe due to memory failure
On 31/01/2021 01:42, Steven Rostedt wrote: On Sat, 30 Jan 2021 09:36:26 -0500 Steven Rostedt wrote: Do you still have the same crash with v3 (that's the one I'm going to go with for now.) https://lore.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home Just curious, does the following patch fix it for v5? Yes it does! -- Steve diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c index 7261fa0f5e3c..cf3a6d104fdb 100644 --- a/kernel/tracepoint.c +++ b/kernel/tracepoint.c @@ -306,6 +306,7 @@ static int tracepoint_remove_func(struct tracepoint *tp, tp->unregfunc(); static_key_disable(>key); + tracepoint_synchronize_unregister(); rcu_assign_pointer(tp->funcs, tp_funcs); } else { rcu_assign_pointer(tp->funcs, tp_funcs); -- Alexey
Re: [PATCH] ARM: dts: imx6qdl-wandboard: add scl/sda gpios definitions for i2c bus recovery
On Sun, Jan 31, 2021 at 12:55 PM Dima Azarkin wrote: > > The i2c bus on imx6qdl-wandboard has intermittent issues where SDA can freeze > on low level at the end of transaction so the bus can no longer work. This > impacts reading of EDID data leading to incorrect TV resolution and no audio. > > This scenario is improved by adding scl/sda gpios definitions to implement the > i2c bus recovery mechanism. > > Signed-off-by: Dima Azarkin Reviewed-by: Fabio Estevam
Re: [PATCH v5 0/4] Scan for an idle sibling in a single pass
On 2021/1/27 21:51, Mel Gorman wrote: > Changelog since v4 > o Avoid use of intermediate variable during select_idle_cpu > > Changelog since v3 > o Drop scanning based on cores, SMT4 results showed problems > > Changelog since v2 > o Remove unnecessary parameters > o Update nr during scan only when scanning for cpus > > Changlog since v1 > o Move extern declaration to header for coding style > o Remove unnecessary parameter from __select_idle_cpu > > This series of 4 patches reposts three patches from Peter entitled > "select_idle_sibling() wreckage". It only scans the runqueues in a single > pass when searching for an idle sibling. > > Three patches from Peter were dropped. The first patch altered how scan > depth was calculated. Scan depth deletion is a random number generator > with two major limitations. The avg_idle time is based on the time > between a CPU going idle and being woken up clamped approximately by > 2*sysctl_sched_migration_cost. This is difficult to compare in a sensible > fashion to avg_scan_cost. The second issue is that only the avg_scan_cost > of scan failures is recorded and it does not decay. This requires deeper > surgery that would justify a patch on its own although Peter notes that > https://lkml.kernel.org/r/20180530143105.977759...@infradead.org is > potentially useful for an alternative avg_idle metric. > > The second patch dropped scanned based on cores instead of CPUs as it > rationalised the difference between core scanning and CPU scanning. > Unfortunately, Vincent reported problems with SMT4 so it's dropped > for now until depth searching can be fixed. > > The third patch dropped converted the idle core scan throttling mechanism > to SIS_PROP. While this would unify the throttling of core and CPU > scanning, it was not free of regressions and has_idle_cores is a fairly > effective throttling mechanism with the caveat that it can have a lot of > false positives for workloads like hackbench. > > Peter's series tried to solve three problems at once, this subset addresses > one problem. > > kernel/sched/fair.c | 151 +++- > kernel/sched/features.h | 1 - > 2 files changed, 70 insertions(+), 82 deletions(-) > 4 benchmarks measured on a x86 4s system with 24 cores per socket and 2 HTs per core, total 192 CPUs. The load level is [25%, 50%, 75%, 100%]. - hackbench almost has a universal win. - netperf high load has notable changes, as well as tbench 50% load. Details below: hackbench: 10 iterations, 1 loops, 40 fds per group == - pipe process group base%stdv5 %std 3 1 19.18 1.0266 9.06 6 1 9.170.987 13.03 9 1 7.111.0195 4.61 12 1 1.070.9927 1.43 - pipe thread group base%stdv5 %std 3 1 11.14 0.9742 7.27 6 1 9.150.9572 7.48 9 1 2.950.986 4.05 12 1 1.750.9992 1.68 - socket process group base%stdv5 %std 3 1 2.9 0.9586 2.39 6 1 0.680.9641 1.3 9 1 0.640.9388 0.76 12 1 0.560.9375 0.55 - socket thread group base%stdv5 %std 3 1 3.820.9686 2.97 6 1 2.060.9667 1.91 9 1 0.440.9354 1.25 12 1 0.540.9362 0.6 netperf: 10 iterations x 100 seconds, transactions rate / sec = - tcp request/response performance thread base%stdv4 %std 25% 1 5.341.0039 5.13 50% 1 4.971.0115 6.3 75% 1 5.090.9257 6.75 100%1 4.530.908 4.83 - udp request/response performance thread base%stdv4 %std 25% 1 6.180.9896 6.09 50% 1 5.881.0198 8.92 75% 1 24.38 0.9236 29.14 100%1 26.16 0.9063 22.16 tbench: 10 iterations x 100 seconds, throughput / sec = thread base%stdv4 %std 25% 1 0.451.003 1.48 50% 1 1.710.9286 0.82 75% 1 0.840.9928 0.94 100%1 0.760.9762 0.59 schbench: 10 iterations x 100 seconds, 99th percentile latency == mthread base%stdv4 %std 25% 1 2.890.9884 7.34 50% 1 40.38 1.0055 38.37 75% 1 4.761.0095 4.62 100%1 10.09 1.0083 8.03 Thanks, -Aubrey
Re: [PATCH 1/3] MIPS: ftrace: Fix N32 save registers
On 01/31/2021 06:38 PM, Jiaxun Yang wrote: On Sun, Jan 31, 2021, at 4:14 PM, Jinyang He wrote: CONFIG_64BIT is confusing. N32 also pass parameters by a0~a7. Do we have NEW kernel build? CONFIG_64BIT assumed N64 as kernel ABI. -Jiaxun Hi, Jiaxun, Thank you for your reply, and now I know. Before that, I saw the macro from arch/mips/include/asm/regdef.h and thought it needed to be modified here. But that seems have no sence. Please ignore this patch. Thanks, Jinyang Signed-off-by: Jinyang He --- arch/mips/kernel/mcount.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S index cff52b2..808257a 100644 --- a/arch/mips/kernel/mcount.S +++ b/arch/mips/kernel/mcount.S @@ -27,7 +27,7 @@ PTR_S a1, PT_R5(sp) PTR_S a2, PT_R6(sp) PTR_S a3, PT_R7(sp) -#ifdef CONFIG_64BIT +#if _MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32 PTR_S a4, PT_R8(sp) PTR_S a5, PT_R9(sp) PTR_S a6, PT_R10(sp) @@ -42,7 +42,7 @@ PTR_L a1, PT_R5(sp) PTR_L a2, PT_R6(sp) PTR_L a3, PT_R7(sp) -#ifdef CONFIG_64BIT +#if _MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32 PTR_L a4, PT_R8(sp) PTR_L a5, PT_R9(sp) PTR_L a6, PT_R10(sp) -- 2.1.0
[PATCH] scripts: switch some more scripts explicitly to Python 3
For the same reason as commit 51839e29cb59 ("scripts: switch explicitly to Python 3"), switch some more scripts, which I tested and confirmed working on Python 3. Signed-off-by: Masahiro Yamada --- scripts/clang-tools/gen_compile_commands.py | 2 +- scripts/clang-tools/run-clang-tools.py | 2 +- scripts/spdxcheck.py| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py index 19963708bcf8..8ddb5d099029 100755 --- a/scripts/clang-tools/gen_compile_commands.py +++ b/scripts/clang-tools/gen_compile_commands.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0 # # Copyright (C) Google LLC, 2018 diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py index fa7655c7cec0..f754415af398 100755 --- a/scripts/clang-tools/run-clang-tools.py +++ b/scripts/clang-tools/run-clang-tools.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0 # # Copyright (C) Google LLC, 2020 diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py index bc87200f9c7c..cbdb5c83c08f 100755 --- a/scripts/spdxcheck.py +++ b/scripts/spdxcheck.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0 # Copyright Thomas Gleixner -- 2.27.0
Re: [PATCH] Updates Documentation/Makefile to use Python3 as fallback
On Sat, Jan 30, 2021 at 9:15 AM Jonathan Corbet wrote: > > Noa Sakurajin writes: > > [CC += kbuild maintainers] > > > Before the command python was needed for the documentation to build. > > This patch checks if python is available and uses python3 as > > fallback. > > > > This is needed because a lot of distribution (at least Ubuntu) > > only provide python3 and not python. scripts/sphinx-pre-install > > checks for python3 first and does not check if python exists > > which causes it to report that everything is installed even > > if the documentation build failed. > > > > Signed-off-by: Noa Sakurajin > > --- > > Documentation/Makefile | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/Makefile b/Documentation/Makefile > > index 61a7310b49e0..8a4a7df3b74a 100644 > > --- a/Documentation/Makefile > > +++ b/Documentation/Makefile > > @@ -75,7 +75,8 @@ quiet_cmd_sphinx = SPHINX $@ --> file://$(abspath > > $(BUILDDIR)/$3/$4) > >cmd_sphinx = $(MAKE) BUILDDIR=$(abspath $(BUILDDIR)) > > $(build)=Documentation/userspace-api/media $2 && \ > > PYTHONDONTWRITEBYTECODE=1 \ > > BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath > > $(srctree)/$(src)/$5/$(SPHINX_CONF)) \ > > - $(PYTHON) $(srctree)/scripts/jobserver-exec \ > > + PY=$(shell command -v $(PYTHON) 2> /dev/null) \ > > + $${PY:-"$(PYTHON3)"} $(srctree)/scripts/jobserver-exec \ > > So I see what you're trying to do, and we definitely want this to work. > I susped this isn't the right fix, though; it could leave us open to > similar issues elsewhere in the tree. > > Personally, I think that $(PYTHON) should get a working Python if it's > installed, so I would suggest fixing the top-level Makefile to set it > correctly. Masahiro, thoughts on that? > > Alternatively, we could just say $(PYTHON3) and explicitly leave Python2 > behind; that needs to happen in the not-too-distant future regardless > but we haven't decided to actually do it yet. We are already doing this in linux-next. I will apply this. https://lore.kernel.org/patchwork/patch/1373422/ > Thanks, > > jon -- Best Regards Masahiro Yamada
[PATCH v2] exfat: improve performance of exfat_free_cluster when using dirsync mount option
There are stressful update of cluster allocation bitmap when using dirsync mount option which is doing sync buffer on every cluster bit clearing. This could result in performance degradation when deleting big size file. Fix to update only when the bitmap buffer index is changed would make less disk access, improving performance especially for truncate operation. Testing with Samsung 256GB sdcard, mounted with dirsync option (mount -t exfat /dev/block/mmcblk0p1 /temp/mount -o dirsync) Remove 4GB file, blktrace result. [Before] : 39 secs. Total (blktrace): Reads Queued: 0,0KiB Writes Queued: 32775,16387KiB Read Dispatches: 0,0KiB Write Dispatches: 32775,16387KiB Reads Requeued:0Writes Requeued:0 Reads Completed: 0,0KiB Writes Completed: 32775,16387KiB Read Merges: 0,0KiB Write Merges: 0,0KiB IO unplugs:2Timer unplugs: 0 [After] : 1 sec. Total (blktrace): Reads Queued: 0,0KiB Writes Queued: 13,6KiB Read Dispatches: 0,0KiB Write Dispatches: 13,6KiB Reads Requeued:0Writes Requeued:0 Reads Completed: 0,0KiB Writes Completed: 13,6KiB Read Merges: 0,0KiB Write Merges: 0,0KiB IO unplugs:1Timer unplugs: 0 Signed-off-by: Hyeongseok Kim --- fs/exfat/balloc.c | 4 ++-- fs/exfat/exfat_fs.h | 2 +- fs/exfat/fatent.c | 43 +-- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/fs/exfat/balloc.c b/fs/exfat/balloc.c index a987919686c0..761c79c3a4ba 100644 --- a/fs/exfat/balloc.c +++ b/fs/exfat/balloc.c @@ -166,7 +166,7 @@ int exfat_set_bitmap(struct inode *inode, unsigned int clu) * If the value of "clu" is 0, it means cluster 2 which is the first cluster of * the cluster heap. */ -void exfat_clear_bitmap(struct inode *inode, unsigned int clu) +void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync) { int i, b; unsigned int ent_idx; @@ -180,7 +180,7 @@ void exfat_clear_bitmap(struct inode *inode, unsigned int clu) b = BITMAP_OFFSET_BIT_IN_SECTOR(sb, ent_idx); clear_bit_le(b, sbi->vol_amap[i]->b_data); - exfat_update_bh(sbi->vol_amap[i], IS_DIRSYNC(inode)); + exfat_update_bh(sbi->vol_amap[i], sync); if (opts->discard) { int ret_discard; diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index b8f0e829ecbd..764bc645241e 100644 --- a/fs/exfat/exfat_fs.h +++ b/fs/exfat/exfat_fs.h @@ -408,7 +408,7 @@ int exfat_count_num_clusters(struct super_block *sb, int exfat_load_bitmap(struct super_block *sb); void exfat_free_bitmap(struct exfat_sb_info *sbi); int exfat_set_bitmap(struct inode *inode, unsigned int clu); -void exfat_clear_bitmap(struct inode *inode, unsigned int clu); +void exfat_clear_bitmap(struct inode *inode, unsigned int clu, bool sync); unsigned int exfat_find_free_bitmap(struct super_block *sb, unsigned int clu); int exfat_count_used_clusters(struct super_block *sb, unsigned int *ret_count); diff --git a/fs/exfat/fatent.c b/fs/exfat/fatent.c index c3c9afee7418..7b2e8af17193 100644 --- a/fs/exfat/fatent.c +++ b/fs/exfat/fatent.c @@ -157,6 +157,7 @@ int exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain) unsigned int clu; struct super_block *sb = inode->i_sb; struct exfat_sb_info *sbi = EXFAT_SB(sb); + int cur_cmap_i, next_cmap_i; /* invalid cluster number */ if (p_chain->dir == EXFAT_FREE_CLUSTER || @@ -176,21 +177,51 @@ int exfat_free_cluster(struct inode *inode, struct exfat_chain *p_chain) clu = p_chain->dir; + cur_cmap_i = next_cmap_i = + BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(clu)); + if (p_chain->flags == ALLOC_NO_FAT_CHAIN) { + unsigned int last_cluster = p_chain->dir + p_chain->size - 1; do { - exfat_clear_bitmap(inode, clu); - clu++; + bool sync = false; + + if (clu < last_cluster) + next_cmap_i = + BITMAP_OFFSET_SECTOR_INDEX(sb, CLUSTER_TO_BITMAP_ENT(clu+1)); + /* flush bitmap only if index would be changed or for last cluster */ + if (clu == last_cluster || cur_cmap_i != next_cmap_i) { + sync = true; + cur_cmap_i = next_cmap_i; + } + + exfat_clear_bitmap(inode, clu, (sync && IS_DIRSYNC(inode))); + clu++; num_clusters++; } while (num_clusters < p_chain->size); } else { do
[PATCH] kbuild: remove PYTHON variable
Python retired in 2020, and some distributions do not provide the 'python' command any more. As in commit 51839e29cb59 ("scripts: switch explicitly to Python 3"), we need to use more specific 'python3' to invoke scripts even if they are written in a way compatible with both Python 2 and 3. This commit removes the variable 'PYTHON', and switches the existing users to 'PYTHON3'. BTW, PEP 394 (https://www.python.org/dev/peps/pep-0394/) is a helpful material. Signed-off-by: Masahiro Yamada --- Documentation/Makefile | 2 +- Documentation/kbuild/makefiles.rst | 2 +- Makefile | 3 +-- arch/ia64/Makefile | 2 +- arch/ia64/scripts/unwcheck.py | 2 +- scripts/jobserver-exec | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 61a7310b49e0..9c42dde97671 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -75,7 +75,7 @@ quiet_cmd_sphinx = SPHINX $@ --> file://$(abspath $(BUILDDIR)/$3/$4) cmd_sphinx = $(MAKE) BUILDDIR=$(abspath $(BUILDDIR)) $(build)=Documentation/userspace-api/media $2 && \ PYTHONDONTWRITEBYTECODE=1 \ BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath $(srctree)/$(src)/$5/$(SPHINX_CONF)) \ - $(PYTHON) $(srctree)/scripts/jobserver-exec \ + $(PYTHON3) $(srctree)/scripts/jobserver-exec \ $(SHELL) $(srctree)/Documentation/sphinx/parallel-wrapper.sh \ $(SPHINXBUILD) \ -b $2 \ diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst index 9f6a11881951..300d8edcb994 100644 --- a/Documentation/kbuild/makefiles.rst +++ b/Documentation/kbuild/makefiles.rst @@ -755,7 +755,7 @@ more details, with real examples. bits on the scripts nonetheless. Kbuild provides variables $(CONFIG_SHELL), $(AWK), $(PERL), - $(PYTHON) and $(PYTHON3) to refer to interpreters for the respective + and $(PYTHON3) to refer to interpreters for the respective scripts. Example:: diff --git a/Makefile b/Makefile index b0e4767735dc..89217e4e68c6 100644 --- a/Makefile +++ b/Makefile @@ -452,7 +452,6 @@ AWK = awk INSTALLKERNEL := installkernel DEPMOD = depmod PERL = perl -PYTHON = python PYTHON3= python3 CHECK = sparse BASH = bash @@ -508,7 +507,7 @@ CLANG_FLAGS := export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL -export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX +export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile index 703b1c4f6d12..45d5368d6a99 100644 --- a/arch/ia64/Makefile +++ b/arch/ia64/Makefile @@ -69,7 +69,7 @@ vmlinux.bin: vmlinux FORCE $(call if_changed,objcopy) unwcheck: vmlinux - -$(Q)READELF=$(READELF) $(PYTHON) $(srctree)/arch/ia64/scripts/unwcheck.py $< + -$(Q)READELF=$(READELF) $(PYTHON3) $(srctree)/arch/ia64/scripts/unwcheck.py $< archclean: diff --git a/arch/ia64/scripts/unwcheck.py b/arch/ia64/scripts/unwcheck.py index bfd1b671e35f..9581742f0db2 100644 --- a/arch/ia64/scripts/unwcheck.py +++ b/arch/ia64/scripts/unwcheck.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0 # # Usage: unwcheck.py FILE diff --git a/scripts/jobserver-exec b/scripts/jobserver-exec index 0fdb31a790a8..48d141e3ec56 100755 --- a/scripts/jobserver-exec +++ b/scripts/jobserver-exec @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0+ # # This determines how many parallel tasks "make" is expecting, as it is -- 2.27.0
[PATCH] f2fs: prevent setting ioprio of thread not in merge mode
From: Daeho Jeong It causes a crash to change the ioprio of checkpoint thread not in checkpoint=merge. I fixed that to prevent setting the ioprio of the thread when checkpoint=merge is not enabled. Signed-off-by: Daeho Jeong --- fs/f2fs/sysfs.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c index 100608bcd517..e38a7f6921dd 100644 --- a/fs/f2fs/sysfs.c +++ b/fs/f2fs/sysfs.c @@ -357,8 +357,12 @@ static ssize_t __sbi_store(struct f2fs_attr *a, return -EINVAL; cprc->ckpt_thread_ioprio = IOPRIO_PRIO_VALUE(class, data); - ret = set_task_ioprio(cprc->f2fs_issue_ckpt, - cprc->ckpt_thread_ioprio); + if (test_opt(sbi, MERGE_CHECKPOINT)) { + ret = set_task_ioprio(cprc->f2fs_issue_ckpt, + cprc->ckpt_thread_ioprio); + if (ret) + return ret; + } return count; } -- 2.30.0.365.g02bc693789-goog
Re: [Patch v3 net-next 0/7] ethtool support for fec and link configuration
On Sun, Jan 31, 2021 at 8:11 AM Hariprasad Kelam wrote: > > This series of patches add support for forward error correction(fec) and > physical link configuration. Patches 1&2 adds necessary mbox handlers for fec > mode configuration request and to fetch stats. Patch 3 registers driver > callbacks for fec mode configuration and display. Patch 4&5 adds support of > mbox > handlers for configuring link parameters like speed/duplex and autoneg etc. > Patche 6&7 registers driver callbacks for physical link configuration. > > Change-log: > v2: > - Fixed review comments > - Corrected indentation issues > - Return -ENOMEM incase of mbox allocation failure > - added validation for input fecparams bitmask values > - added more comments > > V3: > - Removed inline functions > - Make use of ethtool helpers APIs to display supported > advertised modes > - corrected indentation issues > - code changes such that return early in case of failure > to aid branch prediction This addresses my comments to the previous patch series, thanks. It seems that patchwork only picked up only patch 6/7 unfortunately: https://patchwork.kernel.org/project/netdevbpf/list/?series=424969 > > Christina Jacob (6): > octeontx2-af: forward error correction configuration > octeontx2-pf: ethtool fec mode support > octeontx2-af: Physical link configuration support > octeontx2-af: advertised link modes support on cgx > octeontx2-pf: ethtool physical link status > octeontx2-pf: ethtool physical link configuration > > Felix Manlunas (1): > octeontx2-af: Add new CGX_CMD to get PHY FEC statistics > > drivers/net/ethernet/marvell/octeontx2/af/cgx.c| 258 - > drivers/net/ethernet/marvell/octeontx2/af/cgx.h| 10 + > .../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 70 +++- > drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 87 - > drivers/net/ethernet/marvell/octeontx2/af/rvu.h| 4 + > .../net/ethernet/marvell/octeontx2/af/rvu_cgx.c| 80 + > .../ethernet/marvell/octeontx2/nic/otx2_common.c | 20 ++ > .../ethernet/marvell/octeontx2/nic/otx2_common.h | 6 + > .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 399 > - > .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 3 + > 10 files changed, 930 insertions(+), 7 deletions(-) > > -- > 2.7.4
Re: [PATCH v2] ext4: Enable code path when DX_DEBUG is set
On Mon, Feb 01, 2021 at 12:31:25AM +, Vinicius Tinti wrote: > By enabling -Wunreachable-code-aggressive on Clang the following code > paths are unreachable. > > This has been present since commit ac27a0ec112a ("[PATCH] ext4: initial > copy of files from ext3") and fs/ext3 had it present at the beginning of > git history. It has not been changed since. > > Clang warns: > > fs/ext4/namei.c:831:17: warning: code will never be executed > [-Wunreachable-code] > unsigned n = count - 1; > ^ > fs/ext4/namei.c:830:7: note: silence by adding parentheses to mark code as > explicitly dead > if (0) { // linear search cross check > ^ > /* DISABLES CODE */ ( ) > > Signed-off-by: Vinicius Tinti > --- > fs/ext4/namei.c | 23 --- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index cf652ba3e74d..46ae6a4e4be5 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -827,20 +827,21 @@ dx_probe(struct ext4_filename *fname, struct inode *dir, > p = m + 1; > } > > - if (0) { // linear search cross check > - unsigned n = count - 1; > - at = entries; > - while (n--) > +#ifdef DX_DEBUG > + // linear search cross check > + unsigned n = count - 1; You are going to introduce an instance of -Wdeclaration-after-statement here. fs/ext4/namei.c:834:12: warning: ISO C90 forbids mixing declarations and code [-Wdeclaration-after-statement] unsigned n = count - 1; ^ 1 warning generated. The quick hack would be changing the if (0) { to #ifdef DX_DEBUG if (1) { but that seems kind of ugly. Other options would be turning DX_DEBUG into a proper Kconfig symbol so that IS_ENABLED could be used or maybe combine it into CONFIG_EXT4_DEBUG. Cheers, Nathan
[PATCH] ARM: kexec: Fix panic after TLB are invalidated
machine_kexec() need to set rw permission in text and rodata sections to assign some variables (e.g. kexec_start_address). To do that at the end (after flushing pdm in memory, etc.) it needs to invalidate TLB [section] entries. If during the TLB invalidation an interrupt occours, which might cause a context switch, there is the risk to inject invalid TLBs, with ro permissions. When trying to assign .text labels, this lead to the following: Unable to handle kernel paging request at virtual address 80112f38 pgd = fd7ef03e [80112f38] *pgd=0001141e(bad) Internal error: Oops: 80d [#1] PREEMPT SMP ARM ... Signed-off-by: Giancarlo Ferrari --- arch/arm/kernel/machine_kexec.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c index 5d84ad3..23e8816 100644 --- a/arch/arm/kernel/machine_kexec.c +++ b/arch/arm/kernel/machine_kexec.c @@ -174,6 +174,13 @@ void machine_kexec(struct kimage *image) reboot_code_buffer = page_address(image->control_code_page); + /* +* If below part is not atomic TLB entries might be corrupted after TLB +* invalidation, which leads to Data Abort in .text variable assignment +*/ + raw_local_irq_disable(); + local_fiq_disable(); + /* Prepare parameters for reboot_code_buffer*/ set_kernel_text_rw(); kexec_start_address = image->start; @@ -181,6 +188,9 @@ void machine_kexec(struct kimage *image) kexec_mach_type = machine_arch_type; kexec_boot_atags = image->arch.kernel_r2; + local_fiq_enable(); + raw_local_irq_enable(); + /* copy our kernel relocation code to the control code page */ reboot_entry = fncpy(reboot_code_buffer, _new_kernel, -- 2.7.4
Re: [PATCH for -next] docs: hwmon: rectify table in max16601.rst
On 1/30/21 11:54 PM, Lukas Bulwahn wrote: > Commit 90b0f71d62df ("hwmon: (pmbus/max16601) Determine and use number of > populated phases") adjusts content in the table of > ./Documentation/hwmon/max16601.rst, but one row went beyond the column's > length. > > Hence, make htmldocs warns: > > Documentation/hwmon/max16601.rst:94: WARNING: Malformed table. > > Adjust the column length of that table for this longer row to fit. > > Signed-off-by: Lukas Bulwahn > --- > applies cleanly on next-20210129 > > Guenter, please pick this minor fixup for your hwmon-next tree. I already made the same change and squashed it into the original patch. Thanks, Guenter > > Documentation/hwmon/max16601.rst | 143 +++ > 1 file changed, 71 insertions(+), 72 deletions(-) > > diff --git a/Documentation/hwmon/max16601.rst > b/Documentation/hwmon/max16601.rst > index d16792be7533..d265a2224354 100644 > --- a/Documentation/hwmon/max16601.rst > +++ b/Documentation/hwmon/max16601.rst > @@ -53,75 +53,74 @@ Sysfs entries > > The following attributes are supported. > > -=== > === > -in1_label"vin1" > -in1_inputVCORE input voltage. > -in1_alarmInput voltage alarm. > - > -in2_label"vout1" > -in2_inputVCORE output voltage. > -in2_alarmOutput voltage alarm. > - > -curr1_label "iin1" > -curr1_input VCORE input current, derived from duty cycle and output > - current. > -curr1_maxMaximum input current. > -curr1_max_alarm Current high alarm. > - > -curr[P+2]_label "iin1.P" > -curr[P+2]_input VCORE phase P input current. > - > -curr[N+2]_label "iin2" > -curr[N+2]_input VCORE input current, derived from sensor > element. > - 'N' is the number of enabled/populated phases. > - > -curr[N+3]_label "iin3" > -curr[N+3]_input VSA input current. > - > -curr[N+4]_label "iout1" > -curr[N+4]_input VCORE output current. > -curr[N+4]_crit Critical output current. > -curr[N+4]_crit_alarm Output current critical alarm. > -curr[N+4]_maxMaximum output current. > -curr[N+4]_max_alarm Output current high alarm. > - > -curr[N+P+5]_label"iout1.P" > -curr[N+P+5]_inputVCORE phase P output current. > - > -curr[2*N+5]_label"iout3" > -curr[2*N+5]_inputVSA output current. > -curr[2*N+5]_highest Historical maximum VSA output current. > -curr[2*N+5]_reset_history > - Write any value to reset curr21_highest. > -curr[2*N+5]_crit Critical output current. > -curr[2*N+5]_crit_alarm Output current critical alarm. > -curr[2*N+5]_max Maximum output current. > -curr[2*N+5]_max_alarmOutput current high alarm. > - > -power1_label "pin1" > -power1_input Input power, derived from duty cycle and output current. > -power1_alarm Input power alarm. > - > -power2_label "pin2" > -power2_input Input power, derived from input current sensor. > - > -power3_label "pout" > -power3_input Output power. > - > -temp1_input VCORE temperature. > -temp1_crit Critical high temperature. > -temp1_crit_alarm Chip temperature critical high alarm. > -temp1_maxMaximum temperature. > -temp1_max_alarm Chip temperature high alarm. > - > -temp2_input TSENSE_0 temperature > -temp3_input TSENSE_1 temperature > -temp4_input TSENSE_2 temperature > -temp5_input TSENSE_3 temperature > - > -temp6_input VSA temperature. > -temp6_crit Critical high temperature. > -temp6_crit_alarm Chip temperature critical high alarm. > -temp6_maxMaximum temperature. > -temp6_max_alarm Chip temperature high alarm. > -=== > === > += > === > +in1_label "vin1" > +in1_input VCORE input voltage. > +in1_alarm Input voltage alarm. > + > +in2_label "vout1" > +in2_input VCORE output voltage. > +in2_alarm Output voltage alarm. > + > +curr1_label"iin1" > +curr1_inputVCORE input current, derived from duty cycle and > output > + current. > +curr1_max Maximum input current. > +curr1_max_alarmCurrent high alarm. > + > +curr[P+2]_label"iin1.P" > +curr[P+2]_inputVCORE phase P input current. > + > +curr[N+2]_label"iin2" > +curr[N+2]_inputVCORE input current, derived from sensor > element. > + 'N' is the number of
[PATCH v2] ext4: Enable code path when DX_DEBUG is set
By enabling -Wunreachable-code-aggressive on Clang the following code paths are unreachable. This has been present since commit ac27a0ec112a ("[PATCH] ext4: initial copy of files from ext3") and fs/ext3 had it present at the beginning of git history. It has not been changed since. Clang warns: fs/ext4/namei.c:831:17: warning: code will never be executed [-Wunreachable-code] unsigned n = count - 1; ^ fs/ext4/namei.c:830:7: note: silence by adding parentheses to mark code as explicitly dead if (0) { // linear search cross check ^ /* DISABLES CODE */ ( ) Signed-off-by: Vinicius Tinti --- fs/ext4/namei.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index cf652ba3e74d..46ae6a4e4be5 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -827,20 +827,21 @@ dx_probe(struct ext4_filename *fname, struct inode *dir, p = m + 1; } - if (0) { // linear search cross check - unsigned n = count - 1; - at = entries; - while (n--) +#ifdef DX_DEBUG + // linear search cross check + unsigned n = count - 1; + at = entries; + while (n--) + { + dxtrace(printk(KERN_CONT ",")); + if (dx_get_hash(++at) > hash) { - dxtrace(printk(KERN_CONT ",")); - if (dx_get_hash(++at) > hash) - { - at--; - break; - } + at--; + break; } - ASSERT(at == p - 1); } + ASSERT(at == p - 1); +#endif at = p - 1; dxtrace(printk(KERN_CONT " %x->%u\n", -- 2.25.1
ERROR: modpost: "__aeabi_unwind_cpp_pr0" undefined!
Hi Nathan, First bad commit (maybe != root cause): tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 1048ba83fb1c00cd24172e23e8263972f6b5d9ac commit: c39866f268f89868df17724cd2262d121552d8c9 arm/build: Always handle .ARM.exidx and .ARM.extab sections date: 3 months ago config: arm-randconfig-r032-20210201 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 275c6af7d7f1ed63a03d05b4484413e447133269) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c39866f268f89868df17724cd2262d121552d8c9 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout c39866f268f89868df17724cd2262d121552d8c9 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): WARNING: modpost: missing MODULE_LICENSE() in drivers/hwtracing/coresight/coresight.o >> ERROR: modpost: "__aeabi_unwind_cpp_pr0" >> [kernel/trace/preemptirq_delay_test.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH] drm/i915: Remove unreachable code
On Sat, Jan 30, 2021 at 9:45 AM Chris Wilson wrote: > > Quoting Vinicius Tinti (2021-01-30 12:34:11) > > On Fri, Jan 29, 2021 at 08:55:54PM +, Chris Wilson wrote: > > > Quoting Vinicius Tinti (2021-01-29 18:15:19) > > > > By enabling -Wunreachable-code-aggressive on Clang the following code > > > > paths are unreachable. > > > > > > That code exists as commentary and, especially for sdvo, library > > > functions that we may need in future. > > > > I would argue that this code could be removed since it is in git history. > > It can be restored when needed. > > > > This will make the code cleaner. > > It doesn't change the control flow, so no complexity argument. It > removes documentation from the code, so I have the opposite opinion. The last change in sdvo related to this function is from commit ce22c320b8ca ("drm/i915/sdvo: convert to encoder disable/enable"), which dates from 2012. It has not been used or changed for a long time. I think it could be converted to a block comment. This will preserve the documentation purpose. What do you think? All this will avoid having "if (0)". > > > The ivb-gt1 case => as we now set the gt level for ivb, should we not > > > enable the optimisation for ivb unaffected by the w/a? Just no one has > > > taken the time to see if it causes a regression. > > > > I don't know. I just found out that the code is unreachable. > > > > > For error state, the question remains whether we should revert to > > > uncompressed data if the compressed stream is larger than the original. > > > > I don't know too. > > > > In this last two cases the code could be commented and the decisions > > and problems explained in the comment section. > > They already are, that is the point. I meant making them a block comment. For example: /* * Enabling HiZ Raw Stall Optimization, at this point, causes corruption. * * Calling wa_masked_dis with the arguments wal, CACHE_MODE_0_GEN7, * HIZ_RAW_STALL_OPT_DISABLE will cause an HiZ corruption on ivb:g1. */ /* * Should fallback to uncompressed if we increase size * (zstream->total_out > zstream->total_in) by returning -E2BIG? */ > -Chris
[PATCH] f2fs: fix checkpoint mount option wrong combination
From: Daeho Jeong As checkpoint=merge comes in, mount option setting related to checkpoint had been mixed up. Fixed it. Signed-off-by: Daeho Jeong --- fs/f2fs/super.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 56696f6cfa86..8231c888c772 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -930,20 +930,25 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) return -EINVAL; F2FS_OPTION(sbi).unusable_cap_perc = arg; set_opt(sbi, DISABLE_CHECKPOINT); + clear_opt(sbi, MERGE_CHECKPOINT); break; case Opt_checkpoint_disable_cap: if (args->from && match_int(args, )) return -EINVAL; F2FS_OPTION(sbi).unusable_cap = arg; set_opt(sbi, DISABLE_CHECKPOINT); + clear_opt(sbi, MERGE_CHECKPOINT); break; case Opt_checkpoint_disable: set_opt(sbi, DISABLE_CHECKPOINT); + clear_opt(sbi, MERGE_CHECKPOINT); break; case Opt_checkpoint_enable: clear_opt(sbi, DISABLE_CHECKPOINT); + clear_opt(sbi, MERGE_CHECKPOINT); break; case Opt_checkpoint_merge: + clear_opt(sbi, DISABLE_CHECKPOINT); set_opt(sbi, MERGE_CHECKPOINT); break; #ifdef CONFIG_F2FS_FS_COMPRESSION @@ -1142,12 +1147,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) return -EINVAL; } - if (test_opt(sbi, DISABLE_CHECKPOINT) && - test_opt(sbi, MERGE_CHECKPOINT)) { - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); - return -EINVAL; - } - /* Not pass down write hints if the number of active logs is lesser * than NR_CURSEG_PERSIST_TYPE. */ -- 2.30.0.365.g02bc693789-goog
s390-linux-ld: ll_temac_main.c:undefined reference to `devm_platform_ioremap_resource_byname'
Hi Wang, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 1048ba83fb1c00cd24172e23e8263972f6b5d9ac commit: bd69058f50d5ffa659423bcfa6fe6280ce9c760a net: ll_temac: Use devm_platform_ioremap_resource_byname() date: 6 months ago config: s390-randconfig-r034-20210201 (attached as .config) compiler: s390-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bd69058f50d5ffa659423bcfa6fe6280ce9c760a git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout bd69058f50d5ffa659423bcfa6fe6280ce9c760a # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): s390-linux-ld: drivers/net/ethernet/xilinx/ll_temac_main.o: in function `temac_probe': ll_temac_main.c:(.text+0x39b6): undefined reference to `devm_platform_ioremap_resource_byname' >> s390-linux-ld: ll_temac_main.c:(.text+0x3a4c): undefined reference to >> `devm_platform_ioremap_resource_byname' s390-linux-ld: ll_temac_main.c:(.text+0x3bce): undefined reference to `devm_ioremap' s390-linux-ld: drivers/net/ethernet/xilinx/xilinx_axienet_main.o: in function `axienet_probe': xilinx_axienet_main.c:(.text+0x844): undefined reference to `devm_ioremap_resource' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH V2 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
On 1/27/21 6:10 PM, Viresh Kumar wrote: > dev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should > be used instead. Migrate to the new API. > > We don't want the OPP core to manage the clk for this driver, migrate to > dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp() > doesn't have any side effects. > > Signed-off-by: Viresh Kumar > --- > Dmitry, > > This is based over the patches sent here: > > https://protect2.fireeye.com/v1/url?k=72d88562-2d43bc78-72d90e2d-000babff24ad-e4764030101eaedc=1=a25b5db7-346b-47b2-9c0a-3945e579f389=https%3A%2F%2Flore.kernel.org%2Flkml%2F6c2160ff30a8f421563793020264cf9f533f293c.1611738228.git.viresh.kumar%40linaro.org%2F > > This should fix the problem you mentioned earlier. Will push this for > linux-next unless you have any issues with it. > > drivers/devfreq/tegra30-devfreq.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/devfreq/tegra30-devfreq.c > b/drivers/devfreq/tegra30-devfreq.c > index 117cad7968ab..31f7dec5990b 100644 > --- a/drivers/devfreq/tegra30-devfreq.c > +++ b/drivers/devfreq/tegra30-devfreq.c > @@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, > unsigned long *freq, > return PTR_ERR(opp); > } > > - ret = dev_pm_opp_set_bw(dev, opp); > + ret = dev_pm_opp_set_opp(dev, opp); > dev_pm_opp_put(opp); > > return ret; > @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device > *pdev) > return err; > } > > - err = dev_pm_opp_of_add_table(>dev); > + err = dev_pm_opp_of_add_table_noclk(>dev); > if (err) { > dev_err(>dev, "Failed to add OPP table: %d\n", err); > goto put_hw; > Acked-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 3:35 PM Linus Torvalds wrote: > > I wonder if the simple solution is to just > > (a) always set one of the SYSCALL_WORK_EXIT bits on the child in > ptrace (exactly to catch the child on system call exit) > > (b) basically revert 299155244770 ("entry: Drop usage of TIF flags in > the generic syscall code") and have the syscall exit code check the > TIF_SINGLESTEP flag Actually, (b) looks unnecessary - as long as we get to syscall_exit_work(), the current code will work fine. So maybe just add a dummy SYSCALL_WORK_SYSCALL_EXIT_TRAP, and set that flag whenever a singestep is requested for a process that is currently in a system call? IOW, make it a very explicit "do TF for system calls", rather than the old code that was doing so implicitly and not very obviously. Hmm? Linus
Re: [net-next 00/14] Add Marvell CN10K support
On Sat, Jan 30, 2021 at 12:04 PM Geetha sowjanya wrote: > > The current admin function (AF) driver and the netdev driver supports > OcteonTx2 silicon variants. The same OcteonTx2's Resource Virtualization Unit > (RVU) > is carried forward to the next-gen silicon ie OcteonTx3, with some changes > and feature enhancements. > > This patch set adds support for OcteonTx3 (CN10K) silicon and gets the drivers > to the same level as OcteonTx2. No new OcteonTx3 specific features are added. > Changes cover below HW level differences > - PCIe BAR address changes wrt shared mailbox memory region > - Receive buffer freeing to HW > - Transmit packet's descriptor submission to HW > - Programmable HW interface identifiers (channels) > - Increased MTU support > - A Serdes MAC block (RPM) configuration > > Geetha sowjanya (6): > octeontx2-af: cn10k: Update NIX/NPA context structure > octeontx2-af: cn10k: Update NIX and NPA context in debugfs > octeontx2-pf: cn10k: Initialise NIX context > octeontx2-pf: cn10k: Map LMTST region > octeontx2-pf: cn10k: Use LMTST lines for NPA/NIX operations > > Hariprasad Kelam (5): > octeontx2-af: cn10k: Add RPM MAC support > octeontx2-af: cn10K: Add MTU configuration > octeontx2-pf: cn10k: Get max mtu supported from admin function > octeontx2-af: cn10k: Add RPM Rx/Tx stats support > octeontx2-af: cn10k: MAC internal loopback support > > Rakesh Babu (1): > octeontx2-af: cn10k: Add RPM LMAC pause frame support > > Subbaraya Sundeep (2): > octeontx2-af: cn10k: Add mbox support for CN10K platform > octeontx2-pf: cn10k: Add mbox support for CN10K > octeontx2-af: cn10k: Add support for programmable channels > > drivers/net/ethernet/marvell/octeontx2/af/Makefile | 2 +- > drivers/net/ethernet/marvell/octeontx2/af/cgx.c| 315 --- > drivers/net/ethernet/marvell/octeontx2/af/cgx.h| 15 +- > .../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 1 + > drivers/net/ethernet/marvell/octeontx2/af/common.h | 5 + > .../ethernet/marvell/octeontx2/af/lmac_common.h| 129 + > drivers/net/ethernet/marvell/octeontx2/af/mbox.c | 59 +- > drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 70 ++- > drivers/net/ethernet/marvell/octeontx2/af/rpm.c| 272 ++ > drivers/net/ethernet/marvell/octeontx2/af/rpm.h| 57 ++ > drivers/net/ethernet/marvell/octeontx2/af/rvu.c| 157 +- > drivers/net/ethernet/marvell/octeontx2/af/rvu.h| 70 +++ > .../net/ethernet/marvell/octeontx2/af/rvu_cgx.c| 135 - > .../net/ethernet/marvell/octeontx2/af/rvu_cn10k.c | 261 + > .../ethernet/marvell/octeontx2/af/rvu_debugfs.c| 339 +++- > .../net/ethernet/marvell/octeontx2/af/rvu_nix.c| 112 +++- > .../net/ethernet/marvell/octeontx2/af/rvu_npc.c| 4 +- > .../net/ethernet/marvell/octeontx2/af/rvu_reg.h| 24 + > .../net/ethernet/marvell/octeontx2/af/rvu_struct.h | 604 > ++--- > .../net/ethernet/marvell/octeontx2/nic/Makefile| 2 +- > drivers/net/ethernet/marvell/octeontx2/nic/cn10k.c | 182 +++ > drivers/net/ethernet/marvell/octeontx2/nic/cn10k.h | 17 + > .../ethernet/marvell/octeontx2/nic/otx2_common.c | 144 +++-- > .../ethernet/marvell/octeontx2/nic/otx2_common.h | 105 +++- > .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 67 ++- > .../net/ethernet/marvell/octeontx2/nic/otx2_reg.h | 4 + > .../ethernet/marvell/octeontx2/nic/otx2_struct.h | 10 +- > .../net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 70 ++- > .../net/ethernet/marvell/octeontx2/nic/otx2_txrx.h | 8 +- > .../net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 52 +- > include/linux/soc/marvell/octeontx2/asm.h | 8 + > 31 files changed, 2573 insertions(+), 727 deletions(-) > create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/lmac_common.h > create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rpm.c > create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rpm.h > create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c > create mode 100644 drivers/net/ethernet/marvell/octeontx2/nic/cn10k.c > create mode 100644 drivers/net/ethernet/marvell/octeontx2/nic/cn10k.h > FYI, patchwork shows a number of checkpatch and build warnings to fix up https://patchwork.kernel.org/project/netdevbpf/list/?series=424847
RPi4 DWC2 gadget doesn't copy data to a buffer in ep0 SETUP + DATA OUT transaction
Hi Minas and other USB experts, I'm currently developing new features for UAC1/UAC2 audio gadgets like Volume/Mute controls which use Control SETUP + DATA OUT transactions through ep0. While it works fine on BeagleBone black board with MUSB UDC, on Raspberry Pi 4 with DWC2 UDC there is an issue. I found that when DWC2 receives ep0 SETUP + DATA OUT transaction, it doesn't copy data transferred from the host to EP0 in DATA OUT stage to the usb_request->buf buffer. This buffer contains unchanged data from previous transactions. However, when I disable DMA for DWC2 controller (see the patch below) it starts to work as expected and correctly copies data transferred from the host in the DATA OUT stage, to the usb_request->buf buffer. So far I tested it on v5.9 kernel and v5.10.10 stable kernel both have the same issue. This issue is easily reproducible with RNDIS gadget which also uses ep0 SETUP + DATA OUT transactions for transferring RNDIS control messages. During enumeration of RNDIS gadget attached to Linux host, I see next messages for RPi4 DWC2 with DMA enabled: | ## RPi4 DWC2 DMA | [ 91.029881] rndis_msg_parser: unknown RNDIS message 0x0052033A len 4456526 | [ 91.029889] RNDIS command error -524, 24/24 In this case rndis_msg_parser can't parse messages from the host because they are sent through SETUP + DATA OUT transaction and DWC2 didn't copy that messages to the buffer, so buffer contains some garbage from previous transactions which can't be parsed. In case of BBB black or DWC2 with disabled DMA, it looks like: | ## BBB black | [ 32.867751] rndis_msg_parser: RNDIS_MSG_INIT | ## RPi4 DWC2 no DMA | [ 151.080724] rndis_msg_parser: RNDIS_MSG_INIT I also did a quick googling and found that same issue was recently reported for Raspberry pi OS: https://github.com/raspberrypi/Raspberry-Pi-OS-64bit/issues/127 I spent some time on debugging this issue, but without having DWC2 documentation and experience with DWC2 internals that's all that I found so far. Is this a known issue? Anybody debugging it? Any ideas? Thanks, Ruslan -8< >From ced7a3631d9800d04bcbcd756dac4583459fe48c Mon Sep 17 00:00:00 2001 From: Ruslan Bilovol Date: Wed, 20 Jan 2021 00:27:52 +0200 Subject: [PATCH] usb: dwc2: workaround: disable DMA for gadget On Raspberry PI 4 it was observer that in case of control transfers with DATA phase from a host, the driver for some reason doesn't copy transferred data to the buffer, leaving previous data in it. With disabled DMA the issue isn't reproducible, thus temporarily disable it Signed-off-by: Ruslan Bilovol --- drivers/usb/dwc2/params.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index 267543c..46c18af 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -357,7 +357,11 @@ static void dwc2_set_default_params(struct dwc2_hsotg *hsotg) { struct dwc2_hw_params *hw = >hw_params; struct dwc2_core_params *p = >params; +#if 0 bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH); +#else + bool dma_capable = 0; +#endif dwc2_set_param_otg_cap(hsotg); dwc2_set_param_phy_type(hsotg); @@ -651,7 +655,11 @@ static void dwc2_check_params(struct dwc2_hsotg *hsotg) { struct dwc2_hw_params *hw = >hw_params; struct dwc2_core_params *p = >params; +#if 0 bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH); +#else + bool dma_capable = 0; +#endif dwc2_check_param_otg_cap(hsotg); dwc2_check_param_phy_type(hsotg); -- 1.9.1
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 3:39 PM Kyle Huey wrote: > > On Sun, Jan 31, 2021 at 3:36 PM Andy Lutomirski wrote: > > > The odd system call tracing part I have no idea who depends on it > > > (apparently "rr", which I assume is some replay thing), and I suspect > > > our semantics for it has been basically random historical one, and > > > it's apparently what changed. > > > > > > That's the one that we _really_ should have a test-case for, along > > > with some documentation and code comment what the actual semantics > > > need to be so that we don't break it again. > > > > This rr thing may be tangled up with the nonsense semantics of SYSRET. > > I’ll muck around with Kyle’s test and try to figure out what broke. > > > > I’m guessing the issue is that we are correctly setting TF in the EFLAGS > > image, but IRET helpfully only traps after the first user insn executes, > > which isn’t what the tracer is expects. > > The state of TF shouldn't really matter here. There should be no user > space code execution in the example I gave. This behavior all happens > in the kernel and not on the silicon. > I admit that PTRACE_SINGLESTEP seems like an odd way to spell "advance to the end of the syscall", but you're right, it should work.
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 3:36 PM Andy Lutomirski wrote: > > The odd system call tracing part I have no idea who depends on it > > (apparently "rr", which I assume is some replay thing), and I suspect > > our semantics for it has been basically random historical one, and > > it's apparently what changed. > > > > That's the one that we _really_ should have a test-case for, along > > with some documentation and code comment what the actual semantics > > need to be so that we don't break it again. > > This rr thing may be tangled up with the nonsense semantics of SYSRET. I’ll > muck around with Kyle’s test and try to figure out what broke. > > I’m guessing the issue is that we are correctly setting TF in the EFLAGS > image, but IRET helpfully only traps after the first user insn executes, > which isn’t what the tracer is expects. The state of TF shouldn't really matter here. There should be no user space code execution in the example I gave. This behavior all happens in the kernel and not on the silicon. - Kyle
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
> On Jan 31, 2021, at 2:57 PM, Linus Torvalds > wrote: > > On Sun, Jan 31, 2021 at 2:20 PM Andy Lutomirski wrote: >> >> A smallish test that we could stick in selftests would be great if that’s >> straightforward. > > Side note: it would be good to have a test-case for the interaction > with the "block step" code too. > > I hate our name for it ("block step"?), but it modifies TF, to only > trap on taken branches, not after each instruction. > > So there's all these things that interact: > > - you can set TF yourself in user space with 'popf' and get a debug > signal (not after the popf, but after the instruction _after_ popf, > iirc) > > - you can set TF as a debugger and that's basically what > TIF_SINGLESTEP fundamentally means > > - we have TIF_FORCED_TF, which says whether TF was set by ptrace, or > whether it was already set independently by the application before > ptrace, so that we can know whether to clear it or keep it set *after* > the single-step. > > - you can then *modify* the behavior of TF to trap only on control > flow changes (and we use TIF_BLOCKSTEP to specify that behavior) > > - and there's also obviously some very subtle and unclear rules about > when system call instructions cause debug traps > > The basic TF behavior is fairly simple: it caused #DB, and we send a signal. This is all fundamentally impossible to do fully correctly because a program can use PUSHF to read TF, and there is only one TF bit, and the app and the debugger can fight over it. The insn breakpoint mechanism is much better, but even AMD CPUs can’t (I think) be programmed to breakpoint the entire user address space. So we do our best to fudge it. > > The "app set TF _itself_, and we want to debug across that event" is a > lot more interesting, but it's unclear whether anybody does it. It's > really just a "we want to be able to debug that case too", and > TIF_FORCED_TF means that it should be possible. > > I didn't test that it works, though. Sounds worth a test-case? > I can look. We do have tests for apps setting TF with no debugger attached. > The TIF_BLOCKSTEP thing changes no other logic, but basically sets the > bit in the MSR that modifies just when TF traps. I may hate the name, > but I think it works. > It has certainly been busted in the past in corner cases. I don’t think we have tests. > The odd system call tracing part I have no idea who depends on it > (apparently "rr", which I assume is some replay thing), and I suspect > our semantics for it has been basically random historical one, and > it's apparently what changed. > > That's the one that we _really_ should have a test-case for, along > with some documentation and code comment what the actual semantics > need to be so that we don't break it again. This rr thing may be tangled up with the nonsense semantics of SYSRET. I’ll muck around with Kyle’s test and try to figure out what broke. I’m guessing the issue is that we are correctly setting TF in the EFLAGS image, but IRET helpfully only traps after the first user insn executes, which isn’t what the tracer is expects. > > Linus
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 3:18 PM Kyle Huey wrote: > > The key to triggering this bug is to enter a ptrace syscall stop and > then use PTRACE_SINGLESTEP to exit it. On a good kernel this will not > result in any userspace code execution in the tracee because on the > way out of the kernel's syscall handling path the singlestep trap will > be raised immediately. On a bad kernel that stop will not be raised, > and in the example below, the program will crash. Thanks, great explanation, and I can certainly see the behavior you mention. I wonder if the simple solution is to just (a) always set one of the SYSCALL_WORK_EXIT bits on the child in ptrace (exactly to catch the child on system call exit) (b) basically revert 299155244770 ("entry: Drop usage of TIF flags in the generic syscall code") and have the syscall exit code check the TIF_SINGLESTEP flag Hmm? Linus
[PATCH v6 1/4] crypto: Add support for ECDSA signature verification
Add support for parsing the parameters of a NIST P256 or NIST P192 key. Enable signature verification using these keys. The new module is enabled with CONFIG_ECDSA: Elliptic Curve Digital Signature Algorithm (NIST P192, P256 etc.) is A NIST cryptographic standard algorithm. Only signature verification is implemented. Signed-off-by: Stefan Berger Cc: Herbert Xu Cc: "David S. Miller" Cc: linux-cry...@vger.kernel.org --- crypto/Kconfig | 10 + crypto/Makefile| 6 + crypto/ecc.c | 13 +- crypto/ecc.h | 28 +++ crypto/ecdsa.c | 361 + crypto/ecdsasignature.asn1 | 4 + crypto/testmgr.c | 12 ++ crypto/testmgr.h | 267 +++ 8 files changed, 690 insertions(+), 11 deletions(-) create mode 100644 crypto/ecdsa.c create mode 100644 crypto/ecdsasignature.asn1 diff --git a/crypto/Kconfig b/crypto/Kconfig index 094ef56ab7b4..152a4ee54fc6 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -247,6 +247,16 @@ config CRYPTO_ECDH help Generic implementation of the ECDH algorithm +config CRYPTO_ECDSA + tristate "ECDSA (NIST P192, P256 etc.) algorithm" + select CRYPTO_ECC + select CRYPTO_AKCIPHER + select ASN1 + help + Elliptic Curve Digital Signature Algorithm (NIST P192, P256 etc.) + is A NIST cryptographic standard algorithm. Only signature verification + is implemented. + config CRYPTO_ECRDSA tristate "EC-RDSA (GOST 34.10) algorithm" select CRYPTO_ECC diff --git a/crypto/Makefile b/crypto/Makefile index b279483fba50..982066c6bdfb 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -50,6 +50,12 @@ sm2_generic-y += sm2.o obj-$(CONFIG_CRYPTO_SM2) += sm2_generic.o +$(obj)/ecdsasignature.asn1.o: $(obj)/ecdsasignature.asn1.c $(obj)/ecdsasignature.asn1.h +$(obj)/ecdsa.o: $(obj)/ecdsasignature.asn1.h +ecdsa_generic-y += ecdsa.o +ecdsa_generic-y += ecdsasignature.asn1.o +obj-$(CONFIG_CRYPTO_ECDSA) += ecdsa_generic.o + crypto_acompress-y := acompress.o crypto_acompress-y += scompress.o obj-$(CONFIG_CRYPTO_ACOMP2) += crypto_acompress.o diff --git a/crypto/ecc.c b/crypto/ecc.c index c80aa25994a0..25e79fd70566 100644 --- a/crypto/ecc.c +++ b/crypto/ecc.c @@ -42,7 +42,7 @@ typedef struct { u64 m_high; } uint128_t; -static inline const struct ecc_curve *ecc_get_curve(unsigned int curve_id) +const struct ecc_curve *ecc_get_curve(unsigned int curve_id) { switch (curve_id) { /* In FIPS mode only allow P256 and higher */ @@ -54,6 +54,7 @@ static inline const struct ecc_curve *ecc_get_curve(unsigned int curve_id) return NULL; } } +EXPORT_SYMBOL(ecc_get_curve); static u64 *ecc_alloc_digits_space(unsigned int ndigits) { @@ -1281,16 +1282,6 @@ void ecc_point_mult_shamir(const struct ecc_point *result, } EXPORT_SYMBOL(ecc_point_mult_shamir); -static inline void ecc_swap_digits(const u64 *in, u64 *out, - unsigned int ndigits) -{ - const __be64 *src = (__force __be64 *)in; - int i; - - for (i = 0; i < ndigits; i++) - out[i] = be64_to_cpu(src[ndigits - 1 - i]); -} - static int __ecc_is_key_valid(const struct ecc_curve *curve, const u64 *private_key, unsigned int ndigits) { diff --git a/crypto/ecc.h b/crypto/ecc.h index d4e546b9ad79..2ea86dfb5cf7 100644 --- a/crypto/ecc.h +++ b/crypto/ecc.h @@ -33,6 +33,8 @@ #define ECC_DIGITS_TO_BYTES_SHIFT 3 +#define ECC_MAX_BYTES (ECC_MAX_DIGITS << ECC_DIGITS_TO_BYTES_SHIFT) + /** * struct ecc_point - elliptic curve point in affine coordinates * @@ -70,6 +72,32 @@ struct ecc_curve { u64 *b; }; +/** + * ecc_swap_digits() - Copy ndigits from big endian array to native array + * + * @in: input array + * @out: output array + * @ndigits: number of digits to copy + */ +static inline void ecc_swap_digits(const u64 *in, u64 *out, + unsigned int ndigits) +{ + const __be64 *src = (__force __be64 *)in; + int i; + + for (i = 0; i < ndigits; i++) + out[i] = be64_to_cpu(src[ndigits - 1 - i]); +} + +/** + * ecc_get_curve() - Get a curve given its curve_id + * + * @curve_id: Id of the curve + * + * Returns pointer to the curve data, NULL if curve is not available + */ +const struct ecc_curve *ecc_get_curve(unsigned int curve_id); + /** * ecc_is_key_valid() - Validate a given ECDH private key * diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c new file mode 100644 index ..4b45230276b3 --- /dev/null +++ b/crypto/ecdsa.c @@ -0,0 +1,361 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2021 IBM Corporation + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * * Redistributions of
[PATCH v6 4/4] ima: Support EC keys for signature verification
Add support for IMA signature verification for EC keys. Since SHA type of hashes can be used by RSA and ECDSA signature schemes we need to look at the key and derive from the key which signature scheme to use. Since this can be applied to all types of keys, we change the selection of the encoding type to be driven by the key's signature scheme rather than by the hash type. Signed-off-by: Stefan Berger Reviewed-by: Vitaly Chikunov Cc: Mimi Zohar Cc: Dmitry Kasatkin Cc: linux-integr...@vger.kernel.org Cc: Vitaly Chikunov Cc: Tianjia Zhang Cc: David Howells Cc: keyri...@vger.kernel.org --- include/keys/asymmetric-type.h | 6 ++ security/integrity/digsig_asymmetric.c | 29 -- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h index a29d3ff2e7e8..c432fdb8547f 100644 --- a/include/keys/asymmetric-type.h +++ b/include/keys/asymmetric-type.h @@ -72,6 +72,12 @@ const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key) return key->payload.data[asym_key_ids]; } +static inline +const struct public_key *asymmetric_key_public_key(const struct key *key) +{ + return key->payload.data[asym_crypto]; +} + extern struct key *find_asymmetric_key(struct key *keyring, const struct asymmetric_key_id *id_0, const struct asymmetric_key_id *id_1, diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index a662024b4c70..29002d016607 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -84,6 +84,7 @@ int asymmetric_verify(struct key *keyring, const char *sig, { struct public_key_signature pks; struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig; + const struct public_key *pk; struct key *key; int ret; @@ -105,23 +106,19 @@ int asymmetric_verify(struct key *keyring, const char *sig, memset(, 0, sizeof(pks)); pks.hash_algo = hash_algo_name[hdr->hash_algo]; - switch (hdr->hash_algo) { - case HASH_ALGO_STREEBOG_256: - case HASH_ALGO_STREEBOG_512: - /* EC-RDSA and Streebog should go together. */ - pks.pkey_algo = "ecrdsa"; - pks.encoding = "raw"; - break; - case HASH_ALGO_SM3_256: - /* SM2 and SM3 should go together. */ - pks.pkey_algo = "sm2"; - pks.encoding = "raw"; - break; - default: - pks.pkey_algo = "rsa"; + + pk = asymmetric_key_public_key(key); + pks.pkey_algo = pk->pkey_algo; + if (!strcmp(pk->pkey_algo, "rsa")) pks.encoding = "pkcs1"; - break; - } + else if (!strcmp(pk->pkey_algo, "ecdsa")) + pks.encoding = "x962"; + else if (!strcmp(pk->pkey_algo, "ecrdsa") || + !strcmp(pk->pkey_algo, "sm2")) + pks.encoding = "raw"; + else + return -ENOPKG; + pks.digest = (u8 *)data; pks.digest_size = datalen; pks.s = hdr->sig; -- 2.29.2
[PATCH v6 0/4] Add support for x509 certs with NIST p256 and p192 keys
This series of patches adds support for x509 certificates signed by a CA that uses NIST p256 or p192 keys for signing. It also adds support for certificates where the public key is a NIST p256 or p192 key. The math for ECDSA signature verification is also added. Since self-signed certificates are verified upon loading, the following script can be used for testing: k=$(keyctl newring test @u) while :; do for hash in sha1 sha224 sha256 sha384 sha512; do openssl req \ -x509 \ -${hash} \ -newkey ec \ -pkeyopt ec_paramgen_curve:prime256v1 \ -keyout key.pem \ -days 365 \ -subj '/CN=test' \ -nodes \ -outform der \ -out cert.der keyctl padd asymmetric testkey $k < cert.der if [ $? -ne 0 ]; then echo "ERROR" exit 1 fi done done It also works with restricted keyrings where an RSA key is used to sign a NIST P256/P192 key. Scripts for testing are here: https://github.com/stefanberger/eckey-testing The ECDSA signature verification will be used by IMA Appraisal where ECDSA file signatures stored in RPM packages will use substantially less space than if RSA signatures were to be used. Stefan v5->v6: - moved ecdsa code into its own module ecdsa_generic built from ecdsa.c - added script-generated test vectors for NIST P256 & P192 and all hashes - parsing of OID that contain header with new parse_oid() v4->v5: - registering crypto support under names ecdsa-nist-p256/p192 following Hubert Xu's suggestion in other thread - appended IMA ECDSA support patch v3->v4: - split off of ecdsa crypto part; registering akcipher as "ecdsa" and deriving used curve from digits in parsed key v2->v3: - patch 2 now includes linux/scatterlist.h v1->v2: - using faster vli_sub rather than newly added vli_mod_fast to 'reduce' result - rearranged switch statements to follow after RSA - 3rd patch from 1st posting is now 1st patch Stefan Berger (4): crypto: Add support for ECDSA signature verification x509: Detect sm2 keys by their parameters OID x509: Add support for parsing x509 certs with ECDSA keys ima: Support EC keys for signature verification crypto/Kconfig| 10 + crypto/Makefile | 6 + crypto/asymmetric_keys/public_key.c | 19 ++ crypto/asymmetric_keys/x509_cert_parser.c | 44 ++- crypto/ecc.c | 13 +- crypto/ecc.h | 28 ++ crypto/ecdsa.c| 361 ++ crypto/ecdsasignature.asn1| 4 + crypto/testmgr.c | 12 + crypto/testmgr.h | 267 include/keys/asymmetric-type.h| 6 + include/linux/oid_registry.h | 7 + lib/oid_registry.c| 13 + security/integrity/digsig_asymmetric.c| 29 +- 14 files changed, 790 insertions(+), 29 deletions(-) create mode 100644 crypto/ecdsa.c create mode 100644 crypto/ecdsasignature.asn1 -- 2.29.2
[PATCH v6 3/4] x509: Add support for parsing x509 certs with ECDSA keys
This patch adds support for parsing of x509 certificates that contain ECDSA keys, such as NIST P256, that have been signed by a CA using any of the current SHA hash algorithms. Signed-off-by: Stefan Berger Cc: David Howells Cc: keyri...@vger.kernel.org --- crypto/asymmetric_keys/public_key.c | 19 ++ crypto/asymmetric_keys/x509_cert_parser.c | 32 ++- include/linux/oid_registry.h | 6 + 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 8892908ad58c..7dae61b79d5a 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -90,6 +91,24 @@ int software_key_determine_akcipher(const char *encoding, return 0; } + if (strcmp(encoding, "x962") == 0) { + enum OID oid; + + if (parse_OID(pkey->params, pkey->paramlen, ) != 0) + return -EBADMSG; + + switch (oid) { + case OID_id_prime192v1: + strcpy(alg_name, "ecdsa-nist-p192"); + return 0; + case OID_id_prime256v1: + strcpy(alg_name, "ecdsa-nist-p256"); + return 0; + default: + return -EINVAL; + } + } + return -ENOPKG; } diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 1621ceaf5c95..0aff4e584b11 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -227,6 +227,26 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, ctx->cert->sig->hash_algo = "sha224"; goto rsa_pkcs1; + case OID_id_ecdsa_with_sha1: + ctx->cert->sig->hash_algo = "sha1"; + goto ecdsa; + + case OID_id_ecdsa_with_sha224: + ctx->cert->sig->hash_algo = "sha224"; + goto ecdsa; + + case OID_id_ecdsa_with_sha256: + ctx->cert->sig->hash_algo = "sha256"; + goto ecdsa; + + case OID_id_ecdsa_with_sha384: + ctx->cert->sig->hash_algo = "sha384"; + goto ecdsa; + + case OID_id_ecdsa_with_sha512: + ctx->cert->sig->hash_algo = "sha512"; + goto ecdsa; + case OID_gost2012Signature256: ctx->cert->sig->hash_algo = "streebog256"; goto ecrdsa; @@ -255,6 +275,11 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, ctx->cert->sig->encoding = "raw"; ctx->algo_oid = ctx->last_oid; return 0; +ecdsa: + ctx->cert->sig->pkey_algo = "ecdsa"; + ctx->cert->sig->encoding = "x962"; + ctx->algo_oid = ctx->last_oid; + return 0; } /* @@ -276,7 +301,8 @@ int x509_note_signature(void *context, size_t hdrlen, if (strcmp(ctx->cert->sig->pkey_algo, "rsa") == 0 || strcmp(ctx->cert->sig->pkey_algo, "ecrdsa") == 0 || - strcmp(ctx->cert->sig->pkey_algo, "sm2") == 0) { + strcmp(ctx->cert->sig->pkey_algo, "sm2") == 0 || + strcmp(ctx->cert->sig->pkey_algo, "ecdsa") == 0) { /* Discard the BIT STRING metadata */ if (vlen < 1 || *(const u8 *)value != 0) return -EBADMSG; @@ -478,6 +504,10 @@ int x509_extract_key_data(void *context, size_t hdrlen, case OID_sm2: ctx->cert->pub->pkey_algo = "sm2"; break; + case OID_id_prime192v1: + case OID_id_prime256v1: + ctx->cert->pub->pkey_algo = "ecdsa"; + break; default: return -ENOPKG; } diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index d4982e42c0d2..ff3cad9f8c1f 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -21,6 +21,12 @@ enum OID { OID_id_dsa, /* 1.2.840.10040.4.1 */ OID_id_ecdsa_with_sha1, /* 1.2.840.10045.4.1 */ OID_id_ecPublicKey, /* 1.2.840.10045.2.1 */ + OID_id_prime192v1, /* 1.2.840.10045.3.1.1 */ + OID_id_prime256v1, /* 1.2.840.10045.3.1.7 */ + OID_id_ecdsa_with_sha224, /* 1.2.840.10045.4.3.1 */ + OID_id_ecdsa_with_sha256, /* 1.2.840.10045.4.3.2 */ + OID_id_ecdsa_with_sha384, /* 1.2.840.10045.4.3.3 */ + OID_id_ecdsa_with_sha512, /* 1.2.840.10045.4.3.4 */ /* PKCS#1 {iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1) pkcs-1(1)} */ OID_rsaEncryption, /* 1.2.840.113549.1.1.1 */ -- 2.29.2
[PATCH v6 2/4] x509: Detect sm2 keys by their parameters OID
Detect whether a key is an sm2 type of key by its OID in the parameters array rather than assuming that everything under OID_id_ecPublicKey is sm2, which is not the case. Signed-off-by: Stefan Berger Cc: David Howells Cc: keyri...@vger.kernel.org --- crypto/asymmetric_keys/x509_cert_parser.c | 12 +++- include/linux/oid_registry.h | 1 + lib/oid_registry.c| 13 + 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 52c9b455fc7d..1621ceaf5c95 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -459,6 +459,7 @@ int x509_extract_key_data(void *context, size_t hdrlen, const void *value, size_t vlen) { struct x509_parse_context *ctx = context; + enum OID oid; ctx->key_algo = ctx->last_oid; switch (ctx->last_oid) { @@ -470,7 +471,16 @@ int x509_extract_key_data(void *context, size_t hdrlen, ctx->cert->pub->pkey_algo = "ecrdsa"; break; case OID_id_ecPublicKey: - ctx->cert->pub->pkey_algo = "sm2"; + if (parse_OID(ctx->params, ctx->params_size, ) != 0) + return -EBADMSG; + + switch (oid) { + case OID_sm2: + ctx->cert->pub->pkey_algo = "sm2"; + break; + default: + return -ENOPKG; + } break; default: return -ENOPKG; diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index 4462ed2c18cd..d4982e42c0d2 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -117,6 +117,7 @@ enum OID { }; extern enum OID look_up_OID(const void *data, size_t datasize); +extern int parse_OID(const void *data, size_t datasize, enum OID *oid); extern int sprint_oid(const void *, size_t, char *, size_t); extern int sprint_OID(enum OID, char *, size_t); diff --git a/lib/oid_registry.c b/lib/oid_registry.c index f7ad43f28579..508e0b34b5f0 100644 --- a/lib/oid_registry.c +++ b/lib/oid_registry.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "oid_registry_data.c" MODULE_DESCRIPTION("OID Registry"); @@ -92,6 +93,18 @@ enum OID look_up_OID(const void *data, size_t datasize) } EXPORT_SYMBOL_GPL(look_up_OID); +int parse_OID(const void *data, size_t datasize, enum OID *oid) +{ + const unsigned char *v = data; + + if (datasize < 2 || v[0] != ASN1_OID || v[1] != datasize - 2) + return -EBADMSG; + + *oid = look_up_OID(data + 2, datasize - 2); + return 0; +} +EXPORT_SYMBOL_GPL(parse_OID); + /* * sprint_OID - Print an Object Identifier into a buffer * @data: The encoded OID to print -- 2.29.2
[PATCH v3] misc: bcm-vk: only support ttyVK if CONFIG_TTY is set
Correct compile issue if CONFIG_TTY is not set by only adding ttyVK devices if CONFIG_BCM_VK_TTY is set. Reported-by: Randy Dunlap Signed-off-by: Scott Branden --- Changes since v2: - add CONFIG_BCM_VK_TTY - add function and stub for bcm_vk_tty_set_irq_enabled Changes since v1: - add function stubs rather than compiling out code --- drivers/misc/bcm-vk/Kconfig | 16 drivers/misc/bcm-vk/Makefile | 4 +-- drivers/misc/bcm-vk/bcm_vk.h | 42 +--- drivers/misc/bcm-vk/bcm_vk_dev.c | 5 ++-- drivers/misc/bcm-vk/bcm_vk_tty.c | 6 + 5 files changed, 65 insertions(+), 8 deletions(-) diff --git a/drivers/misc/bcm-vk/Kconfig b/drivers/misc/bcm-vk/Kconfig index 052f6f28b540..16ce98c964b8 100644 --- a/drivers/misc/bcm-vk/Kconfig +++ b/drivers/misc/bcm-vk/Kconfig @@ -15,3 +15,19 @@ config BCM_VK accelerators via /dev/bcm-vk.N devices. If unsure, say N. + +if BCM_VK + +config BCM_VK_TTY + bool "Enable ttyVK" + depends on TTY + default y + help + Select this option to enable ttyVK support to allow console + access to VK cards from host. + + Device node will in the form /dev/bcm-vk.x_ttyVKy where: + x is the instance of the VK card + y is the tty device number on the VK card. + +endif # BCM_VK diff --git a/drivers/misc/bcm-vk/Makefile b/drivers/misc/bcm-vk/Makefile index e4a1486f7209..1df2ebe851ca 100644 --- a/drivers/misc/bcm-vk/Makefile +++ b/drivers/misc/bcm-vk/Makefile @@ -7,6 +7,6 @@ obj-$(CONFIG_BCM_VK) += bcm_vk.o bcm_vk-objs := \ bcm_vk_dev.o \ bcm_vk_msg.o \ - bcm_vk_sg.o \ - bcm_vk_tty.o + bcm_vk_sg.o +bcm_vk-$(CONFIG_BCM_VK_TTY) += bcm_vk_tty.o diff --git a/drivers/misc/bcm-vk/bcm_vk.h b/drivers/misc/bcm-vk/bcm_vk.h index 3f37c640a814..a1338f375589 100644 --- a/drivers/misc/bcm-vk/bcm_vk.h +++ b/drivers/misc/bcm-vk/bcm_vk.h @@ -258,7 +258,11 @@ enum pci_barno { BAR_2 }; +#ifdef CONFIG_BCM_VK_TTY #define BCM_VK_NUM_TTY 2 +#else +#define BCM_VK_NUM_TTY 0 +#endif struct bcm_vk_tty { struct tty_port port; @@ -366,11 +370,13 @@ struct bcm_vk { struct miscdevice miscdev; int devid; /* dev id allocated */ +#ifdef CONFIG_BCM_VK_TTY struct tty_driver *tty_drv; struct timer_list serial_timer; struct bcm_vk_tty tty[BCM_VK_NUM_TTY]; struct workqueue_struct *tty_wq_thread; struct work_struct tty_wq_work; +#endif /* Reference-counting to handle file operations */ struct kref kref; @@ -501,13 +507,43 @@ int bcm_vk_send_shutdown_msg(struct bcm_vk *vk, u32 shut_type, const pid_t pid, const u32 q_num); void bcm_to_v_q_doorbell(struct bcm_vk *vk, u32 q_num, u32 db_val); int bcm_vk_auto_load_all_images(struct bcm_vk *vk); -int bcm_vk_tty_init(struct bcm_vk *vk, char *name); -void bcm_vk_tty_exit(struct bcm_vk *vk); -void bcm_vk_tty_terminate_tty_user(struct bcm_vk *vk); void bcm_vk_hb_init(struct bcm_vk *vk); void bcm_vk_hb_deinit(struct bcm_vk *vk); void bcm_vk_handle_notf(struct bcm_vk *vk); bool bcm_vk_drv_access_ok(struct bcm_vk *vk); void bcm_vk_set_host_alert(struct bcm_vk *vk, u32 bit_mask); +#ifdef CONFIG_BCM_VK_TTY +int bcm_vk_tty_init(struct bcm_vk *vk, char *name); +void bcm_vk_tty_exit(struct bcm_vk *vk); +void bcm_vk_tty_terminate_tty_user(struct bcm_vk *vk); +void bcm_vk_tty_wq_exit(struct bcm_vk *vk); + +static inline void bcm_vk_tty_set_irq_enabled(struct bcm_vk *vk, int index) +{ + vk->tty[index].irq_enabled = true; +} +#else +static inline int bcm_vk_tty_init(struct bcm_vk *vk, char *name) +{ + return 0; +} + +static inline void bcm_vk_tty_exit(struct bcm_vk *vk) +{ +} + +static inline void bcm_vk_tty_terminate_tty_user(struct bcm_vk *vk) +{ +} + +static inline void bcm_vk_tty_wq_exit(struct bcm_vk *vk) +{ +} + +static inline void bcm_vk_tty_set_irq_enabled(struct bcm_vk *vk, int index) +{ +} +#endif /* CONFIG_BCM_VK_TTY */ + #endif diff --git a/drivers/misc/bcm-vk/bcm_vk_dev.c b/drivers/misc/bcm-vk/bcm_vk_dev.c index c3d2bba68ef1..59b4859d68d2 100644 --- a/drivers/misc/bcm-vk/bcm_vk_dev.c +++ b/drivers/misc/bcm-vk/bcm_vk_dev.c @@ -1396,7 +1396,7 @@ static int bcm_vk_probe(struct pci_dev *pdev, const struct pci_device_id *ent) pdev->irq + vk->num_irqs, vk->num_irqs + 1); goto err_irq; } - vk->tty[i].irq_enabled = true; + bcm_vk_tty_set_irq_enabled(vk, i); } id = ida_simple_get(_vk_ida, 0, 0, GFP_KERNEL); @@ -1580,8 +1580,7 @@ static void bcm_vk_remove(struct pci_dev *pdev) cancel_work_sync(>wq_work); destroy_workqueue(vk->wq_thread); - cancel_work_sync(>tty_wq_work); - destroy_workqueue(vk->tty_wq_thread); + bcm_vk_tty_wq_exit(vk); for (i = 0; i < MAX_BAR; i++) { if (vk->bar[i]) diff --git
Re: [PATCH v18 05/25] x86/fpu/xstate: Introduce CET MSR and XSAVES supervisor states
On 1/29/21 2:35 PM, Yu, Yu-cheng wrote: >> Andy Cooper just mentioned on IRC about this nugget in the spec: >> >> XRSTORS on CET state will do reserved bit and canonicality >> checks on the state in similar manner as done by the WRMSR to >> these state elements. >> >> We're using copy_kernel_to_xregs_err(), so the #GP *should* be OK. >> Could we prove this out in practice, please? >>> > Do we want to verify that setting reserved bits in CET XSAVES states > triggers GP? Then, yes, I just verified it again. Thanks for > reminding. Do we have any particular case relating to this? I want to confirm that it triggers #GP and kills userspace without the kernel WARN'ing or otherwise being visibly unhappy. What about the return-to-userspace path after a ptracer writes content to the CET fields? I don't see the same tolerance for errors in __fpregs_load_activate(), for instance.
[PATCH 02/11] x86/fault: Fold mm_fault_error() into do_user_addr_fault()
mm_fault_error() is logically just the end of do_user_addr_fault(). Combine the functions. This makes the code easier to read. Most of the churn here is from renaming hw_error_code to error_code in do_user_addr_fault(). This makes no difference at all to the generated code (objdump -dr) as compared to changing noinline to __always_inline in the definition of mm_fault_error(). Cc: Dave Hansen Cc: Peter Zijlstra Signed-off-by: Andy Lutomirski --- arch/x86/mm/fault.c | 97 + 1 file changed, 45 insertions(+), 52 deletions(-) diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 50dfdc71761e..aff35c9ba018 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -944,40 +944,6 @@ do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address, force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address); } -static noinline void -mm_fault_error(struct pt_regs *regs, unsigned long error_code, - unsigned long address, vm_fault_t fault) -{ - if (fatal_signal_pending(current) && !(error_code & X86_PF_USER)) { - no_context(regs, error_code, address, 0, 0); - return; - } - - if (fault & VM_FAULT_OOM) { - /* Kernel mode? Handle exceptions or die: */ - if (!(error_code & X86_PF_USER)) { - no_context(regs, error_code, address, - SIGSEGV, SEGV_MAPERR); - return; - } - - /* -* We ran out of memory, call the OOM killer, and return the -* userspace (which will retry the fault, or kill us if we got -* oom-killed): -*/ - pagefault_out_of_memory(); - } else { - if (fault & (VM_FAULT_SIGBUS|VM_FAULT_HWPOISON| -VM_FAULT_HWPOISON_LARGE)) - do_sigbus(regs, error_code, address, fault); - else if (fault & VM_FAULT_SIGSEGV) - bad_area_nosemaphore(regs, error_code, address); - else - BUG(); - } -} - static int spurious_kernel_fault_check(unsigned long error_code, pte_t *pte) { if ((error_code & X86_PF_WRITE) && !pte_write(*pte)) @@ -1215,7 +1181,7 @@ NOKPROBE_SYMBOL(do_kern_addr_fault); /* Handle faults in the user portion of the address space */ static inline void do_user_addr_fault(struct pt_regs *regs, - unsigned long hw_error_code, + unsigned long error_code, unsigned long address) { struct vm_area_struct *vma; @@ -1235,8 +1201,8 @@ void do_user_addr_fault(struct pt_regs *regs, * Reserved bits are never expected to be set on * entries in the user portion of the page tables. */ - if (unlikely(hw_error_code & X86_PF_RSVD)) - pgtable_bad(regs, hw_error_code, address); + if (unlikely(error_code & X86_PF_RSVD)) + pgtable_bad(regs, error_code, address); /* * If SMAP is on, check for invalid kernel (supervisor) access to user @@ -1246,10 +1212,10 @@ void do_user_addr_fault(struct pt_regs *regs, * enforcement appears to be consistent with the USER bit. */ if (unlikely(cpu_feature_enabled(X86_FEATURE_SMAP) && -!(hw_error_code & X86_PF_USER) && +!(error_code & X86_PF_USER) && !(regs->flags & X86_EFLAGS_AC))) { - bad_area_nosemaphore(regs, hw_error_code, address); + bad_area_nosemaphore(regs, error_code, address); return; } @@ -1258,7 +1224,7 @@ void do_user_addr_fault(struct pt_regs *regs, * in a region with pagefaults disabled then we must not take the fault */ if (unlikely(faulthandler_disabled() || !mm)) { - bad_area_nosemaphore(regs, hw_error_code, address); + bad_area_nosemaphore(regs, error_code, address); return; } @@ -1279,9 +1245,9 @@ void do_user_addr_fault(struct pt_regs *regs, perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address); - if (hw_error_code & X86_PF_WRITE) + if (error_code & X86_PF_WRITE) flags |= FAULT_FLAG_WRITE; - if (hw_error_code & X86_PF_INSTR) + if (error_code & X86_PF_INSTR) flags |= FAULT_FLAG_INSTRUCTION; #ifdef CONFIG_X86_64 @@ -1297,7 +1263,7 @@ void do_user_addr_fault(struct pt_regs *regs, * to consider the PF_PK bit. */ if (is_vsyscall_vaddr(address)) { - if (emulate_vsyscall(hw_error_code, regs, address)) + if (emulate_vsyscall(error_code, regs, address)) return; } #endif @@ -1320,7 +1286,7 @@ void do_user_addr_fault(struct pt_regs *regs,
[PATCH 06/11] x86/fault: Improve kernel-executing-user-memory handling
Right now we treat the case of the kernel trying to execute from user memory more or less just like the kernel getting a page fault on a user access. In the failure path, we check for erratum #93, try to otherwise fix up the error, and then oops. If we manage to jump to the user address space, with or without SMEP, we should not try to resolve the page fault. This is an error, pure and simple. Rearrange the code so that we catch this case early, check for erratum #93, and bail out. Cc: Dave Hansen Cc: Peter Zijlstra Signed-off-by: Andy Lutomirski --- arch/x86/mm/fault.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 602cdf8e070a..1939e546beae 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -406,8 +406,11 @@ static void dump_pagetable(unsigned long address) static int is_errata93(struct pt_regs *regs, unsigned long address) { #if defined(CONFIG_X86_64) && defined(CONFIG_CPU_SUP_AMD) - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD - || boot_cpu_data.x86 != 0xf) + if (likely(boot_cpu_data.x86_vendor != X86_VENDOR_AMD + || boot_cpu_data.x86 != 0xf)) + return 0; + + if (user_mode(regs)) return 0; if (address != regs->ip) @@ -707,9 +710,6 @@ no_context(struct pt_regs *regs, unsigned long error_code, if (is_prefetch(regs, error_code, address)) return; - if (is_errata93(regs, address)) - return; - /* * Buggy firmware could access regions which might page fault, try to * recover from such faults. @@ -1202,6 +1202,19 @@ void do_user_addr_fault(struct pt_regs *regs, tsk = current; mm = tsk->mm; + if (unlikely((error_code & (X86_PF_USER | X86_PF_INSTR)) == X86_PF_INSTR)) { + /* +* Whoops, this is kernel mode code trying to execute from +* user memory. Unless this is AMD erratum #93, we are toast. +* Don't even try to look up the VMA. +*/ + if (is_errata93(regs, address)) + return; + + bad_area_nosemaphore(regs, error_code, address); + return; + } + /* kprobes don't want to hook the spurious faults: */ if (unlikely(kprobe_page_fault(regs, X86_TRAP_PF))) return; -- 2.29.2
Re: linux-next: manual merge of the pidfd tree with the overlayfs tree
Hi all, On Mon, 25 Jan 2021 16:23:36 +1100 Stephen Rothwell wrote: > > Today's linux-next merge of the pidfd tree got a conflict in: > > fs/ecryptfs/inode.c > > between commit: > > 176cfe865da6 ("ecryptfs: fix uid translation for setxattr on > security.capability") > > from the overlayfs tree and commit: > > c7c7a1a18af4 ("xattr: handle idmapped mounts") > > from the pidfd tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. > > > diff --cc fs/ecryptfs/inode.c > index 58d0f7187997,55da9a91f51a.. > --- a/fs/ecryptfs/inode.c > +++ b/fs/ecryptfs/inode.c > @@@ -1024,11 -1043,10 +1045,12 @@@ ecryptfs_setxattr(struct dentry *dentry > rc = -EOPNOTSUPP; > goto out; > } > -rc = vfs_setxattr(_user_ns, lower_dentry, name, value, size, > - flags); > +inode_lock(lower_inode); > - rc = __vfs_setxattr_locked(lower_dentry, name, value, size, flags, > NULL); > ++rc = __vfs_setxattr_locked(_user_ns, lower_dentry, name, > ++ value, size, flags, NULL); > +inode_unlock(lower_inode); > if (!rc && inode) > -fsstack_copy_attr_all(inode, d_inode(lower_dentry)); > +fsstack_copy_attr_all(inode, lower_inode); > out: > return rc; > } This is now a conflict between the pidfd tree and Linus' tree. -- Cheers, Stephen Rothwell pgp_t834RGiPV.pgp Description: OpenPGP digital signature
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 2:27 PM Kyle Huey wrote: > > On Sun, Jan 31, 2021 at 2:20 PM Andy Lutomirski wrote: > > > > > > > > > On Jan 31, 2021, at 2:08 PM, Kyle Huey wrote: > > > > > > On Sun, Jan 31, 2021 at 2:04 PM Andy Lutomirski > > > wrote: > > >> Indeed, and I have tests for this. > > > > > > Do you mean you already have a test case or that you would like a > > > minimized test case? > > > > A smallish test that we could stick in selftests would be great if that’s > > straightforward. > > I'll look into it. > > - Kyle A minimal test case follows. The key to triggering this bug is to enter a ptrace syscall stop and then use PTRACE_SINGLESTEP to exit it. On a good kernel this will not result in any userspace code execution in the tracee because on the way out of the kernel's syscall handling path the singlestep trap will be raised immediately. On a bad kernel that stop will not be raised, and in the example below, the program will crash. - Kyle --- #include #include #include #include #include #include void do_child() { /* Synchronize with the parent */ kill(getpid(), SIGSTOP); /* Do a syscall */ printf("child is alive\n"); /* Return and exit */ } int main() { pid_t child = -1; int status = 0; unsigned long long previous_rip = 0; struct user_regs_struct regs; if ((child = fork()) == 0) { do_child(); return 0; } /* Adds 0x80 to syscall stops so we can see them easily */ intptr_t options = PTRACE_O_TRACESYSGOOD; /* Take control of the child (which should be waiting */ assert(ptrace(PTRACE_SEIZE, child, NULL, options) == 0); assert(waitpid(child, , 0) == child); assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP); /* Advance to the syscall stop for the write underlying * the child's printf. */ assert(ptrace(PTRACE_SYSCALL, child, NULL, 0) == 0); assert(waitpid(child, , 0) == child); /* Should be a syscall stop */ assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGTRAP | 0x80); /* Mess with the child's registers, so it will crash if * it executes any code */ assert(ptrace(PTRACE_GETREGS, child, NULL, ) == 0); previous_rip = regs.rip; regs.rip = 0xdeadbeef; assert(ptrace(PTRACE_SETREGS, child, NULL, ) == 0); /* Singlestep. This should trap without executing any code */ assert(ptrace(PTRACE_SINGLESTEP, child, NULL, 0) == 0); assert(waitpid(child, , 0) == child); /* Should be at a singlestep SIGTRAP. In a buggy kernel, * the SIGTRAP is skipped, execution resumes, and we * get a SIGSEGV at the invalid address. */ assert(WIFSTOPPED(status) && WSTOPSIG(status) == SIGTRAP); /* Restore registers */ assert(ptrace(PTRACE_GETREGS, child, NULL, ) == 0); regs.rip = previous_rip; assert(ptrace(PTRACE_SETREGS, child, NULL, ) == 0); /* Continue to the end of the program */ assert(ptrace(PTRACE_CONT, child, NULL, 0) == 0); assert(waitpid(child, , 0) == child); /* Verify the child exited cleanly */ assert(WIFEXITED(status) && WEXITSTATUS(status) == 0); printf("SUCCESS\n"); return 0; }
Re: [GIT PULL] arm64 fixes for 5.11-rc6
On Sun, 31 Jan 2021 at 19:55, Catalin Marinas wrote: > > On Fri, Jan 29, 2021 at 02:09:05PM -0800, Linus Torvalds wrote: > > On Fri, Jan 29, 2021 at 11:03 AM Catalin Marinas > > wrote: > > > > > > arm64 fixes: > > > > > > - Fix the virt_addr_valid() returning true for < PAGE_OFFSET addresses. > > > > That's a really odd fix. > > > > It went from an incorrect bitwise operation (masking) to an _odd_ > > bitwise operation (xor). > > > > Yes, PAGE_OFFSET has the bit pattern of all upper bits set, so "(addr > > ^ PAGE_OFFSET)" by definition reverses the upper bits - and for a > > valid case turns them to zero. > > > > But isn't the *logical* thing to do to use a subtract instead? For the > > valid cases, the two do the same thing (clear the upper bits), but > > just conceptually, isn't the operation that you actually want to do > > "(addr - PAGE_OFFSET)"? > > > > IOW, why is it using that odd xor pattern that doesn't make much > > sense? I believe it _works_, but it looks very strange to me. > > This macro used to test a single bit and it evolved into a bitmask. So, > yes, basically what we need is: > > #define __is_lm_address(addr) ((u64)(addr) >= PAGE_OFFSET && \ > (u64)(addr) < PAGE_END) > > I wasn't sure whether the code generation with two comparisons is > similar to the xor variant but the compiler should probably be smart > enough to use CMP and CCMP. In the grand scheme, it probably doesn't > even matter. > > Unless I miss something, I don't see any overflow issues even if we do > (((u64)addr - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)). > > We can backport the fix already upstream and clean-up the code in > mainline going forward (after some sanity check on the code generation). > It would be easier to parse in the future. > > > Also, shouldn't _lm_to_phys() do the same? It does that "mask upper > > bits" too that was problematic in __is_lm_address(). Again, shouldn't > > that logically be a subtract op? > > Yes, that's similar and a subtract should do. > The original bit test was written like that because it removes the need to reason about a potential tag in the upper bits. I tried to preserve that behavior when removing the guaranteed 1:1 split between the vmalloc and linear regions, by masking with PAGE_OFFSET and comparing with PAGE_END - PAGE_OFFSET, but unfortunately, both approaches suffer from the issue fixed by this patch, i.e., that virt_addr_valid(0x0) erroneously returns true. I think both proposed fixes are appropriate, but they both reintroduce the need to consider the tag. I don't know whether or where this could pose a problem, but it needs to be taken into account.
[PATCH 5/5] entry/kvm: Explicitly flush pending rcuog wakeup before last rescheduling point
Following the idle loop model, cleanly check for pending rcuog wakeup before the last rescheduling point upon resuming to guest mode. This way we can avoid to do it from rcu_user_enter() with the last resort self-IPI hack that enforces rescheduling. Suggested-by: Peter Zijlstra Signed-off-by: Frederic Weisbecker Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Rafael J. Wysocki Cc: Paolo Bonzini --- arch/x86/kvm/x86.c| 1 + include/linux/entry-kvm.h | 14 + kernel/rcu/tree.c | 44 ++- kernel/rcu/tree_plugin.h | 1 + 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 9a8969a6dd06..7fd4f70c229b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1773,6 +1773,7 @@ EXPORT_SYMBOL_GPL(kvm_emulate_wrmsr); bool kvm_vcpu_exit_request(struct kvm_vcpu *vcpu) { + xfer_to_guest_mode_prepare(); return vcpu->mode == EXITING_GUEST_MODE || kvm_request_pending(vcpu) || xfer_to_guest_mode_work_pending(); } diff --git a/include/linux/entry-kvm.h b/include/linux/entry-kvm.h index 9b93f8584ff7..8b2b1d68b954 100644 --- a/include/linux/entry-kvm.h +++ b/include/linux/entry-kvm.h @@ -46,6 +46,20 @@ static inline int arch_xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu, */ int xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu); +/** + * xfer_to_guest_mode_prepare - Perform last minute preparation work that + * need to be handled while IRQs are disabled + * upon entering to guest. + * + * Has to be invoked with interrupts disabled before the last call + * to xfer_to_guest_mode_work_pending(). + */ +static inline void xfer_to_guest_mode_prepare(void) +{ + lockdep_assert_irqs_disabled(); + rcu_nocb_flush_deferred_wakeup(); +} + /** * __xfer_to_guest_mode_work_pending - Check if work is pending * diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 2ebc211fffcb..ce17b8477442 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -678,9 +678,10 @@ EXPORT_SYMBOL_GPL(rcu_idle_enter); #ifdef CONFIG_NO_HZ_FULL +#if !defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_KVM_XFER_TO_GUEST_WORK) /* * An empty function that will trigger a reschedule on - * IRQ tail once IRQs get re-enabled on userspace resume. + * IRQ tail once IRQs get re-enabled on userspace/guest resume. */ static void late_wakeup_func(struct irq_work *work) { @@ -689,6 +690,37 @@ static void late_wakeup_func(struct irq_work *work) static DEFINE_PER_CPU(struct irq_work, late_wakeup_work) = IRQ_WORK_INIT(late_wakeup_func); +/* + * If either: + * + * 1) the task is about to enter in guest mode and $ARCH doesn't support KVM generic work + * 2) the task is about to enter in user mode and $ARCH doesn't support generic entry. + * + * In these cases the late RCU wake ups aren't supported in the resched loops and our + * last resort is to fire a local irq_work that will trigger a reschedule once IRQs + * get re-enabled again. + */ +noinstr static void rcu_irq_work_resched(void) +{ + struct rcu_data *rdp = this_cpu_ptr(_data); + + if (IS_ENABLED(CONFIG_GENERIC_ENTRY) && !(current->flags & PF_VCPU)) + return; + + if (IS_ENABLED(CONFIG_KVM_XFER_TO_GUEST_WORK) && (current->flags & PF_VCPU)) + return; + + instrumentation_begin(); + if (do_nocb_deferred_wakeup(rdp) && need_resched()) { + irq_work_queue(this_cpu_ptr(_wakeup_work)); + } + instrumentation_end(); +} + +#else +static inline void rcu_irq_work_resched(void) { } +#endif + /** * rcu_user_enter - inform RCU that we are resuming userspace. * @@ -702,8 +734,6 @@ static DEFINE_PER_CPU(struct irq_work, late_wakeup_work) = */ noinstr void rcu_user_enter(void) { - struct rcu_data *rdp = this_cpu_ptr(_data); - lockdep_assert_irqs_disabled(); /* @@ -711,13 +741,7 @@ noinstr void rcu_user_enter(void) * rescheduling opportunity in the entry code. Trigger a self IPI * that will fire and reschedule once we resume in user/guest mode. */ - instrumentation_begin(); - if (!IS_ENABLED(CONFIG_GENERIC_ENTRY) || (current->flags & PF_VCPU)) { - if (do_nocb_deferred_wakeup(rdp) && need_resched()) - irq_work_queue(this_cpu_ptr(_wakeup_work)); - } - instrumentation_end(); - + rcu_irq_work_resched(); rcu_eqs_enter(true); } diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index 384856e4d13e..cdc1b7651c03 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -2197,6 +2197,7 @@ void rcu_nocb_flush_deferred_wakeup(void) { do_nocb_deferred_wakeup(this_cpu_ptr(_data)); } +EXPORT_SYMBOL_GPL(rcu_nocb_flush_deferred_wakeup); void __init rcu_init_nohz(void) { -- 2.25.1
[PATCH 4/5] entry: Explicitly flush pending rcuog wakeup before last rescheduling point
Following the idle loop model, cleanly check for pending rcuog wakeup before the last rescheduling point on resuming to user mode. This way we can avoid to do it from rcu_user_enter() with the last resort self-IPI hack that enforces rescheduling. Signed-off-by: Frederic Weisbecker Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Rafael J. Wysocki --- kernel/entry/common.c | 7 +++ kernel/rcu/tree.c | 12 +++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/kernel/entry/common.c b/kernel/entry/common.c index 378341642f94..7c61460a0867 100644 --- a/kernel/entry/common.c +++ b/kernel/entry/common.c @@ -184,6 +184,10 @@ static unsigned long exit_to_user_mode_loop(struct pt_regs *regs, * enabled above. */ local_irq_disable_exit_to_user(); + + /* Check if any of the above work has queued a deferred wakeup */ + rcu_nocb_flush_deferred_wakeup(); + ti_work = READ_ONCE(current_thread_info()->flags); } @@ -197,6 +201,9 @@ static void exit_to_user_mode_prepare(struct pt_regs *regs) lockdep_assert_irqs_disabled(); + /* Flush pending rcuog wakeup before the last need_resched() check */ + rcu_nocb_flush_deferred_wakeup(); + if (unlikely(ti_work & EXIT_TO_USER_MODE_WORK)) ti_work = exit_to_user_mode_loop(regs, ti_work); diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 4b1e5bd16492..2ebc211fffcb 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -707,13 +707,15 @@ noinstr void rcu_user_enter(void) lockdep_assert_irqs_disabled(); /* -* We may be past the last rescheduling opportunity in the entry code. -* Trigger a self IPI that will fire and reschedule once we resume to -* user/guest mode. +* Other than generic entry implementation, we may be past the last +* rescheduling opportunity in the entry code. Trigger a self IPI +* that will fire and reschedule once we resume in user/guest mode. */ instrumentation_begin(); - if (do_nocb_deferred_wakeup(rdp) && need_resched()) - irq_work_queue(this_cpu_ptr(_wakeup_work)); + if (!IS_ENABLED(CONFIG_GENERIC_ENTRY) || (current->flags & PF_VCPU)) { + if (do_nocb_deferred_wakeup(rdp) && need_resched()) + irq_work_queue(this_cpu_ptr(_wakeup_work)); + } instrumentation_end(); rcu_eqs_enter(true); -- 2.25.1
Re: [PATCH] drm/msm/mdp5: Fix wait-for-commit for cmd panels
Hi Iskren, On Mittwoch, 27. Jänner 2021 16:24:40 CET Iskren Chernev wrote: > Before the offending commit in msm_atomic_commit_tail wait_flush was > called once per frame, after the commit was submitted. After it > wait_flush is also called at the beginning to ensure previous > potentially async commits are done. > > For cmd panels the source of wait_flush is a ping-pong irq notifying > a completion. The completion needs to be notified with complete_all so > multiple waiting parties (new async committers) can proceed. > > Signed-off-by: Iskren Chernev > Suggested-by: Rob Clark > Fixes: 2d99ced787e3d ("drm/msm: async commit support") > --- I've tested this now on fairphone-fp2 and lge-nexus5-hammerhead, works great! Tested-by: Luca Weiss Regards Luca
[PATCH 3/5] rcu/nocb: Trigger self-IPI on late deferred wake up before user resume
Entering RCU idle mode may cause a deferred wake up of an RCU NOCB_GP kthread (rcuog) to be serviced. Unfortunately the call to rcu_user_enter() is already past the last rescheduling opportunity before we resume to userspace or to guest mode. We may escape there with the woken task ignored. The ultimate resort to fix every callsites is to trigger a self-IPI (nohz_full depends on arch to implement arch_irq_work_raise()) that will trigger a reschedule on IRQ tail or guest exit. Eventually every site that want a saner treatment will need to carefully place a call to rcu_nocb_flush_deferred_wakeup() before the last explicit need_resched() check upon resume. Reported-by: Paul E. McKenney Fixes: 96d3fd0d315a (rcu: Break call_rcu() deadlock involving scheduler and perf) Cc: sta...@vger.kernel.org Cc: Rafael J. Wysocki Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Signed-off-by: Frederic Weisbecker --- kernel/rcu/tree.c| 21 - kernel/rcu/tree.h| 2 +- kernel/rcu/tree_plugin.h | 25 - 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 82838e93b498..4b1e5bd16492 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -677,6 +677,18 @@ void rcu_idle_enter(void) EXPORT_SYMBOL_GPL(rcu_idle_enter); #ifdef CONFIG_NO_HZ_FULL + +/* + * An empty function that will trigger a reschedule on + * IRQ tail once IRQs get re-enabled on userspace resume. + */ +static void late_wakeup_func(struct irq_work *work) +{ +} + +static DEFINE_PER_CPU(struct irq_work, late_wakeup_work) = + IRQ_WORK_INIT(late_wakeup_func); + /** * rcu_user_enter - inform RCU that we are resuming userspace. * @@ -694,12 +706,19 @@ noinstr void rcu_user_enter(void) lockdep_assert_irqs_disabled(); + /* +* We may be past the last rescheduling opportunity in the entry code. +* Trigger a self IPI that will fire and reschedule once we resume to +* user/guest mode. +*/ instrumentation_begin(); - do_nocb_deferred_wakeup(rdp); + if (do_nocb_deferred_wakeup(rdp) && need_resched()) + irq_work_queue(this_cpu_ptr(_wakeup_work)); instrumentation_end(); rcu_eqs_enter(true); } + #endif /* CONFIG_NO_HZ_FULL */ /** diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h index 7708ed161f4a..9226f4021a36 100644 --- a/kernel/rcu/tree.h +++ b/kernel/rcu/tree.h @@ -433,7 +433,7 @@ static bool rcu_nocb_try_bypass(struct rcu_data *rdp, struct rcu_head *rhp, static void __call_rcu_nocb_wake(struct rcu_data *rdp, bool was_empty, unsigned long flags); static int rcu_nocb_need_deferred_wakeup(struct rcu_data *rdp); -static void do_nocb_deferred_wakeup(struct rcu_data *rdp); +static bool do_nocb_deferred_wakeup(struct rcu_data *rdp); static void rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp); static void rcu_spawn_cpu_nocb_kthread(int cpu); static void __init rcu_spawn_nocb_kthreads(void); diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index d5b38c28abd1..384856e4d13e 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -1631,8 +1631,8 @@ bool rcu_is_nocb_cpu(int cpu) * Kick the GP kthread for this NOCB group. Caller holds ->nocb_lock * and this function releases it. */ -static void wake_nocb_gp(struct rcu_data *rdp, bool force, - unsigned long flags) +static bool wake_nocb_gp(struct rcu_data *rdp, bool force, +unsigned long flags) __releases(rdp->nocb_lock) { bool needwake = false; @@ -1643,7 +1643,7 @@ static void wake_nocb_gp(struct rcu_data *rdp, bool force, trace_rcu_nocb_wake(rcu_state.name, rdp->cpu, TPS("AlreadyAwake")); rcu_nocb_unlock_irqrestore(rdp, flags); - return; + return false; } del_timer(>nocb_timer); rcu_nocb_unlock_irqrestore(rdp, flags); @@ -1656,6 +1656,8 @@ static void wake_nocb_gp(struct rcu_data *rdp, bool force, raw_spin_unlock_irqrestore(_gp->nocb_gp_lock, flags); if (needwake) wake_up_process(rdp_gp->nocb_gp_kthread); + + return needwake; } /* @@ -2152,20 +2154,23 @@ static int rcu_nocb_need_deferred_wakeup(struct rcu_data *rdp) } /* Do a deferred wakeup of rcu_nocb_kthread(). */ -static void do_nocb_deferred_wakeup_common(struct rcu_data *rdp) +static bool do_nocb_deferred_wakeup_common(struct rcu_data *rdp) { unsigned long flags; int ndw; + int ret; rcu_nocb_lock_irqsave(rdp, flags); if (!rcu_nocb_need_deferred_wakeup(rdp)) { rcu_nocb_unlock_irqrestore(rdp, flags); - return; + return false; } ndw = READ_ONCE(rdp->nocb_defer_wakeup); WRITE_ONCE(rdp->nocb_defer_wakeup,
[PATCH 1/5] rcu: Pull deferred rcuog wake up to rcu_eqs_enter() callers
Deferred wakeup of rcuog kthreads upon RCU idle mode entry is going to be handled differently whether initiated by idle, user or guest. Prepare with pulling that control up to rcu_eqs_enter() callers. Signed-off-by: Frederic Weisbecker Cc: Paul E. McKenney Cc: Rafael J. Wysocki Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar --- kernel/rcu/tree.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 40e5e3dd253e..63032e5620b9 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -644,7 +644,6 @@ static noinstr void rcu_eqs_enter(bool user) trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, atomic_read(>dynticks)); WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current)); rdp = this_cpu_ptr(_data); - do_nocb_deferred_wakeup(rdp); rcu_prepare_for_idle(); rcu_preempt_deferred_qs(current); @@ -672,7 +671,10 @@ static noinstr void rcu_eqs_enter(bool user) */ void rcu_idle_enter(void) { + struct rcu_data *rdp = this_cpu_ptr(_data); + lockdep_assert_irqs_disabled(); + do_nocb_deferred_wakeup(rdp); rcu_eqs_enter(false); } EXPORT_SYMBOL_GPL(rcu_idle_enter); @@ -691,7 +693,14 @@ EXPORT_SYMBOL_GPL(rcu_idle_enter); */ noinstr void rcu_user_enter(void) { + struct rcu_data *rdp = this_cpu_ptr(_data); + lockdep_assert_irqs_disabled(); + + instrumentation_begin(); + do_nocb_deferred_wakeup(rdp); + instrumentation_end(); + rcu_eqs_enter(true); } #endif /* CONFIG_NO_HZ_FULL */ -- 2.25.1
[PATCH 0/5] rcu/sched: Fix ignored rescheduling after rcu_eqs_enter() v4
So, here is a hopefully improved version with the following changes: * No more late wake up debugging, objtool should debug that later with noinstr code calling into the scheduler (Peter suggestion) * Dropped the double rdp fetch patch, just keep the fix part for now * Properly protect irq work call from rcu_user_enter() inside instrumention_begin() * Handle CONFIG_KVM_XFER_TO_GUEST_WORK (as per Peter suggestion) git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git sched/idle-v4 HEAD: d3e956d0b693a572bd5f56241816a6390c5b2797 Thanks, Frederic --- Frederic Weisbecker (5): rcu: Pull deferred rcuog wake up to rcu_eqs_enter() callers rcu/nocb: Perform deferred wake up before last idle's need_resched() check rcu/nocb: Trigger self-IPI on late deferred wake up before user resume entry: Explicitly flush pending rcuog wakeup before last rescheduling point entry/kvm: Explicitly flush pending rcuog wakeup before last rescheduling point arch/x86/kvm/x86.c| 1 + include/linux/entry-kvm.h | 14 + include/linux/rcupdate.h | 2 ++ kernel/entry/common.c | 7 +++ kernel/rcu/tree.c | 53 ++- kernel/rcu/tree.h | 2 +- kernel/rcu/tree_plugin.h | 31 +++ kernel/sched/idle.c | 3 +++ 8 files changed, 102 insertions(+), 11 deletions(-)
[PATCH 2/5] rcu/nocb: Perform deferred wake up before last idle's need_resched() check
Entering RCU idle mode may cause a deferred wake up of an RCU NOCB_GP kthread (rcuog) to be serviced. Usually a local wake up happening while running the idle task is handled in one of the need_resched() checks carefully placed within the idle loop that can break to the scheduler. Unfortunately the call to rcu_idle_enter() is already beyond the last generic need_resched() check and we may halt the CPU with a resched request unhandled, leaving the task hanging. Fix this with splitting the rcuog wakeup handling from rcu_idle_enter() and place it before the last generic need_resched() check in the idle loop. It is then assumed that no call to call_rcu() will be performed after that in the idle loop until the CPU is put in low power mode. Reported-by: Paul E. McKenney Fixes: 96d3fd0d315a (rcu: Break call_rcu() deadlock involving scheduler and perf) Cc: sta...@vger.kernel.org Cc: Rafael J. Wysocki Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Ingo Molnar Signed-off-by: Frederic Weisbecker --- include/linux/rcupdate.h | 2 ++ kernel/rcu/tree.c| 3 --- kernel/rcu/tree_plugin.h | 5 + kernel/sched/idle.c | 3 +++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index fd02c5fa60cb..36c2119de702 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -110,8 +110,10 @@ static inline void rcu_user_exit(void) { } #ifdef CONFIG_RCU_NOCB_CPU void rcu_init_nohz(void); +void rcu_nocb_flush_deferred_wakeup(void); #else /* #ifdef CONFIG_RCU_NOCB_CPU */ static inline void rcu_init_nohz(void) { } +static inline void rcu_nocb_flush_deferred_wakeup(void) { } #endif /* #else #ifdef CONFIG_RCU_NOCB_CPU */ /** diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 63032e5620b9..82838e93b498 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -671,10 +671,7 @@ static noinstr void rcu_eqs_enter(bool user) */ void rcu_idle_enter(void) { - struct rcu_data *rdp = this_cpu_ptr(_data); - lockdep_assert_irqs_disabled(); - do_nocb_deferred_wakeup(rdp); rcu_eqs_enter(false); } EXPORT_SYMBOL_GPL(rcu_idle_enter); diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index 7e291ce0a1d6..d5b38c28abd1 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -2187,6 +2187,11 @@ static void do_nocb_deferred_wakeup(struct rcu_data *rdp) do_nocb_deferred_wakeup_common(rdp); } +void rcu_nocb_flush_deferred_wakeup(void) +{ + do_nocb_deferred_wakeup(this_cpu_ptr(_data)); +} + void __init rcu_init_nohz(void) { int cpu; diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index 305727ea0677..b601a3aa2152 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -55,6 +55,7 @@ __setup("hlt", cpu_idle_nopoll_setup); static noinline int __cpuidle cpu_idle_poll(void) { trace_cpu_idle(0, smp_processor_id()); + rcu_nocb_flush_deferred_wakeup(); stop_critical_timings(); rcu_idle_enter(); local_irq_enable(); @@ -173,6 +174,8 @@ static void cpuidle_idle_call(void) struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); int next_state, entered_state; + rcu_nocb_flush_deferred_wakeup(); + /* * Check if the idle task must be rescheduled. If it is the * case, exit the function after re-enabling the local irq. -- 2.25.1
Re: [PATCH 1/3] perf tools: Use /proc//task//status for synthesis
On Fri, Jan 29, 2021 at 02:48:59PM +0900, Namhyung Kim wrote: > To save memory usage, it needs to reduce number of entries in the proc > filesystem. It's using /proc//task directory to traverse threads > in the process and then kernel creates /proc//task/ entries. > > After that it checks the thread info using the /proc//status file > rather than /proc//task//status. As far as I can see, they > are the same and contain all the info we need. > > Using the latter eliminates the unnecessary /proc/ entry. This > can be useful especially a large number of threads are used in the > system. In my experiment around 1KB of memory on average was saved > for each thread (which is not a thread group leader). > > To do this, pass both pid and tid to perf_event_prepare_comm() if it > knows them. In case it doesn't know, passing 0 as pid will do the old > way. > > Signed-off-by: Namhyung Kim > --- > tools/perf/util/synthetic-events.c | 30 +++--- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/tools/perf/util/synthetic-events.c > b/tools/perf/util/synthetic-events.c > index 3a898520f05c..800522591dde 100644 > --- a/tools/perf/util/synthetic-events.c > +++ b/tools/perf/util/synthetic-events.c > @@ -69,7 +69,7 @@ int perf_tool__process_synth_event(struct perf_tool *tool, > * Assumes that the first 4095 bytes of /proc/pid/stat contains > * the comm, tgid and ppid. > */ > -static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len, > +static int perf_event__get_comm_ids(pid_t pid, pid_t tid, char *comm, size_t > len, > pid_t *tgid, pid_t *ppid) > { > char bf[4096]; > @@ -81,7 +81,10 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, > size_t len, > *tgid = -1; > *ppid = -1; > > - snprintf(bf, sizeof(bf), "/proc/%d/status", pid); > + if (pid) > + snprintf(bf, sizeof(bf), "/proc/%d/task/%d/status", pid, tid); > + else > + snprintf(bf, sizeof(bf), "/proc/%d/status", tid); > > fd = open(bf, O_RDONLY); > if (fd < 0) { > @@ -93,7 +96,7 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, > size_t len, > close(fd); > if (n <= 0) { > pr_warning("Couldn't get COMM, tigd and ppid for pid %d\n", > -pid); > +tid); > return -1; > } > bf[n] = '\0'; > @@ -116,27 +119,32 @@ static int perf_event__get_comm_ids(pid_t pid, char > *comm, size_t len, > memcpy(comm, name, size); > comm[size] = '\0'; > } else { > - pr_debug("Name: string not found for pid %d\n", pid); > + pr_debug("Name: string not found for pid %d\n", tid); > } > > if (tgids) { > tgids += 5; /* strlen("Tgid:") */ > *tgid = atoi(tgids); > + > + if (pid && pid != *tgid) { > + pr_debug("Tgid: not match to given pid: %d vs %d\n", > + pid, *tgid); hm, could this actually happen in our case? jirka
arch/arm/mach-s3c/irq-s3c24xx.c:1034:13: warning: no previous prototype for function 's3c2442_init_irq'
Hi Arnd, First bad commit (maybe != root cause): tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: ac8c6edd20bcb965b22ceb36752499b3d5cf5dd4 commit: 71b9114d2c13a648fbe6523dd859e611c316ad90 ARM: s3c: move into a common directory date: 5 months ago config: arm-randconfig-r004-20210201 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 275c6af7d7f1ed63a03d05b4484413e447133269) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=71b9114d2c13a648fbe6523dd859e611c316ad90 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout 71b9114d2c13a648fbe6523dd859e611c316ad90 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): arch/arm/mach-s3c/irq-s3c24xx.c:360:39: warning: no previous prototype for function 's3c24xx_handle_irq' [-Wmissing-prototypes] asmlinkage void __exception_irq_entry s3c24xx_handle_irq(struct pt_regs *regs) ^ arch/arm/mach-s3c/irq-s3c24xx.c:360:12: note: declare 'static' if the function is not intended to be used outside of this translation unit asmlinkage void __exception_irq_entry s3c24xx_handle_irq(struct pt_regs *regs) ^ static arch/arm/mach-s3c/irq-s3c24xx.c:389:5: warning: no previous prototype for function 's3c24xx_set_fiq' [-Wmissing-prototypes] int s3c24xx_set_fiq(unsigned int irq, u32 *ack_ptr, bool on) ^ arch/arm/mach-s3c/irq-s3c24xx.c:389:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int s3c24xx_set_fiq(unsigned int irq, u32 *ack_ptr, bool on) ^ static arch/arm/mach-s3c/irq-s3c24xx.c:683:13: warning: no previous prototype for function 's3c2410_init_irq' [-Wmissing-prototypes] void __init s3c2410_init_irq(void) ^ arch/arm/mach-s3c/irq-s3c24xx.c:683:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void __init s3c2410_init_irq(void) ^ static arch/arm/mach-s3c/irq-s3c24xx.c:882:13: warning: no previous prototype for function 's3c2416_init_irq' [-Wmissing-prototypes] void __init s3c2416_init_irq(void) ^ arch/arm/mach-s3c/irq-s3c24xx.c:882:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void __init s3c2416_init_irq(void) ^ static arch/arm/mach-s3c/irq-s3c24xx.c:961:13: warning: no previous prototype for function 's3c2440_init_irq' [-Wmissing-prototypes] void __init s3c2440_init_irq(void) ^ arch/arm/mach-s3c/irq-s3c24xx.c:961:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void __init s3c2440_init_irq(void) ^ static >> arch/arm/mach-s3c/irq-s3c24xx.c:1034:13: warning: no previous prototype for >> function 's3c2442_init_irq' [-Wmissing-prototypes] void __init s3c2442_init_irq(void) ^ arch/arm/mach-s3c/irq-s3c24xx.c:1034:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void __init s3c2442_init_irq(void) ^ static arch/arm/mach-s3c/irq-s3c24xx.c:1308:12: warning: no previous prototype for function 's3c2410_init_intc_of' [-Wmissing-prototypes] int __init s3c2410_init_intc_of(struct device_node *np, ^ arch/arm/mach-s3c/irq-s3c24xx.c:1308:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int __init s3c2410_init_intc_of(struct device_node *np, ^ static arch/arm/mach-s3c/irq-s3c24xx.c:1330:12: warning: no previous prototype for function 's3c2416_init_intc_of' [-Wmissing-prototypes] int __init s3c2416_init_intc_of(struct device_node *np, ^ arch/arm/mach-s3c/irq-s3c24xx.c:1330:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int __init s3c2416_init_intc_of(struct device_node *np, ^ static 8 warnings generated. vim +/s3c2442_init_irq +1034 arch/arm/mach-s3c/irq-s3c24xx.c 6f8d7ea275eb2a arch/arm/mach-s3c24xx/irq.c Heiko Stuebner 2013-02-12 1033 70644ade48ae88 arch/arm/mach-s3c24xx/irq.c Heiko Stuebner 2013-02-12 @1034 void __init s3c2442_init_irq(void) 70644ade48ae88
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 2:20 PM Andy Lutomirski wrote: > > A smallish test that we could stick in selftests would be great if that’s > straightforward. Side note: it would be good to have a test-case for the interaction with the "block step" code too. I hate our name for it ("block step"?), but it modifies TF, to only trap on taken branches, not after each instruction. So there's all these things that interact: - you can set TF yourself in user space with 'popf' and get a debug signal (not after the popf, but after the instruction _after_ popf, iirc) - you can set TF as a debugger and that's basically what TIF_SINGLESTEP fundamentally means - we have TIF_FORCED_TF, which says whether TF was set by ptrace, or whether it was already set independently by the application before ptrace, so that we can know whether to clear it or keep it set *after* the single-step. - you can then *modify* the behavior of TF to trap only on control flow changes (and we use TIF_BLOCKSTEP to specify that behavior) - and there's also obviously some very subtle and unclear rules about when system call instructions cause debug traps The basic TF behavior is fairly simple: it caused #DB, and we send a signal. The "app set TF _itself_, and we want to debug across that event" is a lot more interesting, but it's unclear whether anybody does it. It's really just a "we want to be able to debug that case too", and TIF_FORCED_TF means that it should be possible. I didn't test that it works, though. Sounds worth a test-case? The TIF_BLOCKSTEP thing changes no other logic, but basically sets the bit in the MSR that modifies just when TF traps. I may hate the name, but I think it works. The odd system call tracing part I have no idea who depends on it (apparently "rr", which I assume is some replay thing), and I suspect our semantics for it has been basically random historical one, and it's apparently what changed. That's the one that we _really_ should have a test-case for, along with some documentation and code comment what the actual semantics need to be so that we don't break it again. Linus
Re: [PATCH] drm/msm/mdp5: Fix wait-for-commit for cmd panels
On Wed, Jan 27, 2021 at 05:24:40PM +0200, Iskren Chernev wrote: > Before the offending commit in msm_atomic_commit_tail wait_flush was > called once per frame, after the commit was submitted. After it > wait_flush is also called at the beginning to ensure previous > potentially async commits are done. > > For cmd panels the source of wait_flush is a ping-pong irq notifying > a completion. The completion needs to be notified with complete_all so > multiple waiting parties (new async committers) can proceed. > > Signed-off-by: Iskren Chernev > Suggested-by: Rob Clark > Fixes: 2d99ced787e3d ("drm/msm: async commit support") Nice job tracking down this fix! Reviewed-by: Brian Masney Tested-by: Brian Masney
Re: [PATCH 2/3] perf tools: Skip MMAP record synthesis for kernel threads
On Fri, Jan 29, 2021 at 02:49:00PM +0900, Namhyung Kim wrote: > To synthesize information to resolve sample IPs, it needs to scan task > and mmap info from the /proc filesystem. For each process, it > opens (and reads) status and maps file respectively. But as kernel > threads don't have memory maps so we can skip the maps file. > > To find kernel threads, check "VmPeak:" line in /proc//status > file. It's about the peak virtual memory usage so only user-level > tasks have that. Also check "Threads:" line (which follows the VmPeak > line whether or not it exists) to be sure it's read enough data - just > in case of deeply nested pid namespaces or large number of > supplementary groups are involved. I don't understand this Threads: check, could you please show example where it's useful for the check? thanks, jirka > > This is for user process: > > $ head -40 /proc/1/status > Name: systemd > Umask: > State: S (sleeping) > Tgid: 1 > Ngid: 0 > Pid:1 > PPid: 0 > TracerPid: 0 > Uid:0 0 0 0 > Gid:0 0 0 0 > FDSize: 256 > Groups: > NStgid: 1 > NSpid: 1 > NSpgid: 1 > NSsid: 1 > VmPeak: 234192 kB <-- here > VmSize: 169964 kB > VmLck: 0 kB > VmPin: 0 kB > VmHWM: 29528 kB > VmRSS: 6104 kB > RssAnon:2756 kB > RssFile:3348 kB > RssShmem: 0 kB > VmData:19776 kB > VmStk: 1036 kB > VmExe: 784 kB > VmLib: 9532 kB > VmPTE: 116 kB > VmSwap: 2400 kB > HugetlbPages: 0 kB > CoreDumping:0 > THP_enabled:1 > Threads:1 <-- and here > SigQ: 1/62808 > SigPnd: > ShdPnd: > SigBlk: 7be3c0fe28014a03 > SigIgn: 1000 > > And this is for kernel thread: > > $ head -20 /proc/2/status > Name: kthreadd > Umask: > State: S (sleeping) > Tgid: 2 > Ngid: 0 > Pid:2 > PPid: 0 > TracerPid: 0 > Uid:0 0 0 0 > Gid:0 0 0 0 > FDSize: 64 > Groups: > NStgid: 2 > NSpid: 2 > NSpgid: 0 > NSsid: 0 > Threads:1 <-- here > SigQ: 1/62808 > SigPnd: > ShdPnd: > > Signed-off-by: Namhyung Kim > --- > tools/perf/util/synthetic-events.c | 32 +- > 1 file changed, 23 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/util/synthetic-events.c > b/tools/perf/util/synthetic-events.c > index 800522591dde..8b38228c83d8 100644 > --- a/tools/perf/util/synthetic-events.c > +++ b/tools/perf/util/synthetic-events.c > @@ -70,13 +70,13 @@ int perf_tool__process_synth_event(struct perf_tool *tool, > * the comm, tgid and ppid. > */ > static int perf_event__get_comm_ids(pid_t pid, pid_t tid, char *comm, size_t > len, > - pid_t *tgid, pid_t *ppid) > + pid_t *tgid, pid_t *ppid, bool *kernel) > { > char bf[4096]; > int fd; > size_t size = 0; > ssize_t n; > - char *name, *tgids, *ppids; > + char *name, *tgids, *ppids, *vmpeak, *threads; > > *tgid = -1; > *ppid = -1; > @@ -102,8 +102,14 @@ static int perf_event__get_comm_ids(pid_t pid, pid_t > tid, char *comm, size_t len > bf[n] = '\0'; > > name = strstr(bf, "Name:"); > - tgids = strstr(bf, "Tgid:"); > - ppids = strstr(bf, "PPid:"); > + tgids = strstr(name ?: bf, "Tgid:"); > + ppids = strstr(tgids ?: bf, "PPid:"); > + vmpeak = strstr(ppids ?: bf, "VmPeak:"); > + > + if (vmpeak) > + threads = NULL; > + else > + threads = strstr(ppids ?: bf, "Threads:"); > > if (name) { > char *nl; > @@ -141,12 +147,17 @@ static int perf_event__get_comm_ids(pid_t pid, pid_t > tid, char *comm, size_t len > pr_debug("PPid: string not found for pid %d\n", tid); > } > > + if (!vmpeak && threads) > + *kernel = true; > + else > + *kernel = false; > + > return 0; > } > > static int perf_event__prepare_comm(union perf_event *event, pid_t pid, > pid_t tid, > struct machine *machine, > - pid_t *tgid, pid_t *ppid) > + pid_t *tgid, pid_t *ppid, bool *kernel) > { > size_t size; > > @@ -157,7 +168,7 @@ static int perf_event__prepare_comm(union perf_event > *event, pid_t pid, pid_t ti > if (machine__is_host(machine)) { > if (perf_event__get_comm_ids(pid, tid, event->comm.comm, >sizeof(event->comm.comm),
Re: [PATCH v2 0/3] perf tools: Minor improvements in event synthesis
On Fri, Jan 29, 2021 at 02:48:58PM +0900, Namhyung Kim wrote: > Hello, > > This is to optimize the event synthesis during perf record. > > The first patch is to reduce memory usage when many threads are used. > The second is to avoid unncessary syscalls for kernel threads. And > the last one is to reduce the number of threads to iterate when new > threads are being created at the same time. > > Unfortunately there's no dramatic improvement here but I can see ~5% > gain in the 'perf bench internals synthesize' on a big machine. > (The numbers are not stable though) > > > Before: > # perf bench internals synthesize --mt -M1 -I 100 > # Running 'internals/synthesize' benchmark: > Computing performance of multi threaded perf event synthesis by > synthesizing events on CPU 0: > Number of synthesis threads: 1 > Average synthesis took: 68831.480 usec (+- 101.450 usec) > Average num. events: 9982.000 (+- 0.000) > Average time per event 6.896 usec > > > After: > # perf bench internals synthesize --mt -M1 -I 100 > # Running 'internals/synthesize' benchmark: > Computing performance of multi threaded perf event synthesis by > synthesizing events on CPU 0: > Number of synthesis threads: 1 > Average synthesis took: 65036.370 usec (+- 158.121 usec) > Average num. events: 9982.000 (+- 0.000) > Average time per event 6.515 usec > > > Thanks, > Namhyung > > > Namhyung Kim (3): > perf tools: Use /proc//task//status for synthesis > perf tools: Skip MMAP record synthesis for kernel threads > perf tools: Use scandir() to iterate threads heya, is there any change to previous version? jirka > > tools/perf/util/synthetic-events.c | 88 -- > 1 file changed, 58 insertions(+), 30 deletions(-) > > -- > 2.30.0.365.g02bc693789-goog >
Re: [PATCH v9] perf stat: Fix wrong skipping for per-die aggregation
On Thu, Jan 28, 2021 at 09:34:17AM +0800, Jin Yao wrote: > Uncore becomes die-scope on Xeon Cascade Lake-AP and perf has supported > --per-die aggregation yet. > > One issue is found in check_per_pkg() for uncore events running on > AP system. On cascade Lake-AP, we have: > > S0-D0 > S0-D1 > S1-D0 > S1-D1 > > But in check_per_pkg(), S0-D1 and S1-D1 are skipped because the > mask bits for S0 and S1 have been set for S0-D0 and S1-D0. It doesn't > check die_id. So the counting for S0-D1 and S1-D1 are set to zero. > That's not correct. > > root@lkp-csl-2ap4 ~# ./perf stat -a -I 1000 -e llc_misses.mem_read --per-die > -- sleep 5 > 1.001460963 S0-D0 11317376 Bytes > llc_misses.mem_read > 1.001460963 S0-D1 1 998016 Bytes > llc_misses.mem_read > 1.001460963 S1-D0 1 970496 Bytes > llc_misses.mem_read > 1.001460963 S1-D1 11291264 Bytes > llc_misses.mem_read > 2.003488021 S0-D0 11082048 Bytes > llc_misses.mem_read > 2.003488021 S0-D1 11919040 Bytes > llc_misses.mem_read > 2.003488021 S1-D0 1 890752 Bytes > llc_misses.mem_read > 2.003488021 S1-D1 12380800 Bytes > llc_misses.mem_read > 3.005613270 S0-D0 11126080 Bytes > llc_misses.mem_read > 3.005613270 S0-D1 12898176 Bytes > llc_misses.mem_read > 3.005613270 S1-D0 1 870912 Bytes > llc_misses.mem_read > 3.005613270 S1-D1 13388608 Bytes > llc_misses.mem_read > 4.007627598 S0-D0 11124608 Bytes > llc_misses.mem_read > 4.007627598 S0-D1 13884416 Bytes > llc_misses.mem_read > 4.007627598 S1-D0 1 921088 Bytes > llc_misses.mem_read > 4.007627598 S1-D1 14451840 Bytes > llc_misses.mem_read > 5.001479927 S0-D0 1 963328 Bytes > llc_misses.mem_read > 5.001479927 S0-D1 14831936 Bytes > llc_misses.mem_read > 5.001479927 S1-D0 1 895104 Bytes > llc_misses.mem_read > 5.001479927 S1-D1 15496640 Bytes > llc_misses.mem_read > > From above output, we can see S0-D1 and S1-D1 don't report the interval > values, they are continued to grow. That's because check_per_pkg() wrongly > decides to use zero counts for S0-D1 and S1-D1. > > So in check_per_pkg(), we should use hashmap(socket,die) to decide if > the cpu counts needs to skip. Only considering socket is not enough. > > Now with this patch, > > root@lkp-csl-2ap4 ~# ./perf stat -a -I 1000 -e llc_misses.mem_read --per-die > -- sleep 5 > 1.001586691 S0-D0 11229440 Bytes > llc_misses.mem_read > 1.001586691 S0-D1 1 976832 Bytes > llc_misses.mem_read > 1.001586691 S1-D0 1 938304 Bytes > llc_misses.mem_read > 1.001586691 S1-D1 11227328 Bytes > llc_misses.mem_read > 2.003776312 S0-D0 11586752 Bytes > llc_misses.mem_read > 2.003776312 S0-D1 1 875392 Bytes > llc_misses.mem_read > 2.003776312 S1-D0 1 855616 Bytes > llc_misses.mem_read > 2.003776312 S1-D1 1 949376 Bytes > llc_misses.mem_read > 3.006512788 S0-D0 11338880 Bytes > llc_misses.mem_read > 3.006512788 S0-D1 1 920064 Bytes > llc_misses.mem_read > 3.006512788 S1-D0 1 877184 Bytes > llc_misses.mem_read > 3.006512788 S1-D1 11020736 Bytes > llc_misses.mem_read > 4.008895291 S0-D0 1 926592 Bytes > llc_misses.mem_read > 4.008895291 S0-D1 1 906368 Bytes > llc_misses.mem_read > 4.008895291 S1-D0 1 892224 Bytes > llc_misses.mem_read > 4.008895291 S1-D1 1 987712 Bytes > llc_misses.mem_read > 5.001590993 S0-D0 1 962624 Bytes > llc_misses.mem_read > 5.001590993 S0-D1 1 912512 Bytes > llc_misses.mem_read > 5.001590993 S1-D0 1 891200 Bytes > llc_misses.mem_read > 5.001590993 S1-D1 1 978432 Bytes > llc_misses.mem_read > > On no-die system, die_id is 0, actually it's hashmap(socket,0), original > behavior > is not changed. > > Reported-by: Huang Ying > Signed-off-by: Jin Yao > --- > v9: > Rename zero_per_pkg to evsel__zero_per_pkg and move it to evsel.c. Then > evsel__zero_per_pkg can be called under different code path. > > Call evsel__zero_per_pkg in evsel__exit(). Acked-by: Jiri Olsa thanks, jirka
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 2:20 PM Andy Lutomirski wrote: > > > > > On Jan 31, 2021, at 2:08 PM, Kyle Huey wrote: > > > > On Sun, Jan 31, 2021 at 2:04 PM Andy Lutomirski > > wrote: > >> Indeed, and I have tests for this. > > > > Do you mean you already have a test case or that you would like a > > minimized test case? > > A smallish test that we could stick in selftests would be great if that’s > straightforward. I'll look into it. - Kyle
Re: [PATCH net] net: dsa: mv88e6xxx: override existent unicast portvec in port_fdb_add
On Sun, Jan 31, 2021 at 09:13:15AM +0800, DENG Qingfang wrote: > This bug is exposed when I try your patch series on kernel 5.4 > https://lore.kernel.org/netdev/20210106095136.224739-1-olte...@gmail.com/ > https://lore.kernel.org/netdev/20210116012515.3152-1-tob...@waldekranz.com/ > > Without this patch, DSA will add a new port bit to the existing > portvec when a client moves to the software part of a bridge. When it > moves away, DSA will clear the port bit but the existing one will > remain static. This results in connection issues when the client moves > to a different port of the switch, and the kernel log below. > > mv88e6085 f1072004.mdio-mii:00: ATU member violation for > xx:xx:xx:xx:xx:xx portvec dc00 spid 0 Ah, ok, DSA adds an FDB entry behind the user's back and it relies upon the driver behavior being 'override'. A bit subtle, though it gives one good reason against someone suggesting "why don't you just refuse adding the new entry instead of overriding, like the software bridge does". Probably the refusal of overwriting an entry is what needs to be handled at upper layers, we do need to be able to override from DSA. I had a quick look through our other drivers and it seems that all of them are happy to override an existing FDB entry (or at least the software part is).
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
> On Jan 31, 2021, at 2:08 PM, Kyle Huey wrote: > > On Sun, Jan 31, 2021 at 2:04 PM Andy Lutomirski wrote: >> Indeed, and I have tests for this. > > Do you mean you already have a test case or that you would like a > minimized test case? A smallish test that we could stick in selftests would be great if that’s straightforward. > > - Kyle
[PATCH 05/11] x86/fault: Correct a few user vs kernel checks wrt WRUSS
In general, page fault errors for WRUSS should be just like get_user(), etc. Fix three bugs in this area: We have a comment that says that, if we can't handle a page fault on a user address due to OOM, we will skip the OOM-kill-and-retry logic. The code checked kernel *privilege*, not kernel mode, so it missed WRUSS. This means that we would malfunction if we got OOM on a WRUSS fault -- this would be a kernel-mode, user-privilege fault, and we would invoke the OOM killer and retry. A failed user access from kernel while a fatal signal is pending should fail even if the instruction in question was WRUSS. do_sigbus() should not send SIGBUS for WRUSS -- it should handle it like any other kernel mode failure. Cc: Dave Hansen Cc: Peter Zijlstra Signed-off-by: Andy Lutomirski --- arch/x86/mm/fault.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index b52064920f0d..602cdf8e070a 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -908,7 +908,7 @@ do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address, vm_fault_t fault) { /* Kernel mode? Handle exceptions or die: */ - if (!(error_code & X86_PF_USER)) { + if (!user_mode(regs)) { no_context(regs, error_code, address, SIGBUS, BUS_ADRERR); return; } @@ -1180,7 +1180,14 @@ do_kern_addr_fault(struct pt_regs *regs, unsigned long hw_error_code, } NOKPROBE_SYMBOL(do_kern_addr_fault); -/* Handle faults in the user portion of the address space */ +/* + * Handle faults in the user portion of the address space. Nothing in here + * should check X86_PF_USER without a specific justification: for almost + * all purposes, we should treat a normal kernel access to user memory + * (e.g. get_user(), put_user(), etc.) the same as the WRUSS instruction. + * The one exception is AC flag handling, which is, per the x86 + * architecture, special for WRUSS. + */ static inline void do_user_addr_fault(struct pt_regs *regs, unsigned long error_code, @@ -1369,14 +1376,14 @@ void do_user_addr_fault(struct pt_regs *regs, if (likely(!(fault & VM_FAULT_ERROR))) return; - if (fatal_signal_pending(current) && !(error_code & X86_PF_USER)) { + if (fatal_signal_pending(current) && !user_mode(regs)) { no_context(regs, error_code, address, 0, 0); return; } if (fault & VM_FAULT_OOM) { /* Kernel mode? Handle exceptions or die: */ - if (!(error_code & X86_PF_USER)) { + if (!user_mode(regs)) { no_context(regs, error_code, address, SIGSEGV, SEGV_MAPERR); return; -- 2.29.2
[PATCH 11/11] x86/fault: Don't look for extable entries for SMEP violations
If we get a SMEP violation or a fault that would have been a SMEP violation if we had SMEP, we shouldn't run fixups. Just OOPS. Cc: Dave Hansen Cc: Peter Zijlstra Signed-off-by: Andy Lutomirski --- arch/x86/mm/fault.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index d39946ad8a91..08f5f74cf989 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -1211,12 +1211,13 @@ void do_user_addr_fault(struct pt_regs *regs, /* * Whoops, this is kernel mode code trying to execute from * user memory. Unless this is AMD erratum #93, we are toast. -* Don't even try to look up the VMA. +* Don't even try to look up the VMA or look for extable +* entries. */ if (is_errata93(regs, address)) return; - bad_area_nosemaphore(regs, error_code, address); + page_fault_oops(regs, error_code, address); return; } -- 2.29.2
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 2:04 PM Andy Lutomirski wrote: > Indeed, and I have tests for this. Do you mean you already have a test case or that you would like a minimized test case? - Kyle
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
> On Jan 31, 2021, at 1:30 PM, Linus Torvalds > wrote: > > Btw Kyle, do you have a good simple test-case for this? Clearly this > is some weird special case use, and regular single-stepping works > fine. > > Indeed, and I have tests for this. TBH, the TIF_SINGLESTEP code makes no sense, and I think it has at least three overloaded meanings. I can try to look in a bit. I’ve mostly avoided breaking it in the past, but that doesn’t mean I understand the original intent.
Linux 5.11-rc6
Things look a little calmer than last week, and over-all very average for rc6. So - like always this late in the release schedule - I'd certainly have liked things to be even calmer, but nothing here really stands out. The diffstat is quite flat, meaning lots of small fixes, with the exception of one new LED driver, and a flurry of PI futex fixes (and one nouveau patch that is just a lot of trivial lines). And all the stats look normal: average number of commits, and they are all in the usual places, with most of the patch being drivers (gpu, networking, sound, etc), but we obviously have all the usual suspects with arch updates, and a smattering of fixes to core code (kernel, mm, networking, filesystems). A few known issues still, hopefully soon fixed, and on the whole things look quite normal apart from some mailing list hiccups.. Go test, Linus --- Adam Harvey (1): cifs: ignore auto and noauto options if given Alex Deucher (1): Revert "drm/amdgpu/swsmu: drop set_fan_speed_percent (v2)" Alexandru Elisei (1): KVM: arm64: Use the reg_to_encoding() macro instead of sys_reg() Amadeusz Sławiński (1): ASoC: topology: Properly unregister DAI on removal Andrea Righi (1): leds: trigger: fix potential deadlock with libata Arnd Bergmann (7): ARM: imx: fix imx8m dependencies ia64: fix timer cleanup regression ia64: fix xchg() warning ia64: Mark architecture as orphaned clk: imx: fix Kconfig warning for i.MX SCU clk clk: mmp2: fix build without CONFIG_PM amdgpu: fix clang build warning Baolin Wang (1): blk-cgroup: Use cond_resched() when destroy blkgs Baoquan He (1): kernel: kexec: remove the lock operation of system_transition_mutex Bard Liao (1): ALSA: hda: intel-dsp-config: add PCI id for TGL-H Bastian Beranek (1): drm/nouveau/dispnv50: Restore pushing of all data. Ben Skeggs (1): drm/nouveau/nvif: fix method count when pushing an array Bharat Gooty (1): arm64: dts: broadcom: Fix USB DMA address translation for Stingray Borislav Petkov (1): x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument Brett Creeley (2): ice: Don't allow more channels than LAN MSI-X available ice: Fix MSI-X vector fallback logic Bruno Thomsen (1): ARM: dts: imx7d-flex-concentrator: fix pcf2127 reset Bryan Tan (1): RDMA/vmw_pvrdma: Fix network_hdr_type reported in WC Chaitanya Kulkarni (1): nvme-pci: add the DISABLE_WRITE_ZEROES quirk for a SPCC device Chao Leng (1): nvme-core: use list_add_tail_rcu instead of list_add_tail for nvme_init_ns_head Chris Wilson (3): drm/i915/gt: Clear CACHE_MODE prior to clearing residuals drm/i915: Always flush the active worker before returning from the wait drm/i915/gt: Always try to reserve GGTT address 0x0 Claudiu Beznea (1): drivers: soc: atmel: add null entry at the end of at91_soc_allowed_list[] Colin Ian King (1): media: rcar-vin: fix return, use ret instead of zero Coly Li (1): bcache: only check feature sets when sb->version >= BCACHE_SB_VERSION_CDEV_WITH_FEATURES Cong Wang (1): af_key: relax availability checks for skb size calculation Corinna Vinschen (1): igc: fix link speed advertising Damien Le Moal (2): block: fix bd_size_lock use null_blk: cleanup zoned mode initialization Dan Carpenter (2): ASoC: topology: Fix memory corruption in soc_tplg_denum_create_values() can: dev: prevent potential information leak in can_fill_info() Daniel Jurgens (1): net/mlx5: Maintain separate page trees for ECPF and PF functions Daniel Wagner (1): nvme-multipath: Early exit if no path is available Daniel Walker (1): spidev: Add cisco device compatible Danielle Ratson (1): selftests: forwarding: Specify interface when invoking mausezahn Dave Wysochanski (2): SUNRPC: Move simple_get_bytes and simple_get_netobj into private header SUNRPC: Handle 0 length opaque XDR object data properly David Brazdil (1): KVM: arm64: Allow PSCI SYSTEM_OFF/RESET to return David Collins (1): regulator: core: avoid regulator_resolve_supply() race condition David Gow (1): soc: litex: Properly depend on HAS_IOMEM David Woodhouse (1): xen: Fix XenStore initialisation for XS_LOCAL Davidlohr Bueso (1): parisc: Remove leftover reference to the power_tasklet Dinghao Liu (1): block: Fix an error handling in add_partition Dmitry Baryshkov (1): clk: qcom: gcc-sm250: Use floor ops for sdcc clks Dmitry Osipenko (1): regulator: consumer: Add missing stubs to regulator/consumer.h Dom Cobley (2): drm/vc4: Correct lbm size and calculation drm/vc4: Correct POS1_SCL for hvs5 Eliot Blennerhassett (1): ASoC: ak4458: correct reset polarity Emmanuel Grumbach (3): iwlwifi: fix the NMI flow for old devices iwlwifi: queue: don't crash if txq->entries is NULL
[PATCH RESEND v2] dt-bindings: mfd: Convert rn5t618 to json-schema
Convert the RN5T618 binding to DT schema format. Also clearly state which regulators are available. Signed-off-by: Andreas Kemnade Reviewed-by: Rob Herring --- https://lore.kernel.org/lkml/cal_jsqjwt91+azwaweuvjobqgsyw6gbhqmohwu_t5qzabxx...@mail.gmail.com/ Changes in v2: - drop irq description Due to its .txt-format history BSD license was not added. .../bindings/mfd/ricoh,rn5t618.yaml | 111 ++ .../devicetree/bindings/mfd/rn5t618.txt | 52 2 files changed, 111 insertions(+), 52 deletions(-) create mode 100644 Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml delete mode 100644 Documentation/devicetree/bindings/mfd/rn5t618.txt diff --git a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml new file mode 100644 index ..d70e85a09c84 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml @@ -0,0 +1,111 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/ricoh,rn5t618.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ricoh RN5T567/RN5T618/RC5T619 PMIC + +maintainers: + - Andreas Kemnade + +description: | + Ricoh RN5T567/RN5T618/RC5T619 is a power management IC family which + integrates 3 to 5 step-down DCDC converters, 7 to 10 low-dropout regulators, + GPIOs, and a watchdog timer. It can be controlled through an I2C interface. + The RN5T618/RC5T619 provides additionally a Li-ion battery charger, + fuel gauge, and an ADC. + The RC5T619 additionnally includes USB charger detection and an RTC. + +allOf: + - if: + properties: +compatible: + contains: +const: ricoh,rn5t567 +then: + properties: +regulators: + patternProperties: +"^(DCDC[1-4]|LDO[1-5]|LDORTC[12])$": + $ref: ../regulator/regulator.yaml + additionalProperties: false + - if: + properties: +compatible: + contains: +const: ricoh,rn5t618 +then: + properties: +regulators: + patternProperties: +"^(DCDC[1-3]|LDO[1-5]|LDORTC[12])$": + $ref: ../regulator/regulator.yaml + additionalProperties: false + - if: + properties: +compatible: + contains: +const: ricoh,rc5t619 +then: + properties: +regulators: + patternProperties: +"^(DCDC[1-5]|LDO[1-9]|LDO10|LDORTC[12])$": + $ref: ../regulator/regulator.yaml + additionalProperties: false + +properties: + compatible: +enum: + - ricoh,rn5t567 + - ricoh,rn5t618 + - ricoh,rc5t619 + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + system-power-controller: +type: boolean +description: | + See Documentation/devicetree/bindings/power/power-controller.txt + + regulators: +type: object + +additionalProperties: false + +required: + - compatible + - reg + +examples: + - | +#include +i2c { + #address-cells = <1>; + #size-cells = <0>; + + pmic@32 { +compatible = "ricoh,rn5t618"; +reg = <0x32>; +interrupt-parent = <>; +interrupts = <11 IRQ_TYPE_EDGE_FALLING>; +system-power-controller; + +regulators { + DCDC1 { +regulator-min-microvolt = <105>; +regulator-max-microvolt = <105>; + }; + + DCDC2 { +regulator-min-microvolt = <1175000>; +regulator-max-microvolt = <1175000>; + }; +}; + }; +}; diff --git a/Documentation/devicetree/bindings/mfd/rn5t618.txt b/Documentation/devicetree/bindings/mfd/rn5t618.txt deleted file mode 100644 index 16778ea00dbc.. --- a/Documentation/devicetree/bindings/mfd/rn5t618.txt +++ /dev/null @@ -1,52 +0,0 @@ -* Ricoh RN5T567/RN5T618 PMIC - -Ricoh RN5T567/RN5T618/RC5T619 is a power management IC family which -integrates 3 to 5 step-down DCDC converters, 7 to 10 low-dropout regulators, -GPIOs, and a watchdog timer. It can be controlled through an I2C interface. -The RN5T618/RC5T619 provides additionally a Li-ion battery charger, -fuel gauge, and an ADC. -The RC5T619 additionnally includes USB charger detection and an RTC. - -Required properties: - - compatible: must be one of - "ricoh,rn5t567" - "ricoh,rn5t618" - "ricoh,rc5t619" - - reg: the I2C slave address of the device - -Optional properties: - - interrupts: interrupt mapping for IRQ - See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt - - system-power-controller: - See Documentation/devicetree/bindings/power/power-controller.txt - -Sub-nodes: - - regulators: the node is required if the regulator functionality is - needed. The valid regulator names are: DCDC1, DCDC2, DCDC3, DCDC4 - (RN5T567/RC5T619), LDO1, LDO2, LDO3, LDO4, LDO5, LDO6,
Re: [PATCH v4 1/2] x86/setup: always add the beginning of RAM as memblock.memory
On Sun, Jan 31, 2021 at 12:04 AM Mike Rapoport wrote: > > > > > That's *particularly* true when the very line above it did a > > "memblock_reserve()" of the exact same range that the memblock_add() > > "adds". > > The most correct thing to do would have been to > > memblock_add(0, end_of_first_memory_bank); > > Somewhere at e820__memblock_setup(). You miss my complaint. Why does the memblock code care about this magical "memblock_add()", when we just told it that the SAME REGION is reserved by doing a "memblock_reserve()"? IOW, I'm not interested in "the correct thing to do would have been [another memblock_add()]". I'm saying that the memblock code itself is being confused, and no additional thing should have been required at all, because we already *did* that memblock_reserve(). See? Honestly, I'm not seeing it being a good thing to move further towards memblock code as the primary model for memory initialization, when the memblock code is so confused. Linus
Re: [PATCH v1 0/2] Make fw_devlink=on more forgiving
On Fri, Jan 29, 2021 at 8:03 PM Saravana Kannan wrote: > > This patch series solves two general issues with fw_devlink=on > > Patch 1/2 addresses the issue of firmware nodes that look like they'll > have struct devices created for them, but will never actually have > struct devices added for them. For example, DT nodes with a compatible > property that don't have devices added for them. > > Patch 2/2 address (for static kernels) the issue of optional suppliers > that'll never have a driver registered for them. So, if the device could > have probed with fw_devlink=permissive with a static kernel, this patch > should allow those devices to probe with a fw_devlink=on. This doesn't > solve it for the case where modules are enabled because there's no way > to tell if a driver will never be registered or it's just about to be > registered. I have some other ideas for that, but it'll have to come > later thinking about it a bit. > > These two patches might remove the need for several other patches that > went in as fixes for commit e590474768f1 ("driver core: Set > fw_devlink=on by default"), but I think all those fixes are good > changes. So I think we should leave those in. > > Marek, Geert, > > Can you try this series on a static kernel with your OF_POPULATED > changes reverted? I just want to make sure these patches can identify > and fix those cases. > > Tudor, > > You should still make the clock driver fix (because it's a bug), but I > think this series will fix your issue too (even without the clock driver > fix). Can you please give this a shot? > > Marc, > > Can you try this series with the gpiolib fix reverted please? I'm pretty > sure this will fix that case. > > Linus, > > This series very likely removes the need for the gpiolib patch (we can > wait for Marc to confirm). I'm split on whether we should leave it in or > not. Thoughts? Actually, thinking more about this, we should keep the gpiolib patch. It'll ensure the suspend/resume order is always correct. This series basically gives up on creating device links to firmware nodes that don't have a corresponding device added. The gpiolib patch makes sure the nodes have a device that corresponds to them. So device links will get created to the gpio_device and will make sure the parent of the gpio_device doesn't suspend before the consumers of the gpio. -Saravana
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
Btw Kyle, do you have a good simple test-case for this? Clearly this is some weird special case use, and regular single-stepping works fine. Linus
Re: [PATCH v5] gpiolib: Bind gpio_device to a driver to enable fw_devlink=on by default
On Sat, Jan 30, 2021 at 9:39 AM Dmitry Osipenko wrote: > > 22.01.2021 22:35, Saravana Kannan пишет: > > There are multiple instances of GPIO device tree nodes of the form: > > > > foo { > > compatible = "acme,foo"; > > ... > > > > gpio0: gpio0@ { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > gpio1: gpio1@ { > > compatible = "acme,bar"; > > ... > > gpio-controller; > > }; > > > > ... > > } > > > > bazz { > > my-gpios = < ...>; > > } > > > > Case 1: The driver for "foo" populates struct device for these gpio* > > nodes and then probes them using a driver that binds with "acme,bar". > > This driver for "acme,bar" then registers the gpio* nodes with gpiolib. > > This lines up with how DT nodes with the "compatible" property are > > typically converted to struct devices and then registered with driver > > core to probe them. This also allows the gpio* devices to hook into all > > the driver core capabilities like runtime PM, probe deferral, > > suspend/resume ordering, device links, etc. > > > > Case 2: The driver for "foo" doesn't populate struct devices for these > > gpio* nodes before registering them with gpiolib. Instead it just loops > > through its child nodes and directly registers the gpio* nodes with > > gpiolib. > > > > Drivers that follow case 2 cause problems with fw_devlink=on. This is > > because fw_devlink will prevent bazz from probing until there's a struct > > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO > > supplier). Once the struct device is available, fw_devlink will create a > > device link with gpio0 device as the supplier and bazz device as the > > consumer. After this point, since the gpio0 device will never bind to a > > driver, the device link will prevent bazz device from ever probing. > > > > Finding and refactoring all the instances of drivers that follow case 2 > > will cause a lot of code churn and it is not something that can be done > > in one shot. In some instances it might not even be possible to refactor > > them cleanly. Examples of such instances are [1] [2]. > > > > This patch works around this problem and avoids all the code churn by > > simply setting the fwnode of the gpio_device and creating a stub driver > > to bind to the gpio_device. This allows all the consumers to continue > > probing when the driver follows case 2. > > > > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/ > > [2] - > > https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba...@kernel.org/ > > Cc: Marc Zyngier > > Cc: Jisheng Zhang > > Cc: Kever Yang > > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") > > Signed-off-by: Saravana Kannan > > --- > > v1 -> v2: > > - Fixed up compilation errors that were introduced accidentally > > - Fixed a missing put_device() > > > > v2 -> v3: > > - Changed chip_warn() to pr_warn() > > - Changed some variable names > > > > v3 -> v4: > > - Dropped the warning since it's not always valid > > - This simplifies the code a lot > > - Added comments and fixed up commit text > > > > v4 -> v5: > > - Fixed the code to not mess up non-DT cases. > > - Moved code into gpiolib-of.c > > > > drivers/gpio/gpiolib-of.c | 11 +++ > > drivers/gpio/gpiolib-of.h | 5 + > > drivers/gpio/gpiolib.c| 38 +++--- > > 3 files changed, 47 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c > > index b4a71119a4b0..baf0153b7bca 100644 > > --- a/drivers/gpio/gpiolib-of.c > > +++ b/drivers/gpio/gpiolib-of.c > > @@ -1039,3 +1039,14 @@ void of_gpiochip_remove(struct gpio_chip *chip) > > { > > of_node_put(chip->of_node); > > } > > + > > +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev) > > +{ > > + /* If the gpiochip has an assigned OF node this takes precedence */ > > + if (gc->of_node) > > + gdev->dev.of_node = gc->of_node; > > + else > > + gc->of_node = gdev->dev.of_node; > > + if (gdev->dev.of_node) > > + gdev->dev.fwnode = of_fwnode_handle(gdev->dev.of_node); > > +} > > diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h > > index ed26664f1537..8af2bc899aab 100644 > > --- a/drivers/gpio/gpiolib-of.h > > +++ b/drivers/gpio/gpiolib-of.h > > @@ -15,6 +15,7 @@ int of_gpiochip_add(struct gpio_chip *gc); > > void of_gpiochip_remove(struct gpio_chip *gc); > > int of_gpio_get_count(struct device *dev, const char *con_id); > > bool of_gpio_need_valid_mask(const struct gpio_chip *gc); > > +void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev); > > #else > > static inline struct gpio_desc *of_find_gpio(struct device *dev, > >const char *con_id, > > @@ -33,6 +34,10 @@ static inline bool
Re: [PATCH] openrisc: use device tree to determine present cpus
On Sun, Jan 31, 2021 at 09:22:31AM +0100, Jan Henrik Weinstock wrote: > On 31/01/2021 00:03, Stafford Horne wrote: > > > This looks good, one small comment below. Can you send the next patch as a > > v2? > > > > Using 'git format-patch -v2 ...' > > Sorry, was not aware of that, will do better next time! > > > Should we warn on the else case? > > I think it is fine for the kernel to have room for more CPUs than are > actually present (i.e. NR_CPUs > present_cpus is OK). Other Archs do not > show a warning in this case either, therefore I also omitted it. Fair enough. Reviewed-by: Stafford Horne I can queue this for 5.12 when you send v2. -Stafford
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 12:20 PM Gabriel Krisman Bertazi wrote: > > > I think we should migrate TIF_SINGLESTEP to a SYSCALL_WORK flag as that > is just a simple refactor. That makes no sense at all to me. A single-step has absolutely nothing to do with system calls, and it's also not what any other architecture is doing. So honestly, something is wrong if TIF_SINGLESTEP should be moved to the SYSCALL_WORK_SYSCALL_xyz flags. That's not what single-stepping is at all about. It looks like commit 299155244770 ("entry: Drop usage of TIF flags in the generic syscall code") is simply wrong. The whole premise for it was wrong because it didn't notice that TIF_SINGLESTEP isn't a syscall flag - and shouldn't be one. The problem seems to be that TIF_SINGLESTEP has two different meanings: the usual "enable single stepping", *and* then a special magic hack to also do it at system call exit. You only seem to have looked at the magic hack, not the actual REAL meaning of TIF_SINGLESTEP, which causes TF to be set and the debug fault. This whole code is very confusing. What is that "arch_syscall_exit_tracehook()" thing in the first place? I see an arch wrapper, but I don't see any architecture that actually defines it, so it seems to always become just tracehook_report_syscall_exit(). It then does (on x86) the generic version, which does if (step) user_single_step_report(regs); else ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT); where that user_single_step_report() is a nonsensical special case that just does what a debug fault does for all the *usual* single step events in exc_debug_user() (which does not use that pointless "user_single_step_report()" function, but just does send_sigtrap(regs, 0, get_si_code(dr6)); so please, let's take a step back here, and look at the basics: - the *main* user of TIF_SINGLESTEP is regular debugging that causes a #DB after each user space instruction, and causes that send_sigtrap() in exc_debug_user() - there is one very odd and weird special case that is about having system call tracing enabled, which seems to have unclear semantics and this is the case that got broken. What's the fix here? Because no, single-stepping isn't about system calls, and we should not try to change the code to be about system calls, because no other architecture does that EITHER. Now, it's possible that eventually (a) the normal TF flag setting doesn't actually need TIF_SINGESTEP at all (b) the TIF_SINGLESTEP bit really only needs to be about the odd special case for system calls. but as things are now, (a) is *NOT* true. We have very magical behavior wrt TIF_SINGLESTEP, see enable_single_step(), and the comment about /* * If TF was already set, check whether it was us who set it. * If not, we should never attempt a block step. */ which actually ends up depending very subtly on whether TIF_SINGLESTEP was already set *before* we did that enable_single_step(), and/or whether TF was already set on the stack (because users can set it themselves without ptrace). Again, the special system call case is the special case, NOT the normal case. Don't try to make TIF_SINGLESTEP be about the special case. Linus Linus
Re: [PATCH v2 0/2] of: property: Add fw_devlink support for more props
On Sun, Jan 31, 2021 at 8:38 AM Martin Kaiser wrote: > > Dear all, > > Thus wrote Saravana Kannan (sarava...@google.com): > > > Sending again because I messed up the To/Cc for the coverletter. > > > This series combines two patches [1] [2] that'd conflict. > > > Greg, > > > Can you please pull this into driver-core-next? > > > -Saravana > > > [1] - > > https://lore.kernel.org/lkml/20210115210159.3090203-1-sarava...@google.com/ > > [2] - > > https://lore.kernel.org/lkml/20201218210750.3455872-1-sarava...@google.com/ > > I'm running linux-next on my hardware which is based on the imx258 > chipset by Freescale/NXP. > > When those two patches appeared in linux-next, my system would not boot > any more. It was stuck right after > > Uncompressing Linux... done, booting the kernel. > > Reverting the irq-patch made the system boot again. Still, a number of > devices like usb or nand flash controller are not found any more. > If I revert the gpio patch as well, all devices are available again. > > My system's device tree is based on arch/arm/boot/dts/imx25.dtsi with > very few adaptations for my board. > > I tried to play around with the new parse_interrupts() function to > figure out which device causes the boot failure. If I skip the following > device, I can boot again: > > - return of_irq_find_parent(np); > + np_ret = of_irq_find_parent(np); > + if (!strcmp(np->full_name, "serial@50008000")) { > + printk(KERN_ERR "skip serial@50008000\n"); > + return NULL; > + } > + return np_ret; > > This is uart4 of the imx258 chip, which I use as my serial console. The > imx25.dtsi device tree seems ok, we find an interrupt parent. The > problem must be in the code that processes the result of > parse_interrupts(). > > I tried to boot the unmodified code with qemu, simulating the imx25-pdk > device. This wouldn't boot either. > > Does this ring any bells with anyone? This series [1] has a high chance of fixing it for you if CONFIG_MODULES is disabled in your set up. Can you give it a shot? The real problem is that arch/arm/mach-imx/avic.c doesn't set the OF_POPULATED flag for the "fsl,avic" node. fw_devlink uses this information to know that this device node will never have a struct device created for it. The proper way to do this for root IRQCHIP nodes is to use IRQCHIP_DECLARE(). I Cc'ed you on a clean up patch for IMX [2], can you please give [2] a shot *without* [1] and with CONFIG_MODULES enabled? Things should boot properly with this combination too. Btw, for future reference, you can try enabling the logs in device_links_check_suppliers() to see what devices are being blocked on what supplier nodes. [1] - https://lore.kernel.org/lkml/20210130040344.2807439-1-sarava...@google.com/ [2] - https://lore.kernel.org/lkml/20210131205654.3379661-1-sarava...@google.com/T/#u Thanks, Saravana
[PATCH 08/11] x86/fault: Bypass no_context() for implicit kernel faults from usermode
We can drop an indentation level and remove the last user_mode(regs) == true caller of no_context() by directly OOPSing for implicit kernel faults from usermode. Cc: Dave Hansen Cc: Peter Zijlstra Signed-off-by: Andy Lutomirski --- arch/x86/mm/fault.c | 59 - 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 6f43d080e1e8..177b612c7f33 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -789,44 +789,49 @@ __bad_area_nosemaphore(struct pt_regs *regs, unsigned long error_code, { struct task_struct *tsk = current; - /* User mode accesses just cause a SIGSEGV */ - if (user_mode(regs) && (error_code & X86_PF_USER)) { - /* -* It's possible to have interrupts off here: -*/ - local_irq_enable(); + if (!user_mode(regs)) { + no_context(regs, error_code, address, pkey, si_code); + return; + } - /* -* Valid to do another page fault here because this one came -* from user space: -*/ - if (is_prefetch(regs, error_code, address)) - return; + if (!(error_code & X86_PF_USER)) { + /* Implicit user access to kernel memory -- just oops */ + page_fault_oops(regs, error_code, address); + return; + } - if (is_errata100(regs, address)) - return; + /* +* User mode accesses just cause a SIGSEGV. +* It's possible to have interrupts off here: +*/ + local_irq_enable(); - sanitize_error_code(address, _code); + /* +* Valid to do another page fault here because this one came +* from user space: +*/ + if (is_prefetch(regs, error_code, address)) + return; - if (fixup_vdso_exception(regs, X86_TRAP_PF, error_code, address)) - return; + if (is_errata100(regs, address)) + return; - if (likely(show_unhandled_signals)) - show_signal_msg(regs, error_code, address, tsk); + sanitize_error_code(address, _code); - set_signal_archinfo(address, error_code); + if (fixup_vdso_exception(regs, X86_TRAP_PF, error_code, address)) + return; - if (si_code == SEGV_PKUERR) - force_sig_pkuerr((void __user *)address, pkey); + if (likely(show_unhandled_signals)) + show_signal_msg(regs, error_code, address, tsk); - force_sig_fault(SIGSEGV, si_code, (void __user *)address); + set_signal_archinfo(address, error_code); - local_irq_disable(); + if (si_code == SEGV_PKUERR) + force_sig_pkuerr((void __user *)address, pkey); - return; - } + force_sig_fault(SIGSEGV, si_code, (void __user *)address); - no_context(regs, error_code, address, SIGSEGV, si_code); + local_irq_disable(); } static noinline void -- 2.29.2
Re: [PATCH] platform/x86: dell-wmi-sysman: fix a NULL pointer dereference
Hi, On 1/31/21 3:04 PM, Limonciello, Mario wrote: > > >> -Original Message- >> From: Hans de Goede >> Sent: Saturday, January 30, 2021 15:44 >> To: Limonciello, Mario; Mark Gross >> Cc: LKML; platform-driver-...@vger.kernel.org >> Subject: Re: [PATCH] platform/x86: dell-wmi-sysman: fix a NULL pointer >> dereference >> >> >> [EXTERNAL EMAIL] >> >> Hi, >> >> On 1/29/21 6:26 PM, Mario Limonciello wrote: >>> An upcoming Dell platform is causing a NULL pointer dereference >>> in dell-wmi-sysman initialization. Validate that the input from >>> BIOS matches correct ACPI types and abort module initialization >>> if it fails. >>> >>> This leads to a memory leak that needs to be cleaned up properly. >>> >>> Signed-off-by: Mario Limonciello >>> --- >>> drivers/platform/x86/dell-wmi-sysman/sysman.c | 8 +++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/platform/x86/dell-wmi-sysman/sysman.c >> b/drivers/platform/x86/dell-wmi-sysman/sysman.c >>> index dc6dd531c996..38b497991071 100644 >>> --- a/drivers/platform/x86/dell-wmi-sysman/sysman.c >>> +++ b/drivers/platform/x86/dell-wmi-sysman/sysman.c >>> @@ -419,13 +419,19 @@ static int init_bios_attributes(int attr_type, const >> char *guid) >>> return retval; >>> /* need to use specific instance_id and guid combination to get right >> data */ >>> obj = get_wmiobj_pointer(instance_id, guid); >>> - if (!obj) >>> + if (!obj || obj->type != ACPI_TYPE_PACKAGE) { >>> + release_attributes_data(); >> >> All calls of init_bios_attributes() have the following error handling: >> >> ret = init_bios_attributes(INT, >> DELL_WMI_BIOS_INTEGER_ATTRIBUTE_GUID); >> if (ret) { >> pr_debug("failed to populate integer type attributes\n"); >> goto fail_create_group; >> } >> >> ... >> >> fail_create_group: >> release_attributes_data(); >> >> So that added release_attributes_data() call is not necessary. If you can >> respin >> this patch Monday with the release_attributes_data(); addition dropped, then >> I will try to get this to Linus in time for 5.11 . >> >> Or I can fix this up locally if you agree with dropping the unnecessary >> release_attributes_data() call. >> > > Yes, please go ahead and drop the unnecessary call locally. Ok, I've merged this into my review-hans branch and I will push this out to for-next as soon as a local build has finished. I'll also include this in my last fixes pull-req for Linus later this week. While working on this I did notice that the function in question still has a bunch of further issues. But since this patch fixes a crash and has been tested I've decided to move forward with it as is (with the duplicate release_attributes_data() call dropped). The further issues can be fixed with follow-up patches. So the other issues which I noticed are: 1. Calling release_attributes_data() in the error-path here: obj = get_wmiobj_pointer(instance_id, guid); if (!obj || obj->type != ACPI_TYPE_PACKAGE) { return -ENODEV; } Is not necessary as discussed, but the added obj->type != ACPI_TYPE_PACKAGE which I assume triggers to fix the reported crash, means that obj is not NULL in which case we should free it. So this should become: obj = get_wmiobj_pointer(instance_id, guid); if (!obj || obj->type != ACPI_TYPE_PACKAGE) { kfree(obj); return -ENODEV; } Note that the kfree() will be a no-op when obj == NULL. This means that with just the current fix merged there is a small memleak on machines where we hit the error-path. I've decided that we can live with that, since the alternative is the crash or me pushing something untested. 2. There is a while below this if, which gets a new obj pointer: obj = get_wmiobj_pointer(instance_id, guid); if (!obj || obj->type != ACPI_TYPE_PACKAGE) { kfree(obj); return -ENODEV; } elements = obj->package.elements; mutex_lock(_priv.mutex); while (elements) { ... nextobj: kfree(obj); instance_id++; obj = get_wmiobj_pointer(instance_id, guid); elements = obj ? obj->package.elements : NULL; } This is missing a check for the obj->type for the new obj when going into a second (or higher) iteration of the loop. This check can be added by moving the original check to inside the loop like this: obj = get_wmiobj_pointer(instance_id, guid); mutex_lock(_priv.mutex); while (obj) { if (obj->type != ACPI_TYPE_PACKAGE) { err = ENODEV; goto err_attr_init; } elements = obj->package.elements; ... nextobj: kfree(obj); instance_id++; obj =
[PATCH v1] ARM: imx: avic: Convert to using IRQCHIP_DECLARE
Remove a lot of boilerplate code. Also address boot issues on imx25 with fw_devlink=on that were reported by Martin. Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default") Reported-by: Martin Kaiser Signed-off-by: Saravana Kannan --- I've compile tested this for imx25 and imx27. But I don't have any devices to test this on. -Saravana arch/arm/mach-imx/avic.c | 16 +++- arch/arm/mach-imx/common.h | 1 - arch/arm/mach-imx/mach-imx1.c | 11 --- arch/arm/mach-imx/mach-imx25.c | 12 arch/arm/mach-imx/mach-imx27.c | 12 arch/arm/mach-imx/mach-imx31.c | 1 - arch/arm/mach-imx/mach-imx35.c | 1 - arch/arm/mach-imx/mm-imx3.c| 24 8 files changed, 15 insertions(+), 63 deletions(-) diff --git a/arch/arm/mach-imx/avic.c b/arch/arm/mach-imx/avic.c index 322caa21bcb3..e67e1c2799d1 100644 --- a/arch/arm/mach-imx/avic.c +++ b/arch/arm/mach-imx/avic.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -162,7 +163,7 @@ static void __exception_irq_entry avic_handle_irq(struct pt_regs *regs) * interrupts. It registers the interrupt enable and disable functions * to the kernel for each interrupt source. */ -void __init mxc_init_irq(void __iomem *irqbase) +static void __init mxc_init_irq(void __iomem *irqbase) { struct device_node *np; int irq_base; @@ -220,3 +221,16 @@ void __init mxc_init_irq(void __iomem *irqbase) printk(KERN_INFO "MXC IRQ initialized\n"); } + +static int __init imx_avic_init(struct device_node *node, + struct device_node *parent) +{ + void __iomem *avic_base; + + avic_base = of_iomap(node, 0); + BUG_ON(!avic_base); + mxc_init_irq(avic_base); + return 0; +} + +IRQCHIP_DECLARE(imx_avic, "fsl,imx31-avic", imx_avic_init); diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h index 2d76e2c6c99e..e988b0978a42 100644 --- a/arch/arm/mach-imx/common.h +++ b/arch/arm/mach-imx/common.h @@ -22,7 +22,6 @@ void mx35_map_io(void); void imx21_init_early(void); void imx31_init_early(void); void imx35_init_early(void); -void mxc_init_irq(void __iomem *); void mx31_init_irq(void); void mx35_init_irq(void); void mxc_set_cpu_type(unsigned int type); diff --git a/arch/arm/mach-imx/mach-imx1.c b/arch/arm/mach-imx/mach-imx1.c index 32df3b8012f9..8eca92d66a2e 100644 --- a/arch/arm/mach-imx/mach-imx1.c +++ b/arch/arm/mach-imx/mach-imx1.c @@ -17,16 +17,6 @@ static void __init imx1_init_early(void) mxc_set_cpu_type(MXC_CPU_MX1); } -static void __init imx1_init_irq(void) -{ - void __iomem *avic_addr; - - avic_addr = ioremap(MX1_AVIC_ADDR, SZ_4K); - WARN_ON(!avic_addr); - - mxc_init_irq(avic_addr); -} - static const char * const imx1_dt_board_compat[] __initconst = { "fsl,imx1", NULL @@ -34,7 +24,6 @@ static const char * const imx1_dt_board_compat[] __initconst = { DT_MACHINE_START(IMX1_DT, "Freescale i.MX1 (Device Tree Support)") .init_early = imx1_init_early, - .init_irq = imx1_init_irq, .dt_compat = imx1_dt_board_compat, .restart= mxc_restart, MACHINE_END diff --git a/arch/arm/mach-imx/mach-imx25.c b/arch/arm/mach-imx/mach-imx25.c index 95de48a1aa7d..51927bd08aef 100644 --- a/arch/arm/mach-imx/mach-imx25.c +++ b/arch/arm/mach-imx/mach-imx25.c @@ -22,17 +22,6 @@ static void __init imx25_dt_init(void) imx_aips_allow_unprivileged_access("fsl,imx25-aips"); } -static void __init mx25_init_irq(void) -{ - struct device_node *np; - void __iomem *avic_base; - - np = of_find_compatible_node(NULL, NULL, "fsl,avic"); - avic_base = of_iomap(np, 0); - BUG_ON(!avic_base); - mxc_init_irq(avic_base); -} - static const char * const imx25_dt_board_compat[] __initconst = { "fsl,imx25", NULL @@ -42,6 +31,5 @@ DT_MACHINE_START(IMX25_DT, "Freescale i.MX25 (Device Tree Support)") .init_early = imx25_init_early, .init_machine = imx25_dt_init, .init_late = imx25_pm_init, - .init_irq = mx25_init_irq, .dt_compat = imx25_dt_board_compat, MACHINE_END diff --git a/arch/arm/mach-imx/mach-imx27.c b/arch/arm/mach-imx/mach-imx27.c index 262422a9c196..e325c9468105 100644 --- a/arch/arm/mach-imx/mach-imx27.c +++ b/arch/arm/mach-imx/mach-imx27.c @@ -56,17 +56,6 @@ static void __init imx27_init_early(void) mxc_set_cpu_type(MXC_CPU_MX27); } -static void __init mx27_init_irq(void) -{ - void __iomem *avic_base; - struct device_node *np; - - np = of_find_compatible_node(NULL, NULL, "fsl,avic"); - avic_base = of_iomap(np, 0); - BUG_ON(!avic_base); - mxc_init_irq(avic_base); -} - static const char * const imx27_dt_board_compat[] __initconst = { "fsl,imx27", NULL @@ -75,7 +64,6 @@ static const char * const
[PATCH] Staging: wimax: i2400m: fixing several coding style issues.
From: Dmitrii Wolf Fixed a coding style issues. Signed-off-by: Dmitrii Wolf --- drivers/staging/wimax/i2400m/debugfs.c | 2 +- drivers/staging/wimax/i2400m/netdev.c | 2 +- drivers/staging/wimax/i2400m/rx.c | 23 +++ drivers/staging/wimax/i2400m/usb.c | 2 +- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/drivers/staging/wimax/i2400m/debugfs.c b/drivers/staging/wimax/i2400m/debugfs.c index 1c640b41ea4c..4e6e1e3f015e 100644 --- a/drivers/staging/wimax/i2400m/debugfs.c +++ b/drivers/staging/wimax/i2400m/debugfs.c @@ -170,7 +170,7 @@ int debugfs_i2400m_reset_set(void *data, u64 val) int result; struct i2400m *i2400m = data; enum i2400m_reset_type rt = val; - switch(rt) { + switch (rt) { case I2400M_RT_WARM: case I2400M_RT_COLD: case I2400M_RT_BUS: diff --git a/drivers/staging/wimax/i2400m/netdev.c b/drivers/staging/wimax/i2400m/netdev.c index 8339d600e77b..0895a2e441d3 100644 --- a/drivers/staging/wimax/i2400m/netdev.c +++ b/drivers/staging/wimax/i2400m/netdev.c @@ -523,7 +523,7 @@ void i2400m_net_erx(struct i2400m *i2400m, struct sk_buff *skb, d_fnstart(2, dev, "(i2400m %p skb %p [%u] cs %d)\n", i2400m, skb, skb->len, cs); - switch(cs) { + switch (cs) { case I2400M_CS_IPV4_0: case I2400M_CS_IPV4: i2400m_rx_fake_eth_header(i2400m->wimax_dev.net_dev, diff --git a/drivers/staging/wimax/i2400m/rx.c b/drivers/staging/wimax/i2400m/rx.c index c9fb619a9e01..807bd3db69e9 100644 --- a/drivers/staging/wimax/i2400m/rx.c +++ b/drivers/staging/wimax/i2400m/rx.c @@ -485,8 +485,7 @@ struct i2400m_roq_data { * store the sequence number (sn) and the cs (packet type) coming from * the RX payload header from the device. */ -struct i2400m_roq -{ +struct i2400m_roq { unsigned ws; struct sk_buff_head queue; struct i2400m_roq_log *log; @@ -556,7 +555,7 @@ void i2400m_roq_log_entry_print(struct i2400m *i2400m, unsigned index, { struct device *dev = i2400m_dev(i2400m); - switch(e->type) { + switch (e->type) { case I2400M_RO_TYPE_RESET: dev_err(dev, "q#%d reset ws %u cnt %u sn %u/%u" " - new nws %u\n", @@ -764,9 +763,9 @@ unsigned __i2400m_roq_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq, new_nws); __skb_unlink(skb_itr, >queue); i2400m_net_erx(i2400m, skb_itr, roq_data_itr->cs); - } - else + } else { break; /* rest of packets all nsn_itr > nws */ + } } roq->ws = sn; return new_nws; @@ -819,7 +818,7 @@ void i2400m_roq_reset(struct i2400m *i2400m, struct i2400m_roq *roq) */ static void i2400m_roq_queue(struct i2400m *i2400m, struct i2400m_roq *roq, - struct sk_buff * skb, unsigned lbn) + struct sk_buff *skb, unsigned lbn) { struct device *dev = i2400m_dev(i2400m); unsigned nsn, len; @@ -882,7 +881,7 @@ void i2400m_roq_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq, */ static void i2400m_roq_queue_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq, - struct sk_buff * skb, unsigned sn) + struct sk_buff *skb, unsigned sn) { struct device *dev = i2400m_dev(i2400m); unsigned nsn, old_ws, len; @@ -1046,7 +1045,7 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx, ro_type, ro_cin, roq->ws, ro_sn, __i2400m_roq_nsn(roq, ro_sn), size); d_dump(2, dev, payload, size); - switch(ro_type) { + switch (ro_type) { case I2400M_RO_TYPE_RESET: i2400m_roq_reset(i2400m, roq); kfree_skb(skb); /* no data here */ @@ -1068,9 +1067,9 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx, spin_lock_irqsave(>rx_lock, flags); kref_put(>rx_roq_refcount, i2400m_rx_roq_destroy); spin_unlock_irqrestore(>rx_lock, flags); - } - else + } else { i2400m_net_erx(i2400m, skb, cs); + } error_skb_clone: error: d_fnend(2, dev, "(i2400m %p skb_rx %p single %u payload %p " @@ -1346,7 +1345,7 @@ int i2400m_rx_setup(struct i2400m *i2400m) { int result = 0; - i2400m->rx_reorder = i2400m_rx_reorder_disabled? 0 : 1; + i2400m->rx_reorder = i2400m_rx_reorder_disabled ? 0 : 1; if (i2400m->rx_reorder) { unsigned itr; struct i2400m_roq_log *rd; @@ -1365,7 +1364,7 @@ int i2400m_rx_setup(struct i2400m *i2400m) goto error_roq_log_alloc; } - for(itr = 0; itr <
Re: [RFC PATCH v4 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors
Hi a clarification down below regarding something I pointed out in the other thread (just to be sure I have not pointed out something plain wrong :D) Thanks Cristian On Sun, Jan 31, 2021 at 01:11:41PM +, Jonathan Cameron wrote: > On Fri, 29 Jan 2021 22:18:18 + > Jyoti Bhayana wrote: > > > This change provides ARM SCMI Protocol based IIO device. > > This driver provides support for Accelerometer and Gyroscope using > > SCMI Sensor Protocol extensions added in the SCMIv3.0 ARM specification > > > > Signed-off-by: Jyoti Bhayana > > A few minor things noticed on a fresh read through, but mostly I think > we are down to figuring out how to deal with the range (as discussed > in the thread continuing on v3). > > On another note, probably time to drop the RFC or give a bit more detail > on why you think this isn't ready to be applied. > > Thanks, > > Jonathan > [snip] > > + > > +static int scmi_iio_dev_probe(struct scmi_device *sdev) > > +{ > > + const struct scmi_sensor_info *sensor_info; > > + struct scmi_handle *handle = sdev->handle; > > + struct device *dev = >dev; > > + struct iio_dev *scmi_iio_dev; > > + u16 nr_sensors; > > + int err, i; > > + > > + if (!handle || !handle->sensor_ops) { > > + dev_err(dev, "SCMI device has no sensor interface\n"); > I'm going to guess we can't actually get here because the registration > would't have happened if either of those are true? > If so perhaps drop the error message. > > > + return -EINVAL; > > + } > > + > > + nr_sensors = handle->sensor_ops->count_get(handle); > > + if (!nr_sensors) { > > + dev_dbg(dev, "0 sensors found via SCMI bus\n"); > -ENODEV maybe? > > + return -EINVAL; > > + } > > + > > + dev_dbg(dev, "%d sensors found via SCMI bus\n", nr_sensors); > > Clear out any debug prints out that don't provide info that can't be obtained > farily easily from elsewhere. In this case they will either be registered > or not and we'll get error messages. > These sort of prints bitrot over time so we want to limit them to the truely > useful. > > > + > > + for (i = 0; i < nr_sensors; i++) { > > + sensor_info = handle->sensor_ops->info_get(handle, i); > > + if (!sensor_info) { > > + dev_err(dev, "SCMI sensor %d has missing info\n", i); > > + return -EINVAL; > > + } > > + > > + /* Skipping scalar sensor,as this driver only supports accel > > and gyro */ > > + if (sensor_info->num_axis == 0) > > + continue; > > So there is a situation where this driver never creates anything? In that > path I'd > like to see an -ENODEV error return. > You mean -ENODEV only if this driver does not find at least one good/supported GYRO/ACCEL sensor right ? I would expect a system to possibly expose a bunch of other SCMI sensors maybe unsupported by this IIO driver but currently handled by other drivers, as an example on JUNO a number of temps/volts/currents sensors are exposed and handled by the SCMI hwmon driver. > > + > > + err = scmi_alloc_iiodev(dev, handle, sensor_info, > > + _iio_dev); > > + if (err < 0) { > > + dev_err(dev, > > + "failed to allocate IIO device for sensor %s: > > %d\n", > > + sensor_info->name, err); > > + return err; > > + } > > + > > + err = scmi_iio_buffers_setup(scmi_iio_dev); > > + if (err < 0) { > > + dev_err(dev, > > + "IIO buffer setup error at sensor %s: %d\n", > > + sensor_info->name, err); > > + return err; > > + } > > + > > + err = devm_iio_device_register(dev, scmi_iio_dev); > > + if (err) { > > + dev_err(dev, > > + "IIO device registration failed at sensor %s: > > %d\n", > > + sensor_info->name, err); > > + return err; > > + } > > + } > > + return err; > > +} > > + > > +static const struct scmi_device_id scmi_id_table[] = { > > + { SCMI_PROTOCOL_SENSOR, "iiodev" }, > > I'm curious on this. What actually causes a match on that > iiodev? From digging around the scmi core am I right in thinking > that this iiodev id needs to be explicitly listed? > > It would be good to include any changes needed there in this > series. > > > + {}, > > +}; > > + > > +MODULE_DEVICE_TABLE(scmi, scmi_id_table); > > + > > +static struct scmi_driver scmi_iiodev_driver = { > > + .name = "scmi-sensor-iiodev", > > + .probe = scmi_iio_dev_probe, > > + .id_table = scmi_id_table, > > +}; > > + > > +module_scmi_driver(scmi_iiodev_driver); > > + > > +MODULE_AUTHOR("Jyoti Bhayana "); > > +MODULE_DESCRIPTION("SCMI IIO Driver"); > > +MODULE_LICENSE("GPL v2"); >
Re: [PATCH 2/3] hid-sensor-common: Add relative sensitivity check
On Sun, 2021-01-31 at 11:26 +, Jonathan Cameron wrote: > On Fri, 29 Jan 2021 00:35:49 +0800 > "Ye, Xiang" wrote: > > > Hi Srinivas andd Jonathan > > > > Thanks for the review. > > > > On Sun, Jan 24, 2021 at 08:20:12AM -0800, Srinivas Pandruvada > > wrote: > > > On Sun, 2021-01-24 at 13:14 +, Jonathan Cameron wrote: > > > > On Wed, 20 Jan 2021 15:47:05 +0800 > > > > Ye Xiang wrote: > > > > > > > > > Some hid sensors may use relative sensitivity such as als > > > > > sensor. > > > > > This patch add relative sensitivity check for all hid- > > > > > sensors. > > > > > > > > > > Signed-off-by: Ye Xiang > > > > > --- > > > > > .../iio/common/hid-sensors/hid-sensor-attributes.c| 11 > > > > > ++- > > > > > include/linux/hid-sensor-ids.h| 1 + > > > > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor- > > > > > attributes.c > > > > > b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > > > > index d349ace2e33f..b685c292a179 100644 > > > > > --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > > > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > > > > @@ -480,7 +480,7 @@ int > > > > > hid_sensor_parse_common_attributes(struct > > > > > hid_sensor_hub_device *hsdev, > > > > > > > > > > /* > > > > >* Set Sensitivity field ids, when there is no > > > > > individualsha512sum > > > > > modifier, will > > > > > - * check absolute sensitivity of data field > > > > > + * check absolute sensitivity and relative sensitivity > > > > > of data > > > > > field > > > > >*/ > > > > > for (i = 0; i < sensitivity_addresses_len && st- > > > > > > sensitivity.index < 0; i++) { > > > > > sensor_hub_input_get_attribute_info(hsdev, > > > > > @@ -488,6 +488,15 @@ int > > > > > hid_sensor_parse_common_attributes(struct > > > > > hid_sensor_hub_device *hsdev, > > > > > HID_USAGE_SENSOR_DATA_MOD_CHANG > > > > > E_SENSIT > > > > > IVITY_ABS | > > > > > sensitivity_addresses[i > > > > > ], > > > > > >sensitivity); > > > > > + > > > > > + if (st->sensitivity.index >= 0) > > > > > + break; > > > > > + > > > > > + sensor_hub_input_get_attribute_info(hsdev, > > > > > + HID_FEATURE_REPORT, usage_id, > > > > > + HID_USAGE_SENSOR_DATA_MOD_CHANG > > > > > E_SENSIT > > > > > IVITY_REL_PCT | > > > > > + sensitivity_addresses[i > > > > > ], > > > > > + >sensitivity); > > > > > > > > We can't provide the value to userspace without reflecting the > > > > difference between > > > > the two ways of expressing it. > > > > > > > > It seems there are 3 ways sensitivity is expressed. > > > > 1. Raw value in same units as the measurement (easy one and > > > > what is > > > > currently reported) > > > > 2. Percentage of range - also relatively easy to transform into > > > > the > > > > same as 1. > > > > 3. Percentage of prior reading.. This one doesn't fit in any > > > > existing ABI, so > > > >unfortunately we'll have to invent something new along the > > > > lines > > > > of > > > >*_hysteresis_relative > > > > yes, the 3th version sensitivity (Percentage of prior reading) is > > what we > > are using for als sensor now. the 1th version sensitivity is common > > used > > by other hid sensors. Do you have suggestion or reference about > > how to add *_hysteresis_relative field to iio model? > > Follow through how elements of iio_chan_info_enum in > include/linux/iio/types.h are used and you should see how to add a > new > one (basically add an entry to that and also the string to the > right array in industrialio-core.c + document it in > Documentation/ABI/testing/sysfs-bus-iio. > > The issue with putting this in is we are going to be 'fixing' the ABI > for > that ALS sensor which is going to cause problems for any userspace > users > of that interface... I have no idea how commonly this is used, but it > is > possible we'll have to leave that one as incorrect :( Currently we don't have relative usage id treatment, So user space gets error in read/write for ALS. So I think it shouldn't be a problem. Thanks, Srinivas > > > > This is why it was not added before when I developed. But later > > > few > > > years back there was a patch to add this by one of our developer. > > > There > > > was some discussion, I thought it was decided it is OK to add. > > > > > > But I agree, we should add new ABI as you suggested. Now almost > > > every > > > laptop has HID sensors, better to address this. > > > > > > > I think the add relative hystersis patch should be separated into a > > independent > > patch series, for it's a independent function and need
Re: [RFC 08/20] mm: store completed TLB generation
On Sat, Jan 30, 2021 at 4:16 PM Nadav Amit wrote: > > From: Nadav Amit > > To detect deferred TLB flushes in fine granularity, we need to keep > track on the completed TLB flush generation for each mm. > > Add logic to track for each mm the tlb_gen_completed, which tracks the > completed TLB generation. It is the arch responsibility to call > mark_mm_tlb_gen_done() whenever a TLB flush is completed. > > Start the generation numbers from 1 instead of 0. This would allow later > to detect whether flushes of a certain generation were completed. Can you elaborate on how this helps? I think you should document that tlb_gen_completed only means that no outdated TLB entries will be observably used. In the x86 implementation it's possible for older TLB entries to still exist, unused, in TLBs of cpus running other mms. How does this work with arch_tlbbatch_flush()? > > Signed-off-by: Nadav Amit > Cc: Andrea Arcangeli > Cc: Andrew Morton > Cc: Andy Lutomirski > Cc: Dave Hansen > Cc: Peter Zijlstra > Cc: Thomas Gleixner > Cc: Will Deacon > Cc: Yu Zhao > Cc: Nick Piggin > Cc: x...@kernel.org > --- > arch/x86/mm/tlb.c | 10 ++ > include/asm-generic/tlb.h | 33 + > include/linux/mm_types.h | 15 ++- > 3 files changed, 57 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 7ab21430be41..d17b5575531e 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > #include "mm_internal.h" > > @@ -915,6 +916,9 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned > long start, > if (cpumask_any_but(mm_cpumask(mm), cpu) < nr_cpu_ids) > flush_tlb_others(mm_cpumask(mm), info); > > + /* Update the completed generation */ > + mark_mm_tlb_gen_done(mm, new_tlb_gen); > + > put_flush_tlb_info(); > put_cpu(); > } > @@ -1147,6 +1151,12 @@ void arch_tlbbatch_flush(struct > arch_tlbflush_unmap_batch *batch) > > cpumask_clear(>cpumask); > > + /* > +* We cannot call mark_mm_tlb_gen_done() since we do not know which > +* mm's should be flushed. This may lead to some unwarranted TLB > +* flushes, but not to correction problems. > +*/ > + > put_cpu(); > } > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 517c89398c83..427bfcc6cdec 100644 > --- a/include/asm-generic/tlb.h > +++ b/include/asm-generic/tlb.h > @@ -513,6 +513,39 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, > struct vm_area_struct *vm > } > #endif > > +#ifdef CONFIG_ARCH_HAS_TLB_GENERATIONS > + > +/* > + * Helper function to update a generation to have a new value, as long as new > + * value is greater or equal to gen. > + */ I read this a couple of times, and I don't understand it. How about: Helper function to atomically set *gen = max(*gen, new_gen) > +static inline void tlb_update_generation(atomic64_t *gen, u64 new_gen) > +{ > + u64 cur_gen = atomic64_read(gen); > + > + while (cur_gen < new_gen) { > + u64 old_gen = atomic64_cmpxchg(gen, cur_gen, new_gen); > + > + /* Check if we succeeded in the cmpxchg */ > + if (likely(cur_gen == old_gen)) > + break; > + > + cur_gen = old_gen; > + }; > +} > + > + > +static inline void mark_mm_tlb_gen_done(struct mm_struct *mm, u64 gen) > +{ > + /* > +* Update the completed generation to the new generation if the new > +* generation is greater than the previous one. > +*/ > + tlb_update_generation(>tlb_gen_completed, gen); > +} > + > +#endif /* CONFIG_ARCH_HAS_TLB_GENERATIONS */ > + > /* > * tlb_flush_{pte|pmd|pud|p4d}_range() adjust the tlb->start and tlb->end, > * and set corresponding cleared_*. > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 2035ac319c2b..8a5eb4bfac59 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -571,6 +571,13 @@ struct mm_struct { > * This is not used on Xen PV. > */ > atomic64_t tlb_gen; > + > + /* > +* TLB generation which is guarnateed to be flushed, including guaranteed > +* all the PTE changes that were performed before tlb_gen was > +* incremented. > +*/ I will defer judgment to future patches before I believe that this isn't racy :) > + atomic64_t tlb_gen_completed; > #endif > } __randomize_layout; > > @@ -690,7 +697,13 @@ static inline bool mm_tlb_flush_nested(struct mm_struct > *mm) > #ifdef CONFIG_ARCH_HAS_TLB_GENERATIONS > static inline void init_mm_tlb_gen(struct mm_struct *mm) > { > - atomic64_set(>tlb_gen, 0); > + /* > +* Start from generation of 1, so default generation 0 will be > +
Re: [RFC PATCH v4 1/1] iio/scmi: Adding support for IIO SCMI Based Sensors
Hi Jyoti a few remarks down below, but beside those, while testing this series on a JUNO board (so lacking any GYRO/ACCEL sensor, a limited test case for now) I spotted that you missed in this series to add a matching device to the SCMI core. (as spotted by Jonathan too in his v4 review) In other words in your actual codebase you should have something like this, somewhere: diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 3f5e8a42373e..cd6e38a8d8d2 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -743,7 +743,7 @@ static struct scmi_prot_devnames devnames[] = { { SCMI_PROTOCOL_SYSTEM, { "syspower" },}, { SCMI_PROTOCOL_PERF, { "cpufreq" },}, { SCMI_PROTOCOL_CLOCK, { "clocks" },}, - { SCMI_PROTOCOL_SENSOR, { "hwmon" },}, + { SCMI_PROTOCOL_SENSOR, { "hwmon", "iiodev" },}, { SCMI_PROTOCOL_RESET, { "reset" },}, { SCMI_PROTOCOL_VOLTAGE, { "regulator" },}, }; given that without this there's no chance the SCMI driver could be ever matched and probed by the SCMI core at first. So please commmit the above as a separate patch in this series, to have it working too in the upstream. Other comments below Thanks Cristian On Fri, Jan 29, 2021 at 10:18:18PM +, Jyoti Bhayana wrote: > This change provides ARM SCMI Protocol based IIO device. > This driver provides support for Accelerometer and Gyroscope using > SCMI Sensor Protocol extensions added in the SCMIv3.0 ARM specification > > Signed-off-by: Jyoti Bhayana > --- > MAINTAINERS| 6 + > drivers/iio/common/Kconfig | 1 + > drivers/iio/common/Makefile| 1 + > drivers/iio/common/scmi_sensors/Kconfig| 18 + > drivers/iio/common/scmi_sensors/Makefile | 5 + > drivers/iio/common/scmi_sensors/scmi_iio.c | 742 + > 6 files changed, 773 insertions(+) > create mode 100644 drivers/iio/common/scmi_sensors/Kconfig > create mode 100644 drivers/iio/common/scmi_sensors/Makefile > create mode 100644 drivers/iio/common/scmi_sensors/scmi_iio.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index b516bb34a8d5..ccf37d43ab41 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -8567,6 +8567,12 @@ S: Maintained > F: Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt > F: drivers/iio/multiplexer/iio-mux.c > > +IIO SCMI BASED DRIVER > +M: Jyoti Bhayana > +L: linux-...@vger.kernel.org > +S: Maintained > +F: drivers/iio/common/scmi_sensors/scmi_iio.c > + > IIO SUBSYSTEM AND DRIVERS > M: Jonathan Cameron > R: Lars-Peter Clausen > diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig > index 2b9ee9161abd..0334b4954773 100644 > --- a/drivers/iio/common/Kconfig > +++ b/drivers/iio/common/Kconfig > @@ -6,5 +6,6 @@ > source "drivers/iio/common/cros_ec_sensors/Kconfig" > source "drivers/iio/common/hid-sensors/Kconfig" > source "drivers/iio/common/ms_sensors/Kconfig" > +source "drivers/iio/common/scmi_sensors/Kconfig" > source "drivers/iio/common/ssp_sensors/Kconfig" > source "drivers/iio/common/st_sensors/Kconfig" > diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile > index 4bc30bb548e2..fad40e1e1718 100644 > --- a/drivers/iio/common/Makefile > +++ b/drivers/iio/common/Makefile > @@ -11,5 +11,6 @@ > obj-y += cros_ec_sensors/ > obj-y += hid-sensors/ > obj-y += ms_sensors/ > +obj-y += scmi_sensors/ > obj-y += ssp_sensors/ > obj-y += st_sensors/ > diff --git a/drivers/iio/common/scmi_sensors/Kconfig > b/drivers/iio/common/scmi_sensors/Kconfig > new file mode 100644 > index ..67e084cbb1ab > --- /dev/null > +++ b/drivers/iio/common/scmi_sensors/Kconfig > @@ -0,0 +1,18 @@ > +# > +# IIO over SCMI > +# > +# When adding new entries keep the list in alphabetical order > + > +menu "IIO SCMI Sensors" > + > +config IIO_SCMI > + tristate "IIO SCMI" > +depends on ARM_SCMI_PROTOCOL > +select IIO_BUFFER > +select IIO_KFIFO_BUF > + help > + Say yes here to build support for IIO SCMI Driver. > + This provides ARM SCMI Protocol based IIO device. > + This driver provides support for accelerometer and gyroscope > + sensors available on SCMI based platforms. > +endmenu > diff --git a/drivers/iio/common/scmi_sensors/Makefile > b/drivers/iio/common/scmi_sensors/Makefile > new file mode 100644 > index ..f13140a2575a > --- /dev/null > +++ b/drivers/iio/common/scmi_sensors/Makefile > @@ -0,0 +1,5 @@ > +# SPDX - License - Identifier : GPL - 2.0 - only > +# > +# Makefile for the IIO over SCMI > +# > +obj-$(CONFIG_IIO_SCMI) += scmi_iio.o > diff --git a/drivers/iio/common/scmi_sensors/scmi_iio.c > b/drivers/iio/common/scmi_sensors/scmi_iio.c > new file mode 100644 > index ..331ffaffd06f > --- /dev/null > +++ b/drivers/iio/common/scmi_sensors/scmi_iio.c > @@ -0,0 +1,742 @@ > +//
Re: [PATCH 2/2] security.capability: fix conversions on getxattr
"Serge E. Hallyn" writes: > On Fri, Jan 29, 2021 at 04:55:29PM -0600, Eric W. Biederman wrote: >> "Serge E. Hallyn" writes: >> >> > On Thu, Jan 28, 2021 at 02:19:13PM -0600, Eric W. Biederman wrote: >> >> "Serge E. Hallyn" writes: >> >> >> >> > On Tue, Jan 19, 2021 at 07:34:49PM -0600, Eric W. Biederman wrote: >> >> >> Miklos Szeredi writes: >> >> >> >> >> >> > If a capability is stored on disk in v2 format >> >> >> > cap_inode_getsecurity() will >> >> >> > currently return in v2 format unconditionally. >> >> >> > >> >> >> > This is wrong: v2 cap should be equivalent to a v3 cap with zero >> >> >> > rootid, >> >> >> > and so the same conversions performed on it. >> >> >> > >> >> >> > If the rootid cannot be mapped v3 is returned unconverted. Fix this >> >> >> > so >> >> >> > that both v2 and v3 return -EOVERFLOW if the rootid (or the owner of >> >> >> > the fs >> >> >> > user namespace in case of v2) cannot be mapped in the current user >> >> >> > namespace. >> >> >> >> >> >> This looks like a good cleanup. >> >> > >> >> > Sorry, I'm not following. Why is this a good cleanup? Why should >> >> > the xattr be shown as faked v3 in this case? >> >> >> >> If the reader is in _user_ns. If the filesystem was mounted in a >> >> user namespace. Then the reader looses the information that the >> > >> > Can you be more precise about "filesystem was mounted in a user namespace"? >> > Is this a FUSE thing, the fs is marked as being mounted in a non-init >> > userns? >> > If that's a possible case, then yes that must be represented as v3. Using >> > is_v2header() may be the simpler way to check for that, but the more >> > accurate >> > check would be "is it v2 header and mounted by init_user_ns". >> >> I think the filesystems current relevant are fuse,overlayfs,ramfs,tmpfs. >> >> > Basically yes, in as many cases as possible we want to just give a v2 >> > cap because more userspace knows what to do with that, but a >> > non-init-userns >> > mounted fs which provides a v2 fscap should have it represented as v3 cap >> > with rootid being the kuid that owns the userns. >> >> That is the case we that is being fixed in the patch. >> >> > Or am I still thinking wrongly? Wouldn't be entirely surprised :) >> >> No you got it. > > So then can we make faking a v3 gated on whether > sb->s_user_ns != _user_ns ? Sort of. What Miklos's patch implements is always treating a v2 cap xattr on disk as v3 internally. > if (is_v2header((size_t) ret, cap)) { > root = 0; > } else if (is_v3header((size_t) ret, cap)) { > nscap = (struct vfs_ns_cap_data *) tmpbuf; > root = le32_to_cpu(nscap->rootid); > } else { > size = -EINVAL; > goto out_free; > } Then v3 is returned if: > /* If the root kuid maps to a valid uid in current ns, then return >* this as a nscap. */ > mappedroot = from_kuid(current_user_ns(), kroot); > if (mappedroot != (uid_t)-1 && mappedroot != (uid_t)0) { After that we verify that the fs capability can be seen by the caller as a v2 cap xattr with: > > if (!rootid_owns_currentns(kroot)) { > > size = -EOVERFLOW; > > goto out_free; Anything that passes that test and does not encounter a memory allocation error is returned as a v2. ... Which in practice does mean that if sb->s_user_ns != _user_ns, then mappedroot != 0, and is returned as a v3. The rest of the logic takes care of all of the other crazy silly combinations. Like a user namespace that identity maps uid 0, and then mounts a filesystem. Eric
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
Linus Torvalds writes: > On Sun, Jan 31, 2021 at 10:54 AM Yuxuan Shui wrote: >> >> But renaming the definition in x86 is not enough, as TIF_SINGLESTEP is >> set in current_thread_info()->flags, and the same commit has removed the >> code that checks those flags. We have to also migrate TIF_SINGLESTEP from >> thread info flags to syscall work flags, to make the whole thing work again. > > Ok, so I now have the first fix merged, but what's the next step here? > > As you say, the x86 ARCH_SYSCALL_EXIT_WORK is now entirely unused. > > It's called ARCH_SYSCALL_WORK_EXIT these days, but that's for the > SYSCALL_WORK_SYSCALL_xyz flags, not for the TIF_xyz ones. > > Revert? Or does somebody have a fix patch? I think we should migrate TIF_SINGLESTEP to a SYSCALL_WORK flag as that is just a simple refactor. I can get a patch to you and Thomas during the first part of the week, for the next -rc. I will also review the x86 version of ARCH_SYSCALL_EXIT WORK to make sure i didn't miss anything else. Reverting would be slightly be annoying as it requires reverting syscall user dispatch as well. -- Gabriel Krisman Bertazi
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
On Sun, Jan 31, 2021 at 10:54 AM Yuxuan Shui wrote: > > But renaming the definition in x86 is not enough, as TIF_SINGLESTEP is > set in current_thread_info()->flags, and the same commit has removed the > code that checks those flags. We have to also migrate TIF_SINGLESTEP from > thread info flags to syscall work flags, to make the whole thing work again. Ok, so I now have the first fix merged, but what's the next step here? As you say, the x86 ARCH_SYSCALL_EXIT_WORK is now entirely unused. It's called ARCH_SYSCALL_WORK_EXIT these days, but that's for the SYSCALL_WORK_SYSCALL_xyz flags, not for the TIF_xyz ones. Revert? Or does somebody have a fix patch? Linus
Re: [v2] drm/msm/disp/dpu1: turn off vblank irqs aggressively in dpu driver
On Fri, Dec 18, 2020 at 2:27 AM Kalyan Thota wrote: > > Set the flag vblank_disable_immediate = true to turn off vblank irqs > immediately as soon as drm_vblank_put is requested so that there are > no irqs triggered during idle state. This will reduce cpu wakeups > and help in power saving. > > To enable vblank_disable_immediate flag the underlying KMS driver > needs to support high precision vblank timestamping and also a > reliable way of providing vblank counter which is incrementing > at the leading edge of vblank. > > This patch also brings in changes to support vblank_disable_immediate > requirement in dpu driver. > > Changes in v1: > - Specify reason to add vblank timestamp support. (Rob) > - Add changes to provide vblank counter from dpu driver. > > Signed-off-by: Kalyan Thota This seems to be triggering: [ +0.032668] [ cut here ] [ +0.004759] msm ae0.mdss: drm_WARN_ON_ONCE(cur_vblank != vblank->last) [ +0.24] WARNING: CPU: 0 PID: 362 at drivers/gpu/drm/drm_vblank.c:354 drm_update_vblank_count+0x1e4/0x258 [ +0.017154] Modules linked in: joydev [ +0.003784] CPU: 0 PID: 362 Comm: frecon Not tainted 5.11.0-rc5-00037-g33d3504871dd #2 [ +0.008135] Hardware name: Google Lazor (rev1 - 2) with LTE (DT) [ +0.006167] pstate: 60400089 (nZCv daIf +PAN -UAO -TCO BTYPE=--) [ +0.006169] pc : drm_update_vblank_count+0x1e4/0x258 [ +0.005105] lr : drm_update_vblank_count+0x1e4/0x258 [ +0.005106] sp : ffc010003b70 [ +0.003409] x29: ffc010003b70 x28: ff80855d9d98 [ +0.005466] x27: x26: 00fe502a [ +0.005458] x25: 0001 x24: 0001 [ +0.005466] x23: 0001 x22: ff808561ce80 [ +0.005465] x21: x20: [ +0.005468] x19: ff80850d6800 x18: [ +0.005466] x17: x16: [ +0.005465] x15: 000a x14: 263b [ +0.005466] x13: 0006 x12: [ +0.005465] x11: 0010 x10: ffc090003797 [ +0.005466] x9 : ffed200e2a8c x8 : [ +0.005466] x7 : x6 : ffed213b2b51 [ +0.005465] x5 : c000dfff x4 : ffed21218048 [ +0.005465] x3 : x2 : [ +0.005465] x1 : x0 : [ +0.005466] Call trace: [ +0.002520] drm_update_vblank_count+0x1e4/0x258 [ +0.004748] drm_handle_vblank+0xd0/0x35c [ +0.004130] drm_crtc_handle_vblank+0x24/0x30 [ +0.004487] dpu_crtc_vblank_callback+0x3c/0xc4 [ +0.004662] dpu_encoder_vblank_callback+0x70/0xc4 [ +0.004931] dpu_encoder_phys_vid_vblank_irq+0x50/0x12c [ +0.005378] dpu_core_irq_callback_handler+0xf4/0xfc [ +0.005107] dpu_hw_intr_dispatch_irq+0x100/0x120 [ +0.004834] dpu_core_irq+0x44/0x5c [ +0.003597] dpu_irq+0x1c/0x28 [ +0.003141] msm_irq+0x34/0x40 [ +0.003153] __handle_irq_event_percpu+0xfc/0x254 [ +0.004838] handle_irq_event_percpu+0x3c/0x94 [ +0.004574] handle_irq_event+0x54/0x98 [ +0.003944] handle_level_irq+0xa0/0xd0 [ +0.003943] generic_handle_irq+0x30/0x48 [ +0.004131] dpu_mdss_irq+0xe4/0x118 [ +0.003684] generic_handle_irq+0x30/0x48 [ +0.004127] __handle_domain_irq+0xa8/0xac [ +0.004215] gic_handle_irq+0xdc/0x150 [ +0.003856] el1_irq+0xb4/0x180 [ +0.003237] dpu_encoder_vsync_time+0x78/0x230 [ +0.004574] dpu_encoder_kickoff+0x190/0x354 [ +0.004386] dpu_crtc_commit_kickoff+0x194/0x1a0 [ +0.004748] dpu_kms_flush_commit+0xf4/0x108 [ +0.004390] msm_atomic_commit_tail+0x2e8/0x384 [ +0.004661] commit_tail+0x80/0x108 [ +0.003588] drm_atomic_helper_commit+0x118/0x11c [ +0.004834] drm_atomic_commit+0x58/0x68 [ +0.004033] drm_atomic_helper_set_config+0x70/0x9c [ +0.005018] drm_mode_setcrtc+0x390/0x584 [ +0.004131] drm_ioctl_kernel+0xc8/0x11c [ +0.004035] drm_ioctl+0x2f8/0x34c [ +0.003500] drm_compat_ioctl+0x48/0xe8 [ +0.003945] __arm64_compat_sys_ioctl+0xe8/0x104 [ +0.004750] el0_svc_common.constprop.0+0x114/0x188 [ +0.005019] do_el0_svc_compat+0x28/0x38 [ +0.004031] el0_svc_compat+0x20/0x30 [ +0.003772] el0_sync_compat_handler+0x104/0x18c [ +0.004749] el0_sync_compat+0x178/0x180 [ +0.004034] ---[ end trace 2959d178e74f2555 ]--- BR, -R > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 80 > ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 30 > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 11 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 1 + > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 17 + > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 5 ++ > 6 files changed, 144 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index d4662e8..9a80981 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -65,6 +65,83 @@ static void
Re: [REGRESSION] x86/entry: TIF_SINGLESTEP handling is still broken
I didn't understand Kyle's point at first, so I asked for clarification and will record my understanding below for posterity. ARCH_SYSCALL_EXIT_WORK was a flag that was checked by various functions (via SYSCALL_EXIT_WORK) before calling syscall_exit_work, which is what reports single steps. This flag was supposed to be overridden by architecture specific definitions. And indeed, x86 overrides it, to TIF_SINGLESTEP. However, commit 2991552447707d791d9d81a5dc161f9e9e90b163 renamed ARCH_SYSCALL_EXIT_WORK to ARCH_SYSCALL_WORK_EXIT, thus x86's definition no longer override it. Looks like there was an oversight the definition in x86 wasn't updated. But renaming the definition in x86 is not enough, as TIF_SINGLESTEP is set in current_thread_info()->flags, and the same commit has removed the code that checks those flags. We have to also migrate TIF_SINGLESTEP from thread info flags to syscall work flags, to make the whole thing work again.
Re: [GIT PULL] Please pull NFS client bugfixes for 5.11
On Sun, Jan 31, 2021 at 8:59 AM Trond Myklebust wrote: > > git://git.linux-nfs.org/projects/trondmy/linux-nfs.git tags/nfs-for-5.11-3 Merged. However, it looks like you won't get a pr-tracker-bot reply because I'm not seeing this email on lore. So I'm doing these manual replies for now, it looks like the mailing list is not doing great. Linus
Re: [GIT PULL] arm64 fixes for 5.11-rc6
On Fri, Jan 29, 2021 at 02:09:05PM -0800, Linus Torvalds wrote: > On Fri, Jan 29, 2021 at 11:03 AM Catalin Marinas > wrote: > > > > arm64 fixes: > > > > - Fix the virt_addr_valid() returning true for < PAGE_OFFSET addresses. > > That's a really odd fix. > > It went from an incorrect bitwise operation (masking) to an _odd_ > bitwise operation (xor). > > Yes, PAGE_OFFSET has the bit pattern of all upper bits set, so "(addr > ^ PAGE_OFFSET)" by definition reverses the upper bits - and for a > valid case turns them to zero. > > But isn't the *logical* thing to do to use a subtract instead? For the > valid cases, the two do the same thing (clear the upper bits), but > just conceptually, isn't the operation that you actually want to do > "(addr - PAGE_OFFSET)"? > > IOW, why is it using that odd xor pattern that doesn't make much > sense? I believe it _works_, but it looks very strange to me. This macro used to test a single bit and it evolved into a bitmask. So, yes, basically what we need is: #define __is_lm_address(addr) ((u64)(addr) >= PAGE_OFFSET && \ (u64)(addr) < PAGE_END) I wasn't sure whether the code generation with two comparisons is similar to the xor variant but the compiler should probably be smart enough to use CMP and CCMP. In the grand scheme, it probably doesn't even matter. Unless I miss something, I don't see any overflow issues even if we do (((u64)addr - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)). We can backport the fix already upstream and clean-up the code in mainline going forward (after some sanity check on the code generation). It would be easier to parse in the future. > Also, shouldn't _lm_to_phys() do the same? It does that "mask upper > bits" too that was problematic in __is_lm_address(). Again, shouldn't > that logically be a subtract op? Yes, that's similar and a subtract should do. -- Catalin
Re: [PATCH 2/2] vdpa/mlx5: Restore the hardware used index after change map
On Fri, Jan 29, 2021 at 11:49:45AM +0800, Jason Wang wrote: > > On 2021/1/28 下午9:41, Eli Cohen wrote: > > When a change of memory map occurs, the hardware resources are destroyed > > and then re-created again with the new memory map. In such case, we need > > to restore the hardware available and used indices. The driver failed to > > restore the used index which is added here. > > > > Fixes 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices") > > Signed-off-by: Eli Cohen > > > A question. Does this mean after a vq is suspended, the hw used index is not > equal to vq used index? Surely there is just one "Used index" for a VQ. What I was trying to say is that after the VQ is suspended, I read the used index by querying the hardware. The read result is the used index that the hardware wrote to memory. After the I create the new hardware object, I need to tell it what is the used index (and the available index) as a way to sync it with the existing VQ. This sync is especially important when a change of map occurs while the VQ was already used (hence the indices are likely to be non zero). This can be triggered by hot adding memory after the VQs have been used. > > Thanks > > > > --- > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > index 549ded074ff3..3fc8588cecae 100644 > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > @@ -87,6 +87,7 @@ struct mlx5_vq_restore_info { > > u64 device_addr; > > u64 driver_addr; > > u16 avail_index; > > + u16 used_index; > > bool ready; > > struct vdpa_callback cb; > > bool restore; > > @@ -121,6 +122,7 @@ struct mlx5_vdpa_virtqueue { > > u32 virtq_id; > > struct mlx5_vdpa_net *ndev; > > u16 avail_idx; > > + u16 used_idx; > > int fw_state; > > /* keep last in the struct */ > > @@ -804,6 +806,7 @@ static int create_virtqueue(struct mlx5_vdpa_net *ndev, > > struct mlx5_vdpa_virtque > > obj_context = MLX5_ADDR_OF(create_virtio_net_q_in, in, obj_context); > > MLX5_SET(virtio_net_q_object, obj_context, hw_available_index, > > mvq->avail_idx); > > + MLX5_SET(virtio_net_q_object, obj_context, hw_used_index, > > mvq->used_idx); > > MLX5_SET(virtio_net_q_object, obj_context, queue_feature_bit_mask_12_3, > > get_features_12_3(ndev->mvdev.actual_features)); > > vq_ctx = MLX5_ADDR_OF(virtio_net_q_object, obj_context, > > virtio_q_context); > > @@ -1022,6 +1025,7 @@ static int connect_qps(struct mlx5_vdpa_net *ndev, > > struct mlx5_vdpa_virtqueue *m > > struct mlx5_virtq_attr { > > u8 state; > > u16 available_index; > > + u16 used_index; > > }; > > static int query_virtqueue(struct mlx5_vdpa_net *ndev, struct > > mlx5_vdpa_virtqueue *mvq, > > @@ -1052,6 +1056,7 @@ static int query_virtqueue(struct mlx5_vdpa_net > > *ndev, struct mlx5_vdpa_virtqueu > > memset(attr, 0, sizeof(*attr)); > > attr->state = MLX5_GET(virtio_net_q_object, obj_context, state); > > attr->available_index = MLX5_GET(virtio_net_q_object, obj_context, > > hw_available_index); > > + attr->used_index = MLX5_GET(virtio_net_q_object, obj_context, > > hw_used_index); > > kfree(out); > > return 0; > > @@ -1602,6 +1607,7 @@ static int save_channel_info(struct mlx5_vdpa_net > > *ndev, struct mlx5_vdpa_virtqu > > return err; > > ri->avail_index = attr.available_index; > > + ri->used_index = attr.used_index; > > ri->ready = mvq->ready; > > ri->num_ent = mvq->num_ent; > > ri->desc_addr = mvq->desc_addr; > > @@ -1646,6 +1652,7 @@ static void restore_channels_info(struct > > mlx5_vdpa_net *ndev) > > continue; > > mvq->avail_idx = ri->avail_index; > > + mvq->used_idx = ri->used_index; > > mvq->ready = ri->ready; > > mvq->num_ent = ri->num_ent; > > mvq->desc_addr = ri->desc_addr; >
Re: [PATCH 3/3] power: supply: max8997_charger: Switch to new binding
On Sat, Jan 30, 2021 at 05:30:14PM +, Timon Baetz wrote: > Get regulator from parent device's node and extcon by name. > > Signed-off-by: Timon Baetz > --- > drivers/power/supply/max8997_charger.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/power/supply/max8997_charger.c > b/drivers/power/supply/max8997_charger.c > index 321bd6b8ee41..625d8cc4312a 100644 > --- a/drivers/power/supply/max8997_charger.c > +++ b/drivers/power/supply/max8997_charger.c > @@ -168,6 +168,7 @@ static int max8997_battery_probe(struct platform_device > *pdev) > int ret = 0; > struct charger_data *charger; > struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent); > + struct device_node *np = pdev->dev.of_node; > struct i2c_client *i2c = iodev->i2c; > struct max8997_platform_data *pdata = iodev->pdata; > struct power_supply_config psy_cfg = {}; > @@ -237,20 +238,23 @@ static int max8997_battery_probe(struct platform_device > *pdev) > return PTR_ERR(charger->battery); > } > > + // grab regulator from parent device's node > + pdev->dev.of_node = iodev->dev->of_node; > charger->reg = devm_regulator_get_optional(>dev, "charger"); > + pdev->dev.of_node = np; I think the device does not have its own node anymore. Or did I miss something? > if (IS_ERR(charger->reg)) { > if (PTR_ERR(charger->reg) == -EPROBE_DEFER) > return -EPROBE_DEFER; > dev_info(>dev, "couldn't get charger regulator\n"); > } > - charger->edev = extcon_get_edev_by_phandle(>dev, 0); > - if (IS_ERR(charger->edev)) { > - if (PTR_ERR(charger->edev) == -EPROBE_DEFER) > + charger->edev = extcon_get_extcon_dev("max8997-muic"); > + if (IS_ERR_OR_NULL(charger->edev)) { > + if (!charger->edev) Isn't NULL returned when there is simply no extcon? It's different than deferred probe. Returning here EPROBE_DEFER might lead to infinite probe tries (on every new device probe) instead of just failing it. Best regards, Krzysztof > return -EPROBE_DEFER; > dev_info(charger->dev, "couldn't get extcon device\n"); > } > > - if (!IS_ERR(charger->reg) && !IS_ERR(charger->edev)) { > + if (!IS_ERR(charger->reg) && !IS_ERR_OR_NULL(charger->edev)) { > INIT_WORK(>extcon_work, > max8997_battery_extcon_evt_worker); > ret = devm_add_action(>dev, > max8997_battery_extcon_evt_stop_work, charger); > if (ret) { > -- > 2.25.1 > >
Re: [bug] 5.11-rc5 brought page allocation failure issue [ttm][amdgpu]
Am 31.01.21 um 02:03 schrieb David Rientjes: On Sat, 30 Jan 2021, David Rientjes wrote: On Sun, 31 Jan 2021, Mikhail Gavrilov wrote: The 5.11-rc5 (git 76c057c84d28) brought a new issue. Now the kernel log is flooded with the message "page allocation failure". Trace: msedge:cs0: page allocation failure: order:10, Order-10, wow! ttm_pool_alloc() will start at order-10 and back off trying smaller orders if necessary. This is a regression introduced in commit bf9eee249ac2032521677dd74e31ede5429afbc0 Author: Christian König Date: Wed Jan 13 14:02:04 2021 +0100 drm/ttm: stop using GFP_TRANSHUGE_LIGHT Namely, it removed the __GFP_NOWARN that we otherwise require. I'll send a patch in reply. Looks like Michel Dänzer already sent a patch that should fix this: https://lore.kernel.org/lkml/20210128095346.2421-1-mic...@daenzer.net/ Yeah, known issue. I already pushed Michel's fix to drm-misc-fixes. Should land in the next -rc by the weekend. Regards, Christian.
[PATCH 04/13] staging: most: Switch from strlcpy to strscpy
strlcpy is marked as deprecated in Documentation/process/deprecated.rst, and there is no functional difference when the caller expects truncation (when not checking the return value). strscpy is relatively better as it also avoids scanning the whole source string. This silences the related checkpatch warnings from: 5dbdb2d87c29 ("checkpatch: prefer strscpy to strlcpy") Signed-off-by: Kumar Kartikeya Dwivedi --- drivers/staging/most/sound/sound.c | 2 +- drivers/staging/most/video/video.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c index 3a1a59058..c4287994b 100644 --- a/drivers/staging/most/sound/sound.c +++ b/drivers/staging/most/sound/sound.c @@ -525,7 +525,7 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id, pr_err("Incompatible channel type\n"); return -EINVAL; } - strlcpy(arg_list_cpy, arg_list, STRING_SIZE); + strscpy(arg_list_cpy, arg_list, STRING_SIZE); ret = split_arg_list(arg_list_cpy, _num, _res); if (ret < 0) return ret; diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c index 829df899b..90933d78c 100644 --- a/drivers/staging/most/video/video.c +++ b/drivers/staging/most/video/video.c @@ -245,8 +245,8 @@ static int vidioc_querycap(struct file *file, void *priv, struct comp_fh *fh = priv; struct most_video_dev *mdev = fh->mdev; - strlcpy(cap->driver, "v4l2_component", sizeof(cap->driver)); - strlcpy(cap->card, "MOST", sizeof(cap->card)); + strscpy(cap->driver, "v4l2_component", sizeof(cap->driver)); + strscpy(cap->card, "MOST", sizeof(cap->card)); snprintf(cap->bus_info, sizeof(cap->bus_info), "%s", mdev->iface->description); return 0; @@ -483,7 +483,7 @@ static int comp_probe_channel(struct most_interface *iface, int channel_idx, mdev->v4l2_dev.release = comp_v4l2_dev_release; /* Create the v4l2_device */ - strlcpy(mdev->v4l2_dev.name, name, sizeof(mdev->v4l2_dev.name)); + strscpy(mdev->v4l2_dev.name, name, sizeof(mdev->v4l2_dev.name)); ret = v4l2_device_register(NULL, >v4l2_dev); if (ret) { pr_err("v4l2_device_register() failed\n"); -- 2.29.2