LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 2/3] ASoC: bindings: fsl-asoc-card: Support hp-det-gpio and mic-det-gpio
From: Shengjiu Wang @ 2020-07-15 14:09 UTC (permalink / raw)
  To: perex, tiwai, lgirdwood, broonie, alsa-devel, robh+dt, devicetree,
	timur, nicoleotsuka, Xiubo.Lee, festevam
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1594822179-1849-1-git-send-email-shengjiu.wang@nxp.com>

Add headphone and microphone detection GPIO support.
These properties are optional.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
index 133d7e14a4d0..8a6a3d0fda5e 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
@@ -69,6 +69,9 @@ Optional properties:
 			        coexisting in order to support the old bindings
 				of wm8962 and sgtl5000.
 
+  - hp-det-gpio		: The GPIO that detect headphones are plugged in
+  - mic-det-gpio	: The GPIO that detect microphones are plugged in
+
 Optional unless SSI is selected as a CPU DAI:
 
   - mux-int-port	: The internal port of the i.MX audio muxer (AUDMUX)
-- 
2.27.0


^ permalink raw reply related

* [PATCH v2 3/3] ASoC: fsl-asoc-card: Support Headphone and Microphone Jack detection
From: Shengjiu Wang @ 2020-07-15 14:09 UTC (permalink / raw)
  To: perex, tiwai, lgirdwood, broonie, alsa-devel, robh+dt, devicetree,
	timur, nicoleotsuka, Xiubo.Lee, festevam
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1594822179-1849-1-git-send-email-shengjiu.wang@nxp.com>

Use asoc_simple_init_jack function from simple card to implement
the Headphone and Microphone detection.
Register notifier to disable Speaker when Headphone is plugged in
and enable Speaker when Headphone is unplugged.
Register notifier to disable Digital Microphone when Analog Microphone
is plugged in and enable DMIC when Analog Microphone is unplugged.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/Kconfig         |  1 +
 sound/soc/fsl/fsl-asoc-card.c | 77 ++++++++++++++++++++++++++++++++++-
 2 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index ea7b4787a8af..1c4ca5ec8caf 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -315,6 +315,7 @@ config SND_SOC_FSL_ASOC_CARD
 	depends on OF && I2C
 	# enforce SND_SOC_FSL_ASOC_CARD=m if SND_AC97_CODEC=m:
 	depends on SND_AC97_CODEC || SND_AC97_CODEC=n
+	select SND_SIMPLE_CARD_UTILS
 	select SND_SOC_IMX_AUDMUX
 	select SND_SOC_IMX_PCM_DMA
 	select SND_SOC_FSL_ESAI
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index faac6ce9a82c..f0cde3ecb5b7 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -15,6 +15,8 @@
 #endif
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
+#include <sound/jack.h>
+#include <sound/simple_card_utils.h>
 
 #include "fsl_esai.h"
 #include "fsl_sai.h"
@@ -65,6 +67,8 @@ struct cpu_priv {
 /**
  * struct fsl_asoc_card_priv - Freescale Generic ASOC card private data
  * @dai_link: DAI link structure including normal one and DPCM link
+ * @hp_jack: Headphone Jack structure
+ * @mic_jack: Microphone Jack structure
  * @pdev: platform device pointer
  * @codec_priv: CODEC private data
  * @cpu_priv: CPU private data
@@ -79,6 +83,8 @@ struct cpu_priv {
 
 struct fsl_asoc_card_priv {
 	struct snd_soc_dai_link dai_link[3];
+	struct asoc_simple_jack hp_jack;
+	struct asoc_simple_jack mic_jack;
 	struct platform_device *pdev;
 	struct codec_priv codec_priv;
 	struct cpu_priv cpu_priv;
@@ -445,6 +451,44 @@ static int fsl_asoc_card_audmux_init(struct device_node *np,
 	return 0;
 }
 
+static int hp_jack_event(struct notifier_block *nb, unsigned long event,
+			 void *data)
+{
+	struct snd_soc_jack *jack = (struct snd_soc_jack *)data;
+	struct snd_soc_dapm_context *dapm = &jack->card->dapm;
+
+	if (event & SND_JACK_HEADPHONE)
+		/* Disable speaker if headphone is plugged in */
+		snd_soc_dapm_disable_pin(dapm, "Ext Spk");
+	else
+		snd_soc_dapm_enable_pin(dapm, "Ext Spk");
+
+	return 0;
+}
+
+static struct notifier_block hp_jack_nb = {
+	.notifier_call = hp_jack_event,
+};
+
+static int mic_jack_event(struct notifier_block *nb, unsigned long event,
+			  void *data)
+{
+	struct snd_soc_jack *jack = (struct snd_soc_jack *)data;
+	struct snd_soc_dapm_context *dapm = &jack->card->dapm;
+
+	if (event & SND_JACK_MICROPHONE)
+		/* Disable dmic if microphone is plugged in */
+		snd_soc_dapm_disable_pin(dapm, "DMIC");
+	else
+		snd_soc_dapm_enable_pin(dapm, "DMIC");
+
+	return 0;
+}
+
+static struct notifier_block mic_jack_nb = {
+	.notifier_call = mic_jack_event,
+};
+
 static int fsl_asoc_card_late_probe(struct snd_soc_card *card)
 {
 	struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(card);
@@ -745,8 +789,37 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	snd_soc_card_set_drvdata(&priv->card, priv);
 
 	ret = devm_snd_soc_register_card(&pdev->dev, &priv->card);
-	if (ret && ret != -EPROBE_DEFER)
-		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		goto asrc_fail;
+	}
+
+	/*
+	 * Properties "hp-det-gpio" and "mic-det-gpio" are optional, and
+	 * asoc_simple_init_jack uses these properties for creating
+	 * Headphone Jack and Microphone Jack.
+	 *
+	 * The notifier is initialized in snd_soc_card_jack_new(), then
+	 * snd_soc_jack_notifier_register can be called.
+	 */
+	if (of_property_read_bool(np, "hp-det-gpio")) {
+		ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack,
+					    1, NULL, "Headphone Jack");
+		if (ret)
+			goto asrc_fail;
+
+		snd_soc_jack_notifier_register(&priv->hp_jack.jack, &hp_jack_nb);
+	}
+
+	if (of_property_read_bool(np, "mic-det-gpio")) {
+		ret = asoc_simple_init_jack(&priv->card, &priv->mic_jack,
+					    0, NULL, "Mic Jack");
+		if (ret)
+			goto asrc_fail;
+
+		snd_soc_jack_notifier_register(&priv->mic_jack.jack, &mic_jack_nb);
+	}
 
 asrc_fail:
 	of_node_put(asrc_np);
-- 
2.27.0


^ permalink raw reply related

* RE: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: David Laight @ 2020-07-15 14:24 UTC (permalink / raw)
  To: 'Arnd Bergmann', Bjorn Helgaas
  Cc: Keith Busch, Paul Mackerras, sparclinux, Toan Le, Kjetil Oftedal,
	Greg Ungerer, Marek Vasut, Rob Herring, Lorenzo Pieralisi,
	Sagi Grimberg, Russell King, Ley Foon Tan, Christoph Hellwig,
	Geert Uytterhoeven, Kevin Hilman, linux-pci, Jakub Kicinski,
	Matt Turner, linux-kernel-mentees@lists.linuxfoundation.org,
	Guenter Roeck, Ray Jui, Jens Axboe, Ivan Kokshaysky, Shuah Khan,
	bjorn@helgaas.com, Boris Ostrovsky, Richard Henderson,
	Juergen Gross, Bjorn Helgaas, Thomas Bogendoerfer, Scott Branden,
	Jingoo Han, Saheed O. Bolarinwa, linux-kernel@vger.kernel.org,
	Philipp Zabel, Greg Kroah-Hartman, Gustavo Pimentel, linuxppc-dev,
	David S. Miller, Heiner Kallweit
In-Reply-To: <CAK8P3a3OEOosC2VEHb3z7hTA=BjVp0nv9bNxBWzEnmgSDDTX3Q@mail.gmail.com>

From: Arnd Bergmann
> Sent: 15 July 2020 07:47
> On Wed, Jul 15, 2020 at 1:46 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
>  So something like:
> >
> >   void pci_read_config_word(struct pci_dev *dev, int where, u16 *val)
> >
> > and where we used to return anything non-zero, we just set *val = ~0
> > instead?  I think we do that already in most, maybe all, cases.
> 
> Right, this is what I had in mind. If we start by removing the handling
> of the return code in all files that clearly don't need it, looking at
> whatever remains will give a much better idea of what a good interface
> should be.

It would be best to get rid of that nasty 'u16 *' parameter.
Make the return int and return the read value or -1 on error.
(Or maybe 0xffff0000 on error??)

For a 32bit read (there must be one for the BARs) returning
a 64bit signed integer would work even for 32bit systems.

If code cares about the error, and it can be detected then
it can check. Otherwise the it all 'just works'.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH 1/1] ASoC: fsl: fsl-asoc-card: Trivial: Fix misspelling of 'exists'
From: Mark Brown @ 2020-07-15 14:26 UTC (permalink / raw)
  To: Lee Jones
  Cc: alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, lgirdwood,
	linux-kernel, Nicolin Chen, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200715094447.3170843-1-lee.jones@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

On Wed, Jul 15, 2020 at 10:44:47AM +0100, Lee Jones wrote:

>  /*
> - * This dapm route map exits for DPCM link only.
> + * This dapm route map exists for DPCM link only.
>   * The other routes shall go through Device Tree.

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* RE: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: David Laight @ 2020-07-15 14:38 UTC (permalink / raw)
  To: 'Oliver O'Halloran', Arnd Bergmann
  Cc: Greg Kroah-Hartman, linux-pci, bjorn@helgaas.com, Paul Mackerras,
	sparclinux, Toan Le, Christoph Hellwig, Marek Vasut, Rob Herring,
	Lorenzo Pieralisi, Sagi Grimberg, Kevin Hilman, Russell King,
	Ley Foon Tan, Greg Ungerer, Geert Uytterhoeven, Jakub Kicinski,
	Matt Turner, linux-kernel-mentees@lists.linuxfoundation.org,
	Guenter Roeck, Bjorn Helgaas, Ray Jui, linuxppc-dev, Jens Axboe,
	Ivan Kokshaysky, Shuah Khan, Keith Busch, Boris Ostrovsky,
	Richard Henderson, Juergen Gross, Thomas Bogendoerfer,
	Scott Branden, Jingoo Han, linux-kernel@vger.kernel.org,
	Philipp Zabel, Saheed O. Bolarinwa, Gustavo Pimentel,
	Bjorn Helgaas, David S. Miller, Heiner Kallweit
In-Reply-To: <CAOSf1CEviMYySQhyQGks8hHjST-85wGpxEBasuxwSX_homBJ2A@mail.gmail.com>

From: Oliver O'Halloran
> Sent: 15 July 2020 05:19
> 
> On Wed, Jul 15, 2020 at 8:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
...
> > - config space accesses are very rare compared to memory
> >   space access and on the hardware side the error handling
> >   would be similar, but readl/writel don't return errors, they just
> >   access wrong registers or return 0xffffffff.
> >   arch/powerpc/kernel/eeh.c has a ton extra code written to
> >   deal with it, but no other architectures do.
> 
> TBH the EEH MMIO hooks were probably a mistake to begin with. Errors
> detected via MMIO are almost always asynchronous to the error itself
> so you usually just wind up with a misleading stack trace rather than
> any kind of useful synchronous error reporting. It seems like most
> drivers don't bother checking for 0xFFs either and rely on the
> asynchronous reporting via .error_detected() instead, so I have to
> wonder what the point is. I've been thinking of removing the MMIO
> hooks and using a background poller to check for errors on each PHB
> periodically (assuming we don't have an EEH interrupt) instead. That
> would remove the requirement for eeh_dev_check_failure() to be
> interrupt safe too, so it might even let us fix all the godawful races
> in EEH.

I've 'played' with PCIe error handling - without much success.
What might be useful is for a driver that has just read ~0u to
be able to ask 'has there been an error signalled for this device?'.

I got an error generated by doing an MMIO access that was inside
the address range forwarded to the slave, but outside any of its BARs.
(Two BARs of different sizes leaves a nice gap.)
This got reported up to the bridge nearest the slave (which supported
error handling), but not to the root bridge (which I don't think does).
ISTR a message about EEH being handled by the hardware (the machine
is up but dmesg is full of messages from a bouncing USB mouse).

With such partial error reporting useful info can still be extracted.

Of course, what actually happens on a PCIe error is that the signal
gets routed to some 'board support logic' and then passed back into
the kernel as an NMI - which then crashes the kernel!
This even happens when the PCIe link goes down after we've done a
soft-remove of the device itself!
Rather makes updating the board's FPGA without a reboot tricky.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH 1/1] ASoC: fsl: fsl-asoc-card: Trivial: Fix misspelling of 'exists'
From: Lee Jones @ 2020-07-15 14:55 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, lgirdwood,
	linux-kernel, Nicolin Chen, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200715142651.GA44014@sirena.org.uk>

On Wed, 15 Jul 2020, Mark Brown wrote:

> On Wed, Jul 15, 2020 at 10:44:47AM +0100, Lee Jones wrote:
> 
> >  /*
> > - * This dapm route map exits for DPCM link only.
> > + * This dapm route map exists for DPCM link only.
> >   * The other routes shall go through Device Tree.
> 
> This doesn't apply against current code, please check and resend.

What is 'current code'?  It's applied against today's -next.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 1/1] ASoC: fsl: fsl-asoc-card: Trivial: Fix misspelling of 'exists'
From: Lee Jones @ 2020-07-15 14:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, lgirdwood,
	linux-kernel, Nicolin Chen, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200715145516.GH3165313@dell>

On Wed, 15 Jul 2020, Lee Jones wrote:

> On Wed, 15 Jul 2020, Mark Brown wrote:
> 
> > On Wed, Jul 15, 2020 at 10:44:47AM +0100, Lee Jones wrote:
> > 
> > >  /*
> > > - * This dapm route map exits for DPCM link only.
> > > + * This dapm route map exists for DPCM link only.
> > >   * The other routes shall go through Device Tree.
> > 
> > This doesn't apply against current code, please check and resend.
> 
> What is 'current code'?  It's applied against today's -next.

Hmm... something went wrong.  Stand by ...

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v2 1/1] ASoC: fsl: fsl-asoc-card: Trivial: Fix misspelling of 'exists'
From: Lee Jones @ 2020-07-15 15:00 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, linuxppc-dev,
	linux-kernel, Nicolin Chen, Lee Jones, linux-arm-kernel

Cc: Timur Tabi <timur@kernel.org>
Cc: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Xiubo Li <Xiubo.Lee@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/fsl/fsl-asoc-card.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index faac6ce9a82cb..dbacdd25dfe76 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -92,7 +92,7 @@ struct fsl_asoc_card_priv {
 };
 
 /*
- * This dapm route map exits for DPCM link only.
+ * This dapm route map exists for DPCM link only.
  * The other routes shall go through Device Tree.
  *
  * Note: keep all ASRC routes in the second half
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH -next] cpufreq: powernv: Make some symbols static
From: Rafael J. Wysocki @ 2020-07-15 15:31 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Linux PM, Rafael J. Wysocki, Hulk Robot, Viresh Kumar,
	linuxppc-dev
In-Reply-To: <20200714142355.29819-1-weiyongjun1@huawei.com>

On Tue, Jul 14, 2020 at 4:14 PM Wei Yongjun <weiyongjun1@huawei.com> wrote:
>
> The sparse tool complains as follows:
>
> drivers/cpufreq/powernv-cpufreq.c:88:1: warning:
>  symbol 'pstate_revmap' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:383:18: warning:
>  symbol 'cpufreq_freq_attr_cpuinfo_nominal_freq' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:669:6: warning:
>  symbol 'gpstate_timer_handler' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:902:6: warning:
>  symbol 'powernv_cpufreq_work_fn' was not declared. Should it be static?
>
> Those symbols are not used outside of this file, so mark
> them static.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 8646eb197cd9..cf118263ec65 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -85,7 +85,7 @@ struct global_pstate_info {
>
>  static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1];
>
> -DEFINE_HASHTABLE(pstate_revmap, POWERNV_MAX_PSTATES_ORDER);
> +static DEFINE_HASHTABLE(pstate_revmap, POWERNV_MAX_PSTATES_ORDER);
>  /**
>   * struct pstate_idx_revmap_data: Entry in the hashmap pstate_revmap
>   *                               indexed by a function of pstate id.
> @@ -380,7 +380,7 @@ static ssize_t cpuinfo_nominal_freq_show(struct cpufreq_policy *policy,
>                 powernv_freqs[powernv_pstate_info.nominal].frequency);
>  }
>
> -struct freq_attr cpufreq_freq_attr_cpuinfo_nominal_freq =
> +static struct freq_attr cpufreq_freq_attr_cpuinfo_nominal_freq =
>         __ATTR_RO(cpuinfo_nominal_freq);
>
>  #define SCALING_BOOST_FREQS_ATTR_INDEX         2
> @@ -666,7 +666,7 @@ static inline void  queue_gpstate_timer(struct global_pstate_info *gpstates)
>   * according quadratic equation. Queues a new timer if it is still not equal
>   * to local pstate
>   */
> -void gpstate_timer_handler(struct timer_list *t)
> +static void gpstate_timer_handler(struct timer_list *t)
>  {
>         struct global_pstate_info *gpstates = from_timer(gpstates, t, timer);
>         struct cpufreq_policy *policy = gpstates->policy;
> @@ -899,7 +899,7 @@ static struct notifier_block powernv_cpufreq_reboot_nb = {
>         .notifier_call = powernv_cpufreq_reboot_notifier,
>  };
>
> -void powernv_cpufreq_work_fn(struct work_struct *work)
> +static void powernv_cpufreq_work_fn(struct work_struct *work)
>  {
>         struct chip *chip = container_of(work, struct chip, throttle);
>         struct cpufreq_policy *policy;
>

Applied as 5.9 material, thanks!

^ permalink raw reply

* Re: [PATCH -next] cpuidle/pseries: Make symbol 'pseries_idle_driver' static
From: Rafael J. Wysocki @ 2020-07-15 15:31 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Linux PM, Daniel Lezcano, Rafael J. Wysocki, Hulk Robot,
	linuxppc-dev
In-Reply-To: <20200714142424.66648-1-weiyongjun1@huawei.com>

On Tue, Jul 14, 2020 at 4:14 PM Wei Yongjun <weiyongjun1@huawei.com> wrote:
>
> The sparse tool complains as follows:
>
> drivers/cpuidle/cpuidle-pseries.c:25:23: warning:
>  symbol 'pseries_idle_driver' was not declared. Should it be static?
>
> 'pseries_idle_driver' is not used outside of this file, so marks
> it static.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/cpuidle/cpuidle-pseries.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
> index 6513ef2af66a..3e058ad2bb51 100644
> --- a/drivers/cpuidle/cpuidle-pseries.c
> +++ b/drivers/cpuidle/cpuidle-pseries.c
> @@ -22,7 +22,7 @@
>  #include <asm/idle.h>
>  #include <asm/plpar_wrappers.h>
>
> -struct cpuidle_driver pseries_idle_driver = {
> +static struct cpuidle_driver pseries_idle_driver = {
>         .name             = "pseries_idle",
>         .owner            = THIS_MODULE,
>  };

Applied as 5.9 material, thanks!

^ permalink raw reply

* Re: [PATCH v3] powerpc/pseries: detect secure and trusted boot state of the system.
From: Mimi Zohar @ 2020-07-15 16:54 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev; +Cc: linux-kernel, Daniel Axtens
In-Reply-To: <1594813921-12425-1-git-send-email-nayna@linux.ibm.com>

On Wed, 2020-07-15 at 07:52 -0400, Nayna Jain wrote:
> The device-tree property to check secure and trusted boot state is
> different for guests(pseries) compared to baremetal(powernv).
> 
> This patch updates the existing is_ppc_secureboot_enabled() and
> is_ppc_trustedboot_enabled() functions to add support for pseries.
> 
> The secureboot and trustedboot state are exposed via device-tree property:
> /proc/device-tree/ibm,secure-boot and /proc/device-tree/ibm,trusted-boot
> 
> The values of ibm,secure-boot under pseries are interpreted as:
> 
> 0 - Disabled
> 1 - Enabled in Log-only mode. This patch interprets this value as
> disabled, since audit mode is currently not supported for Linux.
> 2 - Enabled and enforced.
> 3-9 - Enabled and enforcing; requirements are at the discretion of the
> operating system.
> 
> The values of ibm,trusted-boot under pseries are interpreted as:
> 0 - Disabled
> 1 - Enabled
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Reviewed-by: Daniel Axtens <dja@axtens.net>

Thanks for updating the patch description.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

^ permalink raw reply

* Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.
From: Christophe Leroy @ 2020-07-15 18:47 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-arch, nathanl, arnd, linux-kernel,
	Tulio Magno Quites Machado Filho, Paul Mackerras,
	Christophe Leroy, luto, tglx, vincenzo.frascino, linuxppc-dev
In-Reply-To: <878sflvbad.fsf@mpe.ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> a écrit :

> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> Prepare for switching VDSO to generic C implementation in following
>> patch. Here, we:
>> - Modify __get_datapage() to take an offset
>> - Prepare the helpers to call the C VDSO functions
>> - Prepare the required callbacks for the C VDSO functions
>> - Prepare the clocksource.h files to define VDSO_ARCH_CLOCKMODES
>> - Add the C trampolines to the generic C VDSO functions
>>
>> powerpc is a bit special for VDSO as well as system calls in the
>> way that it requires setting CR SO bit which cannot be done in C.
>> Therefore, entry/exit needs to be performed in ASM.
>>
>> Implementing __arch_get_vdso_data() would clobber the link register,
>> requiring the caller to save it. As the ASM calling function already
>> has to set a stack frame and saves the link register before calling
>> the C vdso function, retriving the vdso data pointer there is lighter.
> ...
>
>> diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h  
>> b/arch/powerpc/include/asm/vdso/gettimeofday.h
>> new file mode 100644
>> index 000000000000..4452897f9bd8
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
>> @@ -0,0 +1,175 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __ASM_VDSO_GETTIMEOFDAY_H
>> +#define __ASM_VDSO_GETTIMEOFDAY_H
>> +
>> +#include <asm/ptrace.h>
>> +
>> +#ifdef __ASSEMBLY__
>> +
>> +.macro cvdso_call funct
>> +  .cfi_startproc
>> +	PPC_STLU	r1, -STACK_FRAME_OVERHEAD(r1)
>> +	mflr		r0
>> +  .cfi_register lr, r0
>> +	PPC_STL		r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
>
> This doesn't work for me on ppc64(le) with glibc.
>
> glibc doesn't create a stack frame before making the VDSO call, so the
> store of r0 (LR) goes into the caller's frame, corrupting the saved LR,
> leading to an infinite loop.

Where should it be saved if it can't be saved in the standard location ?

>
> This is an example from a statically built program that calls
> clock_gettime():
>
> 0000000010030cb0 <__clock_gettime>:
>     10030cb0:   0e 10 40 3c     lis     r2,4110
>     10030cb4:   00 7a 42 38     addi    r2,r2,31232
>     10030cb8:   a6 02 08 7c     mflr    r0
>     10030cbc:   ff ff 22 3d     addis   r9,r2,-1
>     10030cc0:   58 6d 29 39     addi    r9,r9,27992
>     10030cc4:   f0 ff c1 fb     std     r30,-16(r1)			<-- redzone store
>     10030cc8:   78 23 9e 7c     mr      r30,r4
>     10030ccc:   f8 ff e1 fb     std     r31,-8(r1)			<-- redzone store
>     10030cd0:   78 1b 7f 7c     mr      r31,r3
>     10030cd4:   10 00 01 f8     std     r0,16(r1)			<-- save LR to  
> caller's frame
>     10030cd8:   00 00 09 e8     ld      r0,0(r9)
>     10030cdc:   00 00 20 2c     cmpdi   r0,0
>     10030ce0:   50 00 82 41     beq     10030d30 <__clock_gettime+0x80>
>     10030ce4:   a6 03 09 7c     mtctr   r0
>     10030ce8:   21 04 80 4e     bctrl					<-- vdso call
>     10030cec:   26 00 00 7c     mfcr    r0
>     10030cf0:   00 10 09 74     andis.  r9,r0,4096
>     10030cf4:   78 1b 69 7c     mr      r9,r3
>     10030cf8:   28 00 82 40     bne     10030d20 <__clock_gettime+0x70>
>     10030cfc:   b4 07 23 7d     extsw   r3,r9
>     10030d00:   10 00 01 e8     ld      r0,16(r1)			<-- load saved  
> LR, since clobbered by the VDSO
>     10030d04:   f0 ff c1 eb     ld      r30,-16(r1)
>     10030d08:   f8 ff e1 eb     ld      r31,-8(r1)
>     10030d0c:   a6 03 08 7c     mtlr    r0				<-- restore LR
>     10030d10:   20 00 80 4e     blr					<-- jumps to 10030cec
>
>
> I'm kind of confused how it worked for you on 32-bit.

So am I then. I'm away for 3 weeks, summer break. I'll check when I'm back.

>
> There's also no code to load/restore the TOC pointer on BE, which I
> think we'll need to handle.

What does it means exactly ? Just saving r2 all the time ? Is there a  
dedicated location in the stack frame for it ? Is that only for 64 be ?

Christophe



^ permalink raw reply

* Re: [PATCH v2 1/1] ASoC: fsl: fsl-asoc-card: Trivial: Fix misspelling of 'exists'
From: Nicolin Chen @ 2020-07-15 20:48 UTC (permalink / raw)
  To: Lee Jones
  Cc: alsa-devel, Timur Tabi, Xiubo Li, linuxppc-dev, lgirdwood,
	linux-kernel, broonie, Fabio Estevam, linux-arm-kernel
In-Reply-To: <20200715150009.407442-1-lee.jones@linaro.org>

On Wed, Jul 15, 2020 at 04:00:09PM +0100, Lee Jones wrote:
> Cc: Timur Tabi <timur@kernel.org>
> Cc: Nicolin Chen <nicoleotsuka@gmail.com>

Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>

> Cc: Xiubo Li <Xiubo.Lee@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  sound/soc/fsl/fsl-asoc-card.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index faac6ce9a82cb..dbacdd25dfe76 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -92,7 +92,7 @@ struct fsl_asoc_card_priv {
>  };
>  
>  /*
> - * This dapm route map exits for DPCM link only.
> + * This dapm route map exists for DPCM link only.
>   * The other routes shall go through Device Tree.
>   *
>   * Note: keep all ASRC routes in the second half
> -- 
> 2.25.1
> 

^ permalink raw reply

* Re: [PATCH] pseries: Fix 64 bit logical memory block panic
From: Anton Blanchard @ 2020-07-15 21:00 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: nathanl, paulus, linuxppc-dev
In-Reply-To: <87d04x3q6m.fsf@linux.ibm.com>

Hi Aneesh,

> > Booting with a 4GB LMB size causes us to panic:
> >
> >   qemu-system-ppc64: OS terminated: OS panic:
> >       Memory block size not suitable: 0x0
> >
> > Fix pseries_memory_block_size() to handle 64 bit LMBs.

> We need similar changes at more places?

I agree. I wanted to get a minimal and tested fix (using QEMU) that
could make it into stable, so that the distros will at least boot.

Thanks,
Anton

^ permalink raw reply

* Re: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: Bjorn Helgaas @ 2020-07-15 22:01 UTC (permalink / raw)
  To: David Laight
  Cc: Keith Busch, Paul Mackerras, sparclinux, Toan Le, Kjetil Oftedal,
	Greg Ungerer, Marek Vasut, Rob Herring, Lorenzo Pieralisi,
	Sagi Grimberg, Russell King, Ley Foon Tan, Christoph Hellwig,
	Geert Uytterhoeven, Kevin Hilman, linux-pci, Jakub Kicinski,
	Matt Turner, linux-kernel-mentees@lists.linuxfoundation.org,
	Guenter Roeck, 'Arnd Bergmann', Ray Jui, Jens Axboe,
	Ivan Kokshaysky, Shuah Khan, bjorn@helgaas.com, Boris Ostrovsky,
	Richard Henderson, Juergen Gross, Bjorn Helgaas,
	Thomas Bogendoerfer, Scott Branden, Jingoo Han,
	Saheed O. Bolarinwa, linux-kernel@vger.kernel.org, Philipp Zabel,
	Greg Kroah-Hartman, Gustavo Pimentel, linuxppc-dev,
	David S. Miller, Heiner Kallweit
In-Reply-To: <4c994d9a804b4a2c8555c50b63e20772@AcuMS.aculab.com>

On Wed, Jul 15, 2020 at 02:24:21PM +0000, David Laight wrote:
> From: Arnd Bergmann
> > Sent: 15 July 2020 07:47
> > On Wed, Jul 15, 2020 at 1:46 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > 
> >  So something like:
> > >
> > >   void pci_read_config_word(struct pci_dev *dev, int where, u16 *val)
> > >
> > > and where we used to return anything non-zero, we just set *val = ~0
> > > instead?  I think we do that already in most, maybe all, cases.
> > 
> > Right, this is what I had in mind. If we start by removing the handling
> > of the return code in all files that clearly don't need it, looking at
> > whatever remains will give a much better idea of what a good interface
> > should be.
> 
> It would be best to get rid of that nasty 'u16 *' parameter.

Do you mean nasty because it's basically a return value, but not
returned as the *function's* return value?  I agree that if we were
starting from scratch it would nicer to have:

  u16 pci_read_config_word(struct pci_dev *dev, int where)

but I don't think it's worth changing the thousands of callers just
for that.

> Make the return int and return the read value or -1 on error.
> (Or maybe 0xffff0000 on error??)
> 
> For a 32bit read (there must be one for the BARs) returning
> a 64bit signed integer would work even for 32bit systems.
> 
> If code cares about the error, and it can be detected then
> it can check. Otherwise the it all 'just works'.

There are u8 (byte), u16 (word), and u32 (dword) config reads &
writes.  But I don't think it really helps to return something wider
than the access.  For programmatic errors like invalid alignment, we
could indeed use the extra bits to return an unambiguous error.  But
we still have the "device was unplugged" sort of errors where the
*hardware* typically returns ~0 and the config accessor doesn't know
whether that's valid data or an error.

Bjorn

^ permalink raw reply

* Re: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: Bjorn Helgaas @ 2020-07-15 22:12 UTC (permalink / raw)
  To: David Laight
  Cc: Greg Kroah-Hartman, linux-pci, bjorn@helgaas.com, Paul Mackerras,
	sparclinux, Toan Le, Christoph Hellwig, Marek Vasut, Rob Herring,
	Lorenzo Pieralisi, Sagi Grimberg, Kevin Hilman, Russell King,
	Ley Foon Tan, Greg Ungerer, Geert Uytterhoeven, Jakub Kicinski,
	Matt Turner, linux-kernel-mentees@lists.linuxfoundation.org,
	Guenter Roeck, Arnd Bergmann, Ray Jui, linuxppc-dev, Jens Axboe,
	Ivan Kokshaysky, Shuah Khan, Keith Busch, Boris Ostrovsky,
	Richard Henderson, Juergen Gross, Thomas Bogendoerfer,
	Scott Branden, Jingoo Han, linux-kernel@vger.kernel.org,
	Philipp Zabel, Saheed O. Bolarinwa,
	'Oliver O'Halloran', Gustavo Pimentel, Bjorn Helgaas,
	David S. Miller, Heiner Kallweit
In-Reply-To: <1e2ae69a55f542faa18988a49e9b9491@AcuMS.aculab.com>

On Wed, Jul 15, 2020 at 02:38:29PM +0000, David Laight wrote:
> From: Oliver O'Halloran
> > Sent: 15 July 2020 05:19
> > 
> > On Wed, Jul 15, 2020 at 8:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
> ...
> > > - config space accesses are very rare compared to memory
> > >   space access and on the hardware side the error handling
> > >   would be similar, but readl/writel don't return errors, they just
> > >   access wrong registers or return 0xffffffff.
> > >   arch/powerpc/kernel/eeh.c has a ton extra code written to
> > >   deal with it, but no other architectures do.
> > 
> > TBH the EEH MMIO hooks were probably a mistake to begin with. Errors
> > detected via MMIO are almost always asynchronous to the error itself
> > so you usually just wind up with a misleading stack trace rather than
> > any kind of useful synchronous error reporting. It seems like most
> > drivers don't bother checking for 0xFFs either and rely on the
> > asynchronous reporting via .error_detected() instead, so I have to
> > wonder what the point is. I've been thinking of removing the MMIO
> > hooks and using a background poller to check for errors on each PHB
> > periodically (assuming we don't have an EEH interrupt) instead. That
> > would remove the requirement for eeh_dev_check_failure() to be
> > interrupt safe too, so it might even let us fix all the godawful races
> > in EEH.
> 
> I've 'played' with PCIe error handling - without much success.
> What might be useful is for a driver that has just read ~0u to
> be able to ask 'has there been an error signalled for this device?'.

In many cases a driver will know that ~0 is not a valid value for the
register it's reading.  But if ~0 *could* be valid, an interface like
you suggest could be useful.  I don't think we have anything like that
today, but maybe we could.  It would certainly be nice if the PCI core
noticed, logged, and cleared errors.  We have some of that for AER,
but that's an optional feature, and support for the error bits in the
garden-variety PCI_STATUS register is pretty haphazard.  As you note
below, this sort of SERR/PERR reporting is frequently hard-wired in
ways that takes it out of our purview.

Bjorn

^ permalink raw reply

* Re: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: Benjamin Herrenschmidt @ 2020-07-15 22:26 UTC (permalink / raw)
  To: Arnd Bergmann, Bjorn Helgaas
  Cc: linux-pci, Keith Busch, Paul Mackerras, sparclinux, Toan Le,
	Kjetil Oftedal, Greg Ungerer, Marek Vasut, Rob Herring,
	Lorenzo Pieralisi, Sagi Grimberg, Russell King, Ley Foon Tan,
	Christoph Hellwig, Geert Uytterhoeven, Kevin Hilman,
	Jakub Kicinski, Matt Turner, linux-kernel-mentees, Guenter Roeck,
	Ray Jui, Jens Axboe, Ivan Kokshaysky, Shuah Khan, bjorn,
	Boris Ostrovsky, Richard Henderson, Juergen Gross, Bjorn Helgaas,
	Thomas Bogendoerfer, Scott Branden, Jingoo Han,
	Saheed O. Bolarinwa, linux-kernel@vger.kernel.org, Philipp Zabel,
	Greg Kroah-Hartman, Gustavo Pimentel, linuxppc-dev,
	David S. Miller, Heiner Kallweit
In-Reply-To: <CAK8P3a3OEOosC2VEHb3z7hTA=BjVp0nv9bNxBWzEnmgSDDTX3Q@mail.gmail.com>

On Wed, 2020-07-15 at 08:47 +0200, Arnd Bergmann wrote:
> drivers/misc/cardreader/rts5261.c:      retval =
> rtsx_pci_write_config_dword(pcr, PCR_SETTING_REG2, lval);
> 
> That last one is interesting because I think this is a case in which
> we
> actually want to check for errors, as the driver seems to use it
> to ensure that accessing extended config space at offset 0x814
> works before relying on the value. Unfortunately the implementation
> seems buggy as it a) keeps using the possibly uninitialized value
> after
> printing a warning and b) returns the PCIBIOS_* value in place of a
> negative errno and then ignores it in the caller.

In cases like this, usually checking against ~0 is sufficient

Cheers,
Ben.



^ permalink raw reply

* Re: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: Benjamin Herrenschmidt @ 2020-07-15 22:49 UTC (permalink / raw)
  To: Bjorn Helgaas, David Laight
  Cc: Greg Kroah-Hartman, linux-pci, bjorn@helgaas.com, Paul Mackerras,
	sparclinux, Toan Le, Christoph Hellwig, Marek Vasut, Rob Herring,
	Lorenzo Pieralisi, Sagi Grimberg, Kevin Hilman, Russell King,
	Ley Foon Tan, Greg Ungerer, Geert Uytterhoeven, Jakub Kicinski,
	Matt Turner, linux-kernel-mentees@lists.linuxfoundation.org,
	Guenter Roeck, Arnd Bergmann, Ray Jui, linuxppc-dev, Jens Axboe,
	Ivan Kokshaysky, Shuah Khan, Keith Busch, Boris Ostrovsky,
	Richard Henderson, Juergen Gross, Thomas Bogendoerfer,
	Scott Branden, Jingoo Han, linux-kernel@vger.kernel.org,
	Philipp Zabel, Saheed O. Bolarinwa,
	'Oliver O'Halloran', Gustavo Pimentel, Bjorn Helgaas,
	David S. Miller, Heiner Kallweit
In-Reply-To: <20200715221230.GA563957@bjorn-Precision-5520>

On Wed, 2020-07-15 at 17:12 -0500, Bjorn Helgaas wrote:
> > I've 'played' with PCIe error handling - without much success.
> > What might be useful is for a driver that has just read ~0u to
> > be able to ask 'has there been an error signalled for this device?'.
> 
> In many cases a driver will know that ~0 is not a valid value for the
> register it's reading.  But if ~0 *could* be valid, an interface like
> you suggest could be useful.  I don't think we have anything like that
> today, but maybe we could.  It would certainly be nice if the PCI core
> noticed, logged, and cleared errors.  We have some of that for AER,
> but that's an optional feature, and support for the error bits in the
> garden-variety PCI_STATUS register is pretty haphazard.  As you note
> below, this sort of SERR/PERR reporting is frequently hard-wired in
> ways that takes it out of our purview.

We do have pci_channel_state (via pci_channel_offline()) which covers
the cases where the underlying error handling (such as EEH or unplug)
results in the device being offlined though this tend to be
asynchronous so it might take a few ~0's before you get it.

It's typically used to break potentially infinite loops in some
drivers.

There is no interface to check whether *an* error happened though for
the most cases it will be captured in the status register, which is
harvested (and cleared ?) by some EDAC drivers iirc... 

All this lacks coordination, I agree.

Cheers,
Ben.



^ permalink raw reply

* Re: [PATCH v3 06/12] ppc64/kexec_file: restrict memory usage of kdump kernel
From: Thiago Jung Bauermann @ 2020-07-15 22:52 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Nayna Jain, Kexec-ml, Mahesh J Salgaonkar,
	Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain, Petr Tesarik,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <159466091925.24747.6840028682768745598.stgit@hbathini.in.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

>  /**
> + * get_usable_memory_ranges - Get usable memory ranges. This list includes
> + *                            regions like crashkernel, opal/rtas & tce-table,
> + *                            that kdump kernel could use.
> + * @mem_ranges:               Range list to add the memory ranges to.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int get_usable_memory_ranges(struct crash_mem **mem_ranges)
> +{
> +	int ret;
> +
> +	/* First memory block & crashkernel region */
> +	ret = add_mem_range(mem_ranges, 0, crashk_res.end + 1);

This is a bit surprising. I guess I don't have a complete big picture of
the patch series yet. What prevents the crashkernel from using memory at
the [0, _end] range and overwriting the crashed kernel's memory?

Shouldn't the above range start at crashk_res.start?

> +	if (ret)
> +		goto out;
> +
> +	ret = add_rtas_mem_range(mem_ranges);
> +	if (ret)
> +		goto out;
> +
> +	ret = add_opal_mem_range(mem_ranges);
> +	if (ret)
> +		goto out;
> +
> +	ret = add_tce_mem_ranges(mem_ranges);
> +out:
> +	if (ret)
> +		pr_err("Failed to setup usable memory ranges\n");
> +	return ret;
> +}
> +
> +/**
>   * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
>   *                              in the memory regions between buf_min & buf_max
>   *                              for the buffer. If found, sets kbuf->mem.
> @@ -261,6 +305,322 @@ static int locate_mem_hole_bottom_up_ppc64(struct kexec_buf *kbuf,
>  }
>
>  /**
> + * check_realloc_usable_mem - Reallocate buffer if it can't accommodate entries
> + * @um_info:                  Usable memory buffer and ranges info.
> + * @cnt:                      No. of entries to accommodate.
> + *
> + * Returns 0 on success, negative errno on error.

It actually returns the buffer on success, and NULL on error.

> + */
> +static uint64_t *check_realloc_usable_mem(struct umem_info *um_info, int cnt)
> +{
> +	void *tbuf;
> +
> +	if (um_info->size >=
> +	    ((um_info->idx + cnt) * sizeof(*(um_info->buf))))
> +		return um_info->buf;
> +
> +	um_info->size += MEM_RANGE_CHUNK_SZ;
> +	tbuf = krealloc(um_info->buf, um_info->size, GFP_KERNEL);
> +	if (!tbuf) {
> +		um_info->size -= MEM_RANGE_CHUNK_SZ;
> +		return NULL;
> +	}
> +
> +	memset(tbuf + um_info->idx, 0, MEM_RANGE_CHUNK_SZ);
> +	return tbuf;
> +}

<snip>

> +/**
> + * get_node_path - Get the full path of the given node.
> + * @dn:            Node.
> + * @path:          Updated with the full path of the node.
> + *
> + * Returns nothing.
> + */
> +static void get_node_path(struct device_node *dn, char *path)
> +{
> +	if (!dn)
> +		return;
> +
> +	get_node_path(dn->parent, path);

Is it ok to do recursion in the kernel? In this case I believe it's not
problematic since the maximum call depth will be the maximum depth of a
device tree node which shouldn't be too much. Also, there are no local
variables in this function. But I thought it was worth mentioning.

> +	sprintf(path, "/%s", dn->full_name);
> +}
> +
> +/**
> + * get_node_pathlen - Get the full path length of the given node.
> + * @dn:               Node.
> + *
> + * Returns the length of the full path of the node.
> + */
> +static int get_node_pathlen(struct device_node *dn)
> +{
> +	int len = 0;
> +
> +	while (dn) {
> +		len += strlen(dn->full_name) + 1;
> +		dn = dn->parent;
> +	}
> +	len++;
> +
> +	return len;
> +}
> +
> +/**
> + * add_usable_mem_property - Add usable memory property for the given
> + *                           memory node.
> + * @fdt:                     Flattened device tree for the kdump kernel.
> + * @dn:                      Memory node.
> + * @um_info:                 Usable memory buffer and ranges info.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int add_usable_mem_property(void *fdt, struct device_node *dn,
> +				   struct umem_info *um_info)
> +{
> +	int n_mem_addr_cells, n_mem_size_cells, node;
> +	int i, len, ranges, cnt, ret;
> +	uint64_t base, end, *buf;
> +	const __be32 *prop;
> +	char *pathname;
> +
> +	/* Allocate memory for node path */
> +	pathname = kzalloc(ALIGN(get_node_pathlen(dn), 8), GFP_KERNEL);
> +	if (!pathname)
> +		return -ENOMEM;
> +
> +	/* Get the full path of the memory node */
> +	get_node_path(dn, pathname);
> +	pr_debug("Memory node path: %s\n", pathname);
> +
> +	/* Now that we know the path, find its offset in kdump kernel's fdt */
> +	node = fdt_path_offset(fdt, pathname);
> +	if (node < 0) {
> +		pr_err("Malformed device tree: error reading %s\n",
> +		       pathname);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* Get the address & size cells */
> +	n_mem_addr_cells = of_n_addr_cells(dn);
> +	n_mem_size_cells = of_n_size_cells(dn);
> +	pr_debug("address cells: %d, size cells: %d\n", n_mem_addr_cells,
> +		 n_mem_size_cells);
> +
> +	um_info->idx  = 0;
> +	buf = check_realloc_usable_mem(um_info, 2);
> +	if (!buf) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	um_info->buf = buf;
> +
> +	prop = of_get_property(dn, "reg", &len);
> +	if (!prop || len <= 0) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	/*
> +	 * "reg" property represents sequence of (addr,size) duples

s/duples/tuples/ ?

> +	 * each representing a memory range.
> +	 */
> +	ranges = (len >> 2) / (n_mem_addr_cells + n_mem_size_cells);
> +
> +	for (i = 0; i < ranges; i++) {
> +		base = of_read_number(prop, n_mem_addr_cells);
> +		prop += n_mem_addr_cells;
> +		end = base + of_read_number(prop, n_mem_size_cells) - 1;

You need to `prop += n_mem_size_cells` here.

> +
> +		ret = add_usable_mem(um_info, base, end, &cnt);
> +		if (ret) {
> +			ret = ret;
> +			goto out;
> +		}
> +	}
> +
> +	/*
> +	 * No kdump kernel usable memory found in this memory node.
> +	 * Write (0,0) duple in linux,usable-memory property for

s/duple/tuple/ ?

> +	 * this region to be ignored.
> +	 */
> +	if (um_info->idx == 0) {
> +		um_info->buf[0] = 0;
> +		um_info->buf[1] = 0;
> +		um_info->idx = 2;
> +	}
> +
> +	ret = fdt_setprop(fdt, node, "linux,usable-memory", um_info->buf,
> +			  (um_info->idx * sizeof(*(um_info->buf))));
> +
> +out:
> +	kfree(pathname);
> +	return ret;
> +}

--
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* [PATCH] powerpc/vdso: Fix vdso cpu truncation
From: Anton Blanchard @ 2020-07-15 23:37 UTC (permalink / raw)
  To: benh, paulus, mpe, miltonm; +Cc: linuxppc-dev

From: Milton Miller <miltonm@us.ibm.com>

The code in vdso_cpu_init that exposes the cpu and numa node to
userspace via SPRG_VDSO incorrctly masks the cpu to 12 bits. This means
that any kernel running on a box with more than 4096 threads (NR_CPUS
advertises a limit of of 8192 cpus) would expose userspace to two cpu
contexts running at the same time with the same cpu number.

Note: I'm not aware of any distro shipping a kernel with support for more
than 4096 threads today, nor of any system image that currently exceeds
4096 threads. Found via code browsing.

Fixes: 18ad51dd342a7eb09dbcd059d0b451b616d4dafc ("powerpc: Add VDSO version of getcpu")
Signed-off-by: Milton Miller <miltonm@us.ibm.com>
Signed-off-by: Anton Blanchard <anton@linux.ibm.com>
---
 arch/powerpc/kernel/vdso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index e0f4ba45b6cc..8dad44262e75 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -677,7 +677,7 @@ int vdso_getcpu_init(void)
 	node = cpu_to_node(cpu);
 	WARN_ON_ONCE(node > 0xffff);
 
-	val = (cpu & 0xfff) | ((node & 0xffff) << 16);
+	val = (cpu & 0xffff) | ((node & 0xffff) << 16);
 	mtspr(SPRN_SPRG_VDSO_WRITE, val);
 	get_paca()->sprg_vdso = val;
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH net-next] ibmvnic: Increase driver logging
From: Thomas Falcon @ 2020-07-15 23:51 UTC (permalink / raw)
  To: netdev; +Cc: drt, Thomas Falcon, linuxppc-dev

Improve the ibmvnic driver's logging capabilities by providing
more informational messages to track driver operations, facilitating
first-pass debug.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 76 ++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 0fd7eae25fe9..7382f11872fc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -561,6 +561,7 @@ static int init_rx_pools(struct net_device *netdev)
 	u64 *size_array;
 	int i, j;
 
+	netdev_info(netdev, "Initializing RX queue buffer pools.\n");
 	rxadd_subcrqs =
 		be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
 	size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
@@ -618,6 +619,7 @@ static int init_rx_pools(struct net_device *netdev)
 		rx_pool->next_alloc = 0;
 		rx_pool->next_free = 0;
 	}
+	netdev_info(netdev, "RX queue buffer pools allocated successfully.\n");
 
 	return 0;
 }
@@ -738,6 +740,7 @@ static int init_tx_pools(struct net_device *netdev)
 	int tx_subcrqs;
 	int i, rc;
 
+	netdev_info(netdev, "Initializing TX queue buffer pools.\n");
 	tx_subcrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
 	adapter->tx_pool = kcalloc(tx_subcrqs,
 				   sizeof(struct ibmvnic_tx_pool), GFP_KERNEL);
@@ -768,7 +771,7 @@ static int init_tx_pools(struct net_device *netdev)
 			return rc;
 		}
 	}
-
+	netdev_info(netdev, "TX queue buffer pools allocated successfully.\n");
 	return 0;
 }
 
@@ -792,6 +795,7 @@ static void ibmvnic_napi_disable(struct ibmvnic_adapter *adapter)
 	if (!adapter->napi_enabled)
 		return;
 
+	netdev_info(adapter->netdev, "Disable all NAPI threads.\n");
 	for (i = 0; i < adapter->req_rx_queues; i++) {
 		netdev_dbg(adapter->netdev, "Disabling napi[%d]\n", i);
 		napi_disable(&adapter->napi[i]);
@@ -910,12 +914,14 @@ static int ibmvnic_login(struct net_device *netdev)
 				return -1;
 			}
 		} else if (adapter->init_done_rc) {
-			netdev_warn(netdev, "Adapter login failed\n");
+			netdev_warn(netdev, "Adapter login failed, rc = %d\n",
+				    adapter->init_done_rc);
 			return -1;
 		}
 	} while (retry);
 
 	__ibmvnic_set_mac(netdev, adapter->mac_addr);
+	netdev_info(netdev, "Adapter login success!\n");
 
 	return 0;
 }
@@ -934,6 +940,7 @@ static void release_login_rsp_buffer(struct ibmvnic_adapter *adapter)
 
 static void release_resources(struct ibmvnic_adapter *adapter)
 {
+	netdev_info(adapter->netdev, "Releasing VNIC client device memory structures.\n");
 	release_vpd_data(adapter);
 
 	release_tx_pools(adapter);
@@ -941,6 +948,7 @@ static void release_resources(struct ibmvnic_adapter *adapter)
 
 	release_napi(adapter);
 	release_login_rsp_buffer(adapter);
+	netdev_info(adapter->netdev, "VNIC client device memory released.\n");
 }
 
 static int set_link_state(struct ibmvnic_adapter *adapter, u8 link_state)
@@ -964,7 +972,7 @@ static int set_link_state(struct ibmvnic_adapter *adapter, u8 link_state)
 		reinit_completion(&adapter->init_done);
 		rc = ibmvnic_send_crq(adapter, &crq);
 		if (rc) {
-			netdev_err(netdev, "Failed to set link state\n");
+			netdev_err(netdev, "Failed to set link state, rc = %d\n", rc);
 			return rc;
 		}
 
@@ -1098,6 +1106,8 @@ static int init_resources(struct ibmvnic_adapter *adapter)
 	struct net_device *netdev = adapter->netdev;
 	int rc;
 
