* Re: [RFC PATCH 3/3] mm/memcg: Allow the task_obj optimization only on non-PREEMPTIBLE kernels.
From: Sebastian Andrzej Siewior @ 2022-01-05 20:22 UTC (permalink / raw)
To: Waiman Long
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
Johannes Weiner, Michal Hocko, Vladimir Davydov, Andrew Morton,
Thomas Gleixner, Peter Zijlstra
In-Reply-To: <df637005-6c72-a1c6-c6b9-70f81f74884d-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On 2022-01-03 10:04:29 [-0500], Waiman Long wrote:
> On 1/3/22 09:44, Sebastian Andrzej Siewior wrote:
> > Is there something you recommend as a benchmark where I could get some
> > numbers?
>
> In the case of PREEMPT_DYNAMIC, it depends on the default setting which is
> used by most users. I will support disabling the optimization if
> defined(CONFIG_PREEMPT_RT) || defined(CONFIG_PREEMPT), just not by
> CONFIG_)PREEMPTION alone.
>
> As for microbenchmark, something that makes a lot of calls to malloc() or
> related allocations can be used.
Numbers I made:
Sandy Bridge Haswell Skylake AMD-A8 7100 Zen2 ARM64
PREEMPT 5,123,896,822 5,215,055,226 5,077,611,590 6,012,287,874 6,234,674,489 20,000,000,100
IRQ 7,494,119,638 6,810,367,629 10,620,130,377 4,178,546,086 4,898,076,012 13,538,461,925
For micro benchmarking I did 1.000.000.000 iterations of
preempt_disable()/enable() [PREEMPT] and local_irq_save()/restore()
[IRQ].
On a Sandy Bridge the PREEMPT loop took 5,123,896,822ns while the IRQ
loop took 7,494,119,638ns. The absolute numbers are not important, it is
worth noting that preemption off/on is less expensive than IRQ off/on.
Except for AMD and ARM64 where IRQ off/on was less expensive. The whole
loop was performed with disabled interrupts so I don't expect much
interference - but then I don't know much the µArch optimized away on
local_irq_restore() given that the interrupts were already disabled.
I don't have any recent Intel HW (I think) so I don't know if this is an
Intel only thing (IRQ off/on cheaper than preemption off/on) but I guess
that the recent uArch would behave similar to AMD.
Moving on: For the test I run 100,000,000 iterations of
kfree(kmalloc(128, GFP_ATOMIC | __GFP_ACCOUNT));
The BH suffix means that in_task() reported false during the allocation,
otherwise it reported true.
SD is the standard deviation.
SERVER means PREEMPT_NONE while PREEMPT means CONFIG_PREEMPT.
OPT means the optimisation (in_task() + task_obj) is active, NO-OPT
means no optimisation (irq_obj is always used).
The numbers are the time in ns needed for 100,000,000 iteration (alloc +
free). I run 5 tests and used the median value here. If the standard
deviation exceeded 10^9 then I repeated the test. The values remained in
the same range since usually only one value was off and the other
remained in the same range.
Sandy Bridge
SERVER OPT SERVER NO-OPT PREEMPT OPT PREEMPT NO-OPT
ALLOC/FREE 8,519,295,176 9,051,200,652 10,627,431,395 11,198,189,843
SD 5,309,768 29,253,976 129,102,317 40,681,909
ALLOC/FREE BH 9,996,704,330 8,927,026,031 11,680,149,900 11,139,356,465
SD 38,237,534 72,913,120 23,626,932 116,413,331
The optimisation is visible in the SERVER-OPT case (~1.5s difference in
the runtime (or 14.7ns per iteration)). There is hardly any difference
between BH and !BH in the SERVER-NO-OPT case.
For the SERVER case, the optimisation improves ~0.5s in runtime for the
!BH case.
For the PREEMPT case it also looks like ~0.5s improvement in the BH case
while in the BH case it looks the other way around.
DYN-SRV-OPT DYN-SRV-NO-OPT DYN-FULL-OPT DYN-FULL-NO-OPT
ALLOC/FREE 11,069,180,584 10,773,407,543 10,963,581,285 10,826,207,969
SD 23,195,912 112,763,104 13,145,589 33,543,625
ALLOC/FREE BH 11,443,342,069 10,720,094,700 11,064,914,727 10,955,883,521
SD 81,150,074 171,299,554 58,603,778 84,131,143
DYN is CONFIG_PREEMPT_DYNAMIC enabled and CONFIG_PREEMPT_NONE is
default. I don't see any difference vs CONFIG_PREEMPT except the
default preemption state (so I didn't test that). The preemption counter
is always forced-in so preempt_enable()/disable() is not optimized away.
SRV is the default value (PREEMPT_NONE) and FULL is the overriden
(preempt=full) state.
Based on that, I don't see any added value by the optimisation once
PREEMPT_DYNAMIC is enabled.
----
Zen2:
SERVER OPT SERVER NO-OPT PREEMPT OPT PREEMPT NO-OPT
ALLOC/FREE 8,126,735,313 8,751,307,383 9,822,927,142 10,045,105,425
SD 100,806,471 87,234,047 55,170,179 25,832,386
ALLOC/FREE BH 9,197,455,885 8,394,337,053 10,671,227,095 9,904,954,934
SD 155,223,919 57,800,997 47,529,496 105,260,566
On Zen2, the IRQ off/on was less expensive than preempt-off/on. So it
looks like I mixed up the numbers in for PREEMPT OPT and NO-OPT but I
re-run it twice and nothing significant changed… However the difference
on PREEMPT for the !BH case is not as significant as on Sandy Bridge
(~200ms here vs ~500ms there).
DYN-SRV-OPT DYN-SRV-NO-OPT DYN-FULL-OPT DYN-FULL-NO-OPT
ALLOC/FREE 9,680,498,929 10,180,973,847 9,644,453,405 10,224,416,854
SD 73,944,156 61,850,527 13,277,203 107,145,212
ALLOC/FREE BH 10,680,074,634 9,956,695,323 10,704,442,515 9,942,155,910
SD 75,535,172 34,524,493 54,625,678 87,163,920
For double testing and checking, the full git tree is available at [0]
and the script to parse the results is at [1].
[0] git://git.kernel.org/pub/scm/linux/kernel/git/bigeasy/staging memcg
[1] https://breakpoint.cc/parse-memcg.py
> Cheers,
> Longman
Sebastian
^ permalink raw reply
* Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
From: Richard Henderson @ 2022-01-05 20:20 UTC (permalink / raw)
To: Schwarz, Konrad, qemu-devel@nongnu.org
Cc: Palmer Dabbelt, Bin Meng, Alistair Francis, Ralf Ramsauer
In-Reply-To: <395769321dbe459d803ba78f610eaf3a@siemens.com>
On 1/5/22 6:04 AM, Schwarz, Konrad wrote:
> So the problem is that these files are generated -- somewhat ironically
> via XSLT from complete GDB target descriptions, which are themselves
> generated from a mixture of AWK and shell scripts that I have in a
> different project and which you would probably not want to have
> contributed. Those scripts generate a variety of other definitions
> for C and assembly besides the GDB XML target descriptions, so would
> probably need to be reduced for just QEMU usage.
You may be right that we don't want your original scripts, but that also implies that the
fact your files are generated is irrelevant to us. Why should we care? I think it makes
more sense to manually edit csr.c and be done with it.
> I tried to
> work around this by using C99's designated initializer syntax,
> adding in the new data at the end of the table, and using specific
> enough initializers to not disturb the existing data.
>
> However, this did not work out: despite using very specific initializers,
> the previously initialized CSR structures in the csr_ops array
> were reset to their default values, i.e., 0, breaking the code.
> This was not the way I expected this feature to work in C99 and
> my reading of the C99 standard does not support this either. But
> that’s what GCC does, at least on my machine.
I'm sure your syntax was incorrect. You probably used
>>> + [CSR_CYCLE] { .gdb_type = "uint64", .gdb_group = "user" },
which does indeed overwrite the entire entry in the array. You could have used
[CSR_CYCLE].gdb_type = "uint64"
which will just set the one field.
That said, I don't think that we want this distributed initialization.
>> I think you should be able to use "unsigned long" as a proxy for the native register size.
>
> `unsigned long' is not listed in section `G.3 Predefined Target Types'
> of the GDB manual.
Hmm. I didn't look at the docs; I looked at gdbtypes.h and saw the existence of
builtin_unsigned_long. I suppose this might not be plumbed into the xml.
In which case, since we're generating everything dynamically anyway, we could just as
easily make NULL map to "uint<XLEN>" when generating the xml. Or instead of a string,
perhaps some enum which distinguishes data_ptr/code_ptr/uint<xlen>/uint<min(n,xlen)> which
is then filled in during generation.
> I also have to say that GDB does not handle the target descriptions
> correctly in all cases. In particular, I suspect a bug when
> a field crosses a 32-bit boundary: GDB is showing twice the
> field value.
I wonder if this is an issue of using "uint32" for the field type? You could, for the
moment, drop all of the bitfield descriptions and leave the interpretation to the user.
Giving an accurate integral value for the register as a whole is more helpful than giving
inaccurate field values.
> As I wrote Alistair, I'm not sure this reasoning is correct. Even if a 64-bit
> machine is running in 32-bit mode, the machine itself remains a 64-bit machine,
> and it's CSRs are so too.
And that's where we're mis-communicating.
We're working toward having exactly one qemu-system-riscv binary, which will emulate both
RV32 and RV64 (and RV128) cpus, selected by -cpu foo. At which point there will be no
CONFIG_RISCV32 define at all. The way to distinguish the three cases is
cpu->env.misa_mxl_max.
> We can have the situation of a 64-bit kernel and 32-bit
> user-mode process; would we want the CSRs to change when switching between the two?
Well ideally yes...
> Even if we did, the GDB remote protocol does not have the ability to say "API change,
> please reread the target description" (AFAIK).
... but as you rightly note, gdb can't handle it.
r~
^ permalink raw reply
* Re: [PATCH RFC] Drivers: hv: balloon: Temporary disable the driver on ARM64 when PAGE_SIZE != 4k
From: Wei Liu @ 2022-01-05 20:22 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: linux-hyperv, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Michael Kelley, Boqun Feng, linux-kernel
In-Reply-To: <20220105165028.1343706-1-vkuznets@redhat.com>
On Wed, Jan 05, 2022 at 05:50:28PM +0100, Vitaly Kuznetsov wrote:
> Hyper-V ballooning and memory hotplug protocol always seems to assume
> 4k page size so all PFNs in the structures used for communication are
> 4k PFNs. In case a different page size is in use on the guest (e.g.
> 64k), things go terribly wrong all over:
> - When reporting statistics, post_status() reports them in guest pages
> and hypervisor sees very low memory usage.
> - When ballooning, guest reports back PFNs of the allocated pages but
> the hypervisor treats them as 4k PFNs.
> - When unballooning or memory hotplugging, PFNs coming from the host
> are 4k PFNs and they may not even be 64k aligned making it difficult
> to handle.
>
> While statistics and ballooning requests would be relatively easy to
> handle by converting between guest and hypervisor page sizes in the
> communication structures, handling unballooning and memory hotplug
> requests seem to be harder. In particular, when ballooning up
> alloc_balloon_pages() shatters huge pages so unballooning request can
> be handled for any part of it. It is not possible to shatter a 64k
> page into 4k pages so it's unclear how to handle unballooning for a
> sub-range if such request ever comes so we can't just report a 64k
> page as 16 separate 4k pages.
>
How does virtio-balloon handle it? Does its protocol handle different
page sizes?
> Ideally, the protocol between the guest and the host should be changed
> to allow for different guest page sizes.
>
> While there's no solution for the above mentioned problems, it seems
> we're better off without the driver in problematic cases.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> drivers/hv/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 0747a8f1fcee..fb353a13e5c4 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -25,7 +25,7 @@ config HYPERV_UTILS
>
> config HYPERV_BALLOON
> tristate "Microsoft Hyper-V Balloon driver"
> - depends on HYPERV
> + depends on HYPERV && (X86 || (ARM64 && ARM64_4K_PAGES))
> select PAGE_REPORTING
> help
> Select this option to enable Hyper-V Balloon driver.
> --
> 2.33.1
>
^ permalink raw reply
* Re: slow snd_rawmidi_drain() for VirMidi devcies
From: Takashi Iwai @ 2022-01-05 20:20 UTC (permalink / raw)
To: Stefan Sauer; +Cc: alsa-devel
In-Reply-To: <CAJ4JBRNbi1Suu=uRwe33Wpd-9EPqBEMwnCK1pBh6_TrsmkOGaQ@mail.gmail.com>
On Wed, 05 Jan 2022 20:53:33 +0100,
Stefan Sauer wrote:
>
> From 6b97042bc91ebee267cbb05e6b937cbab5502f61 Mon Sep 17 00:00:00 2001
> From: Stefan Sauer <ensonic@google.com>
> Date: Wed, 5 Jan 2022 20:44:57 +0100
> Subject: [PATCH] ALSA: seq: virmidi: Add a drain operation
>
> If a driver does not supply a drain operation for outputs, a default code
> path will execute msleep(50). Especially for a virtual midi device
> this severely limits the throughput.
>
> This implementation for the virtual midi driver simply flushes the output
> workqueue.
Your signed-off-by is missing. Could you resubmit with it?
thanks,
Takashi
> ---
> sound/core/seq/seq_virmidi.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
> index 4abc38c70cae..1d708dc20443 100644
> --- a/sound/core/seq/seq_virmidi.c
> +++ b/sound/core/seq/seq_virmidi.c
> @@ -262,6 +262,16 @@ static int snd_virmidi_output_close(struct
> snd_rawmidi_substream *substream)
> return 0;
> }
>
> +/*
> + * drain output work queue
> + */
> +static void snd_virmidi_output_drain(struct snd_rawmidi_substream *substream)
> +{
> + struct snd_virmidi *vmidi = substream->runtime->private_data;
> +
> + flush_work(&vmidi->output_work);
> +}
> +
> /*
> * subscribe callback - allow output to rawmidi device
> */
> @@ -336,6 +346,7 @@ static const struct snd_rawmidi_ops snd_virmidi_output_ops
> = {
> .open = snd_virmidi_output_open,
> .close = snd_virmidi_output_close,
> .trigger = snd_virmidi_output_trigger,
> + .drain = snd_virmidi_output_drain,
> };
>
> /*
> --
> 2.34.1
>
> Am Mi., 5. Jan. 2022 um 15:13 Uhr schrieb Takashi Iwai <tiwai@suse.de>:
> >
> > On Wed, 05 Jan 2022 14:39:05 +0100,
> > Stefan Sauer wrote:
> > >
> > > - 28,78% 0,00% rawmidi_alsa [snd_rawmidi] [k]
> > > snd_rawmidi_drain_output
> > > - snd_rawmidi_drain_output
> > > - 26,59% msleep
> > > schedule_timeout
> > > schedule
> > > + __schedule
> > > + 2,13% schedule_timeout
> > >
> > > Seems to be because of:
> > > https://github.com/torvalds/linux/blob/master/sound/core/rawmidi.c#L244
> > > if (substream->ops->drain)
> > > substream->ops->drain(substream);
> > > else
> > > msleep(50);
> > >
> > > It see what fileops have no drain impl:
> > > https://github.com/torvalds/linux/blob/master/sound/core/rawmidi.c#L1708
> > > and I see
> > >
> https://github.com/torvalds/linux/blob/master/sound/core/seq/seq_virmidi.c#L329
> > > that define no drain op. Not which ones are actually used here :/
> > >
> > > The docs confirm teh 50 ms wait though:
> > >
> https://github.com/torvalds/linux/blob/master/Documentation/sound/kernel-api/writing-an-alsa-driver.rst#drain-callback
> > >
> > > would it make sense to have a dummy drain function for the seq_virmidi
> output
> > > ops?
> >
> > Yes, a proper drain implementation is definitely better.
> > I guess just calling flush_work(&vmidi->output_work) there would
> > suffice.
> >
> >
> > thanks,
> >
> > Takashi
> >
> > >
> > > Stefan
> > >
> > > Am Di., 4. Jan. 2022 um 16:54 Uhr schrieb Takashi Iwai <tiwai@suse.de>:
> > >
> > > On Sat, 01 Jan 2022 12:49:13 +0100,
> > > Stefan Sauer wrote:
> > > >
> > > > hi,
> > > >
> > > > I've tried to link BitwigStudio to the webapp cables.gl over
> virmidi.
> > > > Unfortunately Bitwig Studio only supports rawmidi. What I discovered
> is
> > > > that there is a strange slowness when sending data to virmidi caused
> > > > by snd_rawmidi_drain().
> > > >
> > > > I've posted two tiny, self-contained c apps to:
> > > > https://gist.github.com/ensonic/c7588b87fa6c1fa94a8f753b1e0aa394
> > > > See some examples below. 2 observations:
> > > > * snd_rawmidi_type() is *not* reporting virmidi as VIRTUAL
> > > > * snd_rawmidi_drain() takes about 60ms! on virtual vs. less that 0.1
> ms
> > > on
> > > > usb midi (I checked all my hw midi and the worst was avg=1ms on
> physical
> > > > midi image unitor8)
> > > >
> > > > When comparing the implementations:
> > > >
> > >
> https://github.com/alsa-project/alsa-lib/blob/master/src/rawmidi/rawmidi_virt.c#L173
> > > >
> > >
> https://github.com/alsa-project/alsa-lib/blob/master/src/rawmidi/rawmidi_hw.c#L164
> > > > I see that the hw one results in an IOCTL which I can see when
> striking
> > > the
> > > > code and I wonder if this is the root cause? Why is rawmidi_virt.c
> not
> > > used
> > > > for virmidi?
> > > > >From poking at snd_rawmidi_open_conf() I have not yet figured where
> > > this is
> > > > decided ....
> > > >
> > > > Stefan
> > > >
> > > > > amidi -l
> > > > Dir Device Name
> > > > IO hw:0,0,0 Scarlett 18i20 USB MIDI 1
> > > > IO hw:3,0,0 nanoKEY2 nanoKEY2 _ KEYBOARD
> > > > IO hw:5,0,0 nanoKONTROL nanoKONTROL _ SLIDE
> > > > IO hw:10,0 Virtual Raw MIDI (16 subdevices)
> > > > IO hw:11,0 Virtual Raw MIDI (16 subdevices)
> > > >
> > > > # using direct i/o to virmidi - all good
> > > > > ./rawmidi_oss /dev/midi11 0
> > > > Using device '/dev/midi11' without draining
> > > > write took min= 0.0015 ms, avg= 0.0016 ms, max= 0.0110 ms
> > > > > ./rawmidi_oss /dev/midi11 1
> > > > Using device '/dev/midi11' with draining
> > > > write took min= 0.0015 ms, avg= 0.0017 ms, max= 0.0101 ms
> > > > drain took min= 0.0001 ms, avg= 0.0001 ms, max= 0.0008 ms
> > > >
> > > > # using snd_rawmidi to virmidi - slow drain operations
> > > > > ./rawmidi_alsa hw:11,0 0
> > > > Using device 'hw:11,0' without draining
> > > > SND_RAWMIDI_TYPE_HW
> > > > write took min= 0.0010 ms, avg= 0.0011 ms, max= 0.0056 ms
> > > > > ./rawmidi_alsa hw:11,0 1
> > > > Using device 'hw:11,0' with draining
> > > > SND_RAWMIDI_TYPE_HW
> > > > write took min= 0.0016 ms, avg= 0.0040 ms, max= 0.0077 ms
> > > > drain took min= 55.9951 ms, avg= 60.4330 ms, max= 64.0653 ms
> > > >
> > > > # using snd_rawmidi to usb hw - all good
> > > > > ./rawmidi_alsa hw:3,0 0
> > > > Using device 'hw:3,0' without draining
> > > > SND_RAWMIDI_TYPE_HW
> > > > write took min= 0.0012 ms, avg= 0.0015 ms, max= 0.0121 ms
> > > > > ./rawmidi_alsa hw:3,0 1
> > > > Using device 'hw:3,0' with draining
> > > > SND_RAWMIDI_TYPE_HW
> > > > write took min= 0.0024 ms, avg= 0.0032 ms, max= 0.0110 ms
> > > > drain took min= 0.0293 ms, avg= 0.0636 ms, max= 0.2277 ms
> > >
> > > This kind of thing needs profiling. You can try perf or whatever
> > > available, and identify which call takes long. My wild guess is
> > > something about snd_seq_sync_output_queue(), maybe poll syscall takes
> > > unexpected long.
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > >
>
>
^ permalink raw reply
* Re: [GIT PULL] gpio: fixes for v5.16
From: pr-tracker-bot @ 2022-01-05 20:20 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Torvalds, Andy Shevchenko, Linus Walleij, linux-gpio,
linux-kernel, Bartosz Golaszewski
In-Reply-To: <20220105140451.200178-1-brgl@bgdev.pl>
The pull request you sent on Wed, 5 Jan 2022 15:04:51 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-fixes-for-v5.16
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/49ef78e59b0749814d79eed156dcfc175fbd2f74
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2 2/2] sparse-checkout: custom tab completion
From: Lessley Dennington @ 2022-01-05 20:20 UTC (permalink / raw)
To: Elijah Newren
Cc: Lessley Dennington via GitGitGadget, Git Mailing List,
Derrick Stolee, Junio C Hamano, johannes.schindelin
In-Reply-To: <CABPp-BFJF7hyr5onMFQNC7r_x+XfJVn9wXhxyMV6Lu+pxbfyPA@mail.gmail.com>
On 1/4/22 2:42 PM, Elijah Newren wrote:
> On Tue, Jan 4, 2022 at 11:41 AM Lessley Dennington
> <lessleydennington@gmail.com> wrote:
>>
>>
>>
>> On 12/31/21 4:52 PM, Elijah Newren wrote:
>>> On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
>>> <gitgitgadget@gmail.com> wrote:
>>>>
>>>> From: Lessley Dennington <lessleydennington@gmail.com>
>>>>
>>>> Fix custom tab completion for sparse-checkout command. This will ensure:
>>>>
>>>> 1. The full list of subcommands is provided when users enter git
>>>> sparse-checkout <TAB>.
>>>> 2. The --help option is tab-completable.
>>>> 3. Subcommand options are tab-completable.
>>>> 4. A list of directories (but not files) is provided when users enter git
>>>> sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
>>>>
>>>> Failing tests that were added in the previous commit to verify these
>>>> scenarios are now passing with these updates.
>>>>
>>>> Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
>>>> ---
>>>> contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
>>>> t/t9902-completion.sh | 8 +++---
>>>> 2 files changed, 30 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>>>> index c82ccaebcc7..7de997ee64e 100644
>>>> --- a/contrib/completion/git-completion.bash
>>>> +++ b/contrib/completion/git-completion.bash
>>>> @@ -2986,24 +2986,38 @@ _git_show_branch ()
>>>> __git_complete_revlist
>>>> }
>>>>
>>>> +__git_sparse_checkout_subcommand_opts="--cone --no-cone --sparse-index --no-sparse-index"
>>>> +
>>>> _git_sparse_checkout ()
>>>> {
>>>> - local subcommands="list init set disable"
>>>> + local subcommands="list init set disable add reapply"
>>>> local subcommand="$(__git_find_on_cmdline "$subcommands")"
>>>> +
>>>> if [ -z "$subcommand" ]; then
>>>> - __gitcomp "$subcommands"
>>>> - return
>>>> + case "$cur" in
>>>> + --*)
>>>> + __gitcomp "--help"
>>>> + ;;
>>>> + *)
>>>> + __gitcomp "$subcommands"
>>>> + ;;
>>>> + esac
>>>> fi
>>>>
>>>> - case "$subcommand,$cur" in
>>>> - init,--*)
>>>> - __gitcomp "--cone"
>>>> - ;;
>>>> - set,--*)
>>>> - __gitcomp "--stdin"
>>>> - ;;
>>>> - *)
>>>> - ;;
>>>> + case "$prev" in
>>>
>>> Shouldn't this be "$subcommand" rather than "$prev"? I think with
>>> prev, it will only correctly complete the first path after "set",
>>> "add", etc.
>>>
>> Good catch, thank you! Fixing in v3.
>>>> + set)
>>>> + __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
>>>> + __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
>>>> + ;;
>>>> + add)
>>>> + __gitcomp "--stdin"
>>>> + __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
>>>
>>> I was going to make a simple suggestion for making it just complete
>>> one additional level at a time and leaving out the -r, and then tried
>>> it out and found out it wasn't simple. I got something working,
>>> eventually, but it's slightly ugly...so it probably belongs in a
>>> separate patch anyway. If you're curious, it's basically replacing
>>> the second __gitcomp... call for each of set and add with
>>> `__gitcomp_directories`, after first defining:
>>>
>>> __gitcomp_directories ()
>>> {
>>> local _tmp_dir _tmp_completions
>>>
>>> # Get the directory of the current token; this differs from dirname
>>> # in that it keeps up to the final trailing slash. If no slash found
>>> # that's fine too.
>>> [[ "$cur" =~ .*/ ]]
>>> _tmp_dir=$BASH_REMATCH
>>>
>>> # Find possible directory completions, adding trailing '/' characters
>>> _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
>>> sed -e s%$%/%)"
>>>
>>> if [[ -n "$_tmp_completions" ]]; then
>>> # There were some directory completions, so find ones that
>>> # start with "$cur", the current token, and put those in COMPREPLY
>>> local i=0 c IFS=$' \t\n'
>>> for c in $_tmp_completions; do
>>> if [[ $c == "$cur"* ]]; then
>>> COMPREPLY+=("$c")
>>> fi
>>> done
>>> elif [[ "$cur" =~ /$ ]]; then
>>> # No possible further completions any deeper, so assume we're at
>>> # a leaf directory and just consider it complete
>>> __gitcomp_direct_append "$cur "
>>> fi
>>> }
>>>
>>> But I don't think that needs to be part of this series; I can submit
>>> it later and hopefully get a completion expert to point out
>>> better/cleaner ways of what I've done above.
>>>
>> I'm admittedly curious about what made this so difficult. I removed the
>> '-r' and updated my tests to expect only directories at one level, and
>> they passed. But I imagine I'm being too simplistic.
>
> I've forgotten some details since last Saturday, but I think the
> problem was that doing that would only ever complete toplevel
> directories; after completing those you could keep tabbing to get a
> deeper directory. First, let's get a comparison point; ignoring
> sparse-checkout, I can do:
>
> cd $GIT_CLONE
> cd cont<TAB>b<TAB><TAB>
>
> and the ls line will replace those <TAB>s so that I see
>
> ls contrib/buildsystems/Generators
>
> Now, if we just removed the '-r' from your git-completion.bash
> changes, and then typed
>
> git sparse-checkout set cont<TAB>b<TAB><TAB>
>
> Then you'd see
>
> git sparse-checkout set contrib
>
> and see 'bin-wrappers', 'block-sha1', and 'builtin' as potential
> completions, but not as subdirs of contrib but instead sibling dirs to
> contrib. That completion rule wouldn't let you look within contrib/.
> My more complicated rule had to avoid calling __gitcomp to avoid
> adding spaces so that completions could continue (but should add them
> if we have tabbed all the way down and there are no more
> subdirectories), had to add trailing '/' characters so that we know
> when we have the full directory name to pass along to ls-tree, and
> then had to manually do the work that __gitcomp would manually do with
> making sure to only provide completions relevant to what has been
> typed so far.
Ah, I see. Thank you so much for the thorough explanation. I know you said
this series could go through without that update, but I feel like it
should probably be added. Don't want to start off with the wrong behavior.
^ permalink raw reply
* Re: [RFC PATCH 2/8] media: i2c: ov6650: Drop implementation of .set_mbus_config()
From: Laurent Pinchart @ 2022-01-05 20:19 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, linux-renesas-soc, Hans Verkuil, Kieran Bingham,
Jacopo Mondi, Niklas Söderlund, Tomi Valkeinen,
Janusz Krzysztofik
In-Reply-To: <YdXdqJLKzDduVdex@valkosipuli.retiisi.eu>
Hi Sakari,
On Wed, Jan 05, 2022 at 08:04:24PM +0200, Sakari Ailus wrote:
> On Mon, Jan 03, 2022 at 06:24:08PM +0200, Laurent Pinchart wrote:
> > The subdev .set_mbus_config() operation is deprecated. No code in the
> > kernel calls it, so drop its implementation from the ov6650 driver.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > drivers/media/i2c/ov6650.c | 37 -------------------------------------
> > 1 file changed, 37 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> > index f67412150b16..455a627e35a0 100644
> > --- a/drivers/media/i2c/ov6650.c
> > +++ b/drivers/media/i2c/ov6650.c
> > @@ -944,42 +944,6 @@ static int ov6650_get_mbus_config(struct v4l2_subdev *sd,
> > return 0;
> > }
> >
> > -/* Alter bus settings on camera side */
> > -static int ov6650_set_mbus_config(struct v4l2_subdev *sd,
> > - unsigned int pad,
> > - struct v4l2_mbus_config *cfg)
> > -{
> > - struct i2c_client *client = v4l2_get_subdevdata(sd);
> > - int ret = 0;
> > -
> > - if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
> > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_PCLK_RISING, 0);
> > - else if (cfg->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
> > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_PCLK_RISING);
>
> I think this configuration should come from the endpoint which the driver
> currently does not parse. In fact, there are no even DT bindings for the
> device.
There's also no OF match table. While this isn't strictly required, it
may indicate that the sensor hasn't been tested much on DT-based
systems.
I agree that the configuration should come from the device tree, but I
can't test that, so I'm tempted to let someone else implement it if the
driver is actually still in use (I can also write a patch if someone can
test it).
> I wonder what kind of environment it is used in --- assuming it works
> somewhere.
>
> > - if (ret)
> > - return ret;
> > -
> > - if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)
> > - ret = ov6650_reg_rmw(client, REG_COMF, COMF_HREF_LOW, 0);
> > - else if (cfg->flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> > - ret = ov6650_reg_rmw(client, REG_COMF, 0, COMF_HREF_LOW);
> > - if (ret)
> > - return ret;
> > -
> > - if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
> > - ret = ov6650_reg_rmw(client, REG_COMJ, COMJ_VSYNC_HIGH, 0);
> > - else if (cfg->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> > - ret = ov6650_reg_rmw(client, REG_COMJ, 0, COMJ_VSYNC_HIGH);
> > - if (ret)
> > - return ret;
> > -
> > - /*
> > - * Update the configuration to report what is actually applied to
> > - * the hardware.
> > - */
> > - return ov6650_get_mbus_config(sd, pad, cfg);
> > -}
> > -
> > static const struct v4l2_subdev_video_ops ov6650_video_ops = {
> > .s_stream = ov6650_s_stream,
> > .g_frame_interval = ov6650_g_frame_interval,
> > @@ -993,7 +957,6 @@ static const struct v4l2_subdev_pad_ops ov6650_pad_ops = {
> > .get_fmt = ov6650_get_fmt,
> > .set_fmt = ov6650_set_fmt,
> > .get_mbus_config = ov6650_get_mbus_config,
> > - .set_mbus_config = ov6650_set_mbus_config,
> > };
> >
> > static const struct v4l2_subdev_ops ov6650_subdev_ops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v2] rockchip: timer: add OF_PLATDATA support for dw-apb-timer
From: Johan Jonker @ 2022-01-05 20:19 UTC (permalink / raw)
To: Simon Glass; +Cc: Kever Yang, Philipp Tomsich, Marek Vasut, U-Boot Mailing List
In-Reply-To: <bbd764d3-a188-4bb4-ed63-32c91aee94fb@gmail.com>
Hi,
In addition of the previous message, when I compile with the opposite of
OF_REAL (= !OF_PLATDATA) it generates an error in SPL.
Like to know why OF_REAL doesn't work.
For these couple of extra lines to increase build coverage inside does
it matter a lot by adding another messy set of #if ....
Let me know if that's OK to leave it as it is for version 3 with extra
white space patch.
Kind regards,
Johan Jonker
static int dw_apb_timer_of_to_plat(struct udevice *dev)
{
if (!IS_ENABLED(OF_PLATDATA)) {
struct dw_apb_timer_priv *priv = dev_get_priv(dev);
priv->regs = dev_read_addr(dev);
}
return 0;
}
arm-linux-gnueabihf-ld.bfd: drivers/timer/dw-apb-timer.o: in function
`dw_apb_timer_of_to_plat':
/drivers/timer/dw-apb-timer.c:95: undefined reference to `dev_read_addr'
arm-linux-gnueabihf-ld.bfd: drivers/timer/dw-apb-timer.o: in function
`dw_apb_timer_probe':
/drivers/timer/dw-apb-timer.c:73: undefined reference to `clk_get_by_index'
make[1]: *** [scripts/Makefile.spl:509: spl/u-boot-spl] Error 1
make: *** [Makefile:2011: spl/u-boot-spl] Error 2
On 1/5/22 5:40 PM, Johan Jonker wrote:
> Hi Simon,
>
> Thanks you for your comments.
> Shown below are the objdump results of the full U-boot binary
> dw_apb_timer_of_to_plat() function.
> Same goes for the dw_apb_timer_probe() function.
> With if (IS_ENABLED(OF_REAL)) I don't get a useful timer result (boot
> hangs after timer probe, because in full U-boot the reg value isn't
> correct defined. Part of the init probe is missing.
>
> Could you try it yourself?
> Please advise what options we have.
>
> Kind regards,
>
> Johan Jonker
>
> ==========================================================================
> Patch version 1 with if (IS_ENABLED(OF_REAL)) {}:
>
> arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o >
> ../dw-apb-timer-20220105-v1.asm
>
>
> 00000000 <dw_apb_timer_of_to_plat>:
> 0: 2000 movs r0, #0
> 2: 4770 bx lr
>
> arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o >
> ../dw-apb-timer-20220105-spl-v1.asm
> arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o >
> ../dw-apb-timer-20220105-tpl-v1.asm
> ==========================================================================
> patch version 2 with #if CONFIG_IS_ENABLED(OF_REAL):
>
> arm-linux-gnueabihf-objdump -d drivers/timer/dw-apb-timer.o > ../dw-apb-
> timer-20220105-v2.asm
>
> 00000000 <dw_apb_timer_of_to_plat>:
> 0: b538 push {r3, r4, r5, lr}
> 2: 4605 mov r5, r0
> 4: f7ff fffe bl 0 <dev_get_priv>
> 8: 4604 mov r4, r0
> a: 4628 mov r0, r5
> c: f7ff fffe bl 0 <devfdt_get_addr>
> 10: 6020 str r0, [r4, #0]
> 12: 2000 movs r0, #0
> 14: bd38 pop {r3, r4, r5, pc}
>
> arm-linux-gnueabihf-objdump -d spl/drivers/timer/dw-apb-timer.o >
> ../dw-apb-timer-20220105-spl-v2.asm
> arm-linux-gnueabihf-objdump -d tpl/drivers/timer/dw-apb-timer.o >
> ../dw-apb-timer-20220105-tpl-v2.asm
>
> ===========================================================================
>
> On 1/5/22 3:03 PM, Simon Glass wrote:
>> Hi Johan,
>>
>> On Tue, 4 Jan 2022 at 19:15, Johan Jonker <jbx6244@gmail.com> wrote:
>>>
>>> The Rockchip rk3066 SoC has 3 dw-apb-timer nodes.
>>> U-boot is compiled with OF_PLATDATA TPL/SPL options,
>>> so add OF_PLATDATA support for the dw-apb-timer.
>>> Also change driver name to be able to compile with
>>> U-boot scripts. No reset OF_PLATDATA support was added,
>>> because the rk3066 nodes don't need/have them.
>>>
>>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>>> ---
>>>
>>> Changed V2:
>>> replace if (IS_ENABLED(OF_REAL)) by #if CONFIG_IS_ENABLED(OF_REAL)
>>> ---
>>> drivers/timer/dw-apb-timer.c | 30 +++++++++++++++++++++++++-----
>>> 1 file changed, 25 insertions(+), 5 deletions(-)
>>>
>>
>> This seems OK but you have included unrelated changes (whitespace)
>> which should go in a separate patch.
>
> OK
> With whitespace did you mean this or something else?
>
> - .of_to_plat = dw_apb_timer_of_to_plat,
> + .of_to_plat = dw_apb_timer_of_to_plat,
>
>>
>> Can you use if() instead of #if for the CONFIG_IS_ENABLED(OF_REAL) ?
>
> See comments above.
> Currently not it seems.
>
>>
>> Regards,
>> Simon
>>
^ permalink raw reply
* Re: cpufreq: intel_pstate: map utilization into the pstate range
From: Julia Lawall @ 2022-01-05 20:19 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Francisco Jerez, Srinivas Pandruvada, Len Brown, Viresh Kumar,
Linux PM, Linux Kernel Mailing List, Ingo Molnar, Peter Zijlstra,
Juri Lelli, Vincent Guittot
In-Reply-To: <CAJZ5v0i9Rh0Cm3Mbu3N8w6UmgJEnmThk4znWVcp9qeroabjsNw@mail.gmail.com>
On Tue, 4 Jan 2022, Rafael J. Wysocki wrote:
> On Tue, Jan 4, 2022 at 4:49 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> >
> > I tried the whole experiment again on an Intel w2155 (one socket, 10
> > physical cores, pstates 12, 33, and 45).
> >
> > For the CPU there is a small jump a between 32 and 33 - less than for the
> > 6130.
> >
> > For the RAM, there is a big jump between 21 and 22.
> >
> > Combining them leaves a big jump between 21 and 22.
>
> These jumps are most likely related to voltage increases.
>
> > It seems that the definition of efficient is that there is no more cost
> > for the computation than the cost of simply having the machine doing any
> > computation at all. It doesn't take into account the time and energy
> > required to do some actual amount of work.
>
> Well, that's not what I wanted to say.
I was referring to Francisco's comment that the lowest indicated frequency
should be the most efficient one. Turbostat also reports the lowest
frequency as the most efficient one. In my graph, there are the pstates 7
and 10, which give exactly the same energy consumption as 12. 7 and 10
are certainly less efficient, because the energy consumption is the same,
but the execution speed is lower.
> Of course, the configuration that requires less energy to be spent to
> do a given amount of work is more energy-efficient. To measure this,
> the system needs to be given exactly the same amount of work for each
> run and the energy spent by it during each run needs to be compared.
This is bascially my point of view, but there is a question about it. If
over 10 seconds you consume 10J and by running twice as fast you would
consume only 6J, then how do you account for the nest 5 seconds? If the
machine is then idle for the next 5 seconds, maybe you would end up
consuming 8J in total over the 10 seconds. But if you take advantage of
the free 5 seconds to pack in another job, then you end up consuming 12J.
> However, I think that you are interested in answering a different
> question: Given a specific amount of time (say T) to run the workload,
> what frequency to run the CPUs doing the work at in order to get the
> maximum amount of work done per unit of energy spent by the system (as
> a whole)? Or, given 2 different frequency levels, which of them to
> run the CPUs at to get more work done per energy unit?
This is the approach where you assume that the machine will be idle in any
leftover time. And it accounts for the energy consumed in that idle time.
> The work / energy ratio can be estimated as
>
> W / E = C * f / P(f)
>
> where C is a constant and P(f) is the power drawn by the whole system
> while the CPUs doing the work are running at frequency f, and of
> course for the system discussed previously it is greater in the 2 GHz
> case.
>
> However P(f) can be divided into two parts, P_1(f) that really depends
> on the frequency and P_0 that does not depend on it. If P_0 is large
> enough to dominate P(f), which is the case in the 10-20 range of
> P-states on the system in question, it is better to run the CPUs doing
> the work faster (as long as there is always enough work to do for
> them; see below). This doesn't mean that P(f) is not a convex
> function of f, though.
>
> Moreover, this assumes that there will always be enough work for the
> system to do when running the busy CPUs at 2 GHz, or that it can go
> completely idle when it doesn't do any work, but let's see what
> happens if the amount of work to do is W_1 = C * 1 GHz * T and the
> system cannot go completely idle when the work is done.
>
> Then, nothing changes for the busy CPUs running at 1 GHz, but in the 2
> GHz case we get W = W_1 and E = P(2 GHz) * T/2 + P_0 * T/2, because
> the busy CPUs are only busy 1/2 of the time, but power P_0 is drawn by
> the system regardless. Hence, in the 2 GHz case (assuming P(2 GHz) =
> 120 W and P_0 = 90 W), we get
>
> W / E = 2 * C * 1 GHz / (P(2 GHz) + P_0) = 0.0095 * C * 1 GHz
>
> which is slightly less than the W / E ratio at 1 GHz approximately
> equal to 0.01 * C * 1 GHz (assuming P(1 GHz) = 100 W), so in these
> conditions it would be better to run the busy CPUs at 1 GHz.
OK, I'll try to measure this.
thanks,
julia
^ permalink raw reply
* Re: decompress.c:(.text.FSE_buildDTable_internal+0x2cc): undefined reference to `__clzdi2'
From: Nick Terrell @ 2022-01-05 20:18 UTC (permalink / raw)
To: kernel test robot
Cc: kbuild-all@lists.01.org, Linux Kernel Mailing List,
open list:BROADCOM NVRAM DRIVER, Paul Cercueil
In-Reply-To: <202201060233.mO6P39bM-lkp@intel.com>
> On Jan 5, 2022, at 11:02 AM, kernel test robot <lkp@intel.com> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: c9e6606c7fe92b50a02ce51dda82586ebdf99b48
> commit: e0c1b49f5b674cca7b10549c53b3791d0bbc90a8 lib: zstd: Upgrade to latest upstream zstd version 1.4.10
> date: 8 weeks ago
> config: mips-randconfig-r025-20220105 (https://download.01.org/0day-ci/archive/20220106/202201060233.mO6P39bM-lkp@intel.com/config )
> compiler: mips64el-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://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e0c1b49f5b674cca7b10549c53b3791d0bbc90a8
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout e0c1b49f5b674cca7b10549c53b3791d0bbc90a8
> # 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=mips 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 >>):
>
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `FSE_buildDTable_internal':
>>> decompress.c:(.text.FSE_buildDTable_internal+0x2cc): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `BIT_initDStream':
> decompress.c:(.text.BIT_initDStream+0x7c): undefined reference to `__clzdi2'
> mips64el-linux-ld: decompress.c:(.text.BIT_initDStream+0x158): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `ZSTD_buildFSETable_body_default.constprop.0':
>>> decompress.c:(.text.ZSTD_buildFSETable_body_default.constprop.0+0x2a8): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `FSE_readNCount_body_default':
>>> decompress.c:(.text.FSE_readNCount_body_default+0x130): undefined reference to `__ctzdi2'
>>> mips64el-linux-ld: decompress.c:(.text.FSE_readNCount_body_default+0x1a4): undefined reference to `__ctzdi2'
>>> mips64el-linux-ld: decompress.c:(.text.FSE_readNCount_body_default+0x2e4): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `HUF_readStats_body_default':
>>> decompress.c:(.text.HUF_readStats_body_default+0x184): undefined reference to `__clzdi2'
>>> mips64el-linux-ld: decompress.c:(.text.HUF_readStats_body_default+0x1b4): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `ZSTD_DCtx_getParameter':
>>> decompress.c:(.text.ZSTD_DCtx_getParameter+0x60): undefined reference to `__clzdi2'
These are complaining about usage of __builtin_c{l,t}z(). The code before this change also used
__builtin_clz(), so this isn't new. E.g. the call in BIT_initDStream() is present before the change,
it just moved locations. So these errors aren't new.
This is breaking the MIPS boot code when zstd compressed kernels are enabled, and was
Introduced in commit a510b61613 [0][1]. The fix is to include the definitions from `lib/clz_ctz.c` [2].
The same problem was fixed for s390 in this patch [3].
CC: Paul Cercueil
Best,
Nick Terrell
[0] https://lore.kernel.org/lkml/20200901142651.1165237-3-paul@crapouillou.net/
[1] https://github.com/torvalds/linux/commit/a510b616131f85215ba156ed67e5ed1c0701f80f
[2] https://github.com/torvalds/linux/blob/master/lib/clz_ctz.c
[3] https://lore.kernel.org/all/patch-1.thread-f0f589.git-f0f58936888f.your-ad-here.call-01627564869-ext-2765@work.hours/
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v4] KEYS: encrypted: Instantiate key with user-provided decrypted data
From: Yael Tiomkin @ 2022-01-05 20:18 UTC (permalink / raw)
To: Sumit Garg
Cc: Mimi Zohar, linux-integrity, jejb, Jarkko Sakkinen, corbet,
dhowells, jmorris, serge, keyrings, linux-doc, linux-kernel,
linux-security-module, Jan Lübbe, Ahmad Fatoum
In-Reply-To: <CAFA6WYOJt3=YMTt_QQSq6Z-MK42hwWspgSpasw2fuPtVFcP3uA@mail.gmail.com>
Hi Sumit,
On Mon, Jan 3, 2022 at 1:51 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Hi Mimi,
>
> Apologies for the delayed reply as I was on leave for a long new year weekend.
>
> On Thu, 30 Dec 2021 at 18:59, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > Hi Sumit,
> >
> > On Thu, 2021-12-30 at 15:37 +0530, Sumit Garg wrote:
> > > + Jan, Ahmad
> > >
> > > On Thu, 30 Dec 2021 at 03:24, Yael Tiomkin <yaelt@google.com> wrote:
> > > >
> > > > The encrypted.c class supports instantiation of encrypted keys with
> > > > either an already-encrypted key material, or by generating new key
> > > > material based on random numbers. This patch defines a new datablob
> > > > format: [<format>] <master-key name> <decrypted data length>
> > > > <decrypted data> that allows to instantiate encrypted keys using
> > > > user-provided decrypted data, and therefore allows to perform key
> > > > encryption from userspace. The decrypted key material will be
> > > > inaccessible from userspace.
> > >
> > > This type of user-space key import feature has already been discussed
> > > at large in the context of trusted keys here [1]. So what makes it
> > > special in case of encrypted keys such that it isn't a "UNSAFE_IMPORT"
> > > or "DEBUGGING_IMPORT" or "DEVELOPMENT_IMPORT", ...?
> > >
> > > [1] https://lore.kernel.org/linux-integrity/74830d4f-5a76-8ba8-aad0-0d79f7c01af9@pengutronix.de/
> > >
> > > -Sumit
> > >
> > > >
> > > > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > Signed-off-by: Yael Tiomkin <yaelt@google.com>
> >
> > There is a difference between trusted and encrypted keys.
>
> Yeah I understand the implementation differences.
>
> > So in
> > addition to pointing to the rather long discussion thread, please
> > summarize the conclusion and, assuming you agree, include why in once
> > case it was acceptable and in the other it wasn't to provide userspace
> > key data.
>
> My major concern with importing user-space key data in *plain* format
> is that if import is *not* done in a safe (manufacturing or
> production) environment then the plain key data is susceptible to
> user-space compromises when the device is in the field.
I agree this can happen. Key distribution in any scenario needs to be
secure and this could also potentially be an issue if the key is first
encrypted and then imported. We can make sure the documentation
highlights the safety requirement.
>
> And it sounds like we are diverting from basic definition [1] of encrypted keys:
>
> "Trusted and Encrypted Keys are two new key types added to the
> existing kernel key ring service. Both of these new types are variable
> length symmetric keys, and in both cases all keys are created in the
> kernel, and **user space sees, stores, and loads** only encrypted
> blobs."
>
> Also, as Jarrko mentioned earlier the use-case is still not clear to
> me as well. Isn't user logon keys an alternative option for
> non-readable user-space keys?
The goal in this change is to allow key encryption from userspace,
using user-provided decrypted data. This cannot be achieved in logon
keys, which as you mentioned, are simply non-readable user type keys.
>
> [1] https://www.kernel.org/doc/html/v4.13/security/keys/trusted-encrypted.html
>
> -Sumit
>
> >
> > thanks,
> >
> > Mimi
> >
Yael
^ permalink raw reply
* Re: [PATCH v2 0/8] RFC: Server side merges (no ref updating, no commit creating, no touching worktree or index)
From: Junio C Hamano @ 2022-01-05 20:18 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget
Cc: git, Christian Couder, Taylor Blau, Johannes Altmanninger,
Elijah Newren
In-Reply-To: <pull.1114.v2.git.git.1641403655.gitgitgadget@gmail.com>
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> This series introduces a new option to git-merge-tree: --real (best name I
> could come up with). This new option is designed to allow a server-side
> "real" merge (or allow folks client-side to do merges with branches they
> don't even have checked out).
Finally. merge-tree was added by Linus mostly as a demonstration of
idea to trick other developers into enhancing it to implement a full
merge that does not need to touch the index or the working tree, but
everybody failed to be enticed by it so far. It is true that it can
be used server-side, but I do not think that is what we want to sell
it as (after all, receiving a push, merging it to the history in the
central repository, and checking the result out to the working tree,
would be a good "server-side" operation to have, but it can be done
today without this series). The selling point would rather be it is
done mostly in-core, without touching working tree or the index file,
no?
Exciting ;-).
^ permalink raw reply
* Re: decompress.c:(.text.FSE_buildDTable_internal+0x2cc): undefined reference to `__clzdi2'
From: Nick Terrell @ 2022-01-05 20:18 UTC (permalink / raw)
To: kbuild-all
In-Reply-To: <202201060233.mO6P39bM-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 4091 bytes --]
> On Jan 5, 2022, at 11:02 AM, kernel test robot <lkp@intel.com> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: c9e6606c7fe92b50a02ce51dda82586ebdf99b48
> commit: e0c1b49f5b674cca7b10549c53b3791d0bbc90a8 lib: zstd: Upgrade to latest upstream zstd version 1.4.10
> date: 8 weeks ago
> config: mips-randconfig-r025-20220105 (https://download.01.org/0day-ci/archive/20220106/202201060233.mO6P39bM-lkp(a)intel.com/config )
> compiler: mips64el-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://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e0c1b49f5b674cca7b10549c53b3791d0bbc90a8
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout e0c1b49f5b674cca7b10549c53b3791d0bbc90a8
> # 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=mips 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 >>):
>
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `FSE_buildDTable_internal':
>>> decompress.c:(.text.FSE_buildDTable_internal+0x2cc): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `BIT_initDStream':
> decompress.c:(.text.BIT_initDStream+0x7c): undefined reference to `__clzdi2'
> mips64el-linux-ld: decompress.c:(.text.BIT_initDStream+0x158): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `ZSTD_buildFSETable_body_default.constprop.0':
>>> decompress.c:(.text.ZSTD_buildFSETable_body_default.constprop.0+0x2a8): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `FSE_readNCount_body_default':
>>> decompress.c:(.text.FSE_readNCount_body_default+0x130): undefined reference to `__ctzdi2'
>>> mips64el-linux-ld: decompress.c:(.text.FSE_readNCount_body_default+0x1a4): undefined reference to `__ctzdi2'
>>> mips64el-linux-ld: decompress.c:(.text.FSE_readNCount_body_default+0x2e4): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `HUF_readStats_body_default':
>>> decompress.c:(.text.HUF_readStats_body_default+0x184): undefined reference to `__clzdi2'
>>> mips64el-linux-ld: decompress.c:(.text.HUF_readStats_body_default+0x1b4): undefined reference to `__clzdi2'
> mips64el-linux-ld: arch/mips/boot/compressed/decompress.o: in function `ZSTD_DCtx_getParameter':
>>> decompress.c:(.text.ZSTD_DCtx_getParameter+0x60): undefined reference to `__clzdi2'
These are complaining about usage of __builtin_c{l,t}z(). The code before this change also used
__builtin_clz(), so this isn't new. E.g. the call in BIT_initDStream() is present before the change,
it just moved locations. So these errors aren't new.
This is breaking the MIPS boot code when zstd compressed kernels are enabled, and was
Introduced in commit a510b61613 [0][1]. The fix is to include the definitions from `lib/clz_ctz.c` [2].
The same problem was fixed for s390 in this patch [3].
CC: Paul Cercueil
Best,
Nick Terrell
[0] https://lore.kernel.org/lkml/20200901142651.1165237-3-paul(a)crapouillou.net/
[1] https://github.com/torvalds/linux/commit/a510b616131f85215ba156ed67e5ed1c0701f80f
[2] https://github.com/torvalds/linux/blob/master/lib/clz_ctz.c
[3] https://lore.kernel.org/all/patch-1.thread-f0f589.git-f0f58936888f.your-ad-here.call-01627564869-ext-2765(a)work.hours/
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply
* Re: [PATCH V7 17/29] pci: export functions for cpr
From: Michael S. Tsirkin @ 2022-01-05 20:16 UTC (permalink / raw)
To: Steven Sistare
Cc: Daniel P. Berrange, Juan Quintela, Jason Zeng, Alex Bennée,
qemu-devel, Eric Blake, Dr. David Alan Gilbert, Zheng Chuan,
Alex Williamson, Stefan Hajnoczi, Marc-André Lureau,
Paolo Bonzini, Philippe Mathieu-Daudé, Markus Armbruster
In-Reply-To: <02a093fe-b2be-2ecb-7ef7-a95c54443894@oracle.com>
On Wed, Jan 05, 2022 at 12:22:25PM -0500, Steven Sistare wrote:
> On 12/22/2021 6:07 PM, Michael S. Tsirkin wrote:
> > On Wed, Dec 22, 2021 at 11:05:22AM -0800, Steve Sistare wrote:
> >> Export msix_is_pending, msix_init_vector_notifiers, and pci_update_mappings
> >> for use by cpr. No functional change.
> >>
> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> >
> > With things like that, I prefer when the API is exported
> > together with the patch that uses it.
> > This was I can see why we are exporting these APIs.
> > Esp wrt pci_update_mappings, it's designed as an
> > internal API.
>
> Hi Michael, thanks very much for reviewing these patches.
>
> Serendipitously, I stopped calling pci_update_mappings from vfio code earlier
> in the series. I will revert its scope.
>
> I would prefer to keep this patch separate from the use of these functions in
> "vfio-pci cpr part 2 msi", to make the latter smaller and easier to understand.
> How about if I say more in this commit message? :
>
> Export msix_is_pending and msix_init_vector_notifiers for use in vfio cpr.
> Both are needed in the vfio-pci post-load function during cpr-load.
> msix_is_pending is checked to enable the PBA memory region.
> msix_init_vector_notifiers is called to register notifier callbacks, without
> the other side effects of msix_set_vector_notifiers.
>
> - Steve
Well the reason the side effects are there is to avoid losing events,
no? I'd like to figure out a bit better why we don't need them, and when
should users call msix_init_vector_notifiers versus
msix_set_vector_notifiers.
> >> ---
> >> hw/pci/msix.c | 20 ++++++++++++++------
> >> hw/pci/pci.c | 3 +--
> >> include/hw/pci/msix.h | 5 +++++
> >> include/hw/pci/pci.h | 1 +
> >> 4 files changed, 21 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> >> index ae9331c..73f4259 100644
> >> --- a/hw/pci/msix.c
> >> +++ b/hw/pci/msix.c
> >> @@ -64,7 +64,7 @@ static uint8_t *msix_pending_byte(PCIDevice *dev, int vector)
> >> return dev->msix_pba + vector / 8;
> >> }
> >>
> >> -static int msix_is_pending(PCIDevice *dev, int vector)
> >> +int msix_is_pending(PCIDevice *dev, unsigned int vector)
> >> {
> >> return *msix_pending_byte(dev, vector) & msix_pending_mask(vector);
> >> }
> >> @@ -579,6 +579,17 @@ static void msix_unset_notifier_for_vector(PCIDevice *dev, unsigned int vector)
> >> dev->msix_vector_release_notifier(dev, vector);
> >> }
> >>
> >> +void msix_init_vector_notifiers(PCIDevice *dev,
> >> + MSIVectorUseNotifier use_notifier,
> >> + MSIVectorReleaseNotifier release_notifier,
> >> + MSIVectorPollNotifier poll_notifier)
> >> +{
> >> + assert(use_notifier && release_notifier);
> >> + dev->msix_vector_use_notifier = use_notifier;
> >> + dev->msix_vector_release_notifier = release_notifier;
> >> + dev->msix_vector_poll_notifier = poll_notifier;
> >> +}
> >> +
> >> int msix_set_vector_notifiers(PCIDevice *dev,
> >> MSIVectorUseNotifier use_notifier,
> >> MSIVectorReleaseNotifier release_notifier,
> >> @@ -586,11 +597,8 @@ int msix_set_vector_notifiers(PCIDevice *dev,
> >> {
> >> int vector, ret;
> >>
> >> - assert(use_notifier && release_notifier);
> >> -
> >> - dev->msix_vector_use_notifier = use_notifier;
> >> - dev->msix_vector_release_notifier = release_notifier;
> >> - dev->msix_vector_poll_notifier = poll_notifier;
> >> + msix_init_vector_notifiers(dev, use_notifier, release_notifier,
> >> + poll_notifier);
> >>
> >> if ((dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &
> >> (MSIX_ENABLE_MASK | MSIX_MASKALL_MASK)) == MSIX_ENABLE_MASK) {
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index e5993c1..0fd21e1 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> >> @@ -225,7 +225,6 @@ static const TypeInfo pcie_bus_info = {
> >> };
> >>
> >> static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
> >> -static void pci_update_mappings(PCIDevice *d);
> >> static void pci_irq_handler(void *opaque, int irq_num, int level);
> >> static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
> >> static void pci_del_option_rom(PCIDevice *pdev);
> >> @@ -1366,7 +1365,7 @@ static pcibus_t pci_bar_address(PCIDevice *d,
> >> return new_addr;
> >> }
> >>
> >> -static void pci_update_mappings(PCIDevice *d)
> >> +void pci_update_mappings(PCIDevice *d)
> >> {
> >> PCIIORegion *r;
> >> int i;
> >> diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
> >> index 4c4a60c..46606cf 100644
> >> --- a/include/hw/pci/msix.h
> >> +++ b/include/hw/pci/msix.h
> >> @@ -32,6 +32,7 @@ int msix_present(PCIDevice *dev);
> >> bool msix_is_masked(PCIDevice *dev, unsigned vector);
> >> void msix_set_pending(PCIDevice *dev, unsigned vector);
> >> void msix_clr_pending(PCIDevice *dev, int vector);
> >> +int msix_is_pending(PCIDevice *dev, unsigned vector);
> >>
> >> int msix_vector_use(PCIDevice *dev, unsigned vector);
> >> void msix_vector_unuse(PCIDevice *dev, unsigned vector);
> >> @@ -41,6 +42,10 @@ void msix_notify(PCIDevice *dev, unsigned vector);
> >>
> >> void msix_reset(PCIDevice *dev);
> >>
> >> +void msix_init_vector_notifiers(PCIDevice *dev,
> >> + MSIVectorUseNotifier use_notifier,
> >> + MSIVectorReleaseNotifier release_notifier,
> >> + MSIVectorPollNotifier poll_notifier);
> >> int msix_set_vector_notifiers(PCIDevice *dev,
> >> MSIVectorUseNotifier use_notifier,
> >> MSIVectorReleaseNotifier release_notifier,
> >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> >> index e7cdf2d..cc63dd4 100644
> >> --- a/include/hw/pci/pci.h
> >> +++ b/include/hw/pci/pci.h
> >> @@ -910,5 +910,6 @@ extern const VMStateDescription vmstate_pci_device;
> >>
> >> MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
> >> void pci_set_power(PCIDevice *pci_dev, bool state);
> >> +void pci_update_mappings(PCIDevice *d);
> >>
> >> #endif
> >> --
> >> 1.8.3.1
> >
^ permalink raw reply
* [PATCH v1.1 8/8] media: v4l2-mediabus: Drop V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag
From: Laurent Pinchart @ 2022-01-05 20:15 UTC (permalink / raw)
To: linux-media
Cc: linux-renesas-soc, Sakari Ailus, Hans Verkuil, Kieran Bingham,
Jacopo Mondi, Niklas Söderlund, Tomi Valkeinen,
Janusz Krzysztofik, Lars-Peter Clausen, Mats Randgaard
In-Reply-To: <YdXrX9VLOm3W3YBl@valkosipuli.retiisi.eu>
MIPI CSI-2 continuous and non-continuous clock modes are mutually
exclusive. Drop the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag and use
V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK only.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
Changes since v1:
- Check all CSI2 flags in v4l2_fwnode_endpoint_parse_csi2_bus()
---
drivers/media/i2c/adv7180.c | 3 +--
drivers/media/i2c/tc358743.c | 6 +++---
drivers/media/v4l2-core/v4l2-fwnode.c | 6 ++----
include/media/v4l2-mediabus.h | 3 +--
4 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 3ff37a550810..4f5db195e66d 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -785,8 +785,7 @@ static int adv7180_get_mbus_config(struct v4l2_subdev *sd,
if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
cfg->type = V4L2_MBUS_CSI2_DPHY;
cfg->bus.mipi_csi2.num_data_lanes = 1;
- cfg->bus.mipi_csi2.flags =
- V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ cfg->bus.mipi_csi2.flags = 0;
} else {
/*
* The ADV7180 sensor supports BT.601/656 output modes.
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index dfbc42675143..e18b8947ad7e 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -717,7 +717,7 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
((lanes > 3) ? MASK_D3M_HSTXVREGEN : 0x0));
i2c_wr32(sd, TXOPTIONCNTRL, (state->bus.flags &
- V4L2_MBUS_CSI2_CONTINUOUS_CLOCK) ? MASK_CONTCLKMODE : 0);
+ V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK) ? 0 : MASK_CONTCLKMODE);
i2c_wr32(sd, STARTCNTRL, MASK_START);
i2c_wr32(sd, CSI_START, MASK_STRT);
@@ -1613,7 +1613,7 @@ static int tc358743_get_mbus_config(struct v4l2_subdev *sd,
cfg->type = V4L2_MBUS_CSI2_DPHY;
/* Support for non-continuous CSI-2 clock is missing in the driver */
- cfg->bus.mipi_csi2.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ cfg->bus.mipi_csi2.flags = 0;
cfg->bus.mipi_csi2.num_data_lanes = state->csi_lanes_in_use;
return 0;
@@ -2039,7 +2039,7 @@ static int tc358743_probe(struct i2c_client *client)
/* platform data */
if (pdata) {
state->pdata = *pdata;
- state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ state->bus.flags = 0;
} else {
err = tc358743_probe_of(state);
if (err == -ENODEV)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 9ff3ebb230e7..71dcc9a96535 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -207,13 +207,11 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode,
if (fwnode_property_present(fwnode, "clock-noncontinuous")) {
flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
pr_debug("non-continuous clock\n");
- } else {
- flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
}
if (bus_type == V4L2_MBUS_CSI2_DPHY ||
- bus_type == V4L2_MBUS_CSI2_CPHY || lanes_used ||
- have_clk_lane || (flags & ~V4L2_MBUS_CSI2_CONTINUOUS_CLOCK)) {
+ bus_type == V4L2_MBUS_CSI2_CPHY ||
+ lanes_used || have_clk_lane || flags) {
/* Only D-PHY has a clock lane. */
unsigned int dfl_data_lane_index =
bus_type == V4L2_MBUS_CSI2_DPHY;
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index c6626a22b394..e0db3bcff9ed 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -68,8 +68,7 @@
/* Serial flags */
/* Clock non-continuous mode support. */
-#define V4L2_MBUS_CSI2_CONTINUOUS_CLOCK BIT(8)
-#define V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK BIT(9)
+#define V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK BIT(0)
#define V4L2_MBUS_CSI2_MAX_DATA_LANES 8
base-commit: 68b9bcc8a534cd11fe55f8bc82f948aae7d81b3c
prerequisite-patch-id: cc085f7de462ead19772e3f64e1b1d13bb92d5b7
prerequisite-patch-id: d6943d1780603964a753d571e26bad98b4954a8a
prerequisite-patch-id: c63eaa2a61aeea6d4be238644f1324f0170d981c
prerequisite-patch-id: 797385e385b3f71f0bafe7eb8e712d52628c23d7
prerequisite-patch-id: 2f7fe0901f79ac1d445e443b5d3ee7ab9ae41525
prerequisite-patch-id: 70ddd068308b5758872349ec6e0f03e8c9427dc0
prerequisite-patch-id: 987cc8f39cb63badf2d6ceba946deffafba9a249
--
Regards,
Laurent Pinchart
^ permalink raw reply related
* Re: [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs
From: Stephen Boyd @ 2022-01-05 20:15 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, devicetree,
dianders, judyhsiao, linux-arm-msm, linux-kernel, robh+dt,
rohitkr, srinivas.kandagatla
Cc: Venkata Prasad Potturu
In-Reply-To: <1641208380-15510-2-git-send-email-quic_srivasam@quicinc.com>
Quoting Srinivasa Rao Mandadapu (2022-01-03 03:12:58)
> SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
> codecs like WCD938x, max98360a using soundwire masters and i2s bus.
> Add these nodes for sc7280 based platforms audio use case.
> Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 4 ++
> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 52 ++++++++++++++
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 113 +++++++++++++++++++++++++++++++
> 3 files changed, 169 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index cd2755c..035cca9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -72,6 +72,10 @@ ap_ts_pen_1v8: &i2c13 {
> pins = "gpio51";
> };
>
> +&wcd938x {
> + qcom,us-euro-gpios = <&tlmm 81 0>;
Why is this a qcom prefix vs. a standard foo-gpios property?
> +};
> +
> &tlmm {
> tp_int_odl: tp-int-odl {
> pins = "gpio7";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index ddeb508..94614c9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -636,3 +636,55 @@
> bias-pull-up;
> };
> };
> +
> +&swr0 {
> + wcd_rx: wcd938x-rx{
space before { please
Also, are these speakers or amps or something like that? I'd expect the
node name to be more generic.
> + compatible = "sdw20217010d00";
> + reg = <0 4>;
> + #sound-dai-cells = <1>;
> + qcom,rx-port-mapping = <1 2 3 4 5>;
> + };
> +};
> +
> +&swr1 {
> + wcd_tx: wcd938x-tx{
> + compatible = "sdw20217010d00";
> + reg = <0 3>;
> + #sound-dai-cells = <1>;
> + qcom,tx-port-mapping = <1 2 3 4>;
> + };
> +};
> +
> +&soc {
> + max98360a: audio-codec-0 {
This shouldn't be a child of the soc node.
> + compatible = "maxim,max98360a";
> + pinctrl-names = "default";
> + pinctrl-0 = <&_en>;
> + sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
> + #sound-dai-cells = <0>;
> + };
> +
> + wcd938x: codec {
Same comment.
> + compatible = "qcom,wcd9380-codec";
> + #sound-dai-cells = <1>;
> +
> + reset-gpios = <&tlmm 83 0>;
> +
> + qcom,rx-device = <&wcd_rx>;
> + qcom,tx-device = <&wcd_tx>;
> +
> + vdd-rxtx-supply = <&vreg_l18b_1p8>;
> + vdd-io-supply = <&vreg_l18b_1p8>;
> + vdd-buck-supply = <&vreg_l17b_1p8>;
> + vdd-mic-bias-supply = <&vreg_bob>;
> +
> + qcom,micbias1-microvolt = <1800000>;
> + qcom,micbias2-microvolt = <1800000>;
> + qcom,micbias3-microvolt = <1800000>;
> + qcom,micbias4-microvolt = <1800000>;
> +
> + qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>;
> + qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> + qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 6233f2c..c0d9de3 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1744,6 +1744,119 @@
> #clock-cells = <1>;
> };
>
> + rxmacro: rxmacro@3200000 {
What is rxmacro? Maybe 'soundwire'?
> + pinctrl-names = "default";
> + pinctrl-0 = <&rx_swr_active>;
> + compatible = "qcom,sc7280-lpass-rx-macro";
> + reg = <0 0x3200000 0 0x1000>;
> +
> + clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
> + <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
> + <&vamacro>;
> + clock-names = "mclk", "npl", "fsgen";
> + #clock-cells = <0>;
> + clock-frequency = <9600000>;
> + clock-output-names = "mclk";
> + #sound-dai-cells = <1>;
> + };
> +
> + /* rx macro */
> + swr0: soundwire-controller@3210000 {
Is 'controller' redundant? i.e. soundwire is always a controller? Maybe
'soundwire' is better
> + reg = <0 0x3210000 0 0x2000>;
> + compatible = "qcom,soundwire-v1.6.0";
> + interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&rxmacro>;
> + clock-names = "iface";
> + label = "RX";
> +
> + qcom,din-ports = <0>;
> + qcom,dout-ports = <5>;
> + qcom,swrm-hctl-reg = <0x032a90a0>;
> +
> + qcom,ports-word-length = /bits/ 8 <0x01 0x07 0x04 0xFF 0xFF>;
> + qcom,ports-sinterval-low = /bits/ 8 <0x03 0x3F 0x1F 0x03 0x03>;
> + qcom,ports-offset1 = /bits/ 8 <0x00 0x00 0x0B 0x01 0x01>;
> + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x0B 0x00 0x00>;
> + qcom,ports-lane-control = /bits/ 8 <0x01 0x00 0x00 0x00 0x00>;
> + qcom,ports-block-pack-mode = /bits/ 8 <0xFF 0x00 0x01 0xFF 0xFF>;
> + qcom,ports-hstart = /bits/ 8 <0xFF 0x03 0xFF 0xFF 0xFF>;
> + qcom,ports-hstop = /bits/ 8 <0xFF 0x06 0xFF 0xFF 0xFF>;
> + qcom,ports-block-group-count = /bits/ 8 <0xFF 0xFF 0xFF 0xFF 0x00>;
Why is this bit stuff in DT vs. in the device driver?
> +
> + #sound-dai-cells = <1>;
> + #address-cells = <2>;
> + #size-cells = <0>;
> + };
> +
> + txmacro: txmacro@3220000 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&tx_swr_active>;
> + compatible = "qcom,sc7280-lpass-tx-macro";
> + reg = <0 0x3220000 0 0x1000>;
> +
> + clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
> + <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
> + <&vamacro>;
Please align the clocks here so the < starts on the same column.
> + clock-names = "mclk", "npl", "fsgen";
> +
> + #clock-cells = <0>;
> + clock-frequency = <9600000>;
> + clock-output-names = "mclk";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + #sound-dai-cells = <1>;
> + };
> +
> + /* tx macro */
Do we need these comments? Please remove them as they're practically
useless.
> + swr1: soundwire-controller@3230000 {
> + reg = <0 0x3230000 0 0x2000>;
> + compatible = "qcom,soundwire-v1.6.0";
> +
> + interrupts-extended =
> + <&intc GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH>,
> + <&pdc 130 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "swr_master_irq", "swr_wake_irq";
> + clocks = <&txmacro>;
> + clock-names = "iface";
> + label = "TX";
> +
> + qcom,din-ports = <3>;
> + qcom,dout-ports = <0>;
> + qcom,swrm-hctl-reg = <0x032a90a8>;
This looks like some common register location for soundwire oops bits.
Is it another device? Or maybe it's a common register base that needs to
be a syscon?
> +
> + qcom,ports-sinterval-low = /bits/ 8 <0x01 0x03 0x03>;
> + qcom,ports-offset1 = /bits/ 8 <0x01 0x00 0x02>;
> + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x00>;
> + qcom,ports-hstart = /bits/ 8 <0xFF 0xFF 0xFF>;
> + qcom,ports-hstop = /bits/ 8 <0xFF 0xFF 0xFF>;
> + qcom,ports-word-length = /bits/ 8 <0xFF 0x0 0xFF>;
> + qcom,ports-block-pack-mode = /bits/ 8 <0xFF 0xFF 0xFF>;
> + qcom,ports-block-group-count = /bits/ 8 <0xFF 0xFF 0xFF>;
> + qcom,ports-lane-control = /bits/ 8 <0x00 0x01 0x00>;
> + qcom,port-offset = <1>;
> +
> + #sound-dai-cells = <1>;
> + #address-cells = <2>;
> + #size-cells = <0>;
> + };
> +
> + vamacro: codec@3370000 {
> + compatible = "qcom,sc7280-lpass-va-macro";
> + pinctrl-0 = <&dmic01_active>;
> + pinctrl-names = "default";
> +
> + vdd-micb-supply = <&vreg_bob>;
The supplies need to be set in the board file, no the soc file as they
can be different depending on what the hardware engineer wires to the
pins on the SoC.
> + reg = <0 0x3370000 0 0x1000>;
> + clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
> + <&lpass_audiocc LPASS_AUDIO_CC_RX_MCLK_CLK>;
> + clock-names = "mclk", "macro";
> +
> + #clock-cells = <0>;
> + clock-frequency = <9600000>;
> + clock-output-names = "fsgen";
> + #sound-dai-cells = <1>;
> + };
> +
> lpass_tlmm: pinctrl@33c0000 {
> compatible = "qcom,sc7280-lpass-lpi-pinctrl";
> reg = <0 0x33c0000 0x0 0x20000>,
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH][TRIVIAL] btrfs-progs: Allow autodetect_object_types() to handle the link.
From: Goffredo Baroncelli @ 2022-01-05 20:14 UTC (permalink / raw)
To: Boris Burkov; +Cc: linux-btrfs
In-Reply-To: <9a261326-716b-69b6-9e95-bd5e1942e6bd@inwind.it>
Hi all,
On 05/01/2022 20.14, Goffredo Baroncelli wrote:
> On 05/01/2022 20.05, Boris Burkov wrote:
>> On Wed, Jan 05, 2022 at 07:23:33PM +0100, Goffredo Baroncelli wrote:
>>> On 05/01/2022 18.50, Boris Burkov wrote:
>>>> On Wed, Jan 05, 2022 at 02:32:58PM +0100, Goffredo Baroncelli wrote:
>>>>> From: Goffredo Baroncelli <kreijack@inwind.it>
>>> [...]
>>>>
>>>> I took a look at the original lstat and it doesn't seem super strongly
>>>> motivated. It is used to detect a subvolume object (ino==256) so I don't
>>>> see why we would hate to have property get/set work on a symlink to a
>>>> subvol.
>>>
>>> It is not so simple: think about a case where the symlink points to a
>>> subvolume of *another* filesystem.
>>>
>>> Now, "btrfs prop get" returns the property (e.g. the label) of the *underlining*
>>> filesystem. If we change statl to stat, it still return the property of the
>>> underlining filesystem, thinking that it is a subvolume (of a foreign filesystem).
>>>
>>> If fact I added an exception of that rule, because if we pass a block device, we
>>> don't consider the underling filesystem, but the filesystem contained in the block
>>> device. But there is a precedence in get/set label.
>>>
>>> Anyway, symlink created some confusion, and I bet that in btrfs there are areas
>>> with incoherence around symlink between the pointed object and the underling
>>> filesystem.
>>
>> Good point. I agree that btrfs (the tool) is not the most rigorous with
>> symlinks. In this very function, we check if it is a btrfs object by
>> opening the file without O_NOFOLLOW, but then we do this lstat.
>>
>> I'm not exactly sure what you are alluding to regarding the precedent set
>> by label, but I tested links and labels and it seems to exhibit the link
>> following behavior.
>>
>> mkfs.btrfs -f /dev/loop0
>> mkfs.btrfs -f -L LOOP /dev/loop1
>> mount /dev/loop0 /mnt/0
>> mount /dev/loop1 /mnt/1
>> ln -s /mnt/1 /mnt/0/lnk
>> btrfs property get /mnt/0 label
>> label=
>> btrfs property get /mnt/1 label
>> label=LOOP
>> btrfs property get /mnt/0/lnk label
>> label=LOOP
>> btrfs property get /mnt/0/lnk ro
>> ERROR: object is not compatible with property: ro
>>
>> So it looks like root detection follows links but subvol detection does
>> not. Without testing, but judging by the code, I think inode follows
>> symlinks too. So with all that said, I think the most consistent is to
>> make subvol follow the symlink, unless you have a really confusing
>> unexpected behavior in mind with links out to another btrfs that I am
>> failing to see.
>
> Hi Boris,
>
> you are right. I didn't consider that open(path) follows the symlink too.
> So I will update the patch changing statl() to stat() and removing the
> 2nd stat invocation.
Only now I noticed that the code behind set/get_label works only with
"regular" file or "block" device.
This may be a more cleaner solution to avoid this kind of ambiguity.
Of course we can add exception where required.
BR
G.Baroncelli
> BR
> G.Baroncelli
>
>> Thanks,
>> Boris
>>
>>>
>>> BR
>>> G.Baroncelli
>>>
>>> --
>>> gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
>>> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
>
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply
* [chenxing:msc313_mainlining 85/86] Error: arch/arm/boot/dts/mstar-infinity2m-ssd201-wirelesstag-ido-som2d01.dtsi:18.1-9 Label or path pm_uart not found
From: kernel test robot @ 2022-01-05 20:13 UTC (permalink / raw)
To: Daniel Palmer; +Cc: kbuild-all, linux-kernel
tree: git://github.com/linux-chenxing/linux.git msc313_mainlining
head: 2269f75a5b53b9a05d21d432ad75f5e41b344814
commit: 7bf46c9e0566d3046584d7fed53d1587c316fde5 [85/86] ARM: dts: mstar: Add top level dts for ssd201-som2d01
config: arm-buildonly-randconfig-r001-20220105 (https://download.01.org/0day-ci/archive/20220106/202201060459.ALRhCSGK-lkp@intel.com/config)
compiler: arm-linux-gnueabi-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/linux-chenxing/linux/commit/7bf46c9e0566d3046584d7fed53d1587c316fde5
git remote add chenxing git://github.com/linux-chenxing/linux.git
git fetch --no-tags chenxing msc313_mainlining
git checkout 7bf46c9e0566d3046584d7fed53d1587c316fde5
# 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=arm 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 >>):
>> Error: arch/arm/boot/dts/mstar-infinity2m-ssd201-wirelesstag-ido-som2d01.dtsi:18.1-9 Label or path pm_uart not found
>> FATAL ERROR: Syntax error parsing input tree
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH] dt-bindings: spi-mux: Add reference to spi-peripheral-props.yaml schema
From: Chris Packham @ 2022-01-05 20:13 UTC (permalink / raw)
To: Rob Herring, Mark Brown
Cc: linux-spi@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20220105183234.3426649-1-robh@kernel.org>
On 6/01/22 7:32 am, Rob Herring wrote:
> The spi-mux serves as both a SPI peripheral and controller, so add a
> reference to spi-peripheral-props.yaml in addition to
> spi-controller.yaml. This is necessary to avoid unevaluatedProperties
> warnings once those are fully enabled.
>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Cc: linux-spi@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Documentation/devicetree/bindings/spi/spi-mux.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-mux.yaml b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> index 51c7622dc20b..7ea79f6d33f3 100644
> --- a/Documentation/devicetree/bindings/spi/spi-mux.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-mux.yaml
> @@ -31,6 +31,7 @@ description: |
>
> allOf:
> - $ref: "/schemas/spi/spi-controller.yaml#"
> + - $ref: "/schemas/spi/spi-peripheral-props.yaml#"
>
> maintainers:
> - Chris Packham <chris.packham@alliedtelesis.co.nz>
^ permalink raw reply
* Bug report - sparse-checkout ignores prefix when run in subdirectories
From: Lessley Dennington @ 2022-01-05 20:13 UTC (permalink / raw)
To: Git Mailing List; +Cc: Elijah Newren, Derrick Stolee, Junio C Hamano
Hello everyone! See the following bug report pertaining to sparse-checkout
when run outside top-level directories.
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
$ git init -b main tmp
$ cd tmp
$ mkdir -p a/b/c
$ touch a/b/c/d a/b/c/e
$ git add a/
$ git commit -m "initial"
$ cd a/ # Not at the top level anymore
$ git sparse-checkout set --cone b/c
$ git -C .. sparse-checkout list
What did you expect to happen? (Expected behavior)
We expected the output sparsity path to be a/b/c.
What happened instead? (Actual behavior)
The output was b/c. Running $ cat ../.git/info/sparse-checkout
showed that the paths in this file were incorrect:
/*
!/*/
/b/
!/b/*/
/b/c/
What's different between what you expected and what actually happened?
The output path and the paths in the sparse-checkout file are missing the
expected 'a' prefix. This means that the `set` and `add` subcommands of
`sparse-checkout` can only be safely run from a repo's top-level directory.
Anything else you want to add:
This was discovered by newren@gmail.com in my recent tab-completion
series. You can find additional context at [1].
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.34.1.88.gcecf501e07.dirty (local build, based off
en/sparse-checkout-set)
cpu: x86_64
shell-path: /bin/zsh
uname: Darwin
compiler info:
Configured with: --prefix=/Library/Developer/CommandLineTools/usr
--with-gxx-include-dir=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk
/usr/include/c++/4.2.1
Apple clang version 13.0.0 (clang-1300.0.29.3)
Target: x86_64-apple-darwin20.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
$SHELL (typically, interactive shell): /bin/zsh
-Lessley
[1]:
https://lore.kernel.org/git/CABPp-BGdr54XgCXw8k1xRCgkwBtDonyODS3O+_nS_QY3SOEFGQ@mail.gmail.com/
^ permalink raw reply
* [chenxing:msc313_mainlining 85/86] Error: arch/arm/boot/dts/mstar-infinity2m-ssd201-wirelesstag-ido-som2d01.dtsi:18.1-9 Label or path pm_uart not found
From: kernel test robot @ 2022-01-05 20:13 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]
tree: git://github.com/linux-chenxing/linux.git msc313_mainlining
head: 2269f75a5b53b9a05d21d432ad75f5e41b344814
commit: 7bf46c9e0566d3046584d7fed53d1587c316fde5 [85/86] ARM: dts: mstar: Add top level dts for ssd201-som2d01
config: arm-buildonly-randconfig-r001-20220105 (https://download.01.org/0day-ci/archive/20220106/202201060459.ALRhCSGK-lkp(a)intel.com/config)
compiler: arm-linux-gnueabi-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/linux-chenxing/linux/commit/7bf46c9e0566d3046584d7fed53d1587c316fde5
git remote add chenxing git://github.com/linux-chenxing/linux.git
git fetch --no-tags chenxing msc313_mainlining
git checkout 7bf46c9e0566d3046584d7fed53d1587c316fde5
# 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=arm 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 >>):
>> Error: arch/arm/boot/dts/mstar-infinity2m-ssd201-wirelesstag-ido-som2d01.dtsi:18.1-9 Label or path pm_uart not found
>> FATAL ERROR: Syntax error parsing input tree
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply
* Re: nfsd v4 server can crash in COPY_NOTIFY
From: J. Bruce Fields @ 2022-01-05 20:13 UTC (permalink / raw)
To: rtm; +Cc: Chuck Lever, linux-nfs, Olga Kornievskaia
In-Reply-To: <7318.1641394756@crash.local>
On Wed, Jan 05, 2022 at 09:59:16AM -0500, rtm@csail.mit.edu wrote:
> If the special ONE stateid is passed to nfs4_preprocess_stateid_op(),
> it returns status=0 but does not set *cstid. nfsd4_copy_notify()
> depends on stid being set if status=0, and thus can crash if the
> client sends the right COPY_NOTIFY RPC.
>
> I've attached a demo.
>
> # uname -a
> Linux (none) 5.16.0-rc7-00108-g800829388818-dirty #28 SMP Wed Jan 5 14:40:37 UTC 2022 riscv64 riscv64 riscv64 GNU/Linux
> # cc nfsd_5.c
> # ./a.out
> ...
> [ 35.583265] Unable to handle kernel paging request at virtual address ffffffff00000008
> [ 35.596916] status: 0000000200000121 badaddr: ffffffff00000008 cause: 000000000000000d
> [ 35.597781] [<ffffffff80640cc6>] nfs4_alloc_init_cpntf_state+0x94/0xdc
> [ 35.598576] [<ffffffff80274c98>] nfsd4_copy_notify+0xf8/0x28e
> [ 35.599386] [<ffffffff80275c86>] nfsd4_proc_compound+0x2b6/0x4ee
> [ 35.600166] [<ffffffff8025f7f4>] nfsd_dispatch+0x118/0x174
> [ 35.600840] [<ffffffff8061a2e8>] svc_process_common+0x2f4/0x56c
> [ 35.601630] [<ffffffff8061a624>] svc_process+0xc4/0x102
> [ 35.602302] [<ffffffff8025f25a>] nfsd+0xfa/0x162
> [ 35.602979] [<ffffffff80027010>] kthread+0x124/0x136
> [ 35.603668] [<ffffffff8000303e>] ret_from_exception+0x0/0xc
> [ 35.604667] ---[ end trace 69f12ad62072e251 ]---
We could do something like this.--b.
Author: J. Bruce Fields <bfields@redhat.com>
Date: Wed Jan 5 14:15:03 2022 -0500
nfsd: fix crash on COPY_NOTIFY with special stateid
RTM says "If the special ONE stateid is passed to
nfs4_preprocess_stateid_op(), it returns status=0 but does not set
*cstid. nfsd4_copy_notify() depends on stid being set if status=0, and
thus can crash if the client sends the right COPY_NOTIFY RPC."
RFC 7862 says "The cna_src_stateid MUST refer to either open or locking
states provided earlier by the server. If it is invalid, then the
operation MUST fail."
The RFC doesn't specify an error, and the choice doesn't matter much as
this is clearly illegal client behavior, but bad_stateid seems
reasonable.
Simplest is just to guarantee that nfs4_preprocess_stateid_op, called
with non-NULL cstid, errors out if it can't return a stateid.
Reported-by: rtm@csail.mit.edu
Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1956d377d1a6..b94b3bb2b8a6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6040,7 +6040,11 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
*nfp = NULL;
if (ZERO_STATEID(stateid) || ONE_STATEID(stateid)) {
- status = check_special_stateids(net, fhp, stateid, flags);
+ if (cstid)
+ status = nfserr_bad_stateid;
+ else
+ status = check_special_stateids(net, fhp, stateid,
+ flags);
goto done;
}
^ permalink raw reply related
* Re: [PATCH v4] KEYS: encrypted: Instantiate key with user-provided decrypted data
From: Jarkko Sakkinen @ 2022-01-05 20:12 UTC (permalink / raw)
To: Yael Tiomkin, linux-integrity
Cc: jejb, zohar, corbet, dhowells, jmorris, serge, keyrings,
linux-doc, linux-kernel, linux-security-module
In-Reply-To: <20211229215330.4134835-1-yaelt@google.com>
On Wed, 2021-12-29 at 16:53 -0500, Yael Tiomkin wrote:
> The encrypted.c class supports instantiation of encrypted keys with
> either an already-encrypted key material, or by generating new key
> material based on random numbers. This patch defines a new datablob
> format: [<format>] <master-key name> <decrypted data length>
> <decrypted data> that allows to instantiate encrypted keys using
> user-provided decrypted data, and therefore allows to perform key
> encryption from userspace. The decrypted key material will be
> inaccessible from userspace.
The 2nd to last sentence is essentially a tautology but fails to
be even that, as you can already "perform key encryption" from user
space, just not with arbitrary key material.
It does not elighten any applications of this feature.
/Jarkko
^ permalink raw reply
* Re: [PATCH] PCI/portdrv: Skip enabling AER on external facing ports
From: Bjorn Helgaas @ 2022-01-05 20:12 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: bhelgaas, mika.westerberg, koba.ko, Lukas Wunner, Stuart Hayes,
Jan Kiszka, linux-pci, linux-kernel
In-Reply-To: <20220105060643.822111-1-kai.heng.feng@canonical.com>
On Wed, Jan 05, 2022 at 02:06:41PM +0800, Kai-Heng Feng wrote:
> The Thunderbolt root ports may constantly spew out uncorrected errors
> from AER service:
> [ 30.100211] pcieport 0000:00:1d.0: AER: Uncorrected (Non-Fatal) error received: 0000:00:1d.0
> [ 30.100251] pcieport 0000:00:1d.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID)
> [ 30.100256] pcieport 0000:00:1d.0: device [8086:7ab0] error status/mask=00100000/00004000
> [ 30.100262] pcieport 0000:00:1d.0: [20] UnsupReq (First)
> [ 30.100267] pcieport 0000:00:1d.0: AER: TLP Header: 34000000 08000052 00000000 00000000
> [ 30.100372] thunderbolt 0000:0a:00.0: AER: can't recover (no error_detected callback)
> [ 30.100401] xhci_hcd 0000:3e:00.0: AER: can't recover (no error_detected callback)
> [ 30.100427] pcieport 0000:00:1d.0: AER: device recovery failed
No timestamps needed here; they don't add to understanding the
problem.
> The link may not be reliable on external facing ports, so don't enable
> AER on those ports.
I'm not sure what you want to accomplish here. If the errors are
legitimate and the result of some hardware issue like a bad cable, why
should we ignore them? If they're caused by a software problem, we
should figure that out and fix it.
Does this occur on a specific instance of possibly flaky hardware?
You mention a spew of errors; do you think this is a single error that
we fail to clear correctly? Or is it really many separate errors?
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215453
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/pci/pcie/portdrv_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index bda630889f955..d464d00ade8f2 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -219,7 +219,8 @@ static int get_port_device_capability(struct pci_dev *dev)
>
> #ifdef CONFIG_PCIEAER
> if (dev->aer_cap && pci_aer_available() &&
> - (pcie_ports_native || host->native_aer)) {
> + (pcie_ports_native || host->native_aer) &&
> + !dev->external_facing) {
> services |= PCIE_PORT_SERVICE_AER;
>
> /*
> --
> 2.33.1
>
^ permalink raw reply
* RE: [PATCH] doc: simplify baseband features matrix
From: Morten Brørup @ 2022-01-05 20:12 UTC (permalink / raw)
To: Thomas Monjalon, dev; +Cc: nipun.gupta, hemant.agrawal, gakhil, nicolas.chautru
In-Reply-To: <20220105174016.103538-1-thomas@monjalon.net>
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, 5 January 2022 18.40
>
> The "feature" BBDEV API is useless as all baseband drivers
> must implement it by definition.
>
> The non-implemented features should not be marked with "N".
> Keeping them blank is clearer to read in the resulting matrix.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> doc/guides/bbdevs/features/acc100.ini | 1 -
> doc/guides/bbdevs/features/default.ini | 1 -
> doc/guides/bbdevs/features/fpga_5gnr_fec.ini | 1 -
> doc/guides/bbdevs/features/fpga_lte_fec.ini | 1 -
> doc/guides/bbdevs/features/la12xx.ini | 4 ----
> doc/guides/bbdevs/features/null.ini | 1 -
> doc/guides/bbdevs/features/turbo_sw.ini | 1 -
> 7 files changed, 10 deletions(-)
>
> diff --git a/doc/guides/bbdevs/features/acc100.ini
> b/doc/guides/bbdevs/features/acc100.ini
> index 642cd48818..61a31065e1 100644
> --- a/doc/guides/bbdevs/features/acc100.ini
> +++ b/doc/guides/bbdevs/features/acc100.ini
> @@ -11,4 +11,3 @@ LDPC Encoder (5G) = Y
> LLR/HARQ Compression = Y
> External DDR Access = Y
> HW Accelerated = Y
> -BBDEV API = Y
> diff --git a/doc/guides/bbdevs/features/default.ini
> b/doc/guides/bbdevs/features/default.ini
> index 5fe267a625..494be5e400 100644
> --- a/doc/guides/bbdevs/features/default.ini
> +++ b/doc/guides/bbdevs/features/default.ini
> @@ -13,4 +13,3 @@ LDPC Encoder (5G) =
> LLR/HARQ Compression =
> External DDR Access =
> HW Accelerated =
> -BBDEV API =
> diff --git a/doc/guides/bbdevs/features/fpga_5gnr_fec.ini
> b/doc/guides/bbdevs/features/fpga_5gnr_fec.ini
> index 7a0b8d4e75..66c9139409 100644
> --- a/doc/guides/bbdevs/features/fpga_5gnr_fec.ini
> +++ b/doc/guides/bbdevs/features/fpga_5gnr_fec.ini
> @@ -8,4 +8,3 @@ LDPC Decoder (5G) = Y
> LDPC Encoder (5G) = Y
> External DDR Access = Y
> HW Accelerated = Y
> -BBDEV API = Y
> diff --git a/doc/guides/bbdevs/features/fpga_lte_fec.ini
> b/doc/guides/bbdevs/features/fpga_lte_fec.ini
> index f1cfb924a2..c3c398312b 100644
> --- a/doc/guides/bbdevs/features/fpga_lte_fec.ini
> +++ b/doc/guides/bbdevs/features/fpga_lte_fec.ini
> @@ -7,4 +7,3 @@
> Turbo Decoder (4G) = Y
> Turbo Encoder (4G) = Y
> HW Accelerated = Y
> -BBDEV API = Y
> diff --git a/doc/guides/bbdevs/features/la12xx.ini
> b/doc/guides/bbdevs/features/la12xx.ini
> index 0aec5eecb6..70c11990b7 100644
> --- a/doc/guides/bbdevs/features/la12xx.ini
> +++ b/doc/guides/bbdevs/features/la12xx.ini
> @@ -4,10 +4,6 @@
> ; Refer to default.ini for the full list of available PMD features.
> ;
> [Features]
> -Turbo Decoder (4G) = N
> -Turbo Encoder (4G) = N
> LDPC Decoder (5G) = Y
> LDPC Encoder (5G) = Y
> -LLR/HARQ Compression = N
> HW Accelerated = Y
> -BBDEV API = Y
> diff --git a/doc/guides/bbdevs/features/null.ini
> b/doc/guides/bbdevs/features/null.ini
> index d9bbda9cf0..50648cdecb 100644
> --- a/doc/guides/bbdevs/features/null.ini
> +++ b/doc/guides/bbdevs/features/null.ini
> @@ -4,4 +4,3 @@
> ; Refer to default.ini for the full list of available PMD features.
> ;
> [Features]
> -BBDEV API = Y
> diff --git a/doc/guides/bbdevs/features/turbo_sw.ini
> b/doc/guides/bbdevs/features/turbo_sw.ini
> index 2c7075e212..1d908d3ceb 100644
> --- a/doc/guides/bbdevs/features/turbo_sw.ini
> +++ b/doc/guides/bbdevs/features/turbo_sw.ini
> @@ -8,4 +8,3 @@ Turbo Decoder (4G) = Y
> Turbo Encoder (4G) = Y
> LDPC Decoder (5G) = Y
> LDPC Encoder (5G) = Y
> -BBDEV API = Y
> --
> 2.34.1
>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
^ 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.