* Re: [PATCH v2 1/2] ASoC: fsl_mqs: Don't check clock is NULL before calling clk API
From: Markus Elfring @ 2020-06-23 7:36 UTC (permalink / raw)
To: Shengjiu Wang, alsa-devel, linuxppc-dev
Cc: Timur Tabi, Xiubo Li, Takashi Iwai, kernel-janitors, linux-kernel,
Jaroslav Kysela, Nicolin Chen, Mark Brown, Fabio Estevam
> In-Reply-To: <cover.1592888591.git.shengjiu.wang@nxp.com>
I guess that it should be sufficient to specify such a field once
for the header information.
> Because clk_prepare_enable and clk_disable_unprepare should
> check input clock parameter is NULL or not internally,
I find this change description unclear.
> then we don't need to check them before calling the function.
Please use an imperative wording for the commit message.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=dd0d718152e4c65b173070d48ea9dfc06894c3e5#n151
Regards,
Markus
^ permalink raw reply
* [PATCH v1 2/3] powerpc/mm/radix: Fix PTE/PMD fragment count for early page table mappings
From: Bharata B Rao @ 2020-06-23 7:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, npiggin
In-Reply-To: <20200623073017.1951-1-bharata@linux.ibm.com>
We can hit the following BUG_ON during memory unplug:
kernel BUG at arch/powerpc/mm/book3s64/pgtable.c:342!
Oops: Exception in kernel mode, sig: 5 [#1]
LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
NIP [c000000000093308] pmd_fragment_free+0x48/0xc0
LR [c00000000147bfec] remove_pagetable+0x578/0x60c
Call Trace:
0xc000008050000000 (unreliable)
remove_pagetable+0x384/0x60c
radix__remove_section_mapping+0x18/0x2c
remove_section_mapping+0x1c/0x3c
arch_remove_memory+0x11c/0x180
try_remove_memory+0x120/0x1b0
__remove_memory+0x20/0x40
dlpar_remove_lmb+0xc0/0x114
dlpar_memory+0x8b0/0xb20
handle_dlpar_errorlog+0xc0/0x190
pseries_hp_work_fn+0x2c/0x60
process_one_work+0x30c/0x810
worker_thread+0x98/0x540
kthread+0x1c4/0x1d0
ret_from_kernel_thread+0x5c/0x74
This occurs when unplug is attempted for such memory which has
been mapped using memblock pages as part of early kernel page
table setup. We wouldn't have initialized the PMD or PTE fragment
count for those PMD or PTE pages.
Fixing this includes 3 parts:
- Re-walk the init_mm page tables from mem_init() and initialize
the PMD and PTE fragment count to 1.
- When freeing PUD, PMD and PTE page table pages, check explicitly
if they come from memblock and if so free then appropriately.
- When we do early memblock based allocation of PMD and PUD pages,
allocate in PAGE_SIZE granularity so that we are sure the
complete page is used as pagetable page.
Since we now do PAGE_SIZE allocations for both PUD table and
PMD table (Note that PTE table allocation is already of PAGE_SIZE),
we end up allocating more memory for the same amount of system RAM.
Here is a comparision of how much more we need for a 64T and 2G
system after this patch:
1. 64T system
-------------
64T RAM would need 64G for vmemmap with struct page size being 64B.
128 PUD tables for 64T memory (1G mappings)
1 PUD table and 64 PMD tables for 64G vmemmap (2M mappings)
With default PUD[PMD]_TABLE_SIZE(4K), (128+1+64)*4K=772K
With PAGE_SIZE(64K) table allocations, (128+1+64)*64K=12352K
2. 2G system
------------
2G RAM would need 2M for vmemmap with struct page size being 64B.
1 PUD table for 2G memory (1G mapping)
1 PUD table and 1 PMD table for 2M vmemmap (2M mappings)
With default PUD[PMD]_TABLE_SIZE(4K), (1+1+1)*4K=12K
With new PAGE_SIZE(64K) table allocations, (1+1+1)*64K=192K
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgalloc.h | 11 ++-
arch/powerpc/include/asm/book3s/64/radix.h | 1 +
arch/powerpc/include/asm/sparsemem.h | 1 +
arch/powerpc/mm/book3s64/pgtable.c | 31 +++++++-
arch/powerpc/mm/book3s64/radix_pgtable.c | 80 +++++++++++++++++++-
arch/powerpc/mm/mem.c | 5 ++
arch/powerpc/mm/pgtable-frag.c | 9 ++-
7 files changed, 129 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
index 69c5b051734f..56d695f0095c 100644
--- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
@@ -109,7 +109,16 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
static inline void pud_free(struct mm_struct *mm, pud_t *pud)
{
- kmem_cache_free(PGT_CACHE(PUD_CACHE_INDEX), pud);
+ struct page *page = virt_to_page(pud);
+
+ /*
+ * Early pud pages allocated via memblock allocator
+ * can't be directly freed to slab
+ */
+ if (PageReserved(page))
+ free_reserved_page(page);
+ else
+ kmem_cache_free(PGT_CACHE(PUD_CACHE_INDEX), pud);
}
static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h
index 0cba794c4fb8..90f05d52f46d 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -297,6 +297,7 @@ static inline unsigned long radix__get_tree_size(void)
int radix__create_section_mapping(unsigned long start, unsigned long end,
int nid, pgprot_t prot);
int radix__remove_section_mapping(unsigned long start, unsigned long end);
+void radix__fixup_pgtable_fragments(void);
#endif /* CONFIG_MEMORY_HOTPLUG */
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/powerpc/include/asm/sparsemem.h b/arch/powerpc/include/asm/sparsemem.h
index c89b32443cff..d0b22a937a7a 100644
--- a/arch/powerpc/include/asm/sparsemem.h
+++ b/arch/powerpc/include/asm/sparsemem.h
@@ -16,6 +16,7 @@
extern int create_section_mapping(unsigned long start, unsigned long end,
int nid, pgprot_t prot);
extern int remove_section_mapping(unsigned long start, unsigned long end);
+void fixup_pgtable_fragments(void);
#ifdef CONFIG_PPC_BOOK3S_64
extern int resize_hpt_for_hotplug(unsigned long new_mem_size);
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index c58ad1049909..ee94a28dc6f9 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -184,6 +184,13 @@ int __meminit remove_section_mapping(unsigned long start, unsigned long end)
return hash__remove_section_mapping(start, end);
}
+
+void fixup_pgtable_fragments(void)
+{
+ if (radix_enabled())
+ radix__fixup_pgtable_fragments();
+}
+
#endif /* CONFIG_MEMORY_HOTPLUG */
void __init mmu_partition_table_init(void)
@@ -341,13 +348,23 @@ void pmd_fragment_free(unsigned long *pmd)
BUG_ON(atomic_read(&page->pt_frag_refcount) <= 0);
if (atomic_dec_and_test(&page->pt_frag_refcount)) {
- pgtable_pmd_page_dtor(page);
- __free_page(page);
+ /*
+ * Early pmd pages allocated via memblock
+ * allocator wouldn't have called _ctor
+ */
+ if (PageReserved(page))
+ free_reserved_page(page);
+ else {
+ pgtable_pmd_page_dtor(page);
+ __free_page(page);
+ }
}
}
static inline void pgtable_free(void *table, int index)
{
+ struct page *page;
+
switch (index) {
case PTE_INDEX:
pte_fragment_free(table, 0);
@@ -356,7 +373,15 @@ static inline void pgtable_free(void *table, int index)
pmd_fragment_free(table);
break;
case PUD_INDEX:
- kmem_cache_free(PGT_CACHE(PUD_CACHE_INDEX), table);
+ page = virt_to_page(table);
+ /*
+ * Early pud pages allocated via memblock
+ * allocator need to be freed differently
+ */
+ if (PageReserved(page))
+ free_reserved_page(page);
+ else
+ kmem_cache_free(PGT_CACHE(PUD_CACHE_INDEX), table);
break;
#if defined(CONFIG_PPC_4K_PAGES) && defined(CONFIG_HUGETLB_PAGE)
/* 16M hugepd directory at pud level */
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index ffccfe00ca2a..58e42393d5e8 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -37,6 +37,71 @@
unsigned int mmu_pid_bits;
unsigned int mmu_base_pid;
+static void fixup_pte_fragments(pmd_t *pmd)
+{
+ int i;
+
+ for (i = 0; i < PTRS_PER_PMD; i++, pmd++) {
+ pte_t *pte;
+ struct page *page;
+
+ if (pmd_none(*pmd))
+ continue;
+ if (pmd_is_leaf(*pmd))
+ continue;
+
+ pte = pte_offset_kernel(pmd, 0);
+ page = virt_to_page(pte);
+ atomic_inc(&page->pt_frag_refcount);
+ }
+}
+
+static void fixup_pmd_fragments(pud_t *pud)
+{
+ int i;
+
+ for (i = 0; i < PTRS_PER_PUD; i++, pud++) {
+ pmd_t *pmd;
+ struct page *page;
+
+ if (pud_none(*pud))
+ continue;
+ if (pud_is_leaf(*pud))
+ continue;
+
+ pmd = pmd_offset(pud, 0);
+ page = virt_to_page(pmd);
+ atomic_inc(&page->pt_frag_refcount);
+ fixup_pte_fragments(pmd);
+ }
+}
+
+/*
+ * Walk the init_mm page tables and fixup the PMD and PTE fragment
+ * counts. This allows the PUD, PMD and PTE pages to be freed
+ * back to buddy allocator properly during memory unplug.
+ */
+void radix__fixup_pgtable_fragments(void)
+{
+ int i;
+ pgd_t *pgd = pgd_offset_k(0UL);
+
+ spin_lock(&init_mm.page_table_lock);
+ for (i = 0; i < PTRS_PER_PGD; i++, pgd++) {
+ p4d_t *p4d = p4d_offset(pgd, 0UL);
+ pud_t *pud;
+
+ if (p4d_none(*p4d))
+ continue;
+ if (p4d_is_leaf(*p4d))
+ continue;
+
+ pud = pud_offset(p4d, 0);
+ fixup_pmd_fragments(pud);
+ }
+ spin_unlock(&init_mm.page_table_lock);
+}
+
static __ref void *early_alloc_pgtable(unsigned long size, int nid,
unsigned long region_start, unsigned long region_end)
{
@@ -58,6 +123,13 @@ static __ref void *early_alloc_pgtable(unsigned long size, int nid,
return ptr;
}
+/*
+ * When allocating pud or pmd pointers, we allocate a complete page
+ * of PAGE_SIZE rather than PUD_TABLE_SIZE or PMD_TABLE_SIZE. This
+ * is to ensure that the page obtained from the memblock allocator
+ * can be completely used as page table page and can be freed
+ * correctly when the page table entries are removed.
+ */
static int early_map_kernel_page(unsigned long ea, unsigned long pa,
pgprot_t flags,
unsigned int map_page_size,
@@ -74,8 +146,8 @@ static int early_map_kernel_page(unsigned long ea, unsigned long pa,
pgdp = pgd_offset_k(ea);
p4dp = p4d_offset(pgdp, ea);
if (p4d_none(*p4dp)) {
- pudp = early_alloc_pgtable(PUD_TABLE_SIZE, nid,
- region_start, region_end);
+ pudp = early_alloc_pgtable(PAGE_SIZE, nid,
+ region_start, region_end);
p4d_populate(&init_mm, p4dp, pudp);
}
pudp = pud_offset(p4dp, ea);
@@ -84,8 +156,8 @@ static int early_map_kernel_page(unsigned long ea, unsigned long pa,
goto set_the_pte;
}
if (pud_none(*pudp)) {
- pmdp = early_alloc_pgtable(PMD_TABLE_SIZE, nid,
- region_start, region_end);
+ pmdp = early_alloc_pgtable(PAGE_SIZE, nid, region_start,
+ region_end);
pud_populate(&init_mm, pudp, pmdp);
}
pmdp = pmd_offset(pudp, ea);
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 5f7fe13211e9..b8ea004c3ebf 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -54,6 +54,10 @@
#include <mm/mmu_decl.h>
+void __weak fixup_pgtable_fragments(void)
+{
+}
+
#ifndef CPU_FTR_COHERENT_ICACHE
#define CPU_FTR_COHERENT_ICACHE 0 /* XXX for now */
#define CPU_FTR_NOEXECUTE 0
@@ -301,6 +305,7 @@ void __init mem_init(void)
memblock_free_all();
+ fixup_pgtable_fragments();
#ifdef CONFIG_HIGHMEM
{
unsigned long pfn, highmem_mapnr;
diff --git a/arch/powerpc/mm/pgtable-frag.c b/arch/powerpc/mm/pgtable-frag.c
index ee4bd6d38602..16213c09896a 100644
--- a/arch/powerpc/mm/pgtable-frag.c
+++ b/arch/powerpc/mm/pgtable-frag.c
@@ -114,6 +114,13 @@ void pte_fragment_free(unsigned long *table, int kernel)
if (atomic_dec_and_test(&page->pt_frag_refcount)) {
if (!kernel)
pgtable_pte_page_dtor(page);
- __free_page(page);
+ /*
+ * Early pte pages allocated via memblock
+ * allocator need to be freed differently
+ */
+ if (PageReserved(page))
+ free_reserved_page(page);
+ else
+ __free_page(page);
}
}
--
2.21.3
^ permalink raw reply related
* [PATCH v1 3/3] powerpc/mm/radix: Free PUD table when freeing pagetable
From: Bharata B Rao @ 2020-06-23 7:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, npiggin
In-Reply-To: <20200623073017.1951-1-bharata@linux.ibm.com>
remove_pagetable() isn't freeing PUD table. This causes memory
leak during memory unplug. Fix this.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/mm/book3s64/radix_pgtable.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 58e42393d5e8..8ec2110eaa1a 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -782,6 +782,21 @@ static void free_pmd_table(pmd_t *pmd_start, pud_t *pud)
pud_clear(pud);
}
+static void free_pud_table(pud_t *pud_start, p4d_t *p4d)
+{
+ pud_t *pud;
+ int i;
+
+ for (i = 0; i < PTRS_PER_PUD; i++) {
+ pud = pud_start + i;
+ if (!pud_none(*pud))
+ return;
+ }
+
+ pud_free(&init_mm, pud_start);
+ p4d_clear(p4d);
+}
+
struct change_mapping_params {
pte_t *pte;
unsigned long start;
@@ -956,6 +971,7 @@ static void __meminit remove_pagetable(unsigned long start, unsigned long end)
pud_base = (pud_t *)p4d_page_vaddr(*p4d);
remove_pud_table(pud_base, addr, next);
+ free_pud_table(pud_base, p4d);
}
spin_unlock(&init_mm.page_table_lock);
--
2.21.3
^ permalink raw reply related
* [PATCH v1 1/3] powerpc/mm/radix: Create separate mappings for hot-plugged memory
From: Bharata B Rao @ 2020-06-23 7:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, npiggin
In-Reply-To: <20200623073017.1951-1-bharata@linux.ibm.com>
Memory that gets hot-plugged _during_ boot (and not the memory
that gets plugged in after boot), is mapped with 1G mappings
and will undergo splitting when it is unplugged. The splitting
code has a few issues:
1. Recursive locking
--------------------
Memory unplug path takes cpu_hotplug_lock and calls stop_machine()
for splitting the mappings. However stop_machine() takes
cpu_hotplug_lock again causing deadlock.
2. BUG: sleeping function called from in_atomic() context
---------------------------------------------------------
Memory unplug path (remove_pagetable) takes init_mm.page_table_lock
spinlock and later calls stop_machine() which does wait_for_completion()
3. Bad unlock unbalance
-----------------------
Memory unplug path takes init_mm.page_table_lock spinlock and calls
stop_machine(). The stop_machine thread function runs in a different
thread context (migration thread) which tries to release and reaquire
ptl. Releasing ptl from a different thread than which acquired it
causes bad unlock unbalance.
These problems can be avoided if we avoid mapping hot-plugged memory
with 1G mapping, thereby removing the need for splitting them during
unplug. Hence, during radix init, identify the hot-plugged memory region
and create separate mappings for each LMB so that they don't get mapped
with 1G mappings. The identification of hot-plugged memory has become
possible after the commit b6eca183e23e ("powerpc/kernel: Enables memory
hot-remove after reboot on pseries guests").
To create separate mappings for every LMB in the hot-plugged
region, we need lmb-size for which we use memory_block_size_bytes().
Since this is early init time code, the machine type isn't probed yet
and hence memory_block_size_bytes() would return the default LMB size
as 16MB. Hence we end up issuing more number of mapping requests
than earlier.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/mm/book3s64/radix_pgtable.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 8acb96de0e48..ffccfe00ca2a 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -16,6 +16,7 @@
#include <linux/hugetlb.h>
#include <linux/string_helpers.h>
#include <linux/stop_machine.h>
+#include <linux/memory.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -320,6 +321,8 @@ static void __init radix_init_pgtable(void)
{
unsigned long rts_field;
struct memblock_region *reg;
+ phys_addr_t addr;
+ u64 lmb_size = memory_block_size_bytes();
/* We don't support slb for radix */
mmu_slb_size = 0;
@@ -338,9 +341,15 @@ static void __init radix_init_pgtable(void)
continue;
}
- WARN_ON(create_physical_mapping(reg->base,
- reg->base + reg->size,
- -1, PAGE_KERNEL));
+ if (memblock_is_hotpluggable(reg)) {
+ for (addr = reg->base; addr < (reg->base + reg->size);
+ addr += lmb_size)
+ WARN_ON(create_physical_mapping(addr,
+ addr + lmb_size, -1, PAGE_KERNEL));
+ } else
+ WARN_ON(create_physical_mapping(reg->base,
+ reg->base + reg->size,
+ -1, PAGE_KERNEL));
}
/* Find out how many PID bits are supported */
--
2.21.3
^ permalink raw reply related
* [PATCH v1 0/3] powerpc/mm/radix: Memory unplug fixes
From: Bharata B Rao @ 2020-06-23 7:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, npiggin
This is the next version of the fixes for memory unplug on radix.
The issues and the fix are described in the actual patches.
Changes in v1:
==============
- Rebased to latest kernel.
- Took care of p4d changes.
- Addressed Aneesh's review feedback:
- Added comments.
- Indentation fixed.
- Dropped the 1st patch (setting DRCONF_MEM_HOTREMOVABLE lmb flags) as
it is debatable if this flag should be set in the device tree by OS
and not by platform in case of hotplug. This can be looked at separately.
(The fixes in this patchset remain valid without the dropped patch)
- Dropped the last patch that removed split_kernel_mapping() to ensure
that spilitting code is available for any radix guest running on
platforms that don't set DRCONF_MEM_HOTREMOVABLE.
v0: https://lore.kernel.org/linuxppc-dev/20200406034925.22586-1-bharata@linux.ibm.com/
Bharata B Rao (3):
powerpc/mm/radix: Create separate mappings for hot-plugged memory
powerpc/mm/radix: Fix PTE/PMD fragment count for early page table
mappings
powerpc/mm/radix: Free PUD table when freeing pagetable
arch/powerpc/include/asm/book3s/64/pgalloc.h | 11 +-
arch/powerpc/include/asm/book3s/64/radix.h | 1 +
arch/powerpc/include/asm/sparsemem.h | 1 +
arch/powerpc/mm/book3s64/pgtable.c | 31 +++++-
arch/powerpc/mm/book3s64/radix_pgtable.c | 111 +++++++++++++++++--
arch/powerpc/mm/mem.c | 5 +
arch/powerpc/mm/pgtable-frag.c | 9 +-
7 files changed, 157 insertions(+), 12 deletions(-)
--
2.21.3
^ permalink raw reply
* [PATCH v2 00/15] Documentation fixes
From: Mauro Carvalho Chehab @ 2020-06-23 7:08 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: linux-ia64, Peter Zijlstra (Intel), Ram Pai, James E.J. Bottomley,
linux-mm, Eric Dumazet, netdev, Paul Mackerras, Sandipan Das,
linux-kselftest, H. Peter Anvin, Jan Kara, Sukadev Bhattiprolu,
Shuah Khan, Christoph Hellwig, Marek Szyprowski, Stephen Rothwell,
Florian Fainelli, Jonathan Corbet, Mauro Carvalho Chehab,
Will Deacon, Helge Deller, x86, Haren Myneni, Russell King,
kasan-dev, Ingo Molnar, Gerald Schaefer, linux-pci,
Jakub Kicinski, Alexey Dobriyan, linux-media, Fenghua Yu,
Marco Elver, Kees Cook, Robin Murphy, Borislav Petkov,
Alexander Viro, Bjorn Helgaas, Thomas Gleixner, Dmitry Vyukov,
Tony Luck, linux-parisc, Dave Hansen, Alexey Gladkov,
Akira Shimahara, Jeff Layton, linux-kernel, iommu,
Eric W. Biederman, Greg Kroah-Hartman, linux-fsdevel,
Andrew Morton, linuxppc-dev, David S. Miller,
Thiago Jung Bauermann, Mike Kravetz
Hi Jon,
As requested, this is a rebase of a previous series posted on Jan, 15.
Since then, several patches got merged via other trees or became
obsolete. There were also 2 patches before that fits better at the
ReST conversion patchset. So, I'll be sending it on another patch
series together with the remaining ReST conversions.
I also added reviews/acks received.
So, the series reduced from 29 to 15 patches.
Let's hope b4 would be able to properly handle this one.
Regards,
Mauro
Mauro Carvalho Chehab (15):
mm: vmalloc.c: remove a kernel-doc annotation from a removed parameter
net: dev: add a missing kernel-doc annotation
net: netdevice.h: add a description for napi_defer_hard_irqs
scripts/kernel-doc: parse __ETHTOOL_DECLARE_LINK_MODE_MASK
net: pylink.h: add kernel-doc descriptions for new fields at
phylink_config
scripts/kernel-doc: handle function pointer prototypes
fs: fs.h: fix a kernel-doc parameter description
kcsan: fix a kernel-doc warning
selftests/vm/keys: fix a broken reference at protection_keys.c
docs: hugetlbpage.rst: fix some warnings
docs: powerpc: fix some issues at vas-api.rst
docs: driver-model: remove a duplicated markup at driver.rst
docs: ABI: fix a typo when pointing to w1-generic.rst
docs: fix references for DMA*.txt files
docs: fs: proc.rst: convert a new chapter to ReST
.../ABI/testing/sysfs-driver-w1_therm | 2 +-
Documentation/PCI/pci.rst | 6 +--
Documentation/admin-guide/mm/hugetlbpage.rst | 23 +++++++---
Documentation/block/biodoc.rst | 2 +-
Documentation/bus-virt-phys-mapping.txt | 2 +-
Documentation/core-api/dma-api.rst | 6 +--
Documentation/core-api/dma-isa-lpc.rst | 2 +-
.../driver-api/driver-model/driver.rst | 2 -
Documentation/driver-api/usb/dma.rst | 6 +--
Documentation/filesystems/proc.rst | 44 +++++++++----------
Documentation/powerpc/vas-api.rst | 23 +++++++---
.../translations/ko_KR/memory-barriers.txt | 6 +--
arch/ia64/hp/common/sba_iommu.c | 12 ++---
arch/parisc/kernel/pci-dma.c | 2 +-
arch/x86/include/asm/dma-mapping.h | 4 +-
arch/x86/kernel/amd_gart_64.c | 2 +-
drivers/parisc/sba_iommu.c | 14 +++---
include/linux/dma-mapping.h | 2 +-
include/linux/fs.h | 2 +-
include/linux/kcsan-checks.h | 10 +++--
include/linux/netdevice.h | 2 +
include/linux/phylink.h | 4 ++
include/media/videobuf-dma-sg.h | 2 +-
kernel/dma/debug.c | 2 +-
mm/vmalloc.c | 1 -
net/core/dev.c | 1 +
scripts/kernel-doc | 7 +++
tools/testing/selftests/vm/protection_keys.c | 2 +-
28 files changed, 114 insertions(+), 79 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH v2 11/15] docs: powerpc: fix some issues at vas-api.rst
From: Mauro Carvalho Chehab @ 2020-06-23 7:09 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Jonathan Corbet, Mauro Carvalho Chehab, Haren Myneni,
linux-kernel, Paul Mackerras, Sukadev Bhattiprolu, linuxppc-dev
In-Reply-To: <cover.1592895969.git.mchehab+huawei@kernel.org>
There are a few issues on this document, when built via the
building with ``make htmldocs``:
Documentation/powerpc/vas-api.rst:116: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:116: WARNING: Inline emphasis start-string without end-string.
Documentation/powerpc/vas-api.rst:117: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:117: WARNING: Inline emphasis start-string without end-string.
Documentation/powerpc/vas-api.rst:120: WARNING: Definition list ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:124: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:133: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:135: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:150: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:151: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:161: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:176: WARNING: Definition list ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:253: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:253: WARNING: Inline emphasis start-string without end-string.
Documentation/powerpc/vas-api.rst:259: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:261: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:266: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:267: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:270: WARNING: Definition list ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:271: WARNING: Definition list ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:273: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:274: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:277: WARNING: Definition list ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:278: WARNING: Definition list ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:280: WARNING: Unexpected indentation.
Documentation/powerpc/vas-api.rst:287: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/powerpc/vas-api.rst:289: WARNING: Block quote ends without a blank line; unexpected unindent.
Fixes: c12e38b1d52e ("Documentation/powerpc: VAS API")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
Documentation/powerpc/vas-api.rst | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/Documentation/powerpc/vas-api.rst b/Documentation/powerpc/vas-api.rst
index 1217c2f1595e..b7fdbe560010 100644
--- a/Documentation/powerpc/vas-api.rst
+++ b/Documentation/powerpc/vas-api.rst
@@ -87,6 +87,7 @@ Applications may chose a specific instance of the NX co-processor using
the vas_id field in the VAS_TX_WIN_OPEN ioctl as detailed below.
A userspace library libnxz is available here but still in development:
+
https://github.com/abalib/power-gzip
Applications that use inflate / deflate calls can link with libnxz
@@ -110,6 +111,7 @@ Applications should use the VAS_TX_WIN_OPEN ioctl as follows to establish
a connection with NX co-processor engine:
::
+
struct vas_tx_win_open_attr {
__u32 version;
__s16 vas_id; /* specific instance of vas or -1
@@ -119,8 +121,10 @@ a connection with NX co-processor engine:
__u64 reserved2[6];
};
- version: The version field must be currently set to 1.
- vas_id: If '-1' is passed, kernel will make a best-effort attempt
+ version:
+ The version field must be currently set to 1.
+ vas_id:
+ If '-1' is passed, kernel will make a best-effort attempt
to assign an optimal instance of NX for the process. To
select the specific VAS instance, refer
"Discovery of available VAS engines" section below.
@@ -129,7 +133,8 @@ a connection with NX co-processor engine:
and must be set to 0.
The attributes attr for the VAS_TX_WIN_OPEN ioctl are defined as
- follows:
+ follows::
+
#define VAS_MAGIC 'v'
#define VAS_TX_WIN_OPEN _IOW(VAS_MAGIC, 1,
struct vas_tx_win_open_attr)
@@ -141,6 +146,8 @@ a connection with NX co-processor engine:
returns -1 and sets the errno variable to indicate the error.
Error conditions:
+
+ ====== ================================================
EINVAL fd does not refer to a valid VAS device.
EINVAL Invalid vas ID
EINVAL version is not set with proper value
@@ -149,6 +156,7 @@ a connection with NX co-processor engine:
ENOSPC System has too many active windows (connections)
opened
EINVAL reserved fields are not set to 0.
+ ====== ================================================
See the ioctl(2) man page for more details, error codes and
restrictions.
@@ -158,11 +166,13 @@ mmap() NX-GZIP device
The mmap() system call for a NX-GZIP device fd returns a paste_address
that the application can use to copy/paste its CRB to the hardware engines.
+
::
paste_addr = mmap(addr, size, prot, flags, fd, offset);
Only restrictions on mmap for a NX-GZIP device fd are:
+
* size should be PAGE_SIZE
* offset parameter should be 0ULL
@@ -170,10 +180,12 @@ that the application can use to copy/paste its CRB to the hardware engines.
In addition to the error conditions listed on the mmap(2) man
page, can also fail with one of the following error codes:
+ ====== =============================================
EINVAL fd is not associated with an open window
(i.e mmap() does not follow a successful call
to the VAS_TX_WIN_OPEN ioctl).
EINVAL offset field is not 0ULL.
+ ====== =============================================
Discovery of available VAS engines
==================================
@@ -210,7 +222,7 @@ In case if NX encounters translation error (called NX page fault) on CSB
address or any request buffer, raises an interrupt on the CPU to handle the
fault. Page fault can happen if an application passes invalid addresses or
request buffers are not in memory. The operating system handles the fault by
-updating CSB with the following data:
+updating CSB with the following data::
csb.flags = CSB_V;
csb.cc = CSB_CC_TRANSLATION;
@@ -223,7 +235,7 @@ the application can resend this request to NX.
If the OS can not update CSB due to invalid CSB address, sends SEGV signal
to the process who opened the send window on which the original request was
-issued. This signal returns with the following siginfo struct:
+issued. This signal returns with the following siginfo struct::
siginfo.si_signo = SIGSEGV;
siginfo.si_errno = EFAULT;
@@ -248,6 +260,7 @@ Simple example
==============
::
+
int use_nx_gzip()
{
int rc, fd;
--
2.26.2
^ permalink raw reply related
* [PATCH 1/2] ASoC: fsl-asoc-card: Add WM8524 support
From: Shengjiu Wang @ 2020-06-23 6:52 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel
WM8524 only supports playback mode, and only works at
slave mode.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl-asoc-card.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index d0543a53764e..57ea1b072326 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -611,6 +611,15 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
priv->dai_link[2].dpcm_capture = 0;
priv->card.dapm_routes = audio_map_tx;
priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_tx);
+ } else if (of_device_is_compatible(np, "fsl,imx-audio-wm8524")) {
+ codec_dai_name = "wm8524-hifi";
+ priv->card.set_bias_level = NULL;
+ priv->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
+ priv->dai_link[1].dpcm_capture = 0;
+ priv->dai_link[2].dpcm_capture = 0;
+ priv->cpu_priv.slot_width = 32;
+ priv->card.dapm_routes = audio_map_tx;
+ priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_tx);
} else {
dev_err(&pdev->dev, "unknown Device Tree compatible\n");
ret = -EINVAL;
@@ -760,6 +769,7 @@ static const struct of_device_id fsl_asoc_card_dt_ids[] = {
{ .compatible = "fsl,imx-audio-wm8962", },
{ .compatible = "fsl,imx-audio-wm8960", },
{ .compatible = "fsl,imx-audio-mqs", },
+ { .compatible = "fsl,imx-audio-wm8524", },
{}
};
MODULE_DEVICE_TABLE(of, fsl_asoc_card_dt_ids);
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] ASoC: bindings: fsl-asoc-card: Add compatible string for wm8524
From: Shengjiu Wang @ 2020-06-23 6:52 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel
In-Reply-To: <1592895167-30483-1-git-send-email-shengjiu.wang@nxp.com>
In order to support wm8524 codec with fsl-asoc-card machine
driver, add compatible string "fsl,imx-audio-wm8524".
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
index ca9a3a43adfd..133d7e14a4d0 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
@@ -36,6 +36,8 @@ The compatible list for this generic sound card currently:
"fsl,imx-audio-mqs"
+ "fsl,imx-audio-wm8524"
+
Required properties:
- compatible : Contains one of entries in the compatible list.
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] ASoC: fsl_mqs: Fix unchecked return value for clk_prepare_enable
From: Nicolin Chen @ 2020-06-23 6:19 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, perex, broonie,
festevam, linux-kernel
In-Reply-To: <5edd68d03def367d96268f1a9a00bd528ea5aaf2.1592888591.git.shengjiu.wang@nxp.com>
On Tue, Jun 23, 2020 at 02:01:12PM +0800, Shengjiu Wang wrote:
> Fix unchecked return value for clk_prepare_enable, add error
> handler in fsl_mqs_runtime_resume.
>
> Fixes: 9e28f6532c61 ("ASoC: fsl_mqs: Add MQS component driver")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 1/2] ASoC: fsl_mqs: Don't check clock is NULL before calling clk API
From: Nicolin Chen @ 2020-06-23 6:16 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, perex, broonie,
festevam, linux-kernel
In-Reply-To: <743be216bd504c26e8d45d5ce4a84561b67a122b.1592888591.git.shengjiu.wang@nxp.com>
On Tue, Jun 23, 2020 at 02:01:11PM +0800, Shengjiu Wang wrote:
> Because clk_prepare_enable and clk_disable_unprepare should
> check input clock parameter is NULL or not internally, then
> we don't need to check them before calling the function.
>
> Fixes: 9e28f6532c61 ("ASoC: fsl_mqs: Add MQS component driver")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* [PATCH v2 2/2] ASoC: fsl_mqs: Fix unchecked return value for clk_prepare_enable
From: Shengjiu Wang @ 2020-06-23 6:01 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1592888591.git.shengjiu.wang@nxp.com>
Fix unchecked return value for clk_prepare_enable, add error
handler in fsl_mqs_runtime_resume.
Fixes: 9e28f6532c61 ("ASoC: fsl_mqs: Add MQS component driver")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_mqs.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_mqs.c b/sound/soc/fsl/fsl_mqs.c
index b44b134390a3..69aeb0e71844 100644
--- a/sound/soc/fsl/fsl_mqs.c
+++ b/sound/soc/fsl/fsl_mqs.c
@@ -265,10 +265,20 @@ static int fsl_mqs_remove(struct platform_device *pdev)
static int fsl_mqs_runtime_resume(struct device *dev)
{
struct fsl_mqs *mqs_priv = dev_get_drvdata(dev);
+ int ret;
- clk_prepare_enable(mqs_priv->ipg);
+ ret = clk_prepare_enable(mqs_priv->ipg);
+ if (ret) {
+ dev_err(dev, "failed to enable ipg clock\n");
+ return ret;
+ }
- clk_prepare_enable(mqs_priv->mclk);
+ ret = clk_prepare_enable(mqs_priv->mclk);
+ if (ret) {
+ dev_err(dev, "failed to enable mclk clock\n");
+ clk_disable_unprepare(mqs_priv->ipg);
+ return ret;
+ }
if (mqs_priv->use_gpr)
regmap_write(mqs_priv->regmap, IOMUXC_GPR2,
--
2.21.0
^ permalink raw reply related
* [PATCH v2 1/2] ASoC: fsl_mqs: Don't check clock is NULL before calling clk API
From: Shengjiu Wang @ 2020-06-23 6:01 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1592888591.git.shengjiu.wang@nxp.com>
Because clk_prepare_enable and clk_disable_unprepare should
check input clock parameter is NULL or not internally, then
we don't need to check them before calling the function.
Fixes: 9e28f6532c61 ("ASoC: fsl_mqs: Add MQS component driver")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_mqs.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/sound/soc/fsl/fsl_mqs.c b/sound/soc/fsl/fsl_mqs.c
index 0c813a45bba7..b44b134390a3 100644
--- a/sound/soc/fsl/fsl_mqs.c
+++ b/sound/soc/fsl/fsl_mqs.c
@@ -266,11 +266,9 @@ static int fsl_mqs_runtime_resume(struct device *dev)
{
struct fsl_mqs *mqs_priv = dev_get_drvdata(dev);
- if (mqs_priv->ipg)
- clk_prepare_enable(mqs_priv->ipg);
+ clk_prepare_enable(mqs_priv->ipg);
- if (mqs_priv->mclk)
- clk_prepare_enable(mqs_priv->mclk);
+ clk_prepare_enable(mqs_priv->mclk);
if (mqs_priv->use_gpr)
regmap_write(mqs_priv->regmap, IOMUXC_GPR2,
@@ -292,11 +290,8 @@ static int fsl_mqs_runtime_suspend(struct device *dev)
regmap_read(mqs_priv->regmap, REG_MQS_CTRL,
&mqs_priv->reg_mqs_ctrl);
- if (mqs_priv->mclk)
- clk_disable_unprepare(mqs_priv->mclk);
-
- if (mqs_priv->ipg)
- clk_disable_unprepare(mqs_priv->ipg);
+ clk_disable_unprepare(mqs_priv->mclk);
+ clk_disable_unprepare(mqs_priv->ipg);
return 0;
}
--
2.21.0
^ permalink raw reply related
* [PATCH v2 0/2] Fix unchecked return value for clk_prepare_enable
From: Shengjiu Wang @ 2020-06-23 6:01 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
First patch is to remove the check of clock pointer before calling
clk API.
Second patch is to fix the issue that the return value of
clk_prepare_enable is not checked.
changes in v2:
- split the patch to separate patches
Shengjiu Wang (2):
ASoC: fsl_mqs: Don't check clock is NULL before calling clk API
ASoC: fsl_mqs: Fix unchecked return value for clk_prepare_enable
sound/soc/fsl/fsl_mqs.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
From: Aneesh Kumar K.V @ 2020-06-23 5:52 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev, linux-nvdimm; +Cc: Vaibhav Jain
In-Reply-To: <871rm649u0.fsf@linux.ibm.com>
Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> writes:
> Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> + */
>> + seq_buf_init(&s, buf, PAGE_SIZE);
>> + for (index = 0, stat = stats->scm_statistic;
>> + index < stats->num_statistics; ++index, ++stat) {
>> + seq_buf_printf(&s, "%.8s = 0x%016llX\n",
>> + stat->statistic_id, stat->statistic_value);
>
>
> That is raw number (statistic_id). Is that useful? Can we map them to user readable
> strings?
Ok i missed that "%.8s" .
>
>> + }
>> + }
>> +
>> + kfree(stats);
>> + return rc ? rc : seq_buf_used(&s);
>> +}
>> +DEVICE_ATTR_RO(perf_stats);
-aneesh
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
From: Aneesh Kumar K.V @ 2020-06-23 5:42 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev, linux-nvdimm; +Cc: Vaibhav Jain
In-Reply-To: <20200622042451.22448-2-vaibhav@linux.ibm.com>
Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> Update papr_scm.c to query dimm performance statistics from PHYP via
> H_SCM_PERFORMANCE_STATS hcall and export them to user-space as PAPR
> specific NVDIMM attribute 'perf_stats' in sysfs. The patch also
> provide a sysfs ABI documentation for the stats being reported and
> their meanings.
>
> During NVDIMM probe time in papr_scm_nvdimm_init() a special variant
> of H_SCM_PERFORMANCE_STATS hcall is issued to check if collection of
> performance statistics is supported or not. If successful then a PHYP
> returns a maximum possible buffer length needed to read all
> performance stats. This returned value is stored in a per-nvdimm
> attribute 'len_stat_buffer'.
>
> The layout of request buffer for reading NVDIMM performance stats from
> PHYP is defined in 'struct papr_scm_perf_stats' and 'struct
> papr_scm_perf_stat'. These structs are used in newly introduced
> drc_pmem_query_stats() that issues the H_SCM_PERFORMANCE_STATS hcall.
>
> The sysfs access function perf_stats_show() uses value
> 'len_stat_buffer' to allocate a buffer large enough to hold all
> possible NVDIMM performance stats and passes it to
> drc_pmem_query_stats() to populate. Finally statistics reported in the
> buffer are formatted into the sysfs access function output buffer.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Documentation/ABI/testing/sysfs-bus-papr-pmem | 27 ++++
> arch/powerpc/platforms/pseries/papr_scm.c | 139 ++++++++++++++++++
> 2 files changed, 166 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
> index 5b10d036a8d4..c1a67275c43f 100644
> --- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
> +++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
> @@ -25,3 +25,30 @@ Description:
> NVDIMM have been scrubbed.
> * "locked" : Indicating that NVDIMM contents cant
> be modified until next power cycle.
> +
> +What: /sys/bus/nd/devices/nmemX/papr/perf_stats
> +Date: May, 2020
> +KernelVersion: v5.9
> +Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
> +Description:
> + (RO) Report various performance stats related to papr-scm NVDIMM
> + device. Each stat is reported on a new line with each line
> + composed of a stat-identifier followed by it value. Below are
> + currently known dimm performance stats which are reported:
> +
> + * "CtlResCt" : Controller Reset Count
> + * "CtlResTm" : Controller Reset Elapsed Time
> + * "PonSecs " : Power-on Seconds
> + * "MemLife " : Life Remaining
> + * "CritRscU" : Critical Resource Utilization
> + * "HostLCnt" : Host Load Count
> + * "HostSCnt" : Host Store Count
> + * "HostSDur" : Host Store Duration
> + * "HostLDur" : Host Load Duration
> + * "MedRCnt " : Media Read Count
> + * "MedWCnt " : Media Write Count
> + * "MedRDur " : Media Read Duration
> + * "MedWDur " : Media Write Duration
> + * "CchRHCnt" : Cache Read Hit Count
> + * "CchWHCnt" : Cache Write Hit Count
> + * "FastWCnt" : Fast Write Count
> \ No newline at end of file
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 9c569078a09f..cb3f9acc325b 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -62,6 +62,24 @@
> PAPR_PMEM_HEALTH_FATAL | \
> PAPR_PMEM_HEALTH_UNHEALTHY)
>
> +#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
> +#define PAPR_SCM_PERF_STATS_VERSION 0x1
> +
> +/* Struct holding a single performance metric */
> +struct papr_scm_perf_stat {
> + u8 statistic_id[8];
> + u64 statistic_value;
May be stat_id, stat_val ?
> +};
> +
> +/* Struct exchanged between kernel and PHYP for fetching drc perf stats */
> +struct papr_scm_perf_stats {
> + u8 eye_catcher[8];
> + u32 stats_version; /* Should be 0x01 */
> + u32 num_statistics; /* Number of stats following */
> + /* zero or more performance matrics */
> + struct papr_scm_perf_stat scm_statistic[];
> +} __packed;
For Phyp interaction these should be big-endian. I see you do
stats[i].statistic_value = be64_to_cpu(stats[i].statistic_value);
Can we avoid that?
> +
> /* private struct associated with each region */
> struct papr_scm_priv {
> struct platform_device *pdev;
> @@ -89,6 +107,9 @@ struct papr_scm_priv {
>
> /* Health information for the dimm */
> u64 health_bitmap;
> +
> + /* length of the stat buffer as expected by phyp */
> + size_t len_stat_buffer;
how about stat_buffer_len?
> };
>
> static int drc_pmem_bind(struct papr_scm_priv *p)
> @@ -194,6 +215,75 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
> return drc_pmem_bind(p);
> }
>
> +/*
> + * Query the Dimm performance stats from PHYP and copy them (if returned) to
> + * provided struct papr_scm_perf_stats instance 'stats' of 'size' in bytes.
> + * The value of R4 is copied to 'out' if the pointer is provided.
> + */
> +static int drc_pmem_query_stats(struct papr_scm_priv *p,
> + struct papr_scm_perf_stats *buff_stats,
> + size_t size, unsigned int num_stats,
> + uint64_t *out)
> +{
> + unsigned long ret[PLPAR_HCALL_BUFSIZE];
> + struct papr_scm_perf_stat *stats;
> + s64 rc, i;
> +
> + /* Setup the out buffer */
> + if (buff_stats) {
> + memcpy(buff_stats->eye_catcher,
> + PAPR_SCM_PERF_STATS_EYECATCHER, 8);
> + buff_stats->stats_version =
> + cpu_to_be32(PAPR_SCM_PERF_STATS_VERSION);
> + buff_stats->num_statistics =
> + cpu_to_be32(num_stats);
> + } else {
> + /* In case of no out buffer ignore the size */
> + size = 0;
> + }
> +
> + /*
> + * Do the HCALL asking PHYP for info and if R4 was requested
> + * return its value in 'out' variable.
> + */
> + rc = plpar_hcall(H_SCM_PERFORMANCE_STATS, ret, p->drc_index,
> + virt_to_phys(buff_stats), size);
> + if (out)
> + *out = ret[0];
> +
> + if (rc == H_PARTIAL) {
> + dev_err(&p->pdev->dev,
> + "Unknown performance stats, Err:0x%016lX\n", ret[0]);
> + return -ENOENT;
> + } else if (rc != H_SUCCESS) {
> + dev_err(&p->pdev->dev,
> + "Failed to query performance stats, Err:%lld\n", rc);
> + return -ENXIO;
May be just -1? ENXIO is that a suitable error return here?
> + }
> +
> + /* Successfully fetched the requested stats from phyp */
> + if (size != 0) {
> + buff_stats->num_statistics =
> + be32_to_cpu(buff_stats->num_statistics);
> +
> + /* Transform the stats buffer values from BE to cpu native */
> + for (i = 0, stats = buff_stats->scm_statistic;
> + i < buff_stats->num_statistics; ++i) {
> + stats[i].statistic_value =
> + be64_to_cpu(stats[i].statistic_value);
> + }
> + dev_dbg(&p->pdev->dev,
> + "Performance stats returned %d stats\n",
> + buff_stats->num_statistics);
> + } else {
> + /* Handle case where stat buffer size was requested */
> + dev_dbg(&p->pdev->dev,
> + "Performance stats size %ld\n", ret[0]);
> + }
> +
> + return 0;
> +}
> +
> /*
> * Issue hcall to retrieve dimm health info and populate papr_scm_priv with the
> * health information.
> @@ -631,6 +721,45 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
> return 0;
> }
>
> +static ssize_t perf_stats_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int index, rc;
> + struct seq_buf s;
> + struct papr_scm_perf_stat *stat;
> + struct papr_scm_perf_stats *stats;
> + struct nvdimm *dimm = to_nvdimm(dev);
> + struct papr_scm_priv *p = nvdimm_provider_data(dimm);
> +
> + if (!p->len_stat_buffer)
> + return -ENOENT;
> +
> + /* Allocate the buffer for phyp where stats are written */
> + stats = kzalloc(p->len_stat_buffer, GFP_KERNEL);
> + if (!stats)
> + return -ENOMEM;
> +
> + /* Ask phyp to return all dimm perf stats */
> + rc = drc_pmem_query_stats(p, stats, p->len_stat_buffer, 0, NULL);
> + if (!rc) {
> + /*
> + * Go through the returned output buffer and print stats and
> + * values. Since statistic_id is essentially a char string of
> + * 8 bytes, simply use the string format specifier to print it.
> + */
> + seq_buf_init(&s, buf, PAGE_SIZE);
> + for (index = 0, stat = stats->scm_statistic;
> + index < stats->num_statistics; ++index, ++stat) {
> + seq_buf_printf(&s, "%.8s = 0x%016llX\n",
> + stat->statistic_id, stat->statistic_value);
That is raw number (statistic_id). Is that useful? Can we map them to user readable
strings?
> + }
> + }
> +
> + kfree(stats);
> + return rc ? rc : seq_buf_used(&s);
> +}
> +DEVICE_ATTR_RO(perf_stats);
> +
> static ssize_t flags_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -676,6 +805,7 @@ DEVICE_ATTR_RO(flags);
> /* papr_scm specific dimm attributes */
> static struct attribute *papr_nd_attributes[] = {
> &dev_attr_flags.attr,
> + &dev_attr_perf_stats.attr,
> NULL,
> };
>
> @@ -696,6 +826,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> struct nd_region_desc ndr_desc;
> unsigned long dimm_flags;
> int target_nid, online_nid;
> + u64 stat_size;
>
> p->bus_desc.ndctl = papr_scm_ndctl;
> p->bus_desc.module = THIS_MODULE;
> @@ -759,6 +890,14 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> dev_info(dev, "Region registered with target node %d and online node %d",
> target_nid, online_nid);
>
> + /* Try retriving the stat buffer and see if its supported */
> + if (!drc_pmem_query_stats(p, NULL, 0, 0, &stat_size)) {
> + p->len_stat_buffer = (size_t)stat_size;
> + dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
> + p->len_stat_buffer);
> + } else {
> + dev_info(&p->pdev->dev, "Limited dimm stat info available\n");
> + }
> return 0;
>
> err: nvdimm_bus_unregister(p->bus);
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_easrc: Fix uninitialized scalar variable in fsl_easrc_set_ctx_format
From: Nicolin Chen @ 2020-06-23 5:34 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
perex, broonie, festevam, linux-kernel
In-Reply-To: <1592816611-16297-1-git-send-email-shengjiu.wang@nxp.com>
On Mon, Jun 22, 2020 at 05:03:31PM +0800, Shengjiu Wang wrote:
> The "ret" in fsl_easrc_set_ctx_format is not initialized, then
> the unknown value maybe returned by this function.
>
> Fixes: 955ac624058f ("ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_mqs: Fix unchecked return value for clk_prepare_enable
From: Shengjiu Wang @ 2020-06-23 5:13 UTC (permalink / raw)
To: Markus Elfring
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
kernel-janitors, Takashi Iwai, linux-kernel, Nicolin Chen,
Mark Brown, linuxppc-dev
In-Reply-To: <3eab889e-75b6-6287-a668-a2eaa509834c@web.de>
On Tue, Jun 23, 2020 at 12:08 AM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > Fix unchecked return value for clk_prepare_enable.
> >
> > And because clk_prepare_enable and clk_disable_unprepare should
> > check input clock parameter is NULL or not, then we don't need
> > to check it before calling the function.
>
> I propose to split the adjustment of two function implementations
> into separate update steps for a small patch series.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=625d3449788f85569096780592549d0340e9c0c7#n138
>
> I suggest to improve the change descriptions accordingly.
ok, will update the patches in v2.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: Alexey Kardashevskiy @ 2020-06-23 3:52 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <03e82e1a1bcf516d01ca472546d8b31e468aba8b.camel@gmail.com>
On 23/06/2020 12:43, Leonardo Bras wrote:
> On Tue, 2020-06-23 at 12:35 +1000, Alexey Kardashevskiy wrote:
>>> I am not sure if this is true in general, but in this device (SR-IOV
>>> VF) I am testing it will return 0 windows if the default DMA window is
>>> not deleted, and 1 after it's deleted.
>>
>> Since pHyp can only create windows in "64bit space", now (I did not know
>> until a few month back) I expect that thing to return "1" always no
>> matter what happened to the default window. And removing the default
>> window will only affect the maximum number of TCEs but not the number of
>> possible windows.
>
> Humm, something gone wrong then.
>
> This patchset was developed mostly because on testing, DDW would never
> be created because windows_available would always be 0.
? On phyp, if there is a huge window, then it can be 0 or 1. On KVM, it
is 0 or 1 or 2.
>
> I will take a deeper look in that.
>
> Best regards,
> Leonardo
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: Leonardo Bras @ 2020-06-23 2:43 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cfb197e1-c608-71f9-5c98-c120a3496266@ozlabs.ru>
On Tue, 2020-06-23 at 12:35 +1000, Alexey Kardashevskiy wrote:
> > I am not sure if this is true in general, but in this device (SR-IOV
> > VF) I am testing it will return 0 windows if the default DMA window is
> > not deleted, and 1 after it's deleted.
>
> Since pHyp can only create windows in "64bit space", now (I did not know
> until a few month back) I expect that thing to return "1" always no
> matter what happened to the default window. And removing the default
> window will only affect the maximum number of TCEs but not the number of
> possible windows.
Humm, something gone wrong then.
This patchset was developed mostly because on testing, DDW would never
be created because windows_available would always be 0.
I will take a deeper look in that.
Best regards,
Leonardo
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: Alexey Kardashevskiy @ 2020-06-23 2:35 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <205edd45b7bbf39d2fc1d2d75fd7e35336f109ac.camel@gmail.com>
On 23/06/2020 12:31, Leonardo Bras wrote:
> On Tue, 2020-06-23 at 11:11 +1000, Alexey Kardashevskiy wrote:
>>
>> On 23/06/2020 04:59, Leonardo Bras wrote:
>>> Hello Alexey, thanks for the feedback!
>>>
>>> On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>>>> On 19/06/2020 15:06, Leonardo Bras wrote:
>>>>> On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
>>>>> default DMA window for the device, before attempting to configure a DDW,
>>>>> in order to make the maximum resources available for the next DDW to be
>>>>> created.
>>>>>
>>>>> This is a requirement for some devices to use DDW, given they only
>>>>> allow one DMA window.
>>>>>
>>>>> If setting up a new DDW fails anywhere after the removal of this
>>>>> default DMA window, restore it using reset_dma_window.
>>>>
>>>> Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
>>>> as pHyp can only create a single window and it has to map at
>>>> 0x800.0000.0000.0000. They probably do not care though.
>>>>
>>>> Under KVM, this will fail as VFIO allows creating 2 windows and it
>>>> starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
>>>> the window address == 0 as a failure. And we want to keep both DMA
>>>> windows for PCI adapters with both 64bit and 32bit PCI functions (I
>>>> heard AMD GPU video + audio are like this) or someone could hotplug
>>>> 32bit DMA device on a vphb with already present 64bit DMA window so we
>>>> do not remove the default window.
>>>
>>> Well, then I would suggest doing something like this:
>>> query_ddw(...);
>>> if (query.windows_available == 0){
>>> remove_dma_window(...,default_win);
>>> query_ddw(...);
>>> }
>>>
>>> This would make sure to cover cases of windows available == 1
>>> and windows available > 1;
>>
>> Is "1" what pHyp returns on query? And was it always like that? Then it
>> is probably ok. I just never really explored the idea of removing the
>> default window as we did not have to.
>
> I am not sure if this is true in general, but in this device (SR-IOV
> VF) I am testing it will return 0 windows if the default DMA window is
> not deleted, and 1 after it's deleted.
Since pHyp can only create windows in "64bit space", now (I did not know
until a few month back) I expect that thing to return "1" always no
matter what happened to the default window. And removing the default
window will only affect the maximum number of TCEs but not the number of
possible windows.
--
Alexey
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: Leonardo Bras @ 2020-06-23 2:31 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <afd1c5ac-d291-5281-1592-a345ee3c0c8c@ozlabs.ru>
On Tue, 2020-06-23 at 11:11 +1000, Alexey Kardashevskiy wrote:
>
> On 23/06/2020 04:59, Leonardo Bras wrote:
> > Hello Alexey, thanks for the feedback!
> >
> > On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
> > > On 19/06/2020 15:06, Leonardo Bras wrote:
> > > > On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
> > > > default DMA window for the device, before attempting to configure a DDW,
> > > > in order to make the maximum resources available for the next DDW to be
> > > > created.
> > > >
> > > > This is a requirement for some devices to use DDW, given they only
> > > > allow one DMA window.
> > > >
> > > > If setting up a new DDW fails anywhere after the removal of this
> > > > default DMA window, restore it using reset_dma_window.
> > >
> > > Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
> > > as pHyp can only create a single window and it has to map at
> > > 0x800.0000.0000.0000. They probably do not care though.
> > >
> > > Under KVM, this will fail as VFIO allows creating 2 windows and it
> > > starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
> > > the window address == 0 as a failure. And we want to keep both DMA
> > > windows for PCI adapters with both 64bit and 32bit PCI functions (I
> > > heard AMD GPU video + audio are like this) or someone could hotplug
> > > 32bit DMA device on a vphb with already present 64bit DMA window so we
> > > do not remove the default window.
> >
> > Well, then I would suggest doing something like this:
> > query_ddw(...);
> > if (query.windows_available == 0){
> > remove_dma_window(...,default_win);
> > query_ddw(...);
> > }
> >
> > This would make sure to cover cases of windows available == 1
> > and windows available > 1;
>
> Is "1" what pHyp returns on query? And was it always like that? Then it
> is probably ok. I just never really explored the idea of removing the
> default window as we did not have to.
I am not sure if this is true in general, but in this device (SR-IOV
VF) I am testing it will return 0 windows if the default DMA window is
not deleted, and 1 after it's deleted.
>
>
> > > The last discussed thing I remember was that there was supposed to be a
> > > new bit in "ibm,architecture-vec-5" (forgot the details), we could use
> > > that to decide whether to keep the default window or not, like this.
> >
> > I checked on the latest LoPAR draft (soon to be published), for the
> > ibm,architecture-vec 'option array 5' and this entry was the only
> > recently added one that is related to this patchset:
> >
> > Byte 8 - Bit 0:
> > SRIOV Virtual Functions Support Dynamic DMA Windows (DDW):
> > 0: SRIOV Virtual Functions do not support DDW
> > 1: SRIOV Virtual Functions do support DDW
> >
> > Isn't this equivalent to having a "ibm,ddw-applicable" property?
>
> I am not sure, is there anything else to this new bit?
I copied everything from the LoPAR, and IIRC the ACR for this change
only described this change in the document.
> I'd think if the
> client supports it, then pHyp will create one 64bit window per a PE and
> DDW API won't be needed. Thanks,
That would make sense, and be great.
I will dig some more.
Thank you!
> > > > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > > > ---
> > > > arch/powerpc/platforms/pseries/iommu.c | 20 +++++++++++++++++---
> > > > 1 file changed, 17 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > > > index de633f6ae093..68d1ea957ac7 100644
> > > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > > @@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > > > u64 dma_addr, max_addr;
> > > > struct device_node *dn;
> > > > u32 ddw_avail[3];
> > > > +
> > > > struct direct_window *window;
> > > > - struct property *win64;
> > > > + struct property *win64, *dfl_win;
> > >
> > > Make it "default_win" or "def_win", "dfl" hurts to read :)
> >
> > Sure, no problem :)
> >
> > > > struct dynamic_dma_window_prop *ddwprop;
> > > > struct failed_ddw_pdn *fpdn;
> > > >
> > > > @@ -1110,8 +1111,19 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > > > if (ret)
> > > > goto out_failed;
> > > >
> > > > - /*
> > > > - * Query if there is a second window of size to map the
> > > > + /*
> > > > + * First step of setting up DDW is removing the default DMA window,
> > > > + * if it's present. It will make all the resources available to the
> > > > + * new DDW window.
> > > > + * If anything fails after this, we need to restore it.
> > > > + */
> > > > +
> > > > + dfl_win = of_find_property(pdn, "ibm,dma-window", NULL);
> > > > + if (dfl_win)
> > > > + remove_dma_window(pdn, ddw_avail, dfl_win);
> > >
> > > Before doing so, you want to make sure that the "reset" is actually
> > > supported. Thanks,
> >
> > Good catch, I will improve that.
> >
> > >
> > > > +
> > > > + /*
> > > > + * Query if there is a window of size to map the
> > > > * whole partition. Query returns number of windows, largest
> > > > * block assigned to PE (partition endpoint), and two bitmasks
> > > > * of page sizes: supported and supported for migrate-dma.
> > > > @@ -1219,6 +1231,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > > > kfree(win64);
> > > >
> > > > out_failed:
> > > > + if (dfl_win)
> > > > + reset_dma_window(dev, pdn);
> > > >
> > > > fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
> > > > if (!fpdn)
> > > >
> >
> > Best regards,
> > Leonardo
> >
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/pseries/iommu: Update call to ibm,query-pe-dma-windows
From: Alexey Kardashevskiy @ 2020-06-23 2:29 UTC (permalink / raw)
To: Leonardo Bras
Cc: Ram Pai, linux-kernel, Paul Mackerras, linuxppc-dev,
Thiago Jung Bauermann
In-Reply-To: <c331742c9f7a3e3ccead5d9db99a66d3f268b95f.camel@gmail.com>
On 23/06/2020 12:14, Leonardo Bras wrote:
> On Tue, 2020-06-23 at 11:12 +1000, Alexey Kardashevskiy wrote:
> [snip]
>>>>> +static int query_ddw_out_sz(struct device_node *par_dn)
>>>>
>>>> Can easily be folded into query_ddw().
>>>
>>> Sure, but it will get inlined by the compiler, and I think it reads
>>> better this way.
>>> I mean, I understand you have a reason to think it's better to fold it
>>> in query_ddw(), and I would like to better understand that to improve
>>> my code in the future.
>>
>> You have numbers 5 and 6 (the number of parameters) twice in the file,
>> this is why I brought it up. query_ddw_out_sz() can potentially return
>> something else than 5 or 6 and you will have to change the callsite(s)
>> then, since these are not macros, this allows to think there may be more
>> places with 5 and 6. Dunno. A single function will simplify things imho.
>
> That's a good point. Thanks!
>
>>
>>
>>>>> +{
>>>>> + int ret;
>>>>> + u32 ddw_ext[3];
>>>>> +
>>>>> + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
>>>>> + &ddw_ext[0], 3);
>>>>> + if (ret || ddw_ext[0] < 2 || ddw_ext[2] != 1)
>>>>
>>>> Oh that PAPR thing again :-/
>>>>
>>>> ===
>>>> The “ibm,ddw-extensions” property value is a list of integers the first
>>>> integer indicates the number of extensions implemented and subsequent
>>>> integers, one per extension, provide a value associated with that
>>>> extension.
>>>> ===
>>>>
>>>> So ddw_ext[0] is length.
>>>> Listindex==2 is for "reset" says PAPR and
>>>> Listindex==3 is for this new 64bit "largest_available_block".
>>>>
>>>> So I'd expect ddw_ext[2] to have the "reset" token and ddw_ext[3] to
>>>> have "1" for this new feature but indexes are smaller. I am confused.
>>>> Either way these "2" and "3" needs to be defined in macros, "0" probably
>>>> too.
>>>
>>> Remember these indexes are not C-like 0-starting indexes, where the
>>> size would be Listindex==1.
>>
>> Yeah I can see that is the assumption but out of curiosity - is it
>> written anywhere? Across PAPR, they index bytes from 1 but bits from 0 :-/
>
> From LoPAR:
> The “ibm,ddw-extensions” property value is a list of integers the first
> integer indicates the number of extensions implemented and subsequent
> integers, one per extension, provide a value associated with that
> extension.
>
> And the list/table then shows extensions from 2 on onwards:
> List index 2 : Token of the ibm,reset-pe-dma-windows RTAS Call
> (...)
I means a place saying "we number things starting from 1 and not from
zero", this kind of thing. The code implementing the spec uses the C
language so it would make sense to count from zero, otoh the writer
probably did not write any code for ages :)
--
Alexey
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/pseries/iommu: Move window-removing part of remove_ddw into remove_dma_window
From: Leonardo Bras @ 2020-06-23 2:26 UTC (permalink / raw)
To: Oliver O'Halloran, Alexey Kardashevskiy
Cc: Ram Pai, Linux Kernel Mailing List, Paul Mackerras, linuxppc-dev,
Thiago Jung Bauermann
In-Reply-To: <CAOSf1CEC-tYH1so5b4P7dQ7s8v1o4qy_u5CG7LKtKNnRQvC4-w@mail.gmail.com>
On Tue, 2020-06-23 at 11:33 +1000, Oliver O'Halloran wrote:
> On Tue, Jun 23, 2020 at 11:12 AM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > On 23/06/2020 04:59, Leonardo Bras wrote:
> > > > Also, despite this particular file, the "pdn" name is usually used for
> > > > struct pci_dn (not device_node), let's keep it that way.
> > >
> > > Sure, I got confused for some time about this, as we have:
> > > static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn).
> > > but on *_ddw() we have "struct pci_dn *pdn".
> >
> > True again, not the cleanest style here.
> >
> >
> > > I will also add a patch that renames those 'struct device_node *pdn' to
> > > something like 'struct device_node *parent_dn'.
>
> I usually go with "np" or "node". In this case I'd use "parent_np" or
> just "parent." As you said pci_dn conventionally uses pdn so that
> should be avoided if at all possible. There's some places that just
> use "dn" for device_node, but I don't think that's something we should
> encourage due to how similar it is to pdn.
Sure, I will try that.
>
> > I would not go that far, we (well, Oliver) are getting rid of many
> > occurrences of pci_dn and Oliver may have a stronger opinion here.
>
> I'm trying to remove the use of pci_dn from non-RTAS platforms which
> doesn't apply to pseries. For RTAS platforms having pci_dn sort of
> makes sense since it's used to cache data from the device_node and
> having it saves you from needing to parse and validate the DT at
> runtime since we're supposed to be relying on the FW provided settings
> in the DT. I want to get rid of it on PowerNV because it's become a
> dumping ground for random bits and pieces of platform specific data.
> It's confusing at best and IMO it duplicates a lot of what's already
> available in the per-PHB structures which the platform specific stuff
> should actually be looking at.
>
> Oliver
Best regards,
Leonardo Bras
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/pseries/iommu: Move window-removing part of remove_ddw into remove_dma_window
From: Leonardo Bras @ 2020-06-23 2:22 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: Oliver, linuxppc-dev, linux-kernel
In-Reply-To: <887bf30e-ae9e-b0cb-0388-dc555692ff0a@ozlabs.ru>
On Tue, 2020-06-23 at 11:12 +1000, Alexey Kardashevskiy wrote:
>
> On 23/06/2020 04:59, Leonardo Bras wrote:
> > Hello Alexey, thanks for the feedback!
> >
> > On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
> > > On 19/06/2020 15:06, Leonardo Bras wrote:
> > > > Move the window-removing part of remove_ddw into a new function
> > > > (remove_dma_window), so it can be used to remove other DMA windows.
> > > >
> > > > It's useful for removing DMA windows that don't create DIRECT64_PROPNAME
> > > > property, like the default DMA window from the device, which uses
> > > > "ibm,dma-window".
> > > >
> > > > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > > > ---
> > > > arch/powerpc/platforms/pseries/iommu.c | 53 +++++++++++++++-----------
> > > > 1 file changed, 31 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > > > index 5e1fbc176a37..de633f6ae093 100644
> > > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > > @@ -767,25 +767,14 @@ static int __init disable_ddw_setup(char *str)
> > > >
> > > > early_param("disable_ddw", disable_ddw_setup);
> > > >
> > > > -static void remove_ddw(struct device_node *np, bool remove_prop)
> > > > +static void remove_dma_window(struct device_node *pdn, u32 *ddw_avail,
> > >
> > > You do not need the entire ddw_avail here, pass just the token you need.
> >
> > Well, I just emulated the behavior of create_ddw() and query_ddw() as
> > both just pass the array instead of the token, even though they only
> > use a single token.
>
> True, there is a pattern.
>
> > I think it's to make the rest of the code independent of the design of
> > the "ibm,ddw-applicable" array, and if it changes, only local changes
> > on the functions will be needed.
>
> The helper removes a window, if you are going to call other operations
> in remove_dma_window(), then you'll have to change its name ;)
Not only doing new stuff, it can change the order for some reason (as
the order of the output of query), and it would need not change the
caller.
>
>
> > > Also, despite this particular file, the "pdn" name is usually used for
> > > struct pci_dn (not device_node), let's keep it that way.
> >
> > Sure, I got confused for some time about this, as we have:
> > static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn).
> > but on *_ddw() we have "struct pci_dn *pdn".
>
> True again, not the cleanest style here.
>
>
> > I will also add a patch that renames those 'struct device_node *pdn' to
> > something like 'struct device_node *parent_dn'.
>
> I would not go that far, we (well, Oliver) are getting rid of many
> occurrences of pci_dn and Oliver may have a stronger opinion here.
>
>
> > > > + struct property *win)
> > > > {
> > > > struct dynamic_dma_window_prop *dwp;
> > > > - struct property *win64;
> > > > - u32 ddw_avail[3];
> > > > u64 liobn;
> > > > - int ret = 0;
> > > > -
> > > > - ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
> > > > - &ddw_avail[0], 3);
> > > > -
> > > > - win64 = of_find_property(np, DIRECT64_PROPNAME, NULL);
> > > > - if (!win64)
> > > > - return;
> > > > -
> > > > - if (ret || win64->length < sizeof(*dwp))
> > > > - goto delprop;
> > > > + int ret;
> > > >
> > > > - dwp = win64->value;
> > > > + dwp = win->value;
> > > > liobn = (u64)be32_to_cpu(dwp->liobn);
> > > >
> > > > /* clear the whole window, note the arg is in kernel pages */
> > > > @@ -793,24 +782,44 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> > > > 1ULL << (be32_to_cpu(dwp->window_shift) - PAGE_SHIFT), dwp);
> > > > if (ret)
> > > > pr_warn("%pOF failed to clear tces in window.\n",
> > > > - np);
> > > > + pdn);
> > > > else
> > > > pr_debug("%pOF successfully cleared tces in window.\n",
> > > > - np);
> > > > + pdn);
> > > >
> > > > ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn);
> > > > if (ret)
> > > > pr_warn("%pOF: failed to remove direct window: rtas returned "
> > > > "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> > > > - np, ret, ddw_avail[2], liobn);
> > > > + pdn, ret, ddw_avail[2], liobn);
> > > > else
> > > > pr_debug("%pOF: successfully removed direct window: rtas returned "
> > > > "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> > > > - np, ret, ddw_avail[2], liobn);
> > > > + pdn, ret, ddw_avail[2], liobn);
> > > > +}
> > > > +
> > > > +static void remove_ddw(struct device_node *np, bool remove_prop)
> > > > +{
> > > > + struct property *win;
> > > > + u32 ddw_avail[3];
> > > > + int ret = 0;
> > > > +
> > > > + ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
> > > > + &ddw_avail[0], 3);
> > > > + if (ret)
> > > > + return;
> > > > +
> > > > + win = of_find_property(np, DIRECT64_PROPNAME, NULL);
> > > > + if (!win)
> > > > + return;
> > > > +
> > > > + if (win->length >= sizeof(struct dynamic_dma_window_prop))
> > >
> > > Any good reason not to make it "=="? Is there something optional or we
> > > expect extension (which may not grow from the end but may add cells in
> > > between). Thanks,
> >
> > Well, it comes from the old behavior of remove_ddw():
> > - if (ret || win64->length < sizeof(*dwp))
> > - goto delprop;
> > As I reversed the logic from 'if (test) go out' to 'if (!test) do
> > stuff', I also reversed (a < b) to !(a < b) => (a >= b).
> >
> > I have no problem changing that to '==', but it will produce a
> > different behavior than before.
>
> I missed than, never mind then.
>
>
> > >
> > > > + remove_dma_window(np, ddw_avail, win);
> > > > +
> > > > + if (!remove_prop)
> > > > + return;
> > > >
> > > > -delprop:
> > > > - if (remove_prop)
> > > > - ret = of_remove_property(np, win64);
> > > > + ret = of_remove_property(np, win);
> > > > if (ret)
> > > > pr_warn("%pOF: failed to remove direct window property: %d\n",
> > > > np, ret);
> > > >
> >
> > Best regards,
> > Leonardo
> >
^ 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