LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 -next 0/9] CMA: generalize CMA reserved area management code
From: Marek Szyprowski @ 2014-06-25 12:33 UTC (permalink / raw)
  To: Andrew Morton, Joonsoo Kim
  Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
	Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
	linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
	Paolo Bonzini, Zhang Yanfei, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20140618135144.297c785260f9e2aebead867c@linux-foundation.org>

Hello,

On 2014-06-18 22:51, Andrew Morton wrote:
> On Tue, 17 Jun 2014 10:25:07 +0900 Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
>>>> v2:
>>>>    - Although this patchset looks very different with v1, the end result,
>>>>    that is, mm/cma.c is same with v1's one. So I carry Ack to patch 6-7.
>>>>
>>>> This patchset is based on linux-next 20140610.
>>> Thanks for taking care of this. I will test it with my setup and if
>>> everything goes well, I will take it to my -next tree. If any branch
>>> is required for anyone to continue his works on top of those patches,
>>> let me know, I will also prepare it.
>> Hello,
>>
>> I'm glad to hear that. :)
>> But, there is one concern. As you already know, I am preparing further
>> patches (Aggressively allocate the pages on CMA reserved memory). It
>> may be highly related to MM branch and also slightly depends on this CMA
>> changes. In this case, what is the best strategy to merge this
>> patchset? IMHO, Anrew's tree is more appropriate branch. If there is
>> no issue in this case, I am willing to develope further patches based
>> on your tree.
> That's probably easier.  Marek, I'll merge these into -mm (and hence
> -next and git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git)
> and shall hold them pending you review/ack/test/etc, OK?

Ok. I've tested them and they work fine. I'm sorry that you had to wait for
me for a few days. You can now add:

Acked-and-tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

I've also rebased my pending patches onto this set (I will send them soon).

The question is now if you want to keep the discussed patches in your 
-mm tree,
or should I take them to my -next branch. If you like to keep them, I assume
you will also take the patches which depends on the discussed changes.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply

* Re: [PATCH V2] KVM: PPC: BOOK3S: HV: Use base page size when comparing against slb value
From: Alexander Graf @ 2014-06-25 12:08 UTC (permalink / raw)
  To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1402858027-11296-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>


On 15.06.14 20:47, Aneesh Kumar K.V wrote:
> With guests supporting Multiple page size per segment (MPSS),
> hpte_page_size returns the actual page size used. Add a new function to
> return base page size and use that to compare against the the page size
> calculated from SLB. Without this patch a hpte lookup can fail since
> we are comparing wrong page size in kvmppc_hv_find_lock_hpte.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Thanks, applied to for-3.16.


Alex

^ permalink raw reply

* [PATCH v3 3/3] dmaengine: mpc512x: register for device tree channel lookup
From: Alexander Popov @ 2014-06-25 10:53 UTC (permalink / raw)
  To: Mark Rutland, Gerhard Sittig, Dan Williams, Vinod Koul,
	Lars-Peter Clausen, Arnd Bergmann, Anatolij Gustschin,
	Andy Shevchenko, Alexander Popov, linuxppc-dev, dmaengine,
	devicetree
In-Reply-To: <1403693580-9449-1-git-send-email-a13xp0p0v88@gmail.com>

Register the controller for device tree based lookup of DMA channels
(non-fatal for backwards compatibility with older device trees) and
provide the '#dma-cells' property in the shared mpc5121.dtsi file

Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
 arch/powerpc/boot/dts/mpc5121.dtsi |  1 +
 drivers/dma/mpc512x_dma.c          | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
index 2c0e155..7f9d14f 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -498,6 +498,7 @@
 			compatible = "fsl,mpc5121-dma";
 			reg = <0x14000 0x1800>;
 			interrupts = <65 0x8>;
+			#dma-cells = <1>;
 		};
 	};
 
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 2ad4373..881db2b 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -53,6 +53,7 @@
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
+#include <linux/of_dma.h>
 #include <linux/of_platform.h>
 
 #include <linux/random.h>
@@ -1036,7 +1037,15 @@ static int mpc_dma_probe(struct platform_device *op)
 	if (retval)
 		goto err_free2;
 
-	return retval;
+	/* Register with OF helpers for DMA lookups (nonfatal) */
+	if (dev->of_node) {
+		retval = of_dma_controller_register(dev->of_node,
+						of_dma_xlate_by_chan_id, mdma);
+		if (retval)
+			dev_warn(dev, "Could not register for OF lookup\n");
+	}
+
+	return 0;
 
 err_free2:
 	if (mdma->is_mpc8308)
@@ -1057,6 +1066,8 @@ static int mpc_dma_remove(struct platform_device *op)
 	struct device *dev = &op->dev;
 	struct mpc_dma *mdma = dev_get_drvdata(dev);
 
+	if (dev->of_node)
+		of_dma_controller_free(dev->of_node);
 	dma_async_device_unregister(&mdma->dma);
 	if (mdma->is_mpc8308) {
 		free_irq(mdma->irq2, mdma);
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH v3 2/3] dmaengine: of: add common xlate function for matching by channel id
From: Alexander Popov @ 2014-06-25 10:52 UTC (permalink / raw)
  To: Mark Rutland, Gerhard Sittig, Dan Williams, Vinod Koul,
	Lars-Peter Clausen, Arnd Bergmann, Anatolij Gustschin,
	Andy Shevchenko, Alexander Popov, linuxppc-dev, dmaengine,
	devicetree
In-Reply-To: <1403693580-9449-1-git-send-email-a13xp0p0v88@gmail.com>

This patch adds a new common OF dma xlate callback function which will match a
channel by it's id. The binding expects one integer argument which it will use to
lookup the channel by the id.

Unlike of_dma_simple_xlate this function is able to handle a system with
multiple DMA controllers. When registering the of dma provider with
of_dma_controller_register a pointer to the dma_device struct which is
associated with the dt node needs to passed as the data parameter.
New function will use this pointer to match only channels which belong to the
specified DMA controller.

Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
 drivers/dma/of-dma.c   | 35 +++++++++++++++++++++++++++++++++++
 include/linux/of_dma.h |  4 ++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index e8fe9dc..d5fbeaa 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -218,3 +218,38 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 			&dma_spec->args[0]);
 }
 EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
+
+/**
+ * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id
+ * @dma_spec:	pointer to DMA specifier as found in the device tree
+ * @of_dma:	pointer to DMA controller data
+ *
+ * This function can be used as the of xlate callback for DMA driver which wants
+ * to match the channel based on the channel id. When using this xlate function
+ * the #dma-cells propety of the DMA controller dt node needs to be set to 1.
+ * The data parameter of of_dma_controller_register must be a pointer to the
+ * dma_device struct the function should match upon.
+ *
+ * Returns pointer to appropriate dma channel on success or NULL on error.
+ */
+struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+					 struct of_dma *ofdma)
+{
+	struct dma_device *dev = ofdma->of_dma_data;
+	struct dma_chan *chan, *candidate = NULL;
+
+	if (!dev || dma_spec->args_count != 1)
+		return NULL;
+
+	list_for_each_entry(chan, &dev->channels, device_node)
+		if (chan->chan_id == dma_spec->args[0]) {
+			candidate = chan;
+			break;
+		}
+
+	if (!candidate)
+		return NULL;
+
+	return dma_get_slave_channel(candidate);
+}
+EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index ae36298..56bc026 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -41,6 +41,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
 						     const char *name);
 extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 		struct of_dma *ofdma);
+extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+		struct of_dma *ofdma);
 #else
 static inline int of_dma_controller_register(struct device_node *np,
 		struct dma_chan *(*of_dma_xlate)
@@ -66,6 +68,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s
 	return NULL;
 }
 
+#define of_dma_xlate_by_chan_id NULL
+
 #endif
 
 #endif /* __LINUX_OF_DMA_H */
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH v3 1/3] dmaengine: mpc512x: add device tree binding document
From: Alexander Popov @ 2014-06-25 10:52 UTC (permalink / raw)
  To: Mark Rutland, Gerhard Sittig, Dan Williams, Vinod Koul,
	Lars-Peter Clausen, Arnd Bergmann, Anatolij Gustschin,
	Andy Shevchenko, Alexander Popov, linuxppc-dev, dmaengine,
	devicetree
