Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] imx7: fix Kconfig warning and build errors
From: Randy Dunlap @ 2017-04-12 18:22 UTC (permalink / raw)
  To: linux-arm-kernel

From: Randy Dunlap <rdunlap@infradead.org>

Fix Kconfig warning in drivers/soc/imx/Kconfig and subsequent build errors
elsewhere when CONFIG_PM is not enabled.

warning: (IMX7_PM_DOMAINS) selects PM_GENERIC_DOMAINS which has unmet direct dependencies (PM)

This warning causes multiple build errors in drivers/base/power/domain.c:

../drivers/base/power/domain.c: In function 'genpd_queue_power_off_work':
../drivers/base/power/domain.c:279:13: error: 'pm_wq' undeclared (first use in this function)
  queue_work(pm_wq, &genpd->power_off_work);
../drivers/base/power/domain.c: In function 'genpd_dev_pm_qos_notifier':
../drivers/base/power/domain.c:462:25: error: 'struct dev_pm_info' has no member named 'ignore_children'
   if (!dev || dev->power.ignore_children)
../drivers/base/power/domain.c: In function 'rtpm_status_str':
../drivers/base/power/domain.c:2207:16: error: 'struct dev_pm_info' has no member named 'runtime_error'
  if (dev->power.runtime_error)
../drivers/base/power/domain.c:2209:21: error: 'struct dev_pm_info' has no member named 'disable_depth'
  else if (dev->power.disable_depth)
../drivers/base/power/domain.c:2211:21: error: 'struct dev_pm_info' has no member named 'runtime_status'
  else if (dev->power.runtime_status < ARRAY_SIZE(status_lookup))
   p = status_lookup[dev->power.runtime_status];

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc:	Shawn Guo <shawnguo@kernel.org>
Cc:	Sascha Hauer <kernel@pengutronix.de>
Cc:	Fabio Estevam <fabio.estevam@nxp.com>
Cc:	linux-arm-kernel at lists.infradead.org
---
 drivers/soc/imx/Kconfig |    1 +
 1 file changed, 1 insertion(+)

--- linux-next-20170412.orig/drivers/soc/imx/Kconfig
+++ linux-next-20170412/drivers/soc/imx/Kconfig
@@ -2,6 +2,7 @@ menu "i.MX SoC drivers"
 
 config IMX7_PM_DOMAINS
 	bool "i.MX7 PM domains"
+	depends on PM
 	select PM_GENERIC_DOMAINS
 	depends on SOC_IMX7D || (COMPILE_TEST && OF)
 	default y if SOC_IMX7D

^ permalink raw reply

* [PATCH v4 2/2] PCI: quirks: Fix ThunderX2 dma alias handling
From: Jayachandran C @ 2017-04-12 18:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412162118.GC25197@bhelgaas-glaptop.roam.corp.google.com>

On Wed, Apr 12, 2017 at 11:21:18AM -0500, Bjorn Helgaas wrote:
> On Tue, Apr 11, 2017 at 03:27:02PM +0000, Jayachandran C wrote:
> > On Tue, Apr 11, 2017 at 08:41:25AM -0500, Bjorn Helgaas wrote:
> > > [+cc Joerg]
> > > 
> > > On Tue, Apr 11, 2017 at 07:10:48AM +0000, Jayachandran C wrote:
> > > > On Mon, Apr 10, 2017 at 08:28:47PM -0500, Bjorn Helgaas wrote:
> > > > > Hi Jayachandran,
> > > > > 
> > > > > On Mon, Apr 03, 2017 at 01:15:04PM +0000, Jayachandran C wrote:
> > > > > > The Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the PCI
> > > > > > topology is slightly unusual. For a multi-node system, it looks like:
> > > > > > 
> > > > > > [node level PCI bridges - one per node]
> > > > > >     [SoC PCI devices with MSI-X but no IOMMU]
> > > > > >     [PCI-PCIe "glue" bridges - upto 14, one per real port below]
> > > > > >         [PCIe real root ports associated with IOMMU and GICv3 ITS]
> > > > > >             [External PCI devices connected to PCIe links]
> > > > > > 
> > > > > > The top two levels of bridges should have introduced aliases since they
> > > > > > are PCI and PCI/PCIe bridges, but in the case of ThunderX2 they do not.
> > > > > > In the case of external PCIe devices, the "real" root ports are connected
> > > > > > to the SMMU and the GIC ITS, so PCI-PCIe bridge does not introduce an
> > > > > > alias. The SoC PCI devices are directly connected to the GIC ITS, so the
> > > > > > node level bridges do not introduce an alias either.
> > > > > > 
> > > > > > To handle this quirk, we mark the real PCIe root ports and node level
> > > > > > PCI bridges with the flag PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT.  With this,
> > > > > > pci_for_each_dma_alias() works correctly for external PCIe devices and
> > > > > > SoC PCI devices.
> > > > > > 
> > > > > > For the current revision of Cavium ThunderX2, the VendorID and Device ID
> > > > > > are from Broadcom Vulcan (14e4:90XX).
> > > > > 
> > > > > Can you supply some text here about why we want to apply this patch?
> > > > > E.g., does it avoid making unnecessary IOMMU mappings, improve
> > > > > performance, avoid a crash, etc?
> > > > 
> > > > If this is for the commit message, I hope the following is ok:
> > > > 
> > > > "With this change, both MSI-X and IO virtualization work correctly on
> > > > Cavium ThunderX2. The GIC ITS driver gets the correct device ID to
> > > > configure MSI-X, the SMMUv3 driver gets the correct Stream IDs for PCI
> > > > devices, and the IOMMU groups are setup correctly."
> > > 
> > > This doesn't get at what the actual problem is.  I'm hoping for
> > > something like "without this change, we set up an IOMMU mapping for
> > > requestor ID X, but device DMA uses requestor ID Y because ...., which
> > > results in an IOMMU fault"
> > 
> > Ok. I hope this would be better:
> > 
> > "Without this change, the last alias seen while traversing the PCI
> > hierarchy will be used as the RID to generate the device ID for ITS
> > and stream ID for SMMU. This in turn causes the MSI-X generated by the
> > device to fail since the ITS expects to have translation tables based
> > on the actual PCIe RID and not the (irrelevant) alias. Similarly, the
> > device DMA also fails when SMMU is enabled due to incorrect value in
> > SMMU translation tables"
> 
> This description is true, but I don't think it addresses the real
> problem.  I think the real problem is that your IOMMU code doesn't
> handle aliases correctly, and by ignoring these invalid aliases, we
> happen to map an alias that works for the builtin devices.  But that's
> only because we got lucky (those devices use a single RID and they're
> not behind bridges that optionally take ownership).
> 
> It would make sense to me if we fixed the IOMMU code to map *all* the
> aliases, which should be enough to make your devices work.  If we then
> wanted to apply a patch like this on top, it would be simply an
> optimization that avoids unnecessary IOMMU mappings.

The issue that the IOMMU code does not handle valid aliases is
unrelated to what I am trying to fix. The quirk is to make sure
that invalid aliases are not seen on ThunderX2 while doing
pci_for_each_dma_alias().

The DMA and MSI-X requests leave the PCI/PCIe hierarchy at the point
where the SMMU (or ITS) is attached, i.e. at the bridge marked with
PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT. The quirk ensures that we don't look
for aliases above that point.

The toplevel bridge is marked PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT since
the on-chip devices are directly connected to the ITS (they do not
use SMMU).

The top two levels of bridges are not real PCI bridges but just
PCI bridge-like things that were added to tie the whole hierarchy
together for configuration and enumeration. They do not handle
PCI/PCIe transactions in the traditional sense.

I think my problem description is still not correct, maybe:
"The SMMU (and ITS) expects to device tables to use the RID seen
at the bridge they are associated with. Currently the
pci_for_each_dma_alias() code traverses beyond this point and
generates incorrect aliases due to the PCI and PCI/PCIe bridges
above. This causes MSI-X interrupts and device DMA to fail since
the SMMU and ITS tables to be setup with incorrect IDs"

> > > I suspect the reason this patch makes a difference is because the
> > > current pci_for_each_dma_alias() believes one of those top-level
> > > bridges is an alias, and the iterator produces it last, so that's the
> > > one you map.  The IOMMU is attached lower down, so that top-level
> > > bridge is not in fact an alias, but since you only look at the *last*
> > > one, you don't map the correct aliases from lower down in the tree.
> > 
> > Exactly. The IORT spec allows a range of RIDs to map to an SMMU, which
> > means that a PCI RC can multiple SMMUs, each handling a subset of RIDs.
> > 
> > In the case of Cavium ThunderX2, the RID which we should see on the RC
> > - if we follow the standard and factor in the aliasing introduced by the
> > PCI bridge and the PCI/PCIe bridge - is not the RID seen by the SMMU (or
> > ITS).
> > 
> > But, if we stop the traversal at the point where SMMU (or ITS) is
> > attached, we will get the correct RID as seen by these.
> 
> There is a single "correct RID" for your builtin SATA and USB, but in
> general there is no single RID.

In case of external PCIe devices too, the RID (or aliases) seen to the
point where SMMU or ITS is connected (i.e, the bridge marked with flag
PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT) are correct.
 
> > > Stopping the iterator earlier happens to make the last alias be one of
> > > the correct ones, but it doesn't solve the problems of quirked devices
> > > that can use multiple requester IDs, and it doesn't solve the problem
> > > of PCIe-to-PCI bridges that optionally take ownership of transactions.
> >  
> > If these happen below the point where the SMMU is attached, we will
> > consider the last alias introduced, which should be ok. If they are
> > above, the alias introduced is not relevant.  Devices with multiple
> > aliases is not handled anywhere in ARM code, so I don't think we should
> > consider that here.
> 
> I think we *should* consider it here.  The multiple alias situation is
> generic PCIe, independent of ARM.  If you want to support arbitrary
> PCIe plugin devices, you have to handle it.  I think any device with a
> quirk that calls pci_add_dma_alias() will currently fail on your
> system.  And I think devices behind a bridge that optionally takes
> ownership of DMA transactions will also fail.
 
The issue that IOMMU code does not handle all aliases has to be addressed
separately (and Robin seems to be looking at this from his response).

And even when that is addressed, this quirk is still needed on ThunderX2,
as I pointed out above.

Hope I am still not missing the point, and thanks for your patience here.
JC.

^ permalink raw reply

* [PATCH] ARM: dts: imx53-qsrb: Pulldown PMIC IRQ pin
From: Fabio Estevam @ 2017-04-12 18:04 UTC (permalink / raw)
  To: linux-arm-kernel

Currently the following errors are seeing:

[   14.015056] mc13xxx 0-0008: Failed to read IRQ status: -6
[   27.321093] mc13xxx 0-0008: Failed to read IRQ status: -6
[   27.411681] mc13xxx 0-0008: Failed to read IRQ status: -6
[   27.456281] mc13xxx 0-0008: Failed to read IRQ status: -6
[   30.527106] mc13xxx 0-0008: Failed to read IRQ status: -6
[   36.596900] mc13xxx 0-0008: Failed to read IRQ status: -6

Also when reading the interrupts via 'cat /proc/interrupts' the
PMIC GPIO interrupt counter does not stop increasing.

The reason for the storm of interrupts is that the PUS field of
register IOMUXC_SW_PAD_CTL_PAD_CSI0_DAT5 is currently configured as:
10 : 100k pullup

and the PMIC interrupt is being registered as IRQ_TYPE_LEVEL_HIGH type,
which is the correct type as per the MC34708 datasheet.

Use the default power on value for the IOMUX, which sets PUS field as:
00: 360k pull down

This prevents the spurious PMIC interrupts from happening.

Commit e1ffceb078c6 ("ARM: imx53: qsrb: fix PMIC interrupt level")
correctly described the irq type as IRQ_TYPE_LEVEL_HIGH, but
missed to update the IOMUX of the PMIC GPIO to pull down.

Fixes: e1ffceb078c6 ("ARM: imx53: qsrb: fix PMIC interrupt level")
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/boot/dts/imx53-qsrb.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx53-qsrb.dts b/arch/arm/boot/dts/imx53-qsrb.dts
index de22158..4e103a9 100644
--- a/arch/arm/boot/dts/imx53-qsrb.dts
+++ b/arch/arm/boot/dts/imx53-qsrb.dts
@@ -23,7 +23,7 @@
 	imx53-qsrb {
 		pinctrl_pmic: pmicgrp {
 			fsl,pins = <
-				MX53_PAD_CSI0_DAT5__GPIO5_23	0x1e4 /* IRQ */
+				MX53_PAD_CSI0_DAT5__GPIO5_23	0x1c4 /* IRQ */
 			>;
 		};
 	};
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/2] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup
From: Sinan Kaya @ 2017-04-12 18:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412172623.GA11570@red-moon>

On 4/12/2017 1:26 PM, Lorenzo Pieralisi wrote:
>> +	if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) {
>> +		/* preserve existing resource assignment */
>> +		pci_bus_claim_resources(bus);
> Ok. By the PCI FW specs, we should also assign unassigned resources here
> (ie resources that can't be claimed). Furthermore, by the PCI FW spec,
> if !obj this is the path we should be taking (PCI firmware specification
> Rev 3.1, 4.6.5, Description: 0:)
> 
> "...This situation is the same as the legacy situation where this
> _DSM is not provided."
> 
> which makes me think that the PCI FW specification expects FW set-up
> to be claimed on boot, it is my interpretation.
> 
> I wonder how many UEFI developers know this _DSM even exists. If we
> want to enforce it at least we should mandate its usage at SBSA level,
> it is a major change and I want to understand the reasons behind it,
> so far, as I said, I may see why this was needed on x86 but on ARM64
> I really don't.

IMO, DSMs are always considered optional to enable additional features
in the operating system that otherwise are not required or are not defined
in any of the PCI specs. 

I think we are abusing the DSM here. We want to require the presence of
a DSM but we want to require it to be 0 in order to have a UEFI
compatible behavior. 

I think we need to turn it the other way around by having a UEFI compatible
behavior and do reassign only if this DSM is 1.

I understand that you are worried about regressions. We can try to fix
it however time it takes before merging this.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode
From: Sinan Kaya @ 2017-04-12 17:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <855540a9-888d-6419-3add-d8b0dbb43029@codeaurora.org>

On 4/12/2017 1:27 PM, Sinan Kaya wrote:
> I am not a big fan of this. I wonder if we could not make it DT/ACPI
> agnostic by simply setting the numa_node in the pci_bus->dev field,
> and propagate it downstream a PCI hierarcy (as we do with sysdata)
> in pci_alloc_child_bus().

I didn't realize that we were pushing sysdata to the child buses. No
issues then. 

I was under the impression that sysdata only existed for the host bridge
itself and was not shared with the rest of the bus.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH v5 8/8] ARM: ux500: Add vendor prefix to tps61052 node
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>

The tps61052 device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Tony Lindgren <tony@atomide.com>

---

Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/ste-hrefprev60.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/ste-hrefprev60.dtsi b/arch/arm/boot/dts/ste-hrefprev60.dtsi
index 5882a2606ac3..3f14b4df69b4 100644
--- a/arch/arm/boot/dts/ste-hrefprev60.dtsi
+++ b/arch/arm/boot/dts/ste-hrefprev60.dtsi
@@ -30,7 +30,7 @@
 
 		i2c at 80004000 {
 			tps61052 at 33 {
-				compatible = "tps61052";
+				compatible = "ti,tps61052";
 				reg = <0x33>;
 			};
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 4/8] ARM: dts: n8x0: Add vendor prefix to retu node
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>

The retu device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>

---

Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index 1b06430c8013..91886231e5a8 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -16,7 +16,7 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			retu: retu at 1 {
-				compatible = "retu";
+				compatible = "nokia,retu";
 				interrupt-parent = <&gpio4>;
 				interrupts = <12 IRQ_TYPE_EDGE_RISING>;
 				reg = <0x1>;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 2/8] mfd: retu: Drop -mfd suffix from I2C device ID name
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>

It's not correct to encode the subsystem in the I2C device name, so
drop the -mfd suffix. To maintain bisect-ability, change driver and
platform code / DTS users in the same patch.

Suggested-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>

---

Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 4 ++--
 arch/arm/mach-omap1/board-nokia770.c        | 4 ++--
 drivers/mfd/retu-mfd.c                      | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index 7e5ffc583c90..1b06430c8013 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -15,8 +15,8 @@
 				>;
 			#address-cells = <1>;
 			#size-cells = <0>;
-			retu_mfd: retu at 1 {
-				compatible = "retu-mfd";
+			retu: retu at 1 {
+				compatible = "retu";
 				interrupt-parent = <&gpio4>;
 				interrupts = <12 IRQ_TYPE_EDGE_RISING>;
 				reg = <0x1>;
diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
index ee8d9f553db4..06243c0b12d2 100644
--- a/arch/arm/mach-omap1/board-nokia770.c
+++ b/arch/arm/mach-omap1/board-nokia770.c
@@ -233,10 +233,10 @@ static struct platform_device nokia770_cbus_device = {
 
 static struct i2c_board_info nokia770_i2c_board_info_2[] __initdata = {
 	{
-		I2C_BOARD_INFO("retu-mfd", 0x01),
+		I2C_BOARD_INFO("retu", 0x01),
 	},
 	{
-		I2C_BOARD_INFO("tahvo-mfd", 0x02),
+		I2C_BOARD_INFO("tahvo", 0x02),
 	},
 };
 
diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
index d4c114abeb75..53e1d386d2c0 100644
--- a/drivers/mfd/retu-mfd.c
+++ b/drivers/mfd/retu-mfd.c
@@ -302,8 +302,8 @@ static int retu_remove(struct i2c_client *i2c)
 }
 
 static const struct i2c_device_id retu_id[] = {
-	{ "retu-mfd", 0 },
-	{ "tahvo-mfd", 0 },
+	{ "retu", 0 },
+	{ "tahvo", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, retu_id);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 0/8] mfd: Add OF device table to I2C drivers that are missing it
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

This series add OF device ID tables to mfd I2C drivers whose devices are
either used in Device Tree source files or are listed in binding docs as
a compatible string.

That's done because the plan is to change the I2C core to report proper OF
modaliases instead of always reporting a MODALIAS=i2c:<foo> regardless if
a device was registered via DT or using the legacy platform data mechanism.

So these patches will make sure that mfd I2C drivers modules will continue
to be autoloaded once the I2C core is changed to report proper OF modalias.

Users didn't have a vendor prefix in the used compatible strings, but since
there wasn't a DT binding document for these drivers, it can be said that
were working for mere luck and so this series fixes the users and add a DT
binding doc for the drivers.

Most patches can be applied independently, with the exception of patches
2 to 4 that should be applied in the same tree to keep bisect-ability. I
suggest these to go through the MFD subsystem tree.

Best regards,
Javier

Changes in v5:
- Add missing properties for interrupts to DT binding doc (Rob Herring).
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.

Changes in v4:
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
- Drop -mfd suffix in compatible string (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).

Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
- Add a vendor prefix to the compatible string (Rob Herring).

Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.

Javier Martinez Canillas (8):
  dt-bindings: mfd: Add retu/tahvo ASIC chips bindings
  mfd: retu: Drop -mfd suffix from I2C device ID name
  mfd: retu: Add OF device ID table
  ARM: dts: n8x0: Add vendor prefix to retu node
  i2c: i2c-cbus-gpio: Add vendor prefix to retu node in example
  dt-bindings: mfd: Add TI tps6105x chip bindings
  mfd: tps6105x: Add OF device ID table
  ARM: ux500: Add vendor prefix to tps61052 node

 .../devicetree/bindings/i2c/i2c-cbus-gpio.txt      |  4 ++--
 Documentation/devicetree/bindings/mfd/retu.txt     | 23 ++++++++++++++++++++++
 Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 ++++++++++++++++
 arch/arm/boot/dts/omap2420-n8x0-common.dtsi        |  4 ++--
 arch/arm/boot/dts/ste-hrefprev60.dtsi              |  2 +-
 arch/arm/mach-omap1/board-nokia770.c               |  4 ++--
 drivers/mfd/retu-mfd.c                             | 12 +++++++++--
 drivers/mfd/tps6105x.c                             |  8 ++++++++
 8 files changed, 65 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt

-- 
2.9.3

^ permalink raw reply

* [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode
From: Sinan Kaya @ 2017-04-12 17:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412170433.GA7412@red-moon>

Hi Lorenzo,

On 4/12/2017 1:04 PM, Lorenzo Pieralisi wrote:
> On Sat, Apr 08, 2017 at 01:17:32PM -0400, Sinan Kaya wrote:
>> On 12/31/1969 7:00 PM, linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Sergey Temerkhanov wrote:
>>>  int pcibus_to_node(struct pci_bus *bus)  {
>>> -	return dev_to_node(&bus->dev);
>>> +	struct pci_config_window *cfg = bus->sysdata;
>>> +	struct acpi_device *adev = NULL;
>>> +	struct device *dev;
>>> +
>>> +	if (!acpi_disabled)
>>> +		adev = to_acpi_device(cfg->parent);
>>> +
>>
>> I see a problem here that NUMA node information is read from the
>> parent device.  PCI bus can have multiple levels of switches and
>> bridges. The NUMA information is only present on the host bridge.
>>
>> This code only works if the endpoint is directly connected to the root
>> port. 
> 
> That's not what this code does. This code retrieves the struct device
> backing the ACPI device representing the PNP0A08 host bridge and its
> dev->numa_node value (that was set in pci_acpi_scan_root()).

pcibus_to_node() gets called from two places. The first place is 
pci_register_host_bridge() like you mentioned. 

The second place is pci_device_add(). pci_device_add() gets called for
every single device that is getting added to the system. 

> 
> I am not a big fan of this. I wonder if we could not make it DT/ACPI
> agnostic by simply setting the numa_node in the pci_bus->dev field,
> and propagate it downstream a PCI hierarcy (as we do with sysdata)
> in pci_alloc_child_bus().

Yeah, that's what I was going to ask next about how other arches solve
this. Common solution to DT/ACPI is preferable as the issue exists in
both places unless DT has some other mechanism hidden behind the scenes.

> 
> This way pcibus_to_node() would become straightforward (ie as it
> is now - provided the above is doable):
> 
> dev_to_node(&bus->dev);
> 
> This is suspiciously similar to the domain number song and dance
> except that the NUMA node now is in the struct pci_bus->dev->numa_node
> instead of struct pci_bus->domain_nr.

I think this looks like the _CCA dance we are doing.

> 
> Lorenzo
> 

Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH 2/2] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup
From: Lorenzo Pieralisi @ 2017-04-12 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170411163313.18577-3-ard.biesheuvel@linaro.org>

On Tue, Apr 11, 2017 at 05:33:13PM +0100, Ard Biesheuvel wrote:
> On arm64 ACPI systems, we unconditionally reconfigure the entire PCI
> hierarchy at boot. This is a departure from what is customary on ACPI
> systems, and may break assumptions in some places (e.g., EFIFB), that
> the kernel will leave BARs of enabled PCI devices where they are.
> 
> Given that PCI already specifies a device specific ACPI method (_DSM)
> for PCI root bridge nodes that tells us whether the firmware thinks
> the configuration should be left alone, let's sidestep the entire
> policy debate about whether the PCI configuration should be preserved
> or not, and put it under the control of the firmware instead.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/kernel/pci.c  | 20 ++++++++++++++++++--
>  include/linux/pci-acpi.h |  7 ++++---
>  2 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 4f0e3ebfea4b..c88e07e2eb84 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -185,6 +185,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	struct acpi_pci_generic_root_info *ri;
>  	struct pci_bus *bus, *child;
>  	struct acpi_pci_root_ops *root_ops;
> +	union acpi_object *obj;
>  
>  	ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>  	if (!ri)
> @@ -208,8 +209,23 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	if (!bus)
>  		return NULL;
>  
> -	pci_bus_size_bridges(bus);
> -	pci_bus_assign_resources(bus);
> +	/*
> +	 * Invoke the PCI device specific method (_DSM) #5 'Ignore PCI Boot
> +	 * Configuration', which tells us whether the firmware wants us to
> +	 * preserve the configuration of the PCI resource tree for this root
> +	 * bridge.
> +	 */
> +	obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), pci_acpi_dsm_uuid, 1,
> +				IGNORE_PCI_BOOT_CONFIG_DSM, NULL);
> +	if (obj && obj->type == ACPI_TYPE_INTEGER && obj->integer.value == 0) {
> +		/* preserve existing resource assignment */
> +		pci_bus_claim_resources(bus);

Ok. By the PCI FW specs, we should also assign unassigned resources here
(ie resources that can't be claimed). Furthermore, by the PCI FW spec,
if !obj this is the path we should be taking (PCI firmware specification
Rev 3.1, 4.6.5, Description: 0:)

"...This situation is the same as the legacy situation where this
_DSM is not provided."

which makes me think that the PCI FW specification expects FW set-up
to be claimed on boot, it is my interpretation.

I wonder how many UEFI developers know this _DSM even exists. If we
want to enforce it at least we should mandate its usage at SBSA level,
it is a major change and I want to understand the reasons behind it,
so far, as I said, I may see why this was needed on x86 but on ARM64
I really don't.

Lorenzo

> +	} else {
> +		/* reconfigure the resource tree from scratch */
> +		pci_bus_size_bridges(bus);
> +		pci_bus_assign_resources(bus);
> +	}
> +	ACPI_FREE(obj);
>  
>  	list_for_each_entry(child, &bus->children, node)
>  		pcie_bus_configure_settings(child);
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 7a4e83a8c89c..308111489b83 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -106,9 +106,10 @@ static inline void acpiphp_check_host_bridge(struct acpi_device *adev) { }
>  #endif
>  
>  extern const u8 pci_acpi_dsm_uuid[];
> -#define DEVICE_LABEL_DSM	0x07
> -#define RESET_DELAY_DSM		0x08
> -#define FUNCTION_DELAY_DSM	0x09
> +#define IGNORE_PCI_BOOT_CONFIG_DSM	0x05
> +#define DEVICE_LABEL_DSM		0x07
> +#define RESET_DELAY_DSM			0x08
> +#define FUNCTION_DELAY_DSM		0x09
>  
>  #else	/* CONFIG_ACPI */
>  static inline void acpi_pci_add_bus(struct pci_bus *bus) { }
> -- 
> 2.9.3
> 

^ permalink raw reply

* [PATCH v5 1/4] gpio: mvebu: Add limited PWM support
From: Thierry Reding @ 2017-04-12 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170409180931.4884-2-ralph.sennhauser@gmail.com>

On Sun, Apr 09, 2017 at 08:09:27PM +0200, Ralph Sennhauser wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> Armada 370/XP devices can 'blink' GPIO lines with a configurable on
> and off period. This can be modelled as a PWM.
> 
> However, there are only two sets of PWM configuration registers for
> all the GPIO lines. This driver simply allows a single GPIO line per
> GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
> EBUSY.
> 
> Due to the interleaving of registers it is not simple to separate the
> PWM driver from the GPIO driver. Thus the GPIO driver has been
> extended with a PWM driver.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> URL: https://patchwork.ozlabs.org/patch/427287/
> URL: https://patchwork.ozlabs.org/patch/427295/
> [Ralph Sennhauser:
>   * Port forward
>   * Merge PWM portion into gpio-mvebu.c
>   * Switch to atomic PWM API
>   * Add new compatible string marvell,armada-370-xp-gpio
>   * Update and merge documentation patch
>   * Update MAINTAINERS]
> Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  .../devicetree/bindings/gpio/gpio-mvebu.txt        |  32 ++
>  MAINTAINERS                                        |   2 +
>  drivers/gpio/gpio-mvebu.c                          | 324 ++++++++++++++++++++-
>  3 files changed, 346 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> index a6f3bec..fe49e9d 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> @@ -38,6 +38,24 @@ Required properties:
>  - #gpio-cells: Should be two. The first cell is the pin number. The
>    second cell is reserved for flags, unused at the moment.
>  
> +Optional properties:
> +
> +In order to use the gpio lines in PWM mode, some additional optional
> +properties are required. Only Armada 370 and XP support these properties.
> +
> +- compatible: Must contain "marvell,armada-370-xp-gpio"
> +
> +- reg: an additional register set is needed, for the GPIO Blink
> +  Counter on/off registers.
> +
> +- reg-names: Must contain an entry "pwm" corresponding to the
> +  additional register range needed for pwm operation.
> +
> +- #pwm-cells: Should be two. The first cell is the GPIO line number. The
> +  second cell is the period in nanoseconds.
> +
> +- clocks: Must be a phandle to the clock for the gpio controller.

One other thing: there's a mix of pwm/PWM and gpio/GPIO in this hunk. In
prose, always use the all-uppercase variants because they are
abbreviations.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/d8af46fc/attachment.sig>

^ permalink raw reply

* [PATCH v5 0/4] gpio: mvebu: Add PWM fan support
From: Thierry Reding @ 2017-04-12 17:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170409180931.4884-1-ralph.sennhauser@gmail.com>

On Sun, Apr 09, 2017 at 08:09:26PM +0200, Ralph Sennhauser wrote:
> Hi Therry,
> 
> Resending this as v5 with some minor changes since v4. What is missing is
> an ACK from you so Linus can merge the driver and Gregory the dts
> changes. For this driver to make it into 4.12 it would be nice to have
> it in next soon. I hope you can make some room in your schedule to have
> another look at this series.
> 
> Thanks
> Ralph
> 
> ---
> 
> Notes:
> 
>   About npwm = 1:
>     The only way I can think of to achieve that requires reading the
>     GPIO line from the device tree. This would prevent a user to
>     dynamically choose a line. Which is fine for the fan found on Mamba
>     but let's take some development board with freely accessible GPIOs
>     and suddenly we limit the use of this driver. Given the above, npwm
>     = ngpio with only one usable at a time is a more accurate

I think "accurate" is perhaps not the word I'd choose. "npwm" is defined
as "number of PWMs controlled by this chip", and that's effectively just
the one. It's implied that all PWMs exposed by a chip can be used
concurrently.

Anyway, I can see how npwm = ngpio might be more convenient, and if that
is what you want to do, I don't feel strongly enough to object.

>     description of the situation. The only downside is some "wasted"
>     space.
> 
>   About the new compatible string:
>     Orion was chosen for the SoC variant for the same reason as in
>     commit 5f79c651e81e ("arm: mvebu: use global interrupts for GPIOs on
>     Armada XP").
>     The "pwm" property remains optional for the new compatible string so
>     the compatiple string "marvell,armada-370-xp-gpio" can be used by
>     all and not just the first two GPIO chips. A property to select "Set
>     A" / "Set B" registers could be invented though.
> 
> ---
> 
> Pending:
>   * Needs ACK from Thierry Reding to be merged via linux-gpio tree by Linus
>     Walleij. (fine with the general approach, requested changes which
>     should have been taken care of now)

As I said elsewhere, I haven't seen an Acked-by on the binding changes,
so that would be another pending item here.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/e719481c/attachment.sig>

^ permalink raw reply

* [PATCH v5 1/4] gpio: mvebu: Add limited PWM support
From: Thierry Reding @ 2017-04-12 17:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170409180931.4884-2-ralph.sennhauser@gmail.com>

On Sun, Apr 09, 2017 at 08:09:27PM +0200, Ralph Sennhauser wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> Armada 370/XP devices can 'blink' GPIO lines with a configurable on
> and off period. This can be modelled as a PWM.
> 
> However, there are only two sets of PWM configuration registers for
> all the GPIO lines. This driver simply allows a single GPIO line per
> GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
> EBUSY.
> 
> Due to the interleaving of registers it is not simple to separate the
> PWM driver from the GPIO driver. Thus the GPIO driver has been
> extended with a PWM driver.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> URL: https://patchwork.ozlabs.org/patch/427287/
> URL: https://patchwork.ozlabs.org/patch/427295/
> [Ralph Sennhauser:
>   * Port forward
>   * Merge PWM portion into gpio-mvebu.c
>   * Switch to atomic PWM API
>   * Add new compatible string marvell,armada-370-xp-gpio
>   * Update and merge documentation patch
>   * Update MAINTAINERS]
> Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  .../devicetree/bindings/gpio/gpio-mvebu.txt        |  32 ++
>  MAINTAINERS                                        |   2 +
>  drivers/gpio/gpio-mvebu.c                          | 324 ++++++++++++++++++++-
>  3 files changed, 346 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> index a6f3bec..fe49e9d 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> @@ -38,6 +38,24 @@ Required properties:
>  - #gpio-cells: Should be two. The first cell is the pin number. The
>    second cell is reserved for flags, unused at the moment.
>  
> +Optional properties:
> +
> +In order to use the gpio lines in PWM mode, some additional optional
> +properties are required. Only Armada 370 and XP support these properties.
> +
> +- compatible: Must contain "marvell,armada-370-xp-gpio"
> +
> +- reg: an additional register set is needed, for the GPIO Blink
> +  Counter on/off registers.
> +
> +- reg-names: Must contain an entry "pwm" corresponding to the
> +  additional register range needed for pwm operation.
> +
> +- #pwm-cells: Should be two. The first cell is the GPIO line number. The
> +  second cell is the period in nanoseconds.
> +
> +- clocks: Must be a phandle to the clock for the gpio controller.
> +
>  Example:
>  
>  		gpio0: gpio at d0018100 {
> @@ -51,3 +69,17 @@ Example:
>  			#interrupt-cells = <2>;
>  			interrupts = <16>, <17>, <18>, <19>;
>  		};
> +
> +		gpio1: gpio at 18140 {
> +			compatible = "marvell,armada-370-xp-gpio";
> +			reg = <0x18140 0x40>, <0x181c8 0x08>;
> +			reg-names = "gpio", "pwm";
> +			ngpios = <17>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			#pwm-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			interrupts = <87>, <88>, <89>;
> +			clocks = <&coreclk 0>;
> +		};

This is going to need an Acked-by from one of the device tree
maintainers. Rob and devicetree at vger.kernel.org are on Cc, but I suspect
nobody might look for the binding change "hidden" in this patch.

Maybe best to split this off into a separate patch, or explicitly ping
Rob to look at this patch.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 58b3a22..19382f5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10295,6 +10295,8 @@ F:	include/linux/pwm.h
>  F:	drivers/pwm/
>  F:	drivers/video/backlight/pwm_bl.c
>  F:	include/linux/pwm_backlight.h
> +F:	drivers/gpio/gpio-mvebu.c
> +F:	Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
>  
>  PXA2xx/PXA3xx SUPPORT
>  M:	Daniel Mack <daniel@zonque.org>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index fae4db6..e310951 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -42,22 +42,34 @@
>  #include <linux/io.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_device.h>
> +#include <linux/pwm.h>
>  #include <linux/clk.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/irqchip/chained_irq.h>
> +#include <linux/platform_device.h>
>  #include <linux/bitops.h>
>  
> +#include "gpiolib.h"
> +
>  /*
>   * GPIO unit register offsets.
>   */
> -#define GPIO_OUT_OFF		0x0000
> -#define GPIO_IO_CONF_OFF	0x0004
> -#define GPIO_BLINK_EN_OFF	0x0008
> -#define GPIO_IN_POL_OFF		0x000c
> -#define GPIO_DATA_IN_OFF	0x0010
> -#define GPIO_EDGE_CAUSE_OFF	0x0014
> -#define GPIO_EDGE_MASK_OFF	0x0018
> -#define GPIO_LEVEL_MASK_OFF	0x001c
> +#define GPIO_OUT_OFF			0x0000
> +#define GPIO_IO_CONF_OFF		0x0004
> +#define GPIO_BLINK_EN_OFF		0x0008
> +#define GPIO_IN_POL_OFF			0x000c
> +#define GPIO_DATA_IN_OFF		0x0010
> +#define GPIO_EDGE_CAUSE_OFF		0x0014
> +#define GPIO_EDGE_MASK_OFF		0x0018
> +#define GPIO_LEVEL_MASK_OFF		0x001c
> +#define GPIO_BLINK_CNT_SELECT_OFF	0x0020
> +
> +/*
> + * PWM register offsets.
> + */
> +#define PWM_BLINK_ON_DURATION_OFF	0x0
> +#define PWM_BLINK_OFF_DURATION_OFF	0x4
> +
>  
>  /* The MV78200 has per-CPU registers for edge mask and level mask */
>  #define GPIO_EDGE_MASK_MV78200_OFF(cpu)	  ((cpu) ? 0x30 : 0x18)
> @@ -78,6 +90,20 @@
>  
>  #define MVEBU_MAX_GPIO_PER_BANK		32
>  
> +struct mvebu_pwm {
> +	void __iomem		*membase;
> +	unsigned long		 clk_rate;
> +	bool			 used;

I think you could've probably made this a little simpler by making bool
used into struct gpio_desc *gpio; and reference that rather than having
to convert with gpio_to_desc(). More on why I think that's important
below.

> +	struct pwm_chip		 chip;
> +	spinlock_t		 lock;
> +	struct mvebu_gpio_chip	*mvchip;
> +
> +	/* Used to preserve GPIO/PWM registers across suspend/resume */
> +	u32			 blink_select;
> +	u32			 blink_on_duration;
> +	u32			 blink_off_duration;
> +};
> +
>  struct mvebu_gpio_chip {
>  	struct gpio_chip   chip;
>  	spinlock_t	   lock;
> @@ -87,6 +113,10 @@ struct mvebu_gpio_chip {
>  	struct irq_domain *domain;
>  	int		   soc_variant;
>  
> +	/* Used for PWM support */
> +	struct clk	  *clk;
> +	struct mvebu_pwm  *mvpwm;
> +
>  	/* Used to preserve GPIO registers across suspend/resume */
>  	u32		   out_reg;
>  	u32		   io_conf_reg;
> @@ -110,6 +140,12 @@ static void __iomem *mvebu_gpioreg_blink(struct mvebu_gpio_chip *mvchip)
>  	return mvchip->membase + GPIO_BLINK_EN_OFF;
>  }
>  
> +static void __iomem *mvebu_gpioreg_blink_counter_select(struct mvebu_gpio_chip
> +							*mvchip)
> +{
> +	return mvchip->membase + GPIO_BLINK_CNT_SELECT_OFF;
> +}
> +
>  static void __iomem *mvebu_gpioreg_io_conf(struct mvebu_gpio_chip *mvchip)
>  {
>  	return mvchip->membase + GPIO_IO_CONF_OFF;
> @@ -181,6 +217,20 @@ static void __iomem *mvebu_gpioreg_level_mask(struct mvebu_gpio_chip *mvchip)
>  }
>  
>  /*
> + * Functions returning addresses of individual registers for a given
> + * PWM controller.
> + */
> +static void __iomem *mvebu_pwmreg_blink_on_duration(struct mvebu_pwm *mvpwm)
> +{
> +	return mvpwm->membase + PWM_BLINK_ON_DURATION_OFF;
> +}
> +
> +static void __iomem *mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *mvpwm)
> +{
> +	return mvpwm->membase + PWM_BLINK_OFF_DURATION_OFF;
> +}
> +
> +/*
>   * Functions implementing the gpio_chip methods
>   */
>  static void mvebu_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
> @@ -484,6 +534,243 @@ static void mvebu_gpio_irq_handler(struct irq_desc *desc)
>  	chained_irq_exit(chip, desc);
>  }
>  
> +/*
> + * Functions implementing the pwm_chip methods
> + */
> +static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip *chip)
> +{
> +	return container_of(chip, struct mvebu_pwm, chip);
> +}
> +
> +static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> +	struct gpio_desc *desc = gpio_to_desc(pwm->pwm);

This assumes that the GPIO chip base and the PWM chip base are the same.
You make sure of that by setting the PWM chip base to be the same as the
GPIO chip base, which is the easy way out. It's also somewhat brittle
because some other PWM chip may have occupied the region that you want
to use. It's fairly unlikely, but I think you can very easily side-step
any issues by simply doing:

	struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;

	desc = gpio_to_desc(mvchip->chip.base + pwm->hwpwm);

Now that's somewhat complicated, but you only have to do it once if you
store the desc in mvpwm->gpio as pointed out above.

> +	unsigned long flags;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&mvpwm->lock, flags);
> +	if (mvpwm->used) {
> +		ret = -EBUSY;

Then you can also easily use mvpwm->gpio to check for -EBUSY here.

> +	} else {
> +		if (!desc) {
> +			ret = -ENODEV;
> +			goto out;
> +		}
> +		ret = gpiod_request(desc, "mvebu-pwm");
> +		if (ret)
> +			goto out;
> +
> +		ret = gpiod_direction_output(desc, 0);
> +		if (ret) {
> +			gpiod_free(desc);
> +			goto out;
> +		}
> +
> +		mvpwm->used = true;
> +	}
> +
> +out:
> +	spin_unlock_irqrestore(&mvpwm->lock, flags);
> +	return ret;
> +}
> +
> +static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> +	struct gpio_desc *desc = gpio_to_desc(pwm->pwm);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&mvpwm->lock, flags);
> +	gpiod_free(desc);
> +	mvpwm->used = false;
> +	spin_unlock_irqrestore(&mvpwm->lock, flags);
> +}
> +
> +static void mvebu_pwm_get_state(struct pwm_chip *chip,
> +				struct pwm_device *pwm,
> +				struct pwm_state *state) {
> +
> +	struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> +	struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
> +	unsigned long long val;
> +	unsigned long flags;
> +	u32 u;
> +
> +	spin_lock_irqsave(&mvpwm->lock, flags);
> +
> +	val = (unsigned long long)
> +		readl_relaxed(mvebu_pwmreg_blink_on_duration);
> +	val *= NSEC_PER_SEC;
> +	do_div(val, mvpwm->clk_rate);
> +	if (val > UINT_MAX)
> +		state->duty_cycle = UINT_MAX;
> +	else if (val)
> +		state->duty_cycle = val;
> +	else
> +		state->duty_cycle = 1;
> +
> +	val = (unsigned long long)
> +		readl_relaxed(mvebu_pwmreg_blink_off_duration);
> +	val *= NSEC_PER_SEC;
> +	do_div(val, mvpwm->clk_rate);
> +	if (val < state->duty_cycle) {
> +		state->period = 1;
> +	} else {
> +		val -= state->duty_cycle;
> +		if (val > UINT_MAX)
> +			state->period = UINT_MAX;
> +		else if (val)
> +			state->period = val;
> +		else
> +			state->period = 1;
> +	}
> +
> +	u = readl_relaxed(mvebu_gpioreg_blink(mvchip));
> +	if (u)
> +		state->enabled = true;
> +	else
> +		state->enabled = false;
> +
> +	spin_unlock_irqrestore(&mvpwm->lock, flags);
> +}
> +
> +static int mvebu_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			   struct pwm_state *state)
> +{
> +	struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> +	struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
> +	unsigned long long val;
> +	unsigned long flags;
> +	unsigned int on, off;
> +
> +	val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
> +	do_div(val, NSEC_PER_SEC);
> +	if (val > UINT_MAX)
> +		return -EINVAL;
> +	if (val)
> +		on = val;
> +	else
> +		on = 1;
> +
> +	val = (unsigned long long) mvpwm->clk_rate *
> +		(state->period - state->duty_cycle);
> +	do_div(val, NSEC_PER_SEC);
> +	if (val > UINT_MAX)
> +		return -EINVAL;
> +	if (val)
> +		off = val;
> +	else
> +		off = 1;
> +
> +	spin_lock_irqsave(&mvpwm->lock, flags);
> +
> +	writel_relaxed(on, mvebu_pwmreg_blink_on_duration(mvpwm));
> +	writel_relaxed(off, mvebu_pwmreg_blink_off_duration(mvpwm));
> +	if (state->enabled)
> +		mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 1);
> +	else
> +		mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 0);
> +
> +	spin_unlock_irqrestore(&mvpwm->lock, flags);
> +
> +	return 0;
> +}
> +
> +static const struct pwm_ops mvebu_pwm_ops = {
> +	.request = mvebu_pwm_request,
> +	.free = mvebu_pwm_free,
> +	.get_state = mvebu_pwm_get_state,
> +	.apply = mvebu_pwm_apply,
> +	.owner = THIS_MODULE,
> +};
> +
> +static void __maybe_unused mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip)
> +{
> +	struct mvebu_pwm *mvpwm = mvchip->mvpwm;
> +
> +	mvpwm->blink_select =
> +		readl_relaxed(mvebu_gpioreg_blink_counter_select(mvchip));
> +	mvpwm->blink_on_duration =
> +		readl_relaxed(mvebu_pwmreg_blink_on_duration(mvpwm));
> +	mvpwm->blink_off_duration =
> +		readl_relaxed(mvebu_pwmreg_blink_off_duration(mvpwm));
> +}
> +
> +static void __maybe_unused mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip)
> +{
> +	struct mvebu_pwm *mvpwm = mvchip->mvpwm;
> +
> +	writel_relaxed(mvpwm->blink_select,
> +		       mvebu_gpioreg_blink_counter_select(mvchip));
> +	writel_relaxed(mvpwm->blink_on_duration,
> +		       mvebu_pwmreg_blink_on_duration(mvpwm));
> +	writel_relaxed(mvpwm->blink_off_duration,
> +		       mvebu_pwmreg_blink_off_duration(mvpwm));
> +}
> +
> +static int mvebu_pwm_probe(struct platform_device *pdev,
> +			   struct mvebu_gpio_chip *mvchip,
> +			   int id)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mvebu_pwm *mvpwm;
> +	struct resource *res;
> +
> +	if (!of_device_is_compatible(mvchip->chip.of_node,
> +				     "marvell,armada-370-xp-gpio"))
> +		return 0;
> +	/*
> +	 * There are only two sets of PWM configuration registers for
> +	 * all the GPIO lines on those SoCs which this driver reserves
> +	 * for the first two GPIO chips. So if the resource is missing
> +	 * we can't treat it as an error.
> +	 */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm");
> +	if (!res)
> +		return 0;
> +
> +	/*
> +	 * Use set A for lines of GPIO chip with id 0, B for GPIO chip
> +	 * with id 1. Don't allow further GPIO chips to be used for PWM.
> +	 */
> +	if (id == 0)
> +		writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
> +	else if (id == 1)
> +		writel_relaxed(U32_MAX,
> +			       mvebu_gpioreg_blink_counter_select(mvchip));

