From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E14D2C04AAC for ; Mon, 20 May 2019 18:07:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C9CCE214AE for ; Mon, 20 May 2019 18:07:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727412AbfETSHm (ORCPT ); Mon, 20 May 2019 14:07:42 -0400 Received: from mx2.suse.de ([195.135.220.15]:43744 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725951AbfETSH2 (ORCPT ); Mon, 20 May 2019 14:07:28 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 0F507ABE9; Mon, 20 May 2019 18:07:26 +0000 (UTC) Date: Mon, 20 May 2019 20:07:25 +0200 Message-ID: From: Takashi Iwai To: shuah Cc: Kees Cook , Greg Kroah-Hartman , Luis Chamberlain , "Rafael J . Wysocki" , linux-kernel@vger.kernel.org Subject: Re: [PATCH 0/5] firmware: Add support for loading compressed files In-Reply-To: <5a3a0649-ece0-c3e3-3ebb-9d8d19d9499f@kernel.org> References: <20190520092647.8622-1-tiwai@suse.de> <20190520093929.GB15326@kroah.com> <5a3a0649-ece0-c3e3-3ebb-9d8d19d9499f@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 20 May 2019 19:26:59 +0200, shuah wrote: > > On 5/20/19 10:22 AM, Takashi Iwai wrote: > > On Mon, 20 May 2019 17:18:48 +0200, > > Takashi Iwai wrote: > >> > >> On Mon, 20 May 2019 16:39:37 +0200, > >> Takashi Iwai wrote: > >>> > >>> On Mon, 20 May 2019 11:39:29 +0200, > >>> Greg Kroah-Hartman wrote: > >>>> > >>>> On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote: > >>>>> Hi, > >>>>> > >>>>> this is a patch set to add the support for loading compressed firmware > >>>>> files. > >>>>> > >>>>> The primary motivation is to reduce the storage size; e.g. currently > >>>>> the amount of /lib/firmware on my machine counts up to 419MB, and this > >>>>> can be reduced to 130MB file compression. No bad deal. > >>>>> > >>>>> The feature adds only fallback to the compressed file, so it should > >>>>> work as it was as long as the normal firmware file is present. The > >>>>> f/w loader decompresses the content, so that there is no change needed > >>>>> in the caller side. > >>>>> > >>>>> Currently only XZ format is supported. A caveat is that the kernel XZ > >>>>> helper code supports only CRC32 (or none) integrity check type, so > >>>>> you'll have to compress the files via xz -C crc32 option. > >>>>> > >>>>> The patch set begins with a few other improvements and refactoring, > >>>>> followed by the compression support. > >>>>> > >>>>> In addition to this, dracut needs a small fix to deal with the *.xz > >>>>> files. > >>>>> > >>>>> Also, the latest patchset is found in topic/fw-decompress branch of my > >>>>> sound.git tree: > >>>>> git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git > >>>> > >>>> After a quick review, these all look good to me, nice job. > >>>> > >>>> One recommendation, can we add support for testing this to the > >>>> tools/testing/selftests/firmware/ tests? And you did run those > >>>> regression tests to verify that you didn't get any of the config options > >>>> messed up, right? :) > >>> > >>> Now I've been testing the firmware selftest, and this turned out to be > >>> surprisingly difficult on my system. By some reason, the test always > >>> fails at the point triggering the request (line 58 of > >>> fw_filesystem.sh): > >>> > >>> if ! echo -n "$NAME" >"$DIR"/trigger_request ; then > >>> .... > >>> > >>> Judging from the strace output, this echo writes only the first byte > >>> of $NAME. Then kernfs write op is invoked and it deals this one byte > >>> input as if a whole argument were passed, leading to an error. > >>> > >>> My temporary workaround was to replace the all "echo" call with > >>> "/usr/bin/echo". > >>> > >>> Then it hits a similar write error at the places like: > >>> > >>> echo 1 > $DIR/config_sync_direct > >>> > >>> This could be worked around by adding -n option to echo. > >>> > >>> Finally, I noticed that the user-fallback doesn't work on my system > >>> any longer and the test stopped. This is expected, so it implies that > >>> all direct loading tests passed. > >>> > >>> FWIW, my system is openSUSE Leap 15.1. Does anyone experience a > >>> similar problem? > >> > >> This seems to be a regression on 5.2-rc1. > >> The tests on 4.20 worked fine. 5.1 worked, but gave the error at > >> fallback test instead of skipping. This is likely another regression, > >> but irrelevant with the major issue as above. > >> > >> Now bisecting... > > > > Still in bisection, but it's timeout, I'll have to leave now... > > > > FWIW, the regression seems to have been introduced around the latest > > kselftest merge. > > The commit 4c7b63a32d54 is bad, and the commit 9cbda1bddb4c good. > > > > Shuah, could you check it? > > > > > > Kees, > > Could this be related to your selftest Makefile test run output > refactoring work? > > I did a quick test on 5.2 after my first kselftest update without > the refactor work - firmware test worked. > > I am thinking this is related to > > eff3ee303d0d > selftests: Extract single-test shell logic from lib.mk OK, I could identify the culprit commit. It's 5c069b6dedef selftests: Move test output to diagnostic lines It seems that stdbuf usage caused a regression, at least on my system. Takashi > > bash vs. sh difference in "echo" command behavior is the cause > is my best guess. Will you be able to take a look at this? > > sudo make -C tools/testing/selftests/firmware/ run_tests > > will show the difference. > > thanks, > -- Shuah >