Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain bindings

[Geert Uytterhoeven]

Hi Rob, Jolly,

On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring  wrote:
> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
>> Add documentation to describe ZynqMP power domain bindings.
>>
>> Signed-off-by: Jolly Shah 
>> Signed-off-by: Rajan Vaja 
>> ---
>>  .../devicetree/bindings/power/zynqmp-genpd.txt | 46 
>> ++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt 
>> b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>> new file mode 100644
>> index 000..25f9711
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt

>> +This node contains a number of subnodes, each representing a single PM 
>> domain
>> +that PM domain consumer devices reference.
>> +
>> +== PM Domain Nodes ==
>> +
>> +Required properties:
>> + - #power-domain-cells: Number of cells in a PM domain specifier. Must be 0.
>> + - pd-id: List of domain identifiers of as defined by platform firmware. 
>> These
>> +   identifiers are passed to the PM firmware.
>> +
>> +Example:
>> + zynqmp-genpd {
>> + compatible = "xlnx,zynqmp-genpd";
>
> What's the control interface for controlling the domains?
>> +
>> + pd_usb0: pd-usb0 {
>> + pd-id = <22>;
>> + #power-domain-cells = <0>;
>
> There's no need for all these sub nodes. Make #power-domain-cells 1 and
> put the id in the cell value.

That was my first reaction, too...
>
>> + };
>> +
>> + pd_sata: pd-sata {
>> + pd-id = <28>;
>> + #power-domain-cells = <0>;
>> + };
>> +
>> + pd_gpu: pd-gpu {
>> + pd-id = <58 20 21>;

... until I saw the above.
Controlling the GPU power area requires controlling 3 physical areas?

However, doing it this way may bite you in the future, if a need arises to
control a subset. And what about power up/down order?

>> + #power-domain-cells = <0x0>;
>> + };
>> + };

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ipvs: use true and false for boolean values

[Simon Horman]

On Mon, Mar 05, 2018 at 03:35:57PM -0600, Gustavo A. R. Silva wrote:
> Assign true or false to boolean variables instead of an integer value.
> 
> This issue was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva 

Signed-off-by: Simon Horman 

Pablo, could you take this one?

Re: [PATCH 1/3 RESEND] tpm: add longer timeouts for creation commands.

[Jarkko Sakkinen]

On Mon, Mar 05, 2018 at 06:04:56PM +, Winkler, Tomas wrote:
> > 
> > On Mon, Mar 05, 2018 at 01:09:09PM +, Winkler, Tomas wrote:
> > > > enum tpm_duration {
> > > > TPM_DURATION_DEFAULT = 2000,
> > > > TPM_DURATION_LONG = 30,
> > > > };
> > > >
> > > How is this aligned with the spec PTP spec?
> > 
> > For TPM 2.0 that spec only partially defines durations for CCs and thus our
> > look up table is already kind "flakky". In a sense that the default 
> > duration is
> > upper limit for spec defined durations.
> 
> The timeouts for LONG and MEDIUM is defined by the  PTP spec,  we need to 
> maintain those as those effect the system.
> The UNDEFINED and LONG LONG is the implementation choice we driver from 
> empirical data we have so far.

Where can be get this empirical data?

You are not only adding 30s delay but also turning the 2s delay to 12s
delay.

IMHO we could very well use PTP LONG for all commands as the timeout.
Why that would not work?

/Jarkko

Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain bindings

[Marek Szyprowski]

Hi All,

On 2018-03-06 08:59, Geert Uytterhoeven wrote:

Hi Rob, Jolly,

On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring  wrote:

On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:

Add documentation to describe ZynqMP power domain bindings.

Signed-off-by: Jolly Shah 
Signed-off-by: Rajan Vaja 
---
  .../devicetree/bindings/power/zynqmp-genpd.txt | 46 ++
  1 file changed, 46 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt

diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt 
b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
new file mode 100644
index 000..25f9711
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
+This node contains a number of subnodes, each representing a single PM domain
+that PM domain consumer devices reference.
+
+== PM Domain Nodes ==
+
+Required properties:
+ - #power-domain-cells: Number of cells in a PM domain specifier. Must be 0.
+ - pd-id: List of domain identifiers of as defined by platform firmware. These
+   identifiers are passed to the PM firmware.
+
+Example:
+ zynqmp-genpd {
+ compatible = "xlnx,zynqmp-genpd";

What's the control interface for controlling the domains?

+
+ pd_usb0: pd-usb0 {
+ pd-id = <22>;
+ #power-domain-cells = <0>;

There's no need for all these sub nodes. Make #power-domain-cells 1 and
put the id in the cell value.

That was my first reaction, too...

+ };
+
+ pd_sata: pd-sata {
+ pd-id = <28>;
+ #power-domain-cells = <0>;
+ };
+
+ pd_gpu: pd-gpu {
+ pd-id = <58 20 21>;

... until I saw the above.
Controlling the GPU power area requires controlling 3 physical areas?

However, doing it this way may bite you in the future, if a need arises to
control a subset. And what about power up/down order?


What about defining 3 separate domains and arranging them in parent-child
relationship? generic power domains already supports that and this allows
to nicely define the power on/off order.


+ #power-domain-cells = <0x0>;
+ };
+ };


Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

RE: [PATCH 1/3 RESEND] tpm: add longer timeouts for creation commands.

[Winkler, Tomas]

> 
> On Mon, Mar 05, 2018 at 01:09:09PM +, Winkler, Tomas wrote:
> > Why you need cover letter?  What are u missing in the patch description
> 
> If you submit a *patch set* I *require* a cover letter, yes.

It's good but it is not must, you are inventing your own rules.

Thanks
Tomas

[PATCH v2 2/3] ARM: dts: STi: Remove useless stdout-path for STi boards

[patrice.chotard]

From: Patrice Chotard 

As serial interface is already specified into bootargs,
stdout-path can be removed.

Signed-off-by: Patrice Chotard 
---

v2: _ none

 arch/arm/boot/dts/stih407-b2120.dts | 1 -
 arch/arm/boot/dts/stih410-b2120.dts | 1 -
 arch/arm/boot/dts/stih410-b2260.dts | 1 -
 arch/arm/boot/dts/stih418-b2199.dts | 1 -
 4 files changed, 4 deletions(-)

diff --git a/arch/arm/boot/dts/stih407-b2120.dts 
b/arch/arm/boot/dts/stih407-b2120.dts
index 2c4d6033b448..074b4cd0ca89 100644
--- a/arch/arm/boot/dts/stih407-b2120.dts
+++ b/arch/arm/boot/dts/stih407-b2120.dts
@@ -15,7 +15,6 @@
 
chosen {
bootargs = "console=serial0,115200 clk_ignore_unused";
-   stdout-path = &sbc_serial0;
};
 
memory@4000 {
diff --git a/arch/arm/boot/dts/stih410-b2120.dts 
b/arch/arm/boot/dts/stih410-b2120.dts
index 5422850641e8..eae3050984d0 100644
--- a/arch/arm/boot/dts/stih410-b2120.dts
+++ b/arch/arm/boot/dts/stih410-b2120.dts
@@ -15,7 +15,6 @@
 
chosen {
bootargs = "console=serial0,115200 clk_ignore_unused";
-   stdout-path = &sbc_serial0;
};
 
memory@4000 {
diff --git a/arch/arm/boot/dts/stih410-b2260.dts 
b/arch/arm/boot/dts/stih410-b2260.dts
index ca347160e35d..c26e388bd1a4 100644
--- a/arch/arm/boot/dts/stih410-b2260.dts
+++ b/arch/arm/boot/dts/stih410-b2260.dts
@@ -16,7 +16,6 @@
 
chosen {
bootargs = "console=serial1,115200 clk_ignore_unused";
-   stdout-path = &uart1;
};
 
memory@4000 {
diff --git a/arch/arm/boot/dts/stih418-b2199.dts 
b/arch/arm/boot/dts/stih418-b2199.dts
index dbf7bb704a1a..d948f774fee7 100644
--- a/arch/arm/boot/dts/stih418-b2199.dts
+++ b/arch/arm/boot/dts/stih418-b2199.dts
@@ -15,7 +15,6 @@
 
chosen {
bootargs = "console=serial0,115200 clk_ignore_unused";
-   stdout-path = &sbc_serial0;
};
 
memory@4000 {
-- 
1.9.1

[PATCH v2 3/3] tty: st-asc: Update tty alias

[patrice.chotard]

From: Patrice Chotard 

Since dtc v1.4.6-9-gaadd0b65c987, aliases property name
must include only lowercase and '-'.

After having updated all STi boards serial aliases from "ttyASN"
to "serialN", st-asc driver need to be updated accordingly as tty
aliases id is retrieved using of_alias_get_id(np, ASC_SERIAL_NAME);

Signed-off-by: Patrice Chotard 
---

v2: _ update st-asc driver with "serial" alias prefix


 drivers/tty/serial/st-asc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index c763253514e9..f9372b690e54 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -29,7 +29,7 @@
 #include 
 
 #define DRIVER_NAME "st-asc"
-#define ASC_SERIAL_NAME "ttyAS"
+#define ASC_SERIAL_NAME "serial"
 #define ASC_FIFO_SIZE 16
 #define ASC_MAX_PORTS 8
 
-- 
1.9.1

[PATCH v2 1/3] ARM: dts: STi: Fix aliases property name for STi boards

[patrice.chotard]

From: Patrice Chotard 

Update serial aliases from "ttyASN" to more common "serialN".

Since dtc v1.4.6-9-gaadd0b65c987, aliases property name must
be lowercase only. This allows to fix following dtc warnings:

arch/arm/boot/dts/stih418-b2199.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'
arch/arm/boot/dts/stih407-b2120.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'
arch/arm/boot/dts/stih410-b2260.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'
arch/arm/boot/dts/stih410-b2120.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'

Signed-off-by: Patrice Chotard 
---

v2: _ use serialN instead of ttyasN aliases to not break ABI


 arch/arm/boot/dts/stih407-b2120.dts | 4 ++--
 arch/arm/boot/dts/stih410-b2120.dts | 4 ++--
 arch/arm/boot/dts/stih410-b2260.dts | 4 ++--
 arch/arm/boot/dts/stih418-b2199.dts | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/stih407-b2120.dts 
b/arch/arm/boot/dts/stih407-b2120.dts
index de3c8bf129b5..2c4d6033b448 100644
--- a/arch/arm/boot/dts/stih407-b2120.dts
+++ b/arch/arm/boot/dts/stih407-b2120.dts
@@ -14,7 +14,7 @@
compatible = "st,stih407-b2120", "st,stih407";
 
chosen {
-   bootargs = "console=ttyAS0,115200 clk_ignore_unused";
+   bootargs = "console=serial0,115200 clk_ignore_unused";
stdout-path = &sbc_serial0;
};
 
@@ -24,7 +24,7 @@
};
 
aliases {
-   ttyAS0 = &sbc_serial0;
+   serial0 = &sbc_serial0;
ethernet0 = ðernet0;
};
 
diff --git a/arch/arm/boot/dts/stih410-b2120.dts 
b/arch/arm/boot/dts/stih410-b2120.dts
index 0a59b7b0f4b2..5422850641e8 100644
--- a/arch/arm/boot/dts/stih410-b2120.dts
+++ b/arch/arm/boot/dts/stih410-b2120.dts
@@ -14,7 +14,7 @@
compatible = "st,stih410-b2120", "st,stih410";
 
chosen {
-   bootargs = "console=ttyAS0,115200 clk_ignore_unused";
+   bootargs = "console=serial0,115200 clk_ignore_unused";
stdout-path = &sbc_serial0;
};
 
@@ -24,7 +24,7 @@
};
 
aliases {
-   ttyAS0 = &sbc_serial0;
+   serial0 = &sbc_serial0;
ethernet0 = ðernet0;
};
 
diff --git a/arch/arm/boot/dts/stih410-b2260.dts 
b/arch/arm/boot/dts/stih410-b2260.dts
index feb8834478fa..ca347160e35d 100644
--- a/arch/arm/boot/dts/stih410-b2260.dts
+++ b/arch/arm/boot/dts/stih410-b2260.dts
@@ -15,7 +15,7 @@
compatible = "st,stih410-b2260", "st,stih410";
 
chosen {
-   bootargs = "console=ttyAS1,115200 clk_ignore_unused";
+   bootargs = "console=serial1,115200 clk_ignore_unused";
stdout-path = &uart1;
};
 
@@ -25,7 +25,7 @@
};
 
aliases {
-   ttyAS1 = &uart1;
+   serial1 = &uart1;
ethernet0 = ðernet0;
};
 
diff --git a/arch/arm/boot/dts/stih418-b2199.dts 
b/arch/arm/boot/dts/stih418-b2199.dts
index 39b4db2e3507..dbf7bb704a1a 100644
--- a/arch/arm/boot/dts/stih418-b2199.dts
+++ b/arch/arm/boot/dts/stih418-b2199.dts
@@ -14,7 +14,7 @@
compatible = "st,stih418-b2199", "st,stih418";
 
chosen {
-   bootargs = "console=ttyAS0,115200 clk_ignore_unused";
+   bootargs = "console=serial0,115200 clk_ignore_unused";
stdout-path = &sbc_serial0;
};
 
@@ -24,7 +24,7 @@
};
 
aliases {
-   ttyAS0 = &sbc_serial0;
+   serial0 = &sbc_serial0;
ethernet0 = ðernet0;
};
 
-- 
1.9.1

[PATCH v2 0/3] Fix STi aliases property name

[patrice.chotard]

From: Patrice Chotard 

Since dtc v1.4.6-9-gaadd0b65c987, when compiling dtb with W=1 option,
the following warnings are triggered :

arch/arm/boot/dts/stih418-b2199.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'
arch/arm/boot/dts/stih407-b2120.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'
arch/arm/boot/dts/stih410-b2260.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'
arch/arm/boot/dts/stih410-b2120.dtb: Warning (alias_paths): /aliases: aliases 
property name must include only lowercase and '-'

_ Patch 1, convert the aliases property name in lowercase in 
  all STi board dts files.

_ Patch 2, remove useless stdout-path property

_ Patch 3, rework the tty driver st-asc accordingly, as aliases id is retrieved 
  using of_alias_get_id() with a defined string with is not lowercase only.

v2: Fix Rob Herring's remarks :
  _ use serialN instead of ttyasN aliases to not break ABI
  _ remove useless stdout-path property
  _ update st-asc driver with "serial" alias prefix

Patrice Chotard (2):
  ARM: dts: STi: Fix aliases property name for STi boards
  ARM: dts: STi: Remove useless stdout-path for STi boards
  tty: st-asc: Update tty alias

 arch/arm/boot/dts/stih407-b2120.dts | 5 ++---
 arch/arm/boot/dts/stih410-b2120.dts | 5 ++---
 arch/arm/boot/dts/stih410-b2260.dts | 5 ++---
 arch/arm/boot/dts/stih418-b2199.dts | 5 ++---
 drivers/tty/serial/st-asc.c | 2 +-
 5 files changed, 9 insertions(+), 13 deletions(-)

-- 
1.9.1

RE: [PATCH 1/3 RESEND] tpm: add longer timeouts for creation commands.

[Winkler, Tomas]

> 
> On Mon, Mar 05, 2018 at 06:04:56PM +, Winkler, Tomas wrote:
> > >
> > > On Mon, Mar 05, 2018 at 01:09:09PM +, Winkler, Tomas wrote:
> > > > > enum tpm_duration {
> > > > >   TPM_DURATION_DEFAULT = 2000,
> > > > >   TPM_DURATION_LONG = 30,
> > > > > };
> > > > >
> > > > How is this aligned with the spec PTP spec?
> > >
> > > For TPM 2.0 that spec only partially defines durations for CCs and
> > > thus our look up table is already kind "flakky". In a sense that the
> > > default duration is upper limit for spec defined durations.
> >
> > The timeouts for LONG and MEDIUM is defined by the  PTP spec,  we need
> to maintain those as those effect the system.
> > The UNDEFINED and LONG LONG is the implementation choice we driver
> from empirical data we have so far.
> 
> Where can be get this empirical data?
>From testing the HW.
> 
> You are not only adding 30s delay but also turning the 2s delay to 12s delay.


I'm adding 3 min, no other changes.  Where is 12s?

> IMHO we could very well use PTP LONG for all commands as the timeout.
> Why that would not work?


Empirically it doesn't go test it you have the HW.

Thanks
Tomas

Re: Would you help to tell why async printk solution was not taken to upstream kernel ?

[Sergey Senozhatsky]

On (03/05/18 22:16), Steven Rostedt wrote:
> > Yes. My point was that "CPU can print one full buffer max" is not
> > guaranteed and not exactly true. There are ways for CPUs to break
> > that O(logbuf) boundary.
> 
> Yes, when printk or the consoles have a bug, it can be greater than
> O(logbuf).

OK. Now the question is - what is "a bug" in this case? Are those
printk-s really a bug? Consoles are very complex, with dependencies
on timers, networking, etc. having them appending more messages to
the logbuf is not very cool, but at the time the kernel does not
BUG_ON(), nor panic(); it moves on. It's printk()->console_unlock()
that turns it into lockup->panic(). Is the bug really in the consoles
then?

-ss

Re: [PATCH 2/2] trace_uprobe: Simplify probes_seq_show()

[Ravi Bangoria]

On 02/08/2018 09:13 AM, Ravi Bangoria wrote:
>
> On 02/08/2018 08:59 AM, Kees Cook wrote:
>> On Tue, Feb 6, 2018 at 8:34 PM, Ravi Bangoria
>>  wrote:
>>> Simplify probes_seq_show() function. We are using %lx to display
>>> the offset and we don't prepend unnecessary 0s in the offset.
>>>
>>> Signed-off-by: Ravi Bangoria 
>>> ---
>>>  kernel/trace/trace_uprobe.c | 21 +++--
>>>  1 file changed, 3 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
>>> index c2c965398893..c12a3957e1ee 100644
>>> --- a/kernel/trace/trace_uprobe.c
>>> +++ b/kernel/trace/trace_uprobe.c
>>> @@ -602,24 +602,9 @@ static int probes_seq_show(struct seq_file *m, void *v)
>>> char c = is_ret_probe(tu) ? 'r' : 'p';
>>> int i;
>>>
>>> -   seq_printf(m, "%c:%s/%s", c, tu->tp.call.class->system,
>>> -   trace_event_name(&tu->tp.call));
>>> -   seq_printf(m, " %s:", tu->filename);
>>> -
>>> -   /* Don't print "0x  (null)" when offset is 0 */
>>> -   if (tu->offset) {
>>> -   seq_printf(m, "0x%lx", tu->offset);
>>> -   } else {
>>> -   switch (sizeof(void *)) {
>>> -   case 4:
>>> -   seq_printf(m, "0x");
>>> -   break;
>>> -   case 8:
>>> -   default:
>>> -   seq_printf(m, "0x");
>>> -   break;
>>> -   }
>>> -   }
>>> +   seq_printf(m, "%c:%s/%s %s:0x%lx", c, tu->tp.call.class->system,
>>> +   trace_event_name(&tu->tp.call), tu->filename,
>>> +   tu->offset);
>> To keep the prepended zeros (and avoid the redundant 0x prefix):
>>
>> "...%#0*lx...", ... sizeof(void *) * 2, tu->offset);
>>
>> As in:
>>
>> +   seq_printf(m, "%c:%s/%s %s:%#0*lx", c, tu->tp.call.class->system,
>> +   trace_event_name(&tu->tp.call), tu->filename,
>> +   sizeof(void *) * 2, tu->offset);
> This is useful, thanks Kees.
>
> @Wang, Do we really need those 0s? Won't just 0x0 should
> suffice? Here is the sample output...
>
>   # echo "p:probe_a/main /tmp/a.out:0x0" > uprobe_events
>
> Before patch:
>   # cat uprobe_events
>     p:probe_a/main /tmp/a.out:0x
>
> After patch:
>   # cat uprobe_events
>     p:probe_a/main /tmp/a.out:0x0

Wang, ping :)

Kees, I don't hear back from Wang and no one has reported any issues with
the patches yet. Can I have your Acked-by?

Thanks,
Ravi

Re: [RFC/RFT][PATCH 6/7] sched: idle: Predict idle duration before stopping the tick

[Rafael J. Wysocki]

On Tuesday, March 6, 2018 12:27:01 AM CET Rik van Riel wrote:
> On Sun, 2018-03-04 at 23:28 +0100, Rafael J. Wysocki wrote:
> > 
> > +++ linux-pm/kernel/sched/idle.c
> > @@ -188,13 +188,14 @@ static void cpuidle_idle_call(void)
> > } else {
> > unsigned int duration_us;
> >  
> > -   tick_nohz_idle_go_idle(true);
> > -   rcu_idle_enter();
> > -
> > /*
> >  * Ask the cpuidle framework to choose a convenient
> > idle state.
> >  */
> > next_state = cpuidle_select(drv, dev, &duration_us);
> > +
> > +   tick_nohz_idle_go_idle(duration_us > USEC_PER_SEC /
> > HZ);
> > +   rcu_idle_enter();
> > +
> > entered_state = call_cpuidle(drv, dev, next_state);
> 
> When the expected idle period is short enough
> that the timer is not stopped, does it make
> sense to still call rcu_idle_enter?
> 
> Should rcu_idle_enter also be conditional on
> the expected idle period?

Well, that would be the next step. :-)

Re: "x86/boot/compressed/64: Prepare trampoline memory" breaks boot on Zotac CI-321

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 07:57:06PM +0100, Heiner Kallweit wrote:
> Am 05.03.2018 um 09:19 schrieb Kirill A. Shutemov:
> > On Sat, Mar 03, 2018 at 12:46:28PM +0100, Heiner Kallweit wrote:
> >> I wanted to apply the fix mentioned in the link but found that the 
> >> statement was movq already.
> >> Therefore my (most likely false) understanding that it's v2.
> >> I'll re-test once v2 is out and let you know.
> > 
> > movq fix is unrelated to the problem.
> > 
> > Please check if current linux-next plus this patchset causes a problem for
> > you:
> > 
> > http://lkml.kernel.org/r/20180227154217.69347-1-kirill.shute...@linux.intel.com
> > 
> 
> linux-next from today boots fine with the patchset applied.

Thanks for testing!

Ingo, is there anything else I need to do for the patchset to be applied?

-- 
 Kirill A. Shutemov

Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:12:15AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> >  extern void prep_encrypt_page(struct page *page, gfp_t gfp, unsigned int 
> > order);
> > +extern void free_encrypt_page(struct page *page, int keyid, unsigned int 
> > order);
> 
> The grammar here is weird, I think.
> 
> Why not free_encrypted_page()?

Okay, I'll fix this.

-- 
 Kirill A. Shutemov

Re: [PATCH v2] mtdchar: fix usage of mtd_ooblayout_ecc()

[Boris Brezillon]

On Tue, 6 Mar 2018 11:13:25 +0800
欧阳志忠  wrote:

> Section was not properly computed. The value of OOB region definition is
> always ECC section 0 information in the OOB area, but we want to get all
> the ECC bytes information, so we should call
> mtd_ooblayout_ecc(mtd, section++, &oobregion) until it returns -ERANGE.
> 

You still miss the Fixes and Cc-stable tags:

Fixes: c2b78452a9db ("mtd: use mtd_ooblayout_xxx() helpers where appropriate")
Cc: 

Looks good otherwise.

I'm just curious, how did you find the problem? Do you have a userspace
tool that queries the OOB/ECC layout?