You could've just set a variable and then call writel_relaxed() after
the return -EINVAL below.

> +	else
> +		return -EINVAL;
> +
> +	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
> +	if (!mvpwm)
> +		return -ENOMEM;
> +	mvchip->mvpwm = mvpwm;
> +	mvpwm->mvchip = mvchip;
> +
> +	mvpwm->membase = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(mvpwm->membase))
> +		return PTR_ERR(mvpwm->membase);
> +
> +	if (IS_ERR(mvchip->clk))
> +		return PTR_ERR(mvchip->clk);

Maybe do this much earlier to avoid all the unnecessary register
accesses, allocations and mappings?

> +
> +	mvpwm->clk_rate = clk_get_rate(mvchip->clk);
> +	if (!mvpwm->clk_rate) {
> +		dev_err(dev, "failed to get clock rate\n");
> +		return -EINVAL;
> +	}
> +
> +	mvpwm->chip.dev = dev;
> +	mvpwm->chip.ops = &mvebu_pwm_ops;
> +	mvpwm->chip.base = mvchip->chip.base;
> +	mvpwm->chip.npwm = mvchip->chip.ngpio;

I still would've done this differently. If you use this with a PWM user
you have to hook it up via DT anyway, so it doesn't matter whether you
specify the PWM index or the GPIO via some other property. The _only_
use-case where this might actually be an advantage is if you request a
PWM via the sysfs interface.