In-Reply-To: <1403693580-9449-1-git-send-email-a13xp0p0v88@gmail.com>

Introduce a device tree binding document for the MPC512x DMA controller

Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
 .../devicetree/bindings/dma/mpc512x-dma.txt        | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt

diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
new file mode 100644
index 0000000..a6511df
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,29 @@
+* Freescale MPC512x and MPC8308 DMA Controller
+
+The DMA controller in Freescale MPC512x and MPC8308 SoCs can move
+blocks of memory contents between memory and peripherals or
+from memory to memory.
+
+Refer to "Generic DMA Controller and DMA request bindings" in
+the dma/dma.txt file for a more detailed description of binding.
+
+Required properties:
+- compatible: should be "fsl,mpc5121-dma" or "fsl,mpc8308-dma";
+- reg: should contain the DMA controller registers location and length;
+- interrupt for the DMA controller: syntax of interrupt client node
+	is described in interrupt-controller/interrupts.txt file.
+- #dma-cells: the length of the DMA specifier, must be <1>.
+	Each channel of this DMA controller has a peripheral request line,
+	the assignment is fixed in hardware. This one cell
+	in dmas property of a client device represents the channel number.
+
+Example:
+
+	dma0: dma@14000 {
+		compatible = "fsl,mpc5121-dma";
+		reg = <0x14000 0x1800>;
+		interrupts = <65 0x8>;
+		#dma-cells = <1>;
+	};
+
+DMA clients must use the format described in dma/dma.txt file.
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH v3 0/3] dmaengine: mpc512x: add device tree binding document and DMA channel lookup
From: Alexander Popov @ 2014-06-25 10:52 UTC (permalink / raw)
  To: Mark Rutland, Gerhard Sittig, Dan Williams, Vinod Koul,
	Lars-Peter Clausen, Arnd Bergmann, Anatolij Gustschin,
	Andy Shevchenko, Alexander Popov, linuxppc-dev, dmaengine,
	devicetree

This patch series introduces a device tree binding document for
the MPC512x DMA controller and adds device tree based DMA channel lookup
for it.

This version contains the improved device tree binding document:
#dma-cells is made a required property, as it must be according
dma/dma.txt document.

Alexander Popov (3):
  dmaengine: mpc512x: add device tree binding document
  dmaengine: of: add common xlate function for matching by channel id
  dmaengine: mpc512x: register for device tree channel lookup

 .../devicetree/bindings/dma/mpc512x-dma.txt        | 29 ++++++++++++++++++
 arch/powerpc/boot/dts/mpc5121.dtsi                 |  1 +
 drivers/dma/mpc512x_dma.c                          | 13 +++++++-
 drivers/dma/of-dma.c                               | 35 ++++++++++++++++++++++
 include/linux/of_dma.h                             |  4 +++
 5 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt

-- 
1.8.4.2

^ permalink raw reply

* Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: Alexey Kardashevskiy @ 2014-06-25 10:30 UTC (permalink / raw)
  To: David Laight, 'Wei Yang'
  Cc: benh@au1.ibm.com, linux-pci@vger.kernel.org,
	gwshan@linux.vnet.ibm.com, qiudayu@linux.vnet.ibm.com,
	bhelgaas@google.com, yan@linux.vnet.ibm.com,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D17264AD8@AcuExch.aculab.com>

On 06/25/2014 07:20 PM, David Laight wrote:
> From: Wei Yang
>> On Wed, Jun 25, 2014 at 02:12:34PM +1000, Alexey Kardashevskiy wrote:
>>> On 06/25/2014 11:12 AM, Wei Yang wrote:
>>>> On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote:
>>>>> On 06/10/2014 11:56 AM, Wei Yang wrote:
>>>>>> Current iommu_table of a PE is a static field. This will have a problem when
>>>>>> iommu_free_table is called.
>>>>>
>>>>> What kind of problem? This table is per PE and PE is not going anywhere.
>>>>>
>>>>
>>>> Yes, for Bus PE, they will always sit in the system. When VF PE introduced,
>>>> they could be released on the fly. When they are released, so do the iommu
>>>> table for the PE.
>>>
>>> iommu_table is a part of PE struct. When PE is released, iommu_table will
>>> go with it as well. Why to make is a pointer? I would understand it if you
>>> added reference counting there but no - iommu_table's lifetime is equal to
>>> PE lifetime.
>>>
>>
>> Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
>> need to release the iommu table. Currently, there is one function to release
>> the iommu table, iommu_free_table() which takes a pointer of the iommu_table
>> and release it.
>>
>> If the iommu table in PE is just a part of PE, it will have some problem to
>> release it with iommu_free_table(). That's why I make it a pointer in PE
>> structure.
> 
> What are the sizes of the iommu table and the PE structure?

This is all about iommu_table struct (which is just a descriptor), not
IOMMU table per se (which may be megabytes) :)


> If the table is a round number of pages then you probably don't want to
> embed it inside the PE structure.




-- 
Alexey

^ permalink raw reply

* RE: [PATCH v2] fsl-rio: add support for mapping inbound windows
From: Gang.Liu @ 2014-06-25  9:55 UTC (permalink / raw)
  To: Scott Wood, Martijn de Gouw
  Cc: Alexandre.Bounine@idt.com, Martijn de Gouw, Barry.Wood@idt.com,
	linuxppc-dev@lists.ozlabs.org,
	stef.van.os@prodrive-technologies.com
In-Reply-To: <1403656857.2435.61.camel@snotra.buserror.net>

DQo+IFN1YmplY3Q6IFtQQVRDSCB2Ml0gZnNsLXJpbzogYWRkIHN1cHBvcnQgZm9yIG1hcHBpbmcg
aW5ib3VuZCB3aW5kb3dzDQo+IA0KPiBGcm9tOiBNYXJ0aWpuIGRlIEdvdXcgPG1hcnRpam4uZGUu
Z291d0Bwcm9kcml2ZS5ubD4NCj4gDQo+IEFkZCBzdXBwb3J0IGZvciBtYXBwaW5nIGFuZCB1bm1h
cHBpbmcgb2YgaW5ib3VuZCByYXBpZGlvIHdpbmRvd3MuDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBN
YXJ0aWpuIGRlIEdvdXcgPG1hcnRpam4uZGUuZ291d0Bwcm9kcml2ZS10ZWNobm9sb2dpZXMuY29t
Pg0KPiAtLS0NCj4gIGFyY2gvcG93ZXJwYy9zeXNkZXYvZnNsX3Jpby5jIHwgICA5MiArKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKw0KPiAgYXJjaC9wb3dlcnBjL3N5c2Rl
di9mc2xfcmlvLmggfCAgIDEyICsrKysrKw0KPiAgMiBmaWxlcyBjaGFuZ2VkLCAxMDQgaW5zZXJ0
aW9ucygrKQ0KDQo+ICsJLyogY2hlY2sgZm9yIGNvbmZsaWN0aW5nIHJhbmdlcyAqLw0KPiArCWZv
ciAoaSA9IDA7IGkgPCBSSU9fSU5CX0FUTVVfQ09VTlQ7IGkrKykgew0KPiArCQlyaXdhciA9IGlu
X2JlMzIoJnByaXYtPmluYl9hdG11X3JlZ3NbaV0ucml3YXIpOw0KPiArCQlpZiAoKHJpd2FyICYg
UklXQVJfRU5BQkxFKSA9PSAwKQ0KPiArCQkJY29udGludWU7DQo+ICsJCXdpbl9zdGFydCA9ICgo
dTY0KShpbl9iZTMyKCZwcml2LT5pbmJfYXRtdV9yZWdzW2ldLnJpd2JhcikgJg0KPiBSSVdCQVJf
QkFERF9NQVNLKSkNCj4gKwkJCTw8IFJJV0JBUl9CQUREX1ZBTF9TSElGVDsNCj4gKwkJd2luX2Vu
ZCA9IHdpbl9zdGFydCArICgoMSA8PCAoKHJpd2FyICYgUklXQVJfU0laRV9NQVNLKSArIDEpKSAt
IDEpOw0KPiArCQlpZiAocnN0YXJ0IDwgd2luX2VuZCAmJiAocnN0YXJ0ICsgc2l6ZSkgPiB3aW5f
c3RhcnQpDQo+ICsJCQlyZXR1cm4gLUVJTlZBTDsNCj4gKwl9DQoNCkZvciB0aGUgaW5ib3VuZCB3
aW5kb3csIHRoZSBiYXNlIGFkZHJlc3MgbXVzdCBiZSBhbGlnbmVkIGJhc2VkIG9uIHRoZQ0Kc2l6
ZSBzZWxlY3RlZCBpbiB0aGUgd2luZG93IHNpemUgYml0cy4gU28gSSB0aGluayBpdCB3aWxsIGJl
IGJldHRlcg0KaWYgeW91IGNhbiBjaGVjayB0aGUgYWxpZ25tZW50Lg0KDQoNCj4gQEAgLTU5OCw2
ICs2ODcsOCBAQCBpbnQgZnNsX3Jpb19zZXR1cChzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpkZXYp
DQo+ICAJCQlSSU9fQVRNVV9SRUdTX1BPUlQyX09GRlNFVCkpOw0KPiANCj4gIAkJcHJpdi0+bWFp
bnRfYXRtdV9yZWdzID0gcHJpdi0+YXRtdV9yZWdzICsgMTsNCj4gKwkJcHJpdi0+aW5iX2F0bXVf
cmVncyA9IChzdHJ1Y3QgcmlvX2luYl9hdG11X3JlZ3MgKikNCj4gKwkJCShwcml2LT5yZWdzX3dp
biArIFJJT19JTkJfQVRNVV9SRUdTX09GRlNFVCk7DQoNClRoZSBSSU9fSU5CX0FUTVVfUkVHU19P
RkZTRVQgaXMganVzdCBmb3IgcG9ydCAxLCBJIHRoaW5rIHRoZSBwb3J0IDINCmFsc28gc2hvdWxk
IGJlIHN1cHBvcnRlZC4NCg0KQmVzdCBSZWdhcmRzLA0KTGl1IEdhbmcNCg==