+	netdev_info(netdev, "Allocate device resources.\n");
+
 	rc = set_real_num_queues(netdev);
 	if (rc)
 		return rc;
@@ -1126,7 +1136,11 @@ static int init_resources(struct ibmvnic_adapter *adapter)
 		return rc;
 
 	rc = init_tx_pools(netdev);
-	return rc;
+	if (rc)
+		return rc;
+
+	netdev_info(netdev, "Device resources allocated.\n");
+	return 0;
 }
 
 static int __ibmvnic_open(struct net_device *netdev)
@@ -1136,9 +1150,10 @@ static int __ibmvnic_open(struct net_device *netdev)
 	int i, rc;
 
 	adapter->state = VNIC_OPENING;
+	netdev_info(netdev, "Allocating RX buffer pools and enable NAPI structures.\n");
 	replenish_pools(adapter);
 	ibmvnic_napi_enable(adapter);
-
+	netdev_info(netdev, "Activating RX and TX interrupt lines.\n");
 	/* We're ready to receive frames, enable the sub-crq interrupts and
 	 * set the logical link state to up
 	 */
@@ -1155,7 +1170,7 @@ static int __ibmvnic_open(struct net_device *netdev)
 			enable_irq(adapter->tx_scrq[i]->irq);
 		enable_scrq_irq(adapter, adapter->tx_scrq[i]);
 	}
-
+	netdev_info(netdev, "Inform system firmware that device is ready for packet transmission.\n");
 	rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP);
 	if (rc) {
 		for (i = 0; i < adapter->req_rx_queues; i++)
@@ -1163,7 +1178,7 @@ static int __ibmvnic_open(struct net_device *netdev)
 		release_resources(adapter);
 		return rc;
 	}
-
+	netdev_info(netdev, "Activate net device TX queues.\n");
 	netif_tx_start_all_queues(netdev);
 
 	if (prev_state == VNIC_CLOSED) {
@@ -1180,6 +1195,7 @@ static int ibmvnic_open(struct net_device *netdev)
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	int rc;
 
+	netdev_info(netdev, "Opening VNIC client device.\n");
 	/* If device failover is pending, just set device state and return.
 	 * Device operation will be handled by reset routine.
 	 */
@@ -1202,7 +1218,7 @@ static int ibmvnic_open(struct net_device *netdev)
 	}
 
 	rc = __ibmvnic_open(netdev);
-
+	netdev_info(netdev, "VNIC client device opened.\n");
 	return rc;
 }
 
@@ -1216,7 +1232,7 @@ static void clean_rx_pools(struct ibmvnic_adapter *adapter)
 
 	if (!adapter->rx_pool)
 		return;
-
+	netdev_info(adapter->netdev, "Freeing all existing RX buffer pool memory.\n");
 	rx_scrqs = adapter->num_active_rx_pools;
 	rx_entries = adapter->req_rx_add_entries_per_subcrq;
 
@@ -1265,7 +1281,7 @@ static void clean_tx_pools(struct ibmvnic_adapter *adapter)
 
 	if (!adapter->tx_pool || !adapter->tso_pool)
 		return;
-
+	netdev_info(adapter->netdev, "Freeing all outstanding TX buffers awaiting completion.\n");
 	tx_scrqs = adapter->num_active_tx_pools;
 
 	/* Free any remaining skbs in the tx buffer pools */
@@ -1282,6 +1298,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter *adapter)
 	int i;
 
 	if (adapter->tx_scrq) {
+		netdev_info(netdev, "Disabling all TX interrupt lines.\n");
 		for (i = 0; i < adapter->req_tx_queues; i++)
 			if (adapter->tx_scrq[i]->irq) {
 				netdev_dbg(netdev,
@@ -1292,6 +1309,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter *adapter)
 	}
 
 	if (adapter->rx_scrq) {
+		netdev_info(netdev, "Disabling all RX interrupt lines.\n");
 		for (i = 0; i < adapter->req_rx_queues; i++) {
 			if (adapter->rx_scrq[i]->irq) {
 				netdev_dbg(netdev,
@@ -1307,6 +1325,7 @@ static void ibmvnic_cleanup(struct net_device *netdev)
 {
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 
+	netdev_info(netdev, "Halt net device TX queues.\n");
 	/* ensure that transmissions are stopped if called by do_reset */
 	if (test_bit(0, &adapter->resetting))
 		netif_tx_disable(netdev);
@@ -1326,6 +1345,7 @@ static int __ibmvnic_close(struct net_device *netdev)
 	int rc = 0;
 
 	adapter->state = VNIC_CLOSING;
+	netdev_info(netdev, "Halt incoming packets from system firmware.\n");
 	rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
 	if (rc)
 		return rc;
@@ -1338,6 +1358,7 @@ static int ibmvnic_close(struct net_device *netdev)
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	int rc;
 
+	netdev_info(netdev, "Stopping VNIC client device.\n");
 	/* If device failover is pending, just set device state and return.
 	 * Device operation will be handled by reset routine.
 	 */
@@ -1348,7 +1369,7 @@ static int ibmvnic_close(struct net_device *netdev)
 
 	rc = __ibmvnic_close(netdev);
 	ibmvnic_cleanup(netdev);
-
+	netdev_info(netdev, "VNIC client device stopped.\n");
 	return rc;
 }
 
@@ -2941,9 +2962,11 @@ static struct ibmvnic_sub_crq_queue *init_sub_crq_queue(struct ibmvnic_adapter
 
 static void release_sub_crqs(struct ibmvnic_adapter *adapter, bool do_h_free)
 {
+	struct net_device *netdev = adapter->netdev;
 	int i;
 
 	if (adapter->tx_scrq) {
+		netdev_info(netdev, "Releasing TX subordinate Command-Response Queues.\n");
 		for (i = 0; i < adapter->num_active_tx_scrqs; i++) {
 			if (!adapter->tx_scrq[i])
 				continue;
@@ -2964,9 +2987,11 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter, bool do_h_free)
 		kfree(adapter->tx_scrq);
 		adapter->tx_scrq = NULL;
 		adapter->num_active_tx_scrqs = 0;
+		netdev_info(netdev, "TX subordinate Command-Response Queues released.\n");
 	}
 
 	if (adapter->rx_scrq) {
+		netdev_info(netdev, "Releasing RX subordinate Command-Response Queues.\n");
 		for (i = 0; i < adapter->num_active_rx_scrqs; i++) {
 			if (!adapter->rx_scrq[i])
 				continue;
@@ -2987,6 +3012,7 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter, bool do_h_free)
 		kfree(adapter->rx_scrq);
 		adapter->rx_scrq = NULL;
 		adapter->num_active_rx_scrqs = 0;
+		netdev_info(netdev, "RX subordinate Command-Response Queues released.\n");
 	}
 }
 
@@ -3149,6 +3175,7 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
 	int i = 0, j = 0;
 	int rc = 0;
 
+	netdev_info(adapter->netdev, "Request Subordinate Command-Response Queue interrupts.\n");
 	for (i = 0; i < adapter->req_tx_queues; i++) {
 		netdev_dbg(adapter->netdev, "Initializing tx_scrq[%d] irq\n",
 			   i);
@@ -3195,6 +3222,9 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
 			goto req_rx_irq_failed;
 		}
 	}
+
+	netdev_info(adapter->netdev, "Subordinate Command-Response Queue interrupts created.\n");
+
 	return rc;
 
 req_rx_irq_failed:
@@ -3221,6 +3251,8 @@ static int init_sub_crqs(struct ibmvnic_adapter *adapter)
 	int more = 0;
 	int i;
 
+	netdev_info(adapter->netdev, "Creating Command-Response Queues.\n");
+
 	total_queues = adapter->req_tx_queues + adapter->req_rx_queues;
 
 	allqueues = kcalloc(total_queues, sizeof(*allqueues), GFP_KERNEL);
@@ -3285,6 +3317,8 @@ static int init_sub_crqs(struct ibmvnic_adapter *adapter)
 	}
 
 	kfree(allqueues);
+
+	netdev_info(adapter->netdev, "Command-Response Queue creation complete.\n");
 	return 0;
 
 rx_failed:
@@ -3303,6 +3337,8 @@ static void ibmvnic_send_req_caps(struct ibmvnic_adapter *adapter, int retry)
 	union ibmvnic_crq crq;
 	int max_entries;
 
+	netdev_info(adapter->netdev, "Negotiate device capabilities.\n");
+
 	if (!retry) {
 		/* Sub-CRQ entries are 32 byte long */
 		int entries_page = 4 * PAGE_SIZE / (sizeof(u64) * 4);
@@ -3822,6 +3858,8 @@ static void send_cap_queries(struct ibmvnic_adapter *adapter)
 {
 	union ibmvnic_crq crq;
 
+	netdev_info(adapter->netdev, "Requesting device capabilities.\n");
+
 	atomic_set(&adapter->running_cap_crqs, 0);
 	memset(&crq, 0, sizeof(crq));
 	crq.query_capability.first = IBMVNIC_CRQ_CMD;
@@ -4115,7 +4153,8 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
 		adapter->netdev->features |=
 				tmp & adapter->netdev->wanted_features;
 	}
-
+	netdev_info(adapter->netdev,
+		    "Confirming device offload capabilities.\n");
 	memset(&crq, 0, sizeof(crq));
 	crq.control_ip_offload.first = IBMVNIC_CRQ_CMD;
 	crq.control_ip_offload.cmd = CONTROL_IP_OFFLOAD;
@@ -4263,6 +4302,9 @@ static void handle_request_cap_rsp(union ibmvnic_crq *crq,
 		struct ibmvnic_query_ip_offload_buffer *ip_offload_buf =
 		    &adapter->ip_offload_buf;
 
+		netdev_info(adapter->netdev,
+			    "Query device offload features.\n");
+
 		adapter->wait_capability = false;
 		adapter->ip_offload_tok = dma_map_single(dev, ip_offload_buf,
 							 buf_sz,
@@ -4881,7 +4923,7 @@ static void release_crq_queue(struct ibmvnic_adapter *adapter)
 	if (!crq->msgs)
 		return;
 
-	netdev_dbg(adapter->netdev, "Releasing CRQ\n");
+	netdev_info(adapter->netdev, "Releasing VNIC Command-Response Queue.\n");
 	free_irq(vdev->irq, adapter);
 	tasklet_kill(&adapter->tasklet);
 	do {
@@ -4893,6 +4935,7 @@ static void release_crq_queue(struct ibmvnic_adapter *adapter)
 	free_page((unsigned long)crq->msgs);
 	crq->msgs = NULL;
 	crq->active = false;
+	netdev_info(adapter->netdev, "VNIC Command-Response Queue released.\n");
 }
 
 static int init_crq_queue(struct ibmvnic_adapter *adapter)
@@ -5193,6 +5236,7 @@ static int ibmvnic_remove(struct vio_dev *dev)
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	unsigned long flags;
 
+	netdev_info(netdev, "Attempting to remove VNIC client device.\n");
 	spin_lock_irqsave(&adapter->state_lock, flags);
 	if (adapter->state == VNIC_RESETTING) {
 		spin_unlock_irqrestore(&adapter->state_lock, flags);
@@ -5206,22 +5250,26 @@ static int ibmvnic_remove(struct vio_dev *dev)
 	flush_delayed_work(&adapter->ibmvnic_delayed_reset);
 
 	rtnl_lock();
+	netdev_info(netdev, "Unregistering net device.\n");
 	unregister_netdevice(netdev);
 
+	netdev_info(netdev, "Releasing VNIC client device resources.\n");
 	release_resources(adapter);
 	release_sub_crqs(adapter, 1);
 	release_crq_queue(adapter);
 
 	release_stats_token(adapter);
 	release_stats_buffers(adapter);
-
+	netdev_info(netdev, "VNIC client device resources released.\n");
 	adapter->state = VNIC_REMOVED;
 
 	rtnl_unlock();
+	netdev_info(netdev, "Freeing net device and VNIC client driver data.\n");
 	mutex_destroy(&adapter->fw_lock);
 	device_remove_file(&dev->dev, &dev_attr_failover);
 	free_netdev(netdev);
 	dev_set_drvdata(&dev->dev, NULL);
+	netdev_info(netdev, "VNIC client device has been successfully removed.\n");
 
 	return 0;
 }
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH net-next] ibmvnic: Increase driver logging
From: Jakub Kicinski @ 2020-07-16  0:06 UTC (permalink / raw)
  To: Thomas Falcon; +Cc: drt, netdev, linuxppc-dev
In-Reply-To: <1594857115-22380-1-git-send-email-tlfalcon@linux.ibm.com>

On Wed, 15 Jul 2020 18:51:55 -0500 Thomas Falcon wrote:
>  	free_netdev(netdev);
>  	dev_set_drvdata(&dev->dev, NULL);
> +	netdev_info(netdev, "VNIC client device has been successfully removed.\n");

A step too far, perhaps.

In general this patch looks a little questionable IMHO, this amount of
logging output is not commonly seen in drivers. All the the info
messages are just static text, not even carrying any extra information.
In an era of ftrace, and bpftrace, do we really need this?

^ permalink raw reply

* Re: [PATCH v3 07/12] ppc64/kexec_file: add support to relocate purgatory
From: Thiago Jung Bauermann @ 2020-07-16  0:20 UTC (permalink / raw)
  To: Hari Bathini
  Cc: kernel test robot, Pingfan Liu, Nayna Jain, Kexec-ml,
	Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain,
	Petr Tesarik, Andrew Morton, Dave Young, Vivek Goyal,
	Eric Biederman
In-Reply-To: <159466093748.24747.4655547403463921814.stgit@hbathini.in.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> Right now purgatory implementation is only minimal. But if purgatory
> code is to be enhanced to copy memory to the backup region and verify

Can't the memcpy be done in asm? We have arch/powerpc/lib/memcpy_64.S
for example, perhaps it could be linked in with the purgatory?

> sha256 digest, relocations may have to be applied to the purgatory.

Do we want to do the sha256 verification? My original patch series for
kexec_file_load() had a purgatory in C from kexec-tools which did the
sha256 verification but Michael Ellerman thought it was unnecessary and
decided to use the simpler purgatory in asm from kexec-lite.

As a result, this relocation processing became unnecessary.

> So, add support to relocate purgatory in kexec_file_load system call
> by setting up TOC pointer and applying RELA relocations as needed.

If we do want to use a C purgatory, Michael Ellerman had suggested
building it as a Position Independent Executable, which greatly reduces
the number and types of relocations that are needed. See patches 4 and 9
here:

https://lore.kernel.org/linuxppc-dev/1478748449-3894-1-git-send-email-bauerman@linux.vnet.ibm.com/

In the series above I hadn't converted x86 to PIE. If I had done that,
possibly Dave Young's opinion would have been different. :-)

If that's still not desirable, he suggested in that discussion lifting
some code from x86 to generic code, which I implemented and would
simplify this patch as well:

https://lore.kernel.org/linuxppc-dev/5009580.5GxAkTrMYA@morokweng/

> Reported-by: kernel test robot <lkp@intel.com>
> [lkp: In v1, 'struct mem_sym' was declared in parameter list]
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> v2 -> v3:
> * Fixed get_toc_section() to return the section info that had relocations
>   applied, to calculate the correct toc pointer.
> * Fixed how relocation value is converted to relative while applying
>   R_PPC64_REL64 & R_PPC64_REL32 relocations.
>
> v1 -> v2:
> * Fixed wrong use of 'struct mem_sym' in local_entry_offset() as
>   reported by lkp. lkp report for reference:
>     - https://lore.kernel.org/patchwork/patch/1264421/
>
>
>  arch/powerpc/kexec/file_load_64.c      |  337 ++++++++++++++++++++++++++++++++
>  arch/powerpc/purgatory/trampoline_64.S |    8 +
>  2 files changed, 345 insertions(+)

--
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 1/1] powerpc: Fix incorrect stw{, ux, u, x} instructions in __set_pte_at
From: Sasha Levin @ 2020-07-16  0:27 UTC (permalink / raw)
  To: Sasha Levin, Mathieu Desnoyers, Christophe Leroy
  Cc: linux-kernel, Paul Mackerras, stable, linuxppc-dev
In-Reply-To: <20200708175423.28442-1-mathieu.desnoyers@efficios.com>

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 9bf2b5cdc5fe ("powerpc: Fixes for CONFIG_PTE_64BIT for SMP support").

The bot has tested the following trees: v5.7.8, v5.4.51, v4.19.132, v4.14.188, v4.9.230, v4.4.230.

v5.7.8: Build OK!
v5.4.51: Build OK!
v4.19.132: Build OK!
v4.14.188: Failed to apply! Possible dependencies:
    45201c8794693 ("powerpc/nohash: Remove hash related code from nohash headers.")
    4e003747043d5 ("powerpc/64s: Replace CONFIG_PPC_STD_MMU_64 with CONFIG_PPC_BOOK3S_64")
    d5808ffaec817 ("powerpc/nohash: Use IS_ENABLED() to simplify __set_pte_at()")

v4.9.230: Failed to apply! Possible dependencies:
    45201c8794693 ("powerpc/nohash: Remove hash related code from nohash headers.")
    4546561551106 ("powerpc/asm: Use OFFSET macro in asm-offsets.c")
    4e003747043d5 ("powerpc/64s: Replace CONFIG_PPC_STD_MMU_64 with CONFIG_PPC_BOOK3S_64")
    5d451a87e5ebb ("powerpc/64: Retrieve number of L1 cache sets from device-tree")
    7c5b06cadf274 ("KVM: PPC: Book3S HV: Adapt TLB invalidations to work on POWER9")
    83677f551e0a6 ("KVM: PPC: Book3S HV: Adjust host/guest context switch for POWER9")
    902e06eb86cd6 ("powerpc/32: Change the stack protector canary value per task")
    a3d96f70c1477 ("powerpc/64s: Fix system reset vs general interrupt reentrancy")
    bd067f83b0840 ("powerpc/64: Fix naming of cache block vs. cache line")
    d5808ffaec817 ("powerpc/nohash: Use IS_ENABLED() to simplify __set_pte_at()")
    e2827fe5c1566 ("powerpc/64: Clean up ppc64_caches using a struct per cache")
    e9cf1e085647b ("KVM: PPC: Book3S HV: Add new POWER9 guest-accessible SPRs")
    f4c51f841d2ac ("KVM: PPC: Book3S HV: Modify guest entry/exit paths to handle radix guests")

v4.4.230: Failed to apply! Possible dependencies:
    1ca7212932862 ("powerpc/mm: Move PTE bits from generic functions to hash64 functions.")
    26b6a3d9bb48f ("powerpc/mm: move pte headers to book3s directory")
    371352ca0e7f3 ("powerpc/mm: Move hash64 PTE bits from book3s/64/pgtable.h to hash.h")
    3dfcb315d81e6 ("powerpc/mm: make a separate copy for book3s")
    ab537dca2f330 ("powerpc/mm: Move hash specific pte width and other defines to book3s")
    b0412ea94bcbd ("powerpc/mm: Drop pte-common.h from BOOK3S 64")
    cbbb8683fb632 ("powerpc/mm: Delete booke bits from book3s")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

^ permalink raw reply

* Re: [PATCH v3 08/12] ppc64/kexec_file: setup the stack for purgatory
From: Thiago Jung Bauermann @ 2020-07-16  0:35 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Pingfan Liu, Nayna Jain, Kexec-ml, Mahesh J Salgaonkar,
	Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain, Petr Tesarik,
	Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <159466095278.24747.9161591016931052627.stgit@hbathini.in.ibm.com>


Hari Bathini <hbathini@linux.ibm.com> writes:

> To avoid any weird errors, the purgatory should run with its own
> stack. Set one up by adding the stack buffer to .data section of
> the purgatory. Also, setup opal base & entry values in r8 & r9
> registers to help early OPAL debugging.
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> Tested-by: Pingfan Liu <piliu@redhat.com>

Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>

> ---
>
> v2 -> v3:
> * Unchanged. Added Tested-by tag from Pingfan.
>
> v1 -> v2:
> * Setting up opal base & entry values in r8 & r9 for early OPAL debug.
>
>
>  arch/powerpc/include/asm/kexec.h       |    4 ++++
>  arch/powerpc/kexec/file_load_64.c      |   29 +++++++++++++++++++++++++++++
>  arch/powerpc/purgatory/trampoline_64.S |   32 ++++++++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
>

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply


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