> Signed-off-by: OuYang ZhiZhong 
> ---
>  drivers/mtd/mtdchar.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index de8c902..7d80a8b 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -479,7 +479,7 @@ static int shrink_ecclayout(struct mtd_info *mtd,
>   for (i = 0; i < MTD_MAX_ECCPOS_ENTRIES;) {
>   u32 eccpos;
> 
> - ret = mtd_ooblayout_ecc(mtd, section, &oobregion);
> + ret = mtd_ooblayout_ecc(mtd, section++, &oobregion);
>   if (ret < 0) {
>   if (ret != -ERANGE)
>   return ret;
> @@ -526,7 +526,7 @@ static int get_oobinfo(struct mtd_info *mtd, struct
> nand_oobinfo *to)
>   for (i = 0; i < ARRAY_SIZE(to->eccpos);) {
>   u32 eccpos;
> 
> - ret = mtd_ooblayout_ecc(mtd, section, &oobregion);
> + ret = mtd_ooblayout_ecc(mtd, section++, &oobregion);
>   if (ret < 0) {
>   if (ret != -ERANGE)
>   return ret;



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/3] tpm_crb: use __le64 annotated variable for response buffer address

[Jarkko Sakkinen]

On Mon, Mar 05, 2018 at 03:03:20PM +0200, Jarkko Sakkinen wrote:
> On Sun, Mar 04, 2018 at 02:12:05PM +0200, Tomas Winkler wrote:
> > This suppresses sparse warning
> > drivers/char/tpm/tpm_crb.c:558:18: warning: cast to restricted __le64
> > 
> > Signed-off-by: Tomas Winkler 
> > ---
> >  drivers/char/tpm/tpm_crb.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> The guideline is that you should describe what is wrong rather than
> copy-paste the sparse message.

Jason, didn't yo give the feedback to some patch 1-2 years ago that
instead of copy-pasting parse error one should write a clear commit
msg or is this OK?

/Jarkko

Re: [RFC, PATCH 13/22] mm, rmap: Free encrypted pages once mapcount drops to zero

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:13:36AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > @@ -1292,6 +1308,12 @@ static void page_remove_anon_compound_rmap(struct 
> > page *page)
> > __mod_node_page_state(page_pgdat(page), NR_ANON_MAPPED, -nr);
> > deferred_split_huge_page(page);
> > }
> > +
> > +   anon_vma = page_anon_vma(page);
> > +   if (anon_vma_encrypted(anon_vma)) {
> > +   int keyid = anon_vma_keyid(anon_vma);
> > +   free_encrypt_page(page, keyid, compound_order(page));
> > +   }
> >  }
> 
> It's not covered in the description and I'm to lazy to dig into it, so:
> Without this code, where do they get freed?  Why does it not cause any
> problems to free them here?

It's the only place where we get it freed. "Freeing" is not the best
terminology here, but I failed to come up with something batter.
We prepare the encryption page to being freed: flush the cache in MKTME
case.

The page itself gets freed later in a usual manner: once refcount drops to
zero. The problem is that we may not have valid anon_vma around once
mapcount drops to zero, so we have to do "freeing" here.

For anonymous memory once mapcount dropped to zero there's no way it will
get mapped back to userspace. page_remove_anon

Kernel still will be able to access the page with kmap() and I will need
to be very careful to get it right wrt cache management.

I'll update the description.

-- 
 Kirill A. Shutemov

Re: [PATCH] kernel/memremap: Remove stale devres_free() call

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig 

Re: [RFC, PATCH 16/22] x86/mm: Preserve KeyID on pte_modify() and pgprot_modify()

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:09:23AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > + * It includes full range of PFN bits regardless if they were claimed for 
> > KeyID
> > + * or not: we want to preserve KeyID on pte_modify() and pgprot_modify().
> >   */
> > -#define _PAGE_CHG_MASK (PTE_PFN_MASK | _PAGE_PCD | _PAGE_PWT | 
> > \
> > +#define PTE_PFN_MASK_MAX \
> > +   (((signed long)PAGE_MASK) & ((1UL << __PHYSICAL_MASK_SHIFT) - 1))
> > +#define _PAGE_CHG_MASK (PTE_PFN_MASK_MAX | _PAGE_PCD | _PAGE_PWT | 
> > \
> >  _PAGE_SPECIAL | _PAGE_ACCESSED | _PAGE_DIRTY | \
> >  _PAGE_SOFT_DIRTY)
> 
> Is there a way to make this:
> 
> #define _PAGE_CHG_MASK(PTE_PFN_MASK | PTE_KEY_MASK...? | _PAGE_PCD |
> 
> That would be a lot more understandable.

Yes, it would.

But it means we will have *two* variables referenced from _PAGE_CHG_MASK:
one for PTE_PFN_MASK and one for PTE_KEY_MASK as both of them are dynamic.

With this patch we would get rid of both of them.

I'll update the description.

-- 
 Kirill A. Shutemov

RE: [PATCH 3/3] tpm_crb: use __le64 annotated variable for response buffer address

[Winkler, Tomas]

> On Mon, Mar 05, 2018 at 03:03:20PM +0200, Jarkko Sakkinen wrote:
> > On Sun, Mar 04, 2018 at 02:12:05PM +0200, Tomas Winkler wrote:
> > > This suppresses sparse warning
> > > drivers/char/tpm/tpm_crb.c:558:18: warning: cast to restricted
> > > __le64
> > >
> > > Signed-off-by: Tomas Winkler 
> > > ---
> > >  drivers/char/tpm/tpm_crb.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > The guideline is that you should describe what is wrong rather than
> > copy-paste the sparse message.
> 
> Jason, didn't yo give the feedback to some patch 1-2 years ago that instead
> of copy-pasting parse error one should write a clear commit msg or is this
> OK?

I think you are reading wrongly the rule,  the title explains the issue and in 
addition 
I'm adding exact sparse warning. this is usually required. 
What is wrong is putting something like 'Fix sparse error' or "Fix warning' 
into patch subject. 
So the imperative here is 'adding annotation' and not a 'fixing a sparse 
message'.

Thanks
Tomas

Re: [PATH 0/4] usbip: make vhci_hcd.* objects independent of vhci_hcd.0

[Salvador Fandiño]

On 03/06/2018 01:03 AM, Shuah Khan wrote:

On 03/05/2018 02:00 AM, Salvador Fandiño wrote:

On 02/21/2018 01:35 AM, Shuah Khan wrote:

Hi Salvador,

On 01/30/2018 01:36 AM, Salvador Fandino wrote:

Let me start by explaining the problem that have motivated me to write
this patches:

I work on the QVD, a virtual desktop platform for Linux. This software
runs Linux desktops (i.e. XFCE, KDE) and their applications inside LXC
containers, and makes then available through the network to remote
users.

Supporting USB devices is a common feature customers have been
requesting us for a long time (in order to use, for instance, remote
signature pads, bar-code scanners, fingerprint readers, etc.). So, we
have been working on that feature using the USB/IP layer on the
kernel.

Connecting and disconnecting devices and transferring data works
seamless for the devices listed above. But we also want to make the
usbip operations private to the container where they are run.  For
instance, it is unacceptable for our product, that one user could list
the devices connected by other users or access them.

We can control how can access every device using cgroups once those
are attached, but the usbip layer is not providing any mechanism for
controlling who can attach, detach or list the devices.

In this use-case:

- does a container act as usbip client and attach devices from their
   host?
- do containers attach remote devices from other systems?
In my particular case devices are imported from remote machines. But 
well, the thing is that I don't care where the connections come from, 
they could even be devices emulated in user space.



Is the core of the problem really that any remote system can import without
a provision for being able to restrict export to a set of remote machines?
If so, this is a generic problem even without containers and I would like
to solve this with a generic solution that works in all cases, not just for
containers.
No, that is a different issue. You are talking about controlling which 
devices can be connected, from which hosts, etc. That is an interesting 
problem but not the one I am trying to tackle here.


I don't mind which every user does inside its container as far as it 
does not interfere which other users. In practice that means:


1- Not being able to attach/detach devices in other containers
2- Not being able to list devices attached in other containers
3- Not being able to access devices attached in other containers.

Point 3 is already enforceable using the devices hierarchy in cgroups. 
For points 1 and 2, my proposition is making every vhci_hcd device have 
its own fully independent sysfs directory (instead of all of them going 
through vhci_hcd.0) so that they can be selectively exposed with rw 
permissions inside the containers.





The approach in this patch series appears to solve the problem just for
containers.


Did you explore a solution to add a mechanism for access control to
usbip?

Could you elaborate on that?

For "usbip", do you mean the user space tools?
If that is the case, I don't think it would be enough.
My aim is to limit vhci usage from containers and I have no control about what runs 
inside the containers. So, a mangled usbip tool-set could > > be used by a 
malicious user to circumvent any access control set there.>

I mean the driver. There might be changes necessary in the user-space
as well depending on how the access controls are implemented. I am not
proposing implementing access controls in the user-space.



IMO, there is no other choice but to control access to VHCI at the kernel level.


Probably. Please give as many details as possible on your environment
for me to make a call on if this problem can be solved in a different
way.


In our particular real life application, we are targeting the kernel 
interface directly, we don't use the usbip tools at all. It is that way 
because we have our own* transport layer, authentication and 
authorization mechanisms. And once all the handshaking is done we end 
with a socket we can directly pass to the kernel in order to get it 
attached to a vhci_hcd port. We don't like having an extra application 
listening on some TCP port which can be accessed by third parties on the 
client side either.


The imported USB devices used are mostly devices which do not require 
kernel modules and that are accessed though libusb by the applications 
(i.e., id card readers, barcode scanners, signing pads, etc.).


* Just in case you want to know, USBIP data goes through a channel in a 
nx (https://github.com/ArcticaProject/nx-libs) connection running over a 
websocket over TLS. Authentication is performed by the broker (a proxy 
which knows where a user containers are running). Authorization is 
performed following policies configured by the administrator (currently 
it is just an all or nothing policy: USPIP is allowed or not) by the 
control application at container creation time.

Re: [RFC, PATCH 18/22] x86/mm: Handle allocation of encrypted pages

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:03:55AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > -#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
> > -   alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
> >  #define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
> > +#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) 
> > \
> > +({ 
> > \
> > +   struct page *page;  
> > \
> > +   gfp_t gfp = movableflags | GFP_HIGHUSER;
> > \
> > +   if (vma_is_encrypted(vma))  
> > \
> > +   page = __alloc_zeroed_encrypted_user_highpage(gfp, vma, vaddr); 
> > \
> > +   else
> > \
> > +   page = alloc_page_vma(gfp | __GFP_ZERO, vma, vaddr);
> > \
> > +   page;   
> > \
> > +})
> 
> This is pretty darn ugly and also adds a big old branch into the hottest
> path in the page allocator.
> 
> It's also really odd that you strip __GFP_ZERO and then go ahead and
> zero the encrypted page unconditionally.  It really makes me wonder if
> this is the right spot to be doing this.
> 
> Can we not, for instance do it inside alloc_page_vma()?

Yes we can.

It would require substantial change into page allocation path for
CONFIG_NUMA=n as we don't path down vma at the moment. And without vma we
don't have a way to know which KeyID to use.

I will explore how it would fit together.

-- 
 Kirill A. Shutemov

Re: [PATCH 3/3] i2c: xlp9xx: Add support for SMBAlert

[Jan Glauber]

On Tue, Feb 27, 2018 at 01:26:20PM +, George Cherian wrote:
> Add support for SMBus alert mechanism to i2c-xlp9xx driver.
> The second interrupt is parsed to use for SMBus alert.
> The first interrupt is the i2c controller main interrupt.
> 
> Signed-off-by: Kamlakant Patel 
> Signed-off-by: George Cherian 
> ---
>  drivers/i2c/busses/i2c-xlp9xx.c | 26 ++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> index eb8913e..9462eab 100644
> --- a/drivers/i2c/busses/i2c-xlp9xx.c
> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
>   struct device *dev;
>   struct i2c_adapter adapter;
>   struct completion msg_complete;
> + struct i2c_smbus_alert_setup alert_data;
> + struct i2c_client *ara;
>   int irq;
>   bool msg_read;
>   bool len_recv;
> @@ -447,6 +450,21 @@ static int xlp9xx_i2c_get_frequency(struct 
> platform_device *pdev,
>   return 0;
>  }
>  
> +static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,
> +   struct platform_device *pdev)
> +{
> + if (!priv->alert_data.irq)
> + return -EINVAL;
> +
> + priv->alert_data.alert_edge_triggered = 0;

Hi George,

I think this is not needed anymore, see:
9b9f2b8bc2ac i2c: i2c-smbus: Use threaded irq for smbalert

--Jan

> +
> + priv->ara = i2c_setup_smbus_alert(&priv->adapter, &priv->alert_data);
> + if (!priv->ara)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
>  static int xlp9xx_i2c_probe(struct platform_device *pdev)
>  {
>   struct xlp9xx_i2c_dev *priv;
> @@ -467,6 +485,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
>   dev_err(&pdev->dev, "invalid irq!\n");
>   return priv->irq;
>   }
> + /* SMBAlert irq */
> + priv->alert_data.irq = platform_get_irq(pdev, 1);
> + if (priv->alert_data.irq <= 0)
> + priv->alert_data.irq = 0;
>  
>   xlp9xx_i2c_get_frequency(pdev, priv);
>   xlp9xx_i2c_init(priv);
> @@ -493,6 +515,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
>   if (err)
>   return err;
>  
> + err = xlp9xx_i2c_smbus_setup(priv, pdev);
> + if (err)
> + dev_info(&pdev->dev, "No active SMBus alert %d\n", err);
> +
>   platform_set_drvdata(pdev, priv);
>   dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
>  
> -- 
> 2.1.4

Re: [RFC, PATCH 18/22] x86/mm: Handle allocation of encrypted pages

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:07:55AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > kmap_atomic_keyid() would map the page with the specified KeyID.
> > For now it's dummy implementation that would be replaced later.
> 
> I think you need to explain the tradeoffs here.  We could just change
> the linear map around, but you don't.  Why?

I don't think we settled on implementation by this point: kmap() is only
interface and doesn't imply what it acctually does. I *can* change linear
mapping if we would chose so.

I will explain the kmap() implementation in patches that would implement
it.

-- 
 Kirill A. Shutemov

Re: [PATCH 07/34] x86/entry/32: Restore segments before int registers

[Joerg Roedel]

On Mon, Mar 05, 2018 at 01:58:32PM -0800, Linus Torvalds wrote:
> On Mon, Mar 5, 2018 at 1:35 PM, Joerg Roedel  wrote:
> > I could probably add some debug instrumentation to check for that in my
> > future testing, as there is no NX protection in the user address-range
> > for the kernel-cr3.
> 
> Does not NX work with PAE?
> 
> Oh, it looks like the NX bit is marked as "RSVD (must be 0)" in the
> PDPDT. Oh well.

I had a version of these patches running which implemented NX on the PDE
level by allocating 8k PMD pages. But that ended up needing 5 order-1
allocations for each page-table, which I got to fail pretty easily after
some time. So I abandoned this approach for now.

Maybe it can be implemented with order-0 allocations for PMD pages, the
open problem is how to link the user and kernel PMD page-pairs together
then.

Regards,

Joerg

[PATCH resend] usb: quirks: add control message delay for 1b1c:1b20

[Danilo Krummrich]

Corsair Strafe RGB keyboard does not respond to usb control messages
sometimes and hence generates timeouts.

Commit de3af5bf259d ("usb: quirks: add delay init quirk for Corsair
Strafe RGB keyboard") tried to fix those timeouts by adding
USB_QUIRK_DELAY_INIT.

Unfortunately, even with this quirk timeouts of usb_control_msg()
can still be seen, but with a lower frequency (approx. 1 out of 15):

[   29.103520] usb 1-8: string descriptor 0 read error: -110
[   34.363097] usb 1-8: can't set config #1, error -110

Adding further delays to different locations where usb control
messages are issued just moves the timeouts to other locations,
e.g.:

[   35.400533] usbhid 1-8:1.0: can't add hid device: -110
[   35.401014] usbhid: probe of 1-8:1.0 failed with error -110

The only way to reliably avoid those issues is having a pause after
each usb control message. In approx. 200 boot cycles no more timeouts
were seen.

Addionaly, keep USB_QUIRK_DELAY_INIT as it turned out to be necessary
to have the delay in hub_port_connect() after hub_port_init().

The overall boot time seems not to be influenced by these additional
delays, even on fast machines and lightweight distributions.

Fixes: de3af5bf259d ("usb: quirks: add delay init quirk for Corsair Strafe RGB 
keyboard")
Cc: sta...@vger.kernel.org
Signed-off-by: Danilo Krummrich 
---
 drivers/usb/core/message.c | 4 
 drivers/usb/core/quirks.c  | 3 ++-
 include/linux/usb/quirks.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index c64cf6c4a83d..0c11d40a12bc 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -151,6 +151,10 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe, __u8 request,
 
ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
+   /* Linger a bit, prior to the next control message. */
+   if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
+   msleep(200);
+
kfree(dr);
 
return ret;
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index f4a548471f0f..54b019e267c5 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -230,7 +230,8 @@ static const struct usb_device_id usb_quirk_list[] = {
{ USB_DEVICE(0x1b1c, 0x1b13), .driver_info = USB_QUIRK_DELAY_INIT },
 
/* Corsair Strafe RGB */
-   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT },
+   { USB_DEVICE(0x1b1c, 0x1b20), .driver_info = USB_QUIRK_DELAY_INIT |
+ USB_QUIRK_DELAY_CTRL_MSG },
 
/* Corsair K70 LUX */
{ USB_DEVICE(0x1b1c, 0x1b36), .driver_info = USB_QUIRK_DELAY_INIT },
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index f1fcec2fd5f8..b7a99ce56bc9 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -63,4 +63,7 @@
  */
 #define USB_QUIRK_DISCONNECT_SUSPEND   BIT(12)
 
+/* Device needs a pause after every control message. */
+#define USB_QUIRK_DELAY_CTRL_MSG   BIT(13)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.16.2

Re: [RFC, PATCH 19/22] x86/mm: Implement free_encrypt_page()

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:00:00AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > +void free_encrypt_page(struct page *page, int keyid, unsigned int order)
> > +{
> > +   int i;
> > +   void *v;
> > +
> > +   for (i = 0; i < (1 << order); i++) {
> > +   v = kmap_atomic_keyid(page, keyid + i);
> > +   /* See comment in prep_encrypt_page() */
> > +   clflush_cache_range(v, PAGE_SIZE);
> > +   kunmap_atomic(v);
> > +   }
> > +}
> 
> Did you miss adding the call sites for this?

No. It is in "mm, rmap: Free encrypted pages once mapcount drops to zero".
But the call is optimized out since anon_vma_encrypted() is always false
so far.

-- 
 Kirill A. Shutemov

Re: [RFC/RFT][PATCH 4/7] cpuidle: menu: Split idle duration prediction from state selection

[Peter Zijlstra]

On Tue, Mar 06, 2018 at 10:15:10AM +0800, Li, Aubrey wrote:
> On 2018/3/5 21:53, Peter Zijlstra wrote:
> > On Mon, Mar 05, 2018 at 02:05:10PM +0100, Rafael J. Wysocki wrote:
> >> On Mon, Mar 5, 2018 at 1:50 PM, Peter Zijlstra  
> >> wrote:
> >>> On Mon, Mar 05, 2018 at 12:47:23PM +0100, Rafael J. Wysocki wrote:
> > 
>  IOW, the target residency of the selected state doesn't tell you how
>  much time you should expect to be idle in general.
> >>>
> >>> Right, but I think that measure isn't of primary relevance. What we want
> >>> to know is: 'should I stop the tick' and 'what C state do I go to'.
> 
> I understood the benefit of mapping duration to state number, is duration <->
> state number mapping a generic solution to all arches?

Yes, all platforms have a limited set of possible idle states.

> Back to the user's concern is, "I'm running a latency sensitive application, 
> and
> I want idle switching ASAP". So I think the user may not care about what C 
> state
> to go into, that is, even if a deeper state has chance to go, the user 
> striving
> for a higher workload score may still not want it?

The user caring about performance very much cares about the actual idle
state too, exit latency for deeper states is horrific and will screw
them up just as much as the whole nohz timer reprogramming does.

We can basically view the whole nohz thing as an additional entry/exit
latency for the idle state, which is why I don't think its weird to
couple them.

> >> Maybe just return a "nohz" indicator from cpuidle_select() in addition
> >> to the state index and make the decision in the governor?
> > 
> > Much better option than returning a duration :-)
> >
> So what does "nohz = disable and state index = deepest" mean? This combination
> does not make sense for performance only purpose?

I tend to agree with you that the state space allowed by a separate
variable is larger than required, but it's significantly smaller than
preserving 'time' so I can live with it.

Re: Simplifying our RCU models

[Ingo Molnar]

* Paul E. McKenney  wrote:

> > > But if we look at the bigger API picture:
> > >
> > >   !PREEMPT_RCU  PREEMPT_RCU=y
> > >   rcu_read_lock():atomicpreemptible
> > >   rcu_read_lock_sched():  atomicatomic
> > >   srcu_read_lock():   preemptible   preemptible
> > >
> > > Then we could maintain full read side API flexibility by making 
> > > PREEMPT_RCU=y the 
> > > only model, merging it with SRCU and using these main read side APIs:
> > >
> > >   rcu_read_lock_preempt_disable():atomic
> > >   rcu_read_lock():preemptible
> 
> One issue with merging SRCU into rcu_read_lock() is the general blocking 
> within 
> SRCU readers.  Once merged in, these guys block everyone.  We should focus 
> initially on the non-SRCU variants.
> 
> On the other hand, Linus's suggestion of merging rcu_read_lock_sched()
> into rcu_read_lock() just might be feasible.  If that really does pan
> out, we end up with the following:
> 
>   !PREEMPTPREEMPT=y
>   rcu_read_lock():atomic  preemptible
>   srcu_read_lock():   preemptible preemptible
> 
> In this model, rcu_read_lock_sched() maps to preempt_disable() and (as
> you say above) rcu_read_lock_bh() maps to local_bh_disable().  The way
> this works is that in PREEMPT=y kernels, synchronize_rcu() waits not
> only for RCU read-side critical sections, but also for regions of code
> with preemption disabled.  The main caveat seems to be that there be an
> assumed point of preemptibility between each interrupt and each softirq
> handler, which should be OK.
> 
> There will be some adjustments required for lockdep-RCU, but that should
> be reasonably straightforward.
> 
> Seem reasonable?

Yes, that approach sounds very reasonable to me: it is similar to what we do on 
the locking side as well, where we have 'atomic' variants (spinlocks/rwlocks) 
and 
'sleeping' variants (mutexes, rwsems, etc.).

( This means there will be more automatic coupling between BH and preempt 
critical
  sections and RCU models not captured via explicit RCU-namespace APIs, but that
  should be OK I think. )

A couple of small side notes:

 - Could we please also clean up the namespace of the synchronization APIs and 
   change them all to an rcu_ prefix, like all the other RCU APIs are? Right 
now 
   have a mixture like rcu_read_lock() but synchronize_rcu(), while I'd reall 
love 
   to be able to do:

git grep '\ rcu_read_lock# unchanged
 rcu_read_unlock  => rcu_read_unlock  # unchanged

 call_rcu => rcu_call_rcu
 call_rcu_bh  => rcu_call_bh
 call_rcu_sched   => rcu_call_sched

 synchronize_rcu  => rcu_wait_
 synchronize_rcu_bh   => rcu_wait_bh
 synchronize_rcu_bh_expedited => rcu_wait_expedited_bh
 synchronize_rcu_expedited=> rcu_wait_expedited
 synchronize_rcu_mult => rcu_wait_mult
 synchronize_rcu_sched=> rcu_wait_sched
 synchronize_rcu_tasks=> rcu_wait_tasks

 srcu_read_lock   => srcu_read_lock   # unchanged
 srcu_read_unlock => srcu_read_unlock # unchanged

 synchronize_srcu => srcu_wait
 synchronize_srcu_expedited   => srcu_wait_expedited

   Note that due to the prefix approach we gain various new patterns:

   git grep rcu_wait  # matches both rcu and srcu
   git grep rcu_wait  # matches all RCU waiting variants
   git grep wait_expedited# matches all expedited variants

   ... which all increase the organization of the namespace.

 - While we are at it, the two RCU-state API variants, while rarely used, are
   named in a pretty obscure, disconnected fashion as well. A much better 
naming 
   would be:

 get_state_synchronize_rcu=> rcu_get_state
 cond_synchronize_rcu => rcu_wait_state

   ... or so. This would also move them into the new, unified rcu_ prefix 
   namespace.

Note how consistent and hierarchical the new RCU API namespace is:

_[_]

If you agree with the overall concept of this I'd be glad to help out with 
scripting & testing the RCU namespace transition safely in an unintrusive 
fashion 
once you've done the model unification work, with compatibility defines to not 
create conflicts, churn and pain, etc.

Thanks,

Ingo

Re: [PATCHv2 1/3] i2c: xlp9xx: Check for Bus state before every transfer

[Jan Glauber]

I don't know how valuable same-company reviewed-by's are in the end, 
but the patches look good to me with the small change in SMBalert, so you
could add:

Reviewed-by: Jan Glauber 

--Jan

On Tue, Feb 27, 2018 at 01:26:18PM +, George Cherian wrote:
> I2C bus enters the STOP condition after the DATA_DONE interrupt is raised.
> Essentially the driver should be checking the bus state before sending
> any transaction. In case a transaction is initiated while the
> bus is busy, the prior transaction's stop condition is not achieved.
> Add the check to make sure the bus is not busy before every transaction.
> 
> Signed-off-by: George Cherian 
> ---
>  drivers/i2c/busses/i2c-xlp9xx.c | 32 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> index 1f6d780..42dd1fa 100644
> --- a/drivers/i2c/busses/i2c-xlp9xx.c
> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define XLP9XX_I2C_DIV   0x0
>  #define XLP9XX_I2C_CTRL  0x1
> @@ -36,6 +37,8 @@
>  #define XLP9XX_I2C_TIMEOUT   0X10
>  #define XLP9XX_I2C_GENCALLADDR   0x11
>  
> +#define XLP9XX_I2C_STATUS_BUSY   BIT(0)
> +
>  #define XLP9XX_I2C_CMD_START BIT(7)
>  #define XLP9XX_I2C_CMD_STOP  BIT(6)
>  #define XLP9XX_I2C_CMD_READ  BIT(5)
> @@ -71,6 +74,7 @@
>  #define XLP9XX_I2C_HIGH_FREQ 40
>  #define XLP9XX_I2C_FIFO_SIZE 0x80U
>  #define XLP9XX_I2C_TIMEOUT_MS1000
> +#define XLP9XX_I2C_BUSY_TIMEOUT  50
>  
>  #define XLP9XX_I2C_FIFO_WCNT_MASK0xff
>  #define XLP9XX_I2C_STATUS_ERRMASK(XLP9XX_I2C_INTEN_ARLOST | \
> @@ -241,6 +245,26 @@ static irqreturn_t xlp9xx_i2c_isr(int irq, void *dev_id)
>   return IRQ_HANDLED;
>  }
>  
> +static int xlp9xx_i2c_check_bus_status(struct xlp9xx_i2c_dev *priv)
> +{
> + u32 status;
> + u32 busy_timeout = XLP9XX_I2C_BUSY_TIMEOUT;
> +
> + while (busy_timeout) {
> + status = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_STATUS);
> + if ((status & XLP9XX_I2C_STATUS_BUSY) == 0)
> + break;
> +
> + busy_timeout--;
> + usleep_range(1000, 1100);
> + }
> +
> + if (!busy_timeout)
> + return -EIO;
> +
> + return 0;
> +}
> +
>  static int xlp9xx_i2c_init(struct xlp9xx_i2c_dev *priv)
>  {
>   u32 prescale;
> @@ -363,6 +387,14 @@ static int xlp9xx_i2c_xfer(struct i2c_adapter *adap, 
> struct i2c_msg *msgs,
>   int i, ret;
>   struct xlp9xx_i2c_dev *priv = i2c_get_adapdata(adap);
>  
> + ret = xlp9xx_i2c_check_bus_status(priv);
> + if (ret) {
> + xlp9xx_i2c_init(priv);
> + ret = xlp9xx_i2c_check_bus_status(priv);
> + if (ret)
> + return ret;
> + }
> +
>   for (i = 0; i < num; i++) {
>   ret = xlp9xx_i2c_xfer_msg(priv, &msgs[i], i == num - 1);
>   if (ret != 0)
> -- 
> 2.1.4

Re: [PATCH v4 15/38] drm/bridge: analogix_dp: Ensure edp is disabled when shutting down the panel

[Enric Balletbo i Serra]

Hi Marek,

On 06/03/18 08:35, Marek Szyprowski wrote:
> Hi All,
> 
> This is the patch, which introduces the issue I've pointed here:
> 
> https://lists.freedesktop.org/archives/dri-devel/2018-March/167794.html
> 
> On 2018-03-05 23:23, Enric Balletbo i Serra wrote:
>> From: Lin Huang 
>>
>> When panel is shut down, we should make sure edp can be disabled to avoid
>> undefined behavior.
>>
>> Cc: Stéphane Marchesin 
>> Signed-off-by: Lin Huang 
>> Signed-off-by: zain wang 
>> Signed-off-by: Sean Paul 
>> Signed-off-by: Thierry Escande 
>> Reviewed-by: Andrzej Hajda 
>> Signed-off-by: Enric Balletbo i Serra 
>> ---
>>
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 92fb9a072cb6..9b7d530ad24c 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1160,6 +1160,12 @@ static int analogix_dp_set_bridge(struct
>> analogix_dp_device *dp)
>>     pm_runtime_get_sync(dp->dev);
>>   +    ret = clk_prepare_enable(dp->clock);
>> +    if (ret < 0) {
>> +    DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);
>> +    goto out_dp_clk_pre;
>> +    }
>> +
>>   if (dp->plat_data->power_on)
>>   dp->plat_data->power_on(dp->plat_data);
>>   @@ -1191,6 +1197,8 @@ static int analogix_dp_set_bridge(struct
>> analogix_dp_device *dp)
>>   phy_power_off(dp->phy);
>>   if (dp->plat_data->power_off)
>>   dp->plat_data->power_off(dp->plat_data);
>> +    clk_disable_unprepare(dp->clock);
>> +out_dp_clk_pre:
>>   pm_runtime_put_sync(dp->dev);
>>     return ret;
>> @@ -1234,10 +1242,13 @@ static void analogix_dp_bridge_disable(struct
>> drm_bridge *bridge)
>>     disable_irq(dp->irq);
>>   phy_power_off(dp->phy);
>> +    analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
> 
> In case of Exynos DP, external PHY is used to power the DP block, so no
> register access should be performed after phy_power_off(). Please move
> analogix_dp_set_analog_power_down() before phy_power_off().
> 

Oh, sorry, I missed that change, I'll include in next version, but I will wait a
little bit more before send for if I receive feedback on the others patches.


>>     if (dp->plat_data->power_off)
>>   dp->plat_data->power_off(dp->plat_data);
>>   +    clk_disable_unprepare(dp->clock);
>> +
>>   pm_runtime_put_sync(dp->dev);
>>     ret = analogix_dp_prepare_panel(dp, false, true);
> 
> Best regards
Enric

Re: [RFC, PATCH 19/22] x86/mm: Implement free_encrypt_page()

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:07:16AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > +void free_encrypt_page(struct page *page, int keyid, unsigned int order)
> > +{
> > +   int i;
> > +   void *v;
> > +
> > +   for (i = 0; i < (1 << order); i++) {
> > +   v = kmap_atomic_keyid(page, keyid + i);
> > +   /* See comment in prep_encrypt_page() */
> > +   clflush_cache_range(v, PAGE_SIZE);
> > +   kunmap_atomic(v);
> > +   }
> > +}
> 
> Have you measured how slow this is?

No, I have not.

> It's an optimization, but can we find a way to only do this dance when
> we *actually* change the keyid?  Right now, we're doing mapping at alloc
> and free, clflushing at free and zeroing at alloc.  Let's say somebody does:
> 
>   ptr = malloc(PAGE_SIZE);
>   *ptr = foo;
>   free(ptr);
> 
>   ptr = malloc(PAGE_SIZE);
>   *ptr = bar;
>   free(ptr);
> 
> And let's say ptr is in encrypted memory and that we actually munmap()
> at free().  We can theoretically skip the clflush, right?

Yes we can. Theoretically. We would need to find a way to keep KeyID
around after the page is removed from rmap. That's not so trivial as far
as I can see.

I will look into optimization after I'll got functionality in place.

-- 
 Kirill A. Shutemov

Re: [PATCH] perf: correct ctx_event_type in ctx_resched()

[Peter Zijlstra]

On Mon, Mar 05, 2018 at 09:55:04PM -0800, Song Liu wrote:
> In ctx_resched(), EVENT_FLEXIBLE should be sched_out when EVENT_PINNED is
> added. However, ctx_resched() calculates ctx_event_type before checking
> this condition. As a result, pinned events will NOT get higher priority
> than flexible events.
> 
> The following shows this issue on an Intel CPU (where ref-cycles can
> only use one hardware counter).
> 
>   1. First start:
>perf stat -C 0 -e ref-cycles  -I 1000
>   2. Then, in the second console, run:
>perf stat -C 0 -e ref-cycles:D -I 1000
> 
> The second perf uses pinned events, which is expected to have higher
> priority. However, because it failed in ctx_resched(). It is never
> run.
> 
> This patch fixes this by calculating ctx_event_type after re-evaluating
> event_type.
> 
> Fixes: 487f05e18aa4 ("perf/core: Optimize event rescheduling on active 
> contexts")
> Signed-off-by: Song Liu 
> Reported-by: Ephraim Park 

Thanks!

Re: [RFC, PATCH 21/22] x86/mm: Introduce page_keyid() and page_encrypted()

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 09:08:53AM -0800, Dave Hansen wrote:
> On 03/05/2018 08:26 AM, Kirill A. Shutemov wrote:
> > +static inline bool page_encrypted(struct page *page)
> > +{
> > +   /* All pages with non-zero KeyID are encrypted */
> > +   return page_keyid(page) != 0;
> > +}
> 
> Is this true?  I thought there was a KEYID_NO_ENCRYPT "Do not encrypt
> memory when this KeyID is in use."  Is that really only limited to key 0.

Well, it depends on what we mean by "encrypted". For memory management
pruposes we care if the page is encrypted with KeyID different from
default one. All pages with non-default KeyID threated the same by memory
management.

So far we don't have users for the interface. We may reconsider
the meaning once we get users.

-- 
 Kirill A. Shutemov

Re: [RFC, PATCH 00/22] Partial MKTME enabling

[Kirill A. Shutemov]

On Mon, Mar 05, 2018 at 11:05:50AM -0800, Matthew Wilcox wrote:
> On Mon, Mar 05, 2018 at 10:30:50AM -0800, Christoph Hellwig wrote:
> > On Mon, Mar 05, 2018 at 07:25:48PM +0300, Kirill A. Shutemov wrote:
> > > Hi everybody,
> > > 
> > > Here's updated version of my patchset that brings support of MKTME.
> > 
> > It would really help if you'd explain what "MKTME" is..
> 
> You needed to keep reading, to below the -- line.
> 
> I agree though, that should have been up top.

My bad. Will update it for future postings.

-- 
 Kirill A. Shutemov

Re: [PATCH 0/2] ESRT fixes for relocatable kexec'd kernel

[AKASHI Takahiro]

Tyler, Jeffrey,

On Fri, Mar 02, 2018 at 08:27:11AM -0500, Tyler Baicar wrote:
> On 3/2/2018 12:53 AM, AKASHI Takahiro wrote:
> >Tyler, Jeffrey,
> >
> >[Note: This issue takes place in kexec, not kdump. So to be precise,
> >it is not the same phenomenon as what I addressed in [1],[2]:
> >   [1] 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-February/557254.html
> >   [2] 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553098.html
> >]
> >
> >On Thu, Mar 01, 2018 at 12:56:38PM -0500, Tyler Baicar wrote:
> >>Hello,
> >>
> >>On 2/28/2018 9:50 PM, AKASHI Takahiro wrote:
> >>>Hi,
> >>>
> >>>On Wed, Feb 28, 2018 at 08:39:42AM -0700, Jeffrey Hugo wrote:
> On 2/27/2018 11:19 PM, AKASHI Takahiro wrote:
> >Tyler,
> >
> ># I missed catching your patch as its subject doesn't contain arm64.
> >
> >On Fri, Feb 23, 2018 at 12:42:31PM -0700, Tyler Baicar wrote:
> >>Currently on arm64 ESRT memory does not appear to be properly blocked 
> >>off.
> >>Upon successful initialization, ESRT prints out the memory region that 
> >>it
> >>exists in like:
> >>
> >>esrt: Reserving ESRT space from 0x0a4c1c18 to 
> >>0x0a4c1cf0.
> >>
> >>But then by dumping /proc/iomem this region appears as part of System 
> >>RAM
> >>rather than being reserved:
> >>
> >>08f1-0dee : System RAM
> >>
> >>This causes issues when trying to kexec if the kernel is relocatable. 
> >>When
> >>kexec tries to execute, this memory can be selected to relocate the 
> >>kernel to
> >>which then overwrites all the ESRT information. Then when the kexec'd 
> >>kernel
> >>tries to initialize ESRT, it doesn't recognize the ESRT version number 
> >>and
> >>just returns from efi_esrt_init().
> >I'm not sure what is the root cause of your problem.
> >Do you have good confidence that the kernel (2nd kernel image in this 
> >case?)
> >really overwrite ESRT region?
> According to my debug, yes.
> Using JTAG, I was able to determine that the ESRT memory region was 
> getting
> overwritten by the secondary kernel in
> kernel/arch/arm64/kernel/relocate_kernel.S - specifically the "copy_page"
> line of arm64_relocate_new_kernel()
> 
> >To my best knowledge, kexec is carefully designed not to do such a thing
> >as it allocates a temporary buffer for kernel image and copies it to the
> >final destination at the very end of the 1st kernel.
> >
> >My guess is that kexec, or rather kexec-tools, tries to load the kernel 
> >image
> >at 0x8f8 (or 0x908?, not sure) in your case. It may or may not be
> >overlapped with ESRT.
> >(Try "-d" option when executing kexec command for confirmation.)
> With -d, I see
> 
> get_memory_ranges_iomem_cb: 09611000 - 0e5f : System 
> RAM
> 
> That overlaps the ESRT reservation -
> [ 0.00] esrt: Reserving ESRT space from 0x0b708718 to
> 0x0b7087f0
> 
> >Are you using initrd with kexec?
> Yes
> >>>To make the things clear, can you show me, if possible, the followings:
> >>I have attached all of these:
> >Many thanks.
> >According to the data, ESRT was overwritten by initrd, not the kernel image.
> >It doesn't matter to you though :)
> >
> >The solution would be, as Ard suggested, that more information be
> >added to /proc/iomem.
> >I'm going to fix the issue as quickly as possible.
> Great, thank you!! Please add us to the fix and we will gladly test it out.

I have created a workaround patch, attached below, as a kind of PoC.
Can you give it a go, please?
You need another patch for kexec-tools, too. See
https:/git.linaro.org/people/takahiro.akashi/kexecl-tools.git arm64/resv_mem

With this patch, extra entries for firmware-reserved memory resources,
which means any regions that are already reserved before arm64_memblock_init(),
or specifically efi/acpi tables in this case, are added to /proc/iomem.

 $ cat /proc/iomem (on my qemu+edk2 execution)
...
4000-5871 : System RAM
  4008-40f1 : Kernel code
  4104-411e9fff : Kernel data
  5440-583f : Crash kernel
  5859-585e : reserved
  5870-5871 : reserved
5872-58b5 : reserved
58b6-5be3 : System RAM
  58b61000-58b61fff : reserved
  59a7b118-59a7b667 : reserved
5be4-5bec : reserved
5bed-5bed : System RAM
  5bee-5bff : reserved
5c00-5fff : System RAM
  5ec0-5edf : reserved
80-ff : PCI Bus :00
  80-803fff : :00:01.0
80-803fff : virtio-pci-modern

While all the entries are currently marked as just "reserved," we'd better
give them more specific names for general/extens

Re: Simplifying our RCU models

[Ingo Molnar]

* Ingo Molnar  wrote:

>I.e. the new RCU namespace would be something like:
> 
>  call_rcu => rcu_call_rcu

typo: rcu_call().

>  synchronize_rcu  => rcu_wait_

typo: rcu_wait().

Here's the updated table:

 # RCU APIs:

 rcu_read_lock=> rcu_read_lock# unchanged
 rcu_read_unlock  => rcu_read_unlock  # unchanged

 call_rcu => rcu_call
 call_rcu_bh  => rcu_call_bh
 call_rcu_sched   => rcu_call_sched

 synchronize_rcu  => rcu_wait
 synchronize_rcu_bh   => rcu_wait_bh
 synchronize_rcu_bh_expedited => rcu_wait_expedited_bh
 synchronize_rcu_expedited=> rcu_wait_expedited
 synchronize_rcu_mult => rcu_wait_mult
 synchronize_rcu_sched=> rcu_wait_sched
 synchronize_rcu_tasks=> rcu_wait_tasks

 get_state_synchronize_rcu=> rcu_get_state
 cond_synchronize_rcu => rcu_wait_state


 # SRCU APIs:

 srcu_read_lock   => srcu_read_lock   # unchanged
 srcu_read_unlock => srcu_read_unlock # unchanged

 synchronize_srcu => srcu_wait
 synchronize_srcu_expedited   => srcu_wait_expedited


Thanks,

Ingo

[PATCH v4 0/6] Fix the overlapping registers issue for the MediTek audio system

[Ryder Lee]

Hi,

This is the new round of the audsys. I add devm_of_platform_populate() in the 
clock driver
to populate the child devices (AFE) so that we can handle the dependency for 
all functions. 

changes since v4:
 - remove "simple-mfd" from mediatek,audsys.txt.
 - switch to use devm_of_platform_populate() to populate the child devices.
 
changes since v3:
 - Rebase to v4.16.
 - Modify clock driver for the sake of the backward compatibility.
 - Rewrite commit message of patch 4/5 to make it more specific.

changes since v2:
 - Drop useless changes in clk-mt7622-aud.c.
 - Revise binding text: 
- Add more information about audio subsystem.
- Separate clock node and AFE node.
 - Update license header.

changes since v1:
 - To avoid writing an MFD driver, we add "simple-mfd" in the audsys binding.
 - Move three top clocks to audio driver [1] as we remove mfd/mtk-audsys.c in 
v1.

Ryder Lee (6):
  clk: mediatek: update missing clock data for MT7622 audsys
  clk: mediatek: add devm_of_platform_populate() for MT7622 audsys
  clk: mediatek: add audsys support for MT2701
  dt-bindings: clock: mediatek: update audsys documentation to adapt MFD
device
  dt-bindings: clock: mediatek: add audsys support for MT2701
  arm: dts: mediatek: modify audio related nodes for both MT2701 and
MT7623

 .../bindings/arm/mediatek/mediatek,audsys.txt  |  20 ++-
 arch/arm/boot/dts/mt2701.dtsi  | 188 ++--
 arch/arm/boot/dts/mt7623.dtsi  | 190 ++---
 drivers/clk/mediatek/Kconfig   |   6 +
 drivers/clk/mediatek/Makefile  |   1 +
 drivers/clk/mediatek/clk-mt2701-aud.c  | 177 +++
 drivers/clk/mediatek/clk-mt7622-aud.c  |   9 +-
 include/dt-bindings/clock/mt7622-clk.h |   3 +-
 8 files changed, 390 insertions(+), 204 deletions(-)
 create mode 100644 drivers/clk/mediatek/clk-mt2701-aud.c

-- 
1.9.1

[PATCH v4 1/6] clk: mediatek: update missing clock data for MT7622 audsys

[Ryder Lee]

Add missing clock data 'CLK_AUDIO_AFE_CONN' for MT7622 audsys.

Signed-off-by: Ryder Lee 
Reviewed-by: Rob Herring 
Reviewed-by: Matthias Brugger 
---
 drivers/clk/mediatek/clk-mt7622-aud.c  | 1 +
 include/dt-bindings/clock/mt7622-clk.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mt7622-aud.c 
b/drivers/clk/mediatek/clk-mt7622-aud.c
index fad7d9f..13f752d 100644
--- a/drivers/clk/mediatek/clk-mt7622-aud.c
+++ b/drivers/clk/mediatek/clk-mt7622-aud.c
@@ -106,6 +106,7 @@
GATE_AUDIO1(CLK_AUDIO_INTDIR, "audio_intdir", "intdir_sel", 20),
GATE_AUDIO1(CLK_AUDIO_A1SYS, "audio_a1sys", "a1sys_hp_sel", 21),
GATE_AUDIO1(CLK_AUDIO_A2SYS, "audio_a2sys", "a2sys_hp_sel", 22),
+   GATE_AUDIO1(CLK_AUDIO_AFE_CONN, "audio_afe_conn", "a1sys_hp_sel", 23),
/* AUDIO2 */
GATE_AUDIO2(CLK_AUDIO_UL1, "audio_ul1", "a1sys_hp_sel", 0),
GATE_AUDIO2(CLK_AUDIO_UL2, "audio_ul2", "a1sys_hp_sel", 1),
diff --git a/include/dt-bindings/clock/mt7622-clk.h 
b/include/dt-bindings/clock/mt7622-clk.h
index 3e514ed..e9d77f0 100644
--- a/include/dt-bindings/clock/mt7622-clk.h
+++ b/include/dt-bindings/clock/mt7622-clk.h
@@ -235,7 +235,8 @@
 #define CLK_AUDIO_MEM_ASRC343
 #define CLK_AUDIO_MEM_ASRC444
 #define CLK_AUDIO_MEM_ASRC545
-#define CLK_AUDIO_NR_CLK   46
+#define CLK_AUDIO_AFE_CONN 46
+#define CLK_AUDIO_NR_CLK   47
 
 /* SSUSBSYS */
 
-- 
1.9.1

[PATCH v4 4/6] dt-bindings: clock: mediatek: update audsys documentation to adapt MFD device

[Ryder Lee]

The MediaTek audio hardware block that exposes functionalities that are
handled by separate subsystems in the kernel.  These functions are all
mapped somewhere at 0x112x, and there are some control bits are mixed
up with other functions within the same registers.

This patch modifies example to illustrate child nodes.

Signed-off-by: Ryder Lee 
---
 .../bindings/arm/mediatek/mediatek,audsys.txt | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt 
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
index 9b8f578..97b304e 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
@@ -13,10 +13,19 @@ The AUDSYS controller uses the common clk binding from
 Documentation/devicetree/bindings/clock/clock-bindings.txt
 The available clocks are defined in dt-bindings/clock/mt*-clk.h.
 
+Required sub-nodes:
+---
+For common binding part and usage, refer to
+../sonud/mt2701-afe-pcm.txt.
+
 Example:
 
-audsys: audsys@1122 {
-   compatible = "mediatek,mt7622-audsys", "syscon";
-   reg = <0 0x1122 0 0x1000>;
-   #clock-cells = <1>;
-};
+   audsys: clock-controller@1122 {
+   compatible = "mediatek,mt7622-audsys", "syscon";
+   reg = <0 0x1122 0 0x2000>;
+   #clock-cells = <1>;
+
+   afe: audio-controller {
+   ...
+   };
+   };
-- 
1.9.1

[PATCH v4 3/6] clk: mediatek: add audsys support for MT2701

[Ryder Lee]

Add clock driver support for MT2701 audsys.

Signed-off-by: Ryder Lee 
---
 drivers/clk/mediatek/Kconfig  |   6 ++
 drivers/clk/mediatek/Makefile |   1 +
 drivers/clk/mediatek/clk-mt2701-aud.c | 177 ++
 3 files changed, 184 insertions(+)
 create mode 100644 drivers/clk/mediatek/clk-mt2701-aud.c

diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
index 1f9ea0f..92afe59 100644
--- a/drivers/clk/mediatek/Kconfig
+++ b/drivers/clk/mediatek/Kconfig
@@ -54,6 +54,12 @@ config COMMON_CLK_MT2701_BDPSYS
---help---
  This driver supports MediaTek MT2701 bdpsys clocks.
 
+config COMMON_CLK_MT2701_AUDSYS
+   bool "Clock driver for Mediatek MT2701 audsys"
+   depends on COMMON_CLK_MT2701
+   ---help---
+ This driver supports Mediatek MT2701 audsys clocks.
+
 config COMMON_CLK_MT2712
bool "Clock driver for MediaTek MT2712"
depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index 5160fdc..b80eff2 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_COMMON_CLK_MT6797_MMSYS) += clk-mt6797-mm.o
 obj-$(CONFIG_COMMON_CLK_MT6797_VDECSYS) += clk-mt6797-vdec.o
 obj-$(CONFIG_COMMON_CLK_MT6797_VENCSYS) += clk-mt6797-venc.o
 obj-$(CONFIG_COMMON_CLK_MT2701) += clk-mt2701.o
+obj-$(CONFIG_COMMON_CLK_MT2701_AUDSYS) += clk-mt2701-aud.o
 obj-$(CONFIG_COMMON_CLK_MT2701_BDPSYS) += clk-mt2701-bdp.o
 obj-$(CONFIG_COMMON_CLK_MT2701_ETHSYS) += clk-mt2701-eth.o
 obj-$(CONFIG_COMMON_CLK_MT2701_HIFSYS) += clk-mt2701-hif.o
diff --git a/drivers/clk/mediatek/clk-mt2701-aud.c 
b/drivers/clk/mediatek/clk-mt2701-aud.c
new file mode 100644
index 000..3ccd67d
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt2701-aud.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Ryder Lee 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "clk-mtk.h"
+#include "clk-gate.h"
+
+#include 
+
+#define GATE_AUDIO0(_id, _name, _parent, _shift) { \
+   .id = _id,  \
+   .name = _name,  \
+   .parent_name = _parent, \
+   .regs = &audio0_cg_regs,\
+   .shift = _shift,\
+   .ops = &mtk_clk_gate_ops_no_setclr, \
+   }
+
+#define GATE_AUDIO1(_id, _name, _parent, _shift) { \
+   .id = _id,  \
+   .name = _name,  \
+   .parent_name = _parent, \
+   .regs = &audio1_cg_regs,\
+   .shift = _shift,\
+   .ops = &mtk_clk_gate_ops_no_setclr, \
+   }
+
+#define GATE_AUDIO2(_id, _name, _parent, _shift) { \
+   .id = _id,  \
+   .name = _name,  \
+   .parent_name = _parent, \
+   .regs = &audio2_cg_regs,\
+   .shift = _shift,\
+   .ops = &mtk_clk_gate_ops_no_setclr, \
+   }
+
+#define GATE_AUDIO3(_id, _name, _parent, _shift) { \
+   .id = _id,  \
+   .name = _name,  \
+   .parent_name = _parent, \
+   .regs = &audio3_cg_regs,\
+   .shift = _shift,\
+   .ops = &mtk_clk_gate_ops_no_setclr, \
+   }
+
+static const struct mtk_gate_regs audio0_cg_regs = {
+   .set_ofs = 0x0,
+   .clr_ofs = 0x0,
+   .sta_ofs = 0x0,
+};
+
+static const struct mtk_gate_regs audio1_cg_regs = {
+   .set_ofs = 0x10,
+   .clr_ofs = 0x10,
+   .sta_ofs = 0x10,
+};
+
+static const struct mtk_gate_regs audio2_cg_regs = {
+   .set_ofs = 0x14,
+   .clr_ofs = 0x14,
+   .sta_ofs = 0x14,
+};
+
+static const struct mtk_gate_regs audio3_cg_regs = {
+   .set_ofs = 0x634,
+   .clr_ofs = 0x634,
+   .sta_ofs = 0x634,
+};
+
+static const struct mtk_gate audio_clks[] = {
+   /* AUDIO0 */
+   GATE_AUDIO0(CLK_AUD_AFE, "audio_afe", "aud_intbus_sel", 2),
+   GATE_AUDIO0(CLK_AUD_HDMI, "audio_hdmi", "audpll_sel", 20),
+   GATE_AUDIO0(CLK_AUD_SPDF, "audio_spdf", "audpll_sel", 21),
+   GATE_AUDIO0(CLK_AUD_SPDF2, "audio_spdf2", "audpll_sel", 22),
+   GATE_AUDIO0(CLK_AUD_APLL, "audio_apll", "audpll_sel", 23),
+   /* AUDIO1 */
+   GATE_AUDIO1(CLK_AUD_I2SIN1, "audio_i2sin1", "aud_mux1_sel", 0),
+   GATE_AUDIO1(CLK_AUD_I2SIN2, "audio_i2sin2", "aud_mux1_sel", 1),
+   GATE_AUDIO1(CLK_AUD_I2SIN3, "audio_i2sin3", "aud_mux1_sel", 2),
+   GATE_AUDIO

[PATCH v4 2/6] clk: mediatek: add devm_of_platform_populate() for MT7622 audsys

[Ryder Lee]

Add devm_of_platform_populate() to populate devices which are children
of the root node.

Signed-off-by: Ryder Lee 
---
 drivers/clk/mediatek/clk-mt7622-aud.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt7622-aud.c 
b/drivers/clk/mediatek/clk-mt7622-aud.c
index 13f752d..0a1109f 100644
--- a/drivers/clk/mediatek/clk-mt7622-aud.c
+++ b/drivers/clk/mediatek/clk-mt7622-aud.c
@@ -142,6 +142,7 @@ static int clk_mt7622_audiosys_init(struct platform_device 
*pdev)
 {
struct clk_onecell_data *clk_data;
struct device_node *node = pdev->dev.of_node;
+
int r;
 
clk_data = mtk_alloc_clk_data(CLK_AUDIO_NR_CLK);
@@ -150,12 +151,15 @@ static int clk_mt7622_audiosys_init(struct 
platform_device *pdev)
   clk_data);
 
r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
-   if (r)
+   if (r) {
dev_err(&pdev->dev,
"could not register clock provider: %s: %d\n",
pdev->name, r);
 
-   return r;
+   return r;
+   }
+
+   return devm_of_platform_populate(&pdev->dev);
 }
 
 static const struct of_device_id of_match_clk_mt7622_aud[] = {
-- 
1.9.1

[PATCH v4 5/6] dt-bindings: clock: mediatek: add audsys support for MT2701

[Ryder Lee]

This patch adds a compatible string for MT2701.

Signed-off-by: Ryder Lee 
---
 Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt 
b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
index 97b304e..34a69ba 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
@@ -6,6 +6,7 @@ The MediaTek AUDSYS controller provides various clocks to the 
system.
 Required Properties:
 
 - compatible: Should be one of:
+   - "mediatek,mt2701-audsys", "syscon"
- "mediatek,mt7622-audsys", "syscon"
 - #clock-cells: Must be 1
 
-- 
1.9.1

[PATCH v4 6/6] arm: dts: mediatek: modify audio related nodes for both MT2701 and MT7623

[Ryder Lee]

Modify audio related nodes to reflect the actual usage in binding documents.

Signed-off-by: Ryder Lee 
---
 arch/arm/boot/dts/mt2701.dtsi | 188 -
 arch/arm/boot/dts/mt7623.dtsi | 190 --
 2 files changed, 182 insertions(+), 196 deletions(-)

diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index 05557fc..05cf65c 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -426,104 +426,96 @@
status = "disabled";
};
 
