* Re: [PATCH V3 1/2] mm: Update generic gup implementation to handle hugepage directory
From: Steve Capper @ 2014-10-28 10:41 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Andrea Arcangeli, linux-arch, linux-kernel, linux-mm, akpm,
linuxppc-dev
In-Reply-To: <1414233860-7683-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Sat, Oct 25, 2014 at 04:14:19PM +0530, Aneesh Kumar K.V wrote:
> Update generic gup implementation with powerpc specific details.
> On powerpc at pmd level we can have hugepte, normal pmd pointer
> or a pointer to the hugepage directory.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
Hi,
Apologies for not getting to this yesterday.
I have some comments below:
> Changes from V3:
> * Explain pgd_huge, also move the definition to linux/hugetlb.h.
> Both pgd_huge and is_hugepd are related to hugepages and hugetlb.h
> is the right header
>
> arch/arm/include/asm/pgtable.h | 2 +
> arch/arm64/include/asm/pgtable.h | 2 +
> arch/powerpc/include/asm/page.h | 1 +
> include/linux/hugetlb.h | 30 +++++++++++
> include/linux/mm.h | 7 +++
> mm/gup.c | 113 +++++++++++++++++++--------------------
> 6 files changed, 96 insertions(+), 59 deletions(-)
>
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index 3b30062..c52d261 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -181,6 +181,8 @@ extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
> /* to find an entry in a kernel page-table-directory */
> #define pgd_offset_k(addr) pgd_offset(&init_mm, addr)
>
> +#define pgd_huge(pgd) (0)
> +
Please remove this from the patch as it is no longer needed due to
some changes below.
> #define pmd_none(pmd) (!pmd_val(pmd))
> #define pmd_present(pmd) (pmd_val(pmd))
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 41a43bf..f532a14 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -464,6 +464,8 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
> extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
>
> +#define pgd_huge(pgd) (0)
> +
This too can be removed (see below)...
> /*
> * Encode and decode a swap entry:
> * bits 0-1: present (must be zero)
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 26fe1ae..f973fce 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -380,6 +380,7 @@ static inline int hugepd_ok(hugepd_t hpd)
> #endif
>
> #define is_hugepd(pdep) (hugepd_ok(*((hugepd_t *)(pdep))))
> +#define pgd_huge pgd_huge
> int pgd_huge(pgd_t pgd);
> #else /* CONFIG_HUGETLB_PAGE */
> #define is_hugepd(pdep) 0
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 6e6d338..de63dbc 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -175,6 +175,36 @@ static inline void __unmap_hugepage_range(struct mmu_gather *tlb,
> }
>
> #endif /* !CONFIG_HUGETLB_PAGE */
> +/*
> + * hugepages at page global directory. If arch support
> + * hugepages at pgd level, they need to define this.
> + */
> +#ifndef pgd_huge
> +#define pgd_huge(x) 0
> +#endif
This sequence now means we no longer need per-arch non-trivial
definitions of pgd_huge. i.e. it makes the arch/arm and arch/arm64
changes above superfluous.
> +
> +#ifndef is_hugepd
> +/*
> + * Some architectures requires a hugepage directory format that is
> + * required to support multiple hugepage sizes. For example
> + * a4fe3ce7699bfe1bd88f816b55d42d8fe1dac655 introduced the same
> + * on powerpc. This allows for a more flexible hugepage pagetable
> + * layout.
> + */
> +typedef struct { unsigned long pd; } hugepd_t;
> +#define is_hugepd(hugepd) (0)
> +#define __hugepd(x) ((hugepd_t) { (x) })
> +static inline int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
> + unsigned pdshift, unsigned long end,
> + int write, struct page **pages, int *nr)
> +{
> + return 0;
> +}
> +#else
> +extern int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
> + unsigned pdshift, unsigned long end,
> + int write, struct page **pages, int *nr);
> +#endif
>
> #define HUGETLB_ANON_FILE "anon_hugepage"
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 02d11ee..31d7fac 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1219,6 +1219,13 @@ long get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
> struct vm_area_struct **vmas);
> int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> struct page **pages);
> +
> +#ifdef CONFIG_HAVE_GENERIC_RCU_GUP
> +extern int gup_huge_pte(pte_t orig, pte_t *ptep, unsigned long addr,
> + unsigned long sz, unsigned long end, int write,
> + struct page **pages, int *nr);
> +#endif
> +
> struct kvec;
> int get_kernel_pages(const struct kvec *iov, int nr_pages, int write,
> struct page **pages);
> diff --git a/mm/gup.c b/mm/gup.c
> index cd62c8c..30773f3 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -786,65 +786,31 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
> }
> #endif /* __HAVE_ARCH_PTE_SPECIAL */
>
> -static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> - unsigned long end, int write, struct page **pages, int *nr)
> +int gup_huge_pte(pte_t orig, pte_t *ptep, unsigned long addr,
> + unsigned long sz, unsigned long end, int write,
> + struct page **pages, int *nr)
> {
> - struct page *head, *page, *tail;
> int refs;
> + unsigned long pte_end;
> + struct page *head, *page, *tail;
>
> - if (write && !pmd_write(orig))
> - return 0;
> -
> - refs = 0;
> - head = pmd_page(orig);
> - page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
> - tail = page;
> - do {
> - VM_BUG_ON_PAGE(compound_head(page) != head, page);
> - pages[*nr] = page;
> - (*nr)++;
> - page++;
> - refs++;
> - } while (addr += PAGE_SIZE, addr != end);
>
> - if (!page_cache_add_speculative(head, refs)) {
> - *nr -= refs;
> + if (write && !pte_write(orig))
> return 0;
> - }
>
> - if (unlikely(pmd_val(orig) != pmd_val(*pmdp))) {
> - *nr -= refs;
> - while (refs--)
> - put_page(head);
> + if (!pte_present(orig))
> return 0;
> - }
>
> - /*
> - * Any tail pages need their mapcount reference taken before we
> - * return. (This allows the THP code to bump their ref count when
> - * they are split into base pages).
> - */
> - while (refs--) {
> - if (PageTail(tail))
> - get_huge_page_tail(tail);
> - tail++;
> - }
> -
> - return 1;
> -}
> -
> -static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
> - unsigned long end, int write, struct page **pages, int *nr)
> -{
> - struct page *head, *page, *tail;
> - int refs;
> + pte_end = (addr + sz) & ~(sz-1);
> + if (pte_end < end)
> + end = pte_end;
>
> - if (write && !pud_write(orig))
> - return 0;
> + /* hugepages are never "special" */
> + VM_BUG_ON(!pfn_valid(pte_pfn(orig)));
>
> refs = 0;
> - head = pud_page(orig);
> - page = head + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
> + head = pte_page(orig);
> + page = head + ((addr & (sz-1)) >> PAGE_SHIFT);
> tail = page;
> do {
> VM_BUG_ON_PAGE(compound_head(page) != head, page);
> @@ -859,13 +825,18 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
> return 0;
> }
>
> - if (unlikely(pud_val(orig) != pud_val(*pudp))) {
> + if (unlikely(pte_val(orig) != pte_val(*ptep))) {
> *nr -= refs;
> while (refs--)
> put_page(head);
> return 0;
> }
>
> + /*
> + * Any tail pages need their mapcount reference taken before we
> + * return. (This allows the THP code to bump their ref count when
> + * they are split into base pages).
> + */
> while (refs--) {
> if (PageTail(tail))
> get_huge_page_tail(tail);
> @@ -898,10 +869,19 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
> if (pmd_numa(pmd))
> return 0;
>
> - if (!gup_huge_pmd(pmd, pmdp, addr, next, write,
> - pages, nr))
> + if (!gup_huge_pte(__pte(pmd_val(pmd)), (pte_t *)pmdp,
> + addr, PMD_SIZE, next,
> + write, pages, nr))
> return 0;
>
> + } else if (unlikely(is_hugepd(__hugepd(pmd_val(pmd))))) {
> + /*
> + * architecture have different format for hugetlbfs
> + * pmd format and THP pmd format
> + */
> + if (!gup_huge_pd(__hugepd(pmd_val(pmd)), addr,
> + PMD_SHIFT, next, write, pages, nr))
> + return 0;
> } else if (!gup_pte_range(pmd, addr, next, write, pages, nr))
> return 0;
> } while (pmdp++, addr = next, addr != end);
> @@ -909,22 +889,27 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
> return 1;
> }
The new definition of gup_huge_pte does get rid of a lot of code
duplication, but it also introduces the assumption that HugeTLB pages
and THPs can have their attributes referenced by pte_ accessors (rather
than something like pmd_write for instance). Please add this assumption
to the list in the file above.
This will probably be important for pte_pfn and pte_page in case an
architecture packs more attribute bits into what would normally be the
lower address bits of a pte. (for arm and arm64 these bits are
currently unused and expected to be zero so this code works).
>
> -static int gup_pud_range(pgd_t *pgdp, unsigned long addr, unsigned long end,
> +static int gup_pud_range(pgd_t pgd, unsigned long addr, unsigned long end,
> int write, struct page **pages, int *nr)
> {
> unsigned long next;
> pud_t *pudp;
>
> - pudp = pud_offset(pgdp, addr);
> + pudp = pud_offset(&pgd, addr);
> do {
> pud_t pud = ACCESS_ONCE(*pudp);
>
> next = pud_addr_end(addr, end);
> if (pud_none(pud))
> return 0;
> - if (pud_huge(pud)) {
> - if (!gup_huge_pud(pud, pudp, addr, next, write,
> - pages, nr))
> + if (unlikely(pud_huge(pud))) {
> + if (!gup_huge_pte(__pte(pud_val(pud)), (pte_t *)pudp,
> + addr, PUD_SIZE, next,
> + write, pages, nr))
> + return 0;
> + } else if (unlikely(is_hugepd(__hugepd(pud_val(pud))))) {
> + if (!gup_huge_pd(__hugepd(pud_val(pud)), addr,
> + PUD_SHIFT, next, write, pages, nr))
> return 0;
> } else if (!gup_pmd_range(pud, addr, next, write, pages, nr))
> return 0;
> @@ -970,10 +955,21 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> local_irq_save(flags);
> pgdp = pgd_offset(mm, addr);
> do {
> + pgd_t pgd = ACCESS_ONCE(*pgdp);
> +
> next = pgd_addr_end(addr, end);
> - if (pgd_none(*pgdp))
> + if (pgd_none(pgd))
> break;
> - else if (!gup_pud_range(pgdp, addr, next, write, pages, &nr))
> + if (unlikely(pgd_huge(pgd))) {
> + if (!gup_huge_pte(__pte(pgd_val(pgd)), (pte_t *)pgdp,
> + addr, PGDIR_SIZE, next,
> + write, pages, &nr))
> + break;
> + } else if (unlikely(is_hugepd(__hugepd(pgd_val(pgd))))) {
> + if (!gup_huge_pd(__hugepd(pgd_val(pgd)), addr,
> + PGDIR_SHIFT, next, write, pages, &nr))
> + break;
> + } else if (!gup_pud_range(pgd, addr, next, write, pages, &nr))
> break;
> } while (pgdp++, addr = next, addr != end);
> local_irq_restore(flags);
> @@ -1028,5 +1024,4 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
>
> return ret;
> }
> -
> #endif /* CONFIG_HAVE_GENERIC_RCU_GUP */
> --
With the above changes (remove the arch/arm and arch/arm64 changes and
add an extra assumption to the list for gup_huge_pte):
Acked-by: Steve Capper <steve.capper@linaro.org>
Also, I've tested this on an Arndale board (arm) and a Juno board
(arm64); running a custom futex on THP tail test.
Cheers,
--
Steve
^ permalink raw reply
* [PATCH v2 4/4] dt/bindings: Introduce the FSL QorIQ DPAA QMan portal(s)
From: Emil Medve @ 2014-10-28 9:15 UTC (permalink / raw)
To: scottwood, galak, corbet, robh+dt, ijc+devicetree, galak,
pawel.moll, mark.rutland, grant.likely, Geoff.Thorpe,
linuxppc-dev, devicetree, linux-doc
Cc: Emil Medve
In-Reply-To: <1414487751-31568-1-git-send-email-Emilian.Medve@Freescale.com>
Portals are memory mapped interfaces to QMan that allow low-latency,
lock-less interaction by software running on processor cores,
accelerators and network interfaces with the QMan
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
Change-Id: I29764fa8093b5ce65460abc879446795c50d7185
---
.../bindings/powerpc/fsl/qman-portals.txt | 151 +++++++++++++++++++++
1 file changed, 151 insertions(+)
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/qman-portals.txt
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/qman-portals.txt b/Documentation/devicetree/bindings/powerpc/fsl/qman-portals.txt
new file mode 100644
index 0000000..86b06d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/qman-portals.txt
@@ -0,0 +1,151 @@
+QorIQ DPAA Queue Manager Portals Device Tree Binding
+
+Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
+
+CONTENTS
+
+ - QMan Portal
+ - QMan Pool Channel
+ - Example
+
+QMan Portal Node
+
+PROPERTIES
+
+- compatible
+ Usage: Required
+ Value type: <stringlist>
+ Definition: Must include "fsl,qman-portal-<hardware revision>"
+ May include "fsl,<SoC>-qman-portal" or "fsl,qman-portal"
+
+- reg
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Two regions. The first is the cache-enabled region of
+ the portal. The second is the cache-inhibited region of
+ the portal
+
+- interrupts
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Standard property
+
+- fsl,liodn
+ Usage: See pamu.txt
+ Value type: <prop-encoded-array>
+ Definition: PAMU property used for static LIODN assignment
+
+- fsl,iommu-parent
+ Usage: See pamu.txt
+ Value type: <phandle>
+ Definition: PAMU property used for dynamic LIODN assignment. This is
+ an optional property. It is a valid configuration for
+ this phandle to be dangling
+
+ For additional details about the PAMU/LIODN binding(s) see pamu.txt
+
+- fsl,qman-channel-id
+ Usage: Required
+ Value type: <u32>
+ Definition: The hardware index of the channel. This can also be
+ determined by dividing any of the channel's 8 work queue
+ IDs by 8
+
+In addition to these properties the qman-portals should have sub-nodes to
+represent the HW devices/portals that are connected to the software portal
+described here
+
+The currently support sub-nodes are:
+ * fman@0
+ * fman@1
+ * pme@0
+ * crypto@0
+
+These subnodes should have the following properties:
+
+- fsl,liodn
+ Usage: See pamu.txt
+ Value type: <prop-encoded-array>
+ Definition: PAMU property used for static LIODN assignment
+
+- fsl,iommu-parent
+ Usage: See pamu.txt
+ Value type: <phandle>
+ Definition: PAMU property used for dynamic LIODN assignment
+
+- dev-handle
+ Usage: Required
+ Value type: <phandle>
+ Definition: The phandle to the particular hardware device that this
+ portal is connected to.
+
+DPAA QMan Pool Channel Nodes
+
+Pool Channels are defined with the following properties.
+
+PROPERTIES
+
+- compatible
+ Usage: Required
+ Value type: <stringlist>
+ Definition: Must include "fsl,qman-pool-channel"
+ May include "fsl,<SoC>-qman-pool-channel"
+
+- fsl,qman-channel-id
+ Usage: Required
+ Value type: <u32>
+ Definition: The hardware index of the channel. This can also be
+ determined by dividing any of the channel's 8 work queue
+ IDs by 8
+
+EXAMPLE
+
+The example below shows a (P4080) BMan portals container/bus node with two portals
+
+ qman-portals@ff4200000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+ ranges = <0 0xf 0xf4200000 0x200000>;
+
+ qman-portal@0 {
+ compatible = "fsl,qman-portal-1.2.0", "fsl,qman-portal";
+ reg = <0 0x4000>, <0x100000 0x1000>;
+ interrupts = <104 2 0 0>;
+ fsl,liodn = <1 2>;
+ fsl,qman-channel-id = <0>;
+
+ fman@0 {
+ fsl,liodn = <0x21>;
+ dev-handle = <&fman0>;
+ };
+ fman@1 {
+ fsl,liodn = <0xa1>;
+ dev-handle = <&fman1>;
+ };
+ crypto@0 {
+ fsl,liodn = <0x41 0x66>;
+ dev-handle = <&crypto>;
+ };
+ };
+ qman-portal@4000 {
+ compatible = "fsl,qman-portal-1.2.0", "fsl,qman-portal";
+ reg = <0x4000 0x4000>, <0x101000 0x1000>;
+ interrupts = <106 2 0 0>;
+ fsl,liodn = <3 4>;
+ fsl,qman-channel-id = <1>;
+
+ fman@1 {
+ fsl,liodn = <0xa2>;
+ dev-handle = <&fman0>;
+ };
+ fman@0 {
+ fsl,liodn = <0x22>;
+ dev-handle = <&fman1>;
+ };
+ crypto@0 {
+ fsl,liodn = <0x42 0x67>;
+ dev-handle = <&crypto>;
+ };
+ };
+ };
--
2.1.2
^ permalink raw reply related
* [PATCH v2 3/4] dt/bindings: Introduce the FSL QorIQ DPAA QMan
From: Emil Medve @ 2014-10-28 9:15 UTC (permalink / raw)
To: scottwood, galak, corbet, robh+dt, ijc+devicetree, galak,
pawel.moll, mark.rutland, grant.likely, Geoff.Thorpe,
linuxppc-dev, devicetree, linux-doc
Cc: Emil Medve
In-Reply-To: <1414487751-31568-1-git-send-email-Emilian.Medve@Freescale.com>
The Queue Manager is part of the Data-Path Acceleration Architecture (DPAA).
QMan supports queuing and QoS scheduling of frames to CPUs, network interfaces
and DPAA logic modules, maintains packet ordering within flows. Besides
providing flow-level queuing, is also responsible for congestion management
functions such as RED/WRED, congestion notifications and tail discards. This
binding covers the CCSR space programming model
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
Change-Id: I3acb223893e42003d6c9dc061db568ec0b10d29b
---
.../devicetree/bindings/powerpc/fsl/qman.txt | 133 +++++++++++++++++++++
1 file changed, 133 insertions(+)
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/qman.txt
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/qman.txt b/Documentation/devicetree/bindings/powerpc/fsl/qman.txt
new file mode 100644
index 0000000..a21e097
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/qman.txt
@@ -0,0 +1,133 @@
+QorIQ DPAA Queue Manager Device Tree Binding
+
+Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
+
+CONTENTS
+
+ - QMan Node
+ - QMan Private Memory Nodes
+ - Example
+
+QMan Node
+
+PROPERTIES
+
+- compatible
+ Usage: Required
+ Value type: <stringlist>
+ Definition: Must include "fsl,qman"
+ May include "fsl,<SoC>-qman"
+
+- reg
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Registers region within the CCSR address space
+
+- interrupts
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Standard property. The error interrupt
+
+- fsl,liodn
+ Usage: See pamu.txt
+ Value type: <prop-encoded-array>
+ Definition: PAMU property used for static LIODN assignment
+
+- fsl,iommu-parent
+ Usage: See pamu.txt
+ Value type: <phandle>
+ Definition: PAMU property used for dynamic LIODN assignment
+
+ For additional details about the PAMU/LIODN binding(s) see pamu.txt
+
+- clocks
+ Usage: See clock-bindings.txt and qoriq-clock.txt
+ Value type: <prop-encoded-array>
+ Definition: Reference input clock. Its frequency is half of the
+ platform clock
+
+QMan Private Memory Nodes
+
+QMan requires two contiguous range of physical memory used for the backing store
+for QMan Frame Queue Descriptor and Packed Frame Descriptor Record. This memory
+is reserved/allocated as a nodes under the /reserved-memory node
+
+The QMan FQD memory node must be named "qman-fqd"
+
+PROPERTIES
+
+- compatible
+ Usage: required
+ Value type: <stringlist>
+ Definition: Must inclide "fsl,qman-fqd"
+
+The QMan PFDR memory node must be named "qman-pfdr"
+
+PROPERTIES
+
+- compatible
+ Usage: required
+ Value type: <stringlist>
+ Definition: Must inclide "fsl,qman-pfdr"
+
+The following constraints are relevant to the FQD and PFDR private memory:
+ - The size must be 2^(size + 1), with size = 11..29. That is 4 KiB to
+ 1 GiB
+ - The alignment must be a muliptle of the memory size
+
+The size of the FQD and PFDP must be chosen by observing the hardware features
+configured via the RCW and that are relevant to a specific board (e.g. number of
+MAC(s) pinned-out, number of offline/host command FMan ports, etc.). The size
+configured in the DT must reflect the hardware capabilities and not the specific
+needs of an application
+
+For additional details about reserved memory regions see reserved-memory.txt
+
+EXAMPLE
+
+The example below shows a QMan FQD and a PFDR dynamic allocation memory nodes
+
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ qman-fqd {
+ compatible = "fsl,qman-fqd";
+ alloc-ranges = <0 0 0xf 0xffffffff>;
+ size = <0 0x400000>;
+ alignment = <0 0x400000>;
+ };
+ qman-pfdr {
+ compatible = "fsl,qman-pfdr";
+ alloc-ranges = <0 0 0xf 0xffffffff>;
+ size = <0 0x2000000>;
+ alignment = <0 0x2000000>;
+ };
+ };
+
+The example below shows a (P4080) QMan CCSR-space node
+
+ clockgen: global-utilities@e1000 {
+ ...
+ sysclk: sysclk {
+ ...
+ };
+ ...
+ platform_pll: platform-pll@c00 {
+ #clock-cells = <1>;
+ reg = <0xc00 0x4>;
+ compatible = "fsl,qoriq-platform-pll-1.0";
+ clocks = <&sysclk>;
+ clock-output-names = "platform-pll", "platform-pll-div2";
+ };
+ ...
+ };
+
+ qman@318000 {
+ compatible = "fsl,qman";
+ reg = <0x318000 0x1000>;
+ interrupts = <16 2 1 3>
+ fsl,liodn = <0x16>;
+ clocks = <&platform_pll 1>;
+ };
--
2.1.2
^ permalink raw reply related
* [PATCH v2 2/4] dt/bindings: Introduce the FSL QorIQ DPAA BMan portal(s)
From: Emil Medve @ 2014-10-28 9:15 UTC (permalink / raw)
To: scottwood, galak, corbet, robh+dt, ijc+devicetree, galak,
pawel.moll, mark.rutland, grant.likely, Geoff.Thorpe,
linuxppc-dev, devicetree, linux-doc
Cc: Emil Medve
In-Reply-To: <1414487751-31568-1-git-send-email-Emilian.Medve@Freescale.com>
Portals are memory mapped interfaces to BMan that allow low-latency,
lock-less interaction by software running on processor cores, accelerators
and network interfaces with the BMan
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
Change-Id: I6d245ffc14ba3d0e91d403ac7c3b91b75a9e6a95
---
.../bindings/powerpc/fsl/bman-portals.txt | 52 ++++++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
new file mode 100644
index 0000000..02e0231
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
@@ -0,0 +1,52 @@
+QorIQ DPAA Buffer Manager Portals Device Tree Binding
+
+Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
+
+CONTENTS
+
+ - BMan Portal
+ - Example
+
+BMan Portal Node
+
+PROPERTIES
+
+- compatible
+ Usage: Required
+ Value type: <stringlist>
+ Definition: Must include "fsl,bman-portal-<hardware revision>"
+ May include "fsl,<SoC>-bman-portal" or "fsl,bman-portal"
+
+- reg
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Two regions. The first is the cache-enabled region of
+ the portal. The second is the cache-inhibited region of
+ the portal
+
+- interrupts
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Standard property
+
+EXAMPLE
+
+The example below shows a (P4080) BMan portals container/bus node with two portals
+
+ bman-portals@ff4000000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+ ranges = <0 0xf 0xf4000000 0x200000>;
+
+ bman-portal@0 {
+ compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
+ reg = <0x0 0x4000>, <0x100000 0x1000>;
+ interrupts = <105 2 0 0>;
+ };
+ bman-portal@4000 {
+ compatible = "fsl,bman-portal-1.0.0", "fsl,bman-portal";
+ reg = <0x4000 0x4000>, <0x101000 0x1000>;
+ interrupts = <107 2 0 0>;
+ };
+ };
--
2.1.2
^ permalink raw reply related
* [PATCH v2 1/4] dt/bindings: Introduce the FSL QorIQ DPAA BMan
From: Emil Medve @ 2014-10-28 9:15 UTC (permalink / raw)
To: scottwood, galak, corbet, robh+dt, ijc+devicetree, galak,
pawel.moll, mark.rutland, grant.likely, Geoff.Thorpe,
linuxppc-dev, devicetree, linux-doc
Cc: Emil Medve
In-Reply-To: <1414487751-31568-1-git-send-email-Emilian.Medve@Freescale.com>
The Buffer Manager is part of the Data-Path Acceleration Architecture (DPAA).
BMan supports hardware allocation and deallocation of buffers belonging to
pools originally created by software with configurable depletion thresholds.
This binding covers the CCSR space programming model
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
Change-Id: I3ec479bfb3c91951e96902f091f5d7d2adbef3b2
---
.../devicetree/bindings/powerpc/fsl/bman.txt | 95 ++++++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/bman.txt
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/bman.txt b/Documentation/devicetree/bindings/powerpc/fsl/bman.txt
new file mode 100644
index 0000000..d3fd1e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/bman.txt
@@ -0,0 +1,95 @@
+QorIQ DPAA Buffer Manager Device Tree Bindings
+
+Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
+
+CONTENTS
+
+ - BMan Node
+ - BMan Private Memory Node
+ - Example
+
+BMan Node
+
+PROPERTIES
+
+- compatible
+ Usage: Required
+ Value type: <stringlist>
+ Definition: Must include "fsl,bman"
+ May include "fsl,<SoC>-bman"
+
+- reg
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Registers region within the CCSR address space
+
+- interrupts
+ Usage: Required
+ Value type: <prop-encoded-array>
+ Definition: Standard property. The error interrupt
+
+- fsl,liodn
+ Usage: See pamu.txt
+ Value type: <prop-encoded-array>
+ Definition: PAMU property used for static LIODN assignment
+
+- fsl,iommu-parent
+ Usage: See pamu.txt
+ Value type: <phandle>
+ Definition: PAMU property used for dynamic LIODN assignment
+
+ For additional details about the PAMU/LIODN binding(s) see pamu.txt
+
+BMan Private Memory Node
+
+BMan requires a contiguous range of physical memory used for the backing store
+for BMan Free Buffer Proxy Records. This memory is reserved/allocated as a node
+under the /reserved-memory node
+
+The BMan FBPR memory node must be named "bman-fbpr"
+
+PROPERTIES
+
+- compatible
+ Usage: required
+ Value type: <stringlist>
+ Definition: Must inclide "fsl,bman-fbpr"
+
+The following constraints are relevant to the FBPR private memory:
+ - The size must be 2^(size + 1), with size = 11..33. That is 4 KiB to
+ 16 GiB
+ - The alignment must be a muliptle of the memory size
+
+The size of the FBPR must be chosen by observing the hardware features configured
+via the RCW and that are relevant to a specific board (e.g. number of MAC(s)
+pinned-out, number of offline/host command FMan ports, etc.). The size configured
+in the DT must reflect the hardware capabilities and not the specific needs of an
+application
+
+For additional details about reserved memory regions see reserved-memory.txt
+
+EXAMPLE
+
+The example below shows a BMan FBPR dynamic allocation memory node
+
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ bman-fbpr {
+ compatible = "fsl,bman-fbpr";
+ alloc-ranges = <0 0 0xf 0xffffffff>;
+ size = <0 0x1000000>;
+ alignment = <0 0x1000000>;
+ };
+ };
+
+The example below shows a (P4080) BMan CCSR-space node
+
+ bman@31a000 {
+ compatible = "fsl,bman";
+ reg = <0x31a000 0x1000>;
+ interrupts = <16 2 1 2>;
+ fsl,liodn = <0x17>;
+ };
--
2.1.2
^ permalink raw reply related
* [PATCH v2 0/4] dt/bindings: Introduce the FSL B/QMan
From: Emil Medve @ 2014-10-28 9:15 UTC (permalink / raw)
To: scottwood, galak, corbet, robh+dt, ijc+devicetree, galak,
pawel.moll, mark.rutland, grant.likely, Geoff.Thorpe,
linuxppc-dev, devicetree, linux-doc
Cc: Emil Medve
v2: Incorporate feedback from Mark Rutland
- Remove "subject to change" notes
- Add/document the 'interrupts' properties
- Make multiple windows 'reg' properties more readable
- Improve portal description
Emil Medve (4):
dt/bindings: Introduce the FSL QorIQ DPAA BMan
dt/bindings: Introduce the FSL QorIQ DPAA BMan portal(s)
dt/bindings: Introduce the FSL QorIQ DPAA QMan
dt/bindings: Introduce the FSL QorIQ DPAA QMan portal(s)
.../bindings/powerpc/fsl/bman-portals.txt | 52 +++++++
.../devicetree/bindings/powerpc/fsl/bman.txt | 95 +++++++++++++
.../bindings/powerpc/fsl/qman-portals.txt | 151 +++++++++++++++++++++
.../devicetree/bindings/powerpc/fsl/qman.txt | 133 ++++++++++++++++++
4 files changed, 431 insertions(+)
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/bman-portals.txt
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/bman.txt
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/qman-portals.txt
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/qman.txt
--
2.1.2
^ permalink raw reply
* Re: lockdep warning with 2d65a9f48fcdf7866aab6457bc707ca233e0c791
From: Paolo Bonzini @ 2014-10-28 9:02 UTC (permalink / raw)
To: Aneesh Kumar K.V, kvm-ppc@vger.kernel.org, Paul Mackerras,
linuxppc-dev
In-Reply-To: <87d29tkzsi.fsf@linux.vnet.ibm.com>
On 10/15/2014 07:28 PM, Aneesh Kumar K.V wrote:
>
> =============================================
> [ INFO: possible recursive locking detected ]
> 3.17.0+ #31 Not tainted
> ---------------------------------------------
> qemu-system-ppc/9112 is trying to acquire lock:
> (&(&vcpu->arch.tbacct_lock)->rlock){......}, at: [<d000000011591f84>] .vcore_stolen_time+0x44/0xb0 [kvm_hv]
>
> but task is already holding lock:
> (&(&vcpu->arch.tbacct_lock)->rlock){......}, at: [<d000000011592524>] .kvmppc_remove_runnable.part.2+0x34/0xd0 [kvm_hv]
This must come from here:
while (vcpu->arch.state == KVMPPC_VCPU_RUNNABLE &&
(vc->vcore_state == VCORE_RUNNING ||
vc->vcore_state == VCORE_EXITING)) {
spin_unlock(&vc->lock);
kvmppc_wait_for_exec(vcpu, TASK_UNINTERRUPTIBLE);
spin_lock(&vc->lock);
}
if (vcpu->arch.state == KVMPPC_VCPU_RUNNABLE) {
kvmppc_remove_runnable(vc, vcpu);
vcpu->stat.signal_exits++;
kvm_run->exit_reason = KVM_EXIT_INTR;
vcpu->arch.ret = -EINTR;
}
if vc->vcore_state is VCORE_SLEEPING (I think it cannot be VCORE_STARTING)?
Paolo
^ permalink raw reply
* Re: [PATCH v2 4/4] CXL: Fix PSL error due to duplicate segment table entries
From: Aneesh Kumar K.V @ 2014-10-28 8:16 UTC (permalink / raw)
To: Ian Munsie, mpe
Cc: cbe-oss-dev, mikey, arnd, greg, linux-kernel, linuxppc-dev, anton,
imunsie, jk
In-Reply-To: <1414466730-15591-5-git-send-email-imunsie@au.ibm.com>
Ian Munsie <imunsie@au1.ibm.com> writes:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> In certain circumstances the PSL (Power Service Layer, which provides
> translation services for CXL hardware) can send an interrupt for a
> segment miss that the kernel has already handled. This can happen if
> multiple translations for the same segment are queued in the PSL before
> the kernel has restarted the first translation.
>
> The CXL driver does not expect this situation and does not check if a
> segment had already been handled. This could cause a duplicate segment
> table entry which in turn caused a PSL error taking down the card.
>
> This patch fixes the issue by checking for existing entries in the
> segment table that match the segment it is trying to insert to avoid
> inserting duplicate entries.
>
> This patch requires "powerpc/copro: Use appropriate ESID mask in
> copro_calculate_slb" for the sste_matches function to operate correctly.
>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> drivers/misc/cxl/fault.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
> index cb4f323..c99e896 100644
> --- a/drivers/misc/cxl/fault.c
> +++ b/drivers/misc/cxl/fault.c
> @@ -21,11 +21,20 @@
>
> #include "cxl.h"
>
> -/* This finds a free SSTE for the given SLB */
> +static bool sste_matches(struct cxl_sste *sste, struct copro_slb
> *slb)
inline ?
> +{
> + return ((sste->vsid_data == cpu_to_be64(slb->vsid)) &&
> + (sste->esid_data == cpu_to_be64(slb->esid)));
why comparing converting to big endian ? The natural way is to do it
other way right ?.
(be64_to_cpu(sste->vsid_data) == slb->vsid)
> +}
> +
> +/*
> + * This finds a free SSTE for the given SLB, or returns NULL if it's already in
> + * the segment table.
> + */
> static struct cxl_sste* find_free_sste(struct cxl_context *ctx,
> struct copro_slb *slb)
> {
> - struct cxl_sste *primary, *sste;
> + struct cxl_sste *primary, *sste, *ret = NULL;
> unsigned int mask = (ctx->sst_size >> 7) - 1; /* SSTP0[SegTableSize] */
> unsigned int entry;
> unsigned int hash;
> @@ -38,15 +47,19 @@ static struct cxl_sste* find_free_sste(struct cxl_context *ctx,
> primary = ctx->sstp + (hash << 3);
>
> for (entry = 0, sste = primary; entry < 8; entry++, sste++) {
> - if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
> - return sste;
> + if (!ret && !(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
> + ret = sste;
> + if (sste_matches(sste, slb))
> + return NULL;
> }
> + if (ret)
> + return ret;
>
> /* Nothing free, select an entry to cast out */
> - sste = primary + ctx->sst_lru;
> + ret = primary + ctx->sst_lru;
> ctx->sst_lru = (ctx->sst_lru + 1) & 0x7;
>
> - return sste;
> + return ret;
> }
>
> static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
> @@ -57,12 +70,15 @@ static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
>
> spin_lock_irqsave(&ctx->sste_lock, flags);
> sste = find_free_sste(ctx, slb);
> + if (!sste)
> + goto out_unlock;
>
> pr_devel("CXL Populating SST[%li]: %#llx %#llx\n",
> sste - ctx->sstp, slb->vsid, slb->esid);
>
> sste->vsid_data = cpu_to_be64(slb->vsid);
> sste->esid_data = cpu_to_be64(slb->esid);
> +out_unlock:
> spin_unlock_irqrestore(&ctx->sste_lock, flags);
> }
>
> --
> 2.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/copro: Use appropriate ESID mask in copro_calculate_slb
From: Aneesh Kumar K.V @ 2014-10-28 8:13 UTC (permalink / raw)
To: Ian Munsie, mpe
Cc: cbe-oss-dev, mikey, arnd, greg, linux-kernel, linuxppc-dev, anton,
imunsie, jk
In-Reply-To: <1414466730-15591-4-git-send-email-imunsie@au.ibm.com>
Ian Munsie <imunsie@au1.ibm.com> writes:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> This patch makes copro_calculate_slb mask the ESID by the correct mask
> for 1T vs 256M segments.
>
> This has no effect by itself as the extra bits were ignored, but it
> makes debugging the segment table entries easier and means that we can
> directly compare the ESID values for duplicates without needing to worry
> about masking in the comparison.
>
> This will be used to simplify a comparison in the following patch.
>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/copro_fault.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
> index 0f9939e..5a236f0 100644
> --- a/arch/powerpc/mm/copro_fault.c
> +++ b/arch/powerpc/mm/copro_fault.c
> @@ -99,8 +99,6 @@ int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb)
> u64 vsid;
> int psize, ssize;
>
> - slb->esid = (ea & ESID_MASK) | SLB_ESID_V;
> -
> switch (REGION_ID(ea)) {
> case USER_REGION_ID:
> pr_devel("%s: 0x%llx -- USER_REGION_ID\n", __func__, ea);
> @@ -133,6 +131,7 @@ int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb)
> vsid |= mmu_psize_defs[psize].sllp |
> ((ssize == MMU_SEGSIZE_1T) ? SLB_VSID_B_1T : 0);
>
> + slb->esid = (ea & (ssize == MMU_SEGSIZE_1T ? ESID_MASK_1T : ESID_MASK)) | SLB_ESID_V;
> slb->vsid = vsid;
>
> return 0;
> --
> 2.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v2 2/4] CXL: Refactor cxl_load_segment and find_free_sste
From: Aneesh Kumar K.V @ 2014-10-28 8:13 UTC (permalink / raw)
To: Ian Munsie, mpe
Cc: cbe-oss-dev, mikey, arnd, greg, linux-kernel, linuxppc-dev, anton,
imunsie, jk
In-Reply-To: <1414466730-15591-3-git-send-email-imunsie@au.ibm.com>
Ian Munsie <imunsie@au1.ibm.com> writes:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> This moves the segment table hash calculation from cxl_load_segment into
> find_free_sste since that is the only place it is actually used.
>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> drivers/misc/cxl/fault.c | 34 ++++++++++++++++++----------------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
> index d0e97fd..cb4f323 100644
> --- a/drivers/misc/cxl/fault.c
> +++ b/drivers/misc/cxl/fault.c
> @@ -21,20 +21,30 @@
>
> #include "cxl.h"
>
> -static struct cxl_sste* find_free_sste(struct cxl_sste *primary_group,
> - unsigned int *lru)
> +/* This finds a free SSTE for the given SLB */
> +static struct cxl_sste* find_free_sste(struct cxl_context *ctx,
> + struct copro_slb *slb)
> {
> + struct cxl_sste *primary, *sste;
> + unsigned int mask = (ctx->sst_size >> 7) - 1; /* SSTP0[SegTableSize] */
> unsigned int entry;
> - struct cxl_sste *sste, *group = primary_group;
> + unsigned int hash;
> +
> + if (slb->vsid & SLB_VSID_B_1T)
> + hash = (slb->esid >> SID_SHIFT_1T) & mask;
> + else /* 256M */
> + hash = (slb->esid >> SID_SHIFT) & mask;
>
> - for (entry = 0; entry < 8; entry++) {
> - sste = group + entry;
> + primary = ctx->sstp + (hash << 3);
> +
> + for (entry = 0, sste = primary; entry < 8; entry++, sste++) {
> if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
> return sste;
> }
> +
> /* Nothing free, select an entry to cast out */
> - sste = primary_group + *lru;
> - *lru = (*lru + 1) & 0x7;
> + sste = primary + ctx->sst_lru;
> + ctx->sst_lru = (ctx->sst_lru + 1) & 0x7;
>
> return sste;
> }
> @@ -42,19 +52,11 @@ static struct cxl_sste* find_free_sste(struct cxl_sste *primary_group,
> static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
> {
> /* mask is the group index, we search primary and secondary here. */
> - unsigned int mask = (ctx->sst_size >> 7)-1; /* SSTP0[SegTableSize] */
> struct cxl_sste *sste;
> - unsigned int hash;
> unsigned long flags;
>
> -
> - if (slb->vsid & SLB_VSID_B_1T)
> - hash = (slb->esid >> SID_SHIFT_1T) & mask;
> - else /* 256M */
> - hash = (slb->esid >> SID_SHIFT) & mask;
> -
> spin_lock_irqsave(&ctx->sste_lock, flags);
> - sste = find_free_sste(ctx->sstp + (hash << 3), &ctx->sst_lru);
> + sste = find_free_sste(ctx, slb);
>
> pr_devel("CXL Populating SST[%li]: %#llx %#llx\n",
> sste - ctx->sstp, slb->vsid, slb->esid);
> --
> 2.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v2 1/4] CXL: Disable secondary hash in segment table
From: Aneesh Kumar K.V @ 2014-10-28 8:12 UTC (permalink / raw)
To: Ian Munsie, mpe
Cc: cbe-oss-dev, mikey, arnd, greg, linux-kernel, linuxppc-dev, anton,
imunsie, jk
In-Reply-To: <1414466730-15591-2-git-send-email-imunsie@au.ibm.com>
Ian Munsie <imunsie@au1.ibm.com> writes:
> From: Ian Munsie <imunsie@au1.ibm.com>
>
> This patch simplifies the process of finding a free segment table entry
> by disabling the secondary hash. This reduces the number of possible
> entries in the segment table for a given address from 16 to 8.
>
> Due to the large segment sizes we use it is extremely unlikely that the
> secondary hash would ever have been used in practice, so this should not
> have any negative impacts and may even improve performance due to the
> reduced number of comparisons that software & hardware need to perform.
>
> This patch clears the SC bit in the hardware's state register
> (CXL_PSL_SR_An) to disable the secondary hash in the hardware since we
> can no longer fill out entries using it.
This could have also gone as a code comment.
>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> drivers/misc/cxl/fault.c | 30 ++++++++----------------------
> drivers/misc/cxl/native.c | 4 ++--
> 2 files changed, 10 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
> index 69506eb..d0e97fd 100644
> --- a/drivers/misc/cxl/fault.c
> +++ b/drivers/misc/cxl/fault.c
> @@ -22,29 +22,19 @@
> #include "cxl.h"
>
> static struct cxl_sste* find_free_sste(struct cxl_sste *primary_group,
> - bool sec_hash,
> - struct cxl_sste *secondary_group,
> unsigned int *lru)
> {
> - unsigned int i, entry;
> + unsigned int entry;
> struct cxl_sste *sste, *group = primary_group;
>
> - for (i = 0; i < 2; i++) {
> - for (entry = 0; entry < 8; entry++) {
> - sste = group + entry;
> - if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
> - return sste;
> - }
> - if (!sec_hash)
> - break;
> - group = secondary_group;
> + for (entry = 0; entry < 8; entry++) {
> + sste = group + entry;
> + if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
> + return sste;
> }
> /* Nothing free, select an entry to cast out */
> - if (sec_hash && (*lru & 0x8))
> - sste = secondary_group + (*lru & 0x7);
> - else
> - sste = primary_group + (*lru & 0x7);
> - *lru = (*lru + 1) & 0xf;
> + sste = primary_group + *lru;
> + *lru = (*lru + 1) & 0x7;
>
> return sste;
> }
> @@ -53,22 +43,18 @@ static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
> {
> /* mask is the group index, we search primary and secondary here. */
> unsigned int mask = (ctx->sst_size >> 7)-1; /* SSTP0[SegTableSize] */
> - bool sec_hash = 1;
> struct cxl_sste *sste;
> unsigned int hash;
> unsigned long flags;
>
>
> - sec_hash = !!(cxl_p1n_read(ctx->afu, CXL_PSL_SR_An) & CXL_PSL_SR_An_SC);
> -
> if (slb->vsid & SLB_VSID_B_1T)
> hash = (slb->esid >> SID_SHIFT_1T) & mask;
> else /* 256M */
> hash = (slb->esid >> SID_SHIFT) & mask;
>
> spin_lock_irqsave(&ctx->sste_lock, flags);
> - sste = find_free_sste(ctx->sstp + (hash << 3), sec_hash,
> - ctx->sstp + ((~hash & mask) << 3), &ctx->sst_lru);
> + sste = find_free_sste(ctx->sstp + (hash << 3), &ctx->sst_lru);
>
> pr_devel("CXL Populating SST[%li]: %#llx %#llx\n",
> sste - ctx->sstp, slb->vsid, slb->esid);
> diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
> index 623286a..d47532e 100644
> --- a/drivers/misc/cxl/native.c
> +++ b/drivers/misc/cxl/native.c
> @@ -417,7 +417,7 @@ static int attach_afu_directed(struct cxl_context *ctx, u64 wed, u64 amr)
> ctx->elem->haurp = 0; /* disable */
> ctx->elem->sdr = cpu_to_be64(mfspr(SPRN_SDR1));
>
> - sr = CXL_PSL_SR_An_SC;
> + sr = 0;
> if (ctx->master)
> sr |= CXL_PSL_SR_An_MP;
> if (mfspr(SPRN_LPCR) & LPCR_TC)
> @@ -508,7 +508,7 @@ static int attach_dedicated(struct cxl_context *ctx, u64 wed, u64 amr)
> u64 sr;
> int rc;
>
> - sr = CXL_PSL_SR_An_SC;
> + sr = 0;
> set_endian(sr);
> if (ctx->master)
> sr |= CXL_PSL_SR_An_MP;
> --
> 2.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: powerc: fix build failure when CONFIG_HUGETLB_PAGE is not set
From: Cedric Le Goater @ 2014-10-28 7:29 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20141028043122.9E00E14007F@ozlabs.org>
Hello Michael,
On 10/28/2014 05:31 AM, Michael Ellerman wrote:
> On Mon, 2014-27-10 at 14:30:06 UTC, =?utf-8?q?C=C3=A9dric_Le_Goater?= wrote:
>> CC arch/powerpc/mm/slice.o
>> In file included from ../arch/powerpc/mm/slice.c:33:0:
>> ../include/linux/hugetlb.h:141:47: error: expected identifier or ‘(’ before numeric constant
>> #define is_hugepage_only_range(mm, addr, len) 0
>> ^
>> ../arch/powerpc/mm/slice.c:704:5: note: in expansion of macro ‘is_hugepage_only_range’
>> int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
>> ^
>
> Hi Cedric,
>
> I'm pretty sure this is fixed in my fixes branch:
>
> https://git.kernel.org/cgit/linux/kernel/git/mpe/linux.git/log/?h=fixes
Indeed. I just picked -rc2 without checking your branch. I will next time.
Thanks,
C.
^ permalink raw reply
* Re: [RFC PATCH] dt:numa: adding numa node mapping for memory nodes.
From: Hanjun Guo @ 2014-10-28 7:27 UTC (permalink / raw)
To: Mark Rutland, Kumar Gala, Ganapatrao Kulkarni
Cc: open list:OPEN FIRMWARE AND..., Pawel Moll, Ian Campbell,
Rob Herring, linuxppc-dev, linux-arm-kernel@lists.infradead.org,
gpkulkarni@gmail.com
In-Reply-To: <20140917193452.GD7983@leverpostej>
On 2014-9-18 3:34, Mark Rutland wrote:
> On Wed, Sep 17, 2014 at 04:37:30PM +0100, Kumar Gala wrote:
>>
>> On Sep 17, 2014, at 1:56 AM, Ganapatrao Kulkarni <ganapatrao.kulkarni@caviumnetworks.com> wrote:
>>
>>> From: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>>>
>>> This patch adds property "nid" to memory node to provide the memory range to
>>> numa node id mapping.
>>>
>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>>>
>>> —
>>
>> Adding the PPC guys as they’ve been doing NUMA on IBM Power Servers
>> for years with OF/DT. So we should really try and follow what they’ve
>> done.
>
> Agreed.
>
>>> Documentation/devicetree/bindings/numa.txt | 58 ++++++++++++++++++++++++++++++
>>> 1 file changed, 58 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/numa.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
>>> new file mode 100644
>>> index 0000000..c4a94f2
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/numa.txt
>>> @@ -0,0 +1,58 @@
>>> +======================================================
>>> +numa id binding description
>>> +======================================================
>>> +
>>> +======================================================
>>> +1 - Introduction
>>> +======================================================
>>> +The device node property "nid(numa node id)" can be added to memory
>
> Why the quotes?
>
>>> +device node to map the range of memory addresses as defined in property "reg".
>>> +The property "nid" maps the memory range to the numa node id, which is used to
>>> +find the local and remory pages on numa aware systems.
>
> What is a "numa node id", exactly, and how is the OS intended to use it?
I think "Proximity Domain" would be more suitably, processors and memory or IOs
in the same domain will have better performance than crossing other domains.
Thanks
Hanjun
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/ftrace: simplify prepare_ftrace_return
From: Anton Blanchard @ 2014-10-28 4:55 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: paulus, linuxppc-dev, Steven Rostedt, Alan Modra
In-Reply-To: <20140927002228.GC18404@gate.crashing.org>
Hi Segher,
> On Wed, Sep 24, 2014 at 12:33:07PM +1000, Anton Blanchard wrote:
> > We are scratching our heads trying to remember details of the issue
> > right now. In retrospect we should have linked the gcc bugzilla or
> > gcc commit details in the kernel commit message :)
>
> There have been many GCC bugs in this area.
>
> 30282 (for 32-bit)
> 44199 (for 64-bit)
> 52828 (for everything, and this one should finally handle things for
> good) Also a bunch of duplicates, and I'm sure I've missed some more.
>
> The original issue as far as I remember: when using a frame pointer,
> GCC would sometimes schedule the epilogue to do the stack adjust
> before restoring all regs from the stack. Then an interrupt comes
> in, those saved regs are clobbered, kaboom. We cannot disable the
> frame pointer because -pg forces it (although PowerPC does not need
> it). The -mno-sched-epilog flag is a workaround: the epilogue (and
> prologue) will not be reordered by instruction scheduling. Slow code
> is better than blowing up fast ;-)
Thanks for explaining it! It does look like the last issue wasn't
fixed until gcc 4.8. We'll drop that patch.
Anton
^ permalink raw reply
* Re: powerc: fix build failure when CONFIG_HUGETLB_PAGE is not set
From: Michael Ellerman @ 2014-10-28 4:31 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <1414420206-7869-1-git-send-email-clg@fr.ibm.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 689 bytes --]
On Mon, 2014-27-10 at 14:30:06 UTC, =?utf-8?q?C=C3=A9dric_Le_Goater?= wrote:
> CC arch/powerpc/mm/slice.o
> In file included from ../arch/powerpc/mm/slice.c:33:0:
> ../include/linux/hugetlb.h:141:47: error: expected identifier or ‘(’ before numeric constant
> #define is_hugepage_only_range(mm, addr, len) 0
> ^
> ../arch/powerpc/mm/slice.c:704:5: note: in expansion of macro ‘is_hugepage_only_range’
> int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
> ^
Hi Cedric,
I'm pretty sure this is fixed in my fixes branch:
https://git.kernel.org/cgit/linux/kernel/git/mpe/linux.git/log/?h=fixes
cheers
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_asrc: Add reg_defaults for regmap to fix kernel dump
From: Nicolin Chen @ 2014-10-28 4:26 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, linuxppc-dev, linux-kernel
In-Reply-To: <20141028001904.GH18557@sirena.org.uk>
On Tue, Oct 28, 2014 at 12:19:04AM +0000, Mark Brown wrote:
> On Fri, Oct 24, 2014 at 07:03:57PM -0700, Nicolin Chen wrote:
> > Kernel dump (WARN_ON) ocurred during system boot-up inside regmap_write():
> >
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 47 at kernel/locking/lockdep.c:2744 lockdep_trace_alloc+0xe8/0x108()
> > Even though this may be a bug that should be fixed, I still try to send this
> > patch as a quick fix (work around) since it does no harm to assign default
> > values of every registers when using regcache.
>
> It's not a bug, it's not reasonable to default allocations to atomic and
> we can't really tell what context we're in. Anything used inside a
> heavily locked path should either have a default provided or arrange for
> a prior write to set up the cache.
I've a little trouble to understand the prior write over here. Inside my
probe() there's a register_init() call which has a set of regmap_write().
And then the first regmap_write() results the dump. Does that mean this
regmap_write() isn't prior write? If so, how should I do if not setting
default values here -- Some IPs may have default value 0 for registers.
And this would make reg_defaults tedious since there's nothing special
to assign.
And actually I remember I haven't seen any dump when I sent the initial
patch for the whole ASRC driver but it manifested last month. I thought
it might be a partially-merging issue and it would be fixed after merge
window's done.....
Thank you
Nicolin
^ permalink raw reply
* Re: [PATCH] Revert "powerpc/powernv: Fix endian bug in LPC bus debugfs accessors"
From: Benjamin Herrenschmidt @ 2014-10-28 3:53 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <1414458920-7166-1-git-send-email-mpe@ellerman.id.au>
On Tue, 2014-10-28 at 12:15 +1100, Michael Ellerman wrote:
> This reverts commit bf7588a0859580a45c63cb082825d77c13eca357.
>
> Ben says, "The original code was fine, I think the bug was actually in
> the userspace tool I was using (don't worry, nothing on the field uses
> that interface yet and I can still fix the tool)."
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
The original code wasn't fine actually :-) But the fix was completely
wrong and does more damages than it fixes things. So Ack the revert and
I'll sort things out properly later.
Cheers,
Ben.
> ---
> arch/powerpc/platforms/powernv/opal-lpc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-lpc.c b/arch/powerpc/platforms/powernv/opal-lpc.c
> index dd2c285ad170..ad4b31df779a 100644
> --- a/arch/powerpc/platforms/powernv/opal-lpc.c
> +++ b/arch/powerpc/platforms/powernv/opal-lpc.c
> @@ -191,7 +191,6 @@ static ssize_t lpc_debug_read(struct file *filp, char __user *ubuf,
> {
> struct lpc_debugfs_entry *lpc = filp->private_data;
> u32 data, pos, len, todo;
> - __be32 bedata;
> int rc;
>
> if (!access_ok(VERIFY_WRITE, ubuf, count))
> @@ -214,10 +213,9 @@ static ssize_t lpc_debug_read(struct file *filp, char __user *ubuf,
> len = 2;
> }
> rc = opal_lpc_read(opal_lpc_chip_id, lpc->lpc_type, pos,
> - &bedata, len);
> + &data, len);
> if (rc)
> return -ENXIO;
> - data = be32_to_cpu(bedata);
> switch(len) {
> case 4:
> rc = __put_user((u32)data, (u32 __user *)ubuf);
^ permalink raw reply
* Re: powerpc: Replace __get_cpu_var uses
From: Benjamin Herrenschmidt @ 2014-10-28 3:51 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Christoph Lameter
In-Reply-To: <1414447475.1913.1.camel@concordia>
On Tue, 2014-10-28 at 09:04 +1100, Michael Ellerman wrote:
> On Tue, 2014-10-28 at 08:10 +1100, Benjamin Herrenschmidt wrote:
> > On Mon, 2014-10-27 at 10:57 -0500, Christoph Lameter wrote:
> > > Ping? We are planning to remove support for __get_cpu_var in the
> > > 3.19 merge period. I can move the definition for __get_cpu_var into the
> > > powerpc per cpu definition instead if we cannot get this merged?
> >
> > Yes, yes, I know, we should merge that, Michael, can you still throw
> > that in ?
>
> For 3.18, nah. We didn't even see it until after the merge window had closed.
No, we did see it a long time ago Michael, we just forgot about it. We
should put it in, everybody else has their equivalent.
> I realise they're all fairly simple translations, but it's too big and too easy
> for one of them to be wrong.
>
> I'm happy to put it in a topic branch for 3.19, or move the definition or
> whatever, your choice Christoph.
But then we'll have a tricky sync needed to make sure the removal of
__get_cpu_var() doesn't get into Linus tree before this. Unless we just
ack the whole lot and let Christoph carry it in the same branch where
the removal is.
Ben.
^ permalink raw reply
* [PATCH v2 2/4] CXL: Refactor cxl_load_segment and find_free_sste
From: Ian Munsie @ 2014-10-28 3:25 UTC (permalink / raw)
To: mpe
Cc: cbe-oss-dev, mikey, arnd, Aneesh Kumar K.V, greg, linux-kernel,
linuxppc-dev, anton, imunsie, jk
In-Reply-To: <1414466730-15591-1-git-send-email-imunsie@au.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
This moves the segment table hash calculation from cxl_load_segment into
find_free_sste since that is the only place it is actually used.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
drivers/misc/cxl/fault.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index d0e97fd..cb4f323 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -21,20 +21,30 @@
#include "cxl.h"
-static struct cxl_sste* find_free_sste(struct cxl_sste *primary_group,
- unsigned int *lru)
+/* This finds a free SSTE for the given SLB */
+static struct cxl_sste* find_free_sste(struct cxl_context *ctx,
+ struct copro_slb *slb)
{
+ struct cxl_sste *primary, *sste;
+ unsigned int mask = (ctx->sst_size >> 7) - 1; /* SSTP0[SegTableSize] */
unsigned int entry;
- struct cxl_sste *sste, *group = primary_group;
+ unsigned int hash;
+
+ if (slb->vsid & SLB_VSID_B_1T)
+ hash = (slb->esid >> SID_SHIFT_1T) & mask;
+ else /* 256M */
+ hash = (slb->esid >> SID_SHIFT) & mask;
- for (entry = 0; entry < 8; entry++) {
- sste = group + entry;
+ primary = ctx->sstp + (hash << 3);
+
+ for (entry = 0, sste = primary; entry < 8; entry++, sste++) {
if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
return sste;
}
+
/* Nothing free, select an entry to cast out */
- sste = primary_group + *lru;
- *lru = (*lru + 1) & 0x7;
+ sste = primary + ctx->sst_lru;
+ ctx->sst_lru = (ctx->sst_lru + 1) & 0x7;
return sste;
}
@@ -42,19 +52,11 @@ static struct cxl_sste* find_free_sste(struct cxl_sste *primary_group,
static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
{
/* mask is the group index, we search primary and secondary here. */
- unsigned int mask = (ctx->sst_size >> 7)-1; /* SSTP0[SegTableSize] */
struct cxl_sste *sste;
- unsigned int hash;
unsigned long flags;
-
- if (slb->vsid & SLB_VSID_B_1T)
- hash = (slb->esid >> SID_SHIFT_1T) & mask;
- else /* 256M */
- hash = (slb->esid >> SID_SHIFT) & mask;
-
spin_lock_irqsave(&ctx->sste_lock, flags);
- sste = find_free_sste(ctx->sstp + (hash << 3), &ctx->sst_lru);
+ sste = find_free_sste(ctx, slb);
pr_devel("CXL Populating SST[%li]: %#llx %#llx\n",
sste - ctx->sstp, slb->vsid, slb->esid);
--
2.1.1
^ permalink raw reply related
* [PATCH v2 3/4] powerpc/copro: Use appropriate ESID mask in copro_calculate_slb
From: Ian Munsie @ 2014-10-28 3:25 UTC (permalink / raw)
To: mpe
Cc: cbe-oss-dev, mikey, arnd, Aneesh Kumar K.V, greg, linux-kernel,
linuxppc-dev, anton, imunsie, jk
In-Reply-To: <1414466730-15591-1-git-send-email-imunsie@au.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
This patch makes copro_calculate_slb mask the ESID by the correct mask
for 1T vs 256M segments.
This has no effect by itself as the extra bits were ignored, but it
makes debugging the segment table entries easier and means that we can
directly compare the ESID values for duplicates without needing to worry
about masking in the comparison.
This will be used to simplify a comparison in the following patch.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
arch/powerpc/mm/copro_fault.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
index 0f9939e..5a236f0 100644
--- a/arch/powerpc/mm/copro_fault.c
+++ b/arch/powerpc/mm/copro_fault.c
@@ -99,8 +99,6 @@ int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb)
u64 vsid;
int psize, ssize;
- slb->esid = (ea & ESID_MASK) | SLB_ESID_V;
-
switch (REGION_ID(ea)) {
case USER_REGION_ID:
pr_devel("%s: 0x%llx -- USER_REGION_ID\n", __func__, ea);
@@ -133,6 +131,7 @@ int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb)
vsid |= mmu_psize_defs[psize].sllp |
((ssize == MMU_SEGSIZE_1T) ? SLB_VSID_B_1T : 0);
+ slb->esid = (ea & (ssize == MMU_SEGSIZE_1T ? ESID_MASK_1T : ESID_MASK)) | SLB_ESID_V;
slb->vsid = vsid;
return 0;
--
2.1.1
^ permalink raw reply related
* [PATCH v2] CXL: Fix PSL error due to duplicate segment table entries
From: Ian Munsie @ 2014-10-28 3:25 UTC (permalink / raw)
To: mpe
Cc: cbe-oss-dev, mikey, arnd, Aneesh Kumar K.V, greg, linux-kernel,
linuxppc-dev, anton, imunsie, jk
In certain circumstances the PSL (Power Service Layer, which provides
translation services for CXL hardware) can send an interrupt for a segment miss
that the kernel has already handled. This can happen if multiple translations
for the same segment are queued in the PSL before the kernel has restarted the
first translation.
The CXL driver does not expect this situation and does not check if a segment
had already been handled. This could cause a duplicate segment table entry
which in turn caused a PSL error taking down the card.
This patch series fixes the issue by checking for existing entries in the
segment table that match the segment it is trying to insert to avoid inserting
duplicate entries.
Some of the code has been refactored to simplify it - the segment table
hash has been moved from cxl_load_segment to find_free_sste where it is
used and we have disabled the secondary hash in the segment table to
reduce the number of entries that need to be tested from 16 to 8. Due to
the large segment sizes we use it is extremely unlikely that the
secondary hash would ever have been used in practice, so this should not
have any negative impacts and may even improve performance.
copro_calculate_slb didn't use the correct ESID mask for 1T vs 256M segments,
which was not a problem as the extra bits were ignored. This series fixes it to
use the correct mask to make debugging easier and so that we can directly
compare the ESID values for duplicates without needing to worry about masking
in the comparison.
- Patch 1 disables the secondary hash in the segment table to simplify the code.
- Patch 2 cleans up and refactors cxl_load_segment and find_free_sste to move
the hash calculation to where it is actually used.
- Patch 3 fixes the ESID returned by copro_calculate_slb to be properly masked
based on the segment size.
- Patch 4 prevents duplicate segment table entries from being inserted to fix
PSL errors resulting from this situation.
Changes since v1:
- Split patch out into separate patches for cleanups and bug fix
^ permalink raw reply
* [PATCH v2 4/4] CXL: Fix PSL error due to duplicate segment table entries
From: Ian Munsie @ 2014-10-28 3:25 UTC (permalink / raw)
To: mpe
Cc: cbe-oss-dev, mikey, arnd, Aneesh Kumar K.V, greg, linux-kernel,
linuxppc-dev, anton, imunsie, jk
In-Reply-To: <1414466730-15591-1-git-send-email-imunsie@au.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
In certain circumstances the PSL (Power Service Layer, which provides
translation services for CXL hardware) can send an interrupt for a
segment miss that the kernel has already handled. This can happen if
multiple translations for the same segment are queued in the PSL before
the kernel has restarted the first translation.
The CXL driver does not expect this situation and does not check if a
segment had already been handled. This could cause a duplicate segment
table entry which in turn caused a PSL error taking down the card.
This patch fixes the issue by checking for existing entries in the
segment table that match the segment it is trying to insert to avoid
inserting duplicate entries.
This patch requires "powerpc/copro: Use appropriate ESID mask in
copro_calculate_slb" for the sste_matches function to operate correctly.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
drivers/misc/cxl/fault.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index cb4f323..c99e896 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -21,11 +21,20 @@
#include "cxl.h"
-/* This finds a free SSTE for the given SLB */
+static bool sste_matches(struct cxl_sste *sste, struct copro_slb *slb)
+{
+ return ((sste->vsid_data == cpu_to_be64(slb->vsid)) &&
+ (sste->esid_data == cpu_to_be64(slb->esid)));
+}
+
+/*
+ * This finds a free SSTE for the given SLB, or returns NULL if it's already in
+ * the segment table.
+ */
static struct cxl_sste* find_free_sste(struct cxl_context *ctx,
struct copro_slb *slb)
{
- struct cxl_sste *primary, *sste;
+ struct cxl_sste *primary, *sste, *ret = NULL;
unsigned int mask = (ctx->sst_size >> 7) - 1; /* SSTP0[SegTableSize] */
unsigned int entry;
unsigned int hash;
@@ -38,15 +47,19 @@ static struct cxl_sste* find_free_sste(struct cxl_context *ctx,
primary = ctx->sstp + (hash << 3);
for (entry = 0, sste = primary; entry < 8; entry++, sste++) {
- if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
- return sste;
+ if (!ret && !(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
+ ret = sste;
+ if (sste_matches(sste, slb))
+ return NULL;
}
+ if (ret)
+ return ret;
/* Nothing free, select an entry to cast out */
- sste = primary + ctx->sst_lru;
+ ret = primary + ctx->sst_lru;
ctx->sst_lru = (ctx->sst_lru + 1) & 0x7;
- return sste;
+ return ret;
}
static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
@@ -57,12 +70,15 @@ static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
spin_lock_irqsave(&ctx->sste_lock, flags);
sste = find_free_sste(ctx, slb);
+ if (!sste)
+ goto out_unlock;
pr_devel("CXL Populating SST[%li]: %#llx %#llx\n",
sste - ctx->sstp, slb->vsid, slb->esid);
sste->vsid_data = cpu_to_be64(slb->vsid);
sste->esid_data = cpu_to_be64(slb->esid);
+out_unlock:
spin_unlock_irqrestore(&ctx->sste_lock, flags);
}
--
2.1.1
^ permalink raw reply related
* [PATCH v2 1/4] CXL: Disable secondary hash in segment table
From: Ian Munsie @ 2014-10-28 3:25 UTC (permalink / raw)
To: mpe
Cc: cbe-oss-dev, mikey, arnd, Aneesh Kumar K.V, greg, linux-kernel,
linuxppc-dev, anton, imunsie, jk
In-Reply-To: <1414466730-15591-1-git-send-email-imunsie@au.ibm.com>
From: Ian Munsie <imunsie@au1.ibm.com>
This patch simplifies the process of finding a free segment table entry
by disabling the secondary hash. This reduces the number of possible
entries in the segment table for a given address from 16 to 8.
Due to the large segment sizes we use it is extremely unlikely that the
secondary hash would ever have been used in practice, so this should not
have any negative impacts and may even improve performance due to the
reduced number of comparisons that software & hardware need to perform.
This patch clears the SC bit in the hardware's state register
(CXL_PSL_SR_An) to disable the secondary hash in the hardware since we
can no longer fill out entries using it.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
drivers/misc/cxl/fault.c | 30 ++++++++----------------------
drivers/misc/cxl/native.c | 4 ++--
2 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index 69506eb..d0e97fd 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -22,29 +22,19 @@
#include "cxl.h"
static struct cxl_sste* find_free_sste(struct cxl_sste *primary_group,
- bool sec_hash,
- struct cxl_sste *secondary_group,
unsigned int *lru)
{
- unsigned int i, entry;
+ unsigned int entry;
struct cxl_sste *sste, *group = primary_group;
- for (i = 0; i < 2; i++) {
- for (entry = 0; entry < 8; entry++) {
- sste = group + entry;
- if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
- return sste;
- }
- if (!sec_hash)
- break;
- group = secondary_group;
+ for (entry = 0; entry < 8; entry++) {
+ sste = group + entry;
+ if (!(be64_to_cpu(sste->esid_data) & SLB_ESID_V))
+ return sste;
}
/* Nothing free, select an entry to cast out */
- if (sec_hash && (*lru & 0x8))
- sste = secondary_group + (*lru & 0x7);
- else
- sste = primary_group + (*lru & 0x7);
- *lru = (*lru + 1) & 0xf;
+ sste = primary_group + *lru;
+ *lru = (*lru + 1) & 0x7;
return sste;
}
@@ -53,22 +43,18 @@ static void cxl_load_segment(struct cxl_context *ctx, struct copro_slb *slb)
{
/* mask is the group index, we search primary and secondary here. */
unsigned int mask = (ctx->sst_size >> 7)-1; /* SSTP0[SegTableSize] */
- bool sec_hash = 1;
struct cxl_sste *sste;
unsigned int hash;
unsigned long flags;
- sec_hash = !!(cxl_p1n_read(ctx->afu, CXL_PSL_SR_An) & CXL_PSL_SR_An_SC);
-
if (slb->vsid & SLB_VSID_B_1T)
hash = (slb->esid >> SID_SHIFT_1T) & mask;
else /* 256M */
hash = (slb->esid >> SID_SHIFT) & mask;
spin_lock_irqsave(&ctx->sste_lock, flags);
- sste = find_free_sste(ctx->sstp + (hash << 3), sec_hash,
- ctx->sstp + ((~hash & mask) << 3), &ctx->sst_lru);
+ sste = find_free_sste(ctx->sstp + (hash << 3), &ctx->sst_lru);
pr_devel("CXL Populating SST[%li]: %#llx %#llx\n",
sste - ctx->sstp, slb->vsid, slb->esid);
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index 623286a..d47532e 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -417,7 +417,7 @@ static int attach_afu_directed(struct cxl_context *ctx, u64 wed, u64 amr)
ctx->elem->haurp = 0; /* disable */
ctx->elem->sdr = cpu_to_be64(mfspr(SPRN_SDR1));
- sr = CXL_PSL_SR_An_SC;
+ sr = 0;
if (ctx->master)
sr |= CXL_PSL_SR_An_MP;
if (mfspr(SPRN_LPCR) & LPCR_TC)
@@ -508,7 +508,7 @@ static int attach_dedicated(struct cxl_context *ctx, u64 wed, u64 amr)
u64 sr;
int rc;
- sr = CXL_PSL_SR_An_SC;
+ sr = 0;
set_endian(sr);
if (ctx->master)
sr |= CXL_PSL_SR_An_MP;
--
2.1.1
^ permalink raw reply related
* Re: powerpc: Replace __get_cpu_var uses
From: Christoph Lameter @ 2014-10-28 2:31 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <1414447475.1913.1.camel@concordia>
On Tue, 28 Oct 2014, Michael Ellerman wrote:
> I'm happy to put it in a topic branch for 3.19, or move the definition or
> whatever, your choice Christoph.
Get the patch merged please.
^ permalink raw reply
* Re: Warnings reported while building the vanilla kernel on powerpc
From: Michael Ellerman @ 2014-10-28 1:35 UTC (permalink / raw)
To: Preeti U Murthy; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <544E4996.3050403@linux.vnet.ibm.com>
On Mon, 2014-10-27 at 19:03 +0530, Preeti U Murthy wrote:
> Hello,
>
> I noticed the following warnings while building the upstream kernel as of yesterday.
> The commit I checked out at was cac7f2429872d3 : Linux 3.18-rc2.
>
> Please verify if any of these can be fixed.
Patches welcome.
cheers
^ 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