* Re: [PATCH 1/6] Docs: dt: add fsl-mc iommu-parent device-tree binding
From: Robin Murphy @ 2018-03-05 14:53 UTC (permalink / raw)
To: Nipun Gupta, will.deacon, mark.rutland, catalin.marinas
Cc: iommu, robh+dt, hch, m.szyprowski, gregkh, joro, leoyang.li,
shawnguo, linux-kernel, devicetree, linux-arm-kernel,
linuxppc-dev, bharat.bhushan, stuyoder, laurentiu.tudor
In-Reply-To: <1520260166-29387-2-git-send-email-nipun.gupta@nxp.com>
On 05/03/18 14:29, Nipun Gupta wrote:
> The existing IOMMU bindings cannot be used to specify the relationship
> between fsl-mc devices and IOMMUs. This patch adds a binding for
> mapping fsl-mc devices to IOMMUs, using a new iommu-parent property.
Given that allowing "msi-parent" for #msi-cells > 1 is merely a
backward-compatibility bodge full of hard-coded assumptions, why would
we want to knowingly introduce a similarly unpleasant equivalent for
IOMMUs? What's wrong with "iommu-map"?
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 6611a7c..011c7d6 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -9,6 +9,24 @@ blocks that can be used to create functional hardware objects/devices
> such as network interfaces, crypto accelerator instances, L2 switches,
> etc.
>
> +For an overview of the DPAA2 architecture and fsl-mc bus see:
> +drivers/staging/fsl-mc/README.txt
> +
> +As described in the above overview, all DPAA2 objects in a DPRC share the
> +same hardware "isolation context" and a 10-bit value called an ICID
> +(isolation context id) is expressed by the hardware to identify
> +the requester.
IOW, precisely the case for which "{msi,iommu}-map" exist. Yes, I know
they're currently documented under bindings/pci, but they're not really
intended to be absolutely PCI-specific.
Robin.
> +The generic 'iommus' property is cannot be used to describe the relationship
> +between fsl-mc and IOMMUs, so an iommu-parent property is used to define
> +the same.
> +
> +For generic IOMMU bindings, see
> +Documentation/devicetree/bindings/iommu/iommu.txt.
> +
> +For arm-smmu binding, see:
> +Documentation/devicetree/bindings/iommu/arm,smmu.txt.
> +
> Required properties:
>
> - compatible
> @@ -88,14 +106,27 @@ Sub-nodes:
> Value type: <phandle>
> Definition: Specifies the phandle to the PHY device node associated
> with the this dpmac.
> +Optional properties:
> +
> +- iommu-parent: Maps the devices on fsl-mc bus to an IOMMU.
> + The property specifies the IOMMU behind which the devices on
> + fsl-mc bus are residing.
>
> Example:
>
> + smmu: iommu@5000000 {
> + compatible = "arm,mmu-500";
> + #iommu-cells = <1>;
> + stream-match-mask = <0x7C00>;
> + ...
> + };
> +
> fsl_mc: fsl-mc@80c000000 {
> compatible = "fsl,qoriq-mc";
> reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> <0x00000000 0x08340000 0 0x40000>; /* MC control reg */
> msi-parent = <&its>;
> + iommu-parent = <&smmu>;
> #address-cells = <3>;
> #size-cells = <1>;
>
>
^ permalink raw reply
* RE: [PATCH 1/6] Docs: dt: add fsl-mc iommu-parent device-tree binding
From: Nipun Gupta @ 2018-03-05 15:00 UTC (permalink / raw)
To: Robin Murphy, will.deacon@arm.com, mark.rutland@arm.com,
catalin.marinas@arm.com
Cc: iommu@lists.linux-foundation.org, robh+dt@kernel.org, hch@lst.de,
m.szyprowski@samsung.com, gregkh@linuxfoundation.org,
joro@8bytes.org, Leo Li, shawnguo@kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linuxppc-dev@lists.ozlabs.org, Bharat Bhushan, stuyoder@gmail.com,
Laurentiu Tudor
In-Reply-To: <5cdeded1-ca3c-339a-bf73-73401e7dd4ed@arm.com>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogUm9iaW4gTXVycGh5IFtt
YWlsdG86cm9iaW4ubXVycGh5QGFybS5jb21dDQo+IFNlbnQ6IE1vbmRheSwgTWFyY2ggMDUsIDIw
MTggMjA6MjMNCj4gVG86IE5pcHVuIEd1cHRhIDxuaXB1bi5ndXB0YUBueHAuY29tPjsgd2lsbC5k
ZWFjb25AYXJtLmNvbTsNCj4gbWFyay5ydXRsYW5kQGFybS5jb207IGNhdGFsaW4ubWFyaW5hc0Bh
cm0uY29tDQo+IENjOiBpb21tdUBsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZzsgcm9iaCtkdEBr
ZXJuZWwub3JnOyBoY2hAbHN0LmRlOw0KPiBtLnN6eXByb3dza2lAc2Ftc3VuZy5jb207IGdyZWdr
aEBsaW51eGZvdW5kYXRpb24ub3JnOyBqb3JvQDhieXRlcy5vcmc7DQo+IExlbyBMaSA8bGVveWFu
Zy5saUBueHAuY29tPjsgc2hhd25ndW9Aa2VybmVsLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2Vy
Lmtlcm5lbC5vcmc7IGRldmljZXRyZWVAdmdlci5rZXJuZWwub3JnOyBsaW51eC1hcm0tDQo+IGtl
cm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsg
QmhhcmF0IEJodXNoYW4NCj4gPGJoYXJhdC5iaHVzaGFuQG54cC5jb20+OyBzdHV5b2RlckBnbWFp
bC5jb207IExhdXJlbnRpdSBUdWRvcg0KPiA8bGF1cmVudGl1LnR1ZG9yQG54cC5jb20+DQo+IFN1
YmplY3Q6IFJlOiBbUEFUQ0ggMS82XSBEb2NzOiBkdDogYWRkIGZzbC1tYyBpb21tdS1wYXJlbnQg
ZGV2aWNlLXRyZWUgYmluZGluZw0KPiANCj4gT24gMDUvMDMvMTggMTQ6MjksIE5pcHVuIEd1cHRh
IHdyb3RlOg0KPiA+IFRoZSBleGlzdGluZyBJT01NVSBiaW5kaW5ncyBjYW5ub3QgYmUgdXNlZCB0
byBzcGVjaWZ5IHRoZSByZWxhdGlvbnNoaXANCj4gPiBiZXR3ZWVuIGZzbC1tYyBkZXZpY2VzIGFu
ZCBJT01NVXMuIFRoaXMgcGF0Y2ggYWRkcyBhIGJpbmRpbmcgZm9yDQo+ID4gbWFwcGluZyBmc2wt
bWMgZGV2aWNlcyB0byBJT01NVXMsIHVzaW5nIGEgbmV3IGlvbW11LXBhcmVudCBwcm9wZXJ0eS4N
Cj4gDQo+IEdpdmVuIHRoYXQgYWxsb3dpbmcgIm1zaS1wYXJlbnQiIGZvciAjbXNpLWNlbGxzID4g
MSBpcyBtZXJlbHkgYQ0KPiBiYWNrd2FyZC1jb21wYXRpYmlsaXR5IGJvZGdlIGZ1bGwgb2YgaGFy
ZC1jb2RlZCBhc3N1bXB0aW9ucywgd2h5IHdvdWxkDQo+IHdlIHdhbnQgdG8ga25vd2luZ2x5IGlu
dHJvZHVjZSBhIHNpbWlsYXJseSB1bnBsZWFzYW50IGVxdWl2YWxlbnQgZm9yDQo+IElPTU1Vcz8g
V2hhdCdzIHdyb25nIHdpdGggImlvbW11LW1hcCI/DQoNCkhpIFJvYmluLA0KDQpXaXRoICdtc2kt
cGFyZW50JyB0aGUgcHJvcGVydHkgaXMgZml4ZWQgdXAgdG8gaGF2ZSBtc2ktbWFwLiBJbiB0aGlz
IGNhc2UgdGhlcmUgaXMNCm5vIGZpeHVwIHJlcXVpcmVkIGFuZCBzaW1wbGUgJ2lvbW11LXBhcmVu
dCcgcHJvcGVydHkgY2FuIGJlIHVzZWQsIHdpdGggTUMgYnVzDQppdHNlbGYgcHJvdmlkaW5nIHRo
ZSBzdHJlYW0taWQncyAoaW4gdGhlIGNvZGUgZXhlY3V0aW9uIHZpYSBGVykuDQoNCldlIGNhbiBh
bHNvIHVzZSB0aGUgaW9tbXUtbWFwIHByb3BlcnR5IHNpbWlsYXIgdG8gUENJLCB3aGljaCB3aWxs
IHJlcXVpcmUgdS1ib290DQpmaXh1cC4gQnV0IHRoZW4gaXQgbGVhZHMgdG8gbGl0dGxlIGJpdCBj
b21wbGljYXRpb25zIG9mIHUtYm9vdCAtIGtlcm5lbCBjb21wYXRpYmlsaXR5Lg0KDQpJZiB5b3Ug
c3VnZ2VzdCB3ZSBjYW4gcmUtdXNlIHRoZSBpb21tdS1tYXAgcHJvcGVydHkuIFdoYXQgaXMgeW91
ciBvcGluaW9uPw0KDQpUaGFua3MsDQpOaXB1bg0KDQo+IA0KPiA+IFNpZ25lZC1vZmYtYnk6IE5p
cHVuIEd1cHRhIDxuaXB1bi5ndXB0YUBueHAuY29tPg0KPiA+IC0tLQ0KPiA+ICAgLi4uL2Rldmlj
ZXRyZWUvYmluZGluZ3MvbWlzYy9mc2wscW9yaXEtbWMudHh0ICAgICAgfCAzMQ0KPiArKysrKysr
KysrKysrKysrKysrKysrDQo+ID4gICAxIGZpbGUgY2hhbmdlZCwgMzEgaW5zZXJ0aW9ucygrKQ0K
PiA+DQo+ID4gZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9t
aXNjL2ZzbCxxb3JpcS1tYy50eHQNCj4gYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGlu
Z3MvbWlzYy9mc2wscW9yaXEtbWMudHh0DQo+ID4gaW5kZXggNjYxMWE3Yy4uMDExYzdkNiAxMDA2
NDQNCj4gPiAtLS0gYS9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvbWlzYy9mc2ws
cW9yaXEtbWMudHh0DQo+ID4gKysrIGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdz
L21pc2MvZnNsLHFvcmlxLW1jLnR4dA0KPiA+IEBAIC05LDYgKzksMjQgQEAgYmxvY2tzIHRoYXQg
Y2FuIGJlIHVzZWQgdG8gY3JlYXRlIGZ1bmN0aW9uYWwgaGFyZHdhcmUNCj4gb2JqZWN0cy9kZXZp
Y2VzDQo+ID4gICBzdWNoIGFzIG5ldHdvcmsgaW50ZXJmYWNlcywgY3J5cHRvIGFjY2VsZXJhdG9y
IGluc3RhbmNlcywgTDIgc3dpdGNoZXMsDQo+ID4gICBldGMuDQo+ID4NCj4gPiArRm9yIGFuIG92
ZXJ2aWV3IG9mIHRoZSBEUEFBMiBhcmNoaXRlY3R1cmUgYW5kIGZzbC1tYyBidXMgc2VlOg0KPiA+
ICtkcml2ZXJzL3N0YWdpbmcvZnNsLW1jL1JFQURNRS50eHQNCj4gPiArDQo+ID4gK0FzIGRlc2Ny
aWJlZCBpbiB0aGUgYWJvdmUgb3ZlcnZpZXcsIGFsbCBEUEFBMiBvYmplY3RzIGluIGEgRFBSQyBz
aGFyZSB0aGUNCj4gPiArc2FtZSBoYXJkd2FyZSAiaXNvbGF0aW9uIGNvbnRleHQiIGFuZCBhIDEw
LWJpdCB2YWx1ZSBjYWxsZWQgYW4gSUNJRA0KPiA+ICsoaXNvbGF0aW9uIGNvbnRleHQgaWQpIGlz
IGV4cHJlc3NlZCBieSB0aGUgaGFyZHdhcmUgdG8gaWRlbnRpZnkNCj4gPiArdGhlIHJlcXVlc3Rl
ci4NCj4gDQo+IElPVywgcHJlY2lzZWx5IHRoZSBjYXNlIGZvciB3aGljaCAie21zaSxpb21tdX0t
bWFwIiBleGlzdC4gWWVzLCBJIGtub3cNCj4gdGhleSdyZSBjdXJyZW50bHkgZG9jdW1lbnRlZCB1
bmRlciBiaW5kaW5ncy9wY2ksIGJ1dCB0aGV5J3JlIG5vdCByZWFsbHkNCj4gaW50ZW5kZWQgdG8g
YmUgYWJzb2x1dGVseSBQQ0ktc3BlY2lmaWMuDQo+IA0KPiBSb2Jpbi4NCj4gDQo+ID4gK1RoZSBn
ZW5lcmljICdpb21tdXMnIHByb3BlcnR5IGlzIGNhbm5vdCBiZSB1c2VkIHRvIGRlc2NyaWJlIHRo
ZSByZWxhdGlvbnNoaXANCj4gPiArYmV0d2VlbiBmc2wtbWMgYW5kIElPTU1Vcywgc28gYW4gaW9t
bXUtcGFyZW50IHByb3BlcnR5IGlzIHVzZWQgdG8gZGVmaW5lDQo+ID4gK3RoZSBzYW1lLg0KPiA+
ICsNCj4gPiArRm9yIGdlbmVyaWMgSU9NTVUgYmluZGluZ3MsIHNlZQ0KPiA+ICtEb2N1bWVudGF0
aW9uL2RldmljZXRyZWUvYmluZGluZ3MvaW9tbXUvaW9tbXUudHh0Lg0KPiA+ICsNCj4gPiArRm9y
IGFybS1zbW11IGJpbmRpbmcsIHNlZToNCj4gPiArRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2Jp
bmRpbmdzL2lvbW11L2FybSxzbW11LnR4dC4NCj4gPiArDQo+ID4gICBSZXF1aXJlZCBwcm9wZXJ0
aWVzOg0KPiA+DQo+ID4gICAgICAgLSBjb21wYXRpYmxlDQo+ID4gQEAgLTg4LDE0ICsxMDYsMjcg
QEAgU3ViLW5vZGVzOg0KPiA+ICAgICAgICAgICAgICAgICBWYWx1ZSB0eXBlOiA8cGhhbmRsZT4N
Cj4gPiAgICAgICAgICAgICAgICAgRGVmaW5pdGlvbjogU3BlY2lmaWVzIHRoZSBwaGFuZGxlIHRv
IHRoZSBQSFkgZGV2aWNlIG5vZGUgYXNzb2NpYXRlZA0KPiA+ICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICB3aXRoIHRoZSB0aGlzIGRwbWFjLg0KPiA+ICtPcHRpb25hbCBwcm9wZXJ0aWVzOg0K
PiA+ICsNCj4gPiArLSBpb21tdS1wYXJlbnQ6IE1hcHMgdGhlIGRldmljZXMgb24gZnNsLW1jIGJ1
cyB0byBhbiBJT01NVS4NCj4gPiArICBUaGUgcHJvcGVydHkgc3BlY2lmaWVzIHRoZSBJT01NVSBi
ZWhpbmQgd2hpY2ggdGhlIGRldmljZXMgb24NCj4gPiArICBmc2wtbWMgYnVzIGFyZSByZXNpZGlu
Zy4NCj4gPg0KPiA+ICAgRXhhbXBsZToNCj4gPg0KPiA+ICsgICAgICAgIHNtbXU6IGlvbW11QDUw
MDAwMDAgew0KPiA+ICsgICAgICAgICAgICAgICBjb21wYXRpYmxlID0gImFybSxtbXUtNTAwIjsN
Cj4gPiArICAgICAgICAgICAgICAgI2lvbW11LWNlbGxzID0gPDE+Ow0KPiA+ICsgICAgICAgICAg
ICAgICBzdHJlYW0tbWF0Y2gtbWFzayA9IDwweDdDMDA+Ow0KPiA+ICsgICAgICAgICAgICAgICAu
Li4NCj4gPiArICAgICAgICB9Ow0KPiA+ICsNCj4gPiAgICAgICAgICAgZnNsX21jOiBmc2wtbWNA
ODBjMDAwMDAwIHsNCj4gPiAgICAgICAgICAgICAgICAgICBjb21wYXRpYmxlID0gImZzbCxxb3Jp
cS1tYyI7DQo+ID4gICAgICAgICAgICAgICAgICAgcmVnID0gPDB4MDAwMDAwMDggMHgwYzAwMDAw
MCAwIDB4NDA+LCAgICAvKiBNQyBwb3J0YWwgYmFzZSAqLw0KPiA+ICAgICAgICAgICAgICAgICAg
ICAgICAgIDwweDAwMDAwMDAwIDB4MDgzNDAwMDAgMCAweDQwMDAwPjsgLyogTUMgY29udHJvbCBy
ZWcgKi8NCj4gPiAgICAgICAgICAgICAgICAgICBtc2ktcGFyZW50ID0gPCZpdHM+Ow0KPiA+ICsg
ICAgICAgICAgICAgICAgaW9tbXUtcGFyZW50ID0gPCZzbW11PjsNCj4gPiAgICAgICAgICAgICAg
ICAgICAjYWRkcmVzcy1jZWxscyA9IDwzPjsNCj4gPiAgICAgICAgICAgICAgICAgICAjc2l6ZS1j
ZWxscyA9IDwxPjsNCj4gPg0KPiA+DQo=
^ permalink raw reply
* Re: [PATCH 5/6] dma-mapping: support fsl-mc bus
From: Christoph Hellwig @ 2018-03-05 15:08 UTC (permalink / raw)
To: Nipun Gupta
Cc: will.deacon, robin.murphy, mark.rutland, catalin.marinas, iommu,
robh+dt, hch, m.szyprowski, gregkh, joro, leoyang.li, shawnguo,
linux-kernel, devicetree, linux-arm-kernel, linuxppc-dev,
bharat.bhushan, stuyoder, laurentiu.tudor
In-Reply-To: <1520260166-29387-6-git-send-email-nipun.gupta@nxp.com>
We should not add any new hardocded busses here. Please mark them in
OF/ACPI.
^ permalink raw reply
* Re: [PATCH 1/6] Docs: dt: add fsl-mc iommu-parent device-tree binding
From: Robin Murphy @ 2018-03-05 15:37 UTC (permalink / raw)
To: Nipun Gupta, will.deacon@arm.com, mark.rutland@arm.com,
catalin.marinas@arm.com
Cc: iommu@lists.linux-foundation.org, robh+dt@kernel.org, hch@lst.de,
m.szyprowski@samsung.com, gregkh@linuxfoundation.org,
joro@8bytes.org, Leo Li, shawnguo@kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linuxppc-dev@lists.ozlabs.org, Bharat Bhushan, stuyoder@gmail.com,
Laurentiu Tudor
In-Reply-To: <HE1PR0401MB24254CBDB8B6F537D08B925DE6DA0@HE1PR0401MB2425.eurprd04.prod.outlook.com>
On 05/03/18 15:00, Nipun Gupta wrote:
>
>
>> -----Original Message-----
>> From: Robin Murphy [mailto:robin.murphy@arm.com]
>> Sent: Monday, March 05, 2018 20:23
>> To: Nipun Gupta <nipun.gupta@nxp.com>; will.deacon@arm.com;
>> mark.rutland@arm.com; catalin.marinas@arm.com
>> Cc: iommu@lists.linux-foundation.org; robh+dt@kernel.org; hch@lst.de;
>> m.szyprowski@samsung.com; gregkh@linuxfoundation.org; joro@8bytes.org;
>> Leo Li <leoyang.li@nxp.com>; shawnguo@kernel.org; linux-
>> kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org; Bharat Bhushan
>> <bharat.bhushan@nxp.com>; stuyoder@gmail.com; Laurentiu Tudor
>> <laurentiu.tudor@nxp.com>
>> Subject: Re: [PATCH 1/6] Docs: dt: add fsl-mc iommu-parent device-tree binding
>>
>> On 05/03/18 14:29, Nipun Gupta wrote:
>>> The existing IOMMU bindings cannot be used to specify the relationship
>>> between fsl-mc devices and IOMMUs. This patch adds a binding for
>>> mapping fsl-mc devices to IOMMUs, using a new iommu-parent property.
>>
>> Given that allowing "msi-parent" for #msi-cells > 1 is merely a
>> backward-compatibility bodge full of hard-coded assumptions, why would
>> we want to knowingly introduce a similarly unpleasant equivalent for
>> IOMMUs? What's wrong with "iommu-map"?
>
> Hi Robin,
>
> With 'msi-parent' the property is fixed up to have msi-map. In this case there is
> no fixup required and simple 'iommu-parent' property can be used, with MC bus
> itself providing the stream-id's (in the code execution via FW).
>
> We can also use the iommu-map property similar to PCI, which will require u-boot
> fixup. But then it leads to little bit complications of u-boot - kernel compatibility.
What needs fixing up? With a stream-map-mask in place to ignore the
upper Stream ID bits, you just need:
iommu-map = <0 &smmu 0 0x80>;
to say that the lower bits of the ICID value map directly to the lower
bits of the Stream ID value - that's the same fixed property of the
hardware that you're wanting to assume in iommu-parent.
> If you suggest we can re-use the iommu-map property. What is your opinion?
I think it makes a lot more sense to directly use the property which
already exists, than to introduce a new one to merely assume one
hard-coded value of the existing one. Extending msi-parent to msi-map
was a case of "oops, it turns out we need more flexibility here"; for
the case of iommu-map I can't imagine any justification for saying
"oops, we need less flexibility here" (saving 9 whole bytes in the DT
really is irrelevant).
Robin.
^ permalink raw reply
* Re: [PATCH 5/6] dma-mapping: support fsl-mc bus
From: Robin Murphy @ 2018-03-05 15:48 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Nipun Gupta, will.deacon, mark.rutland, catalin.marinas, iommu,
robh+dt, m.szyprowski, gregkh, joro, leoyang.li, shawnguo,
linux-kernel, devicetree, linux-arm-kernel, linuxppc-dev,
bharat.bhushan, stuyoder, laurentiu.tudor
In-Reply-To: <20180305150814.GA15918@lst.de>
On 05/03/18 15:08, Christoph Hellwig wrote:
> We should not add any new hardocded busses here. Please mark them in
> OF/ACPI.
Unfortunately for us, fsl-mc is conceptually rather like PCI in that
it's software-discoverable and the only thing described in DT is the bus
"host", thus we need the same sort of thing as for PCI to map from the
child devices back to the bus root in order to find the appropriate
firmware node. Worse than PCI, though, we wouldn't even have the option
of describing child devices statically in firmware at all, since it's
actually one of these runtime-configurable "build your own network
accelerator" hardware pools where userspace gets to create and destroy
"devices" as it likes.
Robin.
^ permalink raw reply
* RE: [PATCH 1/6] Docs: dt: add fsl-mc iommu-parent device-tree binding
From: Nipun Gupta @ 2018-03-05 15:54 UTC (permalink / raw)
To: Robin Murphy, will.deacon@arm.com, mark.rutland@arm.com,
catalin.marinas@arm.com
Cc: iommu@lists.linux-foundation.org, robh+dt@kernel.org, hch@lst.de,
m.szyprowski@samsung.com, gregkh@linuxfoundation.org,
joro@8bytes.org, Leo Li, shawnguo@kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linuxppc-dev@lists.ozlabs.org, Bharat Bhushan, stuyoder@gmail.com,
Laurentiu Tudor
In-Reply-To: <f382e92a-8ab5-7c93-1f39-fe6797e5c876@arm.com>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogUm9iaW4gTXVycGh5IFtt
YWlsdG86cm9iaW4ubXVycGh5QGFybS5jb21dDQo+IFNlbnQ6IE1vbmRheSwgTWFyY2ggMDUsIDIw
MTggMjE6MDcNCj4gVG86IE5pcHVuIEd1cHRhIDxuaXB1bi5ndXB0YUBueHAuY29tPjsgd2lsbC5k
ZWFjb25AYXJtLmNvbTsNCj4gbWFyay5ydXRsYW5kQGFybS5jb207IGNhdGFsaW4ubWFyaW5hc0Bh
cm0uY29tDQo+IENjOiBpb21tdUBsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZzsgcm9iaCtkdEBr
ZXJuZWwub3JnOyBoY2hAbHN0LmRlOw0KPiBtLnN6eXByb3dza2lAc2Ftc3VuZy5jb207IGdyZWdr
aEBsaW51eGZvdW5kYXRpb24ub3JnOyBqb3JvQDhieXRlcy5vcmc7DQo+IExlbyBMaSA8bGVveWFu
Zy5saUBueHAuY29tPjsgc2hhd25ndW9Aa2VybmVsLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2Vy
Lmtlcm5lbC5vcmc7IGRldmljZXRyZWVAdmdlci5rZXJuZWwub3JnOyBsaW51eC1hcm0tDQo+IGtl
cm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsg
QmhhcmF0IEJodXNoYW4NCj4gPGJoYXJhdC5iaHVzaGFuQG54cC5jb20+OyBzdHV5b2RlckBnbWFp
bC5jb207IExhdXJlbnRpdSBUdWRvcg0KPiA8bGF1cmVudGl1LnR1ZG9yQG54cC5jb20+DQo+IFN1
YmplY3Q6IFJlOiBbUEFUQ0ggMS82XSBEb2NzOiBkdDogYWRkIGZzbC1tYyBpb21tdS1wYXJlbnQg
ZGV2aWNlLXRyZWUgYmluZGluZw0KPiANCj4gT24gMDUvMDMvMTggMTU6MDAsIE5pcHVuIEd1cHRh
IHdyb3RlOg0KPiA+DQo+ID4NCj4gPj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPj4g
RnJvbTogUm9iaW4gTXVycGh5IFttYWlsdG86cm9iaW4ubXVycGh5QGFybS5jb21dDQo+ID4+IFNl
bnQ6IE1vbmRheSwgTWFyY2ggMDUsIDIwMTggMjA6MjMNCj4gPj4gVG86IE5pcHVuIEd1cHRhIDxu
aXB1bi5ndXB0YUBueHAuY29tPjsgd2lsbC5kZWFjb25AYXJtLmNvbTsNCj4gPj4gbWFyay5ydXRs
YW5kQGFybS5jb207IGNhdGFsaW4ubWFyaW5hc0Bhcm0uY29tDQo+ID4+IENjOiBpb21tdUBsaXN0
cy5saW51eC1mb3VuZGF0aW9uLm9yZzsgcm9iaCtkdEBrZXJuZWwub3JnOyBoY2hAbHN0LmRlOw0K
PiA+PiBtLnN6eXByb3dza2lAc2Ftc3VuZy5jb207IGdyZWdraEBsaW51eGZvdW5kYXRpb24ub3Jn
Ow0KPiBqb3JvQDhieXRlcy5vcmc7DQo+ID4+IExlbyBMaSA8bGVveWFuZy5saUBueHAuY29tPjsg
c2hhd25ndW9Aa2VybmVsLm9yZzsgbGludXgtDQo+ID4+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7
IGRldmljZXRyZWVAdmdlci5rZXJuZWwub3JnOyBsaW51eC1hcm0tDQo+ID4+IGtlcm5lbEBsaXN0
cy5pbmZyYWRlYWQub3JnOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgQmhhcmF0IEJo
dXNoYW4NCj4gPj4gPGJoYXJhdC5iaHVzaGFuQG54cC5jb20+OyBzdHV5b2RlckBnbWFpbC5jb207
IExhdXJlbnRpdSBUdWRvcg0KPiA+PiA8bGF1cmVudGl1LnR1ZG9yQG54cC5jb20+DQo+ID4+IFN1
YmplY3Q6IFJlOiBbUEFUQ0ggMS82XSBEb2NzOiBkdDogYWRkIGZzbC1tYyBpb21tdS1wYXJlbnQg
ZGV2aWNlLXRyZWUNCj4gYmluZGluZw0KPiA+Pg0KPiA+PiBPbiAwNS8wMy8xOCAxNDoyOSwgTmlw
dW4gR3VwdGEgd3JvdGU6DQo+ID4+PiBUaGUgZXhpc3RpbmcgSU9NTVUgYmluZGluZ3MgY2Fubm90
IGJlIHVzZWQgdG8gc3BlY2lmeSB0aGUgcmVsYXRpb25zaGlwDQo+ID4+PiBiZXR3ZWVuIGZzbC1t
YyBkZXZpY2VzIGFuZCBJT01NVXMuIFRoaXMgcGF0Y2ggYWRkcyBhIGJpbmRpbmcgZm9yDQo+ID4+
PiBtYXBwaW5nIGZzbC1tYyBkZXZpY2VzIHRvIElPTU1VcywgdXNpbmcgYSBuZXcgaW9tbXUtcGFy
ZW50IHByb3BlcnR5Lg0KPiA+Pg0KPiA+PiBHaXZlbiB0aGF0IGFsbG93aW5nICJtc2ktcGFyZW50
IiBmb3IgI21zaS1jZWxscyA+IDEgaXMgbWVyZWx5IGENCj4gPj4gYmFja3dhcmQtY29tcGF0aWJp
bGl0eSBib2RnZSBmdWxsIG9mIGhhcmQtY29kZWQgYXNzdW1wdGlvbnMsIHdoeSB3b3VsZA0KPiA+
PiB3ZSB3YW50IHRvIGtub3dpbmdseSBpbnRyb2R1Y2UgYSBzaW1pbGFybHkgdW5wbGVhc2FudCBl
cXVpdmFsZW50IGZvcg0KPiA+PiBJT01NVXM/IFdoYXQncyB3cm9uZyB3aXRoICJpb21tdS1tYXAi
Pw0KPiA+DQo+ID4gSGkgUm9iaW4sDQo+ID4NCj4gPiBXaXRoICdtc2ktcGFyZW50JyB0aGUgcHJv
cGVydHkgaXMgZml4ZWQgdXAgdG8gaGF2ZSBtc2ktbWFwLiBJbiB0aGlzIGNhc2UgdGhlcmUgaXMN
Cj4gPiBubyBmaXh1cCByZXF1aXJlZCBhbmQgc2ltcGxlICdpb21tdS1wYXJlbnQnIHByb3BlcnR5
IGNhbiBiZSB1c2VkLCB3aXRoIE1DDQo+IGJ1cw0KPiA+IGl0c2VsZiBwcm92aWRpbmcgdGhlIHN0
cmVhbS1pZCdzIChpbiB0aGUgY29kZSBleGVjdXRpb24gdmlhIEZXKS4NCj4gPg0KPiA+IFdlIGNh
biBhbHNvIHVzZSB0aGUgaW9tbXUtbWFwIHByb3BlcnR5IHNpbWlsYXIgdG8gUENJLCB3aGljaCB3
aWxsIHJlcXVpcmUgdS0NCj4gYm9vdA0KPiA+IGZpeHVwLiBCdXQgdGhlbiBpdCBsZWFkcyB0byBs
aXR0bGUgYml0IGNvbXBsaWNhdGlvbnMgb2YgdS1ib290IC0ga2VybmVsDQo+IGNvbXBhdGliaWxp
dHkuDQo+IA0KPiBXaGF0IG5lZWRzIGZpeGluZyB1cD8gV2l0aCBhIHN0cmVhbS1tYXAtbWFzayBp
biBwbGFjZSB0byBpZ25vcmUgdGhlDQo+IHVwcGVyIFN0cmVhbSBJRCBiaXRzLCB5b3UganVzdCBu
ZWVkOg0KPiANCj4gCWlvbW11LW1hcCA9IDwwICZzbW11IDAgMHg4MD47DQo+IA0KPiB0byBzYXkg
dGhhdCB0aGUgbG93ZXIgYml0cyBvZiB0aGUgSUNJRCB2YWx1ZSBtYXAgZGlyZWN0bHkgdG8gdGhl
IGxvd2VyDQo+IGJpdHMgb2YgdGhlIFN0cmVhbSBJRCB2YWx1ZSAtIHRoYXQncyB0aGUgc2FtZSBm
aXhlZCBwcm9wZXJ0eSBvZiB0aGUNCj4gaGFyZHdhcmUgdGhhdCB5b3UncmUgd2FudGluZyB0byBh
c3N1bWUgaW4gaW9tbXUtcGFyZW50Lg0KDQpNYWtlcyBzZW5zZS4gSSB3YXMgZ29pbmcgaW4gYSBs
aXR0bGUgYml0IHdyb25nIGRpcmVjdGlvbi4gVGhhbmtzIGZvciBjb3JyZWN0aW5nLg0KSSB3aWxs
IHNlbmQgdjIgcGF0Y2hzZXQgd2l0aCBpb21tdS1tYXAgcHJvcGVydHkuDQoNClJlZ2FyZHMsDQpO
aXB1bg0KDQo+IA0KPiA+IElmIHlvdSBzdWdnZXN0IHdlIGNhbiByZS11c2UgdGhlIGlvbW11LW1h
cCBwcm9wZXJ0eS4gV2hhdCBpcyB5b3VyIG9waW5pb24/DQo+IA0KPiBJIHRoaW5rIGl0IG1ha2Vz
IGEgbG90IG1vcmUgc2Vuc2UgdG8gZGlyZWN0bHkgdXNlIHRoZSBwcm9wZXJ0eSB3aGljaA0KPiBh
bHJlYWR5IGV4aXN0cywgdGhhbiB0byBpbnRyb2R1Y2UgYSBuZXcgb25lIHRvIG1lcmVseSBhc3N1
bWUgb25lDQo+IGhhcmQtY29kZWQgdmFsdWUgb2YgdGhlIGV4aXN0aW5nIG9uZS4gRXh0ZW5kaW5n
IG1zaS1wYXJlbnQgdG8gbXNpLW1hcA0KPiB3YXMgYSBjYXNlIG9mICJvb3BzLCBpdCB0dXJucyBv
dXQgd2UgbmVlZCBtb3JlIGZsZXhpYmlsaXR5IGhlcmUiOyBmb3INCj4gdGhlIGNhc2Ugb2YgaW9t
bXUtbWFwIEkgY2FuJ3QgaW1hZ2luZSBhbnkganVzdGlmaWNhdGlvbiBmb3Igc2F5aW5nDQo+ICJv
b3BzLCB3ZSBuZWVkIGxlc3MgZmxleGliaWxpdHkgaGVyZSIgKHNhdmluZyA5IHdob2xlIGJ5dGVz
IGluIHRoZSBEVA0KPiByZWFsbHkgaXMgaXJyZWxldmFudCkuDQo+IA0KPiBSb2Jpbi4NCg==
^ permalink raw reply
* [PATCH v2] On ppc64le we HAVE_RELIABLE_STACKTRACE
From: Torsten Duwe @ 2018-03-05 16:49 UTC (permalink / raw)
To: Michael Ellerman
Cc: Jiri Kosina, Josh Poimboeuf, linuxppc-dev, linux-kernel,
Nicholas Piggin, live-patching
The "Power Architecture 64-Bit ELF V2 ABI" says in section 2.3.2.3:
[...] There are several rules that must be adhered to in order to ensure
reliable and consistent call chain backtracing:
* Before a function calls any other function, it shall establish its
own stack frame, whose size shall be a multiple of 16 bytes.
– In instances where a function’s prologue creates a stack frame, the
back-chain word of the stack frame shall be updated atomically with
the value of the stack pointer (r1) when a back chain is implemented.
(This must be supported as default by all ELF V2 ABI-compliant
environments.)
[...]
– The function shall save the link register that contains its return
address in the LR save doubleword of its caller’s stack frame before
calling another function.
To me this sounds like the equivalent of HAVE_RELIABLE_STACKTRACE.
This patch may be unneccessarily limited to ppc64le, but OTOH the only
user of this flag so far is livepatching, which is only implemented on
PPCs with 64-LE, a.k.a. ELF ABI v2.
This change also implements save_stack_trace_tsk_reliable() for ppc64
that checks for the above conditions, where possible.
Signed-off-by: Torsten Duwe <duwe@suse.de>
---
v2:
* implemented save_stack_trace_tsk_reliable(), with a bunch of sanity
checks. The test for a kernel code pointer is much nicer now, and
the exit condition is exact (when compared to last week's follow-up)
---
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 73ce5dd07642..9f49913e19e3 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -220,6 +220,7 @@ config PPC
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE if SMP
select HAVE_REGS_AND_STACK_ACCESS_API
+ select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_VIRT_CPU_ACCOUNTING
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index d534ed901538..e14c2dfd5311 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -11,8 +11,11 @@
*/
#include <linux/export.h>
+#include <linux/kallsyms.h>
+#include <linux/module.h>
#include <linux/sched.h>
#include <linux/sched/debug.h>
+#include <linux/sched/task_stack.h>
#include <linux/stacktrace.h>
#include <asm/ptrace.h>
#include <asm/processor.h>
@@ -76,3 +79,77 @@ save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
save_context_stack(trace, regs->gpr[1], current, 0);
}
EXPORT_SYMBOL_GPL(save_stack_trace_regs);
+
+#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
+int
+save_stack_trace_tsk_reliable(struct task_struct *tsk,
+ struct stack_trace *trace)
+{
+ unsigned long sp;
+ unsigned long stack_page = (unsigned long)task_stack_page(tsk);
+
+ /* The last frame (unwinding first) may not yet have saved
+ * its LR onto the stack.
+ */
+ int firstframe = 1;
+
+ if (tsk == current)
+ sp = current_stack_pointer();
+ else
+ sp = tsk->thread.ksp;
+
+ if (sp < stack_page + sizeof(struct thread_struct)
+ || sp > stack_page + THREAD_SIZE - STACK_FRAME_OVERHEAD)
+ return 1;
+
+ for (;;) {
+ unsigned long *stack = (unsigned long *) sp;
+ unsigned long newsp, ip;
+
+ /* sanity check: ABI requires SP to be aligned 16 bytes. */
+ if (sp & 0xF)
+ return 1;
+
+ newsp = stack[0];
+ /* Stack grows downwards; unwinder may only go up. */
+ if (newsp <= sp)
+ return 1;
+
+ if (newsp >= stack_page + THREAD_SIZE)
+ return 1; /* invalid backlink, too far up. */
+
+ /* Examine the saved LR: it must point into kernel code. */
+ ip = stack[STACK_FRAME_LR_SAVE];
+ if (!firstframe) {
+ if (!func_ptr_is_kernel_text((void *)ip)) {
+#ifdef CONFIG_MODULES
+ struct module *mod = __module_text_address(ip);
+
+ if (!mod)
+#endif
+ return 1;
+ }
+ }
+ firstframe = 0;
+
+ if (!trace->skip)
+ trace->entries[trace->nr_entries++] = ip;
+ else
+ trace->skip--;
+
+ /* SP value loaded on kernel entry, see "PACAKSAVE(r13)" in
+ * _switch() and system_call_common()
+ */
+ if (newsp == stack_page + THREAD_SIZE - /* SWITCH_FRAME_SIZE */
+ (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs)))
+ break;
+
+ if (trace->nr_entries >= trace->max_entries)
+ return -E2BIG;
+
+ sp = newsp;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk_reliable);
+#endif /* CONFIG_HAVE_RELIABLE_STACKTRACE */
^ permalink raw reply related
* Re: [PATCH v2] On ppc64le we HAVE_RELIABLE_STACKTRACE
From: Segher Boessenkool @ 2018-03-05 17:09 UTC (permalink / raw)
To: Torsten Duwe
Cc: Michael Ellerman, Jiri Kosina, linux-kernel, Nicholas Piggin,
Josh Poimboeuf, live-patching, linuxppc-dev
In-Reply-To: <20180305164928.GA17953@lst.de>
On Mon, Mar 05, 2018 at 05:49:28PM +0100, Torsten Duwe wrote:
> The "Power Architecture 64-Bit ELF V2 ABI" says in section 2.3.2.3:
>
> [...] There are several rules that must be adhered to in order to ensure
> reliable and consistent call chain backtracing:
>
> * Before a function calls any other function, it shall establish its
> own stack frame, whose size shall be a multiple of 16 bytes.
>
> – In instances where a function’s prologue creates a stack frame, the
> back-chain word of the stack frame shall be updated atomically with
> the value of the stack pointer (r1) when a back chain is implemented.
> (This must be supported as default by all ELF V2 ABI-compliant
> environments.)
> [...]
> – The function shall save the link register that contains its return
> address in the LR save doubleword of its caller’s stack frame before
> calling another function.
All of this is also true for the other PowerPC ABIs, fwiw (both 32-bit
and 64-bit; the offset of the LR save slot isn't the same in all ABIs).
Segher
^ permalink raw reply
* Re: [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls
From: Alexei Starovoitov @ 2018-03-05 17:02 UTC (permalink / raw)
To: Naveen N. Rao, Daniel Borkmann, Sandipan Das
Cc: jakub.kicinski, linuxppc-dev, mpe, netdev
In-Reply-To: <1519891203.b146m3c5tj.naveen@linux.ibm.com>
On 3/1/18 12:51 AM, Naveen N. Rao wrote:
> Daniel Borkmann wrote:
>> On 02/27/2018 01:13 PM, Sandipan Das wrote:
>>> With this patch, it will look like this:
>>> 0: (85) call pc+2#bpf_prog_8f85936f29a7790a+3
>>
>> (Note the +2 is the insn->off already.)
>>
>>> 1: (b7) r0 = 1
>>> 2: (95) exit
>>> 3: (b7) r0 = 2
>>> 4: (95) exit
>>>
>>> where 8f85936f29a7790a is the tag of the bpf program and 3 is
>>> the offset to the start of the subprog from the start of the
>>> program.
>>
>> The problem with this approach would be that right now the name is
>> something like bpf_prog_5f76847930402518_F where the subprog tag is
>> just a placeholder so in future, this may well adapt to e.g. the actual
>> function name from the elf file. Note that when kallsyms is enabled
>> then a name like bpf_prog_5f76847930402518_F will also appear in stack
>> traces, perf records, etc, so for correlation/debugging it would really
>> help to have them the same everywhere.
>>
>> Worst case if there's nothing better, potentially what one could do in
>> bpf_prog_get_info_by_fd() is to dump an array of full addresses and
>> have the imm part as the index pointing to one of them, just unfortunate
>> that it's likely only needed in ppc64.
>
> Ok. We seem to have discussed a few different aspects in this thread.
> Let me summarize the different aspects we have discussed:
> 1. Passing address of JIT'ed function to the JIT engines:
> Two approaches discussed:
> a. Existing approach, where the subprog address is encoded as an
> offset from __bpf_call_base() in imm32 field of the BPF call
> instruction. This requires the JIT'ed function to be within 2GB of
> __bpf_call_base(), which won't be true on ppc64, at the least. So,
> this won't on ppc64 (and any other architectures where vmalloc'ed
> (module_alloc()) memory is from a different, far, address range).
it looks like ppc64 doesn't guarantee today that all of module_alloc()
will be within 32-bit, but I think it should be trivial to add such
guarantee. If so, we can define another __bpf_call_base specifically
for bpf-to-bpf calls when jit is on.
Then jit_subprogs() math will fit:
insn->imm = func[subprog]->bpf_func - __bpf_call_base_for_jited_progs;
and will make it easier for ppc64 jit to optimize and use
near calls for bpf-to-bpf calls while still using trampoline
for bpf-to-kernel.
Also it solves bpftool issue.
For all other archs we can keep
__bpf_call_base_for_jited_progs == __bpf_call_base
> There is a third option we can consider:
> c. Convert BPF pseudo call instruction into a 2-instruction sequence
> (similar to BPF_DW) and encode the full 64-bit call target in the
> second bpf instruction. To distinguish this from other instruction
> forms, we can set imm32 to -1.
Adding new instruction just for that case looks like overkill.
^ permalink raw reply
* Linux 4.16: Reported regressions as of Monday, 2018-03-05 (Was: Linux 4.16-rc4)
From: Thorsten Leemhuis @ 2018-03-05 17:58 UTC (permalink / raw)
To: Linus Torvalds, Linux Kernel Mailing List; +Cc: linuxppc-dev, Jonathan Corbet
In-Reply-To: <CA+55aFxXg8hi+T_DNCG_OrAotqSheyREw-Njf9XgUD1vXqAyHQ@mail.gmail.com>
On 05.03.2018 00:15, Linus Torvalds wrote:
> Hmm. A reasonably calm week - the biggest change is to the 'kvm-stat'
> tool, not any actual kernel files.
Hi! Find below my third regression report for Linux 4.16. It lists 7
regressions I'm currently aware of. 3 were fixed since last weeks report.
To anyone reading this: Are you aware of any other regressions that got
introduced this development cycle? Then please let me know by mail (a
simple bounce or forward to the email address is enough!).
For details see http://bit.ly/lnxregtrackid And please tell me if there
is anything in the report that shouldn't be there.
Ciao, Thorsten
== Current regressions ==
Dell R640 does not boot due to SCSI/SATA failure
Status: Reporter looked into this and indicated the change might have
triggered a firmware bug on his machine
Reported: 2018-02-22 Last known developer activity:
https://marc.info/?l=linux-kernel&m=152026091325037
https://marc.info/?l=linux-kernel&m=151931128006031
Cause: 84676c1f21e8
Linux-Regression-ID: 15a115
[mm, mlock, vmscan] 9c4e6b1a70: stress-ng.hdd.ops_per_sec -7.9% regression
Status: WIP; side note: lkp-robot warned about something else triggered
by the same commit:
https://lkml.kernel.org/r/20180302093940.GE25699@yexl-desktop is related
Note: performance regression found by lkp-robot
Reported: 2018-02-25
https://marc.info/?l=linux-kernel&m=151956997301994
Cause: 9c4e6b1a7027f102990c0395296015a812525f4d
aim7.jobs-per-min -18.0% regression
Status: some discussion last week, but no real solution yet
Note: performance regression found by lkp-robot
Reported: 2018-02-25
https://marc.info/?l=linux-kernel&m=151957120702272&w=2
Cause: c0cef30e4ff0dc025f4a1660b8f0ba43ed58426e
Interrupt storm after suspend causes one busy kworker
Status: stalled?
Reported: 2018-02-25
https://bugzilla.kernel.org/show_bug.cgi?id=198929
Linux-Regression-ID: 41c451
hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks
bluetooth on ASUS T100TA
Status: poked reporters for a update if they reported it to the relevant
developers
Reported: 2018-03-01
https://bugzilla.kernel.org/show_bug.cgi?id=198953
Cause: 43fff768346810042836df325d736bd2c2a634a7
== Regressions with fixes heading mainline ==
selftests: memory-hotplug: fix emit_tests regression
https://marc.info/?l=linux-kernel&m=151993543423651
== Going to get removed from the report ==
Debian kernel package tool make-kpkg stalls indefinitely during kernel
build due to commit "kconfig: remove check_stdin()"
Status: stalled after some discussions; seems nobody really cares that much
Note: From the discussion: "Shouldn't be a problem to back this one out
either if it turns out to cause massive amounts of pain in practice I
guess, even if it's the Debian tools doing something weird."
Reported: 2018-02-12
https://marc.info/?l=linux-kernel&m=151846414807219
Cause: d2a04648a5dbc3d1d043b35257364f0197d4d868
Linux-Regression-ID: 2fd778
== Fixed since last report ==
Dell XPS 13 9360 keyboard no longer works
Status: https://git.kernel.org/torvalds/c/de9647efeaa9
Reported: 2018-02-22
https://marc.info/?l=linux-kernel&m=151927645427980
Cause: 30323fb6d552c41997baca5292bf7001366cab57
on Nokia N900:/dev/input/event6 aka AV Jack support disappeared
Status: https://git.kernel.org/torvalds/c/6662ae6af82d
https://git.kernel.org/torvalds/c/ce27fb2c56db
Reported: 2018-02-24
https://marc.info/?l=linux-omap&m=151950886524308&w=2
Cause: 14e3e295b2b9
Linux-Regression-ID: 4b650f
SD card reader stopped working
Status: Fixed in 4.16.0-rc3 according to reporter
Reported: 2018-02-24
https://bugzilla.kernel.org/show_bug.cgi?id=198917
Linux-Regression-ID: 9adeaf
^ permalink raw reply
* Re: [PATCH 5/6] dma-mapping: support fsl-mc bus
From: Christoph Hellwig @ 2018-03-05 18:39 UTC (permalink / raw)
To: Robin Murphy
Cc: Christoph Hellwig, Nipun Gupta, will.deacon, mark.rutland,
catalin.marinas, iommu, robh+dt, m.szyprowski, gregkh, joro,
leoyang.li, shawnguo, linux-kernel, devicetree, linux-arm-kernel,
linuxppc-dev, bharat.bhushan, stuyoder, laurentiu.tudor
In-Reply-To: <7b4f9972-6aaa-fc9d-3854-d48b19a8051c@arm.com>
On Mon, Mar 05, 2018 at 03:48:32PM +0000, Robin Murphy wrote:
> Unfortunately for us, fsl-mc is conceptually rather like PCI in that it's
> software-discoverable and the only thing described in DT is the bus "host",
> thus we need the same sort of thing as for PCI to map from the child
> devices back to the bus root in order to find the appropriate firmware
> node. Worse than PCI, though, we wouldn't even have the option of
> describing child devices statically in firmware at all, since it's actually
> one of these runtime-configurable "build your own network accelerator"
> hardware pools where userspace gets to create and destroy "devices" as it
> likes.
I really hate the PCI special case just as much. Maybe we just
need a dma_configure method on the bus, and move PCI as well as fsl-mc
to it.
^ permalink raw reply
* Re: [PATCH 5/6] dma-mapping: support fsl-mc bus
From: Robin Murphy @ 2018-03-05 18:51 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Nipun Gupta, will.deacon, mark.rutland, catalin.marinas, iommu,
robh+dt, m.szyprowski, gregkh, joro, leoyang.li, shawnguo,
linux-kernel, devicetree, linux-arm-kernel, linuxppc-dev,
bharat.bhushan, stuyoder, laurentiu.tudor
In-Reply-To: <20180305183938.GB20086@lst.de>
On 05/03/18 18:39, Christoph Hellwig wrote:
> On Mon, Mar 05, 2018 at 03:48:32PM +0000, Robin Murphy wrote:
>> Unfortunately for us, fsl-mc is conceptually rather like PCI in that it's
>> software-discoverable and the only thing described in DT is the bus "host",
>> thus we need the same sort of thing as for PCI to map from the child
>> devices back to the bus root in order to find the appropriate firmware
>> node. Worse than PCI, though, we wouldn't even have the option of
>> describing child devices statically in firmware at all, since it's actually
>> one of these runtime-configurable "build your own network accelerator"
>> hardware pools where userspace gets to create and destroy "devices" as it
>> likes.
>
> I really hate the PCI special case just as much. Maybe we just
> need a dma_configure method on the bus, and move PCI as well as fsl-mc
> to it.
Hmm, on reflection, 100% ack to that idea. It would neatly supersede
bus->force_dma *and* mean that we don't have to effectively pull pci.h
into everything, which I've never liked. In hindsight dma_configure()
does feel like it's grown into this odd choke point where we munge
everything in just for it to awkwardly unpick things again.
Robin.
^ permalink raw reply
* Re: [PATCH AUTOSEL for 4.9 005/219] kretprobes: Ensure probe location is at function entry
From: Sasha Levin @ 2018-03-05 20:06 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
Arnaldo Carvalho de Melo, Ananth N Mavinakayanahalli,
linuxppc-dev@lists.ozlabs.org, Michael Ellerman, Steven Rostedt,
Masami Hiramatsu
In-Reply-To: <1520232322.k5aakv6abc.naveen@linux.ibm.com>
On Mon, Mar 05, 2018 at 12:32:57PM +0530, Naveen N. Rao wrote:
>Hi Sasha,
>
>Sasha Levin wrote:
>>From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
>>
>>[ Upstream commit 90ec5e89e393c76e19afc845d8f88a5dc8315919 ]
>>
>
>Sorry if this is obvious, but why was this patch picked up for=20
>-stable? I don't see the upstream commit tagging -stable, so curious=20
>why this was done.
>
>I don't think this patch should be pushed to -stable since this is not=20
>really a bug fix. There are also other dependencies for this change=20
>(see commit a64e3f35a45f4a, for instance), including how userspace=20
>(perf) builds out the retprobe argument. As such, please drop this=20
>from -stable (for 3.18. 4.4 and 4.9).
Hi Naveen,
It's an automatic selection process that attempts to find commits that
should be in stable but weren't tagged as such.
I'll drop this patch, thanks!
--=20
Thanks,
Sasha=
^ permalink raw reply
* [PATCH] Fixes for selftest tm-unavailable
From: Gustavo Romero @ 2018-03-05 20:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: gromero, mpe, cyrilbur
The recent fix by mpe for tm-trap [1] (thanks for fixing it! really
sorry for the late reply, I got dragged away by the darn thingy)
caught my attention to the fact that tm-unavailable needs a similar
fix. Also before that on Feb. Cyril Bur proposed another small fix for
the same selftest [2]. Since Cyril's change is not merged yet I
decided to take Cyril's fix into account as well. Finally, I also
noted that tm-unavailable is not using the test harness, thus I
added it to tm-unavailable.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=192b2e742c06af399e8eecb4a17265
[2] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-February/169111.html
Gustavo Romero (1):
selftests/powerpc: Skip tm-unavailable if TM is not enabled
.../testing/selftests/powerpc/tm/tm-unavailable.c | 24 ++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH] selftests/powerpc: Skip tm-unavailable if TM is not enabled
From: Gustavo Romero @ 2018-03-05 20:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: gromero, mpe, cyrilbur
In-Reply-To: <1520282935-20111-1-git-send-email-gromero@linux.vnet.ibm.com>
Some processor revisions do not support transactional memory, and
additionally kernel support can be disabled. In either case the
tm-unavailable test should be skipped, otherwise it will fail with
a SIGILL.
That commit also sets this selftest to be called through the test
harness as it's done for other TM selftests.
Finally, it avoids using "ping" as a thread name since it's
ambiguous and can be confusing when shown, for instance,
in a kernel backtrace log.
Fixes: 77fad8bfb1d2 ("selftests/powerpc: Check FP/VEC on exception in TM")
Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
---
.../testing/selftests/powerpc/tm/tm-unavailable.c | 24 ++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/powerpc/tm/tm-unavailable.c b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
index e6a0fad..156c8e7 100644
--- a/tools/testing/selftests/powerpc/tm/tm-unavailable.c
+++ b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
@@ -80,7 +80,7 @@ bool is_failure(uint64_t condition_reg)
return ((condition_reg >> 28) & 0xa) == 0xa;
}
-void *ping(void *input)
+void *tm_una_ping(void *input)
{
/*
@@ -280,7 +280,7 @@ void *ping(void *input)
}
/* Thread to force context switch */
-void *pong(void *not_used)
+void *tm_una_pong(void *not_used)
{
/* Wait thread get its name "pong". */
if (DEBUG)
@@ -311,11 +311,11 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
do {
int rc;
- /* Bind 'ping' to CPU 0, as specified in 'attr'. */
- rc = pthread_create(&t0, attr, ping, (void *) &flags);
+ /* Bind to CPU 0, as specified in 'attr'. */
+ rc = pthread_create(&t0, attr, tm_una_ping, (void *) &flags);
if (rc)
pr_err(rc, "pthread_create()");
- rc = pthread_setname_np(t0, "ping");
+ rc = pthread_setname_np(t0, "tm_una_ping");
if (rc)
pr_warn(rc, "pthread_setname_np");
rc = pthread_join(t0, &ret_value);
@@ -333,13 +333,15 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
}
}
-int main(int argc, char **argv)
+int tm_unavailable_test(void)
{
int rc, exception; /* FP = 0, VEC = 1, VSX = 2 */
pthread_t t1;
pthread_attr_t attr;
cpu_set_t cpuset;
+ SKIP_IF(!have_htm());
+
/* Set only CPU 0 in the mask. Both threads will be bound to CPU 0. */
CPU_ZERO(&cpuset);
CPU_SET(0, &cpuset);
@@ -354,12 +356,12 @@ int main(int argc, char **argv)
if (rc)
pr_err(rc, "pthread_attr_setaffinity_np()");
- rc = pthread_create(&t1, &attr /* Bind 'pong' to CPU 0 */, pong, NULL);
+ rc = pthread_create(&t1, &attr /* Bind to CPU 0 */, tm_una_pong, NULL);
if (rc)
pr_err(rc, "pthread_create()");
/* Name it for systemtap convenience */
- rc = pthread_setname_np(t1, "pong");
+ rc = pthread_setname_np(t1, "tm_una_pong");
if (rc)
pr_warn(rc, "pthread_create()");
@@ -394,3 +396,9 @@ int main(int argc, char **argv)
exit(0);
}
}
+
+int main(int argc, char **argv)
+{
+ test_harness_set_timeout(220);
+ return test_harness(tm_unavailable_test, "tm_unavailable_test");
+}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Mauricio Faria de Oliveira @ 2018-03-05 22:46 UTC (permalink / raw)
To: Michal Suchánek, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20180220180621.11265945@kitsune.suse.cz>
Hi Michael, Michal,
I got back from vacation. Checking this one.
On 02/20/2018 02:06 PM, Michal Suchánek wrote:
>> I did it the way I did because otherwise we waste memory on every
>> system on earth just to support a use case that we don't actually
>> intend for anyone to ever use - ie. migrating from a patched machine
>> to an unpatched machine.
If this thread eventually closes in 'ok, so that memory has to be
reserved/wasted anyway', that can be done only in pseries, right?
It seems not so much memory for this particular platform/hardware.
> If you have multiple hosts running some LPMs and want to update them
> without shutting down the whole thing I suppose it might easily happen
> that a machine (re)started on a patched host is migrated to unpatched
> host.
Right, but that should be temporary, I think -- after updating some of
the hosts, the LPAR(s) can be migrated back to one of them, where the
fallback flush is not required anymore.
>> I think I'm inclined to leave it the way it is, unless you feel
>> strongly about it Michal?
> I think it would be more user friendly to either support the fallback
> method 100% or remove it and require patched firmware.
I beg to disagree. Since the matter is a security issue, the option
of still have some sort of fix that works on unpatched firmware does
look good and friendly to users (rather than require 'you _must_ get
the firmware update') IMHO.
cheers,
Mauricio
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Skip tm-unavailable if TM is not enabled
From: Cyril Bur @ 2018-03-05 23:49 UTC (permalink / raw)
To: Gustavo Romero, linuxppc-dev
In-Reply-To: <1520282935-20111-2-git-send-email-gromero@linux.vnet.ibm.com>
On Mon, 2018-03-05 at 15:48 -0500, Gustavo Romero wrote:
> Some processor revisions do not support transactional memory, and
> additionally kernel support can be disabled. In either case the
> tm-unavailable test should be skipped, otherwise it will fail with
> a SIGILL.
>
> That commit also sets this selftest to be called through the test
> harness as it's done for other TM selftests.
>
> Finally, it avoids using "ping" as a thread name since it's
> ambiguous and can be confusing when shown, for instance,
> in a kernel backtrace log.
>
I spent more time than I care to admit looking at backtraces wondering
how "ping" got in the mix ;).
> Fixes: 77fad8bfb1d2 ("selftests/powerpc: Check FP/VEC on exception in TM")
> Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
Reviewed-by: Cyril Bur <cyrilbur@gmail.com>
> ---
> .../testing/selftests/powerpc/tm/tm-unavailable.c | 24 ++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/tm/tm-unavailable.c b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
> index e6a0fad..156c8e7 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-unavailable.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
> @@ -80,7 +80,7 @@ bool is_failure(uint64_t condition_reg)
> return ((condition_reg >> 28) & 0xa) == 0xa;
> }
>
> -void *ping(void *input)
> +void *tm_una_ping(void *input)
> {
>
> /*
> @@ -280,7 +280,7 @@ void *ping(void *input)
> }
>
> /* Thread to force context switch */
> -void *pong(void *not_used)
> +void *tm_una_pong(void *not_used)
> {
> /* Wait thread get its name "pong". */
> if (DEBUG)
> @@ -311,11 +311,11 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
> do {
> int rc;
>
> - /* Bind 'ping' to CPU 0, as specified in 'attr'. */
> - rc = pthread_create(&t0, attr, ping, (void *) &flags);
> + /* Bind to CPU 0, as specified in 'attr'. */
> + rc = pthread_create(&t0, attr, tm_una_ping, (void *) &flags);
> if (rc)
> pr_err(rc, "pthread_create()");
> - rc = pthread_setname_np(t0, "ping");
> + rc = pthread_setname_np(t0, "tm_una_ping");
> if (rc)
> pr_warn(rc, "pthread_setname_np");
> rc = pthread_join(t0, &ret_value);
> @@ -333,13 +333,15 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
> }
> }
>
> -int main(int argc, char **argv)
> +int tm_unavailable_test(void)
> {
> int rc, exception; /* FP = 0, VEC = 1, VSX = 2 */
> pthread_t t1;
> pthread_attr_t attr;
> cpu_set_t cpuset;
>
> + SKIP_IF(!have_htm());
> +
> /* Set only CPU 0 in the mask. Both threads will be bound to CPU 0. */
> CPU_ZERO(&cpuset);
> CPU_SET(0, &cpuset);
> @@ -354,12 +356,12 @@ int main(int argc, char **argv)
> if (rc)
> pr_err(rc, "pthread_attr_setaffinity_np()");
>
> - rc = pthread_create(&t1, &attr /* Bind 'pong' to CPU 0 */, pong, NULL);
> + rc = pthread_create(&t1, &attr /* Bind to CPU 0 */, tm_una_pong, NULL);
> if (rc)
> pr_err(rc, "pthread_create()");
>
> /* Name it for systemtap convenience */
> - rc = pthread_setname_np(t1, "pong");
> + rc = pthread_setname_np(t1, "tm_una_pong");
> if (rc)
> pr_warn(rc, "pthread_create()");
>
> @@ -394,3 +396,9 @@ int main(int argc, char **argv)
> exit(0);
> }
> }
> +
> +int main(int argc, char **argv)
> +{
> + test_harness_set_timeout(220);
> + return test_harness(tm_unavailable_test, "tm_unavailable_test");
> +}
^ permalink raw reply
* [PATCH 0/9] EEH refactoring 1
From: Sam Bobroff @ 2018-03-05 23:58 UTC (permalink / raw)
To: linuxppc-dev
Hello everyone,
Here is a set of some small, mostly idempotent, changes to improve
maintainability in some of the EEH code, primarily in eeh_driver.c.
I've kept them all small to aid review but perhaps they should be squashed down
before being applied.
Cheers,
Sam.
Sam Bobroff (9):
powerpc/eeh: Remove eeh_handle_event()
powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
powerpc/eeh: Clarify arguments to eeh_reset_device()
powerpc/eeh: Remove always-true tests in eeh_reset_device()
powerpc/eeh: Factor out common code eeh_reset_device()
powerpc/eeh: Add eeh_state_active() helper
arch/powerpc/include/asm/eeh.h | 6 ++
arch/powerpc/include/asm/eeh_event.h | 3 +-
arch/powerpc/kernel/eeh.c | 19 ++--
arch/powerpc/kernel/eeh_cache.c | 3 +-
arch/powerpc/kernel/eeh_driver.c | 143 +++++++++++----------------
arch/powerpc/kernel/eeh_event.c | 6 +-
arch/powerpc/platforms/powernv/eeh-powernv.c | 9 +-
7 files changed, 75 insertions(+), 114 deletions(-)
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply
* [PATCH 1/9] powerpc/eeh: Remove eeh_handle_event()
From: Sam Bobroff @ 2018-03-05 23:58 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
The function eeh_handle_event(pe) does nothing other than switching
between calling eeh_handle_normal_event(pe) and
eeh_handle_special_event(). However it is only called in two places,
one where pe can't be NULL and the other where it must be NULL (see
eeh_event_handler()) so it does nothing but obscure the flow of
control.
So, remove it.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/include/asm/eeh_event.h | 3 ++-
arch/powerpc/kernel/eeh_driver.c | 42 +++++++++++++-----------------------
arch/powerpc/kernel/eeh_event.c | 4 ++--
3 files changed, 19 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
index 1e551a2d6f82..0a168038882d 100644
--- a/arch/powerpc/include/asm/eeh_event.h
+++ b/arch/powerpc/include/asm/eeh_event.h
@@ -34,7 +34,8 @@ struct eeh_event {
int eeh_event_init(void);
int eeh_send_failure_event(struct eeh_pe *pe);
void eeh_remove_event(struct eeh_pe *pe, bool force);
-void eeh_handle_event(struct eeh_pe *pe);
+bool eeh_handle_normal_event(struct eeh_pe *pe);
+void eeh_handle_special_event(void);
#endif /* __KERNEL__ */
#endif /* ASM_POWERPC_EEH_EVENT_H */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 0c0b66fc5bfb..51b21c97910f 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -738,9 +738,22 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* Attempts to recover the given PE. If recovery fails or the PE has failed
* too many times, remove the PE.
*
+ * While PHB detects address or data parity errors on particular PCI
+ * slot, the associated PE will be frozen. Besides, DMA's occurring
+ * to wild addresses (which usually happen due to bugs in device
+ * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
+ * #PERR or other misc PCI-related errors also can trigger EEH errors.
+ *
+ * Recovery process consists of unplugging the device driver (which
+ * generated hotplug events to userspace), then issuing a PCI #RST to
+ * the device, then reconfiguring the PCI config space for all bridges
+ * & devices under this slot, and then finally restarting the device
+ * drivers (which cause a second set of hotplug events to go out to
+ * userspace).
+ *
* Returns true if @pe should no longer be used, else false.
*/
-static bool eeh_handle_normal_event(struct eeh_pe *pe)
+bool eeh_handle_normal_event(struct eeh_pe *pe)
{
struct pci_bus *frozen_bus;
struct eeh_dev *edev, *tmp;
@@ -942,7 +955,7 @@ static bool eeh_handle_normal_event(struct eeh_pe *pe)
* specific PE. Iterates through possible failures and handles them as
* necessary.
*/
-static void eeh_handle_special_event(void)
+void eeh_handle_special_event(void)
{
struct eeh_pe *pe, *phb_pe;
struct pci_bus *bus;
@@ -1049,28 +1062,3 @@ static void eeh_handle_special_event(void)
break;
} while (rc != EEH_NEXT_ERR_NONE);
}
-
-/**
- * eeh_handle_event - Reset a PCI device after hard lockup.
- * @pe: EEH PE
- *
- * While PHB detects address or data parity errors on particular PCI
- * slot, the associated PE will be frozen. Besides, DMA's occurring
- * to wild addresses (which usually happen due to bugs in device
- * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
- * #PERR or other misc PCI-related errors also can trigger EEH errors.
- *
- * Recovery process consists of unplugging the device driver (which
- * generated hotplug events to userspace), then issuing a PCI #RST to
- * the device, then reconfiguring the PCI config space for all bridges
- * & devices under this slot, and then finally restarting the device
- * drivers (which cause a second set of hotplug events to go out to
- * userspace).
- */
-void eeh_handle_event(struct eeh_pe *pe)
-{
- if (pe)
- eeh_handle_normal_event(pe);
- else
- eeh_handle_special_event();
-}
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
index accbf8b5fd46..872bcfe8f90e 100644
--- a/arch/powerpc/kernel/eeh_event.c
+++ b/arch/powerpc/kernel/eeh_event.c
@@ -81,10 +81,10 @@ static int eeh_event_handler(void * dummy)
pr_info("EEH: Detected PCI bus error on "
"PHB#%x-PE#%x\n",
pe->phb->global_number, pe->addr);
- eeh_handle_event(pe);
+ eeh_handle_normal_event(pe);
eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
} else {
- eeh_handle_event(NULL);
+ eeh_handle_special_event();
}
kfree(event);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 2/9] powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-05 23:58 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
Currently the EEH_PE_RECOVERING flag for a PE is managed by both the
caller and callee of eeh_handle_normal_event() (among other places not
considered here). This is complicated by the fact that the PE may
or may not have been invalidated by the call.
So move the callee's handling into eeh_handle_normal_event(), which
clarifies it and allows the return type to be changed to void (because
it no longer needs to indicate at the PE has been invalidated).
This should not change behaviour except in eeh_event_handler() where
it was previously possible to cause eeh_pe_state_clear() to be called
on an invalid PE, which is now avoided.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/include/asm/eeh_event.h | 2 +-
arch/powerpc/kernel/eeh_driver.c | 29 +++++++++++------------------
arch/powerpc/kernel/eeh_event.c | 2 --
3 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
index 0a168038882d..9884e872686f 100644
--- a/arch/powerpc/include/asm/eeh_event.h
+++ b/arch/powerpc/include/asm/eeh_event.h
@@ -34,7 +34,7 @@ struct eeh_event {
int eeh_event_init(void);
int eeh_send_failure_event(struct eeh_pe *pe);
void eeh_remove_event(struct eeh_pe *pe, bool force);
-bool eeh_handle_normal_event(struct eeh_pe *pe);
+void eeh_handle_normal_event(struct eeh_pe *pe);
void eeh_handle_special_event(void);
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 51b21c97910f..5b7a5ed4db4d 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -733,7 +733,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
/**
* eeh_handle_normal_event - Handle EEH events on a specific PE
- * @pe: EEH PE
+ * @pe: EEH PE - which should not be used after we return, as it may
+ * have been invalidated.
*
* Attempts to recover the given PE. If recovery fails or the PE has failed
* too many times, remove the PE.
@@ -750,10 +751,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* & devices under this slot, and then finally restarting the device
* drivers (which cause a second set of hotplug events to go out to
* userspace).
- *
- * Returns true if @pe should no longer be used, else false.
*/
-bool eeh_handle_normal_event(struct eeh_pe *pe)
+void eeh_handle_normal_event(struct eeh_pe *pe)
{
struct pci_bus *frozen_bus;
struct eeh_dev *edev, *tmp;
@@ -765,9 +764,11 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
if (!frozen_bus) {
pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
__func__, pe->phb->global_number, pe->addr);
- return false;
+ return;
}
+ eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
+
eeh_pe_update_time_stamp(pe);
pe->freeze_count++;
if (pe->freeze_count > eeh_max_freezes) {
@@ -904,7 +905,7 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Notify device driver to resume\n");
eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
- return false;
+ goto final;
hard_fail:
/*
@@ -940,12 +941,12 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
pci_lock_rescan_remove();
pci_hp_remove_devices(frozen_bus);
pci_unlock_rescan_remove();
-
/* The passed PE should no longer be used */
- return true;
+ return;
}
}
- return false;
+final:
+ eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
}
/**
@@ -1018,15 +1019,7 @@ void eeh_handle_special_event(void)
*/
if (rc == EEH_NEXT_ERR_FROZEN_PE ||
rc == EEH_NEXT_ERR_FENCED_PHB) {
- /*
- * eeh_handle_normal_event() can make the PE stale if it
- * determines that the PE cannot possibly be recovered.
- * Don't modify the PE state if that's the case.
- */
- if (eeh_handle_normal_event(pe))
- continue;
-
- eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
+ eeh_handle_normal_event(pe);
} else {
pci_lock_rescan_remove();
list_for_each_entry(hose, &hose_list, list_node) {
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
index 872bcfe8f90e..61c9356bf9c9 100644
--- a/arch/powerpc/kernel/eeh_event.c
+++ b/arch/powerpc/kernel/eeh_event.c
@@ -73,7 +73,6 @@ static int eeh_event_handler(void * dummy)
/* We might have event without binding PE */
pe = event->pe;
if (pe) {
- eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
if (pe->type & EEH_PE_PHB)
pr_info("EEH: Detected error on PHB#%x\n",
pe->phb->global_number);
@@ -82,7 +81,6 @@ static int eeh_event_handler(void * dummy)
"PHB#%x-PE#%x\n",
pe->phb->global_number, pe->addr);
eeh_handle_normal_event(pe);
- eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
} else {
eeh_handle_special_event();
}
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 3/9] powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
Commit "0ba178888b05 powerpc/eeh: Remove reference to PCI device"
removed a call to pci_dev_get() from __eeh_addr_cache_get_device() but
did not update the comment to match.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_cache.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index d4cc26618809..201943d54a6e 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -84,8 +84,7 @@ static inline struct eeh_dev *__eeh_addr_cache_get_device(unsigned long addr)
* @addr: mmio (PIO) phys address or i/o port number
*
* Given an mmio phys address, or a port number, find a pci device
- * that implements this address. Be sure to pci_dev_put the device
- * when finished. I/O port numbers are assumed to be offset
+ * that implements this address. I/O port numbers are assumed to be offset
* from zero (that is, they do *not* have pci_io_addr added in).
* It is safe to call this function within an interrupt.
*/
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 4/9] powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
Remove a test that checks if "frozen_bus" is NULL, because it cannot
have changed since it was tested at the start of the function and so
must be true here.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 5b7a5ed4db4d..04a5d9db5499 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -930,20 +930,18 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* all removed devices correctly to avoid access
* the their PCI config any more.
*/
- if (frozen_bus) {
- if (pe->type & EEH_PE_VF) {
- eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
- } else {
- eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ if (pe->type & EEH_PE_VF) {
+ eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ } else {
+ eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
- pci_lock_rescan_remove();
- pci_hp_remove_devices(frozen_bus);
- pci_unlock_rescan_remove();
- /* The passed PE should no longer be used */
- return;
- }
+ pci_lock_rescan_remove();
+ pci_hp_remove_devices(frozen_bus);
+ pci_unlock_rescan_remove();
+ /* The passed PE should no longer be used */
+ return;
}
final:
eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 5/9] powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
The name "frozen_bus" is misleading: it's not necessarily frozen, it's
just the PE's PCI bus.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 04a5d9db5499..cb584d72b0a5 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -754,14 +754,14 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
*/
void eeh_handle_normal_event(struct eeh_pe *pe)
{
- struct pci_bus *frozen_bus;
+ struct pci_bus *bus;
struct eeh_dev *edev, *tmp;
int rc = 0;
enum pci_ers_result result = PCI_ERS_RESULT_NONE;
struct eeh_rmv_data rmv_data = {LIST_HEAD_INIT(rmv_data.edev_list), 0};
- frozen_bus = eeh_pe_bus_get(pe);
- if (!frozen_bus) {
+ bus = eeh_pe_bus_get(pe);
+ if (!bus) {
pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
__func__, pe->phb->global_number, pe->addr);
return;
@@ -820,7 +820,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
*/
if (result == PCI_ERS_RESULT_NONE) {
pr_info("EEH: Reset with hotplug activity\n");
- rc = eeh_reset_device(pe, frozen_bus, NULL);
+ rc = eeh_reset_device(pe, bus, NULL);
if (rc) {
pr_warn("%s: Unable to reset, err=%d\n",
__func__, rc);
@@ -938,7 +938,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
pci_lock_rescan_remove();
- pci_hp_remove_devices(frozen_bus);
+ pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
/* The passed PE should no longer be used */
return;
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 6/9] powerpc/eeh: Clarify arguments to eeh_reset_device()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
It is currently difficult to understand the behaviour of
eeh_reset_device() due to the way it's parameters are used. In
particular, when 'bus' is NULL, it's value is still necessary so the
same value is looked up again locally under a different name
('frozen_bus') but behaviour is changed.
To clarify this, add a new parameter 'eeh_aware_driver', and have the
caller set it when it would have passed NULL for 'bus' and always pass
a value for 'bus'. Then change any test that was on 'bus' to one on
'!eeh_aware_driver' and replace uses of 'frozen_bus' with 'bus'.
Also update the function's comment.
This should not change behaviour.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index cb584d72b0a5..6c3577133223 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -619,17 +619,19 @@ int eeh_pe_reset_and_recover(struct eeh_pe *pe)
/**
* eeh_reset_device - Perform actual reset of a pci slot
+ * @eeh_aware_driver: Does the device's driver provide EEH support?
* @pe: EEH PE
* @bus: PCI bus corresponding to the isolcated slot
+ * @rmv_data: Optional, list to record removed devices
*
* This routine must be called to do reset on the indicated PE.
* During the reset, udev might be invoked because those affected
* PCI devices will be removed and then added.
*/
-static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
- struct eeh_rmv_data *rmv_data)
+static int eeh_reset_device(bool eeh_aware_driver,
+ struct eeh_pe *pe, struct pci_bus *bus,
+ struct eeh_rmv_data *rmv_data)
{
- struct pci_bus *frozen_bus = eeh_pe_bus_get(pe);
time64_t tstamp;
int cnt, rc;
struct eeh_dev *edev;
@@ -645,7 +647,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* into pci_hp_add_devices().
*/
eeh_pe_state_mark(pe, EEH_PE_KEEP);
- if (bus) {
+ if (!eeh_aware_driver) {
if (pe->type & EEH_PE_VF) {
eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
} else {
@@ -653,7 +655,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
}
- } else if (frozen_bus) {
+ } else if (bus) {
eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
}
@@ -689,7 +691,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* the device up before the scripts have taken it down,
* potentially weird things happen.
*/
- if (bus) {
+ if (!eeh_aware_driver) {
pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
ssleep(5);
@@ -706,7 +708,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
pci_hp_add_devices(bus);
}
- } else if (frozen_bus && rmv_data->removed) {
+ } else if (bus && rmv_data->removed) {
pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
ssleep(5);
@@ -715,7 +717,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
if (pe->type & EEH_PE_VF)
eeh_add_virt_device(edev, NULL);
else
- pci_hp_add_devices(frozen_bus);
+ pci_hp_add_devices(bus);
}
eeh_pe_state_clear(pe, EEH_PE_KEEP);
@@ -820,7 +822,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
*/
if (result == PCI_ERS_RESULT_NONE) {
pr_info("EEH: Reset with hotplug activity\n");
- rc = eeh_reset_device(pe, bus, NULL);
+ rc = eeh_reset_device(false, pe, bus, NULL);
if (rc) {
pr_warn("%s: Unable to reset, err=%d\n",
__func__, rc);
@@ -872,7 +874,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
/* If any device called out for a reset, then reset the slot */
if (result == PCI_ERS_RESULT_NEED_RESET) {
pr_info("EEH: Reset without hotplug activity\n");
- rc = eeh_reset_device(pe, NULL, &rmv_data);
+ rc = eeh_reset_device(true, pe, bus, &rmv_data);
if (rc) {
pr_warn("%s: Cannot reset, err=%d\n",
__func__, rc);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 7/9] powerpc/eeh: Remove always-true tests in eeh_reset_device()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
eeh_reset_device() tests the value of 'bus' more than once but the
only caller, eeh_handle_normal_device() does this test itself and will
never pass NULL.
So, remove the dead tests.
This should not change behaviour.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 6c3577133223..1272f2c8cbd2 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -655,7 +655,7 @@ static int eeh_reset_device(bool eeh_aware_driver,
pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
}
- } else if (bus) {
+ } else {
eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
}
@@ -708,7 +708,7 @@ static int eeh_reset_device(bool eeh_aware_driver,
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
pci_hp_add_devices(bus);
}
- } else if (bus && rmv_data->removed) {
+ } else if (rmv_data->removed) {
pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
ssleep(5);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox