* Re: [PATCH RFC] mm: remove CONFIG_HAVE_MEMBLOCK_NODE_MAP (was: Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA)
From: Baoquan He @ 2020-04-01 5:42 UTC (permalink / raw)
To: Mike Rapoport
Cc: mmorana, Catalin Marinas, Heiko Carstens, Michal Hocko,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86,
Christian Borntraeger, Ingo Molnar, Hoan Tran, Pavel Tatashin,
lho, Vasily Gorbik, Vlastimil Babka, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200331215618.GG30942@linux.ibm.com>
On 04/01/20 at 12:56am, Mike Rapoport wrote:
> On Mon, Mar 30, 2020 at 11:58:43AM +0200, Michal Hocko wrote:
> >
> > What would it take to make ia64 use HAVE_MEMBLOCK_NODE_MAP? I would
> > really love to see that thing go away. It is causing problems when
> > people try to use memblock api.
>
> Well, it's a small patch in the end :)
>
> Currently all NUMA architectures currently enable
> CONFIG_HAVE_MEMBLOCK_NODE_MAP and use free_area_init_nodes() to initialize
> nodes and zones structures.
I did some investigation, there are nine ARCHes having NUMA config. And
among them, alpha doesn't have HAVE_MEMBLOCK_NODE_MAP support. While the
interesting thing is there are two ARCHes which have
HAVE_MEMBLOCK_NODE_MAP config, but don't have NUMA config adding, they
are microblaze and riscv. Obviously it was not carefully considered to
add HAVE_MEMBLOCK_NODE_MAP config into riscv and microblaze.
arch/alpha/Kconfig:config NUMA
arch/arm64/Kconfig:config NUMA
arch/ia64/Kconfig:config NUMA
arch/mips/Kconfig:config NUMA
arch/powerpc/Kconfig:config NUMA
arch/s390/Kconfig:config NUMA
arch/sh/mm/Kconfig:config NUMA
arch/sparc/Kconfig:config NUMA
arch/x86/Kconfig:config NUMA
From above information, we can remove HAVE_MEMBLOCK_NODE_MAP, and
replace it with CONFIG_NUMA. That sounds more sensible to store nid into
memblock when NUMA support is enabled.
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 079d17d96410..9de81112447e 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -50,9 +50,7 @@ struct memblock_region {
> phys_addr_t base;
> phys_addr_t size;
> enum memblock_flags flags;
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> int nid;
> -#endif
I didn't look into other change very carefully, but feel enabling
memblock node map for all ARCHes looks a little radical. After all, many
ARCHes even don't have NUMA support.
> };
>
> /**
> @@ -215,7 +213,6 @@ static inline bool memblock_is_nomap(struct memblock_region *m)
> return m->flags & MEMBLOCK_NOMAP;
> }
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> int memblock_search_pfn_nid(unsigned long pfn, unsigned long *start_pfn,
> unsigned long *end_pfn);
> void __next_mem_pfn_range(int *idx, int nid, unsigned long *out_start_pfn,
> @@ -234,7 +231,6 @@ void __next_mem_pfn_range(int *idx, int nid, unsigned long *out_start_pfn,
> #define for_each_mem_pfn_range(i, nid, p_start, p_end, p_nid) \
> for (i = -1, __next_mem_pfn_range(&i, nid, p_start, p_end, p_nid); \
> i >= 0; __next_mem_pfn_range(&i, nid, p_start, p_end, p_nid))
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
>
> #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
> @@ -310,7 +306,6 @@ void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
> for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved, \
> nid, flags, p_start, p_end, p_nid)
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> int memblock_set_node(phys_addr_t base, phys_addr_t size,
> struct memblock_type *type, int nid);
>
> @@ -323,16 +318,6 @@ static inline int memblock_get_region_node(const struct memblock_region *r)
> {
> return r->nid;
> }
> -#else
> -static inline void memblock_set_region_node(struct memblock_region *r, int nid)
> -{
> -}
> -
> -static inline int memblock_get_region_node(const struct memblock_region *r)
> -{
> - return 0;
> -}
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
>
> /* Flags for memblock allocation APIs */
> #define MEMBLOCK_ALLOC_ANYWHERE (~(phys_addr_t)0)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c54fb96cb1e6..368a45d4696a 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2125,9 +2125,8 @@ static inline unsigned long get_num_physpages(void)
> return phys_pages;
> }
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> /*
> - * With CONFIG_HAVE_MEMBLOCK_NODE_MAP set, an architecture may initialise its
> + * Using memblock node mappings, an architecture may initialise its
> * zones, allocate the backing mem_map and account for memory holes in a more
> * architecture independent manner. This is a substitute for creating the
> * zone_sizes[] and zholes_size[] arrays and passing them to
> @@ -2148,9 +2147,6 @@ static inline unsigned long get_num_physpages(void)
> * registered physical page range. Similarly
> * sparse_memory_present_with_active_regions() calls memory_present() for
> * each range when SPARSEMEM is enabled.
> - *
> - * See mm/page_alloc.c for more information on each function exposed by
> - * CONFIG_HAVE_MEMBLOCK_NODE_MAP.
> */
> extern void free_area_init_nodes(unsigned long *max_zone_pfn);
> unsigned long node_map_pfn_alignment(void);
> @@ -2165,22 +2161,12 @@ extern void free_bootmem_with_active_regions(int nid,
> unsigned long max_low_pfn);
> extern void sparse_memory_present_with_active_regions(int nid);
>
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
>
> -#if !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && \
> - !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
> -static inline int __early_pfn_to_nid(unsigned long pfn,
> - struct mminit_pfnnid_cache *state)
> -{
> - return 0;
> -}
> -#else
> /* please see mm/page_alloc.c */
> extern int __meminit early_pfn_to_nid(unsigned long pfn);
> /* there is a per-arch backend function. */
> extern int __meminit __early_pfn_to_nid(unsigned long pfn,
> struct mminit_pfnnid_cache *state);
> -#endif
>
> extern void set_dma_reserve(unsigned long new_dma_reserve);
> extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 462f6873905a..4422d1961d0e 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -917,11 +917,7 @@ extern int movable_zone;
> #ifdef CONFIG_HIGHMEM
> static inline int zone_movable_is_highmem(void)
> {
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> return movable_zone == ZONE_HIGHMEM;
> -#else
> - return (ZONE_MOVABLE - 1) == ZONE_HIGHMEM;
> -#endif
> }
> #endif
>
> @@ -1121,15 +1117,6 @@ static inline struct zoneref *first_zones_zonelist(struct zonelist *zonelist,
> #include <asm/sparsemem.h>
> #endif
>
> -#if !defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID) && \
> - !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP)
> -static inline unsigned long early_pfn_to_nid(unsigned long pfn)
> -{
> - BUILD_BUG_ON(IS_ENABLED(CONFIG_NUMA));
> - return 0;
> -}
> -#endif
> -
> #ifdef CONFIG_FLATMEM
> #define pfn_to_nid(pfn) (0)
> #endif
> diff --git a/mm/memblock.c b/mm/memblock.c
> index eba94ee3de0b..819441133a21 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -620,9 +620,7 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
> * area, insert that portion.
> */
> if (rbase > base) {
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> WARN_ON(nid != memblock_get_region_node(rgn));
> -#endif
> WARN_ON(flags != rgn->flags);
> nr_new++;
> if (insert)
> @@ -1197,7 +1195,6 @@ void __init_memblock __next_mem_range_rev(u64 *idx, int nid,
> *idx = ULLONG_MAX;
> }
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> /*
> * Common iterator interface used to define for_each_mem_pfn_range().
> */
> @@ -1258,7 +1255,7 @@ int __init_memblock memblock_set_node(phys_addr_t base, phys_addr_t size,
> memblock_merge_regions(type);
> return 0;
> }
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> +
> #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> /**
> * __next_mem_pfn_range_in_zone - iterator for for_each_*_range_in_zone()
> @@ -1797,7 +1794,6 @@ bool __init_memblock memblock_is_map_memory(phys_addr_t addr)
> return !memblock_is_nomap(&memblock.memory.regions[i]);
> }
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> int __init_memblock memblock_search_pfn_nid(unsigned long pfn,
> unsigned long *start_pfn, unsigned long *end_pfn)
> {
> @@ -1812,7 +1808,6 @@ int __init_memblock memblock_search_pfn_nid(unsigned long pfn,
>
> return type->regions[mid].nid;
> }
> -#endif
>
> /**
> * memblock_is_region_memory - check if a region is a subset of memory
> @@ -1903,11 +1898,9 @@ static void __init_memblock memblock_dump(struct memblock_type *type)
> size = rgn->size;
> end = base + size - 1;
> flags = rgn->flags;
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> if (memblock_get_region_node(rgn) != MAX_NUMNODES)
> snprintf(nid_buf, sizeof(nid_buf), " on node %d",
> memblock_get_region_node(rgn));
> -#endif
> pr_info(" %s[%#x]\t[%pa-%pa], %pa bytes%s flags: %#x\n",
> type->name, idx, &base, &end, &size, nid_buf, flags);
> }
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 19389cdc16a5..dc8828b087bf 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1366,11 +1366,7 @@ check_pages_isolated_cb(unsigned long start_pfn, unsigned long nr_pages,
>
> static int __init cmdline_parse_movable_node(char *p)
> {
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> movable_node_enabled = true;
> -#else
> - pr_warn("movable_node parameter depends on CONFIG_HAVE_MEMBLOCK_NODE_MAP to work properly\n");
> -#endif
> return 0;
> }
> early_param("movable_node", cmdline_parse_movable_node);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3c4eb750a199..84a69d6e7e61 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -335,7 +335,6 @@ static unsigned long nr_kernel_pages __initdata;
> static unsigned long nr_all_pages __initdata;
> static unsigned long dma_reserve __initdata;
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> static unsigned long arch_zone_lowest_possible_pfn[MAX_NR_ZONES] __initdata;
> static unsigned long arch_zone_highest_possible_pfn[MAX_NR_ZONES] __initdata;
> static unsigned long required_kernelcore __initdata;
> @@ -348,7 +347,6 @@ static bool mirrored_kernelcore __meminitdata;
> /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */
> int movable_zone;
> EXPORT_SYMBOL(movable_zone);
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
>
> #if MAX_NUMNODES > 1
> unsigned int nr_node_ids __read_mostly = MAX_NUMNODES;
> @@ -1447,9 +1445,6 @@ void __free_pages_core(struct page *page, unsigned int order)
> __free_pages(page, order);
> }
>
> -#if defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID) || \
> - defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP)
> -
> static struct mminit_pfnnid_cache early_pfnnid_cache __meminitdata;
>
> int __meminit early_pfn_to_nid(unsigned long pfn)
> @@ -1465,7 +1460,6 @@ int __meminit early_pfn_to_nid(unsigned long pfn)
>
> return nid;
> }
> -#endif
>
> #ifdef CONFIG_NODES_SPAN_OTHER_NODES
> /* Only safe to use early in boot when initialisation is single-threaded */
> @@ -5828,7 +5822,6 @@ void __ref build_all_zonelists(pg_data_t *pgdat)
> static bool __meminit
> overlap_memmap_init(unsigned long zone, unsigned long *pfn)
> {
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> static struct memblock_region *r;
>
> if (mirrored_kernelcore && zone == ZONE_MOVABLE) {
> @@ -5844,7 +5837,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
> return true;
> }
> }
> -#endif
> +
> return false;
> }
>
> @@ -6227,7 +6220,6 @@ void __meminit init_currently_empty_zone(struct zone *zone,
> zone->initialized = 1;
> }
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> #ifndef CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID
>
> /*
> @@ -6503,8 +6495,7 @@ static unsigned long __init zone_absent_pages_in_node(int nid,
> return nr_absent;
> }
>
> -#else /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> -static inline unsigned long __init zone_spanned_pages_in_node(int nid,
> +static inline unsigned long __init compat_zone_spanned_pages_in_node(int nid,
> unsigned long zone_type,
> unsigned long node_start_pfn,
> unsigned long node_end_pfn,
> @@ -6523,7 +6514,7 @@ static inline unsigned long __init zone_spanned_pages_in_node(int nid,
> return zones_size[zone_type];
> }
>
> -static inline unsigned long __init zone_absent_pages_in_node(int nid,
> +static inline unsigned long __init compat_zone_absent_pages_in_node(int nid,
> unsigned long zone_type,
> unsigned long node_start_pfn,
> unsigned long node_end_pfn,
> @@ -6535,13 +6526,12 @@ static inline unsigned long __init zone_absent_pages_in_node(int nid,
> return zholes_size[zone_type];
> }
>
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> -
> static void __init calculate_node_totalpages(struct pglist_data *pgdat,
> unsigned long node_start_pfn,
> unsigned long node_end_pfn,
> unsigned long *zones_size,
> - unsigned long *zholes_size)
> + unsigned long *zholes_size,
> + bool compat)
> {
> unsigned long realtotalpages = 0, totalpages = 0;
> enum zone_type i;
> @@ -6549,17 +6539,38 @@ static void __init calculate_node_totalpages(struct pglist_data *pgdat,
> for (i = 0; i < MAX_NR_ZONES; i++) {
> struct zone *zone = pgdat->node_zones + i;
> unsigned long zone_start_pfn, zone_end_pfn;
> + unsigned long spanned, absent;
> unsigned long size, real_size;
>
> - size = zone_spanned_pages_in_node(pgdat->node_id, i,
> - node_start_pfn,
> - node_end_pfn,
> - &zone_start_pfn,
> - &zone_end_pfn,
> - zones_size);
> - real_size = size - zone_absent_pages_in_node(pgdat->node_id, i,
> - node_start_pfn, node_end_pfn,
> - zholes_size);
> + if (compat) {
> + spanned = compat_zone_spanned_pages_in_node(
> + pgdat->node_id, i,
> + node_start_pfn,
> + node_end_pfn,
> + &zone_start_pfn,
> + &zone_end_pfn,
> + zones_size);
> + absent = compat_zone_absent_pages_in_node(
> + pgdat->node_id, i,
> + node_start_pfn,
> + node_end_pfn,
> + zholes_size);
> + } else {
> + spanned = zone_spanned_pages_in_node(pgdat->node_id, i,
> + node_start_pfn,
> + node_end_pfn,
> + &zone_start_pfn,
> + &zone_end_pfn,
> + zones_size);
> + absent = zone_absent_pages_in_node(pgdat->node_id, i,
> + node_start_pfn,
> + node_end_pfn,
> + zholes_size);
> + }
> +
> + size = spanned;
> + real_size = size - absent;
> +
> if (size)
> zone->zone_start_pfn = zone_start_pfn;
> else
> @@ -6859,10 +6870,8 @@ static void __ref alloc_node_mem_map(struct pglist_data *pgdat)
> */
> if (pgdat == NODE_DATA(0)) {
> mem_map = NODE_DATA(0)->node_mem_map;
> -#if defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) || defined(CONFIG_FLATMEM)
> if (page_to_pfn(mem_map) != pgdat->node_start_pfn)
> mem_map -= offset;
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> }
> #endif
> }
> @@ -6879,9 +6888,10 @@ static inline void pgdat_set_deferred_range(pg_data_t *pgdat)
> static inline void pgdat_set_deferred_range(pg_data_t *pgdat) {}
> #endif
>
> -void __init free_area_init_node(int nid, unsigned long *zones_size,
> - unsigned long node_start_pfn,
> - unsigned long *zholes_size)
> +void __init __free_area_init_node(int nid, unsigned long *zones_size,
> + unsigned long node_start_pfn,
> + unsigned long *zholes_size,
> + bool compat)
> {
> pg_data_t *pgdat = NODE_DATA(nid);
> unsigned long start_pfn = 0;
> @@ -6893,16 +6903,17 @@ void __init free_area_init_node(int nid, unsigned long *zones_size,
> pgdat->node_id = nid;
> pgdat->node_start_pfn = node_start_pfn;
> pgdat->per_cpu_nodestats = NULL;
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> - get_pfn_range_for_nid(nid, &start_pfn, &end_pfn);
> - pr_info("Initmem setup node %d [mem %#018Lx-%#018Lx]\n", nid,
> - (u64)start_pfn << PAGE_SHIFT,
> - end_pfn ? ((u64)end_pfn << PAGE_SHIFT) - 1 : 0);
> -#else
> - start_pfn = node_start_pfn;
> -#endif
> + if (!compat) {
> + get_pfn_range_for_nid(nid, &start_pfn, &end_pfn);
> + pr_info("Initmem setup node %d [mem %#018Lx-%#018Lx]\n", nid,
> + (u64)start_pfn << PAGE_SHIFT,
> + end_pfn ? ((u64)end_pfn << PAGE_SHIFT) - 1 : 0);
> + } else {
> + start_pfn = node_start_pfn;
> + }
> +
> calculate_node_totalpages(pgdat, start_pfn, end_pfn,
> - zones_size, zholes_size);
> + zones_size, zholes_size, compat);
>
> alloc_node_mem_map(pgdat);
> pgdat_set_deferred_range(pgdat);
> @@ -6910,6 +6921,14 @@ void __init free_area_init_node(int nid, unsigned long *zones_size,
> free_area_init_core(pgdat);
> }
>
> +void __init free_area_init_node(int nid, unsigned long *zones_size,
> + unsigned long node_start_pfn,
> + unsigned long *zholes_size)
> +{
> + __free_area_init_node(nid, zones_size, node_start_pfn, zholes_size,
> + true);
> +}
> +
> #if !defined(CONFIG_FLAT_NODE_MEM_MAP)
> /*
> * Initialize all valid struct pages in the range [spfn, epfn) and mark them
> @@ -6993,8 +7012,6 @@ static inline void __init init_unavailable_mem(void)
> }
> #endif /* !CONFIG_FLAT_NODE_MEM_MAP */
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> -
> #if MAX_NUMNODES > 1
> /*
> * Figure out the number of possible node ids.
> @@ -7423,8 +7440,8 @@ void __init free_area_init_nodes(unsigned long *max_zone_pfn)
> init_unavailable_mem();
> for_each_online_node(nid) {
> pg_data_t *pgdat = NODE_DATA(nid);
> - free_area_init_node(nid, NULL,
> - find_min_pfn_for_node(nid), NULL);
> + __free_area_init_node(nid, NULL,
> + find_min_pfn_for_node(nid), NULL, false);
>
> /* Any memory on that node */
> if (pgdat->node_present_pages)
> @@ -7489,8 +7506,6 @@ static int __init cmdline_parse_movablecore(char *p)
> early_param("kernelcore", cmdline_parse_kernelcore);
> early_param("movablecore", cmdline_parse_movablecore);
>
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> -
> void adjust_managed_page_count(struct page *page, long count)
> {
> atomic_long_add(count, &page_zone(page)->managed_pages);
> --
> 2.25.1
>
>
^ permalink raw reply
* Re: [PATCH v7 7/7] powerpc/32: use set_memory_attr()
From: Christophe Leroy @ 2020-04-01 5:50 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev; +Cc: kernel-hardening, ajd, npiggin, dja
In-Reply-To: <6b003f8d254d1614cec838e1c032c3005d52d44d.camel@russell.cc>
Le 01/04/2020 à 04:27, Russell Currey a écrit :
> On Tue, 2020-03-31 at 11:56 +0200, Christophe Leroy wrote:
>>
>> Le 31/03/2020 à 06:48, Russell Currey a écrit :
>>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>>>
>>> Use set_memory_attr() instead of the PPC32 specific
>>> change_page_attr()
>>>
>>> change_page_attr() was checking that the address was not mapped by
>>> blocks and was handling highmem, but that's unneeded because the
>>> affected pages can't be in highmem and block mapping verification
>>> is already done by the callers.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> ---
>>> arch/powerpc/mm/pgtable_32.c | 95 ++++---------------------------
>>> -----
>>> 1 file changed, 10 insertions(+), 85 deletions(-)
>>>
>>> diff --git a/arch/powerpc/mm/pgtable_32.c
>>> b/arch/powerpc/mm/pgtable_32.c
>>> index 5fb90edd865e..3d92eaf3ee2f 100644
>>> --- a/arch/powerpc/mm/pgtable_32.c
>>> +++ b/arch/powerpc/mm/pgtable_32.c
>>> @@ -23,6 +23,7 @@
>>> #include <linux/highmem.h>
>>> #include <linux/memblock.h>
>>> #include <linux/slab.h>
>>> +#include <linux/set_memory.h>
>>>
>>> #include <asm/pgtable.h>
>>> #include <asm/pgalloc.h>
>>> @@ -121,99 +122,20 @@ void __init mapin_ram(void)
>>> }
>>> }
>>>
>>> -/* Scan the real Linux page tables and return a PTE pointer for
>>> - * a virtual address in a context.
>>> - * Returns true (1) if PTE was found, zero otherwise. The pointer
>>> to
>>> - * the PTE pointer is unmodified if PTE is not found.
>>> - */
>>> -static int
>>> -get_pteptr(struct mm_struct *mm, unsigned long addr, pte_t **ptep,
>>> pmd_t **pmdp)
>>
>> This will conflict, get_pteptr() is gone now, see
>> https://github.com/linuxppc/linux/commit/2efc7c085f05870eda6f29ac71eeb83f3bd54415
>>
>> Christophe
>
> OK cool, so I can just drop that hunk? Will try that and make sure it
> rebases on powerpc/next
Well, you still have to remove __change_page_attr_noflush() and
change_page_attr(), everything else in this patch should still be the same.
Christophe
>
> - Russell
>
>>
>>
>>> -{
>>> - pgd_t *pgd;
>>> - pud_t *pud;
>>> - pmd_t *pmd;
>>> - pte_t *pte;
>>> - int retval = 0;
>>> -
>>> - pgd = pgd_offset(mm, addr & PAGE_MASK);
>>> - if (pgd) {
>>> - pud = pud_offset(pgd, addr & PAGE_MASK);
>>> - if (pud && pud_present(*pud)) {
>>> - pmd = pmd_offset(pud, addr & PAGE_MASK);
>>> - if (pmd_present(*pmd)) {
>>> - pte = pte_offset_map(pmd, addr &
>>> PAGE_MASK);
>>> - if (pte) {
>>> - retval = 1;
>>> - *ptep = pte;
>>> - if (pmdp)
>>> - *pmdp = pmd;
>>> - /* XXX caller needs to do
>>> pte_unmap, yuck */
>>> - }
>>> - }
>>> - }
>>> - }
>>> - return(retval);
>>> -}
>>> -
>>> -static int __change_page_attr_noflush(struct page *page, pgprot_t
>>> prot)
>>> -{
>>> - pte_t *kpte;
>>> - pmd_t *kpmd;
>>> - unsigned long address;
>>> -
>>> - BUG_ON(PageHighMem(page));
>>> - address = (unsigned long)page_address(page);
>>> -
>>> - if (v_block_mapped(address))
>>> - return 0;
>>> - if (!get_pteptr(&init_mm, address, &kpte, &kpmd))
>>> - return -EINVAL;
>>> - __set_pte_at(&init_mm, address, kpte, mk_pte(page, prot), 0);
>>> - pte_unmap(kpte);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -/*
>>> - * Change the page attributes of an page in the linear mapping.
>>> - *
>>> - * THIS DOES NOTHING WITH BAT MAPPINGS, DEBUG USE ONLY
>>> - */
>>> -static int change_page_attr(struct page *page, int numpages,
>>> pgprot_t prot)
>>> -{
>>> - int i, err = 0;
>>> - unsigned long flags;
>>> - struct page *start = page;
>>> -
>>> - local_irq_save(flags);
>>> - for (i = 0; i < numpages; i++, page++) {
>>> - err = __change_page_attr_noflush(page, prot);
>>> - if (err)
>>> - break;
>>> - }
>>> - wmb();
>>> - local_irq_restore(flags);
>>> - flush_tlb_kernel_range((unsigned long)page_address(start),
>>> - (unsigned long)page_address(page));
>>> - return err;
>>> -}
>>> -
>>> void mark_initmem_nx(void)
>>> {
>>> - struct page *page = virt_to_page(_sinittext);
>>> unsigned long numpages = PFN_UP((unsigned long)_einittext) -
>>> PFN_DOWN((unsigned long)_sinittext);
>>>
>>> if (v_block_mapped((unsigned long)_stext + 1))
>>> mmu_mark_initmem_nx();
>>> else
>>> - change_page_attr(page, numpages, PAGE_KERNEL);
>>> + set_memory_attr((unsigned long)_sinittext, numpages,
>>> PAGE_KERNEL);
>>> }
>>>
>>> #ifdef CONFIG_STRICT_KERNEL_RWX
>>> void mark_rodata_ro(void)
>>> {
>>> - struct page *page;
>>> unsigned long numpages;
>>>
>>> if (v_block_mapped((unsigned long)_sinittext)) {
>>> @@ -222,20 +144,18 @@ void mark_rodata_ro(void)
>>> return;
>>> }
>>>
>>> - page = virt_to_page(_stext);
>>> numpages = PFN_UP((unsigned long)_etext) -
>>> PFN_DOWN((unsigned long)_stext);
>>>
>>> - change_page_attr(page, numpages, PAGE_KERNEL_ROX);
>>> + set_memory_attr((unsigned long)_stext, numpages,
>>> PAGE_KERNEL_ROX);
>>> /*
>>> * mark .rodata as read only. Use __init_begin rather than
>>> __end_rodata
>>> * to cover NOTES and EXCEPTION_TABLE.
>>> */
>>> - page = virt_to_page(__start_rodata);
>>> numpages = PFN_UP((unsigned long)__init_begin) -
>>> PFN_DOWN((unsigned long)__start_rodata);
>>>
>>> - change_page_attr(page, numpages, PAGE_KERNEL_RO);
>>> + set_memory_attr((unsigned long)__start_rodata, numpages,
>>> PAGE_KERNEL_RO);
>>>
>>> // mark_initmem_nx() should have already run by now
>>> ptdump_check_wx();
>>> @@ -245,9 +165,14 @@ void mark_rodata_ro(void)
>>> #ifdef CONFIG_DEBUG_PAGEALLOC
>>> void __kernel_map_pages(struct page *page, int numpages, int
>>> enable)
>>> {
>>> + unsigned long addr = (unsigned long)page_address(page);
>>> +
>>> if (PageHighMem(page))
>>> return;
>>>
>>> - change_page_attr(page, numpages, enable ? PAGE_KERNEL :
>>> __pgprot(0));
>>> + if (enable)
>>> + set_memory_attr(addr, numpages, PAGE_KERNEL);
>>> + else
>>> + set_memory_attr(addr, numpages, __pgprot(0));
>>> }
>>> #endif /* CONFIG_DEBUG_PAGEALLOC */
>>>
^ permalink raw reply
* Re: [PATCH v2 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms
From: Michal Simek @ 2020-04-01 6:01 UTC (permalink / raw)
To: Michael Ellerman, Michal Simek, linux-kernel, monstr, git, sfr,
maz
Cc: Kate Stewart, Mark Rutland, Desnes A. Nunes do Rosario,
Geert Uytterhoeven, linux-doc, alsa-devel, dri-devel,
Jaroslav Kysela, Richard Fontana, Paul Mackerras, Miquel Raynal,
Mauro Carvalho Chehab, Fabio Estevam, Sasha Levin,
Jonathan Corbet, Masahiro Yamada, YueHaibing, Krzysztof Kozlowski,
Allison Randal, linux-arm-kernel, devicetree, Andrew Donnellan,
Arnd Bergmann, Bartlomiej Zolnierkiewicz, Alistair Popple,
linuxppc-dev, Nicholas Piggin, Alexios Zavras, Mark Brown,
linux-fbdev, Jonathan Cameron, Thomas Gleixner, Andy Shevchenko,
Dmitry Vyukov, Wei Hu, Greg Kroah-Hartman, Nick Desaulniers,
Takashi Iwai, Rob Herring, Enrico Weigelt, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <87imikufes.fsf@mpe.ellerman.id.au>
On 01. 04. 20 4:07, Michael Ellerman wrote:
> Michal Simek <michal.simek@xilinx.com> writes:
>> Hi,
>>
>> recently we wanted to update xilinx intc driver and we found that function
>> which we wanted to remove is still wired by ancient Xilinx PowerPC
>> platforms. Here is the thread about it.
>> https://lore.kernel.org/linux-next/48d3232d-0f1d-42ea-3109-f44bbabfa2e8@xilinx.com/
>>
>> I have been talking about it internally and there is no interest in these
>> platforms and it is also orphan for quite a long time. None is really
>> running/testing these platforms regularly that's why I think it makes sense
>> to remove them also with drivers which are specific to this platform.
>>
>> U-Boot support was removed in 2017 without anybody complain about it
>> https://github.com/Xilinx/u-boot-xlnx/commit/98f705c9cefdfdba62c069821bbba10273a0a8ed
>>
>> Based on current ppc/next.
>>
>> If anyone has any objection about it, please let me know.
>
> Thanks for taking the time to find all this code and remove it.
>
> I'm not going to take this series for v5.7, it was posted too close to
> the merge window, and doing so wouldn't give people much time to object,
> especially given people are distracted at the moment.
>
> I'm happy to take it for v5.8, assuming there's no major objections.
Sure. Just to let you know Christophe Leroy included this patch in his
series about ppc405 removal. It should be the same.
If you don't want to take that alsa patch I can send it separately and
this patch can be taken from his series. I don't really mind but please
let me know what way you prefer.
Thanks,
Michal
^ permalink raw reply
* [PATCH v2 00/16] powerpc/watchpoint: Preparation for more than one watchpoint
From: Ravi Bangoria @ 2020-04-01 6:12 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
So far, powerpc Book3S code has been written with an assumption of only
one watchpoint. But future power architecture is introducing second
watchpoint register (DAWR). Even though this patchset does not enable
2nd DAWR, it make the infrastructure ready so that enabling 2nd DAWR
should just be a matter of changing count.
Existing functionality works fine with the patchset. I've tested it with
perf, ptrace(gdb), xmon. All hw-breakpoint selftests are passing as well.
And I've build tested for 8xx and 'AMCC 44x, 46x or 47x'.
Note: kvm or PowerVM guest is not enabled yet.
v1:
https://lore.kernel.org/linuxppc-dev/20200309085806.155823-1-ravi.bangoria@linux.ibm.com
v1->v2:
- No major functionality changes. Changed few minor things as suggested
by Christophe
- Fixed build failure reported by 'kbuild test robot <lkp@intel.com>'
for patch #9.
- Rebased to powerpc/next plus Christophe's ptrace series[1][2]
[1]: https://lore.kernel.org/linuxppc-dev/cover.1582848567.git.christophe.leroy@c-s.fr
[2]: https://lore.kernel.org/linuxppc-dev/5558d8c22ff0ed03cb5392798564dd203bd68501.1584787012.git.christophe.leroy@c-s.fr
Ravi Bangoria (16):
powerpc/watchpoint: Rename current DAWR macros
powerpc/watchpoint: Add SPRN macros for second DAWR
powerpc/watchpoint: Introduce function to get nr watchpoints
dynamically
powerpc/watchpoint/ptrace: Return actual num of available watchpoints
powerpc/watchpoint: Provide DAWR number to set_dawr
powerpc/watchpoint: Provide DAWR number to __set_breakpoint
powerpc/watchpoint: Get watchpoint count dynamically while disabling
them
powerpc/watchpoint: Disable all available watchpoints when
!dawr_force_enable
powerpc/watchpoint: Convert thread_struct->hw_brk to an array
powerpc/watchpoint: Use loop for thread_struct->ptrace_bps
powerpc/watchpoint: Introduce is_ptrace_bp() function
powerpc/watchpoint: Use builtin ALIGN*() macros
powerpc/watchpoint: Prepare handler to handle more than one
watcnhpoint
powerpc/watchpoint: Don't allow concurrent perf and ptrace events
powerpc/watchpoint/xmon: Don't allow breakpoint overwriting
powerpc/watchpoint/xmon: Support 2nd dawr
arch/powerpc/include/asm/cputable.h | 6 +-
arch/powerpc/include/asm/debug.h | 2 +-
arch/powerpc/include/asm/hw_breakpoint.h | 31 +-
arch/powerpc/include/asm/processor.h | 6 +-
arch/powerpc/include/asm/reg.h | 6 +-
arch/powerpc/include/asm/sstep.h | 2 +
arch/powerpc/kernel/dawr.c | 23 +-
arch/powerpc/kernel/hw_breakpoint.c | 636 ++++++++++++++++++----
arch/powerpc/kernel/process.c | 82 +--
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 72 ++-
arch/powerpc/kernel/ptrace/ptrace32.c | 4 +-
arch/powerpc/kernel/signal.c | 9 +-
arch/powerpc/kvm/book3s_hv.c | 12 +-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +-
arch/powerpc/xmon/xmon.c | 99 +++-
kernel/events/hw_breakpoint.c | 16 +
16 files changed, 806 insertions(+), 218 deletions(-)
--
2.21.1
^ permalink raw reply
* [PATCH v2 01/16] powerpc/watchpoint: Rename current DAWR macros
From: Ravi Bangoria @ 2020-04-01 6:12 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Future Power architecture is introducing second DAWR. Use real
register names from ISA for current macros:
s/SPRN_DAWR/SPRN_DAWR0/
s/SPRN_DAWRX/SPRN_DAWRX0/
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/reg.h | 4 ++--
arch/powerpc/kernel/dawr.c | 4 ++--
arch/powerpc/kvm/book3s_hv.c | 12 ++++++------
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +++++++++---------
arch/powerpc/xmon/xmon.c | 2 +-
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index da5cab038e25..156ee89fa9be 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -283,14 +283,14 @@
#define CTRL_CT1 0x40000000 /* thread 1 */
#define CTRL_TE 0x00c00000 /* thread enable */
#define CTRL_RUNLATCH 0x1
-#define SPRN_DAWR 0xB4
+#define SPRN_DAWR0 0xB4
#define SPRN_RPR 0xBA /* Relative Priority Register */
#define SPRN_CIABR 0xBB
#define CIABR_PRIV 0x3
#define CIABR_PRIV_USER 1
#define CIABR_PRIV_SUPER 2
#define CIABR_PRIV_HYPER 3
-#define SPRN_DAWRX 0xBC
+#define SPRN_DAWRX0 0xBC
#define DAWRX_USER __MASK(0)
#define DAWRX_KERNEL __MASK(1)
#define DAWRX_HYP __MASK(2)
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index cc14aa6c4a1b..e91b613bf137 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -39,8 +39,8 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR, dawr);
- mtspr(SPRN_DAWRX, dawrx);
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 33be4d93248a..498c57e1f5c9 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3383,8 +3383,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
int trap;
unsigned long host_hfscr = mfspr(SPRN_HFSCR);
unsigned long host_ciabr = mfspr(SPRN_CIABR);
- unsigned long host_dawr = mfspr(SPRN_DAWR);
- unsigned long host_dawrx = mfspr(SPRN_DAWRX);
+ unsigned long host_dawr = mfspr(SPRN_DAWR0);
+ unsigned long host_dawrx = mfspr(SPRN_DAWRX0);
unsigned long host_psscr = mfspr(SPRN_PSSCR);
unsigned long host_pidr = mfspr(SPRN_PID);
@@ -3413,8 +3413,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
mtspr(SPRN_SPURR, vcpu->arch.spurr);
if (dawr_enabled()) {
- mtspr(SPRN_DAWR, vcpu->arch.dawr);
- mtspr(SPRN_DAWRX, vcpu->arch.dawrx);
+ mtspr(SPRN_DAWR0, vcpu->arch.dawr);
+ mtspr(SPRN_DAWRX0, vcpu->arch.dawrx);
}
mtspr(SPRN_CIABR, vcpu->arch.ciabr);
mtspr(SPRN_IC, vcpu->arch.ic);
@@ -3466,8 +3466,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
(local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
mtspr(SPRN_HFSCR, host_hfscr);
mtspr(SPRN_CIABR, host_ciabr);
- mtspr(SPRN_DAWR, host_dawr);
- mtspr(SPRN_DAWRX, host_dawrx);
+ mtspr(SPRN_DAWR0, host_dawr);
+ mtspr(SPRN_DAWRX0, host_dawrx);
mtspr(SPRN_PID, host_pidr);
/*
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index dbc2fecc37f0..f4b412b7cad8 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -707,8 +707,8 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
BEGIN_FTR_SECTION
mfspr r5, SPRN_CIABR
- mfspr r6, SPRN_DAWR
- mfspr r7, SPRN_DAWRX
+ mfspr r6, SPRN_DAWR0
+ mfspr r7, SPRN_DAWRX0
mfspr r8, SPRN_IAMR
std r5, STACK_SLOT_CIABR(r1)
std r6, STACK_SLOT_DAWR(r1)
@@ -803,8 +803,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
beq 1f
ld r5, VCPU_DAWR(r4)
ld r6, VCPU_DAWRX(r4)
- mtspr SPRN_DAWR, r5
- mtspr SPRN_DAWRX, r6
+ mtspr SPRN_DAWR0, r5
+ mtspr SPRN_DAWRX0, r6
1:
ld r7, VCPU_CIABR(r4)
ld r8, VCPU_TAR(r4)
@@ -1772,8 +1772,8 @@ BEGIN_FTR_SECTION
* If the DAWR doesn't work, it's ok to write these here as
* this value should always be zero
*/
- mtspr SPRN_DAWR, r6
- mtspr SPRN_DAWRX, r7
+ mtspr SPRN_DAWR0, r6
+ mtspr SPRN_DAWRX0, r7
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
BEGIN_FTR_SECTION
ld r5, STACK_SLOT_TID(r1)
@@ -2583,8 +2583,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
mfmsr r6
andi. r6, r6, MSR_DR /* in real mode? */
bne 4f
- mtspr SPRN_DAWR, r4
- mtspr SPRN_DAWRX, r5
+ mtspr SPRN_DAWR0, r4
+ mtspr SPRN_DAWRX0, r5
4: li r3, 0
blr
@@ -3340,7 +3340,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
mtspr SPRN_AMR, r0
mtspr SPRN_IAMR, r0
mtspr SPRN_CIABR, r0
- mtspr SPRN_DAWRX, r0
+ mtspr SPRN_DAWRX0, r0
BEGIN_MMU_FTR_SECTION
b 4f
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index ea303b7e4e29..75ca41c04dd1 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1939,7 +1939,7 @@ static void dump_207_sprs(void)
printf("hfscr = %.16lx dhdes = %.16lx rpr = %.16lx\n",
mfspr(SPRN_HFSCR), mfspr(SPRN_DHDES), mfspr(SPRN_RPR));
printf("dawr = %.16lx dawrx = %.16lx ciabr = %.16lx\n",
- mfspr(SPRN_DAWR), mfspr(SPRN_DAWRX), mfspr(SPRN_CIABR));
+ mfspr(SPRN_DAWR0), mfspr(SPRN_DAWRX0), mfspr(SPRN_CIABR));
#endif
}
--
2.21.1
^ permalink raw reply related
* [PATCH v2 02/16] powerpc/watchpoint: Add SPRN macros for second DAWR
From: Ravi Bangoria @ 2020-04-01 6:12 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Future Power architecture is introducing second DAWR. Add SPRN_ macros
for the same.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/reg.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 156ee89fa9be..062e74cf41fd 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -284,6 +284,7 @@
#define CTRL_TE 0x00c00000 /* thread enable */
#define CTRL_RUNLATCH 0x1
#define SPRN_DAWR0 0xB4
+#define SPRN_DAWR1 0xB5
#define SPRN_RPR 0xBA /* Relative Priority Register */
#define SPRN_CIABR 0xBB
#define CIABR_PRIV 0x3
@@ -291,6 +292,7 @@
#define CIABR_PRIV_SUPER 2
#define CIABR_PRIV_HYPER 3
#define SPRN_DAWRX0 0xBC
+#define SPRN_DAWRX1 0xBD
#define DAWRX_USER __MASK(0)
#define DAWRX_KERNEL __MASK(1)
#define DAWRX_HYP __MASK(2)
--
2.21.1
^ permalink raw reply related
* [PATCH v2 03/16] powerpc/watchpoint: Introduce function to get nr watchpoints dynamically
From: Ravi Bangoria @ 2020-04-01 6:12 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1.
But future Power architecture is introducing 2nd DAWR and thus kernel
should be able to dynamically find actual number of watchpoints
supported by hw it's running on. Introduce function for the same.
Also convert HBP_NUM macro to HBP_NUM_MAX, which will now represent
maximum number of watchpoints supported by Powerpc.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/cputable.h | 6 +++++-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++++
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 40a4d3c6fd99..c67b94f3334c 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -614,7 +614,11 @@ enum {
};
#endif /* __powerpc64__ */
-#define HBP_NUM 1
+/*
+ * Maximum number of hw breakpoint supported on powerpc. Number of
+ * breakpoints supported by actual hw might be less than this.
+ */
+#define HBP_NUM_MAX 1
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index f2f8d8aa8e3b..518b41eef924 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -43,6 +43,11 @@ struct arch_hw_breakpoint {
#define DABR_MAX_LEN 8
#define DAWR_MAX_LEN 512
+static inline int nr_wp_slots(void)
+{
+ return HBP_NUM_MAX;
+}
+
#ifdef CONFIG_HAVE_HW_BREAKPOINT
#include <linux/kdebug.h>
#include <asm/reg.h>
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index eedcbfb9a6ff..90f6dbc7ff00 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -180,7 +180,7 @@ struct thread_struct {
int fpexc_mode; /* floating-point exception mode */
unsigned int align_ctl; /* alignment handling control */
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- struct perf_event *ptrace_bps[HBP_NUM];
+ struct perf_event *ptrace_bps[HBP_NUM_MAX];
/*
* Helps identify source of single-step exception and subsequent
* hw-breakpoint enablement
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 72f461bd70fb..4120349e2abe 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -38,7 +38,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
int hw_breakpoint_slots(int type)
{
if (type == TYPE_DATA)
- return HBP_NUM;
+ return nr_wp_slots();
return 0; /* no instruction breakpoints available */
}
--
2.21.1
^ permalink raw reply related
* [PATCH v2 04/16] powerpc/watchpoint/ptrace: Return actual num of available watchpoints
From: Ravi Bangoria @ 2020-04-01 6:12 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
User can ask for num of available watchpoints(dbginfo.num_data_bps)
using ptrace(PPC_PTRACE_GETHWDBGINFO). Return actual number of
available watchpoints on the machine rather than hardcoded 1.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index f87e7c5c3bf3..12962302d6a4 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -44,7 +44,7 @@ void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
dbginfo->version = 1;
dbginfo->num_instruction_bps = 0;
if (ppc_breakpoint_available())
- dbginfo->num_data_bps = 1;
+ dbginfo->num_data_bps = nr_wp_slots();
else
dbginfo->num_data_bps = 0;
dbginfo->num_condition_regs = 0;
--
2.21.1
^ permalink raw reply related
* [PATCH v2 05/16] powerpc/watchpoint: Provide DAWR number to set_dawr
From: Ravi Bangoria @ 2020-04-01 6:12 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Introduce new parameter 'nr' to set_dawr() which indicates which DAWR
should be programed.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hw_breakpoint.h | 4 ++--
arch/powerpc/kernel/dawr.c | 15 ++++++++++-----
arch/powerpc/kernel/process.c | 2 +-
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index 518b41eef924..62549007c87a 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -104,10 +104,10 @@ static inline bool dawr_enabled(void)
{
return dawr_force_enable;
}
-int set_dawr(struct arch_hw_breakpoint *brk);
+int set_dawr(struct arch_hw_breakpoint *brk, int nr);
#else
static inline bool dawr_enabled(void) { return false; }
-static inline int set_dawr(struct arch_hw_breakpoint *brk) { return -1; }
+static inline int set_dawr(struct arch_hw_breakpoint *brk, int nr) { return -1; }
#endif
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index e91b613bf137..311e51ee09f4 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -16,7 +16,7 @@
bool dawr_force_enable;
EXPORT_SYMBOL_GPL(dawr_force_enable);
-int set_dawr(struct arch_hw_breakpoint *brk)
+int set_dawr(struct arch_hw_breakpoint *brk, int nr)
{
unsigned long dawr, dawrx, mrd;
@@ -39,15 +39,20 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR0, dawr);
- mtspr(SPRN_DAWRX0, dawrx);
+ if (nr == 0) {
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
+ } else {
+ mtspr(SPRN_DAWR1, dawr);
+ mtspr(SPRN_DAWRX1, dawrx);
+ }
return 0;
}
static void set_dawr_cb(void *info)
{
- set_dawr(info);
+ set_dawr(info, 0);
}
static ssize_t dawr_write_file_bool(struct file *file,
@@ -60,7 +65,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
/* Send error to user if they hypervisor won't allow us to write DAWR */
if (!dawr_force_enable &&
firmware_has_feature(FW_FEATURE_LPAR) &&
- set_dawr(&null_brk) != H_SUCCESS)
+ set_dawr(&null_brk, 0) != H_SUCCESS)
return -ENODEV;
rc = debugfs_write_file_bool(file, user_buf, count, ppos);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 110db94cdf3c..b548a584e465 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -799,7 +799,7 @@ void __set_breakpoint(struct arch_hw_breakpoint *brk)
if (dawr_enabled())
// Power8 or later
- set_dawr(brk);
+ set_dawr(brk, 0);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
--
2.21.1
^ permalink raw reply related
* [PATCH v2 06/16] powerpc/watchpoint: Provide DAWR number to __set_breakpoint
From: Ravi Bangoria @ 2020-04-01 6:12 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Introduce new parameter 'nr' to __set_breakpoint() which indicates
which DAWR should be programed. Also convert current_brk variable
to an array.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/debug.h | 2 +-
arch/powerpc/include/asm/hw_breakpoint.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 8 ++++----
arch/powerpc/kernel/process.c | 14 +++++++-------
arch/powerpc/kernel/signal.c | 2 +-
arch/powerpc/xmon/xmon.c | 2 +-
6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 7756026b95ca..6228935a8b64 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -45,7 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs) { return 0; }
static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
-void __set_breakpoint(struct arch_hw_breakpoint *brk);
+void __set_breakpoint(struct arch_hw_breakpoint *brk, int nr);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
extern void do_send_trap(struct pt_regs *regs, unsigned long address,
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index 62549007c87a..4e4976c3248b 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -85,7 +85,7 @@ static inline void hw_breakpoint_disable(void)
brk.len = 0;
brk.hw_len = 0;
if (ppc_breakpoint_available())
- __set_breakpoint(&brk);
+ __set_breakpoint(&brk, 0);
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 4120349e2abe..7562f9ceb77e 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -63,7 +63,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
* If so, DABR will be populated in single_step_dabr_instruction().
*/
if (current->thread.last_hit_ubp != bp)
- __set_breakpoint(info);
+ __set_breakpoint(info, 0);
return 0;
}
@@ -221,7 +221,7 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs)
info = counter_arch_bp(tsk->thread.last_hit_ubp);
regs->msr &= ~MSR_SE;
- __set_breakpoint(info);
+ __set_breakpoint(info, 0);
tsk->thread.last_hit_ubp = NULL;
}
@@ -346,7 +346,7 @@ int hw_breakpoint_handler(struct die_args *args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(info, 0);
out:
rcu_read_unlock();
return rc;
@@ -379,7 +379,7 @@ static int single_step_dabr_instruction(struct die_args *args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(info, 0);
current->thread.last_hit_ubp = NULL;
/*
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index b548a584e465..e0275fcd0c55 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -630,7 +630,7 @@ void do_break (struct pt_regs *regs, unsigned long address,
}
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
-static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk);
+static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk[HBP_NUM_MAX]);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
/*
@@ -707,7 +707,7 @@ EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
static void set_breakpoint(struct arch_hw_breakpoint *brk)
{
preempt_disable();
- __set_breakpoint(brk);
+ __set_breakpoint(brk, 0);
preempt_enable();
}
@@ -793,13 +793,13 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
return 0;
}
-void __set_breakpoint(struct arch_hw_breakpoint *brk)
+void __set_breakpoint(struct arch_hw_breakpoint *brk, int nr)
{
- memcpy(this_cpu_ptr(¤t_brk), brk, sizeof(*brk));
+ memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk));
if (dawr_enabled())
// Power8 or later
- set_dawr(brk, 0);
+ set_dawr(brk, nr);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
@@ -1167,8 +1167,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
* schedule DABR
*/
#ifndef CONFIG_HAVE_HW_BREAKPOINT
- if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk), &new->thread.hw_brk)))
- __set_breakpoint(&new->thread.hw_brk);
+ if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[0]), &new->thread.hw_brk)))
+ __set_breakpoint(&new->thread.hw_brk, 0);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index d215f9554553..bbf237f072d4 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -129,7 +129,7 @@ static void do_signal(struct task_struct *tsk)
* triggered inside the kernel.
*/
if (tsk->thread.hw_brk.address && tsk->thread.hw_brk.type)
- __set_breakpoint(&tsk->thread.hw_brk);
+ __set_breakpoint(&tsk->thread.hw_brk, 0);
#endif
/* Re-enable the breakpoints for the signal stack */
thread_change_pc(tsk, tsk->thread.regs);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 75ca41c04dd1..508d353e7f06 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -935,7 +935,7 @@ static void insert_cpu_bpts(void)
brk.address = dabr.address;
brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
brk.len = DABR_MAX_LEN;
- __set_breakpoint(&brk);
+ __set_breakpoint(&brk, 0);
}
if (iabr)
--
2.21.1
^ permalink raw reply related
* [PATCH v2 07/16] powerpc/watchpoint: Get watchpoint count dynamically while disabling them
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Instead of disabling only one watchpoint, get num of available
watchpoints dynamically and disable all of them.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hw_breakpoint.h | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index 4e4976c3248b..fae33c729ba9 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -78,14 +78,13 @@ extern void ptrace_triggered(struct perf_event *bp,
struct perf_sample_data *data, struct pt_regs *regs);
static inline void hw_breakpoint_disable(void)
{
- struct arch_hw_breakpoint brk;
-
- brk.address = 0;
- brk.type = 0;
- brk.len = 0;
- brk.hw_len = 0;
- if (ppc_breakpoint_available())
- __set_breakpoint(&brk, 0);
+ int i;
+ struct arch_hw_breakpoint null_brk = {0};
+
+ if (ppc_breakpoint_available()) {
+ for (i = 0; i < nr_wp_slots(); i++)
+ __set_breakpoint(&null_brk, i);
+ }
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
--
2.21.1
^ permalink raw reply related
* [PATCH v2 08/16] powerpc/watchpoint: Disable all available watchpoints when !dawr_force_enable
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Instead of disabling only first watchpoint, disable all available
watchpoints while clearing dawr_force_enable.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/dawr.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 311e51ee09f4..5c882f07ac7d 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -50,9 +50,13 @@ int set_dawr(struct arch_hw_breakpoint *brk, int nr)
return 0;
}
-static void set_dawr_cb(void *info)
+static void disable_dawrs(void *info)
{
- set_dawr(info, 0);
+ struct arch_hw_breakpoint null_brk = {0};
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++)
+ set_dawr(&null_brk, i);
}
static ssize_t dawr_write_file_bool(struct file *file,
@@ -74,7 +78,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
/* If we are clearing, make sure all CPUs have the DAWR cleared */
if (!dawr_force_enable)
- smp_call_function(set_dawr_cb, &null_brk, 0);
+ smp_call_function(disable_dawrs, NULL, 0);
return rc;
}
--
2.21.1
^ permalink raw reply related
* [PATCH v2 09/16] powerpc/watchpoint: Convert thread_struct->hw_brk to an array
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
So far powerpc hw supported only one watchpoint. But Future Power
architecture is introducing 2nd DAWR. Convert thread_struct->hw_brk
into an array.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/kernel/process.c | 61 ++++++++++++++---------
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 40 +++++++++++----
arch/powerpc/kernel/ptrace/ptrace32.c | 4 +-
arch/powerpc/kernel/signal.c | 9 +++-
5 files changed, 77 insertions(+), 39 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 90f6dbc7ff00..65b03162cd67 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -187,7 +187,7 @@ struct thread_struct {
*/
struct perf_event *last_hit_ubp;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
- struct arch_hw_breakpoint hw_brk; /* info on the hardware breakpoint */
+ struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint info */
unsigned long trap_nr; /* last trap # on this thread */
u8 load_slb; /* Ages out SLB preload cache entries */
u8 load_fp;
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index e0275fcd0c55..f5b4f21e822b 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -704,21 +704,50 @@ void switch_booke_debug_regs(struct debug_reg *new_debug)
EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
#ifndef CONFIG_HAVE_HW_BREAKPOINT
-static void set_breakpoint(struct arch_hw_breakpoint *brk)
+static void set_breakpoint(struct arch_hw_breakpoint *brk, int i)
{
preempt_disable();
- __set_breakpoint(brk, 0);
+ __set_breakpoint(brk, i);
preempt_enable();
}
static void set_debug_reg_defaults(struct thread_struct *thread)
{
- thread->hw_brk.address = 0;
- thread->hw_brk.type = 0;
- thread->hw_brk.len = 0;
- thread->hw_brk.hw_len = 0;
- if (ppc_breakpoint_available())
- set_breakpoint(&thread->hw_brk);
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ thread->hw_brk[i].address = 0;
+ thread->hw_brk[i].type = 0;
+ thread->hw_brk[i].len = 0;
+ thread->hw_brk[i].hw_len = 0;
+ if (ppc_breakpoint_available())
+ set_breakpoint(&thread->hw_brk[i], i);
+ }
+}
+
+static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
+ struct arch_hw_breakpoint *b)
+{
+ if (a->address != b->address)
+ return false;
+ if (a->type != b->type)
+ return false;
+ if (a->len != b->len)
+ return false;
+ /* no need to check hw_len. it's calculated from address and len */
+ return true;
+}
+
+static void switch_hw_breakpoint(struct task_struct *new)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[i]),
+ &new->thread.hw_brk[i]))) {
+ __set_breakpoint(&new->thread.hw_brk[i], i);
+ }
+ }
}
#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
@@ -822,19 +851,6 @@ bool ppc_breakpoint_available(void)
}
EXPORT_SYMBOL_GPL(ppc_breakpoint_available);
-static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
- struct arch_hw_breakpoint *b)
-{
- if (a->address != b->address)
- return false;
- if (a->type != b->type)
- return false;
- if (a->len != b->len)
- return false;
- /* no need to check hw_len. it's calculated from address and len */
- return true;
-}
-
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
static inline bool tm_enabled(struct task_struct *tsk)
@@ -1167,8 +1183,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
* schedule DABR
*/
#ifndef CONFIG_HAVE_HW_BREAKPOINT
- if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[0]), &new->thread.hw_brk)))
- __set_breakpoint(&new->thread.hw_brk, 0);
+ switch_hw_breakpoint(new);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 12962302d6a4..0dbb35392dd2 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -67,11 +67,16 @@ int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
/* We only support one DABR and no IABRS at the moment */
if (addr > 0)
return -EINVAL;
- dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
- (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
+ dabr_fake = ((child->thread.hw_brk[0].address & (~HW_BRK_TYPE_DABR)) |
+ (child->thread.hw_brk[0].type & HW_BRK_TYPE_DABR));
return put_user(dabr_fake, datalp);
}
+/*
+ * ptrace_set_debugreg() fakes DABR and DABR is only one. So even if
+ * internal hw supports more than one watchpoint, we support only one
+ * watchpoint with this interface.
+ */
int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned long data)
{
#ifdef CONFIG_HAVE_HW_BREAKPOINT
@@ -137,7 +142,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned l
return ret;
thread->ptrace_bps[0] = bp;
- thread->hw_brk = hw_brk;
+ thread->hw_brk[0] = hw_brk;
return 0;
}
@@ -159,12 +164,24 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned l
if (set_bp && (!ppc_breakpoint_available()))
return -ENODEV;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
- task->thread.hw_brk = hw_brk;
+ task->thread.hw_brk[0] = hw_brk;
return 0;
}
+static int find_empty_hw_brk(struct thread_struct *thread)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!thread->hw_brk[i].address)
+ return i;
+ }
+ return -1;
+}
+
long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_info)
{
+ int i;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
int len = 0;
struct thread_struct *thread = &child->thread;
@@ -223,15 +240,16 @@ long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_inf
if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
return -EINVAL;
- if (child->thread.hw_brk.address)
+ i = find_empty_hw_brk(&child->thread);
+ if (i < 0)
return -ENOSPC;
if (!ppc_breakpoint_available())
return -ENODEV;
- child->thread.hw_brk = brk;
+ child->thread.hw_brk[i] = brk;
- return 1;
+ return i + 1;
}
long ppc_del_hwdebug(struct task_struct *child, long data)
@@ -241,7 +259,7 @@ long ppc_del_hwdebug(struct task_struct *child, long data)
struct thread_struct *thread = &child->thread;
struct perf_event *bp;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
- if (data != 1)
+ if (data < 1 || data > nr_wp_slots())
return -EINVAL;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
@@ -254,11 +272,11 @@ long ppc_del_hwdebug(struct task_struct *child, long data)
}
return ret;
#else /* CONFIG_HAVE_HW_BREAKPOINT */
- if (child->thread.hw_brk.address == 0)
+ if (child->thread.hw_brk[data - 1].address == 0)
return -ENOENT;
- child->thread.hw_brk.address = 0;
- child->thread.hw_brk.type = 0;
+ child->thread.hw_brk[data - 1].address = 0;
+ child->thread.hw_brk[data - 1].type = 0;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
return 0;
diff --git a/arch/powerpc/kernel/ptrace/ptrace32.c b/arch/powerpc/kernel/ptrace/ptrace32.c
index 7976ddf29c0e..7589a9665ffb 100644
--- a/arch/powerpc/kernel/ptrace/ptrace32.c
+++ b/arch/powerpc/kernel/ptrace/ptrace32.c
@@ -259,8 +259,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
ret = put_user(child->thread.debug.dac1, (u32 __user *)data);
#else
dabr_fake = (
- (child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
- (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
+ (child->thread.hw_brk[0].address & (~HW_BRK_TYPE_DABR)) |
+ (child->thread.hw_brk[0].type & HW_BRK_TYPE_DABR));
ret = put_user(dabr_fake, (u32 __user *)data);
#endif
break;
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index bbf237f072d4..b559b114d03d 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -107,6 +107,9 @@ static void do_signal(struct task_struct *tsk)
struct ksignal ksig = { .sig = 0 };
int ret;
int is32 = is_32bit_task();
+#ifndef CONFIG_PPC_ADV_DEBUG_REGS
+ int i;
+#endif
BUG_ON(tsk != current);
@@ -128,8 +131,10 @@ static void do_signal(struct task_struct *tsk)
* user space. The DABR will have been cleared if it
* triggered inside the kernel.
*/
- if (tsk->thread.hw_brk.address && tsk->thread.hw_brk.type)
- __set_breakpoint(&tsk->thread.hw_brk, 0);
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (tsk->thread.hw_brk[i].address && tsk->thread.hw_brk[i].type)
+ __set_breakpoint(&tsk->thread.hw_brk[i], i);
+ }
#endif
/* Re-enable the breakpoints for the signal stack */
thread_change_pc(tsk, tsk->thread.regs);
--
2.21.1
^ permalink raw reply related
* [PATCH v2 10/16] powerpc/watchpoint: Use loop for thread_struct->ptrace_bps
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
ptrace_bps is already an array of size HBP_NUM_MAX. But we use
hardcoded index 0 while fetching/updating it. Convert such code
to loop over array.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/hw_breakpoint.c | 7 ++++--
arch/powerpc/kernel/process.c | 6 ++++-
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 28 +++++++++++++++++------
3 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 7562f9ceb77e..6daa95b8bb88 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -419,10 +419,13 @@ NOKPROBE_SYMBOL(hw_breakpoint_exceptions_notify);
*/
void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
{
+ int i;
struct thread_struct *t = &tsk->thread;
- unregister_hw_breakpoint(t->ptrace_bps[0]);
- t->ptrace_bps[0] = NULL;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ unregister_hw_breakpoint(t->ptrace_bps[i]);
+ t->ptrace_bps[i] = NULL;
+ }
}
void hw_breakpoint_pmu_read(struct perf_event *bp)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index f5b4f21e822b..73c0800f0bcf 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1618,6 +1618,9 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
void (*f)(void);
unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
struct thread_info *ti = task_thread_info(p);
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ int i;
+#endif
klp_init_thread_info(p);
@@ -1677,7 +1680,8 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
p->thread.ksp_limit = (unsigned long)end_of_stack(p);
#endif
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- p->thread.ptrace_bps[0] = NULL;
+ for (i = 0; i < nr_wp_slots(); i++)
+ p->thread.ptrace_bps[i] = NULL;
#endif
p->thread.fp_save_area = NULL;
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 0dbb35392dd2..08cb8c1b504c 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -168,6 +168,19 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned l
return 0;
}
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+static int find_empty_ptrace_bp(struct thread_struct *thread)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!thread->ptrace_bps[i])
+ return i;
+ }
+ return -1;
+}
+#endif
+
static int find_empty_hw_brk(struct thread_struct *thread)
{
int i;
@@ -217,8 +230,9 @@ long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_inf
len = 1;
else
return -EINVAL;
- bp = thread->ptrace_bps[0];
- if (bp)
+
+ i = find_empty_ptrace_bp(thread);
+ if (i < 0)
return -ENOSPC;
/* Create a new breakpoint request if one doesn't exist already */
@@ -228,13 +242,13 @@ long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_inf
arch_bp_generic_fields(brk.type, &attr.bp_type);
bp = register_user_hw_breakpoint(&attr, ptrace_triggered, NULL, child);
- thread->ptrace_bps[0] = bp;
+ thread->ptrace_bps[i] = bp;
if (IS_ERR(bp)) {
- thread->ptrace_bps[0] = NULL;
+ thread->ptrace_bps[i] = NULL;
return PTR_ERR(bp);
}
- return 1;
+ return i + 1;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
@@ -263,10 +277,10 @@ long ppc_del_hwdebug(struct task_struct *child, long data)
return -EINVAL;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- bp = thread->ptrace_bps[0];
+ bp = thread->ptrace_bps[data - 1];
if (bp) {
unregister_hw_breakpoint(bp);
- thread->ptrace_bps[0] = NULL;
+ thread->ptrace_bps[data - 1] = NULL;
} else {
ret = -ENOENT;
}
--
2.21.1
^ permalink raw reply related
* [PATCH v2 11/16] powerpc/watchpoint: Introduce is_ptrace_bp() function
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Introduce is_ptrace_bp() function and move the check inside the
function. We will utilize it more in later set of patches.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/kernel/hw_breakpoint.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 6daa95b8bb88..8d3f7d87d790 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -90,6 +90,11 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp)
hw_breakpoint_disable();
}
+static bool is_ptrace_bp(struct perf_event *bp)
+{
+ return bp->overflow_handler == ptrace_triggered;
+}
+
/*
* Perform cleanup of arch-specific counters during unregistration
* of the perf-event
@@ -324,7 +329,7 @@ int hw_breakpoint_handler(struct die_args *args)
* one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
* generated in do_dabr().
*/
- if (bp->overflow_handler == ptrace_triggered) {
+ if (is_ptrace_bp(bp)) {
perf_bp_event(bp, regs);
rc = NOTIFY_DONE;
goto out;
--
2.21.1
^ permalink raw reply related
* [PATCH v2 12/16] powerpc/watchpoint: Use builtin ALIGN*() macros
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Currently we calculate hw aligned start and end addresses manually.
Replace them with builtin ALIGN_DOWN() and ALIGN() macros.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
arch/powerpc/kernel/hw_breakpoint.c | 6 +++---
arch/powerpc/kernel/process.c | 4 ++--
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 2 +-
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index fae33c729ba9..abc4603c0efe 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -34,10 +34,11 @@ struct arch_hw_breakpoint {
#define HW_BRK_TYPE_PRIV_ALL (HW_BRK_TYPE_USER | HW_BRK_TYPE_KERNEL | \
HW_BRK_TYPE_HYP)
+/* Minimum granularity */
#ifdef CONFIG_PPC_8xx
-#define HW_BREAKPOINT_ALIGN 0x3
+#define HW_BREAKPOINT_SIZE 0x4
#else
-#define HW_BREAKPOINT_ALIGN 0x7
+#define HW_BREAKPOINT_SIZE 0x8
#endif
#define DABR_MAX_LEN 8
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 8d3f7d87d790..71274fbbac38 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -145,7 +145,7 @@ int arch_bp_generic_fields(int type, int *gen_bp_type)
* <---8 bytes--->
*
* In this case, we should configure hw as:
- * start_addr = address & ~HW_BREAKPOINT_ALIGN
+ * start_addr = address & ~(HW_BREAKPOINT_SIZE - 1)
* len = 16 bytes
*
* @start_addr and @end_addr are inclusive.
@@ -156,8 +156,8 @@ static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
u16 hw_len;
unsigned long start_addr, end_addr;
- start_addr = hw->address & ~HW_BREAKPOINT_ALIGN;
- end_addr = (hw->address + hw->len - 1) | HW_BREAKPOINT_ALIGN;
+ start_addr = ALIGN_DOWN(hw->address, HW_BREAKPOINT_SIZE);
+ end_addr = ALIGN(hw->address + hw->len, HW_BREAKPOINT_SIZE) - 1;
hw_len = end_addr - start_addr + 1;
if (dawr_enabled()) {
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 73c0800f0bcf..cbd6e9b79401 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -794,8 +794,8 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
unsigned long lctrl1 = LCTRL1_CTE_GT | LCTRL1_CTF_LT | LCTRL1_CRWE_RW |
LCTRL1_CRWF_RW;
unsigned long lctrl2 = LCTRL2_LW0EN | LCTRL2_LW0LADC | LCTRL2_SLW0EN;
- unsigned long start_addr = brk->address & ~HW_BREAKPOINT_ALIGN;
- unsigned long end_addr = (brk->address + brk->len - 1) | HW_BREAKPOINT_ALIGN;
+ unsigned long start_addr = ALIGN_DOWN(brk->address, HW_BREAKPOINT_SIZE);
+ unsigned long end_addr = ALIGN(brk->address + brk->len, HW_BREAKPOINT_SIZE) - 1;
if (start_addr == 0)
lctrl2 |= LCTRL2_LW0LA_F;
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 08cb8c1b504c..697c7e4b5877 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -216,7 +216,7 @@ long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_inf
if ((unsigned long)bp_info->addr >= TASK_SIZE)
return -EIO;
- brk.address = bp_info->addr & ~HW_BREAKPOINT_ALIGN;
+ brk.address = ALIGN_DOWN(bp_info->addr, HW_BREAKPOINT_SIZE);
brk.type = HW_BRK_TYPE_TRANSLATE;
brk.len = DABR_MAX_LEN;
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
--
2.21.1
^ permalink raw reply related
* [PATCH v2 13/16] powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Currently we assume that we have only one watchpoint supported by hw.
Get rid of that assumption and use dynamic loop instead. This should
make supporting more watchpoints very easy.
With more than one watchpoint, exception handler need to know which
DAWR caused the exception, and hw currently does not provide it. So
we need sw logic for the same. To figure out which DAWR caused the
exception, check all different combinations of user specified range,
dawr address range, actual access range and dawrx constrains. For ex,
if user specified range and actual access range overlaps but dawrx is
configured for readonly watchpoint and the instruction is store, this
DAWR must not have caused exception.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/include/asm/sstep.h | 2 +
arch/powerpc/kernel/hw_breakpoint.c | 396 +++++++++++++++++++++------
arch/powerpc/kernel/process.c | 3 -
4 files changed, 313 insertions(+), 90 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 65b03162cd67..4bc4e4a99166 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -185,7 +185,7 @@ struct thread_struct {
* Helps identify source of single-step exception and subsequent
* hw-breakpoint enablement
*/
- struct perf_event *last_hit_ubp;
+ struct perf_event *last_hit_ubp[HBP_NUM_MAX];
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint info */
unsigned long trap_nr; /* last trap # on this thread */
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index 769f055509c9..38919b27a6fa 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -48,6 +48,8 @@ enum instruction_type {
#define INSTR_TYPE_MASK 0x1f
+#define OP_IS_LOAD(type) ((LOAD <= (type) && (type) <= LOAD_VSX) || (type) == LARX)
+#define OP_IS_STORE(type) ((STORE <= (type) && (type) <= STORE_VSX) || (type) == STCX)
#define OP_IS_LOAD_STORE(type) (LOAD <= (type) && (type) <= STCX)
/* Compute flags, ORed in with type */
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 71274fbbac38..07a6cdea84ed 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -30,7 +30,7 @@
* Stores the breakpoints currently in use on each breakpoint address
* register for every cpu
*/
-static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM_MAX]);
/*
* Returns total number of data or instruction breakpoints available.
@@ -42,6 +42,17 @@ int hw_breakpoint_slots(int type)
return 0; /* no instruction breakpoints available */
}
+static bool single_step_pending(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (current->thread.last_hit_ubp[i])
+ return true;
+ }
+ return false;
+}
+
/*
* Install a perf counter breakpoint.
*
@@ -54,16 +65,26 @@ int hw_breakpoint_slots(int type)
int arch_install_hw_breakpoint(struct perf_event *bp)
{
struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg);
+ struct perf_event **slot;
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ slot = this_cpu_ptr(&bp_per_reg[i]);
+ if (!*slot) {
+ *slot = bp;
+ break;
+ }
+ }
- *slot = bp;
+ if (WARN_ONCE(i == nr_wp_slots(), "Can't find any breakpoint slot"))
+ return -EBUSY;
/*
* Do not install DABR values if the instruction must be single-stepped.
* If so, DABR will be populated in single_step_dabr_instruction().
*/
- if (current->thread.last_hit_ubp != bp)
- __set_breakpoint(info, 0);
+ if (!single_step_pending())
+ __set_breakpoint(info, i);
return 0;
}
@@ -79,15 +100,22 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
*/
void arch_uninstall_hw_breakpoint(struct perf_event *bp)
{
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg);
+ struct arch_hw_breakpoint null_brk = {0};
+ struct perf_event **slot;
+ int i;
- if (*slot != bp) {
- WARN_ONCE(1, "Can't find the breakpoint");
- return;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ slot = this_cpu_ptr(&bp_per_reg[i]);
+ if (*slot == bp) {
+ *slot = NULL;
+ break;
+ }
}
- *slot = NULL;
- hw_breakpoint_disable();
+ if (WARN_ONCE(i == nr_wp_slots(), "Can't find any breakpoint slot"))
+ return;
+
+ __set_breakpoint(&null_brk, i);
}
static bool is_ptrace_bp(struct perf_event *bp)
@@ -101,14 +129,20 @@ static bool is_ptrace_bp(struct perf_event *bp)
*/
void arch_unregister_hw_breakpoint(struct perf_event *bp)
{
+ int i;
+
/*
* If the breakpoint is unregistered between a hw_breakpoint_handler()
* and the single_step_dabr_instruction(), then cleanup the breakpoint
* restoration variables to prevent dangling pointers.
* FIXME, this should not be using bp->ctx at all! Sayeth peterz.
*/
- if (bp->ctx && bp->ctx->task && bp->ctx->task != ((void *)-1L))
- bp->ctx->task->thread.last_hit_ubp = NULL;
+ if (bp->ctx && bp->ctx->task && bp->ctx->task != ((void *)-1L)) {
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (bp->ctx->task->thread.last_hit_ubp[i] == bp)
+ bp->ctx->task->thread.last_hit_ubp[i] = NULL;
+ }
+ }
}
/*
@@ -220,90 +254,215 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs)
{
struct arch_hw_breakpoint *info;
+ int i;
- if (likely(!tsk->thread.last_hit_ubp))
- return;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (unlikely(tsk->thread.last_hit_ubp[i]))
+ goto reset;
+ }
+ return;
- info = counter_arch_bp(tsk->thread.last_hit_ubp);
+reset:
regs->msr &= ~MSR_SE;
- __set_breakpoint(info, 0);
- tsk->thread.last_hit_ubp = NULL;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ info = counter_arch_bp(__this_cpu_read(bp_per_reg[i]));
+ __set_breakpoint(info, i);
+ tsk->thread.last_hit_ubp[i] = NULL;
+ }
}
-static bool dar_within_range(unsigned long dar, struct arch_hw_breakpoint *info)
+static bool dar_in_user_range(unsigned long dar, struct arch_hw_breakpoint *info)
{
return ((info->address <= dar) && (dar - info->address < info->len));
}
-static bool
-dar_range_overlaps(unsigned long dar, int size, struct arch_hw_breakpoint *info)
+static bool dar_user_range_overlaps(unsigned long dar, int size,
+ struct arch_hw_breakpoint *info)
{
return ((dar <= info->address + info->len - 1) &&
(dar + size - 1 >= info->address));
}
+static bool dar_in_hw_range(unsigned long dar, struct arch_hw_breakpoint *info)
+{
+ unsigned long hw_start_addr, hw_end_addr;
+
+ hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
+ hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE) - 1;
+
+ return ((hw_start_addr <= dar) && (hw_end_addr >= dar));
+}
+
+static bool dar_hw_range_overlaps(unsigned long dar, int size,
+ struct arch_hw_breakpoint *info)
+{
+ unsigned long hw_start_addr, hw_end_addr;
+
+ hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
+ hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE) - 1;
+
+ return ((dar <= hw_end_addr) && (dar + size - 1 >= hw_start_addr));
+}
+
/*
- * Handle debug exception notifications.
+ * If hw has multiple DAWR registers, we also need to check all
+ * dawrx constraint bits to confirm this is _really_ a valid event.
*/
-static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
- struct arch_hw_breakpoint *info)
+static bool check_dawrx_constraints(struct pt_regs *regs, int type,
+ struct arch_hw_breakpoint *info)
{
- unsigned int instr = 0;
- int ret, type, size;
- struct instruction_op op;
- unsigned long addr = info->address;
+ if (OP_IS_LOAD(type) && !(info->type & HW_BRK_TYPE_READ))
+ return false;
- if (__get_user_inatomic(instr, (unsigned int *)regs->nip))
- goto fail;
+ if (OP_IS_STORE(type) && !(info->type & HW_BRK_TYPE_WRITE))
+ return false;
- ret = analyse_instr(&op, regs, instr);
- type = GETTYPE(op.type);
- size = GETSIZE(op.type);
+ if (is_kernel_addr(regs->nip) && !(info->type & HW_BRK_TYPE_KERNEL))
+ return false;
- if (!ret && (type == LARX || type == STCX)) {
- printk_ratelimited("Breakpoint hit on instruction that can't be emulated."
- " Breakpoint at 0x%lx will be disabled.\n", addr);
- goto disable;
- }
+ if (user_mode(regs) && !(info->type & HW_BRK_TYPE_USER))
+ return false;
+
+ return true;
+}
+
+/*
+ * Returns true if the event is valid wrt dawr configuration,
+ * including extraneous exception. Otherwise return false.
+ */
+static bool check_constraints(struct pt_regs *regs, unsigned int instr,
+ int type, int size,
+ struct arch_hw_breakpoint *info)
+{
+ bool in_user_range = dar_in_user_range(regs->dar, info);
+ bool dawrx_constraints;
/*
- * If it's extraneous event, we still need to emulate/single-
- * step the instruction, but we don't generate an event.
+ * 8xx supports only one breakpoint and thus we can
+ * unconditionally return true.
*/
- if (size && !dar_range_overlaps(regs->dar, size, info))
- info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
+ if (IS_ENABLED(CONFIG_PPC_8xx)) {
+ if (!in_user_range)
+ info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
+ return true;
+ }
- /* Do not emulate user-space instructions, instead single-step them */
- if (user_mode(regs)) {
- current->thread.last_hit_ubp = bp;
- regs->msr |= MSR_SE;
+ if (unlikely(instr == -1)) {
+ if (in_user_range)
+ return true;
+
+ if (dar_in_hw_range(regs->dar, info)) {
+ info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
+ return true;
+ }
return false;
}
- if (!emulate_step(regs, instr))
- goto fail;
+ dawrx_constraints = check_dawrx_constraints(regs, type, info);
- return true;
+ if (dar_user_range_overlaps(regs->dar, size, info))
+ return dawrx_constraints;
+
+ if (dar_hw_range_overlaps(regs->dar, size, info)) {
+ if (dawrx_constraints) {
+ info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
+ return true;
+ }
+ }
+ return false;
+}
+
+static int get_instr_detail(struct pt_regs *regs, int *type, int *size,
+ bool *larx_stcx)
+{
+ unsigned int instr = 0;
+ struct instruction_op op;
+
+ if (__get_user_inatomic(instr, (unsigned int *)regs->nip))
+ return -1;
+
+ analyse_instr(&op, regs, instr);
-fail:
/*
- * We've failed in reliably handling the hw-breakpoint. Unregister
- * it and throw a warning message to let the user know about it.
+ * Set size = 8 if analyse_instr() fails. If it's a userspace
+ * watchpoint(valid or extraneous), we can notify user about it.
+ * If it's a kernel watchpoint, instruction emulation will fail
+ * in stepping_handler() and watchpoint will be disabled.
*/
- WARN(1, "Unable to handle hardware breakpoint. Breakpoint at "
- "0x%lx will be disabled.", addr);
+ *type = GETTYPE(op.type);
+ *size = !(*type == UNKNOWN) ? GETSIZE(op.type) : 8;
+ *larx_stcx = (*type == LARX || *type == STCX);
-disable:
+ return instr;
+}
+
+/*
+ * We've failed in reliably handling the hw-breakpoint. Unregister
+ * it and throw a warning message to let the user know about it.
+ */
+static void handler_error(struct perf_event *bp, struct arch_hw_breakpoint *info)
+{
+ WARN(1, "Unable to handle hardware breakpoint."
+ "Breakpoint at 0x%lx will be disabled.",
+ info->address);
+ perf_event_disable_inatomic(bp);
+}
+
+static void larx_stcx_err(struct perf_event *bp, struct arch_hw_breakpoint *info)
+{
+ printk_ratelimited("Breakpoint hit on instruction that can't "
+ "be emulated. Breakpoint at 0x%lx will be "
+ "disabled.\n", info->address);
perf_event_disable_inatomic(bp);
- return false;
+}
+
+static bool stepping_handler(struct pt_regs *regs, struct perf_event **bp,
+ struct arch_hw_breakpoint **info, int *hit,
+ unsigned int instr)
+{
+ int i;
+ int stepped;
+
+ /* Do not emulate user-space instructions, instead single-step them */
+ if (user_mode(regs)) {
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!hit[i])
+ continue;
+ current->thread.last_hit_ubp[i] = bp[i];
+ info[i] = NULL;
+ }
+ regs->msr |= MSR_SE;
+ return false;
+ }
+
+ stepped = emulate_step(regs, instr);
+ if (!stepped) {
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!hit[i])
+ continue;
+ handler_error(bp[i], info[i]);
+ info[i] = NULL;
+ }
+ return false;
+ }
+ return true;
}
int hw_breakpoint_handler(struct die_args *args)
{
+ bool err = false;
int rc = NOTIFY_STOP;
- struct perf_event *bp;
+ struct perf_event *bp[HBP_NUM_MAX] = {0};
struct pt_regs *regs = args->regs;
- struct arch_hw_breakpoint *info;
+ struct arch_hw_breakpoint *info[HBP_NUM_MAX] = {0};
+ int i;
+ int hit[HBP_NUM_MAX] = {0};
+ int nr_hit = 0;
+ bool ptrace_bp = false;
+ unsigned int instr = 0;
+ int type = 0;
+ int size = 0;
+ bool larx_stcx = false;
/* Disable breakpoints during exception handling */
hw_breakpoint_disable();
@@ -316,12 +475,39 @@ int hw_breakpoint_handler(struct die_args *args)
*/
rcu_read_lock();
- bp = __this_cpu_read(bp_per_reg);
- if (!bp) {
+ if (!IS_ENABLED(CONFIG_PPC_8xx))
+ instr = get_instr_detail(regs, &type, &size, &larx_stcx);
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ bp[i] = __this_cpu_read(bp_per_reg[i]);
+ if (!bp[i])
+ continue;
+
+ info[i] = counter_arch_bp(bp[i]);
+ info[i]->type &= ~HW_BRK_TYPE_EXTRANEOUS_IRQ;
+
+ if (check_constraints(regs, instr, type, size, info[i])) {
+ if (!IS_ENABLED(CONFIG_PPC_8xx) && instr == -1) {
+ handler_error(bp[i], info[i]);
+ info[i] = NULL;
+ err = 1;
+ continue;
+ }
+
+ if (is_ptrace_bp(bp[i]))
+ ptrace_bp = true;
+ hit[i] = 1;
+ nr_hit++;
+ }
+ }
+
+ if (err)
+ goto reset;
+
+ if (!nr_hit) {
rc = NOTIFY_DONE;
goto out;
}
- info = counter_arch_bp(bp);
/*
* Return early after invoking user-callback function without restoring
@@ -329,29 +515,50 @@ int hw_breakpoint_handler(struct die_args *args)
* one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
* generated in do_dabr().
*/
- if (is_ptrace_bp(bp)) {
- perf_bp_event(bp, regs);
+ if (ptrace_bp) {
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!hit[i])
+ continue;
+ perf_bp_event(bp[i], regs);
+ info[i] = NULL;
+ }
rc = NOTIFY_DONE;
- goto out;
+ goto reset;
}
- info->type &= ~HW_BRK_TYPE_EXTRANEOUS_IRQ;
- if (IS_ENABLED(CONFIG_PPC_8xx)) {
- if (!dar_within_range(regs->dar, info))
- info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
- } else {
- if (!stepping_handler(regs, bp, info))
- goto out;
+ if (!IS_ENABLED(CONFIG_PPC_8xx)) {
+ if (larx_stcx) {
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!hit[i])
+ continue;
+ larx_stcx_err(bp[i], info[i]);
+ info[i] = NULL;
+ }
+ goto reset;
+ }
+
+ if (!stepping_handler(regs, bp, info, hit, instr))
+ goto reset;
}
/*
* As a policy, the callback is invoked in a 'trigger-after-execute'
* fashion
*/
- if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
- perf_bp_event(bp, regs);
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!hit[i])
+ continue;
+ if (!(info[i]->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
+ perf_bp_event(bp[i], regs);
+ }
+
+reset:
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!info[i])
+ continue;
+ __set_breakpoint(info[i], i);
+ }
- __set_breakpoint(info, 0);
out:
rcu_read_unlock();
return rc;
@@ -366,26 +573,43 @@ static int single_step_dabr_instruction(struct die_args *args)
struct pt_regs *regs = args->regs;
struct perf_event *bp = NULL;
struct arch_hw_breakpoint *info;
+ int i;
+ bool found = false;
- bp = current->thread.last_hit_ubp;
/*
* Check if we are single-stepping as a result of a
* previous HW Breakpoint exception
*/
- if (!bp)
- return NOTIFY_DONE;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ bp = current->thread.last_hit_ubp[i];
+
+ if (!bp)
+ continue;
+
+ found = true;
+ info = counter_arch_bp(bp);
+
+ /*
+ * We shall invoke the user-defined callback function in the
+ * single stepping handler to confirm to 'trigger-after-execute'
+ * semantics
+ */
+ if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
+ perf_bp_event(bp, regs);
+ current->thread.last_hit_ubp[i] = NULL;
+ }
- info = counter_arch_bp(bp);
+ if (!found)
+ return NOTIFY_DONE;
- /*
- * We shall invoke the user-defined callback function in the single
- * stepping handler to confirm to 'trigger-after-execute' semantics
- */
- if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
- perf_bp_event(bp, regs);
+ for (i = 0; i < nr_wp_slots(); i++) {
+ bp = __this_cpu_read(bp_per_reg[i]);
+ if (!bp)
+ continue;
- __set_breakpoint(info, 0);
- current->thread.last_hit_ubp = NULL;
+ info = counter_arch_bp(bp);
+ __set_breakpoint(info, i);
+ }
/*
* If the process was being single-stepped by ptrace, let the
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index cbd6e9b79401..7c64299b001a 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -622,9 +622,6 @@ void do_break (struct pt_regs *regs, unsigned long address,
if (debugger_break_match(regs))
return;
- /* Clear the breakpoint */
- hw_breakpoint_disable();
-
/* Deliver the signal to userspace */
force_sig_fault(SIGTRAP, TRAP_HWBKPT, (void __user *)address);
}
--
2.21.1
^ permalink raw reply related
* [PATCH v2 14/16] powerpc/watchpoint: Don't allow concurrent perf and ptrace events
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
With Book3s DAWR, ptrace and perf watchpoints on powerpc behaves
differently. Ptrace watchpoint works in one-shot mode and generates
signal before executing instruction. It's ptrace user's job to
single-step the instruction and re-enable the watchpoint. OTOH, in
case of perf watchpoint, kernel emulates/single-steps the instruction
and then generates event. If perf and ptrace creates two events with
same or overlapping address ranges, it's ambiguous to decide who
should single-step the instruction. Because of this issue, don't
allow perf and ptrace watchpoint at the same time if their address
range overlaps.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/include/asm/hw_breakpoint.h | 2 +
arch/powerpc/kernel/hw_breakpoint.c | 222 +++++++++++++++++++++++
kernel/events/hw_breakpoint.c | 16 ++
3 files changed, 240 insertions(+)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index abc4603c0efe..9d3bd1169591 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -70,6 +70,8 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
unsigned long val, void *data);
int arch_install_hw_breakpoint(struct perf_event *bp);
void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+int arch_reserve_bp_slot(struct perf_event *bp);
+void arch_release_bp_slot(struct perf_event *bp);
void arch_unregister_hw_breakpoint(struct perf_event *bp);
void hw_breakpoint_pmu_read(struct perf_event *bp);
extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 07a6cdea84ed..f813acb0d9f0 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -123,6 +123,228 @@ static bool is_ptrace_bp(struct perf_event *bp)
return bp->overflow_handler == ptrace_triggered;
}
+struct breakpoint {
+ struct list_head list;
+ struct perf_event *bp;
+ bool ptrace_bp;
+};
+
+static DEFINE_PER_CPU(struct breakpoint *, cpu_bps[HBP_NUM_MAX]);
+static LIST_HEAD(task_bps);
+
+static struct breakpoint *alloc_breakpoint(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp)
+ return ERR_PTR(-ENOMEM);
+ tmp->bp = bp;
+ tmp->ptrace_bp = is_ptrace_bp(bp);
+ return tmp;
+}
+
+static bool bp_addr_range_overlap(struct perf_event *bp1, struct perf_event *bp2)
+{
+ __u64 bp1_saddr, bp1_eaddr, bp2_saddr, bp2_eaddr;
+
+ bp1_saddr = ALIGN_DOWN(bp1->attr.bp_addr, HW_BREAKPOINT_SIZE);
+ bp1_eaddr = ALIGN(bp1->attr.bp_addr + bp1->attr.bp_len, HW_BREAKPOINT_SIZE) - 1;
+ bp2_saddr = ALIGN_DOWN(bp2->attr.bp_addr, HW_BREAKPOINT_SIZE);
+ bp2_eaddr = ALIGN(bp2->attr.bp_addr + bp2->attr.bp_len, HW_BREAKPOINT_SIZE) - 1;
+
+ return (bp1_saddr <= bp2_eaddr && bp1_eaddr >= bp2_saddr);
+}
+
+static bool alternate_infra_bp(struct breakpoint *b, struct perf_event *bp)
+{
+ return is_ptrace_bp(bp) ? !b->ptrace_bp : b->ptrace_bp;
+}
+
+static bool can_co_exist(struct breakpoint *b, struct perf_event *bp)
+{
+ return !(alternate_infra_bp(b, bp) && bp_addr_range_overlap(b->bp, bp));
+}
+
+static int task_bps_add(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ tmp = alloc_breakpoint(bp);
+ if (IS_ERR(tmp))
+ return PTR_ERR(tmp);
+
+ list_add(&tmp->list, &task_bps);
+ return 0;
+}
+
+static void task_bps_remove(struct perf_event *bp)
+{
+ struct list_head *pos, *q;
+ struct breakpoint *tmp;
+
+ list_for_each_safe(pos, q, &task_bps) {
+ tmp = list_entry(pos, struct breakpoint, list);
+
+ if (tmp->bp == bp) {
+ list_del(&tmp->list);
+ kfree(tmp);
+ break;
+ }
+ }
+}
+
+/*
+ * If any task has breakpoint from alternate infrastructure,
+ * return true. Otherwise return false.
+ */
+static bool all_task_bps_check(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ list_for_each_entry(tmp, &task_bps, list) {
+ if (!can_co_exist(tmp, bp))
+ return true;
+ }
+ return false;
+}
+
+/*
+ * If same task has breakpoint from alternate infrastructure,
+ * return true. Otherwise return false.
+ */
+static bool same_task_bps_check(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ list_for_each_entry(tmp, &task_bps, list) {
+ if (tmp->bp->hw.target == bp->hw.target &&
+ !can_co_exist(tmp, bp))
+ return true;
+ }
+ return false;
+}
+
+static int cpu_bps_add(struct perf_event *bp)
+{
+ struct breakpoint **cpu_bp;
+ struct breakpoint *tmp;
+ int i = 0;
+
+ tmp = alloc_breakpoint(bp);
+ if (IS_ERR(tmp))
+ return PTR_ERR(tmp);
+
+ cpu_bp = per_cpu_ptr(cpu_bps, bp->cpu);
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!cpu_bp[i]) {
+ cpu_bp[i] = tmp;
+ break;
+ }
+ }
+ return 0;
+}
+
+static void cpu_bps_remove(struct perf_event *bp)
+{
+ struct breakpoint **cpu_bp;
+ int i = 0;
+
+ cpu_bp = per_cpu_ptr(cpu_bps, bp->cpu);
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!cpu_bp[i])
+ continue;
+
+ if (cpu_bp[i]->bp == bp) {
+ kfree(cpu_bp[i]);
+ cpu_bp[i] = NULL;
+ break;
+ }
+ }
+}
+
+static bool cpu_bps_check(int cpu, struct perf_event *bp)
+{
+ struct breakpoint **cpu_bp;
+ int i;
+
+ cpu_bp = per_cpu_ptr(cpu_bps, cpu);
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (cpu_bp[i] && !can_co_exist(cpu_bp[i], bp))
+ return true;
+ }
+ return false;
+}
+
+static bool all_cpu_bps_check(struct perf_event *bp)
+{
+ int cpu;
+
+ for_each_online_cpu(cpu) {
+ if (cpu_bps_check(cpu, bp))
+ return true;
+ }
+ return false;
+}
+
+/*
+ * We don't use any locks to serialize accesses to cpu_bps or task_bps
+ * because are already inside nr_bp_mutex.
+ */
+int arch_reserve_bp_slot(struct perf_event *bp)
+{
+ int ret;
+
+ /* ptrace breakpoint */
+ if (is_ptrace_bp(bp)) {
+ if (all_cpu_bps_check(bp))
+ return -ENOSPC;
+
+ if (same_task_bps_check(bp))
+ return -ENOSPC;
+
+ return task_bps_add(bp);
+ }
+
+ /* perf breakpoint */
+ if (is_kernel_addr(bp->attr.bp_addr))
+ return 0;
+
+ if (bp->hw.target && bp->cpu == -1) {
+ if (same_task_bps_check(bp))
+ return -ENOSPC;
+
+ return task_bps_add(bp);
+ } else if (!bp->hw.target && bp->cpu != -1) {
+ if (all_task_bps_check(bp))
+ return -ENOSPC;
+
+ return cpu_bps_add(bp);
+ }
+
+ if (same_task_bps_check(bp))
+ return -ENOSPC;
+
+ ret = cpu_bps_add(bp);
+ if (ret)
+ return ret;
+ ret = task_bps_add(bp);
+ if (ret)
+ cpu_bps_remove(bp);
+
+ return ret;
+}
+
+void arch_release_bp_slot(struct perf_event *bp)
+{
+ if (!is_kernel_addr(bp->attr.bp_addr)) {
+ if (bp->hw.target)
+ task_bps_remove(bp);
+ if (bp->cpu != -1)
+ cpu_bps_remove(bp);
+ }
+}
+
/*
* Perform cleanup of arch-specific counters during unregistration
* of the perf-event
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 3cc8416ec844..b48d7039a015 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -213,6 +213,15 @@ toggle_bp_slot(struct perf_event *bp, bool enable, enum bp_type_idx type,
list_del(&bp->hw.bp_list);
}
+__weak int arch_reserve_bp_slot(struct perf_event *bp)
+{
+ return 0;
+}
+
+__weak void arch_release_bp_slot(struct perf_event *bp)
+{
+}
+
/*
* Function to perform processor-specific cleanup during unregistration
*/
@@ -270,6 +279,7 @@ static int __reserve_bp_slot(struct perf_event *bp, u64 bp_type)
struct bp_busy_slots slots = {0};
enum bp_type_idx type;
int weight;
+ int ret;
/* We couldn't initialize breakpoint constraints on boot */
if (!constraints_initialized)
@@ -294,6 +304,10 @@ static int __reserve_bp_slot(struct perf_event *bp, u64 bp_type)
if (slots.pinned + (!!slots.flexible) > nr_slots[type])
return -ENOSPC;
+ ret = arch_reserve_bp_slot(bp);
+ if (ret)
+ return ret;
+
toggle_bp_slot(bp, true, type, weight);
return 0;
@@ -317,6 +331,8 @@ static void __release_bp_slot(struct perf_event *bp, u64 bp_type)
enum bp_type_idx type;
int weight;
+ arch_release_bp_slot(bp);
+
type = find_slot_idx(bp_type);
weight = hw_breakpoint_weight(bp);
toggle_bp_slot(bp, false, type, weight);
--
2.21.1
^ permalink raw reply related
* [PATCH v2 15/16] powerpc/watchpoint/xmon: Don't allow breakpoint overwriting
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Xmon allows overwriting breakpoints because it's supported by only
one dawr. But with multiple dawrs, overwriting becomes ambiguous
or unnecessary complicated. So let's not allow it.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/xmon/xmon.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 508d353e7f06..a1e36501af25 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1382,6 +1382,10 @@ bpt_cmds(void)
printf("Hardware data breakpoint not supported on this cpu\n");
break;
}
+ if (dabr.enabled) {
+ printf("Couldn't find free breakpoint register\n");
+ break;
+ }
mode = 7;
cmd = inchar();
if (cmd == 'r')
--
2.21.1
^ permalink raw reply related
* [PATCH v2 16/16] powerpc/watchpoint/xmon: Support 2nd dawr
From: Ravi Bangoria @ 2020-04-01 6:13 UTC (permalink / raw)
To: mpe, mikey
Cc: apopple, ravi.bangoria, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-1-ravi.bangoria@linux.ibm.com>
Add support for 2nd DAWR in xmon. With this, we can have two
simultaneous breakpoints from xmon.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
arch/powerpc/xmon/xmon.c | 101 ++++++++++++++++++++++++++-------------
1 file changed, 69 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a1e36501af25..4f2c8d4cbc38 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -111,7 +111,7 @@ struct bpt {
#define NBPTS 256
static struct bpt bpts[NBPTS];
-static struct bpt dabr;
+static struct bpt dabr[HBP_NUM_MAX];
static struct bpt *iabr;
static unsigned bpinstr = 0x7fe00008; /* trap */
@@ -787,10 +787,17 @@ static int xmon_sstep(struct pt_regs *regs)
static int xmon_break_match(struct pt_regs *regs)
{
+ int i;
+
if ((regs->msr & (MSR_IR|MSR_PR|MSR_64BIT)) != (MSR_IR|MSR_64BIT))
return 0;
- if (dabr.enabled == 0)
- return 0;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (dabr[i].enabled)
+ goto found;
+ }
+ return 0;
+
+found:
xmon_core(regs, 0);
return 1;
}
@@ -929,13 +936,16 @@ static void insert_bpts(void)
static void insert_cpu_bpts(void)
{
+ int i;
struct arch_hw_breakpoint brk;
- if (dabr.enabled) {
- brk.address = dabr.address;
- brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
- brk.len = DABR_MAX_LEN;
- __set_breakpoint(&brk, 0);
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (dabr[i].enabled) {
+ brk.address = dabr[i].address;
+ brk.type = (dabr[i].enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
+ brk.len = 8;
+ __set_breakpoint(&brk, i);
+ }
}
if (iabr)
@@ -1349,6 +1359,35 @@ static long check_bp_loc(unsigned long addr)
return 1;
}
+static int find_free_data_bpt(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!dabr[i].enabled)
+ return i;
+ }
+ printf("Couldn't find free breakpoint register\n");
+ return -1;
+}
+
+static void print_data_bpts(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!dabr[i].enabled)
+ continue;
+
+ printf(" data "REG" [", dabr[i].address);
+ if (dabr[i].enabled & 1)
+ printf("r");
+ if (dabr[i].enabled & 2)
+ printf("w");
+ printf("]\n");
+ }
+}
+
static char *breakpoint_help_string =
"Breakpoint command usage:\n"
"b show breakpoints\n"
@@ -1382,10 +1421,9 @@ bpt_cmds(void)
printf("Hardware data breakpoint not supported on this cpu\n");
break;
}
- if (dabr.enabled) {
- printf("Couldn't find free breakpoint register\n");
+ i = find_free_data_bpt();
+ if (i < 0)
break;
- }
mode = 7;
cmd = inchar();
if (cmd == 'r')
@@ -1394,15 +1432,15 @@ bpt_cmds(void)
mode = 6;
else
termch = cmd;
- dabr.address = 0;
- dabr.enabled = 0;
- if (scanhex(&dabr.address)) {
- if (!is_kernel_addr(dabr.address)) {
+ dabr[i].address = 0;
+ dabr[i].enabled = 0;
+ if (scanhex(&dabr[i].address)) {
+ if (!is_kernel_addr(dabr[i].address)) {
printf(badaddr);
break;
}
- dabr.address &= ~HW_BRK_TYPE_DABR;
- dabr.enabled = mode | BP_DABR;
+ dabr[i].address &= ~HW_BRK_TYPE_DABR;
+ dabr[i].enabled = mode | BP_DABR;
}
force_enable_xmon();
@@ -1441,7 +1479,9 @@ bpt_cmds(void)
for (i = 0; i < NBPTS; ++i)
bpts[i].enabled = 0;
iabr = NULL;
- dabr.enabled = 0;
+ for (i = 0; i < nr_wp_slots(); i++)
+ dabr[i].enabled = 0;
+
printf("All breakpoints cleared\n");
break;
}
@@ -1475,14 +1515,7 @@ bpt_cmds(void)
if (xmon_is_ro || !scanhex(&a)) {
/* print all breakpoints */
printf(" type address\n");
- if (dabr.enabled) {
- printf(" data "REG" [", dabr.address);
- if (dabr.enabled & 1)
- printf("r");
- if (dabr.enabled & 2)
- printf("w");
- printf("]\n");
- }
+ print_data_bpts();
for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
if (!bp->enabled)
continue;
@@ -1942,8 +1975,13 @@ static void dump_207_sprs(void)
printf("hfscr = %.16lx dhdes = %.16lx rpr = %.16lx\n",
mfspr(SPRN_HFSCR), mfspr(SPRN_DHDES), mfspr(SPRN_RPR));
- printf("dawr = %.16lx dawrx = %.16lx ciabr = %.16lx\n",
- mfspr(SPRN_DAWR0), mfspr(SPRN_DAWRX0), mfspr(SPRN_CIABR));
+ printf("dawr0 = %.16lx dawrx0 = %.16lx\n",
+ mfspr(SPRN_DAWR0), mfspr(SPRN_DAWRX0));
+ if (nr_wp_slots() > 1) {
+ printf("dawr1 = %.16lx dawrx1 = %.16lx\n",
+ mfspr(SPRN_DAWR1), mfspr(SPRN_DAWRX1));
+ }
+ printf("ciabr = %.16lx\n", mfspr(SPRN_CIABR));
#endif
}
@@ -3868,10 +3906,9 @@ static void clear_all_bpt(void)
bpts[i].enabled = 0;
/* Clear any data or iabr breakpoints */
- if (iabr || dabr.enabled) {
- iabr = NULL;
- dabr.enabled = 0;
- }
+ iabr = NULL;
+ for (i = 0; i < nr_wp_slots(); i++)
+ dabr[i].enabled = 0;
}
#ifdef CONFIG_DEBUG_FS
--
2.21.1
^ permalink raw reply related
* Re: [PATCH v2 03/16] powerpc/watchpoint: Introduce function to get nr watchpoints dynamically
From: Christophe Leroy @ 2020-04-01 6:29 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-4-ravi.bangoria@linux.ibm.com>
Le 01/04/2020 à 08:12, Ravi Bangoria a écrit :
> So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1.
> But future Power architecture is introducing 2nd DAWR and thus kernel
> should be able to dynamically find actual number of watchpoints
> supported by hw it's running on. Introduce function for the same.
> Also convert HBP_NUM macro to HBP_NUM_MAX, which will now represent
> maximum number of watchpoints supported by Powerpc.
Everywhere else in the code, it is called 'breakpoint', not 'watchpoint'.
Wouldn't it be more consistent to call the function nr_bp_slots()
instead of nr_wp_slots() ?
Especially as we are likely going to extend your changes to support
DABR2 in addition to DABR on BOOK3S/32.
Christophe
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/cputable.h | 6 +++++-
> arch/powerpc/include/asm/hw_breakpoint.h | 5 +++++
> arch/powerpc/include/asm/processor.h | 2 +-
> arch/powerpc/kernel/hw_breakpoint.c | 2 +-
> 4 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index 40a4d3c6fd99..c67b94f3334c 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -614,7 +614,11 @@ enum {
> };
> #endif /* __powerpc64__ */
>
> -#define HBP_NUM 1
> +/*
> + * Maximum number of hw breakpoint supported on powerpc. Number of
> + * breakpoints supported by actual hw might be less than this.
> + */
> +#define HBP_NUM_MAX 1
>
> #endif /* !__ASSEMBLY__ */
>
> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
> index f2f8d8aa8e3b..518b41eef924 100644
> --- a/arch/powerpc/include/asm/hw_breakpoint.h
> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -43,6 +43,11 @@ struct arch_hw_breakpoint {
> #define DABR_MAX_LEN 8
> #define DAWR_MAX_LEN 512
>
> +static inline int nr_wp_slots(void)
> +{
> + return HBP_NUM_MAX;
> +}
> +
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> #include <linux/kdebug.h>
> #include <asm/reg.h>
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index eedcbfb9a6ff..90f6dbc7ff00 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -180,7 +180,7 @@ struct thread_struct {
> int fpexc_mode; /* floating-point exception mode */
> unsigned int align_ctl; /* alignment handling control */
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> - struct perf_event *ptrace_bps[HBP_NUM];
> + struct perf_event *ptrace_bps[HBP_NUM_MAX];
> /*
> * Helps identify source of single-step exception and subsequent
> * hw-breakpoint enablement
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index 72f461bd70fb..4120349e2abe 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -38,7 +38,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
> int hw_breakpoint_slots(int type)
> {
> if (type == TYPE_DATA)
> - return HBP_NUM;
> + return nr_wp_slots();
> return 0; /* no instruction breakpoints available */
> }
>
>
^ permalink raw reply
* Re: [PATCH v2 07/16] powerpc/watchpoint: Get watchpoint count dynamically while disabling them
From: Christophe Leroy @ 2020-04-01 6:32 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-8-ravi.bangoria@linux.ibm.com>
Le 01/04/2020 à 08:13, Ravi Bangoria a écrit :
> Instead of disabling only one watchpoint, get num of available
> watchpoints dynamically and disable all of them.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/hw_breakpoint.h | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
> index 4e4976c3248b..fae33c729ba9 100644
> --- a/arch/powerpc/include/asm/hw_breakpoint.h
> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -78,14 +78,13 @@ extern void ptrace_triggered(struct perf_event *bp,
> struct perf_sample_data *data, struct pt_regs *regs);
> static inline void hw_breakpoint_disable(void)
> {
> - struct arch_hw_breakpoint brk;
> -
> - brk.address = 0;
> - brk.type = 0;
> - brk.len = 0;
> - brk.hw_len = 0;
> - if (ppc_breakpoint_available())
> - __set_breakpoint(&brk, 0);
> + int i;
> + struct arch_hw_breakpoint null_brk = {0};
Those declaration should be in the block unsigned them below.
> +
> + if (ppc_breakpoint_available()) {
> + for (i = 0; i < nr_wp_slots(); i++)
> + __set_breakpoint(&null_brk, i);
> + }
I would have had a preference to the following, but that's detail I guess:
int i;
struct arch_hw_breakpoint null_brk = {0};
if (!ppc_breakpoint_available())
return;
for (i = 0; i < nr_wp_slots(); i++)
__set_breakpoint(&null_brk, i);
> }
> extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
> int hw_breakpoint_handler(struct die_args *args);
>
Christophe
^ permalink raw reply
* Re: [PATCH v2 08/16] powerpc/watchpoint: Disable all available watchpoints when !dawr_force_enable
From: Christophe Leroy @ 2020-04-01 6:33 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-9-ravi.bangoria@linux.ibm.com>
Le 01/04/2020 à 08:13, Ravi Bangoria a écrit :
> Instead of disabling only first watchpoint, disable all available
> watchpoints while clearing dawr_force_enable.
Can you also explain why you change the function name ?
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/kernel/dawr.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
> index 311e51ee09f4..5c882f07ac7d 100644
> --- a/arch/powerpc/kernel/dawr.c
> +++ b/arch/powerpc/kernel/dawr.c
> @@ -50,9 +50,13 @@ int set_dawr(struct arch_hw_breakpoint *brk, int nr)
> return 0;
> }
>
> -static void set_dawr_cb(void *info)
> +static void disable_dawrs(void *info)
Wouldn't it be better to keep _cb at the end of the function ?
> {
> - set_dawr(info, 0);
> + struct arch_hw_breakpoint null_brk = {0};
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++)
> + set_dawr(&null_brk, i);
> }
>
> static ssize_t dawr_write_file_bool(struct file *file,
> @@ -74,7 +78,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
>
> /* If we are clearing, make sure all CPUs have the DAWR cleared */
> if (!dawr_force_enable)
> - smp_call_function(set_dawr_cb, &null_brk, 0);
> + smp_call_function(disable_dawrs, NULL, 0);
>
> return rc;
> }
>
Christophe
^ permalink raw reply
* Re: [PATCH v2 09/16] powerpc/watchpoint: Convert thread_struct->hw_brk to an array
From: Christophe Leroy @ 2020-04-01 6:43 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-10-ravi.bangoria@linux.ibm.com>
Le 01/04/2020 à 08:13, Ravi Bangoria a écrit :
> So far powerpc hw supported only one watchpoint. But Future Power
> architecture is introducing 2nd DAWR. Convert thread_struct->hw_brk
> into an array.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/processor.h | 2 +-
> arch/powerpc/kernel/process.c | 61 ++++++++++++++---------
> arch/powerpc/kernel/ptrace/ptrace-noadv.c | 40 +++++++++++----
> arch/powerpc/kernel/ptrace/ptrace32.c | 4 +-
> arch/powerpc/kernel/signal.c | 9 +++-
> 5 files changed, 77 insertions(+), 39 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 90f6dbc7ff00..65b03162cd67 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -187,7 +187,7 @@ struct thread_struct {
> */
> struct perf_event *last_hit_ubp;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> - struct arch_hw_breakpoint hw_brk; /* info on the hardware breakpoint */
> + struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint info */
> unsigned long trap_nr; /* last trap # on this thread */
> u8 load_slb; /* Ages out SLB preload cache entries */
> u8 load_fp;
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index e0275fcd0c55..f5b4f21e822b 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -704,21 +704,50 @@ void switch_booke_debug_regs(struct debug_reg *new_debug)
> EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
> #else /* !CONFIG_PPC_ADV_DEBUG_REGS */
> #ifndef CONFIG_HAVE_HW_BREAKPOINT
> -static void set_breakpoint(struct arch_hw_breakpoint *brk)
> +static void set_breakpoint(struct arch_hw_breakpoint *brk, int i)
> {
> preempt_disable();
> - __set_breakpoint(brk, 0);
> + __set_breakpoint(brk, i);
> preempt_enable();
> }
>
> static void set_debug_reg_defaults(struct thread_struct *thread)
> {
> - thread->hw_brk.address = 0;
> - thread->hw_brk.type = 0;
> - thread->hw_brk.len = 0;
> - thread->hw_brk.hw_len = 0;
> - if (ppc_breakpoint_available())
> - set_breakpoint(&thread->hw_brk);
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
Maybe you could add the following that you added other places:
struct arch_hw_breakpoint null_brk = {0};
Then do
thread->hw_brk[i] = null_brk;
> + thread->hw_brk[i].address = 0;
> + thread->hw_brk[i].type = 0;
> + thread->hw_brk[i].len = 0;
> + thread->hw_brk[i].hw_len = 0;
> + if (ppc_breakpoint_available())
> + set_breakpoint(&thread->hw_brk[i], i);
> + }
> +}
> +
> +static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
> + struct arch_hw_breakpoint *b)
> +{
> + if (a->address != b->address)
> + return false;
> + if (a->type != b->type)
> + return false;
> + if (a->len != b->len)
> + return false;
> + /* no need to check hw_len. it's calculated from address and len */
> + return true;
> +}
> +
> +static void switch_hw_breakpoint(struct task_struct *new)
> +{
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[i]),
> + &new->thread.hw_brk[i]))) {
> + __set_breakpoint(&new->thread.hw_brk[i], i);
> + }
Or could be:
if (likely(hw_brk_match(this_cpu_ptr(¤t_brk[i]),
&new->thread.hw_brk[i])))
continue;
__set_breakpoint(&new->thread.hw_brk[i], i);
> + }
> }
> #endif /* !CONFIG_HAVE_HW_BREAKPOINT */
> #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
> @@ -822,19 +851,6 @@ bool ppc_breakpoint_available(void)
> }
> EXPORT_SYMBOL_GPL(ppc_breakpoint_available);
>
> -static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
> - struct arch_hw_breakpoint *b)
> -{
> - if (a->address != b->address)
> - return false;
> - if (a->type != b->type)
> - return false;
> - if (a->len != b->len)
> - return false;
> - /* no need to check hw_len. it's calculated from address and len */
> - return true;
> -}
> -
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>
> static inline bool tm_enabled(struct task_struct *tsk)
> @@ -1167,8 +1183,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
> * schedule DABR
> */
> #ifndef CONFIG_HAVE_HW_BREAKPOINT
> - if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[0]), &new->thread.hw_brk)))
> - __set_breakpoint(&new->thread.hw_brk, 0);
> + switch_hw_breakpoint(new);
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> #endif
>
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> index 12962302d6a4..0dbb35392dd2 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> @@ -67,11 +67,16 @@ int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> /* We only support one DABR and no IABRS at the moment */
> if (addr > 0)
> return -EINVAL;
> - dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
> - (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> + dabr_fake = ((child->thread.hw_brk[0].address & (~HW_BRK_TYPE_DABR)) |
> + (child->thread.hw_brk[0].type & HW_BRK_TYPE_DABR));
> return put_user(dabr_fake, datalp);
> }
>
> +/*
> + * ptrace_set_debugreg() fakes DABR and DABR is only one. So even if
> + * internal hw supports more than one watchpoint, we support only one
> + * watchpoint with this interface.
> + */
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned long data)
> {
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> @@ -137,7 +142,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned l
> return ret;
>
> thread->ptrace_bps[0] = bp;
> - thread->hw_brk = hw_brk;
> + thread->hw_brk[0] = hw_brk;
> return 0;
> }
>
> @@ -159,12 +164,24 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned l
> if (set_bp && (!ppc_breakpoint_available()))
> return -ENODEV;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> - task->thread.hw_brk = hw_brk;
> + task->thread.hw_brk[0] = hw_brk;
> return 0;
> }
>
> +static int find_empty_hw_brk(struct thread_struct *thread)
> +{
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (!thread->hw_brk[i].address)
> + return i;
> + }
> + return -1;
> +}
> +
> long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_info)
> {
> + int i;
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> int len = 0;
> struct thread_struct *thread = &child->thread;
> @@ -223,15 +240,16 @@ long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_inf
> if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
> return -EINVAL;
>
> - if (child->thread.hw_brk.address)
> + i = find_empty_hw_brk(&child->thread);
> + if (i < 0)
> return -ENOSPC;
>
> if (!ppc_breakpoint_available())
> return -ENODEV;
>
> - child->thread.hw_brk = brk;
> + child->thread.hw_brk[i] = brk;
>
> - return 1;
> + return i + 1;
> }
>
> long ppc_del_hwdebug(struct task_struct *child, long data)
> @@ -241,7 +259,7 @@ long ppc_del_hwdebug(struct task_struct *child, long data)
> struct thread_struct *thread = &child->thread;
> struct perf_event *bp;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> - if (data != 1)
> + if (data < 1 || data > nr_wp_slots())
> return -EINVAL;
>
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> @@ -254,11 +272,11 @@ long ppc_del_hwdebug(struct task_struct *child, long data)
> }
> return ret;
> #else /* CONFIG_HAVE_HW_BREAKPOINT */
> - if (child->thread.hw_brk.address == 0)
> + if (child->thread.hw_brk[data - 1].address == 0)
> return -ENOENT;
>
> - child->thread.hw_brk.address = 0;
> - child->thread.hw_brk.type = 0;
> + child->thread.hw_brk[data - 1].address = 0;
> + child->thread.hw_brk[data - 1].type = 0;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>
> return 0;
> diff --git a/arch/powerpc/kernel/ptrace/ptrace32.c b/arch/powerpc/kernel/ptrace/ptrace32.c
> index 7976ddf29c0e..7589a9665ffb 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace32.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace32.c
> @@ -259,8 +259,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
> ret = put_user(child->thread.debug.dac1, (u32 __user *)data);
> #else
> dabr_fake = (
> - (child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
> - (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> + (child->thread.hw_brk[0].address & (~HW_BRK_TYPE_DABR)) |
> + (child->thread.hw_brk[0].type & HW_BRK_TYPE_DABR));
> ret = put_user(dabr_fake, (u32 __user *)data);
> #endif
> break;
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index bbf237f072d4..b559b114d03d 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -107,6 +107,9 @@ static void do_signal(struct task_struct *tsk)
> struct ksignal ksig = { .sig = 0 };
> int ret;
> int is32 = is_32bit_task();
> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> + int i;
> +#endif >
> BUG_ON(tsk != current);
>
> @@ -128,8 +131,10 @@ static void do_signal(struct task_struct *tsk)
> * user space. The DABR will have been cleared if it
> * triggered inside the kernel.
> */
> - if (tsk->thread.hw_brk.address && tsk->thread.hw_brk.type)
> - __set_breakpoint(&tsk->thread.hw_brk, 0);
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (tsk->thread.hw_brk[i].address && tsk->thread.hw_brk[i].type)
> + __set_breakpoint(&tsk->thread.hw_brk[i], i);
> + }
thread.hwbrk also exists when CONFIG_PPC_ADV_DEBUG_REGS is selected.
You could replace the #ifndef CONFIG_PPC_ADV_DEBUG_REGS by an if
(!IS_ENABLED(CONFIG_PPC_ADV_DEBUG_REGS)) and then no need of an ifdef
when declaring the int i;
> #endif
> /* Re-enable the breakpoints for the signal stack */
> thread_change_pc(tsk, tsk->thread.regs);
>
Christophe
^ permalink raw reply
* Re: [PATCH v2 13/16] powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint
From: Christophe Leroy @ 2020-04-01 6:50 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200401061309.92442-14-ravi.bangoria@linux.ibm.com>
Le 01/04/2020 à 08:13, Ravi Bangoria a écrit :
> Currently we assume that we have only one watchpoint supported by hw.
> Get rid of that assumption and use dynamic loop instead. This should
> make supporting more watchpoints very easy.
>
> With more than one watchpoint, exception handler need to know which
> DAWR caused the exception, and hw currently does not provide it. So
> we need sw logic for the same. To figure out which DAWR caused the
> exception, check all different combinations of user specified range,
> dawr address range, actual access range and dawrx constrains. For ex,
> if user specified range and actual access range overlaps but dawrx is
> configured for readonly watchpoint and the instruction is store, this
> DAWR must not have caused exception.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/processor.h | 2 +-
> arch/powerpc/include/asm/sstep.h | 2 +
> arch/powerpc/kernel/hw_breakpoint.c | 396 +++++++++++++++++++++------
> arch/powerpc/kernel/process.c | 3 -
> 4 files changed, 313 insertions(+), 90 deletions(-)
>
[...]
> -static bool
> -dar_range_overlaps(unsigned long dar, int size, struct arch_hw_breakpoint *info)
> +static bool dar_user_range_overlaps(unsigned long dar, int size,
> + struct arch_hw_breakpoint *info)
> {
> return ((dar <= info->address + info->len - 1) &&
> (dar + size - 1 >= info->address));
> }
Here and several other places, I think it would be more clear if you
could avoid the - 1 :
return ((dar < info->address + info->len) &&
(dar + size > info->address));
>
> +static bool dar_in_hw_range(unsigned long dar, struct arch_hw_breakpoint *info)
> +{
> + unsigned long hw_start_addr, hw_end_addr;
> +
> + hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
> + hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE) - 1;
> +
> + return ((hw_start_addr <= dar) && (hw_end_addr >= dar));
> +}
hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE);
return ((hw_start_addr <= dar) && (hw_end_addr > dar));
Christophe
> +
> +static bool dar_hw_range_overlaps(unsigned long dar, int size,
> + struct arch_hw_breakpoint *info)
> +{
> + unsigned long hw_start_addr, hw_end_addr;
> +
> + hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
> + hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE) - 1;
> +
> + return ((dar <= hw_end_addr) && (dar + size - 1 >= hw_start_addr));
> +}
Same
Christophe
^ 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