[PATCH v7 2/6] secure_seq: use SipHash in place of MD5

[Jason A. Donenfeld]

This gives a clear speed and security improvement. Siphash is both
faster and is more solid crypto than the aging MD5.

Rather than manually filling MD5 buffers, for IPv6, we simply create
a layout by a simple anonymous struct, for which gcc generates
rather efficient code. For IPv4, we pass the values directly to the
short input convenience functions.

64-bit x86_64:
[1.683628] secure_tcpv6_sequence_number_md5# cycles: 99563527
[1.717350] secure_tcp_sequence_number_md5# cycles: 92890502
[1.741968] secure_tcpv6_sequence_number_siphash# cycles: 67825362
[1.762048] secure_tcp_sequence_number_siphash# cycles: 67485526

32-bit x86:
[1.600012] secure_tcpv6_sequence_number_md5# cycles: 103227892
[1.634219] secure_tcp_sequence_number_md5# cycles: 94732544
[1.669102] secure_tcpv6_sequence_number_siphash# cycles: 96299384
[1.700165] secure_tcp_sequence_number_siphash# cycles: 86015473

Signed-off-by: Jason A. Donenfeld 
Cc: Andi Kleen 
Cc: David Miller 
Cc: David Laight 
Cc: Tom Herbert 
Cc: Hannes Frederic Sowa 
Cc: Eric Dumazet 
---
 net/core/secure_seq.c | 135 --
 1 file changed, 54 insertions(+), 81 deletions(-)

diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 88a8e429fc3e..3dc2689bcc64 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -1,3 +1,5 @@
+/* Copyright (C) 2016 Jason A. Donenfeld . All Rights 
Reserved. */
+
 #include 
 #include 
 #include 
@@ -8,14 +10,14 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 
 #if IS_ENABLED(CONFIG_IPV6) || IS_ENABLED(CONFIG_INET)
+#include 
 #include 
-#define NET_SECRET_SIZE (MD5_MESSAGE_BYTES / 4)
 
-static u32 net_secret[NET_SECRET_SIZE] cacheline_aligned;
+static siphash_key_t net_secret;
 
 static __always_inline void net_secret_init(void)
 {
@@ -44,80 +46,65 @@ static u32 seq_scale(u32 seq)
 u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
 __be16 sport, __be16 dport, u32 *tsoff)
 {
-   u32 secret[MD5_MESSAGE_BYTES / 4];
-   u32 hash[MD5_DIGEST_WORDS];
-   u32 i;
-
+   const struct {
+   struct in6_addr saddr;
+   struct in6_addr daddr;
+   __be16 sport;
+   __be16 dport;
+   } __aligned(SIPHASH_ALIGNMENT) combined = {
+   .saddr = *(struct in6_addr *)saddr,
+   .daddr = *(struct in6_addr *)daddr,
+   .sport = sport,
+   .dport = dport
+   };
+   u64 hash;
net_secret_init();
-   memcpy(hash, saddr, 16);
-   for (i = 0; i < 4; i++)
-   secret[i] = net_secret[i] + (__force u32)daddr[i];
-   secret[4] = net_secret[4] +
-   (((__force u16)sport << 16) + (__force u16)dport);
-   for (i = 5; i < MD5_MESSAGE_BYTES / 4; i++)
-   secret[i] = net_secret[i];
-
-   md5_transform(hash, secret);
-
-   *tsoff = sysctl_tcp_timestamps == 1 ? hash[1] : 0;
-   return seq_scale(hash[0]);
+   hash = siphash(, offsetofend(typeof(combined), dport), 
net_secret);
+   *tsoff = sysctl_tcp_timestamps == 1 ? (hash >> 32) : 0;
+   return seq_scale(hash);
 }
 EXPORT_SYMBOL(secure_tcpv6_sequence_number);
 
 u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
   __be16 dport)
 {
-   u32 secret[MD5_MESSAGE_BYTES / 4];
-   u32 hash[MD5_DIGEST_WORDS];
-   u32 i;
-
+   const struct {
+   struct in6_addr saddr;
+   struct in6_addr daddr;
+   __be16 dport;
+   } __aligned(SIPHASH_ALIGNMENT) combined = {
+   .saddr = *(struct in6_addr *)saddr,
+   .daddr = *(struct in6_addr *)daddr,
+   .dport = dport
+   };
net_secret_init();
-   memcpy(hash, saddr, 16);
-   for (i = 0; i < 4; i++)
-   secret[i] = net_secret[i] + (__force u32) daddr[i];
-   secret[4] = net_secret[4] + (__force u32)dport;
-   for (i = 5; i < MD5_MESSAGE_BYTES / 4; i++)
-   secret[i] = net_secret[i];
-
-   md5_transform(hash, secret);
-
-   return hash[0];
+   return siphash(, offsetofend(typeof(combined), dport), 
net_secret);
 }
 EXPORT_SYMBOL(secure_ipv6_port_ephemeral);
 #endif
 
 #ifdef CONFIG_INET
 
+/* secure_tcp_sequence_number(a, b, 0, d) == secure_ipv4_port_ephemeral(a, b, 
d),
+ * but fortunately, `sport' cannot be 0 in any circumstances. If this changes,
+ * it would be easy enough to have the former function use siphash_4u32, 
passing
+ * the arguments as separate u32.
+ */
+
 u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
   __be16 sport, __be16 dport, u32 *tsoff)
 {
-   u32 hash[MD5_DIGEST_WORDS];
-
+   u64 hash;
 

[PATCH v7 1/6] siphash: add cryptographically secure PRF

[Jason A. Donenfeld]

SipHash is a 64-bit keyed hash function that is actually a
cryptographically secure PRF, like HMAC. Except SipHash is super fast,
and is meant to be used as a hashtable keyed lookup function, or as a
general PRF for short input use cases, such as sequence numbers or RNG
chaining.

For the first usage:

There are a variety of attacks known as "hashtable poisoning" in which an
attacker forms some data such that the hash of that data will be the
same, and then preceeds to fill up all entries of a hashbucket. This is
a realistic and well-known denial-of-service vector. Currently
hashtables use jhash, which is fast but not secure, and some kind of
rotating key scheme (or none at all, which isn't good). SipHash is meant
as a replacement for jhash in these cases.

There are a modicum of places in the kernel that are vulnerable to
hashtable poisoning attacks, either via userspace vectors or network
vectors, and there's not a reliable mechanism inside the kernel at the
moment to fix it. The first step toward fixing these issues is actually
getting a secure primitive into the kernel for developers to use. Then
we can, bit by bit, port things over to it as deemed appropriate.

While SipHash is extremely fast for a cryptographically secure function,
it is likely a bit slower than the insecure jhash, and so replacements
will be evaluated on a case-by-case basis based on whether or not the
difference in speed is negligible and whether or not the current jhash usage
poses a real security risk.

For the second usage:

A few places in the kernel are using MD5 or SHA1 for creating secure
sequence numbers, syn cookies, port numbers, or fast random numbers.
SipHash is a faster and more fitting, and more secure replacement for MD5
in those situations. Replacing MD5 and SHA1 with SipHash for these uses is
obvious and straight-forward, and so is submitted along with this patch
series. There shouldn't be much of a debate over its efficacy.

Dozens of languages are already using this internally for their hash
tables and PRFs. Some of the BSDs already use this in their kernels.
SipHash is a widely known high-speed solution to a widely known set of
problems, and it's time we catch-up.

Signed-off-by: Jason A. Donenfeld 
Cc: Jean-Philippe Aumasson 
Cc: Linus Torvalds 
Cc: Eric Biggers 
Cc: David Laight 
Cc: Eric Dumazet 
---
 Documentation/siphash.txt |  79 
 MAINTAINERS   |   7 ++
 include/linux/siphash.h   |  79 
 lib/Kconfig.debug |   6 +-
 lib/Makefile  |   5 +-
 lib/siphash.c | 232 ++
 lib/test_siphash.c| 119 
 7 files changed, 522 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/siphash.txt
 create mode 100644 include/linux/siphash.h
 create mode 100644 lib/siphash.c
 create mode 100644 lib/test_siphash.c

diff --git a/Documentation/siphash.txt b/Documentation/siphash.txt
new file mode 100644
index ..39ff7f0438e7
--- /dev/null
+++ b/Documentation/siphash.txt
@@ -0,0 +1,79 @@
+ SipHash - a short input PRF
+---
+Written by Jason A. Donenfeld 
+
+SipHash is a cryptographically secure PRF -- a keyed hash function -- that
+performs very well for short inputs, hence the name. It was designed by
+cryptographers Daniel J. Bernstein and Jean-Philippe Aumasson. It is intended
+as a replacement for some uses of: `jhash`, `md5_transform`, `sha_transform`,
+and so forth.
+
+SipHash takes a secret key filled with randomly generated numbers and either
+an input buffer or several input integers. It spits out an integer that is
+indistinguishable from random. You may then use that integer as part of secure
+sequence numbers, secure cookies, or mask it off for use in a hash table.
+
+1. Generating a key
+
+Keys should always be generated from a cryptographically secure source of
+random numbers, either using get_random_bytes or get_random_once:
+
+siphash_key_t key;
+get_random_bytes(key, sizeof(key));
+
+If you're not deriving your key from here, you're doing it wrong.
+
+2. Using the functions
+
+There are two variants of the function, one that takes a list of integers, and
+one that takes a buffer:
+
+u64 siphash(const void *data, size_t len, siphash_key_t key);
+
+And:
+
+u64 siphash_1u64(u64, siphash_key_t key);
+u64 siphash_2u64(u64, u64, siphash_key_t key);
+u64 siphash_3u64(u64, u64, u64, siphash_key_t key);
+u64 siphash_4u64(u64, u64, u64, u64, siphash_key_t key);
+u64 siphash_1u32(u32, siphash_key_t key);
+u64 siphash_2u32(u32, u32, siphash_key_t key);
+u64 siphash_3u32(u32, u32, u32, siphash_key_t key);
+u64 siphash_4u32(u32, u32, u32, u32, siphash_key_t key);
+
+If you pass the generic siphash function something of a constant length, it
+will constant fold at compile-time and automatically choose one of the
+optimized functions.
+
+3. Hashtable key function usage:
+
+struct some_hashtable {
+   

Re: [alsa-devel] [PATCH v6 1/3] clk: x86: Add Atom PMC platform clocks

[Stephen Boyd]

On 12/19, Pierre-Louis Bossart wrote:
> On 12/17/16 7:57 AM, Andy Shevchenko wrote:
> >On Sat, Dec 17, 2016 at 3:33 AM, Stephen Boyd  wrote:
> >>On 12/15, Pierre-Louis Bossart wrote:
> >
> >>>Clients use devm_clk_get() with a "pmc_plt_clk_"
> >>>argument.
> >>
> >>This is the problem. Clients should be calling clk_get() like:
> >>
> >>clk_get(dev, "signal name in datasheet")
> >>
> >>where the first argument is the device and the second argument is
> >>some string that is meaningful to the device, not the system as a
> >>whole. The way clkdev is intended is so that the dev argument's
> >>dev_name() is combined with the con_id that matches some signale
> >>name in the datasheet. This way when the same IP is put into some
> >>other chip, the globally unique name doesn't need to change, just
> >>the device name that's registered with the lookup. Obviously this
> >>breaks down quite badly when dev_name() isn't stable. Is that
> >>happening here?
> >
> >PMC Atom is a PCI device and thus each platform would have different
> >dev_name(). Do you want to list all in each consumer if consumer wants
> >to work on all of them or I missed something?
> >
> >So, the question is how clock getting will look like to work on
> >currently both CherryTrail and BayTrail.
> 
> The name pmc_plt_clk_ follows the data sheet specification, where
> this convention is suggested:
>   PLT_CLK[2:0] - Camera
>   PLT_CLK[3] - Audio Codec
>   PLT_CLK[4] -
>   PLT_CLK[5] - COMMs
> 
> These clocks are not internal but are made available to external
> components through dedicated physical pins on the package, this
> external visibility limits the scope for confusions, variations. I
> have not seen any skews where these clocks and pins were changed at
> all.

Ok, by clkdev design if a device is passed but there isn't a
match in the lookup table it allows it to match based solely on
the connection id. Given that the connection id is globally
unique this will work.

Hopefully we don't have two of these devices with pmc_plt_clk_
signals in a single system though. Then having the device name
would help differentiate between the two. And then it may make
sense to have some sort of ACPI lookup system, similar to how we
have lookups for clks in DT.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v7 2/6] secure_seq: use SipHash in place of MD5

[Jason A. Donenfeld]

This gives a clear speed and security improvement. Siphash is both
faster and is more solid crypto than the aging MD5.

Rather than manually filling MD5 buffers, for IPv6, we simply create
a layout by a simple anonymous struct, for which gcc generates
rather efficient code. For IPv4, we pass the values directly to the
short input convenience functions.

64-bit x86_64:
[1.683628] secure_tcpv6_sequence_number_md5# cycles: 99563527
[1.717350] secure_tcp_sequence_number_md5# cycles: 92890502
[1.741968] secure_tcpv6_sequence_number_siphash# cycles: 67825362
[1.762048] secure_tcp_sequence_number_siphash# cycles: 67485526

32-bit x86:
[1.600012] secure_tcpv6_sequence_number_md5# cycles: 103227892
[1.634219] secure_tcp_sequence_number_md5# cycles: 94732544
[1.669102] secure_tcpv6_sequence_number_siphash# cycles: 96299384
[1.700165] secure_tcp_sequence_number_siphash# cycles: 86015473

Signed-off-by: Jason A. Donenfeld 
Cc: Andi Kleen 
Cc: David Miller 
Cc: David Laight 
Cc: Tom Herbert 
Cc: Hannes Frederic Sowa 
Cc: Eric Dumazet 
---
 net/core/secure_seq.c | 135 --
 1 file changed, 54 insertions(+), 81 deletions(-)

diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 88a8e429fc3e..3dc2689bcc64 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -1,3 +1,5 @@
+/* Copyright (C) 2016 Jason A. Donenfeld . All Rights 
Reserved. */
+
 #include 
 #include 
 #include 
@@ -8,14 +10,14 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 
 #if IS_ENABLED(CONFIG_IPV6) || IS_ENABLED(CONFIG_INET)
+#include 
 #include 
-#define NET_SECRET_SIZE (MD5_MESSAGE_BYTES / 4)
 
-static u32 net_secret[NET_SECRET_SIZE] cacheline_aligned;
+static siphash_key_t net_secret;
 
 static __always_inline void net_secret_init(void)
 {
@@ -44,80 +46,65 @@ static u32 seq_scale(u32 seq)
 u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
 __be16 sport, __be16 dport, u32 *tsoff)
 {
-   u32 secret[MD5_MESSAGE_BYTES / 4];
-   u32 hash[MD5_DIGEST_WORDS];
-   u32 i;
-
+   const struct {
+   struct in6_addr saddr;
+   struct in6_addr daddr;
+   __be16 sport;
+   __be16 dport;
+   } __aligned(SIPHASH_ALIGNMENT) combined = {
+   .saddr = *(struct in6_addr *)saddr,
+   .daddr = *(struct in6_addr *)daddr,
+   .sport = sport,
+   .dport = dport
+   };
+   u64 hash;
net_secret_init();
-   memcpy(hash, saddr, 16);
-   for (i = 0; i < 4; i++)
-   secret[i] = net_secret[i] + (__force u32)daddr[i];
-   secret[4] = net_secret[4] +
-   (((__force u16)sport << 16) + (__force u16)dport);
-   for (i = 5; i < MD5_MESSAGE_BYTES / 4; i++)
-   secret[i] = net_secret[i];
-
-   md5_transform(hash, secret);
-
-   *tsoff = sysctl_tcp_timestamps == 1 ? hash[1] : 0;
-   return seq_scale(hash[0]);
+   hash = siphash(, offsetofend(typeof(combined), dport), 
net_secret);
+   *tsoff = sysctl_tcp_timestamps == 1 ? (hash >> 32) : 0;
+   return seq_scale(hash);
 }
 EXPORT_SYMBOL(secure_tcpv6_sequence_number);
 
 u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
   __be16 dport)
 {
-   u32 secret[MD5_MESSAGE_BYTES / 4];
-   u32 hash[MD5_DIGEST_WORDS];
-   u32 i;
-
+   const struct {
+   struct in6_addr saddr;
+   struct in6_addr daddr;
+   __be16 dport;
+   } __aligned(SIPHASH_ALIGNMENT) combined = {
+   .saddr = *(struct in6_addr *)saddr,
+   .daddr = *(struct in6_addr *)daddr,
+   .dport = dport
+   };
net_secret_init();
-   memcpy(hash, saddr, 16);
-   for (i = 0; i < 4; i++)
-   secret[i] = net_secret[i] + (__force u32) daddr[i];
-   secret[4] = net_secret[4] + (__force u32)dport;
-   for (i = 5; i < MD5_MESSAGE_BYTES / 4; i++)
-   secret[i] = net_secret[i];
-
-   md5_transform(hash, secret);
-
-   return hash[0];
+   return siphash(, offsetofend(typeof(combined), dport), 
net_secret);
 }
 EXPORT_SYMBOL(secure_ipv6_port_ephemeral);
 #endif
 
 #ifdef CONFIG_INET
 
+/* secure_tcp_sequence_number(a, b, 0, d) == secure_ipv4_port_ephemeral(a, b, 
d),
+ * but fortunately, `sport' cannot be 0 in any circumstances. If this changes,
+ * it would be easy enough to have the former function use siphash_4u32, 
passing
+ * the arguments as separate u32.
+ */
+
 u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
   __be16 sport, __be16 dport, u32 *tsoff)
 {
-   u32 hash[MD5_DIGEST_WORDS];
-
+   u64 hash;
net_secret_init();
-   hash[0] = (__force u32)saddr;
-   hash[1] = (__force u32)daddr;
-   hash[2] = ((__force u16)sport << 16) + (__force u16)dport;
-  

[PATCH 2/2 v2] ARM: i.MX25: add the optional security violation irq

[Martin Kaiser]

Add the security violation irq as an optional entry to rtc-imxdi's
device tree bindings and to i.MX25's dtsi file.

Signed-off-by: Martin Kaiser 
---
v2:
  - clarify that the security violation interrupt is optional
  - use the same syntax as everwhere else for lists of interrupt numbers

 Documentation/devicetree/bindings/rtc/imxdi-rtc.txt |5 -
 arch/arm/boot/dts/imx25.dtsi|2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt 
b/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
index c9d80d7..323cf26 100644
--- a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
@@ -8,10 +8,13 @@ Required properties:
   region.
 - interrupts: rtc alarm interrupt
 
+Optional properties:
+- interrupts: dryice security violation interrupt
+
 Example:
 
 rtc@80056000 {
compatible = "fsl,imx53-rtc", "fsl,imx25-rtc";
reg = <0x80056000 2000>;
-   interrupts = <29>;
+   interrupts = <29 56>;
 };
diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 831d09a..331d1e1 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -560,7 +560,7 @@
reg = <0x53ffc000 0x4000>;
clocks = < 81>;
clock-names = "ipg";
-   interrupts = <25>;
+   interrupts = <25 56>;
};
};
 
-- 
1.7.10.4

Re: [PATCH 2/2] net: wireless: fix to uses struct

[Ozgur Karatas]

My previous patch is invalid, I'm sorry.
The last patc will be fellow because "regulatory_request" is defined as a 
"static struct".

Signed-off-by: Ozgur Karatas 
---
 net/wireless/reg.c  | 10 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..5b70970 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -490,7 +490,7 @@ static int reg_query_builtin(const char *alpha2)
if (!regdom)
return -ENODATA;
 
-   request = kzalloc(sizeof(struct reg_regdb_apply_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*reg_regdb_apply_request), GFP_KERNEL);
if (!request)
return -ENOMEM;

@@ -2661,7 +2661,7 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
if (processing)
return 0;
 
-   reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
+   reg_beacon = kzalloc(sizeof(*reg_beacon), gfp);
if (!reg_beacon)
return -ENOMEM;
 
-- 
2.1.4

[PATCH 2/2 v2] ARM: i.MX25: add the optional security violation irq

[Martin Kaiser]

Add the security violation irq as an optional entry to rtc-imxdi's
device tree bindings and to i.MX25's dtsi file.

Signed-off-by: Martin Kaiser 
---
v2:
  - clarify that the security violation interrupt is optional
  - use the same syntax as everwhere else for lists of interrupt numbers

 Documentation/devicetree/bindings/rtc/imxdi-rtc.txt |5 -
 arch/arm/boot/dts/imx25.dtsi|2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt 
b/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
index c9d80d7..323cf26 100644
--- a/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/imxdi-rtc.txt
@@ -8,10 +8,13 @@ Required properties:
   region.
 - interrupts: rtc alarm interrupt
 
+Optional properties:
+- interrupts: dryice security violation interrupt
+
 Example:
 
 rtc@80056000 {
compatible = "fsl,imx53-rtc", "fsl,imx25-rtc";
reg = <0x80056000 2000>;
-   interrupts = <29>;
+   interrupts = <29 56>;
 };
diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 831d09a..331d1e1 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -560,7 +560,7 @@
reg = <0x53ffc000 0x4000>;
clocks = < 81>;
clock-names = "ipg";
-   interrupts = <25>;
+   interrupts = <25 56>;
};
};
 
-- 
1.7.10.4

Re: [PATCH 2/2] net: wireless: fix to uses struct

[Ozgur Karatas]

My previous patch is invalid, I'm sorry.
The last patc will be fellow because "regulatory_request" is defined as a 
"static struct".

Signed-off-by: Ozgur Karatas 
---
 net/wireless/reg.c  | 10 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..5b70970 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -490,7 +490,7 @@ static int reg_query_builtin(const char *alpha2)
if (!regdom)
return -ENODATA;
 
-   request = kzalloc(sizeof(struct reg_regdb_apply_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*reg_regdb_apply_request), GFP_KERNEL);
if (!request)
return -ENOMEM;

@@ -2661,7 +2661,7 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
if (processing)
return 0;
 
-   reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
+   reg_beacon = kzalloc(sizeof(*reg_beacon), gfp);
if (!reg_beacon)
return -ENOMEM;
 
-- 
2.1.4

Re: Inlined functions in perf report

[Steinar H. Gunderson]

On Thu, Dec 22, 2016 at 06:56:28AM +0800, Jin, Yao wrote:
> Could you see the inline if you use the addr2line command? For example,
> addr2line -e  -i 

I'm sorry, I don't have this profile anymore. I'll try again once we sort out
the problems of the DWARF error messages everywhere.

/* Steinar */
-- 
Homepage: https://www.sesse.net/

Re: Inlined functions in perf report

[Steinar H. Gunderson]

On Thu, Dec 22, 2016 at 06:56:28AM +0800, Jin, Yao wrote:
> Could you see the inline if you use the addr2line command? For example,
> addr2line -e  -i 

I'm sorry, I don't have this profile anymore. I'll try again once we sort out
the problems of the DWARF error messages everywhere.

/* Steinar */
-- 
Homepage: https://www.sesse.net/

[PATCH 1/2 v3] rtc: imxdi: use the security violation interrupt

[Martin Kaiser]

The DryIce chipset has a dedicated security violation interrupt that is
triggered for security violations (if configured to do so).  According
to the publicly available imx258 reference manual, irq 56 is used for
this interrupt.

If an irq number is provided for the security violation interrupt,
install the same handler that we're already using for the "normal"
interrupt.

imxdi->irq is used only in the probe function, make it a local variable.

Signed-off-by: Martin Kaiser 
---
v3:
  - add the device tree changes to this patch series
  - install the same interrupt handler for normal and sec irq, rename 
the handler function accordingly
  - make imxdi->irq and imxdi->sec_irq local variables in the probe 
function

v2:
  - make sec_irq optional to avoid breaking the Device Tree ABI
  - removed the Device Tree bindings, I'll prepare a separate patch for them

 drivers/rtc/rtc-imxdi.c |   28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 67b56b8..73d6908 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -108,7 +108,6 @@
  * @pdev: pionter to platform dev
  * @rtc: pointer to rtc struct
  * @ioaddr: IO registers pointer
- * @irq: dryice normal interrupt
  * @clk: input reference clock
  * @dsr: copy of the DSR register
  * @irq_lock: interrupt enable register (DIER) lock
@@ -120,7 +119,6 @@ struct imxdi_dev {
struct platform_device *pdev;
struct rtc_device *rtc;
void __iomem *ioaddr;
-   int irq;
struct clk *clk;
u32 dsr;
spinlock_t irq_lock;
@@ -677,9 +675,9 @@ static int dryice_rtc_set_alarm(struct device *dev, struct 
rtc_wkalrm *alarm)
 };
 
 /*
- * dryice "normal" interrupt handler
+ * interrupt handler for dryice "normal" and security violation interrupt
  */
-static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
+static irqreturn_t dryice_irq(int irq, void *dev_id)
 {
struct imxdi_dev *imxdi = dev_id;
u32 dsr, dier;
@@ -765,6 +763,7 @@ static int __init dryice_rtc_probe(struct platform_device 
*pdev)
 {
struct resource *res;
struct imxdi_dev *imxdi;
+   int norm_irq, sec_irq;
int rc;
 
imxdi = devm_kzalloc(>dev, sizeof(*imxdi), GFP_KERNEL);
@@ -780,9 +779,15 @@ static int __init dryice_rtc_probe(struct platform_device 
*pdev)
 
spin_lock_init(>irq_lock);
 
-   imxdi->irq = platform_get_irq(pdev, 0);
-   if (imxdi->irq < 0)
-   return imxdi->irq;
+   norm_irq = platform_get_irq(pdev, 0);
+   if (norm_irq < 0)
+   return norm_irq;
+
+   /* the 2nd irq is the security violation irq
+  make this optional, don't break the device tree ABI */
+   sec_irq = platform_get_irq(pdev, 1);
+   if (sec_irq <= 0)
+   sec_irq = IRQ_NOTCONNECTED;
 
init_waitqueue_head(>write_wait);
 
@@ -808,13 +813,20 @@ static int __init dryice_rtc_probe(struct platform_device 
*pdev)
if (rc != 0)
goto err;
 
-   rc = devm_request_irq(>dev, imxdi->irq, dryice_norm_irq,
+   rc = devm_request_irq(>dev, norm_irq, dryice_irq,
IRQF_SHARED, pdev->name, imxdi);
if (rc) {
dev_warn(>dev, "interrupt not available.\n");
goto err;
}
 
+   rc = devm_request_irq(>dev, sec_irq, dryice_irq,
+   IRQF_SHARED, pdev->name, imxdi);
+   if (rc) {
+   dev_warn(>dev, "security violation interrupt not 
available.\n");
+   /* this is not an error, see above */
+   }
+
platform_set_drvdata(pdev, imxdi);
imxdi->rtc = devm_rtc_device_register(>dev, pdev->name,
  _rtc_ops, THIS_MODULE);
-- 
1.7.10.4

Re: [PATCH 3/3] ARM: dts: sun5i: add support for Lichee Pi One board

[Maxime Ripard]

On Thu, Dec 22, 2016 at 04:02:35AM +0800, Icenowy Zheng wrote:
> Lichee Pi One is a low-cost Allwinner A13-based development board, with
> an AXP209 PMU, a USB2.0 OTG port, a USB2.0 host port (or an onboard
> RTL8723BU Wi-Fi card), optional headers for LCD and CSI, two GPIO
> headers and two MicroSD card slots (connected to mmc0 and mmc2, both
> bootable).
> 
> Add support for it.
> 
> Signed-off-by: Icenowy Zheng 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[PATCH 1/2 v3] rtc: imxdi: use the security violation interrupt

[Martin Kaiser]

The DryIce chipset has a dedicated security violation interrupt that is
triggered for security violations (if configured to do so).  According
to the publicly available imx258 reference manual, irq 56 is used for
this interrupt.

If an irq number is provided for the security violation interrupt,
install the same handler that we're already using for the "normal"
interrupt.

imxdi->irq is used only in the probe function, make it a local variable.

Signed-off-by: Martin Kaiser 
---
v3:
  - add the device tree changes to this patch series
  - install the same interrupt handler for normal and sec irq, rename 
the handler function accordingly
  - make imxdi->irq and imxdi->sec_irq local variables in the probe 
function

v2:
  - make sec_irq optional to avoid breaking the Device Tree ABI
  - removed the Device Tree bindings, I'll prepare a separate patch for them

 drivers/rtc/rtc-imxdi.c |   28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 67b56b8..73d6908 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -108,7 +108,6 @@
  * @pdev: pionter to platform dev
  * @rtc: pointer to rtc struct
  * @ioaddr: IO registers pointer
- * @irq: dryice normal interrupt
  * @clk: input reference clock
  * @dsr: copy of the DSR register
  * @irq_lock: interrupt enable register (DIER) lock
@@ -120,7 +119,6 @@ struct imxdi_dev {
struct platform_device *pdev;
struct rtc_device *rtc;
void __iomem *ioaddr;
-   int irq;
struct clk *clk;
u32 dsr;
spinlock_t irq_lock;
@@ -677,9 +675,9 @@ static int dryice_rtc_set_alarm(struct device *dev, struct 
rtc_wkalrm *alarm)
 };
 
 /*
- * dryice "normal" interrupt handler
+ * interrupt handler for dryice "normal" and security violation interrupt
  */
-static irqreturn_t dryice_norm_irq(int irq, void *dev_id)
+static irqreturn_t dryice_irq(int irq, void *dev_id)
 {
struct imxdi_dev *imxdi = dev_id;
u32 dsr, dier;
@@ -765,6 +763,7 @@ static int __init dryice_rtc_probe(struct platform_device 
*pdev)
 {
struct resource *res;
struct imxdi_dev *imxdi;
+   int norm_irq, sec_irq;
int rc;
 
imxdi = devm_kzalloc(>dev, sizeof(*imxdi), GFP_KERNEL);
@@ -780,9 +779,15 @@ static int __init dryice_rtc_probe(struct platform_device 
*pdev)
 
spin_lock_init(>irq_lock);
 
-   imxdi->irq = platform_get_irq(pdev, 0);
-   if (imxdi->irq < 0)
-   return imxdi->irq;
+   norm_irq = platform_get_irq(pdev, 0);
+   if (norm_irq < 0)
+   return norm_irq;
+
+   /* the 2nd irq is the security violation irq
+  make this optional, don't break the device tree ABI */
+   sec_irq = platform_get_irq(pdev, 1);
+   if (sec_irq <= 0)
+   sec_irq = IRQ_NOTCONNECTED;
 
init_waitqueue_head(>write_wait);
 
@@ -808,13 +813,20 @@ static int __init dryice_rtc_probe(struct platform_device 
*pdev)
if (rc != 0)
goto err;
 
-   rc = devm_request_irq(>dev, imxdi->irq, dryice_norm_irq,
+   rc = devm_request_irq(>dev, norm_irq, dryice_irq,
IRQF_SHARED, pdev->name, imxdi);
if (rc) {
dev_warn(>dev, "interrupt not available.\n");
goto err;
}
 
+   rc = devm_request_irq(>dev, sec_irq, dryice_irq,
+   IRQF_SHARED, pdev->name, imxdi);
+   if (rc) {
+   dev_warn(>dev, "security violation interrupt not 
available.\n");
+   /* this is not an error, see above */
+   }
+
platform_set_drvdata(pdev, imxdi);
imxdi->rtc = devm_rtc_device_register(>dev, pdev->name,
  _rtc_ops, THIS_MODULE);
-- 
1.7.10.4

Re: [PATCH 3/3] ARM: dts: sun5i: add support for Lichee Pi One board

[Maxime Ripard]

On Thu, Dec 22, 2016 at 04:02:35AM +0800, Icenowy Zheng wrote:
> Lichee Pi One is a low-cost Allwinner A13-based development board, with
> an AXP209 PMU, a USB2.0 OTG port, a USB2.0 host port (or an onboard
> RTL8723BU Wi-Fi card), optional headers for LCD and CSI, two GPIO
> headers and two MicroSD card slots (connected to mmc0 and mmc2, both
> bootable).
> 
> Add support for it.
> 
> Signed-off-by: Icenowy Zheng 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: Inlined functions in perf report

[Jin, Yao]

Could you see the inline if you use the addr2line command? For example, 
addr2line -e  -i 


For example, in my case,

root@skl:/home/jinyao/skl-ws/perf-dev/lck-2867/test# addr2line -e 
./test2 -i 40052d

/usr/include/x86_64-linux-gnu/bits/stdio2.h:104
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:27
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:35
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:45
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:61

004004f0 :

 ..

 40052d:   e8 6e ff ff ff  callq  4004a0 

Thanks

Jin Yao


On 12/21/2016 6:20 PM, Steinar H. Gunderson wrote:

On Wed, Dec 21, 2016 at 11:09:42AM +0100, Milian Wolff wrote:

Just to check - did you really compile your code with frame pointers? By
default, that is not the case, and the above will try to do frame pointer
unwinding which will then fail. Put differently - do you any stack frames at
all? Can you try `perf record --call-graph dwarf` instead? Of course, make
sure you compile your code with `-g -O2` or similar.

I don't specifically use -fno-omit-frame-pointer, no. But the normal stack
unwinding works just fine with mainline perf nevertheless; is this expected?

/* Steinar */

Re: Inlined functions in perf report

[Jin, Yao]

Could you see the inline if you use the addr2line command? For example, 
addr2line -e  -i 


For example, in my case,

root@skl:/home/jinyao/skl-ws/perf-dev/lck-2867/test# addr2line -e 
./test2 -i 40052d

/usr/include/x86_64-linux-gnu/bits/stdio2.h:104
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:27
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:35
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:45
/home/jinyao/skl-ws/perf-dev/lck-2867/test/test2.c:61

004004f0 :

 ..

 40052d:   e8 6e ff ff ff  callq  4004a0 

Thanks

Jin Yao


On 12/21/2016 6:20 PM, Steinar H. Gunderson wrote:

On Wed, Dec 21, 2016 at 11:09:42AM +0100, Milian Wolff wrote:

Just to check - did you really compile your code with frame pointers? By
default, that is not the case, and the above will try to do frame pointer
unwinding which will then fail. Put differently - do you any stack frames at
all? Can you try `perf record --call-graph dwarf` instead? Of course, make
sure you compile your code with `-g -O2` or similar.

I don't specifically use -fno-omit-frame-pointer, no. But the normal stack
unwinding works just fine with mainline perf nevertheless; is this expected?

/* Steinar */

Re: [PATCH 2/2] net: wireless: fix to uses struct

[Ozgur Karatas]

22.12.2016, 00:34, "Paul Bolle" :
> On Thu, 2016-12-22 at 00:23 +0200, Ozgur Karatas wrote:
>>  I compiled and didn't to errors.
>
> Really?

I'm very sorry. The "regulatory_request" is defined a static struct. I missed.

line: static struct regulatory_request core_request_world = {

I send to wrong line please can ignore last message and should be fix to as 
follows:

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..5b70970 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -490,7 +490,7 @@ static int reg_query_builtin(const char *alpha2)
if (!regdom)
return -ENODATA;
 
-   request = kzalloc(sizeof(struct reg_regdb_apply_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*reg_regdb_apply_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2291,7 +2291,7 @@ static int regulatory_hint_core(const char *alpha2)
 {
struct regulatory_request *request;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2313,7 +2313,7 @@ int regulatory_hint_user(const char *alpha2,
if (WARN_ON(!alpha2))
return -EINVAL;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2385,7 +2385,7 @@ int regulatory_hint(struct wiphy *wiphy, const char 
*alpha2)
 
wiphy->regulatory_flags &= ~REGULATORY_CUSTOM_REG;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2661,7 +2661,7 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
if (processing)
return 0;
 
-   reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
+   reg_beacon = kzalloc(sizeof(*reg_beacon), gfp);
if (!reg_beacon)
return -ENOMEM;
 
-- 
2.1.4

> $ make net/wireless/reg.o
>   CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CHK include/generated/bounds.h
>   CHK include/generated/timeconst.h
>   CHK include/generated/asm-offsets.h
>   CALL scripts/checksyscalls.sh
>   CC net/wireless/reg.o
> net/wireless/reg.c: In function ‘regulatory_hint_core’:
> net/wireless/reg.c:2294:28: error: ‘regulatory_request’ undeclared (first use 
> in this function)
>   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
> ^~
> net/wireless/reg.c:2294:28: note: each undeclared identifier is reported only 
> once for each function it appears in
> net/wireless/reg.c: In function ‘regulatory_hint_user’:
> net/wireless/reg.c:2316:28: error: ‘regulatory_request’ undeclared (first use 
> in this function)
>   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
> ^~
> net/wireless/reg.c: In function ‘regulatory_hint’:
> net/wireless/reg.c:2388:28: error: ‘regulatory_request’ undeclared (first use 
> in this function)
>   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
> ^~
> scripts/Makefile.build:293: recipe for target 'net/wireless/reg.o' failed
> make[1]: *** [net/wireless/reg.o] Error 1
> Makefile:1640: recipe for target 'net/wireless/reg.o' failed
> make: *** [net/wireless/reg.o] Error 2

$ make M=net/wireless/
  CC  net/wireless//core.o
  CC  net/wireless//sysfs.o
  CC  net/wireless//radiotap.o
  CC  net/wireless//util.o
  CC  net/wireless//reg.o
  CC  net/wireless//scan.o
  CC  net/wireless//nl80211.o
  CC  net/wireless//mlme.o
  CC  net/wireless//ibss.o
  CC  net/wireless//sme.o
  CC  net/wireless//chan.o
  CC  net/wireless//ethtool.o
  CC  net/wireless//mesh.o
  CC  net/wireless//ap.o
  CC  net/wireless//trace.o
  CC  net/wireless//ocb.o
  LD  net/wireless//cfg80211.o
  LD  net/wireless//built-in.o
  Building modules, stage 2.
  MODPOST 0 modules

$ make net/wireless/reg.o
scripts/kconfig/conf  --silentoldconfig Kconfig
*
* Restart config...
*
*
* Memory power savings
*

> Didn't Thomas Gleixner suggest that you do a basic C course just yesterday?

I don't have a problem with C programming, So only I'm learning the kernel. 
Also, this is a lie if say "I'm expert to C".

I think be re-learned every day. wrong?

> Paul Bolle

Regards,

~ Ozgur

Re: [PATCH 2/2] net: wireless: fix to uses struct

[Ozgur Karatas]

22.12.2016, 00:34, "Paul Bolle" :
> On Thu, 2016-12-22 at 00:23 +0200, Ozgur Karatas wrote:
>>  I compiled and didn't to errors.
>
> Really?

I'm very sorry. The "regulatory_request" is defined a static struct. I missed.

line: static struct regulatory_request core_request_world = {

I send to wrong line please can ignore last message and should be fix to as 
follows:

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..5b70970 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -490,7 +490,7 @@ static int reg_query_builtin(const char *alpha2)
if (!regdom)
return -ENODATA;
 
-   request = kzalloc(sizeof(struct reg_regdb_apply_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*reg_regdb_apply_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2291,7 +2291,7 @@ static int regulatory_hint_core(const char *alpha2)
 {
struct regulatory_request *request;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2313,7 +2313,7 @@ int regulatory_hint_user(const char *alpha2,
if (WARN_ON(!alpha2))
return -EINVAL;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2385,7 +2385,7 @@ int regulatory_hint(struct wiphy *wiphy, const char 
*alpha2)
 
wiphy->regulatory_flags &= ~REGULATORY_CUSTOM_REG;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2661,7 +2661,7 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
if (processing)
return 0;
 
-   reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
+   reg_beacon = kzalloc(sizeof(*reg_beacon), gfp);
if (!reg_beacon)
return -ENOMEM;
 
-- 
2.1.4

> $ make net/wireless/reg.o
>   CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CHK include/generated/bounds.h
>   CHK include/generated/timeconst.h
>   CHK include/generated/asm-offsets.h
>   CALL scripts/checksyscalls.sh
>   CC net/wireless/reg.o
> net/wireless/reg.c: In function ‘regulatory_hint_core’:
> net/wireless/reg.c:2294:28: error: ‘regulatory_request’ undeclared (first use 
> in this function)
>   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
> ^~
> net/wireless/reg.c:2294:28: note: each undeclared identifier is reported only 
> once for each function it appears in
> net/wireless/reg.c: In function ‘regulatory_hint_user’:
> net/wireless/reg.c:2316:28: error: ‘regulatory_request’ undeclared (first use 
> in this function)
>   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
> ^~
> net/wireless/reg.c: In function ‘regulatory_hint’:
> net/wireless/reg.c:2388:28: error: ‘regulatory_request’ undeclared (first use 
> in this function)
>   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
> ^~
> scripts/Makefile.build:293: recipe for target 'net/wireless/reg.o' failed
> make[1]: *** [net/wireless/reg.o] Error 1
> Makefile:1640: recipe for target 'net/wireless/reg.o' failed
> make: *** [net/wireless/reg.o] Error 2

$ make M=net/wireless/
  CC  net/wireless//core.o
  CC  net/wireless//sysfs.o
  CC  net/wireless//radiotap.o
  CC  net/wireless//util.o
  CC  net/wireless//reg.o
  CC  net/wireless//scan.o
  CC  net/wireless//nl80211.o
  CC  net/wireless//mlme.o
  CC  net/wireless//ibss.o
  CC  net/wireless//sme.o
  CC  net/wireless//chan.o
  CC  net/wireless//ethtool.o
  CC  net/wireless//mesh.o
  CC  net/wireless//ap.o
  CC  net/wireless//trace.o
  CC  net/wireless//ocb.o
  LD  net/wireless//cfg80211.o
  LD  net/wireless//built-in.o
  Building modules, stage 2.
  MODPOST 0 modules

$ make net/wireless/reg.o
scripts/kconfig/conf  --silentoldconfig Kconfig
*
* Restart config...
*
*
* Memory power savings
*

> Didn't Thomas Gleixner suggest that you do a basic C course just yesterday?

I don't have a problem with C programming, So only I'm learning the kernel. 
Also, this is a lie if say "I'm expert to C".

I think be re-learned every day. wrong?

> Paul Bolle

Regards,

~ Ozgur

Re: [PATCH 1/3] dt-bindings: add vendor prefix for Lichee Pi

[Maxime Ripard]

On Thu, Dec 22, 2016 at 04:02:33AM +0800, Icenowy Zheng wrote:
> Lichee Pi is a new "Pi"-named development board series.
> 
> Currently available device, Lichee Pi One, is by only one person as
> night job, so the device series name is chosen to be the vendor prefix.
> 
> Signed-off-by: Icenowy Zheng 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH 1/3] dt-bindings: add vendor prefix for Lichee Pi

[Maxime Ripard]

On Thu, Dec 22, 2016 at 04:02:33AM +0800, Icenowy Zheng wrote:
> Lichee Pi is a new "Pi"-named development board series.
> 
> Currently available device, Lichee Pi One, is by only one person as
> night job, so the device series name is chosen to be the vendor prefix.
> 
> Signed-off-by: Icenowy Zheng 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[PATCH] arm64: Don't trace __switch_to if function graph tracer is enabled

[Joel Fernandes]

Function graph tracer shows negative time (wrap around) when tracing
__switch_to if the nosleep-time trace option is enabled.

Time compensation for nosleep-time is done by an ftrace probe on
sched_switch. This doesn't work well for the following events (with
letters representing timestamps):
A - sched switch probe called for task T switch out
B - __switch_to calltime is recorded
C - sched_switch probe called for task T switch in
D - __switch_to rettime is recorded

If C - A > D - B, then we end up over compensating for the time spent in
__switch_to giving rise to negative times in the trace output.

On x86, __switch_to is not traced if function graph tracer is enabled.
Do the same for arm64 as well.

Cc: Todd Kjos 
Cc: Steven Rostedt 
Cc: Will Deacon 
Cc: Mark Rutland 
Signed-off-by: Joel Fernandes 
---
 arch/arm64/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 01753cd..e84ee27 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -324,7 +324,7 @@ void uao_thread_switch(struct task_struct *next)
 /*
  * Thread switching.
  */
-struct task_struct *__switch_to(struct task_struct *prev,
+__notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev,
struct task_struct *next)
 {
struct task_struct *last;
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH v5] media: Driver for Toshiba et8ek8 5MP sensor

[Pavel Machek]

Hi!

> Thanks for the update.
> 
> On Wed, Dec 14, 2016 at 01:24:51PM +0100, Pavel Machek wrote:
> ...
> > +static int et8ek8_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +   struct et8ek8_sensor *sensor =
> > +   container_of(ctrl->handler, struct et8ek8_sensor, ctrl_handler);
> > +
> > +   switch (ctrl->id) {
> > +   case V4L2_CID_GAIN:
> > +   return et8ek8_set_gain(sensor, ctrl->val);
> > +
> > +   case V4L2_CID_EXPOSURE:
> > +   {
> > +   int rows;
> > +   struct i2c_client *client = 
> > v4l2_get_subdevdata(>subdev);
> > +   rows = ctrl->val;
> > +   return et8ek8_i2c_write_reg(client, ET8EK8_REG_16BIT, 0x1243,
> > +   swab16(rows));
> 
> Why swab16()? Doesn't the et8ek8_i2c_write_reg() already do the right thing?
> 
> 16-bit writes aren't used elsewhere... and the register address and value
> seem to have different endianness there, it looks like a bug to me in that
> function.

I'm pretty sure I did not invent that swab16(). I checked, and
exposure seems to work properly. I tried swapping the bytes, but then
exposure did not seem to work. So this one seems to be correct.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v5] media: Driver for Toshiba et8ek8 5MP sensor

[Pavel Machek]

Hi!

> Thanks for the update.
> 
> On Wed, Dec 14, 2016 at 01:24:51PM +0100, Pavel Machek wrote:
> ...
> > +static int et8ek8_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +   struct et8ek8_sensor *sensor =
> > +   container_of(ctrl->handler, struct et8ek8_sensor, ctrl_handler);
> > +
> > +   switch (ctrl->id) {
> > +   case V4L2_CID_GAIN:
> > +   return et8ek8_set_gain(sensor, ctrl->val);
> > +
> > +   case V4L2_CID_EXPOSURE:
> > +   {
> > +   int rows;
> > +   struct i2c_client *client = 
> > v4l2_get_subdevdata(>subdev);
> > +   rows = ctrl->val;
> > +   return et8ek8_i2c_write_reg(client, ET8EK8_REG_16BIT, 0x1243,
> > +   swab16(rows));
> 
> Why swab16()? Doesn't the et8ek8_i2c_write_reg() already do the right thing?
> 
> 16-bit writes aren't used elsewhere... and the register address and value
> seem to have different endianness there, it looks like a bug to me in that
> function.

I'm pretty sure I did not invent that swab16(). I checked, and
exposure seems to work properly. I tried swapping the bytes, but then
exposure did not seem to work. So this one seems to be correct.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH] arm64: Don't trace __switch_to if function graph tracer is enabled

[Joel Fernandes]

Function graph tracer shows negative time (wrap around) when tracing
__switch_to if the nosleep-time trace option is enabled.

Time compensation for nosleep-time is done by an ftrace probe on
sched_switch. This doesn't work well for the following events (with
letters representing timestamps):
A - sched switch probe called for task T switch out
B - __switch_to calltime is recorded
C - sched_switch probe called for task T switch in
D - __switch_to rettime is recorded

If C - A > D - B, then we end up over compensating for the time spent in
__switch_to giving rise to negative times in the trace output.

On x86, __switch_to is not traced if function graph tracer is enabled.
Do the same for arm64 as well.

Cc: Todd Kjos 
Cc: Steven Rostedt 
Cc: Will Deacon 
Cc: Mark Rutland 
Signed-off-by: Joel Fernandes 
---
 arch/arm64/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 01753cd..e84ee27 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -324,7 +324,7 @@ void uao_thread_switch(struct task_struct *next)
 /*
  * Thread switching.
  */
-struct task_struct *__switch_to(struct task_struct *prev,
+__notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev,
struct task_struct *next)
 {
struct task_struct *last;
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH 2/2] net: wireless: fix to uses struct

[Paul Bolle]

On Thu, 2016-12-22 at 00:23 +0200, Ozgur Karatas wrote:
> I compiled and didn't to errors.

Really?

$ make net/wireless/reg.o
  CHK include/config/kernel.release
  CHK include/generated/uapi/linux/version.h
  CHK include/generated/utsrelease.h
  CHK include/generated/bounds.h
  CHK include/generated/timeconst.h
  CHK include/generated/asm-offsets.h
  CALLscripts/checksyscalls.sh
  CC  net/wireless/reg.o
net/wireless/reg.c: In function ‘regulatory_hint_core’:
net/wireless/reg.c:2294:28: error: ‘regulatory_request’ undeclared (first use 
in this function)
  request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
^~
net/wireless/reg.c:2294:28: note: each undeclared identifier is reported only 
once for each function it appears in
net/wireless/reg.c: In function ‘regulatory_hint_user’:
net/wireless/reg.c:2316:28: error: ‘regulatory_request’ undeclared (first use 
in this function)
  request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
^~
net/wireless/reg.c: In function ‘regulatory_hint’:
net/wireless/reg.c:2388:28: error: ‘regulatory_request’ undeclared (first use 
in this function)
  request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
^~
scripts/Makefile.build:293: recipe for target 'net/wireless/reg.o' failed
make[1]: *** [net/wireless/reg.o] Error 1
Makefile:1640: recipe for target 'net/wireless/reg.o' failed
make: *** [net/wireless/reg.o] Error 2 

Didn't Thomas Gleixner suggest that you do a basic C course just yesterday?


Paul Bolle

Re: [PATCH 2/2] net: wireless: fix to uses struct

[Paul Bolle]

On Thu, 2016-12-22 at 00:23 +0200, Ozgur Karatas wrote:
> I compiled and didn't to errors.

Really?

$ make net/wireless/reg.o
  CHK include/config/kernel.release
  CHK include/generated/uapi/linux/version.h
  CHK include/generated/utsrelease.h
  CHK include/generated/bounds.h
  CHK include/generated/timeconst.h
  CHK include/generated/asm-offsets.h
  CALLscripts/checksyscalls.sh
  CC  net/wireless/reg.o
net/wireless/reg.c: In function ‘regulatory_hint_core’:
net/wireless/reg.c:2294:28: error: ‘regulatory_request’ undeclared (first use 
in this function)
  request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
^~
net/wireless/reg.c:2294:28: note: each undeclared identifier is reported only 
once for each function it appears in
net/wireless/reg.c: In function ‘regulatory_hint_user’:
net/wireless/reg.c:2316:28: error: ‘regulatory_request’ undeclared (first use 
in this function)
  request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
^~
net/wireless/reg.c: In function ‘regulatory_hint’:
net/wireless/reg.c:2388:28: error: ‘regulatory_request’ undeclared (first use 
in this function)
  request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
^~
scripts/Makefile.build:293: recipe for target 'net/wireless/reg.o' failed
make[1]: *** [net/wireless/reg.o] Error 1
Makefile:1640: recipe for target 'net/wireless/reg.o' failed
make: *** [net/wireless/reg.o] Error 2 

Didn't Thomas Gleixner suggest that you do a basic C course just yesterday?


Paul Bolle

Re: [PATCH] x86/efi Fix regression in efi_arch_mem_reserve

[Matt Fleming]

On Wed, 21 Dec, at 02:11:38PM, Petr Oros wrote:
>   Booting on EFI with ESRT table has been stop since commit:
> 8e80632 efi/esrt: Use efi_mem_reserve() and avoid a kmalloc()
> 
>   This is caused by this commit:
> 816e761 efi: Allow drivers to reserve boot services forever
> 
>   Problem is, that efi_memmap_insert need memory aligned
>   on EFI_PAGE_SIZE. If memory not aligned, efi_memmap_insert
>   just return and let efi.memmap in inconsistent state.
>   This breaking boot.
> 
>   Tested in my machine, which stop booting
>   after upgrade to 4.9
> 
> Signed-off-by: Petr Oros 
> ---
>  arch/x86/platform/efi/quirks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Could you provide some more information? Why does efi_memmap_insert()
require this alignment? How does booting "break"? If you see an Oops,
please post it here.

Re: [PATCH] x86/efi Fix regression in efi_arch_mem_reserve

[Matt Fleming]

On Wed, 21 Dec, at 02:11:38PM, Petr Oros wrote:
>   Booting on EFI with ESRT table has been stop since commit:
> 8e80632 efi/esrt: Use efi_mem_reserve() and avoid a kmalloc()
> 
>   This is caused by this commit:
> 816e761 efi: Allow drivers to reserve boot services forever
> 
>   Problem is, that efi_memmap_insert need memory aligned
>   on EFI_PAGE_SIZE. If memory not aligned, efi_memmap_insert
>   just return and let efi.memmap in inconsistent state.
>   This breaking boot.
> 
>   Tested in my machine, which stop booting
>   after upgrade to 4.9
> 
> Signed-off-by: Petr Oros 
> ---
>  arch/x86/platform/efi/quirks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Could you provide some more information? Why does efi_memmap_insert()
require this alignment? How does booting "break"? If you see an Oops,
please post it here.

Re: [kernel-hardening] Re: HalfSipHash Acceptable Usage

[Jason A. Donenfeld]

On Wed, Dec 21, 2016 at 11:27 PM, Theodore Ts'o  wrote:
> And "with enough registers" includes ARM and MIPS, right?  So the only
> real problem is 32-bit x86, and you're right, at that point, only
> people who might care are people who are using a space-radiation
> hardened 386 --- and they're not likely to be doing high throughput
> TCP connections.  :-)

Plus the benchmark was bogus anyway, and when I built a more specific
harness -- actually comparing the TCP sequence number functions --
SipHash was faster than MD5, even on register starved x86. So I think
we're fine and this chapter of the discussion can come to a close, in
order to move on to more interesting things.

Re: [kernel-hardening] Re: HalfSipHash Acceptable Usage

[Jason A. Donenfeld]

On Wed, Dec 21, 2016 at 11:27 PM, Theodore Ts'o  wrote:
> And "with enough registers" includes ARM and MIPS, right?  So the only
> real problem is 32-bit x86, and you're right, at that point, only
> people who might care are people who are using a space-radiation
> hardened 386 --- and they're not likely to be doing high throughput
> TCP connections.  :-)

Plus the benchmark was bogus anyway, and when I built a more specific
harness -- actually comparing the TCP sequence number functions --
SipHash was faster than MD5, even on register starved x86. So I think
we're fine and this chapter of the discussion can come to a close, in
order to move on to more interesting things.

Re: HalfSipHash Acceptable Usage

[Theodore Ts'o]

On Wed, Dec 21, 2016 at 01:37:51PM -0500, George Spelvin wrote:
> SipHash annihilates the competition on 64-bit superscalar hardware.
> SipHash dominates the field on 64-bit in-order hardware.
> SipHash wins easily on 32-bit hardware *with enough registers*.
> On register-starved 32-bit machines, it really struggles.

And "with enough registers" includes ARM and MIPS, right?  So the only
real problem is 32-bit x86, and you're right, at that point, only
people who might care are people who are using a space-radiation
hardened 386 --- and they're not likely to be doing high throughput
TCP connections.  :-)

- Ted

Re: HalfSipHash Acceptable Usage

[Theodore Ts'o]

On Wed, Dec 21, 2016 at 01:37:51PM -0500, George Spelvin wrote:
> SipHash annihilates the competition on 64-bit superscalar hardware.
> SipHash dominates the field on 64-bit in-order hardware.
> SipHash wins easily on 32-bit hardware *with enough registers*.
> On register-starved 32-bit machines, it really struggles.

And "with enough registers" includes ARM and MIPS, right?  So the only
real problem is 32-bit x86, and you're right, at that point, only
people who might care are people who are using a space-radiation
hardened 386 --- and they're not likely to be doing high throughput
TCP connections.  :-)

- Ted

Re: [4.10, panic, regression] iscsi: null pointer deref at iscsi_tcp_segment_done+0x20d/0x2e0

[Dave Chinner]

On Fri, Dec 16, 2016 at 10:59:06AM -0800, Chris Leech wrote:
> Thanks Dave,
> 
> I'm hitting a bug at scatterlist.h:140 before I even get any iSCSI
> modules loaded (virtio block) so there's something else going on in the
> current merge window.  I'll keep an eye on it and make sure there's
> nothing iSCSI needs fixing for.

OK, so before this slips through the cracks.

Linus - your tree as of a few minutes ago still panics immediately
when starting xfstests on iscsi devices. It appears to be a
scatterlist corruption and not an iscsi problem, so the iscsi guys
seem to have bounced it and no-one is looking at it.

I'm disappearing for several months at the end of tomorrow, so I
thought I better make sure you know about it.  I've also added
linux-scsi, linux-block to the cc list

Cheers,

Dave.

> On Thu, Dec 15, 2016 at 09:29:53AM +1100, Dave Chinner wrote:
> > On Thu, Dec 15, 2016 at 09:24:11AM +1100, Dave Chinner wrote:
> > > Hi folks,
> > > 
> > > Just updated my test boxes from 4.9 to a current Linus 4.10 merge
> > > window kernel to test the XFS merge I am preparing for Linus.
> > > Unfortunately, all my test VMs using iscsi failed pretty much
> > > instantly on the first mount of an iscsi device:
> > > 
> > > [  159.372704] XFS (sdb): EXPERIMENTAL reverse mapping btree feature 
> > > enabled. Use at your own risk!
> > > [  159.374612] XFS (sdb): Mounting V5 Filesystem
> > > [  159.425710] XFS (sdb): Ending clean mount
> > > [  160.274438] BUG: unable to handle kernel NULL pointer dereference at 
> > > 000c
> > > [  160.275851] IP: iscsi_tcp_segment_done+0x20d/0x2e0
> > 
> > FYI, crash is here:
> > 
> > (gdb) l *(iscsi_tcp_segment_done+0x20d)
> > 0x81b950bd is in iscsi_tcp_segment_done 
> > (drivers/scsi/libiscsi_tcp.c:102).
> > 97  iscsi_tcp_segment_init_sg(struct iscsi_segment *segment,
> > 98struct scatterlist *sg, unsigned int offset)
> > 99  {
> > 100 segment->sg = sg;
> > 101 segment->sg_offset = offset;
> > 102 segment->size = min(sg->length - offset,
> > 103 segment->total_size - 
> > segment->total_copied);
> > 104 segment->data = NULL;
> > 105 }
> > 106 
> > 
> > So it looks to be sg = NULL, which means there's probably an issue
> > with the scatterlist...
> > 
> > -Dave.
> > 
> > > [  160.276565] PGD 336ed067 [  160.276885] PUD 31b0d067
> > > PMD 0 [  160.277309]
> > > [  160.277523] Oops:  [#1] PREEMPT SMP
> > > [  160.278004] Modules linked in:
> > > [  160.278407] CPU: 0 PID: 16 Comm: kworker/u2:1 Not tainted 4.9.0-dgc #18
> > > [  160.279224] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
> > > BIOS Debian-1.8.2-1 04/01/2014
> > > [  160.280314] Workqueue: iscsi_q_2 iscsi_xmitworker
> > > [  160.280919] task: 88003e28 task.stack: c908
> > > [  160.281647] RIP: 0010:iscsi_tcp_segment_done+0x20d/0x2e0
> > > [  160.282312] RSP: 0018:c9083c38 EFLAGS: 00010206
> > > [  160.282980] RAX:  RBX: 880039061730 RCX: 
> > > 
> > > [  160.283854] RDX: 1e00 RSI:  RDI: 
> > > 880039061730
> > > [  160.284738] RBP: c9083c90 R08: 0200 R09: 
> > > 05a8
> > > [  160.285627] R10: 9835607d R11:  R12: 
> > > 0200
> > > [  160.286495] R13:  R14: 8800390615a0 R15: 
> > > 880039061730
> > > [  160.287362] FS:  () GS:88003fc0() 
> > > knlGS:
> > > [  160.288340] CS:  0010 DS:  ES:  CR0: 80050033
> > > [  160.289113] CR2: 000c CR3: 31a8d000 CR4: 
> > > 06f0
> > > [  160.290084] Call Trace:
> > > [  160.290429]  ? inet_sendpage+0x4d/0x140
> > > [  160.290957]  iscsi_sw_tcp_xmit_segment+0x89/0x110
> > > [  160.291597]  iscsi_sw_tcp_pdu_xmit+0x56/0x180
> > > [  160.292190]  iscsi_tcp_task_xmit+0xb8/0x280
> > > [  160.292771]  iscsi_xmit_task+0x53/0xc0
> > > [  160.293282]  iscsi_xmitworker+0x274/0x310
> > > [  160.293835]  process_one_work+0x1de/0x4d0
> > > [  160.294388]  worker_thread+0x4b/0x4f0
> > > [  160.294889]  kthread+0x10c/0x140
> > > [  160.295333]  ? process_one_work+0x4d0/0x4d0
> > > [  160.295898]  ? kthread_create_on_node+0x40/0x40
> > > [  160.296525]  ret_from_fork+0x25/0x30
> > > [  160.297015] Code: 43 18 00 00 00 00 e9 ad fe ff ff 48 8b 7b 30 e8 da 
> > > e7 ca ff 8b 53 10 44 89 ee 48 89 df 2b 53 14 48 89 43 30 c7 43 40 00 00 
> > > 00 00 <8b
> > > [  160.300674] RIP: iscsi_tcp_segment_done+0x20d/0x2e0 RSP: 
> > > c9083c38
> > > [  160.301584] CR2: 000c
> > > 
> > > 
> > > Known problem, or something new?
> > > 
> > > Cheers,
> > > 
> > > Dave.
> > > -- 
> > > Dave Chinner
> > > da...@fromorbit.com
> > > 
> > 
> > -- 
> > Dave Chinner
> > da...@fromorbit.com
> 

-- 
Dave Chinner
da...@fromorbit.com

Re: [4.10, panic, regression] iscsi: null pointer deref at iscsi_tcp_segment_done+0x20d/0x2e0

[Dave Chinner]

On Fri, Dec 16, 2016 at 10:59:06AM -0800, Chris Leech wrote:
> Thanks Dave,
> 
> I'm hitting a bug at scatterlist.h:140 before I even get any iSCSI
> modules loaded (virtio block) so there's something else going on in the
> current merge window.  I'll keep an eye on it and make sure there's
> nothing iSCSI needs fixing for.

OK, so before this slips through the cracks.

Linus - your tree as of a few minutes ago still panics immediately
when starting xfstests on iscsi devices. It appears to be a
scatterlist corruption and not an iscsi problem, so the iscsi guys
seem to have bounced it and no-one is looking at it.

I'm disappearing for several months at the end of tomorrow, so I
thought I better make sure you know about it.  I've also added
linux-scsi, linux-block to the cc list

Cheers,

Dave.

> On Thu, Dec 15, 2016 at 09:29:53AM +1100, Dave Chinner wrote:
> > On Thu, Dec 15, 2016 at 09:24:11AM +1100, Dave Chinner wrote:
> > > Hi folks,
> > > 
> > > Just updated my test boxes from 4.9 to a current Linus 4.10 merge
> > > window kernel to test the XFS merge I am preparing for Linus.
> > > Unfortunately, all my test VMs using iscsi failed pretty much
> > > instantly on the first mount of an iscsi device:
> > > 
> > > [  159.372704] XFS (sdb): EXPERIMENTAL reverse mapping btree feature 
> > > enabled. Use at your own risk!
> > > [  159.374612] XFS (sdb): Mounting V5 Filesystem
> > > [  159.425710] XFS (sdb): Ending clean mount
> > > [  160.274438] BUG: unable to handle kernel NULL pointer dereference at 
> > > 000c
> > > [  160.275851] IP: iscsi_tcp_segment_done+0x20d/0x2e0
> > 
> > FYI, crash is here:
> > 
> > (gdb) l *(iscsi_tcp_segment_done+0x20d)
> > 0x81b950bd is in iscsi_tcp_segment_done 
> > (drivers/scsi/libiscsi_tcp.c:102).
> > 97  iscsi_tcp_segment_init_sg(struct iscsi_segment *segment,
> > 98struct scatterlist *sg, unsigned int offset)
> > 99  {
> > 100 segment->sg = sg;
> > 101 segment->sg_offset = offset;
> > 102 segment->size = min(sg->length - offset,
> > 103 segment->total_size - 
> > segment->total_copied);
> > 104 segment->data = NULL;
> > 105 }
> > 106 
> > 
> > So it looks to be sg = NULL, which means there's probably an issue
> > with the scatterlist...
> > 
> > -Dave.
> > 
> > > [  160.276565] PGD 336ed067 [  160.276885] PUD 31b0d067
> > > PMD 0 [  160.277309]
> > > [  160.277523] Oops:  [#1] PREEMPT SMP
> > > [  160.278004] Modules linked in:
> > > [  160.278407] CPU: 0 PID: 16 Comm: kworker/u2:1 Not tainted 4.9.0-dgc #18
> > > [  160.279224] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
> > > BIOS Debian-1.8.2-1 04/01/2014
> > > [  160.280314] Workqueue: iscsi_q_2 iscsi_xmitworker
> > > [  160.280919] task: 88003e28 task.stack: c908
> > > [  160.281647] RIP: 0010:iscsi_tcp_segment_done+0x20d/0x2e0
> > > [  160.282312] RSP: 0018:c9083c38 EFLAGS: 00010206
> > > [  160.282980] RAX:  RBX: 880039061730 RCX: 
> > > 
> > > [  160.283854] RDX: 1e00 RSI:  RDI: 
> > > 880039061730
> > > [  160.284738] RBP: c9083c90 R08: 0200 R09: 
> > > 05a8
> > > [  160.285627] R10: 9835607d R11:  R12: 
> > > 0200
> > > [  160.286495] R13:  R14: 8800390615a0 R15: 
> > > 880039061730
> > > [  160.287362] FS:  () GS:88003fc0() 
> > > knlGS:
> > > [  160.288340] CS:  0010 DS:  ES:  CR0: 80050033
> > > [  160.289113] CR2: 000c CR3: 31a8d000 CR4: 
> > > 06f0
> > > [  160.290084] Call Trace:
> > > [  160.290429]  ? inet_sendpage+0x4d/0x140
> > > [  160.290957]  iscsi_sw_tcp_xmit_segment+0x89/0x110
> > > [  160.291597]  iscsi_sw_tcp_pdu_xmit+0x56/0x180
> > > [  160.292190]  iscsi_tcp_task_xmit+0xb8/0x280
> > > [  160.292771]  iscsi_xmit_task+0x53/0xc0
> > > [  160.293282]  iscsi_xmitworker+0x274/0x310
> > > [  160.293835]  process_one_work+0x1de/0x4d0
> > > [  160.294388]  worker_thread+0x4b/0x4f0
> > > [  160.294889]  kthread+0x10c/0x140
> > > [  160.295333]  ? process_one_work+0x4d0/0x4d0
> > > [  160.295898]  ? kthread_create_on_node+0x40/0x40
> > > [  160.296525]  ret_from_fork+0x25/0x30
> > > [  160.297015] Code: 43 18 00 00 00 00 e9 ad fe ff ff 48 8b 7b 30 e8 da 
> > > e7 ca ff 8b 53 10 44 89 ee 48 89 df 2b 53 14 48 89 43 30 c7 43 40 00 00 
> > > 00 00 <8b
> > > [  160.300674] RIP: iscsi_tcp_segment_done+0x20d/0x2e0 RSP: 
> > > c9083c38
> > > [  160.301584] CR2: 000c
> > > 
> > > 
> > > Known problem, or something new?
> > > 
> > > Cheers,
> > > 
> > > Dave.
> > > -- 
> > > Dave Chinner
> > > da...@fromorbit.com
> > > 
> > 
> > -- 
> > Dave Chinner
> > da...@fromorbit.com
> 

-- 
Dave Chinner
da...@fromorbit.com

[PATCH 2/2] net: wireless: fix to uses struct

[Ozgur Karatas]

The patch fixed to struct uses in reg.c, I think doesn't need to be use to 
"struct". 
There is dataype not have to logical link and each is different definitons.

I'm undecided on this patch. I compiled and didn't to errors.
 
Signed-off-by: Ozgur Karatas 
---
 net/wireless/reg.c  | 10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..5b70970 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -490,7 +490,7 @@ static int reg_query_builtin(const char *alpha2)
if (!regdom)
return -ENODATA;
 
-   request = kzalloc(sizeof(struct reg_regdb_apply_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*reg_regdb_apply_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2291,7 +2291,7 @@ static int regulatory_hint_core(const char *alpha2)
 {
struct regulatory_request *request;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2313,7 +2313,7 @@ int regulatory_hint_user(const char *alpha2,
if (WARN_ON(!alpha2))
return -EINVAL;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2385,7 +2385,7 @@ int regulatory_hint(struct wiphy *wiphy, const char 
*alpha2)
 
wiphy->regulatory_flags &= ~REGULATORY_CUSTOM_REG;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2661,7 +2661,7 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
if (processing)
return 0;
 
-   reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
+   reg_beacon = kzalloc(sizeof(*reg_beacon), gfp);
if (!reg_beacon)
return -ENOMEM;
 
-- 
2.1.4

[PATCH 2/2] net: wireless: fix to uses struct

[Ozgur Karatas]

The patch fixed to struct uses in reg.c, I think doesn't need to be use to 
"struct". 
There is dataype not have to logical link and each is different definitons.

I'm undecided on this patch. I compiled and didn't to errors.
 
Signed-off-by: Ozgur Karatas 
---
 net/wireless/reg.c  | 10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..5b70970 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -490,7 +490,7 @@ static int reg_query_builtin(const char *alpha2)
if (!regdom)
return -ENODATA;
 
-   request = kzalloc(sizeof(struct reg_regdb_apply_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*reg_regdb_apply_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2291,7 +2291,7 @@ static int regulatory_hint_core(const char *alpha2)
 {
struct regulatory_request *request;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2313,7 +2313,7 @@ int regulatory_hint_user(const char *alpha2,
if (WARN_ON(!alpha2))
return -EINVAL;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2385,7 +2385,7 @@ int regulatory_hint(struct wiphy *wiphy, const char 
*alpha2)
 
wiphy->regulatory_flags &= ~REGULATORY_CUSTOM_REG;
 
-   request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
+   request = kzalloc(sizeof(*regulatory_request), GFP_KERNEL);
if (!request)
return -ENOMEM;
 
@@ -2661,7 +2661,7 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
if (processing)
return 0;
 
-   reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
+   reg_beacon = kzalloc(sizeof(*reg_beacon), gfp);
if (!reg_beacon)
return -ENOMEM;
 
-- 
2.1.4

[PATCH 1/2] net: wireless: fixed to checkpatch errors

[Ozgur Karatas]

Fixed to checkpatch errors, Normally, comment's */ had to be finish on the next 
line.
The patch fix to readability and Coding Style.

Sİgned-off-by: Ozgur Karatas 
---
 net/wireless/chan.c |  3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 5497d022..8c7ac7f 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -891,7 +891,8 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
  : CHAN_MODE_EXCLUSIVE;
 
/* consider worst-case - IBSS can try to return to the
-* original user-specified channel as creator */
+* original user-specified channel as creator 
+*/
if (wdev->ibss_dfs_possible)
*radar_detect |= BIT(wdev->chandef.width);
return;
-- 
2.1.4

[PATCH 1/2] net: wireless: fixed to checkpatch errors

[Ozgur Karatas]

Fixed to checkpatch errors, Normally, comment's */ had to be finish on the next 
line.
The patch fix to readability and Coding Style.

Sİgned-off-by: Ozgur Karatas 
---
 net/wireless/chan.c |  3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 5497d022..8c7ac7f 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -891,7 +891,8 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
  : CHAN_MODE_EXCLUSIVE;
 
/* consider worst-case - IBSS can try to return to the
-* original user-specified channel as creator */
+* original user-specified channel as creator 
+*/
if (wdev->ibss_dfs_possible)
*radar_detect |= BIT(wdev->chandef.width);
return;
-- 
2.1.4

[PATCH 2/2] power: supply: bq24735: bring down the noise level

[Peter Rosin]

If there is no ti,ac-detect-gpios configured, it is normal to
have failed reads of the options register. So, hold back on the
log spamming.

Signed-off-by: Peter Rosin 
---
 drivers/power/supply/bq24735-charger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq24735-charger.c 
b/drivers/power/supply/bq24735-charger.c
index d8be81203837..eb0145380def 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -192,7 +192,7 @@ static bool bq24735_charger_is_present(struct bq24735 
*charger)
 
ac = bq24735_read_word(charger->client, BQ24735_CHG_OPT);
if (ac < 0) {
-   dev_err(>client->dev,
+   dev_dbg(>client->dev,
"Failed to read charger options : %d\n",
ac);
return false;
-- 
2.1.4

[PATCH 2/2] power: supply: bq24735: bring down the noise level

[Peter Rosin]

If there is no ti,ac-detect-gpios configured, it is normal to
have failed reads of the options register. So, hold back on the
log spamming.

Signed-off-by: Peter Rosin 
---
 drivers/power/supply/bq24735-charger.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq24735-charger.c 
b/drivers/power/supply/bq24735-charger.c
index d8be81203837..eb0145380def 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -192,7 +192,7 @@ static bool bq24735_charger_is_present(struct bq24735 
*charger)
 
ac = bq24735_read_word(charger->client, BQ24735_CHG_OPT);
if (ac < 0) {
-   dev_err(>client->dev,
+   dev_dbg(>client->dev,
"Failed to read charger options : %d\n",
ac);
return false;
-- 
2.1.4

[PATCH 0/2] power: supply: bq24735: poll register if no ac-detect gpio

[Peter Rosin]

Hi!

My patch [1] "power: supply: bq24735-charger: allow chargers to share the
ac-detect gpio" is perhaps a bit hard to digest. And while I still think
some way of sharing the ac-detect gpio is worthwhile, I thought of another
way to solve the problem at hand. Instead of polling a shared gpio, it's
as simple as polling the chip for what it is outputting on the ACOK pin.
And the code is already there! It just needs a tweak to allow this mode of
operation.

This appears to work just fine for me, and the difference is in the noise
since my shared gpio pin happens to be on an expander on the i2c bus, so
I end up with i2c traffic for each poll either way.

Cheers,
peda

[1] https://lkml.org/lkml/2016/12/13/786

Peter Rosin (2):
  power: supply: bq24735: allow polling even if there is no ac-detect
gpio
  power: supply: bq24735: bring down the noise level

 Documentation/devicetree/bindings/power/supply/ti,bq24735.txt | 4 ++--
 drivers/power/supply/bq24735-charger.c| 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.1.4

[PATCH 0/2] power: supply: bq24735: poll register if no ac-detect gpio

[Peter Rosin]

Hi!

My patch [1] "power: supply: bq24735-charger: allow chargers to share the
ac-detect gpio" is perhaps a bit hard to digest. And while I still think
some way of sharing the ac-detect gpio is worthwhile, I thought of another
way to solve the problem at hand. Instead of polling a shared gpio, it's
as simple as polling the chip for what it is outputting on the ACOK pin.
And the code is already there! It just needs a tweak to allow this mode of
operation.

This appears to work just fine for me, and the difference is in the noise
since my shared gpio pin happens to be on an expander on the i2c bus, so
I end up with i2c traffic for each poll either way.

Cheers,
peda

[1] https://lkml.org/lkml/2016/12/13/786

Peter Rosin (2):
  power: supply: bq24735: allow polling even if there is no ac-detect
gpio
  power: supply: bq24735: bring down the noise level

 Documentation/devicetree/bindings/power/supply/ti,bq24735.txt | 4 ++--
 drivers/power/supply/bq24735-charger.c| 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.1.4

Re: [PATCH] MAINTAINERS: add myself as maintainer of fbdev

[Sudip Mukherjee]

On Wed, Dec 21, 2016 at 05:50:05PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, December 21, 2016 03:06:55 PM Sudip Mukherjee wrote:
> > On Thu, Dec 15, 2016 at 03:45:47PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > I would like to help with fbdev maintenance.  I can dedicate some time
> > > for reviewing and handling patches but won't have time for much more.
> > > 
> > 
> > I thought usually someone takes over the maintainer role after
> > proving that he cares for a subsystem for a certain period of time.
> > 
> > I know Bartlomiej is here for a long time, but fbdev??
> 
> I'm not a fbdev expert and I'm not pretending to be one.
> 
> I decided to officially handle patches for fbdev after this
> subsystem has been marked as Orphan in upstream tree (nobody
> has volunteered to take over from Tomi after he sent his
> resignation patch to a list a week earlier).

ohhh.. right... i am seeing that thread now. Sorry for the noise.
I have two months of lkml and fbdev mails pending, will read them in
next two weeks of holidays. Only managed to see few mails that had me
in to or cc.

> 
> I won't have a lot of time even for this activity so help is
> very much welcomed.

time is the only thing which is very scarce nowadays. but please
do ping me if you need any help.

regards
sudip

Re: [PATCH] MAINTAINERS: add myself as maintainer of fbdev

[Sudip Mukherjee]

On Wed, Dec 21, 2016 at 05:50:05PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, December 21, 2016 03:06:55 PM Sudip Mukherjee wrote:
> > On Thu, Dec 15, 2016 at 03:45:47PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > I would like to help with fbdev maintenance.  I can dedicate some time
> > > for reviewing and handling patches but won't have time for much more.
> > > 
> > 
> > I thought usually someone takes over the maintainer role after
> > proving that he cares for a subsystem for a certain period of time.
> > 
> > I know Bartlomiej is here for a long time, but fbdev??
> 
> I'm not a fbdev expert and I'm not pretending to be one.
> 
> I decided to officially handle patches for fbdev after this
> subsystem has been marked as Orphan in upstream tree (nobody
> has volunteered to take over from Tomi after he sent his
> resignation patch to a list a week earlier).

ohhh.. right... i am seeing that thread now. Sorry for the noise.
I have two months of lkml and fbdev mails pending, will read them in
next two weeks of holidays. Only managed to see few mails that had me
in to or cc.

> 
> I won't have a lot of time even for this activity so help is
> very much welcomed.

time is the only thing which is very scarce nowadays. but please
do ping me if you need any help.

regards
sudip

Re: [GIT pull] x86/cache: Updates for 4.10

[Fenghua Yu]

On Tue, Dec 13, 2016, Thomas Gleixner  wrote:
> On Mon, 12 Dec 2016, Linus Torvalds wrote:
> On Mon, Dec 12, 2016 at 1:53 AM, Thomas Gleixner  wrote:
>> It looks pretty self-contained (good), but it also looks majorly
>> strange. I will have to think about this. What are the main/expected
>> users?
> - Virtualization so a VM can only trash only the associated part of the
> cash w/o disturbing others
> 
> - Real-Time systems to seperate RT and general workloads.
> 
> - Latency sensitive enterprise workloads
> 
> - In theory this also can be used to protect against cache side channel
> attacks.

Quite a few companies and research groups are interested in the hardware
feature and/or the resctrl file system interface. They do demonstrate
performance and QoS improvements in real-time, VM, and native Linux
by partitioning cache for processes to avoid noisy neighbors.

Thanks.

-Fenghua

Re: DAX mapping detection (was: Re: [PATCH] Fix region lost in /proc/self/smaps)

[Dan Williams]

On Wed, Dec 21, 2016 at 1:24 PM, Dave Chinner  wrote:
> On Wed, Dec 21, 2016 at 08:53:46AM -0800, Dan Williams wrote:
>> On Tue, Dec 20, 2016 at 4:40 PM, Darrick J. Wong
>>  wrote:
>> > On Mon, Dec 19, 2016 at 05:18:40PM -0800, Dan Williams wrote:
>> >> On Mon, Dec 19, 2016 at 5:09 PM, Darrick J. Wong
>> >>  wrote:
>> >> > On Mon, Dec 19, 2016 at 02:11:49PM -0700, Ross Zwisler wrote:
>> >> >> On Fri, Sep 16, 2016 at 03:54:05PM +1000, Nicholas Piggin wrote:
>> >> >> <>
>> >> >> > Definitely the first step would be your simple preallocated per
>> >> >> > inode approach until it is shown to be insufficient.
>> >> >>
>> >> >> Reviving this thread a few months later...
>> >> >>
>> >> >> Dave, we're interested in taking a serious look at what it would take 
>> >> >> to get
>> >> >> PMEM_IMMUTABLE working.  Do you still hold the opinion that this is 
>> >> >> (or could
>> >> >> become, with some amount of work) a workable solution?
>> >> >>
>> >> >> We're happy to do the grunt work for this feature, but we will 
>> >> >> probably need
>> >> >> guidance from someone with more XFS experience.  With you out on 
>> >> >> extended leave
>> >> >> the first half of 2017, who would be the best person to ask for this 
>> >> >> guidance?
>> >> >> Darrick?
>> >> >
>> >> > Yes, probably. :)
>> >> >
>> >> > I think where we left off with this (on the XFS side) is some sort of
>> >> > fallocate mode that would allocate blocks, zero them, and then set the
>> >> > DAX and PMEM_IMMUTABLE on-disk inode flags.  After that, you'd mmap the
>> >> > file and thereby gain the ability to control write persistents behavior
>> >> > without having to worry about fs metadata updates.  As an added plus, I
>> >> > think zeroing the pmem also clears media errors, or something like that.
>> >> >
>> >> >  Is that a reasonable starting point?  My memory is a little 
>> >> > foggy.
>> >> >
>> >> > Hmm, I see Dan just posted something about blockdev fallocate.  I'll go
>> >> > read that.
>> >>
>> >> That's for device-dax, which is basically a poor man's PMEM_IMMUTABLE
>> >> via a character device interface. It's useful for cases where you want
>> >> an entire nvdimm namespace/volume in "no fs-metadata to worry about"
>> >> mode.  But, for sub-allocations of a namespace and support for
>> >> existing tooling, PMEM_IMMUTABLE is much more usable.
>> >
>> > Well sure... but otoh I was thinking that it'd be pretty neat if we
>> > could use the same code regardless of whether the target file was a
>> > dax-device or an xfs file:
>> >
>> > fd = open("", O_RDWR);
>> > fstat(fd, ):
>> > fallocate(fd, FALLOC_FL_PMEM_IMMUTABLE, 0, statbuf.st_size);
>> > p = mmap(NULL, statbuf.st_size, PROT_READ | PROT_WRITE, fd, 0);
>> >
>> > *(p + 42) = 0xDEADBEEF;
>> > asm { clflush; } /* or whatever */
>> >
>> > ...so perhaps it would be a good idea to design the fallocate primitive
>> > around "prepare this fd for mmap-only pmem semantics" and let it the
>> > backend do zeroing and inode flag changes as necessary to make it
>> > happen.  We'd need to do some bikeshedding about what the other falloc
>> > flags mean when we're dealing with pmem files and devices, but I think
>> > we should try to keep the userland presentation the same unless there's
>> > a really good reason not to.
>>
>> It would be interesting to use fallocate to size device-dax files...
>
> No. device-dax needs to die, not poison a bunch of existing file and
> block device APIs and behaviours with special snowflakes.  Get
> DAX-enabled filesystems to do what you need, and get rid of this
> ugly, nasty hack.
>

Right, Christoph already killed fallocate for device-dax.

What we're looking for now is the next level of detail on how to get
started on PMEM_IMMUTABLE, as Ross asked a few messages back in this
thread, so we have a reasonable replacement for device-dax.

Re: [GIT pull] x86/cache: Updates for 4.10

[Fenghua Yu]

On Tue, Dec 13, 2016, Thomas Gleixner  wrote:
> On Mon, 12 Dec 2016, Linus Torvalds wrote:
> On Mon, Dec 12, 2016 at 1:53 AM, Thomas Gleixner  wrote:
>> It looks pretty self-contained (good), but it also looks majorly
>> strange. I will have to think about this. What are the main/expected
>> users?
> - Virtualization so a VM can only trash only the associated part of the
> cash w/o disturbing others
> 
> - Real-Time systems to seperate RT and general workloads.
> 
> - Latency sensitive enterprise workloads
> 
> - In theory this also can be used to protect against cache side channel
> attacks.

Quite a few companies and research groups are interested in the hardware
feature and/or the resctrl file system interface. They do demonstrate
performance and QoS improvements in real-time, VM, and native Linux
by partitioning cache for processes to avoid noisy neighbors.

Thanks.

-Fenghua

Re: DAX mapping detection (was: Re: [PATCH] Fix region lost in /proc/self/smaps)

[Dan Williams]

On Wed, Dec 21, 2016 at 1:24 PM, Dave Chinner  wrote:
> On Wed, Dec 21, 2016 at 08:53:46AM -0800, Dan Williams wrote:
>> On Tue, Dec 20, 2016 at 4:40 PM, Darrick J. Wong
>>  wrote:
>> > On Mon, Dec 19, 2016 at 05:18:40PM -0800, Dan Williams wrote:
>> >> On Mon, Dec 19, 2016 at 5:09 PM, Darrick J. Wong
>> >>  wrote:
>> >> > On Mon, Dec 19, 2016 at 02:11:49PM -0700, Ross Zwisler wrote:
>> >> >> On Fri, Sep 16, 2016 at 03:54:05PM +1000, Nicholas Piggin wrote:
>> >> >> <>
>> >> >> > Definitely the first step would be your simple preallocated per
>> >> >> > inode approach until it is shown to be insufficient.
>> >> >>
>> >> >> Reviving this thread a few months later...
>> >> >>
>> >> >> Dave, we're interested in taking a serious look at what it would take 
>> >> >> to get
>> >> >> PMEM_IMMUTABLE working.  Do you still hold the opinion that this is 
>> >> >> (or could
>> >> >> become, with some amount of work) a workable solution?
>> >> >>
>> >> >> We're happy to do the grunt work for this feature, but we will 
>> >> >> probably need
>> >> >> guidance from someone with more XFS experience.  With you out on 
>> >> >> extended leave
>> >> >> the first half of 2017, who would be the best person to ask for this 
>> >> >> guidance?
>> >> >> Darrick?
>> >> >
>> >> > Yes, probably. :)
>> >> >
>> >> > I think where we left off with this (on the XFS side) is some sort of
>> >> > fallocate mode that would allocate blocks, zero them, and then set the
>> >> > DAX and PMEM_IMMUTABLE on-disk inode flags.  After that, you'd mmap the
>> >> > file and thereby gain the ability to control write persistents behavior
>> >> > without having to worry about fs metadata updates.  As an added plus, I
>> >> > think zeroing the pmem also clears media errors, or something like that.
>> >> >
>> >> >  Is that a reasonable starting point?  My memory is a little 
>> >> > foggy.
>> >> >
>> >> > Hmm, I see Dan just posted something about blockdev fallocate.  I'll go
>> >> > read that.
>> >>
>> >> That's for device-dax, which is basically a poor man's PMEM_IMMUTABLE
>> >> via a character device interface. It's useful for cases where you want
>> >> an entire nvdimm namespace/volume in "no fs-metadata to worry about"
>> >> mode.  But, for sub-allocations of a namespace and support for
>> >> existing tooling, PMEM_IMMUTABLE is much more usable.
>> >
>> > Well sure... but otoh I was thinking that it'd be pretty neat if we
>> > could use the same code regardless of whether the target file was a
>> > dax-device or an xfs file:
>> >
>> > fd = open("", O_RDWR);
>> > fstat(fd, ):
>> > fallocate(fd, FALLOC_FL_PMEM_IMMUTABLE, 0, statbuf.st_size);
>> > p = mmap(NULL, statbuf.st_size, PROT_READ | PROT_WRITE, fd, 0);
>> >
>> > *(p + 42) = 0xDEADBEEF;
>> > asm { clflush; } /* or whatever */
>> >
>> > ...so perhaps it would be a good idea to design the fallocate primitive
>> > around "prepare this fd for mmap-only pmem semantics" and let it the
>> > backend do zeroing and inode flag changes as necessary to make it
>> > happen.  We'd need to do some bikeshedding about what the other falloc
>> > flags mean when we're dealing with pmem files and devices, but I think
>> > we should try to keep the userland presentation the same unless there's
>> > a really good reason not to.
>>
>> It would be interesting to use fallocate to size device-dax files...
>
> No. device-dax needs to die, not poison a bunch of existing file and
> block device APIs and behaviours with special snowflakes.  Get
> DAX-enabled filesystems to do what you need, and get rid of this
> ugly, nasty hack.
>

Right, Christoph already killed fallocate for device-dax.

What we're looking for now is the next level of detail on how to get
started on PMEM_IMMUTABLE, as Ross asked a few messages back in this
thread, so we have a reasonable replacement for device-dax.

Re: [PATCH v3 13/15] livepatch: change to a per-task consistency model

[Josh Poimboeuf]

On Tue, Dec 20, 2016 at 06:32:46PM +0100, Petr Mladek wrote:
> On Thu 2016-12-08 12:08:38, Josh Poimboeuf wrote:
> > Change livepatch to use a basic per-task consistency model.  This is the
> > foundation which will eventually enable us to patch those ~10% of
> > security patches which change function or data semantics.  This is the
> > biggest remaining piece needed to make livepatch more generally useful.
> > 
> > [1] https://lkml.kernel.org/r/20141107140458.ga21...@suse.cz
> > 
> > Signed-off-by: Josh Poimboeuf 
> > ---
> > diff --git a/Documentation/livepatch/livepatch.txt 
> > b/Documentation/livepatch/livepatch.txt
> > index 6c43f6e..f87e742 100644
> > --- a/Documentation/livepatch/livepatch.txt
> > +++ b/Documentation/livepatch/livepatch.txt
> 
> I like the description.
> 
> Just a note that we will also need to review the section about
> limitations. But I am not sure that we want to do it in this patch.
> It might open a long discussion on its own.
> 
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > index 1a5a93c..8e06fe5 100644
> > --- a/include/linux/livepatch.h
> > +++ b/include/linux/livepatch.h
> > @@ -28,18 +28,40 @@
> >  
> >  #include 
> >  
> > +/* task patch states */
> > +#define KLP_UNDEFINED  -1
> > +#define KLP_UNPATCHED   0
> > +#define KLP_PATCHED 1
> > +
> >  /**
> >   * struct klp_func - function structure for live patching
> >   * @old_name:  name of the function to be patched
> >   * @new_func:  pointer to the patched function code
> >   * @old_sympos: a hint indicating which symbol position the old function
> >   * can be found (optional)
> > + * @immediate:  patch the func immediately, bypassing backtrace safety 
> > checks
> 
> There are more checks possible. I would use the same description
> as for klp_object.

Agreed.

> >   * @old_addr:  the address of the function being patched
> >   * @kobj:  kobject for sysfs resources
> >   * @stack_node:list node for klp_ops func_stack list
> >   * @old_size:  size of the old function
> >   * @new_size:  size of the new function
> >   * @patched:   the func has been added to the klp_ops list
> > + * @transition:the func is currently being applied or reverted
> > + *
> > @@ -86,6 +110,7 @@ struct klp_object {
> >   * struct klp_patch - patch structure for live patching
> >   * @mod:   reference to the live patch module
> >   * @objs:  object entries for kernel objects to be patched
> > + * @immediate:  patch all funcs immediately, bypassing safety mechanisms
> >   * @list:  list node for global list of registered patches
> >   * @kobj:  kobject for sysfs resources
> >   * @enabled:   the patch is enabled (but operation may be incomplete)
> 
> [...]
> 
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index fc160c6..22c0c01 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -424,7 +477,10 @@ static ssize_t enabled_store(struct kobject *kobj, 
> > struct kobj_attribute *attr,
> > goto err;
> > }
> >  
> > -   if (enabled) {
> > +   if (patch == klp_transition_patch) {
> > +   klp_reverse_transition();
> > +   mod_delayed_work(system_wq, _transition_work, 0);
> 
> I would put this mod_delayed_work() into klp_reverse_transition().
> Also I would put that schedule_delayed_work() into
> klp_try_complete_transition().
> 
> If I did not miss anything, it will allow to move the
> klp_transition_work code to transition.c where it logically
> belongs.

Makes sense, I'll see if I can move all the klp_transition_work code to
transition.c.

> > +   } else if (enabled) {
> > ret = __klp_enable_patch(patch);
> > if (ret)
> > goto err;
> 
> [...]
> 
> > diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
> > index 5efa262..e79ebb5 100644
> > --- a/kernel/livepatch/patch.c
> > +++ b/kernel/livepatch/patch.c
> > @@ -29,6 +29,7 @@
> >  #include 
> >  #include 
> >  #include "patch.h"
> > +#include "transition.h"
> >  
> >  static LIST_HEAD(klp_ops);
> >  
> > @@ -54,15 +55,53 @@ static void notrace klp_ftrace_handler(unsigned long ip,
> >  {
> > struct klp_ops *ops;
> > struct klp_func *func;
> > +   int patch_state;
> >  
> > ops = container_of(fops, struct klp_ops, fops);
> >  
> > rcu_read_lock();
> > +
> > func = list_first_or_null_rcu(>func_stack, struct klp_func,
> >   stack_node);
> > -   if (WARN_ON_ONCE(!func))
> > +
> > +   if (!func)
> > goto unlock;
> 
> Why do you removed the WARN_ON_ONCE(), please?
> 
> We still add the function on the stack before registering
> the ftrace handler. Also we unregister the ftrace handler
> before removing the the last entry from the stack.
> 
> AFAIK, unregister_ftrace_function() calls rcu_synchronize()'
> to make sure that no-one is inside the handler once finished.
> Mirek knows more about it.

Hm, 

Re: [PATCH v3 13/15] livepatch: change to a per-task consistency model

[Josh Poimboeuf]

On Tue, Dec 20, 2016 at 06:32:46PM +0100, Petr Mladek wrote:
> On Thu 2016-12-08 12:08:38, Josh Poimboeuf wrote:
> > Change livepatch to use a basic per-task consistency model.  This is the
> > foundation which will eventually enable us to patch those ~10% of
> > security patches which change function or data semantics.  This is the
> > biggest remaining piece needed to make livepatch more generally useful.
> > 
> > [1] https://lkml.kernel.org/r/20141107140458.ga21...@suse.cz
> > 
> > Signed-off-by: Josh Poimboeuf 
> > ---
> > diff --git a/Documentation/livepatch/livepatch.txt 
> > b/Documentation/livepatch/livepatch.txt
> > index 6c43f6e..f87e742 100644
> > --- a/Documentation/livepatch/livepatch.txt
> > +++ b/Documentation/livepatch/livepatch.txt
> 
> I like the description.
> 
> Just a note that we will also need to review the section about
> limitations. But I am not sure that we want to do it in this patch.
> It might open a long discussion on its own.
> 
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > index 1a5a93c..8e06fe5 100644
> > --- a/include/linux/livepatch.h
> > +++ b/include/linux/livepatch.h
> > @@ -28,18 +28,40 @@
> >  
> >  #include 
> >  
> > +/* task patch states */
> > +#define KLP_UNDEFINED  -1
> > +#define KLP_UNPATCHED   0
> > +#define KLP_PATCHED 1
> > +
> >  /**
> >   * struct klp_func - function structure for live patching
> >   * @old_name:  name of the function to be patched
> >   * @new_func:  pointer to the patched function code
> >   * @old_sympos: a hint indicating which symbol position the old function
> >   * can be found (optional)
> > + * @immediate:  patch the func immediately, bypassing backtrace safety 
> > checks
> 
> There are more checks possible. I would use the same description
> as for klp_object.

Agreed.

> >   * @old_addr:  the address of the function being patched
> >   * @kobj:  kobject for sysfs resources
> >   * @stack_node:list node for klp_ops func_stack list
> >   * @old_size:  size of the old function
> >   * @new_size:  size of the new function
> >   * @patched:   the func has been added to the klp_ops list
> > + * @transition:the func is currently being applied or reverted
> > + *
> > @@ -86,6 +110,7 @@ struct klp_object {
> >   * struct klp_patch - patch structure for live patching
> >   * @mod:   reference to the live patch module
> >   * @objs:  object entries for kernel objects to be patched
> > + * @immediate:  patch all funcs immediately, bypassing safety mechanisms
> >   * @list:  list node for global list of registered patches
> >   * @kobj:  kobject for sysfs resources
> >   * @enabled:   the patch is enabled (but operation may be incomplete)
> 
> [...]
> 
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index fc160c6..22c0c01 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -424,7 +477,10 @@ static ssize_t enabled_store(struct kobject *kobj, 
> > struct kobj_attribute *attr,
> > goto err;
> > }
> >  
> > -   if (enabled) {
> > +   if (patch == klp_transition_patch) {
> > +   klp_reverse_transition();
> > +   mod_delayed_work(system_wq, _transition_work, 0);
> 
> I would put this mod_delayed_work() into klp_reverse_transition().
> Also I would put that schedule_delayed_work() into
> klp_try_complete_transition().
> 
> If I did not miss anything, it will allow to move the
> klp_transition_work code to transition.c where it logically
> belongs.

Makes sense, I'll see if I can move all the klp_transition_work code to
transition.c.

> > +   } else if (enabled) {
> > ret = __klp_enable_patch(patch);
> > if (ret)
> > goto err;
> 
> [...]
> 
> > diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
> > index 5efa262..e79ebb5 100644
> > --- a/kernel/livepatch/patch.c
> > +++ b/kernel/livepatch/patch.c
> > @@ -29,6 +29,7 @@
> >  #include 
> >  #include 
> >  #include "patch.h"
> > +#include "transition.h"
> >  
> >  static LIST_HEAD(klp_ops);
> >  
> > @@ -54,15 +55,53 @@ static void notrace klp_ftrace_handler(unsigned long ip,
> >  {
> > struct klp_ops *ops;
> > struct klp_func *func;
> > +   int patch_state;
> >  
> > ops = container_of(fops, struct klp_ops, fops);
> >  
> > rcu_read_lock();
> > +
> > func = list_first_or_null_rcu(>func_stack, struct klp_func,
> >   stack_node);
> > -   if (WARN_ON_ONCE(!func))
> > +
> > +   if (!func)
> > goto unlock;
> 
> Why do you removed the WARN_ON_ONCE(), please?
> 
> We still add the function on the stack before registering
> the ftrace handler. Also we unregister the ftrace handler
> before removing the the last entry from the stack.
> 
> AFAIK, unregister_ftrace_function() calls rcu_synchronize()'
> to make sure that no-one is inside the handler once finished.
> Mirek knows more about it.

Hm, this is news to me.  

Re: DAX mapping detection (was: Re: [PATCH] Fix region lost in /proc/self/smaps)

[Dave Chinner]

On Wed, Dec 21, 2016 at 08:53:46AM -0800, Dan Williams wrote:
> On Tue, Dec 20, 2016 at 4:40 PM, Darrick J. Wong
>  wrote:
> > On Mon, Dec 19, 2016 at 05:18:40PM -0800, Dan Williams wrote:
> >> On Mon, Dec 19, 2016 at 5:09 PM, Darrick J. Wong
> >>  wrote:
> >> > On Mon, Dec 19, 2016 at 02:11:49PM -0700, Ross Zwisler wrote:
> >> >> On Fri, Sep 16, 2016 at 03:54:05PM +1000, Nicholas Piggin wrote:
> >> >> <>
> >> >> > Definitely the first step would be your simple preallocated per
> >> >> > inode approach until it is shown to be insufficient.
> >> >>
> >> >> Reviving this thread a few months later...
> >> >>
> >> >> Dave, we're interested in taking a serious look at what it would take 
> >> >> to get
> >> >> PMEM_IMMUTABLE working.  Do you still hold the opinion that this is (or 
> >> >> could
> >> >> become, with some amount of work) a workable solution?
> >> >>
> >> >> We're happy to do the grunt work for this feature, but we will probably 
> >> >> need
> >> >> guidance from someone with more XFS experience.  With you out on 
> >> >> extended leave
> >> >> the first half of 2017, who would be the best person to ask for this 
> >> >> guidance?
> >> >> Darrick?
> >> >
> >> > Yes, probably. :)
> >> >
> >> > I think where we left off with this (on the XFS side) is some sort of
> >> > fallocate mode that would allocate blocks, zero them, and then set the
> >> > DAX and PMEM_IMMUTABLE on-disk inode flags.  After that, you'd mmap the
> >> > file and thereby gain the ability to control write persistents behavior
> >> > without having to worry about fs metadata updates.  As an added plus, I
> >> > think zeroing the pmem also clears media errors, or something like that.
> >> >
> >> >  Is that a reasonable starting point?  My memory is a little 
> >> > foggy.
> >> >
> >> > Hmm, I see Dan just posted something about blockdev fallocate.  I'll go
> >> > read that.
> >>
> >> That's for device-dax, which is basically a poor man's PMEM_IMMUTABLE
> >> via a character device interface. It's useful for cases where you want
> >> an entire nvdimm namespace/volume in "no fs-metadata to worry about"
> >> mode.  But, for sub-allocations of a namespace and support for
> >> existing tooling, PMEM_IMMUTABLE is much more usable.
> >
> > Well sure... but otoh I was thinking that it'd be pretty neat if we
> > could use the same code regardless of whether the target file was a
> > dax-device or an xfs file:
> >
> > fd = open("", O_RDWR);
> > fstat(fd, ):
> > fallocate(fd, FALLOC_FL_PMEM_IMMUTABLE, 0, statbuf.st_size);
> > p = mmap(NULL, statbuf.st_size, PROT_READ | PROT_WRITE, fd, 0);
> >
> > *(p + 42) = 0xDEADBEEF;
> > asm { clflush; } /* or whatever */
> >
> > ...so perhaps it would be a good idea to design the fallocate primitive
> > around "prepare this fd for mmap-only pmem semantics" and let it the
> > backend do zeroing and inode flag changes as necessary to make it
> > happen.  We'd need to do some bikeshedding about what the other falloc
> > flags mean when we're dealing with pmem files and devices, but I think
> > we should try to keep the userland presentation the same unless there's
> > a really good reason not to.
> 
> It would be interesting to use fallocate to size device-dax files...

No. device-dax needs to die, not poison a bunch of existing file and
block device APIs and behaviours with special snowflakes.  Get
DAX-enabled filesystems to do what you need, and get rid of this
ugly, nasty hack.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: DAX mapping detection (was: Re: [PATCH] Fix region lost in /proc/self/smaps)

[Dave Chinner]

On Wed, Dec 21, 2016 at 08:53:46AM -0800, Dan Williams wrote:
> On Tue, Dec 20, 2016 at 4:40 PM, Darrick J. Wong
>  wrote:
> > On Mon, Dec 19, 2016 at 05:18:40PM -0800, Dan Williams wrote:
> >> On Mon, Dec 19, 2016 at 5:09 PM, Darrick J. Wong
> >>  wrote:
> >> > On Mon, Dec 19, 2016 at 02:11:49PM -0700, Ross Zwisler wrote:
> >> >> On Fri, Sep 16, 2016 at 03:54:05PM +1000, Nicholas Piggin wrote:
> >> >> <>
> >> >> > Definitely the first step would be your simple preallocated per
> >> >> > inode approach until it is shown to be insufficient.
> >> >>
> >> >> Reviving this thread a few months later...
> >> >>
> >> >> Dave, we're interested in taking a serious look at what it would take 
> >> >> to get
> >> >> PMEM_IMMUTABLE working.  Do you still hold the opinion that this is (or 
> >> >> could
> >> >> become, with some amount of work) a workable solution?
> >> >>
> >> >> We're happy to do the grunt work for this feature, but we will probably 
> >> >> need
> >> >> guidance from someone with more XFS experience.  With you out on 
> >> >> extended leave
> >> >> the first half of 2017, who would be the best person to ask for this 
> >> >> guidance?
> >> >> Darrick?
> >> >
> >> > Yes, probably. :)
> >> >
> >> > I think where we left off with this (on the XFS side) is some sort of
> >> > fallocate mode that would allocate blocks, zero them, and then set the
> >> > DAX and PMEM_IMMUTABLE on-disk inode flags.  After that, you'd mmap the
> >> > file and thereby gain the ability to control write persistents behavior
> >> > without having to worry about fs metadata updates.  As an added plus, I
> >> > think zeroing the pmem also clears media errors, or something like that.
> >> >
> >> >  Is that a reasonable starting point?  My memory is a little 
> >> > foggy.
> >> >
> >> > Hmm, I see Dan just posted something about blockdev fallocate.  I'll go
> >> > read that.
> >>
> >> That's for device-dax, which is basically a poor man's PMEM_IMMUTABLE
> >> via a character device interface. It's useful for cases where you want
> >> an entire nvdimm namespace/volume in "no fs-metadata to worry about"
> >> mode.  But, for sub-allocations of a namespace and support for
> >> existing tooling, PMEM_IMMUTABLE is much more usable.
> >
> > Well sure... but otoh I was thinking that it'd be pretty neat if we
> > could use the same code regardless of whether the target file was a
> > dax-device or an xfs file:
> >
> > fd = open("", O_RDWR);
> > fstat(fd, ):
> > fallocate(fd, FALLOC_FL_PMEM_IMMUTABLE, 0, statbuf.st_size);
> > p = mmap(NULL, statbuf.st_size, PROT_READ | PROT_WRITE, fd, 0);
> >
> > *(p + 42) = 0xDEADBEEF;
> > asm { clflush; } /* or whatever */
> >
> > ...so perhaps it would be a good idea to design the fallocate primitive
> > around "prepare this fd for mmap-only pmem semantics" and let it the
> > backend do zeroing and inode flag changes as necessary to make it
> > happen.  We'd need to do some bikeshedding about what the other falloc
> > flags mean when we're dealing with pmem files and devices, but I think
> > we should try to keep the userland presentation the same unless there's
> > a really good reason not to.
> 
> It would be interesting to use fallocate to size device-dax files...

No. device-dax needs to die, not poison a bunch of existing file and
block device APIs and behaviours with special snowflakes.  Get
DAX-enabled filesystems to do what you need, and get rid of this
ugly, nasty hack.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: Patch to include/linux/kernel.h breaks 3rd party modules.

[Jessica Yu]

+++ Valdis Kletnieks [21/12/16 15:42 -0500]:

Yes, I know that usually out-of-tree modules are on their own.
However, this one may require a rethink..

(Sorry for not catching this sooner, I hadn't tried to deal with the
affected module since this patch hit linux-next in next-20161128)

commit 7fd8329ba502ef76dd91db561c7aed696b2c7720
Author: Petr Mladek 
Date:   Wed Sep 21 13:47:22 2016 +0200

   taint/module: Clean up global and module taint flags handling

Contains this chunk:

--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -506,6 +506,15 @@ extern enum system_states {
#define TAINT_UNSIGNED_MODULE  13
#define TAINT_SOFTLOCKUP   14
#define TAINT_LIVEPATCH15
+#define TAINT_FLAGS_COUNT  16
+
+struct taint_flag {
+   char true;  /* character printed when tainted */
+   char false; /* character printed when not tainted */
+   bool module;/* also show as a per-module taint flag */
+};

and hilarity ensues when an out-of-tree module has this:

# ifndef true
#  define true  (1)
# endif
# ifndef false
#  define false (0)
# endif

My proposed fix: change true/false to tainted/untainted.  If this
is agreeable, I'll code and submit the fix.


Sure, that's fine with me.

Jessica

Re: Patch to include/linux/kernel.h breaks 3rd party modules.

[Jessica Yu]

+++ Valdis Kletnieks [21/12/16 15:42 -0500]:

Yes, I know that usually out-of-tree modules are on their own.
However, this one may require a rethink..

(Sorry for not catching this sooner, I hadn't tried to deal with the
affected module since this patch hit linux-next in next-20161128)

commit 7fd8329ba502ef76dd91db561c7aed696b2c7720
Author: Petr Mladek 
Date:   Wed Sep 21 13:47:22 2016 +0200

   taint/module: Clean up global and module taint flags handling

Contains this chunk:

--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -506,6 +506,15 @@ extern enum system_states {
#define TAINT_UNSIGNED_MODULE  13
#define TAINT_SOFTLOCKUP   14
#define TAINT_LIVEPATCH15
+#define TAINT_FLAGS_COUNT  16
+
+struct taint_flag {
+   char true;  /* character printed when tainted */
+   char false; /* character printed when not tainted */
+   bool module;/* also show as a per-module taint flag */
+};

and hilarity ensues when an out-of-tree module has this:

# ifndef true
#  define true  (1)
# endif
# ifndef false
#  define false (0)
# endif

My proposed fix: change true/false to tainted/untainted.  If this
is agreeable, I'll code and submit the fix.


Sure, that's fine with me.

Jessica

Re: [PATCH] at86rf230: Allow slow GPIO pins for "rstn"

[Stefan Schmidt]

Hello.

On 21/12/16 19:30, Chris Healy wrote:



On Dec 21, 2016 5:11 AM, "Stefan Schmidt" > wrote:

Hello.


On 19/12/16 00:25, Andrey Smirnov wrote:

Driver code never touches "rstn" signal in atomic context, so
there's
no need to implicitly put such restriction on it by using
gpio_set_value
to manipulate it. Replace gpio_set_value to
gpio_set_value_cansleep to
fix that.


We need to make sure we are not assuming it can be called  in such a
context in the future now. But that is something we can worry about
if it comes up.


As a an example of where such restriction might be inconvenient,
consider a hardware design where "rstn" is connected to a pin of
I2C/SPI
GPIO expander chip.


Is this a real life issue you run into?


I have a platform with this configuration.  The DTS for the platform is
in the process of being mainlined right now.


Thanks for letting us know. What platform is that? I'm always interested 
in hearing about devices that use the Linux ieee802154 subsystem. :)


regards
Stefan Schmidt

Re: [PATCH] at86rf230: Allow slow GPIO pins for "rstn"

[Stefan Schmidt]

Hello.

On 21/12/16 19:30, Chris Healy wrote:



On Dec 21, 2016 5:11 AM, "Stefan Schmidt" mailto:ste...@osg.samsung.com>> wrote:

Hello.


On 19/12/16 00:25, Andrey Smirnov wrote:

Driver code never touches "rstn" signal in atomic context, so
there's
no need to implicitly put such restriction on it by using
gpio_set_value
to manipulate it. Replace gpio_set_value to
gpio_set_value_cansleep to
fix that.


We need to make sure we are not assuming it can be called  in such a
context in the future now. But that is something we can worry about
if it comes up.


As a an example of where such restriction might be inconvenient,
consider a hardware design where "rstn" is connected to a pin of
I2C/SPI
GPIO expander chip.


Is this a real life issue you run into?


I have a platform with this configuration.  The DTS for the platform is
in the process of being mainlined right now.


Thanks for letting us know. What platform is that? I'm always interested 
in hearing about devices that use the Linux ieee802154 subsystem. :)


regards
Stefan Schmidt

Re: WARNING: CPU: 3 PID: 1568 at kernel/sched/core.c:7738 __might_sleep+0x69/0x7e

[Michal Hocko]

On Wed 21-12-16 11:24:47, Cong Wang wrote:
> On Wed, Dec 21, 2016 at 2:47 AM, Michal Hocko  wrote:
> >
> > Is there anything I could test?
> 
> I am working on a patchset, will send them for you to test. Meanwhile,
> if you could provide me a reproducer, that would help a lot.

I just boot my kvm image. Nothing really special here.
-- 
Michal Hocko
SUSE Labs

Re: WARNING: CPU: 3 PID: 1568 at kernel/sched/core.c:7738 __might_sleep+0x69/0x7e

[Michal Hocko]

On Wed 21-12-16 11:24:47, Cong Wang wrote:
> On Wed, Dec 21, 2016 at 2:47 AM, Michal Hocko  wrote:
> >
> > Is there anything I could test?
> 
> I am working on a patchset, will send them for you to test. Meanwhile,
> if you could provide me a reproducer, that would help a lot.

I just boot my kvm image. Nothing really special here.
-- 
Michal Hocko
SUSE Labs

Patch to include/linux/kernel.h breaks 3rd party modules.

[Valdis Kletnieks]

Yes, I know that usually out-of-tree modules are on their own.
However, this one may require a rethink..

(Sorry for not catching this sooner, I hadn't tried to deal with the
affected module since this patch hit linux-next in next-20161128)

commit 7fd8329ba502ef76dd91db561c7aed696b2c7720
Author: Petr Mladek 
Date:   Wed Sep 21 13:47:22 2016 +0200

taint/module: Clean up global and module taint flags handling

Contains this chunk:

--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -506,6 +506,15 @@ extern enum system_states {
 #define TAINT_UNSIGNED_MODULE  13
 #define TAINT_SOFTLOCKUP   14
 #define TAINT_LIVEPATCH15
+#define TAINT_FLAGS_COUNT  16
+
+struct taint_flag {
+   char true;  /* character printed when tainted */
+   char false; /* character printed when not tainted */
+   bool module;/* also show as a per-module taint flag */
+};

and hilarity ensues when an out-of-tree module has this:

# ifndef true
#  define true  (1)
# endif
# ifndef false
#  define false (0)
# endif

My proposed fix: change true/false to tainted/untainted.  If this
is agreeable, I'll code and submit the fix.




pgpPygTsuG3dE.pgp
Description: PGP signature

Patch to include/linux/kernel.h breaks 3rd party modules.

[Valdis Kletnieks]

Yes, I know that usually out-of-tree modules are on their own.
However, this one may require a rethink..

(Sorry for not catching this sooner, I hadn't tried to deal with the
affected module since this patch hit linux-next in next-20161128)

commit 7fd8329ba502ef76dd91db561c7aed696b2c7720
Author: Petr Mladek 
Date:   Wed Sep 21 13:47:22 2016 +0200

taint/module: Clean up global and module taint flags handling

Contains this chunk:

--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -506,6 +506,15 @@ extern enum system_states {
 #define TAINT_UNSIGNED_MODULE  13
 #define TAINT_SOFTLOCKUP   14
 #define TAINT_LIVEPATCH15
+#define TAINT_FLAGS_COUNT  16
+
+struct taint_flag {
+   char true;  /* character printed when tainted */
+   char false; /* character printed when not tainted */
+   bool module;/* also show as a per-module taint flag */
+};

and hilarity ensues when an out-of-tree module has this:

# ifndef true
#  define true  (1)
# endif
# ifndef false
#  define false (0)
# endif

My proposed fix: change true/false to tainted/untainted.  If this
is agreeable, I'll code and submit the fix.




pgpPygTsuG3dE.pgp
Description: PGP signature

Re: [PATCH] net: fddi: skfp: use %p format specifier for addresses rather than %x

[David Miller]

From: Colin King 
Date: Wed, 21 Dec 2016 16:03:23 +

> From: Colin Ian King 
> 
> Trivial fix: Addresses should be printed using the %p format specifier
> rather than using %x.
> 
> Signed-off-by: Colin Ian King 

Applied.

Re: [PATCH] net: fddi: skfp: use %p format specifier for addresses rather than %x

[David Miller]

From: Colin King 
Date: Wed, 21 Dec 2016 16:03:23 +

> From: Colin Ian King 
> 
> Trivial fix: Addresses should be printed using the %p format specifier
> rather than using %x.
> 
> Signed-off-by: Colin Ian King 

Applied.

Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Jacek Anaszewski]

Hi,

On 12/21/2016 07:49 PM, Pavel Machek wrote:
> Hi!
> 
> Milo if sysfs is used can't the old userspace be mapped to use the new
> sysfs interface through a wrapper of some sort ? What exactly would be
> needed to ensure old userspace will not break?

 LP5521 and LP5523 have two ways to load hex code from the userspace - the
 sysfs and firmware I/F. So user program supports both interfaces. Even if
 the firmware I/F is not available, user can still run LED effect through 
 the
 sysfs.

 However, LP5562 and LP8501 support only single way which is the firmware
 I/F. So user-space program for LP5562/8501 should be modified if lp55xx
 removes the interface. My idea is
>>>
>>> Actually... it would be good to have some reasonable interface for RGB
>>> LEDs. This way, we need separate "firmware" for each LED
>>> controller. It would be good to have common format for LED effects.
>>
>> We still haven't tried trigger approach discussed over half a year ago.
>> If we used firmware approach we would still have to overcome the problem
>> of defining the LED class drivers affected by the firmware program.
> 
> The firmware approach is in the tree today :-(.

to RGB LEDs? What exactly do you have on mind?

> 
 Device manufactures in Asia & North America requested lp55xx drivers, but I
 don't know how many vendors uses the firmware I/F. Some vendors embeds the
 binary code inside the driver instead of using user-program.
>>>
>>> Nokia N900 uses lp55xx, and I have custom scripts interfacing sysfs.
>>>
>>> Maemo uses the LEDs, too, but maemo is not open source.
>>>
>>> So no, I don't think there's anything important that could be broken.
>>
>> We can't guarantee that. Is there any problem in just using the
>> currently introduced DECLARE_FW_CUSTOM_FALLBACK() in
>> drivers/leds/leds-lp55xx-common.c?
> 
> Well, it would be good to get rid of the custom fallback
> functionality. And no, we don't need to "guarantee" that.  Removing
> obscure functionality noone uses is far game... providing noone
> complains ;-).

As Milo explained:

> Why has no one cried
> after the v4.0 custom fallback mechanism breaking ?

"Well, I don't know the reason exactly but my guess is they maybe still
using old kernel."

and after that:

"Device manufactures in Asia & North America requested lp55xx drivers"

These should be sufficient arguments for us for keeping the API
unchanged. If the users decided to upgrade their kernel then they
would be surprised by the API change.

DECLARE_FW_CUSTOM_FALLBACK macro seems to have been designed for
handling exactly this type of cases.

-- 
Best regards,
Jacek Anaszewski

[PATCH v4 3/3] Bluetooth: btusb: Configure Marvell to use one of the pins for oob wakeup

[Rajat Jain]

The Marvell devices may have many gpio pins, and hence for wakeup
on these out-of-band pins, the chip needs to be told which pin is
to be used for wakeup, using an hci command.

Thus, we read the pin number etc from the device tree node and send
a command to the chip.

Signed-off-by: Rajat Jain 
Reviewed-by: Brian Norris 
Acked-by: Rob Herring 
---
v4: same as v3
v3: * remove the Marvell specific id table and check
* Add reference to marvell-bt-8xxx.txt in btusb.txt
* Add "Reviewed-by" and "Acked-by"
v2: Fix the binding document to specify to use "wakeup" interrupt-name

 Documentation/devicetree/bindings/net/btusb.txt|  3 ++
 .../{marvell-bt-sd8xxx.txt => marvell-bt-8xxx.txt} | 46 +++
 drivers/bluetooth/btusb.c  | 51 ++
 3 files changed, 92 insertions(+), 8 deletions(-)
 rename Documentation/devicetree/bindings/net/{marvell-bt-sd8xxx.txt => 
marvell-bt-8xxx.txt} (50%)

diff --git a/Documentation/devicetree/bindings/net/btusb.txt 
b/Documentation/devicetree/bindings/net/btusb.txt
index 2c0355c..01fa2d4 100644
--- a/Documentation/devicetree/bindings/net/btusb.txt
+++ b/Documentation/devicetree/bindings/net/btusb.txt
@@ -10,6 +10,9 @@ Required properties:
 
  "usb1286,204e" (Marvell 8997)
 
+Also, vendors that use btusb may have device additional properties, e.g:
+Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
+
 Optional properties:
 
   - interrupt-parent: phandle of the parent interrupt controller
diff --git a/Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt 
b/Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
similarity index 50%
rename from Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt
rename to Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
index 6a9a63c..9be1059 100644
--- a/Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt
+++ b/Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
@@ -1,16 +1,21 @@
-Marvell 8897/8997 (sd8897/sd8997) bluetooth SDIO devices
+Marvell 8897/8997 (sd8897/sd8997) bluetooth devices (SDIO or USB based)
 --
+The 8997 devices supports multiple interfaces. When used on SDIO interfaces,
+the btmrvl driver is used and when used on USB interface, the btusb driver is
+used.
 
 Required properties:
 
   - compatible : should be one of the following:
-   * "marvell,sd8897-bt"
-   * "marvell,sd8997-bt"
+   * "marvell,sd8897-bt" (for SDIO)
+   * "marvell,sd8997-bt" (for SDIO)
+   * "usb1286,204e"  (for USB)
 
 Optional properties:
 
   - marvell,cal-data: Calibration data downloaded to the device during
  initialization. This is an array of 28 values(u8).
+ This is only applicable to SDIO devices.
 
   - marvell,wakeup-pin: It represents wakeup pin number of the bluetooth chip.
firmware will use the pin to wakeup host system (u16).
@@ -18,10 +23,15 @@ Optional properties:
  platform. The value will be configured to firmware. This
  is needed to work chip's sleep feature as expected (u16).
   - interrupt-parent: phandle of the parent interrupt controller
-  - interrupts : interrupt pin number to the cpu. Driver will request an irq 
based
-on this interrupt number. During system suspend, the irq will 
be
-enabled so that the bluetooth chip can wakeup host platform 
under
-certain condition. During system resume, the irq will be 
disabled
+  - interrupt-names: Used only for USB based devices (See below)
+  - interrupts : specifies the interrupt pin number to the cpu. For SDIO, the
+driver will use the first interrupt specified in the interrupt
+array. For USB based devices, the driver will use the interrupt
+named "wakeup" from the interrupt-names and interrupt arrays.
+The driver will request an irq based on this interrupt number.
+During system suspend, the irq will be enabled so that the
+bluetooth chip can wakeup host platform under certain
+conditions. During system resume, the irq will be disabled
 to make sure unnecessary interrupt is not received.
 
 Example:
@@ -29,7 +39,9 @@ Example:
 IRQ pin 119 is used as system wakeup source interrupt.
 wakeup pin 13 and gap 100ms are configured so that firmware can wakeup host
 using this device side pin and wakeup latency.
-calibration data is also available in below example.
+
+Example for SDIO device follows (calibration data is also available in
+below example).
 
  {
status = "okay";
@@ -54,3 +66,21 @@ calibration data is also available in below example.
marvell,wakeup-gap-ms = /bits/ 16 <0x64>;
};
 };
+
+Example for USB device:
+
+_host1_ohci {
+status = "okay";
+

[PATCH v4 2/3] Bluetooth: btusb: Add out-of-band wakeup support

[Rajat Jain]

Some onboard BT chips (e.g. Marvell 8997) contain a wakeup pin that
can be connected to a gpio on the CPU side, and can be used to wakeup
the host out-of-band. This can be useful in situations where the
in-band wakeup is not possible or not preferable (e.g. the in-band
wakeup may require the USB host controller to remain active, and
hence consuming more system power during system sleep).

The oob gpio interrupt to be used for wakeup on the CPU side, is
read from the device tree node, (using standard interrupt descriptors).
A devcie tree binding document is also added for the driver. The
compatible string is in compliance with
Documentation/devicetree/bindings/usb/usb-device.txt

Signed-off-by: Rajat Jain 
Reviewed-by: Brian Norris 
---
v4: Move the set_bit(BTUSB_OOB_WAKE_DISABLED,..) call to the beginning of
btusb_config_oob_wake() - caught by Brian.
v3: Add Brian's "Reviewed-by"
v2: * Use interrupt-names ("wakeup") instead of assuming first interrupt.
* Leave it on device tree to specify IRQ flags (level /edge triggered)
* Mark the device as non wakeable on exit.

 Documentation/devicetree/bindings/net/btusb.txt | 40 
 drivers/bluetooth/btusb.c   | 84 +
 2 files changed, 124 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/btusb.txt

diff --git a/Documentation/devicetree/bindings/net/btusb.txt 
b/Documentation/devicetree/bindings/net/btusb.txt
new file mode 100644
index 000..2c0355c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/btusb.txt
@@ -0,0 +1,40 @@
+Generic Bluetooth controller over USB (btusb driver)
+---
+
+Required properties:
+
+  - compatible : should comply with the format "usbVID,PID" specified in
+Documentation/devicetree/bindings/usb/usb-device.txt
+At the time of writing, the only OF supported devices
+(more may be added later) are:
+
+ "usb1286,204e" (Marvell 8997)
+
+Optional properties:
+
+  - interrupt-parent: phandle of the parent interrupt controller
+  - interrupt-names: (see below)
+  - interrupts : The interrupt specified by the name "wakeup" is the interrupt
+that shall be used for out-of-band wake-on-bt. Driver will
+request this interrupt for wakeup. During system suspend, the
+irq will be enabled so that the bluetooth chip can wakeup host
+platform out of band. During system resume, the irq will be
+disabled to make sure unnecessary interrupt is not received.
+
+Example:
+
+Following example uses irq pin number 3 of gpio0 for out of band wake-on-bt:
+
+_host1_ehci {
+status = "okay";
+#address-cells = <1>;
+#size-cells = <0>;
+
+mvl_bt1: bt@1 {
+   compatible = "usb1286,204e";
+   reg = <1>;
+   interrupt-parent = <>;
+   interrupt-name = "wakeup";
+   interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+};
+};
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index ce22cef..2b45f1c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -24,6 +24,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include 
@@ -369,6 +371,7 @@ static const struct usb_device_id blacklist_table[] = {
 #define BTUSB_BOOTING  9
 #define BTUSB_RESET_RESUME 10
 #define BTUSB_DIAG_RUNNING 11
+#define BTUSB_OOB_WAKE_DISABLED12
 
 struct btusb_data {
struct hci_dev   *hdev;
@@ -416,6 +419,8 @@ struct btusb_data {
int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
 
int (*setup_on_usb)(struct hci_dev *hdev);
+
+   int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
 };
 
 static inline void btusb_free_frags(struct btusb_data *data)
@@ -2728,6 +2733,65 @@ static int btusb_bcm_set_diag(struct hci_dev *hdev, bool 
enable)
 }
 #endif
 
+#ifdef CONFIG_PM
+static irqreturn_t btusb_oob_wake_handler(int irq, void *priv)
+{
+   struct btusb_data *data = priv;
+
+   /* Disable only if not already disabled (keep it balanced) */
+   if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, >flags)) {
+   disable_irq_nosync(irq);
+   disable_irq_wake(irq);
+   }
+   pm_wakeup_event(>udev->dev, 0);
+   return IRQ_HANDLED;
+}
+
+static const struct of_device_id btusb_match_table[] = {
+   { .compatible = "usb1286,204e" },
+   { }
+};
+MODULE_DEVICE_TABLE(of, btusb_match_table);
+
+/* Use an oob wakeup pin? */
+static int btusb_config_oob_wake(struct hci_dev *hdev)
+{
+   struct btusb_data *data = hci_get_drvdata(hdev);
+   struct device *dev = >udev->dev;
+   int irq, ret;
+
+   set_bit(BTUSB_OOB_WAKE_DISABLED, >flags);
+
+   if (!of_match_device(btusb_match_table, dev))
+   return 0;
+
+   /* Move on if no IRQ specified */
+   irq = 

[PATCH v4 3/3] Bluetooth: btusb: Configure Marvell to use one of the pins for oob wakeup

[Rajat Jain]

The Marvell devices may have many gpio pins, and hence for wakeup
on these out-of-band pins, the chip needs to be told which pin is
to be used for wakeup, using an hci command.

Thus, we read the pin number etc from the device tree node and send
a command to the chip.

Signed-off-by: Rajat Jain 
Reviewed-by: Brian Norris 
Acked-by: Rob Herring 
---
v4: same as v3
v3: * remove the Marvell specific id table and check
* Add reference to marvell-bt-8xxx.txt in btusb.txt
* Add "Reviewed-by" and "Acked-by"
v2: Fix the binding document to specify to use "wakeup" interrupt-name

 Documentation/devicetree/bindings/net/btusb.txt|  3 ++
 .../{marvell-bt-sd8xxx.txt => marvell-bt-8xxx.txt} | 46 +++
 drivers/bluetooth/btusb.c  | 51 ++
 3 files changed, 92 insertions(+), 8 deletions(-)
 rename Documentation/devicetree/bindings/net/{marvell-bt-sd8xxx.txt => 
marvell-bt-8xxx.txt} (50%)

diff --git a/Documentation/devicetree/bindings/net/btusb.txt 
b/Documentation/devicetree/bindings/net/btusb.txt
index 2c0355c..01fa2d4 100644
--- a/Documentation/devicetree/bindings/net/btusb.txt
+++ b/Documentation/devicetree/bindings/net/btusb.txt
@@ -10,6 +10,9 @@ Required properties:
 
  "usb1286,204e" (Marvell 8997)
 
+Also, vendors that use btusb may have device additional properties, e.g:
+Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
+
 Optional properties:
 
   - interrupt-parent: phandle of the parent interrupt controller
diff --git a/Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt 
b/Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
similarity index 50%
rename from Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt
rename to Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
index 6a9a63c..9be1059 100644
--- a/Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt
+++ b/Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
@@ -1,16 +1,21 @@
-Marvell 8897/8997 (sd8897/sd8997) bluetooth SDIO devices
+Marvell 8897/8997 (sd8897/sd8997) bluetooth devices (SDIO or USB based)
 --
+The 8997 devices supports multiple interfaces. When used on SDIO interfaces,
+the btmrvl driver is used and when used on USB interface, the btusb driver is
+used.
 
 Required properties:
 
   - compatible : should be one of the following:
-   * "marvell,sd8897-bt"
-   * "marvell,sd8997-bt"
+   * "marvell,sd8897-bt" (for SDIO)
+   * "marvell,sd8997-bt" (for SDIO)
+   * "usb1286,204e"  (for USB)
 
 Optional properties:
 
   - marvell,cal-data: Calibration data downloaded to the device during
  initialization. This is an array of 28 values(u8).
+ This is only applicable to SDIO devices.
 
   - marvell,wakeup-pin: It represents wakeup pin number of the bluetooth chip.
firmware will use the pin to wakeup host system (u16).
@@ -18,10 +23,15 @@ Optional properties:
  platform. The value will be configured to firmware. This
  is needed to work chip's sleep feature as expected (u16).
   - interrupt-parent: phandle of the parent interrupt controller
-  - interrupts : interrupt pin number to the cpu. Driver will request an irq 
based
-on this interrupt number. During system suspend, the irq will 
be
-enabled so that the bluetooth chip can wakeup host platform 
under
-certain condition. During system resume, the irq will be 
disabled
+  - interrupt-names: Used only for USB based devices (See below)
+  - interrupts : specifies the interrupt pin number to the cpu. For SDIO, the
+driver will use the first interrupt specified in the interrupt
+array. For USB based devices, the driver will use the interrupt
+named "wakeup" from the interrupt-names and interrupt arrays.
+The driver will request an irq based on this interrupt number.
+During system suspend, the irq will be enabled so that the
+bluetooth chip can wakeup host platform under certain
+conditions. During system resume, the irq will be disabled
 to make sure unnecessary interrupt is not received.
 
 Example:
@@ -29,7 +39,9 @@ Example:
 IRQ pin 119 is used as system wakeup source interrupt.
 wakeup pin 13 and gap 100ms are configured so that firmware can wakeup host
 using this device side pin and wakeup latency.
-calibration data is also available in below example.
+
+Example for SDIO device follows (calibration data is also available in
+below example).
 
  {
status = "okay";
@@ -54,3 +66,21 @@ calibration data is also available in below example.
marvell,wakeup-gap-ms = /bits/ 16 <0x64>;
};
 };
+
+Example for USB device:
+
+_host1_ohci {
+status = "okay";
+#address-cells = <1>;
+#size-cells = <0>;
+
+mvl_bt1: bt@1 {
+

[PATCH v4 2/3] Bluetooth: btusb: Add out-of-band wakeup support

[Rajat Jain]

Some onboard BT chips (e.g. Marvell 8997) contain a wakeup pin that
can be connected to a gpio on the CPU side, and can be used to wakeup
the host out-of-band. This can be useful in situations where the
in-band wakeup is not possible or not preferable (e.g. the in-band
wakeup may require the USB host controller to remain active, and
hence consuming more system power during system sleep).

The oob gpio interrupt to be used for wakeup on the CPU side, is
read from the device tree node, (using standard interrupt descriptors).
A devcie tree binding document is also added for the driver. The
compatible string is in compliance with
Documentation/devicetree/bindings/usb/usb-device.txt

Signed-off-by: Rajat Jain 
Reviewed-by: Brian Norris 
---
v4: Move the set_bit(BTUSB_OOB_WAKE_DISABLED,..) call to the beginning of
btusb_config_oob_wake() - caught by Brian.
v3: Add Brian's "Reviewed-by"
v2: * Use interrupt-names ("wakeup") instead of assuming first interrupt.
* Leave it on device tree to specify IRQ flags (level /edge triggered)
* Mark the device as non wakeable on exit.

 Documentation/devicetree/bindings/net/btusb.txt | 40 
 drivers/bluetooth/btusb.c   | 84 +
 2 files changed, 124 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/btusb.txt

diff --git a/Documentation/devicetree/bindings/net/btusb.txt 
b/Documentation/devicetree/bindings/net/btusb.txt
new file mode 100644
index 000..2c0355c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/btusb.txt
@@ -0,0 +1,40 @@
+Generic Bluetooth controller over USB (btusb driver)
+---
+
+Required properties:
+
+  - compatible : should comply with the format "usbVID,PID" specified in
+Documentation/devicetree/bindings/usb/usb-device.txt
+At the time of writing, the only OF supported devices
+(more may be added later) are:
+
+ "usb1286,204e" (Marvell 8997)
+
+Optional properties:
+
+  - interrupt-parent: phandle of the parent interrupt controller
+  - interrupt-names: (see below)
+  - interrupts : The interrupt specified by the name "wakeup" is the interrupt
+that shall be used for out-of-band wake-on-bt. Driver will
+request this interrupt for wakeup. During system suspend, the
+irq will be enabled so that the bluetooth chip can wakeup host
+platform out of band. During system resume, the irq will be
+disabled to make sure unnecessary interrupt is not received.
+
+Example:
+
+Following example uses irq pin number 3 of gpio0 for out of band wake-on-bt:
+
+_host1_ehci {
+status = "okay";
+#address-cells = <1>;
+#size-cells = <0>;
+
+mvl_bt1: bt@1 {
+   compatible = "usb1286,204e";
+   reg = <1>;
+   interrupt-parent = <>;
+   interrupt-name = "wakeup";
+   interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+};
+};
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index ce22cef..2b45f1c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -24,6 +24,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include 
@@ -369,6 +371,7 @@ static const struct usb_device_id blacklist_table[] = {
 #define BTUSB_BOOTING  9
 #define BTUSB_RESET_RESUME 10
 #define BTUSB_DIAG_RUNNING 11
+#define BTUSB_OOB_WAKE_DISABLED12
 
 struct btusb_data {
struct hci_dev   *hdev;
@@ -416,6 +419,8 @@ struct btusb_data {
int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
 
int (*setup_on_usb)(struct hci_dev *hdev);
+
+   int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
 };
 
 static inline void btusb_free_frags(struct btusb_data *data)
@@ -2728,6 +2733,65 @@ static int btusb_bcm_set_diag(struct hci_dev *hdev, bool 
enable)
 }
 #endif
 
+#ifdef CONFIG_PM
+static irqreturn_t btusb_oob_wake_handler(int irq, void *priv)
+{
+   struct btusb_data *data = priv;
+
+   /* Disable only if not already disabled (keep it balanced) */
+   if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, >flags)) {
+   disable_irq_nosync(irq);
+   disable_irq_wake(irq);
+   }
+   pm_wakeup_event(>udev->dev, 0);
+   return IRQ_HANDLED;
+}
+
+static const struct of_device_id btusb_match_table[] = {
+   { .compatible = "usb1286,204e" },
+   { }
+};
+MODULE_DEVICE_TABLE(of, btusb_match_table);
+
+/* Use an oob wakeup pin? */
+static int btusb_config_oob_wake(struct hci_dev *hdev)
+{
+   struct btusb_data *data = hci_get_drvdata(hdev);
+   struct device *dev = >udev->dev;
+   int irq, ret;
+
+   set_bit(BTUSB_OOB_WAKE_DISABLED, >flags);
+
+   if (!of_match_device(btusb_match_table, dev))
+   return 0;
+
+   /* Move on if no IRQ specified */
+   irq = of_irq_get_byname(dev->of_node, "wakeup");
+  

Re: [PATCH 5/5] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

[Jacek Anaszewski]

Hi,

On 12/21/2016 07:49 PM, Pavel Machek wrote:
> Hi!
> 
> Milo if sysfs is used can't the old userspace be mapped to use the new
> sysfs interface through a wrapper of some sort ? What exactly would be
> needed to ensure old userspace will not break?

 LP5521 and LP5523 have two ways to load hex code from the userspace - the
 sysfs and firmware I/F. So user program supports both interfaces. Even if
 the firmware I/F is not available, user can still run LED effect through 
 the
 sysfs.

 However, LP5562 and LP8501 support only single way which is the firmware
 I/F. So user-space program for LP5562/8501 should be modified if lp55xx
 removes the interface. My idea is
>>>
>>> Actually... it would be good to have some reasonable interface for RGB
>>> LEDs. This way, we need separate "firmware" for each LED
>>> controller. It would be good to have common format for LED effects.
>>
>> We still haven't tried trigger approach discussed over half a year ago.
>> If we used firmware approach we would still have to overcome the problem
>> of defining the LED class drivers affected by the firmware program.
> 
> The firmware approach is in the tree today :-(.

to RGB LEDs? What exactly do you have on mind?

> 
 Device manufactures in Asia & North America requested lp55xx drivers, but I
 don't know how many vendors uses the firmware I/F. Some vendors embeds the
 binary code inside the driver instead of using user-program.
>>>
>>> Nokia N900 uses lp55xx, and I have custom scripts interfacing sysfs.
>>>
>>> Maemo uses the LEDs, too, but maemo is not open source.
>>>
>>> So no, I don't think there's anything important that could be broken.
>>
>> We can't guarantee that. Is there any problem in just using the
>> currently introduced DECLARE_FW_CUSTOM_FALLBACK() in
>> drivers/leds/leds-lp55xx-common.c?
> 
> Well, it would be good to get rid of the custom fallback
> functionality. And no, we don't need to "guarantee" that.  Removing
> obscure functionality noone uses is far game... providing noone
> complains ;-).

As Milo explained:

> Why has no one cried
> after the v4.0 custom fallback mechanism breaking ?

"Well, I don't know the reason exactly but my guess is they maybe still
using old kernel."

and after that:

"Device manufactures in Asia & North America requested lp55xx drivers"

These should be sufficient arguments for us for keeping the API
unchanged. If the users decided to upgrade their kernel then they
would be surprised by the API change.

DECLARE_FW_CUSTOM_FALLBACK macro seems to have been designed for
handling exactly this type of cases.

-- 
Best regards,
Jacek Anaszewski

[PATCH v4 1/3] Bluetooth: btusb: Use an error label for error paths

[Rajat Jain]

Use a label to remove the repetetive cleanup, for error cases.

Signed-off-by: Rajat Jain 
Reviewed-by: Brian Norris 
---
v4: same as v3
v3: Added Brian's "Reviewed-by"
v2: same as v1

 drivers/bluetooth/btusb.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 2f633df..ce22cef 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2991,18 +2991,15 @@ static int btusb_probe(struct usb_interface *intf,
err = usb_set_interface(data->udev, 0, 0);
if (err < 0) {
BT_ERR("failed to set interface 0, alt 0 %d", err);
-   hci_free_dev(hdev);
-   return err;
+   goto out_free_dev;
}
}
 
if (data->isoc) {
err = usb_driver_claim_interface(_driver,
 data->isoc, data);
-   if (err < 0) {
-   hci_free_dev(hdev);
-   return err;
-   }
+   if (err < 0)
+   goto out_free_dev;
}
 
 #ifdef CONFIG_BT_HCIBTUSB_BCM
@@ -3016,14 +3013,16 @@ static int btusb_probe(struct usb_interface *intf,
 #endif
 
err = hci_register_dev(hdev);
-   if (err < 0) {
-   hci_free_dev(hdev);
-   return err;
-   }
+   if (err < 0)
+   goto out_free_dev;
 
usb_set_intfdata(intf, data);
 
return 0;
+
+out_free_dev:
+   hci_free_dev(hdev);
+   return err;
 }
 
 static void btusb_disconnect(struct usb_interface *intf)
-- 
2.8.0.rc3.226.g39d4020

[PATCH v4 1/3] Bluetooth: btusb: Use an error label for error paths

[Rajat Jain]

Use a label to remove the repetetive cleanup, for error cases.

Signed-off-by: Rajat Jain 
Reviewed-by: Brian Norris 
---
v4: same as v3
v3: Added Brian's "Reviewed-by"
v2: same as v1

 drivers/bluetooth/btusb.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 2f633df..ce22cef 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2991,18 +2991,15 @@ static int btusb_probe(struct usb_interface *intf,
err = usb_set_interface(data->udev, 0, 0);
if (err < 0) {
BT_ERR("failed to set interface 0, alt 0 %d", err);
-   hci_free_dev(hdev);
-   return err;
+   goto out_free_dev;
}
}
 
if (data->isoc) {
err = usb_driver_claim_interface(_driver,
 data->isoc, data);
-   if (err < 0) {
-   hci_free_dev(hdev);
-   return err;
-   }
+   if (err < 0)
+   goto out_free_dev;
}
 
 #ifdef CONFIG_BT_HCIBTUSB_BCM
@@ -3016,14 +3013,16 @@ static int btusb_probe(struct usb_interface *intf,
 #endif
 
err = hci_register_dev(hdev);
-   if (err < 0) {
-   hci_free_dev(hdev);
-   return err;
-   }
+   if (err < 0)
+   goto out_free_dev;
 
usb_set_intfdata(intf, data);
 
return 0;
+
+out_free_dev:
+   hci_free_dev(hdev);
+   return err;
 }
 
 static void btusb_disconnect(struct usb_interface *intf)
-- 
2.8.0.rc3.226.g39d4020

[PATCHv2] crypto: testmgr: Use heap buffer for acomp test input

[Laura Abbott]

Christopher Covington reported a crash on aarch64 on recent Fedora
kernels:

kernel BUG at ./include/linux/scatterlist.h:140!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Modules linked in:
CPU: 2 PID: 752 Comm: cryptomgr_test Not tainted 4.9.0-11815-ge93b1cc #162
Hardware name: linux,dummy-virt (DT)
task: 80007c650080 task.stack: 8891
PC is at sg_init_one+0xa0/0xb8
LR is at sg_init_one+0x24/0xb8
...
[] sg_init_one+0xa0/0xb8
[] test_acomp+0x10c/0x438
[] alg_test_comp+0xb0/0x118
[] alg_test+0x17c/0x2f0
[] cryptomgr_test+0x44/0x50
[] kthread+0xf8/0x128
[] ret_from_fork+0x10/0x50

The test vectors used for input are part of the kernel image. These
inputs are passed as a buffer to sg_init_one which eventually blows up
with BUG_ON(!virt_addr_valid(buf)). On arm64, virt_addr_valid returns
false for the kernel image since virt_to_page will not return the
correct page. Fix this by copying the input vectors to heap buffer
before setting up the scatterlist.

Reported-by: Christopher Covington 
Fixes: d7db7a882deb ("crypto: acomp - update testmgr with support for acomp")
Signed-off-by: Laura Abbott 
---
 crypto/testmgr.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f616ad7..44e888b 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1461,16 +1461,25 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
for (i = 0; i < ctcount; i++) {
unsigned int dlen = COMP_BUF_SIZE;
int ilen = ctemplate[i].inlen;
+   void *input_vec;
 
+   input_vec = kmalloc(ilen, GFP_KERNEL);
+   if (!input_vec) {
+   ret = -ENOMEM;
+   goto out;
+   }
+
+   memcpy(input_vec, ctemplate[i].input, ilen);
memset(output, 0, dlen);
init_completion();
-   sg_init_one(, ctemplate[i].input, ilen);
+   sg_init_one(, input_vec, ilen);
sg_init_one(, output, dlen);
 
req = acomp_request_alloc(tfm);
if (!req) {
pr_err("alg: acomp: request alloc failed for %s\n",
   algo);
+   kfree(input_vec);
ret = -ENOMEM;
goto out;
}
@@ -1483,6 +1492,7 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
if (ret) {
pr_err("alg: acomp: compression failed on test %d for 
%s: ret=%d\n",
   i + 1, algo, -ret);
+   kfree(input_vec);
acomp_request_free(req);
goto out;
}
@@ -1491,6 +1501,7 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
pr_err("alg: acomp: Compression test %d failed for %s: 
output len = %d\n",
   i + 1, algo, req->dlen);
ret = -EINVAL;
+   kfree(input_vec);
acomp_request_free(req);
goto out;
}
@@ -1500,26 +1511,37 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
   i + 1, algo);
hexdump(output, req->dlen);
ret = -EINVAL;
+   kfree(input_vec);
acomp_request_free(req);
goto out;
}
 
+   kfree(input_vec);
acomp_request_free(req);
}
 
for (i = 0; i < dtcount; i++) {
unsigned int dlen = COMP_BUF_SIZE;
int ilen = dtemplate[i].inlen;
+   void *input_vec;
+
+   input_vec = kmalloc(ilen, GFP_KERNEL);
+   if (!input_vec) {
+   ret = -ENOMEM;
+   goto out;
+   }
 
+   memcpy(input_vec, dtemplate[i].input, ilen);
memset(output, 0, dlen);
init_completion();
-   sg_init_one(, dtemplate[i].input, ilen);
+   sg_init_one(, input_vec, ilen);
sg_init_one(, output, dlen);
 
req = acomp_request_alloc(tfm);
if (!req) {
pr_err("alg: acomp: request alloc failed for %s\n",
   algo);
+   kfree(input_vec);
ret = -ENOMEM;
goto out;
}
@@ -1532,6 +1554,7 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
if (ret) {
pr_err("alg: acomp: decompression failed on test %d for 
%s: ret=%d\n",
  

[PATCHv2] crypto: testmgr: Use heap buffer for acomp test input

[Laura Abbott]

Christopher Covington reported a crash on aarch64 on recent Fedora
kernels:

kernel BUG at ./include/linux/scatterlist.h:140!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Modules linked in:
CPU: 2 PID: 752 Comm: cryptomgr_test Not tainted 4.9.0-11815-ge93b1cc #162
Hardware name: linux,dummy-virt (DT)
task: 80007c650080 task.stack: 8891
PC is at sg_init_one+0xa0/0xb8
LR is at sg_init_one+0x24/0xb8
...
[] sg_init_one+0xa0/0xb8
[] test_acomp+0x10c/0x438
[] alg_test_comp+0xb0/0x118
[] alg_test+0x17c/0x2f0
[] cryptomgr_test+0x44/0x50
[] kthread+0xf8/0x128
[] ret_from_fork+0x10/0x50

The test vectors used for input are part of the kernel image. These
inputs are passed as a buffer to sg_init_one which eventually blows up
with BUG_ON(!virt_addr_valid(buf)). On arm64, virt_addr_valid returns
false for the kernel image since virt_to_page will not return the
correct page. Fix this by copying the input vectors to heap buffer
before setting up the scatterlist.

Reported-by: Christopher Covington 
Fixes: d7db7a882deb ("crypto: acomp - update testmgr with support for acomp")
Signed-off-by: Laura Abbott 
---
 crypto/testmgr.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f616ad7..44e888b 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1461,16 +1461,25 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
for (i = 0; i < ctcount; i++) {
unsigned int dlen = COMP_BUF_SIZE;
int ilen = ctemplate[i].inlen;
+   void *input_vec;
 
+   input_vec = kmalloc(ilen, GFP_KERNEL);
+   if (!input_vec) {
+   ret = -ENOMEM;
+   goto out;
+   }
+
+   memcpy(input_vec, ctemplate[i].input, ilen);
memset(output, 0, dlen);
init_completion();
-   sg_init_one(, ctemplate[i].input, ilen);
+   sg_init_one(, input_vec, ilen);
sg_init_one(, output, dlen);
 
req = acomp_request_alloc(tfm);
if (!req) {
pr_err("alg: acomp: request alloc failed for %s\n",
   algo);
+   kfree(input_vec);
ret = -ENOMEM;
goto out;
}
@@ -1483,6 +1492,7 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
if (ret) {
pr_err("alg: acomp: compression failed on test %d for 
%s: ret=%d\n",
   i + 1, algo, -ret);
+   kfree(input_vec);
acomp_request_free(req);
goto out;
}
@@ -1491,6 +1501,7 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
pr_err("alg: acomp: Compression test %d failed for %s: 
output len = %d\n",
   i + 1, algo, req->dlen);
ret = -EINVAL;
+   kfree(input_vec);
acomp_request_free(req);
goto out;
}
@@ -1500,26 +1511,37 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
   i + 1, algo);
hexdump(output, req->dlen);
ret = -EINVAL;
+   kfree(input_vec);
acomp_request_free(req);
goto out;
}
 
+   kfree(input_vec);
acomp_request_free(req);
}
 
for (i = 0; i < dtcount; i++) {
unsigned int dlen = COMP_BUF_SIZE;
int ilen = dtemplate[i].inlen;
+   void *input_vec;
+
+   input_vec = kmalloc(ilen, GFP_KERNEL);
+   if (!input_vec) {
+   ret = -ENOMEM;
+   goto out;
+   }
 
+   memcpy(input_vec, dtemplate[i].input, ilen);
memset(output, 0, dlen);
init_completion();
-   sg_init_one(, dtemplate[i].input, ilen);
+   sg_init_one(, input_vec, ilen);
sg_init_one(, output, dlen);
 
req = acomp_request_alloc(tfm);
if (!req) {
pr_err("alg: acomp: request alloc failed for %s\n",
   algo);
+   kfree(input_vec);
ret = -ENOMEM;
goto out;
}
@@ -1532,6 +1554,7 @@ static int test_acomp(struct crypto_acomp *tfm, struct 
comp_testvec *ctemplate,
if (ret) {
pr_err("alg: acomp: decompression failed on test %d for 
%s: ret=%d\n",
   i + 1, 

Re: [patch 10/10] irqchip/armada-xp: Consolidate hotplug state space

[Thomas Gleixner]

On Wed, 21 Dec 2016, Thomas Petazzoni wrote:
> On Wed, 21 Dec 2016 20:19:57 +0100, Thomas Gleixner wrote:
> > The mpic is either the main interrupt controller or sits behind a GIC. But
> > there is no way that both variants are available on the same system.
> 
> By "both variants", you mean the MPIC acting as the main interrupt
> controller on one side, and the MPIC acting as a "cascaded" controller,
> child of the GIC on the other side ?
> 
> If that's what you meant, then indeed it's correct.

Yes. I'll rephrase that.

Thanks,

tglx

Re: [patch 10/10] irqchip/armada-xp: Consolidate hotplug state space

[Thomas Gleixner]

On Wed, 21 Dec 2016, Thomas Petazzoni wrote:
> On Wed, 21 Dec 2016 20:19:57 +0100, Thomas Gleixner wrote:
> > The mpic is either the main interrupt controller or sits behind a GIC. But
> > there is no way that both variants are available on the same system.
> 
> By "both variants", you mean the MPIC acting as the main interrupt
> controller on one side, and the MPIC acting as a "cascaded" controller,
> child of the GIC on the other side ?
> 
> If that's what you meant, then indeed it's correct.

Yes. I'll rephrase that.

Thanks,

tglx

BUG/panic in ctnetlink_conntrack_event in 4.8.11

[Chris Boot]

Hi all,

I've encountered this BUG three times in the last few days, though I
must admit I've only captured the trace once so far so I can't be
completely certain it was exactly this the last few times. I did not
experience this with a 4.7 kernel; it only seemed to start with 4.8.

For some background: I use conntrackd (this is an "HA" firewall pair),
plenty of IPv6, IPsec with vti6 interfaces, conntrack, some NAT on IPv4
but definitely not with IPv6.

Without further ado, here is my crash:

[147965.209318] BUG: unable to handle kernel NULL pointer dereference at 
0018
[147965.217347] IP: [] icmp6_send+0x229/0x9f0
[147965.223051] PGD 0 
[147965.225184] Oops:  [#1] SMP
[147965.228424] Modules linked in: sch_fq_codel sch_htb pppoe pppox ppp_generic 
slhc ip6_vti ip6_tunnel tunnel6 drbg ansi_cprng seqiv esp6 xfrm4_mode_tunnel 
xfrm6_mode_tunnel ghash_generic gcm twofish_generic twofish_x86_64_3way 
twofish_x86_64 twofish_common serpent_sse2_x86_64 serpent_generic 
blowfish_generic blowfish_x86_64 blowfish_common cast5_generic cast_common ctr 
des_generic cbc algif_skcipher camellia_generic camellia_x86_64 xts xcbc 
sha512_ssse3 sha512_generic md4 algif_hash af_alg xfrm_user xfrm4_tunnel 
tunnel4 ipcomp xfrm_ipcomp esp4 ah4 af_key xfrm_algo tun hmac xt_nat xt_policy 
xt_statistic xt_helper xt_CLASSIFY xt_recent ip6table_nat xt_dscp xt_length 
binfmt_misc ip6t_REJECT xt_hashlimit nf_reject_ipv6 ip6table_mangle xt_comment 
iptable_nat ipt_REJECT nf_reject_ipv4 xt_addrtype xt_set ip_set_hash_ip ip_set 
xt_connmark xt_mark iptable_mangle xt_tcpudp iptable_raw xt_CT ip6table_raw 
xt_multiport xt_conntrack nf_log_ipv4 nf_nat_tftp nf_nat_snmp_basic 
nf_conntrack_snmp xt_NFLOG nf_nat_sip xt_LOG nf_log_ipv6 nf_nat_pptp 
nf_log_common nf_nat_proto_gre nf_nat_irc nf_nat_h323 nf_nat_ftp nf_nat_amanda 
ts_kmp nf_conntrack_amanda nf_conntrack_sane nf_conntrack_tftp nf_conntrack_sip 
nf_conntrack_proto_udplite nf_conntrack_proto_sctp nf_conntrack_pptp 
nf_conntrack_proto_gre nf_conntrack_netlink nf_conntrack_netbios_ns 
nf_conntrack_broadcast nf_conntrack_irc nf_conntrack_h323 nf_conntrack_ftp 
ip6table_filter ip6_tables iptable_filter openvswitch nf_conntrack_ipv6 
nf_nat_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_defrag_ipv6 nf_nat 
nf_conntrack 8021q garp mrp stp llc dummy nfnetlink_log nfnetlink evdev kvm_amd 
kvm irqbypass pcspkr k10temp sp5100_tco i2c_piix4 sg shpchp acpi_cpufreq 
tpm_tis tpm_tis_core tpm button drbd lru_cache libcrc32c ip_tables x_tables 
autofs4 ext4 crc16 jbd2 crc32c_generic fscrypto ecb glue_helper lrw gf128mul 
ablk_helper cryptd aes_x86_64 mbcache dm_mod sd_mod uas usb_storage ohci_pci 
ehci_pci ohci_hcd ehci_hcd usbcore ahci libahci libata scsi_mod usb_common 
r8169 mii
[147965.409769] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-2-amd64 #1 
Debian 4.8.11-1
[147965.417773] Hardware name: PC Engines APU, BIOS SageBios_PCEngines_APU-45 
04/05/2014
[147965.425607] task: 96d2d940aec0 task.stack: 96d2d941
[147965.431622] RIP: 0010:[]  [] 
icmp6_send+0x229/0x9f0
[147965.439742] RSP: 0018:96d2ded03d30  EFLAGS: 00010246
[147965.445150] RAX:  RBX: 96d2861e1700 RCX: 
0020
[147965.452377] RDX: 0001 RSI: 0200 RDI: 
96d1cb42799e
[147965.459597] RBP: 96d2ded03e60 R08:  R09: 
96d299832000
[147965.466823] R10: 0001 R11:  R12: 
96d1cb427996
[147965.474042] R13: b50da680 R14:  R15: 
0003
[147965.481263] FS:  () GS:96d2ded0() 
knlGS:
[147965.489444] CS:  0010 DS:  ES:  CR0: 80050033
[147965.495283] CR2: 0018 CR3: 000116d59000 CR4: 
06e0
[147965.502501] Stack:
[147965.504607]  96d291d14880  96d289288400 

[147965.512189]    96d1cb42799e 

[147965.519772]  0001 96d20001 96d1cb4279ae 
96d280fb2900
[147965.527356] Call Trace:
[147965.529895]   
[147965.531932]  [] ? ctnetlink_conntrack_event+0x3ff/0x620 
[nf_conntrack_netlink]
[147965.541005]  [] ? nf_nat_cleanup_conntrack+0xea/0x1a0 
[nf_nat]
[147965.548492]  [] ? get_frag_bucket_locked+0x43/0x70
[147965.554939]  [] ? nf_ct_net_init+0x130/0x130 
[nf_defrag_ipv6]
[147965.562338]  [] ? ip6_expire_frag_queue+0xf8/0x100
[147965.568787]  [] ? call_timer_fn+0x30/0x120
[147965.574539]  [] ? run_timer_softirq+0x216/0x4b0
[147965.580728]  [] ? tick_sched_handle.isra.12+0x20/0x50
[147965.587434]  [] ? tick_sched_timer+0x38/0x70
[147965.593363]  [] ? __do_softirq+0xf8/0x290
[147965.599030]  [] ? irq_exit+0x9b/0xa0
[147965.604266]  [] ? smp_apic_timer_interrupt+0x3e/0x50
[147965.610884]  [] ? apic_timer_interrupt+0x82/0x90
[147965.617155]   
[147965.619184]  [] ? cpuidle_enter_state+0x126/0x2d0
[147965.625740]  [] ? cpuidle_enter_state+0x113/0x2d0
[147965.632100]  [] ? 

BUG/panic in ctnetlink_conntrack_event in 4.8.11

[Chris Boot]

Hi all,

I've encountered this BUG three times in the last few days, though I
must admit I've only captured the trace once so far so I can't be
completely certain it was exactly this the last few times. I did not
experience this with a 4.7 kernel; it only seemed to start with 4.8.

For some background: I use conntrackd (this is an "HA" firewall pair),
plenty of IPv6, IPsec with vti6 interfaces, conntrack, some NAT on IPv4
but definitely not with IPv6.

Without further ado, here is my crash:

[147965.209318] BUG: unable to handle kernel NULL pointer dereference at 
0018
[147965.217347] IP: [] icmp6_send+0x229/0x9f0
[147965.223051] PGD 0 
[147965.225184] Oops:  [#1] SMP
[147965.228424] Modules linked in: sch_fq_codel sch_htb pppoe pppox ppp_generic 
slhc ip6_vti ip6_tunnel tunnel6 drbg ansi_cprng seqiv esp6 xfrm4_mode_tunnel 
xfrm6_mode_tunnel ghash_generic gcm twofish_generic twofish_x86_64_3way 
twofish_x86_64 twofish_common serpent_sse2_x86_64 serpent_generic 
blowfish_generic blowfish_x86_64 blowfish_common cast5_generic cast_common ctr 
des_generic cbc algif_skcipher camellia_generic camellia_x86_64 xts xcbc 
sha512_ssse3 sha512_generic md4 algif_hash af_alg xfrm_user xfrm4_tunnel 
tunnel4 ipcomp xfrm_ipcomp esp4 ah4 af_key xfrm_algo tun hmac xt_nat xt_policy 
xt_statistic xt_helper xt_CLASSIFY xt_recent ip6table_nat xt_dscp xt_length 
binfmt_misc ip6t_REJECT xt_hashlimit nf_reject_ipv6 ip6table_mangle xt_comment 
iptable_nat ipt_REJECT nf_reject_ipv4 xt_addrtype xt_set ip_set_hash_ip ip_set 
xt_connmark xt_mark iptable_mangle xt_tcpudp iptable_raw xt_CT ip6table_raw 
xt_multiport xt_conntrack nf_log_ipv4 nf_nat_tftp nf_nat_snmp_basic 
nf_conntrack_snmp xt_NFLOG nf_nat_sip xt_LOG nf_log_ipv6 nf_nat_pptp 
nf_log_common nf_nat_proto_gre nf_nat_irc nf_nat_h323 nf_nat_ftp nf_nat_amanda 
ts_kmp nf_conntrack_amanda nf_conntrack_sane nf_conntrack_tftp nf_conntrack_sip 
nf_conntrack_proto_udplite nf_conntrack_proto_sctp nf_conntrack_pptp 
nf_conntrack_proto_gre nf_conntrack_netlink nf_conntrack_netbios_ns 
nf_conntrack_broadcast nf_conntrack_irc nf_conntrack_h323 nf_conntrack_ftp 
ip6table_filter ip6_tables iptable_filter openvswitch nf_conntrack_ipv6 
nf_nat_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_defrag_ipv6 nf_nat 
nf_conntrack 8021q garp mrp stp llc dummy nfnetlink_log nfnetlink evdev kvm_amd 
kvm irqbypass pcspkr k10temp sp5100_tco i2c_piix4 sg shpchp acpi_cpufreq 
tpm_tis tpm_tis_core tpm button drbd lru_cache libcrc32c ip_tables x_tables 
autofs4 ext4 crc16 jbd2 crc32c_generic fscrypto ecb glue_helper lrw gf128mul 
ablk_helper cryptd aes_x86_64 mbcache dm_mod sd_mod uas usb_storage ohci_pci 
ehci_pci ohci_hcd ehci_hcd usbcore ahci libahci libata scsi_mod usb_common 
r8169 mii
[147965.409769] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-2-amd64 #1 
Debian 4.8.11-1
[147965.417773] Hardware name: PC Engines APU, BIOS SageBios_PCEngines_APU-45 
04/05/2014
[147965.425607] task: 96d2d940aec0 task.stack: 96d2d941
[147965.431622] RIP: 0010:[]  [] 
icmp6_send+0x229/0x9f0
[147965.439742] RSP: 0018:96d2ded03d30  EFLAGS: 00010246
[147965.445150] RAX:  RBX: 96d2861e1700 RCX: 
0020
[147965.452377] RDX: 0001 RSI: 0200 RDI: 
96d1cb42799e
[147965.459597] RBP: 96d2ded03e60 R08:  R09: 
96d299832000
[147965.466823] R10: 0001 R11:  R12: 
96d1cb427996
[147965.474042] R13: b50da680 R14:  R15: 
0003
[147965.481263] FS:  () GS:96d2ded0() 
knlGS:
[147965.489444] CS:  0010 DS:  ES:  CR0: 80050033
[147965.495283] CR2: 0018 CR3: 000116d59000 CR4: 
06e0
[147965.502501] Stack:
[147965.504607]  96d291d14880  96d289288400 

[147965.512189]    96d1cb42799e 

[147965.519772]  0001 96d20001 96d1cb4279ae 
96d280fb2900
[147965.527356] Call Trace:
[147965.529895]   
[147965.531932]  [] ? ctnetlink_conntrack_event+0x3ff/0x620 
[nf_conntrack_netlink]
[147965.541005]  [] ? nf_nat_cleanup_conntrack+0xea/0x1a0 
[nf_nat]
[147965.548492]  [] ? get_frag_bucket_locked+0x43/0x70
[147965.554939]  [] ? nf_ct_net_init+0x130/0x130 
[nf_defrag_ipv6]
[147965.562338]  [] ? ip6_expire_frag_queue+0xf8/0x100
[147965.568787]  [] ? call_timer_fn+0x30/0x120
[147965.574539]  [] ? run_timer_softirq+0x216/0x4b0
[147965.580728]  [] ? tick_sched_handle.isra.12+0x20/0x50
[147965.587434]  [] ? tick_sched_timer+0x38/0x70
[147965.593363]  [] ? __do_softirq+0xf8/0x290
[147965.599030]  [] ? irq_exit+0x9b/0xa0
[147965.604266]  [] ? smp_apic_timer_interrupt+0x3e/0x50
[147965.610884]  [] ? apic_timer_interrupt+0x82/0x90
[147965.617155]   
[147965.619184]  [] ? cpuidle_enter_state+0x126/0x2d0
[147965.625740]  [] ? cpuidle_enter_state+0x113/0x2d0
[147965.632100]  [] ? 

Re: [Patch v11 4/5] x86/cpufeature: enable RING3MWAIT for Knights Landing

[Thomas Gleixner]

On Tue, 20 Dec 2016, Grzegorz Andrejczuk wrote:

So how am I supposed to know which version of these patches is the right
one? Both subject lines are identical. 

> Enable ring 3 MONITOR/MWAIT for Intel Xeon Phi x200
> codenamed Knights Landing.
> 
> The patch:

>From your cover letter:

 Removed "This patch" from commit messages

Why is 'The patch any better' ?

> - Sets CPU feature X86_FEATURE_RING3MWAIT.
> - Sets HWCAP2_RING3MWAIT bit in ELF_HWCAP2.
> - Adds the ring3mwait=disable command line parameter.
> - Sets bit 1 of the MSR MISC_FEATURE_ENABLES or clears it when
>   the ring3mwait=disable command line parameter is used.

Changelogs should not describe WHAT the patch is doing. We can see that
from the patch. Changelogs should describe the WHY and CONCEPTS not
implementation details.

> +static void probe_xeon_phi_r3mwait(struct cpuinfo_x86 *c)
> +{
> + /*
> +  * Ring 3 MONITOR/MWAIT feature cannot be detected without
> +  * cpu model and family comparison.
> +  */
> + if (c->x86 != 6 || c->x86_model != INTEL_FAM6_XEON_PHI_KNL)
> + return;
> +
> + if (ring3mwait_disabled) {
> + msr_clear_bit(MSR_MISC_FEATURE_ENABLES,
> +   MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT);
> + return;
> + }
> +
> + msr_set_bit(MSR_MISC_FEATURE_ENABLES,
> + MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT);
> + set_cpu_cap(c, X86_FEATURE_RING3MWAIT);
> + set_bit(HWCAP2_RING3MWAIT, (unsigned long *)_HWCAP2);

>From your cover letter:

 "Removed warning from 32-bit build"

First of all, the warning

   arch/x86/include/asm/bitops.h:72:1: note: expected 'volatile long unsigned 
int *'
but argument is of type 'unsigned int *'
set_bit(long nr, volatile unsigned long *addr)

is not at all 32bit specific.

Handing an unsigned int pointer to a function which expects a unsigned long
is even more wrong on 64bit.

So now for your 'removal fix': It's just as sloppy as anything else what
I've seen from you before.

Handing a typecasted unsigned int pointer to a function which expects an
unsigned long pointer is just broken and a clear sign of careless
tinkering.

The only reason why this 'works' is because x86 is a little endian
architecture and the bit number is a constant and set_bit gets translated
it into:

orb 0x02, 0x0(%rip) 

Now if you look really close to that disassembly then you might notice,
that this sets bit 1 and not as you tell in patch 2/5:

   "Introduce ELF_HWCAP2 variable for x86 and reserve its bit 0 to expose
the ring 3 MONITOR/MWAIT."

So why does it not set bit 0?

Simply because you hand in HWCAP2_RING3MWAIT as bit number, which is
defined as:

+#define HWCAP2_RING3MWAIT  (1 << 0)

Crap, crap, crap.

What's so !$@&*(? wrong with doing the simple, obvious and correct:

   ELF_HWCAP2 |= HWCAP2_RING3MWAIT;

C is really hard, right?

Yours grumpy

  tglx



   

Re: [Patch v11 4/5] x86/cpufeature: enable RING3MWAIT for Knights Landing

[Thomas Gleixner]

On Tue, 20 Dec 2016, Grzegorz Andrejczuk wrote:

So how am I supposed to know which version of these patches is the right
one? Both subject lines are identical. 

> Enable ring 3 MONITOR/MWAIT for Intel Xeon Phi x200
> codenamed Knights Landing.
> 
> The patch:

>From your cover letter:

 Removed "This patch" from commit messages

Why is 'The patch any better' ?

> - Sets CPU feature X86_FEATURE_RING3MWAIT.
> - Sets HWCAP2_RING3MWAIT bit in ELF_HWCAP2.
> - Adds the ring3mwait=disable command line parameter.
> - Sets bit 1 of the MSR MISC_FEATURE_ENABLES or clears it when
>   the ring3mwait=disable command line parameter is used.

Changelogs should not describe WHAT the patch is doing. We can see that
from the patch. Changelogs should describe the WHY and CONCEPTS not
implementation details.

> +static void probe_xeon_phi_r3mwait(struct cpuinfo_x86 *c)
> +{
> + /*
> +  * Ring 3 MONITOR/MWAIT feature cannot be detected without
> +  * cpu model and family comparison.
> +  */
> + if (c->x86 != 6 || c->x86_model != INTEL_FAM6_XEON_PHI_KNL)
> + return;
> +
> + if (ring3mwait_disabled) {
> + msr_clear_bit(MSR_MISC_FEATURE_ENABLES,
> +   MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT);
> + return;
> + }
> +
> + msr_set_bit(MSR_MISC_FEATURE_ENABLES,
> + MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT);
> + set_cpu_cap(c, X86_FEATURE_RING3MWAIT);
> + set_bit(HWCAP2_RING3MWAIT, (unsigned long *)_HWCAP2);

>From your cover letter:

 "Removed warning from 32-bit build"

First of all, the warning

   arch/x86/include/asm/bitops.h:72:1: note: expected 'volatile long unsigned 
int *'
but argument is of type 'unsigned int *'
set_bit(long nr, volatile unsigned long *addr)

is not at all 32bit specific.

Handing an unsigned int pointer to a function which expects a unsigned long
is even more wrong on 64bit.

So now for your 'removal fix': It's just as sloppy as anything else what
I've seen from you before.

Handing a typecasted unsigned int pointer to a function which expects an
unsigned long pointer is just broken and a clear sign of careless
tinkering.

The only reason why this 'works' is because x86 is a little endian
architecture and the bit number is a constant and set_bit gets translated
it into:

orb 0x02, 0x0(%rip) 

Now if you look really close to that disassembly then you might notice,
that this sets bit 1 and not as you tell in patch 2/5:

   "Introduce ELF_HWCAP2 variable for x86 and reserve its bit 0 to expose
the ring 3 MONITOR/MWAIT."

So why does it not set bit 0?

Simply because you hand in HWCAP2_RING3MWAIT as bit number, which is
defined as:

+#define HWCAP2_RING3MWAIT  (1 << 0)

Crap, crap, crap.

What's so !$@&*(? wrong with doing the simple, obvious and correct:

   ELF_HWCAP2 |= HWCAP2_RING3MWAIT;

C is really hard, right?

Yours grumpy

  tglx



   

Re: [PATCH v11 1/5] x86/msr: add MSR_MISC_FEATURE_ENABLES and RING3MWAIT bit

[Thomas Gleixner]

On Tue, 20 Dec 2016, Grzegorz Andrejczuk wrote:
> 
> diff --git a/arch/x86/include/asm/msr-index.h 
> b/arch/x86/include/asm/msr-index.h
> index 78f3760..55ffae0 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -539,6 +539,12 @@
>  #define MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE_BIT 39
>  #define MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE (1ULL << 
> MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE_BIT)
>  
> +/* MISC_FEATURE_ENABLES non-architectural features */
> +#define MSR_MISC_FEATURE_ENABLES 0x0140
> +
> +#define MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT  1
> +#define MSR_MISC_FEATURE_ENABLES_RING3MWAIT  (1ULL << 
> MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT)
> +

This last define is not used anywhere. I told you before, but addressing my
review comments completely is an unduly burden, or what?

Thanks,

tglx

Re: [PATCH v11 1/5] x86/msr: add MSR_MISC_FEATURE_ENABLES and RING3MWAIT bit

[Thomas Gleixner]

On Tue, 20 Dec 2016, Grzegorz Andrejczuk wrote:
> 
> diff --git a/arch/x86/include/asm/msr-index.h 
> b/arch/x86/include/asm/msr-index.h
> index 78f3760..55ffae0 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -539,6 +539,12 @@
>  #define MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE_BIT 39
>  #define MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE (1ULL << 
> MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE_BIT)
>  
> +/* MISC_FEATURE_ENABLES non-architectural features */
> +#define MSR_MISC_FEATURE_ENABLES 0x0140
> +
> +#define MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT  1
> +#define MSR_MISC_FEATURE_ENABLES_RING3MWAIT  (1ULL << 
> MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT)
> +

This last define is not used anywhere. I told you before, but addressing my
review comments completely is an unduly burden, or what?

Thanks,

tglx

[PATCH 3/3] KVM: nVMX: refactor nested_vmx_entry_failure()

[Radim Krčmář]

Change recurring pattern

  leave_guest_mode(vcpu);
  vmx_load_vmcs01(vcpu);
  nested_vmx_entry_failure(vcpu, ...);
  return 1;

into

  return nested_vmx_entry_failure(vcpu, ...)

Signed-off-by: Radim Krčmář 
---
 arch/x86/kvm/vmx.c | 46 --
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 050899431b5e..a74cde40e349 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1398,7 +1398,7 @@ static inline bool is_nmi(u32 intr_info)
 static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
  u32 exit_intr_info,
  unsigned long exit_qualification);
-static void nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
+static int nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12,
u32 reason, unsigned long qualification);
 
@@ -10529,20 +10529,13 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
vmx_segment_cache_clear(vmx);
 
if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
-   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4))
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   if (vmcs12->vmcs_link_pointer != -1ull) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+
+   if (vmcs12->vmcs_link_pointer != -1ull)
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, ENTRY_FAIL_VMCS_LINK_PTR);
-   return 1;
-   }
 
/*
 * If the load IA32_EFER VM-entry control is 1, the following checks
@@ -10559,32 +10552,21 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) ||
((vmcs12->guest_cr0 & X86_CR0_PG) &&
 ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
}
}
 
-   if (prepare_vmcs02(vcpu, vmcs12, _qualification)) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   if (prepare_vmcs02(vcpu, vmcs12, _qualification))
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, exit_qualification);
-   return 1;
-   }
 
msr_entry_idx = nested_vmx_load_msr(vcpu,
vmcs12->vm_entry_msr_load_addr,
vmcs12->vm_entry_msr_load_count);
-   if (msr_entry_idx) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   if (msr_entry_idx)
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_MSR_LOAD_FAIL, msr_entry_idx);
-   return 1;
-   }
 
vmcs12->launch_state = 1;
 
@@ -11173,16 +11155,20 @@ static void vmx_leave_nested(struct kvm_vcpu *vcpu)
  * It should only be called before L2 actually succeeded to run, and when
  * vmcs01 is current (it doesn't leave_guest_mode() or switch vmcss).
  */
-static void nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
+static int nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12,
u32 reason, unsigned long qualification)
 {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
load_vmcs12_host_state(vcpu, vmcs12);
vmcs12->vm_exit_reason = reason | VMX_EXIT_REASONS_FAILED_VMENTRY;
vmcs12->exit_qualification = qualification;
nested_vmx_succeed(vcpu);
if (enable_shadow_vmcs)
to_vmx(vcpu)->nested.sync_shadow_vmcs = true;
+
+   return 1;
 }
 
 static int vmx_check_intercept(struct kvm_vcpu *vcpu,
-- 
2.11.0

[PATCH 3/3] KVM: nVMX: refactor nested_vmx_entry_failure()

[Radim Krčmář]

Change recurring pattern

  leave_guest_mode(vcpu);
  vmx_load_vmcs01(vcpu);
  nested_vmx_entry_failure(vcpu, ...);
  return 1;

into

  return nested_vmx_entry_failure(vcpu, ...)

Signed-off-by: Radim Krčmář 
---
 arch/x86/kvm/vmx.c | 46 --
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 050899431b5e..a74cde40e349 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1398,7 +1398,7 @@ static inline bool is_nmi(u32 intr_info)
 static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
  u32 exit_intr_info,
  unsigned long exit_qualification);
-static void nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
+static int nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12,
u32 reason, unsigned long qualification);
 
@@ -10529,20 +10529,13 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
vmx_segment_cache_clear(vmx);
 
if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
-   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4))
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   if (vmcs12->vmcs_link_pointer != -1ull) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+
+   if (vmcs12->vmcs_link_pointer != -1ull)
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, ENTRY_FAIL_VMCS_LINK_PTR);
-   return 1;
-   }
 
/*
 * If the load IA32_EFER VM-entry control is 1, the following checks
@@ -10559,32 +10552,21 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) ||
((vmcs12->guest_cr0 & X86_CR0_PG) &&
 ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
}
}
 
-   if (prepare_vmcs02(vcpu, vmcs12, _qualification)) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   if (prepare_vmcs02(vcpu, vmcs12, _qualification))
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_INVALID_STATE, exit_qualification);
-   return 1;
-   }
 
msr_entry_idx = nested_vmx_load_msr(vcpu,
vmcs12->vm_entry_msr_load_addr,
vmcs12->vm_entry_msr_load_count);
-   if (msr_entry_idx) {
-   leave_guest_mode(vcpu);
-   vmx_load_vmcs01(vcpu);
-   nested_vmx_entry_failure(vcpu, vmcs12,
+   if (msr_entry_idx)
+   return nested_vmx_entry_failure(vcpu, vmcs12,
EXIT_REASON_MSR_LOAD_FAIL, msr_entry_idx);
-   return 1;
-   }
 
vmcs12->launch_state = 1;
 
@@ -11173,16 +11155,20 @@ static void vmx_leave_nested(struct kvm_vcpu *vcpu)
  * It should only be called before L2 actually succeeded to run, and when
  * vmcs01 is current (it doesn't leave_guest_mode() or switch vmcss).
  */
-static void nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
+static int nested_vmx_entry_failure(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12,
u32 reason, unsigned long qualification)
 {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
load_vmcs12_host_state(vcpu, vmcs12);
vmcs12->vm_exit_reason = reason | VMX_EXIT_REASONS_FAILED_VMENTRY;
vmcs12->exit_qualification = qualification;
nested_vmx_succeed(vcpu);
if (enable_shadow_vmcs)
to_vmx(vcpu)->nested.sync_shadow_vmcs = true;
+
+   return 1;
 }
 
 static int vmx_check_intercept(struct kvm_vcpu *vcpu,
-- 
2.11.0

[PATCH 0/3] KVM: nVMX: fix one guest vmcs check and refactor a bunch more

[Radim Krčmář]

v1:
  Applies after David's "KVM: nVMX: fix instruction skipping during
  emulated vm-entry".


Radim Krčmář (3):
  KVM: nVMX: fix handling of invalid host efer
  KVM: nVMX: colocate guest vmcs checks
  KVM: nVMX: refactor nested_vmx_entry_failure()

 arch/x86/kvm/vmx.c | 92 +-
 1 file changed, 42 insertions(+), 50 deletions(-)

-- 
2.11.0

[PATCH 1/3] KVM: nVMX: fix handling of invalid host efer

[Radim Krčmář]

Host state must be checked before attempting vm entry, which means it
throws a different error.

Signed-off-by: Radim Krčmář 
---
 arch/x86/kvm/vmx.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c9d7f1a1ba16..ab65b31ce58c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10481,6 +10481,24 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
goto out;
}
 
+   /*
+* If the load IA32_EFER VM-exit control is 1, bits reserved in the
+* IA32_EFER MSR must be 0 in the field for that register. In addition,
+* the values of the LMA and LME bits in the field must each be that of
+* the host address-space size VM-exit control.
+*/
+   if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
+   ia32e = (vmcs12->vm_exit_controls &
+VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
+   if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
+   ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
+   ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) {
+   nested_vmx_failValid(vcpu,
+   VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+   goto out;
+   }
+   }
+
if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
!nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
nested_vmx_entry_failure(vcpu, vmcs12,
@@ -10515,24 +10533,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
}
 
/*
-* If the load IA32_EFER VM-exit control is 1, bits reserved in the
-* IA32_EFER MSR must be 0 in the field for that register. In addition,
-* the values of the LMA and LME bits in the field must each be that of
-* the host address-space size VM-exit control.
-*/
-   if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
-   ia32e = (vmcs12->vm_exit_controls &
-VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
-   if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
-   ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
-   ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   }
-
-   /*
 * We're finally done with prerequisite checking, and can start with
 * the nested entry.
 */
-- 
2.11.0

[PATCH 2/3] KVM: nVMX: colocate guest vmcs checks

[Radim Krčmář]

Few guest checks were done before entering the guest, for which there is
no reason.  Having them all in one place will be simpler.

Signed-off-by: Radim Krčmář 
---
 arch/x86/kvm/vmx.c | 72 +-
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ab65b31ce58c..050899431b5e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10499,39 +10499,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
}
}
 
-   if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
-   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   if (vmcs12->vmcs_link_pointer != -1ull) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_VMCS_LINK_PTR);
-   return 1;
-   }
-
-   /*
-* If the load IA32_EFER VM-entry control is 1, the following checks
-* are performed on the field for the IA32_EFER MSR:
-* - Bits reserved in the IA32_EFER MSR must be 0.
-* - Bit 10 (corresponding to IA32_EFER.LMA) must equal the value of
-*   the IA-32e mode guest VM-exit control. It must also be identical
-*   to bit 8 (LME) if bit 31 in the CR0 field (corresponding to
-*   CR0.PG) is 1.
-*/
-   if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_EFER) {
-   ia32e = (vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE) != 0;
-   if (!kvm_valid_efer(vcpu, vmcs12->guest_ia32_efer) ||
-   ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) ||
-   ((vmcs12->guest_cr0 & X86_CR0_PG) &&
-ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   }
-
/*
 * We're finally done with prerequisite checking, and can start with
 * the nested entry.
@@ -10561,6 +10528,45 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
 
vmx_segment_cache_clear(vmx);
 
+   if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
+   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
+   nested_vmx_entry_failure(vcpu, vmcs12,
+   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
+   return 1;
+   }
+   if (vmcs12->vmcs_link_pointer != -1ull) {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
+   nested_vmx_entry_failure(vcpu, vmcs12,
+   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_VMCS_LINK_PTR);
+   return 1;
+   }
+
+   /*
+* If the load IA32_EFER VM-entry control is 1, the following checks
+* are performed on the field for the IA32_EFER MSR:
+* - Bits reserved in the IA32_EFER MSR must be 0.
+* - Bit 10 (corresponding to IA32_EFER.LMA) must equal the value of
+*   the IA-32e mode guest VM-exit control. It must also be identical
+*   to bit 8 (LME) if bit 31 in the CR0 field (corresponding to
+*   CR0.PG) is 1.
+*/
+   if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_EFER) {
+   ia32e = (vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE) != 0;
+   if (!kvm_valid_efer(vcpu, vmcs12->guest_ia32_efer) ||
+   ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) ||
+   ((vmcs12->guest_cr0 & X86_CR0_PG) &&
+ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
+   nested_vmx_entry_failure(vcpu, vmcs12,
+   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
+   return 1;
+   }
+   }
+
if (prepare_vmcs02(vcpu, vmcs12, _qualification)) {
leave_guest_mode(vcpu);
vmx_load_vmcs01(vcpu);
-- 
2.11.0

[PATCH 0/3] KVM: nVMX: fix one guest vmcs check and refactor a bunch more

[Radim Krčmář]

v1:
  Applies after David's "KVM: nVMX: fix instruction skipping during
  emulated vm-entry".


Radim Krčmář (3):
  KVM: nVMX: fix handling of invalid host efer
  KVM: nVMX: colocate guest vmcs checks
  KVM: nVMX: refactor nested_vmx_entry_failure()

 arch/x86/kvm/vmx.c | 92 +-
 1 file changed, 42 insertions(+), 50 deletions(-)

-- 
2.11.0

[PATCH 1/3] KVM: nVMX: fix handling of invalid host efer

[Radim Krčmář]

Host state must be checked before attempting vm entry, which means it
throws a different error.

Signed-off-by: Radim Krčmář 
---
 arch/x86/kvm/vmx.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c9d7f1a1ba16..ab65b31ce58c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10481,6 +10481,24 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
goto out;
}
 
+   /*
+* If the load IA32_EFER VM-exit control is 1, bits reserved in the
+* IA32_EFER MSR must be 0 in the field for that register. In addition,
+* the values of the LMA and LME bits in the field must each be that of
+* the host address-space size VM-exit control.
+*/
+   if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
+   ia32e = (vmcs12->vm_exit_controls &
+VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
+   if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
+   ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
+   ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) {
+   nested_vmx_failValid(vcpu,
+   VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+   goto out;
+   }
+   }
+
if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
!nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
nested_vmx_entry_failure(vcpu, vmcs12,
@@ -10515,24 +10533,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
}
 
/*
-* If the load IA32_EFER VM-exit control is 1, bits reserved in the
-* IA32_EFER MSR must be 0 in the field for that register. In addition,
-* the values of the LMA and LME bits in the field must each be that of
-* the host address-space size VM-exit control.
-*/
-   if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
-   ia32e = (vmcs12->vm_exit_controls &
-VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
-   if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
-   ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
-   ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   }
-
-   /*
 * We're finally done with prerequisite checking, and can start with
 * the nested entry.
 */
-- 
2.11.0

[PATCH 2/3] KVM: nVMX: colocate guest vmcs checks

[Radim Krčmář]

Few guest checks were done before entering the guest, for which there is
no reason.  Having them all in one place will be simpler.

Signed-off-by: Radim Krčmář 
---
 arch/x86/kvm/vmx.c | 72 +-
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ab65b31ce58c..050899431b5e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -10499,39 +10499,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
}
}
 
-   if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
-   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   if (vmcs12->vmcs_link_pointer != -1ull) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_VMCS_LINK_PTR);
-   return 1;
-   }
-
-   /*
-* If the load IA32_EFER VM-entry control is 1, the following checks
-* are performed on the field for the IA32_EFER MSR:
-* - Bits reserved in the IA32_EFER MSR must be 0.
-* - Bit 10 (corresponding to IA32_EFER.LMA) must equal the value of
-*   the IA-32e mode guest VM-exit control. It must also be identical
-*   to bit 8 (LME) if bit 31 in the CR0 field (corresponding to
-*   CR0.PG) is 1.
-*/
-   if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_EFER) {
-   ia32e = (vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE) != 0;
-   if (!kvm_valid_efer(vcpu, vmcs12->guest_ia32_efer) ||
-   ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) ||
-   ((vmcs12->guest_cr0 & X86_CR0_PG) &&
-ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) {
-   nested_vmx_entry_failure(vcpu, vmcs12,
-   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
-   return 1;
-   }
-   }
-
/*
 * We're finally done with prerequisite checking, and can start with
 * the nested entry.
@@ -10561,6 +10528,45 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool 
launch)
 
vmx_segment_cache_clear(vmx);
 
+   if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
+   !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
+   nested_vmx_entry_failure(vcpu, vmcs12,
+   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
+   return 1;
+   }
+   if (vmcs12->vmcs_link_pointer != -1ull) {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
+   nested_vmx_entry_failure(vcpu, vmcs12,
+   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_VMCS_LINK_PTR);
+   return 1;
+   }
+
+   /*
+* If the load IA32_EFER VM-entry control is 1, the following checks
+* are performed on the field for the IA32_EFER MSR:
+* - Bits reserved in the IA32_EFER MSR must be 0.
+* - Bit 10 (corresponding to IA32_EFER.LMA) must equal the value of
+*   the IA-32e mode guest VM-exit control. It must also be identical
+*   to bit 8 (LME) if bit 31 in the CR0 field (corresponding to
+*   CR0.PG) is 1.
+*/
+   if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_EFER) {
+   ia32e = (vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE) != 0;
+   if (!kvm_valid_efer(vcpu, vmcs12->guest_ia32_efer) ||
+   ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) ||
+   ((vmcs12->guest_cr0 & X86_CR0_PG) &&
+ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) {
+   leave_guest_mode(vcpu);
+   vmx_load_vmcs01(vcpu);
+   nested_vmx_entry_failure(vcpu, vmcs12,
+   EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
+   return 1;
+   }
+   }
+
if (prepare_vmcs02(vcpu, vmcs12, _qualification)) {
leave_guest_mode(vcpu);
vmx_load_vmcs01(vcpu);
-- 
2.11.0

Re: [patch 10/10] irqchip/armada-xp: Consolidate hotplug state space

[Thomas Petazzoni]

Hello,

On Wed, 21 Dec 2016 20:19:57 +0100, Thomas Gleixner wrote:
> The mpic is either the main interrupt controller or sits behind a GIC. But
> there is no way that both variants are available on the same system.

By "both variants", you mean the MPIC acting as the main interrupt
controller on one side, and the MPIC acting as a "cascaded" controller,
child of the GIC on the other side ?

If that's what you meant, then indeed it's correct.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [patch 10/10] irqchip/armada-xp: Consolidate hotplug state space

[Thomas Petazzoni]

Hello,

On Wed, 21 Dec 2016 20:19:57 +0100, Thomas Gleixner wrote:
> The mpic is either the main interrupt controller or sits behind a GIC. But
> there is no way that both variants are available on the same system.

By "both variants", you mean the MPIC acting as the main interrupt
controller on one side, and the MPIC acting as a "cascaded" controller,
child of the GIC on the other side ?

If that's what you meant, then indeed it's correct.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Sven Schmidt]

On 12/20/2016 08:52 PM, Joe Perches wrote:
> On Tue, 2016-12-20 at 19:53 +0100, Sven Schmidt wrote:
>> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
>> The kernel module is inspired by the previous work by Chanho Min.
>> The updated LZ4 module will not break existing code since there were alias
>> methods added to ensure backwards compatibility.
>
>[]
>
>> diff --git a/include/linux/lz4.h b/include/linux/lz4.h
> []
>> @@ -1,87 +1,218 @@
>>  #ifndef __LZ4_H__
>>  #define __LZ4_H__
>> +
>>  /*
>>   * LZ4 Kernel Interface
>>   *
>> - * Copyright (C) 2013, LG Electronics, Kyungsik Lee 
>> + * Copyright (C) 2016, Sven Schmidt <4ssch...@informatik.uni-hamburg.de>
>
> Deleting copyright notices is poor form and shouldn't be done.
>
> I didn't look at the rest
>

Thanks Joe for your time. If you would take a look at the rest, you may notice
that I changed almost the whole file. That's why I also changed the copyright 
note.
If you think I should keep the original note, I will do that.

What I did change:
- I included additional information stating that the file is based on Yann 
Collets
original header file (especially in matters of the function comments)

Sven

Re: [PATCH 3/3] lib: Update LZ4 compressor module based on LZ4 v1.7.2.

[Sven Schmidt]

On 12/20/2016 08:52 PM, Joe Perches wrote:
> On Tue, 2016-12-20 at 19:53 +0100, Sven Schmidt wrote:
>> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
>> The kernel module is inspired by the previous work by Chanho Min.
>> The updated LZ4 module will not break existing code since there were alias
>> methods added to ensure backwards compatibility.
>
>[]
>
>> diff --git a/include/linux/lz4.h b/include/linux/lz4.h
> []
>> @@ -1,87 +1,218 @@
>>  #ifndef __LZ4_H__
>>  #define __LZ4_H__
>> +
>>  /*
>>   * LZ4 Kernel Interface
>>   *
>> - * Copyright (C) 2013, LG Electronics, Kyungsik Lee 
>> + * Copyright (C) 2016, Sven Schmidt <4ssch...@informatik.uni-hamburg.de>
>
> Deleting copyright notices is poor form and shouldn't be done.
>
> I didn't look at the rest
>

Thanks Joe for your time. If you would take a look at the rest, you may notice
that I changed almost the whole file. That's why I also changed the copyright 
note.
If you think I should keep the original note, I will do that.

What I did change:
- I included additional information stating that the file is based on Yann 
Collets
original header file (especially in matters of the function comments)

Sven

RE: [PATCH 12/15] hyperv: move VMBus connection ids to uapi

[KY Srinivasan]

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Wednesday, December 21, 2016 10:03 AM
> To: Christoph Hellwig 
> Cc: Paolo Bonzini ; Roman Kagan
> ; Radim Krčmář ; KY
> Srinivasan ; Vitaly Kuznetsov
> ; k...@vger.kernel.org; Denis V . Lunev
> ; Haiyang Zhang ;
> x...@kernel.org; linux-kernel@vger.kernel.org; Ingo Molnar
> ; H. Peter Anvin ;
> de...@linuxdriverproject.org; Thomas Gleixner 
> Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> 
> On Wed, 21 Dec 2016 09:58:36 -0800
> Christoph Hellwig  wrote:
> 
> > On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:
> > > Lastly, there is licensing issues on headers. It would be good to have any
> > > userspace ABI headers licensed with a more liberal license so that BSD
> and DPDK drivers
> > > could use them directly. Right now each one reinvents.
> >
> > Microsoft could easily solves this problem by offering a suitably
> > liberally licensed header documenting the full HyperV guest protocol
> > that Linux and other projects could use.
> 
> The issue is if same header file mixes kernel and userspace API stuff.
> 
> Once the files are arranged right, I will submit trivial change to comments
> to indicate the liberal licensing of userspace API headers.

Let us take this one step at a time. I know for a fact that not all the guest 
host
protocols on Hyper-V are guaranteed to be stable. Some of the protocols are 
part of
the published MSFT standards such RNDIS and these obviously are guaranteed to be
stable. For the rest it is less clear. The fact that we need to ensure 
compatibility of existing
Windows guests tells me that any host side changes will be versioned and the 
hosts will always
support older guests.

I would like to minimize what we include in the uapi header; especially when 
MSFT has made no guarantees
with regards how  they may be evolved. I will also work on getting some clarity 
on both stability and
under what license we would expose the uapi header.

Regards,

K. Y

RE: [PATCH 12/15] hyperv: move VMBus connection ids to uapi

[KY Srinivasan]

> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Wednesday, December 21, 2016 10:03 AM
> To: Christoph Hellwig 
> Cc: Paolo Bonzini ; Roman Kagan
> ; Radim Krčmář ; KY
> Srinivasan ; Vitaly Kuznetsov
> ; k...@vger.kernel.org; Denis V . Lunev
> ; Haiyang Zhang ;
> x...@kernel.org; linux-kernel@vger.kernel.org; Ingo Molnar
> ; H. Peter Anvin ;
> de...@linuxdriverproject.org; Thomas Gleixner 
> Subject: Re: [PATCH 12/15] hyperv: move VMBus connection ids to uapi
> 
> On Wed, 21 Dec 2016 09:58:36 -0800
> Christoph Hellwig  wrote:
> 
> > On Wed, Dec 21, 2016 at 09:50:49AM -0800, Stephen Hemminger wrote:
> > > Lastly, there is licensing issues on headers. It would be good to have any
> > > userspace ABI headers licensed with a more liberal license so that BSD
> and DPDK drivers
> > > could use them directly. Right now each one reinvents.
> >
> > Microsoft could easily solves this problem by offering a suitably
> > liberally licensed header documenting the full HyperV guest protocol
> > that Linux and other projects could use.
> 
> The issue is if same header file mixes kernel and userspace API stuff.
> 
> Once the files are arranged right, I will submit trivial change to comments
> to indicate the liberal licensing of userspace API headers.

Let us take this one step at a time. I know for a fact that not all the guest 
host
protocols on Hyper-V are guaranteed to be stable. Some of the protocols are 
part of
the published MSFT standards such RNDIS and these obviously are guaranteed to be
stable. For the rest it is less clear. The fact that we need to ensure 
compatibility of existing
Windows guests tells me that any host side changes will be versioned and the 
hosts will always
support older guests.

I would like to minimize what we include in the uapi header; especially when 
MSFT has made no guarantees
with regards how  they may be evolved. I will also work on getting some clarity 
on both stability and
under what license we would expose the uapi header.

Regards,

K. Y

[PATCH 1/5] ARM: dts: qcom: apq8064: Add missing scm clock

[Bjorn Andersson]

As per the device tree binding the apq8064 scm node requires the core
clock to be specified, so add this.

Cc: John Stultz 
Signed-off-by: Bjorn Andersson 
---
 arch/arm/boot/dts/qcom-apq8064.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi 
b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 268bd470c865..78bf155a52f3 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -303,6 +303,9 @@
firmware {
scm {
compatible = "qcom,scm-apq8064";
+
+   clocks = < CE3_CORE_CLK>;
+   clock-names = "core";
};
};
 
-- 
2.11.0

Re: [RFC][PATCH] make global bitlock waitqueues per-node

[Linus Torvalds]

On Wed, Dec 21, 2016 at 11:01 AM, Nicholas Piggin  wrote:
> Peter's patch is less code and in that regard a bit nicer. I tried
> going that way once, but I just thought it was a bit too sloppy to
> do nicely with wait bit APIs.

So I have to admit that when I read through your and PeterZ's patches
back-to-back, yours was easier to understand.

PeterZ's is smaller but kind of subtle. The whole "return zero from
lock_page_wait() and go around again" and the locking around that
isn't exactly clear. In contrast, yours has the obvious waitqueue
spinlock.

I'll think about it.  And yes, it would be good to have more testing,
but at the same time xmas is imminent, and waiting around too much
isn't going to help either..

  Linus

[PATCH 1/5] ARM: dts: qcom: apq8064: Add missing scm clock

[Bjorn Andersson]

As per the device tree binding the apq8064 scm node requires the core
clock to be specified, so add this.

Cc: John Stultz 
Signed-off-by: Bjorn Andersson 
---
 arch/arm/boot/dts/qcom-apq8064.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi 
b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 268bd470c865..78bf155a52f3 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -303,6 +303,9 @@
firmware {
scm {
compatible = "qcom,scm-apq8064";
+
+   clocks = < CE3_CORE_CLK>;
+   clock-names = "core";
};
};
 
-- 
2.11.0

Re: [RFC][PATCH] make global bitlock waitqueues per-node

[Linus Torvalds]

On Wed, Dec 21, 2016 at 11:01 AM, Nicholas Piggin  wrote:
> Peter's patch is less code and in that regard a bit nicer. I tried
> going that way once, but I just thought it was a bit too sloppy to
> do nicely with wait bit APIs.

So I have to admit that when I read through your and PeterZ's patches
back-to-back, yours was easier to understand.

PeterZ's is smaller but kind of subtle. The whole "return zero from
lock_page_wait() and go around again" and the locking around that
isn't exactly clear. In contrast, yours has the obvious waitqueue
spinlock.

I'll think about it.  And yes, it would be good to have more testing,
but at the same time xmas is imminent, and waiting around too much
isn't going to help either..

  Linus

[PATCH 2/5] ARM: dts: qcom: apq8064: Add riva-pil node

[Bjorn Andersson]

Add nodes for the Riva PIL, IRIS RF module, BT and WiFI services exposed
by the Riva firmware and the related memory reserve.

Also provides pinctrl nodes for devices enabling the riva-pil.

Cc: John Stultz 
Signed-off-by: Bjorn Andersson 
---
 arch/arm/boot/dts/qcom-apq8064-pins.dtsi | 18 +
 arch/arm/boot/dts/qcom-apq8064.dtsi  | 69 +++-
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi 
b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
index 6b801e7e57a2..5c023e649882 100644
--- a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
@@ -284,4 +284,22 @@
bias-disable = <0>;
};
};
+
+   riva_fm_pin_a: riva-fm-active {
+   pins = "gpio14", "gpio15";
+   function = "riva_fm";
+   };
+
+   riva_bt_pin_a: riva-bt-active {
+   pins = "gpio16", "gpio17";
+   function = "riva_bt";
+   };
+
+   riva_wlan_pin_a: riva-wlan-active {
+   pins = "gpio64", "gpio65", "gpio66", "gpio67", "gpio68";
+   function = "riva_wlan";
+
+   drive-strength = <6>;
+   bias-pull-down;
+   };
 };
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi 
b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 78bf155a52f3..3dc7a7aa3450 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -21,6 +21,11 @@
reg = <0x8000 0x20>;
no-map;
};
+
+   wcnss_mem: wcnss@8f00 {
+   reg = <0x8f00 0x70>;
+   no-map;
+   };
};
 
cpus {
@@ -179,7 +184,7 @@
};
 
clocks {
-   cxo_board {
+   cxo_board: cxo_board {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <1920>;
@@ -1419,6 +1424,68 @@
};
};
};
+
+   riva: riva-pil@3204000 {
+   compatible = "qcom,riva-pil";
+
+   reg = <0x03200800 0x1000>, <0x03202000 0x2000>, 
<0x03204000 0x100>;
+   reg-names = "ccu", "dxe", "pmu";
+
+   interrupts-extended = < GIC_SPI 199 
IRQ_TYPE_EDGE_RISING>,
+ <_smsm 6 
IRQ_TYPE_EDGE_RISING>;
+   interrupt-names = "wdog", "fatal";
+
+   memory-region = <_mem>;
+
+   vddcx-supply = <_s3>;
+   vddmx-supply = <_l24>;
+   vddpx-supply = <_s4>;
+
+   status = "disabled";
+
+   iris {
+   compatible = "qcom,wcn3660";
+
+   clocks = <_board>;
+   clock-names = "xo";
+
+   vddxo-supply = <_l4>;
+   vddrfa-supply = <_s2>;
+   vddpa-supply = <_l10>;
+   vdddig-supply = <_lvs2>;
+   };
+
+   smd-edge {
+   interrupts = ;
+
+   qcom,ipc = < 8 25>;
+   qcom,smd-edge = <6>;
+
+   label = "riva";
+
+   wcnss {
+   compatible = "qcom,wcnss";
+   qcom,smd-channels = "WCNSS_CTRL";
+
+   qcom,mmio = <>;
+
+   bt {
+   compatible = "qcom,wcnss-bt";
+   };
+
+   wifi {
+   compatible = "qcom,wcnss-wlan";
+
+   interrupts = ,
+;
+   interrupt-names = "tx", "rx";
+
+   qcom,smem-states = <_smsm 
10>, <_smsm 9>;
+   qcom,smem-state-names = 
"tx-enable", "tx-rings-empty";
+   };
+   };
+   };
+   };
};
 };
 #include "qcom-apq8064-pins.dtsi"
-- 
2.11.0

[PATCH 2/5] ARM: dts: qcom: apq8064: Add riva-pil node

[Bjorn Andersson]

Add nodes for the Riva PIL, IRIS RF module, BT and WiFI services exposed
by the Riva firmware and the related memory reserve.

Also provides pinctrl nodes for devices enabling the riva-pil.

Cc: John Stultz 
Signed-off-by: Bjorn Andersson 
---
 arch/arm/boot/dts/qcom-apq8064-pins.dtsi | 18 +
 arch/arm/boot/dts/qcom-apq8064.dtsi  | 69 +++-
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi 
b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
index 6b801e7e57a2..5c023e649882 100644
--- a/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064-pins.dtsi
@@ -284,4 +284,22 @@
bias-disable = <0>;
};
};
+
+   riva_fm_pin_a: riva-fm-active {
+   pins = "gpio14", "gpio15";
+   function = "riva_fm";
+   };
+
+   riva_bt_pin_a: riva-bt-active {
+   pins = "gpio16", "gpio17";
+   function = "riva_bt";
+   };
+
+   riva_wlan_pin_a: riva-wlan-active {
+   pins = "gpio64", "gpio65", "gpio66", "gpio67", "gpio68";
+   function = "riva_wlan";
+
+   drive-strength = <6>;
+   bias-pull-down;
+   };
 };
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi 
b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 78bf155a52f3..3dc7a7aa3450 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -21,6 +21,11 @@
reg = <0x8000 0x20>;
no-map;
};
+
+   wcnss_mem: wcnss@8f00 {
+   reg = <0x8f00 0x70>;
+   no-map;
+   };
};
 
cpus {
@@ -179,7 +184,7 @@
};
 
clocks {
-   cxo_board {
+   cxo_board: cxo_board {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <1920>;
@@ -1419,6 +1424,68 @@
};
};
};
+
+   riva: riva-pil@3204000 {
+   compatible = "qcom,riva-pil";
+
+   reg = <0x03200800 0x1000>, <0x03202000 0x2000>, 
<0x03204000 0x100>;
+   reg-names = "ccu", "dxe", "pmu";
+
+   interrupts-extended = < GIC_SPI 199 
IRQ_TYPE_EDGE_RISING>,
+ <_smsm 6 
IRQ_TYPE_EDGE_RISING>;
+   interrupt-names = "wdog", "fatal";
+
+   memory-region = <_mem>;
+
+   vddcx-supply = <_s3>;
+   vddmx-supply = <_l24>;
+   vddpx-supply = <_s4>;
+
+   status = "disabled";
+
+   iris {
+   compatible = "qcom,wcn3660";
+
+   clocks = <_board>;
+   clock-names = "xo";
+
+   vddxo-supply = <_l4>;
+   vddrfa-supply = <_s2>;
+   vddpa-supply = <_l10>;
+   vdddig-supply = <_lvs2>;
+   };
+
+   smd-edge {
+   interrupts = ;
+
+   qcom,ipc = < 8 25>;
+   qcom,smd-edge = <6>;
+
+   label = "riva";
+
+   wcnss {
+   compatible = "qcom,wcnss";
+   qcom,smd-channels = "WCNSS_CTRL";
+
+   qcom,mmio = <>;
+
+   bt {
+   compatible = "qcom,wcnss-bt";
+   };
+
+   wifi {
+   compatible = "qcom,wcnss-wlan";
+
+   interrupts = ,
+;
+   interrupt-names = "tx", "rx";
+
+   qcom,smem-states = <_smsm 
10>, <_smsm 9>;
+   qcom,smem-state-names = 
"tx-enable", "tx-rings-empty";
+   };
+   };
+   };
+   };
};
 };
 #include "qcom-apq8064-pins.dtsi"
-- 
2.11.0