Re: [RFC PATCH v2] Hardcoded instruction causes cpu-hotplug fail on be8 big-endian ARM platfrom
> Not handling the Thumb case is a definite bug for any file which may > run on v7, since the kernel could be built in Thumb for that case. > For example, the existing code is mach-realview/hotplug.c is broken > when building an SMP Thumb-2 kernel for the Realview PBX-A9. > > Cheers > ---Dave Thanks for pointing this out. I think we just cannot make any assumption about the versions of the tools used. Based on this, I have made a v2 of the patch against linux-3.7-rc1. I am not touching the CFLAGS in the patch as I am not familiar with the three ARM boards here. Can the respective board maintainers (mach-exynos/mach-realveiw/mach-shmobile) give any comments about v2 of the patch? Thanks. ---Fei v2: Define opcode of the ARM "WFI" instruction in the right way. Signed-off-by: Fei Yang diff -urN linux-3.7-rc1/arch/arm/mach-exynos/hotplug.c linux/arch/arm/mach-exynos/hotplug.c --- linux-3.7-rc1/arch/arm/mach-exynos/hotplug.c2012-10-15 05:41:04.0 +0800 +++ linux/arch/arm/mach-exynos/hotplug.c2012-10-17 19:25:49.0 +0800 @@ -18,11 +18,17 @@ #include #include #include +#include #include #include "common.h" +/* + * Define opcode of the WFI instruction. + */ +#define __WFI __inst_arm_thumb16(0xe320f003, 0xbf30) + static inline void cpu_enter_lowpower(void) { unsigned int v; @@ -72,7 +78,7 @@ /* * here's the WFI */ - asm(".word 0xe320f003\n" + asm(__WFI : : : "memory", "cc"); diff -urN linux-3.7-rc1/arch/arm/mach-realview/hotplug.c linux/arch/arm/mach-realview/hotplug.c --- linux-3.7-rc1/arch/arm/mach-realview/hotplug.c 2012-10-15 05:41:04.0 +0800 +++ linux/arch/arm/mach-realview/hotplug.c 2012-10-17 19:25:20.0 +0800 @@ -15,6 +15,12 @@ #include #include #include +#include + +/* + * Define opcode of the WFI instruction. + */ +#define __WFI __inst_arm_thumb16(0xe320f003, 0xbf30) static inline void cpu_enter_lowpower(void) { @@ -64,7 +70,7 @@ /* * here's the WFI */ - asm(".word 0xe320f003\n" + asm(__WFI : : : "memory", "cc"); diff -urN linux-3.7-rc1/arch/arm/mach-shmobile/hotplug.c linux/arch/arm/mach-shmobile/hotplug.c --- linux-3.7-rc1/arch/arm/mach-shmobile/hotplug.c 2012-10-15 05:41:04.0 +0800 +++ linux/arch/arm/mach-shmobile/hotplug.c 2012-10-17 19:25:34.0 +0800 @@ -20,6 +20,12 @@ #include #include #include +#include + +/* + * Define opcode of the WFI instruction. + */ +#define __WFI __inst_arm_thumb16(0xe320f003, 0xbf30) static cpumask_t dead_cpus; @@ -39,7 +45,7 @@ /* * here's the WFI */ - asm(".word 0xe320f003\n" + asm(__WFI : : : "memory", "cc"); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v2] ARM hardcoded instruction causes cpu-hotplug fail to work on big-endian platfrom
2012/10/17 Fei Yang : > > >> Not handling the Thumb case is a definite bug for any file which may >> run on v7, since the kernel could be built in Thumb for that case. >> For example, the existing code is mach-realview/hotplug.c is broken >> when building an SMP Thumb-2 kernel for the Realview PBX-A9. >> >> Cheers >> ---Dave > > Thanks for pointing this out. I think we just cannot make any > assumption about the versions of the tools used. Based on this, I have > made a v2 of the patch against linux-3.7-rc1. I am not touching the > CFLAGS in the patch as I am not familiar with the three ARM boards > here. > Can the respective board maintainers > (mach-exynos/mach-realveiw/mach-shmobile) give any comments about v2 > of the patch? > > Thanks. > ---Fei > > v2: Define opcode of the ARM "WFI" instruction in the right way. > > Signed-off-by: Fei Yang > > diff -urN linux-3.7-rc1/arch/arm/mach-exynos/hotplug.c > linux/arch/arm/mach-exynos/hotplug.c > --- linux-3.7-rc1/arch/arm/mach-exynos/hotplug.c2012-10-15 > 05:41:04.0 +0800 > +++ linux/arch/arm/mach-exynos/hotplug.c2012-10-17 19:25:49.0 > +0800 > @@ -18,11 +18,17 @@ > #include > #include > #include > +#include > > #include > > #include "common.h" > > +/* > + * Define opcode of the WFI instruction. > + */ > +#define __WFI __inst_arm_thumb16(0xe320f003, 0xbf30) > + > static inline void cpu_enter_lowpower(void) > { > unsigned int v; > @@ -72,7 +78,7 @@ > /* > * here's the WFI > */ > - asm(".word 0xe320f003\n" > + asm(__WFI > : > : > : "memory", "cc"); > diff -urN linux-3.7-rc1/arch/arm/mach-realview/hotplug.c > linux/arch/arm/mach-realview/hotplug.c > --- linux-3.7-rc1/arch/arm/mach-realview/hotplug.c 2012-10-15 > 05:41:04.0 +0800 > +++ linux/arch/arm/mach-realview/hotplug.c 2012-10-17 19:25:20.0 > +0800 > @@ -15,6 +15,12 @@ > #include > #include > #include > +#include > + > +/* > + * Define opcode of the WFI instruction. > + */ > +#define __WFI __inst_arm_thumb16(0xe320f003, 0xbf30) > > static inline void cpu_enter_lowpower(void) > { > @@ -64,7 +70,7 @@ > /* > * here's the WFI > */ > - asm(".word 0xe320f003\n" > + asm(__WFI > : > : > : "memory", "cc"); > diff -urN linux-3.7-rc1/arch/arm/mach-shmobile/hotplug.c > linux/arch/arm/mach-shmobile/hotplug.c > --- linux-3.7-rc1/arch/arm/mach-shmobile/hotplug.c 2012-10-15 > 05:41:04.0 +0800 > +++ linux/arch/arm/mach-shmobile/hotplug.c 2012-10-17 19:25:34.0 > +0800 > @@ -20,6 +20,12 @@ > #include > #include > #include > +#include > + > +/* > + * Define opcode of the WFI instruction. > + */ > +#define __WFI __inst_arm_thumb16(0xe320f003, 0xbf30) > > static cpumask_t dead_cpus; > > @@ -39,7 +45,7 @@ > /* > * here's the WFI > */ > - asm(".word 0xe320f003\n" > + asm(__WFI > : > : > : "memory", "cc"); > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > > Hi, Any comments on this patch? It is posted last week. Please let me know if I missed anything. Thanks. ---Fei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Re: Hardcoded instruction causes certain features to fail on ARM platfrom due to endianness
2012/10/15 Mikael Pettersson : > Yangfei (Felix) writes: > > Hi all, > > > > I found that hardcoded instruction in inline asm can cause certains > certain features fail to work on ARM platform due to endianness. > > As an example, consider the following code snippet of > platform_do_lowpower function from arch/arm/mach-realview/hotplug.c: > > / * > > * here's the WFI > > */ > > asm(".word 0xe320f003\n" > > : > > : > > : "memory", "cc"); > > > > The instruction generated from this inline asm will not work on > big-endian ARM platform, such as ARM BE-8 format. Instead, an exception will > be generated. > > > > Here the code should be: > > / * > > * here's the WFI > > */ > > asm("WFI\n" > > : > > : > > : "memory", "cc"); > > > > Seems the kernel doesn't support ARM BE-8 well. I don't know why this > problem happens. > > Can anyone tell me who owns this part? I can prepare a patch then. > > Thanks. > > Questions regarding the ARM kernel should go to the linux-arm-kernel mailing > list > (see the MAINTAINERS file), with an optional cc: to the regular LKML. > > BE-8 is, if I recall correctly, ARMv7's broken format where code and data have > different endianess. GAS supports an ".inst" directive which is like ".word" > except the data is assumed to be code. This matters for disassembly, and may > also be required for BE-8. > > That is, just s/.word/.inst/g above and report back if that works or not. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > > Hi Mikael, Thanks for the reply. I modified the code as suggested and rebuilt the kernel, cpu-hotplug feature now works on big-endian(BE-8) ARM platform. Since the ARM core can be configured by system software to work in big-endian mode, it's necessary to fix this problem. And here is a small patch : diff -urN linux-3.6.2/arch/arm/mach-exynos/hotplug.c linux/arch/arm/mach-exynos/hotplug.c --- linux-3.6.2/arch/arm/mach-exynos/hotplug.c 2012-10-13 04:50:59.0 +0800 +++ linux/arch/arm/mach-exynos/hotplug.c2012-10-15 23:05:44.0 +0800 @@ -72,7 +72,7 @@ /* * here's the WFI */ - asm(".word 0xe320f003\n" + asm(".inst 0xe320f003\n" : : : "memory", "cc"); diff -urN linux-3.6.2/arch/arm/mach-realview/hotplug.c linux/arch/arm/mach-realview/hotplug.c --- linux-3.6.2/arch/arm/mach-realview/hotplug.c2012-10-13 04:50:59.0 +0800 +++ linux/arch/arm/mach-realview/hotplug.c 2012-10-15 23:05:00.0 +0800 @@ -66,7 +66,7 @@ /* * here's the WFI */ - asm(".word 0xe320f003\n" + asm(".inst 0xe320f003\n" : : : "memory", "cc"); diff -urN linux-3.6.2/arch/arm/mach-shmobile/hotplug.c linux/arch/arm/mach-shmobile/hotplug.c --- linux-3.6.2/arch/arm/mach-shmobile/hotplug.c2012-10-13 04:50:59.0 +0800 +++ linux/arch/arm/mach-shmobile/hotplug.c 2012-10-15 23:05:25.0 +0800 @@ -53,7 +53,7 @@ /* * here's the WFI */ - asm(".word 0xe320f003\n" + asm(".inst 0xe320f003\n" : : : "memory", "cc"); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Re: Hardcoded instruction causes certain features to fail on ARM platfrom due to endianness
2012/10/16 Dave Martin : > On Mon, Oct 15, 2012 at 11:33:08PM +0800, Fei Yang wrote: >> 2012/10/15 Mikael Pettersson : >> > Yangfei (Felix) writes: >> > > Hi all, >> > > >> > > I found that hardcoded instruction in inline asm can cause certains >> > certain features fail to work on ARM platform due to endianness. >> > > As an example, consider the following code snippet of >> > platform_do_lowpower function from arch/arm/mach-realview/hotplug.c: >> > > / * >> > > * here's the WFI >> > > */ >> > > asm(".word 0xe320f003\n" >> > > : >> > > : >> > > : "memory", "cc"); >> > > >> > > The instruction generated from this inline asm will not work on >> > big-endian ARM platform, such as ARM BE-8 format. Instead, an exception >> > will be generated. >> > > >> > > Here the code should be: >> > > / * >> > > * here's the WFI >> > > */ >> > > asm("WFI\n" >> > > : >> > > : >> > > : "memory", "cc"); >> > > >> > > Seems the kernel doesn't support ARM BE-8 well. I don't know why >> > this problem happens. >> > > Can anyone tell me who owns this part? I can prepare a patch then. >> > > Thanks. >> > >> > Questions regarding the ARM kernel should go to the linux-arm-kernel >> > mailing list >> > (see the MAINTAINERS file), with an optional cc: to the regular LKML. >> > >> > BE-8 is, if I recall correctly, ARMv7's broken format where code and data >> > have >> > different endianess. GAS supports an ".inst" directive which is like >> > ".word" >> > except the data is assumed to be code. This matters for disassembly, and >> > may >> > also be required for BE-8. >> > >> > That is, just s/.word/.inst/g above and report back if that works or not. >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> > the body of a message to majord...@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> > Please read the FAQ at http://www.tux.org/lkml/ >> > >> > >> >> Hi Mikael, >> >> Thanks for the reply. I modified the code as suggested and rebuilt the >> kernel, cpu-hotplug feature now works on big-endian(BE-8) ARM >> platform. >> Since the ARM core can be configured by system software to work in >> big-endian mode, it's necessary to fix this problem. And here is a >> small patch : >> >> diff -urN linux-3.6.2/arch/arm/mach-exynos/hotplug.c >> linux/arch/arm/mach-exynos/hotplug.c >> --- linux-3.6.2/arch/arm/mach-exynos/hotplug.c2012-10-13 >> 04:50:59.0 +0800 >> +++ linux/arch/arm/mach-exynos/hotplug.c 2012-10-15 23:05:44.0 >> +0800 >> @@ -72,7 +72,7 @@ >> /* >>* here's the WFI >>*/ >> - asm(".word 0xe320f003\n" >> + asm(".inst 0xe320f003\n" > > The cleanest fix would simply be to build these files with appropriate > modified CFLAGS (-march=armv6k or -march=armv7-a), and use the proper > "wfi" mnemonic. > > Failing that, you could use the facilities in to > declare a wrapper macro for injecting this opcode (see > for an example). However, putting custom > opcodes into the assembler should only be done if it's really > necessary. Nowadays, I think we can consider tools which don't > understand the WFI mnemonic to be obsolete, at least for platforms > which only build for v7 and above. > > The relevant board maintainers would need to sign off on such a > change, so we don't end up breaking their builds. > > If any of these boards needs to build for v6K, the custom opcode might > be worth it -- some people might just possibly be relying on older tools > for such platforms. > > Cheers > ---Dave > >> : >> : >> : "memory", "cc")
[RFC/PATCH] x86/irq: handle chained interrupts during IRQ migration
Chained interrupt handlers dont have an irqaction and hence are not handled during migrating interrupts when some cores go offline. Handle this by introducing a irq_is_chained() check which is based on the the CHAINED flag being set for such interrupts. fixup_irq() can then handle such interrupts and not skip them over. Signed-off-by: Sundar Iyer Signed-off-by: Fei Yang Cc: Fei Yang --- Issue was found with GPIO interrupts whose handlers are registered as chained handler, see drivers/gpio/gpio-langwell.c for details. These interrupts stop coming after offlining a few cpus, and never come back even after put the cpus back online. This patch had been integrated in the kernel for Intel's Android software platform, and proved to solve the issue. arch/x86/kernel/irq.c |4 +++- include/linux/irq.h |1 + include/linux/irqdesc.h |7 +++ kernel/irq/chip.c |1 + kernel/irq/settings.h |7 +++ 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 22d0687..3790ef5 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -286,7 +286,9 @@ void fixup_irqs(void) data = irq_desc_get_irq_data(desc); affinity = data->affinity; - if (!irq_has_action(irq) || irqd_is_per_cpu(data) || + /* include IRQs who have no action, but are chained */ + if ((!irq_has_action(irq) && !irq_is_chained(irq)) || + irqd_is_per_cpu(data) || cpumask_subset(affinity, cpu_online_mask)) { raw_spin_unlock(&desc->lock); continue; diff --git a/include/linux/irq.h b/include/linux/irq.h index 56bb0dc..9fef6ee 100644 --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -94,6 +94,7 @@ enum { IRQ_NESTED_THREAD = (1 << 15), IRQ_NOTHREAD= (1 << 16), IRQ_PER_CPU_DEVID = (1 << 17), + IRQ_CHAINED = (1 << 18), }; #define IRQF_MODIFY_MASK \ diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h index 56fb646..74d2b5e 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -121,6 +121,13 @@ static inline int irq_has_action(unsigned int irq) return desc->action != NULL; } +/* Test to see if the IRQ is chained */ +static inline int irq_is_chained(unsigned int irq) +{ + struct irq_desc *desc = irq_to_desc(irq); + return desc->status_use_accessors & IRQ_CHAINED; +} + /* caller has locked the irq_desc and both params are valid */ static inline void __irq_set_handler_locked(unsigned int irq, irq_flow_handler_t handler) diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c index a3bb14f..d0c4e4a 100644 --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -685,6 +685,7 @@ __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained, irq_settings_set_noprobe(desc); irq_settings_set_norequest(desc); irq_settings_set_nothread(desc); + irq_settings_set_chained(desc); irq_startup(desc, true); } out: diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h index 1162f10..4ea2f96 100644 --- a/kernel/irq/settings.h +++ b/kernel/irq/settings.h @@ -15,6 +15,7 @@ enum { _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD, _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID, _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK, + _IRQ_CHAINED= IRQ_CHAINED, }; #define IRQ_PER_CPUGOT_YOU_MORON @@ -28,6 +29,7 @@ enum { #define IRQ_PER_CPU_DEVID GOT_YOU_MORON #undef IRQF_MODIFY_MASK #define IRQF_MODIFY_MASK GOT_YOU_MORON +#define IRQ_CHAINEDGOT_YOU_MORON static inline void irq_settings_clr_and_set(struct irq_desc *desc, u32 clr, u32 set) @@ -147,3 +149,8 @@ static inline bool irq_settings_is_nested_thread(struct irq_desc *desc) { return desc->status_use_accessors & _IRQ_NESTED_THREAD; } + +static inline bool irq_settings_set_chained(struct irq_desc *desc) +{ + return desc->status_use_accessors |= _IRQ_CHAINED; +} -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 02/14] IOSF: Add interface for the cases requiring fid
From: Fei Yang Some implementations may require an additional step for setting the FID bits to ensure correct transactions over the IOSF side band interface. Add the FID support accordingly for such implementations Change-Id: Ic0227f9e74133a3203aa08e8471939f905d19622 Signed-off-by: Fei Yang --- arch/x86/include/asm/iosf_mbi.h| 27 ++ arch/x86/platform/intel/iosf_mbi.c | 73 ++ 2 files changed, 100 insertions(+) diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h index b72ad0f..1aae73a 100644 --- a/arch/x86/include/asm/iosf_mbi.h +++ b/arch/x86/include/asm/iosf_mbi.h @@ -8,6 +8,7 @@ #define MBI_MCR_OFFSET 0xD0 #define MBI_MDR_OFFSET 0xD4 #define MBI_MCRX_OFFSET0xD8 +#define MBI_MCRP_OFFSET0xDC #define MBI_RD_MASK0xFEFF #define MBI_WR_MASK0X0100 @@ -113,6 +114,32 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr); */ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask); +/** + * iosf_mbi_read_with_fid() - MailBox Interface read command requiring fid + * @fid: fid bits + * @port: port indicating subunit being accessed + * @opcode:port specific read or write opcode + * @offset:register address offset + * @mdr: register data to be read + * + * Locking is handled by spinlock - cannot sleep. + * Return: Nonzero on error + */ +int iosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr); + +/** + * iosf_mbi_write_with_fid() - MailBox unmasked write command requiring fid + * @fid: fid bits + * @port: port indicating subunit being accessed + * @opcode:port specific read or write opcode + * @offset:register address offset + * @mdr: register data to be written + * + * Locking is handled by spinlock - cannot sleep. + * Return: Nonzero on error + */ +int iosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 mdr); + #else /* CONFIG_IOSF_MBI is not enabled */ static inline bool iosf_mbi_available(void) diff --git a/arch/x86/platform/intel/iosf_mbi.c b/arch/x86/platform/intel/iosf_mbi.c index 56a8a47..cec2779 100644 --- a/arch/x86/platform/intel/iosf_mbi.c +++ b/arch/x86/platform/intel/iosf_mbi.c @@ -99,6 +99,24 @@ fail_write: return result; } +static int iosf_mbi_pci_write_fid(u32 fid) +{ + int result; + + if (!mbi_pdev) + return -ENODEV; + + result = pci_write_config_dword(mbi_pdev, MBI_MCRP_OFFSET, fid); + if (result < 0) + goto fail_fid_write; + + return 0; + +fail_fid_write: + dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result); + return result; +} + int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr) { u32 mcr, mcrx; @@ -184,6 +202,61 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) } EXPORT_SYMBOL(iosf_mbi_modify); +/* + * Some IP blocks require fid to access their registers. + * fid value is programmed through MCRP register, see above function + * iosf_mbi_pci_write_fid() for details. + */ +int iosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr) +{ + u32 mcr, mcrx; + unsigned long flags; + int ret; + + /*Access to the GFX unit is handled by GPU code */ + if (port == BT_MBI_UNIT_GFX) { + WARN_ON(1); + return -EPERM; + } + + mcr = iosf_mbi_form_mcr(opcode, port, offset & MBI_MASK_LO); + mcrx = offset & MBI_MASK_HI; + + spin_lock_irqsave(&iosf_mbi_lock, flags); + ret = iosf_mbi_pci_write_fid(fid); + if (!ret) + ret = iosf_mbi_pci_read_mdr(mcrx, mcr, mdr); + spin_unlock_irqrestore(&iosf_mbi_lock, flags); + + return ret; +} +EXPORT_SYMBOL(iosf_mbi_read_with_fid); + +int iosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 mdr) +{ + u32 mcr, mcrx; + unsigned long flags; + int ret; + + /*Access to the GFX unit is handled by GPU code */ + if (port == BT_MBI_UNIT_GFX) { + WARN_ON(1); + return -EPERM; + } + + mcr = iosf_mbi_form_mcr(opcode, port, offset & MBI_MASK_LO); + mcrx = offset & MBI_MASK_HI; + + spin_lock_irqsave(&iosf_mbi_lock, flags); + ret = iosf_mbi_pci_write_fid(fid); + if (!ret) + ret = iosf_mbi_pci_write_mdr(mcrx, mcr, mdr); + spin_unlock_irqrestore(&iosf_mbi_lock, flags); + + return ret; +} +EXPORT_SYMBOL(iosf_mbi_write_with_fid); + bool iosf_mbi_available(void) { /* Mbi isn't hot-pluggable. No remove routine is provided */ -- 1.9.1
[PATCH] IOSF: Add interface for the cases requiring fid
From: Fei Yang Some implementations may require an additional step for setting the FID bits to ensure correct transactions over the IOSF side band interface. Add the FID support accordingly for such implementations Change-Id: Ic0227f9e74133a3203aa08e8471939f905d19622 Signed-off-by: Fei Yang --- arch/x86/include/asm/iosf_mbi.h| 27 ++ arch/x86/platform/intel/iosf_mbi.c | 73 ++ 2 files changed, 100 insertions(+) diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h index b41ee16..71511ba 100644 --- a/arch/x86/include/asm/iosf_mbi.h +++ b/arch/x86/include/asm/iosf_mbi.h @@ -8,6 +8,7 @@ #define MBI_MCR_OFFSET 0xD0 #define MBI_MDR_OFFSET 0xD4 #define MBI_MCRX_OFFSET0xD8 +#define MBI_MCRP_OFFSET0xDC #define MBI_RD_MASK0xFEFF #define MBI_WR_MASK0X0100 @@ -88,6 +89,32 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr); */ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask); +/** + * iosf_mbi_read_with_fid() - MailBox Interface read command requiring fid + * @fid: fid bits + * @port: port indicating subunit being accessed + * @opcode:port specific read or write opcode + * @offset:register address offset + * @mdr: register data to be read + * + * Locking is handled by spinlock - cannot sleep. + * Return: Nonzero on error + */ +int iosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr); + +/** + * iosf_mbi_write_with_fid() - MailBox unmasked write command requiring fid + * @fid: fid bits + * @port: port indicating subunit being accessed + * @opcode:port specific read or write opcode + * @offset:register address offset + * @mdr: register data to be written + * + * Locking is handled by spinlock - cannot sleep. + * Return: Nonzero on error + */ +int iosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 mdr); + #else /* CONFIG_IOSF_MBI is not enabled */ static inline bool iosf_mbi_available(void) diff --git a/arch/x86/platform/intel/iosf_mbi.c b/arch/x86/platform/intel/iosf_mbi.c index edf2c54..af182c1 100644 --- a/arch/x86/platform/intel/iosf_mbi.c +++ b/arch/x86/platform/intel/iosf_mbi.c @@ -98,6 +98,24 @@ fail_write: return result; } +static int iosf_mbi_pci_write_fid(u32 fid) +{ + int result; + + if (!mbi_pdev) + return -ENODEV; + + result = pci_write_config_dword(mbi_pdev, MBI_MCRP_OFFSET, fid); + if (result < 0) + goto fail_fid_write; + + return 0; + +fail_fid_write: + dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result); + return result; +} + int iosf_mbi_read(u8 port, u8 opcode, u32 offset, u32 *mdr) { u32 mcr, mcrx; @@ -183,6 +201,61 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask) } EXPORT_SYMBOL(iosf_mbi_modify); +/* + * Some IP blocks require fid to access their registers. + * fid value is programmed through MCRP register, see above function + * iosf_mbi_pci_write_fid() for details. + */ +int iosf_mbi_read_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 *mdr) +{ + u32 mcr, mcrx; + unsigned long flags; + int ret; + + /*Access to the GFX unit is handled by GPU code */ + if (port == BT_MBI_UNIT_GFX) { + WARN_ON(1); + return -EPERM; + } + + mcr = iosf_mbi_form_mcr(opcode, port, offset & MBI_MASK_LO); + mcrx = offset & MBI_MASK_HI; + + spin_lock_irqsave(&iosf_mbi_lock, flags); + ret = iosf_mbi_pci_write_fid(fid); + if (!ret) + ret = iosf_mbi_pci_read_mdr(mcrx, mcr, mdr); + spin_unlock_irqrestore(&iosf_mbi_lock, flags); + + return ret; +} +EXPORT_SYMBOL(iosf_mbi_read_with_fid); + +int iosf_mbi_write_with_fid(u32 fid, u8 port, u8 opcode, u32 offset, u32 mdr) +{ + u32 mcr, mcrx; + unsigned long flags; + int ret; + + /*Access to the GFX unit is handled by GPU code */ + if (port == BT_MBI_UNIT_GFX) { + WARN_ON(1); + return -EPERM; + } + + mcr = iosf_mbi_form_mcr(opcode, port, offset & MBI_MASK_LO); + mcrx = offset & MBI_MASK_HI; + + spin_lock_irqsave(&iosf_mbi_lock, flags); + ret = iosf_mbi_pci_write_fid(fid); + if (!ret) + ret = iosf_mbi_pci_write_mdr(mcrx, mcr, mdr); + spin_unlock_irqrestore(&iosf_mbi_lock, flags); + + return ret; +} +EXPORT_SYMBOL(iosf_mbi_write_with_fid); + bool iosf_mbi_available(void) { /* Mbi isn't hot-pluggable. No remove routine is provided */ -- 1.9.1
[PATCH v3] usb: gadget: f_fs: don't free buffer prematurely
From: Fei Yang The following kernel panic happens due to the io_data buffer gets deallocated before the async io is completed. Add a check for the case where io_data buffer should be deallocated by ffs_user_copy_worker. [ 41.663334] BUG: unable to handle kernel NULL pointer dereference at 0048 [ 41.672099] #PF error: [normal kernel read fault] [ 41.677356] PGD 20c974067 P4D 20c974067 PUD 20c973067 PMD 0 [ 41.683687] Oops: [#1] PREEMPT SMP [ 41.687976] CPU: 1 PID: 7 Comm: kworker/u8:0 Tainted: G U 5.0.0-quilt-2e5dc0ac-00790-gd8c79f2-dirty #2 [ 41.705309] Workqueue: adb ffs_user_copy_worker [ 41.705316] RIP: 0010:__vunmap+0x2a/0xc0 [ 41.705318] Code: 0f 1f 44 00 00 48 85 ff 0f 84 87 00 00 00 55 f7 c7 ff 0f 00 00 48 89 e5 41 55 41 89 f5 41 54 53 48 89 fb 75 71 e8 56 d7 ff ff <4c> 8b 60 48 4d 85 e4 74 76 48 89 df e8 25 ff ff ff 45 85 ed 74 46 [ 41.705320] RSP: 0018:bc3a40053df0 EFLAGS: 00010286 [ 41.705322] RAX: RBX: bc3a406f1000 RCX: [ 41.705323] RDX: 0001 RSI: 0001 RDI: [ 41.705324] RBP: bc3a40053e08 R08: 0001fb79 R09: 0037 [ 41.705325] R10: bc3a40053b68 R11: bc3a40053cad R12: fff2 [ 41.705326] R13: 0001 R14: R15: [ 41.705328] FS: () GS:9e2977a8() knlGS: [ 41.705329] CS: 0010 DS: ES: CR0: 80050033 [ 41.705330] CR2: 0048 CR3: 00020c994000 CR4: 003406e0 [ 41.705331] Call Trace: [ 41.705338] vfree+0x50/0xb0 [ 41.705341] ffs_user_copy_worker+0xe9/0x1c0 [ 41.705344] process_one_work+0x19f/0x3e0 [ 41.705348] worker_thread+0x3f/0x3b0 [ 41.829766] kthread+0x12b/0x150 [ 41.833371] ? process_one_work+0x3e0/0x3e0 [ 41.838045] ? kthread_create_worker_on_cpu+0x70/0x70 [ 41.843695] ret_from_fork+0x3a/0x50 [ 41.847689] Modules linked in: hci_uart bluetooth ecdh_generic rfkill_gpio dwc3_pci dwc3 snd_usb_audio mei_me tpm_crb snd_usbmidi_lib xhci_pci xhci_hcd mei tpm snd_hwdep cfg80211 snd_soc_skl snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp snd_hda_ext_core snd_hda_core videobuf2_dma_sg crlmodule [ 41.876880] CR2: 0048 [ 41.880584] ---[ end trace 2bc4addff0f2e673 ]--- [ 41.891346] RIP: 0010:__vunmap+0x2a/0xc0 [ 41.895734] Code: 0f 1f 44 00 00 48 85 ff 0f 84 87 00 00 00 55 f7 c7 ff 0f 00 00 48 89 e5 41 55 41 89 f5 41 54 53 48 89 fb 75 71 e8 56 d7 ff ff <4c> 8b 60 48 4d 85 e4 74 76 48 89 df e8 25 ff ff ff 45 85 ed 74 46 [ 41.916740] RSP: 0018:bc3a40053df0 EFLAGS: 00010286 [ 41.922583] RAX: RBX: bc3a406f1000 RCX: [ 41.930563] RDX: 0001 RSI: 0001 RDI: [ 41.938540] RBP: bc3a40053e08 R08: 0001fb79 R09: 0037 [ 41.946520] R10: bc3a40053b68 R11: bc3a40053cad R12: fff2 [ 41.954502] R13: 0001 R14: R15: [ 41.962482] FS: () GS:9e2977a8() knlGS: [ 41.971536] CS: 0010 DS: ES: CR0: 80050033 [ 41.977960] CR2: 0048 CR3: 00020c994000 CR4: 003406e0 [ 41.985930] Kernel panic - not syncing: Fatal exception [ 41.991817] Kernel Offset: 0x1600 from 0x8100 (relocation range: 0x8000-0xbfff) [ 42.009525] Rebooting in 10 seconds.. [ 52.014376] ACPI MEMORY or I/O RESET_REG. Fixes: 772a7a724f6 ("usb: gadget: f_fs: Allow scatter-gather buffers") Signed-off-by: Fei Yang Reviewed-by: Manu Gautam Tested-by: John Stultz --- v2: add tag: "Fixes: 772a7a724f6 ..", Reviewed-by and Tested-by. v3: check data for NULL instead of "ret == -EIOCBQUEUED", which would be safer and keep the original logic intact. --- drivers/usb/gadget/function/f_fs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 20413c2..0bce731 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1133,7 +1133,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) error_mutex: mutex_unlock(&epfile->mutex); error: - ffs_free_buffer(io_data); + if (data) /* don't free if there is iocb queued */ + ffs_free_buffer(io_data); return ret; } -- 2.7.4
[PATCH] usb: gadget: f_fs: data_len used before properly set
From: Fei Yang The following line of code in function ffs_epfile_io is trying to set flag io_data->use_sg in case buffer required is larger than one page. io_data->use_sg = gadget->sg_supported && data_len > PAGE_SIZE; However at this point of time the variable data_len has not been set to the proper buffer size yet. The consequence is that io_data->use_sg is always set regardless what buffer size really is, because the condition (data_len > PAGE_SIZE) is effectively an unsigned comparison between -EINVAL and PAGE_SIZE which would always result in TRUE. Fixes: 772a7a724f69 ("usb: gadget: f_fs: Allow scatter-gather buffers") Signed-off-by: Fei Yang Cc: stable --- drivers/usb/gadget/function/f_fs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 47be961..c7ed900 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -997,7 +997,6 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) * earlier */ gadget = epfile->ffs->gadget; - io_data->use_sg = gadget->sg_supported && data_len > PAGE_SIZE; spin_lock_irq(&epfile->ffs->eps_lock); /* In the meantime, endpoint got disabled or changed. */ @@ -1012,6 +1011,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) */ if (io_data->read) data_len = usb_ep_align_maybe(gadget, ep->ep, data_len); + + io_data->use_sg = gadget->sg_supported && data_len > PAGE_SIZE; spin_unlock_irq(&epfile->ffs->eps_lock); data = ffs_alloc_buffer(io_data, data_len); -- 2.7.4