* Re: [PATCH] powerpc/powernv/pci: Fix typo when releasing DMA resources
From: Frederic Barrat @ 2020-08-19 13:31 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <CAOSf1CG=ecx53ySsmX-7pK_zUJsd9LRMX5V1HBhYZBcOT2z89w@mail.gmail.com>
Le 19/08/2020 à 15:15, Oliver O'Halloran a écrit :
> On Wed, Aug 19, 2020 at 11:07 PM Frederic Barrat <fbarrat@linux.ibm.com> wrote:
>>
>> Fix typo introduced during recent code cleanup, which could lead to
>> silently not freeing resources or oops message (on PCI hotplug or CAPI
>> reset).
>
> oof
>
> Did you actually hit that oops on CAPI reset? Including the stack
> trace is helpful for avoiding this sort of problem in the future.
> Anyway,
yeah, I found it with capi reset. It's actually not a oops, we hit the
WARN_ON in iommu_tce_table_put(), when we try to release the DMA
resource for a PE where it was never configured to start with. So that
was easy to track.
Fred
> Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
>
>> Only impacts ioda2, the code path for ioda1 is correct.
>
> yeah but no ones uses ioda1
>
>> Fixes: 01e12629af4e ("powerpc/powernv/pci: Add explicit tracking of the DMA setup state")
>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index c9c25fb0783c..023a4f987bb2 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -2705,7 +2705,7 @@ void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe)
>> struct iommu_table *tbl = pe->table_group.tables[0];
>> int64_t rc;
>>
>> - if (pe->dma_setup_done)
>> + if (!pe->dma_setup_done)
>> return;
>>
>> rc = pnv_pci_ioda2_unset_window(&pe->table_group, 0);
>> --
>> 2.26.2
>>
^ permalink raw reply
* Re: [PATCH v2 00/13] mm/debug_vm_pgtable fixes
From: Aneesh Kumar K.V @ 2020-08-19 13:45 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <20200819130107.478414-1-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> This patch series includes fixes for debug_vm_pgtable test code so that
> they follow page table updates rules correctly. The first two patches introduce
> changes w.r.t ppc64. The patches are included in this series for completeness. We can
> merge them via ppc64 tree if required.
>
> Hugetlb test is disabled on ppc64 because that needs larger change to satisfy
> page table update rules.
>
> Changes from V1:
> * Address review feedback
> * drop test specific pfn_pte and pfn_pmd.
> * Update ppc64 page table helper to add _PAGE_PTE
>
> Aneesh Kumar K.V (13):
> powerpc/mm: Add DEBUG_VM WARN for pmd_clear
> powerpc/mm: Move setting pte specific flags to pfn_pte
> mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value
> mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge
> vmap support.
> mm/debug_vm_pgtable/savedwrite: Enable savedwrite test with
> CONFIG_NUMA_BALANCING
> mm/debug_vm_pgtable/THP: Mark the pte entry huge before using
> set_pmd/pud_at
> mm/debug_vm_pgtable/set_pte/pmd/pud: Don't use set_*_at to update an
> existing pte entry
> mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
> mm/debug_vm_pgtable/locks: Move non page table modifying test together
> mm/debug_vm_pgtable/locks: Take correct page table lock
> mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries
> mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
> mm/debug_vm_pgtable: populate a pte entry before fetching it
>
> arch/powerpc/include/asm/book3s/64/pgtable.h | 29 +++-
> arch/powerpc/include/asm/nohash/pgtable.h | 5 -
> arch/powerpc/mm/book3s64/pgtable.c | 2 +-
> arch/powerpc/mm/pgtable.c | 5 -
> include/linux/io.h | 12 ++
> mm/debug_vm_pgtable.c | 151 +++++++++++--------
> 6 files changed, 127 insertions(+), 77 deletions(-)
>
BTW I picked a wrong branch when sending this. Attaching the diff
against what I want to send. pfn_pmd() no more updates _PAGE_PTE
because that is handled by pmd_mkhuge().
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index 3b4da7c63e28..e18ae50a275c 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -141,7 +141,7 @@ pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
unsigned long pmdv;
pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
- return __pmd(pmdv | pgprot_val(pgprot) | _PAGE_PTE);
+ return pmd_set_protbits(__pmd(pmdv), pgprot);
}
pmd_t mk_pmd(struct page *page, pgprot_t pgprot)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 7d9f8e1d790f..cad61d22f33a 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -229,7 +229,7 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
{
- pmd_t pmd = pfn_pmd(pfn, prot);
+ pmd_t pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
return;
^ permalink raw reply related
* Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state
From: peterz @ 2020-08-19 15:32 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linux-arch, linuxppc-dev, linux-kernel, Nicholas Piggin,
Ingo Molnar, Will Deacon
In-Reply-To: <7fadb5ab-9869-396d-ff5d-c0adb6fc0b35@ozlabs.ru>
On Wed, Aug 19, 2020 at 08:39:13PM +1000, Alexey Kardashevskiy wrote:
> > or current upstream?
>
> The upstream 18445bf405cb (13 hours old) also shows the problem. Yours
> 1/2 still fixes it.
Afaict that just reduces the window.
Isn't the problem that:
arch/powerpc/kernel/exceptions-64e.S
START_EXCEPTION(perfmon);
NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR,
PROLOG_ADDITION_NONE)
EXCEPTION_COMMON(0x260)
INTS_DISABLE
# RECONCILE_IRQ_STATE
# TRACE_DISABLE_INTS
# TRACE_WITH_FRAME_BUFFER(trace_hardirqs_off)
#
# but we haven't done nmi_enter() yet... whoopsy
CHECK_NAPPING()
addi r3,r1,STACK_FRAME_OVERHEAD
bl performance_monitor_exception
# perf_irq()
# perf_event_interrupt
# __perf_event_interrupt
# nmi_enter()
That is, afaict your entry code is buggered.
^ permalink raw reply
* Re: [PATCH 1/2] lockdep: improve current->(hard|soft)irqs_enabled synchronisation with actual irq state
From: peterz @ 2020-08-19 15:39 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linux-arch, linuxppc-dev, linux-kernel, Nicholas Piggin,
Ingo Molnar, Will Deacon
In-Reply-To: <20200819153250.GF2674@hirez.programming.kicks-ass.net>
On Wed, Aug 19, 2020 at 05:32:50PM +0200, peterz@infradead.org wrote:
> On Wed, Aug 19, 2020 at 08:39:13PM +1000, Alexey Kardashevskiy wrote:
>
> > > or current upstream?
> >
> > The upstream 18445bf405cb (13 hours old) also shows the problem. Yours
> > 1/2 still fixes it.
>
> Afaict that just reduces the window.
>
> Isn't the problem that:
>
> arch/powerpc/kernel/exceptions-64e.S
>
> START_EXCEPTION(perfmon);
> NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR,
> PROLOG_ADDITION_NONE)
> EXCEPTION_COMMON(0x260)
> INTS_DISABLE
> # RECONCILE_IRQ_STATE
> # TRACE_DISABLE_INTS
> # TRACE_WITH_FRAME_BUFFER(trace_hardirqs_off)
> #
> # but we haven't done nmi_enter() yet... whoopsy
>
> CHECK_NAPPING()
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl performance_monitor_exception
> # perf_irq()
> # perf_event_interrupt
> # __perf_event_interrupt
> # nmi_enter()
>
>
>
> That is, afaict your entry code is buggered.
That is, patch 1/2 doesn't change the case:
local_irq_enable()
trace_hardirqs_on()
<NMI>
trace_hardirqs_off()
...
if (regs_irqs_disabled(regs)) // false
trace_hardirqs_on();
</NMI>
raw_local_irq_enable()
Where local_irq_enable() has done trace_hardirqs_on() and the NMI hits
and undoes it, but doesn't re-do it because the hardware state is still
disabled.
What's supposed to happen is:
<NMI>
nmi_enter()
trace_hardirqs_off() // no-op, because in_nmi() (or previously because lockdep_off())
...
</NMI>
^ permalink raw reply
* Re: [PATCH v2 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support.
From: kernel test robot @ 2020-08-19 16:29 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: Aneesh Kumar K.V, kbuild-all, linuxppc-dev, Anshuman Khandual
In-Reply-To: <20200819130107.478414-5-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 4871 bytes --]
Hi "Aneesh,
I love your patch! Yet something to improve:
[auto build test ERROR on hnaz-linux-mm/master]
[also build test ERROR on powerpc/next linus/master v5.9-rc1 next-20200819]
[cannot apply to mmotm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Aneesh-Kumar-K-V/mm-debug_vm_pgtable-fixes/20200819-213446
base: https://github.com/hnaz/linux-mm master
config: i386-randconfig-s002-20200818 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-183-gaa6ede3b-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/x86/mm/ioremap.c:484:12: error: redefinition of 'arch_ioremap_p4d_supported'
484 | int __init arch_ioremap_p4d_supported(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/x86/mm/ioremap.c:12:
include/linux/io.h:41:19: note: previous definition of 'arch_ioremap_p4d_supported' was here
41 | static inline int arch_ioremap_p4d_supported(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/x86/mm/ioremap.c:489:12: error: redefinition of 'arch_ioremap_pud_supported'
489 | int __init arch_ioremap_pud_supported(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/x86/mm/ioremap.c:12:
include/linux/io.h:45:19: note: previous definition of 'arch_ioremap_pud_supported' was here
45 | static inline int arch_ioremap_pud_supported(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/x86/mm/ioremap.c:498:12: error: redefinition of 'arch_ioremap_pmd_supported'
498 | int __init arch_ioremap_pmd_supported(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/x86/mm/ioremap.c:12:
include/linux/io.h:49:19: note: previous definition of 'arch_ioremap_pmd_supported' was here
49 | static inline int arch_ioremap_pmd_supported(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/mm/ioremap.c:737:17: warning: no previous prototype for 'early_memremap_pgprot_adjust' [-Wmissing-prototypes]
737 | pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
# https://github.com/0day-ci/linux/commit/260b675444a7d5afa9ccef47ca9e588fb18d01a3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Aneesh-Kumar-K-V/mm-debug_vm_pgtable-fixes/20200819-213446
git checkout 260b675444a7d5afa9ccef47ca9e588fb18d01a3
vim +/arch_ioremap_p4d_supported +484 arch/x86/mm/ioremap.c
^1da177e4c3f41 arch/i386/mm/ioremap.c Linus Torvalds 2005-04-16 483
0f472d04f59ff8 arch/x86/mm/ioremap.c Anshuman Khandual 2019-07-16 @484 int __init arch_ioremap_p4d_supported(void)
0f472d04f59ff8 arch/x86/mm/ioremap.c Anshuman Khandual 2019-07-16 485 {
0f472d04f59ff8 arch/x86/mm/ioremap.c Anshuman Khandual 2019-07-16 486 return 0;
0f472d04f59ff8 arch/x86/mm/ioremap.c Anshuman Khandual 2019-07-16 487 }
0f472d04f59ff8 arch/x86/mm/ioremap.c Anshuman Khandual 2019-07-16 488
1e6277de3a2337 arch/x86/mm/ioremap.c Jan Beulich 2015-05-28 @489 int __init arch_ioremap_pud_supported(void)
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 490 {
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 491 #ifdef CONFIG_X86_64
b8291adc191abe arch/x86/mm/ioremap.c Borislav Petkov 2016-03-29 492 return boot_cpu_has(X86_FEATURE_GBPAGES);
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 493 #else
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 494 return 0;
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 495 #endif
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 496 }
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 497
1e6277de3a2337 arch/x86/mm/ioremap.c Jan Beulich 2015-05-28 @498 int __init arch_ioremap_pmd_supported(void)
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 499 {
16bf92261b1b6c arch/x86/mm/ioremap.c Borislav Petkov 2016-03-29 500 return boot_cpu_has(X86_FEATURE_PSE);
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 501 }
5d72b4fba40ef4 arch/x86/mm/ioremap.c Toshi Kani 2015-04-14 502
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34736 bytes --]
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
From: Thiago Jung Bauermann @ 2020-08-19 16:50 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Konrad Rzeszutek Wilk, linuxppc-dev, Ram Pai, linux-kernel, iommu,
Satheesh Rajendran, Robin Murphy, Marek Szyprowski
In-Reply-To: <20200819044351.GA19391@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote:
>> POWER secure guests (i.e., guests which use the Protection Execution
>> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but
>> they don't need the SWIOTLB memory to be in low addresses since the
>> hypervisor doesn't have any addressing limitation.
>>
>> This solves a SWIOTLB initialization problem we are seeing in secure guests
>> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved
>> memory, which leaves no space for SWIOTLB in low addresses.
>>
>> To do this, we use mostly the same code as swiotlb_init(), but allocate the
>> buffer using memblock_alloc() instead of memblock_alloc_low().
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Looks fine to me (except for the pointlessly long comment lines, but I've
> been told that's the powerpc way).
Thanks! Do I have your Reviewed-by?
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
From: Konrad Rzeszutek Wilk @ 2020-08-19 17:00 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linuxppc-dev, Ram Pai, linux-kernel, iommu, Satheesh Rajendran,
Robin Murphy, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20200818221126.391073-1-bauerman@linux.ibm.com>
On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote:
> POWER secure guests (i.e., guests which use the Protection Execution
> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but
> they don't need the SWIOTLB memory to be in low addresses since the
> hypervisor doesn't have any addressing limitation.
>
> This solves a SWIOTLB initialization problem we are seeing in secure guests
> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved
> memory, which leaves no space for SWIOTLB in low addresses.
>
> To do this, we use mostly the same code as swiotlb_init(), but allocate the
> buffer using memblock_alloc() instead of memblock_alloc_low().
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> arch/powerpc/include/asm/svm.h | 4 ++++
> arch/powerpc/mm/mem.c | 6 +++++-
> arch/powerpc/platforms/pseries/svm.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 35 insertions(+), 1 deletion(-)
>
> Changes from v2:
> - Panic if unable to allocate buffer, as suggested by Christoph.
>
> Changes from v1:
> - Open-code swiotlb_init() in arch-specific code, as suggested by
> Christoph.
>
> diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
> index 85580b30aba4..7546402d796a 100644
> --- a/arch/powerpc/include/asm/svm.h
> +++ b/arch/powerpc/include/asm/svm.h
> @@ -15,6 +15,8 @@ static inline bool is_secure_guest(void)
> return mfmsr() & MSR_S;
> }
>
> +void __init svm_swiotlb_init(void);
> +
> void dtl_cache_ctor(void *addr);
> #define get_dtl_cache_ctor() (is_secure_guest() ? dtl_cache_ctor : NULL)
>
> @@ -25,6 +27,8 @@ static inline bool is_secure_guest(void)
> return false;
> }
>
> +static inline void svm_swiotlb_init(void) {}
> +
> #define get_dtl_cache_ctor() NULL
>
> #endif /* CONFIG_PPC_SVM */
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index c2c11eb8dcfc..0f21bcb16405 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -50,6 +50,7 @@
> #include <asm/swiotlb.h>
> #include <asm/rtas.h>
> #include <asm/kasan.h>
> +#include <asm/svm.h>
>
> #include <mm/mmu_decl.h>
>
> @@ -290,7 +291,10 @@ void __init mem_init(void)
> * back to to-down.
> */
> memblock_set_bottom_up(true);
> - swiotlb_init(0);
> + if (is_secure_guest())
> + svm_swiotlb_init();
> + else
> + swiotlb_init(0);
> #endif
>
> high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
> diff --git a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c
> index 40c0637203d5..81085eb8f225 100644
> --- a/arch/powerpc/platforms/pseries/svm.c
> +++ b/arch/powerpc/platforms/pseries/svm.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/mm.h>
> +#include <linux/memblock.h>
> #include <asm/machdep.h>
> #include <asm/svm.h>
> #include <asm/swiotlb.h>
> @@ -34,6 +35,31 @@ static int __init init_svm(void)
> }
> machine_early_initcall(pseries, init_svm);
>
> +/*
> + * Initialize SWIOTLB. Essentially the same as swiotlb_init(), except that it
> + * can allocate the buffer anywhere in memory. Since the hypervisor doesn't have
> + * any addressing limitation, we don't need to allocate it in low addresses.
> + */
> +void __init svm_swiotlb_init(void)
> +{
> + unsigned char *vstart;
> + unsigned long bytes, io_tlb_nslabs;
> +
> + io_tlb_nslabs = (swiotlb_size_or_default() >> IO_TLB_SHIFT);
> + io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
> +
> + bytes = io_tlb_nslabs << IO_TLB_SHIFT;
> +
> + vstart = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE);
> + if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false))
> + return;
> +
> + if (io_tlb_start)
> + memblock_free_early(io_tlb_start,
> + PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
> + panic("SVM: Cannot allocate SWIOTLB buffer");
> +}
> +
> int set_memory_encrypted(unsigned long addr, int numpages)
> {
> if (!PAGE_ALIGNED(addr))
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: add a basic stop 0-3 driver for POWER10
From: Vaidyanathan Srinivasan @ 2020-08-19 17:32 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Gautham R . Shenoy, Michael Neuling, linuxppc-dev,
Pratik Rajesh Sampat, Ryan P Grimm
In-Reply-To: <20200819094700.493399-1-npiggin@gmail.com>
* Nicholas Piggin <npiggin@gmail.com> [2020-08-19 19:47:00]:
> This driver does not restore stop > 3 state, so it limits itself
> to states which do not lose full state or TB.
>
> The POWER10 SPRs are sufficiently different from P9 that it seems
> easier to split out the P10 code. The POWER10 deep sleep code
> (e.g., the BHRB restore) has been taken out, but it can be re-added
> when stop > 3 support is added.
>
> Cc: Ryan P Grimm <rgrimm@us.ibm.com>
> Cc: Michael Neuling <mikey@neuling.org>
> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Cc: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Tested-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
This patch series was tested on P9 and P10. Correct set of
stop states were discovered and enabled in the platform.
Further the SPRs saved and restored by this code has also been
tested along with cpuidle state tests and cpu offline/online tests.
Thank for patch.
--Vaidy
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: add a basic stop 0-3 driver for POWER10
From: Gautham R Shenoy @ 2020-08-19 17:36 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Ryan P Grimm, Michael Neuling, linuxppc-dev, Pratik Rajesh Sampat,
Gautham R . Shenoy
In-Reply-To: <20200819094700.493399-1-npiggin@gmail.com>
On Wed, Aug 19, 2020 at 07:47:00PM +1000, Nicholas Piggin wrote:
> This driver does not restore stop > 3 state, so it limits itself
> to states which do not lose full state or TB.
>
> The POWER10 SPRs are sufficiently different from P9 that it seems
> easier to split out the P10 code. The POWER10 deep sleep code
> (e.g., the BHRB restore) has been taken out, but it can be re-added
> when stop > 3 support is added.
MMCRA[BHRB] save/restore was in the shallow stop-state path. But we
can add it back later.
>
> Cc: Ryan P Grimm <rgrimm@us.ibm.com>
> Cc: Michael Neuling <mikey@neuling.org>
> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Cc: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Just a minor comment below. But otherwise the patch looks good to me.
[..snip..]
> @@ -1093,11 +1200,15 @@ int validate_psscr_val_mask(u64 *psscr_val, u64 *psscr_mask, u32 flags)
> * @dt_idle_states: Number of idle state entries
> * Returns 0 on success
> */
> -static void __init pnv_power9_idle_init(void)
> +static void __init pnv_arch300_idle_init(void)
> {
> u64 max_residency_ns = 0;
> int i;
>
> + /* stop is not really architected, we only have p9,p10 drivers */
> + if (!pvr_version_is(PVR_POWER10) && !pvr_version_is(PVR_POWER9))
> + return;
> +
> /*
> * pnv_deepest_stop_{val,mask} should be set to values corresponding to
> * the deepest stop state.
> @@ -1112,6 +1223,11 @@ static void __init pnv_power9_idle_init(void)
> struct pnv_idle_states_t *state = &pnv_idle_states[i];
> u64 psscr_rl = state->psscr_val & PSSCR_RL_MASK;
>
> + /* No deep loss driver implemented for POWER10 yet */
> + if (pvr_version_is(PVR_POWER10) &&
> + state->flags & (OPAL_PM_TIMEBASE_STOP|OPAL_PM_LOSE_FULL_CONTEXT))
> + continue;
> +
Should we have a pr_info() informing the user the kernel is skipping
over these stop states ?
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> if ((state->flags & OPAL_PM_TIMEBASE_STOP) &&
> (pnv_first_tb_loss_level > psscr_rl))
> pnv_first_tb_loss_level = psscr_rl;
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: add a basic stop 0-3 driver for POWER10
From: Pratik Sampat @ 2020-08-19 17:40 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Gautham R . Shenoy, Michael Neuling, Ryan P Grimm
In-Reply-To: <20200819094700.493399-1-npiggin@gmail.com>
On 19/08/20 3:17 pm, Nicholas Piggin wrote:
> This driver does not restore stop > 3 state, so it limits itself
> to states which do not lose full state or TB.
>
> The POWER10 SPRs are sufficiently different from P9 that it seems
> easier to split out the P10 code. The POWER10 deep sleep code
> (e.g., the BHRB restore) has been taken out, but it can be re-added
> when stop > 3 support is added.
>
> Cc: Ryan P Grimm <rgrimm@us.ibm.com>
> Cc: Michael Neuling <mikey@neuling.org>
> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Cc: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
Fixes: 8747bf36f312 ("powerpc/powernv/idle: Replace CPU feature check
with PVR check")
The patch fixes the commit 8747bf36f312 which had setup idle for only
for the P9 PVR.
---
Thanks for fixing this!
Pratik
^ permalink raw reply
* Re: [PATCH net-next 1/5] ibmvnic: print caller in several error messages
From: David Miller @ 2020-08-19 19:08 UTC (permalink / raw)
To: ljp; +Cc: netdev, linuxppc-dev
In-Reply-To: <20200819053512.3619-2-ljp@linux.ibm.com>
From: Lijun Pan <ljp@linux.ibm.com>
Date: Wed, 19 Aug 2020 00:35:08 -0500
> The error messages in the changed functions are exactly the same.
> In order to differentiate them and make debugging easier,
> we print the function names in the error messages.
>
> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
I don't see any value in emitting function names in kernel
error log messages.
Sorry.
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: add a basic stop 0-3 driver for POWER10
From: Pratik Sampat @ 2020-08-19 19:08 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Gautham R . Shenoy, Michael Neuling, Ryan P Grimm
In-Reply-To: <20200819094700.493399-1-npiggin@gmail.com>
On 19/08/20 3:17 pm, Nicholas Piggin wrote:
> This driver does not restore stop > 3 state, so it limits itself
> to states which do not lose full state or TB.
>
> The POWER10 SPRs are sufficiently different from P9 that it seems
> easier to split out the P10 code. The POWER10 deep sleep code
> (e.g., the BHRB restore) has been taken out, but it can be re-added
> when stop > 3 support is added.
>
> Cc: Ryan P Grimm <rgrimm@us.ibm.com>
> Cc: Michael Neuling <mikey@neuling.org>
> Cc: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Cc: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
Tested-by: Pratik Rajesh Sampat<psampat@linux.ibm.com>
Reviewed-by: Pratik Rajesh Sampat<psampat@linux.ibm.com>
I've tested the patch on P9 and P10, can verify the corresponding code
paths are being taken for their respective architecture and cpuidle is
being exercised with the correct set of advertised stop states in each
case.
The patch looks good to me.
---
Thanks,
Pratik
^ permalink raw reply
* Re: [PATCH net-next 3/5] ibmvnic: improve ibmvnic_init and ibmvnic_reset_init
From: David Miller @ 2020-08-19 19:09 UTC (permalink / raw)
To: ljp; +Cc: netdev, linuxppc-dev
In-Reply-To: <20200819053512.3619-4-ljp@linux.ibm.com>
From: Lijun Pan <ljp@linux.ibm.com>
Date: Wed, 19 Aug 2020 00:35:10 -0500
> + if (rc) {
> + dev_err(dev, "%s: Send crq init failed with error %d\n", __func__, rc);
> + return rc;
Consistent with my feedback on patch #1, please get rid of this __func__ stuff.
Thank you.
^ permalink raw reply
* Re: [PATCH v3 09/17] memblock: make memblock_debug and related functionality private
From: Andrew Morton @ 2020-08-19 19:24 UTC (permalink / raw)
To: Mike Rapoport
Cc: Thomas Gleixner, Emil Renner Berthing, linux-sh, Peter Zijlstra,
Dave Hansen, linux-mips, Max Filippov, Paul Mackerras, sparclinux,
linux-riscv, Will Deacon, Christoph Hellwig, Marek Szyprowski,
linux-arch, linux-s390, linux-c6x-dev, Baoquan He, x86,
Russell King, Mike Rapoport, clang-built-linux, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, uclinux-h8-devel, linux-xtensa,
openrisc, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
Stafford Horne, Hari Bathini, Daniel Axtens, Michal Simek,
Yoshinori Sato, linux-mm, linux-kernel, iommu, Palmer Dabbelt,
linuxppc-dev
In-Reply-To: <20200818151634.14343-10-rppt@kernel.org>
On Tue, 18 Aug 2020 18:16:26 +0300 Mike Rapoport <rppt@kernel.org> wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
>
> The only user of memblock_dbg() outside memblock was s390 setup code and it
> is converted to use pr_debug() instead.
> This allows to stop exposing memblock_debug and memblock_dbg() to the rest
> of the kernel.
>
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -137,7 +137,10 @@ struct memblock_type physmem = {
> i < memblock_type->cnt; \
> i++, rgn = &memblock_type->regions[i])
>
> -int memblock_debug __initdata_memblock;
> +#define memblock_dbg(fmt, ...) \
> + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> +
checkpatch doesn't like this much.
ERROR: Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects
#101: FILE: mm/memblock.c:140:
+#define memblock_dbg(fmt, ...) \
+ if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
#102: FILE: mm/memblock.c:141:
+ if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
ERROR: trailing statements should be on next line
#102: FILE: mm/memblock.c:141:
+ if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
The first one is significant:
if (foo)
memblock_dbg(...);
else
save_the_world();
could end up inadvertently destroying the planet.
This?
--- a/mm/memblock.c~memblock-make-memblock_debug-and-related-functionality-private-fix
+++ a/mm/memblock.c
@@ -137,8 +137,11 @@ struct memblock_type physmem = {
i < memblock_type->cnt; \
i++, rgn = &memblock_type->regions[i])
-#define memblock_dbg(fmt, ...) \
- if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
+#define memblock_dbg(fmt, ...) \
+ do { \
+ if (memblock_debug) \
+ pr_info(fmt, ##__VA_ARGS__); \
+ } while (0)
static int memblock_debug __initdata_memblock;
static bool system_has_some_mirror __initdata_memblock = false;
_
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries/svm: Allocate SWIOTLB buffer anywhere in memory
From: Thiago Jung Bauermann @ 2020-08-19 19:48 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linuxppc-dev, Ram Pai, linux-kernel, iommu, Satheesh Rajendran,
Robin Murphy, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20200819170001.GC25497@char.us.oracle.com>
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> writes:
> On Tue, Aug 18, 2020 at 07:11:26PM -0300, Thiago Jung Bauermann wrote:
>> POWER secure guests (i.e., guests which use the Protection Execution
>> Facility) need to use SWIOTLB to be able to do I/O with the hypervisor, but
>> they don't need the SWIOTLB memory to be in low addresses since the
>> hypervisor doesn't have any addressing limitation.
>>
>> This solves a SWIOTLB initialization problem we are seeing in secure guests
>> with 128 GB of RAM: they are configured with 4 GB of crashkernel reserved
>> memory, which leaves no space for SWIOTLB in low addresses.
>>
>> To do this, we use mostly the same code as swiotlb_init(), but allocate the
>> buffer using memblock_alloc() instead of memblock_alloc_low().
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Thanks!
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v3 09/17] memblock: make memblock_debug and related functionality private
From: Mike Rapoport @ 2020-08-19 20:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Thomas Gleixner, Emil Renner Berthing, linux-sh, Peter Zijlstra,
Dave Hansen, linux-mips, Max Filippov, Paul Mackerras, sparclinux,
linux-riscv, Will Deacon, Christoph Hellwig, Marek Szyprowski,
linux-arch, linux-s390, linux-c6x-dev, Baoquan He, x86,
Russell King, Mike Rapoport, clang-built-linux, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, uclinux-h8-devel, linux-xtensa,
openrisc, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
Stafford Horne, Hari Bathini, Daniel Axtens, Michal Simek,
Yoshinori Sato, linux-mm, linux-kernel, iommu, Palmer Dabbelt,
linuxppc-dev
In-Reply-To: <20200819122405.88e9719e86ac7c3c44b4db32@linux-foundation.org>
On Wed, Aug 19, 2020 at 12:24:05PM -0700, Andrew Morton wrote:
> On Tue, 18 Aug 2020 18:16:26 +0300 Mike Rapoport <rppt@kernel.org> wrote:
>
> > From: Mike Rapoport <rppt@linux.ibm.com>
> >
> > The only user of memblock_dbg() outside memblock was s390 setup code and it
> > is converted to use pr_debug() instead.
> > This allows to stop exposing memblock_debug and memblock_dbg() to the rest
> > of the kernel.
> >
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -137,7 +137,10 @@ struct memblock_type physmem = {
> > i < memblock_type->cnt; \
> > i++, rgn = &memblock_type->regions[i])
> >
> > -int memblock_debug __initdata_memblock;
> > +#define memblock_dbg(fmt, ...) \
> > + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> > +
>
> checkpatch doesn't like this much.
>
> ERROR: Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects
> #101: FILE: mm/memblock.c:140:
> +#define memblock_dbg(fmt, ...) \
> + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>
> WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
> #102: FILE: mm/memblock.c:141:
> + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>
> ERROR: trailing statements should be on next line
> #102: FILE: mm/memblock.c:141:
> + if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
>
>
> The first one is significant:
>
> if (foo)
> memblock_dbg(...);
> else
> save_the_world();
>
> could end up inadvertently destroying the planet.
Well, it didn't till now ;-)
> This?
>
> --- a/mm/memblock.c~memblock-make-memblock_debug-and-related-functionality-private-fix
> +++ a/mm/memblock.c
> @@ -137,8 +137,11 @@ struct memblock_type physmem = {
> i < memblock_type->cnt; \
> i++, rgn = &memblock_type->regions[i])
>
> -#define memblock_dbg(fmt, ...) \
> - if (memblock_debug) printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> +#define memblock_dbg(fmt, ...) \
> + do { \
> + if (memblock_debug) \
> + pr_info(fmt, ##__VA_ARGS__); \
> + } while (0)
Sure, thanks!
> static int memblock_debug __initdata_memblock;
> static bool system_has_some_mirror __initdata_memblock = false;
> _
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: fsl_espi errors on v5.7.15
From: Chris Packham @ 2020-08-19 21:08 UTC (permalink / raw)
To: Heiner Kallweit, broonie@kernel.org, mpe@ellerman.id.au,
benh@kernel.crashing.org, paulus@samba.org,
tiago.brusamarello@datacom.ind.br
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org
In-Reply-To: <61bb9800-9f90-9cd4-3b17-c14a7f83d792@gmail.com>
On 19/08/20 6:15 pm, Heiner Kallweit wrote:
> On 19.08.2020 00:44, Chris Packham wrote:
>> Hi Again,
>>
>> On 17/08/20 9:09 am, Chris Packham wrote:
>>
>>> On 14/08/20 6:19 pm, Heiner Kallweit wrote:
>>>> On 14.08.2020 04:48, Chris Packham wrote:
>>>>> Hi,
>>>>>
>>>>> I'm seeing a problem with accessing spi-nor after upgrading a T2081
>>>>> based system to linux v5.7.15
>>>>>
>>>>> For this board u-boot and the u-boot environment live on spi-nor.
>>>>>
>>>>> When I use fw_setenv from userspace I get the following kernel logs
>>>>>
>>>>> # fw_setenv foo=1
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>>>>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>>>> ...
>>>>>
>>>> This error reporting doesn't exist yet in 4.4. So you may have an issue
>>>> under 4.4 too, it's just not reported.
>>>> Did you verify that under 4.4 fw_setenv actually has an effect?
>>> Just double checked and yes under 4.4 the setting does get saved.
>>>>> If I run fw_printenv (before getting it into a bad state) it is able to
>>>>> display the content of the boards u-boot environment.
>>>>>
>>>> This might indicate an issue with spi being locked. I've seen related
>>>> questions, just use the search engine of your choice and check for
>>>> fw_setenv and locked.
>>> I'm running a version of fw_setenv which includes
>>> https://gitlab.denx.de/u-boot/u-boot/-/commit/db820159 so it shouldn't
>>> be locking things unnecessarily.
>>>>> If been unsuccessful in producing a setup for bisecting the issue. I do
>>>>> know the issue doesn't occur on the old 4.4.x based kernel but that's
>>>>> probably not much help.
>>>>>
>>>>> Any pointers on what the issue (and/or solution) might be.
>> I finally managed to get our board running with a vanilla kernel. With
>> corenet64_smp_defconfig I occasionally see
>>
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>>
>> other than the message things seem to be working.
>>
>> With a custom defconfig I see
>>
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>> ...
>>
>> and access to the spi-nor does not work until the board is reset.
>>
>> I'll try and pick apart the differences between the two defconfigs.
I now think my earlier testing is invalid. I have seen the problem with
either defconfig if I try hard enough. I had convinced myself that the
problem was CONFIG_PREEMPT but that was before I found boot-to-boot
differences with the same kernel.
It's possible that I'm chasing multiple issues with the same symptom.
The error I'm most concerned with is in the sequence
1. boot with old image
2. write environment
3. boot with new image
4. write environment
5. write fails and environment is corrupted
After I recover the system things sometimes seem fine. Until I repeat
the sequence above.
> Also relevant may be:
> - Which dts are you using?
Custom but based heavily on the t2080rdb.
> - What's the spi-nor type, and at which frequency are you operating it?
The board has several alternate parts for the spi-nor so the dts just
specifies compatible = "jedec,spi-nor" the actual chip detected on the
board I have is "n25q032a (4096 Kbytes)". The dts sets spi-max-frequency
= <10000000> I haven't measured the actual frequency on the bus.
> - Does the issue still happen if you lower the frequency?
I did play around with the frequency initially but I should probably
give that another go now that I have a better reproduction method.
^ permalink raw reply
* [PATCH net-next v2 0/4] refactoring of ibmvnic code
From: Lijun Pan @ 2020-08-19 22:52 UTC (permalink / raw)
To: netdev; +Cc: Lijun Pan, linuxppc-dev
This patch series refactor reset_init and init functions,
and make some other cosmetic changes to make the code
easier to read and debug. v2 removes __func__ and v1's 1/5.
Lijun Pan (4):
ibmvnic: compare adapter->init_done_rc with more readable
ibmvnic_rc_codes
ibmvnic: improve ibmvnic_init and ibmvnic_reset_init
ibmvnic: remove never executed if statement
ibmvnic: merge ibmvnic_reset_init and ibmvnic_init
drivers/net/ethernet/ibm/ibmvnic.c | 84 ++++++++----------------------
1 file changed, 21 insertions(+), 63 deletions(-)
--
2.23.0
^ permalink raw reply
* [PATCH net-next v2 1/4] ibmvnic: compare adapter->init_done_rc with more readable ibmvnic_rc_codes
From: Lijun Pan @ 2020-08-19 22:52 UTC (permalink / raw)
To: netdev; +Cc: Lijun Pan, linuxppc-dev
In-Reply-To: <20200819225226.10152-1-ljp@linux.ibm.com>
Instead of comparing (adapter->init_done_rc == 1), let it
be (adapter->init_done_rc == PARTIALSUCCESS).
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5afb3c9c52d2..65f5d99f97dc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -974,7 +974,7 @@ static int set_link_state(struct ibmvnic_adapter *adapter, u8 link_state)
return -1;
}
- if (adapter->init_done_rc == 1) {
+ if (adapter->init_done_rc == PARTIALSUCCESS) {
/* Partuial success, delay and re-send */
mdelay(1000);
resend = true;
--
2.23.0
^ permalink raw reply related
* [PATCH net-next v2 2/4] ibmvnic: improve ibmvnic_init and ibmvnic_reset_init
From: Lijun Pan @ 2020-08-19 22:52 UTC (permalink / raw)
To: netdev; +Cc: Lijun Pan, linuxppc-dev
In-Reply-To: <20200819225226.10152-1-ljp@linux.ibm.com>
When H_SEND_CRQ command returns with H_CLOSED, it means the
server's CRQ is not ready yet. Instead of resetting immediately,
we wait for the server to launch passive init.
ibmvnic_init() and ibmvnic_reset_init() should also return the
error code from ibmvnic_send_crq_init() call.
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
v2: removes __func__ in error messages.
drivers/net/ethernet/ibm/ibmvnic.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 65f5d99f97dc..644352e5056d 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3568,8 +3568,7 @@ static int ibmvnic_send_crq(struct ibmvnic_adapter *adapter,
if (rc) {
if (rc == H_CLOSED) {
dev_warn(dev, "CRQ Queue closed\n");
- if (test_bit(0, &adapter->resetting))
- ibmvnic_reset(adapter, VNIC_RESET_FATAL);
+ /* do not reset, report the fail, wait for passive init from server */
}
dev_warn(dev, "Send error (rc=%d)\n", rc);
@@ -4985,7 +4984,12 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
reinit_completion(&adapter->init_done);
adapter->init_done_rc = 0;
- ibmvnic_send_crq_init(adapter);
+ rc = ibmvnic_send_crq_init(adapter);
+ if (rc) {
+ dev_err(dev, "Send crq init failed with error %d\n", rc);
+ return rc;
+ }
+
if (!wait_for_completion_timeout(&adapter->init_done, timeout)) {
dev_err(dev, "Initialization sequence timed out\n");
return -1;
@@ -5039,7 +5043,12 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
adapter->from_passive_init = false;
adapter->init_done_rc = 0;
- ibmvnic_send_crq_init(adapter);
+ rc = ibmvnic_send_crq_init(adapter);
+ if (rc) {
+ dev_err(dev, "Send crq init failed with error %d\n", rc);
+ return rc;
+ }
+
if (!wait_for_completion_timeout(&adapter->init_done, timeout)) {
dev_err(dev, "Initialization sequence timed out\n");
return -1;
--
2.23.0
^ permalink raw reply related
* [PATCH net-next v2 3/4] ibmvnic: remove never executed if statement
From: Lijun Pan @ 2020-08-19 22:52 UTC (permalink / raw)
To: netdev; +Cc: Lijun Pan, linuxppc-dev
In-Reply-To: <20200819225226.10152-1-ljp@linux.ibm.com>
At the beginning of the function, from_passive_init is set false by
"adapter->from_passive_init = false;",
hence the if statement will never run.
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 644352e5056d..4ca4647db72a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -5000,12 +5000,6 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
return adapter->init_done_rc;
}
- if (adapter->from_passive_init) {
- adapter->state = VNIC_OPEN;
- adapter->from_passive_init = false;
- return -1;
- }
-
if (test_bit(0, &adapter->resetting) && !adapter->wait_for_reset &&
adapter->reset_reason != VNIC_RESET_MOBILITY) {
if (adapter->req_rx_queues != old_num_rx_queues ||
@@ -5059,12 +5053,6 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
return adapter->init_done_rc;
}
- if (adapter->from_passive_init) {
- adapter->state = VNIC_OPEN;
- adapter->from_passive_init = false;
- return -1;
- }
-
rc = init_sub_crqs(adapter);
if (rc) {
dev_err(dev, "Initialization of sub crqs failed\n");
--
2.23.0
^ permalink raw reply related
* [PATCH net-next v2 4/4] ibmvnic: merge ibmvnic_reset_init and ibmvnic_init
From: Lijun Pan @ 2020-08-19 22:52 UTC (permalink / raw)
To: netdev; +Cc: Lijun Pan, linuxppc-dev
In-Reply-To: <20200819225226.10152-1-ljp@linux.ibm.com>
These two functions share the majority of the code, hence merge
them together. In the meanwhile, add a reset pass-in parameter
to differentiate them. Thus, the code is easier to read and to tell
the difference between reset_init and regular init.
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 65 ++++++------------------------
1 file changed, 13 insertions(+), 52 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 4ca4647db72a..47fbe0553570 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -104,8 +104,7 @@ static int send_login(struct ibmvnic_adapter *adapter);
static void send_cap_queries(struct ibmvnic_adapter *adapter);
static int init_sub_crqs(struct ibmvnic_adapter *);
static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter);
-static int ibmvnic_init(struct ibmvnic_adapter *);
-static int ibmvnic_reset_init(struct ibmvnic_adapter *);
+static int ibmvnic_reset_init(struct ibmvnic_adapter *, bool reset);
static void release_crq_queue(struct ibmvnic_adapter *);
static int __ibmvnic_set_mac(struct net_device *, u8 *);
static int init_crq_queue(struct ibmvnic_adapter *adapter);
@@ -1868,7 +1867,7 @@ static int do_change_param_reset(struct ibmvnic_adapter *adapter,
return rc;
}
- rc = ibmvnic_reset_init(adapter);
+ rc = ibmvnic_reset_init(adapter, true);
if (rc)
return IBMVNIC_INIT_FAILED;
@@ -1986,7 +1985,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
goto out;
}
- rc = ibmvnic_reset_init(adapter);
+ rc = ibmvnic_reset_init(adapter, true);
if (rc) {
rc = IBMVNIC_INIT_FAILED;
goto out;
@@ -2093,7 +2092,7 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter,
return rc;
}
- rc = ibmvnic_init(adapter);
+ rc = ibmvnic_reset_init(adapter, false);
if (rc)
return rc;
@@ -4970,7 +4969,7 @@ static int init_crq_queue(struct ibmvnic_adapter *adapter)
return retrc;
}
-static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
+static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter, bool reset)
{
struct device *dev = &adapter->vdev->dev;
unsigned long timeout = msecs_to_jiffies(30000);
@@ -4979,10 +4978,12 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
adapter->from_passive_init = false;
- old_num_rx_queues = adapter->req_rx_queues;
- old_num_tx_queues = adapter->req_tx_queues;
+ if (reset) {
+ old_num_rx_queues = adapter->req_rx_queues;
+ old_num_tx_queues = adapter->req_tx_queues;
+ reinit_completion(&adapter->init_done);
+ }
- reinit_completion(&adapter->init_done);
adapter->init_done_rc = 0;
rc = ibmvnic_send_crq_init(adapter);
if (rc) {
@@ -5000,7 +5001,8 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
return adapter->init_done_rc;
}
- if (test_bit(0, &adapter->resetting) && !adapter->wait_for_reset &&
+ if (reset &&
+ test_bit(0, &adapter->resetting) && !adapter->wait_for_reset &&
adapter->reset_reason != VNIC_RESET_MOBILITY) {
if (adapter->req_rx_queues != old_num_rx_queues ||
adapter->req_tx_queues != old_num_tx_queues) {
@@ -5028,47 +5030,6 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
return rc;
}
-static int ibmvnic_init(struct ibmvnic_adapter *adapter)
-{
- struct device *dev = &adapter->vdev->dev;
- unsigned long timeout = msecs_to_jiffies(30000);
- int rc;
-
- adapter->from_passive_init = false;
-
- adapter->init_done_rc = 0;
- rc = ibmvnic_send_crq_init(adapter);
- if (rc) {
- dev_err(dev, "Send crq init failed with error %d\n", rc);
- return rc;
- }
-
- if (!wait_for_completion_timeout(&adapter->init_done, timeout)) {
- dev_err(dev, "Initialization sequence timed out\n");
- return -1;
- }
-
- if (adapter->init_done_rc) {
- release_crq_queue(adapter);
- return adapter->init_done_rc;
- }
-
- rc = init_sub_crqs(adapter);
- if (rc) {
- dev_err(dev, "Initialization of sub crqs failed\n");
- release_crq_queue(adapter);
- return rc;
- }
-
- rc = init_sub_crq_irqs(adapter);
- if (rc) {
- dev_err(dev, "Failed to initialize sub crq irqs\n");
- release_crq_queue(adapter);
- }
-
- return rc;
-}
-
static struct device_attribute dev_attr_failover;
static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
@@ -5131,7 +5092,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
goto ibmvnic_init_fail;
}
- rc = ibmvnic_init(adapter);
+ rc = ibmvnic_reset_init(adapter, false);
if (rc && rc != EAGAIN)
goto ibmvnic_init_fail;
} while (rc == EAGAIN);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v2 3/4] powerpc/memhotplug: Make lmb size 64bit
From: Sasha Levin @ 2020-08-19 23:56 UTC (permalink / raw)
To: Sasha Levin, Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Nathan Lynch, stable
In-Reply-To: <20200806162329.276534-3-aneesh.kumar@linux.ibm.com>
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.8.1, v5.7.15, v5.4.58, v4.19.139, v4.14.193, v4.9.232, v4.4.232.
v5.8.1: Build OK!
v5.7.15: Build OK!
v5.4.58: Build OK!
v4.19.139: Failed to apply! Possible dependencies:
Unable to calculate
v4.14.193: Failed to apply! Possible dependencies:
Unable to calculate
v4.9.232: Failed to apply! Possible dependencies:
1a367063ca0c ("powerpc/pseries: Check memory device state before onlining/offlining")
25b587fba9a4 ("powerpc/pseries: Correct possible read beyond dlpar sysfs buffer")
333f7b76865b ("powerpc/pseries: Implement indexed-count hotplug memory add")
753843471cbb ("powerpc/pseries: Implement indexed-count hotplug memory remove")
943db62c316c ("powerpc/pseries: Revert 'Auto-online hotplugged memory'")
c21f515c7436 ("powerpc/pseries: Make the acquire/release of the drc for memory a seperate step")
e70d59700fc3 ("powerpc/pseries: Introduce memory hotplug READD operation")
f84775c2d5d9 ("powerpc/pseries: Fix build break when MEMORY_HOTREMOVE=n")
v4.4.232: Failed to apply! Possible dependencies:
183deeea5871 ("powerpc/pseries: Consolidate CPU hotplug code to hotplug-cpu.c")
1a367063ca0c ("powerpc/pseries: Check memory device state before onlining/offlining")
1dc759566636 ("powerpc/pseries: Use kernel hotplug queue for PowerVM hotplug events")
1f859adb9253 ("powerpc/pseries: Verify CPU doesn't exist before adding")
25b587fba9a4 ("powerpc/pseries: Correct possible read beyond dlpar sysfs buffer")
333f7b76865b ("powerpc/pseries: Implement indexed-count hotplug memory add")
4a4bdfea7cb7 ("powerpc/pseries: Refactor dlpar_add_lmb() code")
753843471cbb ("powerpc/pseries: Implement indexed-count hotplug memory remove")
9054619ef54a ("powerpc/pseries: Add pseries hotplug workqueue")
943db62c316c ("powerpc/pseries: Revert 'Auto-online hotplugged memory'")
9dc512819e4b ("powerpc: Fix unused function warning 'lmb_to_memblock'")
bdf5fc633804 ("powerpc/pseries: Update LMB associativity index during DLPAR add/remove")
c21f515c7436 ("powerpc/pseries: Make the acquire/release of the drc for memory a seperate step")
e70d59700fc3 ("powerpc/pseries: Introduce memory hotplug READD operation")
e9d764f80396 ("powerpc/pseries: Enable kernel CPU dlpar from sysfs")
ec999072442a ("powerpc/pseries: Auto-online hotplugged memory")
f84775c2d5d9 ("powerpc/pseries: Fix build break when MEMORY_HOTREMOVE=n")
fdb4f6e99ffa ("powerpc/pseries: Remove call to memblock_add()")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc/drmem: Make lmb_size 64 bit
From: Sasha Levin @ 2020-08-19 23:56 UTC (permalink / raw)
To: Sasha Levin, Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Nathan Lynch, stable
In-Reply-To: <20200806162329.276534-1-aneesh.kumar@linux.ibm.com>
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.8.1, v5.7.15, v5.4.58, v4.19.139, v4.14.193, v4.9.232, v4.4.232.
v5.8.1: Build OK!
v5.7.15: Build OK!
v5.4.58: Build OK!
v4.19.139: Build OK!
v4.14.193: Failed to apply! Possible dependencies:
22508f3dc985 ("powerpc/numa: Look up device node in of_get_usable_memory()")
2c77721552e5 ("powerpc: Move of_drconf_cell struct to asm/drmem.h")
35f80debaef0 ("powerpc/numa: Look up device node in of_get_assoc_arrays()")
514a9cb3316a ("powerpc/numa: Update numa code use walk_drmem_lmbs")
6195a5001f1d ("powerpc/pseries: Update memory hotplug code to use drmem LMB array")
6c6ea53725b3 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
b6eca183e23e ("powerpc/kernel: Enables memory hot-remove after reboot on pseries guests")
b88fc309d6ad ("powerpc/numa: Look up associativity array in of_drconf_to_nid_single")
v4.9.232: Failed to apply! Possible dependencies:
3a2df3798d4d ("powerpc/mm: Make switch_mm_irqs_off() out of line")
43ed84a891b7 ("powerpc/mm: Move pgdir setting into a helper")
5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
5d451a87e5eb ("powerpc/64: Retrieve number of L1 cache sets from device-tree")
6c6ea53725b3 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
70cd4c10b290 ("KVM: PPC: Book3S HV: Fix software walk of guest process page tables")
9b081e10805c ("powerpc: port 64 bits pgtable_cache to 32 bits")
a25bd72badfa ("powerpc/mm/radix: Workaround prefetch issue with KVM")
bd067f83b084 ("powerpc/64: Fix naming of cache block vs. cache line")
dbcbfee0c81c ("powerpc/64: More definitions for POWER9")
e2827fe5c156 ("powerpc/64: Clean up ppc64_caches using a struct per cache")
f4329f2ecb14 ("powerpc/64s: Reduce exception alignment")
v4.4.232: Failed to apply! Possible dependencies:
11a6f6abd74a ("powerpc/mm: Move radix/hash common data structures to book3s64 headers")
26b6a3d9bb48 ("powerpc/mm: move pte headers to book3s directory")
3808a88985b4 ("powerpc: Move FW feature probing out of pseries probe()")
3dfcb315d81e ("powerpc/mm: make a separate copy for book3s")
5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
5d31a96e6c01 ("powerpc/livepatch: Add livepatch stack to struct thread_info")
6574ba950bbe ("powerpc/kernel: Convert cpu_has_feature() to returning bool")
6c6ea53725b3 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
a141cca3892b ("powerpc/mm: Add early_[cpu|mmu]_has_feature()")
a8ed87c92adf ("powerpc/mm/radix: Add MMU_FTR_RADIX")
b92a226e5284 ("powerpc: Move cpu_has_feature() to a separate file")
da6a97bf12d5 ("powerpc: Move epapr_paravirt_early_init() to early_init_devtree()")
f63e6d898760 ("powerpc/livepatch: Add livepatch header")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/64s: remove PROT_SAO support
From: Nicholas Piggin @ 2020-08-20 1:05 UTC (permalink / raw)
To: linuxppc-dev, Shawn Anastasio
In-Reply-To: <3c053cc9-751c-9899-79ba-1013af140148@anastas.io>
Excerpts from Shawn Anastasio's message of August 19, 2020 6:59 am:
> On 8/18/20 2:11 AM, Nicholas Piggin wrote> Very reasonable point.
>>
>> The problem we're trying to get a handle on is live partition migration
>> where a running guest might be using SAO then get migrated to a P10. I
>> don't think we have a good way to handle this case. Potentially the
>> hypervisor could revoke the page tables if the guest is running in hash
>> mode and the guest kernel could be taught about that and sigbus the
>> process, but in radix the guest controls those page tables and the SAO
>> state and I don't think there's a way to cause it to take a fault.
>>
>> I also don't know what the proprietary hypervisor does here.
>>
>> We could add it back, default to n, or make it bare metal only, or
>> somehow try to block live migration to a later CPU without the faciliy.
>> I wouldn't be against that.
>
>
> Admittedly I'm not too familiar with the specifics of live migration
> or guest memory management, but restoring the functionality and adding
> a way to prevent migration of SAO-using guests seems like a reasonable
> choice to me. Would this be done with help from the guest using some
> sort of infrastructure to signal to the hypervisor that SAO is in use,
> or entirely on the hypervisor by e.g. scanning the through the process
> table for SAO pages?
The first step might be to just re-add the functionality but disable
it by default if firmware_has_feature(FW_FEATURE_LPAR). You could have
a config or boot option to allow guests to use it at the cost of
migration compatibility.
That would probably be good enough for experimenting with the feature.
I think modifying the hypervisor and/or guest to deal with migration
is probably too much work to be justified at the moment.
>> It would be very interesting to know how it performs in such a "real"
>> situation. I don't know how well POWER9 has optimised it -- it's
>> possible that it's not much better than putting lwsync after every load
>> or store.
>
>
> This is definitely worth investigating in depth. That said, even if the
> performance on P9 isn't super great, I think the feature could still be
> useful, since it would offer more granularity than the sledgehammer
> approach of emitting lwsync everywhere.
Sure, we'd be interested to hear of results.
> I'd be happy to put in some of the work required to get this to a point
> where it can be reintroduced without breaking guest migration - I'd just
> need some pointers on getting started with whatever approach is decided on.
I think re-adding it as I said above would be okay. The code itself is
not complex so that was not the reason for removal.
Thanks,
Nick
^ 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