^ permalink raw reply

* Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: Wei Yang @ 2014-06-25  9:31 UTC (permalink / raw)
  To: David Laight
  Cc: 'Wei Yang', benh@au1.ibm.com, Alexey Kardashevskiy,
	linux-pci@vger.kernel.org, gwshan@linux.vnet.ibm.com,
	yan@linux.vnet.ibm.com, bhelgaas@google.com,
	qiudayu@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D17264AD8@AcuExch.aculab.com>

On Wed, Jun 25, 2014 at 09:20:11AM +0000, David Laight wrote:
>From: Wei Yang
>> On Wed, Jun 25, 2014 at 02:12:34PM +1000, Alexey Kardashevskiy wrote:
>> >On 06/25/2014 11:12 AM, Wei Yang wrote:
>> >> On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote:
>> >>> On 06/10/2014 11:56 AM, Wei Yang wrote:
>> >>>> Current iommu_table of a PE is a static field. This will have a problem when
>> >>>> iommu_free_table is called.
>> >>>
>> >>> What kind of problem? This table is per PE and PE is not going anywhere.
>> >>>
>> >>
>> >> Yes, for Bus PE, they will always sit in the system. When VF PE introduced,
>> >> they could be released on the fly. When they are released, so do the iommu
>> >> table for the PE.
>> >
>> >iommu_table is a part of PE struct. When PE is released, iommu_table will
>> >go with it as well. Why to make is a pointer? I would understand it if you
>> >added reference counting there but no - iommu_table's lifetime is equal to
>> >PE lifetime.
>> >
>> 
>> Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
>> need to release the iommu table. Currently, there is one function to release
>> the iommu table, iommu_free_table() which takes a pointer of the iommu_table
>> and release it.
>> 
>> If the iommu table in PE is just a part of PE, it will have some problem to
>> release it with iommu_free_table(). That's why I make it a pointer in PE
>> structure.
>
>What are the sizes of the iommu table and the PE structure?

I calculated it in my mind, the size of iommu_table, defined in
arch/powerpc/include/asm/iommu.h is 256 bytes.

>If the table is a round number of pages then you probably don't want to
>embed it inside the PE structure.

If my understanding is correct, the iommu table structure size is not that
big.

>
>	David
>

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* RE: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: David Laight @ 2014-06-25  9:20 UTC (permalink / raw)
  To: 'Wei Yang', Alexey Kardashevskiy
  Cc: benh@au1.ibm.com, linux-pci@vger.kernel.org,
	gwshan@linux.vnet.ibm.com, qiudayu@linux.vnet.ibm.com,
	bhelgaas@google.com, yan@linux.vnet.ibm.com,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20140625052758.GA8873@richard>

RnJvbTogV2VpIFlhbmcNCj4gT24gV2VkLCBKdW4gMjUsIDIwMTQgYXQgMDI6MTI6MzRQTSArMTAw
MCwgQWxleGV5IEthcmRhc2hldnNraXkgd3JvdGU6DQo+ID5PbiAwNi8yNS8yMDE0IDExOjEyIEFN
LCBXZWkgWWFuZyB3cm90ZToNCj4gPj4gT24gVHVlLCBKdW4gMjQsIDIwMTQgYXQgMDg6MDY6MzJQ
TSArMTAwMCwgQWxleGV5IEthcmRhc2hldnNraXkgd3JvdGU6DQo+ID4+PiBPbiAwNi8xMC8yMDE0
IDExOjU2IEFNLCBXZWkgWWFuZyB3cm90ZToNCj4gPj4+PiBDdXJyZW50IGlvbW11X3RhYmxlIG9m
IGEgUEUgaXMgYSBzdGF0aWMgZmllbGQuIFRoaXMgd2lsbCBoYXZlIGEgcHJvYmxlbSB3aGVuDQo+
ID4+Pj4gaW9tbXVfZnJlZV90YWJsZSBpcyBjYWxsZWQuDQo+ID4+Pg0KPiA+Pj4gV2hhdCBraW5k
IG9mIHByb2JsZW0/IFRoaXMgdGFibGUgaXMgcGVyIFBFIGFuZCBQRSBpcyBub3QgZ29pbmcgYW55
d2hlcmUuDQo+ID4+Pg0KPiA+Pg0KPiA+PiBZZXMsIGZvciBCdXMgUEUsIHRoZXkgd2lsbCBhbHdh
eXMgc2l0IGluIHRoZSBzeXN0ZW0uIFdoZW4gVkYgUEUgaW50cm9kdWNlZCwNCj4gPj4gdGhleSBj
b3VsZCBiZSByZWxlYXNlZCBvbiB0aGUgZmx5LiBXaGVuIHRoZXkgYXJlIHJlbGVhc2VkLCBzbyBk
byB0aGUgaW9tbXUNCj4gPj4gdGFibGUgZm9yIHRoZSBQRS4NCj4gPg0KPiA+aW9tbXVfdGFibGUg
aXMgYSBwYXJ0IG9mIFBFIHN0cnVjdC4gV2hlbiBQRSBpcyByZWxlYXNlZCwgaW9tbXVfdGFibGUg
d2lsbA0KPiA+Z28gd2l0aCBpdCBhcyB3ZWxsLiBXaHkgdG8gbWFrZSBpcyBhIHBvaW50ZXI/IEkg
d291bGQgdW5kZXJzdGFuZCBpdCBpZiB5b3UNCj4gPmFkZGVkIHJlZmVyZW5jZSBjb3VudGluZyB0
aGVyZSBidXQgbm8gLSBpb21tdV90YWJsZSdzIGxpZmV0aW1lIGlzIGVxdWFsIHRvDQo+ID5QRSBs
aWZldGltZS4NCj4gPg0KPiANCj4gWWVzLCBpb21tdV90YWxiZSdzIGxpZmUgdGltZSBlcXVhbHMg
dG8gUEUgbGlmZXRpbWUsIHNvIHdoZW4gcmVsZWFzaW5nIGEgUEUgd2UNCj4gbmVlZCB0byByZWxl
YXNlIHRoZSBpb21tdSB0YWJsZS4gQ3VycmVudGx5LCB0aGVyZSBpcyBvbmUgZnVuY3Rpb24gdG8g
cmVsZWFzZQ0KPiB0aGUgaW9tbXUgdGFibGUsIGlvbW11X2ZyZWVfdGFibGUoKSB3aGljaCB0YWtl
cyBhIHBvaW50ZXIgb2YgdGhlIGlvbW11X3RhYmxlDQo+IGFuZCByZWxlYXNlIGl0Lg0KPiANCj4g
SWYgdGhlIGlvbW11IHRhYmxlIGluIFBFIGlzIGp1c3QgYSBwYXJ0IG9mIFBFLCBpdCB3aWxsIGhh
dmUgc29tZSBwcm9ibGVtIHRvDQo+IHJlbGVhc2UgaXQgd2l0aCBpb21tdV9mcmVlX3RhYmxlKCku
IFRoYXQncyB3aHkgSSBtYWtlIGl0IGEgcG9pbnRlciBpbiBQRQ0KPiBzdHJ1Y3R1cmUuDQoNCldo
YXQgYXJlIHRoZSBzaXplcyBvZiB0aGUgaW9tbXUgdGFibGUgYW5kIHRoZSBQRSBzdHJ1Y3R1cmU/
DQpJZiB0aGUgdGFibGUgaXMgYSByb3VuZCBudW1iZXIgb2YgcGFnZXMgdGhlbiB5b3UgcHJvYmFi
bHkgZG9uJ3Qgd2FudCB0bw0KZW1iZWQgaXQgaW5zaWRlIHRoZSBQRSBzdHJ1Y3R1cmUuDQoNCglE
YXZpZA0KDQo=

^ permalink raw reply

* Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: Wei Yang @ 2014-06-25  9:18 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Wei Yang, Alexey Kardashevskiy, linux-pci, gwshan, yan, bhelgaas,
	qiudayu, linuxppc-dev
In-Reply-To: <1403682997.4587.208.camel@pasglop>

On Wed, Jun 25, 2014 at 05:56:37PM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2014-06-25 at 17:50 +1000, Alexey Kardashevskiy wrote:
>
>> > Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
>> > need to release the iommu table. Currently, there is one function to release
>> > the iommu table, iommu_free_table() which takes a pointer of the iommu_table
>> > and release it.
>> > 
>> > If the iommu table in PE is just a part of PE, it will have some problem to
>> > release it with iommu_free_table(). That's why I make it a pointer in PE
>> > structure.
>> 
>> So you are saying that you want to release PE by one kfree() and release
>> iommu_table by another kfree (embedded into iommu_free_table()). For me
>> that means that PE and iommu_table have different lifetime.
>> 
>> And I cannot find the exact place in this patchset where you call
>> iommu_free_table(), what do I miss?
>
>He has a point though... iommu_free_table() does a whole bunch of things
>in addition to kfree at the end.
>
>This is a discrepancy in the iommu.c code, we don't allocate the table,
>it's allocated by our callers, but we do free it in iommu_free_table().
>
>My gut feeling is that we should fix that in the core by moving the
>kfree() out of iommu_free_table() and back into vio.c and
>pseries/iommu.c, the only two callers, otherwise we can't wrap the table
>structure inside another object if we are going to ever free it.
>

Yes, this is another option. Move the kfree() outside could keep some logic in
current code, like in pnv_pci_ioda_tce_invalidate(). We could get the tbl from
a PE structure directly, instead of adding a field in tbl to point to the PE
structure.

>Cheers,
>Ben.
>
>
>

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: Wei Yang @ 2014-06-25  9:13 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Wei Yang, benh, linux-pci, gwshan, yan, bhelgaas, qiudayu,
	linuxppc-dev
In-Reply-To: <53AA7F30.40504@ozlabs.ru>

On Wed, Jun 25, 2014 at 05:50:08PM +1000, Alexey Kardashevskiy wrote:
>On 06/25/2014 03:27 PM, Wei Yang wrote:
>> On Wed, Jun 25, 2014 at 02:12:34PM +1000, Alexey Kardashevskiy wrote:
>>> On 06/25/2014 11:12 AM, Wei Yang wrote:
>>>> On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote:
>>>>> On 06/10/2014 11:56 AM, Wei Yang wrote:
>>>>>> Current iommu_table of a PE is a static field. This will have a problem when
>>>>>> iommu_free_table is called.
>>>>>
>>>>> What kind of problem? This table is per PE and PE is not going anywhere.
>>>>>
>>>>
>>>> Yes, for Bus PE, they will always sit in the system. When VF PE introduced,
>>>> they could be released on the fly. When they are released, so do the iommu
>>>> table for the PE.
>>>
>>> iommu_table is a part of PE struct. When PE is released, iommu_table will
>>> go with it as well. Why to make is a pointer? I would understand it if you
>>> added reference counting there but no - iommu_table's lifetime is equal to
>>> PE lifetime.
>>>
>> 
>> Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
>> need to release the iommu table. Currently, there is one function to release
>> the iommu table, iommu_free_table() which takes a pointer of the iommu_table
>> and release it.
>> 
>> If the iommu table in PE is just a part of PE, it will have some problem to
>> release it with iommu_free_table(). That's why I make it a pointer in PE
>> structure.
>
>So you are saying that you want to release PE by one kfree() and release
>iommu_table by another kfree (embedded into iommu_free_table()). For me
>that means that PE and iommu_table have different lifetime.
>

Hmm... it is right, the lifetime of these two may have some difference.

>And I cannot find the exact place in this patchset where you call
>iommu_free_table(), what do I miss?
>

This is called in pnv_pci_release_dev_dma(), which is introduced in the commit
cd740988: powerpc/powernv: allocate VF PE

>
>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* RE: [PATCH] spi: include "int ret" with macro
From: David Laight @ 2014-06-25  8:44 UTC (permalink / raw)
  To: 'Zhao Qiang', linuxppc-dev@lists.ozlabs.org,
	broonie@kernel.org, linux-spi@vger.kernel.org,
	B07421@freescale.com
In-Reply-To: <1403661276-16598-1-git-send-email-B45475@freescale.com>

RnJvbTogWmhhbyBRaWFuZw0KPiByZXQgaXMgdW51c2VkIHdoZW4gQ09ORklHX0ZTTF9TT0MgZGVm
aW5lZCwNCj4gc28gaW5jbHVkZSBpdCB3aXRoICIjaWZuZGVmIENPTkZJR19GU0xfU09DIi4NCj4g
DQo+IFNpZ25lZC1vZmYtYnk6IFpoYW8gUWlhbmcgPEI0NTQ3NUBmcmVlc2NhbGUuY29tPg0KPiAt
LS0NCj4gIGRyaXZlcnMvc3BpL3NwaS1mc2wtbGliLmMgfCAzICsrLQ0KPiAgMSBmaWxlIGNoYW5n
ZWQsIDIgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2Ry
aXZlcnMvc3BpL3NwaS1mc2wtbGliLmMgYi9kcml2ZXJzL3NwaS9zcGktZnNsLWxpYi5jDQo+IGlu
ZGV4IGU1ZDQ1ZmMuLjQ0YWFjZTEgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvc3BpL3NwaS1mc2wt
bGliLmMNCj4gKysrIGIvZHJpdmVycy9zcGkvc3BpLWZzbC1saWIuYw0KPiBAQCAtMTk4LDggKzE5
OCw5IEBAIGludCBvZl9tcGM4eHh4X3NwaV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpv
ZmRldikNCj4gIAlzdHJ1Y3QgbXBjOHh4eF9zcGlfcHJvYmVfaW5mbyAqcGluZm87DQo+ICAJc3Ry
dWN0IGZzbF9zcGlfcGxhdGZvcm1fZGF0YSAqcGRhdGE7DQo+ICAJY29uc3Qgdm9pZCAqcHJvcDsN
Cj4gKyNpZm5kZWYgQ09ORklHX0ZTTF9TT0MNCj4gIAlpbnQgcmV0ID0gLUVOT01FTTsNCj4gLQ0K
PiArI2VuZGlmDQoNCllvdSBhcmUgcmVtb3ZpbmcgdGhlIGJsYW5rIGxpbmUgYWZ0ZXIgdGhlIGRl
ZmluaXRpb24gb2YgdGhlIGxvY2FscywNCmFuZCB0aGUgaW5pdGlhbGlzZXIgaXNuJ3QgbmVlZGVk
Lg0KDQo+ICAJcGluZm8gPSBkZXZtX2t6YWxsb2MoJm9mZGV2LT5kZXYsIHNpemVvZigqcGluZm8p
LCBHRlBfS0VSTkVMKTsNCj4gIAlpZiAoIXBpbmZvKQ0KPiAgCQlyZXR1cm4gLUVOT01FTTsNCj4g
LS0NCg0KSSB0aGluayBpdCBtaWdodCBiZSBwcmVmZXJhYmxlIHRvIGRlZmluZSAncmV0JyBpbnNp
ZGUgdGhlIGNvbmRpdGlvbmFsDQp3aGVyZSBpdCBpcyB1c2VkIC0gd2hpY2ggcmVxdWlyZXMgYW4g
ZXh0cmEgey4uLn0gYmxvY2suDQoNCkEgJ3NuZWFreScgd2F5IHRvIGF2b2lkIHRoZSB3YXJuaW5n
IGlzIHRvICdyZXR1cm4gcmV0JyB3aGVuIHRoZSBremFsbG9jKCkgZmFpbHMuDQoNCglEYXZpZA0K
DQoNCg==