-   afe: audio-controller@1122 {
-   compatible = "mediatek,mt2701-audio";
-   reg = <0 0x1122 0 0x2000>,
- <0 0x112a 0 0x2>;
-   interrupts =  ,
- ;
-   interrupt-names = "afe", "asys";
-   power-domains = <&scpsys MT2701_POWER_DOMAIN_IFR_MSC>;
-
-   clocks = <&infracfg CLK_INFRA_AUDIO>,
-<&topckgen CLK_TOP_AUD_MUX1_SEL>,
-<&topckgen CLK_TOP_AUD_MUX2_SEL>,
-<&topckgen CLK_TOP_AUD_MUX1_DIV>,
-<&topckgen CLK_TOP_AUD_MUX2_DIV>,
-<&topckgen CLK_TOP_AUD_48K_TIMING>,
-<&topckgen CLK_TOP_AUD_44K_TIMING>,
-<&topckgen CLK_TOP_AUDPLL_MUX_SEL>,
-<&topckgen CLK_TOP_APLL_SEL>,
-<&topckgen CLK_TOP_AUD1PLL_98M>,
-<&topckgen CLK_TOP_AUD2PLL_90M>,
-<&topckgen CLK_TOP_HADDS2PLL_98M>,
-<&topckgen CLK_TOP_HADDS2PLL_294M>,
-<&topckgen CLK_TOP_AUDPLL>,
-<&topckgen CLK_TOP_AUDPLL_D4>,
-<&topckgen CLK_TOP_AUDPLL_D8>,
-<&topckgen CLK_TOP_AUDPLL_D16>,
-<&topckgen CLK_TOP_AUDPLL_D24>,
-<&topckgen CLK_TOP_AUDINTBUS_SEL>,
-<&clk26m>,
-<&topckgen CLK_TOP_SYSPLL1_D4>,
-<&topckgen CLK_TOP_AUD_K1_SRC_SEL>,
-<&topckgen CLK_TOP_AUD_K2_SRC_SEL>,
-<&topckgen CLK_TOP_AUD_K3_SRC_SEL>,
-<&topckgen CLK_TOP_AUD_K4_SRC_SEL>,
-<&topckgen CLK_TOP_AUD_K5_SRC_SEL>,
-<&topckgen CLK_TOP_AUD_K6_SRC_SEL>,
-<&topckgen CLK_TOP_AUD_K1_SRC_DIV>,
-<&topckgen CLK_TOP_AUD_K2_SRC_DIV>,
-<&topckgen CLK_TOP_AUD_K3_SRC_DIV>,
-<&topckgen CLK_TOP_AUD_K4_SRC_DIV>,
-<&topckgen CLK_TOP_AUD_K5_SRC_DIV>,
-<&topckgen CLK_TOP_AUD_K6_SRC_DIV>,
-<&topckgen CLK_TOP_AUD_I2S1_MCLK>,
-<&topckgen CLK_TOP_AUD_I2S2_MCLK>,
-<&topckgen CLK_TOP_AUD_I2S3_MCLK>,
-<&topckgen CLK_TOP_AUD_I2S4_MCLK>,
-<&topckgen CLK_TOP_AUD_I2S5_MCLK>,
-<&topckgen CLK_TOP_AUD_I2S6_MCLK>,
-<&topckgen CLK_TOP_ASM_M_SEL>,
-<&topckgen CLK_TOP_ASM_H_SEL>,
-<&topckgen CLK_TOP_UNIVPLL2_D4>,
-<&topckgen CLK_TOP_UNIVPLL2_D2>,
-<&topckgen CLK_TOP_SYSPLL_D5>;
-
-   clock-names = "infra_sys_audio_clk",
-"top_audio_mux1_sel",
-"top_audio_mux2_sel",
-"top_audio_mux1_div",
-"top_audio_mux2_div",
-"top_audio_48k_timing",
-"top_audio_44k_timing",
-"top_audpll_mux_sel",
-"top_apll_sel",
-"top_aud1_pll_98M",
-"top_aud2_pll_90M",
-"top_hadds2_pll_98M",
-"top_hadds2_pll_294M",
-"top_audpll",
-"top_audpll_d4",
-"top_audpll_d8",
-"top_audpll_d16",
-"top_audpll_d24",
-"top_audintbus_sel",
-"clk_26m",
-"top_syspll1_d4",
-"top_aud_k1_src_sel",
-"top_aud_k2_src_sel",
-"top_aud_k3_src_sel",
-"top_aud_k4_src_sel",
-"top_aud_k5_src_sel",
-"top_aud_k6_src_sel",
-"top_aud_k1_src_div",
-"top_aud_k2_src_div",
-"top_aud_k3_src_div",
-"top_aud_k4_src_div",
-"top_aud_k5_src_div",
-

[RFC/RFT][PATCH v2 6/6] time: tick-sched: Avoid running the same code twice in a row

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

To avoid running the same piece of code twice in a row, move the
tick stopping part of __tick_nohz_next_event() into a new function
called __tick_nohz_stop_tick() and invoke them both separately.

Make __tick_nohz_idle_enter() avoid calling __tick_nohz_next_event()
if it has been called already by tick_nohz_get_sleep_length() and
use the new next_idle_tick field in struct tick_sched to pass the
next event time value between tick_nohz_get_sleep_length() and
__tick_nohz_idle_enter().

Signed-off-by: Rafael J. Wysocki 
---

-> v2: No changes.

---
 kernel/time/tick-sched.c |  130 ++-
 kernel/time/tick-sched.h |1 
 2 files changed, 73 insertions(+), 58 deletions(-)

Index: linux-pm/kernel/time/tick-sched.c
===
--- linux-pm.orig/kernel/time/tick-sched.c
+++ linux-pm/kernel/time/tick-sched.c
@@ -655,13 +655,10 @@ static inline bool local_timer_softirq_p
return local_softirq_pending() & TIMER_SOFTIRQ;
 }
 
-static ktime_t __tick_nohz_next_event(struct tick_sched *ts, int cpu,
- bool stop_tick)
+static ktime_t __tick_nohz_next_event(struct tick_sched *ts, int cpu)
 {
-   struct clock_event_device *dev = 
__this_cpu_read(tick_cpu_device.evtdev);
u64 basemono, next_tick, next_tmr, next_rcu, delta, expires;
unsigned long seq, basejiff;
-   ktime_t tick;
 
/* Read jiffies and the time when jiffies were updated last */
do {
@@ -714,34 +711,23 @@ static ktime_t __tick_nohz_next_event(st
 * We've not stopped the tick yet, and there's a timer in the
 * next period, so no point in stopping it either, bail.
 */
-   if (!ts->tick_stopped) {
-   tick = 0;
-   goto out;
-   }
+   if (!ts->tick_stopped)
+   return 0;
}
 
/*
-* If this CPU is the one which updates jiffies, then give up
-* the assignment and let it be taken by the CPU which runs
-* the tick timer next, which might be this CPU as well. If we
-* don't drop this here the jiffies might be stale and
-* do_timer() never invoked. Keep track of the fact that it
-* was the one which had the do_timer() duty last. If this CPU
-* is the one which had the do_timer() duty last, we limit the
-* sleep time to the timekeeping max_deferment value.
+* If this CPU is the one which had the do_timer() duty last, we limit
+* the sleep time to the timekeeping max_deferment value.
 * Otherwise we can sleep as long as we want.
 */
delta = timekeeping_max_deferment();
-   if (cpu == tick_do_timer_cpu) {
-   if (stop_tick) {
-   tick_do_timer_cpu = TICK_DO_TIMER_NONE;
-   ts->do_timer_last = 1;
+   if (cpu != tick_do_timer_cpu) {
+   if (tick_do_timer_cpu != TICK_DO_TIMER_NONE) {
+   delta = KTIME_MAX;
+   ts->do_timer_last = 0;
+   } else if (!ts->do_timer_last) {
+   delta = KTIME_MAX;
}
-   } else if (tick_do_timer_cpu != TICK_DO_TIMER_NONE) {
-   delta = KTIME_MAX;
-   ts->do_timer_last = 0;
-   } else if (!ts->do_timer_last) {
-   delta = KTIME_MAX;
}
 
 #ifdef CONFIG_NO_HZ_FULL
@@ -756,24 +742,37 @@ static ktime_t __tick_nohz_next_event(st
else
expires = KTIME_MAX;
 
-   expires = min_t(u64, expires, next_tick);
-   tick = expires;
+   ts->next_idle_tick = min_t(u64, expires, next_tick);
+   return ts->next_idle_tick;
+}
 
-   if (!stop_tick) {
-   /* Undo the effect of get_next_timer_interrupt(). */
-   timer_clear_idle();
-   goto out;
+static void __tick_nohz_stop_tick(struct tick_sched *ts, int cpu, u64 expires)
+{
+   struct clock_event_device *dev = 
__this_cpu_read(tick_cpu_device.evtdev);
+   ktime_t tick = expires;
+
+   /*
+* If this CPU is the one which updates jiffies, then give up
+* the assignment and let it be taken by the CPU which runs
+* the tick timer next, which might be this CPU as well. If we
+* don't drop this here the jiffies might be stale and
+* do_timer() never invoked. Keep track of the fact that it
+* was the one which had the do_timer() duty last.
+*/
+   if (cpu == tick_do_timer_cpu) {
+   tick_do_timer_cpu = TICK_DO_TIMER_NONE;
+   ts->do_timer_last = 1;
}
 
/* Skip reprogram of event if its not changed */
if (ts->tick_stopped && (expires == ts->next_tick)) {
/* Sanity check: make sure clockevent is actually programmed */
if (tick == KTIME_M

[PATCH] staging: fsl-mc/dpio: remove unused function

[Anders Roxell]

gcc warns that function 'qbman_pull_desc_set_token' is not used.

drivers/staging/fsl-mc/bus/dpio/qbman-portal.c:525:13: warning: 
‘qbman_pull_desc_set_token’ defined but not used [-Wunused-function]

In the current code we remove that function.

Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
Signed-off-by: Anders Roxell 
---
 drivers/staging/fsl-mc/bus/dpio/qbman-portal.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c 
b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
index 7b75c934b201..3522b9c0106b 100644
--- a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
+++ b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
@@ -522,11 +522,6 @@ void qbman_pull_desc_set_numframes(struct qbman_pull_desc 
*d, u8 numframes)
d->numf = numframes - 1;
 }
 
-static void qbman_pull_desc_set_token(struct qbman_pull_desc *d, u8 token)
-{
-   d->tok = token;
-}
-
 /*
  * Exactly one of the following descriptor "actions" should be set. (Calling 
any
  * one of these will replace the effect of any prior call to one of these.)
-- 
2.11.0

[RFC/RFT][PATCH v2 4/6] cpuidle: Return nohz hint from cpuidle_select()

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

Add a new pointer argument to cpuidle_select() and to the ->select
cpuidle governor callback to allow a boolean value indicating
whether or not the tick should be stopped before entering the
selected state to be returned from there.

Make the ladder governor ignore that pointer (to preserve its
current behavior) and make the menu governor return 'false" through
it if:
 (1) the idle exit latency is constrained at 0,
 (2) the selected state is a polling one, or
 (3) the target residency of the selected state is less than the
 length of the tick period and there is at least one deeper
 idle state available within the tick period range.

Since the value returned through the new argument pointer is not
used yet, this change is not expected to alter the functionality of
the code.

Signed-off-by: Rafael J. Wysocki 
---

-> v2: New patch.

---
 drivers/cpuidle/cpuidle.c  |   10 --
 drivers/cpuidle/governors/ladder.c |3 ++-
 drivers/cpuidle/governors/menu.c   |   26 ++
 include/linux/cpuidle.h|6 --
 kernel/sched/idle.c|4 +++-
 5 files changed, 39 insertions(+), 10 deletions(-)

Index: linux-pm/include/linux/cpuidle.h
===
--- linux-pm.orig/include/linux/cpuidle.h
+++ linux-pm/include/linux/cpuidle.h
@@ -131,7 +131,8 @@ extern bool cpuidle_not_available(struct
  struct cpuidle_device *dev);
 
 extern int cpuidle_select(struct cpuidle_driver *drv,
- struct cpuidle_device *dev);
+ struct cpuidle_device *dev,
+ bool *nohz_ret);
 extern int cpuidle_enter(struct cpuidle_driver *drv,
 struct cpuidle_device *dev, int index);
 extern void cpuidle_reflect(struct cpuidle_device *dev, int index);
@@ -246,7 +247,8 @@ struct cpuidle_governor {
struct cpuidle_device *dev);
 
int  (*select)  (struct cpuidle_driver *drv,
-   struct cpuidle_device *dev);
+   struct cpuidle_device *dev,
+   bool *nohz_ret);
void (*reflect) (struct cpuidle_device *dev, int index);
 };
 
Index: linux-pm/kernel/sched/idle.c
===
--- linux-pm.orig/kernel/sched/idle.c
+++ linux-pm/kernel/sched/idle.c
@@ -186,13 +186,15 @@ static void cpuidle_idle_call(void)
next_state = cpuidle_find_deepest_state(drv, dev);
call_cpuidle(drv, dev, next_state);
} else {
+   bool nohz = true;
+
tick_nohz_idle_go_idle(true);
rcu_idle_enter();
 
/*
 * Ask the cpuidle framework to choose a convenient idle state.
 */
-   next_state = cpuidle_select(drv, dev);
+   next_state = cpuidle_select(drv, dev, &nohz);
entered_state = call_cpuidle(drv, dev, next_state);
/*
 * Give the governor an opportunity to reflect on the outcome
Index: linux-pm/drivers/cpuidle/cpuidle.c
===
--- linux-pm.orig/drivers/cpuidle/cpuidle.c
+++ linux-pm/drivers/cpuidle/cpuidle.c
@@ -263,12 +263,18 @@ int cpuidle_enter_state(struct cpuidle_d
  *
  * @drv: the cpuidle driver
  * @dev: the cpuidle device
+ * @nohz_ret: indication on whether or not to stop the tick
  *
  * Returns the index of the idle state.  The return value must not be negative.
+ *
+ * The memory location pointed to by @nohz_ret is expected to be written the
+ * 'false' boolean value if the scheduler tick should not be stopped before
+ * entering the returned state.
  */
-int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
+int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
+  bool *nohz_ret)
 {
-   return cpuidle_curr_governor->select(drv, dev);
+   return cpuidle_curr_governor->select(drv, dev, nohz_ret);
 }
 
 /**
Index: linux-pm/drivers/cpuidle/governors/ladder.c
===
--- linux-pm.orig/drivers/cpuidle/governors/ladder.c
+++ linux-pm/drivers/cpuidle/governors/ladder.c
@@ -63,9 +63,10 @@ static inline void ladder_do_selection(s
  * ladder_select_state - selects the next state to enter
  * @drv: cpuidle driver
  * @dev: the CPU
+ * @dummy: not used
  */
 static int ladder_select_state(struct cpuidle_driver *drv,
-   struct cpuidle_device *dev)
+  struct cpuidle_device *dev, bool *dummy)
 {
struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
struct device *device = get_cpu_device(dev->cpu);
Index: linux-pm/drivers/cpuidle/gover

[RFC/RFT][PATCH v2 2/6] sched: idle: Do not stop the tick upfront in the idle loop

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

Push the decision whether or not to stop the tick somewhat deeper
into the idle loop.

Stopping the tick upfront leads to unpleasant outcomes in case the
idle governor doesn't agree with the timekeeping code on the duration
of the upcoming idle period.  Specifically, if the tick has been
stopped and the idle governor predicts short idle, the situation is
bad regardless of whether or not the prediction is accurate.  If it
is accurate, the tick has been stopped unnecessarily which means
excessive overhead.  If it is not accurate, the CPU is likely to
spend too much time in the (shallow, because short idle has been
predicted) idle state selected by the governor [1].

As the first step towards addressing this problem, change the code
to make the tick stopping decision inside of the loop in do_idle().
In particular, do not stop the tick in the cpu_idle_poll() code path.
Also don't do that in tick_nohz_irq_exit() which doesn't really have
information to whether or not to stop the tick.

Link: https://marc.info/?l=linux-pm&m=150116085925208&w=2 # [1]
Link: 
https://tu-dresden.de/zih/forschung/ressourcen/dateien/projekte/haec/powernightmares.pdf
Signed-off-by: Rafael J. Wysocki 
---

-> v2: No changes.

---
 kernel/sched/idle.c  |   13 ++---
 kernel/time/tick-sched.c |2 +-
 2 files changed, 11 insertions(+), 4 deletions(-)

Index: linux-pm/kernel/sched/idle.c
===
--- linux-pm.orig/kernel/sched/idle.c
+++ linux-pm/kernel/sched/idle.c
@@ -220,13 +220,17 @@ static void do_idle(void)
 */
 
__current_set_polling();
-   tick_nohz_idle_enter();
+   tick_nohz_idle_prepare();
 
while (!need_resched()) {
check_pgt_cache();
rmb();
 
if (cpu_is_offline(cpu)) {
+   local_irq_disable();
+   tick_nohz_idle_go_idle(true);
+   local_irq_enable();
+
cpuhp_report_idle_dead();
arch_cpu_idle_dead();
}
@@ -240,10 +244,13 @@ static void do_idle(void)
 * broadcast device expired for us, we don't want to go deep
 * idle as we know that the IPI is going to arrive right away.
 */
-   if (cpu_idle_force_poll || tick_check_broadcast_expired())
+   if (cpu_idle_force_poll || tick_check_broadcast_expired()) {
+   tick_nohz_idle_go_idle(false);
cpu_idle_poll();
-   else
+   } else {
+   tick_nohz_idle_go_idle(true);
cpuidle_idle_call();
+   }
arch_cpu_idle_exit();
}
 
Index: linux-pm/kernel/time/tick-sched.c
===
--- linux-pm.orig/kernel/time/tick-sched.c
+++ linux-pm/kernel/time/tick-sched.c
@@ -1007,7 +1007,7 @@ void tick_nohz_irq_exit(void)
struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
 
if (ts->inidle)
-   __tick_nohz_idle_enter(ts, true);
+   __tick_nohz_idle_enter(ts, false);
else
tick_nohz_full_update_tick(ts);
 }

[PATCH 7/7] perf auxtrace: Make auxtrace_queues__add_buffer() do CPU filtering

[Adrian Hunter]

In preparation for supporting AUX area sampling buffers,
auxtrace_queues__add_buffer() needs to be more generic. To that end, move
CPU filtering into it.

Signed-off-by: Adrian Hunter 
---
 tools/perf/util/auxtrace.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index e1aff91c54a8..857de69a5361 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -302,6 +302,13 @@ static int auxtrace_queues__split_buffer(struct 
auxtrace_queues *queues,
return 0;
 }
 
+static bool filter_cpu(struct perf_session *session, int cpu)
+{
+   unsigned long *cpu_bitmap = session->itrace_synth_opts->cpu_bitmap;
+
+   return cpu_bitmap && cpu != -1 && !test_bit(cpu, cpu_bitmap);
+}
+
 static int auxtrace_queues__add_buffer(struct auxtrace_queues *queues,
   struct perf_session *session,
   unsigned int idx,
@@ -310,6 +317,9 @@ static int auxtrace_queues__add_buffer(struct 
auxtrace_queues *queues,
 {
int err = -ENOMEM;
 
+   if (filter_cpu(session, buffer->cpu))
+   return 0;
+
buffer = memdup(buffer, sizeof(*buffer));
if (!buffer)
return -ENOMEM;
@@ -344,13 +354,6 @@ static int auxtrace_queues__add_buffer(struct 
auxtrace_queues *queues,
return err;
 }
 
-static bool filter_cpu(struct perf_session *session, int cpu)
-{
-   unsigned long *cpu_bitmap = session->itrace_synth_opts->cpu_bitmap;
-
-   return cpu_bitmap && cpu != -1 && !test_bit(cpu, cpu_bitmap);
-}
-
 int auxtrace_queues__add_event(struct auxtrace_queues *queues,
   struct perf_session *session,
   union perf_event *event, off_t data_offset,
@@ -367,9 +370,6 @@ int auxtrace_queues__add_event(struct auxtrace_queues 
*queues,
};
unsigned int idx = event->auxtrace.idx;
 
-   if (filter_cpu(session, event->auxtrace.cpu))
-   return 0;
-
return auxtrace_queues__add_buffer(queues, session, idx, &buffer,
   buffer_ptr);
 }
-- 
1.9.1

[RFC/RFT][PATCH v2 3/6] sched: idle: Do not stop the tick before cpuidle_idle_call()

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

Make cpuidle_idle_call() decide whether or not to stop the tick.

First, there are code paths in cpuidle_idle_call() that don't need
the tick to be stopped.  In particular, the cpuidle_enter_s2idle()
path deals with the tick (and with the entire timekeeping for that
matter) by itself and it doesn't need the tick to be stopped
beforehand.

Second, to address the issue with short idle duration predictions
by the idle governor after the tick has been stopped, it will be
necessary to change the ordering of some governor code with respect
to some code in tick_nohz_idle_go_idle().  For this purpose, call
tick_nohz_idle_go_idle() in the same branch as cpuidle_select().

Signed-off-by: Rafael J. Wysocki 
---

-> v2: No changes.

---
 kernel/sched/idle.c |   17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

Index: linux-pm/kernel/sched/idle.c
===
--- linux-pm.orig/kernel/sched/idle.c
+++ linux-pm/kernel/sched/idle.c
@@ -146,13 +146,14 @@ static void cpuidle_idle_call(void)
}
 
/*
-* Tell the RCU framework we are entering an idle section,
-* so no more rcu read side critical sections and one more
+* The RCU framework needs to be told that we are entering an idle
+* section, so no more rcu read side critical sections and one more
 * step to the grace period
 */
-   rcu_idle_enter();
 
if (cpuidle_not_available(drv, dev)) {
+   tick_nohz_idle_go_idle(false);
+   rcu_idle_enter();
default_idle_call();
goto exit_idle;
}
@@ -169,6 +170,9 @@ static void cpuidle_idle_call(void)
 
if (idle_should_enter_s2idle() || dev->use_deepest_state) {
if (idle_should_enter_s2idle()) {
+   tick_nohz_idle_go_idle(false);
+   rcu_idle_enter();
+
entered_state = cpuidle_enter_s2idle(drv, dev);
if (entered_state > 0) {
local_irq_enable();
@@ -176,9 +180,15 @@ static void cpuidle_idle_call(void)
}
}
 
+   tick_nohz_idle_go_idle(true);
+   rcu_idle_enter();
+
next_state = cpuidle_find_deepest_state(drv, dev);
call_cpuidle(drv, dev, next_state);
} else {
+   tick_nohz_idle_go_idle(true);
+   rcu_idle_enter();
+
/*
 * Ask the cpuidle framework to choose a convenient idle state.
 */
@@ -248,7 +258,6 @@ static void do_idle(void)
tick_nohz_idle_go_idle(false);
cpu_idle_poll();
} else {
-   tick_nohz_idle_go_idle(true);
cpuidle_idle_call();
}
arch_cpu_idle_exit();

[RFC/RFT][PATCH v2 1/6] time: tick-sched: Reorganize idle tick management code

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

Prepare two pieces of code in tick_nohz_idle_enter() for being
called separately from each other.

First, make it possible to call the initial preparatory part of
tick_nohz_idle_enter() without the tick-stopping part following
it and introduce the tick_nohz_idle_prepare() wrapper for that
(that will be used in the next set of changes).

Second, add a new stop_tick argument to __tick_nohz_idle_enter()
tell it whether or not to stop the tick (that is always set for
now) and add a wrapper allowing this function to be called from
the outside of tick-sched.c.

Just the code reorganization and two new wrapper functions, no
intended functional changes.

Signed-off-by: Rafael J. Wysocki 
---

-> v2: Eliminate assumetry in enabling/disabling interrupts.

---
 include/linux/tick.h |2 +
 kernel/time/tick-sched.c |   62 ---
 2 files changed, 45 insertions(+), 19 deletions(-)

Index: linux-pm/include/linux/tick.h
===
--- linux-pm.orig/include/linux/tick.h
+++ linux-pm/include/linux/tick.h
@@ -114,6 +114,8 @@ enum tick_dep_bits {
 #ifdef CONFIG_NO_HZ_COMMON
 extern bool tick_nohz_enabled;
 extern int tick_nohz_tick_stopped(void);
+extern void tick_nohz_idle_prepare(void);
+extern void tick_nohz_idle_go_idle(bool stop_tick);
 extern void tick_nohz_idle_enter(void);
 extern void tick_nohz_idle_exit(void);
 extern void tick_nohz_irq_exit(void);
Index: linux-pm/kernel/time/tick-sched.c
===
--- linux-pm.orig/kernel/time/tick-sched.c
+++ linux-pm/kernel/time/tick-sched.c
@@ -911,14 +911,14 @@ static bool can_stop_idle_tick(int cpu,
return true;
 }
 
-static void __tick_nohz_idle_enter(struct tick_sched *ts)
+static void __tick_nohz_idle_enter(struct tick_sched *ts, bool stop_tick)
 {
ktime_t now, expires;
int cpu = smp_processor_id();
 
now = tick_nohz_start_idle(ts);
 
-   if (can_stop_idle_tick(cpu, ts)) {
+   if (can_stop_idle_tick(cpu, ts) && stop_tick) {
int was_stopped = ts->tick_stopped;
 
ts->idle_calls++;
@@ -936,22 +936,8 @@ static void __tick_nohz_idle_enter(struc
}
 }
 
-/**
- * tick_nohz_idle_enter - stop the idle tick from the idle task
- *
- * When the next event is more than a tick into the future, stop the idle tick
- * Called when we start the idle loop.
- *
- * The arch is responsible of calling:
- *
- * - rcu_idle_enter() after its last use of RCU before the CPU is put
- *  to sleep.
- * - rcu_idle_exit() before the first use of RCU after the CPU is woken up.
- */
-void tick_nohz_idle_enter(void)
+void __tick_nohz_idle_prepare(void)
 {
-   struct tick_sched *ts;
-
lockdep_assert_irqs_enabled();
/*
 * Update the idle state in the scheduler domain hierarchy
@@ -960,12 +946,50 @@ void tick_nohz_idle_enter(void)
 * exiting idle.
 */
set_cpu_sd_state_idle();
+}
+
+/**
+ * tick_nohz_idle_prepare - prepare for entering idle on the current CPU.
+ *
+ * Called when we start the idle loop.
+ */
+void tick_nohz_idle_prepare(void)
+{
+   struct tick_sched *ts;
+
+   __tick_nohz_idle_prepare();
+
+   local_irq_disable();
+
+   ts = this_cpu_ptr(&tick_cpu_sched);
+   ts->inidle = 1;
+
+   local_irq_enable();
+}
+
+/**
+ * tick_nohz_idle_go_idle - start idle period on the current CPU.
+ * @stop_tick: Whether or not to stop the idle tick.
+ *
+ * When @stop_tick is set and the next event is more than a tick into the
+ * future, stop the idle tick.
+ */
+void tick_nohz_idle_go_idle(bool stop_tick)
+{
+   __tick_nohz_idle_enter(this_cpu_ptr(&tick_cpu_sched), stop_tick);
+}
+
+void tick_nohz_idle_enter(void)
+{
+   struct tick_sched *ts;
+
+   __tick_nohz_idle_prepare();
 
local_irq_disable();
 
ts = this_cpu_ptr(&tick_cpu_sched);
ts->inidle = 1;
-   __tick_nohz_idle_enter(ts);
+   __tick_nohz_idle_enter(ts, true);
 
local_irq_enable();
 }
@@ -983,7 +1007,7 @@ void tick_nohz_irq_exit(void)
struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
 
if (ts->inidle)
-   __tick_nohz_idle_enter(ts);
+   __tick_nohz_idle_enter(ts, true);
else
tick_nohz_full_update_tick(ts);
 }

[PATCH 3/7] perf auxtrace: Add missing parameters from kernel-doc comments

[Adrian Hunter]

Add missing parameters from kernel-doc comments.

Signed-off-by: Adrian Hunter 
---
 tools/perf/util/auxtrace.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 453c148d2158..e731f55da072 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -130,6 +130,7 @@ struct auxtrace_index {
 /**
  * struct auxtrace - session callbacks to allow AUX area data decoding.
  * @process_event: lets the decoder see all session events
+ * @process_auxtrace_event: process a PERF_RECORD_AUXTRACE event
  * @flush_events: process any remaining data
  * @free_events: free resources associated with event processing
  * @free: free resources associated with the session
@@ -301,6 +302,7 @@ struct auxtrace_mmap_params {
  * @parse_snapshot_options: parse snapshot options
  * @reference: provide a 64-bit reference number for auxtrace_event
  * @read_finish: called after reading from an auxtrace mmap
+ * @alignment: alignment (if any) for AUX area data
  */
 struct auxtrace_record {
int (*recording_options)(struct auxtrace_record *itr,
-- 
1.9.1

[PATCH 6/7] perf auxtrace: Make auxtrace_queues__add_buffer() allocate struct buffer

[Adrian Hunter]

In preparation for supporting AUX area sampling buffers,
auxtrace_queues__add_buffer() needs to be more generic. To that end, move
memory allocation for struct buffer into it.

Signed-off-by: Adrian Hunter 
---
 tools/perf/util/auxtrace.c | 54 +-
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index fb357a00dd86..e1aff91c54a8 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -308,7 +308,11 @@ static int auxtrace_queues__add_buffer(struct 
auxtrace_queues *queues,
   struct auxtrace_buffer *buffer,
   struct auxtrace_buffer **buffer_ptr)
 {
-   int err;
+   int err = -ENOMEM;
+
+   buffer = memdup(buffer, sizeof(*buffer));
+   if (!buffer)
+   return -ENOMEM;
 
if (session->one_mmap) {
buffer->data = buffer->data_offset - session->one_mmap_offset +
@@ -316,24 +320,28 @@ static int auxtrace_queues__add_buffer(struct 
auxtrace_queues *queues,
} else if (perf_data__is_pipe(session->data)) {
buffer->data = auxtrace_copy_data(buffer->size, session);
if (!buffer->data)
-   return -ENOMEM;
+   goto out_free;
buffer->data_needs_freeing = true;
} else if (BITS_PER_LONG == 32 &&
   buffer->size > BUFFER_LIMIT_FOR_32_BIT) {
err = auxtrace_queues__split_buffer(queues, idx, buffer);
if (err)
-   return err;
+   goto out_free;
}
 
err = auxtrace_queues__queue_buffer(queues, idx, buffer);
if (err)
-   return err;
+   goto out_free;
 
/* FIXME: Doesn't work for split buffer */
if (buffer_ptr)
*buffer_ptr = buffer;
 
return 0;
+
+out_free:
+   auxtrace_buffer__free(buffer);
+   return err;
 }
 
 static bool filter_cpu(struct perf_session *session, int cpu)
@@ -348,36 +356,22 @@ int auxtrace_queues__add_event(struct auxtrace_queues 
*queues,
   union perf_event *event, off_t data_offset,
   struct auxtrace_buffer **buffer_ptr)
 {
-   struct auxtrace_buffer *buffer;
-   unsigned int idx;
-   int err;
+   struct auxtrace_buffer buffer = {
+   .pid = -1,
+   .tid = event->auxtrace.tid,
+   .cpu = event->auxtrace.cpu,
+   .data_offset = data_offset,
+   .offset = event->auxtrace.offset,
+   .reference = event->auxtrace.reference,
+   .size = event->auxtrace.size,
+   };
+   unsigned int idx = event->auxtrace.idx;
 
if (filter_cpu(session, event->auxtrace.cpu))
return 0;
 
-   buffer = zalloc(sizeof(struct auxtrace_buffer));
-   if (!buffer)
-   return -ENOMEM;
-
-   buffer->pid = -1;
-   buffer->tid = event->auxtrace.tid;
-   buffer->cpu = event->auxtrace.cpu;
-   buffer->data_offset = data_offset;
-   buffer->offset = event->auxtrace.offset;
-   buffer->reference = event->auxtrace.reference;
-   buffer->size = event->auxtrace.size;
-   idx = event->auxtrace.idx;
-
-   err = auxtrace_queues__add_buffer(queues, session, idx, buffer,
- buffer_ptr);
-   if (err)
-   goto out_err;
-
-   return 0;
-
-out_err:
-   auxtrace_buffer__free(buffer);
-   return err;
+   return auxtrace_queues__add_buffer(queues, session, idx, &buffer,
+  buffer_ptr);
 }
 
 static int auxtrace_queues__add_indexed_event(struct auxtrace_queues *queues,
-- 
1.9.1

[PATCH 5/7] perf auxtrace: Make auxtrace_queues__add_buffer() return buffer_ptr

[Adrian Hunter]

In preparation for supporting AUX area sampling buffers,
auxtrace_queues__add_buffer() needs to be more generic. To that end, make
it return buffer_ptr instead of the caller.

Signed-off-by: Adrian Hunter 
---
 tools/perf/util/auxtrace.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 6ea840ec5b7f..fb357a00dd86 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -305,8 +305,11 @@ static int auxtrace_queues__split_buffer(struct 
auxtrace_queues *queues,
 static int auxtrace_queues__add_buffer(struct auxtrace_queues *queues,
   struct perf_session *session,
   unsigned int idx,
-  struct auxtrace_buffer *buffer)
+  struct auxtrace_buffer *buffer,
+  struct auxtrace_buffer **buffer_ptr)
 {
+   int err;
+
if (session->one_mmap) {
buffer->data = buffer->data_offset - session->one_mmap_offset +
   session->one_mmap_addr;
@@ -317,14 +320,20 @@ static int auxtrace_queues__add_buffer(struct 
auxtrace_queues *queues,
buffer->data_needs_freeing = true;
} else if (BITS_PER_LONG == 32 &&
   buffer->size > BUFFER_LIMIT_FOR_32_BIT) {
-   int err;
-
err = auxtrace_queues__split_buffer(queues, idx, buffer);
if (err)
return err;
}
 
-   return auxtrace_queues__queue_buffer(queues, idx, buffer);
+   err = auxtrace_queues__queue_buffer(queues, idx, buffer);
+   if (err)
+   return err;
+
+   /* FIXME: Doesn't work for split buffer */
+   if (buffer_ptr)
+   *buffer_ptr = buffer;
+
+   return 0;
 }
 
 static bool filter_cpu(struct perf_session *session, int cpu)
@@ -359,13 +368,11 @@ int auxtrace_queues__add_event(struct auxtrace_queues 
*queues,
buffer->size = event->auxtrace.size;
idx = event->auxtrace.idx;
 
-   err = auxtrace_queues__add_buffer(queues, session, idx, buffer);
+   err = auxtrace_queues__add_buffer(queues, session, idx, buffer,
+ buffer_ptr);
if (err)
goto out_err;
 
-   if (buffer_ptr)
-   *buffer_ptr = buffer;
-
return 0;
 
 out_err:
-- 
1.9.1

[PATCH 0/7] perf tools: Prepare for AUX area sampling support

[Adrian Hunter]

Hi

Here are a few patches to help clear the way for adding AUX area sampling
support.  There is one minor fix "Prevent decoding when --no-itrace" but
otherwise there are no functional changes.


Adrian Hunter (7):
  perf record: Combine some auxtrace initialization into a single function
  perf auxtrace: Prevent decoding when --no-itrace
  perf auxtrace: Add missing parameters from kernel-doc comments
  perf auxtrace: Rename some buffer-queuing functions
  perf auxtrace: Make auxtrace_queues__add_buffer() return buffer_ptr
  perf auxtrace: Make auxtrace_queues__add_buffer() allocate struct buffer
  perf auxtrace: Make auxtrace_queues__add_buffer() do CPU filtering

 tools/perf/builtin-record.c |  36 +-
 tools/perf/util/auxtrace.c  | 114 +++-
 tools/perf/util/auxtrace.h  |   2 +
 3 files changed, 85 insertions(+), 67 deletions(-)


Regards
Adrian

[PATCH 4/7] perf auxtrace: Rename some buffer-queuing functions

[Adrian Hunter]

Rename some buffer-queuing functions in preparation for supporting AUX area
sampling buffers.

Signed-off-by: Adrian Hunter 
---
 tools/perf/util/auxtrace.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 6470ea2aa25e..6ea840ec5b7f 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -233,9 +233,9 @@ static void *auxtrace_copy_data(u64 size, struct 
perf_session *session)
return p;
 }
 
-static int auxtrace_queues__add_buffer(struct auxtrace_queues *queues,
-  unsigned int idx,
-  struct auxtrace_buffer *buffer)
+static int auxtrace_queues__queue_buffer(struct auxtrace_queues *queues,
+unsigned int idx,
+struct auxtrace_buffer *buffer)
 {
struct auxtrace_queue *queue;
int err;
@@ -286,7 +286,7 @@ static int auxtrace_queues__split_buffer(struct 
auxtrace_queues *queues,
return -ENOMEM;
b->size = BUFFER_LIMIT_FOR_32_BIT;
b->consecutive = consecutive;
-   err = auxtrace_queues__add_buffer(queues, idx, b);
+   err = auxtrace_queues__queue_buffer(queues, idx, b);
if (err) {
auxtrace_buffer__free(b);
return err;
@@ -302,10 +302,10 @@ static int auxtrace_queues__split_buffer(struct 
auxtrace_queues *queues,
return 0;
 }
 
-static int auxtrace_queues__add_event_buffer(struct auxtrace_queues *queues,
-struct perf_session *session,
-unsigned int idx,
-struct auxtrace_buffer *buffer)
+static int auxtrace_queues__add_buffer(struct auxtrace_queues *queues,
+  struct perf_session *session,
+  unsigned int idx,
+  struct auxtrace_buffer *buffer)
 {
if (session->one_mmap) {
buffer->data = buffer->data_offset - session->one_mmap_offset +
@@ -324,7 +324,7 @@ static int auxtrace_queues__add_event_buffer(struct 
auxtrace_queues *queues,
return err;
}
 
-   return auxtrace_queues__add_buffer(queues, idx, buffer);
+   return auxtrace_queues__queue_buffer(queues, idx, buffer);
 }
 
 static bool filter_cpu(struct perf_session *session, int cpu)
@@ -359,7 +359,7 @@ int auxtrace_queues__add_event(struct auxtrace_queues 
*queues,
buffer->size = event->auxtrace.size;
idx = event->auxtrace.idx;
 
-   err = auxtrace_queues__add_event_buffer(queues, session, idx, buffer);
+   err = auxtrace_queues__add_buffer(queues, session, idx, buffer);
if (err)
goto out_err;
 
-- 
1.9.1

[PATCH 2/7] perf auxtrace: Prevent decoding when --no-itrace

[Adrian Hunter]

Prevent auxtrace_queues__process_index() from queuing AUX area data for
decoding when the --no-itrace option has been used.

Signed-off-by: Adrian Hunter 
---
 tools/perf/util/auxtrace.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 9faf3b5367db..6470ea2aa25e 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -60,6 +60,12 @@
 #include "sane_ctype.h"
 #include "symbol/kallsyms.h"
 
+static bool auxtrace__dont_decode(struct perf_session *session)
+{
+   return !session->itrace_synth_opts ||
+  session->itrace_synth_opts->dont_decode;
+}
+
 int auxtrace_mmap__mmap(struct auxtrace_mmap *mm,
struct auxtrace_mmap_params *mp,
void *userpg, int fd)
@@ -762,6 +768,9 @@ int auxtrace_queues__process_index(struct auxtrace_queues 
*queues,
size_t i;
int err;
 
+   if (auxtrace__dont_decode(session))
+   return 0;
+
list_for_each_entry(auxtrace_index, &session->auxtrace_index, list) {
for (i = 0; i < auxtrace_index->nr; i++) {
ent = &auxtrace_index->entries[i];
@@ -892,12 +901,6 @@ int perf_event__synthesize_auxtrace_info(struct 
auxtrace_record *itr,
return err;
 }
 
-static bool auxtrace__dont_decode(struct perf_session *session)
-{
-   return !session->itrace_synth_opts ||
-  session->itrace_synth_opts->dont_decode;
-}
-
 int perf_event__process_auxtrace_info(struct perf_tool *tool __maybe_unused,
  union perf_event *event,
  struct perf_session *session)
-- 
1.9.1

[PATCH 1/7] perf record: Combine some auxtrace initialization into a single function

[Adrian Hunter]

In preparation for adding AUX area sampling support, combine some auxtrace
initialization into a single function.

Signed-off-by: Adrian Hunter 
---
 tools/perf/builtin-record.c | 36 
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 907267206973..9b6338ce9c2b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -273,6 +273,24 @@ static void record__read_auxtrace_snapshot(struct record 
*rec)
}
 }
 
+static int record__auxtrace_init(struct record *rec)
+{
+   int err;
+
+   if (!rec->itr) {
+   rec->itr = auxtrace_record__init(rec->evlist, &err);
+   if (err)
+   return err;
+   }
+
+   err = auxtrace_parse_snapshot_options(rec->itr, &rec->opts,
+ rec->opts.auxtrace_snapshot_opts);
+   if (err)
+   return err;
+
+   return auxtrace_parse_filters(rec->evlist);
+}
+
 #else
 
 static inline
@@ -293,6 +311,11 @@ int auxtrace_record__snapshot_start(struct auxtrace_record 
*itr __maybe_unused)
return 0;
 }
 
+static int record__auxtrace_init(struct record *rec __maybe_unused)
+{
+   return 0;
+}
+
 #endif
 
 static int record__mmap_evlist(struct record *rec,
@@ -1711,17 +1734,6 @@ int cmd_record(int argc, const char **argv)
alarm(rec->switch_output.time);
}
 
-   if (!rec->itr) {
-   rec->itr = auxtrace_record__init(rec->evlist, &err);
-   if (err)
-   goto out;
-   }
-
-   err = auxtrace_parse_snapshot_options(rec->itr, &rec->opts,
- rec->opts.auxtrace_snapshot_opts);
-   if (err)
-   goto out;
-
/*
 * Allow aliases to facilitate the lookup of symbols for address
 * filters. Refer to auxtrace_parse_filters().
@@ -1730,7 +1742,7 @@ int cmd_record(int argc, const char **argv)
 
symbol__init(NULL);
 
-   err = auxtrace_parse_filters(rec->evlist);
+   err = record__auxtrace_init(rec);
if (err)
goto out;
 
-- 
1.9.1

[RFC/RFT][PATCH v2 0/6] sched/cpuidle: Idle loop rework

[Rafael J. Wysocki]

Hi All,

Thanks a lot for the discussion so far!

Here's a new version of the series addressing some comments from the
discussion and (most importantly) replacing patches 4 and 5 with another
(simpler) patch.

The summary below still applies:

On Sunday, March 4, 2018 11:21:30 PM CET Rafael J. Wysocki wrote:
> 
> The problem is that if we stop the sched tick in
> tick_nohz_idle_enter() and then the idle governor predicts short idle
> duration, we lose regardless of whether or not it is right.
> 
> If it is right, we've lost already, because we stopped the tick
> unnecessarily.  If it is not right, we'll lose going forward, because
> the idle state selected by the governor is going to be too shallow and
> we'll draw too much power (that has been reported recently to actually
> happen often enough for people to care).
> 
> This patch series is an attempt to improve the situation and the idea
> here is to make the decision whether or not to stop the tick deeper in
> the idle loop and in particular after running the idle state selection
> in the path where the idle governor is invoked.  This way the problem
> can be avoided, because the idle duration predicted by the idle governor
> can be used to decide whether or not to stop the tick so that the tick
> is only stopped if that value is large enough (and, consequently, the
> idle state selected by the governor is deep enough).
> 
> The series tires to avoid adding too much new code, rather reorder the
> existing code and make it more fine-grained.
> 
> Patch 1 prepares the tick-sched code for the subsequent modifications and it
> doesn't change the code's functionality (at least not intentionally).
> 
> Patch 2 starts pushing the tick stopping decision deeper into the idle
> loop, but it is limited to do_idle() and tick_nohz_irq_exit().
> 
> Patch 3 makes cpuidle_idle_call() decide whether or not to stop the tick
> and sets the stage for the changes in patch 6.

Patch 4 adds a bool pointer argument to cpuidle_select() and the ->select
governor callback allowing them to return a "nohz" hint on whether or not to
stop the tick to the caller.

Patch 5 reorders the idle state selection with respect to the stopping of the
tick and causes the additional "nohz" hint from cpuidle_select() to be used
for deciding whether or not to stop the tick.

Patch 6 cleans up the code to avoid running one piece of it twice in a row
in some cases.

And the two paragraphs below still apply:

> I have tested these patches on a couple of machines, including the very laptop
> I'm sending them from, without any obvious issues, but please give them a go
> if you can, especially if you have an easy way to reproduce the problem they
> are targeting.  The patches are on top of 4.16-rc3 (if you need a git branch
> with them for easier testing, please let me know).
> 
> The above said, this is just RFC, so no pets close to the machines running it,
> please, and I'm kind of expecting Peter and Thomas to tear it into pieces. :-)

Thanks,
Rafael

[RFC/RFT][PATCH v2 5/6] sched: idle: Select idle state before stopping the tick

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

In order to address the issue with short idle duration predictions
by the idle governor after the tick has been stopped, reorder the
code in cpuidle_idle_call() so that the governor idle state selection
runs before tick_nohz_idle_go_idle() and use the "nohz" hint returned
by cpuidle_select() to tell tick_nohz_idle_go_idle() whether or not
to stop the tick.

This isn't straightforward, because menu_predict() invokes
tick_nohz_get_sleep_length() to get the time to the next timer
event and the number returned by the latter comes from
__tick_nohz_idle_enter().  Fortunately, however, it is possible
to compute that number without actually stopping the tick and with
the help of the existing code.

Namely, notice that tick_nohz_stop_sched_tick() already computes the
next timer event time to reprogram the scheduler tick hrtimer and
that time can be used as a proxy for the actual next timer event
time in the idle duration predicition.

Accordingly, rename the original tick_nohz_stop_sched_tick() to
__tick_nohz_next_event() and add the stop_tick argument indicating
whether or not to stop the tick to it.  If that argument is 'true',
the function will work like the original tick_nohz_stop_sched_tick(),
but otherwise it will just compute the next event time without
stopping the tick.  Next, redefine tick_nohz_stop_sched_tick() as
a wrapper around the new function.

Following that, make tick_nohz_get_sleep_length() call
__tick_nohz_next_event() to compute the next timer event time
and make it use the new last_jiffies_update field in struct
tick_sched to tell __tick_nohz_idle_enter() to skip some code
that has run already.

[After this change the __tick_nohz_next_event() code computing the
 next event time will run twice in a row if the expected idle period
 duration coming from cpuidle_select() is large enough which is sort
 of ugly, but the next set of changes deals with that separately.
 To do that, it uses the value of the last_jiffies_update field in
 struct tick_sched introduced here, among other things.]

Finally, drop the now redundant sleep_length field from struct
tick_sched.

Signed-off-by: Rafael J. Wysocki 
---

-> v2: Use the "nohz" hint from cpuidle_select() instead of the expected
   idle duration.

---
 kernel/sched/idle.c  |7 ++---
 kernel/time/tick-sched.c |   64 +--
 kernel/time/tick-sched.h |3 --
 3 files changed, 50 insertions(+), 24 deletions(-)

Index: linux-pm/kernel/sched/idle.c
===
--- linux-pm.orig/kernel/sched/idle.c
+++ linux-pm/kernel/sched/idle.c
@@ -188,13 +188,14 @@ static void cpuidle_idle_call(void)
} else {
bool nohz = true;
 
-   tick_nohz_idle_go_idle(true);
-   rcu_idle_enter();
-
/*
 * Ask the cpuidle framework to choose a convenient idle state.
 */
next_state = cpuidle_select(drv, dev, &nohz);
+
+   tick_nohz_idle_go_idle(nohz);
+   rcu_idle_enter();
+
entered_state = call_cpuidle(drv, dev, next_state);
/*
 * Give the governor an opportunity to reflect on the outcome
Index: linux-pm/kernel/time/tick-sched.c
===
--- linux-pm.orig/kernel/time/tick-sched.c
+++ linux-pm/kernel/time/tick-sched.c
@@ -655,8 +655,8 @@ static inline bool local_timer_softirq_p
return local_softirq_pending() & TIMER_SOFTIRQ;
 }
 
-static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
-ktime_t now, int cpu)
+static ktime_t __tick_nohz_next_event(struct tick_sched *ts, int cpu,
+ bool stop_tick)
 {
struct clock_event_device *dev = 
__this_cpu_read(tick_cpu_device.evtdev);
u64 basemono, next_tick, next_tmr, next_rcu, delta, expires;
@@ -670,6 +670,7 @@ static ktime_t tick_nohz_stop_sched_tick
basejiff = jiffies;
} while (read_seqretry(&jiffies_lock, seq));
ts->last_jiffies = basejiff;
+   ts->last_jiffies_update = basemono;
 
/*
 * Keep the periodic tick, when RCU, architecture or irq_work
@@ -732,8 +733,10 @@ static ktime_t tick_nohz_stop_sched_tick
 */
delta = timekeeping_max_deferment();
if (cpu == tick_do_timer_cpu) {
-   tick_do_timer_cpu = TICK_DO_TIMER_NONE;
-   ts->do_timer_last = 1;
+   if (stop_tick) {
+   tick_do_timer_cpu = TICK_DO_TIMER_NONE;
+   ts->do_timer_last = 1;
+   }
} else if (tick_do_timer_cpu != TICK_DO_TIMER_NONE) {
delta = KTIME_MAX;
ts->do_timer_last = 0;
@@ -756,6 +759,12 @@ static ktime_t tick_nohz_stop_sched_tick
expires = min_t(u64, expires, next_tick);
tick = expire

[PATCH v2 1/2] staging: most: Add a blank line.

[Quytelda Kahja]

Use a blank line after components_show() function declaration.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/most/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 0ab2de5ecf18..67e2d7f29967 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -583,6 +583,7 @@ static ssize_t components_show(struct device_driver *drv, 
char *buf)
}
return offs;
 }
+
 /**
  * split_string - parses buf and extracts ':' separated substrings.
  *
-- 
2.16.2

Re: [PATCH 3/3] i2c: xlp9xx: Add support for SMBAlert

[Phil Reid]

On 6/03/2018 16:36, Jan Glauber wrote:

On Tue, Feb 27, 2018 at 01:26:20PM +, George Cherian wrote:

Add support for SMBus alert mechanism to i2c-xlp9xx driver.
The second interrupt is parsed to use for SMBus alert.
The first interrupt is the i2c controller main interrupt.

Signed-off-by: Kamlakant Patel 
Signed-off-by: George Cherian 
---
  drivers/i2c/busses/i2c-xlp9xx.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index eb8913e..9462eab 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -10,6 +10,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
struct device *dev;
struct i2c_adapter adapter;
struct completion msg_complete;
+   struct i2c_smbus_alert_setup alert_data;
+   struct i2c_client *ara;
int irq;
bool msg_read;
bool len_recv;
@@ -447,6 +450,21 @@ static int xlp9xx_i2c_get_frequency(struct platform_device 
*pdev,
return 0;
  }
  
+static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,

+ struct platform_device *pdev)
+{
+   if (!priv->alert_data.irq)
+   return -EINVAL;
+
+   priv->alert_data.alert_edge_triggered = 0;


Hi George,

I think this is not needed anymore, see:
9b9f2b8bc2ac i2c: i2c-smbus: Use threaded irq for smbalert

--Jan


Yes.

And also all of this is not needed if named interrupts.
- interrupt-names
"irq", "wakeup" and "smbus_alert" names are recognized by I2C core,
other names are left to individual drivers.

presence of named irq smbus_alert should result in alert handler being
created for that bus by the core




+
+   priv->ara = i2c_setup_smbus_alert(&priv->adapter, &priv->alert_data);
+   if (!priv->ara)
+   return -ENODEV;
+
+   return 0;
+}
+
  static int xlp9xx_i2c_probe(struct platform_device *pdev)
  {
struct xlp9xx_i2c_dev *priv;
@@ -467,6 +485,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "invalid irq!\n");
return priv->irq;
}
+   /* SMBAlert irq */
+   priv->alert_data.irq = platform_get_irq(pdev, 1);
+   if (priv->alert_data.irq <= 0)
+   priv->alert_data.irq = 0;
  
  	xlp9xx_i2c_get_frequency(pdev, priv);

xlp9xx_i2c_init(priv);
@@ -493,6 +515,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
if (err)
return err;
  
+	err = xlp9xx_i2c_smbus_setup(priv, pdev);

+   if (err)
+   dev_info(&pdev->dev, "No active SMBus alert %d\n", err);
+
platform_set_drvdata(pdev, priv);
dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
  
--

2.1.4






--
Regards
Phil Reid

Re: [PATCH v2 08/11] kconfig: unittest: test defconfig when two choices interact

[Masahiro Yamada]

2018-03-02 19:41 GMT+09:00 Ulf Magnusson :
> On Fri, Mar 2, 2018 at 5:31 AM, Masahiro Yamada
>  wrote:
>> Commit fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu
>> selects options that another choice menu depends on") fixed defconfig
>> when two choices interact (i.e. calculating the visibility of a choice
>> requires to calculate another choice).
>>
>> The test code in that commit log was based on the real world example,
>> and complicated.  So, I shrunk it down to the following:
>>
>> defconfig.choice:
>> ---8<---
>> CONFIG_CHOICE_VAL0=y
>> ---8<---
>>
>> ---8<---
>> config MODULES
>> bool "Enable loadable module support"
>> option modules
>> default y
>>
>> choice
>> prompt "Choice"
>>
>> config CHOICE_VAL0
>> tristate "Choice 0"
>>
>> config CHOICE_VAL1
>> tristate "Choice 1"
>>
>> endchoice
>>
>> choice
>> prompt "Another choice"
>> depends on CHOICE_VAL0
>>
>> config DUMMY
>> bool "dummy"
>>
>> endchoice
>> ---8<---
>>
>> Prior to commit fbe98bb9ed3d,
>>
>>   $ scripts/kconfig/conf --defconfig=defconfig.choice Kconfig.choice
>>
>> resulted in:
>>
>>   CONFIG_MODULES=y
>>   CONFIG_CHOICE_VAL0=m
>>   # CONFIG_CHOICE_VAL1 is not set
>>   CONFIG_DUMMY=y
>>
>> where the expected result would be:
>>
>>   CONFIG_MODULES=y
>>   CONFIG_CHOICE_VAL0=y
>>   # CONFIG_CHOICE_VAL1 is not set
>>   CONFIG_DUMMY=y
>>
>> Roughly, this weird behavior happened like this:
>>
>> Symbols are calculated a couple of times.  First, all symbols are
>> calculated in conf_read().  The first 'choice' is evaluated to 'y'
>> due to the SYMBOL_DEF_USER flag, but sym_calc_choice() clears it
>> unless all of its choice values are explicitly set by the user.
>>
>> conf_set_all_new_symbols() clears all SYMBOL_VALID flags.  Then, only
>> choices are calculated.  At this point, the SYMBOL_DEF_USER for the
>> first choice is unset, so, it is evaluated to 'm'.  (this is weird)
>
> This is because tristate choices start out in m mode btw (they have an
> implicit select of 'm && ' on them, added add the end of
> menu_finalize()).

Ah, right.  But indeed weird to forget SYMBOL_DEF_USER.


>> set_all_choice_values() sets SYMBOL_DEF_USER again to choice symbols.
>>
>> When calculating the second choice, due to 'depends on CHOICE_VAL0',
>> it triggers the calculation of CHOICE_VAL0.  As a result, SYMBOL_VALID
>> is set for CHOICE_VAL0.
>>
>> Symbols except choices get the final chance of re-calculation in
>> conf_write().  In a normal case, CHOICE_VAL0 would be re-caluculated,
>> then the first choice would be indirectly re-calculated with the
>> SYMBOL_DEF_USER which has been set by set_all_choice_values(), which
>> would be evaluated to 'y'.  But, in this case, CHOICE_VAL0 has been
>> marked SYMBOL_VALID, so it is simply skipped.  Then, =m is written out
>> to the .config file.
>>
>> Add a unit test for this naive case.
>
> At a high level, I think the problem is that the choice mode is
> forgotten. It should be y because of the CONFIG_CHOICE_VAL0=y
> assignment, but reverts back to m temporarily, and during that window
> a choice symbol is evaluated and gets the wrong value.
>
> I wonder if all this twisty code and the weird flags
> (SYMBOL_NEED_SET_CHOICE_VALUES... hmm) are required. Perhaps an extra
> invalidation or the like would be enough.


Agree.

Probably, 5d09598d488f and fbe98bb9ed3d fixed issues in a bad way.

I believe SYMBOL_DEF_USER should be set only in
conf_read(_simple) and conf_set_all_new_symbols().

It is strange to set and clear SYMBOL_DEF_USER
while calculating symbols.




>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>> Changes in v2:
>>   - Newly added
>>
>>  scripts/kconfig/tests/inter_choice/Kconfig | 24 
>> ++
>>  scripts/kconfig/tests/inter_choice/__init__.py | 14 +
>>  scripts/kconfig/tests/inter_choice/defconfig   |  1 +
>>  scripts/kconfig/tests/inter_choice/expected_config |  4 
>>  4 files changed, 43 insertions(+)
>>  create mode 100644 scripts/kconfig/tests/inter_choice/Kconfig
>>  create mode 100644 scripts/kconfig/tests/inter_choice/__init__.py
>>  create mode 100644 scripts/kconfig/tests/inter_choice/defconfig
>>  create mode 100644 scripts/kconfig/tests/inter_choice/expected_config
>>
>> diff --git a/scripts/kconfig/tests/inter_choice/Kconfig 
>> b/scripts/kconfig/tests/inter_choice/Kconfig
>> new file mode 100644
>> index 000..57d55c4
>> --- /dev/null
>> +++ b/scripts/kconfig/tests/inter_choice/Kconfig
>> @@ -0,0 +1,24 @@
>> +config MODULES
>> +   bool "Enable loadable module support"
>> +   option modules
>> +   default y
>> +
>> +choice
>> +   prompt "Choice"
>> +
>> +config CHOICE_VAL0
>> +   tristate "Choice 0"
>> +
>> +config CHOIVE_VAL1
>> +   tristate "Choice 1"
>> +
>> +endchoice
>> +
>> +choice
>> +   prompt "Another choice"
>> +   depends on CHOICE_VAL0
>> +
>> +config DUMMY
>> +   bool "dummy"
>> +
>> +endchoice
>> diff

Re: [REPOST PATCH] arm/arm64: KVM: Add PSCI version selection API

[Andrew Jones]

On Mon, Mar 05, 2018 at 04:47:55PM +, Peter Maydell wrote:
> On 2 March 2018 at 11:11, Marc Zyngier  wrote:
> > On Fri, 02 Mar 2018 10:44:48 +,
> > Auger Eric wrote:
> >> I understand the get/set is called as part of the migration process.
> >> So my understanding is the benefit of this series is migration fails in
> >> those cases:
> >>
> >> >=0.2 source -> 0.1 destination
> >> 0.1 source -> >=0.2 destination
> >
> > It also fails in the case where you migrate a 1.0 guest to something
> > that cannot support it.
> 
> I think it would be useful if we could write out the various
> combinations of source, destination and what we expect/want to
> have happen. My gut feeling here is that we're sacrificing
> exact migration compatibility in favour of having the guest
> automatically get the variant-2 mitigations, but it's not clear
> to me exactly which migration combinations that's intended to
> happen for. Marc?
> 
> If this wasn't a mitigation issue the desired behaviour would be
> straightforward:
>  * kernel should default to 0.2 on the basis that
>that's what it did before
>  * new QEMU version should enable 1.0 by default for virt-2.12
>and 0.2 for virt-2.11 and earlier
>  * PSCI version info shouldn't appear in migration stream unless
>it's something other than 0.2
> But that would leave some setups (which?) unnecessarily without the
> mitigation, so we're not doing that. The question is, exactly
> what *are* we aiming for?

The reason Marc dropped this patch from the series it was first introduced
in was because we didn't have the aim 100% understood. We want the
mitigation by default, but also to have the least chance of migration
failure, and when we must fail (because we're not doing the
straightforward approach listed above, which would prevent failures), then
we want to fail with the least amount of damage to the user.

I experimented with a couple different approaches and provided tables[1]
with my results. I even recommended an approach, but I may have changed
my mind after reading Marc's follow-up[2]. The thread continues from
there as well with follow-ups from Christoffer, Marc, and myself. Anyway,
Marc did this repost for us to debate it and work out the best approach
here.

Thanks,
drew

[1] https://www.spinics.net/lists/arm-kernel/msg632355.html
[2] https://www.spinics.net/lists/arm-kernel/msg632385.html

Re: [PATCH 2/4] staging: most: Replace calls to BUG_ON() with WARN_ONCE() and return.

[Quytelda Kahja]

> Are you sure this will work?
Well, my goal was just to replace the code that could crash the kernel
and let somebody with a better understanding of this particular driver
write the recovery code, if necessary.  It seemed from context that
the BUG_ON() calls were being used like assert() though, so I assumed
there wasn't really much recovery to be made from that problem.  If
you feel this doesn't improve the behavior of the driver, just drop
the patch.

Thank you,
Quytelda Kahja

On Thu, Mar 1, 2018 at 8:21 AM, Greg KH  wrote:
> On Fri, Feb 23, 2018 at 11:58:33PM -0800, Quytelda Kahja wrote:
>> Replace calls to BUG_ON() used to check for NULL pointers with WARN_ONCE()
>> followed by a return.
>
> Are you sure this will work?
>
>>
>> Signed-off-by: Quytelda Kahja 
>> ---
>>  drivers/staging/most/core.c | 13 ++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
>> index 18157dd80324..3f65390a6e17 100644
>> --- a/drivers/staging/most/core.c
>> +++ b/drivers/staging/most/core.c
>> @@ -916,7 +916,11 @@ static void arm_mbo(struct mbo *mbo)
>>   unsigned long flags;
>>   struct most_channel *c;
>>
>> - BUG_ON((!mbo) || (!mbo->context));
>> + if (WARN_ONCE(!mbo || !mbo->context,
>> +   "Bad mbo or missing channel reference.\n")) {
>> + return;
>
> How is the code supposed to recover from this major problem?
>
>> + }
>> +
>>   c = mbo->context;
>>
>>   if (c->is_poisoned) {
>> @@ -1001,7 +1005,7 @@ static int arm_mbo_chain(struct most_channel *c, int 
>> dir,
>>  void most_submit_mbo(struct mbo *mbo)
>>  {
>>   if (WARN_ONCE(!mbo || !mbo->context,
>> -   "bad mbo or missing channel reference\n"))
>> +   "Bad mbo or missing channel reference.\n"))
>
> You did something different here that you did not describe in your
> changelog :(
>
> thanks,
>
> greg k-h

Re: [PATCH] vsprintf: Make "null" pointer dereference more robust

[Petr Mladek]

On Mon 2018-03-05 16:16:37, Rasmus Villemoes wrote:
> On 2 March 2018 at 13:53, Petr Mladek  wrote:
> > %p has many modifiers where the pointer is dereferenced. An invalid
> > pointer might cause kernel to crash silently.
> >
> > Note that printk() formats the string under logbuf_lock. Any recursive
> > printks are redirected to the printk_safe implementation and the messages
> > are stored into per-CPU buffers. These buffers might be eventually flushed
> > in printk_safe_flush_on_panic() but it is not guaranteed.
> 
> Yeah, it's annoying that we can't reliably WARN for bogus vsprintf() uses.
> 
> > In general, we should do our best to get useful message from printk().
> > All pointers to the first memory page must be invalid. Let's prevent
> > the dereference and print "(null)" in this case. This is already done
> > in many other situations, including "%s" format handling and many
> > page fault handlers.
> >
> > Signed-off-by: Petr Mladek 
> > ---
> >  lib/vsprintf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index d7a708f82559..5c2d1f44218a 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1849,7 +1849,7 @@ char *pointer(const char *fmt, char *buf, char *end, 
> > void *ptr,
> >  {
> > const int default_width = 2 * sizeof(void *);
> >
> > -   if (!ptr && *fmt != 'K' && *fmt != 'x') {
> > +   if ((unsigned long)ptr < PAGE_SIZE && *fmt != 'K' && *fmt != 'x') {
> 
> ISTM that accidentally passing an ERR_PTR would be just as likely as
> passing a NULL pointer (or some small offset from one), so if we do
> this, shouldn't the test also cover IS_ERR values?

It would make perfect sense to catch IS_ERR_PTR(). Derefenrecing
such pointer cause crash. But it might be pretty confusing to print
"(null)" in this case.

I would handle this in separate patch and print "(err)" or so.
Any volunteer to prepare the patch?

Best Regards,
Petr

Re: [PATCH v3 10/25] ASoC: qcom: q6asm: Add support to memory map and unmap

[Srinivas Kandagatla]

Thanks for the review,

On 01/03/18 21:28, Mark Brown wrote:

On Tue, Feb 13, 2018 at 04:58:22PM +, srinivas.kandaga...@linaro.org wrote:


+   num_regions = is_contiguous ? 1 : periods;
+   buf_sz = is_contiguous ? (period_sz * periods) : period_sz;


Please write normal if statements, it's much easier to read.



Sure, will fix it in next version.

+   buf_sz = PAGE_ALIGN(buf_sz);


I don't understand what this is doing, buf_sz is a length not an address
so why are we attempting to align it?


Yes, this is a requirement form the DSP side that the size is multiple 
of 4KB. I will fix this properly in next version.


thanks,
srini

Re: [PATCH 9/9] drm/xen-front: Implement communication with backend

[Daniel Vetter]

On Mon, Mar 05, 2018 at 11:30:35AM +0200, Oleksandr Andrushchenko wrote:
> On 03/05/2018 11:25 AM, Daniel Vetter wrote:
> > On Wed, Feb 21, 2018 at 10:03:42AM +0200, Oleksandr Andrushchenko wrote:
> > > From: Oleksandr Andrushchenko 
> > > 
> > > Handle communication with the backend:
> > >   - send requests and wait for the responses according
> > > to the displif protocol
> > >   - serialize access to the communication channel
> > >   - time-out used for backend communication is set to 3000 ms
> > >   - manage display buffers shared with the backend
> > > 
> > > Signed-off-by: Oleksandr Andrushchenko 
> > After the demidlayering it probably makes sense to merge this with the
> > overall kms/basic-drm-driver patch. Up to you really.
> The reason for such partitioning here and before was that
> I can have Xen/DRM parts separate, so those are easier for
> review by Xen/DRM communities. So, I would prefer to have it
> as it is

Well for reviewing the kms parts I need to check what the xen parts are
doing (at least sometimes), since semantics of what you're doing matter,
and there's a few cases which new drivers tend to get wrong. So for me,
this splitting makes stuff actually harder to review.

And I guess for the xen folks it won't hurt if they see a bit clearer how
it's used on the drm side (even if they might not really understand what's
going on). If we have some superficial abstraction in between each of the
subsystem maintainers might make assumptions about what the other side of
the code is doing which turn out to be wrong, and that's not good.

Just explaining my motivation for why I don't like abstractions and
splitting stuff up into patches that don't make much sense on their own
(because the code is just hanging out there without being wired up
anywhere).
-Daniel
> > -Daniel
> > > ---
> > >   drivers/gpu/drm/xen/xen_drm_front.c | 327 
> > > +++-
> > >   drivers/gpu/drm/xen/xen_drm_front.h |   5 +
> > >   2 files changed, 327 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xen/xen_drm_front.c 
> > > b/drivers/gpu/drm/xen/xen_drm_front.c
> > > index 8de88e359d5e..5ad546231d30 100644
> > > --- a/drivers/gpu/drm/xen/xen_drm_front.c
> > > +++ b/drivers/gpu/drm/xen/xen_drm_front.c
> > > @@ -31,12 +31,146 @@
> > >   #include "xen_drm_front_evtchnl.h"
> > >   #include "xen_drm_front_shbuf.h"
> > > +/* timeout in ms to wait for backend to respond */
> > > +#define VDRM_WAIT_BACK_MS3000
> > > +
> > > +struct xen_drm_front_dbuf {
> > > + struct list_head list;
> > > + uint64_t dbuf_cookie;
> > > + uint64_t fb_cookie;
> > > + struct xen_drm_front_shbuf *shbuf;
> > > +};
> > > +
> > > +static int dbuf_add_to_list(struct xen_drm_front_info *front_info,
> > > + struct xen_drm_front_shbuf *shbuf, uint64_t dbuf_cookie)
> > > +{
> > > + struct xen_drm_front_dbuf *dbuf;
> > > +
> > > + dbuf = kzalloc(sizeof(*dbuf), GFP_KERNEL);
> > > + if (!dbuf)
> > > + return -ENOMEM;
> > > +
> > > + dbuf->dbuf_cookie = dbuf_cookie;
> > > + dbuf->shbuf = shbuf;
> > > + list_add(&dbuf->list, &front_info->dbuf_list);
> > > + return 0;
> > > +}
> > > +
> > > +static struct xen_drm_front_dbuf *dbuf_get(struct list_head *dbuf_list,
> > > + uint64_t dbuf_cookie)
> > > +{
> > > + struct xen_drm_front_dbuf *buf, *q;
> > > +
> > > + list_for_each_entry_safe(buf, q, dbuf_list, list)
> > > + if (buf->dbuf_cookie == dbuf_cookie)
> > > + return buf;
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +static void dbuf_flush_fb(struct list_head *dbuf_list, uint64_t 
> > > fb_cookie)
> > > +{
> > > + struct xen_drm_front_dbuf *buf, *q;
> > > +
> > > + list_for_each_entry_safe(buf, q, dbuf_list, list)
> > > + if (buf->fb_cookie == fb_cookie)
> > > + xen_drm_front_shbuf_flush(buf->shbuf);
> > > +}
> > > +
> > > +static void dbuf_free(struct list_head *dbuf_list, uint64_t dbuf_cookie)
> > > +{
> > > + struct xen_drm_front_dbuf *buf, *q;
> > > +
> > > + list_for_each_entry_safe(buf, q, dbuf_list, list)
> > > + if (buf->dbuf_cookie == dbuf_cookie) {
> > > + list_del(&buf->list);
> > > + xen_drm_front_shbuf_unmap(buf->shbuf);
> > > + xen_drm_front_shbuf_free(buf->shbuf);
> > > + kfree(buf);
> > > + break;
> > > + }
> > > +}
> > > +
> > > +static void dbuf_free_all(struct list_head *dbuf_list)
> > > +{
> > > + struct xen_drm_front_dbuf *buf, *q;
> > > +
> > > + list_for_each_entry_safe(buf, q, dbuf_list, list) {
> > > + list_del(&buf->list);
> > > + xen_drm_front_shbuf_unmap(buf->shbuf);
> > > + xen_drm_front_shbuf_free(buf->shbuf);
> > > + kfree(buf);
> > > + }
> > > +}
> > > +
> > > +static struct xendispl_req *be_prepare_req(
> > > + struct xen_drm_front_evtchnl *evtchnl, uint8_t operation)
> > > +{
> > > + struct xendispl_req *req;
> > > +
> > > + req = RING_GET_REQUEST(&evtchn

Re: [PATCH v3 07/25] ASoC: qcom: qdsp6: Add support to Q6ADM

[Srinivas Kandagatla]

Thanks for the review,

On 01/03/18 21:24, Mark Brown wrote:

On Tue, Feb 13, 2018 at 04:58:19PM +, srinivas.kandaga...@linaro.org wrote:


+static struct copp *adm_find_copp(struct q6adm *adm, int port_idx,
+ int copp_idx)
+{
+   struct copp *c;
+
+   spin_lock(&adm->copps_list_lock);
+   list_for_each_entry(c, &adm->copps_list, node) {
+   if ((port_idx == c->afe_port) && (copp_idx == c->copp_idx)) {
+   spin_unlock(&adm->copps_list_lock);
+   return c;
+   }
+   }
+
+   spin_unlock(&adm->copps_list_lock);


We've again got this use of spinlocks here but no IRQ safety - what
exactly is going on with the locking?  In general all of the locking in
this stuff is raising very serious alarm bells with me, I don't
understand what is being protected against what and there's some very
obvious bugs.  We could probably use some documentation about what the
locking is supposed to be doing.

I agree, there are locking issues here, Am revisiting them all before I 
send a next version.



+   case ADM_CMDRSP_DEVICE_OPEN_V5: {



+   copp->id = open->copp_id;
+   wake_up(&copp->wait);
+   }
+   break;
+   default:


This indentation is confusing.


I agree, will fix such instances in next version.

thanks,
srini

Re: [PATCH v3 11/25] ASoC: qcom: q6asm: add support to audio stream apis

[Srinivas Kandagatla]

Thanks for the review comments.

sorry for delay, for some reason these emails endup in my SPAM folder.

On 01/03/18 21:33, Mark Brown wrote:

On Tue, Feb 13, 2018 at 04:58:23PM +, srinivas.kandaga...@linaro.org wrote:


uint32_t used;
@@ -131,7 +191,7 @@ static int q6asm_apr_send_session_pkt(struct q6asm *a, 
struct audio_client *ac,
  
  	rc = wait_event_timeout(a->mem_wait, (a->mem_state <= 0), 5 * HZ);

if (!rc) {
-   dev_err(a->dev, "CMD timeout \n");
+   dev_err(a->dev, "CMD timeout\n");
rc = -ETIMEDOUT;
} else if (a->mem_state < 0) {
rc =  q6dsp_errno(a->mem_state);


This should be folded into whatever patch is being fixed.



My Bad, I will fix this in next version.


+   open.hdr.opcode = ASM_STREAM_CMD_OPEN_WRITE_V3;
+   open.mode_flags = 0x00;
+   open.mode_flags |= ASM_LEGACY_STREAM_SESSION;


What is a legacy stream and why are we using it in new code?


This is basically Ensures backward compatibility to the original 
behavior of ASM_STREAM_CMD_OPEN_WRITE_V2 command.


I will take a closer look and see if its possible to remove this in the 
first place.


thanks,
srini

[PATCH 5/5 V2] tmp: factor out tpm_get_timeouts

[Tomas Winkler]

Factor out tpm_get_timeouts into tpm2_get_timeouts
and tpm1_get_timeouts.

Signed-off-by: Tomas Winkler 
---
V2: Rebase.

 drivers/char/tpm/tpm-interface.c | 127 ++-
 drivers/char/tpm/tpm.h   |   5 +-
 drivers/char/tpm/tpm1-cmd.c  | 107 +
 drivers/char/tpm/tpm2-cmd.c  |  22 +++
 4 files changed, 137 insertions(+), 124 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 40d1770f6b38..7f6968b750c8 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -402,132 +402,13 @@ EXPORT_SYMBOL_GPL(tpm_getcap);
 
 int tpm_get_timeouts(struct tpm_chip *chip)
 {
-   cap_t cap;
-   unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
-   ssize_t rc;
-
if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS)
return 0;
 
-   if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-   /* Fixed timeouts for TPM2 */
-   chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
-   chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
-   chip->timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C);
-   chip->timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D);
-   chip->duration[TPM_SHORT] =
-   msecs_to_jiffies(TPM2_DURATION_SHORT);
-   chip->duration[TPM_MEDIUM] =
-   msecs_to_jiffies(TPM2_DURATION_MEDIUM);
-   chip->duration[TPM_LONG] =
-   msecs_to_jiffies(TPM2_DURATION_LONG);
-   chip->duration[TPM_LONG_LONG] =
-   msecs_to_jiffies(TPM2_DURATION_LONG_LONG);
-
-   chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
-   return 0;
-   }
-
-   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
-   sizeof(cap.timeout));
-   if (rc == TPM_ERR_INVALID_POSTINIT) {
-   if (tpm_startup(chip))
-   return rc;
-
-   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
-   "attempting to determine the timeouts",
-   sizeof(cap.timeout));
-   }
-
-   if (rc) {
-   dev_err(&chip->dev,
-   "A TPM error (%zd) occurred attempting to determine the 
timeouts\n",
-   rc);
-   return rc;
-   }
-
-   timeout_old[0] = jiffies_to_usecs(chip->timeout_a);
-   timeout_old[1] = jiffies_to_usecs(chip->timeout_b);
-   timeout_old[2] = jiffies_to_usecs(chip->timeout_c);
-   timeout_old[3] = jiffies_to_usecs(chip->timeout_d);
-   timeout_chip[0] = be32_to_cpu(cap.timeout.a);
-   timeout_chip[1] = be32_to_cpu(cap.timeout.b);
-   timeout_chip[2] = be32_to_cpu(cap.timeout.c);
-   timeout_chip[3] = be32_to_cpu(cap.timeout.d);
-   memcpy(timeout_eff, timeout_chip, sizeof(timeout_eff));
-
-   /*
-* Provide ability for vendor overrides of timeout values in case
-* of misreporting.
-*/
-   if (chip->ops->update_timeouts != NULL)
-   chip->timeout_adjusted =
-   chip->ops->update_timeouts(chip, timeout_eff);
-
-   if (!chip->timeout_adjusted) {
-   /* Restore default if chip reported 0 */
-   int i;
-
-   for (i = 0; i < ARRAY_SIZE(timeout_eff); i++) {
-   if (timeout_eff[i])
-   continue;
-
-   timeout_eff[i] = timeout_old[i];
-   chip->timeout_adjusted = true;
-   }
-
-   if (timeout_eff[0] != 0 && timeout_eff[0] < 1000) {
-   /* timeouts in msec rather usec */
-   for (i = 0; i != ARRAY_SIZE(timeout_eff); i++)
-   timeout_eff[i] *= 1000;
-   chip->timeout_adjusted = true;
-   }
-   }
-
-   /* Report adjusted timeouts */
-   if (chip->timeout_adjusted) {
-   dev_info(&chip->dev,
-HW_ERR "Adjusting reported timeouts: A %lu->%luus B 
%lu->%luus C %lu->%luus D %lu->%luus\n",
-timeout_chip[0], timeout_eff[0],
-timeout_chip[1], timeout_eff[1],
-timeout_chip[2], timeout_eff[2],
-timeout_chip[3], timeout_eff[3]);
-   }
-
-   chip->timeout_a = usecs_to_jiffies(timeout_eff[0]);
-   chip->timeout_b = usecs_to_jiffies(timeout_eff[1]);
-   chip->timeout_c = usecs_to_jiffies(timeout_eff[2]);
-   chip->timeout_d = usecs_to_jiffies(timeout_eff[3]);
-
-   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_DURATION, &cap,
-   "attempting to determine the durations",
-   sizeof(cap.duration));
-   if (rc)
-   return rc;
-
-   chip->duration[TPM_SHORT] =
-   

[PATCH 4/5 V2] tpm2: add new tpm2 commands according to TCG 1.36

[Tomas Winkler]

1. TPM2_CC_LAST has moved from 182 to 193
2. Convert tpm2_ordinal_duration from an array into a switch statement,
   as there are not so many commands that require special duration
   relative to a number of commands.

Signed-off-by: Tomas Winkler 
---
V2: Rebase.

 drivers/char/tpm/tpm.h  |  41 +
 drivers/char/tpm/tpm2-cmd.c | 196 +++-
 2 files changed, 93 insertions(+), 144 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 30610d97d30c..826f4eef310c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -129,22 +129,31 @@ enum tpm2_algorithms {
 };
 
 enum tpm2_command_codes {
-   TPM2_CC_FIRST   = 0x011F,
-   TPM2_CC_CREATE_PRIMARY  = 0x0131,
-   TPM2_CC_SELF_TEST   = 0x0143,
-   TPM2_CC_STARTUP = 0x0144,
-   TPM2_CC_SHUTDOWN= 0x0145,
-   TPM2_CC_CREATE  = 0x0153,
-   TPM2_CC_LOAD= 0x0157,
-   TPM2_CC_UNSEAL  = 0x015E,
-   TPM2_CC_CONTEXT_LOAD= 0x0161,
-   TPM2_CC_CONTEXT_SAVE= 0x0162,
-   TPM2_CC_FLUSH_CONTEXT   = 0x0165,
-   TPM2_CC_GET_CAPABILITY  = 0x017A,
-   TPM2_CC_GET_RANDOM  = 0x017B,
-   TPM2_CC_PCR_READ= 0x017E,
-   TPM2_CC_PCR_EXTEND  = 0x0182,
-   TPM2_CC_LAST= 0x018F,
+   TPM2_CC_FIRST   = 0x011F,
+   TPM2_CC_HIERARCHY_CONTROL   = 0x0121,
+   TPM2_CC_HIERARCHY_CHANGE_AUTH   = 0x0129,
+   TPM2_CC_CREATE_PRIMARY  = 0x0131,
+   TPM2_CC_SEQUENCE_COMPLETE   = 0x013E,
+   TPM2_CC_SELF_TEST   = 0x0143,
+   TPM2_CC_STARTUP = 0x0144,
+   TPM2_CC_SHUTDOWN= 0x0145,
+   TPM2_CC_NV_READ = 0x014E,
+   TPM2_CC_CREATE  = 0x0153,
+   TPM2_CC_LOAD= 0x0157,
+   TPM2_CC_SEQUENCE_UPDATE = 0x015C,
+   TPM2_CC_UNSEAL  = 0x015E,
+   TPM2_CC_CONTEXT_LOAD= 0x0161,
+   TPM2_CC_CONTEXT_SAVE= 0x0162,
+   TPM2_CC_FLUSH_CONTEXT   = 0x0165,
+   TPM2_CC_VERIFY_SIGNATURE= 0x0177,
+   TPM2_CC_GET_CAPABILITY  = 0x017A,
+   TPM2_CC_GET_RANDOM  = 0x017B,
+   TPM2_CC_PCR_READ= 0x017E,
+   TPM2_CC_PCR_EXTEND  = 0x0182,
+   TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
+   TPM2_CC_HASH_SEQUENCE_START = 0x0186,
+   TPM2_CC_CREATE_LOADED   = 0x0191,
+   TPM2_CC_LAST= 0x0193, /* Spec 1.36 */
 };
 
 enum tpm2_permanent_handles {
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index c1ddbbba406e..aedebd9ca982 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -86,128 +86,73 @@ static struct tpm2_hash tpm2_hash_map[] = {
 };
 
 /*
- * Array with one entry per ordinal defining the maximum amount
+ * tpm2_ordinal_duration returns the maximum amount
  * of time the chip could take to return the result. The values
- * of the SHORT, MEDIUM, and LONG durations are taken from the
- * PC Client Profile (PTP) specification.
+ * of the MEDIUM, and LONG durations are taken from the
+ * PC Client Profile (PTP) specification (750, 2000 msec)
+ *
  * LONG_LONG is for commands that generates keys which empirically
  * takes longer time on some systems.
  */
-static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
-   TPM_UNDEFINED,  /* 11F */
-   TPM_UNDEFINED,  /* 120 */
-   TPM_LONG,   /* 121 */
-   TPM_UNDEFINED,  /* 122 */
-   TPM_UNDEFINED,  /* 123 */
-   TPM_UNDEFINED,  /* 124 */
-   TPM_UNDEFINED,  /* 125 */
-   TPM_UNDEFINED,  /* 126 */
-   TPM_UNDEFINED,  /* 127 */
-   TPM_UNDEFINED,  /* 128 */
-   TPM_LONG,   /* 129 */
-   TPM_UNDEFINED,  /* 12a */
-   TPM_UNDEFINED,  /* 12b */
-   TPM_UNDEFINED,  /* 12c */
-   TPM_UNDEFINED,  /* 12d */
-   TPM_UNDEFINED,  /* 12e */
-   TPM_UNDEFINED,  /* 12f */
-   TPM_UNDEFINED,  /* 130 */
-   TPM_LONG_LONG,  /* 131 */
-   TPM_UNDEFINED,  /* 132 */
-   TPM_UNDEFINED,  /* 133 */
-   TPM_UNDEFINED,  /* 134 */
-   TPM_UNDEFINED,  /* 135 */
-   TPM_UNDEFINED,  /* 136 */
-   TPM_UNDEFINED,  /* 137 */
-   TPM_UNDEFINED,  /* 138 */
-   TPM_UNDEFINED,  /* 139 */
-   TPM_UNDEFINED,  /* 13a */
-   TPM_UNDEFINED,  /* 13b */
-   TPM_UNDEFINED,  /* 13c */
-   TPM_UNDEFINED,  /* 13d */
-   TPM_MEDIUM, /* 13e */
-   TPM_UNDEFINED,  /* 13f */
-   TPM_UNDEFINED,  /* 140 */
-   TPM_UNDEFINED,  /* 141 */
-   TPM_UNDEFIN

[PATCH 2/5 V2] tpm: factor out tpm 1.2 duration calculation to tpm1-cmd.c

[Tomas Winkler]

Factor out tpm1.2 commands calculation into tpm1-cmd.c file.
and change the prefix from tpm_ to tpm1_.
No functional change is done here.

Signed-off-by: Tomas Winkler 
---
V2: Rebase.

 drivers/char/tpm/Makefile|   1 +
 drivers/char/tpm/st33zp24/st33zp24.c |   2 +-
 drivers/char/tpm/tpm-interface.c | 284 +---
 drivers/char/tpm/tpm.h   |   2 +-
 drivers/char/tpm/tpm1-cmd.c  | 309 +++
 drivers/char/tpm/tpm_i2c_nuvoton.c   |  10 +-
 drivers/char/tpm/tpm_tis_core.c  |   2 +-
 drivers/char/tpm/xen-tpmfront.c  |   2 +-
 8 files changed, 321 insertions(+), 291 deletions(-)
 create mode 100644 drivers/char/tpm/tpm1-cmd.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index b2d6ca9e62e4..87f77dfa7fc9 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -9,6 +9,7 @@ tpm-y += tpm-dev-common.o
 tpm-y += tpm-interface.o
 tpm-y += tpmrm-dev.o
 tpm-y += tpm-sysfs.o
+tpm-y += tpm1-cmd.o
 tpm-y += tpm1_eventlog.o
 tpm-y += tpm2-cmd.o
 tpm-y += tpm2_eventlog.o
diff --git a/drivers/char/tpm/st33zp24/st33zp24.c 
b/drivers/char/tpm/st33zp24/st33zp24.c
index f95b9c75175b..ad03c99899fa 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -432,7 +432,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned 
char *buf,
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
 
ret = wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
-   tpm_calc_ordinal_duration(chip, ordinal),
+   tpm1_calc_ordinal_duration(chip, ordinal),
&tpm_dev->read_queue, false);
if (ret < 0)
goto out_err;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index ddf7d937c77c..402e54252b22 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -33,7 +33,6 @@
 
 #include "tpm.h"
 
-#define TPM_MAX_ORDINAL 243
 #define TSC_MAX_ORDINAL 12
 #define TPM_PROTECTED_COMMAND 0x00
 #define TPM_CONNECTION_COMMAND 0x40
@@ -48,285 +47,6 @@ module_param_named(suspend_pcr, tpm_suspend_pcr, uint, 
0644);
 MODULE_PARM_DESC(suspend_pcr,
 "PCR to use for dummy writes to facilitate flush on suspend.");
 
-/*
- * Array with one entry per ordinal defining the maximum amount
- * of time the chip could take to return the result.  The ordinal
- * designation of short, medium or long is defined in a table in
- * TCG Specification TPM Main Part 2 TPM Structures Section 17. The
- * values of the SHORT, MEDIUM, and LONG durations are retrieved
- * from the chip during initialization with a call to tpm_get_timeouts.
- */
-static const u8 tpm_ordinal_duration[TPM_MAX_ORDINAL] = {
-   TPM_UNDEFINED,  /* 0 */
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,  /* 5 */
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_SHORT,  /* 10 */
-   TPM_SHORT,
-   TPM_MEDIUM,
-   TPM_LONG,
-   TPM_LONG,
-   TPM_MEDIUM, /* 15 */
-   TPM_SHORT,
-   TPM_SHORT,
-   TPM_MEDIUM,
-   TPM_LONG,
-   TPM_SHORT,  /* 20 */
-   TPM_SHORT,
-   TPM_MEDIUM,
-   TPM_MEDIUM,
-   TPM_MEDIUM,
-   TPM_SHORT,  /* 25 */
-   TPM_SHORT,
-   TPM_MEDIUM,
-   TPM_SHORT,
-   TPM_SHORT,
-   TPM_MEDIUM, /* 30 */
-   TPM_LONG,
-   TPM_MEDIUM,
-   TPM_SHORT,
-   TPM_SHORT,
-   TPM_SHORT,  /* 35 */
-   TPM_MEDIUM,
-   TPM_MEDIUM,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_MEDIUM, /* 40 */
-   TPM_LONG,
-   TPM_MEDIUM,
-   TPM_SHORT,
-   TPM_SHORT,
-   TPM_SHORT,  /* 45 */
-   TPM_SHORT,
-   TPM_SHORT,
-   TPM_SHORT,
-   TPM_LONG,
-   TPM_MEDIUM, /* 50 */
-   TPM_MEDIUM,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,  /* 55 */
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_MEDIUM, /* 60 */
-   TPM_MEDIUM,
-   TPM_MEDIUM,
-   TPM_SHORT,
-   TPM_SHORT,
-   TPM_MEDIUM, /* 65 */
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_SHORT,  /* 70 */
-   TPM_SHORT,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,  /* 75 */
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_UNDEFINED,
-   TPM_LONG,   /* 80 */
-   TPM_UNDEFINED,
-   TPM_MEDIUM,
-   TPM_LONG,
-   TPM_SHORT,
-   TPM_UNDEFI

[PATCH 0/5 V2] tpm: timeouts revamp

[Tomas Winkler]

This series cleans up tpm timeouts setting and handling.

First motivation was to fix failures coming from too short timeouts
for commands that creates keys.
Key generation may take significant time depending on the underlying
hardware. Rather than increasing default timeout a new constant is
added, to not stall too long on regular commands failures.

Second is to define timeouts for new tpm2 commands
defined in TCG 1.36 spec.

Tomas Winkler (5):
  tpm: sort objects in the Makefile
  tpm: factor out tpm 1.2 duration calculation to tpm1-cmd.c
  tpm2: add longer timeouts for creation commands.
  tpm2: add new tpm2 commands according to TCG 1.36
  tmp: factor out tpm_get_timeouts

V2: 1. Makefile go back tpm-y construct.
2. Add more info to longer timouts patch
3. Rebase other patches
4. Remove patch not connected to timouts from the series.

 drivers/char/tpm/Makefile|  16 +-
 drivers/char/tpm/st33zp24/st33zp24.c |   2 +-
 drivers/char/tpm/tpm-interface.c | 408 +-
 drivers/char/tpm/tpm.h   |  74 ---
 drivers/char/tpm/tpm1-cmd.c  | 416 +++
 drivers/char/tpm/tpm2-cmd.c  | 220 --
 drivers/char/tpm/tpm_i2c_nuvoton.c   |  10 +-
 drivers/char/tpm/tpm_tis_core.c  |   2 +-
 drivers/char/tpm/xen-tpmfront.c  |   2 +-
 9 files changed, 582 insertions(+), 568 deletions(-)
 create mode 100644 drivers/char/tpm/tpm1-cmd.c

-- 
2.14.3

[PATCH 3/5 V2] tpm2: add longer timeouts for creation commands.

[Tomas Winkler]

TPM2_CC_Create(0x153) and TPM2_CC_CreatePrimary (0x131) involve generation
of crypto keys which can be a computationally intensive task.
The timeout is set to 3min.
Rather than increasing default timeout a new constant is
added, to not stall for too long on regular commands failures.

Signed-off-by: Tomas Winkler 
---
V2: add more explanation to the commit message.

 drivers/char/tpm/tpm-interface.c |  3 +++
 drivers/char/tpm/tpm.h   | 28 ++--
 drivers/char/tpm/tpm2-cmd.c  |  8 +---
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 402e54252b22..40d1770f6b38 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -421,6 +421,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
msecs_to_jiffies(TPM2_DURATION_MEDIUM);
chip->duration[TPM_LONG] =
msecs_to_jiffies(TPM2_DURATION_LONG);
+   chip->duration[TPM_LONG_LONG] =
+   msecs_to_jiffies(TPM2_DURATION_LONG_LONG);
 
chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
return 0;
@@ -509,6 +511,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
usecs_to_jiffies(be32_to_cpu(cap.duration.tpm_medium));
chip->duration[TPM_LONG] =
usecs_to_jiffies(be32_to_cpu(cap.duration.tpm_long));
+   chip->duration[TPM_LONG_LONG] = 0; /* not used under 1.2 */
 
/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
 * value wrong and apparently reports msecs rather than usecs. So we
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 38197a30ad7b..30610d97d30c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -67,7 +67,9 @@ enum tpm_duration {
TPM_SHORT = 0,
TPM_MEDIUM = 1,
TPM_LONG = 2,
+   TPM_LONG_LONG = 3,
TPM_UNDEFINED,
+   TPM_DURATION_MAX = TPM_UNDEFINED,
 };
 
 #define TPM_WARN_RETRY  0x800
@@ -79,15 +81,20 @@ enum tpm_duration {
 #define TPM_HEADER_SIZE10
 
 enum tpm2_const {
-   TPM2_PLATFORM_PCR   = 24,
-   TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8),
-   TPM2_TIMEOUT_A  = 750,
-   TPM2_TIMEOUT_B  = 2000,
-   TPM2_TIMEOUT_C  = 200,
-   TPM2_TIMEOUT_D  = 30,
-   TPM2_DURATION_SHORT = 20,
-   TPM2_DURATION_MEDIUM= 750,
-   TPM2_DURATION_LONG  = 2000,
+   TPM2_PLATFORM_PCR   = 24,
+   TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8),
+};
+
+enum tpm2_timeouts {
+   TPM2_TIMEOUT_A  =750,
+   TPM2_TIMEOUT_B  =   2000,
+   TPM2_TIMEOUT_C  =200,
+   TPM2_TIMEOUT_D  = 30,
+   TPM2_DURATION_SHORT = 20,
+   TPM2_DURATION_MEDIUM=750,
+   TPM2_DURATION_LONG  =   2000,
+   TPM2_DURATION_LONG_LONG = 30,
+   TPM2_DURATION_DEFAULT   = 12,
 };
 
 enum tpm2_structures {
@@ -123,6 +130,7 @@ enum tpm2_algorithms {
 
 enum tpm2_command_codes {
TPM2_CC_FIRST   = 0x011F,
+   TPM2_CC_CREATE_PRIMARY  = 0x0131,
TPM2_CC_SELF_TEST   = 0x0143,
TPM2_CC_STARTUP = 0x0144,
TPM2_CC_SHUTDOWN= 0x0145,
@@ -227,7 +235,7 @@ struct tpm_chip {
unsigned long timeout_c; /* jiffies */
unsigned long timeout_d; /* jiffies */
bool timeout_adjusted;
-   unsigned long duration[3]; /* jiffies */
+   unsigned long duration[TPM_DURATION_MAX]; /* jiffies */
bool duration_adjusted;
 
struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a700f8f9ead7..c1ddbbba406e 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -90,6 +90,8 @@ static struct tpm2_hash tpm2_hash_map[] = {
  * of time the chip could take to return the result. The values
  * of the SHORT, MEDIUM, and LONG durations are taken from the
  * PC Client Profile (PTP) specification.
+ * LONG_LONG is for commands that generates keys which empirically
+ * takes longer time on some systems.
  */
 static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
TPM_UNDEFINED,  /* 11F */
@@ -110,7 +112,7 @@ static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - 
TPM2_CC_FIRST + 1] = {
TPM_UNDEFINED,  /* 12e */
TPM_UNDEFINED,  /* 12f */
TPM_UNDEFINED,  /* 130 */
-   TPM_UNDEFINED,  /* 131 */
+   TPM_LONG_LONG,  /* 131 */
TPM_UNDEFINED,  /* 132 */
TPM_UNDEFINED,  /* 133 */
TPM_UNDEFINED,  /* 134 */
@@ -144,7 +146,7 @@ static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - 
TPM2_CC_FIRST + 1] = {
TPM_UNDEFINED,  /* 150 */
TPM_UNDEFINED,  /* 1

Re: [RFC/RFT][PATCH v2 4/6] cpuidle: Return nohz hint from cpuidle_select()

[Rafael J. Wysocki]

Bummer. :-(

On Tue, Mar 6, 2018 at 10:05 AM, Rafael J. Wysocki  wrote:

> +   if (drv->states[idx].flags & CPUIDLE_FLAG_POLLING) {
> +   *nohz_ret = false;
> +   } else if (drv->states[idx].target_residency < TICK_USEC) {
> +   /*
> +* Do not stop the tick if there is at least one more state
> +* within the tick period range that could be used if longer
> +* idle duration was predicted.
> +*/
> +   *nohz_ret = first_idx > idx &&
> +   drv->states[first_idx].target_residency < 
> TICK_USEC;

This is reversed, sent a wrong version of the patch.

I'll resend with this fixed shortly.

[PATCH 1/5 V2] tpm: sort objects in the Makefile

[Tomas Winkler]

Make the tpm Makefile a bit more in order by putting
objects in one column and group together tpm2 modules

Signed-off-by: Tomas Winkler 
---
V2: 1. back to tpm-y notation
2. Partially sort files alphanumerically.

 drivers/char/tpm/Makefile | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index acd758381c58..b2d6ca9e62e4 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,12 +3,21 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
- tpm2-space.o
+tpm-y := tpm-chip.o
+tpm-y += tpm-dev.o
+tpm-y += tpm-dev-common.o
+tpm-y += tpm-interface.o
+tpm-y += tpmrm-dev.o
+tpm-y += tpm-sysfs.o
+tpm-y += tpm1_eventlog.o
+tpm-y += tpm2-cmd.o
+tpm-y += tpm2_eventlog.o
+tpm-y += tpm2-space.o
+
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
+
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
 obj-$(CONFIG_TCG_TIS) += tpm_tis.o
 obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
-- 
2.14.3

Re: [PATCH v4.15.7 1/1] on Intel, VDSO should handle CLOCK_MONOTONIC_RAW and export 'tsc_calibration' pointer

[Thomas Gleixner]

Jason,

On Mon, 5 Mar 2018, Jason Vas Dias wrote:

thanks for providing this. A few formal nits first.

Please read Documentation/process/submitting-patches.rst

Patches need a concise subject line and the subject line wants a prefix, in
this case 'x86/vdso'.

Please don't put anything past the patch. Your delimiters are human
readable, but cannot be handled by tools.

Also please follow the kernel coding style guide lines.

> It also provides a new function in the VDSO :
> 
> struct linux_timestamp_conversion
> { u32  mult;
>   u32  shift;
> };
> extern
>   const struct linux_timestamp_conversion *
>   __vdso_linux_tsc_calibration(void);
> 
> which can be used by user-space rdtsc / rdtscp issuers
> by using code such as in
>tools/testing/selftests/vDSO/parse_vdso.c
> to call vdso_sym("LINUX_2.6", "__vdso_linux_tsc_calibration"),
> which returns a pointer to the function in the VDSO, which
> returns the address of the 'mult' field in the vsyscall_gtod_data.

No, that's just wrong. The VDSO data is solely there for the VDSO accessor
functions and not to be exposed to random user space.

> Thus user-space programs can use rdtscp and interpret its return values
> in exactly the same way the kernel would, but without entering the kernel.

The VDSO clock_gettime() functions are providing exactly this mechanism.

>  As pointed out in Bug # 198961 :
>https://bugzilla.kernel.org/show_bug.cgi?id=198961
>  which contains extra test programs and the full story behind this change,
>  using CLOCK_MONOTONIC_RAW without the patch results in
>  a minimum measurable time (latency) of @ 300 - 700ns because of
>  the syscall used by vdso_fallback_gtod() .
> 
> With the patch, the latency falls to @ 100ns .
> 
> The latency would be @ 16 - 32 ns  if the do_monotonic_raw()
> handler could record its previous TSC value and seconds return value
> somewhere, but since the VDSO has no data region or writable page,
> of course it cannot .

And even if it could, it's not as simple as you want it to be. Clocksources
can change during runtime and without effective protection the values are
just garbage.

> Hence, to enable effective use of TSC by user space programs, Linux must
> provide a way for them to discover the calibration mult and shift values
> the kernel uses for the clock source ; only by doing so can user-space
> get values that are comparable to kernel generated values.

Linux must not do anything. It can provide a vdso implementation of
CLOCK_MONOTONIC_RAW, which does not enter the kernel, but not exposure to
data which is not reliably accessible by random user space code.

> And I'd really like to know: why does the gtod->mult value change ?
> After TSC calibration, it and the shift are calculated to render the
> best approximation of a nanoseconds value from the TSC value.
> 
> The TSC is MEANT to be monotonic and to continue in sleep states
> on modern Intel CPUs . So why does the gtod->mult change ?

You are missing the fact that gtod->mult/shift are used for CLOCK_MONOTONIC
and CLOCK_REALTIME, which are adjusted by NTP/PTP to provide network
synchronized time. That means CLOCK_MONOTONIC is providing accurate
and slope compensated nanoseconds.

The raw TSC conversion, even if it is sane hardware, provides just some
approximation of nanoseconds which can be off by quite a margin.

> But the mult value does change.  Currently there is no way for user-space
> programs to discover that such a change has occurred, or when . With this
> very tiny simple patch, they could know instantly when such changes
> occur, and could implement TSC readers that perform the full conversion
> with latencies of 15-30ns (on my CPU).

No. Accessing the mult/shift pair without protection is racy and can lead
to completely erratic results.

> +notrace static int __always_inline do_monotonic_raw( struct timespec *ts)
> +{
> +volatile u32 tsc_lo=0, tsc_hi=0, tsc_cpu=0; // so same instrs
> generated for 64-bit as for 32-bit builds
> +u64 ns;
> +register u64 tsc=0;
> +if (gtod->vclock_mode == VCLOCK_TSC)
> +{ asm volatile
> +( "rdtscp"
> +: "=a" (tsc_lo)
> +, "=d" (tsc_hi)
> +, "=c" (tsc_cpu)
> +); // : eax, edx, ecx used - NOT rax, rdx, rcx

If you look at the existing VDSO time getters then you'll notice that
they use accessor functions and not open coded asm constructs.

> +   tsc = u64)tsc_hi) & 0xUL) << 32) | (((u64)tsc_lo)
> & 0xUL);
> +   tsc*= gtod->mult;
> +   tsc   >>= gtod->shift;
> +   ts->tv_sec  = __iter_div_u64_rem(tsc, NSEC_PER_SEC, &ns);
> +   ts->tv_nsec = ns;

This is horrible. Please look at the kernel side implementation of
clock_gettime(CLOCK_MONOTONIC_RAW). The VDSO side can be implemented in the
same way. All what is required is to expose the relevant information in the
existing vsyscall_gtod_data data structure.

> +struct linux_timestamp_conversion
> +{ u32  mult;
> +

[PATCH 4/4] tpm: move tpm1 selftest code from tpm-interface tpm1-cmd.c

[Tomas Winkler]

Move the tmp1 selftest code functions to tpm1-cmd.c
tpm_pcr_read_dev to tpm1_pcr_read_dev
tpm_continue_selftest to tpm1_continue_selftest
tpm_do_selftest to tpm1_do_selftest

Signed-off-by: Tomas Winkler 
---
 drivers/char/tpm/st33zp24/st33zp24.c |   2 +-
 drivers/char/tpm/tpm-interface.c | 144 +--
 drivers/char/tpm/tpm-sysfs.c |   2 +-
 drivers/char/tpm/tpm.h   |   4 +-
 drivers/char/tpm/tpm1-cmd.c  | 139 +
 drivers/char/tpm/tpm_tis_core.c  |   2 +-
 6 files changed, 147 insertions(+), 146 deletions(-)

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c 
b/drivers/char/tpm/st33zp24/st33zp24.c
index ad03c99899fa..575b7c2eab25 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -651,7 +651,7 @@ int st33zp24_pm_resume(struct device *dev)
} else {
ret = tpm_pm_resume(dev);
if (!ret)
-   tpm_do_selftest(chip);
+   tpm1_do_selftest(chip);
}
return ret;
 } /* st33zp24_pm_resume() */
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 59ca2e30b4d2..a7dfe286b343 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -366,59 +366,6 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm_get_timeouts);
 
-#define TPM_ORD_CONTINUE_SELFTEST 83
-#define CONTINUE_SELFTEST_RESULT_SIZE 10
-
-static const struct tpm_input_header continue_selftest_header = {
-   .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
-   .length = cpu_to_be32(10),
-   .ordinal = cpu_to_be32(TPM_ORD_CONTINUE_SELFTEST),
-};
-
-/**
- * tpm_continue_selftest -- run TPM's selftest
- * @chip: TPM chip to use
- *
- * Returns 0 on success, < 0 in case of fatal error or a value > 0 representing
- * a TPM error code.
- */
-static int tpm_continue_selftest(struct tpm_chip *chip)
-{
-   int rc;
-   struct tpm_cmd_t cmd;
-
-   cmd.header.in = continue_selftest_header;
-   rc = tpm_transmit_cmd(chip, NULL, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
- 0, 0, "continue selftest");
-   return rc;
-}
-
-#define TPM_ORDINAL_PCRREAD 21
-#define READ_PCR_RESULT_SIZE 30
-#define READ_PCR_RESULT_BODY_SIZE 20
-static const struct tpm_input_header pcrread_header = {
-   .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
-   .length = cpu_to_be32(14),
-   .ordinal = cpu_to_be32(TPM_ORDINAL_PCRREAD)
-};
-
-int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
-{
-   int rc;
-   struct tpm_cmd_t cmd;
-
-   cmd.header.in = pcrread_header;
-   cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
-   rc = tpm_transmit_cmd(chip, NULL, &cmd, READ_PCR_RESULT_SIZE,
- READ_PCR_RESULT_BODY_SIZE, 0,
- "attempting to read a pcr value");
-
-   if (rc == 0)
-   memcpy(res_buf, cmd.params.pcrread_out.pcr_result,
-  TPM_DIGEST_SIZE);
-   return rc;
-}
-
 /**
  * tpm_is_tpm2 - do we a have a TPM2 chip?
  * @chip:  a &struct tpm_chip instance, %NULL for the default chip
@@ -459,10 +406,12 @@ int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 
*res_buf)
chip = tpm_chip_find_get(chip);
if (!chip)
return -ENODEV;
+
if (chip->flags & TPM_CHIP_FLAG_TPM2)
rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
else
-   rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
+   rc = tpm1_pcr_read_dev(chip, pcr_idx, res_buf);
+
tpm_put_ops(chip);
return rc;
 }
@@ -513,93 +462,6 @@ int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
const u8 *hash)
 }
 EXPORT_SYMBOL_GPL(tpm_pcr_extend);
 
-/**
- * tpm_do_selftest - have the TPM continue its selftest and wait until it
- *   can receive further commands
- * @chip: TPM chip to use
- *
- * Returns 0 on success, < 0 in case of fatal error or a value > 0 representing
- * a TPM error code.
- */
-int tpm_do_selftest(struct tpm_chip *chip)
-{
-   int rc;
-   unsigned int loops;
-   unsigned int delay_msec = 100;
-   unsigned long duration;
-   u8 dummy[TPM_DIGEST_SIZE];
-
-   duration = tpm1_calc_ordinal_duration(chip, TPM_ORD_CONTINUE_SELFTEST);
-
-   loops = jiffies_to_msecs(duration) / delay_msec;
-
-   rc = tpm_continue_selftest(chip);
-   /* This may fail if there was no TPM driver during a suspend/resume
-* cycle; some may return 10 (BAD_ORDINAL), others 28 (FAILEDSELFTEST)
-*/
-   if (rc)
-   return rc;
-
-   do {
-   /* Attempt to read a PCR value */
-   rc = tpm_pcr_read_dev(chip, 0, dummy);
-
-   /* Some buggy TPMs will not respond to tpm_tis_ready() for
-* around 300ms while the self test is ongoing, keep trying
-

[PATCH v2 2/2] staging: most: Indent function parameter.

[Quytelda Kahja]

Indent the parameters for a function call that extends past 80 characters.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/most/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index 67e2d7f29967..8d311970225e 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -1473,7 +1473,8 @@ void most_deregister_interface(struct most_interface 
*iface)
int i;
struct most_channel *c;
 
-   pr_info("deregistering device %s (%s)\n", dev_name(&iface->dev), 
iface->description);
+   pr_info("deregistering device %s (%s)\n", dev_name(&iface->dev),
+   iface->description);
for (i = 0; i < iface->num_channels; i++) {
c = iface->p->channel[i];
if (c->pipe0.comp)
-- 
2.16.2

[PATCH 2/4] tmp: move tpm_getcap to tpm1-cmd.c

[Tomas Winkler]

1. Move tpm_getcap to tpm1-cmd. Rename the function to tpm1_getcap.
2. Remove unsed tpm_getcap_header with unused constant
as the functionality is implemented using tpm_buf construct.

Signed-off-by: Tomas Winkler 
---
 drivers/char/tpm/tpm-interface.c | 47 +---
 drivers/char/tpm/tpm-sysfs.c | 48 ++---
 drivers/char/tpm/tpm.h   |  4 ++--
 drivers/char/tpm/tpm1-cmd.c  | 51 +---
 drivers/char/tpm/tpm_tis_core.c  |  2 +-
 5 files changed, 71 insertions(+), 81 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 83eeefb2a4af..6b70cefed505 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -354,52 +354,6 @@ int tpm_startup(struct tpm_chip *chip)
return rc;
 }
 
-#define TPM_DIGEST_SIZE 20
-#define TPM_RET_CODE_IDX 6
-#define TPM_INTERNAL_RESULT_SIZE 200
-#define TPM_ORD_GET_CAP 101
-#define TPM_ORD_GET_RANDOM 70
-
-static const struct tpm_input_header tpm_getcap_header = {
-   .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
-   .length = cpu_to_be32(22),
-   .ordinal = cpu_to_be32(TPM_ORD_GET_CAP)
-};
-
-ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
-  const char *desc, size_t min_cap_length)
-{
-   struct tpm_buf buf;
-   int rc;
-
-   rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_CAP);
-   if (rc)
-   return rc;
-
-   if (subcap_id == TPM_CAP_VERSION_1_1 ||
-   subcap_id == TPM_CAP_VERSION_1_2) {
-   tpm_buf_append_u32(&buf, subcap_id);
-   tpm_buf_append_u32(&buf, 0);
-   } else {
-   if (subcap_id == TPM_CAP_FLAG_PERM ||
-   subcap_id == TPM_CAP_FLAG_VOL)
-   tpm_buf_append_u32(&buf, TPM_CAP_FLAG);
-   else
-   tpm_buf_append_u32(&buf, TPM_CAP_PROP);
-
-   tpm_buf_append_u32(&buf, 4);
-   tpm_buf_append_u32(&buf, subcap_id);
-   }
-   rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
- min_cap_length, 0, desc);
-   if (!rc)
-   *cap = *(cap_t *)&buf.data[TPM_HEADER_SIZE + 4];
-
-   tpm_buf_destroy(&buf);
-   return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_getcap);
-
 int tpm_get_timeouts(struct tpm_chip *chip)
 {
if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS)
@@ -753,6 +707,7 @@ int tpm_pm_resume(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(tpm_pm_resume);
 
+#define TPM_ORD_GET_RANDOM 70
 #define TPM_GETRANDOM_RESULT_SIZE  18
 static const struct tpm_input_header tpm_getrandom_header = {
.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 83a77a445538..008515314ae3 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -106,9 +106,9 @@ static ssize_t pcrs_show(struct device *dev, struct 
device_attribute *attr,
char *str = buf;
struct tpm_chip *chip = to_tpm_chip(dev);
 
-   rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
-   "attempting to determine the number of PCRS",
-   sizeof(cap.num_pcrs));
+   rc = tpm1_getcap(chip, TPM_CAP_PROP_PCR, &cap,
+"attempting to determine the number of PCRS",
+sizeof(cap.num_pcrs));
if (rc)
return 0;
 
@@ -132,9 +132,9 @@ static ssize_t enabled_show(struct device *dev, struct 
device_attribute *attr,
cap_t cap;
ssize_t rc;
 
-   rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
-   "attempting to determine the permanent enabled state",
-   sizeof(cap.perm_flags));
+   rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
+"attempting to determine the permanent enabled state",
+sizeof(cap.perm_flags));
if (rc)
return 0;
 
@@ -149,9 +149,9 @@ static ssize_t active_show(struct device *dev, struct 
device_attribute *attr,
cap_t cap;
ssize_t rc;
 
-   rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
-   "attempting to determine the permanent active state",
-   sizeof(cap.perm_flags));
+   rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
+"attempting to determine the permanent active state",
+sizeof(cap.perm_flags));
if (rc)
return 0;
 
@@ -166,9 +166,9 @@ static ssize_t owned_show(struct device *dev, struct 
device_attribute *attr,
cap_t cap;
ssize_t rc;
 
-   rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
-   "attempting to determine the owner state",
-   sizeof(cap.owned));
+  

[PATCH 3/4] tmp: factor out tpm1_get_random into tpm1-cmd.c

[Tomas Winkler]

Factor out get random implementation from tpm-interface.c
into tpm1_get_random function in tpm1-cmd.c.
No functional changes.

Signed-off-by: Tomas Winkler 
---
 drivers/char/tpm/tpm-interface.c | 56 
 drivers/char/tpm/tpm.h   |  2 ++
 drivers/char/tpm/tpm1-cmd.c  | 54 ++
 3 files changed, 61 insertions(+), 51 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 6b70cefed505..59ca2e30b4d2 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -707,14 +707,6 @@ int tpm_pm_resume(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(tpm_pm_resume);
 
-#define TPM_ORD_GET_RANDOM 70
-#define TPM_GETRANDOM_RESULT_SIZE  18
-static const struct tpm_input_header tpm_getrandom_header = {
-   .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
-   .length = cpu_to_be32(14),
-   .ordinal = cpu_to_be32(TPM_ORD_GET_RANDOM)
-};
-
 /**
  * tpm_get_random() - get random bytes from the TPM's RNG
  * @chip:  a &struct tpm_chip instance, %NULL for the default chip
@@ -725,57 +717,19 @@ static const struct tpm_input_header tpm_getrandom_header 
= {
  */
 int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 {
-   struct tpm_cmd_t tpm_cmd;
-   u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA), rlength;
-   int err, total = 0, retries = 5;
-   u8 *dest = out;
-
-   if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
-   return -EINVAL;
+   int err;
 
chip = tpm_chip_find_get(chip);
if (!chip)
return -ENODEV;
 
-   if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+   if (chip->flags & TPM_CHIP_FLAG_TPM2)
err = tpm2_get_random(chip, out, max);
-   tpm_put_ops(chip);
-   return err;
-   }
-
-   do {
-   tpm_cmd.header.in = tpm_getrandom_header;
-   tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
-
-   err = tpm_transmit_cmd(chip, NULL, &tpm_cmd,
-  TPM_GETRANDOM_RESULT_SIZE + num_bytes,
-  offsetof(struct tpm_getrandom_out,
-   rng_data),
-  0, "attempting get random");
-   if (err)
-   break;
-
-   recd = be32_to_cpu(tpm_cmd.params.getrandom_out.rng_data_len);
-   if (recd > num_bytes) {
-   total = -EFAULT;
-   break;
-   }
-
-   rlength = be32_to_cpu(tpm_cmd.header.out.length);
-   if (rlength < offsetof(struct tpm_getrandom_out, rng_data) +
- recd) {
-   total = -EFAULT;
-   break;
-   }
-   memcpy(dest, tpm_cmd.params.getrandom_out.rng_data, recd);
-
-   dest += recd;
-   total += recd;
-   num_bytes -= recd;
-   } while (retries-- && total < max);
+   else
+   err = tpm1_get_random(chip, out, max);
 
tpm_put_ops(chip);
-   return total ? total : -EIO;
+   return err;
 }
 EXPORT_SYMBOL_GPL(tpm_get_random);
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1a8ef4d3cb1c..b5fe0269a833 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -537,6 +537,8 @@ int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
const u8 *hash,
const char *log_msg);
 ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc, size_t min_cap_length);
+int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+
 int tpm_pm_suspend(struct device *dev);
 int tpm_pm_resume(struct device *dev);
 
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 0e10a40b28b1..2c075a03a17a 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -472,3 +472,57 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, 
cap_t *cap,
return rc;
 }
 EXPORT_SYMBOL_GPL(tpm1_getcap);
+
+#define TPM_ORD_GET_RANDOM 70
+#define TPM_GETRANDOM_RESULT_SIZE  18
+static const struct tpm_input_header tpm_getrandom_header = {
+   .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
+   .length = cpu_to_be32(14),
+   .ordinal = cpu_to_be32(TPM_ORD_GET_RANDOM)
+};
+
+int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max)
+{
+   struct tpm_cmd_t tpm_cmd;
+   u32 recd;
+   u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
+   u32 rlength;
+   int err, total = 0, retries = 5;
+   u8 *dest = out;
+
+   if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
+   return -EINVAL;
+
+   do {
+   tpm_cmd.header.in = tpm_getrandom_header;
+   tpm_cmd.params.getrandom_in.num_bytes = 

[PATCH 0/4] tpm: factor out tpm1 code into tpm1-cmd.c

[Tomas Winkler]

Move tpm 1.x implementation to tpm1-cmd.c similarly to tpm2-cmd.c
This is work in progress of possible dropping compiling out
one tpm 1.x or tpm 2.x in case not needed on a target platform.

Tomas Winkler (4):
  tmp: move tpm1_pcr_extend to tpm1-cmd.c
  tmp: move tpm_getcap to tpm1-cmd.c
  tmp: factor out tpm1_get_random into tpm1-cmd.c
  tpm: move tpm1 selftest code from tpm-interface tpm1-cmd.c

 drivers/char/tpm/st33zp24/st33zp24.c |   2 +-
 drivers/char/tpm/tpm-interface.c | 273 +--
 drivers/char/tpm/tpm-sysfs.c |  50 +++
 drivers/char/tpm/tpm.h   |  12 +-
 drivers/char/tpm/tpm1-cmd.c  | 267 +-
 drivers/char/tpm/tpm_tis_core.c  |   4 +-
 6 files changed, 303 insertions(+), 305 deletions(-)

-- 
2.14.3

[PATCH 1/4] tmp: move tpm1_pcr_extend to tpm1-cmd.c

[Tomas Winkler]

Move tpm1_pcr_extend to tpm1-cmd.c and remove
unused pcrextend_header structure.

Signed-off-by: Tomas Winkler 
---
 drivers/char/tpm/tpm-interface.c | 28 
 drivers/char/tpm/tpm.h   |  2 ++
 drivers/char/tpm/tpm1-cmd.c  | 23 +++
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 7f6968b750c8..83eeefb2a4af 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -514,34 +514,6 @@ int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 
*res_buf)
 }
 EXPORT_SYMBOL_GPL(tpm_pcr_read);
 
-#define TPM_ORD_PCR_EXTEND 20
-#define EXTEND_PCR_RESULT_SIZE 34
-#define EXTEND_PCR_RESULT_BODY_SIZE 20
-static const struct tpm_input_header pcrextend_header = {
-   .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
-   .length = cpu_to_be32(34),
-   .ordinal = cpu_to_be32(TPM_ORD_PCR_EXTEND)
-};
-
-static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
-  char *log_msg)
-{
-   struct tpm_buf buf;
-   int rc;
-
-   rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
-   if (rc)
-   return rc;
-
-   tpm_buf_append_u32(&buf, pcr_idx);
-   tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
-
-   rc = tpm_transmit_cmd(chip, NULL, buf.data, EXTEND_PCR_RESULT_SIZE,
- EXTEND_PCR_RESULT_BODY_SIZE, 0, log_msg);
-   tpm_buf_destroy(&buf);
-   return rc;
-}
-
 /**
  * tpm_pcr_extend - extend a PCR value in SHA1 bank.
  * @chip:  a &struct tpm_chip instance, %NULL for the default chip
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 48706f091856..4306c878f1d9 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -535,6 +535,8 @@ int tpm_do_selftest(struct tpm_chip *chip);
 int tpm1_auto_startup(struct tpm_chip *chip);
 int tpm1_get_timeouts(struct tpm_chip *chip);
 unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
+int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
+   const char *log_msg);
 int tpm_pm_suspend(struct device *dev);
 int tpm_pm_resume(struct device *dev);
 
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index e48be0c09131..dc07eafe9d69 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -414,3 +414,26 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
return 0;
 }
+
+#define TPM_ORD_PCR_EXTEND 20
+#define EXTEND_PCR_RESULT_SIZE 34
+#define EXTEND_PCR_RESULT_BODY_SIZE 20
+int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
+   const char *log_msg)
+{
+   struct tpm_buf buf;
+   int rc;
+
+   rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
+   if (rc)
+   return rc;
+
+   tpm_buf_append_u32(&buf, pcr_idx);
+   tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
+
+   rc = tpm_transmit_cmd(chip, NULL, buf.data, EXTEND_PCR_RESULT_SIZE,
+ EXTEND_PCR_RESULT_BODY_SIZE, 0, log_msg);
+   tpm_buf_destroy(&buf);
+   return rc;
+}
+
-- 
2.14.3

Re: [PATCH 1/7] platform/x86: fujitsu-laptop: Define constants for FUNC operations

[Andy Shevchenko]

On Tue, Mar 6, 2018 at 1:16 AM, Darren Hart  wrote:
> On Sun, Mar 04, 2018 at 08:44:26PM +0100, Michał Kępień wrote:
>> > On Wed, Feb 28, 2018 at 06:08:52PM +0200, Andy Shevchenko wrote:
>> > > On Tue, Feb 27, 2018 at 11:15 PM, Micha?? K??pie??  
>> > > wrote:
>> > > > Various functions exposed by the firmware through the FUNC interface
>> > > > tend to use a consistent set of integers for denoting the type of
>> > > > operation to be performed for a specified feature.  Use named constants
>> > > > instead of integers in each call_fext_func() invocation in order to 
>> > > > more
>> > > > clearly convey the intent of each call.
>> > > >
>> > > > Note that FUNC_FLAGS is a bit peculiar:
>> > >
>> > > > +/* FUNC interface - operations */
>> > > > +#define OP_GET BIT(1)
>> > > > +#define OP_GET_CAPS0
>> > > > +#define OP_GET_EVENTS  BIT(0)
>> > > > +#define OP_GET_EXT BIT(2)
>> > > > +#define OP_SET BIT(0)
>> > > > +#define OP_SET_EXT (BIT(2) | BIT(0))
>> > >
>> > > Hmm... this looks unordered a bit.
>> >
>> > It seems to be ordered alphabetically on the identifier.  Andy, is it
>> > preferred to order defines like this based on resolved numeric order?
>>
>> Just to expand on what Jonathan wrote above: if you take a peek at the
>> end result of the patch series, you will notice a pattern: constants in
>> each section are ordered alphabetically by their name.  I wanted all
>> sections to be consistently ordered.  If you would rather have me order
>> things by the bit index, sure, no problem, just please note that the
>> order above is not accidental.
>
> Hrm. In my experience it is more typical to order by value (bit), that's a
> little less obvious when using the BIT()|BIT() macros though. So long as it's
> consistent, I think that's what matters most.

What I'm trying to tell is about consistency of style.

So, imagine if we have two bitfields in some register, one with one
bit and the other with two.
And for latter one only 3 states are possible (00, 10, 11), so,

REG1_FLDA  BIT(0)
REG1_FLDB_STATE0  0
REG1_FLDB_STATE2  BIT(2)
REG1_FLDB_STATE3  BIT(2) | BIT(3) // or 0x6, or (3 << 1)

Above is not what I would like to see.

The consistent may be one of the following

REG1_FLDA  BIT(0)
REG1_FLDB_STATE0  (0 << 1)
REG1_FLDB_STATE2  (2 << 1)
REG1_FLDB_STATE3  (3 << 1)

or (implying that in the code _SHIFT is used)

REG1_FLDA  BIT(0)
REG1_FLDB_SHIFT  1
REG1_FLDB_STATE0  0
REG1_FLDB_STATE2  2
REG1_FLDB_STATE3  3

or (perhaps less wanted)

REG1_FLDA  (1 << 0)
REG1_FLDB_STATE0  (0 << 1)
REG1_FLDB_STATE2  (2 << 1)
REG1_FLDB_STATE3  (3 << 1)

-- 
With Best Regards,
Andy Shevchenko

[PATCH V2] tpm_crb: use __le64 annotated variable for response buffer address

[Tomas Winkler]

use __le64 annotated variable for response buffer address as this is
read in little endian format form the register.

This suppresses sparse warning
drivers/char/tpm/tpm_crb.c:558:18: warning: cast to restricted __le64

Signed-off-by: Tomas Winkler 
---
V2: Add more info into the commit message

 drivers/char/tpm/tpm_crb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index acfcdc6f31af..29fecdea0b2d 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -489,6 +489,7 @@ static int crb_map_io(struct acpi_device *device, struct 
crb_priv *priv,
u32 pa_high, pa_low;
u64 cmd_pa;
u32 cmd_size;
+   __le64 __rsp_pa;
u64 rsp_pa;
u32 rsp_size;
int ret;
@@ -554,8 +555,8 @@ static int crb_map_io(struct acpi_device *device, struct 
crb_priv *priv,
goto out;
}
 
-   memcpy_fromio(&rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
-   rsp_pa = le64_to_cpu(rsp_pa);
+   memcpy_fromio(&__rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
+   rsp_pa = le64_to_cpu(__rsp_pa);
rsp_size = crb_fixup_cmd_size(dev, &io_res, rsp_pa,
  ioread32(&priv->regs_t->ctrl_rsp_size));
 
-- 
2.14.3

Re: [PATCH v2 09/11] kconfig: unittest: test randconfig for choice in choice

[Masahiro Yamada]

2018-03-03 18:34 GMT+09:00 Ulf Magnusson :
> On Fri, Mar 2, 2018 at 10:29 PM, Luis R. Rodriguez  wrote:
>>
>>
>> On Fri, Mar 2, 2018, 3:14 AM Ulf Magnusson  wrote:
>>>
>>> On Fri, Mar 2, 2018 at 5:31 AM, Masahiro Yamada
>>> > This is complicated usage, but it is still used in the real world;
>>> > drivers/usb/gadget/legacy/Kconfig is source'd in a choice context,
>>> > then creates a sub-choice in it.
>>>
>>> That file is the only one that does all that weird choice stuff btw.
>>>
>>> It's as if it was written to make use of as much obscure Kconfig stuff
>>> as possible. :P
>>
>>
>> Can't we just use another way to describe this requirement on this file,
>> with the trade-off of simplifying kconfig semantics?
>>
>>   Luis
>
> I don't think changing how drivers/usb/gadget/legacy/Kconfig does
> things would allow for any simplifications, unfortunately (except to
> get rid of the fix tested by this patch, maybe).

We could also revert 3b9a19e08960e5cd.  :)


> Being able to have non-choice-value symbols and choices in choices is
> a side effect of automatic submenu creation. Symbols and choices that
> depend on the symbol before them end up in a submenu, and only the
> top-level symbols in the choice are marked as choice value symbols.
>
> I always wondered whether that was an intended feature or just
> something people discovered works (it seems to work anyway...). It
> feels pretty iffy to have cosmetic submenus affect behavior like that,
> but playing devil's advocate, it kinda makes sense to put symbols
> close to the symbols they depend on if you can get away with it.
>

I am happy to drop this test
if removing the seem-to-work feature will clean up the code.


Now I am accumulating test cases.



-- 
Best Regards
Masahiro Yamada

[PATCH RESEND v5] perf/core: Fix installing arbitrary cgroup event into cpu

[linxiulei]

From: "leilei.lin" 

Do not install cgroup event into the CPU context and schedule it
if the cgroup is not running on this CPU

While there is no task of cgroup running specified CPU, current
kernel still install cgroup event into CPU context that causes
another cgroup event can't be installed into this CPU.

This patch prevent scheduling events at __perf_install_in_context()
and installing events at list_update_cgroup_event() if cgroup isn't
running on specified CPU.

Signed-off-by: leilei.lin 
---
 v2: Set cpuctx->cgrp only if the same cgroup is running on this
   CPU otherwise following events couldn't be activated immediately
 v3: Enhance the comments and commit message
 v4: Adjust to config
 v5: Clear cpuctx->cgrp only when no cgroup event exists

 kernel/events/core.c | 54 +++-
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4df5b69..f3ffa70 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -933,31 +933,45 @@ list_update_cgroup_event(struct perf_event *event,
 {
struct perf_cpu_context *cpuctx;
struct list_head *cpuctx_entry;
+   struct perf_cgroup *cgrp;
 
if (!is_cgroup_event(event))
return;
 
-   if (add && ctx->nr_cgroups++)
-   return;
-   else if (!add && --ctx->nr_cgroups)
-   return;
/*
 * Because cgroup events are always per-cpu events,
 * this will always be called from the right CPU.
 */
cpuctx = __get_cpu_context(ctx);
-   cpuctx_entry = &cpuctx->cgrp_cpuctx_entry;
-   /* cpuctx->cgrp is NULL unless a cgroup event is active in this CPU .*/
-   if (add) {
-   struct perf_cgroup *cgrp = perf_cgroup_from_task(current, ctx);
+   cgrp = perf_cgroup_from_task(current, ctx);
+
+   /*
+* if only the cgroup is running on this cpu
+* and cpuctx->cgrp == NULL (otherwise it would've
+* been set with running cgroup), we put this cgroup
+* into cpu context. Or it would case mismatch in
+* following cgroup events at event_filter_match()
+*/
+   if (add && !cpuctx->cgrp &&
+   cgroup_is_descendant(cgrp->css.cgroup,
+   event->cgrp->css.cgroup)) {
+   cpuctx->cgrp = cgrp;
+   }
+
+   if (add && ctx->nr_cgroups++)
+   return;
+   else if (!add && --ctx->nr_cgroups)
+   return;
 
+   /* no cgroup running */
+   if (!add)
+   cpuctx->cgrp = NULL;
+
+   cpuctx_entry = &cpuctx->cgrp_cpuctx_entry;
+   if (add)
list_add(cpuctx_entry, this_cpu_ptr(&cgrp_cpuctx_list));
-   if (cgroup_is_descendant(cgrp->css.cgroup, 
event->cgrp->css.cgroup))
-   cpuctx->cgrp = cgrp;
-   } else {
+   else
list_del(cpuctx_entry);
-   cpuctx->cgrp = NULL;
-   }
 }
 
 #else /* !CONFIG_CGROUP_PERF */
@@ -2311,6 +2325,20 @@ static int  __perf_install_in_context(void *info)
raw_spin_lock(&task_ctx->lock);
}
 
+#ifdef CONFIG_CGROUP_PERF
+   if (is_cgroup_event(event)) {
+   /*
+* Only care about cgroup events.
+*
+* If only the task belongs to cgroup of this event,
+* we will continue the installment
+*/
+   struct perf_cgroup *cgrp = perf_cgroup_from_task(current, ctx);
+   reprogram = cgroup_is_descendant(cgrp->css.cgroup,
+   event->cgrp->css.cgroup);
+   }
+#endif
+
if (reprogram) {
ctx_sched_out(ctx, cpuctx, EVENT_TIME);
add_event_to_ctx(event, ctx);
-- 
2.8.4.31.g9ed660f

Re: [PATCH v3 2/2] kconfig: rename silentoldconfig to syncconfig

[Masahiro Yamada]

2018-03-01 15:34 GMT+09:00 Masahiro Yamada :
> As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help
> and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a
> historical misnomer.  That commit removed it from help and docs since
> it is an internal interface.  If so, it should be allowed to rename
> it to something more intuitive.  'syncconfig' is the one I came up
> with because it updates the .config if necessary, then synchronize
> include/generated/autoconf.h and include/config/* with it.
>
> You should not manually invoke 'silentoldcofig'.  Display warning if
> used in case existing scripts are doing wrong.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
> Changes in v3:
>   - Fix Documentation/networking/i40e.txt
>   - Display warning if silentoldconfig is used
>
> Changes in v2:
>   - newly added
>

Both applied to linux-kbuild/kconfig.


-- 
Best Regards
Masahiro Yamada

[PATCH 0/2] crypto: introduce SM4 symmetric cipher algorithm

[Gilad Ben-Yossef]

Introduce the SM4 cipher algorithms (OSCCA GB/T 32907-2016) and
related tests.

SM4 (GBT.32907-2016) is a cryptographic standard issued by the
Organization of State Commercial Administration of China (OSCCA)
as an authorized cryptographic algorithms for the use within China.

SMS4 was originally created for use in protecting wireless
networks, and is mandated in the Chinese National Standard for
Wireless LAN WAPI (Wired Authentication and Privacy Infrastructure)
(GB.15629.11-2003).

Tested on arm 64 and 32 bit.

Gilad Ben-Yossef (2):
  crypto: introduce SM4 symmetric cipher algorithm
  crypto: introduce SM4 testmgr tests

 crypto/Kconfig   |  25 ++
 crypto/Makefile  |   1 +
 crypto/sm4_generic.c | 244 +++
 crypto/tcrypt.c  |   3 +
 crypto/testmgr.c |   9 ++
 crypto/testmgr.h | 131 +++
 include/crypto/sm4.h |  28 ++
 7 files changed, 441 insertions(+)
 create mode 100644 crypto/sm4_generic.c
 create mode 100644 include/crypto/sm4.h

-- 
2.7.4

[PATCH 1/2] crypto: introduce SM4 symmetric cipher algorithm

[Gilad Ben-Yossef]

Introduce the SM4 cipher algorithms (OSCCA GB/T 32907-2016).

SM4 (GBT.32907-2016) is a cryptographic standard issued by the
Organization of State Commercial Administration of China (OSCCA)
as an authorized cryptographic algorithms for the use within China.

SMS4 was originally created for use in protecting wireless
networks, and is mandated in the Chinese National Standard for
Wireless LAN WAPI (Wired Authentication and Privacy Infrastructure)
(GB.15629.11-2003).

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/Kconfig   |  25 ++
 crypto/Makefile  |   1 +
 crypto/sm4_generic.c | 244 +++
 include/crypto/sm4.h |  28 ++
 4 files changed, 298 insertions(+)
 create mode 100644 crypto/sm4_generic.c
 create mode 100644 include/crypto/sm4.h

diff --git a/crypto/Kconfig b/crypto/Kconfig
index de693e0..4b8df3a 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1475,6 +1475,31 @@ config CRYPTO_SERPENT_AVX2_X86_64
  See also:
  
 
+config CRYPTO_SM4
+   tristate "SM4 cipher algorithm"
+   select CRYPTO_ALGAPI
+   help
+ SM4 cipher algorithms (OSCCA GB/T 32907-2016).
+
+ SM4 (GBT.32907-2016) is a cryptographic standard issued by the
+ Organization of State Commercial Administration of China (OSCCA)
+ as an authorized cryptographic algorithms for the use within China.
+
+ SMS4 was originally created for use in protecting wireless
+ networks, and is mandated in the Chinese National Standard for
+ Wireless LAN WAPI (Wired Authentication and Privacy Infrastructure)
+ (GB.15629.11-2003).
+
+ The latest SM4 standard (GBT.32907-2016) was proposed by OSCCA and
+ standardized through TC 260 of the Standardization Administration
+ of the People's Republic of China (SAC).
+
+ The input, output, and key of SMS4 are each 128 bits.
+
+ See also: 
+
+ If unsure, say N.
+
 config CRYPTO_SPECK
tristate "Speck cipher algorithm"
select CRYPTO_ALGAPI
diff --git a/crypto/Makefile b/crypto/Makefile
index 04517b2..eb4d9cd 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_CRYPTO_SERPENT) += serpent_generic.o
 CFLAGS_serpent_generic.o := $(call cc-option,-fsched-pressure)  # 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149
 obj-$(CONFIG_CRYPTO_AES) += aes_generic.o
 CFLAGS_aes_generic.o := $(call cc-option,-fno-code-hoisting) # 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356
+obj-$(CONFIG_CRYPTO_SM4) += sm4_generic.o
 obj-$(CONFIG_CRYPTO_AES_TI) += aes_ti.o
 obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia_generic.o
 obj-$(CONFIG_CRYPTO_CAST_COMMON) += cast_common.o
diff --git a/crypto/sm4_generic.c b/crypto/sm4_generic.c
new file mode 100644
index 000..f537a27
--- /dev/null
+++ b/crypto/sm4_generic.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * SM4 Cipher Algorithm.
+ *
+ * Copyright (C) 2018 ARM Limited or its affiliates.
+ * All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static const u32 fk[4] = {
+   0xa3b1bac6, 0x56aa3350, 0x677d9197, 0xb27022dc
+};
+
+static const u8 sbox[256] = {
+   0xd6, 0x90, 0xe9, 0xfe, 0xcc, 0xe1, 0x3d, 0xb7,
+   0x16, 0xb6, 0x14, 0xc2, 0x28, 0xfb, 0x2c, 0x05,
+   0x2b, 0x67, 0x9a, 0x76, 0x2a, 0xbe, 0x04, 0xc3,
+   0xaa, 0x44, 0x13, 0x26, 0x49, 0x86, 0x06, 0x99,
+   0x9c, 0x42, 0x50, 0xf4, 0x91, 0xef, 0x98, 0x7a,
+   0x33, 0x54, 0x0b, 0x43, 0xed, 0xcf, 0xac, 0x62,
+   0xe4, 0xb3, 0x1c, 0xa9, 0xc9, 0x08, 0xe8, 0x95,
+   0x80, 0xdf, 0x94, 0xfa, 0x75, 0x8f, 0x3f, 0xa6,
+   0x47, 0x07, 0xa7, 0xfc, 0xf3, 0x73, 0x17, 0xba,
+   0x83, 0x59, 0x3c, 0x19, 0xe6, 0x85, 0x4f, 0xa8,
+   0x68, 0x6b, 0x81, 0xb2, 0x71, 0x64, 0xda, 0x8b,
+   0xf8, 0xeb, 0x0f, 0x4b, 0x70, 0x56, 0x9d, 0x35,
+   0x1e, 0x24, 0x0e, 0x5e, 0x63, 0x58, 0xd1, 0xa2,
+   0x25, 0x22, 0x7c, 0x3b, 0x01, 0x21, 0x78, 0x87,
+   0xd4, 0x00, 0x46, 0x57, 0x9f, 0xd3, 0x27, 0x52,
+   0x4c, 0x36, 0x02, 0xe7, 0xa0, 0xc4, 0xc8, 0x9e,
+   0xea, 0xbf, 0x8a, 0xd2, 0x40, 0xc7, 0x38, 0xb5,
+   0xa3, 0xf7, 0xf2, 0xce, 0xf9, 0x61, 0x15, 0xa1,
+   0xe0, 0xae, 0x5d, 0xa4, 0x9b, 0x34, 0x1a, 0x55,
+   0xad, 0x93, 0x32, 0x30, 0xf5, 0x8c, 0xb1, 0xe3,
+   0x1d, 0xf6, 0xe2, 0x2e, 0x82, 0x66, 0xca, 0x60,
+   0xc0, 0x29, 0x23, 0xab, 0x0d, 0x53, 0x4e, 0x6f,
+   0xd5, 0xdb, 0x37, 0x45, 0xde, 0xfd, 0x8e, 0x2f,
+   0x03, 0xff, 0x6a, 0x72, 0x6d, 0x6c, 0x5b, 0x51,
+   0x8d, 0x1b, 0xaf, 0x92, 0xbb, 0xdd, 0xbc, 0x7f,
+   0x11, 0xd9, 0x5c, 0x41, 0x1f, 0x10, 0x5a, 0xd8,
+   0x0a, 0xc1, 0x31, 0x88, 0xa5, 0xcd, 0x7b, 0xbd,
+   0x2d, 0x74, 0xd0, 0x12, 0xb8, 0xe5, 0xb4, 0xb0,
+   0x89, 0x69, 0x97, 0x4a, 0x0c, 0x96, 0x77, 0x7e,
+   0x65, 0xb9, 0xf1, 0x09, 0xc5

Re: [PATCH 9/9] drm/xen-front: Implement communication with backend

[Oleksandr Andrushchenko]

On 03/06/2018 11:26 AM, Daniel Vetter wrote:

On Mon, Mar 05, 2018 at 11:30:35AM +0200, Oleksandr Andrushchenko wrote:

On 03/05/2018 11:25 AM, Daniel Vetter wrote:

On Wed, Feb 21, 2018 at 10:03:42AM +0200, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Handle communication with the backend:
   - send requests and wait for the responses according
 to the displif protocol
   - serialize access to the communication channel
   - time-out used for backend communication is set to 3000 ms
   - manage display buffers shared with the backend

Signed-off-by: Oleksandr Andrushchenko 

After the demidlayering it probably makes sense to merge this with the
overall kms/basic-drm-driver patch. Up to you really.

The reason for such partitioning here and before was that
I can have Xen/DRM parts separate, so those are easier for
review by Xen/DRM communities. So, I would prefer to have it
as it is

Well for reviewing the kms parts I need to check what the xen parts are
doing (at least sometimes), since semantics of what you're doing matter,
and there's a few cases which new drivers tend to get wrong. So for me,
this splitting makes stuff actually harder to review.

And I guess for the xen folks it won't hurt if they see a bit clearer how
it's used on the drm side (even if they might not really understand what's
going on). If we have some superficial abstraction in between each of the
subsystem maintainers might make assumptions about what the other side of
the code is doing which turn out to be wrong, and that's not good.

Just explaining my motivation for why I don't like abstractions and
splitting stuff up into patches that don't make much sense on their own
(because the code is just hanging out there without being wired up
anywhere).

Ok, no problem here. Will squash relevant patches then

-Daniel

-Daniel

---
   drivers/gpu/drm/xen/xen_drm_front.c | 327 
+++-
   drivers/gpu/drm/xen/xen_drm_front.h |   5 +
   2 files changed, 327 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/xen/xen_drm_front.c 
b/drivers/gpu/drm/xen/xen_drm_front.c
index 8de88e359d5e..5ad546231d30 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -31,12 +31,146 @@
   #include "xen_drm_front_evtchnl.h"
   #include "xen_drm_front_shbuf.h"
+/* timeout in ms to wait for backend to respond */
+#define VDRM_WAIT_BACK_MS  3000
+
+struct xen_drm_front_dbuf {
+   struct list_head list;
+   uint64_t dbuf_cookie;
+   uint64_t fb_cookie;
+   struct xen_drm_front_shbuf *shbuf;
+};
+
+static int dbuf_add_to_list(struct xen_drm_front_info *front_info,
+   struct xen_drm_front_shbuf *shbuf, uint64_t dbuf_cookie)
+{
+   struct xen_drm_front_dbuf *dbuf;
+
+   dbuf = kzalloc(sizeof(*dbuf), GFP_KERNEL);
+   if (!dbuf)
+   return -ENOMEM;
+
+   dbuf->dbuf_cookie = dbuf_cookie;
+   dbuf->shbuf = shbuf;
+   list_add(&dbuf->list, &front_info->dbuf_list);
+   return 0;
+}
+
+static struct xen_drm_front_dbuf *dbuf_get(struct list_head *dbuf_list,
+   uint64_t dbuf_cookie)
+{
+   struct xen_drm_front_dbuf *buf, *q;
+
+   list_for_each_entry_safe(buf, q, dbuf_list, list)
+   if (buf->dbuf_cookie == dbuf_cookie)
+   return buf;
+
+   return NULL;
+}
+
+static void dbuf_flush_fb(struct list_head *dbuf_list, uint64_t fb_cookie)
+{
+   struct xen_drm_front_dbuf *buf, *q;
+
+   list_for_each_entry_safe(buf, q, dbuf_list, list)
+   if (buf->fb_cookie == fb_cookie)
+   xen_drm_front_shbuf_flush(buf->shbuf);
+}
+
+static void dbuf_free(struct list_head *dbuf_list, uint64_t dbuf_cookie)
+{
+   struct xen_drm_front_dbuf *buf, *q;
+
+   list_for_each_entry_safe(buf, q, dbuf_list, list)
+   if (buf->dbuf_cookie == dbuf_cookie) {
+   list_del(&buf->list);
+   xen_drm_front_shbuf_unmap(buf->shbuf);
+   xen_drm_front_shbuf_free(buf->shbuf);
+   kfree(buf);
+   break;
+   }
+}
+
+static void dbuf_free_all(struct list_head *dbuf_list)
+{
+   struct xen_drm_front_dbuf *buf, *q;
+
+   list_for_each_entry_safe(buf, q, dbuf_list, list) {
+   list_del(&buf->list);
+   xen_drm_front_shbuf_unmap(buf->shbuf);
+   xen_drm_front_shbuf_free(buf->shbuf);
+   kfree(buf);
+   }
+}
+
+static struct xendispl_req *be_prepare_req(
+   struct xen_drm_front_evtchnl *evtchnl, uint8_t operation)
+{
+   struct xendispl_req *req;
+
+   req = RING_GET_REQUEST(&evtchnl->u.req.ring,
+   evtchnl->u.req.ring.req_prod_pvt);
+   req->operation = operation;
+   req->id = evtchnl->evt_next_id++;
+   evtchnl->evt_id = req->id;
+   return req;
+}
+
+static int be_stream_do_io(struct xen_drm_front_evtchnl *evtc

[PATCH 2/2] crypto: introduce SM4 testmgr tests

[Gilad Ben-Yossef]

Add testmgr tests for the newly introduced SM4 ECB symmetric cipher.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c  |   3 ++
 crypto/testmgr.c |   9 
 crypto/testmgr.h | 131 +++
 3 files changed, 143 insertions(+)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 14213a0..51fe7c8 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1983,6 +1983,9 @@ static int do_test(const char *alg, u32 type, u32 mask, 
int m)
case 190:
ret += tcrypt_test("authenc(hmac(sha512),cbc(des3_ede))");
break;
+   case 191:
+   ret += tcrypt_test("ecb(sm4)");
+   break;
case 200:
test_cipher_speed("ecb(aes)", ENCRYPT, sec, NULL, 0,
speed_template_16_24_32);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 9f82e7b..af4a01c 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -3001,6 +3001,15 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}
}, {
+   .alg = "ecb(sm4)",
+   .test = alg_test_skcipher,
+   .suite = {
+   .cipher = {
+   .enc = __VECS(sm4_enc_tv_template),
+   .dec = __VECS(sm4_dec_tv_template)
+   }
+   }
+   }, {
.alg = "ecb(speck128)",
.test = alg_test_skcipher,
.suite = {
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 73ba22c..c80ed56 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -14324,6 +14324,137 @@ static const struct cipher_testvec 
serpent_xts_dec_tv_template[] = {
 };
 
 /*
+ * SM4 test vector taken from the draft RFC
+ * https://tools.ietf.org/html/draft-crypto-sm4-00#ref-GBT.32907-2016
+ */
+
+static const struct cipher_testvec sm4_enc_tv_template[] = {
+   { /* SM4 Appendix A: Example Calculations. Example 1. */
+   .key= "\x01\x23\x45\x67\x89\xAB\xCD\xEF"
+ "\xFE\xDC\xBA\x98\x76\x54\x32\x10",
+   .klen   = 16,
+   .input  = "\x01\x23\x45\x67\x89\xAB\xCD\xEF"
+ "\xFE\xDC\xBA\x98\x76\x54\x32\x10",
+   .ilen   = 16,
+   .result = "\x68\x1E\xDF\x34\xD2\x06\x96\x5E"
+ "\x86\xB3\xE9\x4F\x53\x6E\x42\x46",
+   .rlen   = 16,
+   }, { /*
+ *  SM4 Appendix A: Example Calculations.
+ *  Last 10 iterations of Example 2.
+ */
+   .key= "\x01\x23\x45\x67\x89\xAB\xCD\xEF"
+ "\xFE\xDC\xBA\x98\x76\x54\x32\x10",
+   .klen   = 16,
+   .input  = "\x99\x4a\xc3\xe7\xc3\x57\x89\x6a"
+ "\x81\xfc\xa8\xe\x38\x3e\xef\x80"
+ "\xb1\x98\xf2\xde\x3f\x4b\xae\xd1"
+ "\xf0\xf1\x30\x4c\x1\x27\x5a\x8f"
+ "\x45\xe1\x39\xb7\xae\xff\x1f\x27"
+ "\xad\x57\x15\xab\x31\x5d\xc\xef"
+ "\x8c\xc8\x80\xbd\x11\x98\xf3\x7b"
+ "\xa2\xdd\x14\x20\xf9\xe8\xbb\x82"
+ "\xf7\x32\xca\x4b\xa8\xf7\xb3\x4d"
+ "\x27\xd1\xcd\xe6\xb6\x65\x5a\x23"
+ "\xc2\xf3\x54\x84\x53\xe3\xb9\x20"
+ "\xa5\x37\x0\xbe\xe7\x7b\x48\xfb"
+ "\x21\x3d\x9e\x48\x1d\x9e\xf5\xbf"
+ "\x77\xd5\xb4\x4a\x53\x71\x94\x7a"
+ "\x88\xa6\x6e\x6\x93\xca\x43\xa5"
+ "\xc4\xf6\xcd\x53\x4b\x7b\x8e\xfe"
+ "\xb4\x28\x7c\x42\x29\x32\x5d\x88"
+ "\xed\xce\x0\x19\xe\x16\x2\x6e"
+ "\x87\xff\x2c\xac\xe8\xe7\xe9\xbf"
+ "\x31\x51\xec\x47\xc3\x51\x83\xc1",
+   .ilen   = 160,
+   .result = "\xb1\x98\xf2\xde\x3f\x4b\xae\xd1"
+ "\xf0\xf1\x30\x4c\x1\x27\x5a\x8f"
+ "\x45\xe1\x39\xb7\xae\xff\x1f\x27"
+ "\xad\x57\x15\xab\x31\x5d\xc\xef"
+ "\x8c\xc8\x80\xbd\x11\x98\xf3\x7b"
+ "\xa2\xdd\x14\x20\xf9\xe8\xbb\x82"
+ "\xf7\x32\xca\x4b\xa8\xf7\xb3\x4d"
+ "\x27\xd1\xcd\xe6\xb6\x65\x5a\x23"
+ "\xc2\xf3\x54\x84\x53\xe3\xb9\x20"
+ "\xa5\x37\x0\xbe\xe7\x7b\x48\xfb"
+ "\x21\x3d\x9e\x48\x1d\x9e\xf5\xbf"
+ "\x77\xd5\xb4\x4a\x53\x71\x94\x7a"
+ "\x88\xa6\x6e\x6\x93\xca\x43\xa5"
+ "\xc4\xf6\xcd\x53\x4b\x7b\x8e\xfe"
+ "\xb4\x28\x7c\x42\x29\x32\x5d\x88"
+ "\xed\xce\x0\x19\xe\x16\x2\x6e"
+  

Re: [PATCH 2/4] staging: most: Replace calls to BUG_ON() with WARN_ONCE() and return.

[Dan Carpenter]

On Tue, Mar 06, 2018 at 01:23:18AM -0800, Quytelda Kahja wrote:
> > Are you sure this will work?
> Well, my goal was just to replace the code that could crash the kernel
> and let somebody with a better understanding of this particular driver
> write the recovery code, if necessary.  It seemed from context that
> the BUG_ON() calls were being used like assert() though, so I assumed
> there wasn't really much recovery to be made from that problem.  If
> you feel this doesn't improve the behavior of the driver, just drop
> the patch.
> 

mbo is always non-NULL just from a quick glance.  I didn't look hard
enough to verify that mbo->context was OK.

It's generally best to just check the callers and delete these.

Say you have a BUG_ON() then that prevents memory corruption (not an
issue here) but it makes debugging hard.  But if you just have a NULL
dereference it probably kills the driver but you get an Oops which you
can debug.  So a NULL dereference is normally better than a BUG_ON().

regards,
dan carpenter

[PATCH] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio

[Tomas Winkler]

ufshcd_dump_regs should use memcpy_fromio to read host registers
instead of directly accessing using memcpy.
The same function is utilized in ufs-qcom.

Elminite compilation warning
drivers/scsi/ufs/ufshcd.c:356:9: warning: incorrect type in argument 6 
(different address spaces)
drivers/scsi/ufs/ufshcd.c:356:9:expected void const *buf
drivers/scsi/ufs/ufshcd.c:356:9:got void [noderef] *mmio_base

Signed-off-by: Tomas Winkler 
---
 drivers/scsi/ufs/ufs-qcom.c | 21 ++---
 drivers/scsi/ufs/ufshcd.c   | 35 ---
 drivers/scsi/ufs/ufshcd.h   |  3 +++
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2eeafa..77ac98ea80d4 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -50,19 +50,10 @@ static void ufs_qcom_get_default_testbus_cfg(struct 
ufs_qcom_host *host);
 static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
   u32 clk_cycles);
 
-static void ufs_qcom_dump_regs(struct ufs_hba *hba, int offset, int len,
-   char *prefix)
-{
-   print_hex_dump(KERN_ERR, prefix,
-   len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,
-   16, 4, (void __force *)hba->mmio_base + offset,
-   len * 4, false);
-}
-
 static void ufs_qcom_dump_regs_wrapper(struct ufs_hba *hba, int offset, int 
len,
-   char *prefix, void *priv)
+  const char *prefix, void *priv)
 {
-   ufs_qcom_dump_regs(hba, offset, len, prefix);
+   ufshcd_dump_regs(hba, offset, len * 4, prefix);
 }
 
 static int ufs_qcom_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
@@ -1431,7 +1422,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
 
 static void ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba,
void *priv, void (*print_fn)(struct ufs_hba *hba,
-   int offset, int num_regs, char *str, void *priv))
+   int offset, int num_regs, const char *str, void *priv))
 {
u32 reg;
struct ufs_qcom_host *host;
@@ -1613,7 +1604,7 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
 
 static void ufs_qcom_testbus_read(struct ufs_hba *hba)
 {
-   ufs_qcom_dump_regs(hba, UFS_TEST_BUS, 1, "UFS_TEST_BUS ");
+   ufshcd_dump_regs(hba, UFS_TEST_BUS, 4, "UFS_TEST_BUS ");
 }
 
 static void ufs_qcom_print_unipro_testbus(struct ufs_hba *hba)
@@ -1639,8 +1630,8 @@ static void ufs_qcom_print_unipro_testbus(struct ufs_hba 
*hba)
 
 static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)
 {
-   ufs_qcom_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16,
-   "HCI Vendor Specific Registers ");
+   ufshcd_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16 * 4,
+"HCI Vendor Specific Registers ");
 
/* sleep a bit intermittently as we are dumping too much data */
ufs_qcom_print_hw_debug_reg_all(hba, NULL, ufs_qcom_dump_regs_wrapper);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 3abcd31646eb..f2e1da77045c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -98,8 +98,29 @@
_ret;   \
})
 
-#define ufshcd_hex_dump(prefix_str, buf, len) \
-print_hex_dump(KERN_ERR, prefix_str, DUMP_PREFIX_OFFSET, 16, 4, buf, len, 
false)
+#define ufshcd_hex_dump(prefix_str, buf, len) do {   \
+   size_t __len = (len);\
+   print_hex_dump(KERN_ERR, prefix_str, \
+  __len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,\
+  16, 4, buf, __len, false);\
+} while (0)
+
+int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
+const char *prefix)
+{
+   u8 *regs;
+
+   regs = kzalloc(len, GFP_KERNEL);
+   if (!regs)
+   return -ENOMEM;
+
+   memcpy_fromio(regs, hba->mmio_base + offset, len);
+   ufshcd_hex_dump(prefix, regs, len);
+   kfree(regs);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dump_regs);
 
 enum {
UFSHCD_MAX_CHANNEL  = 0,
@@ -343,15 +364,7 @@ static void ufshcd_print_uic_err_hist(struct ufs_hba *hba,
 
 static void ufshcd_print_host_regs(struct ufs_hba *hba)
 {
-   /*
-* hex_dump reads its data without the readl macro. This might
-* cause inconsistency issues on some platform, as the printed
-* values may be from cache and not the most recent value.
-* To know whether you are looking at an un-cached version verify
-* that IORESOURCE_MEM flag is on when xxx_get_resource() is invoked
-* during platform/pci probe function.
-*/
-   ufshcd_hex_dump("host regs: ", hba->mmio_base, UFSHCI_R

[PATCH 6/9] x86/dumpstack: Add loglevel argument to show_opcodes()

[Borislav Petkov]

From: Borislav Petkov 

Will be used in the next patch.

Signed-off-by: Borislav Petkov 
---
 arch/x86/include/asm/stacktrace.h | 1 +
 arch/x86/kernel/dumpstack.c   | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/stacktrace.h 
b/arch/x86/include/asm/stacktrace.h
index 133d9425fced..0630eeb18bbc 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -111,4 +111,5 @@ static inline unsigned long caller_frame_pointer(void)
return (unsigned long)frame;
 }
 
+void show_opcodes(u8 *rip, const char *loglvl);
 #endif /* _ASM_X86_STACKTRACE_H */
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 12ddfc9dcb01..3625f79fbb42 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -73,13 +73,13 @@ static void printk_stack_address(unsigned long address, int 
reliable,
printk("%s %s%pB\n", log_lvl, reliable ? "" : "? ", (void *)address);
 }
 
-static void show_opcodes(u8 *rip)
+void show_opcodes(u8 *rip, const char *loglvl)
 {
unsigned int code_prologue = code_bytes * 43 / OPCODE_BUFSIZE;
u8 *ip;
int i;
 
-   printk(KERN_DEFAULT "Code: ");
+   printk("%sCode: ", loglvl);
 
ip = (u8 *)rip - code_prologue;
if (probe_kernel_read(opcodes, ip, code_bytes)) {
@@ -432,6 +432,6 @@ void show_regs(struct pt_regs *regs)
if (regs->ip < PAGE_OFFSET)
pr_cont(" Bad RIP value.\n");
else
-   show_opcodes((u8 *)regs->ip);
+   show_opcodes((u8 *)regs->ip, KERN_DEFAULT);
}
 }
-- 
2.13.0

Re: [PATCH] kconfig: remove redundant streamline_config.pl prerequisite

[Masahiro Yamada]

2018-03-02 0:23 GMT+09:00 Ulf Magnusson :
> On Thu, Mar 1, 2018 at 3:39 PM, Masahiro Yamada
>  wrote:
>> 2018-03-01 20:18 GMT+09:00 Ulf Magnusson :
>>> The local{yes,mod}config targets currently have streamline_config.pl as
>>> a prerequisite. This is redundant, because streamline_config.pl is a
>>> checked-in file with no prerequisites.
>>>
>>> Remove the prerequisite and reference streamline_config.pl directly in
>>> the recipe of the rule instead.
>>>
>>> Signed-off-by: Ulf Magnusson 
>>> ---
>>>  scripts/kconfig/Makefile | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>>> index 1f74336d4e23..58be52cb464d 100644
>>
>>
>> Thanks! Almost good.
>>
>> Just small nits.
>>
>>
>>> --- a/scripts/kconfig/Makefile
>>> +++ b/scripts/kconfig/Makefile
>>> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf
>>> touch   include/generated/autoksyms.h
>>> $< $(silent) --$@ $(Kconfig)
>>>
>>> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
>>> +localyesconfig localmodconfig: $(obj)/conf
>>> $(Q)mkdir -p include/config include/generated
>>> -   $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config
>>> +   $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > 
>>> .tmp.config
>>
>>
>>
>> '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl'
>> since it is a checked-in file.
>>
>> '$(src)' and '$(obj)' are always the same.
>> (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6)
>>
>>
>> So, there is no effective difference.
>> It is just a coding convention to use $(obj)/ for generated files,
>> and $(src)/ for source files.
>>
>> The original code already used $(obj)/, so this is not your fault
>> but I want to fix it while we are here.
>>
>>
>>
>> One more nit.
>>
>> $(obj)/conf $(silent) --silentoldconfig $(Kconfig);
>>
>> can be
>>
>> $< $(silent) --silentoldconfig $(Kconfig);
>>
>>
>> I guess you did not touch this line to avoid
>> conflict with my patch.
>
> Yep, plus I wanted to keep the patch focused.




Applied to linux-kbuild/kconfig.  Thanks!


I changed

[1]  $(obj)/streamline_config.pl
-> $(srctree)/$(src)/streamline_config.pl

Mandatory change to avoid out-of-tree build error

[2]  $(obj)/conf   ->  $<

Clean-up




-- 
Best Regards
Masahiro Yamada

[PATCH 2/9] x86/fault: Do not print IP in show_fault_oops()

[Borislav Petkov]

From: Borislav Petkov 

... because __show_regs() already does that.

Signed-off-by: Borislav Petkov 
---
 arch/x86/mm/fault.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index e6af2b464c3d..26865147a507 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -699,7 +699,6 @@ show_fault_oops(struct pt_regs *regs, unsigned long 
error_code,
printk(KERN_CONT "paging request");
 
printk(KERN_CONT " at %px\n", (void *) address);
-   printk(KERN_ALERT "IP: %pS\n", (void *)regs->ip);
 
dump_pagetable(address);
 }
-- 
2.13.0

Re: uprobes/perf: KASAN: use-after-free in uprobe_perf_close

[Prashant Bhole]

On 2/23/2018 2:40 AM, Oleg Nesterov wrote:

On 02/22, Peter Zijlstra wrote:


On Thu, Feb 22, 2018 at 06:04:27PM +0100, Peter Zijlstra wrote:

On Thu, Feb 22, 2018 at 05:37:15PM +0100, Oleg Nesterov wrote:



This all makes me think that we should change (fix) kernel/events/core.c...


That's going to be mighty dodgy though, holding a reference on the task
will avoid the task from dying which will avoid the events from being
destroyed which will avoid the task from dying which will... if you get
my drift :-)


Hmm, it might not be all that bad.. I need to re-read some of that code.


I was thinking about the change below below. I do not think this patch is 
actually
correct/complete, but it seems to me that if perf_event_exit_task_context() does
put_task_struct(current) then put_ctx()->put_task_struct() should go away, every
user of ctx->task should check TASK_TOMBSTONE anyway?

Oleg.

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1165,8 +1165,6 @@ static void put_ctx(struct perf_event_context *ctx)
if (atomic_dec_and_test(&ctx->refcount)) {
if (ctx->parent_ctx)
put_ctx(ctx->parent_ctx);
-   if (ctx->task && ctx->task != TASK_TOMBSTONE)
-   put_task_struct(ctx->task);
call_rcu(&ctx->rcu_head, free_ctx);
}
  }
@@ -3731,10 +3729,9 @@ alloc_perf_context(struct pmu *pmu, struct task_struct 
*task)
return NULL;
  
  	__perf_event_init_context(ctx);

-   if (task) {
+   if (task)
ctx->task = task;
-   get_task_struct(task);
-   }
+
ctx->pmu = pmu;
  
  	return ctx;

@@ -4109,6 +4106,8 @@ static void _free_event(struct perf_event *event)
  
  	if (event->ctx)

put_ctx(event->ctx);
+   if (event->hw.target)
+   put_task_struct(event->hw.target);
  
  	exclusive_event_destroy(event);

module_put(event->pmu->module);
@@ -9475,6 +9474,7 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 * and we cannot use the ctx information because we need the
 * pmu before we get a ctx.
 */
+   get_task_struct(task);
event->hw.target = task;
}
  
@@ -9590,6 +9590,8 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,

perf_detach_cgroup(event);
if (event->ns)
put_pid_ns(event->ns);
+   if (task)
+   put_task_struct(task);
kfree(event);
  
  	return ERR_PTR(err);

@@ -10572,7 +10574,6 @@ static void perf_event_exit_task_context(struct 
task_struct *child, int ctxn)
RCU_INIT_POINTER(child->perf_event_ctxp[ctxn], NULL);
put_ctx(child_ctx); /* cannot be last */
WRITE_ONCE(child_ctx->task, TASK_TOMBSTONE);
-   put_task_struct(current); /* cannot be last */
  
  	clone_ctx = unclone_ctx(child_ctx);

raw_spin_unlock_irq(&child_ctx->lock);

Sorry for late reply. I tried these changes. It didn't fix the problem. 
With these changes, the use-after-free access of task_struct occurs at 
_free_event() for the last remaining event.


In your changes, I tried keeping get/put_task_struct() in 
perf_alloc_context()/put_ctx() intact and The problem did not occur. 
Changes are mentioned below.


-Prashant

diff --git a/kernel/events/core.c b/kernel/events/core.c
index c98cce4ceebd..65889d2b5ae2 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4109,6 +4109,8 @@ static void _free_event(struct perf_event *event)

if (event->ctx)
put_ctx(event->ctx);
+   if (event->hw.target)
+   put_task_struct(event->hw.target);

exclusive_event_destroy(event);
module_put(event->pmu->module);
@@ -9593,6 +9595,7 @@ perf_event_alloc(struct perf_event_attr *attr, int 
cpu,

 * and we cannot use the ctx information because we need the
 * pmu before we get a ctx.
 */
+   get_task_struct(task);
event->hw.target = task;
}

@@ -9708,6 +9711,8 @@ perf_event_alloc(struct perf_event_attr *attr, int 
cpu,

perf_detach_cgroup(event);
if (event->ns)
put_pid_ns(event->ns);
+   if (task)
+   put_task_struct(task);
kfree(event);

return ERR_PTR(err);