LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 3/3] arch: powerpc: adjust fadump alignment to pageblock aligned.
From: Zi Yan @ 2021-11-15 19:37 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Robin Murphy, linux-kernel, iommu, Zi Yan, virtualization,
	linuxppc-dev, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20211115193725.737539-1-zi.yan@sent.com>

From: Zi Yan <ziy@nvidia.com>

CMA only requires pageblock alignment. Change 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 8d61c8f3fec4..9198f20b6b68 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.33.0


^ permalink raw reply related

* [RFC PATCH 2/3] drivers: virtio_mem: use pageblock size as the minimum virtio_mem size.
From: Zi Yan @ 2021-11-15 19:37 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Robin Murphy, linux-kernel, iommu, Zi Yan, virtualization,
	linuxppc-dev, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20211115193725.737539-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 | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 0da0af251c73..a0a0994b42e2 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2436,15 +2436,13 @@ 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:
 	 * - Simplifies our page onlining code (virtio_mem_online_page_cb)
 	 *   and fake page onlining code (virtio_mem_fake_online).
 	 * - 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.33.0


^ permalink raw reply related

* [RFC PATCH 3/3] arch: powerpc: adjust fadump alignment to be pageblock aligned.
From: Zi Yan @ 2021-11-15 19:37 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Robin Murphy, linux-kernel, iommu, Zi Yan, virtualization,
	linuxppc-dev, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20211115193725.737539-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 8d61c8f3fec4..9198f20b6b68 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.33.0


^ permalink raw reply related

* [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
From: Zi Yan @ 2021-11-15 19:37 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: Robin Murphy, linux-kernel, iommu, Zi Yan, virtualization,
	linuxppc-dev, Christoph Hellwig, Marek Szyprowski

From: Zi Yan <ziy@nvidia.com>

Hi David,

You suggested to make alloc_contig_range() deal with pageblock_order instead of
MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This
patchset is my attempt to achieve that. Please take a look and let me know if
I am doing it correctly or not.

From what my understanding, cma required alignment of
max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced,
__free_one_page() does not prevent merging two different pageblocks, when
MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation
does prevent that. It should be OK to just align cma to pageblock_order.
alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use
pageblock_order as alignment too.

In terms of virtio_mem, if I understand correctly, it relies on
alloc_contig_range() to obtain contiguous free pages and offlines them to reduce
guest memory size. As the result of alloc_contig_range() alignment change,
virtio_mem should be able to just align PFNs to pageblock_order.

Thanks.


[1] https://lore.kernel.org/linux-mm/28b57903-fae6-47ac-7e1b-a1dd41421349@redhat.com/

Zi Yan (3):
  mm: cma: alloc_contig_range: 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                |  6 ++----
 include/linux/mmzone.h                     |  5 +----
 kernel/dma/contiguous.c                    |  2 +-
 mm/cma.c                                   |  6 ++----
 mm/page_alloc.c                            | 12 +++++-------
 6 files changed, 12 insertions(+), 23 deletions(-)

-- 
2.33.0


^ permalink raw reply

* Re: [PATCH 16/39] irqdomain: Make normal and nomap irqdomains exclusive
From: Cédric Le Goater @ 2021-11-15 19:05 UTC (permalink / raw)
  To: Marc Zyngier, linux-kernel; +Cc: Thomas Gleixner, PowerPC, Greg Kurz
In-Reply-To: <20210520163751.27325-17-maz@kernel.org>

Hello Mark,

On 5/20/21 18:37, Marc Zyngier wrote:
> Direct mappings are completely exclusive of normal mappings, meaning
> that we can refactor the code slightly so that we can get rid of
> the revmap_direct_max_irq field and use the revmap_size field
> instead, reducing the size of the irqdomain structure.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>


This patch is breaking the POWER9/POWER10 XIVE driver (these are not
old PPC systems :) on machines sharing the same LSI HW IRQ. For instance,
a linux KVM guest with a virtio-rng and a virtio-balloon device. In that
case, Linux creates two distinct IRQ mappings which can lead to some
unexpected behavior.

A fix to go forward would be to change the XIVE IRQ domain to use a
'Tree' domain for reverse mapping and not the 'No Map' domain mapping.
I will keep you updated for XIVE.

Thanks,

C.


> ---
>   include/linux/irqdomain.h |  6 +++---
>   kernel/irq/irqdomain.c    | 45 ++++++++++++++++++++++++++++++---------
>   2 files changed, 38 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 723495ec5a2f..0916cf9c6e20 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -149,8 +149,6 @@ struct irq_domain_chip_generic;
>    * @parent: Pointer to parent irq_domain to support hierarchy irq_domains
>    *
>    * Revmap data, used internally by irq_domain
> - * @revmap_direct_max_irq: The largest hwirq that can be set for controllers that
> - *                         support direct mapping
>    * @revmap_size: Size of the linear map table @revmap[]
>    * @revmap_tree: Radix map tree for hwirqs that don't fit in the linear map
>    * @revmap: Linear table of hwirq->virq reverse mappings
> @@ -173,7 +171,6 @@ struct irq_domain {
>   
>   	/* reverse map data. The linear map gets appended to the irq_domain */
>   	irq_hw_number_t hwirq_max;
> -	unsigned int revmap_direct_max_irq;
>   	unsigned int revmap_size;
>   	struct radix_tree_root revmap_tree;
>   	struct mutex revmap_tree_mutex;
> @@ -207,6 +204,9 @@ enum {
>   	 */
>   	IRQ_DOMAIN_MSI_NOMASK_QUIRK	= (1 << 6),
>   
> +	/* Irq domain doesn't translate anything */
> +	IRQ_DOMAIN_FLAG_NO_MAP		= (1 << 7),
> +
>   	/*
>   	 * Flags starting from IRQ_DOMAIN_FLAG_NONCORE are reserved
>   	 * for implementation specific purposes and ignored by the
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index e0143e640683..fa94c86e47d4 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -146,6 +146,10 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
>   
>   	static atomic_t unknown_domains;
>   
> +	if (WARN_ON((size && direct_max) ||
> +		    (!IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) && direct_max)))
> +		return NULL;
> +
>   	domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
>   			      GFP_KERNEL, of_node_to_nid(to_of_node(fwnode)));
>   	if (!domain)
> @@ -213,8 +217,14 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
>   	domain->ops = ops;
>   	domain->host_data = host_data;
>   	domain->hwirq_max = hwirq_max;
> +
> +	if (direct_max) {
> +		size = direct_max;
> +		domain->flags |= IRQ_DOMAIN_FLAG_NO_MAP;
> +	}
> +
>   	domain->revmap_size = size;
> -	domain->revmap_direct_max_irq = direct_max;
> +
>   	irq_domain_check_hierarchy(domain);
>   
>   	mutex_lock(&irq_domain_mutex);
> @@ -482,9 +492,18 @@ struct irq_domain *irq_get_default_host(void)
>   	return irq_default_domain;
>   }
>   
> +static bool irq_domain_is_nomap(struct irq_domain *domain)
> +{
> +	return IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) &&
> +	       (domain->flags & IRQ_DOMAIN_FLAG_NO_MAP);
> +}
> +
>   static void irq_domain_clear_mapping(struct irq_domain *domain,
>   				     irq_hw_number_t hwirq)
>   {
> +	if (irq_domain_is_nomap(domain))
> +		return;
> +
>   	if (hwirq < domain->revmap_size) {
>   		domain->revmap[hwirq] = 0;
>   	} else {
> @@ -498,6 +517,9 @@ static void irq_domain_set_mapping(struct irq_domain *domain,
>   				   irq_hw_number_t hwirq,
>   				   struct irq_data *irq_data)
>   {
> +	if (irq_domain_is_nomap(domain))
> +		return;
> +
>   	if (hwirq < domain->revmap_size) {
>   		domain->revmap[hwirq] = irq_data->irq;
>   	} else {
> @@ -629,9 +651,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
>   		pr_debug("create_direct virq allocation failed\n");
>   		return 0;
>   	}
> -	if (virq >= domain->revmap_direct_max_irq) {
> +	if (virq >= domain->revmap_size) {
>   		pr_err("ERROR: no free irqs available below %i maximum\n",
> -			domain->revmap_direct_max_irq);
> +			domain->revmap_size);
>   		irq_free_desc(virq);
>   		return 0;
>   	}
> @@ -879,10 +901,14 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>   	if (domain == NULL)
>   		return 0;
>   
> -	if (hwirq < domain->revmap_direct_max_irq) {
> -		data = irq_domain_get_irq_data(domain, hwirq);
> -		if (data && data->hwirq == hwirq)
> -			return hwirq;
> +	if (irq_domain_is_nomap(domain)) {
> +		if (hwirq < domain->revmap_size) {
> +			data = irq_domain_get_irq_data(domain, hwirq);
> +			if (data && data->hwirq == hwirq)
> +				return hwirq;
> +		}
> +
> +		return 0;
>   	}
>   
>   	/* Check if the hwirq is in the linear revmap. */
> @@ -1470,7 +1496,7 @@ static void irq_domain_fix_revmap(struct irq_data *d)
>   {
>   	void __rcu **slot;
>   
> -	if (d->hwirq < d->domain->revmap_size)
> +	if (irq_domain_is_nomap(d->domain) || d->hwirq < d->domain->revmap_size)
>   		return; /* Not using radix tree. */
>   
>   	/* Fix up the revmap. */
> @@ -1830,8 +1856,7 @@ static void
>   irq_domain_debug_show_one(struct seq_file *m, struct irq_domain *d, int ind)
>   {
>   	seq_printf(m, "%*sname:   %s\n", ind, "", d->name);
> -	seq_printf(m, "%*ssize:   %u\n", ind + 1, "",
> -		   d->revmap_size + d->revmap_direct_max_irq);
> +	seq_printf(m, "%*ssize:   %u\n", ind + 1, "", d->revmap_size);
>   	seq_printf(m, "%*smapped: %u\n", ind + 1, "", d->mapcount);
>   	seq_printf(m, "%*sflags:  0x%08x\n", ind +1 , "", d->flags);
>   	if (d->ops && d->ops->debug_show)
> 


^ permalink raw reply

* Re: Build regressions/improvements in v5.16-rc1
From: Helge Deller @ 2021-11-15 16:44 UTC (permalink / raw)
  To: Geert Uytterhoeven, Linux Kernel Mailing List
  Cc: Peter Zijlstra, linux-pci, DRI Development, James E.J. Bottomley,
	Andrey Ryabinin, Stan Skowronek, Herbert Xu, kasan-dev,
	Sergio Paracuellos, Anton Altaparmakov, linux-arm-msm,
	open list:GPIO SUBSYSTEM, Nick Terrell, Joey Gouly,
	André Almeida, Linux ARM, Thomas Bogendoerfer, Parisc List,
	linux-ntfs-dev, Hector Martin, open list:BROADCOM NVRAM DRIVER,
	Rob Clark, Linux Crypto Mailing List, linuxppc-dev
In-Reply-To: <CAMuHMdUCsyUxaEf1Lz7+jMnur4ECwK+JoXQqmOCkRKqXdb1hTQ@mail.gmail.com>

On 11/15/21 17:12, Geert Uytterhoeven wrote:
> On Mon, Nov 15, 2021 at 4:54 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> Below is the list of build error/warning regressions/improvements in
>> v5.16-rc1[1] compared to v5.15[2].
>>
>> Summarized:
>>   - build errors: +20/-13
>>   - build warnings: +3/-28
>>
>> Happy fixing! ;-)
>>
>> Thanks to the linux-next team for providing the build service.
>>
>> [1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf/ (all 90 configs)
>> [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/8bb7eca972ad531c9b149c0a51ab43a417385813/ (all 90 configs)
>>
>>
>> *** ERRORS ***
>>
>> 20 error regressions:
>>   + /kisskb/src/arch/parisc/include/asm/jump_label.h: error: expected ':' before '__stringify':  => 33:4, 18:4
>>   + /kisskb/src/arch/parisc/include/asm/jump_label.h: error: label 'l_yes' defined but not used [-Werror=unused-label]:  => 38:1, 23:1
>
>     due to static_branch_likely() in crypto/api.c
>
> parisc-allmodconfig

fixed now in the parisc for-next git tree.


>>   + /kisskb/src/drivers/gpu/drm/msm/msm_drv.h: error: "COND" redefined [-Werror]:  => 531
>>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 3252 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 47:1
>>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 3360 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 499:1
>>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 5344 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 334:1
>>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 5380 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 354:1
>>   + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 1824 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 372:1
>>   + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 2224 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 204:1
>>   + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 3800 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 476:1
>
> parisc-allmodconfig

parisc needs much bigger frame sizes, so I'm not astonished here.
During the v5.15 cycl I increased it to 1536 (from 1280), so I'm simply tempted to
increase it this time to 4096, unless someone has a better idea....

>>   + /kisskb/src/fs/ntfs/aops.c: error: the frame size of 2240 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]:  => 1311:1
>>   + /kisskb/src/fs/ntfs/aops.c: error: the frame size of 2304 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]:  => 1311:1
>>   + /kisskb/src/fs/ntfs/aops.c: error: the frame size of 2320 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]:  => 1311:1
>
> powerpc-allmodconfig
>
>>   + /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_366' declared with attribute error: FIELD_PREP: value too large for the field:  => 335:38
>
>     in drivers/pinctrl/pinctrl-apple-gpio.c
>
> arm64-allmodconfig (gcc8)
>
>>   + /kisskb/src/include/linux/fortify-string.h: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter):  => 263:25, 277:17
>
>     in lib/test_kasan.c
>
> s390-all{mod,yes}config
> arm64-allmodconfig (gcc11)
>
>>   + error: modpost: "mips_cm_is64" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>>   + error: modpost: "mips_cm_lock_other" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>>   + error: modpost: "mips_cm_unlock_other" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>>   + error: modpost: "mips_cpc_base" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>>   + error: modpost: "mips_gcr_base" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>
> mips-allmodconfig
>
>> 3 warning regressions:
>>   + <stdin>: warning: #warning syscall futex_waitv not implemented [-Wcpp]:  => 1559:2
>
> powerpc, m68k, mips, s390, parisc (and probably more)

Will someone update all of them at once?




Helge


>>   + arch/m68k/configs/multi_defconfig: warning: symbol value 'm' invalid for MCTP:  => 322
>>   + arch/m68k/configs/sun3_defconfig: warning: symbol value 'm' invalid for MCTP:  => 295
>
> Yeah, that happens when symbols are changed from tristate to bool...
> Will be fixed in 5.17-rc1, with the next defconfig refresh.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
>


^ permalink raw reply

* Re: Build regressions/improvements in v5.16-rc1
From: Marco Elver @ 2021-11-15 16:44 UTC (permalink / raw)
  To: Kees Cook, Geert Uytterhoeven
  Cc: Peter Zijlstra, linux-pci, DRI Development,
	open list:BROADCOM NVRAM DRIVER, James E.J. Bottomley,
	Andrey Ryabinin, Stan Skowronek, Herbert Xu, Helge Deller,
	kasan-dev, Sergio Paracuellos, Anton Altaparmakov, linux-arm-msm,
	open list:GPIO SUBSYSTEM, Nick Terrell, Joey Gouly,
	André Almeida, Linux ARM, Thomas Bogendoerfer, Parisc List,
	linux-ntfs-dev, Hector Martin, Linux Kernel Mailing List,
	Rob Clark, Linux Crypto Mailing List, linuxppc-dev
In-Reply-To: <CAMuHMdUCsyUxaEf1Lz7+jMnur4ECwK+JoXQqmOCkRKqXdb1hTQ@mail.gmail.com>

On Mon, Nov 15, 2021 at 05:12PM +0100, Geert Uytterhoeven wrote:
[...]
> >   + /kisskb/src/include/linux/fortify-string.h: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter):  => 263:25, 277:17
> 
>     in lib/test_kasan.c
> 
> s390-all{mod,yes}config
> arm64-allmodconfig (gcc11)

Kees, wasn't that what [1] was meant to fix?
[1] https://lkml.kernel.org/r/20211006181544.1670992-1-keescook@chromium.org

Thanks,
-- Marco

^ permalink raw reply

* Re: Build regressions/improvements in v5.16-rc1
From: Geert Uytterhoeven @ 2021-11-15 16:12 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: Peter Zijlstra, linux-pci, DRI Development, James E.J. Bottomley,
	Andrey Ryabinin, Stan Skowronek, Herbert Xu, Helge Deller,
	kasan-dev, Sergio Paracuellos, Anton Altaparmakov, linux-arm-msm,
	open list:GPIO SUBSYSTEM, Nick Terrell, Joey Gouly,
	André Almeida, Linux ARM, Thomas Bogendoerfer, Parisc List,
	linux-ntfs-dev, Hector Martin, open list:BROADCOM NVRAM DRIVER,
	Rob Clark, Linux Crypto Mailing List, linuxppc-dev
In-Reply-To: <20211115155105.3797527-1-geert@linux-m68k.org>

On Mon, Nov 15, 2021 at 4:54 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Below is the list of build error/warning regressions/improvements in
> v5.16-rc1[1] compared to v5.15[2].
>
> Summarized:
>   - build errors: +20/-13
>   - build warnings: +3/-28
>
> Happy fixing! ;-)
>
> Thanks to the linux-next team for providing the build service.
>
> [1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf/ (all 90 configs)
> [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/8bb7eca972ad531c9b149c0a51ab43a417385813/ (all 90 configs)
>
>
> *** ERRORS ***
>
> 20 error regressions:
>   + /kisskb/src/arch/parisc/include/asm/jump_label.h: error: expected ':' before '__stringify':  => 33:4, 18:4
>   + /kisskb/src/arch/parisc/include/asm/jump_label.h: error: label 'l_yes' defined but not used [-Werror=unused-label]:  => 38:1, 23:1

    due to static_branch_likely() in crypto/api.c

parisc-allmodconfig

>   + /kisskb/src/drivers/gpu/drm/msm/msm_drv.h: error: "COND" redefined [-Werror]:  => 531
>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 3252 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 47:1
>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 3360 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 499:1
>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 5344 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 334:1
>   + /kisskb/src/lib/zstd/compress/zstd_double_fast.c: error: the frame size of 5380 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 354:1
>   + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 1824 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 372:1
>   + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 2224 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 204:1
>   + /kisskb/src/lib/zstd/compress/zstd_fast.c: error: the frame size of 3800 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]:  => 476:1

parisc-allmodconfig

>   + /kisskb/src/fs/ntfs/aops.c: error: the frame size of 2240 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]:  => 1311:1
>   + /kisskb/src/fs/ntfs/aops.c: error: the frame size of 2304 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]:  => 1311:1
>   + /kisskb/src/fs/ntfs/aops.c: error: the frame size of 2320 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]:  => 1311:1