^ permalink raw reply

* Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: Benjamin Herrenschmidt @ 2014-06-25  7:56 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Wei Yang, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <53AA7F30.40504@ozlabs.ru>

On Wed, 2014-06-25 at 17:50 +1000, Alexey Kardashevskiy wrote:

> > Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
> > need to release the iommu table. Currently, there is one function to release
> > the iommu table, iommu_free_table() which takes a pointer of the iommu_table
> > and release it.
> > 
> > If the iommu table in PE is just a part of PE, it will have some problem to
> > release it with iommu_free_table(). That's why I make it a pointer in PE
> > structure.
> 
> So you are saying that you want to release PE by one kfree() and release
> iommu_table by another kfree (embedded into iommu_free_table()). For me
> that means that PE and iommu_table have different lifetime.
> 
> And I cannot find the exact place in this patchset where you call
> iommu_free_table(), what do I miss?

He has a point though... iommu_free_table() does a whole bunch of things
in addition to kfree at the end.

This is a discrepancy in the iommu.c code, we don't allocate the table,
it's allocated by our callers, but we do free it in iommu_free_table().

My gut feeling is that we should fix that in the core by moving the
kfree() out of iommu_free_table() and back into vio.c and
pseries/iommu.c, the only two callers, otherwise we can't wrap the table
structure inside another object if we are going to ever free it.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] powerpc: Fix build warning
From: Geert Uytterhoeven @ 2014-06-25  7:53 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Vincent Guittot, Paul Mackerras, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <53A91449.6010701@roeck-us.net>

On Tue, Jun 24, 2014 at 8:01 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> Sigh. Much easier to break something than to fix it. That would mean to get
> approval
> from at least three maintainers, and all that to get rid of a warning. I
> don't
> really have time for that. Let's just forget about it and live with the
> warning.

So you send it to akpm. Or perhaps even trivial.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: Alexey Kardashevskiy @ 2014-06-25  7:50 UTC (permalink / raw)
  To: Wei Yang; +Cc: benh, linux-pci, gwshan, yan, bhelgaas, qiudayu, linuxppc-dev
In-Reply-To: <20140625052758.GA8873@richard>

On 06/25/2014 03:27 PM, Wei Yang wrote:
> On Wed, Jun 25, 2014 at 02:12:34PM +1000, Alexey Kardashevskiy wrote:
>> On 06/25/2014 11:12 AM, Wei Yang wrote:
>>> On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote:
>>>> On 06/10/2014 11:56 AM, Wei Yang wrote:
>>>>> Current iommu_table of a PE is a static field. This will have a problem when
>>>>> iommu_free_table is called.
>>>>
>>>> What kind of problem? This table is per PE and PE is not going anywhere.
>>>>
>>>
>>> Yes, for Bus PE, they will always sit in the system. When VF PE introduced,
>>> they could be released on the fly. When they are released, so do the iommu
>>> table for the PE.
>>
>> iommu_table is a part of PE struct. When PE is released, iommu_table will
>> go with it as well. Why to make is a pointer? I would understand it if you
>> added reference counting there but no - iommu_table's lifetime is equal to
>> PE lifetime.
>>
> 
> Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
> need to release the iommu table. Currently, there is one function to release
> the iommu table, iommu_free_table() which takes a pointer of the iommu_table
> and release it.
> 
> If the iommu table in PE is just a part of PE, it will have some problem to
> release it with iommu_free_table(). That's why I make it a pointer in PE
> structure.

So you are saying that you want to release PE by one kfree() and release
iommu_table by another kfree (embedded into iommu_free_table()). For me
that means that PE and iommu_table have different lifetime.

And I cannot find the exact place in this patchset where you call
iommu_free_table(), what do I miss?




-- 
Alexey

^ permalink raw reply

* Re: [PATCH v5 1/1] powerpc/perf: Adjust callchain based on DWARF debug info
From: Jiri Olsa @ 2014-06-25  7:42 UTC (permalink / raw)
  To: Sukadev Bhattiprolu
  Cc: Michael Ellerman, linuxppc-dev, Anton Blanchard, linux-kernel,
	Arnaldo Carvalho de Melo, ulrich.weigand, Maynard Johnson
In-Reply-To: <20140625000052.GA4484@us.ibm.com>

On Tue, Jun 24, 2014 at 05:00:52PM -0700, Sukadev Bhattiprolu wrote:
> [PATCH v5 1/1] powerpc/perf: Adjust callchain based on DWARF debug info

superflous ^^^

> 
> When saving the callchain on Power, the kernel conservatively saves excess
> entries in the callchain. A few of these entries are needed in some cases
> but not others. We should use the DWARF debug information to determine
> when the entries are  needed.

SNIP

> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 0e5fea9..6221d01 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1291,7 +1291,9 @@ static int machine__resolve_callchain_sample(struct machine *machine,
>  	u8 cpumode = PERF_RECORD_MISC_USER;
>  	int chain_nr = min(max_stack, (int)chain->nr);
>  	int i;
> +	int j;
>  	int err;
> +	int skip_idx;
>  
>  	callchain_cursor_reset(&callchain_cursor);
>  
> @@ -1300,14 +1302,26 @@ static int machine__resolve_callchain_sample(struct machine *machine,
>  		return 0;
>  	}
>  
> +	/*
> +	 * Based on DWARF debug information, some architectures skip
> +	 * a callchain entry saved by the kernel.
> +	 */
> +	skip_idx = arch_skip_callchain_idx(machine, thread, chain);
> +
>  	for (i = 0; i < chain_nr; i++) {
>  		u64 ip;
>  		struct addr_location al;
>  
>  		if (callchain_param.order == ORDER_CALLEE)
> -			ip = chain->ips[i];
> +			j = i;
>  		else
> -			ip = chain->ips[chain->nr - i - 1];
> +			j = chain->nr - i - 1;
> +
> +#ifdef HAVE_SKIP_CALLCHAIN_IDX
> +		if (j == skip_idx)
> +			continue;
> +#endif
> +		ip = chain->ips[j];
>  
>  		if (ip >= PERF_CONTEXT_MAX) {
>  			switch (ip) {
> -- 
> 1.7.9.5
> 

got following compile error:

  CC       util/map.o
util/machine.c: In function ‘machine__resolve_callchain_sample’:
util/machine.c:1286:6: error: variable ‘skip_idx’ set but not used [-Werror=unused-but-set-variable]
  int skip_idx;
      ^
cc1: all warnings being treated as errors
make[2]: *** [util/machine.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [all] Error 2
  test: test -x ./perf
make: *** [make_pure] Error 1


you could use __maybe_unused for the 'skip_idx'

jirka

^ permalink raw reply

* Re: [PATCH v2] sched: Fix compiler warnings
From: Uwe Kleine-König @ 2014-06-25  7:14 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Russell King, Peter Zijlstra, linux-kernel, Dietmar Eggemann,
	Ingo Molnar, Paul Mackerras, Vincent Guittot, linuxppc-dev,
	Ingo Molnar, linux-arm-kernel
In-Reply-To: <1403658329-13196-1-git-send-email-linux@roeck-us.net>

On Tue, Jun 24, 2014 at 06:05:29PM -0700, Guenter Roeck wrote:
> Commit 143e1e28cb (sched: Rework sched_domain topology definition)
> introduced a number of functions with a return value of 'const int'.
> gcc doesn't know what to do with that and, if the kernel is compiled
> with W=1, complains with the following warnings whenever sched.h
> is included.
> 
> include/linux/sched.h:875:25: warning:
> 	type qualifiers ignored on function return type
> include/linux/sched.h:882:25: warning:
> 	type qualifiers ignored on function return type
> include/linux/sched.h:889:25: warning:
> 	type qualifiers ignored on function return type
> include/linux/sched.h:1002:21: warning:
> 	type qualifiers ignored on function return type
> 
> Commits fb2aa855 (sched, ARM: Create a dedicated scheduler topology table)
> and 607b45e9a (sched, powerpc: Create a dedicated topology table) introduce
> the same warning in the arm and powerpc code.
> 
> Drop 'const' from the function declarations to fix the problem.
> 
> The fix for all three patches has to be applied together to avoid
> compilation failures for the affected architectures.
> 
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Fix problem in all affected architectures with a single patch
>     to avoid compilation errors.
> 
>  arch/arm/kernel/topology.c | 2 +-
>  arch/powerpc/kernel/smp.c  | 2 +-
>  include/linux/sched.h      | 8 ++++----
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 9d85318..e35d880 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid)
>  		cpu_topology[cpuid].socket_id, mpidr);
>  }
>  
> -static inline const int cpu_corepower_flags(void)
> +static inline int cpu_corepower_flags(void)
>  {
>  	return SD_SHARE_PKG_RESOURCES  | SD_SHARE_POWERDOMAIN;
>  }
Maybe the author's intention was:

	static inline int cpu_corepower_flags(void) __attribute__((const));

?
This specifies that the function has no side effects and the return value
only depends on the (here non-existing) function arguments.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
From: Mike Qiu @ 2014-06-25  7:27 UTC (permalink / raw)
  To: Gavin Shan; +Cc: weiyang, linuxppc-dev
In-Reply-To: <20140625053312.GA3808@shangw>

On 06/25/2014 01:33 PM, Gavin Shan wrote:
> On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>
> [ cc Richard ]
>
>> Eeh sysfs entry created must be after EEH_ENABLED been set
>> in eeh_subsystem_flags.
>>
>> In PowerNV platform, it try to create sysfs entry before
>> EEH_ENABLED been set, when boot up. So nothing will be
>> created for eeh in sysfs.
>>
> Could you please make the commit log more clear? :-)
>
> I guess the issue is introduced by commit 2213fb1 ("
> powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
> commit checks EEH is enabled while creating PCI device
> EEH sysfs files. If not, the sysfs files won't be created.
> That's to avoid warning reported during PCI hotplug.
>
> The problem you're reporting (if I understand completely):
> You don't see the sysfs files after the system boots up.
> If it's the case, you probably need following changes in
> arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
> Could you have a try with it?
>
> #ifdef CONFIG_EEH
> 	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
> -	eeh_addr_cache_build();
> 	eeh_init();
> +	eeh_addr_cache_build();
> #endif

But this was not work, as I test, see boot log below:

[    0.233993] Unable to handle kernel paging request for data at 
address 0x00000010
[    0.234086] Faulting instruction address: 0xc000000000036c84
[    0.234144] Oops: Kernel access of bad area, sig: 11 [#1]
[    0.234188] SMP NR_CPUS=1024 NUMA PowerNV
[    0.234235] Modules linked in:
[    0.234282] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 3.16.0-rc1+ #61
[    0.234339] task: c0000003bfcc0000 ti: c0000003bfd00000 task.ti: 
c0000003bfd00000
[    0.234405] NIP: c000000000036c84 LR: c000000000036c4c CTR: 
0000000000000000
[    0.234472] REGS: c0000003bfd03430 TRAP: 0300   Not tainted (3.16.0-rc1+)
[    0.234528] MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI> CR: 
44008088  XER: 00000000
[    0.234686] CFAR: c000000000009358 DAR: 0000000000000010 DSISR: 
40000000 SOFTE: 1
GPR00: c000000000036c4c c0000003bfd036b0 c000000001448d58 c0000003bce30080
GPR04: 0000000000000000 0000000000000000 0000000000000001 c0000003bce300c8
GPR08: c0000003bce300e8 0000000000000000 0000000000000000 000000003030f000
GPR12: 0000000022008042 c00000000fee1200 c000000000b0e1f0 0000000000000000
GPR16: f000000000019600 0000000000000008 000000000000003f c000000003022280
GPR20: c000000000b0e058 0000000000400000 0000000000000008 0000000000000007
GPR24: c000000003120f80 c000000000b0e2d0 c0000000013bc6f0 c0000003bca18400
GPR28: 0000000000000000 c000000003010000 c0000003bce30080 c0000003bb2c3b40
[    0.235582] NIP [c000000000036c84] .eeh_add_to_parent_pe+0x164/0x340
[    0.235639] LR [c000000000036c4c] .eeh_add_to_parent_pe+0x12c/0x340
[    0.235695] Call Trace:
[    0.235719] [c0000003bfd036b0] [c000000000036c4c] 
.eeh_add_to_parent_pe+0x12c/0x340 (unreliable)
[    0.235810] [c0000003bfd03730] [c000000000070ee8] 
.powernv_eeh_dev_probe+0x158/0x1d0
[    0.235890] [c0000003bfd037c0] [c00000000048768c] 
.pci_walk_bus+0x8c/0x120
[    0.235957] [c0000003bfd03860] [c0000000000341c4] .eeh_init+0xf4/0x310
[    0.236025] [c0000003bfd03900] [c00000000006e7a8] 
.pnv_pci_ioda_fixup+0x688/0xb30
[    0.236105] [c0000003bfd03a60] [c000000000c2ee90] 
.pcibios_resource_survey+0x334/0x3f4
[    0.236183] [c0000003bfd03b50] [c000000000c2e65c] .pcibios_init+0xa0/0xd4
[    0.236251] [c0000003bfd03be0] [c00000000000bc94] 
.do_one_initcall+0x124/0x280
[    0.236329] [c0000003bfd03cd0] [c000000000c24acc] 
.kernel_init_freeable+0x250/0x348
[    0.236408] [c0000003bfd03db0] [c00000000000c4c4] .kernel_init+0x24/0x140
[    0.236475] [c0000003bfd03e30] [c00000000000a45c] 
.ret_from_kernel_thread+0x58/0x7c
[    0.236553] Instruction dump:
[    0.236586] 815f000c 60000000 e9228890 915e000c 81290000 7926f7e3 
813f0008 913e0008
[    0.236698] 41820018 2fbf0000 419e0154 e93f0088 <e9290010> f93e0018 
e93f0080 48000034
[    0.236819] ---[ end trace e78b31e354e84859 ]---
[    0.236864]
[    2.236933] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0000000b

This may because  edev->pdev is set in eeh_addr_cache_build(), while 
eeh_init() use that entry.

After changed the code, the call patch:

eeh_init() ---->
                         pci_walk_bus()---->
powernv_eeh_dev_probe() ----->
                                        eeh_add_to_parent_pe()
eeh_addr_cache_build()

We can see in
eeh_add_to_parent_pe() {
......
pe->bus = eeh_dev_to_pci_dev(edev)->bus;
......
}

That is sure eeh_dev_to_pci_dev(edev) will be *NULL*, because this is 
set in  eeh_addr_cache_build()


Thanks
Mike
> Eventually PowerNV/pSeries have same function call sequence:
>
> - Set EEH probe mode
> - Doing probe (with device node or PCI device)
> - Build address cache.
>
>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> index 8ad0c5b..5f95581 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> @@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>> 	struct pnv_phb *phb = hose->private_data;
>> 	int ret;
>>
>> +	/* Creat sysfs after EEH_ENABLED been set */
>> +	eeh_add_sysfs_files(hose->bus);
>> +
>> 	/* Register OPAL event notifier */
>> 	if (!ioda_eeh_nb_init) {
>> 		ret = opal_notifier_register(&ioda_eeh_nb);
> Thanks,
> Gavin
>
>

^ permalink raw reply

* Re: [PATCH v2] sched: Fix compiler warnings
From: Vincent Guittot @ 2014-06-25  6:41 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Russell King, Peter Zijlstra, linux-kernel, Ingo Molnar,
	Ingo Molnar, Paul Mackerras, linuxppc-dev, Dietmar Eggemann, LAK
In-Reply-To: <1403658329-13196-1-git-send-email-linux@roeck-us.net>

On 25 June 2014 03:05, Guenter Roeck <linux@roeck-us.net> wrote:
> Commit 143e1e28cb (sched: Rework sched_domain topology definition)
> introduced a number of functions with a return value of 'const int'.
> gcc doesn't know what to do with that and, if the kernel is compiled
> with W=1, complains with the following warnings whenever sched.h
> is included.
>
> include/linux/sched.h:875:25: warning:
>         type qualifiers ignored on function return type
> include/linux/sched.h:882:25: warning:
>         type qualifiers ignored on function return type
> include/linux/sched.h:889:25: warning:
>         type qualifiers ignored on function return type
> include/linux/sched.h:1002:21: warning:
>         type qualifiers ignored on function return type
>
> Commits fb2aa855 (sched, ARM: Create a dedicated scheduler topology table)
> and 607b45e9a (sched, powerpc: Create a dedicated topology table) introduce
> the same warning in the arm and powerpc code.
>
> Drop 'const' from the function declarations to fix the problem.
>
> The fix for all three patches has to be applied together to avoid
> compilation failures for the affected architectures.
>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>

Acked-by Vincent Guittot <vincent.guittot@linaro.org>

> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Fix problem in all affected architectures with a single patch
>     to avoid compilation errors.
>
>  arch/arm/kernel/topology.c | 2 +-
>  arch/powerpc/kernel/smp.c  | 2 +-
>  include/linux/sched.h      | 8 ++++----
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 9d85318..e35d880 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -275,7 +275,7 @@ void store_cpu_topology(unsigned int cpuid)
>                 cpu_topology[cpuid].socket_id, mpidr);
>  }
>
> -static inline const int cpu_corepower_flags(void)
> +static inline int cpu_corepower_flags(void)
>  {
>         return SD_SHARE_PKG_RESOURCES  | SD_SHARE_POWERDOMAIN;
>  }
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 51a3ff7..1007fb8 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -747,7 +747,7 @@ int setup_profiling_timer(unsigned int multiplier)
>
>  #ifdef CONFIG_SCHED_SMT
>  /* cpumask of CPUs with asymetric SMT dependancy */
> -static const int powerpc_smt_flags(void)
> +static int powerpc_smt_flags(void)
>  {
>         int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 306f4f0..0376b05 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -872,21 +872,21 @@ enum cpu_idle_type {
>  #define SD_NUMA                        0x4000  /* cross-node balancing */
>
>  #ifdef CONFIG_SCHED_SMT
> -static inline const int cpu_smt_flags(void)
> +static inline int cpu_smt_flags(void)
>  {
>         return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
>  }
>  #endif
>
>  #ifdef CONFIG_SCHED_MC
> -static inline const int cpu_core_flags(void)
> +static inline int cpu_core_flags(void)
>  {
>         return SD_SHARE_PKG_RESOURCES;
>  }
>  #endif
>
>  #ifdef CONFIG_NUMA
> -static inline const int cpu_numa_flags(void)
> +static inline int cpu_numa_flags(void)
>  {
>         return SD_NUMA;
>  }
> @@ -999,7 +999,7 @@ void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms);
>  bool cpus_share_cache(int this_cpu, int that_cpu);
>
>  typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
> -typedef const int (*sched_domain_flags_f)(void);
> +typedef int (*sched_domain_flags_f)(void);
>
>  #define SDTL_OVERLAP   0x01
>
> --
> 1.9.1
>

^ permalink raw reply

* Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
From: Gavin Shan @ 2014-06-25  6:37 UTC (permalink / raw)
  To: Wei Yang; +Cc: linuxppc-dev, Mike Qiu, Gavin Shan, kvm-ppc
In-Reply-To: <20140625062353.GB15078@richard>

On Wed, Jun 25, 2014 at 02:23:53PM +0800, Wei Yang wrote:
>On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote:
>>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>>
>>[ cc Richard ]
>>
>>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>>in eeh_subsystem_flags.
>>>
>>>In PowerNV platform, it try to create sysfs entry before
>>>EEH_ENABLED been set, when boot up. So nothing will be
>>>created for eeh in sysfs.
>>>
>>
>>Could you please make the commit log more clear? :-)
>>
>>I guess the issue is introduced by commit 2213fb1 ("
>>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>>commit checks EEH is enabled while creating PCI device
>>EEH sysfs files. If not, the sysfs files won't be created.
>>That's to avoid warning reported during PCI hotplug.
>>
>>The problem you're reporting (if I understand completely):
>>You don't see the sysfs files after the system boots up.
>>If it's the case, you probably need following changes in
>>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>>Could you have a try with it?
>>
>>#ifdef CONFIG_EEH
>>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>>-	eeh_addr_cache_build();
>>	eeh_init();
>>+	eeh_addr_cache_build();
>>#endif
>>
>
>I think this is a more proper fix.
>
>BTW, I have one confusion in this mode set.
>
>eeh_init()
>  -> eeh_ops->dev_probe()
>     -> powernv_eeh_dev_probe()
>        -> eeh_set_enable(true)   <- here the eeh is marked enabled
>
>We can see this flag would be set for each pci_dev. So is it possible to make
>this "set" only once?
>

It shouldn't be a problem because there might not have PCI devices
supporting EEH in the guest. All PCI devices are emulated.

>>Eventually PowerNV/pSeries have same function call sequence:
>>
>>- Set EEH probe mode
>>- Doing probe (with device node or PCI device)
>>- Build address cache.
>>
>>>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>>---
>>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>index 8ad0c5b..5f95581 100644
>>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>>> 	struct pnv_phb *phb = hose->private_data;
>>> 	int ret;
>>>
>>>+	/* Creat sysfs after EEH_ENABLED been set */
>>>+	eeh_add_sysfs_files(hose->bus);
>>>+
>>> 	/* Register OPAL event notifier */
>>> 	if (!ioda_eeh_nb_init) {
>>> 		ret = opal_notifier_register(&ioda_eeh_nb);

Thanks,
Gavin

^ permalink raw reply

* Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
From: Wei Yang @ 2014-06-25  6:23 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev, weiyang, Mike Qiu, kvm-ppc
In-Reply-To: <20140625053312.GA3808@shangw>

On Wed, Jun 25, 2014 at 03:33:12PM +1000, Gavin Shan wrote:
>On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:
>
>[ cc Richard ]
>
>>Eeh sysfs entry created must be after EEH_ENABLED been set
>>in eeh_subsystem_flags.
>>
>>In PowerNV platform, it try to create sysfs entry before
>>EEH_ENABLED been set, when boot up. So nothing will be
>>created for eeh in sysfs.
>>
>
>Could you please make the commit log more clear? :-)
>
>I guess the issue is introduced by commit 2213fb1 ("
>powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
>commit checks EEH is enabled while creating PCI device
>EEH sysfs files. If not, the sysfs files won't be created.
>That's to avoid warning reported during PCI hotplug.
>
>The problem you're reporting (if I understand completely):
>You don't see the sysfs files after the system boots up.
>If it's the case, you probably need following changes in
>arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
>Could you have a try with it?
>
>#ifdef CONFIG_EEH
>	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
>-	eeh_addr_cache_build();
>	eeh_init();
>+	eeh_addr_cache_build();
>#endif
>

I think this is a more proper fix.

BTW, I have one confusion in this mode set.

eeh_init()
  -> eeh_ops->dev_probe()
     -> powernv_eeh_dev_probe()
        -> eeh_set_enable(true)   <- here the eeh is marked enabled

We can see this flag would be set for each pci_dev. So is it possible to make
this "set" only once?

>Eventually PowerNV/pSeries have same function call sequence:
>
>- Set EEH probe mode
>- Doing probe (with device node or PCI device)
>- Build address cache.
>
>>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>---
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>index 8ad0c5b..5f95581 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
>> 	struct pnv_phb *phb = hose->private_data;
>> 	int ret;
>>
>>+	/* Creat sysfs after EEH_ENABLED been set */
>>+	eeh_add_sysfs_files(hose->bus);
>>+
>> 	/* Register OPAL event notifier */
>> 	if (!ioda_eeh_nb_init) {
>> 		ret = opal_notifier_register(&ioda_eeh_nb);
>
>Thanks,
>Gavin

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2014-06-25  6:13 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list

Hi Linus !

Here are a handful or two of powerpc fixes and simple/trivial
cleanups. A bunch of them fix ftrace with the new ABI v2 in
Little Endian, the rest is a scattering of fairly simple things.

Cheers,
Ben.

The following changes since commit 68986c9f0f4552c34c248501eb0c690553866d6e:

  Revert "offb: Add palette hack for little endian" (2014-06-16 19:45:45 +1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

for you to fetch changes up to 6663a4fa6711050036562ddfd2086edf735fae21:

  powerpc: Don't skip ePAPR spin-table CPUs (2014-06-25 13:10:49 +1000)

----------------------------------------------------------------
Benjamin Herrenschmidt (1):
      powerpc: Remove __arch_swab*

Catalin Marinas (1):
      powerpc/kmemleak: Do not scan the DART table

Gavin Shan (1):
      powerpc/kerenl: Enable EEH for IO accessors

Laurent Dufour (1):
      powerpc/module: Fix TOC symbol CRC

Michael Ellerman (9):
      powerpc: Remove ancient DEBUG_SIG code
      powerpc: Add ppc_global_function_entry()
      powerpc/ftrace: Fix typo in mask of opcode
      powerpc/ftrace: Fix inverted check of create_branch()
      powerpc/ftrace: Fix nop of modules on 64bit LE (ABIv2)
      powerpc/ftrace: Use pr_fmt() to namespace error messages
      powerpc/kprobes: Fix jprobes on ABI v2 (LE)
      selftests/powerpc: Use the test harness for the TM DSCR test
      powerpc/powernv: Remove OPAL v1 takeover

Rasmus Villemoes (1):
      powerpc/macintosh/smu.c: Fix closing brace followed by if

Rickard Strandqvist (1):
      powerpc/cell: cbe_thermal.c: Cleaning up a variable is of the wrong type

Scott Wood (1):
      powerpc: Don't skip ePAPR spin-table CPUs

 arch/powerpc/Kconfig.debug                         |   1 -
 arch/powerpc/include/asm/code-patching.h           |  11 ++
 arch/powerpc/include/asm/opal.h                    |  29 ---
 arch/powerpc/include/asm/swab.h                    |  43 -----
 arch/powerpc/kernel/ftrace.c                       |  52 +++--
 arch/powerpc/kernel/iomap.c                        |  20 +-
 arch/powerpc/kernel/kprobes.c                      |   9 +-
 arch/powerpc/kernel/module_64.c                    |  11 +-
 arch/powerpc/kernel/prom.c                         |   7 -
 arch/powerpc/kernel/prom_init.c                    | 211 ---------------------
 arch/powerpc/kernel/prom_init_check.sh             |   4 +-
 arch/powerpc/kernel/setup-common.c                 |  10 +-
 arch/powerpc/kernel/signal_32.c                    |   9 -
 arch/powerpc/kernel/signal_64.c                    |   9 -
 arch/powerpc/platforms/cell/cbe_thermal.c          |   2 +-
 arch/powerpc/platforms/powernv/Makefile            |   2 +-
 arch/powerpc/platforms/powernv/opal-takeover.S     | 140 --------------
 arch/powerpc/sysdev/dart_iommu.c                   |   5 +
 drivers/macintosh/smu.c                            |   3 +-
 tools/testing/selftests/powerpc/tm/Makefile        |   2 +-
 .../testing/selftests/powerpc/tm/tm-resched-dscr.c |  14 +-
 21 files changed, 93 insertions(+), 501 deletions(-)
 delete mode 100644 arch/powerpc/platforms/powernv/opal-takeover.S

^ permalink raw reply

* Re: [PATCH] Bugfix: powerpc/eeh: Create eeh sysfs entry in post_init()
From: Gavin Shan @ 2014-06-25  5:33 UTC (permalink / raw)
  To: Mike Qiu; +Cc: weiyang, linuxppc-dev, gwshan, kvm-ppc
In-Reply-To: <1403667127-1622-1-git-send-email-qiudayu@linux.vnet.ibm.com>

On Tue, Jun 24, 2014 at 11:32:07PM -0400, Mike Qiu wrote:

[ cc Richard ]

>Eeh sysfs entry created must be after EEH_ENABLED been set
>in eeh_subsystem_flags.
>
>In PowerNV platform, it try to create sysfs entry before
>EEH_ENABLED been set, when boot up. So nothing will be
>created for eeh in sysfs.
>

Could you please make the commit log more clear? :-)

I guess the issue is introduced by commit 2213fb1 ("
powerpc/eeh: Skip eeh sysfs when eeh is disabled"). The
commit checks EEH is enabled while creating PCI device
EEH sysfs files. If not, the sysfs files won't be created.
That's to avoid warning reported during PCI hotplug.

The problem you're reporting (if I understand completely):
You don't see the sysfs files after the system boots up.
If it's the case, you probably need following changes in
arch/powerpc/platforms/powernv/pci.c::pnv_pci_ioda_fixup().
Could you have a try with it?

#ifdef CONFIG_EEH
	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
-	eeh_addr_cache_build();
	eeh_init();
+	eeh_addr_cache_build();
#endif

Eventually PowerNV/pSeries have same function call sequence:

- Set EEH probe mode
- Doing probe (with device node or PCI device)
- Build address cache.
 
>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>---
> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>index 8ad0c5b..5f95581 100644
>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>@@ -136,6 +136,9 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
> 	struct pnv_phb *phb = hose->private_data;
> 	int ret;
>
>+	/* Creat sysfs after EEH_ENABLED been set */
>+	eeh_add_sysfs_files(hose->bus);
>+
> 	/* Register OPAL event notifier */
> 	if (!ioda_eeh_nb_init) {
> 		ret = opal_notifier_register(&ioda_eeh_nb);

Thanks,
Gavin

^ permalink raw reply

* Re: [RFC PATCH V3 06/17] ppc/pnv: allocate pe->iommu_table dynamically
From: Wei Yang @ 2014-06-25  5:27 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Wei Yang, benh, linux-pci, gwshan, yan, bhelgaas, qiudayu,
	linuxppc-dev
In-Reply-To: <53AA4C32.7060004@ozlabs.ru>

On Wed, Jun 25, 2014 at 02:12:34PM +1000, Alexey Kardashevskiy wrote:
>On 06/25/2014 11:12 AM, Wei Yang wrote:
>> On Tue, Jun 24, 2014 at 08:06:32PM +1000, Alexey Kardashevskiy wrote:
>>> On 06/10/2014 11:56 AM, Wei Yang wrote:
>>>> Current iommu_table of a PE is a static field. This will have a problem when
>>>> iommu_free_table is called.
>>>
>>> What kind of problem? This table is per PE and PE is not going anywhere.
>>>
>> 
>> Yes, for Bus PE, they will always sit in the system. When VF PE introduced,
>> they could be released on the fly. When they are released, so do the iommu
>> table for the PE.
>
>iommu_table is a part of PE struct. When PE is released, iommu_table will
>go with it as well. Why to make is a pointer? I would understand it if you
>added reference counting there but no - iommu_table's lifetime is equal to
>PE lifetime.
>

Yes, iommu_talbe's life time equals to PE lifetime, so when releasing a PE we
need to release the iommu table. Currently, there is one function to release
the iommu table, iommu_free_table() which takes a pointer of the iommu_table
and release it.

If the iommu table in PE is just a part of PE, it will have some problem to
release it with iommu_free_table(). That's why I make it a pointer in PE
structure.

>
>
>-- 
>Alexey

-- 
Richard Yang
Help you, Help me

^ 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