* Re: [PATCH 3/3] powerpc/smp: Cache CPU to chip lookup
From: Gautham R Shenoy @ 2021-04-15 17:19 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R Shenoy, Peter Zijlstra,
Daniel Henrique Barboza, Valentin Schneider, qemu-ppc,
Cedric Le Goater, linuxppc-dev, Ingo Molnar, David Gibson
In-Reply-To: <20210415120934.232271-4-srikar@linux.vnet.ibm.com>
On Thu, Apr 15, 2021 at 05:39:34PM +0530, Srikar Dronamraju wrote:
> On systems with large CPUs per node, even with the filtered matching of
> related CPUs, there can be large number of calls to cpu_to_chip_id for
> the same CPU. For example with 4096 vCPU, 1 node QEMU configuration,
> with 4 threads per core, system could be see upto 1024 calls to
> cpu_to_chip_id() for the same CPU. On a given system, cpu_to_chip_id()
> for a given CPU would always return the same. Hence cache the result in
> a lookup table for use in subsequent calls.
>
> Since all CPUs sharing the same core will belong to the same chip, the
> lookup_table has an entry for one CPU per core. chip_id_lookup_table is
> not being freed and would be used on subsequent CPU online post CPU
> offline.
>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: qemu-ppc@nongnu.org
> Cc: Cedric Le Goater <clg@kaod.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
> Reported-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/smp.h | 1 +
> arch/powerpc/kernel/prom.c | 19 +++++++++++++++----
> arch/powerpc/kernel/smp.c | 21 +++++++++++++++++++--
> 3 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 47081a9e13ca..03b3d010cbab 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -31,6 +31,7 @@ extern u32 *cpu_to_phys_id;
> extern bool coregroup_enabled;
>
> extern int cpu_to_chip_id(int cpu);
> +extern int *chip_id_lookup_table;
>
> #ifdef CONFIG_SMP
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 9a4797d1d40d..6d2e4a5bc471 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -65,6 +65,8 @@
> #define DBG(fmt...)
> #endif
>
> +int *chip_id_lookup_table;
> +
> #ifdef CONFIG_PPC64
> int __initdata iommu_is_off;
> int __initdata iommu_force_on;
> @@ -914,13 +916,22 @@ EXPORT_SYMBOL(of_get_ibm_chip_id);
> int cpu_to_chip_id(int cpu)
> {
> struct device_node *np;
> + int ret = -1, idx;
> +
> + idx = cpu / threads_per_core;
> + if (chip_id_lookup_table && chip_id_lookup_table[idx] != -1)
The value -1 is ambiguous since we won't be able to determine if
it is because we haven't yet made a of_get_ibm_chip_id() call
or if of_get_ibm_chip_id() call was made and it returned a -1.
Thus, perhaps we can initialize chip_id_lookup_table[idx] with a
different unique negative value. How about S32_MIN ? and check
chip_id_lookup_table[idx] is different here ?
> + return chip_id_lookup_table[idx];
>
> np = of_get_cpu_node(cpu, NULL);
> - if (!np)
> - return -1;
> + if (np) {
> + ret = of_get_ibm_chip_id(np);
> + of_node_put(np);
> +
> + if (chip_id_lookup_table)
> + chip_id_lookup_table[idx] = ret;
> + }
>
> - of_node_put(np);
> - return of_get_ibm_chip_id(np);
> + return ret;
> }
> EXPORT_SYMBOL(cpu_to_chip_id);
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 5c7ce1d50631..50520fbea424 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1073,6 +1073,20 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> cpu_smallcore_mask(boot_cpuid));
> }
>
> + if (cpu_to_chip_id(boot_cpuid) != -1) {
> + int idx = num_possible_cpus() / threads_per_core;
> +
> + /*
> + * All threads of a core will all belong to the same core,
> + * chip_id_lookup_table will have one entry per core.
> + * Assumption: if boot_cpuid doesn't have a chip-id, then no
> + * other CPUs, will also not have chip-id.
> + */
> + chip_id_lookup_table = kcalloc(idx, sizeof(int), GFP_KERNEL);
> + if (chip_id_lookup_table)
> + memset(chip_id_lookup_table, -1, sizeof(int) * idx);
> + }
> +
> if (smp_ops && smp_ops->probe)
> smp_ops->probe();
> }
> @@ -1468,8 +1482,8 @@ static void add_cpu_to_masks(int cpu)
> {
> struct cpumask *(*submask_fn)(int) = cpu_sibling_mask;
> int first_thread = cpu_first_thread_sibling(cpu);
> - int chip_id = cpu_to_chip_id(cpu);
> cpumask_var_t mask;
> + int chip_id = -1;
> bool ret;
> int i;
>
> @@ -1492,7 +1506,10 @@ static void add_cpu_to_masks(int cpu)
> if (has_coregroup_support())
> update_coregroup_mask(cpu, &mask);
>
> - if (chip_id == -1 || !ret) {
> + if (chip_id_lookup_table && ret)
> + chip_id = cpu_to_chip_id(cpu);
> +
> + if (chip_id == -1) {
> cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
> goto out;
> }
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/smp: Reintroduce cpu_core_mask
From: Srikar Dronamraju @ 2021-04-15 17:36 UTC (permalink / raw)
To: Gautham R Shenoy
Cc: Nathan Lynch, Peter Zijlstra, Daniel Henrique Barboza,
Valentin Schneider, qemu-ppc, Cedric Le Goater, linuxppc-dev,
Ingo Molnar, David Gibson
In-Reply-To: <20210415171134.GA16351@in.ibm.com>
* Gautham R Shenoy <ego@linux.vnet.ibm.com> [2021-04-15 22:41:34]:
> Hi Srikar,
>
>
Thanks for taking a look.
> > @@ -1485,12 +1486,36 @@ static void add_cpu_to_masks(int cpu)
> > add_cpu_to_smallcore_masks(cpu);
> >
> > /* In CPU-hotplug path, hence use GFP_ATOMIC */
> > - alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
> > + ret = alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
> > update_mask_by_l2(cpu, &mask);
> >
> > if (has_coregroup_support())
> > update_coregroup_mask(cpu, &mask);
> >
> > + if (chip_id == -1 || !ret) {
> > + cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
> > + goto out;
> > + }
> > +
> > + if (shared_caches)
> > + submask_fn = cpu_l2_cache_mask;
> > +
> > + /* Update core_mask with all the CPUs that are part of submask */
> > + or_cpumasks_related(cpu, cpu, submask_fn, cpu_core_mask);
> >
>
> If coregroups exist, we can add the cpus of the coregroup to the
> cpu_core_mask thereby reducing the scope of the for_each_cpu() search
> below. This will still cut down the time on Baremetal systems
> supporting coregroups.
>
Yes, once we upstream coregroup support to Baremetal, we should look
at adding it. Also do note, number of CPUs we support for Baremetal is
comparatively lower than in PowerVM + QEMU. And more importantly the
number of cores per coregroup is also very low. So the optimization
may not yield too much of a benefit.
Its only in the QEMU case, where we end up having too many cores in
the same chip, where we see a drastic increase in the boot-up time.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/smp: Cache CPU to chip lookup
From: Srikar Dronamraju @ 2021-04-15 17:51 UTC (permalink / raw)
To: Gautham R Shenoy
Cc: Nathan Lynch, Peter Zijlstra, Daniel Henrique Barboza,
Valentin Schneider, qemu-ppc, Cedric Le Goater, linuxppc-dev,
Ingo Molnar, David Gibson
In-Reply-To: <20210415171921.GB16351@in.ibm.com>
* Gautham R Shenoy <ego@linux.vnet.ibm.com> [2021-04-15 22:49:21]:
> >
> > +int *chip_id_lookup_table;
> > +
> > #ifdef CONFIG_PPC64
> > int __initdata iommu_is_off;
> > int __initdata iommu_force_on;
> > @@ -914,13 +916,22 @@ EXPORT_SYMBOL(of_get_ibm_chip_id);
> > int cpu_to_chip_id(int cpu)
> > {
> > struct device_node *np;
> > + int ret = -1, idx;
> > +
> > + idx = cpu / threads_per_core;
> > + if (chip_id_lookup_table && chip_id_lookup_table[idx] != -1)
>
> The value -1 is ambiguous since we won't be able to determine if
> it is because we haven't yet made a of_get_ibm_chip_id() call
> or if of_get_ibm_chip_id() call was made and it returned a -1.
>
We don't allocate chip_id_lookup_table unless cpu_to_chip_id() return
!-1 value for the boot-cpuid. So this ensures that we dont
unnecessarily allocate chip_id_lookup_table. Also I check for
chip_id_lookup_table before calling cpu_to_chip_id() for other CPUs.
So this avoids overhead of calling cpu_to_chip_id() for platforms that
dont support it. Also its most likely that if the
chip_id_lookup_table is initialized then of_get_ibm_chip_id() call
would return a valid value.
+ Below we are only populating the lookup table, only when the
of_get_cpu_node is valid.
So I dont see any drawbacks of initializing it to -1. Do you see any?
> Thus, perhaps we can initialize chip_id_lookup_table[idx] with a
> different unique negative value. How about S32_MIN ? and check
> chip_id_lookup_table[idx] is different here ?
>
I had initially initialized to -2, But then I thought we adding in
more confusion than necessary and it was not solving any issues.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
From: Jesper Dangaard Brouer @ 2021-04-15 18:08 UTC (permalink / raw)
To: David Laight
Cc: Arnd Bergmann, Grygorii Strashko, netdev@vger.kernel.org,
Ilias Apalodimas, linux-kernel@vger.kernel.org,
'Matthew Wilcox', linux-mips@vger.kernel.org,
linux-mm@kvack.org, brouer, Matteo Croce,
linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <a50c3156fe8943ef964db4345344862f@AcuMS.aculab.com>
On Wed, 14 Apr 2021 21:56:39 +0000
David Laight <David.Laight@ACULAB.COM> wrote:
> From: Matthew Wilcox
> > Sent: 14 April 2021 22:36
> >
> > On Wed, Apr 14, 2021 at 09:13:22PM +0200, Jesper Dangaard Brouer wrote:
> > > (If others want to reproduce). First I could not reproduce on ARM32.
> > > Then I found out that enabling CONFIG_XEN on ARCH=arm was needed to
> > > cause the issue by enabling CONFIG_ARCH_DMA_ADDR_T_64BIT.
> >
> > hmmm ... you should be able to provoke it by enabling ARM_LPAE,
> > which selects PHYS_ADDR_T_64BIT, and
> >
> > config ARCH_DMA_ADDR_T_64BIT
> > def_bool 64BIT || PHYS_ADDR_T_64BIT
> >
> > > struct page {
> > > long unsigned int flags; /* 0 4 */
> > >
> > > /* XXX 4 bytes hole, try to pack */
> > >
> > > union {
> > > struct {
> > > struct list_head lru; /* 8 8 */
> > > struct address_space * mapping; /* 16 4 */
> > > long unsigned int index; /* 20 4 */
> > > long unsigned int private; /* 24 4 */
> > > }; /* 8 20 */
> > > struct {
> > > dma_addr_t dma_addr
>
> Adding __packed here will remove the 4 byte hole before the union
> and the compiler seems clever enough to know that anything following
> a 'long' must also be 'long' aligned.
Played with __packed in below patch, and I can confirm it seems to work.
> So you don't get anything horrid like byte accesses.
> On 64bit dma_addr will remain 64bit aligned.
> On arm32 dma_addr will be 32bit aligned - but forcing two 32bit access
> won't make any difference.
See below patch. Where I swap32 the dma address to satisfy
page->compound having bit zero cleared. (It is the simplest fix I could
come up with).
[PATCH] page_pool: handling 32-bit archs with 64-bit dma_addr_t
From: Jesper Dangaard Brouer <brouer@redhat.com>
Workaround for storing 64-bit DMA-addr on 32-bit machines in struct
page. The page->dma_addr share area with page->compound_head which
use bit zero to mark compound pages. This is okay, as DMA-addr are
aligned pointers which have bit zero cleared.
In the 32-bit case, page->compound_head is 32-bit. Thus, when
dma_addr_t is 64-bit it will be located in top 32-bit. Solve by
swapping dma_addr 32-bit segments.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
include/linux/mm_types.h | 2 +-
include/linux/types.h | 1 +
include/net/page_pool.h | 21 ++++++++++++++++++++-
net/core/page_pool.c | 8 +++++---
4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 6613b26a8894..27406e3b1e1b 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -100,7 +100,7 @@ struct page {
* @dma_addr: might require a 64-bit value even on
* 32-bit architectures.
*/
- dma_addr_t dma_addr;
+ dma_addr_t dma_addr __packed;
};
struct { /* slab, slob and slub */
union {
diff --git a/include/linux/types.h b/include/linux/types.h
index ac825ad90e44..65fd5d630016 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -141,6 +141,7 @@ typedef u64 blkcnt_t;
*/
#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
typedef u64 dma_addr_t;
+//typedef u64 __attribute__((aligned(sizeof(void *)))) dma_addr_t;
#else
typedef u32 dma_addr_t;
#endif
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index b5b195305346..c2329088665c 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -196,9 +196,28 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
page_pool_put_full_page(pool, page, true);
}
+static inline
+dma_addr_t page_pool_dma_addr_read(dma_addr_t dma_addr)
+{
+ /* Workaround for storing 64-bit DMA-addr on 32-bit machines in struct
+ * page. The page->dma_addr share area with page->compound_head which
+ * use bit zero to mark compound pages. This is okay, as DMA-addr are
+ * aligned pointers which have bit zero cleared.
+ *
+ * In the 32-bit case, page->compound_head is 32-bit. Thus, when
+ * dma_addr_t is 64-bit it will be located in top 32-bit. Solve by
+ * swapping dma_addr 32-bit segments.
+ */
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+ if (sizeof(long unsigned int) == 4) /* 32-bit system */
+ dma_addr = (dma_addr << 32) | (dma_addr >> 32);
+#endif
+ return dma_addr;
+}
+
static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
{
- return page->dma_addr;
+ return page_pool_dma_addr_read(page->dma_addr);
}
static inline bool is_page_pool_compiled_in(void)
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index ad8b0707af04..813598ea23f6 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -174,8 +174,10 @@ static void page_pool_dma_sync_for_device(struct page_pool *pool,
struct page *page,
unsigned int dma_sync_size)
{
+ dma_addr_t dma = page_pool_dma_addr_read(page->dma_addr);
+
dma_sync_size = min(dma_sync_size, pool->p.max_len);
- dma_sync_single_range_for_device(pool->p.dev, page->dma_addr,
+ dma_sync_single_range_for_device(pool->p.dev, dma,
pool->p.offset, dma_sync_size,
pool->p.dma_dir);
}
@@ -226,7 +228,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
put_page(page);
return NULL;
}
- page->dma_addr = dma;
+ page->dma_addr = page_pool_dma_addr_read(dma);
if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
@@ -294,7 +296,7 @@ void page_pool_release_page(struct page_pool *pool, struct page *page)
*/
goto skip_dma_unmap;
- dma = page->dma_addr;
+ dma = page_pool_dma_addr_read(page->dma_addr);
/* When page is unmapped, it cannot be returned our pool */
dma_unmap_page_attrs(pool->p.dev, dma,
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply related
* Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
From: Matthew Wilcox @ 2021-04-15 18:21 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Arnd Bergmann, Grygorii Strashko, netdev@vger.kernel.org,
Ilias Apalodimas, linux-kernel@vger.kernel.org,
linux-mips@vger.kernel.org, linux-mm@kvack.org, David Laight,
Matteo Croce, linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20210415200832.32796445@carbon>
On Thu, Apr 15, 2021 at 08:08:32PM +0200, Jesper Dangaard Brouer wrote:
> +static inline
> +dma_addr_t page_pool_dma_addr_read(dma_addr_t dma_addr)
> +{
> + /* Workaround for storing 64-bit DMA-addr on 32-bit machines in struct
> + * page. The page->dma_addr share area with page->compound_head which
> + * use bit zero to mark compound pages. This is okay, as DMA-addr are
> + * aligned pointers which have bit zero cleared.
> + *
> + * In the 32-bit case, page->compound_head is 32-bit. Thus, when
> + * dma_addr_t is 64-bit it will be located in top 32-bit. Solve by
> + * swapping dma_addr 32-bit segments.
> + */
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
#if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) && defined(__BIG_ENDIAN)
otherwise you'll create the problem on ARM that you're avoiding on PPC ...
I think you want to delete the word '_read' from this function name because
you're using it for both read and write.
^ permalink raw reply
* Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings
From: Andrew Morton @ 2021-04-15 18:55 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, Stephen Rothwell, Ding Tianhong, linux-kernel,
Nicholas Piggin, Christoph Hellwig, linux-mm, Jonathan Cameron,
Rick Edgecombe, linuxppc-dev
In-Reply-To: <a5c57276-737d-930b-670c-58dc0c815501@csgroup.eu>
On Thu, 15 Apr 2021 12:23:55 +0200 Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> > + * is done. STRICT_MODULE_RWX may require extra work to support this
> > + * too.
> > + */
> >
> > - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL,
> > - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>
>
> I think you should add the following in <asm/pgtable.h>
>
> #ifndef MODULES_VADDR
> #define MODULES_VADDR VMALLOC_START
> #define MODULES_END VMALLOC_END
> #endif
>
> And leave module_alloc() as is (just removing the enclosing #ifdef MODULES_VADDR and adding the
> VM_NO_HUGE_VMAP flag)
>
> This would minimise the conflits with the changes I did in powerpc/next reported by Stephen R.
>
I'll drop powerpc-64s-radix-enable-huge-vmalloc-mappings.patch for now,
make life simpler.
Nick, a redo on top of Christophe's changes in linux-next would be best
please.
^ permalink raw reply
* Re: [PATCH 1/1] of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses
From: Rob Herring @ 2021-04-15 18:59 UTC (permalink / raw)
To: Leonardo Bras
Cc: devicetree, Alexey Kardashevskiy, Frank Rowand,
linux-kernel@vger.kernel.org, PCI, linuxppc-dev
In-Reply-To: <20210415180050.373791-1-leobras.c@gmail.com>
+PPC and PCI lists
On Thu, Apr 15, 2021 at 1:01 PM Leonardo Bras <leobras.c@gmail.com> wrote:
>
> Many other resource flag parsers already add this flag when the input
> has bits 24 & 25 set, so update this one to do the same.
Many others? Looks like sparc and powerpc to me. Those would be the
ones I worry about breaking. Sparc doesn't use of/address.c so it's
fine. Powerpc version of the flags code was only fixed in 2019, so I
don't think powerpc will care either.
I noticed both sparc and powerpc set PCI_BASE_ADDRESS_MEM_TYPE_64 in
the flags. AFAICT, that's not set anywhere outside of arch code. So
never for riscv, arm and arm64 at least. That leads me to
pci_std_update_resource() which is where the PCI code sets BARs and
just copies the flags in PCI_BASE_ADDRESS_MEM_MASK ignoring
IORESOURCE_* flags. So it seems like 64-bit is still not handled and
neither is prefetch.
> Some devices (like virtio-net) have more than one memory resource
> (like MMIO32 and MMIO64) and without this flag it would be needed to
> verify the address range to know which one is which.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> drivers/of/address.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 73ddf2540f3f..dc7147843783 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -116,9 +116,12 @@ static unsigned int of_bus_pci_get_flags(const __be32 *addr)
> flags |= IORESOURCE_IO;
> break;
> case 0x02: /* 32 bits */
> - case 0x03: /* 64 bits */
> flags |= IORESOURCE_MEM;
> break;
> +
> + case 0x03: /* 64 bits */
> + flags |= IORESOURCE_MEM | IORESOURCE_MEM_64;
> + break;
> }
> if (w & 0x40000000)
> flags |= IORESOURCE_PREFETCH;
> --
> 2.30.2
>
^ permalink raw reply
* Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()
From: Lakshmi Ramasubramanian @ 2021-04-15 19:18 UTC (permalink / raw)
To: robh, dan.carpenter; +Cc: devicetree, linuxppc-dev, kbuild-all, lkp, bauerman
In-Reply-To: <20210415191437.20212-1-nramas@linux.microsoft.com>
On 4/15/21 12:14 PM, Lakshmi Ramasubramanian wrote:
Sorry - missed copying device-tree and powerpc mailing lists.
> There are a few "goto out;" statements before the local variable "fdt"
> is initialized through the call to of_kexec_alloc_and_setup_fdt() in
> elf64_load(). This will result in an uninitialized "fdt" being passed
> to kvfree() in this function if there is an error before the call to
> of_kexec_alloc_and_setup_fdt().
>
> Initialize the local variable "fdt" to NULL.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> arch/powerpc/kexec/elf_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 5a569bb51349..0051440c1f77 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -32,7 +32,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> int ret;
> unsigned long kernel_load_addr;
> unsigned long initrd_load_addr = 0, fdt_load_addr;
> - void *fdt;
> + void *fdt = NULL;
> const void *slave_code;
> struct elfhdr ehdr;
> char *modified_cmdline = NULL;
>
thanks,
-lakshmi
^ permalink raw reply
* RE: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
From: David Laight @ 2021-04-15 21:11 UTC (permalink / raw)
To: 'Matthew Wilcox', Jesper Dangaard Brouer
Cc: Arnd Bergmann, Grygorii Strashko, netdev@vger.kernel.org,
Ilias Apalodimas, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Matteo Croce,
linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20210415182155.GD2531743@casper.infradead.org>
From: Matthew Wilcox <willy@infradead.org>
> Sent: 15 April 2021 19:22
>
> On Thu, Apr 15, 2021 at 08:08:32PM +0200, Jesper Dangaard Brouer wrote:
> > +static inline
> > +dma_addr_t page_pool_dma_addr_read(dma_addr_t dma_addr)
> > +{
> > + /* Workaround for storing 64-bit DMA-addr on 32-bit machines in struct
> > + * page. The page->dma_addr share area with page->compound_head which
> > + * use bit zero to mark compound pages. This is okay, as DMA-addr are
> > + * aligned pointers which have bit zero cleared.
> > + *
> > + * In the 32-bit case, page->compound_head is 32-bit. Thus, when
> > + * dma_addr_t is 64-bit it will be located in top 32-bit. Solve by
> > + * swapping dma_addr 32-bit segments.
> > + */
> > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>
> #if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) && defined(__BIG_ENDIAN)
> otherwise you'll create the problem on ARM that you're avoiding on PPC ...
>
> I think you want to delete the word '_read' from this function name because
> you're using it for both read and write.
I think I'd use explicit dma_addr_hi and dma_addr_lo and
separate read/write functions just to make absolutely sure
nothing picks up the swapped value.
Isn't it possible to move the field down one long?
This might require an explicit zero - but this is not a common
code path - the extra write will be noise.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: Remove bpf_jit_enable=2 debugging mode
From: Daniel Borkmann @ 2021-04-15 14:37 UTC (permalink / raw)
To: Jianlin Lv, bpf
Cc: irogers, songliubraving, catalin.marinas, linux-doc, zlim.lnx,
paul.walmsley, ast, andrii, paulus, sandipan, hpa, sparclinux,
illusionist.neo, maheshb, will, nicolas.dichtel, linux-s390, iii,
paulburton, corbet, mchehab+huawei, masahiroy, x86,
john.fastabend, linux, linux-riscv, borntraeger, mingo,
linux-arm-kernel, naveen.n.rao, kuba, tklauser, linux-mips,
grantseltzer, xi.wang, aou, keescook, gor, luke.r.nels,
linux-kernel, hca, kpsingh, iecedge, horms, bp, viro, yhs, tglx,
dvyukov, tsbogend, yoshfuji, netdev, dsahern, udknight, kafai,
bjorn, palmer, quentin, linuxppc-dev, davem
In-Reply-To: <20210415093250.3391257-1-Jianlin.Lv@arm.com>
On 4/15/21 11:32 AM, Jianlin Lv wrote:
> For debugging JITs, dumping the JITed image to kernel log is discouraged,
> "bpftool prog dump jited" is much better way to examine JITed dumps.
> This patch get rid of the code related to bpf_jit_enable=2 mode and
> update the proc handler of bpf_jit_enable, also added auxiliary
> information to explain how to use bpf_jit_disasm tool after this change.
>
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
[...]
> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index 0a7a2870f111..8d36b4658076 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c
> @@ -2566,9 +2566,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> cond_resched();
> }
>
> - if (bpf_jit_enable > 1)
> - bpf_jit_dump(prog->len, proglen, pass + 1, image);
> -
> if (image) {
> bpf_jit_binary_lock_ro(header);
> prog->bpf_func = (void *)image;
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index c8496c1142c9..990b1720c7a4 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -273,16 +273,8 @@ static int proc_dointvec_minmax_bpf_enable(struct ctl_table *table, int write,
>
> tmp.data = &jit_enable;
> ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> - if (write && !ret) {
> - if (jit_enable < 2 ||
> - (jit_enable == 2 && bpf_dump_raw_ok(current_cred()))) {
> - *(int *)table->data = jit_enable;
> - if (jit_enable == 2)
> - pr_warn("bpf_jit_enable = 2 was set! NEVER use this in production, only for JIT debugging!\n");
> - } else {
> - ret = -EPERM;
> - }
> - }
> + if (write && !ret)
> + *(int *)table->data = jit_enable;
> return ret;
> }
>
> @@ -389,7 +381,7 @@ static struct ctl_table net_core_table[] = {
> .extra2 = SYSCTL_ONE,
> # else
> .extra1 = SYSCTL_ZERO,
> - .extra2 = &two,
> + .extra2 = SYSCTL_ONE,
> # endif
> },
> # ifdef CONFIG_HAVE_EBPF_JIT
> diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
> index c8ae95804728..efa4b17ae016 100644
> --- a/tools/bpf/bpf_jit_disasm.c
> +++ b/tools/bpf/bpf_jit_disasm.c
> @@ -7,7 +7,7 @@
> *
> * To get the disassembly of the JIT code, do the following:
> *
> - * 1) `echo 2 > /proc/sys/net/core/bpf_jit_enable`
> + * 1) Insert bpf_jit_dump() and recompile the kernel to output JITed image into log
Hmm, if we remove bpf_jit_dump(), the next drive-by cleanup patch will be thrown
at bpf@vger stating that bpf_jit_dump() has no in-tree users and should be removed.
Maybe we should be removing bpf_jit_disasm.c along with it as well as bpf_jit_dump()
itself ... I guess if it's ever needed in those rare occasions for JIT debugging we
can resurrect it from old kernels just locally. But yeah, bpftool's jit dump should
suffice for vast majority of use cases.
There was a recent set for ppc32 jit which was merged into ppc tree which will create
a merge conflict with this one [0]. So we would need a rebase and take it maybe during
merge win once the ppc32 landed..
[0] https://lore.kernel.org/bpf/cover.1616430991.git.christophe.leroy@csgroup.eu/
> * 2) Load a BPF filter (e.g. `tcpdump -p -n -s 0 -i eth1 host 192.168.20.0/24`)
> * 3) Run e.g. `bpf_jit_disasm -o` to read out the last JIT code
> *
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 40a88df275f9..98c7eec2923f 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -203,9 +203,6 @@ static void probe_jit_enable(void)
> case 1:
> printf("JIT compiler is enabled\n");
> break;
> - case 2:
> - printf("JIT compiler is enabled with debugging traces in kernel logs\n");
> - break;
This would still need to be there for older kernels ...
> case -1:
> printf("Unable to retrieve JIT-compiler status\n");
> break;
>
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: Remove bpf_jit_enable=2 debugging mode
From: Quentin Monnet @ 2021-04-15 15:41 UTC (permalink / raw)
To: Daniel Borkmann, Jianlin Lv, bpf
Cc: irogers, songliubraving, catalin.marinas, linux-doc, zlim.lnx,
paul.walmsley, ast, andrii, paulus, sandipan, hpa, sparclinux,
illusionist.neo, maheshb, will, nicolas.dichtel, linux-s390, iii,
paulburton, corbet, mchehab+huawei, masahiroy, x86,
john.fastabend, linux, linux-riscv, borntraeger, mingo,
linux-arm-kernel, naveen.n.rao, kuba, tklauser, linux-mips,
grantseltzer, xi.wang, aou, keescook, gor, luke.r.nels,
linux-kernel, hca, kpsingh, iecedge, horms, bp, viro, yhs, tglx,
dvyukov, tsbogend, yoshfuji, netdev, dsahern, udknight, kafai,
bjorn, palmer, linuxppc-dev, davem
In-Reply-To: <9c4a78d2-f73c-832a-e6e2-4b4daa729e07@iogearbox.net>
2021-04-15 16:37 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
> On 4/15/21 11:32 AM, Jianlin Lv wrote:
>> For debugging JITs, dumping the JITed image to kernel log is discouraged,
>> "bpftool prog dump jited" is much better way to examine JITed dumps.
>> This patch get rid of the code related to bpf_jit_enable=2 mode and
>> update the proc handler of bpf_jit_enable, also added auxiliary
>> information to explain how to use bpf_jit_disasm tool after this change.
>>
>> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
Hello,
For what it's worth, I have already seen people dump the JIT image in
kernel logs in Qemu VMs running with just a busybox, not for kernel
development, but in a context where buiding/using bpftool was not
possible. Maybe not a common case, but still, removing the debugging
mode will make that impossible. Is there a particular incentive to
remove the feature?
Best regards,
Quentin
^ permalink raw reply
* Re: [PATCH 1/1] mm: Fix struct page layout on 32-bit systems
From: Matthew Wilcox @ 2021-04-15 22:22 UTC (permalink / raw)
To: David Laight
Cc: Arnd Bergmann, Grygorii Strashko, netdev@vger.kernel.org,
Ilias Apalodimas, linux-kernel@vger.kernel.org,
linux-mips@vger.kernel.org, linux-mm@kvack.org,
Jesper Dangaard Brouer, Matteo Croce,
linuxppc-dev@lists.ozlabs.org, Christoph Hellwig,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <5179a01a462f43d6951a65de2a299070@AcuMS.aculab.com>
On Thu, Apr 15, 2021 at 09:11:56PM +0000, David Laight wrote:
> Isn't it possible to move the field down one long?
> This might require an explicit zero - but this is not a common
> code path - the extra write will be noise.
Then it overlaps page->mapping. See emails passim.
^ permalink raw reply
* Re: [PATCH v1 1/5] mm: pagewalk: Fix walk for hugepage tables
From: Daniel Axtens @ 2021-04-15 22:43 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <733408f48b1ed191f53518123ee6fc6d42287cc6.1618506910.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Pagewalk ignores hugepd entries and walk down the tables
> as if it was traditionnal entries, leading to crazy result.
>
> Add walk_hugepd_range() and use it to walk hugepage tables.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> mm/pagewalk.c | 54 +++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> index e81640d9f177..410a9d8f7572 100644
> --- a/mm/pagewalk.c
> +++ b/mm/pagewalk.c
> @@ -58,6 +58,32 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> return err;
> }
>
> +static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,
> + unsigned long end, struct mm_walk *walk, int pdshift)
> +{
> + int err = 0;
> +#ifdef CONFIG_ARCH_HAS_HUGEPD
> + const struct mm_walk_ops *ops = walk->ops;
> + int shift = hugepd_shift(*phpd);
> + int page_size = 1 << shift;
> +
> + if (addr & (page_size - 1))
> + return 0;
> +
> + for (;;) {
> + pte_t *pte = hugepte_offset(*phpd, addr, pdshift);
> +
> + err = ops->pte_entry(pte, addr, addr + page_size, walk);
> + if (err)
> + break;
> + if (addr >= end - page_size)
> + break;
> + addr += page_size;
> + }
Initially I thought this was a somewhat unintuitive way to structure
this loop, but I see it parallels the structure of walk_pte_range_inner,
so I think the consistency is worth it.
I notice the pte walking code potentially takes some locks: does this
code need to do that?
arch/powerpc/mm/hugetlbpage.c says that hugepds are protected by the
mm->page_table_lock, but I don't think we're taking it in this code.
> +#endif
> + return err;
> +}
> +
> static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
> struct mm_walk *walk)
> {
> @@ -108,7 +134,10 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
> goto again;
> }
>
> - err = walk_pte_range(pmd, addr, next, walk);
> + if (is_hugepd(__hugepd(pmd_val(*pmd))))
> + err = walk_hugepd_range((hugepd_t *)pmd, addr, next, walk, PMD_SHIFT);
> + else
> + err = walk_pte_range(pmd, addr, next, walk);
> if (err)
> break;
> } while (pmd++, addr = next, addr != end);
> @@ -157,7 +186,10 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end,
> if (pud_none(*pud))
> goto again;
>
> - err = walk_pmd_range(pud, addr, next, walk);
> + if (is_hugepd(__hugepd(pud_val(*pud))))
> + err = walk_hugepd_range((hugepd_t *)pud, addr, next, walk, PUD_SHIFT);
> + else
> + err = walk_pmd_range(pud, addr, next, walk);
I'm a bit worried you might end up calling into walk_hugepd_range with
ops->pte_entry == NULL, and then jumping to 0.
static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end,
struct mm_walk *walk)
{
...
pud = pud_offset(p4d, addr);
do {
...
if ((!walk->vma && (pud_leaf(*pud) || !pud_present(*pud))) ||
walk->action == ACTION_CONTINUE ||
!(ops->pmd_entry || ops->pte_entry)) <<< THIS CHECK
continue;
...
if (is_hugepd(__hugepd(pud_val(*pud))))
err = walk_hugepd_range((hugepd_t *)pud, addr, next, walk, PUD_SHIFT);
else
err = walk_pmd_range(pud, addr, next, walk);
if (err)
break;
} while (pud++, addr = next, addr != end);
walk_pud_range will proceed if there is _either_ an ops->pmd_entry _or_
an ops->pte_entry, but walk_hugepd_range will call ops->pte_entry
unconditionally.
The same issue applies to walk_{p4d,pgd}_range...
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings
From: Stephen Rothwell @ 2021-04-15 23:04 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Ding Tianhong, linux-kernel, Nicholas Piggin,
Christoph Hellwig, linux-mm, Jonathan Cameron, Rick Edgecombe,
linuxppc-dev
In-Reply-To: <20210415115529.9703ba8e9f7a38dea39efa56@linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]
Hi all,
On Thu, 15 Apr 2021 11:55:29 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu, 15 Apr 2021 12:23:55 +0200 Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> > > + * is done. STRICT_MODULE_RWX may require extra work to support this
> > > + * too.
> > > + */
> > >
> > > - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL,
> > > - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
> >
> >
> > I think you should add the following in <asm/pgtable.h>
> >
> > #ifndef MODULES_VADDR
> > #define MODULES_VADDR VMALLOC_START
> > #define MODULES_END VMALLOC_END
> > #endif
> >
> > And leave module_alloc() as is (just removing the enclosing #ifdef MODULES_VADDR and adding the
> > VM_NO_HUGE_VMAP flag)
> >
> > This would minimise the conflits with the changes I did in powerpc/next reported by Stephen R.
> >
>
> I'll drop powerpc-64s-radix-enable-huge-vmalloc-mappings.patch for now,
> make life simpler.
I have dropped that patch from linux-next.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v3 0/2] KVM: PPC: Book3S HV: Nested guest HFSCR changes
From: Fabiano Rosas @ 2021-04-15 23:09 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, npiggin
Applied Nick's suggestions and added a new patch for the Cause bits
issue.
I'm thinking maybe the approach of crashing L1 when L2 tries to access
a facility that L0 has denied is too heavy-handed. But on the other
hand, if L1 were to access the facility itself, the same thing would
happen and L2 runs "inside of L1" in a sense.
Currently, both L0 and L1s handle only msgsndp. All other HV Facility
Unavailable causes are already met with a Program interrupt.
Changes since v2:
- removed the sanitise functions
- moved the entry code into a new load_l2_hv_regs and the exit code
into the existing save_hv_return_state
- new patch: removes the cause bits when L0 has disabled the
corresponding facility
v2:
- made the change more generic, not only applies to hfscr anymore;
- sanitisation is now done directly on the vcpu struct, l2_hv is left unchanged;
https://lkml.kernel.org/r/20210406214645.3315819-1-farosas@linux.ibm.com
v1:
https://lkml.kernel.org/r/20210305231055.2913892-1-farosas@linux.ibm.com
Fabiano Rosas (2):
KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
KVM: PPC: Book3S HV: Stop forwarding all HFSCR cause bits to L1
arch/powerpc/kvm/book3s_hv_nested.c | 72 ++++++++++++++++++++---------
1 file changed, 51 insertions(+), 21 deletions(-)
--
2.29.2
^ permalink raw reply
* [PATCH v3 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Fabiano Rosas @ 2021-04-15 23:09 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, npiggin
In-Reply-To: <20210415230948.3563415-1-farosas@linux.ibm.com>
As one of the arguments of the H_ENTER_NESTED hypercall, the nested
hypervisor (L1) prepares a structure containing the values of various
hypervisor-privileged registers with which it wants the nested guest
(L2) to run. Since the nested HV runs in supervisor mode it needs the
host to write to these registers.
To stop a nested HV manipulating this mechanism and using a nested
guest as a proxy to access a facility that has been made unavailable
to it, we have a routine that sanitises the values of the HV registers
before copying them into the nested guest's vcpu struct.
However, when coming out of the guest the values are copied as they
were back into L1 memory, which means that any sanitisation we did
during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
This patch alters this sanitisation to have effect on the vcpu->arch
registers directly before entering and after exiting the guest,
leaving the structure that is copied back into L1 unchanged (except
when we really want L1 to access the value, e.g the Cause bits of
HFSCR).
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv_nested.c | 55 ++++++++++++++++++-----------
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 0cd0e7aad588..270552dd42c5 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -102,8 +102,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
{
struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ /*
+ * When loading the hypervisor-privileged registers to run L2,
+ * we might have used bits from L1 state to restrict what the
+ * L2 state is allowed to be. Since L1 is not allowed to read
+ * the HV registers, do not include these modifications in the
+ * return state.
+ */
+ hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
+ (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
+
hr->dpdes = vc->dpdes;
- hr->hfscr = vcpu->arch.hfscr;
hr->purr = vcpu->arch.purr;
hr->spurr = vcpu->arch.spurr;
hr->ic = vcpu->arch.ic;
@@ -132,24 +141,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
}
}
-static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
-{
- /*
- * Don't let L1 enable features for L2 which we've disabled for L1,
- * but preserve the interrupt cause field.
- */
- hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr);
-
- /* Don't let data address watchpoint match in hypervisor state */
- hr->dawrx0 &= ~DAWRX_HYP;
- hr->dawrx1 &= ~DAWRX_HYP;
-
- /* Don't let completed instruction address breakpt match in HV state */
- if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
- hr->ciabr &= ~CIABR_PRIV;
-}
-
-static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
+static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state *hr)
{
struct kvmppc_vcore *vc = vcpu->arch.vcore;
@@ -261,6 +253,27 @@ static int kvmhv_write_guest_state_and_regs(struct kvm_vcpu *vcpu,
sizeof(struct pt_regs));
}
+static void load_l2_hv_regs(struct kvm_vcpu *vcpu,
+ const struct hv_guest_state *l2_hv,
+ const struct hv_guest_state *l1_hv)
+{
+ restore_hv_regs(vcpu, l2_hv);
+
+ /*
+ * Don't let L1 enable features for L2 which we've disabled for L1,
+ * but preserve the interrupt cause field.
+ */
+ vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr);
+
+ /* Don't let data address watchpoint match in hypervisor state */
+ vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP;
+ vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP;
+
+ /* Don't let completed instruction address breakpt match in HV state */
+ if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
+ vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV;
+}
+
long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
{
long int err, r;
@@ -324,8 +337,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
LPCR_LPES | LPCR_MER;
lpcr = (vc->lpcr & ~mask) | (l2_hv.lpcr & mask);
- sanitise_hv_regs(vcpu, &l2_hv);
- restore_hv_regs(vcpu, &l2_hv);
+
+ load_l2_hv_regs(vcpu, &l2_hv, &saved_l1_hv);
vcpu->arch.ret = RESUME_GUEST;
vcpu->arch.trap = 0;
--
2.29.2
^ permalink raw reply related
* [PATCH v3 2/2] KVM: PPC: Book3S HV: Stop forwarding all HFSCR cause bits to L1
From: Fabiano Rosas @ 2021-04-15 23:09 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, npiggin
In-Reply-To: <20210415230948.3563415-1-farosas@linux.ibm.com>
Since commit 73937deb4b2d ("KVM: PPC: Book3S HV: Sanitise hv_regs on
nested guest entry") we have been disabling for the nested guest the
hypervisor facility bits that its nested hypervisor don't have access
to.
If the nested guest tries to use one of those facilities, the hardware
will cause a Hypervisor Facility Unavailable interrupt. The HFSCR
register is modified by the hardware to contain information about the
cause of the interrupt.
We have been returning the cause bits to the nested hypervisor but
since commit 549e29b458c5 ("KVM: PPC: Book3S HV: Sanitise vcpu
registers in nested path") we are reducing the amount of information
exposed to L1, so it seems like a good idea to restrict some of the
cause bits as well.
With this patch the L1 guest will be allowed to handle only the
interrupts caused by facilities it has disabled for L2. The interrupts
caused by facilities that L0 denied will cause a Program Interrupt in
L1.
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv_nested.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 270552dd42c5..912a2bcdf7b0 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -138,6 +138,23 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
hr->heir = vcpu->arch.emul_inst;
break;
+ case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
+ {
+ u8 cause = vcpu->arch.hfscr >> 56;
+
+ WARN_ON_ONCE(cause >= BITS_PER_LONG);
+
+ if (hr->hfscr & (1UL << cause)) {
+ hr->hfscr &= ~HFSCR_INTR_CAUSE;
+ /*
+ * We have not restored L1 state yet, so queue
+ * this interrupt instead of delivering it
+ * immediately.
+ */
+ kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_PROGRAM);
+ }
+ break;
+ }
}
}
--
2.29.2
^ permalink raw reply related
* Re: [PATCH v1 3/5] mm: ptdump: Provide page size to notepage()
From: Daniel Axtens @ 2021-04-15 23:12 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1ef6b954fb7b0f4dfc78820f1e612d2166c13227.1618506910.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level,
> - u64 val)
> + u64 val, unsigned long page_size)
Compilers can warn about unused parameters at -Wextra level. However,
reading scripts/Makefile.extrawarn it looks like the warning is
explicitly _disabled_ in the kernel at W=1 and not reenabled at W=2 or
W=3. So I guess this is fine...
> @@ -126,7 +126,7 @@ static int ptdump_hole(unsigned long addr, unsigned long next,
> {
> struct ptdump_state *st = walk->private;
>
> - st->note_page(st, addr, depth, 0);
> + st->note_page(st, addr, depth, 0, 0);
I know it doesn't matter at this point, but I'm not really thrilled by
the idea of passing 0 as the size here. Doesn't the hole have a known
page size?
>
> return 0;
> }
> @@ -153,5 +153,5 @@ void ptdump_walk_pgd(struct ptdump_state *st, struct mm_struct *mm, pgd_t *pgd)
> mmap_read_unlock(mm);
>
> /* Flush out the last page */
> - st->note_page(st, 0, -1, 0);
> + st->note_page(st, 0, -1, 0, 0);
I'm more OK with the idea of passing 0 as the size when the depth is -1
(don't know): if we don't know the depth we conceptually can't know the
page size.
Regards,
Daniel
^ permalink raw reply
* Re: [PATCH v1 4/5] mm: ptdump: Support hugepd table entries
From: Daniel Axtens @ 2021-04-15 23:29 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <f41a177a0fd5a71db616e586a9ec5c51102c6656.1618506910.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Which hugepd, page table entries can be at any level
> and can be of any size.
>
> Add support for them.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> mm/ptdump.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/mm/ptdump.c b/mm/ptdump.c
> index 61cd16afb1c8..6efdb8c15a7d 100644
> --- a/mm/ptdump.c
> +++ b/mm/ptdump.c
> @@ -112,11 +112,24 @@ static int ptdump_pte_entry(pte_t *pte, unsigned long addr,
> {
> struct ptdump_state *st = walk->private;
> pte_t val = ptep_get(pte);
> + unsigned long page_size = next - addr;
> + int level;
> +
> + if (page_size >= PGDIR_SIZE)
> + level = 0;
> + else if (page_size >= P4D_SIZE)
> + level = 1;
> + else if (page_size >= PUD_SIZE)
> + level = 2;
> + else if (page_size >= PMD_SIZE)
> + level = 3;
> + else
> + level = 4;
>
> if (st->effective_prot)
> - st->effective_prot(st, 4, pte_val(val));
> + st->effective_prot(st, level, pte_val(val));
>
> - st->note_page(st, addr, 4, pte_val(val), PAGE_SIZE);
> + st->note_page(st, addr, level, pte_val(val), page_size);
It seems to me that passing both level and page_size is a bit redundant,
but I guess it does reduce the impact on each arch's code?
Kind regards,
Daniel
>
> return 0;
> }
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: Remove bpf_jit_enable=2 debugging mode
From: Alexei Starovoitov @ 2021-04-15 23:49 UTC (permalink / raw)
To: Quentin Monnet
Cc: Ian Rogers, Song Liu, open list:DOCUMENTATION, Zi Shen Lim,
Paul Walmsley, Alexei Starovoitov, Andrii Nakryiko,
Paul Mackerras, Sandipan Das, H. Peter Anvin, sparclinux,
Shubham Bansal, Mahesh Bandewar, Will Deacon, Nicolas Dichtel,
linux-s390, Ilya Leoshkevich, paulburton, Jonathan Corbet,
Mauro Carvalho Chehab, Masahiro Yamada, X86 ML, John Fastabend,
Russell King, linux-riscv, Christian Borntraeger, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, Naveen N . Rao, Jakub Kicinski,
Tobias Klauser, linux-mips, grantseltzer, Xi Wang, Albert Ou,
Kees Cook, Vasily Gorbik, Luke Nelson, LKML, Heiko Carstens,
ppc-dev, KP Singh, iecedge, Simon Horman, Borislav Petkov,
Alexander Viro, Yonghong Song, Thomas Gleixner, Dmitry Vyukov,
tsbogend, Daniel Borkmann, Hideaki YOSHIFUJI, Network Development,
David Ahern, Wang YanQing, Martin KaFai Lau,
Björn Töpel, Palmer Dabbelt, bpf, Jianlin Lv,
David S. Miller
In-Reply-To: <d3949501-8f7d-57c4-b3fe-bcc3b24c09d8@isovalent.com>
On Thu, Apr 15, 2021 at 8:41 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-04-15 16:37 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
> > On 4/15/21 11:32 AM, Jianlin Lv wrote:
> >> For debugging JITs, dumping the JITed image to kernel log is discouraged,
> >> "bpftool prog dump jited" is much better way to examine JITed dumps.
> >> This patch get rid of the code related to bpf_jit_enable=2 mode and
> >> update the proc handler of bpf_jit_enable, also added auxiliary
> >> information to explain how to use bpf_jit_disasm tool after this change.
> >>
> >> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
>
> Hello,
>
> For what it's worth, I have already seen people dump the JIT image in
> kernel logs in Qemu VMs running with just a busybox, not for kernel
> development, but in a context where buiding/using bpftool was not
> possible.
If building/using bpftool is not possible then majority of selftests won't
be exercised. I don't think such environment is suitable for any kind
of bpf development. Much so for JIT debugging.
While bpf_jit_enable=2 is nothing but the debugging tool for JIT developers.
I'd rather nuke that code instead of carrying it from kernel to kernel.
^ permalink raw reply
* Re: [PATCH] ibmvfc: Fix invalid state machine BUG_ON
From: Martin K. Petersen @ 2021-04-16 2:51 UTC (permalink / raw)
To: james.bottomley, Tyrel Datwyler
Cc: linux-scsi, Martin K . Petersen, linux-kernel, Brian King, brking,
linuxppc-dev
In-Reply-To: <20210413001009.902400-1-tyreld@linux.ibm.com>
On Mon, 12 Apr 2021 18:10:09 -0600, Tyrel Datwyler wrote:
> This fixes an issue hitting the BUG_ON in ibmvfc_do_work. When
> going through a host action of IBMVFC_HOST_ACTION_RESET,
> we change the action to IBMVFC_HOST_ACTION_TGT_DEL,
> then drop the host lock, and reset the CRQ, which changes
> the host state to IBMVFC_NO_CRQ. If, prior to setting the
> host state to IBMVFC_NO_CRQ, ibmvfc_init_host is called,
> it can then end up changing the host action to IBMVFC_HOST_ACTION_INIT.
> If we then change the host state to IBMVFC_NO_CRQ, we will then
> hit the BUG_ON. This patch makes a couple of changes to avoid this.
> It leaves the host action to be IBMVFC_HOST_ACTION_RESET
> or IBMVFC_HOST_ACTION_REENABLE until after we drop the host
> lock and reset or reenable the CRQ. It also hardens the
> host state machine to ensure we cannot leave the reset / reenable
> state until we've finished processing the reset or reenable.
Applied to 5.13/scsi-queue, thanks!
[1/1] ibmvfc: Fix invalid state machine BUG_ON
https://git.kernel.org/mkp/scsi/c/15cfef8623a4
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* [PATCH] symbol : Make the size of the compile-related array fixed
From: 韩大鹏(Han Dapeng) @ 2021-04-16 3:12 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86@kernel.org,
H. Peter Anvin, Masahiro Yamada, Michal Marek, Mike Rapoport,
Pekka Enberg, Andrew Morton, Arseny Solokha,
韩大鹏(Han Dapeng), Arvind Sankar, Kees Cook,
Joerg Roedel, Christian Brauner, Kirill Tkhai,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-s390@vger.kernel.org, linux-kbuild@vger.kernel.org
Cc: 陈安庆(Anqing)
[-- Attachment #1.1: Type: text/plain, Size: 722 bytes --]
________________________________
OPPO
本电子邮件及其附件含有OPPO公司的保密信息，仅限于邮件指明的收件人使用（包含个人及群组）。禁止任何人在未经授权的情况下以任何形式使用。如果您错收了本邮件，请立即以电子邮件通知发件人并删除本邮件及其附件。
This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!
[-- Attachment #1.2: Type: text/html, Size: 5266 bytes --]
[-- Attachment #2: 0001-symbol-Make-the-size-of-the-compile-related-array-fi.patch --]
[-- Type: application/octet-stream, Size: 3990 bytes --]
From 540e6a6c36e6372d4f99eeb4a50c8eaa6d7989b3 Mon Sep 17 00:00:00 2001
From: Han Dapeng <handapeng@oppo.com>
Date: Fri, 16 Apr 2021 10:36:38 +0800
Subject: [PATCH] symbol : Make the size of the compile-related array fixed
For the same code, the machine's user name, hostname, or compilation time
may cause the kernel symbol address to be inconsistent, which is not
friendly to some symbol-dependent software, such as Crash.
Signed-off-by: Han Dapeng <handapeng@oppo.com>
---
arch/powerpc/mm/nohash/kaslr_booke.c | 2 +-
arch/s390/boot/version.c | 2 +-
arch/x86/boot/compressed/kaslr.c | 2 +-
arch/x86/boot/version.c | 2 +-
init/version.c | 4 ++--
scripts/mkcompile_h | 2 ++
6 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c
index 4c74e8a5482b..494ef408e60c 100644
--- a/arch/powerpc/mm/nohash/kaslr_booke.c
+++ b/arch/powerpc/mm/nohash/kaslr_booke.c
@@ -37,7 +37,7 @@ struct regions {
};
/* Simplified build-specific string for starting entropy. */
-static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
+static const char build_str[COMPILE_STR_MAX] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
struct regions __initdata regions;
diff --git a/arch/s390/boot/version.c b/arch/s390/boot/version.c
index d32e58bdda6a..627416a27d74 100644
--- a/arch/s390/boot/version.c
+++ b/arch/s390/boot/version.c
@@ -3,5 +3,5 @@
#include <generated/compile.h>
#include "boot.h"
-const char kernel_version[] = UTS_RELEASE
+const char kernel_version[COMPILE_STR_MAX] = UTS_RELEASE
" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ") " UTS_VERSION;
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b92fffbe761f..7b72b518a4c8 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -43,7 +43,7 @@
extern unsigned long get_cmd_line_ptr(void);
/* Simplified build-specific string for starting entropy. */
-static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
+static const char build_str[COMPILE_STR_MAX] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
static unsigned long rotate_xor(unsigned long hash, const void *area,
diff --git a/arch/x86/boot/version.c b/arch/x86/boot/version.c
index a1aaaf6c06a6..08feaa2d7a10 100644
--- a/arch/x86/boot/version.c
+++ b/arch/x86/boot/version.c
@@ -14,6 +14,6 @@
#include <generated/utsrelease.h>
#include <generated/compile.h>
-const char kernel_version[] =
+const char kernel_version[COMPILE_STR_MAX] =
UTS_RELEASE " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ") "
UTS_VERSION;
diff --git a/init/version.c b/init/version.c
index 92afc782b043..adfc9e91b56b 100644
--- a/init/version.c
+++ b/init/version.c
@@ -35,11 +35,11 @@ struct uts_namespace init_uts_ns = {
EXPORT_SYMBOL_GPL(init_uts_ns);
/* FIXED STRINGS! Don't touch! */
-const char linux_banner[] =
+const char linux_banner[COMPILE_STR_MAX] =
"Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
-const char linux_proc_banner[] =
+const char linux_proc_banner[COMPILE_STR_MAX] =
"%s version %s"
" (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
" (" LINUX_COMPILER ") %s\n";
diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
index 4ae735039daf..02b9d9d54da9 100755
--- a/scripts/mkcompile_h
+++ b/scripts/mkcompile_h
@@ -65,6 +65,8 @@ UTS_VERSION="$(echo $UTS_VERSION $CONFIG_FLAGS $TIMESTAMP | cut -b -$UTS_LEN)"
LD_VERSION=$($LD -v | head -n1 | sed 's/(compatible with [^)]*)//' \
| sed 's/[[:space:]]*$//')
printf '#define LINUX_COMPILER "%s"\n' "$CC_VERSION, $LD_VERSION"
+
+ echo \#define COMPILE_STR_MAX 512
} > .tmpcompile
# Only replace the real compile.h if the new one is different,
--
2.27.0
^ permalink raw reply related
* Re: [PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices
From: Benjamin Herrenschmidt @ 2021-04-16 3:24 UTC (permalink / raw)
To: Michael Walle, ath9k-devel, UNGLinuxDriver, linux-arm-kernel,
linux-kernel, linuxppc-dev, netdev, linux-mediatek,
linux-renesas-soc, linux-stm32, linux-amlogic, linux-oxnas,
linux-omap, linux-wireless, devicetree, linux-staging
Cc: Andrew Lunn, Jérôme Pouiller, Kunihiko Hayashi,
Andreas Larsson, Chris Snook, Rob Herring, Michal Simek,
Lorenzo Bianconi, Paul Mackerras, Thomas Petazzoni,
Rafał Miłecki, Nobuhiro Iwamatsu, Li Yang,
Fabio Estevam, Jerome Brunet, Stephen Hemminger, Florian Fainelli,
Frank Rowand, Vivien Didelot, Gregory Clement, Madalin Bucur,
Russell King, Neil Armstrong, Wingman Kwok, Chen-Yu Tsai,
Jose Abreu, bcm-kernel-feedback-list, NXP Linux Team,
Vadym Kochan, Jakub Kicinski, Radhey Shyam Pandey, Yisen Zhuang,
Mark Lee, Sunil Goutham, Sebastian Hesselbarth, Grygorii Strashko,
Byungho An, Alexandre Torgue, Stanislaw Gruszka,
Martin Blumenstingl, Hauke Mehrtens, Sascha Hauer, Sean Wang,
Salil Mehta, Maxime Ripard, Vladimir Zapolskiy, Claudiu Manoil,
Ryder Lee, Greg Kroah-Hartman, Murali Karicheri, John Crispin,
Matthias Brugger, Giuseppe Cavallaro, Pengutronix Kernel Team,
Kalle Valo, Mirko Lindner, Jernej Skrabec, Vladimir Oltean,
Fugang Duan, Kevin Hilman, Bryan Whitehead, Helmut Schaa,
Nicolas Ferre, David S . Miller, Taras Chornyi, Vinod Koul,
Sergei Shtylyov, Maxime Coquelin, Joyce Ooi, Heiner Kallweit,
Shawn Guo, Claudiu Beznea, Felix Fietkau
In-Reply-To: <20210412174718.17382-3-michael@walle.cc>
On Mon, 2021-04-12 at 19:47 +0200, Michael Walle wrote:
>
> /**
> * of_get_phy_mode - Get phy mode for given device_node
> @@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np, const char *name, u8 *addr)
> static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
> {
> struct platform_device *pdev = of_find_device_by_node(np);
> + struct nvmem_cell *cell;
> + const void *mac;
> + size_t len;
> int ret;
>
> - if (!pdev)
> - return -ENODEV;
> + /* Try lookup by device first, there might be a nvmem_cell_lookup
> + * associated with a given device.
> + */
> + if (pdev) {
> + ret = nvmem_get_mac_address(&pdev->dev, addr);
> + put_device(&pdev->dev);
> + return ret;
> + }
> +
This smells like the wrong band aid :)
Any struct device can contain an OF node pointer these days.
This seems all backwards. I think we are dealing with bad evolution.
We need to do a lookup for the device because we get passed an of_node.
We should just get passed a device here... or rather stop calling
of_get_mac_addr() from all those drivers and instead call
eth_platform_get_mac_address() which in turns calls of_get_mac_addr().
Then the nvmem stuff gets put in eth_platform_get_mac_address().
of_get_mac_addr() becomes a low-level thingy that most drivers don't
care about.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/smp: Reintroduce cpu_core_mask
From: David Gibson @ 2021-04-16 3:21 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R Shenoy, Peter Zijlstra,
Daniel Henrique Barboza, Valentin Schneider, qemu-ppc,
Cedric Le Goater, linuxppc-dev, Ingo Molnar
In-Reply-To: <20210415120934.232271-2-srikar@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 6647 bytes --]
On Thu, Apr 15, 2021 at 05:39:32PM +0530, Srikar Dronamraju wrote:
> Daniel reported that with Commit 4ca234a9cbd7 ("powerpc/smp: Stop
> updating cpu_core_mask") QEMU was unable to set single NUMA node SMP
> topologies such as:
> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
> i.e he expected 2 sockets in one NUMA node.
Well, strictly speaking, you can still set that toplogy in qemu but a
PAPR guest with that commit will show as having 1 socket in lscpu and
similar things.
Basically, this is because PAPR has no meaningful distinction between
cores and sockets. So it's kind of a cosmetic problem, but it is a
user-unexpected behaviour that it would be nice to avoid if it's not
excessively difficult.
> The above commit helped to reduce boot time on Large Systems for
> example 4096 vCPU single socket QEMU instance. PAPR is silent on
> having more than one socket within a NUMA node.
>
> cpu_core_mask and cpu_cpu_mask for any CPU would be same unless the
> number of sockets is different from the number of NUMA nodes.
Number of sockets being different from number of NUMA nodes is routine
in qemu, and I don't think it's something we should enforce.
> One option is to reintroduce cpu_core_mask but use a slightly
> different method to arrive at the cpu_core_mask. Previously each CPU's
> chip-id would be compared with all other CPU's chip-id to verify if
> both the CPUs were related at the chip level. Now if a CPU 'A' is
> found related / (unrelated) to another CPU 'B', all the thread
> siblings of 'A' and thread siblings of 'B' are automatically marked as
> related / (unrelated).
>
> Also if a platform doesn't support ibm,chip-id property, i.e its
> cpu_to_chip_id returns -1, cpu_core_map holds a copy of
> cpu_cpu_mask().
Yeah, the other weirdness here is that ibm,chip-id isn't a PAPR
property at all - it was added for powernv. We then added it to qemu
for PAPR guests because that was the way at the time to get the guest
to advertise the expected number of sockets. It therefore basically
*only* exists on PAPR/qemu for that purpose, so if it's not serving it
we need to come up with something else.
>
> Fixes: 4ca234a9cbd7 ("powerpc/smp: Stop updating cpu_core_mask")
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: qemu-ppc@nongnu.org
> Cc: Cedric Le Goater <clg@kaod.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
> Reported-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/smp.h | 5 +++++
> arch/powerpc/kernel/smp.c | 39 ++++++++++++++++++++++++++++------
> 2 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 7a13bc20f0a0..47081a9e13ca 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -121,6 +121,11 @@ static inline struct cpumask *cpu_sibling_mask(int cpu)
> return per_cpu(cpu_sibling_map, cpu);
> }
>
> +static inline struct cpumask *cpu_core_mask(int cpu)
> +{
> + return per_cpu(cpu_core_map, cpu);
> +}
> +
> static inline struct cpumask *cpu_l2_cache_mask(int cpu)
> {
> return per_cpu(cpu_l2_cache_map, cpu);
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 5a4d59a1070d..5c7ce1d50631 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1057,17 +1057,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> local_memory_node(numa_cpu_lookup_table[cpu]));
> }
> #endif
> - /*
> - * cpu_core_map is now more updated and exists only since
> - * its been exported for long. It only will have a snapshot
> - * of cpu_cpu_mask.
> - */
> - cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
> }
>
> /* Init the cpumasks so the boot CPU is related to itself */
> cpumask_set_cpu(boot_cpuid, cpu_sibling_mask(boot_cpuid));
> cpumask_set_cpu(boot_cpuid, cpu_l2_cache_mask(boot_cpuid));
> + cpumask_set_cpu(boot_cpuid, cpu_core_mask(boot_cpuid));
>
> if (has_coregroup_support())
> cpumask_set_cpu(boot_cpuid, cpu_coregroup_mask(boot_cpuid));
> @@ -1408,6 +1403,9 @@ static void remove_cpu_from_masks(int cpu)
> set_cpus_unrelated(cpu, i, cpu_smallcore_mask);
> }
>
> + for_each_cpu(i, cpu_core_mask(cpu))
> + set_cpus_unrelated(cpu, i, cpu_core_mask);
> +
> if (has_coregroup_support()) {
> for_each_cpu(i, cpu_coregroup_mask(cpu))
> set_cpus_unrelated(cpu, i, cpu_coregroup_mask);
> @@ -1468,8 +1466,11 @@ static void update_coregroup_mask(int cpu, cpumask_var_t *mask)
>
> static void add_cpu_to_masks(int cpu)
> {
> + struct cpumask *(*submask_fn)(int) = cpu_sibling_mask;
> int first_thread = cpu_first_thread_sibling(cpu);
> + int chip_id = cpu_to_chip_id(cpu);
> cpumask_var_t mask;
> + bool ret;
> int i;
>
> /*
> @@ -1485,12 +1486,36 @@ static void add_cpu_to_masks(int cpu)
> add_cpu_to_smallcore_masks(cpu);
>
> /* In CPU-hotplug path, hence use GFP_ATOMIC */
> - alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
> + ret = alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
> update_mask_by_l2(cpu, &mask);
>
> if (has_coregroup_support())
> update_coregroup_mask(cpu, &mask);
>
> + if (chip_id == -1 || !ret) {
> + cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
> + goto out;
> + }
> +
> + if (shared_caches)
> + submask_fn = cpu_l2_cache_mask;
> +
> + /* Update core_mask with all the CPUs that are part of submask */
> + or_cpumasks_related(cpu, cpu, submask_fn, cpu_core_mask);
> +
> + /* Skip all CPUs already part of current CPU core mask */
> + cpumask_andnot(mask, cpu_online_mask, cpu_core_mask(cpu));
> +
> + for_each_cpu(i, mask) {
> + if (chip_id == cpu_to_chip_id(i)) {
> + or_cpumasks_related(cpu, i, submask_fn, cpu_core_mask);
> + cpumask_andnot(mask, mask, submask_fn(i));
> + } else {
> + cpumask_andnot(mask, mask, cpu_core_mask(i));
> + }
> + }
> +
> +out:
> free_cpumask_var(mask);
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 0/8] generic command line v4
From: Daniel Walker @ 2021-04-16 4:09 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, H. Peter Anvin,
linux-arm-kernel, linux-kernel, devicetree, linux-kbuild
Cc: linux-efi
v4 release changes
* Updated insert-sys-cert tool to change command line symbols after
compilation.
This tool is used to release binary kernels internally to companies
and then later insert certificates for each product by consumers of
the binary kernel. Cisco uses this tool for this purpose.
Cisco has a similar need for the command line to be modified on a
binary released kernels similar to how certificates are setup.
* Added global symbols to hold append and prepend values.
These changes follow the system certificate code to allow the
insert-sys-cert tool to be used.
* Added a test case to confirm functionality.
Seemed sensible to add this to make sure everything is working.
* Dropped powerpc changes
Christophe Leroy has reservations about the features for powerpc. I
don't think his reservations are founded, and these changes should
fully work on powerpc. However, I dropped these changes so Christophe
can have more time to get comfortable with the changes.
Enjoy!
Daniel Walker (8):
CMDLINE: add generic builtin command line
scripts: insert-sys-cert: add command line insert capability
scripts: insert-sys-cert: change name to insert-symbol
CMDLINE: mips: convert to generic builtin command line
drivers: firmware: efi: libstub: enable generic commandline
CMDLINE: x86: convert to generic builtin command line
of: allow sending a NULL value to early_init_dt_scan_chosen
CMDLINE: arm64: convert to generic builtin command line
arch/arm64/Kconfig | 33 +--
arch/arm64/include/asm/setup.h | 2 +
arch/arm64/kernel/idreg-override.c | 9 +-
arch/mips/Kconfig | 4 +-
arch/mips/Kconfig.debug | 44 ----
arch/mips/configs/ar7_defconfig | 9 +-
arch/mips/configs/bcm47xx_defconfig | 8 +-
arch/mips/configs/bcm63xx_defconfig | 15 +-
arch/mips/configs/bmips_be_defconfig | 11 +-
arch/mips/configs/bmips_stb_defconfig | 11 +-
arch/mips/configs/capcella_defconfig | 11 +-
arch/mips/configs/ci20_defconfig | 10 +-
arch/mips/configs/cu1000-neo_defconfig | 10 +-
arch/mips/configs/cu1830-neo_defconfig | 10 +-
arch/mips/configs/e55_defconfig | 4 +-
arch/mips/configs/generic_defconfig | 6 +-
arch/mips/configs/gpr_defconfig | 18 +-
arch/mips/configs/loongson3_defconfig | 13 +-
arch/mips/configs/mpc30x_defconfig | 7 +-
arch/mips/configs/tb0219_defconfig | 7 +-
arch/mips/configs/tb0226_defconfig | 7 +-
arch/mips/configs/tb0287_defconfig | 7 +-
arch/mips/configs/workpad_defconfig | 11 +-
arch/mips/include/asm/setup.h | 2 +
arch/mips/kernel/relocate.c | 17 +-
arch/mips/kernel/setup.c | 36 +--
arch/mips/pic32/pic32mzda/early_console.c | 2 +-
arch/mips/pic32/pic32mzda/init.c | 3 +-
arch/x86/Kconfig | 44 +---
arch/x86/kernel/setup.c | 18 +-
.../firmware/efi/libstub/efi-stub-helper.c | 29 +++
drivers/firmware/efi/libstub/efi-stub.c | 9 +
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 13 +-
drivers/of/fdt.c | 44 ++--
include/linux/cmdline.h | 103 ++++++++
init/Kconfig | 78 ++++++
lib/Kconfig | 4 +
lib/Makefile | 3 +
lib/generic_cmdline.S | 53 ++++
lib/test_cmdline1.c | 139 ++++++++++
scripts/Makefile | 2 +-
.../{insert-sys-cert.c => insert-symbol.c} | 243 ++++++++++++------
43 files changed, 716 insertions(+), 394 deletions(-)
create mode 100644 include/linux/cmdline.h
create mode 100644 lib/generic_cmdline.S
create mode 100644 lib/test_cmdline1.c
rename scripts/{insert-sys-cert.c => insert-symbol.c} (72%)
--
2.25.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox