LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc/kasan: Fix early region not updated correctly
From: ChenJingwen @ 2022-01-20  3:21 UTC (permalink / raw)
  To: chenjingwen6; +Cc: christophe.leroy, kasan-dev, paulus, linuxppc-dev
In-Reply-To: <20211229035226.59159-1-chenjingwen6@huawei.com>

From: Chen Jingwen <chenjingwen6@huawei.com>

Hi, It can be reproduced with the following kernel configs.
make corenet32_smp_defconfig

CONFIG_PPC_QEMU_E500=y
CONFIG_KASAN=y
CONFIG_KASAN_GENERIC=y
CONFIG_KASAN_OUTLINE=y
# CONFIG_KASAN_INLINE is not set
CONFIG_KASAN_STACK=y
CONFIG_KASAN_VMALLOC=y

And boot the kernel with the rootfs created by buildroot-2021.08.1
qemu-system-ppc -M ppce500 -cpu e500mc -m 256 -kernel /code/linux/vmlinux \
-drive file=output/images/rootfs.ext2,if=virtio,format=raw \
-append "console=ttyS0 rootwait root=/dev/vda" -serial mon:stdio -nographic

Could you help review this patch?
I will add the necessary info if any is needed.

^ permalink raw reply

* [PATCH v2] spi: mpc512x-psc: Convert to use GPIO descriptors
From: Linus Walleij @ 2022-01-20  0:26 UTC (permalink / raw)
  To: Mark Brown, linux-spi
  Cc: Linus Walleij, Anatolij Gustschin, linuxppc-dev,
	Uwe Kleine-König

This driver is already relying on the core to provide
valid GPIO numbers in spi->cs_gpio through
of_spi_get_gpio_numbers(), so we can switch to letting
the core use GPIO descriptors instead.

The driver was assigning a local function to the custom
chipselect callback, but I chose to just open code the
gpiod setting instead, this is easier to read.

The only platform that overrides the cs_control callback
is the mpc832x_rdb.

Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Stop trying to use the core per-transfer CS handling,
  keep the old code intact around this and just deal with
  the GPIO descriptors.

I can't really compile any PPC code, I just hope I coded
this right...
---
 drivers/spi/spi-mpc512x-psc.c | 46 ++++++++++++++---------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index 78a9bca8cc68..8a488d8e4c1b 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -23,7 +23,6 @@
 #include <linux/clk.h>
 #include <linux/spi/spi.h>
 #include <linux/fsl_devices.h>
-#include <linux/gpio.h>
 #include <asm/mpc52xx_psc.h>
 
 enum {
@@ -128,17 +127,28 @@ static void mpc512x_psc_spi_activate_cs(struct spi_device *spi)
 	out_be32(psc_addr(mps, ccr), ccr);
 	mps->bits_per_word = cs->bits_per_word;
 
-	if (mps->cs_control && gpio_is_valid(spi->cs_gpio))
-		mps->cs_control(spi, (spi->mode & SPI_CS_HIGH) ? 1 : 0);
+	if (cs->gpiod) {
+		if (mps->cs_control)
+			/* boardfile override */
+			mps->cs_control(spi, (spi->mode & SPI_CS_HIGH) ? 1 : 0);
+		else
+			/* gpiolib will deal with the inversion */
+			gpiod_set_value(spi->cs_gpiod, 1);
+	}
 }
 
 static void mpc512x_psc_spi_deactivate_cs(struct spi_device *spi)
 {
 	struct mpc512x_psc_spi *mps = spi_master_get_devdata(spi->master);
 
-	if (mps->cs_control && gpio_is_valid(spi->cs_gpio))
-		mps->cs_control(spi, (spi->mode & SPI_CS_HIGH) ? 0 : 1);
-
+	if (spi->cs_gpiod) {
+		if (mps->cs_control)
+			/* boardfile override */
+			mps->cs_control(spi, (spi->mode & SPI_CS_HIGH) ? 0 : 1);
+		else
+			/* gpiolib will deal with the inversion */
+			gpiod_set_value(spi->cs_gpiod, 0);
+	}
 }
 
 /* extract and scale size field in txsz or rxsz */
@@ -373,18 +383,6 @@ static int mpc512x_psc_spi_setup(struct spi_device *spi)
 		if (!cs)
 			return -ENOMEM;
 
-		if (gpio_is_valid(spi->cs_gpio)) {
-			ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev));
-			if (ret) {
-				dev_err(&spi->dev, "can't get CS gpio: %d\n",
-					ret);
-				kfree(cs);
-				return ret;
-			}
-			gpio_direction_output(spi->cs_gpio,
-					spi->mode & SPI_CS_HIGH ? 0 : 1);
-		}
-
 		spi->controller_state = cs;
 	}
 
@@ -396,8 +394,6 @@ static int mpc512x_psc_spi_setup(struct spi_device *spi)
 
 static void mpc512x_psc_spi_cleanup(struct spi_device *spi)
 {
-	if (gpio_is_valid(spi->cs_gpio))
-		gpio_free(spi->cs_gpio);
 	kfree(spi->controller_state);
 }
 
@@ -476,11 +472,6 @@ static irqreturn_t mpc512x_psc_spi_isr(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-static void mpc512x_spi_cs_control(struct spi_device *spi, bool onoff)
-{
-	gpio_set_value(spi->cs_gpio, onoff);
-}
-
 static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
 					      u32 size, unsigned int irq)
 {
@@ -500,9 +491,7 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
 	mps->type = (int)of_device_get_match_data(dev);
 	mps->irq = irq;
 
-	if (pdata == NULL) {
-		mps->cs_control = mpc512x_spi_cs_control;
-	} else {
+	if (pdata) {
 		mps->cs_control = pdata->cs_control;
 		master->bus_num = pdata->bus_num;
 		master->num_chipselect = pdata->max_chipselect;
@@ -513,6 +502,7 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
 	master->prepare_transfer_hardware = mpc512x_psc_spi_prep_xfer_hw;
 	master->transfer_one_message = mpc512x_psc_spi_msg_xfer;
 	master->unprepare_transfer_hardware = mpc512x_psc_spi_unprep_xfer_hw;
+	master->use_gpio_descriptors = true;
 	master->cleanup = mpc512x_psc_spi_cleanup;
 	master->dev.of_node = dev->of_node;
 
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v3 11/12] lkdtm: Fix execute_[user]_location()
From: Kees Cook @ 2022-01-19 22:00 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-arch, linux-ia64, Arnd Bergmann, linux-parisc, Helge Deller,
	linux-kernel, James E.J. Bottomley, linux-mm, Paul Mackerras,
	Greg Kroah-Hartman, Andrew Morton, linuxppc-dev
In-Reply-To: <c635dff6-2bca-3486-014f-12ae00bd1777@csgroup.eu>

On Wed, Jan 19, 2022 at 08:28:54PM +0100, Christophe Leroy wrote:
> Hi Kees,
> 
> 
> Le 17/12/2021 à 12:49, Christophe Leroy a écrit :
> > Hi Kees,
> > 
> > Le 17/10/2021 à 14:38, Christophe Leroy a écrit :
> > > execute_location() and execute_user_location() intent
> > > to copy do_nothing() text and execute it at a new location.
> > > However, at the time being it doesn't copy do_nothing() function
> > > but do_nothing() function descriptor which still points to the
> > > original text. So at the end it still executes do_nothing() at
> > > its original location allthough using a copied function descriptor.
> > > 
> > > So, fix that by really copying do_nothing() text and build a new
> > > function descriptor by copying do_nothing() function descriptor and
> > > updating the target address with the new location.
> > > 
> > > Also fix the displayed addresses by dereferencing do_nothing()
> > > function descriptor.
> > > 
> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > 
> > Do you have any comment to this patch and to patch 12 ?
> > 
> > If not, is it ok to get your acked-by ?
> 
> Any feedback please, even if it's to say no feedback ?

Hi! Thanks for the ping; I haven't had time yet to look at this, but
with -rc1 coming, I should be able to task-switch back to LKDTM for the
dev cycle and I can give some feedback.

-Kees

> 
> Many thanks,
> Christophe
> 
> > 
> > Thanks
> > Christophe
> > 
> > > ---
> > >   drivers/misc/lkdtm/perms.c | 37 ++++++++++++++++++++++++++++---------
> > >   1 file changed, 28 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
> > > index 035fcca441f0..1cf24c4a79e9 100644
> > > --- a/drivers/misc/lkdtm/perms.c
> > > +++ b/drivers/misc/lkdtm/perms.c
> > > @@ -44,19 +44,34 @@ static noinline void do_overwritten(void)
> > >       return;
> > >   }
> > > +static void *setup_function_descriptor(func_desc_t *fdesc, void *dst)
> > > +{
> > > +    if (!have_function_descriptors())
> > > +        return dst;
> > > +
> > > +    memcpy(fdesc, do_nothing, sizeof(*fdesc));
> > > +    fdesc->addr = (unsigned long)dst;
> > > +    barrier();
> > > +
> > > +    return fdesc;
> > > +}
> > > +
> > >   static noinline void execute_location(void *dst, bool write)
> > >   {
> > > -    void (*func)(void) = dst;
> > > +    void (*func)(void);
> > > +    func_desc_t fdesc;
> > > +    void *do_nothing_text = dereference_function_descriptor(do_nothing);
> > > -    pr_info("attempting ok execution at %px\n", do_nothing);
> > > +    pr_info("attempting ok execution at %px\n", do_nothing_text);
> > >       do_nothing();
> > >       if (write == CODE_WRITE) {
> > > -        memcpy(dst, do_nothing, EXEC_SIZE);
> > > +        memcpy(dst, do_nothing_text, EXEC_SIZE);
> > >           flush_icache_range((unsigned long)dst,
> > >                      (unsigned long)dst + EXEC_SIZE);
> > >       }
> > > -    pr_info("attempting bad execution at %px\n", func);
> > > +    pr_info("attempting bad execution at %px\n", dst);
> > > +    func = setup_function_descriptor(&fdesc, dst);
> > >       func();
> > >       pr_err("FAIL: func returned\n");
> > >   }
> > > @@ -66,16 +81,19 @@ static void execute_user_location(void *dst)
> > >       int copied;
> > >       /* Intentionally crossing kernel/user memory boundary. */
> > > -    void (*func)(void) = dst;
> > > +    void (*func)(void);
> > > +    func_desc_t fdesc;
> > > +    void *do_nothing_text = dereference_function_descriptor(do_nothing);
> > > -    pr_info("attempting ok execution at %px\n", do_nothing);
> > > +    pr_info("attempting ok execution at %px\n", do_nothing_text);
> > >       do_nothing();
> > > -    copied = access_process_vm(current, (unsigned long)dst, do_nothing,
> > > +    copied = access_process_vm(current, (unsigned long)dst,
> > > do_nothing_text,
> > >                      EXEC_SIZE, FOLL_WRITE);
> > >       if (copied < EXEC_SIZE)
> > >           return;
> > > -    pr_info("attempting bad execution at %px\n", func);
> > > +    pr_info("attempting bad execution at %px\n", dst);
> > > +    func = setup_function_descriptor(&fdesc, dst);
> > >       func();
> > >       pr_err("FAIL: func returned\n");
> > >   }
> > > @@ -153,7 +171,8 @@ void lkdtm_EXEC_VMALLOC(void)
> > >   void lkdtm_EXEC_RODATA(void)
> > >   {
> > > -    execute_location(lkdtm_rodata_do_nothing, CODE_AS_IS);
> > > +    execute_location(dereference_function_descriptor(lkdtm_rodata_do_nothing),
> > > 
> > > +             CODE_AS_IS);
> > >   }
> > >   void lkdtm_EXEC_USERSPACE(void)
> > > 

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH v3 11/12] lkdtm: Fix execute_[user]_location()
From: Christophe Leroy @ 2022-01-19 19:28 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-arch, linux-ia64, Arnd Bergmann, linux-parisc, Helge Deller,
	linux-kernel, James E.J. Bottomley, linux-mm, Paul Mackerras,
	Greg Kroah-Hartman, Andrew Morton, linuxppc-dev
In-Reply-To: <e7793192-6879-490d-1f37-3d6d6908a121@csgroup.eu>