powerpc-allmodconfig

>   + /kisskb/src/include/linux/compiler_types.h: error: call to '__compiletime_assert_366' declared with attribute error: FIELD_PREP: value too large for the field:  => 335:38

    in drivers/pinctrl/pinctrl-apple-gpio.c

arm64-allmodconfig (gcc8)

>   + /kisskb/src/include/linux/fortify-string.h: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter):  => 263:25, 277:17

    in lib/test_kasan.c

s390-all{mod,yes}config
arm64-allmodconfig (gcc11)

>   + error: modpost: "mips_cm_is64" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>   + error: modpost: "mips_cm_lock_other" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>   + error: modpost: "mips_cm_unlock_other" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>   + error: modpost: "mips_cpc_base" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A
>   + error: modpost: "mips_gcr_base" [drivers/pci/controller/pcie-mt7621.ko] undefined!:  => N/A

mips-allmodconfig

> 3 warning regressions:
>   + <stdin>: warning: #warning syscall futex_waitv not implemented [-Wcpp]:  => 1559:2

powerpc, m68k, mips, s390, parisc (and probably more)

>   + arch/m68k/configs/multi_defconfig: warning: symbol value 'm' invalid for MCTP:  => 322
>   + arch/m68k/configs/sun3_defconfig: warning: symbol value 'm' invalid for MCTP:  => 295

Yeah, that happens when symbols are changed from tristate to bool...
Will be fixed in 5.17-rc1, with the next defconfig refresh.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 3/5] powerpc: Use preemption model accessors
From: Valentin Schneider @ 2021-11-15 15:29 UTC (permalink / raw)
  To: Michael Ellerman, linux-kernel, kasan-dev, linuxppc-dev,
	linux-kbuild
  Cc: Marco Elver, Michal Marek, Peter Zijlstra, Frederic Weisbecker,
	Mike Galbraith, Nick Desaulniers, Steven Rostedt, Paul Mackerras,
	Masahiro Yamada, Ingo Molnar, Dmitry Vyukov
In-Reply-To: <87o86rmgu8.fsf@mpe.ellerman.id.au>


Doh, thought I had sent this one out already...

On 11/11/21 15:55, Michael Ellerman wrote:
> Valentin Schneider <valentin.schneider@arm.com> writes:
>> Per PREEMPT_DYNAMIC, checking CONFIG_PREEMPT doesn't tell you the actual
>> preemption model of the live kernel. Use the newly-introduced accessors
>> instead.
>>
>> sched_init() -> preempt_dynamic_init() happens way before IRQs are set up,
>> so this should be fine.
>
> Despite the name interrupt_exit_kernel_prepare() is called before IRQs
> are setup, traps and page faults are "interrupts" here.
>
> So I'm not sure about adding that call there, because it will trigger a
> WARN if called early in boot, which will trigger a trap and depending on
> the context we may not survive.
>
> I'd be happier if we can make it a build-time check.
>

This can't be done at build-time for PREEMPT_DYNAMIC, but that can be
punted off to whoever will implement ppc support for that :-) AFAICT if
this can't use preempt_dynamic_mode (due to how "late" it is setup), the
preempt_schedule_irq() needs to go and ppc needs to use irqentry_exit() /
irqentry_exit_cond_resched().

I dropped that for v2.

> cheers
>

^ permalink raw reply

* Re: [PATCH v3 1/4] powerpc/watchdog: Fix missed watchdog reset due to memory ordering race
From: Laurent Dufour @ 2021-11-15 15:09 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20211110025056.2084347-2-npiggin@gmail.com>

