* Re: [PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute
From: Nicolin Chen @ 2019-04-10 4:29 UTC (permalink / raw)
To: Viorel Suman
Cc: Mark Rutland, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
Timur Tabi, Xiubo Li, Shawn Guo, Sascha Hauer, Takashi Iwai,
Liam Girdwood, Rob Herring, Jaroslav Kysela, Viorel Suman,
Julia Lawall, Mark Brown, dl-linux-imx, Pengutronix Kernel Team,
Fabio Estevam, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554809242-27475-2-git-send-email-viorel.suman@nxp.com>
On Tue, Apr 09, 2019 at 11:27:39AM +0000, Viorel Suman wrote:
> Use "of_device_id.data" to specify the machine driver
> instead of "model" DTS attribute.
>
> Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
> ---
> sound/soc/fsl/fsl_audmix.c | 43 +++++++++++++++++++++++--------------------
> 1 file changed, 23 insertions(+), 20 deletions(-)
> + priv->pdev = platform_device_register_data(&pdev->dev, mdrv, 0, NULL,
> + 0);
Would you please send a separate patch to replace "pdev->dev"?
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: fsl,audmix: remove "model" attribute
From: Nicolin Chen @ 2019-04-10 4:29 UTC (permalink / raw)
To: Viorel Suman
Cc: Mark Rutland, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
Timur Tabi, Xiubo Li, Shawn Guo, Sascha Hauer, Takashi Iwai,
Liam Girdwood, Rob Herring, Jaroslav Kysela, Viorel Suman,
Julia Lawall, Mark Brown, dl-linux-imx, Pengutronix Kernel Team,
Fabio Estevam, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554809242-27475-3-git-send-email-viorel.suman@nxp.com>
On Tue, Apr 09, 2019 at 11:27:40AM +0000, Viorel Suman wrote:
> Remove "model" attribute.
>
> Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 3/3] ASoC: imx-audmix: fix object reference leaks in probe
From: Nicolin Chen @ 2019-04-10 4:30 UTC (permalink / raw)
To: Viorel Suman
Cc: Mark Rutland, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
Timur Tabi, Xiubo Li, Shawn Guo, Sascha Hauer, Takashi Iwai,
Liam Girdwood, Rob Herring, Jaroslav Kysela, Viorel Suman,
Julia Lawall, Mark Brown, dl-linux-imx, Pengutronix Kernel Team,
Fabio Estevam, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1554809242-27475-4-git-send-email-viorel.suman@nxp.com>
On Tue, Apr 09, 2019 at 11:27:42AM +0000, Viorel Suman wrote:
> Release the reference to the underlying device taken
> by of_find_device_by_node() call.
>
> Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
> Reported-by: Julia Lawall <Julia.Lawall@lip6.fr>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH RFC 1/5] cpu/speculation: Add 'cpu_spec_mitigations=' cmdline options
From: Michael Ellerman @ 2019-04-10 5:48 UTC (permalink / raw)
To: Josh Poimboeuf, Borislav Petkov
Cc: Peter Zijlstra, Heiko Carstens, Paul Mackerras, H . Peter Anvin,
Ingo Molnar, Andrea Arcangeli, linux-s390, x86, Will Deacon,
Linus Torvalds, Catalin Marinas, Waiman Long, linux-arch,
Jon Masters, Jiri Kosina, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Greg Kroah-Hartman, linux-kernel, Tyler Hicks,
Martin Schwidefsky, linuxppc-dev
In-Reply-To: <20190405161852.nmpk22omgiety4df@treble>
Josh Poimboeuf <jpoimboe@redhat.com> writes:
> On Fri, Apr 05, 2019 at 06:01:36PM +0200, Borislav Petkov wrote:
>> Thinking about this more, we can shave off the first 4 chars and have it
>> be:
>>
>> spec_mitigations=
>>
>> I think it is painfully clear which speculation mitigations we mean. And
>> the other switches don't have "cpu_" prefixes too so...
>
> Sure, I'm ok with renaming it to that, if there are no objections.
What about when we have a mitigation for a non-speculation related bug :)
mitigations=xxx
?
cheers
^ permalink raw reply
* Re: [PATCH RFC 3/5] powerpc/speculation: Add support for 'cpu_spec_mitigations=' cmdline options
From: Michael Ellerman @ 2019-04-10 6:06 UTC (permalink / raw)
To: Josh Poimboeuf, linux-kernel
Cc: Peter Zijlstra, Heiko Carstens, Paul Mackerras, H . Peter Anvin,
Ingo Molnar, Andrea Arcangeli, linux-s390, x86, Will Deacon,
Linus Torvalds, Catalin Marinas, Waiman Long, linux-arch,
Jon Masters, Jiri Kosina, Borislav Petkov, Andy Lutomirski,
Thomas Gleixner, linux-arm-kernel, Greg Kroah-Hartman,
Tyler Hicks, Martin Schwidefsky, linuxppc-dev
In-Reply-To: <91b92d8182d2f114d92c95689fcd4bb1a8dda1b0.1554396090.git.jpoimboe@redhat.com>
Josh Poimboeuf <jpoimboe@redhat.com> writes:
> Configure powerpc CPU runtime speculation bug mitigations in accordance
> with the 'cpu_spec_mitigations=' cmdline options. This affects
> Meltdown, Spectre v1, Spectre v2, and Speculative Store Bypass.
>
> The default behavior is unchanged.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 9 +++++----
> arch/powerpc/kernel/security.c | 6 +++---
> arch/powerpc/kernel/setup_64.c | 2 +-
> 3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 29dc03971630..0e8eae1e8a25 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2552,10 +2552,11 @@
>
> off
> Disable all speculative CPU mitigations.
> - Equivalent to: nopti [x86]
> + Equivalent to: nopti [x86, powerpc]
> + nospectre_v1 [powerpc]
> nospectre_v2 [x86]
Not sure if you meant to omit powerpc from nospectre_v2?
You have patched it in the code below.
> spectre_v2_user=off [x86]
> - spec_store_bypass_disable=off [x86]
> + spec_store_bypass_disable=off [x86, powerpc]
> l1tf=off [x86]
>
> auto (default)
> @@ -2568,7 +2569,7 @@
> Equivalent to: pti=auto [x86]
> spectre_v2=auto [x86]
> spectre_v2_user=auto [x86]
> - spec_store_bypass_disable=auto [x86]
> + spec_store_bypass_disable=auto [x86, powerpc]
> l1tf=flush [x86]
>
> auto,nosmt
> @@ -2579,7 +2580,7 @@
> Equivalent to: pti=auto [x86]
> spectre_v2=auto [x86]
> spectre_v2_user=auto [x86]
> - spec_store_bypass_disable=auto [x86]
> + spec_store_bypass_disable=auto [x86, powerpc]
> l1tf=flush,nosmt [x86]
>
> mminit_loglevel=
> diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> index b33bafb8fcea..5aed4ad729ba 100644
> --- a/arch/powerpc/kernel/security.c
> +++ b/arch/powerpc/kernel/security.c
> @@ -57,7 +57,7 @@ void setup_barrier_nospec(void)
> enable = security_ftr_enabled(SEC_FTR_FAVOUR_SECURITY) &&
> security_ftr_enabled(SEC_FTR_BNDS_CHK_SPEC_BAR);
>
> - if (!no_nospec)
> + if (!no_nospec && cpu_spec_mitigations != CPU_SPEC_MITIGATIONS_OFF)
> enable_barrier_nospec(enable);
Adding a wrapper func that checks for CPU_SPEC_MITIGATIONS_OFF would
make these a little less verbose, eg:
if (!no_nospec && !cpu_spec_mitigations_off())
enable_barrier_nospec(enable);
But that's a nitpick.
> @@ -116,7 +116,7 @@ static int __init handle_nospectre_v2(char *p)
> early_param("nospectre_v2", handle_nospectre_v2);
> void setup_spectre_v2(void)
> {
> - if (no_spectrev2)
> + if (no_spectrev2 || cpu_spec_mitigations == CPU_SPEC_MITIGATIONS_OFF)
> do_btb_flush_fixups();
> else
> btb_flush_enabled = true;
> @@ -300,7 +300,7 @@ void setup_stf_barrier(void)
>
> stf_enabled_flush_types = type;
>
> - if (!no_stf_barrier)
> + if (!no_stf_barrier && cpu_spec_mitigations != CPU_SPEC_MITIGATIONS_OFF)
> stf_barrier_enable(enable);
> }
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index ba404dd9ce1d..d9d796a66a79 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -932,7 +932,7 @@ void setup_rfi_flush(enum l1d_flush_type types, bool enable)
>
> enabled_flush_types = types;
>
> - if (!no_rfi_flush)
> + if (!no_rfi_flush || cpu_spec_mitigations != CPU_SPEC_MITIGATIONS_OFF)
> rfi_flush_enable(enable);
> }
LGTM.
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute
From: Daniel Baluta @ 2019-04-10 6:20 UTC (permalink / raw)
To: Nicolin Chen
Cc: Mark Rutland, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, Fabio Estevam,
Pengutronix Kernel Team, Timur Tabi, Xiubo Li,
linux-arm-kernel@lists.infradead.org, Shawn Guo, Sascha Hauer,
Takashi Iwai, Liam Girdwood, Viorel Suman, Julia Lawall,
Rob Herring, Mark Brown, Viorel Suman,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
dl-linux-imx
In-Reply-To: <20190410042906.GC3032@Asurada>
Hi Nicolin,
On Wed, Apr 10, 2019 at 7:30 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Tue, Apr 09, 2019 at 11:27:39AM +0000, Viorel Suman wrote:
> > Use "of_device_id.data" to specify the machine driver
> > instead of "model" DTS attribute.
> >
> > Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
>
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
> > ---
> > sound/soc/fsl/fsl_audmix.c | 43 +++++++++++++++++++++++--------------------
> > 1 file changed, 23 insertions(+), 20 deletions(-)
>
> > + priv->pdev = platform_device_register_data(&pdev->dev, mdrv, 0, NULL,
> > + 0);
>
> Would you please send a separate patch to replace "pdev->dev"?
I am not sure exactly how to explain this change in the commit message. It does
make code easier to read and avoids dereferencing pdev pointer each time.
Is it enough for commit description?
thanks,
Daniel.
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute
From: Nicolin Chen @ 2019-04-10 6:37 UTC (permalink / raw)
To: Daniel Baluta
Cc: Mark Rutland, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, Fabio Estevam,
Pengutronix Kernel Team, Timur Tabi, Xiubo Li,
linux-arm-kernel@lists.infradead.org, Shawn Guo, Sascha Hauer,
Takashi Iwai, Liam Girdwood, Viorel Suman, Julia Lawall,
Rob Herring, Mark Brown, Viorel Suman,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
dl-linux-imx
In-Reply-To: <CAEnQRZBDSrpje2-ML0zr+oJkwMbbkxV5hUNc_HzZtPZv-et8mA@mail.gmail.com>
On Wed, Apr 10, 2019 at 09:20:29AM +0300, Daniel Baluta wrote:
> Hi Nicolin,
>
> On Wed, Apr 10, 2019 at 7:30 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> >
> > On Tue, Apr 09, 2019 at 11:27:39AM +0000, Viorel Suman wrote:
> > > Use "of_device_id.data" to specify the machine driver
> > > instead of "model" DTS attribute.
> > >
> > > Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
> >
> > Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
> >
> > > ---
> > > sound/soc/fsl/fsl_audmix.c | 43 +++++++++++++++++++++++--------------------
> > > 1 file changed, 23 insertions(+), 20 deletions(-)
> >
> > > + priv->pdev = platform_device_register_data(&pdev->dev, mdrv, 0, NULL,
> > > + 0);
> >
> > Would you please send a separate patch to replace "pdev->dev"?
>
> I am not sure exactly how to explain this change in the commit message. It does
> make code easier to read and avoids dereferencing pdev pointer each time.
>
> Is it enough for commit description?
You mean this? https://lore.kernel.org/patchwork/patch/862610/
^ permalink raw reply
* [PATCH kernel] powerpc/powernv/ioda: Handle failures correctly in pnv_pci_ioda_iommu_bypass_supported
From: Alexey Kardashevskiy @ 2019-04-10 6:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Christoph Hellwig
When the return value type was changed from int to bool, few places were
left unchanged, this fixes them. We did not hit these failures as
the first one is not happening at all and the second one is little more
likely to happen if the user switches a 33..58bit DMA capable device
between the VFIO and vendor drivers and there are not so many of these.
Fixes: 2d6ad41b2c21 ("powerpc/powernv: use the generic iommu bypass code", 2019-02-13)
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 0565d8b8b33d..dcfb5469678e 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1836,7 +1836,7 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
struct pnv_ioda_pe *pe;
if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
- return -ENODEV;
+ return false;
pe = &phb->ioda.pe_array[pdn->pe_number];
if (pe->tce_bypass_enabled) {
@@ -1859,7 +1859,7 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
/* Configure the bypass mode */
s64 rc = pnv_pci_ioda_dma_64bit_bypass(pe);
if (rc)
- return rc;
+ return false;
/* 4GB offset bypasses 32-bit space */
pdev->dev.archdata.dma_offset = (1ULL << 32);
return true;
--
2.17.1
^ permalink raw reply related
* Re: [alsa-devel] [PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute
From: Daniel Baluta @ 2019-04-10 6:48 UTC (permalink / raw)
To: Nicolin Chen
Cc: Mark Rutland, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, Fabio Estevam,
Pengutronix Kernel Team, Timur Tabi, Xiubo Li,
linux-arm-kernel@lists.infradead.org, Shawn Guo, Sascha Hauer,
Takashi Iwai, Liam Girdwood, Viorel Suman, Julia Lawall,
Rob Herring, Mark Brown, Viorel Suman,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
dl-linux-imx
In-Reply-To: <20190410063714.GA4156@Asurada>
On Wed, Apr 10, 2019 at 9:37 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Wed, Apr 10, 2019 at 09:20:29AM +0300, Daniel Baluta wrote:
> > Hi Nicolin,
> >
> > On Wed, Apr 10, 2019 at 7:30 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> > >
> > > On Tue, Apr 09, 2019 at 11:27:39AM +0000, Viorel Suman wrote:
> > > > Use "of_device_id.data" to specify the machine driver
> > > > instead of "model" DTS attribute.
> > > >
> > > > Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
> > >
> > > Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
> > >
> > > > ---
> > > > sound/soc/fsl/fsl_audmix.c | 43 +++++++++++++++++++++++--------------------
> > > > 1 file changed, 23 insertions(+), 20 deletions(-)
> > >
> > > > + priv->pdev = platform_device_register_data(&pdev->dev, mdrv, 0, NULL,
> > > > + 0);
> > >
> > > Would you please send a separate patch to replace "pdev->dev"?
> >
> > I am not sure exactly how to explain this change in the commit message. It does
> > make code easier to read and avoids dereferencing pdev pointer each time.
> >
> > Is it enough for commit description?
>
> You mean this? https://lore.kernel.org/patchwork/patch/862610/
Yes! Thanks.
^ permalink raw reply
* Re: [PATCH] Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE [BZ #10339]
From: Florian Weimer @ 2019-04-10 6:50 UTC (permalink / raw)
To: hpa; +Cc: linux-api, libc-alpha, linuxppc-dev, adhemerval.zanella
In-Reply-To: <EF5CE34B-1A82-4391-B07E-DB832814AD1C@zytor.com>
* hpa:
> PowerPC doesn't need it at all.
Because the definition would be the same as struct termios?
I think we should still define struct termios2 using the struct termios
definition, and also define TCGETS2 and TCSETS2. This way, applications
can use the struct termios2 type without affecting portability to POWER.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH v2] powerpc/xmon: add read-only mode
From: Oliver @ 2019-04-10 6:51 UTC (permalink / raw)
To: Christopher M Riedl; +Cc: linuxppc-dev, Andrew Donnellan
In-Reply-To: <864072306.101924.1554863718473@privateemail.com>
On Wed, Apr 10, 2019 at 12:35 PM Christopher M Riedl <cmr@informatik.wtf> wrote:
>
>
> > On April 8, 2019 at 1:34 AM Oliver <oohall@gmail.com> wrote:
> >
> >
> > On Mon, Apr 8, 2019 at 1:06 PM Christopher M. Riedl <cmr@informatik.wtf> wrote:
> > >
> > > Operations which write to memory and special purpose registers should be
> > > restricted on systems with integrity guarantees (such as Secure Boot)
> > > and, optionally, to avoid self-destructive behaviors.
> > >
> > > Add a config option, XMON_RW, to control default xmon behavior along
> > > with kernel cmdline options xmon=ro and xmon=rw for explicit control.
> > > Use XMON_RW instead of XMON in the condition to set PAGE_KERNEL_TEXT to
> > > allow xmon in read-only mode alongside write-protected kernel text.
> > > XMON_RW defaults to !STRICT_KERNEL_RWX.
> > >
> > > The following xmon operations are affected:
> > > memops:
> > > disable memmove
> > > disable memset
> > > disable memzcan
> > > memex:
> > > no-op'd mwrite
> > > super_regs:
> > > no-op'd write_spr
> > > bpt_cmds:
> > > disable
> > > proc_call:
> > > disable
> > >
> > > Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
> > > ---
> > > v1->v2:
> > > Use bool type for xmon_is_ro flag
> > > Replace XMON_RO with XMON_RW config option
> > > Make XMON_RW dependent on STRICT_KERNEL_RWX
> > Do you mean make it dependent on XMON?
> >
>
> Yeah that's really not clear at all -- XMON_RW is set based on the value of
> STRICT_KERNEL_RWX.
>
> >
> > > Use XMON_RW to control PAGE_KERNEL_TEXT
> > > Add printf in xmon read-only mode when dropping/skipping writes
> > > Disable memzcan (zero-fill memop) in xmon read-only mode
> > >
> > > arch/powerpc/Kconfig.debug | 10 +++++
> > > arch/powerpc/include/asm/book3s/32/pgtable.h | 5 ++-
> > > arch/powerpc/include/asm/book3s/64/pgtable.h | 5 ++-
> > > arch/powerpc/include/asm/nohash/pgtable.h | 5 ++-
> > > arch/powerpc/xmon/xmon.c | 42 ++++++++++++++++++++
> > > 5 files changed, 61 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> > > index 4e00cb0a5464..0c7f21476018 100644
> > > --- a/arch/powerpc/Kconfig.debug
> > > +++ b/arch/powerpc/Kconfig.debug
> > > @@ -117,6 +117,16 @@ config XMON_DISASSEMBLY
> > > to say Y here, unless you're building for a memory-constrained
> > > system.
> > >
> >
> > > +config XMON_RW
> > > + bool "Allow xmon read and write operations"
> > > + depends on XMON
> > > + default !STRICT_KERNEL_RWX
> > > + help
> > > + Allow xmon to read and write to memory and special-purpose registers.
> > > + Conversely, prevent xmon write access when set to N. Read and write
> > > + access can also be explicitly controlled with 'xmon=rw' or 'xmon=ro'
> > > + (read-only) cmdline options. Default is !STRICT_KERNEL_RWX.
> >
> > Maybe I am a dumb, but I found this *extremely* confusing.
> > Conventionally Kconfig options will control what code is and is not
> > included in the kernel (see XMON_DISASSEMBLY) rather than changing the
> > default behaviour of code. It's not wrong to do so and I'm going to
> > assume that you were following the pattern of XMON_DEFAULT, but I
> > think you need to be a little more clear about what option actually
> > does. Renaming it to XMON_DEFAULT_RO_MODE and re-wording the
> > description to indicate it's a only a mode change would help a lot.
> >
> > Sorry if this comes across as pointless bikeshedding since it's the
> > opposite of what Christophe said in the last patch, but this was a bit
> > of a head scratcher.
> >
>
> If anyone is dumb here it's me for making this confusing :)
> I chatted with Michael Ellerman about this, so let me try to explain this more clearly.
>
> There are two things I am trying to address with XMON_RW:
> 1) provide a default access mode for xmon based on system "security"
> 2) replace XMON in the decision to write-protect kernel text at compile-time
>
> I think a single Kconfig for both of those things is sensible as ultimately the
> point is to allow xmon to operate in read-only mode on "secure" systems -- without
> violating any integrity/security guarantees (such as write-protected kernel text).
>
> Christophe suggested looking at STRICT_KERNEL_RWX and I think that option makes the
> most sense to base XMON_RW on since the description for STRICT_KERNEL_RWX states:
>
> > If this is set, kernel text and rodata memory will be made read-only,
> > and non-text memory will be made non-executable. This provides
> > protection against certain security exploits (e.g. executing the heap
> > or modifying text)
> >
> > These features are considered standard security practice these days.
> > You should say Y here in almost all cases.
>
> Considering this, does XMON_DEFAULT_RO_MODE really make things more clear?
Not really.
> With that said, I will remove the 'xmon=rw' cmdline option as it really doesn't work
> since kernel text is write-protected at compile time.
I think you're overestimating the importance of being able to write to
the kernel text. The only features that require a writeable text are
using the mem commands to modify the kernel text (a bad idea) and
software breakpoints.
Funnily enough, enabling STRICT_RWX make software breakpoints work
again since it enables a smarter implementation of patch_instruction()
that uses a temporary RW mapping to do the patch. Considering that
kprobes and dynamic ftrace are also implemented with
patch_instruction() too we could just default to the newer
implementation and force PAGE_KERNEL_TEXT to be PAGE_KERNEL_ROX all
the time.
> 'xmon=ro' remains for those who would optionally like to prevent shooting themselves
> in the foot/feet while using xmon.
>
> Hopefully this makes a bit more sense now?
>
> > > config DEBUGGER
> > > bool
> > > depends on KGDB || XMON
> > > diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
> > > index aa8406b8f7ba..615144ad667d 100644
> > > --- a/arch/powerpc/include/asm/book3s/32/pgtable.h
> > > +++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
> > > @@ -86,8 +86,9 @@ static inline bool pte_user(pte_t pte)
> > > * set breakpoints anywhere, so don't write protect the kernel text
> > > * on platforms where such control is possible.
> > > */
> > > -#if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || defined(CONFIG_BDI_SWITCH) ||\
> > > - defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE)
> > > +#if defined(CONFIG_KGDB) || defined(CONFIG_XMON_RW) || \
> > > + defined(CONFIG_BDI_SWITCH) || defined(CONFIG_KPROBES) || \
> > > + defined(CONFIG_DYNAMIC_FTRACE)
> > > #define PAGE_KERNEL_TEXT PAGE_KERNEL_X
> > > #else
> > > #define PAGE_KERNEL_TEXT PAGE_KERNEL_ROX
> > > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> > > index 581f91be9dd4..bc4655122f6b 100644
> > > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> > > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> > > @@ -168,8 +168,9 @@
> > > * set breakpoints anywhere, so don't write protect the kernel text
> > > * on platforms where such control is possible.
> > > */
> > > -#if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || defined(CONFIG_BDI_SWITCH) || \
> > > - defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE)
> > > +#if defined(CONFIG_KGDB) || defined(CONFIG_XMON_RW) || \
> > > + defined(CONFIG_BDI_SWITCH) || defined(CONFIG_KPROBES) || \
> > > + defined(CONFIG_DYNAMIC_FTRACE)
> > > #define PAGE_KERNEL_TEXT PAGE_KERNEL_X
> > > #else
> > > #define PAGE_KERNEL_TEXT PAGE_KERNEL_ROX
> > > diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
> > > index 1ca1c1864b32..c052931bd243 100644
> > > --- a/arch/powerpc/include/asm/nohash/pgtable.h
> > > +++ b/arch/powerpc/include/asm/nohash/pgtable.h
> > > @@ -22,8 +22,9 @@
> > > * set breakpoints anywhere, so don't write protect the kernel text
> > > * on platforms where such control is possible.
> > > */
> > > -#if defined(CONFIG_KGDB) || defined(CONFIG_XMON) || defined(CONFIG_BDI_SWITCH) ||\
> > > - defined(CONFIG_KPROBES) || defined(CONFIG_DYNAMIC_FTRACE)
> > > +#if defined(CONFIG_KGDB) || defined(CONFIG_XMON_RW) || \
> > > + defined(CONFIG_BDI_SWITCH) || defined(CONFIG_KPROBES) || \
> > > + defined(CONFIG_DYNAMIC_FTRACE)
> > > #define PAGE_KERNEL_TEXT PAGE_KERNEL_X
> > > #else
> > > #define PAGE_KERNEL_TEXT PAGE_KERNEL_ROX
> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > index a0f44f992360..224ca0b3506b 100644
> > > --- a/arch/powerpc/xmon/xmon.c
> > > +++ b/arch/powerpc/xmon/xmon.c
> > > @@ -80,6 +80,7 @@ static int set_indicator_token = RTAS_UNKNOWN_SERVICE;
> > > #endif
> > > static unsigned long in_xmon __read_mostly = 0;
> > > static int xmon_on = IS_ENABLED(CONFIG_XMON_DEFAULT);
> > > +static bool xmon_is_ro = !IS_ENABLED(CONFIG_XMON_RW);
> > >
> > > static unsigned long adrs;
> > > static int size = 1;
> > > @@ -202,6 +203,8 @@ static void dump_tlb_book3e(void);
> > > #define GETWORD(v) (((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
> > > #endif
> > >
> > > +static const char *xmon_is_ro_warn = "xmon read-only mode: skipping write\n";
> > > +
> > > static char *help_string = "\
> > > Commands:\n\
> > > b show breakpoints\n\
> > > @@ -989,6 +992,10 @@ cmds(struct pt_regs *excp)
> > > memlocate();
> > > break;
> > > case 'z':
> > > + if (xmon_is_ro) {
> > > + printf(xmon_is_ro_warn);
> > > + break;
> > > + }
> > > memzcan();
> > > break;
> > > case 'i':
> > > @@ -1042,6 +1049,10 @@ cmds(struct pt_regs *excp)
> > > set_lpp_cmd();
> > > break;
> > > case 'b':
> > > + if (xmon_is_ro) {
> > > + printf(xmon_is_ro_warn);
> > > + break;
> > > + }
> > > bpt_cmds();
> > > break;
> > > case 'C':
> > > @@ -1055,6 +1066,10 @@ cmds(struct pt_regs *excp)
> > > bootcmds();
> > > break;
> > > case 'p':
> > > + if (xmon_is_ro) {
> > > + printf(xmon_is_ro_warn);
> > > + break;
> > > + }
> > > proccall();
> > > break;
> > > case 'P':
> > > @@ -1777,6 +1792,11 @@ read_spr(int n, unsigned long *vp)
> > > static void
> > > write_spr(int n, unsigned long val)
> > > {
> > > + if (xmon_is_ro) {
> > > + printf(xmon_is_ro_warn);
> > > + return;
> > > + }
> > > +
> > > if (setjmp(bus_error_jmp) == 0) {
> > > catch_spr_faults = 1;
> > > sync();
> > > @@ -2016,6 +2036,12 @@ mwrite(unsigned long adrs, void *buf, int size)
> > > char *p, *q;
> > >
> > > n = 0;
> > > +
> > > + if (xmon_is_ro) {
> > > + printf(xmon_is_ro_warn);
> > > + return n;
> > > + }
> > > +
> > > if (setjmp(bus_error_jmp) == 0) {
> > > catch_memory_errors = 1;
> > > sync();
> > > @@ -2884,9 +2910,17 @@ memops(int cmd)
> > > scanhex((void *)&mcount);
> > > switch( cmd ){
> > > case 'm':
> > > + if (xmon_is_ro) {
> > > + printf(xmon_is_ro_warn);
> > > + break;
> > > + }
> > > memmove((void *)mdest, (void *)msrc, mcount);
> > > break;
> > > case 's':
> > > + if (xmon_is_ro) {
> > > + printf(xmon_is_ro_warn);
> > > + break;
> > > + }
> > > memset((void *)mdest, mval, mcount);
> > > break;
> > > case 'd':
> > > @@ -3796,6 +3830,14 @@ static int __init early_parse_xmon(char *p)
> > > } else if (strncmp(p, "on", 2) == 0) {
> > > xmon_init(1);
> > > xmon_on = 1;
> > > + } else if (strncmp(p, "rw", 2) == 0) {
> > > + xmon_init(1);
> > > + xmon_on = 1;
> > > + xmon_is_ro = false;
> > > + } else if (strncmp(p, "ro", 2) == 0) {
> > > + xmon_init(1);
> > > + xmon_on = 1;
> > > + xmon_is_ro = true;
> > > } else if (strncmp(p, "off", 3) == 0)
> > > xmon_on = 0;
> > > else
> > > --
> > > 2.21.0
> > >
^ permalink raw reply
* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Eric Biggers @ 2019-04-10 7:02 UTC (permalink / raw)
To: Daniel Axtens
Cc: leo.barbosa, Herbert Xu, Stephan Mueller, nayna, omosnacek,
leitao, pfsmorigo, linux-crypto, marcelo.cerri, linuxppc-dev
In-Reply-To: <8736nou2x5.fsf@dja-thinkpad.axtens.net>
Hi Daniel,
On Fri, Mar 15, 2019 at 04:23:02PM +1100, Daniel Axtens wrote:
> Eric Biggers <ebiggers@kernel.org> writes:
>
> > Hi Daniel,
> >
> > On Fri, Mar 15, 2019 at 03:24:35PM +1100, Daniel Axtens wrote:
> >> Hi Eric,
> >>
> >> >> The original assembly imported from OpenSSL has two copy-paste
> >> >> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
> >> >> the code branches to the CBC decryption exit path, rather than to
> >> >> the CTR exit path.
> >> >
> >> > So does this need to be fixed in OpenSSL too?
> >>
> >> Yes, I'm getting in touch with some people internally (at IBM) about
> >> doing that.
> >>
> >> >> This leads to corruption of the IV, which leads to subsequent blocks
> >> >> being corrupted.
> >> >>
> >> >> This can be detected with libkcapi test suite, which is available at
> >> >> https://github.com/smuellerDD/libkcapi
> >> >>
> >> >
> >> > Is this also detected by the kernel's crypto self-tests, and if not why not?
> >> > What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?
> >>
> >> It seems the self-tests do not catch it. To catch it, there has to be a
> >> test where the blkcipher_walk creates a walk.nbytes such that
> >> [(the number of AES blocks) mod 8] is either 2 or 3. This happens with
> >> AF_ALG pretty frequently, but when I booted with self-tests it only hit
> >> 1, 4, 5, 6 and 7 - it missed 0, 2 and 3.
> >>
> >> I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in
> >> -next?
> >>
> >> Regards,
> >> Daniel
> >
> > The improvements I recently made to the self-tests are intended to catch exactly
> > this sort of bug. They were just merged for v5.1, so try the latest mainline.
> > This almost certainly would be caught by EXTRA_TESTS (and if not I'd want to
> > know), but it may be caught by the regular self-tests now too.
>
> Well, even the patched code fails with the new self-tests, so clearly
> they're catching something! I'll investigate in more detail next week.
>
> Regards,
> Daniel
>
> >
> > - Eric
Are you still planning to fix the remaining bug? I booted a ppc64le VM, and I
see the same test failure (I think) you were referring to:
alg: skcipher: p8_aes_ctr encryption test failed (wrong result) on test vector 3, cfg="uneven misaligned splits, may sleep"
- Eric
^ permalink raw reply
* RE: [EXT] Re: [PATCH] ASoC: fsl_asrc: replace the process_option table with function
From: S.j. Wang @ 2019-04-10 7:22 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com, broonie@kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20190410042638.GB3032@Asurada>
Hi
>
> On Wed, Apr 10, 2019 at 03:15:26AM +0000, S.j. Wang wrote:
> > The table is not flexible if supported sample rate is not in the
> > table, so use a function to replace it.
>
> Could you please elaborate a bit the special use case here?
>
> The table was copied directly from the Reference Manual. We also have
> listed all supported input and output sample rates just right behind that table.
> If there're missing rates, we probably should update those two lists also?
> Otherwise, how could we have a driver limiting both I/O sample rates while
> we still see something not in the table?
>
Yes, I plan to send another patch to update the in/out rate list. Do I need
To merge that to this commit? Actually we want to support 12k and 24kHz
> > +static int proc_autosel(int Fsin, int Fsout, int *pre_proc, int
> > +*post_proc)
>
> Please add some comments to this function to explain what it does, and how
> it works. And better to rename it to something like "fsl_asrc_sel_proc".
>
Yes, some comments should be added, but not so detail, because this function
Is get from the design team, but the owner has left.
> > +{
> > + bool det_out_op2_cond;
> > + bool det_out_op0_cond;
> > +
> > + det_out_op2_cond = (((Fsin * 15 > Fsout * 16) & (Fsout < 56000)) |
> > + ((Fsin > 56000) & (Fsout < 56000)));
> > + det_out_op0_cond = (Fsin * 23 < Fsout * 8);
>
> "detect output option condition"? Please explain a bit or add comments to
> explain.
>
> > +
> > + /*
> > + * Not supported case: Tsout>16.125*Tsin, and Tsout>8.125*Tsin.
>
> Could be "unsupported". And it should fit within one line:
> /* Unsupported case: Tsout > 16.125 * Tsin, and Tsout > 8.125 * Tsin */
>
> > + */
> > + if (Fsin * 8 > 129 * Fsout)
> > + *pre_proc = 5;
> > + else if (Fsin * 8 > 65 * Fsout)
> > + *pre_proc = 4;
> > + else if (Fsin * 8 > 33 * Fsout)
> > + *pre_proc = 2;
> > + else if (Fsin * 8 > 15 * Fsout) {
> > + if (Fsin > 152000)
> > + *pre_proc = 2;
> > + else
> > + *pre_proc = 1;
> > + } else if (Fsin < 76000)
> > + *pre_proc = 0;
> > + else if (Fsin > 152000)
> > + *pre_proc = 2;
> > + else
> > + *pre_proc = 1;
> > +
> > + if (det_out_op2_cond)
> > + *post_proc = 2;
> > + else if (det_out_op0_cond)
> > + *post_proc = 0;
> > + else
> > + *post_proc = 1;
> > +
> > + if (*pre_proc == 4 || *pre_proc == 5)
> > + return -EINVAL;
>
> I think you'd better add some necessary comments here too.
>
> > @@ -377,11 +404,17 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
> *pair)
> > ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index),
> > ASRCTR_IDR(index) | ASRCTR_USR(index));
> >
> > + ret = proc_autosel(inrate, outrate, &pre_proc, &post_proc);
> > + if (ret) {
> > + pair_err("No supported pre-processing options\n");
> > + return ret;
> > + }
>
> I think we should do this earlier in this function, once We know the inrate
> and outrate, instead of having all register being configured then going for an
> error-out.
Ok.
>
> Another thing confuses me: so we could have supported sample rates in the
> list but the hardware might not support some of them because we couldn't
> calculate their processing options?
No, just want to support 12k, 24KHz, or others as customer like.
^ permalink raw reply
* Re: [PATCH net] vhost: flush dcache page when logging dirty pages
From: Jason Wang @ 2019-04-10 7:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Andrea Arcangeli, James Bottomley, kvm, netdev, linux-kernel,
Ralf Baechle, open list:VIRTIO GPU DRIVER, Christoph Hellwig,
Linux-MM, Paul Burton, Paul Mackerras, James Hogan, linux,
linux-mips, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20190409085607-mutt-send-email-mst@kernel.org>
On 2019/4/9 下午9:14, Michael S. Tsirkin wrote:
> On Tue, Apr 09, 2019 at 12:16:47PM +0800, Jason Wang wrote:
>> We set dirty bit through setting up kmaps and access them through
>> kernel virtual address, this may result alias in virtually tagged
>> caches that require a dcache flush afterwards.
>>
>> Cc: Christoph Hellwig <hch@infradead.org>
>> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>> Fixes: 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server")
> This is like saying "everyone with vhost needs this".
> In practice only might affect some architectures.
For the archs that does need dcache flushing, the function is just a nop.
> Which ones?
There're more than 10 archs that have ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
defined, just cc some maintainers of some more influenced ones.
> You want to Cc the relevant maintainers
> who understand this...
>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> I am not sure this is a good idea.
> The region in question is supposed to be accessed
> by userspace at the same time, through atomic operations.
>
> How do we know userspace didn't access it just before?
get_user_pages() will do both flush_annon_page() to make sure the
userspace write is visible to kernel.
>
> Is that an issue at all given we use
> atomics for access? Documentation/core-api/cachetlb.rst does
> not mention atomics.
> Which architectures are affected?
> Assuming atomics actually do need a flush, then don't we need
> a flush in the other direction too? How are atomics
> supposed to work at all?
It's the issue of visibility, atomic operation is just one of the
possible operations. If we can finally makes the write visible to each
other, there will be no issue.
It looks to me we could still end up alias if userspace is accessing the
dirty log between get_user_pages_fast() and flush_dcache_page(). But the
flush_dcache_page() can guarantee what kernel wrote is visible to
userspace finally though some bits cleared by userspace might still
there. We may end up with more dirty pages noticed by userspace which
should be harmless.
>
>
> I really think we need new APIs along the lines of
> set_bit_to_user.
Can we simply do:
get_user()
set bit
put_user()
instead?
>
>> ---
>> drivers/vhost/vhost.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 351af88231ad..34a1cedbc5ba 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -1711,6 +1711,7 @@ static int set_bit_to_user(int nr, void __user *addr)
>> base = kmap_atomic(page);
>> set_bit(bit, base);
>> kunmap_atomic(base);
>> + flush_dcache_page(page);
>> set_page_dirty_lock(page);
>> put_page(page);
>> return 0;
> Ignoring the question of whether this actually helps, I doubt
> flush_dcache_page is appropriate here. Pls take a look at
> Documentation/core-api/cachetlb.rst as well as the actual
> implementation.
>
> I think you meant flush_kernel_dcache_page, and IIUC it must happen
> before kunmap, not after (which you still have the va locked).
Looks like you're right.
Thanks
>
>> --
>> 2.19.1
^ permalink raw reply
* [PATCH] powerpc/pseries: Provide verbose info about HPT resizing attempt
From: Bharata B Rao @ 2019-04-10 7:55 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Bharata B Rao, aneesh.kumar, david
When HPT resizing is attempted in response to memory hotplug, we see
the following messages from the kernel:
lpar: Attempting to resize HPT to shift 23
Unable to resize hash page table to target order 23: -28
This gives a feeling as though we are trying to grow HPT but failed and
hence bad things might happen in future. Improve the message a bit
by explicitly printing the existing HPT shift value in addtion to
the newly targeted value so that it is clear that we haven't failed
to grow HPT. After this commit, the same message will appear like this:
lpar: Attempting to resize HPT from shift 25 to 23
Unable to resize hash page table to target order 23: -28
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/platforms/pseries/lpar.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index f2a9f0adc2d3..ecc7fa1876a9 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -875,7 +875,8 @@ static int pseries_lpar_resize_hpt(unsigned long shift)
if (!firmware_has_feature(FW_FEATURE_HPT_RESIZE))
return -ENODEV;
- pr_info("Attempting to resize HPT to shift %lu\n", shift);
+ pr_info("Attempting to resize HPT from shift %llu to %lu\n", ppc64_pft_size,
+ shift);
t0 = ktime_get();
--
2.17.1
^ permalink raw reply related
* Re: [EXT] Re: [PATCH] ASoC: fsl_asrc: replace the process_option table with function
From: Nicolin Chen @ 2019-04-10 8:00 UTC (permalink / raw)
To: S.j. Wang
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com, broonie@kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <VE1PR04MB64798A5ADF025047AF5B9076E32E0@VE1PR04MB6479.eurprd04.prod.outlook.com>
On Wed, Apr 10, 2019 at 07:22:31AM +0000, S.j. Wang wrote:
> > The table was copied directly from the Reference Manual. We also have
> > listed all supported input and output sample rates just right behind that table.
> > If there're missing rates, we probably should update those two lists also?
> > Otherwise, how could we have a driver limiting both I/O sample rates while
> > we still see something not in the table?
> >
>
> Yes, I plan to send another patch to update the in/out rate list. Do I need
> To merge that to this commit? Actually we want to support 12k and 24KHz
Please send separate patches but in one series. And a question:
Is it possible to update the table? It'd be way quicker to use
lookup table than real-time calculation all the time. I believe
you can simply calculate all the values out for 12KHz and 24KHz
since you have the function. If there are certain combinations
of these two not being supported, then we could mark it with a
special value and add an if-check to error out.
> > > +static int proc_autosel(int Fsin, int Fsout, int *pre_proc, int
> > > +*post_proc)
> >
> > Please add some comments to this function to explain what it does, and how
> > it works. And better to rename it to something like "fsl_asrc_sel_proc".
> >
> Yes, some comments should be added, but not so detail, because this function
As much comments as possible.
> Is get from the design team, but the owner has left.
OK...that's sad...
> > Another thing confuses me: so we could have supported sample rates in the
> > list but the hardware might not support some of them because we couldn't
> > calculate their processing options?
>
> No, just want to support 12k, 24KHz, or others as customer like.
I was confused because the I/O rate lists not getting updated.
It makes sense now if you are abort to update them.
^ permalink raw reply
* RE: [EXT] Re: [PATCH] ASoC: fsl_asrc: replace the process_option table with function
From: S.j. Wang @ 2019-04-10 8:26 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com, broonie@kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20190410080052.GA5180@Asurada>
> -----Original Message-----
> From: Nicolin Chen <nicoleotsuka@gmail.com>
> Sent: Wednesday, April 10, 2019 4:01 PM
> To: S.j. Wang <shengjiu.wang@nxp.com>
> Cc: timur@kernel.org; Xiubo.Lee@gmail.com; festevam@gmail.com;
> broonie@kernel.org; alsa-devel@alsa-project.org; linuxppc-
> dev@lists.ozlabs.org
> Subject: Re: [EXT] Re: [PATCH] ASoC: fsl_asrc: replace the process_option
> table with function
>
> WARNING: This email was created outside of NXP. DO NOT CLICK links or
> attachments unless you recognize the sender and know the content is safe.
>
>
>
> On Wed, Apr 10, 2019 at 07:22:31AM +0000, S.j. Wang wrote:
> > > The table was copied directly from the Reference Manual. We also
> > > have listed all supported input and output sample rates just right behind
> that table.
> > > If there're missing rates, we probably should update those two lists also?
> > > Otherwise, how could we have a driver limiting both I/O sample rates
> > > while we still see something not in the table?
> > >
> >
> > Yes, I plan to send another patch to update the in/out rate list. Do
> > I need To merge that to this commit? Actually we want to support 12k
> > and 24KHz
>
> Please send separate patches but in one series. And a question:
>
> Is it possible to update the table? It'd be way quicker to use lookup table
> than real-time calculation all the time. I believe you can simply calculate all
> the values out for 12KHz and 24KHz since you have the function. If there are
> certain combinations of these two not being supported, then we could mark
> it with a special value and add an if-check to error out.
>
Yes, but I think the function should be more flexible, if someday we need to support
Other sample rate, only need to update the list.
> > > > +static int proc_autosel(int Fsin, int Fsout, int *pre_proc, int
> > > > +*post_proc)
> > >
> > > Please add some comments to this function to explain what it does,
> > > and how it works. And better to rename it to something like
> "fsl_asrc_sel_proc".
> > >
> > Yes, some comments should be added, but not so detail, because this
> > function
>
> As much comments as possible.
>
> > Is get from the design team, but the owner has left.
>
> OK...that's sad...
>
> > > Another thing confuses me: so we could have supported sample rates
> > > in the list but the hardware might not support some of them because
> > > we couldn't calculate their processing options?
> >
> > No, just want to support 12k, 24KHz, or others as customer like.
>
> I was confused because the I/O rate lists not getting updated.
> It makes sense now if you are abort to update them.
^ permalink raw reply
* Re: [PATCH RFC 1/5] cpu/speculation: Add 'cpu_spec_mitigations=' cmdline options
From: Borislav Petkov @ 2019-04-10 8:30 UTC (permalink / raw)
To: Michael Ellerman
Cc: Peter Zijlstra, Heiko Carstens, Paul Mackerras, H . Peter Anvin,
Ingo Molnar, Andrea Arcangeli, linux-s390, x86, Will Deacon,
Linus Torvalds, Catalin Marinas, Waiman Long, linux-arch,
Jon Masters, Jiri Kosina, Andy Lutomirski, Josh Poimboeuf,
Thomas Gleixner, linux-arm-kernel, Greg Kroah-Hartman,
linux-kernel, Tyler Hicks, Martin Schwidefsky, linuxppc-dev
In-Reply-To: <87sguqwgu7.fsf@concordia.ellerman.id.au>
On Wed, Apr 10, 2019 at 03:48:48PM +1000, Michael Ellerman wrote:
> What about when we have a mitigation for a non-speculation related bug :)
Like that is *ever* going to happen... :-P
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [PATCH-tip 00/22] locking/rwsem: Rework rwsem-xadd & enable new rwsem features
From: huang ying @ 2019-04-10 8:15 UTC (permalink / raw)
To: Waiman Long
Cc: linux-ia64, Linux-sh list, Peter Zijlstra, Will Deacon,
H. Peter Anvin, sparclinux, linux-arch, Davidlohr Bueso,
Chen Rong, linux-hexagon, the arch/x86 maintainers,
IngoMolnar@shao2-debian, linux-xtensa, Arnd Bergmann,
linuxppc-dev, Borislav Petkov, Thomas Gleixner,
linux-alpha@vger.kernel.org, Linus Torvalds,
Linux List Kernel Mailing, Andrew Morton, Tim Chen
In-Reply-To: <CAHk-=wiF2qv4TdKOTaTYeMnmsn4h6z9ot+MRj1=s7dm5y1FUwQ@mail.gmail.com>
Hi, Waiman,
What's the status of this patchset? And its merging plan?
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Provide verbose info about HPT resizing attempt
From: Michael Ellerman @ 2019-04-10 10:20 UTC (permalink / raw)
To: Bharata B Rao, linuxppc-dev
Cc: Laurent Vivier, david, aneesh.kumar, Bharata B Rao
In-Reply-To: <20190410075555.9357-1-bharata@linux.ibm.com>
Bharata B Rao <bharata@linux.ibm.com> writes:
> When HPT resizing is attempted in response to memory hotplug, we see
> the following messages from the kernel:
>
> lpar: Attempting to resize HPT to shift 23
> Unable to resize hash page table to target order 23: -28
>
> This gives a feeling as though we are trying to grow HPT but failed and
> hence bad things might happen in future. Improve the message a bit
> by explicitly printing the existing HPT shift value in addtion to
> the newly targeted value so that it is clear that we haven't failed
> to grow HPT. After this commit, the same message will appear like this:
>
> lpar: Attempting to resize HPT from shift 25 to 23
> Unable to resize hash page table to target order 23: -28
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
I'd be inclined to just remove the "Attempting .." message.
But I have this patch from Laurent queued, which already reworks things:
https://patchwork.ozlabs.org/patch/1055996/
Does that address your problem?
cheers
> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
> index f2a9f0adc2d3..ecc7fa1876a9 100644
> --- a/arch/powerpc/platforms/pseries/lpar.c
> +++ b/arch/powerpc/platforms/pseries/lpar.c
> @@ -875,7 +875,8 @@ static int pseries_lpar_resize_hpt(unsigned long shift)
> if (!firmware_has_feature(FW_FEATURE_HPT_RESIZE))
> return -ENODEV;
>
> - pr_info("Attempting to resize HPT to shift %lu\n", shift);
> + pr_info("Attempting to resize HPT from shift %llu to %lu\n", ppc64_pft_size,
> + shift);
>
> t0 = ktime_get();
>
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Provide verbose info about HPT resizing attempt
From: Bharata B Rao @ 2019-04-10 9:49 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Laurent Vivier, linuxppc-dev, aneesh.kumar, david
In-Reply-To: <87h8b66u0q.fsf@concordia.ellerman.id.au>
On Wed, Apr 10, 2019 at 08:20:53PM +1000, Michael Ellerman wrote:
> Bharata B Rao <bharata@linux.ibm.com> writes:
>
> > When HPT resizing is attempted in response to memory hotplug, we see
> > the following messages from the kernel:
> >
> > lpar: Attempting to resize HPT to shift 23
> > Unable to resize hash page table to target order 23: -28
> >
> > This gives a feeling as though we are trying to grow HPT but failed and
> > hence bad things might happen in future. Improve the message a bit
> > by explicitly printing the existing HPT shift value in addtion to
> > the newly targeted value so that it is clear that we haven't failed
> > to grow HPT. After this commit, the same message will appear like this:
> >
> > lpar: Attempting to resize HPT from shift 25 to 23
> > Unable to resize hash page table to target order 23: -28
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
>
> I'd be inclined to just remove the "Attempting .." message.
>
> But I have this patch from Laurent queued, which already reworks things:
>
> https://patchwork.ozlabs.org/patch/1055996/
>
>
> Does that address your problem?
Yes, I should have seen this earlier.
Regards,
Bharata.
^ permalink raw reply
* Re: [EXT] Re: [PATCH] ASoC: fsl_asrc: replace the process_option table with function
From: Nicolin Chen @ 2019-04-10 9:51 UTC (permalink / raw)
To: S.j. Wang
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com, broonie@kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <VE1PR04MB6479C577E9C2C58E624C5611E32E0@VE1PR04MB6479.eurprd04.prod.outlook.com>
On Wed, Apr 10, 2019 at 08:26:59AM +0000, S.j. Wang wrote:
> > Is it possible to update the table? It'd be way quicker to use lookup table
> > than real-time calculation all the time. I believe you can simply calculate all
> > the values out for 12KHz and 24KHz since you have the function. If there are
> > certain combinations of these two not being supported, then we could mark
> > it with a special value and add an if-check to error out.
> >
>
> Yes, but I think the function should be more flexible, if someday we need to support
> Other sample rate, only need to update the list.
Given the fact that the owner of the function cannot give more
comments, I feel the function wouldn't be very maintainable as
none of us understands it well. On the other hand, you'll need
to update the supported I/O rate lists anyway, so why not just
update the table as well? The supported sample rates from ALSA
are limited too. Overall, I think that continue using a lookup
table wins.
^ permalink raw reply
* RE: [EXT] Re: [PATCH] ASoC: fsl_asrc: replace the process_option table with function
From: S.j. Wang @ 2019-04-10 10:34 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com, broonie@kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20190410095129.GA3468@Asurada>
Hi
>
>
> On Wed, Apr 10, 2019 at 08:26:59AM +0000, S.j. Wang wrote:
> > > Is it possible to update the table? It'd be way quicker to use
> > > lookup table than real-time calculation all the time. I believe you
> > > can simply calculate all the values out for 12KHz and 24KHz since
> > > you have the function. If there are certain combinations of these
> > > two not being supported, then we could mark it with a special value and
> add an if-check to error out.
> > >
> >
> > Yes, but I think the function should be more flexible, if someday we
> > need to support Other sample rate, only need to update the list.
>
> Given the fact that the owner of the function cannot give more comments, I
> feel the function wouldn't be very maintainable as none of us understands it
> well. On the other hand, you'll need to update the supported I/O rate lists
> anyway, so why not just update the table as well? The supported sample
> rates from ALSA are limited too. Overall, I think that continue using a lookup
> table wins.
Alsa support SNDRV_PCM_RATE_KNOT, we can define the rate that we want
To support, and use function is more flexible, and we have use the function
Internally for long time, it is stable😊
^ permalink raw reply
* Re: [EXT] Re: [PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute
From: Viorel Suman @ 2019-04-10 10:34 UTC (permalink / raw)
To: nicoleotsuka@gmail.com
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, lgirdwood@gmail.com,
timur@kernel.org, Xiubo.Lee@gmail.com, festevam@gmail.com,
broonie@kernel.org, linux-kernel@vger.kernel.org, tiwai@suse.com,
viorel.suman@gmail.com, Julia.Lawall@lip6.fr, robh+dt@kernel.org,
dl-linux-imx, kernel@pengutronix.de, shawnguo@kernel.org,
perex@perex.cz, linuxppc-dev@lists.ozlabs.org,
s.hauer@pengutronix.de, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190410042906.GC3032@Asurada>
Hi Nicolin,
On Ma, 2019-04-09 at 21:29 -0700, Nicolin Chen wrote:
> WARNING: This email was created outside of NXP. DO NOT CLICK links or
> attachments unless you recognize the sender and know the content is
> safe.
>
>
>
> On Tue, Apr 09, 2019 at 11:27:39AM +0000, Viorel Suman wrote:
> >
> > Use "of_device_id.data" to specify the machine driver
> > instead of "model" DTS attribute.
> >
> > Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
> >
> > ---
> > sound/soc/fsl/fsl_audmix.c | 43 +++++++++++++++++++++++-----------
> > ---------
> > 1 file changed, 23 insertions(+), 20 deletions(-)
> >
> > + priv->pdev = platform_device_register_data(&pdev->dev, mdrv,
> > 0, NULL,
> > + 0);
> Would you please send a separate patch to replace "pdev->dev"?
Thank you for review. Yes, will send V3.
/Viorel
^ permalink raw reply
* [PATCH v3 0/4] ASoC: fsl: audmix: remove "model" attribute and fix ref leaks
From: Viorel Suman @ 2019-04-10 10:37 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Rob Herring, Mark Rutland,
Takashi Iwai, Shawn Guo, Sascha Hauer, Julia Lawall
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Viorel Suman, linux-kernel@vger.kernel.org, Viorel Suman,
dl-linux-imx, Pengutronix Kernel Team,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
The latest audmix patch-set (v5) had the "model" attribute removed as
requested by Nicolin Chen, but looks like (v4) version of DAI driver
reached "for-next" branch - fix this by removing "model" attribute.
Asside of this fix object reference leaks in machine probe reported by
Julia Lawall.
Viorel Suman (4):
ASoC: fsl_audmix: remove "model" attribute
dt-bindings: fsl,audmix: remove "model" attribute
ASoC: imx-audmix: fix object reference leaks in probe
ASoC: fsl_audmix: cache pdev->dev pointer
Changes since V1:
a) Removed "model" attribute from dt-bindings documentation
b) Adressed Daniel's comments
Changes since V2:
a) Cache pdev->dev pointer in fsl_audmix probe as suggested by Nicolin
.../devicetree/bindings/sound/fsl,audmix.txt | 4 --
sound/soc/fsl/fsl_audmix.c | 61 ++++++++++++----------
sound/soc/fsl/imx-audmix.c | 4 ++
3 files changed, 36 insertions(+), 33 deletions(-)
--
2.7.4
^ 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