Hi Kees,


Le 17/12/2021 à 12:49, Christophe Leroy a écrit :
> Hi Kees,
> 
> Le 17/10/2021 à 14:38, Christophe Leroy a écrit :
>> execute_location() and execute_user_location() intent
>> to copy do_nothing() text and execute it at a new location.
>> However, at the time being it doesn't copy do_nothing() function
>> but do_nothing() function descriptor which still points to the
>> original text. So at the end it still executes do_nothing() at
>> its original location allthough using a copied function descriptor.
>>
>> So, fix that by really copying do_nothing() text and build a new
>> function descriptor by copying do_nothing() function descriptor and
>> updating the target address with the new location.
>>
>> Also fix the displayed addresses by dereferencing do_nothing()
>> function descriptor.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> 
> Do you have any comment to this patch and to patch 12 ?
> 
> If not, is it ok to get your acked-by ?

Any feedback please, even if it's to say no feedback ?

Many thanks,
Christophe

> 
> Thanks
> Christophe
> 
>> ---
>>   drivers/misc/lkdtm/perms.c | 37 ++++++++++++++++++++++++++++---------
>>   1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
>> index 035fcca441f0..1cf24c4a79e9 100644
>> --- a/drivers/misc/lkdtm/perms.c
>> +++ b/drivers/misc/lkdtm/perms.c
>> @@ -44,19 +44,34 @@ static noinline void do_overwritten(void)
>>       return;
>>   }
>> +static void *setup_function_descriptor(func_desc_t *fdesc, void *dst)
>> +{
>> +    if (!have_function_descriptors())
>> +        return dst;
>> +
>> +    memcpy(fdesc, do_nothing, sizeof(*fdesc));
>> +    fdesc->addr = (unsigned long)dst;
>> +    barrier();
>> +
>> +    return fdesc;
>> +}
>> +
>>   static noinline void execute_location(void *dst, bool write)
>>   {
>> -    void (*func)(void) = dst;
>> +    void (*func)(void);
>> +    func_desc_t fdesc;
>> +    void *do_nothing_text = dereference_function_descriptor(do_nothing);
>> -    pr_info("attempting ok execution at %px\n", do_nothing);
>> +    pr_info("attempting ok execution at %px\n", do_nothing_text);
>>       do_nothing();
>>       if (write == CODE_WRITE) {
>> -        memcpy(dst, do_nothing, EXEC_SIZE);
>> +        memcpy(dst, do_nothing_text, EXEC_SIZE);
>>           flush_icache_range((unsigned long)dst,
>>                      (unsigned long)dst + EXEC_SIZE);
>>       }
>> -    pr_info("attempting bad execution at %px\n", func);
>> +    pr_info("attempting bad execution at %px\n", dst);
>> +    func = setup_function_descriptor(&fdesc, dst);
>>       func();
>>       pr_err("FAIL: func returned\n");
>>   }
>> @@ -66,16 +81,19 @@ static void execute_user_location(void *dst)
>>       int copied;
>>       /* Intentionally crossing kernel/user memory boundary. */
>> -    void (*func)(void) = dst;
>> +    void (*func)(void);
>> +    func_desc_t fdesc;
>> +    void *do_nothing_text = dereference_function_descriptor(do_nothing);
>> -    pr_info("attempting ok execution at %px\n", do_nothing);
>> +    pr_info("attempting ok execution at %px\n", do_nothing_text);
>>       do_nothing();
>> -    copied = access_process_vm(current, (unsigned long)dst, do_nothing,
>> +    copied = access_process_vm(current, (unsigned long)dst, 
>> do_nothing_text,
>>                      EXEC_SIZE, FOLL_WRITE);
>>       if (copied < EXEC_SIZE)
>>           return;
>> -    pr_info("attempting bad execution at %px\n", func);
>> +    pr_info("attempting bad execution at %px\n", dst);
>> +    func = setup_function_descriptor(&fdesc, dst);
>>       func();
>>       pr_err("FAIL: func returned\n");
>>   }
>> @@ -153,7 +171,8 @@ void lkdtm_EXEC_VMALLOC(void)
>>   void lkdtm_EXEC_RODATA(void)
>>   {
>> -    execute_location(lkdtm_rodata_do_nothing, CODE_AS_IS);
>> +    
>> execute_location(dereference_function_descriptor(lkdtm_rodata_do_nothing), 
>>
>> +             CODE_AS_IS);
>>   }
>>   void lkdtm_EXEC_USERSPACE(void)
>>

^ permalink raw reply

* [PATCH v4 6/7] drivers: virtio_mem: use pageblock size as the minimum virtio_mem size.
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski
In-Reply-To: <20220119190623.1029355-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

alloc_contig_range() now only needs to be aligned to pageblock_order,
drop virtio_mem size requirement that it needs to be the max of
pageblock_order and MAX_ORDER.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 drivers/virtio/virtio_mem.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index a6a78685cfbe..eafba2119ae3 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2476,13 +2476,12 @@ static int virtio_mem_init_hotplug(struct virtio_mem *vm)
 				      VIRTIO_MEM_DEFAULT_OFFLINE_THRESHOLD);
 
 	/*
-	 * We want subblocks to span at least MAX_ORDER_NR_PAGES and
-	 * pageblock_nr_pages pages. This:
+	 * We want subblocks to span at least pageblock_nr_pages pages.
+	 * This:
 	 * - Is required for now for alloc_contig_range() to work reliably -
 	 *   it doesn't properly handle smaller granularity on ZONE_NORMAL.
 	 */
-	sb_size = max_t(uint64_t, MAX_ORDER_NR_PAGES,
-			pageblock_nr_pages) * PAGE_SIZE;
+	sb_size = pageblock_nr_pages * PAGE_SIZE;
 	sb_size = max_t(uint64_t, vm->device_block_size, sb_size);
 
 	if (sb_size < memory_block_size_bytes() && !force_bbm) {
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 5/7] mm: cma: use pageblock_order as the single alignment
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski
In-Reply-To: <20220119190623.1029355-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

