lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 25 Jul 2022 15:03:23 +0100 From: John Garry <john.garry@...wei.com> To: Arnd Bergmann <arnd@...db.de> CC: Will Deacon <will@...nel.org>, Catalin Marinas <catalin.marinas@....com>, Linux ARM <linux-arm-kernel@...ts.infradead.org>, Olof Johansson <olof@...om.net>, SoC Team <soc@...nel.org>, <jpoimboe@...nel.org>, "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH 0/2] arm64 defconfig: Get faddr2line working On 25/07/2022 13:51, Arnd Bergmann wrote: >> From commit fdf4632aa834, we enable config ATH11K_PCI, which selects That should have been 231a136fdf4632a - I chopped off the leading characters by accident. >> config QRTR, so no need to explicitly enable in the defconfig. > Something is wrong here, as qrtr is used the same way in both ath10k and > ath11k, but only one of them selects the symbol. My guess is that there is > no actual hard requirement to enable it. I'm not sure. From commit 1399fb87ea3e ("ath11k: register MHI controller device for QCA6390"), QRTR/MHI bus seems to be used for ath11k only: Extract from that commit: diff --git a/drivers/net/wireless/ath/ath11k/Kconfig b/drivers/net/wireless/ath/ath11k/Kconfig index 2a792ddd6fea..7e5094e0e7bb 100644 --- a/drivers/net/wireless/ath/ath11k/Kconfig +++ b/drivers/net/wireless/ath/ath11k/Kconfig @@ -21,6 +21,9 @@ config ATH11K_AHB config ATH11K_PCI tristate "Atheros ath11k PCI support" depends on ATH11K && PCI + select MHI_BUS + select QRTR + select QRTR_MHI > >>> CONFIG_PINCTRL_MSM=y >> From commit c807a335d3b1, config PINCTRL_SM8450 is enabled, which >> selects PINCTRL_MSM, so no need to explicitly enable in the defconfig. > This looks like a bug, where the driver should have used 'depends on' > rather than 'select'. ok, I see that this is the odd one out in that kconfig as the other configs "depends on", like you say > >>> CONFIG_SND_SOC_TEGRA210_OPE=m >> There is no issue on next-20220722. >> >> On arm-soc arm/defconfig, config SND_SOC_TEGRA210_OPE just does not yet >> exist, so that's why it get removed from the sync of the defconfig. > Ok > >>> CONFIG_MAILBOX=y >> In commit fc739069aa92, config MAILBOX was enabled in the defconfig but >> it was already being enabled from elsewhere. There was definitely no >> sync of the savedefconfig going on there:) > I see it's selected by a couple of drivers using mailboxes, and I > agree we shouldn't > need it here. It might be good to just hide the symbol in this case > and select it > consistently from all drivers using it. Uhh, we have ~15x "select" and ~18x "depends on"... > >>> CONFIG_QCOM_ICC_BWMON=m >> Commit 76f11e77f919 enabled config QCOM_ICC_BWMON, but like config >> ARCH_BCMBCA, that config does not exist on arm-soc arm/defconfig branch >> >> On next-20220722, no sync is required > ok > >>> CONFIG_SLIMBUS=m >> Config 5bd773242f75 added a kconfig "imply" on config SLIMBUS from >> config SOUNDWIRE_QCOM, and this happens to mean that we don't explicitly >> require config SLIMBUS enabled in the defconfig. > That 'imply' looks like it was added to solve a problem using the old 'imply' > semantics that are not what this keyword does today. I didn't even know it existed. > I suspect it's still > broken when CONFIG=SOUNDWIRE_QCOM=y and CONFIG_SLIBMUS=m, > and the correct fix is to use a dependency like > > depends on SLIMBUS || !SLIMBUS > > so the defconfig symbol should stay. Let me see if I can break it and then fix it. > >>> CONFIG_INTERCONNECT=y >> Since commit 06f079816d4c, config TEGRA_MC added a kconfig select on >> config INTERCONNECT, so no need to explicitly enable from the kconfig > We have one driver using 'depends on INTERCONNECT' and two drivers > using 'select INTERCONNECT', so at least one of them is wrong. INTERCONNECT has no dependencies, so "select" - like for MAILBOX - should be fine, I suppose > > There are also a couple of drivers that use neither and instead rely on the > IS_ENABLED() check in include/linux/interconnect.h, but this is potentially > wrong in the same way as the SLIMBUS dependency above. > > This clearly needs some more discussion. I would suggest removing the > #iff check in the header and forcing all users to use 'depends on', leaving > the defconfig symbol. > >>> CONFIG_CONFIGFS_FS=y >> From commit cd8bc7d4eb66, config PCI_ENDPOINT_CONFIGFS is enabled in >> the defconfig, and this selects CONFIG_CONFIGFS_FS, so no need to have >> explicit enabling in the defconfig. > Again, half the users of CONFIGFS_FS use 'depends on', the other half use > 'select'. We should be consistent here and always use the same method, > probably 'depends on' if we want to keep it as user-visible. I don't know .... > >> We still have issues on next-22072022: >> CONFIG_ARM_CPUIDE >> CONFIG_DEBUG_INFO >> CONFIG_DEBUG_INFO_REDUCED >> >> The latter two are not an issue on the arm-soc arm/defconfig. > Ok. > >> So let me know if any comments or how to proceed. >> >> And would each config item deletion merit a separate patch? > You send a combined patch for the obvious ones (secccomp > and mailbox AFAICT) or send them separately. For the other ones I think > we should try fixing the Kconfig files first, otherwise we just end up > putting them back afterwards. ok, fine. I'll deal with the obvious changes first plus CONFIG_DEBUG_INFO and then the non-obvious, non-trivial ones. I'll base on your arm/defconfig branch (for defconfig changes). Thanks, John
Powered by blists - more mailing lists