Re: [PATCH 075/141] crypto: ccree - Fix fall-through warnings for Clang
On Fri, Nov 20, 2020 at 8:34 PM Gustavo A. R. Silva wrote: > > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple > warnings by explicitly adding multiple break statements instead of > letting the code fall through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva > --- > drivers/crypto/ccree/cc_cipher.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/crypto/ccree/cc_cipher.c > b/drivers/crypto/ccree/cc_cipher.c > index dafa6577a845..cdfee501fbd9 100644 > --- a/drivers/crypto/ccree/cc_cipher.c > +++ b/drivers/crypto/ccree/cc_cipher.c > @@ -97,6 +97,7 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p, > u32 size) > case S_DIN_to_SM4: > if (size == SM4_KEY_SIZE) > return 0; > + break; > default: > break; > } > @@ -139,9 +140,11 @@ static int validate_data_size(struct cc_cipher_ctx > *ctx_p, > case DRV_CIPHER_CBC: > if (IS_ALIGNED(size, SM4_BLOCK_SIZE)) > return 0; > + break; > default: > break; > } > + break; > default: > break; > } > -- > 2.27.0 > Acked-by: Gilad Ben-Yossef Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH v3] crypto: ccree - rework cache parameters handling
Rework the setting of DMA cache parameters, program more appropriate values and explicitly set sharability domain. Signed-off-by: Gilad Ben-Yossef --- Changes from previous versions: - After discussion with Rob H., drop notion of setting the parameters from device tree and just use good defaults for cached/non cached. drivers/crypto/ccree/cc_driver.c | 75 +--- drivers/crypto/ccree/cc_driver.h | 6 +-- drivers/crypto/ccree/cc_pm.c | 2 +- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c index 6f519d3e896c..d0e59e942568 100644 --- a/drivers/crypto/ccree/cc_driver.c +++ b/drivers/crypto/ccree/cc_driver.c @@ -100,6 +100,57 @@ static const struct of_device_id arm_ccree_dev_of_match[] = { }; MODULE_DEVICE_TABLE(of, arm_ccree_dev_of_match); +static void init_cc_cache_params(struct cc_drvdata *drvdata) +{ + struct device *dev = drvdata_to_dev(drvdata); + u32 cache_params, ace_const, val, mask; + + /* compute CC_AXIM_CACHE_PARAMS */ + cache_params = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS)); + dev_dbg(dev, "Cache params previous: 0x%08X\n", cache_params); + + /* non cached or write-back, write allocate */ + val = drvdata->coherent ? 0xb : 0x2; + + mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE); + cache_params &= ~mask; + cache_params |= FIELD_PREP(mask, val); + + mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE_LAST); + cache_params &= ~mask; + cache_params |= FIELD_PREP(mask, val); + + mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_ARCACHE); + cache_params &= ~mask; + cache_params |= FIELD_PREP(mask, val); + + drvdata->cache_params = cache_params; + + dev_dbg(dev, "Cache params current: 0x%08X\n", cache_params); + + if (drvdata->hw_rev <= CC_HW_REV_710) + return; + + /* compute CC_AXIM_ACE_CONST */ + ace_const = cc_ioread(drvdata, CC_REG(AXIM_ACE_CONST)); + dev_dbg(dev, "ACE-const previous: 0x%08X\n", ace_const); + + /* system or outer-sharable */ + val = drvdata->coherent ? 0x2 : 0x3; + + mask = CC_GENMASK(CC_AXIM_ACE_CONST_ARDOMAIN); + ace_const &= ~mask; + ace_const |= FIELD_PREP(mask, val); + + mask = CC_GENMASK(CC_AXIM_ACE_CONST_AWDOMAIN); + ace_const &= ~mask; + ace_const |= FIELD_PREP(mask, val); + + dev_dbg(dev, "ACE-const current: 0x%08X\n", ace_const); + + drvdata->ace_const = ace_const; +} + static u32 cc_read_idr(struct cc_drvdata *drvdata, const u32 *idr_offsets) { int i; @@ -218,9 +269,9 @@ bool cc_wait_for_reset_completion(struct cc_drvdata *drvdata) return false; } -int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe) +int init_cc_regs(struct cc_drvdata *drvdata) { - unsigned int val, cache_params; + unsigned int val; struct device *dev = drvdata_to_dev(drvdata); /* Unmask all AXI interrupt sources AXI_CFG1 register */ @@ -245,19 +296,9 @@ int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe) cc_iowrite(drvdata, CC_REG(HOST_IMR), ~val); - cache_params = (drvdata->coherent ? CC_COHERENT_CACHE_PARAMS : 0x0); - - val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS)); - - if (is_probe) - dev_dbg(dev, "Cache params previous: 0x%08X\n", val); - - cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), cache_params); - val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS)); - - if (is_probe) - dev_dbg(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n", - val, cache_params); + cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), drvdata->cache_params); + if (drvdata->hw_rev >= CC_HW_REV_712) + cc_iowrite(drvdata, CC_REG(AXIM_ACE_CONST), drvdata->ace_const); return 0; } @@ -445,7 +486,9 @@ static int init_cc_resources(struct platform_device *plat_dev) } dev_dbg(dev, "Registered to IRQ: %d\n", irq); - rc = init_cc_regs(new_drvdata, true); + init_cc_cache_params(new_drvdata); + + rc = init_cc_regs(new_drvdata); if (rc) { dev_err(dev, "init_cc_regs failed\n"); goto post_pm_err; diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h index af77b2020350..12cd68d59baa 100644 --- a/drivers/crypto/ccree/cc_driver.h +++ b/drivers/crypto/ccree/cc_driver.h @@ -49,8 +49,6 @@ enum cc_std_body { CC_STD_ALL = 0x3 }; -#define CC_COHERENT_CACHE_PARAMS 0xEEE - #define CC_PINS_FULL 0x0 #define CC_PINS_SLIM 0x9F @@ -155,6 +153,8 @@ struct cc_drvdata { int std_bodies; bool sec_disabled; u32 comp_mask; + u32 cache_params; + u32 ace_const; }; struct cc_crypto_alg { @@ -205,7 +205,7 @@ static inline void dump_byte_array(const char *name, const u8 *the_array, }
[RFC PATCH v3 2/2] ACPI: Advertise Interrupt ResourceSource support
As mentioned in ACPI v6.3, Table 6-200, the platform will indicate to the OS whether or not it supports usage of ResourceSource. If not set, the OS may choose to ignore the ResourceSource parameter in the extended interrupt descriptor. Since we support parsing ResoureSource field of interrupts both for platform devices and PCI Interrupt Link devices now, this patch sets the relevant OSC bit and checks the capability as described in ACPI specification. Signed-off-by: Chen Baozi Cc: Jonathan Cameron --- drivers/acpi/bus.c | 5 + drivers/acpi/irq.c | 3 ++- include/linux/acpi.h | 4 +++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 1682f8b454a2..a6af1270bea6 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -281,6 +281,8 @@ bool osc_sb_apei_support_acked; bool osc_pc_lpi_support_confirmed; EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed); +bool osc_sb_intr_ressrc_support_confirmed; + static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48"; static void acpi_bus_osc_support(void) { @@ -303,6 +305,7 @@ static void acpi_bus_osc_support(void) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT; capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT; + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_INTR_RESSRC_SUPPORT; #ifdef CONFIG_ARM64 capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_GENERIC_INITIATOR_SUPPORT; #endif @@ -328,6 +331,8 @@ static void acpi_bus_osc_support(void) capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; osc_pc_lpi_support_confirmed = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; + osc_sb_intr_ressrc_support_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_INTR_RESSRC_SUPPORT; } kfree(context.ret.pointer); } diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c index 2fff5401c3f3..649e09bbd612 100644 --- a/drivers/acpi/irq.c +++ b/drivers/acpi/irq.c @@ -106,7 +106,8 @@ acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source) acpi_handle handle; acpi_status status; - if (!source->string_length) + if (!osc_sb_intr_ressrc_support_confirmed || + !source->string_length) return acpi_gsi_domain_id; status = acpi_get_handle(NULL, source->string_ptr, ); diff --git a/include/linux/acpi.h b/include/linux/acpi.h index b182a267fe66..f9ca8e117f31 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -555,10 +555,12 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context); #define OSC_SB_PCLPI_SUPPORT 0x0080 #define OSC_SB_OSLPI_SUPPORT 0x0100 #define OSC_SB_CPC_DIVERSE_HIGH_SUPPORT0x1000 -#define OSC_SB_GENERIC_INITIATOR_SUPPORT 0x2000 +#define OSC_SB_INTR_RESSRC_SUPPORT 0x2000 +#define OSC_SB_GENERIC_INITIATOR_SUPPORT 0x0002 extern bool osc_sb_apei_support_acked; extern bool osc_pc_lpi_support_confirmed; +extern bool osc_sb_intr_ressrc_support_confirmed; /* PCI Host Bridge _OSC: Capabilities DWORD 2: Support Field */ #define OSC_PCI_EXT_CONFIG_SUPPORT 0x0001 -- 2.28.0
[RFC PATCH v3 1/2] PCI/ACPI: Add stacked IRQ domain support to PCI Interrupt Link
The ResourceSource field of an Extended Interrupt Descriptor was ignored when the driver is parsing _PRS method of PNP0C0F PCI Interrupt Link devices, which means PCI INTx would be always registered under the GSI domain. This patch introduces stacked IRQ domain support to PCI Interrupt Link devices for ACPI. With this support, we can populate the ResourceSource field in _PRS method of PCI Interrupt Link devices to refer to a device object that describes an interrupt controller as the following examples: Device (IXIU) { ... } Device(LINKA) { Name(_HID, EISAID("PNP0C0F")) Name(_PRS, ResourceTemplate(){ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, 0, "\\SB.IXIU") { 60 } }) ... } Signed-off-by: Chen Baozi --- drivers/acpi/internal.h | 12 drivers/acpi/irq.c | 34 -- drivers/acpi/pci_irq.c | 8 ++-- drivers/acpi/pci_link.c | 17 +++-- include/acpi/acpi_drivers.h | 2 +- include/linux/acpi.h| 10 ++ 6 files changed, 64 insertions(+), 19 deletions(-) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index e3638bafb941..38ebe24bce3b 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -88,6 +88,18 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent); acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context); void acpi_scan_table_handler(u32 event, void *table, void *context); +#ifdef CONFIG_ACPI_GENERIC_GSI +int acpi_register_irq(struct device *dev, u32 hwirq, int trigger, + int polarity, struct fwnode_handle *fwnode); +#else +static inline +int acpi_register_irq(struct device *dev, u32 hwirq, int trigger, + int polarity, struct fwnode_handle *fwnode) +{ + return acpi_register_gsi(dev, hwirq, trigger, polarity); +} +#endif + /* -- Device Node Initialization / Removal -- */ diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c index e209081d644b..2fff5401c3f3 100644 --- a/drivers/acpi/irq.c +++ b/drivers/acpi/irq.c @@ -38,6 +38,24 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) } EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); +int acpi_register_irq(struct device *dev, u32 hwirq, int trigger, + int polarity, struct fwnode_handle *fwnode) +{ + struct irq_fwspec fwspec; + + if (!fwnode) { + dev_warn(dev, "No registered irqchip for hwirq %d\n", hwirq); + return -EINVAL; + } + + fwspec.fwnode = fwnode; + fwspec.param[0] = hwirq; + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity); + fwspec.param_count = 2; + + return irq_create_fwspec_mapping(); +} + /** * acpi_register_gsi() - Map a GSI to a linux IRQ number * @dev: device for which IRQ has to be mapped @@ -51,19 +69,7 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) { - struct irq_fwspec fwspec; - - if (WARN_ON(!acpi_gsi_domain_id)) { - pr_warn("GSI: No registered irqchip, giving up\n"); - return -EINVAL; - } - - fwspec.fwnode = acpi_gsi_domain_id; - fwspec.param[0] = gsi; - fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity); - fwspec.param_count = 2; - - return irq_create_fwspec_mapping(); + return acpi_register_irq(dev, gsi, trigger, polarity, acpi_gsi_domain_id); } EXPORT_SYMBOL_GPL(acpi_register_gsi); @@ -92,7 +98,7 @@ EXPORT_SYMBOL_GPL(acpi_unregister_gsi); * Return: * The referenced device fwhandle or NULL on failure */ -static struct fwnode_handle * +struct fwnode_handle * acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source) { struct fwnode_handle *result; diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c index 14ee631cb7cf..50f70781 100644 --- a/drivers/acpi/pci_irq.c +++ b/drivers/acpi/pci_irq.c @@ -22,6 +22,8 @@ #include #include +#include "internal.h" + #define PREFIX "ACPI: " #define _COMPONENT ACPI_PCI_COMPONENT @@ -410,6 +412,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev) char *link = NULL; char link_desc[16]; int rc; + struct fwnode_handle *rs_fwnode; pin = dev->pin; if (!pin) { @@ -438,7 +441,8 @@ int acpi_pci_irq_enable(struct pci_dev *dev) gsi = acpi_pci_link_allocate_irq(entry->link, entry->index, , , -); +, +
Re: [External] Re: [PATCH v5 00/21] Free some vmemmap pages of hugetlb page
On Sat, Nov 21, 2020 at 1:47 AM Mike Kravetz wrote: > > On 11/20/20 1:43 AM, David Hildenbrand wrote: > > On 20.11.20 10:39, Michal Hocko wrote: > >> On Fri 20-11-20 10:27:05, David Hildenbrand wrote: > >>> On 20.11.20 09:42, Michal Hocko wrote: > On Fri 20-11-20 14:43:04, Muchun Song wrote: > [...] > > Thanks for improving the cover letter and providing some numbers. I have > only glanced through the patchset because I didn't really have more time > to dive depply into them. > > Overall it looks promissing. To summarize. I would prefer to not have > the feature enablement controlled by compile time option and the kernel > command line option should be opt-in. I also do not like that freeing > the pool can trigger the oom killer or even shut the system down if no > oom victim is eligible. > > One thing that I didn't really get to think hard about is what is the > effect of vmemmap manipulation wrt pfn walkers. pfn_to_page can be > invalid when racing with the split. How do we enforce that this won't > blow up? > >>> > >>> I have the same concerns - the sections are online the whole time and > >>> anybody with pfn_to_online_page() can grab them > >>> > >>> I think we have similar issues with memory offlining when removing the > >>> vmemmap, it's just very hard to trigger and we can easily protect by > >>> grabbing the memhotplug lock. > >> > >> I am not sure we can/want to span memory hotplug locking out to all pfn > >> walkers. But you are right that the underlying problem is similar but > >> much harder to trigger because vmemmaps are only removed when the > >> physical memory is hotremoved and that happens very seldom. Maybe it > >> will happen more with virtualization usecases. But this work makes it > >> even more tricky. If a pfn walker races with a hotremove then it would > >> just blow up when accessing the unmapped physical address space. For > >> this feature a pfn walker would just grab a real struct page re-used for > >> some unpredictable use under its feet. Any failure would be silent and > >> hard to debug. > > > > Right, we don't want the memory hotplug locking, thus discussions regarding > > rcu. Luckily, for now I never saw a BUG report regarding this - maybe > > because the time between memory offlining (offline_pages()) and > > memory/vmemmap getting removed (try_remove_memory()) is just too long. > > Someone would have to sleep after pfn_to_online_page() for quite a while to > > trigger it. > > > >> > >> [...] > >>> To keep things easy, maybe simply never allow to free these hugetlb pages > >>> again for now? If they were reserved during boot and the vmemmap > >>> condensed, > >>> then just let them stick around for all eternity. > >> > >> Not sure I understand. Do you propose to only free those vmemmap pages > >> when the pool is initialized during boot time and never allow to free > >> them up? That would certainly make it safer and maybe even simpler wrt > >> implementation. > > > > Exactly, let's keep it simple for now. I guess most use cases of this > > (virtualization, databases, ...) will allocate hugepages during boot and > > never free them. > > Not sure if I agree with that last statement. Database and virtualization > use cases from my employer allocate allocate hugetlb pages after boot. It > is shortly after boot, but still not from boot/kernel command line. > > Somewhat related, but not exactly addressing this issue ... > > One idea discussed in a previous patch set was to disable PMD/huge page > mapping of vmemmap if this feature was enabled. This would eliminate a bunch > of the complex code doing page table manipulation. It does not address > the issue of struct page pages going away which is being discussed here, > but it could be a way to simply the first version of this code. If this > is going to be an 'opt in' feature as previously suggested, then eliminating > the PMD/huge page vmemmap mapping may be acceptable. My guess is that > sysadmins would only 'opt in' if they expect most of system memory to be used > by hugetlb pages. We certainly have database and virtualization use cases > where this is true. Hi Mike, Yeah, I agree with you that the first version of this feature should be simply. I can do that (disable PMD/huge page mapping of vmemmap) in the next version patch. But I have another question: what the problem is when struct page pages go away? I have not understood the issues discussed here, hope you can answer for me. Thanks. > -- > Mike Kravetz -- Yours, Muchun
Notice-004
Once again we are trying to reach you as regards the estate of Late George Brumley, you were made one of the beneficiaries of his estate. Do get back to me at your earliest convenience. The Trustees
[PATCH] arm64: update RANDOMIZE_MODULE_REGION_FULL config description
module randomization is reduced. (range to 2 GB) RANDOMIZE_MODULE_REGION_FULL config description is not updated. update RANDOMIZE_MODULE_REGION_FULL config description. Signed-off-by: youngjun --- arch/arm64/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1515f6f153a0..0da551828a59 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1804,11 +1804,11 @@ config RANDOMIZE_BASE If unsure, say N. config RANDOMIZE_MODULE_REGION_FULL - bool "Randomize the module region over a 4 GB range" + bool "Randomize the module region over a 2 GB range" depends on RANDOMIZE_BASE default y help - Randomizes the location of the module region inside a 4 GB window + Randomizes the location of the module region inside a 2 GB window covering the core kernel. This way, it is less likely for modules to leak information about the location of core kernel data structures but it does imply that function calls between modules and the core -- 2.17.1
Re: [PATCH v2 05/12] x86: rework arch_local_irq_restore() to not use popf
On 20.11.20 12:59, Peter Zijlstra wrote: On Fri, Nov 20, 2020 at 12:46:23PM +0100, Juergen Gross wrote: +static __always_inline void arch_local_irq_restore(unsigned long flags) +{ + if (!arch_irqs_disabled_flags(flags)) + arch_local_irq_enable(); +} If someone were to write horrible code like: local_irq_disable(); local_irq_save(flags); local_irq_enable(); local_irq_restore(flags); we'd be up some creek without a paddle... now I don't _think_ we have genius code like that, but I'd feel saver if we can haz an assertion in there somewhere... Maybe something like: #ifdef CONFIG_DEBUG_ENTRY // for lack of something saner WARN_ON_ONCE((arch_local_save_flags() ^ flags) & X86_EFLAGS_IF); #endif At the end? I'd like to, but using WARN_ON_ONCE() in include/asm/irqflags.h sounds like a perfect receipt for include dependency hell. We could use a plain asm("ud2") instead. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 4.9 00/16] 4.9.245-rc1 review
On Fri, 20 Nov 2020 at 16:34, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.9.245 release. > There are 16 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 Sun, 22 Nov 2020 10:45:32 +. > 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/v4.x/stable-review/patch-4.9.245-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-4.9.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: 4.4.245-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.4.y git commit: 11095ab90e22ac875983239a445f6b4ad64b6e08 git describe: v4.4.244-16-g11095ab90e22 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-4.4.y/build/v4.4.244-16-g11095ab90e22 No regressions (compared to build v4.4.244) No fixes (compared to build v4.4.244) Ran 32775 total tests in the following environments and test suites. Environments -- - i386 - juno-r2 - arm64 - juno-r2-compat - juno-r2-kasan - qemu-arm64-clang - qemu-arm64-kasan - qemu-x86_64-clang - qemu-x86_64-kasan - qemu_arm - qemu_arm64 - qemu_arm64-compat - qemu_i386 - qemu_x86_64 - qemu_x86_64-compat - x15 - arm - x86_64 - x86-kasan Test Suites --- * build * 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-ipc-tests * ltp-math-tests * ltp-mm-tests * ltp-nptl-tests * ltp-open-posix-tests * ltp-pty-tests * ltp-sched-tests * ltp-securebits-tests * ltp-syscalls-tests * network-basic-tests * perf * v4l2-compliance * kvm-unit-tests * ltp-tracing-tests * install-android-platform-tools-r2600 Summary kernel: 4.4.245-rc1 git repo: https://git.linaro.org/lkft/arm64-stable-rc.git git branch: 4.4.245-rc1-hikey-20201120-861 git commit: a395e149575bc8d8ec23a677f979301bfefd8862 git describe: 4.4.245-rc1-hikey-20201120-861 Test details: https://qa-reports.linaro.org/lkft/linaro-hikey-stable-rc-4.4-oe/build/4.4.245-rc1-hikey-20201120-861 No regressions (compared to build 4.4.244-rc1-hikey-20201117-859) No fixes (compared to build 4.4.244-rc1-hikey-20201117-859) Ran 1722 total tests in the following environments and test suites. Environments -- - hi6220-hikey - arm64 Test Suites --- * build * install-android-platform-tools-r2600 * libhugetlbfs * linux-log-parser * ltp-cap_bounds-tests * ltp-commands-tests * ltp-containers-tests * ltp-cpuhotplug-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-nptl-tests * ltp-pty-tests * ltp-sched-tests * ltp-securebits-tests * ltp-syscalls-tests * perf * spectre-meltdown-checker-test * v4l2-compliance -- Linaro LKFT https://lkft.linaro.org
Re: [PATCH 4.9 00/16] 4.9.245-rc1 review
On Fri, 20 Nov 2020 at 16:34, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.9.245 release. > There are 16 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 Sun, 22 Nov 2020 10:45:32 +. > 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/v4.x/stable-review/patch-4.9.245-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-4.9.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: 4.9.245-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.9.y git commit: b75776b03db0bcff278a791d60b6ed02df615c1c git describe: v4.9.244-17-gb75776b03db0 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-4.9.y/build/v4.9.244-17-gb75776b03db0 No regressions (compared to build v4.9.244) No fixes (compared to build v4.9.244) Ran 40992 total tests in the following environments and test suites. Environments -- - dragonboard-410c - arm64 - hi6220-hikey - arm64 - i386 - juno-r2 - arm64 - qemu-arm64-clang - qemu-arm64-kasan - qemu-x86_64-clang - qemu-x86_64-kasan - qemu_arm - qemu_arm64 - qemu_arm64-compat - qemu_i386 - qemu_x86_64 - qemu_x86_64-compat - x15 - arm - x86_64 - x86-kasan Test Suites --- * build * install-android-platform-tools-r2600 * libhugetlbfs * linux-log-parser * ltp-cap_bounds-tests * ltp-commands-tests * ltp-containers-tests * ltp-controllers-tests * ltp-cpuhotplug-tests * ltp-crypto-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-sched-tests * ltp-securebits-tests * ltp-syscalls-tests * perf * v4l2-compliance * ltp-cve-tests * network-basic-tests * ltp-open-posix-tests * ltp-tracing-tests * kvm-unit-tests -- Linaro LKFT https://lkft.linaro.org
Re: [PATCH kernel v3] genirq/irqdomain: Add reference counting to IRQs
Hi Alexey, Thank you for the patch! Yet something to improve: [auto build test ERROR on tip/irq/core] [also build test ERROR on linus/master linux/master v5.10-rc4 next-20201120] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Alexey-Kardashevskiy/genirq-irqdomain-Add-reference-counting-to-IRQs/20201109-175020 base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git d315c627a18249930750fe4eb2b21f3fe9b32ea4 config: m68k-randconfig-m031-20201122 (attached as .config) compiler: m68k-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/3fe0622aa0aeca70507a5e71b599bed6be0be581 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Alexey-Kardashevskiy/genirq-irqdomain-Add-reference-counting-to-IRQs/20201109-175020 git checkout 3fe0622aa0aeca70507a5e71b599bed6be0be581 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): kernel/irq/irqdomain.c: In function 'irq_create_mapping': >> kernel/irq/irqdomain.c:672:20: error: 'struct irq_desc' has no member named >> 'kobj' 672 | kobject_get(>kobj); |^~ kernel/irq/irqdomain.c: In function 'irq_create_fwspec_mapping': kernel/irq/irqdomain.c:807:21: error: 'struct irq_desc' has no member named 'kobj' 807 |kobject_get(>kobj); | ^~ kernel/irq/irqdomain.c:822:21: error: 'struct irq_desc' has no member named 'kobj' 822 |kobject_get(>kobj); | ^~ kernel/irq/irqdomain.c: In function 'irq_dispose_mapping': kernel/irq/irqdomain.c:880:19: error: 'struct irq_desc' has no member named 'kobj' 880 | kobject_put(>kobj); | ^~ kernel/irq/irqdomain.c: In function '__irq_domain_alloc_irqs': kernel/irq/irqdomain.c:1473:21: error: 'struct irq_desc' has no member named 'kobj' 1473 |kobject_get(>kobj); | ^~ vim +672 kernel/irq/irqdomain.c 636 637 /** 638 * irq_create_mapping() - Map a hardware interrupt into linux irq space 639 * @domain: domain owning this hardware interrupt or NULL for default domain 640 * @hwirq: hardware irq number in that domain space 641 * 642 * Only one mapping per hardware interrupt is permitted. Returns a linux 643 * irq number. 644 * If the sense/trigger is to be specified, set_irq_type() should be called 645 * on the number returned from that call. 646 */ 647 unsigned int irq_create_mapping(struct irq_domain *domain, 648 irq_hw_number_t hwirq) 649 { 650 struct device_node *of_node; 651 int virq; 652 struct irq_desc *desc; 653 654 pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq); 655 656 /* Look for default domain if nececssary */ 657 if (domain == NULL) 658 domain = irq_default_domain; 659 if (domain == NULL) { 660 WARN(1, "%s(, %lx) called with NULL domain\n", __func__, hwirq); 661 return 0; 662 } 663 pr_debug("-> using domain @%p\n", domain); 664 665 of_node = irq_domain_get_of_node(domain); 666 667 /* Check if mapping already exists */ 668 virq = irq_find_mapping(domain, hwirq); 669 if (virq) { 670 desc = irq_to_desc(virq); 671 pr_debug("-> existing mapping on virq %d\n", virq); > 672 kobject_get(>kobj); 673 return virq; 674 } 675 676 /* Allocate a virtual interrupt number */ 677 virq = irq_domain_alloc_descs(-1, 1, hwirq, of_node_to_nid(of_node), NULL); 678 if (virq <= 0) { 679 pr_debug("-> virq allocation failed\n"); 680 return 0; 681 } 682 683 if (irq_domain_associate(domain, virq, hwirq)) { 684 irq_free_desc(virq); 685 return 0; 686 } 687 688 pr_debug("irq %lu on domain %s mapped to virtual irq %u\n", 689 hwirq, of_node_full_name(of_node), virq); 690 691 return virq; 692 } 693 EXPORT_SYMBOL_GPL(irq_create_mapping); 694 ---
[PATCH] platform/x86: toshiba_acpi: Fix the wrong variable assignment
From: Kaixu Xia The commit 78429e55e4057 ("platform/x86: toshiba_acpi: Clean up variable declaration") cleans up variable declaration in video_proc_write(). Seems it does the variable assignment in the wrong place, this results in dead code and changes the source code logic. Fix it by doing the assignment at the beginning of the funciton. Fixes: 78429e55e4057 ("platform/x86: toshiba_acpi: Clean up variable declaration") Reported-by: Tosk Robot Signed-off-by: Kaixu Xia --- drivers/platform/x86/toshiba_acpi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c index e557d757c647..fa7232ad8c39 100644 --- a/drivers/platform/x86/toshiba_acpi.c +++ b/drivers/platform/x86/toshiba_acpi.c @@ -1478,7 +1478,7 @@ static ssize_t video_proc_write(struct file *file, const char __user *buf, struct toshiba_acpi_dev *dev = PDE_DATA(file_inode(file)); char *buffer; char *cmd; - int lcd_out, crt_out, tv_out; + int lcd_out = -1, crt_out = -1, tv_out = -1; int remain = count; int value; int ret; @@ -1510,7 +1510,6 @@ static ssize_t video_proc_write(struct file *file, const char __user *buf, kfree(cmd); - lcd_out = crt_out = tv_out = -1; ret = get_video_status(dev, _out); if (!ret) { unsigned int new_video_out = video_out; -- 2.20.0
[PATCH v3 1/4] remoteproc: sysmon: Ensure remote notification ordering
The reliance on the remoteproc's state for determining when to send sysmon notifications to a remote processor is racy with regard to concurrent remoteproc operations. Further more the advertisement of the state of other remote processor to a newly started remote processor might not only send the wrong state, but might result in a stream of state changes that are out of order. Address this by introducing state tracking within the sysmon instances themselves and extend the locking to ensure that the notifications are consistent with this state. Fixes: 1f36ab3f6e3b ("remoteproc: sysmon: Inform current rproc about all active rprocs") Fixes: 1877f54f75ad ("remoteproc: sysmon: Add notifications for events") Fixes: 1fb82ee806d1 ("remoteproc: qcom: Introduce sysmon") Cc: sta...@vger.kernel.org Signed-off-by: Bjorn Andersson --- Changes since v2: - Hold sysmon_lock during traversal of sysmons in sysmon_start() drivers/remoteproc/qcom_sysmon.c | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c index 9eb2f6bccea6..b37b111b15b3 100644 --- a/drivers/remoteproc/qcom_sysmon.c +++ b/drivers/remoteproc/qcom_sysmon.c @@ -22,6 +22,9 @@ struct qcom_sysmon { struct rproc_subdev subdev; struct rproc *rproc; + int state; + struct mutex state_lock; + struct list_head node; const char *name; @@ -448,7 +451,10 @@ static int sysmon_prepare(struct rproc_subdev *subdev) .ssr_event = SSCTL_SSR_EVENT_BEFORE_POWERUP }; + mutex_lock(>state_lock); + sysmon->state = SSCTL_SSR_EVENT_BEFORE_POWERUP; blocking_notifier_call_chain(_notifiers, 0, (void *)); + mutex_unlock(>state_lock); return 0; } @@ -472,20 +478,25 @@ static int sysmon_start(struct rproc_subdev *subdev) .ssr_event = SSCTL_SSR_EVENT_AFTER_POWERUP }; + mutex_lock(>state_lock); + sysmon->state = SSCTL_SSR_EVENT_AFTER_POWERUP; blocking_notifier_call_chain(_notifiers, 0, (void *)); + mutex_unlock(>state_lock); mutex_lock(_lock); list_for_each_entry(target, _list, node) { - if (target == sysmon || - target->rproc->state != RPROC_RUNNING) + if (target == sysmon) continue; + mutex_lock(>state_lock); event.subsys_name = target->name; + event.ssr_event = target->state; if (sysmon->ssctl_version == 2) ssctl_send_event(sysmon, ); else if (sysmon->ept) sysmon_send_event(sysmon, ); + mutex_unlock(>state_lock); } mutex_unlock(_lock); @@ -500,7 +511,10 @@ static void sysmon_stop(struct rproc_subdev *subdev, bool crashed) .ssr_event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN }; + mutex_lock(>state_lock); + sysmon->state = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN; blocking_notifier_call_chain(_notifiers, 0, (void *)); + mutex_unlock(>state_lock); /* Don't request graceful shutdown if we've crashed */ if (crashed) @@ -521,7 +535,10 @@ static void sysmon_unprepare(struct rproc_subdev *subdev) .ssr_event = SSCTL_SSR_EVENT_AFTER_SHUTDOWN }; + mutex_lock(>state_lock); + sysmon->state = SSCTL_SSR_EVENT_AFTER_SHUTDOWN; blocking_notifier_call_chain(_notifiers, 0, (void *)); + mutex_unlock(>state_lock); } /** @@ -534,11 +551,10 @@ static int sysmon_notify(struct notifier_block *nb, unsigned long event, void *data) { struct qcom_sysmon *sysmon = container_of(nb, struct qcom_sysmon, nb); - struct rproc *rproc = sysmon->rproc; struct sysmon_event *sysmon_event = data; /* Skip non-running rprocs and the originating instance */ - if (rproc->state != RPROC_RUNNING || + if (sysmon->state != SSCTL_SSR_EVENT_AFTER_POWERUP || !strcmp(sysmon_event->subsys_name, sysmon->name)) { dev_dbg(sysmon->dev, "not notifying %s\n", sysmon->name); return NOTIFY_DONE; @@ -591,6 +607,7 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc, init_completion(>ind_comp); init_completion(>shutdown_comp); mutex_init(>lock); + mutex_init(>state_lock); sysmon->shutdown_irq = of_irq_get_byname(sysmon->dev->of_node, "shutdown-ack"); -- 2.28.0
[PATCH v3 2/4] remoteproc: sysmon: Expose the shutdown result
A graceful shutdown of the Qualcomm remote processors where traditionally performed by invoking a shared memory state signal and waiting for the associated ack. This was later superseded by the "sysmon" mechanism, where some form of shared memory bus is used to send a "graceful shutdown request" message and one of more signals comes back to indicate its success. But when this newer mechanism is in effect the firmware is shut down by the time the older mechanism, implemented in the remoteproc drivers, attempts to perform a graceful shutdown - and as such it will never receive an ack back. This patch therefor track the success of the latest shutdown attempt in sysmon and exposes a new function in the API that the remoteproc driver can use to query the success and the necessity of invoking the older mechanism. Tested-by: Steev Klimaszewski Reviewed-by: Rishabh Bhatnagar Signed-off-by: Bjorn Andersson --- Change since v2: - None drivers/remoteproc/qcom_common.h | 6 +++ drivers/remoteproc/qcom_sysmon.c | 82 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h index dfc641c3a98b..8ba9052955bd 100644 --- a/drivers/remoteproc/qcom_common.h +++ b/drivers/remoteproc/qcom_common.h @@ -51,6 +51,7 @@ struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc, const char *name, int ssctl_instance); void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon); +bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon); #else static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc, const char *name, @@ -62,6 +63,11 @@ static inline struct qcom_sysmon *qcom_add_sysmon_subdev(struct rproc *rproc, static inline void qcom_remove_sysmon_subdev(struct qcom_sysmon *sysmon) { } + +static inline bool qcom_sysmon_shutdown_acked(struct qcom_sysmon *sysmon) +{ + return false; +} #endif #endif diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c index b37b111b15b3..a428b707a6de 100644 --- a/drivers/remoteproc/qcom_sysmon.c +++ b/drivers/remoteproc/qcom_sysmon.c @@ -44,6 +44,7 @@ struct qcom_sysmon { struct mutex lock; bool ssr_ack; + bool shutdown_acked; struct qmi_handle qmi; struct sockaddr_qrtr ssctl; @@ -115,10 +116,13 @@ static void sysmon_send_event(struct qcom_sysmon *sysmon, /** * sysmon_request_shutdown() - request graceful shutdown of remote * @sysmon:sysmon context + * + * Return: boolean indicator of the remote processor acking the request */ -static void sysmon_request_shutdown(struct qcom_sysmon *sysmon) +static bool sysmon_request_shutdown(struct qcom_sysmon *sysmon) { char *req = "ssr:shutdown"; + bool acked = false; int ret; mutex_lock(>lock); @@ -141,9 +145,13 @@ static void sysmon_request_shutdown(struct qcom_sysmon *sysmon) if (!sysmon->ssr_ack) dev_err(sysmon->dev, "unexpected response to sysmon shutdown request\n"); + else + acked = true; out_unlock: mutex_unlock(>lock); + + return acked; } static int sysmon_callback(struct rpmsg_device *rpdev, void *data, int count, @@ -297,14 +305,33 @@ static struct qmi_msg_handler qmi_indication_handler[] = { {} }; +static bool ssctl_request_shutdown_wait(struct qcom_sysmon *sysmon) +{ + int ret; + + ret = wait_for_completion_timeout(>shutdown_comp, 10 * HZ); + if (ret) + return true; + + ret = try_wait_for_completion(>ind_comp); + if (ret) + return true; + + dev_err(sysmon->dev, "timeout waiting for shutdown ack\n"); + return false; +} + /** * ssctl_request_shutdown() - request shutdown via SSCTL QMI service * @sysmon:sysmon context + * + * Return: boolean indicator of the remote processor acking the request */ -static void ssctl_request_shutdown(struct qcom_sysmon *sysmon) +static bool ssctl_request_shutdown(struct qcom_sysmon *sysmon) { struct ssctl_shutdown_resp resp; struct qmi_txn txn; + bool acked = false; int ret; reinit_completion(>ind_comp); @@ -312,7 +339,7 @@ static void ssctl_request_shutdown(struct qcom_sysmon *sysmon) ret = qmi_txn_init(>qmi, , ssctl_shutdown_resp_ei, ); if (ret < 0) { dev_err(sysmon->dev, "failed to allocate QMI txn\n"); - return; + return false; } ret = qmi_send_request(>qmi, >ssctl, , @@ -320,27 +347,23 @@ static void ssctl_request_shutdown(struct qcom_sysmon *sysmon) if (ret < 0) { dev_err(sysmon->dev, "failed to send shutdown request\n"); qmi_txn_cancel(); - return; +
Re: [RFC PATCH v2 00/26] Make reporting-bugs easier to grasp and yet more detailed & helpful
On 11/21/20 9:33 PM, Thorsten Leemhuis wrote: > Am 20.11.20 um 22:58 schrieb Jonathan Corbet: >> On Fri, 20 Nov 2020 11:46:07 +0100 >> Thorsten Leemhuis wrote: >>> Am 19.11.20 um 01:29 schrieb Jonathan Corbet: On Sun, 15 Nov 2020 11:13:52 +0100 Thorsten Leemhuis wrote: >>> - Collapse the whole thing down to a patch adding reporting-bugs-v2.rst (or some suitable name). I do wonder if it should also move to the process manual as part of this; not only admins will report bugs. >>> After a night's sleep and Randy's comment I for now settled on >>> Documentation/admin-guide/reporting-issues.rst >> Keeping it in the admin guide is OK. Not sure about the name, though; if >> you're really dead set against bugs, maybe reporting-problems.rst? > > Well, I'm not dead set against bugs, but it somehow seems wrong to me: people > have problems/issues they deal with, which in the end might turn out to not > be a bug/error in the code at all. That afaics why tracker software for such > reports is often called "issue tracker" instead of "bug tracker" (and nearly > nobody calls them problem trackers afaics).. That's why I went with "issues" > in the name and the text. > > But in the end I'm not a native English speaker, so I guess it's better if I > follow advice from those. Randy, what would you choose? I'm fine with "issues." I do recall that at my first job (that was in the previous century or previous millennium) they were called "trouble reports." :) -- ~Randy
[PATCH v3 4/4] remoteproc: sysmon: Improve error messages
Improve the style of a few of the error messages printed by the sysmon implementation and fix the copy-pasted shutdown error in the send-event function. Tested-by: Steev Klimaszewski Reviewed-by: Rishabh Bhatnagar Signed-off-by: Bjorn Andersson --- Change since v2: - None drivers/remoteproc/qcom_sysmon.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c index a428b707a6de..9fed11a2b4ba 100644 --- a/drivers/remoteproc/qcom_sysmon.c +++ b/drivers/remoteproc/qcom_sysmon.c @@ -352,9 +352,9 @@ static bool ssctl_request_shutdown(struct qcom_sysmon *sysmon) ret = qmi_txn_wait(, 5 * HZ); if (ret < 0) { - dev_err(sysmon->dev, "failed receiving QMI response\n"); + dev_err(sysmon->dev, "timeout waiting for shutdown response\n"); } else if (resp.resp.result) { - dev_err(sysmon->dev, "shutdown request failed\n"); + dev_err(sysmon->dev, "shutdown request rejected\n"); } else { dev_dbg(sysmon->dev, "shutdown request completed\n"); acked = true; @@ -397,18 +397,18 @@ static void ssctl_send_event(struct qcom_sysmon *sysmon, SSCTL_SUBSYS_EVENT_REQ, 40, ssctl_subsys_event_req_ei, ); if (ret < 0) { - dev_err(sysmon->dev, "failed to send shutdown request\n"); + dev_err(sysmon->dev, "failed to send subsystem event\n"); qmi_txn_cancel(); return; } ret = qmi_txn_wait(, 5 * HZ); if (ret < 0) - dev_err(sysmon->dev, "failed receiving QMI response\n"); + dev_err(sysmon->dev, "timeout waiting for subsystem event response\n"); else if (resp.resp.result) - dev_err(sysmon->dev, "ssr event send failed\n"); + dev_err(sysmon->dev, "subsystem event rejected\n"); else - dev_dbg(sysmon->dev, "ssr event send completed\n"); + dev_dbg(sysmon->dev, "subsystem event accepted\n"); } /** -- 2.28.0
[PATCH v3 0/4] remoteproc: Improvement for the Qualcomm sysmon
The core part of this series is the update to the sysmon driver to ensure that notifications sent to the remote processor are consistent and always present valid state transitions. In testing this I finally took the time to fix up the issue of the SMP2P based graceful shutdown in the remoteproc drivers always timing out if sysmon has already successfully shut down the remote processor. Bjorn Andersson (4): remoteproc: sysmon: Ensure remote notification ordering remoteproc: sysmon: Expose the shutdown result remoteproc: qcom: q6v5: Query sysmon before graceful shutdown remoteproc: sysmon: Improve error messages drivers/remoteproc/qcom_common.h| 6 ++ drivers/remoteproc/qcom_q6v5.c | 8 +- drivers/remoteproc/qcom_q6v5.h | 3 +- drivers/remoteproc/qcom_q6v5_adsp.c | 2 +- drivers/remoteproc/qcom_q6v5_mss.c | 2 +- drivers/remoteproc/qcom_q6v5_pas.c | 2 +- drivers/remoteproc/qcom_q6v5_wcss.c | 2 +- drivers/remoteproc/qcom_sysmon.c| 119 +--- 8 files changed, 109 insertions(+), 35 deletions(-) -- 2.28.0
[PATCH v3 3/4] remoteproc: qcom: q6v5: Query sysmon before graceful shutdown
Requesting a graceful shutdown through the shared memory state signals will not be acked in the event that sysmon has already successfully shut down the remote firmware. So extend the stop request API to optionally take the remoteproc's sysmon instance and query if there's already been a successful shutdown attempt, before doing the signal dance. Tested-by: Steev Klimaszewski Reviewed-by: Rishabh Bhatnagar Signed-off-by: Bjorn Andersson --- Change since v2: - Fixed spelling of optionally in commit message drivers/remoteproc/qcom_q6v5.c | 8 +++- drivers/remoteproc/qcom_q6v5.h | 3 ++- drivers/remoteproc/qcom_q6v5_adsp.c | 2 +- drivers/remoteproc/qcom_q6v5_mss.c | 2 +- drivers/remoteproc/qcom_q6v5_pas.c | 2 +- drivers/remoteproc/qcom_q6v5_wcss.c | 2 +- 6 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c index fd6fd36268d9..9627a950928e 100644 --- a/drivers/remoteproc/qcom_q6v5.c +++ b/drivers/remoteproc/qcom_q6v5.c @@ -13,6 +13,7 @@ #include #include #include +#include "qcom_common.h" #include "qcom_q6v5.h" #define Q6V5_PANIC_DELAY_MS200 @@ -146,15 +147,20 @@ static irqreturn_t q6v5_stop_interrupt(int irq, void *data) /** * qcom_q6v5_request_stop() - request the remote processor to stop * @q6v5: reference to qcom_q6v5 context + * @sysmon:reference to the remote's sysmon instance, or NULL * * Return: 0 on success, negative errno on failure */ -int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5) +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5, struct qcom_sysmon *sysmon) { int ret; q6v5->running = false; + /* Don't perform SMP2P dance if sysmon already shut down the remote */ + if (qcom_sysmon_shutdown_acked(sysmon)) + return 0; + qcom_smem_state_update_bits(q6v5->state, BIT(q6v5->stop_bit), BIT(q6v5->stop_bit)); diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h index c4ed887c1499..1c212f670cbc 100644 --- a/drivers/remoteproc/qcom_q6v5.h +++ b/drivers/remoteproc/qcom_q6v5.h @@ -8,6 +8,7 @@ struct rproc; struct qcom_smem_state; +struct qcom_sysmon; struct qcom_q6v5 { struct device *dev; @@ -40,7 +41,7 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5); int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5); -int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5); +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5, struct qcom_sysmon *sysmon); int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout); unsigned long qcom_q6v5_panic(struct qcom_q6v5 *q6v5); diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c index f0b7363b5b26..2f8f38408eb7 100644 --- a/drivers/remoteproc/qcom_q6v5_adsp.c +++ b/drivers/remoteproc/qcom_q6v5_adsp.c @@ -266,7 +266,7 @@ static int adsp_stop(struct rproc *rproc) int handover; int ret; - ret = qcom_q6v5_request_stop(>q6v5); + ret = qcom_q6v5_request_stop(>q6v5, adsp->sysmon); if (ret == -ETIMEDOUT) dev_err(adsp->dev, "timed out on wait\n"); diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c index ac289e08062e..55f7c5740920 100644 --- a/drivers/remoteproc/qcom_q6v5_mss.c +++ b/drivers/remoteproc/qcom_q6v5_mss.c @@ -1373,7 +1373,7 @@ static int q6v5_stop(struct rproc *rproc) struct q6v5 *qproc = (struct q6v5 *)rproc->priv; int ret; - ret = qcom_q6v5_request_stop(>q6v5); + ret = qcom_q6v5_request_stop(>q6v5, qproc->sysmon); if (ret == -ETIMEDOUT) dev_err(qproc->dev, "timed out on wait\n"); diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index 0678b417707e..49ea0133ff04 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -217,7 +217,7 @@ static int adsp_stop(struct rproc *rproc) int handover; int ret; - ret = qcom_q6v5_request_stop(>q6v5); + ret = qcom_q6v5_request_stop(>q6v5, adsp->sysmon); if (ret == -ETIMEDOUT) dev_err(adsp->dev, "timed out on wait\n"); diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c index 8846ef0b0f1a..d6639856069b 100644 --- a/drivers/remoteproc/qcom_q6v5_wcss.c +++ b/drivers/remoteproc/qcom_q6v5_wcss.c @@ -390,7 +390,7 @@ static int q6v5_wcss_stop(struct rproc *rproc) int ret; /* WCSS powerdown */ - ret = qcom_q6v5_request_stop(>q6v5); + ret = qcom_q6v5_request_stop(>q6v5, wcss->sysmon); if (ret == -ETIMEDOUT) { dev_err(wcss->dev, "timed out on wait\n"); return ret; -- 2.28.0
Re: [RFC PATCH v2 00/26] Make reporting-bugs easier to grasp and yet more detailed & helpful
Am 20.11.20 um 22:58 schrieb Jonathan Corbet: On Fri, 20 Nov 2020 11:46:07 +0100 Thorsten Leemhuis wrote: Am 19.11.20 um 01:29 schrieb Jonathan Corbet: On Sun, 15 Nov 2020 11:13:52 +0100 Thorsten Leemhuis wrote: - Collapse the whole thing down to a patch adding reporting-bugs-v2.rst (or some suitable name). I do wonder if it should also move to the process manual as part of this; not only admins will report bugs. After a night's sleep and Randy's comment I for now settled on Documentation/admin-guide/reporting-issues.rst Keeping it in the admin guide is OK. Not sure about the name, though; if you're really dead set against bugs, maybe reporting-problems.rst? Well, I'm not dead set against bugs, but it somehow seems wrong to me: people have problems/issues they deal with, which in the end might turn out to not be a bug/error in the code at all. That afaics why tracker software for such reports is often called "issue tracker" instead of "bug tracker" (and nearly nobody calls them problem trackers afaics).. That's why I went with "issues" in the name and the text. But in the end I'm not a native English speaker, so I guess it's better if I follow advice from those. Randy, what would you choose? I'd be a bit more straightforward: This document is obsolete, and will be replaced by Documentation/admin-guide/$NAME in the near future. Not sure that more is really needed? Totally fine for me (I guess I tried to be less bold and was overly careful). @Jonathan, one more question: when I submit this again, should I CC more people (Linus, Greg, ?) to give them a chance to speak up before this lands in your tree? Ciao, Thorsten
Re: [PATCH v2 2/3] remoteproc: Introduce deny_sysfs_ops flag
On Fri 20 Nov 21:44 CST 2020, Suman Anna wrote: > On 11/20/20 9:38 PM, Bjorn Andersson wrote: > > On Fri 20 Nov 21:01 CST 2020, Suman Anna wrote: > > > >> The remoteproc framework provides sysfs interfaces for changing > >> the firmware name and for starting/stopping a remote processor > >> through the sysfs files 'state' and 'firmware'. The 'recovery' > >> sysfs file can also be used similarly to control the error recovery > >> state machine of a remoteproc. These interfaces are currently > >> allowed irrespective of how the remoteprocs were booted (like > >> remoteproc self auto-boot, remoteproc client-driven boot etc). > >> These interfaces can adversely affect a remoteproc and its clients > >> especially when a remoteproc is being controlled by a remoteproc > >> client driver(s). Also, not all remoteproc drivers may want to > >> support the sysfs interfaces by default. > >> > >> Add support to deny the sysfs state/firmware/recovery change by > >> introducing a state flag 'deny_sysfs_ops' that the individual > >> remoteproc drivers can set based on their usage needs. The default > >> behavior is to allow the sysfs operations as before. > >> > > > > This makes sense, but can't we implement attribute_group->is_visible to > > simply hide these entries from userspace instead of leaving them > > "broken"? > > I would have to look into that, but can that be changed dynamically? > Also, note that the enforcement is only on the writes/stores which impact > the state-machine, but not the reads/shows. > > For PRU usecases, we will be setting this dynamically. > It looks to be dynamic, but I don't know if there's any "caching" involved. Please have a look and let me know. Regards, Bjorn > regards > Suman > > > > > Regards, > > Bjorn > > > >> Signed-off-by: Suman Anna > >> --- > >> v2: revised to account for the 'recovery' sysfs file as well, patch > >> description updated accordingly > >> v1: > >> https://patchwork.kernel.org/project/linux-remoteproc/patch/20180915003725.17549-5-s-a...@ti.com/ > >> > >> drivers/remoteproc/remoteproc_sysfs.c | 12 > >> include/linux/remoteproc.h| 2 ++ > >> 2 files changed, 14 insertions(+) > >> > >> diff --git a/drivers/remoteproc/remoteproc_sysfs.c > >> b/drivers/remoteproc/remoteproc_sysfs.c > >> index bd2950a246c9..3fd18a71c188 100644 > >> --- a/drivers/remoteproc/remoteproc_sysfs.c > >> +++ b/drivers/remoteproc/remoteproc_sysfs.c > >> @@ -49,6 +49,10 @@ static ssize_t recovery_store(struct device *dev, > >> { > >>struct rproc *rproc = to_rproc(dev); > >> > >> + /* restrict sysfs operations if not allowed by remoteproc drivers */ > >> + if (rproc->deny_sysfs_ops) > >> + return -EPERM; > >> + > >>if (sysfs_streq(buf, "enabled")) { > >>/* change the flag and begin the recovery process if needed */ > >>rproc->recovery_disabled = false; > >> @@ -158,6 +162,10 @@ static ssize_t firmware_store(struct device *dev, > >>char *p; > >>int err, len = count; > >> > >> + /* restrict sysfs operations if not allowed by remoteproc drivers */ > >> + if (rproc->deny_sysfs_ops) > >> + return -EPERM; > >> + > >>err = mutex_lock_interruptible(>lock); > >>if (err) { > >>dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err); > >> @@ -225,6 +233,10 @@ static ssize_t state_store(struct device *dev, > >>struct rproc *rproc = to_rproc(dev); > >>int ret = 0; > >> > >> + /* restrict sysfs operations if not allowed by remoteproc drivers */ > >> + if (rproc->deny_sysfs_ops) > >> + return -EPERM; > >> + > >>if (sysfs_streq(buf, "start")) { > >>if (rproc->state == RPROC_RUNNING) > >>return -EBUSY; > >> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h > >> index 3fa3ba6498e8..dbc3767f7d0e 100644 > >> --- a/include/linux/remoteproc.h > >> +++ b/include/linux/remoteproc.h > >> @@ -508,6 +508,7 @@ struct rproc_dump_segment { > >> * @has_iommu: flag to indicate if remote processor is behind an MMU > >> * @auto_boot: flag to indicate if remote processor should be auto-started > >> * @autonomous: true if an external entity has booted the remote processor > >> + * @deny_sysfs_ops: flag to not permit sysfs operations on state, > >> firmware and recovery > >> * @dump_segments: list of segments in the firmware > >> * @nb_vdev: number of vdev currently handled by rproc > >> * @char_dev: character device of the rproc > >> @@ -545,6 +546,7 @@ struct rproc { > >>bool has_iommu; > >>bool auto_boot; > >>bool autonomous; > >> + bool deny_sysfs_ops; > >>struct list_head dump_segments; > >>int nb_vdev; > >>u8 elf_class; > >> -- > >> 2.28.0 > >> >
Re: [PATCH] remoteproc: Add module parameter 'auto_boot'
On Sat 21 Nov 12:38 CST 2020, Paul Cercueil wrote: > Hi Mathieu, > > Le ven. 20 nov. 2020 à 15:37, Mathieu Poirier a > écrit : > > Hi Paul, > > > > On Sun, Nov 15, 2020 at 11:50:56AM +, Paul Cercueil wrote: > > > Until now the remoteproc core would always default to trying to > > > boot the > > > remote processor at startup. The various remoteproc drivers could > > > however override that setting. > > > > > > Whether or not we want the remote processor to boot, really depends > > > on > > > the nature of the processor itself - a processor built into a WiFi > > > chip > > > will need to be booted for the WiFi hardware to be usable, for > > > instance, > > > but a general-purpose co-processor does not have any predeterminated > > > function, and as such we cannot assume that the OS will want the > > > processor to be booted - yet alone that we have a single do-it-all > > > firmware to load. > > > > > > > If I understand correctly you have various remote processors that use > > the same firmware > > but are serving different purposes - is this correct? > > That's the opposite actually. I have one remote processor which is > general-purpose, and as such userspace may or may not want it started at > boot time - depending on what it wants to do with it. The kernel shouldn't > decide itself whether or not the remote processor should be started, because > that's policy. > > > > > > Add a 'auto_boot' module parameter that instructs the remoteproc > > > whether > > > or not it should auto-boot the remote processor, which will default > > > to > > > "true" to respect the previous behaviour. > > > > > > > Given that the core can't be a module I wonder if this isn't something > > that > > would be better off in the specific platform driver or the device > > tree... Other > > people might have an opinion as well. > > Hardcoded in the platform driver or flagged in the device tree, doesn't > change the fundamental problem - it should be up to the userspace to decide > whether or not the remote processor should boot. > Unfortunately it depends on what you're using your remoteprocs for. And in a system with multiple remoteproc instances I don't think a single global parameter is sufficient - not even a per-driver setting is. I do agree with you that there are types of systems where the decision to auto boot things would happen after the kernel and/or DT has been written. Regards, Bjorn > Cheers, > -Paul > > > > > > Signed-off-by: Paul Cercueil > > > --- > > > drivers/remoteproc/remoteproc_core.c | 7 ++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/remoteproc/remoteproc_core.c > > > b/drivers/remoteproc/remoteproc_core.c > > > index dab2c0f5caf0..687b1bfd49db 100644 > > > --- a/drivers/remoteproc/remoteproc_core.c > > > +++ b/drivers/remoteproc/remoteproc_core.c > > > @@ -44,6 +44,11 @@ > > > > > > #define HIGH_BITS_MASK 0xULL > > > > > > +static bool auto_boot = true; > > > +module_param(auto_boot, bool, 0400); > > > +MODULE_PARM_DESC(auto_boot, > > > + "Auto-boot the remote processor [default=true]"); > > > + > > > static DEFINE_MUTEX(rproc_list_mutex); > > > static LIST_HEAD(rproc_list); > > > static struct notifier_block rproc_panic_nb; > > > @@ -2176,7 +2181,7 @@ struct rproc *rproc_alloc(struct device *dev, > > > const char *name, > > > return NULL; > > > > > > rproc->priv = [1]; > > > -rproc->auto_boot = true; > > > +rproc->auto_boot = auto_boot; > > > rproc->elf_class = ELFCLASSNONE; > > > rproc->elf_machine = EM_NONE; > > > > > > -- > > > 2.29.2 > > > > >
Re: [PATCH] ACPI: Let ACPI know we support Generic Initiator Affinity Structures
Hi Jonathan, I have found the value of OSC_SB_GENERIC_INITIATOR_SUPPORT is wrong when reading source code of driver/acpi/bus.c in the linux-next On Wed, Sep 30, 2020 at 10:05:44PM +0800, Jonathan Cameron wrote: > Until we tell ACPI that we support generic initiators, it will have > to operate in fall back domain mode and all _PXM entries should > be on existing non GI domains. > > This patch sets the relevant OSC bit to make that happen. > > Signed-off-by: Jonathan Cameron > Signed-off-by: Rafael J. Wysocki > --- > drivers/acpi/bus.c | 4 > include/linux/acpi.h | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 54002670cb7a..113c661eb848 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -303,7 +303,11 @@ static void acpi_bus_osc_support(void) > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT; > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT; > > +#ifdef CONFIG_ARM64 > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_GENERIC_INITIATOR_SUPPORT; > +#endif > #ifdef CONFIG_X86 > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_GENERIC_INITIATOR_SUPPORT; > if (boot_cpu_has(X86_FEATURE_HWP)) { > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT; > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT; > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index e9f6cd67943e..edbf3c4116b4 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -545,6 +545,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct > acpi_osc_context *context); > #define OSC_SB_PCLPI_SUPPORT 0x0080 > #define OSC_SB_OSLPI_SUPPORT 0x0100 > #define OSC_SB_CPC_DIVERSE_HIGH_SUPPORT 0x1000 > +#define OSC_SB_GENERIC_INITIATOR_SUPPORT 0x2000 Since the Generic Initiator Support is the Bit 17, it should be 0x2 rather than 0x2000. Baozi.
Re: [PATCH 2/2] hwspinlock: add sunxi hardware spinlock support
On Wed 18 Nov 04:02 CST 2020, Wilken Gottwalt wrote: > Adds the sunxi_hwspinlock driver and updates makefiles/maintainers. > > Signed-off-by: Wilken Gottwalt > --- > MAINTAINERS | 6 + > drivers/hwspinlock/Kconfig| 9 + > drivers/hwspinlock/Makefile | 1 + > drivers/hwspinlock/sunxi_hwspinlock.c | 282 ++ > 4 files changed, 298 insertions(+) > create mode 100644 drivers/hwspinlock/sunxi_hwspinlock.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 5cc595ac7b28..7765da172f10 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -722,6 +722,12 @@ L: linux-cry...@vger.kernel.org > S: Maintained > F: drivers/crypto/allwinner/ > > +ALLWINNER HARDWARE SPINLOCK SUPPORT > +M: Wilken Gottwalt > +S: Maintained > +F: Documentation/devicetree/bindings/hwlock/sunxi-hwspinlock.yaml > +F: drivers/hwspinlock/sunxi_hwspinlock.c > + > ALLWINNER THERMAL DRIVER > M: Vasily Khoruzhick > M: Yangtao Li > diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig > index 32cd26352f38..27b6e22126f7 100644 > --- a/drivers/hwspinlock/Kconfig > +++ b/drivers/hwspinlock/Kconfig > @@ -55,6 +55,15 @@ config HWSPINLOCK_STM32 > > If unsure, say N. > > +config HWSPINLOCK_SUNXI > + tristate "SUNXI Hardware Spinlock device" > + depends on ARCH_SUNXI || COMPILE_TEST > + help > + Say y here to support the Allwinner hardware mutex device available > + in the H2+, H3, H5 and H6 SoCs. > + > + If unsure, say N. > + > config HSEM_U8500 > tristate "STE Hardware Semaphore functionality" > depends on ARCH_U8500 || COMPILE_TEST > diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile > index ed053e3f02be..bf46bee95226 100644 > --- a/drivers/hwspinlock/Makefile > +++ b/drivers/hwspinlock/Makefile > @@ -9,4 +9,5 @@ obj-$(CONFIG_HWSPINLOCK_QCOM) += qcom_hwspinlock.o > obj-$(CONFIG_HWSPINLOCK_SIRF)+= sirf_hwspinlock.o > obj-$(CONFIG_HWSPINLOCK_SPRD)+= sprd_hwspinlock.o > obj-$(CONFIG_HWSPINLOCK_STM32) += stm32_hwspinlock.o > +obj-$(CONFIG_HWSPINLOCK_SUNXI) += sunxi_hwspinlock.o > obj-$(CONFIG_HSEM_U8500) += u8500_hsem.o > diff --git a/drivers/hwspinlock/sunxi_hwspinlock.c > b/drivers/hwspinlock/sunxi_hwspinlock.c > new file mode 100644 > index ..54e6ad3ac1c2 > --- /dev/null > +++ b/drivers/hwspinlock/sunxi_hwspinlock.c > @@ -0,0 +1,282 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * sunxi_hwspinlock.c - hardware spinlock driver for Allwinner SoCs > + * Copyright (C) 2020 Wilken Gottwalt > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "hwspinlock_internal.h" > + > +#define DRIVER_NAME "sunxi_hwspinlock" > + > +#define SPINLOCK_BASE_ID 0 /* there is only one hwspinlock device in > each SoC */ > +#define SPINLOCK_SYSSTATUS_REG 0x > +#define SPINLOCK_STATUS_REG 0x0010 > +#define SPINLOCK_LOCK_REGN 0x0100 > +#define SPINLOCK_NOTTAKEN0 > +#define SPINLOCK_TAKEN 1 > + > +struct sunxi_hwspinlock { > + void __iomem *io_base; > +}; > + > +struct sunxi_hwspinlock_data { > + void __iomem *io_base; > + struct hwspinlock_device *bank; > + struct reset_control *reset; > + struct clk *ahb_clock; > + struct dentry *debugfs; > + int nlocks; > +}; > + > +#ifdef CONFIG_DEBUG_FS > + > +static int hwlocks_supported_show(struct seq_file *seqf, void *unused) > +{ > + struct sunxi_hwspinlock_data *priv = seqf->private; > + > + seq_printf(seqf, "%d\n", priv->nlocks); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(hwlocks_supported); > + > +static int hwlocks_inuse_show(struct seq_file *seqf, void *unused) > +{ > + struct sunxi_hwspinlock_data *priv = seqf->private; > + int inuse; > + > + /* getting the status of only the main 32 spinlocks is supported */ > + inuse = hweight32(readl(priv->io_base + SPINLOCK_STATUS_REG)); So this returns how many of the locks are taken? How is that useful? > + seq_printf(seqf, "%d\n", inuse); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(hwlocks_inuse); > + > +static void sunxi_hwspinlock_debugfs_init(struct sunxi_hwspinlock_data *priv) > +{ > + char name[32]; > + > + scnprintf(name, sizeof(name), "%s", DRIVER_NAME); Why not just pass DRIVER_NAME directly to debugfs_create_dir()? > + > + priv->debugfs = debugfs_create_dir(name, NULL); > + debugfs_create_file("hwlocks_supported", 0444, priv->debugfs, priv, > + _supported_fops); > + debugfs_create_file("hwlocks_inuse", 0444, priv->debugfs, priv, > _inuse_fops); > +} > + > +#else > + > +static void sunxi_hwspinlock_debugfs_init(struct sunxi_hwspinlock_data *priv) > +{ > +} > + > +#endif > + >
Re: [PATCH v4 2/3] fscrypt: Have filesystems handle their d_ops
Hi all, On Thu, Nov 19, 2020 at 06:09:03AM +, Daniel Rosenberg wrote: > This shifts the responsibility of setting up dentry operations from > fscrypt to the individual filesystems, allowing them to have their own > operations while still setting fscrypt's d_revalidate as appropriate. > > Most filesystems can just use generic_set_encrypted_ci_d_ops, unless > they have their own specific dentry operations as well. That operation > will set the minimal d_ops required under the circumstances. > > Since the fscrypt d_ops are set later on, we must set all d_ops there, > since we cannot adjust those later on. This should not result in any > change in behavior. > > Signed-off-by: Daniel Rosenberg > Acked-by: Eric Biggers > --- ... > extern const struct file_operations ext4_dir_operations; > > -#ifdef CONFIG_UNICODE > -extern const struct dentry_operations ext4_dentry_ops; > -#endif > - > /* file.c */ > extern const struct inode_operations ext4_file_inode_operations; > extern const struct file_operations ext4_file_operations; > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 33509266f5a0..12a417ff5648 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct > inode *dir, > struct buffer_head *bh; > > err = ext4_fname_prepare_lookup(dir, dentry, ); > + generic_set_encrypted_ci_d_ops(dentry); One thing might be worth noticing is that currently overlayfs might not work properly when dentry->d_sb->s_encoding is set even only some subdirs are CI-enabled but the others not, see generic_set_encrypted_ci_d_ops(), ovl_mount_dir_noesc => ovl_dentry_weird() For more details, see: https://android-review.googlesource.com/c/device/linaro/hikey/+/1483316/2#message-2e1f6ab0010a3e35e7d8effea73f60341f84ee4d Just found it by chance (and not sure if it's vital for now), and a kind reminder about this. Thanks, Gao Xiang
Re: [PATCH v4 1/3] libfs: Add generic function for setting dentry_ops
Daniel Rosenberg writes: > This adds a function to set dentry operations at lookup time that will > work for both encrypted filenames and casefolded filenames. > > A filesystem that supports both features simultaneously can use this > function during lookup preparations to set up its dentry operations once > fscrypt no longer does that itself. > > Currently the casefolding dentry operation are always set if the > filesystem defines an encoding because the features is toggleable on > empty directories. Unlike in the encryption case, the dentry operations > used come from the parent. Since we don't know what set of functions > we'll eventually need, and cannot change them later, we enable the > casefolding operations if the filesystem supports them at all. > > By splitting out the various cases, we support as few dentry operations > as we can get away with, maximizing compatibility with overlayfs, which > will not function if a filesystem supports certain dentry_operations. > > Signed-off-by: Daniel Rosenberg > Reviewed-by: Eric Biggers Reviewed-by: Gabriel Krisman Bertazi -- Gabriel Krisman Bertazi
Re: [PATCH v4 2/3] fscrypt: Have filesystems handle their d_ops
Daniel Rosenberg writes: > This shifts the responsibility of setting up dentry operations from > fscrypt to the individual filesystems, allowing them to have their own > operations while still setting fscrypt's d_revalidate as appropriate. > > Most filesystems can just use generic_set_encrypted_ci_d_ops, unless > they have their own specific dentry operations as well. That operation > will set the minimal d_ops required under the circumstances. > > Since the fscrypt d_ops are set later on, we must set all d_ops there, > since we cannot adjust those later on. This should not result in any > change in behavior. > > Signed-off-by: Daniel Rosenberg > Acked-by: Eric Biggers > --- > fs/crypto/fname.c | 4 > fs/crypto/fscrypt_private.h | 1 - > fs/crypto/hooks.c | 1 - > fs/ext4/dir.c | 7 --- > fs/ext4/ext4.h | 4 > fs/ext4/namei.c | 1 + > fs/ext4/super.c | 5 - > fs/f2fs/dir.c | 7 --- > fs/f2fs/f2fs.h | 3 --- > fs/f2fs/namei.c | 1 + > fs/f2fs/super.c | 1 - > fs/ubifs/dir.c | 1 + > include/linux/fscrypt.h | 7 +-- > 13 files changed, 8 insertions(+), 35 deletions(-) > > diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c > index 1fbe6c24d705..cb3cfa6329ba 100644 > --- a/fs/crypto/fname.c > +++ b/fs/crypto/fname.c > @@ -570,7 +570,3 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned > int flags) > return valid; > } > EXPORT_SYMBOL_GPL(fscrypt_d_revalidate); > - > -const struct dentry_operations fscrypt_d_ops = { > - .d_revalidate = fscrypt_d_revalidate, > -}; > diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h > index 4f5806a3b73d..df9c48c1fbf7 100644 > --- a/fs/crypto/fscrypt_private.h > +++ b/fs/crypto/fscrypt_private.h > @@ -294,7 +294,6 @@ int fscrypt_fname_encrypt(const struct inode *inode, > const struct qstr *iname, > bool fscrypt_fname_encrypted_size(const union fscrypt_policy *policy, > u32 orig_len, u32 max_len, > u32 *encrypted_len_ret); > -extern const struct dentry_operations fscrypt_d_ops; > > /* hkdf.c */ > > diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c > index 20b0df47fe6a..9006fa983335 100644 > --- a/fs/crypto/hooks.c > +++ b/fs/crypto/hooks.c > @@ -117,7 +117,6 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct > dentry *dentry, > spin_lock(>d_lock); > dentry->d_flags |= DCACHE_NOKEY_NAME; > spin_unlock(>d_lock); > - d_set_d_op(dentry, _d_ops); > } > return err; > } > diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c > index ca50c90adc4c..e757319a4472 100644 > --- a/fs/ext4/dir.c > +++ b/fs/ext4/dir.c > @@ -667,10 +667,3 @@ const struct file_operations ext4_dir_operations = { > .open = ext4_dir_open, > .release= ext4_release_dir, > }; > - > -#ifdef CONFIG_UNICODE > -const struct dentry_operations ext4_dentry_ops = { > - .d_hash = generic_ci_d_hash, > - .d_compare = generic_ci_d_compare, > -}; > -#endif > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index bf9429484462..ad77f01d9e20 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -3380,10 +3380,6 @@ static inline void ext4_unlock_group(struct > super_block *sb, > /* dir.c */ > extern const struct file_operations ext4_dir_operations; > > -#ifdef CONFIG_UNICODE > -extern const struct dentry_operations ext4_dentry_ops; > -#endif > - > /* file.c */ > extern const struct inode_operations ext4_file_inode_operations; > extern const struct file_operations ext4_file_operations; > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 33509266f5a0..12a417ff5648 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -1614,6 +1614,7 @@ static struct buffer_head *ext4_lookup_entry(struct > inode *dir, > struct buffer_head *bh; > > err = ext4_fname_prepare_lookup(dir, dentry, ); > + generic_set_encrypted_ci_d_ops(dentry); > if (err == -ENOENT) > return NULL; > if (err) > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 6633b20224d5..0288bedf46e1 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -4968,11 +4968,6 @@ static int ext4_fill_super(struct super_block *sb, > void *data, int silent) > goto failed_mount4; > } > > -#ifdef CONFIG_UNICODE > - if (sb->s_encoding) > - sb->s_d_op = _dentry_ops; > -#endif This change has the side-effect of removing the capability of the root directory from being case-insensitive. It is not a backward incompatible change because there is no way to make the root directory CI at the moment (it is never empty). But this restriction seems artificial. Is there a real reason to prevent the root inode from being case-insensitive? > - > sb->s_root = d_make_root(root); > if (!sb->s_root) { >
Re: [RFC PATCH 1/3] leds: Add driver for QPNP flash led
On Fri 06 Nov 10:58 CST 2020, N?colas F. R. A. Prado wrote: > Add driver for the QPNP flash LED. It works over SPMI and is part of the > PM8941 PMIC. > > Signed-off-by: Nícolas F. R. A. Prado > --- > drivers/leds/Kconfig |9 + > drivers/leds/Makefile|1 + > drivers/leds/leds-qpnp.c | 1351 ++ > 3 files changed, 1361 insertions(+) > create mode 100644 drivers/leds/leds-qpnp.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 849d3c5f908e..ca5f6e81c064 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -928,6 +928,15 @@ config LEDS_ACER_A500 > This option enables support for the Power Button LED of > Acer Iconia Tab A500. > > +config LEDS_QPNP > + tristate "Support for QPNP LEDs" > + depends on SPMI > + help > + This driver supports the flash/torch led of Qualcomm PNP PMIC. > + > + To compile this driver as a module, choose M here: the module will > + be called leds-qpnp. > + Downstream they seem to have a single "led driver" dealing with all the LED related interfaces in the PMIC. We have WLED upstream already and I've been poking at an "LPG" driver. So as you look into Jacek's request please make this a "qcom spmi flash driver", instead of a "qpnp leds" driver. Regards, Bjorn
Re: [PATCH] clk: qcom: camcc-sc7180: Use runtime PM ops instead of clk ones
On Sat 14 Nov 11:44 CST 2020, Stephen Boyd wrote: > Let's call pm_runtime_get() here instead of calling the PM clk APIs > directly. This avoids a compilation problem on CONFIG_PM=n where the > pm_clk_runtime_{resume,suspend}() functions don't exist and covers the > intent, i.e. enable the clks for this device so we can program PLL > settings. > > Reported-by: Randy Dunlap > Reported-by: Geert Uytterhoeven > Cc: Nathan Chancellor > Cc: Stephen Rothwell > Cc: Taniya Das > Cc: "Rafael J. Wysocki" > Fixes: 15d09e830bbc ("clk: qcom: camcc: Add camera clock controller driver > for SC7180") > Signed-off-by: Stephen Boyd Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/clk/qcom/camcc-sc7180.c | 12 > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/qcom/camcc-sc7180.c b/drivers/clk/qcom/camcc-sc7180.c > index f51bf5b6decc..dbac5651ab85 100644 > --- a/drivers/clk/qcom/camcc-sc7180.c > +++ b/drivers/clk/qcom/camcc-sc7180.c > @@ -1669,16 +1669,14 @@ static int cam_cc_sc7180_probe(struct platform_device > *pdev) > goto disable_pm_runtime; > } > > - ret = pm_clk_runtime_resume(>dev); > - if (ret < 0) { > - dev_err(>dev, "pm runtime resume failed\n"); > + ret = pm_runtime_get(>dev); > + if (ret) > goto destroy_pm_clk; > - } > > regmap = qcom_cc_map(pdev, _cc_sc7180_desc); > if (IS_ERR(regmap)) { > ret = PTR_ERR(regmap); > - pm_clk_runtime_suspend(>dev); > + pm_runtime_put(>dev); > goto destroy_pm_clk; > } > > @@ -1688,9 +1686,7 @@ static int cam_cc_sc7180_probe(struct platform_device > *pdev) > clk_fabia_pll_configure(_cc_pll3, regmap, _cc_pll3_config); > > ret = qcom_cc_really_probe(pdev, _cc_sc7180_desc, regmap); > - > - pm_clk_runtime_suspend(>dev); > - > + pm_runtime_put(>dev); > if (ret < 0) { > dev_err(>dev, "Failed to register CAM CC clocks\n"); > goto destroy_pm_clk; > -- > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/ >
Re: [PATCH v5 5/5] PCI: qcom: Add support for configuring BDF to SID mapping for SM8250
On Tue 27 Oct 12:00 CDT 2020, Manivannan Sadhasivam wrote: > For SM8250, we need to write the BDF to SID mapping in PCIe controller > register space for proper working. This is accomplished by extracting > the BDF and SID values from "iommu-map" property in DT and writing those > in the register address calculated from the hash value of BDF. In case > of collisions, the index of the next entry will also be written. > > For the sake of it, let's introduce a "config_sid" callback and do it > conditionally for SM8250. > > Signed-off-by: Manivannan Sadhasivam Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > > Rob: I've dropped your review tag as this patch has gone through some > change (mostly cleanups though) > > drivers/pci/controller/dwc/Kconfig | 1 + > drivers/pci/controller/dwc/pcie-qcom.c | 81 ++ > 2 files changed, 82 insertions(+) > > diff --git a/drivers/pci/controller/dwc/Kconfig > b/drivers/pci/controller/dwc/Kconfig > index bc049865f8e0..875ebc6e8884 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -169,6 +169,7 @@ config PCIE_QCOM > depends on OF && (ARCH_QCOM || COMPILE_TEST) > depends on PCI_MSI_IRQ_DOMAIN > select PCIE_DW_HOST > + select CRC8 > help > Say Y here to enable PCIe controller support on Qualcomm SoCs. The > PCIe controller uses the DesignWare core plus Qualcomm-specific > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c > b/drivers/pci/controller/dwc/pcie-qcom.c > index 0b180a19b0ea..2148fcf74294 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -9,6 +9,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -57,6 +58,7 @@ > #define PCIE20_PARF_SID_OFFSET 0x234 > #define PCIE20_PARF_BDF_TRANSLATE_CFG0x24C > #define PCIE20_PARF_DEVICE_TYPE 0x1000 > +#define PCIE20_PARF_BDF_TO_SID_TABLE_N 0x2000 > > #define PCIE20_ELBI_SYS_CTRL 0x04 > #define PCIE20_ELBI_SYS_CTRL_LT_ENABLE BIT(0) > @@ -97,6 +99,9 @@ > > #define QCOM_PCIE_2_1_0_MAX_SUPPLY 3 > #define QCOM_PCIE_2_1_0_MAX_CLOCKS 5 > + > +#define QCOM_PCIE_CRC8_POLYNOMIAL (BIT(2) | BIT(1) | BIT(0)) > + > struct qcom_pcie_resources_2_1_0 { > struct clk_bulk_data clks[QCOM_PCIE_2_1_0_MAX_CLOCKS]; > struct reset_control *pci_reset; > @@ -179,6 +184,7 @@ struct qcom_pcie_ops { > void (*deinit)(struct qcom_pcie *pcie); > void (*post_deinit)(struct qcom_pcie *pcie); > void (*ltssm_enable)(struct qcom_pcie *pcie); > + int (*config_sid)(struct qcom_pcie *pcie); > }; > > struct qcom_pcie { > @@ -1261,6 +1267,74 @@ static int qcom_pcie_link_up(struct dw_pcie *pci) > return !!(val & PCI_EXP_LNKSTA_DLLLA); > } > > +static int qcom_pcie_config_sid_sm8250(struct qcom_pcie *pcie) > +{ > + /* iommu map structure */ > + struct { > + u32 bdf; > + u32 phandle; > + u32 smmu_sid; > + u32 smmu_sid_len; > + } *map; > + void __iomem *bdf_to_sid_base = pcie->parf + > PCIE20_PARF_BDF_TO_SID_TABLE_N; > + struct device *dev = pcie->pci->dev; > + u8 qcom_pcie_crc8_table[CRC8_TABLE_SIZE]; > + int i, nr_map, size = 0; > + u32 smmu_sid_base; > + > + of_get_property(dev->of_node, "iommu-map", ); > + if (!size) > + return 0; > + > + map = kzalloc(size, GFP_KERNEL); > + if (!map) > + return -ENOMEM; > + > + of_property_read_u32_array(dev->of_node, > + "iommu-map", (u32 *)map, size / sizeof(u32)); > + > + nr_map = size / (sizeof(*map)); > + > + crc8_populate_msb(qcom_pcie_crc8_table, QCOM_PCIE_CRC8_POLYNOMIAL); > + > + /* Registers need to be zero out first */ > + memset_io(bdf_to_sid_base, 0, CRC8_TABLE_SIZE * sizeof(u32)); > + > + /* Look for an available entry to hold the mapping */ > + for (i = 0; i < nr_map; i++) { > + u16 bdf_be = cpu_to_be16(map[i].bdf); > + u32 val; > + u8 hash; > + > + hash = crc8(qcom_pcie_crc8_table, (u8 *)_be, sizeof(bdf_be), > + 0); > + > + val = readl(bdf_to_sid_base + hash * sizeof(u32)); > + > + /* If the register is already populated, look for next > available entry */ > + while (val) { > + u8 current_hash = hash++; > + u8 next_mask = 0xff; > + > + /* If NEXT field is NULL then update it with next hash > */ > + if (!(val & next_mask)) { > + val |= (u32)hash; > + writel(val, bdf_to_sid_base + current_hash * > sizeof(u32)); > + } > + > + val = readl(bdf_to_sid_base + hash * sizeof(u32)); > + } > + > +
Re: [PATCH v5 4/5] PCI: qcom: Add SM8250 SoC support
On Tue 27 Oct 12:00 CDT 2020, Manivannan Sadhasivam wrote: > The PCIe IP (rev 1.9.0) on SM8250 SoC is similar to the one used on > SDM845. Hence the support is added reusing the members of ops_2_7_0. > The key difference between ops_2_7_0 and ops_1_9_0 is the config_sid > callback, which will be added in successive commit. > > Signed-off-by: Manivannan Sadhasivam > Reviewed-by: Rob Herring Reviewed-by: Bjorn Andersson Regards, Bjorn > --- > drivers/pci/controller/dwc/pcie-qcom.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c > b/drivers/pci/controller/dwc/pcie-qcom.c > index b4761640ffd9..0b180a19b0ea 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -1361,6 +1361,16 @@ static const struct qcom_pcie_ops ops_2_7_0 = { > .post_deinit = qcom_pcie_post_deinit_2_7_0, > }; > > +/* Qcom IP rev.: 1.9.0 */ > +static const struct qcom_pcie_ops ops_1_9_0 = { > + .get_resources = qcom_pcie_get_resources_2_7_0, > + .init = qcom_pcie_init_2_7_0, > + .deinit = qcom_pcie_deinit_2_7_0, > + .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable, > + .post_init = qcom_pcie_post_init_2_7_0, > + .post_deinit = qcom_pcie_post_deinit_2_7_0, > +}; > + > static const struct dw_pcie_ops dw_pcie_ops = { > .link_up = qcom_pcie_link_up, > }; > @@ -1474,6 +1484,7 @@ static const struct of_device_id qcom_pcie_match[] = { > { .compatible = "qcom,pcie-ipq4019", .data = _2_4_0 }, > { .compatible = "qcom,pcie-qcs404", .data = _2_4_0 }, > { .compatible = "qcom,pcie-sdm845", .data = _2_7_0 }, > + { .compatible = "qcom,pcie-sm8250", .data = _1_9_0 }, > { } > }; > > -- > 2.17.1 >
Re: [PATCH v5 3/5] dt-bindings: pci: qcom: Document PCIe bindings for SM8250 SoC
On Tue 27 Oct 12:00 CDT 2020, Manivannan Sadhasivam wrote: > Document the PCIe DT bindings for SM8250 SoC. The PCIe IP is similar to > the one used on SDM845, hence just add the compatible along with the > optional "atu" register region. > > Signed-off-by: Manivannan Sadhasivam > Acked-by: Rob Herring Reviewed-by: Bjorn Andersson > --- > Documentation/devicetree/bindings/pci/qcom,pcie.txt | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > index 02bc81bb8b2d..3b55310390a0 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > @@ -13,6 +13,7 @@ > - "qcom,pcie-ipq8074" for ipq8074 > - "qcom,pcie-qcs404" for qcs404 > - "qcom,pcie-sdm845" for sdm845 > + - "qcom,pcie-sm8250" for sm8250 > > - reg: > Usage: required > @@ -27,6 +28,7 @@ > - "dbi"DesignWare PCIe registers > - "elbi" External local bus interface registers > - "config" PCIe configuration space > + - "atu"ATU address space (optional) > > - device_type: > Usage: required > @@ -131,7 +133,7 @@ > - "slave_bus" AXI Slave clock > > -clock-names: > - Usage: required for sdm845 > + Usage: required for sdm845 and sm8250 > Value type: > Definition: Should contain the following entries > - "aux" Auxiliary clock > @@ -206,7 +208,7 @@ > - "ahb" AHB reset > > - reset-names: > - Usage: required for sdm845 > + Usage: required for sdm845 and sm8250 > Value type: > Definition: Should contain the following entries > - "pci" PCIe core reset > -- > 2.17.1 >
Re: [PATCH RFC 01/10] vmalloc: Add basic perm alloc implementation
On Fri, Nov 20, 2020 at 12:30 PM Rick Edgecombe wrote: > > In order to allow for future arch specific optimizations for vmalloc > permissions, first add an implementation of a new interface that will > work cross arch by using the existing set_memory_() functions. > > When allocating some memory that will be RO, for example it should be used > like: > > /* Reserve va */ > struct perm_allocation *alloc = perm_alloc(vstart, vend, page_cnt, PERM_R); I'm sure I could reverse-engineer this from the code, but: Where do vstart and vend come from? Does perm_alloc() allocate memory or just virtual addresses? Is the caller expected to call vmalloc()? How does one free this thing? > unsigned long ro = (unsigned long)perm_alloc_address(alloc); > > /* Write to writable address */ > strcpy((char *)perm_writable_addr(alloc, ro), "Some data to be RO"); > /* Signal that writing is done and mapping should be live */ > perm_writable_finish(alloc); > /* Print from RO address */ > printk("Read only data is: %s\n", (char *)ro); >
Re: [PATCH] i2c: qup: Fix error return code in qup_i2c_bam_schedule_desc()
On Mon 16 Nov 08:10 CST 2020, Zhihao Cheng wrote: > Fix to return the error code from qup_i2c_change_state() > instaed of 0 in qup_i2c_bam_schedule_desc(). > > Fixes: fbf9921f8b35d9b2 ("i2c: qup: Fix error handling") > Reported-by: Hulk Robot > Signed-off-by: Zhihao Cheng > --- > drivers/i2c/busses/i2c-qup.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c > index fbc04b60cfd1..5a47915869ae 100644 > --- a/drivers/i2c/busses/i2c-qup.c > +++ b/drivers/i2c/busses/i2c-qup.c > @@ -801,7 +801,8 @@ static int qup_i2c_bam_schedule_desc(struct qup_i2c_dev > *qup) > if (ret || qup->bus_err || qup->qup_err) { > reinit_completion(>xfer); > > - if (qup_i2c_change_state(qup, QUP_RUN_STATE)) { > + ret = qup_i2c_change_state(qup, QUP_RUN_STATE); In the case that we entered this block because ret was -ETIMEDOUT then this will overwrite this and the function will return -EIO. But in the other paths out of this block ret is being overwritten anyways, so I think it's fine. Reviewed-by: Bjorn Andersson Regards, Bjorn > + if (ret) { > dev_err(qup->dev, "change to run state timed out"); > goto desc_err; > } > -- > 2.25.4 >
Re: [PATCH v7 3/7] kernel: Implement selective syscall userspace redirection
On Fri, Nov 20, 2020 at 4:18 PM Kees Cook wrote: > > On Thu, Nov 19, 2020 at 12:43:05PM -0500, Gabriel Krisman Bertazi wrote: > > The existing interface could be extended with a flags field as part of > > the opcode passed in argument 2, which is currently reserved, and then > > return a FD, just like seccomp(2) does. So it is not like the current > > patches couldn't be extended in the future if needed, unless I'm > > mistaken. > > Yes, I'd prefer this series go in as-is, and if there is a need for > extending the API, arg2 can have more values added. I agree. > > -- > Kees Cook
ERROR: "dfltcc_inflate" undefined!
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: a349e4c659609fd20e4beea89e5c4a4038e33a95 commit: 12619610006371bfc30159937d4546e731d7b297 lib/zlib: add s390 hardware support for kernel zlib_inflate date: 10 months ago config: s390-randconfig-r003-20201122 (attached as .config) compiler: s390-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=12619610006371bfc30159937d4546e731d7b297 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout 12619610006371bfc30159937d4546e731d7b297 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> ERROR: "dfltcc_inflate" [lib/zlib_inflate/zlib_inflate.ko] undefined! >> ERROR: "dfltcc_can_inflate" [lib/zlib_inflate/zlib_inflate.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH 5/5] i2c: geni: sdm845: dont perform DMA for OnePlus 6 devices
On Thu 12 Nov 10:22 CST 2020, Caleb Connolly wrote: > The OnePlus 6/T has the same issue as the Yoga c630 causing a crash when DMA > is used for i2c, so disable it. > > https://patchwork.kernel.org/patch/11133827/ > > Signed-off-by: Caleb Connolly > --- > drivers/i2c/busses/i2c-qcom-geni.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c > b/drivers/i2c/busses/i2c-qcom-geni.c > index 8b4c35f47a70..9acdcfe73be2 100644 > --- a/drivers/i2c/busses/i2c-qcom-geni.c > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > @@ -357,7 +357,8 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, > struct i2c_msg *msg, > struct geni_se *se = >se; > size_t len = msg->len; > > - if (!of_machine_is_compatible("lenovo,yoga-c630")) > + if (!of_machine_is_compatible("lenovo,yoga-c630") && > + !of_machine_is_compatible("oneplus,oneplus6")) This hack seems to have been working around two separate issues. First with iommu active the GENI wrappers needs to have their stream mapping configured. Secondly there was a bug in the transaction setup that was recently fixed by Doug Anderson. So can you please give the following patch a go? I've yet to test it on the Lenovo machine, but I think it allows us to remove the quirk. https://lore.kernel.org/lkml/20201122034149.626045-1-bjorn.anders...@linaro.org/T/#u Regards, Bjorn > dma_buf = i2c_get_dma_safe_msg_buf(msg, 32); > > if (dma_buf) > @@ -399,7 +400,8 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, > struct i2c_msg *msg, > struct geni_se *se = >se; > size_t len = msg->len; > > - if (!of_machine_is_compatible("lenovo,yoga-c630")) > + if (!of_machine_is_compatible("lenovo,yoga-c630") && > + !of_machine_is_compatible("oneplus,oneplus6")) > dma_buf = i2c_get_dma_safe_msg_buf(msg, 32); > > if (dma_buf) > -- > 2.29.2 > >
[PATCH v2] arm64: dts: sdm845: Add iommus property to qup
From: Stephen Boyd The SMMU that sits in front of the QUP needs to be programmed properly so that the i2c geni driver can allocate DMA descriptors. Failure to do this leads to faults when using devices such as an i2c touchscreen where the transaction is larger than 32 bytes and we use a DMA buffer. arm-smmu 1500.iommu: Unexpected global fault, this could be serious arm-smmu 1500.iommu: GFSR 0x0002, GFSYNR0 0x0002, GFSYNR1 0x06c0, GFSYNR2 0x Add the right SID and mask so this works. Signed-off-by: Stephen Boyd [bjorn: Define for second QUP as well, be more specific in sdm845.dtsi] Signed-off-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 2 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 ++ 2 files changed, 4 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi index 39f23cdcbd02..216a74f0057c 100644 --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi @@ -653,10 +653,12 @@ _pwrkey { _id_0 { status = "okay"; + iommus = <_smmu 0x0 0x3>; }; _id_1 { status = "okay"; + iommus = <_smmu 0x6c0 0x3>; }; _2 { diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 6465a6653ad9..d6b7b1bfa202 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -1120,6 +1120,7 @@ qupv3_id_0: geniqup@8c { clock-names = "m-ahb", "s-ahb"; clocks = < GCC_QUPV3_WRAP_0_M_AHB_CLK>, < GCC_QUPV3_WRAP_0_S_AHB_CLK>; + iommus = <_smmu 0x3 0x0>; #address-cells = <2>; #size-cells = <2>; ranges; @@ -1460,6 +1461,7 @@ qupv3_id_1: geniqup@ac { clock-names = "m-ahb", "s-ahb"; clocks = < GCC_QUPV3_WRAP_1_M_AHB_CLK>, < GCC_QUPV3_WRAP_1_S_AHB_CLK>; + iommus = <_smmu 0x6c3 0x0>; #address-cells = <2>; #size-cells = <2>; ranges; -- 2.28.0
Re: INFO: task hung in __io_uring_files_cancel
On 22/11/2020 03:04, Hillf Danton wrote: > On Sat, 21 Nov 2020 14:35:15 -0800 >> syzbot found the following issue on: >> >> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1040172650 >> final oops: https://syzkaller.appspot.com/x/report.txt?x=1240172650 >> console output: https://syzkaller.appspot.com/x/log.txt?x=1440172650 >> >> IMPORTANT: if you fix the issue, please add the following tag to the commit: >> Reported-by: syzbot+c0d52d0b3c0c3ffb9...@syzkaller.appspotmail.com >> Fixes: 4d004099a668 ("lockdep: Fix lockdep recursion") >> >> INFO: task syz-executor.0:9557 blocked for more than 143 seconds. >> Not tainted 5.10.0-rc4-next-20201117-syzkaller #0 >> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. >> task:syz-executor.0 state:D stack:28584 pid: 9557 ppid: 8485 >> flags:0x4002 >> Call Trace: >> context_switch kernel/sched/core.c:4269 [inline] >> __schedule+0x890/0x2030 kernel/sched/core.c:5019 >> schedule+0xcf/0x270 kernel/sched/core.c:5098 >> io_uring_cancel_files fs/io_uring.c:8720 [inline] >> io_uring_cancel_task_requests fs/io_uring.c:8772 [inline] >> __io_uring_files_cancel+0xc4d/0x14b0 fs/io_uring.c:8868 >> io_uring_files_cancel include/linux/io_uring.h:51 [inline] >> exit_files+0xe4/0x170 fs/file.c:456 >> do_exit+0xb61/0x29f0 kernel/exit.c:818 >> do_group_exit+0x125/0x310 kernel/exit.c:920 >> get_signal+0x3ea/0x1f70 kernel/signal.c:2750 >> arch_do_signal_or_restart+0x2a6/0x1ea0 arch/x86/kernel/signal.c:811 >> handle_signal_work kernel/entry/common.c:145 [inline] >> exit_to_user_mode_loop kernel/entry/common.c:169 [inline] >> exit_to_user_mode_prepare+0x124/0x200 kernel/entry/common.c:199 >> syscall_exit_to_user_mode+0x38/0x260 kernel/entry/common.c:274 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> RIP: 0033:0x45deb9 >> Code: Unable to access opcode bytes at RIP 0x45de8f. >> RSP: 002b:7fa68397ccf8 EFLAGS: 0246 ORIG_RAX: 00ca >> RAX: fe00 RBX: 0118bf28 RCX: 0045deb9 >> RDX: RSI: 0080 RDI: 0118bf28 >> RBP: 0118bf20 R08: R09: >> R10: R11: 0246 R12: 0118bf2c >> R13: 7fff50acc9af R14: 7fa68397d9c0 R15: 0118bf2c >> ... > > Fix 311daef8013a ("io_uring: replace inflight_wait with tctx->wait") > by cutting the pre-condition to wakeup because waitqueue_active() > speaks the language in the East End while atomic_read() may speak > the language in Paris. Your description doesn't help, why do you think this is the problem? ->in_idle is always set when io_uring_cancel_files() sleeps on it, and ->inflight_lock should guarantee ordering. > > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -6082,8 +6082,7 @@ static void io_req_drop_files(struct io_ > > spin_lock_irqsave(>inflight_lock, flags); > list_del(>inflight_entry); > - if (atomic_read(>in_idle)) > - wake_up(>wait); > + wake_up(>wait); > spin_unlock_irqrestore(>inflight_lock, flags); > req->flags &= ~REQ_F_INFLIGHT; > put_files_struct(req->work.identity->files); > -- Pavel Begunkov
[PATCH] dt-bindings: phy: Convert Broadcom SATA PHY to YAML
Update the Broadcom SATA PHY Device Tree binding to a YAML format. Suggested-by: Vinod Koul Signed-off-by: Florian Fainelli --- This is based on phy/next and it depends on the following commit: https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git/commit/?h=next=6d3b3f88423e4edc0fad5853c10558b42e1a91dd it would make sense to have Vinod apply this change. .../bindings/phy/brcm,sata-phy.yaml | 148 ++ .../devicetree/bindings/phy/brcm-sata-phy.txt | 61 2 files changed, 148 insertions(+), 61 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/brcm,sata-phy.yaml delete mode 100644 Documentation/devicetree/bindings/phy/brcm-sata-phy.txt diff --git a/Documentation/devicetree/bindings/phy/brcm,sata-phy.yaml b/Documentation/devicetree/bindings/phy/brcm,sata-phy.yaml new file mode 100644 index ..979b7419dc69 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/brcm,sata-phy.yaml @@ -0,0 +1,148 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/phy/brcm,sata-phy.yaml#; +$schema: "http://devicetree.org/meta-schemas/core.yaml#; + +title: Broadcom SATA3 PHY + +maintainers: + - Florian Fainelli + +properties: + $nodename: +pattern: "^sata[-|_]phy(@.*)?$" + + compatible: +oneOf: + - items: +- enum: + - brcm,bcm7216-sata-phy + - brcm,bcm7425-sata-phy + - brcm,bcm7445-sata-phy + - brcm,bcm63138-sata-phy +- const: brcm,phy-sata3 + - items: +- const: brcm,iproc-nsp-sata-phy + - items: +- const: brcm,iproc-ns2-sata-phy + - items: +- const: brcm,iproc-sr-sata-phy + + reg: +minItems: 1 +maxItems: 2 + + reg-names: +minItems: 1 +maxItems: 2 +items: + - const: phy + - const: phy-ctrl + + "#address-cells": +const: 1 + + "#size-cells": +const: 0 + +patternProperties: + "^sata-phy@[0-9]+$": + type: object + description: | + Each port's PHY should be represented as a sub-node. + + properties: + reg: + description: The SATA PHY port number + maxItems: 1 + + "#phy-cells": + const: 0 + + "brcm,enable-ssc": + $ref: /schemas/types.yaml#/definitions/flag + description: | + Use spread spectrum clocking (SSC) on this port + This property is not applicable for "brcm,iproc-ns2-sata-phy", + "brcm,iproc-nsp-sata-phy" and "brcm,iproc-sr-sata-phy". + + "brcm,rxaeq-mode": + $ref: /schemas/types.yaml#/definitions/string + description: +String that indicates the desired RX equalizer mode. + enum: +- off +- auto +- manual + + "brcm,rxaeq-value": + $ref: /schemas/types.yaml#/definitions/uint32 + description: | +When 'brcm,rxaeq-mode' is set to "manual", provides the RX +equalizer value that should be used. + minimum: 0 + maximum: 63 + + "brcm,tx-amplitude-millivolt": + description: | +Transmit amplitude voltage in millivolt. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [400, 500, 600, 800] + + required: + - reg + - "#phy-cells" + + additionalProperties: false + +if: + properties: +compatible: + items: +const: brcm,iproc-ns2-sata-phy +then: + properties: +reg: + maxItems: 2 +reg-names: + items: +- const: "phy" +- const: "phy-ctrl" +else: + properties: +reg: + maxItems: 1 +reg-names: + maxItems: 1 + items: +- const: "phy" + +required: + - compatible + - "#address-cells" + - "#size-cells" + - reg + - reg-names + +additionalProperties: false + +examples: + - | +sata-phy@f0458100 { +compatible = "brcm,bcm7445-sata-phy", "brcm,phy-sata3"; +reg = <0xf0458100 0x1e00>; +reg-names = "phy"; +#address-cells = <1>; +#size-cells = <0>; + +sata-phy@0 { +reg = <0>; +#phy-cells = <0>; +}; + +sata-phy@1 { +reg = <1>; +#phy-cells = <0>; +}; +}; diff --git a/Documentation/devicetree/bindings/phy/brcm-sata-phy.txt b/Documentation/devicetree/bindings/phy/brcm-sata-phy.txt deleted file mode 100644 index e5abbace93a3.. --- a/Documentation/devicetree/bindings/phy/brcm-sata-phy.txt +++ /dev/null @@ -1,61 +0,0 @@ -* Broadcom SATA3 PHY - -Required properties: -- compatible: should be one or more of - "brcm,bcm7216-sata-phy" - "brcm,bcm7425-sata-phy" - "brcm,bcm7445-sata-phy" - "brcm,iproc-ns2-sata-phy" - "brcm,iproc-nsp-sata-phy" - "brcm,phy-sata3" - "brcm,iproc-sr-sata-phy" - "brcm,bcm63138-sata-phy" -- address-cells: should be 1 -- size-cells: should be 0 -- reg: register ranges
Re: [PATCH V6 1/3] soc: qcom: geni: Remove "iova" check
On Fri 30 Oct 09:59 CDT 2020, Roja Rani Yarubandi wrote: > Remove "iova" check from geni_se_tx_dma_unprep and geni_se_rx_dma_unprep > functions as checking with dma_mapping_error() is enough. > Applied this patch towards v5.11. Thank you, Bjorn > Signed-off-by: Roja Rani Yarubandi > --- > Changes in V5: > - This is newly added patch in V5. As per Stephen's comments separted >this patch from shutdown callback patch. > > Changes in V6: > - Fixed nit-picks in commit text. > > drivers/soc/qcom/qcom-geni-se.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c > index d0e4f520cff8..0216b38c1e9a 100644 > --- a/drivers/soc/qcom/qcom-geni-se.c > +++ b/drivers/soc/qcom/qcom-geni-se.c > @@ -705,7 +705,7 @@ void geni_se_tx_dma_unprep(struct geni_se *se, dma_addr_t > iova, size_t len) > { > struct geni_wrapper *wrapper = se->wrapper; > > - if (iova && !dma_mapping_error(wrapper->dev, iova)) > + if (!dma_mapping_error(wrapper->dev, iova)) > dma_unmap_single(wrapper->dev, iova, len, DMA_TO_DEVICE); > } > EXPORT_SYMBOL(geni_se_tx_dma_unprep); > @@ -722,7 +722,7 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t > iova, size_t len) > { > struct geni_wrapper *wrapper = se->wrapper; > > - if (iova && !dma_mapping_error(wrapper->dev, iova)) > + if (!dma_mapping_error(wrapper->dev, iova)) > dma_unmap_single(wrapper->dev, iova, len, DMA_FROM_DEVICE); > } > EXPORT_SYMBOL(geni_se_rx_dma_unprep); > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
Re: [PATCH 2/2] soc: qcom: aoss: Add debugfs send entry
On Mon 02 Nov 21:19 CST 2020, Chris Lew wrote: > It can be useful to control the different power states of various > parts of hardware for device testing. Add a debugfs node to send > messages through qmp to aoss for debugging and testing purposes. > > Signed-off-by: Chris Lew > --- > drivers/soc/qcom/qcom_aoss.c | 36 > 1 file changed, 36 insertions(+) > > diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c > index 8f052db1880a..2fd755d2a92d 100644 > --- a/drivers/soc/qcom/qcom_aoss.c > +++ b/drivers/soc/qcom/qcom_aoss.c > @@ -4,6 +4,7 @@ > */ > #include > #include > +#include > #include > #include > #include > @@ -85,6 +86,8 @@ struct qmp { > struct clk_hw qdss_clk; > struct genpd_onecell_data pd_data; > struct qmp_cooling_device *cooling_devs; > + > + struct dentry *debugfs_fp; > }; > > struct qmp_pd { > @@ -541,6 +544,34 @@ struct qmp_device *qmp_get(struct device_node *np) > } > EXPORT_SYMBOL_GPL(qmp_get); > > +static ssize_t aoss_dbg_write(struct file *file, const char __user *userstr, > + size_t len, loff_t *pos) > +{ > + struct qmp *qmp = file->private_data; > + char buf[QMP_MSG_LEN] = {}; > + int ret; > + > + if (!len || len >= QMP_MSG_LEN) > + return len; > + > + ret = copy_from_user(buf, userstr, len); > + if (ret) { > + dev_err(qmp->dev, "copy from user failed, ret:%d\n", ret); > + return len; > + } > + > + ret = qmp_send(qmp, buf, QMP_MSG_LEN); > + if (ret) > + dev_err(qmp->dev, "debug send failed, ret:%d\n", ret); You should propagate this error to the caller, i.e. return ret ? ret : len; And with that the error print doesn't really add any value, so please drop it. > + > + return len; > +} > + This will result in a compile warning when compiled without CONFIG_DEBUG_FS, so either mark it __maybe_unused or wrap the whole hunk in a #if IS_ENABLED(CONFIG_DEBUG_FS). PS. Feel free to resubmit this change on its own, as it can be merged independently from patch 1. Regards, Bjorn > +static const struct file_operations aoss_dbg_fops = { > + .open = simple_open, > + .write = aoss_dbg_write, > +}; > + > static int qmp_probe(struct platform_device *pdev) > { > struct resource *res; > @@ -595,6 +626,9 @@ static int qmp_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, qmp); > > + qmp->debugfs_fp = debugfs_create_file("aoss_send_message", 0220, NULL, > + qmp, _dbg_fops); > + > return 0; > > err_remove_qdss_clk: > @@ -611,6 +645,8 @@ static int qmp_remove(struct platform_device *pdev) > { > struct qmp *qmp = platform_get_drvdata(pdev); > > + debugfs_remove(qmp->debugfs_fp); > + > qmp_qdss_clk_remove(qmp); > qmp_pd_remove(qmp); > qmp_cooling_devices_remove(qmp); > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
Re: [RFC] MAINTAINERS tag for cleanup robot
On Sat, Nov 21, 2020 at 08:50:58AM -0800, t...@redhat.com wrote: > The fixer review is > https://reviews.llvm.org/D91789 > > A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives. > The false positives are caused by macros passed to other macros and by > some macro expansions that did not have an extra semicolon. > > This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt > warnings in linux-next. Are any of them not false-positives? It's all very well to enable stricter warnings, but if they don't fix any bugs, they're just churn.
[PATCH v2] lib/lz4: explicitly support in-place decompression
LZ4 final literal copy could be overlapped when doing in-place decompression, so it's unsafe to just use memcpy() on an optimized memcpy approach but memmove() instead. Upstream LZ4 has updated this years ago [1] (and the impact is non-sensible [2] plus only a few bytes remain), this commit just synchronizes LZ4 upstream code to the kernel side as well. It can be observed as EROFS in-place decompression failure on specific files when X86_FEATURE_ERMS is unsupported, memcpy() optimization of commit 59daa706fbec ("x86, mem: Optimize memcpy by avoiding memory false dependece") will be enabled then. Currently most modern x86-CPUs support ERMS, these CPUs just use "rep movsb" approach so no problem at all. However, it can still be verified with forcely disabling ERMS feature... arch/x86/lib/memcpy_64.S: ALTERNATIVE_2 "jmp memcpy_orig", "", X86_FEATURE_REP_GOOD, \ - "jmp memcpy_erms", X86_FEATURE_ERMS + "jmp memcpy_orig", X86_FEATURE_ERMS We didn't observe any strange on arm64/arm/x86 platform before since most memcpy() would behave in an increasing address order ("copy upwards" [3]) and it's the correct order of in-place decompression but it really needs an update to memmove() for sure considering it's an undefined behavior according to the standard and some unique optimization already exists in the kernel. [1] https://github.com/lz4/lz4/commit/33cb8518ac385835cc17be9a770b27b40cd0e15b [2] https://github.com/lz4/lz4/pull/717#issuecomment-497818921 [3] https://sourceware.org/bugzilla/show_bug.cgi?id=12518 Cc: Yann Collet Cc: Nick Terrell Cc: Miao Xie Cc: Chao Yu Cc: Li Guifu Cc: Guo Xuenan Signed-off-by: Gao Xiang --- changes since v1: - refine commit message; - Cc more people. Hi Andrew, Could you kindly consider picking this patch up, although the impact is EROFS but it touchs in-kernel lz4 library anyway... Thanks, Gao Xiang lib/lz4/lz4_decompress.c | 6 +- lib/lz4/lz4defs.h| 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c index 00cb0d0b73e1..8a7724a6ce2f 100644 --- a/lib/lz4/lz4_decompress.c +++ b/lib/lz4/lz4_decompress.c @@ -263,7 +263,11 @@ static FORCE_INLINE int LZ4_decompress_generic( } } - LZ4_memcpy(op, ip, length); + /* +* supports overlapping memory regions; only matters +* for in-place decompression scenarios +*/ + LZ4_memmove(op, ip, length); ip += length; op += length; diff --git a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h index c91dd96ef629..673bd206aa98 100644 --- a/lib/lz4/lz4defs.h +++ b/lib/lz4/lz4defs.h @@ -146,6 +146,7 @@ static FORCE_INLINE void LZ4_writeLE16(void *memPtr, U16 value) * environments. This is needed when decompressing the Linux Kernel, for example. */ #define LZ4_memcpy(dst, src, size) __builtin_memcpy(dst, src, size) +#define LZ4_memmove(dst, src, size) __builtin_memmove(dst, src, size) static FORCE_INLINE void LZ4_copy8(void *dst, const void *src) { -- 2.18.4
Re: [PATCH 072/141] can: peak_usb: Fix fall-through warnings for Clang
On Sun, 2020-11-22 at 00:04 +0100, Marc Kleine-Budde wrote: > On 11/21/20 8:50 PM, Joe Perches wrote: > > > What about moving the default to the end if the case, which is more > > > common anyways: > > > > > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c > > > b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > > [] > > > @@ -295,16 +295,16 @@ static void peak_usb_write_bulk_callback(struct urb > > > *urb) > > > netif_trans_update(netdev); > > > break; > > > > > > > > > - default: > > > - if (net_ratelimit()) > > > - netdev_err(netdev, "Tx urb aborted (%d)\n", > > > - urb->status); > > > case -EPROTO: > > > case -ENOENT: > > > case -ECONNRESET: > > > case -ESHUTDOWN: > > > - > > > break; > > > + > > > + default: > > > + if (net_ratelimit()) > > > + netdev_err(netdev, "Tx urb aborted (%d)\n", > > > + urb->status); > > > > That's fine and is more generally used style but this > > default: case should IMO also end with a break; > > > > + break; > > I don't mind. > > process/coding-style.rst is not totally clear about the break after the > default, > if this is the lase one the switch statement. deprecated.rst has: All switch/case blocks must end in one of: * break; * fallthrough; * continue; * goto ; * return [expression]; I suppose that could be moved into coding-style along with maybe a change to "all switch/case/default blocks"
Re: [PATCH v4] checkpatch: add fix option for LOGICAL_CONTINUATIONS
On Sun, 2020-11-22 at 02:34 +0530, Aditya Srivastava wrote: > Currently, checkpatch warns if logical continuations are placed at the > start of a line and not at the end of previous line. > > E.g., running checkpatch on commit 3485507fc272 ("staging: > bcm2835-camera: Reduce length of enum names") reports: > > CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the > previous line > + if (!ret > + && camera_port == > > Provide a simple fix by adding logical operator at the end of previous > line and removing from current line, if both the lines are additions > (ie start with '+') > > Signed-off-by: Aditya Srivastava > --- > changes in v2: quote $operator at substitution > > changes in v3: add a check for previous line ending with comment; > If so, insert $operator at the last non-comment, non-whitespace char of the > previous line > > changes in v4: improve the matching mechanism by matching line termination at > comment or white space; > insert the operator before comments (if any) separated by a whitespace; > append the comment and its pre-whitespace after the inserted operator (if > comment was present), > ie if no comment was present nothing will be inserted after the operator nak. I gave you a hint to the match string to use. $prevline =~ /[\s$;]*$/ this matches either /* foo */ or // foo comment styles (or optional blanks before EOL) Try something like: --- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fab38b493cef..3c78cf0c219f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3434,8 +3434,15 @@ sub process { # check for && or || at the start of a line if ($rawline =~ /^\+\s*(&&|\|\|)/) { - CHK("LOGICAL_CONTINUATIONS", - "Logical continuations should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("LOGICAL_CONTINUATIONS", + "Logical continuations should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + # add logical operator to the previous line, remove from current line + $prevline =~ /([\s$;]*$)/; + substr($fixed[$fixlinenr - 1], $-[0]) = " $operator" . substr($prevrawline, $-[0],$+[0]); + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } } # check indentation starts on a tab stop
[PATCH net] ipv6: addrlabel: fix possible memory leak in ip6addrlbl_net_init
kmemleak report a memory leak as follows: unreferenced object 0x8880059c6a00 (size 64): comm "ip", pid 23696, jiffies 4296590183 (age 1755.384s) hex dump (first 32 bytes): 20 01 00 10 00 00 00 00 00 00 00 00 00 00 00 00 ... 1c 00 00 00 00 00 00 00 00 00 00 00 07 00 00 00 backtrace: [] ip6addrlbl_add+0x90/0xbb0 [<70b8d7f1>] ip6addrlbl_net_init+0x109/0x170 [<6a9ca9d4>] ops_init+0xa8/0x3c0 [<2da57bf2>] setup_net+0x2de/0x7e0 [<4e52d573>] copy_net_ns+0x27d/0x530 [ ] create_new_namespaces+0x382/0xa30 [<3b76d36f>] unshare_nsproxy_namespaces+0xa1/0x1d0 [<30653721>] ksys_unshare+0x3a4/0x780 [<07e82e40>] __x64_sys_unshare+0x2d/0x40 [<31a10c08>] do_syscall_64+0x33/0x40 [<99df30e7>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 We should free all rules when we catch an error in ip6addrlbl_net_init(). otherwise a memory leak will occur. Fixes: 2a8cc6c89039 ("[IPV6] ADDRCONF: Support RFC3484 configurable address selection policy table.") Reported-by: Hulk Robot Signed-off-by: Wang Hai --- net/ipv6/addrlabel.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c index 642fc6ac13d2..637e323a0224 100644 --- a/net/ipv6/addrlabel.c +++ b/net/ipv6/addrlabel.c @@ -306,6 +306,8 @@ static int ip6addrlbl_del(struct net *net, /* add default label */ static int __net_init ip6addrlbl_net_init(struct net *net) { + struct ip6addrlbl_entry *p = NULL; + struct hlist_node *n; int err = 0; int i; @@ -320,9 +322,17 @@ static int __net_init ip6addrlbl_net_init(struct net *net) ip6addrlbl_init_table[i].prefixlen, 0, ip6addrlbl_init_table[i].label, 0); - /* XXX: should we free all rules when we catch an error? */ - if (ret && (!err || err != -ENOMEM)) + if (ret && (!err || err != -ENOMEM)) { err = ret; + goto err_ip6addrlbl_add; + } + } + return err; + +err_ip6addrlbl_add: + hlist_for_each_entry_safe(p, n, >ipv6.ip6addrlbl_table.head, list) { + hlist_del_rcu(>list); + kfree_rcu(p, rcu); } return err; } -- 2.17.1
Re: [PATCH v2 4/4] iio: hid-sensors: Add hinge sensor driver
On Sat, 2020-11-21 at 17:46 -0800, Srinivas Pandruvada wrote: > On Sat, 2020-11-21 at 17:56 +, Jonathan Cameron wrote: > > On Thu, 19 Nov 2020 18:03:31 +0800 > > Ye Xiang wrote: > > > > > The Hinge sensor is a common custom sensor on laptops. It > > > calculates > > > the angle between the lid (screen) and the base (keyboard). In > > > addition, > > > it also exposes screen and the keyboard angels with respect to > > > the > > > ground. Applications can easily get laptop's status in space > > > through > > > this sensor, in order to display appropriate user interface. > > > > I'm a little unclear on why the 3 axes aren't treated as a single > > sensor. > > You seem to always grab the 3 together or am I missing something? > > > > That will greatly simplify things and get rid of the need to have > > a shared trigger with the problems that causes in the previous > > patch. > > They are not three axes, they are independent. Xiang did try adding > x, > y and z component to represent x as hinge, y as keyboard and z as > lid. > But I was not convinced. > The problem is that then what will be sysfs interface? They are > really > a three sensors. Or we create new interface to call > in_angl_raw_keyboard > in_angl_raw_screen > in_angl_raw_lid. > You seem to indicate this is possible now some new "label" patch. Is this the patch? commit 2c3d0c9ffd24d9b4c62c5dfb2104695a614be28c Author: Phil Reid Date: Thu Sep 19 22:36:08 2019 +0800 Ideally, one iio device here is much easy to manage as other HID sensors. If we can add something other that "x", "y" and "z" component. Thanks, Srinivas > Thanks, > Srinivas > > > > Thanks, > > > > Jonathan > > > > > Signed-off-by: Ye Xiang > > > --- > > > .../hid-sensors/hid-sensor-attributes.c | 2 + > > > drivers/iio/position/Kconfig | 16 + > > > drivers/iio/position/Makefile | 3 + > > > .../iio/position/hid-sensor-custom-hinge.c| 412 > > > ++ > > > > Given it's custom probably needs a more specific name. I guess > > hid-sensor-custom-intel-hinge.c might be safe? > > > > Same for other places we need names in here. > > > > > 4 files changed, 433 insertions(+) > > > create mode 100644 drivers/iio/position/hid-sensor-custom- > > > hinge.c > > > > > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor- > > > attributes.c > > > b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > > index 442ff787f7af..5b822a4298a0 100644 > > > --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > > @@ -71,6 +71,8 @@ static struct { > > > {HID_USAGE_SENSOR_TEMPERATURE, HID_USAGE_SENSOR_UNITS_DEGREES, > > > 1000, 0}, > > > > > > {HID_USAGE_SENSOR_HUMIDITY, 0, 1000, 0}, > > > + {HID_USAGE_SENSOR_HINGE, 0, 0, 17453293}, > > > + {HID_USAGE_SENSOR_HINGE, HID_USAGE_SENSOR_UNITS_DEGREES, 0, > > > 17453293}, > > > }; > > > > > > static void simple_div(int dividend, int divisor, int *whole, > > > diff --git a/drivers/iio/position/Kconfig > > > b/drivers/iio/position/Kconfig > > > index eda67f008c5b..0346f6f2b422 100644 > > > --- a/drivers/iio/position/Kconfig > > > +++ b/drivers/iio/position/Kconfig > > > @@ -16,4 +16,20 @@ config IQS624_POS > > > To compile this driver as a module, choose M here: the module > > > will be called iqs624-pos. > > > > > > +config HID_SENSOR_CUSTOM_HINGE > > > + depends on HID_SENSOR_HUB > > > + select IIO_BUFFER > > > + select IIO_TRIGGERED_BUFFER > > > + select HID_SENSOR_IIO_COMMON > > > + select HID_SENSOR_IIO_TRIGGER > > > + tristate "HID Hinge" > > > + help > > > + This sensor present three angles, hinge angel, screen angles > > > + and keyboard angle respect to horizon (ground). > > > + Say yes here to build support for the HID SENSOR CUSTOM > > > + HINGE. > > > > Capitalization is a bit odd looking. I'd drop it. > > > > > + > > > + To compile this driver as a module, choose M here: the > > > + module will be called hid-sensor-custom-hinge. > > > + > > > endmenu > > > diff --git a/drivers/iio/position/Makefile > > > b/drivers/iio/position/Makefile > > > index 3cbe7a734352..7a6225977a01 100644 > > > --- a/drivers/iio/position/Makefile > > > +++ b/drivers/iio/position/Makefile > > > @@ -5,3 +5,6 @@ > > > # When adding new entries keep the list in alphabetical order > > > > > > obj-$(CONFIG_IQS624_POS) += iqs624-pos.o > > > + > > > +obj-$(CONFIG_HID_SENSOR_CUSTOM_HINGE) += hid-sensor-custom- > > > hinge.o > > > > Alphabetical order preferred. > > > > > +ccflags-y+= -I$(srctree)/drivers/iio/common/hid-sensors > > > > Why? > > > > > diff --git a/drivers/iio/position/hid-sensor-custom-hinge.c > > > b/drivers/iio/position/hid-sensor-custom-hinge.c > > > new file mode 100644 > > > index ..a91b333f36fa > > > --- /dev/null > > > +++ b/drivers/iio/position/hid-sensor-custom-hinge.c > > > @@ -0,0 +1,412 @@ > > > +// SPDX-License-Identifier:
Re: [PATCH v5 2/4] staging: media: Introduce NVIDIA Tegra video decoder driver
22.11.2020 04:02, Ezequiel Garcia пишет: > Hi Dmitry, > ... >> +++ b/drivers/staging/media/tegra-vde/TODO >> @@ -0,0 +1,4 @@ >> +TODO: >> + - Implement V4L2 API once it gains support for stateless decoders. >> + >> +Contact: Dmitry Osipenko > > The API for H264 stateless decoding is ready. > See https://lkml.org/lkml/2020/11/18/795. Hello Ezequiel, Thank you for the notification! My last attempt at implementing V4L API support was about a year ago and it stopped once I realized that there is no userspace which uses that API. FFMPEG and chromium browser had some kind of V4L support, but it all was oriented at downstream driver stacks, and thus, not usable. Do you know what is the current status? > One minor comment below. > ... >> + // PPS >> + __u8 pic_init_qp; >> + __u8 deblocking_filter_control_present_flag; >> + __u8 constrained_intra_pred_flag; >> + __u8 chroma_qp_index_offset; >> + __u8 pic_order_present_flag; >> + > > This seems to be bottom_field_pic_order_in_frame_present_flag, > as there is no "pic_order_present_flag" syntax element. Correct, looks like I borrowed that name from the libvdpau API. https://vdpau.pages.freedesktop.org/libvdpau/struct_vdp_picture_info_h264.html#a405f7ef26ea76bb2c446e151062fc001
Re: [PATCH v3] lockdep: Allow tuning tracing capacity constants.
On 2020/11/20 18:27, Dmitry Vyukov wrote: > Peter, so far it looks like just a very large, but normal graph to me. > The cheapest from an engineering point of view solution would be just > to increase the constants. I assume a 2x increase should buy us lots > of time to overflow. > I can think of more elaborate solutions, e.g. using bitmasks to > represent hot leaf and top-level locks. But it will both increase the > resulting code complexity (no uniform representation anymore, all code > will need to deal with different representations) and require some > time investments (that I can't justify for me at least as compared to > just throwing a bit more machine memory at it). And in the end it > won't really reduce the size of the graph. > What do you think? > Yes, I think it is a normal graph; simply syzkaller kernels tend to record a few times more dependencies than my idle kernel (shown bottom). Peter, you guessed that the culprit is sysfs at https://lkml.kernel.org/r/20200916115057.go2...@hirez.programming.kicks-ass.net , but syzbot reported at https://syzkaller.appspot.com/text?tag=MachineInfo=99b8f2b092d9714f that "BUG: MAX_LOCKDEP_ENTRIES too low!" can occur on a VM with only 2 CPUs. Is your guess catching the culprit? We could improve a few locks, but as a whole we won't be able to afford keeping sum of individual dependencies under current threshold. Therefore, allow lockdep to tune the capacity and allow syzkaller to dump when reaching the capacity will be the way to go. # cat /proc/lockdep_stats lock-classes: 1236 [max: 8192] direct dependencies: 9610 [max: 32768] indirect dependencies: 40401 all direct dependencies:174635 dependency chains: 11398 [max: 65536] dependency chain hlocks used:42830 [max: 327680] dependency chain hlocks lost:0 in-hardirq chains: 61 in-softirq chains: 414 in-process chains: 10923 stack-trace entries: 93041 [max: 524288] number of stack traces: 4997 number of stack hash chains: 4292 combined max dependencies: 281074520 hardirq-safe locks: 50 hardirq-unsafe locks: 805 softirq-safe locks:146 softirq-unsafe locks: 722 irq-safe locks:155 irq-unsafe locks: 805 hardirq-read-safe locks: 2 hardirq-read-unsafe locks: 129 softirq-read-safe locks:11 softirq-read-unsafe locks: 123 irq-read-safe locks:11 irq-read-unsafe locks: 129 uncategorized locks: 224 unused locks:0 max locking depth: 15 max bfs queue depth: 215 chain lookup misses: 11664 chain lookup hits:37393935 cyclic checks: 11053 redundant checks:0 redundant links: 0 find-mask forwards checks:1588 find-mask backwards checks: 1779 hardirq on events:17502380 hardirq off events: 17502376 redundant hardirq ons: 0 redundant hardirq offs: 0 softirq on events: 90845 softirq off events: 90845 redundant softirq ons: 0 redundant softirq offs: 0 debug_locks: 1 zapped classes: 0 zapped lock chains: 0 large chain blocks: 1 # awk ' { if ($2 == "OPS:") print $5" "$9 } ' /proc/lockdep | sort -rV | head -n 30 423 (wq_completion)events 405 (wq_completion)events_unbound 393 >f_pos_lock 355 >lock 349 sb_writers#3 342 sb_writers#6 338 >mutex 330 (work_completion)(>work) 330 pernet_ops_rwsem 289 epmutex 288 >mtx 281 tty_mutex 280 >legacy_mutex 273 >legacy_mutex/1 269 >ldisc_sem 268 (wq_completion)ipv6_addrconf 266 (work_completion)(&(>dad_work)->work) 266 (linkwatch_work).work 266 (addr_chk_work).work 266 >atomic_read_lock 265 (work_completion)(>work) 265 rtnl_mutex 263 >atomic_write_lock 262 >lock 261 >termios_rwsem 242 >buf.lock/1 242 kn->active#40 241 _tty->termios_rwsem/1 240 registration_lock 239 >output_lock # awk ' { if ($2 == "OPS:") print $7" "$9 } ' /proc/lockdep | sort -rV | head -n 30 642 pool_lock 641 _hash[i].lock 582 tk_core.seq.seqcount 561 hrtimer_bases.lock 560 _rq->rt_runtime_lock 559 _b->rt_runtime_lock 559 >lock 559 _rq->removed.lock 559 _b->lock 558 >lock 550 >delays->lock 549 >pi_lock 506 >lock 504 >list_lock 432 &s->seqcount 404 >wait#10 401 >lock 369 >lock 330 rcu_node_0 326 &(kretprobe_table_locks[i].lock) 326 pgd_lock 324 >lru_lock 323 lock#5 321 _wait_table[i] 319 ptlock_ptr(page)#2/1 318 ptlock_ptr(page)#2
Re: [PATCH v2 2/4] iio: hid-sensor-trigger: Decrement runtime pm enable count on driver removal
On Sat, 2020-11-21 at 17:22 +, Jonathan Cameron wrote: > On Thu, 19 Nov 2020 18:03:29 +0800 > Ye Xiang wrote: > > > To avoid pm_runtime_disable called repeatedly by hid sensor > > drivers, > > decrease runtime pm enable count after call it. > > > > Signed-off-by: Ye Xiang > This sounds like a fix. If so please make that clear and add a fixes > tag. > > If it couldn't have been triggered before, then please explain why in > this > patch description. Normally it is not a problem as this is called during driver remove and next time insmod this data structure is recreated and count will init again. This is the artifect of the shared iio devices and one of them is still open, so the instance will not be freed. But patch is fine as to do opposite of what is done during alloc. Thanks, Srinivas > > Thanks, > > Jonathan > > > --- > > drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > index ff375790b7e8..30340abcbc8d 100644 > > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > @@ -227,8 +227,10 @@ static int > > hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig, > > void hid_sensor_remove_trigger(struct iio_dev *indio_dev, > >struct hid_sensor_common *attrb) > > { > > - if (atomic_read(>runtime_pm_enable)) > > + if (atomic_read(>runtime_pm_enable)) { > > pm_runtime_disable(>pdev->dev); > > + atomic_dec(>runtime_pm_enable); > > + } > > > > pm_runtime_set_suspended(>pdev->dev); > > pm_runtime_put_noidle(>pdev->dev);
Re: [PATCH net-next v3 00/12] net: dsa: microchip: PTP support for KSZ956x
On Sat, Nov 21, 2020 at 03:26:11AM +0200, Vladimir Oltean wrote: > On Thu, Nov 19, 2020 at 06:51:15PM +, tristram...@microchip.com wrote: > > I think the right implementation is for the driver to remember this receive > > timestamp > > of Pdelay_Req and puts it in the tail tag when it sees a 1-step Pdelay_Resp > > is sent. As long as this is transparent to user space, it could work. Remember that user space simply copies the correction field from the Request into the Response. If the driver correctly accumulates the turnaround time into the correction field of the response, then all is well. > I have mixed feelings about this. IIUC, you're saying "let's implement a > fixed-size FIFO of RX timestamps of Pdelay_Req messages, and let's match > them on TX to Pdelay_Resp messages, by {sequenceId, domainNumber}." > > But how deep should we make that FIFO? I.e. how many Pdelay_Req messages > should we expect before the user space will inject back a Pdelay_Resp > for transmission? Good question. Normally you would expect just one Request pending at any one time, but nothing guarantees that, and so the driver would have to match the Req/Resp exactly and deal with rogue/buggy requests and responses. Thanks, Richard
Re: [PATCH v2 4/4] iio: hid-sensors: Add hinge sensor driver
On Sat, 2020-11-21 at 17:56 +, Jonathan Cameron wrote: > On Thu, 19 Nov 2020 18:03:31 +0800 > Ye Xiang wrote: > > > The Hinge sensor is a common custom sensor on laptops. It > > calculates > > the angle between the lid (screen) and the base (keyboard). In > > addition, > > it also exposes screen and the keyboard angels with respect to the > > ground. Applications can easily get laptop's status in space > > through > > this sensor, in order to display appropriate user interface. > > I'm a little unclear on why the 3 axes aren't treated as a single > sensor. > You seem to always grab the 3 together or am I missing something? > > That will greatly simplify things and get rid of the need to have > a shared trigger with the problems that causes in the previous > patch. They are not three axes, they are independent. Xiang did try adding x, y and z component to represent x as hinge, y as keyboard and z as lid. But I was not convinced. The problem is that then what will be sysfs interface? They are really a three sensors. Or we create new interface to call in_angl_raw_keyboard in_angl_raw_screen in_angl_raw_lid. Thanks, Srinivas > > Thanks, > > Jonathan > > > Signed-off-by: Ye Xiang > > --- > > .../hid-sensors/hid-sensor-attributes.c | 2 + > > drivers/iio/position/Kconfig | 16 + > > drivers/iio/position/Makefile | 3 + > > .../iio/position/hid-sensor-custom-hinge.c| 412 > > ++ > > Given it's custom probably needs a more specific name. I guess > hid-sensor-custom-intel-hinge.c might be safe? > > Same for other places we need names in here. > > > 4 files changed, 433 insertions(+) > > create mode 100644 drivers/iio/position/hid-sensor-custom-hinge.c > > > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > index 442ff787f7af..5b822a4298a0 100644 > > --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > @@ -71,6 +71,8 @@ static struct { > > {HID_USAGE_SENSOR_TEMPERATURE, HID_USAGE_SENSOR_UNITS_DEGREES, > > 1000, 0}, > > > > {HID_USAGE_SENSOR_HUMIDITY, 0, 1000, 0}, > > + {HID_USAGE_SENSOR_HINGE, 0, 0, 17453293}, > > + {HID_USAGE_SENSOR_HINGE, HID_USAGE_SENSOR_UNITS_DEGREES, 0, > > 17453293}, > > }; > > > > static void simple_div(int dividend, int divisor, int *whole, > > diff --git a/drivers/iio/position/Kconfig > > b/drivers/iio/position/Kconfig > > index eda67f008c5b..0346f6f2b422 100644 > > --- a/drivers/iio/position/Kconfig > > +++ b/drivers/iio/position/Kconfig > > @@ -16,4 +16,20 @@ config IQS624_POS > > To compile this driver as a module, choose M here: the module > > will be called iqs624-pos. > > > > +config HID_SENSOR_CUSTOM_HINGE > > + depends on HID_SENSOR_HUB > > + select IIO_BUFFER > > + select IIO_TRIGGERED_BUFFER > > + select HID_SENSOR_IIO_COMMON > > + select HID_SENSOR_IIO_TRIGGER > > + tristate "HID Hinge" > > + help > > + This sensor present three angles, hinge angel, screen angles > > + and keyboard angle respect to horizon (ground). > > + Say yes here to build support for the HID SENSOR CUSTOM > > + HINGE. > > Capitalization is a bit odd looking. I'd drop it. > > > + > > + To compile this driver as a module, choose M here: the > > + module will be called hid-sensor-custom-hinge. > > + > > endmenu > > diff --git a/drivers/iio/position/Makefile > > b/drivers/iio/position/Makefile > > index 3cbe7a734352..7a6225977a01 100644 > > --- a/drivers/iio/position/Makefile > > +++ b/drivers/iio/position/Makefile > > @@ -5,3 +5,6 @@ > > # When adding new entries keep the list in alphabetical order > > > > obj-$(CONFIG_IQS624_POS) += iqs624-pos.o > > + > > +obj-$(CONFIG_HID_SENSOR_CUSTOM_HINGE) += hid-sensor-custom-hinge.o > > Alphabetical order preferred. > > > +ccflags-y += -I$(srctree)/drivers/iio/common/hid-sensors > > Why? > > > diff --git a/drivers/iio/position/hid-sensor-custom-hinge.c > > b/drivers/iio/position/hid-sensor-custom-hinge.c > > new file mode 100644 > > index ..a91b333f36fa > > --- /dev/null > > +++ b/drivers/iio/position/hid-sensor-custom-hinge.c > > @@ -0,0 +1,412 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * HID Sensors Driver > > + * Copyright (c) 2020, Intel Corporation. > > + */ > > +#include > > +#include > > +#include > > +#include > > + > > +#include "hid-sensor-trigger.h" > > + > > +/* Channel definitions */ > > +static const struct iio_chan_spec hinge_channels[] = { > > + { .type = IIO_ANGL, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + .info_mask_shared_by_type = > > + BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_SCALE) > > | > > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | > > BIT(IIO_CHAN_INFO_HYSTERESIS), > > + .scan_type.realbits = 16, > > + .scan_type.storagebits = 32,
[Patch v2 1/1] PCI: pciehp: Add support for handling MRL events
When Mechanical Retention Lock (MRL) is present, Linux doesn't process those change events. Support for these can be found starting Icelake Server. The following changes need to be enabled when MRL is present. 1. Subscribe to MRL change events in SlotControl. 2. When MRL is closed, - If there is no ATTN button, then POWER on the slot. - If there is ATTN button, and an MRL event pending, ignore Presence Detect. Since we want ATTN button to drive the hotplug event. - If currently slot is powered on, but MRL is open, PCIe Base Spec 5.0 Chapter 6.7.1.3 states. If an MRL Sensor is implemented without a corresponding MRL Sensor input on the Hot-Plug Controller, it is recommended that the MRL Sensor be routed to power fault input of the Hot-Plug Controller. This allows an active adapter to be powered off when the MRL is opened." This seems to suggest that the slot should be brought down as soon as MRL is opened. Signed-off-by: Ashok Raj Co-developed-by: Kuppuswamy Sathyanarayanan --- Changes since v1: - Changes suggested by Lucas Wunner https://lore.kernel.org/linux-pci/20201119223749.GA103783@otc-nc-03/T/#m1f661ae901e7dedad73dea370bb63abd52c610eb - Consolidate MRL handling in pciehp_handle_presence_or_link_change() - Added helped latch_closed() - Add comments why MRL open should function as hot-remove. - Don't nuke PDC, it might mask a button PUSH synthesized event after 5 secs. - Bjorn: Fix Subject to be consistent with other commits. --- drivers/pci/hotplug/pciehp_ctrl.c | 36 +++- drivers/pci/hotplug/pciehp_hpc.c | 14 -- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 9f85815b4f53..aa8b187ff769 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -224,9 +224,22 @@ void pciehp_handle_disable_request(struct controller *ctrl) ctrl->request_result = pciehp_disable_slot(ctrl, SAFE_REMOVAL); } +static bool latch_closed(struct controller *ctrl) +{ + u8 getstatus = 0; + + if (!MRL_SENS(ctrl)) + return true; + + pciehp_get_latch_status(ctrl, ); + + return (getstatus == 0 ? true : false); +} + void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events) { int present, link_active; + u8 getstatus = 0; /* * If the slot is on and presence or link has changed, turn it off. @@ -246,6 +259,20 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events) if (events & PCI_EXP_SLTSTA_PDC) ctrl_info(ctrl, "Slot(%s): Card not present\n", slot_name(ctrl)); + if (events & PCI_EXP_SLTSTA_MRLSC) + ctrl_info(ctrl, "Slot(%s): Latch %s\n", + slot_name(ctrl), getstatus ? "Open" : "Closed"); + /* +* PCIe Base Spec 5.0 Chapter 6.7.1.3 states. +* +* If an MRL Sensor is implemented without a corresponding MRL Sensor input +* on the Hot-Plug Controller, it is recommended that the MRL Sensor be +* routed to power fault input of the Hot-Plug Controller. +* This allows an active adapter to be powered off when the MRL is opened." +* +* This seems to suggest that the slot should be brought down as soon as MRL +* is opened. +*/ pciehp_disable_slot(ctrl, SURPRISE_REMOVAL); break; default: @@ -257,7 +284,7 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events) mutex_lock(>state_lock); present = pciehp_card_present(ctrl); link_active = pciehp_check_link_active(ctrl); - if (present <= 0 && link_active <= 0) { + if ((present <= 0 && link_active <= 0) || !latch_closed(ctrl)) { mutex_unlock(>state_lock); return; } @@ -275,6 +302,13 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events) if (link_active) ctrl_info(ctrl, "Slot(%s): Link Up\n", slot_name(ctrl)); + /* +* If slot is closed && ATTN button exists +* don't continue, let the ATTN button +* drive the hot-plug +*/ + if (((events & PCI_EXP_SLTSTA_MRLSC) && ATTN_BUTTN(ctrl))) + return; ctrl->request_result = pciehp_enable_slot(ctrl); break; default: diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 53433b37e181..7cfa27bcf951 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++
Re: [PATCH net-next v3 00/12] net: dsa: microchip: PTP support for KSZ956x
On Sat, Nov 21, 2020 at 03:26:11AM +0200, Vladimir Oltean wrote: > On Thu, Nov 19, 2020 at 06:51:15PM +, tristram...@microchip.com wrote: > > The receive and transmit latencies are different for different connected > > speed. So the > > driver needs to change them when the link changes. For that reason the PTP > > stack > > should not use its own latency values as generally the application does not > > care about > > the linked speed. > > The thing is, ptp4l already has ingressLatency and egressLatency > settings, and I would not be surprised if those config options would get > extended to cover values at multiple link speeds. > > In the general case, the ksz9477 MAC could be attached to any external > PHY, having its own propagation delay characteristics, or any number of > other things that cause clock domain crossings. I'm not sure how feasible > it is for the kernel to abstract this away completely, and adjust > timestamps automatically based on any and all combinations of MAC and > PHY. Maybe this is just wishful thinking. The idea that the driver will correctly adjust time stamps according to link speed sounds nice in theory, but in practice it fails. There is a at least one other driver that attempted this, but, surprise, surprise, the hard coded correction values turned out to be wrong. I think the best way would be to let user space monitor the link speed and apply the matching correction value. That way, we avoid bogus, hard coded values in kernel space. (This isn't implemented in linuxptp, but it certainly could be.) Thanks, Richard
Re: [PATCH 1/1] pci: pciehp: Handle MRL interrupts to enable slot for hotplug.
On Sat, Nov 21, 2020 at 12:10:50PM +0100, Lukas Wunner wrote: > > > > > + /* > > > > +* If ATTN is present and MRL is triggered > > > > +* ignore the Presence Change Event. > > > > +*/ > > > > + if (ATTN_BUTTN(ctrl) && (events & PCI_EXP_SLTSTA_MRLSC)) > > > > + events &= ~PCI_EXP_SLTSTA_PDC; > > > > > > An Attention Button press results in a synthesized PDC event after > > > 5 seconds, which may get lost due to the above if-statement. > > > > When its synthesized you don't get the MRLSC? So we won't nuke the PDC then > > correct? > > I just meant to say, pciehp_queue_pushbutton_work() will synthesize > a PDC event after 5 seconds and with the above code snippet, if an > MRL event happens simultaneously, that synthesized PDC event would > be lost. So I'd just drop the above code snippet. I think you > just need to subscribe to MRL events and propagate them to > pciehp_handle_presence_or_link_change(). There, you'd bring down > the slot if an MRL event has occurred (same as DLLSC or PDC). > Then, check whether MRL is closed. If so, and if presence or link > is up, try to bring up the slot. > > If the MRL is open when slot or presence has gone up, the slot is not > brought up. But once MRL is closed, there'll be another MRL event and > *then* the slot is brought up. > Sounds good.. I'll send the update patch.
Re: [PATCH v1] qnx4_match: do not over run the buffer
Thanks for the clarification! This sounds good to me. I will send a revised patch. Best, - Tong > On Nov 21, 2020, at 4:57 PM, Anders Larsen wrote: > > On Saturday, 2020-11-21 22:47 Tong Zhang wrote: >> >>> On Nov 21, 2020, at 4:40 PM, Anders Larsen wrote: >>> >>> On Friday, 2020-11-20 22:21 Tong Zhang wrote: the di_fname may not terminated by '\0', use strnlen to prevent buffer overrun --- fs/qnx4/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/qnx4/namei.c b/fs/qnx4/namei.c index 8d72221735d7..c0e79094f578 100644 --- a/fs/qnx4/namei.c +++ b/fs/qnx4/namei.c @@ -40,7 +40,7 @@ static int qnx4_match(int len, const char *name, } else { namelen = QNX4_SHORT_NAME_MAX; } - thislen = strlen( de->di_fname ); + thislen = strnlen( de->di_fname, QNX4_SHORT_NAME_MAX ); >>> >>> that should be >>> + thislen = strnlen( de->di_fname, namelen ); >>> otherwise the length of a filename would always be limited to >>> QNX4_SHORT_NAME_MAX (16) characters. >>> >> Why should we put something bigger here if the size of >> qnx4_inode_entry->di_fname is QNX4_SHORT_NAME_MAX. >> Won’t that be a problem? > > If QNX4_FILE_LINK is set in de->di_status (see line 38), 'de' actually points > to a struct qnx4_link_info which can hold a longer name. > That's the reason for the namelen massage. > (Please don't ask why it is not a union) > > Cheers > Anders > >
[PATCH v2] qnx4_match: do not over run the buffer
the di_fname may not terminated by '\0', use strnlen to prevent buffer overrun [ 513.248784] qnx4_readdir: bread failed (3718095557) [ 513.250880] == [ 513.251109] BUG: KASAN: use-after-free in strlen+0x1f/0x40 [ 513.251268] Read of size 1 at addr 88800270 by task find/230 [ 513.251419] [ 513.251677] CPU: 0 PID: 230 Comm: find Not tainted 5.10.0-rc4+ #64 [ 513.251805] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-48-gd84 [ 513.252069] Call Trace: [ 513.252310] dump_stack+0x7d/0xa3 [ 513.252443] print_address_description.constprop.0+0x1e/0x220 [ 513.252572] ? _raw_spin_lock_irqsave+0x7b/0xd0 [ 513.252681] ? _raw_write_lock_irqsave+0xd0/0xd0 [ 513.252785] ? strlen+0x1f/0x40 [ 513.252869] ? strlen+0x1f/0x40 [ 513.252955] kasan_report.cold+0x37/0x7c [ 513.253059] ? qnx4_block_map+0x130/0x1d0 [ 513.253152] ? strlen+0x1f/0x40 [ 513.253237] strlen+0x1f/0x40 [ 513.253329] qnx4_lookup+0xab/0x220 [ 513.253431] __lookup_slow+0x103/0x220 [ 513.253531] ? vfs_unlink+0x2e0/0x2e0 [ 513.253626] ? down_read+0xd8/0x190 [ 513.253721] ? down_write_killable+0x110/0x110 [ 513.253823] ? generic_permission+0x4c/0x240 [ 513.253929] walk_component+0x214/0x2c0 [ 513.254035] ? handle_dots.part.0+0x760/0x760 [ 513.254137] ? walk_component+0x2c0/0x2c0 [ 513.254233] ? path_init+0x546/0x6b0 [ 513.254327] ? __kernel_text_address+0x9/0x30 [ 513.254430] ? unwind_get_return_address+0x2a/0x40 [ 513.254538] ? create_prof_cpu_mask+0x20/0x20 [ 513.254637] ? arch_stack_walk+0x99/0xf0 [ 513.254736] path_lookupat.isra.0+0xb0/0x240 [ 513.254840] filename_lookup+0x128/0x250 [ 513.254939] ? may_linkat+0xb0/0xb0 [ 513.255033] ? __fput+0x199/0x3c0 [ 513.255127] ? kasan_save_stack+0x32/0x40 [ 513.255224] ? kasan_save_stack+0x1b/0x40 [ 513.255323] ? kasan_unpoison_shadow+0x33/0x40 [ 513.255426] ? __kasan_kmalloc.constprop.0+0xc2/0xd0 [ 513.255538] ? getname_flags+0x100/0x2a0 [ 513.255635] vfs_statx+0xd8/0x1d0 [ 513.255728] ? vfs_getattr+0x40/0x40 [ 513.255821] ? lockref_put_return+0xb2/0x120 [ 513.255922] __do_sys_newfstatat+0x7d/0xd0 [ 513.256022] ? __ia32_sys_newlstat+0x30/0x30 [ 513.256122] ? __kasan_slab_free+0x121/0x150 [ 513.256222] ? rcu_segcblist_enqueue+0x72/0x80 [ 513.256333] ? fpregs_assert_state_consistent+0x4d/0x60 [ 513.256446] ? exit_to_user_mode_prepare+0x2d/0xf0 [ 513.256551] ? __x64_sys_newfstatat+0x39/0x60 [ 513.256651] do_syscall_64+0x33/0x40 [ 513.256750] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Co-Developed-by: Anders Larsen Signed-off-by: Tong Zhang --- v2: The name can grow longer than QNX4_SHORT_NAME_MAX if de is a QNX4_FILE_LINK type and de should points to a qnx4_link_info struct, so this is safe. We also remove redundant checks in this version. fs/qnx4/namei.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs/qnx4/namei.c b/fs/qnx4/namei.c index 8d72221735d7..2bcbbd7c772e 100644 --- a/fs/qnx4/namei.c +++ b/fs/qnx4/namei.c @@ -40,9 +40,7 @@ static int qnx4_match(int len, const char *name, } else { namelen = QNX4_SHORT_NAME_MAX; } - thislen = strlen( de->di_fname ); - if ( thislen > namelen ) - thislen = namelen; + thislen = strnlen( de->di_fname, namelen ); if (len != thislen) { return 0; } -- 2.25.1
include/net/sock.h:379:34: error: 'struct sock_common' has no member named 'skc_v6_daddr'; did you mean
Hi Tariq, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: a349e4c659609fd20e4beea89e5c4a4038e33a95 commit: 5229a96e59ec32466add5e87b537cc3f244afb06 net/mlx5e: Accel, Expose flow steering API for rules add/del date: 5 months ago config: sparc-randconfig-r013-20201122 (attached as .config) compiler: sparc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5229a96e59ec32466add5e87b537cc3f244afb06 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout 5229a96e59ec32466add5e87b537cc3f244afb06 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): In file included from include/linux/string.h:20, from include/linux/bitmap.h:9, from include/linux/cpumask.h:12, from include/linux/smp.h:13, from include/linux/percpu.h:7, from include/linux/netdevice.h:32, from drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c:4: drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c: In function 'accel_fs_tcp_set_ipv6_flow': >> include/net/sock.h:379:34: error: 'struct sock_common' has no member named >> 'skc_v6_daddr'; did you mean 'skc_daddr'? 379 | #define sk_v6_daddr __sk_common.skc_v6_daddr | ^~~~ arch/sparc/include/asm/string.h:15:45: note: in definition of macro 'memcpy' 15 | #define memcpy(t, f, n) __builtin_memcpy(t, f, n) | ^ drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c:55:14: note: in expansion of macro 'sk_v6_daddr' 55 | >sk_v6_daddr, 16); | ^~~ At top level: drivers/net/ethernet/mellanox/mlx5/core/en_accel/fs_tcp.c:47:13: warning: 'accel_fs_tcp_set_ipv6_flow' defined but not used [-Wunused-function] 47 | static void accel_fs_tcp_set_ipv6_flow(struct mlx5_flow_spec *spec, struct sock *sk) | ^~ vim +379 include/net/sock.h 4dc6dc7162c08b9 Eric Dumazet 2009-07-15 359 68835aba4d9b74e Eric Dumazet 2010-11-30 360 #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin 68835aba4d9b74e Eric Dumazet 2010-11-30 361 #define sk_dontcopy_end __sk_common.skc_dontcopy_end 4dc6dc7162c08b9 Eric Dumazet 2009-07-15 362 #define sk_hash __sk_common.skc_hash 5080546682bae3d Eric Dumazet 2013-10-02 363 #define sk_portpair __sk_common.skc_portpair 05dbc7b59481ca8 Eric Dumazet 2013-10-03 364 #define sk_num __sk_common.skc_num 05dbc7b59481ca8 Eric Dumazet 2013-10-03 365 #define sk_dport __sk_common.skc_dport 5080546682bae3d Eric Dumazet 2013-10-02 366 #define sk_addrpair __sk_common.skc_addrpair 5080546682bae3d Eric Dumazet 2013-10-02 367 #define sk_daddr __sk_common.skc_daddr 5080546682bae3d Eric Dumazet 2013-10-02 368 #define sk_rcv_saddr __sk_common.skc_rcv_saddr ^1da177e4c3f415 Linus Torvalds 2005-04-16 369 #define sk_family __sk_common.skc_family ^1da177e4c3f415 Linus Torvalds 2005-04-16 370 #define sk_state __sk_common.skc_state ^1da177e4c3f415 Linus Torvalds 2005-04-16 371 #define sk_reuse __sk_common.skc_reuse 055dc21a1d1d219 Tom Herbert 2013-01-22 372 #define sk_reuseport __sk_common.skc_reuseport 9fe516ba3fb29b6 Eric Dumazet 2014-06-27 373 #define sk_ipv6only __sk_common.skc_ipv6only 26abe14379f8e2f Eric W. Biederman2015-05-08 374 #define sk_net_refcnt __sk_common.skc_net_refcnt ^1da177e4c3f415 Linus Torvalds 2005-04-16 375 #define sk_bound_dev_if __sk_common.skc_bound_dev_if ^1da177e4c3f415 Linus Torvalds 2005-04-16 376 #define sk_bind_node __sk_common.skc_bind_node 8feaf0c0a5488b3 Arnaldo Carvalho de Melo 2005-08-09 377 #define sk_prot __sk_common.skc_prot 07feaebfcc10cd3 Eric W. Biederman2007-09-12 378 #define sk_net __sk_common.skc_net efe4208f47f907b Eric Dumazet 2013-10-03 @379 #define sk_v6_daddr
Re: [PATCH v2 2/2] fpga: dfl: look for vendor specific capability
Hi Matthew, On Wed, Nov 18, 2020 at 11:01:51AM -0800, matthew.gerl...@linux.intel.com wrote: > From: Matthew Gerlach > > A DFL may not begin at offset 0 of BAR 0. A PCIe vendor > specific capability can be used to specify the start of a > number of DFLs. > > Signed-off-by: Matthew Gerlach > --- > v2: Update documentation for clarity. > Clean up macro names. > Use GENMASK. > Removed spurious blank lines. > Changed some calls from dev_info to dev_dbg. > Specifically check for VSEC not found, -ENODEV. > Ensure correct pci vendor id. > Remove check for page alignment. > Rename find_dfl_in_cfg to find_dfls_by_vsec. > Initialize target memory of pci_read_config_dword to invalid values > before use. > --- > Documentation/fpga/dfl.rst | 13 ++ > drivers/fpga/dfl-pci.c | 87 +- > 2 files changed, 98 insertions(+), 2 deletions(-) > > diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst > index 0404fe6ffc74..37016ff35a90 100644 > --- a/Documentation/fpga/dfl.rst > +++ b/Documentation/fpga/dfl.rst > @@ -501,6 +501,19 @@ Developer only needs to provide a sub feature driver > with matched feature id. > FME Partial Reconfiguration Sub Feature driver (see > drivers/fpga/dfl-fme-pr.c) > could be a reference. > > +Location of DFLs on PCI Device > +=== Maybe start with: "There are two ways of locating the DFLs" > +The start of the first DFL is assumed to be offset 0 of bar 0. > +If the first node of the DFL is an FME, then further DFLs > +in the port(s) are specified in FME header registers. > +Alternatively, a vendor specific capability structure can be used to > +specify the location of all the DFLs on the device, providing flexibility > +for the type of starting node in the DFL. Intel has reserved the > +VSEC ID of 0x43 for this purpose. The vendor specific > +data begins with a 4 byte vendor specific register for the number of DFLs > followed 4 byte > +Offset/BIR vendor specific registers for each DFL. Bits 2:0 of Offset/BIR > register > +indicates the BAR, and bits 31:3 form the 8 byte aligned offset where bits > 2:0 are > +zero. It's nice to have details, thanks! Nit: This could be a table maybe? > > Open discussion > === > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c > index b27fae045536..3a6807e3e10c 100644 > --- a/drivers/fpga/dfl-pci.c > +++ b/drivers/fpga/dfl-pci.c > @@ -27,6 +27,14 @@ > #define DRV_VERSION "0.8" > #define DRV_NAME "dfl-pci" > > +#define PCI_VSEC_ID_INTEL_DFLS 0x43 > + > +#define PCI_VNDR_DFLS_CNT 8 > +#define PCI_VNDR_DFLS_RES 0x0c > + > +#define PCI_VNDR_DFLS_RES_BAR_MASK GENMASK(2, 0) > +#define PCI_VNDR_DFLS_RES_OFF_MASK GENMASK(31, 3) > + > struct cci_drvdata { > struct dfl_fpga_cdev *cdev; /* container device */ > }; > @@ -119,8 +127,80 @@ static int *cci_pci_create_irq_table(struct pci_dev > *pcidev, unsigned int nvec) > return table; > } > > +static int find_dfls_by_vsec(struct pci_dev *pcidev, struct > dfl_fpga_enum_info *info) > +{ > + u32 bar, offset, vndr_hdr, dfl_cnt, dfl_res; > + int dfl_res_off, i, voff = 0; > + resource_size_t start, len; > + > + if (pcidev->vendor != PCI_VENDOR_ID_INTEL) > + return -ENODEV; > + > + while ((voff = pci_find_next_ext_capability(pcidev, voff, > PCI_EXT_CAP_ID_VNDR))) { > + vndr_hdr = 0; > + pci_read_config_dword(pcidev, voff + PCI_VNDR_HEADER, > _hdr); Are there concerns around those failing? > + > + dev_dbg(>dev, > + "vendor-specific capability id 0x%x, rev 0x%x len > 0x%x\n", > + PCI_VNDR_HEADER_ID(vndr_hdr), > + PCI_VNDR_HEADER_REV(vndr_hdr), > + PCI_VNDR_HEADER_LEN(vndr_hdr)); > + > + if (PCI_VNDR_HEADER_ID(vndr_hdr) == PCI_VSEC_ID_INTEL_DFLS) > + break; > + } > + > + if (!voff) { > + dev_dbg(>dev, "%s no VSEC found\n", __func__); > + return -ENODEV; > + } > + > + dfl_cnt = 0; > + pci_read_config_dword(pcidev, voff + PCI_VNDR_DFLS_CNT, _cnt); I guess this could fall on it's face if you'd read back ~0 ... maybe I'm being too paranoid :) > + dev_dbg(>dev, "dfl_cnt %d\n", dfl_cnt); > + for (i = 0; i < dfl_cnt; i++) { > + dfl_res_off = voff + PCI_VNDR_DFLS_RES + > + (i * sizeof(dfl_res)); > + dfl_res = GENMASK(31, 0); > + pci_read_config_dword(pcidev, dfl_res_off, _res); > + > + dev_dbg(>dev, "dfl_res 0x%x\n", dfl_res); > + > + bar = dfl_res & PCI_VNDR_DFLS_RES_BAR_MASK; > + if (bar >= PCI_STD_NUM_BARS) { > + dev_err(>dev, "%s bad bar number %d\n", > + __func__, bar); > + return -EINVAL; > + } > + > + len =
Re: [PATCHv2 1/5] firmware: stratix10-svc: add COMMAND_AUTHENTICATE_BITSTREAM flag
Richard, On Wed, Nov 18, 2020 at 12:16:09PM -0600, Richard Gong wrote: > > > -#define COMMAND_RECONFIG_FLAG_PARTIAL1 > > > +#define COMMAND_RECONFIG_FLAG_PARTIAL0 > > > +#define COMMAND_AUTHENTICATE_BITSTREAM 1 > > > > Can you explain how this commit by itself doesn't break things? > > > > Before this change firmware expected BIT(0) to be set for partial > > reconfiguration, now BIT(0) suddenly means authentication? How doest his > > work? :) > > > Was there a firmware version change? Did this never work before? > > > > If this is version depenedent for firmware, then this might need a > > different compatible string / id / some form of probing? > > > > Entirely possible that I'm missing something, but it doesn't *seem* > > right. > > It did work before. > > Before this change, firmware only checks if the received flag value is zero. > If the value is zero, it preforms full reconfiguration. Otherwise it does > partial reconfiguration. > > To support bitstream authentication feature, firmware is updated to check > the received flag value as below: > 0 --- full reconfiguration > BIT(0) --- partial reconfiguration > BIT(1) --- bitstream authentication So there are two different versions of firmware involved that behave differently? Old firmware: - ctype.flags = 0x0 -> Full reconfig - ctype.flags != 0 -> Partial reconfig New firmware: - ctype.flags = 0x0 -> Full reconfig - ctype.flags = 0x1 -> Partial reconfig - ctype.flags = 0x2 -> Authenticate Old software: - Send 0x0 for Full - Send 0x1 for Partial New software: - Send 0x0 for Full - Send 0x1 for Partial - Send 0x2 for Auth If I send request for authentication BIT(1) (new software) to old firmware it'd try and attempt a partial reconfiguration with the data I send? Is that safe? Is there a way for software to figure out the firmware version and do the right thing? > Therefore I have updated the command flag setting at Intel service layer > driver to align with firmware. > > Regards, > Richard > > > > /** > > >* Timeout settings for service clients: > > > -- > > > 2.7.4 > > > > > > > Cheers, > > Moritz > > Thanks, Moritz
Re: Performance regressions in "boot_time" tests in Linux 5.8 Kernel
On 11/20/20 at 03:11am, Rahul Gopakumar wrote: > Hi Baoquan, > > To which commit should we apply the draft patch. We tried applying > the patch to the commit 3e4fb4346c781068610d03c12b16c0cfb0fd24a3 > (the one we used for applying the previous patch) but it fails. I tested on 5.10-rc3+. You can append below change to the old patch in your testing kernel. diff --git a/mm/page_alloc.c b/mm/page_alloc.c index fa6076e1a840..5e5b74e88d69 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -448,6 +448,8 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn) if (end_pfn < pgdat_end_pfn(NODE_DATA(nid))) return false; + if (NODE_DATA(nid)->first_deferred_pfn != ULONG_MAX) + return true; /* * We start only with one section of pages, more pages are added as * needed until the rest of deferred pages are initialized.
Re: [PATCH v5 2/4] staging: media: Introduce NVIDIA Tegra video decoder driver
Hi Dmitry, On Mon, 11 Dec 2017 at 21:27, Dmitry Osipenko wrote: > > NVIDIA Tegra20/30/114/124/132 SoC's have video decoder engine that > supports standard set of video formats like H.264 / MPEG-4 / WMV / VC1. > > Signed-off-by: Dmitry Osipenko > --- > MAINTAINERS |9 + > drivers/staging/media/Kconfig |2 + > drivers/staging/media/Makefile |1 + > drivers/staging/media/tegra-vde/Kconfig |7 + > drivers/staging/media/tegra-vde/Makefile|1 + > drivers/staging/media/tegra-vde/TODO|4 + > drivers/staging/media/tegra-vde/tegra-vde.c | 1213 > +++ > drivers/staging/media/tegra-vde/uapi.h | 78 ++ > 8 files changed, 1315 insertions(+) > create mode 100644 drivers/staging/media/tegra-vde/Kconfig > create mode 100644 drivers/staging/media/tegra-vde/Makefile > create mode 100644 drivers/staging/media/tegra-vde/TODO > create mode 100644 drivers/staging/media/tegra-vde/tegra-vde.c > create mode 100644 drivers/staging/media/tegra-vde/uapi.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7d195739f892..7f7c24949a06 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -8706,6 +8706,15 @@ T: git git://linuxtv.org/media_tree.git > S: Maintained > F: drivers/media/dvb-frontends/stv6111* > > +MEDIA DRIVERS FOR NVIDIA TEGRA - VDE > +M: Dmitry Osipenko > +L: linux-me...@vger.kernel.org > +L: linux-te...@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt > +F: drivers/staging/media/tegra-vde/ > + > MEDIA INPUT INFRASTRUCTURE (V4L/DVB) > M: Mauro Carvalho Chehab > M: Mauro Carvalho Chehab > diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig > index 3a09140700e6..227437f22acf 100644 > --- a/drivers/staging/media/Kconfig > +++ b/drivers/staging/media/Kconfig > @@ -31,4 +31,6 @@ source "drivers/staging/media/imx/Kconfig" > > source "drivers/staging/media/omap4iss/Kconfig" > > +source "drivers/staging/media/tegra-vde/Kconfig" > + > endif > diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile > index f25327163c67..59a47f69884f 100644 > --- a/drivers/staging/media/Makefile > +++ b/drivers/staging/media/Makefile > @@ -5,3 +5,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx/ > obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/ > obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ > obj-$(CONFIG_INTEL_ATOMISP) += atomisp/ > +obj-$(CONFIG_TEGRA_VDE)+= tegra-vde/ > diff --git a/drivers/staging/media/tegra-vde/Kconfig > b/drivers/staging/media/tegra-vde/Kconfig > new file mode 100644 > index ..ec3ebdaa > --- /dev/null > +++ b/drivers/staging/media/tegra-vde/Kconfig > @@ -0,0 +1,7 @@ > +config TEGRA_VDE > + tristate "NVIDIA Tegra Video Decoder Engine driver" > + depends on ARCH_TEGRA || COMPILE_TEST > + select SRAM > + help > + Say Y here to enable support for the NVIDIA Tegra video decoder > + driver. > diff --git a/drivers/staging/media/tegra-vde/Makefile > b/drivers/staging/media/tegra-vde/Makefile > new file mode 100644 > index ..444c1d62daa1 > --- /dev/null > +++ b/drivers/staging/media/tegra-vde/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_TEGRA_VDE)+= tegra-vde.o > diff --git a/drivers/staging/media/tegra-vde/TODO > b/drivers/staging/media/tegra-vde/TODO > new file mode 100644 > index ..31aaa3e66d80 > --- /dev/null > +++ b/drivers/staging/media/tegra-vde/TODO > @@ -0,0 +1,4 @@ > +TODO: > + - Implement V4L2 API once it gains support for stateless decoders. > + > +Contact: Dmitry Osipenko The API for H264 stateless decoding is ready. See https://lkml.org/lkml/2020/11/18/795. One minor comment below. > diff --git a/drivers/staging/media/tegra-vde/uapi.h > b/drivers/staging/media/tegra-vde/uapi.h > new file mode 100644 > index ..a50c7bcae057 > --- /dev/null > +++ b/drivers/staging/media/tegra-vde/uapi.h > @@ -0,0 +1,78 @@ > +/* > + * Copyright (C) 2016-2017 Dmitry Osipenko > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + */ > + > +#ifndef _UAPI_TEGRA_VDE_H_ > +#define _UAPI_TEGRA_VDE_H_ > + > +#include > +#include > + > +#define FLAG_B_FRAME (1 << 0) > +#define FLAG_REFERENCE (1 << 1) > + > +struct tegra_vde_h264_frame { > + __s32 y_fd; > + __s32 cb_fd; > + __s32 cr_fd; > + __s32 aux_fd; > + __u32 y_offset; > + __u32 cb_offset; > + __u32 cr_offset; > + __u32 aux_offset; > + __u32 frame_num; > + __u32 flags; > + > + __u32 reserved; > +} __attribute__((packed)); > + > +struct
Re: [PATCH 086/141] hwmon: (corsair-cpro) Fix fall-through warnings for Clang
On Sat, Nov 21, 2020 at 12:00:30PM -0800, Joe Perches wrote: > On Sat, 2020-11-21 at 10:50 -0800, Guenter Roeck wrote: > > On Fri, Nov 20, 2020 at 12:36:04PM -0600, Gustavo A. R. Silva wrote: > > > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning > > > by explicitly adding a break statement instead of letting the code fall > > > through to the next case. > > > > > > Link: https://github.com/KSPP/linux/issues/115 > > > Signed-off-by: Gustavo A. R. Silva > > > > Acked-by: Guenter Roeck > [] > > > diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c > [] > > > @@ -310,6 +310,7 @@ static int ccp_write(struct device *dev, enum > > > hwmon_sensor_types type, > > > default: > > > break; > > > } > > > + break; > > The > default: > break; > > bit above (but not below as it's a switch on an enum) could also be deleted > no? > I prefer to keep it the way it is to indicate that the default case was not forgotten (unless that is against coding style). Guenter > > > default: > > > break; > > > } > > --- > drivers/hwmon/corsair-cpro.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c > index 591929ec217a..580173fca0f6 100644 > --- a/drivers/hwmon/corsair-cpro.c > +++ b/drivers/hwmon/corsair-cpro.c > @@ -299,17 +299,14 @@ static int ccp_write(struct device *dev, enum > hwmon_sensor_types type, > switch (attr) { > case hwmon_pwm_input: > return set_pwm(ccp, channel, val); > - default: > - break; > } > break; > case hwmon_fan: > switch (attr) { > case hwmon_fan_target: > return set_target(ccp, channel, val); > - default: > - break; > } > + break; > default: > break; > } > >
Re: [PATCH v3 07/13] media: controls: Validate H264 stateless controls
On Fri, 2020-11-20 at 10:30 +0100, Hans Verkuil wrote: > On 18/11/2020 19:46, Ezequiel Garcia wrote: > > Check that all the fields that correspond or are related > > to a H264 specification syntax element have legal values. > > > > Signed-off-by: Ezequiel Garcia > > --- > > drivers/media/v4l2-core/v4l2-ctrls.c | 83 > > 1 file changed, 83 insertions(+) > > [..] > > case V4L2_CTRL_TYPE_H264_DECODE_PARAMS: > > p_h264_dec_params = p; > > > > + if (p_h264_dec_params->nal_ref_idc > 3) > > + return -EINVAL; > > for (i = 0; i < V4L2_H264_NUM_DPB_ENTRIES; i++) { > > struct v4l2_h264_dpb_entry *dpb_entry = > > _h264_dec_params->dpb[i]; > > > > General question: I don't see anything in std_init_compound() for these > controls. Is initializing these compound controls to 0 enough to make them > pass std_validate_compound()? It probably is, otherwise you'd see errors > in the compliance test, I guess. > guess. > Indeed. You can see all the checks are for fields to not exceed some maximum value. This is common in H264/HEVC: you'll see spread of _minusN syntax. This is so to make zero-valued syntax common, which in turns creates more redundancy, and make headers more compressed. Thanks, Ezequiel
Re: [PATCH] mtd: parser: cmdline: Support MTD names containing one or more colons
On Wednesday, 29 April 2020 18:53:47 CET Ronald G. Minnich wrote: > From: Boris Brezillon > > Looks like some drivers define MTD names with a colon in it, thus > making mtdpart= parsing impossible. Let's fix the parser to gracefully > handle that case: the last ':' in a partition definition sequence is > considered instead of the first one. > > Signed-off-by: Boris Brezillon > Signed-off-by: Ron Minnich > Tested-by: Ron Minnich > --- > drivers/mtd/parsers/cmdlinepart.c | 23 --- > 1 file changed, 20 insertions(+), 3 deletions(-) This change broke OpenWrt booting on some IPQ40xx devices. Here for example the parts of the cmdline which the u-boot on an OpenMesh A62 sets automatically: root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b(kernel),0x00c8(rootfs),15552k(inactive) rootfsname=rootfs rootwait This is then parsed by newpart as: KEYS),0x002b(kernel),0x00c8(rootfs),15552k(inactive) And of course, this results in: mtd: partition has size 0 [...] /dev/root: Can't open blockdev VFS: Cannot open root device "31:11" or unknown-block(31,11): error -6 Please append a correct "root=" boot option; here are the available partitions: 1f00 32768 mtdblock0 (driver?) Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,11) CPU1: stopping CPU: 1 PID: 0 Comm: This affects OpenWrt since the commit d6a9a92e3217 ("kernel: bump 5.4 to 5.4.69") because this change was backported to Linux v5.4.69. Reverting this change fixes the problem for me. And if I see it correctly, this is also affecting the kernel (4.14) for the OpenWrt 18.06.x + 19.07.x branch because it can also be found in there as part of the v4.14.200 release. Another workaround is to replace the first "(" with a NULL too. See the attached patch for the one which I used to fix the OpenWrt bootup. Kind regards, SvenFrom: Sven Eckelmann Date: Sun, 22 Nov 2020 00:48:33 +0100 Subject: [PATCH RFC] mtd: parser: cmdline: Fix parsing of part-names with colons Some devices (especially QCA ones) are already using hardcoded partition names with colons in it. The OpenMesh A62 for example provides following mtd relevant information via cmdline: root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b(kernel),0x00c8(rootfs),15552k(inactive) rootfsname=rootfs rootwait The change to split only on the last colon between mtd-id and partitions will cause newpart to see following string for the first partition: KEYS),0x002b(kernel),0x00c8(rootfs),15552k(inactive) Such a partition list cannot be parsed and thus the device fails to boot. Avoid this behavior by making sure that the start of the first part-name ("(") will also be the last byte the colon search algorithm is using for its mtd-id search. Fixes: eb13fa022741 ("mtd: parser: cmdline: Support MTD names containing one or more colons") Signed-off-by: Sven Eckelmann diff --git a/drivers/mtd/parsers/cmdlinepart.c b/drivers/mtd/parsers/cmdlinepart.c index 0625b25620ca766318ea4646a6e3761ff4d3a4cc..22881ea4c132ea5a5ba7aebd025d91bf1cd023af 100644 --- a/drivers/mtd/parsers/cmdlinepart.c +++ b/drivers/mtd/parsers/cmdlinepart.c @@ -218,7 +218,7 @@ static int mtdpart_setup_real(char *s) struct cmdline_mtd_partition *this_mtd; struct mtd_partition *parts; int mtd_id_len, num_parts; - char *p, *mtd_id, *semicol; + char *p, *mtd_id, *semicol, *open_parenth; /* * Replace the first ';' by a NULL char so strrchr can work @@ -228,6 +228,13 @@ static int mtdpart_setup_real(char *s) if (semicol) *semicol = '\0'; + /* make sure that part-names with ":" will not be handled as + * part of the mtd-id with an ":" + */ + open_parenth = strchr(s, '('); + if (open_parenth) + *open_parenth = '\0'; + mtd_id = s; /* @@ -237,6 +244,10 @@ static int mtdpart_setup_real(char *s) */ p = strrchr(s, ':'); + /* Restore the '(' now. */ + if (open_parenth) + *open_parenth = '('; + /* Restore the ';' now. */ if (semicol) *semicol = ';'; signature.asc Description: This is a digitally signed message part.
Re: [PATCH v2] drivers: fpga: Specify HAS_IOMEM dependency for FPGA_DFL
On Fri, Nov 20, 2020 at 03:46:48PM -0800, David Gow wrote: > Because dfl.c uses the 'devm_ioremap', 'devm_iounmap', > 'devm_ioremap_resource', and 'devm_platform_ioremap_resource' > functions, it should depend on HAS_IOMEM. > > This fixes make allyesconfig under UML (ARCH=um), which doesn't provide > HAS_IOMEM. > > Fixes: 89eb35e810a8 ("fpga: dfl: map feature mmio resources in their own > feature drivers") > Signed-off-by: David Gow > --- > > Changes since v1: > ( > https://lore.kernel.org/linux-fpga/20201119082209.3598354-1-david...@google.com/ > ) > - Add Fixes tag > > drivers/fpga/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index 7cd5a29fc437..5645226ca3ce 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -142,6 +142,7 @@ config FPGA_DFL > tristate "FPGA Device Feature List (DFL) support" > select FPGA_BRIDGE > select FPGA_REGION > + depends on HAS_IOMEM > help > Device Feature List (DFL) defines a feature list structure that > creates a linked list of feature headers within the MMIO space > -- > 2.29.2.454.gaff20da3a2-goog > Applied to for-5.10, I fixed up your commit message (dropped the drivers:) - Moritz
Re: [PATCH] octeontx2-af: Add support for RSS hashing based on Transport protocol field
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Fri, 20 Nov 2020 15:09:06 +0530 you wrote: > Add support to choose RSS flow key algorithm with IPv4 transport protocol > field included in hashing input data. This will be enabled by default. > There-by enabling 3/5 tuple hash > > Signed-off-by: Sunil Kovvuri Goutham > Signed-off-by: George Cherian > > [...] Here is the summary with links: - octeontx2-af: Add support for RSS hashing based on Transport protocol field https://git.kernel.org/netdev/net-next/c/f9e425e99b07 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH v7 1/5] dma-buf: system_heap: Rework system heap to use sgtables instead of pagelists
In preparation for some patches to optmize the system heap code, rework the dmabuf exporter to utilize sgtables rather then pageslists for tracking the associated pages. This will allow for large order page allocations, as well as more efficient page pooling. In doing so, the system heap stops using the heap-helpers logic which sadly is not quite as generic as I was hoping it to be, so this patch adds heap specific implementations of the dma_buf_ops function handlers. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Reviewed-by: Brian Starkey Signed-off-by: John Stultz --- v2: * Fix locking issue and an unused return value Reported-by: kernel test robot Julia Lawall * Make system_heap_buf_ops static Reported-by: kernel test robot v3: * Use the new sgtable mapping functions, as Suggested-by: Daniel Mentz v4: * Make sys_heap static (indirectly) Reported-by: kernel test robot * Spelling fix suggested by BrianS v6: * Fixups against drm-misc-next, from Sumit v7: * Redo the incorrect dma-buf-map adaptation in the last revision --- drivers/dma-buf/heaps/system_heap.c | 349 1 file changed, 303 insertions(+), 46 deletions(-) diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 0bf688e3c023..b2d02f50f9ed 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -3,7 +3,11 @@ * DMABUF System heap exporter * * Copyright (C) 2011 Google, Inc. - * Copyright (C) 2019 Linaro Ltd. + * Copyright (C) 2019, 2020 Linaro Ltd. + * + * Portions based off of Andrew Davis' SRAM heap: + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ + * Andrew F. Davis */ #include @@ -15,72 +19,326 @@ #include #include #include -#include -#include +#include + +static struct dma_heap *sys_heap; -#include "heap-helpers.h" +struct system_heap_buffer { + struct dma_heap *heap; + struct list_head attachments; + struct mutex lock; + unsigned long len; + struct sg_table sg_table; + int vmap_cnt; + void *vaddr; +}; -struct dma_heap *sys_heap; +struct dma_heap_attachment { + struct device *dev; + struct sg_table *table; + struct list_head list; +}; -static void system_heap_free(struct heap_helper_buffer *buffer) +static struct sg_table *dup_sg_table(struct sg_table *table) { - pgoff_t pg; + struct sg_table *new_table; + int ret, i; + struct scatterlist *sg, *new_sg; + + new_table = kzalloc(sizeof(*new_table), GFP_KERNEL); + if (!new_table) + return ERR_PTR(-ENOMEM); + + ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL); + if (ret) { + kfree(new_table); + return ERR_PTR(-ENOMEM); + } + + new_sg = new_table->sgl; + for_each_sgtable_sg(table, sg, i) { + sg_set_page(new_sg, sg_page(sg), sg->length, sg->offset); + new_sg = sg_next(new_sg); + } + + return new_table; +} + +static int system_heap_attach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) +{ + struct system_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a; + struct sg_table *table; + + a = kzalloc(sizeof(*a), GFP_KERNEL); + if (!a) + return -ENOMEM; + + table = dup_sg_table(>sg_table); + if (IS_ERR(table)) { + kfree(a); + return -ENOMEM; + } + + a->table = table; + a->dev = attachment->dev; + INIT_LIST_HEAD(>list); + + attachment->priv = a; + + mutex_lock(>lock); + list_add(>list, >attachments); + mutex_unlock(>lock); + + return 0; +} + +static void system_heap_detach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) +{ + struct system_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a = attachment->priv; + + mutex_lock(>lock); + list_del(>list); + mutex_unlock(>lock); + + sg_free_table(a->table); + kfree(a->table); + kfree(a); +} + +static struct sg_table *system_heap_map_dma_buf(struct dma_buf_attachment *attachment, + enum dma_data_direction direction) +{ + struct dma_heap_attachment *a = attachment->priv; + struct sg_table *table = a->table; + int ret; + + ret = dma_map_sgtable(attachment->dev, table, direction, 0); + if (ret) + return ERR_PTR(ret); + + return table; +} + +static void
[PATCH v7 5/5] dma-buf: system_heap: Allocate higher order pages if available
While the system heap can return non-contiguous pages, try to allocate larger order pages if possible. This will allow slight performance gains and make implementing page pooling easier. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Reviewed-by: Brian Starkey Signed-off-by: John Stultz --- v3: * Use page_size() rather then opencoding it v5: * Add comment explaining order size rational --- drivers/dma-buf/heaps/system_heap.c | 89 +++-- 1 file changed, 71 insertions(+), 18 deletions(-) diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 32b17a5c8079..17e0e9a68baf 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -40,6 +40,20 @@ struct dma_heap_attachment { bool mapped; }; +#define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \ + | __GFP_NORETRY) & ~__GFP_RECLAIM) \ + | __GFP_COMP) +#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO | __GFP_COMP) +static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, LOW_ORDER_GFP}; +/* + * The selection of the orders used for allocation (1MB, 64K, 4K) is designed + * to match with the sizes often found in IOMMUs. Using order 4 pages instead + * of order 0 pages can significantly improve the performance of many IOMMUs + * by reducing TLB pressure and time spent updating page tables. + */ +static const unsigned int orders[] = {8, 4, 0}; +#define NUM_ORDERS ARRAY_SIZE(orders) + static struct sg_table *dup_sg_table(struct sg_table *table) { struct sg_table *new_table; @@ -275,8 +289,11 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf) int i; table = >sg_table; - for_each_sgtable_sg(table, sg, i) - __free_page(sg_page(sg)); + for_each_sg(table->sgl, sg, table->nents, i) { + struct page *page = sg_page(sg); + + __free_pages(page, compound_order(page)); + } sg_free_table(table); kfree(buffer); } @@ -294,6 +311,26 @@ static const struct dma_buf_ops system_heap_buf_ops = { .release = system_heap_dma_buf_release, }; +static struct page *alloc_largest_available(unsigned long size, + unsigned int max_order) +{ + struct page *page; + int i; + + for (i = 0; i < NUM_ORDERS; i++) { + if (size < (PAGE_SIZE << orders[i])) + continue; + if (max_order < orders[i]) + continue; + + page = alloc_pages(order_flags[i], orders[i]); + if (!page) + continue; + return page; + } + return NULL; +} + static int system_heap_allocate(struct dma_heap *heap, unsigned long len, unsigned long fd_flags, @@ -301,11 +338,13 @@ static int system_heap_allocate(struct dma_heap *heap, { struct system_heap_buffer *buffer; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + unsigned long size_remaining = len; + unsigned int max_order = orders[0]; struct dma_buf *dmabuf; struct sg_table *table; struct scatterlist *sg; - pgoff_t pagecount; - pgoff_t pg; + struct list_head pages; + struct page *page, *tmp_page; int i, ret = -ENOMEM; buffer = kzalloc(sizeof(*buffer), GFP_KERNEL); @@ -317,25 +356,35 @@ static int system_heap_allocate(struct dma_heap *heap, buffer->heap = heap; buffer->len = len; - table = >sg_table; - pagecount = len / PAGE_SIZE; - if (sg_alloc_table(table, pagecount, GFP_KERNEL)) - goto free_buffer; - - sg = table->sgl; - for (pg = 0; pg < pagecount; pg++) { - struct page *page; + INIT_LIST_HEAD(); + i = 0; + while (size_remaining > 0) { /* * Avoid trying to allocate memory if the process * has been killed by SIGKILL */ if (fatal_signal_pending(current)) - goto free_pages; - page = alloc_page(GFP_KERNEL | __GFP_ZERO); + goto free_buffer; + + page = alloc_largest_available(size_remaining, max_order); if (!page) - goto free_pages; + goto free_buffer; + + list_add_tail(>lru, ); + size_remaining -= page_size(page); + max_order = compound_order(page); + i++; + } + + table =
[PATCH v7 2/5] dma-buf: heaps: Move heap-helper logic into the cma_heap implementation
Since the heap-helpers logic ended up not being as generic as hoped, move the heap-helpers dma_buf_ops implementations into the cma_heap directly. This will allow us to remove the heap_helpers code in a following patch. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Reviewed-by: Brian Starkey Signed-off-by: John Stultz --- v2: * Fix unused return value and locking issue Reported-by: kernel test robot Julia Lawall * Make cma_heap_buf_ops static suggested by kernel test robot * Fix uninitialized return in cma Reported-by: kernel test robot * Minor cleanups v3: * Use the new sgtable mapping functions, as Suggested-by: Daniel Mentz v4: * Spelling fix suggested by BrianS v6: * Fixups against drm-misc-next v7: * Redo the incorrect dma-buf-map adaptation in the last revision --- drivers/dma-buf/heaps/cma_heap.c | 319 ++- 1 file changed, 270 insertions(+), 49 deletions(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index e55384dc115b..05aaa4f29397 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -2,76 +2,295 @@ /* * DMABUF CMA heap exporter * - * Copyright (C) 2012, 2019 Linaro Ltd. + * Copyright (C) 2012, 2019, 2020 Linaro Ltd. * Author: for ST-Ericsson. + * + * Also utilizing parts of Andrew Davis' SRAM heap: + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ + * Andrew F. Davis */ - #include -#include #include #include #include #include -#include #include +#include +#include #include -#include #include -#include +#include -#include "heap-helpers.h" struct cma_heap { struct dma_heap *heap; struct cma *cma; }; -static void cma_heap_free(struct heap_helper_buffer *buffer) +struct cma_heap_buffer { + struct cma_heap *heap; + struct list_head attachments; + struct mutex lock; + unsigned long len; + struct page *cma_pages; + struct page **pages; + pgoff_t pagecount; + int vmap_cnt; + void *vaddr; +}; + +struct dma_heap_attachment { + struct device *dev; + struct sg_table table; + struct list_head list; +}; + +static int cma_heap_attach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) { - struct cma_heap *cma_heap = dma_heap_get_drvdata(buffer->heap); - unsigned long nr_pages = buffer->pagecount; - struct page *cma_pages = buffer->priv_virt; + struct cma_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a; + int ret; - /* free page list */ - kfree(buffer->pages); - /* release memory */ - cma_release(cma_heap->cma, cma_pages, nr_pages); + a = kzalloc(sizeof(*a), GFP_KERNEL); + if (!a) + return -ENOMEM; + + ret = sg_alloc_table_from_pages(>table, buffer->pages, + buffer->pagecount, 0, + buffer->pagecount << PAGE_SHIFT, + GFP_KERNEL); + if (ret) { + kfree(a); + return ret; + } + + a->dev = attachment->dev; + INIT_LIST_HEAD(>list); + + attachment->priv = a; + + mutex_lock(>lock); + list_add(>list, >attachments); + mutex_unlock(>lock); + + return 0; +} + +static void cma_heap_detach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) +{ + struct cma_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a = attachment->priv; + + mutex_lock(>lock); + list_del(>list); + mutex_unlock(>lock); + + sg_free_table(>table); + kfree(a); +} + +static struct sg_table *cma_heap_map_dma_buf(struct dma_buf_attachment *attachment, +enum dma_data_direction direction) +{ + struct dma_heap_attachment *a = attachment->priv; + struct sg_table *table = >table; + int ret; + + ret = dma_map_sgtable(attachment->dev, table, direction, 0); + if (ret) + return ERR_PTR(-ENOMEM); + return table; +} + +static void cma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, + struct sg_table *table, + enum dma_data_direction direction) +{ + dma_unmap_sgtable(attachment->dev, table, direction, 0); +} + +static int cma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, +enum dma_data_direction direction) +{ + struct cma_heap_buffer
[PATCH v7 3/5] dma-buf: heaps: Remove heap-helpers code
The heap-helpers code was not as generic as initially hoped and it is now not being used, so remove it from the tree. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Reviewed-by: Brian Starkey Signed-off-by: John Stultz --- v6: Rebased onto drm-misc-next --- drivers/dma-buf/heaps/Makefile | 1 - drivers/dma-buf/heaps/heap-helpers.c | 274 --- drivers/dma-buf/heaps/heap-helpers.h | 53 -- 3 files changed, 328 deletions(-) delete mode 100644 drivers/dma-buf/heaps/heap-helpers.c delete mode 100644 drivers/dma-buf/heaps/heap-helpers.h diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile index 6e54cdec3da0..974467791032 100644 --- a/drivers/dma-buf/heaps/Makefile +++ b/drivers/dma-buf/heaps/Makefile @@ -1,4 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y += heap-helpers.o obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c deleted file mode 100644 index fcf4ce3e2cbb.. --- a/drivers/dma-buf/heaps/heap-helpers.c +++ /dev/null @@ -1,274 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "heap-helpers.h" - -void init_heap_helper_buffer(struct heap_helper_buffer *buffer, -void (*free)(struct heap_helper_buffer *)) -{ - buffer->priv_virt = NULL; - mutex_init(>lock); - buffer->vmap_cnt = 0; - buffer->vaddr = NULL; - buffer->pagecount = 0; - buffer->pages = NULL; - INIT_LIST_HEAD(>attachments); - buffer->free = free; -} - -struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer, - int fd_flags) -{ - DEFINE_DMA_BUF_EXPORT_INFO(exp_info); - - exp_info.ops = _helper_ops; - exp_info.size = buffer->size; - exp_info.flags = fd_flags; - exp_info.priv = buffer; - - return dma_buf_export(_info); -} - -static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer) -{ - void *vaddr; - - vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL); - if (!vaddr) - return ERR_PTR(-ENOMEM); - - return vaddr; -} - -static void dma_heap_buffer_destroy(struct heap_helper_buffer *buffer) -{ - if (buffer->vmap_cnt > 0) { - WARN(1, "%s: buffer still mapped in the kernel\n", __func__); - vunmap(buffer->vaddr); - } - - buffer->free(buffer); -} - -static void *dma_heap_buffer_vmap_get(struct heap_helper_buffer *buffer) -{ - void *vaddr; - - if (buffer->vmap_cnt) { - buffer->vmap_cnt++; - return buffer->vaddr; - } - vaddr = dma_heap_map_kernel(buffer); - if (IS_ERR(vaddr)) - return vaddr; - buffer->vaddr = vaddr; - buffer->vmap_cnt++; - return vaddr; -} - -static void dma_heap_buffer_vmap_put(struct heap_helper_buffer *buffer) -{ - if (!--buffer->vmap_cnt) { - vunmap(buffer->vaddr); - buffer->vaddr = NULL; - } -} - -struct dma_heaps_attachment { - struct device *dev; - struct sg_table table; - struct list_head list; -}; - -static int dma_heap_attach(struct dma_buf *dmabuf, - struct dma_buf_attachment *attachment) -{ - struct dma_heaps_attachment *a; - struct heap_helper_buffer *buffer = dmabuf->priv; - int ret; - - a = kzalloc(sizeof(*a), GFP_KERNEL); - if (!a) - return -ENOMEM; - - ret = sg_alloc_table_from_pages(>table, buffer->pages, - buffer->pagecount, 0, - buffer->pagecount << PAGE_SHIFT, - GFP_KERNEL); - if (ret) { - kfree(a); - return ret; - } - - a->dev = attachment->dev; - INIT_LIST_HEAD(>list); - - attachment->priv = a; - - mutex_lock(>lock); - list_add(>list, >attachments); - mutex_unlock(>lock); - - return 0; -} - -static void dma_heap_detach(struct dma_buf *dmabuf, - struct dma_buf_attachment *attachment) -{ - struct dma_heaps_attachment *a = attachment->priv; - struct heap_helper_buffer *buffer = dmabuf->priv; - - mutex_lock(>lock); - list_del(>list); - mutex_unlock(>lock); - - sg_free_table(>table); - kfree(a); -} - -static -struct
[PATCH v7 4/5] dma-buf: heaps: Skip sync if not mapped
This patch is basically a port of Ørjan Eide's similar patch for ION https://lore.kernel.org/lkml/20200414134629.54567-1-orjan.e...@arm.com/ Only sync the sg-list of dma-buf heap attachment when the attachment is actually mapped on the device. dma-bufs may be synced at any time. It can be reached from user space via DMA_BUF_IOCTL_SYNC, so there are no guarantees from callers on when syncs may be attempted, and dma_buf_end_cpu_access() and dma_buf_begin_cpu_access() may not be paired. Since the sg_list's dma_address isn't set up until the buffer is used on the device, and dma_map_sg() is called on it, the dma_address will be NULL if sync is attempted on the dma-buf before it's mapped on a device. Before v5.0 (commit 55897af63091 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code")) this was a problem as the dma-api (at least the swiotlb_dma_ops on arm64) would use the potentially invalid dma_address. How that failed depended on how the device handled physical address 0. If 0 was a valid address to physical ram, that page would get flushed a lot, while the actual pages in the buffer would not get synced correctly. While if 0 is an invalid physical address it may cause a fault and trigger a crash. In v5.0 this was incidentally fixed by commit 55897af63091 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code"), as this moved the dma-api to use the page pointer in the sg_list, and (for Ion buffers at least) this will always be valid if the sg_list exists at all. But, this issue is re-introduced in v5.3 with commit 449fa54d6815 ("dma-direct: correct the physical addr in dma_direct_sync_sg_for_cpu/device") moves the dma-api back to the old behaviour and picks the dma_address that may be invalid. dma-buf core doesn't ensure that the buffer is mapped on the device, and thus have a valid sg_list, before calling the exporter's begin_cpu_access. Logic and commit message originally by: Ørjan Eide Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Reviewed-by: Brian Starkey Signed-off-by: John Stultz --- drivers/dma-buf/heaps/cma_heap.c| 10 ++ drivers/dma-buf/heaps/system_heap.c | 10 ++ 2 files changed, 20 insertions(+) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 05aaa4f29397..5e7c3436310c 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -43,6 +43,7 @@ struct dma_heap_attachment { struct device *dev; struct sg_table table; struct list_head list; + bool mapped; }; static int cma_heap_attach(struct dma_buf *dmabuf, @@ -67,6 +68,7 @@ static int cma_heap_attach(struct dma_buf *dmabuf, a->dev = attachment->dev; INIT_LIST_HEAD(>list); + a->mapped = false; attachment->priv = a; @@ -101,6 +103,7 @@ static struct sg_table *cma_heap_map_dma_buf(struct dma_buf_attachment *attachme ret = dma_map_sgtable(attachment->dev, table, direction, 0); if (ret) return ERR_PTR(-ENOMEM); + a->mapped = true; return table; } @@ -108,6 +111,9 @@ static void cma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { + struct dma_heap_attachment *a = attachment->priv; + + a->mapped = false; dma_unmap_sgtable(attachment->dev, table, direction, 0); } @@ -122,6 +128,8 @@ static int cma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, mutex_lock(>lock); list_for_each_entry(a, >attachments, list) { + if (!a->mapped) + continue; dma_sync_sgtable_for_cpu(a->dev, >table, direction); } mutex_unlock(>lock); @@ -140,6 +148,8 @@ static int cma_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf, mutex_lock(>lock); list_for_each_entry(a, >attachments, list) { + if (!a->mapped) + continue; dma_sync_sgtable_for_device(a->dev, >table, direction); } mutex_unlock(>lock); diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index b2d02f50f9ed..32b17a5c8079 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -37,6 +37,7 @@ struct dma_heap_attachment { struct device *dev; struct sg_table *table; struct list_head list; + bool mapped; }; static struct sg_table *dup_sg_table(struct sg_table *table) @@ -84,6 +85,7 @@ static int system_heap_attach(struct dma_buf *dmabuf, a->table = table;
[PATCH v7 0/5] dma-buf: Code rework and performance improvements for system heap
Hey All, So Sumit noted a flub I made in adapting the last series to the new dma-buf-map code that is in drm-misc-next. Thus I wanted to send this (hopefully) last revision of my patch series of performance optimizations to the dma-buf system heap, once again against drm-misc-next. This series reworks the system heap to use sgtables, and then consolidates the pagelist method from the heap-helpers into the CMA heap. After which the heap-helpers logic is removed (as it is unused). I'd still like to find a better way to avoid some of the logic duplication in implementing the entire dma_buf_ops handlers per heap. But unfortunately that code is tied somewhat to how the buffer's memory is tracked. As more heaps show up I think we'll have a better idea how to best share code, so for now I think this is ok. After this, the series introduces an optimization that Ørjan Eide implemented for ION that avoids calling sync on attachments that don't have a mapping. Finally, an optimization to use larger order pages for the system heap. This change brings us closer to the current performance of the ION allocation code (though there still is a gap due to ION using a mix of deferred-freeing and page pools, I'll be looking at integrating those eventually). This version of the series does not include the system-uncached heap as Daniel wanted further demonstration that it is useful with devices that use the mesa stack. I'm working on such a justification but I don't want to hold up these rework patches in the meantime. thanks -john New in v7: * Fixed the incorrect adaptation to the dma-buf-map usage Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org John Stultz (5): dma-buf: system_heap: Rework system heap to use sgtables instead of pagelists dma-buf: heaps: Move heap-helper logic into the cma_heap implementation dma-buf: heaps: Remove heap-helpers code dma-buf: heaps: Skip sync if not mapped dma-buf: system_heap: Allocate higher order pages if available drivers/dma-buf/heaps/Makefile | 1 - drivers/dma-buf/heaps/cma_heap.c | 329 + drivers/dma-buf/heaps/heap-helpers.c | 274 -- drivers/dma-buf/heaps/heap-helpers.h | 53 drivers/dma-buf/heaps/system_heap.c | 414 --- 5 files changed, 647 insertions(+), 424 deletions(-) delete mode 100644 drivers/dma-buf/heaps/heap-helpers.c delete mode 100644 drivers/dma-buf/heaps/heap-helpers.h -- 2.17.1
[PATCH v2] m68k: Fix WARNING splat in pmac_zilog driver
Don't add platform resources that won't be used. This avoids a recently-added warning from the driver core, that can show up on a multi-platform kernel when !MACH_IS_MAC. [ cut here ] WARNING: CPU: 0 PID: 0 at drivers/base/platform.c:224 platform_get_irq_optional+0x8e/0xce 0 is an invalid IRQ number Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 5.9.0-multi #1 Stack from 004b3f04: 004b3f04 00462c2f 00462c2f 004b3f20 0002e128 004754db 004b6ad4 004b3f4c 0002e19c 004754f7 00e0 00285ba0 0009 004b3f44 004754db 004b3f64 004b3f74 00285ba0 004754f7 00e0 0009 004754db 004fdf0c 005269e2 004fdf0c 004b3f88 00285cae 004b6964 004fdf0c 004b3fac 0051cc68 004b6964 004b6964 0200 0051cc3e 0023c18a 004b3fc0 0051cd8a 004fdf0c 0002 0052b43c 004b3fc8 Call Trace: [<0002e128>] __warn+0xa6/0xd6 [<0002e19c>] warn_slowpath_fmt+0x44/0x76 [<00285ba0>] platform_get_irq_optional+0x8e/0xce [<00285ba0>] platform_get_irq_optional+0x8e/0xce [<00285cae>] platform_get_irq+0x12/0x4c [<0051cc68>] pmz_init_port+0x2a/0xa6 [<0051cc3e>] pmz_init_port+0x0/0xa6 [<0023c18a>] strlen+0x0/0x22 [<0051cd8a>] pmz_probe+0x34/0x88 [<0051cde6>] pmz_console_init+0x8/0x28 [<00511776>] console_init+0x1e/0x28 [<0005a3bc>] printk+0x0/0x16 [<0050a8a6>] start_kernel+0x368/0x4ce [<005094f8>] _sinittext+0x4f8/0xc48 random: get_random_bytes called from print_oops_end_marker+0x56/0x80 with crng_init=0 ---[ end trace 392d8e82eed68d6c ]--- Commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid"), which introduced the WARNING, suggests that testing for irq == 0 is undesirable. Instead of that comparison, just test for resource existence. Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Joshua Thompson Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: sta...@vger.kernel.org # v5.8+ References: commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid") Reported-by: Laurent Vivier Signed-off-by: Finn Thain --- Changed since v1: - Add a comment to explain the need for the global structs. - Expand the commit log to better explain the intention of the patch. --- arch/m68k/mac/config.c | 17 + drivers/tty/serial/pmac_zilog.c | 14 +- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c index 0ac53d87493c..2bea1799b8de 100644 --- a/arch/m68k/mac/config.c +++ b/arch/m68k/mac/config.c @@ -777,16 +777,12 @@ static struct resource scc_b_rsrcs[] = { struct platform_device scc_a_pdev = { .name = "scc", .id = 0, - .num_resources = ARRAY_SIZE(scc_a_rsrcs), - .resource = scc_a_rsrcs, }; EXPORT_SYMBOL(scc_a_pdev); struct platform_device scc_b_pdev = { .name = "scc", .id = 1, - .num_resources = ARRAY_SIZE(scc_b_rsrcs), - .resource = scc_b_rsrcs, }; EXPORT_SYMBOL(scc_b_pdev); @@ -813,10 +809,15 @@ static void __init mac_identify(void) /* Set up serial port resources for the console initcall. */ - scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2; - scc_a_rsrcs[0].end = scc_a_rsrcs[0].start; - scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase; - scc_b_rsrcs[0].end = scc_b_rsrcs[0].start; + scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2; + scc_a_rsrcs[0].end = scc_a_rsrcs[0].start; + scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs); + scc_a_pdev.resource = scc_a_rsrcs; + + scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase; + scc_b_rsrcs[0].end = scc_b_rsrcs[0].start; + scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs); + scc_b_pdev.resource = scc_b_rsrcs; switch (macintosh_config->scc_type) { case MAC_SCC_PSC: diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c index 96e7aa479961..216b75ef5048 100644 --- a/drivers/tty/serial/pmac_zilog.c +++ b/drivers/tty/serial/pmac_zilog.c @@ -1693,22 +1693,26 @@ static int __init pmz_probe(void) #else +/* On PCI PowerMacs, pmz_probe() does an explicit search of the OpenFirmware + * tree to obtain the device_nodes needed to start the console before the + * macio driver. On Macs without OpenFirmware, global platform_devices take + * the place of those device_nodes. + */ extern struct platform_device scc_a_pdev, scc_b_pdev; static int __init pmz_init_port(struct uart_pmac_port *uap) { - struct resource *r_ports; - int irq; + struct resource *r_ports, *r_irq; r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0); - irq = platform_get_irq(uap->pdev, 0); - if (!r_ports || irq <= 0) + r_irq = platform_get_resource(uap->pdev,
Re: [PATCH] rtc: mxc{,_v2}: enable COMPILE_TEST
On Sat, Nov 21, 2020 at 7:45 PM Alexandre Belloni wrote: > > Extend code coverage for the rtc-mxc and rtc-mxc-v2 drivers. > > Signed-off-by: Alexandre Belloni Reviewed-by: Fabio Estevam
[tip:ras/core] BUILD SUCCESS 4a24d80b8c3e9f89d6a6a7b89bd057c463b638d3
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git ras/core branch HEAD: 4a24d80b8c3e9f89d6a6a7b89bd057c463b638d3 x86/mce, cper: Pass x86 CPER through the MCA handling chain elapsed time: 720m configs tested: 92 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig arm efm32_defconfig sh rsk7203_defconfig sh rsk7269_defconfig sh sh03_defconfig powerpcgamecube_defconfig arcvdk_hs38_defconfig sh sh7724_generic_defconfig powerpc mpc8540_ads_defconfig riscv rv32_defconfig arm hackkit_defconfig mips fuloong2e_defconfig arm simpad_defconfig armtrizeps4_defconfig arcnsimosci_defconfig arm cns3420vb_defconfig arm am200epdkit_defconfig alpha defconfig mips ip32_defconfig powerpc ppa8548_defconfig sh se7780_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a004-20201121 i386 randconfig-a003-20201121 i386 randconfig-a002-20201121 i386 randconfig-a005-20201121 i386 randconfig-a001-20201121 i386 randconfig-a006-20201121 x86_64 randconfig-a015-20201121 x86_64 randconfig-a011-20201121 x86_64 randconfig-a014-20201121 x86_64 randconfig-a016-20201121 x86_64 randconfig-a012-20201121 x86_64 randconfig-a013-20201121 i386 randconfig-a012-20201121 i386 randconfig-a013-20201121 i386 randconfig-a011-20201121 i386 randconfig-a016-20201121 i386 randconfig-a014-20201121 i386 randconfig-a015-20201121 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec clang tested configs: x86_64 randconfig-a006-20201121 x86_64 randconfig-a003-20201121 x86_64 randconfig-a004-20201121 x86_64 randconfig-a005-20201121 x86_64 randconfig-a002-20201121 x86_64 randconfig-a001-20201121 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[PATCH] rtc: test: remove debug message
Remove leftover debug message Signed-off-by: Alexandre Belloni --- drivers/rtc/rtc-test.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/rtc/rtc-test.c b/drivers/rtc/rtc-test.c index b092a1648513..7e0d8fb26465 100644 --- a/drivers/rtc/rtc-test.c +++ b/drivers/rtc/rtc-test.c @@ -50,7 +50,6 @@ static int test_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) if (expires > U32_MAX) expires = U32_MAX; - pr_err("ABE: %s +%d %s\n", __FILE__, __LINE__, __func__); rtd->alarm.expires = expires; if (alrm->enabled) -- 2.28.0
Re: [PATCH 072/141] can: peak_usb: Fix fall-through warnings for Clang
On 11/21/20 8:50 PM, Joe Perches wrote: >> What about moving the default to the end if the case, which is more common >> anyways: >> >> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c >> b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > [] >> @@ -295,16 +295,16 @@ static void peak_usb_write_bulk_callback(struct urb >> *urb) >> netif_trans_update(netdev); >> break; >> >> >> - default: >> - if (net_ratelimit()) >> - netdev_err(netdev, "Tx urb aborted (%d)\n", >> - urb->status); >> case -EPROTO: >> case -ENOENT: >> case -ECONNRESET: >> case -ESHUTDOWN: >> - >> break; >> + >> + default: >> + if (net_ratelimit()) >> + netdev_err(netdev, "Tx urb aborted (%d)\n", >> + urb->status); > > That's fine and is more generally used style but this > default: case should IMO also end with a break; > > + break; I don't mind. process/coding-style.rst is not totally clear about the break after the default, if this is the lase one the switch statement. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | signature.asc Description: OpenPGP digital signature
Re: [GIT PULL] io_uring fixes for 5.10-rc
On 11/21/20 11:07 AM, Linus Torvalds wrote: > On Fri, Nov 20, 2020 at 7:00 PM Jens Axboe wrote: >> >> Actually, I think we can do even better. How about just having >> do_filp_open() exit after LOOKUP_RCU fails, if LOOKUP_RCU was already >> set in the lookup flags? Then we don't need to change much else, and >> most of it falls out naturally. > > So I was thinking doing the RCU lookup unconditionally, and then doing > the nn-RCU lookup if that fails afterwards. > > But your patch looks good to me. > > Except for the issue you noticed. After having taken a closer look, I think the saner approach is LOOKUP_NONBLOCK instead of using LOOKUP_RCU which is used more as a state than lookup flag. I'll try and hack something up that looks passable. >> Except it seems that should work, except LOOKUP_RCU does not guarantee >> that we're not going to do IO: > > Well, almost nothing guarantees lack of IO, since allocations etc can > still block, but.. Sure, and we can't always avoid that - but blatant block on waiting for IO should be avoided. >> [ 20.463195] schedule+0x5f/0xd0 >> [ 20.463444] io_schedule+0x45/0x70 >> [ 20.463712] bit_wait_io+0x11/0x50 >> [ 20.463981] __wait_on_bit+0x2c/0x90 >> [ 20.464264] out_of_line_wait_on_bit+0x86/0x90 >> [ 20.464611] ? var_wake_function+0x30/0x30 >> [ 20.464932] __ext4_find_entry+0x2b5/0x410 >> [ 20.465254] ? d_alloc_parallel+0x241/0x4e0 >> [ 20.465581] ext4_lookup+0x51/0x1b0 >> [ 20.465855] ? __d_lookup+0x77/0x120 >> [ 20.466136] path_openat+0x4e8/0xe40 >> [ 20.466417] do_filp_open+0x79/0x100 > > Hmm. Is this perhaps an O_CREAT case? I think we only do the dcache > lookups under RCU, not the final path component creation. It's just a basic test that opens all files under a directory. So no O_CREAT, it's all existing files. I think this is just a case of not aborting early enough for LOOKUP_NONBLOCK, and we've obviously already dropped LOOKUP_RCU (and done rcu_read_unlock() again) at this point. > And there are probably lots of other situations where we finish with > LOOKUP_RCU (with unlazy_walk()), and then continue.> > Example: look at "may_lookup()" - if inode_permission() says "I can't > do this without blocking" the logic actually just tries to validate > the current state (that "unlazy_walk()" thing), and then continue > without RCU. > > It obviously hasn't been about lockless semantics, it's been about > really being lockless. So LOOKUP_RCU has been a "try to do this > locklessly" rather than "you cannot take any locks". > > I guess we would have to add a LOOKUP_NOBLOCK thing to actually then > say "if the RCU lookup fails, return -EAGAIN". > > That's probably not a huge undertaking, but yeah, I didn't think of > it. I think this is a "we need to have Al tell us if it's reasonable". Definitely. I did have a weak attempt at LOOKUP_NONBLOCK earlier, I'll try and resurrect it and see what that leads to. Outside of just pure lookup, the d_revalidate() was a bit interesting as it may block for certain cases, but those should be (hopefully) detectable upfront. -- Jens Axboe
Re: [PATCH net] devlink: Fix reload stats structure
On Fri, 20 Nov 2020 15:40:37 +0200 Moshe Shemesh wrote: > Fix reload stats structure exposed to the user. Change stats structure > hierarchy to have the reload action as a parent of the stat entry and > then stat entry includes value per limit. This will also help to avoid > string concatenation on iproute2 output. > > Reload stats structure before this fix: > "stats": { > "reload": { > "driver_reinit": 2, > "fw_activate": 1, > "fw_activate_no_reset": 0 > } > } > > After this fix: > "stats": { > "reload": { > "driver_reinit": { > "unspecified": 2 > }, > "fw_activate": { > "unspecified": 1, > "no_reset": 0 > } > } > > Fixes: a254c264267e ("devlink: Add reload stats") > Signed-off-by: Moshe Shemesh > Reviewed-by: Jiri Pirko At least try to fold the core networking code at 80 characters *please*. You folded the comments at 86 chars, neither 100 nor 80.
[PATCH] rtc: mxc{,_v2}: enable COMPILE_TEST
Extend code coverage for the rtc-mxc and rtc-mxc-v2 drivers. Signed-off-by: Alexandre Belloni --- drivers/rtc/Kconfig | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index 4d2c5d1f75cc..9341ab15241e 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -1754,7 +1754,9 @@ config RTC_DRV_LOONGSON1 config RTC_DRV_MXC tristate "Freescale MXC Real Time Clock" - depends on ARCH_MXC + depends on ARCH_MXC || COMPILE_TEST + depends on HAS_IOMEM + depends on OF help If you say yes here you get support for the Freescale MXC RTC module. @@ -1764,7 +1766,9 @@ config RTC_DRV_MXC config RTC_DRV_MXC_V2 tristate "Freescale MXC Real Time Clock for i.MX53" - depends on ARCH_MXC + depends on ARCH_MXC || COMPILE_TEST + depends on HAS_IOMEM + depends on OF help If you say yes here you get support for the Freescale MXC SRTC module in i.MX53 processor. -- 2.28.0
Re: [PATCH net-next 0/5] net: hns3: misc updates for -next
Hello: This series was applied to netdev/net-next.git (refs/heads/master): On Fri, 20 Nov 2020 17:16:18 +0800 you wrote: > This series includes some misc updates for the HNS3 ethernet driver. > > #1 adds support for 1280 queues > #2 adds mapping for BAR45 which is needed by RoCE client. > #3 extend the interrupt resources. > #4 add support to query firmware's calculated shaping parameters. > > [...] Here is the summary with links: - [net-next,1/5] net: hns3: add support for 1280 queues https://git.kernel.org/netdev/net-next/c/9a5ef4aa5457 - [net-next,2/5] net: hns3: add support for mapping device memory https://git.kernel.org/netdev/net-next/c/30ae7f8a6aa7 - [net-next,3/5] net: hns3: add support for pf querying new interrupt resources https://git.kernel.org/netdev/net-next/c/3a6863e4e8ee - [net-next,4/5] net: hns3: add support to utilize the firmware calculated shaping parameters https://git.kernel.org/netdev/net-next/c/e364ad303fe3 - [net-next,5/5] net: hns3: adds debugfs to dump more info of shaping parameters https://git.kernel.org/netdev/net-next/c/c331ecf1afc1 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
INFO: task hung in __io_uring_files_cancel
Hello, syzbot found the following issue on: HEAD commit:7c8ca812 Add linux-next specific files for 20201117 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=12d2d19150 kernel config: https://syzkaller.appspot.com/x/.config?x=ff4bc71371dc5b13 dashboard link: https://syzkaller.appspot.com/bug?extid=c0d52d0b3c0c3ffb9525 compiler: gcc (GCC) 10.1.0-syz 20200507 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1481166a50 The issue was bisected to: commit 4d004099a668c41522242aa146a38cc4eb59cb1e Author: Peter Zijlstra Date: Fri Oct 2 09:04:21 2020 + lockdep: Fix lockdep recursion bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1040172650 final oops: https://syzkaller.appspot.com/x/report.txt?x=1240172650 console output: https://syzkaller.appspot.com/x/log.txt?x=1440172650 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+c0d52d0b3c0c3ffb9...@syzkaller.appspotmail.com Fixes: 4d004099a668 ("lockdep: Fix lockdep recursion") INFO: task syz-executor.0:9557 blocked for more than 143 seconds. Not tainted 5.10.0-rc4-next-20201117-syzkaller #0 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:syz-executor.0 state:D stack:28584 pid: 9557 ppid: 8485 flags:0x4002 Call Trace: context_switch kernel/sched/core.c:4269 [inline] __schedule+0x890/0x2030 kernel/sched/core.c:5019 schedule+0xcf/0x270 kernel/sched/core.c:5098 io_uring_cancel_files fs/io_uring.c:8720 [inline] io_uring_cancel_task_requests fs/io_uring.c:8772 [inline] __io_uring_files_cancel+0xc4d/0x14b0 fs/io_uring.c:8868 io_uring_files_cancel include/linux/io_uring.h:51 [inline] exit_files+0xe4/0x170 fs/file.c:456 do_exit+0xb61/0x29f0 kernel/exit.c:818 do_group_exit+0x125/0x310 kernel/exit.c:920 get_signal+0x3ea/0x1f70 kernel/signal.c:2750 arch_do_signal_or_restart+0x2a6/0x1ea0 arch/x86/kernel/signal.c:811 handle_signal_work kernel/entry/common.c:145 [inline] exit_to_user_mode_loop kernel/entry/common.c:169 [inline] exit_to_user_mode_prepare+0x124/0x200 kernel/entry/common.c:199 syscall_exit_to_user_mode+0x38/0x260 kernel/entry/common.c:274 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x45deb9 Code: Unable to access opcode bytes at RIP 0x45de8f. RSP: 002b:7fa68397ccf8 EFLAGS: 0246 ORIG_RAX: 00ca RAX: fe00 RBX: 0118bf28 RCX: 0045deb9 RDX: RSI: 0080 RDI: 0118bf28 RBP: 0118bf20 R08: R09: R10: R11: 0246 R12: 0118bf2c R13: 7fff50acc9af R14: 7fa68397d9c0 R15: 0118bf2c Showing all locks held in the system: 1 lock held by khungtaskd/1654: #0: 8b339ca0 (rcu_read_lock){}-{1:2}, at: debug_show_all_locks+0x53/0x260 kernel/locking/lockdep.c:6252 1 lock held by in:imklog/8177: #0: 88801cf22d70 (>f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0xe9/0x100 fs/file.c:932 = NMI backtrace for cpu 1 CPU: 1 PID: 1654 Comm: khungtaskd Not tainted 5.10.0-rc4-next-20201117-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:79 [inline] dump_stack+0x107/0x163 lib/dump_stack.c:120 nmi_cpu_backtrace.cold+0x44/0xd7 lib/nmi_backtrace.c:105 nmi_trigger_cpumask_backtrace+0x1b3/0x230 lib/nmi_backtrace.c:62 trigger_all_cpu_backtrace include/linux/nmi.h:147 [inline] check_hung_uninterruptible_tasks kernel/hung_task.c:253 [inline] watchdog+0xd89/0xf30 kernel/hung_task.c:338 kthread+0x3af/0x4a0 kernel/kthread.c:292 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296 Sending NMI from CPU 1 to CPUs 0: NMI backtrace for cpu 0 CPU: 0 PID: 4887 Comm: systemd-journal Not tainted 5.10.0-rc4-next-20201117-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:__sanitizer_cov_trace_const_cmp4+0x4/0x20 kernel/kcov.c:284 Code: 84 00 00 00 00 00 48 8b 0c 24 0f b7 d6 0f b7 f7 bf 03 00 00 00 e9 cc fe ff ff 66 90 66 2e 0f 1f 84 00 00 00 00 00 48 8b 0c 24 <89> f2 89 fe bf 05 00 00 00 e9 ae fe ff ff 0f 1f 40 00 66 2e 0f 1f RSP: 0018:c9f0fe90 EFLAGS: 0206 RAX: RBX: 00080042 RCX: 81bd2a21 RDX: 88802594b500 RSI: 0040 RDI: RBP: 1920001e1fd3 R08: R09: 8880110f3c17 R10: R11: R12: 0040 R13: 01a0 R14: 5616bdf3c270 R15: FS: 7f934ff098c0() GS:8880b9e0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 00a47b58 CR3: 26162000 CR4: 001506f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7:
[PATCH] dm crypt: Constify static crypt_iv_operations
The only usage of these structs is to assign their address to the iv_gen_ops field in the crypt config struct, which is a pointer to const. Make them const like the rest of the static crypt_iv_operations structs. This allows the compiler to put them in read-only memory. Signed-off-by: Rikard Falkeborn --- drivers/md/dm-crypt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 392337f16ecf..53791138d78b 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1090,16 +1090,16 @@ static const struct crypt_iv_operations crypt_iv_tcw_ops = { .post = crypt_iv_tcw_post }; -static struct crypt_iv_operations crypt_iv_random_ops = { +static const struct crypt_iv_operations crypt_iv_random_ops = { .generator = crypt_iv_random_gen }; -static struct crypt_iv_operations crypt_iv_eboiv_ops = { +static const struct crypt_iv_operations crypt_iv_eboiv_ops = { .ctr = crypt_iv_eboiv_ctr, .generator = crypt_iv_eboiv_gen }; -static struct crypt_iv_operations crypt_iv_elephant_ops = { +static const struct crypt_iv_operations crypt_iv_elephant_ops = { .ctr = crypt_iv_elephant_ctr, .dtr = crypt_iv_elephant_dtr, .init = crypt_iv_elephant_init, -- 2.29.2
Re: [PATCH] usbnet: ipheth: fix connectivity with iOS 14
On Thu, 19 Nov 2020 18:24:39 +0100 Yves-Alexis Perez wrote: > Starting with iOS 14 released in September 2020, connectivity using the > personal hotspot USB tethering function of iOS devices is broken. > > Communication between the host and the device (for example ICMP traffic > or DNS resolution using the DNS service running in the device itself) > works fine, but communication to endpoints further away doesn't work. > > Investigation on the matter shows that UDP and ICMP traffic from the > tethered host is reaching the Internet at all. For TCP traffic there are > exchanges between tethered host and server but packets are modified in > transit leading to impossible communication. > > After some trials Matti Vuorela discovered that reducing the URB buffer > size by two bytes restored the previous behavior. While a better > solution might exist to fix the issue, since the protocol is not > publicly documented and considering the small size of the fix, let's do > that. > > Tested-by: Matti Vuorela > Signed-off-by: Yves-Alexis Perez > Link: > https://lore.kernel.org/linux-usb/caan0qaxmysj9vx3zemkviv_b19ju-_exn8yn_usefxpjs6g...@mail.gmail.com/ > Link: https://github.com/libimobiledevice/libimobiledevice/issues/1038 Applied to net with the typo fixed, thanks!
Re: [PATCH 1/3] pwm: pca9685: Switch to atomic API
On Sat, Nov 21, 2020 at 9:58 AM Clemens Gruber wrote: > > I'd rather continue supporting this driver with !CONFIG_PM. (In our > company we have a product with a !CONFIG_PM build using this driver) Absolutely, makes sense. If you do add support for !CONFIG_PM, then it's important that both PM and !PM cases get tested by you. > > I am thinking about the following solution: > #ifdef CONFIG_PM > /* Set runtime PM status according to chip sleep state */ > if (reg & MODE1_SLEEP) > pm_runtime_set_suspended(..); > else > pm_runtime_set_active(..); > > pm_runtime_enable(..); > #else > /* If in SLEEP state on non-PM environments, wake the chip up */ > if (reg & MODE1_SLEEP) > pca9685_set_sleep_mode(.., false) > #endif I don't think we need the #ifdef CONFIG_PM, because all pm_runtime_xxx functions become no-ops when !CONFIG_PM. Also, I believe "if (IS_ENABLED(CONFIG_XXX))" is preferred, because it allows the compiler to syntax-check disabled code. How about the following? It should be correct, short, and easy to understand. Yes, there's one single unnecessary register write (+ 500us delay if !PM) when the chip is already active on probe(). But maybe that's worth it if it makes the code easier to understand? probe() { ... pm_runtime_set_active(>dev); pm_runtime_enable(>dev); if (!IS_ENABLED(CONFIG_PM)) pca9685_set_sleep_mode(pca, false); return 0; } remove() { ... pm_runtime_disable(>dev); if (!IS_ENABLED(CONFIG_PM)) pca9685_set_sleep_mode(pca, true); return 0; } > > About the regmap cache: I looked into it and think it is a good idea but > it's probably best to get these patches merged first and then rework the > driver to using the regmap cache? Good suggestion, I agree.
Re: [PATCH v1] qnx4_match: do not over run the buffer
On Saturday, 2020-11-21 22:47 Tong Zhang wrote: > > > On Nov 21, 2020, at 4:40 PM, Anders Larsen wrote: > > > > On Friday, 2020-11-20 22:21 Tong Zhang wrote: > >> the di_fname may not terminated by '\0', use strnlen to prevent buffer > >> overrun > >> > >> --- > >> fs/qnx4/namei.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/fs/qnx4/namei.c b/fs/qnx4/namei.c > >> index 8d72221735d7..c0e79094f578 100644 > >> --- a/fs/qnx4/namei.c > >> +++ b/fs/qnx4/namei.c > >> @@ -40,7 +40,7 @@ static int qnx4_match(int len, const char *name, > >>} else { > >>namelen = QNX4_SHORT_NAME_MAX; > >>} > >> - thislen = strlen( de->di_fname ); > >> + thislen = strnlen( de->di_fname, QNX4_SHORT_NAME_MAX ); > > > > that should be > > + thislen = strnlen( de->di_fname, namelen ); > > otherwise the length of a filename would always be limited to > > QNX4_SHORT_NAME_MAX (16) characters. > > > Why should we put something bigger here if the size of > qnx4_inode_entry->di_fname is QNX4_SHORT_NAME_MAX. > Won’t that be a problem? If QNX4_FILE_LINK is set in de->di_status (see line 38), 'de' actually points to a struct qnx4_link_info which can hold a longer name. That's the reason for the namelen massage. (Please don't ask why it is not a union) Cheers Anders
Re: [PATCH v1] qnx4_match: do not over run the buffer
> On Nov 21, 2020, at 4:40 PM, Anders Larsen wrote: > > On Friday, 2020-11-20 22:21 Tong Zhang wrote: >> the di_fname may not terminated by '\0', use strnlen to prevent buffer >> overrun >> >> --- >> fs/qnx4/namei.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/qnx4/namei.c b/fs/qnx4/namei.c >> index 8d72221735d7..c0e79094f578 100644 >> --- a/fs/qnx4/namei.c >> +++ b/fs/qnx4/namei.c >> @@ -40,7 +40,7 @@ static int qnx4_match(int len, const char *name, >> } else { >> namelen = QNX4_SHORT_NAME_MAX; >> } >> -thislen = strlen( de->di_fname ); >> +thislen = strnlen( de->di_fname, QNX4_SHORT_NAME_MAX ); > > that should be > + thislen = strnlen( de->di_fname, namelen ); > otherwise the length of a filename would always be limited to > QNX4_SHORT_NAME_MAX (16) characters. > Why should we put something bigger here if the size of qnx4_inode_entry->di_fname is QNX4_SHORT_NAME_MAX. Won’t that be a problem? >> if ( thislen > namelen ) >> thislen = namelen; > > These two lines can be dropped now, as the result of strnlen() cannot exceed > namelen anyway. > > Cheers > Anders > >
Re: [PATCH v2 net] ptp: clockmatrix: bug fix for idtcm_strverscmp
On Wed, 18 Nov 2020 22:50:24 -0500 min.li...@renesas.com wrote: > From: Min Li > > Feed kstrtou8 with NULL terminated string. > > Changes since v1: > -Only strcpy 15 characters to leave 1 space for '\0' > > Signed-off-by: Min Li > -static int idtcm_strverscmp(const char *ver1, const char *ver2) > +static int idtcm_strverscmp(const char *version1, const char *version2) > { > u8 num1; > u8 num2; > int result = 0; > + char ver1[16]; > + char ver2[16]; > + char *cur1; > + char *cur2; > + char *next1; > + char *next2; > + > + strncpy(ver1, version1, 15); > + strncpy(ver2, version2, 15); > + cur1 = ver1; > + cur2 = ver2; Now there is no guarantee that the buffer is null terminated. You need to use strscpy(ver... 16) or add ver[15] = '\0'; > /* loop through each level of the version string */ > while (result == 0) { > + next1 = strchr(cur1, '.'); > + next2 = strchr(cur2, '.'); > + > + /* kstrtou8 could fail for dot */ > + if (next1) { > + *next1 = '\0'; > + next1++; > + } > + > + if (next2) { > + *next2 = '\0'; > + next2++; > + } > + > /* extract leading version numbers */ > - if (kstrtou8(ver1, 10, ) < 0) > + if (kstrtou8(cur1, 10, ) < 0) > return -1; > > - if (kstrtou8(ver2, 10, ) < 0) > + if (kstrtou8(cur2, 10, ) < 0) > return -1; > > /* if numbers differ, then set the result */ > - if (num1 < num2) > + if (num1 < num2) { > result = -1; Why do you set the result to something instead of just returning that value? The kstrtou8() checks above just return value directly... If you use a return you can save yourself all the else branches. > - else if (num1 > num2) > + } else if (num1 > num2) { > result = 1; > - else { > + } else { > /* if numbers are the same, go to next level */ > - ver1 = strchr(ver1, '.'); > - ver2 = strchr(ver2, '.'); > - if (!ver1 && !ver2) > + if (!next1 && !next2) > break; > - else if (!ver1) > + else if (!next1) { > result = -1; > - else if (!ver2) > + } else if (!next2) { > result = 1; > - else { > - ver1++; > - ver2++; > + } else { > + cur1 = next1; > + cur2 = next2; > } > } > } > + > return result; > } >
Re: [PATCH v1] qnx4_match: do not over run the buffer
On Friday, 2020-11-20 22:21 Tong Zhang wrote: > the di_fname may not terminated by '\0', use strnlen to prevent buffer > overrun > > --- > fs/qnx4/namei.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/qnx4/namei.c b/fs/qnx4/namei.c > index 8d72221735d7..c0e79094f578 100644 > --- a/fs/qnx4/namei.c > +++ b/fs/qnx4/namei.c > @@ -40,7 +40,7 @@ static int qnx4_match(int len, const char *name, > } else { > namelen = QNX4_SHORT_NAME_MAX; > } > - thislen = strlen( de->di_fname ); > + thislen = strnlen( de->di_fname, QNX4_SHORT_NAME_MAX ); that should be + thislen = strnlen( de->di_fname, namelen ); otherwise the length of a filename would always be limited to QNX4_SHORT_NAME_MAX (16) characters. > if ( thislen > namelen ) > thislen = namelen; These two lines can be dropped now, as the result of strnlen() cannot exceed namelen anyway. Cheers Anders
[tip:x86/cleanups] BUILD SUCCESS ab09b58e4bdfdbcec425e54ebeaf6e209a96318f
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cleanups branch HEAD: ab09b58e4bdfdbcec425e54ebeaf6e209a96318f x86/boot/compressed/64: Use TEST %reg,%reg instead of CMP $0,%reg elapsed time: 720m configs tested: 94 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig arm efm32_defconfig sh rsk7203_defconfig sh rsk7269_defconfig sh sh03_defconfig powerpcgamecube_defconfig arm mxs_defconfig powerpc mpc83xx_defconfig mips ip32_defconfig arm badge4_defconfig powerpc tqm8540_defconfig sh se7721_defconfig powerpcamigaone_defconfig arm bcm2835_defconfig m68kq40_defconfig mips decstation_64_defconfig arm ixp4xx_defconfig shsh7785lcr_defconfig arm colibri_pxa300_defconfig arcvdk_hs38_smp_defconfig powerpc canyonlands_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a004-20201121 i386 randconfig-a003-20201121 i386 randconfig-a002-20201121 i386 randconfig-a005-20201121 i386 randconfig-a001-20201121 i386 randconfig-a006-20201121 x86_64 randconfig-a015-20201121 x86_64 randconfig-a011-20201121 x86_64 randconfig-a014-20201121 x86_64 randconfig-a016-20201121 x86_64 randconfig-a012-20201121 x86_64 randconfig-a013-20201121 i386 randconfig-a012-20201121 i386 randconfig-a013-20201121 i386 randconfig-a011-20201121 i386 randconfig-a016-20201121 i386 randconfig-a014-20201121 i386 randconfig-a015-20201121 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec clang tested configs: x86_64 randconfig-a006-20201121 x86_64 randconfig-a003-20201121 x86_64 randconfig-a004-20201121 x86_64 randconfig-a005-20201121 x86_64 randconfig-a002-20201121 x86_64 randconfig-a001-20201121 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[PATCH v2] checkpatch: add --fix option for INCLUDE_LINUX
Provide fix option to INCLUDE_LINUX check to replace asm includes. Macros of type: #include are corrected to: #include Signed-off-by: Dwaipayan Ray --- Changes in v2: - Use \Q..\E quoting - Use @ as regex delimiter scripts/checkpatch.pl | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 0da6422cd0fd..e4feb91a0fe4 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5468,8 +5468,11 @@ sub process { CHK("ARCH_INCLUDE_LINUX", "Consider using #include instead of \n" . $herecurr); } else { - WARN("INCLUDE_LINUX", -"Use #include instead of \n" . $herecurr); + if (WARN("INCLUDE_LINUX", +"Use #include instead of \n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s@\Q\E@@; + } } } } -- 2.27.0
Re: [PATCH] cxgb4: Fix build failure when CONFIG_TLS=m
On Fri, 20 Nov 2020 13:25:28 -0600 Tom Seewald wrote: > After commit 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > whenever CONFIG_TLS=m and CONFIG_CHELSIO_T4=y, the following build > failure occurs: > > ld: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.o: in function > `cxgb_select_queue': > cxgb4_main.c:(.text+0x2dac): undefined reference to `tls_validate_xmit_skb' > > Fix this by ensuring that if TLS is set to be a module, CHELSIO_T4 will > also be compiled as a module. As otherwise the cxgb4 driver will not be > able to access TLS' symbols. > > Fixes: 9d2e5e9eeb59 ("cxgb4/ch_ktls: decrypted bit is not enough") > Signed-off-by: Tom Seewald Applied, thanks!
[PATCH v4] checkpatch: add fix option for LOGICAL_CONTINUATIONS
Currently, checkpatch warns if logical continuations are placed at the start of a line and not at the end of previous line. E.g., running checkpatch on commit 3485507fc272 ("staging: bcm2835-camera: Reduce length of enum names") reports: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line + if (!ret + && camera_port == Provide a simple fix by adding logical operator at the end of previous line and removing from current line, if both the lines are additions (ie start with '+') Signed-off-by: Aditya Srivastava --- changes in v2: quote $operator at substitution changes in v3: add a check for previous line ending with comment; If so, insert $operator at the last non-comment, non-whitespace char of the previous line changes in v4: improve the matching mechanism by matching line termination at comment or white space; insert the operator before comments (if any) separated by a whitespace; append the comment and its pre-whitespace after the inserted operator (if comment was present), ie if no comment was present nothing will be inserted after the operator scripts/checkpatch.pl | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 5b1a5a65e69a..43a05519665d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3553,8 +3553,19 @@ sub process { # check for && or || at the start of a line if ($rawline =~ /^\+\s*(&&|\|\|)/) { - CHK("LOGICAL_CONTINUATIONS", - "Logical continuations should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("LOGICAL_CONTINUATIONS", + "Logical continuations should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + # add logical operator to the previous line, remove from current line + # match line termination at comment or whitespace + if ($prevrawline =~ /(\s*(?:\/\/.*)*)$/) { + my $match = $1; + $fixed[$fixlinenr - 1] =~ s/$match//; + $fixed[$fixlinenr - 1] .= " $operator$match"; + } + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } } # check indentation starts on a tab stop -- 2.17.1
Re: [PATCH v5 2/3] net: add kcov handle to skb extensions
On Sat, 21 Nov 2020 21:58:37 +0100 Johannes Berg wrote: > On Sat, 2020-11-21 at 12:55 -0800, Jakub Kicinski wrote: > > It is more complicated. We can go back to an skb field if this work is > > expected to yield results for mac80211. Would you mind sending a patch? > > I can do that, but I'm not going to be able to do it now/tonight (GMT+1 > here), so probably only Monday/Tuesday or so, sorry. Oh yea, no worries, took someone a month to notice this is broken, as long as it's fixed before the merge window it's fine ;)
[PATCH 2/2] HID: elecom: add support for EX-G M-XGL20DLBK wireless mouse
Enables three buttons (Fn1, Fn2, and Fn3) on the ELECOM M-XGL20DLBK wireless mouse. While this mouse is EX-G brand, report descriptor is a bit different from EX-G trackball mouse. To enable extra buttons, report should be rewritten in a similar way to trackballs, but with different position parameters. Signed-off-by: YOSHIOKA Takuma --- drivers/hid/hid-elecom.c | 11 +++ drivers/hid/hid-ids.h| 1 + drivers/hid/hid-quirks.c | 1 + 3 files changed, 13 insertions(+) diff --git a/drivers/hid/hid-elecom.c b/drivers/hid/hid-elecom.c index 233188100884..3bd7207de2e8 100644 --- a/drivers/hid/hid-elecom.c +++ b/drivers/hid/hid-elecom.c @@ -64,6 +64,16 @@ static __u8 *elecom_report_fixup(struct hid_device *hdev, __u8 *rdesc, rdesc[47] = 0x00; } break; + case USB_DEVICE_ID_ELECOM_M_XGL20DLBK: + /* +* Report descriptor format: +* 20: button bit count +* 28: padding bit count +* 22: button report size +* 14: button usage maximum +*/ + mouse_button_fixup(hdev, rdesc, *rsize, 20, 28, 22, 14, 8); + break; case USB_DEVICE_ID_ELECOM_M_XT3URBK: case USB_DEVICE_ID_ELECOM_M_XT3DRBK: case USB_DEVICE_ID_ELECOM_M_XT4DRBK: @@ -96,6 +106,7 @@ static __u8 *elecom_report_fixup(struct hid_device *hdev, __u8 *rdesc, static const struct hid_device_id elecom_devices[] = { { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084) }, + { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XGL20DLBK) }, { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK) }, { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3DRBK) }, { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT4DRBK) }, diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index d69842f79fc6..a29f4dacf640 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -388,6 +388,7 @@ #define USB_VENDOR_ID_ELECOM 0x056e #define USB_DEVICE_ID_ELECOM_BM084 0x0061 +#define USB_DEVICE_ID_ELECOM_M_XGL20DLBK 0x00e6 #define USB_DEVICE_ID_ELECOM_M_XT3URBK 0x00fb #define USB_DEVICE_ID_ELECOM_M_XT3DRBK 0x00fc #define USB_DEVICE_ID_ELECOM_M_XT4DRBK 0x00fd diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 7a2be0205dfd..9355d92c6f14 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -361,6 +361,7 @@ static const struct hid_device_id hid_have_special_driver[] = { #endif #if IS_ENABLED(CONFIG_HID_ELECOM) { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084) }, + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XGL20DLBK) }, { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3URBK) }, { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT3DRBK) }, { HID_USB_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_M_XT4DRBK) }, -- 2.29.2
[PATCH 1/2] HID: elecom: rewrite report based on model specific parameters
The report descriptor for EX-G wireless mouse (M-XGL20DLBK) is a bit different from that for trackball mice such as DEFT. For such mouse, the current `mouse_button_fixup` cannot be used as is, because it uses hard-coded indices for a report descriptor. Add parameters to `mouse_button_fixup` function, in order to support fixing report descriptors for more models. Signed-off-by: YOSHIOKA Takuma --- drivers/hid/hid-elecom.c | 41 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/drivers/hid/hid-elecom.c b/drivers/hid/hid-elecom.c index 8c712d4bc075..233188100884 100644 --- a/drivers/hid/hid-elecom.c +++ b/drivers/hid/hid-elecom.c @@ -11,6 +11,7 @@ * Copyright (c) 2017 Diego Elio Pettenò * Copyright (c) 2017 Alex Manoussakis * Copyright (c) 2017 Tomasz Kramkowski + * Copyright (c) 2020 YOSHIOKA Takuma */ /* @@ -29,25 +30,26 @@ * report descriptor but it does not appear that these enable software to * control what the extra buttons map to. The only simple and straightforward * solution seems to involve fixing up the report descriptor. - * - * Report descriptor format: - * Positions 13, 15, 21 and 31 store the button bit count, button usage minimum, - * button usage maximum and padding bit count respectively. */ #define MOUSE_BUTTONS_MAX 8 static void mouse_button_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int rsize, + unsigned int button_bit_count, + unsigned int padding_bit, + unsigned int button_report_size, + unsigned int button_usage_maximum, int nbuttons) { - if (rsize < 32 || rdesc[12] != 0x95 || - rdesc[14] != 0x75 || rdesc[15] != 0x01 || - rdesc[20] != 0x29 || rdesc[30] != 0x75) + if (rsize < 32 || rdesc[button_bit_count] != 0x95 || + rdesc[button_report_size] != 0x75 || + rdesc[button_report_size + 1] != 0x01 || + rdesc[button_usage_maximum] != 0x29 || rdesc[padding_bit] != 0x75) return; hid_info(hdev, "Fixing up Elecom mouse button count\n"); nbuttons = clamp(nbuttons, 0, MOUSE_BUTTONS_MAX); - rdesc[13] = nbuttons; - rdesc[21] = nbuttons; - rdesc[31] = MOUSE_BUTTONS_MAX - nbuttons; + rdesc[button_bit_count + 1] = nbuttons; + rdesc[button_usage_maximum + 1] = nbuttons; + rdesc[padding_bit + 1] = MOUSE_BUTTONS_MAX - nbuttons; } static __u8 *elecom_report_fixup(struct hid_device *hdev, __u8 *rdesc, @@ -65,13 +67,28 @@ static __u8 *elecom_report_fixup(struct hid_device *hdev, __u8 *rdesc, case USB_DEVICE_ID_ELECOM_M_XT3URBK: case USB_DEVICE_ID_ELECOM_M_XT3DRBK: case USB_DEVICE_ID_ELECOM_M_XT4DRBK: - mouse_button_fixup(hdev, rdesc, *rsize, 6); + /* +* Report descriptor format: +* 12: button bit count +* 30: padding bit count +* 14: button report size +* 20: button usage maximum +*/ + mouse_button_fixup(hdev, rdesc, *rsize, 12, 30, 14, 20, 6); break; case USB_DEVICE_ID_ELECOM_M_DT1URBK: case USB_DEVICE_ID_ELECOM_M_DT1DRBK: case USB_DEVICE_ID_ELECOM_M_HT1URBK: case USB_DEVICE_ID_ELECOM_M_HT1DRBK: - mouse_button_fixup(hdev, rdesc, *rsize, 8); + /*mouse_button_fixup(hdev, rdesc, *rsize, 13, 15, 21, 31, 8);*/ + /* +* Report descriptor format: +* 12: button bit count +* 30: padding bit count +* 14: button report size +* 20: button usage maximum +*/ + mouse_button_fixup(hdev, rdesc, *rsize, 12, 30, 14, 20, 8); break; } return rdesc; -- 2.29.2
Re: [PATCH v5 2/3] net: add kcov handle to skb extensions
On Sat, 2020-11-21 at 12:55 -0800, Jakub Kicinski wrote: > [snip] > Ack, you have to figure out all the places anyway, the question is > whether you put probes there or calls in the source code. > > Shifting the maintenance burden but also BPF is flexibility. Yeah, true. Though I'd argue also visibility - this stuff is pretty simple now, if it gets into lots of lines of BPF code to track it that is maintained "elsewhere", we won't see the bugs in it :-) And it's kinda a thing that we as kernel developers _should_ be the ones looking at since it's testing our code. > Yup, the point is you can feed a raw skb pointer (and all other > possible context you may want) to a BPF prog in kcov_remote_start() > and let BPF/BTF give you the handle it recorded in its maps. Yeah, it's possible. Personally, I don't think it's worth the complexity. > It is more complicated. We can go back to an skb field if this work is > expected to yield results for mac80211. Would you mind sending a patch? I can do that, but I'm not going to be able to do it now/tonight (GMT+1 here), so probably only Monday/Tuesday or so, sorry. johannes
Re: [PATCH v5 2/3] net: add kcov handle to skb extensions
On Sat, 21 Nov 2020 20:30:44 +0100 Johannes Berg wrote: > On Sat, 2020-11-21 at 10:35 -0800, Jakub Kicinski wrote: > > On Sat, 21 Nov 2020 19:12:21 +0100 Johannes Berg wrote: > > > > So I'm leaning towards reverting the whole thing. You can attach > > > > kretprobes and record the information you need in BPF maps. > > > > > > I'm not going to object to reverting it (and perhaps redoing it better > > > later), but I will point out that kretprobe isn't going to work, you > > > eventually need kcov_remote_start() to be called in strategic points > > > before processing the skb after it bounced through the system. > > > > > > IOW, it's not really about serving userland, it's about enabling (and > > > later disabling) coverage collection for the bits of code it cares > > > about, mostly because collecting it for _everything_ is going to be too > > > slow and will mess up the data since for coverage guided fuzzing you > > > really need the reported coverage data to be only about the injected > > > fuzz data... > > > > All you need is make kcov_remote_start_common() be BPF-able, like > > the LSM hooks are now, right? And then BPF can return whatever handle > > it pleases. > > Not sure I understand. Are you saying something should call > "kcov_remote_start_common()" with, say, the SKB, and leave it to a mass > of bpf hooks to figure out where the SKB got cloned or copied or > whatnot, track that in a map, and then ... no, wait, I don't really see > what you mean, sorry. > > IIUC, fundamentally, you have this: > > - at the beginning, a task is tagged with "please collect coverage >data for this handle" Write the tag into task local storage, or map indexed by PID. > - this task creates an SKB, etc, and all of the code that this task >executes is captured and the coverage data is reported kprobe the right places to record the skb -> handle mapping. > - However, the SKB traverses lots of things, gets copied, cloned, or >whatnot, and eventually leaves the annotated task, say for further >processing in softirq context or elsewhere. Which is fine. > Now since the whole point is to see what chaos this SKB created from > beginning (allocation) to end (free), since it was filled with fuzzed > data, you now have to figure out where to pick back up when the SKB is > processed further. > > This is what the infrastructure was meant to solve. But note that the > SKB might be further cloned etc, so in order to track it you'd have to > (out-of-band) figure out all the possible places where it could > be reallocated, any time the skb pointer could change. Ack, you have to figure out all the places anyway, the question is whether you put probes there or calls in the source code. Shifting the maintenance burden but also BPF is flexibility. > Then, when you know you've got interesting code on your hands, like in > mac80211 that was annotated in patch 3 here, you basically say > > "oohhh, this SKB was annotated before, let's continue capturing >coverage data here" > > (and turn it off again later by the corresponding kcov_remote_stop(). Yup, the point is you can feed a raw skb pointer (and all other possible context you may want) to a BPF prog in kcov_remote_start() and let BPF/BTF give you the handle it recorded in its maps. > So the only way I could _possibly_ see how to do this would be to > > * capture all possible places where the skb pointer can change > * still call something like skb_get_kcov_handle() but let it call out >to a BPF program to query a map or something to figure out if this >SKB has a handle attached to it > > > Or if you don't like BPF or what to KCOV BPF itself in the future you > > can roll your own mechanism. The point is - this should be relatively > > easily doable out of line... > > Seems pretty complicated to me though ... It is more complicated. We can go back to an skb field if this work is expected to yield results for mac80211. Would you mind sending a patch?
Re: [PATCH] ilog2: Improve ilog2 for constant arguments
On Sat, Nov 21, 2020 at 09:29:54PM +0100, Jakub Jelinek wrote: > On Sat, Nov 21, 2020 at 09:23:10PM +0100, Luc Van Oostenryck wrote: > > On Fri, Nov 20, 2020 at 01:51:54PM +0100, Peter Zijlstra wrote: > > > > > > Other option would be to change the const_ilog2 macro, though as the > > > description says it is meant to be used also in C constant expressions, > > > and while GCC will fold it to constant with constant argument even in > > > those, perhaps it is better to avoid using extensions in that case. > > > > Just for info, the description is outdated and Sparse is just fine with > > __builtin_clzll() and friends in constant expressions (since Feb 2017) > > Why is the description outdated? It is still an extension that not every > compiler might fold in constant expressions. And, the large expressions > aren't really a problem in constant expressions, they will be folded there > to constant or error. > The problem the patch was trying to solve is that the large expressions are > a problem at least for GCC in runtime code when guarded by > __builtin_constant_p, because __builtin_constant_p is folded quite late > (intentionally so, so that more constants can be propagated into it, e.g. > after inlining etc.), and the large expressions might confuse inliner > heuristics. I was only talking about the part "Use this where *sparse* expects a true constant expression, e.g. for array 75 indices." and wanted to say that __builtin_clzll() with a constant argument is now also expanded to a constant, like GCC does (it wasn't the case before 2017 and I think it was the main reason why const_ilog2() is written as it is). -- Luc