* Re: [RFC PATCH 0/3] Use per-CPU temporary mappings for patching
From: Christophe Leroy @ 2020-03-24 16:02 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev
In-Reply-To: <3bfcb682-a33a-b41c-74c6-4bae0d277ddf@c-s.fr>
Le 23/03/2020 à 15:04, Christophe Leroy a écrit :
>
>
> On 03/23/2020 11:30 AM, Christophe Leroy wrote:
>>
>>
>> On 03/23/2020 04:52 AM, Christopher M. Riedl wrote:
>>> When compiled with CONFIG_STRICT_KERNEL_RWX, the kernel must create
>>> temporary mappings when patching itself. These mappings temporarily
>>> override the strict RWX text protections to permit a write. Currently,
>>> powerpc allocates a per-CPU VM area for patching. Patching occurs as
>>> follows:
>>>
>>> 1. Map page of text to be patched to per-CPU VM area w/
>>> PAGE_KERNEL protection
>>> 2. Patch text
>>> 3. Remove the temporary mapping
>>>
>>> While the VM area is per-CPU, the mapping is actually inserted into the
>>> kernel page tables. Presumably, this could allow another CPU to access
>>> the normally write-protected text - either malicously or accidentally -
>>> via this same mapping if the address of the VM area is known. Ideally,
>>> the mapping should be kept local to the CPU doing the patching (or any
>>> other sensitive operations requiring temporarily overriding memory
>>> protections) [0].
>>>
>>> x86 introduced "temporary mm" structs which allow the creation of
>>> mappings local to a particular CPU [1]. This series intends to bring the
>>> notion of a temporary mm to powerpc and harden powerpc by using such a
>>> mapping for patching a kernel with strict RWX permissions.
>>>
>>> The first patch introduces the temporary mm struct and API for powerpc
>>> along with a new function to retrieve a current hw breakpoint.
>>>
>>> The second patch uses the `poking_init` init hook added by the x86
>>> patches to initialize a temporary mm and patching address. The patching
>>> address is randomized between 0 and DEFAULT_MAP_WINDOW-PAGE_SIZE. The
>>> upper limit is necessary due to how the hash MMU operates - by default
>>> the space above DEFAULT_MAP_WINDOW is not available. For now, both hash
>>> and radix randomize inside this range. The number of possible random
>>> addresses is dependent on PAGE_SIZE and limited by DEFAULT_MAP_WINDOW.
>>>
>>> Bits of entropy with 64K page size on BOOK3S_64:
>>>
>>> bits-o-entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)
>>>
>>> PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
>>> bits-o-entropy = log2(128TB / 64K)
>>> bits-o-entropy = 31
>>>
>>> Currently, randomization occurs only once during initialization at boot.
>>>
>>> The third patch replaces the VM area with the temporary mm in the
>>> patching code. The page for patching has to be mapped PAGE_SHARED with
>>> the hash MMU since hash prevents the kernel from accessing userspace
>>> pages with PAGE_PRIVILEGED bit set. There is on-going work on my side to
>>> explore if this is actually necessary in the hash codepath.
>>>
>>> Testing so far is limited to booting on QEMU (power8 and power9 targets)
>>> and a POWER8 VM along with setting some simple xmon breakpoints (which
>>> makes use of code-patching). A POC lkdtm test is in-progress to actually
>>> exploit the existing vulnerability (ie. the mapping during patching is
>>> exposed in kernel page tables and accessible by other CPUS) - this will
>>> accompany a future v1 of this series.
>>
>> Got following failures on an 8xx. Note that "fault blocked by AP
>> register !" means an unauthorised access from Kernel to Userspace.
>>
>
> Still a problem even without CONFIG_PPC_KUAP:
>
I've been able to dig into the problem.
With CONFIG_PPC_KUAP, it can definitely not work. See why in commit
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=ef296729b735e083d8919e76ac213b8ff237eb78
Without CONFIG_PPC_KUAP, on the 8xx, __put_user_asm() in
__patch_instruction() returns -EFAULT. That's because _PAGE_DIRTY is not
set on the page. Normally it should be a minor fault and the fault
handler should set the _PAGE_DIRTY flag. It must be something in the way
the page is allocated and mapped which prevents that. If I forge
_PAGE_DIRTY in addition to PAGE_SHARED, it works. But I don't think it
is valid approach to solve the issue.
Christophe
^ permalink raw reply
* Re: [PATCH v8 04/14] powerpc/vas: Alloc and setup IRQ and trigger port address
From: Cédric Le Goater @ 2020-03-24 14:48 UTC (permalink / raw)
To: Haren Myneni, mpe
Cc: mikey, Frederic Bonnard, herbert, npiggin, hch, oohall, sukadev,
linuxppc-dev, ajd
In-Reply-To: <1584598473.9256.15248.camel@hbabu-laptop>
On 3/19/20 7:14 AM, Haren Myneni wrote:
>
> Alloc IRQ and get trigger port address for each VAS instance. Kernel
> register this IRQ per VAS instance and sets this port for each send
> window. NX interrupts the kernel when it sees page fault.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/vas.c | 34 ++++++++++++++++++++++++++++------
> arch/powerpc/platforms/powernv/vas.h | 2 ++
> 2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
> index ed9cc6d..168ab68 100644
> --- a/arch/powerpc/platforms/powernv/vas.c
> +++ b/arch/powerpc/platforms/powernv/vas.c
> @@ -15,6 +15,7 @@
> #include <linux/of_address.h>
> #include <linux/of.h>
> #include <asm/prom.h>
> +#include <asm/xive.h>
>
> #include "vas.h"
>
> @@ -25,10 +26,12 @@
>
> static int init_vas_instance(struct platform_device *pdev)
> {
> - int rc, cpu, vasid;
> - struct resource *res;
> - struct vas_instance *vinst;
> struct device_node *dn = pdev->dev.of_node;
> + struct vas_instance *vinst;
> + uint32_t chipid, irq;
> + struct resource *res;
> + int rc, cpu, vasid;
> + uint64_t port;
>
> rc = of_property_read_u32(dn, "ibm,vas-id", &vasid);
> if (rc) {
> @@ -36,6 +39,12 @@ static int init_vas_instance(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + rc = of_property_read_u32(dn, "ibm,chip-id", &chipid);
> + if (rc) {
> + pr_err("No ibm,chip-id property for %s?\n", pdev->name);
> + return -ENODEV;
> + }
> +
> if (pdev->num_resources != 4) {
> pr_err("Unexpected DT configuration for [%s, %d]\n",
> pdev->name, vasid);
> @@ -69,9 +78,22 @@ static int init_vas_instance(struct platform_device *pdev)
>
> vinst->paste_win_id_shift = 63 - res->end;
>
> - pr_devel("Initialized instance [%s, %d], paste_base 0x%llx, "
> - "paste_win_id_shift 0x%llx\n", pdev->name, vasid,
> - vinst->paste_base_addr, vinst->paste_win_id_shift);
> + rc = xive_native_alloc_get_irq_info(chipid, &irq, &port);
> + if (rc)
> + return rc;
> +
> + vinst->virq = irq_create_mapping(NULL, irq);
> + if (!vinst->virq) {
> + pr_err("Inst%d: Unable to map global irq %d\n",
> + vinst->vas_id, irq);
> + return -EINVAL;
> + }
> +
> + vinst->irq_port = port;
I would prefer something like this :
hwirq = xive_native_alloc_irq_on_chip(chip_id);
vinst->virq = irq_create_mapping(NULL, hwirq);
if (!vinst->virq) {
...
}
{
struct irq_data *d = irq_get_irq_data(vinst->virq);
struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
vinst->irq_port = xd->trig_page;
}
and dump xive_native_alloc_get_irq_info().
C.
> + pr_devel("Initialized instance [%s, %d] paste_base 0x%llx paste_win_id_shift 0x%llx IRQ %d Port 0x%llx\n",
> + pdev->name, vasid, vinst->paste_base_addr,
> + vinst->paste_win_id_shift, vinst->virq,
> + vinst->irq_port);
>
> for_each_possible_cpu(cpu) {
> if (cpu_to_chip_id(cpu) == of_get_ibm_chip_id(dn))
> diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
> index 5574aec..598608b 100644
> --- a/arch/powerpc/platforms/powernv/vas.h
> +++ b/arch/powerpc/platforms/powernv/vas.h
> @@ -313,6 +313,8 @@ struct vas_instance {
> u64 paste_base_addr;
> u64 paste_win_id_shift;
>
> + u64 irq_port;
> + int virq;
> struct mutex mutex;
> struct vas_window *rxwin[VAS_COP_TYPE_MAX];
> struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
>
^ permalink raw reply
* Re: [PATCH v8 01/14] powerpc/xive: Define xive_native_alloc_irq_on_chip()
From: Cédric Le Goater @ 2020-03-24 13:48 UTC (permalink / raw)
To: Haren Myneni, mpe
Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev, ajd
In-Reply-To: <1584598352.9256.15242.camel@hbabu-laptop>
On 3/19/20 7:12 AM, Haren Myneni wrote:
>
> This function allocates IRQ on a specific chip. VAS needs per chip
> IRQ allocation and will have IRQ handler per VAS instance.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> arch/powerpc/include/asm/xive.h | 9 ++++++++-
> arch/powerpc/sysdev/xive/native.c | 6 +++---
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index 93f982db..d08ea11 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -5,6 +5,8 @@
> #ifndef _ASM_POWERPC_XIVE_H
> #define _ASM_POWERPC_XIVE_H
>
> +#include <asm/opal-api.h>
> +
> #define XIVE_INVALID_VP 0xffffffff
>
> #ifdef CONFIG_PPC_XIVE
> @@ -108,7 +110,6 @@ struct xive_q {
> int xive_native_populate_irq_data(u32 hw_irq,
> struct xive_irq_data *data);
> void xive_cleanup_irq_data(struct xive_irq_data *xd);
> -u32 xive_native_alloc_irq(void);
> void xive_native_free_irq(u32 irq);
> int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq);
>
> @@ -137,6 +138,12 @@ int xive_native_set_queue_state(u32 vp_id, uint32_t prio, u32 qtoggle,
> u32 qindex);
> int xive_native_get_vp_state(u32 vp_id, u64 *out_state);
> bool xive_native_has_queue_state_support(void);
> +extern u32 xive_native_alloc_irq_on_chip(u32 chip_id);
> +
> +static inline u32 xive_native_alloc_irq(void)
> +{
> + return xive_native_alloc_irq_on_chip(OPAL_XIVE_ANY_CHIP);
> +}
>
> #else
>
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index 0ff6b73..14d4406 100644
> --- a/arch/powerpc/sysdev/xive/native.c
> +++ b/arch/powerpc/sysdev/xive/native.c
> @@ -279,12 +279,12 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
> }
> #endif /* CONFIG_SMP */
>
> -u32 xive_native_alloc_irq(void)
> +u32 xive_native_alloc_irq_on_chip(u32 chip_id)
> {
> s64 rc;
>
> for (;;) {
> - rc = opal_xive_allocate_irq(OPAL_XIVE_ANY_CHIP);
> + rc = opal_xive_allocate_irq(chip_id);
> if (rc != OPAL_BUSY)
> break;
> msleep(OPAL_BUSY_DELAY_MS);
> @@ -293,7 +293,7 @@ u32 xive_native_alloc_irq(void)
> return 0;
> return rc;
> }
> -EXPORT_SYMBOL_GPL(xive_native_alloc_irq);
> +EXPORT_SYMBOL_GPL(xive_native_alloc_irq_on_chip);
>
> void xive_native_free_irq(u32 irq)
> {
>
^ permalink raw reply
* Re: [PATCH v8 02/14] powerpc/xive: Define xive_native_alloc_get_irq_info()
From: Cédric Le Goater @ 2020-03-24 13:51 UTC (permalink / raw)
To: Haren Myneni, mpe
Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev, ajd
In-Reply-To: <1584598402.9256.15244.camel@hbabu-laptop>
On 3/19/20 7:13 AM, Haren Myneni wrote:
>
> pnv_ocxl_alloc_xive_irq() in ocxl.c allocates IRQ and gets trigger port
> address. VAS also needs this function, but based on chip ID. So moved
> this common function to xive/native.c.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
I think we should work on a new interface for generic IPI use.
This is a beginning.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> arch/powerpc/include/asm/xive.h | 2 ++
> arch/powerpc/platforms/powernv/ocxl.c | 20 ++------------------
> arch/powerpc/sysdev/xive/native.c | 23 +++++++++++++++++++++++
> 3 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index d08ea11..fd337da 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -139,6 +139,8 @@ int xive_native_set_queue_state(u32 vp_id, uint32_t prio, u32 qtoggle,
> int xive_native_get_vp_state(u32 vp_id, u64 *out_state);
> bool xive_native_has_queue_state_support(void);
> extern u32 xive_native_alloc_irq_on_chip(u32 chip_id);
> +extern int xive_native_alloc_get_irq_info(u32 chip_id, u32 *irq,
> + u64 *trigger_addr);
>
> static inline u32 xive_native_alloc_irq(void)
> {
> diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
> index 8c65aac..fb8f99a 100644
> --- a/arch/powerpc/platforms/powernv/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/ocxl.c
> @@ -487,24 +487,8 @@ int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
>
> int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr)
> {
> - __be64 flags, trigger_page;
> - s64 rc;
> - u32 hwirq;
> -
> - hwirq = xive_native_alloc_irq();
> - if (!hwirq)
> - return -ENOENT;
> -
> - rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
> - NULL);
> - if (rc || !trigger_page) {
> - xive_native_free_irq(hwirq);
> - return -ENOENT;
> - }
> - *irq = hwirq;
> - *trigger_addr = be64_to_cpu(trigger_page);
> - return 0;
> -
> + return xive_native_alloc_get_irq_info(OPAL_XIVE_ANY_CHIP, irq,
> + trigger_addr);
alignment ^
> }
> EXPORT_SYMBOL_GPL(pnv_ocxl_alloc_xive_irq);
>
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index 14d4406..abdd892 100644
> --- a/arch/powerpc/sysdev/xive/native.c
> +++ b/arch/powerpc/sysdev/xive/native.c
> @@ -295,6 +295,29 @@ u32 xive_native_alloc_irq_on_chip(u32 chip_id)
> }
> EXPORT_SYMBOL_GPL(xive_native_alloc_irq_on_chip);
>
> +int xive_native_alloc_get_irq_info(u32 chip_id, u32 *irq, u64 *trigger_addr)
> +{
> + __be64 flags, trigger_page;
> + u32 hwirq;
> + s64 rc;
> +
> + hwirq = xive_native_alloc_irq_on_chip(chip_id);
> + if (!hwirq)
> + return -ENOENT;
> +
> + rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
> + NULL);
> + if (rc || !trigger_page) {
> + xive_native_free_irq(hwirq);
> + return -ENOENT;
> + }
> + *irq = hwirq;
> + *trigger_addr = be64_to_cpu(trigger_page);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(xive_native_alloc_get_irq_info);
> +
> void xive_native_free_irq(u32 irq)
> {
> for (;;) {
>
^ permalink raw reply
* Re: [PATCH v8 04/14] powerpc/vas: Alloc and setup IRQ and trigger port address
From: Cédric Le Goater @ 2020-03-24 13:27 UTC (permalink / raw)
To: Oliver O'Halloran
Cc: Michael Neuling, Herbert Xu, Frederic Barrat, Haren Myneni,
Nicholas Piggin, Christoph Hellwig, Sukadev Bhattiprolu,
linuxppc-dev, Andrew Donnellan
In-Reply-To: <CAOSf1CFyERZE_am5uXVY2Y65=Vkm=afd39a_2RysKR6nkqvV0A@mail.gmail.com>
On 3/24/20 3:26 AM, Oliver O'Halloran wrote:
> On Mon, Mar 23, 2020 at 8:28 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 3/23/20 10:06 AM, Cédric Le Goater wrote:
>>> On 3/19/20 7:14 AM, Haren Myneni wrote:
>>>>
>>>> Alloc IRQ and get trigger port address for each VAS instance. Kernel
>>>> register this IRQ per VAS instance and sets this port for each send
>>>> window. NX interrupts the kernel when it sees page fault.
>>>
>>> I don't understand why this is not done by the OPAL driver for each VAS
>>> of the system. Is the VAS unit very different from OpenCAPI regarding
>>> the fault ?
>>
>> I checked the previous patchsets and I see that v3 was more like I expected
>> it: one interrupt for faults allocated by the skiboot driver and exposed
>> in the DT.
>>
>> What made you change your mind ?
>
> From init_vas_inst() in arch/powerpc/platforms/powernv/vas.c:
>
> if (pdev->num_resources != 4) {
> pr_err("Unexpected DT configuration for [%s, %d]\n",
> pdev->name, vasid);
> return -ENODEV;
> }
>
> This code should never have been written, but here we are. Due to the
> above adding an interrupt in the DT makes the driver unable to bind on
> older kernels. In an older version of the patches (don't think it was
> posted) Haren was using a non-standard interrupt property and we could
> work around the problem by going back to that.
ok ... :/ I didn't know. Don't we have a rule on LinuxPPC for such
things ? Such as, the culprit should send a croissant to everyone
involved.
> However, we already have the OPAL calls for allocating / freeing
> hardware interrupt numbers so why not do that?
It's a good way to work around the problem but we are bypassing the
irqchip which does other things for the driver.
> If we ever want to take
> advantage of the job completion interrupts we'd want to have the
> ability to allocate them since the completion interrupts are
> per-window rather than per-VAS.
Yes. That's what I thought it was about to begin with. OCXL has a
first implementation of such interrupts.
>> This version is hijacking the lowlevel routines of the XIVE irqchip which
>> is not the best approach. OCXL is doing that because it needs to allocate
>> interrupts for the user space processes using the AFU and we should rework
>> that part.
>
> What'd you have in mind for the reworking the oxcl interrupt allocation?
> I didn't find it that objectionable since it's more or less the same as
> what happens when allocating IPIs.
I think we need to work a bit more on the concepts, on the interfaces,
internal at the platform kernel level and at the user space level, and
on the configuration, with chip affinity in mind. There are bunch of
information on the sources that are retrieved from the firmware or
hypervisor that we care about. An irqchip might be the best option
for the moment.
At the same time, it would be good to keep in mind user interrupts.
C.
>
>> However, the translation fault interrupt is allocated by skiboot.
>>
>> Sorry for the noise, I would like to understand more how this works. I also
>> have passthrough in mind.
>>
>> C.
^ permalink raw reply
* [PATCH 5.4 091/102] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Greg Kroah-Hartman @ 2020-03-24 13:11 UTC (permalink / raw)
To: linux-kernel
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju,
PUVICHAKRAVARTHY RAMACHANDRAN, Greg Kroah-Hartman, David Rientjes,
linuxppc-dev, stable, Bharata B Rao, Pekka Enberg, Linus Torvalds,
Kirill Tkhai, Joonsoo Kim, Andrew Morton, Michal Hocko,
Mel Gorman, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200324130806.544601211@linuxfoundation.org>
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1953,8 +1953,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2543,17 +2541,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
^ permalink raw reply
* [PATCH 5.5 099/119] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Greg Kroah-Hartman @ 2020-03-24 13:11 UTC (permalink / raw)
To: linux-kernel
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju,
PUVICHAKRAVARTHY RAMACHANDRAN, Greg Kroah-Hartman, David Rientjes,
linuxppc-dev, stable, Bharata B Rao, Pekka Enberg, Linus Torvalds,
Kirill Tkhai, Joonsoo Kim, Andrew Morton, Michal Hocko,
Mel Gorman, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200324130808.041360967@linuxfoundation.org>
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1954,8 +1954,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2544,17 +2542,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
^ permalink raw reply
* [PATCH 4.19 49/65] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Greg Kroah-Hartman @ 2020-03-24 13:11 UTC (permalink / raw)
To: linux-kernel
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju,
PUVICHAKRAVARTHY RAMACHANDRAN, Greg Kroah-Hartman, David Rientjes,
linuxppc-dev, stable, Bharata B Rao, Pekka Enberg, Linus Torvalds,
Kirill Tkhai, Joonsoo Kim, Andrew Morton, Michal Hocko,
Mel Gorman, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200324130756.679112147@linuxfoundation.org>
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1906,8 +1906,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2504,17 +2502,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
^ permalink raw reply
* Re: [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls
From: Laurent Dufour @ 2020-03-24 13:13 UTC (permalink / raw)
To: Greg Kurz, Paul Mackerras
Cc: linux-kernel, kvm-ppc, Bharata B Rao, linuxppc-dev
In-Reply-To: <20200324130052.373fdf89@bahia.lan>
Le 24/03/2020 à 13:00, Greg Kurz a écrit :
> On Tue, 24 Mar 2020 10:43:23 +1100
> Paul Mackerras <paulus@ozlabs.org> wrote:
>
>> On Fri, Mar 20, 2020 at 01:22:48PM +0100, Greg Kurz wrote:
>>> On Fri, 20 Mar 2020 11:26:42 +0100
>>> Laurent Dufour <ldufour@linux.ibm.com> wrote:
>>>
>>>> The Hcall named H_SVM_* are reserved to the Ultravisor. However, nothing
>>>> prevent a malicious VM or SVM to call them. This could lead to weird result
>>>> and should be filtered out.
>>>>
>>>> Checking the Secure bit of the calling MSR ensure that the call is coming
>>>> from either the Ultravisor or a SVM. But any system call made from a SVM
>>>> are going through the Ultravisor, and the Ultravisor should filter out
>>>> these malicious call. This way, only the Ultravisor is able to make such a
>>>> Hcall.
>>>
>>> "Ultravisor should filter" ? And what if it doesn't (eg. because of a bug) ?
>>>
>>> Shouldn't we also check the HV bit of the calling MSR as well to
>>> disambiguate SVM and UV ?
>>
>> The trouble with doing that (checking the HV bit) is that KVM does not
>> expect to see the HV bit set on an interrupt that occurred while we
>> were in the guest, and if it is set, it indicates a serious problem,
>> i.e. that an interrupt occurred while we were in the code that
>> transitions from host context to guest context, or from guest context
>> to host context. In those cases we don't know how much of the
>> transition has been completed and therefore whether we have guest
>> values or host values in the CPU registers (GPRs, FPRs/VSRs, SPRs).
>> If we do see HV set then KVM reports a severe error to userspace which
>> should cause userspace to terminate the guest.
>>
>> Therefore the UV should *always* have the HV bit clear in HSRR1/SRR1
>> when transitioning to KVM.
>>
>
> Indeed... thanks for the clarification. So I guess we'll just assume
> that the UV doesn't reflect these SVM specific hcalls if they happened
> to be issued by the guest then.
As mentioned in the series's description:
"It is assumed that the UV will filtered out such Hcalls made by a malicious
SVM."
> Cheers,
>
> --
> Greg
>
>> Paul.
>
^ permalink raw reply
* Re: [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
From: Jiri Olsa @ 2020-03-24 13:11 UTC (permalink / raw)
To: Kajol Jain
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, sukadev, ravi.bangoria, acme,
jmario, namhyung, tglx, mpetlan, gregkh, linux-kernel,
linux-perf-users, jolsa, linuxppc-dev
In-Reply-To: <20200320125406.30995-10-kjain@linux.ibm.com>
On Fri, Mar 20, 2020 at 06:24:04PM +0530, Kajol Jain wrote:
> Patch enhances current metric infrastructure to handle "?" in the metric
> expression. The "?" can be use for parameters whose value not known while
> creating metric events and which can be replace later at runtime to
> the proper value. It also add flexibility to create multiple events out
> of single metric event added in json file.
>
> Patch adds function 'arch_get_runtimeparam' which is a arch specific
> function, returns the count of metric events need to be created.
> By default it return 1.
>
> This infrastructure needed for hv_24x7 socket/chip level events.
> "hv_24x7" chip level events needs specific chip-id to which the
> data is requested. Function 'arch_get_runtimeparam' implemented
> in header.c which extract number of sockets from sysfs file
> "sockets" under "/sys/devices/hv_24x7/interface/".
>
>
> With this patch basically we are trying to create as many metric events
> as define by runtime_param.
>
> For that one loop is added in function 'metricgroup__add_metric',
> which create multiple events at run time depend on return value of
> 'arch_get_runtimeparam' and merge that event in 'group_list'.
>
> To achieve that we are actually passing this parameter value as part of
> `expr__find_other` function and changing "?" present in metric expression
> with this value.
>
> As in our json file, there gonna be single metric event, and out of
> which we are creating multiple events, I am also merging this value
> to the original metric name to specify parameter value.
>
> For example,
> command:# ./perf stat -M PowerBUS_Frequency -C 0 -I 1000
> # time counts unit events
> 1.000101867 9,356,933 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
> 1.000101867 9,366,134 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
> 2.000314878 9,365,868 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
> 2.000314878 9,366,092 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
>
> So, here _0 and _1 after PowerBUS_Frequency specify parameter value.
>
> As after adding this to group_list, again we call expr__parse
> in 'generic_metric' function present in util/stat-display.c.
> By this time again we need to pass this parameter value. So, now to get this value
> actually I am trying to extract it from metric name itself. Because
> otherwise it gonna point to last updated value present in runtime_param.
> And gonna match for that value only.
so why can't we pass that param as integer value through the metric objects?
it get's created in metricgroup__add_metric_param:
- as struct egroup *eg
- we can add egroup::param and store the param value there
then in metricgroup__setup_events it moves to:
- struct metric_expr *expr
- we can add metric_expr::param to keep the param
then in perf_stat__print_shadow_stats there's:
- struct metric_expr *mexp loop
- calling generic_metric metric - we could call it with mexp::param
- and pass the param to expr__parse
jirka
^ permalink raw reply
* Re: [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
From: Jiri Olsa @ 2020-03-24 13:11 UTC (permalink / raw)
To: Kajol Jain
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, sukadev, ravi.bangoria, acme,
jmario, namhyung, tglx, mpetlan, gregkh, linux-kernel,
linux-perf-users, jolsa, linuxppc-dev
In-Reply-To: <20200320125406.30995-10-kjain@linux.ibm.com>
On Fri, Mar 20, 2020 at 06:24:04PM +0530, Kajol Jain wrote:
SNIP
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 52fb119d25c8..b4b91d8ad5be 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -474,8 +474,13 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
> return false;
> }
>
> +int __weak arch_get_runtimeparam(void)
> +{
> + return 1;
> +}
> +
> static int metricgroup__add_metric_param(struct strbuf *events,
> - struct list_head *group_list, struct pmu_event *pe)
> + struct list_head *group_list, struct pmu_event *pe, int param)
> {
>
> const char **ids;
> @@ -483,7 +488,7 @@ static int metricgroup__add_metric_param(struct strbuf *events,
could you please call this function __metricgroup__add_metric instead?
> struct egroup *eg;
> int ret = -EINVAL;
>
> - if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
> + if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum, param) < 0)
> return ret;
>
> if (events->len > 0)
> @@ -502,11 +507,21 @@ static int metricgroup__add_metric_param(struct strbuf *events,
>
> eg->ids = ids;
> eg->idnum = idnum;
> - eg->metric_name = pe->metric_name;
> + if (strstr(pe->metric_expr, "?")) {
> + char value[PATH_MAX];
> +
> + sprintf(value, "%s%c%d", pe->metric_name, '_', param);
> + eg->metric_name = strdup(value);
how is eg->metric_name getting released?
> + if (!eg->metric_name) {
> + ret = -ENOMEM;
> + return ret;
return -ENOMEM; ??
> + }
> + }
> + else
> + eg->metric_name = pe->metric_name;
> eg->metric_expr = pe->metric_expr;
> eg->metric_unit = pe->unit;
> list_add_tail(&eg->nd, group_list);
> -
> return 0;
> }
>
SNIP
thanks,
jirka
^ permalink raw reply
* Re: [PATCH v8 04/14] powerpc/vas: Alloc and setup IRQ and trigger port address
From: Cédric Le Goater @ 2020-03-24 12:20 UTC (permalink / raw)
To: Haren Myneni
Cc: mikey, herbert, Frederic Barrat, npiggin, hch, oohall, sukadev,
linuxppc-dev, ajd
In-Reply-To: <1584990135.9256.15381.camel@hbabu-laptop>
On 3/23/20 8:02 PM, Haren Myneni wrote:
> On Mon, 2020-03-23 at 10:27 +0100, Cédric Le Goater wrote:
>> On 3/23/20 10:06 AM, Cédric Le Goater wrote:
>>> On 3/19/20 7:14 AM, Haren Myneni wrote:
>>>>
>>>> Alloc IRQ and get trigger port address for each VAS instance. Kernel
>>>> register this IRQ per VAS instance and sets this port for each send
>>>> window. NX interrupts the kernel when it sees page fault.
>>>
>>> I don't understand why this is not done by the OPAL driver for each VAS
>>> of the system. Is the VAS unit very different from OpenCAPI regarding
>>> the fault ?
>>
>> I checked the previous patchsets and I see that v3 was more like I expected
>> it: one interrupt for faults allocated by the skiboot driver and exposed
>> in the DT.
>>
>> What made you change your mind ?
>>
>> This version is hijacking the lowlevel routines of the XIVE irqchip which
>> is not the best approach. OCXL is doing that because it needs to allocate
>> interrupts for the user space processes using the AFU and we should rework
>> that part.
>>
>> However, the translation fault interrupt is allocated by skiboot.
>
> Sorry my mistake. I should have CC you earlier.
>
> Each VAS instance will generate fault interrupt which is per chip. There
> won't be other job completion interrupts.
That's a very good reason to set everything in the skiboot driver and
advertise the interrupt number in the DT. The interrupt will be mapped
automatically by OF routines and the driver will only have to install
an interrupt handler.
> Correct, V3 used allocating interrupts per chip in skiboot and exposed
> in DT. Since XIVE code has similar feature, exploited this approach so
> that we do not need skiboot changes.
It's not the same. These are the low level (OPAL) interface used by the
XIVE driver. The exception is the KVM XIVE device which needs a finer
grain.
C.
^ permalink raw reply
* Re: [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls
From: Greg Kurz @ 2020-03-24 12:00 UTC (permalink / raw)
To: Paul Mackerras
Cc: linux-kernel, kvm-ppc, Bharata B Rao, Laurent Dufour,
linuxppc-dev
In-Reply-To: <20200323234323.GA5604@blackberry>
On Tue, 24 Mar 2020 10:43:23 +1100
Paul Mackerras <paulus@ozlabs.org> wrote:
> On Fri, Mar 20, 2020 at 01:22:48PM +0100, Greg Kurz wrote:
> > On Fri, 20 Mar 2020 11:26:42 +0100
> > Laurent Dufour <ldufour@linux.ibm.com> wrote:
> >
> > > The Hcall named H_SVM_* are reserved to the Ultravisor. However, nothing
> > > prevent a malicious VM or SVM to call them. This could lead to weird result
> > > and should be filtered out.
> > >
> > > Checking the Secure bit of the calling MSR ensure that the call is coming
> > > from either the Ultravisor or a SVM. But any system call made from a SVM
> > > are going through the Ultravisor, and the Ultravisor should filter out
> > > these malicious call. This way, only the Ultravisor is able to make such a
> > > Hcall.
> >
> > "Ultravisor should filter" ? And what if it doesn't (eg. because of a bug) ?
> >
> > Shouldn't we also check the HV bit of the calling MSR as well to
> > disambiguate SVM and UV ?
>
> The trouble with doing that (checking the HV bit) is that KVM does not
> expect to see the HV bit set on an interrupt that occurred while we
> were in the guest, and if it is set, it indicates a serious problem,
> i.e. that an interrupt occurred while we were in the code that
> transitions from host context to guest context, or from guest context
> to host context. In those cases we don't know how much of the
> transition has been completed and therefore whether we have guest
> values or host values in the CPU registers (GPRs, FPRs/VSRs, SPRs).
> If we do see HV set then KVM reports a severe error to userspace which
> should cause userspace to terminate the guest.
>
> Therefore the UV should *always* have the HV bit clear in HSRR1/SRR1
> when transitioning to KVM.
>
Indeed... thanks for the clarification. So I guess we'll just assume
that the UV doesn't reflect these SVM specific hcalls if they happened
to be issued by the guest then.
Cheers,
--
Greg
> Paul.
^ permalink raw reply
* Re: [PATCH v4] powerpc: Replace setup_irq() by request_irq()
From: afzal mohammed @ 2020-03-24 11:06 UTC (permalink / raw)
To: Michael Ellerman, Thomas Gleixner
Cc: linux-kernel, Scott Wood, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200312064256.18735-1-afzal.mohd.ma@gmail.com>
Hi Michael Ellerman,
On Thu, Mar 12, 2020 at 12:12:55PM +0530, afzal mohammed wrote:
> request_irq() is preferred over setup_irq(). Invocations of setup_irq()
> occur after memory allocators are ready.
>
> Per tglx[1], setup_irq() existed in olden days when allocators were not
> ready by the time early interrupts were initialized.
>
> Hence replace setup_irq() by request_irq().
>
> [1] https://lkml.kernel.org/r/alpine.DEB.2.20.1710191609480.1971@nanos
>
> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
This patch is seen in next-test branch for last 4-5 days, i don't know
exactly how powerpc workflow happens, so a question - this would be
appear in linux-next soon right ? (for last 4-5 days i had been daily
checking -next, but not appearing there).
Sorry for the query for this trivial patch, i am asking because Thomas
had mentioned [1] to get setup_irq() cleanup thr' respective
maintainers (earlier it was part of tree-wide series), check -next after
-rc6 & resubmit ignored ones to him, this patch is neither in -next,
neither ignored, so i am at a loss what to do :(
And i would prefer to let each patch go thr' respective maintainers so
that only least patches has to be sent to Thomas. Bigger problem is that
core removal patch of setup_irq() can be sent to him only after making
sure that it's tree-wide usage has been removed.
Regards
afzal
[1] https://lkml.kernel.org/r/87y2somido.fsf@nanos.tec.linutronix.de
^ permalink raw reply
* Re: [5.6.0-rc7] Kernel crash while running ndctl tests
From: Baoquan He @ 2020-03-24 10:14 UTC (permalink / raw)
To: Aneesh Kumar K.V, Sachin Sant
Cc: Dan Williams, linuxppc-dev, LKML, linux-nvdimm
In-Reply-To: <33E32320-C371-4A41-A3E1-4B9D2DDAFBFC@linux.vnet.ibm.com>
On 03/24/20 at 03:06pm, Sachin Sant wrote:
>
>
> > On 24-Mar-2020, at 2:45 PM, Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote:
> >
> > Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
> >
> >> While running ndctl[1] tests against 5.6.0-rc7 following crash is encountered.
> >>
> >> Bisect leads me to commit d41e2f3bd546
> >> mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case
> >>
> >> Reverting this commit helps and the tests complete without any crash.
> >
> >
> > Can you try this change?
> >
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index aadb7298dcef..3012d1f3771a 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> > ms->usage = NULL;
> > }
> > memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > + /* Mark the section invalid */
> > + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> > }
> >
> > if (section_is_early && memmap)
> >
>
> This patch works for me. The test ran successfully without any crash/failure.
Hi Aneesh,
Could you make a formal patch to post, since Sachin has tested and
confirmed it works?
>
> Thanks
> -Sachin
>
> > a pfn_valid check involves pnf_section_valid() check if section is
> > having MEM_MAP. In this case we did end up setting the ms->uage = NULL.
> > So when we do that tupdate the section to not have MEM_MAP.
> >
> > -aneesh
>
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to, struct device
From: Christian Zigotzky @ 2020-03-24 9:39 UTC (permalink / raw)
To: linuxppc-dev
Hi All,
The DMA mapping works great on our PowerPC machines currently. It was a
long way to get the new DMA mapping code to work successfully on our
PowerPC machines.
P L E A S E don't modify the good working DMA mapping code. There are
many other topics which needs improvements. For us (first level + second
level support) it is really laborious to find your problematic code and
patch it. It takes a long time to find the problematic code because we
have to do it besides our main work.
P L E A S E test your code on PowerPC machines before you add it to the
mainline vanilla kernel.
Thanks,
Christian
On Tue, Mar 24, 2020 at 12:00:09PM +0530, Aneesh Kumar K.V wrote:
> dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
> unsigned long offset, size_t size, enum dma_data_direction dir,
> unsigned long attrs)
> {
> phys_addr_t phys = page_to_phys(page) + offset;
> dma_addr_t dma_addr = phys_to_dma(dev, phys);
>
> if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
> return iommu_map(dev, phys, size, dir, attrs);
>
> return DMA_MAPPING_ERROR;
If powerpc hardware / firmware people really come up with crap that
stupid you'll have to handle it yourself and will always pay the
indirect call penality.
^ permalink raw reply
* Re: [5.6.0-rc7] Kernel crash while running ndctl tests
From: Sachin Sant @ 2020-03-24 9:36 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Dan Williams, linuxppc-dev, LKML, Baoquan He, linux-nvdimm
In-Reply-To: <87a746cdva.fsf@linux.ibm.com>
> On 24-Mar-2020, at 2:45 PM, Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote:
>
> Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
>
>> While running ndctl[1] tests against 5.6.0-rc7 following crash is encountered.
>>
>> Bisect leads me to commit d41e2f3bd546
>> mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case
>>
>> Reverting this commit helps and the tests complete without any crash.
>
>
> Can you try this change?
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index aadb7298dcef..3012d1f3771a 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> ms->usage = NULL;
> }
> memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> + /* Mark the section invalid */
> + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> }
>
> if (section_is_early && memmap)
>
This patch works for me. The test ran successfully without any crash/failure.
Thanks
-Sachin
> a pfn_valid check involves pnf_section_valid() check if section is
> having MEM_MAP. In this case we did end up setting the ms->uage = NULL.
> So when we do that tupdate the section to not have MEM_MAP.
>
> -aneesh
^ permalink raw reply
* Re: [5.6.0-rc7] Kernel crash while running ndctl tests
From: Aneesh Kumar K.V @ 2020-03-24 9:15 UTC (permalink / raw)
To: Sachin Sant, LKML, linuxppc-dev; +Cc: Dan Williams, Baoquan He, linux-nvdimm
In-Reply-To: <CF20E440-4DCB-4EFD-88B6-0AB98DC7FBD4@linux.vnet.ibm.com>
Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
> While running ndctl[1] tests against 5.6.0-rc7 following crash is encountered.
>
> Bisect leads me to commit d41e2f3bd546
> mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case
>
> Reverting this commit helps and the tests complete without any crash.
>
> pmem0: detected capacity change from 0 to 10720641024
> BUG: Kernel NULL pointer dereference on read at 0x00000000
> Faulting instruction address: 0xc000000000c3447c
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in: dm_mod nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip6_tables nft_compat ip_set rfkill nf_tables nfnetlink sunrpc sg pseries_rng papr_scm uio_pdrv_genirq uio sch_fq_codel ip_tables sd_mod t10_pi ibmvscsi scsi_transport_srp ibmveth
> CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
> NIP: c000000000c3447c LR: c000000000088354 CTR: c00000000018e990
> REGS: c0000006223fb630 TRAP: 0300 Not tainted (5.6.0-rc7-autotest)
> MSR: 800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 24048888 XER: 00000000
> CFAR: c00000000000dec4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
> GPR00: c0000000003c5820 c0000006223fb8c0 c000000001684900 0000000004000000
> GPR04: c00c000101000000 0000000007ffffff c00000067ff20900 c00c000000000000
> GPR08: 0000000000000000 c00c000100000000 0000000000000000 c000000003f00000
> GPR12: 0000000000008000 c00000001ec70200 00007fffc102f9e8 000000001002e088
> GPR16: 0000000000000000 0000000010050d88 000000001002f778 000000001002f770
> GPR20: 0000000000000000 0000000000000100 0000000000000001 0000000000001000
> GPR24: 0000000000000008 0000000000000000 0000000004000000 c00c000100004000
> GPR28: c000000003101aa0 c00c000100000000 0000000001000000 0000000004000100
> NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
> LR [c000000000088354] vmemmap_free+0x144/0x320
> Call Trace:
> [c0000006223fb8c0] [c0000006223fb960] 0xc0000006223fb960 (unreliable)
> [c0000006223fb980] [c0000000003c5820] section_deactivate+0x220/0x240
> [c0000006223fba30] [c0000000003dc1d8] __remove_pages+0x118/0x170
> [c0000006223fba80] [c000000000086e5c] arch_remove_memory+0x3c/0x150
> [c0000006223fbb00] [c00000000041a3bc] memunmap_pages+0x1cc/0x2f0
> [c0000006223fbb80] [c0000000007d6d00] devm_action_release+0x30/0x50
> [c0000006223fbba0] [c0000000007d7de8] release_nodes+0x2f8/0x3e0
> [c0000006223fbc50] [c0000000007d0b38] device_release_driver_internal+0x168/0x270
> [c0000006223fbc90] [c0000000007ccf50] unbind_store+0x130/0x170
> [c0000006223fbcd0] [c0000000007cc0b4] drv_attr_store+0x44/0x60
> [c0000006223fbcf0] [c00000000051fdb8] sysfs_kf_write+0x68/0x80
> [c0000006223fbd10] [c00000000051f200] kernfs_fop_write+0x100/0x290
> [c0000006223fbd60] [c00000000042037c] __vfs_write+0x3c/0x70
> [c0000006223fbd80] [c00000000042404c] vfs_write+0xcc/0x240
> [c0000006223fbdd0] [c00000000042442c] ksys_write+0x7c/0x140
> [c0000006223fbe20] [c00000000000b278] system_call+0x5c/0x68
> Instruction dump:
> 2ea80000 4196003c 794a2428 7d685215 41820030 7d48502a 71480002 41820024
> 714a0008 4082002c e90b0008 786adf62 <e8680000> 7c635436 70630001 4c820020
> ---[ end trace 579b48162da1b890 ]—
Can you try this change?
diff --git a/mm/sparse.c b/mm/sparse.c
index aadb7298dcef..3012d1f3771a 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
ms->usage = NULL;
}
memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
+ /* Mark the section invalid */
+ ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
}
if (section_is_early && memmap)
a pfn_valid check involves pnf_section_valid() check if section is
having MEM_MAP. In this case we did end up setting the ms->uage = NULL.
So when we do that tupdate the section to not have MEM_MAP.
-aneesh
^ permalink raw reply related
* Re: [PATCH v11 5/8] powerpc/64: make buildable without CONFIG_COMPAT
From: Nicholas Piggin @ 2020-03-24 8:54 UTC (permalink / raw)
To: linuxppc-dev, Michal Suchanek
Cc: Mark Rutland, Gustavo Luiz Duarte, Alexander Shishkin,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Eric Richter,
Masahiro Yamada, Nayna Jain, Peter Zijlstra, Ingo Molnar,
Hari Bathini, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Allison Randal,
Greg Kroah-Hartman, Claudio Carvalho, Mauro Carvalho Chehab,
Eric W. Biederman, linux-fsdevel, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <4b7058eb0f5558fb7e2cee1b8f7cf99ebd03084e.1584620202.git.msuchanek@suse.de>
Michal Suchanek's on March 19, 2020 10:19 pm:
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index 4b0152108f61..a264989626fd 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -247,7 +247,6 @@ static void do_signal(struct task_struct *tsk)
> sigset_t *oldset = sigmask_to_save();
> struct ksignal ksig = { .sig = 0 };
> int ret;
> - int is32 = is_32bit_task();
>
> BUG_ON(tsk != current);
>
> @@ -277,7 +276,7 @@ static void do_signal(struct task_struct *tsk)
>
> rseq_signal_deliver(&ksig, tsk->thread.regs);
>
> - if (is32) {
> + if (is_32bit_task()) {
> if (ksig.ka.sa.sa_flags & SA_SIGINFO)
> ret = handle_rt_signal32(&ksig, oldset, tsk);
> else
Unnecessary?
> diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
> index 87d95b455b83..2dcbfe38f5ac 100644
> --- a/arch/powerpc/kernel/syscall_64.c
> +++ b/arch/powerpc/kernel/syscall_64.c
> @@ -24,7 +24,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
> long r6, long r7, long r8,
> unsigned long r0, struct pt_regs *regs)
> {
> - unsigned long ti_flags;
> syscall_fn f;
>
> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> @@ -68,8 +67,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
>
> local_irq_enable();
>
> - ti_flags = current_thread_info()->flags;
> - if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
> + if (unlikely(current_thread_info()->flags & _TIF_SYSCALL_DOTRACE)) {
> /*
> * We use the return value of do_syscall_trace_enter() as the
> * syscall number. If the syscall was rejected for any reason
> @@ -94,7 +92,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> /* May be faster to do array_index_nospec? */
> barrier_nospec();
>
> - if (unlikely(ti_flags & _TIF_32BIT)) {
> + if (unlikely(is_32bit_task())) {
Problem is, does this allow the load of ti_flags to be used for both
tests, or does test_bit make it re-load?
This could maybe be fixed by testing if(IS_ENABLED(CONFIG_COMPAT) &&
Other than these, the patches all look pretty good to me.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v11 3/8] powerpc/perf: consolidate read_user_stack_32
From: Nicholas Piggin @ 2020-03-24 8:48 UTC (permalink / raw)
To: linuxppc-dev, Michal Suchanek
Cc: Mark Rutland, Gustavo Luiz Duarte, Alexander Shishkin,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Eric Richter,
Masahiro Yamada, Nayna Jain, Peter Zijlstra, Ingo Molnar,
Hari Bathini, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Allison Randal,
Greg Kroah-Hartman, Claudio Carvalho, Mauro Carvalho Chehab,
Eric W. Biederman, linux-fsdevel, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <184347595442b4ca664613008a09e8cea7188c36.1584620202.git.msuchanek@suse.de>
Michal Suchanek's on March 19, 2020 10:19 pm:
> There are two almost identical copies for 32bit and 64bit.
>
> The function is used only in 32bit code which will be split out in next
> patch so consolidate to one function.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> v6: new patch
> v8: move the consolidated function out of the ifdef block.
> v11: rebase on top of def0bfdbd603
> ---
> arch/powerpc/perf/callchain.c | 48 +++++++++++++++++------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
> index cbc251981209..c9a78c6e4361 100644
> --- a/arch/powerpc/perf/callchain.c
> +++ b/arch/powerpc/perf/callchain.c
> @@ -161,18 +161,6 @@ static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
> return read_user_stack_slow(ptr, ret, 8);
> }
>
> -static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> -{
> - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> - ((unsigned long)ptr & 3))
> - return -EFAULT;
> -
> - if (!probe_user_read(ret, ptr, sizeof(*ret)))
> - return 0;
> -
> - return read_user_stack_slow(ptr, ret, 4);
> -}
> -
> static inline int valid_user_sp(unsigned long sp, int is_64)
> {
> if (!sp || (sp & 7) || sp > (is_64 ? TASK_SIZE : 0x100000000UL) - 32)
> @@ -277,19 +265,9 @@ static void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
> }
>
> #else /* CONFIG_PPC64 */
> -/*
> - * On 32-bit we just access the address and let hash_page create a
> - * HPTE if necessary, so there is no need to fall back to reading
> - * the page tables. Since this is called at interrupt level,
> - * do_page_fault() won't treat a DSI as a page fault.
> - */
> -static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> +static int read_user_stack_slow(void __user *ptr, void *buf, int nb)
> {
> - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> - ((unsigned long)ptr & 3))
> - return -EFAULT;
> -
> - return probe_user_read(ret, ptr, sizeof(*ret));
> + return 0;
> }
>
> static inline void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
> @@ -312,6 +290,28 @@ static inline int valid_user_sp(unsigned long sp, int is_64)
>
> #endif /* CONFIG_PPC64 */
>
> +/*
> + * On 32-bit we just access the address and let hash_page create a
> + * HPTE if necessary, so there is no need to fall back to reading
> + * the page tables. Since this is called at interrupt level,
> + * do_page_fault() won't treat a DSI as a page fault.
> + */
The comment is actually probably better to stay in the 32-bit
read_user_stack_slow implementation. Is that function defined
on 32-bit purely so that you can use IS_ENABLED()? In that case
I would prefer to put a BUG() there which makes it self documenting.
Thanks,
Nick
> +static int read_user_stack_32(unsigned int __user *ptr, unsigned int *ret)
> +{
> + int rc;
> +
> + if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> + ((unsigned long)ptr & 3))
> + return -EFAULT;
> +
> + rc = probe_user_read(ret, ptr, sizeof(*ret));
> +
> + if (IS_ENABLED(CONFIG_PPC64) && rc)
> + return read_user_stack_slow(ptr, ret, 4);
> +
> + return rc;
> +}
> +
> /*
> * Layout for non-RT signal frames
> */
> --
> 2.23.0
>
>
^ permalink raw reply
* Re: [RESEND PATCH v2 6/9] drm/mgag200: Constify ioreadX() iomem argument (as in generic implementation)
From: Thomas Zimmermann @ 2020-03-24 8:39 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rich Felker, Jiri Slaby, Geert Uytterhoeven, Michael S. Tsirkin,
David Airlie, Jason Wang, dri-devel, virtualization,
James E.J. Bottomley, netdev, Paul Mackerras, linux-arch,
Dave Jiang, Yoshinori Sato, Helge Deller, linux-sh,
Alexey Brodkin, Ben Skeggs, nouveau, Dave Airlie, Matt Turner,
linux-snps-arc, Nick Kossifidis, Allen Hubbe, Arnd Bergmann,
linux-alpha, Ivan Kokshaysky, Thomas Gleixner,
Mauro Carvalho Chehab, Kalle Valo, Richard Henderson,
linux-parisc, Vineet Gupta, linux-wireless, linux-kernel,
Luis Chamberlain, Daniel Vetter, Jon Mason, linux-ntb,
Andrew Morton, linux-media, linuxppc-dev, David S. Miller
In-Reply-To: <20200314105944.GA16044@kozik-lap>
[-- Attachment #1.1: Type: text/plain, Size: 883 bytes --]
Hi
Am 14.03.20 um 11:59 schrieb Krzysztof Kozlowski:
> On Thu, Mar 12, 2020 at 11:49:05AM +0100, Thomas Zimmermann wrote:
>> Hi Krzysztof,
>>
>> I just received a resend email from 3 weeks ago :/
>>
>> Do you want me to merge the mgag200 patch into drm-misc-next?
>
> Thanks but it depends on the first patch in the series so either it
> could go with your ack through other tree or I will send it later (once
> 1st patch gets to mainline).
Ok. You're welcome to send it through any tree that works best for you.
mgag200 sees only little change. I wouldn't expect major merge
conflicts, if any.
Best regards
Thomas
>
>
> Best regards,
> Krzysztof
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] Documentation: Clarify better about the rwsem non-owner release issue
From: Will Deacon @ 2020-03-24 8:15 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-doc, Peter Zijlstra, Sebastian Andrzej Siewior,
linux-kernel, Thomas Gleixner, Davidlohr Bueso, Arnd Bergmann,
Jonathan Corbet, Ingo Molnar, Linus Torvalds, Ingo Molnar,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo,
linux-pci@vger.kernel.org Felipe Balbi, Greg Kroah-Hartman,
Randy Dunlap, linux-wireless, Oleg Nesterov, netdev,
Logan Gunthorpe, David S. Miller
In-Reply-To: <20200323182349.GA203600@google.com>
On Mon, Mar 23, 2020 at 02:23:49PM -0400, Joel Fernandes wrote:
> On Sun, Mar 22, 2020 at 08:51:15AM +0200, Kalle Valo wrote:
> > "Joel Fernandes (Google)" <joel@joelfernandes.org> writes:
> >
> > > Reword and clarify better about the rwsem non-owner release issue.
> > >
> > > Link: https://lore.kernel.org/linux-pci/20200321212144.GA6475@google.com/
> > >
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> >
> > There's something wrong with your linux-pci and linux-usb addresses:
> >
> > "linux-pci@vger.kernel.org Felipe Balbi" <balbi@kernel.org>,
> >
> >
> > "linux-usb@vger.kernel.org Kalle Valo" <kvalo@codeaurora.org>,
>
> Not sure. It appears fine in the archive.
Hmm, I don't think it does. Here's the copy from LKML:
https://lore.kernel.org/lkml/20200322021938.175736-1-joel@joelfernandes.org/
Which works because it's in the To: correctly. But both linux-pci and
linux-usb were *not* CC'd:
"linux-usb@vger.kernel.org Kalle Valo" <kvalo@codeaurora.org>
"linux-pci@vger.kernel.org Felipe Balbi" <balbi@kernel.org>
and searching for the message in the linux-pci archives doesn't find it:
https://lore.kernel.org/linux-pci/?q=Reword+and+clarify+better+about+the+rwsem+non-owner+release+issue
So it looks like there is an issue with your mail setup.
Will
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-03-24 7:55 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Alexey Kardashevskiy, Greg Kroah-Hartman, Joerg Roedel,
Robin Murphy, linux-kernel, iommu, linuxppc-dev,
Christoph Hellwig, Lu Baolu
In-Reply-To: <87pnd22rke.fsf@linux.ibm.com>
On Tue, Mar 24, 2020 at 12:00:09PM +0530, Aneesh Kumar K.V wrote:
> dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
> unsigned long offset, size_t size, enum dma_data_direction dir,
> unsigned long attrs)
> {
> phys_addr_t phys = page_to_phys(page) + offset;
> dma_addr_t dma_addr = phys_to_dma(dev, phys);
>
> if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
> return iommu_map(dev, phys, size, dir, attrs);
>
> return DMA_MAPPING_ERROR;
If powerpc hardware / firmware people really come up with crap that
stupid you'll have to handle it yourself and will always pay the
indirect call penality.
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-03-24 7:54 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
iommu, Aneesh Kumar K.V, linuxppc-dev, Christoph Hellwig,
Lu Baolu
In-Reply-To: <ffce1af6-a215-dee8-7b5c-2111f43accfd@ozlabs.ru>
On Tue, Mar 24, 2020 at 02:05:54PM +1100, Alexey Kardashevskiy wrote:
> This is for persistent memory which you can DMA to/from but yet it does
> not appear in the system as a normal memory and therefore requires
> special handling anyway (O_DIRECT or DAX, I do not know the exact
> mechanics). All other devices in the system should just run as usual,
> i.e. use 1:1 mapping if possible.
On other systems (x86 and arm) pmem as long as it is page backed does
not require any special handling. This must be some weird way powerpc
fucked up again, and I suspect you'll have to suffer from it.
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-03-24 7:52 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
iommu, Aneesh Kumar K.V, linuxppc-dev, Christoph Hellwig,
Lu Baolu
In-Reply-To: <d4bf6058-aa77-d0bc-8196-f4c27fb21b74@ozlabs.ru>
On Tue, Mar 24, 2020 at 02:37:59PM +1100, Alexey Kardashevskiy wrote:
> dma_alloc_direct() and dma_map_direct() do the same thing now which is
> good, did I miss anything else?
dma_alloc_direct looks at coherent_dma_mask, dma_map_direct looks
at dma_mask.
^ 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;
as well as URLs for NNTP newsgroup(s).