Re: [PATCH] random32: Restore __latent_entropy attribute on net_rand_state
On Tue, Oct 06, 2020 at 04:28:09AM +0200, Willy Tarreau wrote:
> Hi Kees,
>
> On Mon, Oct 05, 2020 at 07:12:29PM -0700, Kees Cook wrote:
> > On Fri, Oct 02, 2020 at 05:16:11PM +0200, Thibaut Sautereau wrote:
> > > From: Thibaut Sautereau
> > >
> > > Commit f227e3ec3b5c ("random32: update the net random state on interrupt
> > > and activity") broke compilation and was temporarily fixed by Linus in
> > > 83bdc7275e62 ("random32: remove net_rand_state from the latent entropy
> > > gcc plugin") by entirely moving net_rand_state out of the things handled
> > > by the latent_entropy GCC plugin.
> > >
> > > From what I understand when reading the plugin code, using the
> > > __latent_entropy attribute on a declaration was the wrong part and
> > > simply keeping the __latent_entropy attribute on the variable definition
> > > was the correct fix.
> > >
> > > Fixes: 83bdc7275e62 ("random32: remove net_rand_state from the latent
> > > entropy gcc plugin")
> > > Cc: Linus Torvalds
> > > Cc: Willy Tarreau
> > > Cc: Emese Revfy
> > > Signed-off-by: Thibaut Sautereau
> >
> > Yes, that looks correct. Thank you!
> >
> > Acked-by: Kees Cook
> >
> > I'm not sure the best tree for this. Ted, Andrew, Linus? I'll take it
> > via my gcc plugin tree if no one else takes it. :)
>
> It was already merged as commit 09a6b0bc3be79 and queued for -stable.
Ah, perfect! Thanks.
--
Kees Cook
Re: [PATCH 5.4 00/57] 5.4.70-rc1 review
On Mon, 5 Oct 2020 at 20:59, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 5.4.70 release. > There are 57 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 07 Oct 2020 14:20:55 +. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.70-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-5.4.y > and the diffstat can be found below. > > thanks, > > greg k-h > Results from Linaro’s test farm. No regressions on arm64, arm, x86_64, and i386. Tested-by: Linux Kernel Functional Testing Summary kernel: 5.4.70-rc1 git repo: ['https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git', 'https://gitlab.com/Linaro/lkft/mirrors/stable/linux-stable-rc'] git branch: linux-5.4.y git commit: 7b199c4db17f19594dcf4d24cc26c8ddff8443da git describe: v5.4.69-58-g7b199c4db17f Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-5.4.y/build/v5.4.69-58-g7b199c4db17f No regressions (compared to build v5.4.69) No fixes (compared to build v5.4.69) Ran 34627 total tests in the following environments and test suites. Environments -- - dragonboard-410c - hi6220-hikey - i386 - juno-r2 - juno-r2-compat - juno-r2-kasan - qemu_arm - qemu_arm64 - qemu_i386 - qemu_x86_64 - x15 - x86 - x86-kasan Test Suites --- * build * install-android-platform-tools-r2600 * kselftest * libhugetlbfs * linux-log-parser * ltp-cap_bounds-tests * ltp-commands-tests * ltp-containers-tests * ltp-controllers-tests * ltp-cpuhotplug-tests * ltp-crypto-tests * ltp-cve-tests * ltp-dio-tests * ltp-fcntl-locktests-tests * ltp-filecaps-tests * ltp-fs-tests * ltp-fs_bind-tests * ltp-fs_perms_simple-tests * ltp-fsx-tests * ltp-hugetlb-tests * ltp-io-tests * ltp-math-tests * ltp-nptl-tests * ltp-pty-tests * ltp-sched-tests * ltp-securebits-tests * ltp-syscalls-tests * ltp-tracing-tests * perf * v4l2-compliance * ltp-mm-tests * network-basic-tests * ltp-ipc-tests * ltp-open-posix-tests * kselftest-vsyscall-mode-native * kselftest-vsyscall-mode-none * ssuite -- Linaro LKFT https://lkft.linaro.org
Re: [PATCH v2 00/29] [Set 1,2,3] Rid W=1 warnings in Wireless
Lee Jones writes:
> On Thu, 10 Sep 2020, Lee Jones wrote:
>
>> This is a rebased/re-worked set of patches which have been
>> previously posted to the mailing list(s).
>>
>> This set is part of a larger effort attempting to clean-up W=1
>> kernel builds, which are currently overwhelmingly riddled with
>> niggly little warnings.
>>
>> There are quite a few W=1 warnings in the Wireless. My plan
>> is to work through all of them over the next few weeks.
>> Hopefully it won't be too long before drivers/net/wireless
>> builds clean with W=1 enabled.
>>
>> Lee Jones (29):
>> iwlwifi: dvm: Demote non-compliant kernel-doc headers
>> iwlwifi: rs: Demote non-compliant kernel-doc headers
>> iwlwifi: dvm: tx: Demote non-compliant kernel-doc headers
>> iwlwifi: dvm: lib: Demote non-compliant kernel-doc headers
>> iwlwifi: calib: Demote seemingly unintentional kerneldoc header
>> wil6210: Fix a couple of formatting issues in 'wil6210_debugfs_init'
>> iwlwifi: dvm: sta: Demote a bunch of nonconformant kernel-doc headers
>> iwlwifi: mvm: ops: Remove unused static struct 'iwl_mvm_debug_names'
>> iwlwifi: dvm: Demote a couple of nonconformant kernel-doc headers
>> iwlwifi: mvm: utils: Fix some doc-rot
>> iwlwifi: dvm: scan: Demote a few nonconformant kernel-doc headers
>> iwlwifi: dvm: rxon: Demote non-conformant kernel-doc headers
>> iwlwifi: mvm: tx: Demote misuse of kernel-doc headers
>> iwlwifi: dvm: devices: Fix function documentation formatting issues
>> iwlwifi: iwl-drv: Provide descriptions debugfs dentries
>> wil6210: wmi: Fix formatting and demote non-conforming function
>> headers
>> wil6210: interrupt: Demote comment header which is clearly not
>> kernel-doc
>> wil6210: txrx: Demote obvious abuse of kernel-doc
>> wil6210: txrx_edma: Demote comments which are clearly not kernel-doc
>> wil6210: pmc: Demote a few nonconformant kernel-doc function headers
>> wil6210: wil_platform: Demote kernel-doc header to standard comment
>> block
>> wil6210: wmi: Correct misnamed function parameter 'ptr_'
>> ath6kl: wmi: Remove unused variable 'rate'
>> ath9k: ar9002_initvals: Remove unused array
>> 'ar9280PciePhy_clkreq_off_L1_9280'
>> ath9k: ar9001_initvals: Remove unused array 'ar5416Bank6_9100'
>> ath9k: ar5008_initvals: Remove unused table entirely
>> ath9k: ar5008_initvals: Move ar5416Bank{0,1,2,3,7} to where they are
>> used
>> brcmsmac: phytbl_lcn: Remove unused array 'dot11lcn_gain_tbl_rev1'
>> brcmsmac: phy_lcn: Remove unused variable
>> 'lcnphy_rx_iqcomp_table_rev0'
>
> What's happening with all of these iwlwifi patches?
>
> Looks like they are still not applied.
Luca (CCed) takes iwlwifi patches to his iwlwifi tree.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [RFC] net: phy: add shutdown hook to struct phy_driver
On 05.10.2020 18:00, Florian Fainelli wrote: > > > On 10/5/2020 8:54 AM, Heiner Kallweit wrote: >> On 05.10.2020 17:41, Florian Fainelli wrote: >>> >>> >>> On 10/5/2020 1:53 AM, Jisheng Zhang wrote: On Wed, 30 Sep 2020 13:23:29 -0700 Florian Fainelli wrote: > > On 9/30/2020 1:11 PM, Andrew Lunn wrote: >> On Wed, Sep 30, 2020 at 01:07:19PM -0700, Florian Fainelli wrote: >>> >>> >>> On 9/30/2020 12:09 PM, Andrew Lunn wrote: On Wed, Sep 30, 2020 at 05:47:43PM +0800, Jisheng Zhang wrote: > Hi, > > A GE phy supports pad isolation which can save power in WOL mode. But > once the > isolation is enabled, the MAC can't send/receive pkts to/from the phy > because > the phy is "isolated". To make the PHY work normally, I need to move > the > enabling isolation to suspend hook, so far so good. But the isolation > isn't > enabled in system shutdown case, to support this, I want to add > shutdown hook > to net phy_driver, then also enable the isolation in the shutdown > hook. Is > there any elegant solution? > Or we can break the assumption: ethernet can still send/receive pkts > after > enabling WoL, no? That is not an easy assumption to break. The MAC might be doing WOL, so it needs to be able to receive packets. What you might be able to assume is, if this PHY device has had WOL enabled, it can assume the MAC does not need to send/receive after suspend. The problem is, phy_suspend() will not call into the driver is WOL is enabled, so you have no idea when you can isolate the MAC from the PHY. So adding a shutdown in mdio_driver_register() seems reasonable. But you need to watch out for ordering. Is the MDIO bus driver still running? >>> >>> If your Ethernet MAC controller implements a shutdown callback and that >>> callback takes care of unregistering the network device which should >>> also >>> ensure that phy_disconnect() gets called, then your PHY's suspend >>> function >>> will be called. >> >> Hi Florian >> >> I could be missing something here, but: >> >> phy_suspend does not call into the PHY driver if WOL is enabled. So >> Jisheng needs a way to tell the PHY it should isolate itself from the >> MAC, and suspend is not that. > > I missed that part, that's right if WoL is enabled at the PHY level then > the suspend callback is not called, how about we change that and we > always call the PHY's suspend callback? This would require that we audit Hi all, The PHY's suspend callback usually calls genphy_suspend() which will set BMCR_PDOWN bit, this may break WoL. I think this is one the reason why we ignore the phydrv->suspend() when WoL is enabled. If we goes to this directly, it looks like we need to change each phy's suspend implementation, I.E if WoL is enabled, ignore genphy_suspend() and do possible isolation; If WoL is disabled, keep the code path as is. So compared with the shutdown hook, which direction is better? >>> >>> I believe you will have an easier time to add an argument to the PHY driver >>> suspend's function to indicate the WoL status, or to move down the check >>> for WoL being enabled/supported compared to adding support for shutdown >>> into the MDIO bus layer, and then PHY drivers. >> >> Maybe the shutdown callback of mdio_bus_type could be implemented. >> It could iterate over all PHY's on the bus, check for WoL (similar to >> mdio_bus_phy_may_suspend) and do whatever is needed. >> Seems to me to be the most generic way. > > OK and we optionally call into a PHY device's shutdown function if defined so > it can perform PHY specific work? That would work for me. If suspend and shutdown procedure are the same for the PHY, then we may not need a shutdown hook in phy_driver. This seems to be the case here. I just wonder what the actual use case is, because typically MAC drivers call phy_stop (that calls phy_suspend) in their shutdown hook.
[PATCH v2] net/x25: Fix null-ptr-deref in x25_connect
This fixes a regression for blocking connects introduced by commit
4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25 disconnect").
The x25->neighbour is already set to "NULL" by x25_disconnect() now,
while a blocking connect is waiting in
x25_wait_for_connection_establishment(). Therefore x25->neighbour must
not be accessed here again and x25->state is also already set to
X25_STATE_0 by x25_disconnect().
Fixes: 4becb7ee5b3d ("net/x25: Fix x25_neigh refcnt leak when x25 disconnect")
Signed-off-by: Martin Schiller
---
Change from v1:
also handle interrupting signals correctly
---
net/x25/af_x25.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 0bbb283f23c9..046d3fee66a9 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -825,7 +825,7 @@ static int x25_connect(struct socket *sock, struct sockaddr
*uaddr,
sock->state = SS_CONNECTED;
rc = 0;
out_put_neigh:
- if (rc) {
+ if (rc && x25->neighbour) {
read_lock_bh(_list_lock);
x25_neigh_put(x25->neighbour);
x25->neighbour = NULL;
--
2.20.1
Re: [PATCH 3/4] dt-bindings: Explicitly allow additional properties in board/SoC schemas
On 05-10-20, 13:38, Rob Herring wrote: > In order to add meta-schema checks for additional/unevaluatedProperties > being present, all schema need to make this explicit. As the top-level > board/SoC schemas always have additional properties, add > 'additionalProperties: true'. > > Signed-off-by: Rob Herring > --- > Documentation/devicetree/bindings/arm/spear.yaml | 3 +++ Acked-by: Viresh Kumar -- viresh
Re: linux-next: build failure after merge of the net-next tree
Hi Christoph, On Tue, 6 Oct 2020 07:13:01 +0200 Christoph Hellwig wrote: > > On Tue, Oct 06, 2020 at 02:58:47PM +1100, Stephen Rothwell wrote: > > Hi all, > > > > After merging the net-next tree, today's linux-next build (x86_64 > > allmodconfig) failed like this: > > It actually doesn't need that or the two other internal headers. > Bjoern has a fixed, and it was supposed to be queued up according to > patchwork. Yeah, it is in the bpf-next tree but not merged into the net-next tree yet. -- Cheers, Stephen Rothwell pgpQk_Co1JM6P.pgp Description: OpenPGP digital signature
Re: [PATCH] perf inject: Flush ordered events on FINISHED_ROUND
On Tue, Oct 06, 2020 at 11:39:49AM +0900, namhy...@kernel.org wrote: > > > On Fri, Oct 02, 2020 at 10:03:17PM +0900, Namhyung Kim wrote: > > > > Below measures time and memory usage during the perf inject and > > > > report using ~190MB data file. > > > > > > > > Before: > > > > perf inject: 11.09 s, 382148 KB > > > > perf report: 8.05 s, 397440 KB > > > > > > > > After: > > > > perf inject: 16.24 s, 83376 KB > > > > perf report: 7.96 s, 216184 KB > > > > > > > > As you can see, it used 2x memory of the input size. I guess it's > > > > because it needs to keep the copy for the whole input. But I don't > > > > understand why processing time of perf inject increased.. > > Measuring it with time shows: > >before after > real11.309s 17.040s > user 8.084s 13.940s > sys 6.535s 6.732s > > So it's user space to make the difference. I've run perf record on > both (with cycles:U) and the dominant function is same: queue_event. > (46.98% vs 65.87%) > > It seems the flushing the queue makes more overhead on sorting. So I suspect the cache-miss ratio affects the performance. With flushing, data is processed in the middle and all the entries are reused after flush so it would invalidate all the cache lines occasionally. This is the perf stat result: * Before 7,167,414,019 L1-dcache-loads 337,471,761 L1-dcache-read-misses #4.71% of all L1-dcache hits 11.011224671 seconds time elapsed * After 7,075,556,792 L1-dcache-loads 771,810,388 L1-dcache-read-misses # 10.91% of all L1-dcache hits 17.015901863 seconds time elapsed Hmm.. it's a memory & time trade-off then. Maybe we need a switch to select which one? Thanks Namhyung
Re: [Patch 2/2] cpufreq: tegra194: Fix unlisted boot freq warning
On 06-10-20, 00:24, Sumit Gupta wrote: > > > > Warning coming during boot because the boot freq set by bootloader > > > gets filtered out due to big freq steps while creating freq_table. > > > Fixing this by setting closest ndiv value from freq_table. > > > Warning: > > >cpufreq: cpufreq_online: CPU0: Running at unlisted freq > > >cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed > > > > > > Also, added change in init to wait till current frequency becomes > > > equal or near to the previously requested frequency. This is done > > > because it takes some time to restore the previous frequency while > > > turning-on non-boot cores during exit from SC7(Suspend-to-RAM). > > > > So you are trying to figure if the frequency is listed in freq-table or not, > > otherwise setting the frequency to a value you think is appropriate. Right ? > During boot, want to set the frequency from freq_table which is closest to > the one set by bootloader. Right. > During resume from suspend-to-idle, want to restore the frequency which was > set on non-boot cores before they were hotplug powered off. Why exactly do you want to do that ? Rather you should provide online/offline hooks for the cpufreq driver and do light-weight suspend/resume as is done by cpufreq-dt.c as well. > > > > This is what the cpufreq core already does when it printed these boot time > > messages. Do we really need to add this much code in here ? > We want to avoid the warning messages. Hmm, okay. > > > > If you really don't want to see the warning, how about fixing it the way > > cpufreq > > core does ? i.e. with this call: > > > > ret = __cpufreq_driver_target(policy, policy->cur - 1, CPUFREQ_RELATION_L); > > > The cpufreq core change will help in bootup case but not during the case of > resume. > In this change, reading the previously stored value and restoring it will > also fix the warning message during resume. You were getting the message during resume as well ? Why ? The firmware is updating the frequency to a previous value ? If that is so, you should just set the frequency again to some other value during resume to fix it. -- viresh
Re: [PATCH 4/4] dt-bindings: Explicitly allow additional properties in common schemas
On 05-10-20, 13:38, Rob Herring wrote: > In order to add meta-schema checks for additional/unevaluatedProperties > being present, all schema need to make this explicit. As common/shared > schema are included by other schemas, they should always allow for > additionalProperties. Acked-By: Vinod Koul -- ~Vinod
Re: [PATCH 1/4] dt-bindings: Add missing 'unevaluatedProperties'
On 05-10-20, 13:38, Rob Herring wrote: > This doesn't yet do anything in the tools, but make it explicit so we can > check either 'unevaluatedProperties' or 'additionalProperties' is present > in schemas. > > 'unevaluatedProperties' is appropriate when including another schema (via > '$ref') and all possible properties and/or child nodes are not > explicitly listed in the schema with the '$ref'. > > This is in preparation to add a meta-schema to check for missing > 'unevaluatedProperties' or 'additionalProperties'. This has been a > constant source of review issues. Acked-By: Vinod Koul -- ~Vinod
linux-next: build failure after merge of the mmc tree
Hi all,
After merging the mmc tree, today's linux-next build (x86_64 allmodconfig)
failed like this:
drivers/mmc/host/sdhci-acpi.c: In function 'amd_select_drive_strength':
drivers/mmc/host/sdhci-acpi.c:562:39: error: 'SDHCI_PRESET_DRV_SHIFT'
undeclared (first use in this function); did you mean 'SDHCI_PRESET_DRV_MASK'?
562 | (preset & SDHCI_PRESET_DRV_MASK) >> SDHCI_PRESET_DRV_SHIFT;
| ^~
| SDHCI_PRESET_DRV_MASK
Caused by commit
e9b80bb74fdd ("mmc: sdhci-acpi: AMDI0040: Allow changing HS200/HS400 driver
strength")
I have used the mmc tree from next-20201002 for today.
--
Cheers,
Stephen Rothwell
pgpPY_Sp1XF1V.pgp
Description: OpenPGP digital signature
Re: [PATCH 5.8 00/85] 5.8.14-rc1 review
On Mon, 5 Oct 2020 at 21:01, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 5.8.14 release. > There are 85 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 07 Oct 2020 14:20:55 +. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.8.14-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-5.8.y > and the diffstat can be found below. > > thanks, > > greg k-h > Results from Linaro’s test farm. No regressions on arm64, arm, x86_64, and i386. Tested-by: Linux Kernel Functional Testing Summary kernel: 5.8.14-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-5.8.y git commit: 8bb413de12d0ad809391ab5a965b0fcf1b9bb3fb git describe: v5.8.13-86-g8bb413de12d0 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-5.8.y/build/v5.8.13-86-g8bb413de12d0 No regressions (compared to build v5.8.13) No fixes (compared to build v5.8.13) Ran 39163 total tests in the following environments and test suites. Environments -- - dragonboard-410c - hi6220-hikey - i386 - juno-r2 - juno-r2-compat - juno-r2-kasan - nxp-ls2088 - qemu_arm - qemu_arm64 - qemu_i386 - qemu_x86_64 - x15 - x86 - x86-kasan Test Suites --- * build * install-android-platform-tools-r2600 * kselftest * libhugetlbfs * linux-log-parser * ltp-cap_bounds-tests * ltp-commands-tests * ltp-controllers-tests * ltp-cpuhotplug-tests * ltp-crypto-tests * ltp-cve-tests * ltp-dio-tests * ltp-fcntl-locktests-tests * ltp-filecaps-tests * ltp-fs-tests * ltp-fs_bind-tests * ltp-fs_perms_simple-tests * ltp-fsx-tests * ltp-hugetlb-tests * ltp-io-tests * ltp-ipc-tests * ltp-math-tests * ltp-mm-tests * ltp-nptl-tests * ltp-pty-tests * ltp-securebits-tests * ltp-syscalls-tests * perf * network-basic-tests * v4l2-compliance * ltp-containers-tests * ltp-open-posix-tests * ltp-sched-tests * ltp-tracing-tests * timesync-off * kselftest-vsyscall-mode-native * kselftest-vsyscall-mode-none * ssuite -- Linaro LKFT https://lkft.linaro.org
Re: [PATCH] Revert "mtd: spi-nor: Prefer asynchronous probe"
On Mon, 5 Oct 2020 14:33:21 +0530, Vignesh Raghavendra wrote: > This reverts commit 03edda0e1edaa3c2e99239c66e3c14d749318fd6. > > This leads to warn dump like [1] on some platforms and reorders MTD > devices thus may break user space expectations. So revert the change. > > [1]: > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next, thanks! [1/1] Revert "mtd: spi-nor: Prefer asynchronous probe" https://git.kernel.org/mtd/c/9a3422a110 -- Regards Vignesh
Re: linux-next: build failure after merge of the net-next tree
On Tue, Oct 06, 2020 at 02:58:47PM +1100, Stephen Rothwell wrote: > Hi all, > > After merging the net-next tree, today's linux-next build (x86_64 > allmodconfig) failed like this: It actually doesn't need that or the two other internal headers. Bjoern has a fixed, and it was supposed to be queued up according to patchwork.
Re: [PATCH] printk: handle blank console arguments passed in.
On Mon, Oct 05, 2020 at 08:35:59PM -0700, Guenter Roeck wrote:
> On 10/5/20 7:59 PM, Sergey Senozhatsky wrote:
> > Cc-ing Guenter,
> >
> > On (20/05/22 12:00), Petr Mladek wrote:
> >> On Fri 2020-05-22 16:53:06, Shreyas Joshi wrote:
> >>> If uboot passes a blank string to console_setup then it results in a
> >>> trashed memory.
> >>> Ultimately, the kernel crashes during freeing up the memory. This fix
> >>> checks if there
> >>> is a blank parameter being passed to console_setup from uboot.
> >>> In case it detects that the console parameter is blank then
> >>> it doesn't setup the serial device and it gracefully exits.
> >>>
> >>> Signed-off-by: Shreyas Joshi
> >>> ---
> >>> V1:
> >>> Fixed console_loglevel to default as per the review comments
> >>>
> >>> kernel/printk/printk.c | 5 -
> >>> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> >>> index ad4606234545..e9ad730991e0 100644
> >>> --- a/kernel/printk/printk.c
> >>> +++ b/kernel/printk/printk.c
> >>> @@ -2165,7 +2165,10 @@ static int __init console_setup(char *str)
> >>> char buf[sizeof(console_cmdline[0].name) + 4]; /* 4 for "ttyS" */
> >>> char *s, *options, *brl_options = NULL;
> >>> int idx;
> >>> -
> >>> + if (str[0] == 0) {
> >>> + return 1;
> >>> + }
> >>> if (_braille_console_setup(, _options))
> >>> return 1;
> >>
> >> I have fixed formatting and pushed it into printk/linux.git,
> >> branch for-5.8.
> >
> > Petr, this patch's causing regressions for us. We use blank console= boot
> > param to bypass dts. It appears that it'd be better to revert the change.
> >
>
> Not just to bypass dts, it was also possible to use console= to disable
> consoles
> passed as config option, as well as other default console options. A quick
> test
> confirms that this affects all platforms/architectures, not just Chromebooks.
> Prior to this patch, it was possible to disable a default console with an
> empty "console=" parameter. This is no longer possible. This means that
> this patch results in a substantial (and, as far as I can see, completely
> undiscussed) functionality change.
>
> I don't understand why (yet), but the patch also causes regressions with
> seemingly unrelated functionality, specifically with dm-verity on at least
> one Chromebook platform. I filed crbug.com/1135157 to track the problem,
> and reverted the patch from all our stable releases immediately after
> the last round of stable release merges.
>
> On a side note, I don't see the problem presumably fixed with this
> patch in any of my tests.
I have no problem reverting this in the stable trees, but are you going
to hit this issue in Linus's tree in the next release?
thanks,
greg k-h
linux-next: manual merge of the input tree with the arm-soc tree
Hi all,
Today's linux-next merge of the input tree got a conflict in:
Documentation/devicetree/bindings/vendor-prefixes.yaml
between commit:
cb1cc137a2c1 ("dt-bindings: Add vendor prefix for Shenzhen Zkmagic Technology
Co., Ltd.")
from the arm-soc tree and commit:
8f445ffa851e ("dt-bindings: input/touchscreen: add bindings for zinitix")
from the input 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 Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6242f1baa24c,bd7b2d404124..
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@@ -1214,8 -1143,8 +1214,10 @@@ patternProperties
description: Shenzhen Zidoo Technology Co., Ltd.
"^zii,.*":
description: Zodiac Inflight Innovations
+ "^zinitix,.*":
+ description: Zinitix Co., Ltd
+ "^zkmagic,.*":
+description: Shenzhen Zkmagic Technology Co., Ltd.
"^zte,.*":
description: ZTE Corp.
"^zyxel,.*":
pgpjhbvlNVVF1.pgp
Description: OpenPGP digital signature
Re: [PATCH v2 0/2] arm: sti: LL_UART updates & STiH418 addition
Hi Russell, Could you have a look a those two patches for the STi platform ? Regards, Alain On Sat, Sep 12, 2020 at 12:13:59PM +0200, Linus Walleij wrote: > On Sun, Aug 30, 2020 at 9:58 PM Alain Volmat wrote: > > > This serie update the STi Platform LL_UART code to rely on > > DEBUG_UART_PHYS & DEBUG_UART_VIRT and add the STiH418 SoC support. > > > > Alain Volmat (2): > > arm: use DEBUG_UART_PHYS and DEBUG_UART_VIRT for sti LL_UART > > arm: sti LL_UART: add STiH418 SBC UART0 support > > Both patches looks good to me: > Reviewed-by: Linus Walleij > > I made some patches to the debug UARTs that are pending in Russell's > patch tracker: > https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9004/1 > https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9005/1 > https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9006/1 > > It doesn't look like these will conflict with your patches but please take > a look just to make sure. > > Yours, > Linus Walleij
Re: ext4 regression in v5.9-rc2 from e7bfb5c9bb3d on ro fs with overlapped bitmaps
On Mon, Oct 05, 2020 at 11:18:34PM -0400, Theodore Y. Ts'o wrote: > What Josh is proposing I'm pretty sure would also break "e2fsck -E > unshare_blocks", so that's another reason not to accept this as a > valid format change. The kernel already accepted this as a valid mountable filesystem format, without a single error or warning of any kind, and has done so stably for years. > As far as I'm concerned, contrib/e2fsdroid is the canonical definition > of how to create valid file systems with shared_blocks. I'm not trying to create a problem here; I'm trying to address a whole family of problems. I was generally under the impression that mounting existing root filesystems fell under the scope of the kernel<->userspace or kernel<->existing-system boundary, as defined by what the kernel accepts and existing userspace has used successfully, and that upgrading the kernel should work with existing userspace and systems. If there's some other rule that applies for filesystems, I'm not aware of that. (I'm also not trying to suggest that every random corner case of what the kernel *could* accept needs to be the format definition, but rather, cases that correspond to existing userspace.) It wouldn't be *impossible* to work around this, this time; it may be possible to adapt the existing userspace to work on the new and old kernels. My concern is, if a filesystem format accepted by previous kernels can be rejected by future kernels, what stops a future kernel from further changing the format definition or its strictness (co-evolving with one specific userspace) and causing further regressions? I don't *want* to rely on what apparently turned out to be an undocumented bug in the kernel's validator. That's why I was trying to fix the issue in what seemed like the right way, by detecting the situation and turning off the validator. That seemed like it would fully address the issue. If it would help, I could also supply a tiny filesystem image for regression testing. I'm trying to figure out what solution you'd like to see here, as long as it isn't "any userspace that isn't e2fsdroid can be broken at will". I'd be willing to work to adapt the userspace bits I have to work around the regression, but I'd like to get this on the radar so this doesn't happen again. - Josh Triplett
Re: [PATCH v6 02/14] ASoC: sun4i-i2s: Change set_chan_cfg() params
On 10/5/20 7:13 AM, Maxime Ripard wrote:
> On Sat, Oct 03, 2020 at 04:19:38PM +0200, Clément Péron wrote:
>> As slots and slot_width can be set manually using set_tdm().
>> These values are then kept in sun4i_i2s struct.
>> So we need to check if these values are setted or not
>> in the struct.
>>
>> Avoid to check for this logic in set_chan_cfg(). This will
>> duplicate the same check instead pass the required values
>> as params to set_chan_cfg().
>>
>> This will also avoid a bug when we will enable 20/24bits support,
>> i2s->slot_width is not actually used in the lrck_period computation.
>>
>> Suggested-by: Samuel Holland
>> Signed-off-by: Clément Péron
>> ---
>> sound/soc/sunxi/sun4i-i2s.c | 36 ++--
>> 1 file changed, 14 insertions(+), 22 deletions(-)
>>
>> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
>> index c5ccd423e6d3..1f577dbc20a6 100644
>> --- a/sound/soc/sunxi/sun4i-i2s.c
>> +++ b/sound/soc/sunxi/sun4i-i2s.c
>> @@ -177,8 +177,9 @@ struct sun4i_i2s_quirks {
>> unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
>> s8 (*get_sr)(const struct sun4i_i2s *, int);
>> s8 (*get_wss)(const struct sun4i_i2s *, int);
>> -int (*set_chan_cfg)(const struct sun4i_i2s *,
>> -const struct snd_pcm_hw_params *);
>> +int (*set_chan_cfg)(const struct sun4i_i2s *i2s,
>> +unsigned int channels, unsigned int slots,
>> +unsigned int slot_width);
>> int (*set_fmt)(const struct sun4i_i2s *, unsigned int);
>> };
>>
>> @@ -414,10 +415,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s
>> *i2s, int width)
>> }
>>
>> static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>> - const struct snd_pcm_hw_params *params)
>> + unsigned int channels, unsigned int slots,
>> + unsigned int slot_width)
>> {
>> -unsigned int channels = params_channels(params);
>> -
>> /* Map the channels for playback and capture */
>> regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
>> regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x3210);
>> @@ -434,15 +434,11 @@ static int sun4i_i2s_set_chan_cfg(const struct
>> sun4i_i2s *i2s,
>> }
>>
>> static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>> - const struct snd_pcm_hw_params *params)
>> + unsigned int channels, unsigned int slots,
>> + unsigned int slot_width)
>> {
>> -unsigned int channels = params_channels(params);
>> -unsigned int slots = channels;
>> unsigned int lrck_period;
>>
>> -if (i2s->slots)
>> -slots = i2s->slots;
>> -
>> /* Map the channels for playback and capture */
>> regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
>> regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);
>> @@ -467,11 +463,11 @@ static int sun8i_i2s_set_chan_cfg(const struct
>> sun4i_i2s *i2s,
>> case SND_SOC_DAIFMT_DSP_B:
>> case SND_SOC_DAIFMT_LEFT_J:
>> case SND_SOC_DAIFMT_RIGHT_J:
>> -lrck_period = params_physical_width(params) * slots;
>> +lrck_period = slot_width * slots;
>> break;
>>
>> case SND_SOC_DAIFMT_I2S:
>> -lrck_period = params_physical_width(params);
>> +lrck_period = slot_width;
>> break;
>>
>> default:
>> @@ -490,15 +486,11 @@ static int sun8i_i2s_set_chan_cfg(const struct
>> sun4i_i2s *i2s,
>> }
>>
>> static int sun50i_h6_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
>> - const struct snd_pcm_hw_params *params)
>> + unsigned int channels, unsigned int slots,
>> + unsigned int slot_width)
>> {
>> -unsigned int channels = params_channels(params);
>> -unsigned int slots = channels;
>> unsigned int lrck_period;
>>
>> -if (i2s->slots)
>> -slots = i2s->slots;
>> -
>> /* Map the channels for playback and capture */
>> regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP0_REG, 0xFEDCBA98);
>> regmap_write(i2s->regmap, SUN50I_H6_I2S_TX_CHAN_MAP1_REG, 0x76543210);
>> @@ -525,11 +517,11 @@ static int sun50i_h6_i2s_set_chan_cfg(const struct
>> sun4i_i2s *i2s,
>> case SND_SOC_DAIFMT_DSP_B:
>> case SND_SOC_DAIFMT_LEFT_J:
>> case SND_SOC_DAIFMT_RIGHT_J:
>> -lrck_period = params_physical_width(params) * slots;
>> +lrck_period = slot_width * slots;
>> break;
>>
>> case SND_SOC_DAIFMT_I2S:
>> -lrck_period = params_physical_width(params);
>> +lrck_period = slot_width;
>> break;
>>
>> default:
>> @@ -565,7 +557,7 @@ static int
Re: [PATCH blk-next 1/2] blk-mq-rdma: Delete not-used multi-queue RDMA map queue code
On Mon, Oct 05, 2020 at 10:38:17AM +0200, Christoph Hellwig wrote: > On Fri, Oct 02, 2020 at 01:20:35PM -0700, Sagi Grimberg wrote: > >> Well, why would they change it? The whole point of the infrastructure > >> is that there is a single sane affinity setting for a given setup. Now > >> that setting needed some refinement from the original series (e.g. the > >> current series about only using housekeeping cpus if cpu isolation is > >> in use). But allowing random users to modify affinity is just a receipe > >> for a trainwreck. > > > > Well allowing people to mangle irq affinity settings seem to be a hard > > requirement from the discussions in the past. > > > >> So I think we need to bring this back ASAP, as doing affinity right > >> out of the box is an absolute requirement for sane performance without > >> all the benchmarketing deep magic. > > > > Well, it's hard to say that setting custom irq affinity settings is > > deemed non-useful to anyone and hence should be prevented. I'd expect > > that irq settings have a sane default that works and if someone wants to > > change it, it can but there should be no guarantees on optimal > > performance. But IIRC this had some dependencies on drivers and some > > more infrastructure to handle dynamic changes... > > The problem is that people change random settings. We need to generalize > it into a sane API (e.g. the housekeeping CPUs thing which totally makes > sense). I don't see many people jump on the bandwagon, someone should do it, but who will? I personally have no knowledge in that area to do anything meaningful. Thanks
Re: [RFC PATCH] DRM: amd: powerplay: don't undef pr_warn() {causes ARC build errors}
On Mon, 2020-10-05 at 21:50 -0700, Randy Dunlap wrote: > From: Randy Dunlap > > arch/arc/ implements BUG_ON() with BUG(). ARC has its own BUG() > function and that function uses pr_warn() as part of its implementation. > > Several (8) files in amd/powerplay/ #undef various pr_xyz() functions so > that they won't be used by these drivers, since dev_() functions are > preferred here and the #undefs make the pr_() functions unavailable. [] > --- lnx-59-rc7.orig/drivers/gpu/drm/amd/powerplay/navi10_ppt.c > +++ lnx-59-rc7/drivers/gpu/drm/amd/powerplay/navi10_ppt.c > @@ -52,7 +52,7 @@ > * They are more MGPU friendly. > */ > #undef pr_err > -#undef pr_warn > +//#undef pr_warn > #undef pr_info > #undef pr_debug > > --- lnx-59-rc7.orig/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c > +++ lnx-59-rc7/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c > @@ -54,7 +54,7 @@ > * They are more MGPU friendly. > */ > #undef pr_err > -#undef pr_warn > +//#undef pr_warn > #undef pr_info > #undef pr_debug These are bad ideas as all of these pr_ entries may become functions in a near-term future.
Re: INFO: trying to register non-static key in uhid_dev_destroy
syzbot suspects this issue was fixed by commit: commit bce1305c0ece3dc549663605e567655dd701752c Author: Marc Zyngier Date: Sat Aug 29 11:26:01 2020 + HID: core: Correctly handle ReportSize being zero bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=10b82f5050 start commit: 152036d1 Merge tag 'nfsd-5.7-rc-2' of git://git.linux-nfs... git tree: upstream kernel config: https://syzkaller.appspot.com/x/.config?x=efdde85c3af536b5 dashboard link: https://syzkaller.appspot.com/bug?extid=0c601d7fbb8122d39093 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10ebad0c10 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14d6c21c10 If the result looks correct, please mark the issue as fixed by replying with: #syz fix: HID: core: Correctly handle ReportSize being zero For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Re: [PATCH 23/25] ASoC: sun8i-codec: Generalize AIF clock control
On 10/5/20 7:04 AM, Maxime Ripard wrote:
> Hi,
>
> On Wed, Sep 30, 2020 at 09:11:46PM -0500, Samuel Holland wrote:
>> The AIF clock control register has the same layout for all three AIFs.
>> The only difference between them is that AIF3 is missing some fields. We
>> can reuse the same register field definitions for all three registers,
>> and use the DAI ID to select the correct register address.
>>
>> Signed-off-by: Samuel Holland
>> ---
>> sound/soc/sunxi/sun8i-codec.c | 64 +++
>> 1 file changed, 34 insertions(+), 30 deletions(-)
>>
>> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
>> index 032a3f714dbb..1c34502ac47a 100644
>> --- a/sound/soc/sunxi/sun8i-codec.c
>> +++ b/sound/soc/sunxi/sun8i-codec.c
>> @@ -37,23 +37,23 @@
>> #define SUN8I_MOD_CLK_ENA_DAC 2
>> #define SUN8I_MOD_RST_CTL 0x014
>> #define SUN8I_MOD_RST_CTL_AIF1 15
>> #define SUN8I_MOD_RST_CTL_ADC 3
>> #define SUN8I_MOD_RST_CTL_DAC 2
>> #define SUN8I_SYS_SR_CTRL 0x018
>> #define SUN8I_SYS_SR_CTRL_AIF1_FS 12
>> #define SUN8I_SYS_SR_CTRL_AIF2_FS 8
>> -#define SUN8I_AIF1CLK_CTRL 0x040
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_MSTR_MOD15
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_CLK_INV 13
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV9
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV6
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ4
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT2
>> +#define SUN8I_AIF_CLK_CTRL(n) (0x040 * (1 +
>> (n)))
>> +#define SUN8I_AIF_CLK_CTRL_MSTR_MOD 15
>> +#define SUN8I_AIF_CLK_CTRL_CLK_INV 13
>> +#define SUN8I_AIF_CLK_CTRL_BCLK_DIV 9
>> +#define SUN8I_AIF_CLK_CTRL_LRCK_DIV 6
>> +#define SUN8I_AIF_CLK_CTRL_WORD_SIZ 4
>> +#define SUN8I_AIF_CLK_CTRL_DATA_FMT 2
>> #define SUN8I_AIF1_ADCDAT_CTRL 0x044
>> #define SUN8I_AIF1_ADCDAT_CTRL_AIF1_AD0L_ENA15
>> #define SUN8I_AIF1_ADCDAT_CTRL_AIF1_AD0R_ENA14
>> #define SUN8I_AIF1_ADCDAT_CTRL_AIF1_AD0L_SRC10
>> #define SUN8I_AIF1_ADCDAT_CTRL_AIF1_AD0R_SRC8
>> #define SUN8I_AIF1_DACDAT_CTRL 0x048
>> #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA15
>> #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA14
>> @@ -83,21 +83,21 @@
>> #define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_AIF1DA1R 10
>> #define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_AIF2DACR 9
>> #define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR 8
>>
>> #define SUN8I_SYSCLK_CTL_AIF1CLK_SRC_MASK GENMASK(9, 8)
>> #define SUN8I_SYSCLK_CTL_AIF2CLK_SRC_MASK GENMASK(5, 4)
>> #define SUN8I_SYS_SR_CTRL_AIF1_FS_MASK GENMASK(15, 12)
>> #define SUN8I_SYS_SR_CTRL_AIF2_FS_MASK GENMASK(11, 8)
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_CLK_INV_MASKGENMASK(14, 13)
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK GENMASK(12, 9)
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV_MASK GENMASK(8, 6)
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_MASK GENMASK(5, 4)
>> -#define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT_MASK GENMASK(3, 2)
>> +#define SUN8I_AIF_CLK_CTRL_CLK_INV_MASK GENMASK(14, 13)
>> +#define SUN8I_AIF_CLK_CTRL_BCLK_DIV_MASKGENMASK(12, 9)
>> +#define SUN8I_AIF_CLK_CTRL_LRCK_DIV_MASKGENMASK(8, 6)
>> +#define SUN8I_AIF_CLK_CTRL_WORD_SIZ_MASKGENMASK(5, 4)
>> +#define SUN8I_AIF_CLK_CTRL_DATA_FMT_MASKGENMASK(3, 2)
>>
>> #define SUN8I_CODEC_PASSTHROUGH_SAMPLE_RATE 48000
>>
>> #define SUN8I_CODEC_PCM_FORMATS (SNDRV_PCM_FMTBIT_S8 |\
>> SNDRV_PCM_FMTBIT_S16_LE |\
>> SNDRV_PCM_FMTBIT_S20_LE |\
>> SNDRV_PCM_FMTBIT_S24_LE |\
>> SNDRV_PCM_FMTBIT_S20_3LE|\
>> @@ -223,32 +223,34 @@ static int sun8i_codec_update_sample_rate(struct
>> sun8i_codec *scodec)
>> hw_rate << SUN8I_SYS_SR_CTRL_AIF1_FS);
>>
>> return 0;
>> }
>>
>> static int sun8i_codec_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>> {
>> struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai);
>> +u32 reg = SUN8I_AIF_CLK_CTRL(dai->id);
>> u32 format, invert, value;
>>
>> /* clock masters */
>> switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
>> case SND_SOC_DAIFMT_CBS_CFS: /* Codec slave, DAI master */
>> value = 0x1;
>> break;
>> case SND_SOC_DAIFMT_CBM_CFM: /* Codec Master, DAI slave */
>> value = 0x0;
>> break;
>>
Re: [PATCH 1/2] x86/stackprotector/32: Make the canary into a regular percpu variable
On Mon, Oct 5, 2020 at 7:29 PM Sean Christopherson
wrote:
>
> On Mon, Oct 05, 2020 at 12:30:03PM -0700, Andy Lutomirski wrote:
> > On 32-bit kernels, the stackprotector canary is quite nasty -- it is
> > stored at %gs:(20), which is nasty because 32-bit kernels use %fs for
> > percpu storage. It's even nastier because it means that whether %gs
> > contains userspace state or kernel state while running kernel code
> > sepends on whether stackprotector is enabled (this is
>
> depends
>
> > CONFIG_X86_32_LAZY_GS), and this setting radically changes the way
> > that segment selectors work. Supporting both variants is a
> > maintenance and testing mess.
> >
> > Merely rearranging so that percpu and the stack canary
> > share the same segment would be messy as the 32-bit percpu address
> > layout isn't currently compatible with putting a variable at a fixed
> > offset.
> >
> > Fortunately, GCC 8.1 added options that allow the stack canary to be
> > accessed as %fs:stack_canary, effectively turning it into an ordinary
> > percpu variable. This lets us get rid of all of the code to manage
> > the stack canary GDT descriptor and the CONFIG_X86_32_LAZY_GS mess.
> >
> > This patch forcibly disables stackprotector on older compilers that
> > don't support the new options and makes the stack canary into a
> > percpu variable.
>
> It'd be helpful to explicitly state that the so called "lazy GS" approach is
> now always used for i386.
>
> > Signed-off-by: Andy Lutomirski
> > ---
>
> ...
>
> > diff --git a/arch/x86/include/asm/suspend_32.h
> > b/arch/x86/include/asm/suspend_32.h
> > index fdbd9d7b7bca..eb872363ca82 100644
> > --- a/arch/x86/include/asm/suspend_32.h
> > +++ b/arch/x86/include/asm/suspend_32.h
> > @@ -16,9 +16,7 @@ struct saved_context {
> >* On x86_32, all segment registers, with the possible exception of
>
> Is this still a "possible" exception, or is it now always an exception?
Good catch.
>
> >* gs, are saved at kernel entry in pt_regs.
> >*/
> > -#ifdef CONFIG_X86_32_LAZY_GS
> > u16 gs;
> > -#endif
> > unsigned long cr0, cr2, cr3, cr4;
> > u64 misc_enable;
> > bool misc_enable_saved;
>
> ...
>
> > diff --git a/arch/x86/kernel/tls.c b/arch/x86/kernel/tls.c
> > index 64a496a0687f..3c883e064242 100644
> > --- a/arch/x86/kernel/tls.c
> > +++ b/arch/x86/kernel/tls.c
> > @@ -164,17 +164,11 @@ int do_set_thread_area(struct task_struct *p, int idx,
> > savesegment(fs, sel);
> > if (sel == modified_sel)
> > loadsegment(fs, sel);
> > -
> > - savesegment(gs, sel);
> > - if (sel == modified_sel)
> > - load_gs_index(sel);
> > #endif
> >
> > -#ifdef CONFIG_X86_32_LAZY_GS
> > savesegment(gs, sel);
> > if (sel == modified_sel)
> > - loadsegment(gs, sel);
> > -#endif
> > + load_gs_index(sel);
>
> Side topic, the "index" part of this is super confusing. I had to reread
> this entire patch after discovering load_gs_index is loadsegment on i386.
>
> Maybe also worth a shout out in the changelog?
Sure.
load_gs_index() makes perfect sense to me because I've been drinking
the kool-aid for too long. Maybe some day we should rename it, but
I"m not sure what the best name would be. set_gs_update_user_base()?
The semantics are that it loads GS except that it changes the user
GSBASE instead of the kernel GSBASE. Thanks, AMD.
--Andy
[RFC PATCH] DRM: amd: powerplay: don't undef pr_warn() {causes ARC build errors}
From: Randy Dunlap
arch/arc/ implements BUG_ON() with BUG(). ARC has its own BUG()
function and that function uses pr_warn() as part of its implementation.
Several (8) files in amd/powerplay/ #undef various pr_xyz() functions so
that they won't be used by these drivers, since dev_() functions are
preferred here and the #undefs make the pr_() functions unavailable.
Hence the following build errors are reported in ARC builds:
../drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c: In function
'navi10_fill_i2c_req':
../arch/arc/include/asm/bug.h:24:2: error: implicit declaration of function
'pr_warn'; did you mean 'drm_warn'? [-Werror=implicit-function-declaration]
../drivers/gpu/drm/amd/amdgpu/../powerplay/sienna_cichlid_ppt.c: In function
'sienna_cichlid_fill_i2c_req':
../arch/arc/include/asm/bug.h:24:2: error: implicit declaration of function
'pr_warn'; did you mean 'drm_warn'? [-Werror=implicit-function-declaration]
Fixes: 55084d7f4022 ("drm/amd/powerplay: forbid to use pr_err/warn/info/debug")
Reported-by: kernel test robot
Signed-off-by: Randy Dunlap
Cc: Evan Quan
Cc: amd-...@lists.freedesktop.org
Cc: Alex Deucher
Cc: Vineet Gupta
Cc: linux-snps-...@lists.infradead.org
---
Another alternative is for amd/powerplay/ drivers not to use BUG()
or BUG_ON().
A third alternative is to ask the ARC developers to implement BUG()
without using any pr_() functions.
drivers/gpu/drm/amd/powerplay/navi10_ppt.c |2 +-
drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--- lnx-59-rc7.orig/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ lnx-59-rc7/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -52,7 +52,7 @@
* They are more MGPU friendly.
*/
#undef pr_err
-#undef pr_warn
+//#undef pr_warn
#undef pr_info
#undef pr_debug
--- lnx-59-rc7.orig/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
+++ lnx-59-rc7/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
@@ -54,7 +54,7 @@
* They are more MGPU friendly.
*/
#undef pr_err
-#undef pr_warn
+//#undef pr_warn
#undef pr_info
#undef pr_debug
Re: [PATCH v2] drivers:tty:pty: Fix a race causing data loss on close
On 05. 10. 20, 13:31, Greg Kroah-Hartman wrote: > On Fri, Oct 02, 2020 at 08:03:04AM -0500, miny...@acm.org wrote: >> From: Corey Minyard >> >> If you write to a pty master an immediately close the pty master, the >> receiver might get a chunk of data dropped, but then receive some later >> data. That's obviously something rather unexpected for a user. It >> certainly confused my test program. ... > Jiri, any thoughts about this? At first glance, it looks sane to me, > but a second review would be great. Hi, I haven't had a chance to look into it yet (labsconf this week). Moving it upwards on my TODO. thanks, -- js
Re: [PATCH v9 00/32] Improvements for Tegra I2C driver
Hi Dmitry, > Hello, Wolfram! Thank you! This series started with 10 small patches and > then was growing with every new review round because more ideas were > suggested and I needed to rebase/redo majority of the patches, hence it > was a bit difficult to split it up into a smaller parts that could be > applied incrementally. But I'll try to improve this in the future, thanks! Ah, you got me wrong. What I meant was: If these patches still need changes or updates, don't send a new version of all the patches, but rather send incremental fixes on top of these patches. Sometimes it happens that you end up with 30+ patches, no worries. > > http://patchwork.ozlabs.org/project/linux-i2c/list/?series=191802 > > > > Is it obsolete by now? > > > > To be honest, I don't know. The author never answered, guess he may > reappear sometime in the future with a v2. Those patches need to be > corrected and rebased. Then, I will mark them as "Changes requested". Thanks for the heads up! Regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH 20/25] ASoC: sun8i-codec: Protect the clock rate while streams are open
On 10/5/20 8:15 AM, Chen-Yu Tsai wrote:
> On Mon, Oct 5, 2020 at 8:01 PM Maxime Ripard wrote:
>>
>> On Wed, Sep 30, 2020 at 09:11:43PM -0500, Samuel Holland wrote:
>>> The codec's clock input is shared among all AIFs, and shared with other
>>> audio-related hardware in the SoC, including I2S and SPDIF controllers.
>>> To ensure sample rates selected by userspace or by codec2codec DAI links
>>> are maintained, the clock rate must be protected while it is in use.
>>>
>>> Signed-off-by: Samuel Holland
>>> ---
>>> sound/soc/sunxi/sun8i-codec.c | 25 ++---
>>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
>>> index 501af64d43a0..86065bee7cd3 100644
>>> --- a/sound/soc/sunxi/sun8i-codec.c
>>> +++ b/sound/soc/sunxi/sun8i-codec.c
...
>>> @@ -466,17 +471,30 @@ static int sun8i_codec_hw_params(struct
>>> snd_pcm_substream *substream,
>>> (lrck_div_order - 4) <<
>>> SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV);
>>>
>>> /* BCLK divider (SYSCLK/BCLK ratio) */
>>> bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order,
>>> sample_rate);
>>> regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
>>> SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK,
>>> bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV);
>>>
>>> - if (!aif->open_streams) {
>>> + /* SYSCLK rate */
>>> + if (aif->open_streams) {
>>> + ret = clk_set_rate(scodec->clk_module, sysclk_rate);
>>> + if (ret < 0)
>>> + return ret;
>>> + } else {
>>> + ret = clk_set_rate_exclusive(scodec->clk_module, sysclk_rate);
>>
>> It's not really clear to me why we wouldn't want to always protect the
>> clock rate here?
>
> I believe the intention is to allow a window, i.e. when no audio
> blocks are running,
> when it is possible to switch between sample rate families?
Yes, this is an advantage now. It would not be the case if sun4i-i2s did
something similar. It has the same problem that multiple separate sound
cards compete for one PLL.
> ChenYu
Cheers,
Samuel
Re: [PATCH 20/25] ASoC: sun8i-codec: Protect the clock rate while streams are open
On 10/5/20 7:01 AM, Maxime Ripard wrote:
> On Wed, Sep 30, 2020 at 09:11:43PM -0500, Samuel Holland wrote:
>> The codec's clock input is shared among all AIFs, and shared with other
>> audio-related hardware in the SoC, including I2S and SPDIF controllers.
>> To ensure sample rates selected by userspace or by codec2codec DAI links
>> are maintained, the clock rate must be protected while it is in use.
>>
>> Signed-off-by: Samuel Holland
>> ---
>> sound/soc/sunxi/sun8i-codec.c | 25 ++---
>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
>> index 501af64d43a0..86065bee7cd3 100644
>> --- a/sound/soc/sunxi/sun8i-codec.c
>> +++ b/sound/soc/sunxi/sun8i-codec.c
>> @@ -416,27 +416,32 @@ static int sun8i_codec_get_lrck_div_order(unsigned int
>> slots,
>> unsigned int div = slots * slot_width;
>>
>> if (div < 16 || div > 256)
>> return -EINVAL;
>>
>> return order_base_2(div);
>> }
>>
>> +static unsigned int sun8i_codec_get_sysclk_rate(unsigned int sample_rate)
>> +{
>> +return sample_rate % 4000 ? 22579200 : 24576000;
>> +}
>> +
>> static int sun8i_codec_hw_params(struct snd_pcm_substream *substream,
>> struct snd_pcm_hw_params *params,
>> struct snd_soc_dai *dai)
>> {
>> struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai);
>> struct sun8i_codec_aif *aif = >aifs[dai->id];
>> unsigned int sample_rate = params_rate(params);
>> unsigned int slots = aif->slots ?: params_channels(params);
>> unsigned int slot_width = aif->slot_width ?: params_width(params);
>> -unsigned int sysclk_rate = clk_get_rate(scodec->clk_module);
>> -int lrck_div_order, word_size;
>> +unsigned int sysclk_rate = sun8i_codec_get_sysclk_rate(sample_rate);
>> +int lrck_div_order, ret, word_size;
>> u8 bclk_div;
>>
>> /* word size */
>> switch (params_width(params)) {
>> case 8:
>> word_size = 0x0;
>> break;
>> case 16:
>> @@ -466,17 +471,30 @@ static int sun8i_codec_hw_params(struct
>> snd_pcm_substream *substream,
>> (lrck_div_order - 4) <<
>> SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV);
>>
>> /* BCLK divider (SYSCLK/BCLK ratio) */
>> bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order,
>> sample_rate);
>> regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
>> SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK,
>> bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV);
>>
>> -if (!aif->open_streams) {
>> +/* SYSCLK rate */
>> +if (aif->open_streams) {
>> +ret = clk_set_rate(scodec->clk_module, sysclk_rate);
>> +if (ret < 0)
>> +return ret;
>> +} else {
>> +ret = clk_set_rate_exclusive(scodec->clk_module, sysclk_rate);
>
> It's not really clear to me why we wouldn't want to always protect the
> clock rate here?
>From Documentation/sound/kernel-api/writing-an-alsa-driver.rst:
hw_params callback
...
Note that this and ``prepare`` callbacks may be called multiple
times per initialization. For example, the OSS emulation may call
these callbacks at each change via its ioctl.
Clock rate protection is reference counted, so we must only take one
reference (or at least a known number of references) per stream.
>> +if (ret == -EBUSY)
>> +dev_err(dai->dev, "%s: clock is busy! Sample rate %u Hz
>> "
>> +"conflicts with other audio streams.\n",
>
> This string creates a checkpatch warning.
I will put it on one line, though >100 columns is also a checkpatch warning.
> Maxime
Cheers,
Samuel
Re: [PATCH v3] checkpatch: add new warnings to author signoff checks.
On Tue, 6 Oct 2020, Dwaipayan Ray wrote: > On Tue, Oct 6, 2020 at 2:39 AM Joe Perches wrote: > > > > On Tue, 2020-10-06 at 01:37 +0530, Dwaipayan Ray wrote: > > > On Tue, Oct 6, 2020 at 1:07 AM Joe Perches wrote: > > > > On Tue, 2020-10-06 at 00:54 +0530, Dwaipayan Ray wrote: > > > > > The author signed-off-by checks are currently very vague. > > > > > Cases like same name or same address are not handled separately. > > > > > > > > When you run tests for this, how many mismatches are > > > > caused by name formatting changes like: > > > > > > > > From: "Developer, J. Random" > > > > ... > > > > Signed-off-by: "J. Random Developer" ? > > > > > > > > Should these differences generate a warning? > > > > > > > > > > Hi, > > > I ran my tests on non merge commits between v5.7 and v5.8. > > > > > > There were a total of 250 NO_AUTHOR_SIGN_OFF Warnings > > > > > > 203 of these were email address mismatches. > > > 32 of these were name mismatches. > > > > > > So for the name mismatches, the typical cases are like: > > > > > > 'From: tannerlove ' != 'Signed-off-by: Tanner > > > Love ' > > > 'From: "朱灿灿" ' != 'Signed-off-by: zhucancan > > > ' > > > 'From: Yuval Basson ' != 'Signed-off-by: Yuval > > > Bason ' > > > 'From: allen ' != 'Signed-off-by: Allen Chen > > > ' > > > > > > I didn't find the exact formatting change you mentioned in my commit > > > range. > > > But I did find something like: > > > > > > 'From: "Paul A. Clarke" ' != 'Signed-off-by: Paul > > > Clarke ' > > > > > > So it's like some have parts of their names removed, some have language > > > conflicts, and yet some have well different spellings, or initials, > > > etc. It's like > > > a wide variety of things happening here. > > > > > > I think considering these, it should be warned about, and let people know > > > that there might be something wrong going on. > > > > > > What do you think? > > > > Except for comments and quotes like: > > > > From: J. Random Developer (BigCorp) > > Signed-off-by: "J. Random Developer" > > > > I think any time there's a mismatch, there > > should be a warning emitted. > > > > That includes any subaddress detail difference. > > > > > Hi, > Yeah these cases are being handled. > > Comments and quotes don't generate any warning message but > all the other mismatches do. > > Only the check for subaddress generates a --strict check message. > others are all WARN messages. It was followed from our discussion at > https://lore.kernel.org/linux-kernel-mentees/7b52e085f0b69ad1742966f8eacd02deb9299b96.ca...@perches.com/ > > So does it need to be changed to a WARN or is it fine like that? > I will repeat what I suggested before: I think the complete mismatch where we cannot even find a name or an email match the author deserves to be reported as ERROR. Dwaipayan, if Joe does not disagree, could you change that in your PATCH v4? Lukas
Re: [PATCH] usb: typec: tcpm: Fix if vbus before cc, hard_reset_count not reset issue
Guenter Roeck 於 2020年10月5日 週一 下午11:30寫道： > > On 10/5/20 4:08 AM, Greg KH wrote: > [ ... ] > >>> What ever happened with this patch, is there still disagreement? > >>> > >> > >> Yes, there is. I wouldn't have added the conditional without reason, > >> and I am concerned that removing it entirely will open another problem. > >> Feel free to apply, though - I can't prove that my concern is valid, > >> and after all we'll get reports from the field later if it is. > > > > Ok, can I get an ack so I know who to come back to in the future if > > there are issues? :) > > > > Not from me, for the reasons I stated. I would be ok with something like: > > - if (tcpm_port_is_disconnected(port)) > + if (tcpm_port_is_disconnected(port) || > + (tcpm_cc_is_open(port->cc1) && tcpm_cc_is_open(port->cc2))) > > to narrow down the condition. I have tried the above comment and It doesn't work. How about to change the judgement like as below - if (tcpm_port_is_disconnected(port)) + if (tcpm_port_is_disconnected(port) || !port->vbus_present) The hard_reset_count not reset issue is following by the below order 1. VBUS off ( at the same time, cc is still detected as attached) port->attached become false and cc is not open 2. After that, cc detached. due to port->attached is false, tcpm_detach() directly return. And that's why hard_reset_count is not reset to 0. > > Guenter
Re: [PATCH v3 01/24] dt-bindings: iommu: mediatek: Convert IOMMU to DT schema
On Fri, 2020-10-02 at 13:07 +0200, Krzysztof Kozlowski wrote: > On Wed, Sep 30, 2020 at 03:06:24PM +0800, Yong Wu wrote: > > Convert MediaTek IOMMU to DT schema. > > > > Signed-off-by: Yong Wu > > --- > > .../bindings/iommu/mediatek,iommu.txt | 103 > > .../bindings/iommu/mediatek,iommu.yaml| 154 ++ > > 2 files changed, 154 insertions(+), 103 deletions(-) > > delete mode 100644 > > Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > > create mode 100644 > > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > > [...] > > +properties: > > + compatible: > > +oneOf: > > + - enum: > > + - mediatek,mt2701-m4u # mt2701 generation one HW > > + - mediatek,mt2712-m4u # mt2712 generation two HW > > + - mediatek,mt6779-m4u # mt6779 generation two HW > > + - mediatek,mt8173-m4u # mt8173 generation two HW > > + - mediatek,mt8183-m4u # mt8183 generation two HW > > + > > + - description: mt7623 generation one HW > > +items: > > + - const: mediatek,mt7623-m4u > > + - const: mediatek,mt2701-m4u > > + > > + reg: > > +maxItems: 1 > > + > > + interrupts: > > +maxItems: 1 > > + > > + clocks: > > +description: | > > + bclk is optional. here is the list which require this bclk: > > + mt2701, mt2712, mt7623 and mt8173. > > Similarly to my comment in other patch, this should be part of schema > within 'if-then'. Thanks for the review. I will change like this: = clocks: items: - description: bclk is the block clock. clock-names: items: - const: bclk required: - compatible - reg - interrupts - mediatek,larbs - '#iommu-cells' if: properties: compatible: contains: enum: - mediatek,mt2701-m4u - mediatek,mt2712-m4u - mediatek,mt8173-m4u then: required: - clocks == If this is not right, please tell me. (dt_binding_check is ok.) > > Best regards, > Krzysztof
Re: [PATCH] Revert "gpu/drm: ingenic: Add option to mmap GEM buffers cached"
Hi all, On Mon, 5 Oct 2020 23:01:50 +1100 Stephen Rothwell wrote: > > On Sun, 04 Oct 2020 22:11:23 +0200 Paul Cercueil wrote: > > > > Pushed to drm-misc-next with the changelog fix, thanks. > > > > Stephen: > > Now it should build fine again. Could you remove the BROKEN flag? > > Thanks for letting me know, but the fix has not appeared in any drm > tree included in linux-next yet ... > > If it doesn't show up by the time I will merge the drm tree tomorrow, I > will apply this revert patch myself (instead of the patch marking the > driver BROKEN). Unfortunately, the revert patch does not apply to the drm tree merge, so I have just marked the driver BROKEN again today. -- Cheers, Stephen Rothwell pgpm2BhugBYw4.pgp Description: OpenPGP digital signature
Re: [PATCH 11/25] ASoC: sun8i-codec: Enable all supported clock inversions
On 10/5/20 6:30 AM, Maxime Ripard wrote:
On Wed, Sep 30, 2020 at 09:11:34PM -0500, Samuel Holland wrote:
When using the I2S, LEFT_J, or RIGHT_J format, the hardware supports
independent BCLK and LRCK inversion control. When using DSP_A or DSP_B,
LRCK inversion is not supported. The register bit is repurposed to
select between DSP_A and DSP_B. Extend the driver to support this.
Signed-off-by: Samuel Holland
---
sound/soc/sunxi/sun8i-codec.c | 57 +++
1 file changed, 37 insertions(+), 20 deletions(-)
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c
index 0b713b2a2028..506420fb355c 100644
--- a/sound/soc/sunxi/sun8i-codec.c
+++ b/sound/soc/sunxi/sun8i-codec.c
@@ -39,18 +39,17 @@
#define SUN8I_MOD_RST_CTL_AIF115
#define SUN8I_MOD_RST_CTL_ADC 3
#define SUN8I_MOD_RST_CTL_DAC 2
#define SUN8I_SYS_SR_CTRL 0x018
#define SUN8I_SYS_SR_CTRL_AIF1_FS 12
#define SUN8I_SYS_SR_CTRL_AIF2_FS 8
#define SUN8I_AIF1CLK_CTRL0x040
#define SUN8I_AIF1CLK_CTRL_AIF1_MSTR_MOD 15
-#define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_INV 14
-#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_INV 13
+#define SUN8I_AIF1CLK_CTRL_AIF1_CLK_INV13
#define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV 9
#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV 6
#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4
#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4)
#define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2
#define SUN8I_AIF1_ADCDAT_CTRL0x044
#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_AD0L_ENA 15
#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_AD0R_ENA 14
@@ -85,16 +84,17 @@
#define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_AIF1DA1R 10
#define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_AIF2DACR 9
#define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR 8
#define SUN8I_SYSCLK_CTL_AIF1CLK_SRC_MASK GENMASK(9, 8)
#define SUN8I_SYSCLK_CTL_AIF2CLK_SRC_MASK GENMASK(5, 4)
#define SUN8I_SYS_SR_CTRL_AIF1_FS_MASKGENMASK(15, 12)
#define SUN8I_SYS_SR_CTRL_AIF2_FS_MASKGENMASK(11, 8)
+#define SUN8I_AIF1CLK_CTRL_AIF1_CLK_INV_MASK GENMASK(14, 13)
#define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK GENMASK(12, 9)
#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV_MASK GENMASK(8, 6)
#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_MASK GENMASK(5, 4)
#define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT_MASK GENMASK(3, 2)
enum {
AIF1,
NAIFS
@@ -168,17 +168,17 @@ static int sun8i_codec_get_hw_rate(struct
snd_pcm_hw_params *params)
default:
return -EINVAL;
}
}
static int sun8i_codec_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
{
struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai);
- u32 format, value;
+ u32 format, invert, value;
/* clock masters */
switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
case SND_SOC_DAIFMT_CBS_CFS: /* Codec slave, DAI master */
value = 0x1;
break;
case SND_SOC_DAIFMT_CBM_CFM: /* Codec Master, DAI slave */
value = 0x0;
@@ -197,55 +197,72 @@ static int sun8i_codec_set_fmt(struct snd_soc_dai *dai,
unsigned int fmt)
break;
case SND_SOC_DAIFMT_LEFT_J:
format = 0x1;
break;
case SND_SOC_DAIFMT_RIGHT_J:
format = 0x2;
break;
case SND_SOC_DAIFMT_DSP_A:
+ format = 0x3;
+ invert = 0x0; /* Set LRCK_INV to 0 */
+ break;
case SND_SOC_DAIFMT_DSP_B:
format = 0x3;
+ invert = 0x1; /* Set LRCK_INV to 1 */
break;
default:
return -EINVAL;
}
regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT_MASK,
format << SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT);
/* clock inversion */
switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
case SND_SOC_DAIFMT_NB_NF: /* Normal */
value = 0x0;
break;
- case SND_SOC_DAIFMT_IB_IF: /* Inversion */
+ case SND_SOC_DAIFMT_NB_IF: /* Inverted LRCK */
value = 0x1;
break;
+ case SND_SOC_DAIFMT_IB_NF: /* Inverted BCLK */
+ value = 0x2;
+ break;
+ case SND_SOC_DAIFMT_IB_IF: /* Both inverted */
+ value = 0x3;
+ break;
default:
return -EINVAL;
}
- regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
- BIT(SUN8I_AIF1CLK_CTRL_AIF1_BCLK_INV),
-
Re: [PATCH v3 02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema
On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote: > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote: > > Convert MediaTek SMI to DT schema. > > > > Signed-off-by: Yong Wu > > --- > > .../mediatek,smi-common.txt | 49 - > > .../mediatek,smi-common.yaml | 100 ++ > > .../memory-controllers/mediatek,smi-larb.txt | 49 - > > .../memory-controllers/mediatek,smi-larb.yaml | 91 > > 4 files changed, 191 insertions(+), 98 deletions(-) > > delete mode 100644 > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > > create mode 100644 > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml > > delete mode 100644 > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt > > create mode 100644 > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml ... > > +properties: > > + compatible: > > +oneOf: > > + - enum: > > + - mediatek,mt2701-smi-common > > + - mediatek,mt2712-smi-common > > + - mediatek,mt6779-smi-common > > + - mediatek,mt8173-smi-common > > + - mediatek,mt8183-smi-common > > + > > + - description: for mt7623 > > +items: > > + - const: mediatek,mt7623-smi-common > > + - const: mediatek,mt2701-smi-common > > + > > + reg: > > +maxItems: 1 > > + > > + clocks: > > +description: | > > + apb and smi are mandatory. the async is only for generation 1 smi HW. > > + gals(global async local sync) also is optional, here is the list > > which > > + require gals: mt6779 and mt8183. > > +minItems: 2 > > +maxItems: 4 > > +items: > > + - description: apb is Advanced Peripheral Bus clock, It's the clock > > for > > + setting the register. > > + - description: smi is the clock for transfer data and command. > > + - description: async is asynchronous clock, it help transform the > > smi clock > > + into the emi clock domain. > > + - description: gals0 is the path0 clock of gals. > > + - description: gals1 is the path1 clock of gals. > > + > > + clock-names: > > +oneOf: > > + - items: > > + - const: apb > > + - const: smi > > + - items: > > + - const: apb > > + - const: smi > > + - const: async > > + - items: > > + - const: apb > > + - const: smi > > + - const: gals0 > > + - const: gals1 > > Similarly to my comment to other properties, this requirement per > compatible should be part of the schema within 'if-then'. I'm not so familiar with this format. Do this has "if-then-'else if'-then-else"? I tried below instead of the clocks segment above: === if: properties: compatible: enum: - mediatek,mt6779-smi-common - mediatek,mt8183-smi-common then: properties: clock: items: - description: apb is the clock for setting the register.. - description: smi is the clock for transfer data and command. - description: gals0 is the path0 clock of gals(global async local sync). - description: gals1 is the path1 clock of gals. clock-names: items: - const: apb - const: smi - const: gals0 - const: gals1 else: if: properties: compatible: contains: enum: - mediatek,mt2701-smi-common then: properties: clocks: items: - description: apb is the clock for setting the register. - description: smi is the clock for transfer data and command. - description: async is asynchronous clock, it help transform the smi clock into the emi clock domain. clock-names: items: - const: apb - const: smi - const: async else: properties: clocks: items: - description: apb is the clock for setting the register. - description: smi is the clock for transfer data and command. clock-names: items: - const: apb - const: smi But I got a warning when dt_binding_check: CHKDT Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml SCHEMA Documentation/devicetree/bindings/processed-schema-examples.yaml DTC Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml CHECK Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml .../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml: smi@14022000: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+' From schema: .../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml Any
Re: [PATCH v3 06/24] dt-bindings: mediatek: Add binding for mt8192 IOMMU
Hi Krzysztof, On Fri, 2020-10-02 at 13:10 +0200, Krzysztof Kozlowski wrote: > On Wed, Sep 30, 2020 at 03:06:29PM +0800, Yong Wu wrote: > > This patch adds decriptions for mt8192 IOMMU and SMI. > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > > table format. The M4U-SMI HW diagram is as below: > > > > EMI > >| > > M4U > >| > > > >SMI Common > > > >| > > +---+--+--+--+---+ > > | | | | .. | | > > | | | | | | > > larb0 larb1 larb2 larb4 .. larb19 larb20 > > disp0 disp1 mdpvdec IPE IPE > > > > All the connections are HW fixed, SW can NOT adjust it. > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines > > into different iova ranges: > > > > domain-id module iova-range larbs > >0 disp0 ~ 4G larb0/1 > >1 vcodec 4G ~ 8G larb4/5/7 > >2 cam/mdp 8G ~ 12G larb2/9/11/13/14/16/17/18/19/20 > >3 CCU00x4000_ ~ 0x43ff_ larb13: port 9/10 > >4 CCU10x4400_ ~ 0x47ff_ larb14: port 4/5 > > > > The iova range for CCU0/1(camera control unit) is HW requirement. > > > > Signed-off-by: Yong Wu > > Reviewed-by: Rob Herring > > --- > > .../bindings/iommu/mediatek,iommu.yaml| 9 +- > > .../mediatek,smi-common.yaml | 5 +- > > .../memory-controllers/mediatek,smi-larb.yaml | 3 +- > > include/dt-bindings/memory/mt8192-larb-port.h | 239 ++ > > 4 files changed, 251 insertions(+), 5 deletions(-) > > create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h > > I see it depends on previous patches but does it have to be within one > commit? Is it not bisectable? The memory changes/bindings could go via > memory tree if this is split. Thanks for the view. I can split this into two patchset in next version, one is for iommu and the other is for smi. Only the patch [18/24] change both the code(iommu and smi). I don't plan to split it, and the smi patch[24/24] don't depend on it(won't conflict). since 18/24 also touch the smi code, I expect it could get Acked-by from you or Matthias, then Joerg could take it. Thanks. > > Best regards, > Krzysztof
Re: [PATCH v3] checkpatch: add new warnings to author signoff checks.
On Tue, Oct 6, 2020 at 2:39 AM Joe Perches wrote: > > On Tue, 2020-10-06 at 01:37 +0530, Dwaipayan Ray wrote: > > On Tue, Oct 6, 2020 at 1:07 AM Joe Perches wrote: > > > On Tue, 2020-10-06 at 00:54 +0530, Dwaipayan Ray wrote: > > > > The author signed-off-by checks are currently very vague. > > > > Cases like same name or same address are not handled separately. > > > > > > When you run tests for this, how many mismatches are > > > caused by name formatting changes like: > > > > > > From: "Developer, J. Random" > > > ... > > > Signed-off-by: "J. Random Developer" ? > > > > > > Should these differences generate a warning? > > > > > > > Hi, > > I ran my tests on non merge commits between v5.7 and v5.8. > > > > There were a total of 250 NO_AUTHOR_SIGN_OFF Warnings > > > > 203 of these were email address mismatches. > > 32 of these were name mismatches. > > > > So for the name mismatches, the typical cases are like: > > > > 'From: tannerlove ' != 'Signed-off-by: Tanner > > Love ' > > 'From: "朱灿灿" ' != 'Signed-off-by: zhucancan > > ' > > 'From: Yuval Basson ' != 'Signed-off-by: Yuval > > Bason ' > > 'From: allen ' != 'Signed-off-by: Allen Chen > > ' > > > > I didn't find the exact formatting change you mentioned in my commit range. > > But I did find something like: > > > > 'From: "Paul A. Clarke" ' != 'Signed-off-by: Paul > > Clarke ' > > > > So it's like some have parts of their names removed, some have language > > conflicts, and yet some have well different spellings, or initials, > > etc. It's like > > a wide variety of things happening here. > > > > I think considering these, it should be warned about, and let people know > > that there might be something wrong going on. > > > > What do you think? > > Except for comments and quotes like: > > From: J. Random Developer (BigCorp) > Signed-off-by: "J. Random Developer" > > I think any time there's a mismatch, there > should be a warning emitted. > > That includes any subaddress detail difference. > > Hi, Yeah these cases are being handled. Comments and quotes don't generate any warning message but all the other mismatches do. Only the check for subaddress generates a --strict check message. others are all WARN messages. It was followed from our discussion at https://lore.kernel.org/linux-kernel-mentees/7b52e085f0b69ad1742966f8eacd02deb9299b96.ca...@perches.com/ So does it need to be changed to a WARN or is it fine like that? Thanks, Dwaipayan.
Re: linux-next: Fixes tag needs some work in the battery tree
On Tue, Oct 6, 2020 at 11:57 AM Ikjoon Jang wrote:
>
> On Mon, Oct 5, 2020 at 7:50 PM Stephen Rothwell wrote:
> >
> > Hi all,
> >
> > In commit
> >
> > f9d293364b45 ("power: supply: sbs-battery: keep error code when
> > get_property() fails")
> >
> > Fixes tag
> >
> > Fixes: c4f382930145 (power: supply: sbs-battery: don't assume i2c errors
> > as battery disconnect)
> >
> > has these problem(s):
> >
> > - Target SHA1 does not exist
> >
> > Maybe you meant
> >
> > Fixes: 395a7251dc2b ("power: supply: sbs-battery: don't assume i2c errors
> > as battery disconnect")
> >
>
> Yes, you're right. I guess I made a mistake here.
> I'll send a v2 patch.
Oh I'm sorry, it's from linux-next!
I found d6e24aa0bf15 ("power: supply: sbs-battery: keep error code
when get_property() fails") on sre/for-next branch
with a valid Fixes tag:
power: supply: sbs-battery: keep error code when get_property() fails
Commit c4f382930145 (power: supply: sbs-battery: don't assume
i2c errors as battery disconnect) overwrites the original error code
returned from internal functions. On such a sporadic i2c error,
a user will get a wrong value without errors.
Fixes: 395a7251dc2b (power: supply: sbs-battery: don't assume i2c
errors as battery disconnect)
Signed-off-by: Ikjoon Jang
Signed-off-by: Sebastian Reichel
but there is still a wrong sha-1 hash in the commit message,
Sebastian, can you please amend the commit message before merge?
>
> Thank you!
>
> > --
> > Cheers,
> > Stephen Rothwell
Re: [PATCH v2 1/3] staging: greybus: fix warnings about endianness detected by sparse
On Tue, Oct 06, 2020 at 12:47:37AM +0530, Vaibhav Agarwal wrote: On Sat, Oct 3, 2020 at 5:01 AM Coiby Xu wrote: This patch fix the following warnings from sparse, $ make C=2 drivers/staging/greybus/ drivers/staging/greybus/audio_module.c:222:25: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_module.c:222:25:expected restricted __le16 [usertype] data_cport drivers/staging/greybus/audio_module.c:222:25:got unsigned short [usertype] intf_cport_id drivers/staging/greybus/audio_topology.c:460:40: warning: restricted __le32 degrades to integer drivers/staging/greybus/audio_topology.c:691:41: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_topology.c:691:41:expected unsigned int access drivers/staging/greybus/audio_topology.c:691:41:got restricted __le32 [usertype] access drivers/staging/greybus/audio_topology.c:746:44: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_topology.c:746:44:expected unsigned int drivers/staging/greybus/audio_topology.c:746:44:got restricted __le32 drivers/staging/greybus/audio_topology.c:748:52: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_topology.c:748:52:expected unsigned int drivers/staging/greybus/audio_topology.c:748:52:got restricted __le32 drivers/staging/greybus/audio_topology.c:802:42: warning: restricted __le32 degrades to integer drivers/staging/greybus/audio_topology.c:805:50: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_topology.c:805:50:expected restricted __le32 drivers/staging/greybus/audio_topology.c:805:50:got unsigned int drivers/staging/greybus/audio_topology.c:814:50: warning: restricted __le32 degrades to integer drivers/staging/greybus/audio_topology.c:817:58: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_topology.c:817:58:expected restricted __le32 drivers/staging/greybus/audio_topology.c:817:58:got unsigned int drivers/staging/greybus/audio_topology.c:889:25: warning: incorrect type in assignment (different base types) drivers/staging/greybus/audio_topology.c:889:25:expected unsigned int access drivers/staging/greybus/audio_topology.c:889:25:got restricted __le32 [usertype] access Suggested-by: Dan Carpenter Reviewed-by: Dan Carpenter Reviewed-by: Alex Elder Signed-off-by: Coiby Xu --- Hi Coiby, Thanks for sharing the patch. Sorry, I could not reply to the v1 series. Now, I have gone through the patches. Looks good (all 3 patches). Reviewed-by: Vaibhav Agarwal -- Thanks, Hi Vaibhav, Thank you for reviewing these patches and giving the credit! -- Best regards, Coiby
[PATCH v2] iio: adc: exynos: do not rely on 'users' counter in ISR
The order in which 'users' counter is decremented vs calling drivers'
close() method is implementation specific, and we should not rely on
it. Let's introduce driver private flag and use it to signal ISR
to exit when device is being closed.
This has a side-effect of fixing issue of accessing inut->users
outside of input->mutex protection.
Reported-by: Andrzej Pietrasiewicz
Signed-off-by: Dmitry Torokhov
---
v2: switched from ordinary read/write to READ_ONCE/WRITE_ONCE per Michał
Mirosław
drivers/iio/adc/exynos_adc.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 22131a677445..6c705fe599a3 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -7,6 +7,7 @@
* Copyright (C) 2013 Naveen Krishna Chatradhi
*/
+#include
#include
#include
#include
@@ -135,6 +136,8 @@ struct exynos_adc {
u32 value;
unsigned intversion;
+ boolts_enabled;
+
boolread_ts;
u32 ts_x;
u32 ts_y;
@@ -633,7 +636,7 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
bool pressed;
int ret;
- while (info->input->users) {
+ while (READ_ONCE(info->ts_enabled)) {
ret = exynos_read_s3c64xx_ts(dev, , );
if (ret == -ETIMEDOUT)
break;
@@ -712,6 +715,7 @@ static int exynos_adc_ts_open(struct input_dev *dev)
{
struct exynos_adc *info = input_get_drvdata(dev);
+ WRITE_ONCE(info->ts_enabled, true);
enable_irq(info->tsirq);
return 0;
@@ -721,6 +725,7 @@ static void exynos_adc_ts_close(struct input_dev *dev)
{
struct exynos_adc *info = input_get_drvdata(dev);
+ WRITE_ONCE(info->ts_enabled, true);
disable_irq(info->tsirq);
}
--
2.28.0.806.g8561365e88-goog
--
Dmitry
Re: [PATCH] arm/arm64: xen: Fix to convert percpu address to gfn correctly
On Tue, 6 Oct 2020 11:40:58 +0900 Masami Hiramatsu wrote: > On Mon, 5 Oct 2020 18:13:22 -0700 (PDT) > Stefano Stabellini wrote: > > > On Mon, 5 Oct 2020, Julien Grall wrote: > > > Hi Masami, > > > > > > On 05/10/2020 14:39, Masami Hiramatsu wrote: > > > > Use per_cpu_ptr_to_phys() instead of virt_to_phys() for per-cpu > > > > address conversion. > > > > > > > > In xen_starting_cpu(), per-cpu xen_vcpu_info address is converted > > > > to gfn by virt_to_gfn() macro. However, since the virt_to_gfn(v) > > > > assumes the given virtual address is in contiguous kernel memory > > > > area, it can not convert the per-cpu memory if it is allocated on > > > > vmalloc area (depends on CONFIG_SMP). > > > > > > Are you sure about this? I have a .config with CONFIG_SMP=y where the > > > per-cpu > > > region for CPU0 is allocated outside of vmalloc area. > > > > > > However, I was able to trigger the bug as soon as CONFIG_NUMA_BALANCING > > > was > > > enabled. > > > > I cannot reproduce the issue with defconfig, but I can with Masami's > > kconfig. > > > > If I disable just CONFIG_NUMA_BALANCING from Masami's kconfig, the > > problem still appears. > > > > If I disable CONFIG_NUMA from Masami's kconfig, it works, which is > > strange because CONFIG_NUMA is enabled in defconfig, and defconfig > > works. > > Hmm, strange, because when I disabled CONFIG_NUMA_BALANCING, the issue > disappeared. Ah, OK. It depends on NUMA. On arm64, CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK is enabled if CONFIG_NUMA=y. Since per-cpu first chunk has been allocated by memblock if the CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK is enabled(See pcpu_embed_first_chunk()), when the kernel allocate the xen_vcpu_info on the first chunk, it will be in the linear address space. However, if we disable CONFIG_NUMA, it will be on vmalloc page. And if the first chunk has been filled up before initializing xen, the xen_vcpu_info will be allocated on the 2nd chunk which is has been allocated by the backend allocator (kernel memory or vmalloc, depends on CONFIG_SMP). So anyway we have to check it carefully with a special function, which is per_cpu_ptr_to_phys(). Thank you, -- Masami Hiramatsu
Re: [PATCH] rtlwifi: rtl8192se: remove duplicated legacy_httxpowerdiff
On Tue, 2020-10-06 at 11:59 +0800, Chris Chiu wrote: > From: Chris Chiu > > The legacy_httxpowerdiff in rtl8192se is pretty much the same as > the legacy_ht_txpowerdiff for other chips. Use the same name to > keep the consistency. > > Signed-off-by: Chris Chiu > --- > drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c | 2 +- > drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c | 2 +- > drivers/net/wireless/realtek/rtlwifi/wifi.h | 1 - > 3 files changed, 2 insertions(+), 3 deletions(-) Then can't all the struct definitions that include legacy_ht_txpowerdiff other than wifi.h delete it too? $ git grep -P -n '\blegacy_ht_?txpower' -- '*.h' drivers/net/wireless/realtek/rtlwifi/rtl8188ee/phy.h:162: u8 legacy_ht_txpowerdiff; drivers/net/wireless/realtek/rtlwifi/rtl8192c/phy_common.h:155: u8 legacy_ht_txpowerdiff; drivers/net/wireless/realtek/rtlwifi/rtl8723ae/phy.h:140: u8 legacy_ht_txpowerdiff; drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.h:170: u8 legacy_ht_txpowerdiff; drivers/net/wireless/realtek/rtlwifi/wifi.h:1969: u8 legacy_httxpowerdiff;/* Legacy to HT rate power diff */ drivers/net/wireless/realtek/rtlwifi/wifi.h:1980: u8 legacy_ht_txpowerdiff; /*Legacy to HT rate power diff */
Re: [PATCH] arc: include/asm: fix typos of "themselves"
On 10/5/20 8:30 PM, Randy Dunlap wrote: > From: Randy Dunlap > > Fix copy/paste spello of "themselves" in 3 places. > > Signed-off-by: Randy Dunlap > Cc: Vineet Gupta > Cc: linux-snps-...@lists.infradead.org Thx for the fix Randy. Added to for-curr. -Vineet > --- > arch/arc/include/asm/atomic.h |4 ++-- > arch/arc/include/asm/cmpxchg.h |2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > --- lnx-59-rc7.orig/arch/arc/include/asm/atomic.h > +++ lnx-59-rc7/arch/arc/include/asm/atomic.h > @@ -45,7 +45,7 @@ static inline int atomic_##op##_return(i > \ > /* \ >* Explicit full memory barrier needed before/after as \ > - * LLOCK/SCOND thmeselves don't provide any such semantics \ > + * LLOCK/SCOND themselves don't provide any such semantics \ >*/ \ > smp_mb(); \ > \ > @@ -71,7 +71,7 @@ static inline int atomic_fetch_##op(int > \ > /* \ >* Explicit full memory barrier needed before/after as \ > - * LLOCK/SCOND thmeselves don't provide any such semantics \ > + * LLOCK/SCOND themselves don't provide any such semantics \ >*/ \ > smp_mb(); \ > \ > --- lnx-59-rc7.orig/arch/arc/include/asm/cmpxchg.h > +++ lnx-59-rc7/arch/arc/include/asm/cmpxchg.h > @@ -20,7 +20,7 @@ __cmpxchg(volatile void *ptr, unsigned l > > /* >* Explicit full memory barrier needed before/after as > - * LLOCK/SCOND thmeselves don't provide any such semantics > + * LLOCK/SCOND themselves don't provide any such semantics >*/ > smp_mb(); > > >
Re: [PATCH] ARC: SMP: fix typo and use "come up" instead of "comeup"
On 10/5/20 9:12 AM, Mike Rapoport wrote:
> From: Mike Rapoport
>
> When a secondary CPU fails to come up, there is a missing space in the
> log:
>
> Timeout: CPU1 FAILED to comeup !!!
>
> Fix it.
>
> Signed-off-by: Mike Rapoport
Thx for the fix Mike. Added to for-curr.
-Vineet
> ---
> arch/arc/kernel/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> index eca35e02ce06..52906d314537 100644
> --- a/arch/arc/kernel/smp.c
> +++ b/arch/arc/kernel/smp.c
> @@ -226,7 +226,7 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
> }
>
> if (!cpu_online(cpu)) {
> - pr_info("Timeout: CPU%u FAILED to comeup !!!\n", cpu);
> + pr_info("Timeout: CPU%u FAILED to come up !!!\n", cpu);
> return -1;
> }
>
>
[PATCH] rtlwifi: rtl8192se: remove duplicated legacy_httxpowerdiff
From: Chris Chiu
The legacy_httxpowerdiff in rtl8192se is pretty much the same as
the legacy_ht_txpowerdiff for other chips. Use the same name to
keep the consistency.
Signed-off-by: Chris Chiu
---
drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/wifi.h | 1 -
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c
index 81313e0ca834..0cdcddfebca9 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c
@@ -1906,7 +1906,7 @@ static void _rtl92se_read_adapter_info(struct
ieee80211_hw *hw)
* index diff of legacy to HT OFDM rate. */
tempval = hwinfo[EEPROM_RFIND_POWERDIFF] & 0xff;
rtlefuse->eeprom_txpowerdiff = tempval;
- rtlefuse->legacy_httxpowerdiff =
+ rtlefuse->legacy_ht_txpowerdiff =
rtlefuse->txpwr_legacyhtdiff[RF90_PATH_A][0];
RTPRINT(rtlpriv, FINIT, INIT_TXPOWER,
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c
index a37855f57e76..54576566083c 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c
@@ -25,7 +25,7 @@ static void _rtl92s_get_powerbase(struct ieee80211_hw *hw, u8
*p_pwrlevel,
/* We only care about the path A for legacy. */
if (rtlefuse->eeprom_version < 2) {
- pwrbase0 = pwrlevel[0] + (rtlefuse->legacy_httxpowerdiff & 0xf);
+ pwrbase0 = pwrlevel[0] + (rtlefuse->legacy_ht_txpowerdiff &
0xf);
} else {
legacy_pwrdiff = rtlefuse->txpwr_legacyhtdiff
[RF90_PATH_A][chnl - 1];
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h
b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 13421cf2d201..0a516c3c7cea 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -1966,7 +1966,6 @@ struct rtl_efuse {
u8 txpwr_safetyflag;/* Band edge enable flag */
u16 eeprom_txpowerdiff;
- u8 legacy_httxpowerdiff;/* Legacy to HT rate power diff */
u8 antenna_txpwdiff[3];
u8 eeprom_regulatory;
--
2.20.1
[PATCH v10 0/2] Renovate memcpy_mcsafe with copy_mc_to_{user, kernel}
Changes since v9 [1]:
- (Boris) Compile out the copy_mc_fragile() infrastructure in the
CONFIG_X86_MCE=n case.
This had several knock-on effects. The proposed x86: copy_mc_generic()
was internally checking for X86_FEATURE_ERMS and falling back to
copy_mc_fragile(), however that fallback is not possible in the
CONFIG_X86_MCE=n case when copy_mc_fragile() is compiled out. Instead,
copy_mc_to_user() is rewritten similar to copy_user_generic() that walks
through several fallback implementations copy_mc_fragile ->
copy_mc_enhanced_fast_string (new) -> copy_user_generic (no #MC
recovery).
[1]:
http://lore.kernel.org/r/160087928642.3520.17063139768910633998.st...@dwillia2-desk3.amr.corp.intel.com
---
Hi Boris,
I gave this some soak time over the weekend for the robots to chew on
for regressions. No reports, and the updates pass my testing. Please
consider including this in your updates for v5.10, and thanks for
offering to pick this up.
---
The motivations to go rework memcpy_mcsafe() are that the benefit of
doing slow and careful copies is obviated on newer CPUs, and that the
current opt-in list of cpus to instrument recovery is broken relative to
those cpus. There is no need to keep an opt-in list up to date on an
ongoing basis if pmem/dax operations are instrumented for recovery by
default. With recovery enabled by default the old "mcsafe_key" opt-in to
careful copying can be made a "fragile" opt-out. Where the "fragile"
list takes steps to not consume poison across cachelines.
The discussion with Linus made clear that the current "_mcsafe" suffix
was imprecise to a fault. The operations that are needed by pmem/dax are
to copy from a source address that might throw #MC to a destination that
may write-fault, if it is a user page. So copy_to_user_mcsafe() becomes
copy_mc_to_user() to indicate the separate precautions taken on source
and destination. copy_mc_to_kernel() is introduced as a non-SMAP version
that does not expect write-faults on the destination, but is still
prepared to abort with an error code upon taking #MC.
---
Dan Williams (2):
x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user,kernel}()
x86/copy_mc: Introduce copy_mc_enhanced_fast_string()
arch/powerpc/Kconfig |2
arch/powerpc/include/asm/string.h |2
arch/powerpc/include/asm/uaccess.h | 40 +++--
arch/powerpc/lib/Makefile |2
arch/powerpc/lib/copy_mc_64.S |4
arch/x86/Kconfig |2
arch/x86/Kconfig.debug |2
arch/x86/include/asm/copy_mc_test.h| 75 +
arch/x86/include/asm/mce.h |9 +
arch/x86/include/asm/mcsafe_test.h | 75 -
arch/x86/include/asm/string_64.h | 32
arch/x86/include/asm/uaccess.h |9 +
arch/x86/include/asm/uaccess_64.h | 20 --
arch/x86/kernel/cpu/mce/core.c |8 -
arch/x86/kernel/quirks.c | 10 -
arch/x86/lib/Makefile |1
arch/x86/lib/copy_mc.c | 96
arch/x86/lib/copy_mc_64.S | 163
arch/x86/lib/memcpy_64.S | 115 --
arch/x86/lib/usercopy_64.c | 21 ---
drivers/md/dm-writecache.c | 15 +-
drivers/nvdimm/claim.c |2
drivers/nvdimm/pmem.c |6 -
include/linux/string.h |9 -
include/linux/uaccess.h| 13 ++
include/linux/uio.h| 10 +
lib/Kconfig|7 +
lib/iov_iter.c | 48 +++---
tools/arch/x86/include/asm/mcsafe_test.h | 13 --
tools/arch/x86/lib/memcpy_64.S | 115 --
tools/objtool/check.c |5 -
tools/perf/bench/Build |1
tools/perf/bench/mem-memcpy-x86-64-lib.c | 24 ---
tools/testing/nvdimm/test/nfit.c | 49 +++---
.../testing/selftests/powerpc/copyloops/.gitignore |2
tools/testing/selftests/powerpc/copyloops/Makefile |6 -
.../selftests/powerpc/copyloops/copy_mc_64.S |1
.../selftests/powerpc/copyloops/memcpy_mcsafe_64.S |1
38 files changed, 484 insertions(+), 531 deletions(-)
rename arch/powerpc/lib/{memcpy_mcsafe_64.S => copy_mc_64.S} (98%)
create mode 100644 arch/x86/include/asm/copy_mc_test.h
delete mode 100644 arch/x86/include/asm/mcsafe_test.h
create mode 100644 arch/x86/lib/copy_mc.c
create mode 100644 arch/x86/lib/copy_mc_64.S
delete mode 100644
[PATCH v10 2/2] x86/copy_mc: Introduce copy_mc_enhanced_fast_string()
The original copy_mc_fragile() implementation had negative performance
implications since it did not use the fast-string instruction sequence
to perform copies. For this reason copy_mc_to_kernel() fell back to
plain memcpy() to preserve performance on platform that did not indicate
the capability to recover from machine check exceptions. However, that
capability detection was not architectural and now that some platforms
can recover from fast-string consumption of memory errors the memcpy()
fallback now causes these more capable platforms to fail.
Introduce copy_mc_enhanced_fast_string() as the fast default
implementation of copy_mc_to_kernel() and finalize the transition of
copy_mc_fragile() to be a platform quirk to indicate 'copy-carefully'.
With this in place copy_mc_to_kernel() is fast and recovery-ready by
default regardless of hardware capability.
Thanks to Vivek for identifying that copy_user_generic() is not suitable
as the copy_mc_to_user() backend since the #MC handler explicitly checks
ex_has_fault_handler(). Thanks to the 0day robot for catching a
performance bug in the x86/copy_mc_to_user implementation.
Cc: x...@kernel.org
Cc:
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: Vivek Goyal
Cc: "H. Peter Anvin"
Cc: Andy Lutomirski
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Linus Torvalds
Reviewed-by: Tony Luck
Reported-by: Erwin Tsaur
Tested-by: Erwin Tsaur
Reported-by: 0day robot
Fixes: 92b0729c34ca ("x86/mm, x86/mce: Add memcpy_mcsafe()")
Signed-off-by: Dan Williams
---
arch/x86/lib/copy_mc.c| 32 +++-
arch/x86/lib/copy_mc_64.S | 36
tools/objtool/check.c |1 +
3 files changed, 60 insertions(+), 9 deletions(-)
diff --git a/arch/x86/lib/copy_mc.c b/arch/x86/lib/copy_mc.c
index 2633635530b7..c13e8c9ee926 100644
--- a/arch/x86/lib/copy_mc.c
+++ b/arch/x86/lib/copy_mc.c
@@ -45,6 +45,8 @@ void enable_copy_mc_fragile(void)
#define copy_mc_fragile_enabled (0)
#endif
+unsigned long copy_mc_enhanced_fast_string(void *dst, const void *src,
unsigned len);
+
/**
* copy_mc_to_kernel - memory copy that handles source exceptions
*
@@ -52,9 +54,11 @@ void enable_copy_mc_fragile(void)
* @src: source address
* @len: number of bytes to copy
*
- * Call into the 'fragile' version on systems that have trouble
- * actually do machine check recovery. Everyone else can just
- * use memcpy().
+ * Call into the 'fragile' version on systems that benefit from avoiding
+ * corner case poison consumption scenarios, For example, accessing
+ * poison across 2 cachelines with a single instruction. Almost all
+ * other uses case can use copy_mc_enhanced_fast_string() for a fast
+ * recoverable copy, or fallback to plain memcpy.
*
* Return 0 for success, or number of bytes not copied if there was an
* exception.
@@ -63,6 +67,8 @@ unsigned long __must_check copy_mc_to_kernel(void *dst, const
void *src, unsigne
{
if (copy_mc_fragile_enabled)
return copy_mc_fragile(dst, src, len);
+ if (static_cpu_has(X86_FEATURE_ERMS))
+ return copy_mc_enhanced_fast_string(dst, src, len);
memcpy(dst, src, len);
return 0;
}
@@ -72,11 +78,19 @@ unsigned long __must_check copy_mc_to_user(void *dst, const
void *src, unsigned
{
unsigned long ret;
- if (!copy_mc_fragile_enabled)
- return copy_user_generic(dst, src, len);
+ if (copy_mc_fragile_enabled) {
+ __uaccess_begin();
+ ret = copy_mc_fragile(dst, src, len);
+ __uaccess_end();
+ return ret;
+ }
+
+ if (static_cpu_has(X86_FEATURE_ERMS)) {
+ __uaccess_begin();
+ ret = copy_mc_enhanced_fast_string(dst, src, len);
+ __uaccess_end();
+ return ret;
+ }
- __uaccess_begin();
- ret = copy_mc_fragile(dst, src, len);
- __uaccess_end();
- return ret;
+ return copy_user_generic(dst, src, len);
}
diff --git a/arch/x86/lib/copy_mc_64.S b/arch/x86/lib/copy_mc_64.S
index c3b613c4544a..892d8915f609 100644
--- a/arch/x86/lib/copy_mc_64.S
+++ b/arch/x86/lib/copy_mc_64.S
@@ -124,4 +124,40 @@ EXPORT_SYMBOL_GPL(copy_mc_fragile)
_ASM_EXTABLE(.L_write_words, .E_write_words)
_ASM_EXTABLE(.L_write_trailing_bytes, .E_trailing_bytes)
#endif /* CONFIG_X86_MCE */
+
+/*
+ * copy_mc_enhanced_fast_string - memory copy with exception handling
+ *
+ * Fast string copy + fault / exception handling. If the CPU does
+ * support machine check exception recovery, but does not support
+ * recovering from fast-string exceptions then this CPU needs to be
+ * added to the copy_mc_fragile_key set of quirks. Otherwise, absent any
+ * machine check recovery support this version should be no slower than
+ * standard memcpy.
+ */
+SYM_FUNC_START(copy_mc_enhanced_fast_string)
+ movq %rdi, %rax
+ movq %rdx, %rcx
+.L_copy:
+ rep
[PATCH v10 1/2] x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}()
In reaction to a proposal to introduce a memcpy_mcsafe_fast()
implementation Linus points out that memcpy_mcsafe() is poorly named
relative to communicating the scope of the interface. Specifically what
addresses are valid to pass as source, destination, and what faults /
exceptions are handled. Of particular concern is that even though x86
might be able to handle the semantics of copy_mc_to_user() with its
common copy_user_generic() implementation other archs likely need / want
an explicit path for this case:
On Fri, May 1, 2020 at 11:28 AM Linus Torvalds
wrote:
>
> On Thu, Apr 30, 2020 at 6:21 PM Dan Williams
wrote:
> >
> > However now I see that copy_user_generic() works for the wrong reason.
> > It works because the exception on the source address due to poison
> > looks no different than a write fault on the user address to the
> > caller, it's still just a short copy. So it makes copy_to_user() work
> > for the wrong reason relative to the name.
>
> Right.
>
> And it won't work that way on other architectures. On x86, we have a
> generic function that can take faults on either side, and we use it
> for both cases (and for the "in_user" case too), but that's an
> artifact of the architecture oddity.
>
> In fact, it's probably wrong even on x86 - because it can hide bugs -
> but writing those things is painful enough that everybody prefers
> having just one function.
The rename replaces a single top-level memcpy_mcsafe() with either
copy_mc_to_user(), or copy_mc_to_kernel().
An x86 copy_mc_fragile() name is introduced as the rename for the
low-level x86 implementation formerly named memcpy_mcsafe(). It is used
as the slow / careful backend that is supplanted by a fast
copy_mc_generic() in a follow-on patch.
One side-effect of this reorganization is that separating copy_mc_64.S
to its own file means that perf no longer needs to track dependencies
for its memcpy_64.S benchmarks.
Cc: x...@kernel.org
Cc:
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: "H. Peter Anvin"
Cc: Paul Mackerras
Cc: Thomas Gleixner
Cc: Peter Zijlstra
Cc: Mikulas Patocka
Cc: Alexander Viro
Cc: Arnaldo Carvalho de Melo
Cc: Linus Torvalds
Cc: Benjamin Herrenschmidt
Reviewed-by: Tony Luck
Acked-by: Michael Ellerman
Link:
http://lore.kernel.org/r/CAHk-=wjsqtxaqfujxftwnwmgufastgb0dz1dt3v-78quiez...@mail.gmail.com
Signed-off-by: Dan Williams
---
arch/powerpc/Kconfig |2
arch/powerpc/include/asm/string.h |2
arch/powerpc/include/asm/uaccess.h | 40 --
arch/powerpc/lib/Makefile |2
arch/powerpc/lib/copy_mc_64.S |4 -
arch/x86/Kconfig |2
arch/x86/Kconfig.debug |2
arch/x86/include/asm/copy_mc_test.h| 75
arch/x86/include/asm/mce.h |9 +
arch/x86/include/asm/mcsafe_test.h | 75
arch/x86/include/asm/string_64.h | 32 -
arch/x86/include/asm/uaccess.h |9 +
arch/x86/include/asm/uaccess_64.h | 20 ---
arch/x86/kernel/cpu/mce/core.c |8 -
arch/x86/kernel/quirks.c | 10 --
arch/x86/lib/Makefile |1
arch/x86/lib/copy_mc.c | 82 +
arch/x86/lib/copy_mc_64.S | 127
arch/x86/lib/memcpy_64.S | 115 --
arch/x86/lib/usercopy_64.c | 21 ---
drivers/md/dm-writecache.c | 15 +-
drivers/nvdimm/claim.c |2
drivers/nvdimm/pmem.c |6 -
include/linux/string.h |9 -
include/linux/uaccess.h| 13 ++
include/linux/uio.h| 10 +-
lib/Kconfig|7 +
lib/iov_iter.c | 48
tools/arch/x86/include/asm/mcsafe_test.h | 13 --
tools/arch/x86/lib/memcpy_64.S | 115 --
tools/objtool/check.c |4 -
tools/perf/bench/Build |1
tools/perf/bench/mem-memcpy-x86-64-lib.c | 24
tools/testing/nvdimm/test/nfit.c | 49
.../testing/selftests/powerpc/copyloops/.gitignore |2
tools/testing/selftests/powerpc/copyloops/Makefile |6 -
.../selftests/powerpc/copyloops/copy_mc_64.S |1
.../selftests/powerpc/copyloops/memcpy_mcsafe_64.S |1
38 files changed, 433 insertions(+), 531 deletions(-)
rename arch/powerpc/lib/{memcpy_mcsafe_64.S => copy_mc_64.S} (98%)
create mode 100644
linux-next: build failure after merge of the net-next tree
Hi all,
After merging the net-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:
net/xdp/xsk_buff_pool.c:7:10: fatal error: linux/dma-noncoherent.h: No such
file or directory
7 | #include
| ^
Caused by commit
1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
interacting with commit
a3cf4abf ("dma-mapping: merge into
")
from the dma-mapping tree.
I have applied teh following merge fix patch.
From: Stephen Rothwell
Date: Tue, 6 Oct 2020 14:53:30 +1100
Subject: [PATCH] xsk: fix up for "dma-mapping: merge
into "
Signed-off-by: Stephen Rothwell
---
net/xdp/xsk_buff_pool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index e63fadd000db..dbed16648607 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -4,7 +4,7 @@
#include
#include
#include
-#include
+#include
#include
#include "xsk_queue.h"
--
2.28.0
--
Cheers,
Stephen Rothwell
pgpPL4UxZFV8z.pgp
Description: OpenPGP digital signature
Re: linux-next: Fixes tag needs some work in the battery tree
On Mon, Oct 5, 2020 at 7:50 PM Stephen Rothwell wrote:
>
> Hi all,
>
> In commit
>
> f9d293364b45 ("power: supply: sbs-battery: keep error code when
> get_property() fails")
>
> Fixes tag
>
> Fixes: c4f382930145 (power: supply: sbs-battery: don't assume i2c errors as
> battery disconnect)
>
> has these problem(s):
>
> - Target SHA1 does not exist
>
> Maybe you meant
>
> Fixes: 395a7251dc2b ("power: supply: sbs-battery: don't assume i2c errors as
> battery disconnect")
>
Yes, you're right. I guess I made a mistake here.
I'll send a v2 patch.
Thank you!
> --
> Cheers,
> Stephen Rothwell
Re: [ANNOUNCE] Git v2.29.0-rc0
Hi Junio,
Thanks for the release candidate!
Minor comments follow.
On Tue, 6 Oct 2020 at 01:00, Junio C Hamano wrote:
> * The final leg of SHA-256 transition plus doc updates. Note that
>there is no inter-operability between SHA-1 and SHA-256
>repositories yet.
I suspect the dash in "inter-operability" should be dropped.
> * Various callers of run_command API has been modernized.
>(merge afbdba391e jc/run-command-use-embedded-args later to maint).
s/has/have/
> * List of options offered and accepted by "git add -i/-p" were
>inconsistent, which have been corrected.
>(merge ce910287e7 pw/add-p-allowed-options-fix later to maint).
>
> * Various callers of run_command API has been modernized.
>(merge afbdba391e jc/run-command-use-embedded-args later to maint).
Here's that entry again from my previous comment.
> * "git status" has trouble showing where it came from by interpreting
>reflog entries that record certain events, e.g. "checkout @{u}", and
>gives a hard/fatal error. Even though it inherently is impossible
>to give a correct answer because the reflog entries lose some
>information (e.g. "@{u}" does not record what branch the user was
>on hence which branch 'the upstream' needs to be computed, and even
>if the record were available, the relationship between branches may
>have changed), at least hide the error to allow "status" show its
>output.
s/show/to &/ ?
> * There is a logic to estimate how many objects are in the
>repository, which is mean to run once per process invocation, but
s/mean/meant/, I think.
> * The "unshelve" subcommand of "git p4" used incorrectly used
s/used // (without 'g' flag!)
Martin
linux-next: manual merge of the sunxi tree with the arm-soc tree
Hi all,
Today's linux-next merge of the sunxi tree got a conflict in:
arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
between commit:
0dea1794f3b4 ("arm64: allwinner: A100: add the basical Allwinner A100 DTSI
file")
from the arm-soc tree and commit:
7e66a778cb8b ("arm64: allwinner: A100: add the basical Allwinner A100 DTSI
file")
from the sunxi tree.
These are 2 versions of the same patch. For now I am just using the
version in the arm-soc tree ... please sort this out.
I fixed it up (see above) 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
pgpc7aGsy4ImS.pgp
Description: OpenPGP digital signature
[BUG][PATCH] crypto: arm64: Avoid indirect branch to bti_c
The AES code uses a 'br x7' as part of a function called by
a macro. That branch needs a bti_j as a target. This results
in a panic as seen below. Instead of trying to replace the branch
target with a bti_jc, lets replace the indirect branch with a
bl/ret, bl sequence that can target the existing bti_c.
Bad mode in Synchronous Abort handler detected on CPU1, code 0x3403 -- BTI
CPU: 1 PID: 265 Comm: cryptomgr_test Not tainted 5.8.11-300.fc33.aarch64 #1
pstate: 20400c05 (nzCv daif +PAN -UAO BTYPE=j-)
pc : aesbs_encrypt8+0x0/0x5f0 [aes_neon_bs]
lr : aesbs_xts_encrypt+0x48/0xe0 [aes_neon_bs]
sp : 80001052b730
aesbs_encrypt8+0x0/0x5f0 [aes_neon_bs]
__xts_crypt+0xb0/0x2dc [aes_neon_bs]
xts_encrypt+0x28/0x3c [aes_neon_bs]
crypto_skcipher_encrypt+0x50/0x84
simd_skcipher_encrypt+0xc8/0xe0
crypto_skcipher_encrypt+0x50/0x84
test_skcipher_vec_cfg+0x224/0x5f0
test_skcipher+0xbc/0x120
alg_test_skcipher+0xa0/0x1b0
alg_test+0x3dc/0x47c
cryptomgr_test+0x38/0x60
Fixes: commit 0e89640b640d ("crypto: arm64 - Use modern annotations for
assembly functions")
Signed-off-by: Jeremy Linton
---
arch/arm64/crypto/aes-neonbs-core.S | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/crypto/aes-neonbs-core.S
b/arch/arm64/crypto/aes-neonbs-core.S
index b357164379f6..32f53ebe5e2c 100644
--- a/arch/arm64/crypto/aes-neonbs-core.S
+++ b/arch/arm64/crypto/aes-neonbs-core.S
@@ -788,7 +788,7 @@ SYM_FUNC_START_LOCAL(__xts_crypt8)
0: mov bskey, x21
mov rounds, x22
- br x7
+ ret
SYM_FUNC_END(__xts_crypt8)
.macro __xts_crypt, do8, o0, o1, o2, o3, o4, o5, o6, o7
@@ -806,8 +806,8 @@ SYM_FUNC_END(__xts_crypt8)
uzp1v30.4s, v30.4s, v25.4s
ld1 {v25.16b}, [x24]
-99:adr x7, \do8
- bl __xts_crypt8
+99:bl __xts_crypt8
+ bl \do8
ldp q16, q17, [sp, #.Lframe_local_offset]
ldp q18, q19, [sp, #.Lframe_local_offset + 32]
--
2.25.4
Re: [PATCH 13/14] drm/msm: Drop struct_mutex in shrinker path
On Mon, Oct 5, 2020 at 5:44 PM Hillf Danton wrote:
>
>
> On Mon, 5 Oct 2020 18:17:01 Kristian H. Kristensen wrote:
> > On Mon, Oct 5, 2020 at 4:02 PM Daniel Vetter wrote:
> > >
> > > On Mon, Oct 05, 2020 at 05:24:19PM +0800, Hillf Danton wrote:
> > > >
> > > > On Sun, 4 Oct 2020 12:21:45
> > > > > From: Rob Clark
> > > > >
> > > > > Now that the inactive_list is protected by mm_lock, and everything
> > > > > else on per-obj basis is protected by obj->lock, we no longer depend
> > > > > on struct_mutex.
> > > > >
> > > > > Signed-off-by: Rob Clark
> > > > > ---
> > > > > drivers/gpu/drm/msm/msm_gem.c | 1 -
> > > > > drivers/gpu/drm/msm/msm_gem_shrinker.c | 54
> > > > > --
> > > > > 2 files changed, 55 deletions(-)
> > > > >
> > > > [...]
> > > >
> > > > > @@ -71,13 +33,8 @@ msm_gem_shrinker_scan(struct shrinker *shrinker,
> > > > > struct shrink_control *sc)
> > > > > {
> > > > > struct msm_drm_private *priv =
> > > > > container_of(shrinker, struct msm_drm_private, shrinker);
> > > > > - struct drm_device *dev = priv->dev;
> > > > > struct msm_gem_object *msm_obj;
> > > > > unsigned long freed = 0;
> > > > > - bool unlock;
> > > > > -
> > > > > - if (!msm_gem_shrinker_lock(dev, ))
> > > > > - return SHRINK_STOP;
> > > > >
> > > > > mutex_lock(>mm_lock);
> > > >
> > > > Better if the change in behavior is documented that SHRINK_STOP will
> > > > no longer be needed.
> > >
> > > btw I read through this and noticed you have your own obj lock, plus
> > > mutex_lock_nested. I strongly recommend to just cut over to dma_resv_lock
> > > for all object lock needs (soc drivers have been terrible with this
> > > unfortuntaly), and in the shrinker just use dma_resv_trylock instead of
> > > trying to play clever games outsmarting lockdep.
>
> The trylock makes page reclaimers turn to their next target e.g. inode
> cache instead of waiting for the mutex to be released. It makes sense
> for instance in scenarios of mild memory pressure.
is there some behind-the-scenes signalling for this, or is this just
down to what the shrinker callbacks return? Generally when we get
into shrinking, there are a big set of purgable bo's to consider, so
the shrinker callback return wouldn't be considering just one
potentially lock contended bo (buffer object). Ie failing one
trylock, we just move on to the next.
fwiw, what I've seen on the userspace bo cache vs shrinker (anything
that is shrinker potential is in userspace bo cache and
MADV(WONTNEED)) is that in steady state I see a very strong recycling
of bo's (which avoids allocating and mmap'ing or mapping to gpu a new
buffer object), so it is definitely a win in mmap/realloc bandwidth..
in steady state there is a lot of free and realloc of same-sized
buffers from frame to frame.
But in transient situations like moving to new game level when there
is a heavy memory pressure and lots of freeing old
buffers/textures/etc and then allocating new ones, I see shrinker
kicking in hard (in android situations, not so much so with
traditional linux userspace)
BR,
-R
>
> > >
> > > I recently wrote an entire blog length rant on why I think
> > > mutex_lock_nested is too dangerous to be useful:
> > >
> > > https://blog.ffwll.ch/2020/08/lockdep-false-positives.html
> > >
> > > Not anything about this here, just general comment. The problem extends to
> > > shmem helpers and all that also having their own locks for everything.
> >
> > This is definitely a tangible improvement though - very happy to see
> > msm_gem_shrinker_lock() go.
> >
> > Reviewed-by: Kristian H. Kristensen
> >
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > > ___
> > > dri-devel mailing list
> > > dri-de...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Re: [PATCH v3] arm64/mm: add fallback option to allocate virtually contiguous memory
On 10/02/2020 01:46 AM, Sudarshan Rajagopalan wrote:
> When section mappings are enabled, we allocate vmemmap pages from physically
> continuous memory of size PMD_SIZE using vmemmap_alloc_block_buf(). Section
> mappings are good to reduce TLB pressure. But when system is highly fragmented
> and memory blocks are being hot-added at runtime, its possible that such
> physically continuous memory allocations can fail. Rather than failing the
> memory hot-add procedure, add a fallback option to allocate vmemmap pages from
> discontinuous pages using vmemmap_populate_basepages().
>
> Signed-off-by: Sudarshan Rajagopalan
> Cc: Catalin Marinas
> Cc: Will Deacon
> Cc: Anshuman Khandual
> Cc: Mark Rutland
> Cc: Logan Gunthorpe
> Cc: David Hildenbrand
> Cc: Andrew Morton
> Cc: Steven Price
> ---
> arch/arm64/mm/mmu.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 75df62f..11f8639 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1121,8 +1121,15 @@ int __meminit vmemmap_populate(unsigned long start,
> unsigned long end, int node,
> void *p = NULL;
>
> p = vmemmap_alloc_block_buf(PMD_SIZE, node, altmap);
> - if (!p)
> - return -ENOMEM;
> + if (!p) {
> + /*
> + * fallback allocating with virtually
> + * contiguous memory for this section
> + */
Mapping is always virtually contiguous with or without huge pages.
Please drop this comment here, as it's obvious.
> + if (vmemmap_populate_basepages(addr, next,
> node, NULL))
> + return -ENOMEM;
Please send in the 'altmap' instead of NULL for allocation from
device memory if and when requested.
Re: [PATCH] printk: handle blank console arguments passed in.
On 10/5/20 7:59 PM, Sergey Senozhatsky wrote:
> Cc-ing Guenter,
>
> On (20/05/22 12:00), Petr Mladek wrote:
>> On Fri 2020-05-22 16:53:06, Shreyas Joshi wrote:
>>> If uboot passes a blank string to console_setup then it results in a
>>> trashed memory.
>>> Ultimately, the kernel crashes during freeing up the memory. This fix
>>> checks if there
>>> is a blank parameter being passed to console_setup from uboot.
>>> In case it detects that the console parameter is blank then
>>> it doesn't setup the serial device and it gracefully exits.
>>>
>>> Signed-off-by: Shreyas Joshi
>>> ---
>>> V1:
>>> Fixed console_loglevel to default as per the review comments
>>>
>>> kernel/printk/printk.c | 5 -
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>>> index ad4606234545..e9ad730991e0 100644
>>> --- a/kernel/printk/printk.c
>>> +++ b/kernel/printk/printk.c
>>> @@ -2165,7 +2165,10 @@ static int __init console_setup(char *str)
>>> char buf[sizeof(console_cmdline[0].name) + 4]; /* 4 for "ttyS" */
>>> char *s, *options, *brl_options = NULL;
>>> int idx;
>>> -
>>> + if (str[0] == 0) {
>>> + return 1;
>>> + }
>>> if (_braille_console_setup(, _options))
>>> return 1;
>>
>> I have fixed formatting and pushed it into printk/linux.git,
>> branch for-5.8.
>
> Petr, this patch's causing regressions for us. We use blank console= boot
> param to bypass dts. It appears that it'd be better to revert the change.
>
Not just to bypass dts, it was also possible to use console= to disable consoles
passed as config option, as well as other default console options. A quick test
confirms that this affects all platforms/architectures, not just Chromebooks.
Prior to this patch, it was possible to disable a default console with an
empty "console=" parameter. This is no longer possible. This means that
this patch results in a substantial (and, as far as I can see, completely
undiscussed) functionality change.
I don't understand why (yet), but the patch also causes regressions with
seemingly unrelated functionality, specifically with dm-verity on at least
one Chromebook platform. I filed crbug.com/1135157 to track the problem,
and reverted the patch from all our stable releases immediately after
the last round of stable release merges.
On a side note, I don't see the problem presumably fixed with this
patch in any of my tests.
Guenter
Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM
On Mon, 5 Oct 2020 14:47:47 -0300 Jason Gunthorpe wrote: > Andrew please let me know if you need a resend Andrew is rather confused. Can we please identify the minimal patch(es) which are needed for 5.9 and -stable?
[PATCH] arc: include/asm: fix typos of "themselves"
From: Randy Dunlap Fix copy/paste spello of "themselves" in 3 places. Signed-off-by: Randy Dunlap Cc: Vineet Gupta Cc: linux-snps-...@lists.infradead.org --- arch/arc/include/asm/atomic.h |4 ++-- arch/arc/include/asm/cmpxchg.h |2 +- 2 files changed, 3 insertions(+), 3 deletions(-) --- lnx-59-rc7.orig/arch/arc/include/asm/atomic.h +++ lnx-59-rc7/arch/arc/include/asm/atomic.h @@ -45,7 +45,7 @@ static inline int atomic_##op##_return(i \ /* \ * Explicit full memory barrier needed before/after as \ -* LLOCK/SCOND thmeselves don't provide any such semantics \ +* LLOCK/SCOND themselves don't provide any such semantics \ */ \ smp_mb(); \ \ @@ -71,7 +71,7 @@ static inline int atomic_fetch_##op(int \ /* \ * Explicit full memory barrier needed before/after as \ -* LLOCK/SCOND thmeselves don't provide any such semantics \ +* LLOCK/SCOND themselves don't provide any such semantics \ */ \ smp_mb(); \ \ --- lnx-59-rc7.orig/arch/arc/include/asm/cmpxchg.h +++ lnx-59-rc7/arch/arc/include/asm/cmpxchg.h @@ -20,7 +20,7 @@ __cmpxchg(volatile void *ptr, unsigned l /* * Explicit full memory barrier needed before/after as -* LLOCK/SCOND thmeselves don't provide any such semantics +* LLOCK/SCOND themselves don't provide any such semantics */ smp_mb();
Re: [Freedreno] [PATCH 00/14] drm/msm: de-struct_mutex-ification
On Mon, Oct 5, 2020 at 11:20 AM Daniel Vetter wrote: > > On Mon, Oct 5, 2020 at 6:24 PM Kristian Høgsberg wrote: > > > > On Sun, Oct 4, 2020 at 9:21 PM Rob Clark wrote: > > > > > > From: Rob Clark > > > > > > This doesn't remove *all* the struct_mutex, but it covers the worst > > > of it, ie. shrinker/madvise/free/retire. The submit path still uses > > > struct_mutex, but it still needs *something* serialize a portion of > > > the submit path, and lock_stat mostly just shows the lock contention > > > there being with other submits. And there are a few other bits of > > > struct_mutex usage in less critical paths (debugfs, etc). But this > > > seems like a reasonable step in the right direction. > > > > What a great patch set. Daniel has some good points and nothing that > > requires big changes, but on the other hand, I'm not sure it's > > something that needs to block this set either. > > Personally I'd throw the lockdep priming on top to make sure this > stays correct (it's 3 lines), but yes imo this is all good to go. Just > figured I'll sprinkle the latest in terms of gem locking over the > series while it's here :-) Yeah, I'll defn throw the lockdep priming into v2.. and I've got using obj->resv for locking on the todo list but looks like enough churn that it will probably be it's own series (but seems like there is room to introduce some lock/unlock helpers that don't really change anything but make an obj->lock transition easier) BR, -R > -Daniel > > > Either way, for the series > > > > Reviewed-by: Kristian H. Kristensen > > > > > Rob Clark (14): > > > drm/msm: Use correct drm_gem_object_put() in fail case > > > drm/msm: Drop chatty trace > > > drm/msm: Move update_fences() > > > drm/msm: Add priv->mm_lock to protect active/inactive lists > > > drm/msm: Document and rename preempt_lock > > > drm/msm: Protect ring->submits with it's own lock > > > drm/msm: Refcount submits > > > drm/msm: Remove obj->gpu > > > drm/msm: Drop struct_mutex from the retire path > > > drm/msm: Drop struct_mutex in free_object() path > > > drm/msm: remove msm_gem_free_work > > > drm/msm: drop struct_mutex in madvise path > > > drm/msm: Drop struct_mutex in shrinker path > > > drm/msm: Don't implicit-sync if only a single ring > > > > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- > > > drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 +-- > > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 +- > > > drivers/gpu/drm/msm/msm_debugfs.c | 7 ++ > > > drivers/gpu/drm/msm/msm_drv.c | 15 +--- > > > drivers/gpu/drm/msm/msm_drv.h | 19 +++-- > > > drivers/gpu/drm/msm/msm_gem.c | 76 ++ > > > drivers/gpu/drm/msm/msm_gem.h | 53 + > > > drivers/gpu/drm/msm/msm_gem_shrinker.c| 58 ++ > > > drivers/gpu/drm/msm/msm_gem_submit.c | 17 ++-- > > > drivers/gpu/drm/msm/msm_gpu.c | 96 ++- > > > drivers/gpu/drm/msm/msm_gpu.h | 5 +- > > > drivers/gpu/drm/msm/msm_ringbuffer.c | 3 +- > > > drivers/gpu/drm/msm/msm_ringbuffer.h | 13 ++- > > > 14 files changed, 188 insertions(+), 194 deletions(-) > > > > > > -- > > > 2.26.2 > > > > > > ___ > > > Freedreno mailing list > > > freedr...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/freedreno > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Tue, 2020-10-06 at 00:34 +, Joel Stanley wrote: > arch/powerpc/boot is the powerpc wrapper, and it's not built with the > same includes or flags as the rest of the kernel. It doesn't include > any of the headers in the top level include/ directory for hysterical > raisins. > > The straightforward fix would be to exclude this directory from your script. I agree and that's why I submitted another script that does just that. https://lore.kernel.org/lkml/75393e5ddc272dc7403de74d645e6c6e0f4e70eb.ca...@perches.com/
RE: [PATCH] media: ov2740: select regmap
Sergey, thanks for your patch.
Reviewed-by: Bingbu Cao
BRs,
Bingbu Cao
> -Original Message-
> From: Sergey Senozhatsky
> Sent: Wednesday, September 30, 2020 9:13 AM
> To: Mauro Carvalho Chehab ; Cao, Bingbu
>
> Cc: Sakari Ailus ; linux-me...@vger.kernel.org;
> linux-kernel@vger.kernel.org; Sergey Senozhatsky
>
> Subject: [PATCH] media: ov2740: select regmap
>
> Fix OV2740 build breakage by selecting REGMAP_I2C config:
>
> ov2740.c:1011:23: error: variable has incomplete type 'struct regmap_config'
> struct regmap_config regmap_config = { };
> ^
> ov2740.c:1011:9: note: forward declaration of 'struct regmap_config'
> struct regmap_config regmap_config = { };
>^
> ov2740.c:1028:11: error: implicit declaration of function
> 'devm_regmap_init_i2c'
> regmap = devm_regmap_init_i2c(client, _config);
>
> Signed-off-by: Sergey Senozhatsky
> ---
> drivers/media/i2c/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index
> 878f66ef2719..c64326ca331c 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -877,6 +877,7 @@ config VIDEO_OV2740
> select MEDIA_CONTROLLER
> select VIDEO_V4L2_SUBDEV_API
> select V4L2_FWNODE
> + select REGMAP_I2C
> help
> This is a Video4Linux2 sensor driver for the OmniVision
> OV2740 camera.
> --
> 2.28.0
Re: ext4 regression in v5.9-rc2 from e7bfb5c9bb3d on ro fs with overlapped bitmaps
On Mon, Oct 05, 2020 at 07:51:10PM -0700, Darrick J. Wong wrote: > > > Could /somebody/ please document the ondisk format changes that are > > > associated with this feature? > > > > I pretty much had to sort it out by looking at a combination of > > e2fsprogs and the kernel, and a lot of experimentation, until I ended up > > with something that the kernel was completely happy with without a > > single complaint. > > > > I'd be happy to write up a summary of the format. > > Seems like a good idea, particularly since you're asking for a format > change that requires kernel support and the ondisk format documentation > lives under Documentation/. That said... > > If you set up the rest of the metadata consistently with it (for > > instance, 0 free blocks and 0 free inodes), you'll only get a single > > complaint, from the e2fsck equivalent of block_validity. See > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956509 for details on > > that; > > ...Ted shot down this whole thing six months ago. > > The Debian bug database is /not/ the designated forum to discuss changes > to the ondisk format; linux-ext4 is. What Josh is proposing I'm pretty sure would also break "e2fsck -E unshare_blocks", so that's another reason not to accept this as a valid format change. As far as I'm concerned, contrib/e2fsdroid is the canonical definition of how to create valid file systems with shared_blocks. - Ted
Re: [PATCH V4 3/3] arm64/mm/hotplug: Ensure early memory sections are all online
On 10/01/2020 06:23 AM, Gavin Shan wrote:
> Hi Anshuman,
>
> On 9/29/20 11:54 PM, Anshuman Khandual wrote:
>> This adds a validation function that scans the entire boot memory and makes
>> sure that all early memory sections are online. This check is essential for
>> the memory notifier to work properly, as it cannot prevent any boot memory
>> from offlining, if all sections are not online to begin with. The notifier
>> registration is skipped, if this validation does not go through. Although
>> the boot section scanning is selectively enabled with DEBUG_VM.
>>
>> Cc: Catalin Marinas
>> Cc: Will Deacon
>> Cc: Mark Rutland
>> Cc: Marc Zyngier
>> Cc: Steve Capper
>> Cc: Mark Brown
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual
>> ---
>> arch/arm64/mm/mmu.c | 59 +
>> 1 file changed, 59 insertions(+)
>
> I don't understand why this is necessary. The core already ensure the
> corresponding section is online when trying to offline it. It's guranteed
> that section is online when the notifier is triggered. I'm not sure if
> there is anything I missed?
Current memory notifier blocks any boot memory hot removal attempt via
blocking its offlining step itself. So if some sections in boot memory
are not online (because of a bug or change in init sequence) by the
time memory block device can be removed, the notifier loses the ability
to prevent its removal. This validation here, ensures that entire boot
memory is in online state, otherwise call out sections that are not,
with an warning that those boot memory can be removed.
>
>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 90a30f5ebfc0..b67a657ea1ad 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1522,6 +1522,62 @@ static struct notifier_block
>> prevent_bootmem_remove_nb = {
>> .notifier_call = prevent_bootmem_remove_notifier,
>> };
>> +/*
>> + * This ensures that boot memory sections on the plaltform are online
Will fix.
> ^
>> + * during early boot. They could not be prevented from being offlined
>> + * if for some reason they are not brought online to begin with. This
>> + * help validate the basic assumption on which the above memory event
>> + * notifier works to prevent boot memory offlining and it's possible
>> + * removal.
>> + */
>> +static bool validate_bootmem_online(void)
>> +{
>> + struct memblock_region *mblk;
>> + struct mem_section *ms;
>> + unsigned long pfn, end_pfn, start, end;
>> + bool all_online = true;
>> +
>> + /*
>> + * Scanning across all memblock might be expensive
>> + * on some big memory systems. Hence enable this
>> + * validation only with DEBUG_VM.
>> + */
>> + if (!IS_ENABLED(CONFIG_DEBUG_VM))
>> + return all_online;
>> +
>> + for_each_memblock(memory, mblk) {
>> + pfn = PHYS_PFN(mblk->base);
>> + end_pfn = PHYS_PFN(mblk->base + mblk->size);
>> +
>
> It's not a good idea to access @mblk->{base, size}. There are two
> accessors: memblock_region_memory_{base, end}_pfn().
Sure, will replace.
>
>> + for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
>> + ms = __pfn_to_section(pfn);
>> +
>> + /*
>> + * All memory ranges in the system at this point
>> + * should have been marked early sections.
>> + */
>> + WARN_ON(!early_section(ms));
>> +
>> + /*
>> + * Memory notifier mechanism here to prevent boot
>> + * memory offlining depends on the fact that each
>> + * early section memory on the system is intially
>> + * online. Otherwise a given memory section which
>> + * is already offline will be overlooked and can
>> + * be removed completely. Call out such sections.
>> + */
>
> s/intially/initially
Will change.
>
>> + if (!online_section(ms)) {
>> + start = PFN_PHYS(pfn);
>> + end = start + (1UL << PA_SECTION_SHIFT);
>> + pr_err("Memory range [%lx %lx] is offline\n", start, end);
>> + pr_err("Memory range [%lx %lx] can be removed\n", start,
>> end);
>> + all_online = false;
>
> These two error messages can be combined:
>
> pr_err("Memory range [%lx %lx] not online, can't be offlined\n",
> start, end);
Will change but it is actually s/can't be offlined/can be removed/ instead.
>
> I think you need to return @all_online immediately, without
> checking if the subsequent sections are online or not? :)
Thinking about this again. It might be better if the notifier registration
does not depend on return value from validate_bootmem_online(). Instead it
should proceed either way but after calling out all boot memory sections
that are not online. In that case
Re: [PATCH] printk: handle blank console arguments passed in.
Cc-ing Guenter,
On (20/05/22 12:00), Petr Mladek wrote:
> On Fri 2020-05-22 16:53:06, Shreyas Joshi wrote:
> > If uboot passes a blank string to console_setup then it results in a
> > trashed memory.
> > Ultimately, the kernel crashes during freeing up the memory. This fix
> > checks if there
> > is a blank parameter being passed to console_setup from uboot.
> > In case it detects that the console parameter is blank then
> > it doesn't setup the serial device and it gracefully exits.
> >
> > Signed-off-by: Shreyas Joshi
> > ---
> > V1:
> > Fixed console_loglevel to default as per the review comments
> >
> > kernel/printk/printk.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index ad4606234545..e9ad730991e0 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2165,7 +2165,10 @@ static int __init console_setup(char *str)
> > char buf[sizeof(console_cmdline[0].name) + 4]; /* 4 for "ttyS" */
> > char *s, *options, *brl_options = NULL;
> > int idx;
> > -
> > + if (str[0] == 0) {
> > + return 1;
> > + }
> > if (_braille_console_setup(, _options))
> > return 1;
>
> I have fixed formatting and pushed it into printk/linux.git,
> branch for-5.8.
Petr, this patch's causing regressions for us. We use blank console= boot
param to bypass dts. It appears that it'd be better to revert the change.
-ss
Re: [PATCH V4 2/3] arm64/mm/hotplug: Enable MEM_OFFLINE event handling
On 10/01/2020 05:27 AM, Gavin Shan wrote:
> Hi Anshuman,
>
> On 9/29/20 11:54 PM, Anshuman Khandual wrote:
>> This enables MEM_OFFLINE memory event handling. It will help intercept any
>> possible error condition such as if boot memory some how still got offlined
>> even after an explicit notifier failure, potentially by a future change in
>> generic hot plug framework. This would help detect such scenarios and help
>> debug further. While here, also call out the first section being attempted
>> for offline or got offlined.
>>
>> Cc: Catalin Marinas
>> Cc: Will Deacon
>> Cc: Mark Rutland
>> Cc: Marc Zyngier
>> Cc: Steve Capper
>> Cc: Mark Brown
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual
>> ---
>> arch/arm64/mm/mmu.c | 29 +++--
>> 1 file changed, 27 insertions(+), 2 deletions(-)
>>
>
> This looks good to me except a nit and it can be improved if
> that looks reasonable and only when you get a chance for
> respin.
>
> Reviewed-by: Gavin Shan
>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 4e70f4fea06c..90a30f5ebfc0 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1482,13 +1482,38 @@ static int prevent_bootmem_remove_notifier(struct
>> notifier_block *nb,
>> unsigned long end_pfn = arg->start_pfn + arg->nr_pages;
>> unsigned long pfn = arg->start_pfn;
>> - if (action != MEM_GOING_OFFLINE)
>> + if ((action != MEM_GOING_OFFLINE) && (action != MEM_OFFLINE))
>> return NOTIFY_OK;
>> for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
>> + unsigned long start = PFN_PHYS(pfn);
>> + unsigned long end = start + (1UL << PA_SECTION_SHIFT);
>> +
>> ms = __pfn_to_section(pfn);
>> - if (early_section(ms))
>> + if (!early_section(ms))
>> + continue;
>> +
>
> The discussion here is irrelevant to this patch itself. It seems
> early_section() is coarse, which means all memory detected during
> boot time won't be hotpluggable?
Right, thats the policy being enforced on arm64 platform for various
critical reasons. Please refer to earlier discussions around memory
hot remove development on arm64.
>
>> + if (action == MEM_GOING_OFFLINE) {
>> + pr_warn("Boot memory [%lx %lx] offlining attempted\n", start,
>> end);
>> return NOTIFY_BAD;
>> + } else if (action == MEM_OFFLINE) {
>> + /*
>> + * This should have never happened. Boot memory
>> + * offlining should have been prevented by this
>> + * very notifier. Probably some memory removal
>> + * procedure might have changed which would then
>> + * require further debug.
>> + */
>> + pr_err("Boot memory [%lx %lx] offlined\n", start, end);
>> +
>> + /*
>> + * Core memory hotplug does not process a return
>> + * code from the notifier for MEM_OFFLINE event.
>> + * Error condition has been reported. Report as
>> + * ignored.
>> + */
>> + return NOTIFY_DONE;
>> + }
>> }
>> return NOTIFY_OK;
>> }
>>
>
> I think NOTIFY_BAD is returned for MEM_OFFLINE wouldn't be a
> bad idea, even the core isn't handling the errno. With this,
> the code can be simplified. However, it's not a big deal and
> you probably evaluate and change when you need another respin:
>
> pr_warn("Boot memory [%lx %lx] %s\n",
> (action == MEM_GOING_OFFLINE) ? "offlining attempted" :
> "offlined",
> start, end);
> return NOTIFY_BAD;
Wondering whether returning a NOTIFY_BAD for MEM_OFFLINE event could
be somewhat risky if generic hotplug mechanism to change later. But
again, probably it might just be OK.
Regardless, also wanted to differentiate error messages for both the
cases. An warning messages i.e pr_warn() for MEM_GOING_OFFLINE which
suggests an unexpected user action but an error message i.e pr_err()
for MEM_OFFLINE which clearly indicates an error condition that needs
to be debugged further.
Re: [RFC V2] mm/vmstat: Add events for HugeTLB migration
On 10/05/2020 11:35 AM, Michal Hocko wrote: > On Mon 05-10-20 07:59:12, Anshuman Khandual wrote: >> >> >> On 10/02/2020 05:34 PM, Michal Hocko wrote: >>> On Wed 30-09-20 11:30:49, Anshuman Khandual wrote: Add following new vmstat events which will track HugeTLB page migration. 1. HUGETLB_MIGRATION_SUCCESS 2. HUGETLB_MIGRATION_FAILURE It follows the existing semantics to accommodate HugeTLB subpages in total page migration statistics. While here, this updates current trace event 'mm_migrate_pages' to accommodate now available HugeTLB based statistics. >>> What is the actual usecase? And how do you deal with the complexity >>> introduced by many different hugetlb page sizes. Really, what is the >>> point to having such a detailed view on hugetlb migration? >>> >> >> It helps differentiate various page migration events i.e normal, THP and >> HugeTLB and gives us more reliable and accurate measurement. Current stats >> as per PGMIGRATE_SUCCESS and PGMIGRATE_FAIL are misleading, as they contain >> both normal and HugeTLB pages as single entities, which is not accurate. > > Yes this is true. But why does it really matter? Do you have a specific > example? An example which demonstrates that mixing and misrepresentation in these stats create some problem ? Well, we could just create one scenario via an application with different VMA types and triggering some migrations. But the fact remains, that these stats are inaccurate and misleading which is very clear and apparent. > >> After this change, PGMIGRATE_SUCCESS and PGMIGRATE_FAIL will contain page >> migration statistics in terms of normal pages irrespective of whether any >> previous migrations until that point involved normal pages, THP or HugeTLB >> (any size) pages. At the least, this fixes existing misleading stats with >> PGMIGRATE_SUCCESS and PGMIGRATE_FAIL. >> >> Besides, it helps us understand HugeTLB migrations in more detail. Even >> though HugeTLB can be of various sizes on a given platform, these new >> stats HUGETLB_MIGRATION_SUCCESS and HUGETLB_MIGRATION_FAILURE give enough >> overall insight into HugeTLB migration events. > > While true this all is way too vague to add yet another imprecise > counter. Given that user knows about all HugeTLB mappings it has got, these counters are not really vague and could easily be related back. Moreover this change completes the migration stats restructuring which was started with adding THP counters. Otherwise it remains incomplete.
Re: [PATCH v39 21/24] x86/vdso: Implement a vDSO for Intel SGX enclave call
On Sat, Oct 03, 2020 at 07:50:56AM +0300, Jarkko Sakkinen wrote:
> From: Sean Christopherson
> + /* Validate that the reserved area contains only zeros. */
> + push%rax
> + push%rbx
> + mov $SGX_ENCLAVE_RUN_RESERVED_START, %rbx
> +1:
> + mov (%rcx, %rbx), %rax
> + cmpq$0, %rax
> + jne .Linvalid_input
> +
> + add $8, %rbx
> + cmpq$SGX_ENCLAVE_RUN_RESERVED_END, %rbx
> + jne 1b
> + pop %rbx
> + pop %rax
This can more succinctly be (untested):
movqSGX_ENCLAVE_RUN_RESERVED_1(%rbp), %rbx
orq SGX_ENCLAVE_RUN_RESERVED_2(%rbp), %rbx
orq SGX_ENCLAVE_RUN_RESERVED_3(%rbp), %rbx
jnz .Linvalid_input
Note, %rbx is getting clobbered anyways, no need to save/restore it.
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> index b6ba036a9b82..3dd2df44d569 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -74,4 +74,102 @@ struct sgx_enclave_provision {
> __u64 attribute_fd;
> };
>
> +struct sgx_enclave_run;
> +
> +/**
> + * typedef sgx_enclave_user_handler_t - Exit handler function accepted by
> + * __vdso_sgx_enter_enclave()
> + * @run: Pointer to the caller provided struct sgx_enclave_run
> + *
> + * The register parameters contain the snapshot of their values at enclave
> + * exit
> + *
> + * Return:
> + * 0 or negative to exit vDSO
> + * positive to re-enter enclave (must be EENTER or ERESUME leaf)
> + */
> +typedef int (*sgx_enclave_user_handler_t)(long rdi, long rsi, long rdx,
> + long rsp, long r8, long r9,
> + struct sgx_enclave_run *run);
> +
> +/**
> + * struct sgx_enclave_run - the execution context of
> __vdso_sgx_enter_enclave()
> + * @tcs: TCS used to enter the enclave
> + * @user_handler:User provided callback run on exception
> + * @user_data: Data passed to the user handler
> + * @leaf:The ENCLU leaf we were at (EENTER/ERESUME/EEXIT)
> + * @exception_vector:The interrupt vector of the exception
> + * @exception_error_code:The exception error code pulled out of the stack
> + * @exception_addr: The address that triggered the exception
> + * @reserved Reserved for possible future use
> + */
> +struct sgx_enclave_run {
> + __u64 tcs;
> + __u64 user_handler;
> + __u64 user_data;
> + __u32 leaf;
I am still very strongly opposed to omitting exit_reason. It is not at all
difficult to imagine scenarios where 'leaf' alone is insufficient for the
caller or its handler to deduce why the CPU exited the enclave. E.g. see
Jethro's request for intercepting interrupts.
I don't buy the argument that the N bytes needed for the exit_reason are at
all expensive.
> + __u16 exception_vector;
> + __u16 exception_error_code;
> + __u64 exception_addr;
> + __u8 reserved[24];
I also think it's a waste of space to bother with multiple reserved fields.
24 bytes isn't so much that it guarantees we'll never run into problems in
the future. But I care far less about this than I do about exit_reason.
> +};
Re: ext4 regression in v5.9-rc2 from e7bfb5c9bb3d on ro fs with overlapped bitmaps
On Mon, Oct 05, 2020 at 05:32:16PM -0700, Josh Triplett wrote:
> On Mon, Oct 05, 2020 at 10:36:39AM -0700, Darrick J. Wong wrote:
> > On Mon, Oct 05, 2020 at 01:14:54AM -0700, Josh Triplett wrote:
> > > Ran into an ext4 regression when testing upgrades to 5.9-rc kernels:
> > >
> > > Commit e7bfb5c9bb3d ("ext4: handle add_system_zone() failure in
> > > ext4_setup_system_zone()") breaks mounting of read-only ext4 filesystems
> > > with intentionally overlapping bitmap blocks.
> > >
> > > On an always-read-only filesystem explicitly marked with
> > > EXT4_FEATURE_RO_COMPAT_SHARED_BLOCKS, prior to that commit, it's safe to
> > > point all the block and inode bitmaps to a single block
> >
> > LOL, WHAT?
> >
> > I didn't know shared blocks applied to fs metadata. I thought that
> > "shared" only applied to file extent maps being able to share physical
> > blocks.
>
> The flag isn't documented very well yet, but since it specifically
> forces the filesystem to always be mounted read-only, the bitmaps really
> shouldn't matter at all. (In an ideal world, a permanently read-only
> filesystem should be able to omit all the bitmaps entirely. Pointing
> them all to a single disk block is the next best thing.)
I disagree; creating undocumented forks of an existing ondisk format
(especially one that presents as inconsistent metadata to regular tools)
is creating a ton of pain for future users and maintainers when the
incompat forks collide with the canonical implementation(s).
(Granted, I don't know if you /created/ this new interpretation of the
feature flag or if you've merely been stuck with it...)
I don't say that as a theoretical argument -- you've just crashed right
into this, because nobody knew that the in-kernel block validity doesn't
know how to deal with this other than to error out.
> > Could /somebody/ please document the ondisk format changes that are
> > associated with this feature?
>
> I pretty much had to sort it out by looking at a combination of
> e2fsprogs and the kernel, and a lot of experimentation, until I ended up
> with something that the kernel was completely happy with without a
> single complaint.
>
> I'd be happy to write up a summary of the format.
Seems like a good idea, particularly since you're asking for a format
change that requires kernel support and the ondisk format documentation
lives under Documentation/. That said...
> I'd still really like to see this patch applied for 5.9, to avoid having
> filesystems that an old kernel can mount but a new one can't. This still
> seems like a regression to me.
>
> > > of all 1s,
> > > because a read-only filesystem will never allocate or free any blocks or
> > > inodes.
> >
> > All 1s? So the inode bitmap says that every inode table slot is in use,
> > even if the inode record itself says it isn't?
>
> Yes.
>
> > What does e2fsck -n
> > think about that kind of metadata inconsistency?
>
> If you set up the rest of the metadata consistently with it (for
> instance, 0 free blocks and 0 free inodes), you'll only get a single
> complaint, from the e2fsck equivalent of block_validity. See
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956509 for details on
> that;
...Ted shot down this whole thing six months ago.
The Debian bug database is /not/ the designated forum to discuss changes
to the ondisk format; linux-ext4 is.
--D
> with that fixed, e2fsck wouldn't complain at all. The kernel,
> prior to 5.9-rc2, doesn't have a single complaint, whether at mount,
> unmount, or read of every file and directory on the filesystem.
>
> The errors you got in your e2fsck run came because you just overrode the
> bitmaps, but didn't make the rest of the metadata consistent with that.
> I can provide a sample filesystem if that would help.
>
> - Josh
Re: [PATCH v3] bluetooth: hci_h5: fix memory leak in h5_close
On 05-10-2020 14:48, Hans de Goede wrote: > To fully fix the memleak you also need to add a kfree_skb(h5->rx_skb); > call to the end of h5_serdev_remove(), because in the hu->serdev case > that is where the h5 struct will be free-ed (it is free-ed after that > function exits). Hi Hans, I'm not entirely convinced that it might be entirely the best idea to do that. * The bug detected by syzbot only provides us with reproducer and information about this bug (which gets triggered when !hu->serdev). Even if like you said, there might be a memory leak unattended to when hu->serdev exists, then this might not necessarily be the right place to fix it. * From what I can see, all the drivers that have modified to provide serdev support have different close() mechanisms. However, one thing they do have in common (in this context) is that their respective serdev_remove() function simply calls hci_uart_unregister_device() to unregister the device. It is primarily for this reason that I feel adding a kfree_skb() call at the end of h5_serdev_remove() might not exactly be the best way we could solve this (and since this hasn't been picked up by syzbot yet, there's no way to know if this just fixes things or ends up causing unforeseen complications). Alternatively, wouldn't freeing h5->rx_skb and assigning it to NULL, for both hu->serdev and !hu->serdev cases within h5_close() itself be a better approach? I've also taken the liberty of testing a patch that does this, and it seems to work correctly too. :) But then again, I'm not exactly an authority on how this works. Thanks, Anant
sysfs filenames with spaces
This doesn't seem like a great idea to me. For my system I've got: /sys/devices/platform/Fixed MDIO bus.0/ /sys/bus/platform/drivers/int3401 thermal/ /sys/bus/platform/drivers/int3403 thermal/ /sys/bus/platform/drivers/int3400 thermal/ /sys/bus/mdio_bus/drivers/Generic PHY/ /sys/bus/mdio_bus/drivers/Generic Clause 45 PHY/ /sys/bus/pnp/drivers/i8042 aux/ /sys/bus/pnp/drivers/i8042 kbd/ /sys/bus/i2c/drivers/CHT Whiskey Cove PMIC/ Could these filenames be avoided in the future or even renamed today?
Re: [PATCH] arm/arm64: xen: Fix to convert percpu address to gfn correctly
On Mon, 5 Oct 2020 18:13:22 -0700 (PDT)
Stefano Stabellini wrote:
> On Mon, 5 Oct 2020, Julien Grall wrote:
> > Hi Masami,
> >
> > On 05/10/2020 14:39, Masami Hiramatsu wrote:
> > > Use per_cpu_ptr_to_phys() instead of virt_to_phys() for per-cpu
> > > address conversion.
> > >
> > > In xen_starting_cpu(), per-cpu xen_vcpu_info address is converted
> > > to gfn by virt_to_gfn() macro. However, since the virt_to_gfn(v)
> > > assumes the given virtual address is in contiguous kernel memory
> > > area, it can not convert the per-cpu memory if it is allocated on
> > > vmalloc area (depends on CONFIG_SMP).
> >
> > Are you sure about this? I have a .config with CONFIG_SMP=y where the
> > per-cpu
> > region for CPU0 is allocated outside of vmalloc area.
> >
> > However, I was able to trigger the bug as soon as CONFIG_NUMA_BALANCING was
> > enabled.
>
> I cannot reproduce the issue with defconfig, but I can with Masami's
> kconfig.
>
> If I disable just CONFIG_NUMA_BALANCING from Masami's kconfig, the
> problem still appears.
>
> If I disable CONFIG_NUMA from Masami's kconfig, it works, which is
> strange because CONFIG_NUMA is enabled in defconfig, and defconfig
> works.
Hmm, strange, because when I disabled CONFIG_NUMA_BALANCING, the issue
disappeared.
--- config-5.9.0-rc4+ 2020-10-06 11:36:20.620107129 +0900
+++ config-5.9.0-rc4+.buggy 2020-10-05 21:04:40.369936461 +0900
@@ -131,7 +131,8 @@
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_CC_HAS_INT128=y
CONFIG_ARCH_SUPPORTS_INT128=y
-# CONFIG_NUMA_BALANCING is not set
+CONFIG_NUMA_BALANCING=y
+CONFIG_NUMA_BALANCING_DEFAULT_ENABLED=y
CONFIG_CGROUPS=y
CONFIG_PAGE_COUNTER=y
CONFIG_MEMCG=y
So buggy config just enabled NUMA_BALANCING (and default enabled)
> > [...]
> >
> > > Fixes: 250c9af3d831 ("arm/xen: Add support for 64KB page granularity")
> >
> > FWIW, I think the bug was already present before 250c9af3d831.
>
> Yeah, I bet 250c9af3d831 is not what introduced the issue. Whatever
> caused virt_to_phys to stop working on vmalloc'ed addresses is the cause
> of the problem. It is something that went in 5.9 (5.8 works) but I don't
> know what for sure.
OK.
>
>
> > > Signed-off-by: Masami Hiramatsu
> > > ---
> > > arch/arm/xen/enlighten.c |2 +-
> > > include/xen/arm/page.h |3 +++
> > > 2 files changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > > index e93145d72c26..a6ab3689b2f4 100644
> > > --- a/arch/arm/xen/enlighten.c
> > > +++ b/arch/arm/xen/enlighten.c
> > > @@ -150,7 +150,7 @@ static int xen_starting_cpu(unsigned int cpu)
> > > pr_info("Xen: initializing cpu%d\n", cpu);
> > > vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> > > - info.mfn = virt_to_gfn(vcpup);
> > > + info.mfn = percpu_to_gfn(vcpup);
> > > info.offset = xen_offset_in_page(vcpup);
> > > err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info,
> > > xen_vcpu_nr(cpu),
> > > diff --git a/include/xen/arm/page.h b/include/xen/arm/page.h
> > > index 39df751d0dc4..ac1b65470563 100644
> > > --- a/include/xen/arm/page.h
> > > +++ b/include/xen/arm/page.h
> > > @@ -83,6 +83,9 @@ static inline unsigned long bfn_to_pfn(unsigned long
> > > bfn)
> > > })
> > > #define gfn_to_virt(m) (__va(gfn_to_pfn(m) <<
> > > XEN_PAGE_SHIFT))
> > > +#define percpu_to_gfn(v) \
> > > + (pfn_to_gfn(per_cpu_ptr_to_phys(v) >> XEN_PAGE_SHIFT))
> > > +
> > > /* Only used in PV code. But ARM guests are always HVM. */
> > > static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
> > > {
>
>
> The fix is fine for me. I tested it and it works. We need to remove the
> "Fixes:" line from the commit message. Ideally, replacing it with a
> reference to what is the source of the problem.
OK, as I said, it seems commit 9a9ab3cc00dc ("xen/arm: SMP support") has
introduced the per-cpu code. So note it instead of Fixes tag.
>
> Aside from that:
>
> Reviewed-by: Stefano Stabellini
Thank you!
--
Masami Hiramatsu
Re: [PATCH] perf inject: Flush ordered events on FINISHED_ROUND
> > On Fri, Oct 02, 2020 at 10:03:17PM +0900, Namhyung Kim wrote: > > > Below measures time and memory usage during the perf inject and > > > report using ~190MB data file. > > > > > > Before: > > > perf inject: 11.09 s, 382148 KB > > > perf report: 8.05 s, 397440 KB > > > > > > After: > > > perf inject: 16.24 s, 83376 KB > > > perf report: 7.96 s, 216184 KB > > > > > > As you can see, it used 2x memory of the input size. I guess it's > > > because it needs to keep the copy for the whole input. But I don't > > > understand why processing time of perf inject increased.. Measuring it with time shows: before after real11.309s 17.040s user 8.084s 13.940s sys 6.535s 6.732s So it's user space to make the difference. I've run perf record on both (with cycles:U) and the dominant function is same: queue_event. (46.98% vs 65.87%) It seems the flushing the queue makes more overhead on sorting. Thanks Namhyung
RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call
HI Sudeep and Michal,
>-Original Message-
>From: Sudeep Holla
>Sent: Tuesday, October 6, 2020 4:08 AM
>To: Zulkifli, Muhammad Husaini
>Cc: Michal Simek ; Hunter, Adrian
>; ulf.hans...@linaro.org; linux-
>m...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
>ker...@vger.kernel.org; Raja Subramanian, Lakshmi Bai
>; a...@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 05:04:10PM +, Zulkifli, Muhammad Husaini wrote:
>
>> To be clarify keembay_sd_voltage_selection function as Michal's
>> prefers is actually using the firmware driver. This function located
>> in firmware driver.
>
>OK, it can be just one function place it in any file you think is more
>appropriate
>need not be arasan controller driver. Any reasons why this can't work ? Can
>even
>be in some header.
>
>int keembay_sd_voltage_selection(int volt) {
> int res;
>
> arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt,
>)
>
> /* appropriate error check if needed here */
>
> return res;
>}
>
Yeah I believe it can work. I will create one header file in
include/linux/firmware/intel/Keembay_firmware.h
To handle this func and arasan controller can call this func.
Are you guys ok with this?
>> I will call this func during voltage switching from arasan controller.
>> I believe this implementation require DT to specify the compatible
>> name and method use either smc/hvc.
>
>No, use the standard one as detected by arm_smccc_1_1_invoke (It calls
>arm_smccc_get_conduit internally and use SMC/HVC based on that)
>
>>
>> Are you saying that by using simple smcc based function library I
>> should call below func() in arasan controller. For example
>> 1) arm_smccc_get_version(void)
>> 2) arm_smccc_version_init(arm_smccc_get_version(),
>SMCCC_CONDUIT_SMC);
>
>Nope
>
>> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID,
>voltage_value
>> , );
>
>Just this.
>
>--
>Regards,
>Sudeep
Re: [PATCH 1/2] x86/stackprotector/32: Make the canary into a regular percpu variable
On Mon, Oct 05, 2020 at 12:30:03PM -0700, Andy Lutomirski wrote:
> On 32-bit kernels, the stackprotector canary is quite nasty -- it is
> stored at %gs:(20), which is nasty because 32-bit kernels use %fs for
> percpu storage. It's even nastier because it means that whether %gs
> contains userspace state or kernel state while running kernel code
> sepends on whether stackprotector is enabled (this is
depends
> CONFIG_X86_32_LAZY_GS), and this setting radically changes the way
> that segment selectors work. Supporting both variants is a
> maintenance and testing mess.
>
> Merely rearranging so that percpu and the stack canary
> share the same segment would be messy as the 32-bit percpu address
> layout isn't currently compatible with putting a variable at a fixed
> offset.
>
> Fortunately, GCC 8.1 added options that allow the stack canary to be
> accessed as %fs:stack_canary, effectively turning it into an ordinary
> percpu variable. This lets us get rid of all of the code to manage
> the stack canary GDT descriptor and the CONFIG_X86_32_LAZY_GS mess.
>
> This patch forcibly disables stackprotector on older compilers that
> don't support the new options and makes the stack canary into a
> percpu variable.
It'd be helpful to explicitly state that the so called "lazy GS" approach is
now always used for i386.
> Signed-off-by: Andy Lutomirski
> ---
...
> diff --git a/arch/x86/include/asm/suspend_32.h
> b/arch/x86/include/asm/suspend_32.h
> index fdbd9d7b7bca..eb872363ca82 100644
> --- a/arch/x86/include/asm/suspend_32.h
> +++ b/arch/x86/include/asm/suspend_32.h
> @@ -16,9 +16,7 @@ struct saved_context {
>* On x86_32, all segment registers, with the possible exception of
Is this still a "possible" exception, or is it now always an exception?
>* gs, are saved at kernel entry in pt_regs.
>*/
> -#ifdef CONFIG_X86_32_LAZY_GS
> u16 gs;
> -#endif
> unsigned long cr0, cr2, cr3, cr4;
> u64 misc_enable;
> bool misc_enable_saved;
...
> diff --git a/arch/x86/kernel/tls.c b/arch/x86/kernel/tls.c
> index 64a496a0687f..3c883e064242 100644
> --- a/arch/x86/kernel/tls.c
> +++ b/arch/x86/kernel/tls.c
> @@ -164,17 +164,11 @@ int do_set_thread_area(struct task_struct *p, int idx,
> savesegment(fs, sel);
> if (sel == modified_sel)
> loadsegment(fs, sel);
> -
> - savesegment(gs, sel);
> - if (sel == modified_sel)
> - load_gs_index(sel);
> #endif
>
> -#ifdef CONFIG_X86_32_LAZY_GS
> savesegment(gs, sel);
> if (sel == modified_sel)
> - loadsegment(gs, sel);
> -#endif
> + load_gs_index(sel);
Side topic, the "index" part of this is super confusing. I had to reread
this entire patch after discovering load_gs_index is loadsegment on i386.
Maybe also worth a shout out in the changelog?
> } else {
> #ifdef CONFIG_X86_64
> if (p->thread.fsindex == modified_sel)
Re: [PATCH] random32: Restore __latent_entropy attribute on net_rand_state
Hi Kees,
On Mon, Oct 05, 2020 at 07:12:29PM -0700, Kees Cook wrote:
> On Fri, Oct 02, 2020 at 05:16:11PM +0200, Thibaut Sautereau wrote:
> > From: Thibaut Sautereau
> >
> > Commit f227e3ec3b5c ("random32: update the net random state on interrupt
> > and activity") broke compilation and was temporarily fixed by Linus in
> > 83bdc7275e62 ("random32: remove net_rand_state from the latent entropy
> > gcc plugin") by entirely moving net_rand_state out of the things handled
> > by the latent_entropy GCC plugin.
> >
> > From what I understand when reading the plugin code, using the
> > __latent_entropy attribute on a declaration was the wrong part and
> > simply keeping the __latent_entropy attribute on the variable definition
> > was the correct fix.
> >
> > Fixes: 83bdc7275e62 ("random32: remove net_rand_state from the latent
> > entropy gcc plugin")
> > Cc: Linus Torvalds
> > Cc: Willy Tarreau
> > Cc: Emese Revfy
> > Signed-off-by: Thibaut Sautereau
>
> Yes, that looks correct. Thank you!
>
> Acked-by: Kees Cook
>
> I'm not sure the best tree for this. Ted, Andrew, Linus? I'll take it
> via my gcc plugin tree if no one else takes it. :)
It was already merged as commit 09a6b0bc3be79 and queued for -stable.
Cheers,
Willy
Re: [PATCH] sched: watchdog: Touch kernel watchdog in sched code
On Mon, Oct 5, 2020 at 4:19 AM Peter Zijlstra wrote: > > On Fri, Mar 06, 2020 at 02:34:20PM -0800, Xi Wang wrote: > > On Fri, Mar 6, 2020 at 12:40 AM Peter Zijlstra wrote: > > > > > > On Thu, Mar 05, 2020 at 02:11:49PM -0800, Paul Turner wrote: > > > > The goal is to improve jitter since we're constantly periodically > > > > preempting other classes to run the watchdog. Even on a single CPU > > > > this is measurable as jitter in the us range. But, what increases the > > > > motivation is this disruption has been recently magnified by CPU > > > > "gifts" which require evicting the whole core when one of the siblings > > > > schedules one of these watchdog threads. > > > > > > > > The majority outcome being asserted here is that we could actually > > > > exercise pick_next_task if required -- there are other potential > > > > things this will catch, but they are much more braindead generally > > > > speaking (e.g. a bug in pick_next_task itself). > > > > > > I still utterly hate what the patch does though; there is no way I'll > > > have watchdog code hook in the scheduler like this. That's just asking > > > for trouble. > > > > > > Why isn't it sufficient to sample the existing context switch counters > > > from the watchdog? And why can't we fix that? > > > > We could go to pick next and repick the same task. There won't be a > > context switch but we still want to hold the watchdog. I assume such a > > counter also needs to be per cpu and inside the rq lock. There doesn't > > seem to be an existing one that fits this purpose. > > Sorry, your reply got lost, but I just ran into something that reminded > me of this. > > There's sched_count. That's currently schedstat, but if you can find a > spot in a hot cacheline (from schedule()'s perspective) then it > should be cheap to incremenent unconditionally. > > If only someone were to write a useful cacheline perf tool (and no that > c2c trainwreck doesn't count). > Thanks, I'll try the alternative implementation. -Xi
Re: [PATCH RFC v2 0/6] Break heap spraying needed for exploiting use-after-free
It will reuse the memory for other things when the whole slab is freed though. Not really realistic to change that without it being backed by virtual memory along with higher-level management of regions to avoid intense fragmentation and metadata waste. It would depend a lot on having much finer-grained slab caches, otherwise it's not going to be much of an alternative to a quarantine feature. Even then, a quarantine feature is still useful, but is less suitable for a mainstream feature due to performance cost. Even a small quarantine has a fairly high performance cost.
Re: [PATCH v7 03/12] dt-bindings: mfd: Fix schema warnings for pwm-leds
Hi Alexander,
On Mon, Oct 05, 2020 at 10:34:42PM +0200, Alexander Dahl wrote:
> The node names for devices using the pwm-leds driver follow a certain
> naming scheme (now). Parent node name is not enforced, but recommended
> by DT project.
>
> DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
> CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
> /home/alex/build/linux/Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml:
> pwmleds: 'panel' does not match any of the regexes: '^led(-[0-9a-f]+)?$',
> 'pinctrl-[0-9]+'
> From schema:
> /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/leds-pwm.yaml
>
> Signed-off-by: Alexander Dahl
> ---
>
> Notes:
> v6 -> v7:
> * added warning message to commit message (Krzysztof Kozlowski)
>
> v6:
> * added this patch to series
>
> Documentation/devicetree/bindings/mfd/iqs62x.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/iqs62x.yaml
> b/Documentation/devicetree/bindings/mfd/iqs62x.yaml
> index 541b06d80e73..92dc48a8dfa7 100644
> --- a/Documentation/devicetree/bindings/mfd/iqs62x.yaml
> +++ b/Documentation/devicetree/bindings/mfd/iqs62x.yaml
> @@ -90,10 +90,11 @@ examples:
> };
> };
>
> -pwmleds {
> +led-controller {
> compatible = "pwm-leds";
>
> -panel {
> +led-1 {
> +label = "panel";
> pwms = <_pwm 0 100>;
> max-brightness = <255>;
> };
> --
> 2.20.1
>
I like the consistency this brings. My only feedback is that in the other
examples I found (common.yaml and leds-gpio.yaml), the children count off
from 0 (e.g. led-0) instead of 1 as your series appears to.
That's not a huge deal; it simply seems more consistent to count from the
first index allowed by the regex (0). The patch is still fine, and so:
Acked-by: Jeff LaBundy
Kind regards,
Jeff LaBundy
Re: [PATCH RFC v2 0/6] Break heap spraying needed for exploiting use-after-free
On Tue, Oct 6, 2020 at 4:09 AM Kees Cook wrote: > On Tue, Oct 06, 2020 at 01:44:14AM +0100, Matthew Wilcox wrote: > > On Tue, Oct 06, 2020 at 12:56:33AM +0200, Jann Horn wrote: > > > It seems to me like, if you want to make UAF exploitation harder at > > > the heap allocator layer, you could do somewhat more effective things > > > with a probably much smaller performance budget. Things like > > > preventing the reallocation of virtual kernel addresses with different > > > types, such that an attacker can only replace a UAF object with > > > another object of the same type. (That is not an idea I like very much > > > either, but I would like it more than this proposal.) (E.g. some > > > browsers implement things along those lines, I believe.) > > > > The slab allocator already has that functionality. We call it > > TYPESAFE_BY_RCU, but if forcing that on by default would enhance security > > by a measurable amount, it wouldn't be a terribly hard sell ... > > Isn't the "easy" version of this already controlled by slab_merge? (i.e. > do not share same-sized/flagged kmem_caches between different caches) Yes, but slab_merge still normally frees slab pages to the page allocator. > The large trouble are the kmalloc caches, which don't have types > associated with them. Having implicit kmem caches based on the type > being allocated there would need some pretty extensive plumbing, I > think? Well, a bit of plumbing, at least. You'd need to teach the compiler frontend to grab type names from sizeof() and stuff that type information somewhere, e.g. by generating an extra function argument referring to the type, or something like that. Could be as simple as a reference to a bss section variable that encodes the type in the name, and the linker already has the logic to automatically deduplicate those across compilation units - that way, on the compiler side, a pure frontend plugin might do the job?
Re: [PATCH] random32: Restore __latent_entropy attribute on net_rand_state
On Fri, Oct 02, 2020 at 05:16:11PM +0200, Thibaut Sautereau wrote:
> From: Thibaut Sautereau
>
> Commit f227e3ec3b5c ("random32: update the net random state on interrupt
> and activity") broke compilation and was temporarily fixed by Linus in
> 83bdc7275e62 ("random32: remove net_rand_state from the latent entropy
> gcc plugin") by entirely moving net_rand_state out of the things handled
> by the latent_entropy GCC plugin.
>
> From what I understand when reading the plugin code, using the
> __latent_entropy attribute on a declaration was the wrong part and
> simply keeping the __latent_entropy attribute on the variable definition
> was the correct fix.
>
> Fixes: 83bdc7275e62 ("random32: remove net_rand_state from the latent entropy
> gcc plugin")
> Cc: Linus Torvalds
> Cc: Willy Tarreau
> Cc: Emese Revfy
> Signed-off-by: Thibaut Sautereau
Yes, that looks correct. Thank you!
Acked-by: Kees Cook
I'm not sure the best tree for this. Ted, Andrew, Linus? I'll take it
via my gcc plugin tree if no one else takes it. :)
--
Kees Cook
Re: [PATCH v12 9/9] kdump: update Documentation about crashkernel
Hi Catalin, On 2020/10/6 1:19, Catalin Marinas wrote: > On Mon, Sep 07, 2020 at 09:47:45PM +0800, Chen Zhou wrote: >> diff --git a/Documentation/admin-guide/kdump/kdump.rst >> b/Documentation/admin-guide/kdump/kdump.rst >> index 2da65fef2a1c..549611abc581 100644 >> --- a/Documentation/admin-guide/kdump/kdump.rst >> +++ b/Documentation/admin-guide/kdump/kdump.rst > [...] >> @@ -316,8 +325,18 @@ Boot into System Kernel >> kernel will automatically locate the crash kernel image within the >> first 512MB of RAM if X is not given. >> >> - On arm64, use "crashkernel=Y[@X]". Note that the start address of >> - the kernel, X if explicitly specified, must be aligned to 2MiB >> (0x20). >> + On arm64, use "crashkernel=X" to try low allocation in DMA zone, and >> + fall back to high allocation if it fails. And go for high allocation >> + directly if the required size is too large. >> + We can also use "crashkernel=X,high" to select a high region above >> + DMA zone, which also tries to allocate at least 256M low memory in >> + DMA zone automatically. >> + "crashkernel=Y,low" can be used to allocate specified size low memory >> + in DMA zone. >> + For non-RPi4 platforms, change DMA zone memtioned above to DMA32 zone. > I don't think we should mention non-RPi4 explicitly here. I don't even > understand what the suggestion is since the only way is to disable > ZONE_DMA in the kernel config. I'd just stick to ZONE_DMA description > here. How about like this: If the kernel config ZONE_DMA is disabled, just try low allocation in DMA32 zone and high allocation above DMA32 zone. Thanks, Chen Zhou > >> + Use "crashkernel=Y@X" if you really have to reserve memory from >> + specified start address X. Note that the start address of the kernel, >> + X if explicitly specified, must be aligned to 2MiB (0x20). >> >> Load the Dump-capture Kernel >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt >> b/Documentation/admin-guide/kernel-parameters.txt >> index a1068742a6df..f7df572d8f64 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -727,6 +727,10 @@ >> [KNL, X86-64] Select a region under 4G first, and >> fall back to reserve region above 4G when '@offset' >> hasn't been specified. >> +[KNL, arm64] Try low allocation in DMA zone, fall back >> +to high allocation if it fails when '@offset' hasn't >> been >> +specified. For non-RPi4 platforms, change DMA zone to >> +DMA32 zone. > Same here, unclear what "change DMA zone to DMA32 zone" means. > >> See Documentation/admin-guide/kdump/kdump.rst for >> further details. >> >> crashkernel=range1:size1[,range2:size2,...][@offset] >> @@ -743,6 +747,8 @@ >> Otherwise memory region will be allocated below 4G, if >> available. >> It will be ignored if crashkernel=X is specified. >> +[KNL, arm64] range in high memory. >> +Allow kernel to allocate physical memory region from >> top. >> crashkernel=size[KMG],low >> [KNL, X86-64] range under 4G. When crashkernel=X,high >> is passed, kernel could allocate physical memory region >> @@ -751,13 +757,16 @@ >> requires at least 64M+32K low memory, also enough extra >> low memory is needed to make sure DMA buffers for 32-bit >> devices won't run out. Kernel would try to allocate at >> -at least 256M below 4G automatically. >> +least 256M below 4G automatically. >> This one let user to specify own low range under 4G >> for second kernel instead. >> 0: to disable low allocation. >> It will be ignored when crashkernel=X,high is not used >> or memory reserved is below 4G. >> - >> +[KNL, arm64] range in low memory. >> +This one let user to specify a low range in DMA zone for >> +crash dump kernel. For non-RPi4 platforms, change DMA >> zone >> +to DMA32 zone. > And again here. >
Re: [PATCH RFC v2 0/6] Break heap spraying needed for exploiting use-after-free
On Tue, Oct 06, 2020 at 01:44:14AM +0100, Matthew Wilcox wrote: > On Tue, Oct 06, 2020 at 12:56:33AM +0200, Jann Horn wrote: > > It seems to me like, if you want to make UAF exploitation harder at > > the heap allocator layer, you could do somewhat more effective things > > with a probably much smaller performance budget. Things like > > preventing the reallocation of virtual kernel addresses with different > > types, such that an attacker can only replace a UAF object with > > another object of the same type. (That is not an idea I like very much > > either, but I would like it more than this proposal.) (E.g. some > > browsers implement things along those lines, I believe.) > > The slab allocator already has that functionality. We call it > TYPESAFE_BY_RCU, but if forcing that on by default would enhance security > by a measurable amount, it wouldn't be a terribly hard sell ... Isn't the "easy" version of this already controlled by slab_merge? (i.e. do not share same-sized/flagged kmem_caches between different caches) The large trouble are the kmalloc caches, which don't have types associated with them. Having implicit kmem caches based on the type being allocated there would need some pretty extensive plumbing, I think? -- Kees Cook
[PATCH] kernel/: fix repeated words in comments
From: Randy Dunlap Fix multiple occurrences of duplicated words in kernel/. Fix one typo/spello on the same line as a duplicate word. Change one instance of "the the" to "that the". Otherwise just drop one of the repeated words. Signed-off-by: Randy Dunlap Cc: Andrew Morton --- kernel/acct.c|2 +- kernel/cgroup/cpuset.c |2 +- kernel/dma/direct.c |2 +- kernel/fork.c|2 +- kernel/futex.c |2 +- kernel/irq/timings.c |2 +- kernel/jump_label.c |2 +- kernel/kcsan/encoding.h |2 +- kernel/kexec_core.c |2 +- kernel/kthread.c |2 +- kernel/livepatch/state.c |2 +- kernel/pid_namespace.c |2 +- kernel/power/snapshot.c |2 +- kernel/smp.c |2 +- kernel/user_namespace.c |2 +- 15 files changed, 15 insertions(+), 15 deletions(-) --- lnx-59-rc8.orig/kernel/acct.c +++ lnx-59-rc8/kernel/acct.c @@ -25,7 +25,7 @@ * Now we silently close acct_file on attempt to reopen. Cleaned sys_acct(). * XTerms and EMACS are manifestations of pure evil. 21/10/98, AV. * - * Fixed a nasty interaction with with sys_umount(). If the accointing + * Fixed a nasty interaction with sys_umount(). If the accounting * was suspeneded we failed to stop it on umount(). Messy. * Another one: remount to readonly didn't stop accounting. * Question: what should we do if we have CAP_SYS_ADMIN but not --- lnx-59-rc8.orig/kernel/fork.c +++ lnx-59-rc8/kernel/fork.c @@ -2164,7 +2164,7 @@ static __latent_entropy struct task_stru /* * Ensure that the cgroup subsystem policies allow the new process to be -* forked. It should be noted the the new process's css_set can be changed +* forked. It should be noted that the new process's css_set can be changed * between here and cgroup_post_fork() if an organisation operation is in * progress. */ --- lnx-59-rc8.orig/kernel/futex.c +++ lnx-59-rc8/kernel/futex.c @@ -916,7 +916,7 @@ static inline void exit_pi_state_list(st * [10] Found | Found| task | !=taskTID | 0/1| Invalid * * [1] Indicates that the kernel can acquire the futex atomically. We - * came came here due to a stale FUTEX_WAITERS/FUTEX_OWNER_DIED bit. + * came here due to a stale FUTEX_WAITERS/FUTEX_OWNER_DIED bit. * * [2] Valid, if TID does not belong to a kernel thread. If no matching * thread is found then it indicates that the owner TID has died. --- lnx-59-rc8.orig/kernel/jump_label.c +++ lnx-59-rc8/kernel/jump_label.c @@ -19,7 +19,7 @@ #include #include -/* mutex to protect coming/going of the the jump_label table */ +/* mutex to protect coming/going of the jump_label table */ static DEFINE_MUTEX(jump_label_mutex); void jump_label_lock(void) --- lnx-59-rc8.orig/kernel/kexec_core.c +++ lnx-59-rc8/kernel/kexec_core.c @@ -109,7 +109,7 @@ EXPORT_SYMBOL_GPL(kexec_crash_loaded); * defined more restrictively in . * * The code for the transition from the current kernel to the - * the new kernel is placed in the control_code_buffer, whose size + * new kernel is placed in the control_code_buffer, whose size * is given by KEXEC_CONTROL_PAGE_SIZE. In the best case only a single * page of memory is necessary, but some architectures require more. * Because this memory must be identity mapped in the transition from --- lnx-59-rc8.orig/kernel/kthread.c +++ lnx-59-rc8/kernel/kthread.c @@ -775,7 +775,7 @@ EXPORT_SYMBOL(kthread_create_worker); /** * kthread_create_worker_on_cpu - create a kthread worker and bind it - * it to a given CPU and the associated NUMA node. + * to a given CPU and the associated NUMA node. * @cpu: CPU number * @flags: flags modifying the default behavior of the worker * @namefmt: printf-style name for the kthread worker (task). --- lnx-59-rc8.orig/kernel/pid_namespace.c +++ lnx-59-rc8/kernel/pid_namespace.c @@ -233,7 +233,7 @@ void zap_pid_ns_processes(struct pid_nam * to pid_ns->child_reaper. Thus pidns->child_reaper needs to * stay valid until they all go away. * -* The code relies on the the pid_ns->child_reaper ignoring +* The code relies on the pid_ns->child_reaper ignoring * SIGCHILD to cause those EXIT_ZOMBIE processes to be * autoreaped if reparented. * --- lnx-59-rc8.orig/kernel/smp.c +++ lnx-59-rc8/kernel/smp.c @@ -741,7 +741,7 @@ EXPORT_SYMBOL(on_each_cpu_mask); * for all the required CPUs to finish. This may include the local * processor. * @cond_func: A callback function that is passed a cpu id and - * the the info parameter. The function is called + * the info parameter. The function is called * with preemption disabled. The function should * return a blooean value indicating whether to IPI * the specified CPU. --- lnx-59-rc8.orig/kernel/user_namespace.c +++
Re: [PATCH v12 0/9] support reserving crashkernel above 4G on arm64 kdump
Hi Bhupesh, On 2020/10/6 1:42, Bhupesh Sharma wrote: > Hi Catalin, Chen, > > On Mon, Oct 5, 2020 at 10:39 PM Catalin Marinas > wrote: >> On Sat, Sep 12, 2020 at 06:44:29AM -0500, John Donnelly wrote: >>> On 9/7/20 8:47 AM, Chen Zhou wrote: Chen Zhou (9): x86: kdump: move CRASH_ALIGN to 2M x86: kdump: make the lower bound of crash kernel reservation consistent x86: kdump: use macro CRASH_ADDR_LOW_MAX in functions reserve_crashkernel[_low]() x86: kdump: move reserve_crashkernel[_low]() into crash_core.c arm64: kdump: introduce some macroes for crash kernel reservation arm64: kdump: reimplement crashkernel=X kdump: add threshold for the required memory arm64: kdump: add memory for devices by DT property linux,usable-memory-range kdump: update Documentation about crashkernel >> [...] >>> I did a brief unit-test on 5.9-rc4. >>> >>> Please add: >>> >>> Tested-by: John Donnelly >> Thanks for testing. >> >>> This activity is over a year old. It needs accepted. >> It's getting there, hopefully in 5.11. There are some minor tweaks to >> address. > I think my earlier email with the test results on this series bounced > off the mailing list server (for some weird reason), but I still see > several issues with this patchset. I will add specific issues in the > review comments for each patch again, but overall, with a crashkernel > size of say 786M, I see the following issue: > > # cat /proc/cmdline > BOOT_IMAGE=(hd7,gpt2)/vmlinuz-5.9.0-rc7+ root=<..snip..> > rd.lvm.lv=<..snip..> crashkernel=786M > > I see two regions of size 786M and 256M reserved in low and high > regions respectively, So we reserve a total of 1042M of memory, which > is an incorrect behaviour: > > # dmesg | grep -i crash > [0.00] Reserving 256MB of low memory at 2816MB for crashkernel > (System low RAM: 768MB) > [0.00] Reserving 786MB of memory at 654158MB for crashkernel > (System RAM: 130816MB) > [0.00] Kernel command line: > BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.9.0-rc7+ > root=/dev/mapper/rhel_ampere--hr330a--03-root ro > rd.lvm.lv=rhel_ampere-hr330a-03/root > rd.lvm.lv=rhel_ampere-hr330a-03/swap crashkernel=786M cma=1024M > > # cat /proc/iomem | grep -i crash > b000-bfff : Crash kernel (low) > bfcbe0-bffcff : Crash kernel > > IMO, we should test this feature more before including this in 5.11 Thanks for you test. This behavior is what we what. What is the correct behavior you think? Besides, this feature is been tested by John and PK, and i test for various parameters. We may miss something, any comments are welcome. Thanks, Chen Zhou > > Thanks, > Bhupesh > > . >
Re: [Question] rtc wake behavior and sysfs
On Mon, Oct 5, 2020 at 6:29 PM Alexandre Belloni wrote: > > On 05/10/2020 09:13:08-0400, Peter Geis wrote: > > Good Morning, > > > > While testing suspend to ram on the Ouya, I encountered an interesting > > issue with the rtc-tps65910 driver. > > Attempting to use rtc-wake on the default configuration returned: > > rtcwake: /dev/rtc0 not enabled for wakeup events > > This is due to: > > eb5eba4ef722 drivers/rtc/rtc-tps65910.c: enable/disable wake in > > suspend/resume > > This commit changed this driver's behavior to not enable wakeup by > > default, but enables it when entering sleep mode. > > This seems to be odd behavior to me. > > Looking at a few other rtc drivers show they simply enable themselves > > as wakeup sources by default. > > > > I also found the sysfs entries are at /sys/devices/ .. > > /tps65910-rtc/power but are missing at /sys/class/rtc/rtc0/power/ > > > > I have two questions. > > - Should the sysfs wakeup entries be missing at /sys/class/rtc/rtc0/power/ > > ? > > I would be in /sys/class/rtc/rtc0/device/power > > > - Shouldn't a rtc be enabled as a wakeup source by default? > > > > The short answer is no, the reason being that not all RTCs are connected > to an IRQ or a pin that can wakeup or start the platform. What should be > done is enabling wakeup only when interrupts are available or the > wakeup-source property is in the rtc device tree node. Thank you for your explanation. So it would seem we have two issues. - The sysfs wakeup entries are not populating in /sys/class/rtc/rtc0/power when they are populating in /sys/devices/<..>/tps65910-rtc/power. - The wakeup-source property is not being applied to the tps65910-rtc sub-device when it is applied in the tps65910 devicetree node. Should wakeup-source handling be the tps65910-rtc driver's responsibility, or is that a limitation of the driver core that needs to be fixed? > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Re: [PATCHv2 1/9] perf tools: Add build id shell test
On Sat, Oct 3, 2020 at 4:29 AM Jiri Olsa wrote:
>
> On Fri, Oct 02, 2020 at 10:34:51AM -0700, Ian Rogers wrote:
>
> SNIP
>
> > > > +
> > > > LIBJVMTI = libperf-jvmti.so
> > > >
> > > > ifndef NO_JVMTI
> > > > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c
> > > > util/find-map.c
> > > > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall
> > > > -Werror -o $@ perf-read-vdso.c
> > > > endif
> > > >
> > > > +ifndef NO_BUILDID_EX
> > > > +$(OUTPUT)buildid-ex-sha1:
> > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC)
> > > > -Wl,--build-id=sha1 -o $@ -x c -
> > > > +$(OUTPUT)buildid-ex-md5:
> > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC)
> > > > -Wl,--build-id=md5 -o $@ -x c -
> > > > +endif
> > >
> > > Can we just build them in the test shell script instead?
>
> it would solve the build-directory/install-directory
> lookup search.. but it'd need to do detect compiler
> and depend on it as Ian said
>
> do you have some other reason to compile it in test?
No I just wanted to make it easy to find the binaries
and assumed a compiler is available in the test machine
(which is not true for my company setup :-/)
But otherwise we should keep the binaries somewhere
in the install directory..
Thanks
Namhyung
Re: [PATCH v12 7/9] kdump: add threshold for the required memory
On 2020/10/6 1:12, Catalin Marinas wrote: > On Mon, Sep 07, 2020 at 09:47:43PM +0800, Chen Zhou wrote: >> diff --git a/kernel/crash_core.c b/kernel/crash_core.c >> index 3f735cb37ace..d11d597a470d 100644 >> --- a/kernel/crash_core.c >> +++ b/kernel/crash_core.c >> @@ -378,6 +378,15 @@ int __init reserve_crashkernel_low(void) >> } >> >> #if defined(CONFIG_X86) || defined(CONFIG_ARM64) >> + >> +/* >> + * Add a threshold for required memory size of crashkernel. If required >> memory >> + * size is greater than threshold, just go for high allocation directly. The >> + * value of threshold is set as half of the total low memory. >> + */ >> +#define REQUIRED_MEMORY_THRESHOLD (memblock_mem_size(CRASH_ADDR_LOW_MAX >> >> \ >> +PAGE_SHIFT) >> 1) >> + >> #ifdef CONFIG_KEXEC_CORE >> /* >> * reserve_crashkernel() - reserves memory for crash kernel >> @@ -422,7 +431,7 @@ void __init reserve_crashkernel(void) >> * So try low memory first and fall back to high memory >> * unless "crashkernel=size[KMG],high" is specified. >> */ >> -if (!high) >> +if (!high && crash_size <= REQUIRED_MEMORY_THRESHOLD) >> crash_base = memblock_find_in_range(CRASH_ALIGN, >> CRASH_ADDR_LOW_MAX, >> crash_size, CRASH_ALIGN); > Since any change now is affecting the x86 semantics slightly, I'd > suggest you drop this patch. We can add it later if needed, once the > core changes are in. Ok, i will drop this patch in next version. Thanks, Chen Zhou > > Thinking about this, if one requires a crashkernel reservation that > allocates all of the ZONE_DMA, it would probably be noticed and explicit > ,high/,low options can be used. > > Note that we are also trying to make ZONE_DMA full 32-bit on non-RPi4 > hardware. >
Re: [PATCH v12 6/9] arm64: kdump: reimplement crashkernel=X
On 2020/10/6 1:16, Catalin Marinas wrote:
> On Mon, Sep 07, 2020 at 09:47:42PM +0800, Chen Zhou wrote:
>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>> index 53acbeca4f57..1b24072f2bae 100644
>> --- a/arch/arm64/kernel/setup.c
>> +++ b/arch/arm64/kernel/setup.c
>> @@ -238,7 +238,18 @@ static void __init request_standard_resources(void)
>> kernel_data.end <= res->end)
>> request_resource(res, _data);
>> #ifdef CONFIG_KEXEC_CORE
>> -/* Userspace will find "Crash kernel" region in /proc/iomem. */
>> +/*
>> + * Userspace will find "Crash kernel" or "Crash kernel (low)"
>> + * region in /proc/iomem.
>> + * In order to distinct from the high region and make no effect
>> + * to the use of existing kexec-tools, rename the low region as
>> + * "Crash kernel (low)".
>> + */
>> +if (crashk_low_res.end && crashk_low_res.start >= res->start &&
>> +crashk_low_res.end <= res->end) {
>> +crashk_low_res.name = "Crash kernel (low)";
>> +request_resource(res, _low_res);
>> +}
> With the changes in this series (including the above), how do the
> current kexec-tools behave? Do they pick just the high region and the
> loaded kernel will subsequently fail to boot?
Yes，just pick the high region and will boot fail if low memory is needed.
Thanks,
Chen Zhou
>
Re: [PATCH] perf inject: Flush ordered events on FINISHED_ROUND
Hi Jiri, On Mon, Oct 5, 2020 at 4:52 AM Jiri Olsa wrote: > > On Fri, Oct 02, 2020 at 10:03:17PM +0900, Namhyung Kim wrote: > > Currently perf inject just repipes the event without any flush. It > > makes an issue that it changes the order of events processed. > > > > Normally we want to process events in time order, but events are > > recorded from several cpus and they weren't sorted with each other. > > So we put them into the ordered event queue, sorted by time, and flush > > them when we see a next FINISHED_ROUND event. > > > > But this is for events from kernel, user space events (like the > > FINISHED_ROUND) are processed without queueing. So during the perf > > inject, it writes all the FINISHED_ROUND events directly while > > SAMPLE (and other) events are kept in the queue. This makes the user > > space events are put before any kernel events. > > > > You can see that with the following command: > > > > $ perf record -a -o- sleep 1 | perf inject -b | perf script -i- > > --show-round-events > > PERF_RECORD_FINISHED_ROUND > > PERF_RECORD_FINISHED_ROUND > > PERF_RECORD_FINISHED_ROUND > > ... > > > > Omitting perf inject, you can see the events are placed in the middle > > of the data. > > > > You might argue that the whole point of the FINISHED_ROUND event is to > > sort (kernel) events. And after perf inject, all events are written > > in a proper time order so we don't care about the FINISHED_ROUND event > > anymore. > > > > But the problem is we don't know whether the input data is sorted or > > not (maybe we can add a feature bit for this?) so it should use an > > ordered event queue when processing the input like in perf report. > > I like the idea of storing the information that the data is sorted, > and when it's there, let's not use ordered_oevets Thanks for your input. Yeah, I think it's better not to use it if possible. > > > > > Remember all the FINISHED_ROUND events now come before other events so > > the tool cannot know when it can flush the data so everything will be > > queued until it meets the end of the input. Actually it's same for > > perf inject itself as it doesn't flush the queue. > > > > Below measures time and memory usage during the perf inject and > > report using ~190MB data file. > > > > Before: > > perf inject: 11.09 s, 382148 KB > > perf report: 8.05 s, 397440 KB > > > > After: > > perf inject: 16.24 s, 83376 KB > > perf report: 7.96 s, 216184 KB > > > > As you can see, it used 2x memory of the input size. I guess it's > > because it needs to keep the copy for the whole input. But I don't > > understand why processing time of perf inject increased.. > > would be good to find out first Will do! Thanks Namhyung
Re: [PATCH] perf evlist: fix memory corruption for Kernel PMU event
Hello,
On Fri, Oct 2, 2020 at 12:02 PM Song Bao Hua (Barry Song)
wrote:
>
>
>
> > -Original Message-
> > From: Andi Kleen [mailto:a...@linux.intel.com]
> > Sent: Friday, October 2, 2020 12:07 PM
> > To: Song Bao Hua (Barry Song)
> > Cc: linux-kernel@vger.kernel.org; Linuxarm ; Peter
> > Zijlstra ; Ingo Molnar ; Arnaldo
> > Carvalho de Melo ; Mark Rutland
> > ; Alexander Shishkin
> > ; Jiri Olsa ;
> > Namhyung Kim ; Adrian Hunter
> > ; Alexey Budankov
> >
> > Subject: Re: [PATCH] perf evlist: fix memory corruption for Kernel PMU event
> >
> > On Fri, Oct 02, 2020 at 12:57:29AM +1300, Barry Song wrote:
> > > Commit 7736627b865d ("perf stat: Use affinity for closing file
> > > descriptors") will use FD(evsel, cpu, thread) to read and write file
> > > descriptors xyarray. For a kernel PMU event, this leads to serious
> > > memory corruption and perf crash.
> > > I have seen evlist->core.cpus->nr is 1 while evsel has cpus->nr with
> > > the total number of CPUs. so xyarray which is allocated by
> > > evlist->core.cpus->nr will get overflow. This leads to various
> > > segmentation faults in perf tool for kernel PMU events, eg:
> > > ./perf stat -e bus_cycles sleep 1
> > > *** Error in `./perf': free(): invalid next size (fast):
> > > 0x401e6370 *** Aborted (core dumped)
> >
> > Thanks.
> >
> > I believe there is already a patch queued for this.
>
> Andi, thanks! Could you share the link or the commit ID? I'd like to take a
> look at the fix.
> I could still reproduce this issue in the latest linus' tree and I didn't
> find any commit
> related to this issue in linux-next and tip/perf/core.
I think Andi was referring to this discussion which is not merged yet:
https://lore.kernel.org/lkml/20200922031346.15051-2-liwei...@huawei.com/
I suggested a patch at the end. Can you please try it?
Thanks
Namhyung
Re: [PATCH] usb: host: ehci-sched: avoid possible NULL dereference
On Mon, Oct 05, 2020 at 11:19:02PM +, Harley A.W. Lorenzo wrote: > On Monday, October 5, 2020 5:31 PM, Sudip Mukherjee > wrote: > > > find_tt() can return NULL or the error value in ERR_PTR() and > > dereferencing the return value without checking for the error can > > lead to a possible dereference of NULL pointer or ERR_PTR(). > > Looks fine to me. There is in fact no checks of the return value > before a dereference here, and this solves that. > > Reviewed-by: Harley A.W. Lorenzo
Re: Is usb_hcd_giveback_urb() allowed in task context?
On Mon, Oct 05, 2020 at 05:38:22PM -0600, Shuah Khan wrote: > On 10/5/20 9:25 AM, Alan Stern wrote: > > On Mon, Oct 05, 2020 at 05:21:30PM +0200, Andrey Konovalov wrote: > > No, no -- it won't work right if it's called in process context. Not > > only do the spinlock calls leave the interrupt flag unchanged, also the > > driver callback routines may expect to be invoked with interrupts > > disabled. (We have tried to fix this, but I'm not at all certain that > > all the cases have been updated.) > > > > In the case of vhci case, usb_hcd_giveback_urb() is called from vhci's > urb_enqueue, when it determines it doesn't need to xmit the urb and can give > it back. This path runs in task context. > > Do you have any recommendation on how this case can be handled? Just call local_irq_disable() before usb_hcd_giveback_urb(), and local_irq_enable() afterward. Alan Stern
Re: [PATCH 1/4] drivers core: Introduce CPU type sysfs interface
On Fri, Oct 02, 2020 at 08:27:41PM -0700, Randy Dunlap wrote:
> On 10/2/20 6:17 PM, Ricardo Neri wrote:
> > +/**
> > + * arch_get_cpu_type() - Get the CPU type number
> > + * @cpu: Index of the CPU of which the index is needed
> > + *
> > + * Get the CPU type number of @cpu, a non-zero unsigned 32-bit number that
Thank you for your feedback Randy!
> Are you sure that @cpu is non-zero?
This is the intent. Maybe it can be reworked to return -EINVAL instead?
I gues it is plausible but less likely that an arch defines a type as
0xffea;
>
> > + * uniquely identifies a type of CPU micro-architecture. All CPUs of the
> > same
> > + * type have the same type number. Type numbers are defined by each CPU
> > + * architecture.
> > + */
> > +u32 __weak arch_get_cpu_type(int cpu)
> > +{
> > + return 0;
> > +}
>
> arch_get_cpu_type() in patch 4/4 allows @cpu to be 0.
It should not return 0 if the system does not have
X86_FEATURE_HYBRID_CPU. The currently existing CPU types are all
non-zero as per the Intel SDM. Am I missing anything?
Thanks and BR,
Ricardo
Re: [PATCH v5 2/3] iommu/tegra-smmu: Rework tegra_smmu_probe_device()
On Mon, Oct 05, 2020 at 12:56:38PM +0200, Thierry Reding wrote:
> On Mon, Oct 05, 2020 at 11:41:08AM +0300, Dmitry Osipenko wrote:
> > 05.10.2020 00:57, Nicolin Chen пишет:
> > > On Sat, Oct 03, 2020 at 05:06:42PM +0300, Dmitry Osipenko wrote:
> > >> 03.10.2020 09:59, Nicolin Chen пишет:
> > >>> static int tegra_smmu_of_xlate(struct device *dev,
> > >>>struct of_phandle_args *args)
> > >>> {
> > >>> + struct platform_device *iommu_pdev =
> > >>> of_find_device_by_node(args->np);
> > >>> + struct tegra_mc *mc = platform_get_drvdata(iommu_pdev);
> > >>> u32 id = args->args[0];
> > >>>
> > >>> + put_device(_pdev->dev);
> > >>> +
> > >>> + if (!mc || !mc->smmu)
> > >>> + return -EPROBE_DEFER;
> > >>
> > >> I'm not very excited by seeing code in the patches that can't be
> > >> explained by the patch authors and will appreciate if you could provide
> > >> a detailed explanation about why this NULL checking is needed because I
> > >> think it is unneeded, especially given that other IOMMU drivers don't
> > >> have such check.
> > >
> > > This function could be called from of_iommu_configure(), which is
> > > a part of other driver's probe(). So I think it's safer to have a
> > > check. Yet, given mc driver is added to the "arch_initcall" stage,
> > > you are probably right that there's no really need at this moment
> > > because all clients should be called after mc/smmu are inited. So
> > > I'll resend a v6, if that makes you happy.
> >
> > I wanted to get the explanation :) I'm very curious why it's actually
> > needed because I'm not 100% sure whether it's not needed at all.
> >
> > I'd assume that the only possible problem could be if some device is
> > created in parallel with the MC probing and there is no locking that
> > could prevent this in the drivers core. It's not apparent to me whether
> > this situation could happen at all in practice.
> >
> > The MC is created early and at that time everything is sequential, so
> > it's indeed should be safe to remove the check.
>
> I think I now remember exactly why the "hack" in tegra_smmu_probe()
> exists. The reason is that the MC driver does this:
>
> mc->smmu = tegra_smmu_probe(...);
>
> That means that mc->smmu is going to be NULL until tegra_smmu_probe()
> has finished. But tegra_smmu_probe() calls bus_set_iommu() and that in
> turn calls ->probe_device(). So the purpose of the "hack" in the
> tegra_smmu_probe() function was to make sure mc->smmu was available at
> that point, because, well, it is already known, but we haven't gotten
> around to storing it yet.
Yea, that's exactly what I described in the commit message of a
later version of this patch. Yet, we can drop it now as a device
will probe() and call ->probe_device() again.
> ->of_xlate() can theoretically be called as early as right after
> bus_set_iommu() via of_iommu_configure() if that is called in parallel
> with tegra_smmu_probe(). I think that's very unlikely, but I'm not 100%
> sure that it can't happen.
As my previous reply to Dmitry above, I don't think it'd happen,
given that mc/smmu drivers are inited during arch_initcall stage
while all clients should be probed during device_initcall stage,
once this rework patch gets merged.
> In any case, I do agree with Dmitry that we should have a comment here
> explaining why this is necessary. Even if we're completely certain that
> this is necessary, it's not obvious and therefore should get that
> comment. And if we're not certain that it's necessary, it's probably
> also good to mention that in the comment so that eventually it can be
> determined or the check removed if it proves to be unnecessary.
Well, I have answered him, and also removed the mc/smmu check in
v6 already.
RE: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call
Hi Michal,
>-Original Message-
>From: Sudeep Holla
>Sent: Tuesday, October 6, 2020 4:08 AM
>To: Zulkifli, Muhammad Husaini
>Cc: Michal Simek ; Hunter, Adrian
>; ulf.hans...@linaro.org; linux-
>m...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
>ker...@vger.kernel.org; Raja Subramanian, Lakshmi Bai
>; a...@arndb.de; Wan Mohamad,
>Wan Ahmad Zainie
>Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted
>Firmware Service call
>
>On Mon, Oct 05, 2020 at 05:04:10PM +, Zulkifli, Muhammad Husaini wrote:
>
>> To be clarify keembay_sd_voltage_selection function as Michal's
>> prefers is actually using the firmware driver. This function located
>> in firmware driver.
>
>OK, it can be just one function place it in any file you think is more
>appropriate
>need not be arasan controller driver. Any reasons why this can't work ? Can
>even
>be in some header.
>
>int keembay_sd_voltage_selection(int volt) {
> int res;
>
> arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt,
>)
>
> /* appropriate error check if needed here */
>
> return res;
>}
>
>> I will call this func during voltage switching from arasan controller.
>> I believe this implementation require DT to specify the compatible
>> name and method use either smc/hvc.
>
>No, use the standard one as detected by arm_smccc_1_1_invoke (It calls
>arm_smccc_get_conduit internally and use SMC/HVC based on that)
>
>>
>> Are you saying that by using simple smcc based function library I
>> should call below func() in arasan controller. For example
>> 1) arm_smccc_get_version(void)
>> 2) arm_smccc_version_init(arm_smccc_get_version(),
>SMCCC_CONDUIT_SMC);
>
>Nope
>
>> 3) arm_smccc_1_1_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID,
>voltage_value
>> , );
>
>Just this.
Is it ok not using the centralize firmware drivers?
I would just revert back everything as in V1 by directly call the
arm_smccc_1_1_invoke in arasan controller.
>
>--
>Regards,
>Sudeep
Re: [PATCH] arm/arm64: xen: Fix to convert percpu address to gfn correctly
On Mon, 5 Oct 2020, Julien Grall wrote:
> Hi Masami,
>
> On 05/10/2020 14:39, Masami Hiramatsu wrote:
> > Use per_cpu_ptr_to_phys() instead of virt_to_phys() for per-cpu
> > address conversion.
> >
> > In xen_starting_cpu(), per-cpu xen_vcpu_info address is converted
> > to gfn by virt_to_gfn() macro. However, since the virt_to_gfn(v)
> > assumes the given virtual address is in contiguous kernel memory
> > area, it can not convert the per-cpu memory if it is allocated on
> > vmalloc area (depends on CONFIG_SMP).
>
> Are you sure about this? I have a .config with CONFIG_SMP=y where the per-cpu
> region for CPU0 is allocated outside of vmalloc area.
>
> However, I was able to trigger the bug as soon as CONFIG_NUMA_BALANCING was
> enabled.
I cannot reproduce the issue with defconfig, but I can with Masami's
kconfig.
If I disable just CONFIG_NUMA_BALANCING from Masami's kconfig, the
problem still appears.
If I disable CONFIG_NUMA from Masami's kconfig, it works, which is
strange because CONFIG_NUMA is enabled in defconfig, and defconfig
works.
> [...]
>
> > Fixes: 250c9af3d831 ("arm/xen: Add support for 64KB page granularity")
>
> FWIW, I think the bug was already present before 250c9af3d831.
Yeah, I bet 250c9af3d831 is not what introduced the issue. Whatever
caused virt_to_phys to stop working on vmalloc'ed addresses is the cause
of the problem. It is something that went in 5.9 (5.8 works) but I don't
know what for sure.
> > Signed-off-by: Masami Hiramatsu
> > ---
> > arch/arm/xen/enlighten.c |2 +-
> > include/xen/arm/page.h |3 +++
> > 2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index e93145d72c26..a6ab3689b2f4 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -150,7 +150,7 @@ static int xen_starting_cpu(unsigned int cpu)
> > pr_info("Xen: initializing cpu%d\n", cpu);
> > vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
> > - info.mfn = virt_to_gfn(vcpup);
> > + info.mfn = percpu_to_gfn(vcpup);
> > info.offset = xen_offset_in_page(vcpup);
> > err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info,
> > xen_vcpu_nr(cpu),
> > diff --git a/include/xen/arm/page.h b/include/xen/arm/page.h
> > index 39df751d0dc4..ac1b65470563 100644
> > --- a/include/xen/arm/page.h
> > +++ b/include/xen/arm/page.h
> > @@ -83,6 +83,9 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
> > })
> > #define gfn_to_virt(m)(__va(gfn_to_pfn(m) <<
> > XEN_PAGE_SHIFT))
> > +#define percpu_to_gfn(v) \
> > + (pfn_to_gfn(per_cpu_ptr_to_phys(v) >> XEN_PAGE_SHIFT))
> > +
> > /* Only used in PV code. But ARM guests are always HVM. */
> > static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
> > {
The fix is fine for me. I tested it and it works. We need to remove the
"Fixes:" line from the commit message. Ideally, replacing it with a
reference to what is the source of the problem.
Aside from that:
Reviewed-by: Stefano Stabellini
Re: [PATCH v4 2/3] iommu/tegra-smmu: Rework tegra_smmu_probe_device()
On Mon, Oct 05, 2020 at 11:57:54AM +0200, Thierry Reding wrote:
> On Fri, Oct 02, 2020 at 11:58:29AM -0700, Nicolin Chen wrote:
> > On Fri, Oct 02, 2020 at 06:02:18PM +0300, Dmitry Osipenko wrote:
> > > 02.10.2020 09:08, Nicolin Chen пишет:
> > > > static int tegra_smmu_of_xlate(struct device *dev,
> > > >struct of_phandle_args *args)
> > > > {
> > > > + struct platform_device *iommu_pdev =
> > > > of_find_device_by_node(args->np);
> > > > + struct tegra_mc *mc = platform_get_drvdata(iommu_pdev);
> > > > u32 id = args->args[0];
> > > >
> > > > + of_node_put(args->np);
> > >
> > > of_find_device_by_node() takes device reference and not the np
> > > reference. This is a bug, please remove of_node_put().
> >
> > Looks like so. Replacing it with put_device(_pdev->dev);
>
> Putting the put_device() here is wrong, though. You need to make sure
> you keep a reference to it as long as you keep accessing the data that
> is owned by it.
I am confused. You said in the other reply (to Dmitry) that we do
need to put_device(mc->dev), where mc->dev should be the same as
iommu_pdev->dev. But here your comments sounds that we should not
put_device at all since ->probe_device/group_device/attach_dev()
will use it later.
> Like I said earlier, this is a bit weird in this case because we're
> self-referencing, so iommu_pdev->dev is going to stay around as long as
> the SMMU is. However, it might be worth to properly track the lifetime
> anyway just so that the code can serve as a good example of how to do
> things.
What's this "track-the-lifetime"?
> If you decide to go for the shortcut and not track this reference
> properly, then at least you need to add a comment as to why it is safe
> to do in this case. This ensures that readers are away of the
> circumstances and don't copy this bad code into a context where the
> circumstances are different.
I don't quite get this "shortcut" here either...mind elaborating?
Re: [PATCH 4/4] dt-bindings: Explicitly allow additional properties in common schemas
On 10/6/20 3:38 AM, Rob Herring wrote: > In order to add meta-schema checks for additional/unevaluatedProperties > being present, all schema need to make this explicit. As common/shared > schema are included by other schemas, they should always allow for > additionalProperties. > > Signed-off-by: Rob Herring > --- (snip) > Documentation/devicetree/bindings/extcon/wlf,arizona.yaml| 2 ++ (snip) For the extcon part, Acked-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH 1/4] drivers core: Introduce CPU type sysfs interface
On Sat, Oct 03, 2020 at 01:05:48PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Oct 03, 2020 at 10:53:45AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Oct 02, 2020 at 06:17:42PM -0700, Ricardo Neri wrote:
> > > +/**
> > > + * arch_get_cpu_type_name() - Get the CPU type name
> > > + * @cpu_type:Type of CPU micro-architecture.
> > > + *
> > > + * Returns a string name associated with the CPU micro-architecture type
> > > as
> > > + * indicated in @cpu_type. The format shall be _.
> > > Returns
> > > + * NULL if the CPU type is not known.
> > > + */
> > > +const char __weak *arch_get_cpu_type_name(u32 cpu_type)
> > > +{
> > > + return NULL;
> > > +}
> >
> > Why is vendor part of this? Shouldn't it just be arch?
> >
> > I say this as "vendor" is kind of "interesting" when it comes to other
> > arches...
> >
> > Speaking of other arches, we all know that other arches have this
> > feature as well, have you worked with any other groups to verify that
> > this interface will also work with them?
>
> Here's one set of patches for ARM64 for much the same type of cpu
> design:
> https://android-review.googlesource.com/c/kernel/common/+/1437098/3
> Yes, it's not been posted to any kernel lists, but this is public so you
> need to work with the ARM developers to come up with an interface that
> works for everyone please.
Thanks for the pointer, Greg! I will study this proposal and work with
the ARM engineers.
BR,
Ricardo
