From: Nicolas Pitre <nico@fluxnic.net> To: Arnd Bergmann <arnd@arndb.de> Cc: linux-arm-kernel@lists.infradead.org, lkml <linux-kernel@vger.kernel.org>, Russell King - ARM Linux <linux@arm.linux.org.uk> Subject: Re: Dynamic patching in discarded sections Date: Wed, 08 Jun 2011 10:15:02 -0400 (EDT) [thread overview] Message-ID: <alpine.LFD.2.00.1106080946000.2142@xanadu.home> (raw) In-Reply-To: <201106081123.59414.arnd@arndb.de> On Wed, 8 Jun 2011, Arnd Bergmann wrote: > I've been playing with randconfig builds and found an interesting problem > with the combination of: > > CONFIG_ARM_PATCH_PHYS_VIRT=y > CONFIG_HOTPLUG=n > CONFIG_DMABOUNCE=n > CONFIG_MMC_SPI=y > > The problem is shown with this code: > > static int __devexit mmc_spi_remove(struct spi_device *spi) > { > ... > dma_unmap_single(host->dma_dev, host->ones_dma, > MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE); > dma_unmap_single(host->dma_dev, host->data_dma, > sizeof(*host->data), DMA_BIDIRECTIONAL); > ... > } > > and the error message (in case someone looks for this using google) is > > `.devexit.text' referenced in section `.pv_table' of drivers/built-in.o: defined in discarded section `.devexit.text' of drivers/built-in.o > `.devexit.text' referenced in section `.pv_table' of drivers/built-in.o: defined in discarded section `.devexit.text' of drivers/built-in.o > > What happens is that dma_unmap_single() calls > __dma_single_dev_to_cpu(dma_to_virt(dev, handle), size, dir), which requires > patching in the caller. However, due to CONFIG_HOTPLUG being disabled, the > __devexit section gets discarded, and the linker cannot create an entry in the > .pvtable section for the mmc_spi_remove function. We really need to push for better toolchain support here. This could easily be solved with some way to query the currently active section from the assembler and use that to construct the argument to the .pushsection directive. This way, the .pvtable section could become .pvtable.__devexit when the reference is made from a __devexit section, and that could be discarded altogether at link time if unneeded. Dave Martin proposed an extension to gas here (also sent to binutils@sourceware.org): http://article.gmane.org/gmane.linux.linaro.toolchain/701 But as far as I know, nothing further happened. And this is not an ARM specific issue either as the X86 alternates have the same problem. > I don't know if the same problem exists in other places in the code, but it's > entirely possible. I also couldn't think of a good solution for this, short of > moving the definition of dma_unmap_single() to out of line code. Probably the best interim solution would be: diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S index dfbb377..f231c92 100644 --- a/arch/arm/kernel/vmlinux.lds.S +++ b/arch/arm/kernel/vmlinux.lds.S @@ -21,7 +21,8 @@ #define ARM_CPU_KEEP(x) #endif -#if defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK) +#if (defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK)) || \ + defined(CONFIG_ARM_PATCH_PHYS_VIRT) #define ARM_EXIT_KEEP(x) x #else #define ARM_EXIT_KEEP(x) But clearly the toolchain should be more accommodating instead. Nicolas
WARNING: multiple messages have this Message-ID (diff)
From: nico@fluxnic.net (Nicolas Pitre) To: linux-arm-kernel@lists.infradead.org Subject: Dynamic patching in discarded sections Date: Wed, 08 Jun 2011 10:15:02 -0400 (EDT) [thread overview] Message-ID: <alpine.LFD.2.00.1106080946000.2142@xanadu.home> (raw) In-Reply-To: <201106081123.59414.arnd@arndb.de> On Wed, 8 Jun 2011, Arnd Bergmann wrote: > I've been playing with randconfig builds and found an interesting problem > with the combination of: > > CONFIG_ARM_PATCH_PHYS_VIRT=y > CONFIG_HOTPLUG=n > CONFIG_DMABOUNCE=n > CONFIG_MMC_SPI=y > > The problem is shown with this code: > > static int __devexit mmc_spi_remove(struct spi_device *spi) > { > ... > dma_unmap_single(host->dma_dev, host->ones_dma, > MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE); > dma_unmap_single(host->dma_dev, host->data_dma, > sizeof(*host->data), DMA_BIDIRECTIONAL); > ... > } > > and the error message (in case someone looks for this using google) is > > `.devexit.text' referenced in section `.pv_table' of drivers/built-in.o: defined in discarded section `.devexit.text' of drivers/built-in.o > `.devexit.text' referenced in section `.pv_table' of drivers/built-in.o: defined in discarded section `.devexit.text' of drivers/built-in.o > > What happens is that dma_unmap_single() calls > __dma_single_dev_to_cpu(dma_to_virt(dev, handle), size, dir), which requires > patching in the caller. However, due to CONFIG_HOTPLUG being disabled, the > __devexit section gets discarded, and the linker cannot create an entry in the > .pvtable section for the mmc_spi_remove function. We really need to push for better toolchain support here. This could easily be solved with some way to query the currently active section from the assembler and use that to construct the argument to the .pushsection directive. This way, the .pvtable section could become .pvtable.__devexit when the reference is made from a __devexit section, and that could be discarded altogether at link time if unneeded. Dave Martin proposed an extension to gas here (also sent to binutils at sourceware.org): http://article.gmane.org/gmane.linux.linaro.toolchain/701 But as far as I know, nothing further happened. And this is not an ARM specific issue either as the X86 alternates have the same problem. > I don't know if the same problem exists in other places in the code, but it's > entirely possible. I also couldn't think of a good solution for this, short of > moving the definition of dma_unmap_single() to out of line code. Probably the best interim solution would be: diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S index dfbb377..f231c92 100644 --- a/arch/arm/kernel/vmlinux.lds.S +++ b/arch/arm/kernel/vmlinux.lds.S @@ -21,7 +21,8 @@ #define ARM_CPU_KEEP(x) #endif -#if defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK) +#if (defined(CONFIG_SMP_ON_UP) && !defined(CONFIG_DEBUG_SPINLOCK)) || \ + defined(CONFIG_ARM_PATCH_PHYS_VIRT) #define ARM_EXIT_KEEP(x) x #else #define ARM_EXIT_KEEP(x) But clearly the toolchain should be more accommodating instead. Nicolas
next prev parent reply other threads:[~2011-06-08 14:15 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-06-08 9:23 Dynamic patching in discarded sections Arnd Bergmann 2011-06-08 9:23 ` Arnd Bergmann 2011-06-08 14:15 ` Nicolas Pitre [this message] 2011-06-08 14:15 ` Nicolas Pitre 2011-06-16 15:21 ` Arnd Bergmann 2011-06-16 15:21 ` Arnd Bergmann 2011-06-16 15:25 ` Arnd Bergmann 2011-06-16 15:25 ` Arnd Bergmann 2011-06-16 17:07 ` Dave Martin 2011-06-16 17:07 ` Dave Martin 2011-06-16 17:41 ` Nicolas Pitre 2011-06-16 17:41 ` Nicolas Pitre
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.LFD.2.00.1106080946000.2142@xanadu.home \ --to=nico@fluxnic.net \ --cc=arnd@arndb.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.