> +	spin_lock_init(&mvpwm->lock);
> +
> +	return pwmchip_add(&mvpwm->chip);
> +}
> +
>  #ifdef CONFIG_DEBUG_FS
>  #include <linux/seq_file.h>
>  
> @@ -555,6 +842,10 @@ static const struct of_device_id mvebu_gpio_of_match[] = {
>  		.data	    = (void *) MVEBU_GPIO_SOC_VARIANT_ARMADAXP,
>  	},
>  	{
> +		.compatible = "marvell,armada-370-xp-gpio",
> +		.data	    = (void *) MVEBU_GPIO_SOC_VARIANT_ORION,
> +	},
> +	{
>  		/* sentinel */
>  	},
>  };
> @@ -600,6 +891,9 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)
>  		BUG();
>  	}
>  
> +	if (IS_ENABLED(CONFIG_PWM))
> +		mvebu_pwm_suspend(mvchip);
> +
>  	return 0;
>  }
>  
> @@ -643,6 +937,9 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
>  		BUG();
>  	}
>  
> +	if (IS_ENABLED(CONFIG_PWM))
> +		mvebu_pwm_resume(mvchip);
> +
>  	return 0;
>  }
>  
> @@ -654,7 +951,6 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct irq_chip_generic *gc;
>  	struct irq_chip_type *ct;
> -	struct clk *clk;
>  	unsigned int ngpios;
>  	bool have_irqs;
>  	int soc_variant;
> @@ -688,10 +984,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
>  		return id;
>  	}
>  
> -	clk = devm_clk_get(&pdev->dev, NULL);
> +	mvchip->clk = devm_clk_get(&pdev->dev, NULL);
>  	/* Not all SoCs require a clock.*/
> -	if (!IS_ERR(clk))
> -		clk_prepare_enable(clk);
> +	if (!IS_ERR(mvchip->clk))
> +		clk_prepare_enable(mvchip->clk);
>  
>  	mvchip->soc_variant = soc_variant;
>  	mvchip->chip.label = dev_name(&pdev->dev);
> @@ -822,6 +1118,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
>  						 mvchip);
>  	}
>  
> +	/* Armada 370/XP has simple PWM support for GPIO lines */
> +	if (IS_ENABLED(CONFIG_PWM))
> +		return mvebu_pwm_probe(pdev, mvchip, id);
> +
>  	return 0;
>  
>  err_domain:
> -- 
> 2.10.2