Le 10/11/2021 à 03:50, Nicholas Piggin a écrit :
> It is possible for all CPUs to miss the pending cpumask becoming clear,
> and then nobody resetting it, which will cause the lockup detector to
> stop working. It will eventually expire, but watchdog_smp_panic will
> avoid doing anything if the pending mask is clear and it will never be
> reset.
> 
> Order the cpumask clear vs the subsequent test to close this race.
> 
> Add an extra check for an empty pending mask when the watchdog fires and
> finds its bit still clear, to try to catch any other possible races or
> bugs here and keep the watchdog working. The extra test in
> arch_touch_nmi_watchdog is required to prevent the new warning from
> firing off.
> 
> Debugged-by: Laurent Dufour <ldufour@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/kernel/watchdog.c | 41 +++++++++++++++++++++++++++++++++-
>   1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
> index f9ea0e5357f9..3c60872b6a2c 100644
> --- a/arch/powerpc/kernel/watchdog.c
> +++ b/arch/powerpc/kernel/watchdog.c
> @@ -135,6 +135,10 @@ static void set_cpumask_stuck(const struct cpumask *cpumask, u64 tb)
>   {
>   	cpumask_or(&wd_smp_cpus_stuck, &wd_smp_cpus_stuck, cpumask);
>   	cpumask_andnot(&wd_smp_cpus_pending, &wd_smp_cpus_pending, cpumask);
> +	/*
> +	 * See wd_smp_clear_cpu_pending()
> +	 */
> +	smp_mb();
>   	if (cpumask_empty(&wd_smp_cpus_pending)) {
>   		wd_smp_last_reset_tb = tb;
>   		cpumask_andnot(&wd_smp_cpus_pending,
> @@ -215,13 +219,44 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
>   
>   			cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);
>   			wd_smp_unlock(&flags);
> +		} else {
> +			/*
> +			 * The last CPU to clear pending should have reset the
> +			 * watchdog so we generally should not find it empty
> +			 * here if our CPU was clear. However it could happen
> +			 * due to a rare race with another CPU taking the
> +			 * last CPU out of the mask concurrently.
> +			 *
> +			 * We can't add a warning for it. But just in case
> +			 * there is a problem with the watchdog that is causing
> +			 * the mask to not be reset, try to kick it along here.
> +			 */
> +			if (unlikely(cpumask_empty(&wd_smp_cpus_pending)))
> +				goto none_pending;

If I understand correctly, that branch is a security in case the code is not 
working as expected. But I'm really wondering if that's really needed, and we 
will end up with a contention on the watchdog lock while this path should be 
lockless, and I'd say that in most of the case there is nothing to do after 
grabbing that lock. Am I missing something risky here?

>   		}
>   		return;
>   	}
> +
>   	cpumask_clear_cpu(cpu, &wd_smp_cpus_pending);
> +
> +	/*
> +	 * Order the store to clear pending with the load(s) to check all
> +	 * words in the pending mask to check they are all empty. This orders
> +	 * with the same barrier on another CPU. This prevents two CPUs
> +	 * clearing the last 2 pending bits, but neither seeing the other's
> +	 * store when checking if the mask is empty, and missing an empty
> +	 * mask, which ends with a false positive.
> +	 */
> +	smp_mb();
>   	if (cpumask_empty(&wd_smp_cpus_pending)) {
>   		unsigned long flags;
>   
> +none_pending:
> +		/*
> +		 * Double check under lock because more than one CPU could see
> +		 * a clear mask with the lockless check after clearing their
> +		 * pending bits.
> +		 */
>   		wd_smp_lock(&flags);
>   		if (cpumask_empty(&wd_smp_cpus_pending)) {
>   			wd_smp_last_reset_tb = tb;
> @@ -312,8 +347,12 @@ void arch_touch_nmi_watchdog(void)
>   {
>   	unsigned long ticks = tb_ticks_per_usec * wd_timer_period_ms * 1000;
>   	int cpu = smp_processor_id();
> -	u64 tb = get_tb();
> +	u64 tb;
>   
> +	if (!cpumask_test_cpu(cpu, &watchdog_cpumask))
> +		return;
> +
> +	tb = get_tb();
>   	if (tb - per_cpu(wd_timer_tb, cpu) >= ticks) {
>   		per_cpu(wd_timer_tb, cpu) = tb;
>   		wd_smp_clear_cpu_pending(cpu, tb);
> 


^ permalink raw reply

* Re: [PATCH v2] powerpc/64s: Get LPID bit width from device tree
From: kernel test robot @ 2021-11-15 14:43 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: kbuild-all, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20211110080004.2148773-1-npiggin@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1975 bytes --]

Hi Nicholas,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.16-rc1 next-20211115]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-Get-LPID-bit-width-from-device-tree/20211110-160038
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-c024-20211115 (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/b8fe7181506bf3011295456a9ee471ddf49d694d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nicholas-Piggin/powerpc-64s-Get-LPID-bit-width-from-device-tree/20211110-160038
        git checkout b8fe7181506bf3011295456a9ee471ddf49d694d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/powerpc/mm/init_64.c: In function 'mmu_early_init_devtree':
>> arch/powerpc/mm/init_64.c:472:13: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]
     472 |         int rc;
         |             ^~
   cc1: all warnings being treated as errors


vim +/rc +472 arch/powerpc/mm/init_64.c

   469	
   470	void __init mmu_early_init_devtree(void)
   471	{
 > 472		int rc;

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27282 bytes --]

^ permalink raw reply

* Re: [PATCH] net/wan/fsl_ucc_hdlc: fix sparse warnings
From: patchwork-bot+netdevbpf @ 2021-11-15 14:30 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: lkp, netdev, linux, linux-kernel, kuba, linuxppc-dev, davem,
	qiang.zhao
In-Reply-To: <28f87309a3bb26e91d93e808a3b0e43baf79cc3b.1636974508.git.christophe.leroy@csgroup.eu>

Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 15 Nov 2021 12:08:59 +0100 you wrote:
> CHECK   drivers/net/wan/fsl_ucc_hdlc.c
> drivers/net/wan/fsl_ucc_hdlc.c:309:57: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:309:57:    expected void [noderef] __iomem *
> drivers/net/wan/fsl_ucc_hdlc.c:309:57:    got restricted __be16 *
> drivers/net/wan/fsl_ucc_hdlc.c:311:46: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:311:46:    expected void [noderef] __iomem *
> drivers/net/wan/fsl_ucc_hdlc.c:311:46:    got restricted __be32 *
> drivers/net/wan/fsl_ucc_hdlc.c:320:57: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:320:57:    expected void [noderef] __iomem *
> drivers/net/wan/fsl_ucc_hdlc.c:320:57:    got restricted __be16 *
> drivers/net/wan/fsl_ucc_hdlc.c:322:46: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:322:46:    expected void [noderef] __iomem *
> drivers/net/wan/fsl_ucc_hdlc.c:322:46:    got restricted __be32 *
> drivers/net/wan/fsl_ucc_hdlc.c:372:29: warning: incorrect type in assignment (different base types)
> drivers/net/wan/fsl_ucc_hdlc.c:372:29:    expected unsigned short [usertype]
> drivers/net/wan/fsl_ucc_hdlc.c:372:29:    got restricted __be16 [usertype]
> drivers/net/wan/fsl_ucc_hdlc.c:379:36: warning: restricted __be16 degrades to integer
> drivers/net/wan/fsl_ucc_hdlc.c:402:12: warning: incorrect type in assignment (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:402:12:    expected struct qe_bd [noderef] __iomem *bd
> drivers/net/wan/fsl_ucc_hdlc.c:402:12:    got struct qe_bd *curtx_bd
> drivers/net/wan/fsl_ucc_hdlc.c:425:20: warning: incorrect type in assignment (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:425:20:    expected struct qe_bd [noderef] __iomem *[assigned] bd
> drivers/net/wan/fsl_ucc_hdlc.c:425:20:    got struct qe_bd *tx_bd_base
> drivers/net/wan/fsl_ucc_hdlc.c:427:16: error: incompatible types in comparison expression (different address spaces):
> drivers/net/wan/fsl_ucc_hdlc.c:427:16:    struct qe_bd [noderef] __iomem *
> drivers/net/wan/fsl_ucc_hdlc.c:427:16:    struct qe_bd *
> drivers/net/wan/fsl_ucc_hdlc.c:462:33: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:506:41: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:528:33: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:552:38: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:596:67: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:611:41: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:851:38: warning: incorrect type in initializer (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:854:40: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:855:40: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:858:39: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:861:37: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:866:38: warning: incorrect type in initializer (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:868:21: warning: incorrect type in argument 1 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:870:40: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:871:40: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:873:39: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:993:57: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:995:46: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:1004:57: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:1006:46: warning: incorrect type in argument 2 (different address spaces)
> drivers/net/wan/fsl_ucc_hdlc.c:412:35: warning: dereference of noderef expression
> drivers/net/wan/fsl_ucc_hdlc.c:412:35: warning: dereference of noderef expression
> drivers/net/wan/fsl_ucc_hdlc.c:724:29: warning: dereference of noderef expression
> drivers/net/wan/fsl_ucc_hdlc.c:815:21: warning: dereference of noderef expression
> drivers/net/wan/fsl_ucc_hdlc.c:1021:29: warning: dereference of noderef expression
> 
> [...]

Here is the summary with links:
  - net/wan/fsl_ucc_hdlc: fix sparse warnings
    https://git.kernel.org/netdev/net-next/c/5cf46d8e741f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* Re: [PATCH v2 3/3] soc: fsl: Replace kernel.h with the necessary inclusions
From: Andy Shevchenko @ 2021-11-15 11:30 UTC (permalink / raw)
  To: linux-kernel, linuxppc-dev, linux-arm-kernel; +Cc: Qiang Zhao, Li Yang
In-Reply-To: <20211110105952.62013-3-andriy.shevchenko@linux.intel.com>

On Wed, Nov 10, 2021 at 12:59:52PM +0200, Andy Shevchenko wrote:
> When kernel.h is used in the headers it adds a lot into dependency hell,
> especially when there are circular dependencies are involved.
> 
> Replace kernel.h inclusion with the list of what is really being used.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: updated Cc list based on previous changes to MAINTAINERS

Any comments on this, please?

I really want to decrease amount of kernel.h usage in the common headers.
So others won't copy'n'paste bad example.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH] powerpc/book3e: Fix TLBCAM preset at boot
From: Christian Zigotzky @ 2021-11-15 11:15 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman
  Cc: Darren Stevens, R.T.Dickinson, linux-kernel, mad skateman,
	Matthew Leaman, linuxppc-dev
In-Reply-To: <d319f2a9367d4d08fd2154e506101bd5f100feeb.1636967119.git.christophe.leroy@csgroup.eu>

On 15 November 2021 at 10:05 am, Christophe Leroy wrote:
> Commit 52bda69ae8b5 ("powerpc/fsl_booke: Tell map_mem_in_cams() if
> init is done") was supposed to just add an additional parameter to
> map_mem_in_cams() and always set it to 'true' at that time.
>
> But a few call sites were messed up. Fix them.
>
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Fixes: 52bda69ae8b5 ("powerpc/fsl_booke: Tell map_mem_in_cams() if init is done")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>   arch/powerpc/mm/nohash/kaslr_booke.c | 2 +-
>   arch/powerpc/mm/nohash/tlb.c         | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c
> index 8fc49b1b4a91..6ec978967da0 100644
> --- a/arch/powerpc/mm/nohash/kaslr_booke.c
> +++ b/arch/powerpc/mm/nohash/kaslr_booke.c
> @@ -314,7 +314,7 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
>   		pr_warn("KASLR: No safe seed for randomizing the kernel base.\n");
>   
>   	ram = min_t(phys_addr_t, __max_low_memory, size);
> -	ram = map_mem_in_cams(ram, CONFIG_LOWMEM_CAM_NUM, true, false);
> +	ram = map_mem_in_cams(ram, CONFIG_LOWMEM_CAM_NUM, true, true);
>   	linear_sz = min_t(unsigned long, ram, SZ_512M);
>   
>   	/* If the linear size is smaller than 64M, do not randmize */
> diff --git a/arch/powerpc/mm/nohash/tlb.c b/arch/powerpc/mm/nohash/tlb.c
> index 89353d4f5604..647bf454a0fa 100644
> --- a/arch/powerpc/mm/nohash/tlb.c
> +++ b/arch/powerpc/mm/nohash/tlb.c
> @@ -645,7 +645,7 @@ static void early_init_this_mmu(void)
>   
>   		if (map)
>   			linear_map_top = map_mem_in_cams(linear_map_top,
> -							 num_cams, true, true);
> +							 num_cams, false, true);
>   	}
>   #endif
>   
> @@ -766,7 +766,7 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
>   		num_cams = (mfspr(SPRN_TLB1CFG) & TLBnCFG_N_ENTRY) / 4;
>   
>   		linear_sz = map_mem_in_cams(first_memblock_size, num_cams,
> -					    false, true);
> +					    true, true);
>   
>   		ppc64_rma_size = min_t(u64, linear_sz, 0x40000000);
>   	} else
Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>

Thanks

^ permalink raw reply

* [PATCH] net/wan/fsl_ucc_hdlc: fix sparse warnings
From: Christophe Leroy @ 2021-11-15 11:08 UTC (permalink / raw)
  To: Zhao Qiang, David S. Miller, Jakub Kicinski
  Cc: kernel test robot, netdev, Rasmus Villemoes, linux-kernel,
	linuxppc-dev

  CHECK   drivers/net/wan/fsl_ucc_hdlc.c
drivers/net/wan/fsl_ucc_hdlc.c:309:57: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:309:57:    expected void [noderef] __iomem *
drivers/net/wan/fsl_ucc_hdlc.c:309:57:    got restricted __be16 *
drivers/net/wan/fsl_ucc_hdlc.c:311:46: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:311:46:    expected void [noderef] __iomem *
drivers/net/wan/fsl_ucc_hdlc.c:311:46:    got restricted __be32 *
drivers/net/wan/fsl_ucc_hdlc.c:320:57: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:320:57:    expected void [noderef] __iomem *
drivers/net/wan/fsl_ucc_hdlc.c:320:57:    got restricted __be16 *
drivers/net/wan/fsl_ucc_hdlc.c:322:46: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:322:46:    expected void [noderef] __iomem *
drivers/net/wan/fsl_ucc_hdlc.c:322:46:    got restricted __be32 *
drivers/net/wan/fsl_ucc_hdlc.c:372:29: warning: incorrect type in assignment (different base types)
drivers/net/wan/fsl_ucc_hdlc.c:372:29:    expected unsigned short [usertype]
drivers/net/wan/fsl_ucc_hdlc.c:372:29:    got restricted __be16 [usertype]
drivers/net/wan/fsl_ucc_hdlc.c:379:36: warning: restricted __be16 degrades to integer
drivers/net/wan/fsl_ucc_hdlc.c:402:12: warning: incorrect type in assignment (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:402:12:    expected struct qe_bd [noderef] __iomem *bd
drivers/net/wan/fsl_ucc_hdlc.c:402:12:    got struct qe_bd *curtx_bd
drivers/net/wan/fsl_ucc_hdlc.c:425:20: warning: incorrect type in assignment (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:425:20:    expected struct qe_bd [noderef] __iomem *[assigned] bd
drivers/net/wan/fsl_ucc_hdlc.c:425:20:    got struct qe_bd *tx_bd_base
drivers/net/wan/fsl_ucc_hdlc.c:427:16: error: incompatible types in comparison expression (different address spaces):
drivers/net/wan/fsl_ucc_hdlc.c:427:16:    struct qe_bd [noderef] __iomem *
drivers/net/wan/fsl_ucc_hdlc.c:427:16:    struct qe_bd *
drivers/net/wan/fsl_ucc_hdlc.c:462:33: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:506:41: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:528:33: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:552:38: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:596:67: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:611:41: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:851:38: warning: incorrect type in initializer (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:854:40: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:855:40: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:858:39: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:861:37: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:866:38: warning: incorrect type in initializer (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:868:21: warning: incorrect type in argument 1 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:870:40: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:871:40: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:873:39: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:993:57: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:995:46: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:1004:57: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:1006:46: warning: incorrect type in argument 2 (different address spaces)
drivers/net/wan/fsl_ucc_hdlc.c:412:35: warning: dereference of noderef expression
drivers/net/wan/fsl_ucc_hdlc.c:412:35: warning: dereference of noderef expression
drivers/net/wan/fsl_ucc_hdlc.c:724:29: warning: dereference of noderef expression
drivers/net/wan/fsl_ucc_hdlc.c:815:21: warning: dereference of noderef expression
drivers/net/wan/fsl_ucc_hdlc.c:1021:29: warning: dereference of noderef expression

Most of the warnings are due to DMA memory being incorrectly handled as IO memory.
Fix it by doing direct read/write and doing proper dma_rmb() / dma_wmb().

Other problems are type mismatches or lack of use of IO accessors.

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lkml.org/lkml/2021/11/12/647
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 62 ++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index cda1b4ce6b21..5ae2d27b5da9 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -306,9 +306,8 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 		else
 			bd_status = R_E_S | R_I_S | R_W_S;
 
-		iowrite16be(bd_status, &priv->rx_bd_base[i].status);
-		iowrite32be(priv->dma_rx_addr + i * MAX_RX_BUF_LENGTH,
-			    &priv->rx_bd_base[i].buf);
+		priv->rx_bd_base[i].status = cpu_to_be16(bd_status);
+		priv->rx_bd_base[i].buf = cpu_to_be32(priv->dma_rx_addr + i * MAX_RX_BUF_LENGTH);
 	}
 
 	for (i = 0; i < TX_BD_RING_LEN; i++) {
@@ -317,10 +316,10 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
 		else
 			bd_status =  T_I_S | T_TC_S | T_W_S;
 
-		iowrite16be(bd_status, &priv->tx_bd_base[i].status);
-		iowrite32be(priv->dma_tx_addr + i * MAX_RX_BUF_LENGTH,
-			    &priv->tx_bd_base[i].buf);
+		priv->tx_bd_base[i].status = cpu_to_be16(bd_status);
+		priv->tx_bd_base[i].buf = cpu_to_be32(priv->dma_tx_addr + i * MAX_RX_BUF_LENGTH);
 	}
+	dma_wmb();
 
 	return 0;
 
@@ -352,10 +351,10 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 {
 	hdlc_device *hdlc = dev_to_hdlc(dev);
 	struct ucc_hdlc_private *priv = (struct ucc_hdlc_private *)hdlc->priv;
-	struct qe_bd __iomem *bd;
+	struct qe_bd *bd;
 	u16 bd_status;
 	unsigned long flags;
-	u16 *proto_head;
+	__be16 *proto_head;
 
 	switch (dev->type) {
 	case ARPHRD_RAWHDLC:
@@ -368,14 +367,14 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 
 		skb_push(skb, HDLC_HEAD_LEN);
 
-		proto_head = (u16 *)skb->data;
+		proto_head = (__be16 *)skb->data;
 		*proto_head = htons(DEFAULT_HDLC_HEAD);
 
 		dev->stats.tx_bytes += skb->len;
 		break;
 
 	case ARPHRD_PPP:
-		proto_head = (u16 *)skb->data;
+		proto_head = (__be16 *)skb->data;
 		if (*proto_head != htons(DEFAULT_PPP_HEAD)) {
 			dev->stats.tx_dropped++;
 			dev_kfree_skb(skb);
@@ -398,9 +397,10 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 	netdev_sent_queue(dev, skb->len);
 	spin_lock_irqsave(&priv->lock, flags);
 
+	dma_rmb();
 	/* Start from the next BD that should be filled */
 	bd = priv->curtx_bd;
-	bd_status = ioread16be(&bd->status);
+	bd_status = be16_to_cpu(bd->status);
 	/* Save the skb pointer so we can free it later */
 	priv->tx_skbuff[priv->skb_curtx] = skb;
 
@@ -415,8 +415,8 @@ static netdev_tx_t ucc_hdlc_tx(struct sk_buff *skb, struct net_device *dev)
 	/* set bd status and length */
 	bd_status = (bd_status & T_W_S) | T_R_S | T_I_S | T_L_S | T_TC_S;
 
-	iowrite16be(skb->len, &bd->length);
-	iowrite16be(bd_status, &bd->status);
+	bd->length = cpu_to_be16(skb->len);
+	bd->status = cpu_to_be16(bd_status);
 
 	/* Move to next BD in the ring */
 	if (!(bd_status & T_W_S))
@@ -458,8 +458,9 @@ static int hdlc_tx_done(struct ucc_hdlc_private *priv)
 	u16 bd_status;
 	int tx_restart = 0;
 
+	dma_rmb();
 	bd = priv->dirty_tx;
-	bd_status = ioread16be(&bd->status);
+	bd_status = be16_to_cpu(bd->status);
 
 	/* Normal processing. */
 	while ((bd_status & T_R_S) == 0) {
@@ -503,7 +504,7 @@ static int hdlc_tx_done(struct ucc_hdlc_private *priv)
 			bd += 1;
 		else
 			bd = priv->tx_bd_base;
-		bd_status = ioread16be(&bd->status);
+		bd_status = be16_to_cpu(bd->status);
 	}
 	priv->dirty_tx = bd;
 
@@ -524,8 +525,9 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
 	u16 length, howmany = 0;
 	u8 *bdbuffer;
 
+	dma_rmb();
 	bd = priv->currx_bd;
-	bd_status = ioread16be(&bd->status);
+	bd_status = be16_to_cpu(bd->status);
 
 	/* while there are received buffers and BD is full (~R_E) */
 	while (!((bd_status & (R_E_S)) || (--rx_work_limit < 0))) {
@@ -549,7 +551,7 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
 		}
 		bdbuffer = priv->rx_buffer +
 			(priv->currx_bdnum * MAX_RX_BUF_LENGTH);
-		length = ioread16be(&bd->length);
+		length = be16_to_cpu(bd->length);
 
 		switch (dev->type) {
 		case ARPHRD_RAWHDLC:
@@ -593,7 +595,7 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
 		netif_receive_skb(skb);
 
 recycle:
-		iowrite16be((bd_status & R_W_S) | R_E_S | R_I_S, &bd->status);
+		bd->status = cpu_to_be16((bd_status & R_W_S) | R_E_S | R_I_S);
 
 		/* update to point at the next bd */
 		if (bd_status & R_W_S) {
@@ -608,8 +610,9 @@ static int hdlc_rx_done(struct ucc_hdlc_private *priv, int rx_work_limit)
 			bd += 1;
 		}
 
-		bd_status = ioread16be(&bd->status);
+		bd_status = be16_to_cpu(bd->status);
 	}
+	dma_rmb();
 
 	priv->currx_bd = bd;
 	return howmany;
@@ -721,7 +724,7 @@ static int uhdlc_open(struct net_device *dev)
 
 		/* Enable the TDM port */
 		if (priv->tsa)
-			utdm->si_regs->siglmr1_h |= (0x1 << utdm->tdm_port);
+			qe_setbits_8(&utdm->si_regs->siglmr1_h, 0x1 << utdm->tdm_port);
 
 		priv->hdlc_busy = 1;
 		netif_device_attach(priv->ndev);
@@ -812,7 +815,7 @@ static int uhdlc_close(struct net_device *dev)
 		     (u8)QE_CR_PROTOCOL_UNSPECIFIED, 0);
 
 	if (priv->tsa)
-		utdm->si_regs->siglmr1_h &= ~(0x1 << utdm->tdm_port);
+		qe_clrbits_8(&utdm->si_regs->siglmr1_h, 0x1 << utdm->tdm_port);
 
 	ucc_fast_disable(priv->uccf, COMM_DIR_RX | COMM_DIR_TX);
 
@@ -848,7 +851,7 @@ static int ucc_hdlc_attach(struct net_device *dev, unsigned short encoding,
 #ifdef CONFIG_PM
 static void store_clk_config(struct ucc_hdlc_private *priv)
 {
-	struct qe_mux *qe_mux_reg = &qe_immr->qmx;
+	struct qe_mux __iomem *qe_mux_reg = &qe_immr->qmx;
 
 	/* store si clk */
 	priv->cmxsi1cr_h = ioread32be(&qe_mux_reg->cmxsi1cr_h);
@@ -863,7 +866,7 @@ static void store_clk_config(struct ucc_hdlc_private *priv)
 
 static void resume_clk_config(struct ucc_hdlc_private *priv)
 {
-	struct qe_mux *qe_mux_reg = &qe_immr->qmx;
+	struct qe_mux __iomem *qe_mux_reg = &qe_immr->qmx;
 
 	memcpy_toio(qe_mux_reg->cmxucr, priv->cmxucr, 4 * sizeof(u32));
 
@@ -990,9 +993,8 @@ static int uhdlc_resume(struct device *dev)
 		else
 			bd_status = R_E_S | R_I_S | R_W_S;
 
-		iowrite16be(bd_status, &priv->rx_bd_base[i].status);
-		iowrite32be(priv->dma_rx_addr + i * MAX_RX_BUF_LENGTH,
-			    &priv->rx_bd_base[i].buf);
+		priv->rx_bd_base[i].status = cpu_to_be16(bd_status);
+		priv->rx_bd_base[i].buf = cpu_to_be32(priv->dma_rx_addr + i * MAX_RX_BUF_LENGTH);
 	}
 
 	for (i = 0; i < TX_BD_RING_LEN; i++) {
@@ -1001,10 +1003,10 @@ static int uhdlc_resume(struct device *dev)
 		else
 			bd_status =  T_I_S | T_TC_S | T_W_S;
 
-		iowrite16be(bd_status, &priv->tx_bd_base[i].status);
-		iowrite32be(priv->dma_tx_addr + i * MAX_RX_BUF_LENGTH,
-			    &priv->tx_bd_base[i].buf);
+		priv->tx_bd_base[i].status = cpu_to_be16(bd_status);
+		priv->tx_bd_base[i].buf = cpu_to_be32(priv->dma_tx_addr + i * MAX_RX_BUF_LENGTH);
 	}
+	dma_wmb();
 
 	/* if hdlc is busy enable TX and RX */
 	if (priv->hdlc_busy == 1) {
@@ -1018,7 +1020,7 @@ static int uhdlc_resume(struct device *dev)
 
 		/* Enable the TDM port */
 		if (priv->tsa)
-			utdm->si_regs->siglmr1_h |= (0x1 << utdm->tdm_port);
+			qe_setbits_8(&utdm->si_regs->siglmr1_h, 0x1 << utdm->tdm_port);
 	}
 
 	napi_enable(&priv->napi);
-- 
2.31.1


^ permalink raw reply related

* [PATCH v3] powerpc/code-patching: Improve verification of patchability
From: Christophe Leroy @ 2021-11-15 10:12 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Michael Neuling, linuxppc-dev, linux-kernel

Today, patch_instruction() assumes that it is called exclusively on
valid addresses, and only checks that it is not called on an init
address after init section has been freed.

Improve verification by calling kernel_text_address() instead.

kernel_text_address() already includes a verification of
initmem release.

Cc: Michael Neuling <mikey@neuling.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v3:
- At the same time also remove init_mem_is_free as it was the last user (Was done in patch 3 in v2)
- Drop patch 2 (was merged via mm tree)
---
 arch/powerpc/include/asm/setup.h | 1 -
 arch/powerpc/lib/code-patching.c | 5 ++---
 arch/powerpc/mm/mem.c            | 2 --
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 6c1a7d217d1a..426a2d8d028f 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -9,7 +9,6 @@ extern void ppc_printk_progress(char *s, unsigned short hex);
 
 extern unsigned int rtas_data;
 extern unsigned long long memory_limit;
-extern bool init_mem_is_free;
 extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
 
 struct device_node;
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index c5ed98823835..5e2fe133639e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -190,10 +190,9 @@ static int do_patch_instruction(u32 *addr, struct ppc_inst instr)
 int patch_instruction(u32 *addr, struct ppc_inst instr)
 {
 	/* Make sure we aren't patching a freed init section */
-	if (init_mem_is_free && init_section_contains(addr, 4)) {
-		pr_debug("Skipping init section patching addr: 0x%px\n", addr);
+	if (!kernel_text_address((unsigned long)addr))
 		return 0;
-	}
+
 	return do_patch_instruction(addr, instr);
 }
 NOKPROBE_SYMBOL(patch_instruction);
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index bd5d91a31183..8e301cd8925b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -26,7 +26,6 @@
 #include <mm/mmu_decl.h>
 
 unsigned long long memory_limit;
-bool init_mem_is_free;
 
 unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)] __page_aligned_bss;
 EXPORT_SYMBOL(empty_zero_page);
@@ -312,7 +311,6 @@ void free_initmem(void)
 {
 	ppc_md.progress = ppc_printk_progress;
 	mark_initmem_nx();
-	init_mem_is_free = true;
 	free_initmem_default(POISON_FREE_INITMEM);
 }
 
-- 
2.31.1


^ permalink raw reply related

* Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to unrecoverable loop.
From: Thorsten Leemhuis @ 2021-11-15  8:36 UTC (permalink / raw)
  To: Joakim Tjernlund, linuxppc-dev@lists.ozlabs.org,
	Eugene_Bordenkircher@selinc.com, linux-usb@vger.kernel.org
  Cc: gregkh@linuxfoundation.org, balbi@kernel.org, leoyang.li@nxp.com
In-Reply-To: <6659a2c7fd9fffac766b8389244e5885ccbd38bd.camel@infinera.com>

Hi, this is your Linux kernel regression tracker speaking.

This looks stalled, as afaics nothing to get this regression fixed
happened since below mail. How can we things rolling again?

Eugene, were you able to look into the patch from Joakim?

Or did I miss anything and some progress to fix this was made elsewhere?
Please let me know if that's the case.

Ciao, Thorsten (carrying his Linux kernel regression tracker hat)

P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Unfortunately
therefore I sometimes will get things wrong or miss something important.
I hope that's not the case here; if you think it is, don't hesitate to
tell me about it in a public reply. That's in everyone's interest, as
what I wrote above might be misleading to everyone reading this; any
suggestion I gave they thus might sent someone reading this down the
wrong rabbit hole, which none of us wants.

P.P.S.: Feel free to ignore the following lines, they are only meant for
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/):

#regzbot poke

On 02.11.21 22:15, Joakim Tjernlund wrote:
> On Sat, 2021-10-30 at 14:20 +0000, Joakim Tjernlund wrote:
>> On Fri, 2021-10-29 at 17:14 +0000, Eugene Bordenkircher wrote:
>
>>> We've discovered a situation where the FSL udc driver (drivers/usb/gadget/udc/fsl_udc_core.c) will enter a loop iterating over the request queue, but the queue has been corrupted at some point so it loops infinitely.  I believe we have narrowed into the offending code, but we are in need of assistance trying to find an appropriate fix for the problem.  The identified code appears to be in all versions of the Linux kernel the driver exists in.
>>>
>>> The problem appears to be when handling a USB_REQ_GET_STATUS request.  The driver gets this request and then calls the ch9getstatus() function.  In this function, it starts a request by "borrowing" the per device status_req, filling it in, and then queuing it with a call to list_add_tail() to add the request to the endpoint queue.  Right before it exits the function however, it's calling ep0_prime_status(), which is filling out that same status_req structure and then queuing it with another call to list_add_tail() to add the request to the endpoint queue.  This adds two instances of the exact same LIST_HEAD to the endpoint queue, which breaks the list since the prev and next pointers end up pointing to the wrong things.  This ends up causing a hard loop the next time nuke() gets called, which happens on the next setup IRQ.
>>>
>>> I'm not sure what the appropriate fix to this problem is, mostly due to my lack of expertise in USB and this driver stack.  The code has been this way in the kernel for a very long time, which suggests that it has been working, unless USB_REQ_GET_STATUS requests are never made.  This further suggests that there is something else going on that I don't understand.  Deleting the call to ep0_prime_status() and the following ep0stall() call appears, on the surface, to get the device working again, but may have side effects that I'm not seeing.
>>>
>>> I'm hopeful someone in the community can help provide some information on what I may be missing or help come up with a solution to the problem.  A big thank you to anyone who would like to help out.
>>>
>>> Eugene
>>
>> Run into this to a while ago. Found the bug and a few more fixes.
>> This is against 4.19 so you may have to tweak them a bit.
>> Feel free to upstream them.
>>
>>  Jocke 
> 
> Curious, did my patches help? Good to known once we upgrade as well.
> 
>  Jocke

^ permalink raw reply

* [PATCH] powerpc/book3e: Fix TLBCAM preset at boot
From: Christophe Leroy @ 2021-11-15  9:05 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel, Christian Zigotzky

Commit 52bda69ae8b5 ("powerpc/fsl_booke: Tell map_mem_in_cams() if
init is done") was supposed to just add an additional parameter to
map_mem_in_cams() and always set it to 'true' at that time.

But a few call sites were messed up. Fix them.

Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Fixes: 52bda69ae8b5 ("powerpc/fsl_booke: Tell map_mem_in_cams() if init is done")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/mm/nohash/kaslr_booke.c | 2 +-
 arch/powerpc/mm/nohash/tlb.c         | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c
index 8fc49b1b4a91..6ec978967da0 100644
--- a/arch/powerpc/mm/nohash/kaslr_booke.c
+++ b/arch/powerpc/mm/nohash/kaslr_booke.c
@@ -314,7 +314,7 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
 		pr_warn("KASLR: No safe seed for randomizing the kernel base.\n");
 
 	ram = min_t(phys_addr_t, __max_low_memory, size);
-	ram = map_mem_in_cams(ram, CONFIG_LOWMEM_CAM_NUM, true, false);
+	ram = map_mem_in_cams(ram, CONFIG_LOWMEM_CAM_NUM, true, true);
 	linear_sz = min_t(unsigned long, ram, SZ_512M);
 
 	/* If the linear size is smaller than 64M, do not randmize */
diff --git a/arch/powerpc/mm/nohash/tlb.c b/arch/powerpc/mm/nohash/tlb.c
index 89353d4f5604..647bf454a0fa 100644
--- a/arch/powerpc/mm/nohash/tlb.c
+++ b/arch/powerpc/mm/nohash/tlb.c
@@ -645,7 +645,7 @@ static void early_init_this_mmu(void)
 
 		if (map)
 			linear_map_top = map_mem_in_cams(linear_map_top,
-							 num_cams, true, true);
+							 num_cams, false, true);
 	}
 #endif
 
@@ -766,7 +766,7 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
 		num_cams = (mfspr(SPRN_TLB1CFG) & TLBnCFG_N_ENTRY) / 4;
 
 		linear_sz = map_mem_in_cams(first_memblock_size, num_cams,
-					    false, true);
+					    true, true);
 
 		ppc64_rma_size = min_t(u64, linear_sz, 0x40000000);
 	} else
-- 
2.31.1


^ permalink raw reply related

* [PATCH] powerpc/signal32: Fix sigset_t copy
From: Christophe Leroy @ 2021-11-15  8:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Stan Johnson, Finn Thain, linux-kernel, stable,
	Christopher M . Riedl, linuxppc-dev

The conversion from __copy_from_user() to __get_user() by
commit d3ccc9781560 ("powerpc/signal: Use __get_user() to copy
sigset_t") introduced a regression in __get_user_sigset() for
powerpc/32. The bug was subsequently moved into
unsafe_get_user_sigset().

The bug is due to the copied 64 bit value being truncated to
32 bits while being assigned to dst->sig[0]

The regression was reported by users of the Xorg packages distributed in
Debian/powerpc --

    "The symptoms are that the fb screen goes blank, with the backlight
    remaining on and no errors logged in /var/log; wdm (or startx) run
    with no effect (I tried logging in in the blind, with no effect).
    And they are hard to kill, requiring 'kill -KILL ...'"

Fix the regression by copying each word of the sigset, not only the
first one.

__get_user_sigset() was tentatively optimised to copy 64 bits at once
in order to minimise KUAP unlock/lock impact, but the unsafe variant
doesn't suffer that, so it can just copy words.

Cc: Christopher M. Riedl <cmr@bluescreens.de>
Fixes: 887f3ceb51cd ("powerpc/signal32: Convert do_setcontext[_tm]() to user access block")
Cc: stable@vger.kernel.org
Reported-by: Finn Thain <fthain@linux-m68k.org>
Reported-and-tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/signal.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 1f07317964e4..618aeccdf691 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -25,8 +25,14 @@ static inline int __get_user_sigset(sigset_t *dst, const sigset_t __user *src)
 
 	return __get_user(dst->sig[0], (u64 __user *)&src->sig[0]);
 }
-#define unsafe_get_user_sigset(dst, src, label) \
-	unsafe_get_user((dst)->sig[0], (u64 __user *)&(src)->sig[0], label)
+#define unsafe_get_user_sigset(dst, src, label) do {			\
+	sigset_t *__dst = dst;						\
+	const sigset_t __user *__src = src;				\
+	int i;								\
+									\
+	for (i = 0; i < _NSIG_WORDS; i++)				\
+		unsafe_get_user(__dst->sig[i], &__src->sig[i], label);	\
+} while (0)
 
 #ifdef CONFIG_VSX
 extern unsigned long copy_vsx_to_user(void __user *to,
-- 
2.31.1


^ permalink raw reply related

* [PATCH] powerpc/8xx: Fix pinned TLBs with CONFIG_STRICT_KERNEL_RWX
From: Christophe Leroy @ 2021-11-15  8:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: stable, linuxppc-dev, linux-kernel

As spotted and explained in commit c12ab8dbc492 ("powerpc/8xx: Fix
Oops with STRICT_KERNEL_RWX without DEBUG_RODATA_TEST"), the selection
of STRICT_KERNEL_RWX without selecting DEBUG_RODATA_TEST has spotted
the lack of the DIRTY bit in the pinned kernel data TLBs.

This problem should have been detected a lot earlier if things had
been working as expected. But due to an incredible level of chance or
mishap, this went undetected because of a set of bugs: In fact the
DTLBs were not pinned, because instead of setting the reserve bit
in MD_CTR, it was set in MI_CTR that is the register for ITLBs.

But then, another huge bug was there: the physical address was
reset to 0 at the boundary between RO and RW areas, leading to the
same physical space being mapped at both 0xc0000000 and 0xc8000000.
This had by miracle no consequence until now because the entry was
not really pinned so it was overwritten soon enough to go undetected.

Of course, now that we really pin the DTLBs, it must be fixed as well.

Depends-on: c12ab8dbc492 ("powerpc/8xx: Fix Oops with STRICT_KERNEL_RWX without DEBUG_RODATA_TEST")
Fixes: f76c8f6d257c ("powerpc/8xx: Add function to set pinned TLBs")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/head_8xx.S | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 2d596881b70e..0d073b9fd52c 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -733,6 +733,7 @@ _GLOBAL(mmu_pin_tlb)
 #ifdef CONFIG_PIN_TLB_DATA
 	LOAD_REG_IMMEDIATE(r6, PAGE_OFFSET)
 	LOAD_REG_IMMEDIATE(r7, MI_SVALID | MI_PS8MEG | _PMD_ACCESSED)
+	li	r8, 0
 #ifdef CONFIG_PIN_TLB_IMMR
 	li	r0, 3
 #else
@@ -741,26 +742,26 @@ _GLOBAL(mmu_pin_tlb)
 	mtctr	r0
 	cmpwi	r4, 0
 	beq	4f
-	LOAD_REG_IMMEDIATE(r8, 0xf0 | _PAGE_RO | _PAGE_SPS | _PAGE_SH | _PAGE_PRESENT)
 	LOAD_REG_ADDR(r9, _sinittext)
 
 2:	ori	r0, r6, MD_EVALID
+	ori	r12, r8, 0xf0 | _PAGE_RO | _PAGE_SPS | _PAGE_SH | _PAGE_PRESENT
 	mtspr	SPRN_MD_CTR, r5
 	mtspr	SPRN_MD_EPN, r0
 	mtspr	SPRN_MD_TWC, r7
-	mtspr	SPRN_MD_RPN, r8
+	mtspr	SPRN_MD_RPN, r12
 	addi	r5, r5, 0x100
 	addis	r6, r6, SZ_8M@h
 	addis	r8, r8, SZ_8M@h
 	cmplw	r6, r9
 	bdnzt	lt, 2b
-
-4:	LOAD_REG_IMMEDIATE(r8, 0xf0 | _PAGE_DIRTY | _PAGE_SPS | _PAGE_SH | _PAGE_PRESENT)
+4:
 2:	ori	r0, r6, MD_EVALID
+	ori	r12, r8, 0xf0 | _PAGE_DIRTY | _PAGE_SPS | _PAGE_SH | _PAGE_PRESENT
 	mtspr	SPRN_MD_CTR, r5
 	mtspr	SPRN_MD_EPN, r0
 	mtspr	SPRN_MD_TWC, r7
-	mtspr	SPRN_MD_RPN, r8
+	mtspr	SPRN_MD_RPN, r12
 	addi	r5, r5, 0x100
 	addis	r6, r6, SZ_8M@h
 	addis	r8, r8, SZ_8M@h
@@ -781,7 +782,7 @@ _GLOBAL(mmu_pin_tlb)
 #endif
 #if defined(CONFIG_PIN_TLB_IMMR) || defined(CONFIG_PIN_TLB_DATA)
 	lis	r0, (MD_RSV4I | MD_TWAM)@h
-	mtspr	SPRN_MI_CTR, r0
+	mtspr	SPRN_MD_CTR, r0
 #endif
 	mtspr	SPRN_SRR1, r10
 	mtspr	SPRN_SRR0, r11
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH] soc: fsl: qe: Fix typo in a comment
From: Christophe Leroy @ 2021-11-15  8:06 UTC (permalink / raw)
  To: Jason Wang, leoyang.li
  Cc: linuxppc-dev, linux-kernel, linux-arm-kernel, qiang.zhao
In-Reply-To: <20211113055900.212568-1-wangborong@cdjrlc.com>



Le 13/11/2021 à 06:59, Jason Wang a écrit :
> The double `is' in a comment is repeated, thus one of them
> should be removed.

You are also clean blanks it seems, so say it or don't do it.

> 
> Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
> ---
>   drivers/soc/fsl/qe/qe.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> index 4d38c80f8be8..b3c226eb5292 100644
> --- a/drivers/soc/fsl/qe/qe.c
> +++ b/drivers/soc/fsl/qe/qe.c
> @@ -147,7 +147,7 @@ EXPORT_SYMBOL(qe_issue_cmd);
>    * memory mapped space.
>    * The BRG clock is the QE clock divided by 2.
>    * It was set up long ago during the initial boot phase and is
> - * is given to us.
> + * given to us.
>    * Baud rate clocks are zero-based in the driver code (as that maps
>    * to port numbers). Documentation uses 1-based numbering.
>    */
> @@ -421,7 +421,7 @@ static void qe_upload_microcode(const void *base,
>   
>   	for (i = 0; i < be32_to_cpu(ucode->count); i++)
>   		iowrite32be(be32_to_cpu(code[i]), &qe_immr->iram.idata);
> -	
> +

This change is not linked to the double "is".

>   	/* Set I-RAM Ready Register */
>   	iowrite32be(QE_IRAM_READY, &qe_immr->iram.iready);
>   }
> 

^ permalink raw reply

* Re: [PATCH] mm: Remove a repeated word in a comment
From: Christophe Leroy @ 2021-11-15  8:04 UTC (permalink / raw)
  To: Jason Wang, benh; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20211113044018.130835-1-wangborong@cdjrlc.com>



Le 13/11/2021 à 05:40, Jason Wang a écrit :
> The double word `up' in a comment is repeated, thus one of
> them should be removed.

If you do other changes than this one, say it here as well, see below.

> 
> Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
> ---
>   drivers/macintosh/mediabay.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
> index eab7e83c11c4..ec23094263e7 100644
> --- a/drivers/macintosh/mediabay.c
> +++ b/drivers/macintosh/mediabay.c
> @@ -129,7 +129,7 @@ enum {
>   /*
>    * Functions for polling content of media bay
>    */
> -
> +

This change is not part of your description.

>   static u8
>   ohare_mb_content(struct media_bay_info *bay)
>   {
> @@ -331,12 +331,12 @@ static void keylargo_mb_un_reset_ide(struct media_bay_info* bay)
>   
>   static inline void set_mb_power(struct media_bay_info* bay, int onoff)
>   {
> -	/* Power up up and assert the bay reset line */
> +	/* Power up and assert the bay reset line */
>   	if (onoff) {
>   		bay->ops->power(bay, 1);
>   		bay->state = mb_powering_up;
>   		pr_debug("mediabay%d: powering up\n", bay->index);
> -	} else {
> +	} else {

This change is not part of your description.

>   		/* Make sure everything is powered down & disabled */
>   		bay->ops->power(bay, 0);
>   		bay->state = mb_powering_down;
> @@ -577,7 +577,7 @@ static int media_bay_attach(struct macio_dev *mdev,
>   		macio_release_resources(mdev);
>   		return -ENOMEM;
>   	}
> -	
> +

This change is not part of your description.

>   	i = media_bay_count++;
>   	bay = &media_bays[i];
>   	bay->mdev = mdev;
> @@ -745,7 +745,7 @@ static int __init media_bay_init(void)
>   	if (!machine_is(powermac))
>   		return 0;
>   
> -	macio_register_driver(&media_bay_driver);	
> +	macio_register_driver(&media_bay_driver);

This change is not part of your description.

>   
>   	return 0;
>   }
> 

^ permalink raw reply

* [PATCH] powerpc: tsi108: make EXPORT_SYMBOL follow its function immediately
From: Jason Wang @ 2021-11-14 11:56 UTC (permalink / raw)
  To: mpe; +Cc: andrew, linux-kernel, wangborong, michael, paulus, linuxppc-dev,
	davem

EXPORT_SYMBOL(foo); should immediately follow its function/variable.

Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
---
 arch/powerpc/sysdev/tsi108_dev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/tsi108_dev.c b/arch/powerpc/sysdev/tsi108_dev.c
index 4c4a6efd5e5f..9e13fb35ed5c 100644
--- a/arch/powerpc/sysdev/tsi108_dev.c
+++ b/arch/powerpc/sysdev/tsi108_dev.c
@@ -51,13 +51,12 @@ phys_addr_t get_csrbase(void)
 	}
 	return tsi108_csr_base;
 }
+EXPORT_SYMBOL(get_csrbase);
 
 u32 get_vir_csrbase(void)
 {
 	return (u32) (ioremap(get_csrbase(), 0x10000));
 }
-
-EXPORT_SYMBOL(get_csrbase);
 EXPORT_SYMBOL(get_vir_csrbase);
 
 static int __init tsi108_eth_of_init(void)
-- 
2.33.0



^ permalink raw reply related

* [PATCH] soc: fsl: qe: Fix typo in a comment
From: Jason Wang @ 2021-11-13  5:59 UTC (permalink / raw)
  To: leoyang.li
  Cc: Jason Wang, linuxppc-dev, linux-kernel, linux-arm-kernel,
	qiang.zhao

The double `is' in a comment is repeated, thus one of them
should be removed.

Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
---
 drivers/soc/fsl/qe/qe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 4d38c80f8be8..b3c226eb5292 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -147,7 +147,7 @@ EXPORT_SYMBOL(qe_issue_cmd);
  * memory mapped space.
  * The BRG clock is the QE clock divided by 2.
  * It was set up long ago during the initial boot phase and is
- * is given to us.
+ * given to us.
  * Baud rate clocks are zero-based in the driver code (as that maps
  * to port numbers). Documentation uses 1-based numbering.
  */
@@ -421,7 +421,7 @@ static void qe_upload_microcode(const void *base,
 
 	for (i = 0; i < be32_to_cpu(ucode->count); i++)
 		iowrite32be(be32_to_cpu(code[i]), &qe_immr->iram.idata);
-	
+
 	/* Set I-RAM Ready Register */
 	iowrite32be(QE_IRAM_READY, &qe_immr->iram.iready);
 }
-- 
2.33.0


^ permalink raw reply related

* [PATCH] mm: Remove a repeated word in a comment
From: Jason Wang @ 2021-11-13  4:40 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, linux-kernel, wangborong

The double word `up' in a comment is repeated, thus one of
them should be removed.

Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
---
 drivers/macintosh/mediabay.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
index eab7e83c11c4..ec23094263e7 100644
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -129,7 +129,7 @@ enum {
 /*
  * Functions for polling content of media bay
  */
- 
+
 static u8
 ohare_mb_content(struct media_bay_info *bay)
 {
@@ -331,12 +331,12 @@ static void keylargo_mb_un_reset_ide(struct media_bay_info* bay)
 
 static inline void set_mb_power(struct media_bay_info* bay, int onoff)
 {
-	/* Power up up and assert the bay reset line */
+	/* Power up and assert the bay reset line */
 	if (onoff) {
 		bay->ops->power(bay, 1);
 		bay->state = mb_powering_up;
 		pr_debug("mediabay%d: powering up\n", bay->index);
-	} else { 
+	} else {
 		/* Make sure everything is powered down & disabled */
 		bay->ops->power(bay, 0);
 		bay->state = mb_powering_down;
@@ -577,7 +577,7 @@ static int media_bay_attach(struct macio_dev *mdev,
 		macio_release_resources(mdev);
 		return -ENOMEM;
 	}
-	
+
 	i = media_bay_count++;
 	bay = &media_bays[i];
 	bay->mdev = mdev;
@@ -745,7 +745,7 @@ static int __init media_bay_init(void)
 	if (!machine_is(powermac))
 		return 0;
 
-	macio_register_driver(&media_bay_driver);	
+	macio_register_driver(&media_bay_driver);
 
 	return 0;
 }
-- 
2.33.0


^ permalink raw reply related


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