Re: [PATCH RFC 1/5] cpu/speculation: Add 'cpu_spec_mitigations=' cmdline options

[Michael Ellerman]

Josh Poimboeuf  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

Re: [PATCH v2 3/3] ASoC: imx-audmix: fix object reference leaks in probe

[Nicolin Chen]

On Tue, Apr 09, 2019 at 11:27:42AM +, Viorel Suman wrote:
> Release the reference to the underlying device taken
> by of_find_device_by_node() call.
> 
> Signed-off-by: Viorel Suman 
> Reported-by: Julia Lawall 

Acked-by: Nicolin Chen 

Re: [PATCH v2 2/3] dt-bindings: fsl,audmix: remove "model" attribute

[Nicolin Chen]

On Tue, Apr 09, 2019 at 11:27:40AM +, Viorel Suman wrote:
> Remove "model" attribute.
> 
> Signed-off-by: Viorel Suman 

Acked-by: Nicolin Chen 

Re: [PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute

[Nicolin Chen]

On Tue, Apr 09, 2019 at 11:27:39AM +, Viorel Suman wrote:
> Use "of_device_id.data" to specify the machine driver
> instead of "model" DTS attribute.
> 
> Signed-off-by: Viorel Suman 

Acked-by: Nicolin Chen 

> ---
>  sound/soc/fsl/fsl_audmix.c | 43 +++
>  1 file changed, 23 insertions(+), 20 deletions(-)

> + priv->pdev = platform_device_register_data(>dev, mdrv, 0, NULL,
> +0);

Would you please send a separate patch to replace "pdev->dev"?

Re: [PATCH] ASoC: fsl_asrc: replace the process_option table with function

[Nicolin Chen]

On Wed, Apr 10, 2019 at 03:15:26AM +, 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, _proc, _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?

Re: [PATCH V2] ASoC: fsl_esai: replace fall-through with break

[Nicolin Chen]

On Wed, Apr 10, 2019 at 02:42:45AM +, 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 

Acked-by: Nicolin Chen 

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
> 

Re: [PATCH V1] ASoC: fsl_esai: replace fall-through with break

[Nicolin Chen]

On Tue, Apr 09, 2019 at 08:50:59PM -0700, Nicolin Chen wrote:
> On Mon, Apr 08, 2019 at 09:28:06AM +, 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 
> 
> Acked-by: Nicolin Chen 
> 
> Thanks

Oops. Acked the older version...should have gong for v2.

Please ignore it.

Re: [PATCH V1] ASoC: fsl_esai: replace fall-through with break

[Nicolin Chen]

On Mon, Apr 08, 2019 at 09:28:06AM +, 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 

Acked-by: Nicolin Chen 

Thanks

[PATCH] ASoC: fsl_asrc: replace the process_option table with function

[S.j. Wang]

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 
---
 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(_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, _proc, _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, 

[PATCH V2] ASoC: fsl_esai: replace fall-through with break

[S.j. Wang]

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 
---
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

Re: [PATCH v2] powerpc/xmon: add read-only mode

[Christopher M Riedl]

> On April 8, 2019 at 2:37 AM Andrew Donnellan  
> 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 
> > ---
> > 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 

RE: [EXT] Re: [PATCH V1] ASoC: fsl_esai: replace fall-through with break

[S.j. Wang]

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

Re: [PATCH v2] powerpc/xmon: add read-only mode

[Christopher M Riedl]

> On April 8, 2019 at 1:34 AM Oliver  wrote:
> 
> 
> On Mon, Apr 8, 2019 at 1:06 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 
> > ---
> > 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?

>
> >  

Re: [PATCH 0/4] Enabling secure boot on PowerNV systems

[Claudio Carvalho]

On 4/5/19 7:19 PM, Matthew Garrett wrote:
> On Fri, Apr 5, 2019 at 2:11 PM Claudio Carvalho  
> 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

Re: [PATCH v10 02/18] counter: Documentation: Add Generic Counter sysfs documentation

[David Lechner]

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

[PATCH 10/10] powerpc: select DYNAMIC_DEBUG_RELATIVE_POINTERS for PPC64

[Rasmus Villemoes]

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 
---
 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

[PATCH 07/10] dynamic_debug: add asm-generic implementation for DYNAMIC_DEBUG_RELATIVE_POINTERS

[Rasmus Villemoes]

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 
---
 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 ..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 
+#ifdef CONFIG_JUMP_LABEL
+#include 
+#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 

[PATCH 00/10] implement DYNAMIC_DEBUG_RELATIVE_POINTERS

[Rasmus Villemoes]

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

Re: [PATCH] Linux: Define struct termios2 in under _GNU_SOURCE [BZ #10339]

[hpa]

On April 9, 2019 4:47:30 AM MDT, Florian Weimer  wrote:
>struct termios2 is required for setting arbitrary baud rates on serial
>ports.   and  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  
>
>   [BZ #10339]
>   Linux: Define struct termios2 in  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,  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 00..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
>+   .  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include  directly; use
> 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 intspeed_t;
> typedef unsigned int  tcflag_t;
> 
> #include 
>+#ifdef __USE_GNU
>+# include 
>+#endif
>+
> #include 
> #include 
> #include 
>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 00..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 

[PATCH 2/2] powerpc/powernv: Add new opal message type

[Vasant Hegde]

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 
Signed-off-by: Vasant Hegde 
---
 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, _prd_event_nb);
+   if (rc) {
+   pr_err("%s: Couldn't register event notifier (%d)\n",
+  __func__, OPAL_MSG_PRD2);
+   return rc;
+   }
+
rc = misc_register(_prd_dev);
if (rc) {
pr_err("failed to register miscdev\n");
-- 
2.14.3

[PATCH 1/2] powerpc/powernv: Enhance opal message read interface

[Vasant Hegde]

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 
Cc: Jeremy Kerr 
Signed-off-by: Vasant Hegde 
---
 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(), 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 *));
+   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(_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

Re: [PATCH] Linux: Define struct termios2 in under _GNU_SOURCE [BZ #10339]

[hpa]

On April 9, 2019 4:47:30 AM MDT, Florian Weimer  wrote:
>struct termios2 is required for setting arbitrary baud rates on serial
>ports.   and  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  
>
>   [BZ #10339]
>   Linux: Define struct termios2 in  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,  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 00..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
>+   .  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include  directly; use
> 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 intspeed_t;
> typedef unsigned int  tcflag_t;
> 
> #include 
>+#ifdef __USE_GNU
>+# include 
>+#endif
>+
> #include 
> #include 
> #include 
>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 00..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 

Re: [PATCH] Linux: Define struct termios2 in under _GNU_SOURCE [BZ #10339]

[Florian Weimer]

* hpa:

> Anything blocking the POC code I provided?

I don't recall that, sorry.  Would you please provide a reference?

Thanks,
Florian

Re: [PATCH v4 16/17] KVM: introduce a KVM_DESTROY_DEVICE ioctl

[Cédric Le Goater]

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 
> Signed-off-by: Cédric Le Goater 
> Reviewed-by: David Gibson 
> ---
> 
>  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(>lock);
> + list_del(>vm_node);
> + dev->ops->destroy(dev);
> + mutex_unlock(>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(, argp, sizeof(dd)))
> + goto out;
> +
> + r = kvm_ioctl_destroy_device(kvm, );
> + 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
>  
>  
> 

[PATCH] powerpc/mm: Define MAX_PHYSMEM_BITS for all 64-bit configs

[Christian Zigotzky]

Hi All,

Shall I activate FLATMEM or SPARSEMEM for the Nemo board with the PA6T-1682M 
SoC?

Thanks,
Christian

[PATCH] powerpc/security: Show powerpc_security_features in debugfs

[Michael Ellerman]

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 
---
 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 *)_security_features);
+   return 0;
+}
+device_initcall(security_feature_debugfs_init);
 #endif /* CONFIG_DEBUG_FS */
 
 #ifdef CONFIG_PPC_FSL_BOOK3E
-- 
2.20.1

[PATCH v5 4/6] s390/pci: add support for generic boot option iommu.dma_mode

[Zhen Lei]

s390_iommu=strict is equivalent to iommu.dma_mode=strict.

Signed-off-by: Zhen Lei 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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(>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

[PATCH v5 5/6] powernv/iommu: add support for generic boot option iommu.dma_mode

[Zhen Lei]

iommu=nobypass can be replaced with iommu.dma_mode=strict.

Signed-off-by: Zhen Lei 
---
 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

[PATCH v5 6/6] x86/iommu: add support for generic boot option iommu.dma_mode

[Zhen Lei]

The following equivalence or replacement relationship exists:
iommu=pt <--> iommu.dma_mode=passthrough.
iommu=nopt can be replaced with iommu.dma_mode=lazy.
intel_iommu=strict <--> iommu.dma_mode=strict.
amd_iommu=fullflush <--> iommu.dma_mode=strict.

Signed-off-by: Zhen Lei 
---
 Documentation/admin-guide/kernel-parameters.txt |  6 ++---
 arch/ia64/include/asm/iommu.h   |  2 --
 arch/ia64/kernel/pci-dma.c  |  2 --
 arch/x86/include/asm/iommu.h|  1 -
 arch/x86/kernel/pci-dma.c   | 35 -
 drivers/iommu/Kconfig   |  2 +-
 drivers/iommu/amd_iommu.c   | 10 +++
 drivers/iommu/amd_iommu_init.c  |  4 +--
 drivers/iommu/amd_iommu_types.h |  6 -
 drivers/iommu/intel-iommu.c |  9 +++
 10 files changed, 31 insertions(+), 46 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 176f96032d9d62a..ca6edc529d6a614 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/S390/PPC
-   support this boot option, and hope other ARCHs to use
-   this as generic boot option.
+   their specific options, but strongly recommended switch
+   to use this one, the new ARCHs should use this generic
+   boot option.
passthrough
Configure DMA to bypass the IOMMU by default.
lazy
diff --git a/arch/ia64/include/asm/iommu.h b/arch/ia64/include/asm/iommu.h
index 7429a72f3f92199..92aceef63710861 100644
--- a/arch/ia64/include/asm/iommu.h
+++ b/arch/ia64/include/asm/iommu.h
@@ -8,10 +8,8 @@
 extern void no_iommu_init(void);
 #ifdef CONFIG_INTEL_IOMMU
 extern int force_iommu, no_iommu;
-extern int iommu_pass_through;
 extern int iommu_detected;
 #else
-#define iommu_pass_through (0)
 #define no_iommu   (1)
 #define iommu_detected (0)
 #endif
diff --git a/arch/ia64/kernel/pci-dma.c b/arch/ia64/kernel/pci-dma.c
index fe988c49f01ce6a..f5d49cd3fbb01a9 100644
--- a/arch/ia64/kernel/pci-dma.c
+++ b/arch/ia64/kernel/pci-dma.c
@@ -22,8 +22,6 @@
 int force_iommu __read_mostly;
 #endif
 
-int iommu_pass_through;
-
 static int __init pci_iommu_init(void)
 {
if (iommu_detected)
diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h
index baedab8ac5385f7..b91623d521d9f0f 100644
--- a/arch/x86/include/asm/iommu.h
+++ b/arch/x86/include/asm/iommu.h
@@ -4,7 +4,6 @@
 
 extern int force_iommu, no_iommu;
 extern int iommu_detected;
-extern int iommu_pass_through;
 
 /* 10 seconds */
 #define DMAR_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000)
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index d460998ae828514..fc64928e47cb860 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -34,21 +35,6 @@
 /* Set this to 1 if there is a HW IOMMU in the system */
 int iommu_detected __read_mostly = 0;
 
-/*
- * This variable becomes 1 if iommu=pt is passed on the kernel command line.
- * If this variable is 1, IOMMU implementations do no DMA translation for
- * devices and allow every device to access to whole physical memory. This is
- * useful if a user wants to use an IOMMU only for KVM device assignment to
- * guests and not for driver dma translation.
- * It is also possible to disable by default in kernel config, and enable with
- * iommu=nopt at boot time.
- */
-#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
-int iommu_pass_through __read_mostly = 1;
-#else
-int iommu_pass_through __read_mostly;
-#endif
-
 extern struct iommu_table_entry __iommu_table[], __iommu_table_end[];
 
 /* Dummy device used for NULL arguments (normally ISA). */
@@ -139,10 +125,23 @@ static __init int iommu_setup(char *p)
if (!strncmp(p, "soft", 4))
swiotlb = 1;
 #endif
+
+   /*
+* IOMMU implementations do no DMA translation for devices and
+* allow every device to access to whole physical memory. This
+* is useful if a user wants to use an IOMMU only for KVM
+* device assignment to guests and not for driver dma
+* translation.
+*/
if (!strncmp(p, "pt", 2))
-   iommu_pass_through = 1;
-   if (!strncmp(p, "nopt", 4))
-   

[PATCH v5 0/6] add generic boot option for IOMMU dma mode

[Zhen Lei]

v4 --> v5:
As Hanjun and Thomas Gleixner's suggestion:
1. Keep the old ARCH specific boot options no change.
2. Keep build option CONFIG_IOMMU_DEFAULT_PASSTHROUGH no change.

v4:
As Robin Murphy's suggestion:
"It's also not necessarily obvious to the user how this interacts with
IOMMU_DEFAULT_PASSTHROUGH, so if we really do go down this route, maybe it
would be better to refactor the whole lot into a single selection of something
like IOMMU_DEFAULT_MODE anyway."

In this version, I tried to normalize the IOMMU dma mode boot options for all
ARCHs. When IOMMU is enabled, there are 3 dma modes: paasthrough(bypass),
lazy(mapping but defer the IOTLB invalidation), strict. But currently each
ARCHs defined their private boot options, different with each other. For
example, to enable/disable "passthrough", ARM64 use iommu.passthrough=1/0,
X86 use iommu=pt/nopt, PPC/POWERNV use iommu=nobypass.

Zhen Lei (6):
  iommu: add generic boot option iommu.dma_mode
  iommu: add build options corresponding to iommu.dma_mode
  iommu: add iommu_default_dma_mode_get/set() helper
  s390/pci: add support for generic boot option iommu.dma_mode
  powernv/iommu: add support for generic boot option iommu.dma_mode
  x86/iommu: add support for generic boot option iommu.dma_mode

 Documentation/admin-guide/kernel-parameters.txt | 19 +++
 arch/ia64/include/asm/iommu.h   |  2 -
 arch/ia64/kernel/pci-dma.c  |  2 -
 arch/powerpc/platforms/powernv/pci-ioda.c   |  5 +-
 arch/s390/pci/pci_dma.c | 14 ++---
 arch/x86/include/asm/iommu.h|  1 -
 arch/x86/kernel/pci-dma.c   | 35 ++--
 drivers/iommu/Kconfig   | 45 ---
 drivers/iommu/amd_iommu.c   | 10 ++--
 drivers/iommu/amd_iommu_init.c  |  4 +-
 drivers/iommu/amd_iommu_types.h |  6 --
 drivers/iommu/intel-iommu.c |  9 ++-
 drivers/iommu/iommu.c   | 73 -
 include/linux/iommu.h   | 23 
 14 files changed, 176 insertions(+), 72 deletions(-)

-- 
1.8.3

[PATCH v5 3/6] iommu: add iommu_default_dma_mode_get/set() helper

[Zhen Lei]

Also add IOMMU_DMA_MODE_IS_{STRICT|LAZT|PASSTHROUGH}() to make the code
looks cleaner.

There is no functional change, just prepare for the following patches.

Signed-off-by: Zhen Lei 
---
 drivers/iommu/iommu.c | 18 ++
 include/linux/iommu.h | 18 ++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f4171bf4b46eaeb..86239dd46003fd4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -195,6 +195,17 @@ static int __init iommu_dma_mode_setup(char *str)
 }
 early_param("iommu.dma_mode", iommu_dma_mode_setup);
 
+int iommu_default_dma_mode_get(void)
+{
+   return iommu_default_dma_mode;
+}
+
+void iommu_default_dma_mode_set(int mode)
+{
+   WARN_ON(mode > IOMMU_DMA_MODE_PASSTHROUGH);
+   iommu_default_dma_mode = mode;
+}
+
 static ssize_t iommu_group_attr_show(struct kobject *kobj,
 struct attribute *__attr, char *buf)
 {
@@ -1136,9 +1147,8 @@ struct iommu_group *iommu_group_get_for_dev(struct device 
*dev)
 */
if (!group->default_domain) {
struct iommu_domain *dom;
-   int def_domain_type =
-   (iommu_default_dma_mode == IOMMU_DMA_MODE_PASSTHROUGH)
-   ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
+   int def_domain_type = IOMMU_DMA_MODE_IS_PASSTHROUGH() ?
+   IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
 
dom = __iommu_domain_alloc(dev->bus, def_domain_type);
if (!dom && def_domain_type != IOMMU_DOMAIN_DMA) {
@@ -1154,7 +1164,7 @@ struct iommu_group *iommu_group_get_for_dev(struct device 
*dev)
if (!group->domain)
group->domain = dom;
 
-   if (dom && (iommu_default_dma_mode == IOMMU_DMA_MODE_LAZY)) {
+   if (dom && IOMMU_DMA_MODE_IS_LAZY()) {
int attr = 1;
iommu_domain_set_attr(dom,
  DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index c3f4e3416176496..3668a8b3846996a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -46,6 +46,12 @@
 #define IOMMU_DMA_MODE_STRICT  0x0
 #define IOMMU_DMA_MODE_LAZY0x1
 #define IOMMU_DMA_MODE_PASSTHROUGH 0x2
+#define IOMMU_DMA_MODE_IS_STRICT() \
+   (iommu_default_dma_mode_get() == IOMMU_DMA_MODE_STRICT)
+#define IOMMU_DMA_MODE_IS_LAZY() \
+   (iommu_default_dma_mode_get() == IOMMU_DMA_MODE_LAZY)
+#define IOMMU_DMA_MODE_IS_PASSTHROUGH() \
+   (iommu_default_dma_mode_get() == IOMMU_DMA_MODE_PASSTHROUGH)
 
 struct iommu_ops;
 struct iommu_group;
@@ -421,6 +427,9 @@ static inline void dev_iommu_fwspec_set(struct device *dev,
 int iommu_probe_device(struct device *dev);
 void iommu_release_device(struct device *dev);
 
+extern int iommu_default_dma_mode_get(void);
+extern void iommu_default_dma_mode_set(int mode);
+
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
@@ -705,6 +714,15 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct 
fwnode_handle *fwnode)
return NULL;
 }
 
+static inline int iommu_default_dma_mode_get(void)
+{
+   return IOMMU_DMA_MODE_PASSTHROUGH;
+}
+
+static inline void iommu_default_dma_mode_set(int mode)
+{
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS
-- 
1.8.3

[PATCH v5 2/6] iommu: add build options corresponding to iommu.dma_mode

[Zhen Lei]

First, add build option IOMMU_DEFAULT_{LAZY|STRICT}, so that we have the
opportunity to set {lazy|strict} mode as default at build time. Then put
the three config options in an choice, make people can only choose one of
the three at a time, the same to the boot options iommu.dma_mode.

Signed-off-by: Zhen Lei 
---
 Documentation/admin-guide/kernel-parameters.txt |  6 ++--
 drivers/iommu/Kconfig   | 43 +
 drivers/iommu/iommu.c   |  4 ++-
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index f7766f8ac8b9084..92d1b3151d003c2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1811,9 +1811,9 @@
1 - Bypass the IOMMU for DMA.
unset - Use value of CONFIG_IOMMU_DEFAULT_PASSTHROUGH.
 
-   iommu.dma_mode= Configure default dma mode. if unset, use the value
-   of CONFIG_IOMMU_DEFAULT_PASSTHROUGH to determine
-   passthrough or not.
+   iommu.dma_mode= Configure default dma mode. if unset, use the build
+   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
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 6f07f3b21816c64..1986f9767da488b 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -74,17 +74,46 @@ config IOMMU_DEBUGFS
  debug/iommu directory, and then populate a subdirectory with
  entries as required.
 
-config IOMMU_DEFAULT_PASSTHROUGH
-   bool "IOMMU passthrough by default"
+choice
+   prompt "IOMMU dma mode"
depends on IOMMU_API
-help
- Enable passthrough by default, removing the need to pass in
- iommu.passthrough=on or iommu=pt through command line. If this
- is enabled, you can still disable with iommu.passthrough=off
- or iommu=nopt depending on the architecture.
+   default IOMMU_DEFAULT_STRICT
+   help
+ This option allows IOMMU dma mode to be chose at build time, to
+ override the default dma mode of each ARCHs, removing the need to
+ pass in kernel parameters through command line. You can still use the
+ generic boot option iommu.dma_mode or ARCHs specific boot options to
+ override this option again.
+
+config IOMMU_DEFAULT_PASSTHROUGH
+   bool "passthrough"
+   help
+ In this mode, the dma access through IOMMU without any addresses
+ transformation. That means, the wrong or illegal dma access can not
+ be caught, no error information will be reported.
 
  If unsure, say N here.
 
+config IOMMU_DEFAULT_LAZY
+   bool "lazy"
+   help
+ Support lazy mode, where for every IOMMU DMA unmap operation, the
+ flush operation of IOTLB and the free operation of IOVA are deferred.
+ They are only guaranteed to be done before the related IOVA will be
+ reused.
+
+config IOMMU_DEFAULT_STRICT
+   bool "strict"
+   help
+ For every IOMMU DMA unmap operation, the flush operation of IOTLB and
+ the free operation of IOVA are guaranteed to be done in the unmap
+ function.
+
+ This mode is safer than the two above, but it maybe slow in some high
+ performace scenarios.
+
+endchoice
+
 config OF_IOMMU
def_bool y
depends on OF && IOMMU_API
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index df1ce8e22385b48..f4171bf4b46eaeb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -39,8 +39,10 @@
 static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
 
-#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
+#if defined(CONFIG_IOMMU_DEFAULT_PASSTHROUGH)
 #define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_PASSTHROUGH
+#elif defined(CONFIG_IOMMU_DEFAULT_LAZY)
+#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_LAZY
 #else
 #define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_STRICT
 #endif
-- 
1.8.3

[PATCH v5 1/6] iommu: add generic boot option iommu.dma_mode

[Zhen Lei]

Currently the IOMMU dma contains 3 modes: passthrough, lazy, strict. The
passthrough mode bypass the IOMMU, the lazy mode defer the invalidation
of hardware TLBs, and the strict mode invalidate IOMMU hardware TLBs
synchronously. The three modes are mutually exclusive. But the current
boot options are confused, such as: iommu.passthrough and iommu.strict,
because they are no good to be coexist. So add iommu.dma_mode.

Signed-off-by: Zhen Lei 
---
 Documentation/admin-guide/kernel-parameters.txt | 19 
 drivers/iommu/iommu.c   | 59 -
 include/linux/iommu.h   |  5 +++
 3 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb64470d..f7766f8ac8b9084 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1811,6 +1811,25 @@
1 - Bypass the IOMMU for DMA.
unset - Use value of CONFIG_IOMMU_DEFAULT_PASSTHROUGH.
 
+   iommu.dma_mode= Configure default dma mode. if unset, use the value
+   of CONFIG_IOMMU_DEFAULT_PASSTHROUGH to determine
+   passthrough or not.
+   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.
+   passthrough
+   Configure DMA to bypass the IOMMU by default.
+   lazy
+   Request that DMA unmap operations use deferred
+   invalidation of hardware TLBs, for increased
+   throughput at the cost of reduced device isolation.
+   Will fall back to strict mode if not supported by
+   the relevant IOMMU driver.
+   strict
+   DMA unmap operations invalidate IOMMU hardware TLBs
+   synchronously.
+
io7=[HW] IO7 for Marvel based alpha systems
See comment before marvel_specify_io7 in
arch/alpha/kernel/core_marvel.c.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 109de67d5d727c2..df1ce8e22385b48 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -38,12 +38,13 @@
 
 static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
+
 #ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
+#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_PASSTHROUGH
 #else
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
+#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_STRICT
 #endif
-static bool iommu_dma_strict __read_mostly = true;
+static int iommu_default_dma_mode __read_mostly = IOMMU_DEFAULT_DMA_MODE;
 
 struct iommu_callback_data {
const struct iommu_ops *ops;
@@ -147,20 +148,51 @@ static int __init iommu_set_def_domain_type(char *str)
int ret;
 
ret = kstrtobool(str, );
-   if (ret)
-   return ret;
+   if (!ret && pt)
+   iommu_default_dma_mode = IOMMU_DMA_MODE_PASSTHROUGH;
 
-   iommu_def_domain_type = pt ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
-   return 0;
+   return ret;
 }
 early_param("iommu.passthrough", iommu_set_def_domain_type);
 
 static int __init iommu_dma_setup(char *str)
 {
-   return kstrtobool(str, _dma_strict);
+   bool strict;
+   int ret;
+
+   ret = kstrtobool(str, );
+   if (!ret)
+   iommu_default_dma_mode = strict ?
+   IOMMU_DMA_MODE_STRICT : IOMMU_DMA_MODE_LAZY;
+
+   return ret;
 }
 early_param("iommu.strict", iommu_dma_setup);
 
+static int __init iommu_dma_mode_setup(char *str)
+{
+   if (!str)
+   goto fail;
+
+   if (!strncmp(str, "passthrough", 11))
+   iommu_default_dma_mode = IOMMU_DMA_MODE_PASSTHROUGH;
+   else if (!strncmp(str, "lazy", 4))
+   iommu_default_dma_mode = IOMMU_DMA_MODE_LAZY;
+   else if (!strncmp(str, "strict", 6))
+   iommu_default_dma_mode = IOMMU_DMA_MODE_STRICT;
+   else
+   goto fail;
+
+   pr_info("Force dma mode to be %d\n", iommu_default_dma_mode);
+
+   return 0;
+
+fail:
+   pr_debug("Boot option iommu.dma_mode is incorrect, ignored\n");
+   return -EINVAL;
+}
+early_param("iommu.dma_mode", iommu_dma_mode_setup);
+
 static ssize_t iommu_group_attr_show(struct kobject *kobj,
 struct attribute *__attr, char *buf)
 {
@@ -1102,14 +1134,17 @@ struct iommu_group *iommu_group_get_for_dev(struct 
device *dev)
 */
if (!group->default_domain) 

Re: [RFC PATCH v2 10/14] kernel/watchdog: Add a function to obtain the watchdog_allowed_mask

[Peter Zijlstra]

On Wed, Feb 27, 2019 at 08:05:14AM -0800, Ricardo Neri wrote:
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 8fbfda94a67b..367aa81294ef 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -44,7 +44,7 @@ int __read_mostly soft_watchdog_user_enabled = 1;
>  int __read_mostly watchdog_thresh = 10;
>  int __read_mostly nmi_watchdog_available;
>  
> -struct cpumask watchdog_allowed_mask __read_mostly;
> +static struct cpumask watchdog_allowed_mask __read_mostly;
>  
>  struct cpumask watchdog_cpumask __read_mostly;
>  unsigned long *watchdog_cpumask_bits = cpumask_bits(_cpumask);

Hurmph, more struct cpumask, ideally this would get converted to
cpumask_var_t, I don't think we need this before the allocators work, do
we?

[PATCH v2 2/3] dt-bindings: fsl,audmix: remove "model" attribute

[Viorel Suman]

Remove "model" attribute.

Signed-off-by: Viorel Suman 
---
 Documentation/devicetree/bindings/sound/fsl,audmix.txt | 4 
 1 file changed, 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl,audmix.txt 
b/Documentation/devicetree/bindings/sound/fsl,audmix.txt
index 45f807e..840b7e0 100644
--- a/Documentation/devicetree/bindings/sound/fsl,audmix.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,audmix.txt
@@ -38,9 +38,6 @@ Device driver required properties:
  to SAI interfaces to be provided, the first SAI in the
  list being used to route the AUDMIX output.
 
-  - model  : Must contain machine driver name which will configure
- and instantiate the appropriate audio card.
-
 Device driver configuration example:
 ==
   audmix: audmix@5984 {
@@ -50,5 +47,4 @@ Device driver configuration example:
 clock-names = "ipg";
 power-domains = <_audmix>;
 dais = <>, <>;
-model = "imx-audmix";
   };
-- 
2.7.4

[PATCH v2 1/3] ASoC: fsl_audmix: remove "model" attribute

[Viorel Suman]

Use "of_device_id.data" to specify the machine driver
instead of "model" DTS attribute.

Signed-off-by: Viorel Suman 
---
 sound/soc/fsl/fsl_audmix.c | 43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index dabde03..dc802d5 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -445,13 +445,29 @@ static const struct regmap_config 
fsl_audmix_regmap_config = {
.cache_type = REGCACHE_FLAT,
 };
 
+static const struct of_device_id fsl_audmix_ids[] = {
+   {
+   .compatible = "fsl,imx8qm-audmix",
+   .data = "imx-audmix",
+   },
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
+
 static int fsl_audmix_probe(struct platform_device *pdev)
 {
struct fsl_audmix *priv;
struct resource *res;
+   const char *mdrv;
+   const struct of_device_id *of_id;
void __iomem *regs;
int ret;
-   const char *sprop;
+
+   of_id = of_match_device(fsl_audmix_ids, >dev);
+   if (!of_id || !of_id->data)
+   return -EINVAL;
+
+   mdrv = of_id->data;
 
priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -487,19 +503,12 @@ static int fsl_audmix_probe(struct platform_device *pdev)
return ret;
}
 
-   sprop = of_get_property(pdev->dev.of_node, "model", NULL);
-   if (sprop) {
-   priv->pdev = platform_device_register_data(>dev, sprop, 0,
-  NULL, 0);
-   if (IS_ERR(priv->pdev)) {
-   ret = PTR_ERR(priv->pdev);
-   dev_err(>dev,
-   "failed to register platform %s: %d\n", sprop,
-ret);
-   }
-   } else {
-   dev_err(>dev, "[model] attribute missing.\n");
-   ret = -EINVAL;
+   priv->pdev = platform_device_register_data(>dev, mdrv, 0, NULL,
+  0);
+   if (IS_ERR(priv->pdev)) {
+   ret = PTR_ERR(priv->pdev);
+   dev_err(>dev, "failed to register platform %s: %d\n",
+   mdrv, ret);
}
 
return ret;
@@ -553,12 +562,6 @@ static const struct dev_pm_ops fsl_audmix_pm = {
pm_runtime_force_resume)
 };
 
-static const struct of_device_id fsl_audmix_ids[] = {
-   { .compatible = "fsl,imx8qm-audmix", },
-   { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
-
 static struct platform_driver fsl_audmix_driver = {
.probe = fsl_audmix_probe,
.remove = fsl_audmix_remove,
-- 
2.7.4

[PATCH v2 0/3] ASoC: fsl: audmix: remove "model" attribute and fix ref leaks

[Viorel Suman]

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 (3):
  ASoC: fsl_audmix: remove "model" attribute
  dt-bindings: fsl,audmix: remove "model" attribute
  ASoC: imx-audmix: fix object reference leaks in probe

Changes since V1:
  a) Removed "model" attribute from dt-bindings documentation
  b) Adressed Daniel's comments

 .../devicetree/bindings/sound/fsl,audmix.txt   |  4 --
 sound/soc/fsl/fsl_audmix.c | 43 --
 sound/soc/fsl/imx-audmix.c |  4 ++
 3 files changed, 27 insertions(+), 24 deletions(-)

-- 
2.7.4

[PATCH v2 3/3] ASoC: imx-audmix: fix object reference leaks in probe

[Viorel Suman]

Release the reference to the underlying device taken
by of_find_device_by_node() call.

Signed-off-by: Viorel Suman 
Reported-by: Julia Lawall 
---
 sound/soc/fsl/imx-audmix.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c
index 7983bd3..9aaf3e5 100644
--- a/sound/soc/fsl/imx-audmix.c
+++ b/sound/soc/fsl/imx-audmix.c
@@ -171,6 +171,7 @@ static int imx_audmix_probe(struct platform_device *pdev)
np->full_name);
return -EINVAL;
}
+   put_device(_pdev->dev);
 
num_dai = of_count_phandle_with_args(audmix_np, "dais", NULL);
if (num_dai != FSL_AUDMIX_MAX_DAIS) {
@@ -216,6 +217,7 @@ static int imx_audmix_probe(struct platform_device *pdev)
dev_err(>dev, "failed to find SAI platform 
device\n");
return -EINVAL;
}
+   put_device(_pdev->dev);
 
dai_name = devm_kasprintf(>dev, GFP_KERNEL, "%s%s",
  fe_name_pref, args.np->full_name + 1);
@@ -280,6 +282,8 @@ static int imx_audmix_probe(struct platform_device *pdev)
dev_err(>dev, "failed to find SAI platform device\n");
return -EINVAL;
}
+   put_device(_pdev->dev);
+
priv->cpu_mclk = devm_clk_get(_pdev->dev, "mclk1");
if (IS_ERR(priv->cpu_mclk)) {
ret = PTR_ERR(priv->cpu_mclk);
-- 
2.7.4

[PATCH] Linux: Define struct termios2 in under _GNU_SOURCE [BZ #10339]

[Florian Weimer]

struct termios2 is required for setting arbitrary baud rates on serial
ports.   and  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  

[BZ #10339]
Linux: Define struct termios2 in  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,  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 00..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
+   .  */
+
+#ifndef _TERMIOS_H
+# error "Never include  directly; use  
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 
+#ifdef __USE_GNU
+# include 
+#endif
+
 #include 
 #include 
 #include 
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 00..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
+   .  */
+
+#ifndef _TERMIOS_H
+# error "Never include  directly; use  
instead."
+#endif
+
+struct termios2

Re: [PATCH 2/2] ASoC: imx-audmix: fix object reference leaks in probe

[Daniel Baluta]

On Tue, Apr 9, 2019 at 11:36 AM Viorel Suman  wrote:
>
> Release the reference to the underlying device taken
> by of_find_device_by_node() call.
>
> Signed-off-by: Viorel Suman 

Please add here the Reported-by tag pointing to Julia.

> ---
>  sound/soc/fsl/imx-audmix.c | 31 +++
>  1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c
> index 7983bd3..7c24095 100644
> --- a/sound/soc/fsl/imx-audmix.c
> +++ b/sound/soc/fsl/imx-audmix.c
> @@ -20,10 +20,7 @@
>  #include "fsl_audmix.h"
>
>  struct imx_audmix {
> -   struct platform_device *pdev;
> struct snd_soc_card card;
> -   struct platform_device *audmix_pdev;
> -   struct platform_device *out_pdev;

I am not sure why are you removing these members here. It doesn't seem to match
with patch description. If these are needed to simplify the code please do it in
another patch.

This patch should only fix one problem and that is the refleak.

thanks,
Daniel.

Re: [PATCH 1/2] ASoC: fsl_audmix: remove "model" attribute

[Daniel Baluta]

Hi Viorel,

Few comments inline.

On Tue, Apr 9, 2019 at 11:36 AM Viorel Suman  wrote:
>
> Use "of_device_id.data" to specify the machine driver,
> instead of "model" DTS attribute.



>  static int fsl_audmix_probe(struct platform_device *pdev)
>  {
> +   struct device *dev = >dev;

You might want to remove this change from the patch because it touches
a lot of lines
and it makes the review harder.

I don't see any reason to have it at all.

thanks,
Daniel.

[PATCH v1 4/4] mm/memory_hotplug: Make __remove_pages() and arch_remove_memory() never fail

[David Hildenbrand]

All callers of arch_remove_memory() ignore errors. And we should really
try to remove any errors from the memory removal path.
No more errors are reported from __remove_pages(). BUG() in s390x code
in case arch_remove_memory() is triggered. We may implement that properly
later. WARN in case powerpc code failed to remove the section mapping,
which is better than ignoring the error completely right now.

Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Yoshinori Sato 
Cc: Rich Felker 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: "H. Peter Anvin" 
Cc: x...@kernel.org
Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Mike Rapoport 
Cc: Oscar Salvador 
Cc: "Kirill A. Shutemov" 
Cc: Christophe Leroy 
Cc: Stefan Agner 
Cc: Nicholas Piggin 
Cc: Pavel Tatashin 
Cc: Vasily Gorbik 
Cc: Arun KS 
Cc: Geert Uytterhoeven 
Cc: Masahiro Yamada 
Cc: Rob Herring 
Cc: Joonsoo Kim 
Cc: Wei Yang 
Cc: Qian Cai 
Cc: Mathieu Malaterre 
Cc: linux-i...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux...@vger.kernel.org
Cc: linux-s...@vger.kernel.org
Signed-off-by: David Hildenbrand 
---
 arch/ia64/mm/init.c| 11 +++
 arch/powerpc/mm/mem.c  | 11 ---
 arch/s390/mm/init.c|  5 +++--
 arch/sh/mm/init.c  | 11 +++
 arch/x86/mm/init_32.c  |  5 +++--
 arch/x86/mm/init_64.c  | 10 +++---
 include/linux/memory_hotplug.h |  8 
 mm/memory_hotplug.c|  5 ++---
 8 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 379eb1f9adc9..d28e29103bdb 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -682,20 +682,15 @@ int arch_add_memory(int nid, u64 start, u64 size,
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(int nid, u64 start, u64 size, struct vmem_altmap 
*altmap)
+void arch_remove_memory(int nid, u64 start, u64 size,
+   struct vmem_altmap *altmap)
 {
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
struct zone *zone;
-   int ret;
 
zone = page_zone(pfn_to_page(start_pfn));
-   ret = __remove_pages(zone, start_pfn, nr_pages, altmap);
-   if (ret)
-   pr_warn("%s: Problem encountered in __remove_pages() as"
-   " ret=%d\n", __func__,  ret);
-
-   return ret;
+   __remove_pages(zone, start_pfn, nr_pages, altmap);
 }
 #endif
 #endif
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 76deaa8525db..cc9425fb9056 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -131,8 +131,8 @@ int __meminit arch_add_memory(int nid, u64 start, u64 size,
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int __meminit arch_remove_memory(int nid, u64 start, u64 size,
-   struct vmem_altmap *altmap)
+void __meminit arch_remove_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap)
 {
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
@@ -147,14 +147,13 @@ int __meminit arch_remove_memory(int nid, u64 start, u64 
size,
if (altmap)
page += vmem_altmap_offset(altmap);
 
-   ret = __remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
-   if (ret)
-   return ret;
+   __remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
 
/* Remove htab bolted mappings for this section of memory */
start = (unsigned long)__va(start);
flush_inval_dcache_range(start, start + size);
ret = remove_section_mapping(start, start + size);
+   WARN_ON_ONCE(ret);
 
/* Ensure all vmalloc mappings are flushed in case they also
 * hit that section of memory
@@ -162,8 +161,6 @@ int __meminit arch_remove_memory(int nid, u64 start, u64 
size,
vm_unmap_aliases();
 
resize_hpt_for_hotplug(memblock_phys_mem_size());
-
-   return ret;
 }
 #endif
 #endif /* CONFIG_MEMORY_HOTPLUG */
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index f5db961ad792..31b1071315d7 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -234,14 +234,15 @@ int arch_add_memory(int nid, u64 start, u64 size,
 }
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(int nid, u64 start, u64 size, struct vmem_altmap 
*altmap)
+void arch_remove_memory(int nid, u64 start, u64 size,
+   struct vmem_altmap *altmap)
 {
/*
 * There is no hardware or firmware interface which could trigger a
 * hot memory remove on s390. So there is nothing that needs to be
 * implemented.
 */
-   return -EBUSY;
+   BUG();
 }
 #endif
 #endif /* CONFIG_MEMORY_HOTPLUG */
diff --git 

Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states

[Rafael J. Wysocki]

On Fri, Apr 5, 2019 at 11:17 AM Abhishek Goel
 wrote:
>
> Currently, the cpuidle governors (menu /ladder) determine what idle state

There are three governors in 5.1-rc.

> an idling CPU should enter into based on heuristics that depend on the
> idle history on that CPU. Given that no predictive heuristic is perfect,
> there are cases where the governor predicts a shallow idle state, hoping
> that the CPU will be busy soon. However, if no new workload is scheduled
> on that CPU in the near future, the CPU will end up in the shallow state.
>
> In case of POWER, this is problematic, when the predicted state in the
> aforementioned scenario is a lite stop state, as such lite states will
> inhibit SMT folding, thereby depriving the other threads in the core from
> using the core resources.
>
> To address this, such lite states need to be autopromoted.

I don't quite agree with this statement and it doesn't even match what
the patch does AFAICS.  "Autopromotion" would be going from the given
state to a deeper one without running state selection in between, but
that's not what's going on here.

> The cpuidle-core can queue timer to correspond with the residency value of 
> the next
> available state. Thus leading to auto-promotion to a deeper idle state as
> soon as possible.

No, it doesn't automatically cause a deeper state to be used next
time.  It simply kicks the CPU out of the idle state and one more
iteration of the idle loop runs on it.  Whether or not a deeper state
will be selected in that iteration depends on the governor
computations carried out in it.

Now, this appears to be almost analogous to the "polling" state used
on x86 which uses the next idle state's target residency as a timeout.

While generally I'm not a big fan of setting up timers in the idle
loop (it sort of feels like pulling your own hair in order to get
yourself out of a swamp), if idle states like these are there in your
platform, setting up a timer to get out of them in the driver's
->enter() routine might not be particularly objectionable.  Doing that
in the core is a whole different story, though.

Generally, this adds quite a bit of complexity (on the "ugly" side of
things IMO) to the core to cover a corner case present in one
platform, while IMO it can be covered in the driver for that platform
directly.

> Signed-off-by: Abhishek Goel 
> ---
>
> v1->v2 : Removed timeout_needed and rebased to current upstream kernel
>
>  drivers/cpuidle/cpuidle.c  | 68 +-
>  drivers/cpuidle/governors/ladder.c |  3 +-
>  drivers/cpuidle/governors/menu.c   | 22 +-
>  include/linux/cpuidle.h| 10 -
>  4 files changed, 99 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 7f108309e..11ce43f19 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -36,6 +36,11 @@ static int enabled_devices;
>  static int off __read_mostly;
>  static int initialized __read_mostly;
>
> +struct auto_promotion {
> +   struct hrtimer  hrtimer;
> +   unsigned long   timeout_us;
> +};
> +
>  int cpuidle_disabled(void)
>  {
> return off;
> @@ -188,6 +193,54 @@ int cpuidle_enter_s2idle(struct cpuidle_driver *drv, 
> struct cpuidle_device *dev)
>  }
>  #endif /* CONFIG_SUSPEND */
>
> +enum hrtimer_restart auto_promotion_hrtimer_callback(struct hrtimer *hrtimer)
> +{
> +   return HRTIMER_NORESTART;
> +}
> +
> +#ifdef CONFIG_CPU_IDLE_AUTO_PROMOTION
> +DEFINE_PER_CPU(struct auto_promotion, ap);
> +
> +static void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state 
> *state)
> +{
> +   struct auto_promotion *this_ap = _cpu(ap, cpu);
> +
> +   if (state->flags & CPUIDLE_FLAG_AUTO_PROMOTION)
> +   hrtimer_start(_ap->hrtimer, 
> ns_to_ktime(this_ap->timeout_us
> +   * 1000), HRTIMER_MODE_REL_PINNED);
> +}
> +
> +static void cpuidle_auto_promotion_cancel(int cpu)
> +{
> +   struct hrtimer *hrtimer;
> +
> +   hrtimer = _cpu(ap, cpu).hrtimer;
> +   if (hrtimer_is_queued(hrtimer))
> +   hrtimer_cancel(hrtimer);
> +}
> +
> +static void cpuidle_auto_promotion_update(int cpu, unsigned long timeout)
> +{
> +   per_cpu(ap, cpu).timeout_us = timeout;
> +}
> +
> +static void cpuidle_auto_promotion_init(int cpu, struct cpuidle_driver *drv)
> +{
> +   struct auto_promotion *this_ap = _cpu(ap, cpu);
> +
> +   hrtimer_init(_ap->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +   this_ap->hrtimer.function = auto_promotion_hrtimer_callback;
> +}
> +#else
> +static inline void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state
> +   *state) { }
> +static inline void cpuidle_auto_promotion_cancel(int cpu) { }
> +static inline void cpuidle_auto_promotion_update(int cpu, unsigned long
> +   timeout) { }
> +static inline void 

Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states

[Abhishek]

On 04/09/2019 03:00 PM, Rafael J. Wysocki wrote:

On Tue, Apr 9, 2019 at 11:29 AM Abhishek  wrote:

Hi Daniel,

Thanks for such a descriptive review. I will include all the suggestions
made in my next iteration.

Please give me some time to send comments before that.

Sure, I will wait for your review.

Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states

[Abhishek]

Hi Daniel,

On 04/08/2019 07:55 PM, Daniel Axtens wrote:

Hi,

Sorry, just realised another thing I wanted to ask:


@@ -442,6 +442,26 @@ static int menu_select(struct cpuidle_driver *drv, struct 
cpuidle_device *dev,
}
}


+#ifdef CPUIDLE_FLAG_AUTO_PROMOTION

Why is this based on CPUIDLE_FLAG_ rather than CONFIG_CPU_IDLE_? Won't
this always be true, given that the flag is defined regardless of the
config option in the header?

Yeah, You are right. This should have been CONFIG_CPU_IDLE_AUTO_PROMOTION.

--Abhishek

Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states

[Rafael J. Wysocki]

On Tue, Apr 9, 2019 at 11:29 AM Abhishek  wrote:
>
> Hi Daniel,
>
> Thanks for such a descriptive review. I will include all the suggestions
> made in my next iteration.

Please give me some time to send comments before that.

Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states

[Abhishek]

Hi Daniel,

Thanks for such a descriptive review. I will include all the suggestions 
made in my next iteration.


--Abhishek

On 04/08/2019 07:42 PM, Daniel Axtens wrote:

Hi Abhishek,


Currently, the cpuidle governors (menu /ladder) determine what idle state
an idling CPU should enter into based on heuristics that depend on the
idle history on that CPU. Given that no predictive heuristic is perfect,
there are cases where the governor predicts a shallow idle state, hoping
that the CPU will be busy soon. However, if no new workload is scheduled
on that CPU in the near future, the CPU will end up in the shallow state.

In case of POWER, this is problematic, when the predicted state in the
aforementioned scenario is a lite stop state, as such lite states will
inhibit SMT folding, thereby depriving the other threads in the core from
using the core resources.

To address this, such lite states need to be autopromoted. The cpuidle-
core can queue timer to correspond with the residency value of the next
available state. Thus leading to auto-promotion to a deeper idle state as
soon as possible.


This sounds sensible to me, although I'm not really qualified to offer a
full power-management opinion on it. I have some general code questions
and comments, however, which are below:


Signed-off-by: Abhishek Goel 
---

v1->v2 : Removed timeout_needed and rebased to current upstream kernel

  drivers/cpuidle/cpuidle.c  | 68 +-
  drivers/cpuidle/governors/ladder.c |  3 +-
  drivers/cpuidle/governors/menu.c   | 22 +-
  include/linux/cpuidle.h| 10 -
  4 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 7f108309e..11ce43f19 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -36,6 +36,11 @@ static int enabled_devices;
  static int off __read_mostly;
  static int initialized __read_mostly;
  
+struct auto_promotion {

+   struct hrtimer  hrtimer;
+   unsigned long   timeout_us;
+};
+
  int cpuidle_disabled(void)
  {
return off;
@@ -188,6 +193,54 @@ int cpuidle_enter_s2idle(struct cpuidle_driver *drv, 
struct cpuidle_device *dev)
  }
  #endif /* CONFIG_SUSPEND */
  
+enum hrtimer_restart auto_promotion_hrtimer_callback(struct hrtimer *hrtimer)

+{
+   return HRTIMER_NORESTART;
+}
+
+#ifdef CONFIG_CPU_IDLE_AUTO_PROMOTION

As far as I can tell, this config flag isn't defined until the next
patch, making this dead code for now. Is this intentional?


+DEFINE_PER_CPU(struct auto_promotion, ap);

A quick grep suggests that most per-cpu variable have more descriptive
names, perhaps this one should too.


+
+static void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state *state)
+{
+   struct auto_promotion *this_ap = _cpu(ap, cpu);
+
+   if (state->flags & CPUIDLE_FLAG_AUTO_PROMOTION)
+   hrtimer_start(_ap->hrtimer, ns_to_ktime(this_ap->timeout_us
+   * 1000), HRTIMER_MODE_REL_PINNED);

Would it be clearer to have both sides of the multiplication on the same
line? i.e.
+   hrtimer_start(_ap->hrtimer,
+ ns_to_ktime(this_ap->timeout_us * 1000),
+ HRTIMER_MODE_REL_PINNED);


+}
+
+static void cpuidle_auto_promotion_cancel(int cpu)
+{
+   struct hrtimer *hrtimer;
+
+   hrtimer = _cpu(ap, cpu).hrtimer;
+   if (hrtimer_is_queued(hrtimer))
+   hrtimer_cancel(hrtimer);
+}
+
+static void cpuidle_auto_promotion_update(int cpu, unsigned long timeout)
+{
+   per_cpu(ap, cpu).timeout_us = timeout;
+}
+
+static void cpuidle_auto_promotion_init(int cpu, struct cpuidle_driver *drv)
+{
+   struct auto_promotion *this_ap = _cpu(ap, cpu);
+
+   hrtimer_init(_ap->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+   this_ap->hrtimer.function = auto_promotion_hrtimer_callback;
+}
+#else
+static inline void cpuidle_auto_promotion_start(int cpu, struct cpuidle_state
+   *state) { }
+static inline void cpuidle_auto_promotion_cancel(int cpu) { }
+static inline void cpuidle_auto_promotion_update(int cpu, unsigned long
+   timeout) { }
+static inline void cpuidle_auto_promotion_init(int cpu, struct cpuidle_driver
+   *drv) { }

Several of these have the type, then a line break, and then the name
(unsigned long\n  timeout). This is a bit harder to read, they should
probably all be on the same line.


+#endif
+
  /**
   * cpuidle_enter_state - enter the state and update stats
   * @dev: cpuidle device for this cpu
@@ -225,12 +278,17 @@ int cpuidle_enter_state(struct cpuidle_device *dev, 
struct cpuidle_driver *drv,
trace_cpu_idle_rcuidle(index, dev->cpu);
time_start = ns_to_ktime(local_clock());
  
+	cpuidle_auto_promotion_start(dev->cpu, target_state);

+

Re: [PATCH v4 10/17] KVM: PPC: Book3S HV: XIVE: add get/set accessors for the VP XIVE state

[Cédric Le Goater]

On 4/9/19 8:19 AM, Paul Mackerras wrote:
> On Wed, Mar 20, 2019 at 09:37:44AM +0100, Cédric Le Goater wrote:
>> The state of the thread interrupt management registers needs to be
>> collected for migration. These registers are cached under the
>> 'xive_saved_state.w01' field of the VCPU when the VPCU context is
>> pulled from the HW thread. An OPAL call retrieves the backup of the
>> IPB register in the underlying XIVE NVT structure and merges it in the
>> KVM state.
> 
> Since you're adding a new one_reg identifier value, you need to update
> the list in Documentation/virtual/kvm/api.txt.

yes. I missed that file.

I will keep a description of the VP state register layout in the device 
documentation file : 

  Documentation/virtual/kvm/devices/xive.txt

I don't think it belongs to kvm/api.txt file.

Thanks,

C. 

Re: [Qemu-ppc] pseries on qemu-system-ppc64le crashes in doorbell_core_ipi()

[Nicholas Piggin]

Frederic Weisbecker's on April 6, 2019 10:06 am:
> On Mon, Apr 01, 2019 at 10:38:27AM +0200, Peter Zijlstra wrote:
>> 
>> + fweisbec, who did the remote bits
>> 
>> On Sat, Mar 30, 2019 at 01:10:28PM +1000, Nicholas Piggin wrote:
>> > diff --git a/kernel/irq_work.c b/kernel/irq_work.c
>> > index 6b7cdf17ccf8..f0e539d0f879 100644
>> > --- a/kernel/irq_work.c
>> > +++ b/kernel/irq_work.c
>> > -/* Enqueue the irq work @work on the current CPU */
>> > -bool irq_work_queue(struct irq_work *work)
>> > +/*
>> > + * Enqueue the irq_work @work on @cpu unless it's already pending
>> > + * somewhere.
>> > + *
>> > + * Can be re-enqueued while the callback is still in progress.
>> > + */
>> > +bool irq_work_queue_on(struct irq_work *work, int cpu)
>> >  {
>> > +#ifndef CONFIG_SMP
>> > +  return irq_work_queue(work);
>> > +
> 
> I'd suggest to use "if (!IS_ENABLED(CONFIG_SMP))" here to avoid the large
> ifdeffery.

Sadly you can't do it because arch_send_call_function_single_ipi is
not defined for !SMP. I made your suggested name change though (with
the __ prefix because work needs to be claimed and preempt disabled).

Thanks,
Nick

Re: [PATCH 0/2] ASoC: fsl: audmix: fix two issues

[Viorel Suman]

Please ignore this series, the device bindings documentation, [1],
still contains "model" attribute. Will send V2.

[1] Documentation/devicetree/bindings/sound/fsl,audmix.txt

Regards,
Viorel

On Ma, 2019-04-09 at 08:35 +, Viorel Suman wrote:
> 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.
> Asside of this fix object reference leaks in machine probe reported
> by Julia Lawall.
> 
> Viorel Suman (2):
>   ASoC: fsl_audmix: remove "model" attribute
>   ASoC: imx-audmix: fix object reference leaks in probe
> 
>  sound/soc/fsl/fsl_audmix.c | 61 --
> 
>  sound/soc/fsl/imx-audmix.c | 31 +--
>  2 files changed, 43 insertions(+), 49 deletions(-)
> 
> -- 
> 2.7.4
> 

[PATCH 1/2] ASoC: fsl_audmix: remove "model" attribute

[Viorel Suman]

Use "of_device_id.data" to specify the machine driver,
instead of "model" DTS attribute.

Signed-off-by: Viorel Suman 
---
 sound/soc/fsl/fsl_audmix.c | 61 --
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index dabde03..2d10d8b 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -445,61 +445,70 @@ static const struct regmap_config 
fsl_audmix_regmap_config = {
.cache_type = REGCACHE_FLAT,
 };
 
+static const struct of_device_id fsl_audmix_ids[] = {
+   {
+   .compatible = "fsl,imx8qm-audmix",
+   .data = "imx-audmix",
+   },
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
+
 static int fsl_audmix_probe(struct platform_device *pdev)
 {
+   struct device *dev = >dev;
struct fsl_audmix *priv;
struct resource *res;
+   const char *mdrv;
+   const struct of_device_id *of_id;
void __iomem *regs;
int ret;
-   const char *sprop;
 
-   priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
+   of_id = of_match_device(fsl_audmix_ids, dev);
+   if (!of_id || !of_id->data)
+   return -EINVAL;
+
+   mdrv = of_id->data;
+
+   priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
 
/* Get the addresses */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   regs = devm_ioremap_resource(>dev, res);
+   regs = devm_ioremap_resource(dev, res);
if (IS_ERR(regs))
return PTR_ERR(regs);
 
-   priv->regmap = devm_regmap_init_mmio_clk(>dev, "ipg", regs,
+   priv->regmap = devm_regmap_init_mmio_clk(dev, "ipg", regs,
 _audmix_regmap_config);
if (IS_ERR(priv->regmap)) {
-   dev_err(>dev, "failed to init regmap\n");
+   dev_err(dev, "failed to init regmap\n");
return PTR_ERR(priv->regmap);
}
 
-   priv->ipg_clk = devm_clk_get(>dev, "ipg");
+   priv->ipg_clk = devm_clk_get(dev, "ipg");
if (IS_ERR(priv->ipg_clk)) {
-   dev_err(>dev, "failed to get ipg clock\n");
+   dev_err(dev, "failed to get ipg clock\n");
return PTR_ERR(priv->ipg_clk);
}
 
platform_set_drvdata(pdev, priv);
-   pm_runtime_enable(>dev);
+   pm_runtime_enable(dev);
 
-   ret = devm_snd_soc_register_component(>dev, _audmix_component,
+   ret = devm_snd_soc_register_component(dev, _audmix_component,
  fsl_audmix_dai,
  ARRAY_SIZE(fsl_audmix_dai));
if (ret) {
-   dev_err(>dev, "failed to register ASoC DAI\n");
+   dev_err(dev, "failed to register ASoC DAI\n");
return ret;
}
 
-   sprop = of_get_property(pdev->dev.of_node, "model", NULL);
-   if (sprop) {
-   priv->pdev = platform_device_register_data(>dev, sprop, 0,
-  NULL, 0);
-   if (IS_ERR(priv->pdev)) {
-   ret = PTR_ERR(priv->pdev);
-   dev_err(>dev,
-   "failed to register platform %s: %d\n", sprop,
-ret);
-   }
-   } else {
-   dev_err(>dev, "[model] attribute missing.\n");
-   ret = -EINVAL;
+   priv->pdev = platform_device_register_data(dev, mdrv, 0, NULL, 0);
+   if (IS_ERR(priv->pdev)) {
+   ret = PTR_ERR(priv->pdev);
+   dev_err(dev, "failed to register platform %s: %d\n",
+   mdrv, ret);
}
 
return ret;
@@ -553,12 +562,6 @@ static const struct dev_pm_ops fsl_audmix_pm = {
pm_runtime_force_resume)
 };
 
-static const struct of_device_id fsl_audmix_ids[] = {
-   { .compatible = "fsl,imx8qm-audmix", },
-   { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
-
 static struct platform_driver fsl_audmix_driver = {
.probe = fsl_audmix_probe,
.remove = fsl_audmix_remove,
-- 
2.7.4

[PATCH 0/2] ASoC: fsl: audmix: fix two issues

[Viorel Suman]

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.
Asside of this fix object reference leaks in machine probe reported
by Julia Lawall.

Viorel Suman (2):
  ASoC: fsl_audmix: remove "model" attribute
  ASoC: imx-audmix: fix object reference leaks in probe

 sound/soc/fsl/fsl_audmix.c | 61 --
 sound/soc/fsl/imx-audmix.c | 31 +--
 2 files changed, 43 insertions(+), 49 deletions(-)

-- 
2.7.4

[PATCH 2/2] ASoC: imx-audmix: fix object reference leaks in probe

[Viorel Suman]

Release the reference to the underlying device taken
by of_find_device_by_node() call.

Signed-off-by: Viorel Suman 
---
 sound/soc/fsl/imx-audmix.c | 31 +++
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c
index 7983bd3..7c24095 100644
--- a/sound/soc/fsl/imx-audmix.c
+++ b/sound/soc/fsl/imx-audmix.c
@@ -20,10 +20,7 @@
 #include "fsl_audmix.h"
 
 struct imx_audmix {
-   struct platform_device *pdev;
struct snd_soc_card card;
-   struct platform_device *audmix_pdev;
-   struct platform_device *out_pdev;
struct clk *cpu_mclk;
int num_dai;
struct snd_soc_dai_link *dai;
@@ -144,7 +141,7 @@ static struct snd_soc_ops imx_audmix_be_ops = {
 static int imx_audmix_probe(struct platform_device *pdev)
 {
struct device_node *np = pdev->dev.of_node;
-   struct device_node *audmix_np = NULL, *out_cpu_np = NULL;
+   struct device_node *audmix_np = NULL;
struct platform_device *audmix_pdev = NULL;
struct platform_device *cpu_pdev;
struct of_phandle_args args;
@@ -171,6 +168,7 @@ static int imx_audmix_probe(struct platform_device *pdev)
np->full_name);
return -EINVAL;
}
+   put_device(_pdev->dev);
 
num_dai = of_count_phandle_with_args(audmix_np, "dais", NULL);
if (num_dai != FSL_AUDMIX_MAX_DAIS) {
@@ -216,6 +214,7 @@ static int imx_audmix_probe(struct platform_device *pdev)
dev_err(>dev, "failed to find SAI platform 
device\n");
return -EINVAL;
}
+   put_device(_pdev->dev);
 
dai_name = devm_kasprintf(>dev, GFP_KERNEL, "%s%s",
  fe_name_pref, args.np->full_name + 1);
@@ -223,7 +222,14 @@ static int imx_audmix_probe(struct platform_device *pdev)
dev_info(pdev->dev.parent, "DAI FE name:%s\n", dai_name);
 
if (i == 0) {
-   out_cpu_np = args.np;
+   priv->cpu_mclk = devm_clk_get(_pdev->dev, "mclk1");
+   if (IS_ERR(priv->cpu_mclk)) {
+   ret = PTR_ERR(priv->cpu_mclk);
+   dev_err(_pdev->dev,
+   "failed to get DAI mclk1: %d\n", ret);
+   return -EINVAL;
+   }
+
capture_dai_name =
devm_kasprintf(>dev, GFP_KERNEL, "%s %s",
   dai_name, "CPU-Capture");
@@ -275,21 +281,6 @@ static int imx_audmix_probe(struct platform_device *pdev)
priv->dapm_routes[2 * num_dai + i].sink   = capture_dai_name;
}
 
-   cpu_pdev = of_find_device_by_node(out_cpu_np);
-   if (!cpu_pdev) {
-   dev_err(>dev, "failed to find SAI platform device\n");
-   return -EINVAL;
-   }
-   priv->cpu_mclk = devm_clk_get(_pdev->dev, "mclk1");
-   if (IS_ERR(priv->cpu_mclk)) {
-   ret = PTR_ERR(priv->cpu_mclk);
-   dev_err(_pdev->dev, "failed to get DAI mclk1: %d\n", ret);
-   return -EINVAL;
-   }
-
-   priv->audmix_pdev = audmix_pdev;
-   priv->out_pdev  = cpu_pdev;
-
priv->card.dai_link = priv->dai;
priv->card.num_links = priv->num_dai;
priv->card.codec_conf = priv->dai_conf;
-- 
2.7.4

Re: [PATCH] powerpc/mm: Define MAX_PHYSMEM_BITS for all 64-bit configs

[Aneesh Kumar K.V]

Michael Ellerman  writes:

> The recent commit 8bc086899816 ("powerpc/mm: Only define
> MAX_PHYSMEM_BITS in SPARSEMEM configurations") removed our definition
> of MAX_PHYSMEM_BITS when SPARSEMEM is disabled.
>
> This inadvertently broke some 64-bit FLATMEM using configs with eg:
>
>   arch/powerpc/include/asm/book3s/64/mmu-hash.h:584:6: error: 
> "MAX_PHYSMEM_BITS" is not defined, evaluates to 0
>#if (MAX_PHYSMEM_BITS > MAX_EA_BITS_PER_CONTEXT)
> ^~~~
>
> Fix it by making sure we define MAX_PHYSMEM_BITS for all 64-bit
> configs regardless of SPARSEMEM.


Reviewed-by: Aneesh Kumar K.V 

But I still like the patch I posted which move this to the correct
platform header.

>
> Fixes: 8bc086899816 ("powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM 
> configurations")
> Reported-by: Andreas Schwab 
> Reported-by: Hugh Dickins 
> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/include/asm/mmu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> index 598cdcdd1355..8ddd4a91bdc1 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -352,7 +352,7 @@ static inline bool strict_kernel_rwx_enabled(void)
>  #if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) 
> &&\
>   defined (CONFIG_PPC_64K_PAGES)
>  #define MAX_PHYSMEM_BITS51
> -#elif defined(CONFIG_SPARSEMEM)
> +#elif defined(CONFIG_PPC64)
>  #define MAX_PHYSMEM_BITS46
>  #endif
>
> -- 
> 2.20.1

Re: [PATCH v2] powerpc/64s/radix: Fix radix segment exception handling

[Aneesh Kumar K.V]

Nicholas Piggin  writes:

> Commit 48e7b76957 ("powerpc/64s/hash: Convert SLB miss handlers to C")
> broke the radix-mode segment exception handler. In radix mode, this is
> exception is not an SLB miss, rather it signals that the EA is outside
> the range translated by any page table.
>
> The commit lost the radix feature alternate code patch, which can
> cause faults to some EAs to kernel BUG at arch/powerpc/mm/slb.c:639!
>
> The original radix code would send faults to slb_miss_large_addr,
> which would end up faulting due to slb_addr_limit being 0. This patch
> sends radix directly to do_bad_slb_fault, which is a bit clearer.
>

Reviewed-by: Aneesh Kumar K.V 

> Fixes: 48e7b76957 ("powerpc/64s/hash: Convert SLB miss handlers to C")
> Cc: Aneesh Kumar K.V 
> Reported-by: Anton Blanchard 
> Signed-off-by: Nicholas Piggin 
> ---
> - Add a selftests that triggers the crash
>
>  arch/powerpc/kernel/exceptions-64s.S  | 12 +++
>  tools/testing/selftests/powerpc/mm/Makefile   |  3 +-
>  .../selftests/powerpc/mm/access_tests.c   | 94 +++
>  3 files changed, 108 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/powerpc/mm/access_tests.c
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S 
> b/arch/powerpc/kernel/exceptions-64s.S
> index a5b8fbae56a0..9481a117e242 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -656,11 +656,17 @@ EXC_COMMON_BEGIN(data_access_slb_common)
>   ld  r4,PACA_EXSLB+EX_DAR(r13)
>   std r4,_DAR(r1)
>   addir3,r1,STACK_FRAME_OVERHEAD
> +BEGIN_MMU_FTR_SECTION
> + /* HPT case, do SLB fault */
>   bl  do_slb_fault
>   cmpdi   r3,0
>   bne-1f
>   b   fast_exception_return
>  1:   /* Error case */
> +MMU_FTR_SECTION_ELSE
> + /* Radix case, access is outside page table range */
> + li  r3,-EFAULT
> +ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>   std r3,RESULT(r1)
>   bl  save_nvgprs
>   RECONCILE_IRQ_STATE(r10, r11)
> @@ -705,11 +711,17 @@ EXC_COMMON_BEGIN(instruction_access_slb_common)
>   EXCEPTION_PROLOG_COMMON(0x480, PACA_EXSLB)
>   ld  r4,_NIP(r1)
>   addir3,r1,STACK_FRAME_OVERHEAD
> +BEGIN_MMU_FTR_SECTION
> + /* HPT case, do SLB fault */
>   bl  do_slb_fault
>   cmpdi   r3,0
>   bne-1f
>   b   fast_exception_return
>  1:   /* Error case */
> +MMU_FTR_SECTION_ELSE
> + /* Radix case, access is outside page table range */
> + li  r3,-EFAULT
> +ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>   std r3,RESULT(r1)
>   bl  save_nvgprs
>   RECONCILE_IRQ_STATE(r10, r11)
> diff --git a/tools/testing/selftests/powerpc/mm/Makefile 
> b/tools/testing/selftests/powerpc/mm/Makefile
> index 43d68420e363..68b7add5086d 100644
> --- a/tools/testing/selftests/powerpc/mm/Makefile
> +++ b/tools/testing/selftests/powerpc/mm/Makefile
> @@ -2,7 +2,7 @@
>  noarg:
>   $(MAKE) -C ../
>  
> -TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors 
> wild_bctr
> +TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors 
> wild_bctr access_tests
>  TEST_GEN_FILES := tempfile
>  
>  top_srcdir = ../../../../..
> @@ -13,6 +13,7 @@ $(TEST_GEN_PROGS): ../harness.c
>  $(OUTPUT)/prot_sao: ../utils.c
>  
>  $(OUTPUT)/wild_bctr: CFLAGS += -m64
> +$(OUTPUT)/access_tests: CFLAGS += -m64
>  
>  $(OUTPUT)/tempfile:
>   dd if=/dev/zero of=$@ bs=64k count=1
> diff --git a/tools/testing/selftests/powerpc/mm/access_tests.c 
> b/tools/testing/selftests/powerpc/mm/access_tests.c
> new file mode 100644
> index ..ad300d7d9d43
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/mm/access_tests.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2017 John Sperbeck
> + *
> + * Test faults to "interesting" locations.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "utils.h"
> +
> +#define PAGE_SIZE(64*1024)
> +#define TB   (1024ULL*1024*1024*1024)
> +static volatile bool faulted;
> +static volatile int si_code;
> +
> +static void segv_handler(int n, siginfo_t *info, void *ctxt_v)
> +{
> + ucontext_t *ctxt = (ucontext_t *)ctxt_v;
> + struct pt_regs *regs = ctxt->uc_mcontext.regs;
> +
> + faulted = true;
> + si_code = info->si_code;
> + regs->nip += 4;
> +}
> +
> +int test_segv_errors(void)
> +{
> + struct sigaction act = {
> + .sa_sigaction = segv_handler,
> + .sa_flags = SA_SIGINFO,
> + };
> + static unsigned long ptrs[] = {
> + 0x0f00ULL, /* Radix Q0 out of pgtable range */
> + 0x4000ULL, /* Radix Q1 */
> + 0x4f00ULL, /* Radix Q1 out of pgtable range */
> + 0x8000ULL, /* Radix Q2 */
> + 

Re: [PATCH v4 10/17] KVM: PPC: Book3S HV: XIVE: add get/set accessors for the VP XIVE state

[Paul Mackerras]

On Wed, Mar 20, 2019 at 09:37:44AM +0100, Cédric Le Goater wrote:
> The state of the thread interrupt management registers needs to be
> collected for migration. These registers are cached under the
> 'xive_saved_state.w01' field of the VCPU when the VPCU context is
> pulled from the HW thread. An OPAL call retrieves the backup of the
> IPB register in the underlying XIVE NVT structure and merges it in the
> KVM state.

Since you're adding a new one_reg identifier value, you need to update
the list in Documentation/virtual/kvm/api.txt.

Paul.

Re: [PATCH] powerpc/mm/radix: Do slb preload only with hash translation mode

[Nicholas Piggin]

Aneesh Kumar K.V's on April 9, 2019 2:03 pm:
> Add radix_enabled check to avoid slb preload with radix translation.
> 
> Signed-off-by: Aneesh Kumar K.V 

Acked-by: Nicholas Piggin 

> ---
> 
> Without this we result in kernel crash as we move hash related variables
> out of mm_context_t
> 
>  arch/powerpc/kernel/process.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index dd9e0d5386ee..f7b2e3b3db28 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1729,7 +1729,8 @@ void start_thread(struct pt_regs *regs, unsigned long 
> start, unsigned long sp)
>   unsigned long load_addr = regs->gpr[2]; /* saved by ELF_PLAT_INIT */
>  
>  #ifdef CONFIG_PPC_BOOK3S_64
> - preload_new_slb_context(start, sp);
> + if (!radix_enabled())
> + preload_new_slb_context(start, sp);
>  #endif
>  #endif
>  
> -- 
> 2.20.1
> 
> 

[PATCH] powerpc/mm: Define MAX_PHYSMEM_BITS for all 64-bit configs

[Michael Ellerman]

The recent commit 8bc086899816 ("powerpc/mm: Only define
MAX_PHYSMEM_BITS in SPARSEMEM configurations") removed our definition
of MAX_PHYSMEM_BITS when SPARSEMEM is disabled.

This inadvertently broke some 64-bit FLATMEM using configs with eg:

  arch/powerpc/include/asm/book3s/64/mmu-hash.h:584:6: error: 
"MAX_PHYSMEM_BITS" is not defined, evaluates to 0
   #if (MAX_PHYSMEM_BITS > MAX_EA_BITS_PER_CONTEXT)
^~~~

Fix it by making sure we define MAX_PHYSMEM_BITS for all 64-bit
configs regardless of SPARSEMEM.

Fixes: 8bc086899816 ("powerpc/mm: Only define MAX_PHYSMEM_BITS in SPARSEMEM 
configurations")
Reported-by: Andreas Schwab 
Reported-by: Hugh Dickins 
Signed-off-by: Michael Ellerman 
---
 arch/powerpc/include/asm/mmu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 598cdcdd1355..8ddd4a91bdc1 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -352,7 +352,7 @@ static inline bool strict_kernel_rwx_enabled(void)
 #if defined(CONFIG_SPARSEMEM_VMEMMAP) && defined(CONFIG_SPARSEMEM_EXTREME) &&  
\
defined (CONFIG_PPC_64K_PAGES)
 #define MAX_PHYSMEM_BITS51
-#elif defined(CONFIG_SPARSEMEM)
+#elif defined(CONFIG_PPC64)
 #define MAX_PHYSMEM_BITS46
 #endif
 
-- 
2.20.1