All of my comments are effectively of a bikeshed nature, so from a PWM
perspective this is:

Acked-by: Thierry Reding <thierry.reding@gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/5fdbbbe3/attachment-0001.sig>

^ permalink raw reply

* [RFC PATCH 0/3] arm64: queued spinlocks and rw-locks
From: Adam Wallis @ 2017-04-12 17:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1491860104-4103-1-git-send-email-ynorov@caviumnetworks.com>

On 4/10/2017 5:35 PM, Yury Norov wrote:
> The patch of Jan Glauber enables queued spinlocks on arm64. I rebased it on
> latest kernel sources, and added a couple of fixes to headers to apply it 
> smoothly.
> 
> Though, locktourture test shows significant performance degradation in the
> acquisition of rw-lock for read on qemu:
> 
>                           Before           After
> spin_lock-torture:      38957034        37076367         -4.83
> rw_lock-torture W:       5369471        18971957        253.33
> rw_lock-torture R:       6413179         3668160        -42.80
> 

On our 48 core QDF2400 part, I am seeing huge improvements with these patches on
the torture tests. The improvements go up even further when I apply Jason Low's
MCS Spinlock patch: https://lkml.org/lkml/2016/4/20/725

> I'm  not much experienced in locking, and so wonder how it's possible that
> simple switching to generic queued rw-lock causes so significant performance
> degradation, while in theory it should improve it. Even more, on x86 there
> are no such problems probably.
> 
> I also think that patches 1 and 2 are correct and useful, and should be applied
> anyway.
> 
> Any comments appreciated.
> 
> Yury.
> 

I will be happy to tests these patches more thoroughly after you get some
additional comments/feedback.

> Jan Glauber (1):
>   arm64/locking: qspinlocks and qrwlocks support
> 
> Yury Norov (2):
>   kernel/locking: #include <asm/spinlock.h> in qrwlock.c
>   asm-generic: don't #include <linux/atomic.h> in qspinlock_types.h
> 
>  arch/arm64/Kconfig                      |  2 ++
>  arch/arm64/include/asm/qrwlock.h        |  7 +++++++
>  arch/arm64/include/asm/qspinlock.h      | 20 ++++++++++++++++++++
>  arch/arm64/include/asm/spinlock.h       | 12 ++++++++++++
>  arch/arm64/include/asm/spinlock_types.h | 14 +++++++++++---
>  include/asm-generic/qspinlock.h         |  1 +
>  include/asm-generic/qspinlock_types.h   |  8 --------
>  kernel/locking/qrwlock.c                |  1 +
>  8 files changed, 54 insertions(+), 11 deletions(-)
>  create mode 100644 arch/arm64/include/asm/qrwlock.h
>  create mode 100644 arch/arm64/include/asm/qspinlock.h
> 

Thanks

-- 
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH] arm64: acpi: Support PCI devices numa_node property in ACPI mode
From: Lorenzo Pieralisi @ 2017-04-12 17:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0dada57f-4215-4ced-482d-88203857e06d@codeaurora.org>

On Sat, Apr 08, 2017 at 01:17:32PM -0400, Sinan Kaya wrote:
> On 12/31/1969 7:00 PM, linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Sergey Temerkhanov wrote:
> >  int pcibus_to_node(struct pci_bus *bus)  {
> > -	return dev_to_node(&bus->dev);
> > +	struct pci_config_window *cfg = bus->sysdata;
> > +	struct acpi_device *adev = NULL;
> > +	struct device *dev;
> > +
> > +	if (!acpi_disabled)
> > +		adev = to_acpi_device(cfg->parent);
> > +
> 
> I see a problem here that NUMA node information is read from the
> parent device.  PCI bus can have multiple levels of switches and
> bridges. The NUMA information is only present on the host bridge.
> 
> This code only works if the endpoint is directly connected to the root
> port. 

That's not what this code does. This code retrieves the struct device
backing the ACPI device representing the PNP0A08 host bridge and its
dev->numa_node value (that was set in pci_acpi_scan_root()).

I am not a big fan of this. I wonder if we could not make it DT/ACPI
agnostic by simply setting the numa_node in the pci_bus->dev field,
and propagate it downstream a PCI hierarcy (as we do with sysdata)
in pci_alloc_child_bus().

This way pcibus_to_node() would become straightforward (ie as it
is now - provided the above is doable):

dev_to_node(&bus->dev);

This is suspiciously similar to the domain number song and dance
except that the NUMA node now is in the struct pci_bus->dev->numa_node
instead of struct pci_bus->domain_nr.

Lorenzo

^ permalink raw reply

* [RFC PATCH v2 6/6] arm64: signal: Report signal frame size to userspace via auxv
From: Dave Martin @ 2017-04-12 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492016495-19689-1-git-send-email-Dave.Martin@arm.com>

Stateful CPU architecture extensions may require the signal frame
to grow to a size that exceeds the arch's MINSIGSTKSZ #define.
However, changing this #define is an ABI break.

To allow userspace the option of determining the signal frame size
in a more forwards-compatible way, this patch adds a new auxv entry
tagged with AT_MINSIGSTKSZ, which provides the maximum signal frame
size that the process can observe during its lifetime.

If AT_MINSIGSTKSZ is absent from the aux vector, the caller can
assume that the MINSIGSTKSZ #define is sufficient.  This allows for
a consistent interface with older kernels that do not provide
AT_MINSIGSTKSZ.

The idea is that libc could expose this via sysconf() or some
similar mechanism.

There is deliberately no AT_SIGSTKSZ.  The kernel knows nothing
about userspace's own stack overheads and should not pretend to
know.

For arm64:

The primary motivation for this interface is the Scalable Vector
Extension, which can require at least 4KB or so of extra space
in the signal frame for the largest hardware implementations.

To determine the correct value, a "Christmas tree" mode (via the
add_all argument) is added to setup_sigframe_layout(), to simulate
addition of all possible records to the signal frame at maximum
possible size.

If this procedure goes wrong somehow, resulting in a stupidly large
frame layout and hence failure of sigframe_alloc() to allocate a
record to the frame, then this is indicative of a kernel bug: the
kernel's internal SIGFRAME_MAXSZ is supposed to sanity-check
against generting frames that we consider _impossibly_ large.  In
this case, SIGSTKSZ is returned as a "reasonable guess that is at
least bigger than MINSIGSTKSZ" and we WARN().

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---

Notes:

1)

Should AT_MINSIGSTKSZ be defined globally?

If there's a likelihood that one or more other arches may make use
of this mechanism, then the define could be moved to
linux/include/uapi/linux/auxvec.h.

Either way, userspace code can do #ifdef AT_MINSIGSTKSZ, so the
define can be left arch-specific without serious source
incompatibility for userspace.  The chosen number is ABI though:
currently it's in the arch-specific space (>= 32).

2)

The kernel has an ABI commitment not to generate larger signal
frames than userspace expects, so another mechanism would be needed
to "turn on" the generation of larger frames.

For now I don't anticipate a global mechanism for this.  For SVE,
I currently have a mechanism to limit the vector length given to
new processes by default, thus avoiding signal frames larger than
MINSIGSTKSZ being seen unless the program explicitly asks for
longer vectors.

3)

This patch feels like an abuse of ARCH_DLINFO, but I didn't
feel it was worth hacking the core code just for this...

If anyone objects, I'm happy to propose a new macro (or a new name
for the existing macro).

4)

For arm64, when the signal frame has not been enlarged, the true
size is returned, which is slightly smaller than MINSIGSTKSZ.

Since callers must know their own stack overheads in order to make
use of the result, and since we have no way to know those, this
probably doesn't matter.

There could be an argument for returning MINSIGSTKSZ though, in
case there is software that assumes for other reasons that stacks
are at least MINSIGSTKSZ in size.  By definition this interface
can't expect to ensure 100% compatibility, so it's not clear
whether this matters.  Userspace that cares could equally implement
this clamping itself.

---
 arch/arm64/include/asm/elf.h         |  5 +++++
 arch/arm64/include/asm/processor.h   |  3 +++
 arch/arm64/include/uapi/asm/auxvec.h |  3 ++-
 arch/arm64/kernel/signal.c           | 36 +++++++++++++++++++++++++++++++-----
 4 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 5d17004..5958487 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -24,6 +24,10 @@
 #include <asm/ptrace.h>
 #include <asm/user.h>
 
+#ifndef __ASSEMBLY__
+#include <asm/processor.h> /* for get_minsigstksz(), used by ARCH_DLINFO */
+#endif
+
 /*
  * AArch64 static relocation types.
  */
@@ -149,6 +153,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
 do {									\
 	NEW_AUX_ENT(AT_SYSINFO_EHDR,					\
 		    (elf_addr_t)current->mm->context.vdso);		\
+	NEW_AUX_ENT(AT_MINSIGSTKSZ, get_minsigstksz());			\
 } while (0)
 
 #define ARCH_HAS_SETUP_ADDITIONAL_PAGES
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 0502007..9bf5804 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -194,4 +194,7 @@ static inline void spin_lock_prefetch(const void *ptr)
 int cpu_enable_pan(void *__unused);
 int cpu_enable_cache_maint_trap(void *__unused);
 
+/* User signal frame size discovery: */
+int get_minsigstksz(void);
+
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/uapi/asm/auxvec.h b/arch/arm64/include/uapi/asm/auxvec.h
index 4cf0c17..1d45b28 100644
--- a/arch/arm64/include/uapi/asm/auxvec.h
+++ b/arch/arm64/include/uapi/asm/auxvec.h
@@ -18,7 +18,8 @@
 
 /* vDSO location */
 #define AT_SYSINFO_EHDR	33
+#define AT_MINSIGSTKSZ	34	/* stack needed for signal delivery */
 
-#define AT_VECTOR_SIZE_ARCH 1 /* entries in ARCH_DLINFO */
+#define AT_VECTOR_SIZE_ARCH 2 /* entries in ARCH_DLINFO */
 
 #endif
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 983cddf..c4ac046 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -407,8 +407,15 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
-/* Determine the layout of optional records in the signal frame */
-static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
+/*
+ * Determine the layout of optional records in the signal frame
+ *
+ * add_all: if true, lays out the biggest possible signal frame for
+ *	this task; otherwise, generates a layout for the current state
+ *	of the task.
+ */
+static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
+				 bool add_all)
 {
 	int err;
 
@@ -418,7 +425,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 		return err;
 
 	/* fault information, if valid */
-	if (current->thread.fault_code) {
+	if (add_all || current->thread.fault_code) {
 		err = sigframe_alloc(user, &user->esr_offset,
 				     sizeof(struct esr_context));
 		if (err)
@@ -428,7 +435,6 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 	return sigframe_alloc_end(user);
 }
 
-
 static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
@@ -505,7 +511,7 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
 	int err;
 
 	init_user_layout(user);
-	err = setup_sigframe_layout(user);
+	err = setup_sigframe_layout(user, false);
 	if (err)
 		return err;
 
@@ -728,3 +734,23 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
 		thread_flags = READ_ONCE(current_thread_info()->flags);
 	} while (thread_flags & _TIF_WORK_MASK);
 }
