All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [igt-dev] [PATCH i-g-t 2/3] runner/resultgen: Don't pass NULL to str*() functions
From: Petri Latvala @ 2019-06-20 12:33 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev
In-Reply-To: <20190620122902.20761-2-arkadiusz.hiler@intel.com>

On Thu, Jun 20, 2019 at 03:29:01PM +0300, Arkadiusz Hiler wrote:
> If we don't get values from the JSON strlen() and strcmp() on them would
> dereference NULL pointer.
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>


Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  runner/resultgen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index 7b4cd519..58b95220 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -866,7 +866,7 @@ static void fill_from_journal(int fd,
>  
>  static void override_result_single(struct json_object *obj)
>  {
> -	const char *errtext = NULL, *result = NULL;
> +	const char *errtext = "", *result = "";
>  	struct json_object *textobj;
>  	bool dmesgwarns = false;
>  
> -- 
> 2.21.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply

* Re: [igt-dev] [PATCH i-g-t 1/3] runner/executor: Make sure that intervals_left is always initialized
From: Petri Latvala @ 2019-06-20 12:33 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev
In-Reply-To: <20190620122902.20761-1-arkadiusz.hiler@intel.com>

On Thu, Jun 20, 2019 at 03:29:00PM +0300, Arkadiusz Hiler wrote:
> intervals_left got initialized only when when we had a timeout exceeding
> watchdog capabilities, meaning we had to use multiple shorter intervals
> 
> by moving intervals_left = timeout_intervals down we are always
> initializing it
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  runner/executor.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/runner/executor.c b/runner/executor.c
> index 7e5fbe8f..bce4d420 100644
> --- a/runner/executor.c
> +++ b/runner/executor.c
> @@ -493,7 +493,6 @@ static int monitor_output(pid_t child,
>  			if (wd_timeout - wd_extra < 0)
>  				wd_extra = wd_timeout / 2;
>  			timeout_intervals = timeout / (wd_timeout - wd_extra);
> -			intervals_left = timeout_intervals;
>  			timeout /= timeout_intervals;
>  
>  			if (settings->log_level >= LOG_LEVEL_VERBOSE) {
> @@ -504,6 +503,8 @@ static int monitor_output(pid_t child,
>  		}
>  	}
>  
> +	intervals_left = timeout_intervals;
> +
>  	while (outfd >= 0 || errfd >= 0 || sigfd >= 0) {
>  		struct timeval tv = { .tv_sec = timeout };
>  
> -- 
> 2.21.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply

* Re: [PATCH 1/9] SUNRPC: Cache cred of process creating the rpc_client
From: Ido Schimmel @ 2019-06-20 12:33 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Anna Schumaker, linux-nfs
In-Reply-To: <20190614185237.GA550@splinter>

On Fri, Jun 14, 2019 at 09:52:37PM +0300, Ido Schimmel wrote:
> On Wed, Apr 24, 2019 at 05:46:42PM -0400, Trond Myklebust wrote:
> > When converting kuids to AUTH_UNIX creds, etc we will want to use the
> > same user namespace as the process that created the rpc client.
> > 
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> Hi,
> 
> Since upgrading to v5.2-rc1 I started encountering these memory leaks
> [1]. Bisection using this reproducer [2] points to this patch. Attached
> the full bisection log [3].
> 
> Please let me know if more information is required.

Ping?

> 
> Thanks
> 
> [1]
> unreferenced object 0xffffa35dfaea3000 (size 192):
>   comm "mount", pid 428, jiffies 4294703475 (age 7.578s)
>   hex dump (first 32 bytes):
>     03 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<00000000b43bed74>] prepare_exec_creds+0x6/0x40
>     [<00000000422eb980>] __do_execve_file.isra.56+0x124/0x930
>     [<00000000aa848639>] __x64_sys_execve+0x2f/0x40
>     [<000000008d5c43e1>] do_syscall_64+0x43/0xf0
>     [<0000000068b03b0e>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>     [<000000001fab781f>] 0xffffffffffffffff
> 
> [2]
> #!/bin/bash
> 
> umount /mnt/share155
> mount -t nfs <nfs-server>:/images/share /mnt/share155
> echo scan > /sys/kernel/debug/kmemleak
> cat /sys/kernel/debug/kmemleak
> 
> [3]
> git bisect start
> # good: [e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd] Linux 5.1
> git bisect good e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
> # bad: [a188339ca5a396acc588e5851ed7e19f66b0ebd9] Linux 5.2-rc1
> git bisect bad a188339ca5a396acc588e5851ed7e19f66b0ebd9
> # good: [2646719a48c21ba0cae82a3f57382a9573fd8400] Merge tag 'kbuild-v5.2' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild
> git bisect good 2646719a48c21ba0cae82a3f57382a9573fd8400
> # bad: [b970afcfcabd63cd3832e95db096439c177c3592] Merge tag 'powerpc-5.2-1' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> git bisect bad b970afcfcabd63cd3832e95db096439c177c3592
> # good: [eb85d03e01c3e9f3b0ba7282b2e3515a635decb2] Merge tag 'drm-misc-next-fixes-2019-05-08' of git://anongit.freedesktop.org/drm/drm-misc into drm-next
> git bisect good eb85d03e01c3e9f3b0ba7282b2e3515a635decb2
> # good: [dce45af5c2e9e85f22578f2f8065f225f5d11764] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma
> git bisect good dce45af5c2e9e85f22578f2f8065f225f5d11764
> # bad: [8e4ff713ce313dcabbb60e6ede1ffc193e67631f] Merge tag 'rtc-5.2' of git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux
> git bisect bad 8e4ff713ce313dcabbb60e6ede1ffc193e67631f
> # bad: [45182e4e1f8ac04708ca7508c51d9103f07d81ab] Merge branch 'i2c/for-5.2' of git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux
> git bisect bad 45182e4e1f8ac04708ca7508c51d9103f07d81ab
> # bad: [5940d1cf9f42f67e9cc3f7df9eda39f5888d6e9e] SUNRPC: Rebalance a kref in auth_gss.c
> git bisect bad 5940d1cf9f42f67e9cc3f7df9eda39f5888d6e9e
> # good: [23146500b32fbee7eaa57c5002fcd64e5d9b32ca] xprtrdma: Clean up rpcrdma_create_rep() and rpcrdma_destroy_rep()
> git bisect good 23146500b32fbee7eaa57c5002fcd64e5d9b32ca
> # good: [2cfd11f16f01c0ee8f83bb07027c9d2f43565473] xprtrdma: Remove stale comment
> git bisect good 2cfd11f16f01c0ee8f83bb07027c9d2f43565473
> # bad: [b422df915cef80333d7a1732e6ed81f41db12b79] lockd: Store the lockd client credential in struct nlm_host
> git bisect bad b422df915cef80333d7a1732e6ed81f41db12b79
> # bad: [ac83228a7101e655ba5a7fa61ae10b058ada15db] SUNRPC: Use namespace of listening daemon in the client AUTH_GSS upcall
> git bisect bad ac83228a7101e655ba5a7fa61ae10b058ada15db
> # bad: [1a58e8a0e5c1f188a80eb9e505bc77d78a31a4ec] NFS: Store the credential of the mount process in the nfs_server
> git bisect bad 1a58e8a0e5c1f188a80eb9e505bc77d78a31a4ec
> # bad: [79caa5fad47c69874f9efc4ac3128cc3f6d36f6e] SUNRPC: Cache cred of process creating the rpc_client
> git bisect bad 79caa5fad47c69874f9efc4ac3128cc3f6d36f6e
> # first bad commit: [79caa5fad47c69874f9efc4ac3128cc3f6d36f6e] SUNRPC: Cache cred of process creating the rpc_client

^ permalink raw reply

* Re: [PATCH] drm/imx: correct order of crtc disable
From: Daniel Vetter @ 2019-06-20 12:32 UTC (permalink / raw)
  To: Robert Beckett; +Cc: dri-devel
In-Reply-To: <4fceab36a9d800249b66bfd37114d63fc3386d29.camel@collabora.com>

On Thu, Jun 20, 2019 at 12:12:13PM +0100, Robert Beckett wrote:
> On Thu, 2019-06-20 at 10:50 +0200, Daniel Vetter wrote:
> > On Wed, Jun 19, 2019 at 11:40 AM Philipp Zabel <
> > p.zabel@pengutronix.de> wrote:
> > > 
> > > Hi Robert,
> > > 
> > > thank you for the patch.
> > > 
> > > On Tue, 2019-06-18 at 16:50 +0100, Robert Beckett wrote:
> > > > Notify drm core before sending pending events during crtc
> > > > disable.
> > > > This fixes the first event after disable having an old stale
> > > > timestamp
> > > > by having drm_crtc_vblank_off update the timestamp to now.
> > > > 
> > > > This was seen while debugging weston log message:
> > > > Warning: computed repaint delay is insane: -8212 msec
> > > > 
> > > 
> > > Would you say this
> > > Fixes: a474478642d5 ("drm/imx: fix crtc vblank state regression")
> > > ?
> > > 
> > > > Signed-off-by: Robert Beckett <bob.beckett@collabora.com>
> > > > ---
> > > >  drivers/gpu/drm/imx/ipuv3-crtc.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > > b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > > index 9cc1d678674f..c436a28d50e4 100644
> > > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > > @@ -91,14 +91,14 @@ static void ipu_crtc_atomic_disable(struct
> > > > drm_crtc *crtc,
> > > >       ipu_dc_disable(ipu);
> > > >       ipu_prg_disable(ipu);
> > > > 
> > > > +     drm_crtc_vblank_off(crtc);
> > > > +
> > > 
> > > This is explained in the commit message and aligns with the
> > > drm_crtc_state @event documentation.
> > 
> > This part here looks fishy. The drm_vblank.c code is supposed to do
> > the right thing, no matter where or when you ask it to generate an
> > event. It definitely shouldn't generate a timestamp that's a few
> > seconds too early. Bunch of options:
> > - there's a bug in drm_vblank.c and it's mixing up something and
> > generating a totally bogus value.
> > - there's a lie in your imx vblank code, which trips the drm_vblank.c
> > counter interpolation and results in a totally bogus value.
> > 
> > drm_vblank.c assumes that if you do claim to have a hw counter and
> > generate timestamps, that those are perfectly accurate. It only falls
> > back to guestimating using the system timer if that's not present.
> > 
> > Either way, this very much smells like papering over a bug if this
> > change indeed fixes your wrong vblank timestamps.
> > 
> 
> A quick explaination of where the dodgy timestamp came from:
> 1. driver starts up
> 2. fbcon comes along and restores fbdev, enabling vblank
> 3. vblank_disable_fn fires via timer disabling vblank, keeping vblank
> seq number and time set at current value
> (some time later)
> 4. weston starts and does a modeset
> 5. atomic commit disables crtc while it does the modeset
> 6. ipu_crtc_atomic_disable sends vblank with old seq number and time
> 
> It turns out the actual fix for the old vblank is the next change,
> which stops it being sent at all during the crtc disable as it is is
> still active, it would then go through drm_crtc_vblank_off, reseting
> the timestamp, and get delivered during the vblank enable as part of
> the atomic commit.

This shouldn't fix your vblank timestamp troubles either. It might mean
that the timestamp is slightly too early (because you take it while
shutting down the crtc, not while re-enabling), but not by seconds.

Quick experiment: Disable vblank disabling with drm.vblankoffdelay = 0. If
that also fixes the timestamps, then I'm pretty sure you have a driver bug
somewhere and lie to the vblank core code about something.
-Daniel

> 
> So, in theory, we could just have the following change to fix the
> specific issue of a stale timestamp.
> 
> However, given the documentation for the event in
> include/drm/drm_crtc.h:
> 
>          *  - The event is for a CRTC which is being disabled through
> this
>          *    atomic commit. In that case the event can be send out any
> time
>          *    after the hardware has stopped scanning out the current
>          *    framebuffers. It should contain the timestamp and counter
> for the
>          *    last vblank before the display pipeline was shut off. The
> simplest
>          *    way to achieve that is calling
> drm_crtc_send_vblank_event()
>          *    somewhen after drm_crtc_vblank_off() has been called.
> 
> This still seems like a sensible change for when the crtc is being
> disabled.
> 
> 
> 
> > > >       spin_lock_irq(&crtc->dev->event_lock);
> > > > -     if (crtc->state->event)
> > > > {
> > > > +     if (crtc->state->event && !crtc->state->active) {
> > > 
> > > This is not mentioned though.
> > > 
> > > If the pending event is not sent here, I assume it will be picked
> > > up by
> > > .atomic_flush and will then be sent after the first EOF interrupt
> > > after
> > > the modeset is complete. Can you explain this in the commit
> > > message?
> > 
> > Yeah looks correct (you only want to generate the event here when the
> > crtc stays off), if it gets re-enabled the event should only be
> > generated later on once that's all finished. But separate bugfix.
> > -Daniel
> > 
> 
> It looks like this is actually the fix needed to avoid the bogus
> timestamp.
> 
> I can split this patch up in to 2 commits if desired?
> 
> > > 
> > > With that,
> > > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > 
> > > >               drm_crtc_send_vblank_event(crtc, crtc->state-
> > > > >event);
> > > >               crtc->state->event = NULL;
> > > >       }
> > > >       spin_unlock_irq(&crtc->dev->event_lock);
> > > > -
> > > > -     drm_crtc_vblank_off(crtc);
> > > >  }
> > > > 
> > > >  static void imx_drm_crtc_reset(struct drm_crtc *crtc)
> > > 
> > > regards
> > > Philipp
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > 
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [dpdk-dev] [PATCH v9] app/pdump: exit with primary process
From: Pattan, Reshma @ 2019-06-20 12:32 UTC (permalink / raw)
  To: Suanming.Mou, dev@dpdk.org
  Cc: Varghese, Vipin, Burakov, Anatoly, thomas@monjalon.net
In-Reply-To: <1557897039-89555-1-git-send-email-mousuanming@huawei.com>



> -----Original Message-----
> From: Suanming.Mou [mailto:mousuanming@huawei.com]
> Sent: Wednesday, May 15, 2019 6:11 AM
> To: dev@dpdk.org
> Cc: Varghese, Vipin <vipin.varghese@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; thomas@monjalon.net; Pattan, Reshma
> <reshma.pattan@intel.com>
> Subject: [PATCH v9] app/pdump: exit with primary process
> 
> From: Suanming Mou <mousuanming@huawei.com>
> 
> The pdump tool works as the secondary process. When the primary process
> exits and the residual secondary process keeps running, it will make the
> primary process can't start up again. Since the ex-fbarry files are still
> attached by the secondary process pdump, the 'new' primary process can't
> get these files locked.
> 
> The patch is to set up an alarm which runs every 0.5s periodically to monitor
> the primary process in the pdump. Once the primary exits, so will the
> pdump.
> 
> Signed-off-by: Suanming Mou <mousuanming@huawei.com>
> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Reviewed-by: Vipin Varghese <vipin.varghese@intel.com>
> Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
> Reviewed-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> V9:
> * reword the comments and update the git log.
> * move release note to release_19_08.rst.
> * remove dot in name.
> 
Acked-by line is missing, so re acking. Keep my ack if you have to send next version.
Acked-by: Reshma Pattan <reshma.pattan@intel.com>

^ permalink raw reply

* [Cocci] Checking redundant variable initialisations with SmPL?
From: Markus Elfring @ 2019-06-20 12:30 UTC (permalink / raw)
  To: Coccinelle

Hello,

A patch on a topic like “[next] lkdtm: remove redundant initialization of ret”
caught also my software development attention.
https://lkml.org/lkml/2019/6/14/265
https://lore.kernel.org/patchwork/patch/1088971/
https://lore.kernel.org/lkml/20190614094311.24024-1-colin.king@canonical.com/


I hoped that the following script for the semantic patch language can point
such an update candidate also out.

@display@
binary operator bo;
expression e1, e2, e3;
identifier var, work;
statement is, es;
type t;
@@
*t var = e1;
 ... when != if (var bo e2) is else es
*var =
(      work(...)
|      e3
)


elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/show_questionable_variable_initialisation1.cocci drivers/misc/lkdtm/core.c
…
exn while in timeout_function
Fatal error: exception Coccinelle_modules.Common.Impossible(56)


How do you think about the software situation?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply

* Re: [PATCH v10 00/13] mm: Sub-section memory hotplug support
From: Aneesh Kumar K.V @ 2019-06-20 12:30 UTC (permalink / raw)
  To: Dan Williams, akpm
  Cc: David Hildenbrand, Jérôme Glisse, Mike Rapoport,
	Jane Chu, Pavel Tatashin, Jonathan Corbet, Qian Cai,
	Logan Gunthorpe, Toshi Kani, Oscar Salvador, Jeff Moyer,
	Michal Hocko, Vlastimil Babka, stable, Wei Yang, linux-mm,
	linux-nvdimm, linux-kernel
In-Reply-To: <156092349300.979959.17603710711957735135.stgit@dwillia2-desk3.amr.corp.intel.com>

Dan Williams <dan.j.williams@intel.com> writes:

> Changes since v9 [1]:
> - Fix multiple issues related to the fact that pfn_valid() has
>   traditionally returned true for any pfn in an 'early' (onlined at
>   boot) section regardless of whether that pfn represented 'System RAM'.
>   Teach pfn_valid() to maintain its traditional behavior in the presence
>   of subsections. Specifically, subsection precision for pfn_valid() is
>   only considered for non-early / hot-plugged sections. (Qian)
>
> - Related to the first item introduce a SECTION_IS_EARLY
>   (->section_mem_map flag) to remove the existing hacks for determining
>   an early section by looking at whether the usemap was allocated from the
>   slab.
>
> - Kill off the EEXIST hackery in __add_pages(). It breaks
>   (arch_add_memory() false-positive) the detection of subsection
>   collisions reported by section_activate(). It is also obviated by
>   David's recent reworks to move the 'System RAM' request_region() earlier
>   in the add_memory() sequence().
>
> - Switch to an arch-independent / static subsection-size of 2MB.
>   Otherwise, a per-arch subsection-size is a roadblock on the path to
>   persistent memory namespace compatibility across archs. (Jeff)
>
> - Update the changelog for "libnvdimm/pfn: Fix fsdax-mode namespace
>   info-block zero-fields" to clarify that the "Cc: stable" is only there
>   as safety measure for a distro that decides to backport "libnvdimm/pfn:
>   Stop padding pmem namespaces to section alignment", otherwise there is
>   no known bug exposure in older kernels. (Andrew)
>   
> - Drop some redundant subsection checks (Oscar)
>
> - Collect some reviewed-bys
>
> [1]: https://lore.kernel.org/lkml/155977186863.2443951.9036044808311959913.stgit@dwillia2-desk3.amr.corp.intel.com/


You can add Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
for ppc64.

BTW even after this series we have the kernel crash mentioned in the
below email on reconfigure. 

https://lore.kernel.org/linux-mm/20190514025354.9108-1-aneesh.kumar@linux.ibm.com

I guess we need to conclude how the reserve space struct page should be
initialized ?

>
> ---
>
> The memory hotplug section is an arbitrary / convenient unit for memory
> hotplug. 'Section-size' units have bled into the user interface
> ('memblock' sysfs) and can not be changed without breaking existing
> userspace. The section-size constraint, while mostly benign for typical
> memory hotplug, has and continues to wreak havoc with 'device-memory'
> use cases, persistent memory (pmem) in particular. Recall that pmem uses
> devm_memremap_pages(), and subsequently arch_add_memory(), to allocate a
> 'struct page' memmap for pmem. However, it does not use the 'bottom
> half' of memory hotplug, i.e. never marks pmem pages online and never
> exposes the userspace memblock interface for pmem. This leaves an
> opening to redress the section-size constraint.
>
> To date, the libnvdimm subsystem has attempted to inject padding to
> satisfy the internal constraints of arch_add_memory(). Beyond
> complicating the code, leading to bugs [2], wasting memory, and limiting
> configuration flexibility, the padding hack is broken when the platform
> changes this physical memory alignment of pmem from one boot to the
> next. Device failure (intermittent or permanent) and physical
> reconfiguration are events that can cause the platform firmware to
> change the physical placement of pmem on a subsequent boot, and device
> failure is an everyday event in a data-center.
>
> It turns out that sections are only a hard requirement of the
> user-facing interface for memory hotplug and with a bit more
> infrastructure sub-section arch_add_memory() support can be added for
> kernel internal usages like devm_memremap_pages(). Here is an analysis
> of the current design assumptions in the current code and how they are
> addressed in the new implementation:
>
> Current design assumptions:
>
> - Sections that describe boot memory (early sections) are never
>   unplugged / removed.
>
> - pfn_valid(), in the CONFIG_SPARSEMEM_VMEMMAP=y, case devolves to a
>   valid_section() check
>
> - __add_pages() and helper routines assume all operations occur in
>   PAGES_PER_SECTION units.
>
> - The memblock sysfs interface only comprehends full sections
>
> New design assumptions:
>
> - Sections are instrumented with a sub-section bitmask to track (on x86)
>   individual 2MB sub-divisions of a 128MB section.
>
> - Partially populated early sections can be extended with additional
>   sub-sections, and those sub-sections can be removed with
>   arch_remove_memory(). With this in place we no longer lose usable memory
>   capacity to padding.
>
> - pfn_valid() is updated to look deeper than valid_section() to also check the
>   active-sub-section mask. This indication is in the same cacheline as
>   the valid_section() so the performance impact is expected to be
>   negligible. So far the lkp robot has not reported any regressions.
>
> - Outside of the core vmemmap population routines which are replaced,
>   other helper routines like shrink_{zone,pgdat}_span() are updated to
>   handle the smaller granularity. Core memory hotplug routines that deal
>   with online memory are not touched.
>
> - The existing memblock sysfs user api guarantees / assumptions are
>   not touched since this capability is limited to !online
>   !memblock-sysfs-accessible sections.
>
> Meanwhile the issue reports continue to roll in from users that do not
> understand when and how the 128MB constraint will bite them. The current
> implementation relied on being able to support at least one misaligned
> namespace, but that immediately falls over on any moderately complex
> namespace creation attempt. Beyond the initial problem of 'System RAM'
> colliding with pmem, and the unsolvable problem of physical alignment
> changes, Linux is now being exposed to platforms that collide pmem
> ranges with other pmem ranges by default [3]. In short,
> devm_memremap_pages() has pushed the venerable section-size constraint
> past the breaking point, and the simplicity of section-aligned
> arch_add_memory() is no longer tenable.
>
> These patches are exposed to the kbuild robot on a subsection-v10 branch
> [4], and a preview of the unit test for this functionality is available
> on the 'subsection-pending' branch of ndctl [5].
>
> [2]: https://lore.kernel.org/r/155000671719.348031.2347363160141119237.stgit@dwillia2-desk3.amr.corp.intel.com
> [3]: https://github.com/pmem/ndctl/issues/76
> [4]: https://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git/log/?h=subsection-v10
> [5]: https://github.com/pmem/ndctl/commit/7c59b4867e1c
>
> ---
>
> Dan Williams (13):
>       mm/sparsemem: Introduce struct mem_section_usage
>       mm/sparsemem: Introduce a SECTION_IS_EARLY flag
>       mm/sparsemem: Add helpers track active portions of a section at boot
>       mm/hotplug: Prepare shrink_{zone,pgdat}_span for sub-section removal
>       mm/sparsemem: Convert kmalloc_section_memmap() to populate_section_memmap()
>       mm/hotplug: Kill is_dev_zone() usage in __remove_pages()
>       mm: Kill is_dev_zone() helper
>       mm/sparsemem: Prepare for sub-section ranges
>       mm/sparsemem: Support sub-section hotplug
>       mm: Document ZONE_DEVICE memory-model implications
>       mm/devm_memremap_pages: Enable sub-section remap
>       libnvdimm/pfn: Fix fsdax-mode namespace info-block zero-fields
>       libnvdimm/pfn: Stop padding pmem namespaces to section alignment
>
>
>  Documentation/vm/memory-model.rst |   39 ++++
>  arch/x86/mm/init_64.c             |    4 
>  drivers/nvdimm/dax_devs.c         |    2 
>  drivers/nvdimm/pfn.h              |   15 --
>  drivers/nvdimm/pfn_devs.c         |   95 +++-------
>  include/linux/memory_hotplug.h    |    7 -
>  include/linux/mm.h                |    4 
>  include/linux/mmzone.h            |   84 +++++++--
>  kernel/memremap.c                 |   61 +++----
>  mm/memory_hotplug.c               |  173 +++++++++----------
>  mm/page_alloc.c                   |   16 +-
>  mm/sparse-vmemmap.c               |   21 ++
>  mm/sparse.c                       |  335 ++++++++++++++++++++++++-------------
>  13 files changed, 494 insertions(+), 362 deletions(-)


^ permalink raw reply

* Re: [PATCH v10 00/13] mm: Sub-section memory hotplug support
From: Aneesh Kumar K.V @ 2019-06-20 12:30 UTC (permalink / raw)
  To: Dan Williams, akpm
  Cc: Michal Hocko, Pavel Tatashin, Jonathan Corbet, linux-kernel,
	David Hildenbrand, stable, Mike Rapoport, linux-mm,
	Jérôme Glisse, Qian Cai, linux-nvdimm, Vlastimil Babka,
	Oscar Salvador
In-Reply-To: <156092349300.979959.17603710711957735135.stgit@dwillia2-desk3.amr.corp.intel.com>

Dan Williams <dan.j.williams@intel.com> writes:

> Changes since v9 [1]:
> - Fix multiple issues related to the fact that pfn_valid() has
>   traditionally returned true for any pfn in an 'early' (onlined at
>   boot) section regardless of whether that pfn represented 'System RAM'.
>   Teach pfn_valid() to maintain its traditional behavior in the presence
>   of subsections. Specifically, subsection precision for pfn_valid() is
>   only considered for non-early / hot-plugged sections. (Qian)
>
> - Related to the first item introduce a SECTION_IS_EARLY
>   (->section_mem_map flag) to remove the existing hacks for determining
>   an early section by looking at whether the usemap was allocated from the
>   slab.
>
> - Kill off the EEXIST hackery in __add_pages(). It breaks
>   (arch_add_memory() false-positive) the detection of subsection
>   collisions reported by section_activate(). It is also obviated by
>   David's recent reworks to move the 'System RAM' request_region() earlier
>   in the add_memory() sequence().
>
> - Switch to an arch-independent / static subsection-size of 2MB.
>   Otherwise, a per-arch subsection-size is a roadblock on the path to
>   persistent memory namespace compatibility across archs. (Jeff)
>
> - Update the changelog for "libnvdimm/pfn: Fix fsdax-mode namespace
>   info-block zero-fields" to clarify that the "Cc: stable" is only there
>   as safety measure for a distro that decides to backport "libnvdimm/pfn:
>   Stop padding pmem namespaces to section alignment", otherwise there is
>   no known bug exposure in older kernels. (Andrew)
>   
> - Drop some redundant subsection checks (Oscar)
>
> - Collect some reviewed-bys
>
> [1]: https://lore.kernel.org/lkml/155977186863.2443951.9036044808311959913.stgit@dwillia2-desk3.amr.corp.intel.com/


You can add Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
for ppc64.

BTW even after this series we have the kernel crash mentioned in the
below email on reconfigure. 

https://lore.kernel.org/linux-mm/20190514025354.9108-1-aneesh.kumar@linux.ibm.com

I guess we need to conclude how the reserve space struct page should be
initialized ?

>
> ---
>
> The memory hotplug section is an arbitrary / convenient unit for memory
> hotplug. 'Section-size' units have bled into the user interface
> ('memblock' sysfs) and can not be changed without breaking existing
> userspace. The section-size constraint, while mostly benign for typical
> memory hotplug, has and continues to wreak havoc with 'device-memory'
> use cases, persistent memory (pmem) in particular. Recall that pmem uses
> devm_memremap_pages(), and subsequently arch_add_memory(), to allocate a
> 'struct page' memmap for pmem. However, it does not use the 'bottom
> half' of memory hotplug, i.e. never marks pmem pages online and never
> exposes the userspace memblock interface for pmem. This leaves an
> opening to redress the section-size constraint.
>
> To date, the libnvdimm subsystem has attempted to inject padding to
> satisfy the internal constraints of arch_add_memory(). Beyond
> complicating the code, leading to bugs [2], wasting memory, and limiting
> configuration flexibility, the padding hack is broken when the platform
> changes this physical memory alignment of pmem from one boot to the
> next. Device failure (intermittent or permanent) and physical
> reconfiguration are events that can cause the platform firmware to
> change the physical placement of pmem on a subsequent boot, and device
> failure is an everyday event in a data-center.
>
> It turns out that sections are only a hard requirement of the
> user-facing interface for memory hotplug and with a bit more
> infrastructure sub-section arch_add_memory() support can be added for
> kernel internal usages like devm_memremap_pages(). Here is an analysis
> of the current design assumptions in the current code and how they are
> addressed in the new implementation:
>
> Current design assumptions:
>
> - Sections that describe boot memory (early sections) are never
>   unplugged / removed.
>
> - pfn_valid(), in the CONFIG_SPARSEMEM_VMEMMAP=y, case devolves to a
>   valid_section() check
>
> - __add_pages() and helper routines assume all operations occur in
>   PAGES_PER_SECTION units.
>
> - The memblock sysfs interface only comprehends full sections
>
> New design assumptions:
>
> - Sections are instrumented with a sub-section bitmask to track (on x86)
>   individual 2MB sub-divisions of a 128MB section.
>
> - Partially populated early sections can be extended with additional
>   sub-sections, and those sub-sections can be removed with
>   arch_remove_memory(). With this in place we no longer lose usable memory
>   capacity to padding.
>
> - pfn_valid() is updated to look deeper than valid_section() to also check the
>   active-sub-section mask. This indication is in the same cacheline as
>   the valid_section() so the performance impact is expected to be
>   negligible. So far the lkp robot has not reported any regressions.
>
> - Outside of the core vmemmap population routines which are replaced,
>   other helper routines like shrink_{zone,pgdat}_span() are updated to
>   handle the smaller granularity. Core memory hotplug routines that deal
>   with online memory are not touched.
>
> - The existing memblock sysfs user api guarantees / assumptions are
>   not touched since this capability is limited to !online
>   !memblock-sysfs-accessible sections.
>
> Meanwhile the issue reports continue to roll in from users that do not
> understand when and how the 128MB constraint will bite them. The current
> implementation relied on being able to support at least one misaligned
> namespace, but that immediately falls over on any moderately complex
> namespace creation attempt. Beyond the initial problem of 'System RAM'
> colliding with pmem, and the unsolvable problem of physical alignment
> changes, Linux is now being exposed to platforms that collide pmem
> ranges with other pmem ranges by default [3]. In short,
> devm_memremap_pages() has pushed the venerable section-size constraint
> past the breaking point, and the simplicity of section-aligned
> arch_add_memory() is no longer tenable.
>
> These patches are exposed to the kbuild robot on a subsection-v10 branch
> [4], and a preview of the unit test for this functionality is available
> on the 'subsection-pending' branch of ndctl [5].
>
> [2]: https://lore.kernel.org/r/155000671719.348031.2347363160141119237.stgit@dwillia2-desk3.amr.corp.intel.com
> [3]: https://github.com/pmem/ndctl/issues/76
> [4]: https://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git/log/?h=subsection-v10
> [5]: https://github.com/pmem/ndctl/commit/7c59b4867e1c
>
> ---
>
> Dan Williams (13):
>       mm/sparsemem: Introduce struct mem_section_usage
>       mm/sparsemem: Introduce a SECTION_IS_EARLY flag
>       mm/sparsemem: Add helpers track active portions of a section at boot
>       mm/hotplug: Prepare shrink_{zone,pgdat}_span for sub-section removal
>       mm/sparsemem: Convert kmalloc_section_memmap() to populate_section_memmap()
>       mm/hotplug: Kill is_dev_zone() usage in __remove_pages()
>       mm: Kill is_dev_zone() helper
>       mm/sparsemem: Prepare for sub-section ranges
>       mm/sparsemem: Support sub-section hotplug
>       mm: Document ZONE_DEVICE memory-model implications
>       mm/devm_memremap_pages: Enable sub-section remap
>       libnvdimm/pfn: Fix fsdax-mode namespace info-block zero-fields
>       libnvdimm/pfn: Stop padding pmem namespaces to section alignment
>
>
>  Documentation/vm/memory-model.rst |   39 ++++
>  arch/x86/mm/init_64.c             |    4 
>  drivers/nvdimm/dax_devs.c         |    2 
>  drivers/nvdimm/pfn.h              |   15 --
>  drivers/nvdimm/pfn_devs.c         |   95 +++-------
>  include/linux/memory_hotplug.h    |    7 -
>  include/linux/mm.h                |    4 
>  include/linux/mmzone.h            |   84 +++++++--
>  kernel/memremap.c                 |   61 +++----
>  mm/memory_hotplug.c               |  173 +++++++++----------
>  mm/page_alloc.c                   |   16 +-
>  mm/sparse-vmemmap.c               |   21 ++
>  mm/sparse.c                       |  335 ++++++++++++++++++++++++-------------
>  13 files changed, 494 insertions(+), 362 deletions(-)

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply

* Re: [PATCH v10 10/13] mm: Document ZONE_DEVICE memory-model implications
From: Mike Rapoport @ 2019-06-20 12:30 UTC (permalink / raw)
  To: Dan Williams; +Cc: akpm, Jonathan Corbet, linux-mm, linux-nvdimm, linux-kernel
In-Reply-To: <156092354985.979959.15763234410543451710.stgit@dwillia2-desk3.amr.corp.intel.com>

On Tue, Jun 18, 2019 at 10:52:29PM -0700, Dan Williams wrote:
> Explain the general mechanisms of 'ZONE_DEVICE' pages and list the users
> of 'devm_memremap_pages()'.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Reported-by: Mike Rapoport <rppt@linux.ibm.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

With one nit below

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
>  Documentation/vm/memory-model.rst |   39 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/vm/memory-model.rst b/Documentation/vm/memory-model.rst
> index 382f72ace1fc..e0af47e02e78 100644
> --- a/Documentation/vm/memory-model.rst
> +++ b/Documentation/vm/memory-model.rst
> @@ -181,3 +181,42 @@ that is eventually passed to vmemmap_populate() through a long chain
>  of function calls. The vmemmap_populate() implementation may use the
>  `vmem_altmap` along with :c:func:`altmap_alloc_block_buf` helper to
>  allocate memory map on the persistent memory device.
> +
> +ZONE_DEVICE
> +===========
> +The `ZONE_DEVICE` facility builds upon `SPARSEMEM_VMEMMAP` to offer
> +`struct page` `mem_map` services for device driver identified physical
> +address ranges. The "device" aspect of `ZONE_DEVICE` relates to the fact
> +that the page objects for these address ranges are never marked online,
> +and that a reference must be taken against the device, not just the page
> +to keep the memory pinned for active use. `ZONE_DEVICE`, via
> +:c:func:`devm_memremap_pages`, performs just enough memory hotplug to
> +turn on :c:func:`pfn_to_page`, :c:func:`page_to_pfn`, and
> +:c:func:`get_user_pages` service for the given range of pfns. Since the
> +page reference count never drops below 1 the page is never tracked as
> +free memory and the page's `struct list_head lru` space is repurposed
> +for back referencing to the host device / driver that mapped the memory.
> +
> +While `SPARSEMEM` presents memory as a collection of sections,
> +optionally collected into memory blocks, `ZONE_DEVICE` users have a need
> +for smaller granularity of populating the `mem_map`. Given that
> +`ZONE_DEVICE` memory is never marked online it is subsequently never
> +subject to its memory ranges being exposed through the sysfs memory
> +hotplug api on memory block boundaries. The implementation relies on
> +this lack of user-api constraint to allow sub-section sized memory
> +ranges to be specified to :c:func:`arch_add_memory`, the top-half of
> +memory hotplug. Sub-section support allows for `PMD_SIZE` as the minimum
> +alignment granularity for :c:func:`devm_memremap_pages`.
> +
> +The users of `ZONE_DEVICE` are:

Sphinx wants an empty line here:
/home/rapoport/git/linux-docs/Documentation/vm/memory-model.rst:213: ERROR:
Unexpected indentation.

> +* pmem: Map platform persistent memory to be used as a direct-I/O target
> +  via DAX mappings.
> +
> +* hmm: Extend `ZONE_DEVICE` with `->page_fault()` and `->page_free()`
> +  event callbacks to allow a device-driver to coordinate memory management
> +  events related to device-memory, typically GPU memory. See
> +  Documentation/vm/hmm.rst.
> +
> +* p2pdma: Create `struct page` objects to allow peer devices in a
> +  PCI/-E topology to coordinate direct-DMA operations between themselves,
> +  i.e. bypass host memory.
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply

* Re: [PATCH v10 10/13] mm: Document ZONE_DEVICE memory-model implications
From: Mike Rapoport @ 2019-06-20 12:30 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-mm, akpm, linux-nvdimm, linux-kernel, Jonathan Corbet
In-Reply-To: <156092354985.979959.15763234410543451710.stgit@dwillia2-desk3.amr.corp.intel.com>

On Tue, Jun 18, 2019 at 10:52:29PM -0700, Dan Williams wrote:
> Explain the general mechanisms of 'ZONE_DEVICE' pages and list the users
> of 'devm_memremap_pages()'.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Reported-by: Mike Rapoport <rppt@linux.ibm.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

With one nit below

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
>  Documentation/vm/memory-model.rst |   39 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/vm/memory-model.rst b/Documentation/vm/memory-model.rst
> index 382f72ace1fc..e0af47e02e78 100644
> --- a/Documentation/vm/memory-model.rst
> +++ b/Documentation/vm/memory-model.rst
> @@ -181,3 +181,42 @@ that is eventually passed to vmemmap_populate() through a long chain
>  of function calls. The vmemmap_populate() implementation may use the
>  `vmem_altmap` along with :c:func:`altmap_alloc_block_buf` helper to
>  allocate memory map on the persistent memory device.
> +
> +ZONE_DEVICE
> +===========
> +The `ZONE_DEVICE` facility builds upon `SPARSEMEM_VMEMMAP` to offer
> +`struct page` `mem_map` services for device driver identified physical
> +address ranges. The "device" aspect of `ZONE_DEVICE` relates to the fact
> +that the page objects for these address ranges are never marked online,
> +and that a reference must be taken against the device, not just the page
> +to keep the memory pinned for active use. `ZONE_DEVICE`, via
> +:c:func:`devm_memremap_pages`, performs just enough memory hotplug to
> +turn on :c:func:`pfn_to_page`, :c:func:`page_to_pfn`, and
> +:c:func:`get_user_pages` service for the given range of pfns. Since the
> +page reference count never drops below 1 the page is never tracked as
> +free memory and the page's `struct list_head lru` space is repurposed
> +for back referencing to the host device / driver that mapped the memory.
> +
> +While `SPARSEMEM` presents memory as a collection of sections,
> +optionally collected into memory blocks, `ZONE_DEVICE` users have a need
> +for smaller granularity of populating the `mem_map`. Given that
> +`ZONE_DEVICE` memory is never marked online it is subsequently never
> +subject to its memory ranges being exposed through the sysfs memory
> +hotplug api on memory block boundaries. The implementation relies on
> +this lack of user-api constraint to allow sub-section sized memory
> +ranges to be specified to :c:func:`arch_add_memory`, the top-half of
> +memory hotplug. Sub-section support allows for `PMD_SIZE` as the minimum
> +alignment granularity for :c:func:`devm_memremap_pages`.
> +
> +The users of `ZONE_DEVICE` are:

Sphinx wants an empty line here:
/home/rapoport/git/linux-docs/Documentation/vm/memory-model.rst:213: ERROR:
Unexpected indentation.

> +* pmem: Map platform persistent memory to be used as a direct-I/O target
> +  via DAX mappings.
> +
> +* hmm: Extend `ZONE_DEVICE` with `->page_fault()` and `->page_free()`
> +  event callbacks to allow a device-driver to coordinate memory management
> +  events related to device-memory, typically GPU memory. See
> +  Documentation/vm/hmm.rst.
> +
> +* p2pdma: Create `struct page` objects to allow peer devices in a
> +  PCI/-E topology to coordinate direct-DMA operations between themselves,
> +  i.e. bypass host memory.
> 

-- 
Sincerely yours,
Mike.

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply

* Re: [Xen-devel] [PATCH 4/4] xen/link: Misc cleanup
From: Julien Grall @ 2019-06-20 12:29 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel
  Cc: Stefano Stabellini, Wei Liu, Jan Beulich, Roger Pau Monné
In-Reply-To: <f898001f-3cf2-699d-6bd6-cb527c87c863@citrix.com>

Hi Andrew,

On 6/19/19 10:38 PM, Andrew Cooper wrote:
> On 19/06/2019 22:30, Julien Grall wrote:
>> Hi,
>>
>> On 6/19/19 9:11 PM, Andrew Cooper wrote:
>>>    * Drop .gnu.warning.  Xen, not being a library, has no need for
>>>      __attribute__((__warning__("str"))) and isn't liable to ever gain
>>> such
>>>      annotations for link time warnings.
>>
>> What if this is introduced?
> 
> Then attempting to link Xen as a library against another object file
> won't emit the custom linker warning.

Ok, so it is not like Xen will crash.

> Its main use is for phase-out of problematic API's, but for Xen (and
> other standalone binaries) we do that by replacing problematic functions
> entirely.
>> How do we catch it?
> 
> Code review?

I usually quite like when the tools help us to catch such issue :).

Anyway, as this is not overly critical:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* [igt-dev] [PATCH i-g-t 3/3] runner/settings: Be consistent with empty blacklist
From: Arkadiusz Hiler @ 2019-06-20 12:29 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala
In-Reply-To: <20190620122902.20761-1-arkadiusz.hiler@intel.com>

If provided an empty blacklist let's fail instead of returning a value
of uninitialized variable.

Cc: Oleg Vasilev <oleg.vasilev@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 runner/settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/runner/settings.c b/runner/settings.c
index 9920e1a6..8b39c063 100644
--- a/runner/settings.c
+++ b/runner/settings.c
@@ -227,7 +227,7 @@ static bool parse_blacklist(struct regex_list *exclude_regexes,
 	FILE *f;
 	char *line = NULL;
 	size_t line_len = 0;
-	bool status;
+	bool status = false;
 
 	if ((f = fopen(blacklist_filename, "r")) == NULL) {
 		fprintf(stderr, "Cannot open blacklist file %s\n", blacklist_filename);
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related

* [igt-dev] [PATCH i-g-t 2/3] runner/resultgen: Don't pass NULL to str*() functions
From: Arkadiusz Hiler @ 2019-06-20 12:29 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala
In-Reply-To: <20190620122902.20761-1-arkadiusz.hiler@intel.com>

If we don't get values from the JSON strlen() and strcmp() on them would
dereference NULL pointer.

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 runner/resultgen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/runner/resultgen.c b/runner/resultgen.c
index 7b4cd519..58b95220 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -866,7 +866,7 @@ static void fill_from_journal(int fd,
 
 static void override_result_single(struct json_object *obj)
 {
-	const char *errtext = NULL, *result = NULL;
+	const char *errtext = "", *result = "";
 	struct json_object *textobj;
 	bool dmesgwarns = false;
 
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related

* [igt-dev] [PATCH i-g-t 1/3] runner/executor: Make sure that intervals_left is always initialized
From: Arkadiusz Hiler @ 2019-06-20 12:29 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

intervals_left got initialized only when when we had a timeout exceeding
watchdog capabilities, meaning we had to use multiple shorter intervals

by moving intervals_left = timeout_intervals down we are always
initializing it

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 runner/executor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/runner/executor.c b/runner/executor.c
index 7e5fbe8f..bce4d420 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -493,7 +493,6 @@ static int monitor_output(pid_t child,
 			if (wd_timeout - wd_extra < 0)
 				wd_extra = wd_timeout / 2;
 			timeout_intervals = timeout / (wd_timeout - wd_extra);
-			intervals_left = timeout_intervals;
 			timeout /= timeout_intervals;
 
 			if (settings->log_level >= LOG_LEVEL_VERBOSE) {
@@ -504,6 +503,8 @@ static int monitor_output(pid_t child,
 		}
 	}
 
+	intervals_left = timeout_intervals;
+
 	while (outfd >= 0 || errfd >= 0 || sigfd >= 0) {
 		struct timeval tv = { .tv_sec = timeout };
 
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related

* Re: [PATCH] mfd: da9063: occupy second I2C address, too
From: Lee Jones @ 2019-06-20 12:28 UTC (permalink / raw)
  To: Steve Twiss
  Cc: wsa+renesas@sang-engineering.com, bgolaszewski@baylibre.com,
	kieran.bingham+renesas@ideasonboard.com,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	peda@axentia.se, Support Opensource
In-Reply-To: <AM6PR10MB218184C8F2206024C6CB77EAFEE40@AM6PR10MB2181.EURPRD10.PROD.OUTLOOK.COM>

On Thu, 20 Jun 2019, Steve Twiss wrote:

> (resend because the e-mail client added HTML formatting to my last reply)
> 
> Hi Wolfram,
> 
> On Wed, 19 Jun 2019 19:18:06, Wolfram Sang wrote:
> 
> > Subject: [PATCH] mfd: da9063: occupy second I2C address, too
> > 
> > Even though we don't use it yet, we should mark the second I2C address
> > this device is listening to as used.
> 
> Sure. There is a second method for accessing higher pages of registers.
> The DA9063 Datasheet Revision 2.2, 12-Mar-2019, page 96, says this:
> 
> In 2-WIRE operation, the DA9063 offers an alternative method to access register pages 2 and 3.
> These pages can be accessed directly by incrementing the device address by one (default read
> address 0xB3; write address 0xB2). This removes the need to write to the page register before
> access to pages 2 and 3, thus reducing the traffic on the 2-WIRE bus.
> 
> Is this a safety clause? What I mean is, shouldn't the hardware design make
> sure there are not two devices located on the same I2C bus with the same slave
> address?

Why isn't this reply attached (threaded) to the patch.

Is your mailer broken?

> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Reviewed-by: Peter Rosin <peda@axentia.se>
> > Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >  drivers/mfd/da9063-i2c.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
> > index 455de74c0dd2..2133b09f6e7a 100644
> > --- a/drivers/mfd/da9063-i2c.c
> > +++ b/drivers/mfd/da9063-i2c.c
> > @@ -221,6 +221,8 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
> >  		return ret;
> >  	}
> >  
> > +	devm_i2c_new_dummy_device(&i2c->dev, i2c->adapter, i2c->addr + 1);
> > +
> >  	return da9063_device_init(da9063, i2c->irq);
> >  }
> >  
> 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH 2/2] spi-nor: intel-spi: Convert to use SPDX identifier
From: Mika Westerberg @ 2019-06-20 12:26 UTC (permalink / raw)
  To: Marek Vasut, Tudor Ambarus
  Cc: Vignesh Raghavendra, Richard Weinberger, linux-mtd, Miquel Raynal,
	Brian Norris, Mika Westerberg, David Woodhouse
In-Reply-To: <20190620122629.20838-1-mika.westerberg@linux.intel.com>

This gets rid of the license boilerplate duplicated in each file.

No functional changes intended.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/mtd/spi-nor/intel-spi-pci.c      | 5 +----
 drivers/mtd/spi-nor/intel-spi-platform.c | 5 +----
 drivers/mtd/spi-nor/intel-spi.c          | 5 +----
 drivers/mtd/spi-nor/intel-spi.h          | 5 +----
 include/linux/platform_data/intel-spi.h  | 5 +----
 5 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi-pci.c b/drivers/mtd/spi-nor/intel-spi-pci.c
index 578f0c74e536..1b9c2d99ba38 100644
--- a/drivers/mtd/spi-nor/intel-spi-pci.c
+++ b/drivers/mtd/spi-nor/intel-spi-pci.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Intel PCH/PCU SPI flash PCI driver.
  *
  * Copyright (C) 2016, Intel Corporation
  * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
 #include <linux/ioport.h>
diff --git a/drivers/mtd/spi-nor/intel-spi-platform.c b/drivers/mtd/spi-nor/intel-spi-platform.c
index 5c943df9398f..25b18804e9bb 100644
--- a/drivers/mtd/spi-nor/intel-spi-platform.c
+++ b/drivers/mtd/spi-nor/intel-spi-platform.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Intel PCH/PCU SPI flash platform driver.
  *
  * Copyright (C) 2016, Intel Corporation
  * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
 #include <linux/ioport.h>
diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index d60cbf23d9aa..021cef930f9f 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Intel PCH/PCU SPI flash driver.
  *
  * Copyright (C) 2016, Intel Corporation
  * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
 #include <linux/err.h>
diff --git a/drivers/mtd/spi-nor/intel-spi.h b/drivers/mtd/spi-nor/intel-spi.h
index 5ab7dc250050..b03bf296fda3 100644
--- a/drivers/mtd/spi-nor/intel-spi.h
+++ b/drivers/mtd/spi-nor/intel-spi.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Intel PCH/PCU SPI flash driver.
  *
  * Copyright (C) 2016, Intel Corporation
  * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
 #ifndef INTEL_SPI_H
diff --git a/include/linux/platform_data/intel-spi.h b/include/linux/platform_data/intel-spi.h
index 942b0c3f8f08..001f377fb5ef 100644
--- a/include/linux/platform_data/intel-spi.h
+++ b/include/linux/platform_data/intel-spi.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Intel PCH/PCU SPI flash driver.
  *
  * Copyright (C) 2016, Intel Corporation
  * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
 #ifndef INTEL_SPI_PDATA_H
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related

* Re: [PATCH v17 04/10] acpi: add build_append_ghes_generic_data() helper for Generic Error Data Entry
From: Igor Mammedov @ 2019-06-20 12:28 UTC (permalink / raw)
  To: Dongjiu Geng
  Cc: pbonzini, mst, shannon.zhaosl, peter.maydell, lersek, james.morse,
	mtosatti, rth, ehabkost, zhengxiang9, jonathan.cameron, xuwei5,
	kvm, qemu-devel, qemu-arm, linuxarm
In-Reply-To: <1557832703-42620-5-git-send-email-gengdongjiu@huawei.com>

On Tue, 14 May 2019 04:18:17 -0700
Dongjiu Geng <gengdongjiu@huawei.com> wrote:

> It will help to add Generic Error Data Entry to ACPI tables
> without using packed C structures and avoid endianness
> issues as API doesn't need explicit conversion.
> 
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
>  hw/acpi/aml-build.c         | 32 ++++++++++++++++++++++++++++++++
>  include/hw/acpi/aml-build.h |  6 ++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index fb53f21..102a288 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -296,6 +296,38 @@ void build_append_ghes_notify(GArray *table, const uint8_t type,
>          build_append_int_noprefix(table, error_threshold_window, 4);
>  }
>  
> +/* Generic Error Data Entry
> + * ACPI 4.0: 17.3.2.6.1 Generic Error Data
> + */
> +void build_append_ghes_generic_data(GArray *table, const char *section_type,
s/build_append_ghes_generic_data/build_append_ghes_generic_error_data/

> +                                    uint32_t error_severity, uint16_t revision,
> +                                    uint8_t validation_bits, uint8_t flags,
> +                                    uint32_t error_data_length, uint8_t *fru_id,
> +                                    uint8_t *fru_text, uint64_t time_stamp)
checkpatch probably will complain due to too long lines
you can use:
void build_append_ghe...
         uint32_t error_severity, uint16_t revision,
         ...

> +{
> +    int i;
> +
> +    for (i = 0; i < 16; i++) {
> +        build_append_int_noprefix(table, section_type[i], 1);
                                            ^^^
use QemuUUID instead, see vmgenid_build_acpi

> +    }
> +
> +    build_append_int_noprefix(table, error_severity, 4);
> +    build_append_int_noprefix(table, revision, 2);
> +    build_append_int_noprefix(table, validation_bits, 1);
> +    build_append_int_noprefix(table, flags, 1);
> +    build_append_int_noprefix(table, error_data_length, 4);
> +
> +    for (i = 0; i < 16; i++) {
> +        build_append_int_noprefix(table, fru_id[i], 1);
same as section_type

> +    }
> +
> +    for (i = 0; i < 20; i++) {
> +        build_append_int_noprefix(table, fru_text[i], 1);
> +    }
instead of loop use g_array_insert_vals()

> +
> +    build_append_int_noprefix(table, time_stamp, 8);
that's not part of 'Table 17-13'
where does it come from?

> +}
> +
>  /*
>   * Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a dword,
>   * and return the offset to 0x00000000 for runtime patching.
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 90c8ef8..a71db2f 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -419,6 +419,12 @@ void build_append_ghes_notify(GArray *table, const uint8_t type,
>                                uint32_t error_threshold_value,
>                                uint32_t error_threshold_window);
>  
> +void build_append_ghes_generic_data(GArray *table, const char *section_type,
> +                                    uint32_t error_severity, uint16_t revision,
> +                                    uint8_t validation_bits, uint8_t flags,
> +                                    uint32_t error_data_length, uint8_t *fru_id,
> +                                    uint8_t *fru_text, uint64_t time_stamp);
> +
>  void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>                         uint64_t len, int node, MemoryAffinityFlags flags);
>  

^ permalink raw reply

* [Bug 110949] Continuious warnings from agd5f 5.3-wip branch
From: bugzilla-daemon @ 2019-06-20 12:28 UTC (permalink / raw)
  To: dri-devel
In-Reply-To: <bug-110949-502@http.bugs.freedesktop.org/>


[-- Attachment #1.1: Type: text/plain, Size: 399 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=110949

--- Comment #3 from Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> ---
Seems like there's still issues with dropping the check depending on the ASIC
revision and probably userspace that's being used.

I can revert this for now while investigating the issue.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1191 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 04/10] ARM: dts: rockchip: add startup delay to rk3288-veyron panel-regulators
From: Heiko Stübner @ 2019-06-20 12:28 UTC (permalink / raw)
  To: Doug Anderson
  Cc: LKML, open list:ARM/Rockchip SoC..., Matthias Kaehlcke,
	Yakir Yang, Enric Balletbo i Serra, Linux ARM
In-Reply-To: <CAD=FV=U23+5pcze=6zDTx0dAYF8HTmbR8s8zem93VhgYgaZeGQ@mail.gmail.com>

Hi Doug,

Am Donnerstag, 20. Juni 2019, 03:27:55 CEST schrieb Doug Anderson:
> On Wed, Fri, 18 Mar 2016 Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > The panels need a bit of time to actually turn on. If this isn't
> > observed, this results in problems when trying talk to the panels
> > and thus produces detection errors. 100ms seem to be a safe value
> > for the time being.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  arch/arm/boot/dts/rk3288-veyron-jaq.dts    | 1 +
> >  arch/arm/boot/dts/rk3288-veyron-jerry.dts  | 1 +
> >  arch/arm/boot/dts/rk3288-veyron-minnie.dts | 1 +
> >  arch/arm/boot/dts/rk3288-veyron-speedy.dts | 1 +
> >  4 files changed, 4 insertions(+)
> 
> I know it was 3 years ago, but any idea how to reproduce the problems
> you were seeing without this patch?  I believe the downstream kernel
> never had any delay like this and I'm not aware of any issues.
>
> I wonder if the need for this extra 100 ms delay is no longer there
> now that we have:
> 
> 3157694d8c7f pwm-backlight: Add support for PWM delays proprieties.
> 5fb5caee92ba pwm-backlight: Enable/disable the PWM before/after LCD
> enable toggle.
> 6d5922dd0d60 ARM: dts: rockchip: set PWM delay backlight settings for Veyron

I just did a non-scientific test on my jerry+minnie and yes, simply
reverting that patch does not seem to affect display bringup and I still
get a prompt.

So I guess we could just revert that patch in light of the changes.
[patches welcome ;-) ]

Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [Qemu-devel] [PATCH v5 3/3] hw/arm/virt: Use edk2_add_host_crypto_policy()
From: Philippe Mathieu-Daudé @ 2019-06-20 12:21 UTC (permalink / raw)
  To: qemu-devel, Laszlo Ersek
  Cc: Peter Maydell, Andrew Jones, Eduardo Habkost, Michael S. Tsirkin,
	Markus Armbruster, qemu-arm, Paolo Bonzini,
	Philippe Mathieu-Daudé
In-Reply-To: <20190620122132.10075-1-philmd@redhat.com>

Enable the EDK2 Crypto Policy features on the Virt machine.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/virt.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 431e2900fd..15727b3d59 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -59,6 +59,7 @@
 #include "hw/intc/arm_gicv3_common.h"
 #include "kvm_arm.h"
 #include "hw/firmware/smbios.h"
+#include "hw/firmware/uefi_edk2.h"
 #include "qapi/visitor.h"
 #include "standard-headers/linux/input.h"
 #include "hw/arm/smmuv3.h"
@@ -1355,6 +1356,11 @@ static void virt_build_smbios(VirtMachineState *vms)
     }
 }
 
+static void virt_uefi_setup(VirtMachineState *vms)
+{
+    edk2_add_host_crypto_policy(vms->fw_cfg);
+}
+
 static
 void virt_machine_done(Notifier *notifier, void *data)
 {
@@ -1383,6 +1389,7 @@ void virt_machine_done(Notifier *notifier, void *data)
 
     virt_acpi_setup(vms);
     virt_build_smbios(vms);
+    virt_uefi_setup(vms);
 }
 
 static uint64_t virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
-- 
2.20.1


^ permalink raw reply related

* Re: [Qemu-devel] [PATCH v3 2/4] hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()
From: Philippe Mathieu-Daudé @ 2019-06-20 12:07 UTC (permalink / raw)
  To: Laszlo Ersek, Gerd Hoffmann, Michael S. Tsirkin, qemu-devel
  Cc: Daniel P . Berrange, Markus Armbruster
In-Reply-To: <124e54f9-c7e1-0157-61f1-673154872749@redhat.com>

Hi Laszlo,

On 3/13/19 11:11 AM, Laszlo Ersek wrote:
> On 03/13/19 10:43, Laszlo Ersek wrote:
>> On 03/10/19 01:47, Philippe Mathieu-Daudé wrote:
>>> The Edk2Crypto object is used to hold configuration values specific
>>> to EDK2.
>>>
>>> The edk2_add_host_crypto_policy() function loads crypto policies
>>> from the host, and register them as fw_cfg named file items.
>>> So far only the 'https' policy is supported.
>>>
>>> A usercase example is the 'HTTPS Boof' feature of OVMF [*].
>>>
>>> Usage example:
>>>
>>>   $ qemu-system-x86_64 \
>>>       --object edk2_crypto,id=https,\
>>>               ciphers=/etc/crypto-policies/back-ends/openssl.config,\
>>>               cacerts=/etc/pki/ca-trust/extracted/edk2/cacerts.bin
>>>
>>> (On Fedora these files are provided by the ca-certificates and
>>> crypto-policies packages).
>>>
>>> [*]: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>> v3:
>>> - '-object' -> '--object' in commit description (Eric)
>>> - reworded the 'TODO: g_free' comment
>>> ---
>>>  MAINTAINERS                             |   8 ++
>>>  hw/Makefile.objs                        |   1 +
>>>  hw/firmware/Makefile.objs               |   1 +
>>>  hw/firmware/uefi_edk2_crypto_policies.c | 177 ++++++++++++++++++++++++
>>>  include/hw/firmware/uefi_edk2.h         |  28 ++++
>>>  5 files changed, 215 insertions(+)
>>>  create mode 100644 hw/firmware/Makefile.objs
>>>  create mode 100644 hw/firmware/uefi_edk2_crypto_policies.c
>>>  create mode 100644 include/hw/firmware/uefi_edk2.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index cf09a4c127..70122b3d0d 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -2206,6 +2206,14 @@ F: include/hw/i2c/smbus_master.h
>>>  F: include/hw/i2c/smbus_slave.h
>>>  F: include/hw/i2c/smbus_eeprom.h
>>>  
>>> +EDK2 Firmware
>>> +M: Laszlo Ersek <lersek@redhat.com>
>>> +M: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> +S: Maintained
>>> +F: docs/interop/firmware.json
>>> +F: hw/firmware/uefi_edk2_crypto_policies.c
>>> +F: include/hw/firmware/uefi_edk2.h
>>> +
>>
>> I'm not happy with this.
>>
>> First, "docs/interop/firmware.json" is meant for more than just EDK2. We
>> shouldn't list it in a section called "EDK2 Firmware". I can't suggest
>> an alternative (MAINTAINERS is *huge* -- 2500+ lines), but this one
>> would be misleading.
>>
>> Second, we expose fw_cfg files to edk2 platform firmware from a bunch of
>> other places. For example -- and in this case I do mean to provide a
>> complex example! --, see "etc/smi/supported-features",
>> "etc/smi/requested-features", and "etc/smi/features-ok", in file
>> "hw/isa/lpc_ich9.c". I'm unconvinced that the present feature merits new
>> directories and new files.
>>
>> Then again, I also don't know where to put the logic. I guess I'll have
>> to defer to more experienced reviewers.
>>
>> [snipping lots of QOM boilerplate]
>>
>>> +void edk2_add_host_crypto_policy(FWCfgState *fw_cfg)
>>> +{
>>> +    Edk2Crypto *s;
>>> +
>>> +    s = edk2_crypto_by_id("https", NULL);
>>> +    if (!s) {
>>> +        return;
>>> +    }
>>> +
>>> +    if (s->ciphers_path) {
>>> +        /*
>>> +         * Note:
>>> +         * Unlike with fw_cfg_add_file() where the allocated data has
>>> +         * to be valid for the lifetime of the FwCfg object, there is
>>> +         * no such contract interface with fw_cfg_add_file_from_host().
>>> +         * It would be easier that the FwCfg object keeps reference of
>>> +         * its allocated memory and releases it when destroy, but it
>>> +         * currently doesn't. Meanwhile we simply add this TODO comment.
>>> +         */
>>> +        fw_cfg_add_file_from_host(fw_cfg, "etc/edk2/https/ciphers",
>>> +                                  s->ciphers_path, NULL);
>>> +    }
>>> +
>>> +    if (s->cacerts_path) {
>>> +        /*
>>> +         * TODO: g_free the returned pointer
>>> +         * (see previous comment for ciphers_path in this function).
>>> +         */
>>> +        fw_cfg_add_file_from_host(fw_cfg, "etc/edk2/https/cacerts",
>>> +                                  s->cacerts_path, NULL);
>>> +    }
>>> +}
>>
>> Shouldn't we do some error checking here?
>>
>> I mean, printing an error message in fw_cfg_add_file_from_host(), and
>> then continuing without exposing the named files in question to the
>> firmware, could be OK if this was a "default on" feature. But (IIUC)
>> here the user provided an explicit "-object" option, and we've just
>> failed to construct the object. Doesn't such a situation usually prevent
>> QEMU startup?
> 
> Wait, I could be totally confused here. (Returning to this patch after
> seeing the rest of the series.)
> 
> Is it actually the case that the Edk2Crypto object holds nothing more
> than two pathnames -- and so its construction can virtually never fail?
> While the actual fw_cfg population occurs separately, in a machine_done
> notifier?
> 
> If that's the case, I don't think it's the right approach. Reading the
> host files, and populating fw_cfg with them, should be part of the
> object construction. And if those steps fail, the object should not be
> possible to construct.
> 
> We did something similar with the vmgenid device [hw/acpi/vmgenid.c], if
> I remember correctly. It also has a dependency on fw_cfg...
> 
> Ahh, no, I'm absolutely wrong about that. vmgenid_realize() doesn't do
> anything with fw_cfg. Instead, we have acpi_setup() in
> "hw/i386/acpi-build.c", which calls find_vmgenid_dev() and
> vmgenid_add_fw_cfg(). And acpi_setup() is certainly called from
> pc_machine_done().
> 
> In other words, the pattern that you use here matches existing practice.
> Realize the device (or object) first, then add the fw_cfg thingies in
> the "machine done" callback. OK.
> 
> *Still*, I would like to see better error handling/reporting (or an
> explanation why I'm wrong). How about reworking the edk2crypto class
> itself -- it shouldn't just hold the pathnames of the files, but also
> their contents. That is:
> 
> - g_file_get_contents() should be called in the realize method
> - the object would own the file contents
> - the realize method would ensure that there wouldn't be any other
> instance of the class (i.e. that it would be a singleton -- see the same
> idea in vmgenid)
> - there would be no need for the fw_cfg_add_file_from_host() API
> - the machine done notifier would be extended to locate the object
> (there could be zero or one instances), and if the one instance were
> found, the machine done callback would hook the file contents into
> fw_cfg. fw_cfg_add_file() cannot fail, so no errors to report at this stage.
> 
> Again I think this would follow the pattern from vmgenid.

I want to say I am impressed by your deep review. Your design is
obviously way cleaner/safer. I think I was missing some part of the big
picture here, thank you for your detailed comments!

I did not know how vmgenid is processed. The only difference is I don't
want the edk2crypto class to be a device, but rather a simple user
object, and we already have an interface that does that:
TYPE_USER_CREATABLE. Its UserCreatableClass::complete() method is
similar to DeviceClass::realize() in managing errors at object
instantiation, so the machine done notifier never fails.
I'll respin.

Regards,

Phil.


^ permalink raw reply

* Re: [PATCH 04/10] ARM: dts: rockchip: add startup delay to rk3288-veyron panel-regulators
From: Heiko Stübner @ 2019-06-20 12:28 UTC (permalink / raw)
  To: Doug Anderson
  Cc: open list:ARM/Rockchip SoC..., Linux ARM, Yakir Yang,
	Matthias Kaehlcke, Enric Balletbo i Serra, LKML
In-Reply-To: <CAD=FV=U23+5pcze=6zDTx0dAYF8HTmbR8s8zem93VhgYgaZeGQ@mail.gmail.com>

Hi Doug,

Am Donnerstag, 20. Juni 2019, 03:27:55 CEST schrieb Doug Anderson:
> On Wed, Fri, 18 Mar 2016 Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > The panels need a bit of time to actually turn on. If this isn't
> > observed, this results in problems when trying talk to the panels
> > and thus produces detection errors. 100ms seem to be a safe value
> > for the time being.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  arch/arm/boot/dts/rk3288-veyron-jaq.dts    | 1 +
> >  arch/arm/boot/dts/rk3288-veyron-jerry.dts  | 1 +
> >  arch/arm/boot/dts/rk3288-veyron-minnie.dts | 1 +
> >  arch/arm/boot/dts/rk3288-veyron-speedy.dts | 1 +
> >  4 files changed, 4 insertions(+)
> 
> I know it was 3 years ago, but any idea how to reproduce the problems
> you were seeing without this patch?  I believe the downstream kernel
> never had any delay like this and I'm not aware of any issues.
>
> I wonder if the need for this extra 100 ms delay is no longer there
> now that we have:
> 
> 3157694d8c7f pwm-backlight: Add support for PWM delays proprieties.
> 5fb5caee92ba pwm-backlight: Enable/disable the PWM before/after LCD
> enable toggle.
> 6d5922dd0d60 ARM: dts: rockchip: set PWM delay backlight settings for Veyron

I just did a non-scientific test on my jerry+minnie and yes, simply
reverting that patch does not seem to affect display bringup and I still
get a prompt.

So I guess we could just revert that patch in light of the changes.
[patches welcome ;-) ]

Heiko

^ permalink raw reply

* [Qemu-arm] [PATCH v5 2/3] hw/i386: Use edk2_add_host_crypto_policy()
From: Philippe Mathieu-Daudé @ 2019-06-20 12:21 UTC (permalink / raw)
  To: qemu-devel, Laszlo Ersek
  Cc: Peter Maydell, Andrew Jones, Eduardo Habkost, Michael S. Tsirkin,
	Markus Armbruster, qemu-arm, Marcel Apfelbaum, Paolo Bonzini,
	Philippe Mathieu-Daudé, Richard Henderson
In-Reply-To: <20190620122132.10075-1-philmd@redhat.com>

Enable the EDK2 Crypto Policy features on the PC machine.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/pc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2c5446b095..fe99ebfe3d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -39,6 +39,7 @@
 #include "hw/nvram/fw_cfg.h"
 #include "hw/timer/hpet.h"
 #include "hw/firmware/smbios.h"
+#include "hw/firmware/uefi_edk2.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "multiboot.h"
@@ -1049,6 +1050,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
     return fw_cfg;
 }
 
+static void pc_uefi_setup(PCMachineState *pcms)
+{
+    edk2_add_host_crypto_policy(pcms->fw_cfg);
+}
+
 static long get_file_size(FILE *f)
 {
     long where, size;
@@ -1653,6 +1659,7 @@ void pc_machine_done(Notifier *notifier, void *data)
     if (pcms->fw_cfg) {
         pc_build_smbios(pcms);
         pc_build_feature_control_file(pcms);
+        pc_uefi_setup(pcms);
         /* update FW_CFG_NB_CPUS to account for -device added CPUs */
         fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
     }
-- 
2.20.1


^ permalink raw reply related

* Re: [dpdk-dev] [PATCH v1] lib/ipsec: add support for header construction
From: Akhil Goyal @ 2019-06-20 12:27 UTC (permalink / raw)
  To: Ananyev, Konstantin, Kovacevic, Marko, dev@dpdk.org; +Cc: Zhang, Roy Fan
In-Reply-To: <2601191342CEEE43887BDE71AB9772580161635D40@irsmsx105.ger.corp.intel.com>

Hi Marko,

Could you please address to the comments from Konstantin? We have an RC1 date coming.

Thanks,
Akhil

> Hi,
> 
> >
> > Add support for RFC 4301(5.1.2) to update of
> > Type of service field and Traffic class field
> > bits inside ipv4/ipv6 packets for outbound cases
> > and inbound cases which deals with the update of
> > the DSCP/ENC bits inside each of the fields.
> >
> > Signed-off-by: Marko Kovacevic <marko.kovacevic@intel.com>
> > ---
> >  examples/ipsec-secgw/sa.c          |   2 +
> >  lib/librte_ipsec/esp_inb.c         |  14 ++++-
> >  lib/librte_ipsec/esp_outb.c        |   4 +-
> >  lib/librte_ipsec/iph.h             | 119 +++++++++++++++++++++++++++++++++++-
> -
> >  lib/librte_ipsec/rte_ipsec_sa.h    |  25 ++++++++
> >  lib/librte_ipsec/sa.c              |  17 ++++++
> >  lib/librte_ipsec/sa.h              |   2 +
> >  lib/librte_net/rte_ip.h            |   8 +++
> >  lib/librte_security/rte_security.h |   9 +++
> >  9 files changed, 191 insertions(+), 9 deletions(-)
> 
> Looks good in general, some generic comments:
> - I think it is better to split the patch into few sub-pathces:
>   One for rte_security, second for rte_net, third - rte_ipsec, forth -
> examples/ipsec-secgw
> - Would be good to add support for other options too (ttl, etc.)
> - Would be good to add new test-case for it into examples/ipsec-secgw/test/
> 
> Plus few nits in the code below.
> Konstantin
> 

^ permalink raw reply

* [PATCH 1/2] spi-nor: intel-spi: Add support for Intel Elkhart Lake SPI serial flash
From: Mika Westerberg @ 2019-06-20 12:26 UTC (permalink / raw)
  To: Marek Vasut, Tudor Ambarus
  Cc: Vignesh Raghavendra, Richard Weinberger, linux-mtd, Miquel Raynal,
	Brian Norris, Mika Westerberg, David Woodhouse

Intel Elkhart Lake has the same SPI serial flash controller as Ice Lake.
Add Elkhart Lake PCI ID to the driver list of supported devices.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/mtd/spi-nor/intel-spi-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/spi-nor/intel-spi-pci.c b/drivers/mtd/spi-nor/intel-spi-pci.c
index bfbfc17ed6aa..578f0c74e536 100644
--- a/drivers/mtd/spi-nor/intel-spi-pci.c
+++ b/drivers/mtd/spi-nor/intel-spi-pci.c
@@ -67,6 +67,7 @@ static const struct pci_device_id intel_spi_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x18e0), (unsigned long)&bxt_info },
 	{ PCI_VDEVICE(INTEL, 0x19e0), (unsigned long)&bxt_info },
 	{ PCI_VDEVICE(INTEL, 0x34a4), (unsigned long)&bxt_info },
+	{ PCI_VDEVICE(INTEL, 0x4b24), (unsigned long)&bxt_info },
 	{ PCI_VDEVICE(INTEL, 0xa1a4), (unsigned long)&bxt_info },
 	{ PCI_VDEVICE(INTEL, 0xa224), (unsigned long)&bxt_info },
 	{ },
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.