* Re: [PATCH 1/3] CXL: Add image control to sysfs
From: Ian Munsie @ 2015-01-15 4:41 UTC (permalink / raw)
To: Ryan Grimm; +Cc: mikey, linuxppc-dev
In-Reply-To: <1421290601-3293-1-git-send-email-grimm@linux.vnet.ibm.com>
Excerpts from Ryan Grimm's message of 2015-01-15 13:56:39 +1100:
> Add reset_loads_image and reset_image_select to sysfs.
>
> reset_image_select identifies which image will be loaded to the card on the
> next PERST. Valid entries are: "user" and "factory".
>
> reset_loads_image defines functionality on a PERST. Value of 0 means PERST
> will not cause image load. A power cycle is required to load the image. Value
> of 1 means PERST will cause image load.
>
> sysfs updates the cxl struct in the driver then calls cxl_update_image_control
> to write the vals in the VSEC.
Let's combine both of these into a single sysfs file, with "none",
"user" and "factory" options and have the show & read functions handle
mapping those three options to the two bits in the register.
Of the two names I'd probably go with reset_image_select.
> +What: /sys/class/cxl/<card>/reset_loads_image
> +Date: December 2014
> +Contact: linuxppc-dev@lists.ozlabs.org
> +Description: read/write
> + Value of 0 means PERST will not cause image load. A power
> + cycle is required to load the image. Value of 1 means PERST
> + will cause image load.
It also seems to be that having this disabled also means that PERST
doesn't fully reset the card. Might want to clarify that somewhat and
recommend it only be disabled for debugging purposes (e.g. to retain
the contents of the PSL trace arrays across a reset), and to always
enable it for production.
At the moment we don't set it at boot - we just go with whatever the
card is already set to do. I'm thinking it might be a good idea to
always set this bit on boot so the only time it's disabled is if a user
has explicitly gone and disabled it.
> +static ssize_t reset_loads_image_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct cxl *adapter = to_cxl_adapter(device);
> + return sprintf(buf, "%d\n", adapter->perst_loads_image);
We've used scnprintf for the other sysfs reads in this file, why sprintf
here?
> +static ssize_t reset_loads_image_store(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cxl *adapter = to_cxl_adapter(device);
> + unsigned long val;
> + int rc;
> +
> + if (kstrtoul(buf, 0, &val) < 0)
> + return -EINVAL;
> +
> + adapter->perst_loads_image = !!val;
> + if ((rc = cxl_update_image_control(adapter)))
> + return rc;
Seems to be some indentation mismatches here - some lines are using
spaces other are using tabs. Please use tabs for everything.
> +static ssize_t reset_image_select_store(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cxl *adapter = to_cxl_adapter(device);
> + int rc;
> +
> + if (!strncmp(buf, "user", 4))
> + adapter->perst_select_user = true;
> + else if (!strncmp(buf, "factory", 7))
> + adapter->perst_select_user = false;
> + else
> + return -EINVAL;
More indentation mismatches here.
Cheers,
-Ian
^ permalink raw reply
* Re: [PATCH 1/3] CXL: Add image control to sysfs
From: Ian Munsie @ 2015-01-15 4:46 UTC (permalink / raw)
To: Ryan Grimm; +Cc: mikey, linuxppc-dev
In-Reply-To: <1421295393-sup-926@delenn.ozlabs.ibm.com>
Excerpts from Ian Munsie's message of 2015-01-15 15:41:24 +1100:
> At the moment we don't set it at boot - we just go with whatever the
> card is already set to do. I'm thinking it might be a good idea to
> always set this bit on boot so the only time it's disabled is if a user
> has explicitly gone and disabled it.
While I think of it - if we change this on boot we should also change
reset_image_select to match the currently loaded image. e.g. if
reset_loads_image has defaulted to off and reset_image_select has
defaulted to factory, but the user image has been loaded - that way we
avoid unexpectedly switching to factory if the card gets reset.
Cheers,
-Ian
^ permalink raw reply
* Re: [PATCH 1/3] CXL: Add image control to sysfs
From: Ian Munsie @ 2015-01-15 4:54 UTC (permalink / raw)
To: Ryan Grimm; +Cc: mikey, linuxppc-dev
In-Reply-To: <1421296950-sup-6378@delenn.ozlabs.ibm.com>
> While I think of it - if we change this on boot we should also change
> reset_image_select to match the currently loaded image. e.g. if
> reset_loads_image has defaulted to off and reset_image_select has
> defaulted to factory, but the user image has been loaded - that way we
> avoid unexpectedly switching to factory if the card gets reset.
Nevermind - I see you have done exactly this in patch 3 :-)
-Ian
^ permalink raw reply
* Re: [PATCH 1/3] CXL: Add image control to sysfs
From: Michael Ellerman @ 2015-01-15 5:07 UTC (permalink / raw)
To: Ian Munsie; +Cc: Ryan Grimm, mikey, linuxppc-dev
In-Reply-To: <1421295393-sup-926@delenn.ozlabs.ibm.com>
On Thu, 2015-01-15 at 15:41 +1100, Ian Munsie wrote:
> Excerpts from Ryan Grimm's message of 2015-01-15 13:56:39 +1100:
> > Add reset_loads_image and reset_image_select to sysfs.
> >
> > reset_image_select identifies which image will be loaded to the card on the
> > next PERST. Valid entries are: "user" and "factory".
> >
> > reset_loads_image defines functionality on a PERST. Value of 0 means PERST
> > will not cause image load. A power cycle is required to load the image. Value
> > of 1 means PERST will cause image load.
> >
> > sysfs updates the cxl struct in the driver then calls cxl_update_image_control
> > to write the vals in the VSEC.
>
> Let's combine both of these into a single sysfs file, with "none",
> "user" and "factory" options and have the show & read functions handle
> mapping those three options to the two bits in the register.
>
> Of the two names I'd probably go with reset_image_select.
Three words, all can be verbs, two can be nouns, it's not too clear.
Maybe "load_image_on_perst" ?
cheers
^ permalink raw reply
* Re: [PATCH 2/3] CXL: Snoop control
From: Ian Munsie @ 2015-01-15 5:16 UTC (permalink / raw)
To: Ryan Grimm; +Cc: mikey, linuxppc-dev
In-Reply-To: <1421290601-3293-2-git-send-email-grimm@linux.vnet.ibm.com>
Excerpts from Ryan Grimm's message of 2015-01-15 13:56:40 +1100:
> Add mode to opal call. SNOOP control turns CAPP unit snooping on/off. This is
> needed for the following reset patch, which turns snoops off in the CAPP
> recovery path.
Looking at patch 3 in this series I think this description needs to be
updated, as it doesn't seem to turn off snoops?
> +/* CAPI modes for PHB */
> +enum {
> + OPAL_PHB_CAPI_MODE_PCIE = 0,
> + OPAL_PHB_CAPI_MODE_CAPI = 1,
> + OPAL_PHB_CAPI_MODE_SNOOP_OFF = 2,
> + OPAL_PHB_CAPI_MODE_SNOOP_ON = 3,
> +};
Spaces have been used for indention here
> +/* CAPI feature flags (in device-tree) */
> +#define OPAL_PHB_CAPI_FLAG_SNOOP_CONTROL 0x00000001
> +#define OPAL_PHB_CAPI_FLAG_REVERT_TO_PCIE 0x00000002
It doesn't look like these are used?
> -int pnv_phb_to_cxl(struct pci_dev *dev)
> +int pnv_phb_to_cxl(struct pci_dev *dev, uint64_t mode)
Should we rename this function since it no longer just sets the PHB to
CXL mode? Maybe something like pnv_phb_set_cxl_mode?
> + if ((rc = pnv_phb_to_cxl(dev, OPAL_PHB_CAPI_MODE_SNOOP_ON))) {
> + dev_err(&dev->dev, "enable capp snoops: %i\n", rc);
> + }
Ok, we turn on snooping here, but I don't see where we turned it off -
has patch 3 changed so that never happens?
Also - why this late in in the init sequence? Not saying it's wrong, just
wondering if this has to happen after all the AFUs have been initialised, or if
it can happen earlier in the adapter initialisation, like when we set
the PHB to capi mode?
Cheers,
-Ian
^ permalink raw reply
* Re: [PATCH 3/3] CXL: Add reset to sysfs
From: Ian Munsie @ 2015-01-15 5:42 UTC (permalink / raw)
To: Ryan Grimm; +Cc: mikey, linuxppc-dev
In-Reply-To: <1421290601-3293-3-git-send-email-grimm@linux.vnet.ibm.com>
Excerpts from Ryan Grimm's message of 2015-01-15 13:56:41 +1100:
> This allows an image to be downloaded to the flash without rebooting the
> machine. The driver perform a PERST, which results in FPGA image downloaded to
> flash and the CAPP unit enters recovery. CAPP recovery triggers an HMI, which
> is handled by EEH in Linux. EEH removes the driver, calls into Sapphire to
> reinitialize the PHB, and then loads the driver.
>
> reset_image_select must be set to "user" and reset_load_image set to 1. The
> driver writes "user" to the vsec if a user image was loaded. It writes 1 to
> reset_load_image on initialization by default. Other values could be used by
> hand for debugging purposes.
That last paragraph will need to be updated if we merge those two sysfs
files into one. Might as well mention an example of why someone might do
a reset with no image selected for reload, e.g. the PSL trace arrays are
preserved, which can be read out through debugfs after the card comes
back up.
> +What: /sys/class/cxl/<card>/reset
> +Date: October 2014
> +Contact: linuxppc-dev@lists.ozlabs.org
> +Description: write only
> + Writing 1 here will issue a PERST to card.
"..., which may cause the card to reload the FPGA image depending on the
settings of reset_image_select."
> + if ((rc = pci_set_pcie_reset_state(dev, pcie_warm_reset))) {
Can you add a comment here to explain why we first do a warm reset?
> + dev_err(&dev->dev, "cxl: pcie_warm_reset failed\n");
> + return rc;
> + }
> +
> + /* Do mmio read to trigger EEH. Retry for a few seconds. */
This seems a little unusual - can you expand this comment a little to
explain *why* we are using this method to trigger an EEH and reset the
card?
> + i = 0;
> + while ((val = mmio_read32be(adapter->p1_mmio) != 0xffffffff) &&
> + (i < 5)) {
> + msleep(500);
> + i++;
> + }
> +
> + if (val != 0xffffffff)
> + dev_err(&dev->dev, "cxl: PERST failed to trigger EEH\n");
> +
> + return rc;
Some of the indentation here is a bit funky - some lines are using tabs,
others are using spaces.
> @@ -806,8 +837,8 @@ static int cxl_read_vsec(struct cxl *adapter, struct pci_dev *dev)
> CXL_READ_VSEC_BASE_IMAGE(dev, vsec, &adapter->base_image);
> CXL_READ_VSEC_IMAGE_STATE(dev, vsec, &image_state);
> adapter->user_image_loaded = !!(image_state & CXL_VSEC_USER_IMAGE_LOADED);
> - adapter->perst_loads_image = !!(image_state & CXL_VSEC_PERST_LOADS_IMAGE);
> - adapter->perst_select_user = !!(image_state & CXL_VSEC_PERST_SELECT_USER);
> + adapter->perst_loads_image = true;
> + adapter->perst_select_user = !!(image_state & CXL_VSEC_USER_IMAGE_LOADED);
...
> + if ((rc = cxl_update_image_control(adapter)))
> + goto err2;
Thanks - that seems like a better default than what we had before,
should make things more stable :)
Cheers,
-Ian
^ permalink raw reply
* Re: [PATCH 1/3] CXL: Add image control to sysfs
From: Ian Munsie @ 2015-01-15 5:44 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Ryan Grimm, mikey, linuxppc-dev
In-Reply-To: <1421298437.11409.4.camel@ellerman.id.au>
Excerpts from Michael Ellerman's message of 2015-01-15 16:07:17 +1100:
> > Of the two names I'd probably go with reset_image_select.
>
> Three words, all can be verbs, two can be nouns, it's not too clear.
>
> Maybe "load_image_on_perst" ?
Works for me :)
Cheers,
-Ian
^ permalink raw reply
* Re: [PATCH 3/3] CXL: Add reset to sysfs
From: Ian Munsie @ 2015-01-15 5:51 UTC (permalink / raw)
To: Ryan Grimm; +Cc: mikey, linuxppc-dev
In-Reply-To: <1421290601-3293-3-git-send-email-grimm@linux.vnet.ibm.com>
Excerpts from Ryan Grimm's message of 2015-01-15 13:56:41 +1100:
> +What: /sys/class/cxl/<card>/reset
> +Date: October 2014
> +Contact: linuxppc-dev@lists.ozlabs.org
> +Description: write only
> + Writing 1 here will issue a PERST to card.
...
> +static ssize_t reset_adapter_store(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cxl *adapter = to_cxl_adapter(device);
> + int rc;
> +
> + if ((rc = cxl_reset(adapter)))
> + return rc;
> + return count;
> +}
Looks like we reset the card no matter what is written to that file?
I like the description better - add a test here to match what it says.
Cheers,
-Ian
^ permalink raw reply
* [PATCH 1/2] clock: redefine variable clocks_per_pll as a struct member
From: Yuantian.Tang @ 2015-01-15 6:03 UTC (permalink / raw)
To: mturquette; +Cc: b07421, linuxppc-dev, Tang Yuantian
From: Tang Yuantian <Yuantian.Tang@freescale.com>
redefine variable clocks_per_pll as a struct member
If there are multiple PLL clock nodes, this variable will
get overwritten. Redefining it as a struct member can avoid that.
Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
These patches are based on following three patches which are acked
by Scott wood <scottwood@freescale.com>:
1. http://patchwork.ozlabs.org/patch/417292/
Revert "clk: ppc-corenet: Fix Section mismatch warning"
2. http://patchwork.ozlabs.org/patch/417295/
powerpc: call of_clk_init() from time_init()
3. http://patchwork.ozlabs.org/patch/417297/
clk: ppc-corenet: fix section mismatch warning
drivers/clk/clk-ppc-corenet.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c
index 57a2de4..5e9bb18 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-ppc-corenet.c
@@ -19,6 +19,7 @@
struct cmux_clk {
struct clk_hw hw;
void __iomem *reg;
+ unsigned int clk_per_pll;
u32 flags;
};
@@ -27,14 +28,12 @@ struct cmux_clk {
#define CLKSEL_ADJUST BIT(0)
#define to_cmux_clk(p) container_of(p, struct cmux_clk, hw)
-static unsigned int clocks_per_pll;
-
static int cmux_set_parent(struct clk_hw *hw, u8 idx)
{
struct cmux_clk *clk = to_cmux_clk(hw);
u32 clksel;
- clksel = ((idx / clocks_per_pll) << 2) + idx % clocks_per_pll;
+ clksel = ((idx / clk->clk_per_pll) << 2) + idx % clk->clk_per_pll;
if (clk->flags & CLKSEL_ADJUST)
clksel += 8;
clksel = (clksel & 0xf) << CLKSEL_SHIFT;
@@ -52,7 +51,7 @@ static u8 cmux_get_parent(struct clk_hw *hw)
clksel = (clksel >> CLKSEL_SHIFT) & 0xf;
if (clk->flags & CLKSEL_ADJUST)
clksel -= 8;
- clksel = (clksel >> 2) * clocks_per_pll + clksel % 4;
+ clksel = (clksel >> 2) * clk->clk_per_pll + clksel % 4;
return clksel;
}
@@ -72,6 +71,7 @@ static void __init core_mux_init(struct device_node *np)
u32 offset;
const char *clk_name;
const char **parent_names;
+ struct of_phandle_args clkspec;
rc = of_property_read_u32(np, "reg", &offset);
if (rc) {
@@ -105,6 +105,17 @@ static void __init core_mux_init(struct device_node *np)
goto err_clk;
}
+ rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", 0,
+ &clkspec);
+ if (rc) {
+ pr_err("%s: parse clock node error\n", __func__);
+ goto err_clk;
+ }
+
+ cmux_clk->clk_per_pll = of_property_count_strings(clkspec.np,
+ "clock-output-names");
+ of_node_put(clkspec.np);
+
node = of_find_compatible_node(NULL, NULL, "fsl,p4080-clockgen");
if (node && (offset >= 0x80))
cmux_clk->flags = CLKSEL_ADJUST;
@@ -181,9 +192,6 @@ static void __init core_pll_init(struct device_node *np)
goto err_map;
}
- /* output clock number per PLL */
- clocks_per_pll = count;
-
subclks = kzalloc(sizeof(struct clk *) * count, GFP_KERNEL);
if (!subclks) {
pr_err("%s: could not allocate subclks\n", __func__);
--
2.1.0.27.g96db324
^ permalink raw reply related
* [PATCH 2/2] clk: ppc-corenet: rename driver to clk-qoriq
From: Yuantian.Tang @ 2015-01-15 6:03 UTC (permalink / raw)
To: mturquette; +Cc: b07421, linuxppc-dev, Tang Yuantian
In-Reply-To: <1421301821-18917-1-git-send-email-Yuantian.Tang@freescale.com>
From: Tang Yuantian <Yuantian.Tang@freescale.com>
Freescale introduced new ARM-based socs which using the compatible
clock IP block with PowerPC-based socs'. So this driver can be used
on both platforms.
Updated relevant descriptions and renamed this driver to better
represent its meaning and keep the function of driver untouched.
Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
Documentation/devicetree/bindings/clock/qoriq-clock.txt | 5 +++--
drivers/clk/Kconfig | 10 +++++-----
drivers/clk/Makefile | 2 +-
drivers/clk/{clk-ppc-corenet.c => clk-qoriq.c} | 6 +++---
drivers/cpufreq/Kconfig.powerpc | 2 +-
5 files changed, 13 insertions(+), 12 deletions(-)
rename drivers/clk/{clk-ppc-corenet.c => clk-qoriq.c} (98%)
diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
index 266ff9d..df4a259 100644
--- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
+++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
@@ -1,6 +1,6 @@
-* Clock Block on Freescale CoreNet Platforms
+* Clock Block on Freescale QorIQ Platforms
-Freescale CoreNet chips take primary clocking input from the external
+Freescale qoriq chips take primary clocking input from the external
SYSCLK signal. The SYSCLK input (frequency) is multiplied using
multiple phase locked loops (PLL) to create a variety of frequencies
which can then be passed to a variety of internal logic, including
@@ -29,6 +29,7 @@ Required properties:
* "fsl,t4240-clockgen"
* "fsl,b4420-clockgen"
* "fsl,b4860-clockgen"
+ * "fsl,ls1021a-clockgen"
Chassis clock strings include:
* "fsl,qoriq-clockgen-1.0": for chassis 1.0 clocks
* "fsl,qoriq-clockgen-2.0": for chassis 2.0 clocks
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 3f44f29..a896fbc 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -101,12 +101,12 @@ config COMMON_CLK_AXI_CLKGEN
Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
FPGAs. It is commonly used in Analog Devices' reference designs.
-config CLK_PPC_CORENET
- bool "Clock driver for PowerPC corenet platforms"
- depends on PPC_E500MC && OF
+config CLK_QORIQ
+ bool "Clock driver for Freescale QorIQ platforms"
+ depends on (PPC_E500MC || ARM) && OF
---help---
- This adds the clock driver support for Freescale PowerPC corenet
- platforms using common clock framework.
+ This adds the clock driver support for Freescale QorIQ platforms
+ using common clock framework.
config COMMON_CLK_XGENE
bool "Clock driver for APM XGene SoC"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d5fba5b..4ff94cd 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -30,7 +30,7 @@ obj-$(CONFIG_ARCH_MOXART) += clk-moxart.o
obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o
obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o
obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o
-obj-$(CONFIG_CLK_PPC_CORENET) += clk-ppc-corenet.o
+obj-$(CONFIG_CLK_QORIQ) += clk-qoriq.o
obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o
obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o
obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o
diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-qoriq.c
similarity index 98%
rename from drivers/clk/clk-ppc-corenet.c
rename to drivers/clk/clk-qoriq.c
index 5e9bb18..f9b7eb4 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-qoriq.c
@@ -5,7 +5,7 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*
- * clock driver for Freescale PowerPC corenet SoCs.
+ * clock driver for Freescale QorIQ SoCs.
*/
#include <linux/clk-provider.h>
#include <linux/io.h>
@@ -166,7 +166,7 @@ static void __init core_pll_init(struct device_node *np)
base = of_iomap(np, 0);
if (!base) {
- pr_err("clk-ppc: iomap error\n");
+ pr_err("clk-qoriq: iomap error\n");
return;
}
@@ -260,7 +260,7 @@ static void __init sysclk_init(struct device_node *node)
u32 rate;
if (!np) {
- pr_err("ppc-clk: could not get parent node\n");
+ pr_err("qoriq-clk: could not get parent node\n");
return;
}
diff --git a/drivers/cpufreq/Kconfig.powerpc b/drivers/cpufreq/Kconfig.powerpc
index 72564b7..7ea2441 100644
--- a/drivers/cpufreq/Kconfig.powerpc
+++ b/drivers/cpufreq/Kconfig.powerpc
@@ -26,7 +26,7 @@ config CPU_FREQ_MAPLE
config PPC_CORENET_CPUFREQ
tristate "CPU frequency scaling driver for Freescale E500MC SoCs"
depends on PPC_E500MC && OF && COMMON_CLK
- select CLK_PPC_CORENET
+ select CLK_QORIQ
help
This adds the CPUFreq driver support for Freescale e500mc,
e5500 and e6500 series SoCs which are capable of changing
--
2.1.0.27.g96db324
^ permalink raw reply related
* [PATCH] fsl/smp: add low power boot support to replace spin boot
From: Dongsheng Wang @ 2015-01-15 6:05 UTC (permalink / raw)
To: scottwood, yorksun, leoli; +Cc: linuxppc-dev, Wang Dongsheng
From: Wang Dongsheng <dongsheng.wang@freescale.com>
U-boot put non-boot cpus into an low power state(PW10/PW20 or DOZE) when cpu
powered up. To exit low power state kernel will send DOORBELL or MPIC-IPI
signal to all those CPUs.
e500/e500v2 use mpic to send IPI signal.
e500mc and later use doorbell to send IPI signal.
This feature tested on:
POWER UP TEST:
P1022DS(e500v2),96k times.
P4080(e500mc), 110k times.
T1024(e5500), 83k times.
T4240(e6500), 150k times.
CPU HOTPLUG TEST:
P1022DS(e500v2),1.4 million times.
P4080(e500mc), 1.8 million times.
T1024(e5500), 1.3 million times.
T4240(e6500), 1.1 million times.
Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index 754f93d..8af6a25 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -474,6 +474,15 @@ extern int mpic_cpu_get_priority(void);
/* Set the current cpu priority for this cpu */
extern void mpic_cpu_set_priority(int prio);
+/* Set cpu priority */
+void mpic_set_cpu_priority(int nr, int prio);
+
+/* Set cpu EOI */
+void mpic_cpu_eoi_write(int cpu);
+
+/* CPU ACK interrupt */
+void mpic_cpu_ack(int cpu);
+
/* Request IPIs on primary mpic */
extern void mpic_request_ipis(void);
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index d7c1e69..6c54632 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -193,6 +193,30 @@ static int smp_85xx_kick_cpu(int nr)
const u64 *cpu_rel_addr;
__iomem struct epapr_spin_table *spin_table;
struct device_node *np;
+
+ /*
+ * DOORBELL:
+ * When kernel kick one of cpus, all cpus will be wakenup. To make
+ * sure that only the target cpu is effected, other cpus (by checking
+ * spin_table->addr_l) should go back to low power state.
+ *
+ * U-boot has renumber the cpu PIR Why we need to set all of PIR to
+ * the same value?
+ * A: Before kernel kicking cpu, the doorbell message was not configured
+ * for target cpu(cpu_messages->data). If we try to send a
+ * non-configured message to target cpu, it cannot correctly receive
+ * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
+ * let all cpus catch the interrupt.
+ *
+ * Why set PIR to zero?
+ * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us
+ * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot
+ * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be
+ * there. U-boot set PIR to zero as a default PIR ID for each CPU, so
+ * initialize the kick_cpus to 0.
+ */
+ u32 kick_cpus = 0;
+
int hw_cpu = get_hard_smp_processor_id(nr);
int ioremappable;
int ret = 0;
@@ -251,8 +275,7 @@ static int smp_85xx_kick_cpu(int nr)
spin_table = phys_to_virt(*cpu_rel_addr);
local_irq_save(flags);
-#ifdef CONFIG_PPC32
-#ifdef CONFIG_HOTPLUG_CPU
+#if defined(CONFIG_PPC32) && defined(CONFIG_HOTPLUG_CPU)
/* Corresponding to generic_set_cpu_dead() */
generic_set_cpu_up(nr);
@@ -292,11 +315,58 @@ static int smp_85xx_kick_cpu(int nr)
__secondary_hold_acknowledge = -1;
}
#endif
+
flush_spin_table(spin_table);
- out_be32(&spin_table->pir, hw_cpu);
+ /*
+ * U-boot will wait kernel send eoi to MPIC, after EOI has send
+ * kernel will set PIR for uboot, let uboot know EOI has send.
+ */
+ out_be32(&spin_table->pir, 0);
+
+#ifdef CONFIG_PPC32
out_be32(&spin_table->addr_l, __pa(__early_start));
+#else
+ out_be64((u64 *)(&spin_table->addr_h),
+ __pa(ppc_function_entry(generic_secondary_smp_init)));
+#endif
flush_spin_table(spin_table);
+ /*
+ * e500, e500v2 need to use MPIC to send IPI signal, so we need to
+ * open IPI firstly.
+ */
+ if (!cpu_has_feature(CPU_FTR_DBELL)) {
+ mpic_set_cpu_priority(nr, 0);
+ kick_cpus = nr;
+ }
+
+ /* Let cpu exit low power state, and from u-boot jump to kernel */
+ arch_send_call_function_single_ipi(kick_cpus);
+
+ /*
+ * Let we ACK interrput and Send EOI signal to finish INT server
+ * U-boot has read EPR to ACK interrput when MPIC work in external
+ * proxy mode. Without the external proxy facility, we need to read
+ * MPIC ACK register.
+ *
+ * There just ACK interrput, we don't need to get the interrupt vector
+ * and to handle it. Because there just IPI or DOORBELL interrupt to
+ * make u-boot exit low power state and jump to kernel.
+ */
+ mpic_cpu_ack(nr);
+ /* Send EOI to clear ISR bit to remove interrupt from service */
+ mpic_cpu_eoi_write(nr);
+
+ /* After wakeup CPU disable IPI, IPI will be opened in setup_cpu */
+ if (!cpu_has_feature(CPU_FTR_DBELL))
+ mpic_set_cpu_priority(nr, 0xf);
+
+ /* After EOI finish, let we release cpu */
+ flush_spin_table(spin_table);
+ out_be32(&spin_table->pir, hw_cpu);
+ flush_spin_table(spin_table);
+
+#ifdef CONFIG_PPC32
/* Wait a bit for the CPU to ack. */
if (!spin_event_timeout(__secondary_hold_acknowledge == hw_cpu,
10000, 100)) {
@@ -308,12 +378,6 @@ static int smp_85xx_kick_cpu(int nr)
out:
#else
smp_generic_kick_cpu(nr);
-
- flush_spin_table(spin_table);
- out_be32(&spin_table->pir, hw_cpu);
- out_be64((u64 *)(&spin_table->addr_h),
- __pa(ppc_function_entry(generic_secondary_smp_init)));
- flush_spin_table(spin_table);
#endif
local_irq_restore(flags);
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index c4648ad..b2ba47e 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -658,6 +658,21 @@ static inline void mpic_eoi(struct mpic *mpic)
(void)mpic_cpu_read(MPIC_INFO(CPU_WHOAMI));
}
+void mpic_cpu_eoi_write(int cpu)
+{
+ struct mpic *mpic = mpic_primary;
+
+ _mpic_write(mpic->reg_type, &mpic->cpuregs[cpu], MPIC_INFO(CPU_EOI), 0);
+ _mpic_read(mpic->reg_type, &mpic->cpuregs[cpu], MPIC_INFO(CPU_WHOAMI));
+}
+
+void mpic_cpu_ack(int cpu)
+{
+ struct mpic *mpic = mpic_primary;
+
+ _mpic_read(mpic->reg_type, &mpic->cpuregs[cpu], MPIC_INFO(CPU_INTACK));
+}
+
/*
* Linux descriptor level callbacks
*/
@@ -1778,6 +1793,16 @@ void mpic_cpu_set_priority(int prio)
mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), prio);
}
+void mpic_set_cpu_priority(int nr, int prio)
+{
+ struct mpic *mpic = mpic_primary;
+ int hw_cpu = get_hard_smp_processor_id(nr);
+
+ prio &= MPIC_CPU_TASKPRI_MASK;
+ _mpic_write(mpic->reg_type, &mpic->cpuregs[hw_cpu],
+ MPIC_INFO(CPU_CURRENT_TASK_PRI), prio);
+}
+
void mpic_teardown_this_cpu(int secondary)
{
struct mpic *mpic = mpic_primary;
--
2.1.0.27.g96db324
^ permalink raw reply related
* Re: [PATCH 3/3] CXL: Add reset to sysfs
From: Ian Munsie @ 2015-01-15 6:18 UTC (permalink / raw)
To: Ryan Grimm; +Cc: mikey, linuxppc-dev
In-Reply-To: <1421299043-sup-4240@delenn.ozlabs.ibm.com>
> > @@ -806,8 +837,8 @@ static int cxl_read_vsec(struct cxl *adapter, struct pci_dev *dev)
> > CXL_READ_VSEC_BASE_IMAGE(dev, vsec, &adapter->base_image);
> > CXL_READ_VSEC_IMAGE_STATE(dev, vsec, &image_state);
> > adapter->user_image_loaded = !!(image_state & CXL_VSEC_USER_IMAGE_LOADED);
> > - adapter->perst_loads_image = !!(image_state & CXL_VSEC_PERST_LOADS_IMAGE);
> > - adapter->perst_select_user = !!(image_state & CXL_VSEC_PERST_SELECT_USER);
> > + adapter->perst_loads_image = true;
> > + adapter->perst_select_user = !!(image_state & CXL_VSEC_USER_IMAGE_LOADED);
> ...
> > + if ((rc = cxl_update_image_control(adapter)))
> > + goto err2;
>
> Thanks - that seems like a better default than what we had before,
> should make things more stable :)
In fact, would you mind pulling this part out into a separate patch? It
seems like a serious contender to go to stable as it might help with
cards that get into a funny state and don't come back up properly after
a reboot (symptoms are that the adapter wide tlbia / slbia times out and
the driver aborts initialisation).
Cheers,
-Ian
^ permalink raw reply
* [git pull] Please pull mpe/linux.git powerpc-3.19-4 tag
From: Michael Ellerman @ 2015-01-15 6:28 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linuxppc-dev, anton, imunsie
[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]
Hi Linus,
Please pull some more powerpc fixes for 3.19:
The following changes since commit eaa27f34e91a14cdceed26ed6c6793ec1d186115:
linux 3.19-rc4 (2015-01-11 12:44:53 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux.git tags/powerpc-3.19-4
for you to fetch changes up to a87e810f61b49f19bd29ea564b7cd1e92e43d989:
powerpc: Work around gcc bug in current_thread_info() (2015-01-12 16:40:02 +1100)
----------------------------------------------------------------
Anton Blanchard (1):
powernv: Fix OPAL tracepoint code
Ian Munsie (1):
cxl: Fix issues when unmapping contexts
Michael Ellerman (1):
powerpc: Work around gcc bug in current_thread_info()
arch/powerpc/include/asm/thread_info.h | 13 ++--
arch/powerpc/platforms/powernv/opal-wrappers.S | 1 -
drivers/misc/cxl/context.c | 82 ++++++++++++++++++++------
drivers/misc/cxl/file.c | 14 +++--
4 files changed, 78 insertions(+), 32 deletions(-)
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH] fsl/smp: add low power boot support to replace spin boot
From: Dongsheng.Wang @ 2015-01-15 6:46 UTC (permalink / raw)
To: Dongsheng.Wang@freescale.com, Scott Wood, York Sun,
LeoLi@freescale.com
Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1421301930-10035-1-git-send-email-dongsheng.wang@freescale.com>
Hi all,
U-boot patch link:
http://patchwork.ozlabs.org/patch/429265/
Regards,
-Dongsheng
> -----Original Message-----
> From: Dongsheng Wang [mailto:dongsheng.wang@freescale.com]
> Sent: Thursday, January 15, 2015 2:06 PM
> To: Wood Scott-B07421; Sun York-R58495; Li Yang-Leo-R58472
> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> Subject: [PATCH] fsl/smp: add low power boot support to replace spin boot
>=20
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
>=20
> U-boot put non-boot cpus into an low power state(PW10/PW20 or DOZE) when =
cpu
> powered up. To exit low power state kernel will send DOORBELL or MPIC-IPI
> signal to all those CPUs.
>=20
> e500/e500v2 use mpic to send IPI signal.
> e500mc and later use doorbell to send IPI signal.
>=20
> This feature tested on:
> POWER UP TEST:
> P1022DS(e500v2),96k times.
> P4080(e500mc), 110k times.
> T1024(e5500), 83k times.
> T4240(e6500), 150k times.
>=20
> CPU HOTPLUG TEST:
> P1022DS(e500v2),1.4 million times.
> P4080(e500mc), 1.8 million times.
> T1024(e5500), 1.3 million times.
> T4240(e6500), 1.1 million times.
>=20
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
>=20
> diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/m=
pic.h
> index 754f93d..8af6a25 100644
> --- a/arch/powerpc/include/asm/mpic.h
> +++ b/arch/powerpc/include/asm/mpic.h
> @@ -474,6 +474,15 @@ extern int mpic_cpu_get_priority(void);
> /* Set the current cpu priority for this cpu */
> extern void mpic_cpu_set_priority(int prio);
>=20
> +/* Set cpu priority */
> +void mpic_set_cpu_priority(int nr, int prio);
> +
> +/* Set cpu EOI */
> +void mpic_cpu_eoi_write(int cpu);
> +
> +/* CPU ACK interrupt */
> +void mpic_cpu_ack(int cpu);
> +
> /* Request IPIs on primary mpic */
> extern void mpic_request_ipis(void);
>=20
> diff --git a/arch/powerpc/platforms/85xx/smp.c
> b/arch/powerpc/platforms/85xx/smp.c
> index d7c1e69..6c54632 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -193,6 +193,30 @@ static int smp_85xx_kick_cpu(int nr)
> const u64 *cpu_rel_addr;
> __iomem struct epapr_spin_table *spin_table;
> struct device_node *np;
> +
> + /*
> + * DOORBELL:
> + * When kernel kick one of cpus, all cpus will be wakenup. To make
> + * sure that only the target cpu is effected, other cpus (by checking
> + * spin_table->addr_l) should go back to low power state.
> + *
> + * U-boot has renumber the cpu PIR Why we need to set all of PIR to
> + * the same value?
> + * A: Before kernel kicking cpu, the doorbell message was not configure=
d
> + * for target cpu(cpu_messages->data). If we try to send a
> + * non-configured message to target cpu, it cannot correctly receive
> + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
> + * let all cpus catch the interrupt.
> + *
> + * Why set PIR to zero?
> + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow u=
s
> + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot
> + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always b=
e
> + * there. U-boot set PIR to zero as a default PIR ID for each CPU, so
> + * initialize the kick_cpus to 0.
> + */
> + u32 kick_cpus =3D 0;
> +
> int hw_cpu =3D get_hard_smp_processor_id(nr);
> int ioremappable;
> int ret =3D 0;
> @@ -251,8 +275,7 @@ static int smp_85xx_kick_cpu(int nr)
> spin_table =3D phys_to_virt(*cpu_rel_addr);
>=20
> local_irq_save(flags);
> -#ifdef CONFIG_PPC32
> -#ifdef CONFIG_HOTPLUG_CPU
> +#if defined(CONFIG_PPC32) && defined(CONFIG_HOTPLUG_CPU)
> /* Corresponding to generic_set_cpu_dead() */
> generic_set_cpu_up(nr);
>=20
> @@ -292,11 +315,58 @@ static int smp_85xx_kick_cpu(int nr)
> __secondary_hold_acknowledge =3D -1;
> }
> #endif
> +
> flush_spin_table(spin_table);
> - out_be32(&spin_table->pir, hw_cpu);
> + /*
> + * U-boot will wait kernel send eoi to MPIC, after EOI has send
> + * kernel will set PIR for uboot, let uboot know EOI has send.
> + */
> + out_be32(&spin_table->pir, 0);
> +
> +#ifdef CONFIG_PPC32
> out_be32(&spin_table->addr_l, __pa(__early_start));
> +#else
> + out_be64((u64 *)(&spin_table->addr_h),
> + __pa(ppc_function_entry(generic_secondary_smp_init)));
> +#endif
> flush_spin_table(spin_table);
>=20
> + /*
> + * e500, e500v2 need to use MPIC to send IPI signal, so we need to
> + * open IPI firstly.
> + */
> + if (!cpu_has_feature(CPU_FTR_DBELL)) {
> + mpic_set_cpu_priority(nr, 0);
> + kick_cpus =3D nr;
> + }
> +
> + /* Let cpu exit low power state, and from u-boot jump to kernel */
> + arch_send_call_function_single_ipi(kick_cpus);
> +
> + /*
> + * Let we ACK interrput and Send EOI signal to finish INT server
> + * U-boot has read EPR to ACK interrput when MPIC work in external
> + * proxy mode. Without the external proxy facility, we need to read
> + * MPIC ACK register.
> + *
> + * There just ACK interrput, we don't need to get the interrupt vector
> + * and to handle it. Because there just IPI or DOORBELL interrupt to
> + * make u-boot exit low power state and jump to kernel.
> + */
> + mpic_cpu_ack(nr);
> + /* Send EOI to clear ISR bit to remove interrupt from service */
> + mpic_cpu_eoi_write(nr);
> +
> + /* After wakeup CPU disable IPI, IPI will be opened in setup_cpu */
> + if (!cpu_has_feature(CPU_FTR_DBELL))
> + mpic_set_cpu_priority(nr, 0xf);
> +
> + /* After EOI finish, let we release cpu */
> + flush_spin_table(spin_table);
> + out_be32(&spin_table->pir, hw_cpu);
> + flush_spin_table(spin_table);
> +
> +#ifdef CONFIG_PPC32
> /* Wait a bit for the CPU to ack. */
> if (!spin_event_timeout(__secondary_hold_acknowledge =3D=3D hw_cpu,
> 10000, 100)) {
> @@ -308,12 +378,6 @@ static int smp_85xx_kick_cpu(int nr)
> out:
> #else
> smp_generic_kick_cpu(nr);
> -
> - flush_spin_table(spin_table);
> - out_be32(&spin_table->pir, hw_cpu);
> - out_be64((u64 *)(&spin_table->addr_h),
> - __pa(ppc_function_entry(generic_secondary_smp_init)));
> - flush_spin_table(spin_table);
> #endif
>=20
> local_irq_restore(flags);
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index c4648ad..b2ba47e 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -658,6 +658,21 @@ static inline void mpic_eoi(struct mpic *mpic)
> (void)mpic_cpu_read(MPIC_INFO(CPU_WHOAMI));
> }
>=20
> +void mpic_cpu_eoi_write(int cpu)
> +{
> + struct mpic *mpic =3D mpic_primary;
> +
> + _mpic_write(mpic->reg_type, &mpic->cpuregs[cpu], MPIC_INFO(CPU_EOI), 0)=
;
> + _mpic_read(mpic->reg_type, &mpic->cpuregs[cpu], MPIC_INFO(CPU_WHOAMI));
> +}
> +
> +void mpic_cpu_ack(int cpu)
> +{
> + struct mpic *mpic =3D mpic_primary;
> +
> + _mpic_read(mpic->reg_type, &mpic->cpuregs[cpu], MPIC_INFO(CPU_INTACK));
> +}
> +
> /*
> * Linux descriptor level callbacks
> */
> @@ -1778,6 +1793,16 @@ void mpic_cpu_set_priority(int prio)
> mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), prio);
> }
>=20
> +void mpic_set_cpu_priority(int nr, int prio)
> +{
> + struct mpic *mpic =3D mpic_primary;
> + int hw_cpu =3D get_hard_smp_processor_id(nr);
> +
> + prio &=3D MPIC_CPU_TASKPRI_MASK;
> + _mpic_write(mpic->reg_type, &mpic->cpuregs[hw_cpu],
> + MPIC_INFO(CPU_CURRENT_TASK_PRI), prio);
> +}
> +
> void mpic_teardown_this_cpu(int secondary)
> {
> struct mpic *mpic =3D mpic_primary;
> --
> 2.1.0.27.g96db324
^ permalink raw reply
* [PATCH 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/powerpc/kvm/book3s_hv_rm_xics.c | 8 ++++----
arch/powerpc/kvm/book3s_xics.c | 16 ++++++++--------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c b/arch/powerpc/kvm/book3s_hv_rm_xics.c
index 7b066f6..7c22997 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
@@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics, struct kvmppc_icp *icp,
* in virtual mode.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -211,7 +211,7 @@ unsigned long kvmppc_rm_h_xirr(struct kvm_vcpu *vcpu)
* pending priority
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
xirr = old_state.xisr | (((u32)old_state.cppr) << 24);
if (!old_state.xisr)
@@ -277,7 +277,7 @@ int kvmppc_rm_h_ipi(struct kvm_vcpu *vcpu, unsigned long server,
* whenever the MFRR is made less favored.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -352,7 +352,7 @@ int kvmppc_rm_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr)
icp_rm_clr_vcpu_irq(icp->vcpu);
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
reject = 0;
new_state.cppr = cppr;
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 807351f..a4a8d9f 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -327,7 +327,7 @@ static bool icp_try_to_deliver(struct kvmppc_icp *icp, u32 irq, u8 priority,
icp->server_num);
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
*reject = 0;
@@ -512,7 +512,7 @@ static void icp_down_cppr(struct kvmppc_xics *xics, struct kvmppc_icp *icp,
* in virtual mode.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -567,7 +567,7 @@ static noinline unsigned long kvmppc_h_xirr(struct kvm_vcpu *vcpu)
* pending priority
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
xirr = old_state.xisr | (((u32)old_state.cppr) << 24);
if (!old_state.xisr)
@@ -634,7 +634,7 @@ static noinline int kvmppc_h_ipi(struct kvm_vcpu *vcpu, unsigned long server,
* whenever the MFRR is made less favored.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -679,7 +679,7 @@ static int kvmppc_h_ipoll(struct kvm_vcpu *vcpu, unsigned long server)
if (!icp)
return H_PARAMETER;
}
- state = ACCESS_ONCE(icp->state);
+ state = READ_ONCE(icp->state);
kvmppc_set_gpr(vcpu, 4, ((u32)state.cppr << 24) | state.xisr);
kvmppc_set_gpr(vcpu, 5, state.mfrr);
return H_SUCCESS;
@@ -721,7 +721,7 @@ static noinline void kvmppc_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr)
BOOK3S_INTERRUPT_EXTERNAL_LEVEL);
do {
- old_state = new_state = ACCESS_ONCE(icp->state);
+ old_state = new_state = READ_ONCE(icp->state);
reject = 0;
new_state.cppr = cppr;
@@ -885,7 +885,7 @@ static int xics_debug_show(struct seq_file *m, void *private)
if (!icp)
continue;
- state.raw = ACCESS_ONCE(icp->state.raw);
+ state.raw = READ_ONCE(icp->state.raw);
seq_printf(m, "cpu server %#lx XIRR:%#x PPRI:%#x CPPR:%#x MFRR:%#x OUT:%d NR:%d\n",
icp->server_num, state.xisr,
state.pending_pri, state.cppr, state.mfrr,
@@ -1082,7 +1082,7 @@ int kvmppc_xics_set_icp(struct kvm_vcpu *vcpu, u64 icpval)
* the ICS states before the ICP states.
*/
do {
- old_state = ACCESS_ONCE(icp->state);
+ old_state = READ_ONCE(icp->state);
if (new_state.mfrr <= old_state.mfrr) {
resend = false;
--
1.9.3
^ permalink raw reply related
* [PATCH 0/8] current ACCESS_ONCE patch queue
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, linuxppc-dev
Folks,
fyi, this is my current patch queue for the next merge window. It
does contain a patch that will disallow ACCESS_ONCE on non-scalar
types.
The tree is part of linux-next and can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/borntraeger/linux.git linux-next
Christian Borntraeger (7):
ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE
x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
mm/gup: Replace ACCESS_ONCE with READ_ONCE
kernel: tighten rules for ACCESS ONCE
kernel: Fix sparse warning for ACCESS_ONCE
Guenter Roeck (1):
next: sh: Fix compile error
arch/powerpc/kvm/book3s_hv_rm_xics.c | 8 ++++----
arch/powerpc/kvm/book3s_xics.c | 16 ++++++++--------
arch/powerpc/mm/hugetlbpage.c | 4 ++--
arch/sh/mm/gup.c | 2 +-
arch/x86/include/asm/spinlock.h | 2 +-
arch/x86/xen/p2m.c | 2 +-
include/linux/compiler.h | 21 ++++++++++++++++-----
mm/gup.c | 2 +-
8 files changed, 34 insertions(+), 23 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH 2/8] ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the ppc/hugetlbfs code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/powerpc/mm/hugetlbpage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 5ff4e07..620d0ec 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -978,7 +978,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea, unsigned *shift
*/
pdshift = PUD_SHIFT;
pudp = pud_offset(&pgd, ea);
- pud = ACCESS_ONCE(*pudp);
+ pud = READ_ONCE(*pudp);
if (pud_none(pud))
return NULL;
@@ -990,7 +990,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned long ea, unsigned *shift
else {
pdshift = PMD_SHIFT;
pmdp = pmd_offset(&pud, ea);
- pmd = ACCESS_ONCE(*pmdp);
+ pmd = READ_ONCE(*pmdp);
/*
* A hugepage collapse is captured by pmd_none, because
* it mark the pmd none and do a hpte invalidate.
--
1.9.3
^ permalink raw reply related
* [PATCH 5/8] mm/gup: Replace ACCESS_ONCE with READ_ONCE
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Fixup gup_pmd_range.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/gup.c b/mm/gup.c
index a900759..bed30efa 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -926,7 +926,7 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
pmdp = pmd_offset(&pud, addr);
do {
- pmd_t pmd = ACCESS_ONCE(*pmdp);
+ pmd_t pmd = READ_ONCE(*pmdp);
next = pmd_addr_end(addr, end);
if (pmd_none(pmd) || pmd_trans_splitting(pmd))
--
1.9.3
^ permalink raw reply related
* [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE->READ_ONCE
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, Oleg Nesterov,
kvm-ppc, linux-mm, xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
commit 78bff1c8684f ("x86/ticketlock: Fix spin_unlock_wait() livelock")
introduced another ACCESS_ONCE case in x86 spinlock.h.
Change that as well.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Oleg Nesterov <oleg@redhat.com>
---
arch/x86/include/asm/spinlock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 625660f..9264f0f 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
__ticket_t head = ACCESS_ONCE(lock->tickets.head);
for (;;) {
- struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
+ struct __raw_tickets tmp = READ_ONCE(lock->tickets);
/*
* We need to check "unlocked" in a loop, tmp.head == head
* can be false positive because of overflow.
--
1.9.3
^ permalink raw reply related
* [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/x86/xen/p2m.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index edbc7a6..cb71016 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -554,7 +554,7 @@ static bool alloc_p2m(unsigned long pfn)
mid_mfn = NULL;
}
- p2m_pfn = pte_pfn(ACCESS_ONCE(*ptep));
+ p2m_pfn = pte_pfn(READ_ONCE(*ptep));
if (p2m_pfn == PFN_DOWN(__pa(p2m_identity)) ||
p2m_pfn == PFN_DOWN(__pa(p2m_missing))) {
/* p2m leaf page is missing */
--
1.9.3
^ permalink raw reply related
* [PATCH 6/8] kernel: tighten rules for ACCESS ONCE
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
Now that all non-scalar users of ACCESS_ONCE have been converted
to READ_ONCE or ASSIGN once, lets tighten ACCESS_ONCE to only
work on scalar types.
This variant was proposed by Alexei Starovoitov.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
include/linux/compiler.h | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index a1c81f8..5e186bf 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -447,12 +447,23 @@ static __always_inline void __assign_once_size(volatile void *p, void *res, int
* to make the compiler aware of ordering is to put the two invocations of
* ACCESS_ONCE() in different C statements.
*
- * This macro does absolutely -nothing- to prevent the CPU from reordering,
- * merging, or refetching absolutely anything at any time. Its main intended
- * use is to mediate communication between process-level code and irq/NMI
- * handlers, all running on the same CPU.
+ * ACCESS_ONCE will only work on scalar types. For union types, ACCESS_ONCE
+ * on a union member will work as long as the size of the member matches the
+ * size of the union and the size is smaller than word size.
+ *
+ * The major use cases of ACCESS_ONCE used to be (1) Mediating communication
+ * between process-level code and irq/NMI handlers, all running on the same CPU,
+ * and (2) Ensuring that the compiler does not fold, spindle, or otherwise
+ * mutilate accesses that either do not require ordering or that interact
+ * with an explicit memory barrier or atomic instruction that provides the
+ * required ordering.
+ *
+ * If possible use READ_ONCE/ASSIGN_ONCE instead.
*/
-#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
+#define __ACCESS_ONCE(x) ({ \
+ __maybe_unused typeof(x) __var = 0; \
+ (volatile typeof(x) *)&(x); })
+#define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
/* Ignore/forbid kprobes attach on very low level functions marked by this attribute: */
#ifdef CONFIG_KPROBES
--
1.9.3
^ permalink raw reply related
* [PATCH 8/8] kernel: Fix sparse warning for ACCESS_ONCE
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
Commit a91ed664749c ("kernel: tighten rules for ACCESS ONCE") results in
sparse warnings like "Using plain integer as NULL pointer" - Let's add a
type cast to the dummy assignment.
To avoid warnings lik "sparse: warning: cast to restricted __hc32" we also
use __force on that cast.
Fixes: a91ed664749c ("kernel: tighten rules for ACCESS ONCE")
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
include/linux/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 5e186bf..7bebf05 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -461,7 +461,7 @@ static __always_inline void __assign_once_size(volatile void *p, void *res, int
* If possible use READ_ONCE/ASSIGN_ONCE instead.
*/
#define __ACCESS_ONCE(x) ({ \
- __maybe_unused typeof(x) __var = 0; \
+ __maybe_unused typeof(x) __var = (__force typeof(x)) 0; \
(volatile typeof(x) *)&(x); })
#define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
--
1.9.3
^ permalink raw reply related
* [PATCH 7/8] next: sh: Fix compile error
From: Christian Borntraeger @ 2015-01-15 8:58 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arch, kvm, Christian Borntraeger, x86, kvm-ppc, linux-mm,
xen-devel, Paul E. McKenney, linuxppc-dev, Guenter Roeck
In-Reply-To: <1421312314-72330-1-git-send-email-borntraeger@de.ibm.com>
From: Guenter Roeck <linux@roeck-us.net>
Commit a91ed664749c ("kernel: tighten rules for ACCESS ONCE") results in a
compile failure for sh builds with CONFIG_X2TLB enabled.
arch/sh/mm/gup.c: In function 'gup_get_pte':
arch/sh/mm/gup.c:20:2: error: invalid initializer
make[1]: *** [arch/sh/mm/gup.o] Error 1
Replace ACCESS_ONCE with READ_ONCE to fix the problem.
Fixes: a91ed664749c ("kernel: tighten rules for ACCESS ONCE")
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/sh/mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sh/mm/gup.c b/arch/sh/mm/gup.c
index 37458f3..e113bb4 100644
--- a/arch/sh/mm/gup.c
+++ b/arch/sh/mm/gup.c
@@ -17,7 +17,7 @@
static inline pte_t gup_get_pte(pte_t *ptep)
{
#ifndef CONFIG_X2TLB
- return ACCESS_ONCE(*ptep);
+ return READ_ONCE(*ptep);
#else
/*
* With get_user_pages_fast, we walk down the pagetables without
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
From: Jürgen Groß @ 2015-01-15 9:26 UTC (permalink / raw)
To: Christian Borntraeger, linux-kernel
Cc: linux-arch, kvm, x86, kvm-ppc, linux-mm, xen-devel, linuxppc-dev
In-Reply-To: <1421312314-72330-4-git-send-email-borntraeger@de.ibm.com>
On 01/15/2015 09:58 AM, Christian Borntraeger wrote:
> ACCESS_ONCE does not work reliably on non-scalar types. For
> example gcc 4.6 and 4.7 might remove the volatile tag for such
> accesses during the SRA (scalar replacement of aggregates) step
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
>
> Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
> ---
> arch/x86/xen/p2m.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
> index edbc7a6..cb71016 100644
> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -554,7 +554,7 @@ static bool alloc_p2m(unsigned long pfn)
> mid_mfn = NULL;
> }
>
> - p2m_pfn = pte_pfn(ACCESS_ONCE(*ptep));
> + p2m_pfn = pte_pfn(READ_ONCE(*ptep));
> if (p2m_pfn == PFN_DOWN(__pa(p2m_identity)) ||
> p2m_pfn == PFN_DOWN(__pa(p2m_missing))) {
> /* p2m leaf page is missing */
>
^ permalink raw reply
* Re: linux-next: Tree for Jan 12 (build failures: m68k, ppc)
From: Geert Uytterhoeven @ 2015-01-15 10:12 UTC (permalink / raw)
To: Guenter Roeck
Cc: Stephen Rothwell, linux-m68k, linux-kernel@vger.kernel.org,
Rob Clark, Linux-Next, Daniel Vetter,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20150112175920.GC19868@roeck-us.net>
On Mon, Jan 12, 2015 at 6:59 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> > Build failures, seen since next-20150109:
>> > m68k:allmodconfig
>> > powerpc:ppc6xx_defconfig
It looks like parisc is also suffering:
http://kisskb.ellerman.id.au/kisskb/buildresult/12343847/
>> > Due to:
>> > ERROR: "__get_user_bad" [drivers/gpu/drm/drm.ko] undefined!
>> > make[1]: *** [__modpost] Error 1
>> >
>> > Caused by commit d34f20d6e2f (drm: Atomic modeset ioctl).
>>
>> Yeah, it needs a get_user() that supports 64-bit data.
>>
> Hi Geert,
>
> I assume you mean m68k, where 64 bit support for get_user has been disabled.
>
> The problem on powerpc is different though: __get_user_nocheck()
> and __get_user_check() use
> unsigned long __gu_val;
> followed by
> __get_user_size(__gu_val, __gu_addr, (size), __gu_err);
>
> __get_user_size() fails in
> if (size > sizeof(x))
> (x) = __get_user_bad();
>
> Presumably "unsigned long" is 32 bit on 32 bit powerpc, not 64 bit.
>
> Overall, the explicit 64-bit use of get_user() seems to be quite unusual.
I noticed you've sent a fix for DRM.
Doh, and I was just fixing m68k...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox