* Re: [PATCH v1] soc: fsl: rcpm: Add ACPI support
From: Li Yang @ 2020-09-22 23:13 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Biwen Li, kuldip dwivedi, Arokia Samy, Paul Yang,
linux-kernel@vger.kernel.org, Samer El-Haj-Mahmoud, Varun Sethi,
tanveer, Ran Wang, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <caf01871-1c3d-bdf8-867d-daf7138966a8@arm.com>
On Wed, Sep 16, 2020 at 1:12 AM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
>
> On 9/16/20 3:32 AM, Ran Wang wrote:
> > Hi Ard,
> >
> > On Tuesday, September 15, 2020 7:10 PM, Ard Biesheuvel wrote:
> >> Subject: Re: [PATCH v1] soc: fsl: rcpm: Add ACPI support
> >>
> >> On 9/15/20 1:06 PM, kuldip dwivedi wrote:
> >>> Add ACPI support in fsl RCPM driver. This is required to support ACPI
> >>> S3 state. S3 is the ACPI sleep state that is known as "sleep" or
> >>> "suspend to RAM".
> >>> It essentially turns off most power of the system but keeps memory
> >>> powered.
> >>>
> >>> Signed-off-by: tanveer <tanveer.alam@puresoftware.com>
> >>> Signed-off-by: kuldip dwivedi <kuldip.dwivedi@puresoftware.com>
> >>
> >> Why does the OS need to program this device? Can't this be done by
> >> firmware?
> >
> > This device is use to tell HW which IP (such as USB, SDHC, SATA, etc) should not be
> > clock gated during system enter low power state (to allow that IP work as a
> > wakeup source). And user does this configuration in device tree.
>
> The point of ACPI is *not* to describe a DT topology using a table
> format that is not suited for it. The point of ACPI is to describe a
> machine that is more abstracted from the hardware than is typically
> possible with DT, where the abstractions are implemented by AML code
> that is provided by the firmware, but executed in the context of the OS.
>
> So the idea is *not* finding the shortest possible path to get your
> existing DT driver code running on a system that boots via ACPI.
> Instead, you should carefully think about the abstract ACPI machine that
> you will expose to the OS, and hide everything else in firmware.
>
> In this particular case, it seems like your USB, SDHC and SATA device
> objects may need power state dependent AML methods that program this
> block directly.
The platform PM driver was created to support PM on systems without a
runtime PM firmware. Even with PSCI firmware on later systems, there
is no standard interface to communicate the wakeup source information
directly from peripheral drivers to the PSCI firmware. So we still
need this platform power management driver in kernel to deal with this
setup for non-ACPI scenarios. From the code re-use perspective, I
think it is probably better to keep this generic implementation in
kernel instead of moving it to ACPI byte-code for each platform.
>
>
>
> > So implement
> > this RCPM driver to do it in kernel rather than firmware.
> >
> > Regards,
> > Ran
> >
> >>> ---
> >>>
> >>> Notes:
> >>> 1. Add ACPI match table
> >>> 2. NXP team members are added for confirming HID changes
> >>> 3. There is only one node in ACPI so no need to check for
> >>> current device explicitly
> >>> 4. These changes are tested on LX2160A and LS1046A platforms
> >>>
> >>> drivers/soc/fsl/rcpm.c | 22 +++++++++++++++++++---
> >>> 1 file changed, 19 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c index
> >>> a093dbe6d2cb..e75a436fb159 100644
> >>> --- a/drivers/soc/fsl/rcpm.c
> >>> +++ b/drivers/soc/fsl/rcpm.c
> >>> @@ -2,10 +2,12 @@
> >>> //
> >>> // rcpm.c - Freescale QorIQ RCPM driver
> >>> //
> >>> -// Copyright 2019 NXP
> >>> +// Copyright 2019-2020 NXP
> >>> +// Copyright 2020 Puresoftware Ltd.
> >>> //
> >>> // Author: Ran Wang <ran.wang_1@nxp.com>
> >>>
> >>> +#include <linux/acpi.h>
> >>> #include <linux/init.h>
> >>> #include <linux/module.h>
> >>> #include <linux/platform_device.h>
> >>> @@ -57,8 +59,13 @@ static int rcpm_pm_prepare(struct device *dev)
> >>> rcpm->wakeup_cells + 1);
> >>>
> >>> /* Wakeup source should refer to current rcpm device */
> >>> - if (ret || (np->phandle != value[0]))
> >>> - continue;
> >>> + if (is_acpi_node(dev->fwnode)) {
> >>> + if (ret)
> >>> + continue;
> >>> + } else {
> >>> + if (ret || (np->phandle != value[0]))
> >>> + continue;
> >>> + }
> >>>
> >>> /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
> >>> * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
> >>> @@ -139,10 +146,19 @@ static const struct of_device_id rcpm_of_match[]
> >> = {
> >>> };
> >>> MODULE_DEVICE_TABLE(of, rcpm_of_match);
> >>>
> >>> +#ifdef CONFIG_ACPI
> >>> +static const struct acpi_device_id rcpm_acpi_match[] = {
> >>> + { "NXP0015", },
> >>> + { }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_match); #endif
> >>> +
> >>> static struct platform_driver rcpm_driver = {
> >>> .driver = {
> >>> .name = "rcpm",
> >>> .of_match_table = rcpm_of_match,
> >>> + .acpi_match_table = ACPI_PTR(rcpm_acpi_match),
> >>> .pm = &rcpm_pm_ops,
> >>> },
> >>> .probe = rcpm_probe,
> >>>
> >
>
^ permalink raw reply
* Re: [PATCH] soc: fsl: dpio: remove set but not used 'addr_cena'
From: Li Yang @ 2020-09-22 22:44 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jason Yan, Roy Pledge, linux-kernel@vger.kernel.org, Youri Querry,
Hulk Robot, linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAJKOXPcsrL2T9EmiSnOhrC5Lh+ZM=nY4Go52cVDv1S6wxn5DHQ@mail.gmail.com>
On Sun, Sep 20, 2020 at 3:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Thu, 10 Sep 2020 at 16:57, Jason Yan <yanaijie@huawei.com> wrote:
> >
> > This addresses the following gcc warning with "make W=1":
> >
> > drivers/soc/fsl/dpio/qbman-portal.c: In function
> > ‘qbman_swp_enqueue_multiple_direct’:
> > drivers/soc/fsl/dpio/qbman-portal.c:650:11: warning: variable
> > ‘addr_cena’ set but not used [-Wunused-but-set-variable]
> > 650 | uint64_t addr_cena;
> > | ^~~~~~~~~
> >
> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Signed-off-by: Jason Yan <yanaijie@huawei.com>
>
> This was already reported:
> Reported-by: kernel test robot <lkp@intel.com>
> https://lkml.org/lkml/2020/6/12/290
>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Applied for next. Thanks.
>
> Best regards,
> Krzysztof
>
> > ---
> > drivers/soc/fsl/dpio/qbman-portal.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
> > index 0ab85bfb116f..659b4a570d5b 100644
> > --- a/drivers/soc/fsl/dpio/qbman-portal.c
> > +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> > @@ -647,7 +647,6 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
> > const uint32_t *cl = (uint32_t *)d;
> > uint32_t eqcr_ci, eqcr_pi, half_mask, full_mask;
> > int i, num_enqueued = 0;
> > - uint64_t addr_cena;
> >
> > spin_lock(&s->access_spinlock);
> > half_mask = (s->eqcr.pi_ci_mask>>1);
> > @@ -701,7 +700,6 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
> >
> > /* Flush all the cacheline without load/store in between */
> > eqcr_pi = s->eqcr.pi;
> > - addr_cena = (size_t)s->addr_cena;
> > for (i = 0; i < num_enqueued; i++)
> > eqcr_pi++;
> > s->eqcr.pi = eqcr_pi & full_mask;
> > --
> > 2.25.4
> >
^ permalink raw reply
* Re: [PATCH] soc: fsl: qbman: Fix return value on success
From: Li Yang @ 2020-09-22 22:42 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Roy Pledge, linuxppc-dev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200920202625.11377-1-krzk@kernel.org>
On Sun, Sep 20, 2020 at 3:27 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On error the function was meant to return -ERRNO. This also fixes
> compile warning:
>
> drivers/soc/fsl/qbman/bman.c:640:6: warning: variable 'err' set but not used [-Wunused-but-set-variable]
>
> Fixes: 0505d00c8dba ("soc/fsl/qbman: Cleanup buffer pools if BMan was initialized prior to bootup")
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Applied for next. Thanks.
> ---
> drivers/soc/fsl/qbman/bman.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman.c b/drivers/soc/fsl/qbman/bman.c
> index f4fb527d8301..c5dd026fe889 100644
> --- a/drivers/soc/fsl/qbman/bman.c
> +++ b/drivers/soc/fsl/qbman/bman.c
> @@ -660,7 +660,7 @@ int bm_shutdown_pool(u32 bpid)
> }
> done:
> put_affine_portal();
> - return 0;
> + return err;
> }
>
> struct gen_pool *bm_bpalloc;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH] ibmvfc: Protect vhost->task_set increment by the host lock
From: Martin K. Petersen @ 2020-09-22 21:39 UTC (permalink / raw)
To: Brian King; +Cc: tyreld, linuxppc-dev, martin.petersen, linux-scsi
In-Reply-To: <1600286999-22059-1-git-send-email-brking@linux.vnet.ibm.com>
Brian,
> In the discovery thread, ibmvfc does a vhost->task_set++ without any
> lock held. This could result in two targets getting the same cancel
> key, which could have strange effects in error recovery. The actual
> probability of this occurring should be extremely small, since this
> should all be done in a single threaded loop from the discovery
> thread, but let's fix it up anyway to be safe.
Applied to 5.10/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH v3 2/5] powerpc: apm82181: create shared dtsi for APM bluestone
From: Rob Herring @ 2020-09-22 19:14 UTC (permalink / raw)
To: Christian Lamparter
Cc: devicetree, Arnd Bergmann, Chris Blake, Paul Mackerras,
Andy Shevchenko, linuxppc-dev
In-Reply-To: <7bf866fd-6499-68e4-9825-5c3e2042ef65@gmail.com>
On Sat, Sep 19, 2020 at 2:23 PM Christian Lamparter <chunkeey@gmail.com> wrote:
>
> On 2020-09-15 03:05, Rob Herring wrote:
> > On Sun, Sep 06, 2020 at 12:06:12AM +0200, Christian Lamparter wrote:
> >> This patch adds an DTSI-File that can be used by various device-tree
> >> files for APM82181-based devices.
> >>
> >> Some of the nodes (like UART, PCIE, SATA) are used by the uboot and
> >> need to stick with the naming-conventions of the old times'.
> >> I've added comments whenever this was the case.
> >>
> >> Signed-off-by: Chris Blake <chrisrblake93@gmail.com>
> >> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> >> ---
> >> rfc v1 -> v2:
> >> - removed PKA (this CryptoPU will need driver)
> >> - stick with compatibles, nodes, ... from either
> >> Bluestone (APM82181) or Canyonlands (PPC460EX).
> >> - add labels for NAND and NOR to help with access.
> >> v2 -> v3:
> >> - nodename of pciex@d.... was changed to pcie@d..
> >> due to upstream patch.
> >> - use simple-bus on the ebc, opb and plb nodes
> >> ---
> >> arch/powerpc/boot/dts/apm82181.dtsi | 466 ++++++++++++++++++++++++++++
> >> 1 file changed, 466 insertions(+)
> >> create mode 100644 arch/powerpc/boot/dts/apm82181.dtsi
> >>
> >> diff --git a/arch/powerpc/boot/dts/apm82181.dtsi b/arch/powerpc/boot/dts/apm82181.dtsi
> >> new file mode 100644
> >> index 000000000000..60283430978d
> >> --- /dev/null
> >> +++ b/arch/powerpc/boot/dts/apm82181.dtsi
> >> @@ -0,0 +1,466 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * Device Tree template include for various APM82181 boards.
> >> + *
> >> + * The SoC is an evolution of the PPC460EX predecessor.
> >> + * This is why dt-nodes from the canyonlands EBC, OPB, USB,
> >> + * DMA, SATA, EMAC, ... ended up in here.
> >> + *
> >> + * Copyright (c) 2010, Applied Micro Circuits Corporation
> >> + * Author: Tirumala R Marri <tmarri@apm.com>,
> >> + * Christian Lamparter <chunkeey@gmail.com>,
> >> + * Chris Blake <chrisrblake93@gmail.com>
> >> + */
> >> +
> >> +#include <dt-bindings/dma/dw-dmac.h>
> >> +#include <dt-bindings/input/input.h>
> >> +#include <dt-bindings/interrupt-controller/irq.h>
> >> +#include <dt-bindings/gpio/gpio.h>
> >> +
> >> +/ {
> >> + #address-cells = <2>;
> >> + #size-cells = <1>;
> >> + dcr-parent = <&{/cpus/cpu@0}>;
> >> +
> >> + aliases {
> >> + ethernet0 = &EMAC0; /* needed for BSP u-boot */
> >> + };
> >> +
> >> + cpus {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + CPU0: cpu@0 {
> >> + device_type = "cpu";
> >> + model = "PowerPC,apm82181";
> >
> > This doesn't match the existing bluestone dts file.
> >
> > Please separate any restructuring from changes.
>
>
> "I see" (I'm including your comment of the dt-binding as well).
>
> I'm getting the vibe that I better should not touch that bluestone.dts.
I don't know about that.
> An honestly, looking at the series and patches that the APM-engineers
> posted back in the day, I can see why this well is so poisoned... and
> stuff like SATA/AHBDMA/USB/GPIO/CPM/... was missing.
>
> As for the devices. In the spirit of Arnd Bergmann's post of
> <https://lkml.org/lkml/2020/3/30/195>
>
> |It would be nice to move over the the bluestone .dts to the apm82181.dtsi file
> |when that gets added, if only to ensure they use the same description for each
> |node, but that shouldn't stop the addition of the new file if that is needed for
> |distros to make use of a popular device.
> |I see a couple of additional files in openwrt.
>
> I mean I don't have the bluestone dev board, just the consumer devices.
This stuff is old enough, I'd guess no one cares about a dev board.
But we should figure that out and document that with any changes.
> Would it be possible to support those? I can start from a "skeleton" apm82181.dtsi
> This would just include CPU, Memory (SD-RAM+L2C+OCM), UIC (Interrupt-Controller),
> the PLB+OBP+EBC Busses and UART. Just enough to make a board "boot from ram".
This skeleton would be chunks moved over or duplicated? I don't think
we want 2 of the same thing.
The order I would go is split into an apm82181.dtsi with 0 changes to
the built dtb(s). Then make changes/additions you need. As far as
changes to existing bindings, it's only an ABI if someone notices.
Rob
^ permalink raw reply
* Re: [PATCH 1/9] kernel: add a PF_FORCE_COMPAT flag
From: Andy Lutomirski @ 2020-09-22 16:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-aio, open list:MIPS, David Howells, Linux-MM, keyrings,
sparclinux, Christoph Hellwig, linux-arch, linux-s390,
Linux SCSI List, X86 ML, linux-block, Al Viro, Andy Lutomirski,
io-uring, linux-arm-kernel, Jens Axboe, Parisc List,
Network Development, LKML, LSM List, Linux FS Devel,
Andrew Morton, linuxppc-dev, Pavel Begunkov
In-Reply-To: <CAK8P3a39jN+t2hhLg0oKZnbYATQXmYE2-Z1JkmFyc1EPdg1HXw@mail.gmail.com>
> On Sep 22, 2020, at 2:01 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Sep 22, 2020 at 9:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>> On 22/09/2020 10:23, Arnd Bergmann wrote:
>>> On Tue, Sep 22, 2020 at 8:32 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>> On 22/09/2020 03:58, Andy Lutomirski wrote:
>>>>> On Mon, Sep 21, 2020 at 5:24 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>>> I may be looking at a different kernel than you, but aren't you
>>>>> preventing creating an io_uring regardless of whether SQPOLL is
>>>>> requested?
>>>>
>>>> I diffed a not-saved file on a sleepy head, thanks for noticing.
>>>> As you said, there should be an SQPOLL check.
>>>>
>>>> ...
>>>> if (ctx->compat && (p->flags & IORING_SETUP_SQPOLL))
>>>> goto err;
>>>
>>> Wouldn't that mean that now 32-bit containers behave differently
>>> between compat and native execution?
>>>
>>> I think if you want to prevent 32-bit applications from using SQPOLL,
>>> it needs to be done the same way on both to be consistent:
>>
>> The intention was to disable only compat not native 32-bit.
>
> I'm not following why that would be considered a valid option,
> as that clearly breaks existing users that update from a 32-bit
> kernel to a 64-bit one.
>
> Taking away the features from users that are still on 32-bit kernels
> already seems questionable to me, but being inconsistent
> about it seems much worse, in particular when the regression
> is on the upgrade path.
>
>>> Can we expect all existing and future user space to have a sane
>>> fallback when IORING_SETUP_SQPOLL fails?
>>
>> SQPOLL has a few differences with non-SQPOLL modes, but it's easy
>> to convert between them. Anyway, SQPOLL is a privileged special
>> case that's here for performance/latency reasons, I don't think
>> there will be any non-accidental users of it.
>
> Ok, so the behavior of 32-bit tasks would be the same as running
> the same application as unprivileged 64-bit tasks, with applications
> already having to implement that fallback, right?
>
>
I don’t have any real preference wrt SQPOLL, and it may be that we have a problem even without SQPOLL when IO gets punted without one of the fixes discussed.
But banning the mismatched io_uring and io_uring_enter seems like it may be worthwhile regardless.
^ permalink raw reply
* Re: [PATCH v2 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver
From: Philipp Zabel @ 2020-09-22 12:08 UTC (permalink / raw)
To: Viorel Suman (OSS)
Cc: devicetree, alsa-devel, Matthias Schiffer, Timur Tabi, Xiubo Li,
Shengjiu Wang, linuxppc-dev, Takashi Iwai, Rob Herring,
Liam Girdwood, Nicolin Chen, Mark Brown, NXP Linux Team,
Viorel Suman, Viorel Suman, Cosmin-Gabriel Samoila,
Jaroslav Kysela, Fabio Estevam, linux-kernel
In-Reply-To: <1600715292-28529-2-git-send-email-viorel.suman@oss.nxp.com>
On Mon, Sep 21, 2020 at 10:08:11PM +0300, Viorel Suman (OSS) wrote:
> From: Viorel Suman <viorel.suman@nxp.com>
>
> XCVR (Audio Transceiver) is a on-chip functional module found
> on i.MX8MP. It support HDMI2.1 eARC, HDMI1.4 ARC and SPDIF.
>
> Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
> ---
> sound/soc/fsl/Kconfig | 10 +
> sound/soc/fsl/Makefile | 2 +
> sound/soc/fsl/fsl_xcvr.c | 1343 ++++++++++++++++++++++++++++++++++++++++++++++
> sound/soc/fsl/fsl_xcvr.h | 266 +++++++++
> 4 files changed, 1621 insertions(+)
> create mode 100644 sound/soc/fsl/fsl_xcvr.c
> create mode 100644 sound/soc/fsl/fsl_xcvr.h
>
> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> index 3f76ff7..d04b64d 100644
> --- a/sound/soc/fsl/Kconfig
> +++ b/sound/soc/fsl/Kconfig
> @@ -95,6 +95,16 @@ config SND_SOC_FSL_EASRC
> destination sample rate. It is a new design module compare with the
> old ASRC.
>
> +config SND_SOC_FSL_XCVR
> + tristate "NXP Audio Transceiver (XCVR) module support"
> + select REGMAP_MMIO
> + select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
> + select SND_SOC_GENERIC_DMAENGINE_PCM
> + help
> + Say Y if you want to add Audio Transceiver (XCVR) support for NXP
> + iMX CPUs. XCVR is a digital module that supports HDMI2.1 eARC,
> + HDMI1.4 ARC and SPDIF.
> +
> config SND_SOC_FSL_UTILS
> tristate
>
> diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
> index b835eeb..1d2231f 100644
> --- a/sound/soc/fsl/Makefile
> +++ b/sound/soc/fsl/Makefile
> @@ -25,6 +25,7 @@ snd-soc-fsl-utils-objs := fsl_utils.o
> snd-soc-fsl-dma-objs := fsl_dma.o
> snd-soc-fsl-mqs-objs := fsl_mqs.o
> snd-soc-fsl-easrc-objs := fsl_easrc.o
> +snd-soc-fsl-xcvr-objs := fsl_xcvr.o
>
> obj-$(CONFIG_SND_SOC_FSL_AUDMIX) += snd-soc-fsl-audmix.o
> obj-$(CONFIG_SND_SOC_FSL_ASOC_CARD) += snd-soc-fsl-asoc-card.o
> @@ -38,6 +39,7 @@ obj-$(CONFIG_SND_SOC_FSL_UTILS) += snd-soc-fsl-utils.o
> obj-$(CONFIG_SND_SOC_FSL_MQS) += snd-soc-fsl-mqs.o
> obj-$(CONFIG_SND_SOC_FSL_EASRC) += snd-soc-fsl-easrc.o
> obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o
> +obj-$(CONFIG_SND_SOC_FSL_XCVR) += snd-soc-fsl-xcvr.o
>
> # MPC5200 Platform Support
> obj-$(CONFIG_SND_MPC52xx_DMA) += mpc5200_dma.o
> diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
> new file mode 100644
> index 00000000..7391bca
> --- /dev/null
> +++ b/sound/soc/fsl/fsl_xcvr.c
> @@ -0,0 +1,1343 @@
[...]
> +static int fsl_xcvr_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + const struct of_device_id *of_id;
> + struct fsl_xcvr *xcvr;
> + struct resource *ram_res, *regs_res, *rx_res, *tx_res;
> + void __iomem *regs;
> + int ret, irq;
> +
> + of_id = of_match_device(fsl_xcvr_dt_ids, dev);
> + if (!of_id)
> + return -EINVAL;
> +
> + xcvr = devm_kzalloc(dev, sizeof(*xcvr), GFP_KERNEL);
> + if (!xcvr)
> + return -ENOMEM;
> +
> + xcvr->pdev = pdev;
> + xcvr->ipg_clk = devm_clk_get(dev, "ipg");
> + if (IS_ERR(xcvr->ipg_clk)) {
> + dev_err(dev, "failed to get ipg clock\n");
> + return PTR_ERR(xcvr->ipg_clk);
> + }
> +
> + xcvr->phy_clk = devm_clk_get(dev, "phy");
> + if (IS_ERR(xcvr->phy_clk)) {
> + dev_err(dev, "failed to get phy clock\n");
> + return PTR_ERR(xcvr->phy_clk);
> + }
> +
> + xcvr->spba_clk = devm_clk_get(dev, "spba");
> + if (IS_ERR(xcvr->spba_clk)) {
> + dev_err(dev, "failed to get spba clock\n");
> + return PTR_ERR(xcvr->spba_clk);
> + }
> +
> + xcvr->pll_ipg_clk = devm_clk_get(dev, "pll_ipg");
> + if (IS_ERR(xcvr->pll_ipg_clk)) {
> + dev_err(dev, "failed to get pll_ipg clock\n");
> + return PTR_ERR(xcvr->pll_ipg_clk);
> + }
> +
> + ram_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ram");
> + xcvr->ram_addr = devm_ioremap_resource(dev, ram_res);
> + if (IS_ERR(xcvr->ram_addr))
> + return PTR_ERR(xcvr->ram_addr);
> +
> + regs_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> + regs = devm_ioremap_resource(dev, regs_res);
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
> +
> + xcvr->regmap = devm_regmap_init_mmio_clk(dev, NULL, regs,
> + &fsl_xcvr_regmap_cfg);
> + if (IS_ERR(xcvr->regmap)) {
> + dev_err(dev, "failed to init XCVR regmap: %ld\n",
> + PTR_ERR(xcvr->regmap));
> + return PTR_ERR(xcvr->regmap);
> + }
> +
> + xcvr->reset = of_reset_control_get(np, NULL);
Please use devm_reset_control_get_exclusive().
[...]
> +static __maybe_unused int fsl_xcvr_runtime_resume(struct device *dev)
> +{
> + struct fsl_xcvr *xcvr = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = clk_prepare_enable(xcvr->ipg_clk);
> + if (ret) {
> + dev_err(dev, "failed to start IPG clock.\n");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(xcvr->pll_ipg_clk);
> + if (ret) {
> + dev_err(dev, "failed to start PLL IPG clock.\n");
> + goto stop_ipg_clk;
> + }
> +
> + ret = clk_prepare_enable(xcvr->phy_clk);
> + if (ret) {
> + dev_err(dev, "failed to start PHY clock: %d\n", ret);
> + goto stop_pll_ipg_clk;
> + }
> +
> + ret = clk_prepare_enable(xcvr->spba_clk);
> + if (ret) {
> + dev_err(dev, "failed to start SPBA clock.\n");
> + goto stop_phy_clk;
> + }
> +
> + regcache_cache_only(xcvr->regmap, false);
> + regcache_mark_dirty(xcvr->regmap);
> + ret = regcache_sync(xcvr->regmap);
> +
> + if (ret) {
> + dev_err(dev, "failed to sync regcache.\n");
> + goto stop_spba_clk;
> + }
> +
> + reset_control_assert(xcvr->reset);
> + reset_control_deassert(xcvr->reset);
No delay required between the two?
regards
Philipp
^ permalink raw reply
* Re: [PATCH] i2c: cpm: Fix i2c_ram structure
From: Christophe Leroy @ 2020-09-22 12:38 UTC (permalink / raw)
To: nicolas.vincent, jochen; +Cc: linuxppc-dev, linux-i2c
In-Reply-To: <20200922090400.6282-1-nicolas.vincent@vossloh.com>
Le 22/09/2020 à 11:04, nico.vince@gmail.com a écrit :
> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> the i2c_ram structure is missing the sdmatmp field mentionned in
> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
> hardware would write past the allocated memory done through
> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
> field is only set during setup(), the first i2c transaction would work
> and the following would send data read from an arbitrary memory
> location.
>
> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
> ---
> drivers/i2c/busses/i2c-cpm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 1213e1932ccb..c5700addbf65 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -64,7 +64,8 @@ struct i2c_ram {
> uint txtmp; /* Internal */
> char res1[4]; /* Reserved */
> ushort rpbase; /* Relocation pointer */
> - char res2[2]; /* Reserved */
> + char res2[6]; /* Reserved */
> + uint sdmatmp; /* Internal */
On CPM1, I2C param RAM has size 0x30 (offset 0x1c80-0x1caf)
Your change overlaps the miscellaneous area that contains CP Microcode
Revision Number, ref MPC885 Reference Manual §18.7.3
> };
>
> #define I2COM_START 0x80
>
Christophe
^ permalink raw reply
* Re: [PATCH] i2c: cpm: Fix i2c_ram structure
From: Joakim Tjernlund @ 2020-09-22 11:50 UTC (permalink / raw)
To: nicolas.vincent@vossloh.com, jochen@scram.de,
christophe.leroy@c-s.fr
Cc: linuxppc-dev@lists.ozlabs.org, linux-i2c@vger.kernel.org
In-Reply-To: <20200922090400.6282-1-nicolas.vincent@vossloh.com>
On Tue, 2020-09-22 at 11:04 +0200, nico.vince@gmail.com wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> the i2c_ram structure is missing the sdmatmp field mentionned in
> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
> hardware would write past the allocated memory done through
> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
> field is only set during setup(), the first i2c transaction would work
> and the following would send data read from an arbitrary memory
> location.
>
> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
> ---
> drivers/i2c/busses/i2c-cpm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 1213e1932ccb..c5700addbf65 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -64,7 +64,8 @@ struct i2c_ram {
> uint txtmp; /* Internal */
> char res1[4]; /* Reserved */
> ushort rpbase; /* Relocation pointer */
> - char res2[2]; /* Reserved */
> + char res2[6]; /* Reserved */
> + uint sdmatmp; /* Internal */
> };
Not sure if this will fit on 8xx CPUs, Leroy ?
Jocke
^ permalink raw reply
* Re: [PATCH V2] powerpc/perf: Exclude pmc5/6 from the irrelevant PMU group constraints
From: Paul A. Clarke @ 2020-09-22 10:46 UTC (permalink / raw)
To: Athira Rajeev; +Cc: maddy, linuxppc-dev
In-Reply-To: <1600672204-1610-1-git-send-email-atrajeev@linux.vnet.ibm.com>
Just one nit in a comment below...
(and this is not worthy of tags like "reviewed-by" ;-)
On Mon, Sep 21, 2020 at 03:10:04AM -0400, Athira Rajeev wrote:
> PMU counter support functions enforces event constraints for group of
> events to check if all events in a group can be monitored. Incase of
> event codes using PMC5 and PMC6 ( 500fa and 600f4 respectively ),
> not all constraints are applicable, say the threshold or sample bits.
> But current code includes pmc5 and pmc6 in some group constraints (like
> IC_DC Qualifier bits) which is actually not applicable and hence results
> in those events not getting counted when scheduled along with group of
> other events. Patch fixes this by excluding PMC5/6 from constraints
> which are not relevant for it.
>
> Fixes: 7ffd948 ("powerpc/perf: factor out power8 pmu functions")
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index 964437a..12153da 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -288,6 +288,15 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)
>
> mask |= CNST_PMC_MASK(pmc);
> value |= CNST_PMC_VAL(pmc);
> +
> + /*
> + * PMC5 and PMC6 are used to count cycles and instructions
> + * and these doesnot support most of the constraint bits.
s/doesnot/do not/
> + * Add a check to exclude PMC5/6 from most of the constraints
> + * except for ebb/bhrb.
> + */
> + if (pmc >= 5)
> + goto ebb_bhrb;
PC
^ permalink raw reply
* [PATCH] i2c: cpm: Fix i2c_ram structure
From: nico.vince @ 2020-09-22 9:04 UTC (permalink / raw)
To: jochen; +Cc: Nicolas VINCENT, linuxppc-dev, linux-i2c
From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
the i2c_ram structure is missing the sdmatmp field mentionned in
datasheet for MPC8272 at paragraph 36.5. With this field missing, the
hardware would write past the allocated memory done through
cpm_muram_alloc for the i2c_ram structure and land in memory allocated
for the buffers descriptors corrupting the cbd_bufaddr field. Since this
field is only set during setup(), the first i2c transaction would work
and the following would send data read from an arbitrary memory
location.
Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
---
drivers/i2c/busses/i2c-cpm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 1213e1932ccb..c5700addbf65 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -64,7 +64,8 @@ struct i2c_ram {
uint txtmp; /* Internal */
char res1[4]; /* Reserved */
ushort rpbase; /* Relocation pointer */
- char res2[2]; /* Reserved */
+ char res2[6]; /* Reserved */
+ uint sdmatmp; /* Internal */
};
#define I2COM_START 0x80
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/9] kernel: add a PF_FORCE_COMPAT flag
From: Arnd Bergmann @ 2020-09-22 9:01 UTC (permalink / raw)
To: Pavel Begunkov
Cc: linux-aio, open list:MIPS, David Howells, Linux-MM, keyrings,
sparclinux, Christoph Hellwig, linux-arch, linux-s390,
Linux SCSI List, X86 ML, linux-block, Al Viro, Andy Lutomirski,
io-uring, linux-arm-kernel, Jens Axboe, Parisc List,
Network Development, LKML, LSM List, Linux FS Devel,
Andrew Morton, linuxppc-dev
In-Reply-To: <f25b4708-eba6-78d6-03f9-5bfb04e07627@gmail.com>
On Tue, Sep 22, 2020 at 9:59 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> On 22/09/2020 10:23, Arnd Bergmann wrote:
> > On Tue, Sep 22, 2020 at 8:32 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> >> On 22/09/2020 03:58, Andy Lutomirski wrote:
> >>> On Mon, Sep 21, 2020 at 5:24 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
> >>> I may be looking at a different kernel than you, but aren't you
> >>> preventing creating an io_uring regardless of whether SQPOLL is
> >>> requested?
> >>
> >> I diffed a not-saved file on a sleepy head, thanks for noticing.
> >> As you said, there should be an SQPOLL check.
> >>
> >> ...
> >> if (ctx->compat && (p->flags & IORING_SETUP_SQPOLL))
> >> goto err;
> >
> > Wouldn't that mean that now 32-bit containers behave differently
> > between compat and native execution?
> >
> > I think if you want to prevent 32-bit applications from using SQPOLL,
> > it needs to be done the same way on both to be consistent:
>
> The intention was to disable only compat not native 32-bit.
I'm not following why that would be considered a valid option,
as that clearly breaks existing users that update from a 32-bit
kernel to a 64-bit one.
Taking away the features from users that are still on 32-bit kernels
already seems questionable to me, but being inconsistent
about it seems much worse, in particular when the regression
is on the upgrade path.
> > Can we expect all existing and future user space to have a sane
> > fallback when IORING_SETUP_SQPOLL fails?
>
> SQPOLL has a few differences with non-SQPOLL modes, but it's easy
> to convert between them. Anyway, SQPOLL is a privileged special
> case that's here for performance/latency reasons, I don't think
> there will be any non-accidental users of it.
Ok, so the behavior of 32-bit tasks would be the same as running
the same application as unprivileged 64-bit tasks, with applications
already having to implement that fallback, right?
Arnd
^ permalink raw reply
* Re: [PATCH v3] powerpc: fix EDEADLOCK redefinition error in uapi/asm/errno.h
From: Tony Ambardar @ 2020-09-22 8:38 UTC (permalink / raw)
To: Sasha Levin; +Cc: Paul Mackerras, linuxppc-dev, Arnd Bergmann, Stable
In-Reply-To: <20200921125452.32E0E21D7A@mail.kernel.org>
On Mon, 21 Sep 2020 at 05:54, Sasha Levin <sashal@kernel.org> wrote:
>
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.8.10, v5.4.66, v4.19.146, v4.14.198, v4.9.236, v4.4.236.
>
> v5.8.10: Build OK!
> v5.4.66: Build OK!
> v4.19.146: Build OK!
> v4.14.198: Failed to apply! Possible dependencies:
> 7af7919f0f4b ("tools include s390: Grab a copy of arch/s390/include/uapi/asm/unistd.h")
> 95f28190aa01 ("tools include arch: Grab a copy of errno.h for arch's supported by perf")
> a3f22d505f56 ("s390/perf: add callback to perf to enable using AUX buffer")
> a81c42136604 ("perf s390: add regs_query_register_offset()")
> a9fc2db0a8ab ("s390/perf: define common DWARF register string table")
> f704ef44602f ("s390/perf: add support for perf_regs and libdw")
>
> v4.9.236: Failed to apply! Possible dependencies:
> 0c744ea4f77d ("Linux 4.10-rc2")
> 2bd6bf03f4c1 ("Linux 4.14-rc1")
> 2ea659a9ef48 ("Linux 4.12-rc1")
> 49def1853334 ("Linux 4.10-rc4")
> 566cf877a1fc ("Linux 4.10-rc6")
> 5771a8c08880 ("Linux v4.13-rc1")
> 7089db84e356 ("Linux 4.10-rc8")
> 7a308bb3016f ("Linux 4.10-rc5")
> 7af7919f0f4b ("tools include s390: Grab a copy of arch/s390/include/uapi/asm/unistd.h")
> 7ce7d89f4883 ("Linux 4.10-rc1")
> 95f28190aa01 ("tools include arch: Grab a copy of errno.h for arch's supported by perf")
> a121103c9228 ("Linux 4.10-rc3")
> a81c42136604 ("perf s390: add regs_query_register_offset()")
> a9fc2db0a8ab ("s390/perf: define common DWARF register string table")
> b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license")
> c1ae3cfa0e89 ("Linux 4.11-rc1")
> c470abd4fde4 ("Linux 4.10")
> d5adbfcd5f7b ("Linux 4.10-rc7")
>
> v4.4.236: Failed to apply! Possible dependencies:
> 0c4d40d58075 ("tools build: Add BPF feature check to test-all")
> 1925459b4d92 ("tools build: Fix feature Makefile issues with 'O='")
> 58683600dfe3 ("perf build: Use FEATURE-DUMP in bpf subproject")
> 76ee2ff34274 ("tools build feature: Move dwarf post unwind choice output into perf")
> 7af7919f0f4b ("tools include s390: Grab a copy of arch/s390/include/uapi/asm/unistd.h")
> 8ee4646038e4 ("perf build: Add libcrypto feature detection")
> 95f28190aa01 ("tools include arch: Grab a copy of errno.h for arch's supported by perf")
> 96b9e70b8e6c ("perf build: Introduce FEATURES_DUMP make variable")
> 9fd4186ac19a ("tools build: Allow subprojects select all feature checkers")
> abb26210a395 ("perf tools: Force fixdep compilation at the start of the build")
> aeafd623f866 ("perf tools: Move headers check into bash script")
> c053a1506fae ("perf build: Select all feature checkers for feature-dump")
> d4dfdf00d43e ("perf jvmti: Plug compilation into perf build")
> d58ac0bf8d1e ("perf build: Add clang and llvm compile and linking support")
> d8ad6a15cc3a ("tools lib bpf: Don't do a feature check when cleaning")
> e12b202f8fb9 ("perf jitdump: Build only on supported archs")
> e26e63be64a1 ("perf build: Add sdt feature detection")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
>
[cc: linux-ppc, Arnd, Paul]
The patch makes identical changes to
'arch/powerpc/include/uapi/asm/errno.h' and its copy
'tools/arch/powerpc/include/uapi/asm/errno.h' first created in kernel
v4.16. Since it's the patch
hunk for the latter file which is failing on backports to < v4.16, I
would think it OK to skip
that hunk where the latter file is missing. I'd prefer to let Michael
decide the best course as
he's still reviewing the patch.
Thanks,
Tony
> --
> Thanks
> Sasha
^ permalink raw reply
* [PATCH] cpufreq: powernv: Fix frame-size-overflow in powernv_cpufreq_reboot_notifier
From: Srikar Dronamraju @ 2020-09-22 8:02 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Srikar Dronamraju, Pratik Rajesh Sampat,
Daniel Axtens
The patch avoids allocating cpufreq_policy on stack hence fixing frame
size overflow in 'powernv_cpufreq_reboot_notifier'
./drivers/cpufreq/powernv-cpufreq.c: In function _powernv_cpufreq_reboot_notifier_:
./drivers/cpufreq/powernv-cpufreq.c:906:1: error: the frame size of 2064 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
}
^
cc1: all warnings being treated as errors
make[3]: *** [./scripts/Makefile.build:316: drivers/cpufreq/powernv-cpufreq.o] Error 1
make[2]: *** [./scripts/Makefile.build:556: drivers/cpufreq] Error 2
make[1]: *** [./Makefile:1072: drivers] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:157: sub-make] Error 2
Fixes: cf30af76 ("cpufreq: powernv: Set the cpus to nominal frequency during reboot/kexec")
Cc: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
drivers/cpufreq/powernv-cpufreq.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index a9af15e..e439b43 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -885,12 +885,15 @@ static int powernv_cpufreq_reboot_notifier(struct notifier_block *nb,
unsigned long action, void *unused)
{
int cpu;
- struct cpufreq_policy cpu_policy;
+ struct cpufreq_policy *cpu_policy;
rebooting = true;
for_each_online_cpu(cpu) {
- cpufreq_get_policy(&cpu_policy, cpu);
- powernv_cpufreq_target_index(&cpu_policy, get_nominal_index());
+ cpu_policy = cpufreq_cpu_get(cpu);
+ if (!cpu_policy)
+ continue;
+ powernv_cpufreq_target_index(cpu_policy, get_nominal_index());
+ cpufreq_cpu_put(cpu_policy);
}
return NOTIFY_DONE;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/9] kernel: add a PF_FORCE_COMPAT flag
From: Pavel Begunkov @ 2020-09-22 7:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-aio, open list:MIPS, David Howells, Linux-MM, keyrings,
sparclinux, Christoph Hellwig, linux-arch, linux-s390,
Linux SCSI List, X86 ML, linux-block, Al Viro, Andy Lutomirski,
io-uring, linux-arm-kernel, Jens Axboe, Parisc List,
Network Development, LKML, LSM List, Linux FS Devel,
Andrew Morton, linuxppc-dev
In-Reply-To: <CAK8P3a2-6JNS38EbZcLrk=cTT526oP=Rf0aoqWNSJ-k4XTYehQ@mail.gmail.com>
On 22/09/2020 10:23, Arnd Bergmann wrote:
> On Tue, Sep 22, 2020 at 8:32 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>> On 22/09/2020 03:58, Andy Lutomirski wrote:
>>> On Mon, Sep 21, 2020 at 5:24 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>> I may be looking at a different kernel than you, but aren't you
>>> preventing creating an io_uring regardless of whether SQPOLL is
>>> requested?
>>
>> I diffed a not-saved file on a sleepy head, thanks for noticing.
>> As you said, there should be an SQPOLL check.
>>
>> ...
>> if (ctx->compat && (p->flags & IORING_SETUP_SQPOLL))
>> goto err;
>
> Wouldn't that mean that now 32-bit containers behave differently
> between compat and native execution?
>
> I think if you want to prevent 32-bit applications from using SQPOLL,
> it needs to be done the same way on both to be consistent:
The intention was to disable only compat not native 32-bit.
>
> if ((!IS_ENABLED(CONFIG_64BIT) || ctx->compat) &&
> (p->flags & IORING_SETUP_SQPOLL))
> goto err;
>
> I don't really see how taking away SQPOLL from 32-bit tasks is
> any better than just preventing access to the known-broken files
> as Al suggested, or adding the hack to make it work as in
> Christoph's original patch.
That's why I'm hoping that Christoph's work and the discussion will
reach consensus, but the bug should be patched in the end. IMHO,
it's a good and easy enough fallback option (temporal?).
>
> Can we expect all existing and future user space to have a sane
> fallback when IORING_SETUP_SQPOLL fails?
SQPOLL has a few differences with non-SQPOLL modes, but it's easy
to convert between them. Anyway, SQPOLL is a privileged special
case that's here for performance/latency reasons, I don't think
there will be any non-accidental users of it.
--
Pavel Begunkov
^ permalink raw reply
* Re: [PATCH] powerpc/64: Make VDSO32 track COMPAT on 64-bit
From: Srikar Dronamraju @ 2020-09-22 7:55 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, msuchanek, srikar, Stephen Rothwell
In-Reply-To: <160034211494.3342081.16977957933269532766.b4-ty@ellerman.id.au>
* Michael Ellerman <patch-notifications@ellerman.id.au> [2020-09-17 21:28:46]:
> On Tue, 8 Sep 2020 22:58:50 +1000, Michael Ellerman wrote:
> > When we added the VDSO32 kconfig symbol, which controls building of
> > the 32-bit VDSO, we made it depend on CPU_BIG_ENDIAN (for 64-bit).
> >
> > That was because back then COMPAT was always enabled for 64-bit, so
> > depending on it would have left the 32-bit VDSO always enabled, which
> > we didn't want.
> >
> > [...]
>
> Applied to powerpc/next.
>
> [1/1] powerpc/64: Make VDSO32 track COMPAT on 64-bit
> https://git.kernel.org/powerpc/c/231b232df8f67e7d37af01259c21f2a131c3911e
>
> cheers
With this commit which is part of powerpc/next and with
/opt/at12.0/bin/gcc --version
gcc (GCC) 8.4.1 20191125 (Advance-Toolchain 12.0-3) [e25f27eea473]
throws up a compile error on a witherspoon/PowerNV with CONFIG_COMPAT.
CONFIG_COMPAT got carried from the distro config. (And looks like most
distros seem to be having this config)
cc1: error: _-m32_ not supported in this configuration
make[4]: *** [arch/powerpc/kernel/vdso32/sigtramp.o] Error 1
make[4]: *** Waiting for unfinished jobs....
cc1: error: _-m32_ not supported in this configuration
make[4]: *** [arch/powerpc/kernel/vdso32/gettimeofday.o] Error 1
make[3]: *** [arch/powerpc/kernel/vdso32] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [arch/powerpc/kernel] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [arch/powerpc] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [__sub-make] Error 2
I don't seem to be facing with other compilers like "gcc (Ubuntu
7.4.0-1ubuntu1~18.04.1) 7.4.0" and I was able to disable CONFIG_COMPAT and
proceed with the build.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH 1/9] kernel: add a PF_FORCE_COMPAT flag
From: Arnd Bergmann @ 2020-09-22 7:23 UTC (permalink / raw)
To: Pavel Begunkov
Cc: linux-aio, open list:MIPS, David Howells, Linux-MM, keyrings,
sparclinux, Christoph Hellwig, linux-arch, linux-s390,
Linux SCSI List, X86 ML, linux-block, Al Viro, Andy Lutomirski,
io-uring, linux-arm-kernel, Jens Axboe, Parisc List,
Network Development, LKML, LSM List, Linux FS Devel,
Andrew Morton, linuxppc-dev
In-Reply-To: <e0a1b4d1-ff47-18d1-d535-c62812cb3105@gmail.com>
On Tue, Sep 22, 2020 at 8:32 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> On 22/09/2020 03:58, Andy Lutomirski wrote:
> > On Mon, Sep 21, 2020 at 5:24 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
> > I may be looking at a different kernel than you, but aren't you
> > preventing creating an io_uring regardless of whether SQPOLL is
> > requested?
>
> I diffed a not-saved file on a sleepy head, thanks for noticing.
> As you said, there should be an SQPOLL check.
>
> ...
> if (ctx->compat && (p->flags & IORING_SETUP_SQPOLL))
> goto err;
Wouldn't that mean that now 32-bit containers behave differently
between compat and native execution?
I think if you want to prevent 32-bit applications from using SQPOLL,
it needs to be done the same way on both to be consistent:
if ((!IS_ENABLED(CONFIG_64BIT) || ctx->compat) &&
(p->flags & IORING_SETUP_SQPOLL))
goto err;
I don't really see how taking away SQPOLL from 32-bit tasks is
any better than just preventing access to the known-broken files
as Al suggested, or adding the hack to make it work as in
Christoph's original patch.
Can we expect all existing and future user space to have a sane
fallback when IORING_SETUP_SQPOLL fails?
Arnd
^ permalink raw reply
* Re: [PATCH V2] powerpc/perf: Exclude pmc5/6 from the irrelevant PMU group constraints
From: Madhavan Srinivasan @ 2020-09-22 6:53 UTC (permalink / raw)
To: Athira Rajeev, mpe; +Cc: maddy, linuxppc-dev
In-Reply-To: <1600672204-1610-1-git-send-email-atrajeev@linux.vnet.ibm.com>
On 9/21/20 12:40 PM, Athira Rajeev wrote:
> PMU counter support functions enforces event constraints for group of
> events to check if all events in a group can be monitored. Incase of
> event codes using PMC5 and PMC6 ( 500fa and 600f4 respectively ),
> not all constraints are applicable, say the threshold or sample bits.
> But current code includes pmc5 and pmc6 in some group constraints (like
> IC_DC Qualifier bits) which is actually not applicable and hence results
> in those events not getting counted when scheduled along with group of
> other events. Patch fixes this by excluding PMC5/6 from constraints
> which are not relevant for it.
Changes looks fine to me.
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
we need to CC this in Stable too.
> Fixes: 7ffd948 ("powerpc/perf: factor out power8 pmu functions")
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changes in v2:
> - Added a block comment in the fix path explaining
> why the change is needed.
>
> arch/powerpc/perf/isa207-common.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index 964437a..12153da 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -288,6 +288,15 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)
>
> mask |= CNST_PMC_MASK(pmc);
> value |= CNST_PMC_VAL(pmc);
> +
> + /*
> + * PMC5 and PMC6 are used to count cycles and instructions
> + * and these doesnot support most of the constraint bits.
> + * Add a check to exclude PMC5/6 from most of the constraints
> + * except for ebb/bhrb.
> + */
> + if (pmc >= 5)
> + goto ebb_bhrb;
> }
>
> if (pmc <= 4) {
> @@ -357,6 +366,7 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)
> }
> }
>
> +ebb_bhrb:
> if (!pmc && ebb)
> /* EBB events must specify the PMC */
> return -1;
^ permalink raw reply
* Re: [PATCH 1/9] kernel: add a PF_FORCE_COMPAT flag
From: Pavel Begunkov @ 2020-09-22 6:30 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-aio, open list:MIPS, David Howells, Linux-MM, keyrings,
sparclinux, Christoph Hellwig, linux-arch, linux-s390,
Linux SCSI List, X86 ML, Arnd Bergmann, linux-block, Al Viro,
io-uring, linux-arm-kernel, Jens Axboe, Parisc List,
Network Development, LKML, LSM List, Linux FS Devel,
Andrew Morton, linuxppc-dev
In-Reply-To: <CALCETrUEC81va8-fuUXG1uA5rbKxnKDYsDOXC70_HtKD4LAeAg@mail.gmail.com>
On 22/09/2020 03:58, Andy Lutomirski wrote:
> On Mon, Sep 21, 2020 at 5:24 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>>>>> Ah, so reading /dev/input/event* would suffer from the same issue,
>>>>>>> and that one would in fact be broken by your patch in the hypothetical
>>>>>>> case that someone tried to use io_uring to read /dev/input/event on x32...
>>>>>>>
>>>>>>> For reference, I checked the socket timestamp handling that has a
>>>>>>> number of corner cases with time32/time64 formats in compat mode,
>>>>>>> but none of those appear to be affected by the problem.
>>>>>>>
>>>>>>>> Aside from the potentially nasty use of per-task variables, one thing
>>>>>>>> I don't like about PF_FORCE_COMPAT is that it's one-way. If we're
>>>>>>>> going to have a generic mechanism for this, shouldn't we allow a full
>>>>>>>> override of the syscall arch instead of just allowing forcing compat
>>>>>>>> so that a compat syscall can do a non-compat operation?
>>>>>>>
>>>>>>> The only reason it's needed here is that the caller is in a kernel
>>>>>>> thread rather than a system call. Are there any possible scenarios
>>>>>>> where one would actually need the opposite?
>>>>>>>
>>>>>>
>>>>>> I can certainly imagine needing to force x32 mode from a kernel thread.
>>>>>>
>>>>>> As for the other direction: what exactly are the desired bitness/arch semantics of io_uring? Is the operation bitness chosen by the io_uring creation or by the io_uring_enter() bitness?
>>>>>
>>>>> It's rather the second one. Even though AFAIR it wasn't discussed
>>>>> specifically, that how it works now (_partially_).
>>>>
>>>> Double checked -- I'm wrong, that's the former one. Most of it is based
>>>> on a flag that was set an creation.
>>>>
>>>
>>> Could we get away with making io_uring_enter() return -EINVAL (or
>>> maybe -ENOTTY?) if you try to do it with bitness that doesn't match
>>> the io_uring? And disable SQPOLL in compat mode?
>>
>> Something like below. If PF_FORCE_COMPAT or any other solution
>> doesn't lend by the time, I'll take a look whether other io_uring's
>> syscalls need similar checks, etc.
>>
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 0458f02d4ca8..aab20785fa9a 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -8671,6 +8671,10 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
>> if (ctx->flags & IORING_SETUP_R_DISABLED)
>> goto out;
>>
>> + ret = -EINVAl;
>> + if (ctx->compat != in_compat_syscall())
>> + goto out;
>> +
>
> This seems entirely reasonable to me. Sharing an io_uring ring
> between programs with different ABIs seems a bit nutty.
>
>> /*
>> * For SQ polling, the thread will do all submissions and completions.
>> * Just return the requested submit count, and wake the thread if
>> @@ -9006,6 +9010,10 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
>> if (ret)
>> goto err;
>>
>> + ret = -EINVAL;
>> + if (ctx->compat)
>> + goto err;
>> +
>
> I may be looking at a different kernel than you, but aren't you
> preventing creating an io_uring regardless of whether SQPOLL is
> requested?
I diffed a not-saved file on a sleepy head, thanks for noticing.
As you said, there should be an SQPOLL check.
...
if (ctx->compat && (p->flags & IORING_SETUP_SQPOLL))
goto err;
--
Pavel Begunkov
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc/64: Set up a kernel stack for secondaries before cpu_restore()
From: Jordan Niethe @ 2020-09-22 6:10 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Oliver O'Halloran, linuxppc-dev, Nicholas Piggin
In-Reply-To: <a4bc673b-74e2-98ea-dac7-4e6d86d10d15@csgroup.eu>
On Tue, Sep 22, 2020 at 3:59 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 22/09/2020 à 07:53, Jordan Niethe a écrit :
> > Currently in generic_secondary_smp_init(), cur_cpu_spec->cpu_restore()
> > is called before a stack has been set up in r1. This was previously fine
> > as the cpu_restore() functions were implemented in assembly and did not
> > use a stack. However commit 5a61ef74f269 ("powerpc/64s: Support new
> > device tree binding for discovering CPU features") used
> > __restore_cpu_cpufeatures() as the cpu_restore() function for a
> > device-tree features based cputable entry. This is a C function and
> > hence uses a stack in r1.
> >
> > generic_secondary_smp_init() is entered on the secondary cpus via the
> > primary cpu using the OPAL call opal_start_cpu(). In OPAL, each hardware
> > thread has its own stack. The OPAL call is ran in the primary's hardware
> > thread. During the call, a job is scheduled on a secondary cpu that will
> > start executing at the address of generic_secondary_smp_init(). Hence
> > the value that will be left in r1 when the secondary cpu enters the
> > kernel is part of that secondary cpu's individual OPAL stack. This means
> > that __restore_cpu_cpufeatures() will write to that OPAL stack. This is
> > not horribly bad as each hardware thread has its own stack and the call
> > that enters the kernel from OPAL never returns, but it is still wrong
> > and should be corrected.
> >
> > Create the temp kernel stack before calling cpu_restore().
> >
> > As noted by mpe, for a kexec boot, the secondary CPUs are released from
> > the spin loop at address 0x60 by smp_release_cpus() and then jump to
> > generic_secondary_smp_init(). The call to smp_release_cpus() is in
> > setup_arch(), and it comes before the call to emergency_stack_init().
> > emergency_stack_init() allocates an emergency stack in the PACA for each
> > CPU. This address in the PACA is what is used to set up the temp kernel
> > stack in generic_secondary_smp_init(). Move releasing the secondary CPUs
> > to after the PACAs have been allocated an emergency stack, otherwise the
> > PACA stack pointer will contain garbage and hence the temp kernel stack
> > created from it will be broken.
> >
> > Fixes: 5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v2: Add more detail to the commit message
> > v3: Release secondary CPUs after the emergency stack is created
> > ---
> > arch/powerpc/kernel/head_64.S | 8 ++++----
> > arch/powerpc/kernel/setup-common.c | 6 ++++--
> > 2 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> > index 0e05a9a47a4b..4b7f4c6c2600 100644
> > --- a/arch/powerpc/kernel/head_64.S
> > +++ b/arch/powerpc/kernel/head_64.S
> > @@ -420,6 +420,10 @@ generic_secondary_common_init:
> > /* From now on, r24 is expected to be logical cpuid */
> > mr r24,r5
> >
> > + /* Create a temp kernel stack for use before relocation is on. */
> > + ld r1,PACAEMERGSP(r13)
> > + subi r1,r1,STACK_FRAME_OVERHEAD
> > +
> > /* See if we need to call a cpu state restore handler */
> > LOAD_REG_ADDR(r23, cur_cpu_spec)
> > ld r23,0(r23)
> > @@ -448,10 +452,6 @@ generic_secondary_common_init:
> > sync /* order paca.run and cur_cpu_spec */
> > isync /* In case code patching happened */
> >
> > - /* Create a temp kernel stack for use before relocation is on. */
> > - ld r1,PACAEMERGSP(r13)
> > - subi r1,r1,STACK_FRAME_OVERHEAD
> > -
> > b __secondary_start
> > #endif /* SMP */
> >
> > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> > index 808ec9fab605..fff714e36b37 100644
> > --- a/arch/powerpc/kernel/setup-common.c
> > +++ b/arch/powerpc/kernel/setup-common.c
> > @@ -919,8 +919,6 @@ void __init setup_arch(char **cmdline_p)
> >
> > /* On BookE, setup per-core TLB data structures. */
> > setup_tlb_core_data();
> > -
> > - smp_release_cpus();
> > #endif
> >
> > /* Print various info about the machine that has been gathered so far. */
> > @@ -944,6 +942,10 @@ void __init setup_arch(char **cmdline_p)
> > exc_lvl_early_init();
> > emergency_stack_init();
> >
> > +#ifdef CONFIG_SMP
> > + smp_release_cpus();
> > +#endif
>
> Are you sure you need that #ifdef ?
Thanks, you are right, should not be necessary.
>
> In asm/smp.h, we have:
>
> #if defined(CONFIG_PPC64) && (defined(CONFIG_SMP) ||
> defined(CONFIG_KEXEC_CORE))
> extern void smp_release_cpus(void);
> #else
> static inline void smp_release_cpus(void) { };
> #endif
>
>
> > +
> > initmem_init();
> >
> > early_memtest(min_low_pfn << PAGE_SHIFT, max_low_pfn << PAGE_SHIFT);
> >
>
> Christophe
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc/64: Set up a kernel stack for secondaries before cpu_restore()
From: Christophe Leroy @ 2020-09-22 5:59 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: oohall, npiggin
In-Reply-To: <20200922055307.10647-1-jniethe5@gmail.com>
Le 22/09/2020 à 07:53, Jordan Niethe a écrit :
> Currently in generic_secondary_smp_init(), cur_cpu_spec->cpu_restore()
> is called before a stack has been set up in r1. This was previously fine
> as the cpu_restore() functions were implemented in assembly and did not
> use a stack. However commit 5a61ef74f269 ("powerpc/64s: Support new
> device tree binding for discovering CPU features") used
> __restore_cpu_cpufeatures() as the cpu_restore() function for a
> device-tree features based cputable entry. This is a C function and
> hence uses a stack in r1.
>
> generic_secondary_smp_init() is entered on the secondary cpus via the
> primary cpu using the OPAL call opal_start_cpu(). In OPAL, each hardware
> thread has its own stack. The OPAL call is ran in the primary's hardware
> thread. During the call, a job is scheduled on a secondary cpu that will
> start executing at the address of generic_secondary_smp_init(). Hence
> the value that will be left in r1 when the secondary cpu enters the
> kernel is part of that secondary cpu's individual OPAL stack. This means
> that __restore_cpu_cpufeatures() will write to that OPAL stack. This is
> not horribly bad as each hardware thread has its own stack and the call
> that enters the kernel from OPAL never returns, but it is still wrong
> and should be corrected.
>
> Create the temp kernel stack before calling cpu_restore().
>
> As noted by mpe, for a kexec boot, the secondary CPUs are released from
> the spin loop at address 0x60 by smp_release_cpus() and then jump to
> generic_secondary_smp_init(). The call to smp_release_cpus() is in
> setup_arch(), and it comes before the call to emergency_stack_init().
> emergency_stack_init() allocates an emergency stack in the PACA for each
> CPU. This address in the PACA is what is used to set up the temp kernel
> stack in generic_secondary_smp_init(). Move releasing the secondary CPUs
> to after the PACAs have been allocated an emergency stack, otherwise the
> PACA stack pointer will contain garbage and hence the temp kernel stack
> created from it will be broken.
>
> Fixes: 5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v2: Add more detail to the commit message
> v3: Release secondary CPUs after the emergency stack is created
> ---
> arch/powerpc/kernel/head_64.S | 8 ++++----
> arch/powerpc/kernel/setup-common.c | 6 ++++--
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index 0e05a9a47a4b..4b7f4c6c2600 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -420,6 +420,10 @@ generic_secondary_common_init:
> /* From now on, r24 is expected to be logical cpuid */
> mr r24,r5
>
> + /* Create a temp kernel stack for use before relocation is on. */
> + ld r1,PACAEMERGSP(r13)
> + subi r1,r1,STACK_FRAME_OVERHEAD
> +
> /* See if we need to call a cpu state restore handler */
> LOAD_REG_ADDR(r23, cur_cpu_spec)
> ld r23,0(r23)
> @@ -448,10 +452,6 @@ generic_secondary_common_init:
> sync /* order paca.run and cur_cpu_spec */
> isync /* In case code patching happened */
>
> - /* Create a temp kernel stack for use before relocation is on. */
> - ld r1,PACAEMERGSP(r13)
> - subi r1,r1,STACK_FRAME_OVERHEAD
> -
> b __secondary_start
> #endif /* SMP */
>
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 808ec9fab605..fff714e36b37 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -919,8 +919,6 @@ void __init setup_arch(char **cmdline_p)
>
> /* On BookE, setup per-core TLB data structures. */
> setup_tlb_core_data();
> -
> - smp_release_cpus();
> #endif
>
> /* Print various info about the machine that has been gathered so far. */
> @@ -944,6 +942,10 @@ void __init setup_arch(char **cmdline_p)
> exc_lvl_early_init();
> emergency_stack_init();
>
> +#ifdef CONFIG_SMP
> + smp_release_cpus();
> +#endif
Are you sure you need that #ifdef ?
In asm/smp.h, we have:
#if defined(CONFIG_PPC64) && (defined(CONFIG_SMP) ||
defined(CONFIG_KEXEC_CORE))
extern void smp_release_cpus(void);
#else
static inline void smp_release_cpus(void) { };
#endif
> +
> initmem_init();
>
> early_memtest(min_low_pfn << PAGE_SHIFT, max_low_pfn << PAGE_SHIFT);
>
Christophe
^ permalink raw reply
* [PATCH v3 2/2] powerpc/64s: Convert some cpu_setup() and cpu_restore() functions to C
From: Jordan Niethe @ 2020-09-22 5:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: oohall, npiggin, Jordan Niethe
In-Reply-To: <20200922055307.10647-1-jniethe5@gmail.com>
The only thing keeping the cpu_setup() and cpu_restore() functions used
in the cputable entries for Power7, Power8, Power9 and Power10 in
assembly was cpu_restore() being called before there was a stack in
generic_secondary_smp_init(). Commit ("powerpc/64: Set up a kernel stack
for secondaries before cpu_restore()") means that it is now possible to
use C.
Rewrite the functions in C so they are a little bit easier to read. This
is not changing their functionality.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
arch/powerpc/include/asm/cpu_setup_power.h | 12 +
arch/powerpc/kernel/cpu_setup_power.S | 252 -------------------
arch/powerpc/kernel/cpu_setup_power.c | 269 +++++++++++++++++++++
arch/powerpc/kernel/cputable.c | 12 +-
4 files changed, 285 insertions(+), 260 deletions(-)
create mode 100644 arch/powerpc/include/asm/cpu_setup_power.h
delete mode 100644 arch/powerpc/kernel/cpu_setup_power.S
create mode 100644 arch/powerpc/kernel/cpu_setup_power.c
diff --git a/arch/powerpc/include/asm/cpu_setup_power.h b/arch/powerpc/include/asm/cpu_setup_power.h
new file mode 100644
index 000000000000..24be9131f803
--- /dev/null
+++ b/arch/powerpc/include/asm/cpu_setup_power.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2020 IBM Corporation
+ */
+void __setup_cpu_power7(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power7(void);
+void __setup_cpu_power8(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power8(void);
+void __setup_cpu_power9(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power9(void);
+void __setup_cpu_power10(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power10(void);
diff --git a/arch/powerpc/kernel/cpu_setup_power.S b/arch/powerpc/kernel/cpu_setup_power.S
deleted file mode 100644
index 704e8b9501ee..000000000000
--- a/arch/powerpc/kernel/cpu_setup_power.S
+++ /dev/null
@@ -1,252 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * This file contains low level CPU setup functions.
- * Copyright (C) 2003 Benjamin Herrenschmidt (benh@kernel.crashing.org)
- */
-
-#include <asm/processor.h>
-#include <asm/page.h>
-#include <asm/cputable.h>
-#include <asm/ppc_asm.h>
-#include <asm/asm-offsets.h>
-#include <asm/cache.h>
-#include <asm/book3s/64/mmu-hash.h>
-
-/* Entry: r3 = crap, r4 = ptr to cputable entry
- *
- * Note that we can be called twice for pseudo-PVRs
- */
-_GLOBAL(__setup_cpu_power7)
- mflr r11
- bl __init_hvmode_206
- mtlr r11
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- li r4,(LPCR_LPES1 >> LPCR_LPES_SH)
- bl __init_LPCR_ISA206
- mtlr r11
- blr
-
-_GLOBAL(__restore_cpu_power7)
- mflr r11
- mfmsr r3
- rldicl. r0,r3,4,63
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- li r4,(LPCR_LPES1 >> LPCR_LPES_SH)
- bl __init_LPCR_ISA206
- mtlr r11
- blr
-
-_GLOBAL(__setup_cpu_power8)
- mflr r11
- bl __init_FSCR
- bl __init_PMU
- bl __init_PMU_ISA207
- bl __init_hvmode_206
- mtlr r11
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- ori r3, r3, LPCR_PECEDH
- li r4,0 /* LPES = 0 */
- bl __init_LPCR_ISA206
- bl __init_HFSCR
- bl __init_PMU_HV
- bl __init_PMU_HV_ISA207
- mtlr r11
- blr
-
-_GLOBAL(__restore_cpu_power8)
- mflr r11
- bl __init_FSCR
- bl __init_PMU
- bl __init_PMU_ISA207
- mfmsr r3
- rldicl. r0,r3,4,63
- mtlr r11
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- ori r3, r3, LPCR_PECEDH
- li r4,0 /* LPES = 0 */
- bl __init_LPCR_ISA206
- bl __init_HFSCR
- bl __init_PMU_HV
- bl __init_PMU_HV_ISA207
- mtlr r11
- blr
-
-_GLOBAL(__setup_cpu_power10)
- mflr r11
- bl __init_FSCR_power10
- bl __init_PMU
- bl __init_PMU_ISA31
- b 1f
-
-_GLOBAL(__setup_cpu_power9)
- mflr r11
- bl __init_FSCR_power9
- bl __init_PMU
-1: bl __init_hvmode_206
- mtlr r11
- beqlr
- li r0,0
- mtspr SPRN_PSSCR,r0
- mtspr SPRN_LPID,r0
- mtspr SPRN_PID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- LOAD_REG_IMMEDIATE(r4, LPCR_PECEDH | LPCR_PECE_HVEE | LPCR_HVICE | LPCR_HEIC)
- or r3, r3, r4
- LOAD_REG_IMMEDIATE(r4, LPCR_UPRT | LPCR_HR)
- andc r3, r3, r4
- li r4,0 /* LPES = 0 */
- bl __init_LPCR_ISA300
- bl __init_HFSCR
- bl __init_PMU_HV
- mtlr r11
- blr
-
-_GLOBAL(__restore_cpu_power10)
- mflr r11
- bl __init_FSCR_power10
- bl __init_PMU
- bl __init_PMU_ISA31
- b 1f
-
-_GLOBAL(__restore_cpu_power9)
- mflr r11
- bl __init_FSCR_power9
- bl __init_PMU
-1: mfmsr r3
- rldicl. r0,r3,4,63
- mtlr r11
- beqlr
- li r0,0
- mtspr SPRN_PSSCR,r0
- mtspr SPRN_LPID,r0
- mtspr SPRN_PID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- LOAD_REG_IMMEDIATE(r4, LPCR_PECEDH | LPCR_PECE_HVEE | LPCR_HVICE | LPCR_HEIC)
- or r3, r3, r4
- LOAD_REG_IMMEDIATE(r4, LPCR_UPRT | LPCR_HR)
- andc r3, r3, r4
- li r4,0 /* LPES = 0 */
- bl __init_LPCR_ISA300
- bl __init_HFSCR
- bl __init_PMU_HV
- mtlr r11
- blr
-
-__init_hvmode_206:
- /* Disable CPU_FTR_HVMODE and exit if MSR:HV is not set */
- mfmsr r3
- rldicl. r0,r3,4,63
- bnelr
- ld r5,CPU_SPEC_FEATURES(r4)
- LOAD_REG_IMMEDIATE(r6,CPU_FTR_HVMODE | CPU_FTR_P9_TM_HV_ASSIST)
- andc r5,r5,r6
- std r5,CPU_SPEC_FEATURES(r4)
- blr
-
-__init_LPCR_ISA206:
- /* Setup a sane LPCR:
- * Called with initial LPCR in R3 and desired LPES 2-bit value in R4
- *
- * LPES = 0b01 (HSRR0/1 used for 0x500)
- * PECE = 0b111
- * DPFD = 4
- * HDICE = 0
- * VC = 0b100 (VPM0=1, VPM1=0, ISL=0)
- * VRMASD = 0b10000 (L=1, LP=00)
- *
- * Other bits untouched for now
- */
- li r5,0x10
- rldimi r3,r5, LPCR_VRMASD_SH, 64-LPCR_VRMASD_SH-5
-
- /* POWER9 has no VRMASD */
-__init_LPCR_ISA300:
- rldimi r3,r4, LPCR_LPES_SH, 64-LPCR_LPES_SH-2
- ori r3,r3,(LPCR_PECE0|LPCR_PECE1|LPCR_PECE2)
- li r5,4
- rldimi r3,r5, LPCR_DPFD_SH, 64-LPCR_DPFD_SH-3
- clrrdi r3,r3,1 /* clear HDICE */
- li r5,4
- rldimi r3,r5, LPCR_VC_SH, 0
- mtspr SPRN_LPCR,r3
- isync
- blr
-
-__init_FSCR_power10:
- mfspr r3, SPRN_FSCR
- ori r3, r3, FSCR_PREFIX
- mtspr SPRN_FSCR, r3
- // fall through
-
-__init_FSCR_power9:
- mfspr r3, SPRN_FSCR
- ori r3, r3, FSCR_SCV
- mtspr SPRN_FSCR, r3
- // fall through
-
-__init_FSCR:
- mfspr r3,SPRN_FSCR
- ori r3,r3,FSCR_TAR|FSCR_EBB
- mtspr SPRN_FSCR,r3
- blr
-
-__init_HFSCR:
- mfspr r3,SPRN_HFSCR
- ori r3,r3,HFSCR_TAR|HFSCR_TM|HFSCR_BHRB|HFSCR_PM|\
- HFSCR_DSCR|HFSCR_VECVSX|HFSCR_FP|HFSCR_EBB|HFSCR_MSGP
- mtspr SPRN_HFSCR,r3
- blr
-
-__init_PMU_HV:
- li r5,0
- mtspr SPRN_MMCRC,r5
- blr
-
-__init_PMU_HV_ISA207:
- li r5,0
- mtspr SPRN_MMCRH,r5
- blr
-
-__init_PMU:
- li r5,0
- mtspr SPRN_MMCRA,r5
- mtspr SPRN_MMCR0,r5
- mtspr SPRN_MMCR1,r5
- mtspr SPRN_MMCR2,r5
- blr
-
-__init_PMU_ISA207:
- li r5,0
- mtspr SPRN_MMCRS,r5
- blr
-
-__init_PMU_ISA31:
- li r5,0
- mtspr SPRN_MMCR3,r5
- LOAD_REG_IMMEDIATE(r5, MMCRA_BHRB_DISABLE)
- mtspr SPRN_MMCRA,r5
- blr
diff --git a/arch/powerpc/kernel/cpu_setup_power.c b/arch/powerpc/kernel/cpu_setup_power.c
new file mode 100644
index 000000000000..cf5201b0579d
--- /dev/null
+++ b/arch/powerpc/kernel/cpu_setup_power.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 IBM Corporation
+ * This file contains low level CPU setup functions.
+ * Originally written in assembly by
+ * Benjamin Herrenschmidt (benh@kernel.crashing.org)
+ */
+#include <asm/reg.h>
+#include <asm/synch.h>
+#include <linux/bitops.h>
+#include <asm/cputable.h>
+#include <asm/cpu_setup_power.h>
+
+/* Disable CPU_FTR_HVMODE and return false if MSR:HV is not set */
+static bool init_hvmode_206(struct cpu_spec *t)
+{
+ u64 msr;
+
+ msr = mfmsr();
+ if (msr & MSR_HV)
+ return true;
+
+ t->cpu_features &= ~(CPU_FTR_HVMODE | CPU_FTR_P9_TM_HV_ASSIST);
+ return false;
+}
+
+static void init_LPCR_ISA300(u64 lpcr, u64 lpes)
+{
+ /* POWER9 has no VRMASD */
+ lpcr |= (lpes << LPCR_LPES_SH) & LPCR_LPES;
+ lpcr |= LPCR_PECE0|LPCR_PECE1|LPCR_PECE2;
+ lpcr |= (4ull << LPCR_DPFD_SH) & LPCR_DPFD;
+ lpcr &= ~LPCR_HDICE; /* clear HDICE */
+ lpcr |= (4ull << LPCR_VC_SH);
+ mtspr(SPRN_LPCR, lpcr);
+ isync();
+}
+
+/*
+ * Setup a sane LPCR:
+ * Called with initial LPCR and desired LPES 2-bit value
+ *
+ * LPES = 0b01 (HSRR0/1 used for 0x500)
+ * PECE = 0b111
+ * DPFD = 4
+ * HDICE = 0
+ * VC = 0b100 (VPM0=1, VPM1=0, ISL=0)
+ * VRMASD = 0b10000 (L=1, LP=00)
+ *
+ * Other bits untouched for now
+ */
+static void init_LPCR_ISA206(u64 lpcr, u64 lpes)
+{
+ lpcr |= (0x10ull << LPCR_VRMASD_SH) & LPCR_VRMASD;
+ init_LPCR_ISA300(lpcr, lpes);
+}
+
+static void init_FSCR(void)
+{
+ u64 fscr;
+
+ fscr = mfspr(SPRN_FSCR);
+ fscr |= FSCR_TAR|FSCR_EBB;
+ mtspr(SPRN_FSCR, fscr);
+}
+
+static void init_FSCR_power9(void)
+{
+ u64 fscr;
+
+ fscr = mfspr(SPRN_FSCR);
+ fscr |= FSCR_SCV;
+ mtspr(SPRN_FSCR, fscr);
+ init_FSCR();
+}
+
+static void init_FSCR_power10(void)
+{
+ u64 fscr;
+
+ fscr = mfspr(SPRN_FSCR);
+ fscr |= FSCR_PREFIX;
+ mtspr(SPRN_FSCR, fscr);
+ init_FSCR_power9();
+}
+
+static void init_HFSCR(void)
+{
+ u64 hfscr;
+
+ hfscr = mfspr(SPRN_HFSCR);
+ hfscr |= HFSCR_TAR|HFSCR_TM|HFSCR_BHRB|HFSCR_PM|HFSCR_DSCR|\
+ HFSCR_VECVSX|HFSCR_FP|HFSCR_EBB|HFSCR_MSGP;
+ mtspr(SPRN_HFSCR, hfscr);
+}
+
+static void init_PMU_HV(void)
+{
+ mtspr(SPRN_MMCRC, 0);
+}
+
+static void init_PMU_HV_ISA207(void)
+{
+ mtspr(SPRN_MMCRH, 0);
+}
+
+static void init_PMU(void)
+{
+ mtspr(SPRN_MMCRA, 0);
+ mtspr(SPRN_MMCR0, 0);
+ mtspr(SPRN_MMCR1, 0);
+ mtspr(SPRN_MMCR2, 0);
+}
+
+static void init_PMU_ISA207(void)
+{
+ mtspr(SPRN_MMCRS, 0);
+}
+
+static void init_PMU_ISA31(void)
+{
+ mtspr(SPRN_MMCR3, 0);
+ mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
+}
+
+/*
+ * Note that we can be called twice of pseudo-PVRs.
+ * The parameter offset is not used.
+ */
+
+void __setup_cpu_power7(unsigned long offset, struct cpu_spec *t)
+{
+ if (!init_hvmode_206(t))
+ return;
+
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA206(mfspr(SPRN_LPCR), LPCR_LPES1 >> LPCR_LPES_SH);
+}
+
+void __restore_cpu_power7(void)
+{
+ u64 msr;
+
+ msr = mfmsr();
+ if (!(msr & MSR_HV))
+ return;
+
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA206(mfspr(SPRN_LPCR), LPCR_LPES1 >> LPCR_LPES_SH);
+}
+
+void __setup_cpu_power8(unsigned long offset, struct cpu_spec *t)
+{
+ init_FSCR();
+ init_PMU();
+ init_PMU_ISA207();
+
+ if (!init_hvmode_206(t))
+ return;
+
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA206(mfspr(SPRN_LPCR) | LPCR_PECEDH, 0); /* LPES = 0 */
+ init_HFSCR();
+ init_PMU_HV();
+ init_PMU_HV_ISA207();
+}
+
+void __restore_cpu_power8(void)
+{
+ u64 msr;
+
+ init_FSCR();
+ init_PMU();
+ init_PMU_ISA207();
+
+ msr = mfmsr();
+ if (!(msr & MSR_HV))
+ return;
+
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA206(mfspr(SPRN_LPCR) | LPCR_PECEDH, 0); /* LPES = 0 */
+ init_HFSCR();
+ init_PMU_HV();
+ init_PMU_HV_ISA207();
+}
+
+void __setup_cpu_power9(unsigned long offset, struct cpu_spec *t)
+{
+ init_FSCR_power9();
+ init_PMU();
+
+ if (!init_hvmode_206(t))
+ return;
+
+ mtspr(SPRN_PSSCR, 0);
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
+ LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
+ init_HFSCR();
+ init_PMU_HV();
+}
+
+void __restore_cpu_power9(void)
+{
+ u64 msr;
+
+ init_FSCR_power9();
+ init_PMU();
+
+ msr = mfmsr();
+ if (!(msr & MSR_HV))
+ return;
+
+ mtspr(SPRN_PSSCR, 0);
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
+ LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
+ init_HFSCR();
+ init_PMU_HV();
+}
+
+void __setup_cpu_power10(unsigned long offset, struct cpu_spec *t)
+{
+ init_FSCR_power10();
+ init_PMU();
+ init_PMU_ISA31();
+
+ if (!init_hvmode_206(t))
+ return;
+
+ mtspr(SPRN_PSSCR, 0);
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
+ LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
+ init_HFSCR();
+ init_PMU_HV();
+}
+
+void __restore_cpu_power10(void)
+{
+ u64 msr;
+
+ init_FSCR_power10();
+ init_PMU();
+ init_PMU_ISA31();
+
+ msr = mfmsr();
+ if (!(msr & MSR_HV))
+ return;
+
+ mtspr(SPRN_PSSCR, 0);
+ mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_PID, 0);
+ mtspr(SPRN_PCR, PCR_MASK);
+ init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
+ LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
+ init_HFSCR();
+ init_PMU_HV();
+}
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 2aa89c6b2896..26a56c9d6650 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -59,19 +59,15 @@ extern void __setup_cpu_7410(unsigned long offset, struct cpu_spec* spec);
extern void __setup_cpu_745x(unsigned long offset, struct cpu_spec* spec);
#endif /* CONFIG_PPC32 */
#ifdef CONFIG_PPC64
+#include <asm/cpu_setup_power.h>
extern void __setup_cpu_ppc970(unsigned long offset, struct cpu_spec* spec);
extern void __setup_cpu_ppc970MP(unsigned long offset, struct cpu_spec* spec);
extern void __setup_cpu_pa6t(unsigned long offset, struct cpu_spec* spec);
extern void __restore_cpu_pa6t(void);
extern void __restore_cpu_ppc970(void);
-extern void __setup_cpu_power7(unsigned long offset, struct cpu_spec* spec);
-extern void __restore_cpu_power7(void);
-extern void __setup_cpu_power8(unsigned long offset, struct cpu_spec* spec);
-extern void __restore_cpu_power8(void);
-extern void __setup_cpu_power9(unsigned long offset, struct cpu_spec* spec);
-extern void __restore_cpu_power9(void);
-extern void __setup_cpu_power10(unsigned long offset, struct cpu_spec* spec);
-extern void __restore_cpu_power10(void);
+extern long __machine_check_early_realmode_p7(struct pt_regs *regs);
+extern long __machine_check_early_realmode_p8(struct pt_regs *regs);
+extern long __machine_check_early_realmode_p9(struct pt_regs *regs);
#endif /* CONFIG_PPC64 */
#if defined(CONFIG_E500)
extern void __setup_cpu_e5500(unsigned long offset, struct cpu_spec* spec);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 1/2] powerpc/64: Set up a kernel stack for secondaries before cpu_restore()
From: Jordan Niethe @ 2020-09-22 5:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: oohall, npiggin, Jordan Niethe
Currently in generic_secondary_smp_init(), cur_cpu_spec->cpu_restore()
is called before a stack has been set up in r1. This was previously fine
as the cpu_restore() functions were implemented in assembly and did not
use a stack. However commit 5a61ef74f269 ("powerpc/64s: Support new
device tree binding for discovering CPU features") used
__restore_cpu_cpufeatures() as the cpu_restore() function for a
device-tree features based cputable entry. This is a C function and
hence uses a stack in r1.
generic_secondary_smp_init() is entered on the secondary cpus via the
primary cpu using the OPAL call opal_start_cpu(). In OPAL, each hardware
thread has its own stack. The OPAL call is ran in the primary's hardware
thread. During the call, a job is scheduled on a secondary cpu that will
start executing at the address of generic_secondary_smp_init(). Hence
the value that will be left in r1 when the secondary cpu enters the
kernel is part of that secondary cpu's individual OPAL stack. This means
that __restore_cpu_cpufeatures() will write to that OPAL stack. This is
not horribly bad as each hardware thread has its own stack and the call
that enters the kernel from OPAL never returns, but it is still wrong
and should be corrected.
Create the temp kernel stack before calling cpu_restore().
As noted by mpe, for a kexec boot, the secondary CPUs are released from
the spin loop at address 0x60 by smp_release_cpus() and then jump to
generic_secondary_smp_init(). The call to smp_release_cpus() is in
setup_arch(), and it comes before the call to emergency_stack_init().
emergency_stack_init() allocates an emergency stack in the PACA for each
CPU. This address in the PACA is what is used to set up the temp kernel
stack in generic_secondary_smp_init(). Move releasing the secondary CPUs
to after the PACAs have been allocated an emergency stack, otherwise the
PACA stack pointer will contain garbage and hence the temp kernel stack
created from it will be broken.
Fixes: 5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Add more detail to the commit message
v3: Release secondary CPUs after the emergency stack is created
---
arch/powerpc/kernel/head_64.S | 8 ++++----
arch/powerpc/kernel/setup-common.c | 6 ++++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 0e05a9a47a4b..4b7f4c6c2600 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -420,6 +420,10 @@ generic_secondary_common_init:
/* From now on, r24 is expected to be logical cpuid */
mr r24,r5
+ /* Create a temp kernel stack for use before relocation is on. */
+ ld r1,PACAEMERGSP(r13)
+ subi r1,r1,STACK_FRAME_OVERHEAD
+
/* See if we need to call a cpu state restore handler */
LOAD_REG_ADDR(r23, cur_cpu_spec)
ld r23,0(r23)
@@ -448,10 +452,6 @@ generic_secondary_common_init:
sync /* order paca.run and cur_cpu_spec */
isync /* In case code patching happened */
- /* Create a temp kernel stack for use before relocation is on. */
- ld r1,PACAEMERGSP(r13)
- subi r1,r1,STACK_FRAME_OVERHEAD
-
b __secondary_start
#endif /* SMP */
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 808ec9fab605..fff714e36b37 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -919,8 +919,6 @@ void __init setup_arch(char **cmdline_p)
/* On BookE, setup per-core TLB data structures. */
setup_tlb_core_data();
-
- smp_release_cpus();
#endif
/* Print various info about the machine that has been gathered so far. */
@@ -944,6 +942,10 @@ void __init setup_arch(char **cmdline_p)
exc_lvl_early_init();
emergency_stack_init();
+#ifdef CONFIG_SMP
+ smp_release_cpus();
+#endif
+
initmem_init();
early_memtest(min_low_pfn << PAGE_SHIFT, max_low_pfn << PAGE_SHIFT);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] ibmvfc: Avoid link down on FS9100 canister reboot
From: Martin K. Petersen @ 2020-09-22 3:56 UTC (permalink / raw)
To: linux-scsi, Brian King; +Cc: tyreld, linuxppc-dev, Martin K . Petersen
In-Reply-To: <1599859706-8505-1-git-send-email-brking@linux.vnet.ibm.com>
On Fri, 11 Sep 2020 16:28:26 -0500, Brian King wrote:
> When a canister on a FS9100, or similar storage, running in NPIV mode,
> is rebooted, its WWPNs will fail over to another canister. When this
> occurs, we see a WWPN going away from the fabric at one N-Port ID,
> and, a short time later, the same WWPN appears at a different N-Port ID.
> When the canister is fully operational again, the WWPNs fail back to
> the original canister. If there is any I/O outstanding to the target
> when this occurs, it will result in the implicit logout the ibmvfc driver
> issues before removing the rport to fail. When the WWPN then shows up at a
> different N-Port ID, and we issue a PLOGI to it, the VIOS will
> see that it still has a login for this WWPN at the old N-Port ID,
> which results in the VIOS simulating a link down / link up sequence
> to the client, in order to get the VIOS and client LPAR in sync.
>
> [...]
Applied to 5.10/scsi-queue, thanks!
[1/1] scsi: ibmvfc: Avoid link down on FS9100 canister reboot
https://git.kernel.org/mkp/scsi/c/4b29cb6197d9
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/64: Set up a kernel stack for secondaries before cpu_restore()
From: Jordan Niethe @ 2020-09-22 3:52 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Oliver O'Halloran, linuxppc-dev, Nicholas Piggin
In-Reply-To: <877dsro8iy.fsf@mpe.ellerman.id.au>
On Fri, Sep 18, 2020 at 5:21 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Hi Jordan,
>
> Jordan Niethe <jniethe5@gmail.com> writes:
> > Currently in generic_secondary_smp_init(), cur_cpu_spec->cpu_restore()
> > is called before a stack has been set up in r1. This was previously fine
> > as the cpu_restore() functions were implemented in assembly and did not
> > use a stack. However commit 5a61ef74f269 ("powerpc/64s: Support new
> > device tree binding for discovering CPU features") used
> > __restore_cpu_cpufeatures() as the cpu_restore() function for a
> > device-tree features based cputable entry. This is a C function and
> > hence uses a stack in r1.
> >
> > generic_secondary_smp_init() is entered on the secondary cpus via the
> > primary cpu using the OPAL call opal_start_cpu(). In OPAL, each hardware
> > thread has its own stack. The OPAL call is ran in the primary's hardware
> > thread. During the call, a job is scheduled on a secondary cpu that will
> > start executing at the address of generic_secondary_smp_init(). Hence
> > the value that will be left in r1 when the secondary cpu enters the
> > kernel is part of that secondary cpu's individual OPAL stack. This means
> > that __restore_cpu_cpufeatures() will write to that OPAL stack. This is
> > not horribly bad as each hardware thread has its own stack and the call
> > that enters the kernel from OPAL never returns, but it is still wrong
> > and should be corrected.
> >
> > Create the temp kernel stack before calling cpu_restore().
> >
> > Fixes: 5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v2: Add more detail to the commit message
> > ---
> > arch/powerpc/kernel/head_64.S | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
>
> Unfortunately this breaks booting via kexec.
>
> In that case the secondaries come in to 0x60 and spin until they're
> released by smp_release_cpus(), which is before emergency_stack_init()
> has run. That means they pick up a bad r1 value and crash/get stuck.
>
> I'm not sure what the best solution is.
Would it be simplest to just call smp_release_cpus() after setting up the stack?
>
> I've thought in the past that it would be nicer if the CPU setup didn't
> run until the secondary is told to start (via PACAPROCSTART), ie. more
> the CPU setup call below there.
>
> But that opens the possibility that we run threads with different
> settings of some SPRs until SMP bringup, and if the user has said not to
> start secondaries then possibly for ever. And I haven't though hard
> enough about whether that's actually problematic (running with different
> SPR values).
>
> cheers
>
>
> > diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> > index 0e05a9a47a4b..4b7f4c6c2600 100644
> > --- a/arch/powerpc/kernel/head_64.S
> > +++ b/arch/powerpc/kernel/head_64.S
> > @@ -420,6 +420,10 @@ generic_secondary_common_init:
> > /* From now on, r24 is expected to be logical cpuid */
> > mr r24,r5
> >
> > + /* Create a temp kernel stack for use before relocation is on. */
> > + ld r1,PACAEMERGSP(r13)
> > + subi r1,r1,STACK_FRAME_OVERHEAD
> > +
> > /* See if we need to call a cpu state restore handler */
> > LOAD_REG_ADDR(r23, cur_cpu_spec)
> > ld r23,0(r23)
> > @@ -448,10 +452,6 @@ generic_secondary_common_init:
> > sync /* order paca.run and cur_cpu_spec */
> > isync /* In case code patching happened */
> >
> > - /* Create a temp kernel stack for use before relocation is on. */
> > - ld r1,PACAEMERGSP(r13)
> > - subi r1,r1,STACK_FRAME_OVERHEAD
> > -
> > b __secondary_start
> > #endif /* SMP */
> >
> > --
> > 2.17.1
^ 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