+
+/*
+ * Determine the stack space required for guaranteed signal devliery.
+ * This function is used to populate AT_MINSIGSTKSZ at process startup.
+ */
+int get_minsigstksz(void)
+{
+	struct rt_sigframe_user_layout user;
+	int err;
+
+	init_user_layout(&user);
+	err = setup_sigframe_layout(&user, true);
+
+	if (err) {
+		WARN_ON(1);
+
+		return SIGSTKSZ;
+	} else
+		return sigframe_size(&user) + 16; /* max alignment padding */
+}
-- 
2.1.4

^ permalink raw reply related

* [RFC PATCH v2 5/6] arm64: signal: Parse extra_context during sigreturn
From: Dave Martin @ 2017-04-12 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492016495-19689-1-git-send-email-Dave.Martin@arm.com>

If extra_context is present, parse it.

To avoid abuse by userspace, this patch attempts to ensure that:
 * no more than one extra_context is accepted;
 * the extra_context is a sensible size;
 * the extra context data is properly aligned.

The extra_context data is required to start immediately after
struct rt_sigframe (as during signal delivery).  This serves as a
sanity-check that the signal frame has not been moved or copied
without taking the extra data into account.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h |  6 ++++-
 arch/arm64/kernel/signal.c               | 46 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index b5e2523..a2f7211 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -96,7 +96,11 @@ struct esr_context {
  * extra_context must be the last record in sigcontext.__reserved[]
  * except for the terminator).
  *
- * 4) The extra space must itself be terminated with a null
+ * 4) The extra space to which data points must start at the first
+ * 16-byte aligned address immediately after the end of the sigcontext
+ * strucutre.
+ *
+ * 5) The extra space must itself be terminated with a null
  * _aarch64_ctx.
  */
 #define EXTRA_MAGIC	0x45585401
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 95547e1..983cddf 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -223,6 +223,10 @@ static int parse_user_sigframe(struct user_ctxs *user,
 	char __user *base = (char __user *)&sc->__reserved;
 	size_t offset = 0;
 	size_t limit = sizeof(sc->__reserved);
+	bool have_extra_context = false;
+
+	/* Expected location of extra_data (if present): */
+	char __user *const extra_base = (char __user *)sf + BASE_SIGFRAME_SIZE;
 
 	user->fpsimd = NULL;
 
@@ -232,6 +236,9 @@ static int parse_user_sigframe(struct user_ctxs *user,
 	while (1) {
 		int err = 0;
 		u32 magic, size;
+		struct extra_context const __user *extra;
+		void __user *extra_data;
+		u32 extra_size;
 
 		if (limit - offset < sizeof(*head))
 			goto invalid;
@@ -269,6 +276,45 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			/* ignore */
 			break;
 
+		case EXTRA_MAGIC:
+			if (have_extra_context)
+				goto invalid;
+
+			if (size < sizeof(*extra))
+				goto invalid;
+
+			extra = (struct extra_context const __user *)head;
+			__get_user_error(extra_data, &extra->data, err);
+			__get_user_error(extra_size, &extra->size, err);
+			if (err)
+				return err;
+
+			/* Prevent looping/repeated parsing of extra_conext */
+			have_extra_context = true;
+
+			/*
+			 * Rely on the __user accessors to reject bogus
+			 * pointers.
+			 */
+			base = extra_data;
+			if (!IS_ALIGNED((unsigned long)base, 16))
+				goto invalid;
+
+			if (extra_data != extra_base)
+				goto invalid;
+
+			/* Reject "unreasonably large" frames: */
+			limit = extra_size;
+			if (limit > SIGFRAME_MAXSZ - sizeof(sc->__reserved))
+				goto invalid;
+
+			/*
+			 * Ignore trailing terminator in __reserved[]
+			 * and start parsing extra_data:
+			 */
+			offset = 0;
+			continue;
+
 		default:
 			goto invalid;
 		}
-- 
2.1.4

^ permalink raw reply related

* [RFC PATCH v2 4/6] arm64: signal: Allocate extra sigcontext space as needed
From: Dave Martin @ 2017-04-12 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492016495-19689-1-git-send-email-Dave.Martin@arm.com>

This patch modifies the context block allocator to create an
extra_context expansion block as necessary, and adds the necessary
code to populate, parse and decode this block.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h |  27 ++++++++
 arch/arm64/kernel/signal.c               | 111 ++++++++++++++++++++++++++-----
 2 files changed, 120 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index 1328a2c..b5e2523 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -80,4 +80,31 @@ struct esr_context {
 	__u64 esr;
 };
 
+/*
+ * Pointer to extra space for additional structures that don't fit in
+ * sigcontext.__reserved[].  Note:
+ *
+ * 1) fpsimd_context, esr_context and extra_context must be placed in
+ * sigcontext.__reserved[] if present.  They cannot be placed in the
+ * extra space.  Any other record can be placed either in the extra
+ * space or in sigcontext.__reserved[].
+ *
+ * 2) There must not be more than one extra_context.
+ *
+ * 3) If extra_context is present, it must be followed immediately in
+ * sigcontext.__reserved[] by the terminating null _aarch64_ctx (i.e.,
+ * extra_context must be the last record in sigcontext.__reserved[]
+ * except for the terminator).
+ *
+ * 4) The extra space must itself be terminated with a null
+ * _aarch64_ctx.
+ */
+#define EXTRA_MAGIC	0x45585401
+
+struct extra_context {
+	struct _aarch64_ctx head;
+	void __user *data;	/* 16-byte aligned pointer to extra space */
+	__u32 size;		/* size in bytes of the extra space */
+};
+
 #endif /* _UAPI__ASM_SIGCONTEXT_H */
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 47592af..95547e1 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -25,6 +25,7 @@
 #include <linux/freezer.h>
 #include <linux/stddef.h>
 #include <linux/uaccess.h>
+#include <linux/sizes.h>
 #include <linux/string.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
@@ -56,18 +57,27 @@ struct rt_sigframe_user_layout {
 
 	unsigned long fpsimd_offset;
 	unsigned long esr_offset;
+	unsigned long extra_offset;
 	unsigned long end_offset;
 };
 
+#define BASE_SIGFRAME_SIZE round_up(sizeof(struct rt_sigframe), 16)
+#define TERMINATOR_SIZE round_up(sizeof(struct _aarch64_ctx), 16)
+#define EXTRA_CONTEXT_SIZE round_up(sizeof(struct extra_context), 16)
+
 static void init_user_layout(struct rt_sigframe_user_layout *user)
 {
+	const size_t reserved_size =
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
+
 	memset(user, 0, sizeof(*user));
 	user->size = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved);
 
-	user->limit = user->size +
-		sizeof(user->sigframe->uc.uc_mcontext.__reserved) -
-		round_up(sizeof(struct _aarch64_ctx), 16);
-		/* ^ reserve space for terminator */
+	user->limit = user->size + reserved_size;
+
+	user->limit -= TERMINATOR_SIZE;
+	user->limit -= EXTRA_CONTEXT_SIZE;
+	/* Reserve space for extension and terminator ^ */
 }
 
 static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
@@ -75,6 +85,48 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
 }
 
+/* Sanity limit on the maximum size of signal frame we'll try to generate. */
+/* This is NOT ABI. */
+#define SIGFRAME_MAXSZ SZ_64K
+
+static int __sigframe_alloc(struct rt_sigframe_user_layout *user,
+			    unsigned long *offset, size_t size, bool extend)
+{
+	size_t padded_size = round_up(size, 16);
+
+	if (padded_size > user->limit - user->size &&
+	    !user->extra_offset &&
+	    extend) {
+		int ret;
+
+		ret = __sigframe_alloc(user, &user->extra_offset,
+				       sizeof(struct extra_context), false);
+		if (ret)
+			return ret;
+
+		/*
+		 * Further allocations must go after the fixed-size
+		 * part of the signal frame:
+		 */
+		user->size = BASE_SIGFRAME_SIZE;
+
+		/*
+		 * Allow expansion up to SIGFRAME_MAXSZ, ensuring space for
+		 * the terminator:
+		 */
+		user->limit = SIGFRAME_MAXSZ - TERMINATOR_SIZE;
+	}
+
+	/* Still not enough space?  Bad luck! */
+	if (padded_size > user->limit - user->size)
+		return -ENOMEM;
+
+	*offset = user->size;
+	user->size += padded_size;
+
+	return 0;
+}
+
 /*
  * Allocate space for an optional record of <size> bytes in the user
  * signal frame.  The offset from the signal frame base address to the
@@ -83,11 +135,24 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 static int sigframe_alloc(struct rt_sigframe_user_layout *user,
 			  unsigned long *offset, size_t size)
 {
-	size_t padded_size = round_up(size, 16);
+	return __sigframe_alloc(user, offset, size, true);
+}
 
-	*offset = user->size;
-	user->size += padded_size;
+/* Allocate the null terminator record and prevent further allocations */
+static int sigframe_alloc_end(struct rt_sigframe_user_layout *user)
+{
+	int ret;
+
+	/* Un-reserve the space reserved for the terminator: */
+	user->limit += TERMINATOR_SIZE;
+
+	ret = sigframe_alloc(user, &user->end_offset,
+			     sizeof(struct _aarch64_ctx));
+	if (ret)
+		return ret;
 
+	/* Prevent further allocation: */
+	user->limit = user->size;
 	return 0;
 }
 
@@ -314,17 +379,7 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 			return err;
 	}
 
-	/*
-	 * Allocate space for the terminator record.
-	 * HACK: here we undo the reservation of space for the end record.
-	 * This bodge should be replaced with a cleaner approach later on.
-	 */
-	user->limit = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved) +
-		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
-
-	err = sigframe_alloc(user, &user->end_offset,
-			     sizeof(struct _aarch64_ctx));
-	return err;
+	return sigframe_alloc_end(user);
 }
 
 
@@ -365,6 +420,26 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
 	}
 
+	if (err == 0 && user->extra_offset) {
+		struct extra_context __user *extra =
+			apply_user_offset(user, user->extra_offset);
+		struct _aarch64_ctx __user *end =
+			(struct _aarch64_ctx __user *)((char __user *)extra +
+				round_up(sizeof(*extra), 16));
+		void __user *extra_data =
+			apply_user_offset(user, BASE_SIGFRAME_SIZE);
+		u32 extra_size = round_up(user->size, 16) - BASE_SIGFRAME_SIZE;
+
+		__put_user_error(EXTRA_MAGIC, &extra->head.magic, err);
+		__put_user_error(sizeof(*extra), &extra->head.size, err);
+		__put_user_error(extra_data, &extra->data, err);
+		__put_user_error(extra_size, &extra->size, err);
+
+		/* Add the terminator */
+		__put_user_error(0, &end->magic, err);
+		__put_user_error(0, &end->size, err);
+	}
+
 	/* set the "end" magic */
 	if (err == 0) {
 		struct _aarch64_ctx __user *end =
-- 
2.1.4

^ permalink raw reply related

* [RFC PATCH v2 3/6] arm64: signal: factor out signal frame record allocation
From: Dave Martin @ 2017-04-12 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492016495-19689-1-git-send-email-Dave.Martin@arm.com>

This patch factors out the allocator for signal frame optional
records into a separate function, to ensure consistency and
facilitate later expansion of the signal frame, so that the
allocation mechanism can be elaborated more easily in future
patches.

No overrun checking is currently done, because the allocation is in
user memory, and because the kernel statically does not attempt to
allocate enough space in the signal frame yet for an overrun to
occur.  This behaviour will be refined in future patches.

The approach taken in this patch to allocation of the terminator
record is not very clean: this will also be replaced in subsequent
patches.

For future extension, a table is added documenting the current
static allocations in __reserved[].  This will be important for
determining under what circumstances userspace may or may not see
an expanded signal frame.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/include/uapi/asm/sigcontext.h | 19 ++++++++++++++
 arch/arm64/kernel/signal.c               | 43 ++++++++++++++++++++++++++------
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
index ee469be..1328a2c 100644
--- a/arch/arm64/include/uapi/asm/sigcontext.h
+++ b/arch/arm64/include/uapi/asm/sigcontext.h
@@ -34,6 +34,25 @@ struct sigcontext {
 };
 
 /*
+ * Allocation of __reserved[]:
+ * (Note: records do not necessarily occur in the order shown here.)
+ *
+ *	size		description
+ *
+ *	0x210		fpsimd_context
+ *	 0x10		esr_context
+ *	 0x10		terminator (null _aarch64_ctx)
+ *
+ *	0xdd0		(reserved for future allocation)
+ *
+ * New records that can exceed this space need to be opt-in for userspace, so
+ * that an expanded signal frame is not generated unexpectedly.  The mechanism
+ * for opting in will depend on the extension that generates each new record.
+ * The above table documents the maximum set and sizes of records than can be
+ * generated when userspace does not opt in for any such extension.
+ */
+
+/*
  * Header to be used at the beginning of structures extending the user
  * context. Such structures must be placed after the rt_sigframe on the stack
  * and be 16-byte aligned. The last structure must be a dummy one with the
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 6202753..47592af 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -75,6 +75,22 @@ static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
 	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
 }
 
+/*
+ * Allocate space for an optional record of <size> bytes in the user
+ * signal frame.  The offset from the signal frame base address to the
+ * allocated block is assigned to *offset.
+ */
+static int sigframe_alloc(struct rt_sigframe_user_layout *user,
+			  unsigned long *offset, size_t size)
+{
+	size_t padded_size = round_up(size, 16);
+
+	*offset = user->size;
+	user->size += padded_size;
+
+	return 0;
+}
+
 static void __user *apply_user_offset(
 	struct rt_sigframe_user_layout const *user, unsigned long offset)
 {
@@ -283,19 +299,32 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 /* Determine the layout of optional records in the signal frame */
 static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
 {
-	user->fpsimd_offset = user->size;
-	user->size += round_up(sizeof(struct fpsimd_context), 16);
+	int err;
+
+	err = sigframe_alloc(user, &user->fpsimd_offset,
+			     sizeof(struct fpsimd_context));
+	if (err)
+		return err;
 
 	/* fault information, if valid */
 	if (current->thread.fault_code) {
-		user->esr_offset = user->size;
-		user->size += round_up(sizeof(struct esr_context), 16);
+		err = sigframe_alloc(user, &user->esr_offset,
+				     sizeof(struct esr_context));
+		if (err)
+			return err;
 	}
 
-	/* set the "end" magic */
-	user->end_offset = user->size;
+	/*
+	 * Allocate space for the terminator record.
+	 * HACK: here we undo the reservation of space for the end record.
+	 * This bodge should be replaced with a cleaner approach later on.
+	 */
+	user->limit = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved) +
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved);
 
-	return 0;
+	err = sigframe_alloc(user, &user->end_offset,
+			     sizeof(struct _aarch64_ctx));
+	return err;
 }
 
 
-- 
2.1.4

^ permalink raw reply related

* [RFC PATCH v2 2/6] arm64: signal: factor frame layout and population into separate passes
From: Dave Martin @ 2017-04-12 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492016495-19689-1-git-send-email-Dave.Martin@arm.com>

In preparation for expanding the signal frame, this patch refactors
the signal frame setup code in setup_sigframe() into two separate
passes.

The first pass, setup_sigframe_layout(), determines the size of the
signal frame and its internal layout, including the presence and
location of optional records.  The resulting knowledge is used to
allocate and locate the user stack space required for the signal
frame and to determine which optional records to include.

The second pass, setup_sigframe(), is called once the stack frame
is allocated in order to populate it with the necessary context
information.

As a result of these changes, it becomes more natural to represent
locations in the signal frame by a base pointer and an offset,
since the absolute address of each location is not known during the
layout pass.  To be more consistent with this logic,
parse_user_sigframe() is refactored to describe signal frame
locations in the same way.  This enables a common representation
(struct rt_sigframe_user_layout) of the signal frame layout to be
used in all relevant code.

This change has no effect on the signal ABI, but will make it
easier to expand the signal frame in future patches.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/kernel/signal.c | 112 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 88 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index d4c5e3e..6202753 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -25,6 +25,7 @@
 #include <linux/freezer.h>
 #include <linux/stddef.h>
 #include <linux/uaccess.h>
+#include <linux/string.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
 
@@ -49,8 +50,39 @@ struct rt_sigframe {
 
 struct rt_sigframe_user_layout {
 	struct rt_sigframe __user *sigframe;
+
+	unsigned long size;	/* size of allocated sigframe data */
+	unsigned long limit;	/* largest allowed size */
+
+	unsigned long fpsimd_offset;
+	unsigned long esr_offset;
+	unsigned long end_offset;
 };
 
+static void init_user_layout(struct rt_sigframe_user_layout *user)
+{
+	memset(user, 0, sizeof(*user));
+	user->size = offsetof(struct rt_sigframe, uc.uc_mcontext.__reserved);
+
+	user->limit = user->size +
+		sizeof(user->sigframe->uc.uc_mcontext.__reserved) -
+		round_up(sizeof(struct _aarch64_ctx), 16);
+		/* ^ reserve space for terminator */
+}
+
+static size_t sigframe_size(struct rt_sigframe_user_layout const *user)
+{
+	return round_up(max(user->size, sizeof(struct rt_sigframe)), 16);
+}
+
+static void __user *apply_user_offset(
+	struct rt_sigframe_user_layout const *user, unsigned long offset)
+{
+	char __user *base = (char __user *)user->sigframe;
+
+	return base + offset;
+}
+
 static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
 {
 	struct fpsimd_state *fpsimd = &current->thread.fpsimd_state;
@@ -106,26 +138,35 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			       struct rt_sigframe __user *sf)
 {
 	struct sigcontext __user *sc = &sf->uc.uc_mcontext;
-	struct _aarch64_ctx __user *head =
-		(struct _aarch64_ctx __user *)&sc->__reserved;
+	struct _aarch64_ctx __user *head;
+	char __user *base = (char __user *)&sc->__reserved;
 	size_t offset = 0;
+	size_t limit = sizeof(sc->__reserved);
 
 	user->fpsimd = NULL;
 
+	if (!IS_ALIGNED((unsigned long)base, 16))
+		goto invalid;
+
 	while (1) {
-		int err;
+		int err = 0;
 		u32 magic, size;
 
-		head = (struct _aarch64_ctx __user *)&sc->__reserved[offset];
-		if (!IS_ALIGNED((unsigned long)head, 16))
+		if (limit - offset < sizeof(*head))
 			goto invalid;
 
-		err = 0;
+		if (!IS_ALIGNED(offset, 16))
+			goto invalid;
+
+		head = (struct _aarch64_ctx __user *)(base + offset);
 		__get_user_error(magic, &head->magic, err);
 		__get_user_error(size, &head->size, err);
 		if (err)
 			return err;
 
+		if (limit - offset < size)
+			goto invalid;
+
 		switch (magic) {
 		case 0:
 			if (size)
@@ -137,9 +178,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 			if (user->fpsimd)
 				goto invalid;
 
-			if (offset > sizeof(sc->__reserved) -
-					sizeof(*user->fpsimd) ||
-			    size < sizeof(*user->fpsimd))
+			if (size < sizeof(*user->fpsimd))
 				goto invalid;
 
 			user->fpsimd = (struct fpsimd_context __user *)head;
@@ -156,7 +195,7 @@ static int parse_user_sigframe(struct user_ctxs *user,
 		if (size < sizeof(*head))
 			goto invalid;
 
-		if (size > sizeof(sc->__reserved) - (sizeof(*head) + offset))
+		if (limit - offset < size)
 			goto invalid;
 
 		offset += size;
@@ -241,13 +280,30 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
+/* Determine the layout of optional records in the signal frame */
+static int setup_sigframe_layout(struct rt_sigframe_user_layout *user)
+{
+	user->fpsimd_offset = user->size;
+	user->size += round_up(sizeof(struct fpsimd_context), 16);
+
+	/* fault information, if valid */
+	if (current->thread.fault_code) {
+		user->esr_offset = user->size;
+		user->size += round_up(sizeof(struct esr_context), 16);
+	}
+
+	/* set the "end" magic */
+	user->end_offset = user->size;
+
+	return 0;
+}
+
+
 static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
 	int i, err = 0;
 	struct rt_sigframe __user *sf = user->sigframe;
-	void *aux = sf->uc.uc_mcontext.__reserved;
-	struct _aarch64_ctx *end;
 
 	/* set up the stack frame for unwinding */
 	__put_user_error(regs->regs[29], &sf->fp, err);
@@ -265,26 +321,29 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
 	err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
 
 	if (err == 0) {
-		struct fpsimd_context *fpsimd_ctx =
-			container_of(aux, struct fpsimd_context, head);
+		struct fpsimd_context __user *fpsimd_ctx =
+			apply_user_offset(user, user->fpsimd_offset);
 		err |= preserve_fpsimd_context(fpsimd_ctx);
-		aux += sizeof(*fpsimd_ctx);
 	}
 
 	/* fault information, if valid */
-	if (current->thread.fault_code) {
-		struct esr_context *esr_ctx =
-			container_of(aux, struct esr_context, head);
+	if (err == 0 && user->esr_offset) {
+		struct esr_context __user *esr_ctx =
+			apply_user_offset(user, user->esr_offset);
+
 		__put_user_error(ESR_MAGIC, &esr_ctx->head.magic, err);
 		__put_user_error(sizeof(*esr_ctx), &esr_ctx->head.size, err);
 		__put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
-		aux += sizeof(*esr_ctx);
 	}
 
 	/* set the "end" magic */
-	end = aux;
-	__put_user_error(0, &end->magic, err);
-	__put_user_error(0, &end->size, err);
+	if (err == 0) {
+		struct _aarch64_ctx __user *end =
+			apply_user_offset(user, user->end_offset);
+
+		__put_user_error(0, &end->magic, err);
+		__put_user_error(0, &end->size, err);
+	}
 
 	return err;
 }
@@ -293,10 +352,15 @@ static int get_sigframe(struct rt_sigframe_user_layout *user,
 			 struct ksignal *ksig, struct pt_regs *regs)
 {
 	unsigned long sp, sp_top;
+	int err;
 
-	sp = sp_top = sigsp(regs->sp, ksig);
+	init_user_layout(user);
+	err = setup_sigframe_layout(user);
+	if (err)
+		return err;
 
-	sp = (sp - sizeof(struct rt_sigframe)) & ~15;
+	sp = sp_top = sigsp(regs->sp, ksig);
+	sp = (sp & ~15) - sigframe_size(user);
 	user->sigframe = (struct rt_sigframe __user *)sp;
 
 	/*
-- 
2.1.4

^ permalink raw reply related

* [RFC PATCH v2 1/6] arm64: signal: Refactor sigcontext parsing in rt_sigreturn
From: Dave Martin @ 2017-04-12 17:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492016239-19511-1-git-send-email-Dave.Martin@arm.com>

Currently, rt_sigreturn does very limited checking on the
sigcontext coming from userspace.

Future additions of extra dynamic sigcontext data will increase the
potential for surprises.  Also, it is not clear whether the
sigcontext extension records are supposed to occur in a particular
order.

This patch factors out the sigcontext parsing into a separate
function, and adds extra checks to validate the well-formedness of
the sigcontext structure.

To help with this, an abstraction for the signal frame layout is
also added, using offsets to track the location of different
records in the frame.  Although trivial, this provides a base to
extend upon in order to track more complex layouts.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm64/kernel/signal.c | 121 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 101 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index afed8f5..d4c5e3e 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -19,9 +19,11 @@
 
 #include <linux/compat.h>
 #include <linux/errno.h>
+#include <linux/kernel.h>
 #include <linux/signal.h>
 #include <linux/personality.h>
 #include <linux/freezer.h>
+#include <linux/stddef.h>
 #include <linux/uaccess.h>
 #include <linux/tracehook.h>
 #include <linux/ratelimit.h>
@@ -45,6 +47,10 @@ struct rt_sigframe {
 	u64 lr;
 };
 
+struct rt_sigframe_user_layout {
+	struct rt_sigframe __user *sigframe;
+};
+
 static int preserve_fpsimd_context(struct fpsimd_context __user *ctx)
 {
 	struct fpsimd_state *fpsimd = &current->thread.fpsimd_state;
@@ -92,12 +98,86 @@ static int restore_fpsimd_context(struct fpsimd_context __user *ctx)
 	return err ? -EFAULT : 0;
 }
 
+struct user_ctxs {
+	struct fpsimd_context __user *fpsimd;
+};
+
+static int parse_user_sigframe(struct user_ctxs *user,
+			       struct rt_sigframe __user *sf)
+{
+	struct sigcontext __user *sc = &sf->uc.uc_mcontext;
+	struct _aarch64_ctx __user *head =
+		(struct _aarch64_ctx __user *)&sc->__reserved;
+	size_t offset = 0;
+
+	user->fpsimd = NULL;
+
+	while (1) {
+		int err;
+		u32 magic, size;
+
+		head = (struct _aarch64_ctx __user *)&sc->__reserved[offset];
+		if (!IS_ALIGNED((unsigned long)head, 16))
+			goto invalid;
+
+		err = 0;
+		__get_user_error(magic, &head->magic, err);
+		__get_user_error(size, &head->size, err);
+		if (err)
+			return err;
+
+		switch (magic) {
+		case 0:
+			if (size)
+				goto invalid;
+
+			goto done;
+
+		case FPSIMD_MAGIC:
+			if (user->fpsimd)
+				goto invalid;
+
+			if (offset > sizeof(sc->__reserved) -
+					sizeof(*user->fpsimd) ||
+			    size < sizeof(*user->fpsimd))
+				goto invalid;
+
+			user->fpsimd = (struct fpsimd_context __user *)head;
+			break;
+
+		case ESR_MAGIC:
+			/* ignore */
+			break;
+
+		default:
+			goto invalid;
+		}
+
+		if (size < sizeof(*head))
+			goto invalid;
+
+		if (size > sizeof(sc->__reserved) - (sizeof(*head) + offset))
+			goto invalid;
+
+		offset += size;
+	}
+
+done:
+	if (!user->fpsimd)
+		goto invalid;
+
+	return 0;
+
+invalid:
+	return -EINVAL;
+}
+
 static int restore_sigframe(struct pt_regs *regs,
 			    struct rt_sigframe __user *sf)
 {
 	sigset_t set;
 	int i, err;
-	void *aux = sf->uc.uc_mcontext.__reserved;
+	struct user_ctxs user;
 
 	err = __copy_from_user(&set, &sf->uc.uc_sigmask, sizeof(set));
 	if (err == 0)
@@ -116,12 +196,11 @@ static int restore_sigframe(struct pt_regs *regs,
 	zap_syscall(regs);
 
 	err |= !valid_user_regs(&regs->user_regs, current);
+	if (err == 0)
+		err = parse_user_sigframe(&user, sf);
 
-	if (err == 0) {
-		struct fpsimd_context *fpsimd_ctx =
-			container_of(aux, struct fpsimd_context, head);
-		err |= restore_fpsimd_context(fpsimd_ctx);
-	}
+	if (err == 0)
+		err = restore_fpsimd_context(user.fpsimd);
 
 	return err;
 }
@@ -162,10 +241,11 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 	return 0;
 }
 
-static int setup_sigframe(struct rt_sigframe __user *sf,
+static int setup_sigframe(struct rt_sigframe_user_layout *user,
 			  struct pt_regs *regs, sigset_t *set)
 {
 	int i, err = 0;
+	struct rt_sigframe __user *sf = user->sigframe;
 	void *aux = sf->uc.uc_mcontext.__reserved;
 	struct _aarch64_ctx *end;
 
@@ -209,33 +289,32 @@ static int setup_sigframe(struct rt_sigframe __user *sf,
 	return err;
 }
 
-static struct rt_sigframe __user *get_sigframe(struct ksignal *ksig,
-					       struct pt_regs *regs)
+static int get_sigframe(struct rt_sigframe_user_layout *user,
+			 struct ksignal *ksig, struct pt_regs *regs)
 {
 	unsigned long sp, sp_top;
-	struct rt_sigframe __user *frame;
 
 	sp = sp_top = sigsp(regs->sp, ksig);
 
 	sp = (sp - sizeof(struct rt_sigframe)) & ~15;
-	frame = (struct rt_sigframe __user *)sp;
+	user->sigframe = (struct rt_sigframe __user *)sp;
 
 	/*
 	 * Check that we can actually write to the signal frame.
 	 */
-	if (!access_ok(VERIFY_WRITE, frame, sp_top - sp))
-		frame = NULL;
+	if (!access_ok(VERIFY_WRITE, user->sigframe, sp_top - sp))
+		return -EFAULT;
 
-	return frame;
+	return 0;
 }
 
 static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
-			 void __user *frame, int usig)
+			 struct rt_sigframe_user_layout *user, int usig)
 {
 	__sigrestore_t sigtramp;
 
 	regs->regs[0] = usig;
-	regs->sp = (unsigned long)frame;
+	regs->sp = (unsigned long)user->sigframe;
 	regs->regs[29] = regs->sp + offsetof(struct rt_sigframe, fp);
 	regs->pc = (unsigned long)ka->sa.sa_handler;
 
@@ -250,20 +329,22 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
 static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
 			  struct pt_regs *regs)
 {
+	struct rt_sigframe_user_layout user;
 	struct rt_sigframe __user *frame;
 	int err = 0;
 
-	frame = get_sigframe(ksig, regs);
-	if (!frame)
+	if (get_sigframe(&user, ksig, regs))
 		return 1;
 
+	frame = user.sigframe;
+
 	__put_user_error(0, &frame->uc.uc_flags, err);
 	__put_user_error(NULL, &frame->uc.uc_link, err);
 
 	err |= __save_altstack(&frame->uc.uc_stack, regs->sp);
-	err |= setup_sigframe(frame, regs, set);
+	err |= setup_sigframe(&user, regs, set);
 	if (err == 0) {
-		setup_return(regs, &ksig->ka, frame, usig);
+		setup_return(regs, &ksig->ka, &user, usig);
 		if (ksig->ka.sa.sa_flags & SA_SIGINFO) {
 			err |= copy_siginfo_to_user(&frame->info, &ksig->info);
 			regs->regs[1] = (unsigned long)&frame->info;
-- 
2.1.4

^ permalink raw reply related

* [RFC PATCH v2 0/6] Signal frame expansion support
From: Dave Martin @ 2017-04-12 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

(Note: This is an arm64-specific series, but the concepts introduced may
be of interest to other arches -- see in particular patch 6.)

Blurb:

An architecture advertises the maximum possible signal frame size via
the MINSIGSTKSZ #define (mandated by POSIX).

However, CPU architecture extensions may increase the amount of space
required to store the interrupted context when a signal is delivered.


Eventually the amount of space needed in the signal frame may exceed
MINSIGSTKSZ -- whether and when this happens is largely a matter of
luck, depending on the initial guess for MINSIGSTKSZ and the evolution
of that particular CPU architecture.  Unfortunately MINSIGSTKSZ cannot
be changed without an ABI break, and POSIX provides no mechanism for
migration.

arm64 initially reserved 4KB of space in the signal frame for
extensions, of which about 0.5KB is allocated to the FP/SIMD registers
initially.

Depending on the vector length supported by the hardware, SVE requires
up to around 8KB of space to store the full SIMD register context, which
is too large to fit in the existing frame.

This series adds a mechanism for optionally enlarging the signal frame
(patches 4-5) and reporting the actual maximum signal frame size to
userspace (patch 6).  Patches 1-3 do some refactoring to support this
change by abstracting the way signal frame records are allocated onto
the user stack.

Full backwards compatibility is not possible -- there is no way to hide
the fact that the signal frame has grown -- so it is expected that
support for new architecture extensions that can cause the signal frame
to grow will be opt-in for userspace, in addition to using the extension
mechanism defined by this series.

[1] ARM Scalable Vector Extension
https://community.arm.com/groups/processors/blog/2016/08/22/technology-update-the-scalable-vector-extension-sve-for-the-armv8-a-architecture
https://developer.arm.com/docs/ddi0584/latest/arm-architecture-reference-manual-supplement-the-scalable-vector-extension-sve-for-armv8-a

Dave Martin (6):
  arm64: signal: Refactor sigcontext parsing in rt_sigreturn
  arm64: signal: factor frame layout and population into separate passes
  arm64: signal: factor out signal frame record allocation
  arm64: signal: Allocate extra sigcontext space as needed
  arm64: signal: Parse extra_context during sigreturn
  arm64: signal: Report signal frame size to userspace via auxv

 arch/arm64/include/asm/elf.h             |   5 +
 arch/arm64/include/asm/processor.h       |   3 +
 arch/arm64/include/uapi/asm/auxvec.h     |   3 +-
 arch/arm64/include/uapi/asm/sigcontext.h |  50 ++++
 arch/arm64/kernel/signal.c               | 389 ++++++++++++++++++++++++++++---
 5 files changed, 415 insertions(+), 35 deletions(-)

-- 
2.1.4

^ permalink raw reply

* [PATCH V14 03/10] efi: parse ARM processor error
From: Borislav Petkov @ 2017-04-12 16:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1490729440-32591-4-git-send-email-tbaicar@codeaurora.org>

On Tue, Mar 28, 2017 at 01:30:33PM -0600, Tyler Baicar wrote:
> Add support for ARM Common Platform Error Record (CPER).
> UEFI 2.6 specification adds support for ARM specific
> processor error information to be reported as part of the
> CPER records. This provides more detail on for processor error logs.
> 
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Reviewed-by: James Morse <james.morse@arm.com>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/cper.c | 133 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/cper.h        |  54 ++++++++++++++++++
>  2 files changed, 187 insertions(+)
> 
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 8fa4e23..56aa516 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -110,12 +110,15 @@ void cper_print_bits(const char *pfx, unsigned int bits,
>  static const char * const proc_type_strs[] = {
>  	"IA32/X64",
>  	"IA64",
> +	"ARM",
>  };
>  
>  static const char * const proc_isa_strs[] = {
>  	"IA32",
>  	"IA64",
>  	"X64",
> +	"ARM A32/T32",
> +	"ARM A64",
>  };
>  
>  static const char * const proc_error_type_strs[] = {
> @@ -139,6 +142,18 @@ void cper_print_bits(const char *pfx, unsigned int bits,
>  	"corrected",
>  };
>  
> +static const char * const arm_reg_ctx_strs[] = {
> +	"AArch32 general purpose registers",
> +	"AArch32 EL1 context registers",
> +	"AArch32 EL2 context registers",
> +	"AArch32 secure context registers",
> +	"AArch64 general purpose registers",
> +	"AArch64 EL1 context registers",
> +	"AArch64 EL2 context registers",
> +	"AArch64 EL3 context registers",
> +	"Misc. system register structure",
> +};

That...

> +
>  static void cper_print_proc_generic(const char *pfx,
>  				    const struct cper_sec_proc_generic *proc)
>  {
> @@ -184,6 +199,114 @@ static void cper_print_proc_generic(const char *pfx,
>  		printk("%s""IP: 0x%016llx\n", pfx, proc->ip);
>  }
>  
> +static void cper_print_proc_arm(const char *pfx,
> +				const struct cper_sec_proc_arm *proc)

... and that function should go into:

#if defined(CONFIG_ARM64) || defined(CONFIG_ARM)

Just put them close together so that you don't have too much ifdeffery.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* [PATCH 2/2] ARM: dts: imx6qp: add specific compatible for GPC
From: Lucas Stach @ 2017-04-12 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412164600.16356-1-l.stach@pengutronix.de>

Add the more specific QuadPlus compatible to the GPC node, to trigger the
required workarounds in the power domain code.

In regard to the interrupt mapping the QuadPlus controller is fully
compatible to the Quad one, so keep that compatible in place.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/boot/dts/imx6qp.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qp.dtsi b/arch/arm/boot/dts/imx6qp.dtsi
index 59453f2ac4ba..299d863690c5 100644
--- a/arch/arm/boot/dts/imx6qp.dtsi
+++ b/arch/arm/boot/dts/imx6qp.dtsi
@@ -120,6 +120,10 @@
 		     <0 119 IRQ_TYPE_LEVEL_HIGH>;
 };
 
+&gpc {
+	compatible = "fsl,imx6qp-gpc", "fsl,imx6q-gpc";
+};
+
 &ipu1 {
 	compatible = "fsl,imx6qp-ipu", "fsl,imx6q-ipu";
 	fsl,prg = <&prg1>;
-- 
2.11.0

^ permalink raw reply related


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