* [RFC Patch v3] binman: add support for creating dummy files for external blobs
From: Heiko Thiery @ 2022-01-05 12:58 UTC (permalink / raw)
To: u-boot
Cc: Simon Glass, Stefano Babic, Fabio Estevam, Michael Walle,
Tom Rini, Wolfgang Denk, Heiko Thiery
While converting to binman for an imx8mq board, it has been found that
building in the u-boot CI fails. This is because an imx8mq requires an
external binary (signed_hdmi_imx8m.bin). If this file cannot be found
mkimage fails.
To be able to build this board in the u-boot CI a binman option
(--fake-ext-blobs) is introduced that can be switched on via the u-boot
makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are
created.
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
v3:
- add CheckFakedBlobs() and print a list of faked files at the end
- add unittest
v2:
- pass allow_fake_blobs to ProcessImage()
- set AllowAllowFakeBlob() to images/entries
- create fake blob in Entry_blot.ObtainContents() when file is missing and
creation is allowed
still missing:
- unittest
- option to set BINMAN_FAKE_EXT_BLOBS in Makefile via environment
variable. With that we could simply set this env variable in the CI
(gitlab-ci.yml) with adding support to buildman.
Makefile | 1 +
tools/binman/cmdline.py | 2 ++
tools/binman/control.py | 26 +++++++++++++++++++-------
tools/binman/entry.py | 23 +++++++++++++++++++++++
tools/binman/etype/blob.py | 18 ++++++++++++++++++
tools/binman/etype/blob_ext.py | 8 ++++++++
tools/binman/etype/mkimage.py | 20 ++++++++++++++++++++
tools/binman/etype/section.py | 20 ++++++++++++++++++++
tools/binman/ftest.py | 13 ++++++++++++-
tools/binman/test/203_fake_blob.dts | 14 ++++++++++++++
10 files changed, 137 insertions(+), 8 deletions(-)
create mode 100644 tools/binman/test/203_fake_blob.dts
diff --git a/Makefile b/Makefile
index ae9bfab91a..63d286a4c1 100644
--- a/Makefile
+++ b/Makefile
@@ -1315,6 +1315,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
-a tpl-dtb=$(CONFIG_TPL_OF_REAL) \
+ $(if $(BINMAN_FAKE_EXT_BLOBS),--fake-ext-blobs) \
$(BINMAN_$(@F))
OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index e73ff78095..700e5a29a5 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -52,6 +52,8 @@ controlled by a description in the board device tree.'''
help='Configuration file (.dtb) to use')
build_parser.add_argument('--fake-dtb', action='store_true',
help='Use fake device tree contents (for testing only)')
+ build_parser.add_argument('--fake-ext-blobs', action='store_true',
+ help='Create fake ext blobs with dummy content (for testing only)')
build_parser.add_argument('-i', '--image', type=str, action='append',
help='Image filename to build (if not specified, build all)')
build_parser.add_argument('-I', '--indir', action='append',
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 304fc70f56..ec0905602e 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -479,7 +479,8 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded):
def ProcessImage(image, update_fdt, write_map, get_contents=True,
- allow_resize=True, allow_missing=False):
+ allow_resize=True, allow_missing=False,
+ allow_fake_blobs=False):
"""Perform all steps for this image, including checking and # writing it.
This means that errors found with a later image will be reported after
@@ -495,12 +496,15 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
allow_resize: True to allow entries to change size (this does a re-pack
of the entries), False to raise an exception
allow_missing: Allow blob_ext objects to be missing
+ allow_fake_blobs: Allow blob_ext objects to be faked with dummy files
Returns:
- True if one or more external blobs are missing, False if all are present
+ True if one or more external blobs are missing or faked,
+ False if all are present
"""
if get_contents:
image.SetAllowMissing(allow_missing)
+ image.SetAllowFakeBlob(allow_fake_blobs)
image.GetEntryContents()
image.GetEntryOffsets()
@@ -549,7 +553,13 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
tout.Warning("Image '%s' is missing external blobs and is non-functional: %s" %
(image.name, ' '.join([e.name for e in missing_list])))
_ShowHelpForMissingBlobs(missing_list)
- return bool(missing_list)
+ faked_blob_list = []
+ image.CheckFakedBlobs(faked_blob_list)
+ if faked_blob_list:
+ tout.Warning("Image '%s:%s' has faked external blobs and is non-functional: %s" %
+ (image.name, image.image_name,
+ ' '.join([e.GetDefaultFilename() for e in faked_blob_list])))
+ return bool(missing_list) or bool(faked_blob_list)
def Binman(args):
@@ -636,13 +646,15 @@ def Binman(args):
images = PrepareImagesAndDtbs(dtb_fname, args.image,
args.update_fdt, use_expanded)
+
if args.test_section_timeout:
# Set the first image to timeout, used in testThreadTimeout()
images[list(images.keys())[0]].test_section_timeout = True
- missing = False
+ invalid = False
for image in images.values():
- missing |= ProcessImage(image, args.update_fdt, args.map,
- allow_missing=args.allow_missing)
+ invalid |= ProcessImage(image, args.update_fdt, args.map,
+ allow_missing=args.allow_missing,
+ allow_fake_blobs=args.fake_ext_blobs)
# Write the updated FDTs to our output files
for dtb_item in state.GetAllFdts():
@@ -652,7 +664,7 @@ def Binman(args):
data = state.GetFdtForEtype('u-boot-dtb').GetContents()
elf.UpdateFile(*elf_params, data)
- if missing:
+ if invalid:
tout.Warning("\nSome images are invalid")
# Use this to debug the time take to pack the image
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 70222718ea..bf99c9ea10 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -70,6 +70,8 @@ class Entry(object):
missing: True if this entry is missing its contents
allow_missing: Allow children of this entry to be missing (used by
subclasses such as Entry_section)
+ allow_fake: Allow creating a dummy fake file if the blob file is not
+ available. This is mainly used for testing.
external: True if this entry contains an external binary blob
"""
def __init__(self, section, etype, node, name_prefix=''):
@@ -98,8 +100,10 @@ class Entry(object):
self._expand_size = False
self.compress = 'none'
self.missing = False
+ self.faked = False
self.external = False
self.allow_missing = False
+ self.allow_fake = False
@staticmethod
def Lookup(node_path, etype, expanded):
@@ -898,6 +902,14 @@ features to produce new behaviours.
# This is meaningless for anything other than sections
pass
+ def SetAllowFakeBlob(self, allow_fake):
+ """Set whether a section allows to create a fake blob
+
+ Args:
+ allow_fake: True if allowed, False if not allowed
+ """
+ pass
+
def CheckMissing(self, missing_list):
"""Check if any entries in this section have missing external blobs
@@ -909,6 +921,17 @@ features to produce new behaviours.
if self.missing:
missing_list.append(self)
+ def CheckFakedBlobs(self, faked_blobs_list):
+ """Check if any entries in this section have faked external blobs
+
+ If there are faked blobs, the entries are added to the list
+
+ Args:
+ fake_blobs_list: List of Entry objects to be added to
+ """
+ if self.faked:
+ faked_blobs_list.append(self)
+
def GetAllowMissing(self):
"""Get whether a section allows missing external blobs
diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
index fae86ca3ec..6e63d777eb 100644
--- a/tools/binman/etype/blob.py
+++ b/tools/binman/etype/blob.py
@@ -5,6 +5,8 @@
# Entry-type module for blobs, which are binary objects read from files
#
+import pathlib
+
from binman.entry import Entry
from binman import state
from dtoc import fdt_util
@@ -36,6 +38,11 @@ class Entry_blob(Entry):
self._filename = fdt_util.GetString(self._node, 'filename', self.etype)
def ObtainContents(self):
+ if self.allow_fake and not pathlib.Path(self._filename).is_file():
+ with open(self._filename, "wb") as out:
+ out.truncate(1024)
+ self.faked = True
+
self._filename = self.GetDefaultFilename()
self._pathname = tools.GetInputFilename(self._filename,
self.external and self.section.GetAllowMissing())
@@ -75,3 +82,14 @@ class Entry_blob(Entry):
def ProcessContents(self):
# The blob may have changed due to WriteSymbols()
return self.ProcessContentsUpdate(self.data)
+
+ def CheckFakedBlobs(self, faked_blobs_list):
+ """Check if any entries in this section have faked external blobs
+
+ If there are faked blobs, the entries are added to the list
+
+ Args:
+ fake_blobs_list: List of Entry objects to be added to
+ """
+ if self.faked:
+ faked_blobs_list.append(self)
diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py
index d6b0ca17c3..fba6271de2 100644
--- a/tools/binman/etype/blob_ext.py
+++ b/tools/binman/etype/blob_ext.py
@@ -26,3 +26,11 @@ class Entry_blob_ext(Entry_blob):
def __init__(self, section, etype, node):
Entry_blob.__init__(self, section, etype, node)
self.external = True
+
+ def SetAllowFakeBlob(self, allow_fake):
+ """Set whether the entry allows to create a fake blob
+
+ Args:
+ allow_fake_blob: True if allowed, False if not allowed
+ """
+ self.allow_fake = allow_fake
diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index e49977522e..80fdce0b14 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -61,3 +61,23 @@ class Entry_mkimage(Entry):
entry = Entry.Create(self, node)
entry.ReadNode()
self._mkimage_entries[entry.name] = entry
+
+ def SetAllowFakeBlob(self, allow_fake):
+ """Set whether the sub nodes allows to create a fake blob
+
+ Args:
+ allow_fake: True if allowed, False if not allowed
+ """
+ for entry in self._mkimage_entries.values():
+ entry.SetAllowFakeBlob(allow_fake)
+
+ def CheckFakedBlobs(self, faked_blobs_list):
+ """Check if any entries in this section have faked external blobs
+
+ If there are faked blobs, the entries are added to the list
+
+ Args:
+ faked_blobs_list: List of Entry objects to be added to
+ """
+ for entry in self._mkimage_entries.values():
+ entry.CheckFakedBlobs(faked_blobs_list)
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index e2949fc916..4e423855d9 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -689,6 +689,15 @@ class Entry_section(Entry):
for entry in self._entries.values():
entry.SetAllowMissing(allow_missing)
+ def SetAllowFakeBlob(self, allow_fake):
+ """Set whether a section allows to create a fake blob
+
+ Args:
+ allow_fake_blob: True if allowed, False if not allowed
+ """
+ for entry in self._entries.values():
+ entry.SetAllowFakeBlob(allow_fake)
+
def CheckMissing(self, missing_list):
"""Check if any entries in this section have missing external blobs
@@ -700,6 +709,17 @@ class Entry_section(Entry):
for entry in self._entries.values():
entry.CheckMissing(missing_list)
+ def CheckFakedBlobs(self, faked_blobs_list):
+ """Check if any entries in this section have faked external blobs
+
+ If there are faked blobs, the entries are added to the list
+
+ Args:
+ fake_blobs_list: List of Entry objects to be added to
+ """
+ for entry in self._entries.values():
+ entry.CheckFakedBlobs(faked_blobs_list)
+
def _CollectEntries(self, entries, entries_by_name, add_entry):
"""Collect all the entries in an section
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 6be003786e..79a91d1780 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -308,7 +308,7 @@ class TestFunctional(unittest.TestCase):
def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False,
entry_args=None, images=None, use_real_dtb=False,
use_expanded=False, verbosity=None, allow_missing=False,
- extra_indirs=None, threads=None,
+ allow_fake_blobs=False, extra_indirs=None, threads=None,
test_section_timeout=False, update_fdt_in_elf=None):
"""Run binman with a given test file
@@ -331,6 +331,7 @@ class TestFunctional(unittest.TestCase):
verbosity: Verbosity level to use (0-3, None=don't set it)
allow_missing: Set the '--allow-missing' flag so that missing
external binaries just produce a warning instead of an error
+ allow_fake_blobs: Set the '--fake-ext-blobs' flag
extra_indirs: Extra input directories to add using -I
threads: Number of threads to use (None for default, 0 for
single-threaded)
@@ -369,6 +370,8 @@ class TestFunctional(unittest.TestCase):
args.append('-a%s=%s' % (arg, value))
if allow_missing:
args.append('-M')
+ if allow_fake_blobs:
+ args.append('--fake-ext-blobs')
if update_fdt_in_elf:
args += ['--update-fdt-in-elf', update_fdt_in_elf]
if images:
@@ -4661,6 +4664,14 @@ class TestFunctional(unittest.TestCase):
str(e.exception),
"Not enough space in '.*u_boot_binman_embed_sm' for data length.*")
+ def testFakeBlob(self):
+ """Test handling of faking an external blob"""
+ with test_util.capture_sys_output() as (stdout, stderr):
+ self._DoTestFile('203_fake_blob.dts', allow_missing=True, allow_fake_blobs=True)
+ err = stderr.getvalue()
+ self.assertRegex(err,
+ "Image '.*' has faked external blobs and is non-functional: .*")
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/203_fake_blob.dts b/tools/binman/test/203_fake_blob.dts
new file mode 100644
index 0000000000..9222f5ce6c
--- /dev/null
+++ b/tools/binman/test/203_fake_blob.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ blob-ext {
+ filename = "faking";
+ };
+ };
+};
--
2.30.2
^ permalink raw reply related
* RE: [PATCH v2 1/5] RISC-V: larger and more consistent register set for 'info registers'
From: Schwarz, Konrad @ 2022-01-05 12:38 UTC (permalink / raw)
To: Richard Henderson, qemu-devel@nongnu.org
Cc: Palmer Dabbelt, Bin Meng, Alistair Francis, Ralf Ramsauer
In-Reply-To: <18d1fbb3-ddff-6d3c-55a1-cbec27ac9f1e@linaro.org>
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Tuesday, January 4, 2022 21:57
> Subject: Re: [PATCH v2 1/5] RISC-V: larger and more consistent register set for 'info registers'
>
> On 1/4/22 7:51 AM, Konrad Schwarz wrote:
> > static const int dump_csrs[] = {
> > +
> > +# if 0
> > + CSR_USTATUS,
> > + CSR_UIE,
> > + CSR_UTVEC,
>
> Adding huge sections of #if 0 code is not acceptable.
I'm not sure on how to solve the dilemma of
* transgressing on QEMUs coding guidelines on the one side
(large sections of commented out code)
* having `info registers' output a huge swath of CSRs,
swamping the user and making the command impractical
I feel that providing some control at compile
time via `# if' conditional compilation is preferrable to just dumping
everything. I could of course only list the CSRs that
are interesting to me, currently, but I thought it
would be better to list (almost) all of them and give at least
programmers an easy way to enable the blocks of CSRs
that are of interest to them.
Obviously, the best solution would be to extend the command to
add a filter argument, similar to GDB's `info registers'
(i.e. info registers XXX), but I don't know how to do that in QEMU and
it would work differently from other target architectures.
What would you suggest?
^ permalink raw reply
* Re: Occasional hung with UM after enable VMAP_STACK
From: Walter Lozano @ 2022-01-05 12:59 UTC (permalink / raw)
To: Anton Ivanov, Johannes Berg, linux-um, linux-kernel
Cc: Sjoerd Simons, ritesh sarraf
In-Reply-To: <94613ccb-ab4d-851d-01d0-dfa72b73fdb0@kot-begemot.co.uk>
Hi Anton,
On 1/4/22 18:39, Anton Ivanov wrote:
> On 04/01/2022 19:26, Walter Lozano wrote:
>> Hi Johannes,
>>
>> On 1/4/22 16:04, Johannes Berg wrote:
>>> On Tue, 2022-01-04 at 15:10 -0300, Walter Lozano wrote:
>>>> Hi all,
>>>>
>>>> I noticed that after "um: enable VMAP_STACK" [1] I experienced some
>>>> occasional hung in my Gitlab CI jobs that use user-mode-linux to build
>>>> distro images.
>>>>
>>> Did you actually *enable* VMAP_STACK in the config as well? The commit
>>> just makes it *possible* to enable it, you still have to set it
>>> yourself. So you should be able to easily check with/without that
>>> setting.
>>
>> Thank you for your quick response. The Debian configuration on
>> package user-mode-linux have these settings
>>
>> CONFIG_HAVE_ARCH_VMAP_STACK=y
>> CONFIG_VMAP_STACK=y
>>
>>
>> as you can see in [1]. I did run some tests disabling those settings,
>> which passed without any hung.
>>
>> Unfortunately the "occasional" behavior makes this issue a bit tricky
>> to debug.
>>
>> Regards,
>>
>> Walter
>>
>> [1]
>> https://salsa.debian.org/uml-team/user-mode-linux/-/blob/master/config.amd64#L321
>>
>>
>
> Just to narrow things down - 64 bit or 32 bit?
>
Thank you for commenting on this thread. All my tests were done with 64
bits.
Regards,
--
Walter Lozano
Collabora Ltd.
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply
* Re: Occasional hung with UM after enable VMAP_STACK
From: Walter Lozano @ 2022-01-05 12:59 UTC (permalink / raw)
To: Anton Ivanov, Johannes Berg, linux-um, linux-kernel
Cc: Sjoerd Simons, ritesh sarraf
In-Reply-To: <94613ccb-ab4d-851d-01d0-dfa72b73fdb0@kot-begemot.co.uk>
Hi Anton,
On 1/4/22 18:39, Anton Ivanov wrote:
> On 04/01/2022 19:26, Walter Lozano wrote:
>> Hi Johannes,
>>
>> On 1/4/22 16:04, Johannes Berg wrote:
>>> On Tue, 2022-01-04 at 15:10 -0300, Walter Lozano wrote:
>>>> Hi all,
>>>>
>>>> I noticed that after "um: enable VMAP_STACK" [1] I experienced some
>>>> occasional hung in my Gitlab CI jobs that use user-mode-linux to build
>>>> distro images.
>>>>
>>> Did you actually *enable* VMAP_STACK in the config as well? The commit
>>> just makes it *possible* to enable it, you still have to set it
>>> yourself. So you should be able to easily check with/without that
>>> setting.
>>
>> Thank you for your quick response. The Debian configuration on
>> package user-mode-linux have these settings
>>
>> CONFIG_HAVE_ARCH_VMAP_STACK=y
>> CONFIG_VMAP_STACK=y
>>
>>
>> as you can see in [1]. I did run some tests disabling those settings,
>> which passed without any hung.
>>
>> Unfortunately the "occasional" behavior makes this issue a bit tricky
>> to debug.
>>
>> Regards,
>>
>> Walter
>>
>> [1]
>> https://salsa.debian.org/uml-team/user-mode-linux/-/blob/master/config.amd64#L321
>>
>>
>
> Just to narrow things down - 64 bit or 32 bit?
>
Thank you for commenting on this thread. All my tests were done with 64
bits.
Regards,
--
Walter Lozano
Collabora Ltd.
^ permalink raw reply
* Re: [PATCH] serial: imx: reduce RX interrupt frequency
From: Sergey Organov @ 2022-01-05 13:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Tomasz Moń, linux-serial, Jiri Slaby, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, k.drobinski
In-Reply-To: <YdQndwYc9xaauvpS@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Tue, Jan 04, 2022 at 11:32:03AM +0100, Tomasz Moń wrote:
>> Triggering RX interrupt for every byte defeats the purpose of aging
>> timer and leads to interrupt starvation at high baud rates.
>>
>> Increase receiver trigger level to 8 to increase the minimum period
>> between RX interrupts to 8 characters time. The tradeoff is increased
>> latency. In the worst case scenario, where RX data has intercharacter
>> delay slightly less than aging timer (8 characters time), it can take
>> up to 63 characters time for the interrupt to be raised since the
>> reception of the first character.
>
> Why can't you do this dynamically based on the baud rate so as to always
> work properly for all speeds without increased delays for slower ones?
I don't like the idea of dynamic threshold as I don't think increased
complexity is worth it.
In fact the threshold works "properly" on any baud rate, as maximum
latency is proportional to the current baud rate, and if somebody does
care about *absolute* latency, increasing baud rate is the primary
solution. At most I'd think about some ioctl() to manually tweak the
threshold, for some exotic applications.
That said, are there examples in the kernel where this threshold is
dynamic? If not, then it's another argument against introducing it, and
if yes, then the OP will have reference implementation of exact
dependency curve.
Thanks,
-- Sergey Organov
^ permalink raw reply
* Re: [PATCH v2 net-next 1/2] net: bpf: handle return value of BPF_CGROUP_RUN_PROG_INET{4,6}_POST_BIND()
From: Daniel Borkmann @ 2022-01-05 13:01 UTC (permalink / raw)
To: menglong8.dong, kuba
Cc: davem, yoshfuji, dsahern, edumazet, ast, andrii, kafai,
songliubraving, yhs, john.fastabend, kpsingh, netdev,
linux-kernel, bpf, Menglong Dong
In-Reply-To: <20211230080305.1068950-2-imagedong@tencent.com>
On 12/30/21 9:03 AM, menglong8.dong@gmail.com wrote:
[...]
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 44cc25f0bae7..f5fc0432374e 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1209,6 +1209,7 @@ struct proto {
> void (*unhash)(struct sock *sk);
> void (*rehash)(struct sock *sk);
> int (*get_port)(struct sock *sk, unsigned short snum);
> + void (*put_port)(struct sock *sk);
> #ifdef CONFIG_BPF_SYSCALL
> int (*psock_update_sk_prot)(struct sock *sk,
> struct sk_psock *psock,
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 5d18d32557d2..8784e72d4b8b 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -531,6 +531,8 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> err = BPF_CGROUP_RUN_PROG_INET4_POST_BIND(sk);
> if (err) {
> inet->inet_saddr = inet->inet_rcv_saddr = 0;
> + if (sk->sk_prot->get_port)
> + sk->sk_prot->put_port(sk);
> goto out_release_sock;
> }
> }
[...]
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index d1636425654e..ddfc6821e682 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -413,6 +413,8 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> if (err) {
> sk->sk_ipv6only = saved_ipv6only;
> inet_reset_saddr(sk);
> + if (sk->sk_prot->get_port)
> + sk->sk_prot->put_port(sk);
> goto out;
> }
> }
I presume both tests above should test for non-zero sk->sk_prot->put_port
callback given that is what they end up calling when true, no?
Thanks,
Daniel
^ permalink raw reply
* [PULL 8/8] docs/tools/qemu-trace-stap.rst: Do not hard-code the QEMU binary name
From: Thomas Huth @ 2022-01-05 12:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson, Philippe Mathieu-Daudé
In-Reply-To: <20220105123612.432038-1-thuth@redhat.com>
In downstream, we want to use a different name for the QEMU binary,
and some people might also use the docs for non-x86 binaries, that's
why we already created the |qemu_system| placeholder in the past.
Use it now in the stap trace doc, too.
Message-Id: <20220104103319.179870-1-thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
docs/tools/qemu-trace-stap.rst | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/docs/tools/qemu-trace-stap.rst b/docs/tools/qemu-trace-stap.rst
index d53073b52b..2169ce5d17 100644
--- a/docs/tools/qemu-trace-stap.rst
+++ b/docs/tools/qemu-trace-stap.rst
@@ -46,19 +46,19 @@ The following commands are valid:
any of the listed names. If no *PATTERN* is given, the all possible
probes will be listed.
- For example, to list all probes available in the ``qemu-system-x86_64``
+ For example, to list all probes available in the |qemu_system|
binary:
- ::
+ .. parsed-literal::
- $ qemu-trace-stap list qemu-system-x86_64
+ $ qemu-trace-stap list |qemu_system|
To filter the list to only cover probes related to QEMU's cryptographic
subsystem, in a binary outside ``$PATH``
- ::
+ .. parsed-literal::
- $ qemu-trace-stap list /opt/qemu/4.0.0/bin/qemu-system-x86_64 'qcrypto*'
+ $ qemu-trace-stap list /opt/qemu/|version|/bin/|qemu_system| 'qcrypto*'
.. option:: run OPTIONS BINARY PATTERN...
@@ -90,26 +90,26 @@ The following commands are valid:
Restrict the tracing session so that it only triggers for the process
identified by *PID*.
- For example, to monitor all processes executing ``qemu-system-x86_64``
+ For example, to monitor all processes executing |qemu_system|
as found on ``$PATH``, displaying all I/O related probes:
- ::
+ .. parsed-literal::
- $ qemu-trace-stap run qemu-system-x86_64 'qio*'
+ $ qemu-trace-stap run |qemu_system| 'qio*'
To monitor only the QEMU process with PID 1732
- ::
+ .. parsed-literal::
- $ qemu-trace-stap run --pid=1732 qemu-system-x86_64 'qio*'
+ $ qemu-trace-stap run --pid=1732 |qemu_system| 'qio*'
To monitor QEMU processes running an alternative binary outside of
``$PATH``, displaying verbose information about setup of the
tracing environment:
- ::
+ .. parsed-literal::
- $ qemu-trace-stap -v run /opt/qemu/4.0.0/qemu-system-x86_64 'qio*'
+ $ qemu-trace-stap -v run /opt/qemu/|version|/bin/|qemu_system| 'qio*'
See also
--------
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 5.15 00/72] 5.15.13-rc2 review
From: Jeffrin Jose T @ 2022-01-05 13:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, linux-kernel
Cc: torvalds, akpm, linux, shuah, patches, lkft-triage, pavel,
jonathanh, f.fainelli, stable
In-Reply-To: <20220104073845.629257314@linuxfoundation.org>
On Tue, 2022-01-04 at 08:41 +0100, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.15.13 release.
> There are 72 patches in this series, all will be posted as a response
> to this one. If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Thu, 06 Jan 2022 07:38:29 +0000.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
>
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.15.13-rc2.gz
> or in the git tree and branch at:
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-
> stable-rc.git linux-5.15.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h
>
hello,
There was a compilation error....
-----------x--------------x------------------x--
MODPOST vmlinux.symvers
MODINFO modules.builtin.modinfo
GEN modules.builtin
BTF: .tmp_vmlinux.btf: pahole (pahole) is not available
Failed to generate BTF for vmlinux
Try to disable CONFIG_DEBUG_INFO_BTF
make: *** [Makefile:1183: vmlinux] Error 1
---------x--------------x-------------x---
i did CONFIG_DEBUG_INFO_BTF=n in .config and then compilation was
success.
Compiled and booted 5.15.13-rc2+. dmesg gave no major problems.
Tested-by: Jeffrin Jose T <jeffrin@rajagiritech.edu.in>
--
software engineer
rajagiri school of engineering and technology - autonomous
^ permalink raw reply
* [PATCH] qla2xxx: fix error status checking in qla24xx_modify_vp_config()
From: Dirk Müller @ 2022-01-05 13:03 UTC (permalink / raw)
To: linux-scsi; +Cc: Dirk Müller
The function was checking vpmod->comp_status and reported it as error
status, which is however already checked a few lines below.
Guessing from other occurrences it was supposed to check entry_status
instead.
Fixes: 2c3dfe3f6ad8 ("[SCSI] qla2xxx: add support for NPIV")
Signed-off-by: Dirk Müller <dmueller@suse.de>
---
drivers/scsi/qla2xxx/qla_mbx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 10d2655ef676..e0dce38b65cf 100644
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -4253,7 +4253,7 @@ qla24xx_modify_vp_config(scsi_qla_host_t *vha)
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_mbx, vha, 0x10bd,
"Failed to issue VP config IOCB (%x).\n", rval);
- } else if (vpmod->comp_status != 0) {
+ } else if (vpmod->entry_status != 0) {
ql_dbg(ql_dbg_mbx, vha, 0x10be,
"Failed to complete IOCB -- error status (%x).\n",
vpmod->comp_status);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] serial: imx: reduce RX interrupt frequency
From: Marc Kleine-Budde @ 2022-01-05 13:04 UTC (permalink / raw)
To: Sergey Organov
Cc: Greg Kroah-Hartman, Fabio Estevam, linux-serial, Jiri Slaby,
Tomasz Moń, k.drobinski, NXP Linux Team,
Pengutronix Kernel Team, Shawn Guo, Sascha Hauer
In-Reply-To: <877dbe5oct.fsf@osv.gnss.ru>
[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]
On 05.01.2022 16:00:34, Sergey Organov wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>
> > On Tue, Jan 04, 2022 at 11:32:03AM +0100, Tomasz Moń wrote:
> >> Triggering RX interrupt for every byte defeats the purpose of aging
> >> timer and leads to interrupt starvation at high baud rates.
> >>
> >> Increase receiver trigger level to 8 to increase the minimum period
> >> between RX interrupts to 8 characters time. The tradeoff is increased
> >> latency. In the worst case scenario, where RX data has intercharacter
> >> delay slightly less than aging timer (8 characters time), it can take
> >> up to 63 characters time for the interrupt to be raised since the
> >> reception of the first character.
> >
> > Why can't you do this dynamically based on the baud rate so as to always
> > work properly for all speeds without increased delays for slower ones?
>
> I don't like the idea of dynamic threshold as I don't think increased
> complexity is worth it.
>
> In fact the threshold works "properly" on any baud rate, as maximum
> latency is proportional to the current baud rate, and if somebody does
> care about *absolute* latency, increasing baud rate is the primary
> solution.
Nope - this only works if you have both sides under control.... Which is
not the case in our $CUSTROMER's use case.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH V2 2/2] pinctrl: bcm: add driver for BCM4908 pinmux
From: kernel test robot @ 2022-01-05 13:05 UTC (permalink / raw)
To: kbuild-all
In-Reply-To: <20211222111108.13260-2-zajec5@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]
Hi "Rafał,
I love your patch! Yet something to improve:
[auto build test ERROR on linusw-pinctrl/devel]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211222-191252
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: parisc-randconfig-c023-20220105 (https://download.01.org/0day-ci/archive/20220105/202201052013.fqoOZG0m-lkp(a)intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/337a257cc34c7f7e883bf90ccade5bf4fb71684b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/dt-bindings-pinctrl-Add-binding-for-BCM4908-pinctrl/20211222-191252
git checkout 337a257cc34c7f7e883bf90ccade5bf4fb71684b
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=parisc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "pinconf_generic_dt_free_map" [drivers/pinctrl/bcm/pinctrl-bcm4908.ko] undefined!
>> ERROR: modpost: "pinconf_generic_dt_node_to_map" [drivers/pinctrl/bcm/pinctrl-bcm4908.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply
* Re: [syzbot] KMSAN: uninit-value in ax88178_reset
From: syzbot @ 2022-01-05 13:05 UTC (permalink / raw)
To: andrew, davem, glider, kuba, linux-kernel, linux-usb, linux,
netdev, paskripkin, syzkaller-bugs
In-Reply-To: <66341bb1-a479-cdc8-0928-3c882ac77712@gmail.com>
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: syzbot+6ca9f7867b77c2d316ac@syzkaller.appspotmail.com
Tested on:
commit: 81c325bb kmsan: hooks: do not check memory in kmsan_in..
git tree: https://github.com/google/kmsan.git master
kernel config: https://syzkaller.appspot.com/x/.config?x=46a956fc7a887c60
dashboard link: https://syzkaller.appspot.com/bug?extid=6ca9f7867b77c2d316ac
compiler: clang version 14.0.0 (/usr/local/google/src/llvm-git-monorepo 2b554920f11c8b763cd9ed9003f4e19b919b8e1f), GNU ld (GNU Binutils for Debian) 2.35.2
patch: https://syzkaller.appspot.com/x/patch.diff?x=10d0da73b00000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply
* [meta-networking][dunfell][PATCH] strongswan: Fix for CVE-2021-41990 and CVE-2021-41991
From: virendra thakur @ 2022-01-05 13:05 UTC (permalink / raw)
To: openembedded-core, raj.khem; +Cc: akuster808, Virendra Thakur, Virendra Thakur
From: Virendra Thakur <virendrak@kpit.com>
Add patch to fix CVE-2021-41990 and CVE-2021-41991
Signed-off-by: Virendra Thakur <virendra.thakur@kpit.com>
---
.../strongswan/files/CVE-2021-41990.patch | 60 +++++++++++++++++++
.../strongswan/files/CVE-2021-41991.patch | 39 ++++++++++++
.../strongswan/strongswan_5.8.4.bb | 2 +
3 files changed, 101 insertions(+)
create mode 100644 meta-networking/recipes-support/strongswan/files/CVE-2021-41990.patch
create mode 100644 meta-networking/recipes-support/strongswan/files/CVE-2021-41991.patch
diff --git a/meta-networking/recipes-support/strongswan/files/CVE-2021-41990.patch b/meta-networking/recipes-support/strongswan/files/CVE-2021-41990.patch
new file mode 100644
index 000000000..279a08b67
--- /dev/null
+++ b/meta-networking/recipes-support/strongswan/files/CVE-2021-41990.patch
@@ -0,0 +1,60 @@
+From 423a5d56274a1d343e0d2107dfc4fbf0df2dcca5 Mon Sep 17 00:00:00 2001
+From: Tobias Brunner <tobias@strongswan.org>
+Date: Tue, 28 Sep 2021 17:52:08 +0200
+Subject: [PATCH] Reject RSASSA-PSS params with negative salt length
+
+The `salt_len` member in the struct is of type `ssize_t` because we use
+negative values for special automatic salt lengths when generating
+signatures.
+
+Not checking this could lead to an integer overflow. The value is assigned
+to the `len` field of a chunk (`size_t`), which is further used in
+calculations to check the padding structure and (if that is passed by a
+matching crafted signature value) eventually a memcpy() that will result
+in a segmentation fault.
+
+Fixes: a22316520b91 ("signature-params: Add functions to parse/build ASN.1 RSASSA-PSS params")
+Fixes: 7d6b81648b2d ("gmp: Add support for RSASSA-PSS signature verification")
+Fixes: CVE-2021-41990
+
+Upstream-Status: Backport [https://download.strongswan.org/security/CVE-2021-41990]
+Signed-off-by: Virendra Thakur <virendra.thakur@kpit.com>
+
+---
+ src/libstrongswan/credentials/keys/signature_params.c | 6 +++++-
+ src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c | 2 +-
+ 2 files changed, 6 insertions(+), 2 deletions(-)
+
+diff --git a/src/libstrongswan/credentials/keys/signature_params.c b/src/libstrongswan/credentials/keys/signature_params.c
+index d89bd2c96bb5..837de8443d43 100644
+--- a/src/libstrongswan/credentials/keys/signature_params.c
++++ b/src/libstrongswan/credentials/keys/signature_params.c
+@@ -322,7 +322,11 @@ bool rsa_pss_params_parse(chunk_t asn1, int level0, rsa_pss_params_t *params)
+ case RSASSA_PSS_PARAMS_SALT_LEN:
+ if (object.len)
+ {
+- params->salt_len = (size_t)asn1_parse_integer_uint64(object);
++ params->salt_len = (ssize_t)asn1_parse_integer_uint64(object);
++ if (params->salt_len < 0)
++ {
++ goto end;
++ }
+ }
+ break;
+ case RSASSA_PSS_PARAMS_TRAILER:
+diff --git a/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c b/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c
+index f9bd1d314dec..3a775090883e 100644
+--- a/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c
++++ b/src/libstrongswan/plugins/gmp/gmp_rsa_public_key.c
+@@ -168,7 +168,7 @@ static bool verify_emsa_pss_signature(private_gmp_rsa_public_key_t *this,
+ int i;
+ bool success = FALSE;
+
+- if (!params)
++ if (!params || params->salt_len < 0)
+ {
+ return FALSE;
+ }
+--
+2.25.1
+
diff --git a/meta-networking/recipes-support/strongswan/files/CVE-2021-41991.patch b/meta-networking/recipes-support/strongswan/files/CVE-2021-41991.patch
new file mode 100644
index 000000000..0e5bce2bd
--- /dev/null
+++ b/meta-networking/recipes-support/strongswan/files/CVE-2021-41991.patch
@@ -0,0 +1,39 @@
+From b667237b3a84f601ef5a707ce8eb861c3a5002d3 Mon Sep 17 00:00:00 2001
+From: Tobias Brunner <tobias@strongswan.org>
+Date: Tue, 28 Sep 2021 19:38:22 +0200
+Subject: [PATCH] cert-cache: Prevent crash due to integer overflow/sign change
+
+random() allocates values in the range [0, RAND_MAX], with RAND_MAX usually
+equaling INT_MAX = 2^31-1. Previously, values between 0 and 31 were added
+directly to that offset before applying`% CACHE_SIZE` to get an index into
+the cache array. If the random value was very high, this resulted in an
+integer overflow and a negative index value and, therefore, an out-of-bounds
+access of the array and in turn dereferencing invalid pointers when trying
+to acquire the read lock. This most likely results in a segmentation fault.
+
+Fixes: 764e8b2211ce ("reimplemented certificate cache")
+Fixes: CVE-2021-41991
+
+Upstream-Status: Backport [https://download.strongswan.org/security/CVE-2021-41991]
+Signed-off-by: Virendra Thakur <virendra.thakur@kpit.com>
+
+---
+ src/libstrongswan/credentials/sets/cert_cache.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/libstrongswan/credentials/sets/cert_cache.c b/src/libstrongswan/credentials/sets/cert_cache.c
+index f1579c60a9bc..ceebb3843725 100644
+--- a/src/libstrongswan/credentials/sets/cert_cache.c
++++ b/src/libstrongswan/credentials/sets/cert_cache.c
+@@ -151,7 +151,7 @@ static void cache(private_cert_cache_t *this,
+ for (try = 0; try < REPLACE_TRIES; try++)
+ {
+ /* replace a random relation */
+- offset = random();
++ offset = random() % CACHE_SIZE;
+ for (i = 0; i < CACHE_SIZE; i++)
+ {
+ rel = &this->relations[(i + offset) % CACHE_SIZE];
+--
+2.25.1
+
diff --git a/meta-networking/recipes-support/strongswan/strongswan_5.8.4.bb b/meta-networking/recipes-support/strongswan/strongswan_5.8.4.bb
index 8a8809243..b45b8074c 100644
--- a/meta-networking/recipes-support/strongswan/strongswan_5.8.4.bb
+++ b/meta-networking/recipes-support/strongswan/strongswan_5.8.4.bb
@@ -11,6 +11,8 @@ SRC_URI = "http://download.strongswan.org/strongswan-${PV}.tar.bz2 \
file://fix-funtion-parameter.patch \
file://0001-memory.h-Include-stdint.h-for-uintptr_t.patch \
file://0001-Remove-obsolete-setting-regarding-the-Standard-Outpu.patch \
+ file://CVE-2021-41990.patch \
+ file://CVE-2021-41991.patch \
"
SRC_URI[md5sum] = "0634e7f40591bd3f6770e583c3f27d29"
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 0/8] Add low power hibernation support to cs35l41
From: Charles Keepax @ 2022-01-05 13:05 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, patches, tiwai, lgirdwood, david.rhodes, broonie
In-Reply-To: <s5hr19mie3i.wl-tiwai@suse.de>
On Wed, Jan 05, 2022 at 01:03:45PM +0100, Takashi Iwai wrote:
> On Wed, 05 Jan 2022 12:30:18 +0100,
> Charles Keepax wrote:
> > Patches 7,8 specifically will cause some very minor conflicts with
> > Lucas's currently outstanding work on the HDA version of cs35l41.
> > Whilst things will still build, this patch adds a test key function
> > his code will now have to call. If his patches are getting merged
> > first I will respin this series to update his code, he is currently on
> > holiday until the 12th of Jan, so if we want to wait for another spin
> > of those patches I can work with him to update them at that time. Or
> > alternatively we could just merge them all and I will do a quick fixup
> > patch at the end, since there is no build breakage.
>
> FWIW, the ASoC part of Lucas's patch set has been already merged in
> Mark's asoc tree. (HD-audio part isn't merged yet though).
>
Yeah its the HDA part that would require a small change after
those last two patches to call the additional function. The
series is already based on top of the merged ASoC changes.
Thanks,
Charles
^ permalink raw reply
* Re: [PATCH v2 net-next 1/2] net: bpf: handle return value of BPF_CGROUP_RUN_PROG_INET{4,6}_POST_BIND()
From: Menglong Dong @ 2022-01-05 13:03 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Jakub Kicinski, David Miller, Hideaki YOSHIFUJI, David Ahern,
Eric Dumazet, Alexei Starovoitov, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, John Fastabend, KP Singh, netdev, LKML,
bpf, Menglong Dong
In-Reply-To: <5cf64605-7005-ac06-6ee1-18547910697a@iogearbox.net>
On Wed, Jan 5, 2022 at 9:01 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 12/30/21 9:03 AM, menglong8.dong@gmail.com wrote:
> [...]
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 44cc25f0bae7..f5fc0432374e 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1209,6 +1209,7 @@ struct proto {
> > void (*unhash)(struct sock *sk);
> > void (*rehash)(struct sock *sk);
> > int (*get_port)(struct sock *sk, unsigned short snum);
> > + void (*put_port)(struct sock *sk);
> > #ifdef CONFIG_BPF_SYSCALL
> > int (*psock_update_sk_prot)(struct sock *sk,
> > struct sk_psock *psock,
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index 5d18d32557d2..8784e72d4b8b 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -531,6 +531,8 @@ int __inet_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> > err = BPF_CGROUP_RUN_PROG_INET4_POST_BIND(sk);
> > if (err) {
> > inet->inet_saddr = inet->inet_rcv_saddr = 0;
> > + if (sk->sk_prot->get_port)
> > + sk->sk_prot->put_port(sk);
> > goto out_release_sock;
> > }
> > }
> [...]
> > diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> > index d1636425654e..ddfc6821e682 100644
> > --- a/net/ipv6/af_inet6.c
> > +++ b/net/ipv6/af_inet6.c
> > @@ -413,6 +413,8 @@ static int __inet6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len,
> > if (err) {
> > sk->sk_ipv6only = saved_ipv6only;
> > inet_reset_saddr(sk);
> > + if (sk->sk_prot->get_port)
> > + sk->sk_prot->put_port(sk);
> > goto out;
> > }
> > }
>
> I presume both tests above should test for non-zero sk->sk_prot->put_port
> callback given that is what they end up calling when true, no?
>
You are right, I think that I made a big mistake here...:/
Thanks!
Menglong Dong
^ permalink raw reply
* Re: [PATCH v5 07/21] x86/fpu: Provide fpu_enable_guest_xfd_features() for KVM
From: Paolo Bonzini @ 2022-01-05 13:06 UTC (permalink / raw)
To: Yang Zhong, x86, kvm, linux-kernel, linux-doc, linux-kselftest,
tglx, mingo, bp, dave.hansen, corbet, shuah, seanjc
Cc: jun.nakajima, kevin.tian, jing2.liu, jing2.liu, guang.zeng,
wei.w.wang
In-Reply-To: <20220105123532.12586-8-yang.zhong@intel.com>
On 1/5/22 13:35, Yang Zhong wrote:
> +int fpu_enable_guest_xfd_features(struct fpu_guest *guest_fpu, u64 xfeatures)
> +{
> + lockdep_assert_preemption_enabled();
> +
The old fpu_update_guest_perm_features(guest_fpu) is equivalent to
fpu_enable_guest_xfd_features(guest_fpu, guest_fpu->perm);
Missing doc comment:
/*
* fpu_enable_guest_xfd_features - Enable xfeatures according to guest perm
* @guest_fpu: Pointer to the guest FPU container
* @xfeatures: Features requested by guest CPUID
*
* Enable all dynamic xfeatures according to guest perm and requested CPUID.
* Invoked if the caller wants to conservatively expand fpstate buffer instead
* of waiting until XCR0 or XFD MSR is written.
*
* Return: 0 on success, error code otherwise
*/
Also, the check for 32-bit is slightly imprecise:
/* Dynamic xfeatures are not supported with 32-bit kernels. */
if (!IS_ENABLED(CONFIG_X86_64))
- return 0;
+ return -EINVAL;
since we only get here with xfeatures != 0 (if it compiles, just removing
the IS_ENABLED check altogether would be even better). With these changes,
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Thanks,
Paolo
^ permalink raw reply
* Re: [PATCH] iio: gyro: bmg160: Fix error handling in bmg160_core_probe
From: Jonathan Cameron @ 2022-01-05 13:07 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko,
Linus Walleij, Alexandru Ardelean, Stephan Gerhold,
Gwendal Grignou, Adriana Reus, linux-iio, linux-kernel
In-Reply-To: <20220105125633.21989-1-linmq006@gmail.com>
On Wed, 5 Jan 2022 12:56:30 +0000
Miaoqian Lin <linmq006@gmail.com> wrote:
> The pm_runtime_enable will increase power disable depth.
> If the probe fails, we should use pm_runtime_disable() to balance
> pm_runtime_enable(). In the PM Runtime docs:
> Drivers in ->remove() callback should undo the runtime PM changes done
> in ->probe(). Usually this means calling pm_runtime_disable(),
> pm_runtime_dont_use_autosuspend() etc.
> We should do this in error handling.
>
> Fixes: 7d0ead5 ("iio: Reconcile operation order between iio_register/unregister and pm functions")
Hi Miaoqian,
Please check this fixes tag against the format it should have.
Thanks,
Jonathan
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/iio/gyro/bmg160_core.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
> index 17b939a367ad..81a6d09788bd 100644
> --- a/drivers/iio/gyro/bmg160_core.c
> +++ b/drivers/iio/gyro/bmg160_core.c
> @@ -1188,11 +1188,14 @@ int bmg160_core_probe(struct device *dev, struct regmap *regmap, int irq,
> ret = iio_device_register(indio_dev);
> if (ret < 0) {
> dev_err(dev, "unable to register iio device\n");
> - goto err_buffer_cleanup;
> + goto err_pm_cleanup;
> }
>
> return 0;
>
> +err_pm_cleanup:
> + pm_runtime_dont_use_autosuspend(dev);
> + pm_runtime_disable(dev);
> err_buffer_cleanup:
> iio_triggered_buffer_cleanup(indio_dev);
> err_trigger_unregister:
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Improve the PowerPC machines section
From: Daniel Henrique Barboza @ 2022-01-05 12:51 UTC (permalink / raw)
To: Thomas Huth, Cédric Le Goater, qemu-devel
Cc: Mark Cave-Ayland, Hervé Poussineau, qemu-ppc, Greg Kurz,
David Gibson
In-Reply-To: <20220105104800.407570-1-thuth@redhat.com>
On 1/5/22 07:48, Thomas Huth wrote:
> Add some documentation files to the corresponding machine sections
> and mention the machine names in the section titles where it is
> not so obvious (e.g. that "taihu" is a 405 machine).
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> MAINTAINERS | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f871d759fd..53cf0fdc00 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1245,7 +1245,7 @@ F: hw/openrisc/openrisc_sim.c
>
> PowerPC Machines
> ----------------
> -405
> +405 (ref405ep and taihu)
> L: qemu-ppc@nongnu.org
> S: Orphan
> F: hw/ppc/ppc405_boards.c
> @@ -1281,6 +1281,7 @@ New World (mac99)
> M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> L: qemu-ppc@nongnu.org
> S: Odd Fixes
> +F: docs/system/ppc/powermac.rst
> F: hw/ppc/mac_newworld.c
> F: hw/pci-host/uninorth.c
> F: hw/pci-bridge/dec.[hc]
> @@ -1299,6 +1300,7 @@ Old World (g3beige)
> M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> L: qemu-ppc@nongnu.org
> S: Odd Fixes
> +F: docs/system/ppc/powermac.rst
> F: hw/ppc/mac_oldworld.c
> F: hw/pci-host/grackle.c
> F: hw/misc/macio/
> @@ -1312,6 +1314,7 @@ PReP
> M: Hervé Poussineau <hpoussin@reactos.org>
> L: qemu-ppc@nongnu.org
> S: Maintained
> +F: docs/system/ppc/prep.rst
> F: hw/ppc/prep.c
> F: hw/ppc/prep_systemio.c
> F: hw/ppc/rs6000_mc.c
> @@ -1324,7 +1327,7 @@ F: include/hw/isa/pc87312.h
> F: include/hw/rtc/m48t59.h
> F: tests/avocado/ppc_prep_40p.py
>
> -sPAPR
> +sPAPR (pseries)
> M: Cédric Le Goater <clg@kaod.org>
> M: Daniel Henrique Barboza <danielhb413@gmail.com>
> R: David Gibson <david@gibson.dropbear.id.au>
> @@ -1336,8 +1339,8 @@ F: include/hw/*/spapr*
> F: hw/*/xics*
> F: include/hw/*/xics*
> F: pc-bios/slof.bin
> -F: docs/specs/ppc-spapr-hcalls.txt
> -F: docs/specs/ppc-spapr-hotplug.txt
> +F: docs/system/ppc/pseries.rst
> +F: docs/specs/ppc-spapr-*
> F: tests/qtest/spapr*
> F: tests/qtest/libqos/*spapr*
> F: tests/qtest/rtas*
> @@ -1348,6 +1351,7 @@ PowerNV (Non-Virtualized)
> M: Cédric Le Goater <clg@kaod.org>
> L: qemu-ppc@nongnu.org
> S: Maintained
> +F: docs/system/ppc/powernv.rst
> F: hw/ppc/pnv*
> F: hw/intc/pnv*
> F: hw/intc/xics_pnv.c
^ permalink raw reply
* Re: [PATCH 1/1] softmmu: fix device deletion events with -device JSON syntax
From: Thomas Huth @ 2022-01-05 12:49 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Kevin Wolf, Laurent Vivier, Peter Krempa, Markus Armbruster,
Eduardo Habkost, Paolo Bonzini, Eric Blake
In-Reply-To: <20220105123847.4047954-2-berrange@redhat.com>
On 05/01/2022 13.38, Daniel P. Berrangé wrote:
> The -device JSON syntax impl leaks a reference on the created
> DeviceState instance. As a result when you hot-unplug the
> device, the device_finalize method won't be called and thus
> it will fail to emit the required DEVICE_DELETED event.
>
> A 'json-cli' feature was previously added against the
> 'device_add' QMP command QAPI schema to indicated to mgmt
> apps that -device supported JSON syntax. Given the hotplug
> bug that feature flag is no unusable for its purpose, so
> we add a new 'json-cli-hotplug' feature to indicate the
> -device supports JSON without breaking hotplug.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/802
We're mostly using "Fixes:" to refer to previous commit IDs, and "Resolves:"
for referring to bugs in the gitlab issue tracker, so in case you respin,
I'd suggest to replace it (but both keywords should work to close issues, so
it's just a cosmetical thing).
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> qapi/qdev.json | 5 ++++-
> softmmu/vl.c | 4 +++-
> tests/qtest/device-plug-test.c | 19 +++++++++++++++++++
> 3 files changed, 26 insertions(+), 2 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply
* Re: [PATCH v8 8/8] ls-tree.c: introduce "--format" option
From: Johannes Schindelin @ 2022-01-05 13:09 UTC (permalink / raw)
To: Teng Long; +Cc: avarab, congdanhqx, git, Junio C Hamano, peff, tenglong.tl
In-Reply-To: <CADMgQSSjoxqzBDyGXiNC4wHqYGK7z4O0SG0zai85D-DtDHem=w@mail.gmail.com>
Hi Teng,
On Wed, 5 Jan 2022, Teng Long wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > This, along with two other similar instances, triggers the
> > `static-analysis` job in the CI failure of `seen`. The suggested diff is:
>
> The second and third I will optimize in the next patch.
>
> The first one. Actually I am a little puzzled from this :
>
> > - strbuf_addf(line, "%7s", "-");
> > + strbuf_addstr(line, "-");
>
> > But I think that the first hunk indicates a deeper issue, as `%7s`
> > probably meant to pad the dash to seven dashes (which that format won't
> > accomplish, but `strbuf_addchars()` would)?
>
> "strbuf_addf(line, "%7s", "-");" here is used to align the columns
> with a width of seven chars, not repeat one DASH to seven.
Ah. I misremembered and thought that `"% 7s"` would do that, but you're
correct. See below for more on this.
But first, I wonder why the test suite passes with the `strbuf_addstr()`
call... Is this line not covered by any test case?
About the `%7s` thing: The most obvious resolution is to use `" -"`
with `strbuf_addstr()`. And I would argue that this is the best
resolution.
If you disagree (and want to spin up a full `sprintf()` every time, just
to add those six space characters), feel free to integrate the following
into your patch series:
-- snip --
From a390fcf7eec261c7f0e341bda79f2b1f326d151e Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 5 Jan 2022 14:02:19 +0100
Subject: [PATCH] cocci: allow padding with `strbuf_addf()`
A convenient way to pad strings is to use something like
`strbuf_addf(&buf, "%20s", "Hello, world!")`.
However, the Coccinelle rule that forbids a format `"%s"` with a
constant string argument cast too wide a net, and also forbade such
padding.
Let's be a bit stricter in that Coccinelle rule.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
contrib/coccinelle/strbuf.cocci | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci
index d9ada69b432..2d6e0f58fc8 100644
--- a/contrib/coccinelle/strbuf.cocci
+++ b/contrib/coccinelle/strbuf.cocci
@@ -44,7 +44,7 @@ struct strbuf *SBP;
@@
expression E1, E2;
-format F =~ "s";
+format F =~ "^s$";
@@
- strbuf_addf(E1, "%@F@", E2);
+ strbuf_addstr(E1, E2);
--
2.33.0.windows.2
-- snap --
Ciao,
Dscho
>
> A little weird about the fix recommendation of "strbuf_addstr(line, "-");" ,
> because it will only add a single DASH here.
>
> It's the identical result which compares to the "master"[1] I think with the
> current codes and I tested the "strbuf_addf()" simply and it seems to work
> fine.
>
> [1] https://github.com/git/git/blob/master/builtin/ls-tree.c#L106
^ permalink raw reply related
* Re: [PATCH 4/4] RDMA/rxe: Use the standard method to produce udp source port
From: Yanjun Zhu @ 2022-01-05 13:10 UTC (permalink / raw)
To: Saleem, Shiraz, liangwenpeng@huawei.com, liweihang@huawei.com,
jgg@ziepe.ca, Ismail, Mustafa, zyjzyj2000@gmail.com,
linux-rdma@vger.kernel.org
In-Reply-To: <898d7419-7e29-6258-a41e-2c62f251a1b6@linux.dev>
在 2022/1/5 8:33, Yanjun Zhu 写道:
> 在 2022/1/5 1:17, Saleem, Shiraz 写道:
>>> Subject: [PATCH 4/4] RDMA/rxe: Use the standard method to produce
>>> udp source
>>> port
>>>
>>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>>
>>> Use the standard method to produce udp source port.
>>>
>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>>> ---
>>> drivers/infiniband/sw/rxe/rxe_verbs.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> index 0aa0d7e52773..f30d98ad13cd 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> @@ -469,6 +469,16 @@ static int rxe_modify_qp(struct ib_qp *ibqp,
>>> struct
>>> ib_qp_attr *attr,
>>> if (err)
>>> goto err1;
>>>
>>> + if (mask & IB_QP_AV) {
>>> + if (attr->ah_attr.ah_flags & IB_AH_GRH) {
>>> + u32 fl = attr->ah_attr.grh.flow_label;
>>> + u32 lqp = qp->ibqp.qp_num;
>>> + u32 rqp = qp->attr.dest_qp_num;
>>> +
>> Isn't the randomization for src_port done in rxe_qp_init_req
>> redundant then?
>>
>> https://elixir.bootlin.com/linux/v5.16-rc8/source/drivers/infiniband/sw/rxe/rxe_qp.c#L220
>>
>>
>> Can we remove it?
Based on the discussion with Leon Romanovsky, when in-subnet
communications, GRH is optional.
So It is possible that GRH is not set. Without the randomization for
src_port done in rxe_qp_init_req, udp source
port will be 0xC000 in that case.
So it is better to keep this randomization for src_port done in
rxe_qp_init_req.
Zhu Yanjun
>
> Yes. We can remove it.
> Because this "randomization for src_port done in rxe_qp_init_req" is
> replaced by rdma_get_udp_sport in rxe_modify_qp, I do not remove it.
>
> I will remove it in the latest commits soon.
>
> Zhu Yanjun
>
>>
>>> + qp->src_port = rdma_get_udp_sport(fl, lqp, rqp);
>>> + }
>>> + }
>>> +
>>> return 0;
>>>
>>> err1:
>>> --
>>> 2.27.0
>>
>
^ permalink raw reply
* [PATCH v3] remoteproc: Fix NULL vs IS_ERR() checking in rproc_create_trace_file
From: Miaoqian Lin @ 2022-01-05 13:10 UTC (permalink / raw)
To: linmq006
Cc: bjorn.andersson, linux-kernel, linux-remoteproc, mathieu.poirier,
ohad
In-Reply-To: <20220105064201.3907-1-linmq006@gmail.com>
The debugfs_create_file() function doesn't return NULL.
It returns error pointers. Fix check in rproc_create_trace_file
and make it returns return error pointers.
Fix check in rproc_handle_trace to propagate the error code.
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
Changes in v2:
- return PTR_ERR(tfile) in rproc_create_trace_file
- fix check in rproc_handle_trace()
Changes in v3:
- return tfile to fix incorrect return type in v2
---
drivers/remoteproc/remoteproc_core.c | 6 ++++--
drivers/remoteproc/remoteproc_debugfs.c | 4 +---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 775df165eb45..5608408f8eac 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -656,6 +656,7 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
struct rproc_debug_trace *trace;
struct device *dev = &rproc->dev;
char name[15];
+ int ret;
if (sizeof(*rsc) > avail) {
dev_err(dev, "trace rsc is truncated\n");
@@ -684,9 +685,10 @@ static int rproc_handle_trace(struct rproc *rproc, void *ptr,
/* create the debugfs entry */
trace->tfile = rproc_create_trace_file(name, rproc, trace);
- if (!trace->tfile) {
+ if (IS_ERR(trace->tfile)) {
+ ret = PTR_ERR(trace->tfile);
kfree(trace);
- return -EINVAL;
+ return ret;
}
list_add_tail(&trace->node, &rproc->traces);
diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
index b5a1e3b697d9..2ae59a365b7e 100644
--- a/drivers/remoteproc/remoteproc_debugfs.c
+++ b/drivers/remoteproc/remoteproc_debugfs.c
@@ -390,10 +390,8 @@ struct dentry *rproc_create_trace_file(const char *name, struct rproc *rproc,
tfile = debugfs_create_file(name, 0400, rproc->dbg_dir, trace,
&trace_rproc_ops);
- if (!tfile) {
+ if (IS_ERR(tfile))
dev_err(&rproc->dev, "failed to create debugfs trace entry\n");
- return NULL;
- }
return tfile;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/5] tools/libxl: Mark pointer args of many functions constant
From: Luca Fancellu @ 2022-01-05 13:09 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Elliott Mitchell, xen-devel, Wei Liu, Juergen Gross
In-Reply-To: <YdVuZyvwrDUxn4O4@perard>
> On 5 Jan 2022, at 10:09, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> On Fri, Dec 18, 2020 at 01:37:44PM -0800, Elliott Mitchell wrote:
>> Anything *_is_empty(), *_is_default(), or *_gen_json() is going to be
>> examining the pointed to thing, not modifying it. This potentially
>> results in higher-performance output. This also allows spreading
>> constants further, allowing more checking and security.
>>
>> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
>
> This patch doesn't build.
>
> libxl_cpuid.c:510:17: error: conflicting types for ‘libxl_cpuid_policy_list_gen_json’
> 510 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from libxl_internal.h:89,
> from libxl_cpuid.c:15:
> /home/sheep/work/xen/tools/libs/light/../../../tools/include/libxl_json.h:30:17: note: previous declaration of ‘libxl_cpuid_policy_list_gen_json’ was here
> 30 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Also we talked about this patch before, in
> https://lore.kernel.org/xen-devel/YImXfc4oaPgWzkeT@perard/
>
> Cheers,
Sorry about that, when I’ve put my R-by I thought it was building, I’ve checked again and it’s not building for x86_64, arm32 and arm64 looks fine so far.
Cheers,
Luca
>
> --
> Anthony PERARD
>
^ permalink raw reply
* Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs
From: Lazar, Lijo @ 2022-01-05 13:11 UTC (permalink / raw)
To: Christian König, Andrey Grodzovsky, dri-devel, amd-gfx
Cc: horace.chen, daniel, Monk.Liu
In-Reply-To: <9dc55576-19b1-d5e3-f4da-eede4db8b289@amd.com>
On 1/5/2022 6:01 PM, Christian König wrote:
> Am 05.01.22 um 10:54 schrieb Lazar, Lijo:
>> On 12/23/2021 3:35 AM, Andrey Grodzovsky wrote:
>>> Use reset domain wq also for non TDR gpu recovery trigers
>>> such as sysfs and RAS. We must serialize all possible
>>> GPU recoveries to gurantee no concurrency there.
>>> For TDR call the original recovery function directly since
>>> it's already executed from within the wq. For others just
>>> use a wrapper to qeueue work and wait on it to finish.
>>>
>>> v2: Rename to amdgpu_recover_work_struct
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +++++++++++++++++++++-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
>>> 3 files changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index b5ff76aae7e0..8e96b9a14452 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -1296,6 +1296,8 @@ bool amdgpu_device_has_job_running(struct
>>> amdgpu_device *adev);
>>> bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
>>> int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>> struct amdgpu_job* job);
>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>> + struct amdgpu_job *job);
>>> void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
>>> int amdgpu_device_pci_reset(struct amdgpu_device *adev);
>>> bool amdgpu_device_need_post(struct amdgpu_device *adev);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 7c063fd37389..258ec3c0b2af 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -4979,7 +4979,7 @@ static void amdgpu_device_recheck_guilty_jobs(
>>> * Returns 0 for success or an error on failure.
>>> */
>>> -int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>> struct amdgpu_job *job)
>>> {
>>> struct list_head device_list, *device_list_handle = NULL;
>>> @@ -5237,6 +5237,37 @@ int amdgpu_device_gpu_recover(struct
>>> amdgpu_device *adev,
>>> return r;
>>> }
>>> +struct amdgpu_recover_work_struct {
>>> + struct work_struct base;
>>> + struct amdgpu_device *adev;
>>> + struct amdgpu_job *job;
>>> + int ret;
>>> +};
>>> +
>>> +static void amdgpu_device_queue_gpu_recover_work(struct work_struct
>>> *work)
>>> +{
>>> + struct amdgpu_recover_work_struct *recover_work =
>>> container_of(work, struct amdgpu_recover_work_struct, base);
>>> +
>>> + recover_work->ret =
>>> amdgpu_device_gpu_recover_imp(recover_work->adev, recover_work->job);
>>> +}
>>> +/*
>>> + * Serialize gpu recover into reset domain single threaded wq
>>> + */
>>> +int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>> + struct amdgpu_job *job)
>>> +{
>>> + struct amdgpu_recover_work_struct work = {.adev = adev, .job =
>>> job};
>>> +
>>> + INIT_WORK(&work.base, amdgpu_device_queue_gpu_recover_work);
>>> +
>>> + if (!queue_work(adev->reset_domain.wq, &work.base))
>>> + return -EAGAIN;
>>> +
>>
>> The decision to schedule a reset is made at this point. Subsequent
>> accesses to hardware may not be reliable. So should the flag in_reset
>> be set here itself rather than waiting for the work to start execution?
>
> No, when we race and lose the VM is completely lost and probably
> restarted by the hypervisor.
>
> And when we race and win we properly set the flag before signaling the
> hypervisor that it can continue with the reset.
>
I was talking about baremetal case. When this was synchronous, in_reset
flag is set as one of the first things and amdgpu_in_reset is checked to
prevent further hardware accesses. This design only changes the recover
part and doesn't change the hardware perspective. Potential accesses
from other processes need to be blocked as soon as we determine a reset
is required. Are we expecting the work to be immediately executed and
set the flags?
Thanks,
Lijo
>> Also, what about having the reset_active or in_reset flag in the
>> reset_domain itself?
>
> Of hand that sounds like a good idea.
>
> Regards,
> Christian.
>
>>
>> Thanks,
>> Lijo
>>
>>> + flush_work(&work.base);
>>> +
>>> + return work.ret;
>>> +}
>>> +
>>> /**
>>> * amdgpu_device_get_pcie_info - fence pcie info about the PCIE slot
>>> *
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> index bfc47bea23db..38c9fd7b7ad4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> @@ -63,7 +63,7 @@ static enum drm_gpu_sched_stat
>>> amdgpu_job_timedout(struct drm_sched_job *s_job)
>>> ti.process_name, ti.tgid, ti.task_name, ti.pid);
>>> if (amdgpu_device_should_recover_gpu(ring->adev)) {
>>> - amdgpu_device_gpu_recover(ring->adev, job);
>>> + amdgpu_device_gpu_recover_imp(ring->adev, job);
>>> } else {
>>> drm_sched_suspend_timeout(&ring->sched);
>>> if (amdgpu_sriov_vf(adev))
>>>
>
^ permalink raw reply
* Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs
From: Lazar, Lijo @ 2022-01-05 13:11 UTC (permalink / raw)
To: Christian König, Andrey Grodzovsky, dri-devel, amd-gfx
Cc: horace.chen, Monk.Liu
In-Reply-To: <9dc55576-19b1-d5e3-f4da-eede4db8b289@amd.com>
On 1/5/2022 6:01 PM, Christian König wrote:
> Am 05.01.22 um 10:54 schrieb Lazar, Lijo:
>> On 12/23/2021 3:35 AM, Andrey Grodzovsky wrote:
>>> Use reset domain wq also for non TDR gpu recovery trigers
>>> such as sysfs and RAS. We must serialize all possible
>>> GPU recoveries to gurantee no concurrency there.
>>> For TDR call the original recovery function directly since
>>> it's already executed from within the wq. For others just
>>> use a wrapper to qeueue work and wait on it to finish.
>>>
>>> v2: Rename to amdgpu_recover_work_struct
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +++++++++++++++++++++-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
>>> 3 files changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index b5ff76aae7e0..8e96b9a14452 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -1296,6 +1296,8 @@ bool amdgpu_device_has_job_running(struct
>>> amdgpu_device *adev);
>>> bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
>>> int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>> struct amdgpu_job* job);
>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>> + struct amdgpu_job *job);
>>> void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
>>> int amdgpu_device_pci_reset(struct amdgpu_device *adev);
>>> bool amdgpu_device_need_post(struct amdgpu_device *adev);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 7c063fd37389..258ec3c0b2af 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -4979,7 +4979,7 @@ static void amdgpu_device_recheck_guilty_jobs(
>>> * Returns 0 for success or an error on failure.
>>> */
>>> -int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>> struct amdgpu_job *job)
>>> {
>>> struct list_head device_list, *device_list_handle = NULL;
>>> @@ -5237,6 +5237,37 @@ int amdgpu_device_gpu_recover(struct
>>> amdgpu_device *adev,
>>> return r;
>>> }
>>> +struct amdgpu_recover_work_struct {
>>> + struct work_struct base;
>>> + struct amdgpu_device *adev;
>>> + struct amdgpu_job *job;
>>> + int ret;
>>> +};
>>> +
>>> +static void amdgpu_device_queue_gpu_recover_work(struct work_struct
>>> *work)
>>> +{
>>> + struct amdgpu_recover_work_struct *recover_work =
>>> container_of(work, struct amdgpu_recover_work_struct, base);
>>> +
>>> + recover_work->ret =
>>> amdgpu_device_gpu_recover_imp(recover_work->adev, recover_work->job);
>>> +}
>>> +/*
>>> + * Serialize gpu recover into reset domain single threaded wq
>>> + */
>>> +int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>> + struct amdgpu_job *job)
>>> +{
>>> + struct amdgpu_recover_work_struct work = {.adev = adev, .job =
>>> job};
>>> +
>>> + INIT_WORK(&work.base, amdgpu_device_queue_gpu_recover_work);
>>> +
>>> + if (!queue_work(adev->reset_domain.wq, &work.base))
>>> + return -EAGAIN;
>>> +
>>
>> The decision to schedule a reset is made at this point. Subsequent
>> accesses to hardware may not be reliable. So should the flag in_reset
>> be set here itself rather than waiting for the work to start execution?
>
> No, when we race and lose the VM is completely lost and probably
> restarted by the hypervisor.
>
> And when we race and win we properly set the flag before signaling the
> hypervisor that it can continue with the reset.
>
I was talking about baremetal case. When this was synchronous, in_reset
flag is set as one of the first things and amdgpu_in_reset is checked to
prevent further hardware accesses. This design only changes the recover
part and doesn't change the hardware perspective. Potential accesses
from other processes need to be blocked as soon as we determine a reset
is required. Are we expecting the work to be immediately executed and
set the flags?
Thanks,
Lijo
>> Also, what about having the reset_active or in_reset flag in the
>> reset_domain itself?
>
> Of hand that sounds like a good idea.
>
> Regards,
> Christian.
>
>>
>> Thanks,
>> Lijo
>>
>>> + flush_work(&work.base);
>>> +
>>> + return work.ret;
>>> +}
>>> +
>>> /**
>>> * amdgpu_device_get_pcie_info - fence pcie info about the PCIE slot
>>> *
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> index bfc47bea23db..38c9fd7b7ad4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>> @@ -63,7 +63,7 @@ static enum drm_gpu_sched_stat
>>> amdgpu_job_timedout(struct drm_sched_job *s_job)
>>> ti.process_name, ti.tgid, ti.task_name, ti.pid);
>>> if (amdgpu_device_should_recover_gpu(ring->adev)) {
>>> - amdgpu_device_gpu_recover(ring->adev, job);
>>> + amdgpu_device_gpu_recover_imp(ring->adev, job);
>>> } else {
>>> drm_sched_suspend_timeout(&ring->sched);
>>> if (amdgpu_sriov_vf(adev))
>>>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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.