* Re: [PATCH 0/2] Fix arm64 boot regression in 5.14
From: Marc Zyngier @ 2021-07-20 12:53 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Catalin Marinas, Andrew Morton, Jonathan Marek,
linux-kernel, Paul Mackerras, linux-arm-kernel,
Geert Uytterhoeven, Thomas Gleixner, linuxppc-dev, Ard Biesheuvel,
Mike Rapoport
In-Reply-To: <20210720123512.8740-1-will@kernel.org>
On 2021-07-20 13:35, Will Deacon wrote:
> Hi folks,
>
> Jonathan reports [1] that commit c742199a014d ("mm/pgtable: add stubs
> for {pmd/pub}_{set/clear}_huge") breaks the boot on arm64 when huge
> mappings are used to map the kernel linear map but the VA size is
> configured such that PUDs are folded. This is because the
> non-functional
> pud_set_huge() stub is used to create the linear map, which results in
> 1GB holes and a fatal data abort when the kernel attemps to access
> them.
>
> Digging further into the issue, it also transpired that huge-vmap is
> silently disabled in these configurations as well [2], despite working
> correctly in 5.13. The latter issue causes the pgtable selftests to
> scream due to a failing consistency check [3].
>
> Rather than leave mainline in a terminally broken state for arm64 while
> we figure this out, revert the offending commit to get things working
> again. Unfortunately, reverting the change in isolation causes a build
> breakage for 32-bit PowerPC 8xx machines which recently started relying
> on the problematic stubs to support pte-level huge-vmap entries [4].
> Since Christophe is away at the moment, this series first reverts the
> PowerPC 8xx change in order to avoid breaking the build.
>
> I would really like this to land for -rc3 and I can take these via the
> arm64 fixes queue if the PowerPC folks are alright with them.
>
> Cheers,
>
> Will
>
> [1] https://lore.kernel.org/r/20210717160118.9855-1-jonathan@marek.ca
> [2] https://lore.kernel.org/r/20210719104918.GA6440@willie-the-truck
> [3]
> https://lore.kernel.org/r/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/
> [4]
> https://lore.kernel.org/r/8b972f1c03fb6bd59953035f0a3e4d26659de4f8.1620795204.git.christophe.leroy@csgroup.eu/
>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Jonathan Marek <jonathan@marek.ca>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nicholas Piggin <npiggin@gmail.com
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-arm-kernel@lists.infradead.org
>
> --->8
>
> Jonathan Marek (1):
> Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"
>
> Will Deacon (1):
> Revert "powerpc/8xx: add support for huge pages on VMAP and VMALLOC"
>
> arch/arm64/mm/mmu.c | 20 ++++-----
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/include/asm/nohash/32/mmu-8xx.h | 43 --------------------
> arch/x86/mm/pgtable.c | 34 +++++++---------
> include/linux/pgtable.h | 26 +-----------
> 5 files changed, 25 insertions(+), 100 deletions(-)
Acked-by: Marc Zyngier <maz@kernel.org>
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH 0/2] Fix arm64 boot regression in 5.14
From: Ard Biesheuvel @ 2021-07-20 12:39 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Marc Zyngier, Catalin Marinas, Jonathan Marek,
Linux Kernel Mailing List, Paul Mackerras, Linux ARM,
Geert Uytterhoeven, Thomas Gleixner,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Andrew Morton,
Mike Rapoport
In-Reply-To: <20210720123512.8740-1-will@kernel.org>
On Tue, 20 Jul 2021 at 14:35, Will Deacon <will@kernel.org> wrote:
>
> Hi folks,
>
> Jonathan reports [1] that commit c742199a014d ("mm/pgtable: add stubs
> for {pmd/pub}_{set/clear}_huge") breaks the boot on arm64 when huge
> mappings are used to map the kernel linear map but the VA size is
> configured such that PUDs are folded. This is because the non-functional
> pud_set_huge() stub is used to create the linear map, which results in
> 1GB holes and a fatal data abort when the kernel attemps to access them.
>
> Digging further into the issue, it also transpired that huge-vmap is
> silently disabled in these configurations as well [2], despite working
> correctly in 5.13. The latter issue causes the pgtable selftests to
> scream due to a failing consistency check [3].
>
> Rather than leave mainline in a terminally broken state for arm64 while
> we figure this out, revert the offending commit to get things working
> again. Unfortunately, reverting the change in isolation causes a build
> breakage for 32-bit PowerPC 8xx machines which recently started relying
> on the problematic stubs to support pte-level huge-vmap entries [4].
> Since Christophe is away at the moment, this series first reverts the
> PowerPC 8xx change in order to avoid breaking the build.
>
> I would really like this to land for -rc3 and I can take these via the
> arm64 fixes queue if the PowerPC folks are alright with them.
>
> Cheers,
>
> Will
>
> [1] https://lore.kernel.org/r/20210717160118.9855-1-jonathan@marek.ca
> [2] https://lore.kernel.org/r/20210719104918.GA6440@willie-the-truck
> [3] https://lore.kernel.org/r/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/
> [4] https://lore.kernel.org/r/8b972f1c03fb6bd59953035f0a3e4d26659de4f8.1620795204.git.christophe.leroy@csgroup.eu/
>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Jonathan Marek <jonathan@marek.ca>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nicholas Piggin <npiggin@gmail.com
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-arm-kernel@lists.infradead.org
>
> --->8
>
> Jonathan Marek (1):
> Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"
>
> Will Deacon (1):
> Revert "powerpc/8xx: add support for huge pages on VMAP and VMALLOC"
>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> arch/arm64/mm/mmu.c | 20 ++++-----
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/include/asm/nohash/32/mmu-8xx.h | 43 --------------------
> arch/x86/mm/pgtable.c | 34 +++++++---------
> include/linux/pgtable.h | 26 +-----------
> 5 files changed, 25 insertions(+), 100 deletions(-)
>
> --
> 2.32.0.402.g57bb445576-goog
>
^ permalink raw reply
* [PATCH 2/2] Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"
From: Will Deacon @ 2021-07-20 12:35 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Rutland, Marc Zyngier, Catalin Marinas, Andrew Morton,
Jonathan Marek, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Geert Uytterhoeven, Thomas Gleixner, Will Deacon, Ard Biesheuvel,
Mike Rapoport
In-Reply-To: <20210720123512.8740-1-will@kernel.org>
From: Jonathan Marek <jonathan@marek.ca>
This reverts commit c742199a014de23ee92055c2473d91fe5561ffdf.
c742199a014d ("mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge")
breaks arm64 in at least two ways for configurations where PUD or PMD
folding occur:
1. We no longer install huge-vmap mappings and silently fall back to
page-granular entries, despite being able to install block entries
at what is effectively the PGD level.
2. If the linear map is backed with block mappings, these will now
silently fail to be created in alloc_init_pud(), causing a panic
early during boot.
The pgtable selftests caught this, although a fix has not been
forthcoming and Christophe is AWOL at the moment, so just revert the
change for now to get a working -rc3 on which we can queue patches for
5.15.
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nicholas Piggin <npiggin@gmail.com
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/linux-arm-kernel/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/
Link: https://lore.kernel.org/r/20210717160118.9855-1-jonathan@marek.ca
Fixes: c742199a014d ("mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge")
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/mm/mmu.c | 20 ++++++++------------
arch/x86/mm/pgtable.c | 34 +++++++++++++++-------------------
include/linux/pgtable.h | 26 +-------------------------
3 files changed, 24 insertions(+), 56 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index d74586508448..9ff0de1b2b93 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1339,7 +1339,6 @@ void *__init fixmap_remap_fdt(phys_addr_t dt_phys, int *size, pgprot_t prot)
return dt_virt;
}
-#if CONFIG_PGTABLE_LEVELS > 3
int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
{
pud_t new_pud = pfn_pud(__phys_to_pfn(phys), mk_pud_sect_prot(prot));
@@ -1354,16 +1353,6 @@ int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
return 1;
}
-int pud_clear_huge(pud_t *pudp)
-{
- if (!pud_sect(READ_ONCE(*pudp)))
- return 0;
- pud_clear(pudp);
- return 1;
-}
-#endif
-
-#if CONFIG_PGTABLE_LEVELS > 2
int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
{
pmd_t new_pmd = pfn_pmd(__phys_to_pfn(phys), mk_pmd_sect_prot(prot));
@@ -1378,6 +1367,14 @@ int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
return 1;
}
+int pud_clear_huge(pud_t *pudp)
+{
+ if (!pud_sect(READ_ONCE(*pudp)))
+ return 0;
+ pud_clear(pudp);
+ return 1;
+}
+
int pmd_clear_huge(pmd_t *pmdp)
{
if (!pmd_sect(READ_ONCE(*pmdp)))
@@ -1385,7 +1382,6 @@ int pmd_clear_huge(pmd_t *pmdp)
pmd_clear(pmdp);
return 1;
}
-#endif
int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
{
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 3364fe62b903..3481b35cb4ec 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -682,7 +682,6 @@ int p4d_clear_huge(p4d_t *p4d)
}
#endif
-#if CONFIG_PGTABLE_LEVELS > 3
/**
* pud_set_huge - setup kernel PUD mapping
*
@@ -721,23 +720,6 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
return 1;
}
-/**
- * pud_clear_huge - clear kernel PUD mapping when it is set
- *
- * Returns 1 on success and 0 on failure (no PUD map is found).
- */
-int pud_clear_huge(pud_t *pud)
-{
- if (pud_large(*pud)) {
- pud_clear(pud);
- return 1;
- }
-
- return 0;
-}
-#endif
-
-#if CONFIG_PGTABLE_LEVELS > 2
/**
* pmd_set_huge - setup kernel PMD mapping
*
@@ -768,6 +750,21 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
return 1;
}
+/**
+ * pud_clear_huge - clear kernel PUD mapping when it is set
+ *
+ * Returns 1 on success and 0 on failure (no PUD map is found).
+ */
+int pud_clear_huge(pud_t *pud)
+{
+ if (pud_large(*pud)) {
+ pud_clear(pud);
+ return 1;
+ }
+
+ return 0;
+}
+
/**
* pmd_clear_huge - clear kernel PMD mapping when it is set
*
@@ -782,7 +779,6 @@ int pmd_clear_huge(pmd_t *pmd)
return 0;
}
-#endif
#ifdef CONFIG_X86_64
/**
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index d147480cdefc..e24d2c992b11 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1397,34 +1397,10 @@ static inline int p4d_clear_huge(p4d_t *p4d)
}
#endif /* !__PAGETABLE_P4D_FOLDED */
-#ifndef __PAGETABLE_PUD_FOLDED
int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot);
-int pud_clear_huge(pud_t *pud);
-#else
-static inline int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
-{
- return 0;
-}
-static inline int pud_clear_huge(pud_t *pud)
-{
- return 0;
-}
-#endif /* !__PAGETABLE_PUD_FOLDED */
-
-#ifndef __PAGETABLE_PMD_FOLDED
int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot);
+int pud_clear_huge(pud_t *pud);
int pmd_clear_huge(pmd_t *pmd);
-#else
-static inline int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
-{
- return 0;
-}
-static inline int pmd_clear_huge(pmd_t *pmd)
-{
- return 0;
-}
-#endif /* !__PAGETABLE_PMD_FOLDED */
-
int p4d_free_pud_page(p4d_t *p4d, unsigned long addr);
int pud_free_pmd_page(pud_t *pud, unsigned long addr);
int pmd_free_pte_page(pmd_t *pmd, unsigned long addr);
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply related
* [PATCH 1/2] Revert "powerpc/8xx: add support for huge pages on VMAP and VMALLOC"
From: Will Deacon @ 2021-07-20 12:35 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Rutland, Marc Zyngier, Catalin Marinas, Andrew Morton,
Jonathan Marek, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Geert Uytterhoeven, Thomas Gleixner, Will Deacon, Ard Biesheuvel,
Mike Rapoport
In-Reply-To: <20210720123512.8740-1-will@kernel.org>
This reverts commit a6a8f7c4aa7eb50304b5c4e68eccd24313f3a785.
Commit c742199a014d ("mm/pgtable: add stubs for
{pmd/pub}_{set/clear}_huge") breaks the boot for arm64 when block
mappings are used to create the linear map, as this relies on a working
implementation of pXd_set_huge() even if the corresponding page-table
levels have been folded.
Although the problematic patch reverts cleanly, doing so breaks the
build for 32-bit PowerPC 8xx machines, which rely on the default
function definitions when the corresponding page-table levels are
folded:
| powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pud_range':
| linux/mm/vmalloc.c:362: undefined reference to `pud_clear_huge'
| powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pmd_range':
| linux/mm/vmalloc.c:337: undefined reference to `pmd_clear_huge'
| powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pud_range':
| linux/mm/vmalloc.c:362: undefined reference to `pud_clear_huge'
| powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pmd_range':
| linux/mm/vmalloc.c:337: undefined reference to `pmd_clear_huge'
| make: *** [Makefile:1177: vmlinux] Error 1
Although Christophe has kindly offered to look into the arm64 breakage,
he's on holiday for another 10 days and there isn't an obvious fix on
the arm64 side which allows us to continue using huge-vmap for affected
configurations.
In the interest of quickly getting things back to a working state as
they were in 5.13, revert the huge-vmap changes for PowerPC 8xx prior to
reverting the change which breaks arm64. We can then work on this
together for 5.15 once Christophe is back.
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nicholas Piggin <npiggin@gmail.com
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20210719170615.Horde.Qio1wp3k5ebLo-d9xXHdOg1@messagerie.c-s.fr
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/nohash/32/mmu-8xx.h | 43 --------------------
2 files changed, 1 insertion(+), 44 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d01e3401581d..5fc19ac62cb9 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -189,7 +189,7 @@ config PPC
select GENERIC_VDSO_TIME_NS
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
- select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx
+ select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
diff --git a/arch/powerpc/include/asm/nohash/32/mmu-8xx.h b/arch/powerpc/include/asm/nohash/32/mmu-8xx.h
index 997cec973406..6e4faa0a9b35 100644
--- a/arch/powerpc/include/asm/nohash/32/mmu-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/mmu-8xx.h
@@ -178,7 +178,6 @@
#ifndef __ASSEMBLY__
#include <linux/mmdebug.h>
-#include <linux/sizes.h>
void mmu_pin_tlb(unsigned long top, bool readonly);
@@ -226,48 +225,6 @@ static inline unsigned int mmu_psize_to_shift(unsigned int mmu_psize)
BUG();
}
-static inline bool arch_vmap_try_size(unsigned long addr, unsigned long end, u64 pfn,
- unsigned int max_page_shift, unsigned long size)
-{
- if (end - addr < size)
- return false;
-
- if ((1UL << max_page_shift) < size)
- return false;
-
- if (!IS_ALIGNED(addr, size))
- return false;
-
- if (!IS_ALIGNED(PFN_PHYS(pfn), size))
- return false;
-
- return true;
-}
-
-static inline unsigned long arch_vmap_pte_range_map_size(unsigned long addr, unsigned long end,
- u64 pfn, unsigned int max_page_shift)
-{
- if (arch_vmap_try_size(addr, end, pfn, max_page_shift, SZ_512K))
- return SZ_512K;
- if (PAGE_SIZE == SZ_16K)
- return SZ_16K;
- if (arch_vmap_try_size(addr, end, pfn, max_page_shift, SZ_16K))
- return SZ_16K;
- return PAGE_SIZE;
-}
-#define arch_vmap_pte_range_map_size arch_vmap_pte_range_map_size
-
-static inline int arch_vmap_pte_supported_shift(unsigned long size)
-{
- if (size >= SZ_512K)
- return 19;
- else if (size >= SZ_16K)
- return 14;
- else
- return PAGE_SHIFT;
-}
-#define arch_vmap_pte_supported_shift arch_vmap_pte_supported_shift
-
/* patch sites */
extern s32 patch__itlbmiss_exit_1, patch__dtlbmiss_exit_1;
extern s32 patch__itlbmiss_perf, patch__dtlbmiss_perf;
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply related
* [PATCH 0/2] Fix arm64 boot regression in 5.14
From: Will Deacon @ 2021-07-20 12:35 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Rutland, Marc Zyngier, Catalin Marinas, Andrew Morton,
Jonathan Marek, linuxppc-dev, Paul Mackerras, linux-arm-kernel,
Geert Uytterhoeven, Thomas Gleixner, Will Deacon, Ard Biesheuvel,
Mike Rapoport
Hi folks,
Jonathan reports [1] that commit c742199a014d ("mm/pgtable: add stubs
for {pmd/pub}_{set/clear}_huge") breaks the boot on arm64 when huge
mappings are used to map the kernel linear map but the VA size is
configured such that PUDs are folded. This is because the non-functional
pud_set_huge() stub is used to create the linear map, which results in
1GB holes and a fatal data abort when the kernel attemps to access them.
Digging further into the issue, it also transpired that huge-vmap is
silently disabled in these configurations as well [2], despite working
correctly in 5.13. The latter issue causes the pgtable selftests to
scream due to a failing consistency check [3].
Rather than leave mainline in a terminally broken state for arm64 while
we figure this out, revert the offending commit to get things working
again. Unfortunately, reverting the change in isolation causes a build
breakage for 32-bit PowerPC 8xx machines which recently started relying
on the problematic stubs to support pte-level huge-vmap entries [4].
Since Christophe is away at the moment, this series first reverts the
PowerPC 8xx change in order to avoid breaking the build.
I would really like this to land for -rc3 and I can take these via the
arm64 fixes queue if the PowerPC folks are alright with them.
Cheers,
Will
[1] https://lore.kernel.org/r/20210717160118.9855-1-jonathan@marek.ca
[2] https://lore.kernel.org/r/20210719104918.GA6440@willie-the-truck
[3] https://lore.kernel.org/r/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/
[4] https://lore.kernel.org/r/8b972f1c03fb6bd59953035f0a3e4d26659de4f8.1620795204.git.christophe.leroy@csgroup.eu/
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Jonathan Marek <jonathan@marek.ca>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nicholas Piggin <npiggin@gmail.com
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
--->8
Jonathan Marek (1):
Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"
Will Deacon (1):
Revert "powerpc/8xx: add support for huge pages on VMAP and VMALLOC"
arch/arm64/mm/mmu.c | 20 ++++-----
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/nohash/32/mmu-8xx.h | 43 --------------------
arch/x86/mm/pgtable.c | 34 +++++++---------
include/linux/pgtable.h | 26 +-----------
5 files changed, 25 insertions(+), 100 deletions(-)
--
2.32.0.402.g57bb445576-goog
^ permalink raw reply
* Re: [PATCH] powerpc: use IRQF_NO_DEBUG for IPIs
From: Thomas Gleixner @ 2021-07-20 10:28 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Cédric Le Goater
In-Reply-To: <20210719130614.195886-1-clg@kaod.org>
On Mon, Jul 19 2021 at 15:06, Cédric Le Goater wrote:
> There is no need to use the lockup detector ("noirqdebug") for IPIs.
> The ipistorm benchmark measures a ~10% improvement on high systems
> when this flag is set.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* [PATCH v2] PCI: Move pci_dev_is/assign_added() to pci.h
From: Niklas Schnelle @ 2021-07-20 9:58 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, linux-s390, linux-pci, linux-kernel, Paul Mackerras,
linuxppc-dev
The helper function pci_dev_is_added() from drivers/pci/pci.h is used in
PCI arch code of both s390 and powerpc leading to awkward relative
includes. Move it to the global include/linux/pci.h and get rid of these
includes just for that one function.
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Since v1:
- Fixed accidental removal of PCI_DPC_RECOVERED, PCI_DPC_RECOVERING
defines and also move these to include/linux/pci.h
arch/powerpc/platforms/powernv/pci-sriov.c | 3 ---
arch/powerpc/platforms/pseries/setup.c | 1 -
arch/s390/pci/pci_sysfs.c | 2 --
drivers/pci/hotplug/acpiphp_glue.c | 1 -
drivers/pci/pci.h | 15 ---------------
include/linux/pci.h | 13 +++++++++++++
6 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
index 28aac933a439..2e0ca5451e85 100644
--- a/arch/powerpc/platforms/powernv/pci-sriov.c
+++ b/arch/powerpc/platforms/powernv/pci-sriov.c
@@ -9,9 +9,6 @@
#include "pci.h"
-/* for pci_dev_is_added() */
-#include "../../../../drivers/pci/pci.h"
-
/*
* The majority of the complexity in supporting SR-IOV on PowerNV comes from
* the need to put the MMIO space for each VF into a separate PE. Internally
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 631a0d57b6cd..17585ec9f955 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -74,7 +74,6 @@
#include <asm/hvconsole.h>
#include "pseries.h"
-#include "../../../../drivers/pci/pci.h"
DEFINE_STATIC_KEY_FALSE(shared_processor);
EXPORT_SYMBOL_GPL(shared_processor);
diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c
index 6e2450c2b9c1..8dbe54ef8f8e 100644
--- a/arch/s390/pci/pci_sysfs.c
+++ b/arch/s390/pci/pci_sysfs.c
@@ -13,8 +13,6 @@
#include <linux/stat.h>
#include <linux/pci.h>
-#include "../../../drivers/pci/pci.h"
-
#include <asm/sclp.h>
#define zpci_attr(name, fmt, member) \
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index f031302ad401..4cb963f88183 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -38,7 +38,6 @@
#include <linux/slab.h>
#include <linux/acpi.h>
-#include "../pci.h"
#include "acpiphp.h"
static LIST_HEAD(bridge_list);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 93dcdd431072..a159cd0f6f05 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -383,21 +383,6 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
return dev->error_state == pci_channel_io_perm_failure;
}
-/* pci_dev priv_flags */
-#define PCI_DEV_ADDED 0
-#define PCI_DPC_RECOVERED 1
-#define PCI_DPC_RECOVERING 2
-
-static inline void pci_dev_assign_added(struct pci_dev *dev, bool added)
-{
- assign_bit(PCI_DEV_ADDED, &dev->priv_flags, added);
-}
-
-static inline bool pci_dev_is_added(const struct pci_dev *dev)
-{
- return test_bit(PCI_DEV_ADDED, &dev->priv_flags);
-}
-
#ifdef CONFIG_PCIEAER
#include <linux/aer.h>
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 540b377ca8f6..b3b7bafa17e5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -507,6 +507,19 @@ struct pci_dev {
unsigned long priv_flags; /* Private flags for the PCI driver */
};
+/* pci_dev priv_flags */
+#define PCI_DEV_ADDED 0
+
+static inline void pci_dev_assign_added(struct pci_dev *dev, bool added)
+{
+ assign_bit(PCI_DEV_ADDED, &dev->priv_flags, added);
+}
+
+static inline bool pci_dev_is_added(const struct pci_dev *dev)
+{
+ return test_bit(PCI_DEV_ADDED, &dev->priv_flags);
+}
+
static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
{
#ifdef CONFIG_PCI_IOV
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v5 02/11] powerpc/kernel/iommu: Add new iommu_table_in_use() helper
From: Alexey Kardashevskiy @ 2021-07-20 9:41 UTC (permalink / raw)
To: Leonardo Brás, Frederic Barrat, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras, David Gibson,
kernel test robot, Nicolin Chen
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <283e44591010dcb0690769afaef147add3b36978.camel@gmail.com>
On 20/07/2021 15:38, Leonardo Brás wrote:
> Hello Fred, thanks for this feedback!
>
> Sorry if I miss anything, this snippet was written for v1 over an year
> ago, and I have not taken a look at it ever since.
>
> On Mon, 2021-07-19 at 15:53 +0200, Frederic Barrat wrote:
>>
>>
>> On 16/07/2021 10:27, Leonardo Bras wrote:
>>> @@ -1099,18 +1105,13 @@ int iommu_take_ownership(struct iommu_table
>>> *tbl)
>>> for (i = 0; i < tbl->nr_pools; i++)
>>> spin_lock_nest_lock(&tbl->pools[i].lock, &tbl-
>>>> large_pool.lock);
>>>
>>> - iommu_table_release_pages(tbl);
>>> -
>>> - if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
>>> + if (iommu_table_in_use(tbl)) {
>>> pr_err("iommu_tce: it_map is not empty");
>>> ret = -EBUSY;
>>> - /* Undo iommu_table_release_pages, i.e. restore
>>> bit#0, etc */
>>> - iommu_table_reserve_pages(tbl, tbl-
>>>> it_reserved_start,
>>> - tbl->it_reserved_end);
>>> - } else {
>>> - memset(tbl->it_map, 0xff, sz);
>>> }
>>>
>>> + memset(tbl->it_map, 0xff, sz);
>>> +
>>
>>
>> So if the table is not empty, we fail (EBUSY) but we now also
>> completely
>> overwrite the bitmap. It was in an unexpected state, but we're making
>> it
>> worse. Or am I missing something?
>
> IIRC there was a reason to do that at the time, but TBH I don't really
> remember it, and by looking at the code right now you seem to be
> correct about this causing trouble.
>
> I will send a v6 fixing it soon.
> Please review the remaining patches for some issue I may be missing.
>
> Alexey, any comments on that?
Agree with Fred, this is a bug, EBUSY is not that unexpected :-/ Thanks,
>
>>
>> Fred
>>
>
> Again, thank you for reviewing Fred!
> Best regards,
> Leonardo Bras
>
>
>
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH] PCI: Move pci_dev_is/assign_added() to pci.h
From: Niklas Schnelle @ 2021-07-20 9:32 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, linux-pci, linuxppc-dev, linux-kernel, linux-s390
In-Reply-To: <bd731ed627344a3a2eaeffabff21d499c4e2c3fd.camel@linux.ibm.com>
On Tue, 2021-07-20 at 10:34 +0200, Niklas Schnelle wrote:
> On Mon, 2021-07-19 at 14:11 +0200, Niklas Schnelle wrote:
> > The helper function pci_dev_is_added() from drivers/pci/pci.h is used in
> > PCI arch code of both s390 and powerpc leading to awkward relative
> > includes. Move it to the global include/linux/pci.h and get rid of these
> > includes just for that one function.
> >
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> >
> ... snip ...
> >
> > static LIST_HEAD(bridge_list);
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index 93dcdd431072..a159cd0f6f05 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -383,21 +383,6 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
> > return dev->error_state == pci_channel_io_perm_failure;
> > }
> >
> > -/* pci_dev priv_flags */
> > -#define PCI_DEV_ADDED 0
> > -#define PCI_DPC_RECOVERED 1
> > -#define PCI_DPC_RECOVERING 2
>
> Sorry, the above two PCI_DPC_* lines should remain in drivers/pci/pci.h
> I messed this up on rebasing to v5.14-rc1 and it didn't lead to
> problems on either s390x defconfig, nor pp64_defconfig but breaks ppc
> allyesconfig. Will resend a fixed version.
Ok looking at this more closely, maybe it would also make sense to move
the PCI_DPC_* flags also to include/linux/pci.h that would also put
them in direct proximity of the priv_flags field decleration itself
which I think also improves maintainability.
>
>
> .. snip ..
>
^ permalink raw reply
* Re: [PATCH] PCI: Move pci_dev_is/assign_added() to pci.h
From: Niklas Schnelle @ 2021-07-20 8:34 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-arch, linux-pci, linuxppc-dev, linux-kernel, linux-s390
In-Reply-To: <20210719121148.2403239-1-schnelle@linux.ibm.com>
On Mon, 2021-07-19 at 14:11 +0200, Niklas Schnelle wrote:
> The helper function pci_dev_is_added() from drivers/pci/pci.h is used in
> PCI arch code of both s390 and powerpc leading to awkward relative
> includes. Move it to the global include/linux/pci.h and get rid of these
> includes just for that one function.
>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>
... snip ...
>
> static LIST_HEAD(bridge_list);
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 93dcdd431072..a159cd0f6f05 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -383,21 +383,6 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
> return dev->error_state == pci_channel_io_perm_failure;
> }
>
> -/* pci_dev priv_flags */
> -#define PCI_DEV_ADDED 0
> -#define PCI_DPC_RECOVERED 1
> -#define PCI_DPC_RECOVERING 2
Sorry, the above two PCI_DPC_* lines should remain in drivers/pci/pci.h
I messed this up on rebasing to v5.14-rc1 and it didn't lead to
problems on either s390x defconfig, nor pp64_defconfig but breaks ppc
allyesconfig. Will resend a fixed version.
>
.. snip ..
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/smp: Use existing L2 cache_map cpumask to find L3 cache siblings
From: Gautham R Shenoy @ 2021-07-20 6:42 UTC (permalink / raw)
To: Parth Shah; +Cc: ego, mikey, srikar, svaidy, linuxppc-dev
In-Reply-To: <20210615070804.390341-4-parth@linux.ibm.com>
Hi Parth,
Sorry for the late review.
On Tue, Jun 15, 2021 at 12:38:04PM +0530, Parth Shah wrote:
> On POWER10 systems, the "ibm,thread-groups" property "2" indicates the cpus
> in thread-group share both L2 and L3 caches. Hence, use cache_property = 2
> itself to find both the L2 and L3 cache siblings.
> Hence, rename existing macros to detect if the cache property is for L2 or
> L3 and use the L2 cache map itself to find the presence of L3 siblings.
>
> Signed-off-by: Parth Shah <parth@linux.ibm.com>
> ---
> arch/powerpc/include/asm/smp.h | 2 ++
> arch/powerpc/kernel/cacheinfo.c | 3 +++
> arch/powerpc/kernel/smp.c | 20 +++++++++++++++-----
> 3 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 1259040cc3a4..55082d343bd2 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -144,6 +144,7 @@ extern int cpu_to_core_id(int cpu);
>
> extern bool has_big_cores;
> extern bool thread_group_shares_l2;
> +extern bool thread_group_shares_l3;
>
> #define cpu_smt_mask cpu_smt_mask
> #ifdef CONFIG_SCHED_SMT
> @@ -198,6 +199,7 @@ extern void __cpu_die(unsigned int cpu);
> #define hard_smp_processor_id() get_hard_smp_processor_id(0)
> #define smp_setup_cpu_maps()
> #define thread_group_shares_l2 0
> +#define thread_group_shares_l3 0
> static inline void inhibit_secondary_onlining(void) {}
> static inline void uninhibit_secondary_onlining(void) {}
> static inline const struct cpumask *cpu_sibling_mask(int cpu)
> diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
> index 20d91693eac1..378ae20d05a9 100644
> --- a/arch/powerpc/kernel/cacheinfo.c
> +++ b/arch/powerpc/kernel/cacheinfo.c
> @@ -469,6 +469,9 @@ static int get_group_id(unsigned int cpu_id, int level)
> else if (thread_group_shares_l2 && level == 2)
> return cpumask_first(per_cpu(thread_group_l2_cache_map,
> cpu_id));
> + else if (thread_group_shares_l3 && level == 3)
> + return cpumask_first(per_cpu(thread_group_l2_cache_map,
> + cpu_id));
We should either rename thread_group_l2_cache_map as
thread_group_l2_l3_cache_map or we should create a separate
thread_group_l3_cache_map. I prefer the latter approach since it makes
the code consistent.
Otherwise, the patch looks good to me.
--
Thanks and Regards
gautham.
> return -1;
> }
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index a34877257f2d..d0c70fcd0068 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -78,6 +78,7 @@ struct task_struct *secondary_current;
> bool has_big_cores;
> bool coregroup_enabled;
> bool thread_group_shares_l2;
> +bool thread_group_shares_l3;
>
> DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
> DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
> @@ -101,7 +102,7 @@ enum {
>
> #define MAX_THREAD_LIST_SIZE 8
> #define THREAD_GROUP_SHARE_L1 1
> -#define THREAD_GROUP_SHARE_L2 2
> +#define THREAD_GROUP_SHARE_L2_L3 2
> struct thread_groups {
> unsigned int property;
> unsigned int nr_groups;
> @@ -887,9 +888,16 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
> cpumask_var_t *mask = NULL;
>
> if (cache_property != THREAD_GROUP_SHARE_L1 &&
> - cache_property != THREAD_GROUP_SHARE_L2)
> + cache_property != THREAD_GROUP_SHARE_L2_L3)
> return -EINVAL;
>
> + /*
> + * On P10 fused-core system, the L3 cache is shared between threads of a
> + * small core only, but the "ibm,thread-groups" property is indicated as
> + * "2" only which is interpreted as the thread-groups sharing both L2
> + * and L3 caches. Hence cache_property of THREAD_GROUP_SHARE_L2_L3 is
> + * used for both L2 and L3 cache sibling detection.
> + */
> tg = get_thread_groups(cpu, cache_property, &err);
> if (!tg)
> return err;
> @@ -903,7 +911,7 @@ static int __init init_thread_group_cache_map(int cpu, int cache_property)
>
> if (cache_property == THREAD_GROUP_SHARE_L1)
> mask = &per_cpu(thread_group_l1_cache_map, cpu);
> - else if (cache_property == THREAD_GROUP_SHARE_L2)
> + else if (cache_property == THREAD_GROUP_SHARE_L2_L3)
> mask = &per_cpu(thread_group_l2_cache_map, cpu);
>
> zalloc_cpumask_var_node(mask, GFP_KERNEL, cpu_to_node(cpu));
> @@ -1009,14 +1017,16 @@ static int __init init_big_cores(void)
> has_big_cores = true;
>
> for_each_possible_cpu(cpu) {
> - int err = init_thread_group_cache_map(cpu, THREAD_GROUP_SHARE_L2);
> + int err = init_thread_group_cache_map(cpu, THREAD_GROUP_SHARE_L2_L3);
>
> if (err)
> return err;
> }
>
> thread_group_shares_l2 = true;
> - pr_debug("L2 cache only shared by the threads in the small core\n");
> + thread_group_shares_l3 = true;
> + pr_debug("L2/L3 cache only shared by the threads in the small core\n");
> +
> return 0;
> }
>
> --
> 2.26.3
>
^ permalink raw reply
* Re: [RFC 0/2] Add generic FPU api similar to x86
From: Christian König @ 2021-07-20 6:17 UTC (permalink / raw)
To: Anson Jacob, mpe, benh, paulus, christophe.leroy, linuxppc-dev,
amd-gfx
Cc: Sunpeng.Li, Bhawanpreet.Lakha, qingqing.zhuo, Rodrigo.Siqueira,
roman.li, Aurabindo.Pillai, Harry.Wentland, bindu.r
In-Reply-To: <20210719195211.520428-1-Anson.Jacob@amd.com>
While you already CCed a bunch of people stuff like that needs to go to
the appropriate mailing list and not just amd-gfx.
Especially LKML so that other core devs can take a look as well.
Regards,
Christian.
Am 19.07.21 um 21:52 schrieb Anson Jacob:
> This is an attempt to have generic FPU enable/disable
> calls similar to x86.
> So that we can simplify gpu/drm/amd/display/dc/os_types.h
>
> Also adds FPU correctness logic seen in x86.
>
> Anson Jacob (2):
> ppc/fpu: Add generic FPU api similar to x86
> drm/amd/display: Use PPC FPU functions
>
> arch/powerpc/include/asm/switch_to.h | 29 ++---
> arch/powerpc/kernel/process.c | 130 ++++++++++++++++++++++
> drivers/gpu/drm/amd/display/dc/os_types.h | 28 +----
> 3 files changed, 139 insertions(+), 48 deletions(-)
>
^ permalink raw reply
* Re: [RFC 2/2] drm/amd/display: Use PPC FPU functions
From: Christoph Hellwig @ 2021-07-20 6:06 UTC (permalink / raw)
To: Anson Jacob
Cc: Harry.Wentland, Sunpeng.Li, qingqing.zhuo, Rodrigo.Siqueira,
roman.li, amd-gfx, Bhawanpreet.Lakha, Aurabindo.Pillai, paulus,
linuxppc-dev, bindu.r
In-Reply-To: <20210719195211.520428-3-Anson.Jacob@amd.com>
> #define DC_FP_END() kernel_fpu_end()
> #elif defined(CONFIG_PPC64)
> #include <asm/switch_to.h>
> +#define DC_FP_START() kernel_fpu_begin()
> +#define DC_FP_END() kernel_fpu_end()
> #endif
Please use the same header as x86 in your first patch and then kill
this ifdefered and the DC_FP_START/DC_FP_END definitions entirely.
^ permalink raw reply
* Re: [RFC 1/2] ppc/fpu: Add generic FPU api similar to x86
From: Christoph Hellwig @ 2021-07-20 6:04 UTC (permalink / raw)
To: Anson Jacob
Cc: Harry.Wentland, Sunpeng.Li, qingqing.zhuo, Rodrigo.Siqueira,
roman.li, amd-gfx, Bhawanpreet.Lakha, Aurabindo.Pillai, paulus,
linuxppc-dev, bindu.r
In-Reply-To: <20210719195211.520428-2-Anson.Jacob@amd.com>
On Mon, Jul 19, 2021 at 03:52:10PM -0400, Anson Jacob wrote:
> - Add kernel_fpu_begin & kernel_fpu_end API as x86
> - Add logic similar to x86 to ensure fpu
> begin/end call correctness
> - Add kernel_fpu_enabled to know if FPU is enabled
>
> Signed-off-by: Anson Jacob <Anson.Jacob@amd.com>
All the x86 FPU support is EXPORT_SYMBOL_GPL for a good reason, so
please stick to that.
^ permalink raw reply
* Re: [PATCH v5 02/11] powerpc/kernel/iommu: Add new iommu_table_in_use() helper
From: Leonardo Brás @ 2021-07-20 5:38 UTC (permalink / raw)
To: Frederic Barrat, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <29c199f3-63a8-3edb-b29e-de157431d89f@linux.ibm.com>
Hello Fred, thanks for this feedback!
Sorry if I miss anything, this snippet was written for v1 over an year
ago, and I have not taken a look at it ever since.
On Mon, 2021-07-19 at 15:53 +0200, Frederic Barrat wrote:
>
>
> On 16/07/2021 10:27, Leonardo Bras wrote:
> > @@ -1099,18 +1105,13 @@ int iommu_take_ownership(struct iommu_table
> > *tbl)
> > for (i = 0; i < tbl->nr_pools; i++)
> > spin_lock_nest_lock(&tbl->pools[i].lock, &tbl-
> > >large_pool.lock);
> >
> > - iommu_table_release_pages(tbl);
> > -
> > - if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
> > + if (iommu_table_in_use(tbl)) {
> > pr_err("iommu_tce: it_map is not empty");
> > ret = -EBUSY;
> > - /* Undo iommu_table_release_pages, i.e. restore
> > bit#0, etc */
> > - iommu_table_reserve_pages(tbl, tbl-
> > >it_reserved_start,
> > - tbl->it_reserved_end);
> > - } else {
> > - memset(tbl->it_map, 0xff, sz);
> > }
> >
> > + memset(tbl->it_map, 0xff, sz);
> > +
>
>
> So if the table is not empty, we fail (EBUSY) but we now also
> completely
> overwrite the bitmap. It was in an unexpected state, but we're making
> it
> worse. Or am I missing something?
IIRC there was a reason to do that at the time, but TBH I don't really
remember it, and by looking at the code right now you seem to be
correct about this causing trouble.
I will send a v6 fixing it soon.
Please review the remaining patches for some issue I may be missing.
Alexey, any comments on that?
>
> Fred
>
Again, thank you for reviewing Fred!
Best regards,
Leonardo Bras
^ permalink raw reply
* [PATCH v6 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: Pratik R. Sampat @ 2021-07-20 5:34 UTC (permalink / raw)
To: mpe, benh, paulus, farosas, linuxppc-dev, kvm-ppc, linux-kernel,
psampat, pratik.r.sampat
In-Reply-To: <20210720053434.32921-1-psampat@linux.ibm.com>
Adds a generic interface to represent the energy and frequency related
PAPR attributes on the system using the new H_CALL
"H_GET_ENERGY_SCALE_INFO".
H_GET_EM_PARMS H_CALL was previously responsible for exporting this
information in the lparcfg, however the H_GET_EM_PARMS H_CALL
will be deprecated P10 onwards.
The H_GET_ENERGY_SCALE_INFO H_CALL is of the following call format:
hcall(
uint64 H_GET_ENERGY_SCALE_INFO, // Get energy scale info
uint64 flags, // Per the flag request
uint64 firstAttributeId,// The attribute id
uint64 bufferAddress, // Guest physical address of the output buffer
uint64 bufferSize // The size in bytes of the output buffer
);
This H_CALL can query either all the attributes at once with
firstAttributeId = 0, flags = 0 as well as query only one attribute
at a time with firstAttributeId = id, flags = 1.
The output buffer consists of the following
1. number of attributes - 8 bytes
2. array offset to the data location - 8 bytes
3. version info - 1 byte
4. A data array of size num attributes, which contains the following:
a. attribute ID - 8 bytes
b. attribute value in number - 8 bytes
c. attribute name in string - 64 bytes
d. attribute value in string - 64 bytes
The new H_CALL exports information in direct string value format, hence
a new interface has been introduced in
/sys/firmware/papr/energy_scale_info to export this information to
userspace in an extensible pass-through format.
The H_CALL returns the name, numeric value and string value (if exists)
The format of exposing the sysfs information is as follows:
/sys/firmware/papr/energy_scale_info/
|-- <id>/
|-- desc
|-- value
|-- value_desc (if exists)
|-- <id>/
|-- desc
|-- value
|-- value_desc (if exists)
...
The energy information that is exported is useful for userspace tools
such as powerpc-utils. Currently these tools infer the
"power_mode_data" value in the lparcfg, which in turn is obtained from
the to be deprecated H_GET_EM_PARMS H_CALL.
On future platforms, such userspace utilities will have to look at the
data returned from the new H_CALL being populated in this new sysfs
interface and report this information directly without the need of
interpretation.
Signed-off-by: Pratik R. Sampat <psampat@linux.ibm.com>
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
.../sysfs-firmware-papr-energy-scale-info | 26 ++
arch/powerpc/include/asm/hvcall.h | 24 +-
arch/powerpc/kvm/trace_hv.h | 1 +
arch/powerpc/platforms/pseries/Makefile | 3 +-
.../pseries/papr_platform_attributes.c | 312 ++++++++++++++++++
5 files changed, 364 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
create mode 100644 arch/powerpc/platforms/pseries/papr_platform_attributes.c
diff --git a/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
new file mode 100644
index 000000000000..139a576c7c9d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
@@ -0,0 +1,26 @@
+What: /sys/firmware/papr/energy_scale_info
+Date: June 2021
+Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description: Directory hosting a set of platform attributes like
+ energy/frequency on Linux running as a PAPR guest.
+
+ Each file in a directory contains a platform
+ attribute hierarchy pertaining to performance/
+ energy-savings mode and processor frequency.
+
+What: /sys/firmware/papr/energy_scale_info/<id>
+ /sys/firmware/papr/energy_scale_info/<id>/desc
+ /sys/firmware/papr/energy_scale_info/<id>/value
+ /sys/firmware/papr/energy_scale_info/<id>/value_desc
+Date: June 2021
+Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
+Description: Energy, frequency attributes directory for POWERVM servers
+
+ This directory provides energy, frequency, folding information. It
+ contains below sysfs attributes:
+
+ - desc: String description of the attribute <id>
+
+ - value: Numeric value of attribute <id>
+
+ - value_desc: String value of attribute <id>
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index e3b29eda8074..c91714ea6719 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -316,7 +316,8 @@
#define H_SCM_PERFORMANCE_STATS 0x418
#define H_RPT_INVALIDATE 0x448
#define H_SCM_FLUSH 0x44C
-#define MAX_HCALL_OPCODE H_SCM_FLUSH
+#define H_GET_ENERGY_SCALE_INFO 0x450
+#define MAX_HCALL_OPCODE H_GET_ENERGY_SCALE_INFO
/* Scope args for H_SCM_UNBIND_ALL */
#define H_UNBIND_SCOPE_ALL (0x1)
@@ -631,6 +632,27 @@ struct hv_gpci_request_buffer {
uint8_t bytes[HGPCI_MAX_DATA_BYTES];
} __packed;
+#define ESI_VERSION 0x1
+#define MAX_ESI_ATTRS 10
+#define MAX_BUF_SZ (sizeof(struct h_energy_scale_info_hdr) + \
+ (sizeof(struct energy_scale_attribute) * MAX_ESI_ATTRS))
+
+struct energy_scale_attribute {
+ __be64 id;
+ __be64 value;
+ unsigned char desc[64];
+ unsigned char value_desc[64];
+} __packed;
+
+struct h_energy_scale_info_hdr {
+ __be64 num_attrs;
+ __be64 array_offset;
+ __u8 data_header_version;
+} __packed;
+
+/* /sys/firmware/papr */
+extern struct kobject *papr_kobj;
+
#endif /* __ASSEMBLY__ */
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_HVCALL_H */
diff --git a/arch/powerpc/kvm/trace_hv.h b/arch/powerpc/kvm/trace_hv.h
index 830a126e095d..38cd0ed0a617 100644
--- a/arch/powerpc/kvm/trace_hv.h
+++ b/arch/powerpc/kvm/trace_hv.h
@@ -115,6 +115,7 @@
{H_VASI_STATE, "H_VASI_STATE"}, \
{H_ENABLE_CRQ, "H_ENABLE_CRQ"}, \
{H_GET_EM_PARMS, "H_GET_EM_PARMS"}, \
+ {H_GET_ENERGY_SCALE_INFO, "H_GET_ENERGY_SCALE_INFO"}, \
{H_SET_MPP, "H_SET_MPP"}, \
{H_GET_MPP, "H_GET_MPP"}, \
{H_HOME_NODE_ASSOCIATIVITY, "H_HOME_NODE_ASSOCIATIVITY"}, \
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index c8a2b0b05ac0..d14fca89ac25 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -6,7 +6,8 @@ obj-y := lpar.o hvCall.o nvram.o reconfig.o \
of_helpers.o \
setup.o iommu.o event_sources.o ras.o \
firmware.o power.o dlpar.o mobility.o rng.o \
- pci.o pci_dlpar.o eeh_pseries.o msi.o
+ pci.o pci_dlpar.o eeh_pseries.o msi.o \
+ papr_platform_attributes.o
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_SCANLOG) += scanlog.o
obj-$(CONFIG_KEXEC_CORE) += kexec.o
diff --git a/arch/powerpc/platforms/pseries/papr_platform_attributes.c b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
new file mode 100644
index 000000000000..d0373925ad32
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
@@ -0,0 +1,312 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Platform energy and frequency attributes driver
+ *
+ * This driver creates a sys file at /sys/firmware/papr/ which encapsulates a
+ * directory structure containing files in keyword - value pairs that specify
+ * energy and frequency configuration of the system.
+ *
+ * The format of exposing the sysfs information is as follows:
+ * /sys/firmware/papr/energy_scale_info/
+ * |-- <id>/
+ * |-- desc
+ * |-- value
+ * |-- value_desc (if exists)
+ * |-- <id>/
+ * |-- desc
+ * |-- value
+ * |-- value_desc (if exists)
+ *
+ * Copyright 2021 IBM Corp.
+ */
+
+#include <asm/hvcall.h>
+#include <asm/machdep.h>
+
+#include "pseries.h"
+
+/*
+ * Flag attributes to fetch either all or one attribute from the HCALL
+ * flag = BE(0) => fetch all attributes with firstAttributeId = 0
+ * flag = BE(1) => fetch a single attribute with firstAttributeId = id
+ */
+#define ESI_FLAGS_ALL 0
+#define ESI_FLAGS_SINGLE PPC_BIT(0)
+
+#define MAX_ATTRS 3
+
+struct papr_attr {
+ u64 id;
+ struct kobj_attribute kobj_attr;
+};
+struct papr_group {
+ struct attribute_group pg;
+ struct papr_attr pgattrs[MAX_ATTRS];
+} *pgs;
+
+/* /sys/firmware/papr */
+struct kobject *papr_kobj;
+/* /sys/firmware/papr/energy_scale_info */
+struct kobject *esi_kobj;
+
+/*
+ * Extract and export the description of the energy scale attributes
+ */
+static ssize_t papr_show_desc(struct kobject *kobj,
+ struct kobj_attribute *kobj_attr,
+ char *buf)
+{
+ struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
+ kobj_attr);
+ struct h_energy_scale_info_hdr *t_hdr;
+ struct energy_scale_attribute *t_esi;
+ char *t_buf;
+ int ret = 0;
+
+ t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
+ if (t_buf == NULL)
+ return -ENOMEM;
+
+ ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
+ pattr->id, virt_to_phys(t_buf),
+ MAX_BUF_SZ);
+
+ if (ret != H_SUCCESS) {
+ pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
+ goto out;
+ }
+
+ t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
+ t_esi = (struct energy_scale_attribute *)
+ (t_buf + be64_to_cpu(t_hdr->array_offset));
+
+ ret = snprintf(buf, sizeof(t_esi->desc), "%s\n", t_esi->desc);
+ if (ret < 0)
+ ret = -EIO;
+out:
+ kfree(t_buf);
+
+ return ret;
+}
+
+/*
+ * Extract and export the numeric value of the energy scale attributes
+ */
+static ssize_t papr_show_value(struct kobject *kobj,
+ struct kobj_attribute *kobj_attr,
+ char *buf)
+{
+ struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
+ kobj_attr);
+ struct h_energy_scale_info_hdr *t_hdr;
+ struct energy_scale_attribute *t_esi;
+ char *t_buf;
+ int ret = 0;
+
+ t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
+ if (t_buf == NULL)
+ return -ENOMEM;
+
+ ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
+ pattr->id, virt_to_phys(t_buf),
+ MAX_BUF_SZ);
+
+ if (ret != H_SUCCESS) {
+ pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
+ goto out;
+ }
+
+ t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
+ t_esi = (struct energy_scale_attribute *)
+ (t_buf + be64_to_cpu(t_hdr->array_offset));
+
+ ret = snprintf(buf, sizeof(t_esi->value), "%llu\n",
+ be64_to_cpu(t_esi->value));
+ if (ret < 0)
+ ret = -EIO;
+out:
+ kfree(t_buf);
+
+ return ret;
+}
+
+/*
+ * Extract and export the value description in string format of the energy
+ * scale attributes
+ */
+static ssize_t papr_show_value_desc(struct kobject *kobj,
+ struct kobj_attribute *kobj_attr,
+ char *buf)
+{
+ struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
+ kobj_attr);
+ struct h_energy_scale_info_hdr *t_hdr;
+ struct energy_scale_attribute *t_esi;
+ char *t_buf;
+ int ret = 0;
+
+ t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
+ if (t_buf == NULL)
+ return -ENOMEM;
+
+ ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
+ pattr->id, virt_to_phys(t_buf),
+ MAX_BUF_SZ);
+
+ if (ret != H_SUCCESS) {
+ pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
+ goto out;
+ }
+
+ t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
+ t_esi = (struct energy_scale_attribute *)
+ (t_buf + be64_to_cpu(t_hdr->array_offset));
+
+ ret = snprintf(buf, sizeof(t_esi->value_desc), "%s\n",
+ t_esi->value_desc);
+ if (ret < 0)
+ ret = -EIO;
+out:
+ kfree(t_buf);
+
+ return ret;
+}
+
+static struct papr_ops_info {
+ const char *attr_name;
+ ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *kobj_attr,
+ char *buf);
+} ops_info[MAX_ATTRS] = {
+ { "desc", papr_show_desc },
+ { "value", papr_show_value },
+ { "value_desc", papr_show_value_desc },
+};
+
+static void add_attr(u64 id, int index, struct papr_attr *attr)
+{
+ attr->id = id;
+ sysfs_attr_init(&attr->kobj_attr.attr);
+ attr->kobj_attr.attr.name = ops_info[index].attr_name;
+ attr->kobj_attr.attr.mode = 0444;
+ attr->kobj_attr.show = ops_info[index].show;
+}
+
+static int add_attr_group(u64 id, struct papr_group *pg, bool show_val_desc)
+{
+ int i;
+
+ for (i = 0; i < MAX_ATTRS; i++) {
+ if (!strcmp(ops_info[i].attr_name, "value_desc") &&
+ !show_val_desc) {
+ continue;
+ }
+ add_attr(id, i, &pg->pgattrs[i]);
+ pg->pg.attrs[i] = &pg->pgattrs[i].kobj_attr.attr;
+ }
+
+ return sysfs_create_group(esi_kobj, &pg->pg);
+}
+
+static int __init papr_init(void)
+{
+ struct h_energy_scale_info_hdr *esi_hdr;
+ struct energy_scale_attribute *esi_attrs;
+ uint64_t num_attrs;
+ int ret, idx, i;
+ char *esi_buf;
+
+ if (!firmware_has_feature(FW_FEATURE_LPAR))
+ return -ENXIO;
+
+ esi_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
+ if (esi_buf == NULL)
+ return -ENOMEM;
+ /*
+ * hcall(
+ * uint64 H_GET_ENERGY_SCALE_INFO, // Get energy scale info
+ * uint64 flags, // Per the flag request
+ * uint64 firstAttributeId, // The attribute id
+ * uint64 bufferAddress, // Guest physical address of the output buffer
+ * uint64 bufferSize); // The size in bytes of the output buffer
+ */
+ ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_ALL, 0,
+ virt_to_phys(esi_buf), MAX_BUF_SZ);
+ if (ret != H_SUCCESS) {
+ pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
+ goto out;
+ }
+
+ esi_hdr = (struct h_energy_scale_info_hdr *) esi_buf;
+ if (esi_hdr->data_header_version != ESI_VERSION) {
+ pr_warn("H_GET_ENERGY_SCALE_INFO VER MISMATCH - EXP: 0x%x, REC: 0x%x",
+ ESI_VERSION, esi_hdr->data_header_version);
+ }
+
+ num_attrs = be64_to_cpu(esi_hdr->num_attrs);
+ esi_attrs = (struct energy_scale_attribute *)
+ (esi_buf + be64_to_cpu(esi_hdr->array_offset));
+
+ pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
+ if (!pgs)
+ goto out;
+
+ papr_kobj = kobject_create_and_add("papr", firmware_kobj);
+ if (!papr_kobj) {
+ pr_warn("kobject_create_and_add papr failed\n");
+ goto out_pgs;
+ }
+
+ esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
+ if (!esi_kobj) {
+ pr_warn("kobject_create_and_add energy_scale_info failed\n");
+ goto out_kobj;
+ }
+
+ for (idx = 0; idx < num_attrs; idx++) {
+ bool show_val_desc = true;
+
+ pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
+ sizeof(*pgs[idx].pg.attrs),
+ GFP_KERNEL);
+ if (!pgs[idx].pg.attrs) {
+ goto out_pgattrs;
+ }
+
+ pgs[idx].pg.name = kasprintf(GFP_KERNEL, "%lld",
+ be64_to_cpu(esi_attrs[idx].id));
+ if (pgs[idx].pg.name == NULL) {
+ goto out_pgattrs;
+ }
+ /* Do not add the value description if it does not exist */
+ if (strnlen(esi_attrs[idx].value_desc,
+ sizeof(esi_attrs[idx].value_desc)) == 0)
+ show_val_desc = false;
+
+ if (add_attr_group(be64_to_cpu(esi_attrs[idx].id), &pgs[idx],
+ show_val_desc)) {
+ pr_warn("Failed to create papr attribute group %s\n",
+ pgs[idx].pg.name);
+ goto out_pgattrs;
+ }
+ }
+
+ kfree(esi_buf);
+ return 0;
+
+out_pgattrs:
+ for (i = 0; i < num_attrs ; i++) {
+ kfree(pgs[i].pg.attrs);
+ kfree(pgs[i].pg.name);
+ }
+ kobject_put(esi_kobj);
+out_kobj:
+ kobject_put(papr_kobj);
+out_pgs:
+ kfree(pgs);
+out:
+ kfree(esi_buf);
+
+ return -ENOMEM;
+}
+
+machine_device_initcall(pseries, papr_init);
--
2.31.1
^ permalink raw reply related
* [PATCH v6 0/1] Interface to represent PAPR firmware attributes
From: Pratik R. Sampat @ 2021-07-20 5:34 UTC (permalink / raw)
To: mpe, benh, paulus, farosas, linuxppc-dev, kvm-ppc, linux-kernel,
psampat, pratik.r.sampat
RFC: https://lkml.org/lkml/2021/6/4/791
PATCH v1: https://lkml.org/lkml/2021/6/16/805
PATCH v2: https://lkml.org/lkml/2021/7/6/138
PATCH v3: https://lkml.org/lkml/2021/7/12/2799
PATCH v4: https://lkml.org/lkml/2021/7/16/532
PATCH v5: https://lkml.org/lkml/2021/7/19/247
Changelog v5 --> v6
1. On allocation faliure of "pgs[idx].pg attributes", redirect the free
using "goto" to a common cleanups section for consistency
Also, have implemented a POC using this interface for the powerpc-utils'
ppc64_cpu --frequency command-line tool to utilize this information
in userspace.
The POC for the new interface has been hosted here:
https://github.com/pratiksampat/powerpc-utils/tree/H_GET_ENERGY_SCALE_INFO_v2
Sample output from the powerpc-utils tool is as follows:
# ppc64_cpu --frequency
Power and Performance Mode: XXXX
Idle Power Saver Status : XXXX
Processor Folding Status : XXXX --> Printed if Idle power save status is supported
Platform reported frequencies --> Frequencies reported from the platform's H_CALL i.e PAPR interface
min : NNNN GHz
max : NNNN GHz
static : NNNN GHz
Tool Computed frequencies
min : NNNN GHz (cpu XX)
max : NNNN GHz (cpu XX)
avg : NNNN GHz
Pratik R. Sampat (1):
powerpc/pseries: Interface to represent PAPR firmware attributes
.../sysfs-firmware-papr-energy-scale-info | 26 ++
arch/powerpc/include/asm/hvcall.h | 24 +-
arch/powerpc/kvm/trace_hv.h | 1 +
arch/powerpc/platforms/pseries/Makefile | 3 +-
.../pseries/papr_platform_attributes.c | 312 ++++++++++++++++++
5 files changed, 364 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
create mode 100644 arch/powerpc/platforms/pseries/papr_platform_attributes.c
--
2.31.1
^ permalink raw reply
* Re: [PATCH v5 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: Pratik Sampat @ 2021-07-20 5:04 UTC (permalink / raw)
To: Fabiano Rosas, mpe, benh, paulus, linuxppc-dev, kvm-ppc,
linux-kernel, pratik.r.sampat
In-Reply-To: <87fswa2k93.fsf@linux.ibm.com>
On 20/07/21 12:39 am, Fabiano Rosas wrote:
> "Pratik R. Sampat" <psampat@linux.ibm.com> writes:
>
>> + pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
>> + if (!pgs)
>> + goto out;
>> +
>> + papr_kobj = kobject_create_and_add("papr", firmware_kobj);
>> + if (!papr_kobj) {
>> + pr_warn("kobject_create_and_add papr failed\n");
>> + goto out_pgs;
>> + }
>> +
>> + esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
>> + if (!esi_kobj) {
>> + pr_warn("kobject_create_and_add energy_scale_info failed\n");
>> + goto out_kobj;
>> + }
>> +
>> + for (idx = 0; idx < num_attrs; idx++) {
>> + bool show_val_desc = true;
>> +
>> + pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
>> + sizeof(*pgs[idx].pg.attrs),
>> + GFP_KERNEL);
>> + if (!pgs[idx].pg.attrs) {
>> + for (i = idx - 1; i >= 0; i--)
>> + kfree(pgs[i].pg.attrs);
> What about the pg.name from the previous iterations?
>
>> + goto out_ekobj;
>> + }
>> +
>> + pgs[idx].pg.name = kasprintf(GFP_KERNEL, "%lld",
>> + be64_to_cpu(esi_attrs[idx].id));
>> + if (pgs[idx].pg.name == NULL) {
>> + for (i = idx; i >= 0; i--)
>> + kfree(pgs[i].pg.attrs);
> Here too.
>
> You could just 'goto out_pgattrs' in both cases.
Yeah, you're right. I may have over-complicated the free, in case of
failure in both the cases above I could just free from "out_pgattrs"
with no issues
>> + goto out_ekobj;
>> + }
>> + /* Do not add the value description if it does not exist */
>> + if (strnlen(esi_attrs[idx].value_desc,
>> + sizeof(esi_attrs[idx].value_desc)) == 0)
>> + show_val_desc = false;
>> +
>> + if (add_attr_group(be64_to_cpu(esi_attrs[idx].id), &pgs[idx],
>> + show_val_desc)) {
>> + pr_warn("Failed to create papr attribute group %s\n",
>> + pgs[idx].pg.name);
>> + goto out_pgattrs;
>> + }
>> + }
>> +
>> + kfree(esi_buf);
>> + return 0;
>> +
>> +out_pgattrs:
>> + for (i = 0; i < num_attrs ; i++) {
>> + kfree(pgs[i].pg.attrs);
>> + kfree(pgs[i].pg.name);
>> + }
>> +out_ekobj:
>> + kobject_put(esi_kobj);
>> +out_kobj:
>> + kobject_put(papr_kobj);
>> +out_pgs:
>> + kfree(pgs);
>> +out:
>> + kfree(esi_buf);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> +machine_device_initcall(pseries, papr_init);
^ permalink raw reply
* [RFC 1/2] ppc/fpu: Add generic FPU api similar to x86
From: Anson Jacob @ 2021-07-19 19:52 UTC (permalink / raw)
To: mpe, benh, paulus, christophe.leroy, linuxppc-dev, amd-gfx
Cc: Sunpeng.Li, Harry.Wentland, qingqing.zhuo, Rodrigo.Siqueira,
roman.li, Anson.Jacob, Aurabindo.Pillai, Bhawanpreet.Lakha,
bindu.r
In-Reply-To: <20210719195211.520428-1-Anson.Jacob@amd.com>
- Add kernel_fpu_begin & kernel_fpu_end API as x86
- Add logic similar to x86 to ensure fpu
begin/end call correctness
- Add kernel_fpu_enabled to know if FPU is enabled
Signed-off-by: Anson Jacob <Anson.Jacob@amd.com>
---
arch/powerpc/include/asm/switch_to.h | 29 ++----
arch/powerpc/kernel/process.c | 130 +++++++++++++++++++++++++++
2 files changed, 137 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 9d1fbd8be1c7..aded7aa661c0 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -41,10 +41,7 @@ extern void enable_kernel_fp(void);
extern void flush_fp_to_thread(struct task_struct *);
extern void giveup_fpu(struct task_struct *);
extern void save_fpu(struct task_struct *);
-static inline void disable_kernel_fp(void)
-{
- msr_check_and_clear(MSR_FP);
-}
+extern void disable_kernel_fp(void);
#else
static inline void save_fpu(struct task_struct *t) { }
static inline void flush_fp_to_thread(struct task_struct *t) { }
@@ -55,10 +52,7 @@ extern void enable_kernel_altivec(void);
extern void flush_altivec_to_thread(struct task_struct *);
extern void giveup_altivec(struct task_struct *);
extern void save_altivec(struct task_struct *);
-static inline void disable_kernel_altivec(void)
-{
- msr_check_and_clear(MSR_VEC);
-}
+extern void disable_kernel_altivec(void);
#else
static inline void save_altivec(struct task_struct *t) { }
static inline void __giveup_altivec(struct task_struct *t) { }
@@ -67,20 +61,7 @@ static inline void __giveup_altivec(struct task_struct *t) { }
#ifdef CONFIG_VSX
extern void enable_kernel_vsx(void);
extern void flush_vsx_to_thread(struct task_struct *);
-static inline void disable_kernel_vsx(void)
-{
- msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
-}
-#else
-static inline void enable_kernel_vsx(void)
-{
- BUILD_BUG();
-}
-
-static inline void disable_kernel_vsx(void)
-{
- BUILD_BUG();
-}
+extern void disable_kernel_vsx(void);
#endif
#ifdef CONFIG_SPE
@@ -114,4 +95,8 @@ static inline void clear_task_ebb(struct task_struct *t)
extern int set_thread_tidr(struct task_struct *t);
+bool kernel_fpu_enabled(void);
+void kernel_fpu_begin(void);
+void kernel_fpu_end(void);
+
#endif /* _ASM_POWERPC_SWITCH_TO_H */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 185beb290580..2ced8c6a3fab 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -75,6 +75,17 @@
#define TM_DEBUG(x...) do { } while(0)
#endif
+/*
+ * Track whether the kernel is using the FPU state
+ * currently.
+ *
+ * This flag is used:
+ *
+ * - kernel_fpu_begin()/end() correctness
+ * - kernel_fpu_enabled info
+ */
+static DEFINE_PER_CPU(bool, in_kernel_fpu);
+
extern unsigned long _get_SP(void);
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -212,6 +223,9 @@ void enable_kernel_fp(void)
unsigned long cpumsr;
WARN_ON(preemptible());
+ WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+
+ this_cpu_write(in_kernel_fpu, true);
cpumsr = msr_check_and_set(MSR_FP);
@@ -231,6 +245,15 @@ void enable_kernel_fp(void)
}
}
EXPORT_SYMBOL(enable_kernel_fp);
+
+void disable_kernel_fp(void)
+{
+ WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+
+ this_cpu_write(in_kernel_fpu, false);
+ msr_check_and_clear(MSR_FP);
+}
+EXPORT_SYMBOL(disable_kernel_fp);
#else
static inline void __giveup_fpu(struct task_struct *tsk) { }
#endif /* CONFIG_PPC_FPU */
@@ -263,6 +286,9 @@ void enable_kernel_altivec(void)
unsigned long cpumsr;
WARN_ON(preemptible());
+ WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+
+ this_cpu_write(in_kernel_fpu, true);
cpumsr = msr_check_and_set(MSR_VEC);
@@ -283,6 +309,14 @@ void enable_kernel_altivec(void)
}
EXPORT_SYMBOL(enable_kernel_altivec);
+void disable_kernel_altivec(void)
+{
+ WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+
+ this_cpu_write(in_kernel_fpu, false);
+ msr_check_and_clear(MSR_VEC);
+}
+EXPORT_SYMBOL(disable_kernel_altivec);
/*
* Make sure the VMX/Altivec register state in the
* the thread_struct is up to date for task tsk.
@@ -333,6 +367,9 @@ void enable_kernel_vsx(void)
unsigned long cpumsr;
WARN_ON(preemptible());
+ WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+
+ this_cpu_write(in_kernel_fpu, true);
cpumsr = msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX);
@@ -354,6 +391,15 @@ void enable_kernel_vsx(void)
}
EXPORT_SYMBOL(enable_kernel_vsx);
+void disable_kernel_vsx(void)
+{
+ WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+
+ this_cpu_write(in_kernel_fpu, false);
+ msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
+}
+EXPORT_SYMBOL(disable_kernel_vsx);
+
void flush_vsx_to_thread(struct task_struct *tsk)
{
if (tsk->thread.regs) {
@@ -406,6 +452,90 @@ void flush_spe_to_thread(struct task_struct *tsk)
}
#endif /* CONFIG_SPE */
+static bool fpu_support(void)
+{
+ if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+ return true;
+ } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+ return true;
+ } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+ return true;
+ }
+
+ return false;
+}
+
+bool kernel_fpu_enabled(void)
+{
+ return this_cpu_read(in_kernel_fpu);
+}
+EXPORT_SYMBOL(kernel_fpu_enabled);
+
+void kernel_fpu_begin(void)
+{
+ if (!fpu_support()) {
+ WARN_ON_ONCE(1);
+ return;
+ }
+
+ preempt_disable();
+
+#ifdef CONFIG_VSX
+ if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+ enable_kernel_vsx();
+ return;
+ }
+#endif
+
+#ifdef CONFIG_ALTIVEC
+ if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+ enable_kernel_altivec();
+ return;
+ }
+#endif
+
+#ifdef CONFIG_PPC_FPU
+ if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+ enable_kernel_fp();
+ return;
+ }
+#endif
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_begin);
+
+void kernel_fpu_end(void)
+{
+ if (!fpu_support()) {
+ WARN_ON_ONCE(1);
+ return;
+ }
+
+#ifdef CONFIG_VSX
+ if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+ disable_kernel_vsx();
+ goto done;
+ }
+#endif
+
+#ifdef CONFIG_ALTIVEC
+ if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+ disable_kernel_altivec();
+ goto done;
+ }
+#endif
+
+#ifdef CONFIG_PPC_FPU
+ if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+ disable_kernel_fp();
+ goto done;
+ }
+#endif
+
+done:
+ preempt_enable();
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_end);
+
static unsigned long msr_all_available;
static int __init init_msr_all_available(void)
--
2.25.1
^ permalink raw reply related
* [RFC 2/2] drm/amd/display: Use PPC FPU functions
From: Anson Jacob @ 2021-07-19 19:52 UTC (permalink / raw)
To: mpe, benh, paulus, christophe.leroy, linuxppc-dev, amd-gfx
Cc: Sunpeng.Li, Harry.Wentland, qingqing.zhuo, Rodrigo.Siqueira,
roman.li, Anson.Jacob, Aurabindo.Pillai, Bhawanpreet.Lakha,
bindu.r
In-Reply-To: <20210719195211.520428-1-Anson.Jacob@amd.com>
Use kernel_fpu_begin & kernel_fpu_end for PPC
Depends on "ppc/fpu: Add generic FPU api similar to x86"
Signed-off-by: Anson Jacob <Anson.Jacob@amd.com>
---
drivers/gpu/drm/amd/display/dc/os_types.h | 28 ++---------------------
1 file changed, 2 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
index 126c2f3a4dd3..999c5103357e 100644
--- a/drivers/gpu/drm/amd/display/dc/os_types.h
+++ b/drivers/gpu/drm/amd/display/dc/os_types.h
@@ -57,32 +57,8 @@
#define DC_FP_END() kernel_fpu_end()
#elif defined(CONFIG_PPC64)
#include <asm/switch_to.h>
-#include <asm/cputable.h>
-#define DC_FP_START() { \
- if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
- preempt_disable(); \
- enable_kernel_vsx(); \
- } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
- preempt_disable(); \
- enable_kernel_altivec(); \
- } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
- preempt_disable(); \
- enable_kernel_fp(); \
- } \
-}
-#define DC_FP_END() { \
- if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
- disable_kernel_vsx(); \
- preempt_enable(); \
- } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
- disable_kernel_altivec(); \
- preempt_enable(); \
- } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
- disable_kernel_fp(); \
- preempt_enable(); \
- } \
-}
-#endif
+#define DC_FP_START() kernel_fpu_begin()
+#define DC_FP_END() kernel_fpu_end()
#endif
/*
--
2.25.1
^ permalink raw reply related
* [RFC 0/2] Add generic FPU api similar to x86
From: Anson Jacob @ 2021-07-19 19:52 UTC (permalink / raw)
To: mpe, benh, paulus, christophe.leroy, linuxppc-dev, amd-gfx
Cc: Sunpeng.Li, Harry.Wentland, qingqing.zhuo, Rodrigo.Siqueira,
roman.li, Anson.Jacob, Aurabindo.Pillai, Bhawanpreet.Lakha,
bindu.r
This is an attempt to have generic FPU enable/disable
calls similar to x86.
So that we can simplify gpu/drm/amd/display/dc/os_types.h
Also adds FPU correctness logic seen in x86.
Anson Jacob (2):
ppc/fpu: Add generic FPU api similar to x86
drm/amd/display: Use PPC FPU functions
arch/powerpc/include/asm/switch_to.h | 29 ++---
arch/powerpc/kernel/process.c | 130 ++++++++++++++++++++++
drivers/gpu/drm/amd/display/dc/os_types.h | 28 +----
3 files changed, 139 insertions(+), 48 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg()
From: Boris Ostrovsky @ 2021-07-19 20:22 UTC (permalink / raw)
To: Logan Gunthorpe, linux-kernel, linux-alpha, linux-arm-kernel,
linux-ia64, linux-mips, linuxppc-dev, linux-s390, sparclinux,
iommu, linux-parisc, xen-devel
Cc: Juergen Gross, Stefano Stabellini, Konrad Rzeszutek Wilk,
Martin Oliveira, Stephen Bates, Robin Murphy, Christoph Hellwig,
Marek Szyprowski
In-Reply-To: <20210715164544.6827-14-logang@deltatee.com>
On 7/15/21 12:45 PM, Logan Gunthorpe wrote:
> From: Martin Oliveira <martin.oliveira@eideticom.com>
>
> The .map_sg() op now expects an error code instead of zero on failure.
>
> xen_swiotlb_map_sg() may only fail if xen_swiotlb_map_page() fails, but
> xen_swiotlb_map_page() only supports returning errors as
> DMA_MAPPING_ERROR. So coalesce all errors into EINVAL.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
^ permalink raw reply
* Re: [PATCH v5 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: Fabiano Rosas @ 2021-07-19 19:09 UTC (permalink / raw)
To: Pratik R. Sampat, mpe, benh, paulus, linuxppc-dev, kvm-ppc,
linux-kernel, psampat, pratik.r.sampat
In-Reply-To: <20210719093250.41405-2-psampat@linux.ibm.com>
"Pratik R. Sampat" <psampat@linux.ibm.com> writes:
> + pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
> + if (!pgs)
> + goto out;
> +
> + papr_kobj = kobject_create_and_add("papr", firmware_kobj);
> + if (!papr_kobj) {
> + pr_warn("kobject_create_and_add papr failed\n");
> + goto out_pgs;
> + }
> +
> + esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
> + if (!esi_kobj) {
> + pr_warn("kobject_create_and_add energy_scale_info failed\n");
> + goto out_kobj;
> + }
> +
> + for (idx = 0; idx < num_attrs; idx++) {
> + bool show_val_desc = true;
> +
> + pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
> + sizeof(*pgs[idx].pg.attrs),
> + GFP_KERNEL);
> + if (!pgs[idx].pg.attrs) {
> + for (i = idx - 1; i >= 0; i--)
> + kfree(pgs[i].pg.attrs);
What about the pg.name from the previous iterations?
> + goto out_ekobj;
> + }
> +
> + pgs[idx].pg.name = kasprintf(GFP_KERNEL, "%lld",
> + be64_to_cpu(esi_attrs[idx].id));
> + if (pgs[idx].pg.name == NULL) {
> + for (i = idx; i >= 0; i--)
> + kfree(pgs[i].pg.attrs);
Here too.
You could just 'goto out_pgattrs' in both cases.
> + goto out_ekobj;
> + }
> + /* Do not add the value description if it does not exist */
> + if (strnlen(esi_attrs[idx].value_desc,
> + sizeof(esi_attrs[idx].value_desc)) == 0)
> + show_val_desc = false;
> +
> + if (add_attr_group(be64_to_cpu(esi_attrs[idx].id), &pgs[idx],
> + show_val_desc)) {
> + pr_warn("Failed to create papr attribute group %s\n",
> + pgs[idx].pg.name);
> + goto out_pgattrs;
> + }
> + }
> +
> + kfree(esi_buf);
> + return 0;
> +
> +out_pgattrs:
> + for (i = 0; i < num_attrs ; i++) {
> + kfree(pgs[i].pg.attrs);
> + kfree(pgs[i].pg.name);
> + }
> +out_ekobj:
> + kobject_put(esi_kobj);
> +out_kobj:
> + kobject_put(papr_kobj);
> +out_pgs:
> + kfree(pgs);
> +out:
> + kfree(esi_buf);
> +
> + return -ENOMEM;
> +}
> +
> +machine_device_initcall(pseries, papr_init);
^ permalink raw reply
* Re: [PATCH v5 04/11] powerpc/pseries/iommu: Add ddw_list_new_entry() helper
From: Leonardo Brás @ 2021-07-19 18:47 UTC (permalink / raw)
To: Frederic Barrat, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <ae8fe44e-24c1-9d5f-70f9-96098167170a@linux.ibm.com>
On Mon, 2021-07-19 at 16:14 +0200, Frederic Barrat wrote:
>
>
> On 16/07/2021 10:27, Leonardo Bras wrote:
> > There are two functions creating direct_window_list entries in a
> > similar way, so create a ddw_list_new_entry() to avoid duplicity
> > and
> > simplify those functions.
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
>
> LGTM
> Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
>
Thanks!
^ permalink raw reply
* Re: [PATCH v5 03/11] powerpc/pseries/iommu: Add iommu_pseries_alloc_table() helper
From: Leonardo Brás @ 2021-07-19 18:47 UTC (permalink / raw)
To: Frederic Barrat, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <8110756f-a41f-3ba1-d7bc-af15630c4fbc@linux.ibm.com>
On Mon, 2021-07-19 at 16:04 +0200, Frederic Barrat wrote:
>
>
> On 16/07/2021 10:27, Leonardo Bras wrote:
> > Creates a helper to allow allocating a new iommu_table without the
> > need
> > to reallocate the iommu_group.
> >
> > This will be helpful for replacing the iommu_table for the new DMA
> > window,
> > after we remove the old one with iommu_tce_table_put().
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> > arch/powerpc/platforms/pseries/iommu.c | 25 ++++++++++++++-------
> > ----
> > 1 file changed, 14 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c
> > b/arch/powerpc/platforms/pseries/iommu.c
> > index b1b8d12bab39..33d82865d6e6 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -53,28 +53,31 @@ enum {
> > DDW_EXT_QUERY_OUT_SIZE = 2
> > };
> >
> > -static struct iommu_table_group *iommu_pseries_alloc_group(int
> > node)
> > +static struct iommu_table *iommu_pseries_alloc_table(int node)
> > {
> > - struct iommu_table_group *table_group;
> > struct iommu_table *tbl;
> >
> > - table_group = kzalloc_node(sizeof(struct
> > iommu_table_group), GFP_KERNEL,
> > - node);
> > - if (!table_group)
> > - return NULL;
> > -
> > tbl = kzalloc_node(sizeof(struct iommu_table), GFP_KERNEL,
> > node);
> > if (!tbl)
> > - goto free_group;
> > + return NULL;
> >
> > INIT_LIST_HEAD_RCU(&tbl->it_group_list);
> > kref_init(&tbl->it_kref);
> > + return tbl;
> > +}
> >
> > - table_group->tables[0] = tbl;
> > +static struct iommu_table_group *iommu_pseries_alloc_group(int
> > node)
> > +{
> > + struct iommu_table_group *table_group;
> > +
> > + table_group = kzalloc_node(sizeof(*table_group),
> > GFP_KERNEL, node);
> > + if (!table_group)
> > + return NULL;
> >
> > - return table_group;
> > + table_group->tables[0] = iommu_pseries_alloc_table(node);
> > + if (table_group->tables[0])
> > + return table_group;
>
>
> Nitpick: for readability, we'd usually expect the error path to be
> detected with the if statement and keep going on the good path, and
> here
> the code does the opposite. No big deal though, so
>
> Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
>
>
Thanks for the tip and review!
Best regards,
Leonardo Bras
> >
> > -free_group:
> > kfree(table_group);
> > return NULL;
> > }
> >
^ 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