Re: [PATCH v4 1/5] sched/topology: Add check to backup comment about hotplug lock
On Wed, 13 Jun 2018 14:17:07 +0200 Juri Lelli wrote: > From: Mathieu Poirier > > The comment above function partition_sched_domains() clearly state that > the cpu_hotplug_lock should be held but doesn't mandate one to abide to > it. > > Add an explicit check backing that comment, so to make it impossible > for anyone to miss the requirement. > > Suggested-by: Juri Lelli > Signed-off-by: Mathieu Poirier > [modified changelog] > Signed-off-by: Juri Lelli Reviewed-by: Steven Rostedt (VMware) -- Steve > --- > kernel/sched/topology.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c > index 61a1125c1ae4..96eee22fafe8 100644 > --- a/kernel/sched/topology.c > +++ b/kernel/sched/topology.c > @@ -1858,6 +1858,7 @@ void partition_sched_domains(int ndoms_new, > cpumask_var_t doms_new[], > int i, j, n; > int new_topology; > > + lockdep_assert_cpus_held(); > mutex_lock(_domains_mutex); > > /* Always unregister in case we don't destroy any domains: */
Re: [PATCH v4 1/5] sched/topology: Add check to backup comment about hotplug lock
On Wed, 13 Jun 2018 14:17:07 +0200 Juri Lelli wrote: > From: Mathieu Poirier > > The comment above function partition_sched_domains() clearly state that > the cpu_hotplug_lock should be held but doesn't mandate one to abide to > it. > > Add an explicit check backing that comment, so to make it impossible > for anyone to miss the requirement. > > Suggested-by: Juri Lelli > Signed-off-by: Mathieu Poirier > [modified changelog] > Signed-off-by: Juri Lelli Reviewed-by: Steven Rostedt (VMware) -- Steve > --- > kernel/sched/topology.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c > index 61a1125c1ae4..96eee22fafe8 100644 > --- a/kernel/sched/topology.c > +++ b/kernel/sched/topology.c > @@ -1858,6 +1858,7 @@ void partition_sched_domains(int ndoms_new, > cpumask_var_t doms_new[], > int i, j, n; > int new_topology; > > + lockdep_assert_cpus_held(); > mutex_lock(_domains_mutex); > > /* Always unregister in case we don't destroy any domains: */
Re: Restartable Sequences system call merged into Linux
On 06/14/2018 03:25 PM, Pavel Machek wrote: But the proposal wanted to add a syscall to thread creation, right? And I believe that may be noticeable. We already call set_robust_list, so we could just pass a larger area to that and the kernel could use it. Then no additional system call would be needed in the common case (new kernel which recognizes the new area size). But then we cannot use an initial-exec thread local variable for it (although the offset from the thread pointer will still be constant, of course). Thanks, Florian
Re: Restartable Sequences system call merged into Linux
On 06/14/2018 03:25 PM, Pavel Machek wrote: But the proposal wanted to add a syscall to thread creation, right? And I believe that may be noticeable. We already call set_robust_list, so we could just pass a larger area to that and the kernel could use it. Then no additional system call would be needed in the common case (new kernel which recognizes the new area size). But then we cannot use an initial-exec thread local variable for it (although the offset from the thread pointer will still be constant, of course). Thanks, Florian
Re: [PATCH] drivers/of: Add devm_of_iomap()
Hi Benjamin, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.17 next-20180614] [cannot apply to glikely/devicetree/next] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Benjamin-Herrenschmidt/drivers-of-Add-devm_of_iomap/20180612-080800 config: um-allyesconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=um All errors (new ones prefixed by >>): arch/um/drivers/vde.o: In function `vde_open_real': (.text+0x951): warning: Using 'getgrnam' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking (.text+0x79c): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/vector_user.o: In function `user_init_socket_fds': vector_user.c:(.text+0x334): warning: Using 'getaddrinfo' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametoaddr': (.text+0xdee5): warning: Using 'gethostbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametonetaddr': (.text+0xdf85): warning: Using 'getnetbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametoproto': (.text+0xe1a5): warning: Using 'getprotobyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametoport': (.text+0xdfd7): warning: Using 'getservbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking drivers/of/address.o: In function `devm_of_iomap': >> (.text+0x1882): undefined reference to `devm_ioremap_resource' collect2: error: ld returned 1 exit status --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH] drivers/of: Add devm_of_iomap()
Hi Benjamin, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.17 next-20180614] [cannot apply to glikely/devicetree/next] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Benjamin-Herrenschmidt/drivers-of-Add-devm_of_iomap/20180612-080800 config: um-allyesconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=um All errors (new ones prefixed by >>): arch/um/drivers/vde.o: In function `vde_open_real': (.text+0x951): warning: Using 'getgrnam' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking (.text+0x79c): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/vector_user.o: In function `user_init_socket_fds': vector_user.c:(.text+0x334): warning: Using 'getaddrinfo' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametoaddr': (.text+0xdee5): warning: Using 'gethostbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametonetaddr': (.text+0xdf85): warning: Using 'getnetbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametoproto': (.text+0xe1a5): warning: Using 'getprotobyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking arch/um/drivers/pcap.o: In function `pcap_nametoport': (.text+0xdfd7): warning: Using 'getservbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking drivers/of/address.o: In function `devm_of_iomap': >> (.text+0x1882): undefined reference to `devm_ioremap_resource' collect2: error: ld returned 1 exit status --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: Restartable Sequences system call merged into Linux
Hi! > >> It should be noted that there can be only one rseq TLS area > >> registered per > >> thread, > >> which can then be used by many libraries and by the executable, so > >> this is a > >> process-wide (per-thread) resource that we need to manage carefully. > >> >>> > >> >>> Is it possible to resize the area after thread creation, perhaps even > >> >>> from other threads? > >> >> > >> >> I'm not sure why we would want to resize it. The per-thread area is > >> >> fixed-size. > >> >> Its layout is here: include/uapi/linux/rseq.h: struct rseq > >> > > >> > Looks I was mistaken and this is very similar to the robust mutex list. > >> > > >> > Should we treat it the same way? Always allocate it for each new thread > >> > and register it with the kernel? > >> > >> That would be an efficient way to do it, indeed. There is very little > >> performance overhead to have rseq registered for all threads, whether or > >> not they intend to run rseq critical sections. > > > > People with slow / low memory machines would prefer not to see > > overhead they don't need... > > In terms of memory usage, if people don't want the extra few bytes of memory > used by rseq in the kernel, they should use CONFIG_RSEQ=n. > > In terms of overhead, let's have a closer look at what it means: when a thread > is registered to rseq, but does not enter rseq critical sections, only this > extra work is done by the kernel: > > - rseq_preempt(): on preemption, the scheduler sets the TIF_NOTIFY_RESUME > thread > flag, so rseq_handle_notify_resume() can check whether it's in a rseq > critical > section when returning to user-space, > - rseq_signal_deliver(): on signal delivery, rseq_handle_notify_resume() > checks > whether it's in a rseq critical section, > - rseq_migrate: on migration, the scheduler sets TIF_NOTIFY_RESUME as well, Yes, this is not likely to be noticeable. But the proposal wanted to add a syscall to thread creation, right? And I believe that may be noticeable. 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: Restartable Sequences system call merged into Linux
Hi! > >> It should be noted that there can be only one rseq TLS area > >> registered per > >> thread, > >> which can then be used by many libraries and by the executable, so > >> this is a > >> process-wide (per-thread) resource that we need to manage carefully. > >> >>> > >> >>> Is it possible to resize the area after thread creation, perhaps even > >> >>> from other threads? > >> >> > >> >> I'm not sure why we would want to resize it. The per-thread area is > >> >> fixed-size. > >> >> Its layout is here: include/uapi/linux/rseq.h: struct rseq > >> > > >> > Looks I was mistaken and this is very similar to the robust mutex list. > >> > > >> > Should we treat it the same way? Always allocate it for each new thread > >> > and register it with the kernel? > >> > >> That would be an efficient way to do it, indeed. There is very little > >> performance overhead to have rseq registered for all threads, whether or > >> not they intend to run rseq critical sections. > > > > People with slow / low memory machines would prefer not to see > > overhead they don't need... > > In terms of memory usage, if people don't want the extra few bytes of memory > used by rseq in the kernel, they should use CONFIG_RSEQ=n. > > In terms of overhead, let's have a closer look at what it means: when a thread > is registered to rseq, but does not enter rseq critical sections, only this > extra work is done by the kernel: > > - rseq_preempt(): on preemption, the scheduler sets the TIF_NOTIFY_RESUME > thread > flag, so rseq_handle_notify_resume() can check whether it's in a rseq > critical > section when returning to user-space, > - rseq_signal_deliver(): on signal delivery, rseq_handle_notify_resume() > checks > whether it's in a rseq critical section, > - rseq_migrate: on migration, the scheduler sets TIF_NOTIFY_RESUME as well, Yes, this is not likely to be noticeable. But the proposal wanted to add a syscall to thread creation, right? And I believe that may be noticeable. 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 1/3] perf alias: Remove trailing newline when reading sysfs files
On 06/14/2018 06:48 AM, Thomas Richter wrote:
> Remove a trailing newline when reading sysfs file contents
> such as /sys/devices/cpum_cf/events/TX_NC_TEND.
> This show when verbose option -v is used.
>
> Output before:
> tx_nc_tend -> 'cpum_cf'/'event=0x008d
> '/
>
> Output after:
> tx_nc_tend -> 'cpum_cf'/'event=0x8d'/
>
> Signed-off-by: Thomas Richter
> ---
> tools/perf/util/pmu.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 7878934ebb23..26c79a9c4142 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -294,7 +294,7 @@ static int __perf_pmu__new_alias(struct list_head *list,
> char *dir, char *name,
>
> static int perf_pmu__new_alias(struct list_head *list, char *dir, char
> *name, FILE *file)
> {
> - char buf[256];
> + char *cp, buf[256];
> int ret;
>
> ret = fread(buf, 1, sizeof(buf), file);
> @@ -303,6 +303,11 @@ static int perf_pmu__new_alias(struct list_head *list,
> char *dir, char *name, FI
>
> buf[ret] = 0;
>
> + /* Remove trailing newline from sysfs file */
> + cp = strrchr(buf, '\n');
> + if (cp)
> + *cp = '\0';
A nit, perhaps, but this will search backwards through the entire string if a
newline is not found, which is the most common case, I presume. Would it be
more efficient to just look at the last character? Something like:
i = strlen(buf);
if (i > 0 && buf[i-1] == '\n')
buf[i-1] = '\0';
> +
> return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL,
> NULL,
>NULL, NULL, NULL);
> }
>
PC
Re: [RFC PATCH RESEND] tcp: avoid F-RTO if SACK and timestamps are disabled
On Thu, Jun 14, 2018 at 02:51:18PM +0300, Ilpo Järvinen wrote: > On Thu, 14 Jun 2018, Michal Kubecek wrote: > > On Thu, Jun 14, 2018 at 11:42:43AM +0300, Ilpo Järvinen wrote: > > > On Wed, 13 Jun 2018, Yuchung Cheng wrote: > > > > On Wed, Jun 13, 2018 at 9:55 AM, Michal Kubecek > > > > wrote: > > > > AFAICS RFC 5682 is not explicit about this and offers multiple options. > > Anyway, this is not essential and in most of the customer provided > > captures, it wasn't the case. > > Lacking the new segments is essential for hiding the actual bug as the > trace would look weird otherwise with a burst of new data segments (due > to the other bug). The trace wouldn't look so nice but it can be reproduced even with more data to send. I've copied an example below. I couldn't find a really nice one quickly so that first few retransmits (17:22:13.865105 through 17:23:05.841105) are without new data but starting at 17:23:58.189150, you can see that sending new (previously unsent) data may not suffice to break the loop. > > Normally, we would have timestamps (and even SACK). Without them, you > > cannot reliably recognize a dupack with changed window size from > > a spontaneous window update. > > No! The window should not update window on ACKs the receiver intends to > designate as "duplicate ACKs". That is not without some potential cost > though as it requires delaying window updates up to the next cumulative > ACK. In the non-SACK series one of the changes is fixing this for > non-SACK Linux TCP flows. That sounds like a reasonable change (at least at the first glance, I didn't think about it too deeply) but even if we fix Linux stack to behave like this, we cannot force everyone else to do the same. Michal Kubecek 17:22:13.660030 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 1101588007:1101650727, ack 1871152053, win 28, length 62720 17:22:13.660039 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294151936, win 12146, length 0 17:22:13.660047 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 62720:125440, ack 1, win 28, length 62720 17:22:13.660050 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294178816, win 12146, length 0 17:22:13.660052 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294196736, win 12146, length 0 17:22:13.660131 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 125440:188160, ack 1, win 28, length 62720 17:22:13.660142 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294223616, win 12146, length 0 17:22:13.660164 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 188160:250880, ack 1, win 28, length 62720 17:22:13.660171 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294250496, win 12146, length 0 17:22:13.660177 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294277376, win 12146, length 0 17:22:13.660181 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294304256, win 12146, length 0 17:22:13.660185 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294331136, win 12146, length 0 17:22:13.660196 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294349056, win 12146, length 0 17:22:13.660212 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 250880:313600, ack 1, win 28, length 62720 17:22:13.660224 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 313600:376320, ack 1, win 28, length 62720 17:22:13.660266 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294384896, win 12146, length 0 17:22:13.660292 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294411776, win 12146, length 0 17:22:13.660294 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294438656, win 12146, length 0 17:22:13.660295 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294465536, win 12146, length 0 17:22:13.660353 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 376320:439040, ack 1, win 28, length 62720 17:22:13.660377 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 439040:501760, ack 1, win 28, length 62720 17:22:13.660391 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294501376, win 12146, length 0 17:22:13.660396 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294519296, win 12146, length 0 17:22:13.660400 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 501760:564480, ack 1, win 28, length 62720 17:22:13.660409 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294555136, win 12146, length 0 17:22:13.660420 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294582016, win 12146, length 0 17:22:13.660434 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294608896, win 12146, length 0 17:22:13.660458 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 564480:627200, ack 1, win 28, length 62720 17:22:13.660515 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294644736, win 12146, length 0 17:22:13.660527 IP 10.31.112.14.30284 >
Re: [PATCH 1/3] perf alias: Remove trailing newline when reading sysfs files
On 06/14/2018 06:48 AM, Thomas Richter wrote:
> Remove a trailing newline when reading sysfs file contents
> such as /sys/devices/cpum_cf/events/TX_NC_TEND.
> This show when verbose option -v is used.
>
> Output before:
> tx_nc_tend -> 'cpum_cf'/'event=0x008d
> '/
>
> Output after:
> tx_nc_tend -> 'cpum_cf'/'event=0x8d'/
>
> Signed-off-by: Thomas Richter
> ---
> tools/perf/util/pmu.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 7878934ebb23..26c79a9c4142 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -294,7 +294,7 @@ static int __perf_pmu__new_alias(struct list_head *list,
> char *dir, char *name,
>
> static int perf_pmu__new_alias(struct list_head *list, char *dir, char
> *name, FILE *file)
> {
> - char buf[256];
> + char *cp, buf[256];
> int ret;
>
> ret = fread(buf, 1, sizeof(buf), file);
> @@ -303,6 +303,11 @@ static int perf_pmu__new_alias(struct list_head *list,
> char *dir, char *name, FI
>
> buf[ret] = 0;
>
> + /* Remove trailing newline from sysfs file */
> + cp = strrchr(buf, '\n');
> + if (cp)
> + *cp = '\0';
A nit, perhaps, but this will search backwards through the entire string if a
newline is not found, which is the most common case, I presume. Would it be
more efficient to just look at the last character? Something like:
i = strlen(buf);
if (i > 0 && buf[i-1] == '\n')
buf[i-1] = '\0';
> +
> return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL,
> NULL,
>NULL, NULL, NULL);
> }
>
PC
Re: [RFC PATCH RESEND] tcp: avoid F-RTO if SACK and timestamps are disabled
On Thu, Jun 14, 2018 at 02:51:18PM +0300, Ilpo Järvinen wrote: > On Thu, 14 Jun 2018, Michal Kubecek wrote: > > On Thu, Jun 14, 2018 at 11:42:43AM +0300, Ilpo Järvinen wrote: > > > On Wed, 13 Jun 2018, Yuchung Cheng wrote: > > > > On Wed, Jun 13, 2018 at 9:55 AM, Michal Kubecek > > > > wrote: > > > > AFAICS RFC 5682 is not explicit about this and offers multiple options. > > Anyway, this is not essential and in most of the customer provided > > captures, it wasn't the case. > > Lacking the new segments is essential for hiding the actual bug as the > trace would look weird otherwise with a burst of new data segments (due > to the other bug). The trace wouldn't look so nice but it can be reproduced even with more data to send. I've copied an example below. I couldn't find a really nice one quickly so that first few retransmits (17:22:13.865105 through 17:23:05.841105) are without new data but starting at 17:23:58.189150, you can see that sending new (previously unsent) data may not suffice to break the loop. > > Normally, we would have timestamps (and even SACK). Without them, you > > cannot reliably recognize a dupack with changed window size from > > a spontaneous window update. > > No! The window should not update window on ACKs the receiver intends to > designate as "duplicate ACKs". That is not without some potential cost > though as it requires delaying window updates up to the next cumulative > ACK. In the non-SACK series one of the changes is fixing this for > non-SACK Linux TCP flows. That sounds like a reasonable change (at least at the first glance, I didn't think about it too deeply) but even if we fix Linux stack to behave like this, we cannot force everyone else to do the same. Michal Kubecek 17:22:13.660030 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 1101588007:1101650727, ack 1871152053, win 28, length 62720 17:22:13.660039 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294151936, win 12146, length 0 17:22:13.660047 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 62720:125440, ack 1, win 28, length 62720 17:22:13.660050 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294178816, win 12146, length 0 17:22:13.660052 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294196736, win 12146, length 0 17:22:13.660131 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 125440:188160, ack 1, win 28, length 62720 17:22:13.660142 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294223616, win 12146, length 0 17:22:13.660164 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 188160:250880, ack 1, win 28, length 62720 17:22:13.660171 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294250496, win 12146, length 0 17:22:13.660177 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294277376, win 12146, length 0 17:22:13.660181 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294304256, win 12146, length 0 17:22:13.660185 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294331136, win 12146, length 0 17:22:13.660196 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294349056, win 12146, length 0 17:22:13.660212 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 250880:313600, ack 1, win 28, length 62720 17:22:13.660224 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 313600:376320, ack 1, win 28, length 62720 17:22:13.660266 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294384896, win 12146, length 0 17:22:13.660292 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294411776, win 12146, length 0 17:22:13.660294 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294438656, win 12146, length 0 17:22:13.660295 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294465536, win 12146, length 0 17:22:13.660353 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 376320:439040, ack 1, win 28, length 62720 17:22:13.660377 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 439040:501760, ack 1, win 28, length 62720 17:22:13.660391 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294501376, win 12146, length 0 17:22:13.660396 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294519296, win 12146, length 0 17:22:13.660400 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 501760:564480, ack 1, win 28, length 62720 17:22:13.660409 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294555136, win 12146, length 0 17:22:13.660420 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294582016, win 12146, length 0 17:22:13.660434 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294608896, win 12146, length 0 17:22:13.660458 IP 10.30.59.58.1556 > 10.31.112.14.30284: Flags [.], seq 564480:627200, ack 1, win 28, length 62720 17:22:13.660515 IP 10.31.112.14.30284 > 10.30.59.58.1556: Flags [.], ack 4294644736, win 12146, length 0 17:22:13.660527 IP 10.31.112.14.30284 >
Re: [PATCH 4/4] arm: dts: add support for Laird SOM60 module and DVK boards
On 14/06/2018 at 10:51, Ben Whitten wrote:
Signed-off-by: Ben Whitten
---
arch/arm/boot/dts/Makefile| 3 +-
arch/arm/boot/dts/at91-dvk_som60.dts | 95 +++
arch/arm/boot/dts/at91-dvk_su60_somc.dtsi | 159 ++
arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi | 96 +++
arch/arm/boot/dts/at91-som60.dtsi | 229 ++
5 files changed, 581 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/at91-dvk_som60.dts
create mode 100644 arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
create mode 100644 arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi
create mode 100644 arch/arm/boot/dts/at91-som60.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 486ab59..4d3d9ca 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -63,7 +63,8 @@ dtb-$(CONFIG_SOC_SAM_V7) += \
at91-sama5d4ek.dtb \
at91-vinco.dtb \
About where you added dtbs...
at91-wb50n.dtb \
- at91-gatwick.dtb
+ at91-gatwick.dtb \
+ at91-dvk_som60.dtb
1/ As they are based on sama5d3, I would like to see them between
"at91-sama5d2_xplained.dtb" and "sama5d31ek.dtb"
2/ within this range, please sort all these 4 alphabetically
3/ don't laugh at me, I try to deal with our historical way of "sorting"
entries in this Makefile for AT91... ;-)
BTW, I realize now that your "at91-wb45n.dtb" entry from patch 1 should
go just after the "at91-kizboxmini.dts" (alphabetical order in
at91sam9x5 "location").
dtb-$(CONFIG_ARCH_ATLAS6) += \
atlas6-evb.dtb
dtb-$(CONFIG_ARCH_ATLAS7) += \
diff --git a/arch/arm/boot/dts/at91-dvk_som60.dts
b/arch/arm/boot/dts/at91-dvk_som60.dts
new file mode 100644
index 000..ededd5b
--- /dev/null
+++ b/arch/arm/boot/dts/at91-dvk_som60.dts
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-dvk_som60.dts - Device Tree file for the DVK SOM60 board
+ *
+ * Copyright (C) 2018 Laird,
+ * 2018 Ben Whitten
+ *
+ */
+/dts-v1/;
+#include "at91-som60.dtsi"
+#include "at91-dvk_su60_somc.dtsi"
+#include "at91-dvk_su60_somc_lcm.dtsi"
+
+/ {
+ model = "Laird DVK SOM60";
+ compatible = "laird,dvk-som60", "laird,som60", "atmel,sama5d36", "atmel,sama5d3",
"atmel,sama5";
+
+ chosen {
+ stdout-path =
+ tick-timer =
+ };
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
diff --git a/arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
b/arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
new file mode 100644
index 000..6031c2f
--- /dev/null
+++ b/arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-dvk_su60_somc.dtsi - Device Tree file for the DVK SOM60 base board
+ *
+ * Copyright (C) 2018 Laird,
+ * 2018 Ben Whitten
+ *
+ */
+
+/ {
+ sound {
+ compatible = "atmel,asoc-wm8904";
+ pinctrl-names = "default";
+ pinctrl-0 = <_pck2_as_audio_mck>;
+
+ atmel,model = "wm8904 @ DVK-SOM60";
+ atmel,audio-routing =
+ "Headphone Jack", "HPOUTL",
+ "Headphone Jack", "HPOUTR",
+ "IN2L", "Line In Jack",
+ "IN2R", "Line In Jack",
+ "Mic", "MICBIAS",
+ "IN1L", "Mic";
+
+ atmel,ssc-controller = <>;
+ atmel,audio-codec = <>;
+
+ status = "okay";
+ };
+};
+
+ {
+ status = "okay";
+
+ pinctrl-0 = <_mmc0_clk_cmd_dat0 _mmc0_dat1_3
_mmc0_cd>;
+ slot@0 {
+ bus-width = <4>;
+ cd-gpios = < 31 GPIO_ACTIVE_HIGH>;
+ cd-inverted;
+ };
+};
+
+ {
+ status = "okay";
+
+ /* spi0.0: 4M Flash Macronix MX25R4035FM1IL0 */
+ spi-flash@0 {
+ compatible = "mxicy,mx25u4035", "jedec,spi-nor";
+ spi-max-frequency = <3300>;
+ reg = <0>;
+ };
+};
+
+ {
+ atmel,clk-from-rk-pin;
+ status = "okay";
+};
+
+ {
+ status = "okay";
+
+ wm8904: wm8904@1a {
+ compatible = "wlf,wm8904";
+ reg = <0x1a>;
+ clocks = <>;
+ clock-names = "mclk";
+ };
+};
+
+ {
+ status = "okay";
+
+ eeprom@87 {
+
Re: [PATCH 4/4] arm: dts: add support for Laird SOM60 module and DVK boards
On 14/06/2018 at 10:51, Ben Whitten wrote:
Signed-off-by: Ben Whitten
---
arch/arm/boot/dts/Makefile| 3 +-
arch/arm/boot/dts/at91-dvk_som60.dts | 95 +++
arch/arm/boot/dts/at91-dvk_su60_somc.dtsi | 159 ++
arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi | 96 +++
arch/arm/boot/dts/at91-som60.dtsi | 229 ++
5 files changed, 581 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/at91-dvk_som60.dts
create mode 100644 arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
create mode 100644 arch/arm/boot/dts/at91-dvk_su60_somc_lcm.dtsi
create mode 100644 arch/arm/boot/dts/at91-som60.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 486ab59..4d3d9ca 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -63,7 +63,8 @@ dtb-$(CONFIG_SOC_SAM_V7) += \
at91-sama5d4ek.dtb \
at91-vinco.dtb \
About where you added dtbs...
at91-wb50n.dtb \
- at91-gatwick.dtb
+ at91-gatwick.dtb \
+ at91-dvk_som60.dtb
1/ As they are based on sama5d3, I would like to see them between
"at91-sama5d2_xplained.dtb" and "sama5d31ek.dtb"
2/ within this range, please sort all these 4 alphabetically
3/ don't laugh at me, I try to deal with our historical way of "sorting"
entries in this Makefile for AT91... ;-)
BTW, I realize now that your "at91-wb45n.dtb" entry from patch 1 should
go just after the "at91-kizboxmini.dts" (alphabetical order in
at91sam9x5 "location").
dtb-$(CONFIG_ARCH_ATLAS6) += \
atlas6-evb.dtb
dtb-$(CONFIG_ARCH_ATLAS7) += \
diff --git a/arch/arm/boot/dts/at91-dvk_som60.dts
b/arch/arm/boot/dts/at91-dvk_som60.dts
new file mode 100644
index 000..ededd5b
--- /dev/null
+++ b/arch/arm/boot/dts/at91-dvk_som60.dts
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-dvk_som60.dts - Device Tree file for the DVK SOM60 board
+ *
+ * Copyright (C) 2018 Laird,
+ * 2018 Ben Whitten
+ *
+ */
+/dts-v1/;
+#include "at91-som60.dtsi"
+#include "at91-dvk_su60_somc.dtsi"
+#include "at91-dvk_su60_somc_lcm.dtsi"
+
+/ {
+ model = "Laird DVK SOM60";
+ compatible = "laird,dvk-som60", "laird,som60", "atmel,sama5d36", "atmel,sama5d3",
"atmel,sama5";
+
+ chosen {
+ stdout-path =
+ tick-timer =
+ };
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
diff --git a/arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
b/arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
new file mode 100644
index 000..6031c2f
--- /dev/null
+++ b/arch/arm/boot/dts/at91-dvk_su60_somc.dtsi
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-dvk_su60_somc.dtsi - Device Tree file for the DVK SOM60 base board
+ *
+ * Copyright (C) 2018 Laird,
+ * 2018 Ben Whitten
+ *
+ */
+
+/ {
+ sound {
+ compatible = "atmel,asoc-wm8904";
+ pinctrl-names = "default";
+ pinctrl-0 = <_pck2_as_audio_mck>;
+
+ atmel,model = "wm8904 @ DVK-SOM60";
+ atmel,audio-routing =
+ "Headphone Jack", "HPOUTL",
+ "Headphone Jack", "HPOUTR",
+ "IN2L", "Line In Jack",
+ "IN2R", "Line In Jack",
+ "Mic", "MICBIAS",
+ "IN1L", "Mic";
+
+ atmel,ssc-controller = <>;
+ atmel,audio-codec = <>;
+
+ status = "okay";
+ };
+};
+
+ {
+ status = "okay";
+
+ pinctrl-0 = <_mmc0_clk_cmd_dat0 _mmc0_dat1_3
_mmc0_cd>;
+ slot@0 {
+ bus-width = <4>;
+ cd-gpios = < 31 GPIO_ACTIVE_HIGH>;
+ cd-inverted;
+ };
+};
+
+ {
+ status = "okay";
+
+ /* spi0.0: 4M Flash Macronix MX25R4035FM1IL0 */
+ spi-flash@0 {
+ compatible = "mxicy,mx25u4035", "jedec,spi-nor";
+ spi-max-frequency = <3300>;
+ reg = <0>;
+ };
+};
+
+ {
+ atmel,clk-from-rk-pin;
+ status = "okay";
+};
+
+ {
+ status = "okay";
+
+ wm8904: wm8904@1a {
+ compatible = "wlf,wm8904";
+ reg = <0x1a>;
+ clocks = <>;
+ clock-names = "mclk";
+ };
+};
+
+ {
+ status = "okay";
+
+ eeprom@87 {
+
RE: [PATCH v4 0/7] add virt-dma support for imx-sdma
Hi Lucas,
I didn't met your unstable console issue before, anyway, I'll do more
test and look into Audio issue.
-Original Message-
From: Lucas Stach [mailto:l.st...@pengutronix.de]
Sent: 2018年6月14日 18:22
To: Robin Gong ; s.ha...@pengutronix.de; vk...@kernel.org;
dan.j.willi...@intel.com; gre...@linuxfoundation.org; jsl...@suse.com
Cc: dmaeng...@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-arm-ker...@lists.infradead.org; dl-linux-imx ;
linux-ser...@vger.kernel.org
Subject: Re: [PATCH v4 0/7] add virt-dma support for imx-sdma
Am Donnerstag, den 14.06.2018, 10:09 + schrieb Robin Gong:
> Hi Lucas,
> Could you double check again? Still can reproduce lockdep warning on
> UART if change spin_lock_lockirqsave/spinlock_unlock_irqrestore to
> spin_lock/spin_unlock in sdma_int_handler as you said without
> patch7/7.
Yes, I can reproduce the lockdep issue with _irqsave variants in the IRQ
handler and 7/7 reverted. It fixes the pcm issue though.
> Would you please ask below two more questions?
> 1. Does your uart case pass now with my v4 patchset?
It seems to work, but the change seems to impact the non-DMA serial somehow. On
several boots the system was unable to present a login prompt, so patch 7/7
seems to break other stuff and it's a pretty invasive change to the serial
driver, which is known to be non-trivial.
I don't have the bandwidth to dig through this patch currently, but I wonder if
it couldn't be simplified with the spinlock stuff in the sdma driver fixed.
> 2. Do you make some code change in your local('I just gave this series
> a spin')to reproduce your below issue? If yes, could you post your
> change?
The lockdep splat below was without any changes to your series.
Regards,
Lucas
> On 四, 2018-06-14 at 10:53 +0200, Lucas Stach wrote:
> > Hi Robin,
> >
> > I just gave this series a spin and it seems there is even more
> > locking fun, see the lockdep output below. After taking a short look
> > it seems this is caused by using the wrong spinlock variants in
> > sdma_int_handler(), those should also use the _irqsave ones. When
> > fixing this you might want to reconsider patch 7/7, as it's probably
> > not needed at all.
> >
> > Regards,
> > Lucas
> >
> > [ 20.479401]
> >
> > [ 20.485769] WARNING: possible irq lock inversion dependency
> > detected [ 20.492140] 4.17.0+ #1538 Not tainted [ 20.495814]
> >
> > --
> > --
> > [ 20.502181] systemd/1 just changed the state of lock:
> > [ 20.507247] c0abdafc (&(>self_group.lock)->rlock){-
> > ...}, at: snd_pcm_stream_lock+0x54/0x60 [ 20.516523] but this lock
> > took another, HARDIRQ-unsafe lock in the
> > past:
> > [ 20.523234] (fs_reclaim){+.+.}
> > [ 20.523253]
> > [ 20.523253]
> > [ 20.523253] and interrupts could create inverse lock ordering
> > between them.
> > [ 20.523253]
> > [ 20.537804]
> > [ 20.537804] other info that might help us debug this:
> > [ 20.544344] Possible interrupt unsafe locking scenario:
> > [ 20.544344]
> > [ 20.551144]CPU0CPU1 [ 20.555686]
> > [ 20.560224] lock(fs_reclaim); [
> > 20.563386]local_irq_disable(); [
> > 20.569315]lock(&(
> > > self_group.lock)->rlock);
> >
> > [ 20.577337]lock(fs_reclaim); [
> > 20.583014] [ 20.585643]
> > lock(&(>self_group.lock)->rlock);
> > [ 20.591322]
> > [ 20.591322] *** DEADLOCK ***
> > [ 20.591322]
> > [ 20.597260] 1 lock held by systemd/1:
> > [ 20.607806] #0: ab23d11c (snd_pcm_link_rwlock){.-..}, at:
> > snd_pcm_stream_lock+0x4c/0x60
> > [ 20.615951]
> > [ 20.615951] the shortest dependencies between 2nd lock and 1st
> > lock:
> > [ 20.623799] -> (fs_reclaim){+.+.} ops: 248474 { [ 20.628456]
> > HARDIRQ-ON-W at:
> > [ 20.631716] lock_acquire+0x260/0x29c [
> > 20.637223] fs_reclaim_acquire+0x48/0x58 [
> > 20.643075] kmem_cache_alloc_trace+0x3c/0x
> > 36
> > 4
> > [ 20.649366] alloc_worker.constprop.15+0x28
> > /0
> > x64
> > [ 20.655824] init_rescuer.part.5+0x20/0xa4 [
> > 20.661764] workqueue_init+0x124/0x1f8 [
> > 20.667446] kernel_init_freeable+0x60/0x55
> > 0
> > [ 20.673561] kernel_init+0x18/0x120 [
> > 20.678890] ret_from_fork+0x14/0x20 [
> > 20.684299] (null) [ 20.688408]
> > SOFTIRQ-ON-W at:
> > [ 20.691659] lock_acquire+0x260/0x29c [
> > 20.697158] fs_reclaim_acquire+0x48/0x58 [
> > 20.703006] kmem_cache_alloc_trace+0x3c/0x
> > 36
> > 4
> > [
RE: [PATCH v4 0/7] add virt-dma support for imx-sdma
Hi Lucas,
I didn't met your unstable console issue before, anyway, I'll do more
test and look into Audio issue.
-Original Message-
From: Lucas Stach [mailto:l.st...@pengutronix.de]
Sent: 2018年6月14日 18:22
To: Robin Gong ; s.ha...@pengutronix.de; vk...@kernel.org;
dan.j.willi...@intel.com; gre...@linuxfoundation.org; jsl...@suse.com
Cc: dmaeng...@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-arm-ker...@lists.infradead.org; dl-linux-imx ;
linux-ser...@vger.kernel.org
Subject: Re: [PATCH v4 0/7] add virt-dma support for imx-sdma
Am Donnerstag, den 14.06.2018, 10:09 + schrieb Robin Gong:
> Hi Lucas,
> Could you double check again? Still can reproduce lockdep warning on
> UART if change spin_lock_lockirqsave/spinlock_unlock_irqrestore to
> spin_lock/spin_unlock in sdma_int_handler as you said without
> patch7/7.
Yes, I can reproduce the lockdep issue with _irqsave variants in the IRQ
handler and 7/7 reverted. It fixes the pcm issue though.
> Would you please ask below two more questions?
> 1. Does your uart case pass now with my v4 patchset?
It seems to work, but the change seems to impact the non-DMA serial somehow. On
several boots the system was unable to present a login prompt, so patch 7/7
seems to break other stuff and it's a pretty invasive change to the serial
driver, which is known to be non-trivial.
I don't have the bandwidth to dig through this patch currently, but I wonder if
it couldn't be simplified with the spinlock stuff in the sdma driver fixed.
> 2. Do you make some code change in your local('I just gave this series
> a spin')to reproduce your below issue? If yes, could you post your
> change?
The lockdep splat below was without any changes to your series.
Regards,
Lucas
> On 四, 2018-06-14 at 10:53 +0200, Lucas Stach wrote:
> > Hi Robin,
> >
> > I just gave this series a spin and it seems there is even more
> > locking fun, see the lockdep output below. After taking a short look
> > it seems this is caused by using the wrong spinlock variants in
> > sdma_int_handler(), those should also use the _irqsave ones. When
> > fixing this you might want to reconsider patch 7/7, as it's probably
> > not needed at all.
> >
> > Regards,
> > Lucas
> >
> > [ 20.479401]
> >
> > [ 20.485769] WARNING: possible irq lock inversion dependency
> > detected [ 20.492140] 4.17.0+ #1538 Not tainted [ 20.495814]
> >
> > --
> > --
> > [ 20.502181] systemd/1 just changed the state of lock:
> > [ 20.507247] c0abdafc (&(>self_group.lock)->rlock){-
> > ...}, at: snd_pcm_stream_lock+0x54/0x60 [ 20.516523] but this lock
> > took another, HARDIRQ-unsafe lock in the
> > past:
> > [ 20.523234] (fs_reclaim){+.+.}
> > [ 20.523253]
> > [ 20.523253]
> > [ 20.523253] and interrupts could create inverse lock ordering
> > between them.
> > [ 20.523253]
> > [ 20.537804]
> > [ 20.537804] other info that might help us debug this:
> > [ 20.544344] Possible interrupt unsafe locking scenario:
> > [ 20.544344]
> > [ 20.551144]CPU0CPU1 [ 20.555686]
> > [ 20.560224] lock(fs_reclaim); [
> > 20.563386]local_irq_disable(); [
> > 20.569315]lock(&(
> > > self_group.lock)->rlock);
> >
> > [ 20.577337]lock(fs_reclaim); [
> > 20.583014] [ 20.585643]
> > lock(&(>self_group.lock)->rlock);
> > [ 20.591322]
> > [ 20.591322] *** DEADLOCK ***
> > [ 20.591322]
> > [ 20.597260] 1 lock held by systemd/1:
> > [ 20.607806] #0: ab23d11c (snd_pcm_link_rwlock){.-..}, at:
> > snd_pcm_stream_lock+0x4c/0x60
> > [ 20.615951]
> > [ 20.615951] the shortest dependencies between 2nd lock and 1st
> > lock:
> > [ 20.623799] -> (fs_reclaim){+.+.} ops: 248474 { [ 20.628456]
> > HARDIRQ-ON-W at:
> > [ 20.631716] lock_acquire+0x260/0x29c [
> > 20.637223] fs_reclaim_acquire+0x48/0x58 [
> > 20.643075] kmem_cache_alloc_trace+0x3c/0x
> > 36
> > 4
> > [ 20.649366] alloc_worker.constprop.15+0x28
> > /0
> > x64
> > [ 20.655824] init_rescuer.part.5+0x20/0xa4 [
> > 20.661764] workqueue_init+0x124/0x1f8 [
> > 20.667446] kernel_init_freeable+0x60/0x55
> > 0
> > [ 20.673561] kernel_init+0x18/0x120 [
> > 20.678890] ret_from_fork+0x14/0x20 [
> > 20.684299] (null) [ 20.688408]
> > SOFTIRQ-ON-W at:
> > [ 20.691659] lock_acquire+0x260/0x29c [
> > 20.697158] fs_reclaim_acquire+0x48/0x58 [
> > 20.703006] kmem_cache_alloc_trace+0x3c/0x
> > 36
> > 4
> > [
Re: [PATCH v5 04/13] s390: vfio-ap: base implementation of VFIO AP device driver
On 06/13/2018 08:16 AM, Pierre Morel wrote:
On 13/06/2018 14:12, Cornelia Huck wrote:
On Wed, 13 Jun 2018 14:01:54 +0200
Pierre Morel wrote:
On 13/06/2018 13:14, Cornelia Huck wrote:
On Wed, 13 Jun 2018 12:54:40 +0200
Pierre Morel wrote:
On 13/06/2018 09:48, Cornelia Huck wrote:
On Wed, 13 Jun 2018 09:41:16 +0200
Pierre Morel wrote:
On 07/05/2018 17:11, Tony Krowiak wrote:
Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.
...snip...
+static int vfio_ap_matrix_dev_create(void)
+{
+int ret;
+
+vfio_ap_root_device =
root_device_register(VFIO_AP_ROOT_NAME);
+
+if (IS_ERR(vfio_ap_root_device)) {
+ret = PTR_ERR(vfio_ap_root_device);
+goto done;
+}
+
+ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL);
+if (!ap_matrix) {
+ret = -ENOMEM;
+goto matrix_alloc_err;
+}
+
+ap_matrix->device.type = _ap_dev_type;
+dev_set_name(_matrix->device, "%s", VFIO_AP_DEV_NAME);
+ap_matrix->device.parent = vfio_ap_root_device;
+ap_matrix->device.release = vfio_ap_matrix_dev_release;
+ap_matrix->device.driver = _ap_drv.driver;
+
+ret = device_register(_matrix->device);
+if (ret)
+goto matrix_reg_err;
+
+goto done;
+
+matrix_reg_err:
+put_device(_matrix->device);
Did not see this before but here you certainly want to
do a kfree and not a put_device.
No, this must not be a kfree. Once you've tried to register
something
embedding a struct device with the driver core, you need to use
put_device, as another path may have acquired a reference, even if
registering ultimately failed. See the comment for
device_register().
IOW, the code is correct.
learned something again :) ,
but still, a kfree is needed for the kzalloc.
Does'nt it?
No, the put callback for the embedding structure needs to take care of
freeing things. Otherwise it is buggy.
Seems buggy to me.
How does the put_device knows if it is embedded and then in what it is
embedded ?
It does not need to know; the code registering the structure needs to
set up device->release correctly.
yes right, thanks.
See the vfio_ap_matrix_dev_release() callback.
+
+matrix_alloc_err:
+root_device_unregister(vfio_ap_root_device);
+
+done:
+return ret;
+}
+
+static void vfio_ap_matrix_dev_destroy(struct ap_matrix
*ap_matrix)
+{
+device_unregister(_matrix->device);
+root_device_unregister(vfio_ap_root_device);
Also here you need a kfree(ap_matrix) too .
Same here.
Re: [PATCH v5 04/13] s390: vfio-ap: base implementation of VFIO AP device driver
On 06/13/2018 08:16 AM, Pierre Morel wrote:
On 13/06/2018 14:12, Cornelia Huck wrote:
On Wed, 13 Jun 2018 14:01:54 +0200
Pierre Morel wrote:
On 13/06/2018 13:14, Cornelia Huck wrote:
On Wed, 13 Jun 2018 12:54:40 +0200
Pierre Morel wrote:
On 13/06/2018 09:48, Cornelia Huck wrote:
On Wed, 13 Jun 2018 09:41:16 +0200
Pierre Morel wrote:
On 07/05/2018 17:11, Tony Krowiak wrote:
Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.
...snip...
+static int vfio_ap_matrix_dev_create(void)
+{
+int ret;
+
+vfio_ap_root_device =
root_device_register(VFIO_AP_ROOT_NAME);
+
+if (IS_ERR(vfio_ap_root_device)) {
+ret = PTR_ERR(vfio_ap_root_device);
+goto done;
+}
+
+ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL);
+if (!ap_matrix) {
+ret = -ENOMEM;
+goto matrix_alloc_err;
+}
+
+ap_matrix->device.type = _ap_dev_type;
+dev_set_name(_matrix->device, "%s", VFIO_AP_DEV_NAME);
+ap_matrix->device.parent = vfio_ap_root_device;
+ap_matrix->device.release = vfio_ap_matrix_dev_release;
+ap_matrix->device.driver = _ap_drv.driver;
+
+ret = device_register(_matrix->device);
+if (ret)
+goto matrix_reg_err;
+
+goto done;
+
+matrix_reg_err:
+put_device(_matrix->device);
Did not see this before but here you certainly want to
do a kfree and not a put_device.
No, this must not be a kfree. Once you've tried to register
something
embedding a struct device with the driver core, you need to use
put_device, as another path may have acquired a reference, even if
registering ultimately failed. See the comment for
device_register().
IOW, the code is correct.
learned something again :) ,
but still, a kfree is needed for the kzalloc.
Does'nt it?
No, the put callback for the embedding structure needs to take care of
freeing things. Otherwise it is buggy.
Seems buggy to me.
How does the put_device knows if it is embedded and then in what it is
embedded ?
It does not need to know; the code registering the structure needs to
set up device->release correctly.
yes right, thanks.
See the vfio_ap_matrix_dev_release() callback.
+
+matrix_alloc_err:
+root_device_unregister(vfio_ap_root_device);
+
+done:
+return ret;
+}
+
+static void vfio_ap_matrix_dev_destroy(struct ap_matrix
*ap_matrix)
+{
+device_unregister(_matrix->device);
+root_device_unregister(vfio_ap_root_device);
Also here you need a kfree(ap_matrix) too .
Same here.
I will definately enlighten you more as soon as I hear from you
Good Morning, Although, I am not comfortable discussing the content of my mail on the Internet owing to lots of unsolicited/Spam mails on the net nowadays. However, I have made up my mind to will my late Husband's funds to you so you can use it for charity duties and good work to humanity. The amounts are relatively huge, but please get back to me for further information. I will definately enlighten you more as soon as I hear from you. Best of wishes to you, Mrs. Rania Hassya
I will definately enlighten you more as soon as I hear from you
Good Morning, Although, I am not comfortable discussing the content of my mail on the Internet owing to lots of unsolicited/Spam mails on the net nowadays. However, I have made up my mind to will my late Husband's funds to you so you can use it for charity duties and good work to humanity. The amounts are relatively huge, but please get back to me for further information. I will definately enlighten you more as soon as I hear from you. Best of wishes to you, Mrs. Rania Hassya
BUG: unable to handle kernel NULL pointer dereference in vmx_set_msr
Hello, syzbot found the following crash on: HEAD commit:be779f03d563 Merge tag 'kbuild-v4.18-2' of git://git.kerne.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=10f9185f80 kernel config: https://syzkaller.appspot.com/x/.config?x=855fb54e1e019da2 dashboard link: https://syzkaller.appspot.com/bug?extid=405a50b23dd790f609b1 compiler: gcc (GCC) 8.0.1 20180413 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+405a50b23dd790f60...@syzkaller.appspotmail.com BUG: unable to handle kernel NULL pointer dereference at PGD 1890f1067 P4D 1890f1067 PUD 1890f2067 PMD 0 netlink: 8 bytes leftover after parsing attributes in process `syz-executor0'. Oops: 0010 [#1] SMP KASAN CPU: 0 PID: 12381 Comm: syz-executor7 Not tainted 4.17.0+ #101 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010: (null) Code: unchecked MSR access error: WRMSR to 0x49 (tried to write 0x0001) at rIP: 0x81347e18 (native_write_msr+0x8/0x30) Bad RIP value. RSP: 0018:88019151f4c0 EFLAGS: 00010246 RAX: RBX: 8801d2dca800 RCX: 110eac7d RDX: 88019151f9e0 RSI: 8801c922da40 RDI: 8801c4a5a540 RBP: 88019151f630 R08: 0001 R09: Call Trace: R10: R11: R12: 1100322a3e9d R13: 88019151f9e0 R14: 8801d2dca812 R15: 8801d2dcac58 FS: 7f5039c16700() GS:8801dae0() knlGS: CS: 0010 DS: ES: CR0: 80050033 paravirt_write_msr arch/x86/include/asm/paravirt.h:117 [inline] wrmsrl arch/x86/include/asm/paravirt.h:150 [inline] vmx_set_msr+0x19b/0x2010 arch/x86/kvm/vmx.c:3901 CR2: ffd6 CR3: 0001890f CR4: 001426f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: kvm_set_msr+0x18a/0x370 arch/x86/kvm/x86.c:1208 do_set_msr+0x106/0x190 arch/x86/kvm/x86.c:1237 sock_poll+0x1d1/0x710 net/socket.c:1168 vfs_poll+0x77/0x2a0 fs/select.c:40 do_pollfd fs/select.c:848 [inline] do_poll fs/select.c:896 [inline] do_sys_poll+0x6fd/0x1100 fs/select.c:990 __msr_io arch/x86/kvm/x86.c:2784 [inline] msr_io+0x21a/0x360 arch/x86/kvm/x86.c:2820 kvm_arch_vcpu_ioctl+0x14cb/0x36f0 arch/x86/kvm/x86.c:3782 __do_sys_ppoll fs/select.c:1098 [inline] __se_sys_ppoll fs/select.c:1070 [inline] __x64_sys_ppoll+0x2fa/0x5f0 fs/select.c:1070 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x455b29 Code: 1d ba fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb b9 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f5039c15c68 EFLAGS: 0246 ORIG_RAX: 010f RAX: ffda RBX: 7f5039c166d4 RCX: 00455b29 RDX: 2140 RSI: 0001 RDI: 20c0 RBP: 0072bea0 R08: 0008 R09: R10: 2180 R11: 0246 R12: R13: 004c05f3 R14: 004cfb20 R15: Modules linked in: Dumping ftrace buffer: (ftrace buffer empty) CR2: ---[ end trace 2f889d9e79d87fbd ]--- RIP: 0010: (null) Code: Bad RIP value. RSP: 0018:88019151f4c0 EFLAGS: 00010246 RAX: RBX: 8801d2dca800 RCX: 110eac7d RDX: 88019151f9e0 RSI: 8801c922da40 RDI: 8801c4a5a540 RBP: 88019151f630 R08: 0001 R09: R10: R11: R12: 1100322a3e9d R13: 88019151f9e0 R14: 8801d2dca812 R15: 8801d2dcac58 FS: 7f5039c16700() GS:8801dae0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: ffd6 CR3: 0001890f CR4: 001426f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
BUG: unable to handle kernel NULL pointer dereference in vmx_set_msr
Hello, syzbot found the following crash on: HEAD commit:be779f03d563 Merge tag 'kbuild-v4.18-2' of git://git.kerne.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=10f9185f80 kernel config: https://syzkaller.appspot.com/x/.config?x=855fb54e1e019da2 dashboard link: https://syzkaller.appspot.com/bug?extid=405a50b23dd790f609b1 compiler: gcc (GCC) 8.0.1 20180413 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+405a50b23dd790f60...@syzkaller.appspotmail.com BUG: unable to handle kernel NULL pointer dereference at PGD 1890f1067 P4D 1890f1067 PUD 1890f2067 PMD 0 netlink: 8 bytes leftover after parsing attributes in process `syz-executor0'. Oops: 0010 [#1] SMP KASAN CPU: 0 PID: 12381 Comm: syz-executor7 Not tainted 4.17.0+ #101 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010: (null) Code: unchecked MSR access error: WRMSR to 0x49 (tried to write 0x0001) at rIP: 0x81347e18 (native_write_msr+0x8/0x30) Bad RIP value. RSP: 0018:88019151f4c0 EFLAGS: 00010246 RAX: RBX: 8801d2dca800 RCX: 110eac7d RDX: 88019151f9e0 RSI: 8801c922da40 RDI: 8801c4a5a540 RBP: 88019151f630 R08: 0001 R09: Call Trace: R10: R11: R12: 1100322a3e9d R13: 88019151f9e0 R14: 8801d2dca812 R15: 8801d2dcac58 FS: 7f5039c16700() GS:8801dae0() knlGS: CS: 0010 DS: ES: CR0: 80050033 paravirt_write_msr arch/x86/include/asm/paravirt.h:117 [inline] wrmsrl arch/x86/include/asm/paravirt.h:150 [inline] vmx_set_msr+0x19b/0x2010 arch/x86/kvm/vmx.c:3901 CR2: ffd6 CR3: 0001890f CR4: 001426f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: kvm_set_msr+0x18a/0x370 arch/x86/kvm/x86.c:1208 do_set_msr+0x106/0x190 arch/x86/kvm/x86.c:1237 sock_poll+0x1d1/0x710 net/socket.c:1168 vfs_poll+0x77/0x2a0 fs/select.c:40 do_pollfd fs/select.c:848 [inline] do_poll fs/select.c:896 [inline] do_sys_poll+0x6fd/0x1100 fs/select.c:990 __msr_io arch/x86/kvm/x86.c:2784 [inline] msr_io+0x21a/0x360 arch/x86/kvm/x86.c:2820 kvm_arch_vcpu_ioctl+0x14cb/0x36f0 arch/x86/kvm/x86.c:3782 __do_sys_ppoll fs/select.c:1098 [inline] __se_sys_ppoll fs/select.c:1070 [inline] __x64_sys_ppoll+0x2fa/0x5f0 fs/select.c:1070 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x455b29 Code: 1d ba fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb b9 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f5039c15c68 EFLAGS: 0246 ORIG_RAX: 010f RAX: ffda RBX: 7f5039c166d4 RCX: 00455b29 RDX: 2140 RSI: 0001 RDI: 20c0 RBP: 0072bea0 R08: 0008 R09: R10: 2180 R11: 0246 R12: R13: 004c05f3 R14: 004cfb20 R15: Modules linked in: Dumping ftrace buffer: (ftrace buffer empty) CR2: ---[ end trace 2f889d9e79d87fbd ]--- RIP: 0010: (null) Code: Bad RIP value. RSP: 0018:88019151f4c0 EFLAGS: 00010246 RAX: RBX: 8801d2dca800 RCX: 110eac7d RDX: 88019151f9e0 RSI: 8801c922da40 RDI: 8801c4a5a540 RBP: 88019151f630 R08: 0001 R09: R10: R11: R12: 1100322a3e9d R13: 88019151f9e0 R14: 8801d2dca812 R15: 8801d2dcac58 FS: 7f5039c16700() GS:8801dae0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: ffd6 CR3: 0001890f CR4: 001426f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
On 12:38-20180614, Tony Lindgren wrote:
> Some comments on the ranges below.
Thanks for reviewing in detail (I understand we are in the middle of
merge window, so thanks for the extra effort).
>
> * Nishanth Menon [180607 16:41]:
> > + soc0: soc0 {
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
>
> I suggest you leave out the soc0, that's not real. Just make
Why is that so, on a more complex board representation with multiple
SoCs, this is a clear node indicating what the main SoC is in the final
dtb representation.
> the cbass@0 the top level interconnect. It can then provide
> ranges to mcu interconnect which can provide ranges to the wkup
> interconnect. So just model it after what's in the hardware :)
That might blow up things quite a bit - it is like the comment in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/dra7.dtsi#n141
The trees are pretty deep with many interconnections (example main does
have direct connections to wkup as well, which is simplified off in
top level diagram) - basically it is not a direct one dimensional
relationship. But then, the same is the case for other SoCs..
we can represent NAVSS as a bus segment as well.
>
> I found the following ranges based on a quick look at the TRM,
> they could be split further if needed for power domains for
> genpd for example.
genpd is not really an issue, since it is handled in system firmware and
OSes dont have a visibility into the permitted ranges that the OS is
allowed to use.
I think it is just how accurate a representation is it worth.
>
> main covers
> 0x00 - 0x540200
>
> main provides at least the following ranges for mcu
> 0x002838 - 0x002bc0
> 0x004008 - 0x0041c8
> 0x004510 - 0x004518
> 0x004560 - 0x004564
> 0x004581 - 0x004586
> 0x004595 - 0x0045950400
> 0x0045a5 - 0x0045a50400
> 0x0045b04000 - 0x0045b06400
> 0x0045d1 - 0x0045d24000
> 0x004600 - 0x006000
> 0x04 - 0x08
> 0x4c3c02 - 0x4c3c03
> 0x4c3e00 - 0x4c3e04
> 0x54 - 0x540200
>
> then mcu provides the following ranges for wkup
> 0x004200 - 0x0044410020
> 0x004500 - 0x004503
> 0x004508 - 0x00450a
> 0x0045808000 - 0x0045808800
> 0x0045b0 - 0x0045b02400
>
> This based on looking at "figure 1-1. device top-level
> block diagram" and the memory map in TRM.
Thanks for researching. I did debate something like:
>From A53 view, a more accurate view might be - from an interconnect
view of the world (still simplified - i have ignored the sub bus
segments in the representations below):
msmc {
navss_main {
cbass_main{
cbass_mcu {
navss_mcu {
};
cbass_wkup{
};
};
};
};
};
>From R5 view, the view will be very different ofcourse:
view of the world (still simplified):
cbass_mcu {
navss_mcu {
};
cbass_wkup{
};
cbass_main{
navss_main {
msmc {
};
};
};
};
Do we really need this level of representation, I am not sure I had seen
this detailed a representation in other aarch64 SoCs (I am sure they are
as complex as TI SoCs as well).
I am trying to understand the direction and logic why we'd want to have
such a detailed representation.
A more flatter representation of just the main segments allow for dts
reuse between r5 and a53 as well (but that is minor).
Thoughts?
--
Regards,
Nishanth Menon
Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
On 12:38-20180614, Tony Lindgren wrote:
> Some comments on the ranges below.
Thanks for reviewing in detail (I understand we are in the middle of
merge window, so thanks for the extra effort).
>
> * Nishanth Menon [180607 16:41]:
> > + soc0: soc0 {
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
>
> I suggest you leave out the soc0, that's not real. Just make
Why is that so, on a more complex board representation with multiple
SoCs, this is a clear node indicating what the main SoC is in the final
dtb representation.
> the cbass@0 the top level interconnect. It can then provide
> ranges to mcu interconnect which can provide ranges to the wkup
> interconnect. So just model it after what's in the hardware :)
That might blow up things quite a bit - it is like the comment in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/dra7.dtsi#n141
The trees are pretty deep with many interconnections (example main does
have direct connections to wkup as well, which is simplified off in
top level diagram) - basically it is not a direct one dimensional
relationship. But then, the same is the case for other SoCs..
we can represent NAVSS as a bus segment as well.
>
> I found the following ranges based on a quick look at the TRM,
> they could be split further if needed for power domains for
> genpd for example.
genpd is not really an issue, since it is handled in system firmware and
OSes dont have a visibility into the permitted ranges that the OS is
allowed to use.
I think it is just how accurate a representation is it worth.
>
> main covers
> 0x00 - 0x540200
>
> main provides at least the following ranges for mcu
> 0x002838 - 0x002bc0
> 0x004008 - 0x0041c8
> 0x004510 - 0x004518
> 0x004560 - 0x004564
> 0x004581 - 0x004586
> 0x004595 - 0x0045950400
> 0x0045a5 - 0x0045a50400
> 0x0045b04000 - 0x0045b06400
> 0x0045d1 - 0x0045d24000
> 0x004600 - 0x006000
> 0x04 - 0x08
> 0x4c3c02 - 0x4c3c03
> 0x4c3e00 - 0x4c3e04
> 0x54 - 0x540200
>
> then mcu provides the following ranges for wkup
> 0x004200 - 0x0044410020
> 0x004500 - 0x004503
> 0x004508 - 0x00450a
> 0x0045808000 - 0x0045808800
> 0x0045b0 - 0x0045b02400
>
> This based on looking at "figure 1-1. device top-level
> block diagram" and the memory map in TRM.
Thanks for researching. I did debate something like:
>From A53 view, a more accurate view might be - from an interconnect
view of the world (still simplified - i have ignored the sub bus
segments in the representations below):
msmc {
navss_main {
cbass_main{
cbass_mcu {
navss_mcu {
};
cbass_wkup{
};
};
};
};
};
>From R5 view, the view will be very different ofcourse:
view of the world (still simplified):
cbass_mcu {
navss_mcu {
};
cbass_wkup{
};
cbass_main{
navss_main {
msmc {
};
};
};
};
Do we really need this level of representation, I am not sure I had seen
this detailed a representation in other aarch64 SoCs (I am sure they are
as complex as TI SoCs as well).
I am trying to understand the direction and logic why we'd want to have
such a detailed representation.
A more flatter representation of just the main segments allow for dts
reuse between r5 and a53 as well (but that is minor).
Thoughts?
--
Regards,
Nishanth Menon
Re: Restartable Sequences system call merged into Linux
- On Jun 14, 2018, at 8:27 AM, Pavel Machek pa...@ucw.cz wrote: > On Tue 2018-06-12 12:31:24, Mathieu Desnoyers wrote: >> - On Jun 12, 2018, at 9:11 AM, Florian Weimer fwei...@redhat.com wrote: >> >> > On 06/11/2018 10:04 PM, Mathieu Desnoyers wrote: >> >> - On Jun 11, 2018, at 3:55 PM, Florian Weimer fwei...@redhat.com >> >> wrote: >> >> >> >>> On 06/11/2018 09:49 PM, Mathieu Desnoyers wrote: >> It should be noted that there can be only one rseq TLS area registered >> per >> thread, >> which can then be used by many libraries and by the executable, so this >> is a >> process-wide (per-thread) resource that we need to manage carefully. >> >>> >> >>> Is it possible to resize the area after thread creation, perhaps even >> >>> from other threads? >> >> >> >> I'm not sure why we would want to resize it. The per-thread area is >> >> fixed-size. >> >> Its layout is here: include/uapi/linux/rseq.h: struct rseq >> > >> > Looks I was mistaken and this is very similar to the robust mutex list. >> > >> > Should we treat it the same way? Always allocate it for each new thread >> > and register it with the kernel? >> >> That would be an efficient way to do it, indeed. There is very little >> performance overhead to have rseq registered for all threads, whether or >> not they intend to run rseq critical sections. > > People with slow / low memory machines would prefer not to see > overhead they don't need... In terms of memory usage, if people don't want the extra few bytes of memory used by rseq in the kernel, they should use CONFIG_RSEQ=n. In terms of overhead, let's have a closer look at what it means: when a thread is registered to rseq, but does not enter rseq critical sections, only this extra work is done by the kernel: - rseq_preempt(): on preemption, the scheduler sets the TIF_NOTIFY_RESUME thread flag, so rseq_handle_notify_resume() can check whether it's in a rseq critical section when returning to user-space, - rseq_signal_deliver(): on signal delivery, rseq_handle_notify_resume() checks whether it's in a rseq critical section, - rseq_migrate: on migration, the scheduler sets TIF_NOTIFY_RESUME as well, > >> I have a few possible approaches in mind (feel free to suggest other >> options): >> >> A) glibc exposes a strong __rseq_abi TLS symbol: >> >>- should ideally *not* be global-dynamic for performance reasons, >>- registration to kernel can either be handled explicitly by requiring >> application or libraries to call an API, or implicitly at thread >> creation, > > ...so I'd prefer explicit API call. I have use-cases where a library wants to link against librseq and have rseq critical sections, without requiring the application to explicitly add rseq registration calls on thread creation/destruction. Is there a way to register callbacks to glibc which could be invoked on thread creation/destruction ? Then if we include dynamic loading of libraries (dlopen/dlclose) in the picture, this gets even worse, as we'd need to be able to iterate on all existing threads to invoke registration/unregistration callbacks. One alternative approach would be to let the user library lazily register rseq when needed, and use a pthread_key for unregistration. However, this does not allow dlclose of the user library without figuring a way to iterate on all threads. Another alternative would be to somehow let glibc handle the registration, perhaps only doing it for applications expressing their interest for rseq. Thoughts ? Thanks, Mathieu > >> B) librseq.so exposes a strong __rseq_abi symbol: > > Works for me. > Pavel > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: Restartable Sequences system call merged into Linux
- On Jun 14, 2018, at 8:27 AM, Pavel Machek pa...@ucw.cz wrote: > On Tue 2018-06-12 12:31:24, Mathieu Desnoyers wrote: >> - On Jun 12, 2018, at 9:11 AM, Florian Weimer fwei...@redhat.com wrote: >> >> > On 06/11/2018 10:04 PM, Mathieu Desnoyers wrote: >> >> - On Jun 11, 2018, at 3:55 PM, Florian Weimer fwei...@redhat.com >> >> wrote: >> >> >> >>> On 06/11/2018 09:49 PM, Mathieu Desnoyers wrote: >> It should be noted that there can be only one rseq TLS area registered >> per >> thread, >> which can then be used by many libraries and by the executable, so this >> is a >> process-wide (per-thread) resource that we need to manage carefully. >> >>> >> >>> Is it possible to resize the area after thread creation, perhaps even >> >>> from other threads? >> >> >> >> I'm not sure why we would want to resize it. The per-thread area is >> >> fixed-size. >> >> Its layout is here: include/uapi/linux/rseq.h: struct rseq >> > >> > Looks I was mistaken and this is very similar to the robust mutex list. >> > >> > Should we treat it the same way? Always allocate it for each new thread >> > and register it with the kernel? >> >> That would be an efficient way to do it, indeed. There is very little >> performance overhead to have rseq registered for all threads, whether or >> not they intend to run rseq critical sections. > > People with slow / low memory machines would prefer not to see > overhead they don't need... In terms of memory usage, if people don't want the extra few bytes of memory used by rseq in the kernel, they should use CONFIG_RSEQ=n. In terms of overhead, let's have a closer look at what it means: when a thread is registered to rseq, but does not enter rseq critical sections, only this extra work is done by the kernel: - rseq_preempt(): on preemption, the scheduler sets the TIF_NOTIFY_RESUME thread flag, so rseq_handle_notify_resume() can check whether it's in a rseq critical section when returning to user-space, - rseq_signal_deliver(): on signal delivery, rseq_handle_notify_resume() checks whether it's in a rseq critical section, - rseq_migrate: on migration, the scheduler sets TIF_NOTIFY_RESUME as well, > >> I have a few possible approaches in mind (feel free to suggest other >> options): >> >> A) glibc exposes a strong __rseq_abi TLS symbol: >> >>- should ideally *not* be global-dynamic for performance reasons, >>- registration to kernel can either be handled explicitly by requiring >> application or libraries to call an API, or implicitly at thread >> creation, > > ...so I'd prefer explicit API call. I have use-cases where a library wants to link against librseq and have rseq critical sections, without requiring the application to explicitly add rseq registration calls on thread creation/destruction. Is there a way to register callbacks to glibc which could be invoked on thread creation/destruction ? Then if we include dynamic loading of libraries (dlopen/dlclose) in the picture, this gets even worse, as we'd need to be able to iterate on all existing threads to invoke registration/unregistration callbacks. One alternative approach would be to let the user library lazily register rseq when needed, and use a pthread_key for unregistration. However, this does not allow dlclose of the user library without figuring a way to iterate on all threads. Another alternative would be to somehow let glibc handle the registration, perhaps only doing it for applications expressing their interest for rseq. Thoughts ? Thanks, Mathieu > >> B) librseq.so exposes a strong __rseq_abi symbol: > > Works for me. > Pavel > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH 1/4] arm: dts: add support for Laird WB45N cpu module and DVK
On 14/06/2018 14:52:25+0200, Nicolas Ferre wrote:
> > + gpio_keys {
> > + compatible = "gpio-keys";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + irqbtn@pb18 {
>
> I'm not sure that the @pb18 can be used like this. This address extension
> must be used in a "reg" property in the node. dtc used with warning switch
> on might trigger an error for this.
>
Indeed, no unit-address without a reg property.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/4] arm: dts: add support for Laird WB45N cpu module and DVK
On 14/06/2018 14:52:25+0200, Nicolas Ferre wrote:
> > + gpio_keys {
> > + compatible = "gpio-keys";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + irqbtn@pb18 {
>
> I'm not sure that the @pb18 can be used like this. This address extension
> must be used in a "reg" property in the node. dtc used with warning switch
> on might trigger an error for this.
>
Indeed, no unit-address without a reg property.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/1] arm64: dts: rockchip: correct voltage selector Firefly-RK3399
Am Montag, 4. Juni 2018, 19:15:23 CEST schrieb Heinrich Schuchardt: > Without this patch the Firefly-RK3399 board boot process hangs after these > lines: > >fan53555-regulator 0-0040: FAN53555 Option[8] Rev[1] Detected! >fan53555-reg: supplied by vcc_sys >vcc1v8_s3: supplied by vcc_1v8 > > Blacklisting driver fan53555 allows booting. > > The device tree uses a value of fcs,suspend-voltage-selector different to > any other board. > > Changing this setting to the usual value is sufficient to enable booting. > > Signed-off-by: Heinrich Schuchardt applied for 4.19. I've amended your commit message with the info from your reply about the vendor kernel using the same value and added an appropriate Fixes tag to possibly get it merged into stable. Thanks Heiko
Re: [PATCH 1/1] arm64: dts: rockchip: correct voltage selector Firefly-RK3399
Am Montag, 4. Juni 2018, 19:15:23 CEST schrieb Heinrich Schuchardt: > Without this patch the Firefly-RK3399 board boot process hangs after these > lines: > >fan53555-regulator 0-0040: FAN53555 Option[8] Rev[1] Detected! >fan53555-reg: supplied by vcc_sys >vcc1v8_s3: supplied by vcc_1v8 > > Blacklisting driver fan53555 allows booting. > > The device tree uses a value of fcs,suspend-voltage-selector different to > any other board. > > Changing this setting to the usual value is sufficient to enable booting. > > Signed-off-by: Heinrich Schuchardt applied for 4.19. I've amended your commit message with the info from your reply about the vendor kernel using the same value and added an appropriate Fixes tag to possibly get it merged into stable. Thanks Heiko
Re: [PATCH 1/4] arm: dts: add support for Laird WB45N cpu module and DVK
On 14/06/2018 at 10:51, Ben Whitten wrote:
Signed-off-by: Ben Whitten
---
arch/arm/boot/dts/Makefile| 3 +-
arch/arm/boot/dts/at91-wb45n.dts | 66 +++
arch/arm/boot/dts/at91-wb45n.dtsi | 169 ++
3 files changed, 237 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/at91-wb45n.dts
create mode 100644 arch/arm/boot/dts/at91-wb45n.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7e24249..1ee94ee 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -42,7 +42,8 @@ dtb-$(CONFIG_SOC_AT91SAM9) += \
at91sam9g25ek.dtb \
at91sam9g35ek.dtb \
at91sam9x25ek.dtb \
- at91sam9x35ek.dtb
+ at91sam9x35ek.dtb \
+ at91-wb45n.dtb
dtb-$(CONFIG_SOC_SAM_V7) += \
at91-kizbox2.dtb \
at91-nattis-2-natte-2.dtb \
diff --git a/arch/arm/boot/dts/at91-wb45n.dts b/arch/arm/boot/dts/at91-wb45n.dts
new file mode 100644
index 000..4e88815
--- /dev/null
+++ b/arch/arm/boot/dts/at91-wb45n.dts
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-wb45n.dts - Device Tree file for WB45NBT board
+ *
+ * Copyright (C) 2018 Laird
+ *
+*/
+/dts-v1/;
+#include "at91-wb45n.dtsi"
+
+/ {
+ model = "Laird Workgroup Bridge 45N - Atmel AT91SAM (dt)";
+ compatible = "laird,wb45n", "laird,wbxx", "atmel,at91sam9x5",
"atmel,at91sam9";
"laird" prefix must be added to
Documentation/devicetree/bindings/vendor-prefixes.txt before using it:
you can do a little patch as a first patch of this series.
Otherwise it will trigger a warning message while running
scripts/checkpatch.pl on top of your patch.
+
+ ahb {
+ apb {
+ watchdog@fe40 {
+ status = "okay";
+ };
+ };
+ };
+
+ gpio_keys {
+ compatible = "gpio-keys";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ irqbtn@pb18 {
I'm not sure that the @pb18 can be used like this. This address
extension must be used in a "reg" property in the node. dtc used with
warning switch on might trigger an error for this.
+ label = "IRQBTN";
+ linux,code = <99>;
+ gpios = < 18 GPIO_ACTIVE_LOW>;
+ gpio-key,wakeup = <1>;
+ };
+ };
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/at91-wb45n.dtsi
b/arch/arm/boot/dts/at91-wb45n.dtsi
new file mode 100644
index 000..2fa58e2
--- /dev/null
+++ b/arch/arm/boot/dts/at91-wb45n.dtsi
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-wb45n.dtsi - Device Tree file for WB45NBT board
+ *
+ * Copyright (C) 2018 Laird
+ *
+ */
+
+#include "at91sam9g25.dtsi"
+
+/ {
+ model = "Laird Workgroup Bridge 45N - Atmel AT91SAM (dt)";
+ compatible = "laird,wb45n", "laird,wbxx", "atmel,at91sam9x5",
"atmel,at91sam9";
+
+ chosen {
+ bootargs = "ubi.mtd=6 root=ubi0:rootfs rootfstype=ubifs rw";
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ reg = <0x2000 0x400>;
+ };
+
+ ahb {
+ apb {
+ shdwc@fe10 {
I would advice you to take exactly the node name:
"shutdown-controller@fe10"; Anyway, it will go away after you use
the label notation as advised by Alexandre.
+ atmel,wakeup-mode = "low";
+ };
+
+ pinctrl@f400 {
+ usb2 {
+ pinctrl_board_usb2: usb2-board {
+ atmel,pins =
+ ;/* PB11 gpio vbus sense,
deglitch */
+ };
+ };
+ };
+
+ rstc@fe00 {
+ compatible = "atmel,sama5d3-rstc";
+ };
I don't think this node is needed.
+
+ };
+ };
+
+ atheros {
+ compatible = "atheros,ath6kl";
+ atheros,board-id = "SD32";
+ };
+};
+
+_xtal {
+ clock-frequency = <32768>;
+};
+
+_xtal {
+ clock-frequency = <1200>;
+};
+
+ {
+ status = "okay";
+ nand_controller: nand-controller {
+ pinctrl-0 = <_nand_cs _nand_rb
_nand_oe_we>;
+ pinctrl-names = "default";
+ status = "okay";
+
+ nand@3 {
+ reg = <0x3 0x0
Re: [PATCH 1/4] arm: dts: add support for Laird WB45N cpu module and DVK
On 14/06/2018 at 10:51, Ben Whitten wrote:
Signed-off-by: Ben Whitten
---
arch/arm/boot/dts/Makefile| 3 +-
arch/arm/boot/dts/at91-wb45n.dts | 66 +++
arch/arm/boot/dts/at91-wb45n.dtsi | 169 ++
3 files changed, 237 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/at91-wb45n.dts
create mode 100644 arch/arm/boot/dts/at91-wb45n.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7e24249..1ee94ee 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -42,7 +42,8 @@ dtb-$(CONFIG_SOC_AT91SAM9) += \
at91sam9g25ek.dtb \
at91sam9g35ek.dtb \
at91sam9x25ek.dtb \
- at91sam9x35ek.dtb
+ at91sam9x35ek.dtb \
+ at91-wb45n.dtb
dtb-$(CONFIG_SOC_SAM_V7) += \
at91-kizbox2.dtb \
at91-nattis-2-natte-2.dtb \
diff --git a/arch/arm/boot/dts/at91-wb45n.dts b/arch/arm/boot/dts/at91-wb45n.dts
new file mode 100644
index 000..4e88815
--- /dev/null
+++ b/arch/arm/boot/dts/at91-wb45n.dts
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-wb45n.dts - Device Tree file for WB45NBT board
+ *
+ * Copyright (C) 2018 Laird
+ *
+*/
+/dts-v1/;
+#include "at91-wb45n.dtsi"
+
+/ {
+ model = "Laird Workgroup Bridge 45N - Atmel AT91SAM (dt)";
+ compatible = "laird,wb45n", "laird,wbxx", "atmel,at91sam9x5",
"atmel,at91sam9";
"laird" prefix must be added to
Documentation/devicetree/bindings/vendor-prefixes.txt before using it:
you can do a little patch as a first patch of this series.
Otherwise it will trigger a warning message while running
scripts/checkpatch.pl on top of your patch.
+
+ ahb {
+ apb {
+ watchdog@fe40 {
+ status = "okay";
+ };
+ };
+ };
+
+ gpio_keys {
+ compatible = "gpio-keys";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ irqbtn@pb18 {
I'm not sure that the @pb18 can be used like this. This address
extension must be used in a "reg" property in the node. dtc used with
warning switch on might trigger an error for this.
+ label = "IRQBTN";
+ linux,code = <99>;
+ gpios = < 18 GPIO_ACTIVE_LOW>;
+ gpio-key,wakeup = <1>;
+ };
+ };
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/at91-wb45n.dtsi
b/arch/arm/boot/dts/at91-wb45n.dtsi
new file mode 100644
index 000..2fa58e2
--- /dev/null
+++ b/arch/arm/boot/dts/at91-wb45n.dtsi
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-wb45n.dtsi - Device Tree file for WB45NBT board
+ *
+ * Copyright (C) 2018 Laird
+ *
+ */
+
+#include "at91sam9g25.dtsi"
+
+/ {
+ model = "Laird Workgroup Bridge 45N - Atmel AT91SAM (dt)";
+ compatible = "laird,wb45n", "laird,wbxx", "atmel,at91sam9x5",
"atmel,at91sam9";
+
+ chosen {
+ bootargs = "ubi.mtd=6 root=ubi0:rootfs rootfstype=ubifs rw";
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ reg = <0x2000 0x400>;
+ };
+
+ ahb {
+ apb {
+ shdwc@fe10 {
I would advice you to take exactly the node name:
"shutdown-controller@fe10"; Anyway, it will go away after you use
the label notation as advised by Alexandre.
+ atmel,wakeup-mode = "low";
+ };
+
+ pinctrl@f400 {
+ usb2 {
+ pinctrl_board_usb2: usb2-board {
+ atmel,pins =
+ ;/* PB11 gpio vbus sense,
deglitch */
+ };
+ };
+ };
+
+ rstc@fe00 {
+ compatible = "atmel,sama5d3-rstc";
+ };
I don't think this node is needed.
+
+ };
+ };
+
+ atheros {
+ compatible = "atheros,ath6kl";
+ atheros,board-id = "SD32";
+ };
+};
+
+_xtal {
+ clock-frequency = <32768>;
+};
+
+_xtal {
+ clock-frequency = <1200>;
+};
+
+ {
+ status = "okay";
+ nand_controller: nand-controller {
+ pinctrl-0 = <_nand_cs _nand_rb
_nand_oe_we>;
+ pinctrl-names = "default";
+ status = "okay";
+
+ nand@3 {
+ reg = <0x3 0x0
Re: [PATCH 4.4 119/268] xen/pirq: fix error path cleanup when binding MSIs
On 06/14/2018 04:21 AM, Roger Pau Monné wrote: > On Wed, Jun 13, 2018 at 07:48:50PM +0100, Ben Hutchings wrote: >> On Wed, 2018-02-28 at 09:19 +, Roger Pau Monne wrote: >>> From: Roger Pau Monne >>> >>> [ Upstream commit 910f8befdf5bccf25287d9f1743e3e546bcb7ce0 ] >>> >>> Current cleanup in the error path of xen_bind_pirq_msi_to_irq is >>> wrong. First of all there's an off-by-one in the cleanup loop, which >>> can lead to unbinding wrong IRQs. >>> >>> Secondly IRQs not bound won't be freed, thus leaking IRQ numbers. >>> >>> Note that there's no need to differentiate between bound and unbound >>> IRQs when freeing them, __unbind_from_irq will deal with both of them >>> correctly. >> It appears to me that it is safe to call __unbind_from_irq() after >> xen_irq_info_common_setup() fails, but *not* if the latter hasn't been >> called at all. In that case the IRQ type will still be set to >> IRQT_UNBOUND and this will trigger the BUG_ON() in __unbind_from_irq(). >> >> [...] >>> --- a/drivers/xen/events/events_base.c >>> +++ b/drivers/xen/events/events_base.c >>> @@ -764,8 +764,8 @@ out: >>> mutex_unlock(_mapping_update_lock); >>> return irq; >>> error_irq: >>> - for (; i >= 0; i--) >>> - __unbind_from_irq(irq + i); >>> + while (nvec--) >>> + __unbind_from_irq(irq + nvec); >> If nvec > 1, and xen_irq_info_pirq_setup() fails for i != nvec - 1, >> then we reach here without having called xen_irq_info_common_setup() >> for all these IRQs. >> >> In that case, I think we will still want to call xen_free_irq() for all >> IRQs. So maybe the fix would be to remove the BUG_ON() in >> __unbind_from_irq()? > I think your analysis is right, and I agree that removing the BUG_ON > from __unbind_from_irq seems like the right solution. > > I can't see any issues from calling xen_free_irq with type == > IRQT_UNBOUND, but I've already attempted to fix this once and failed, > so I would like to get second opinions. Also I'm not sure of the > reason behind that BUG_ON. I don't see a reason for the BUG_ON either. -boris
Re: [PATCH 4.4 119/268] xen/pirq: fix error path cleanup when binding MSIs
On 06/14/2018 04:21 AM, Roger Pau Monné wrote: > On Wed, Jun 13, 2018 at 07:48:50PM +0100, Ben Hutchings wrote: >> On Wed, 2018-02-28 at 09:19 +, Roger Pau Monne wrote: >>> From: Roger Pau Monne >>> >>> [ Upstream commit 910f8befdf5bccf25287d9f1743e3e546bcb7ce0 ] >>> >>> Current cleanup in the error path of xen_bind_pirq_msi_to_irq is >>> wrong. First of all there's an off-by-one in the cleanup loop, which >>> can lead to unbinding wrong IRQs. >>> >>> Secondly IRQs not bound won't be freed, thus leaking IRQ numbers. >>> >>> Note that there's no need to differentiate between bound and unbound >>> IRQs when freeing them, __unbind_from_irq will deal with both of them >>> correctly. >> It appears to me that it is safe to call __unbind_from_irq() after >> xen_irq_info_common_setup() fails, but *not* if the latter hasn't been >> called at all. In that case the IRQ type will still be set to >> IRQT_UNBOUND and this will trigger the BUG_ON() in __unbind_from_irq(). >> >> [...] >>> --- a/drivers/xen/events/events_base.c >>> +++ b/drivers/xen/events/events_base.c >>> @@ -764,8 +764,8 @@ out: >>> mutex_unlock(_mapping_update_lock); >>> return irq; >>> error_irq: >>> - for (; i >= 0; i--) >>> - __unbind_from_irq(irq + i); >>> + while (nvec--) >>> + __unbind_from_irq(irq + nvec); >> If nvec > 1, and xen_irq_info_pirq_setup() fails for i != nvec - 1, >> then we reach here without having called xen_irq_info_common_setup() >> for all these IRQs. >> >> In that case, I think we will still want to call xen_free_irq() for all >> IRQs. So maybe the fix would be to remove the BUG_ON() in >> __unbind_from_irq()? > I think your analysis is right, and I agree that removing the BUG_ON > from __unbind_from_irq seems like the right solution. > > I can't see any issues from calling xen_free_irq with type == > IRQT_UNBOUND, but I've already attempted to fix this once and failed, > so I would like to get second opinions. Also I'm not sure of the > reason behind that BUG_ON. I don't see a reason for the BUG_ON either. -boris
WARNING in sk_stream_kill_queues (3)
Hello, syzbot found the following crash on: HEAD commit:81c310582f0e kmsan: unpoison virtio input buffers when add.. git tree: https://github.com/google/kmsan.git/master console output: https://syzkaller.appspot.com/x/log.txt?x=1747c21f80 kernel config: https://syzkaller.appspot.com/x/.config?x=848e40757852af3e dashboard link: https://syzkaller.appspot.com/bug?extid=13e1ee9caeab5a9abc62 compiler: clang version 7.0.0 (trunk 334104) syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=105f5eaf80 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13b15b6f80 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+13e1ee9caeab5a9ab...@syzkaller.appspotmail.com WARNING: CPU: 0 PID: 4964 at net/core/stream.c:206 sk_stream_kill_queues+0x944/0x970 net/core/stream.c:206 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 4964 Comm: syz-executor457 Not tainted 4.17.0+ #6 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x185/0x1d0 lib/dump_stack.c:113 panic+0x3d0/0x990 kernel/panic.c:184 __warn+0x40f/0x580 kernel/panic.c:536 report_bug+0x72a/0x880 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] do_error_trap+0x1c1/0x620 arch/x86/kernel/traps.c:298 do_invalid_op+0x46/0x50 arch/x86/kernel/traps.c:317 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992 RIP: 0010:sk_stream_kill_queues+0x944/0x970 net/core/stream.c:206 RSP: 0018:8801a867f368 EFLAGS: 00010293 RAX: 87dbf654 RBX: 0813 RCX: 8801ab7bd7c0 RDX: RSI: b000 RDI: ea00 RBP: 8801a867f3e8 R08: R09: 0002 R10: 8801a66d3a00 R11: 88c44c40 R12: R13: R14: R15: 0813 inet_csk_destroy_sock+0x2a4/0x5d0 net/ipv4/inet_connection_sock.c:833 tcp_close+0xe37/0x18f0 net/ipv4/tcp.c:2323 tls_sk_proto_close+0xc2f/0xcd0 net/tls/tls_main.c:291 inet_release+0x249/0x2b0 net/ipv4/af_inet.c:427 inet6_release+0xaf/0x100 net/ipv6/af_inet6.c:460 sock_release net/socket.c:594 [inline] sock_close+0xeb/0x310 net/socket.c:1149 __fput+0x458/0xa30 fs/file_table.c:209 fput+0x37/0x40 fs/file_table.c:243 task_work_run+0x22e/0x2b0 kernel/task_work.c:113 exit_task_work include/linux/task_work.h:22 [inline] do_exit+0x110e/0x3930 kernel/exit.c:867 do_group_exit+0x1a0/0x360 kernel/exit.c:970 get_signal+0x1405/0x1ec0 kernel/signal.c:2482 do_signal+0xb8/0x1d20 arch/x86/kernel/signal.c:810 exit_to_usermode_loop arch/x86/entry/common.c:162 [inline] prepare_exit_to_usermode+0x271/0x3a0 arch/x86/entry/common.c:196 syscall_return_slowpath+0xe9/0x710 arch/x86/entry/common.c:265 do_syscall_64+0x1ad/0x230 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x447ce9 RSP: 002b:7feb54132d98 EFLAGS: 0212 ORIG_RAX: 002c RAX: 8000 RBX: 006dec5c RCX: 00447ce9 RDX: fdef RSI: 25c0 RDI: 0007 RBP: R08: 2000 R09: 001c R10: R11: 0212 R12: 006dec58 R13: 0100 R14: 7feb541339c0 R15: 000c Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
WARNING in sk_stream_kill_queues (3)
Hello, syzbot found the following crash on: HEAD commit:81c310582f0e kmsan: unpoison virtio input buffers when add.. git tree: https://github.com/google/kmsan.git/master console output: https://syzkaller.appspot.com/x/log.txt?x=1747c21f80 kernel config: https://syzkaller.appspot.com/x/.config?x=848e40757852af3e dashboard link: https://syzkaller.appspot.com/bug?extid=13e1ee9caeab5a9abc62 compiler: clang version 7.0.0 (trunk 334104) syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=105f5eaf80 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13b15b6f80 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+13e1ee9caeab5a9ab...@syzkaller.appspotmail.com WARNING: CPU: 0 PID: 4964 at net/core/stream.c:206 sk_stream_kill_queues+0x944/0x970 net/core/stream.c:206 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 4964 Comm: syz-executor457 Not tainted 4.17.0+ #6 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x185/0x1d0 lib/dump_stack.c:113 panic+0x3d0/0x990 kernel/panic.c:184 __warn+0x40f/0x580 kernel/panic.c:536 report_bug+0x72a/0x880 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] do_error_trap+0x1c1/0x620 arch/x86/kernel/traps.c:298 do_invalid_op+0x46/0x50 arch/x86/kernel/traps.c:317 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992 RIP: 0010:sk_stream_kill_queues+0x944/0x970 net/core/stream.c:206 RSP: 0018:8801a867f368 EFLAGS: 00010293 RAX: 87dbf654 RBX: 0813 RCX: 8801ab7bd7c0 RDX: RSI: b000 RDI: ea00 RBP: 8801a867f3e8 R08: R09: 0002 R10: 8801a66d3a00 R11: 88c44c40 R12: R13: R14: R15: 0813 inet_csk_destroy_sock+0x2a4/0x5d0 net/ipv4/inet_connection_sock.c:833 tcp_close+0xe37/0x18f0 net/ipv4/tcp.c:2323 tls_sk_proto_close+0xc2f/0xcd0 net/tls/tls_main.c:291 inet_release+0x249/0x2b0 net/ipv4/af_inet.c:427 inet6_release+0xaf/0x100 net/ipv6/af_inet6.c:460 sock_release net/socket.c:594 [inline] sock_close+0xeb/0x310 net/socket.c:1149 __fput+0x458/0xa30 fs/file_table.c:209 fput+0x37/0x40 fs/file_table.c:243 task_work_run+0x22e/0x2b0 kernel/task_work.c:113 exit_task_work include/linux/task_work.h:22 [inline] do_exit+0x110e/0x3930 kernel/exit.c:867 do_group_exit+0x1a0/0x360 kernel/exit.c:970 get_signal+0x1405/0x1ec0 kernel/signal.c:2482 do_signal+0xb8/0x1d20 arch/x86/kernel/signal.c:810 exit_to_usermode_loop arch/x86/entry/common.c:162 [inline] prepare_exit_to_usermode+0x271/0x3a0 arch/x86/entry/common.c:196 syscall_return_slowpath+0xe9/0x710 arch/x86/entry/common.c:265 do_syscall_64+0x1ad/0x230 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x447ce9 RSP: 002b:7feb54132d98 EFLAGS: 0212 ORIG_RAX: 002c RAX: 8000 RBX: 006dec5c RCX: 00447ce9 RDX: fdef RSI: 25c0 RDI: 0007 RBP: R08: 2000 R09: 001c R10: R11: 0212 R12: 006dec58 R13: 0100 R14: 7feb541339c0 R15: 000c Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
WARNING in input_alloc_absinfo
Hello, syzbot found the following crash on: HEAD commit:d2d741e5d189 kmsan: add initialization for shmem pages git tree: https://github.com/google/kmsan.git/master console output: https://syzkaller.appspot.com/x/log.txt?x=1775bae780 kernel config: https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f dashboard link: https://syzkaller.appspot.com/bug?extid=c382812c78d98ecd9fb8 compiler: clang version 7.0.0 (trunk 329391) syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13b31ae780 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1733255b80 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+c382812c78d98ecd9...@syzkaller.appspotmail.com RBP: 006cb018 R08: 0001 R09: 7ffe93080031 R10: R11: 0246 R12: 0004 R13: R14: R15: [ cut here ] input_alloc_absinfo(): kcalloc() failed? WARNING: CPU: 1 PID: 4498 at drivers/input/input.c:487 input_alloc_absinfo+0x183/0x190 drivers/input/input.c:487 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 4498 Comm: syz-executor465 Not tainted 4.16.0+ #87 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x185/0x1d0 lib/dump_stack.c:53 panic+0x39d/0x940 kernel/panic.c:183 __warn+0x40f/0x580 kernel/panic.c:547 report_bug+0x72a/0x880 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] do_error_trap+0x1aa/0x600 arch/x86/kernel/traps.c:297 do_invalid_op+0x46/0x50 arch/x86/kernel/traps.c:316 invalid_op+0x1b/0x40 arch/x86/entry/entry_64.S:986 RIP: 0010:input_alloc_absinfo+0x183/0x190 drivers/input/input.c:487 RSP: 0018:88019651faa8 EFLAGS: 00010282 RAX: 0028 RBX: RCX: RDX: RSI: b000 RDI: ea00 RBP: 88019651fae0 R08: 01080020 R09: 0002 R10: R11: R12: R13: 8801a19ec140 R14: 88019796e198 R15: uinput_abs_setup drivers/input/misc/uinput.c:507 [inline] uinput_ioctl_handler+0x38a2/0x39f0 drivers/input/misc/uinput.c:1035 uinput_ioctl+0x9a/0xb0 drivers/input/misc/uinput.c:1047 vfs_ioctl fs/ioctl.c:46 [inline] do_vfs_ioctl+0xaf0/0x2440 fs/ioctl.c:686 SYSC_ioctl+0x1d2/0x260 fs/ioctl.c:701 SyS_ioctl+0x54/0x80 fs/ioctl.c:692 do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x3d/0xa2 RIP: 0033:0x440429 RSP: 002b:7ffe9308d2b8 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: RCX: 00440429 RDX: RSI: 40005504 RDI: 0003 RBP: 006cb018 R08: 0001 R09: 7ffe93080031 R10: R11: 0246 R12: 0004 R13: R14: R15: Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
WARNING in input_alloc_absinfo
Hello, syzbot found the following crash on: HEAD commit:d2d741e5d189 kmsan: add initialization for shmem pages git tree: https://github.com/google/kmsan.git/master console output: https://syzkaller.appspot.com/x/log.txt?x=1775bae780 kernel config: https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f dashboard link: https://syzkaller.appspot.com/bug?extid=c382812c78d98ecd9fb8 compiler: clang version 7.0.0 (trunk 329391) syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13b31ae780 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1733255b80 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+c382812c78d98ecd9...@syzkaller.appspotmail.com RBP: 006cb018 R08: 0001 R09: 7ffe93080031 R10: R11: 0246 R12: 0004 R13: R14: R15: [ cut here ] input_alloc_absinfo(): kcalloc() failed? WARNING: CPU: 1 PID: 4498 at drivers/input/input.c:487 input_alloc_absinfo+0x183/0x190 drivers/input/input.c:487 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 4498 Comm: syz-executor465 Not tainted 4.16.0+ #87 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x185/0x1d0 lib/dump_stack.c:53 panic+0x39d/0x940 kernel/panic.c:183 __warn+0x40f/0x580 kernel/panic.c:547 report_bug+0x72a/0x880 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] do_error_trap+0x1aa/0x600 arch/x86/kernel/traps.c:297 do_invalid_op+0x46/0x50 arch/x86/kernel/traps.c:316 invalid_op+0x1b/0x40 arch/x86/entry/entry_64.S:986 RIP: 0010:input_alloc_absinfo+0x183/0x190 drivers/input/input.c:487 RSP: 0018:88019651faa8 EFLAGS: 00010282 RAX: 0028 RBX: RCX: RDX: RSI: b000 RDI: ea00 RBP: 88019651fae0 R08: 01080020 R09: 0002 R10: R11: R12: R13: 8801a19ec140 R14: 88019796e198 R15: uinput_abs_setup drivers/input/misc/uinput.c:507 [inline] uinput_ioctl_handler+0x38a2/0x39f0 drivers/input/misc/uinput.c:1035 uinput_ioctl+0x9a/0xb0 drivers/input/misc/uinput.c:1047 vfs_ioctl fs/ioctl.c:46 [inline] do_vfs_ioctl+0xaf0/0x2440 fs/ioctl.c:686 SYSC_ioctl+0x1d2/0x260 fs/ioctl.c:701 SyS_ioctl+0x54/0x80 fs/ioctl.c:692 do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287 entry_SYSCALL_64_after_hwframe+0x3d/0xa2 RIP: 0033:0x440429 RSP: 002b:7ffe9308d2b8 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: RCX: 00440429 RDX: RSI: 40005504 RDI: 0003 RBP: 006cb018 R08: 0001 R09: 7ffe93080031 R10: R11: 0246 R12: 0004 R13: R14: R15: Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
[PATCH] Use 'imply' with SEV Kconfig CRYPTO dependencies
On 14/06/18 14:08, Brijesh Singh wrote:
>
>
> On 6/14/18 2:58 AM, Richard Weinberger wrote:
>> On Wed, May 23, 2018 at 4:46 PM, Borislav Petkov wrote:
>>> + Tom and Brijesh.
>>>
>>> On Mon, May 21, 2018 at 10:12:53AM -0500, Janakarajan Natarajan wrote:
Use Kconfig imply 'option' when specifying SEV CRYPTO dependencies.
Example configuration:
.
.
CONFIG_CRYPTO_DEV_CCP=y
CONFIG_CRYPTO_DEV_CCP_DD=m
CONFIG_CRYPTO_DEV_SP_CCP=y
CONFIG_CRYPTO_DEV_CCP_CRYPTO=m
CONFIG_CRYPTO_DEV_SP_PSP=y
.
.
CONFIG_KVM_AMD=y
CONFIG_KVM_AMD_SEV=y
.
.
When the CRYPTO_DEV_SP_DD is m, KVM_AMD set to y produces compile time
errors.
Since KVM_AMD_SEV depends on KVM_AMD and CRYPTO_DEV_CCP_DD, the
issue can be prevented by using 'imply' Kconfig option when specifying
the CRYPTO dependencies.
Fixes: 505c9e94d832 ("KVM: x86: prefer "depends on" to "select" for SEV")
Signed-off-by: Janakarajan Natarajan
---
arch/x86/kvm/Kconfig | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 92fd433..d9b16b7 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -85,7 +85,9 @@ config KVM_AMD_SEV
def_bool y
bool "AMD Secure Encrypted Virtualization (SEV) support"
depends on KVM_AMD && X86_64
- depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
+ imply CRYPTO_DEV_CCP
+ imply CRYPTO_DEV_CCP_DD
+ imply CRYPTO_DEV_SP_PSP
---help---
Provides support for launching Encrypted VMs on AMD processors.
>>> Well, this solves the build issue but I just created a config:
>>>
>>> $ grep -E "(KVM|PSP)" .config | grep -v '#'
>>> CONFIG_HAVE_KVM=y
>>> CONFIG_HAVE_KVM_IRQCHIP=y
>>> CONFIG_HAVE_KVM_IRQFD=y
>>> CONFIG_HAVE_KVM_IRQ_ROUTING=y
>>> CONFIG_HAVE_KVM_EVENTFD=y
>>> CONFIG_KVM_MMIO=y
>>> CONFIG_KVM_ASYNC_PF=y
>>> CONFIG_HAVE_KVM_MSI=y
>>> CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT=y
>>> CONFIG_KVM_VFIO=y
>>> CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT=y
>>> CONFIG_KVM_COMPAT=y
>>> CONFIG_HAVE_KVM_IRQ_BYPASS=y
>>> CONFIG_KVM=y
>>> CONFIG_KVM_AMD=m
>>>
>>> which builds but the PSP functionality is not there. And I don't think
>>> this is serving the user: she should be able to select what she wants
>>> and get the required functionality added and not have build errors with
>>> any possible configuration.
>
> Yes, I agree.
>
>>>
>>> Now, looking at the security processor Kconfig stuff, it is somewhat
>>> confusing but maybe I didn't look at it long enough. A couple of points:
>>>
>>> config CRYPTO_DEV_CCP_DD
>>> tristate "Secure Processor device driver"
>>>
>>> If this is going to be the top-level menu item for the SP, call that
>>>
>>> CRYPTO_DEV_SP
>>>
>>> to mean, this is the security processor. CCP_DD is confusing because you
>>> have CRYPTO_DEV_SP_CCP which is the crypto coprocessor support.
>
> IIRC, the patch series which added this support started with naming it
> to CRYPTO_DEV_SP but somewhere during review process we discussed that
> since the module name is ccp.ko hence we kept the config with same name.
> We can submit a follow-up patch to correct it to avoid any further
> confusion.
>
>
>>> And "DD" for device driver is a pure tautology - most of the Kconfig
>>> items are device drivers :)
>>>
>>> Then,
>>>
>>> config KVM_AMD_SEV
>>> def_bool y
>>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>>> depends on KVM_AMD && X86_64
>>> depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
>>>
>>> that last line is pulling the required functionality for SEV but - and
>>> I *think* we have talked about this before - having a hierarchical
>>> dependency should make this a lot clearer and fix the build issues along
>>> the way.
>
> The first set of SEV patches accepted in upstream was using select
> instead of depends on. I used select mainly to ensure that all the
> drivers needed to run SEV is either builtin or module. A follow up
> patch was submitted by Paolo to use 'depends on' so that we don't create
> a circular dependency - I agree with that patch.
>
>>> Because, IMHO, KVM_AMD_SEV should depend only on CRYPTO_DEV_SP_PSP -
>>> i.e., the PSP device because SEV needs the PSP, right?
>
> I think depends should look like this:
>
> config KVM_AMD_SEV
> def_bool y
> bool "AMD Secure Encrypted Virtualization (SEV) support"
> depends KVM_AMD && X86_64
> depends CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
>
>
> Like you said, the KVM_AMD_SEV should depends only on
> "CRYPTO_DEV_SP_PSP". But when SEV support is enabled in KVM, we need a
> CCP driver at the runtime, hence we should ensure that if user selects
> KVM_AMD=y then CCP driver is also builtin otherwise she will not get SEV
> feature.
>
>
>>> Now, the
[PATCH] Use 'imply' with SEV Kconfig CRYPTO dependencies
On 14/06/18 14:08, Brijesh Singh wrote:
>
>
> On 6/14/18 2:58 AM, Richard Weinberger wrote:
>> On Wed, May 23, 2018 at 4:46 PM, Borislav Petkov wrote:
>>> + Tom and Brijesh.
>>>
>>> On Mon, May 21, 2018 at 10:12:53AM -0500, Janakarajan Natarajan wrote:
Use Kconfig imply 'option' when specifying SEV CRYPTO dependencies.
Example configuration:
.
.
CONFIG_CRYPTO_DEV_CCP=y
CONFIG_CRYPTO_DEV_CCP_DD=m
CONFIG_CRYPTO_DEV_SP_CCP=y
CONFIG_CRYPTO_DEV_CCP_CRYPTO=m
CONFIG_CRYPTO_DEV_SP_PSP=y
.
.
CONFIG_KVM_AMD=y
CONFIG_KVM_AMD_SEV=y
.
.
When the CRYPTO_DEV_SP_DD is m, KVM_AMD set to y produces compile time
errors.
Since KVM_AMD_SEV depends on KVM_AMD and CRYPTO_DEV_CCP_DD, the
issue can be prevented by using 'imply' Kconfig option when specifying
the CRYPTO dependencies.
Fixes: 505c9e94d832 ("KVM: x86: prefer "depends on" to "select" for SEV")
Signed-off-by: Janakarajan Natarajan
---
arch/x86/kvm/Kconfig | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 92fd433..d9b16b7 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -85,7 +85,9 @@ config KVM_AMD_SEV
def_bool y
bool "AMD Secure Encrypted Virtualization (SEV) support"
depends on KVM_AMD && X86_64
- depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
+ imply CRYPTO_DEV_CCP
+ imply CRYPTO_DEV_CCP_DD
+ imply CRYPTO_DEV_SP_PSP
---help---
Provides support for launching Encrypted VMs on AMD processors.
>>> Well, this solves the build issue but I just created a config:
>>>
>>> $ grep -E "(KVM|PSP)" .config | grep -v '#'
>>> CONFIG_HAVE_KVM=y
>>> CONFIG_HAVE_KVM_IRQCHIP=y
>>> CONFIG_HAVE_KVM_IRQFD=y
>>> CONFIG_HAVE_KVM_IRQ_ROUTING=y
>>> CONFIG_HAVE_KVM_EVENTFD=y
>>> CONFIG_KVM_MMIO=y
>>> CONFIG_KVM_ASYNC_PF=y
>>> CONFIG_HAVE_KVM_MSI=y
>>> CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT=y
>>> CONFIG_KVM_VFIO=y
>>> CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT=y
>>> CONFIG_KVM_COMPAT=y
>>> CONFIG_HAVE_KVM_IRQ_BYPASS=y
>>> CONFIG_KVM=y
>>> CONFIG_KVM_AMD=m
>>>
>>> which builds but the PSP functionality is not there. And I don't think
>>> this is serving the user: she should be able to select what she wants
>>> and get the required functionality added and not have build errors with
>>> any possible configuration.
>
> Yes, I agree.
>
>>>
>>> Now, looking at the security processor Kconfig stuff, it is somewhat
>>> confusing but maybe I didn't look at it long enough. A couple of points:
>>>
>>> config CRYPTO_DEV_CCP_DD
>>> tristate "Secure Processor device driver"
>>>
>>> If this is going to be the top-level menu item for the SP, call that
>>>
>>> CRYPTO_DEV_SP
>>>
>>> to mean, this is the security processor. CCP_DD is confusing because you
>>> have CRYPTO_DEV_SP_CCP which is the crypto coprocessor support.
>
> IIRC, the patch series which added this support started with naming it
> to CRYPTO_DEV_SP but somewhere during review process we discussed that
> since the module name is ccp.ko hence we kept the config with same name.
> We can submit a follow-up patch to correct it to avoid any further
> confusion.
>
>
>>> And "DD" for device driver is a pure tautology - most of the Kconfig
>>> items are device drivers :)
>>>
>>> Then,
>>>
>>> config KVM_AMD_SEV
>>> def_bool y
>>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>>> depends on KVM_AMD && X86_64
>>> depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
>>>
>>> that last line is pulling the required functionality for SEV but - and
>>> I *think* we have talked about this before - having a hierarchical
>>> dependency should make this a lot clearer and fix the build issues along
>>> the way.
>
> The first set of SEV patches accepted in upstream was using select
> instead of depends on. I used select mainly to ensure that all the
> drivers needed to run SEV is either builtin or module. A follow up
> patch was submitted by Paolo to use 'depends on' so that we don't create
> a circular dependency - I agree with that patch.
>
>>> Because, IMHO, KVM_AMD_SEV should depend only on CRYPTO_DEV_SP_PSP -
>>> i.e., the PSP device because SEV needs the PSP, right?
>
> I think depends should look like this:
>
> config KVM_AMD_SEV
> def_bool y
> bool "AMD Secure Encrypted Virtualization (SEV) support"
> depends KVM_AMD && X86_64
> depends CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
>
>
> Like you said, the KVM_AMD_SEV should depends only on
> "CRYPTO_DEV_SP_PSP". But when SEV support is enabled in KVM, we need a
> CCP driver at the runtime, hence we should ensure that if user selects
> KVM_AMD=y then CCP driver is also builtin otherwise she will not get SEV
> feature.
>
>
>>> Now, the
Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
Hi,
Some comments on the ranges below.
* Nishanth Menon [180607 16:41]:
> + soc0: soc0 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
I suggest you leave out the soc0, that's not real. Just make
the cbass@0 the top level interconnect. It can then provide
ranges to mcu interconnect which can provide ranges to the wkup
interconnect. So just model it after what's in the hardware :)
I found the following ranges based on a quick look at the TRM,
they could be split further if needed for power domains for
genpd for example.
main covers
0x00 - 0x540200
main provides at least the following ranges for mcu
0x002838 - 0x002bc0
0x004008 - 0x0041c8
0x004510 - 0x004518
0x004560 - 0x004564
0x004581 - 0x004586
0x004595 - 0x0045950400
0x0045a5 - 0x0045a50400
0x0045b04000 - 0x0045b06400
0x0045d1 - 0x0045d24000
0x004600 - 0x006000
0x04 - 0x08
0x4c3c02 - 0x4c3c03
0x4c3e00 - 0x4c3e04
0x54 - 0x540200
then mcu provides the following ranges for wkup
0x004200 - 0x0044410020
0x004500 - 0x004503
0x004508 - 0x00450a
0x0045808000 - 0x0045808800
0x0045b0 - 0x0045b02400
This based on looking at "figure 1-1. device top-level
block diagram" and the memory map in TRM.
Regards,
Tony
Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
Hi,
Some comments on the ranges below.
* Nishanth Menon [180607 16:41]:
> + soc0: soc0 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
I suggest you leave out the soc0, that's not real. Just make
the cbass@0 the top level interconnect. It can then provide
ranges to mcu interconnect which can provide ranges to the wkup
interconnect. So just model it after what's in the hardware :)
I found the following ranges based on a quick look at the TRM,
they could be split further if needed for power domains for
genpd for example.
main covers
0x00 - 0x540200
main provides at least the following ranges for mcu
0x002838 - 0x002bc0
0x004008 - 0x0041c8
0x004510 - 0x004518
0x004560 - 0x004564
0x004581 - 0x004586
0x004595 - 0x0045950400
0x0045a5 - 0x0045a50400
0x0045b04000 - 0x0045b06400
0x0045d1 - 0x0045d24000
0x004600 - 0x006000
0x04 - 0x08
0x4c3c02 - 0x4c3c03
0x4c3e00 - 0x4c3e04
0x54 - 0x540200
then mcu provides the following ranges for wkup
0x004200 - 0x0044410020
0x004500 - 0x004503
0x004508 - 0x00450a
0x0045808000 - 0x0045808800
0x0045b0 - 0x0045b02400
This based on looking at "figure 1-1. device top-level
block diagram" and the memory map in TRM.
Regards,
Tony
Re: [PATCH 2/2] ALSA: usb-audio: UAC3. Add insertion control for BADD.
On Tue, 12 Jun 2018 16:32:01 +0200, Jorge Sanjuan wrote: > > The HEADSET ADAPTER profile for BADD devices is meant to support > Insertion Control for the Input and Output Terminals of the headset. > Furthermore, this profile may also include the interrupt status pipe > to report changes on these terminals. > > This patch creates the jack-detect controls for the Headset Adapter > profile and enables the interrupt status pipe creation for BADD devices. > > Signed-off-by: Jorge Sanjuan The changes look good, but I'd prefer splitting this: - A preliminary work to change get_connector_control_name() and build_connector_control() to receive usb_mixer_interface; this is no functional change - Add build_connector_control() calls for UAC3 BADD - Apply snd_usb_mixer_status_create() for UAC3 BADD, too thanks, Takashi
Re: [PATCH 2/2] ALSA: usb-audio: UAC3. Add insertion control for BADD.
On Tue, 12 Jun 2018 16:32:01 +0200, Jorge Sanjuan wrote: > > The HEADSET ADAPTER profile for BADD devices is meant to support > Insertion Control for the Input and Output Terminals of the headset. > Furthermore, this profile may also include the interrupt status pipe > to report changes on these terminals. > > This patch creates the jack-detect controls for the Headset Adapter > profile and enables the interrupt status pipe creation for BADD devices. > > Signed-off-by: Jorge Sanjuan The changes look good, but I'd prefer splitting this: - A preliminary work to change get_connector_control_name() and build_connector_control() to receive usb_mixer_interface; this is no functional change - Add build_connector_control() calls for UAC3 BADD - Apply snd_usb_mixer_status_create() for UAC3 BADD, too thanks, Takashi
Re: [PATCH] mm, page_alloc: actually ignore mempolicies for high priority allocations
On 06/13/2018 09:42 PM, David Rientjes wrote:
> On Tue, 12 Jun 2018, Vlastimil Babka wrote:
>
>> The __alloc_pages_slowpath() function has for a long time contained code to
>> ignore node restrictions from memory policies for high priority allocations.
>> The current code that resets the zonelist iterator however does effectively
>> nothing after commit 7810e6781e0f ("mm, page_alloc: do not break
>> __GFP_THISNODE
>> by zonelist reset") removed a buggy zonelist reset. Even before that commit,
>> mempolicy restrictions were still not ignored, as they are passed in
>> ac->nodemask which is untouched by the code.
>>
>> We can either remove the code, or make it work as intended. Since
>> ac->nodemask can be set from task's mempolicy via alloc_pages_current() and
>> thus also alloc_pages(), it may indeed affect kernel allocations, and it
>> makes
>> sense to ignore it to allow progress for high priority allocations.
>>
>> Thus, this patch resets ac->nodemask to NULL in such cases. This assumes all
>> callers can handle it (i.e. there are no guarantees as in the case of
>> __GFP_THISNODE) which seems to be the case. The same assumption is already
>> present in check_retry_cpuset() for some time.
>>
>> The expected effect is that high priority kernel allocations in the context
>> of
>> userspace tasks (e.g. OOM victims) restricted by mempolicies will have higher
>> chance to succeed if they are restricted to nodes with depleted memory, while
>> there are other nodes with free memory left.
>>
>
> Hi Vlastimil,
>
> Is this expected as a change back to previous behavior that we have lost
> or is this new development for high priority allocations? I don't think
> we have ignored mempolicies for things like GFP_ATOMIC allocations in the
> past.
Well, it's not a new intention, but for the first time the code will
match the intention, AFAICS. It was intended by commit 183f6371aac2
("mm: ignore mempolicies when using ALLOC_NO_WATERMARK") in v3.6 but I
think it never really worked, as mempolicy restriction was already
encoded in nodemask, not zonelist, at that time.
So originally that was for ALLOC_NO_WATERMARK only. Then it was adjusted
by e46e7b77c909 ("mm, page_alloc: recalculate the preferred zoneref if
the context can ignore memory policies") and cd04ae1e2dc8 ("mm, oom: do
not rely on TIF_MEMDIE for memory reserves access") to the current
state. So yeah even GFP_ATOMIC would now ignore mempolicies after the
initial attempts fail... if the code worked as people thought it does.
>> Signed-off-by: Vlastimil Babka
>> Cc: Mel Gorman
>> Cc: Michal Hocko
>> Cc: David Rientjes
>> Cc: Joonsoo Kim
>> ---
>> mm/page_alloc.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 07b3c23762ad..ec8c92ff8b3c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -4164,11 +4164,12 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int
>> order,
>> alloc_flags = reserve_flags;
>>
>> /*
>> - * Reset the zonelist iterators if memory policies can be ignored.
>> - * These allocations are high priority and system rather than user
>> - * orientated.
>> + * Reset the nodemask and zonelist iterators if memory policies can be
>> + * ignored. These allocations are high priority and system rather than
>> + * user oriented.
>> */
>> if (!(alloc_flags & ALLOC_CPUSET) || reserve_flags) {
>> +ac->nodemask = NULL;
>> ac->preferred_zoneref = first_zones_zonelist(ac->zonelist,
>> ac->high_zoneidx, ac->nodemask);
>> }
Re: [PATCH] mm, page_alloc: actually ignore mempolicies for high priority allocations
On 06/13/2018 09:42 PM, David Rientjes wrote:
> On Tue, 12 Jun 2018, Vlastimil Babka wrote:
>
>> The __alloc_pages_slowpath() function has for a long time contained code to
>> ignore node restrictions from memory policies for high priority allocations.
>> The current code that resets the zonelist iterator however does effectively
>> nothing after commit 7810e6781e0f ("mm, page_alloc: do not break
>> __GFP_THISNODE
>> by zonelist reset") removed a buggy zonelist reset. Even before that commit,
>> mempolicy restrictions were still not ignored, as they are passed in
>> ac->nodemask which is untouched by the code.
>>
>> We can either remove the code, or make it work as intended. Since
>> ac->nodemask can be set from task's mempolicy via alloc_pages_current() and
>> thus also alloc_pages(), it may indeed affect kernel allocations, and it
>> makes
>> sense to ignore it to allow progress for high priority allocations.
>>
>> Thus, this patch resets ac->nodemask to NULL in such cases. This assumes all
>> callers can handle it (i.e. there are no guarantees as in the case of
>> __GFP_THISNODE) which seems to be the case. The same assumption is already
>> present in check_retry_cpuset() for some time.
>>
>> The expected effect is that high priority kernel allocations in the context
>> of
>> userspace tasks (e.g. OOM victims) restricted by mempolicies will have higher
>> chance to succeed if they are restricted to nodes with depleted memory, while
>> there are other nodes with free memory left.
>>
>
> Hi Vlastimil,
>
> Is this expected as a change back to previous behavior that we have lost
> or is this new development for high priority allocations? I don't think
> we have ignored mempolicies for things like GFP_ATOMIC allocations in the
> past.
Well, it's not a new intention, but for the first time the code will
match the intention, AFAICS. It was intended by commit 183f6371aac2
("mm: ignore mempolicies when using ALLOC_NO_WATERMARK") in v3.6 but I
think it never really worked, as mempolicy restriction was already
encoded in nodemask, not zonelist, at that time.
So originally that was for ALLOC_NO_WATERMARK only. Then it was adjusted
by e46e7b77c909 ("mm, page_alloc: recalculate the preferred zoneref if
the context can ignore memory policies") and cd04ae1e2dc8 ("mm, oom: do
not rely on TIF_MEMDIE for memory reserves access") to the current
state. So yeah even GFP_ATOMIC would now ignore mempolicies after the
initial attempts fail... if the code worked as people thought it does.
>> Signed-off-by: Vlastimil Babka
>> Cc: Mel Gorman
>> Cc: Michal Hocko
>> Cc: David Rientjes
>> Cc: Joonsoo Kim
>> ---
>> mm/page_alloc.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 07b3c23762ad..ec8c92ff8b3c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -4164,11 +4164,12 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int
>> order,
>> alloc_flags = reserve_flags;
>>
>> /*
>> - * Reset the zonelist iterators if memory policies can be ignored.
>> - * These allocations are high priority and system rather than user
>> - * orientated.
>> + * Reset the nodemask and zonelist iterators if memory policies can be
>> + * ignored. These allocations are high priority and system rather than
>> + * user oriented.
>> */
>> if (!(alloc_flags & ALLOC_CPUSET) || reserve_flags) {
>> +ac->nodemask = NULL;
>> ac->preferred_zoneref = first_zones_zonelist(ac->zonelist,
>> ac->high_zoneidx, ac->nodemask);
>> }
Re: Restartable Sequences system call merged into Linux
On Tue 2018-06-12 12:31:24, Mathieu Desnoyers wrote: > - On Jun 12, 2018, at 9:11 AM, Florian Weimer fwei...@redhat.com wrote: > > > On 06/11/2018 10:04 PM, Mathieu Desnoyers wrote: > >> - On Jun 11, 2018, at 3:55 PM, Florian Weimer fwei...@redhat.com wrote: > >> > >>> On 06/11/2018 09:49 PM, Mathieu Desnoyers wrote: > It should be noted that there can be only one rseq TLS area registered > per > thread, > which can then be used by many libraries and by the executable, so this > is a > process-wide (per-thread) resource that we need to manage carefully. > >>> > >>> Is it possible to resize the area after thread creation, perhaps even > >>> from other threads? > >> > >> I'm not sure why we would want to resize it. The per-thread area is > >> fixed-size. > >> Its layout is here: include/uapi/linux/rseq.h: struct rseq > > > > Looks I was mistaken and this is very similar to the robust mutex list. > > > > Should we treat it the same way? Always allocate it for each new thread > > and register it with the kernel? > > That would be an efficient way to do it, indeed. There is very little > performance overhead to have rseq registered for all threads, whether or > not they intend to run rseq critical sections. People with slow / low memory machines would prefer not to see overhead they don't need... > I have a few possible approaches in mind (feel free to suggest other > options): > > A) glibc exposes a strong __rseq_abi TLS symbol: > >- should ideally *not* be global-dynamic for performance reasons, >- registration to kernel can either be handled explicitly by requiring > application or libraries to call an API, or implicitly at thread > creation, ...so I'd prefer explicit API call. > B) librseq.so exposes a strong __rseq_abi symbol: Works for me. 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: Restartable Sequences system call merged into Linux
On Tue 2018-06-12 12:31:24, Mathieu Desnoyers wrote: > - On Jun 12, 2018, at 9:11 AM, Florian Weimer fwei...@redhat.com wrote: > > > On 06/11/2018 10:04 PM, Mathieu Desnoyers wrote: > >> - On Jun 11, 2018, at 3:55 PM, Florian Weimer fwei...@redhat.com wrote: > >> > >>> On 06/11/2018 09:49 PM, Mathieu Desnoyers wrote: > It should be noted that there can be only one rseq TLS area registered > per > thread, > which can then be used by many libraries and by the executable, so this > is a > process-wide (per-thread) resource that we need to manage carefully. > >>> > >>> Is it possible to resize the area after thread creation, perhaps even > >>> from other threads? > >> > >> I'm not sure why we would want to resize it. The per-thread area is > >> fixed-size. > >> Its layout is here: include/uapi/linux/rseq.h: struct rseq > > > > Looks I was mistaken and this is very similar to the robust mutex list. > > > > Should we treat it the same way? Always allocate it for each new thread > > and register it with the kernel? > > That would be an efficient way to do it, indeed. There is very little > performance overhead to have rseq registered for all threads, whether or > not they intend to run rseq critical sections. People with slow / low memory machines would prefer not to see overhead they don't need... > I have a few possible approaches in mind (feel free to suggest other > options): > > A) glibc exposes a strong __rseq_abi TLS symbol: > >- should ideally *not* be global-dynamic for performance reasons, >- registration to kernel can either be handled explicitly by requiring > application or libraries to call an API, or implicitly at thread > creation, ...so I'd prefer explicit API call. > B) librseq.so exposes a strong __rseq_abi symbol: Works for me. 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 2/2] PCI: cleanup PCI_REBAR_CTRL_BAR_SHIFT handling
That was hard coded instead of properly defined in the header for some reason. Signed-off-by: Christian König --- drivers/pci/pci.c | 6 +++--- include/uapi/linux/pci_regs.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d4685090378b..a905e35d9360 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1181,7 +1181,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev) res = pdev->resource + bar_idx; size = order_base_2((resource_size(res) >> 20) | 1) - 1; ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE; - ctrl |= size << 8; + ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT; pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl); } } @@ -3066,7 +3066,7 @@ int pci_rebar_get_current_size(struct pci_dev *pdev, int bar) return pos; pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, ); - return (ctrl & PCI_REBAR_CTRL_BAR_SIZE) >> 8; + return (ctrl & PCI_REBAR_CTRL_BAR_SIZE) >> PCI_REBAR_CTRL_BAR_SHIFT; } /** @@ -3089,7 +3089,7 @@ int pci_rebar_set_size(struct pci_dev *pdev, int bar, int size) pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, ); ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE; - ctrl |= size << 8; + ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT; pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl); return 0; } diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 0c79eac5e9b8..7358ad00f5dd 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -950,8 +950,9 @@ #define PCI_REBAR_CTRL 8 /* control register */ #define PCI_REBAR_CTRL_BAR_IDX0x0007 /* BAR index */ #define PCI_REBAR_CTRL_NBAR_MASK 0x00E0 /* # of resizable BARs */ -#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # of BARs */ +#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # of BARs */ #define PCI_REBAR_CTRL_BAR_SIZE 0x1F00 /* BAR size */ +#define PCI_REBAR_CTRL_BAR_SHIFT 8 /* shift for BAR size */ /* Dynamic Power Allocation */ #define PCI_DPA_CAP4 /* capability register */ -- 2.14.1
[PATCH 2/2] PCI: cleanup PCI_REBAR_CTRL_BAR_SHIFT handling
That was hard coded instead of properly defined in the header for some reason. Signed-off-by: Christian König --- drivers/pci/pci.c | 6 +++--- include/uapi/linux/pci_regs.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d4685090378b..a905e35d9360 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1181,7 +1181,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev) res = pdev->resource + bar_idx; size = order_base_2((resource_size(res) >> 20) | 1) - 1; ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE; - ctrl |= size << 8; + ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT; pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl); } } @@ -3066,7 +3066,7 @@ int pci_rebar_get_current_size(struct pci_dev *pdev, int bar) return pos; pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, ); - return (ctrl & PCI_REBAR_CTRL_BAR_SIZE) >> 8; + return (ctrl & PCI_REBAR_CTRL_BAR_SIZE) >> PCI_REBAR_CTRL_BAR_SHIFT; } /** @@ -3089,7 +3089,7 @@ int pci_rebar_set_size(struct pci_dev *pdev, int bar, int size) pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, ); ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE; - ctrl |= size << 8; + ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT; pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl); return 0; } diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 0c79eac5e9b8..7358ad00f5dd 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -950,8 +950,9 @@ #define PCI_REBAR_CTRL 8 /* control register */ #define PCI_REBAR_CTRL_BAR_IDX0x0007 /* BAR index */ #define PCI_REBAR_CTRL_NBAR_MASK 0x00E0 /* # of resizable BARs */ -#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # of BARs */ +#define PCI_REBAR_CTRL_NBAR_SHIFT 5 /* shift for # of BARs */ #define PCI_REBAR_CTRL_BAR_SIZE 0x1F00 /* BAR size */ +#define PCI_REBAR_CTRL_BAR_SHIFT 8 /* shift for BAR size */ /* Dynamic Power Allocation */ #define PCI_DPA_CAP4 /* capability register */ -- 2.14.1
[PATCH 1/2] PCI: fix restoring resized BAR state on resume
Resize BARs after resume to the expected size again.
Signed-off-by: Christian König
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
CC: sta...@vger.kernel.org # v4.15+
---
drivers/pci/pci.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bd6f156dc3cf..d4685090378b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev
*pdev)
}
}
+static void pci_restore_rebar_state(struct pci_dev *pdev)
+{
+ unsigned int pos, nbars, i;
+ u32 ctrl;
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+ if (!pos)
+ return;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, );
+ nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
+ PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ for (i = 0; i < nbars; i++, pos += 8) {
+ struct resource *res;
+ int bar_idx, size;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, );
+ bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
+ res = pdev->resource + bar_idx;
+ size = order_base_2((resource_size(res) >> 20) | 1) - 1;
+ ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
+ ctrl |= size << 8;
+ pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
+ }
+}
+
/**
* pci_restore_state - Restore the saved state of a PCI device
* @dev: - PCI device that we're dealing with
@@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
pci_restore_pri_state(dev);
pci_restore_ats_state(dev);
pci_restore_vc_state(dev);
+ pci_restore_rebar_state(dev);
pci_cleanup_aer_error_status_regs(dev);
--
2.14.1
[PATCH 1/2] PCI: fix restoring resized BAR state on resume
Resize BARs after resume to the expected size again.
Signed-off-by: Christian König
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
CC: sta...@vger.kernel.org # v4.15+
---
drivers/pci/pci.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bd6f156dc3cf..d4685090378b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev
*pdev)
}
}
+static void pci_restore_rebar_state(struct pci_dev *pdev)
+{
+ unsigned int pos, nbars, i;
+ u32 ctrl;
+
+ pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+ if (!pos)
+ return;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, );
+ nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
+ PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ for (i = 0; i < nbars; i++, pos += 8) {
+ struct resource *res;
+ int bar_idx, size;
+
+ pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, );
+ bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
+ res = pdev->resource + bar_idx;
+ size = order_base_2((resource_size(res) >> 20) | 1) - 1;
+ ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
+ ctrl |= size << 8;
+ pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
+ }
+}
+
/**
* pci_restore_state - Restore the saved state of a PCI device
* @dev: - PCI device that we're dealing with
@@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
pci_restore_pri_state(dev);
pci_restore_ats_state(dev);
pci_restore_vc_state(dev);
+ pci_restore_rebar_state(dev);
pci_cleanup_aer_error_status_regs(dev);
--
2.14.1
Re: BUG: drivers/pinctrl/core: races in pinctrl_groups and deferred probing
Hi Tony, > Am 14.06.2018 um 14:01 schrieb Tony Lindgren : > > * H. Nikolaus Schaller [180613 12:41]: >> >> Now if I look into pinctrl_generic_add_group() and >> pinctrl_generic_get_group_name(), >> pctldev->num_groups++ is not protected if pinctrl_generic_add_group() may be >> called by >> two threads in parallel for the same pctldev. Hence a second thread may try >> to insert >> a different node into the radix tree at the same selector index. This fails >> but there >> is no error check - and the second entry is completely missing (but probably >> assumed to >> be there). > > Sounds like pinctrl-single.c is missing mutex around calls to > pinctrl_generic_add_group()? Yes, that could be. I didn't research the call path, just the one of devm_pinctrl_get(). That uses a mutex in https://elixir.bootlin.com/linux/v4.17.1/source/drivers/pinctrl/core.c#L1021 Maybe a similar mutex is missing elsewhere. BR, Nikolaus
Re: BUG: drivers/pinctrl/core: races in pinctrl_groups and deferred probing
Hi Tony, > Am 14.06.2018 um 14:01 schrieb Tony Lindgren : > > * H. Nikolaus Schaller [180613 12:41]: >> >> Now if I look into pinctrl_generic_add_group() and >> pinctrl_generic_get_group_name(), >> pctldev->num_groups++ is not protected if pinctrl_generic_add_group() may be >> called by >> two threads in parallel for the same pctldev. Hence a second thread may try >> to insert >> a different node into the radix tree at the same selector index. This fails >> but there >> is no error check - and the second entry is completely missing (but probably >> assumed to >> be there). > > Sounds like pinctrl-single.c is missing mutex around calls to > pinctrl_generic_add_group()? Yes, that could be. I didn't research the call path, just the one of devm_pinctrl_get(). That uses a mutex in https://elixir.bootlin.com/linux/v4.17.1/source/drivers/pinctrl/core.c#L1021 Maybe a similar mutex is missing elsewhere. BR, Nikolaus
Re: [PATCH] Use 'imply' with SEV Kconfig CRYPTO dependencies
On 6/14/18 2:58 AM, Richard Weinberger wrote:
> On Wed, May 23, 2018 at 4:46 PM, Borislav Petkov wrote:
>> + Tom and Brijesh.
>>
>> On Mon, May 21, 2018 at 10:12:53AM -0500, Janakarajan Natarajan wrote:
>>> Use Kconfig imply 'option' when specifying SEV CRYPTO dependencies.
>>>
>>> Example configuration:
>>> .
>>> .
>>> CONFIG_CRYPTO_DEV_CCP=y
>>> CONFIG_CRYPTO_DEV_CCP_DD=m
>>> CONFIG_CRYPTO_DEV_SP_CCP=y
>>> CONFIG_CRYPTO_DEV_CCP_CRYPTO=m
>>> CONFIG_CRYPTO_DEV_SP_PSP=y
>>> .
>>> .
>>> CONFIG_KVM_AMD=y
>>> CONFIG_KVM_AMD_SEV=y
>>> .
>>> .
>>>
>>> When the CRYPTO_DEV_SP_DD is m, KVM_AMD set to y produces compile time
>>> errors.
>>>
>>> Since KVM_AMD_SEV depends on KVM_AMD and CRYPTO_DEV_CCP_DD, the
>>> issue can be prevented by using 'imply' Kconfig option when specifying
>>> the CRYPTO dependencies.
>>>
>>> Fixes: 505c9e94d832 ("KVM: x86: prefer "depends on" to "select" for SEV")
>>>
>>> Signed-off-by: Janakarajan Natarajan
>>> ---
>>> arch/x86/kvm/Kconfig | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>>> index 92fd433..d9b16b7 100644
>>> --- a/arch/x86/kvm/Kconfig
>>> +++ b/arch/x86/kvm/Kconfig
>>> @@ -85,7 +85,9 @@ config KVM_AMD_SEV
>>> def_bool y
>>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>>> depends on KVM_AMD && X86_64
>>> - depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
>>> + imply CRYPTO_DEV_CCP
>>> + imply CRYPTO_DEV_CCP_DD
>>> + imply CRYPTO_DEV_SP_PSP
>>> ---help---
>>> Provides support for launching Encrypted VMs on AMD processors.
>> Well, this solves the build issue but I just created a config:
>>
>> $ grep -E "(KVM|PSP)" .config | grep -v '#'
>> CONFIG_HAVE_KVM=y
>> CONFIG_HAVE_KVM_IRQCHIP=y
>> CONFIG_HAVE_KVM_IRQFD=y
>> CONFIG_HAVE_KVM_IRQ_ROUTING=y
>> CONFIG_HAVE_KVM_EVENTFD=y
>> CONFIG_KVM_MMIO=y
>> CONFIG_KVM_ASYNC_PF=y
>> CONFIG_HAVE_KVM_MSI=y
>> CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT=y
>> CONFIG_KVM_VFIO=y
>> CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT=y
>> CONFIG_KVM_COMPAT=y
>> CONFIG_HAVE_KVM_IRQ_BYPASS=y
>> CONFIG_KVM=y
>> CONFIG_KVM_AMD=m
>>
>> which builds but the PSP functionality is not there. And I don't think
>> this is serving the user: she should be able to select what she wants
>> and get the required functionality added and not have build errors with
>> any possible configuration.
Yes, I agree.
>>
>> Now, looking at the security processor Kconfig stuff, it is somewhat
>> confusing but maybe I didn't look at it long enough. A couple of points:
>>
>> config CRYPTO_DEV_CCP_DD
>> tristate "Secure Processor device driver"
>>
>> If this is going to be the top-level menu item for the SP, call that
>>
>> CRYPTO_DEV_SP
>>
>> to mean, this is the security processor. CCP_DD is confusing because you
>> have CRYPTO_DEV_SP_CCP which is the crypto coprocessor support.
IIRC, the patch series which added this support started with naming it
to CRYPTO_DEV_SP but somewhere during review process we discussed that
since the module name is ccp.ko hence we kept the config with same name.
We can submit a follow-up patch to correct it to avoid any further
confusion.
>> And "DD" for device driver is a pure tautology - most of the Kconfig
>> items are device drivers :)
>>
>> Then,
>>
>> config KVM_AMD_SEV
>> def_bool y
>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>> depends on KVM_AMD && X86_64
>> depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
>>
>> that last line is pulling the required functionality for SEV but - and
>> I *think* we have talked about this before - having a hierarchical
>> dependency should make this a lot clearer and fix the build issues along
>> the way.
The first set of SEV patches accepted in upstream was using select
instead of depends on. I used select mainly to ensure that all the
drivers needed to run SEV is either builtin or module. A follow up
patch was submitted by Paolo to use 'depends on' so that we don't create
a circular dependency - I agree with that patch.
>> Because, IMHO, KVM_AMD_SEV should depend only on CRYPTO_DEV_SP_PSP -
>> i.e., the PSP device because SEV needs the PSP, right?
I think depends should look like this:
config KVM_AMD_SEV
def_bool y
bool "AMD Secure Encrypted Virtualization (SEV) support"
depends KVM_AMD && X86_64
depends CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
Like you said, the KVM_AMD_SEV should depends only on
"CRYPTO_DEV_SP_PSP". But when SEV support is enabled in KVM, we need a
CCP driver at the runtime, hence we should ensure that if user selects
KVM_AMD=y then CCP driver is also builtin otherwise she will not get SEV
feature.
>> Now, the PSP device *itself* should depend on whatever it needs to
>> function properly, CRYPTO_DEV_CCP_DD I guess.
>>
>> But SEV should not depend on CRYPTO_DEV_CCP - which is the top-level
>> Kconfig item - that should be
Re: [PATCH] Use 'imply' with SEV Kconfig CRYPTO dependencies
On 6/14/18 2:58 AM, Richard Weinberger wrote:
> On Wed, May 23, 2018 at 4:46 PM, Borislav Petkov wrote:
>> + Tom and Brijesh.
>>
>> On Mon, May 21, 2018 at 10:12:53AM -0500, Janakarajan Natarajan wrote:
>>> Use Kconfig imply 'option' when specifying SEV CRYPTO dependencies.
>>>
>>> Example configuration:
>>> .
>>> .
>>> CONFIG_CRYPTO_DEV_CCP=y
>>> CONFIG_CRYPTO_DEV_CCP_DD=m
>>> CONFIG_CRYPTO_DEV_SP_CCP=y
>>> CONFIG_CRYPTO_DEV_CCP_CRYPTO=m
>>> CONFIG_CRYPTO_DEV_SP_PSP=y
>>> .
>>> .
>>> CONFIG_KVM_AMD=y
>>> CONFIG_KVM_AMD_SEV=y
>>> .
>>> .
>>>
>>> When the CRYPTO_DEV_SP_DD is m, KVM_AMD set to y produces compile time
>>> errors.
>>>
>>> Since KVM_AMD_SEV depends on KVM_AMD and CRYPTO_DEV_CCP_DD, the
>>> issue can be prevented by using 'imply' Kconfig option when specifying
>>> the CRYPTO dependencies.
>>>
>>> Fixes: 505c9e94d832 ("KVM: x86: prefer "depends on" to "select" for SEV")
>>>
>>> Signed-off-by: Janakarajan Natarajan
>>> ---
>>> arch/x86/kvm/Kconfig | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>>> index 92fd433..d9b16b7 100644
>>> --- a/arch/x86/kvm/Kconfig
>>> +++ b/arch/x86/kvm/Kconfig
>>> @@ -85,7 +85,9 @@ config KVM_AMD_SEV
>>> def_bool y
>>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>>> depends on KVM_AMD && X86_64
>>> - depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
>>> + imply CRYPTO_DEV_CCP
>>> + imply CRYPTO_DEV_CCP_DD
>>> + imply CRYPTO_DEV_SP_PSP
>>> ---help---
>>> Provides support for launching Encrypted VMs on AMD processors.
>> Well, this solves the build issue but I just created a config:
>>
>> $ grep -E "(KVM|PSP)" .config | grep -v '#'
>> CONFIG_HAVE_KVM=y
>> CONFIG_HAVE_KVM_IRQCHIP=y
>> CONFIG_HAVE_KVM_IRQFD=y
>> CONFIG_HAVE_KVM_IRQ_ROUTING=y
>> CONFIG_HAVE_KVM_EVENTFD=y
>> CONFIG_KVM_MMIO=y
>> CONFIG_KVM_ASYNC_PF=y
>> CONFIG_HAVE_KVM_MSI=y
>> CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT=y
>> CONFIG_KVM_VFIO=y
>> CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT=y
>> CONFIG_KVM_COMPAT=y
>> CONFIG_HAVE_KVM_IRQ_BYPASS=y
>> CONFIG_KVM=y
>> CONFIG_KVM_AMD=m
>>
>> which builds but the PSP functionality is not there. And I don't think
>> this is serving the user: she should be able to select what she wants
>> and get the required functionality added and not have build errors with
>> any possible configuration.
Yes, I agree.
>>
>> Now, looking at the security processor Kconfig stuff, it is somewhat
>> confusing but maybe I didn't look at it long enough. A couple of points:
>>
>> config CRYPTO_DEV_CCP_DD
>> tristate "Secure Processor device driver"
>>
>> If this is going to be the top-level menu item for the SP, call that
>>
>> CRYPTO_DEV_SP
>>
>> to mean, this is the security processor. CCP_DD is confusing because you
>> have CRYPTO_DEV_SP_CCP which is the crypto coprocessor support.
IIRC, the patch series which added this support started with naming it
to CRYPTO_DEV_SP but somewhere during review process we discussed that
since the module name is ccp.ko hence we kept the config with same name.
We can submit a follow-up patch to correct it to avoid any further
confusion.
>> And "DD" for device driver is a pure tautology - most of the Kconfig
>> items are device drivers :)
>>
>> Then,
>>
>> config KVM_AMD_SEV
>> def_bool y
>> bool "AMD Secure Encrypted Virtualization (SEV) support"
>> depends on KVM_AMD && X86_64
>> depends on CRYPTO_DEV_CCP && CRYPTO_DEV_CCP_DD && CRYPTO_DEV_SP_PSP
>>
>> that last line is pulling the required functionality for SEV but - and
>> I *think* we have talked about this before - having a hierarchical
>> dependency should make this a lot clearer and fix the build issues along
>> the way.
The first set of SEV patches accepted in upstream was using select
instead of depends on. I used select mainly to ensure that all the
drivers needed to run SEV is either builtin or module. A follow up
patch was submitted by Paolo to use 'depends on' so that we don't create
a circular dependency - I agree with that patch.
>> Because, IMHO, KVM_AMD_SEV should depend only on CRYPTO_DEV_SP_PSP -
>> i.e., the PSP device because SEV needs the PSP, right?
I think depends should look like this:
config KVM_AMD_SEV
def_bool y
bool "AMD Secure Encrypted Virtualization (SEV) support"
depends KVM_AMD && X86_64
depends CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
Like you said, the KVM_AMD_SEV should depends only on
"CRYPTO_DEV_SP_PSP". But when SEV support is enabled in KVM, we need a
CCP driver at the runtime, hence we should ensure that if user selects
KVM_AMD=y then CCP driver is also builtin otherwise she will not get SEV
feature.
>> Now, the PSP device *itself* should depend on whatever it needs to
>> function properly, CRYPTO_DEV_CCP_DD I guess.
>>
>> But SEV should not depend on CRYPTO_DEV_CCP - which is the top-level
>> Kconfig item - that should be
Re: [PATCH v1] aio: mark __aio_sigset::sigmask const
On Fri, Jun 08, 2018 at 05:55:05PM +0300, Avi Kivity wrote:
> io_pgetevents() will not change the signal mask. Mark it const
> to make it clear and to reduce the need for casts in user code.
>
> Signed-off-by: Avi Kivity
Al, can you pick this one and the masking fix before -rc1?
> ---
> include/uapi/linux/aio_abi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
> index ed0185945bb2..cdf115b03761 100644
> --- a/include/uapi/linux/aio_abi.h
> +++ b/include/uapi/linux/aio_abi.h
> @@ -106,11 +106,11 @@ struct iocb {
>
> #undef IFBIG
> #undef IFLITTLE
>
> struct __aio_sigset {
> - sigset_t __user *sigmask;
> + const sigset_t __user *sigmask;
> size_t sigsetsize;
> };
>
> #endif /* __LINUX__AIO_ABI_H */
>
> --
> 2.14.4
---end quoted text---
Re: [PATCH v1] aio: mark __aio_sigset::sigmask const
On Fri, Jun 08, 2018 at 05:55:05PM +0300, Avi Kivity wrote:
> io_pgetevents() will not change the signal mask. Mark it const
> to make it clear and to reduce the need for casts in user code.
>
> Signed-off-by: Avi Kivity
Al, can you pick this one and the masking fix before -rc1?
> ---
> include/uapi/linux/aio_abi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
> index ed0185945bb2..cdf115b03761 100644
> --- a/include/uapi/linux/aio_abi.h
> +++ b/include/uapi/linux/aio_abi.h
> @@ -106,11 +106,11 @@ struct iocb {
>
> #undef IFBIG
> #undef IFLITTLE
>
> struct __aio_sigset {
> - sigset_t __user *sigmask;
> + const sigset_t __user *sigmask;
> size_t sigsetsize;
> };
>
> #endif /* __LINUX__AIO_ABI_H */
>
> --
> 2.14.4
---end quoted text---
Re: [PATCH v1] bitmap: Drop unnecessary 0 check for u32 array operations
On Thu, May 31, 2018 at 5:47 PM, Yury Norov wrote: > On Thu, May 31, 2018 at 04:19:14PM +0300, Andy Shevchenko wrote: >> The nbits == 0 is safe to be supplied to the function body, so, >> remove unnecessary checks in bitmap_to_arr32() and bitmap_from_arr32(). >> +void bitmap_from_arr32(unsigned long *bitmap, const u32 *buf, unsigned int >> nbits) > > This line is 81 characters length, and it will trigger checkpatch warning. > (But I'm OK with it.) I really don't care about ) at the end potentially not visible on the screen (who is using nice old hardware terminals? :-) ). > Acked-by: Yury Norov Thanks. -- With Best Regards, Andy Shevchenko
Re: [PATCH v1] bitmap: Drop unnecessary 0 check for u32 array operations
On Thu, May 31, 2018 at 5:47 PM, Yury Norov wrote: > On Thu, May 31, 2018 at 04:19:14PM +0300, Andy Shevchenko wrote: >> The nbits == 0 is safe to be supplied to the function body, so, >> remove unnecessary checks in bitmap_to_arr32() and bitmap_from_arr32(). >> +void bitmap_from_arr32(unsigned long *bitmap, const u32 *buf, unsigned int >> nbits) > > This line is 81 characters length, and it will trigger checkpatch warning. > (But I'm OK with it.) I really don't care about ) at the end potentially not visible on the screen (who is using nice old hardware terminals? :-) ). > Acked-by: Yury Norov Thanks. -- With Best Regards, Andy Shevchenko
Re: BUG: drivers/pinctrl/core: races in pinctrl_groups and deferred probing
* H. Nikolaus Schaller [180613 12:41]: > > Now if I look into pinctrl_generic_add_group() and > pinctrl_generic_get_group_name(), > pctldev->num_groups++ is not protected if pinctrl_generic_add_group() may be > called by > two threads in parallel for the same pctldev. Hence a second thread may try > to insert > a different node into the radix tree at the same selector index. This fails > but there > is no error check - and the second entry is completely missing (but probably > assumed to > be there). Sounds like pinctrl-single.c is missing mutex around calls to pinctrl_generic_add_group()? Regards, Tony
Re: BUG: drivers/pinctrl/core: races in pinctrl_groups and deferred probing
* H. Nikolaus Schaller [180613 12:41]: > > Now if I look into pinctrl_generic_add_group() and > pinctrl_generic_get_group_name(), > pctldev->num_groups++ is not protected if pinctrl_generic_add_group() may be > called by > two threads in parallel for the same pctldev. Hence a second thread may try > to insert > a different node into the radix tree at the same selector index. This fails > but there > is no error check - and the second entry is completely missing (but probably > assumed to > be there). Sounds like pinctrl-single.c is missing mutex around calls to pinctrl_generic_add_group()? Regards, Tony
[PATCH 2/3] perf alias: Rebuild alias expression string to make it comparable
PMU alias definitions in sysfs files may have spaces, newlines
and number with leading zeroes. Same alias definitions may
also appear in JSON files without spaces, etc.
Scan alias definitions and remove leading zeroes, spaces,
newlines, etc and rebuild string to make alias->str member
comparable.
s390 for example has terms specified as
event=0x0091 (read from files ..//events/
and terms specified as event=0x91 (read from JSON files).
Signed-off-by: Thomas Richter
---
tools/perf/util/pmu.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 26c79a9c4142..da8f243743d3 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -241,9 +241,11 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
char *metric_expr,
char *metric_name)
{
+ struct parse_events_term *term;
struct perf_pmu_alias *alias;
int ret;
int num;
+ char newval[256];
alias = malloc(sizeof(*alias));
if (!alias)
@@ -262,6 +264,27 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
return ret;
}
+ /* Scan event and remove leading zeroes, spaces, newlines, some
+* platforms have terms specified as
+* event=0x0091 (read from files ..//events/
+* and terms specified as event=0x91 (read from JSON files).
+*
+* Rebuild string to make alias->str member comparable.
+*/
+ memset(newval, 0, sizeof(newval));
+ ret = 0;
+ list_for_each_entry(term, >terms, list) {
+ if (ret)
+ ret += scnprintf(newval + ret, sizeof(newval) - ret,
+",");
+ if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+ ret += scnprintf(newval + ret, sizeof(newval) - ret,
+"%s=%#x", term->config, term->val.num);
+ else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
+ ret += scnprintf(newval + ret, sizeof(newval) - ret,
+"%s=%s", term->config, term->val.str);
+ }
+
alias->name = strdup(name);
if (dir) {
/*
@@ -285,7 +308,7 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
}
alias->per_pkg = perpkg && sscanf(perpkg, "%d", ) == 1 && num == 1;
- alias->str = strdup(val);
+ alias->str = strdup(newval);
list_add_tail(>list, list);
--
2.14.3
[PATCH 1/3] perf alias: Remove trailing newline when reading sysfs files
Remove a trailing newline when reading sysfs file contents
such as /sys/devices/cpum_cf/events/TX_NC_TEND.
This show when verbose option -v is used.
Output before:
tx_nc_tend -> 'cpum_cf'/'event=0x008d
'/
Output after:
tx_nc_tend -> 'cpum_cf'/'event=0x8d'/
Signed-off-by: Thomas Richter
---
tools/perf/util/pmu.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 7878934ebb23..26c79a9c4142 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -294,7 +294,7 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
FILE *file)
{
- char buf[256];
+ char *cp, buf[256];
int ret;
ret = fread(buf, 1, sizeof(buf), file);
@@ -303,6 +303,11 @@ static int perf_pmu__new_alias(struct list_head *list,
char *dir, char *name, FI
buf[ret] = 0;
+ /* Remove trailing newline from sysfs file */
+ cp = strrchr(buf, '\n');
+ if (cp)
+ *cp = '\0';
+
return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL,
NULL,
NULL, NULL, NULL);
}
--
2.14.3
[PATCH 2/3] perf alias: Rebuild alias expression string to make it comparable
PMU alias definitions in sysfs files may have spaces, newlines
and number with leading zeroes. Same alias definitions may
also appear in JSON files without spaces, etc.
Scan alias definitions and remove leading zeroes, spaces,
newlines, etc and rebuild string to make alias->str member
comparable.
s390 for example has terms specified as
event=0x0091 (read from files ..//events/
and terms specified as event=0x91 (read from JSON files).
Signed-off-by: Thomas Richter
---
tools/perf/util/pmu.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 26c79a9c4142..da8f243743d3 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -241,9 +241,11 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
char *metric_expr,
char *metric_name)
{
+ struct parse_events_term *term;
struct perf_pmu_alias *alias;
int ret;
int num;
+ char newval[256];
alias = malloc(sizeof(*alias));
if (!alias)
@@ -262,6 +264,27 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
return ret;
}
+ /* Scan event and remove leading zeroes, spaces, newlines, some
+* platforms have terms specified as
+* event=0x0091 (read from files ..//events/
+* and terms specified as event=0x91 (read from JSON files).
+*
+* Rebuild string to make alias->str member comparable.
+*/
+ memset(newval, 0, sizeof(newval));
+ ret = 0;
+ list_for_each_entry(term, >terms, list) {
+ if (ret)
+ ret += scnprintf(newval + ret, sizeof(newval) - ret,
+",");
+ if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+ ret += scnprintf(newval + ret, sizeof(newval) - ret,
+"%s=%#x", term->config, term->val.num);
+ else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
+ ret += scnprintf(newval + ret, sizeof(newval) - ret,
+"%s=%s", term->config, term->val.str);
+ }
+
alias->name = strdup(name);
if (dir) {
/*
@@ -285,7 +308,7 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
}
alias->per_pkg = perpkg && sscanf(perpkg, "%d", ) == 1 && num == 1;
- alias->str = strdup(val);
+ alias->str = strdup(newval);
list_add_tail(>list, list);
--
2.14.3
[PATCH 1/3] perf alias: Remove trailing newline when reading sysfs files
Remove a trailing newline when reading sysfs file contents
such as /sys/devices/cpum_cf/events/TX_NC_TEND.
This show when verbose option -v is used.
Output before:
tx_nc_tend -> 'cpum_cf'/'event=0x008d
'/
Output after:
tx_nc_tend -> 'cpum_cf'/'event=0x8d'/
Signed-off-by: Thomas Richter
---
tools/perf/util/pmu.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 7878934ebb23..26c79a9c4142 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -294,7 +294,7 @@ static int __perf_pmu__new_alias(struct list_head *list,
char *dir, char *name,
static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
FILE *file)
{
- char buf[256];
+ char *cp, buf[256];
int ret;
ret = fread(buf, 1, sizeof(buf), file);
@@ -303,6 +303,11 @@ static int perf_pmu__new_alias(struct list_head *list,
char *dir, char *name, FI
buf[ret] = 0;
+ /* Remove trailing newline from sysfs file */
+ cp = strrchr(buf, '\n');
+ if (cp)
+ *cp = '\0';
+
return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL,
NULL,
NULL, NULL, NULL);
}
--
2.14.3
[PATCH 3/3] perf stat: Remove duplicate event counting
Perf stat shows a mismatch in perf stat regarding counter names
on s390:
Run command
[root@s35lp76 perf]# ./perf stat -e tx_nc_tend -v --
~/mytesttx 1 >/tmp/111
tx_nc_tend: 1 573146 573146
tx_nc_tend: 1 573146 573146
Performance counter stats for '/root/mytesttx 1':
3 tx_nc_tend
0.001037252 seconds time elapsed
[root@s35lp76 perf]#
shows transaction counter tx_nc_tend with value 3 but it
was triggered only once as seen by the output of mytesttx.
When looking up the event name tx_nc_tend the following
function sequence is called:
parse_events_multi_pmu_add()
+--> perf_pmu__scan() being called with NULL argument
+--> pmu_read_sysfs() scans directory ../devices/ for
all PMUs
+--> perf_pmu__find() tries to find a PMU in the
global pmu list.
+--> pmu_lookup() called to read all file
entries when not in global
list.
pmu_lookup() causes the issue. It calls
+---> pmu_aliases() to read all the entries in the PMU directory.
On s390 this is named
/sys/devices/cpum_cf/events.
+--> pmu_aliases_parse() reads all files and creates an
alias for each file name.
So we end up with first entry created by
reading the sysfs file
[root@s35lp76 perf]# cat /sys/devices/cpum_cf
/events/TX_NC_TEND
event=0x008d
[root@s35lp76 perf]#
Debug output shows this entry
tx_nc_tend -> 'cpum_cf'/'event=0x008d
'/
After all files in this directory have been
read and aliases created this function is called:
+--> pmu_add_cpu_aliases()
This function looks up the CPU tables
created by the json files.
With json files for s390 now available all
the aliases are added to
the PMU alias list a second time.
The second entry is added by
reading the json file converted by jevent
resulting in file pmu-events/pmu-events.c:
{
.name = "tx_nc_tend",
.event = "event=0x8d",
.desc = "Unit: cpum_cf Completed TEND \
instructions \
in non-constrained TX mode",
.topic = "extended",
.long_desc = "A TEND instruction has \
completed in a \
non-constrained \
transactional-execution mode",
.pmu = "cpum_cf",
},
Debug output shows this entry
tx_nc_tend -> 'cpum_cf'/'event=0x8d'/
Function pmu_aliases_parse() and pmu_add_cpu_aliases()
both use __perf_pmu__new_alias() to add an alias to the
PMU alias list. There is no check if an alias already exist
So we end up with 2 entries for tx_nc_tend in the
PMU alias list.
Having set up the PMU alias list for this PMU now
parse_events_multi_add_pmu() reads the complete alias
list and adds each alias with parse_events_add_pmu()
to the global perfev_list. This causes the alias to
be added multiple times to the event list.
Fix this by making __perf_pmu__new_alias()
to merge alias definitions if an alias is already on the
alias list.
Also print a debug message when the alias has mismatches
in some fields.
Output before:
[root@s35lp76 perf]# ./perf stat -e tx_nc_tend -v \
-- ~/mytesttx 1 >/tmp/111
tx_nc_tend: 1 551446 551446
Performance counter stats for '/root/mytesttx 1':
3 tx_nc_tend
0.000961134 seconds time elapsed
[root@s35lp76 perf]#
Output after:
[root@s35lp76 perf]# ./perf stat -e tx_nc_tend -v \
-- ~/mytesttx 1 >/tmp/111
tx_nc_tend: 1 551446 551446
Performance counter stats for '/root/mytesttx 1':
1 tx_nc_tend
0.000961134 seconds time elapsed
[root@s35lp76 perf]#
Signed-off-by: Thomas Richter
---
tools/perf/util/pmu.c | 71 ++-
1 file changed, 70 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index da8f243743d3..a266da1db139 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -234,6 +234,74 @@ static int perf_pmu__parse_snapshot(struct perf_pmu_alias
*alias,
return 0;
}
+static void perf_pmu_assign_str(char *name, const char
[PATCH 3/3] perf stat: Remove duplicate event counting
Perf stat shows a mismatch in perf stat regarding counter names
on s390:
Run command
[root@s35lp76 perf]# ./perf stat -e tx_nc_tend -v --
~/mytesttx 1 >/tmp/111
tx_nc_tend: 1 573146 573146
tx_nc_tend: 1 573146 573146
Performance counter stats for '/root/mytesttx 1':
3 tx_nc_tend
0.001037252 seconds time elapsed
[root@s35lp76 perf]#
shows transaction counter tx_nc_tend with value 3 but it
was triggered only once as seen by the output of mytesttx.
When looking up the event name tx_nc_tend the following
function sequence is called:
parse_events_multi_pmu_add()
+--> perf_pmu__scan() being called with NULL argument
+--> pmu_read_sysfs() scans directory ../devices/ for
all PMUs
+--> perf_pmu__find() tries to find a PMU in the
global pmu list.
+--> pmu_lookup() called to read all file
entries when not in global
list.
pmu_lookup() causes the issue. It calls
+---> pmu_aliases() to read all the entries in the PMU directory.
On s390 this is named
/sys/devices/cpum_cf/events.
+--> pmu_aliases_parse() reads all files and creates an
alias for each file name.
So we end up with first entry created by
reading the sysfs file
[root@s35lp76 perf]# cat /sys/devices/cpum_cf
/events/TX_NC_TEND
event=0x008d
[root@s35lp76 perf]#
Debug output shows this entry
tx_nc_tend -> 'cpum_cf'/'event=0x008d
'/
After all files in this directory have been
read and aliases created this function is called:
+--> pmu_add_cpu_aliases()
This function looks up the CPU tables
created by the json files.
With json files for s390 now available all
the aliases are added to
the PMU alias list a second time.
The second entry is added by
reading the json file converted by jevent
resulting in file pmu-events/pmu-events.c:
{
.name = "tx_nc_tend",
.event = "event=0x8d",
.desc = "Unit: cpum_cf Completed TEND \
instructions \
in non-constrained TX mode",
.topic = "extended",
.long_desc = "A TEND instruction has \
completed in a \
non-constrained \
transactional-execution mode",
.pmu = "cpum_cf",
},
Debug output shows this entry
tx_nc_tend -> 'cpum_cf'/'event=0x8d'/
Function pmu_aliases_parse() and pmu_add_cpu_aliases()
both use __perf_pmu__new_alias() to add an alias to the
PMU alias list. There is no check if an alias already exist
So we end up with 2 entries for tx_nc_tend in the
PMU alias list.
Having set up the PMU alias list for this PMU now
parse_events_multi_add_pmu() reads the complete alias
list and adds each alias with parse_events_add_pmu()
to the global perfev_list. This causes the alias to
be added multiple times to the event list.
Fix this by making __perf_pmu__new_alias()
to merge alias definitions if an alias is already on the
alias list.
Also print a debug message when the alias has mismatches
in some fields.
Output before:
[root@s35lp76 perf]# ./perf stat -e tx_nc_tend -v \
-- ~/mytesttx 1 >/tmp/111
tx_nc_tend: 1 551446 551446
Performance counter stats for '/root/mytesttx 1':
3 tx_nc_tend
0.000961134 seconds time elapsed
[root@s35lp76 perf]#
Output after:
[root@s35lp76 perf]# ./perf stat -e tx_nc_tend -v \
-- ~/mytesttx 1 >/tmp/111
tx_nc_tend: 1 551446 551446
Performance counter stats for '/root/mytesttx 1':
1 tx_nc_tend
0.000961134 seconds time elapsed
[root@s35lp76 perf]#
Signed-off-by: Thomas Richter
---
tools/perf/util/pmu.c | 71 ++-
1 file changed, 70 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index da8f243743d3..a266da1db139 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -234,6 +234,74 @@ static int perf_pmu__parse_snapshot(struct perf_pmu_alias
*alias,
return 0;
}
+static void perf_pmu_assign_str(char *name, const char
Re: [PATCH 2/4] arm: dts: add support for Laird WB50N cpu module and DVK
On 14/06/2018 at 11:50, Alexandre Belloni wrote:
On 14/06/2018 09:51:55+0100, Ben Whitten wrote:
Signed-off-by: Ben Whitten
---
arch/arm/boot/dts/Makefile| 3 +-
arch/arm/boot/dts/at91-wb50n.dts | 116 ++
arch/arm/boot/dts/at91-wb50n.dtsi | 202 ++
3 files changed, 320 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/at91-wb50n.dts
create mode 100644 arch/arm/boot/dts/at91-wb50n.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 1ee94ee..fd5f8a6 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -61,7 +61,8 @@ dtb-$(CONFIG_SOC_SAM_V7) += \
at91-sama5d4_ma5d4evk.dtb \
at91-sama5d4_xplained.dtb \
at91-sama5d4ek.dtb \
- at91-vinco.dtb
+ at91-vinco.dtb \
+ at91-wb50n.dtb
I know we have been bad at this but this should be
at91--.dtb so at91-sama5d31-wb50n.dtb
See new message by Alexandre.
Actually, the current convention is explained here:
https://elixir.bootlin.com/linux/latest/source/Documentation/arm/Microchip/README#L159
dtb-$(CONFIG_ARCH_ATLAS6) += \
atlas6-evb.dtb
dtb-$(CONFIG_ARCH_ATLAS7) += \
diff --git a/arch/arm/boot/dts/at91-wb50n.dts b/arch/arm/boot/dts/at91-wb50n.dts
new file mode 100644
index 000..ee4f823
--- /dev/null
+++ b/arch/arm/boot/dts/at91-wb50n.dts
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-wb50n.dts - Device Tree file for wb50n evaluation board
+ *
+ * Copyright (C) 2018 Laird
+ *
+ */
+
+/dts-v1/;
+#include "at91-wb50n.dtsi"
+
+/ {
+ model = "Laird Workgroup Bridge 50N - Atmel SAMA5D";
+ compatible = "laird,wb50n", "atmel,sama5d31", "atmel,sama5d3",
"atmel,sama5";
+
+ ahb {
+ apb {
+ watchdog@fe40 {
I don't mind if you want to have a preparation patch adding the
necessary labels in the soc dtsi so you don't have to reproduce the
ahb/apb hierarchy here.
I agree: +1
+ ahb {
+ apb {
+ pinctrl@f200 {
Ditto
+ board {
+ pinctrl_mmc0_cd: mmc0_cd {
+ atmel,pins = ; /* PC26 GPIO with pullup deglitch
*/
+ };
+
+ pinctrl_usba_vbus: usba_vbus {
+ atmel,pins = ; /* PB13 GPIO with deglitch */
+ };
+ };
+ };
+ };
+ };
+};
+
+_osc {
+ atmel,osc-bypass;
+};
After the clock binding rework, this will have to be moved to the pmc
node (the rework is not posted, this is just to remind me that this will
have to be done).
+
+_clk {
+ atmel,clk-output-range = <0 13200>;
+};
The datasheet explicitly states that 66 MHz is the maximum allowed
frequency for the USART. Note that the new binding will not allow you to
do that.
However, I see the table disappeared from the latest datasheet. Maybe
Nicolas can comment on that?
You're right, 66 MHz is the maximum frequency for all USART and UART on
this sama5d3 SoC.
The disappearing of this table is a bug in the latest datasheet. I can
see that the one "11121B–ATARM–08-Mar-13" still have it. I report this
issue to the team in charge of datasheets (it will be certainly fixed
for next release of this document).
Best regards,
--
Nicolas Ferre
Re: [PATCH 2/4] arm: dts: add support for Laird WB50N cpu module and DVK
On 14/06/2018 at 11:50, Alexandre Belloni wrote:
On 14/06/2018 09:51:55+0100, Ben Whitten wrote:
Signed-off-by: Ben Whitten
---
arch/arm/boot/dts/Makefile| 3 +-
arch/arm/boot/dts/at91-wb50n.dts | 116 ++
arch/arm/boot/dts/at91-wb50n.dtsi | 202 ++
3 files changed, 320 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/at91-wb50n.dts
create mode 100644 arch/arm/boot/dts/at91-wb50n.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 1ee94ee..fd5f8a6 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -61,7 +61,8 @@ dtb-$(CONFIG_SOC_SAM_V7) += \
at91-sama5d4_ma5d4evk.dtb \
at91-sama5d4_xplained.dtb \
at91-sama5d4ek.dtb \
- at91-vinco.dtb
+ at91-vinco.dtb \
+ at91-wb50n.dtb
I know we have been bad at this but this should be
at91--.dtb so at91-sama5d31-wb50n.dtb
See new message by Alexandre.
Actually, the current convention is explained here:
https://elixir.bootlin.com/linux/latest/source/Documentation/arm/Microchip/README#L159
dtb-$(CONFIG_ARCH_ATLAS6) += \
atlas6-evb.dtb
dtb-$(CONFIG_ARCH_ATLAS7) += \
diff --git a/arch/arm/boot/dts/at91-wb50n.dts b/arch/arm/boot/dts/at91-wb50n.dts
new file mode 100644
index 000..ee4f823
--- /dev/null
+++ b/arch/arm/boot/dts/at91-wb50n.dts
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * at91-wb50n.dts - Device Tree file for wb50n evaluation board
+ *
+ * Copyright (C) 2018 Laird
+ *
+ */
+
+/dts-v1/;
+#include "at91-wb50n.dtsi"
+
+/ {
+ model = "Laird Workgroup Bridge 50N - Atmel SAMA5D";
+ compatible = "laird,wb50n", "atmel,sama5d31", "atmel,sama5d3",
"atmel,sama5";
+
+ ahb {
+ apb {
+ watchdog@fe40 {
I don't mind if you want to have a preparation patch adding the
necessary labels in the soc dtsi so you don't have to reproduce the
ahb/apb hierarchy here.
I agree: +1
+ ahb {
+ apb {
+ pinctrl@f200 {
Ditto
+ board {
+ pinctrl_mmc0_cd: mmc0_cd {
+ atmel,pins = ; /* PC26 GPIO with pullup deglitch
*/
+ };
+
+ pinctrl_usba_vbus: usba_vbus {
+ atmel,pins = ; /* PB13 GPIO with deglitch */
+ };
+ };
+ };
+ };
+ };
+};
+
+_osc {
+ atmel,osc-bypass;
+};
After the clock binding rework, this will have to be moved to the pmc
node (the rework is not posted, this is just to remind me that this will
have to be done).
+
+_clk {
+ atmel,clk-output-range = <0 13200>;
+};
The datasheet explicitly states that 66 MHz is the maximum allowed
frequency for the USART. Note that the new binding will not allow you to
do that.
However, I see the table disappeared from the latest datasheet. Maybe
Nicolas can comment on that?
You're right, 66 MHz is the maximum frequency for all USART and UART on
this sama5d3 SoC.
The disappearing of this table is a bug in the latest datasheet. I can
see that the one "11121B–ATARM–08-Mar-13" still have it. I report this
issue to the team in charge of datasheets (it will be certainly fixed
for next release of this document).
Best regards,
--
Nicolas Ferre
[PATCH v3] ALSA: sonicvibes: add error handling for snd_ctl_add
When snd_ctl_add fails, the lack of error-handling code may
cause unexpected results.
This patch adds error-handling code after calling snd_ctl_add.
Signed-off-by: Zhouyang Jia
---
v1->v2:
- Check the return value of snd_sonicvibes_create_gameport.
v2->v3:
- Fix the code style.
---
sound/pci/sonicvibes.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
index a8abb15..7fbdb70 100644
--- a/sound/pci/sonicvibes.c
+++ b/sound/pci/sonicvibes.c
@@ -1188,6 +1188,7 @@ SONICVIBES_SINGLE("Joystick Speed", 0, SV_IREG_GAME_PORT,
1, 15, 0);
static int snd_sonicvibes_create_gameport(struct sonicvibes *sonic)
{
struct gameport *gp;
+ int err;
sonic->gameport = gp = gameport_allocate_port();
if (!gp) {
@@ -1203,7 +1204,10 @@ static int snd_sonicvibes_create_gameport(struct
sonicvibes *sonic)
gameport_register_port(gp);
- snd_ctl_add(sonic->card, snd_ctl_new1(_sonicvibes_game_control,
sonic));
+ err = snd_ctl_add(sonic->card,
+ snd_ctl_new1(_sonicvibes_game_control, sonic));
+ if (err < 0)
+ return err;
return 0;
}
@@ -1515,7 +1519,11 @@ static int snd_sonic_probe(struct pci_dev *pci,
return err;
}
- snd_sonicvibes_create_gameport(sonic);
+ err = snd_sonicvibes_create_gameport(sonic);
+ if (err < 0) {
+ snd_card_free(card);
+ return err;
+ }
if ((err = snd_card_register(card)) < 0) {
snd_card_free(card);
--
2.7.4
[PATCH v3] ALSA: sonicvibes: add error handling for snd_ctl_add
When snd_ctl_add fails, the lack of error-handling code may
cause unexpected results.
This patch adds error-handling code after calling snd_ctl_add.
Signed-off-by: Zhouyang Jia
---
v1->v2:
- Check the return value of snd_sonicvibes_create_gameport.
v2->v3:
- Fix the code style.
---
sound/pci/sonicvibes.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
index a8abb15..7fbdb70 100644
--- a/sound/pci/sonicvibes.c
+++ b/sound/pci/sonicvibes.c
@@ -1188,6 +1188,7 @@ SONICVIBES_SINGLE("Joystick Speed", 0, SV_IREG_GAME_PORT,
1, 15, 0);
static int snd_sonicvibes_create_gameport(struct sonicvibes *sonic)
{
struct gameport *gp;
+ int err;
sonic->gameport = gp = gameport_allocate_port();
if (!gp) {
@@ -1203,7 +1204,10 @@ static int snd_sonicvibes_create_gameport(struct
sonicvibes *sonic)
gameport_register_port(gp);
- snd_ctl_add(sonic->card, snd_ctl_new1(_sonicvibes_game_control,
sonic));
+ err = snd_ctl_add(sonic->card,
+ snd_ctl_new1(_sonicvibes_game_control, sonic));
+ if (err < 0)
+ return err;
return 0;
}
@@ -1515,7 +1519,11 @@ static int snd_sonic_probe(struct pci_dev *pci,
return err;
}
- snd_sonicvibes_create_gameport(sonic);
+ err = snd_sonicvibes_create_gameport(sonic);
+ if (err < 0) {
+ snd_card_free(card);
+ return err;
+ }
if ((err = snd_card_register(card)) < 0) {
snd_card_free(card);
--
2.7.4
Re: [PATCH v4 2/2] gpio: davinci: Do not assume continuous IRQ numbering
On Wed, Jun 13, 2018 at 5:40 AM, Keerthy wrote: > Currently the driver assumes that the interrupts are continuous > and does platform_get_irq only once and assumes the rest are continuous, > instead call platform_get_irq for all the interrupts and store them > in an array for later use. > > Signed-off-by: Keerthy This v4 applied with Grygorii's ACK. Yours, Linus Walleij
Re: [PATCH v4 2/2] gpio: davinci: Do not assume continuous IRQ numbering
On Wed, Jun 13, 2018 at 5:40 AM, Keerthy wrote: > Currently the driver assumes that the interrupts are continuous > and does platform_get_irq only once and assumes the rest are continuous, > instead call platform_get_irq for all the interrupts and store them > in an array for later use. > > Signed-off-by: Keerthy This v4 applied with Grygorii's ACK. Yours, Linus Walleij
Re: [PATCH v2 0/3] mtd: rawnand: denali: add new clocks and improve setup_data_interface
On Thu, 14 Jun 2018 20:31:59 +0900 Masahiro Yamada wrote: > Hi Boris, > > 2018-06-14 16:38 GMT+09:00 Boris Brezillon : > > On Thu, 14 Jun 2018 16:29:59 +0900 > > Masahiro Yamada wrote: > > > >> Hi Richard, > >> > >> 2018-06-14 16:25 GMT+09:00 Richard Weinberger : > >> > Masahiro, > >> > > >> > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada: > >> >> > >> >> The ->setup_data_interface() hook needs to know the clock frequency. > >> >> In fact, this IP needs three clocks, bus "which clock?" is really > >> >> confusing. (It is not described in the DT-binding at all.) > >> >> > >> >> This series adds more clocks. In the new binding, three clocks > >> >> are required: core clock, bus interface clock, ECC engine clock. > >> >> > >> >> This series also takes care of the backward compatibility by > >> >> providing hardcoded values in case the new clocks are missing. > >> >> So, existing DT should work. > >> >> > >> >> > >> >> Changes in v2: > >> >> - Split patches into sensible chunks > >> >> > >> >> Masahiro Yamada (3): > >> >> mtd: rawnand: denali_dt: use dev as a shorthand of >dev > >> >> mtd: rawnand: denali_dt: add more clocks based on IP datasheet > >> >> mtd: rawnand: denali: optimize timing parameters for data interface > >> > > >> > This still means that we need to feed at least 2/3 and 3/3 into -stable > >> > to > >> > unbreak the driver. > >> > >> > >> 3/3 is not mandatory. > >> > >> You can only back-port 1/3 and 2/3. > > > > Well, patch 1 is not a fix, can we move it after patch 2 so that only > > patch 2 is flagged with the Fixes+Cc-stable tags? > > > OK, will do that. > > If you try to port this back to v4.14.* > you need to fix-up the file path > since the driver did not reside in the raw/ sub-directory at that time. Yes I know :-/. We'll do that when we receive GKH's notification saying that the patch does not apply.
Re: [PATCH v2 0/3] mtd: rawnand: denali: add new clocks and improve setup_data_interface
On Thu, 14 Jun 2018 20:31:59 +0900 Masahiro Yamada wrote: > Hi Boris, > > 2018-06-14 16:38 GMT+09:00 Boris Brezillon : > > On Thu, 14 Jun 2018 16:29:59 +0900 > > Masahiro Yamada wrote: > > > >> Hi Richard, > >> > >> 2018-06-14 16:25 GMT+09:00 Richard Weinberger : > >> > Masahiro, > >> > > >> > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada: > >> >> > >> >> The ->setup_data_interface() hook needs to know the clock frequency. > >> >> In fact, this IP needs three clocks, bus "which clock?" is really > >> >> confusing. (It is not described in the DT-binding at all.) > >> >> > >> >> This series adds more clocks. In the new binding, three clocks > >> >> are required: core clock, bus interface clock, ECC engine clock. > >> >> > >> >> This series also takes care of the backward compatibility by > >> >> providing hardcoded values in case the new clocks are missing. > >> >> So, existing DT should work. > >> >> > >> >> > >> >> Changes in v2: > >> >> - Split patches into sensible chunks > >> >> > >> >> Masahiro Yamada (3): > >> >> mtd: rawnand: denali_dt: use dev as a shorthand of >dev > >> >> mtd: rawnand: denali_dt: add more clocks based on IP datasheet > >> >> mtd: rawnand: denali: optimize timing parameters for data interface > >> > > >> > This still means that we need to feed at least 2/3 and 3/3 into -stable > >> > to > >> > unbreak the driver. > >> > >> > >> 3/3 is not mandatory. > >> > >> You can only back-port 1/3 and 2/3. > > > > Well, patch 1 is not a fix, can we move it after patch 2 so that only > > patch 2 is flagged with the Fixes+Cc-stable tags? > > > OK, will do that. > > If you try to port this back to v4.14.* > you need to fix-up the file path > since the driver did not reside in the raw/ sub-directory at that time. Yes I know :-/. We'll do that when we receive GKH's notification saying that the patch does not apply.
Re: [PATCH v2 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
On Thu, Jun 14, 2018 at 09:29:21AM +0200, Thomas Hellstrom wrote:
> __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx
> *ww_ctx)
> {
> struct mutex_waiter *cur;
> + unsigned int is_wait_die = ww_ctx->ww_class->is_wait_die;
>
> lockdep_assert_held(>wait_lock);
>
> @@ -310,13 +348,14 @@ __ww_mutex_wakeup_for_backoff(struct mutex *lock,
> struct ww_acquire_ctx *ww_ctx)
> if (!cur->ww_ctx)
> continue;
>
> - if (cur->ww_ctx->acquired > 0 &&
> + if (is_wait_die && cur->ww_ctx->acquired > 0 &&
> __ww_ctx_stamp_after(cur->ww_ctx, ww_ctx)) {
> debug_mutex_wake_waiter(lock, cur);
> wake_up_process(cur->task);
> }
>
> - break;
> + if (is_wait_die || __ww_mutex_wound(lock, cur->ww_ctx, ww_ctx))
> + break;
> }
> }
I ended up with:
static void __sched
__ww_mutex_check_waiters(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
{
bool is_wait_die = ww_ctx->ww_class->is_wait_die;
struct mutex_waiter *cur;
lockdep_assert_held(>wait_lock);
list_for_each_entry(cur, >wait_list, list) {
if (!cur->ww_ctx)
continue;
if (is_wait_die) {
/*
* Because __ww_mutex_add_waiter() and
* __ww_mutex_check_stamp() wake any but the earliest
* context, this can only affect the first waiter (with
* a context).
*/
if (cur->ww_ctx->acquired > 0 &&
__ww_ctx_stamp_after(cur->ww_ctx, ww_ctx)) {
debug_mutex_wake_waiter(lock, cur);
wake_up_process(cur->task);
}
break;
}
if (__ww_mutex_wound(lock, cur->ww_ctx, ww_ctx))
break;
}
}
Currently you don't allow mixing WD and WW contexts (which is not
immediately obvious from the above code), and the above hard relies on
that. Are there sensible use cases for mixing them? IOW will your
current restriction stand without hassle?
Re: [PATCH v2 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
On Thu, Jun 14, 2018 at 09:29:21AM +0200, Thomas Hellstrom wrote:
> __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx
> *ww_ctx)
> {
> struct mutex_waiter *cur;
> + unsigned int is_wait_die = ww_ctx->ww_class->is_wait_die;
>
> lockdep_assert_held(>wait_lock);
>
> @@ -310,13 +348,14 @@ __ww_mutex_wakeup_for_backoff(struct mutex *lock,
> struct ww_acquire_ctx *ww_ctx)
> if (!cur->ww_ctx)
> continue;
>
> - if (cur->ww_ctx->acquired > 0 &&
> + if (is_wait_die && cur->ww_ctx->acquired > 0 &&
> __ww_ctx_stamp_after(cur->ww_ctx, ww_ctx)) {
> debug_mutex_wake_waiter(lock, cur);
> wake_up_process(cur->task);
> }
>
> - break;
> + if (is_wait_die || __ww_mutex_wound(lock, cur->ww_ctx, ww_ctx))
> + break;
> }
> }
I ended up with:
static void __sched
__ww_mutex_check_waiters(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
{
bool is_wait_die = ww_ctx->ww_class->is_wait_die;
struct mutex_waiter *cur;
lockdep_assert_held(>wait_lock);
list_for_each_entry(cur, >wait_list, list) {
if (!cur->ww_ctx)
continue;
if (is_wait_die) {
/*
* Because __ww_mutex_add_waiter() and
* __ww_mutex_check_stamp() wake any but the earliest
* context, this can only affect the first waiter (with
* a context).
*/
if (cur->ww_ctx->acquired > 0 &&
__ww_ctx_stamp_after(cur->ww_ctx, ww_ctx)) {
debug_mutex_wake_waiter(lock, cur);
wake_up_process(cur->task);
}
break;
}
if (__ww_mutex_wound(lock, cur->ww_ctx, ww_ctx))
break;
}
}
Currently you don't allow mixing WD and WW contexts (which is not
immediately obvious from the above code), and the above hard relies on
that. Are there sensible use cases for mixing them? IOW will your
current restriction stand without hassle?
Re: [PATCH v4 1/2] gpio: davinci: Shuffle IRQ resource fetching from DT to beginning of probe
On Wed, Jun 13, 2018 at 5:40 AM, Keerthy wrote: > This is needed in case of PROBE_DEFER if IRQ resource is not yet ready. > > Signed-off-by: Keerthy Took out the previous version and applied this v4 instead. Yours, Linus Walleij
Re: [PATCH v4 1/2] gpio: davinci: Shuffle IRQ resource fetching from DT to beginning of probe
On Wed, Jun 13, 2018 at 5:40 AM, Keerthy wrote: > This is needed in case of PROBE_DEFER if IRQ resource is not yet ready. > > Signed-off-by: Keerthy Took out the previous version and applied this v4 instead. Yours, Linus Walleij
Re: [LTP] [PATCH 4.4 00/24] 4.4.137-stable review
On Thu, Jun 14, 2018 at 06:36:03AM -0400, Jan Stancek wrote:
>
> - Original Message -
> > On Thu, Jun 14, 2018 at 05:49:52AM -0400, Jan Stancek wrote:
> > >
> > > - Original Message -
> > > > On Thu, Jun 14, 2018 at 02:24:25PM +0530, Naresh Kamboju wrote:
> > > > > On 14 June 2018 at 12:04, Greg Kroah-Hartman
> > > > >
> > > > > wrote:
> > > > > > On Wed, Jun 13, 2018 at 10:48:50PM -0300, Rafael Tinoco wrote:
> > > > > >> On 13 June 2018 at 18:08, Rafael David Tinoco
> > > > > >> wrote:
> > > > > >> > On Wed, Jun 13, 2018 at 6:00 PM, Greg Kroah-Hartman
> > > > > >> > wrote:
> > > > > >> >> On Wed, Jun 13, 2018 at 05:47:49PM -0300, Rafael Tinoco wrote:
> > > > > >> >>> Results from Linaro’s test farm.
> > > > > >> >>> Regressions detected.
> > > > > >> >>>
> > > > > >> >>> NOTE:
> > > > > >> >>>
> > > > > >> >>> 1) LTP vma03 test (cve-2011-2496) broken on v4.4-137-rc1
> > > > > >> >>> because
> > > > > >> >>> of:
> > > > > >> >>>
> > > > > >> >>> 6ea1dc96a03a mmap: relax file size limit for regular files
> > > > > >> >>> bd2f9ce5bacb mmap: introduce sane default mmap limits
> > > > > >> >>>
> > > > > >> >>>discussion:
> > > > > >> >>>
> > > > > >> >>> https://github.com/linux-test-project/ltp/issues/341
> > > > > >> >>>
> > > > > >> >>>mainline commit (v4.13-rc7):
> > > > > >> >>>
> > > > > >> >>> 0cc3b0ec23ce Clarify (and fix) MAX_LFS_FILESIZE macros
> > > > > >> >>>
> > > > > >> >>>should be backported to 4.4.138-rc2 and fixes the issue.
> > > > > >> >>
> > > > > >> >> Really? That commit says it fixes c2a9737f45e2 ("vfs,mm: fix a
> > > > > >> >> dead
> > > > > >> >> loop in truncate_inode_pages_range()") which is not in 4.4.y at
> > > > > >> >> all.
> > > > > >> >>
> > > > > >> >> Did you test this out?
> > > > > >> >
> > > > > >> > Yes, the LTP contains the tests (last comment is the final test
> > > > > >> > for
> > > > > >> > arm32, right before Jan tests i686).
> > > > > >> >
> > > > > >> > Fixing MAX_LFS_FILESIZE fixes the new limit for mmap() brought by
> > > > > >> > those 2 commits (file_mmap_size_max()).
> > > > > >> > offset tested by the LTP test is 0xfffe000.
> > > > > >> > file_mmap_size_max gives: 0x000 as max value, but only
> > > > > >> > after
> > > > > >> > the mentioned patch.
> > > > > >> >
> > > > > >> > Original intent for this fix was other though.
> > > > > >>
> > > > > >> To clarify this a bit further.
> > > > > >>
> > > > > >> The LTP CVE test is breaking in the first call to mmap(), even
> > > > > >> before
> > > > > >> trying to remap and test the security issue. That start happening
> > > > > >> in
> > > > > >> this round because of those mmap() changes and the offset used in
> > > > > >> the
> > > > > >> LTP test. Linus changed limit checks and made them to be related to
> > > > > >> MAX_LFS_FILESIZE. Unfortunately, in 4.4 stable, we were missing the
> > > > > >> fix for MAX_LFS_FILESIZE (which before commit 0cc3b0ec23ce was less
> > > > > >> than the REAL 32 bit limit).
> > > > > >>
> > > > > >> Commit 0cc3b0ec23ce was made because an user noticed the FS limit
> > > > > >> not
> > > > > >> being what it should be. In our case, the 4.4 stable kernel, we are
> > > > > >> facing this 32 bit lower limit (than the real 32 bit real limit),
> > > > > >> because of the LTP CVE test, so we need this fix to have the real
> > > > > >> 32
> > > > > >> bit limit set for that macro (mmap limits did not use that macro
> > > > > >> before).
> > > > > >>
> > > > > >> I have tested in arm32 and Jan Stancek, who first responded to LTP
> > > > > >> issue, has tested this in i686 and both worked after that patch was
> > > > > >> included to v4.4-137-rc1 (my last test was even with 4.4.138-rc1).
> > > > > >>
> > > > > >> Hope that helps a bit.
> > > > > >
> > > > > > Ok, thanks, it didn't apply cleanly but I've fixed it up now.
> > > > >
> > > > > On the latest 4.4.138-rc1,
> > > > > LTP "cve-2011-2496" test still fails on arm32 beagleboard x15 and
> > > > > qemu_arm.
> > > > >
> > > > > Summary
> > > > >
> > > > > kernel: 4.4.138-rc1
> > > > > git repo:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > > > > git branch: linux-4.4.y
> > > > > git commit: 7d690c56754ef7be647fbcf7bcdceebd59926b3f
> > > > > git describe: v4.4.137-15-g7d690c56754e
> > > > > Test details:
> > > > > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.4-oe/build/v4.4.137-15-g7d690c56754e
> > > >
> > > > Ok, but what does this mean? Is there a commit somewhere that I need to
> > > > pick up for 4.4.y that is already in newer kernels?
> > >
> > > Hi Greg,
> > >
> > > I think the expectations was that:
> > > 0cc3b0ec23ce Clarify (and fix) MAX_LFS_FILESIZE macros
> > > has been included to linux-4.4.y HEAD, so they re-ran the tests.
> > >
> > > Report from Naresh above looks like original report: LTP vma03 is
> > > cve-2011-2496 test.
Re: [LTP] [PATCH 4.4 00/24] 4.4.137-stable review
On Thu, Jun 14, 2018 at 06:36:03AM -0400, Jan Stancek wrote:
>
> - Original Message -
> > On Thu, Jun 14, 2018 at 05:49:52AM -0400, Jan Stancek wrote:
> > >
> > > - Original Message -
> > > > On Thu, Jun 14, 2018 at 02:24:25PM +0530, Naresh Kamboju wrote:
> > > > > On 14 June 2018 at 12:04, Greg Kroah-Hartman
> > > > >
> > > > > wrote:
> > > > > > On Wed, Jun 13, 2018 at 10:48:50PM -0300, Rafael Tinoco wrote:
> > > > > >> On 13 June 2018 at 18:08, Rafael David Tinoco
> > > > > >> wrote:
> > > > > >> > On Wed, Jun 13, 2018 at 6:00 PM, Greg Kroah-Hartman
> > > > > >> > wrote:
> > > > > >> >> On Wed, Jun 13, 2018 at 05:47:49PM -0300, Rafael Tinoco wrote:
> > > > > >> >>> Results from Linaro’s test farm.
> > > > > >> >>> Regressions detected.
> > > > > >> >>>
> > > > > >> >>> NOTE:
> > > > > >> >>>
> > > > > >> >>> 1) LTP vma03 test (cve-2011-2496) broken on v4.4-137-rc1
> > > > > >> >>> because
> > > > > >> >>> of:
> > > > > >> >>>
> > > > > >> >>> 6ea1dc96a03a mmap: relax file size limit for regular files
> > > > > >> >>> bd2f9ce5bacb mmap: introduce sane default mmap limits
> > > > > >> >>>
> > > > > >> >>>discussion:
> > > > > >> >>>
> > > > > >> >>> https://github.com/linux-test-project/ltp/issues/341
> > > > > >> >>>
> > > > > >> >>>mainline commit (v4.13-rc7):
> > > > > >> >>>
> > > > > >> >>> 0cc3b0ec23ce Clarify (and fix) MAX_LFS_FILESIZE macros
> > > > > >> >>>
> > > > > >> >>>should be backported to 4.4.138-rc2 and fixes the issue.
> > > > > >> >>
> > > > > >> >> Really? That commit says it fixes c2a9737f45e2 ("vfs,mm: fix a
> > > > > >> >> dead
> > > > > >> >> loop in truncate_inode_pages_range()") which is not in 4.4.y at
> > > > > >> >> all.
> > > > > >> >>
> > > > > >> >> Did you test this out?
> > > > > >> >
> > > > > >> > Yes, the LTP contains the tests (last comment is the final test
> > > > > >> > for
> > > > > >> > arm32, right before Jan tests i686).
> > > > > >> >
> > > > > >> > Fixing MAX_LFS_FILESIZE fixes the new limit for mmap() brought by
> > > > > >> > those 2 commits (file_mmap_size_max()).
> > > > > >> > offset tested by the LTP test is 0xfffe000.
> > > > > >> > file_mmap_size_max gives: 0x000 as max value, but only
> > > > > >> > after
> > > > > >> > the mentioned patch.
> > > > > >> >
> > > > > >> > Original intent for this fix was other though.
> > > > > >>
> > > > > >> To clarify this a bit further.
> > > > > >>
> > > > > >> The LTP CVE test is breaking in the first call to mmap(), even
> > > > > >> before
> > > > > >> trying to remap and test the security issue. That start happening
> > > > > >> in
> > > > > >> this round because of those mmap() changes and the offset used in
> > > > > >> the
> > > > > >> LTP test. Linus changed limit checks and made them to be related to
> > > > > >> MAX_LFS_FILESIZE. Unfortunately, in 4.4 stable, we were missing the
> > > > > >> fix for MAX_LFS_FILESIZE (which before commit 0cc3b0ec23ce was less
> > > > > >> than the REAL 32 bit limit).
> > > > > >>
> > > > > >> Commit 0cc3b0ec23ce was made because an user noticed the FS limit
> > > > > >> not
> > > > > >> being what it should be. In our case, the 4.4 stable kernel, we are
> > > > > >> facing this 32 bit lower limit (than the real 32 bit real limit),
> > > > > >> because of the LTP CVE test, so we need this fix to have the real
> > > > > >> 32
> > > > > >> bit limit set for that macro (mmap limits did not use that macro
> > > > > >> before).
> > > > > >>
> > > > > >> I have tested in arm32 and Jan Stancek, who first responded to LTP
> > > > > >> issue, has tested this in i686 and both worked after that patch was
> > > > > >> included to v4.4-137-rc1 (my last test was even with 4.4.138-rc1).
> > > > > >>
> > > > > >> Hope that helps a bit.
> > > > > >
> > > > > > Ok, thanks, it didn't apply cleanly but I've fixed it up now.
> > > > >
> > > > > On the latest 4.4.138-rc1,
> > > > > LTP "cve-2011-2496" test still fails on arm32 beagleboard x15 and
> > > > > qemu_arm.
> > > > >
> > > > > Summary
> > > > >
> > > > > kernel: 4.4.138-rc1
> > > > > git repo:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > > > > git branch: linux-4.4.y
> > > > > git commit: 7d690c56754ef7be647fbcf7bcdceebd59926b3f
> > > > > git describe: v4.4.137-15-g7d690c56754e
> > > > > Test details:
> > > > > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.4-oe/build/v4.4.137-15-g7d690c56754e
> > > >
> > > > Ok, but what does this mean? Is there a commit somewhere that I need to
> > > > pick up for 4.4.y that is already in newer kernels?
> > >
> > > Hi Greg,
> > >
> > > I think the expectations was that:
> > > 0cc3b0ec23ce Clarify (and fix) MAX_LFS_FILESIZE macros
> > > has been included to linux-4.4.y HEAD, so they re-ran the tests.
> > >
> > > Report from Naresh above looks like original report: LTP vma03 is
> > > cve-2011-2496 test.
Re: REGRESSION: "wlcore: sdio: allow pm to handle sdio power" breaks wifi on HiKey960
* John Stultz [180613 17:17]:
> On Wed, Jun 13, 2018 at 7:42 AM, Valentin Schneider
> wrote:
> > On 13/06/18 05:13, Tony Lindgren wrote:
> >> * John Stultz [180612 22:15]:
> >>> Hey Folks,
> >>> I noticed with linus/master wifi wasn't coming up on HiKey960. I
> >>> bisected it down and it seems to be due to:
> >>>
> >>> 60f36637bbbd ("wlcore: sdio: allow pm to handle sdio power") and
> >>> 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> >>>
> >>> When wifi fails to load, the only useful error message I see is:
> >>> [8.466097] wl1271_sdio mmc1:0001:2: wl12xx_sdio_power_on: failed
> >>> to get_sync(-13)
> >>
> >> Sorry to hear about that.
> >>
> >>> Reverting those two changes gets wifi working again for me:
> >>> [8.754953] wlcore: wl18xx HW: 183x or 180x, PG 2.2 (ROM 0x11)
> >>> [8.761778] random: crng init done
> >>> [8.765185] random: 7 urandom warning(s) missed due to ratelimiting
> >>> [8.779149] wlcore: loaded
> >>> ...
> >>> [ 12.945903] wlcore: PHY firmware version: Rev 8.2.0.0.237
> >>> [ 13.058077] wlcore: firmware booted (Rev 8.9.0.0.70)
> >>>
> >>>
> >>> Any suggestions how to resolve this w/o a revert?
> >>
> >> Sounds like we need to ignore also -EACCES if runtime PM is
> >> disabled for MMC. Care to try and see if the patch below
> >> helps?
> >>
> >
> > I don't use wifi with my board (I have an USB ethernet adapter), but I do
> > get the same error message as John.
> >
> > Reverting the patches works and I do see wlan0 being brought up. Sadly,
> > applying your patch doesn't seem to fix the issue - and actually it seems
> > to freeze my board on first boot with this error:
> >
> > [ 11.169127] wlcore: ERROR timeout waiting for the hardware to complete
> > initialization
> >
> > On second boot it finally comes to life, but issuing ifconfig freezes it
> > again.
> >
> > $ dmesg | grep wl
> > [5.922661] wl18xx_driver wl18xx.0.auto: Direct firmware load for
> > ti-connectivity/wl18xx-conf.bin failed with error -2
> > [5.933904] wlcore: ERROR could not get configuration binary
> > ti-connectivity/wl18xx-conf.bin: -2
> > [5.949158] wlcore: WARNING falling back to default config
> > [6.199806] wlcore: wl18xx HW: 183x or 180x, PG 2.2 (ROM 0x11)
> > [6.210738] wlcore: WARNING Detected unconfigured mac address in nvs,
> > derive from fuse instead.
> > [6.221644] wlcore: WARNING This default nvs file can be removed from
> > the file system
> > [6.235180] wlcore: loaded
> > [6.820146] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> > [7.280611] wlcore: PHY firmware version: Rev 8.2.0.0.236
> > [7.388339] wlcore: firmware booted (Rev 8.9.0.0.69)
> > [7.409610] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> > [7.417815] wlcore: down
> > [ 10.628867] wlcore: ERROR timeout waiting for the hardware to complete
> > initialization
> >
> >
> > Seems like it's not getting powered on, which might be why those mmc_power_*
> > calls were in there originally ?
>
> Ryan Grachek came up with a different solution. I still need to
> validate it, but it seems promising:
> https://lkml.org/lkml/2018/6/13/481
Oh OK good to hear. Yeah that makes sense to me now.
Regards,
Tony
Re: REGRESSION: "wlcore: sdio: allow pm to handle sdio power" breaks wifi on HiKey960
* John Stultz [180613 17:17]:
> On Wed, Jun 13, 2018 at 7:42 AM, Valentin Schneider
> wrote:
> > On 13/06/18 05:13, Tony Lindgren wrote:
> >> * John Stultz [180612 22:15]:
> >>> Hey Folks,
> >>> I noticed with linus/master wifi wasn't coming up on HiKey960. I
> >>> bisected it down and it seems to be due to:
> >>>
> >>> 60f36637bbbd ("wlcore: sdio: allow pm to handle sdio power") and
> >>> 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> >>>
> >>> When wifi fails to load, the only useful error message I see is:
> >>> [8.466097] wl1271_sdio mmc1:0001:2: wl12xx_sdio_power_on: failed
> >>> to get_sync(-13)
> >>
> >> Sorry to hear about that.
> >>
> >>> Reverting those two changes gets wifi working again for me:
> >>> [8.754953] wlcore: wl18xx HW: 183x or 180x, PG 2.2 (ROM 0x11)
> >>> [8.761778] random: crng init done
> >>> [8.765185] random: 7 urandom warning(s) missed due to ratelimiting
> >>> [8.779149] wlcore: loaded
> >>> ...
> >>> [ 12.945903] wlcore: PHY firmware version: Rev 8.2.0.0.237
> >>> [ 13.058077] wlcore: firmware booted (Rev 8.9.0.0.70)
> >>>
> >>>
> >>> Any suggestions how to resolve this w/o a revert?
> >>
> >> Sounds like we need to ignore also -EACCES if runtime PM is
> >> disabled for MMC. Care to try and see if the patch below
> >> helps?
> >>
> >
> > I don't use wifi with my board (I have an USB ethernet adapter), but I do
> > get the same error message as John.
> >
> > Reverting the patches works and I do see wlan0 being brought up. Sadly,
> > applying your patch doesn't seem to fix the issue - and actually it seems
> > to freeze my board on first boot with this error:
> >
> > [ 11.169127] wlcore: ERROR timeout waiting for the hardware to complete
> > initialization
> >
> > On second boot it finally comes to life, but issuing ifconfig freezes it
> > again.
> >
> > $ dmesg | grep wl
> > [5.922661] wl18xx_driver wl18xx.0.auto: Direct firmware load for
> > ti-connectivity/wl18xx-conf.bin failed with error -2
> > [5.933904] wlcore: ERROR could not get configuration binary
> > ti-connectivity/wl18xx-conf.bin: -2
> > [5.949158] wlcore: WARNING falling back to default config
> > [6.199806] wlcore: wl18xx HW: 183x or 180x, PG 2.2 (ROM 0x11)
> > [6.210738] wlcore: WARNING Detected unconfigured mac address in nvs,
> > derive from fuse instead.
> > [6.221644] wlcore: WARNING This default nvs file can be removed from
> > the file system
> > [6.235180] wlcore: loaded
> > [6.820146] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> > [7.280611] wlcore: PHY firmware version: Rev 8.2.0.0.236
> > [7.388339] wlcore: firmware booted (Rev 8.9.0.0.69)
> > [7.409610] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> > [7.417815] wlcore: down
> > [ 10.628867] wlcore: ERROR timeout waiting for the hardware to complete
> > initialization
> >
> >
> > Seems like it's not getting powered on, which might be why those mmc_power_*
> > calls were in there originally ?
>
> Ryan Grachek came up with a different solution. I still need to
> validate it, but it seems promising:
> https://lkml.org/lkml/2018/6/13/481
Oh OK good to hear. Yeah that makes sense to me now.
Regards,
Tony
Re: [PATCH] extcon: Release locking when sending the notification of connector state
> Am 14.06.2018 um 12:39 schrieb H. Nikolaus Schaller :
>
> Hi Roger and Chanwoo,
>
>> Am 14.06.2018 um 12:18 schrieb Chanwoo Choi :
>>
>> + H. Nikolaus Schaller
>>
>> On 2018년 06월 14일 13:14, Chanwoo Choi wrote:
>>> Previously, extcon used the spinlock before calling the notifier_call_chain
>>> to prevent the scheduled out of task and to prevent the notification delay.
>>> When spinlock is locked for sending the notification, deadlock issue
>>> occured on the side of extcon consumer device. To fix this issue,
>>> extcon consumer device should always use the work. it is always not
>>> reasonable to use work.
>>>
>>> To fix this issue on extcon consumer device, release locking when sending
>>> the notification of connector state.
>>>
>>> Fixes: ab11af049f88 ("extcon: Add the synchronization extcon APIs to
>>> support the notification")
>>> Cc: sta...@vger.kernel.org
>>> Cc: Roger Quadros
>>> Cc: Kishon Vijay Abraham I
>>> Signed-off-by: Chanwoo Choi
>>> ---
>>> drivers/extcon/extcon.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>>> index 8bff5fd18185..f75b08a45d4e 100644
>>> --- a/drivers/extcon/extcon.c
>>> +++ b/drivers/extcon/extcon.c
>>> @@ -433,8 +433,8 @@ int extcon_sync(struct extcon_dev *edev, unsigned int
>>> id)
>>> return index;
>>>
>>> spin_lock_irqsave(>lock, flags);
>>> -
>>> state = !!(edev->state & BIT(index));
>>> + spin_unlock_irqrestore(>lock, flags);
>>>
>>> /*
>>> * Call functions in a raw notifier chain for the specific one
>>> @@ -448,6 +448,7 @@ int extcon_sync(struct extcon_dev *edev, unsigned int
>>> id)
>>> */
>>> raw_notifier_call_chain(>nh_all, state, edev);
>>>
>>> + spin_lock_irqsave(>lock, flags);
>>> /* This could be in interrupt handler */
>>> prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
>>> if (!prop_buf) {
>>>
>
> I have tested on the Pyra handheld prototype and now it works. Plugging in an
> OTG cable
> enables/disables OTG power as expected and there are no kernel oops any more.
I did take some minutes to check and it now also works again on the OMAP5EVM.
BR,
Nikolaus
Re: [PATCH] extcon: Release locking when sending the notification of connector state
> Am 14.06.2018 um 12:39 schrieb H. Nikolaus Schaller :
>
> Hi Roger and Chanwoo,
>
>> Am 14.06.2018 um 12:18 schrieb Chanwoo Choi :
>>
>> + H. Nikolaus Schaller
>>
>> On 2018년 06월 14일 13:14, Chanwoo Choi wrote:
>>> Previously, extcon used the spinlock before calling the notifier_call_chain
>>> to prevent the scheduled out of task and to prevent the notification delay.
>>> When spinlock is locked for sending the notification, deadlock issue
>>> occured on the side of extcon consumer device. To fix this issue,
>>> extcon consumer device should always use the work. it is always not
>>> reasonable to use work.
>>>
>>> To fix this issue on extcon consumer device, release locking when sending
>>> the notification of connector state.
>>>
>>> Fixes: ab11af049f88 ("extcon: Add the synchronization extcon APIs to
>>> support the notification")
>>> Cc: sta...@vger.kernel.org
>>> Cc: Roger Quadros
>>> Cc: Kishon Vijay Abraham I
>>> Signed-off-by: Chanwoo Choi
>>> ---
>>> drivers/extcon/extcon.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>>> index 8bff5fd18185..f75b08a45d4e 100644
>>> --- a/drivers/extcon/extcon.c
>>> +++ b/drivers/extcon/extcon.c
>>> @@ -433,8 +433,8 @@ int extcon_sync(struct extcon_dev *edev, unsigned int
>>> id)
>>> return index;
>>>
>>> spin_lock_irqsave(>lock, flags);
>>> -
>>> state = !!(edev->state & BIT(index));
>>> + spin_unlock_irqrestore(>lock, flags);
>>>
>>> /*
>>> * Call functions in a raw notifier chain for the specific one
>>> @@ -448,6 +448,7 @@ int extcon_sync(struct extcon_dev *edev, unsigned int
>>> id)
>>> */
>>> raw_notifier_call_chain(>nh_all, state, edev);
>>>
>>> + spin_lock_irqsave(>lock, flags);
>>> /* This could be in interrupt handler */
>>> prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
>>> if (!prop_buf) {
>>>
>
> I have tested on the Pyra handheld prototype and now it works. Plugging in an
> OTG cable
> enables/disables OTG power as expected and there are no kernel oops any more.
I did take some minutes to check and it now also works again on the OMAP5EVM.
BR,
Nikolaus
Re: [PATCH v2 0/3] mtd: rawnand: denali: add new clocks and improve setup_data_interface
Hi Boris, 2018-06-14 16:38 GMT+09:00 Boris Brezillon : > On Thu, 14 Jun 2018 16:29:59 +0900 > Masahiro Yamada wrote: > >> Hi Richard, >> >> 2018-06-14 16:25 GMT+09:00 Richard Weinberger : >> > Masahiro, >> > >> > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada: >> >> >> >> The ->setup_data_interface() hook needs to know the clock frequency. >> >> In fact, this IP needs three clocks, bus "which clock?" is really >> >> confusing. (It is not described in the DT-binding at all.) >> >> >> >> This series adds more clocks. In the new binding, three clocks >> >> are required: core clock, bus interface clock, ECC engine clock. >> >> >> >> This series also takes care of the backward compatibility by >> >> providing hardcoded values in case the new clocks are missing. >> >> So, existing DT should work. >> >> >> >> >> >> Changes in v2: >> >> - Split patches into sensible chunks >> >> >> >> Masahiro Yamada (3): >> >> mtd: rawnand: denali_dt: use dev as a shorthand of >dev >> >> mtd: rawnand: denali_dt: add more clocks based on IP datasheet >> >> mtd: rawnand: denali: optimize timing parameters for data interface >> > >> > This still means that we need to feed at least 2/3 and 3/3 into -stable to >> > unbreak the driver. >> >> >> 3/3 is not mandatory. >> >> You can only back-port 1/3 and 2/3. > > Well, patch 1 is not a fix, can we move it after patch 2 so that only > patch 2 is flagged with the Fixes+Cc-stable tags? OK, will do that. If you try to port this back to v4.14.* you need to fix-up the file path since the driver did not reside in the raw/ sub-directory at that time. Except that, I do not see backport issue. > Thanks, > > Boris > > > __ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ -- Best Regards Masahiro Yamada
Re: [PATCH v2 0/3] mtd: rawnand: denali: add new clocks and improve setup_data_interface
Hi Boris, 2018-06-14 16:38 GMT+09:00 Boris Brezillon : > On Thu, 14 Jun 2018 16:29:59 +0900 > Masahiro Yamada wrote: > >> Hi Richard, >> >> 2018-06-14 16:25 GMT+09:00 Richard Weinberger : >> > Masahiro, >> > >> > Am Donnerstag, 14. Juni 2018, 07:11:04 CEST schrieb Masahiro Yamada: >> >> >> >> The ->setup_data_interface() hook needs to know the clock frequency. >> >> In fact, this IP needs three clocks, bus "which clock?" is really >> >> confusing. (It is not described in the DT-binding at all.) >> >> >> >> This series adds more clocks. In the new binding, three clocks >> >> are required: core clock, bus interface clock, ECC engine clock. >> >> >> >> This series also takes care of the backward compatibility by >> >> providing hardcoded values in case the new clocks are missing. >> >> So, existing DT should work. >> >> >> >> >> >> Changes in v2: >> >> - Split patches into sensible chunks >> >> >> >> Masahiro Yamada (3): >> >> mtd: rawnand: denali_dt: use dev as a shorthand of >dev >> >> mtd: rawnand: denali_dt: add more clocks based on IP datasheet >> >> mtd: rawnand: denali: optimize timing parameters for data interface >> > >> > This still means that we need to feed at least 2/3 and 3/3 into -stable to >> > unbreak the driver. >> >> >> 3/3 is not mandatory. >> >> You can only back-port 1/3 and 2/3. > > Well, patch 1 is not a fix, can we move it after patch 2 so that only > patch 2 is flagged with the Fixes+Cc-stable tags? OK, will do that. If you try to port this back to v4.14.* you need to fix-up the file path since the driver did not reside in the raw/ sub-directory at that time. Except that, I do not see backport issue. > Thanks, > > Boris > > > __ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ -- Best Regards Masahiro Yamada
Re: [PATCH] sched/util_est: fix util_est_dequeue() for throttled cfs rq
On 14-Jun 12:33, Vincent Guittot wrote:
> When a cfs_rq is throttled, parent cfs_rq->nr_running is decreased and
> everything happens at cfs_rq level. Currently util_est stays unchanged
> in such case and it keeps accounting the utilization of throttled tasks.
> This can somewhat make sense as we don't dequeue tasks but only throttled
> cfs_rq.
I think the idea here was that, if tasks are throttled, this should
manifest in a reduction of their utilization... and thus the estimated
utilization should still represent the amount of bandwidth required by
that tasks. Although one could argue that, while a TG is throttled we
would like to be able to drop the frequency if possible.
This has not been implemented that way so far because the
attach/detach of TGs will require to walk them to account for all
child tasks's util_est or, otherwise, to aggregate util_est across TGs.
> If a task of another group is enqueued/dequeued and root cfs_rq becomes
> idle during the dequeue, util_est will be cleared whereas it was
> accounting util_est of throttled tasks before.
Yep :/
> So the behavior of util_est
> is not always the same regarding throttled tasks and depends of side
> activity. Furthermore, util_est will not be updated when the cfs_rq is
> unthrottled
right... that happens because (un)throttling does not involve (en/de)queue.
> as everything happens at cfs rq level. Main results is that
> util_est will stay null whereas we now have running tasks. We have to wait
> for the next dequeue/enqueue of the previously throttled tasks to get an
> up to date util_est.
>
> Remove the assumption that cfs_rq's estimated utilization of a CPU is 0
> if there is no running task so the util_est of a task remains until the
> latter is dequeued even if its cfs_rq has been throttled.
Right...
> Fixes: 7f65ea42eb00 ("sched/fair: Add util_est on top of PELT")
> Signed-off-by: Vincent Guittot
> ---
> kernel/sched/fair.c | 16
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e497c05..d3121fc 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3982,18 +3982,10 @@ util_est_dequeue(struct cfs_rq *cfs_rq, struct
> task_struct *p, bool task_sleep)
> if (!sched_feat(UTIL_EST))
> return;
>
> - /*
> - * Update root cfs_rq's estimated utilization
> - *
> - * If *p is the last task then the root cfs_rq's estimated utilization
> - * of a CPU is 0 by definition.
> - */
> - ue.enqueued = 0;
... AFAIR, this reset what there since one of the first posts as an
"optimization". But actually I was not considering the scenario you
describe.
> - if (cfs_rq->nr_running) {
> - ue.enqueued = cfs_rq->avg.util_est.enqueued;
> - ue.enqueued -= min_t(unsigned int, ue.enqueued,
> - (_task_util_est(p) | UTIL_AVG_UNCHANGED));
> - }
> + /* Update root cfs_rq's estimated utilization */
> + ue.enqueued = cfs_rq->avg.util_est.enqueued;
> + ue.enqueued -= min_t(unsigned int, ue.enqueued,
> + (_task_util_est(p) | UTIL_AVG_UNCHANGED));
So, this should still be bound-safe thanks to the min() for the
subtraction.
> WRITE_ONCE(cfs_rq->avg.util_est.enqueued, ue.enqueued);
>
> /*
> --
> 2.7.4
>
LGTM:
Reviewed-by: Patrick Bellasi
--
#include
Patrick Bellasi
Re: [PATCH] sched/util_est: fix util_est_dequeue() for throttled cfs rq
On 14-Jun 12:33, Vincent Guittot wrote:
> When a cfs_rq is throttled, parent cfs_rq->nr_running is decreased and
> everything happens at cfs_rq level. Currently util_est stays unchanged
> in such case and it keeps accounting the utilization of throttled tasks.
> This can somewhat make sense as we don't dequeue tasks but only throttled
> cfs_rq.
I think the idea here was that, if tasks are throttled, this should
manifest in a reduction of their utilization... and thus the estimated
utilization should still represent the amount of bandwidth required by
that tasks. Although one could argue that, while a TG is throttled we
would like to be able to drop the frequency if possible.
This has not been implemented that way so far because the
attach/detach of TGs will require to walk them to account for all
child tasks's util_est or, otherwise, to aggregate util_est across TGs.
> If a task of another group is enqueued/dequeued and root cfs_rq becomes
> idle during the dequeue, util_est will be cleared whereas it was
> accounting util_est of throttled tasks before.
Yep :/
> So the behavior of util_est
> is not always the same regarding throttled tasks and depends of side
> activity. Furthermore, util_est will not be updated when the cfs_rq is
> unthrottled
right... that happens because (un)throttling does not involve (en/de)queue.
> as everything happens at cfs rq level. Main results is that
> util_est will stay null whereas we now have running tasks. We have to wait
> for the next dequeue/enqueue of the previously throttled tasks to get an
> up to date util_est.
>
> Remove the assumption that cfs_rq's estimated utilization of a CPU is 0
> if there is no running task so the util_est of a task remains until the
> latter is dequeued even if its cfs_rq has been throttled.
Right...
> Fixes: 7f65ea42eb00 ("sched/fair: Add util_est on top of PELT")
> Signed-off-by: Vincent Guittot
> ---
> kernel/sched/fair.c | 16
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e497c05..d3121fc 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3982,18 +3982,10 @@ util_est_dequeue(struct cfs_rq *cfs_rq, struct
> task_struct *p, bool task_sleep)
> if (!sched_feat(UTIL_EST))
> return;
>
> - /*
> - * Update root cfs_rq's estimated utilization
> - *
> - * If *p is the last task then the root cfs_rq's estimated utilization
> - * of a CPU is 0 by definition.
> - */
> - ue.enqueued = 0;
... AFAIR, this reset what there since one of the first posts as an
"optimization". But actually I was not considering the scenario you
describe.
> - if (cfs_rq->nr_running) {
> - ue.enqueued = cfs_rq->avg.util_est.enqueued;
> - ue.enqueued -= min_t(unsigned int, ue.enqueued,
> - (_task_util_est(p) | UTIL_AVG_UNCHANGED));
> - }
> + /* Update root cfs_rq's estimated utilization */
> + ue.enqueued = cfs_rq->avg.util_est.enqueued;
> + ue.enqueued -= min_t(unsigned int, ue.enqueued,
> + (_task_util_est(p) | UTIL_AVG_UNCHANGED));
So, this should still be bound-safe thanks to the min() for the
subtraction.
> WRITE_ONCE(cfs_rq->avg.util_est.enqueued, ue.enqueued);
>
> /*
> --
> 2.7.4
>
LGTM:
Reviewed-by: Patrick Bellasi
--
#include
Patrick Bellasi
[PATCH v2] staging: gdm724x: add error handling for nlmsg_put
When nlmsg_put fails, the lack of error-handling code may
cause unexpected results.
This patch adds error-handling code after calling nlmsg_put.
Signed-off-by: Zhouyang Jia
---
v1->v2:
- Add some cleanup
---
drivers/staging/gdm724x/netlink_k.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/gdm724x/netlink_k.c
b/drivers/staging/gdm724x/netlink_k.c
index abe2425..16da03b 100644
--- a/drivers/staging/gdm724x/netlink_k.c
+++ b/drivers/staging/gdm724x/netlink_k.c
@@ -119,6 +119,11 @@ int netlink_send(struct sock *sock, int group, u16 type,
void *msg, int len)
seq++;
nlh = nlmsg_put(skb, 0, seq, type, len, 0);
+ if (!nlh) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
memcpy(NLMSG_DATA(nlh), msg, len);
NETLINK_CB(skb).portid = 0;
NETLINK_CB(skb).dst_group = 0;
--
2.7.4
[PATCH v2] staging: gdm724x: add error handling for nlmsg_put
When nlmsg_put fails, the lack of error-handling code may
cause unexpected results.
This patch adds error-handling code after calling nlmsg_put.
Signed-off-by: Zhouyang Jia
---
v1->v2:
- Add some cleanup
---
drivers/staging/gdm724x/netlink_k.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/gdm724x/netlink_k.c
b/drivers/staging/gdm724x/netlink_k.c
index abe2425..16da03b 100644
--- a/drivers/staging/gdm724x/netlink_k.c
+++ b/drivers/staging/gdm724x/netlink_k.c
@@ -119,6 +119,11 @@ int netlink_send(struct sock *sock, int group, u16 type,
void *msg, int len)
seq++;
nlh = nlmsg_put(skb, 0, seq, type, len, 0);
+ if (!nlh) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
memcpy(NLMSG_DATA(nlh), msg, len);
NETLINK_CB(skb).portid = 0;
NETLINK_CB(skb).dst_group = 0;
--
2.7.4
Re: [PATCH v2] ALSA: sonicvibes: add error handling for snd_ctl_add
On Thu, 14 Jun 2018 13:22:18 +0200,
Zhouyang Jia wrote:
>
> When snd_ctl_add fails, the lack of error-handling code may
> cause unexpected results.
>
> This patch adds error-handling code after calling snd_ctl_add.
>
> Signed-off-by: Zhouyang Jia
> ---
> v1->v2:
> - Check the return value of snd_sonicvibes_create_gameport.
> ---
> sound/pci/sonicvibes.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
> index a8abb15..75225b04 100644
> --- a/sound/pci/sonicvibes.c
> +++ b/sound/pci/sonicvibes.c
> @@ -1188,6 +1188,7 @@ SONICVIBES_SINGLE("Joystick Speed", 0,
> SV_IREG_GAME_PORT, 1, 15, 0);
> static int snd_sonicvibes_create_gameport(struct sonicvibes *sonic)
> {
> struct gameport *gp;
> + int err;
>
> sonic->gameport = gp = gameport_allocate_port();
> if (!gp) {
> @@ -1203,7 +1204,10 @@ static int snd_sonicvibes_create_gameport(struct
> sonicvibes *sonic)
>
> gameport_register_port(gp);
>
> - snd_ctl_add(sonic->card, snd_ctl_new1(_sonicvibes_game_control,
> sonic));
> + err = snd_ctl_add(sonic->card,
> + snd_ctl_new1(_sonicvibes_game_control, sonic));
> + if (err < 0)
> + return err;
>
> return 0;
> }
> @@ -1515,7 +1519,10 @@ static int snd_sonic_probe(struct pci_dev *pci,
> return err;
> }
>
> - snd_sonicvibes_create_gameport(sonic);
> + if ((err = snd_sonicvibes_create_gameport(sonic)) < 0) {
> + snd_card_free(card);
> + return err;
> +}
You don't need to inherit the old-fashioned style "if ((err = xxx)"
in a new code. Check what checkpatch.pl complains.
thanks,
Takashi
Re: [PATCH v2] ALSA: sonicvibes: add error handling for snd_ctl_add
On Thu, 14 Jun 2018 13:22:18 +0200,
Zhouyang Jia wrote:
>
> When snd_ctl_add fails, the lack of error-handling code may
> cause unexpected results.
>
> This patch adds error-handling code after calling snd_ctl_add.
>
> Signed-off-by: Zhouyang Jia
> ---
> v1->v2:
> - Check the return value of snd_sonicvibes_create_gameport.
> ---
> sound/pci/sonicvibes.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
> index a8abb15..75225b04 100644
> --- a/sound/pci/sonicvibes.c
> +++ b/sound/pci/sonicvibes.c
> @@ -1188,6 +1188,7 @@ SONICVIBES_SINGLE("Joystick Speed", 0,
> SV_IREG_GAME_PORT, 1, 15, 0);
> static int snd_sonicvibes_create_gameport(struct sonicvibes *sonic)
> {
> struct gameport *gp;
> + int err;
>
> sonic->gameport = gp = gameport_allocate_port();
> if (!gp) {
> @@ -1203,7 +1204,10 @@ static int snd_sonicvibes_create_gameport(struct
> sonicvibes *sonic)
>
> gameport_register_port(gp);
>
> - snd_ctl_add(sonic->card, snd_ctl_new1(_sonicvibes_game_control,
> sonic));
> + err = snd_ctl_add(sonic->card,
> + snd_ctl_new1(_sonicvibes_game_control, sonic));
> + if (err < 0)
> + return err;
>
> return 0;
> }
> @@ -1515,7 +1519,10 @@ static int snd_sonic_probe(struct pci_dev *pci,
> return err;
> }
>
> - snd_sonicvibes_create_gameport(sonic);
> + if ((err = snd_sonicvibes_create_gameport(sonic)) < 0) {
> + snd_card_free(card);
> + return err;
> +}
You don't need to inherit the old-fashioned style "if ((err = xxx)"
in a new code. Check what checkpatch.pl complains.
thanks,
Takashi
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 09:21:03AM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > ...
> > > >
> > > > My concern is that there are a few E820 memory types rather than
> > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > > all into memblock.reserved is really acceptable.
> > >
> > > Hi Naoya,
> > >
> > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > within
> > > E820_TYPE_RAM.
> > > Actually, in your original patch, you are walking memblock.memory, which
> > > should
> > > only contain E820_TYPE_RAM ranges (talking about x86).
> > >
> > > So I think the below would to the trick as well?
> > >
> > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > {
> > > int i;
> > > u64 end;
> > > + u64 next = 0;
> > >
> > > /*
> > > * The bootstrap memblock region count maximum is 128 entries
> > >
> > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > >
> > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > E820_TYPE_RESERVED_KERN)
> > > continue;
> > >
> > > +
> > > + if (entry->type == E820_TYPE_RAM)
> > > + if (next < entry->addr) {
> > > + memblock_reserve (next, next + (entry->addr -
> > > next));
> > > + next = end;
> > > + }
> > >
> > > With the above patch, I can no longer see the issues either.
> >
> > I double-checked and this change looks good to me.
> >
> > >
> > > Although, there is a difference between this and your original patch.
> > > In your original patch, you are just zeroing the pages, while with this
> > > one (or with your second patch),
> > > we will zero the page in reserve_bootmem_region(), but that function also
> > > init
> > > some other fields of the struct page:
> > >
> > > mm_zero_struct_page(page);
> > > set_page_links(page, zone, nid, pfn);
> > > init_page_count(page);
> > > page_mapcount_reset(page);
> > > page_cpupid_reset_last(page);
> > >
> > > So I am not sure we want to bother doing that for pages that are really
> > > unreachable.
> >
> > I think that considering that /proc/kpageflags can check them, some data
> > (even if it's trivial) might be better than just zeros.
> >
> > Here's the updated patch.
> > Thanks for the suggestion and testing!
> >
> > ---
> > From: Naoya Horiguchi
> > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
> >
> > There is a kernel panic that is triggered when reading /proc/kpageflags
> > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> >
> > BUG: unable to handle kernel paging request at fffe
> > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > Oops: [#1] SMP PTI
> > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> > 04/01/2014
> > RIP: 0010:stable_page_flags+0x27/0x3c0
> > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > RAX: fffe RBX: 7fffeff9 RCX:
> > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > RBP: R08: 0001 R09: 0001
> > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > FS: 7efc4335a500() GS:93a5bfc0()
> > knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > Call Trace:
> >kpageflags_read+0xc7/0x120
> >proc_reg_read+0x3c/0x60
> >__vfs_read+0x36/0x170
> >vfs_read+0x89/0x130
> >ksys_pread64+0x71/0x90
> >do_syscall_64+0x5b/0x160
> >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > RIP: 0033:0x7efc42e75e23
> > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > ff ff 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
> >
> > According to kernel bisection, this problem became visible due to commit
> > f7f99100d8d9 which changes how struct pages are initialized.
> >
> > Memblock layout affects the pfn ranges covered by node/zone. Consider
> > that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> > the
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 09:21:03AM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > ...
> > > >
> > > > My concern is that there are a few E820 memory types rather than
> > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > > all into memblock.reserved is really acceptable.
> > >
> > > Hi Naoya,
> > >
> > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > within
> > > E820_TYPE_RAM.
> > > Actually, in your original patch, you are walking memblock.memory, which
> > > should
> > > only contain E820_TYPE_RAM ranges (talking about x86).
> > >
> > > So I think the below would to the trick as well?
> > >
> > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > {
> > > int i;
> > > u64 end;
> > > + u64 next = 0;
> > >
> > > /*
> > > * The bootstrap memblock region count maximum is 128 entries
> > >
> > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > >
> > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > E820_TYPE_RESERVED_KERN)
> > > continue;
> > >
> > > +
> > > + if (entry->type == E820_TYPE_RAM)
> > > + if (next < entry->addr) {
> > > + memblock_reserve (next, next + (entry->addr -
> > > next));
> > > + next = end;
> > > + }
> > >
> > > With the above patch, I can no longer see the issues either.
> >
> > I double-checked and this change looks good to me.
> >
> > >
> > > Although, there is a difference between this and your original patch.
> > > In your original patch, you are just zeroing the pages, while with this
> > > one (or with your second patch),
> > > we will zero the page in reserve_bootmem_region(), but that function also
> > > init
> > > some other fields of the struct page:
> > >
> > > mm_zero_struct_page(page);
> > > set_page_links(page, zone, nid, pfn);
> > > init_page_count(page);
> > > page_mapcount_reset(page);
> > > page_cpupid_reset_last(page);
> > >
> > > So I am not sure we want to bother doing that for pages that are really
> > > unreachable.
> >
> > I think that considering that /proc/kpageflags can check them, some data
> > (even if it's trivial) might be better than just zeros.
> >
> > Here's the updated patch.
> > Thanks for the suggestion and testing!
> >
> > ---
> > From: Naoya Horiguchi
> > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
> >
> > There is a kernel panic that is triggered when reading /proc/kpageflags
> > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> >
> > BUG: unable to handle kernel paging request at fffe
> > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > Oops: [#1] SMP PTI
> > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> > 04/01/2014
> > RIP: 0010:stable_page_flags+0x27/0x3c0
> > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > RAX: fffe RBX: 7fffeff9 RCX:
> > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > RBP: R08: 0001 R09: 0001
> > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > FS: 7efc4335a500() GS:93a5bfc0()
> > knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > Call Trace:
> >kpageflags_read+0xc7/0x120
> >proc_reg_read+0x3c/0x60
> >__vfs_read+0x36/0x170
> >vfs_read+0x89/0x130
> >ksys_pread64+0x71/0x90
> >do_syscall_64+0x5b/0x160
> >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > RIP: 0033:0x7efc42e75e23
> > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > ff ff 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
> >
> > According to kernel bisection, this problem became visible due to commit
> > f7f99100d8d9 which changes how struct pages are initialized.
> >
> > Memblock layout affects the pfn ranges covered by node/zone. Consider
> > that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> > the
Re: [PATCH 1/4] arm: dts: add support for Laird WB45N cpu module and DVK
On 14/06/2018 11:07:33+0200, Alexandre Belloni wrote: > Hi, > > On 14/06/2018 09:51:54+0100, Ben Whitten wrote: > > This need a proper commit message. Maybe you can also add a link to the > technical brief for the platform? > > > Signed-off-by: Ben Whitten > > --- > > arch/arm/boot/dts/Makefile| 3 +- > > arch/arm/boot/dts/at91-wb45n.dts | 66 +++ > > arch/arm/boot/dts/at91-wb45n.dtsi | 169 > > ++ > > 3 files changed, 237 insertions(+), 1 deletion(-) > > create mode 100644 arch/arm/boot/dts/at91-wb45n.dts > > create mode 100644 arch/arm/boot/dts/at91-wb45n.dtsi > > > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > > index 7e24249..1ee94ee 100644 > > --- a/arch/arm/boot/dts/Makefile > > +++ b/arch/arm/boot/dts/Makefile > > @@ -42,7 +42,8 @@ dtb-$(CONFIG_SOC_AT91SAM9) += \ > > at91sam9g25ek.dtb \ > > at91sam9g35ek.dtb \ > > at91sam9x25ek.dtb \ > > - at91sam9x35ek.dtb > > + at91sam9x35ek.dtb \ > > + at91-wb45n.dtb > > The proper name for the file is -board.dtb so this should be > at91sam9g25-wb45n.dtb. > Nicolas tells me that the name was right, please disregard my comment (also on the other patches). -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 1/4] arm: dts: add support for Laird WB45N cpu module and DVK
On 14/06/2018 11:07:33+0200, Alexandre Belloni wrote: > Hi, > > On 14/06/2018 09:51:54+0100, Ben Whitten wrote: > > This need a proper commit message. Maybe you can also add a link to the > technical brief for the platform? > > > Signed-off-by: Ben Whitten > > --- > > arch/arm/boot/dts/Makefile| 3 +- > > arch/arm/boot/dts/at91-wb45n.dts | 66 +++ > > arch/arm/boot/dts/at91-wb45n.dtsi | 169 > > ++ > > 3 files changed, 237 insertions(+), 1 deletion(-) > > create mode 100644 arch/arm/boot/dts/at91-wb45n.dts > > create mode 100644 arch/arm/boot/dts/at91-wb45n.dtsi > > > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > > index 7e24249..1ee94ee 100644 > > --- a/arch/arm/boot/dts/Makefile > > +++ b/arch/arm/boot/dts/Makefile > > @@ -42,7 +42,8 @@ dtb-$(CONFIG_SOC_AT91SAM9) += \ > > at91sam9g25ek.dtb \ > > at91sam9g35ek.dtb \ > > at91sam9x25ek.dtb \ > > - at91sam9x35ek.dtb > > + at91sam9x35ek.dtb \ > > + at91-wb45n.dtb > > The proper name for the file is -board.dtb so this should be > at91sam9g25-wb45n.dtb. > Nicolas tells me that the name was right, please disregard my comment (also on the other patches). -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
[PATCH v2] ALSA: sonicvibes: add error handling for snd_ctl_add
When snd_ctl_add fails, the lack of error-handling code may
cause unexpected results.
This patch adds error-handling code after calling snd_ctl_add.
Signed-off-by: Zhouyang Jia
---
v1->v2:
- Check the return value of snd_sonicvibes_create_gameport.
---
sound/pci/sonicvibes.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
index a8abb15..75225b04 100644
--- a/sound/pci/sonicvibes.c
+++ b/sound/pci/sonicvibes.c
@@ -1188,6 +1188,7 @@ SONICVIBES_SINGLE("Joystick Speed", 0, SV_IREG_GAME_PORT,
1, 15, 0);
static int snd_sonicvibes_create_gameport(struct sonicvibes *sonic)
{
struct gameport *gp;
+ int err;
sonic->gameport = gp = gameport_allocate_port();
if (!gp) {
@@ -1203,7 +1204,10 @@ static int snd_sonicvibes_create_gameport(struct
sonicvibes *sonic)
gameport_register_port(gp);
- snd_ctl_add(sonic->card, snd_ctl_new1(_sonicvibes_game_control,
sonic));
+ err = snd_ctl_add(sonic->card,
+ snd_ctl_new1(_sonicvibes_game_control, sonic));
+ if (err < 0)
+ return err;
return 0;
}
@@ -1515,7 +1519,10 @@ static int snd_sonic_probe(struct pci_dev *pci,
return err;
}
- snd_sonicvibes_create_gameport(sonic);
+ if ((err = snd_sonicvibes_create_gameport(sonic)) < 0) {
+ snd_card_free(card);
+ return err;
+}
if ((err = snd_card_register(card)) < 0) {
snd_card_free(card);
--
2.7.4
[PATCH v2] ALSA: sonicvibes: add error handling for snd_ctl_add
When snd_ctl_add fails, the lack of error-handling code may
cause unexpected results.
This patch adds error-handling code after calling snd_ctl_add.
Signed-off-by: Zhouyang Jia
---
v1->v2:
- Check the return value of snd_sonicvibes_create_gameport.
---
sound/pci/sonicvibes.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
index a8abb15..75225b04 100644
--- a/sound/pci/sonicvibes.c
+++ b/sound/pci/sonicvibes.c
@@ -1188,6 +1188,7 @@ SONICVIBES_SINGLE("Joystick Speed", 0, SV_IREG_GAME_PORT,
1, 15, 0);
static int snd_sonicvibes_create_gameport(struct sonicvibes *sonic)
{
struct gameport *gp;
+ int err;
sonic->gameport = gp = gameport_allocate_port();
if (!gp) {
@@ -1203,7 +1204,10 @@ static int snd_sonicvibes_create_gameport(struct
sonicvibes *sonic)
gameport_register_port(gp);
- snd_ctl_add(sonic->card, snd_ctl_new1(_sonicvibes_game_control,
sonic));
+ err = snd_ctl_add(sonic->card,
+ snd_ctl_new1(_sonicvibes_game_control, sonic));
+ if (err < 0)
+ return err;
return 0;
}
@@ -1515,7 +1519,10 @@ static int snd_sonic_probe(struct pci_dev *pci,
return err;
}
- snd_sonicvibes_create_gameport(sonic);
+ if ((err = snd_sonicvibes_create_gameport(sonic)) < 0) {
+ snd_card_free(card);
+ return err;
+}
if ((err = snd_card_register(card)) < 0) {
snd_card_free(card);
--
2.7.4