Now alloc_contig_range() works at pageblock granularity. Change CMA
allocation, which uses alloc_contig_range(), to use pageblock_order
alignment.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/mmzone.h  | 5 +----
 kernel/dma/contiguous.c | 2 +-
 mm/cma.c                | 6 ++----
 mm/page_alloc.c         | 6 +++---
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 71b77aab748d..7bd3694b24b4 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -54,10 +54,7 @@ enum migratetype {
 	 *
 	 * The way to use it is to change migratetype of a range of
 	 * pageblocks to MIGRATE_CMA which can be done by
-	 * __free_pageblock_cma() function.  What is important though
-	 * is that a range of pageblocks must be aligned to
-	 * MAX_ORDER_NR_PAGES should biggest page be bigger than
-	 * a single pageblock.
+	 * __free_pageblock_cma() function.
 	 */
 	MIGRATE_CMA,
 #endif
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 3d63d91cba5c..ac35b14b0786 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -399,7 +399,7 @@ static const struct reserved_mem_ops rmem_cma_ops = {
 
 static int __init rmem_cma_setup(struct reserved_mem *rmem)
 {
-	phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+	phys_addr_t align = PAGE_SIZE << pageblock_order;
 	phys_addr_t mask = align - 1;
 	unsigned long node = rmem->fdt_node;
 	bool default_cma = of_get_flat_dt_prop(node, "linux,cma-default", NULL);
diff --git a/mm/cma.c b/mm/cma.c
index bc9ca8f3c487..d171158bd418 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -180,8 +180,7 @@ int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
 		return -EINVAL;
 
 	/* ensure minimal alignment required by mm core */
-	alignment = PAGE_SIZE <<
-			max_t(unsigned long, MAX_ORDER - 1, pageblock_order);
+	alignment = PAGE_SIZE << pageblock_order;
 
 	/* alignment should be aligned with order_per_bit */
 	if (!IS_ALIGNED(alignment >> PAGE_SHIFT, 1 << order_per_bit))
@@ -268,8 +267,7 @@ int __init cma_declare_contiguous_nid(phys_addr_t base,
 	 * migratetype page by page allocator's buddy algorithm. In the case,
 	 * you couldn't get a contiguous memory, which is not what we want.
 	 */
-	alignment = max(alignment,  (phys_addr_t)PAGE_SIZE <<
-			  max_t(unsigned long, MAX_ORDER - 1, pageblock_order));
+	alignment = max(alignment,  (phys_addr_t)PAGE_SIZE << pageblock_order);
 	if (fixed && base & (alignment - 1)) {
 		ret = -EINVAL;
 		pr_err("Region at %pa must be aligned to %pa bytes\n",
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6ed506234efa..a8ced1a00ce8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -9008,8 +9008,8 @@ static inline void split_free_page_into_pageblocks(struct page *free_page,
  *			be either of the two.
  * @gfp_mask:	GFP mask to use during compaction
  *
- * The PFN range does not have to be pageblock or MAX_ORDER_NR_PAGES
- * aligned.  The PFN range must belong to a single zone.
+ * The PFN range does not have to be pageblock aligned. The PFN range must
+ * belong to a single zone.
  *
  * The first thing this routine does is attempt to MIGRATE_ISOLATE all
  * pageblocks in the range.  Once isolated, the pageblocks should not
@@ -9125,7 +9125,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	ret = 0;
 
 	/*
-	 * Pages from [start, end) are within a MAX_ORDER_NR_PAGES
+	 * Pages from [start, end) are within a pageblock_nr_pages
 	 * aligned blocks that are marked as MIGRATE_ISOLATE.  What's
 	 * more, all pages in [start, end) are free in page allocator.
 	 * What we are going to do is to allocate all pages from
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 7/7] arch: powerpc: adjust fadump alignment to be pageblock aligned.
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski
In-Reply-To: <20220119190623.1029355-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

CMA only requires pageblock alignment now. Change CMA alignment in
fadump too.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 arch/powerpc/include/asm/fadump-internal.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/fadump-internal.h b/arch/powerpc/include/asm/fadump-internal.h
index 52189928ec08..fbfca85b4200 100644
--- a/arch/powerpc/include/asm/fadump-internal.h
+++ b/arch/powerpc/include/asm/fadump-internal.h
@@ -20,9 +20,7 @@
 #define memblock_num_regions(memblock_type)	(memblock.memblock_type.cnt)
 
 /* Alignment per CMA requirement. */
-#define FADUMP_CMA_ALIGNMENT	(PAGE_SIZE <<				\
-				 max_t(unsigned long, MAX_ORDER - 1,	\
-				 pageblock_order))
+#define FADUMP_CMA_ALIGNMENT	(PAGE_SIZE << pageblock_order)
 
 /* FAD commands */
 #define FADUMP_REGISTER			1
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 1/7] mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski
In-Reply-To: <20220119190623.1029355-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

This is done in addition to MIGRATE_ISOLATE pageblock merge avoidance.
It prepares for the upcoming removal of the MAX_ORDER-1 alignment
requirement for CMA and alloc_contig_range().

MIGRARTE_HIGHATOMIC should not merge with other migratetypes like
MIGRATE_ISOLATE and MIGRARTE_CMA[1], so this commit prevents that too.
Also add MIGRARTE_HIGHATOMIC to fallbacks array for completeness.

[1] https://lore.kernel.org/linux-mm/20211130100853.GP3366@techsingularity.net/

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/mmzone.h | 11 +++++++++++
 mm/page_alloc.c        | 37 ++++++++++++++++++++-----------------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index aed44e9b5d89..71b77aab748d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -83,6 +83,17 @@ static inline bool is_migrate_movable(int mt)
 	return is_migrate_cma(mt) || mt == MIGRATE_MOVABLE;
 }
 
+/*
+ * Check whether a migratetype can be merged with another migratetype.
+ *
+ * It is only mergeable when it can fall back to other migratetypes for
+ * allocation. See fallbacks[MIGRATE_TYPES][3] in page_alloc.c.
+ */
+static inline bool migratetype_is_mergeable(int mt)
+{
+	return mt < MIGRATE_PCPTYPES;
+}
+
 #define for_each_migratetype_order(order, type) \
 	for (order = 0; order < MAX_ORDER; order++) \
 		for (type = 0; type < MIGRATE_TYPES; type++)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8dd6399bafb5..15de65215c02 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1117,25 +1117,24 @@ static inline void __free_one_page(struct page *page,
 	}
 	if (order < MAX_ORDER - 1) {
 		/* If we are here, it means order is >= pageblock_order.
-		 * We want to prevent merge between freepages on isolate
-		 * pageblock and normal pageblock. Without this, pageblock
-		 * isolation could cause incorrect freepage or CMA accounting.
+		 * We want to prevent merge between freepages on pageblock
+		 * without fallbacks and normal pageblock. Without this,
+		 * pageblock isolation could cause incorrect freepage or CMA
+		 * accounting or HIGHATOMIC accounting.
 		 *
 		 * We don't want to hit this code for the more frequent
 		 * low-order merging.
 		 */
-		if (unlikely(has_isolate_pageblock(zone))) {
-			int buddy_mt;
+		int buddy_mt;
 
-			buddy_pfn = __find_buddy_pfn(pfn, order);
-			buddy = page + (buddy_pfn - pfn);
-			buddy_mt = get_pageblock_migratetype(buddy);
+		buddy_pfn = __find_buddy_pfn(pfn, order);
+		buddy = page + (buddy_pfn - pfn);
+		buddy_mt = get_pageblock_migratetype(buddy);
 
-			if (migratetype != buddy_mt
-					&& (is_migrate_isolate(migratetype) ||
-						is_migrate_isolate(buddy_mt)))
-				goto done_merging;
-		}
+		if (migratetype != buddy_mt
+				&& (!migratetype_is_mergeable(migratetype) ||
+					!migratetype_is_mergeable(buddy_mt)))
+			goto done_merging;
 		max_order = order + 1;
 		goto continue_merging;
 	}
@@ -2484,6 +2483,7 @@ static int fallbacks[MIGRATE_TYPES][3] = {
 	[MIGRATE_UNMOVABLE]   = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE,   MIGRATE_TYPES },
 	[MIGRATE_MOVABLE]     = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_TYPES },
 	[MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE,   MIGRATE_MOVABLE,   MIGRATE_TYPES },
+	[MIGRATE_HIGHATOMIC] = { MIGRATE_TYPES }, /* Never used */
 #ifdef CONFIG_CMA
 	[MIGRATE_CMA]         = { MIGRATE_TYPES }, /* Never used */
 #endif
@@ -2795,8 +2795,8 @@ static void reserve_highatomic_pageblock(struct page *page, struct zone *zone,
 
 	/* Yoink! */
 	mt = get_pageblock_migratetype(page);
-	if (!is_migrate_highatomic(mt) && !is_migrate_isolate(mt)
-	    && !is_migrate_cma(mt)) {
+	/* Only reserve normal pageblocks (i.e., they can merge with others) */
+	if (migratetype_is_mergeable(mt)) {
 		zone->nr_reserved_highatomic += pageblock_nr_pages;
 		set_pageblock_migratetype(page, MIGRATE_HIGHATOMIC);
 		move_freepages_block(zone, page, MIGRATE_HIGHATOMIC, NULL);
@@ -3545,8 +3545,11 @@ int __isolate_free_page(struct page *page, unsigned int order)
 		struct page *endpage = page + (1 << order) - 1;
 		for (; page < endpage; page += pageblock_nr_pages) {
 			int mt = get_pageblock_migratetype(page);
-			if (!is_migrate_isolate(mt) && !is_migrate_cma(mt)
-			    && !is_migrate_highatomic(mt))
+			/*
+			 * Only change normal pageblocks (i.e., they can merge
+			 * with others)
+			 */
+			if (migratetype_is_mergeable(mt))
 				set_pageblock_migratetype(page,
 							  MIGRATE_MOVABLE);
 		}
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 4/7] mm: make alloc_contig_range work at pageblock granularity
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski
In-Reply-To: <20220119190623.1029355-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

alloc_contig_range() worked at MAX_ORDER-1 granularity to avoid merging
pageblocks with different migratetypes. It might unnecessarily convert
extra pageblocks at the beginning and at the end of the range. Change
alloc_contig_range() to work at pageblock granularity.

It is done by restoring pageblock types and split >pageblock_order free
pages after isolating at MAX_ORDER-1 granularity and migrating pages
away at pageblock granularity. The reason for this process is that
during isolation, some pages, either free or in-use, might have >pageblock
sizes and isolating part of them can cause free accounting issues.
Restoring the migratetypes of the pageblocks not in the interesting
range later is much easier.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 mm/page_alloc.c | 175 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 155 insertions(+), 20 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 812cf557b20f..6ed506234efa 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8862,8 +8862,8 @@ void *__init alloc_large_system_hash(const char *tablename,
 #ifdef CONFIG_CONTIG_ALLOC
 static unsigned long pfn_max_align_down(unsigned long pfn)
 {
-	return pfn & ~(max_t(unsigned long, MAX_ORDER_NR_PAGES,
-			     pageblock_nr_pages) - 1);
+	return ALIGN_DOWN(pfn, max_t(unsigned long, MAX_ORDER_NR_PAGES,
+				     pageblock_nr_pages));
 }
 
 static unsigned long pfn_max_align_up(unsigned long pfn)
@@ -8952,6 +8952,52 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 	return 0;
 }
 
+static inline int save_migratetypes(unsigned char *migratetypes,
+				unsigned long start_pfn, unsigned long end_pfn)
+{
+	unsigned long pfn = start_pfn;
+	int num = 0;
+
+	while (pfn < end_pfn) {
+		migratetypes[num] = get_pageblock_migratetype(pfn_to_page(pfn));
+		num++;
+		pfn += pageblock_nr_pages;
+	}
+	return num;
+}
+
+static inline int restore_migratetypes(unsigned char *migratetypes,
+				unsigned long start_pfn, unsigned long end_pfn)
+{
+	unsigned long pfn = start_pfn;
+	int num = 0;
+
+	while (pfn < end_pfn) {
+		set_pageblock_migratetype(pfn_to_page(pfn), migratetypes[num]);
+		num++;
+		pfn += pageblock_nr_pages;
+	}
+	return num;
+}
+
+static inline void split_free_page_into_pageblocks(struct page *free_page,
+				int order, struct zone *zone)
+{
+	unsigned long pfn;
+
+	spin_lock(&zone->lock);
+	del_page_from_free_list(free_page, zone, order);
+	for (pfn = page_to_pfn(free_page);
+	     pfn < page_to_pfn(free_page) + (1UL << order);
+	     pfn += pageblock_nr_pages) {
+		int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
+
+		__free_one_page(pfn_to_page(pfn), pfn, zone, pageblock_order,
+				mt, FPI_NONE);
+	}
+	spin_unlock(&zone->lock);
+}
+
 /**
  * alloc_contig_range() -- tries to allocate given range of pages
  * @start:	start PFN to allocate
@@ -8977,8 +9023,15 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 		       unsigned migratetype, gfp_t gfp_mask)
 {
 	unsigned long outer_start, outer_end;
+	unsigned long isolate_start = pfn_max_align_down(start);
+	unsigned long isolate_end = pfn_max_align_up(end);
+	unsigned long alloc_start = ALIGN_DOWN(start, pageblock_nr_pages);
+	unsigned long alloc_end = ALIGN(end, pageblock_nr_pages);
+	unsigned long num_pageblock_to_save;
 	unsigned int order;
 	int ret = 0;
+	unsigned char *saved_mt;
+	int num;
 
 	struct compact_control cc = {
 		.nr_migratepages = 0,
@@ -8992,11 +9045,30 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	};
 	INIT_LIST_HEAD(&cc.migratepages);
 
+	/*
+	 * TODO: make MIGRATE_ISOLATE a standalone bit to avoid overwriting
+	 * the exiting migratetype. Then, we will not need the save and restore
+	 * process here.
+	 */
+
+	/* Save the migratepages of the pageblocks before start and after end */
+	num_pageblock_to_save = (alloc_start - isolate_start) / pageblock_nr_pages
+				+ (isolate_end - alloc_end) / pageblock_nr_pages;
+	saved_mt =
+		kmalloc_array(num_pageblock_to_save,
+			      sizeof(unsigned char), GFP_KERNEL);
+	if (!saved_mt)
+		return -ENOMEM;
+
+	num = save_migratetypes(saved_mt, isolate_start, alloc_start);
+
+	num = save_migratetypes(&saved_mt[num], alloc_end, isolate_end);
+
 	/*
 	 * What we do here is we mark all pageblocks in range as
 	 * MIGRATE_ISOLATE.  Because pageblock and max order pages may
 	 * have different sizes, and due to the way page allocator
-	 * work, we align the range to biggest of the two pages so
+	 * work, we align the isolation range to biggest of the two so
 	 * that page allocator won't try to merge buddies from
 	 * different pageblocks and change MIGRATE_ISOLATE to some
 	 * other migration type.
@@ -9006,6 +9078,20 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * we are interested in).  This will put all the pages in
 	 * range back to page allocator as MIGRATE_ISOLATE.
 	 *
+	 * Afterwards, we restore the migratetypes of the pageblocks not
+	 * in range, split free pages spanning outside the range,
+	 * and put split free pages (at pageblock_order) to the right
+	 * migratetype list.
+	 *
+	 * NOTE: the above approach is used because it can cause free
+	 * page accounting issues during isolation, if a page, either
+	 * free or in-use, contains multiple pageblocks and we only
+	 * isolate a subset of them. For example, if only the second
+	 * pageblock is isolated from a page with 2 pageblocks, after
+	 * the page is free, it will be put in the first pageblock
+	 * migratetype list instead of having 2 pageblocks in two
+	 * separate migratetype lists.
+	 *
 	 * When this is done, we take the pages in range from page
 	 * allocator removing them from the buddy system.  This way
 	 * page allocator will never consider using them.
@@ -9016,10 +9102,10 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * put back to page allocator so that buddy can use them.
 	 */
 
-	ret = start_isolate_page_range(start, end, pfn_max_align_down(start),
-				       pfn_max_align_up(end), migratetype, 0);
+	ret = start_isolate_page_range(start, end, isolate_start, isolate_end,
+				migratetype, 0);
 	if (ret)
-		return ret;
+		goto done;
 
 	drain_all_pages(cc.zone);
 
@@ -9055,6 +9141,19 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * isolated thus they won't get removed from buddy.
 	 */
 
+	/*
+	 * Restore migratetypes of pageblocks outside [start, end)
+	 * TODO: remove it when MIGRATE_ISOLATE becomes a standalone bit
+	 */
+
+	num = restore_migratetypes(saved_mt, isolate_start, alloc_start);
+
+	num = restore_migratetypes(&saved_mt[num], alloc_end, isolate_end);
+
+	/*
+	 * Split free page spanning [isolate_start, alloc_start) and put the
+	 * pageblocks in the right migratetype lists.
+	 */
 	order = 0;
 	outer_start = start;
 	while (!PageBuddy(pfn_to_page(outer_start))) {
@@ -9069,37 +9168,73 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 		order = buddy_order(pfn_to_page(outer_start));
 
 		/*
-		 * outer_start page could be small order buddy page and
-		 * it doesn't include start page. Adjust outer_start
-		 * in this case to report failed page properly
-		 * on tracepoint in test_pages_isolated()
+		 * split the free page has start page and put the pageblocks
+		 * in the right migratetype list
 		 */
-		if (outer_start + (1UL << order) <= start)
-			outer_start = start;
+		if (outer_start + (1UL << order) > start) {
+			struct page *free_page = pfn_to_page(outer_start);
+
+			split_free_page_into_pageblocks(free_page, order, cc.zone);
+		}
+	}
+
+	/*
+	 * Split free page spanning [alloc_end, isolate_end) and put the
+	 * pageblocks in the right migratetype list
+	 */
+	for (outer_end = alloc_end; outer_end < isolate_end;) {
+		unsigned long begin_pfn = outer_end;
+
+		order = 0;
+		while (!PageBuddy(pfn_to_page(outer_end))) {
+			if (++order >= MAX_ORDER) {
+				outer_end = begin_pfn;
+				break;
+			}
+			outer_end &= ~0UL << order;
+		}
+
+		if (outer_end != begin_pfn) {
+			order = buddy_order(pfn_to_page(outer_end));
+
+			/*
+			 * split the free page has start page and put the pageblocks
+			 * in the right migratetype list
+			 */
+			VM_BUG_ON(outer_end + (1UL << order) <= begin_pfn);
+			{
+				struct page *free_page = pfn_to_page(outer_end);
+
+				split_free_page_into_pageblocks(free_page, order, cc.zone);
+			}
+			outer_end += 1UL << order;
+		} else
+			outer_end = begin_pfn + 1;
 	}
 
 	/* Make sure the range is really isolated. */
-	if (test_pages_isolated(outer_start, end, 0)) {
+	if (test_pages_isolated(alloc_start, alloc_end, 0)) {
 		ret = -EBUSY;
 		goto done;
 	}
 
 	/* Grab isolated pages from freelists. */
-	outer_end = isolate_freepages_range(&cc, outer_start, end);
+	outer_end = isolate_freepages_range(&cc, alloc_start, alloc_end);
 	if (!outer_end) {
 		ret = -EBUSY;
 		goto done;
 	}
 
 	/* Free head and tail (if any) */
-	if (start != outer_start)
-		free_contig_range(outer_start, start - outer_start);
-	if (end != outer_end)
-		free_contig_range(end, outer_end - end);
+	if (start != alloc_start)
+		free_contig_range(alloc_start, start - alloc_start);
+	if (end != alloc_end)
+		free_contig_range(end, alloc_end - end);
 
 done:
-	undo_isolate_page_range(pfn_max_align_down(start),
-				pfn_max_align_up(end), migratetype);
+	kfree(saved_mt);
+	undo_isolate_page_range(alloc_start,
+				alloc_end, migratetype);
 	return ret;
 }
 EXPORT_SYMBOL(alloc_contig_range);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski
In-Reply-To: <20220119190623.1029355-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

Enable set_migratetype_isolate() to check specified sub-range for
unmovable pages during isolation. Page isolation is done
at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
pages within that granularity are intended to be isolated. For example,
alloc_contig_range(), which uses page isolation, allows ranges without
alignment. This commit makes unmovable page check only look for
interesting pages, so that page isolation can succeed for any
non-overlapping ranges.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/page-isolation.h |  1 +
 mm/memory_hotplug.c            | 12 +++++++-
 mm/page_alloc.c                |  2 +-
 mm/page_isolation.c            | 53 +++++++++++++++++++++-------------
 4 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index e14eddf6741a..a4d2687ed4e6 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -42,6 +42,7 @@ int move_freepages_block(struct zone *zone, struct page *page,
  */
 int
 start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
+			 unsigned long isolate_start, unsigned long isolate_end,
 			 unsigned migratetype, int flags);
 
 /*
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0139b77c51d5..5db84c3fa882 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1901,8 +1901,18 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages,
 	zone_pcp_disable(zone);
 	lru_cache_disable();
 
-	/* set above range as isolated */
+	/*
+	 * set above range as isolated
+	 *
+	 * start_pfn and end_pfn are the same as isolate_start and isolate_end,
+	 * because start_pfn and end_pfn are already PAGES_PER_SECTION
+	 * (>= MAX_ORDER_NR_PAGES) aligned; if start_pfn is
+	 * pageblock_nr_pages aligned in memmap_on_memory case, there is no
+	 * need to isolate pages before start_pfn, since they are used by
+	 * memmap thus not user visible.
+	 */
 	ret = start_isolate_page_range(start_pfn, end_pfn,
+				       start_pfn, end_pfn,
 				       MIGRATE_MOVABLE,
 				       MEMORY_OFFLINE | REPORT_FAILURE);
 	if (ret) {
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1d812268c2a9..812cf557b20f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -9016,7 +9016,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 * put back to page allocator so that buddy can use them.
 	 */
 
-	ret = start_isolate_page_range(pfn_max_align_down(start),
+	ret = start_isolate_page_range(start, end, pfn_max_align_down(start),
 				       pfn_max_align_up(end), migratetype, 0);
 	if (ret)
 		return ret;
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 6c841274bf46..d17ad9a7d4bf 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -16,7 +16,8 @@
 #include <trace/events/page_isolation.h>
 
 /*
- * This function checks whether pageblock includes unmovable pages or not.
+ * This function checks whether pageblock within [start_pfn, end_pfn) includes
+ * unmovable pages or not.
  *
  * PageLRU check without isolation or lru_lock could race so that
  * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
@@ -29,11 +30,14 @@
  *
  */
 static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
-				 int migratetype, int flags)
+				 int migratetype, int flags,
+				 unsigned long start_pfn, unsigned long end_pfn)
 {
-	unsigned long iter = 0;
-	unsigned long pfn = page_to_pfn(page);
-	unsigned long offset = pfn % pageblock_nr_pages;
+	unsigned long first_pfn = max(page_to_pfn(page), start_pfn);
+	unsigned long pfn = first_pfn;
+	unsigned long last_pfn = min(ALIGN(pfn + 1, pageblock_nr_pages), end_pfn);
+
+	page = pfn_to_page(pfn);
 
 	if (is_migrate_cma_page(page)) {
 		/*
@@ -47,8 +51,8 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 		return page;
 	}
 
-	for (; iter < pageblock_nr_pages - offset; iter++) {
-		page = pfn_to_page(pfn + iter);
+	for (pfn = first_pfn; pfn < last_pfn; pfn++) {
+		page = pfn_to_page(pfn);
 
 		/*
 		 * Both, bootmem allocations and memory holes are marked
@@ -85,7 +89,7 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 			}
 
 			skip_pages = compound_nr(head) - (page - head);
-			iter += skip_pages - 1;
+			pfn += skip_pages - 1;
 			continue;
 		}
 
@@ -97,7 +101,7 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 		 */
 		if (!page_ref_count(page)) {
 			if (PageBuddy(page))
-				iter += (1 << buddy_order(page)) - 1;
+				pfn += (1 << buddy_order(page)) - 1;
 			continue;
 		}
 
@@ -134,7 +138,13 @@ static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
 	return NULL;
 }
 
-static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags)
+/*
+ * This function set pageblock migratetype to isolate if no unmovable page is
+ * present in [start_pfn, end_pfn). The pageblock must be within
+ * [start_pfn, end_pfn).
+ */
+static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags,
+			unsigned long start_pfn, unsigned long end_pfn)
 {
 	struct zone *zone = page_zone(page);
 	struct page *unmovable;
@@ -156,7 +166,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
 	 * We just check MOVABLE pages.
 	 */
-	unmovable = has_unmovable_pages(zone, page, migratetype, isol_flags);
+	unmovable = has_unmovable_pages(zone, page, migratetype, isol_flags, start_pfn, end_pfn);
 	if (!unmovable) {
 		unsigned long nr_pages;
 		int mt = get_pageblock_migratetype(page);
@@ -265,8 +275,12 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
 /**
  * start_isolate_page_range() - make page-allocation-type of range of pages to
  * be MIGRATE_ISOLATE.
- * @start_pfn:		The lower PFN of the range to be isolated.
- * @end_pfn:		The upper PFN of the range to be isolated.
+ * @start_pfn:		The lower PFN of the range to be checked for
+ *			possibility of isolation.
+ * @end_pfn:		The upper PFN of the range to be checked for
+ *			possibility of isolation.
+ * @isolate_start:		The lower PFN of the range to be isolated.
+ * @isolate_end:		The upper PFN of the range to be isolated.
  *			start_pfn/end_pfn must be aligned to pageblock_order.
  * @migratetype:	Migrate type to set in error recovery.
  * @flags:		The following flags are allowed (they can be combined in
@@ -304,20 +318,19 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * Return: 0 on success and -EBUSY if any part of range cannot be isolated.
  */
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
+			     unsigned long isolate_start, unsigned long isolate_end,
 			     unsigned migratetype, int flags)
 {
 	unsigned long pfn;
 	struct page *page;
 
-	BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
-	BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
-
-	for (pfn = start_pfn;
-	     pfn < end_pfn;
+	for (pfn = isolate_start;
+	     pfn < isolate_end;
 	     pfn += pageblock_nr_pages) {
 		page = __first_valid_page(pfn, pageblock_nr_pages);
-		if (page && set_migratetype_isolate(page, migratetype, flags)) {
-			undo_isolate_page_range(start_pfn, pfn, migratetype);
+		if (page && set_migratetype_isolate(page, migratetype, flags,
+					start_pfn, end_pfn)) {
+			undo_isolate_page_range(isolate_start, pfn, migratetype);
 			return -EBUSY;
 		}
 	}
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 0/7] Use pageblock_order for cma and alloc_contig_range alignment.
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski

From: Zi Yan <ziy@nvidia.com>

Hi all,

This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA
and alloc_contig_range(). It prepares for my upcoming changes to make
MAX_ORDER adjustable at boot time[1]. It is on top of mmotm-2021-12-29-20-07.

Changelog from RFC
===
1. Dropped two irrelevant patches on non-lru compound page handling, as
   it is not supported upstream.
2. Renamed migratetype_has_fallback() to migratetype_is_mergeable().
3. Always check whether two pageblocks can be merged in
   __free_one_page() when order is >= pageblock_order, as the case (not
   mergeable pageblocks are isolated, CMA, and HIGHATOMIC) becomes more common.
3. Moving has_unmovable_pages() is now a separate patch.
4. Removed MAX_ORDER-1 alignment requirement in the comment in virtio_mem code.

Description
===

The MAX_ORDER - 1 alignment requirement comes from that alloc_contig_range()
isolates pageblocks to remove free memory from buddy allocator but isolating
only a subset of pageblocks within a page spanning across multiple pageblocks
causes free page accounting issues. Isolated page might not be put into the
right free list, since the code assumes the migratetype of the first pageblock
as the whole free page migratetype. This is based on the discussion at [2].

To remove the requirement, this patchset:
1. still isolates pageblocks at MAX_ORDER - 1 granularity;
2. but saves the pageblock migratetypes outside the specified range of
   alloc_contig_range() and restores them after all pages within the range
   become free after __alloc_contig_migrate_range();
3. only checks unmovable pages within the range instead of MAX_ORDER - 1 aligned
   range during isolation to avoid alloc_contig_range() failure when pageblocks
   within a MAX_ORDER - 1 aligned range are allocated separately.
3. splits free pages spanning multiple pageblocks at the beginning and the end
   of the range and puts the split pages to the right migratetype free lists
   based on the pageblock migratetypes;
4. returns pages not in the range as it did before.

Isolation needs to be done at MAX_ORDER - 1 granularity, because otherwise
either 1) it is needed to detect to-be-isolated page size (free, PageHuge, THP,
or other PageCompound) to make sure all pageblocks belonging to a single page
are isolated together and later restore pageblock migratetypes outside the
range, or 2) assuming isolation happens at pageblock granularity, a free page
with multi-migratetype pageblocks can seen in free page path and needs
to be split and freed at pageblock granularity.

One optimization might come later:
1. make MIGRATE_ISOLATE a separate bit to avoid saving and restoring existing
   migratetypes before and after isolation respectively.

Feel free to give comments and suggestions. Thanks.

[1] https://lore.kernel.org/linux-mm/20210805190253.2795604-1-zi.yan@sent.com/
[2] https://lore.kernel.org/linux-mm/d19fb078-cb9b-f60f-e310-fdeea1b947d2@redhat.com/


Zi Yan (7):
  mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
  mm: page_isolation: move has_unmovable_pages() to mm/page_isolation.c
  mm: page_isolation: check specified range for unmovable pages
  mm: make alloc_contig_range work at pageblock granularity
  mm: cma: use pageblock_order as the single alignment
  drivers: virtio_mem: use pageblock size as the minimum virtio_mem
    size.
  arch: powerpc: adjust fadump alignment to be pageblock aligned.

 arch/powerpc/include/asm/fadump-internal.h |   4 +-
 drivers/virtio/virtio_mem.c                |   7 +-
 include/linux/mmzone.h                     |  16 +-
 include/linux/page-isolation.h             |   3 +-
 kernel/dma/contiguous.c                    |   2 +-
 mm/cma.c                                   |   6 +-
 mm/memory_hotplug.c                        |  12 +-
 mm/page_alloc.c                            | 337 +++++++++++----------
 mm/page_isolation.c                        | 154 +++++++++-
 9 files changed, 352 insertions(+), 189 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH v4 2/7] mm: page_isolation: move has_unmovable_pages() to mm/page_isolation.c
From: Zi Yan @ 2022-01-19 19:06 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Mel Gorman, Zi Yan, Robin Murphy, linux-kernel, iommu, Eric Ren,
	virtualization, linuxppc-dev, Christoph Hellwig, Vlastimil Babka,
	Marek Szyprowski
In-Reply-To: <20220119190623.1029355-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

has_unmovable_pages() is only used in mm/page_isolation.c. Move it from
mm/page_alloc.c and make it static.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/page-isolation.h |   2 -
 mm/page_alloc.c                | 119 ---------------------------------
 mm/page_isolation.c            | 119 +++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 121 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 572458016331..e14eddf6741a 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -33,8 +33,6 @@ static inline bool is_migrate_isolate(int migratetype)
 #define MEMORY_OFFLINE	0x1
 #define REPORT_FAILURE	0x2
 
-struct page *has_unmovable_pages(struct zone *zone, struct page *page,
-				 int migratetype, int flags);
 void set_pageblock_migratetype(struct page *page, int migratetype);
 int move_freepages_block(struct zone *zone, struct page *page,
 				int migratetype, int *num_movable);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 15de65215c02..1d812268c2a9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8859,125 +8859,6 @@ void *__init alloc_large_system_hash(const char *tablename,
 	return table;
 }
 
-/*
- * This function checks whether pageblock includes unmovable pages or not.
- *
- * PageLRU check without isolation or lru_lock could race so that
- * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
- * check without lock_page also may miss some movable non-lru pages at
- * race condition. So you can't expect this function should be exact.
- *
- * Returns a page without holding a reference. If the caller wants to
- * dereference that page (e.g., dumping), it has to make sure that it
- * cannot get removed (e.g., via memory unplug) concurrently.
- *
- */
-struct page *has_unmovable_pages(struct zone *zone, struct page *page,
-				 int migratetype, int flags)
-{
-	unsigned long iter = 0;
-	unsigned long pfn = page_to_pfn(page);
-	unsigned long offset = pfn % pageblock_nr_pages;
-
-	if (is_migrate_cma_page(page)) {
-		/*
-		 * CMA allocations (alloc_contig_range) really need to mark
-		 * isolate CMA pageblocks even when they are not movable in fact
-		 * so consider them movable here.
-		 */
-		if (is_migrate_cma(migratetype))
-			return NULL;
-
-		return page;
-	}
-
-	for (; iter < pageblock_nr_pages - offset; iter++) {
-		page = pfn_to_page(pfn + iter);
-
-		/*
-		 * Both, bootmem allocations and memory holes are marked
-		 * PG_reserved and are unmovable. We can even have unmovable
-		 * allocations inside ZONE_MOVABLE, for example when
-		 * specifying "movablecore".
-		 */
-		if (PageReserved(page))
-			return page;
-
-		/*
-		 * If the zone is movable and we have ruled out all reserved
-		 * pages then it should be reasonably safe to assume the rest
-		 * is movable.
-		 */
-		if (zone_idx(zone) == ZONE_MOVABLE)
-			continue;
-
-		/*
-		 * Hugepages are not in LRU lists, but they're movable.
-		 * THPs are on the LRU, but need to be counted as #small pages.
-		 * We need not scan over tail pages because we don't
-		 * handle each tail page individually in migration.
-		 */
-		if (PageHuge(page) || PageTransCompound(page)) {
-			struct page *head = compound_head(page);
-			unsigned int skip_pages;
-
-			if (PageHuge(page)) {
-				if (!hugepage_migration_supported(page_hstate(head)))
-					return page;
-			} else if (!PageLRU(head) && !__PageMovable(head)) {
-				return page;
-			}
-
-			skip_pages = compound_nr(head) - (page - head);
-			iter += skip_pages - 1;
-			continue;
-		}
-
-		/*
-		 * We can't use page_count without pin a page
-		 * because another CPU can free compound page.
-		 * This check already skips compound tails of THP
-		 * because their page->_refcount is zero at all time.
-		 */
-		if (!page_ref_count(page)) {
-			if (PageBuddy(page))
-				iter += (1 << buddy_order(page)) - 1;
-			continue;
-		}
-
-		/*
-		 * The HWPoisoned page may be not in buddy system, and
-		 * page_count() is not 0.
-		 */
-		if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
-			continue;
-
-		/*
-		 * We treat all PageOffline() pages as movable when offlining
-		 * to give drivers a chance to decrement their reference count
-		 * in MEM_GOING_OFFLINE in order to indicate that these pages
-		 * can be offlined as there are no direct references anymore.
-		 * For actually unmovable PageOffline() where the driver does
-		 * not support this, we will fail later when trying to actually
-		 * move these pages that still have a reference count > 0.
-		 * (false negatives in this function only)
-		 */
-		if ((flags & MEMORY_OFFLINE) && PageOffline(page))
-			continue;
-
-		if (__PageMovable(page) || PageLRU(page))
-			continue;
-
-		/*
-		 * If there are RECLAIMABLE pages, we need to check
-		 * it.  But now, memory offline itself doesn't call
-		 * shrink_node_slabs() and it still to be fixed.
-		 */
-		return page;
-	}
-	return NULL;
-}
-
 #ifdef CONFIG_CONTIG_ALLOC
 static unsigned long pfn_max_align_down(unsigned long pfn)
 {
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 6a0ddda6b3c5..6c841274bf46 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -15,6 +15,125 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/page_isolation.h>
 
+/*
+ * This function checks whether pageblock includes unmovable pages or not.
+ *
+ * PageLRU check without isolation or lru_lock could race so that
+ * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
+ * check without lock_page also may miss some movable non-lru pages at
+ * race condition. So you can't expect this function should be exact.
+ *
+ * Returns a page without holding a reference. If the caller wants to
+ * dereference that page (e.g., dumping), it has to make sure that it
+ * cannot get removed (e.g., via memory unplug) concurrently.
+ *
+ */
+static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
+				 int migratetype, int flags)
+{
+	unsigned long iter = 0;
+	unsigned long pfn = page_to_pfn(page);
+	unsigned long offset = pfn % pageblock_nr_pages;
+
+	if (is_migrate_cma_page(page)) {
+		/*
+		 * CMA allocations (alloc_contig_range) really need to mark
+		 * isolate CMA pageblocks even when they are not movable in fact
+		 * so consider them movable here.
+		 */
+		if (is_migrate_cma(migratetype))
+			return NULL;
+
+		return page;
+	}
+
+	for (; iter < pageblock_nr_pages - offset; iter++) {
+		page = pfn_to_page(pfn + iter);
+
+		/*
+		 * Both, bootmem allocations and memory holes are marked
+		 * PG_reserved and are unmovable. We can even have unmovable
+		 * allocations inside ZONE_MOVABLE, for example when
+		 * specifying "movablecore".
+		 */
+		if (PageReserved(page))
+			return page;
+
+		/*
+		 * If the zone is movable and we have ruled out all reserved
+		 * pages then it should be reasonably safe to assume the rest
+		 * is movable.
+		 */
+		if (zone_idx(zone) == ZONE_MOVABLE)
+			continue;
+
+		/*
+		 * Hugepages are not in LRU lists, but they're movable.
+		 * THPs are on the LRU, but need to be counted as #small pages.
+		 * We need not scan over tail pages because we don't
+		 * handle each tail page individually in migration.
+		 */
+		if (PageHuge(page) || PageTransCompound(page)) {
+			struct page *head = compound_head(page);
+			unsigned int skip_pages;
+
+			if (PageHuge(page)) {
+				if (!hugepage_migration_supported(page_hstate(head)))
+					return page;
+			} else if (!PageLRU(head) && !__PageMovable(head)) {
+				return page;
+			}
+
+			skip_pages = compound_nr(head) - (page - head);
+			iter += skip_pages - 1;
+			continue;
+		}
+
+		/*
+		 * We can't use page_count without pin a page
+		 * because another CPU can free compound page.
+		 * This check already skips compound tails of THP
+		 * because their page->_refcount is zero at all time.
+		 */
+		if (!page_ref_count(page)) {
+			if (PageBuddy(page))
+				iter += (1 << buddy_order(page)) - 1;
+			continue;
+		}
+
+		/*
+		 * The HWPoisoned page may be not in buddy system, and
+		 * page_count() is not 0.
+		 */
+		if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
+			continue;
+
+		/*
+		 * We treat all PageOffline() pages as movable when offlining
+		 * to give drivers a chance to decrement their reference count
+		 * in MEM_GOING_OFFLINE in order to indicate that these pages
+		 * can be offlined as there are no direct references anymore.
+		 * For actually unmovable PageOffline() where the driver does
+		 * not support this, we will fail later when trying to actually
+		 * move these pages that still have a reference count > 0.
+		 * (false negatives in this function only)
+		 */
+		if ((flags & MEMORY_OFFLINE) && PageOffline(page))
+			continue;
+
+		if (__PageMovable(page) || PageLRU(page))
+			continue;
+
+		/*
+		 * If there are RECLAIMABLE pages, we need to check
+		 * it.  But now, memory offline itself doesn't call
+		 * shrink_node_slabs() and it still to be fixed.
+		 */
+		return page;
+	}
+	return NULL;
+}
+
 static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags)
 {
 	struct zone *zone = page_zone(page);
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH] powerpc/process, kasan: Silence KASAN warnings in __get_wchan()
From: Kees Cook @ 2022-01-19 18:51 UTC (permalink / raw)
  To: He Ying
  Cc: catalin.marinas, linux-kernel, sxwjean, peterz, paulus, npiggin,
	linuxppc-dev
In-Reply-To: <20220119015025.136902-1-heying24@huawei.com>

On Tue, Jan 18, 2022 at 08:50:25PM -0500, He Ying wrote:
> The following KASAN warning was reported in our kernel.
> 
>   BUG: KASAN: stack-out-of-bounds in get_wchan+0x188/0x250
>   Read of size 4 at addr d216f958 by task ps/14437
> 
>   CPU: 3 PID: 14437 Comm: ps Tainted: G           O      5.10.0 #1
>   Call Trace:
>   [daa63858] [c0654348] dump_stack+0x9c/0xe4 (unreliable)
>   [daa63888] [c035cf0c] print_address_description.constprop.3+0x8c/0x570
>   [daa63908] [c035d6bc] kasan_report+0x1ac/0x218
>   [daa63948] [c00496e8] get_wchan+0x188/0x250
>   [daa63978] [c0461ec8] do_task_stat+0xce8/0xe60
>   [daa63b98] [c0455ac8] proc_single_show+0x98/0x170
>   [daa63bc8] [c03cab8c] seq_read_iter+0x1ec/0x900
>   [daa63c38] [c03cb47c] seq_read+0x1dc/0x290
>   [daa63d68] [c037fc94] vfs_read+0x164/0x510
>   [daa63ea8] [c03808e4] ksys_read+0x144/0x1d0
>   [daa63f38] [c005b1dc] ret_from_syscall+0x0/0x38
>   --- interrupt: c00 at 0x8fa8f4
>       LR = 0x8fa8cc
> 
>   The buggy address belongs to the page:
>   page:98ebcdd2 refcount:0 mapcount:0 mapping:00000000 index:0x2 pfn:0x1216f
>   flags: 0x0()
>   raw: 00000000 00000000 01010122 00000000 00000002 00000000 ffffffff 00000000
>   raw: 00000000
>   page dumped because: kasan: bad access detected
> 
>   Memory state around the buggy address:
>    d216f800: 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00
>    d216f880: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   >d216f900: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
>                                             ^
>    d216f980: f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
>    d216fa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 
> After looking into this issue, I find the buggy address belongs
> to the task stack region. It seems KASAN has something wrong.
> I look into the code of __get_wchan in x86 architecture and
> find the same issue has been resolved by the commit
> f7d27c35ddff ("x86/mm, kasan: Silence KASAN warnings in get_wchan()").
> The solution could be applied to powerpc architecture too.
> 
> As Andrey Ryabinin said, get_wchan() is racy by design, it may
> access volatile stack of running task, thus it may access
> redzone in a stack frame and cause KASAN to warn about this.
> 
> Use READ_ONCE_NOCHECK() to silence these warnings.
> 
> Signed-off-by: He Ying <heying24@huawei.com>

Looks reasonable to me; thanks!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/powerpc/kernel/process.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 984813a4d5dc..a75d20f23dac 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -2160,12 +2160,12 @@ static unsigned long ___get_wchan(struct task_struct *p)
>  		return 0;
>  
>  	do {
> -		sp = *(unsigned long *)sp;
> +		sp = READ_ONCE_NOCHECK(*(unsigned long *)sp);
>  		if (!validate_sp(sp, p, STACK_FRAME_OVERHEAD) ||
>  		    task_is_running(p))
>  			return 0;
>  		if (count > 0) {
> -			ip = ((unsigned long *)sp)[STACK_FRAME_LR_SAVE];
> +			ip = READ_ONCE_NOCHECK(((unsigned long *)sp)[STACK_FRAME_LR_SAVE]);
>  			if (!in_sched_functions(ip))
>  				return ip;
>  		}
> -- 
> 2.17.1
> 

-- 
Kees Cook

^ permalink raw reply

* Re: [RFC PATCH 1/2] powerpc/64: remove system call instruction emulation
From: Naveen N. Rao @ 2022-01-19 17:46 UTC (permalink / raw)
  To: linuxppc-dev, Nicholas Piggin; +Cc: Cédric Le Goater
In-Reply-To: <20220118055821.3065241-2-npiggin@gmail.com>

Nicholas Piggin wrote:
> emulate_step instruction emulation including sc instruction emulation
> initially appeared in xmon. It then emulation code was then moved into
> sstep.c where kprobes could use it too, and later hw_breakpoint and
> uprobes started to use it.
> 
> Until uprobes, the only instruction emulation users were for kernel
> mode instructions.
> 
> - xmon only steps / breaks on kernel addresses.
> - kprobes is kernel only.
> - hw_breakpoint only emulates kernel instructions, single steps user.
> 
> At one point there was support for the kernel to execute sc
> instructions, although that is long removed and it's not clear whether
> there was any upstream instructions or this was used by out of tree
> modules. So system call emulation is not required by the above users.
> 
> uprobes uses emulate_step and it appears possible to emulate sc
> instruction in userspace. This type of system call emulation is broken
> and it's not clear it ever worked well.
> 
> The big complication is that userspace takes an interrupt to the kernel
> to emulate the instruction. The user->kernel interrupt sets up registers
> and interrupt stack frame expecting to return to userspace, then system
> call instruction emulation re-directs that stack frame to the kernel,
> early in the system call interrupt handler. This means the the interrupt
> return code takes the kernel->kernel restore path, which does not restore
> everything as the system call interrupt handler would expect coming from
> userspace. regs->iamr appears to get lost for example, because the
> kernel->kernel return does not restore the user iamr. Accounting such as
> irqflags tracing and CPU accounting does not get flipped back to user
> mode as the system call handler expects, so those appear to enter the
> kernel twice without returning to userspace.
> 
> These things may be individually fixable with various complication, but
> it is a big complexity to support this just for uprobes.
> 
> uprobes emulates the instruction rather than stepping for performance
> reasons. System calls instructions should not be a significant source
> of uprobes and already expensive, so skipping emulation should not be
> a significant problem.

I agree with that assessment, though I think we will need to disable 
probing on 'sc'/'scv' instructions. Per the ISA, section 6.5.15 on 
'Trace Interrupt', we can't single step a system call instruction:
    "Successful completion for an instruction means that the
    instruction caused no other interrupt and, if the thread
    is in Transactional state, did not cause the transaction
    to fail in such a way that the instruction did not com-
    plete (see Section 5.3.1 of Book II). Thus a Trace inter-
    rupt never occurs for a System Call or System Call
    Vectored instruction, or for a Trap instruction that traps,
    or for a dcbf that is executed in Transactional state.
    The instruction that causes a Trace interrupt is called
    the “traced instruction”."

I am not aware of any use case requiring probes on a system call 
instruction, so I think we can disable probing on system call 
instructions (patch further below).

> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/lib/sstep.c | 36 ------------------------------------
>  1 file changed, 36 deletions(-)
> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index a94b0cd0bdc5..ee3bc45fb23b 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -15,9 +15,6 @@
>  #include <asm/cputable.h>
>  #include <asm/disassemble.h>
> 
> -extern char system_call_common[];
> -extern char system_call_vectored_emulate[];
> -
>  #ifdef CONFIG_PPC64
>  /* Bits in SRR1 that are copied from MSR */
>  #define MSR_MASK	0xffffffff87c0ffffUL
> @@ -3650,39 +3647,6 @@ int emulate_step(struct pt_regs *regs, ppc_inst_t instr)
>  		goto instr_done;
> 
>  #ifdef CONFIG_PPC64
> -	case SYSCALL:	/* sc */
> -		/*
> -		 * N.B. this uses knowledge about how the syscall
> -		 * entry code works.  If that is changed, this will
> -		 * need to be changed also.
> -		 */
> -		if (IS_ENABLED(CONFIG_PPC_FAST_ENDIAN_SWITCH) &&
> -				cpu_has_feature(CPU_FTR_REAL_LE) &&
> -				regs->gpr[0] == 0x1ebe) {
> -			regs_set_return_msr(regs, regs->msr ^ MSR_LE);
> -			goto instr_done;
> -		}
> -		regs->gpr[9] = regs->gpr[13];
> -		regs->gpr[10] = MSR_KERNEL;
> -		regs->gpr[11] = regs->nip + 4;
> -		regs->gpr[12] = regs->msr & MSR_MASK;
> -		regs->gpr[13] = (unsigned long) get_paca();
> -		regs_set_return_ip(regs, (unsigned long) &system_call_common);
> -		regs_set_return_msr(regs, MSR_KERNEL);
> -		return 1;
> -
> -#ifdef CONFIG_PPC_BOOK3S_64
> -	case SYSCALL_VECTORED_0:	/* scv 0 */
> -		regs->gpr[9] = regs->gpr[13];
> -		regs->gpr[10] = MSR_KERNEL;
> -		regs->gpr[11] = regs->nip + 4;
> -		regs->gpr[12] = regs->msr & MSR_MASK;
> -		regs->gpr[13] = (unsigned long) get_paca();
> -		regs_set_return_ip(regs, (unsigned long) &system_call_vectored_emulate);
> -		regs_set_return_msr(regs, MSR_KERNEL);
> -		return 1;
> -#endif
> -

Given that we should not be probing syscall instructions, I think it is 
better to return -1 for these two, similar to the RFI below. With that 
change, for this patch:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

>  	case RFI:
>  		return -1;
>  #endif


Thanks,
Naveen

--
[PATCH] powerpc/uprobes: Reject uprobe on a system call instruction

Per the ISA, a Trace interrupt is not generated for a system call
[vectored] instruction. Reject uprobes on such instructions as we are
not emulating a system call [vectored] instruction anymore.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h | 1 +
 arch/powerpc/kernel/uprobes.c         | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index efad07081cc0e5..fedf843bcdddeb 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -411,6 +411,7 @@
 #define PPC_RAW_DCBFPS(a, b)		(0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (4 << 21))
 #define PPC_RAW_DCBSTPS(a, b)		(0x7c0000ac | ___PPC_RA(a) | ___PPC_RB(b) | (6 << 21))
 #define PPC_RAW_SC()			(0x44000002)
+#define PPC_RAW_SCV()			(0x44000001)
 #define PPC_RAW_SYNC()			(0x7c0004ac)
 #define PPC_RAW_ISYNC()			(0x4c00012c)
 
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index c6975467d9ffdc..bedca31391d043 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -41,6 +41,12 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
 	if (addr & 0x03)
 		return -EINVAL;
 
+	if (ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SC() ||
+	    ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SCV()) {
+		pr_info("Rejecting uprobe on system call instruction\n");
+		return -EINVAL;
+	}
+
 	if (cpu_has_feature(CPU_FTR_ARCH_31) &&
 	    ppc_inst_prefixed(ppc_inst_read(auprobe->insn)) &&
 	    (addr & 0x3f) == 60) {

base-commit: 863a7c25c334ed368b4508fccae92d6bb61e71a4
-- 
2.34.1



^ permalink raw reply related

* Re: [PATCH v3] powerpc/papr_scm: Implement initial support for injecting smart errors
From: Vaibhav Jain @ 2022-01-19 15:05 UTC (permalink / raw)
  To: Ira Weiny
  Cc: nvdimm, Shivaprasad G Bhat, Aneesh Kumar K . V, Dan Williams,
	linuxppc-dev
In-Reply-To: <20220118175915.GB209936@iweiny-DESK2.sc.intel.com>


Hi Ira, Thanks for reviewing this patch.

Ira Weiny <ira.weiny@intel.com> writes:

> On Thu, Jan 13, 2022 at 05:32:52PM +0530, Vaibhav Jain wrote:
> [snip]
>
>>  
>> +/* Inject a smart error Add the dirty-shutdown-counter value to the pdsm */
>> +static int papr_pdsm_smart_inject(struct papr_scm_priv *p,
>> +				  union nd_pdsm_payload *payload)
>> +{
>> +	int rc;
>> +	u32 supported_flags = 0;
>> +	u64 mask = 0, override = 0;
>> +
>> +	/* Check for individual smart error flags and update mask and override */
>> +	if (payload->smart_inject.flags & PDSM_SMART_INJECT_HEALTH_FATAL) {
>> +		supported_flags |= PDSM_SMART_INJECT_HEALTH_FATAL;
>> +		mask |= PAPR_PMEM_HEALTH_FATAL;
>> +		override |= payload->smart_inject.fatal_enable ?
>> +			PAPR_PMEM_HEALTH_FATAL : 0;
>> +	}
>> +
>> +	if (payload->smart_inject.flags & PDSM_SMART_INJECT_BAD_SHUTDOWN) {
>> +		supported_flags |= PDSM_SMART_INJECT_BAD_SHUTDOWN;
>> +		mask |= PAPR_PMEM_SHUTDOWN_DIRTY;
>> +		override |= payload->smart_inject.unsafe_shutdown_enable ?
>> +			PAPR_PMEM_SHUTDOWN_DIRTY : 0;
>> +	}
>> +
>
> I'm struggling to see why there is a need for both a flag and an 8 bit 'enable'
> value?
>
This is to enable the inject/uninject error usecase with ndctl which
lets user select individual error conditions like bad_shutdown or
fatal-health state.

The nd_papr_pdsm_smart_inject.flag field indicates which error
conditions needs to be tweaked and individual __u8 fields like
'fatal_enable' are boolean values to indicate the inject/uninject state
of that error condition.

For e.g to uninject fatal-health and inject unsafe-shutdown following
nd_papr_pdsm_smart_inject payload can be sent:

{
.flags = PDSM_SMART_INJECT_HEALTH_FATAL |
       PDSM_SMART_INJECT_BAD_SHUTDOWN,
.fatal_enable = 0,
.unsafe_shutdown_enable = 1,
}


To just to inject fatal-health following nd_papr_pdsm_smart_inject
payload can be sent:

{
.flags = PDSM_SMART_INJECT_HEALTH_FATAL,
.fatal_enable = 1,
.unsafe_shutdown_enable = <dont-care>,
}


> Ira
>

-- 
Cheers
~ Vaibhav

^ permalink raw reply

* Re: [PATCH v2 1/3] mm: vmalloc: Let user to control huge vmalloc default behavior
From: Matthew Wilcox @ 2022-01-19 13:48 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: x86, H. Peter Anvin, Dave Hansen, linux-doc, Jonathan Corbet,
	linux-kernel, Nicholas Piggin, linux-mm, Will Deacon, Ingo Molnar,
	Borislav Petkov, Catalin Marinas, Paul Mackerras, Andrew Morton,
	linuxppc-dev, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <93db576e-9a62-ee98-5af2-a62f8386212c@huawei.com>

On Wed, Jan 19, 2022 at 09:44:20PM +0800, Kefeng Wang wrote:
> 
> On 2022/1/19 21:22, Matthew Wilcox wrote:
> > On Wed, Jan 19, 2022 at 08:57:58PM +0800, Kefeng Wang wrote:
> > > Only parts of our products wants this feature,  we add some interfaces which
> > > only
> > > 
> > > alloc hugevmalloc for them, eg,
> > > vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..
> > > 
> > > for our products, but it's not the choice of most products, also add
> > > nohugevmalloc
> > > 
> > > for most products is expensive, so this is the reason for adding the patch.
> > > 
> > > more config/cmdline are more flexible for test/products,
> > But why do only some products want it?  What goes wrong if all products
> > enable it?  Features should be auto-tuning, not relying on admins to
> > understand them.
> 
> Because this feature will use more memory for vmalloc/vmap, that's why we
> add some explicit
> interfaces as said above in our kernel to control the user.

Have you validated that?  What sort of performance penalty do you see?

^ permalink raw reply

* Re: [PATCH v2 1/3] mm: vmalloc: Let user to control huge vmalloc default behavior
From: Kefeng Wang @ 2022-01-19 13:44 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: x86, H. Peter Anvin, Dave Hansen, linux-doc, Jonathan Corbet,
	linux-kernel, Nicholas Piggin, linux-mm, Will Deacon, Ingo Molnar,
	Borislav Petkov, Catalin Marinas, Paul Mackerras, Andrew Morton,
	linuxppc-dev, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <YegQfIQibQi993dp@casper.infradead.org>


On 2022/1/19 21:22, Matthew Wilcox wrote:
> On Wed, Jan 19, 2022 at 08:57:58PM +0800, Kefeng Wang wrote:
>> Only parts of our products wants this feature,  we add some interfaces which
>> only
>>
>> alloc hugevmalloc for them, eg,
>> vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..
>>
>> for our products, but it's not the choice of most products, also add
>> nohugevmalloc
>>
>> for most products is expensive, so this is the reason for adding the patch.
>>
>> more config/cmdline are more flexible for test/products,
> But why do only some products want it?  What goes wrong if all products
> enable it?  Features should be auto-tuning, not relying on admins to
> understand them.

Because this feature will use more memory for vmalloc/vmap, that's why 
we add some explicit
interfaces as said above in our kernel to control the user.


^ permalink raw reply

* Re: [PATCH v2 3/3] x86: Support huge vmalloc mappings
From: Kefeng Wang @ 2022-01-19 13:32 UTC (permalink / raw)
  To: Nicholas Piggin, Andrew Morton, Jonathan Corbet, Dave Hansen,
	linux-arm-kernel, linux-doc, linux-kernel, linux-mm, linuxppc-dev,
	x86
  Cc: Matthew Wilcox, Catalin Marinas, Dave Hansen, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Paul Mackerras, Thomas Gleixner,
	Will Deacon
In-Reply-To: <1642565468.c0jax91tvn.astroid@bobo.none>


On 2022/1/19 12:17, Nicholas Piggin wrote:
> Excerpts from Dave Hansen's message of January 19, 2022 3:28 am:
>> On 1/17/22 6:46 PM, Nicholas Piggin wrote:
>>>> This all sounds very fragile to me.  Every time a new architecture would
>>>> get added for huge vmalloc() support, the developer needs to know to go
>>>> find that architecture's module_alloc() and add this flag.
>>> This is documented in the Kconfig.
>>>
>>>   #
>>>   #  Archs that select this would be capable of PMD-sized vmaps (i.e.,
>>>   #  arch_vmap_pmd_supported() returns true), and they must make no assumptions
>>>   #  that vmalloc memory is mapped with PAGE_SIZE ptes. The VM_NO_HUGE_VMAP flag
>>>   #  can be used to prohibit arch-specific allocations from using hugepages to
>>>   #  help with this (e.g., modules may require it).
>>>   #
>>>   config HAVE_ARCH_HUGE_VMALLOC
>>>           depends on HAVE_ARCH_HUGE_VMAP
>>>           bool
>>>
>>> Is it really fair to say it's *very* fragile? Surely it's reasonable to
>>> read the (not very long) documentation ad understand the consequences for
>>> the arch code before enabling it.
>> Very fragile or not, I think folks are likely to get it wrong.  It would
>> be nice to have it default *everyone* to safe and slow and make *sure*
> It's not safe to enable though. That's the problem. If it was just
> modules then you'd have a point but it could be anything.
>
>> they go look at the architecture modules code itself before enabling
>> this for modules.
> This is required not just for modules for the whole arch code, it
> has to be looked at and decided this will work.
>
>> Just from that Kconfig text, I don't think I'd know off the top of my
>> head what do do for x86, or what code I needed to go touch.
> You have to make sure arch/x86 makes no assumptions that vmalloc memory
> is backed by PAGE_SIZE ptes. If you can't do that then you shouldn't
> enable the option. The option can not explain it any more because any
> arch could do anything with its mappings. The module code is an example,
> not the recipe.

Hi Nick, Dave and Christophe,thanks for your review,  a little 
confused,   I think,

1) for ppc/arm64 module_alloc(),  it must set VM_NO_HUGE_VMAP because the

arch's set_memory_* funcitons can only support PAGE_SIZE mapping, due to the

limit of apply_to_page_range().

2) but for x86's module_alloc(), add VM_NO_HUGE_VMAP is to avoid 
fragmentation,

x86's __change_page_attr functions will split the huge mapping. this 
flags is not a must.


and the behavior above occurred when STRICT_MODULE_RWX enabled, so

1) add a unified function to set vm flags(suggested by Dave ) or

2) add vm flags with some comments to per-arch's module_alloc()

are both acceptable, for the way of unified function ,  we could make 
this a default recipe

with STRICT_MODULE_RWX, also make two more vm flags into it, eg,

+unsigned long module_alloc_vm_flags(bool need_flush_reset_perms)
+{
+       unsigned long vm_flags = VM_DEFER_KMEMLEAK;
+
+       if (need_flush_reset_perms)
+               vm_flags |= VM_FLUSH_RESET_PERMS;
+       /*
+        * Modules use a single, large vmalloc(). Different permissions
+        * are applied later and will fragment huge mappings or even
+        * fails in set_memory_* on some architectures. Avoid using
+        * huge pages for modules.
+        */
+       if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
+               vm_flags |= VM_NO_HUGE_VMAP;
+
+       return vm_flags;
+}

then called each arch's module_alloc().

Any suggestion, many thanks.


>
> Thanks,
> Nick
> .

^ permalink raw reply

* Re: [PATCH v2 1/3] mm: vmalloc: Let user to control huge vmalloc default behavior
From: Matthew Wilcox @ 2022-01-19 13:22 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: x86, H. Peter Anvin, Dave Hansen, linux-doc, Jonathan Corbet,
	linux-kernel, Nicholas Piggin, linux-mm, Will Deacon, Ingo Molnar,
	Borislav Petkov, Catalin Marinas, Paul Mackerras, Andrew Morton,
	linuxppc-dev, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <f0dd59eb-6eb8-5b60-508d-7f4022f655ec@huawei.com>

On Wed, Jan 19, 2022 at 08:57:58PM +0800, Kefeng Wang wrote:
> Only parts of our products wants this feature,  we add some interfaces which
> only
> 
> alloc hugevmalloc for them, eg,
> vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..
> 
> for our products, but it's not the choice of most products, also add
> nohugevmalloc
> 
> for most products is expensive, so this is the reason for adding the patch.
> 
> more config/cmdline are more flexible for test/products,

But why do only some products want it?  What goes wrong if all products
enable it?  Features should be auto-tuning, not relying on admins to
understand them.

^ permalink raw reply

* Re: [PATCH v2 1/3] mm: vmalloc: Let user to control huge vmalloc default behavior
From: Kefeng Wang @ 2022-01-19 12:57 UTC (permalink / raw)
  To: Nicholas Piggin, Andrew Morton, Jonathan Corbet, linux-arm-kernel,
	linux-doc, linux-kernel, linux-mm, linuxppc-dev, x86
  Cc: Matthew Wilcox, Catalin Marinas, Dave Hansen, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Paul Mackerras, Thomas Gleixner,
	Will Deacon
In-Reply-To: <1642473992.qrnqczjfna.astroid@bobo.none>


On 2022/1/18 10:52, Nicholas Piggin wrote:
> Excerpts from Kefeng Wang's message of December 28, 2021 12:59 am:
>> Introduce HUGE_VMALLOC_DEFAULT_ENABLED and make it default y, this
>> let user to choose whether or not enable huge vmalloc mappings by
>> default.
>>
>> Meanwhile, add new hugevmalloc=on/off parameter to enable or disable
>> this feature at boot time, nohugevmalloc is still supported and
>> equivalent to hugevmalloc=off.
> Runtime options are bad enough, Kconfig and boot options are even worse.

nohugevmalloc is like blacklists, on the other hand, Add a 
HUGE_VMALLOC_DEFAULT_ENABLED

to close hugevmalloc default and enable it only via hugevmalloc=on is 
whiteList.


Only parts of our products wants this feature,  we add some interfaces 
which only

alloc hugevmalloc for them, eg, 
vmap_hugepage/vmalloc_hugepage/remap_vmalloc_hugepage_range..

for our products, but it's not the choice of most products, also add 
nohugevmalloc

for most products is expensive, so this is the reason for adding the patch.

more config/cmdline are more flexible for test/products,

>
> The 'nohugevmalloc' option mirrors 'nohugeiomap' and is not expected to
> ever be understood by an administrator unless a kernel developer is
> working with them to hunt down a regression.
>
> IMO there should be no new options. You could switch it off for
> CONFIG_BASE_SMALL perhaps, and otherwise just try to work on heuristics
> first. Bring in new options once it's proven they're needed.

but yes, this patch is optional, could others give some more comments 
about this way?

Thanks.

> Aside from that, thanks for working on these ports, great work.
>
> Thanks,
> Nick
> .

^ permalink raw reply

* Re: [PATCH v3 2/2] powerpc: Add set_memory_{p/np}() and remove set_memory_attr()
From: Christophe Leroy @ 2022-01-19 12:28 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Paul Mackerras, Maxime Bizon, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cda2b44b55c96f9ac69fa92e68c01084ec9495c5.1640344012.git.christophe.leroy@csgroup.eu>

Hi Michael,

Can we get this series in fixes as well ?

Thanks
Christophe

Le 24/12/2021 à 12:07, Christophe Leroy a écrit :
> set_memory_attr() was implemented by commit 4d1755b6a762 ("powerpc/mm:
> implement set_memory_attr()") because the set_memory_xx() couldn't
> be used at that time to modify memory "on the fly" as explained it
> the commit.
> 
> But set_memory_attr() uses set_pte_at() which leads to warnings when
> CONFIG_DEBUG_VM is selected, because set_pte_at() is unexpected for
> updating existing page table entries.
> 
> The check could be bypassed by using __set_pte_at() instead,
> as it was the case before commit c988cfd38e48 ("powerpc/32:
> use set_memory_attr()") but since commit 9f7853d7609d ("powerpc/mm:
> Fix set_memory_*() against concurrent accesses") it is now possible
> to use set_memory_xx() functions to update page table entries
> "on the fly" because the update is now atomic.
> 
> For DEBUG_PAGEALLOC we need to clear and set back _PAGE_PRESENT.
> Add set_memory_np() and set_memory_p() for that.
> 
> Replace all uses of set_memory_attr() by the relevant set_memory_xx()
> and remove set_memory_attr().
> 
> Reported-by: Maxime Bizon <mbizon@freebox.fr>
> Fixes: c988cfd38e48 ("powerpc/32: use set_memory_attr()")
> Cc: stable@vger.kernel.org
> Depends-on: 9f7853d7609d ("powerpc/mm: Fix set_memory_*() against concurrent accesses")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Reviewed-by: Russell Currey <ruscur@russell.cc>
> Tested-by: Maxime Bizon <mbizon@freebox.fr>
> ---
> v3: Use _PAGE_PRESENT directly as all platforms have the bit
> 
> v2: Add comment to SET_MEMORY_P and SET_MEMORY_NP
> ---
>   arch/powerpc/include/asm/set_memory.h | 12 ++++++++-
>   arch/powerpc/mm/pageattr.c            | 39 +++++----------------------
>   arch/powerpc/mm/pgtable_32.c          | 24 ++++++++---------
>   3 files changed, 28 insertions(+), 47 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> index b040094f7920..7ebc807aa8cc 100644
> --- a/arch/powerpc/include/asm/set_memory.h
> +++ b/arch/powerpc/include/asm/set_memory.h
> @@ -6,6 +6,8 @@
>   #define SET_MEMORY_RW	1
>   #define SET_MEMORY_NX	2
>   #define SET_MEMORY_X	3
> +#define SET_MEMORY_NP	4	/* Set memory non present */
> +#define SET_MEMORY_P	5	/* Set memory present */
>   
>   int change_memory_attr(unsigned long addr, int numpages, long action);
>   
> @@ -29,6 +31,14 @@ static inline int set_memory_x(unsigned long addr, int numpages)
>   	return change_memory_attr(addr, numpages, SET_MEMORY_X);
>   }
>   
> -int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot);
> +static inline int set_memory_np(unsigned long addr, int numpages)
> +{
> +	return change_memory_attr(addr, numpages, SET_MEMORY_NP);
> +}
> +
> +static inline int set_memory_p(unsigned long addr, int numpages)
> +{
> +	return change_memory_attr(addr, numpages, SET_MEMORY_P);
> +}
>   
>   #endif
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> index 8812454e70ff..85753e32a4de 100644
> --- a/arch/powerpc/mm/pageattr.c
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -46,6 +46,12 @@ static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
>   	case SET_MEMORY_X:
>   		pte_update_delta(ptep, addr, _PAGE_KERNEL_RO, _PAGE_KERNEL_ROX);
>   		break;
> +	case SET_MEMORY_NP:
> +		pte_update(&init_mm, addr, ptep, _PAGE_PRESENT, 0, 0);
> +		break;
> +	case SET_MEMORY_P:
> +		pte_update(&init_mm, addr, ptep, 0, _PAGE_PRESENT, 0);
> +		break;
>   	default:
>   		WARN_ON_ONCE(1);
>   		break;
> @@ -90,36 +96,3 @@ int change_memory_attr(unsigned long addr, int numpages, long action)
>   	return apply_to_existing_page_range(&init_mm, start, size,
>   					    change_page_attr, (void *)action);
>   }
> -
> -/*
> - * Set the attributes of a page:
> - *
> - * This function is used by PPC32 at the end of init to set final kernel memory
> - * protection. It includes changing the maping of the page it is executing from
> - * and data pages it is using.
> - */
> -static int set_page_attr(pte_t *ptep, unsigned long addr, void *data)
> -{
> -	pgprot_t prot = __pgprot((unsigned long)data);
> -
> -	spin_lock(&init_mm.page_table_lock);
> -
> -	set_pte_at(&init_mm, addr, ptep, pte_modify(*ptep, prot));
> -	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> -
> -	spin_unlock(&init_mm.page_table_lock);
> -
> -	return 0;
> -}
> -
> -int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot)
> -{
> -	unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> -	unsigned long sz = numpages * PAGE_SIZE;
> -
> -	if (numpages <= 0)
> -		return 0;
> -
> -	return apply_to_existing_page_range(&init_mm, start, sz, set_page_attr,
> -					    (void *)pgprot_val(prot));
> -}
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 906e4e4328b2..f71ededdc02a 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -135,10 +135,12 @@ void mark_initmem_nx(void)
>   	unsigned long numpages = PFN_UP((unsigned long)_einittext) -
>   				 PFN_DOWN((unsigned long)_sinittext);
>   
> -	if (v_block_mapped((unsigned long)_sinittext))
> +	if (v_block_mapped((unsigned long)_sinittext)) {
>   		mmu_mark_initmem_nx();
> -	else
> -		set_memory_attr((unsigned long)_sinittext, numpages, PAGE_KERNEL);
> +	} else {
> +		set_memory_nx((unsigned long)_sinittext, numpages);
> +		set_memory_rw((unsigned long)_sinittext, numpages);
> +	}
>   }
>   
>   #ifdef CONFIG_STRICT_KERNEL_RWX
> @@ -152,18 +154,14 @@ void mark_rodata_ro(void)
>   		return;
>   	}
>   
> -	numpages = PFN_UP((unsigned long)_etext) -
> -		   PFN_DOWN((unsigned long)_stext);
> -
> -	set_memory_attr((unsigned long)_stext, numpages, PAGE_KERNEL_ROX);
>   	/*
> -	 * mark .rodata as read only. Use __init_begin rather than __end_rodata
> -	 * to cover NOTES and EXCEPTION_TABLE.
> +	 * mark .text and .rodata as read only. Use __init_begin rather than
> +	 * __end_rodata to cover NOTES and EXCEPTION_TABLE.
>   	 */
>   	numpages = PFN_UP((unsigned long)__init_begin) -
> -		   PFN_DOWN((unsigned long)__start_rodata);
> +		   PFN_DOWN((unsigned long)_stext);
>   
> -	set_memory_attr((unsigned long)__start_rodata, numpages, PAGE_KERNEL_RO);
> +	set_memory_ro((unsigned long)_stext, numpages);
>   
>   	// mark_initmem_nx() should have already run by now
>   	ptdump_check_wx();
> @@ -179,8 +177,8 @@ void __kernel_map_pages(struct page *page, int numpages, int enable)
>   		return;
>   
>   	if (enable)
> -		set_memory_attr(addr, numpages, PAGE_KERNEL);
> +		set_memory_p(addr, numpages);
>   	else
> -		set_memory_attr(addr, numpages, __pgprot(0));
> +		set_memory_np(addr, numpages);
>   }
>   #endif /* CONFIG_DEBUG_PAGEALLOC */

