* [PATCH v2 12/16] openrisc: Use INITRAMFS_GENERIC_UNLOAD
From: Shea Levy @ 2018-03-25 22:18 UTC (permalink / raw)
To: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel
Cc: Shea Levy
In-Reply-To: <20180325221853.10839-1-shea@shealevy.com>
Signed-off-by: Shea Levy <shea@shealevy.com>
---
arch/openrisc/Kconfig | 1 +
arch/openrisc/mm/init.c | 7 -------
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index dfb6a79ba7ff..0f8d2132baa5 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -36,6 +36,7 @@ config OPENRISC
select OMPIC if SMP
select ARCH_WANT_FRAME_POINTERS
select GENERIC_IRQ_MULTI_HANDLER
+ select INITRAMFS_GENERIC_UNLOAD
config CPU_BIG_ENDIAN
def_bool y
diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
index 6972d5d6f23f..c1a3dcf9ad40 100644
--- a/arch/openrisc/mm/init.c
+++ b/arch/openrisc/mm/init.c
@@ -222,13 +222,6 @@ void __init mem_init(void)
return;
}
-#ifdef CONFIG_BLK_DEV_INITRD
-void free_initrd_mem(unsigned long start, unsigned long end)
-{
- free_reserved_area((void *)start, (void *)end, -1, "initrd");
-}
-#endif
-
void free_initmem(void)
{
free_initmem_default(-1);
--
2.16.2
^ permalink raw reply related
* [PATCH v2 13/16] parisc: Use INITRAMFS_GENERIC_UNLOAD
From: Shea Levy @ 2018-03-25 22:18 UTC (permalink / raw)
To: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel
Cc: Shea Levy
In-Reply-To: <20180325221853.10839-1-shea@shealevy.com>
Signed-off-by: Shea Levy <shea@shealevy.com>
---
arch/parisc/Kconfig | 1 +
arch/parisc/mm/init.c | 7 -------
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 9792d8cf4f56..7410c2094987 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -51,6 +51,7 @@ config PARISC
select GENERIC_CLOCKEVENTS
select ARCH_NO_COHERENT_DMA_MMAP
select CPU_NO_EFFICIENT_FFS
+ select INITRAMFS_GENERIC_UNLOAD
help
The PA-RISC microprocessor is designed by Hewlett-Packard and used
diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
index cab32ee824d2..3643399230f3 100644
--- a/arch/parisc/mm/init.c
+++ b/arch/parisc/mm/init.c
@@ -932,10 +932,3 @@ void flush_tlb_all(void)
spin_unlock(&sid_lock);
}
#endif
-
-#ifdef CONFIG_BLK_DEV_INITRD
-void free_initrd_mem(unsigned long start, unsigned long end)
-{
- free_reserved_area((void *)start, (void *)end, -1, "initrd");
-}
-#endif
--
2.16.2
^ permalink raw reply related
* [PATCH v2 14/16] powerpc: Use INITRAMFS_GENERIC_UNLOAD
From: Shea Levy @ 2018-03-25 22:18 UTC (permalink / raw)
To: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel
Cc: Shea Levy
In-Reply-To: <20180325221853.10839-1-shea@shealevy.com>
Signed-off-by: Shea Levy <shea@shealevy.com>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/mm/mem.c | 7 -------
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 73ce5dd07642..8cf384068e79 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -223,6 +223,7 @@ config PPC
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_VIRT_CPU_ACCOUNTING
select HAVE_IRQ_TIME_ACCOUNTING
+ select INITRAMFS_GENERIC_UNLOAD
select IRQ_DOMAIN
select IRQ_FORCED_THREADING
select MODULES_USE_ELF_RELA
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index fe8c61149fb8..e85b2a3cd264 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -404,13 +404,6 @@ void free_initmem(void)
free_initmem_default(POISON_FREE_INITMEM);
}
-#ifdef CONFIG_BLK_DEV_INITRD
-void __init free_initrd_mem(unsigned long start, unsigned long end)
-{
- free_reserved_area((void *)start, (void *)end, -1, "initrd");
-}
-#endif
-
/*
* This is called when a page has been modified by the kernel.
* It just marks the page as not i-cache clean. We do the i-cache
--
2.16.2
^ permalink raw reply related
* [PATCH v2 15/16] sh: Use INITRAMFS_GENERIC_UNLOAD
From: Shea Levy @ 2018-03-25 22:18 UTC (permalink / raw)
To: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel
Cc: Shea Levy
In-Reply-To: <20180325221853.10839-1-shea@shealevy.com>
Signed-off-by: Shea Levy <shea@shealevy.com>
---
arch/sh/Kconfig | 1 +
arch/sh/mm/init.c | 7 -------
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 97fe29316476..b6f80dad2152 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -50,6 +50,7 @@ config SUPERH
select HAVE_ARCH_AUDITSYSCALL
select HAVE_FUTEX_CMPXCHG if FUTEX
select HAVE_NMI
+ select INITRAMFS_GENERIC_UNLOAD
help
The SuperH is a RISC processor targeted for use in embedded systems
and consumer electronics; it was also used in the Sega Dreamcast
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index ce0bbaa7e404..7451459d0725 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -477,13 +477,6 @@ void free_initmem(void)
free_initmem_default(-1);
}
-#ifdef CONFIG_BLK_DEV_INITRD
-void free_initrd_mem(unsigned long start, unsigned long end)
-{
- free_reserved_area((void *)start, (void *)end, -1, "initrd");
-}
-#endif
-
#ifdef CONFIG_MEMORY_HOTPLUG
int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
bool want_memblock)
--
2.16.2
^ permalink raw reply related
* [PATCH v2 16/16] um: Use INITRAMFS_GENERIC_UNLOAD
From: Shea Levy @ 2018-03-25 22:18 UTC (permalink / raw)
To: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel
Cc: Shea Levy
In-Reply-To: <20180325221853.10839-1-shea@shealevy.com>
Signed-off-by: Shea Levy <shea@shealevy.com>
---
arch/um/Kconfig.common | 1 +
arch/um/kernel/mem.c | 7 -------
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/um/Kconfig.common b/arch/um/Kconfig.common
index c68add8df3ae..1cb8a023938b 100644
--- a/arch/um/Kconfig.common
+++ b/arch/um/Kconfig.common
@@ -13,6 +13,7 @@ config UML
select GENERIC_CLOCKEVENTS
select HAVE_GCC_PLUGINS
select TTY # Needed for line.c
+ select INITRAMFS_GENERIC_UNLOAD
config MMU
bool
diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c
index 3c0e470ea646..2d26eec92126 100644
--- a/arch/um/kernel/mem.c
+++ b/arch/um/kernel/mem.c
@@ -170,13 +170,6 @@ void free_initmem(void)
{
}
-#ifdef CONFIG_BLK_DEV_INITRD
-void free_initrd_mem(unsigned long start, unsigned long end)
-{
- free_reserved_area((void *)start, (void *)end, -1, "initrd");
-}
-#endif
-
/* Allocate and free page tables. */
pgd_t *pgd_alloc(struct mm_struct *mm)
--
2.16.2
^ permalink raw reply related
* Re: [PATCH 01/16] initrd: Add generic code path for common initrd unloading logic.
From: Shea Levy @ 2018-03-25 22:20 UTC (permalink / raw)
To: LEROY Christophe
Cc: user-mode-linux-devel, linux-sh, linux-riscv, linuxppc-dev,
linux-parisc, openrisc, nios2-dev, linux-m68k, uclinux-h8-devel,
linux-c6x-dev, linux-snps-arc, linux-kernel, linux-alpha
In-Reply-To: <20180325191740.Horde.Cymi0yr0xcQQV1s42wkNkg2@messagerie.si.c-s.fr>
[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]
Hi Christophe,
LEROY Christophe <christophe.leroy@c-s.fr> writes:
> Shea Levy <shea@shealevy.com> a écrit :
>
>> Signed-off-by: Shea Levy <shea@shealevy.com>
>> ---
>> init/initramfs.c | 7 +++++++
>> usr/Kconfig | 4 ++++
>> 2 files changed, 11 insertions(+)
>>
>> diff --git a/init/initramfs.c b/init/initramfs.c
>> index 7e99a0038942..de5ce873eb5a 100644
>> --- a/init/initramfs.c
>> +++ b/init/initramfs.c
>> @@ -526,6 +526,13 @@ extern unsigned long __initramfs_size;
>> #include <linux/initrd.h>
>> #include <linux/kexec.h>
>>
>> +#ifdef CONFIG_INITRAMFS_GENERIC_UNLOAD
>> +void free_initrd_mem(unsigned long start, unsigned long end)
>> +{
>> + free_reserved_area((void *)start, (void *)end, -1, "initrd");
>> +}
>> +#endif
>
> In powerpc this was an __init function. Why not also put the generic
> one in __init section ?
>
v2 series sent, thanks!
>
> Christophe
>
>
>> +
>> static void __init free_initrd(void)
>> {
>> #ifdef CONFIG_KEXEC_CORE
>> diff --git a/usr/Kconfig b/usr/Kconfig
>> index 43658b8a975e..fd79d4d6fa26 100644
>> --- a/usr/Kconfig
>> +++ b/usr/Kconfig
>> @@ -233,3 +233,7 @@ config INITRAMFS_COMPRESSION
>> default ".lzma" if RD_LZMA
>> default ".bz2" if RD_BZIP2
>> default ""
>> +
>> +# Arches can select this for a generic initrd unloading codepath
>> +config INITRAMFS_GENERIC_UNLOAD
>> + bool
>> --
>> 2.16.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH 1/6] libnvdimm: Add of_node to region and bus descriptors
From: Balbir Singh @ 2018-03-25 23:16 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linux-nvdimm, devicetree, linuxppc-dev
In-Reply-To: <20180323081209.31387-1-oohall@gmail.com>
On Fri, 23 Mar 2018 19:12:04 +1100
Oliver O'Halloran <oohall@gmail.com> wrote:
> We want to be able to cross reference the region and bus devices
> with the device tree node that they were spawned from. libNVDIMM
> handles creating the actual devices for these internally, so we
> need to pass in a pointer to the relevant node in the descriptor.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> ---
Acked-by: Balbir Singh <bsingharora@gmail.com>
^ permalink raw reply
* Re: [PATCH 2/6] libnvdimm: Add nd_region_destroy()
From: Balbir Singh @ 2018-03-25 23:24 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linux-nvdimm, devicetree, linuxppc-dev
In-Reply-To: <20180323081209.31387-2-oohall@gmail.com>
On Fri, 23 Mar 2018 19:12:05 +1100
Oliver O'Halloran <oohall@gmail.com> wrote:
> Currently there's no way to remove a region from and nvdimm_bus without
> tearing down the whole bus. This patch adds an API for removing a single
> region from the bus so that we can implement a sensible unbind operation
> for the of_nd_region platform driver.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> drivers/nvdimm/region_devs.c | 6 ++++++
> include/linux/libnvdimm.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index 2f1d5771100e..76f46fd1fae0 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -1039,6 +1039,12 @@ struct nd_region *nvdimm_blk_region_create(struct nvdimm_bus *nvdimm_bus,
> }
> EXPORT_SYMBOL_GPL(nvdimm_blk_region_create);
>
> +void nd_region_destroy(struct nd_region *region)
> +{
> + nd_device_unregister(®ion->dev, ND_SYNC);
child_unregister seems to do the same thing, but is expected to be used
as a callback from device_for_each_child()
I'd suggest we merge the two and rename child_unregister and nd_region_unregister
Balbir Singh.
^ permalink raw reply
* Re: DMA Mapping Error in ppc64
From: Oliver @ 2018-03-25 23:27 UTC (permalink / raw)
To: Jared Bents; +Cc: linuxppc-dev
In-Reply-To: <CAM=7fberG6pwy5kz1h=xnNWO4AeSbCR-N-Yw079qMB1tnhSzvQ@mail.gmail.com>
On Fri, Mar 23, 2018 at 11:41 PM, Jared Bents
<jared.bents@rockwellcollins.com> wrote:
> Thank you for the advice. Looks like I get to try to rewrite the ath9k and
> ath10k drivers to use dma_alloc_coherent() instead of kmemdup() and
> dev_alloc_skb()
I don't think you need to go that far. It looks like you might be able
to fix the uses of kmemdup() and kzalloc() in
ath10k_pci_hif_exchange_bmi_msg() and call it a day. Auditing the
other uses of dma_map_single() to see if they're using kmalloc()
memory might be a good idea too.
Anyway this is probably something you're better off taking to the ath10k list.
Thanks,
Oliver
>
> On Thu, Mar 22, 2018 at 8:19 PM, Oliver <oohall@gmail.com> wrote:
>>
>> On Fri, Mar 23, 2018 at 1:37 AM, Jared Bents
>> <jared.bents@rockwellcollins.com> wrote:
>> > Thank you for the response but unfortunately, it looks like I already
>> > have that and it is being used. To verify, I commented that out and
>> > got the failure "dma_direct_alloc_coherent: No suitable zone for pfn
>> > 0xe0000". Below is the code flow for function
>> > ath10k_pci_hif_exchange_bmi_msg which is showing the first dma mapping
>> > error.
>> >
>> > ath10k_pci_hif_exchange_bmi_msg -> dma_map_single ->
>> > dma_map_single_attrs -> swiotlb_map_page -> dma_capable (returns
>> > false)
>> >
>> >
>> > dma_capable is what reports the failure in that flow.
>> >
>> > static inline bool dma_capable(struct device *dev, dma_addr_t addr,
>> > size_t size)
>> > {
>> > #ifdef CONFIG_SWIOTLB
>> > struct dev_archdata *sd = &dev->archdata;
>> >
>> > if (sd->max_direct_dma_addr && addr + size > sd->max_direct_dma_addr)
>> > return false;
>> > #endif
>> >
>> > if (!dev->dma_mask)
>> > return false;
>> >
>> > return addr + size - 1 <= *dev->dma_mask;
>> > }
>> > Getting the below values:
>> > addr = 1ee376218
>> > size = 4
>> > sd->max_direct_dma_addr = e0000000 which is I believe DMA window size
>> > (e0000000)
>> >
>> > when executed sd->max_direct_dma_addr(e0000000) && addr(1ee376218) +
>> > size(4) becomes e0000004 which is > sd->max_direct_dma_addr (e0000000)
>> >
>> >
>> > So even though limit_zone_pfn(ZONE_DMA32, 1UL << (31 - PAGE_SHIFT)) is
>> > being used in arch/powerpc/platforms/85xx/corenet_generic.c,
>>
>> > kmemdup(req, req_len, GFP_KERNEL) is returning an address that when
>> > sent to dma_map_single(), results in a bad map.
>>
>> You need to use (GFP_KERNEL | GFP_DMA32) to constrain the allocations
>> to ZONE_DMA32. Without that the kmemdup() will allocate from any zone
>> so you'll probably get an unmappable address.
>>
>> That said, the driver probably shouldn't be using kmemdup() here.
>> DMA-API.txt pretty explicitly says that drivers should not assume that
>> dma_map_single() will work with arbitrary memory. It should be using
>> dma_alloc_coherent() or a dma pool here.
>>
>> > - Jared
>> >
>> > On Wed, Mar 21, 2018 at 11:54 PM, Oliver <oohall@gmail.com> wrote:
>> >> On Thu, Mar 22, 2018 at 8:00 AM, Jared Bents
>> >> <jared.bents@rockwellcollins.com> wrote:
>> >>> Hi all,
>> >>>
>> >>> Apologies for the amount of information but we've been debugging this
>> >>> for a while and I wanted to get what we are seeing captured as much as
>> >>> possible. We are a T1042 processor and have a total 8GB DDR and our
>> >>> kernel version is fsl-sdk-v2.0-1703 (linux v4.1.35) as that is the
>> >>> latest version supplied by NXP.
>> >>>
>> >>> A while ago we ported from 32 bit to 64 bit. Everything continued to
>> >>> work except the ath10k module we have. So as a first step, we checked
>> >>> to see if an ath9k module also failed to work and it was also no
>> >>> longer working. The ath10k is working fine on a 32 bit system but
>> >>> it's not working on 64 bit system as we are getting dma mapping errors
>> >>> when trying to initialize the wifi modules.
>> >>>
>> >>> pci_bus 0002:01: bus scan returning with max=01
>> >>> pci_bus 0002:01: busn_res: [bus 01] end is updated to 01
>> >>> pci_bus 0002:00: bus scan returning with max=01
>> >>> ath10k_pci 0000:01:00.0: unable to get target info from device
>> >>> ath10k_pci 0000:01:00.0: could not get target info (-5)
>> >>> ath10k_pci 0000:01:00.0: could not probe fw (-5)
>> >>> ath10k_pci 0001:01:00.0: Direct firmware load for
>> >>> ath10k/cal-pci-0001:01:00.0.bin failed with error -2
>> >>>
>> >>>
>> >>> First, we have tried the mainline kernel (v4.15) to see if that would
>> >>> fix the issue, it did not. So I made a patch for the ath10k driver to
>> >>> restrict to just GFP_DMA areas when allocating memory or creating
>> >>> sk_buffs and have attached it. The ath10k wifi modules now initialize
>> >>> correctly but when I try to connect them and send traffic, they get a
>> >>> DMA mapping error from the sk_buff that it receives from elsewhere in
>> >>> the kernel. So while the driver appears to be fixable with the patch,
>> >>> the modules are still unusable due to data being sent to the driver
>> >>> when ath10k_tx is called and it tries to dma map with the provided
>> >>> skb. Also, according to the ath10k mailing list, GFP_DMA is not
>> >>> supposed to be used in general. The error below is the same sort of
>> >>> dma mapping error that is seen when initializing the modules without
>> >>> the patch to OR with GFP_DMA.
>> >>>
>> >>> ath10k_pci 0001:01:00.0: failed to transmit packet, dropping: -5
>> >>>
>> >>>
>> >>> We asked on the ath10k mailing list if anyone else is having this
>> >>> problem and no one else seems to have the issue but they are using
>> >>> different architectures (ARM or X86). As a result, it does not seem to
>> >>> be a driver issue to us but something within the PowerPC arch. So we
>> >>> dug a little deeper to try to find what addresses being mapped are
>> >>> working and what address being mapped are not working.
>> >>>
>> >>> We found that when the virtual address of data pointer (a member of
>> >>> sk_buff) is above ~3.7 GB RAM address range then return address from
>> >>> dma_map_single API is failed to validate in dma_mapping_error
>> >>> function.
>> >>>
>> >>> We also noticed that in a 64bit machine sometimes ping is working and
>> >>> because of the virtual address is under ~3.7GAM RAM address range. So
>> >>> if we set mem=2048M in the bootargs, the ath10k module works
>> >>> perfectly, however this isn't a real solution since it cuts our
>> >>> available RAM from 8GB to 2GB.
>> >>
>> >> I think there's a known issue with the freescale PCIe root complex
>> >> where it can't DMA beyond the 4GB mark. There's a workaround in
>> >> the form of limit_zone_pfn() which you can use to put the lower 4GB
>> >> into
>> >> ZONE_DMA32 and allocate from there rather than ZONE_NORMAL.
>> >> For details of how to use it have a look at corenet_gen_setup_arch() in
>> >> arch/powerpc/platforms/85xx/corenet_generic.c
>> >>
>> >> Hope that helps,
>> >> Oliver
>
>
^ permalink raw reply
* Re: [PATCH] powerpc/eeh: Fix race with driver un/bind
From: Russell Currey @ 2018-03-25 23:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michael Neuling, mpe; +Cc: linuxppc-dev, sam.bobroff
In-Reply-To: <1521786791.16434.339.camel@kernel.crashing.org>
On Fri, 2018-03-23 at 17:33 +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2018-03-23 at 16:44 +1100, Michael Neuling wrote:
>
> .../...
>
> > This fixes the problem in the same way the generic PCIe AER code
> > (in
> > drivers/pci/pcie/aer/aerdrv_core.c) does. It makes the EEH code
> > hold
> > the device_lock() before performing the driver EEH callbacks. This
> > ensures either the callbacks are no longer register, or if they are
> > registered the driver will not be removed from underneath us.
> >
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
>
> Generally ok, minor nits though and do we want a CC stable ?
I definitely think this should go to stable.
^ permalink raw reply
* [RFC] new SYSCALL_DEFINE/COMPAT_SYSCALL_DEFINE wrappers
From: Al Viro @ 2018-03-26 0:40 UTC (permalink / raw)
To: Ingo Molnar
Cc: Linus Torvalds, Dominik Brodowski, Linux Kernel Mailing List,
Arnd Bergmann, linux-arch, Ralf Baechle, James Hogan, linux-mips,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <20180322001532.GA18399@ZenIV.linux.org.uk>
On Thu, Mar 22, 2018 at 12:15:32AM +0000, Al Viro wrote:
> FWIW, I have something that is almost reasonable on preprocessor side;
> however, that has uncovered the following fun:
[snip]
According to gcc folks, the right way to do it is type-punning via
union. Anyway, below is something that kinda-sorta works as a
replacement of macrology in compat.h and syscalls.h.
Kinda-sorta part:
* asmlinkage_protect is taken out for now, so m68k has problems.
* syscalls that run out of 6 slots barf violently. For mips it's
wrong (there we have 8 slots); for stuff like arm and ppc it's right, but
it means that things like e.g. compat sync_file_range() should not even
be compiled on those. __ARCH_WANT_SYS_SYNC_FILE_RANGE, presumably...
In any case, we *can't* do pt_regs-based wrappers for those syscalls on
such architectures, so ifdefs around those puppies are probably the right
thing to do.
* s390 macrology in compat_wrapper.c not even touched; it needs
a trivial update to keep working (__MAP callbacks take an extra argument,
unused for those users).
* sys_... and compat_sys_... aliases are unchanged; if we kill
direct callers, we can trivially rename SyS##name and compat_SyS##name
to sys##name and compat_sys##name and get rid of aliases.
It allows an architecture to decide whether it wants 6-argument wrappers
or pt_regs ones - that's encapsulated into several macros. Padding is
done properly; things like truncate64() et.al. can be done as single
COMPAT_SYSCALL_DEFINE. Moreover, AFAICS the same wrappers work just fine
for arm64 - no worse than asm glue currently used there.
I've tried to put the sane amount of comments in there, hopefully
explaining what the hell is that nest of horrors doing. It's ugly,
but less eye-bleeding that I thought it would be.
I'm not sure if it's a good idea; something of that sort would probably be
useful for any kind of pt_regs-passing wrappers, though. Review would be
welcome...
#include <linux/compiler.h>
#include <linux/linkage.h>
#include <linux/build_bug.h>
#include <trace/syscall.h>
/*
* __MAP - apply a macro to syscall arguments
* __MAP(n, m, t1, a1, t2, a2, ..., tn, an) will expand to
* m(, t1, a1), m(_, t2, a2), ..., m(_..._, tn, an)
* The first argument must be equal to the amount of type/name
* pairs given. Note that this list of pairs (i.e. the arguments
* of __MAP starting at the third one) is in the same format as
* for SYSCALL_DEFINE<n>/COMPAT_SYSCALL_DEFINE<n>
*/
#define __MAP0(m,p,...)
#define __MAP1(m,p,t,a) m(p,t,a)
#define __MAP2(m,p,t,a,...) m(p,t,a), __MAP1(m,p##_,__VA_ARGS__)
#define __MAP3(m,p,t,a,...) m(p,t,a), __MAP2(m,p##_,__VA_ARGS__)
#define __MAP4(m,p,t,a,...) m(p,t,a), __MAP3(m,p##_,__VA_ARGS__)
#define __MAP5(m,p,t,a,...) m(p,t,a), __MAP4(m,p##_,__VA_ARGS__)
#define __MAP6(m,p,t,a,...) m(p,t,a), __MAP5(m,p##_,__VA_ARGS__)
#define __MAP(n,m,...) __MAP##n(m,,__VA_ARGS__)
#define __TYPE_AS(t, v) __same_type((__force t)0, v)
#define __TYPE_IS_L(t) (__TYPE_AS(t, 0L))
#define __TYPE_IS_UL(t) (__TYPE_AS(t, 0UL))
#define __TYPE_IS_LL(t) (__TYPE_AS(t, 0LL) || __TYPE_AS(t, 0ULL))
/*
* __SC_SIZES32(n, t1, a1, ..., tn, an)
* __SC_SIZES64(n, t1, a1, ..., tn, an)
* Build an enum describing the number of slots taken by syscall arguments.
* Example: __SC_SIZES32(3, int, a, loff_t, b, void *, c) ends up with
* enum {__SC_BIG_ = 0, __SC_BIG__ = 1, __SC_BIG___ = 0};
* __SC_BIG<k underscores> => does k-th argument take two slots.
* Note that __SC_SIZES64 will always define them as zeroes.
*/
#define __SC_SIZE32(p,t,a) __SC_BIG_##p = __TYPE_IS_LL(t)
#define __SC_SIZE64(p,t,a) __SC_BIG_##p = 0
#define __SC_SIZES32(n,...) enum{__MAP(n,__SC_SIZE32,__VA_ARGS__)}
#define __SC_SIZES64(n,...) enum{__MAP(n,__SC_SIZE64,__VA_ARGS__)}
/*
* Distributing the argument slots. That depends upon the C ABI for target
* architecture. Background: there is an arch-specific sequence of word-sized
* objects and function arguments are mapped on that sequence. Each argument
* takes one or two slots (we are dealing only with integer and pointer types;
* the rules for floating point, structs, etc. can be a lot more complex).
*
* Syscall ABI marshals the arguments across the userland boundary. In almost
* all cases it is done by having 6 registers, each assigned to a slot in
* the sequence. The value of those registers are stored in the corresponding
* slots and control is passed to a wrapper C function implementing the syscall.
* There the values are normalized and passed to the syscall itself.
*
* In case of biarch architectures we need to know the allocation rules for
* 32bit counterpart - the 64bit ones are trivial (each argument takes exactly
* one slot, the nth argument goes into nth slot and that's it).
*
* Rules are encoded in a pair of macros - __SC_FIRST_ARG and __SC_NEXT_ARG.
* For the absolute majority of architectures it is simply "take the next
* one or two slots, depending upon the argument size", and those are fine
* with defaults. Something trickier needs to override those.
*/
#ifndef __SC_FIRST_ARG
/*
* Unless the ABI is really pathological, default will do.
*/
#define __SC_FIRST_ARG 0
#endif
#ifndef __SC_NEXT_ARG
/* For architectures with no alignment requirements. Most of our targets are
* such: alpha arc cris frv h8300 ia64 m32r m68k microblaze mn10300 nios2
* openrisc riscv sparc x86.
*/
#define __SC_NEXT_ARG(p, next, big) next
#endif
#if 0
// Those should live in the corresponding arch/.../include/asm/ABI.h:
// arm, mips, parisc, powerpc, xtensa: double-slot arguments are aligned
// to even slot there.
#define __SC_NEXT_ARG(p, next, big) next + (big & next)
// s390: double-slot can't occupy slots 4 and 5.
#define __SC_NEXT_ARG(p, next, big) next + (big && next == 4)
// blackfin: double-slot can't occupy slots 2 and 3.
#define __SC_NEXT_ARG(p, next, big) next + (big && next == 2)
// sh: this is really nasty.
// Double-slot ones can't occupy slots 3 and 4; if slot 3 had been skipped due
// to that, the next single-slot argument goes there. That's where we need
// to keep track of more than just the size of the argument and the slots occupied
// by the previous one.
#define __SC_FIRST_ARG 0, __SC_S_ = 0
#define __SC_NEXT_ARG(p, next, big) \
(big && next == 3) ? 4 : \
(!big && next == __SC_S##p) ? 3 : \
(next < __SC_S##p) ? __SC_S##p : next, \
__SC_S_##p = \
(big && next == 3) ? 6 : \
(next != __SC_S##p) ? 0 : \
__SC_S##p + (big ? 2 : 0)
#endif
/*
* __SC_SLOTS(n, t1, a1, ..., tn, an)
* Build an enum describing the slots taken by each argument.
* Assumes __SC_BIG_... already in the scope and __SC_{FIRST,NEXT}_ARG
* defined according to the architecture.
*/
#define __SC_IF(x,y) y
#define __SC_IF_(x,y) x
#define __SC_IF__ __SC_IF_
#define __SC_IF___ __SC_IF_
#define __SC_IF____ __SC_IF_
#define __SC_IF_____ __SC_IF_
#define __SC_SLOT(p,t,a) __SC_OFF_##p = \
__SC_IF##p(__SC_NEXT_ARG(p, (__SC_OFF##p + __SC_BIG##p + 1), __SC_BIG_##p),\
__SC_FIRST_ARG)
#define __SC_SLOTS(n,...) enum{ __MAP(n,__SC_SLOT,__VA_ARGS__) }
/*
* Syscall ABI marshalling. Syscall dispatcher can either arrange loading
* of registers into slots on its own and have the syscall wrapper called
* (in that case we want the wrapper looking like a 6-argument function,
* with each argument taking a slot) or pass a pointer to struct pt_regs
* and have wrapper fetch the individual registers from there. In the
* former case all knowledge of the mapping from registers to slots sits
* in the caller, in the latter - in wrappers. Other strategies are
* also possible; depends upon the architecture.
*
* For syscall wrappers we want the prototype to be used for wrapper and
* macros for accessing individual slots. Default is 6-argument wrappers;
* if architecture wants something different, it should supply
* __SC_{32,64}PROTO for the prototype and __SC_{32,64}ARG{1..6} for
* arguments. Example: for struct pt_regs on x86 we want,
* in arch/x86/include/asm/ABI.h,
* #define __SC_64PROTO struct pt_regs *regs
* #define __SC_64ARG1 regs->di
* #define __SC_64ARG2 regs->si
* #define __SC_64ARG3 regs->dx
* #define __SC_64ARG4 regs->r10
* #define __SC_64ARG5 regs->r8
* #define __SC_64ARG6 regs->r9
* #define __SC_32PROTO struct pt_regs *regs
* #define __SC_32ARG1 regs->bx
* #define __SC_32ARG2 regs->cx
* #define __SC_32ARG3 regs->dx
* #define __SC_32ARG4 regs->si
* #define __SC_32ARG5 regs->di
* #define __SC_32ARG6 regs->bp
*/
#ifndef __SC_64PROTO
#define __SC_64PROTO u64 __SC_64ARG1, u64 __SC_64ARG2, u64 __SC_64ARG3,\
u64 __SC_64ARG4, u64 __SC_64ARG5, u64 __SC_64ARG6
#endif
#ifndef __SC_32PROTO
#define __SC_32PROTO u32 __SC_32ARG1, u32 __SC_32ARG2, u32 __SC_32ARG3,\
u32 __SC_32ARG4, u32 __SC_32ARG5, u32 __SC_32ARG6
#endif
static __always_inline __nostackprotector u64 combine_long_long(u32 a, u32 b)
{
union { u32 c[2]; u64 d; } x = {.c={a,b}};
return x.d;
}
extern u32 __SC_32_ARG7(void) __compiletime_error("too many argument slots");
extern u64 __SC_64_ARG7(void) __compiletime_error("too many argument slots");
#define __SC_32ARG7 __SC_64_ARG7()
#define __SC_64ARG7 __SC_32_ARG7()
#define __SC_W(s,n) (n == 0 ? __SC_##s##ARG1 : \
n == 1 ? __SC_##s##ARG2 : \
n == 2 ? __SC_##s##ARG3 : \
n == 3 ? __SC_##s##ARG4 : \
n == 4 ? __SC_##s##ARG5 : \
n == 5 ? __SC_##s##ARG6 : \
__SC_##s##ARG7)
#define __SC_D(s,n) \
(n == 0 ? combine_long_long(__SC_##s##ARG1,__SC_##s##ARG2) : \
n == 1 ? combine_long_long(__SC_##s##ARG2,__SC_##s##ARG3) : \
n == 2 ? combine_long_long(__SC_##s##ARG3,__SC_##s##ARG4) : \
n == 3 ? combine_long_long(__SC_##s##ARG4,__SC_##s##ARG5) : \
n == 4 ? combine_long_long(__SC_##s##ARG5,__SC_##s##ARG6) : \
combine_long_long(__SC_##s##ARG6,__SC_##s##ARG7))
#define __SC_ARG32(p, t, a) \
(__force t)__builtin_choose_expr(__SC_BIG_##p, \
__SC_D(32, __SC_OFF_##p), __SC_W(32, __SC_OFF_##p))
#define __SC_ARG64(p, t, a) \
(__force t)__builtin_choose_expr(__SC_BIG_##p, \
__SC_D(64, __SC_OFF_##p), __SC_W(64, __SC_OFF_##p))
#define __SC_ARGS32(n, ...) __MAP(n, __SC_ARG32, __VA_ARGS__)
#define __SC_ARGS64(n, ...) __MAP(n, __SC_ARG64, __VA_ARGS__)
#ifdef CONFIG_64BIT
#define __SC_PROTO __SC_64PROTO
#define __SC_ARGS __SC_ARGS64
#define __SC_SIZES __SC_SIZES64
#else
#define __SC_PROTO __SC_32PROTO
#define __SC_ARGS __SC_ARGS32
#define __SC_SIZES __SC_SIZES32
#endif
/* The following is ftrace glue, almost verbatim from mainline */
#ifdef CONFIG_FTRACE_SYSCALLS
#define __SC_STR_ADECL(p, t, a) #a
#define __SC_STR_TDECL(p, t, a) #t
extern struct trace_event_class event_class_syscall_enter;
extern struct trace_event_class event_class_syscall_exit;
extern struct trace_event_functions enter_syscall_print_funcs;
extern struct trace_event_functions exit_syscall_print_funcs;
#define SYSCALL_TRACE_ENTER_EVENT(sname) \
static struct syscall_metadata __syscall_meta_##sname; \
static struct trace_event_call __used \
event_enter_##sname = { \
.class = &event_class_syscall_enter, \
{ \
.name = "sys_enter"#sname, \
}, \
.event.funcs = &enter_syscall_print_funcs, \
.data = (void *)&__syscall_meta_##sname,\
.flags = TRACE_EVENT_FL_CAP_ANY, \
}; \
static struct trace_event_call __used \
__attribute__((section("_ftrace_events"))) \
*__event_enter_##sname = &event_enter_##sname;
#define SYSCALL_TRACE_EXIT_EVENT(sname) \
static struct syscall_metadata __syscall_meta_##sname; \
static struct trace_event_call __used \
event_exit_##sname = { \
.class = &event_class_syscall_exit, \
{ \
.name = "sys_exit"#sname, \
}, \
.event.funcs = &exit_syscall_print_funcs, \
.data = (void *)&__syscall_meta_##sname,\
.flags = TRACE_EVENT_FL_CAP_ANY, \
}; \
static struct trace_event_call __used \
__attribute__((section("_ftrace_events"))) \
*__event_exit_##sname = &event_exit_##sname;
#define SYSCALL_METADATA(sname, nb, ...) \
static const char *types_##sname[] = { \
__MAP(nb,__SC_STR_TDECL,__VA_ARGS__) \
}; \
static const char *args_##sname[] = { \
__MAP(nb,__SC_STR_ADECL,__VA_ARGS__) \
}; \
SYSCALL_TRACE_ENTER_EVENT(sname); \
SYSCALL_TRACE_EXIT_EVENT(sname); \
static struct syscall_metadata __used \
__syscall_meta_##sname = { \
.name = "sys"#sname, \
.syscall_nr = -1, /* Filled in at boot */ \
.nb_args = nb, \
.types = nb ? types_##sname : NULL, \
.args = nb ? args_##sname : NULL, \
.enter_event = &event_enter_##sname, \
.exit_event = &event_exit_##sname, \
.enter_fields = LIST_HEAD_INIT(__syscall_meta_##sname.enter_fields), \
}; \
static struct syscall_metadata __used \
__attribute__((section("__syscalls_metadata"))) \
*__p_syscall_meta_##sname = &__syscall_meta_##sname;
static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
{
return tp_event->class == &event_class_syscall_enter ||
tp_event->class == &event_class_syscall_exit;
}
#else
#define SYSCALL_METADATA(sname, nb, ...)
static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
{
return 0;
}
#endif
#define SYSCALL_DEFINE0(sname) \
SYSCALL_METADATA(_##sname, 0); \
asmlinkage long sys_##sname(void)
#define SYSCALL_DEFINE1(name, ...) SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
#define SYSCALL_DEFINE2(name, ...) SYSCALL_DEFINEx(2, _##name, __VA_ARGS__)
#define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
#define SYSCALL_DEFINE4(name, ...) SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
#define SYSCALL_DEFINE5(name, ...) SYSCALL_DEFINEx(5, _##name, __VA_ARGS__)
#define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
#define SYSCALL_DEFINEx(x, sname, ...) \
SYSCALL_METADATA(sname, x, __VA_ARGS__) \
__SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
#define __SC_DECL(p, t, a) t a
#define __SC_TEST(p, t, a) (void)BUILD_BUG_ON_ZERO(!__TYPE_IS_LL(t) && sizeof(t) > sizeof(long))
#define __SYSCALL_DEFINEx(x, name, ...) \
asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
__attribute__((alias(__stringify(SyS##name)))); \
static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__)); \
asmlinkage long SyS##name(__SC_PROTO); \
asmlinkage long SyS##name(__SC_PROTO) \
{ \
__SC_SIZES(x,__VA_ARGS__); \
__SC_SLOTS(x,__VA_ARGS__); \
__MAP(x,__SC_TEST,__VA_ARGS__); \
return SYSC##name(__SC_ARGS(x,__VA_ARGS__)); \
} \
static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__))
#ifdef CONFIG_COMPAT
#define __SC_COMPAT_PROTO __SC_32PROTO
#define __SC_COMPAT_ARGS __SC_ARGS32
#define __SC_COMPAT_SIZES __SC_SIZES32
#define COMPAT_SYSCALL_DEFINE0(name) \
asmlinkage long compat_sys_##name(void)
#define COMPAT_SYSCALL_DEFINE1(name, ...) \
COMPAT_SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
#define COMPAT_SYSCALL_DEFINE2(name, ...) \
COMPAT_SYSCALL_DEFINEx(2, _##name, __VA_ARGS__)
#define COMPAT_SYSCALL_DEFINE3(name, ...) \
COMPAT_SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
#define COMPAT_SYSCALL_DEFINE4(name, ...) \
COMPAT_SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
#define COMPAT_SYSCALL_DEFINE5(name, ...) \
COMPAT_SYSCALL_DEFINEx(5, _##name, __VA_ARGS__)
#define COMPAT_SYSCALL_DEFINE6(name, ...) \
COMPAT_SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
#define COMPAT_SYSCALL_DEFINEx(x, name, ...) \
asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
__attribute__((alias(__stringify(compat_SyS##name)))); \
static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
asmlinkage long compat_SyS##name(__SC_COMPAT_PROTO); \
asmlinkage long compat_SyS##name(__SC_COMPAT_PROTO) \
{ \
__SC_COMPAT_SIZES(x,__VA_ARGS__); \
__SC_SLOTS(x,__VA_ARGS__); \
return C_SYSC##name(__SC_COMPAT_ARGS(x,__VA_ARGS__)); \
} \
static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__))
#endif
^ permalink raw reply
* Re: [PATCH v2 02/16] riscv: Use INITRAMFS_GENERIC_UNLOAD.
From: Palmer Dabbelt @ 2018-03-26 0:41 UTC (permalink / raw)
To: shea
Cc: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel, shea
In-Reply-To: <20180325221853.10839-3-shea@shealevy.com>
On Sun, 25 Mar 2018 15:18:39 PDT (-0700), shea@shealevy.com wrote:
> Signed-off-by: Shea Levy <shea@shealevy.com>
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/mm/init.c | 6 ------
> 2 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index c22ebe08e902..ab1b4cee84fc 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -37,6 +37,7 @@ config RISCV
> select THREAD_INFO_IN_TASK
> select RISCV_TIMER
> select GENERIC_IRQ_MULTI_HANDLER
> + select INITRAMFS_GENERIC_UNLOAD
>
> config MMU
> def_bool y
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index c77df8142be2..36f83fe8a726 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -62,9 +62,3 @@ void free_initmem(void)
> {
> free_initmem_default(0);
> }
> -
> -#ifdef CONFIG_BLK_DEV_INITRD
> -void free_initrd_mem(unsigned long start, unsigned long end)
> -{
> -}
> -#endif /* CONFIG_BLK_DEV_INITRD */
I haven't looked through the rest of the patch set, but this is a pretty
trivial change so feel free to add a
Reviewed-By: Palmer Dabbelt <palmer@sifive.com>
if you'd like. If you'd like it merged through my tree then just say
something!
^ permalink raw reply
* Re: [PATCH v2 02/16] riscv: Use INITRAMFS_GENERIC_UNLOAD.
From: Shea Levy @ 2018-03-26 0:55 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: linux-alpha, linux-kernel, linux-snps-arc, linux-c6x-dev,
uclinux-h8-devel, linux-m68k, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-sh, user-mode-linux-devel
In-Reply-To: <mhng-5691a509-dedf-4092-bd6d-020a81512c15@palmer-si-x1c4>
[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]
Hi Palmer,
Palmer Dabbelt <palmer@sifive.com> writes:
> On Sun, 25 Mar 2018 15:18:39 PDT (-0700), shea@shealevy.com wrote:
>> Signed-off-by: Shea Levy <shea@shealevy.com>
>> ---
>> arch/riscv/Kconfig | 1 +
>> arch/riscv/mm/init.c | 6 ------
>> 2 files changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index c22ebe08e902..ab1b4cee84fc 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -37,6 +37,7 @@ config RISCV
>> select THREAD_INFO_IN_TASK
>> select RISCV_TIMER
>> select GENERIC_IRQ_MULTI_HANDLER
>> + select INITRAMFS_GENERIC_UNLOAD
>>
>> config MMU
>> def_bool y
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index c77df8142be2..36f83fe8a726 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -62,9 +62,3 @@ void free_initmem(void)
>> {
>> free_initmem_default(0);
>> }
>> -
>> -#ifdef CONFIG_BLK_DEV_INITRD
>> -void free_initrd_mem(unsigned long start, unsigned long end)
>> -{
>> -}
>> -#endif /* CONFIG_BLK_DEV_INITRD */
>
> I haven't looked through the rest of the patch set, but this is a pretty
> trivial change so feel free to add a
>
> Reviewed-By: Palmer Dabbelt <palmer@sifive.com>
>
> if you'd like. If you'd like it merged through my tree then just say
> something!
I'm not sure how these cross-cutting changes go, if you can take the
series through your tree that'd be great!
Thanks,
Shea
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH 3/6] libnvdimm: Add device-tree based driver
From: Oliver @ 2018-03-26 1:07 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm, Device Tree, linuxppc-dev
In-Reply-To: <CAPcyv4h=b9fCN7mCU8Yge5804oxVfErXRa_3h7QVcKdEhBbVHg@mail.gmail.com>
On Sat, Mar 24, 2018 at 4:07 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Fri, Mar 23, 2018 at 1:12 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
>> This patch adds peliminary device-tree bindings for the NVDIMM driver.
>
> *preliminary
>
>> Currently this only supports one bus (created at probe time) which all
>> regions are added to with individual regions being created by a platform
>> device driver.
>>
>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>> ---
>> I suspect the platform driver should be holding a reference to the
>> created region. I left that out here since previously Dan has said
>> he'd rather keep the struct device internal to libnvdimm and the only
>> other way a region device can disappear is when the bus is unregistered.
> Hmm, but this still seems broken if bus teardown races region
> teardown.
Yeah looks like it.
>I think the more natural model, given the way libnvdimm is
> structured, to have each of these of_nd_region instances create their
> own nvdimm bus. Thoughts?
I'd prefer this too tbh. The main reason I'm looking at this approach
is that we plan on doing all interleave set configuration, etc from
Linux. So we'd like to have "similar" region/dimm devices end up in
the same bus to simplify validating managment ops, etc. That said, the
real enablement work for that is blocked on teams so I'm ok with
ignoring all of that for now.
Oliver
^ permalink raw reply
* Re: [PATCH net-next] ibmvnic: Potential NULL dereference in clean_one_tx_pool()
From: David Miller @ 2018-03-26 1:14 UTC (permalink / raw)
To: dan.carpenter
Cc: benh, tlfalcon, jallen, paulus, mpe, netdev, linuxppc-dev,
kernel-janitors
In-Reply-To: <20180323113615.GA28518@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Fri, 23 Mar 2018 14:36:15 +0300
> There is an && vs || typo here, which potentially leads to a NULL
> dereference.
>
> Fixes: e9e1e97884b7 ("ibmvnic: Update TX pool cleaning routine")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks Dan.
^ permalink raw reply
* Re: [PATCH] powerpc/eeh: Fix race with driver un/bind
From: Michael Neuling @ 2018-03-26 2:47 UTC (permalink / raw)
To: Benjamin Herrenschmidt, mpe; +Cc: linuxppc-dev, ruscur, sam.bobroff
In-Reply-To: <1521786791.16434.339.camel@kernel.crashing.org>
On Fri, 2018-03-23 at 17:33 +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2018-03-23 at 16:44 +1100, Michael Neuling wrote:
>=20
> .../...
>=20
> > This fixes the problem in the same way the generic PCIe AER code (in
> > drivers/pci/pcie/aer/aerdrv_core.c) does. It makes the EEH code hold
> > the device_lock() before performing the driver EEH callbacks. This
> > ensures either the callbacks are no longer register, or if they are
> > registered the driver will not be removed from underneath us.
> >=20
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
>=20
> Generally ok, minor nits though and do we want a CC stable ?
ok, I'll cc stable.
>=20
> > ---
> > arch/powerpc/kernel/eeh_driver.c | 67 ++++++++++++++++++++++++--------=
-----
> > ---
> > 1 file changed, 41 insertions(+), 26 deletions(-)
> >=20
> > diff --git a/arch/powerpc/kernel/eeh_driver.c
> > b/arch/powerpc/kernel/eeh_driver.c
> > index 0c0b66fc5b..7cf946ae9a 100644
> > --- a/arch/powerpc/kernel/eeh_driver.c
> > +++ b/arch/powerpc/kernel/eeh_driver.c
> > @@ -207,18 +207,18 @@ static void *eeh_report_error(void *data, void
> > *userdata)
> > =20
> > if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> > return NULL;
> > +
> > + device_lock(&dev->dev);
> > dev->error_state =3D pci_channel_io_frozen;
> > =20
> > driver =3D eeh_pcid_get(dev);
> > - if (!driver) return NULL;
> > + if (!driver) goto out2;
>=20
> I don't like out1/out2, why not call them out_nodev and out_no_handler
> ? (same comment for the other ones).
OK, will change.
> > =20
> > eeh_disable_irq(dev);
> > =20
> > if (!driver->err_handler ||
> > - !driver->err_handler->error_detected) {
> > - eeh_pcid_put(dev);
> > - return NULL;
> > - }
> > + !driver->err_handler->error_detected)
> > + goto out1;
> > =20
> > rc =3D driver->err_handler->error_detected(dev,
> > pci_channel_io_frozen);
> > =20
> > @@ -227,8 +227,11 @@ static void *eeh_report_error(void *data, void
> > *userdata)
> > if (*res =3D=3D PCI_ERS_RESULT_NONE) *res =3D rc;
> > =20
> > edev->in_error =3D true;
> > - eeh_pcid_put(dev);
> > pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
> > +out1:
> > + eeh_pcid_put(dev);
> > +out2:
>=20
> This also changes doing the uevent while holding a reference and the
> the device lock, is that ok ? (I guess a reference is a good thing, the
> device lock, not sure... I hope so but you should at least document it
> as a chance in the cset comment).
The AER code does this, so it should be ok. See report_error_detected().
Mikey
^ permalink raw reply
* Re: [RFC] new SYSCALL_DEFINE/COMPAT_SYSCALL_DEFINE wrappers
From: Al Viro @ 2018-03-26 3:47 UTC (permalink / raw)
To: Ingo Molnar
Cc: Linus Torvalds, Dominik Brodowski, Linux Kernel Mailing List,
Arnd Bergmann, linux-arch, Ralf Baechle, James Hogan, linux-mips,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <20180326004017.GA2211@ZenIV.linux.org.uk>
On Mon, Mar 26, 2018 at 01:40:17AM +0100, Al Viro wrote:
> Kinda-sorta part:
> * asmlinkage_protect is taken out for now, so m68k has problems.
> * syscalls that run out of 6 slots barf violently. For mips it's
> wrong (there we have 8 slots); for stuff like arm and ppc it's right, but
> it means that things like e.g. compat sync_file_range() should not even
> be compiled on those. __ARCH_WANT_SYS_SYNC_FILE_RANGE, presumably...
> In any case, we *can't* do pt_regs-based wrappers for those syscalls on
> such architectures, so ifdefs around those puppies are probably the right
> thing to do.
> * s390 macrology in compat_wrapper.c not even touched; it needs
> a trivial update to keep working (__MAP callbacks take an extra argument,
> unused for those users).
> * sys_... and compat_sys_... aliases are unchanged; if we kill
> direct callers, we can trivially rename SyS##name and compat_SyS##name
> to sys##name and compat_sys##name and get rid of aliases.
* mips n32 and x86 x32 can become an extra source of headache.
That actually applies to any plans of passing struct pt_regs *. As it
is, e.g. syscall 515 on amd64 is compat_sys_readv(). Dispatched via
this:
/*
* NB: Native and x32 syscalls are dispatched from the same
* table. The only functional difference is the x32 bit in
* regs->orig_ax, which changes the behavior of some syscalls.
*/
if (likely((nr & __SYSCALL_MASK) < NR_syscalls)) {
nr = array_index_nospec(nr & __SYSCALL_MASK, NR_syscalls);
regs->ax = sys_call_table[nr](
regs->di, regs->si, regs->dx,
regs->r10, regs->r8, regs->r9);
}
Now, syscall 145 via 32bit call is *also* compat_sys_readv(), dispatched
via
nr = array_index_nospec(nr, IA32_NR_syscalls);
/*
* It's possible that a 32-bit syscall implementation
* takes a 64-bit parameter but nonetheless assumes that
* the high bits are zero. Make sure we zero-extend all
* of the args.
*/
regs->ax = ia32_sys_call_table[nr](
(unsigned int)regs->bx, (unsigned int)regs->cx,
(unsigned int)regs->dx, (unsigned int)regs->si,
(unsigned int)regs->di, (unsigned int)regs->bp);
Right now it works - we call the same function, passing it arguments picked
from different set of registers (di/si/dx in x32 case, bx/cx/dx in i386 one).
But if we switch to passing struct pt_regs * and have the wrapper fetch
regs->{bx,cx,dx}, we have a problem. It won't work for both entry points.
IMO it's a good reason to have dispatcher(s) handle extraction from pt_regs
and let the wrapper deal with the resulting 6 u64 or 6 u32, normalizing
them and arranging them into arguments expected by syscall body.
Linus, Dominik - how do you plan dealing with that fun? Regardless of the
way we generate the glue, the issue remains. We can't get the same
struct pt_regs *-taking function for both; we either need to produce
a separate chunk of glue for each compat_sys_... involved (either making
COMPAT_SYSCALL_DEFINE generate both, or having duplicate X32_SYSCALL_DEFINE
for each of those COMPAT_SYSCALL_DEFINE - with identical body, at that)
or we need to have the registers-to-slots mapping done in dispatcher...
^ permalink raw reply
* Re: [PATCH 3/6] libnvdimm: Add device-tree based driver
From: Balbir Singh @ 2018-03-26 4:05 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linux-nvdimm, devicetree, linuxppc-dev
In-Reply-To: <20180323081209.31387-3-oohall@gmail.com>
On Fri, 23 Mar 2018 19:12:06 +1100
Oliver O'Halloran <oohall@gmail.com> wrote:
> This patch adds peliminary device-tree bindings for the NVDIMM driver.
> Currently this only supports one bus (created at probe time) which all
> regions are added to with individual regions being created by a platform
> device driver.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
It's a good idea to keep a changelog from the previous revisions, but I'm
just nit-picking. The reason I want to bring that up is because this revision
has different bindings from what was proposed earlier.
> I suspect the platform driver should be holding a reference to the
> created region. I left that out here since previously Dan has said
> he'd rather keep the struct device internal to libnvdimm and the only
> other way a region device can disappear is when the bus is unregistered.
I think the previous bindings did not have this issue, but the suggestion
was to move regions to being platform devices so as to avoid custom
code for tracking what is populated and what is not.
Personally I liked the previous binding better, but then I am not a device
tree expert.
> ---
> MAINTAINERS | 8 +++
> drivers/nvdimm/Kconfig | 10 ++++
> drivers/nvdimm/Makefile | 1 +
> drivers/nvdimm/of_nvdimm.c | 130 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 149 insertions(+)
> create mode 100644 drivers/nvdimm/of_nvdimm.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4e62756936fa..e3fc47fbfc7a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8035,6 +8035,14 @@ Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
> S: Supported
> F: drivers/nvdimm/pmem*
>
> +LIBNVDIMM: DEVICETREE BINDINGS
> +M: Oliver O'Halloran <oohall@gmail.com>
> +L: linux-nvdimm@lists.01.org
> +Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
> +S: Supported
> +F: drivers/nvdimm/of_nvdimm.c
> +F: Documentation/devicetree/bindings/nvdimm/nvdimm-bus.txt
> +
> LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
> M: Dan Williams <dan.j.williams@intel.com>
> L: linux-nvdimm@lists.01.org
> diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
> index a65f2e1d9f53..505a9bbbe49f 100644
> --- a/drivers/nvdimm/Kconfig
> +++ b/drivers/nvdimm/Kconfig
> @@ -102,4 +102,14 @@ config NVDIMM_DAX
>
> Select Y if unsure
>
> +config OF_NVDIMM
> + tristate "Device-tree support for NVDIMMs"
> + depends on OF
> + default LIBNVDIMM
> + help
> + Allows byte addressable persistent memory regions to be described in the
> + device-tree.
Again nit-picking, but I thought we supported volatile mappings too.
> +
> + Select Y if unsure.
> +
> endif
> diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> index 70d5f3ad9909..fd6a5838aa25 100644
> --- a/drivers/nvdimm/Makefile
> +++ b/drivers/nvdimm/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o
> obj-$(CONFIG_ND_BTT) += nd_btt.o
> obj-$(CONFIG_ND_BLK) += nd_blk.o
> obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
> +obj-$(CONFIG_OF_NVDIMM) += of_nvdimm.o
>
> nd_pmem-y := pmem.o
>
> diff --git a/drivers/nvdimm/of_nvdimm.c b/drivers/nvdimm/of_nvdimm.c
> new file mode 100644
> index 000000000000..79c28291f420
> --- /dev/null
> +++ b/drivers/nvdimm/of_nvdimm.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#define pr_fmt(fmt) "of_nvdimm: " fmt
> +
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/libnvdimm.h>
> +#include <linux/module.h>
> +#include <linux/ioport.h>
> +#include <linux/slab.h>
> +
> +/*
> + * Container bus stuff. For now we just chunk regions into a default
> + * bus with no ndctl support. In the future we'll add some mechanism
> + * for dispatching regions into the correct bus type, but this is useful
> + * for now.
I liked that the previous binding handled this better, it looked very
similar to the e820 binding in terms of simplicity.
> + */
> +struct nvdimm_bus_descriptor bus_desc;
> +struct nvdimm_bus *bus;
> +
> +/* region driver */
> +
> +static const struct attribute_group *region_attr_groups[] = {
> + &nd_region_attribute_group,
> + &nd_device_attribute_group,
> + NULL,
> +};
> +
> +static const struct attribute_group *bus_attr_groups[] = {
> + &nvdimm_bus_attribute_group,
> + NULL,
> +};
> +
> +static int of_nd_region_probe(struct platform_device *pdev)
> +{
> + struct nd_region_desc ndr_desc;
> + struct resource temp_res;
> + struct nd_region *region;
> + struct device_node *np;
> +
> + np = dev_of_node(&pdev->dev);
> + if (!np)
> + return -ENXIO;
> +
> + pr_err("registering region for %pOF\n", np);
> +
> + if (of_address_to_resource(np, 0, &temp_res)) {
> + pr_warn("Unable to parse reg[0] for %pOF\n", np);
> + return -ENXIO;
> + }
> +
> + memset(&ndr_desc, 0, sizeof(ndr_desc));
> + ndr_desc.res = &temp_res;
> + ndr_desc.of_node = np;
> + ndr_desc.attr_groups = region_attr_groups;
> + ndr_desc.numa_node = of_node_to_nid(np);
We probably need an ndr_desc.provider
> + set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
> +
> + /*
> + * NB: libnvdimm copies the data from ndr_desc into it's own structures
> + * so passing stack pointers is fine.
> + */
> + if (of_get_property(np, "volatile", NULL))
> + region = nvdimm_volatile_region_create(bus, &ndr_desc);
> + else
> + region = nvdimm_pmem_region_create(bus, &ndr_desc);
> +
> + pr_warn("registered pmem region %px\n", region);
> + if (!region)
> + return -ENXIO;
> +
> + platform_set_drvdata(pdev, region);
> +
> + return 0;
> +}
> +
> +static int of_nd_region_remove(struct platform_device *pdev)
> +{
> + struct nd_region *r = platform_get_drvdata(pdev);
> +
> + nd_region_destroy(r);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id of_nd_region_match[] = {
> + { .compatible = "nvdimm-region" },
> + { },
> +};
> +
> +static struct platform_driver of_nd_region_driver = {
> + .probe = of_nd_region_probe,
> + .remove = of_nd_region_remove,
> + .driver = {
> + .name = "of_nd_region",
> + .owner = THIS_MODULE,
> + .of_match_table = of_nd_region_match,
> + },
> +};
> +
> +/* bus wrangling */
> +
> +static int __init of_nvdimm_init(void)
> +{
> + /* register */
> + bus_desc.attr_groups = bus_attr_groups;
> + bus_desc.provider_name = "of_nvdimm";
> + bus_desc.module = THIS_MODULE;
> +
> + /* does parent == NULL work? */
> + bus = nvdimm_bus_register(NULL, &bus_desc);
> + if (!bus)
> + return -ENODEV;
> +
> + platform_driver_register(&of_nd_region_driver);
> +
> + return 0;
> +}
> +module_init(of_nvdimm_init);
> +
> +static void __init of_nvdimm_exit(void)
> +{
> + nvdimm_bus_unregister(bus);
> + platform_driver_unregister(&of_nd_region_driver);
> +}
> +module_exit(of_nvdimm_exit);
> +
> +MODULE_DEVICE_TABLE(of, of_nd_region_match);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("IBM Corporation");
Balbir Singh.
^ permalink raw reply
* [PATCH v2] powerpc/eeh: Fix race with driver un/bind
From: Michael Neuling @ 2018-03-26 4:17 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, benh, ruscur, sam.bobroff
The current EEH callbacks can race with a driver unbind. This
can result in a backtraces like this:
[ 7.573055] EEH: Frozen PHB#0-PE#1fc detected
[ 7.573063] EEH: PE location: S000009, PHB location: N/A
[ 7.573069] CPU: 2 PID: 2312 Comm: kworker/u258:3 Not tainted 4.15.6-openpower1 #2
[ 7.573078] Workqueue: nvme-wq nvme_reset_work [nvme]
[ 7.573080] Call Trace:
[ 7.573088] [c000000ff12a3a30] [c0000000005f5000] dump_stack+0x9c/0xd0
[ 7.573093] (unreliable)
[ 7.573106] [c000000ff12a3a70] [c00000000002385c] eeh_dev_check_failure+0x420/0x470
[ 7.573111] [c000000ff12a3b10] [c00000000002394c] eeh_check_failure+0xa0/0xa4
[ 7.573115] [c000000ff12a3b50] [c0080000088c2ff0] nvme_reset_work+0x138/0x1414 [nvme]
[ 7.573122] [c000000ff12a3cb0] [c000000000089c78] process_one_work+0x1ec/0x328
[ 7.573132] [c000000ff12a3d40] [c00000000008a3b4] worker_thread+0x2e4/0x3a8
[ 7.573140] [c000000ff12a3dc0] [c00000000008fed0] kthread+0x14c/0x154
[ 7.573150] [c000000ff12a3e30] [c00000000000b594] ret_from_kernel_thread+0x5c/0xc8
[ 7.573183] nvme nvme1: Removing after probe failure status: -19
<snip>
cpu 0x23: Vector: 300 (Data Access) at [c000000ff50f3800]
pc: c0080000089a0eb0: nvme_error_detected+0x4c/0x90 [nvme]
lr: c000000000026564: eeh_report_error+0xe0/0x110
sp: c000000ff50f3a80
msr: 9000000000009033
dar: 400
dsisr: 40000000
current = 0xc000000ff507c000
paca = 0xc00000000fdc9d80 softe: 0 irq_happened: 0x01
pid = 782, comm = eehd
Linux version 4.15.6-openpower1 (smc@smc-desktop) (gcc version 6.4.0 (Buildroot 2017.11.2-00008-g4b6188e)) #2 SM P Tue Feb 27 12:33:27 PST 2018
enter ? for help
[c000000ff50f3af0] c000000000026564 eeh_report_error+0xe0/0x110
[c000000ff50f3b30] c000000000025520 eeh_pe_dev_traverse+0xc0/0xdc
[c000000ff50f3bc0] c000000000026bd0 eeh_handle_normal_event+0x184/0x4c4
[c000000ff50f3c70] c000000000026ff4 eeh_handle_event+0x30/0x288
[c000000ff50f3d10] c00000000002758c eeh_event_handler+0x124/0x170
[c000000ff50f3dc0] c00000000008fed0 kthread+0x14c/0x154
[c000000ff50f3e30] c00000000000b594 ret_from_kernel_thread+0x5c/0xc8
The first part is an EEH (on boot), the second half is the resulting
crash. nvme probe starts the nvme_reset_work() worker thread. This
worker thread starts touching the device which see a device error
(EEH) and hence queues up an event in the powerpc EEH worker
thread. nvme_reset_work() then continues and runs
nvme_remove_dead_ctrl_work() which results in unbinding the driver
from the device and hence releases all resources. At the same time,
the EEH worker thread starts doing the EEH .error_detected() driver
callback, which no longer works since the resources have been freed.
This fixes the problem in the same way the generic PCIe AER code (in
drivers/pci/pcie/aer/aerdrv_core.c) does. It makes the EEH code hold
the device_lock() while performing the driver EEH callbacks and
associated code. This ensures either the callbacks are no longer
register, or if they are registered the driver will not be removed
from underneath us.
This has been broken forever. The EEH call backs were first introduced
in 2005 (in 77bd7415610) but it's not clear if a lock was needed back
then.
Signed-off-by: Michael Neuling <mikey@neuling.org>
Cc: stable@vger.kernel.org
---
v2:
Updates based on benh's comments:
- Change jump labels
- Clarify where lock is held in commit message
---
arch/powerpc/kernel/eeh_driver.c | 68 +++++++++++++++++++++++++---------------
1 file changed, 42 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 0c0b66fc5b..295ba83384 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -207,18 +207,18 @@ static void *eeh_report_error(void *data, void *userdata)
if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
return NULL;
+
+ device_lock(&dev->dev);
dev->error_state = pci_channel_io_frozen;
driver = eeh_pcid_get(dev);
- if (!driver) return NULL;
+ if (!driver) goto out_no_dev;
eeh_disable_irq(dev);
if (!driver->err_handler ||
- !driver->err_handler->error_detected) {
- eeh_pcid_put(dev);
- return NULL;
- }
+ !driver->err_handler->error_detected)
+ goto out;
rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen);
@@ -227,8 +227,12 @@ static void *eeh_report_error(void *data, void *userdata)
if (*res == PCI_ERS_RESULT_NONE) *res = rc;
edev->in_error = true;
- eeh_pcid_put(dev);
pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
+
+out:
+ eeh_pcid_put(dev);
+out_no_dev:
+ device_unlock(&dev->dev);
return NULL;
}
@@ -251,15 +255,14 @@ static void *eeh_report_mmio_enabled(void *data, void *userdata)
if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
return NULL;
+ device_lock(&dev->dev);
driver = eeh_pcid_get(dev);
- if (!driver) return NULL;
+ if (!driver) goto out_no_dev;
if (!driver->err_handler ||
!driver->err_handler->mmio_enabled ||
- (edev->mode & EEH_DEV_NO_HANDLER)) {
- eeh_pcid_put(dev);
- return NULL;
- }
+ (edev->mode & EEH_DEV_NO_HANDLER))
+ goto out;
rc = driver->err_handler->mmio_enabled(dev);
@@ -267,7 +270,10 @@ static void *eeh_report_mmio_enabled(void *data, void *userdata)
if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
if (*res == PCI_ERS_RESULT_NONE) *res = rc;
+out:
eeh_pcid_put(dev);
+out_no_dev:
+ device_unlock(&dev->dev);
return NULL;
}
@@ -290,20 +296,20 @@ static void *eeh_report_reset(void *data, void *userdata)
if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
return NULL;
+
+ device_lock(&dev->dev);
dev->error_state = pci_channel_io_normal;
driver = eeh_pcid_get(dev);
- if (!driver) return NULL;
+ if (!driver) goto out_no_dev;
eeh_enable_irq(dev);
if (!driver->err_handler ||
!driver->err_handler->slot_reset ||
(edev->mode & EEH_DEV_NO_HANDLER) ||
- (!edev->in_error)) {
- eeh_pcid_put(dev);
- return NULL;
- }
+ (!edev->in_error))
+ goto out;
rc = driver->err_handler->slot_reset(dev);
if ((*res == PCI_ERS_RESULT_NONE) ||
@@ -311,7 +317,10 @@ static void *eeh_report_reset(void *data, void *userdata)
if (*res == PCI_ERS_RESULT_DISCONNECT &&
rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
+out:
eeh_pcid_put(dev);
+out_no_dev:
+ device_unlock(&dev->dev);
return NULL;
}
@@ -362,10 +371,12 @@ static void *eeh_report_resume(void *data, void *userdata)
if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
return NULL;
+
+ device_lock(&dev->dev);
dev->error_state = pci_channel_io_normal;
driver = eeh_pcid_get(dev);
- if (!driver) return NULL;
+ if (!driver) goto out_no_dev;
was_in_error = edev->in_error;
edev->in_error = false;
@@ -375,18 +386,20 @@ static void *eeh_report_resume(void *data, void *userdata)
!driver->err_handler->resume ||
(edev->mode & EEH_DEV_NO_HANDLER) || !was_in_error) {
edev->mode &= ~EEH_DEV_NO_HANDLER;
- eeh_pcid_put(dev);
- return NULL;
+ goto out;
}
driver->err_handler->resume(dev);
- eeh_pcid_put(dev);
pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED);
+out:
+ eeh_pcid_put(dev);
#ifdef CONFIG_PCI_IOV
if (eeh_ops->notify_resume && eeh_dev_to_pdn(edev))
eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
#endif
+out_no_dev:
+ device_unlock(&dev->dev);
return NULL;
}
@@ -406,23 +419,26 @@ static void *eeh_report_failure(void *data, void *userdata)
if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
return NULL;
+
+ device_lock(&dev->dev);
dev->error_state = pci_channel_io_perm_failure;
driver = eeh_pcid_get(dev);
- if (!driver) return NULL;
+ if (!driver) goto out_no_dev;
eeh_disable_irq(dev);
if (!driver->err_handler ||
- !driver->err_handler->error_detected) {
- eeh_pcid_put(dev);
- return NULL;
- }
+ !driver->err_handler->error_detected)
+ goto out;
driver->err_handler->error_detected(dev, pci_channel_io_perm_failure);
- eeh_pcid_put(dev);
pci_uevent_ers(dev, PCI_ERS_RESULT_DISCONNECT);
+out:
+ eeh_pcid_put(dev);
+out_no_dev:
+ device_unlock(&dev->dev);
return NULL;
}
--
2.14.1
^ permalink raw reply related
* Re: [RFC] new SYSCALL_DEFINE/COMPAT_SYSCALL_DEFINE wrappers
From: Linus Torvalds @ 2018-03-26 6:15 UTC (permalink / raw)
To: Al Viro
Cc: Ingo Molnar, Dominik Brodowski, Linux Kernel Mailing List,
Arnd Bergmann, linux-arch, Ralf Baechle, James Hogan, linux-mips,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <20180326034750.GN30522@ZenIV.linux.org.uk>
On Sun, Mar 25, 2018 at 5:47 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Linus, Dominik - how do you plan dealing with that fun?
Secretly, I was hoping to kill x32, because it's not being used afaik.
More realistically, I was thinking we'd just use a separate table or
system calls, and generate different versions.
In fact, you can see exactly that in my WIP branch, except it uses the
wrong name.
So see the "WIP-syscall" branch in my normal git kernel repo, and in
particular the patch to <linux/syscalls.h>, which generates
"sys_x64##name" and "sys_i86##name()" inline functions that do that
mapping correcty for native x86-64, and for the (misnamed) x32 cases.
So there are three different cases:
- native: sys_x64_name() generated by SYSCALL_DEFINEx()
- compat -bit: compat_sys_i86_name() generated by COMPAT_SYSCALL_DEFINEx()
- x32: sys_i86_name() generated by SYSCALL_DEFINEx().
and then I actually changed the names in the tables (ie in
arch/x86/entry/syscalls/syscall_64.tbl etc).
HOWEVER.
I didn't actually test any of the compat or x32 ones, and the way I
did it there also was no type-checking or other automated catching of
getting it wrong. So it's almost certainly completely buggy, but the
_intent_ is there and there is a remote possibility that it might even
work.
Linus
^ permalink raw reply
* Re: [RFC] new SYSCALL_DEFINE/COMPAT_SYSCALL_DEFINE wrappers
From: Linus Torvalds @ 2018-03-26 6:20 UTC (permalink / raw)
To: Al Viro
Cc: Ingo Molnar, Dominik Brodowski, Linux Kernel Mailing List,
Arnd Bergmann, linux-arch, Ralf Baechle, James Hogan, linux-mips,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <CA+55aFw8VGnVgaWHVFP-LChMNaoANOwT18jJEWzSCRLFeRGcmA@mail.gmail.com>
On Sun, Mar 25, 2018 at 8:15 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> HOWEVER.
>
> I didn't actually test any of the compat or x32 ones, and the way I
> did it there also was no type-checking or other automated catching of
> getting it wrong. So it's almost certainly completely buggy, but the
> _intent_ is there and there is a remote possibility that it might even
> work.
Note: the commit message is "broken, but working, ptregs system call
conversion for x86-64".
The "but working" is not because it would be right, but because it
booted a real 64-bit distribution successfully, and I actually used
that kernel.
But that only tests the native 64-bit case, it in no way tests the
compat or x32 cases what-so-ever. That part was literally a "it
compiles - it must be perfect" with absolutely zero testing of even
the most trivial kind.
And it really would have been most trivial to just do a "Hello world"
and build it with "-m32". I didn't do even that, and if I had, I would
have been honestly surprised had it worked.
But there was a _theoretical_ chance that it could have worked. Maybe.
Linus
^ permalink raw reply
* Re: [RFC] new SYSCALL_DEFINE/COMPAT_SYSCALL_DEFINE wrappers
From: Dominik Brodowski @ 2018-03-26 6:24 UTC (permalink / raw)
To: Al Viro
Cc: Ingo Molnar, Linus Torvalds, Linux Kernel Mailing List,
Arnd Bergmann, linux-arch, Ralf Baechle, James Hogan, linux-mips,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <20180326034750.GN30522@ZenIV.linux.org.uk>
On Mon, Mar 26, 2018 at 04:47:50AM +0100, Al Viro wrote:
> * mips n32 and x86 x32 can become an extra source of headache.
> That actually applies to any plans of passing struct pt_regs *. As it
> is, e.g. syscall 515 on amd64 is compat_sys_readv(). Dispatched via
> this:
> /*
> * NB: Native and x32 syscalls are dispatched from the same
> * table. The only functional difference is the x32 bit in
> * regs->orig_ax, which changes the behavior of some syscalls.
> */
> if (likely((nr & __SYSCALL_MASK) < NR_syscalls)) {
> nr = array_index_nospec(nr & __SYSCALL_MASK, NR_syscalls);
> regs->ax = sys_call_table[nr](
> regs->di, regs->si, regs->dx,
> regs->r10, regs->r8, regs->r9);
> }
> Now, syscall 145 via 32bit call is *also* compat_sys_readv(), dispatched
> via
> nr = array_index_nospec(nr, IA32_NR_syscalls);
> /*
> * It's possible that a 32-bit syscall implementation
> * takes a 64-bit parameter but nonetheless assumes that
> * the high bits are zero. Make sure we zero-extend all
> * of the args.
> */
> regs->ax = ia32_sys_call_table[nr](
> (unsigned int)regs->bx, (unsigned int)regs->cx,
> (unsigned int)regs->dx, (unsigned int)regs->si,
> (unsigned int)regs->di, (unsigned int)regs->bp);
> Right now it works - we call the same function, passing it arguments picked
> from different set of registers (di/si/dx in x32 case, bx/cx/dx in i386 one).
> But if we switch to passing struct pt_regs * and have the wrapper fetch
> regs->{bx,cx,dx}, we have a problem. It won't work for both entry points.
>
> IMO it's a good reason to have dispatcher(s) handle extraction from pt_regs
> and let the wrapper deal with the resulting 6 u64 or 6 u32, normalizing
> them and arranging them into arguments expected by syscall body.
>
> Linus, Dominik - how do you plan dealing with that fun? Regardless of the
> way we generate the glue, the issue remains. We can't get the same
> struct pt_regs *-taking function for both; we either need to produce
> a separate chunk of glue for each compat_sys_... involved (either making
> COMPAT_SYSCALL_DEFINE generate both, or having duplicate X32_SYSCALL_DEFINE
> for each of those COMPAT_SYSCALL_DEFINE - with identical body, at that)
> or we need to have the registers-to-slots mapping done in dispatcher...
Nice catch. A similar thing is needed already for non-compat syscalls like
sys_close(), which takes pt_regs->bx on IA32_EMULATION and pt_regs->di on
native x86-64. Therefore, I propose to generate all the stubs we need within
SYSCALL_DEFINEx() and COMPAT_SYSCALL_DEFINEx() (actually, within the
arch-provided version of these macros). See
https://git.kernel.org/pub/scm/linux/kernel/git/brodo/linux.git syscalls-WIP
for details on my current plans.
Thanks,
Dominik
^ permalink raw reply
* Re: [PATCH v2] powerpc/eeh: Fix race with driver un/bind
From: Benjamin Herrenschmidt @ 2018-03-26 6:27 UTC (permalink / raw)
To: Michael Neuling, mpe; +Cc: linuxppc-dev, ruscur, sam.bobroff
In-Reply-To: <20180326041707.11246-1-mikey@neuling.org>
On Mon, 2018-03-26 at 15:17 +1100, Michael Neuling wrote:
> The current EEH callbacks can race with a driver unbind. This
> can result in a backtraces like this:
>
> [ 7.573055] EEH: Frozen PHB#0-PE#1fc detected
> [ 7.573063] EEH: PE location: S000009, PHB location: N/A
> [ 7.573069] CPU: 2 PID: 2312 Comm: kworker/u258:3 Not tainted 4.15.6-openpower1 #2
> [ 7.573078] Workqueue: nvme-wq nvme_reset_work [nvme]
> [ 7.573080] Call Trace:
> [ 7.573088] [c000000ff12a3a30] [c0000000005f5000] dump_stack+0x9c/0xd0
> [ 7.573093] (unreliable)
> [ 7.573106] [c000000ff12a3a70] [c00000000002385c] eeh_dev_check_failure+0x420/0x470
> [ 7.573111] [c000000ff12a3b10] [c00000000002394c] eeh_check_failure+0xa0/0xa4
> [ 7.573115] [c000000ff12a3b50] [c0080000088c2ff0] nvme_reset_work+0x138/0x1414 [nvme]
> [ 7.573122] [c000000ff12a3cb0] [c000000000089c78] process_one_work+0x1ec/0x328
> [ 7.573132] [c000000ff12a3d40] [c00000000008a3b4] worker_thread+0x2e4/0x3a8
> [ 7.573140] [c000000ff12a3dc0] [c00000000008fed0] kthread+0x14c/0x154
> [ 7.573150] [c000000ff12a3e30] [c00000000000b594] ret_from_kernel_thread+0x5c/0xc8
> [ 7.573183] nvme nvme1: Removing after probe failure status: -19
> <snip>
> cpu 0x23: Vector: 300 (Data Access) at [c000000ff50f3800]
> pc: c0080000089a0eb0: nvme_error_detected+0x4c/0x90 [nvme]
> lr: c000000000026564: eeh_report_error+0xe0/0x110
> sp: c000000ff50f3a80
> msr: 9000000000009033
> dar: 400
> dsisr: 40000000
> current = 0xc000000ff507c000
> paca = 0xc00000000fdc9d80 softe: 0 irq_happened: 0x01
> pid = 782, comm = eehd
> Linux version 4.15.6-openpower1 (smc@smc-desktop) (gcc version 6.4.0 (Buildroot 2017.11.2-00008-g4b6188e)) #2 SM P Tue Feb 27 12:33:27 PST 2018
> enter ? for help
> [c000000ff50f3af0] c000000000026564 eeh_report_error+0xe0/0x110
> [c000000ff50f3b30] c000000000025520 eeh_pe_dev_traverse+0xc0/0xdc
> [c000000ff50f3bc0] c000000000026bd0 eeh_handle_normal_event+0x184/0x4c4
> [c000000ff50f3c70] c000000000026ff4 eeh_handle_event+0x30/0x288
> [c000000ff50f3d10] c00000000002758c eeh_event_handler+0x124/0x170
> [c000000ff50f3dc0] c00000000008fed0 kthread+0x14c/0x154
> [c000000ff50f3e30] c00000000000b594 ret_from_kernel_thread+0x5c/0xc8
>
> The first part is an EEH (on boot), the second half is the resulting
> crash. nvme probe starts the nvme_reset_work() worker thread. This
> worker thread starts touching the device which see a device error
> (EEH) and hence queues up an event in the powerpc EEH worker
> thread. nvme_reset_work() then continues and runs
> nvme_remove_dead_ctrl_work() which results in unbinding the driver
> from the device and hence releases all resources. At the same time,
> the EEH worker thread starts doing the EEH .error_detected() driver
> callback, which no longer works since the resources have been freed.
>
> This fixes the problem in the same way the generic PCIe AER code (in
> drivers/pci/pcie/aer/aerdrv_core.c) does. It makes the EEH code hold
> the device_lock() while performing the driver EEH callbacks and
> associated code. This ensures either the callbacks are no longer
> register, or if they are registered the driver will not be removed
> from underneath us.
>
> This has been broken forever. The EEH call backs were first introduced
> in 2005 (in 77bd7415610) but it's not clear if a lock was needed back
> then.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Cc: stable@vger.kernel.org
Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> v2:
> Updates based on benh's comments:
> - Change jump labels
> - Clarify where lock is held in commit message
> ---
> arch/powerpc/kernel/eeh_driver.c | 68 +++++++++++++++++++++++++---------------
> 1 file changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index 0c0b66fc5b..295ba83384 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -207,18 +207,18 @@ static void *eeh_report_error(void *data, void *userdata)
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
> +
> + device_lock(&dev->dev);
> dev->error_state = pci_channel_io_frozen;
>
> driver = eeh_pcid_get(dev);
> - if (!driver) return NULL;
> + if (!driver) goto out_no_dev;
>
> eeh_disable_irq(dev);
>
> if (!driver->err_handler ||
> - !driver->err_handler->error_detected) {
> - eeh_pcid_put(dev);
> - return NULL;
> - }
> + !driver->err_handler->error_detected)
> + goto out;
>
> rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen);
>
> @@ -227,8 +227,12 @@ static void *eeh_report_error(void *data, void *userdata)
> if (*res == PCI_ERS_RESULT_NONE) *res = rc;
>
> edev->in_error = true;
> - eeh_pcid_put(dev);
> pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
> +
> +out:
> + eeh_pcid_put(dev);
> +out_no_dev:
> + device_unlock(&dev->dev);
> return NULL;
> }
>
> @@ -251,15 +255,14 @@ static void *eeh_report_mmio_enabled(void *data, void *userdata)
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
>
> + device_lock(&dev->dev);
> driver = eeh_pcid_get(dev);
> - if (!driver) return NULL;
> + if (!driver) goto out_no_dev;
>
> if (!driver->err_handler ||
> !driver->err_handler->mmio_enabled ||
> - (edev->mode & EEH_DEV_NO_HANDLER)) {
> - eeh_pcid_put(dev);
> - return NULL;
> - }
> + (edev->mode & EEH_DEV_NO_HANDLER))
> + goto out;
>
> rc = driver->err_handler->mmio_enabled(dev);
>
> @@ -267,7 +270,10 @@ static void *eeh_report_mmio_enabled(void *data, void *userdata)
> if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
> if (*res == PCI_ERS_RESULT_NONE) *res = rc;
>
> +out:
> eeh_pcid_put(dev);
> +out_no_dev:
> + device_unlock(&dev->dev);
> return NULL;
> }
>
> @@ -290,20 +296,20 @@ static void *eeh_report_reset(void *data, void *userdata)
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
> +
> + device_lock(&dev->dev);
> dev->error_state = pci_channel_io_normal;
>
> driver = eeh_pcid_get(dev);
> - if (!driver) return NULL;
> + if (!driver) goto out_no_dev;
>
> eeh_enable_irq(dev);
>
> if (!driver->err_handler ||
> !driver->err_handler->slot_reset ||
> (edev->mode & EEH_DEV_NO_HANDLER) ||
> - (!edev->in_error)) {
> - eeh_pcid_put(dev);
> - return NULL;
> - }
> + (!edev->in_error))
> + goto out;
>
> rc = driver->err_handler->slot_reset(dev);
> if ((*res == PCI_ERS_RESULT_NONE) ||
> @@ -311,7 +317,10 @@ static void *eeh_report_reset(void *data, void *userdata)
> if (*res == PCI_ERS_RESULT_DISCONNECT &&
> rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
>
> +out:
> eeh_pcid_put(dev);
> +out_no_dev:
> + device_unlock(&dev->dev);
> return NULL;
> }
>
> @@ -362,10 +371,12 @@ static void *eeh_report_resume(void *data, void *userdata)
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
> +
> + device_lock(&dev->dev);
> dev->error_state = pci_channel_io_normal;
>
> driver = eeh_pcid_get(dev);
> - if (!driver) return NULL;
> + if (!driver) goto out_no_dev;
>
> was_in_error = edev->in_error;
> edev->in_error = false;
> @@ -375,18 +386,20 @@ static void *eeh_report_resume(void *data, void *userdata)
> !driver->err_handler->resume ||
> (edev->mode & EEH_DEV_NO_HANDLER) || !was_in_error) {
> edev->mode &= ~EEH_DEV_NO_HANDLER;
> - eeh_pcid_put(dev);
> - return NULL;
> + goto out;
> }
>
> driver->err_handler->resume(dev);
>
> - eeh_pcid_put(dev);
> pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED);
> +out:
> + eeh_pcid_put(dev);
> #ifdef CONFIG_PCI_IOV
> if (eeh_ops->notify_resume && eeh_dev_to_pdn(edev))
> eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
> #endif
> +out_no_dev:
> + device_unlock(&dev->dev);
> return NULL;
> }
>
> @@ -406,23 +419,26 @@ static void *eeh_report_failure(void *data, void *userdata)
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
> +
> + device_lock(&dev->dev);
> dev->error_state = pci_channel_io_perm_failure;
>
> driver = eeh_pcid_get(dev);
> - if (!driver) return NULL;
> + if (!driver) goto out_no_dev;
>
> eeh_disable_irq(dev);
>
> if (!driver->err_handler ||
> - !driver->err_handler->error_detected) {
> - eeh_pcid_put(dev);
> - return NULL;
> - }
> + !driver->err_handler->error_detected)
> + goto out;
>
> driver->err_handler->error_detected(dev, pci_channel_io_perm_failure);
>
> - eeh_pcid_put(dev);
> pci_uevent_ers(dev, PCI_ERS_RESULT_DISCONNECT);
> +out:
> + eeh_pcid_put(dev);
> +out_no_dev:
> + device_unlock(&dev->dev);
> return NULL;
> }
>
^ permalink raw reply
* Re: [RFC] new SYSCALL_DEFINE/COMPAT_SYSCALL_DEFINE wrappers
From: John Paul Adrian Glaubitz @ 2018-03-26 6:44 UTC (permalink / raw)
To: Linus Torvalds, Al Viro
Cc: Ingo Molnar, Dominik Brodowski, Linux Kernel Mailing List,
Arnd Bergmann, linux-arch, Ralf Baechle, James Hogan, linux-mips,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <CA+55aFw8VGnVgaWHVFP-LChMNaoANOwT18jJEWzSCRLFeRGcmA@mail.gmail.com>
Hi Linus!
On 03/26/2018 03:15 PM, Linus Torvalds wrote:
> Secretly, I was hoping to kill x32, because it's not being used afaik.
FWIW, we are maintaining an x32 port in Debian and there are some people
actually using it [1]. There is one build instance running on VMWare that
I am hosting [2] and around 10800 out of 12900 source packages build fine
on x32 (12900 being the number for x86_64).
The port is mostly stable but the fact that it's a 32-bit target with a
64-bit kernel API often blows up in people's faces because they don't
expect things like a 64-bit time_t on a 32-bit system.
The port also has some issues with software unaware of the x32 and builds
fail when they just test for __x86_64__ but not for __ILP32__, but overall
it's usable and stable and has some performance advantages over x86_64.
Adrian
> [1] http://debian-x32.org/
> [2] https://buildd.debian.org/status/architecture.php?a=x32&suite=sid
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Michal Hocko @ 2018-03-26 8:46 UTC (permalink / raw)
To: Ilya Smith
Cc: Matthew Wilcox, rth, ink, mattst88, vgupta, linux, tony.luck,
fenghua.yu, ralf, jejb, Helge Deller, benh, paulus, mpe,
schwidefsky, heiko.carstens, ysato, dalias, davem, tglx, mingo,
hpa, x86, nyc, viro, arnd, gregkh, deepa.kernel, Hugh Dickins,
kstewart, pombredanne, Andrew Morton, steve.capper, punit.agrawal,
aneesh.kumar, npiggin, Kees Cook, bhsharma, riel, nitin.m.gupta,
Kirill A. Shutemov, Dan Williams, Jan Kara, ross.zwisler,
Jerome Glisse, Andrea Arcangeli, Oleg Nesterov, linux-alpha, LKML,
linux-snps-arc, linux-ia64, linux-metag, linux-mips, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <651E0DB6-4507-4DA1-AD46-9C26ED9792A8@gmail.com>
On Fri 23-03-18 20:55:49, Ilya Smith wrote:
>
> > On 23 Mar 2018, at 15:48, Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Thu, Mar 22, 2018 at 07:36:36PM +0300, Ilya Smith wrote:
> >> Current implementation doesn't randomize address returned by mmap.
> >> All the entropy ends with choosing mmap_base_addr at the process
> >> creation. After that mmap build very predictable layout of address
> >> space. It allows to bypass ASLR in many cases. This patch make
> >> randomization of address on any mmap call.
> >
> > Why should this be done in the kernel rather than libc? libc is perfectly
> > capable of specifying random numbers in the first argument of mmap.
> Well, there is following reasons:
> 1. It should be done in any libc implementation, what is not possible IMO;
Is this really so helpful?
> 2. User mode is not that layer which should be responsible for choosing
> random address or handling entropy;
Why?
> 3. Memory fragmentation is unpredictable in this case
>
> Off course user mode could use random ‘hint’ address, but kernel may
> discard this address if it is occupied for example and allocate just before
> closest vma. So this solution doesn’t give that much security like
> randomization address inside kernel.
The userspace can use the new MAP_FIXED_NOREPLACE to probe for the
address range atomically and chose a different range on failure.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox