LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 6/6] memory-hotplug.txt: Add some details about locking internals
From: David Hildenbrand @ 2018-09-25  9:14 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, David Hildenbrand, Jonathan Corbet, Michal Hocko,
	Andrew Morton
In-Reply-To: <20180925091457.28651-1-david@redhat.com>

Let's document the magic a bit, especially why device_hotplug_lock is
required when adding/removing memory and how it all play together with
requests to online/offline memory from user space.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 Documentation/memory-hotplug.txt | 42 +++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/memory-hotplug.txt b/Documentation/memory-hotplug.txt
index 7f49ebf3ddb2..ce4faa5530fa 100644
--- a/Documentation/memory-hotplug.txt
+++ b/Documentation/memory-hotplug.txt
@@ -3,7 +3,7 @@ Memory Hotplug
 ==============
 
 :Created:							Jul 28 2007
-:Updated: Add description of notifier of memory hotplug:	Oct 11 2007
+:Updated: Add some details about locking internals:		Aug 20 2018
 
 This document is about memory hotplug including how-to-use and current status.
 Because Memory Hotplug is still under development, contents of this text will
@@ -495,6 +495,46 @@ further processing of the notification queue.
 
 NOTIFY_STOP stops further processing of the notification queue.
 
+
+Locking Internals
+=================
+
+When adding/removing memory that uses memory block devices (i.e. ordinary RAM),
+the device_hotplug_lock should be held to:
+
+- synchronize against online/offline requests (e.g. via sysfs). This way, memory
+  block devices can only be accessed (.online/.state attributes) by user
+  space once memory has been fully added. And when removing memory, we
+  know nobody is in critical sections.
+- synchronize against CPU hotplug and similar (e.g. relevant for ACPI and PPC)
+
+Especially, there is a possible lock inversion that is avoided using
+device_hotplug_lock when adding memory and user space tries to online that
+memory faster than expected:
+
+- device_online() will first take the device_lock(), followed by
+  mem_hotplug_lock
+- add_memory_resource() will first take the mem_hotplug_lock, followed by
+  the device_lock() (while creating the devices, during bus_add_device()).
+
+As the device is visible to user space before taking the device_lock(), this
+can result in a lock inversion.
+
+onlining/offlining of memory should be done via device_online()/
+device_offline() - to make sure it is properly synchronized to actions
+via sysfs. Holding device_hotplug_lock is advised (to e.g. protect online_type)
+
+When adding/removing/onlining/offlining memory or adding/removing
+heterogeneous/device memory, we should always hold the mem_hotplug_lock in
+write mode to serialise memory hotplug (e.g. access to global/zone
+variables).
+
+In addition, mem_hotplug_lock (in contrast to device_hotplug_lock) in read
+mode allows for a quite efficient get_online_mems/put_online_mems
+implementation, so code accessing memory can protect from that memory
+vanishing.
+
+
 Future Work
 ===========
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 5/6] powerpc/powernv: hold device_hotplug_lock when calling memtrace_offline_pages()
From: David Hildenbrand @ 2018-09-25  9:14 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, David Hildenbrand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Rashmica Gupta, Balbir Singh, Michael Neuling
In-Reply-To: <20180925091457.28651-1-david@redhat.com>

Let's perform all checking + offlining + removing under
device_hotplug_lock, so nobody can mess with these devices via
sysfs concurrently.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Rashmica Gupta <rashmica.g@gmail.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Michael Neuling <mikey@neuling.org>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/powerpc/platforms/powernv/memtrace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index fdd48f1a39f7..d84d09c56af9 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -70,6 +70,7 @@ static int change_memblock_state(struct memory_block *mem, void *arg)
 	return 0;
 }
 
+/* called with device_hotplug_lock held */
 static bool memtrace_offline_pages(u32 nid, u64 start_pfn, u64 nr_pages)
 {
 	u64 end_pfn = start_pfn + nr_pages - 1;
@@ -111,6 +112,7 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
 	end_pfn = round_down(end_pfn - nr_pages, nr_pages);
 
 	for (base_pfn = end_pfn; base_pfn > start_pfn; base_pfn -= nr_pages) {
+		lock_device_hotplug();
 		if (memtrace_offline_pages(nid, base_pfn, nr_pages) == true) {
 			/*
 			 * Remove memory in memory block size chunks so that
@@ -118,7 +120,6 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
 			 * we never try to remove memory that spans two iomem
 			 * resources.
 			 */
-			lock_device_hotplug();
 			end_pfn = base_pfn + nr_pages;
 			for (pfn = base_pfn; pfn < end_pfn; pfn += bytes>> PAGE_SHIFT) {
 				__remove_memory(nid, pfn << PAGE_SHIFT, bytes);
@@ -126,6 +127,7 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
 			unlock_device_hotplug();
 			return base_pfn << PAGE_SHIFT;
 		}
+		unlock_device_hotplug();
 	}
 
 	return 0;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 4/6] powerpc/powernv: hold device_hotplug_lock when calling device_online()
From: David Hildenbrand @ 2018-09-25  9:14 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, David Hildenbrand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Rashmica Gupta, Balbir Singh, Michael Neuling
In-Reply-To: <20180925091457.28651-1-david@redhat.com>

device_online() should be called with device_hotplug_lock() held.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Rashmica Gupta <rashmica.g@gmail.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Michael Neuling <mikey@neuling.org>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/powerpc/platforms/powernv/memtrace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index 773623f6bfb1..fdd48f1a39f7 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -242,9 +242,11 @@ static int memtrace_online(void)
 		 * we need to online the memory ourselves.
 		 */
 		if (!memhp_auto_online) {
+			lock_device_hotplug();
 			walk_memory_range(PFN_DOWN(ent->start),
 					  PFN_UP(ent->start + ent->size - 1),
 					  NULL, online_mem_block);
+			unlock_device_hotplug();
 		}
 
 		/*
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 3/6] mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock
From: David Hildenbrand @ 2018-09-25  9:14 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, David Hildenbrand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Rafael J. Wysocki, Len Brown,
	Greg Kroah-Hartman, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Martin Schwidefsky, Heiko Carstens,
	Boris Ostrovsky, Juergen Gross, Rashmica Gupta, Michael Neuling,
	Balbir Singh, Kate Stewart, Thomas Gleixner, Philippe Ombredanne,
	Andrew Morton, Michal Hocko, Pavel Tatashin, Vlastimil Babka,
	Dan Williams, Oscar Salvador, YASUAKI ISHIMATSU,
	Mathieu Malaterre
In-Reply-To: <20180925091457.28651-1-david@redhat.com>

There seem to be some problems as result of 30467e0b3be ("mm, hotplug:
fix concurrent memory hot-add deadlock"), which tried to fix a possible
lock inversion reported and discussed in [1] due to the two locks
	a) device_lock()
	b) mem_hotplug_lock

While add_memory() first takes b), followed by a) during
bus_probe_device(), onlining of memory from user space first took a),
followed by b), exposing a possible deadlock.

In [1], and it was decided to not make use of device_hotplug_lock, but
rather to enforce a locking order.

The problems I spotted related to this:

1. Memory block device attributes: While .state first calls
   mem_hotplug_begin() and the calls device_online() - which takes
   device_lock() - .online does no longer call mem_hotplug_begin(), so
   effectively calls online_pages() without mem_hotplug_lock.

2. device_online() should be called under device_hotplug_lock, however
   onlining memory during add_memory() does not take care of that.

In addition, I think there is also something wrong about the locking in

3. arch/powerpc/platforms/powernv/memtrace.c calls offline_pages()
   without locks. This was introduced after 30467e0b3be. And skimming over
   the code, I assume it could need some more care in regards to locking
   (e.g. device_online() called without device_hotplug_lock. This will
   be addressed in the following patches.

Now that we hold the device_hotplug_lock when
- adding memory (e.g. via add_memory()/add_memory_resource())
- removing memory (e.g. via remove_memory())
- device_online()/device_offline()

We can move mem_hotplug_lock usage back into
online_pages()/offline_pages().

Why is mem_hotplug_lock still needed? Essentially to make
get_online_mems()/put_online_mems() be very fast (relying on
device_hotplug_lock would be very slow), and to serialize against
addition of memory that does not create memory block devices (hmm).

[1] http://driverdev.linuxdriverproject.org/pipermail/ driverdev-devel/
    2015-February/065324.html

This patch is partly based on a patch by Vitaly Kuznetsov.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Rashmica Gupta <rashmica.g@gmail.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Pavel Tatashin <pavel.tatashin@microsoft.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>
Cc: Mathieu Malaterre <malat@debian.org>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/base/memory.c | 13 +------------
 mm/memory_hotplug.c   | 28 ++++++++++++++++++++--------
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 40cac122ec73..0e5985682642 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -228,7 +228,6 @@ static bool pages_correctly_probed(unsigned long start_pfn)
 /*
  * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is
  * OK to have direct references to sparsemem variables in here.
- * Must already be protected by mem_hotplug_begin().
  */
 static int
 memory_block_action(unsigned long phys_index, unsigned long action, int online_type)
@@ -294,7 +293,6 @@ static int memory_subsys_online(struct device *dev)
 	if (mem->online_type < 0)
 		mem->online_type = MMOP_ONLINE_KEEP;
 
-	/* Already under protection of mem_hotplug_begin() */
 	ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
 
 	/* clear online_type */
@@ -341,19 +339,11 @@ store_mem_state(struct device *dev,
 		goto err;
 	}
 
-	/*
-	 * Memory hotplug needs to hold mem_hotplug_begin() for probe to find
-	 * the correct memory block to online before doing device_online(dev),
-	 * which will take dev->mutex.  Take the lock early to prevent an
-	 * inversion, memory_subsys_online() callbacks will be implemented by
-	 * assuming it's already protected.
-	 */
-	mem_hotplug_begin();
-
 	switch (online_type) {
 	case MMOP_ONLINE_KERNEL:
 	case MMOP_ONLINE_MOVABLE:
 	case MMOP_ONLINE_KEEP:
+		/* mem->online_type is protected by device_hotplug_lock */
 		mem->online_type = online_type;
 		ret = device_online(&mem->dev);
 		break;
@@ -364,7 +354,6 @@ store_mem_state(struct device *dev,
 		ret = -EINVAL; /* should never happen */
 	}
 
-	mem_hotplug_done();
 err:
 	unlock_device_hotplug();
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index affb03e0dfef..d4c7e42e46f3 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -860,7 +860,6 @@ static struct zone * __meminit move_pfn_range(int online_type, int nid,
 	return zone;
 }
 
-/* Must be protected by mem_hotplug_begin() or a device_lock */
 int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_type)
 {
 	unsigned long flags;
@@ -872,6 +871,8 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
 	struct memory_notify arg;
 	struct memory_block *mem;
 
+	mem_hotplug_begin();
+
 	/*
 	 * We can't use pfn_to_nid() because nid might be stored in struct page
 	 * which is not yet initialized. Instead, we find nid from memory block.
@@ -936,6 +937,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
 
 	if (onlined_pages)
 		memory_notify(MEM_ONLINE, &arg);
+	mem_hotplug_done();
 	return 0;
 
 failed_addition:
@@ -943,6 +945,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
 		 (unsigned long long) pfn << PAGE_SHIFT,
 		 (((unsigned long long) pfn + nr_pages) << PAGE_SHIFT) - 1);
 	memory_notify(MEM_CANCEL_ONLINE, &arg);
+	mem_hotplug_done();
 	return ret;
 }
 #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
@@ -1147,20 +1150,20 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
 	/* create new memmap entry */
 	firmware_map_add_hotplug(start, start + size, "System RAM");
 
+	/* device_online() will take the lock when calling online_pages() */
+	mem_hotplug_done();
+
 	/* online pages if requested */
 	if (online)
 		walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1),
 				  NULL, online_memory_block);
 
-	goto out;
-
+	return ret;
 error:
 	/* rollback pgdat allocation and others */
 	if (new_node)
 		rollback_node_hotadd(nid);
 	memblock_remove(start, size);
-
-out:
 	mem_hotplug_done();
 	return ret;
 }
@@ -1588,10 +1591,16 @@ static int __ref __offline_pages(unsigned long start_pfn,
 		return -EINVAL;
 	if (!IS_ALIGNED(end_pfn, pageblock_nr_pages))
 		return -EINVAL;
+
+	mem_hotplug_begin();
+
 	/* This makes hotplug much easier...and readable.
 	   we assume this for now. .*/
-	if (!test_pages_in_a_zone(start_pfn, end_pfn, &valid_start, &valid_end))
+	if (!test_pages_in_a_zone(start_pfn, end_pfn, &valid_start,
+				  &valid_end)) {
+		mem_hotplug_done();
 		return -EINVAL;
+	}
 
 	zone = page_zone(pfn_to_page(valid_start));
 	node = zone_to_nid(zone);
@@ -1600,8 +1609,10 @@ static int __ref __offline_pages(unsigned long start_pfn,
 	/* set above range as isolated */
 	ret = start_isolate_page_range(start_pfn, end_pfn,
 				       MIGRATE_MOVABLE, true);
-	if (ret)
+	if (ret) {
+		mem_hotplug_done();
 		return ret;
+	}
 
 	arg.start_pfn = start_pfn;
 	arg.nr_pages = nr_pages;
@@ -1672,6 +1683,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
 	writeback_set_ratelimit();
 
 	memory_notify(MEM_OFFLINE, &arg);
+	mem_hotplug_done();
 	return 0;
 
 failed_removal:
@@ -1681,10 +1693,10 @@ static int __ref __offline_pages(unsigned long start_pfn,
 	memory_notify(MEM_CANCEL_OFFLINE, &arg);
 	/* pushback to free area */
 	undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE);
+	mem_hotplug_done();
 	return ret;
 }
 
-/* Must be protected by mem_hotplug_begin() or a device_lock */
 int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
 {
 	return __offline_pages(start_pfn, start_pfn + nr_pages);
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 2/6] mm/memory_hotplug: make add_memory() take the device_hotplug_lock
From: David Hildenbrand @ 2018-09-25  9:14 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, David Hildenbrand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Rafael J. Wysocki, Len Brown,
	Greg Kroah-Hartman, Boris Ostrovsky, Juergen Gross,
	Nathan Fontenot, John Allen, Andrew Morton, Michal Hocko,
	Dan Williams, Joonsoo Kim, Vlastimil Babka, Oscar Salvador,
	Mathieu Malaterre, Pavel Tatashin, YASUAKI ISHIMATSU
In-Reply-To: <20180925091457.28651-1-david@redhat.com>

add_memory() currently does not take the device_hotplug_lock, however
is aleady called under the lock from
	arch/powerpc/platforms/pseries/hotplug-memory.c
	drivers/acpi/acpi_memhotplug.c
to synchronize against CPU hot-remove and similar.

In general, we should hold the device_hotplug_lock when adding memory
to synchronize against online/offline request (e.g. from user space) -
which already resulted in lock inversions due to device_lock() and
mem_hotplug_lock - see 30467e0b3be ("mm, hotplug: fix concurrent memory
hot-add deadlock"). add_memory()/add_memory_resource() will create memory
block devices, so this really feels like the right thing to do.

Holding the device_hotplug_lock makes sure that a memory block device
can really only be accessed (e.g. via .online/.state) from user space,
once the memory has been fully added to the system.

The lock is not held yet in
	drivers/xen/balloon.c
	arch/powerpc/platforms/powernv/memtrace.c
	drivers/s390/char/sclp_cmd.c
	drivers/hv/hv_balloon.c
So, let's either use the locked variants or take the lock.

Don't export add_memory_resource(), as it once was exported to be used
by XEN, which is never built as a module. If somebody requires it, we
also have to export a locked variant (as device_hotplug_lock is never
exported).

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: John Allen <jallen@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Mathieu Malaterre <malat@debian.org>
Cc: Pavel Tatashin <pavel.tatashin@microsoft.com>
Cc: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 .../platforms/pseries/hotplug-memory.c        |  2 +-
 drivers/acpi/acpi_memhotplug.c                |  2 +-
 drivers/base/memory.c                         |  9 ++++++--
 drivers/xen/balloon.c                         |  3 +++
 include/linux/memory_hotplug.h                |  1 +
 mm/memory_hotplug.c                           | 22 ++++++++++++++++---
 6 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index dd0264c43f3e..d26a771d985e 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -673,7 +673,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 	nid = memory_add_physaddr_to_nid(lmb->base_addr);
 
 	/* Add the memory */
-	rc = add_memory(nid, lmb->base_addr, block_sz);
+	rc = __add_memory(nid, lmb->base_addr, block_sz);
 	if (rc) {
 		invalidate_lmb_associativity_index(lmb);
 		return rc;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 811148415993..8fe0960ea572 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -228,7 +228,7 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
 		if (node < 0)
 			node = memory_add_physaddr_to_nid(info->start_addr);
 
-		result = add_memory(node, info->start_addr, info->length);
+		result = __add_memory(node, info->start_addr, info->length);
 
 		/*
 		 * If the memory block has been used by the kernel, add_memory()
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 817320c7c4c1..40cac122ec73 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -519,15 +519,20 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
 	if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
 		return -EINVAL;
 
+	ret = lock_device_hotplug_sysfs();
+	if (ret)
+		goto out;
+
 	nid = memory_add_physaddr_to_nid(phys_addr);
-	ret = add_memory(nid, phys_addr,
-			 MIN_MEMORY_BLOCK_SIZE * sections_per_block);
+	ret = __add_memory(nid, phys_addr,
+			   MIN_MEMORY_BLOCK_SIZE * sections_per_block);
 
 	if (ret)
 		goto out;
 
 	ret = count;
 out:
+	unlock_device_hotplug();
 	return ret;
 }
 
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index a3f5cbfcd4a1..fdfc64f5acea 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -395,7 +395,10 @@ static enum bp_state reserve_additional_memory(void)
 	 * callers drop the mutex before trying again.
 	 */
 	mutex_unlock(&balloon_mutex);
+	/* add_memory_resource() requires the device_hotplug lock */
+	lock_device_hotplug();
 	rc = add_memory_resource(nid, resource, memhp_auto_online);
+	unlock_device_hotplug();
 	mutex_lock(&balloon_mutex);
 
 	if (rc) {
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 1f096852f479..ffd9cd10fcf3 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -324,6 +324,7 @@ static inline void __remove_memory(int nid, u64 start, u64 size) {}
 extern void __ref free_area_init_core_hotplug(int nid);
 extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
 		void *arg, int (*func)(struct memory_block *, void *));
+extern int __add_memory(int nid, u64 start, u64 size);
 extern int add_memory(int nid, u64 start, u64 size);
 extern int add_memory_resource(int nid, struct resource *resource, bool online);
 extern int arch_add_memory(int nid, u64 start, u64 size,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index f6dbd5d8fffd..affb03e0dfef 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1090,7 +1090,12 @@ static int online_memory_block(struct memory_block *mem, void *arg)
 	return device_online(&mem->dev);
 }
 
-/* we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG */
+/*
+ * NOTE: The caller must call lock_device_hotplug() to serialize hotplug
+ * and online/offline operations (triggered e.g. by sysfs).
+ *
+ * we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG
+ */
 int __ref add_memory_resource(int nid, struct resource *res, bool online)
 {
 	u64 start, size;
@@ -1159,9 +1164,9 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
 	mem_hotplug_done();
 	return ret;
 }
-EXPORT_SYMBOL_GPL(add_memory_resource);
 
-int __ref add_memory(int nid, u64 start, u64 size)
+/* requires device_hotplug_lock, see add_memory_resource() */
+int __ref __add_memory(int nid, u64 start, u64 size)
 {
 	struct resource *res;
 	int ret;
@@ -1175,6 +1180,17 @@ int __ref add_memory(int nid, u64 start, u64 size)
 		release_memory_resource(res);
 	return ret;
 }
+
+int add_memory(int nid, u64 start, u64 size)
+{
+	int rc;
+
+	lock_device_hotplug();
+	rc = __add_memory(nid, start, size);
+	unlock_device_hotplug();
+
+	return rc;
+}
 EXPORT_SYMBOL_GPL(add_memory);
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 1/6] mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
From: David Hildenbrand @ 2018-09-25  9:14 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, David Hildenbrand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Rafael J. Wysocki, Len Brown, Rashmica Gupta,
	Michael Neuling, Balbir Singh, Nathan Fontenot, John Allen,
	Andrew Morton, Michal Hocko, Dan Williams, Joonsoo Kim,
	Vlastimil Babka, Pavel Tatashin, Greg Kroah-Hartman,
	Oscar Salvador, YASUAKI ISHIMATSU, Mathieu Malaterre
In-Reply-To: <20180925091457.28651-1-david@redhat.com>

remove_memory() is exported right now but requires the
device_hotplug_lock, which is not exported. So let's provide a variant
that takes the lock and only export that one.

The lock is already held in
	arch/powerpc/platforms/pseries/hotplug-memory.c
	drivers/acpi/acpi_memhotplug.c
	arch/powerpc/platforms/powernv/memtrace.c

Apart from that, there are not other users in the tree.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Rashmica Gupta <rashmica.g@gmail.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: John Allen <jallen@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>
Cc: Mathieu Malaterre <malat@debian.org>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/powerpc/platforms/powernv/memtrace.c       | 2 +-
 arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++---
 drivers/acpi/acpi_memhotplug.c                  | 2 +-
 include/linux/memory_hotplug.h                  | 3 ++-
 mm/memory_hotplug.c                             | 9 ++++++++-
 5 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index a29fdf8a2e56..773623f6bfb1 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -121,7 +121,7 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
 			lock_device_hotplug();
 			end_pfn = base_pfn + nr_pages;
 			for (pfn = base_pfn; pfn < end_pfn; pfn += bytes>> PAGE_SHIFT) {
-				remove_memory(nid, pfn << PAGE_SHIFT, bytes);
+				__remove_memory(nid, pfn << PAGE_SHIFT, bytes);
 			}
 			unlock_device_hotplug();
 			return base_pfn << PAGE_SHIFT;
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 9a15d39995e5..dd0264c43f3e 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -305,7 +305,7 @@ static int pseries_remove_memblock(unsigned long base, unsigned int memblock_siz
 	nid = memory_add_physaddr_to_nid(base);
 
 	for (i = 0; i < sections_per_block; i++) {
-		remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
+		__remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
 		base += MIN_MEMORY_BLOCK_SIZE;
 	}
 
@@ -394,7 +394,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
 	block_sz = pseries_memory_block_size();
 	nid = memory_add_physaddr_to_nid(lmb->base_addr);
 
-	remove_memory(nid, lmb->base_addr, block_sz);
+	__remove_memory(nid, lmb->base_addr, block_sz);
 
 	/* Update memory regions for memory remove */
 	memblock_remove(lmb->base_addr, block_sz);
@@ -681,7 +681,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 
 	rc = dlpar_online_lmb(lmb);
 	if (rc) {
-		remove_memory(nid, lmb->base_addr, block_sz);
+		__remove_memory(nid, lmb->base_addr, block_sz);
 		invalidate_lmb_associativity_index(lmb);
 	} else {
 		lmb->flags |= DRCONF_MEM_ASSIGNED;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 6b0d3ef7309c..811148415993 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -282,7 +282,7 @@ static void acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
 			nid = memory_add_physaddr_to_nid(info->start_addr);
 
 		acpi_unbind_memory_blocks(info);
-		remove_memory(nid, info->start_addr, info->length);
+		__remove_memory(nid, info->start_addr, info->length);
 		list_del(&info->list);
 		kfree(info);
 	}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 34a28227068d..1f096852f479 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -301,6 +301,7 @@ extern bool is_mem_section_removable(unsigned long pfn, unsigned long nr_pages);
 extern void try_offline_node(int nid);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
 extern void remove_memory(int nid, u64 start, u64 size);
+extern void __remove_memory(int nid, u64 start, u64 size);
 
 #else
 static inline bool is_mem_section_removable(unsigned long pfn,
@@ -317,6 +318,7 @@ static inline int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
 }
 
 static inline void remove_memory(int nid, u64 start, u64 size) {}
+static inline void __remove_memory(int nid, u64 start, u64 size) {}
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 
 extern void __ref free_area_init_core_hotplug(int nid);
@@ -330,7 +332,6 @@ extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 		unsigned long nr_pages, struct vmem_altmap *altmap);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
 extern bool is_memblock_offlined(struct memory_block *mem);
-extern void remove_memory(int nid, u64 start, u64 size);
 extern int sparse_add_one_section(struct pglist_data *pgdat,
 		unsigned long start_pfn, struct vmem_altmap *altmap);
 extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 2698664bfd54..f6dbd5d8fffd 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1839,7 +1839,7 @@ EXPORT_SYMBOL(try_offline_node);
  * and online/offline operations before this call, as required by
  * try_offline_node().
  */
-void __ref remove_memory(int nid, u64 start, u64 size)
+void __ref __remove_memory(int nid, u64 start, u64 size)
 {
 	int ret;
 
@@ -1868,5 +1868,12 @@ void __ref remove_memory(int nid, u64 start, u64 size)
 
 	mem_hotplug_done();
 }
+
+void remove_memory(int nid, u64 start, u64 size)
+{
+	lock_device_hotplug();
+	__remove_memory(nid, start, size);
+	unlock_device_hotplug();
+}
 EXPORT_SYMBOL_GPL(remove_memory);
 #endif /* CONFIG_MEMORY_HOTREMOVE */
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock
From: David Hildenbrand @ 2018-09-25  9:14 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, David Hildenbrand, Andrew Morton, Balbir Singh,
	Benjamin Herrenschmidt, Boris Ostrovsky, Dan Williams,
	Greg Kroah-Hartman, Haiyang Zhang, Heiko Carstens, John Allen,
	Jonathan Corbet, Joonsoo Kim, Juergen Gross, Kate Stewart,
	K. Y. Srinivasan, Len Brown, Martin Schwidefsky,
	Mathieu Malaterre, Michael Ellerman, Michael Neuling,
	Michal Hocko, Nathan Fontenot, Oscar Salvador, Paul Mackerras,
	Pavel Tatashin, Pavel Tatashin, Philippe Ombredanne,
	Rafael J. Wysocki, Rafael J. Wysocki, Rashmica Gupta,
	Stephen Hemminger, Thomas Gleixner, Vlastimil Babka,
	YASUAKI ISHIMATSU

Reading through the code and studying how mem_hotplug_lock is to be used,
I noticed that there are two places where we can end up calling
device_online()/device_offline() - online_pages()/offline_pages() without
the mem_hotplug_lock. And there are other places where we call
device_online()/device_offline() without the device_hotplug_lock.

While e.g.
	echo "online" > /sys/devices/system/memory/memory9/state
is fine, e.g.
	echo 1 > /sys/devices/system/memory/memory9/online
Will not take the mem_hotplug_lock. However the device_lock() and
device_hotplug_lock.

E.g. via memory_probe_store(), we can end up calling
add_memory()->online_pages() without the device_hotplug_lock. So we can
have concurrent callers in online_pages(). We e.g. touch in online_pages()
basically unprotected zone->present_pages then.

Looks like there is a longer history to that (see Patch #2 for details),
and fixing it to work the way it was intended is not really possible. We
would e.g. have to take the mem_hotplug_lock in device/base/core.c, which
sounds wrong.

Summary: We had a lock inversion on mem_hotplug_lock and device_lock().
More details can be found in patch 3 and patch 6.

I propose the general rules (documentation added in patch 6):

1. add_memory/add_memory_resource() must only be called with
   device_hotplug_lock.
2. remove_memory() must only be called with device_hotplug_lock. This is
   already documented and holds for all callers.
3. device_online()/device_offline() must only be called with
   device_hotplug_lock. This is already documented and true for now in core
   code. Other callers (related to memory hotplug) have to be fixed up.
4. mem_hotplug_lock is taken inside of add_memory/remove_memory/
   online_pages/offline_pages.

To me, this looks way cleaner than what we have right now (and easier to
verify). And looking at the documentation of remove_memory, using
lock_device_hotplug also for add_memory() feels natural.


v1 -> v2:
- Upstream changes in powerpc/powernv code required modifications to
  patch #1, #4 and #5.
- Minor patch description changes.
- Added more locking details in patch #6.
- Added rb's

RFCv2 -> v1:
- Dropped an unnecessary _ref from remove_memory() in patch #1
- Minor patch description fixes.
- Added rb's

RFC -> RFCv2:
- Don't export device_hotplug_lock, provide proper remove_memory/add_memory
  wrappers.
- Split up the patches a bit.
- Try to improve powernv memtrace locking
- Add some documentation for locking that matches my knowledge

David Hildenbrand (6):
  mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
  mm/memory_hotplug: make add_memory() take the device_hotplug_lock
  mm/memory_hotplug: fix online/offline_pages called w.o.
    mem_hotplug_lock
  powerpc/powernv: hold device_hotplug_lock when calling device_online()
  powerpc/powernv: hold device_hotplug_lock when calling
    memtrace_offline_pages()
  memory-hotplug.txt: Add some details about locking internals

 Documentation/memory-hotplug.txt              | 42 ++++++++++++-
 arch/powerpc/platforms/powernv/memtrace.c     |  8 ++-
 .../platforms/pseries/hotplug-memory.c        |  8 +--
 drivers/acpi/acpi_memhotplug.c                |  4 +-
 drivers/base/memory.c                         | 22 +++----
 drivers/xen/balloon.c                         |  3 +
 include/linux/memory_hotplug.h                |  4 +-
 mm/memory_hotplug.c                           | 59 +++++++++++++++----
 8 files changed, 114 insertions(+), 36 deletions(-)

-- 
2.17.1

^ permalink raw reply

* Re: [PATCH 0/7 v7] Support for fsl-mc bus and its devices in SMMU
From: Joerg Roedel @ 2018-09-25  7:48 UTC (permalink / raw)
  To: Nipun Gupta
  Cc: robin.murphy, will.deacon, robh+dt, robh, mark.rutland,
	catalin.marinas, gregkh, laurentiu.tudor, bhelgaas, hch,
	m.szyprowski, shawnguo, frowand.list, iommu, linux-kernel,
	devicetree, linux-arm-kernel, linuxppc-dev, linux-pci,
	bharat.bhushan, stuyoder, leoyang.li
In-Reply-To: <1536587361-11047-1-git-send-email-nipun.gupta@nxp.com>

On Mon, Sep 10, 2018 at 07:19:14PM +0530, Nipun Gupta wrote:
> Nipun Gupta (7):
>   Documentation: fsl-mc: add iommu-map device-tree binding for fsl-mc
>     bus
>   iommu/of: make of_pci_map_rid() available for other devices too
>   iommu/of: support iommu configuration for fsl-mc devices
>   iommu/arm-smmu: Add support for the fsl-mc bus
>   bus: fsl-mc: support dma configure for devices on fsl-mc bus
>   bus: fsl-mc: set coherent dma mask for devices on fsl-mc bus
>   arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc

Applied, thanks Nipun.

^ permalink raw reply

* Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
From: Neil Armstrong @ 2018-09-25  7:53 UTC (permalink / raw)
  To: Florian Fainelli, Corentin Labbe, Gilles.Muller, Julia.Lawall,
	agust, airlied, alexandre.torgue, alistair, benh, carlo, davem,
	galak, joabreu, khilman, maxime.ripard, michal.lkml, mpe, mporter,
	nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
  Cc: netdev, linux-kernel, dri-devel, linux-ide, linux-amlogic,
	linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <a8d06d0a-b13b-384c-ade6-b0a3b0ee61f6@gmail.com>

Hi Florian,

On 24/09/2018 21:17, Florian Fainelli wrote:
> On 09/24/2018 12:04 PM, Corentin Labbe wrote:
>> This patch convert meson stmmac glue driver to use all xxxsetbits32 functions.
>>
>> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
>> ---
>>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 56 +++++++++-------------
>>  1 file changed, 22 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> index c5979569fd60..abcf65588576 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/stmmac.h>
>> +#include <linux/setbits.h>
>>  
>>  #include "stmmac_platform.h"
>>  
>> @@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
>>  	struct clk_gate		rgmii_tx_en;
>>  };
>>  
>> -static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
>> -				    u32 mask, u32 value)
>> -{
>> -	u32 data;
>> -
>> -	data = readl(dwmac->regs + reg);
>> -	data &= ~mask;
>> -	data |= (value & mask);
>> -
>> -	writel(data, dwmac->regs + reg);
>> -}
> 
> Why not make mseon8b_dwmac_mask_bits() a wrapper around
> clrsetbits_le32() whose purpose is only to dereference dwmac->regs and
> pass it to clrsetbits_le32()? That would be far less changes to review
> and audit for correctness, same goes with every other patch in this
> series touching the meson drivers.
> 

Personally, I'll prefer dropping my custom writel_bits_relaxed() with something
more future proof (I also use it in spi-meson-spicc and ao-cec),
and I think the same for dwmac-meson8b.c

Neil

^ permalink raw reply

* Re: Not able to boot cuImage for the target board with MPC8270 processor
From: sgosavi1 @ 2018-09-25  7:02 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1535599586827-0.post@n7.nabble.com>

Hi All,

I debugged the bootwrapper code by printing messages in RAM using memcpy.
The bootwrapper code appears to be executing correctly. I ensured that the
"serial_console_init" function has executed successfully. But I am still not
able to get any of the "printf" output in the wrapper code on the serial
console. 

Can anybody suggests what else I need to look at?

Thanks,
Sachin



--
Sent from: http://linuxppc.10917.n7.nabble.com/linuxppc-dev-f3.html

^ permalink raw reply

* Re: [RFC PATCH v1 1/9] timer: fix circular header dependency
From: Christophe LEROY @ 2018-09-25  6:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	aneesh.kumar, npiggin, Thomas Gleixner, Alexander Viro,
	John Stultz, Stephen Boyd
  Cc: linux-fsdevel, linuxppc-dev, linux-kernel
In-Reply-To: <5c9530da-f5c8-e0b2-61ab-294a0da7f3fc@c-s.fr>



Le 25/09/2018 à 07:34, Christophe LEROY a écrit :
> 
> 
> Le 24/09/2018 à 17:52, Christophe Leroy a écrit :
>> When switching powerpc to CONFIG_THREAD_INFO_IN_TASK, include/sched.h
>> has to be included in asm/smp.h for the following change, in order
>> to avoid uncomplete definition of task_struct:
>>
>> -#define raw_smp_processor_id() (current_thread_info()->cpu)
>> +#define raw_smp_processor_id() (current->cpu)
>>
>> But this generates the following compilation error, due to circular
>> header dependency.
>>
>>    CC      kernel/time/alarmtimer.o
>> In file included from ./arch/powerpc/include/asm/smp.h:31,
>>                   from ./include/linux/smp.h:64,
>>                   from ./include/linux/percpu.h:7,
>>                   from ./include/linux/hrtimer.h:22,
>>                   from kernel/time/alarmtimer.c:19:
>> ./include/linux/sched.h:558:19: error: field 'dl_timer' has incomplete 
>> type
>>    struct hrtimer   dl_timer;
>>                     ^~~~~~~~
>> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has 
>> incomplete type
>>    struct hrtimer inactive_timer;
>>                   ^~~~~~~~~~~~~~
>> make[1]: *** [kernel/time/alarmtimer.o] Error 1
>> make: *** [kernel/time/alarmtimer.o] Error 2
>>
>>    CC      fs/timerfd.o
>> In file included from ./arch/powerpc/include/asm/smp.h:31,
>>                   from ./include/linux/smp.h:64,
>>                   from ./include/linux/percpu.h:7,
>>                   from ./include/linux/hrtimer.h:22,
>>                   from ./include/linux/alarmtimer.h:6,
>>                   from fs/timerfd.c:12:
>> ./include/linux/sched.h:558:19: error: field 'dl_timer' has incomplete 
>> type
>>    struct hrtimer   dl_timer;
>>                     ^~~~~~~~
>> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has 
>> incomplete type
>>    struct hrtimer inactive_timer;
>>                   ^~~~~~~~~~~~~~
>> make[1]: *** [fs/timerfd.o] Error 1
>> make: *** [fs/timerfd.o] Error 2
>>
>> This patch fixes it by including linux/hrtimer.h after linux/sched.h
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   Should it be fixed in powerpc instead ? In that case, how ? Any idea ?
> 
> 
> Looks like there are several other places where the problem occurs, so 
> it has to be fixed in the powerpc headers instead.
> 
> Seems like RISC arch faced the same issue, and fixed it the following way:
> 
> /*
>   * This is particularly ugly: it appears we can't actually get the 
> definition
>   * of task_struct here, but we need access to the CPU this task is 
> running on.
>   * Instead of using C we're using asm-offsets.h to get the current 
> processor
>   * ID.
>   */
> #define raw_smp_processor_id() (*((int*)((char*)get_current() + 
> TASK_TI_CPU)))
> 
> 
> Unless someone has a better idea, I'll fixed it that way.


Not easy. I'm stuck at the moment, because asm-offsets.h redefines a lot 
of things with the same name as in C, so when including asm-offsets.h in 
C it conflicts. Argh !. Need to fix all those duplicates ?

Christophe

> 
> Christophe
> 
> 
>>
>>   fs/timerfd.c             | 2 +-
>>   kernel/time/alarmtimer.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/timerfd.c b/fs/timerfd.c
>> index d69ad801eb80..0fc01b241382 100644
>> --- a/fs/timerfd.c
>> +++ b/fs/timerfd.c
>> @@ -9,12 +9,12 @@
>>    *
>>    */
>> -#include <linux/alarmtimer.h>
>>   #include <linux/file.h>
>>   #include <linux/poll.h>
>>   #include <linux/init.h>
>>   #include <linux/fs.h>
>>   #include <linux/sched.h>
>> +#include <linux/alarmtimer.h>
>>   #include <linux/kernel.h>
>>   #include <linux/slab.h>
>>   #include <linux/list.h>
>> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
>> index fa5de5e8de61..5fb75c9b3f06 100644
>> --- a/kernel/time/alarmtimer.c
>> +++ b/kernel/time/alarmtimer.c
>> @@ -16,11 +16,11 @@
>>    * published by the Free Software Foundation.
>>    */
>>   #include <linux/time.h>
>> -#include <linux/hrtimer.h>
>>   #include <linux/timerqueue.h>
>>   #include <linux/rtc.h>
>>   #include <linux/sched/signal.h>
>>   #include <linux/sched/debug.h>
>> +#include <linux/hrtimer.h>
>>   #include <linux/alarmtimer.h>
>>   #include <linux/mutex.h>
>>   #include <linux/platform_device.h>
>>

^ permalink raw reply

* Re: [PATCH] powerpc/pseries: Export raw per-CPU VPA data via debugfs
From: Aravinda Prasad @ 2018-09-25  6:24 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: nfont, naveen.n.rao
In-Reply-To: <87pnx35dfq.fsf@concordia.ellerman.id.au>

Hi Michael,

On Monday 24 September 2018 05:12 PM, Michael Ellerman wrote:
> Hi Aravinda,
> 
> Aravinda Prasad <aravinda@linux.vnet.ibm.com> writes:
> 
>> This patch exports the raw per-CPU VPA data via debugfs.
>> A per-CPU file is created which exports the VPA data of
>> that CPU to help debug some of the VPA related issues or
>> to analyze the per-CPU VPA related statistics.
> 
> Do we really need this in debugfs? I'm not saying we don't, but I'm also
> not really clear why we need it.

Two reasons. First, if a new stat from VPA needs to be exported to user,
then we end up with a kernel patch. By exporting the VPA data, only the
user space tool needs to be updated.

Second is that we need this if we want to debug any per-CPU statistics
like stolen time etc.

> 
> If there is a good reason for exporting it, do we really want to export
> it in distro kernels, or should it be behind a CONFIG ?

I am fine to put this behind a CONFIG.

> 
>> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
>> index d3992ce..cc12c12 100644
>> --- a/arch/powerpc/platforms/pseries/lpar.c
>> +++ b/arch/powerpc/platforms/pseries/lpar.c
>> @@ -48,6 +48,7 @@
>>  #include <asm/kexec.h>
>>  #include <asm/fadump.h>
>>  #include <asm/asm-prototypes.h>
>> +#include <asm/debugfs.h>
>>  
>>  #include "pseries.h"
>>  
>> @@ -64,6 +65,16 @@ EXPORT_SYMBOL(plpar_hcall);
>>  EXPORT_SYMBOL(plpar_hcall9);
>>  EXPORT_SYMBOL(plpar_hcall_norets);
>>  
>> +#ifdef CONFIG_DEBUG_FS
>> +struct vpa {
>> +	struct dentry	*file;
> 
> You never use this other than temporarily when creating the file.
> 
>> +	int		cpu;
>> +};
>> +static DEFINE_PER_CPU(struct vpa, cpu_vpa);
> 
> And you never really use the per_cpu() either.
> 
> So you don't need the vpa struct at all AFAICS.

I now think so. Will rework on the patch.

> 
>> +
>> +static struct dentry *vpa_dir;
> 
> That can just be a local in vpa_debugfs_init().

ok.

> 
>> +#endif
>> +
>>  void vpa_init(int cpu)
>>  {
>>  	int hwcpu = get_hard_smp_processor_id(cpu);
>> @@ -1028,3 +1039,77 @@ static int __init reserve_vrma_context_id(void)
>>  	return 0;
>>  }
>>  machine_device_initcall(pseries, reserve_vrma_context_id);
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +/* debugfs file interface for vpa data */
>> +static ssize_t vpa_file_read(struct file *filp, char __user *buf, size_t len,
>> +		loff_t *pos)
> 
> Like this please:
> 
> static ssize_t vpa_file_read(struct file *filp, char __user *buf, size_t len,
> 			     loff_t *pos)
> 

noted

>> +{
>> +	long int rc;
>> +	struct vpa *vpa = filp->private_data;
>> +	struct lppaca *lppaca = &lppaca_of(vpa->cpu);
> 
> Once you've stashed the CPU number in private_data (see below) you can
> get it back with:
> 
> 	int cpu = (long)filp->private_data;
> 	struct lppaca *lppaca = &lppaca_of(cpu);

sure

> 
>> +
>> +	if (len < sizeof(struct lppaca))
>> +		return -EINVAL;
>> +
>> +	rc = copy_to_user(buf, lppaca, sizeof(struct lppaca));
>> +	if (rc)
>> +		return -EFAULT;
> 
> You should use simple_read_from_buffer().

ok

> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int vpa_file_open(struct inode *inode, struct file *filp)
>> +{
>> +	struct vpa *vpa = inode->i_private;
>> +
>> +	filp->private_data = vpa;
>> +	return 0;
>> +}
> 
> You can just use simple_open().

ok

> 
>> +static int vpa_file_release(struct inode *inode, struct file *filp)
>> +{
>> +	return 0;
>> +}
> 
> I don't think you need release if it's empty.

noted

> 
>> +static const struct file_operations vpa_fops = {
>> +	.open		= vpa_file_open,
>> +	.release	= vpa_file_release,
>> +	.read		= vpa_file_read,
>> +	.llseek		= no_llseek,
>> +};
>> +
>> +static int __init vpa_debugfs_init(void)
>> +{
>> +	char name[10];
> 
> That's not big enough if you end up with 4 billion CPUs. Make it 16.

Will make it 16.

> 
>> +	int i;
>> +
>> +	if (!firmware_has_feature(FW_FEATURE_SPLPAR))
>> +		return 0;
>> +
>> +	vpa_dir = debugfs_create_dir("vpa", powerpc_debugfs_root);
>> +	if (!vpa_dir) {
>> +		pr_warn("%s: can't create vpa root dir\n", __func__);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	/* set up the per-cpu vpa file*/
>> +	for_each_possible_cpu(i) {
> 
> What happens when you read the file for a possible but not present CPU?

Right now the VPA data is still fetched. We can have an additional check
in open or read to check if the CPU is online and return error if the
CPU is offline.

> 
>> +		struct vpa *vpa = &per_cpu(cpu_vpa, i);
>> +
>> +		vpa->cpu = i;
>> +		sprintf(name, "cpu-%d", i);
>> +
>> +		vpa->file = debugfs_create_file(name, 0400, vpa_dir, vpa,
>> +					&vpa_fops);
> 
> Can be:
> 		struct dentry *d;
> 		d = debugfs_create_file(name, 0400, dir, (void *)cpu, &vpa_fops);
> 
> Where you're stashing the cpu number in the private_data as a void *.

ok.

Regards,
Aravinda

> 
>> +		if (!vpa->file) {
>> +			pr_warn("%s: can't create per-cpu vpa file\n",
>> +					__func__);
>> +			return -ENOMEM;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +machine_arch_initcall(pseries, vpa_debugfs_init);
>> +#endif /* CONFIG_DEBUG_FS */
> 
> 
> cheers
> 

-- 
Regards,
Aravinda

^ permalink raw reply

* Re: [PATCH] powerpc/pseries: Disable CPU hotplug across migrations
From: Gautham R Shenoy @ 2018-09-25  6:19 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Gautham R Shenoy, Nathan Fontenot, tyreld, linuxppc-dev
In-Reply-To: <87h8ie5rxe.fsf@concordia.ellerman.id.au>

On Tue, Sep 25, 2018 at 10:42:05AM +1000, Michael Ellerman wrote:
[..snip..]
> I'm not suggesting we try to bring them online after we've disabled CPU
> hotplug, if we detect that race we can just fail the migration.
> 
> Can't we do:
>  - save mask of offline CPUs
>  - bring all offline CPUs online
>  - disable CPU hotplug
>  - check if any CPUs are offline
>    - if so, we've raced with an offline
>    - bail out of the migration with an error
> 
> 
> Instead of bailing out we could go back to the start and try again for
> some number of retries, but that's probably overkill anyway.
> 
> What am I missing?

I guess that will work. The race is unlikely anyway, so I doubt
CPU-Hotplug can DDOS the partition migration.

Does the following implementation of the same look ok ? (Build tested) 

------------------------------------ X8------------------------------------- 
>From acb9eb9f8bb14cf3121aeb0589255cbc31292be7 Mon Sep 17 00:00:00 2001
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Date: Tue, 25 Sep 2018 11:01:18 +0530
Subject: [PATCH] powerpc/rtas: Fix a potential race between CPU-Offline & Migration

commit 85a88cabad57 ("powerpc/pseries: Disable CPU hotplug across
migrations") disables any CPU-hotplug operations when Live Partition
Migration is in progress. However, there is a minor race-window
between the time all the CPUs are onlined by rtas_ibm_suspend_me() and
the CPU-Hotplugs are disabled via cpu_hotplug_disable() when some CPUs
could be offlined by the userspace, thus nullifying the assumption
that all the CPUs are online at this point.

This patch fixes this by checking if all the present CPUs are brought
online after disabling CPU-Hotplug. Otherwise, it retries to bring the
CPUs online again for a finite number of times failing which
rtas_ibm_suspend_me() returns -EBUSY.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/rtas.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 2c7ed31..e6a6425 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -934,6 +934,8 @@ int rtas_offline_cpus_mask(cpumask_var_t cpus)
 }
 EXPORT_SYMBOL(rtas_offline_cpus_mask);
 
+#define MAX_SUSPEND_HOTPLUG_RETRIES    5
+
 int rtas_ibm_suspend_me(u64 handle)
 {
 	long state;
@@ -943,6 +945,7 @@ int rtas_ibm_suspend_me(u64 handle)
 	DECLARE_COMPLETION_ONSTACK(done);
 	cpumask_var_t offline_mask;
 	int cpuret;
+	int retries = MAX_SUSPEND_HOTPLUG_RETRIES;
 
 	if (!rtas_service_present("ibm,suspend-me"))
 		return -ENOSYS;
@@ -972,6 +975,7 @@ int rtas_ibm_suspend_me(u64 handle)
 	data.token = rtas_token("ibm,suspend-me");
 	data.complete = &done;
 
+again:
 	/* All present CPUs must be online */
 	cpumask_andnot(offline_mask, cpu_present_mask, cpu_online_mask);
 	cpuret = rtas_online_cpus_mask(offline_mask);
@@ -982,6 +986,19 @@ int rtas_ibm_suspend_me(u64 handle)
 	}
 
 	cpu_hotplug_disable();
+
+	/* Check if we raced with a CPU-Offline Operation */
+	if (unlikely(!cpumask_equal(cpu_present_mask, cpu_online_mask))) {
+		cpu_hotplug_enable();
+		if (retries-- > 0)
+			goto again;
+
+		pr_err("%s: Too many concurrent CPU-Offline operation in progress\n",
+		       __func__);
+		atomic_set(&data.error, -EBUSY);
+		goto out;
+	}
+
 	stop_topology_update();
 
 	/* Call function on all CPUs.  One of us will make the
-- 
1.9.4

^ permalink raw reply related

* Re: [PATCH 2/3] powerpc: Add system call table generation support
From: Arnd Bergmann @ 2018-09-25  6:00 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Firoz Khan, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras,
	linuxram, leitao, Boqun Feng, gregkh, Philippe Ombredanne,
	Thomas Gleixner, Kate Stewart, y2038 Mailman List,
	Linux Kernel Mailing List, linux-arch, Deepa Dinamani,
	Marcin Juszkiewicz
In-Reply-To: <87efdi5rm6.fsf@concordia.ellerman.id.au>

On Tue, Sep 25, 2018 at 2:48 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> > On Tue, Sep 18, 2018 at 2:15 PM Firoz Khan <firoz.khan@linaro.org> wrote:
> >> On 14 September 2018 at 15:31, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Fri, Sep 14, 2018 at 10:33 AM Firoz Khan <firoz.khan@linaro.org> wrote:
> >
> > But all three existing architectures (x86, s390 and arm) already
> > have the capability to parse the table and generate different output
> > from that.
>
> Yeah, we want that on powerpc too.
>
> If the script needs to be more complex that's fine, if it can't be
> shared across arches that's fine, the main thing for me is that wiring
> up a syscall can be done by adding a single line in a single file.

Yes, that's definitely the idea, we want to make it easier for everyone.
We need at least a special case for mips, which needs three separate
input files (the tables are completely different) to generate four versions
of the output, plus a future extension to use the generic table for their
new one.

For powerpc, I'm hoping that both the table format and script can be
completely generic and not need a special case that is different
from the others, but if we need some extra magic to handle the SPU
syscalls, we can still do that with a private script.

      Arnd

^ permalink raw reply

* Re: [RFC PATCH v1 1/9] timer: fix circular header dependency
From: Christophe LEROY @ 2018-09-25  5:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	aneesh.kumar, npiggin, Thomas Gleixner, Alexander Viro,
	John Stultz, Stephen Boyd
  Cc: linux-fsdevel, linuxppc-dev, linux-kernel
In-Reply-To: <72d3e8c38f729dc22cb3235319b72f96190e01ee.1537802981.git.christophe.leroy@c-s.fr>



Le 24/09/2018 à 17:52, Christophe Leroy a écrit :
> When switching powerpc to CONFIG_THREAD_INFO_IN_TASK, include/sched.h
> has to be included in asm/smp.h for the following change, in order
> to avoid uncomplete definition of task_struct:
> 
> -#define raw_smp_processor_id() (current_thread_info()->cpu)
> +#define raw_smp_processor_id() (current->cpu)
> 
> But this generates the following compilation error, due to circular
> header dependency.
> 
>    CC      kernel/time/alarmtimer.o
> In file included from ./arch/powerpc/include/asm/smp.h:31,
>                   from ./include/linux/smp.h:64,
>                   from ./include/linux/percpu.h:7,
>                   from ./include/linux/hrtimer.h:22,
>                   from kernel/time/alarmtimer.c:19:
> ./include/linux/sched.h:558:19: error: field 'dl_timer' has incomplete type
>    struct hrtimer   dl_timer;
>                     ^~~~~~~~
> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has incomplete type
>    struct hrtimer inactive_timer;
>                   ^~~~~~~~~~~~~~
> make[1]: *** [kernel/time/alarmtimer.o] Error 1
> make: *** [kernel/time/alarmtimer.o] Error 2
> 
>    CC      fs/timerfd.o
> In file included from ./arch/powerpc/include/asm/smp.h:31,
>                   from ./include/linux/smp.h:64,
>                   from ./include/linux/percpu.h:7,
>                   from ./include/linux/hrtimer.h:22,
>                   from ./include/linux/alarmtimer.h:6,
>                   from fs/timerfd.c:12:
> ./include/linux/sched.h:558:19: error: field 'dl_timer' has incomplete type
>    struct hrtimer   dl_timer;
>                     ^~~~~~~~
> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has incomplete type
>    struct hrtimer inactive_timer;
>                   ^~~~~~~~~~~~~~
> make[1]: *** [fs/timerfd.o] Error 1
> make: *** [fs/timerfd.o] Error 2
> 
> This patch fixes it by including linux/hrtimer.h after linux/sched.h
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>   Should it be fixed in powerpc instead ? In that case, how ? Any idea ?


Looks like there are several other places where the problem occurs, so 
it has to be fixed in the powerpc headers instead.

Seems like RISC arch faced the same issue, and fixed it the following way:

/*
  * This is particularly ugly: it appears we can't actually get the 
definition
  * of task_struct here, but we need access to the CPU this task is 
running on.
  * Instead of using C we're using asm-offsets.h to get the current 
processor
  * ID.
  */
#define raw_smp_processor_id() (*((int*)((char*)get_current() + 
TASK_TI_CPU)))


Unless someone has a better idea, I'll fixed it that way.

Christophe


> 
>   fs/timerfd.c             | 2 +-
>   kernel/time/alarmtimer.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/timerfd.c b/fs/timerfd.c
> index d69ad801eb80..0fc01b241382 100644
> --- a/fs/timerfd.c
> +++ b/fs/timerfd.c
> @@ -9,12 +9,12 @@
>    *
>    */
>   
> -#include <linux/alarmtimer.h>
>   #include <linux/file.h>
>   #include <linux/poll.h>
>   #include <linux/init.h>
>   #include <linux/fs.h>
>   #include <linux/sched.h>
> +#include <linux/alarmtimer.h>
>   #include <linux/kernel.h>
>   #include <linux/slab.h>
>   #include <linux/list.h>
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index fa5de5e8de61..5fb75c9b3f06 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -16,11 +16,11 @@
>    * published by the Free Software Foundation.
>    */
>   #include <linux/time.h>
> -#include <linux/hrtimer.h>
>   #include <linux/timerqueue.h>
>   #include <linux/rtc.h>
>   #include <linux/sched/signal.h>
>   #include <linux/sched/debug.h>
> +#include <linux/hrtimer.h>
>   #include <linux/alarmtimer.h>
>   #include <linux/mutex.h>
>   #include <linux/platform_device.h>
> 

^ permalink raw reply

* Re: [PATCH 4/4] dma-mapping: clear dev->dma_ops in arch_teardown_dma_ops
From: Michael Ellerman @ 2018-09-25  5:32 UTC (permalink / raw)
  To: Guenter Roeck, Christoph Hellwig
  Cc: iommu, Greg Kroah-Hartman, Robin Murphy, linux-kernel,
	linux-arm-kernel, Marek Szyprowski, Bjorn Helgaas, linux-pci,
	Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev
In-Reply-To: <20180922150119.GA14761@roeck-us.net>

Guenter Roeck <linux@roeck-us.net> writes:
> Hi,
>
> On Mon, Aug 27, 2018 at 10:47:11AM +0200, Christoph Hellwig wrote:
>> There is no reason to leave the per-device dma_ops around when
>> deconfiguring a device, so move this code from arm64 into the
>> common code.
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>
> This patch causes various ppc images to crash in -next due to NULL
> DMA ops in dma_alloc_attrs().

I finally remembered where you autobuilder is :)

https://kerneltests.org/builders/qemu-ppc-next/builds/970/steps/qemubuildcommand_1/logs/stdio

Looks like mac99 at least is failing.

Just reproduced it on my setup.

> Looking into the code, the macio driver tries to instantiate on
> the 0000:f0:0d.0 PCI address (the driver maps to all Apple PCI IDs)
> and fails. This results in a call to arch_teardown_dma_ops(). Later,
> the same device pointer is used to instantiate ohci-pci, which
> crashes in probe because the dma_ops pointer has been cleared.
>
> I don't claim to fully understand the code, but to me it looks like
> the pci device is allocated and waiting for a driver to attach to.
> See arch/powerpc/kernel/pci_of_scan.c:of_create_pci_dev().
> If attaching a driver (here: macio) fails, the same device pointer
> is then reused for the next matching driver until a match is found
> and the device is successfully instantiated. Of course this fails
> quite badly if the device pointer has been scrubbed after the first
> failure.
>
> I don't know if this is generic PCI behavior or ppc specific.
> I am copying pci and ppc maintainers for additional input.

I would guess this is some PPC special sauce  O_o

Will have a look.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/tm: Fix userspace r13 corruption
From: Michael Neuling @ 2018-09-25  5:24 UTC (permalink / raw)
  To: Breno Leitao, mpe
  Cc: linuxppc-dev, Nicholas Piggin, paulus, benh, aneesh.kumar
In-Reply-To: <2675a034-288e-a3c2-a5df-7a223d79dc88@debian.org>

On Mon, 2018-09-24 at 11:32 -0300, Breno Leitao wrote:
> Hi Mikey,
>=20
> On 09/24/2018 04:27 AM, Michael Neuling wrote:
> > When we treclaim we store the userspace checkpointed r13 to a scratch
> > SPR and then later save the scratch SPR to the user thread struct.
> >=20
> > Unfortunately, this doesn't work as accessing the user thread struct
> > can take an SLB fault and the SLB fault handler will write the same
> > scratch SPRG that now contains the userspace r13.
> >=20
> > To fix this, we store r13 to the kernel stack (which can't fault)
> > before we access the user thread struct.
> >=20
> > Found by running P8 guest + powervm + disable_1tb_segments + TM. Seen
> > as a random userspace segfault with r13 looking like a kernel address.
> >=20
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
>=20
> Reviewed-by: Breno Leitao <leitao@debian.org>
>=20
> I am wondering if the same problem could not happen with r1 as well, sinc=
e r1
> is kept in the paca->tm_scratch after MSR[RI] is enabled, in a code simil=
ar
> to this:
>=20
>         std     r1, PACATMSCRATCH(r13)
> 	..
>         li      r11, MSR_RI
> 	mtmsrd  r11, 1
> 	....
>         ld      r3, PACATMSCRATCH(r13)          /* user r1 */
>         std     r3, GPR1(r7)
>=20
> There might be a corruption if the exception that is raised just after
> MSR[RI] is enabled somehow exits through 'fast_exception_return' (being
> called by most of the exception handlers as I understand) which will
> overwrite paca->tm_scratch with the upcoming MSR (aka SRR1) just before t=
he
> SRR1, as:
>=20
> 	ld      r3,_MSR(r1)
>=20
> 	...
> 	std     r3, PACATMSCRATCH(r13) /* Stash returned-to MSR */
>=20
>=20
> It seems that slb_miss_common and instruction_access_slb does not go thro=
ugh
> this path, but handle_page_fault calls
> ret_from_except_lite->fast_exception_return, which might cause this
> 'possible' issue.

Yeah, good catch! I think we are ok but it's delicate.=20

I'll store it in the kernel stack and avoid PACATMSCRATCH before I turn RI =
on. =20

This look ok?

diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
index 701b0f5b09..ff781b2852 100644
--- a/arch/powerpc/kernel/tm.S
+++ b/arch/powerpc/kernel/tm.S
@@ -178,6 +178,12 @@ _GLOBAL(tm_reclaim)
=20
 	std	r11, GPR11(r1)			/* Temporary stash */
=20
+	/* Move r1 to kernel stack in case PACATMSCRATCH is used once
+	 * we turn on RI
+	 */
+	ld	r11, PACATMSCRATCH(r13)
+	std	r11, GPR1(r1)
+
 	/* Store r13 away so we can free up the scratch SPR for the
 	 * SLB fault handler (needed once we start access the
 	 * thread_struct)
@@ -214,7 +220,7 @@ _GLOBAL(tm_reclaim)
 	SAVE_GPR(8, r7)				/* user r8 */
 	SAVE_GPR(9, r7)				/* user r9 */
 	SAVE_GPR(10, r7)			/* user r10 */
-	ld	r3, PACATMSCRATCH(r13)		/* user r1 */
+	ld	r3, GPR1(r1)			/* user r1 */
 	ld	r4, GPR7(r1)			/* user r7 */
 	ld	r5, GPR11(r1)			/* user r11 */
 	ld	r6, GPR12(r1)			/* user r12 */

^ permalink raw reply related

* Re: [PATCH v2 2/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in linux/setbits.h
From: Christophe LEROY @ 2018-09-25  5:05 UTC (permalink / raw)
  To: Corentin Labbe, Gilles.Muller, Julia.Lawall, agust, airlied,
	alexandre.torgue, alistair, benh, carlo, davem, galak, joabreu,
	khilman, maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
	nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
  Cc: netdev, linux-kernel, dri-devel, linux-ide, linux-amlogic,
	linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <1537815856-31728-3-git-send-email-clabbe@baylibre.com>



Le 24/09/2018 à 21:04, Corentin Labbe a écrit :
> This patch adds setbits32/clrbits32/clrsetbits32 and
> setbits64/clrbits64/clrsetbits64 in linux/setbits.h header.

Fix the patch subject and description.

> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>   include/linux/setbits.h | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 88 insertions(+)
>   create mode 100644 include/linux/setbits.h
> 
> diff --git a/include/linux/setbits.h b/include/linux/setbits.h
> new file mode 100644
> index 000000000000..6e7e257134ae
> --- /dev/null
> +++ b/include/linux/setbits.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_SETBITS_H
> +#define __LINUX_SETBITS_H
> +
> +#include <linux/io.h>
> +
> +#define __setbits(readfunction, writefunction, addr, set) \
> +	writefunction((readfunction(addr) | (set)), addr)

You don't need so long names for parameters in a 2 lines macro (See 
Linux Kernel Codying style §4 Naming).

A single line macro would be feasible with only 3 chars names:

#define __setbits(rfn, wfn, addr, set) wfn((rfn(addr) | (set)), addr)

> +#define __clrbits(readfunction, writefunction, addr, mask) \
> +	writefunction((readfunction(addr) & ~(mask)), addr)
> +#define __clrsetbits(readfunction, writefunction, addr, mask, set) \
> +	writefunction(((readfunction(addr) & ~(mask)) | (set)), addr)
> +#define __setclrbits(readfunction, writefunction, addr, mask, set) \
> +	writefunction(((readfunction(addr) | (set)) & ~(mask)), addr)
> +
> +#ifndef setbits_le32
> +#define setbits_le32(addr, set) __setbits(readl, writel, addr, set)
> +#endif
> +#ifndef setbits_le32_relaxed
> +#define setbits_le32_relaxed(addr, set) __setbits(readl_relaxed, writel_relaxed, \
> +					       addr, set)
> +#endif
> +
> +#ifndef clrbits_le32
> +#define clrbits_le32(addr, mask) __clrbits(readl, writel, addr, mask)
> +#endif
> +#ifndef clrbits_le32_relaxed
> +#define clrbits_le32_relaxed(addr, mask) __clrbits(readl_relaxed, writel_relaxed, \
> +						addr, mask)
> +#endif
> +
> +#ifndef clrsetbits_le32
> +#define clrsetbits_le32(addr, mask, set) __clrsetbits(readl, writel, addr, mask, set)
> +#endif
> +#ifndef clrsetbits_le32_relaxed
> +#define clrsetbits_le32_relaxed(addr, mask, set) __clrsetbits(readl_relaxed, \
> +							   writel_relaxed, \
> +							   addr, mask, set)
> +#endif
> +
> +#ifndef setclrbits_le32
> +#define setclrbits_le32(addr, mask, set) __setclrbits(readl, writel, addr, mask, set)
> +#endif
> +#ifndef setclrbits_le32_relaxed
> +#define setclrbits_le32_relaxed(addr, mask, set) __setclrbits(readl_relaxed, \
> +							   writel_relaxed, \
> +							   addr, mask, set)
> +#endif
> +
> +/* We cannot use CONFIG_64BIT as some x86 drivers use non-atomicwriteq() */
> +#if defined(writeq) && defined(readq)

Take care. At least Alpha Arch defines it as a static inline without
redefining it as a #define. (see arch/alpha/kernel/io.c)

Christophe

> +#ifndef setbits_le64
> +#define setbits_le64(addr, set) __setbits(readq, writeq, addr, set)
> +#endif
> +#ifndef setbits_le64_relaxed
> +#define setbits_le64_relaxed(addr, set) __setbits(readq_relaxed, writeq_relaxed, \
> +					       addr, set)
> +#endif
> +
> +#ifndef clrbits_le64
> +#define clrbits_le64(addr, mask) __clrbits(readq, writeq, addr, mask)
> +#endif
> +#ifndef clrbits_le64_relaxed
> +#define clrbits_le64_relaxed(addr, mask) __clrbits(readq_relaxed, writeq_relaxed, \
> +						addr, mask)
> +#endif
> +
> +#ifndef clrsetbits_le64
> +#define clrsetbits_le64(addr, mask, set) __clrsetbits(readq, writeq, addr, mask, set)
> +#endif
> +#ifndef clrsetbits_le64_relaxed
> +#define clrsetbits_le64_relaxed(addr, mask, set) __clrsetbits(readq_relaxed, \
> +							   writeq_relaxed, \
> +							   addr, mask, set)
> +#endif
> +
> +#ifndef setclrbits_le64
> +#define setclrbits_le64(addr, mask, set) __setclrbits(readq, writeq, addr, mask, set)
> +#endif
> +#ifndef setclrbits_le64_relaxed
> +#define setclrbits_le64_relaxed(addr, mask, set) __setclrbits(readq_relaxed, \
> +							   writeq_relaxed, \
> +							   addr, mask, set)
> +#endif
> +
> +#endif /* writeq/readq */
> +
> +#endif /* __LINUX_SETBITS_H */
> 

^ permalink raw reply

* Re: [PATCH v2 1/7] powerpc: rename setbits32/clrbits32 to setbits32_be/clrbits32_be
From: Christophe LEROY @ 2018-09-25  4:56 UTC (permalink / raw)
  To: Corentin Labbe, Gilles.Muller, Julia.Lawall, agust, airlied,
	alexandre.torgue, alistair, benh, carlo, davem, galak, joabreu,
	khilman, maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
	nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
  Cc: netdev, linux-kernel, dri-devel, linux-ide, linux-amlogic,
	linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <1537815856-31728-2-git-send-email-clabbe@baylibre.com>

Fix the patch title.


Le 24/09/2018 à 21:04, Corentin Labbe a écrit :
> Since setbits32/clrbits32 work on be32, it's better to remove ambiguity on
> the used data type.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>   arch/powerpc/include/asm/fsl_lbc.h               |  2 +-
>   arch/powerpc/include/asm/io.h                    |  5 +-
>   arch/powerpc/platforms/44x/canyonlands.c         |  4 +-
>   arch/powerpc/platforms/4xx/gpio.c                | 28 ++++-----
>   arch/powerpc/platforms/512x/pdm360ng.c           |  6 +-
>   arch/powerpc/platforms/52xx/mpc52xx_common.c     |  6 +-
>   arch/powerpc/platforms/52xx/mpc52xx_gpt.c        | 10 ++--
>   arch/powerpc/platforms/82xx/ep8248e.c            |  2 +-
>   arch/powerpc/platforms/82xx/km82xx.c             |  6 +-
>   arch/powerpc/platforms/82xx/mpc8272_ads.c        | 10 ++--
>   arch/powerpc/platforms/82xx/pq2.c                |  2 +-
>   arch/powerpc/platforms/82xx/pq2ads-pci-pic.c     |  4 +-
>   arch/powerpc/platforms/82xx/pq2fads.c            | 10 ++--
>   arch/powerpc/platforms/83xx/km83xx.c             |  6 +-
>   arch/powerpc/platforms/83xx/mpc836x_mds.c        |  2 +-
>   arch/powerpc/platforms/85xx/mpc85xx_mds.c        |  2 +-
>   arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c     |  4 +-
>   arch/powerpc/platforms/85xx/mpc85xx_rdb.c        |  2 +-
>   arch/powerpc/platforms/85xx/p1022_ds.c           |  4 +-
>   arch/powerpc/platforms/85xx/p1022_rdk.c          |  4 +-
>   arch/powerpc/platforms/85xx/t1042rdb_diu.c       |  4 +-
>   arch/powerpc/platforms/85xx/twr_p102x.c          |  2 +-
>   arch/powerpc/platforms/86xx/mpc8610_hpcd.c       |  4 +-
>   arch/powerpc/platforms/8xx/adder875.c            |  2 +-
>   arch/powerpc/platforms/8xx/m8xx_setup.c          |  4 +-
>   arch/powerpc/platforms/8xx/mpc86xads_setup.c     |  4 +-
>   arch/powerpc/platforms/8xx/mpc885ads_setup.c     | 28 ++++-----
>   arch/powerpc/platforms/embedded6xx/flipper-pic.c |  6 +-
>   arch/powerpc/platforms/embedded6xx/hlwd-pic.c    |  8 +--
>   arch/powerpc/platforms/embedded6xx/wii.c         | 10 ++--
>   arch/powerpc/sysdev/cpm1.c                       | 26 ++++-----
>   arch/powerpc/sysdev/cpm2.c                       | 16 ++---
>   arch/powerpc/sysdev/cpm_common.c                 |  4 +-
>   arch/powerpc/sysdev/fsl_85xx_l2ctlr.c            |  8 +--
>   arch/powerpc/sysdev/fsl_lbc.c                    |  2 +-
>   arch/powerpc/sysdev/fsl_pci.c                    |  8 +--
>   arch/powerpc/sysdev/fsl_pmc.c                    |  2 +-
>   arch/powerpc/sysdev/fsl_rcpm.c                   | 74 ++++++++++++------------
>   arch/powerpc/sysdev/fsl_rio.c                    |  4 +-
>   arch/powerpc/sysdev/fsl_rmu.c                    |  8 +--
>   arch/powerpc/sysdev/mpic_timer.c                 | 12 ++--
>   41 files changed, 178 insertions(+), 177 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/fsl_lbc.h b/arch/powerpc/include/asm/fsl_lbc.h
> index c7240a024b96..4d6a56b48a28 100644
> --- a/arch/powerpc/include/asm/fsl_lbc.h
> +++ b/arch/powerpc/include/asm/fsl_lbc.h
> @@ -276,7 +276,7 @@ static inline void fsl_upm_start_pattern(struct fsl_upm *upm, u8 pat_offset)
>    */
>   static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
>   {
> -	clrbits32(upm->mxmr, MxMR_OP_RP);
> +	clrbits_be32(upm->mxmr, MxMR_OP_RP);
>   
>   	while (in_be32(upm->mxmr) & MxMR_OP_RP)
>   		cpu_relax();
> diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> index e0331e754568..57486a1b9992 100644
> --- a/arch/powerpc/include/asm/io.h
> +++ b/arch/powerpc/include/asm/io.h
> @@ -873,8 +873,8 @@ static inline void * bus_to_virt(unsigned long address)
>   #endif /* CONFIG_PPC32 */
>   
>   /* access ports */
> -#define setbits32(_addr, _v) out_be32((_addr), in_be32(_addr) |  (_v))
> -#define clrbits32(_addr, _v) out_be32((_addr), in_be32(_addr) & ~(_v))
> +#define setbits_be32(_addr, _v) out_be32((_addr), in_be32(_addr) |  (_v))
> +#define clrbits_be32(_addr, _v) out_be32((_addr), in_be32(_addr) & ~(_v))
>   
>   #define setbits16(_addr, _v) out_be16((_addr), in_be16(_addr) |  (_v))
>   #define clrbits16(_addr, _v) out_be16((_addr), in_be16(_addr) & ~(_v))
> @@ -904,6 +904,7 @@ static inline void * bus_to_virt(unsigned long address)
>   #define clrsetbits_le16(addr, clear, set) clrsetbits(le16, addr, clear, set)
>   
>   #define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)
> +#define clrsetbits_be32(addr, clear, set) clrsetbits(be32, addr, clear, set)

This one already exists a few lines above.

>   
>   #endif /* __KERNEL__ */
>   
> diff --git a/arch/powerpc/platforms/44x/canyonlands.c b/arch/powerpc/platforms/44x/canyonlands.c
> index 157f4ce46386..6aeb4ca64d09 100644
> --- a/arch/powerpc/platforms/44x/canyonlands.c
> +++ b/arch/powerpc/platforms/44x/canyonlands.c
> @@ -113,8 +113,8 @@ static int __init ppc460ex_canyonlands_fixup(void)
>   	 * USB2HStop and gpio19 will be USB2DStop. For more details refer to
>   	 * table 34-7 of PPC460EX user manual.
>   	 */
> -	setbits32((vaddr + GPIO0_OSRH), 0x42000000);
> -	setbits32((vaddr + GPIO0_TSRH), 0x42000000);
> +	setbits_be32((vaddr + GPIO0_OSRH), 0x42000000);
> +	setbits_be32((vaddr + GPIO0_TSRH), 0x42000000);
>   err_gpio:
>   	iounmap(vaddr);
>   err_bcsr:
> diff --git a/arch/powerpc/platforms/4xx/gpio.c b/arch/powerpc/platforms/4xx/gpio.c
> index 2238e369cde4..8436da0617fd 100644
> --- a/arch/powerpc/platforms/4xx/gpio.c
> +++ b/arch/powerpc/platforms/4xx/gpio.c
> @@ -82,9 +82,9 @@ __ppc4xx_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   	struct ppc4xx_gpio __iomem *regs = mm_gc->regs;
>   
>   	if (val)
> -		setbits32(&regs->or, GPIO_MASK(gpio));
> +		setbits_be32(&regs->or, GPIO_MASK(gpio));
>   	else
> -		clrbits32(&regs->or, GPIO_MASK(gpio));
> +		clrbits_be32(&regs->or, GPIO_MASK(gpio));
>   }
>   
>   static void
> @@ -112,18 +112,18 @@ static int ppc4xx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>   	spin_lock_irqsave(&chip->lock, flags);
>   
>   	/* Disable open-drain function */
> -	clrbits32(&regs->odr, GPIO_MASK(gpio));
> +	clrbits_be32(&regs->odr, GPIO_MASK(gpio));
>   
>   	/* Float the pin */
> -	clrbits32(&regs->tcr, GPIO_MASK(gpio));
> +	clrbits_be32(&regs->tcr, GPIO_MASK(gpio));
>   
>   	/* Bits 0-15 use TSRL/OSRL, bits 16-31 use TSRH/OSRH */
>   	if (gpio < 16) {
> -		clrbits32(&regs->osrl, GPIO_MASK2(gpio));
> -		clrbits32(&regs->tsrl, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->osrl, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->tsrl, GPIO_MASK2(gpio));
>   	} else {
> -		clrbits32(&regs->osrh, GPIO_MASK2(gpio));
> -		clrbits32(&regs->tsrh, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->osrh, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->tsrh, GPIO_MASK2(gpio));
>   	}
>   
>   	spin_unlock_irqrestore(&chip->lock, flags);
> @@ -145,18 +145,18 @@ ppc4xx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>   	__ppc4xx_gpio_set(gc, gpio, val);
>   
>   	/* Disable open-drain function */
> -	clrbits32(&regs->odr, GPIO_MASK(gpio));
> +	clrbits_be32(&regs->odr, GPIO_MASK(gpio));
>   
>   	/* Drive the pin */
> -	setbits32(&regs->tcr, GPIO_MASK(gpio));
> +	setbits_be32(&regs->tcr, GPIO_MASK(gpio));
>   
>   	/* Bits 0-15 use TSRL, bits 16-31 use TSRH */
>   	if (gpio < 16) {
> -		clrbits32(&regs->osrl, GPIO_MASK2(gpio));
> -		clrbits32(&regs->tsrl, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->osrl, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->tsrl, GPIO_MASK2(gpio));
>   	} else {
> -		clrbits32(&regs->osrh, GPIO_MASK2(gpio));
> -		clrbits32(&regs->tsrh, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->osrh, GPIO_MASK2(gpio));
> +		clrbits_be32(&regs->tsrh, GPIO_MASK2(gpio));
>   	}
>   
>   	spin_unlock_irqrestore(&chip->lock, flags);
> diff --git a/arch/powerpc/platforms/512x/pdm360ng.c b/arch/powerpc/platforms/512x/pdm360ng.c
> index dc81f05e0bce..06b95795267a 100644
> --- a/arch/powerpc/platforms/512x/pdm360ng.c
> +++ b/arch/powerpc/platforms/512x/pdm360ng.c
> @@ -38,7 +38,7 @@ static int pdm360ng_get_pendown_state(void)
>   
>   	reg = in_be32(pdm360ng_gpio_base + 0xc);
>   	if (reg & 0x40)
> -		setbits32(pdm360ng_gpio_base + 0xc, 0x40);
> +		setbits_be32(pdm360ng_gpio_base + 0xc, 0x40);
>   
>   	reg = in_be32(pdm360ng_gpio_base + 0x8);
>   
> @@ -69,8 +69,8 @@ static int __init pdm360ng_penirq_init(void)
>   		return -ENODEV;
>   	}
>   	out_be32(pdm360ng_gpio_base + 0xc, 0xffffffff);
> -	setbits32(pdm360ng_gpio_base + 0x18, 0x2000);
> -	setbits32(pdm360ng_gpio_base + 0x10, 0x40);
> +	setbits_be32(pdm360ng_gpio_base + 0x18, 0x2000);
> +	setbits_be32(pdm360ng_gpio_base + 0x10, 0x40);
>   
>   	return 0;
>   }
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> index 565e3a83dc9e..edfe619d67bf 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> @@ -314,13 +314,13 @@ int mpc5200_psc_ac97_gpio_reset(int psc_number)
>   
>   	/* enable gpio pins for output */
>   	setbits8(&wkup_gpio->wkup_gpioe, reset);
> -	setbits32(&simple_gpio->simple_gpioe, sync | out);
> +	setbits_be32(&simple_gpio->simple_gpioe, sync | out);
>   
>   	setbits8(&wkup_gpio->wkup_ddr, reset);
> -	setbits32(&simple_gpio->simple_ddr, sync | out);
> +	setbits_be32(&simple_gpio->simple_ddr, sync | out);
>   
>   	/* Assert cold reset */
> -	clrbits32(&simple_gpio->simple_dvo, sync | out);
> +	clrbits_be32(&simple_gpio->simple_dvo, sync | out);
>   	clrbits8(&wkup_gpio->wkup_dvo, reset);
>   
>   	/* wait for 1 us */
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> index 17cf249b18ee..e9f4dec06077 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> @@ -142,7 +142,7 @@ static void mpc52xx_gpt_irq_unmask(struct irq_data *d)
>   	unsigned long flags;
>   
>   	raw_spin_lock_irqsave(&gpt->lock, flags);
> -	setbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
> +	setbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
>   	raw_spin_unlock_irqrestore(&gpt->lock, flags);
>   }
>   
> @@ -152,7 +152,7 @@ static void mpc52xx_gpt_irq_mask(struct irq_data *d)
>   	unsigned long flags;
>   
>   	raw_spin_lock_irqsave(&gpt->lock, flags);
> -	clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
> +	clrbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
>   	raw_spin_unlock_irqrestore(&gpt->lock, flags);
>   }
>   
> @@ -308,7 +308,7 @@ static int mpc52xx_gpt_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
>   	dev_dbg(gpt->dev, "%s: gpio:%d\n", __func__, gpio);
>   
>   	raw_spin_lock_irqsave(&gpt->lock, flags);
> -	clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK);
> +	clrbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_GPIO_MASK);
>   	raw_spin_unlock_irqrestore(&gpt->lock, flags);
>   
>   	return 0;
> @@ -482,7 +482,7 @@ int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt)
>   		return -EBUSY;
>   	}
>   
> -	clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_COUNTER_ENABLE);
> +	clrbits_be32(&gpt->regs->mode, MPC52xx_GPT_MODE_COUNTER_ENABLE);
>   	raw_spin_unlock_irqrestore(&gpt->lock, flags);
>   	return 0;
>   }
> @@ -639,7 +639,7 @@ static int mpc52xx_wdt_release(struct inode *inode, struct file *file)
>   	unsigned long flags;
>   
>   	raw_spin_lock_irqsave(&gpt_wdt->lock, flags);
> -	clrbits32(&gpt_wdt->regs->mode,
> +	clrbits_be32(&gpt_wdt->regs->mode,
>   		  MPC52xx_GPT_MODE_COUNTER_ENABLE | MPC52xx_GPT_MODE_WDT_EN);

The alignment needs to be fixed here (and all other places). The second 
line should start under the &
Eventually use checkpatch to locate all places that need to be fixed. 
(checkpatch may even fix it for you)

Christophe

>   	gpt_wdt->wdt_mode &= ~MPC52xx_GPT_IS_WDT;
>   	raw_spin_unlock_irqrestore(&gpt_wdt->lock, flags);
> diff --git a/arch/powerpc/platforms/82xx/ep8248e.c b/arch/powerpc/platforms/82xx/ep8248e.c
> index 8fec050f2d5b..18626cd3db16 100644
> --- a/arch/powerpc/platforms/82xx/ep8248e.c
> +++ b/arch/powerpc/platforms/82xx/ep8248e.c
> @@ -262,7 +262,7 @@ static void __init ep8248e_setup_arch(void)
>   	/* When this is set, snooping CPM DMA from RAM causes
>   	 * machine checks.  See erratum SIU18.
>   	 */
> -	clrbits32(&cpm2_immr->im_siu_conf.siu_82xx.sc_bcr, MPC82XX_BCR_PLDP);
> +	clrbits_be32(&cpm2_immr->im_siu_conf.siu_82xx.sc_bcr, MPC82XX_BCR_PLDP);
>   
>   	ep8248e_bcsr_node =
>   		of_find_compatible_node(NULL, NULL, "fsl,ep8248e-bcsr");
> diff --git a/arch/powerpc/platforms/82xx/km82xx.c b/arch/powerpc/platforms/82xx/km82xx.c
> index 28860e40b5db..27d16d1a89f5 100644
> --- a/arch/powerpc/platforms/82xx/km82xx.c
> +++ b/arch/powerpc/platforms/82xx/km82xx.c
> @@ -157,9 +157,9 @@ static void __init init_ioports(void)
>   	cpm2_clk_setup(CPM_CLK_FCC2, CPM_CLK14, CPM_CLK_TX);
>   
>   	/* Force USB FULL SPEED bit to '1' */
> -	setbits32(&cpm2_immr->im_ioport.iop_pdata, 1 << (31 - 10));
> +	setbits_be32(&cpm2_immr->im_ioport.iop_pdata, 1 << (31 - 10));
>   	/* clear USB_SLAVE */
> -	clrbits32(&cpm2_immr->im_ioport.iop_pdata, 1 << (31 - 11));
> +	clrbits_be32(&cpm2_immr->im_ioport.iop_pdata, 1 << (31 - 11));
>   }
>   
>   static void __init km82xx_setup_arch(void)
> @@ -172,7 +172,7 @@ static void __init km82xx_setup_arch(void)
>   	/* When this is set, snooping CPM DMA from RAM causes
>   	 * machine checks.  See erratum SIU18.
>   	 */
> -	clrbits32(&cpm2_immr->im_siu_conf.siu_82xx.sc_bcr, MPC82XX_BCR_PLDP);
> +	clrbits_be32(&cpm2_immr->im_siu_conf.siu_82xx.sc_bcr, MPC82XX_BCR_PLDP);
>   
>   	init_ioports();
>   
> diff --git a/arch/powerpc/platforms/82xx/mpc8272_ads.c b/arch/powerpc/platforms/82xx/mpc8272_ads.c
> index d23c10a96bde..75338e9e8acc 100644
> --- a/arch/powerpc/platforms/82xx/mpc8272_ads.c
> +++ b/arch/powerpc/platforms/82xx/mpc8272_ads.c
> @@ -164,13 +164,13 @@ static void __init mpc8272_ads_setup_arch(void)
>   #define BCSR3_FETHIEN2		0x10000000
>   #define BCSR3_FETH2_RST		0x08000000
>   
> -	clrbits32(&bcsr[1], BCSR1_RS232_EN1 | BCSR1_RS232_EN2 | BCSR1_FETHIEN);
> -	setbits32(&bcsr[1], BCSR1_FETH_RST);
> +	clrbits_be32(&bcsr[1], BCSR1_RS232_EN1 | BCSR1_RS232_EN2 | BCSR1_FETHIEN);
> +	setbits_be32(&bcsr[1], BCSR1_FETH_RST);
>   
> -	clrbits32(&bcsr[3], BCSR3_FETHIEN2);
> -	setbits32(&bcsr[3], BCSR3_FETH2_RST);
> +	clrbits_be32(&bcsr[3], BCSR3_FETHIEN2);
> +	setbits_be32(&bcsr[3], BCSR3_FETH2_RST);
>   
> -	clrbits32(&bcsr[3], BCSR3_USB_nEN);
> +	clrbits_be32(&bcsr[3], BCSR3_USB_nEN);
>   
>   	iounmap(bcsr);
>   
> diff --git a/arch/powerpc/platforms/82xx/pq2.c b/arch/powerpc/platforms/82xx/pq2.c
> index c4f7029fc9ae..92f2b4a5dcc8 100644
> --- a/arch/powerpc/platforms/82xx/pq2.c
> +++ b/arch/powerpc/platforms/82xx/pq2.c
> @@ -25,7 +25,7 @@
>   void __noreturn pq2_restart(char *cmd)
>   {
>   	local_irq_disable();
> -	setbits32(&cpm2_immr->im_clkrst.car_rmr, RMR_CSRE);
> +	setbits_be32(&cpm2_immr->im_clkrst.car_rmr, RMR_CSRE);
>   
>   	/* Clear the ME,EE,IR & DR bits in MSR to cause checkstop */
>   	mtmsr(mfmsr() & ~(MSR_ME | MSR_EE | MSR_IR | MSR_DR));
> diff --git a/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c b/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c
> index 8b065bdf7412..060400ec3ebb 100644
> --- a/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c
> +++ b/arch/powerpc/platforms/82xx/pq2ads-pci-pic.c
> @@ -47,7 +47,7 @@ static void pq2ads_pci_mask_irq(struct irq_data *d)
>   		unsigned long flags;
>   		raw_spin_lock_irqsave(&pci_pic_lock, flags);
>   
> -		setbits32(&priv->regs->mask, 1 << irq);
> +		setbits_be32(&priv->regs->mask, 1 << irq);
>   		mb();
>   
>   		raw_spin_unlock_irqrestore(&pci_pic_lock, flags);
> @@ -63,7 +63,7 @@ static void pq2ads_pci_unmask_irq(struct irq_data *d)
>   		unsigned long flags;
>   
>   		raw_spin_lock_irqsave(&pci_pic_lock, flags);
> -		clrbits32(&priv->regs->mask, 1 << irq);
> +		clrbits_be32(&priv->regs->mask, 1 << irq);
>   		raw_spin_unlock_irqrestore(&pci_pic_lock, flags);
>   	}
>   }
> diff --git a/arch/powerpc/platforms/82xx/pq2fads.c b/arch/powerpc/platforms/82xx/pq2fads.c
> index 6c654dc74a4b..84b637e019ed 100644
> --- a/arch/powerpc/platforms/82xx/pq2fads.c
> +++ b/arch/powerpc/platforms/82xx/pq2fads.c
> @@ -140,18 +140,18 @@ static void __init pq2fads_setup_arch(void)
>   
>   	/* Enable the serial and ethernet ports */
>   
> -	clrbits32(&bcsr[1], BCSR1_RS232_EN1 | BCSR1_RS232_EN2 | BCSR1_FETHIEN);
> -	setbits32(&bcsr[1], BCSR1_FETH_RST);
> +	clrbits_be32(&bcsr[1], BCSR1_RS232_EN1 | BCSR1_RS232_EN2 | BCSR1_FETHIEN);
> +	setbits_be32(&bcsr[1], BCSR1_FETH_RST);
>   
> -	clrbits32(&bcsr[3], BCSR3_FETHIEN2);
> -	setbits32(&bcsr[3], BCSR3_FETH2_RST);
> +	clrbits_be32(&bcsr[3], BCSR3_FETHIEN2);
> +	setbits_be32(&bcsr[3], BCSR3_FETH2_RST);
>   
>   	iounmap(bcsr);
>   
>   	init_ioports();
>   
>   	/* Enable external IRQs */
> -	clrbits32(&cpm2_immr->im_siu_conf.siu_82xx.sc_siumcr, 0x0c000000);
> +	clrbits_be32(&cpm2_immr->im_siu_conf.siu_82xx.sc_siumcr, 0x0c000000);
>   
>   	pq2_init_pci();
>   
> diff --git a/arch/powerpc/platforms/83xx/km83xx.c b/arch/powerpc/platforms/83xx/km83xx.c
> index d8642a4afc74..d036b179dc65 100644
> --- a/arch/powerpc/platforms/83xx/km83xx.c
> +++ b/arch/powerpc/platforms/83xx/km83xx.c
> @@ -101,19 +101,19 @@ static void quirk_mpc8360e_qe_enet10(void)
>   		 * UCC1: write 0b11 to bits 18:19
>   		 * at address IMMRBAR+0x14A8
>   		 */
> -		setbits32((base + 0xa8), 0x00003000);
> +		setbits_be32((base + 0xa8), 0x00003000);
>   
>   		/*
>   		 * UCC2 option 1: write 0b11 to bits 4:5
>   		 * at address IMMRBAR+0x14A8
>   		 */
> -		setbits32((base + 0xa8), 0x0c000000);
> +		setbits_be32((base + 0xa8), 0x0c000000);
>   
>   		/*
>   		 * UCC2 option 2: write 0b11 to bits 16:17
>   		 * at address IMMRBAR+0x14AC
>   		 */
> -		setbits32((base + 0xac), 0x0000c000);
> +		setbits_be32((base + 0xac), 0x0000c000);
>   	}
>   	iounmap(base);
>   	of_node_put(np_par);
> diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> index fd44dd03e1f3..83a5e27e2f63 100644
> --- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> @@ -118,7 +118,7 @@ static void __init mpc836x_mds_setup_arch(void)
>   			 * IMMR + 0x14A8[4:5] = 11 (clk delay for UCC 2)
>   			 * IMMR + 0x14A8[18:19] = 11 (clk delay for UCC 1)
>   			 */
> -			setbits32(immap, 0x0c003000);
> +			setbits_be32(immap, 0x0c003000);
>   
>   			/*
>   			 * IMMR + 0x14AC[20:27] = 10101010
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> index d7e440e6dba3..52b4fb179c9e 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> @@ -262,7 +262,7 @@ static void __init mpc85xx_mds_qe_init(void)
>   			 * and QE12 for QE MII management signals in PMUXCR
>   			 * register.
>   			 */
> -				setbits32(&guts->pmuxcr, MPC85xx_PMUXCR_QE(0) |
> +				setbits_be32(&guts->pmuxcr, MPC85xx_PMUXCR_QE(0) |
>   						  MPC85xx_PMUXCR_QE(3) |
>   						  MPC85xx_PMUXCR_QE(9) |
>   						  MPC85xx_PMUXCR_QE(12));
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c
> index f05325f0cc03..926d0f9dc29d 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c
> @@ -60,9 +60,9 @@ static void mpc85xx_freeze_time_base(bool freeze)
>   
>   	mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
>   	if (freeze)
> -		setbits32(&guts->devdisr, mask);
> +		setbits_be32(&guts->devdisr, mask);
>   	else
> -		clrbits32(&guts->devdisr, mask);
> +		clrbits_be32(&guts->devdisr, mask);
>   
>   	in_be32(&guts->devdisr);
>   }
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> index 10069503e39f..fdea28dd90dd 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> @@ -115,7 +115,7 @@ static void __init mpc85xx_rdb_setup_arch(void)
>   			* and QE12 for QE MII management singals in PMUXCR
>   			* register.
>   			*/
> -				setbits32(&guts->pmuxcr, MPC85xx_PMUXCR_QE(0) |
> +				setbits_be32(&guts->pmuxcr, MPC85xx_PMUXCR_QE(0) |
>   						MPC85xx_PMUXCR_QE(3) |
>   						MPC85xx_PMUXCR_QE(9) |
>   						MPC85xx_PMUXCR_QE(12));
> diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
> index 9fb57f78cdbe..88860b270a7b 100644
> --- a/arch/powerpc/platforms/85xx/p1022_ds.c
> +++ b/arch/powerpc/platforms/85xx/p1022_ds.c
> @@ -405,11 +405,11 @@ void p1022ds_set_pixel_clock(unsigned int pixclock)
>   	pxclk = clamp_t(u32, pxclk, 2, 255);
>   
>   	/* Disable the pixel clock, and set it to non-inverted and no delay */
> -	clrbits32(&guts->clkdvdr,
> +	clrbits_be32(&guts->clkdvdr,
>   		  CLKDVDR_PXCKEN | CLKDVDR_PXCKDLY | CLKDVDR_PXCLK_MASK);
>   
>   	/* Enable the clock and set the pxclk */
> -	setbits32(&guts->clkdvdr, CLKDVDR_PXCKEN | (pxclk << 16));
> +	setbits_be32(&guts->clkdvdr, CLKDVDR_PXCKEN | (pxclk << 16));
>   
>   	iounmap(guts);
>   }
> diff --git a/arch/powerpc/platforms/85xx/p1022_rdk.c b/arch/powerpc/platforms/85xx/p1022_rdk.c
> index 276e00ab3dde..051713cf6c0e 100644
> --- a/arch/powerpc/platforms/85xx/p1022_rdk.c
> +++ b/arch/powerpc/platforms/85xx/p1022_rdk.c
> @@ -75,11 +75,11 @@ void p1022rdk_set_pixel_clock(unsigned int pixclock)
>   	pxclk = clamp_t(u32, pxclk, 2, 255);
>   
>   	/* Disable the pixel clock, and set it to non-inverted and no delay */
> -	clrbits32(&guts->clkdvdr,
> +	clrbits_be32(&guts->clkdvdr,
>   		  CLKDVDR_PXCKEN | CLKDVDR_PXCKDLY | CLKDVDR_PXCLK_MASK);
>   
>   	/* Enable the clock and set the pxclk */
> -	setbits32(&guts->clkdvdr, CLKDVDR_PXCKEN | (pxclk << 16));
> +	setbits_be32(&guts->clkdvdr, CLKDVDR_PXCKEN | (pxclk << 16));
>   
>   	iounmap(guts);
>   }
> diff --git a/arch/powerpc/platforms/85xx/t1042rdb_diu.c b/arch/powerpc/platforms/85xx/t1042rdb_diu.c
> index dac36ba82fea..e78970d2b292 100644
> --- a/arch/powerpc/platforms/85xx/t1042rdb_diu.c
> +++ b/arch/powerpc/platforms/85xx/t1042rdb_diu.c
> @@ -114,11 +114,11 @@ static void t1042rdb_set_pixel_clock(unsigned int pixclock)
>   	pxclk = clamp_t(u32, pxclk, 2, 255);
>   
>   	/* Disable the pixel clock, and set it to non-inverted and no delay */
> -	clrbits32(scfg + CCSR_SCFG_PIXCLKCR,
> +	clrbits_be32(scfg + CCSR_SCFG_PIXCLKCR,
>   		  PIXCLKCR_PXCKEN | PIXCLKCR_PXCKDLY | PIXCLKCR_PXCLK_MASK);
>   
>   	/* Enable the clock and set the pxclk */
> -	setbits32(scfg + CCSR_SCFG_PIXCLKCR, PIXCLKCR_PXCKEN | (pxclk << 16));
> +	setbits_be32(scfg + CCSR_SCFG_PIXCLKCR, PIXCLKCR_PXCKEN | (pxclk << 16));
>   
>   	iounmap(scfg);
>   }
> diff --git a/arch/powerpc/platforms/85xx/twr_p102x.c b/arch/powerpc/platforms/85xx/twr_p102x.c
> index 360f6253e9ff..5c385ebf6cac 100644
> --- a/arch/powerpc/platforms/85xx/twr_p102x.c
> +++ b/arch/powerpc/platforms/85xx/twr_p102x.c
> @@ -95,7 +95,7 @@ static void __init twr_p1025_setup_arch(void)
>   			 * and QE12 for QE MII management signals in PMUXCR
>   			 * register.
>   			 * Set QE mux bits in PMUXCR */
> -			setbits32(&guts->pmuxcr, MPC85xx_PMUXCR_QE(0) |
> +			setbits_be32(&guts->pmuxcr, MPC85xx_PMUXCR_QE(0) |
>   					MPC85xx_PMUXCR_QE(3) |
>   					MPC85xx_PMUXCR_QE(9) |
>   					MPC85xx_PMUXCR_QE(12));
> diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> index a5d73fabe4d1..8b6c12f634c4 100644
> --- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> +++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
> @@ -261,11 +261,11 @@ void mpc8610hpcd_set_pixel_clock(unsigned int pixclock)
>   	pxclk = clamp_t(u32, pxclk, 2, 31);
>   
>   	/* Disable the pixel clock, and set it to non-inverted and no delay */
> -	clrbits32(&guts->clkdvdr,
> +	clrbits_be32(&guts->clkdvdr,
>   		  CLKDVDR_PXCKEN | CLKDVDR_PXCKDLY | CLKDVDR_PXCLK_MASK);
>   
>   	/* Enable the clock and set the pxclk */
> -	setbits32(&guts->clkdvdr, CLKDVDR_PXCKEN | (pxclk << 16));
> +	setbits_be32(&guts->clkdvdr, CLKDVDR_PXCKEN | (pxclk << 16));
>   
>   	iounmap(guts);
>   }
> diff --git a/arch/powerpc/platforms/8xx/adder875.c b/arch/powerpc/platforms/8xx/adder875.c
> index bcef9f66191e..7bfae1617cfa 100644
> --- a/arch/powerpc/platforms/8xx/adder875.c
> +++ b/arch/powerpc/platforms/8xx/adder875.c
> @@ -77,7 +77,7 @@ static void __init init_ioports(void)
>   	cpm1_clk_setup(CPM_CLK_SMC1, CPM_BRG1, CPM_CLK_RTX);
>   
>   	/* Set FEC1 and FEC2 to MII mode */
> -	clrbits32(&mpc8xx_immr->im_cpm.cp_cptr, 0x00000180);
> +	clrbits_be32(&mpc8xx_immr->im_cpm.cp_cptr, 0x00000180);
>   }
>   
>   static void __init adder875_setup(void)
> diff --git a/arch/powerpc/platforms/8xx/m8xx_setup.c b/arch/powerpc/platforms/8xx/m8xx_setup.c
> index 027c42d8966c..355b2974123d 100644
> --- a/arch/powerpc/platforms/8xx/m8xx_setup.c
> +++ b/arch/powerpc/platforms/8xx/m8xx_setup.c
> @@ -103,7 +103,7 @@ void __init mpc8xx_calibrate_decr(void)
>   
>   	/* Force all 8xx processors to use divide by 16 processor clock. */
>   	clk_r2 = immr_map(im_clkrst);
> -	setbits32(&clk_r2->car_sccr, 0x02000000);
> +	setbits_be32(&clk_r2->car_sccr, 0x02000000);
>   	immr_unmap(clk_r2);
>   
>   	/* Processor frequency is MHz.
> @@ -203,7 +203,7 @@ void __noreturn mpc8xx_restart(char *cmd)
>   
>   	local_irq_disable();
>   
> -	setbits32(&clk_r->car_plprcr, 0x00000080);
> +	setbits_be32(&clk_r->car_plprcr, 0x00000080);
>   	/* Clear the ME bit in MSR to cause checkstop on machine check
>   	*/
>   	mtmsr(mfmsr() & ~0x1000);
> diff --git a/arch/powerpc/platforms/8xx/mpc86xads_setup.c b/arch/powerpc/platforms/8xx/mpc86xads_setup.c
> index 8d02f5ff4481..88c611ecee0a 100644
> --- a/arch/powerpc/platforms/8xx/mpc86xads_setup.c
> +++ b/arch/powerpc/platforms/8xx/mpc86xads_setup.c
> @@ -87,7 +87,7 @@ static void __init init_ioports(void)
>   	cpm1_clk_setup(CPM_CLK_SCC1, CPM_CLK2, CPM_CLK_RX);
>   
>   	/* Set FEC1 and FEC2 to MII mode */
> -	clrbits32(&mpc8xx_immr->im_cpm.cp_cptr, 0x00000180);
> +	clrbits_be32(&mpc8xx_immr->im_cpm.cp_cptr, 0x00000180);
>   }
>   
>   static void __init mpc86xads_setup_arch(void)
> @@ -112,7 +112,7 @@ static void __init mpc86xads_setup_arch(void)
>   		return;
>   	}
>   
> -	clrbits32(bcsr_io, BCSR1_RS232EN_1 | BCSR1_RS232EN_2 | BCSR1_ETHEN);
> +	clrbits_be32(bcsr_io, BCSR1_RS232EN_1 | BCSR1_RS232EN_2 | BCSR1_ETHEN);
>   	iounmap(bcsr_io);
>   }
>   
> diff --git a/arch/powerpc/platforms/8xx/mpc885ads_setup.c b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
> index a0c83c1905c6..17e10250830b 100644
> --- a/arch/powerpc/platforms/8xx/mpc885ads_setup.c
> +++ b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
> @@ -123,7 +123,7 @@ static void __init init_ioports(void)
>   	cpm1_clk_setup(CPM_CLK_SCC3, CPM_CLK6, CPM_CLK_RX);
>   
>   	/* Set FEC1 and FEC2 to MII mode */
> -	clrbits32(&mpc8xx_immr->im_cpm.cp_cptr, 0x00000180);
> +	clrbits_be32(&mpc8xx_immr->im_cpm.cp_cptr, 0x00000180);
>   }
>   
>   static void __init mpc885ads_setup_arch(void)
> @@ -148,33 +148,33 @@ static void __init mpc885ads_setup_arch(void)
>   		return;
>   	}
>   
> -	clrbits32(&bcsr[1], BCSR1_RS232EN_1);
> +	clrbits_be32(&bcsr[1], BCSR1_RS232EN_1);
>   #ifdef CONFIG_MPC8xx_SECOND_ETH_FEC2
> -	setbits32(&bcsr[1], BCSR1_RS232EN_2);
> +	setbits_be32(&bcsr[1], BCSR1_RS232EN_2);
>   #else
> -	clrbits32(&bcsr[1], BCSR1_RS232EN_2);
> +	clrbits_be32(&bcsr[1], BCSR1_RS232EN_2);
>   #endif
>   
> -	clrbits32(bcsr5, BCSR5_MII1_EN);
> -	setbits32(bcsr5, BCSR5_MII1_RST);
> +	clrbits_be32(bcsr5, BCSR5_MII1_EN);
> +	setbits_be32(bcsr5, BCSR5_MII1_RST);
>   	udelay(1000);
> -	clrbits32(bcsr5, BCSR5_MII1_RST);
> +	clrbits_be32(bcsr5, BCSR5_MII1_RST);
>   
>   #ifdef CONFIG_MPC8xx_SECOND_ETH_FEC2
> -	clrbits32(bcsr5, BCSR5_MII2_EN);
> -	setbits32(bcsr5, BCSR5_MII2_RST);
> +	clrbits_be32(bcsr5, BCSR5_MII2_EN);
> +	setbits_be32(bcsr5, BCSR5_MII2_RST);
>   	udelay(1000);
> -	clrbits32(bcsr5, BCSR5_MII2_RST);
> +	clrbits_be32(bcsr5, BCSR5_MII2_RST);
>   #else
> -	setbits32(bcsr5, BCSR5_MII2_EN);
> +	setbits_be32(bcsr5, BCSR5_MII2_EN);
>   #endif
>   
>   #ifdef CONFIG_MPC8xx_SECOND_ETH_SCC3
> -	clrbits32(&bcsr[4], BCSR4_ETH10_RST);
> +	clrbits_be32(&bcsr[4], BCSR4_ETH10_RST);
>   	udelay(1000);
> -	setbits32(&bcsr[4], BCSR4_ETH10_RST);
> +	setbits_be32(&bcsr[4], BCSR4_ETH10_RST);
>   
> -	setbits32(&bcsr[1], BCSR1_ETHEN);
> +	setbits_be32(&bcsr[1], BCSR1_ETHEN);
>   
>   	np = of_find_node_by_path("/soc@ff000000/cpm@9c0/serial@a80");
>   #else
> diff --git a/arch/powerpc/platforms/embedded6xx/flipper-pic.c b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
> index db0be007fd06..658f972d277a 100644
> --- a/arch/powerpc/platforms/embedded6xx/flipper-pic.c
> +++ b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
> @@ -53,7 +53,7 @@ static void flipper_pic_mask_and_ack(struct irq_data *d)
>   	void __iomem *io_base = irq_data_get_irq_chip_data(d);
>   	u32 mask = 1 << irq;
>   
> -	clrbits32(io_base + FLIPPER_IMR, mask);
> +	clrbits_be32(io_base + FLIPPER_IMR, mask);
>   	/* this is at least needed for RSW */
>   	out_be32(io_base + FLIPPER_ICR, mask);
>   }
> @@ -72,7 +72,7 @@ static void flipper_pic_mask(struct irq_data *d)
>   	int irq = irqd_to_hwirq(d);
>   	void __iomem *io_base = irq_data_get_irq_chip_data(d);
>   
> -	clrbits32(io_base + FLIPPER_IMR, 1 << irq);
> +	clrbits_be32(io_base + FLIPPER_IMR, 1 << irq);
>   }
>   
>   static void flipper_pic_unmask(struct irq_data *d)
> @@ -80,7 +80,7 @@ static void flipper_pic_unmask(struct irq_data *d)
>   	int irq = irqd_to_hwirq(d);
>   	void __iomem *io_base = irq_data_get_irq_chip_data(d);
>   
> -	setbits32(io_base + FLIPPER_IMR, 1 << irq);
> +	setbits_be32(io_base + FLIPPER_IMR, 1 << irq);
>   }
>   
>   
> diff --git a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
> index 8112b39879d6..a5431ad4a529 100644
> --- a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
> +++ b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
> @@ -50,7 +50,7 @@ static void hlwd_pic_mask_and_ack(struct irq_data *d)
>   	void __iomem *io_base = irq_data_get_irq_chip_data(d);
>   	u32 mask = 1 << irq;
>   
> -	clrbits32(io_base + HW_BROADWAY_IMR, mask);
> +	clrbits_be32(io_base + HW_BROADWAY_IMR, mask);
>   	out_be32(io_base + HW_BROADWAY_ICR, mask);
>   }
>   
> @@ -67,7 +67,7 @@ static void hlwd_pic_mask(struct irq_data *d)
>   	int irq = irqd_to_hwirq(d);
>   	void __iomem *io_base = irq_data_get_irq_chip_data(d);
>   
> -	clrbits32(io_base + HW_BROADWAY_IMR, 1 << irq);
> +	clrbits_be32(io_base + HW_BROADWAY_IMR, 1 << irq);
>   }
>   
>   static void hlwd_pic_unmask(struct irq_data *d)
> @@ -75,10 +75,10 @@ static void hlwd_pic_unmask(struct irq_data *d)
>   	int irq = irqd_to_hwirq(d);
>   	void __iomem *io_base = irq_data_get_irq_chip_data(d);
>   
> -	setbits32(io_base + HW_BROADWAY_IMR, 1 << irq);
> +	setbits_be32(io_base + HW_BROADWAY_IMR, 1 << irq);
>   
>   	/* Make sure the ARM (aka. Starlet) doesn't handle this interrupt. */
> -	clrbits32(io_base + HW_STARLET_IMR, 1 << irq);
> +	clrbits_be32(io_base + HW_STARLET_IMR, 1 << irq);
>   }
>   
>   
> diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/powerpc/platforms/embedded6xx/wii.c
> index 403523c061ba..f3dd1889d303 100644
> --- a/arch/powerpc/platforms/embedded6xx/wii.c
> +++ b/arch/powerpc/platforms/embedded6xx/wii.c
> @@ -134,7 +134,7 @@ static void __init wii_setup_arch(void)
>   	hw_gpio = wii_ioremap_hw_regs("hw_gpio", HW_GPIO_COMPATIBLE);
>   	if (hw_gpio) {
>   		/* turn off the front blue led and IR light */
> -		clrbits32(hw_gpio + HW_GPIO_OUT(0),
> +		clrbits_be32(hw_gpio + HW_GPIO_OUT(0),
>   			  HW_GPIO_SLOT_LED | HW_GPIO_SENSOR_BAR);
>   	}
>   }
> @@ -145,7 +145,7 @@ static void __noreturn wii_restart(char *cmd)
>   
>   	if (hw_ctrl) {
>   		/* clear the system reset pin to cause a reset */
> -		clrbits32(hw_ctrl + HW_CTRL_RESETS, HW_CTRL_RESETS_SYS);
> +		clrbits_be32(hw_ctrl + HW_CTRL_RESETS, HW_CTRL_RESETS_SYS);
>   	}
>   	wii_spin();
>   }
> @@ -159,13 +159,13 @@ static void wii_power_off(void)
>   		 * set the owner of the shutdown pin to ARM, because it is
>   		 * accessed through the registers for the ARM, below
>   		 */
> -		clrbits32(hw_gpio + HW_GPIO_OWNER, HW_GPIO_SHUTDOWN);
> +		clrbits_be32(hw_gpio + HW_GPIO_OWNER, HW_GPIO_SHUTDOWN);
>   
>   		/* make sure that the poweroff GPIO is configured as output */
> -		setbits32(hw_gpio + HW_GPIO_DIR(1), HW_GPIO_SHUTDOWN);
> +		setbits_be32(hw_gpio + HW_GPIO_DIR(1), HW_GPIO_SHUTDOWN);
>   
>   		/* drive the poweroff GPIO high */
> -		setbits32(hw_gpio + HW_GPIO_OUT(1), HW_GPIO_SHUTDOWN);
> +		setbits_be32(hw_gpio + HW_GPIO_OUT(1), HW_GPIO_SHUTDOWN);
>   	}
>   	wii_spin();
>   }
> diff --git a/arch/powerpc/sysdev/cpm1.c b/arch/powerpc/sysdev/cpm1.c
> index 4f8dcf124828..7fcbf8c059eb 100644
> --- a/arch/powerpc/sysdev/cpm1.c
> +++ b/arch/powerpc/sysdev/cpm1.c
> @@ -60,14 +60,14 @@ static void cpm_mask_irq(struct irq_data *d)
>   {
>   	unsigned int cpm_vec = (unsigned int)irqd_to_hwirq(d);
>   
> -	clrbits32(&cpic_reg->cpic_cimr, (1 << cpm_vec));
> +	clrbits_be32(&cpic_reg->cpic_cimr, (1 << cpm_vec));
>   }
>   
>   static void cpm_unmask_irq(struct irq_data *d)
>   {
>   	unsigned int cpm_vec = (unsigned int)irqd_to_hwirq(d);
>   
> -	setbits32(&cpic_reg->cpic_cimr, (1 << cpm_vec));
> +	setbits_be32(&cpic_reg->cpic_cimr, (1 << cpm_vec));
>   }
>   
>   static void cpm_end_irq(struct irq_data *d)
> @@ -188,7 +188,7 @@ unsigned int cpm_pic_init(void)
>   	if (setup_irq(eirq, &cpm_error_irqaction))
>   		printk(KERN_ERR "Could not allocate CPM error IRQ!");
>   
> -	setbits32(&cpic_reg->cpic_cicr, CICR_IEN);
> +	setbits_be32(&cpic_reg->cpic_cicr, CICR_IEN);
>   
>   end:
>   	of_node_put(np);
> @@ -317,14 +317,14 @@ static void cpm1_set_pin32(int port, int pin, int flags)
>   		      &mpc8xx_immr->im_cpm.cp_pedir;
>   
>   	if (flags & CPM_PIN_OUTPUT)
> -		setbits32(&iop->dir, pin);
> +		setbits_be32(&iop->dir, pin);
>   	else
> -		clrbits32(&iop->dir, pin);
> +		clrbits_be32(&iop->dir, pin);
>   
>   	if (!(flags & CPM_PIN_GPIO))
> -		setbits32(&iop->par, pin);
> +		setbits_be32(&iop->par, pin);
>   	else
> -		clrbits32(&iop->par, pin);
> +		clrbits_be32(&iop->par, pin);
>   
>   	if (port == CPM_PORTB) {
>   		if (flags & CPM_PIN_OPENDRAIN)
> @@ -335,14 +335,14 @@ static void cpm1_set_pin32(int port, int pin, int flags)
>   
>   	if (port == CPM_PORTE) {
>   		if (flags & CPM_PIN_SECONDARY)
> -			setbits32(&iop->sor, pin);
> +			setbits_be32(&iop->sor, pin);
>   		else
> -			clrbits32(&iop->sor, pin);
> +			clrbits_be32(&iop->sor, pin);
>   
>   		if (flags & CPM_PIN_OPENDRAIN)
> -			setbits32(&mpc8xx_immr->im_cpm.cp_peodr, pin);
> +			setbits_be32(&mpc8xx_immr->im_cpm.cp_peodr, pin);
>   		else
> -			clrbits32(&mpc8xx_immr->im_cpm.cp_peodr, pin);
> +			clrbits_be32(&mpc8xx_immr->im_cpm.cp_peodr, pin);
>   	}
>   }
>   
> @@ -732,7 +732,7 @@ static int cpm1_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>   
>   	spin_lock_irqsave(&cpm1_gc->lock, flags);
>   
> -	setbits32(&iop->dir, pin_mask);
> +	setbits_be32(&iop->dir, pin_mask);
>   	__cpm1_gpio32_set(mm_gc, pin_mask, val);
>   
>   	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
> @@ -750,7 +750,7 @@ static int cpm1_gpio32_dir_in(struct gpio_chip *gc, unsigned int gpio)
>   
>   	spin_lock_irqsave(&cpm1_gc->lock, flags);
>   
> -	clrbits32(&iop->dir, pin_mask);
> +	clrbits_be32(&iop->dir, pin_mask);
>   
>   	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
>   
> diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c
> index 07718b9a2c99..e8c7a0117eed 100644
> --- a/arch/powerpc/sysdev/cpm2.c
> +++ b/arch/powerpc/sysdev/cpm2.c
> @@ -335,22 +335,22 @@ void cpm2_set_pin(int port, int pin, int flags)
>   	pin = 1 << (31 - pin);
>   
>   	if (flags & CPM_PIN_OUTPUT)
> -		setbits32(&iop[port].dir, pin);
> +		setbits_be32(&iop[port].dir, pin);
>   	else
> -		clrbits32(&iop[port].dir, pin);
> +		clrbits_be32(&iop[port].dir, pin);
>   
>   	if (!(flags & CPM_PIN_GPIO))
> -		setbits32(&iop[port].par, pin);
> +		setbits_be32(&iop[port].par, pin);
>   	else
> -		clrbits32(&iop[port].par, pin);
> +		clrbits_be32(&iop[port].par, pin);
>   
>   	if (flags & CPM_PIN_SECONDARY)
> -		setbits32(&iop[port].sor, pin);
> +		setbits_be32(&iop[port].sor, pin);
>   	else
> -		clrbits32(&iop[port].sor, pin);
> +		clrbits_be32(&iop[port].sor, pin);
>   
>   	if (flags & CPM_PIN_OPENDRAIN)
> -		setbits32(&iop[port].odr, pin);
> +		setbits_be32(&iop[port].odr, pin);
>   	else
> -		clrbits32(&iop[port].odr, pin);
> +		clrbits_be32(&iop[port].odr, pin);
>   }
> diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
> index b74508175b67..8f4fba3067c9 100644
> --- a/arch/powerpc/sysdev/cpm_common.c
> +++ b/arch/powerpc/sysdev/cpm_common.c
> @@ -165,7 +165,7 @@ static int cpm2_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>   
>   	spin_lock_irqsave(&cpm2_gc->lock, flags);
>   
> -	setbits32(&iop->dir, pin_mask);
> +	setbits_be32(&iop->dir, pin_mask);
>   	__cpm2_gpio32_set(mm_gc, pin_mask, val);
>   
>   	spin_unlock_irqrestore(&cpm2_gc->lock, flags);
> @@ -183,7 +183,7 @@ static int cpm2_gpio32_dir_in(struct gpio_chip *gc, unsigned int gpio)
>   
>   	spin_lock_irqsave(&cpm2_gc->lock, flags);
>   
> -	clrbits32(&iop->dir, pin_mask);
> +	clrbits_be32(&iop->dir, pin_mask);
>   
>   	spin_unlock_irqrestore(&cpm2_gc->lock, flags);
>   
> diff --git a/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c b/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
> index c27058e5df26..2a99e56d3c7d 100644
> --- a/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
> +++ b/arch/powerpc/sysdev/fsl_85xx_l2ctlr.c
> @@ -124,23 +124,23 @@ static int mpc85xx_l2ctlr_of_probe(struct platform_device *dev)
>   
>   	switch (ways) {
>   	case LOCK_WAYS_EIGHTH:
> -		setbits32(&l2ctlr->ctl,
> +		setbits_be32(&l2ctlr->ctl,
>   			L2CR_L2E | L2CR_L2FI | L2CR_SRAM_EIGHTH);
>   		break;
>   
>   	case LOCK_WAYS_TWO_EIGHTH:
> -		setbits32(&l2ctlr->ctl,
> +		setbits_be32(&l2ctlr->ctl,
>   			L2CR_L2E | L2CR_L2FI | L2CR_SRAM_QUART);
>   		break;
>   
>   	case LOCK_WAYS_HALF:
> -		setbits32(&l2ctlr->ctl,
> +		setbits_be32(&l2ctlr->ctl,
>   			L2CR_L2E | L2CR_L2FI | L2CR_SRAM_HALF);
>   		break;
>   
>   	case LOCK_WAYS_FULL:
>   	default:
> -		setbits32(&l2ctlr->ctl,
> +		setbits_be32(&l2ctlr->ctl,
>   			L2CR_L2E | L2CR_L2FI | L2CR_SRAM_FULL);
>   		break;
>   	}
> diff --git a/arch/powerpc/sysdev/fsl_lbc.c b/arch/powerpc/sysdev/fsl_lbc.c
> index 5340a483cf55..0264f8c67a96 100644
> --- a/arch/powerpc/sysdev/fsl_lbc.c
> +++ b/arch/powerpc/sysdev/fsl_lbc.c
> @@ -192,7 +192,7 @@ static int fsl_lbc_ctrl_init(struct fsl_lbc_ctrl *ctrl,
>   	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
>   
>   	/* clear event registers */
> -	setbits32(&lbc->ltesr, LTESR_CLEAR);
> +	setbits_be32(&lbc->ltesr, LTESR_CLEAR);
>   	out_be32(&lbc->lteatr, 0);
>   	out_be32(&lbc->ltear, 0);
>   	out_be32(&lbc->lteccr, LTECCR_CLEAR);
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 918be816b097..e8488c9c284c 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -1196,11 +1196,11 @@ static int fsl_pci_pme_probe(struct pci_controller *hose)
>   	pci = hose->private_data;
>   
>   	/* Enable PTOD, ENL23D & EXL23D */
> -	clrbits32(&pci->pex_pme_mes_disr,
> +	clrbits_be32(&pci->pex_pme_mes_disr,
>   		  PME_DISR_EN_PTOD | PME_DISR_EN_ENL23D | PME_DISR_EN_EXL23D);
>   
>   	out_be32(&pci->pex_pme_mes_ier, 0);
> -	setbits32(&pci->pex_pme_mes_ier,
> +	setbits_be32(&pci->pex_pme_mes_ier,
>   		  PME_DISR_EN_PTOD | PME_DISR_EN_ENL23D | PME_DISR_EN_EXL23D);
>   
>   	/* PME Enable */
> @@ -1218,7 +1218,7 @@ static void send_pme_turnoff_message(struct pci_controller *hose)
>   	int i;
>   
>   	/* Send PME_Turn_Off Message Request */
> -	setbits32(&pci->pex_pmcr, PEX_PMCR_PTOMR);
> +	setbits_be32(&pci->pex_pmcr, PEX_PMCR_PTOMR);
>   
>   	/* Wait trun off done */
>   	for (i = 0; i < 150; i++) {
> @@ -1254,7 +1254,7 @@ static void fsl_pci_syscore_do_resume(struct pci_controller *hose)
>   	int i;
>   
>   	/* Send Exit L2 State Message */
> -	setbits32(&pci->pex_pmcr, PEX_PMCR_EXL2S);
> +	setbits_be32(&pci->pex_pmcr, PEX_PMCR_EXL2S);
>   
>   	/* Wait exit done */
>   	for (i = 0; i < 150; i++) {
> diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
> index 232225e7f863..ff29fa6af01c 100644
> --- a/arch/powerpc/sysdev/fsl_pmc.c
> +++ b/arch/powerpc/sysdev/fsl_pmc.c
> @@ -37,7 +37,7 @@ static int pmc_suspend_enter(suspend_state_t state)
>   {
>   	int ret;
>   
> -	setbits32(&pmc_regs->pmcsr, PMCSR_SLP);
> +	setbits_be32(&pmc_regs->pmcsr, PMCSR_SLP);
>   	/* At this point, the CPU is asleep. */
>   
>   	/* Upon resume, wait for SLP bit to be clear. */
> diff --git a/arch/powerpc/sysdev/fsl_rcpm.c b/arch/powerpc/sysdev/fsl_rcpm.c
> index 9259a94f70e1..fce703c400e6 100644
> --- a/arch/powerpc/sysdev/fsl_rcpm.c
> +++ b/arch/powerpc/sysdev/fsl_rcpm.c
> @@ -33,10 +33,10 @@ static void rcpm_v1_irq_mask(int cpu)
>   	int hw_cpu = get_hard_smp_processor_id(cpu);
>   	unsigned int mask = 1 << hw_cpu;
>   
> -	setbits32(&rcpm_v1_regs->cpmimr, mask);
> -	setbits32(&rcpm_v1_regs->cpmcimr, mask);
> -	setbits32(&rcpm_v1_regs->cpmmcmr, mask);
> -	setbits32(&rcpm_v1_regs->cpmnmimr, mask);
> +	setbits_be32(&rcpm_v1_regs->cpmimr, mask);
> +	setbits_be32(&rcpm_v1_regs->cpmcimr, mask);
> +	setbits_be32(&rcpm_v1_regs->cpmmcmr, mask);
> +	setbits_be32(&rcpm_v1_regs->cpmnmimr, mask);
>   }
>   
>   static void rcpm_v2_irq_mask(int cpu)
> @@ -44,10 +44,10 @@ static void rcpm_v2_irq_mask(int cpu)
>   	int hw_cpu = get_hard_smp_processor_id(cpu);
>   	unsigned int mask = 1 << hw_cpu;
>   
> -	setbits32(&rcpm_v2_regs->tpmimr0, mask);
> -	setbits32(&rcpm_v2_regs->tpmcimr0, mask);
> -	setbits32(&rcpm_v2_regs->tpmmcmr0, mask);
> -	setbits32(&rcpm_v2_regs->tpmnmimr0, mask);
> +	setbits_be32(&rcpm_v2_regs->tpmimr0, mask);
> +	setbits_be32(&rcpm_v2_regs->tpmcimr0, mask);
> +	setbits_be32(&rcpm_v2_regs->tpmmcmr0, mask);
> +	setbits_be32(&rcpm_v2_regs->tpmnmimr0, mask);
>   }
>   
>   static void rcpm_v1_irq_unmask(int cpu)
> @@ -55,10 +55,10 @@ static void rcpm_v1_irq_unmask(int cpu)
>   	int hw_cpu = get_hard_smp_processor_id(cpu);
>   	unsigned int mask = 1 << hw_cpu;
>   
> -	clrbits32(&rcpm_v1_regs->cpmimr, mask);
> -	clrbits32(&rcpm_v1_regs->cpmcimr, mask);
> -	clrbits32(&rcpm_v1_regs->cpmmcmr, mask);
> -	clrbits32(&rcpm_v1_regs->cpmnmimr, mask);
> +	clrbits_be32(&rcpm_v1_regs->cpmimr, mask);
> +	clrbits_be32(&rcpm_v1_regs->cpmcimr, mask);
> +	clrbits_be32(&rcpm_v1_regs->cpmmcmr, mask);
> +	clrbits_be32(&rcpm_v1_regs->cpmnmimr, mask);
>   }
>   
>   static void rcpm_v2_irq_unmask(int cpu)
> @@ -66,26 +66,26 @@ static void rcpm_v2_irq_unmask(int cpu)
>   	int hw_cpu = get_hard_smp_processor_id(cpu);
>   	unsigned int mask = 1 << hw_cpu;
>   
> -	clrbits32(&rcpm_v2_regs->tpmimr0, mask);
> -	clrbits32(&rcpm_v2_regs->tpmcimr0, mask);
> -	clrbits32(&rcpm_v2_regs->tpmmcmr0, mask);
> -	clrbits32(&rcpm_v2_regs->tpmnmimr0, mask);
> +	clrbits_be32(&rcpm_v2_regs->tpmimr0, mask);
> +	clrbits_be32(&rcpm_v2_regs->tpmcimr0, mask);
> +	clrbits_be32(&rcpm_v2_regs->tpmmcmr0, mask);
> +	clrbits_be32(&rcpm_v2_regs->tpmnmimr0, mask);
>   }
>   
>   static void rcpm_v1_set_ip_power(bool enable, u32 mask)
>   {
>   	if (enable)
> -		setbits32(&rcpm_v1_regs->ippdexpcr, mask);
> +		setbits_be32(&rcpm_v1_regs->ippdexpcr, mask);
>   	else
> -		clrbits32(&rcpm_v1_regs->ippdexpcr, mask);
> +		clrbits_be32(&rcpm_v1_regs->ippdexpcr, mask);
>   }
>   
>   static void rcpm_v2_set_ip_power(bool enable, u32 mask)
>   {
>   	if (enable)
> -		setbits32(&rcpm_v2_regs->ippdexpcr[0], mask);
> +		setbits_be32(&rcpm_v2_regs->ippdexpcr[0], mask);
>   	else
> -		clrbits32(&rcpm_v2_regs->ippdexpcr[0], mask);
> +		clrbits_be32(&rcpm_v2_regs->ippdexpcr[0], mask);
>   }
>   
>   static void rcpm_v1_cpu_enter_state(int cpu, int state)
> @@ -95,10 +95,10 @@ static void rcpm_v1_cpu_enter_state(int cpu, int state)
>   
>   	switch (state) {
>   	case E500_PM_PH10:
> -		setbits32(&rcpm_v1_regs->cdozcr, mask);
> +		setbits_be32(&rcpm_v1_regs->cdozcr, mask);
>   		break;
>   	case E500_PM_PH15:
> -		setbits32(&rcpm_v1_regs->cnapcr, mask);
> +		setbits_be32(&rcpm_v1_regs->cnapcr, mask);
>   		break;
>   	default:
>   		pr_warn("Unknown cpu PM state (%d)\n", state);
> @@ -114,16 +114,16 @@ static void rcpm_v2_cpu_enter_state(int cpu, int state)
>   	switch (state) {
>   	case E500_PM_PH10:
>   		/* one bit corresponds to one thread for PH10 of 6500 */
> -		setbits32(&rcpm_v2_regs->tph10setr0, 1 << hw_cpu);
> +		setbits_be32(&rcpm_v2_regs->tph10setr0, 1 << hw_cpu);
>   		break;
>   	case E500_PM_PH15:
> -		setbits32(&rcpm_v2_regs->pcph15setr, mask);
> +		setbits_be32(&rcpm_v2_regs->pcph15setr, mask);
>   		break;
>   	case E500_PM_PH20:
> -		setbits32(&rcpm_v2_regs->pcph20setr, mask);
> +		setbits_be32(&rcpm_v2_regs->pcph20setr, mask);
>   		break;
>   	case E500_PM_PH30:
> -		setbits32(&rcpm_v2_regs->pcph30setr, mask);
> +		setbits_be32(&rcpm_v2_regs->pcph30setr, mask);
>   		break;
>   	default:
>   		pr_warn("Unknown cpu PM state (%d)\n", state);
> @@ -172,10 +172,10 @@ static void rcpm_v1_cpu_exit_state(int cpu, int state)
>   
>   	switch (state) {
>   	case E500_PM_PH10:
> -		clrbits32(&rcpm_v1_regs->cdozcr, mask);
> +		clrbits_be32(&rcpm_v1_regs->cdozcr, mask);
>   		break;
>   	case E500_PM_PH15:
> -		clrbits32(&rcpm_v1_regs->cnapcr, mask);
> +		clrbits_be32(&rcpm_v1_regs->cnapcr, mask);
>   		break;
>   	default:
>   		pr_warn("Unknown cpu PM state (%d)\n", state);
> @@ -196,16 +196,16 @@ static void rcpm_v2_cpu_exit_state(int cpu, int state)
>   
>   	switch (state) {
>   	case E500_PM_PH10:
> -		setbits32(&rcpm_v2_regs->tph10clrr0, 1 << hw_cpu);
> +		setbits_be32(&rcpm_v2_regs->tph10clrr0, 1 << hw_cpu);
>   		break;
>   	case E500_PM_PH15:
> -		setbits32(&rcpm_v2_regs->pcph15clrr, mask);
> +		setbits_be32(&rcpm_v2_regs->pcph15clrr, mask);
>   		break;
>   	case E500_PM_PH20:
> -		setbits32(&rcpm_v2_regs->pcph20clrr, mask);
> +		setbits_be32(&rcpm_v2_regs->pcph20clrr, mask);
>   		break;
>   	case E500_PM_PH30:
> -		setbits32(&rcpm_v2_regs->pcph30clrr, mask);
> +		setbits_be32(&rcpm_v2_regs->pcph30clrr, mask);
>   		break;
>   	default:
>   		pr_warn("Unknown cpu PM state (%d)\n", state);
> @@ -226,7 +226,7 @@ static int rcpm_v1_plat_enter_state(int state)
>   
>   	switch (state) {
>   	case PLAT_PM_SLEEP:
> -		setbits32(pmcsr_reg, RCPM_POWMGTCSR_SLP);
> +		setbits_be32(pmcsr_reg, RCPM_POWMGTCSR_SLP);
>   
>   		/* Upon resume, wait for RCPM_POWMGTCSR_SLP bit to be clear. */
>   		result = spin_event_timeout(
> @@ -253,9 +253,9 @@ static int rcpm_v2_plat_enter_state(int state)
>   	switch (state) {
>   	case PLAT_PM_LPM20:
>   		/* clear previous LPM20 status */
> -		setbits32(pmcsr_reg, RCPM_POWMGTCSR_P_LPM20_ST);
> +		setbits_be32(pmcsr_reg, RCPM_POWMGTCSR_P_LPM20_ST);
>   		/* enter LPM20 status */
> -		setbits32(pmcsr_reg, RCPM_POWMGTCSR_LPM20_RQ);
> +		setbits_be32(pmcsr_reg, RCPM_POWMGTCSR_LPM20_RQ);
>   
>   		/* At this point, the device is in LPM20 status. */
>   
> @@ -291,9 +291,9 @@ static void rcpm_common_freeze_time_base(u32 *tben_reg, int freeze)
>   
>   	if (freeze) {
>   		mask = in_be32(tben_reg);
> -		clrbits32(tben_reg, mask);
> +		clrbits_be32(tben_reg, mask);
>   	} else {
> -		setbits32(tben_reg, mask);
> +		setbits_be32(tben_reg, mask);
>   	}
>   
>   	/* read back to push the previous write */
> diff --git a/arch/powerpc/sysdev/fsl_rio.c b/arch/powerpc/sysdev/fsl_rio.c
> index 5011ffea4e4b..891e11d12222 100644
> --- a/arch/powerpc/sysdev/fsl_rio.c
> +++ b/arch/powerpc/sysdev/fsl_rio.c
> @@ -668,10 +668,10 @@ int fsl_rio_setup(struct platform_device *dev)
>   			out_be32(priv->regs_win
>   				+ RIO_CCSR + i*0x20, 0);
>   			/* Set 1x lane */
> -			setbits32(priv->regs_win
> +			setbits_be32(priv->regs_win
>   				+ RIO_CCSR + i*0x20, 0x02000000);
>   			/* Enable ports */
> -			setbits32(priv->regs_win
> +			setbits_be32(priv->regs_win
>   				+ RIO_CCSR + i*0x20, 0x00600000);
>   			msleep(100);
>   			if (in_be32((priv->regs_win
> diff --git a/arch/powerpc/sysdev/fsl_rmu.c b/arch/powerpc/sysdev/fsl_rmu.c
> index 88b35a3dcdc5..1a2567df9afc 100644
> --- a/arch/powerpc/sysdev/fsl_rmu.c
> +++ b/arch/powerpc/sysdev/fsl_rmu.c
> @@ -355,7 +355,7 @@ fsl_rio_dbell_handler(int irq, void *dev_instance)
>   				dmsg->sid, dmsg->tid,
>   				dmsg->info);
>   		}
> -		setbits32(&fsl_dbell->dbell_regs->dmr, DOORBELL_DMR_DI);
> +		setbits_be32(&fsl_dbell->dbell_regs->dmr, DOORBELL_DMR_DI);
>   		out_be32(&fsl_dbell->dbell_regs->dsr, DOORBELL_DSR_DIQI);
>   	}
>   
> @@ -909,10 +909,10 @@ fsl_open_inb_mbox(struct rio_mport *mport, void *dev_id, int mbox, int entries)
>   	out_be32(&rmu->msg_regs->imr, 0x001b0060);
>   
>   	/* Set number of queue entries */
> -	setbits32(&rmu->msg_regs->imr, (get_bitmask_order(entries) - 2) << 12);
> +	setbits_be32(&rmu->msg_regs->imr, (get_bitmask_order(entries) - 2) << 12);
>   
>   	/* Now enable the unit */
> -	setbits32(&rmu->msg_regs->imr, 0x1);
> +	setbits_be32(&rmu->msg_regs->imr, 0x1);
>   
>   out:
>   	return rc;
> @@ -1015,7 +1015,7 @@ void *fsl_get_inb_message(struct rio_mport *mport, int mbox)
>   	rmu->msg_rx_ring.virt_buffer[buf_idx] = NULL;
>   
>   out1:
> -	setbits32(&rmu->msg_regs->imr, RIO_MSG_IMR_MI);
> +	setbits_be32(&rmu->msg_regs->imr, RIO_MSG_IMR_MI);
>   
>   out2:
>   	return buf;
> diff --git a/arch/powerpc/sysdev/mpic_timer.c b/arch/powerpc/sysdev/mpic_timer.c
> index 87e7c42777a8..5cc8216a85e5 100644
> --- a/arch/powerpc/sysdev/mpic_timer.c
> +++ b/arch/powerpc/sysdev/mpic_timer.c
> @@ -154,7 +154,7 @@ static int set_cascade_timer(struct timer_group_priv *priv, u64 ticks,
>   
>   	tcr = casc_priv->tcr_value |
>   		(casc_priv->tcr_value << MPIC_TIMER_TCR_ROVR_OFFSET);
> -	setbits32(priv->group_tcr, tcr);
> +	setbits_be32(priv->group_tcr, tcr);
>   
>   	tmp_ticks = div_u64_rem(ticks, MAX_TICKS_CASCADE, &rem_ticks);
>   
> @@ -253,7 +253,7 @@ void mpic_start_timer(struct mpic_timer *handle)
>   	struct timer_group_priv *priv = container_of(handle,
>   			struct timer_group_priv, timer[handle->num]);
>   
> -	clrbits32(&priv->regs[handle->num].gtbcr, TIMER_STOP);
> +	clrbits_be32(&priv->regs[handle->num].gtbcr, TIMER_STOP);
>   }
>   EXPORT_SYMBOL(mpic_start_timer);
>   
> @@ -269,7 +269,7 @@ void mpic_stop_timer(struct mpic_timer *handle)
>   			struct timer_group_priv, timer[handle->num]);
>   	struct cascade_priv *casc_priv;
>   
> -	setbits32(&priv->regs[handle->num].gtbcr, TIMER_STOP);
> +	setbits_be32(&priv->regs[handle->num].gtbcr, TIMER_STOP);
>   
>   	casc_priv = priv->timer[handle->num].cascade_handle;
>   	if (casc_priv) {
> @@ -340,7 +340,7 @@ void mpic_free_timer(struct mpic_timer *handle)
>   		u32 tcr;
>   		tcr = casc_priv->tcr_value | (casc_priv->tcr_value <<
>   					MPIC_TIMER_TCR_ROVR_OFFSET);
> -		clrbits32(priv->group_tcr, tcr);
> +		clrbits_be32(priv->group_tcr, tcr);
>   		priv->idle |= casc_priv->cascade_map;
>   		priv->timer[handle->num].cascade_handle = NULL;
>   	} else {
> @@ -508,7 +508,7 @@ static void timer_group_init(struct device_node *np)
>   
>   	/* Init FSL timer hardware */
>   	if (priv->flags & FSL_GLOBAL_TIMER)
> -		setbits32(priv->group_tcr, MPIC_TIMER_TCR_CLKDIV);
> +		setbits_be32(priv->group_tcr, MPIC_TIMER_TCR_CLKDIV);
>   
>   	list_add_tail(&priv->node, &timer_group_list);
>   
> @@ -531,7 +531,7 @@ static void mpic_timer_resume(void)
>   	list_for_each_entry(priv, &timer_group_list, node) {
>   		/* Init FSL timer hardware */
>   		if (priv->flags & FSL_GLOBAL_TIMER)
> -			setbits32(priv->group_tcr, MPIC_TIMER_TCR_CLKDIV);
> +			setbits_be32(priv->group_tcr, MPIC_TIMER_TCR_CLKDIV);
>   	}
>   }
>   
> 

^ permalink raw reply

* Re: [PATCH] powerpc/traps: merge unrecoverable_exception() and nonrecoverable_exception()
From: Christophe LEROY @ 2018-09-25  4:43 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <875zyu5pr6.fsf@concordia.ellerman.id.au>



Le 25/09/2018 à 03:29, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> 
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 6ab66a88db14..4567eeb6524e 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -2090,8 +2082,9 @@ void SPEFloatingPointRoundException(struct pt_regs *regs)
>>    */
>>   void unrecoverable_exception(struct pt_regs *regs)
>>   {
>> -	printk(KERN_EMERG "Unrecoverable exception %lx at %lx\n",
>> -	       regs->trap, regs->nip);
>> +	pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
>> +		 regs->trap, regs->nip, regs->msr);
>> +	debugger(regs);
>>   	die("Unrecoverable exception", regs, SIGABRT);
> 
> die() already calls debugger() if the trap is != 0x100.
> 
> I don't think we want to call it twice?

Oh, good spot. So nonrecoverable_exception() is calling it twice.

I'll fix the patch. Thanks.

Christophe

^ permalink raw reply

* Call for speakers for the Devicetree Microconference @ the 2018 LPC in Vancouver
From: darknighte @ 2018-09-25  4:01 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Frank Rowand

The Devicetree Microconference has been accepted into the 2018 Linux
Plumbers Conference, which will be held in Vancouver, British
Columbia, Canada from Tuesday, November 13 through Thursday, November
15.  If you are interested in speaking, please contact myself, or
co-leader Frank Rowand (on CC), to request a slot.

For more details, here is the LPC announcement :

We are pleased to announce the the Devicetree Microconference
has been accepted into the 2018 Linux Plumbers Conference!

Devicetree provides hardware description for many platforms, such
as Linux [1], U-Boot [2], BSD [3], and Zephyr [4]. Devicetree
continues to evolve to become more robust and attempt to provide the
features desired by the varied users.

Some of the overlay related needs are now being addressed by
U-boot, but there remain use cases for run time overlay management in
the Linux kernel. Support for run time overlay management in the Linux
kernel is slowly moving forward, but significant issues remain [5].

Devicetree verification has been an ongoing project for several years,
with the most recent in person discussion occurring at the Devicetree
Workshop [6] at Kernel Summit 2017. Progress continues on mail lists,
and will be an important topic at the microconference.

Other Devicetree related tools, such as the dtc compiler and libfdt [7]
continue to see active development.

Additional possible issues to be discussed may include potential
changes to the Flattened Device Tree (FDT) format, reducing the
Devicetree memory and storage size in the Linux kernel, creating new
architecture to provide solutions to current problems, updating the
Devicetree Specification, and using devicetrees in constrained contexts.

LPC [8] will be held in Vancouver, British Columbia, Canada
from Tuesday, November 13 through Thursday, November 15.

[1] https://elinux.org/Device_Tree_Reference
[2] https://github.com/lentinj/u-boot/blob/master/doc/README.fdt-control
[3] https://wiki.freebsd.org/FlattenedDeviceTree
[4] http://docs.zephyrproject.org/devices/dts/device_tree.html
[5] https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
[6] https://elinux.org/Device_tree_future#Kernel_Summit_2017.2C_Devicetree_Workshop
[7] https://elinux.org/Device_Tree_Reference#dtc_.28upstream_project.29
[8] https://linuxplumbersconf.org/

^ permalink raw reply

* Re: [PATCH] powerpc/traps: merge unrecoverable_exception() and nonrecoverable_exception()
From: Michael Ellerman @ 2018-09-25  1:29 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <f49e2e4ff1eacbad161eb6c1645cca7a281dcf3e.1537765881.git.christophe.leroy@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 6ab66a88db14..4567eeb6524e 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -2090,8 +2082,9 @@ void SPEFloatingPointRoundException(struct pt_regs *regs)
>   */
>  void unrecoverable_exception(struct pt_regs *regs)
>  {
> -	printk(KERN_EMERG "Unrecoverable exception %lx at %lx\n",
> -	       regs->trap, regs->nip);
> +	pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
> +		 regs->trap, regs->nip, regs->msr);
> +	debugger(regs);
>  	die("Unrecoverable exception", regs, SIGABRT);

die() already calls debugger() if the trap is != 0x100.

I don't think we want to call it twice?

cheers

^ permalink raw reply

* Re: [PATCH RFCv2 3/6] mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock
From: Rashmica Gupta @ 2018-09-25  1:26 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm
  Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
	devel, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Rafael J. Wysocki, Len Brown, Greg Kroah-Hartman,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Martin Schwidefsky, Heiko Carstens, Boris Ostrovsky,
	Juergen Gross, Michael Neuling, Balbir Singh, Kate Stewart,
	Thomas Gleixner, Philippe Ombredanne, Andrew Morton, Michal Hocko,
	Pavel Tatashin, Vlastimil Babka, Dan Williams, Oscar Salvador,
	YASUAKI ISHIMATSU, Mathieu Malaterre
In-Reply-To: <5f80ca56-9f34-4e6e-bc83-8f8b3c888163@redhat.com>

On Mon, 2018-09-17 at 09:32 +0200, David Hildenbrand wrote:
> Am 03.09.18 um 02:36 schrieb Rashmica:
> > Hi David,
> > 
> > 
> > On 21/08/18 20:44, David Hildenbrand wrote:
> > 
> > > There seem to be some problems as result of 30467e0b3be ("mm,
> > > hotplug:
> > > fix concurrent memory hot-add deadlock"), which tried to fix a
> > > possible
> > > lock inversion reported and discussed in [1] due to the two locks
> > > 	a) device_lock()
> > > 	b) mem_hotplug_lock
> > > 
> > > While add_memory() first takes b), followed by a) during
> > > bus_probe_device(), onlining of memory from user space first took
> > > b),
> > > followed by a), exposing a possible deadlock.
> > 
> > Do you mean "onlining of memory from user space first took a),
> > followed by b)"? 
> 
> Very right, thanks.
> 
> > 
> > > In [1], and it was decided to not make use of
> > > device_hotplug_lock, but
> > > rather to enforce a locking order.
> > > 
> > > The problems I spotted related to this:
> > > 
> > > 1. Memory block device attributes: While .state first calls
> > >    mem_hotplug_begin() and the calls device_online() - which
> > > takes
> > >    device_lock() - .online does no longer call
> > > mem_hotplug_begin(), so
> > >    effectively calls online_pages() without mem_hotplug_lock.
> > > 
> > > 2. device_online() should be called under device_hotplug_lock,
> > > however
> > >    onlining memory during add_memory() does not take care of
> > > that.
> > > 
> > > In addition, I think there is also something wrong about the
> > > locking in
> > > 
> > > 3. arch/powerpc/platforms/powernv/memtrace.c calls
> > > offline_pages()
> > >    without locks. This was introduced after 30467e0b3be. And
> > > skimming over
> > >    the code, I assume it could need some more care in regards to
> > > locking
> > >    (e.g. device_online() called without device_hotplug_lock - but
> > > I'll
> > >    not touch that for now).
> > 
> > Can you mention that you fixed this in later patches?
> 
> Sure!
> 
> > 
> > 
> > The series looks good to me. Feel free to add my reviewed-by:
> > 
> > Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
> > 
> 
> Thanks, r-b only for this patch or all of the series?

Sorry, I somehow missed this. To all of the series.
> 

^ permalink raw reply

* Re: [PATCH] powerpc/pseries: Track LMB nid instead of using device tree
From: Michael Ellerman @ 2018-09-25  1:17 UTC (permalink / raw)
  To: Nathan Fontenot, linuxppc-dev
In-Reply-To: <514f0192-4403-4305-1c2b-41dc752bcd36@linux.vnet.ibm.com>

Nathan Fontenot <nfont@linux.vnet.ibm.com> writes:
> On 09/19/2018 11:38 PM, Michael Ellerman wrote:
>> Nathan Fontenot <nfont@linux.vnet.ibm.com> writes:
>>> When removing memory we need to remove the memory from the node
>>> it was added to instead of looking up the node it should be in
>>> in the device tree.
>>>
>>> During testing we have seen scenarios where the affinity for a
>>> LMB changes due to a partition migration or PRRN event. In these
>>> cases the node the LMB exists in may not match the node the device
>>> tree indicates it belongs in. This can lead to a system crash
>>> when trying to DLAPR remove the LMB after a migration or PRRN
>>> event. The current code looks up the node in the device tree to
>>> remove the LMB from, the crash occurs when we try to offline this
>>> node and it does not have any data, i.e. node_data[nid] == NULL.
>> 
>> This isn't building for 32-bit etc:
>> 
>> arch/powerpc/mm/drmem.c: In function 'init_drmem_v1_lmbs':
>> arch/powerpc/mm/drmem.c:371:14: error: implicit declaration of function 'memory_add_physaddr_to_nid' [-Werror=implicit-function-declaration]
>>    lmb->nid = memory_add_physaddr_to_nid(lmb->base_addr);
>>               ^
>> cc1: all warnings being treated as errors
>> scripts/Makefile.build:317: recipe for target 'arch/powerpc/mm/drmem.o' failed
>> 
>> See the failed checks here:
>>   https://patchwork.ozlabs.org/patch/969150/
>> 
>> 
>> Probably drmem.c should only be compiled for 64-bit NUMA etc.
>
> Looks like the root cause is that memory hotplug relies on sparsemem which
> is not supported on 32-bit.

Yeah that could be it.

Making drmem.c built just for MEMORY_HOTPLUG would make sense.

> This patch is also going to need a refresh to apply cleanly due to other
> patches that have gone in. I'll re-submit after looking at the build break issues more.

OK thanks.

cheers

^ permalink raw reply

* Re: How to define some additional KBUILD_CFLAGS after building include/generated/asm-offsets.h ?
From: Michael Ellerman @ 2018-09-25  1:16 UTC (permalink / raw)
  To: Christophe LEROY, linuxppc-dev, linux-kbuild, Masahiro Yamada,
	Michal Marek, Segher Boessenkool
  Cc: LKML
In-Reply-To: <56227965-464a-d972-1c25-d6a4d190e1a1@c-s.fr>

Christophe LEROY <christophe.leroy@c-s.fr> writes:

> Le 24/09/2018 =C3=A0 14:10, Michael Ellerman a =C3=A9crit=C2=A0:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>=20
>>> I'm trying to implement TLS based stack protector in the Linux Kernel.
>>> For that I need to give to GCC the offset at which it will find the
>>> canary (register r2 is pointing to the current task struct).
>>>
>>> I have been able to do it with the below patch, but it only works when
>>> include/generated/asm-offsets.h already exists from the start of the bu=
ild.
>>>
>>> Is there a way to evaluate CANARY_OFFSET and add the stack-protector
>>> flags to KBUILD_FLAGS only after include/generated/asm-offsets.h is bui=
lt ?
>>>
>>> Or another way of add -mstack-protector-guard-offset=3Doffsetof(struct
>>> task_struct, stack_canary) ?
>>=20
>> This seems to work, at least I see the value in CFLAGS:
>>=20
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 07d9dce..39ee113 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -404,6 +394,11 @@ archclean:
>>=20=20=20
>>   archprepare: checkbin
>>=20=20=20
>> +prepare: stack_protector_prepare
>> +
>> +stack_protector_prepare: prepare0
>> +	$(eval KBUILD_CFLAGS +=3D -mstack-protector-guard-offset=3D$(shell awk=
 '{if ($$2 =3D=3D "TSK_STACK_CANARY") print $$3;}' include/generated/asm-of=
fsets.h))
>> +
>
> Great, it works !
> Thanks, I have sent v3 of the patches.

Cool.

It would be good to here from someone who knows Kbuild better than me if
this is acceptable or just a gross hack :)

cheers

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox