* 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] ASoC: fsl_asrc: replace the process_option table with function
From: Nicolin Chen @ 2019-04-10 4:26 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: <1554866131-12088-1-git-send-email-shengjiu.wang@nxp.com>
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?
> +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".
> +{
> + 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.
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?
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_esai: replace fall-through with break
From: Nicolin Chen @ 2019-04-10 3:53 UTC (permalink / raw)
To: S.j. Wang
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
gustavo@embeddedor.com, linuxppc-dev@lists.ozlabs.org,
Xiubo.Lee@gmail.com, broonie@kernel.org, festevam@gmail.com
In-Reply-To: <46e9b55edb223894392a90c7cdb6241a43cb9cff.1554864022.git.shengjiu.wang@nxp.com>
On Wed, Apr 10, 2019 at 02:42:45AM +0000, S.j. Wang wrote:
> case ESAI_HCKT_EXTAL and case ESAI_HCKR_EXTAL should be independent of
> each other, so replace fall-through with break.
>
> Fixes: 43d24e76b698 ("ASoC: fsl_esai: Add ESAI CPU DAI driver")
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Thanks
> ---
> Changes in v2
> - fix the fixes tag.
>
> sound/soc/fsl/fsl_esai.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
> index c7410bbfd2af..bad0dfed6b68 100644
> --- a/sound/soc/fsl/fsl_esai.c
> +++ b/sound/soc/fsl/fsl_esai.c
> @@ -251,7 +251,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
> break;
> case ESAI_HCKT_EXTAL:
> ecr |= ESAI_ECR_ETI;
> - /* fall through */
> + break;
> case ESAI_HCKR_EXTAL:
> ecr |= esai_priv->synchronous ? ESAI_ECR_ETI : ESAI_ECR_ERI;
> break;
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_esai: replace fall-through with break
From: Nicolin Chen @ 2019-04-10 3:52 UTC (permalink / raw)
To: S.j. Wang
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
gustavo@embeddedor.com, linuxppc-dev@lists.ozlabs.org,
Xiubo.Lee@gmail.com, broonie@kernel.org, festevam@gmail.com
In-Reply-To: <20190410035059.GA8844@Asurada>
On Tue, Apr 09, 2019 at 08:50:59PM -0700, Nicolin Chen wrote:
> On Mon, Apr 08, 2019 at 09:28:06AM +0000, S.j. Wang wrote:
> > case ESAI_HCKT_EXTAL and case ESAI_HCKR_EXTAL should be independent of
> > each other, so replace fall-through with break.
> >
> > Fixes: 16bbeb2b43c3 ("ASoC: fsl_esai: Mark expected switch fall-through")
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
>
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
> Thanks
Oops. Acked the older version...should have gong for v2.
Please ignore it.
^ permalink raw reply
* Re: [PATCH V1] ASoC: fsl_esai: replace fall-through with break
From: Nicolin Chen @ 2019-04-10 3:51 UTC (permalink / raw)
To: S.j. Wang
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
gustavo@embeddedor.com, linuxppc-dev@lists.ozlabs.org,
Xiubo.Lee@gmail.com, broonie@kernel.org, festevam@gmail.com
In-Reply-To: <66c117128f300b0ec5f278f538412789c78b60a6.1554715589.git.shengjiu.wang@nxp.com>
On Mon, Apr 08, 2019 at 09:28:06AM +0000, S.j. Wang wrote:
> case ESAI_HCKT_EXTAL and case ESAI_HCKR_EXTAL should be independent of
> each other, so replace fall-through with break.
>
> Fixes: 16bbeb2b43c3 ("ASoC: fsl_esai: Mark expected switch fall-through")
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Thanks
^ permalink raw reply
* [PATCH] ASoC: fsl_asrc: replace the process_option table with function
From: S.j. Wang @ 2019-04-10 3:15 UTC (permalink / raw)
To: timur@kernel.org, nicoleotsuka@gmail.com, Xiubo.Lee@gmail.com,
festevam@gmail.com, broonie@kernel.org,
alsa-devel@alsa-project.org
Cc: linuxppc-dev@lists.ozlabs.org
The table is not flexible if supported sample rate is not in the
table, so use a function to replace it.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_asrc.c | 73 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 53 insertions(+), 20 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 0b937924d2e4..a57c6c829060 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -26,24 +26,6 @@
#define pair_dbg(fmt, ...) \
dev_dbg(&asrc_priv->pdev->dev, "Pair %c: " fmt, 'A' + index, ##__VA_ARGS__)
-/* Sample rates are aligned with that defined in pcm.h file */
-static const u8 process_option[][12][2] = {
- /* 8kHz 11.025kHz 16kHz 22.05kHz 32kHz 44.1kHz 48kHz 64kHz 88.2kHz 96kHz 176kHz 192kHz */
- {{0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0},}, /* 5512Hz */
- {{0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0},}, /* 8kHz */
- {{0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0},}, /* 11025Hz */
- {{1, 2}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0},}, /* 16kHz */
- {{1, 2}, {1, 2}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0},}, /* 22050Hz */
- {{1, 2}, {2, 1}, {2, 1}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0},}, /* 32kHz */
- {{2, 2}, {2, 2}, {2, 1}, {2, 1}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0},}, /* 44.1kHz */
- {{2, 2}, {2, 2}, {2, 1}, {2, 1}, {0, 2}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0},}, /* 48kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 1}, {1, 2}, {0, 2}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0},}, /* 64kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {1, 2}, {1, 2}, {1, 2}, {1, 1}, {1, 1}, {1, 1}, {1, 1}, {1, 1},}, /* 88.2kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {1, 2}, {1, 2}, {1, 2}, {1, 1}, {1, 1}, {1, 1}, {1, 1}, {1, 1},}, /* 96kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 1}, {2, 1}, {2, 1}, {2, 1}, {2, 1},}, /* 176kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 1}, {2, 1}, {2, 1}, {2, 1}, {2, 1},}, /* 192kHz */
-};
-
/* Corresponding to process_option */
static int supported_input_rate[] = {
5512, 8000, 11025, 16000, 22050, 32000, 44100, 48000, 64000, 88200,
@@ -79,6 +61,49 @@
static unsigned char *clk_map[2];
+static int proc_autosel(int Fsin, int Fsout, int *pre_proc, int *post_proc)
+{
+ 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);
+
+ /*
+ * Not supported 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;
+
+ return 0;
+}
+
/**
* Request ASRC pair
*
@@ -239,8 +264,10 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
u32 inrate, outrate, indiv, outdiv;
u32 clk_index[2], div[2];
int in, out, channels;
+ int pre_proc, post_proc;
struct clk *clk;
bool ideal;
+ int ret;
if (!config) {
pair_err("invalid pair config\n");
@@ -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;
+ }
+
/* Apply configurations for pre- and post-processing */
regmap_update_bits(asrc_priv->regmap, REG_ASRCFG,
ASRCFG_PREMODi_MASK(index) | ASRCFG_POSTMODi_MASK(index),
- ASRCFG_PREMOD(index, process_option[in][out][0]) |
- ASRCFG_POSTMOD(index, process_option[in][out][1]));
+ ASRCFG_PREMOD(index, pre_proc) |
+ ASRCFG_POSTMOD(index, post_proc));
return fsl_asrc_set_ideal_ratio(pair, inrate, outrate);
}
--
1.9.1
^ permalink raw reply related
* [PATCH V2] ASoC: fsl_esai: replace fall-through with break
From: S.j. Wang @ 2019-04-10 2:42 UTC (permalink / raw)
To: timur@kernel.org, nicoleotsuka@gmail.com, Xiubo.Lee@gmail.com,
festevam@gmail.com, broonie@kernel.org,
alsa-devel@alsa-project.org, gustavo@embeddedor.com
Cc: linuxppc-dev@lists.ozlabs.org
case ESAI_HCKT_EXTAL and case ESAI_HCKR_EXTAL should be independent of
each other, so replace fall-through with break.
Fixes: 43d24e76b698 ("ASoC: fsl_esai: Add ESAI CPU DAI driver")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
Changes in v2
- fix the fixes tag.
sound/soc/fsl/fsl_esai.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index c7410bbfd2af..bad0dfed6b68 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -251,7 +251,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
break;
case ESAI_HCKT_EXTAL:
ecr |= ESAI_ECR_ETI;
- /* fall through */
+ break;
case ESAI_HCKR_EXTAL:
ecr |= esai_priv->synchronous ? ESAI_ECR_ETI : ESAI_ECR_ERI;
break;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/xmon: add read-only mode
From: Christopher M Riedl @ 2019-04-10 2:40 UTC (permalink / raw)
To: Andrew Donnellan, linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <f7a3d198-235d-b38c-e7a8-0ed103d7366d@au1.ibm.com>
> On April 8, 2019 at 2:37 AM Andrew Donnellan <andrew.donnellan@au1.ibm.com> wrote:
>
>
> On 8/4/19 1:08 pm, Christopher M. Riedl 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
> > 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"
>
> "Allow xmon write operations" would be clearer. This option has no
> impact on read operations.
>
Agreed, if the option isn't renamed again I will fix this in the next version :)
>
> > + 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.
>
> This is an improvement but still doesn't clearly explain the
> relationship between selecting this option and using the cmdline options.
>
I will reword this in the next version.
>
> > +
> > 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)
>
> We might need to rethink how the cmdline options work.
>
> What happens if CONFIG_XMON_RW=n, but "xmon=rw" on the command line?
>
> I think I agree with Oliver that we're not being clear whether this
> config option is selecting code to be compiled in, or whether it's just
> setting a default.
>
Please refer to my reply to Oliver's comments -- 'xmon=rw' is broken and will be removed
in the next version. I think 'xmon=ro' still makes sense to optionally enable xmon's read-
only mode even when writes are allowed on a non-secure system.
>
> > #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";
>
> Yay, error messages! Though something like: "Operation disabled: xmon is
> in read-only mode" would be clearer.
>
Agreed, changing this in the next version.
>
> > +
> > 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
> >
>
> --
> Andrew Donnellan OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com IBM Australia Limited
>
^ permalink raw reply
* RE: [EXT] Re: [PATCH V1] ASoC: fsl_esai: replace fall-through with break
From: S.j. Wang @ 2019-04-10 2:39 UTC (permalink / raw)
To: Nicolin Chen, Gustavo A. R. Silva
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: <20190409035450.GA8476@Asurada>
Hi
>
> Hi Gustavo,
>
> On Mon, Apr 08, 2019 at 10:20:25PM -0500, Gustavo A. R. Silva wrote:
> > >>> diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
> > >>> index
> > >>> c7410bbfd2af..bad0dfed6b68 100644
> > >>> --- a/sound/soc/fsl/fsl_esai.c
> > >>> +++ b/sound/soc/fsl/fsl_esai.c
> > >>> @@ -251,7 +251,7 @@ static int fsl_esai_set_dai_sysclk(struct
> > >> snd_soc_dai *dai, int clk_id,
> > >>> break;
> > >>> case ESAI_HCKT_EXTAL:
> > >>> ecr |= ESAI_ECR_ETI;
> > >>
> > >> Also, you should use a simple assignment operator "=" instead of
> > >> "|=" in both cases.
> > >
> > > The result is same for "=" and "|=", because there is "ecr = 0" in
> > > beginning of This function.
> > >
> >
> > Following that same logic, then why not use "+=" instead?
> >
> > The point is: is "|=" or any other assignment operator other than "="
> necessary?
> > The answer in this case is: no, it is not. So, go for the simple one
> > and avoid any unnecessary confusion.
>
> I would like to keep "|=" here, just in case that someday it'd be easier to
> insert something to ecr before this chunk. So please get easy on this one.
>
> Thanks
> Nicolin
Thanks for reviewing, I will send v2.
Best regards
Wang shengjiu
^ permalink raw reply
* Re: [PATCH v2] powerpc/xmon: add read-only mode
From: Christopher M Riedl @ 2019-04-10 2:35 UTC (permalink / raw)
To: Oliver; +Cc: linuxppc-dev, Andrew Donnellan
In-Reply-To: <CAOSf1CEZgHtz5u6F+N7egf3B+MiquFcsopm1yiMZvdYvKcip1w@mail.gmail.com>
> 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?
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.
'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 0/4] Enabling secure boot on PowerNV systems
From: Claudio Carvalho @ 2019-04-09 22:55 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-efi, Ard Biesheuvel, Nayna Jain, Linux Kernel Mailing List,
Matthew Garret, linuxppc-dev, Paul Mackerras, Jeremy Kerr,
linux-integrity
In-Reply-To: <CACdnJuv_5A1_6CH1+Jn7SCrmW8Y6JuzsMmJoc1=_vn0nKdYMjQ@mail.gmail.com>
On 4/5/19 7:19 PM, Matthew Garrett wrote:
> On Fri, Apr 5, 2019 at 2:11 PM Claudio Carvalho <cclaudio@linux.ibm.com> wrote:
>>
>> On 4/3/19 7:27 PM, Matthew Garrett wrote:
>>> Not supporting dbx seems like a pretty significant shortcoming. How
>>> are signatures meant to be revoked?
>>
>> We began by focusing on certificates for keys that can be added at
>> runtime. Before adding support for revocation, we plan to gather
>> additional use cases. In the meantime, unwanted certificates can be
>> removed by the administrator.
> Based on our experience doing this in UEFI, that's insufficient - you
> want to be able to block individual binaries or leaf certificates
> without dropping trust in an intermediate certificate entirely.
We agree that a dbx would be useful for blacklisting particular kernels
signed with given certificate. However, we have been avoiding doing so for
the initial release of secure boot on OpenPOWER. We don't have individual
firmware binaries in OpenPOWER. Kernels are currently the only concern for
the OS secure boot certificates we're discussing here. Also, we have a very
limited keystore space in POWER9.
Petitboot doesn't have standardized OS kernel verification at all right
now. Having the capability even without dbx seems valuable.
>
>>>> PK, KEK and db updates will be signed the same way, that is, using
>>>> userspace tooling like efitools in PowerNV. As for the authentication
>>>> descriptors, only the EFI_VARIABLE_AUTHENTICATION_2 descriptor will be
>>>> supported.
>>> Is this API documented?
>>
>> The API is still a work in progress. We are planning to publish a document
>> describing the current API and overall design shortly.
> Ok. How are the attributes interpreted by the API?
We support a subset of standard EFI variable attributes, and we only use
EFI variables that relate to secure boot. Our goal is not to implement
UEFI. However, we do seek to be compatible with user space tooling and
reuse as much existing infrastructure as possible. We don’t support the
following: EFI_VARIABLE_HARDWARE_ERROR_RECORD,
EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS and
EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS.
>
>> Perhaps the biggest departure is that the secure variables are stored in
>> flash memory that is not lockable. In order to protect the secure
>> variables, hashes of the flash regions where they're stored are written to
>> TPM NVRAM indices. The TPM NVRAM indices we use are write locked at
>> runtime. The sysadmin enqueues update commands in flash. During the next
>> boot, the firmware verifies and processes the commands to update the
>> certificate store and accompanying integrity hashes in the TPM NVRAM
>> indices and write locks them. Before certificates read from flash are
>> used, the certificate store is hashed and compared against the hashes
>> stored from the TPM. The one exception is the PK. We store it in a TPM
>> NVRAM index by itself rather than flash because updates to it must be
>> guaranteed to be atomic.
> What's the behaviour if multiple updates are enqueued? Does reading
> back show a mocked up updated variable or the original state?
Our secure variable updates are only applied at boot time. If any one of
them fails, they all fail.
>
>>> I think that depends on exactly what problem you're trying to solve.
>>> Some aspects of the EFI secure boot design end up mirroring the
>>> economics of the PC ecosystem rather than being inherently good design
>>> goals, so it'd be helpful to know whether you're taking this solution
>>> because you want the same three-level key infrastructure or because
>>> that just leaves you compatible with the tooling.
>>
>> In our use case, the three-level key hierarchy conveniently supports the
>> concept of (1) an administrator authority, who authorizes (2) other
>> organizations, e.g., distros, to provide (3) certificates for their code
>> signing keys. By using efivars, we leverage pre-existing userspace EFI
>> tools to generate authenticated updates and certificates. Additionally,
>> pre-existing kernel infrastructure simplifies efivars processing.
> I'm not really clear on the workflow here. Who's the administrator
> authority? When would they be updating the second level of keys? If
> there's no support for revocation, why would distributions need two
> levels of key in the system database rather than just distributing a
> single intermediate and signing their actual signing certs with that?
In OpenPOWER systems, we enable our customers and business partners to
establish and manage the platform key certificate, which is the root of our
key hierarchy. From there, through the KEK, they can delegate authority to
intermediate level organizations, e.g. distros or IT departments or
business operations. Those intermediate level organizations then manage the
code signing certificates in the DB. If this answer doesn’t address your
question, can you please rephrase?
Thanks,
Claudio
^ permalink raw reply
* Re: [PATCH v10 02/18] counter: Documentation: Add Generic Counter sysfs documentation
From: David Lechner @ 2019-04-09 13:12 UTC (permalink / raw)
To: Pavel Machek, William Breathitt Gray
Cc: mark.rutland, benjamin.gaignard, linux-pwm, linux-iio,
patrick.havelange, thierry.reding, pmeerw, shawnguo, lars,
daniel.lezcano, linux-arm-kernel, devicetree, robh+dt,
Jonathan Cameron, tglx, fabrice.gasnier, esben, gregkh,
linux-kernel, leoyang.li, knaack.h, akpm, linuxppc-dev, jic23
In-Reply-To: <20190408215531.GB27592@amd>
On 4/8/19 4:55 PM, Pavel Machek wrote:
>
> pavel@amd:~$ cat /sys/power/state
> freeze mem disk
>
> And we use [] to mark current selection:
I know this is used elsewhere, but I don't think it is a good pattern
to copy. Parsing it from user-space is annoying. Also it breaks the
sysfs rule of "one item per file" since it tells us two things - the
list of possible values _and_ the current active value.
>
> pavel@amd:~$ cat /sys/class/leds/tpacpi\:\:thinkvantage/trigger
> [none] bluetooth-power rfkill-any rfkill-none kbd-scrolllock
> kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock
> kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock
> kbd-ctrlrlock AC-online BAT0-charging-or-full BAT0-charging BAT0-full
> BAT0-charging-blink-full-solid rfkill0 phy0rx phy0tx phy0assoc
> phy0radio phy0tpt mmc0 timer heartbeat audio-mute audio-micmute
> rfkill1
>
> Note this only works if you have less than PAGE_SIZE of entries... and
> will never have more.
>
> Pavel
>
^ permalink raw reply
* [PATCH 10/10] powerpc: select DYNAMIC_DEBUG_RELATIVE_POINTERS for PPC64
From: Rasmus Villemoes @ 2019-04-09 21:25 UTC (permalink / raw)
To: Andrew Morton, linuxppc-dev; +Cc: Jason Baron, Rasmus Villemoes, linux-kernel
In-Reply-To: <20190409212517.7321-1-linux@rasmusvillemoes.dk>
Similar to GENERIC_BUG_RELATIVE_POINTERS, one can now relativize the
four const char* members of struct _ddebug, thus saving 16 bytes per
instance (one for each pr_debug(), dev_debug() etc. in a
CONFIG_DYNAMIC_DEBUG kernel). The asm-generic implementation seems to
work out-of-the-box, though this is only compile-tested.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2d0be82c3061..6821c8ae1d62 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -155,6 +155,7 @@ config PPC
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS
select DCACHE_WORD_ACCESS if PPC64 && CPU_LITTLE_ENDIAN
+ select DYNAMIC_DEBUG_RELATIVE_POINTERS if PPC64
select DYNAMIC_FTRACE if FUNCTION_TRACER
select EDAC_ATOMIC_SCRUB
select EDAC_SUPPORT
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index a0c132bedfae..f332e202192a 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -3,6 +3,7 @@ generated-y += syscall_table_64.h
generated-y += syscall_table_c32.h
generated-y += syscall_table_spu.h
generic-y += div64.h
+generic-y += dynamic_debug.h
generic-y += export.h
generic-y += irq_regs.h
generic-y += local64.h
--
2.20.1
^ permalink raw reply related
* [PATCH 07/10] dynamic_debug: add asm-generic implementation for DYNAMIC_DEBUG_RELATIVE_POINTERS
From: Rasmus Villemoes @ 2019-04-09 21:25 UTC (permalink / raw)
To: Andrew Morton
Cc: x86, Rasmus Villemoes, linux-kernel, Will Deacon, Jason Baron,
Ingo Molnar, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20190409212517.7321-1-linux@rasmusvillemoes.dk>
A 64 bit architecture can reduce the size of the kernel image by
selecting CONFIG_DYNAMIC_DEBUG_RELATIVE_POINTERS, but it must provide
a proper DEFINE_DYNAMIC_DEBUG_METADATA macro for emitting the struct
_ddebug in assembly. However, since that does not involve any
instructions, this generic implementation should be usable by most if
not all.
It relies on
(1) standard assembly directives that should work on
all architectures
(2) the "i" constraint for an constant, and
(3) %cN emitting the constant operand N without punctuation
and of course the layout of _ddebug being what one expects. I've
succesfully built x86-64, arm64 and ppc64 defconfig +
CONFIG_DYNAMIC_DEBUG + patches to hook up this header, and boot-tested
the x86-64 one.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
include/asm-generic/dynamic_debug.h | 116 ++++++++++++++++++++++++++++
include/linux/jump_label.h | 2 +
2 files changed, 118 insertions(+)
create mode 100644 include/asm-generic/dynamic_debug.h
diff --git a/include/asm-generic/dynamic_debug.h b/include/asm-generic/dynamic_debug.h
new file mode 100644
index 000000000000..f1dd3019cd98
--- /dev/null
+++ b/include/asm-generic/dynamic_debug.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_GENERIC_DYNAMIC_DEBUG_H
+#define _ASM_GENERIC_DYNAMIC_DEBUG_H
+
+#ifndef _DYNAMIC_DEBUG_H
+#error "don't include asm/dynamic_debug.h directly"
+#endif
+
+#include <linux/build_bug.h>
+#ifdef CONFIG_JUMP_LABEL
+#include <linux/jump_label.h>
+#endif
+
+/*
+ * We need to know the exact layout of struct _ddebug in order to
+ * initialize it in assembly. Check that all members are at expected
+ * offsets - if any of these fail, the arch cannot use this generic
+ * dynamic_debug.h. DYNAMIC_DEBUG_RELATIVE_POINTERS is pointless for
+ * !64BIT, so we expect the static_key to be at an 8-byte boundary
+ * since it contains stuff which is long-aligned.
+ */
+
+static_assert(BITS_PER_LONG == 64);
+static_assert(offsetof(struct _ddebug, modname_disp) == 0);
+static_assert(offsetof(struct _ddebug, function_disp) == 4);
+static_assert(offsetof(struct _ddebug, filename_disp) == 8);
+static_assert(offsetof(struct _ddebug, format_disp) == 12);
+static_assert(offsetof(struct _ddebug, flags_lineno) == 16);
+
+#ifdef CONFIG_JUMP_LABEL
+static_assert(offsetof(struct _ddebug, key) == 24);
+static_assert(offsetof(struct static_key, enabled) == 0);
+static_assert(offsetof(struct static_key, type) == 8);
+
+/* <4 bytes padding>, .enabled, <4 bytes padding>, .type */
+# ifdef DEBUG
+# define _DPRINTK_ASM_KEY_INIT \
+ "\t.int 0\n" "\t.int 1\n" "\t.int 0\n" "\t.quad "__stringify(__JUMP_TYPE_TRUE)"\n"
+# else
+# define _DPRINTK_ASM_KEY_INIT \
+ "\t.int 0\n" "\t.int 0\n" "\t.int 0\n" "\t.quad "__stringify(__JUMP_TYPE_FALSE)"\n"
+# endif
+#else /* !CONFIG_JUMP_LABEL */
+#define _DPRINTK_ASM_KEY_INIT ""
+#endif
+
+/*
+ * There's a bit of magic involved here.
+ *
+ * First, unlike the bug table entries, we need to define an object in
+ * assembly which we can reference from C code (for use by the
+ * DYNAMIC_DEBUG_BRANCH macro), but we don't want 'name' to have
+ * external linkage (that would require use of globally unique
+ * identifiers, which we can't guarantee). Fortunately, the "extern"
+ * keyword just tells the compiler that _somebody_ provides that
+ * symbol - usually that somebody is the linker, but in this case it's
+ * the assembler, and since we do not do .globl name, the symbol gets
+ * internal linkage.
+ *
+ * So far so good. The next problem is that there's no scope in
+ * assembly, so the identifier 'name' has to be unique within each
+ * translation unit - otherwise all uses of that identifier end up
+ * referring to the same struct _ddebug instance. pr_debug and friends
+ * do this by use of indirection and __UNIQUE_ID(), and new users of
+ * DEFINE_DYNAMIC_DEBUG_METADATA() should do something similar. We
+ * need to catch cases where this is not done at build time.
+ *
+ * With assembly-level .ifndef we can ensure that we only define a
+ * given identifier once, preventing "symbol 'foo' already defined"
+ * errors. But we still need to detect and fail on multiple uses of
+ * the same identifer. The simplest, and wrong, solution to that is to
+ * add an .else .error branch to the .ifndef. The problem is that just
+ * because the DEFINE_DYNAMIC_DEBUG_METADATA() macro is only expanded
+ * once with a given identifier, the compiler may emit the assembly
+ * code multiple times, e.g. if the macro appears in an inline
+ * function. Now, in a normal case like
+ *
+ * static inline get_next_id(void) { static int v; return ++v; }
+ *
+ * all inlined copies of get_next_id are _supposed_ to refer to the
+ * same object 'v'. So we do need to allow this chunk of assembly to
+ * appear multiple times with the same 'name', as long as they all
+ * came from the same DEFINE_DYNAMIC_DEBUG_METADATA() instance. To do
+ * that, we pass __COUNTER__ to the asm(), and set an assembler symbol
+ * name.ddebug.once to that value when we first define 'name'. When we
+ * meet a second attempt at defining 'name', we compare
+ * name.ddebug.once to %6 and error out if they are different.
+ */
+
+#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
+ extern struct _ddebug name; \
+ asm volatile(".ifndef " __stringify(name) "\n" \
+ ".pushsection __verbose,\"aw\"\n" \
+ ".type "__stringify(name)", STT_OBJECT\n" \
+ ".size "__stringify(name)", %c5\n" \
+ "1:\n" \
+ __stringify(name) ":\n" \
+ "\t.int %c0 - 1b /* _ddebug::modname_disp */\n" \
+ "\t.int %c1 - 1b /* _ddebug::function_disp */\n" \
+ "\t.int %c2 - 1b /* _ddebug::filename_disp */\n" \
+ "\t.int %c3 - 1b /* _ddebug::format_disp */\n" \
+ "\t.int %c4 /* _ddebug::flags_lineno */\n" \
+ _DPRINTK_ASM_KEY_INIT \
+ "\t.org 1b+%c5\n" \
+ ".popsection\n" \
+ ".set "__stringify(name)".ddebug.once, %c6\n" \
+ ".elseif "__stringify(name)".ddebug.once - %c6\n" \
+ ".line "__stringify(__LINE__) " - 1\n" \
+ ".error \"'"__stringify(name)"' used as _ddebug identifier more than once\"\n" \
+ ".endif\n" \
+ : : "i" (KBUILD_MODNAME), "i" (__func__), \
+ "i" (__FILE__), "i" (fmt), \
+ "i" (_DPRINTK_FLAGS_LINENO_INIT), \
+ "i" (sizeof(struct _ddebug)), "i" (__COUNTER__))
+
+#endif /* _ASM_GENERIC_DYNAMIC_DEBUG_H */
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 3e113a1fa0f1..2b027d2ef3d0 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -190,6 +190,8 @@ struct module;
#ifdef CONFIG_JUMP_LABEL
+#define __JUMP_TYPE_FALSE 0
+#define __JUMP_TYPE_TRUE 1
#define JUMP_TYPE_FALSE 0UL
#define JUMP_TYPE_TRUE 1UL
#define JUMP_TYPE_LINKED 2UL
--
2.20.1
^ permalink raw reply related
* [PATCH 00/10] implement DYNAMIC_DEBUG_RELATIVE_POINTERS
From: Rasmus Villemoes @ 2019-04-09 21:25 UTC (permalink / raw)
To: Andrew Morton
Cc: x86, Rasmus Villemoes, linux-kernel, Will Deacon, Jason Baron,
Ingo Molnar, linuxppc-dev, linux-arm-kernel
Similar to CONFIG_GENERIC_BUG_RELATIVE_POINTERS that replaces (8 byte)
const char* members by (4 byte) signed offsets from the bug_entry,
this implements the similar thing for struct _ddebug, the descriptors
underlying pr_debug() and friends in a CONFIG_DYNAMIC_DEBUG kernel.
Since struct _ddebug has four string members, we save 16 byte per
instance. The total savings seem to be comparable to what is saved by
an architecture selecting GENERIC_BUG_RELATIVE_POINTERS (see patch 8
for some numbers for a common distro config).
While refreshing these patches, which were orignally just targeted at
x86-64, it occured to me that despite the implementation relying on
inline asm, there's nothing x86 specific about it, and indeed it seems
to work out-of-the-box for ppc64 and arm64 as well, but those have
only been compile-tested.
The first 6 patches are rather pedestrian preparations. The fun stuff
is in patch 7, and the remaining three patches are just the minimal
boilerplate to hook up the config option and asm-generic header on the
three architectures.
Rasmus Villemoes (10):
linux/device.h: use unique identifier for each struct _ddebug
linux/net.h: use unique identifier for each struct _ddebug
linux/printk.h: use unique identifier for each struct _ddebug
dynamic_debug: introduce accessors for string members of struct
_ddebug
dynamic_debug: drop use of bitfields in struct _ddebug
dynamic_debug: introduce CONFIG_DYNAMIC_DEBUG_RELATIVE_POINTERS
dynamic_debug: add asm-generic implementation for
DYNAMIC_DEBUG_RELATIVE_POINTERS
x86-64: select DYNAMIC_DEBUG_RELATIVE_POINTERS
arm64: select DYNAMIC_DEBUG_RELATIVE_POINTERS
powerpc: select DYNAMIC_DEBUG_RELATIVE_POINTERS for PPC64
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/Kbuild | 1 +
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
arch/x86/Kconfig | 1 +
arch/x86/entry/vdso/vdso32/vclock_gettime.c | 1 +
arch/x86/include/asm/Kbuild | 1 +
include/asm-generic/dynamic_debug.h | 116 ++++++++++++++++++++
include/linux/device.h | 4 +-
include/linux/dynamic_debug.h | 26 +++--
include/linux/jump_label.h | 2 +
include/linux/net.h | 4 +-
include/linux/printk.h | 4 +-
lib/Kconfig.debug | 3 +
lib/dynamic_debug.c | 111 ++++++++++++++-----
15 files changed, 237 insertions(+), 40 deletions(-)
create mode 100644 include/asm-generic/dynamic_debug.h
--
2.20.1
^ permalink raw reply
* Re: [PATCH] Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE [BZ #10339]
From: hpa @ 2019-04-09 15:02 UTC (permalink / raw)
To: Florian Weimer, libc-alpha; +Cc: linux-api, linuxppc-dev, adhemerval.zanella
In-Reply-To: <875zrnlakd.fsf@oldenburg2.str.redhat.com>
On April 9, 2019 4:47:30 AM MDT, Florian Weimer <fweimer@redhat.com> wrote:
>struct termios2 is required for setting arbitrary baud rates on serial
>ports. <sys/ioctl.h> and <linux/termios.h> have conflicting
>definitions in the existing termios definitions, which means that it
>is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
>with glibc. Providing a definition within glibc resolves this problem.
>
>This does not completely address bug 10339, but it at least exposes
>the current kernel functionality in this area.
>
>Support for struct termios2 is architecture-specific in the kernel.
>Support on alpha was only added in Linux 4.20. POWER support is
>currently missing. The expectation is that the kernel will eventually
>use the generic UAPI definition for struct termios2.
>
>2019-04-09 Florian Weimer <fweimer@redhat.com>
>
> [BZ #10339]
> Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE.
> * sysdeps/unix/sysv/linux/Makefile [$(subdir) == termios] (tests):
> Add tst-termios2.
> * sysdeps/unix/sysv/linux/tst-termios2.c: New file.
> * sysdeps/unix/sysv/linux/bits/termios2-struct.h: Likewise.
> * sysdeps/unix/sysv/linux/bits/termios.h [__USE_GNU]: Include it.
> * sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h: New file.
> * sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h: Likewise.
>
>diff --git a/NEWS b/NEWS
>index b58e2469d4..5e6ecb9c7d 100644
>--- a/NEWS
>+++ b/NEWS
>@@ -18,6 +18,9 @@ Major new features:
>
> * On Linux, the gettid function has been added.
>
>+* On Linux, <termios.h> now provides a definition of struct termios2
>with
>+ the _GNU_SOURCE feature test macro.
>+
> * Minguo (Republic of China) calendar support has been added as an
> alternative calendar for the following locales: zh_TW, cmn_TW, hak_TW,
> nan_TW, lzh_TW.
>diff --git a/sysdeps/unix/sysv/linux/Makefile
>b/sysdeps/unix/sysv/linux/Makefile
>index 52ac6ad484..4cb5e4f0d2 100644
>--- a/sysdeps/unix/sysv/linux/Makefile
>+++ b/sysdeps/unix/sysv/linux/Makefile
>@@ -156,6 +156,7 @@ endif
>
> ifeq ($(subdir),termios)
> sysdep_headers += termio.h
>+tests += tst-termios2
> endif
>
> ifeq ($(subdir),posix)
>diff --git a/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5f09445e23
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition. Linux/alpha version.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+ tcflag_t c_iflag;
>+ tcflag_t c_oflag;
>+ tcflag_t c_cflag;
>+ tcflag_t c_lflag;
>+ cc_t c_cc[NCCS];
>+ cc_t c_line;
>+ speed_t c_ispeed;
>+ speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/bits/termios.h
>b/sysdeps/unix/sysv/linux/bits/termios.h
>index 997231cd03..45ac7affdf 100644
>--- a/sysdeps/unix/sysv/linux/bits/termios.h
>+++ b/sysdeps/unix/sysv/linux/bits/termios.h
>@@ -25,6 +25,10 @@ typedef unsigned int speed_t;
> typedef unsigned int tcflag_t;
>
> #include <bits/termios-struct.h>
>+#ifdef __USE_GNU
>+# include <bits/termios2-struct.h>
>+#endif
>+
> #include <bits/termios-c_cc.h>
> #include <bits/termios-c_iflag.h>
> #include <bits/termios-c_oflag.h>
>diff --git a/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5a48e45ef3
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition. Linux/generic version.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+ tcflag_t c_iflag;
>+ tcflag_t c_oflag;
>+ tcflag_t c_cflag;
>+ tcflag_t c_lflag;
>+ cc_t c_line;
>+ cc_t c_cc[NCCS];
>+ speed_t c_ispeed;
>+ speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..7c889e575c
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition. Linux/sparc version.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+ tcflag_t c_iflag;
>+ tcflag_t c_oflag;
>+ tcflag_t c_cflag;
>+ tcflag_t c_lflag;
>+ cc_t c_line;
>+ cc_t c_cc[NCCS + 2];
>+ speed_t c_ispeed;
>+ speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/tst-termios2.c
>b/sysdeps/unix/sysv/linux/tst-termios2.c
>new file mode 100644
>index 0000000000..82326a1288
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/tst-termios2.c
>@@ -0,0 +1,48 @@
>+/* Minimal test of struct termios2 definition.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#include <termios.h>
>+#include <sys/ioctl.h>
>+
>+/* This function is never executed, but must be compiled successfully.
>+ Accessing serial ports in the test suite is problematic because
>+ they likely correspond with low-level system functionality. */
>+void
>+not_executed (int fd)
>+{
>+ /* Avoid a compilation failure if TCGETS2, TCSETS2 are not
>+ defined. */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+ struct termios2 ti;
>+ ioctl (fd, TCGETS2, &ti);
>+ ioctl (fd, TCSETS2, &ti);
>+#endif
>+}
>+
>+static int
>+do_test (void)
>+{
>+ /* Fail at run time if TCGETS2 or TCSETS2 is not defined. */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+ return 0;
>+#else
>+ return 1;
>+#endif
>+}
>+
>+#include <support/test-driver.c>
PowerPC doesn't need it at all.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* [PATCH 2/2] powerpc/powernv: Add new opal message type
From: Vasant Hegde @ 2019-04-09 16:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vasant Hegde, Jeremy Kerr
In-Reply-To: <20190409165202.30187-1-hegdevasant@linux.vnet.ibm.com>
We have OPAL_MSG_PRD message type to pass prd related messages from OPAL
to `opal-prd`. It can handle messages upto 64 bytes. We have a requirement
to send bigger than 64 bytes of data from OPAL to `opal-prd`. Lets add new
message type (OPAL_MSG_PRD2) to pass bigger data.
Cc: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/opal-api.h | 1 +
arch/powerpc/platforms/powernv/opal-prd.c | 9 ++++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 821ed4fa95ad..79272e145281 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -455,6 +455,7 @@ enum opal_msg_type {
OPAL_MSG_DPO = 5,
OPAL_MSG_PRD = 6,
OPAL_MSG_OCC = 7,
+ OPAL_MSG_PRD2 = 8,
OPAL_MSG_TYPE_MAX,
};
diff --git a/arch/powerpc/platforms/powernv/opal-prd.c b/arch/powerpc/platforms/powernv/opal-prd.c
index 4070bb4e9da4..ef9a19de7cca 100644
--- a/arch/powerpc/platforms/powernv/opal-prd.c
+++ b/arch/powerpc/platforms/powernv/opal-prd.c
@@ -350,7 +350,7 @@ static int opal_prd_msg_notifier(struct notifier_block *nb,
int msg_size, item_size;
unsigned long flags;
- if (msg_type != OPAL_MSG_PRD)
+ if (msg_type != OPAL_MSG_PRD && msg_type != OPAL_MSG_PRD2)
return 0;
/* Calculate total size of the message and item we need to store. The
@@ -401,6 +401,13 @@ static int opal_prd_probe(struct platform_device *pdev)
return rc;
}
+ rc = opal_message_notifier_register(OPAL_MSG_PRD2, &opal_prd_event_nb);
+ if (rc) {
+ pr_err("%s: Couldn't register event notifier (%d)\n",
+ __func__, OPAL_MSG_PRD2);
+ return rc;
+ }
+
rc = misc_register(&opal_prd_dev);
if (rc) {
pr_err("failed to register miscdev\n");
--
2.14.3
^ permalink raw reply related
* [PATCH 1/2] powerpc/powernv: Enhance opal message read interface
From: Vasant Hegde @ 2019-04-09 16:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vasant Hegde, Mahesh Salgaonkar, Jeremy Kerr
Use "opal-msg-size" device tree property to allocate memory for "opal_msg".
Also replace `reserved` field in "struct opal_msg" with `size`. So that
opal_get_msg() user can get actual message size.
Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Cc: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/opal-api.h | 2 +-
arch/powerpc/platforms/powernv/opal.c | 30 ++++++++++++++++++++----------
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 870fb7b239ea..821ed4fa95ad 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -460,7 +460,7 @@ enum opal_msg_type {
struct opal_msg {
__be32 msg_type;
- __be32 reserved;
+ __be32 size;
__be64 params[8];
};
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2b0eca104f86..52fb52c2ca4b 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -62,6 +62,8 @@ static DEFINE_SPINLOCK(opal_write_lock);
static struct atomic_notifier_head opal_msg_notifier_head[OPAL_MSG_TYPE_MAX];
static uint32_t opal_heartbeat;
static struct task_struct *kopald_tsk;
+static struct opal_msg *opal_msg;
+static uint64_t opal_msg_size;
void opal_configure_cores(void)
{
@@ -268,14 +270,9 @@ static void opal_message_do_notify(uint32_t msg_type, void *msg)
static void opal_handle_message(void)
{
s64 ret;
- /*
- * TODO: pre-allocate a message buffer depending on opal-msg-size
- * value in /proc/device-tree.
- */
- static struct opal_msg msg;
u32 type;
- ret = opal_get_msg(__pa(&msg), sizeof(msg));
+ ret = opal_get_msg(__pa(opal_msg), opal_msg_size);
/* No opal message pending. */
if (ret == OPAL_RESOURCE)
return;
@@ -287,14 +284,14 @@ static void opal_handle_message(void)
return;
}
- type = be32_to_cpu(msg.msg_type);
+ type = be32_to_cpu(opal_msg->msg_type);
/* Sanity check */
if (type >= OPAL_MSG_TYPE_MAX) {
pr_warn_once("%s: Unknown message type: %u\n", __func__, type);
return;
}
- opal_message_do_notify(type, (void *)&msg);
+ opal_message_do_notify(type, (void *)opal_msg);
}
static irqreturn_t opal_message_notify(int irq, void *data)
@@ -303,9 +300,22 @@ static irqreturn_t opal_message_notify(int irq, void *data)
return IRQ_HANDLED;
}
-static int __init opal_message_init(void)
+static int __init opal_message_init(struct device_node *opal_node)
{
int ret, i, irq;
+ const __be32 *val;
+
+ val = of_get_property(opal_node, "opal-msg-size", NULL);
+ if (val)
+ opal_msg_size = be32_to_cpup(val);
+
+ /* If opal-msg-size property is not available then use default size */
+ if (!opal_msg_size)
+ opal_msg_size = sizeof(struct opal_msg);
+
+ opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);
+ if (!opal_msg)
+ return -ENOMEM;
for (i = 0; i < OPAL_MSG_TYPE_MAX; i++)
ATOMIC_INIT_NOTIFIER_HEAD(&opal_msg_notifier_head[i]);
@@ -886,7 +896,7 @@ static int __init opal_init(void)
}
/* Initialise OPAL messaging system */
- opal_message_init();
+ opal_message_init(opal_node);
/* Initialise OPAL asynchronous completion interface */
opal_async_comp_init();
--
2.14.3
^ permalink raw reply related
* Re: [PATCH] Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE [BZ #10339]
From: hpa @ 2019-04-09 15:07 UTC (permalink / raw)
To: Florian Weimer, libc-alpha; +Cc: linux-api, linuxppc-dev, adhemerval.zanella
In-Reply-To: <875zrnlakd.fsf@oldenburg2.str.redhat.com>
On April 9, 2019 4:47:30 AM MDT, Florian Weimer <fweimer@redhat.com> wrote:
>struct termios2 is required for setting arbitrary baud rates on serial
>ports. <sys/ioctl.h> and <linux/termios.h> have conflicting
>definitions in the existing termios definitions, which means that it
>is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
>with glibc. Providing a definition within glibc resolves this problem.
>
>This does not completely address bug 10339, but it at least exposes
>the current kernel functionality in this area.
>
>Support for struct termios2 is architecture-specific in the kernel.
>Support on alpha was only added in Linux 4.20. POWER support is
>currently missing. The expectation is that the kernel will eventually
>use the generic UAPI definition for struct termios2.
>
>2019-04-09 Florian Weimer <fweimer@redhat.com>
>
> [BZ #10339]
> Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE.
> * sysdeps/unix/sysv/linux/Makefile [$(subdir) == termios] (tests):
> Add tst-termios2.
> * sysdeps/unix/sysv/linux/tst-termios2.c: New file.
> * sysdeps/unix/sysv/linux/bits/termios2-struct.h: Likewise.
> * sysdeps/unix/sysv/linux/bits/termios.h [__USE_GNU]: Include it.
> * sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h: New file.
> * sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h: Likewise.
>
>diff --git a/NEWS b/NEWS
>index b58e2469d4..5e6ecb9c7d 100644
>--- a/NEWS
>+++ b/NEWS
>@@ -18,6 +18,9 @@ Major new features:
>
> * On Linux, the gettid function has been added.
>
>+* On Linux, <termios.h> now provides a definition of struct termios2
>with
>+ the _GNU_SOURCE feature test macro.
>+
> * Minguo (Republic of China) calendar support has been added as an
> alternative calendar for the following locales: zh_TW, cmn_TW, hak_TW,
> nan_TW, lzh_TW.
>diff --git a/sysdeps/unix/sysv/linux/Makefile
>b/sysdeps/unix/sysv/linux/Makefile
>index 52ac6ad484..4cb5e4f0d2 100644
>--- a/sysdeps/unix/sysv/linux/Makefile
>+++ b/sysdeps/unix/sysv/linux/Makefile
>@@ -156,6 +156,7 @@ endif
>
> ifeq ($(subdir),termios)
> sysdep_headers += termio.h
>+tests += tst-termios2
> endif
>
> ifeq ($(subdir),posix)
>diff --git a/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5f09445e23
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition. Linux/alpha version.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+ tcflag_t c_iflag;
>+ tcflag_t c_oflag;
>+ tcflag_t c_cflag;
>+ tcflag_t c_lflag;
>+ cc_t c_cc[NCCS];
>+ cc_t c_line;
>+ speed_t c_ispeed;
>+ speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/bits/termios.h
>b/sysdeps/unix/sysv/linux/bits/termios.h
>index 997231cd03..45ac7affdf 100644
>--- a/sysdeps/unix/sysv/linux/bits/termios.h
>+++ b/sysdeps/unix/sysv/linux/bits/termios.h
>@@ -25,6 +25,10 @@ typedef unsigned int speed_t;
> typedef unsigned int tcflag_t;
>
> #include <bits/termios-struct.h>
>+#ifdef __USE_GNU
>+# include <bits/termios2-struct.h>
>+#endif
>+
> #include <bits/termios-c_cc.h>
> #include <bits/termios-c_iflag.h>
> #include <bits/termios-c_oflag.h>
>diff --git a/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5a48e45ef3
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition. Linux/generic version.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+ tcflag_t c_iflag;
>+ tcflag_t c_oflag;
>+ tcflag_t c_cflag;
>+ tcflag_t c_lflag;
>+ cc_t c_line;
>+ cc_t c_cc[NCCS];
>+ speed_t c_ispeed;
>+ speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..7c889e575c
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition. Linux/sparc version.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+ tcflag_t c_iflag;
>+ tcflag_t c_oflag;
>+ tcflag_t c_cflag;
>+ tcflag_t c_lflag;
>+ cc_t c_line;
>+ cc_t c_cc[NCCS + 2];
>+ speed_t c_ispeed;
>+ speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/tst-termios2.c
>b/sysdeps/unix/sysv/linux/tst-termios2.c
>new file mode 100644
>index 0000000000..82326a1288
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/tst-termios2.c
>@@ -0,0 +1,48 @@
>+/* Minimal test of struct termios2 definition.
>+ Copyright (C) 2019 Free Software Foundation, Inc.
>+ This file is part of the GNU C Library.
>+
>+ The GNU C Library is free software; you can redistribute it and/or
>+ modify it under the terms of the GNU Lesser General Public
>+ License as published by the Free Software Foundation; either
>+ version 2.1 of the License, or (at your option) any later version.
>+
>+ The GNU C Library is distributed in the hope that it will be
>useful,
>+ but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ Lesser General Public License for more details.
>+
>+ You should have received a copy of the GNU Lesser General Public
>+ License along with the GNU C Library. If not, see
>+ <http://www.gnu.org/licenses/>. */
>+
>+#include <termios.h>
>+#include <sys/ioctl.h>
>+
>+/* This function is never executed, but must be compiled successfully.
>+ Accessing serial ports in the test suite is problematic because
>+ they likely correspond with low-level system functionality. */
>+void
>+not_executed (int fd)
>+{
>+ /* Avoid a compilation failure if TCGETS2, TCSETS2 are not
>+ defined. */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+ struct termios2 ti;
>+ ioctl (fd, TCGETS2, &ti);
>+ ioctl (fd, TCSETS2, &ti);
>+#endif
>+}
>+
>+static int
>+do_test (void)
>+{
>+ /* Fail at run time if TCGETS2 or TCSETS2 is not defined. */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+ return 0;
>+#else
>+ return 1;
>+#endif
>+}
>+
>+#include <support/test-driver.c>
Anything blocking the POC code I provided?
Note: for PowerPC, define termios2 = kernel termios, and the ioctls as aliases.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH] Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE [BZ #10339]
From: Florian Weimer @ 2019-04-09 16:04 UTC (permalink / raw)
To: hpa; +Cc: linux-api, libc-alpha, linuxppc-dev, adhemerval.zanella
In-Reply-To: <CA8DB086-0ABB-4E37-93F3-89A64B8012DF@zytor.com>
* hpa:
> Anything blocking the POC code I provided?
I don't recall that, sorry. Would you please provide a reference?
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH v4 16/17] KVM: introduce a KVM_DESTROY_DEVICE ioctl
From: Cédric Le Goater @ 2019-04-09 14:12 UTC (permalink / raw)
To: kvm-ppc; +Cc: kvm, Paul Mackerras, Paolo Bonzini, linuxppc-dev, David Gibson
In-Reply-To: <20190320083751.27001-17-clg@kaod.org>
On 3/20/19 9:37 AM, Cédric Le Goater wrote:
> The 'destroy' method is currently used to destroy all devices when the
> VM is destroyed after the vCPUs have been freed.
>
> This new KVM ioctl exposes the same KVM device method. It acts as a
> software reset of the VM to 'destroy' selected devices when necessary
> and perform the required cleanups on the vCPUs. Called with the
> kvm->lock.
>
> The 'destroy' method could be improved by returning an error code.
I am working on a new approach for the interrupt device switching
that I will send as a replacement of patch 16 and 17. This is for
discussion and a v5 will be needed at the end.
C.
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>
> Changes since v3 :
>
> - Removed temporary TODO comment in kvm_ioctl_destroy_device()
> regarding kvm_put_kvm()
>
> Changes since v2 :
>
> - checked that device is owned by VM
>
> include/uapi/linux/kvm.h | 7 ++++++
> virt/kvm/kvm_main.c | 41 +++++++++++++++++++++++++++++++
> Documentation/virtual/kvm/api.txt | 20 +++++++++++++++
> 3 files changed, 68 insertions(+)
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 52bf74a1616e..d78fafa54274 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1183,6 +1183,11 @@ struct kvm_create_device {
> __u32 flags; /* in: KVM_CREATE_DEVICE_xxx */
> };
>
> +struct kvm_destroy_device {
> + __u32 fd; /* in: device handle */
> + __u32 flags; /* in: unused */
> +};
> +
> struct kvm_device_attr {
> __u32 flags; /* no flags currently defined */
> __u32 group; /* device-defined */
> @@ -1331,6 +1336,8 @@ struct kvm_s390_ucas_mapping {
> #define KVM_GET_DEVICE_ATTR _IOW(KVMIO, 0xe2, struct kvm_device_attr)
> #define KVM_HAS_DEVICE_ATTR _IOW(KVMIO, 0xe3, struct kvm_device_attr)
>
> +#define KVM_DESTROY_DEVICE _IOWR(KVMIO, 0xf0, struct kvm_destroy_device)
> +
> /*
> * ioctls for vcpu fds
> */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 5e2fa5c7dd1a..9601c2ddecc5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3032,6 +3032,33 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
> return 0;
> }
>
> +static int kvm_ioctl_destroy_device(struct kvm *kvm,
> + struct kvm_destroy_device *dd)
> +{
> + struct fd f;
> + struct kvm_device *dev;
> +
> + f = fdget(dd->fd);
> + if (!f.file)
> + return -EBADF;
> +
> + dev = kvm_device_from_filp(f.file);
> + fdput(f);
> +
> + if (!dev)
> + return -ENODEV;
> +
> + if (dev->kvm != kvm)
> + return -EPERM;
> +
> + mutex_lock(&kvm->lock);
> + list_del(&dev->vm_node);
> + dev->ops->destroy(dev);
> + mutex_unlock(&kvm->lock);
> +
> + return 0;
> +}
> +
> static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
> {
> switch (arg) {
> @@ -3276,6 +3303,20 @@ static long kvm_vm_ioctl(struct file *filp,
> r = 0;
> break;
> }
> + case KVM_DESTROY_DEVICE: {
> + struct kvm_destroy_device dd;
> +
> + r = -EFAULT;
> + if (copy_from_user(&dd, argp, sizeof(dd)))
> + goto out;
> +
> + r = kvm_ioctl_destroy_device(kvm, &dd);
> + if (r)
> + goto out;
> +
> + r = 0;
> + break;
> + }
> case KVM_CHECK_EXTENSION:
> r = kvm_vm_ioctl_check_extension_generic(kvm, arg);
> break;
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 8022ecce2c47..abe8433adf4f 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3874,6 +3874,26 @@ number of valid entries in the 'entries' array, which is then filled.
> 'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are currently reserved,
> userspace should not expect to get any particular value there.
>
> +4.119 KVM_DESTROY_DEVICE
> +
> +Capability: KVM_CAP_DEVICE_CTRL
> +Type: vm ioctl
> +Parameters: struct kvm_destroy_device (in)
> +Returns: 0 on success, -1 on error
> +Errors:
> + ENODEV: The device type is unknown or unsupported
> + EPERM: The device does not belong to the VM
> +
> + Other error conditions may be defined by individual device types or
> + have their standard meanings.
> +
> +Destroys an emulated device in the kernel.
> +
> +struct kvm_destroy_device {
> + __u32 fd; /* in: device handle */
> + __u32 flags; /* unused */
> +};
> +
> 5. The kvm_run structure
> ------------------------
>
>
^ permalink raw reply
* [PATCH] powerpc/mm: Define MAX_PHYSMEM_BITS for all 64-bit configs
From: Christian Zigotzky @ 2019-04-09 14:23 UTC (permalink / raw)
To: linuxppc-dev, darren, olof
Hi All,
Shall I activate FLATMEM or SPARSEMEM for the Nemo board with the PA6T-1682M SoC?
Thanks,
Christian
^ permalink raw reply
* [PATCH] powerpc/security: Show powerpc_security_features in debugfs
From: Michael Ellerman @ 2019-04-09 13:14 UTC (permalink / raw)
To: linuxppc-dev
This can be helpful for debugging problems with the security feature
flags, especially on guests where the flags come from the hypervisor
via an hcall and so can't be observed in the device tree.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/security.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index b33bafb8fcea..d6ba696d0ed0 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -104,6 +104,14 @@ static __init int barrier_nospec_debugfs_init(void)
return 0;
}
device_initcall(barrier_nospec_debugfs_init);
+
+static __init int security_feature_debugfs_init(void)
+{
+ debugfs_create_x64("security_features", 0400, powerpc_debugfs_root,
+ (u64 *)&powerpc_security_features);
+ return 0;
+}
+device_initcall(security_feature_debugfs_init);
#endif /* CONFIG_DEBUG_FS */
#ifdef CONFIG_PPC_FSL_BOOK3E
--
2.20.1
^ permalink raw reply related
* [PATCH v5 4/6] s390/pci: add support for generic boot option iommu.dma_mode
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
In-Reply-To: <20190409125308.18304-1-thunder.leizhen@huawei.com>
s390_iommu=strict is equivalent to iommu.dma_mode=strict.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 6 +++---
arch/s390/pci/pci_dma.c | 14 +++++++-------
drivers/iommu/Kconfig | 1 +
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 92d1b3151d003c2..ab8e3c4798c0a2a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1815,9 +1815,9 @@
options(such as CONFIG_IOMMU_DEFAULT_PASSTHROUGH) to
choose which mode to be used.
Note: For historical reasons, ARM64/S390/PPC/X86 have
- their specific options. Currently, only ARM64 support
- this boot option, and hope other ARCHs to use this as
- generic boot option.
+ their specific options. Currently, only ARM64/S390
+ support this boot option, and hope other ARCHs to use
+ this as generic boot option.
passthrough
Configure DMA to bypass the IOMMU by default.
lazy
diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index 9e52d1527f71495..f658ca41547eed5 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -9,6 +9,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/export.h>
+#include <linux/iommu.h>
#include <linux/iommu-helper.h>
#include <linux/dma-mapping.h>
#include <linux/vmalloc.h>
@@ -17,7 +18,6 @@
static struct kmem_cache *dma_region_table_cache;
static struct kmem_cache *dma_page_table_cache;
-static int s390_iommu_strict;
static int zpci_refresh_global(struct zpci_dev *zdev)
{
@@ -193,13 +193,13 @@ static int __dma_purge_tlb(struct zpci_dev *zdev, dma_addr_t dma_addr,
if (!zdev->tlb_refresh)
return 0;
} else {
- if (!s390_iommu_strict)
+ if (!IOMMU_DMA_MODE_IS_STRICT())
return 0;
}
ret = zpci_refresh_trans((u64) zdev->fh << 32, dma_addr,
PAGE_ALIGN(size));
- if (ret == -ENOMEM && !s390_iommu_strict) {
+ if (ret == -ENOMEM && !IOMMU_DMA_MODE_IS_STRICT()) {
/* enable the hypervisor to free some resources */
if (zpci_refresh_global(zdev))
goto out;
@@ -278,7 +278,7 @@ static dma_addr_t dma_alloc_address(struct device *dev, int size)
spin_lock_irqsave(&zdev->iommu_bitmap_lock, flags);
offset = __dma_alloc_iommu(dev, zdev->next_bit, size);
if (offset == -1) {
- if (!s390_iommu_strict) {
+ if (!IOMMU_DMA_MODE_IS_STRICT()) {
/* global flush before DMA addresses are reused */
if (zpci_refresh_global(zdev))
goto out_error;
@@ -313,7 +313,7 @@ static void dma_free_address(struct device *dev, dma_addr_t dma_addr, int size)
if (!zdev->iommu_bitmap)
goto out;
- if (s390_iommu_strict)
+ if (IOMMU_DMA_MODE_IS_STRICT())
bitmap_clear(zdev->iommu_bitmap, offset, size);
else
bitmap_set(zdev->lazy_bitmap, offset, size);
@@ -584,7 +584,7 @@ int zpci_dma_init_device(struct zpci_dev *zdev)
rc = -ENOMEM;
goto free_dma_table;
}
- if (!s390_iommu_strict) {
+ if (!IOMMU_DMA_MODE_IS_STRICT()) {
zdev->lazy_bitmap = vzalloc(zdev->iommu_pages / 8);
if (!zdev->lazy_bitmap) {
rc = -ENOMEM;
@@ -675,7 +675,7 @@ void zpci_dma_exit(void)
static int __init s390_iommu_setup(char *str)
{
if (!strncmp(str, "strict", 6))
- s390_iommu_strict = 1;
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_STRICT);
return 0;
}
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 1986f9767da488b..b7173b106cd816a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -77,6 +77,7 @@ config IOMMU_DEBUGFS
choice
prompt "IOMMU dma mode"
depends on IOMMU_API
+ default IOMMU_DEFAULT_LAZY if S390_IOMMU
default IOMMU_DEFAULT_STRICT
help
This option allows IOMMU dma mode to be chose at build time, to
--
1.8.3
^ permalink raw reply related
* [PATCH v5 5/6] powernv/iommu: add support for generic boot option iommu.dma_mode
From: Zhen Lei @ 2019-04-09 12:53 UTC (permalink / raw)
To: Jean-Philippe Brucker, John Garry, Robin Murphy, Will Deacon,
Joerg Roedel, Jonathan Corbet, linux-doc, Sebastian Ott,
Gerald Schaefer, Martin Schwidefsky, Heiko Carstens,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, David Woodhouse, iommu,
linux-kernel, linux-s390, linuxppc-dev, x86, linux-ia64
Cc: Hanjun Guo, Zhen Lei
In-Reply-To: <20190409125308.18304-1-thunder.leizhen@huawei.com>
iommu=nobypass can be replaced with iommu.dma_mode=strict.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
arch/powerpc/platforms/powernv/pci-ioda.c | 5 ++---
drivers/iommu/Kconfig | 1 +
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ab8e3c4798c0a2a..176f96032d9d62a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1815,7 +1815,7 @@
options(such as CONFIG_IOMMU_DEFAULT_PASSTHROUGH) to
choose which mode to be used.
Note: For historical reasons, ARM64/S390/PPC/X86 have
- their specific options. Currently, only ARM64/S390
+ their specific options. Currently, only ARM64/S390/PPC
support this boot option, and hope other ARCHs to use
this as generic boot option.
passthrough
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 3ead4c237ed0ec9..8862885d866418f 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -85,7 +85,6 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
va_end(args);
}
-static bool pnv_iommu_bypass_disabled __read_mostly;
static bool pci_reset_phbs __read_mostly;
static int __init iommu_setup(char *str)
@@ -95,7 +94,7 @@ static int __init iommu_setup(char *str)
while (*str) {
if (!strncmp(str, "nobypass", 8)) {
- pnv_iommu_bypass_disabled = true;
+ iommu_default_dma_mode_set(IOMMU_DMA_MODE_STRICT);
pr_info("PowerNV: IOMMU bypass window disabled.\n");
break;
}
@@ -2456,7 +2455,7 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
return rc;
}
- if (!pnv_iommu_bypass_disabled)
+ if (IOMMU_DMA_MODE_IS_PASSTHROUGH())
pnv_pci_ioda2_set_bypass(pe, true);
return 0;
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index b7173b106cd816a..5dca666b22e6cd5 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -77,6 +77,7 @@ config IOMMU_DEBUGFS
choice
prompt "IOMMU dma mode"
depends on IOMMU_API
+ default IOMMU_DEFAULT_PASSTHROUGH if (PPC_POWERNV && PCI)
default IOMMU_DEFAULT_LAZY if S390_IOMMU
default IOMMU_DEFAULT_STRICT
help
--
1.8.3
^ permalink raw reply related
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