^ permalink raw reply

* Re: [PATCH] powerpc/603: Fix boot failure with DEBUG_PAGEALLOC and KFENCE
From: Christophe Leroy @ 2022-01-19 11:19 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Paul Mackerras, Maxime Bizon, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <aea33b4813a26bdb9378b5f273f00bd5d4abe240.1638857364.git.christophe.leroy@csgroup.eu>

Michael, ping.

Le 07/12/2021 à 07:10, Christophe Leroy a écrit :
> Allthough kernel text is always mapped with BATs, we still have
> inittext mapped with pages, so TLB miss handling is required
> when CONFIG_DEBUG_PAGEALLOC or CONFIG_KFENCE is set.
> 
> The final solution should be to set a BAT that also maps inittext
> but that BAT then needs to be cleared at end of init, and it will
> require more changes to be able to do it properly.
> 
> As DEBUG_PAGEALLOC or KFENCE are debugging, performance is not a big
> deal so let's fix it simply for now to enable easy stable application.
> 
> Reported-by: Maxime Bizon <mbizon@freebox.fr>
> Fixes: 035b19a15a98 ("powerpc/32s: Always map kernel text and rodata with BATs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>   arch/powerpc/kernel/head_book3s_32.S | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S
> index 68e5c0a7e99d..2e2a8211b17b 100644
> --- a/arch/powerpc/kernel/head_book3s_32.S
> +++ b/arch/powerpc/kernel/head_book3s_32.S
> @@ -421,14 +421,14 @@ InstructionTLBMiss:
>    */
>   	/* Get PTE (linux-style) and check access */
>   	mfspr	r3,SPRN_IMISS
> -#ifdef CONFIG_MODULES
> +#if defined(CONFIG_MODULES) || defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE)
>   	lis	r1, TASK_SIZE@h		/* check if kernel address */
>   	cmplw	0,r1,r3
>   #endif
>   	mfspr	r2, SPRN_SDR1
>   	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC | _PAGE_USER
>   	rlwinm	r2, r2, 28, 0xfffff000
> -#ifdef CONFIG_MODULES
> +#if defined(CONFIG_MODULES) || defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE)
>   	bgt-	112f
>   	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
>   	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC

^ permalink raw reply

* Re: [PATCH] powerpc/64s: Mask SRR0 before checking against the masked NIP
From: Michael Ellerman @ 2022-01-19 11:06 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20220117134403.2995059-1-npiggin@gmail.com>

On Mon, 17 Jan 2022 23:44:03 +1000, Nicholas Piggin wrote:
> Commit 314f6c23dd8d ("powerpc/64s: Mask NIP before checking against
> SRR0") masked off the low 2 bits of the NIP value in the interrupt
> stack frame in case they are non-zero and mis-compare against a SRR0
> register value of a CPU which always reads back 0 from the 2 low bits
> which are reserved.
> 
> This now causes the opposite problem that an implementation which does
> implement those bits in SRR0 will mis-compare against the masked NIP
> value in which they have been cleared. QEMU is one such implementation,
> and this is allowed by the architecture.
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/64s: Mask SRR0 before checking against the masked NIP
      https://git.kernel.org/powerpc/c/aee101d7b95a03078945681dd7f7ea5e4a1e7686

cheers

^ permalink raw reply

* Re: [PATCH v3] powerpc/32s: Fix kasan_init_region() for KASAN
From: Michael Ellerman @ 2022-01-19 11:06 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Michael Ellerman,
	Paul Mackerras
  Cc: Maxime Bizon, linuxppc-dev, linux-kernel, stable
In-Reply-To: <7a50ef902494d1325227d47d33dada01e52e5518.1641818726.git.christophe.leroy@csgroup.eu>

On Mon, 10 Jan 2022 15:29:25 +0000, Christophe Leroy wrote:
> It has been reported some configuration where the kernel doesn't
> boot with KASAN enabled.
> 
> This is due to wrong BAT allocation for the KASAN area:
> 
> 	---[ Data Block Address Translation ]---
> 	0: 0xc0000000-0xcfffffff 0x00000000       256M Kernel rw      m
> 	1: 0xd0000000-0xdfffffff 0x10000000       256M Kernel rw      m
> 	2: 0xe0000000-0xefffffff 0x20000000       256M Kernel rw      m
> 	3: 0xf8000000-0xf9ffffff 0x2a000000        32M Kernel rw      m
> 	4: 0xfa000000-0xfdffffff 0x2c000000        64M Kernel rw      m
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/32s: Fix kasan_init_region() for KASAN
      https://git.kernel.org/powerpc/c/d37823c3528e5e0705fc7746bcbc2afffb619259

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/time: Fix build failure due to do_hard_irq_enable() on PPC32
From: Michael Ellerman @ 2022-01-19 11:06 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Michael Ellerman,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel, Nicholas Piggin
In-Reply-To: <247e01e0e10f4dbc59b5ff89e81702eb1ee7641e.1641828571.git.christophe.leroy@csgroup.eu>

On Mon, 10 Jan 2022 15:29:53 +0000, Christophe Leroy wrote:
> 	  CC      arch/powerpc/kernel/time.o
> 	In file included from <command-line>:
> 	./arch/powerpc/include/asm/hw_irq.h: In function 'do_hard_irq_enable':
> 	././include/linux/compiler_types.h:335:45: error: call to '__compiletime_assert_35' declared with attribute error: BUILD_BUG failed
> 	  335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> 	      |                                             ^
> 	././include/linux/compiler_types.h:316:25: note: in definition of macro '__compiletime_assert'
> 	  316 |                         prefix ## suffix();                             \
> 	      |                         ^~~~~~
> 	././include/linux/compiler_types.h:335:9: note: in expansion of macro '_compiletime_assert'
> 	  335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> 	      |         ^~~~~~~~~~~~~~~~~~~
> 	./include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
> 	   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> 	      |                                     ^~~~~~~~~~~~~~~~~~
> 	./include/linux/build_bug.h:59:21: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> 	   59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
> 	      |                     ^~~~~~~~~~~~~~~~
> 	./arch/powerpc/include/asm/hw_irq.h:483:9: note: in expansion of macro 'BUILD_BUG'
> 	  483 |         BUILD_BUG();
> 	      |         ^~~~~~~~~
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/time: Fix build failure due to do_hard_irq_enable() on PPC32
      https://git.kernel.org/powerpc/c/87b9d74fb0be80054c729e8d6a119ca0955cedf3

cheers

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox