[PATCH v2 0/2] spi: fix incorrect property items
This series fixes incorrect property because it was different
from the actual.
The commit 71b8dfc691a3 ("spi: uniphier: re-add addressing properties")
is the updated patch, therefore, it must be applied after
the commit 7662d1dc17d4 ("spi: uniphier: fix incorrect property items").
Changes since v1:
- Add commit 71b8dfc691a3 ("spi: uniphier: re-add addressing properties")
The addressing properties of #address-cells and #size-cells
are back again.
Keiji Hayashibara (2):
spi: uniphier: fix incorrect property items
spi: uniphier: re-add addressing properties
Documentation/devicetree/bindings/spi/spi-uniphier.txt | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
--
2.7.4
[PATCH v2 0/2] spi: fix incorrect property items
This series fixes incorrect property because it was different
from the actual.
The commit 71b8dfc691a3 ("spi: uniphier: re-add addressing properties")
is the updated patch, therefore, it must be applied after
the commit 7662d1dc17d4 ("spi: uniphier: fix incorrect property items").
Changes since v1:
- Add commit 71b8dfc691a3 ("spi: uniphier: re-add addressing properties")
The addressing properties of #address-cells and #size-cells
are back again.
Keiji Hayashibara (2):
spi: uniphier: fix incorrect property items
spi: uniphier: re-add addressing properties
Documentation/devicetree/bindings/spi/spi-uniphier.txt | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
--
2.7.4
[PATCH v2 1/2] spi: uniphier: fix incorrect property items
This commit fixes incorrect property because it was different
from the actual.
The parameters of '#address-cells' and '#size-cells' were removed,
and 'interrupts', 'pinctrl-names' and 'pinctrl-0' were added.
Fixes: 4dcd5c2781f3 ("spi: add DT bindings for UniPhier SPI controller")
Signed-off-by: Keiji Hayashibara
---
Documentation/devicetree/bindings/spi/spi-uniphier.txt | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
index 504a4ec..b04e66a 100644
--- a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
+++ b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
@@ -5,18 +5,20 @@ UniPhier SoCs have SCSSI which supports SPI single channel.
Required properties:
- compatible: should be "socionext,uniphier-scssi"
- reg: address and length of the spi master registers
- - #address-cells: must be <1>, see spi-bus.txt
- - #size-cells: must be <0>, see spi-bus.txt
- - clocks: A phandle to the clock for the device.
- - resets: A phandle to the reset control for the device.
+ - interrupts: a single interrupt specifier
+ - pinctrl-names: should be "default"
+ - pinctrl-0: pin control state for the default mode
+ - clocks: a phandle to the clock for the device
+ - resets: a phandle to the reset control for the device
Example:
spi0: spi@54006000 {
compatible = "socionext,uniphier-scssi";
reg = <0x54006000 0x100>;
- #address-cells = <1>;
- #size-cells = <0>;
+ interrupts = <0 39 4>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_spi0>;
clocks = <_clk 11>;
resets = <_rst 11>;
};
--
2.7.4
[PATCH v2 2/2] spi: uniphier: re-add addressing properties
In commit 7662d1dc17d4 ("spi: uniphier: fix incorrect property items")
addressing properties of #address-cells and #size-cells were removed.
Since it is not necessary to remove them, they are back again.
Signed-off-by: Keiji Hayashibara
---
Documentation/devicetree/bindings/spi/spi-uniphier.txt | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
index b04e66a..e120157 100644
--- a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
+++ b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
@@ -5,6 +5,8 @@ UniPhier SoCs have SCSSI which supports SPI single channel.
Required properties:
- compatible: should be "socionext,uniphier-scssi"
- reg: address and length of the spi master registers
+ - #address-cells: must be <1>, see spi-bus.txt
+ - #size-cells: must be <0>, see spi-bus.txt
- interrupts: a single interrupt specifier
- pinctrl-names: should be "default"
- pinctrl-0: pin control state for the default mode
@@ -16,6 +18,8 @@ Example:
spi0: spi@54006000 {
compatible = "socionext,uniphier-scssi";
reg = <0x54006000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
interrupts = <0 39 4>;
pinctrl-names = "default";
pinctrl-0 = <_spi0>;
--
2.7.4
[PATCH v2 2/2] spi: uniphier: re-add addressing properties
In commit 7662d1dc17d4 ("spi: uniphier: fix incorrect property items")
addressing properties of #address-cells and #size-cells were removed.
Since it is not necessary to remove them, they are back again.
Signed-off-by: Keiji Hayashibara
---
Documentation/devicetree/bindings/spi/spi-uniphier.txt | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
index b04e66a..e120157 100644
--- a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
+++ b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
@@ -5,6 +5,8 @@ UniPhier SoCs have SCSSI which supports SPI single channel.
Required properties:
- compatible: should be "socionext,uniphier-scssi"
- reg: address and length of the spi master registers
+ - #address-cells: must be <1>, see spi-bus.txt
+ - #size-cells: must be <0>, see spi-bus.txt
- interrupts: a single interrupt specifier
- pinctrl-names: should be "default"
- pinctrl-0: pin control state for the default mode
@@ -16,6 +18,8 @@ Example:
spi0: spi@54006000 {
compatible = "socionext,uniphier-scssi";
reg = <0x54006000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
interrupts = <0 39 4>;
pinctrl-names = "default";
pinctrl-0 = <_spi0>;
--
2.7.4
[PATCH v2 1/2] spi: uniphier: fix incorrect property items
This commit fixes incorrect property because it was different
from the actual.
The parameters of '#address-cells' and '#size-cells' were removed,
and 'interrupts', 'pinctrl-names' and 'pinctrl-0' were added.
Fixes: 4dcd5c2781f3 ("spi: add DT bindings for UniPhier SPI controller")
Signed-off-by: Keiji Hayashibara
---
Documentation/devicetree/bindings/spi/spi-uniphier.txt | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
index 504a4ec..b04e66a 100644
--- a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
+++ b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
@@ -5,18 +5,20 @@ UniPhier SoCs have SCSSI which supports SPI single channel.
Required properties:
- compatible: should be "socionext,uniphier-scssi"
- reg: address and length of the spi master registers
- - #address-cells: must be <1>, see spi-bus.txt
- - #size-cells: must be <0>, see spi-bus.txt
- - clocks: A phandle to the clock for the device.
- - resets: A phandle to the reset control for the device.
+ - interrupts: a single interrupt specifier
+ - pinctrl-names: should be "default"
+ - pinctrl-0: pin control state for the default mode
+ - clocks: a phandle to the clock for the device
+ - resets: a phandle to the reset control for the device
Example:
spi0: spi@54006000 {
compatible = "socionext,uniphier-scssi";
reg = <0x54006000 0x100>;
- #address-cells = <1>;
- #size-cells = <0>;
+ interrupts = <0 39 4>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_spi0>;
clocks = <_clk 11>;
resets = <_rst 11>;
};
--
2.7.4
Re: The linux devs can rescind their license grant.
On Thu, Oct 25, 2018 at 10:28:36PM -0400, Eric S. Raymond wrote: > Al Viro : > > * in case it needs to be spelled out: I am not at all interested > > in that kind of stunts. One of the reasons I thoroughly despise RMS > > and his bunch is the leverage game they tried to play with GPLv3; > > damned if I'm going to lower myself to their level. > > Sorry, I did not mean to imply that you are one with the license-revokers. > More like "if even Al Viro thinks you fucked up your process, it would be > unwise to dismiss the opposition to the CoC as mere trolls". Kindly stop conflating opposition to CoC with that kind of idiocy.
Re: The linux devs can rescind their license grant.
On Thu, Oct 25, 2018 at 10:28:36PM -0400, Eric S. Raymond wrote: > Al Viro : > > * in case it needs to be spelled out: I am not at all interested > > in that kind of stunts. One of the reasons I thoroughly despise RMS > > and his bunch is the leverage game they tried to play with GPLv3; > > damned if I'm going to lower myself to their level. > > Sorry, I did not mean to imply that you are one with the license-revokers. > More like "if even Al Viro thinks you fucked up your process, it would be > unwise to dismiss the opposition to the CoC as mere trolls". Kindly stop conflating opposition to CoC with that kind of idiocy.
Attention: Beneficiary,
Attention: Beneficiary, Due to the Federal Government new Policy to fight corruption. I want to bring to your Notice that the President of the Federal Republic of Nigeria, Muhammadu Buhari, has Mandated that all the Overdue Foreign Payment should be Paid to the Beneficiary not Later than 30th of October 2018. We are very sorry for all the stress/Fees you spent to the Fraudulent Officials who claim to be a Good Nigeria, Nigerian Image has been tarnished in the eye of the International Authorities, so Mr President Has Promised the International Authorities in the G8 Summit Meeting held in South Africa recently, that he will make sure that all the Payment owed by Nigerian Government will be paid to the Foreign Beneficiary without any Further Delay. I want to Inform you that A Certified Draft of ( US$2M } or ATM Card will be Made available to you, We therefore Request you to Reconfirm the Information as stated Bellow: (1) YOUR FULL NAME / ADDRESS (2) YOUR DIRECT TEL/FAX NUMBER (3) YOUR NATIONALITY / IDENTITY CARD (4) YOUR AGE (5) YOUR OCCUPATION. (6) MODE OF PAYMENT ( ATM CARD OR CERTIFIED DRAFT) Choose one Mode of Payment. You should reply Immediately with the Information if you really want to receive this Certified Draft or ATM Card before the Closing Date, Reply Directly to the Bellow Email Address: ( henrynwa...@aol.com ) and Let Me Know Your Stands as well as sending the Information. Thanks For Your Understanding, Dr.Henry Nwosu.
Attention: Beneficiary,
Attention: Beneficiary, Due to the Federal Government new Policy to fight corruption. I want to bring to your Notice that the President of the Federal Republic of Nigeria, Muhammadu Buhari, has Mandated that all the Overdue Foreign Payment should be Paid to the Beneficiary not Later than 30th of October 2018. We are very sorry for all the stress/Fees you spent to the Fraudulent Officials who claim to be a Good Nigeria, Nigerian Image has been tarnished in the eye of the International Authorities, so Mr President Has Promised the International Authorities in the G8 Summit Meeting held in South Africa recently, that he will make sure that all the Payment owed by Nigerian Government will be paid to the Foreign Beneficiary without any Further Delay. I want to Inform you that A Certified Draft of ( US$2M } or ATM Card will be Made available to you, We therefore Request you to Reconfirm the Information as stated Bellow: (1) YOUR FULL NAME / ADDRESS (2) YOUR DIRECT TEL/FAX NUMBER (3) YOUR NATIONALITY / IDENTITY CARD (4) YOUR AGE (5) YOUR OCCUPATION. (6) MODE OF PAYMENT ( ATM CARD OR CERTIFIED DRAFT) Choose one Mode of Payment. You should reply Immediately with the Information if you really want to receive this Certified Draft or ATM Card before the Closing Date, Reply Directly to the Bellow Email Address: ( henrynwa...@aol.com ) and Let Me Know Your Stands as well as sending the Information. Thanks For Your Understanding, Dr.Henry Nwosu.
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
Re: [v5] i2c: Add PCI and platform drivers for the AMD MP2 I2C controller
> On Oct 24, 2018, at 11:59 PM, Elie Morisse wrote: > > This contains two drivers: > * i2c-amd-plat-mp2: platform driver managing an i2c adapter (one of > the two busses of the MP2) and routing any i2c read/write command to > the PCI driver. > * i2c-amd-pci-mp2: PCI driver communicating through the C2P/P2C > mailbox registers, or through DMA for more than 32 bytes transfers. > > This is major rework of the patch submitted by Nehal-bakulchandra Shah > from AMD (https://patchwork.kernel.org/patch/10597369/). > > Most of the event handling of v2/v3 was rewritten since it couldn't work > if more than one bus was enabled, and contains many more fixes listed > in the patch changelog. > > With those changes both the touchpad and the touchscreen of the > Ryzen-based Lenovo Yoga 530 which lie in separate busses work beautifully. > > Signed-off-by: Elie Morisse Re-test v5 on Dell Latitude 5495, works good. Tested-by: Kai-Heng Feng > --- > Changes since v1:(https://www.spinics.net/lists/linux-i2c/msg34650.html) > -> Add fix for IOMMU > -> Add depedency of ACPI > -> Add locks to avoid the crash > > Changes since v2:(https://patchwork.ozlabs.org/patch/961270/) > > -> fix for review comments > -> fix for more than 32 bytes write issue > > Changes since v3 (https://patchwork.kernel.org/patch/10597369/) by Elie M.: > > -> support more than one bus/adapter > -> support more than one slave per bus > -> use the bus speed specified by the slaves declared in the DSDT instead of > assuming speed == 400kbits/s > -> instead of kzalloc'ing a buffer for every less than 32 bytes reads, simply > use i2c_msg.buf > -> fix buffer overreads/overflows when (<=32 bytes) message lengths aren't a > multiple of 4 by using memcpy_fromio and memcpy_toio > -> use streaming DMA mappings instead of allocating a coherent DMA buffer for > every >32 bytes read/write > -> properly check for timeouts during i2c_amd_xfer and increase it from 50 > jiffies to 250 msecs (which is more in line with other drivers) > -> complete amd_i2c_dev.msg even if the device doesn't return a xxx_success > event, instead of stalling i2c_amd_xfer > -> removed the spinlock and mdelay during i2c_amd_pci_configure, I didn't see > the point since it's already waiting for a i2c_busenable_complete event > -> add an adapter-specific mutex lock for i2c_amd_xfer, since we don't want > parallel calls writing to AMD_C2P_MSG0 (or AMD_C2P_MSG1) > -> add a global mutex lock for registers AMD_C2P_MSG2 to AMD_C2P_MSG9, which > are shared across the two busses/adapters > -> add MODULE_DEVICE_TABLE to automatically load i2c-amd-platdrv if the DSDT > enumerates devices with the "AMDI0011" HID > -> set maximum length of reads/writes to 4095 (event's length field is 12 > bits) > -> basic PM support > -> style corrections to match the kernel code style, and tried to reduce code > duplication whenever possible > > Changes since v4 (https://marc.info/?l=linux-kernel=154031133019835) by > Elie M.: > > -> fix missing typecast warning > -> removed the duplicated entry in Kconfig > > MAINTAINERS | 6 + > drivers/i2c/busses/Kconfig| 15 + > drivers/i2c/busses/Makefile | 2 + > drivers/i2c/busses/i2c-amd-pci-mp2.c | 633 ++ > drivers/i2c/busses/i2c-amd-pci-mp2.h | 219 + > drivers/i2c/busses/i2c-amd-plat-mp2.c | 362 +++ > 6 files changed, 1237 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c > create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h > create mode 100644 drivers/i2c/busses/i2c-amd-plat-mp2.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6ac000cc006d..8ff2bddc1ac2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -791,6 +791,12 @@ F: drivers/gpu/drm/amd/include/vi_structs.h > F:drivers/gpu/drm/amd/include/v9_structs.h > F:include/uapi/linux/kfd_ioctl.h > > +AMD MP2 I2C DRIVER > +M: Elie Morisse > +L: linux-...@vger.kernel.org > +S: Maintained > +F: drivers/i2c/busses/i2c-amd-*-mp2* > + > AMD POWERPLAY > M:Rex Zhu > M:Evan Quan > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 451d4ae50e66..e20f2d1ce381 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -77,6 +77,21 @@ config I2C_AMD8111 > This driver can also be built as a module. If so, the module > will be called i2c-amd8111. > > +config I2C_AMD_MP2_PCI > + tristate > + depends on PCI > + > +config I2C_AMD_MP2_PLATFORM > + tristate "AMD MP2 Platform" > + select I2C_AMD_MP2_PCI > + depends on ACPI > + help > + If you say yes to this option, support will be included for the AMD > MP2 > + PCI I2C adapter. > + > + This driver can also be built as a module. If so, the module > + will be called i2c-amd-plat-mp2. > + > config I2C_HIX5HD2 > tristate "Hix5hd2 high-speed I2C driver" > depends on ARCH_HISI
Re: [v5] i2c: Add PCI and platform drivers for the AMD MP2 I2C controller
> On Oct 24, 2018, at 11:59 PM, Elie Morisse wrote: > > This contains two drivers: > * i2c-amd-plat-mp2: platform driver managing an i2c adapter (one of > the two busses of the MP2) and routing any i2c read/write command to > the PCI driver. > * i2c-amd-pci-mp2: PCI driver communicating through the C2P/P2C > mailbox registers, or through DMA for more than 32 bytes transfers. > > This is major rework of the patch submitted by Nehal-bakulchandra Shah > from AMD (https://patchwork.kernel.org/patch/10597369/). > > Most of the event handling of v2/v3 was rewritten since it couldn't work > if more than one bus was enabled, and contains many more fixes listed > in the patch changelog. > > With those changes both the touchpad and the touchscreen of the > Ryzen-based Lenovo Yoga 530 which lie in separate busses work beautifully. > > Signed-off-by: Elie Morisse Re-test v5 on Dell Latitude 5495, works good. Tested-by: Kai-Heng Feng > --- > Changes since v1:(https://www.spinics.net/lists/linux-i2c/msg34650.html) > -> Add fix for IOMMU > -> Add depedency of ACPI > -> Add locks to avoid the crash > > Changes since v2:(https://patchwork.ozlabs.org/patch/961270/) > > -> fix for review comments > -> fix for more than 32 bytes write issue > > Changes since v3 (https://patchwork.kernel.org/patch/10597369/) by Elie M.: > > -> support more than one bus/adapter > -> support more than one slave per bus > -> use the bus speed specified by the slaves declared in the DSDT instead of > assuming speed == 400kbits/s > -> instead of kzalloc'ing a buffer for every less than 32 bytes reads, simply > use i2c_msg.buf > -> fix buffer overreads/overflows when (<=32 bytes) message lengths aren't a > multiple of 4 by using memcpy_fromio and memcpy_toio > -> use streaming DMA mappings instead of allocating a coherent DMA buffer for > every >32 bytes read/write > -> properly check for timeouts during i2c_amd_xfer and increase it from 50 > jiffies to 250 msecs (which is more in line with other drivers) > -> complete amd_i2c_dev.msg even if the device doesn't return a xxx_success > event, instead of stalling i2c_amd_xfer > -> removed the spinlock and mdelay during i2c_amd_pci_configure, I didn't see > the point since it's already waiting for a i2c_busenable_complete event > -> add an adapter-specific mutex lock for i2c_amd_xfer, since we don't want > parallel calls writing to AMD_C2P_MSG0 (or AMD_C2P_MSG1) > -> add a global mutex lock for registers AMD_C2P_MSG2 to AMD_C2P_MSG9, which > are shared across the two busses/adapters > -> add MODULE_DEVICE_TABLE to automatically load i2c-amd-platdrv if the DSDT > enumerates devices with the "AMDI0011" HID > -> set maximum length of reads/writes to 4095 (event's length field is 12 > bits) > -> basic PM support > -> style corrections to match the kernel code style, and tried to reduce code > duplication whenever possible > > Changes since v4 (https://marc.info/?l=linux-kernel=154031133019835) by > Elie M.: > > -> fix missing typecast warning > -> removed the duplicated entry in Kconfig > > MAINTAINERS | 6 + > drivers/i2c/busses/Kconfig| 15 + > drivers/i2c/busses/Makefile | 2 + > drivers/i2c/busses/i2c-amd-pci-mp2.c | 633 ++ > drivers/i2c/busses/i2c-amd-pci-mp2.h | 219 + > drivers/i2c/busses/i2c-amd-plat-mp2.c | 362 +++ > 6 files changed, 1237 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c > create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h > create mode 100644 drivers/i2c/busses/i2c-amd-plat-mp2.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6ac000cc006d..8ff2bddc1ac2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -791,6 +791,12 @@ F: drivers/gpu/drm/amd/include/vi_structs.h > F:drivers/gpu/drm/amd/include/v9_structs.h > F:include/uapi/linux/kfd_ioctl.h > > +AMD MP2 I2C DRIVER > +M: Elie Morisse > +L: linux-...@vger.kernel.org > +S: Maintained > +F: drivers/i2c/busses/i2c-amd-*-mp2* > + > AMD POWERPLAY > M:Rex Zhu > M:Evan Quan > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 451d4ae50e66..e20f2d1ce381 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -77,6 +77,21 @@ config I2C_AMD8111 > This driver can also be built as a module. If so, the module > will be called i2c-amd8111. > > +config I2C_AMD_MP2_PCI > + tristate > + depends on PCI > + > +config I2C_AMD_MP2_PLATFORM > + tristate "AMD MP2 Platform" > + select I2C_AMD_MP2_PCI > + depends on ACPI > + help > + If you say yes to this option, support will be included for the AMD > MP2 > + PCI I2C adapter. > + > + This driver can also be built as a module. If so, the module > + will be called i2c-amd-plat-mp2. > + > config I2C_HIX5HD2 > tristate "Hix5hd2 high-speed I2C driver" > depends on ARCH_HISI
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
Re: [PATCH] dt-bindings: arm: Fix cpu capacity mismatch in example
On 25-10-18, 14:04, Daniel Lezcano wrote:
> I think it is actually correct. The example is confusing on what the
> numbers are. IIUC, it is:
>
> (after normalizing)
>
> dhrystone result on big CPU is 1024 at 1100MHz
> dhrystone result on little CPU is 446 at 850MHz
>
> We have to scale the result of the little for 1100MHz in order to compare.
>
> 1100/850 = 1.294
>
> dhrystone result on little CPU is 446 * 1.294 = 577 at 1100MHz
>
> So we put the normalized values 1024 and 577. The arch_topology will
> scale 577 back to 446 as it will compute the max capacity based on the
> max freq which is 850MHz.
>
> The *final* capacities are 1024 for cluster0 and 446 for cluster1 (read
> the cpu max capacity, the ones showed in the sysfs).
>
> Did I miss something ?
No. What about making it more clear in the example to save the next idiot (like
me) from wasting time :)
diff --git a/Documentation/devicetree/bindings/arm/cpu-capacity.txt
b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
index 9b5685a1d15d..d061e6575bde 100644
--- a/Documentation/devicetree/bindings/arm/cpu-capacity.txt
+++ b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
@@ -61,7 +61,10 @@ mhz values (normalized w.r.t. the highest value found while
parsing the DT).
Example 1 (ARM 64-bit, 6-cpu system, two clusters):
capacities-dmips-mhz are scaled w.r.t. 1024 (cpu@0 and cpu@1)
supposing cluster0@max-freq=1100 and custer1@max-freq=850,
-final capacities are 1024 for cluster0 and 446 for cluster1
+final capacities are 1024 for cluster0 and 446 for cluster1.
+Note that the values mentioned below in the example (1024 and 578)
+aren't normalized based on max frequency of each cluster and that is
+left for the operating system to do.
cpus {
#address-cells = <2>;
--
viresh
Re: [PATCH] dt-bindings: arm: Fix cpu capacity mismatch in example
On 25-10-18, 14:04, Daniel Lezcano wrote:
> I think it is actually correct. The example is confusing on what the
> numbers are. IIUC, it is:
>
> (after normalizing)
>
> dhrystone result on big CPU is 1024 at 1100MHz
> dhrystone result on little CPU is 446 at 850MHz
>
> We have to scale the result of the little for 1100MHz in order to compare.
>
> 1100/850 = 1.294
>
> dhrystone result on little CPU is 446 * 1.294 = 577 at 1100MHz
>
> So we put the normalized values 1024 and 577. The arch_topology will
> scale 577 back to 446 as it will compute the max capacity based on the
> max freq which is 850MHz.
>
> The *final* capacities are 1024 for cluster0 and 446 for cluster1 (read
> the cpu max capacity, the ones showed in the sysfs).
>
> Did I miss something ?
No. What about making it more clear in the example to save the next idiot (like
me) from wasting time :)
diff --git a/Documentation/devicetree/bindings/arm/cpu-capacity.txt
b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
index 9b5685a1d15d..d061e6575bde 100644
--- a/Documentation/devicetree/bindings/arm/cpu-capacity.txt
+++ b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
@@ -61,7 +61,10 @@ mhz values (normalized w.r.t. the highest value found while
parsing the DT).
Example 1 (ARM 64-bit, 6-cpu system, two clusters):
capacities-dmips-mhz are scaled w.r.t. 1024 (cpu@0 and cpu@1)
supposing cluster0@max-freq=1100 and custer1@max-freq=850,
-final capacities are 1024 for cluster0 and 446 for cluster1
+final capacities are 1024 for cluster0 and 446 for cluster1.
+Note that the values mentioned below in the example (1024 and 578)
+aren't normalized based on max frequency of each cluster and that is
+left for the operating system to do.
cpus {
#address-cells = <2>;
--
viresh
[PATCH] pstore: Remove duplicate invoking of persistent_ram_zap()
From: Peng Wang
When initialing przs with invalid data in buffer(no PERSISTENT_RAM_SIG),
function call path is like this:
ramoops_init_prz ->
|
|-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
|
|--> persistent_ram_zap
As we can see, persistent_ram_zap() is called twice.
We can avoid this by removing it in ramoops_init_prz(),
and only call it in persistent_ram_post_init().
Signed-off-by: Peng Wang
---
fs/pstore/ram.c | 2 --
fs/pstore/ram_core.c | 3 +--
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ffcff6516e89..3f57074a4fab 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
return err;
}
- persistent_ram_zap(*prz);
-
*paddr += sz;
return 0;
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 12e21f789194..3ef0fd5a7cd5 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -505,15 +505,14 @@ static int persistent_ram_post_init(struct
persistent_ram_zone *prz, u32 sig,
pr_debug("found existing buffer, size %zu, start %zu\n",
buffer_size(prz), buffer_start(prz));
persistent_ram_save_old(prz);
- return 0;
}
} else {
pr_debug("no valid data in buffer (sig = 0x%08x)\n",
prz->buffer->sig);
+ prz->buffer->sig = sig;
}
/* Rewind missing or invalid memory area. */
- prz->buffer->sig = sig;
persistent_ram_zap(prz);
return 0;
--
2.19.1
[PATCH] pstore: Remove duplicate invoking of persistent_ram_zap()
From: Peng Wang
When initialing przs with invalid data in buffer(no PERSISTENT_RAM_SIG),
function call path is like this:
ramoops_init_prz ->
|
|-> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap
|
|--> persistent_ram_zap
As we can see, persistent_ram_zap() is called twice.
We can avoid this by removing it in ramoops_init_prz(),
and only call it in persistent_ram_post_init().
Signed-off-by: Peng Wang
---
fs/pstore/ram.c | 2 --
fs/pstore/ram_core.c | 3 +--
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ffcff6516e89..3f57074a4fab 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -649,8 +649,6 @@ static int ramoops_init_prz(const char *name,
return err;
}
- persistent_ram_zap(*prz);
-
*paddr += sz;
return 0;
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 12e21f789194..3ef0fd5a7cd5 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -505,15 +505,14 @@ static int persistent_ram_post_init(struct
persistent_ram_zone *prz, u32 sig,
pr_debug("found existing buffer, size %zu, start %zu\n",
buffer_size(prz), buffer_start(prz));
persistent_ram_save_old(prz);
- return 0;
}
} else {
pr_debug("no valid data in buffer (sig = 0x%08x)\n",
prz->buffer->sig);
+ prz->buffer->sig = sig;
}
/* Rewind missing or invalid memory area. */
- prz->buffer->sig = sig;
persistent_ram_zap(prz);
return 0;
--
2.19.1
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
[PATCH sparse] parse: shifting by full number of bits is undefined
The type checker wasn't identifying upper bounds for huge unsigned
64-bit numbers, because the right shift turned into a no-op:
zx2c4@thinkpad /tmp $ cat sparse.c
enum { sparse_does_not_like_this = 0x8003ULL };
zx2c4@thinkpad /tmp $ sparse sparse.c
sparse.c:1:36: warning: cast truncates bits from constant value
(8003 becomes 3)
This works around the issue by detecting when we're going to shift by
the size of the variable and treat that as always zero.
Signed-off-by: Jason A. Donenfeld
---
parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/parse.c b/parse.c
index 02a55a7..02d0615 100644
--- a/parse.c
+++ b/parse.c
@@ -788,7 +788,7 @@ static int type_is_ok(struct symbol *type, Num *upper, Num
*lower)
if (!is_unsigned)
shift--;
- if (upper->x == 0 && upper->y >> shift)
+ if (upper->x == 0 && (shift < (sizeof(upper->y) << 3)) && upper->y >>
shift)
return 0;
if (lower->x == 0 || (!is_unsigned && (~lower->y >> shift) == 0))
return 1;
--
2.19.1
[PATCH sparse] parse: shifting by full number of bits is undefined
The type checker wasn't identifying upper bounds for huge unsigned
64-bit numbers, because the right shift turned into a no-op:
zx2c4@thinkpad /tmp $ cat sparse.c
enum { sparse_does_not_like_this = 0x8003ULL };
zx2c4@thinkpad /tmp $ sparse sparse.c
sparse.c:1:36: warning: cast truncates bits from constant value
(8003 becomes 3)
This works around the issue by detecting when we're going to shift by
the size of the variable and treat that as always zero.
Signed-off-by: Jason A. Donenfeld
---
parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/parse.c b/parse.c
index 02a55a7..02d0615 100644
--- a/parse.c
+++ b/parse.c
@@ -788,7 +788,7 @@ static int type_is_ok(struct symbol *type, Num *upper, Num
*lower)
if (!is_unsigned)
shift--;
- if (upper->x == 0 && upper->y >> shift)
+ if (upper->x == 0 && (shift < (sizeof(upper->y) << 3)) && upper->y >>
shift)
return 0;
if (lower->x == 0 || (!is_unsigned && (~lower->y >> shift) == 0))
return 1;
--
2.19.1
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
linux-kernel@vger.kernel.org RE:For Your Urgent Perusal
Dear linux-kernel@vger.kernel.org , I am sorry for invading your privacy because we do not know each other but I am contacting you because of my late client who died without a will. This is to notify you that your are the beneficiary to the bequest of the sum of £10,500,000.00 GBP in the intent of my deceased client. If you accept please forward your full names, current address and your direct cell for the court documentations and so that we can obtain the probate division of court papers required for you to claim the funds from the holding bank. Thanks, Steven Walter Disclaimer: Adobe® reserves the rights to protect sensitive documents against fraudsters and spyware. Adobe® secured files are encrypted with the receivers' email and can only be viewed by the recipient. For more details visit http://www.adobe.com/legal/terms.html for details.
Re: The linux devs can rescind their license grant.
Al Viro : > * in case it needs to be spelled out: I am not at all interested > in that kind of stunts. One of the reasons I thoroughly despise RMS > and his bunch is the leverage game they tried to play with GPLv3; > damned if I'm going to lower myself to their level. Sorry, I did not mean to imply that you are one with the license-revokers. More like "if even Al Viro thinks you fucked up your process, it would be unwise to dismiss the opposition to the CoC as mere trolls". *I* can't dismiss it, because I read my mail. Apparently lots of people think that it is *my* job to fix this mess, or at least to give it a good hard try. I doubt they're trolling; what would be the point? -- http://www.catb.org/~esr/;>Eric S. Raymond My work is funded by the Internet Civil Engineering Institute: https://icei.org Please visit their site and donate: the civilization you save might be your own.
Re: The linux devs can rescind their license grant.
Al Viro : > * in case it needs to be spelled out: I am not at all interested > in that kind of stunts. One of the reasons I thoroughly despise RMS > and his bunch is the leverage game they tried to play with GPLv3; > damned if I'm going to lower myself to their level. Sorry, I did not mean to imply that you are one with the license-revokers. More like "if even Al Viro thinks you fucked up your process, it would be unwise to dismiss the opposition to the CoC as mere trolls". *I* can't dismiss it, because I read my mail. Apparently lots of people think that it is *my* job to fix this mess, or at least to give it a good hard try. I doubt they're trolling; what would be the point? -- http://www.catb.org/~esr/;>Eric S. Raymond My work is funded by the Internet Civil Engineering Institute: https://icei.org Please visit their site and donate: the civilization you save might be your own.
RE: [PATCH] PCI/Layerscape: fix wrongly invoking of outbound window disable accessor
Hi Bjorn,
Thanks a lot for your comments!
> -Original Message-
> From: Bjorn Helgaas
> Sent: 2018年10月25日 21:45
> To: Z.q. Hou
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org;
> bhelg...@google.com; lorenzo.pieral...@arm.com; Roy Zang
> ; M.h. Lian
> Subject: Re: [PATCH] PCI/Layerscape: fix wrongly invoking of outbound
> window disable accessor
>
> $ git log --oneline --follow drivers/pci/controller/dwc/pci-layerscape.c |
> head
> 6e0832fa432e PCI: Collect all native drivers under drivers/pci/controller/
> 3f43ccc4ea1b PCI: dwc: Remove old MSI IRQs API
> 8cfab3cf63cf PCI: Add SPDX GPL-2.0 to replace GPL v2 boilerplate
> 84d897d69938 PCI: layerscape: Change default error response behavior
> a335b122ba27 PCI: layerscape: Add support for ls1012a
> 03fc6134c260 PCI: layerscape: Add support for ls1088a
> 8f89357094e6 PCI: layerscape: Add support for ls2088a
> c3f909398827 PCI: layerscape: Remove unnecessary class code fixup
> e44abfed6fcb PCI: dwc: Add accessors for write permission of DBI
> read-only registers
> 4a2745d760fa PCI: layerscape: Disable outbound windows configured by
> bootloader
>
> Make yours match, e.g., "PCI: layerscape: Call dw_pcie_disable_atu()
> correctly"
>
Yes, will uniform the subject in v2.
> On Thu, Oct 25, 2018 at 08:53:37AM +, Z.q. Hou wrote:
> > From: Hou Zhiqiang
> >
> > This issue is introduced by commit 4a2745d760fac ("PCI: layerscape:
> > Disable outbound windows configured by bootloader").
>
> Conventional commit reference:
>
> $ git --no-pager show -s --abbrev-commit --abbrev=12 --pretty=format:"%h
> (\"%s\")%n" 4a2745d760fac
> 4a2745d760fa ("PCI: layerscape: Disable outbound windows configured by
> bootloader")
>
Will correct the commit ID length.
> Holy cow, this has been broken since v4.14. If fixing this makes a
> difference,
> you might want to tag it for stable.
>
How can I tag it for stable?
> > Signed-off-by: Hou Zhiqiang
> > ---
> > drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c
> > b/drivers/pci/controller/dwc/pci-layerscape.c
> > index 69f3f1a5a782..b2085988dbee 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> > @@ -88,7 +88,7 @@ static void ls_pcie_disable_outbound_atus(struct
> ls_pcie *pcie)
> > int i;
> >
> > for (i = 0; i < PCIE_IATU_NUM; i++)
> > - dw_pcie_disable_atu(pcie->pci, DW_PCIE_REGION_OUTBOUND, i);
> > + dw_pcie_disable_atu(pcie->pci, i, DW_PCIE_REGION_OUTBOUND);
> > }
> >
> > static int ls1021_pcie_link_up(struct dw_pcie *pci)
> > --
> > 2.17.1
> >
Thanks,
Zhiqiang
RE: [PATCH] PCI/Layerscape: fix wrongly invoking of outbound window disable accessor
Hi Bjorn,
Thanks a lot for your comments!
> -Original Message-
> From: Bjorn Helgaas
> Sent: 2018年10月25日 21:45
> To: Z.q. Hou
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org;
> bhelg...@google.com; lorenzo.pieral...@arm.com; Roy Zang
> ; M.h. Lian
> Subject: Re: [PATCH] PCI/Layerscape: fix wrongly invoking of outbound
> window disable accessor
>
> $ git log --oneline --follow drivers/pci/controller/dwc/pci-layerscape.c |
> head
> 6e0832fa432e PCI: Collect all native drivers under drivers/pci/controller/
> 3f43ccc4ea1b PCI: dwc: Remove old MSI IRQs API
> 8cfab3cf63cf PCI: Add SPDX GPL-2.0 to replace GPL v2 boilerplate
> 84d897d69938 PCI: layerscape: Change default error response behavior
> a335b122ba27 PCI: layerscape: Add support for ls1012a
> 03fc6134c260 PCI: layerscape: Add support for ls1088a
> 8f89357094e6 PCI: layerscape: Add support for ls2088a
> c3f909398827 PCI: layerscape: Remove unnecessary class code fixup
> e44abfed6fcb PCI: dwc: Add accessors for write permission of DBI
> read-only registers
> 4a2745d760fa PCI: layerscape: Disable outbound windows configured by
> bootloader
>
> Make yours match, e.g., "PCI: layerscape: Call dw_pcie_disable_atu()
> correctly"
>
Yes, will uniform the subject in v2.
> On Thu, Oct 25, 2018 at 08:53:37AM +, Z.q. Hou wrote:
> > From: Hou Zhiqiang
> >
> > This issue is introduced by commit 4a2745d760fac ("PCI: layerscape:
> > Disable outbound windows configured by bootloader").
>
> Conventional commit reference:
>
> $ git --no-pager show -s --abbrev-commit --abbrev=12 --pretty=format:"%h
> (\"%s\")%n" 4a2745d760fac
> 4a2745d760fa ("PCI: layerscape: Disable outbound windows configured by
> bootloader")
>
Will correct the commit ID length.
> Holy cow, this has been broken since v4.14. If fixing this makes a
> difference,
> you might want to tag it for stable.
>
How can I tag it for stable?
> > Signed-off-by: Hou Zhiqiang
> > ---
> > drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c
> > b/drivers/pci/controller/dwc/pci-layerscape.c
> > index 69f3f1a5a782..b2085988dbee 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> > @@ -88,7 +88,7 @@ static void ls_pcie_disable_outbound_atus(struct
> ls_pcie *pcie)
> > int i;
> >
> > for (i = 0; i < PCIE_IATU_NUM; i++)
> > - dw_pcie_disable_atu(pcie->pci, DW_PCIE_REGION_OUTBOUND, i);
> > + dw_pcie_disable_atu(pcie->pci, i, DW_PCIE_REGION_OUTBOUND);
> > }
> >
> > static int ls1021_pcie_link_up(struct dw_pcie *pci)
> > --
> > 2.17.1
> >
Thanks,
Zhiqiang
[PATCH v2] libata: Apply NOLPM quirk for SAMSUNG MZ7TD256HAFV-000L9
med_power_with_dipm causes my T450 to freeze with a SAMSUNG
MZ7TD256HAFV-000L9 SSD (firmware DXT02L5Q).
Switching the LPM to max_performance fixes this issue.
Signed-off-by: Diego Viola
---
drivers/ata/libata-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a9dd4ea7467d..a7f5202a4815 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4553,6 +4553,7 @@ static const struct ata_blacklist_entry
ata_device_blacklist [] = {
/* These specific Samsung models/firmware-revs do not handle LPM well */
{ "SAMSUNG MZMPC128HBFU-000MV", "CXM14M1Q", ATA_HORKAGE_NOLPM, },
{ "SAMSUNG SSD PM830 mSATA *", "CXM13D1Q", ATA_HORKAGE_NOLPM, },
+ { "SAMSUNG MZ7TD256HAFV-000L9", NULL, ATA_HORKAGE_NOLPM, },
/* devices that don't properly handle queued TRIM commands */
{ "Micron_M500IT_*","MU01", ATA_HORKAGE_NO_NCQ_TRIM |
--
2.19.1
[PATCH v2] libata: Apply NOLPM quirk for SAMSUNG MZ7TD256HAFV-000L9
med_power_with_dipm causes my T450 to freeze with a SAMSUNG
MZ7TD256HAFV-000L9 SSD (firmware DXT02L5Q).
Switching the LPM to max_performance fixes this issue.
Signed-off-by: Diego Viola
---
drivers/ata/libata-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a9dd4ea7467d..a7f5202a4815 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4553,6 +4553,7 @@ static const struct ata_blacklist_entry
ata_device_blacklist [] = {
/* These specific Samsung models/firmware-revs do not handle LPM well */
{ "SAMSUNG MZMPC128HBFU-000MV", "CXM14M1Q", ATA_HORKAGE_NOLPM, },
{ "SAMSUNG SSD PM830 mSATA *", "CXM13D1Q", ATA_HORKAGE_NOLPM, },
+ { "SAMSUNG MZ7TD256HAFV-000L9", NULL, ATA_HORKAGE_NOLPM, },
/* devices that don't properly handle queued TRIM commands */
{ "Micron_M500IT_*","MU01", ATA_HORKAGE_NO_NCQ_TRIM |
--
2.19.1
Re: [PATCH v6 1/6] dt-bindings: power: Introduce one property to present the battery internal resistance
On 26 October 2018 at 04:13, Sebastian Reichel wrote: > Hi Baolin, > > On Thu, Oct 25, 2018 at 10:01:55AM +0800, Baolin Wang wrote: >> Hi Sebastian, >> >> On 22 October 2018 at 15:43, Baolin Wang wrote: >> > The internal resistance of a battery is not a constant in its life cycle, >> > this varies over the age of the battery or temperature and so on. But we >> > just want use one constant battery internal resistance to estimate the >> > battery capacity. Thus this patch introduces one property to present >> > the battery factory internal resistance for battery information. >> > >> > Signed-off-by: Baolin Wang >> > Reviewed-by: Linus Walleij >> > --- >> > Changes from v5: >> > - None. >> > >> > Changes from v4: >> > - None. >> > >> > Changes from v3: >> > - Split binding into one separate patch. >> > - Add LinusW reviewed tag. >> > >> > Changes from v2: >> > - Rename the property. >> > - Improve the commit message. >> > >> > Changes from v1: >> > - New patch in v2. >> > --- >> >> I think this v6 patch set have addressed your comments on v5, so could >> you apply this patch set if there are no other comments? I hope this >> driver can be merged into 4.20 finally. Thanks. > > Everything looks fine to me. I will merge this directly after the > 4.20 merge window has been closed and linux-next is open again. > I'm not merging non-trivial, non-bugfix patches this lates during the > release process. Understood. Thanks Sebastian. -- Baolin Wang Best Regards
Re: [PATCH v6 1/6] dt-bindings: power: Introduce one property to present the battery internal resistance
On 26 October 2018 at 04:13, Sebastian Reichel wrote: > Hi Baolin, > > On Thu, Oct 25, 2018 at 10:01:55AM +0800, Baolin Wang wrote: >> Hi Sebastian, >> >> On 22 October 2018 at 15:43, Baolin Wang wrote: >> > The internal resistance of a battery is not a constant in its life cycle, >> > this varies over the age of the battery or temperature and so on. But we >> > just want use one constant battery internal resistance to estimate the >> > battery capacity. Thus this patch introduces one property to present >> > the battery factory internal resistance for battery information. >> > >> > Signed-off-by: Baolin Wang >> > Reviewed-by: Linus Walleij >> > --- >> > Changes from v5: >> > - None. >> > >> > Changes from v4: >> > - None. >> > >> > Changes from v3: >> > - Split binding into one separate patch. >> > - Add LinusW reviewed tag. >> > >> > Changes from v2: >> > - Rename the property. >> > - Improve the commit message. >> > >> > Changes from v1: >> > - New patch in v2. >> > --- >> >> I think this v6 patch set have addressed your comments on v5, so could >> you apply this patch set if there are no other comments? I hope this >> driver can be merged into 4.20 finally. Thanks. > > Everything looks fine to me. I will merge this directly after the > 4.20 merge window has been closed and linux-next is open again. > I'm not merging non-trivial, non-bugfix patches this lates during the > release process. Understood. Thanks Sebastian. -- Baolin Wang Best Regards
Re: [PATCH] mm/thp: Correctly differentiate between mapped THP and PMD migration entry
On 10/26/2018 12:15 AM, Zi Yan wrote:
> On 25 Oct 2018, at 4:10, Anshuman Khandual wrote:
>
>> On 10/16/2018 08:01 PM, Zi Yan wrote:
>>> On 15 Oct 2018, at 0:06, Anshuman Khandual wrote:
>>>
On 10/15/2018 06:23 AM, Zi Yan wrote:
> On 12 Oct 2018, at 4:00, Anshuman Khandual wrote:
>
>> On 10/10/2018 06:13 PM, Zi Yan wrote:
>>> On 10 Oct 2018, at 0:05, Anshuman Khandual wrote:
>>>
On 10/09/2018 07:28 PM, Zi Yan wrote:
> cc: Naoya Horiguchi (who proposed to use !_PAGE_PRESENT && !_PAGE_PSE
> for x86
> PMD migration entry check)
>
> On 8 Oct 2018, at 23:58, Anshuman Khandual wrote:
>
>> A normal mapped THP page at PMD level should be correctly
>> differentiated
>> from a PMD migration entry while walking the page table. A mapped
>> THP would
>> additionally check positive for pmd_present() along with
>> pmd_trans_huge()
>> as compared to a PMD migration entry. This just adds a new
>> conditional test
>> differentiating the two while walking the page table.
>>
>> Fixes: 616b8371539a6 ("mm: thp: enable thp migration in generic
>> path")
>> Signed-off-by: Anshuman Khandual
>> ---
>> On X86, pmd_trans_huge() and is_pmd_migration_entry() are always
>> mutually
>> exclusive which makes the current conditional block work for both
>> mapped
>> and migration entries. This is not same with arm64 where
>> pmd_trans_huge()
>
> !pmd_present() && pmd_trans_huge() is used to represent THPs under
> splitting,
Not really if we just look at code in the conditional blocks.
>>>
>>> Yeah, I explained it wrong above. Sorry about that.
>>>
>>> In x86, pmd_present() checks (_PAGE_PRESENT | _PAGE_PROTNONE |
>>> _PAGE_PSE),
>>> thus, it returns true even if the present bit is cleared but PSE bit is
>>> set.
>>
>> Okay.
>>
>>> This is done so, because THPs under splitting are regarded as present
>>> in the kernel
>>> but not present when a hardware page table walker checks it.
>>
>> Okay.
>>
>>>
>>> For PMD migration entry, which should be regarded as not present, if
>>> PSE bit
>>> is set, which makes pmd_trans_huge() returns true, like ARM64 does, all
>>> PMD migration entries will be regarded as present
>>
>> Okay to make pmd_present() return false pmd_trans_huge() has to return
>> false
>> as well. Is there anything which can be done to get around this problem
>> on
>> X86 ? pmd_trans_huge() returning true for a migration entry sounds
>> logical.
>> Otherwise we would revert the condition block order to accommodate both
>> the
>> implementation for pmd_trans_huge() as suggested by Kirill before or just
>> consider this patch forward.
>>
>> Because I am not really sure yet about the idea of getting pmd_present()
>> check into pmd_trans_huge() on arm64 just to make it fit into this
>> semantics
>> as suggested by Will. If a PMD is trans huge page or not should not
>> depend on
>> whether it is present or not.
>
> In terms of THPs, we have three cases: a present THP, a THP under
> splitting,
> and a THP under migration. pmd_present() and pmd_trans_huge() both return
> true
> for a present THP and a THP under splitting, because they discover
> _PAGE_PSE bit
Then how do we differentiate between a mapped THP and a splitting THP.
>>>
>>> AFAIK, in x86, there is no distinction between a mapped THP and a splitting
>>> THP
>>> using helper functions.
>>>
>>> A mapped THP has _PAGE_PRESENT bit and _PAGE_PSE bit set, whereas a
>>> splitting THP
>>> has only _PAGE_PSE bit set. But both pmd_present() and pmd_trans_huge()
>>> return
>>> true as long as _PAGE_PSE bit is set.
>>
>> I understand that. What I was wondering was since there is a need to
>> differentiate
>> between a mapped THP and a splitting THP at various places in generic THP,
>> we would
>> need to way to identify each of them unambiguously some how. Is that
>> particular
>> assumption wrong ? Dont we need to differentiate between a mapped THP and
>> THP under
>> splitting ?
>
> According to Andrea's explanation here:
> https://lore.kernel.org/patchwork/patch/997412/#1184298,
> we do not distinguish between a mapped THP and a splitting THP, because
> pmd_to_page()
> can return valid pages for both cases.
I have gone through Andrea's explanation but still trying to understand all
those
details in there. Will get back on this.
>
>>>
> is set for both cases, whereas they both return false for a THP under
> migration.
> You want to change them to make pmd_trans_huge() returns true for a THP
> under migration
> instead of false to help ARM64’s
Re: [PATCH] mm/thp: Correctly differentiate between mapped THP and PMD migration entry
On 10/26/2018 12:15 AM, Zi Yan wrote:
> On 25 Oct 2018, at 4:10, Anshuman Khandual wrote:
>
>> On 10/16/2018 08:01 PM, Zi Yan wrote:
>>> On 15 Oct 2018, at 0:06, Anshuman Khandual wrote:
>>>
On 10/15/2018 06:23 AM, Zi Yan wrote:
> On 12 Oct 2018, at 4:00, Anshuman Khandual wrote:
>
>> On 10/10/2018 06:13 PM, Zi Yan wrote:
>>> On 10 Oct 2018, at 0:05, Anshuman Khandual wrote:
>>>
On 10/09/2018 07:28 PM, Zi Yan wrote:
> cc: Naoya Horiguchi (who proposed to use !_PAGE_PRESENT && !_PAGE_PSE
> for x86
> PMD migration entry check)
>
> On 8 Oct 2018, at 23:58, Anshuman Khandual wrote:
>
>> A normal mapped THP page at PMD level should be correctly
>> differentiated
>> from a PMD migration entry while walking the page table. A mapped
>> THP would
>> additionally check positive for pmd_present() along with
>> pmd_trans_huge()
>> as compared to a PMD migration entry. This just adds a new
>> conditional test
>> differentiating the two while walking the page table.
>>
>> Fixes: 616b8371539a6 ("mm: thp: enable thp migration in generic
>> path")
>> Signed-off-by: Anshuman Khandual
>> ---
>> On X86, pmd_trans_huge() and is_pmd_migration_entry() are always
>> mutually
>> exclusive which makes the current conditional block work for both
>> mapped
>> and migration entries. This is not same with arm64 where
>> pmd_trans_huge()
>
> !pmd_present() && pmd_trans_huge() is used to represent THPs under
> splitting,
Not really if we just look at code in the conditional blocks.
>>>
>>> Yeah, I explained it wrong above. Sorry about that.
>>>
>>> In x86, pmd_present() checks (_PAGE_PRESENT | _PAGE_PROTNONE |
>>> _PAGE_PSE),
>>> thus, it returns true even if the present bit is cleared but PSE bit is
>>> set.
>>
>> Okay.
>>
>>> This is done so, because THPs under splitting are regarded as present
>>> in the kernel
>>> but not present when a hardware page table walker checks it.
>>
>> Okay.
>>
>>>
>>> For PMD migration entry, which should be regarded as not present, if
>>> PSE bit
>>> is set, which makes pmd_trans_huge() returns true, like ARM64 does, all
>>> PMD migration entries will be regarded as present
>>
>> Okay to make pmd_present() return false pmd_trans_huge() has to return
>> false
>> as well. Is there anything which can be done to get around this problem
>> on
>> X86 ? pmd_trans_huge() returning true for a migration entry sounds
>> logical.
>> Otherwise we would revert the condition block order to accommodate both
>> the
>> implementation for pmd_trans_huge() as suggested by Kirill before or just
>> consider this patch forward.
>>
>> Because I am not really sure yet about the idea of getting pmd_present()
>> check into pmd_trans_huge() on arm64 just to make it fit into this
>> semantics
>> as suggested by Will. If a PMD is trans huge page or not should not
>> depend on
>> whether it is present or not.
>
> In terms of THPs, we have three cases: a present THP, a THP under
> splitting,
> and a THP under migration. pmd_present() and pmd_trans_huge() both return
> true
> for a present THP and a THP under splitting, because they discover
> _PAGE_PSE bit
Then how do we differentiate between a mapped THP and a splitting THP.
>>>
>>> AFAIK, in x86, there is no distinction between a mapped THP and a splitting
>>> THP
>>> using helper functions.
>>>
>>> A mapped THP has _PAGE_PRESENT bit and _PAGE_PSE bit set, whereas a
>>> splitting THP
>>> has only _PAGE_PSE bit set. But both pmd_present() and pmd_trans_huge()
>>> return
>>> true as long as _PAGE_PSE bit is set.
>>
>> I understand that. What I was wondering was since there is a need to
>> differentiate
>> between a mapped THP and a splitting THP at various places in generic THP,
>> we would
>> need to way to identify each of them unambiguously some how. Is that
>> particular
>> assumption wrong ? Dont we need to differentiate between a mapped THP and
>> THP under
>> splitting ?
>
> According to Andrea's explanation here:
> https://lore.kernel.org/patchwork/patch/997412/#1184298,
> we do not distinguish between a mapped THP and a splitting THP, because
> pmd_to_page()
> can return valid pages for both cases.
I have gone through Andrea's explanation but still trying to understand all
those
details in there. Will get back on this.
>
>>>
> is set for both cases, whereas they both return false for a THP under
> migration.
> You want to change them to make pmd_trans_huge() returns true for a THP
> under migration
> instead of false to help ARM64’s
Re: [GIT] Sparc
On Thu, Oct 25, 2018 at 11:28 AM David Miller wrote: > > Please pull to get this build regression fix. Pulled, Linus
Re: [GIT] Sparc
On Thu, Oct 25, 2018 at 11:28 AM David Miller wrote: > > Please pull to get this build regression fix. Pulled, Linus
[PATCH] libata: Apply NOLPM quirk for SAMSUNG MZ7TD256HAFV-000L9
med_power_with_dipm causes my T450 to freeze with a SAMSUNG
MZ7TD256HAFV-000L9 SSD (firmware DXT02L5Q).
Switching the LPM to max_performance fixes this issue.
Signed-off-by: Diego Viola
---
drivers/ata/libata-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a9dd4ea7467d..57706ab1b545 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4553,6 +4553,7 @@ static const struct ata_blacklist_entry
ata_device_blacklist [] = {
/* These specific Samsung models/firmware-revs do not handle LPM well */
{ "SAMSUNG MZMPC128HBFU-000MV", "CXM14M1Q", ATA_HORKAGE_NOLPM, },
{ "SAMSUNG SSD PM830 mSATA *", "CXM13D1Q", ATA_HORKAGE_NOLPM, },
+ { "SAMSUNG MZ7TD256HAFV-000L9", NULL, ATA_HORKAGE_NOLPM, },
/* devices that don't properly handle queued TRIM commands */
{ "Micron_M500IT_*","MU01", ATA_HORKAGE_NO_NCQ_TRIM |
--
2.19.1
[PATCH] libata: Apply NOLPM quirk for SAMSUNG MZ7TD256HAFV-000L9
med_power_with_dipm causes my T450 to freeze with a SAMSUNG
MZ7TD256HAFV-000L9 SSD (firmware DXT02L5Q).
Switching the LPM to max_performance fixes this issue.
Signed-off-by: Diego Viola
---
drivers/ata/libata-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a9dd4ea7467d..57706ab1b545 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4553,6 +4553,7 @@ static const struct ata_blacklist_entry
ata_device_blacklist [] = {
/* These specific Samsung models/firmware-revs do not handle LPM well */
{ "SAMSUNG MZMPC128HBFU-000MV", "CXM14M1Q", ATA_HORKAGE_NOLPM, },
{ "SAMSUNG SSD PM830 mSATA *", "CXM13D1Q", ATA_HORKAGE_NOLPM, },
+ { "SAMSUNG MZ7TD256HAFV-000L9", NULL, ATA_HORKAGE_NOLPM, },
/* devices that don't properly handle queued TRIM commands */
{ "Micron_M500IT_*","MU01", ATA_HORKAGE_NO_NCQ_TRIM |
--
2.19.1
Re: [GIT PULL] KVM updates for Linux 4.20-rc1
On Wed, Oct 24, 2018 at 4:14 AM Radim Krčmář wrote: > > KVM updates for v4.20 Pulled, Linus
Re: [GIT PULL] KVM updates for Linux 4.20-rc1
On Wed, Oct 24, 2018 at 4:14 AM Radim Krčmář wrote: > > KVM updates for v4.20 Pulled, Linus
Re: [v3 07/12] x86/fsgsbase/64: Introduce the new FIND_PERCPU_BASE macro
From: Andy Lutomirski Sent: October 26, 2018 at 12:25:17 AM GMT > To: Bae, Chang Seok , Nadav Amit > Cc: Ingo Molnar , Thomas Gleixner , > Andrew Lutomirski , H. Peter Anvin , Andi > Kleen , Dave Hansen , > Metzger, Markus T , Ravi V. Shankar > , LKML > Subject: Re: [v3 07/12] x86/fsgsbase/64: Introduce the new FIND_PERCPU_BASE > macro > > > On Tue, Oct 23, 2018 at 11:43 AM Chang S. Bae > wrote: >> GSBASE is used to find per-CPU data in the kernel. But when it is unknown, >> the per-CPU base can be found from the per_cpu_offset table with a CPU NR. >> The CPU NR is extracted from the limit field of the CPUNODE entry in GDT, >> or by the RDPID instruction. >> >> Also, add the GAS-compatible RDPID macro. >> >> The new macro will be used on a following patch. >> >> Suggested-by: H. Peter Anvin >> Signed-off-by: Chang S. Bae >> Cc: Andi Kleen >> Cc: Andy Lutomirski >> Cc: Dave Hansen >> Cc: Thomas Gleixner >> Cc: Ingo Molnar >> --- >> arch/x86/include/asm/fsgsbase.h | 52 + >> arch/x86/include/asm/inst.h | 15 ++ >> 2 files changed, 67 insertions(+) >> >> diff --git a/arch/x86/include/asm/fsgsbase.h >> b/arch/x86/include/asm/fsgsbase.h >> index e500d771155f..0c2d7d8a8c01 100644 >> --- a/arch/x86/include/asm/fsgsbase.h >> +++ b/arch/x86/include/asm/fsgsbase.h >> @@ -111,6 +111,58 @@ extern void x86_gsbase_write_cpu_inactive(unsigned long >> gsbase); >>MODRM 0xd0 wrgsbase_opd 1 >> .endm >> >> +#if CONFIG_SMP >> + >> +/* >> + * Fetch the per-CPU GSBASE value for this processor and put it in @reg. >> + * We normally use %gs for accessing per-CPU data, but we are setting up >> + * %gs here and obviously can not use %gs itself to access per-CPU data. >> + */ >> +.macro FIND_PERCPU_BASE_RDPID reg:req >> + /* >> +* The CPU/node NR is initialized earlier, directly in cpu_init(). > P >> +*/ >> + RDPID \reg > > I would suggest that you instead add a macro LOAD_CPU_AND_NODE \reg > and have that macro contain the alternative. It can switch between > RDPID and LSL. This way you avoid duplicating the rest of it. > > This should end up in the same header as __getcpu() -- it probably > makes sense to just move __getcpu() for this purpose. > > Also, hpa and Nadav, shouldn't asm/inst.h end up in macros.S? If there are going to be C uses, yes. Ingo was concerned that the .s file will be too big, so it should not be overly abused. In addition, I want to send a patch that recompiles all the .c files in macro.S changes. I’m worried it will start creating build problems.
Re: [v3 07/12] x86/fsgsbase/64: Introduce the new FIND_PERCPU_BASE macro
From: Andy Lutomirski Sent: October 26, 2018 at 12:25:17 AM GMT > To: Bae, Chang Seok , Nadav Amit > Cc: Ingo Molnar , Thomas Gleixner , > Andrew Lutomirski , H. Peter Anvin , Andi > Kleen , Dave Hansen , > Metzger, Markus T , Ravi V. Shankar > , LKML > Subject: Re: [v3 07/12] x86/fsgsbase/64: Introduce the new FIND_PERCPU_BASE > macro > > > On Tue, Oct 23, 2018 at 11:43 AM Chang S. Bae > wrote: >> GSBASE is used to find per-CPU data in the kernel. But when it is unknown, >> the per-CPU base can be found from the per_cpu_offset table with a CPU NR. >> The CPU NR is extracted from the limit field of the CPUNODE entry in GDT, >> or by the RDPID instruction. >> >> Also, add the GAS-compatible RDPID macro. >> >> The new macro will be used on a following patch. >> >> Suggested-by: H. Peter Anvin >> Signed-off-by: Chang S. Bae >> Cc: Andi Kleen >> Cc: Andy Lutomirski >> Cc: Dave Hansen >> Cc: Thomas Gleixner >> Cc: Ingo Molnar >> --- >> arch/x86/include/asm/fsgsbase.h | 52 + >> arch/x86/include/asm/inst.h | 15 ++ >> 2 files changed, 67 insertions(+) >> >> diff --git a/arch/x86/include/asm/fsgsbase.h >> b/arch/x86/include/asm/fsgsbase.h >> index e500d771155f..0c2d7d8a8c01 100644 >> --- a/arch/x86/include/asm/fsgsbase.h >> +++ b/arch/x86/include/asm/fsgsbase.h >> @@ -111,6 +111,58 @@ extern void x86_gsbase_write_cpu_inactive(unsigned long >> gsbase); >>MODRM 0xd0 wrgsbase_opd 1 >> .endm >> >> +#if CONFIG_SMP >> + >> +/* >> + * Fetch the per-CPU GSBASE value for this processor and put it in @reg. >> + * We normally use %gs for accessing per-CPU data, but we are setting up >> + * %gs here and obviously can not use %gs itself to access per-CPU data. >> + */ >> +.macro FIND_PERCPU_BASE_RDPID reg:req >> + /* >> +* The CPU/node NR is initialized earlier, directly in cpu_init(). > P >> +*/ >> + RDPID \reg > > I would suggest that you instead add a macro LOAD_CPU_AND_NODE \reg > and have that macro contain the alternative. It can switch between > RDPID and LSL. This way you avoid duplicating the rest of it. > > This should end up in the same header as __getcpu() -- it probably > makes sense to just move __getcpu() for this purpose. > > Also, hpa and Nadav, shouldn't asm/inst.h end up in macros.S? If there are going to be C uses, yes. Ingo was concerned that the .s file will be too big, so it should not be overly abused. In addition, I want to send a patch that recompiles all the .c files in macro.S changes. I’m worried it will start creating build problems.
Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Loic, > >> -Original Message- >> From: Suman Anna >> Sent: mercredi 24 octobre 2018 05:47 >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >> o...@wizery.com >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >> Arnaud POULIQUEN ; >> benjamin.gaign...@linaro.org >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device >> address requested >> >> On 10/23/18 2:40 PM, Loic PALLARDY wrote: >>> Hi Suman, >>> -Original Message- From: Suman Anna Sent: mardi 23 octobre 2018 19:26 To: Loic PALLARDY ; bjorn.anders...@linaro.org; o...@wizery.com Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; Arnaud POULIQUEN ; benjamin.gaign...@linaro.org Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if >> device address requested Hi Loic, On 7/27/18 8:14 AM, Loic Pallardy wrote: > If there is no IOMMU associate to remote processor device, > remoteproc_core won't be able to satisfy device address requested > in firmware resource table. > Return an error as configuration won't be coherent. > > Signed-off-by: Loic Pallardy This patch is breaking my Davinci platforms. It is not really required that you _should_ have IOMMUs when a valid DA is mentioned. Please >> see the existing description (paras 4 and 5) on the fw_rsc_carveout kerneldoc in remoteproc.h file. >>> >>> Thanks for pointing this comment. Indeed sMMU is not mandatory, and at >> first sight I agree we should remove the restriction introduced by the patch. >>> Driver porting on the series should be done before adding this. We do have platforms where we have some internal sub-modules within >> the remote processor sub-system that provides some linear address-translation (most common case with 32-bit processors supporting 64-bit addresses). Also, we have some upcoming SoCs where we have an MMU but is not programmable by Linux. There is one comment there, but I don't think this is actually handled in the current remoteproc core. "If @da is set to * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and >> then * overwrite @da with the dynamically allocated address." >>> I don't remember it was implemented like described. >> >> Yes, it was missing, and one of your patches seem to add this behavior >> now. That said, I really don't think the remoteproc core can dictate the >> da. Even if the individual remoteproc driver were to furnish this, how >> would you get such data without forcing a fixed behavior for all >> possible firmwares (not desirable). We should get rid of this comment, >> and any code that seems to do this. > > Agree that if some rules are fixed by firmware, rproc core can't overwrite > them. > It was the goal of the patch as without sMMU if you don't have a match > between rproc core carveout allocation and firmware rsc request, rproc core > can't answer the request. Question is what are we going to use to check that - da or pa? With fixed memory support, I would think that this would have to be based on pa if specified. Given that the meaning of da can change from firmware to firmware (in the case of some translators), but you can only reserve a fixed reserved memory at kernel bootup time. > In the rest of the series, da in rsc table is updated only in some specific > cases (i.e. da is equal to FW_RSC_ADDR_ANY). If there is a force update I > agree we should not allow it. >> >>> >>> I have remarks about the comment: >>> "* We will always use @da to negotiate the device addresses, even if it >>> * isn't using an iommu. In that case, though, it will obviously contain >>> * physical addresses." >>> >>> When there is no sMMU, we can't consider that da contains a physical >> address because coprocessor can have its own memory map just because it >> is a 32bit processor accessing only a part of the memory and the main is >> 64bit >> one. The 2 processors won't see the internal memory at the same base >> address for example. >> >> Agreed, believe it was valid when it was written (32-bit platforms >> supporting 32-bit addresses). I think this is akin to an IPA >> (Intermediate Physical Address). >> >>> So what should we do when carveout allocated by host is not fitting with >> resource table request? >>> - put a warning and overwrite da address in the resource table? >> >> Hmm, why da? This goes to my earlier comment about how you are able to >> decide the da. Atleast your current ST driver seems to be assigning the >> same value as the physical bus address for da, which would prompt why >> you would still need a carveout entry in the resource table if it is >> truly one-to-one. >> >> Eg, I have an upcoming usecase with R5Fs on newer TI SoCs where we >> actually have a sub-module called Region Address Translator (RAT) which >> can only be programmed by the R5F
Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device address requested
Hi Loic, > >> -Original Message- >> From: Suman Anna >> Sent: mercredi 24 octobre 2018 05:47 >> To: Loic PALLARDY ; bjorn.anders...@linaro.org; >> o...@wizery.com >> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; >> Arnaud POULIQUEN ; >> benjamin.gaign...@linaro.org >> Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if device >> address requested >> >> On 10/23/18 2:40 PM, Loic PALLARDY wrote: >>> Hi Suman, >>> -Original Message- From: Suman Anna Sent: mardi 23 octobre 2018 19:26 To: Loic PALLARDY ; bjorn.anders...@linaro.org; o...@wizery.com Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; Arnaud POULIQUEN ; benjamin.gaign...@linaro.org Subject: Re: [PATCH v4 01/17] remoteproc: configure IOMMU only if >> device address requested Hi Loic, On 7/27/18 8:14 AM, Loic Pallardy wrote: > If there is no IOMMU associate to remote processor device, > remoteproc_core won't be able to satisfy device address requested > in firmware resource table. > Return an error as configuration won't be coherent. > > Signed-off-by: Loic Pallardy This patch is breaking my Davinci platforms. It is not really required that you _should_ have IOMMUs when a valid DA is mentioned. Please >> see the existing description (paras 4 and 5) on the fw_rsc_carveout kerneldoc in remoteproc.h file. >>> >>> Thanks for pointing this comment. Indeed sMMU is not mandatory, and at >> first sight I agree we should remove the restriction introduced by the patch. >>> Driver porting on the series should be done before adding this. We do have platforms where we have some internal sub-modules within >> the remote processor sub-system that provides some linear address-translation (most common case with 32-bit processors supporting 64-bit addresses). Also, we have some upcoming SoCs where we have an MMU but is not programmable by Linux. There is one comment there, but I don't think this is actually handled in the current remoteproc core. "If @da is set to * FW_RSC_ADDR_ANY, then the host will dynamically allocate it, and >> then * overwrite @da with the dynamically allocated address." >>> I don't remember it was implemented like described. >> >> Yes, it was missing, and one of your patches seem to add this behavior >> now. That said, I really don't think the remoteproc core can dictate the >> da. Even if the individual remoteproc driver were to furnish this, how >> would you get such data without forcing a fixed behavior for all >> possible firmwares (not desirable). We should get rid of this comment, >> and any code that seems to do this. > > Agree that if some rules are fixed by firmware, rproc core can't overwrite > them. > It was the goal of the patch as without sMMU if you don't have a match > between rproc core carveout allocation and firmware rsc request, rproc core > can't answer the request. Question is what are we going to use to check that - da or pa? With fixed memory support, I would think that this would have to be based on pa if specified. Given that the meaning of da can change from firmware to firmware (in the case of some translators), but you can only reserve a fixed reserved memory at kernel bootup time. > In the rest of the series, da in rsc table is updated only in some specific > cases (i.e. da is equal to FW_RSC_ADDR_ANY). If there is a force update I > agree we should not allow it. >> >>> >>> I have remarks about the comment: >>> "* We will always use @da to negotiate the device addresses, even if it >>> * isn't using an iommu. In that case, though, it will obviously contain >>> * physical addresses." >>> >>> When there is no sMMU, we can't consider that da contains a physical >> address because coprocessor can have its own memory map just because it >> is a 32bit processor accessing only a part of the memory and the main is >> 64bit >> one. The 2 processors won't see the internal memory at the same base >> address for example. >> >> Agreed, believe it was valid when it was written (32-bit platforms >> supporting 32-bit addresses). I think this is akin to an IPA >> (Intermediate Physical Address). >> >>> So what should we do when carveout allocated by host is not fitting with >> resource table request? >>> - put a warning and overwrite da address in the resource table? >> >> Hmm, why da? This goes to my earlier comment about how you are able to >> decide the da. Atleast your current ST driver seems to be assigning the >> same value as the physical bus address for da, which would prompt why >> you would still need a carveout entry in the resource table if it is >> truly one-to-one. >> >> Eg, I have an upcoming usecase with R5Fs on newer TI SoCs where we >> actually have a sub-module called Region Address Translator (RAT) which >> can only be programmed by the R5F
[PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
From: Victor Colombo
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
---
drivers/staging/iio/resolver/ad2s90.c | 32 ++-
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index b4a6a89c11b0..52b656875ed1 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
+ *val = 0;
+ *val2 = 1534355;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
[PATCH 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 52b656875ed1..24002042a5c5 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
--
2.18.0
[PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
From: Victor Colombo
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
---
drivers/staging/iio/resolver/ad2s90.c | 32 ++-
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index b4a6a89c11b0..52b656875ed1 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
+ *val = 0;
+ *val2 = 1534355;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
[PATCH 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 52b656875ed1..24002042a5c5 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
--
2.18.0
[PATCH 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 55 ++- 1 file changed, 37 insertions(+), 18 deletions(-) -- 2.18.0
[PATCH 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 55 ++- 1 file changed, 37 insertions(+), 18 deletions(-) -- 2.18.0
[PATCH 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index c20d37dc065a..b4a6a89c11b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -81,10 +81,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -95,7 +91,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
[PATCH 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..11fac9f90148 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
struct ad2s90_state *st = iio_priv(indio_dev);
mutex_lock(>lock);
+
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
[PATCH 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index d6a42e3f1bd8..c20d37dc065a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -64,7 +64,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
[PATCH 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index c20d37dc065a..b4a6a89c11b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -81,10 +81,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -95,7 +91,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
[PATCH 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..11fac9f90148 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
struct ad2s90_state *st = iio_priv(indio_dev);
mutex_lock(>lock);
+
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
[PATCH 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index d6a42e3f1bd8..c20d37dc065a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -64,7 +64,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
[PATCH 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
[PATCH 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
Re: [PATCH RFC v2 1/1] hugetlbfs: use i_mmap_rwsem for pmd sharing and truncate/fault sync
Hi Mike,
On Tue, Oct 23, 2018 at 09:50:53PM -0700, Mike Kravetz wrote:
> hugetlbfs does not correctly handle page faults racing with truncation.
> In addition, shared pmds can cause additional issues.
>
> Without pmd sharing, issues can occur as follows:
> A huegtlbfs file is mmap(MAP_SHARED) with a size of 4 pages. At
> mmap time, 4 huge pages are reserved for the file/mapping. So,
> the global reserve count is 4. In addition, since this is a shared
> mapping an entry for 4 pages is added to the file's reserve map.
> The first 3 of the 4 pages are faulted into the file. As a result,
> the global reserve count is now 1.
>
> Task A starts to fault in the last page (routines hugetlb_fault,
> hugetlb_no_page). It allocates a huge page (alloc_huge_page).
> The reserve map indicates there is a reserved page, so this is
> used and the global reserve count goes to 0.
>
> Now, task B truncates the file to size 0. It starts by setting
> inode size to 0(hugetlb_vmtruncate). It then unmaps all mapping
> of the file (hugetlb_vmdelete_list). Since task A's page table
> lock is not held at the time, truncation is not blocked. Truncation
> removes the 3 pages from the file (remove_inode_hugepages). When
> cleaning up the reserved pages (hugetlb_unreserve_pages), it notices
> the reserve map was for 4 pages. However, it has only freed 3 pages.
> So it assumes there is still (4 - 3) 1 reserved pages. It then
> decrements the global reserve count by 1 and it goes negative.
>
> Task A then continues the page fault process and adds it's newly
> acquired page to the page cache. Note that the index of this page
> is beyond the size of the truncated file (0). The page fault process
> then notices the file has been truncated and exits. However, the
> page is left in the cache associated with the file.
>
> Now, if the file is immediately deleted the truncate code runs again.
> It will find and free the one page associated with the file. When
> cleaning up reserves, it notices the reserve map is empty. Yet, one
> page freed. So, the global reserve count is decremented by (0 - 1) -1.
> This returns the global count to 0 as it should be. But, it is
> possible for someone else to mmap this file/range before it is deleted.
> If this happens, a reserve map entry for the allocated page is created
> and the reserved page is forever leaked.
>
> With pmd sharing, the situation is even worse. Consider the following:
> A task processes a page fault on a shared hugetlbfs file and calls
> huge_pte_alloc to get a ptep. Suppose the returned ptep points to a
> shared pmd.
>
> Now, anopther task truncates the hugetlbfs file. As part of truncation,
> it unmaps everyone who has the file mapped. If a task has a shared pmd
> in this range, huge_pmd_unshhare will be called. If this is not the last
(sorry, nitpicking ..) a few typos ("anophter" and "unshhare").
> user sharing the pmd, huge_pmd_unshare will clear pud pointing to the
> pmd. For the task in the middle of the page fault, the ptep returned by
> huge_pte_alloc points to another task's page table or worse. This leads
> to bad things such as incorrect page map/reference counts or invalid
> memory references.
>
> i_mmap_rwsem is currently used for pmd sharing synchronization. It is also
> held during unmap and whenever a call to huge_pmd_unshare is possible. It
> is only acquired in write mode. Expand and modify the use of i_mmap_rwsem
> as follows:
> - i_mmap_rwsem is held in write mode for the duration of truncate
> processing.
> - i_mmap_rwsem is held in write mode whenever huge_pmd_share is called.
I guess you mean huge_pmd_unshare here, right?
> - i_mmap_rwsem is held in read mode whenever huge_pmd_share is called.
> Today that is only via huge_pte_alloc.
> - i_mmap_rwsem is held in read mode after huge_pte_alloc, until the caller
> is finished with the returned ptep.
>
> Signed-off-by: Mike Kravetz
> ---
> fs/hugetlbfs/inode.c | 21 ++
> mm/hugetlb.c | 65 +---
> mm/rmap.c| 10 +++
> mm/userfaultfd.c | 11 ++--
> 4 files changed, 84 insertions(+), 23 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 32920a10100e..6ee97622a231 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -426,10 +426,16 @@ static void remove_inode_hugepages(struct inode *inode,
> loff_t lstart,
> u32 hash;
>
> index = page->index;
> - hash = hugetlb_fault_mutex_hash(h, current->mm,
> + /*
> + * No need to take fault mutex for truncation as we
> + * are synchronized via i_mmap_rwsem.
> + */
> + if (!truncate_op) {
> + hash = hugetlb_fault_mutex_hash(h,
Re: [PATCH RFC v2 1/1] hugetlbfs: use i_mmap_rwsem for pmd sharing and truncate/fault sync
Hi Mike,
On Tue, Oct 23, 2018 at 09:50:53PM -0700, Mike Kravetz wrote:
> hugetlbfs does not correctly handle page faults racing with truncation.
> In addition, shared pmds can cause additional issues.
>
> Without pmd sharing, issues can occur as follows:
> A huegtlbfs file is mmap(MAP_SHARED) with a size of 4 pages. At
> mmap time, 4 huge pages are reserved for the file/mapping. So,
> the global reserve count is 4. In addition, since this is a shared
> mapping an entry for 4 pages is added to the file's reserve map.
> The first 3 of the 4 pages are faulted into the file. As a result,
> the global reserve count is now 1.
>
> Task A starts to fault in the last page (routines hugetlb_fault,
> hugetlb_no_page). It allocates a huge page (alloc_huge_page).
> The reserve map indicates there is a reserved page, so this is
> used and the global reserve count goes to 0.
>
> Now, task B truncates the file to size 0. It starts by setting
> inode size to 0(hugetlb_vmtruncate). It then unmaps all mapping
> of the file (hugetlb_vmdelete_list). Since task A's page table
> lock is not held at the time, truncation is not blocked. Truncation
> removes the 3 pages from the file (remove_inode_hugepages). When
> cleaning up the reserved pages (hugetlb_unreserve_pages), it notices
> the reserve map was for 4 pages. However, it has only freed 3 pages.
> So it assumes there is still (4 - 3) 1 reserved pages. It then
> decrements the global reserve count by 1 and it goes negative.
>
> Task A then continues the page fault process and adds it's newly
> acquired page to the page cache. Note that the index of this page
> is beyond the size of the truncated file (0). The page fault process
> then notices the file has been truncated and exits. However, the
> page is left in the cache associated with the file.
>
> Now, if the file is immediately deleted the truncate code runs again.
> It will find and free the one page associated with the file. When
> cleaning up reserves, it notices the reserve map is empty. Yet, one
> page freed. So, the global reserve count is decremented by (0 - 1) -1.
> This returns the global count to 0 as it should be. But, it is
> possible for someone else to mmap this file/range before it is deleted.
> If this happens, a reserve map entry for the allocated page is created
> and the reserved page is forever leaked.
>
> With pmd sharing, the situation is even worse. Consider the following:
> A task processes a page fault on a shared hugetlbfs file and calls
> huge_pte_alloc to get a ptep. Suppose the returned ptep points to a
> shared pmd.
>
> Now, anopther task truncates the hugetlbfs file. As part of truncation,
> it unmaps everyone who has the file mapped. If a task has a shared pmd
> in this range, huge_pmd_unshhare will be called. If this is not the last
(sorry, nitpicking ..) a few typos ("anophter" and "unshhare").
> user sharing the pmd, huge_pmd_unshare will clear pud pointing to the
> pmd. For the task in the middle of the page fault, the ptep returned by
> huge_pte_alloc points to another task's page table or worse. This leads
> to bad things such as incorrect page map/reference counts or invalid
> memory references.
>
> i_mmap_rwsem is currently used for pmd sharing synchronization. It is also
> held during unmap and whenever a call to huge_pmd_unshare is possible. It
> is only acquired in write mode. Expand and modify the use of i_mmap_rwsem
> as follows:
> - i_mmap_rwsem is held in write mode for the duration of truncate
> processing.
> - i_mmap_rwsem is held in write mode whenever huge_pmd_share is called.
I guess you mean huge_pmd_unshare here, right?
> - i_mmap_rwsem is held in read mode whenever huge_pmd_share is called.
> Today that is only via huge_pte_alloc.
> - i_mmap_rwsem is held in read mode after huge_pte_alloc, until the caller
> is finished with the returned ptep.
>
> Signed-off-by: Mike Kravetz
> ---
> fs/hugetlbfs/inode.c | 21 ++
> mm/hugetlb.c | 65 +---
> mm/rmap.c| 10 +++
> mm/userfaultfd.c | 11 ++--
> 4 files changed, 84 insertions(+), 23 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 32920a10100e..6ee97622a231 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -426,10 +426,16 @@ static void remove_inode_hugepages(struct inode *inode,
> loff_t lstart,
> u32 hash;
>
> index = page->index;
> - hash = hugetlb_fault_mutex_hash(h, current->mm,
> + /*
> + * No need to take fault mutex for truncation as we
> + * are synchronized via i_mmap_rwsem.
> + */
> + if (!truncate_op) {
> + hash = hugetlb_fault_mutex_hash(h,
Re: [v3 07/12] x86/fsgsbase/64: Introduce the new FIND_PERCPU_BASE macro
On Tue, Oct 23, 2018 at 11:43 AM Chang S. Bae wrote: > > GSBASE is used to find per-CPU data in the kernel. But when it is unknown, > the per-CPU base can be found from the per_cpu_offset table with a CPU NR. > The CPU NR is extracted from the limit field of the CPUNODE entry in GDT, > or by the RDPID instruction. > > Also, add the GAS-compatible RDPID macro. > > The new macro will be used on a following patch. > > Suggested-by: H. Peter Anvin > Signed-off-by: Chang S. Bae > Cc: Andi Kleen > Cc: Andy Lutomirski > Cc: Dave Hansen > Cc: Thomas Gleixner > Cc: Ingo Molnar > --- > arch/x86/include/asm/fsgsbase.h | 52 + > arch/x86/include/asm/inst.h | 15 ++ > 2 files changed, 67 insertions(+) > > diff --git a/arch/x86/include/asm/fsgsbase.h b/arch/x86/include/asm/fsgsbase.h > index e500d771155f..0c2d7d8a8c01 100644 > --- a/arch/x86/include/asm/fsgsbase.h > +++ b/arch/x86/include/asm/fsgsbase.h > @@ -111,6 +111,58 @@ extern void x86_gsbase_write_cpu_inactive(unsigned long > gsbase); > MODRM 0xd0 wrgsbase_opd 1 > .endm > > +#if CONFIG_SMP > + > +/* > + * Fetch the per-CPU GSBASE value for this processor and put it in @reg. > + * We normally use %gs for accessing per-CPU data, but we are setting up > + * %gs here and obviously can not use %gs itself to access per-CPU data. > + */ > +.macro FIND_PERCPU_BASE_RDPID reg:req > + /* > +* The CPU/node NR is initialized earlier, directly in cpu_init(). P > +*/ > + RDPID \reg I would suggest that you instead add a macro LOAD_CPU_AND_NODE \reg and have that macro contain the alternative. It can switch between RDPID and LSL. This way you avoid duplicating the rest of it. This should end up in the same header as __getcpu() -- it probably makes sense to just move __getcpu() for this purpose. Also, hpa and Nadav, shouldn't asm/inst.h end up in macros.S? --Andy
Re: [v3 07/12] x86/fsgsbase/64: Introduce the new FIND_PERCPU_BASE macro
On Tue, Oct 23, 2018 at 11:43 AM Chang S. Bae wrote: > > GSBASE is used to find per-CPU data in the kernel. But when it is unknown, > the per-CPU base can be found from the per_cpu_offset table with a CPU NR. > The CPU NR is extracted from the limit field of the CPUNODE entry in GDT, > or by the RDPID instruction. > > Also, add the GAS-compatible RDPID macro. > > The new macro will be used on a following patch. > > Suggested-by: H. Peter Anvin > Signed-off-by: Chang S. Bae > Cc: Andi Kleen > Cc: Andy Lutomirski > Cc: Dave Hansen > Cc: Thomas Gleixner > Cc: Ingo Molnar > --- > arch/x86/include/asm/fsgsbase.h | 52 + > arch/x86/include/asm/inst.h | 15 ++ > 2 files changed, 67 insertions(+) > > diff --git a/arch/x86/include/asm/fsgsbase.h b/arch/x86/include/asm/fsgsbase.h > index e500d771155f..0c2d7d8a8c01 100644 > --- a/arch/x86/include/asm/fsgsbase.h > +++ b/arch/x86/include/asm/fsgsbase.h > @@ -111,6 +111,58 @@ extern void x86_gsbase_write_cpu_inactive(unsigned long > gsbase); > MODRM 0xd0 wrgsbase_opd 1 > .endm > > +#if CONFIG_SMP > + > +/* > + * Fetch the per-CPU GSBASE value for this processor and put it in @reg. > + * We normally use %gs for accessing per-CPU data, but we are setting up > + * %gs here and obviously can not use %gs itself to access per-CPU data. > + */ > +.macro FIND_PERCPU_BASE_RDPID reg:req > + /* > +* The CPU/node NR is initialized earlier, directly in cpu_init(). P > +*/ > + RDPID \reg I would suggest that you instead add a macro LOAD_CPU_AND_NODE \reg and have that macro contain the alternative. It can switch between RDPID and LSL. This way you avoid duplicating the rest of it. This should end up in the same header as __getcpu() -- it probably makes sense to just move __getcpu() for this purpose. Also, hpa and Nadav, shouldn't asm/inst.h end up in macros.S? --Andy
Re: [GIT PULL] cgroup changes for v4.20-rc1
On Thu, Oct 25, 2018 at 7:40 AM Tejun Heo wrote: > > All trivial changes - simplification, typo fix and adding > cond_resched() in a netclassid update loop. Thanks, pulled, Linus
Re: [GIT PULL] cgroup changes for v4.20-rc1
On Thu, Oct 25, 2018 at 7:40 AM Tejun Heo wrote: > > All trivial changes - simplification, typo fix and adding > cond_resched() in a netclassid update loop. Thanks, pulled, Linus
Re: [GIT PULL] printk for 4.20
On Wed, Oct 24, 2018 at 6:48 AM Petr Mladek wrote: > > please pull the latest printk changes [...] Pulled, Linus
Re: [GIT PULL] printk for 4.20
On Wed, Oct 24, 2018 at 6:48 AM Petr Mladek wrote: > > please pull the latest printk changes [...] Pulled, Linus
RE: [PATCH] spi: uniphier: fix incorrect property items
Hello Rob,
> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Friday, October 26, 2018 4:59 AM
> To: Hayashibara, Keiji/林原 啓二
> Cc: broo...@kernel.org; mark.rutl...@arm.com; Yamada, Masahiro/山田 真弘
> ;
> linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> devicet...@vger.kernel.org;
> sta...@vger.kernel.org; masami.hirama...@linaro.org;
> jaswinder.si...@linaro.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] spi: uniphier: fix incorrect property items
>
> On Wed, Oct 24, 2018 at 06:34:29PM +0900, Keiji Hayashibara wrote:
> > This commit fixes incorrect property because it was different from the
> > actual.
> > The parameters of '#address-cells' and '#size-cells' were removed, and
> > 'interrupts', 'pinctrl-names' and 'pinctrl-0' were added.
> >
> > Fixes: 4dcd5c2781f3 ("spi: add DT bindings for UniPhier SPI
> > controller")
> > Signed-off-by: Keiji Hayashibara
> > ---
> > Documentation/devicetree/bindings/spi/spi-uniphier.txt | 14
> > --
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > index 504a4ec..b04e66a 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > +++ b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > @@ -5,18 +5,20 @@ UniPhier SoCs have SCSSI which supports SPI single
> > channel.
> > Required properties:
> > - compatible: should be "socionext,uniphier-scssi"
> > - reg: address and length of the spi master registers
> > - - #address-cells: must be <1>, see spi-bus.txt
> > - - #size-cells: must be <0>, see spi-bus.txt
>
> How is removing these correct? They are needed if you have any child devices.
They are necessary when the child device is connected.
I removed it because it was specified in spi-bus.txt,
but I will leave them in next patch.
>
> > - - clocks: A phandle to the clock for the device.
> > - - resets: A phandle to the reset control for the device.
> > + - interrupts: a single interrupt specifier
> > + - pinctrl-names: should be "default"
> > + - pinctrl-0: pin control state for the default mode
> > + - clocks: a phandle to the clock for the device
> > + - resets: a phandle to the reset control for the device
> >
> > Example:
> >
> > spi0: spi@54006000 {
> > compatible = "socionext,uniphier-scssi";
> > reg = <0x54006000 0x100>;
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > + interrupts = <0 39 4>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <_spi0>;
> > clocks = <_clk 11>;
> > resets = <_rst 11>;
> > };
> > --
> > 2.7.4
> >
Thank you.
---
Best Regards,
Keiji Hayashibara
RE: [PATCH] spi: uniphier: fix incorrect property items
Hello Rob,
> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: Friday, October 26, 2018 4:59 AM
> To: Hayashibara, Keiji/林原 啓二
> Cc: broo...@kernel.org; mark.rutl...@arm.com; Yamada, Masahiro/山田 真弘
> ;
> linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> devicet...@vger.kernel.org;
> sta...@vger.kernel.org; masami.hirama...@linaro.org;
> jaswinder.si...@linaro.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] spi: uniphier: fix incorrect property items
>
> On Wed, Oct 24, 2018 at 06:34:29PM +0900, Keiji Hayashibara wrote:
> > This commit fixes incorrect property because it was different from the
> > actual.
> > The parameters of '#address-cells' and '#size-cells' were removed, and
> > 'interrupts', 'pinctrl-names' and 'pinctrl-0' were added.
> >
> > Fixes: 4dcd5c2781f3 ("spi: add DT bindings for UniPhier SPI
> > controller")
> > Signed-off-by: Keiji Hayashibara
> > ---
> > Documentation/devicetree/bindings/spi/spi-uniphier.txt | 14
> > --
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > index 504a4ec..b04e66a 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > +++ b/Documentation/devicetree/bindings/spi/spi-uniphier.txt
> > @@ -5,18 +5,20 @@ UniPhier SoCs have SCSSI which supports SPI single
> > channel.
> > Required properties:
> > - compatible: should be "socionext,uniphier-scssi"
> > - reg: address and length of the spi master registers
> > - - #address-cells: must be <1>, see spi-bus.txt
> > - - #size-cells: must be <0>, see spi-bus.txt
>
> How is removing these correct? They are needed if you have any child devices.
They are necessary when the child device is connected.
I removed it because it was specified in spi-bus.txt,
but I will leave them in next patch.
>
> > - - clocks: A phandle to the clock for the device.
> > - - resets: A phandle to the reset control for the device.
> > + - interrupts: a single interrupt specifier
> > + - pinctrl-names: should be "default"
> > + - pinctrl-0: pin control state for the default mode
> > + - clocks: a phandle to the clock for the device
> > + - resets: a phandle to the reset control for the device
> >
> > Example:
> >
> > spi0: spi@54006000 {
> > compatible = "socionext,uniphier-scssi";
> > reg = <0x54006000 0x100>;
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > + interrupts = <0 39 4>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <_spi0>;
> > clocks = <_clk 11>;
> > resets = <_rst 11>;
> > };
> > --
> > 2.7.4
> >
Thank you.
---
Best Regards,
Keiji Hayashibara
Re: [v3 03/12] x86/fsgsbase/64: Add intrinsics/macros for FSGSBASE instructions
On Thu, Oct 25, 2018 at 4:31 PM Linus Torvalds wrote: > > On Thu, Oct 25, 2018 at 4:14 PM Andy Lutomirski wrote: > > > > On Wed, Oct 24, 2018 at 12:21 PM Andi Kleen wrote: > > > > > > BTW the other option would be to update the min-binutils requirement > > > to 2.21 (currently it is 2.20) and then write it directly without .byte. > > > I believe 2.21 added support for these instructions. > > > > > > (It's only a binutils requirement, don't need gcc support) > > > > I'd personally be fine with this. Linus? Thomas? Ingo? > > I always vote for "require modern tools" as long as it doesn't cause problems. > > binutils-2.21 is something like seven years old by now, but the real > issue would be what versions distros are actually shipping. I don't > want people to have to build their own binutils just to build a > kernel. > > It's usually some ancient enterprise distro that is stuck on old > versions. Anybody have any idea? > With some basic Googling: CentOS 6 is binutils 2.23. CentOS 5 is EOL. RHEL 5 has "extended life", which means that it's officially zombified and paying customers can still download (unsupported) packages. SLES 11 is binutils 2.19, which is already unsupported. SLES 12 is 2.24. So I would guess we're okay and we can bump the requirement to 2.21.
Re: [v3 03/12] x86/fsgsbase/64: Add intrinsics/macros for FSGSBASE instructions
On Thu, Oct 25, 2018 at 4:31 PM Linus Torvalds wrote: > > On Thu, Oct 25, 2018 at 4:14 PM Andy Lutomirski wrote: > > > > On Wed, Oct 24, 2018 at 12:21 PM Andi Kleen wrote: > > > > > > BTW the other option would be to update the min-binutils requirement > > > to 2.21 (currently it is 2.20) and then write it directly without .byte. > > > I believe 2.21 added support for these instructions. > > > > > > (It's only a binutils requirement, don't need gcc support) > > > > I'd personally be fine with this. Linus? Thomas? Ingo? > > I always vote for "require modern tools" as long as it doesn't cause problems. > > binutils-2.21 is something like seven years old by now, but the real > issue would be what versions distros are actually shipping. I don't > want people to have to build their own binutils just to build a > kernel. > > It's usually some ancient enterprise distro that is stuck on old > versions. Anybody have any idea? > With some basic Googling: CentOS 6 is binutils 2.23. CentOS 5 is EOL. RHEL 5 has "extended life", which means that it's officially zombified and paying customers can still download (unsupported) packages. SLES 11 is binutils 2.19, which is already unsupported. SLES 12 is 2.24. So I would guess we're okay and we can bump the requirement to 2.21.
Re: [PATCH v2] x86/microcode: Handle negative microcode revisions
On Sat, Oct 20, 2018 at 07:41:36PM +0200, Borislav Petkov wrote:
> Dropping stable.
>
> On Sat, Oct 20, 2018 at 07:41:58AM -0700, Andi Kleen wrote:
> > From: Andi Kleen
> >
> > The Intel microcode revision space is unsigned. Inside Intel there are
> > special
> > microcodes that have the highest bit set, and they are considered to have
> > a higher revision than any microcodes that don't have this bit set.
> >
> > The function comparing the microcode revision in the Linux driver compares
> > u32 with int, which ends up being signed extended to long on 64bit
> > systems. This results in these highest bit set microcode revision not
> > loading
> > because their revision appears negative and smaller than the
> > existing microcode.
> >
> > Change the comparison to unsigned. With that the loading works
> > as expected.
> >
> > Cc: sta...@vger.kernel.org # Any supported stable
> > Signed-off-by: Andi Kleen
> > --
> > v2: White space changes.
> > ---
> > arch/x86/kernel/cpu/microcode/intel.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/cpu/microcode/intel.c
> > b/arch/x86/kernel/cpu/microcode/intel.c
> > index 16936a24795c..e54d402500d3 100644
> > --- a/arch/x86/kernel/cpu/microcode/intel.c
> > +++ b/arch/x86/kernel/cpu/microcode/intel.c
> > @@ -93,7 +93,8 @@ static int find_matching_signature(void *mc, unsigned int
> > csig, int cpf)
> > /*
> > * Returns 1 if update has been found, 0 otherwise.
> > */
> > -static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int
> > new_rev)
> > +static int has_newer_microcode(void *mc, unsigned int csig, int cpf,
> > + unsigned new_rev)
> > {
> > struct microcode_header_intel *mc_hdr = mc;
> >
> > --
>
> Please incorporate all review comments before sending a new version of
> your patch.
I replaced one more microcodes with microcodes revisions if that is
what you meant.
-Andi
[PATCH v4 3/5] hwmon: (ina3221) Serialize sysfs ABI accesses
This change adds a mutex to serialize accesses of sysfs attributes.
This is required when polling CVRF bit of the MASK/ENABLE register
because this bit is cleared on a read of this MASK/ENABLE register
or a write to CONFIG register, which means that this bit might be
accidentally cleared by reading other fields like alert flags.
So this patch adds a mutex lock to protect the write() and read()
callbacks. The read_string() callback won't need the lock since it
just returns the label without touching any hardware register.
Signed-off-by: Nicolin Chen
---
Changlog
v1->v4:
* N/A
drivers/hwmon/ina3221.c | 51 -
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index 26cdf3342d80..10e8347a3c80 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -94,12 +95,14 @@ struct ina3221_input {
* @regmap: Register map of the device
* @fields: Register fields of the device
* @inputs: Array of channel input source specific structures
+ * @lock: mutex lock to serialize sysfs attribute accesses
* @reg_config: Register value of INA3221_CONFIG
*/
struct ina3221_data {
struct regmap *regmap;
struct regmap_field *fields[F_MAX_FIELDS];
struct ina3221_input inputs[INA3221_NUM_CHANNELS];
+ struct mutex lock;
u32 reg_config;
};
@@ -265,29 +268,53 @@ static int ina3221_write_enable(struct device *dev, int
channel, bool enable)
static int ina3221_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
+ struct ina3221_data *ina = dev_get_drvdata(dev);
+ int ret;
+
+ mutex_lock(>lock);
+
switch (type) {
case hwmon_in:
/* 0-align channel ID */
- return ina3221_read_in(dev, attr, channel - 1, val);
+ ret = ina3221_read_in(dev, attr, channel - 1, val);
+ break;
case hwmon_curr:
- return ina3221_read_curr(dev, attr, channel, val);
+ ret = ina3221_read_curr(dev, attr, channel, val);
+ break;
default:
- return -EOPNOTSUPP;
+ ret = -EOPNOTSUPP;
+ break;
}
+
+ mutex_unlock(>lock);
+
+ return ret;
}
static int ina3221_write(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long val)
{
+ struct ina3221_data *ina = dev_get_drvdata(dev);
+ int ret;
+
+ mutex_lock(>lock);
+
switch (type) {
case hwmon_in:
/* 0-align channel ID */
- return ina3221_write_enable(dev, channel - 1, val);
+ ret = ina3221_write_enable(dev, channel - 1, val);
+ break;
case hwmon_curr:
- return ina3221_write_curr(dev, attr, channel, val);
+ ret = ina3221_write_curr(dev, attr, channel, val);
+ break;
default:
- return -EOPNOTSUPP;
+ ret = -EOPNOTSUPP;
+ break;
}
+
+ mutex_unlock(>lock);
+
+ return ret;
}
static int ina3221_read_string(struct device *dev, enum hwmon_sensor_types
type,
@@ -582,6 +609,7 @@ static int ina3221_probe(struct i2c_client *client,
if (ret)
return ret;
+ mutex_init(>lock);
dev_set_drvdata(dev, ina);
hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, ina,
@@ -589,12 +617,22 @@ static int ina3221_probe(struct i2c_client *client,
ina3221_groups);
if (IS_ERR(hwmon_dev)) {
dev_err(dev, "Unable to register hwmon device\n");
+ mutex_destroy(>lock);
return PTR_ERR(hwmon_dev);
}
return 0;
}
+static int ina3221_remove(struct i2c_client *client)
+{
+ struct ina3221_data *ina = dev_get_drvdata(>dev);
+
+ mutex_destroy(>lock);
+
+ return 0;
+}
+
static int __maybe_unused ina3221_suspend(struct device *dev)
{
struct ina3221_data *ina = dev_get_drvdata(dev);
@@ -663,6 +701,7 @@ MODULE_DEVICE_TABLE(i2c, ina3221_ids);
static struct i2c_driver ina3221_i2c_driver = {
.probe = ina3221_probe,
+ .remove = ina3221_remove,
.driver = {
.name = INA3221_DRIVER_NAME,
.of_match_table = ina3221_of_match_table,
--
2.17.1
[PATCH v4 2/5] hwmon: (ina3221) Check channel status for alarms attribute read
There is nothing critically wrong to read these two attributes
without having a is_enabled() check at this point. But reading
the MASK_ENABLE register would clear the CVRF bit according to
the datasheet. So it'd be safer to fence for disabled channels
in order to add pm runtime feature.
Signed-off-by: Nicolin Chen
---
Changelog
v2->v4:
* N/A
v1->v2:
* Returned 0 for alert flags instead of -ENODATA
drivers/hwmon/ina3221.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index d61688f04594..26cdf3342d80 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -200,6 +200,12 @@ static int ina3221_read_curr(struct device *dev, u32 attr,
return 0;
case hwmon_curr_crit_alarm:
case hwmon_curr_max_alarm:
+ /* No actual register read if channel is disabled */
+ if (!ina3221_is_enabled(ina, channel)) {
+ /* Return 0 for alert flags */
+ *val = 0;
+ return 0;
+ }
ret = regmap_field_read(ina->fields[reg], );
if (ret)
return ret;
--
2.17.1
Re: [PATCH v2] x86/microcode: Handle negative microcode revisions
On Sat, Oct 20, 2018 at 07:41:36PM +0200, Borislav Petkov wrote:
> Dropping stable.
>
> On Sat, Oct 20, 2018 at 07:41:58AM -0700, Andi Kleen wrote:
> > From: Andi Kleen
> >
> > The Intel microcode revision space is unsigned. Inside Intel there are
> > special
> > microcodes that have the highest bit set, and they are considered to have
> > a higher revision than any microcodes that don't have this bit set.
> >
> > The function comparing the microcode revision in the Linux driver compares
> > u32 with int, which ends up being signed extended to long on 64bit
> > systems. This results in these highest bit set microcode revision not
> > loading
> > because their revision appears negative and smaller than the
> > existing microcode.
> >
> > Change the comparison to unsigned. With that the loading works
> > as expected.
> >
> > Cc: sta...@vger.kernel.org # Any supported stable
> > Signed-off-by: Andi Kleen
> > --
> > v2: White space changes.
> > ---
> > arch/x86/kernel/cpu/microcode/intel.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/cpu/microcode/intel.c
> > b/arch/x86/kernel/cpu/microcode/intel.c
> > index 16936a24795c..e54d402500d3 100644
> > --- a/arch/x86/kernel/cpu/microcode/intel.c
> > +++ b/arch/x86/kernel/cpu/microcode/intel.c
> > @@ -93,7 +93,8 @@ static int find_matching_signature(void *mc, unsigned int
> > csig, int cpf)
> > /*
> > * Returns 1 if update has been found, 0 otherwise.
> > */
> > -static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int
> > new_rev)
> > +static int has_newer_microcode(void *mc, unsigned int csig, int cpf,
> > + unsigned new_rev)
> > {
> > struct microcode_header_intel *mc_hdr = mc;
> >
> > --
>
> Please incorporate all review comments before sending a new version of
> your patch.
I replaced one more microcodes with microcodes revisions if that is
what you meant.
-Andi
[PATCH v4 3/5] hwmon: (ina3221) Serialize sysfs ABI accesses
This change adds a mutex to serialize accesses of sysfs attributes.
This is required when polling CVRF bit of the MASK/ENABLE register
because this bit is cleared on a read of this MASK/ENABLE register
or a write to CONFIG register, which means that this bit might be
accidentally cleared by reading other fields like alert flags.
So this patch adds a mutex lock to protect the write() and read()
callbacks. The read_string() callback won't need the lock since it
just returns the label without touching any hardware register.
Signed-off-by: Nicolin Chen
---
Changlog
v1->v4:
* N/A
drivers/hwmon/ina3221.c | 51 -
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index 26cdf3342d80..10e8347a3c80 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -94,12 +95,14 @@ struct ina3221_input {
* @regmap: Register map of the device
* @fields: Register fields of the device
* @inputs: Array of channel input source specific structures
+ * @lock: mutex lock to serialize sysfs attribute accesses
* @reg_config: Register value of INA3221_CONFIG
*/
struct ina3221_data {
struct regmap *regmap;
struct regmap_field *fields[F_MAX_FIELDS];
struct ina3221_input inputs[INA3221_NUM_CHANNELS];
+ struct mutex lock;
u32 reg_config;
};
@@ -265,29 +268,53 @@ static int ina3221_write_enable(struct device *dev, int
channel, bool enable)
static int ina3221_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
+ struct ina3221_data *ina = dev_get_drvdata(dev);
+ int ret;
+
+ mutex_lock(>lock);
+
switch (type) {
case hwmon_in:
/* 0-align channel ID */
- return ina3221_read_in(dev, attr, channel - 1, val);
+ ret = ina3221_read_in(dev, attr, channel - 1, val);
+ break;
case hwmon_curr:
- return ina3221_read_curr(dev, attr, channel, val);
+ ret = ina3221_read_curr(dev, attr, channel, val);
+ break;
default:
- return -EOPNOTSUPP;
+ ret = -EOPNOTSUPP;
+ break;
}
+
+ mutex_unlock(>lock);
+
+ return ret;
}
static int ina3221_write(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long val)
{
+ struct ina3221_data *ina = dev_get_drvdata(dev);
+ int ret;
+
+ mutex_lock(>lock);
+
switch (type) {
case hwmon_in:
/* 0-align channel ID */
- return ina3221_write_enable(dev, channel - 1, val);
+ ret = ina3221_write_enable(dev, channel - 1, val);
+ break;
case hwmon_curr:
- return ina3221_write_curr(dev, attr, channel, val);
+ ret = ina3221_write_curr(dev, attr, channel, val);
+ break;
default:
- return -EOPNOTSUPP;
+ ret = -EOPNOTSUPP;
+ break;
}
+
+ mutex_unlock(>lock);
+
+ return ret;
}
static int ina3221_read_string(struct device *dev, enum hwmon_sensor_types
type,
@@ -582,6 +609,7 @@ static int ina3221_probe(struct i2c_client *client,
if (ret)
return ret;
+ mutex_init(>lock);
dev_set_drvdata(dev, ina);
hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, ina,
@@ -589,12 +617,22 @@ static int ina3221_probe(struct i2c_client *client,
ina3221_groups);
if (IS_ERR(hwmon_dev)) {
dev_err(dev, "Unable to register hwmon device\n");
+ mutex_destroy(>lock);
return PTR_ERR(hwmon_dev);
}
return 0;
}
+static int ina3221_remove(struct i2c_client *client)
+{
+ struct ina3221_data *ina = dev_get_drvdata(>dev);
+
+ mutex_destroy(>lock);
+
+ return 0;
+}
+
static int __maybe_unused ina3221_suspend(struct device *dev)
{
struct ina3221_data *ina = dev_get_drvdata(dev);
@@ -663,6 +701,7 @@ MODULE_DEVICE_TABLE(i2c, ina3221_ids);
static struct i2c_driver ina3221_i2c_driver = {
.probe = ina3221_probe,
+ .remove = ina3221_remove,
.driver = {
.name = INA3221_DRIVER_NAME,
.of_match_table = ina3221_of_match_table,
--
2.17.1
[PATCH v4 2/5] hwmon: (ina3221) Check channel status for alarms attribute read
There is nothing critically wrong to read these two attributes
without having a is_enabled() check at this point. But reading
the MASK_ENABLE register would clear the CVRF bit according to
the datasheet. So it'd be safer to fence for disabled channels
in order to add pm runtime feature.
Signed-off-by: Nicolin Chen
---
Changelog
v2->v4:
* N/A
v1->v2:
* Returned 0 for alert flags instead of -ENODATA
drivers/hwmon/ina3221.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index d61688f04594..26cdf3342d80 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -200,6 +200,12 @@ static int ina3221_read_curr(struct device *dev, u32 attr,
return 0;
case hwmon_curr_crit_alarm:
case hwmon_curr_max_alarm:
+ /* No actual register read if channel is disabled */
+ if (!ina3221_is_enabled(ina, channel)) {
+ /* Return 0 for alert flags */
+ *val = 0;
+ return 0;
+ }
ret = regmap_field_read(ina->fields[reg], );
if (ret)
return ret;
--
2.17.1
[PATCH v4 2/2] perf/x86/kvm: Avoid unnecessary work in guest filtering
From: Andi Kleen
KVM added a workaround for PEBS events leaking
into guests with 26a4f3c08de4 ("perf/x86: disable PEBS on a guest entry.")
This uses the VT entry/exit list to add an extra disable of the PEBS_ENABLE MSR.
Intel also added a fix for this issue to microcode updates on
Haswell/Broadwell/Skylake.
It turns out using the MSR entry/exit list makes VM exits
significantly slower. The list is only needed for disabling
PEBS, because the GLOBAL_CTRL change gets optimized by
KVM into changing the VMCS.
Check for the microcode updates that have the microcode
fix for leaking PEBS, and disable the extra entry/exit list
entry for PEBS_ENABLE. In addition we always clear the
GLOBAL_CTRL for the PEBS counter while running in the guest,
which is enough to make them never fire at the wrong
side of the host/guest transition.
We see significantly reduced overhead for VM exits with the
filtering active with the patch from 8% to 4%.
Signed-off-by: Andi Kleen
---
v2:
Use match_ucode, not match_ucode_all
Remove cpu lock
Use INTEL_MIN_UCODE and move to header
Update Table to include skylake clients.
v3:
Use x86_min_microcode
---
arch/x86/events/intel/core.c | 80
arch/x86/events/perf_event.h | 3 +-
2 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 0fb8659b20d8..89ec85c3359c 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include "../perf_event.h"
@@ -3170,16 +3171,27 @@ static struct perf_guest_switch_msr
*intel_guest_get_msrs(int *nr)
arr[0].msr = MSR_CORE_PERF_GLOBAL_CTRL;
arr[0].host = x86_pmu.intel_ctrl & ~cpuc->intel_ctrl_guest_mask;
arr[0].guest = x86_pmu.intel_ctrl & ~cpuc->intel_ctrl_host_mask;
- /*
-* If PMU counter has PEBS enabled it is not enough to disable counter
-* on a guest entry since PEBS memory write can overshoot guest entry
-* and corrupt guest memory. Disabling PEBS solves the problem.
-*/
- arr[1].msr = MSR_IA32_PEBS_ENABLE;
- arr[1].host = cpuc->pebs_enabled;
- arr[1].guest = 0;
+ if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+ arr[0].guest &= ~cpuc->pebs_enabled;
+ else
+ arr[0].guest &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+ *nr = 1;
+
+ if (!x86_pmu.pebs_isolated) {
+ /*
+* If PMU counter has PEBS enabled it is not enough to
+* disable counter on a guest entry since PEBS memory
+* write can overshoot guest entry and corrupt guest
+* memory. Disabling PEBS solves the problem.
+*
+* Don't do this if the CPU already enforces it.
+*/
+ arr[1].msr = MSR_IA32_PEBS_ENABLE;
+ arr[1].host = cpuc->pebs_enabled;
+ arr[1].guest = 0;
+ *nr = 2;
+ }
- *nr = 2;
return arr;
}
@@ -3697,6 +3709,45 @@ static __init void intel_clovertown_quirk(void)
x86_pmu.pebs_constraints = NULL;
}
+static const struct x86_ucode_id isolation_ucodes[] = {
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_CORE, 3, 0x001f),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_ULT, 1, 0x001e),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_GT3E, 1, 0x0015),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_X,2, 0x0037),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_X,4, 0x000a),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_CORE, 4, 0x0023),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_GT3E, 1, 0x0014),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 2, 0x0010),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 3, 0x0709),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 4, 0x0f09),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 5, 0x0e02),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_X, 2, 0x0b14),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_X,3, 0x0021),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_X,4, 0x),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_MOBILE, 3, 0x007c),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_DESKTOP, 3, 0x007c),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP, 9, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 9, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 10, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 11, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 12, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP,10, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP,11, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP,12,
[PATCH v4 1/2] x86/cpufeature: Add facility to match microcode revisions
From: Andi Kleen
For bug workarounds or checks it is useful to check for specific
microcode revisionss. Add a new table format to check for steppings
with min microcode revisions.
This does not change the existing x86_cpu_id because it's an ABI
shared with modutils, and also has quite different requirements,
as in no wildcards, but everything has to be matched exactly.
Signed-off-by: Andi Kleen
---
v2:
Remove all CPU match, only check boot cpu
Move INTEL_MIN_UCODE macro to header.
Minor cleanups.
Remove max ucode and driver data
v3:
Rename function
Update comments.
Document mixed stepping caveats.
Use u8 for model
Remove vendor 0 check.
Change return check
v4:
Rename to x86_min_microcode
Change return value to bool
---
arch/x86/include/asm/cpu_device_id.h | 28
arch/x86/kernel/cpu/match.c | 19 +++
2 files changed, 47 insertions(+)
diff --git a/arch/x86/include/asm/cpu_device_id.h
b/arch/x86/include/asm/cpu_device_id.h
index baeba0567126..28847d5ea1fa 100644
--- a/arch/x86/include/asm/cpu_device_id.h
+++ b/arch/x86/include/asm/cpu_device_id.h
@@ -11,4 +11,32 @@
extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
+/*
+ * Match specific microcode revisions.
+ *
+ * vendor/family/model/stepping must be all set.
+ *
+ * only checks against the boot cpu. When mixed-stepping configs are
+ * valid for a CPU model, add a quirk for every valid stepping and
+ * do the fine-tuning in the quirk handler.
+ */
+
+struct x86_ucode_id {
+ u8 vendor;
+ u8 family;
+ u8 model;
+ u8 stepping;
+ u32 min_ucode;
+};
+
+#define INTEL_MIN_UCODE(mod, step, rev) { \
+ .vendor = X86_VENDOR_INTEL, \
+ .family = 6,\
+ .model = mod, \
+ .stepping = step, \
+ .min_ucode = rev, \
+}
+
+extern bool x86_min_microcode(const struct x86_ucode_id *mt);
+
#endif
diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
index 3fed38812eea..12db14232d62 100644
--- a/arch/x86/kernel/cpu/match.c
+++ b/arch/x86/kernel/cpu/match.c
@@ -48,3 +48,22 @@ const struct x86_cpu_id *x86_match_cpu(const struct
x86_cpu_id *match)
return NULL;
}
EXPORT_SYMBOL(x86_match_cpu);
+
+bool x86_min_microcode(const struct x86_ucode_id *mt)
+{
+ struct cpuinfo_x86 *c = _cpu_data;
+ const struct x86_ucode_id *m;
+
+ for (m = mt; m->family | m->model; m++) {
+ if (c->x86_vendor != m->vendor)
+ continue;
+ if (c->x86 != m->family)
+ continue;
+ if (c->x86_model != m->model)
+ continue;
+ if (c->x86_stepping != m->stepping)
+ continue;
+ return c->microcode >= m->min_ucode;
+ }
+ return false;
+}
--
2.17.2
[PATCH v4 2/2] perf/x86/kvm: Avoid unnecessary work in guest filtering
From: Andi Kleen
KVM added a workaround for PEBS events leaking
into guests with 26a4f3c08de4 ("perf/x86: disable PEBS on a guest entry.")
This uses the VT entry/exit list to add an extra disable of the PEBS_ENABLE MSR.
Intel also added a fix for this issue to microcode updates on
Haswell/Broadwell/Skylake.
It turns out using the MSR entry/exit list makes VM exits
significantly slower. The list is only needed for disabling
PEBS, because the GLOBAL_CTRL change gets optimized by
KVM into changing the VMCS.
Check for the microcode updates that have the microcode
fix for leaking PEBS, and disable the extra entry/exit list
entry for PEBS_ENABLE. In addition we always clear the
GLOBAL_CTRL for the PEBS counter while running in the guest,
which is enough to make them never fire at the wrong
side of the host/guest transition.
We see significantly reduced overhead for VM exits with the
filtering active with the patch from 8% to 4%.
Signed-off-by: Andi Kleen
---
v2:
Use match_ucode, not match_ucode_all
Remove cpu lock
Use INTEL_MIN_UCODE and move to header
Update Table to include skylake clients.
v3:
Use x86_min_microcode
---
arch/x86/events/intel/core.c | 80
arch/x86/events/perf_event.h | 3 +-
2 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 0fb8659b20d8..89ec85c3359c 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include "../perf_event.h"
@@ -3170,16 +3171,27 @@ static struct perf_guest_switch_msr
*intel_guest_get_msrs(int *nr)
arr[0].msr = MSR_CORE_PERF_GLOBAL_CTRL;
arr[0].host = x86_pmu.intel_ctrl & ~cpuc->intel_ctrl_guest_mask;
arr[0].guest = x86_pmu.intel_ctrl & ~cpuc->intel_ctrl_host_mask;
- /*
-* If PMU counter has PEBS enabled it is not enough to disable counter
-* on a guest entry since PEBS memory write can overshoot guest entry
-* and corrupt guest memory. Disabling PEBS solves the problem.
-*/
- arr[1].msr = MSR_IA32_PEBS_ENABLE;
- arr[1].host = cpuc->pebs_enabled;
- arr[1].guest = 0;
+ if (x86_pmu.flags & PMU_FL_PEBS_ALL)
+ arr[0].guest &= ~cpuc->pebs_enabled;
+ else
+ arr[0].guest &= ~(cpuc->pebs_enabled & PEBS_COUNTER_MASK);
+ *nr = 1;
+
+ if (!x86_pmu.pebs_isolated) {
+ /*
+* If PMU counter has PEBS enabled it is not enough to
+* disable counter on a guest entry since PEBS memory
+* write can overshoot guest entry and corrupt guest
+* memory. Disabling PEBS solves the problem.
+*
+* Don't do this if the CPU already enforces it.
+*/
+ arr[1].msr = MSR_IA32_PEBS_ENABLE;
+ arr[1].host = cpuc->pebs_enabled;
+ arr[1].guest = 0;
+ *nr = 2;
+ }
- *nr = 2;
return arr;
}
@@ -3697,6 +3709,45 @@ static __init void intel_clovertown_quirk(void)
x86_pmu.pebs_constraints = NULL;
}
+static const struct x86_ucode_id isolation_ucodes[] = {
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_CORE, 3, 0x001f),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_ULT, 1, 0x001e),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_GT3E, 1, 0x0015),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_X,2, 0x0037),
+ INTEL_MIN_UCODE(INTEL_FAM6_HASWELL_X,4, 0x000a),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_CORE, 4, 0x0023),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_GT3E, 1, 0x0014),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 2, 0x0010),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 3, 0x0709),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 4, 0x0f09),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_XEON_D, 5, 0x0e02),
+ INTEL_MIN_UCODE(INTEL_FAM6_BROADWELL_X, 2, 0x0b14),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_X,3, 0x0021),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_X,4, 0x),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_MOBILE, 3, 0x007c),
+ INTEL_MIN_UCODE(INTEL_FAM6_SKYLAKE_DESKTOP, 3, 0x007c),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP, 9, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 9, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 10, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 11, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_MOBILE, 12, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP,10, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP,11, 0x004e),
+ INTEL_MIN_UCODE(INTEL_FAM6_KABYLAKE_DESKTOP,12,
[PATCH v4 1/2] x86/cpufeature: Add facility to match microcode revisions
From: Andi Kleen
For bug workarounds or checks it is useful to check for specific
microcode revisionss. Add a new table format to check for steppings
with min microcode revisions.
This does not change the existing x86_cpu_id because it's an ABI
shared with modutils, and also has quite different requirements,
as in no wildcards, but everything has to be matched exactly.
Signed-off-by: Andi Kleen
---
v2:
Remove all CPU match, only check boot cpu
Move INTEL_MIN_UCODE macro to header.
Minor cleanups.
Remove max ucode and driver data
v3:
Rename function
Update comments.
Document mixed stepping caveats.
Use u8 for model
Remove vendor 0 check.
Change return check
v4:
Rename to x86_min_microcode
Change return value to bool
---
arch/x86/include/asm/cpu_device_id.h | 28
arch/x86/kernel/cpu/match.c | 19 +++
2 files changed, 47 insertions(+)
diff --git a/arch/x86/include/asm/cpu_device_id.h
b/arch/x86/include/asm/cpu_device_id.h
index baeba0567126..28847d5ea1fa 100644
--- a/arch/x86/include/asm/cpu_device_id.h
+++ b/arch/x86/include/asm/cpu_device_id.h
@@ -11,4 +11,32 @@
extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
+/*
+ * Match specific microcode revisions.
+ *
+ * vendor/family/model/stepping must be all set.
+ *
+ * only checks against the boot cpu. When mixed-stepping configs are
+ * valid for a CPU model, add a quirk for every valid stepping and
+ * do the fine-tuning in the quirk handler.
+ */
+
+struct x86_ucode_id {
+ u8 vendor;
+ u8 family;
+ u8 model;
+ u8 stepping;
+ u32 min_ucode;
+};
+
+#define INTEL_MIN_UCODE(mod, step, rev) { \
+ .vendor = X86_VENDOR_INTEL, \
+ .family = 6,\
+ .model = mod, \
+ .stepping = step, \
+ .min_ucode = rev, \
+}
+
+extern bool x86_min_microcode(const struct x86_ucode_id *mt);
+
#endif
diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
index 3fed38812eea..12db14232d62 100644
--- a/arch/x86/kernel/cpu/match.c
+++ b/arch/x86/kernel/cpu/match.c
@@ -48,3 +48,22 @@ const struct x86_cpu_id *x86_match_cpu(const struct
x86_cpu_id *match)
return NULL;
}
EXPORT_SYMBOL(x86_match_cpu);
+
+bool x86_min_microcode(const struct x86_ucode_id *mt)
+{
+ struct cpuinfo_x86 *c = _cpu_data;
+ const struct x86_ucode_id *m;
+
+ for (m = mt; m->family | m->model; m++) {
+ if (c->x86_vendor != m->vendor)
+ continue;
+ if (c->x86 != m->family)
+ continue;
+ if (c->x86_model != m->model)
+ continue;
+ if (c->x86_stepping != m->stepping)
+ continue;
+ return c->microcode >= m->min_ucode;
+ }
+ return false;
+}
--
2.17.2
Re: The linux devs can rescind their license grant.
El jue, 25-10-2018 a las 16:47 -0400, Theodore Y. Ts'o escribió: > On Thu, Oct 25, 2018 at 03:39:01PM -0400, Eric S. Raymond wrote: > > > > Under Jacobsen vs. Katzer (535 f 3d 1373 fed cir 2008) authors of > > GPLed software have a specific right to relief (including > > injunctive > > relief) against misappropriation of their software. That ruling > > (which > > was the case of first impression on the binding status of the GPL) > > reputational damage is *specifically* recognized as grounds for > > relief. > > I've read the legal briefs, and I'm pretty sure they don't say what > you are claiming they say. Yes, I'm not a lawyer --- but that's OK > --- neither are you. > > > The anti-CoC dissidents don't have to rescind their license grant > > to > > cause a great deal of trouble. > > The *vast* majority of the "anti-CoC dissidents" who have been > advancing this argument, have, as near as I can tell, little or no > copyright ownership in the kernel. They are external trolls who are > not members of the kernel development community, to the best of my > belief and knowledge. I understand the point, but being an outsider from the kernel development community, does not mean i should not care about Linux development and this particular community. I have been promoting, using, installing, testing and loving Linux for more than 20 years now. One of the things that got me into it and still appreciate a lot is the meritocracy: the norm of technical excellence, the: "it does not matter who you are as long as you contribute good code". This CoC "movement" is part of an anti meritocracy "movement" that should not be influencing a project that thanks to this meritocracy has become the most important in the world. I'll take one of the arguments of the CoC promoters and use it on my favor: The Linux community has grown more than just a bunch of developers it has: promoters, users, testers, enthusiast, companies, foundations, etc... I wouldn't like that a negative code of conduct (yes its languaje is aggresive and charged with negativity, politics and victimization) compromise the technical aspects of a critical system like the Linux kernel. When it comes to coding, Engineers should be above that stuff. > > In short; they are adding noise to the conversation, and have been > presuming that in fact people are going to be regularly and summarily > ejected from the community. In short, they are adding FUD, probably > because they have their own agenda. I have one agenda: World Domination :P > I would recommend that before people respond such provocation > messages, that they do a quick "git log" and find out whether they > have in fact contributed code to the kernel at all, and if so, how > long ago it was. Just want to clarify that this is not a provocation, just got tired of being a passive member of this discussion. You're right the people with more merit in this list should have a more important voice since it shows the commitment to the development. BTW Thanks for the comment, i will start contributing to the extent of my capacities. As i say, the reach of Linux is broader than just the development community. Too bad you can't git log all the servers that people has installed and maintaining during the years. > Regards, > > - Ted Cheers, Ivan
Re: The linux devs can rescind their license grant.
El jue, 25-10-2018 a las 16:47 -0400, Theodore Y. Ts'o escribió: > On Thu, Oct 25, 2018 at 03:39:01PM -0400, Eric S. Raymond wrote: > > > > Under Jacobsen vs. Katzer (535 f 3d 1373 fed cir 2008) authors of > > GPLed software have a specific right to relief (including > > injunctive > > relief) against misappropriation of their software. That ruling > > (which > > was the case of first impression on the binding status of the GPL) > > reputational damage is *specifically* recognized as grounds for > > relief. > > I've read the legal briefs, and I'm pretty sure they don't say what > you are claiming they say. Yes, I'm not a lawyer --- but that's OK > --- neither are you. > > > The anti-CoC dissidents don't have to rescind their license grant > > to > > cause a great deal of trouble. > > The *vast* majority of the "anti-CoC dissidents" who have been > advancing this argument, have, as near as I can tell, little or no > copyright ownership in the kernel. They are external trolls who are > not members of the kernel development community, to the best of my > belief and knowledge. I understand the point, but being an outsider from the kernel development community, does not mean i should not care about Linux development and this particular community. I have been promoting, using, installing, testing and loving Linux for more than 20 years now. One of the things that got me into it and still appreciate a lot is the meritocracy: the norm of technical excellence, the: "it does not matter who you are as long as you contribute good code". This CoC "movement" is part of an anti meritocracy "movement" that should not be influencing a project that thanks to this meritocracy has become the most important in the world. I'll take one of the arguments of the CoC promoters and use it on my favor: The Linux community has grown more than just a bunch of developers it has: promoters, users, testers, enthusiast, companies, foundations, etc... I wouldn't like that a negative code of conduct (yes its languaje is aggresive and charged with negativity, politics and victimization) compromise the technical aspects of a critical system like the Linux kernel. When it comes to coding, Engineers should be above that stuff. > > In short; they are adding noise to the conversation, and have been > presuming that in fact people are going to be regularly and summarily > ejected from the community. In short, they are adding FUD, probably > because they have their own agenda. I have one agenda: World Domination :P > I would recommend that before people respond such provocation > messages, that they do a quick "git log" and find out whether they > have in fact contributed code to the kernel at all, and if so, how > long ago it was. Just want to clarify that this is not a provocation, just got tired of being a passive member of this discussion. You're right the people with more merit in this list should have a more important voice since it shows the commitment to the development. BTW Thanks for the comment, i will start contributing to the extent of my capacities. As i say, the reach of Linux is broader than just the development community. Too bad you can't git log all the servers that people has installed and maintaining during the years. > Regards, > > - Ted Cheers, Ivan
Re: [v3 03/12] x86/fsgsbase/64: Add intrinsics/macros for FSGSBASE instructions
On Thu, Oct 25, 2018 at 4:14 PM Andy Lutomirski wrote: > > On Wed, Oct 24, 2018 at 12:21 PM Andi Kleen wrote: > > > > BTW the other option would be to update the min-binutils requirement > > to 2.21 (currently it is 2.20) and then write it directly without .byte. > > I believe 2.21 added support for these instructions. > > > > (It's only a binutils requirement, don't need gcc support) > > I'd personally be fine with this. Linus? Thomas? Ingo? I always vote for "require modern tools" as long as it doesn't cause problems. binutils-2.21 is something like seven years old by now, but the real issue would be what versions distros are actually shipping. I don't want people to have to build their own binutils just to build a kernel. It's usually some ancient enterprise distro that is stuck on old versions. Anybody have any idea? Linus
Re: [v3 03/12] x86/fsgsbase/64: Add intrinsics/macros for FSGSBASE instructions
On Thu, Oct 25, 2018 at 4:14 PM Andy Lutomirski wrote: > > On Wed, Oct 24, 2018 at 12:21 PM Andi Kleen wrote: > > > > BTW the other option would be to update the min-binutils requirement > > to 2.21 (currently it is 2.20) and then write it directly without .byte. > > I believe 2.21 added support for these instructions. > > > > (It's only a binutils requirement, don't need gcc support) > > I'd personally be fine with this. Linus? Thomas? Ingo? I always vote for "require modern tools" as long as it doesn't cause problems. binutils-2.21 is something like seven years old by now, but the real issue would be what versions distros are actually shipping. I don't want people to have to build their own binutils just to build a kernel. It's usually some ancient enterprise distro that is stuck on old versions. Anybody have any idea? Linus
Re: [PATCH v2 1/2] x86/cpufeature: Add facility to match microcode revisions
On Sun, Oct 21, 2018 at 12:20:47PM +0200, Thomas Gleixner wrote: > Andi, > > On Sat, 20 Oct 2018, Andi Kleen wrote: > > On Sat, Oct 20, 2018 at 10:19:37AM +0200, Thomas Gleixner wrote: > > > On Fri, 19 Oct 2018, Andi Kleen wrote: > > > There is no point to return the pointer because it's not a compound > > > structure. If you want to provide the possibility to use the index then > > > return the index and an error code if it does not match. > > > > It will be useful with the driver_data pointer, which you short sightedly > > forced me to remove, and likely will need to be readded at some point > > anyways if this gets more widely used. > > It's good and established practice not to add functionality on a 'might be > used' basis. If you'd provide at least one or two patches which demonstrate > how that is useful then that would be convincing. > > > At least with the pointer not all callers will need to be changed then. > > It doesn't need to be changed at all, when done correctly. Thanks. I opted for the simpler method of returning a boolean now. -Andi
Re: [PATCH v2 1/2] x86/cpufeature: Add facility to match microcode revisions
On Sun, Oct 21, 2018 at 12:20:47PM +0200, Thomas Gleixner wrote: > Andi, > > On Sat, 20 Oct 2018, Andi Kleen wrote: > > On Sat, Oct 20, 2018 at 10:19:37AM +0200, Thomas Gleixner wrote: > > > On Fri, 19 Oct 2018, Andi Kleen wrote: > > > There is no point to return the pointer because it's not a compound > > > structure. If you want to provide the possibility to use the index then > > > return the index and an error code if it does not match. > > > > It will be useful with the driver_data pointer, which you short sightedly > > forced me to remove, and likely will need to be readded at some point > > anyways if this gets more widely used. > > It's good and established practice not to add functionality on a 'might be > used' basis. If you'd provide at least one or two patches which demonstrate > how that is useful then that would be convincing. > > > At least with the pointer not all callers will need to be changed then. > > It doesn't need to be changed at all, when done correctly. Thanks. I opted for the simpler method of returning a boolean now. -Andi
Re: [v3 03/12] x86/fsgsbase/64: Add intrinsics/macros for FSGSBASE instructions
On Wed, Oct 24, 2018 at 12:21 PM Andi Kleen wrote: > > On Wed, Oct 24, 2018 at 11:53:54AM -0700, Andy Lutomirski wrote: > > On Tue, Oct 23, 2018 at 11:43 AM Chang S. Bae > > wrote: > > > > > > From: Andi Kleen > > > > > > Add C intrinsics and assembler macros for the new FSBASE and GSBASE > > > instructions. > > > > > > Very straight forward. Used in followon patches. > > > > > > [ luto: Rename the variables from FS and GS to FSBASE and GSBASE and > > > make safe to include on 32-bit kernels. ] > > > > > > v2: Use __always_inline > > > > > > [ chang: Revise the changelog. Place them in . Replace > > > the macros with GAS-compatible ones. ] > > > > > > If GCC supports it, we can add -mfsgsbase to CFLAGS and use the builtins > > > here for extra performance. > > > > Reviewed-by: Andy Lutomirski # C parts only > > > > With the caveat that I'm not convinced that the memory clobbers are > > needed. The __force_order trick in special_insns.h would probably be > > more appropriate. > > > > I don't feel qualified to review the asm part without some research. > > Whereas hpa or Boris could probably review it with their eyes closed. > > BTW the other option would be to update the min-binutils requirement > to 2.21 (currently it is 2.20) and then write it directly without .byte. > I believe 2.21 added support for these instructions. > > (It's only a binutils requirement, don't need gcc support) > I'd personally be fine with this. Linus? Thomas? Ingo?
Re: [v3 03/12] x86/fsgsbase/64: Add intrinsics/macros for FSGSBASE instructions
On Wed, Oct 24, 2018 at 12:21 PM Andi Kleen wrote: > > On Wed, Oct 24, 2018 at 11:53:54AM -0700, Andy Lutomirski wrote: > > On Tue, Oct 23, 2018 at 11:43 AM Chang S. Bae > > wrote: > > > > > > From: Andi Kleen > > > > > > Add C intrinsics and assembler macros for the new FSBASE and GSBASE > > > instructions. > > > > > > Very straight forward. Used in followon patches. > > > > > > [ luto: Rename the variables from FS and GS to FSBASE and GSBASE and > > > make safe to include on 32-bit kernels. ] > > > > > > v2: Use __always_inline > > > > > > [ chang: Revise the changelog. Place them in . Replace > > > the macros with GAS-compatible ones. ] > > > > > > If GCC supports it, we can add -mfsgsbase to CFLAGS and use the builtins > > > here for extra performance. > > > > Reviewed-by: Andy Lutomirski # C parts only > > > > With the caveat that I'm not convinced that the memory clobbers are > > needed. The __force_order trick in special_insns.h would probably be > > more appropriate. > > > > I don't feel qualified to review the asm part without some research. > > Whereas hpa or Boris could probably review it with their eyes closed. > > BTW the other option would be to update the min-binutils requirement > to 2.21 (currently it is 2.20) and then write it directly without .byte. > I believe 2.21 added support for these instructions. > > (It's only a binutils requirement, don't need gcc support) > I'd personally be fine with this. Linus? Thomas? Ingo?
[PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data
From: Sai Praneeth Prakhya
The basic resctrl file system operations and data are added for future
usage by resctrl selftest tool.
Signed-off-by: Sai Praneeth Prakhya
Signed-off-by: Arshiya Hayatkhan Pathan
Signed-off-by: Fenghua Yu
---
tools/testing/selftests/resctrl/Makefile| 10 +
tools/testing/selftests/resctrl/resctrl.h | 53
tools/testing/selftests/resctrl/resctrlfs.c | 465
3 files changed, 528 insertions(+)
create mode 100644 tools/testing/selftests/resctrl/Makefile
create mode 100644 tools/testing/selftests/resctrl/resctrl.h
create mode 100644 tools/testing/selftests/resctrl/resctrlfs.c
diff --git a/tools/testing/selftests/resctrl/Makefile
b/tools/testing/selftests/resctrl/Makefile
new file mode 100644
index ..bd5c5418961e
--- /dev/null
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -0,0 +1,10 @@
+CC = gcc
+CFLAGS = -g -Wall
+
+*.o: *.c
+ $(CC) $(CFLAGS) -c *.c
+
+.PHONY: clean
+
+clean:
+ $(RM) *.o *~
diff --git a/tools/testing/selftests/resctrl/resctrl.h
b/tools/testing/selftests/resctrl/resctrl.h
new file mode 100644
index ..fe3c3434df97
--- /dev/null
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#define _GNU_SOURCE
+#ifndef RESCTRL_H
+#define RESCTRL_H
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MB (1024 * 1024)
+#define RESCTRL_PATH "/sys/fs/resctrl"
+#define PHYS_ID_PATH "/sys/devices/system/cpu/cpu"
+#define RESCTRL_MBM"L3 monitoring detected"
+#define RESCTRL_MBA"MB allocation detected"
+#define MAX_RESCTRL_FEATURES 2
+#define RM_SIG_FILE"rm -rf sig"
+
+#define PARENT_EXIT(err_msg) \
+ do {\
+ perror(err_msg);\
+ kill(ppid, SIGKILL);\
+ exit(EXIT_FAILURE); \
+ } while (0)
+
+pid_t bm_pid, ppid;
+int ben_count;
+
+int remount_resctrlfs(bool mum_resctrlfs);
+char get_sock_num(int cpu_no);
+int validate_bw_report_request(char *bw_report);
+int validate_resctrl_feature_request(char *resctrl_val);
+int taskset_benchmark(pid_t bm_pid, int cpu_no);
+void run_benchmark(int signum, siginfo_t *info, void *ucontext);
+int write_schemata(char *ctrlgrp, char *schemata, int cpu_no,
+ char *resctrl_val);
+int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
+ char *resctrl_val);
+int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu,
+ int group_fd, unsigned long flags);
+int run_fill_buf(int span, int malloc_and_init_memory, int memflush, int op);
+
+#endif /* RESCTRL_H */
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
b/tools/testing/selftests/resctrl/resctrlfs.c
new file mode 100644
index ..d73726ef2002
--- /dev/null
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -0,0 +1,465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Basic resctrl file system operations
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *Arshiya Hayatkhan Pathan
+ *Sai Praneeth Prakhya ,
+ *Fenghua Yu
+ */
+#include "resctrl.h"
+
+/*
+ * remount_resctrlfs: Remount resctrl FS at /sys/fs/resctrl
+ * @mum_resctrlfs: Should the resctrl FS be remounted?
+ *
+ * If not mounted, mount it.
+ * If mounted and mum_resctrlfs then remount resctrl FS.
+ * If mounted and !mum_resctrlfs then noop
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+int remount_resctrlfs(bool mum_resctrlfs)
+{
+ DIR *dp;
+ struct dirent *ep;
+ unsigned int count = 0;
+
+ /*
+* If kernel is built with CONFIG_RESCTRL, then /sys/fs/resctrl should
+* be present by default
+*/
+ dp = opendir(RESCTRL_PATH);
+ if (dp) {
+ while ((ep = readdir(dp)) != NULL)
+ count++;
+ closedir(dp);
+ } else {
+ perror("Unable to read /sys/fs/resctrl");
+
+ return errno;
+ }
+
+ /*
+* If resctrl FS has more than two entries, it means that resctrl FS has
+* already been mounted. The two default entries are "." and "..", these
+* are present even when resctrl FS is not mounted
+*/
+ if (count > 2) {
+ if (mum_resctrlfs) {
+ if (umount(RESCTRL_PATH) != 0) {
+ perror("Unable to umount resctrl");
+
+ return errno;
+ }
+ printf("Remount: done!\n");
+ } else {
+ printf("Mounted already. Not remounting!\n");
+
+ return 0;
+ }
+
[PATCH v2 2/8] selftests/resctrl: Add basic resctrl file system operations and data
From: Sai Praneeth Prakhya
The basic resctrl file system operations and data are added for future
usage by resctrl selftest tool.
Signed-off-by: Sai Praneeth Prakhya
Signed-off-by: Arshiya Hayatkhan Pathan
Signed-off-by: Fenghua Yu
---
tools/testing/selftests/resctrl/Makefile| 10 +
tools/testing/selftests/resctrl/resctrl.h | 53
tools/testing/selftests/resctrl/resctrlfs.c | 465
3 files changed, 528 insertions(+)
create mode 100644 tools/testing/selftests/resctrl/Makefile
create mode 100644 tools/testing/selftests/resctrl/resctrl.h
create mode 100644 tools/testing/selftests/resctrl/resctrlfs.c
diff --git a/tools/testing/selftests/resctrl/Makefile
b/tools/testing/selftests/resctrl/Makefile
new file mode 100644
index ..bd5c5418961e
--- /dev/null
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -0,0 +1,10 @@
+CC = gcc
+CFLAGS = -g -Wall
+
+*.o: *.c
+ $(CC) $(CFLAGS) -c *.c
+
+.PHONY: clean
+
+clean:
+ $(RM) *.o *~
diff --git a/tools/testing/selftests/resctrl/resctrl.h
b/tools/testing/selftests/resctrl/resctrl.h
new file mode 100644
index ..fe3c3434df97
--- /dev/null
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#define _GNU_SOURCE
+#ifndef RESCTRL_H
+#define RESCTRL_H
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MB (1024 * 1024)
+#define RESCTRL_PATH "/sys/fs/resctrl"
+#define PHYS_ID_PATH "/sys/devices/system/cpu/cpu"
+#define RESCTRL_MBM"L3 monitoring detected"
+#define RESCTRL_MBA"MB allocation detected"
+#define MAX_RESCTRL_FEATURES 2
+#define RM_SIG_FILE"rm -rf sig"
+
+#define PARENT_EXIT(err_msg) \
+ do {\
+ perror(err_msg);\
+ kill(ppid, SIGKILL);\
+ exit(EXIT_FAILURE); \
+ } while (0)
+
+pid_t bm_pid, ppid;
+int ben_count;
+
+int remount_resctrlfs(bool mum_resctrlfs);
+char get_sock_num(int cpu_no);
+int validate_bw_report_request(char *bw_report);
+int validate_resctrl_feature_request(char *resctrl_val);
+int taskset_benchmark(pid_t bm_pid, int cpu_no);
+void run_benchmark(int signum, siginfo_t *info, void *ucontext);
+int write_schemata(char *ctrlgrp, char *schemata, int cpu_no,
+ char *resctrl_val);
+int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
+ char *resctrl_val);
+int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu,
+ int group_fd, unsigned long flags);
+int run_fill_buf(int span, int malloc_and_init_memory, int memflush, int op);
+
+#endif /* RESCTRL_H */
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
b/tools/testing/selftests/resctrl/resctrlfs.c
new file mode 100644
index ..d73726ef2002
--- /dev/null
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -0,0 +1,465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Basic resctrl file system operations
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *Arshiya Hayatkhan Pathan
+ *Sai Praneeth Prakhya ,
+ *Fenghua Yu
+ */
+#include "resctrl.h"
+
+/*
+ * remount_resctrlfs: Remount resctrl FS at /sys/fs/resctrl
+ * @mum_resctrlfs: Should the resctrl FS be remounted?
+ *
+ * If not mounted, mount it.
+ * If mounted and mum_resctrlfs then remount resctrl FS.
+ * If mounted and !mum_resctrlfs then noop
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+int remount_resctrlfs(bool mum_resctrlfs)
+{
+ DIR *dp;
+ struct dirent *ep;
+ unsigned int count = 0;
+
+ /*
+* If kernel is built with CONFIG_RESCTRL, then /sys/fs/resctrl should
+* be present by default
+*/
+ dp = opendir(RESCTRL_PATH);
+ if (dp) {
+ while ((ep = readdir(dp)) != NULL)
+ count++;
+ closedir(dp);
+ } else {
+ perror("Unable to read /sys/fs/resctrl");
+
+ return errno;
+ }
+
+ /*
+* If resctrl FS has more than two entries, it means that resctrl FS has
+* already been mounted. The two default entries are "." and "..", these
+* are present even when resctrl FS is not mounted
+*/
+ if (count > 2) {
+ if (mum_resctrlfs) {
+ if (umount(RESCTRL_PATH) != 0) {
+ perror("Unable to umount resctrl");
+
+ return errno;
+ }
+ printf("Remount: done!\n");
+ } else {
+ printf("Mounted already. Not remounting!\n");
+
+ return 0;
+ }
+
[PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark
From: Sai Praneeth Prakhya
The callback starts a child process and puts the child pid in created
resctrl group with specified memory bandwidth in schemata. The child
starts running benchmark.
Signed-off-by: Sai Praneeth Prakhya
Signed-off-by: Arshiya Hayatkhan Pathan
Signed-off-by: Fenghua Yu
---
tools/testing/selftests/resctrl/membw.c | 273 ++
tools/testing/selftests/resctrl/resctrl.h | 27 +++
2 files changed, 300 insertions(+)
diff --git a/tools/testing/selftests/resctrl/membw.c
b/tools/testing/selftests/resctrl/membw.c
index 1bced7e7f148..dfcd9c1244d8 100644
--- a/tools/testing/selftests/resctrl/membw.c
+++ b/tools/testing/selftests/resctrl/membw.c
@@ -401,3 +401,276 @@ static unsigned long get_mem_bw_resctrl(void)
return mbm_total;
}
+
+pid_t bm_pid, ppid;
+
+static void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
+{
+ kill(bm_pid, SIGKILL);
+ printf("Ending\n\n");
+
+ exit(EXIT_SUCCESS);
+}
+
+/*
+ * print_results_bw: the memory bandwidth results are stored in a file
+ * @filename: file that stores the results
+ * @bm_pid:child pid that runs benchmark
+ * @bw_imc:perf imc counter value
+ * @bw_resc: memory bandwidth value
+ *
+ * Return: 0 on success. non-zero on failure.
+ */
+static int print_results_bw(char *filename, int bm_pid, float bw_imc,
+ unsigned long bw_resc)
+{
+ int diff = abs(bw_imc - bw_resc);
+ FILE *fp;
+
+ if (strcmp(filename, "stdio") == 0 || strcmp(filename, "stderr") == 0) {
+ printf("Pid: %d \t Mem_BW_iMC: %f \t ", bm_pid, bw_imc);
+ printf("Mem_BW_resc: %lu \t Difference: %d\n", bw_resc, diff);
+ } else {
+ fp = fopen(filename, "a");
+ if (!fp) {
+ perror("Cannot open results file");
+
+ return errno;
+ }
+ if (fprintf(fp, "Pid: %d \t Mem_BW_iMC: %f \t ",
+ bm_pid, bw_imc) <= 0 ||
+ fprintf(fp, "Mem_BW_resc: %lu \t Difference: %d\n",
+ bw_resc, diff) <= 0) {
+ fclose(fp);
+ perror("Could not log results.");
+
+ return errno;
+ }
+ fclose(fp);
+ }
+
+ return 0;
+}
+
+static int
+measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start)
+{
+ unsigned long bw_imc, bw_resc, bw_resc_end;
+ int ret;
+
+ /*
+* Measure memory bandwidth from resctrl and from
+* another source which is perf imc value or could
+* be something else if perf imc event is not available.
+* Compare the two values to validate resctrl value.
+* It takes 1sec to measure the data.
+*/
+ bw_imc = get_mem_bw_imc(param->cpu_no, param->bw_report);
+ if (bw_imc < 0)
+ return bw_imc;
+
+ bw_resc_end = get_mem_bw_resctrl();
+ if (bw_resc_end < 0)
+ return bw_resc_end;
+
+ bw_resc = (bw_resc_end - *bw_resc_start) / MB;
+ ret = print_results_bw(param->filename, bm_pid, bw_imc, bw_resc);
+ if (ret)
+ return ret;
+
+ *bw_resc_start = bw_resc_end;
+
+ return 0;
+}
+
+/*
+ * membw_val: execute benchmark and measure memory bandwidth on
+ * the benchmark
+ * @benchmark_cmd: benchmark command and its arguments
+ * @param: parameters passed to membw_val()
+ *
+ * Return: 0 on success. non-zero on failure.
+ */
+int membw_val(char **benchmark_cmd, struct resctrl_val_param *param)
+{
+ char *resctrl_val = param->resctrl_val;
+ unsigned long bw_resc_start = 0;
+ struct sigaction sigact;
+ union sigval value;
+ int sig = 0, ret = 0;
+ FILE *fp;
+
+ if (strcmp(param->filename, "") == 0)
+ sprintf(param->filename, "stdio");
+
+ if (strcmp(param->bw_report, "") == 0)
+ param->bw_report = "total";
+
+ ret = validate_resctrl_feature_request(resctrl_val);
+ if (ret)
+ return ret;
+
+ if ((strcmp(resctrl_val, "mba")) == 0 ||
+ (strcmp(resctrl_val, "mbm")) == 0) {
+ ret = validate_bw_report_request(param->bw_report);
+ if (ret)
+ return ret;
+ }
+
+ ret = remount_resctrlfs(param->mum_resctrlfs);
+ if (ret)
+ return ret;
+
+ /*
+* If benchmark wasn't successfully started by child, then child should
+* kill parent, so save parent's pid
+*/
+ ppid = getpid();
+
+ /* File based synchronization between parent and child */
+ fp = fopen("sig", "w");
+ if (!fp || (fprintf(fp, "%d\n", 0) <= 0) || (fclose(fp) == EOF)) {
+ perror("Unable to establish sync bw parent &
[PATCH v2 1/8] selftests/resctrl: Add README for resctrl tests
resctrl tests will be implemented. README is added for the tool first. Signed-off-by: Fenghua Yu --- tools/testing/selftests/resctrl/README | 53 ++ 1 file changed, 53 insertions(+) create mode 100644 tools/testing/selftests/resctrl/README diff --git a/tools/testing/selftests/resctrl/README b/tools/testing/selftests/resctrl/README new file mode 100644 index ..09851a619547 --- /dev/null +++ b/tools/testing/selftests/resctrl/README @@ -0,0 +1,53 @@ +resctrl_tests - resctrl file system test suit + +Authors: + Fenghua Yu + Sai Praneeth Prakhya , + Arshiya Hayatkhan Pathan + +resctrl_tests tests various resctrl functionalities and interfaces including +both software and hardware. + +Currently it supports Memory Bandwidth Monitoring test and Memory Bandwidth +Allocation test on Intel RDT hardware. More tests will be added in the future. +And the test suit can be extended to cover AMD QoS and ARM MPAM hardware +as well. + +BUILD +- + +Run "make" to build executable file "resctrl_tests". + +RUN +--- + +To use resctrl_tests, root or sudoer privileges are required. This is because +the test needs to mount resctrl file system and change contents in the file +system. + +Executing the test without any parameter will run all supported tests: + + sudo ./resctrl_tests + +OVERVIEW OF EXECUTION +- + +A test case has four stages: + + - setup: mount resctrl file system, create group, setup schemata, move test +process pids to tasks, start benchmark. + - execute: let benchmark run + - verify: get resctrl data and verify the data with another source, e.g. +perf event. + - teardown: umount resctrl and clear temporary files. + +ARGUMENTS +- + +Parameter '-h' shows usage information. + +usage: resctrl_tests [-h] [-b benchmark_cmd [options]] [-t test list] +-b benchmark_cmd [options]: run specified benchmark + default benchmark is builtin fill_buf +-t test list: run tests specified in the test list, e.g. -t mbm,mba +-h: help -- 2.5.0
[PATCH v2 5/8] selftests/resctrl: Add built in benchmark
From: Sai Praneeth Prakhya
Built-in benchmark fill_buf generates stressful memory bandwidth
and cache traffic.
Later it will be used as a default benchmark by various resctrl tests
such as MBA (Memory Bandwidth Allocation) and MBM (Memory Bandwidth
Monitoring) tests.
Signed-off-by: Sai Praneeth Prakhya
Signed-off-by: Arshiya Hayatkhan Pathan
Signed-off-by: Fenghua Yu
---
tools/testing/selftests/resctrl/fill_buf.c | 175 +
1 file changed, 175 insertions(+)
create mode 100644 tools/testing/selftests/resctrl/fill_buf.c
diff --git a/tools/testing/selftests/resctrl/fill_buf.c
b/tools/testing/selftests/resctrl/fill_buf.c
new file mode 100644
index ..d9950b5d068d
--- /dev/null
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * fill_buf benchmark
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *Arshiya Hayatkhan Pathan
+ *Sai Praneeth Prakhya ,
+ *Fenghua Yu
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "resctrl.h"
+
+#define CL_SIZE(64)
+#define PAGE_SIZE (4 * 1024)
+#define MB (1024 * 1024)
+
+static unsigned char *startptr;
+
+static void sb(void)
+{
+ asm volatile("sfence\n\t"
+: : : "memory");
+}
+
+static void ctrl_handler(int signo)
+{
+ free(startptr);
+ printf("\nEnding\n");
+ sb();
+ exit(EXIT_SUCCESS);
+}
+
+static void cl_flush(void *p)
+{
+ asm volatile("clflush (%0)\n\t"
+: : "r"(p) : "memory");
+}
+
+static void mem_flush(void *p, size_t s)
+{
+ char *cp = (char *)p;
+ size_t i = 0;
+
+ s = s / CL_SIZE; /* mem size in cache llines */
+
+ for (i = 0; i < s; i++)
+ cl_flush([i * CL_SIZE]);
+
+ sb();
+}
+
+static void *malloc_and_init_memory(size_t s)
+{
+ uint64_t *p64;
+ size_t s64;
+
+ void *p = memalign(PAGE_SIZE, s);
+
+ p64 = (uint64_t *)p;
+ s64 = s / sizeof(uint64_t);
+
+ while (s64 > 0) {
+ *p64 = (uint64_t)rand();
+ p64 += (CL_SIZE / sizeof(uint64_t));
+ s64 -= (CL_SIZE / sizeof(uint64_t));
+ }
+
+ return p;
+}
+
+static void fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr)
+{
+ while (1) {
+ unsigned char sum, *p;
+
+ p = start_ptr;
+ /* Read two chars in each cache line to stress cache */
+ while (p < (end_ptr - 1024)) {
+ sum += p[0] + p[32] + p[64] + p[96] + p[128] +
+ p[160] + p[192] + p[224] + p[256] + p[288] +
+ p[320] + p[352] + p[384] + p[416] + p[448] +
+ p[480] + p[512] + p[544] + p[576] + p[608] +
+ p[640] + p[672] + p[704] + p[736] + p[768] +
+ p[800] + p[832] + p[864] + p[896] + p[928] +
+ p[960] + p[992];
+ p += 1024;
+ }
+ }
+}
+
+static void fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr)
+{
+ while (1) {
+ while (start_ptr < end_ptr) {
+ *start_ptr = '1';
+ start_ptr += (CL_SIZE / 2);
+ }
+ start_ptr = startptr;
+ }
+}
+
+static void
+fill_cache(unsigned long long buf_size, int malloc_and_init,
+ int memflush, int op)
+{
+ unsigned char *start_ptr, *end_ptr;
+ unsigned long long i;
+
+ if (malloc_and_init) {
+ start_ptr = malloc_and_init_memory(buf_size);
+ printf("Started benchmark with memalign\n");
+ } else {
+ start_ptr = malloc(buf_size);
+ printf("Started benchmark with malloc\n");
+ }
+
+ if (!start_ptr)
+ return;
+
+ startptr = start_ptr;
+ end_ptr = start_ptr + buf_size;
+
+ /*
+* It's better to touch the memory once to avoid any compiler
+* optimizations
+*/
+ if (!malloc_and_init) {
+ for (i = 0; i < buf_size; i++)
+ *start_ptr++ = (unsigned char)rand();
+ }
+
+ start_ptr = startptr;
+
+ /* Flush the memory before using to avoid "cache hot pages" effect */
+ if (memflush) {
+ mem_flush(start_ptr, buf_size);
+ printf("Started benchmark with memflush\n");
+ } else {
+ printf("Started benchmark *without* memflush\n");
+ }
+
+ if (op == 0)
+ fill_cache_read(start_ptr, end_ptr);
+ else
+ fill_cache_write(start_ptr, end_ptr);
+
+ free(startptr);
+}
+
+int run_fill_buf(int span, int malloc_and_init_memory, int memflush, int op)
+{
+ unsigned long long cache_size
[PATCH v2 4/8] selftests/resctrl: Add callback to start a benchmark
From: Sai Praneeth Prakhya
The callback starts a child process and puts the child pid in created
resctrl group with specified memory bandwidth in schemata. The child
starts running benchmark.
Signed-off-by: Sai Praneeth Prakhya
Signed-off-by: Arshiya Hayatkhan Pathan
Signed-off-by: Fenghua Yu
---
tools/testing/selftests/resctrl/membw.c | 273 ++
tools/testing/selftests/resctrl/resctrl.h | 27 +++
2 files changed, 300 insertions(+)
diff --git a/tools/testing/selftests/resctrl/membw.c
b/tools/testing/selftests/resctrl/membw.c
index 1bced7e7f148..dfcd9c1244d8 100644
--- a/tools/testing/selftests/resctrl/membw.c
+++ b/tools/testing/selftests/resctrl/membw.c
@@ -401,3 +401,276 @@ static unsigned long get_mem_bw_resctrl(void)
return mbm_total;
}
+
+pid_t bm_pid, ppid;
+
+static void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
+{
+ kill(bm_pid, SIGKILL);
+ printf("Ending\n\n");
+
+ exit(EXIT_SUCCESS);
+}
+
+/*
+ * print_results_bw: the memory bandwidth results are stored in a file
+ * @filename: file that stores the results
+ * @bm_pid:child pid that runs benchmark
+ * @bw_imc:perf imc counter value
+ * @bw_resc: memory bandwidth value
+ *
+ * Return: 0 on success. non-zero on failure.
+ */
+static int print_results_bw(char *filename, int bm_pid, float bw_imc,
+ unsigned long bw_resc)
+{
+ int diff = abs(bw_imc - bw_resc);
+ FILE *fp;
+
+ if (strcmp(filename, "stdio") == 0 || strcmp(filename, "stderr") == 0) {
+ printf("Pid: %d \t Mem_BW_iMC: %f \t ", bm_pid, bw_imc);
+ printf("Mem_BW_resc: %lu \t Difference: %d\n", bw_resc, diff);
+ } else {
+ fp = fopen(filename, "a");
+ if (!fp) {
+ perror("Cannot open results file");
+
+ return errno;
+ }
+ if (fprintf(fp, "Pid: %d \t Mem_BW_iMC: %f \t ",
+ bm_pid, bw_imc) <= 0 ||
+ fprintf(fp, "Mem_BW_resc: %lu \t Difference: %d\n",
+ bw_resc, diff) <= 0) {
+ fclose(fp);
+ perror("Could not log results.");
+
+ return errno;
+ }
+ fclose(fp);
+ }
+
+ return 0;
+}
+
+static int
+measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start)
+{
+ unsigned long bw_imc, bw_resc, bw_resc_end;
+ int ret;
+
+ /*
+* Measure memory bandwidth from resctrl and from
+* another source which is perf imc value or could
+* be something else if perf imc event is not available.
+* Compare the two values to validate resctrl value.
+* It takes 1sec to measure the data.
+*/
+ bw_imc = get_mem_bw_imc(param->cpu_no, param->bw_report);
+ if (bw_imc < 0)
+ return bw_imc;
+
+ bw_resc_end = get_mem_bw_resctrl();
+ if (bw_resc_end < 0)
+ return bw_resc_end;
+
+ bw_resc = (bw_resc_end - *bw_resc_start) / MB;
+ ret = print_results_bw(param->filename, bm_pid, bw_imc, bw_resc);
+ if (ret)
+ return ret;
+
+ *bw_resc_start = bw_resc_end;
+
+ return 0;
+}
+
+/*
+ * membw_val: execute benchmark and measure memory bandwidth on
+ * the benchmark
+ * @benchmark_cmd: benchmark command and its arguments
+ * @param: parameters passed to membw_val()
+ *
+ * Return: 0 on success. non-zero on failure.
+ */
+int membw_val(char **benchmark_cmd, struct resctrl_val_param *param)
+{
+ char *resctrl_val = param->resctrl_val;
+ unsigned long bw_resc_start = 0;
+ struct sigaction sigact;
+ union sigval value;
+ int sig = 0, ret = 0;
+ FILE *fp;
+
+ if (strcmp(param->filename, "") == 0)
+ sprintf(param->filename, "stdio");
+
+ if (strcmp(param->bw_report, "") == 0)
+ param->bw_report = "total";
+
+ ret = validate_resctrl_feature_request(resctrl_val);
+ if (ret)
+ return ret;
+
+ if ((strcmp(resctrl_val, "mba")) == 0 ||
+ (strcmp(resctrl_val, "mbm")) == 0) {
+ ret = validate_bw_report_request(param->bw_report);
+ if (ret)
+ return ret;
+ }
+
+ ret = remount_resctrlfs(param->mum_resctrlfs);
+ if (ret)
+ return ret;
+
+ /*
+* If benchmark wasn't successfully started by child, then child should
+* kill parent, so save parent's pid
+*/
+ ppid = getpid();
+
+ /* File based synchronization between parent and child */
+ fp = fopen("sig", "w");
+ if (!fp || (fprintf(fp, "%d\n", 0) <= 0) || (fclose(fp) == EOF)) {
+ perror("Unable to establish sync bw parent &
[PATCH v2 1/8] selftests/resctrl: Add README for resctrl tests
resctrl tests will be implemented. README is added for the tool first. Signed-off-by: Fenghua Yu --- tools/testing/selftests/resctrl/README | 53 ++ 1 file changed, 53 insertions(+) create mode 100644 tools/testing/selftests/resctrl/README diff --git a/tools/testing/selftests/resctrl/README b/tools/testing/selftests/resctrl/README new file mode 100644 index ..09851a619547 --- /dev/null +++ b/tools/testing/selftests/resctrl/README @@ -0,0 +1,53 @@ +resctrl_tests - resctrl file system test suit + +Authors: + Fenghua Yu + Sai Praneeth Prakhya , + Arshiya Hayatkhan Pathan + +resctrl_tests tests various resctrl functionalities and interfaces including +both software and hardware. + +Currently it supports Memory Bandwidth Monitoring test and Memory Bandwidth +Allocation test on Intel RDT hardware. More tests will be added in the future. +And the test suit can be extended to cover AMD QoS and ARM MPAM hardware +as well. + +BUILD +- + +Run "make" to build executable file "resctrl_tests". + +RUN +--- + +To use resctrl_tests, root or sudoer privileges are required. This is because +the test needs to mount resctrl file system and change contents in the file +system. + +Executing the test without any parameter will run all supported tests: + + sudo ./resctrl_tests + +OVERVIEW OF EXECUTION +- + +A test case has four stages: + + - setup: mount resctrl file system, create group, setup schemata, move test +process pids to tasks, start benchmark. + - execute: let benchmark run + - verify: get resctrl data and verify the data with another source, e.g. +perf event. + - teardown: umount resctrl and clear temporary files. + +ARGUMENTS +- + +Parameter '-h' shows usage information. + +usage: resctrl_tests [-h] [-b benchmark_cmd [options]] [-t test list] +-b benchmark_cmd [options]: run specified benchmark + default benchmark is builtin fill_buf +-t test list: run tests specified in the test list, e.g. -t mbm,mba +-h: help -- 2.5.0
[PATCH v2 5/8] selftests/resctrl: Add built in benchmark
From: Sai Praneeth Prakhya
Built-in benchmark fill_buf generates stressful memory bandwidth
and cache traffic.
Later it will be used as a default benchmark by various resctrl tests
such as MBA (Memory Bandwidth Allocation) and MBM (Memory Bandwidth
Monitoring) tests.
Signed-off-by: Sai Praneeth Prakhya
Signed-off-by: Arshiya Hayatkhan Pathan
Signed-off-by: Fenghua Yu
---
tools/testing/selftests/resctrl/fill_buf.c | 175 +
1 file changed, 175 insertions(+)
create mode 100644 tools/testing/selftests/resctrl/fill_buf.c
diff --git a/tools/testing/selftests/resctrl/fill_buf.c
b/tools/testing/selftests/resctrl/fill_buf.c
new file mode 100644
index ..d9950b5d068d
--- /dev/null
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * fill_buf benchmark
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Authors:
+ *Arshiya Hayatkhan Pathan
+ *Sai Praneeth Prakhya ,
+ *Fenghua Yu
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "resctrl.h"
+
+#define CL_SIZE(64)
+#define PAGE_SIZE (4 * 1024)
+#define MB (1024 * 1024)
+
+static unsigned char *startptr;
+
+static void sb(void)
+{
+ asm volatile("sfence\n\t"
+: : : "memory");
+}
+
+static void ctrl_handler(int signo)
+{
+ free(startptr);
+ printf("\nEnding\n");
+ sb();
+ exit(EXIT_SUCCESS);
+}
+
+static void cl_flush(void *p)
+{
+ asm volatile("clflush (%0)\n\t"
+: : "r"(p) : "memory");
+}
+
+static void mem_flush(void *p, size_t s)
+{
+ char *cp = (char *)p;
+ size_t i = 0;
+
+ s = s / CL_SIZE; /* mem size in cache llines */
+
+ for (i = 0; i < s; i++)
+ cl_flush([i * CL_SIZE]);
+
+ sb();
+}
+
+static void *malloc_and_init_memory(size_t s)
+{
+ uint64_t *p64;
+ size_t s64;
+
+ void *p = memalign(PAGE_SIZE, s);
+
+ p64 = (uint64_t *)p;
+ s64 = s / sizeof(uint64_t);
+
+ while (s64 > 0) {
+ *p64 = (uint64_t)rand();
+ p64 += (CL_SIZE / sizeof(uint64_t));
+ s64 -= (CL_SIZE / sizeof(uint64_t));
+ }
+
+ return p;
+}
+
+static void fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr)
+{
+ while (1) {
+ unsigned char sum, *p;
+
+ p = start_ptr;
+ /* Read two chars in each cache line to stress cache */
+ while (p < (end_ptr - 1024)) {
+ sum += p[0] + p[32] + p[64] + p[96] + p[128] +
+ p[160] + p[192] + p[224] + p[256] + p[288] +
+ p[320] + p[352] + p[384] + p[416] + p[448] +
+ p[480] + p[512] + p[544] + p[576] + p[608] +
+ p[640] + p[672] + p[704] + p[736] + p[768] +
+ p[800] + p[832] + p[864] + p[896] + p[928] +
+ p[960] + p[992];
+ p += 1024;
+ }
+ }
+}
+
+static void fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr)
+{
+ while (1) {
+ while (start_ptr < end_ptr) {
+ *start_ptr = '1';
+ start_ptr += (CL_SIZE / 2);
+ }
+ start_ptr = startptr;
+ }
+}
+
+static void
+fill_cache(unsigned long long buf_size, int malloc_and_init,
+ int memflush, int op)
+{
+ unsigned char *start_ptr, *end_ptr;
+ unsigned long long i;
+
+ if (malloc_and_init) {
+ start_ptr = malloc_and_init_memory(buf_size);
+ printf("Started benchmark with memalign\n");
+ } else {
+ start_ptr = malloc(buf_size);
+ printf("Started benchmark with malloc\n");
+ }
+
+ if (!start_ptr)
+ return;
+
+ startptr = start_ptr;
+ end_ptr = start_ptr + buf_size;
+
+ /*
+* It's better to touch the memory once to avoid any compiler
+* optimizations
+*/
+ if (!malloc_and_init) {
+ for (i = 0; i < buf_size; i++)
+ *start_ptr++ = (unsigned char)rand();
+ }
+
+ start_ptr = startptr;
+
+ /* Flush the memory before using to avoid "cache hot pages" effect */
+ if (memflush) {
+ mem_flush(start_ptr, buf_size);
+ printf("Started benchmark with memflush\n");
+ } else {
+ printf("Started benchmark *without* memflush\n");
+ }
+
+ if (op == 0)
+ fill_cache_read(start_ptr, end_ptr);
+ else
+ fill_cache_write(start_ptr, end_ptr);
+
+ free(startptr);
+}
+
+int run_fill_buf(int span, int malloc_and_init_memory, int memflush, int op)
+{
+ unsigned long long cache_size
[PATCH v2 8/8] selftests/resctrl: Add the test in MAINTAINERS
The resctrl selftests will be maintained by RDT maintainers. Signed-off-by: Fenghua Yu --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 48a65c3a4189..ecd1369ef761 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12270,6 +12270,7 @@ S: Supported F: arch/x86/kernel/cpu/intel_rdt* F: arch/x86/include/asm/intel_rdt_sched.h F: Documentation/x86/intel_rdt* +F: tools/testing/selftests/resctrl READ-COPY UPDATE (RCU) M: "Paul E. McKenney" -- 2.5.0
[PATCH v2 8/8] selftests/resctrl: Add the test in MAINTAINERS
The resctrl selftests will be maintained by RDT maintainers. Signed-off-by: Fenghua Yu --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 48a65c3a4189..ecd1369ef761 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12270,6 +12270,7 @@ S: Supported F: arch/x86/kernel/cpu/intel_rdt* F: arch/x86/include/asm/intel_rdt_sched.h F: Documentation/x86/intel_rdt* +F: tools/testing/selftests/resctrl READ-COPY UPDATE (RCU) M: "Paul E. McKenney" -- 2.5.0
