Re: [PATCH] Staging: speakup: spk_types: fixed an unnamed parameter style issue
On Sat, Aug 17, 2019 at 02:54:26AM -0400, Matthew Hanzelik wrote: > Fixed an unnamed parameter style issue. > > Signed-off-by: Matthew Hanzelik > --- > drivers/staging/speakup/spk_types.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/speakup/spk_types.h > b/drivers/staging/speakup/spk_types.h > index a2fc72c29894..afa64eb9afb4 100644 > --- a/drivers/staging/speakup/spk_types.h > +++ b/drivers/staging/speakup/spk_types.h > @@ -189,7 +189,7 @@ struct spk_synth { > void (*flush)(struct spk_synth *synth); > int (*is_alive)(struct spk_synth *synth); > int (*synth_adjust)(struct st_var_header *var); > - void (*read_buff_add)(u_char); > + void (*read_buff_add)(u_char *add); You just changed the function prototype from taking a single character, to taking a pointer to a character, are you sure this is correct? No other build warnings with this patch enabled? thanks, greg k-h
Re: Lay common foundation to make PVR/SGX work without hacks on OMAP34xx, OMAP36xx, AM335x and potentially OMAP4, OMAP5
* Adam Ford [190816 23:02]: > On Wed, Aug 14, 2019 at 8:16 AM Tony Lindgren wrote: > > Well I just posted some sgx interconnect target module patches. We might > > still have them in v5.4 assuming people manage to review and test them. > > Nikolaus, > > I tested Tony's change and I can confirm that I can read the value > when enabled. Should I apply his patches to your branch before I > test, or is it go too to go as-is? I've got an AM3517, OMAP35 and a > DM3730. I am not sure if the AM3517 is even on the radar, but it has > an sgx530 as well. My estimate is am3517 is wired the same way as omap34xx with a 60% chance, then 30% chance it's wired the same way as omap36xx, and then 10% chance for similar wiring to am335x.. So hopefully that leaves 0% chance for yet something different for it's wiring :) If you have a chance please give it a try. Also check the TRM for the sgx sysconfig register bits to see which of the above matches, and if 3517 has a related rstctrl register like am335x has. Regards, Tony
[PATCH] media: dvb-frontends: fix a memory leak bug
In cx24117_load_firmware(), 'buf' is allocated through kmalloc() to hold the firmware. However, if i2c_transfer() fails, it is not deallocated, leading to a memory leak bug. Signed-off-by: Wenwen Wang --- drivers/media/dvb-frontends/cx24117.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/cx24117.c b/drivers/media/dvb-frontends/cx24117.c index 42697a5..9fccc90 100644 --- a/drivers/media/dvb-frontends/cx24117.c +++ b/drivers/media/dvb-frontends/cx24117.c @@ -619,8 +619,10 @@ static int cx24117_load_firmware(struct dvb_frontend *fe, /* send fw */ ret = i2c_transfer(state->priv->i2c, &msg, 1); - if (ret < 0) + if (ret < 0) { + kfree(buf); return ret; + } kfree(buf); -- 2.7.4
Re: INFO: rcu detected stall in __do_softirq
On 16-08-19, 09:55, Srinivas Kandagatla wrote: > > > On 16/08/2019 01:10, syzbot wrote: > > syzbot has bisected this bug to: > > > > commit 2aeac95d1a4cc85aae57ab842d5c3340df0f817f > > Author: Srinivas Kandagatla > > Date: Tue Jun 11 10:40:41 2019 + > > > > soundwire: add module_sdw_driver helper macro > > Not sure how adding a macro with no users triggers this rcu stall. And config used doesn't have soundwire set :D > > BTW this was in mainline since rc1. This is caused by something else! > > --srini > > > > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=114b45ee60 > > start commit: 882e8691 Add linux-next specific files for 20190801 > > git tree: linux-next > > final crash: https://syzkaller.appspot.com/x/report.txt?x=134b45ee60 > > console output: https://syzkaller.appspot.com/x/log.txt?x=154b45ee60 > > kernel config: https://syzkaller.appspot.com/x/.config?x=466b331af3f34e94 > > dashboard link: > > https://syzkaller.appspot.com/bug?extid=6593c6b8c8b66a07cd98 > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16b216b260 > > > > Reported-by: syzbot+6593c6b8c8b66a07c...@syzkaller.appspotmail.com > > Fixes: 2aeac95d1a4c ("soundwire: add module_sdw_driver helper macro") > > > > For information about bisection process see: > > https://goo.gl/tpsmEJ#bisection -- ~Vinod
iwlwifi: microcode SW error detected
Hi. I just found the following error in the output from dmesg. [ 4023.460058] iwlwifi :02:00.0: Microcode SW error detected. Restarting 0x0. [ 4023.460178] iwlwifi :02:00.0: Start IWL Error Log Dump: [ 4023.460179] iwlwifi :02:00.0: Status: 0x0080, count: 6 [ 4023.460180] iwlwifi :02:00.0: Loaded firmware version: 46.93e59cf4.0 [ 4023.460181] iwlwifi :02:00.0: 0x22CE | ADVANCED_SYSASSERT [ 4023.460182] iwlwifi :02:00.0: 0x0590A2F0 | trm_hw_status0 [ 4023.460182] iwlwifi :02:00.0: 0x | trm_hw_status1 [ 4023.460183] iwlwifi :02:00.0: 0x00488472 | branchlink2 [ 4023.460183] iwlwifi :02:00.0: 0x00479392 | interruptlink1 [ 4023.460184] iwlwifi :02:00.0: 0x | interruptlink2 [ 4023.460184] iwlwifi :02:00.0: 0x012C | data1 [ 4023.460185] iwlwifi :02:00.0: 0x | data2 [ 4023.460186] iwlwifi :02:00.0: 0x0400 | data3 [ 4023.460186] iwlwifi :02:00.0: 0x42001A44 | beacon time [ 4023.460187] iwlwifi :02:00.0: 0x4E9F05CD | tsf low [ 4023.460187] iwlwifi :02:00.0: 0x00D8 | tsf hi [ 4023.460188] iwlwifi :02:00.0: 0x | time gp1 [ 4023.460188] iwlwifi :02:00.0: 0xEF55F6D0 | time gp2 [ 4023.460189] iwlwifi :02:00.0: 0x0001 | uCode revision type [ 4023.460190] iwlwifi :02:00.0: 0x002E | uCode version major [ 4023.460190] iwlwifi :02:00.0: 0x93E59CF4 | uCode version minor [ 4023.460191] iwlwifi :02:00.0: 0x0321 | hw version [ 4023.460191] iwlwifi :02:00.0: 0x00C89004 | board version [ 4023.460192] iwlwifi :02:00.0: 0x0A05001C | hcmd [ 4023.460192] iwlwifi :02:00.0: 0xA2F93802 | isr0 [ 4023.460193] iwlwifi :02:00.0: 0x0004 | isr1 [ 4023.460193] iwlwifi :02:00.0: 0x1802 | isr2 [ 4023.460194] iwlwifi :02:00.0: 0x40417DCD | isr3 [ 4023.460195] iwlwifi :02:00.0: 0x | isr4 [ 4023.460195] iwlwifi :02:00.0: 0x0A04001C | last cmd Id [ 4023.460196] iwlwifi :02:00.0: 0x00018802 | wait_event [ 4023.460196] iwlwifi :02:00.0: 0x4A88 | l2p_control [ 4023.460197] iwlwifi :02:00.0: 0x0020 | l2p_duration [ 4023.460197] iwlwifi :02:00.0: 0x03BF | l2p_mhvalid [ 4023.460198] iwlwifi :02:00.0: 0x00EF | l2p_addr_match [ 4023.460198] iwlwifi :02:00.0: 0x000D | lmpm_pmg_sel [ 4023.460199] iwlwifi :02:00.0: 0x19071250 | timestamp [ 4023.460199] iwlwifi :02:00.0: 0x14C0E8E8 | flow_handler [ 4023.460257] iwlwifi :02:00.0: 0x | ADVANCED_SYSASSERT [ 4023.460257] iwlwifi :02:00.0: 0x | umac branchlink1 [ 4023.460258] iwlwifi :02:00.0: 0x | umac branchlink2 [ 4023.460258] iwlwifi :02:00.0: 0x | umac interruptlink1 [ 4023.460259] iwlwifi :02:00.0: 0x | umac interruptlink2 [ 4023.460260] iwlwifi :02:00.0: 0x | umac data1 [ 4023.460260] iwlwifi :02:00.0: 0x | umac data2 [ 4023.460261] iwlwifi :02:00.0: 0x | umac data3 [ 4023.460261] iwlwifi :02:00.0: 0x | umac major [ 4023.460262] iwlwifi :02:00.0: 0x | umac minor [ 4023.460262] iwlwifi :02:00.0: 0x | frame pointer [ 4023.460263] iwlwifi :02:00.0: 0x | stack pointer [ 4023.460263] iwlwifi :02:00.0: 0x | last host cmd [ 4023.460264] iwlwifi :02:00.0: 0x | isr status reg [ 4023.460278] iwlwifi :02:00.0: Fseq Registers: [ 4023.460282] iwlwifi :02:00.0: 0x0568FC22 | FSEQ_ERROR_CODE [ 4023.460289] iwlwifi :02:00.0: 0x | FSEQ_TOP_INIT_VERSION [ 4023.460297] iwlwifi :02:00.0: 0xDFFC324F | FSEQ_CNVIO_INIT_VERSION [ 4023.460304] iwlwifi :02:00.0: 0xA371 | FSEQ_OTP_VERSION [ 4023.460312] iwlwifi :02:00.0: 0xC338B29A | FSEQ_TOP_CONTENT_VERSION [ 4023.460319] iwlwifi :02:00.0: 0xD9E91E16 | FSEQ_ALIVE_TOKEN [ 4023.460327] iwlwifi :02:00.0: 0xAC99E6BF | FSEQ_CNVI_ID [ 4023.460334] iwlwifi :02:00.0: 0x07665623 | FSEQ_CNVR_ID [ 4023.460342] iwlwifi :02:00.0: 0x01000200 | CNVI_AUX_MISC_CHIP [ 4023.460349] iwlwifi :02:00.0: 0x01300202 | CNVR_AUX_MISC_CHIP [ 4023.460357] iwlwifi :02:00.0: 0x485B | CNVR_SCU_SD_REGS_SD_REG_DIG_DCDC_VTRIM [ 4023.460413] iwlwifi :02:00.0: 0x0BADCAFE | CNVR_SCU_SD_REGS_SD_REG_ACTIVE_VDIG_MIRROR [ 4023.460421] iwlwifi :02:00.0: Collecting data: trigger 2 fired. [ 4023.460424] ieee80211 phy0: Hardware restart was requested [ 4024.639366] iwlwifi :02:00.0: Applying debug destination EXTERNAL_DRAM [ 4024.753171] iwlwifi :02:00.0: Applying debug destination EXTERNAL_DRAM [ 4024.817999] iwlwifi :02:00.0: FW already configured (0) - re-configuring [ 4024.829374] iwlwifi :02:00.0: BIOS contains WGDS but no WRDS The output messages from the driver when the system starts are: [3.667365] iwlwifi :02:00.0: enabling device ( -> 0002) [3.670357] iwlwifi :02:00.0: Found debug destination: EXTERNAL_DRAM [3.670360] iwlwifi :02:00.0: Found debug configuration: 0 [3.670525] iwlwifi 000
Re: [PATCH] clk: Remove extraneous 'for' word in comments
On Sat, 2019-08-17 at 12:05 +0530, Rishi Gupta wrote: > An extra 'for' word is grammatically incorrect in the comment > 'verifying ops for multi-parent clks'. This commit removes > this extra for word. A few other repeated word typos in comments are common in the kernel and most could be changed. $ git grep -P '^\s*/?\*.*\bthe the\b' | wc -l 285 $ git grep -P '^\s*/?\*.*\bto to\b' | wc -l 62 $ git grep -P '^\s*/?\*.*\bfor for\b' | wc -l 31 $ git grep -P '^\s*/?\*.*\bfrom from\b' | wc -l 22 $ git grep -P '^\s*/?\*.*\bare are\b' | wc -l 16
Re: cleanup the walk_page_range interface
On Fri, Aug 16, 2019 at 11:41 PM Stephen Rothwell wrote: > > I certainly prefer that method of API change :-) > (see the current "keys: Replace uid/gid/perm permissions checking with > an ACL" in linux-next Side note: I will *not* be pulling that again. It was broken last time, and without more people reviewing the code, I' m not pulling it for 5.4 either even if David has an additional commit on top that might have fixed the original problem. There's still a pending kernel test report on commit f771fde82051 ("keys: Simplify key description management") from another of David's pulls for 5.3 - one that didn't get reverted. David, look in your inbox for a kernel test report about kernel BUG at security/keys/keyring.c:1245! (It's the BUG_ON(index_key->desc_len == 0); line - the line numbers have since changed, and it's line 1304 in the current tree). I'm not sure why _that_ BUG_ON() now triggers, but I wonder if it's because 'desc_len' went from a 'size_t' to an 'u8', and now a desc_len of 256 is 0. Or something. The point being that there have been too many bugs in the pulls and nobody but David apparently ever had anything to do with any of the development. This code needs more eyes, not more random changes. So I won't be compounding on that workflow problem next merge window. Linus
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Fri, Aug 16, 2019, 18:36 Mathieu Desnoyers wrote: > > If WRITE_ONCE has any use at all (protecting against store tearing and > invented stores), it should be used even with a lock held in this > scenario, because the lock does not prevent READ_ONCE() from observing > transient states caused by lack of WRITE_ONCE() for the update. The thing is, we really haven't requred WRITE_ONCE() to protect against store tearing. We have lots of traditional code that does stuff along the lines of .. set of data structure .. smp_wmb(); *ptr = newdata; and we simply *depend* on the compiler doing the "*ptr" as a single write. We've simply always done that. Even on UP we've had the "interrupts will see old value or new value but not some random half-way value", going all the way back to the original kernel sources. The data tearing issue is almost a non-issue. We're not going to add WRITE_ONCE() to these kinds of places for no good reason. > So why does WRITE_ONCE exist in the first place ? Is it for documentation > purposes only or are there actual situations where omitting it can cause > bugs with real-life compilers ? WRITE_ONCE should be seen mainly as (a) documentation and (b) for new code. Although I suspect often you'd be better off using smb_load_acquire() and smp_store_release() when you have code sequences where you have unlocked READ_ONCE/WRITE_ONCE and memory ordering. WRITE_ONCE() doesn't even protect against data tearing. If you do a "WRITE_ONCE()" on a type larger than 8 bytes, it will fall back to __builtin_memcpy(). So honestly, WRITE_ONCE() is often more documentation than protecting against something, but overdoing it doesn't help document anything, it just obscures the point. Yeah, yeah, it will use a "volatile" access for the proper normal types, but even then that won't protect against data tearing on 64-bit writes on a 32-bit machine, for example. It doesn't even give ordering guarantees for the sub-words. So you still end up with the almost the same basic rule: a natural byte/word write will be atomic. But really, it's going to be so even without the WRITE_ONCE(), so... It does ostensibly protect against the compiler re-ordering the write against other writes (note: *compiler*, not CPU), and it does make sure the write only happens once. But it's really hard to see a valid compiler optimization that wouldn't do that anyway. Because of the compiler ordering of WRITE_ONCE against other WRITE_ONCE cases, it could be used for irq-safe ordering on the local cpu, for example. If that matters. Although I suspect any such case is practically going to use per-cpu variables anyway. End result: it's *mostly* about documentation. Just do a grep for WRITE_ONCE() vs READ_ONCE(). You'll find a lot more users of the latter. And quite frankly, I looked at some of the WRITE_ONCE() users and a lot of them were kind of pointless. Somebody tell me just exactly how they expect the WRITE_ONCE() cases in net/tls/tls_sw.c to matter, for example (and that was literally a random case I happened to look at). It's not clear what they do, since they certainly don't imply any memory ordering. They do imply a certain amount of compiler re-ordering due to the volatile, but that's pretty limited too. Only wrt _other_ things with side effects, not the normal code around them anyway. In contrast, READ_ONCE() has very practical examples of mattering, because unlike writes, compilers really can reasonably split reads. For example, if you're testing multiple bits in the same word, and you want to make sure they are coherent, you should very much do val = READ_ONCE(mem); .. test different bits in val .. because without the READ_ONCE(), the compiler could end up just doing the read multiple times. Similarly, even if you only use a value once, this is actually something a compiler can do: if (a) { lock(); B() unlock(); } else B(); and a compiler might end up generating that as if (a) lock(); B(); if (a) unlock(); instead. So doing "if (READ_ONCE(a))" actually makes a difference - it guarantees that 'a' is only read once, even if that value was _literally_ only used on a source level, the compiler really could have decided "let's read it twice". See how duplicating a read is fundamentally different from duplicating a write? Duplicating or splitting a read is not visible to other threads. Notice how nothiing like the above is reasonable for a write. That said, the most common case really is none of the above half-way subtle cases. It's literally the whole "write pointer once". Making existing code that does that use WRITE_ONCE() is completely pointless. It's not going to really change or fix anything at all. Linus
[tip:x86/urgent] x86/cpu: Explain Intel model naming convention
Commit-ID: 12ece2d53d3e8f827e972caf497c165f7729c717 Gitweb: https://git.kernel.org/tip/12ece2d53d3e8f827e972caf497c165f7729c717 Author: Tony Luck AuthorDate: Thu, 15 Aug 2019 11:16:24 -0700 Committer: Borislav Petkov CommitDate: Sat, 17 Aug 2019 10:06:32 +0200 x86/cpu: Explain Intel model naming convention Dave Hansen spelled out the rules in an e-mail: https://lkml.kernel.org/r/91eefbe4-e32b-d762-be4d-672ff915d...@intel.com Copy those right into the file to make it easy for people to find them. Suggested-by: Borislav Petkov Signed-off-by: Tony Luck Signed-off-by: Borislav Petkov Acked-by: Thomas Gleixner Cc: "H. Peter Anvin" Cc: Dave Hansen Cc: Ingo Molnar Cc: x86-ml Link: https://lkml.kernel.org/r/20190815224704.ga10...@agluck-desk2.amr.corp.intel.com --- arch/x86/include/asm/intel-family.h | 15 +++ 1 file changed, 15 insertions(+) diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h index 0278aa66ef62..fe7c205233f1 100644 --- a/arch/x86/include/asm/intel-family.h +++ b/arch/x86/include/asm/intel-family.h @@ -11,6 +11,21 @@ * While adding a new CPUID for a new microarchitecture, add a new * group to keep logically sorted out in chronological order. Within * that group keep the CPUID for the variants sorted by model number. + * + * The defined symbol names have the following form: + * INTEL_FAM6{OPTFAMILY}_{MICROARCH}{OPTDIFF} + * where: + * OPTFAMILY Describes the family of CPUs that this belongs to. Default + * is assumed to be "_CORE" (and should be omitted). Other values + * currently in use are _ATOM and _XEON_PHI + * MICROARCH Is the code name for the micro-architecture for this core. + * N.B. Not the platform name. + * OPTDIFF If needed, a short string to differentiate by market segment. + * Exact strings here will vary over time. _DESKTOP, _MOBILE, and + * _X (short for Xeon server) should be used when they are + * appropriate. + * + * The #define line may optionally include a comment including platform names. */ #define INTEL_FAM6_CORE_YONAH 0x0E
Re: 5.3-rc3-ish VM crash: RIP: 0010:tcp_trim_head+0x20/0xe0
On 12/08/2019 19:56, Eric Dumazet wrote: > > > On 8/12/19 2:50 PM, Sander Eikelenboom wrote: >> L.S., >> >> While testing a somewhere-after-5.3-rc3 kernel (which included the latest >> net merge (33920f1ec5bf47c5c0a1d2113989bdd9dfb3fae9), >> one of my Xen VM's (which gets quite some network load) crashed. >> See below for the stacktrace. >> >> Unfortunately I haven't got a clear trigger, so bisection doesn't seem to be >> an option at the moment. >> I haven't encountered this on 5.2, so it seems to be an regression against >> 5.2. >> >> Any ideas ? >> >> -- >> Sander >> >> >> [16930.653595] general protection fault: [#1] SMP NOPTI >> [16930.653624] CPU: 0 PID: 3275 Comm: rsync Not tainted >> 5.3.0-rc3-20190809-doflr+ #1 >> [16930.653657] RIP: 0010:tcp_trim_head+0x20/0xe0 >> [16930.653677] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 >> fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> >> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8 >> [16930.653741] RSP: :c9003ad8 EFLAGS: 00010286 >> [16930.653762] RAX: fffe888005bf62c0 RBX: 8880115fb800 RCX: >> 801b > > crash in " mov0x20(%rax),%eax" and RAX=fffe888005bf62c0 (not a valid > kernel address) > > Look like one bit corruption maybe. > > Nothing comes to mind really between 5.2 and 53 that could explain this. > >> [16930.653791] RDX: 05a0 RSI: 8880115fb800 RDI: >> 888016b00880 >> [16930.653819] RBP: 888016b00880 R08: 0001 R09: >> >> [16930.653848] R10: 88800ae00800 R11: bfe632e6 R12: >> 05a0 >> [16930.653875] R13: 0001 R14: bfe62d46 R15: >> 0004 >> [16930.653913] FS: 7fe71fe2cb80() GS:88801f20() >> knlGS: >> [16930.653943] CS: 0010 DS: ES: CR0: 80050033 >> [16930.653965] CR2: 55de0f3e7000 CR3: 11f32000 CR4: >> 06f0 >> [16930.653993] Call Trace: >> [16930.654005] >> [16930.654018] tcp_ack+0xbb0/0x1230 >> [16930.654033] tcp_rcv_established+0x2e8/0x630 >> [16930.654053] tcp_v4_do_rcv+0x129/0x1d0 >> [16930.654070] tcp_v4_rcv+0xac9/0xcb0 >> [16930.654088] ip_protocol_deliver_rcu+0x27/0x1b0 >> [16930.654109] ip_local_deliver_finish+0x3f/0x50 >> [16930.654128] ip_local_deliver+0x4d/0xe0 >> [16930.654145] ? ip_protocol_deliver_rcu+0x1b0/0x1b0 >> [16930.654163] ip_rcv+0x4c/0xd0 >> [16930.654179] __netif_receive_skb_one_core+0x79/0x90 >> [16930.654200] netif_receive_skb_internal+0x2a/0xa0 >> [16930.654219] napi_gro_receive+0xe7/0x140 >> [16930.654237] xennet_poll+0x9be/0xae0 >> [16930.654254] net_rx_action+0x136/0x340 >> [16930.654271] __do_softirq+0xdd/0x2cf >> [16930.654287] irq_exit+0x7a/0xa0 >> [16930.654304] xen_evtchn_do_upcall+0x27/0x40 >> [16930.654320] xen_hvm_callback_vector+0xf/0x20 >> [16930.654339] >> [16930.654349] RIP: 0033:0x55de0d87db99 >> [16930.654364] Code: 00 00 48 89 7c 24 f8 45 39 fe 45 0f 42 fe 44 89 7c 24 >> f4 eb 09 0f 1f 40 00 83 e9 01 74 3e 89 f2 48 63 f8 4c 01 d2 44 38 1c 3a <75> >> 25 44 38 6c 3a ff 75 1e 41 0f b6 3c 24 40 38 3a 75 14 41 0f b6 >> [16930.654432] RSP: 002b:7ffd5531eec8 EFLAGS: 0a87 ORIG_RAX: >> ff0c >> [16930.655004] RAX: 0002 RBX: 55de0f3e8e50 RCX: >> 007f >> [16930.655034] RDX: 55de0f3dc2d2 RSI: 3492 RDI: >> 0002 >> [16930.655062] RBP: 7fff R08: 80ea R09: >> 01f0 >> [16930.655089] R10: 55de0f3d8e40 R11: 0094 R12: >> 55de0f3e0f2a >> [16930.655116] R13: 0010 R14: 7f16 R15: >> 0080 >> [16930.655144] Modules linked in: >> [16930.655200] ---[ end trace 533367c95501b645 ]--- >> [16930.655223] RIP: 0010:tcp_trim_head+0x20/0xe0 >> [16930.655243] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 >> fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> >> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8 >> [16930.655312] RSP: :c9003ad8 EFLAGS: 00010286 >> [16930.655331] RAX: fffe888005bf62c0 RBX: 8880115fb800 RCX: >> 801b >> [16930.655360] RDX: 05a0 RSI: 8880115fb800 RDI: >> 888016b00880 >> [16930.655387] RBP: 888016b00880 R08: 0001 R09: >> >> [16930.655414] R10: 88800ae00800 R11: bfe632e6 R12: >> 05a0 >> [16930.655441] R13: 0001 R14: bfe62d46 R15: >> 0004 >> [16930.655475] FS: 7fe71fe2cb80() GS:88801f20() >> knlGS: >> [16930.655502] CS: 0010 DS: ES: CR0: 80050033 >> [16930.655525] CR2: 55de0f3e7000 CR3: 11f32000 CR4: >> 06f0 >> [16930.63] Kernel panic - not syncing: Fatal exception in interrupt >> [16930.655789] Kernel Offset: disabled >> Hi Eric, Got another VM
[PATCH] pinctrl/qcom: Fix -Wimplicit-fallthrough
In pinctrl-spmi-gpio.c there is a switch case which is obviously intended to fall through to the next label. Add a comment to suppress -Wimplicit-fallthrough warning. Signed-off-by: Alex Dewar --- drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c index f39da87ea185..b035dd5e25b8 100644 --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c @@ -813,6 +813,7 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state, switch (subtype) { case PMIC_GPIO_SUBTYPE_GPIO_4CH: pad->have_buffer = true; + /* FALLS THROUGH */ case PMIC_GPIO_SUBTYPE_GPIOC_4CH: pad->num_sources = 4; break; -- 2.22.1
Re: [PATCH][next] bus: moxtet: fix unsigned comparison to less than zero
On Sat, Aug 17, 2019 at 02:04:34AM +0200, Marek Behun wrote: > On Fri, 16 Aug 2019 23:41:06 +0100 > Colin King wrote: > > > From: Colin Ian King > > > > Currently the size_t variable res is being checked for > > an error failure however the unsigned variable is never > > less than zero so this test is always false. Fix this by > > making variable res ssize_t > > > > Addresses-Coverity: ("Unsigned compared against 0") > > Fixes: 5bc7f990cd98 ("bus: Add support for Moxtet bus") > > Signed-off-by: Colin Ian King > > --- > > drivers/bus/moxtet.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/bus/moxtet.c b/drivers/bus/moxtet.c > > index 1ee4570e7e17..288a9e4c6c7b 100644 > > --- a/drivers/bus/moxtet.c > > +++ b/drivers/bus/moxtet.c > > @@ -514,7 +514,7 @@ static ssize_t output_write(struct file *file, const > > char __user *buf, > > struct moxtet *moxtet = file->private_data; > > u8 bin[TURRIS_MOX_MAX_MODULES]; > > u8 hex[sizeof(bin) * 2 + 1]; > > - size_t res; > > + ssize_t res; > > loff_t dummy = 0; > > int err, i; > > > > Hi Colin, > thanks. Should I just Ack this, or do I need to send patch to the > developer who commited my patches? According to MAINTAINERS, you're the maintainer and not Arnd. You should forward this to him. But in the future it might be easier if Arnd added himself to the MAINTAINERS file for this. You're probably better off if you have a subsystem mailing list for this driver instead of using LKML. That way more people can get involved with the development if they want to. Anyway, as the maintainer, you need to collect patches and forward them on to Arnd or someone else. Since you are handling the patches, that means you need to Sign them to certify that you haven't added any of SCOs private super secret UNIXWARE intellectual property. You can't just use the Acked-by, you have to use the Signed-off-by tag. If Arnd or someone else is collecting the patches then you could use Reviewed-by or Acked-by. Acked-by basically means you approve the patch and often it's going through a different maintainer's tree. Or it can be you like the approach the patch is taking. It's sort of vague. I seldom Ack patches because I'm not a maintainer in an official sense, but I do give people a Reviewed-by tag if I review their patch and I want to make their day a little happier. Also if it's a huge patch series and I want to help out Greg to know that he can skip reviewing the patch if he wants to. regards, dan carpenter
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Fri, Aug 16, 2019 at 9:52 PM Paul E. McKenney wrote: > > > I'd love to have a flag that says "all undefined behavior is treated > > as implementation-defined". There's a somewhat subtle - but very > > important - difference there. > > It would also be nice to downgrade some of the undefined behavior in > the standard itself. Compiler writers usually hate this because it > would require them to document what their compiler does in each case. > So they would prefer "unspecified" if the could not have "undefined". That certainly would sound very good to me. It's not the "documented behavior" that is important to me (since it is still going to be potentially different across different platforms), it's the "at least it's *some* consistent behavior for that platform". That's just _so_ much better than "undefined behavior" which basically says the compiler writer can do absolutely anything, whether it makes sense or not, and whether it might open a small bug up to absolutely catastrophic end results. > >if (a) > > global_var = 1 > >else > > global_var = 0 > > Me, I would still want to use WRITE_ONCE() in this case. I actually suspect that if we had this kind of code, and a real reason why readers would see it locklessly, then yes, WRITE_ONCE() makes sense. But most of the cases where people add WRITE_ONCE() aren't even the above kind of half-questionable cases. They are just literally WRITE_ONCE(flag, value); and since there is no real memory ordering implied by this, what's the actual value of that statement? What problem does it really solve wrt just writing it as flag = value; which is generally a lot easier to read. If the flag has semantic behavior wrt other things, and the write can race with a read (whether it's the read or the write that is unlocked is irrelevant), is still doesn't tend to make a lot of real difference. In the end, the most common reason for a WRITE_ONCE() is mostly just "to visually pair up with the non-synchronized read that uses READ_ONCE()" Put another way: a WRITE_ONCE() without a paired READ_ONCE() is almost certainly pointless. But the reverse is not really true. All a READ_ONCE() says is "I want either the old or the new value", and it can get that _without_ being paired with a WRITE_ONCE(). See? They just aren't equally important. > > And yes, reads are different from writes. Reads don't have the same > > kind of "other threads of execution can see them" effects, so a > > compiler turning a single read into multiple reads is much more > > realistic and not the same kind of "we need to expect a certain kind > > of sanity from the compiler" issue. > > Though each of those compiler-generated multiple reads might return a > different value, right? Right. See the examples I have in the email to Mathieu: unsigned int bits = some_global_value; ...test different bits in in 'bits' ... can easily cause multiple reads (particularly on a CPU that has a "test bits in memory" instruction and a lack of registers. So then doing it as unsigned int bits = READ_ONCE(some_global_value); .. test different bits in 'bits'... makes a real and obvious semantic difference. In ways that changing a one-time ptr->flag = true; to WRITE_ONCE(ptr->flag, true); does _not_ make any obvious semantic difference what-so-ever. Caching reads is also something that makes sense and is common, in ways that caching writes does not. So doing while (in_progress_global) /* twiddle your thumbs */; obviously trivially translates to an infinite loop with a single conditional in front of it, in ways that while (READ_ONCE(in_progress_global)) /* twiddle */; does not. So there are often _obvious_ reasons to use READ_ONCE(). I really do not find the same to be true of WRITE_ONCE(). There are valid uses, but they are definitely much less common, and much less obvious. Linus
[tip:x86/cleanups] x86/cpu: Use constant definitions for CPU models
Commit-ID: bba10c5cab4ddd8725a7998e064fc72c9770c667 Gitweb: https://git.kernel.org/tip/bba10c5cab4ddd8725a7998e064fc72c9770c667 Author: Rahul Tanwar AuthorDate: Fri, 16 Aug 2019 16:18:57 +0800 Committer: Borislav Petkov CommitDate: Sat, 17 Aug 2019 10:34:09 +0200 x86/cpu: Use constant definitions for CPU models Replace model numbers with their respective macro definitions when comparing CPU models. Suggested-by: Andy Shevchenko Signed-off-by: Rahul Tanwar Signed-off-by: Borislav Petkov Cc: a...@linux.intel.com Cc: cheol.yong@intel.com Cc: Hans de Goede Cc: "H. Peter Anvin" Cc: Ingo Molnar Cc: qi-ming...@intel.com Cc: "Rafael J. Wysocki" Cc: Ricardo Neri Cc: Thomas Gleixner Cc: Tony Luck Cc: x86-ml Link: https://lkml.kernel.org/r/f7a0e142faa953a53d5f81f78055e1b3c793b134.1565940653.git.rahul.tan...@linux.intel.com --- arch/x86/kernel/cpu/intel.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 8d6d92ebeb54..66de4b84c369 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -265,9 +265,9 @@ static void early_init_intel(struct cpuinfo_x86 *c) /* Penwell and Cloverview have the TSC which doesn't sleep on S3 */ if (c->x86 == 6) { switch (c->x86_model) { - case 0x27: /* Penwell */ - case 0x35: /* Cloverview */ - case 0x4a: /* Merrifield */ + case INTEL_FAM6_ATOM_SALTWELL_MID: + case INTEL_FAM6_ATOM_SALTWELL_TABLET: + case INTEL_FAM6_ATOM_SILVERMONT_MID: set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC_S3); break; default:
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Sat, Aug 17, 2019 at 1:28 AM Linus Torvalds wrote: > >unsigned int bits = some_global_value; >...test different bits in in 'bits' ... > > can easily cause multiple reads (particularly on a CPU that has a > "test bits in memory" instruction and a lack of registers. > > So then doing it as > >unsigned int bits = READ_ONCE(some_global_value); >.. test different bits in 'bits'... Side note: this is likely the best example of actual WRITE_ONCE() use too: if you have that global value with multiple bits that actually have some interdependencies, then doing some_global_value = some_complex_expression(); might be reasonably compiled to do several rmw instructions to update 'some_global_value' So then WRITE_ONCE(some_global_value, some_complex_expression()); really can be a good thing - it clearly just writes things once, and it also documents the whole "write one or the other" value, not some mid-way one, when you then look at the READ_ONCE() thing. But I'm seeing a lot of WRITE_ONCE(x, constantvalue) kind of things and don't seem to find a lot of reason to think that they are any inherently better than "x = constantvalue". (In contrast, using "smp_store_release(flag, true)" has inherent value, because it actually implies a memory barrier wrt previous writes, in ways that WRITE_ONCE() or a direct assignment does not.) Ok, enough blathering. I think I've made my point. Linus
Re: [PATCH] arch : arm : add a criteria for pfn_valid
On Sat, Aug 17, 2019 at 11:00:13AM +0800, Zhaoyang Huang wrote: > From: Zhaoyang Huang > > pfn_valid can be wrong while the MSB of physical address be trimed as pfn > larger than the max_pfn. How the overflow of __pfn_to_phys() is related to max_pfn? Where is the guarantee that __pfn_to_phys(max_pfn) won't overflow? > Signed-off-by: Zhaoyang Huang > --- > arch/arm/mm/init.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > index c2daabb..9c4d938 100644 > --- a/arch/arm/mm/init.c > +++ b/arch/arm/mm/init.c > @@ -177,7 +177,8 @@ static void __init zone_sizes_init(unsigned long min, > unsigned long max_low, > #ifdef CONFIG_HAVE_ARCH_PFN_VALID > int pfn_valid(unsigned long pfn) > { > - return memblock_is_map_memory(__pfn_to_phys(pfn)); > + return (pfn > max_pfn) ? > + false : memblock_is_map_memory(__pfn_to_phys(pfn)); > } > EXPORT_SYMBOL(pfn_valid); > #endif > -- > 1.9.1 > -- Sincerely yours, Mike.
Re: Lay common foundation to make PVR/SGX work without hacks on OMAP34xx, OMAP36xx, AM335x and potentially OMAP4, OMAP5
Hi Adam, > Am 17.08.2019 um 01:01 schrieb Adam Ford : > > > Nikolaus, > > I tested Tony's change and I can confirm that I can read the value > when enabled. Should I apply his patches to your branch before I > test, or is it go too to go as-is? My branch is currently as-is and not aware of Tony's patches and based on v5.3-rc3. I think I will have to remove some glue code which tries to do the platform reset and enables clocks and replace by pm_runtime_get_sync() before it fits together. Then we can likely remove the omap-pvr-soc-glue branch at least partially and/or replace by Tony's patches before they arrive in mainline. The current status of my branch is that it works on OMAP5/Pyra but a quick test on BeagleBone or GTA04 did show some reset/clock errors and it did not more than creating /proc/pvr. pvrsrvctl --start did fail. Which means that the omap-pvr-soc-glue patches are currently broken with 5.3-rc3 anyways... I'll look at it as soon as possible. > I've got an AM3517, OMAP35 and a > DM3730. I am not sure if the AM3517 is even on the radar, but it has > an sgx530 as well. Good to know and keep in mind. BR, Nikolaus
[PATCH] powerpc: optimise WARN_ON()
Unlike BUG_ON(x), WARN_ON(x) uses !!(x) as the trigger of the t(d/w)nei instruction instead of using directly the value of x. This leads to GCC adding unnecessary pair of addic/subfe. This was revealed after adding a WARN_ON() on top of clear_page() in order to detect misaligned destination: @@ -49,6 +51,8 @@ static inline void clear_page(void *addr) { unsigned int i; + WARN_ON((unsigned long)addr & (L1_CACHE_BYTES - 1)); + for (i = 0; i < PAGE_SIZE / L1_CACHE_BYTES; i++, addr += L1_CACHE_BYTES) dcbz(addr); } This resulted on: 019c : 19c: 54 68 06 fe clrlwi r8,r3,27 1a0: 31 48 ff ff addic r10,r8,-1 1a4: 7d 4a 41 10 subfe r10,r10,r8 1a8: 0f 0a 00 00 twnei r10,0 1ac: 39 20 00 80 li r9,128 1b0: 7d 29 03 a6 mtctr r9 1b4: 7c 00 1f ec dcbz0,r3 1b8: 38 63 00 20 addir3,r3,32 1bc: 42 00 ff f8 bdnz1b4 1c0: 7c a3 2b 78 mr r3,r5 1c4: 48 00 00 00 b 1c4 1c4: R_PPC_REL24flush_dcache_page By using (x) instead of !!(x) like BUG_ON() does, the additional instructions go away: 019c : 19c: 54 6a 06 fe clrlwi r10,r3,27 1a0: 0f 0a 00 00 twnei r10,0 1a4: 39 20 00 80 li r9,128 1a8: 7d 29 03 a6 mtctr r9 1ac: 7c 00 1f ec dcbz0,r3 1b0: 38 63 00 20 addir3,r3,32 1b4: 42 00 ff f8 bdnz1ac 1b8: 7c a3 2b 78 mr r3,r5 1bc: 48 00 00 00 b 1bc 1bc: R_PPC_REL24flush_dcache_page Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/bug.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index fed7e6241349..77074582fe65 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -107,7 +107,7 @@ : : "i" (__FILE__), "i" (__LINE__), \ "i" (BUGFLAG_WARNING|BUGFLAG_TAINT(TAINT_WARN)),\ "i" (sizeof(struct bug_entry)), \ - "r" (__ret_warn_on)); \ + "r" ((__force long)(x))); \ } \ unlikely(__ret_warn_on);\ }) -- 2.17.1
Re: PROBLEM: 5.3.0-rc* causes iwlwifi failure
> I am on an Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz running Linux > x86_64 (Slackware), with a custom-compiled 5.3.0-rc4 (.config > attached). > > I am using the Intel wifi adapter on this machine: > > 02:00.0 Network controller: Intel Corporation Device 24fb (rev 10) > > with the iwlwifi driver. I am attaching the output to 'lspci -vv -s > 02:00.0' as the file device-info. > > All 5.3.0-rc* versions I have tried (including rc4) cause multiple > dmesg iwlwifi-related errors (dmesg attached). Examples: > > iwlwifi :02:00.0: Failed to get geographic profile info -5 > iwlwifi :02:00.0: Microcode SW error detected. Restarting 0x8200 > iwlwifi :02:00.0: 0x0038 | BAD_COMMAND > I have my logs filled with similar garbage throughout 5.3-rc*. Also since 5.3-rcsomething not only it WARNS in dmesg about firmware failure, but completely stops working after suspend/resume cycle. It looks like that: commit 4fd445a2c855bbcab81fbe06d110e78dbd974a5b Author: Haim Dreyfuss Date: Thu May 2 11:45:02 2019 +0300 iwlwifi: mvm: Add log information about SAR status Inform users when SAR status is changing. Signed-off-by: Haim Dreyfuss Signed-off-by: Luca Coelho is the culprit. (manually) reverting it on top of 5.3-rc4 makes everything work again.
Re: [PATCH] arch : arm : add a criteria for pfn_valid
On Sat, Aug 17, 2019 at 5:00 PM Mike Rapoport wrote: > > On Sat, Aug 17, 2019 at 11:00:13AM +0800, Zhaoyang Huang wrote: > > From: Zhaoyang Huang > > > > pfn_valid can be wrong while the MSB of physical address be trimed as pfn > > larger than the max_pfn. > > How the overflow of __pfn_to_phys() is related to max_pfn? > Where is the guarantee that __pfn_to_phys(max_pfn) won't overflow? eg, the invalid pfn value as 0x1bffc0 will pass pfn_valid if there is a memory block while the max_pfn is 0xbffc0. In ARM64, bellowing condition check will help to > > > Signed-off-by: Zhaoyang Huang > > --- > > arch/arm/mm/init.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > > index c2daabb..9c4d938 100644 > > --- a/arch/arm/mm/init.c > > +++ b/arch/arm/mm/init.c > > @@ -177,7 +177,8 @@ static void __init zone_sizes_init(unsigned long min, > > unsigned long max_low, > > #ifdef CONFIG_HAVE_ARCH_PFN_VALID > > int pfn_valid(unsigned long pfn) > > { > > - return memblock_is_map_memory(__pfn_to_phys(pfn)); > > + return (pfn > max_pfn) ? > > + false : memblock_is_map_memory(__pfn_to_phys(pfn)); > > } > > EXPORT_SYMBOL(pfn_valid); > > #endif > > -- > > 1.9.1 > > > > -- > Sincerely yours, > Mike. >
[PATCH 3.16 3/4] ext4: cleanup bh release code in ext4_ind_remove_space()
3.16.73-rc1 review patch. If anyone has any objections, please let me know. -- From: "zhangyi (F)" commit 5e86bdda41534e17621d5a071b294943cae4376e upstream. Currently, we are releasing the indirect buffer where we are done with it in ext4_ind_remove_space(), so we can see the brelse() and BUFFER_TRACE() everywhere. It seems fragile and hard to read, and we may probably forget to release the buffer some day. This patch cleans up the code by putting of the code which releases the buffers to the end of the function. Signed-off-by: zhangyi (F) Signed-off-by: Theodore Ts'o Reviewed-by: Jan Kara Signed-off-by: Ben Hutchings --- fs/ext4/indirect.c | 47 ++ 1 file changed, 22 insertions(+), 25 deletions(-) --- a/fs/ext4/indirect.c +++ b/fs/ext4/indirect.c @@ -1313,6 +1313,7 @@ int ext4_ind_remove_space(handle_t *hand ext4_lblk_t offsets[4], offsets2[4]; Indirect chain[4], chain2[4]; Indirect *partial, *partial2; + Indirect *p = NULL, *p2 = NULL; ext4_lblk_t max_block; __le32 nr = 0, nr2 = 0; int n = 0, n2 = 0; @@ -1354,7 +1355,7 @@ int ext4_ind_remove_space(handle_t *hand } - partial = ext4_find_shared(inode, n, offsets, chain, &nr); + partial = p = ext4_find_shared(inode, n, offsets, chain, &nr); if (nr) { if (partial == chain) { /* Shared branch grows from the inode */ @@ -1379,13 +1380,11 @@ int ext4_ind_remove_space(handle_t *hand partial->p + 1, (__le32 *)partial->bh->b_data+addr_per_block, (chain+n-1) - partial); - BUFFER_TRACE(partial->bh, "call brelse"); - brelse(partial->bh); partial--; } end_range: - partial2 = ext4_find_shared(inode, n2, offsets2, chain2, &nr2); + partial2 = p2 = ext4_find_shared(inode, n2, offsets2, chain2, &nr2); if (nr2) { if (partial2 == chain2) { /* @@ -1415,16 +1414,14 @@ end_range: (__le32 *)partial2->bh->b_data, partial2->p, (chain2+n2-1) - partial2); - BUFFER_TRACE(partial2->bh, "call brelse"); - brelse(partial2->bh); partial2--; } goto do_indirects; } /* Punch happened within the same level (n == n2) */ - partial = ext4_find_shared(inode, n, offsets, chain, &nr); - partial2 = ext4_find_shared(inode, n2, offsets2, chain2, &nr2); + partial = p = ext4_find_shared(inode, n, offsets, chain, &nr); + partial2 = p2 = ext4_find_shared(inode, n2, offsets2, chain2, &nr2); /* Free top, but only if partial2 isn't its subtree. */ if (nr) { @@ -1481,15 +1478,7 @@ end_range: partial->p + 1, partial2->p, (chain+n-1) - partial); - while (partial > chain) { - BUFFER_TRACE(partial->bh, "call brelse"); - brelse(partial->bh); - } - while (partial2 > chain2) { - BUFFER_TRACE(partial2->bh, "call brelse"); - brelse(partial2->bh); - } - return 0; + goto cleanup; } /* @@ -1504,8 +1493,6 @@ end_range: partial->p + 1, (__le32 *)partial->bh->b_data+addr_per_block, (chain+n-1) - partial); - BUFFER_TRACE(partial->bh, "call brelse"); - brelse(partial->bh); partial--; } if (partial2 > chain2 && depth2 <= depth) { @@ -1513,11 +1500,21 @@ end_range: (__le32 *)partial2->bh->b_data, partial2->p, (chain2+n2-1) - partial2); - BUFFER_TRACE(partial2->bh, "call brelse"); - brelse(partial2->bh); partial2--; } } + +cleanup: + while (p && p > chain) { + BUFFER_TRACE(p->bh, "call brelse"); + brelse(p->bh); + p--; + } + while (p2 && p2 > chain2) { + BUFFER_TRACE(p2->bh, "call brelse"); +
[PATCH 3.16 1/4] tcp: Clear sk_send_head after purging the write queue
3.16.73-rc1 review patch. If anyone has any objections, please let me know. -- From: Ben Hutchings Denis Andzakovic discovered a potential use-after-free in older kernel versions, using syzkaller. tcp_write_queue_purge() frees all skbs in the TCP write queue and can leave sk->sk_send_head pointing to freed memory. tcp_disconnect() clears that pointer after calling tcp_write_queue_purge(), but tcp_connect() does not. It is (surprisingly) possible to add to the write queue between disconnection and reconnection, so this needs to be done in both places. This bug was introduced by backports of commit 7f582b248d0a ("tcp: purge write queue in tcp_connect_init()") and does not exist upstream because of earlier changes in commit 75c119afe14f ("tcp: implement rb-tree based retransmit queue"). The latter is a major change that's not suitable for stable. Reported-by: Denis Andzakovic Bisected-by: Salvatore Bonaccorso Fixes: 7f582b248d0a ("tcp: purge write queue in tcp_connect_init()") Cc: # before 4.15 Cc: Eric Dumazet Signed-off-by: Ben Hutchings --- include/net/tcp.h | 3 +++ 1 file changed, 3 insertions(+) --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -1352,6 +1352,8 @@ struct tcp_fastopen_context { struct rcu_head rcu; }; +static inline void tcp_init_send_head(struct sock *sk); + /* write queue abstraction */ static inline void tcp_write_queue_purge(struct sock *sk) { @@ -1359,6 +1361,7 @@ static inline void tcp_write_queue_purge while ((skb = __skb_dequeue(&sk->sk_write_queue)) != NULL) sk_wmem_free_skb(sk, skb); + tcp_init_send_head(sk); sk_mem_reclaim(sk); tcp_clear_all_retrans_hints(tcp_sk(sk)); }
[PATCH 3.16 4/4] siphash: implement HalfSipHash1-3 for hash tables
3.16.73-rc1 review patch. If anyone has any objections, please let me know. -- From: "Jason A. Donenfeld" commit 1ae2324f732c9c4e2fa4ebd885fa1001b70d52e1 upstream. HalfSipHash, or hsiphash, is a shortened version of SipHash, which generates 32-bit outputs using a weaker 64-bit key. It has *much* lower security margins, and shouldn't be used for anything too sensitive, but it could be used as a hashtable key function replacement, if the output is never exposed, and if the security requirement is not too high. The goal is to make this something that performance-critical jhash users would be willing to use. On 64-bit machines, HalfSipHash1-3 is slower than SipHash1-3, so we alias SipHash1-3 to HalfSipHash1-3 on those systems. 64-bit x86_64: [0.509409] test_siphash: SipHash2-4 cycles: 4049181 [0.510650] test_siphash: SipHash1-3 cycles: 2512884 [0.512205] test_siphash: HalfSipHash1-3 cycles: 3429920 [0.512904] test_siphash:JenkinsHash cycles: 978267 So, we map hsiphash() -> SipHash1-3 32-bit x86: [0.509868] test_siphash: SipHash2-4 cycles: 14812892 [0.513601] test_siphash: SipHash1-3 cycles: 9510710 [0.515263] test_siphash: HalfSipHash1-3 cycles: 3856157 [0.515952] test_siphash:JenkinsHash cycles: 1148567 So, we map hsiphash() -> HalfSipHash1-3 hsiphash() is roughly 3 times slower than jhash(), but comes with a considerable security improvement. Signed-off-by: Jason A. Donenfeld Reviewed-by: Jean-Philippe Aumasson Signed-off-by: David S. Miller [bwh: Backported to 3.16 to avoid a build regression for WireGuard with only part of the siphash API available] Signed-off-by: Ben Hutchings --- Documentation/siphash.txt | 75 + include/linux/siphash.h | 57 ++- lib/siphash.c | 321 +- lib/test_siphash.c| 98 +++- 4 files changed, 546 insertions(+), 5 deletions(-) --- a/Documentation/siphash.txt +++ b/Documentation/siphash.txt @@ -98,3 +98,78 @@ u64 h = siphash(&combined, offsetofend(t Read the SipHash paper if you're interested in learning more: https://131002.net/siphash/siphash.pdf + + +~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~=~ + +HalfSipHash - SipHash's insecure younger cousin +--- +Written by Jason A. Donenfeld + +On the off-chance that SipHash is not fast enough for your needs, you might be +able to justify using HalfSipHash, a terrifying but potentially useful +possibility. HalfSipHash cuts SipHash's rounds down from "2-4" to "1-3" and, +even scarier, uses an easily brute-forcable 64-bit key (with a 32-bit output) +instead of SipHash's 128-bit key. However, this may appeal to some +high-performance `jhash` users. + +Danger! + +Do not ever use HalfSipHash except for as a hashtable key function, and only +then when you can be absolutely certain that the outputs will never be +transmitted out of the kernel. This is only remotely useful over `jhash` as a +means of mitigating hashtable flooding denial of service attacks. + +1. Generating a key + +Keys should always be generated from a cryptographically secure source of +random numbers, either using get_random_bytes or get_random_once: + +hsiphash_key_t key; +get_random_bytes(&key, sizeof(key)); + +If you're not deriving your key from here, you're doing it wrong. + +2. Using the functions + +There are two variants of the function, one that takes a list of integers, and +one that takes a buffer: + +u32 hsiphash(const void *data, size_t len, const hsiphash_key_t *key); + +And: + +u32 hsiphash_1u32(u32, const hsiphash_key_t *key); +u32 hsiphash_2u32(u32, u32, const hsiphash_key_t *key); +u32 hsiphash_3u32(u32, u32, u32, const hsiphash_key_t *key); +u32 hsiphash_4u32(u32, u32, u32, u32, const hsiphash_key_t *key); + +If you pass the generic hsiphash function something of a constant length, it +will constant fold at compile-time and automatically choose one of the +optimized functions. + +3. Hashtable key function usage: + +struct some_hashtable { + DECLARE_HASHTABLE(hashtable, 8); + hsiphash_key_t key; +}; + +void init_hashtable(struct some_hashtable *table) +{ + get_random_bytes(&table->key, sizeof(table->key)); +} + +static inline hlist_head *some_hashtable_bucket(struct some_hashtable *table, struct interesting_input *input) +{ + return &table->hashtable[hsiphash(input, sizeof(*input), &table->key) & (HASH_SIZE(table->hashtable) - 1)]; +} + +You may then iterate like usual over the returned hash bucket. + +4. Performance + +HalfSipHash is roughly 3 times slower than JenkinsHash. For many replacements, +this will not be a problem, as the hashtable lookup isn't the bottleneck. And +in general, this is probably a good sacrifice to make for the security and DoS +resistance of HalfSipHash. --- a/include/linux/siphash.h +++ b/include/linux/siphash.h @@ -5,7 +5,9 @@ * SipHash: a fast short-input PRF
[PATCH 3.16 0/4] 3.16.73-rc1 review
This is the start of the stable review cycle for the 3.16.73 release. There are 4 patches in this series, which will be posted as responses to this one. If anyone has any issues with these being applied, please let me know. Responses should be made by Mon Aug 19 20:00:00 UTC 2019. Anything received after that time might be too late. All the patches have also been committed to the linux-3.16.y-rc branch of https://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-stable-rc.git . A shortlog and diffstat can be found below. Ben. - Ben Hutchings (1): tcp: Clear sk_send_head after purging the write queue [not upstream; fixes bug specific to stable] Jason A. Donenfeld (1): siphash: implement HalfSipHash1-3 for hash tables [1ae2324f732c9c4e2fa4ebd885fa1001b70d52e1] Zhangyi (2): ext4: brelse all indirect buffer in ext4_ind_remove_space() [674a2b27234d1b7afcb0a9162e81b2e53aeef217] ext4: cleanup bh release code in ext4_ind_remove_space() [5e86bdda41534e17621d5a071b294943cae4376e] Documentation/siphash.txt | 75 +++ Makefile | 4 +- fs/ext4/indirect.c| 43 --- include/linux/siphash.h | 57 +++- include/net/tcp.h | 3 + lib/siphash.c | 321 +- lib/test_siphash.c| 98 +- 7 files changed, 573 insertions(+), 28 deletions(-) -- Ben Hutchings The obvious mathematical breakthrough [to break modern encryption] would be development of an easy way to factor large prime numbers. - Bill Gates
[PATCH 3.16 2/4] ext4: brelse all indirect buffer in ext4_ind_remove_space()
3.16.73-rc1 review patch. If anyone has any objections, please let me know. -- From: "zhangyi (F)" commit 674a2b27234d1b7afcb0a9162e81b2e53aeef217 upstream. All indirect buffers get by ext4_find_shared() should be released no mater the branch should be freed or not. But now, we forget to release the lower depth indirect buffers when removing space from the same higher depth indirect block. It will lead to buffer leak and futher more, it may lead to quota information corruption when using old quota, consider the following case. - Create and mount an empty ext4 filesystem without extent and quota features, - quotacheck and enable the user & group quota, - Create some files and write some data to them, and then punch hole to some files of them, it may trigger the buffer leak problem mentioned above. - Disable quota and run quotacheck again, it will create two new aquota files and write the checked quota information to them, which probably may reuse the freed indirect block(the buffer and page cache was not freed) as data block. - Enable quota again, it will invoke vfs_load_quota_inode()->invalidate_bdev() to try to clean unused buffers and pagecache. Unfortunately, because of the buffer of quota data block is still referenced, quota code cannot read the up to date quota info from the device and lead to quota information corruption. This problem can be reproduced by xfstests generic/231 on ext3 file system or ext4 file system without extent and quota features. This patch fix this problem by releasing the missing indirect buffers, in ext4_ind_remove_space(). Reported-by: Hulk Robot Signed-off-by: zhangyi (F) Signed-off-by: Theodore Ts'o Reviewed-by: Jan Kara Signed-off-by: Ben Hutchings --- fs/ext4/indirect.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) --- a/fs/ext4/indirect.c +++ b/fs/ext4/indirect.c @@ -1481,10 +1481,14 @@ end_range: partial->p + 1, partial2->p, (chain+n-1) - partial); - BUFFER_TRACE(partial->bh, "call brelse"); - brelse(partial->bh); - BUFFER_TRACE(partial2->bh, "call brelse"); - brelse(partial2->bh); + while (partial > chain) { + BUFFER_TRACE(partial->bh, "call brelse"); + brelse(partial->bh); + } + while (partial2 > chain2) { + BUFFER_TRACE(partial2->bh, "call brelse"); + brelse(partial2->bh); + } return 0; }
[PATCH] mm/page_alloc: cleanup __alloc_pages_direct_compact()
This patch cleans up the if(page). No functional change. Signed-off-by: Pengfei Li --- mm/page_alloc.c | 28 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 272c6de1bf4e..51f056ac09f5 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3890,6 +3890,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, enum compact_priority prio, enum compact_result *compact_result) { struct page *page = NULL; + struct zone *zone; unsigned long pflags; unsigned int noreclaim_flag; @@ -3911,23 +3912,26 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order, */ count_vm_event(COMPACTSTALL); - /* Prep a captured page if available */ - if (page) + if (page) { + /* Prep a captured page if available */ prep_new_page(page, order, gfp_mask, alloc_flags); - - /* Try get a page from the freelist if available */ - if (!page) + } else { + /* Try get a page from the freelist if available */ page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac); - if (page) { - struct zone *zone = page_zone(page); - - zone->compact_blockskip_flush = false; - compaction_defer_reset(zone, order, true); - count_vm_event(COMPACTSUCCESS); - return page; + if (!page) + goto failed; } + zone = page_zone(page); + zone->compact_blockskip_flush = false; + compaction_defer_reset(zone, order, true); + + count_vm_event(COMPACTSUCCESS); + + return page; + +failed: /* * It's bad if compaction run occurs and fails. The most likely reason * is that pages exist, but not enough to satisfy watermarks. -- 2.21.0
Re: devm_memremap_pages() triggers a kasan_add_zero_shadow() warning
> On Aug 16, 2019, at 11:57 PM, Dan Williams wrote: > > On Fri, Aug 16, 2019 at 8:34 PM Qian Cai wrote: >> >> >> >>> On Aug 16, 2019, at 5:48 PM, Dan Williams wrote: >>> >>> On Fri, Aug 16, 2019 at 2:36 PM Qian Cai wrote: Every so often recently, booting Intel CPU server on linux-next triggers this warning. Trying to figure out if the commit 7cc7867fb061 ("mm/devm_memremap_pages: enable sub-section remap") is the culprit here. # ./scripts/faddr2line vmlinux devm_memremap_pages+0x894/0xc70 devm_memremap_pages+0x894/0xc70: devm_memremap_pages at mm/memremap.c:307 >>> >>> Previously the forced section alignment in devm_memremap_pages() would >>> cause the implementation to never violate the KASAN_SHADOW_SCALE_SIZE >>> (12K on x86) constraint. >>> >>> Can you provide a dump of /proc/iomem? I'm curious what resource is >>> triggering such a small alignment granularity. >> >> This is with memmap=4G!4G , >> >> # cat /proc/iomem > [..] >> 1-155df : Persistent Memory (legacy) >> 1-155df : namespace0.0 >> 155e0-15982bfff : System RAM >> 155e0-156a00fa0 : Kernel code >> 156a00fa1-15765d67f : Kernel data >> 157837000-1597f : Kernel bss >> 15982c000-1 : Persistent Memory (legacy) >> 2-87fff : System RAM > > Ok, looks like 4G is bad choice to land the pmem emulation on this > system because it collides with where the kernel is deployed and gets > broken into tiny pieces that violate kasan's. This is a known problem > with memmap=. You need to pick an memory range that does not collide > with anything else. See: > > > https://nvdimm.wiki.kernel.org/how_to_choose_the_correct_memmap_kernel_parameter_for_pmem_on_your_system > > ...for more info. Well, it seems I did exactly follow the information in that link, [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: [mem 0x-0x00093fff] usable [0.00] BIOS-e820: [mem 0x00094000-0x0009] reserved [0.00] BIOS-e820: [mem 0x000e-0x000f] reserved [0.00] BIOS-e820: [mem 0x0010-0x5a7a0fff] usable [0.00] BIOS-e820: [mem 0x5a7a1000-0x5b5e0fff] reserved [0.00] BIOS-e820: [mem 0x5b5e1000-0x790fefff] usable [0.00] BIOS-e820: [mem 0x790ff000-0x791fefff] reserved [0.00] BIOS-e820: [mem 0x791ff000-0x7b5fefff] ACPI NVS [0.00] BIOS-e820: [mem 0x7b5ff000-0x7b7fefff] ACPI data [0.00] BIOS-e820: [mem 0x7b7ff000-0x7b7f] usable [0.00] BIOS-e820: [mem 0x7b80-0x8fff] reserved [0.00] BIOS-e820: [mem 0xff80-0x] reserved [0.00] BIOS-e820: [mem 0x0001-0x00087fff] usable Where 4G is good. Then, [0.00] user-defined physical RAM map: [0.00] user: [mem 0x-0x00093fff] usable [0.00] user: [mem 0x00094000-0x0009] reserved [0.00] user: [mem 0x000e-0x000f] reserved [0.00] user: [mem 0x0010-0x5a7a0fff] usable [0.00] user: [mem 0x5a7a1000-0x5b5e0fff] reserved [0.00] user: [mem 0x5b5e1000-0x790fefff] usable [0.00] user: [mem 0x790ff000-0x791fefff] reserved [0.00] user: [mem 0x791ff000-0x7b5fefff] ACPI NVS [0.00] user: [mem 0x7b5ff000-0x7b7fefff] ACPI data [0.00] user: [mem 0x7b7ff000-0x7b7f] usable [0.00] user: [mem 0x7b80-0x8fff] reserved [0.00] user: [mem 0xff80-0x] reserved [0.00] user: [mem 0x0001-0x0001] persistent (type 12) [0.00] user: [mem 0x0002-0x00087fff] usable The doc did mention that “There seems to be an issue with CONFIG_KSAN at the moment however.” without more detail though.
Re: [PATCH 3.16 0/4] 3.16.73-rc1 review
On 8/17/19 3:35 AM, Ben Hutchings wrote: This is the start of the stable review cycle for the 3.16.73 release. There are 4 patches in this series, which will be posted as responses to this one. If anyone has any issues with these being applied, please let me know. Responses should be made by Mon Aug 19 20:00:00 UTC 2019. Anything received after that time might be too late. Build results: total: 136 pass: 136 fail: 0 Qemu test results: total: 229 pass: 229 fail: 0 Guenter
[PATCH v2] vfio_pci: Replace pci_try_reset_function() with __pci_reset_function_locked() to ensure that the pci device configuration space is restored to its original state
In vfio_pci_enable(), save the device's initial configuration information and then restore the configuration in vfio_pci_disable(). However, the execution result is not the same. Since the pci_try_reset_function() function saves the current state before resetting, the configuration information restored by pci_load_and_free_saved_state() will be overwritten. The __pci_reset_function_locked() function can be used to prevent the configuration space from being overwritten. Fixes: 890ed578df82 ("vfio-pci: Use pci "try" reset interface") Signed-off-by: hexin Signed-off-by: Liu Qi Signed-off-by: Zhang Yu --- drivers/vfio/pci/vfio_pci.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 703948c..0220616 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -438,11 +438,20 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev) pci_write_config_word(pdev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE); /* -* Try to reset the device. The success of this is dependent on -* being able to lock the device, which is not always possible. +* Try to get the locks ourselves to prevent a deadlock. The +* success of this is dependent on being able to lock the device, +* which is not always possible. +* We can not use the "try" reset interface here, which will +* overwrite the previously restored configuration information. */ - if (vdev->reset_works && !pci_try_reset_function(pdev)) - vdev->needs_reset = false; + if (vdev->reset_works && pci_cfg_access_trylock(pdev)) { + if (device_trylock(&pdev->dev)) { + if (!__pci_reset_function_locked(pdev)) + vdev->needs_reset = false; + device_unlock(&pdev->dev); + } + pci_cfg_access_unlock(pdev); + } pci_restore_state(pdev); out: -- 1.8.3.1
Greetings...
Greetings to you, I am Mrs.Margaret Ko May-Yee Leung Managing and Executive Director of Chong Hing Bank Limited. I contact you briefly in regard of a profitable business proposal i have to share with you, your collaboration is needed in a multi-million transaction with good return for us on participation for more details. I look forward to your prompt reply for more details. Regards, Margaret Ko Leung
Donation of $7,800,000.00.
Hello, I am an America Jackpot winner, I have a donation of $7,800,000.00 for you to assist your community. Kindly contact me for more information at : (jbasson...@aol.com) David.
Re: PROBLEM: 5.3.0-rc* causes iwlwifi failure
On Sat, Aug 17, 2019 at 11:59:59AM +0300, Serge Belyshev wrote: > It looks like that: > > commit 4fd445a2c855bbcab81fbe06d110e78dbd974a5b > Author: Haim Dreyfuss > Date: Thu May 2 11:45:02 2019 +0300 > > iwlwifi: mvm: Add log information about SAR status > > Inform users when SAR status is changing. > > Signed-off-by: Haim Dreyfuss > Signed-off-by: Luca Coelho > > > is the culprit. (manually) reverting it on top of 5.3-rc4 makes > everything work again. Revert how? git revert 4fd445a2c855bbcab81fbe06d110e78dbd974a5b errors out: warning: inexact rename detection was skipped due to too many files. warning: you may want to set your merge.renamelimit variable to at least 1583 and retry the command. error: could not revert 4fd445a2c855... iwlwifi: mvm: Add log information about SAR status hint: after resolving the conflicts, mark the corrected paths hint: with 'git add ' or 'git rm ' hint: and commit the result with 'git commit' Making a patch out of the commit with git format-patch -1 4fd445a2c855bbcab81fbe06d110e78dbd974a5b and attempting to apply the patch in reverse with git apply -R --ignore-space-change --ignore-whitespace also fails: error: patch failed: drivers/net/wireless/intel/iwlwifi/mvm/fw.c:861 error: drivers/net/wireless/intel/iwlwifi/mvm/fw.c: patch does not apply error: patch failed: drivers/net/wireless/intel/iwlwifi/mvm/nvm.c:620 error: drivers/net/wireless/intel/iwlwifi/mvm/nvm.c: patch does not apply
Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
On Fri 2019-08-16 14:27:28, Matthias Kaehlcke wrote: > On Fri, Aug 16, 2019 at 10:13:42PM +0200, Pavel Machek wrote: > > On Tue 2019-08-13 12:11:47, Matthias Kaehlcke wrote: > > > Add a .config_led hook which is called by the PHY core when > > > configuration data for a PHY LED is available. Each LED can be > > > configured to be solid 'off, solid 'on' for certain (or all) > > > link speeds or to blink on RX/TX activity. > > > > > > Signed-off-by: Matthias Kaehlcke > > > > THis really needs to go through the LED subsystem, > > Sorry, I used what get_maintainers.pl threw at me, I should have > manually cc-ed the LED list. > > > and use the same userland interfaces as the rest of the system. > > With the PHY maintainers we discussed to define a binding that is > compatible with that of the LED one, to have the option to integrate > it with the LED subsystem later. The integration itself is beyond the > scope of this patchset. Yes, I believe the integration is neccessary. Using same binding is neccessary for that, but not sufficient. For example, we need compatible trigger names, too. So... I'd really like to see proper integration is possible before we merge this. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[PATCH 1/1] Fix: trace sched switch start/stop refcount racy updates
Reading the sched_cmdline_ref and sched_tgid_ref initial state within tracing_start_sched_switch without holding the sched_register_mutex is racy against concurrent updates, which can lead to tracepoint probes being registered more than once (and thus trigger warnings within tracepoint.c). [ Compile-tested only. I suspect it might fix the following syzbot report: syzbot+774fddf07b7ab29a1...@syzkaller.appspotmail.com ] Signed-off-by: Mathieu Desnoyers CC: Steven Rostedt (VMware) CC: Joel Fernandes (Google) CC: Peter Zijlstra CC: Thomas Gleixner CC: Paul E. McKenney --- kernel/trace/trace_sched_switch.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c index e288168661e1..e304196d7c28 100644 --- a/kernel/trace/trace_sched_switch.c +++ b/kernel/trace/trace_sched_switch.c @@ -89,8 +89,10 @@ static void tracing_sched_unregister(void) static void tracing_start_sched_switch(int ops) { - bool sched_register = (!sched_cmdline_ref && !sched_tgid_ref); + bool sched_register; + mutex_lock(&sched_register_mutex); + sched_register = (!sched_cmdline_ref && !sched_tgid_ref); switch (ops) { case RECORD_CMDLINE: -- 2.11.0
[PULL REQUEST] i2c for 5.3
Linus, I2C has one revert because of a regression, two fixes for tiny race windows (which we were not able to trigger), a MAINTAINERS addition, and a SPDX fix. Please pull. Thanks, Wolfram The following changes since commit d45331b00ddb179e291766617259261c112db872: Linux 5.3-rc4 (2019-08-11 13:26:41 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current for you to fetch changes up to 90865a3dc597bd8463efacb749561095ba70b0aa: i2c: stm32: Use the correct style for SPDX License Identifier (2019-08-14 14:56:54 +0200) Fabio Estevam (1): Revert "i2c: imx: improve the error handling in i2c_imx_dma_request()" Nishad Kamdar (1): i2c: stm32: Use the correct style for SPDX License Identifier Oleksij Rempel (1): MAINTAINERS: i2c-imx: take over maintainership Wolfram Sang (2): i2c: rcar: avoid race when unregistering slave client i2c: emev2: avoid race when unregistering slave client MAINTAINERS| 8 drivers/i2c/busses/i2c-emev2.c | 16 drivers/i2c/busses/i2c-imx.c | 18 ++ drivers/i2c/busses/i2c-rcar.c | 11 +++ drivers/i2c/busses/i2c-stm32.h | 2 +- 5 files changed, 34 insertions(+), 21 deletions(-) signature.asc Description: PGP signature
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
- On Aug 16, 2019, at 3:15 PM, rostedt rost...@goodmis.org wrote: > On Fri, 16 Aug 2019 13:19:20 -0400 (EDT) > Mathieu Desnoyers wrote: > >> - On Aug 16, 2019, at 12:25 PM, rostedt rost...@goodmis.org wrote: >> >> > On Fri, 16 Aug 2019 10:26:43 -0400 Mathieu Desnoyers >> > wrote: >> > >> [...] >> >> >> >> Also, write and read to/from those variables should be done with >> >> WRITE_ONCE() and READ_ONCE(), given that those are read within tracing >> >> probes without holding the sched_register_mutex. >> >> >> > >> > I understand the READ_ONCE() but is the WRITE_ONCE() truly necessary? >> > It's done while holding the mutex. It's not that critical of a path, >> > and makes the code look ugly. >> >> The update is done while holding the mutex, but the read-side does not >> hold that mutex, so it can observe the intermediate state caused by >> store-tearing or invented stores which can be generated by the compiler >> on the update-side. >> >> Please refer to the following LWN article: >> >> https://lwn.net/Articles/793253/ >> >> Sections: >> - "Store tearing" >> - "Invented stores" >> >> Arguably, based on that article, store tearing is only observed in the >> wild for constants (which is not the case here), and invented stores >> seem to require specific code patterns. But I wonder why we would ever want >> to >> pair a fragile non-volatile store with a READ_ONCE() ? Considering the pain >> associated to reproduce and hunt down this kind of issue in the wild, I would >> be tempted to enforce that any READ_ONCE() operating on a variable would >> either >> need to be paired with WRITE_ONCE() or with atomic operations, so those can >> eventually be validated by static code checkers and code sanitizers. > > My issue is that this is just a case to decide if we should cache a > comm or not. It's a helper, nothing more. There's no guarantee that > something will get cached. I get your point wrt WRITE_ONCE(): since it's a cache it should not have user-visible effects if a temporary incorrect value is observed. Well in reality, it's not a cache: if the lookup fails, it returns "<...>" instead, so cache lookup failure ends up not providing any useful data in the trace. Let's assume this is a known and documented tracer limitation. However, wrt READ_ONCE(), things are different. The variable read ends up being used to control various branches in the code, and the compiler could decide to re-fetch the variable (with a different state), and therefore cause _some_ of the branches to be inconsistent. See tracing_record_taskinfo_sched_switch() and tracing_record_taskinfo() @flags parameter. AFAIU the current code should not generate any out-of-bound writes in case of re-fetch, but no comment in there documents how fragile this is. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
- On Aug 16, 2019, at 10:13 PM, rostedt rost...@goodmis.org wrote: > On Fri, 16 Aug 2019 21:36:49 -0400 (EDT) > Mathieu Desnoyers wrote: > >> - On Aug 16, 2019, at 5:04 PM, Linus Torvalds >> torva...@linux-foundation.org >> wrote: >> >> > On Fri, Aug 16, 2019 at 1:49 PM Thomas Gleixner wrote: >> >> >> >> Can we finally put a foot down and tell compiler and standard committee >> >> people to stop this insanity? >> > >> > It's already effectively done. >> > >> > Yes, values can be read from memory multiple times if they need >> > reloading. So "READ_ONCE()" when the value can change is a damn good >> > idea. >> > >> > But it should only be used if the value *can* change. Inside a locked >> > region it is actively pointless and misleading. >> > >> > Similarly, WRITE_ONCE() should only be used if you have a _reason_ for >> > using it (notably if you're not holding a lock). >> > >> > If people use READ_ONCE/WRITE_ONCE when there are locks that prevent >> > the values from changing, they are only making the code illegible. >> > Don't do it. >> >> I agree with your argument in the case where both read-side and write-side >> are protected by the same lock: READ/WRITE_ONCE are useless then. However, >> in the scenario we have here, only the write-side is protected by the lock >> against concurrent updates, but reads don't take any lock. > > And because reads are not protected by any lock or memory barrier, > using READ_ONCE() is pointless. The CPU could be doing a lot of hidden > manipulation of that variable too. Please enlighten me by providing some details on what the CPU could do to this word-aligned, word-sized variable in the absence of lock and barrier that is relevant to this specific use-case ? I suspect most of the barriers you refer to here are taken care of by the tracepoint code which uses RCU to synchronize probe registration wrt probe callback execution. > > Again, this is just to enable caching of the comm. Nothing more. It's a > "best effort" algorithm. We get it, we then can map a pid to a name. If > not, we just have a pid and we map "<...>". > > Next you'll be asking for the memory barriers to guarantee a real hit. > And honestly, this information is not worth any overhead. No, that's not my intent to add overhead to guarantee trace data availability near trace beginning and end. However, considering that READ_ONCE() and WRITE_ONCE() can provide additional data availability guarantees in the middle of traces at no runtime cost, it seems like a good trade off. It's easier for an analysis to disregard missing information at the beginning and end of trace without generating false-positive reports than when it happens spuriously in the middle of traces. > > And most often we enable this before we enable the tracepoint we want > this information from, which requires modification of the text area and > will do a bunch of syncs across CPUs. That alone will most likely keep > any race from happening. Indeed the tracepoint use of RCU to synchronize registration vs probes should take care of those barriers. > > The only real bug is the check to know if we should add the probe or > not was done outside the lock, and when we hit the race, we could add a > probe twice, causing the kernel to spit out a warning. You fixed that, > and that's all that needs to be done. I just sent that fix in a separate patch. > I'm now even more against adding the READ_ONCE() or WRITE_ONCE(). I'm not convinced by your arguments. Thanks, Mathieu > > > -- Steve > > > >> >> If WRITE_ONCE has any use at all (protecting against store tearing and >> invented stores), it should be used even with a lock held in this >> scenario, because the lock does not prevent READ_ONCE() from observing >> transient states caused by lack of WRITE_ONCE() for the update. >> >> So why does WRITE_ONCE exist in the first place ? Is it for documentation >> purposes only or are there actual situations where omitting it can cause >> bugs with real-life compilers ? >> >> In terms of code change, should we favor only introducing WRITE_ONCE >> in new code, or should existing code matching those conditions be >> moved to WRITE_ONCE as bug fixes ? >> -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH v2 4/6] dt: bindings: add mt7621-pll dt binding documentation
Hi! On Tue, Aug 13, 2019 at 11:51 PM Rob Herring wrote: > [...] > > +Example: > > + pll { > > + compatible = "mediatek,mt7621-pll"; > > You didn't answer Stephen's question on v1. I thought he was asking why there's a syscon in compatible string. I noticed that the syscon in my previous patch is a copy-paste error from elsewhere and dropped it. > > Based on this binding, there is no way to control/program the PLL. Is > this part of some IP block? The entire section is called "system control" in datasheet and is occupied in arch/mips/ralink/mt7621.c [0] Two clocks provided here is determined by reading some read-only registers in this part. There's another register in this section providing clock gates for every peripherals, but MTK doesn't provide a clock plan in their datasheet. I can't determine corresponding clock frequencies for every peripherals, thus unable to write a working clock driver. > > > + > > + #clock-cells = <1>; > > + clock-output-names = "cpu", "bus"; > > + }; > > -- > > 2.21.0 > > Regards, Chuanhong Guo [0] https://elixir.bootlin.com/linux/latest/source/arch/mips/ralink/mt7621.c#L156
Re: [PATCH] clk: Remove extraneous 'for' word in comments
Thanks Joe for higlighting this. I am going to send patches for them as well soon.
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
- On Aug 17, 2019, at 4:44 AM, Linus Torvalds torva...@linux-foundation.org wrote: > > But I'm seeing a lot of WRITE_ONCE(x, constantvalue) kind of things > and don't seem to find a lot of reason to think that they are any > inherently better than "x = constantvalue". If the only states that "x" can take is 1 or 0, then indeed there seems to be no point in using a WRITE_ONCE() when paired with a READ_ONCE() other than for documentation purposes. However, if the state of "x" can be any pointer value, or a reference count value, then not using "WRITE_ONCE()" to store a constant leaves the compiler free to perform that store in more than one memory access. Based on [1], section "Store tearing", there are situations where this happens on x86 in the wild today when storing 64-bit constants: the compiler is then free to decide to use two 32-bit immediate store instructions. Thanks, Mathieu [1] https://lwn.net/Articles/793253/ -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Sat, 17 Aug 2019 10:40:31 -0400 (EDT) Mathieu Desnoyers wrote: > > I'm now even more against adding the READ_ONCE() or WRITE_ONCE(). > > I'm not convinced by your arguments. Prove to me that there's an issue here beyond theoretical analysis, then I'll consider that patch. Show me a compiler used to compile the kernel that zeros out the increment. Show me were the race actually occurs. I think the READ/WRITE_ONCE() is more confusing than helpful. And unneeded churn to the code. And really not needed for something that's not critical to execution. -- Steve
Re: kernel BUG at include/linux/skbuff.h:LINE! (2)
syzbot has bisected this bug to: commit bc389fd101e57b36aacfaec2df8fe479eabb44ea Author: David S. Miller Date: Tue Jul 2 21:12:30 2019 + Merge branch 'macsec-fix-some-bugs-in-the-receive-path' bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=125c5c4c60 start commit: 459c5fb4 Merge branch 'mscc-PTP-support' git tree: net-next final crash:https://syzkaller.appspot.com/x/report.txt?x=115c5c4c60 console output: https://syzkaller.appspot.com/x/log.txt?x=165c5c4c60 kernel config: https://syzkaller.appspot.com/x/.config?x=d4cf1ffb87d590d7 dashboard link: https://syzkaller.appspot.com/bug?extid=eb349854e389c36d syz repro: https://syzkaller.appspot.com/x/repro.syz?x=111849e260 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1442c25a60 Reported-by: syzbot+eb349854e389c...@syzkaller.appspotmail.com Fixes: bc389fd101e5 ("Merge branch 'macsec-fix-some-bugs-in-the-receive-path'") For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Re: [PATCH v2 4/6] dt: bindings: add mt7621-pll dt binding documentation
Hi, Am 17.08.19 um 16:42 schrieb Chuanhong Guo: Hi! On Tue, Aug 13, 2019 at 11:51 PM Rob Herring wrote: [...] +Example: + pll { + compatible = "mediatek,mt7621-pll"; You didn't answer Stephen's question on v1. I thought he was asking why there's a syscon in compatible string. I noticed that the syscon in my previous patch is a copy-paste error from elsewhere and dropped it. Based on this binding, there is no way to control/program the PLL. Is this part of some IP block? The entire section is called "system control" in datasheet and is occupied in arch/mips/ralink/mt7621.c [0] Two clocks provided here is determined by reading some read-only registers in this part. There's another register in this section providing clock gates for every peripherals, but MTK doesn't provide a clock plan in their datasheet. I can't determine corresponding clock frequencies for every peripherals, thus unable to write a working clock driver. In provided link [0] the ralink_clk_init function is reading SYSC_REG_CPLL_CLKCFG0 R/W register. This register is used to determine clock source, clock freq and CPU or bus clocks. SYSC_REG_CPLL_CLKCFG1 register is a clock gate controller. It is used to enable or disable clocks. Jist wild assumption. All peripheral devices are suing bus clock. IMO - this information is enough to create full blown drivers/clk/mediatek/clk-mt7621.c + + #clock-cells = <1>; + clock-output-names = "cpu", "bus"; + }; -- 2.21.0 Regards, Chuanhong Guo [0] https://elixir.bootlin.com/linux/latest/source/arch/mips/ralink/mt7621.c#L156
Re: [PATCH v2 4/6] dt: bindings: add mt7621-pll dt binding documentation
Hi, Am 17.08.19 um 16:42 schrieb Chuanhong Guo: Hi! On Tue, Aug 13, 2019 at 11:51 PM Rob Herring wrote: [...] +Example: + pll { + compatible = "mediatek,mt7621-pll"; You didn't answer Stephen's question on v1. I thought he was asking why there's a syscon in compatible string. I noticed that the syscon in my previous patch is a copy-paste error from elsewhere and dropped it. Based on this binding, there is no way to control/program the PLL. Is this part of some IP block? The entire section is called "system control" in datasheet and is occupied in arch/mips/ralink/mt7621.c [0] Two clocks provided here is determined by reading some read-only registers in this part. There's another register in this section providing clock gates for every peripherals, but MTK doesn't provide a clock plan in their datasheet. I can't determine corresponding clock frequencies for every peripherals, thus unable to write a working clock driver. In provided link [0] the ralink_clk_init function is reading SYSC_REG_CPLL_CLKCFG0 R/W register. This register is used to determine clock source, clock freq and CPU or bus clocks. SYSC_REG_CPLL_CLKCFG1 register is a clock gate controller. It is used to enable or disable clocks. Jist wild assumption. All peripheral devices are suing bus clock. IMO - this information is enough to create full blown drivers/clk/mediatek/clk-mt7621.c + + #clock-cells = <1>; + clock-output-names = "cpu", "bus"; + }; -- 2.21.0 Regards, Chuanhong Guo [0] https://elixir.bootlin.com/linux/latest/source/arch/mips/ralink/mt7621.c#L156 -- Regards, Oleksij
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Sat, 17 Aug 2019 10:27:39 -0400 (EDT) Mathieu Desnoyers wrote: > I get your point wrt WRITE_ONCE(): since it's a cache it should not have > user-visible effects if a temporary incorrect value is observed. Well in > reality, it's not a cache: if the lookup fails, it returns "<...>" instead, > so cache lookup failure ends up not providing any useful data in the trace. > Let's assume this is a known and documented tracer limitation. Note, this is done at every sched switch, for both next and prev tasks. And the update is only done at the enabling of a tracepoint (very rare occurrence) If it missed it scheduling in, it has a really good chance of getting it while scheduling out. And 99.999% of my tracing that I do, the tasks scheduling in when enabling a tracepoint is not what I even care about, as I enable tracing then start what I want to trace. > > However, wrt READ_ONCE(), things are different. The variable read ends up > being used to control various branches in the code, and the compiler could > decide to re-fetch the variable (with a different state), and therefore > cause _some_ of the branches to be inconsistent. See > tracing_record_taskinfo_sched_switch() and tracing_record_taskinfo() @flags > parameter. I'm more OK with using a READ_ONCE() on the flags so it is consistent. But the WRITE_ONCE() is going a bit overboard. > > AFAIU the current code should not generate any out-of-bound writes in case of > re-fetch, but no comment in there documents how fragile this is. Which part of the code are you talking about here? -- Steve
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
- On Aug 17, 2019, at 11:42 AM, rostedt rost...@goodmis.org wrote: > On Sat, 17 Aug 2019 10:27:39 -0400 (EDT) > Mathieu Desnoyers wrote: > >> I get your point wrt WRITE_ONCE(): since it's a cache it should not have >> user-visible effects if a temporary incorrect value is observed. Well in >> reality, it's not a cache: if the lookup fails, it returns "<...>" instead, >> so cache lookup failure ends up not providing any useful data in the trace. >> Let's assume this is a known and documented tracer limitation. > > Note, this is done at every sched switch, for both next and prev tasks. > And the update is only done at the enabling of a tracepoint (very rare > occurrence) If it missed it scheduling in, it has a really good chance > of getting it while scheduling out. > > And 99.999% of my tracing that I do, the tasks scheduling in when > enabling a tracepoint is not what I even care about, as I enable > tracing then start what I want to trace. Since it's refcount based, my concern is about the side-effect of incrementing or decrementing that reference count without WRITE_ONCE which would lead to a transient corrupted value observed by _another_ active tracing user. For you use-case, it would lead to a missing comm when you are actively tracing what you want to trace, caused by another user of that refcount incrementing or decrementing it. I agree with you that missing tracing data at the beginning or end of a trace is not important. >> >> However, wrt READ_ONCE(), things are different. The variable read ends up >> being used to control various branches in the code, and the compiler could >> decide to re-fetch the variable (with a different state), and therefore >> cause _some_ of the branches to be inconsistent. See >> tracing_record_taskinfo_sched_switch() and tracing_record_taskinfo() @flags >> parameter. > > I'm more OK with using a READ_ONCE() on the flags so it is consistent. > But the WRITE_ONCE() is going a bit overboard. Hence my request for additional guidance on the usefulness of WRITE_ONCE(), whether it's mainly there for documentation purposes, or if we should consider that it takes care of real-life problems introduced by compiler optimizations in the wild. The LWN article seems to imply that it's not just a theoretical issue, but I'll have to let the article authors justify their conclusions, because I have limited time to investigate this myself. > >> >> AFAIU the current code should not generate any out-of-bound writes in case of >> re-fetch, but no comment in there documents how fragile this is. > > Which part of the code are you talking about here? kernel/trace/trace.c:tracing_record_taskinfo_sched_switch() kernel/trace/trace.c:tracing_record_taskinfo() where @flags is used to control a few branches. I don't think any of those would end up causing corruption if the flags is re-fetched between two branches, but it seems rather fragile. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
- On Aug 17, 2019, at 11:26 AM, rostedt rost...@goodmis.org wrote: > On Sat, 17 Aug 2019 10:40:31 -0400 (EDT) > Mathieu Desnoyers wrote: > >> > I'm now even more against adding the READ_ONCE() or WRITE_ONCE(). >> >> I'm not convinced by your arguments. > > Prove to me that there's an issue here beyond theoretical analysis, > then I'll consider that patch. > > Show me a compiler used to compile the kernel that zeros out the > increment. Show me were the race actually occurs. > > I think the READ/WRITE_ONCE() is more confusing than helpful. And > unneeded churn to the code. And really not needed for something that's > not critical to execution. I'll have to let the authors of the LWN article speak up on this, because I have limited time to replicate this investigation myself. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH 1/2] PTP: introduce new versions of IOCTLs
On Wed, Aug 14, 2019 at 10:47:11AM +0300, Felipe Balbi wrote: > The current version of the IOCTL have a small problem which prevents us > from extending the API by making use of reserved fields. In these new > IOCTLs, we are now making sure that flags and rsv fields are zero which > will allow us to extend the API in the future. > > Signed-off-by: Felipe Balbi > --- > drivers/ptp/ptp_chardev.c | 58 -- > include/uapi/linux/ptp_clock.h | 12 +++ > 2 files changed, 68 insertions(+), 2 deletions(-) > > diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c > index 18ffe449efdf..204212fc3f8c 100644 > --- a/drivers/ptp/ptp_chardev.c > +++ b/drivers/ptp/ptp_chardev.c > @@ -123,9 +123,11 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, > unsigned long arg) > struct timespec64 ts; > int enable, err = 0; > > + memset(&req, 0, sizeof(req)); Nit: please leave a blank line between memset() and switch/case. > switch (cmd) { > > case PTP_CLOCK_GETCAPS: > + case PTP_CLOCK_GETCAPS2: > memset(&caps, 0, sizeof(caps)); > caps.max_adj = ptp->info->max_adj; > caps.n_alarm = ptp->info->n_alarm; Reviewed-by: Richard Cochran
Re: [PATCH 2/2] PTP: add support for one-shot output
On Wed, Aug 14, 2019 at 10:47:12AM +0300, Felipe Balbi wrote: > diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h > index 039cd62ec706..9412b16cc8ed 100644 > --- a/include/uapi/linux/ptp_clock.h > +++ b/include/uapi/linux/ptp_clock.h > @@ -67,7 +67,9 @@ struct ptp_perout_request { > struct ptp_clock_time start; /* Absolute start time. */ > struct ptp_clock_time period; /* Desired period, zero means disable. */ > unsigned int index; /* Which channel to configure. */ > - unsigned int flags; /* Reserved for future use. */ > + > +#define PTP_PEROUT_ONE_SHOT BIT(0) > + unsigned int flags; /* Bit 0 -> oneshot output. */ The .flags field doesn't need this comment. The individual BIT macro names should be clear enough, and if not, then comment the macros. > unsigned int rsv[4]; /* Reserved for future use. */ > }; > > -- > 2.22.0 > Reviewed-by: Richard Cochran
Re: [PATCH 1/2] PTP: introduce new versions of IOCTLs
On Sat, 2019-08-17 at 08:59 -0700, Richard Cochran wrote: > On Wed, Aug 14, 2019 at 10:47:11AM +0300, Felipe Balbi wrote: > > The current version of the IOCTL have a small problem which prevents us > > from extending the API by making use of reserved fields. In these new > > IOCTLs, we are now making sure that flags and rsv fields are zero which > > will allow us to extend the API in the future. > > > > Signed-off-by: Felipe Balbi > > --- > > drivers/ptp/ptp_chardev.c | 58 -- > > include/uapi/linux/ptp_clock.h | 12 +++ > > 2 files changed, 68 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c [] > > @@ -123,9 +123,11 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int > > cmd, unsigned long arg) > > struct timespec64 ts; > > int enable, err = 0; > > > > + memset(&req, 0, sizeof(req)); > > Nit: please leave a blank line between memset() and switch/case. or just initialize the declaration of req with = {} Is there a case where this initialization is unnecessary such that it impacts performance given the use in ptp_ioctl? caps for instance is memset to zero only in PTP_CLOCK_GETCAP req is used in only 3 of the case blocks. case PTP_EXTTS_REQUEST: case PTP_PEROUT_REQUEST: case PTP_ENABLE_PPS: Maybe it would be better to move the memset(&req...) into each of the case blocks. > > switch (cmd) { > > > > case PTP_CLOCK_GETCAPS: > > + case PTP_CLOCK_GETCAPS2: > > memset(&caps, 0, sizeof(caps)); > > caps.max_adj = ptp->info->max_adj; > > caps.n_alarm = ptp->info->n_alarm; > > Reviewed-by: Richard Cochran >
Re: [PATCH] Fix an OOB bug in uac_mixer_unit_bmControls
On Sat, 17 Aug 2019 17:57:38 +0200, Hui Peng wrote: > > Looking around, there are other suspicious codes. E.g., in the following > function, it seems to be the same as `uac_mixer_unit_bmControls`, but it is > accessing `desc->bNrInPins + 5`, in case of UAC_VERSION_1. > Is this intended? Yes, this isn't for the mixer unit but for the processing unit. They have different definitions. Now back to the original report: I read the code again but fail to see where is OOB access. Let's see the function: static int uac_mixer_unit_get_channels(struct mixer_build *state, struct uac_mixer_unit_descriptor *desc) { int mu_channels; void *c; if (desc->bLength < sizeof(*desc)) return -EINVAL; if (!desc->bNrInPins) return -EINVAL; if (desc->bLength < sizeof(*desc) + desc->bNrInPins) return -EINVAL; switch (state->mixer->protocol) { case UAC_VERSION_1: case UAC_VERSION_2: default: if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 1) return 0; /* no bmControls -> skip */ mu_channels = uac_mixer_unit_bNrChannels(desc); break; case UAC_VERSION_3: mu_channels = get_cluster_channels_v3(state, uac3_mixer_unit_wClusterDescrID(desc)); break; } if (!mu_channels) return 0; ... until this point, mu_channels is calculated but no actual access happens. Then: c = uac_mixer_unit_bmControls(desc, state->mixer->protocol); ... this returns the *address* of the bmControls bitmap. At this point, it's not accessed yet. Now: if (c - (void *)desc + (mu_channels - 1) / 8 >= desc->bLength) return 0; /* no bmControls -> skip */ ... here we check whether the actual bitmap address plus the max bitmap size overflows bLength. And if it overflows, returns 0, indicating no bitmap available. So the code doesn't access but checks properly beforehand as far as I understand. Is the actual OOB access triggered by some program? thanks, Takashi > > static inline __u8 *uac_processing_unit_bmControls(struct > uac_processing_unit_descriptor *desc, >int protocol) > { > switch (protocol) { > case UAC_VERSION_1: > return &desc->baSourceID[desc->bNrInPins + 5]; > case UAC_VERSION_2: > return &desc->baSourceID[desc->bNrInPins + 6]; > case UAC_VERSION_3: > return &desc->baSourceID[desc->bNrInPins + 2]; > default: > return NULL; > } > } > > On Sat, Aug 17, 2019 at 2:55 AM Takashi Iwai wrote: > > On Sat, 17 Aug 2019 06:32:07 +0200, > Hui Peng wrote: > > > > `uac_mixer_unit_get_channels` calls `uac_mixer_unit_bmControls` > > to get pointer to bmControls field. The current implementation of > > `uac_mixer_unit_get_channels` does properly check the size of > > uac_mixer_unit_descriptor descriptor and may allow OOB access > > in `uac_mixer_unit_bmControls`. > > > > Reported-by: Hui Peng > > Reported-by: Mathias Payer > > Signed-off-by: Hui Peng > > Ah a good catch. > > One easier fix in this case would be to get the offset from > uac_mixer_unit_bmControls(), e.g. > > /* calculate the offset of bmControls field */ > size_t bmc_offset = uac_mixer_unit_bmControls(NULL, protocol) - > NULL; > > if (desc->bLength < bmc_offset) > return 0; > > thanks, > > Takashi > > > --- > > sound/usb/mixer.c | 25 ++--- > > 1 file changed, 18 insertions(+), 7 deletions(-) > > > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c > > index b5927c3d5bc0..00e6274a63c3 100644 > > --- a/sound/usb/mixer.c > > +++ b/sound/usb/mixer.c > > @@ -738,28 +738,39 @@ static int get_cluster_channels_v3(struct > mixer_build *state, unsigned int clust > > static int uac_mixer_unit_get_channels(struct mixer_build *state, > > struct uac_mixer_unit_descriptor > *desc) > > { > > - int mu_channels; > > + int mu_channels = 0; > > void *c; > > > > - if (desc->bLength < sizeof(*desc)) > > - return -EINVAL; > > if (!desc->bNrInPins) > > return -EINVAL; > > - if (desc->bLength < sizeof(*desc) + desc->bNrInPins) > > - return -EINVAL; > > > > switch (state->mixer->protocol) { > > case UAC_VERSION_1: > > + // limit derived from uac_mixer_unit_bmControls > > + if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 4) > > +
Re: [PATCH v2 4/6] dt: bindings: add mt7621-pll dt binding documentation
Hi! On Sat, Aug 17, 2019 at 11:40 PM Oleksij Rempel wrote: > In provided link [0] the ralink_clk_init function is reading > SYSC_REG_CPLL_CLKCFG0 R/W register. > This register is used to determine clock source, clock freq and CPU or bus > clocks. This register should only be changed by bootloader, not kernel. So it's read-only in kernel's perspective. > SYSC_REG_CPLL_CLKCFG1 register is a clock gate controller. It is used to > enable or disable clocks. > Jist wild assumption. All peripheral devices are suing bus clock. This assumption is incorrect. When this patchset is applied in OpenWrt, I asked the author why there's still a fixed clock in mt7621.dtsi, He told me that there's another clock for those unchanged peripherals and he doesn't have time to write a clock provider for it. I don't know how many undocumented clocks are there since this piece of info is missing in datasheet. > > IMO - this information is enough to create full blown > drivers/clk/mediatek/clk-mt7621.c And this information isn't enough because the assumption above is incorrect :P Regards, Chuanhong Guo
Re: 5.3-rc3-ish VM crash: RIP: 0010:tcp_trim_head+0x20/0xe0
On 8/17/19 10:24 AM, Sander Eikelenboom wrote: > On 12/08/2019 19:56, Eric Dumazet wrote: >> >> >> On 8/12/19 2:50 PM, Sander Eikelenboom wrote: >>> L.S., >>> >>> While testing a somewhere-after-5.3-rc3 kernel (which included the latest >>> net merge (33920f1ec5bf47c5c0a1d2113989bdd9dfb3fae9), >>> one of my Xen VM's (which gets quite some network load) crashed. >>> See below for the stacktrace. >>> >>> Unfortunately I haven't got a clear trigger, so bisection doesn't seem to >>> be an option at the moment. >>> I haven't encountered this on 5.2, so it seems to be an regression against >>> 5.2. >>> >>> Any ideas ? >>> >>> -- >>> Sander >>> >>> >>> [16930.653595] general protection fault: [#1] SMP NOPTI >>> [16930.653624] CPU: 0 PID: 3275 Comm: rsync Not tainted >>> 5.3.0-rc3-20190809-doflr+ #1 >>> [16930.653657] RIP: 0010:tcp_trim_head+0x20/0xe0 >>> [16930.653677] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 >>> fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 >>> <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8 >>> [16930.653741] RSP: :c9003ad8 EFLAGS: 00010286 >>> [16930.653762] RAX: fffe888005bf62c0 RBX: 8880115fb800 RCX: >>> 801b >> >> crash in " mov0x20(%rax),%eax" and RAX=fffe888005bf62c0 (not a valid >> kernel address) >> >> Look like one bit corruption maybe. >> >> Nothing comes to mind really between 5.2 and 53 that could explain this. >> >>> [16930.653791] RDX: 05a0 RSI: 8880115fb800 RDI: >>> 888016b00880 >>> [16930.653819] RBP: 888016b00880 R08: 0001 R09: >>> >>> [16930.653848] R10: 88800ae00800 R11: bfe632e6 R12: >>> 05a0 >>> [16930.653875] R13: 0001 R14: bfe62d46 R15: >>> 0004 >>> [16930.653913] FS: 7fe71fe2cb80() GS:88801f20() >>> knlGS: >>> [16930.653943] CS: 0010 DS: ES: CR0: 80050033 >>> [16930.653965] CR2: 55de0f3e7000 CR3: 11f32000 CR4: >>> 06f0 >>> [16930.653993] Call Trace: >>> [16930.654005] >>> [16930.654018] tcp_ack+0xbb0/0x1230 >>> [16930.654033] tcp_rcv_established+0x2e8/0x630 >>> [16930.654053] tcp_v4_do_rcv+0x129/0x1d0 >>> [16930.654070] tcp_v4_rcv+0xac9/0xcb0 >>> [16930.654088] ip_protocol_deliver_rcu+0x27/0x1b0 >>> [16930.654109] ip_local_deliver_finish+0x3f/0x50 >>> [16930.654128] ip_local_deliver+0x4d/0xe0 >>> [16930.654145] ? ip_protocol_deliver_rcu+0x1b0/0x1b0 >>> [16930.654163] ip_rcv+0x4c/0xd0 >>> [16930.654179] __netif_receive_skb_one_core+0x79/0x90 >>> [16930.654200] netif_receive_skb_internal+0x2a/0xa0 >>> [16930.654219] napi_gro_receive+0xe7/0x140 >>> [16930.654237] xennet_poll+0x9be/0xae0 >>> [16930.654254] net_rx_action+0x136/0x340 >>> [16930.654271] __do_softirq+0xdd/0x2cf >>> [16930.654287] irq_exit+0x7a/0xa0 >>> [16930.654304] xen_evtchn_do_upcall+0x27/0x40 >>> [16930.654320] xen_hvm_callback_vector+0xf/0x20 >>> [16930.654339] >>> [16930.654349] RIP: 0033:0x55de0d87db99 >>> [16930.654364] Code: 00 00 48 89 7c 24 f8 45 39 fe 45 0f 42 fe 44 89 7c 24 >>> f4 eb 09 0f 1f 40 00 83 e9 01 74 3e 89 f2 48 63 f8 4c 01 d2 44 38 1c 3a >>> <75> 25 44 38 6c 3a ff 75 1e 41 0f b6 3c 24 40 38 3a 75 14 41 0f b6 >>> [16930.654432] RSP: 002b:7ffd5531eec8 EFLAGS: 0a87 ORIG_RAX: >>> ff0c >>> [16930.655004] RAX: 0002 RBX: 55de0f3e8e50 RCX: >>> 007f >>> [16930.655034] RDX: 55de0f3dc2d2 RSI: 3492 RDI: >>> 0002 >>> [16930.655062] RBP: 7fff R08: 80ea R09: >>> 01f0 >>> [16930.655089] R10: 55de0f3d8e40 R11: 0094 R12: >>> 55de0f3e0f2a >>> [16930.655116] R13: 0010 R14: 7f16 R15: >>> 0080 >>> [16930.655144] Modules linked in: >>> [16930.655200] ---[ end trace 533367c95501b645 ]--- >>> [16930.655223] RIP: 0010:tcp_trim_head+0x20/0xe0 >>> [16930.655243] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 >>> fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 >>> <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8 >>> [16930.655312] RSP: :c9003ad8 EFLAGS: 00010286 >>> [16930.655331] RAX: fffe888005bf62c0 RBX: 8880115fb800 RCX: >>> 801b >>> [16930.655360] RDX: 05a0 RSI: 8880115fb800 RDI: >>> 888016b00880 >>> [16930.655387] RBP: 888016b00880 R08: 0001 R09: >>> >>> [16930.655414] R10: 88800ae00800 R11: bfe632e6 R12: >>> 05a0 >>> [16930.655441] R13: 0001 R14: bfe62d46 R15: >>> 0004 >>> [16930.655475] FS: 7fe71fe2cb80() GS:88801f20() >>> knlGS: >>> [16930.655502] CS: 0010 DS: ES: CR0: 80050033 >>> [16930.655525] CR2: 55de0f3e7000 CR3: 11f32000 CR4: >>> 00
Why the edge-triggered mode doesn't work for epoll file descriptor?
Hello, I've added a pipe file descriptor (fd1) to an epoll (fd3) with EPOLLOUT in edge-triggered mode, and then added the fd3 to another epoll (fd4) with EPOLLIN in edge-triggered too. Next, waiting for fd4 without timeout. When fd1 to be writable, i think epoll_wait(fd4, ...) only return once, because all file descriptors are added in edge-triggered mode. But, the actual result is returns many and many times until do once eopll_wait(fd3, ...). #include #include #include int main (int argc, char *argv[]) { int efd[2]; struct epoll_event e; efd[0] = epoll_create (1); if (efd[0] < 0) return -1; efd[1] = epoll_create (1); if (efd[1] < 0) return -2; e.events = EPOLLIN | EPOLLET; e.data.u64 = 1; if (epoll_ctl (efd[0], EPOLL_CTL_ADD, efd[1], &e) < 0) return -3; e.events = EPOLLOUT | EPOLLET; e.data.u64 = 2; if (epoll_ctl (efd[1], EPOLL_CTL_ADD, 1, &e) < 0) return -4; for (;;) { struct epoll_event events[16]; int nfds; nfds = epoll_wait (efd[0], events, 16, -1); printf ("nfds: %d\n", nfds); } close (efd[1]); close (efd[0]); return 0; } -- Best regards! Hev https://hev.cc
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Sat, 17 Aug 2019 11:55:17 -0400 (EDT) Mathieu Desnoyers wrote: > - On Aug 17, 2019, at 11:26 AM, rostedt rost...@goodmis.org wrote: > > > On Sat, 17 Aug 2019 10:40:31 -0400 (EDT) > > Mathieu Desnoyers wrote: > > > >> > I'm now even more against adding the READ_ONCE() or WRITE_ONCE(). > >> > >> I'm not convinced by your arguments. > > > > Prove to me that there's an issue here beyond theoretical analysis, > > then I'll consider that patch. > > > > Show me a compiler used to compile the kernel that zeros out the > > increment. Show me were the race actually occurs. > > > > I think the READ/WRITE_ONCE() is more confusing than helpful. And > > unneeded churn to the code. And really not needed for something that's > > not critical to execution. > > I'll have to let the authors of the LWN article speak up on this, because > I have limited time to replicate this investigation myself. I'll let Paul McKenney convince me then, if he has any spare cycles ;-) The one instance in that article is from a 2013 bug, which talks about storing a 64 bit value on a 32 bit machine. But the ref count is an int (32 bit), and I highly doubt any compiler will split it into 16 bit stores for a simple increment. And I don't believe Linux even supports any architecture that requires 16 bit stores anymore. -- Steve
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Sat, 17 Aug 2019 11:53:41 -0400 (EDT) Mathieu Desnoyers wrote: > kernel/trace/trace.c:tracing_record_taskinfo_sched_switch() > kernel/trace/trace.c:tracing_record_taskinfo() > > where @flags is used to control a few branches. I don't think any of those > would end up causing corruption if the flags is re-fetched between two > branches, but it seems rather fragile. There shouldn't be any corruption, as each flag test is not dependent on any of the other tests. But I do agree, a READ_ONCE() would be appropriate for just making a consistent state among the tests, even though they are independent. -- Steve
Re: devm_memremap_pages() triggers a kasan_add_zero_shadow() warning
On Sat, Aug 17, 2019 at 4:13 AM Qian Cai wrote: > > > > > On Aug 16, 2019, at 11:57 PM, Dan Williams wrote: > > > > On Fri, Aug 16, 2019 at 8:34 PM Qian Cai wrote: > >> > >> > >> > >>> On Aug 16, 2019, at 5:48 PM, Dan Williams > >>> wrote: > >>> > >>> On Fri, Aug 16, 2019 at 2:36 PM Qian Cai wrote: > > Every so often recently, booting Intel CPU server on linux-next triggers > this > warning. Trying to figure out if the commit 7cc7867fb061 > ("mm/devm_memremap_pages: enable sub-section remap") is the culprit here. > > # ./scripts/faddr2line vmlinux devm_memremap_pages+0x894/0xc70 > devm_memremap_pages+0x894/0xc70: > devm_memremap_pages at mm/memremap.c:307 > >>> > >>> Previously the forced section alignment in devm_memremap_pages() would > >>> cause the implementation to never violate the KASAN_SHADOW_SCALE_SIZE > >>> (12K on x86) constraint. > >>> > >>> Can you provide a dump of /proc/iomem? I'm curious what resource is > >>> triggering such a small alignment granularity. > >> > >> This is with memmap=4G!4G , > >> > >> # cat /proc/iomem > > [..] > >> 1-155df : Persistent Memory (legacy) > >> 1-155df : namespace0.0 > >> 155e0-15982bfff : System RAM > >> 155e0-156a00fa0 : Kernel code > >> 156a00fa1-15765d67f : Kernel data > >> 157837000-1597f : Kernel bss > >> 15982c000-1 : Persistent Memory (legacy) > >> 2-87fff : System RAM > > > > Ok, looks like 4G is bad choice to land the pmem emulation on this > > system because it collides with where the kernel is deployed and gets > > broken into tiny pieces that violate kasan's. This is a known problem > > with memmap=. You need to pick an memory range that does not collide > > with anything else. See: > > > > > > https://nvdimm.wiki.kernel.org/how_to_choose_the_correct_memmap_kernel_parameter_for_pmem_on_your_system > > > > ...for more info. > > Well, it seems I did exactly follow the information in that link, > > [0.00] BIOS-provided physical RAM map: > [0.00] BIOS-e820: [mem 0x-0x00093fff] usable > [0.00] BIOS-e820: [mem 0x00094000-0x0009] reserved > [0.00] BIOS-e820: [mem 0x000e-0x000f] reserved > [0.00] BIOS-e820: [mem 0x0010-0x5a7a0fff] usable > [0.00] BIOS-e820: [mem 0x5a7a1000-0x5b5e0fff] reserved > [0.00] BIOS-e820: [mem 0x5b5e1000-0x790fefff] usable > [0.00] BIOS-e820: [mem 0x790ff000-0x791fefff] reserved > [0.00] BIOS-e820: [mem 0x791ff000-0x7b5fefff] ACPI NVS > [0.00] BIOS-e820: [mem 0x7b5ff000-0x7b7fefff] ACPI > data > [0.00] BIOS-e820: [mem 0x7b7ff000-0x7b7f] usable > [0.00] BIOS-e820: [mem 0x7b80-0x8fff] reserved > [0.00] BIOS-e820: [mem 0xff80-0x] reserved > [0.00] BIOS-e820: [mem 0x0001-0x00087fff] usable > > Where 4G is good. Then, > > [0.00] user-defined physical RAM map: > [0.00] user: [mem 0x-0x00093fff] usable > [0.00] user: [mem 0x00094000-0x0009] reserved > [0.00] user: [mem 0x000e-0x000f] reserved > [0.00] user: [mem 0x0010-0x5a7a0fff] usable > [0.00] user: [mem 0x5a7a1000-0x5b5e0fff] reserved > [0.00] user: [mem 0x5b5e1000-0x790fefff] usable > [0.00] user: [mem 0x790ff000-0x791fefff] reserved > [0.00] user: [mem 0x791ff000-0x7b5fefff] ACPI NVS > [0.00] user: [mem 0x7b5ff000-0x7b7fefff] ACPI data > [0.00] user: [mem 0x7b7ff000-0x7b7f] usable > [0.00] user: [mem 0x7b80-0x8fff] reserved > [0.00] user: [mem 0xff80-0x] reserved > [0.00] user: [mem 0x0001-0x0001] persistent > (type 12) > [0.00] user: [mem 0x0002-0x00087fff] usable > > The doc did mention that “There seems to be an issue with CONFIG_KSAN at the > moment however.” > without more detail though. Does disabling CONFIG_RANDOMIZE_BASE help? Maybe that workaround has regressed. Effectively we need to find what is causing the kernel to sometimes be placed in the middle of a custom reserved memmap= range.
Re: [PATCH] Staging: speakup: spk_types: fixed an unnamed parameter style issue
Hi Matthew, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [cannot apply to v5.3-rc4 next-20190816] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Matthew-Hanzelik/Staging-speakup-spk_types-fixed-an-unnamed-parameter-style-issue/20190817-235230 config: x86_64-allyesconfig (attached as .config) compiler: gcc-7 (Debian 7.4.0-10) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All warnings (new ones prefixed by >>): drivers/staging/speakup/serialio.c: In function 'synth_readbuf_handler': >> drivers/staging/speakup/serialio.c:120:24: warning: passing argument 1 of >> 'synth->read_buff_add' makes pointer from integer without a cast >> [-Wint-conversion] synth->read_buff_add((u_char)c); ^ drivers/staging/speakup/serialio.c:120:24: note: expected 'u_char * {aka unsigned char *}' but argument is of type 'unsigned char' -- drivers/staging/speakup/spk_ttyio.c: In function 'spk_ttyio_receive_buf2': >> drivers/staging/speakup/spk_ttyio.c:82:35: warning: passing argument 1 of >> 'spk_ttyio_synth->read_buff_add' makes pointer from integer without a cast >> [-Wint-conversion] spk_ttyio_synth->read_buff_add(cp[i]); ^~ drivers/staging/speakup/spk_ttyio.c:82:35: note: expected 'u_char * {aka unsigned char *}' but argument is of type 'unsigned char' vim +120 drivers/staging/speakup/serialio.c c6e3fd22cd5383 William Hubbs 2010-10-07 111 c6e3fd22cd5383 William Hubbs 2010-10-07 112 static irqreturn_t synth_readbuf_handler(int irq, void *dev_id) c6e3fd22cd5383 William Hubbs 2010-10-07 113 { c6e3fd22cd5383 William Hubbs 2010-10-07 114unsigned long flags; c6e3fd22cd5383 William Hubbs 2010-10-07 115int c; 8e69a811068657 Domagoj Trsan 2014-09-09 116 9fb31b1abdabab William Hubbs 2013-05-13 117 spin_lock_irqsave(&speakup_info.spinlock, flags); c6e3fd22cd5383 William Hubbs 2010-10-07 118while (inb_p(speakup_info.port_tts + UART_LSR) & UART_LSR_DR) { c6e3fd22cd5383 William Hubbs 2010-10-07 119c = inb_p(speakup_info.port_tts + UART_RX); c6e3fd22cd5383 William Hubbs 2010-10-07 @120 synth->read_buff_add((u_char)c); c6e3fd22cd5383 William Hubbs 2010-10-07 121} 9fb31b1abdabab William Hubbs 2013-05-13 122 spin_unlock_irqrestore(&speakup_info.spinlock, flags); c6e3fd22cd5383 William Hubbs 2010-10-07 123return IRQ_HANDLED; c6e3fd22cd5383 William Hubbs 2010-10-07 124 } c6e3fd22cd5383 William Hubbs 2010-10-07 125 :: The code at line 120 was first introduced by commit :: c6e3fd22cd538365bfeb82997d5b89562e077d42 Staging: add speakup to the staging directory :: TO: William Hubbs :: CC: Greg Kroah-Hartman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [GIT PULL] RISC-V updates for v5.3-rc5
The pull request you sent on Fri, 16 Aug 2019 18:25:40 -0700 (PDT): > git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git > tags/riscv/for-v5.3-rc5 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2f478b60118f48bf66eaddcca0d23e80f87a682d Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PULL REQUEST] i2c for 5.3
The pull request you sent on Sat, 17 Aug 2019 16:12:32 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/05c525326957b504561d271f669d3b315930422f Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH] Fix an OOB bug in uac_mixer_unit_bmControls
On Sat, 17 Aug 2019 18:47:05 +0200, Hui Peng wrote: > > No, there was not triggering. I found it accidentally when I was going through > the code. > > Yeah, you are right. it is handled in the last check. Is it defined in the > spec that the descriptor needs to have 4/6/2 additional bytes for the > bmControl field?, if so, it is easier to understand the using the code in the > way in my first patch. > > If you think this is unnecessary, we can skip this patch. Well, the patch essentially open-codes what the helper function does adds one more check unnecessarily, so I don't think it worth. Let's skip it. thanks, Takashi > On Sat, Aug 17, 2019 at 12:18 PM Takashi Iwai wrote: > > On Sat, 17 Aug 2019 17:57:38 +0200, > Hui Peng wrote: > > > > Looking around, there are other suspicious codes. E.g., in the following > > function, it seems to be the same as `uac_mixer_unit_bmControls`, but it > is > > accessing `desc->bNrInPins + 5`, in case of UAC_VERSION_1. > > Is this intended? > > Yes, this isn't for the mixer unit but for the processing unit. > They have different definitions. > > Now back to the original report: I read the code again but fail to see > where is OOB access. Let's see the function: > > static int uac_mixer_unit_get_channels(struct mixer_build *state, > struct uac_mixer_unit_descriptor > *desc) > { > int mu_channels; > void *c; > > if (desc->bLength < sizeof(*desc)) > return -EINVAL; > if (!desc->bNrInPins) > return -EINVAL; > if (desc->bLength < sizeof(*desc) + desc->bNrInPins) > return -EINVAL; > > switch (state->mixer->protocol) { > case UAC_VERSION_1: > case UAC_VERSION_2: > default: > if (desc->bLength < sizeof(*desc) + desc->bNrInPins + 1) > return 0; /* no bmControls -> skip */ > mu_channels = uac_mixer_unit_bNrChannels(desc); > break; > case UAC_VERSION_3: > mu_channels = get_cluster_channels_v3(state, > uac3_mixer_unit_wClusterDescrID(desc)); > break; > } > > if (!mu_channels) > return 0; > > ... until this point, mu_channels is calculated but no actual access > happens. Then: > > c = uac_mixer_unit_bmControls(desc, state->mixer->protocol); > > ... this returns the *address* of the bmControls bitmap. At this > point, it's not accessed yet. Now: > > if (c - (void *)desc + (mu_channels - 1) / 8 >= desc->bLength) > return 0; /* no bmControls -> skip */ > > ... here we check whether the actual bitmap address plus the max > bitmap size overflows bLength. And if it overflows, returns 0, > indicating no bitmap available. > > So the code doesn't access but checks properly beforehand as far as I > understand. Is the actual OOB access triggered by some program? > > thanks, > > Takashi > > > > > static inline __u8 *uac_processing_unit_bmControls(struct > uac_processing_unit_descriptor *desc, > > int protocol) > > { > > switch (protocol) { > > case UAC_VERSION_1: > > return &desc->baSourceID[desc->bNrInPins + 5]; > > case UAC_VERSION_2: > > return &desc->baSourceID[desc->bNrInPins + 6]; > > case UAC_VERSION_3: > > return &desc->baSourceID[desc->bNrInPins + 2]; > > default: > > return NULL; > > } > > } > > > > On Sat, Aug 17, 2019 at 2:55 AM Takashi Iwai wrote: > > > > On Sat, 17 Aug 2019 06:32:07 +0200, > > Hui Peng wrote: > > > > > > `uac_mixer_unit_get_channels` calls `uac_mixer_unit_bmControls` > > > to get pointer to bmControls field. The current implementation of > > > `uac_mixer_unit_get_channels` does properly check the size of > > > uac_mixer_unit_descriptor descriptor and may allow OOB access > > > in `uac_mixer_unit_bmControls`. > > > > > > Reported-by: Hui Peng > > > Reported-by: Mathias Payer > > > Signed-off-by: Hui Peng > > > > Ah a good catch. > > > > One easier fix in this case would be to get the offset from > > uac_mixer_unit_bmControls(), e.g. > > > > /* calculate the offset of bmControls field */ > > size_t bmc_offset = uac_mixer_unit_bmControls(NULL, > protocol) - > > NULL; > > > >
Re: [PATCH v2 4/6] dt: bindings: add mt7621-pll dt binding documentation
Am 17.08.19 um 18:22 schrieb Chuanhong Guo: > Hi! > > On Sat, Aug 17, 2019 at 11:40 PM Oleksij Rempel wrote: > >> In provided link [0] the ralink_clk_init function is reading >> SYSC_REG_CPLL_CLKCFG0 R/W register. >> This register is used to determine clock source, clock freq and CPU or bus >> clocks. > > This register should only be changed by bootloader, not kernel. So > it's read-only in kernel's perspective. there is no kernel perspective, until you have some kind of privilege separation. There is only: "i decided not to write on to writeable register". >> SYSC_REG_CPLL_CLKCFG1 register is a clock gate controller. It is used to >> enable or disable clocks. >> Jist wild assumption. All peripheral devices are suing bus clock. > > This assumption is incorrect. When this patchset is applied in > OpenWrt, I asked the author why there's still a fixed clock in > mt7621.dtsi, He told me that there's another clock for those unchanged > peripherals and he doesn't have time to write a clock provider for it. Can you please provide a link to this patch or email. > I don't know how many undocumented clocks are there since this piece > of info is missing in datasheet. > >> >> IMO - this information is enough to create full blown >> drivers/clk/mediatek/clk-mt7621.c > > And this information isn't enough because the assumption above is incorrect :P Ok, let's assume I accept this not technical argumentation. We have at least 2 know registers: SYSC_REG_CPLL_CLKCFG0 - it provides some information about boostrapped refclock. PLL and dividers used for CPU and some sort of BUS (AHB?). SYSC_REG_CPLL_CLKCFG1 - a banch of gates to enable/disable clocks for all or some ip cores. What is probably missing is a set of dividers for each ip core. From your words it is not document. With this information the clk driver will provide gate functionality and a set of hardcoded clocks. With this driver will work part of power management and nice devicetree without fixed clocks. -- Regards, Oleksij
Re: [PATCH 05/19] irqchip/mmp: do not use of_address_to_resource() to get mux regs
On Fri, 16 Aug 2019 20:41:22 +0200 Lubomir Rintel wrote: > On Fri, 2019-08-09 at 13:12 +0100, Marc Zyngier wrote: > > On 09/08/2019 10:31, Lubomir Rintel wrote: > > > The "regs" property of the "mrvl,mmp2-mux-intc" devices are silly. They > > > are offsets from intc's base, not addresses on the parent bus. At this > > > point it probably can't be fixed. > > > > > > On an OLPC XO-1.75 machine, the muxes are children of the intc, not the > > > axi bus, and thus of_address_to_resource() won't work. We should treat > > > the values as mere integers as opposed to bus addresses. > > > > > > Signed-off-by: Lubomir Rintel > > > Acked-by: Pavel Machek > > > > > > --- > > > drivers/irqchip/irq-mmp.c | 20 +++- > > > 1 file changed, 11 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c > > > index 14618dc0bd396..af9cba4a51c2e 100644 > > > --- a/drivers/irqchip/irq-mmp.c > > > +++ b/drivers/irqchip/irq-mmp.c > > > @@ -424,9 +424,9 @@ IRQCHIP_DECLARE(mmp2_intc, "mrvl,mmp2-intc", > > > mmp2_of_init); > > > static int __init mmp2_mux_of_init(struct device_node *node, > > > struct device_node *parent) > > > { > > > - struct resource res; > > > int i, ret, irq, j = 0; > > > u32 nr_irqs, mfp_irq; > > > + u32 reg[4]; > > > > > > if (!parent) > > > return -ENODEV; > > > @@ -438,18 +438,20 @@ static int __init mmp2_mux_of_init(struct > > > device_node *node, > > > pr_err("Not found mrvl,intc-nr-irqs property\n"); > > > return -EINVAL; > > > } > > > - ret = of_address_to_resource(node, 0, &res); > > > + > > > + /* > > > + * For historical reasonsm, the "regs" property of the > > > + * mrvl,mmp2-mux-intc is not a regular * "regs" property containing > > > + * addresses on the parent bus, but offsets from the intc's base. > > > + * That is why we can't use of_address_to_resource() here. > > > + */ > > > + ret = of_property_read_u32_array(node, "reg", reg, ARRAY_SIZE(reg)); > > > > This will return 0 even if you've read less than your expected 4 u32s. > > You may want to try of_property_read_variable_u32_array instead. > > Will it? Unless I'm reading the of_property_read_u32_array() > documentation wrong, it suggests that would return -EOVERFLOW in that > case. You're appear to be right, and I read it wrong. > > It ignores the extra values it the property is larger. I guess that is > not a good thing and we still want to use > of_property_read_variable_u32_array() though. It doesn't hurt to check for all possible problems, specially given that this machine doesn't appear to have a mainline DT (and its OF implementation looks a bit buggy). Thanks, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH] arch : arm : add a criteria for pfn_valid
On Sat, Aug 17, 2019 at 11:00:13AM +0800, Zhaoyang Huang wrote: > From: Zhaoyang Huang > > pfn_valid can be wrong while the MSB of physical address be trimed as pfn > larger than the max_pfn. What scenario are you addressing here? At a guess, you're addressing the non-LPAE case with PFNs that correspond with >= 4GiB of memory? > > Signed-off-by: Zhaoyang Huang > --- > arch/arm/mm/init.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > index c2daabb..9c4d938 100644 > --- a/arch/arm/mm/init.c > +++ b/arch/arm/mm/init.c > @@ -177,7 +177,8 @@ static void __init zone_sizes_init(unsigned long min, > unsigned long max_low, > #ifdef CONFIG_HAVE_ARCH_PFN_VALID > int pfn_valid(unsigned long pfn) > { > - return memblock_is_map_memory(__pfn_to_phys(pfn)); > + return (pfn > max_pfn) ? > + false : memblock_is_map_memory(__pfn_to_phys(pfn)); > } > EXPORT_SYMBOL(pfn_valid); > #endif > -- > 1.9.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
[PATCH v2 3/7] arm64: dts: bitmain: Source common clock for UART controllers
Remove fixed clock and source common clock for UART controllers. Signed-off-by: Manivannan Sadhasivam --- arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts | 9 - arch/arm64/boot/dts/bitmain/bm1880.dtsi| 12 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts b/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts index 3e8c70778e24..7a2c7f9c2660 100644 --- a/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts +++ b/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts @@ -49,12 +49,6 @@ reg = <0x1 0x 0x0 0x4000>; // 1GB }; - uart_clk: uart-clk { - compatible = "fixed-clock"; - clock-frequency = <5>; - #clock-cells = <0>; - }; - soc { gpio0: gpio@50027000 { porta: gpio-controller@0 { @@ -173,21 +167,18 @@ &uart0 { status = "okay"; - clocks = <&uart_clk>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_uart0_default>; }; &uart1 { status = "okay"; - clocks = <&uart_clk>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_uart1_default>; }; &uart2 { status = "okay"; - clocks = <&uart_clk>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_uart2_default>; }; diff --git a/arch/arm64/boot/dts/bitmain/bm1880.dtsi b/arch/arm64/boot/dts/bitmain/bm1880.dtsi index 8471662413da..fa6e6905f588 100644 --- a/arch/arm64/boot/dts/bitmain/bm1880.dtsi +++ b/arch/arm64/boot/dts/bitmain/bm1880.dtsi @@ -174,6 +174,9 @@ uart0: serial@58018000 { compatible = "snps,dw-apb-uart"; reg = <0x0 0x58018000 0x0 0x2000>; + clocks = <&clk BM1880_CLK_UART_500M>, +<&clk BM1880_CLK_APB_UART>; + clock-names = "baudclk", "apb_pclk"; interrupts = ; reg-shift = <2>; reg-io-width = <4>; @@ -184,6 +187,9 @@ uart1: serial@5801A000 { compatible = "snps,dw-apb-uart"; reg = <0x0 0x5801a000 0x0 0x2000>; + clocks = <&clk BM1880_CLK_UART_500M>, +<&clk BM1880_CLK_APB_UART>; + clock-names = "baudclk", "apb_pclk"; interrupts = ; reg-shift = <2>; reg-io-width = <4>; @@ -194,6 +200,9 @@ uart2: serial@5801C000 { compatible = "snps,dw-apb-uart"; reg = <0x0 0x5801c000 0x0 0x2000>; + clocks = <&clk BM1880_CLK_UART_500M>, +<&clk BM1880_CLK_APB_UART>; + clock-names = "baudclk", "apb_pclk"; interrupts = ; reg-shift = <2>; reg-io-width = <4>; @@ -204,6 +213,9 @@ uart3: serial@5801E000 { compatible = "snps,dw-apb-uart"; reg = <0x0 0x5801e000 0x0 0x2000>; + clocks = <&clk BM1880_CLK_UART_500M>, +<&clk BM1880_CLK_APB_UART>; + clock-names = "baudclk", "apb_pclk"; interrupts = ; reg-shift = <2>; reg-io-width = <4>; -- 2.17.1
[PATCH v2 2/7] arm64: dts: bitmain: Add clock controller support for BM1880 SoC
Add clock controller support for Bitmain BM1880 SoC. Signed-off-by: Manivannan Sadhasivam --- arch/arm64/boot/dts/bitmain/bm1880.dtsi | 16 1 file changed, 16 insertions(+) diff --git a/arch/arm64/boot/dts/bitmain/bm1880.dtsi b/arch/arm64/boot/dts/bitmain/bm1880.dtsi index d65453f99a99..8471662413da 100644 --- a/arch/arm64/boot/dts/bitmain/bm1880.dtsi +++ b/arch/arm64/boot/dts/bitmain/bm1880.dtsi @@ -4,6 +4,7 @@ * Author: Manivannan Sadhasivam */ +#include #include #include @@ -66,6 +67,12 @@ ; }; + osc: osc { + compatible = "fixed-clock"; + clock-frequency = <2500>; + #clock-cells = <0>; + }; + soc { compatible = "simple-bus"; #address-cells = <2>; @@ -94,6 +101,15 @@ reg = <0x400 0x120>; }; + clk: clock-controller@e8 { + compatible = "bitmain,bm1880-clk"; + reg = <0xe8 0x0c>, <0x800 0xb0>; + reg-names = "pll", "sys"; + clocks = <&osc>; + clock-names = "osc"; + #clock-cells = <1>; + }; + rst: reset-controller@c00 { compatible = "bitmain,bm1880-reset"; reg = <0xc00 0x8>; -- 2.17.1
[PATCH v2 0/7] Add Bitmain BM1880 clock driver
Hello, This patchset adds common clock driver for Bitmain BM1880 SoC clock controller. The clock controller consists of gate, divider, mux and pll clocks with different compositions. Hence, the driver uses composite clock structure in place where multiple clocking units are combined together. This patchset also removes UART fixed clock and sources clocks from clock controller for Sophon Edge board where the driver has been validated. Thanks, Mani Changes in v2: * Converted the dt binding to YAML * Incorporated review comments from Stephen (majority of change is switching to new way of specifying clk parents) Manivannan Sadhasivam (7): dt-bindings: clock: Add devicetree binding for BM1880 SoC arm64: dts: bitmain: Add clock controller support for BM1880 SoC arm64: dts: bitmain: Source common clock for UART controllers clk: Add common clock driver for BM1880 SoC MAINTAINERS: Add entry for BM1880 SoC clock driver clk: Warn if clk_init_data is not zero initialized clk: Zero init clk_init_data in helpers .../bindings/clock/bitmain,bm1880-clk.yaml| 83 ++ MAINTAINERS | 2 + .../boot/dts/bitmain/bm1880-sophon-edge.dts | 9 - arch/arm64/boot/dts/bitmain/bm1880.dtsi | 28 + drivers/clk/Kconfig | 6 + drivers/clk/Makefile | 1 + drivers/clk/clk-bm1880.c | 970 ++ drivers/clk/clk-composite.c | 2 +- drivers/clk/clk-divider.c | 2 +- drivers/clk/clk-fixed-rate.c | 2 +- drivers/clk/clk-gate.c| 2 +- drivers/clk/clk-mux.c | 2 +- drivers/clk/clk.c | 8 + include/dt-bindings/clock/bm1880-clock.h | 82 ++ 14 files changed, 1185 insertions(+), 14 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml create mode 100644 drivers/clk/clk-bm1880.c create mode 100644 include/dt-bindings/clock/bm1880-clock.h -- 2.17.1
[PATCH v2 1/7] dt-bindings: clock: Add devicetree binding for BM1880 SoC
Add YAML devicetree binding for Bitmain BM1880 SoC. Signed-off-by: Manivannan Sadhasivam --- .../bindings/clock/bitmain,bm1880-clk.yaml| 83 +++ include/dt-bindings/clock/bm1880-clock.h | 82 ++ 2 files changed, 165 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml create mode 100644 include/dt-bindings/clock/bm1880-clock.h diff --git a/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml b/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml new file mode 100644 index ..a457f996287d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml @@ -0,0 +1,83 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/bindings/clock/bitmain,bm1880-clk.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Bitmain BM1880 Clock Controller + +maintainers: + - Manivannan Sadhasivam + +description: | + The Bitmain BM1880 clock controller generates and supplies clock to + various peripherals within the SoC. + + This binding uses common clock bindings + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +properties: + compatible: +oneOf: + - items: + - enum: + - bitmain,bm1880-clk + + reg: +minItems: 2 +maxItems: 2 +items: + - description: pll registers + - description: system registers + + reg-names: +items: + - const: pll + - const: sys + + clocks: +maxItems: 1 +description: Phandle of the input reference clock + + clock-names: +maxItems: 1 +items: + - const: osc + + '#clock-cells': +const: 1 + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - '#clock-cells' + +examples: + # Clock controller node: + - | +clk: clock-controller@e8 { +compatible = "bitmain,bm1880-clk"; +reg = <0xe8 0x0c>, <0x800 0xb0>; +reg-names = "pll", "sys"; +clocks = <&osc>; +clock-names = "osc"; +#clock-cells = <1>; +}; + + # Example UART controller node that consumes clock generated by the clock controller: + - | +uart0: serial@58018000 { + compatible = "snps,dw-apb-uart"; + reg = <0x0 0x58018000 0x0 0x2000>; + clocks = <&clk BM1880_CLK_UART_500M>; + <&clk BM1880_CLK_APB_UART>; + clock-names = "baudclk", "apb_pclk"; + interrupts = ; + reg-shift = <2>; + reg-io-width = <4>; +}; + +... diff --git a/include/dt-bindings/clock/bm1880-clock.h b/include/dt-bindings/clock/bm1880-clock.h new file mode 100644 index ..895646d66b07 --- /dev/null +++ b/include/dt-bindings/clock/bm1880-clock.h @@ -0,0 +1,82 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Device Tree binding constants for Bitmain BM1880 SoC + * + * Copyright (c) 2019 Linaro Ltd. + */ + +#ifndef __DT_BINDINGS_CLOCK_BM1880_H +#define __DT_BINDINGS_CLOCK_BM1880_H + +#define BM1880_CLK_OSC 0 +#define BM1880_CLK_MPLL1 +#define BM1880_CLK_SPLL2 +#define BM1880_CLK_FPLL3 +#define BM1880_CLK_DDRPLL 4 +#define BM1880_CLK_A53 5 +#define BM1880_CLK_50M_A53 6 +#define BM1880_CLK_AHB_ROM 7 +#define BM1880_CLK_AXI_SRAM8 +#define BM1880_CLK_DDR_AXI 9 +#define BM1880_CLK_EFUSE 10 +#define BM1880_CLK_APB_EFUSE 11 +#define BM1880_CLK_AXI5_EMMC 12 +#define BM1880_CLK_EMMC13 +#define BM1880_CLK_100K_EMMC 14 +#define BM1880_CLK_AXI5_SD 15 +#define BM1880_CLK_SD 16 +#define BM1880_CLK_100K_SD 17 +#define BM1880_CLK_500M_ETH0 18 +#define BM1880_CLK_AXI4_ETH0 19 +#define BM1880_CLK_500M_ETH1 20 +#define BM1880_CLK_AXI4_ETH1 21 +#define BM1880_CLK_AXI1_GDMA 22 +#define BM1880_CLK_APB_GPIO23 +#define BM1880_CLK_APB_GPIO_INTR 24 +#define BM1880_CLK_GPIO_DB 25 +#define BM1880_CLK_AXI1_MINER 26 +#define BM1880_CLK_AHB_SF 27 +#define BM1880_CLK_SDMA_AXI28 +#define BM1880_CLK_SDMA_AUD29 +#define BM1880_CLK_APB_I2C 30 +#define BM1880_CLK_APB_WDT 31 +#define BM1880_CLK_APB_JPEG32 +#define BM1880_CLK_JPEG_AXI33 +#define BM1880_CLK_AXI5_NF 34 +#define BM1880_CLK_APB_NF 35 +#define BM1880_CLK_NF 36 +#define BM1880_CLK_APB_PWM 37 +#define BM1880_CLK_DIV_0_RV38 +#define BM1880_CLK_DIV_1_RV39 +#define BM1880_CLK_MUX_RV 40 +#define BM1880_CLK_RV 41 +#define BM1880_CLK_APB_SPI 42 +#define BM1880_CLK_TPU_AXI 43 +#define BM1880_CLK_DIV_UART_500M 44 +#defin
[PATCH v2 4/7] clk: Add common clock driver for BM1880 SoC
Add common clock driver for Bitmain BM1880 SoC. The clock controller on BM1880 has supplies clocks to all peripherals in the form of gate clocks and composite clocks (fixed factor + gate). Signed-off-by: Manivannan Sadhasivam --- drivers/clk/Kconfig | 6 + drivers/clk/Makefile | 1 + drivers/clk/clk-bm1880.c | 970 +++ 3 files changed, 977 insertions(+) create mode 100644 drivers/clk/clk-bm1880.c diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 801fa1cd0321..9dc19d16d9d9 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -139,6 +139,12 @@ config COMMON_CLK_SI570 This driver supports Silicon Labs 570/571/598/599 programmable clock generators. +config COMMON_CLK_BM1880 + bool "Clock driver for Bitmain BM1880 SoC" + depends on ARCH_BITMAIN || COMPILE_TEST + help + This driver supports the clocks on Bitmain BM1880 SoC. + config COMMON_CLK_CDCE706 tristate "Clock driver for TI CDCE706 clock synthesizer" depends on I2C diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 0cad76021297..2c1ae6289a78 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_MACH_ASM9260)+= clk-asm9260.o obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)+= clk-axi-clkgen.o obj-$(CONFIG_ARCH_AXXIA) += clk-axm5516.o obj-$(CONFIG_COMMON_CLK_BD718XX) += clk-bd718x7.o +obj-$(CONFIG_COMMON_CLK_BM1880)+= clk-bm1880.o obj-$(CONFIG_COMMON_CLK_CDCE706) += clk-cdce706.o obj-$(CONFIG_COMMON_CLK_CDCE925) += clk-cdce925.o obj-$(CONFIG_ARCH_CLPS711X)+= clk-clps711x.o diff --git a/drivers/clk/clk-bm1880.c b/drivers/clk/clk-bm1880.c new file mode 100644 index ..1a7871daaef4 --- /dev/null +++ b/drivers/clk/clk-bm1880.c @@ -0,0 +1,970 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Bitmain BM1880 SoC clock driver + * + * Copyright (c) 2019 Linaro Ltd. + * Author: Manivannan Sadhasivam + */ + +#include +#include +#include +#include +#include +#include +#include + +#include + +#define BM1880_CLK_MPLL_CTL0x00 +#define BM1880_CLK_SPLL_CTL0x04 +#define BM1880_CLK_FPLL_CTL0x08 +#define BM1880_CLK_DDRPLL_CTL 0x0c + +#define BM1880_CLK_ENABLE0 0x00 +#define BM1880_CLK_ENABLE1 0x04 +#define BM1880_CLK_SELECT 0x20 +#define BM1880_CLK_DIV00x40 +#define BM1880_CLK_DIV10x44 +#define BM1880_CLK_DIV20x48 +#define BM1880_CLK_DIV30x4c +#define BM1880_CLK_DIV40x50 +#define BM1880_CLK_DIV50x54 +#define BM1880_CLK_DIV60x58 +#define BM1880_CLK_DIV70x5c +#define BM1880_CLK_DIV80x60 +#define BM1880_CLK_DIV90x64 +#define BM1880_CLK_DIV10 0x68 +#define BM1880_CLK_DIV11 0x6c +#define BM1880_CLK_DIV12 0x70 +#define BM1880_CLK_DIV13 0x74 +#define BM1880_CLK_DIV14 0x78 +#define BM1880_CLK_DIV15 0x7c +#define BM1880_CLK_DIV16 0x80 +#define BM1880_CLK_DIV17 0x84 +#define BM1880_CLK_DIV18 0x88 +#define BM1880_CLK_DIV19 0x8c +#define BM1880_CLK_DIV20 0x90 +#define BM1880_CLK_DIV21 0x94 +#define BM1880_CLK_DIV22 0x98 +#define BM1880_CLK_DIV23 0x9c +#define BM1880_CLK_DIV24 0xa0 +#define BM1880_CLK_DIV25 0xa4 +#define BM1880_CLK_DIV26 0xa8 +#define BM1880_CLK_DIV27 0xac +#define BM1880_CLK_DIV28 0xb0 + +#define to_bm1880_pll_clk(_hw) container_of(_hw, struct bm1880_pll_hw_clock, hw) +#define to_bm1880_div_clk(_hw) container_of(_hw, struct bm1880_div_hw_clock, hw) + +static DEFINE_SPINLOCK(bm1880_clk_lock); + +struct bm1880_clock_data { + void __iomem *pll_base; + void __iomem *sys_base; + struct clk_onecell_data clk_data; +}; + +struct bm1880_gate_clock { + unsigned intid; + const char *name; + const char *parent; + u32 gate_reg; + s8 gate_shift; + unsigned long flags; +}; + +struct bm1880_mux_clock { + unsigned intid; + const char *name; + const char * const *parents; + s8 num_parents; + u32 reg; + s8 shift; + unsigned long flags; +}; + +struct bm1880_div_clock { + unsigned intid; + const char *name; + u32 reg; + u8 shift; + u8 width; + u32 initval; + const struct clk_div_table *table; + unsigned long flags; +}; + +struct bm1880_div_hw_clock { + struct bm1880_div_clock div; + void __iomem *base; + spinlock_t *lock; + struct clk_hw hw; + struct clk_init_data init; +}; + +struct bm1880_composite_clock { + unsigned intid; + const char *name; + const char *parent; +
[PATCH v2 6/7] clk: Warn if clk_init_data is not zero initialized
The new implementation for determining parent map uses multiple ways to pass parent info. The order in which it gets processed depends on the first available member. Hence, it is necessary to zero init the clk_init_data struct so that the expected member gets processed correctly. So, add a warning if multiple clk_init_data members are available during clk registration. Signed-off-by: Manivannan Sadhasivam --- drivers/clk/clk.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c0990703ce54..7d6d6984c979 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3497,6 +3497,14 @@ static int clk_core_populate_parent_map(struct clk_core *core) if (!num_parents) return 0; + /* +* Check for non-zero initialized clk_init_data struct. This is +* required because, we only require one of the (parent_names/ +* parent_data/parent_hws) to be set at a time. Otherwise, the +* current code would use first available member. +*/ + WARN_ON((parent_names && parent_data) || (parent_names && parent_hws)); + /* * Avoid unnecessary string look-ups of clk_core's possible parents by * having a cache of names/clk_hw pointers to clk_core pointers. -- 2.17.1
[PATCH v2 7/7] clk: Zero init clk_init_data in helpers
The clk_init_data struct needs to be initialized to zero for the new parent_map implementation to work correctly. Otherwise, the member which is available first will get processed. Signed-off-by: Manivannan Sadhasivam --- drivers/clk/clk-composite.c | 2 +- drivers/clk/clk-divider.c| 2 +- drivers/clk/clk-fixed-rate.c | 2 +- drivers/clk/clk-gate.c | 2 +- drivers/clk/clk-mux.c| 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index b06038b8f658..4d579f9d20f6 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -208,7 +208,7 @@ struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name, unsigned long flags) { struct clk_hw *hw; - struct clk_init_data init; + struct clk_init_data init = { NULL }; struct clk_composite *composite; struct clk_ops *clk_composite_ops; int ret; diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index 3f9ff78c4a2a..65dd8137f9ec 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -471,7 +471,7 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name, { struct clk_divider *div; struct clk_hw *hw; - struct clk_init_data init; + struct clk_init_data init = { NULL }; int ret; if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) { diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index a7e4aef7a376..746c3ecdc5b3 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -58,7 +58,7 @@ struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev, { struct clk_fixed_rate *fixed; struct clk_hw *hw; - struct clk_init_data init; + struct clk_init_data init = { NULL }; int ret; /* allocate fixed-rate clock */ diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index 1b99fc962745..8ed83ec730cb 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -141,7 +141,7 @@ struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name, { struct clk_gate *gate; struct clk_hw *hw; - struct clk_init_data init; + struct clk_init_data init = { NULL }; int ret; if (clk_gate_flags & CLK_GATE_HIWORD_MASK) { diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 66e91f740508..2caa6b2a9ee5 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -153,7 +153,7 @@ struct clk_hw *clk_hw_register_mux_table(struct device *dev, const char *name, { struct clk_mux *mux; struct clk_hw *hw; - struct clk_init_data init; + struct clk_init_data init = { NULL }; u8 width = 0; int ret; -- 2.17.1
[PATCH v2 5/7] MAINTAINERS: Add entry for BM1880 SoC clock driver
Add MAINTAINERS entry for Bitmain BM1880 SoC clock driver. Signed-off-by: Manivannan Sadhasivam --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 997a4f8fe88e..280defec35b2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1503,8 +1503,10 @@ M: Manivannan Sadhasivam L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) S: Maintained F: arch/arm64/boot/dts/bitmain/ +F: drivers/clk/clk-bm1880.c F: drivers/pinctrl/pinctrl-bm1880.c F: Documentation/devicetree/bindings/arm/bitmain.yaml +F: Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml F: Documentation/devicetree/bindings/pinctrl/bitmain,bm1880-pinctrl.txt ARM/CALXEDA HIGHBANK ARCHITECTURE -- 2.17.1
[RFC PATCH] powerpc: use __builtin_trap() in BUG/WARN macros.
The below exemples of use of WARN_ON() show that the result is sub-optimal in regard of the capabilities of powerpc. void test_warn1(unsigned long long a) { WARN_ON(a); } void test_warn2(unsigned long a) { WARN_ON(a); } void test_warn3(unsigned long a, unsigned long b) { WARN_ON(a < b); } void test_warn4(unsigned long a, unsigned long b) { WARN_ON(!a); } void test_warn5(unsigned long a, unsigned long b) { WARN_ON(!a && b); } : 0: 7c 64 23 78 or r4,r3,r4 4: 31 24 ff ff addic r9,r4,-1 8: 7c 89 21 10 subfe r4,r9,r4 c: 0f 04 00 00 twnei r4,0 10: 4e 80 00 20 blr 0014 : 14: 31 23 ff ff addic r9,r3,-1 18: 7c 69 19 10 subfe r3,r9,r3 1c: 0f 03 00 00 twnei r3,0 20: 4e 80 00 20 blr 0024 : 24: 7c 84 18 10 subfc r4,r4,r3 28: 7d 29 49 10 subfe r9,r9,r9 2c: 7d 29 00 d0 neg r9,r9 30: 0f 09 00 00 twnei r9,0 34: 4e 80 00 20 blr 0038 : 38: 7c 63 00 34 cntlzw r3,r3 3c: 54 63 d9 7e rlwinm r3,r3,27,5,31 40: 0f 03 00 00 twnei r3,0 44: 4e 80 00 20 blr 0048 : 48: 2f 83 00 00 cmpwi cr7,r3,0 4c: 39 20 00 00 li r9,0 50: 41 9e 00 0c beq cr7,5c 54: 7c 84 00 34 cntlzw r4,r4 58: 54 89 d9 7e rlwinm r9,r4,27,5,31 5c: 0f 09 00 00 twnei r9,0 60: 4e 80 00 20 blr RELOCATION RECORDS FOR [__bug_table]: OFFSET TYPE VALUE R_PPC_ADDR32 .text+0x000c 000c R_PPC_ADDR32 .text+0x001c 0018 R_PPC_ADDR32 .text+0x0030 0018 R_PPC_ADDR32 .text+0x0030 0024 R_PPC_ADDR32 .text+0x0040 0030 R_PPC_ADDR32 .text+0x005c Using __builtin_trap() instead of inline assembly of twnei/tdnei provides a far better result: : 0: 7c 64 23 78 or r4,r3,r4 4: 0f 04 00 00 twnei r4,0 8: 4e 80 00 20 blr 000c : c: 0f 03 00 00 twnei r3,0 10: 4e 80 00 20 blr 0014 : 14: 7c 43 20 08 twllt r3,r4 18: 4e 80 00 20 blr 001c : 1c: 0c 83 00 00 tweqi r3,0 20: 4e 80 00 20 blr 0024 : 24: 2f 83 00 00 cmpwi cr7,r3,0 28: 41 9e 00 08 beq cr7,30 2c: 0c 84 00 00 tweqi r4,0 30: 4e 80 00 20 blr RELOCATION RECORDS FOR [__bug_table]: OFFSET TYPE VALUE R_PPC_ADDR32 .text+0x0004 000c R_PPC_ADDR32 .text+0x000c 0018 R_PPC_ADDR32 .text+0x0014 0024 R_PPC_ADDR32 .text+0x001c 0030 R_PPC_ADDR32 .text+0x002c Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/bug.h | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index fed7e6241349..1a37c8d30b78 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -44,14 +44,14 @@ #ifdef CONFIG_DEBUG_BUGVERBOSE #define _EMIT_BUG_ENTRY\ ".section __bug_table,\"aw\"\n" \ - "2:\t" PPC_LONG "1b, %0\n" \ + "2:\t" PPC_LONG "1b - 4, %0\n" \ "\t.short %1, %2\n" \ ".org 2b+%3\n" \ ".previous\n" #else #define _EMIT_BUG_ENTRY\ ".section __bug_table,\"aw\"\n" \ - "2:\t" PPC_LONG "1b\n" \ + "2:\t" PPC_LONG "1b - 4\n" \ "\t.short %2\n" \ ".org 2b+%3\n" \ ".previous\n" @@ -64,8 +64,9 @@ */ #define BUG() do { \ + __builtin_trap(); \ __asm__ __volatile__( \ - "1: twi 31,0,0\n" \ + "1:\n" \ _EMIT_BUG_ENTRY \ : : "i" (__FILE__), "i" (__LINE__), \ "i" (0), "i" (sizeof(struct bug_entry))); \ @@ -77,18 +78,20 @@ if (x) \ BUG(); \ } else {\ + if (x) \ + __builtin_trap(); \ __asm__ __volatile__( \ - "1: "PPC_TLNEI" %4,0\n" \ + "1:\n" \ _EMIT_BUG_ENTRY \ : : "i" (__FILE__), "i" (__LINE__), "i" (0),
[PATCH net-next v3 03/12] net: stmmac: xgmac: Correctly return that RX descriptor is not last one
Return the correct value when RX descriptor is not the last one. Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c index 58b69fa97837..2c1ed8c2a9d3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c @@ -26,16 +26,15 @@ static int dwxgmac2_get_rx_status(void *data, struct stmmac_extra_stats *x, struct dma_desc *p) { unsigned int rdes3 = le32_to_cpu(p->des3); - int ret = good_frame; if (unlikely(rdes3 & XGMAC_RDES3_OWN)) return dma_own; if (likely(!(rdes3 & XGMAC_RDES3_LD))) + return rx_not_ls; + if (unlikely((rdes3 & XGMAC_RDES3_ES) && (rdes3 & XGMAC_RDES3_LD))) return discard_frame; - if (unlikely(rdes3 & XGMAC_RDES3_ES)) - ret = discard_frame; - return ret; + return good_frame; } static int dwxgmac2_get_tx_len(struct dma_desc *p) -- 2.7.4
[PATCH net-next v3 11/12] net: stmmac: Add support for VLAN Insertion Offload
Adds the logic to insert a given VLAN ID in a packet. This is offloaded to HW and its descriptor based. For now, only XGMAC implements the necessary callbacks. Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/common.h | 8 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 15 +++ .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c| 14 ++ .../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 35 +++ drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 2 + drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 52 +- 7 files changed, 135 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 1303ec81fd3d..49aa56ca09cc 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -358,6 +358,8 @@ struct dma_features { unsigned int rssen; unsigned int vlhash; unsigned int sphen; + unsigned int vlins; + unsigned int dvlan; }; /* GMAC TX FIFO is 8K, Rx FIFO is 16K */ @@ -389,6 +391,12 @@ struct dma_features { #define STMMAC_RSS_HASH_KEY_SIZE 40 #define STMMAC_RSS_MAX_TABLE_SIZE 256 +/* VLAN */ +#define STMMAC_VLAN_NONE 0x0 +#define STMMAC_VLAN_REMOVE 0x1 +#define STMMAC_VLAN_INSERT 0x2 +#define STMMAC_VLAN_REPLACE0x3 + extern const struct stmmac_desc_ops enh_desc_ops; extern const struct stmmac_desc_ops ndesc_ops; diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 79c145ba25a8..7357b8bdc128 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -63,6 +63,11 @@ #define XGMAC_VLAN_ETV BIT(16) #define XGMAC_VLAN_VID GENMASK(15, 0) #define XGMAC_VLAN_HASH_TABLE 0x0058 +#define XGMAC_VLAN_INCL0x0060 +#define XGMAC_VLAN_VLTIBIT(20) +#define XGMAC_VLAN_CSVLBIT(19) +#define XGMAC_VLAN_VLC GENMASK(17, 16) +#define XGMAC_VLAN_VLC_SHIFT 16 #define XGMAC_RXQ_CTRL00x00a0 #define XGMAC_RXQEN(x) GENMASK((x) * 2 + 1, (x) * 2) #define XGMAC_RXQEN_SHIFT(x) ((x) * 2) @@ -128,6 +133,7 @@ #define XGMAC_HWFEAT_RXQCNTGENMASK(3, 0) #define XGMAC_HW_FEATURE3 0x0128 #define XGMAC_HWFEAT_ASP GENMASK(15, 14) +#define XGMAC_HWFEAT_DVLAN BIT(13) #define XGMAC_HWFEAT_FRPES GENMASK(12, 11) #define XGMAC_HWFEAT_FRPPB GENMASK(10, 9) #define XGMAC_HWFEAT_FRPSELBIT(3) @@ -337,10 +343,14 @@ #define XGMAC_REGSIZE ((0x317c + (0x80 * 15)) / 4) /* Descriptors */ +#define XGMAC_TDES2_IVTGENMASK(31, 16) +#define XGMAC_TDES2_IVT_SHIFT 16 #define XGMAC_TDES2_IOCBIT(31) #define XGMAC_TDES2_TTSE BIT(30) #define XGMAC_TDES2_B2LGENMASK(29, 16) #define XGMAC_TDES2_B2L_SHIFT 16 +#define XGMAC_TDES2_VTIR GENMASK(15, 14) +#define XGMAC_TDES2_VTIR_SHIFT 14 #define XGMAC_TDES2_B1LGENMASK(13, 0) #define XGMAC_TDES3_OWNBIT(31) #define XGMAC_TDES3_CTXT BIT(30) @@ -353,10 +363,15 @@ #define XGMAC_TDES3_SAIC_SHIFT 23 #define XGMAC_TDES3_THLGENMASK(22, 19) #define XGMAC_TDES3_THL_SHIFT 19 +#define XGMAC_TDES3_IVTIR GENMASK(19, 18) +#define XGMAC_TDES3_IVTIR_SHIFT18 #define XGMAC_TDES3_TSEBIT(18) +#define XGMAC_TDES3_IVLTV BIT(17) #define XGMAC_TDES3_CICGENMASK(17, 16) #define XGMAC_TDES3_CIC_SHIFT 16 #define XGMAC_TDES3_TPLGENMASK(17, 0) +#define XGMAC_TDES3_VLTV BIT(16) +#define XGMAC_TDES3_VT GENMASK(15, 0) #define XGMAC_TDES3_FL GENMASK(14, 0) #define XGMAC_RDES2_HL GENMASK(9, 0) #define XGMAC_RDES3_OWNBIT(31) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index d8483d088711..e534a3aaf4a3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -1150,6 +1150,19 @@ static void dwxgmac2_sarc_configure(void __iomem *ioaddr, int val) w
[PATCH net-next v3 00/12] net: stmmac: Improvements for -next
Couple of improvements for -next tree. More info in commit logs. --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- Jose Abreu (12): net: stmmac: Get correct timestamp values from XGMAC net: stmmac: Prepare to add Split Header support net: stmmac: xgmac: Correctly return that RX descriptor is not last one net: stmmac: Add Split Header support and enable it in XGMAC cores net: stmmac: Add a counter for Split Header packets net: stmmac: dwxgmac: Add Flexible PPS support net: stmmac: Add ethtool register dump for XGMAC cores net: stmmac: Add support for SA Insertion/Replacement in XGMAC cores net: stmmac: selftests: Add tests for SA Insertion/Replacement net: stmmac: xgmac: Add EEE support net: stmmac: Add support for VLAN Insertion Offload net: stmmac: selftests: Add selftest for VLAN TX Offload drivers/net/ethernet/stmicro/stmmac/common.h | 10 + drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 56 .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c| 182 - .../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 85 +- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 31 ++- drivers/net/ethernet/stmicro/stmmac/hwif.h | 30 +++ drivers/net/ethernet/stmicro/stmmac/stmmac.h | 10 + .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 24 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 286 - .../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 194 +- 10 files changed, 817 insertions(+), 91 deletions(-) -- 2.7.4
[PATCH net-next v3 04/12] net: stmmac: Add Split Header support and enable it in XGMAC cores
Add the support for Split Header feature in the RX path and enable it in XGMAC cores. This does not impact neither beneficts bandwidth but it does reduces CPU usage because without the feature all the entire packet is memcpy'ed, while that with the feature only the header is. With Split Header disabled 'perf stat -d' gives: 86870.624945 task-clock (msec) #0.429 CPUs utilized 1073352 context-switches #0.012 M/sec 1 cpu-migrations #0.000 K/sec 213 page-faults#0.002 K/sec 327113872376 cycles #3.766 GHz (62.53%) 56618161216 instructions #0.17 insn per cycle (75.06%) 10742205071 branches # 123.658 M/sec (75.36%) 584309242 branch-misses #5.44% of all branches (75.19%) 17594787965 L1-dcache-loads# 202.540 M/sec (74.88%) 4003773131 L1-dcache-load-misses # 22.76% of all L1-dcache hits (74.89%) 1313301468 LLC-loads # 15.118 M/sec (49.75%) 355906510 LLC-load-misses# 27.10% of all LL-cache hits (49.92%) With Split Header enabled 'perf stat -d' gives: 49324.456539 task-clock (msec) #0.245 CPUs utilized 2542387 context-switches #0.052 M/sec 1 cpu-migrations#0.000 K/sec 213 page-faults #0.004 K/sec 177092791469 cycles#3.590 GHz (62.30%) 68555756017 instructions #0.39 insn per cycle (75.16%) 12697019382 branches # 257.418 M/sec (74.81%) 442081897 branch-misses #3.48% of all branches (74.79%) 20337958358 L1-dcache-loads # 412.330 M/sec (75.46%) 3820210140 L1-dcache-load-misses # 18.78% of all L1-dcache hits (75.35%) 1257719198 LLC-loads # 25.499 M/sec (49.73%) 685543923 LLC-load-misses # 54.51% of all LL-cache hits (49.86%) Changes from v2: - Reword commit message (Jakub) Changes from v1: - Add performance info (David) - Add misssing dma_sync_single_for_device() Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/common.h | 1 + drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 6 ++ .../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 18 +- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 18 ++ drivers/net/ethernet/stmicro/stmmac/hwif.h | 9 +++ drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 75 +- 7 files changed, 128 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index e1e6f67041ec..527f961579f4 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -356,6 +356,7 @@ struct dma_features { unsigned int addr64; unsigned int rssen; unsigned int vlhash; + unsigned int sphen; }; /* GMAC TX FIFO is 8K, Rx FIFO is 16K */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 429c94e40c73..995d533b9316 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -32,6 +32,9 @@ #define XGMAC_CONFIG_ARPEN BIT(31) #define XGMAC_CONFIG_GPSL GENMASK(29, 16) #define XGMAC_CONFIG_GPSL_SHIFT16 +#define XGMAC_CONFIG_HDSMS GENMASK(14, 12) +#define XGMAC_CONFIG_HDSMS_SHIFT 12 +#define XGMAC_CONFIG_HDSMS_256 (0x2 << XGMAC_CONFIG_HDSMS_SHIFT) #define XGMAC_CONFIG_S2KP BIT(11) #define XGMAC_CONFIG_LMBIT(10) #define XGMAC_CONFIG_IPC BIT(9) @@ -101,6 +104,7 @@ #define XGMAC_HW_FEATURE1 0x0120 #define XGMAC_HWFEAT_RSSEN BIT(20) #define XGMAC_HWFEAT_TSOEN BIT(18) +#define XGMAC_HWFEAT_SPHEN BIT(17) #define XGMAC_HWFEAT_ADDR64GENMASK(15, 14) #define XGMAC_HWFEAT_TXFIFOSIZEGENMASK(10, 6) #define XGMAC_HWFEAT_RXFIFOSIZEGENMASK(4, 0) @@ -258,6 +262,7 @@ #define XGMAC_TCEIEBIT(0) #define XGMAC_DMA_ECC_INT_STATUS 0x306c #define XGMAC_DMA_CH_CONTROL(x)(0x3100 + (0x80 * (x))) +#define XGMAC_SPH BIT(24) #define XGMAC_PBLx8BIT(16) #define XGMAC_DMA_CH_TX_CONTROL(x) (0x3104 + (0x80 * (x))) #define XGMAC_TxPBLGENMASK(21, 16) @@ -318,6 +323,7 @@ #define XGMAC_TDES3_CIC_SHIFT 16 #define XGMAC_TDES3_TPLGENMASK(17, 0) #define X
[PATCH net-next v3 06/12] net: stmmac: dwxgmac: Add Flexible PPS support
Add the support for Flexible PPS in XGMAC cores. Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 19 .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c| 56 ++ 2 files changed, 75 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 995d533b9316..dbac63972faf 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -149,6 +149,25 @@ #define XGMAC_TXTIMESTAMP_NSEC 0x0d30 #define XGMAC_TXTSSTSLOGENMASK(30, 0) #define XGMAC_TXTIMESTAMP_SEC 0x0d34 +#define XGMAC_PPS_CONTROL 0x0d70 +#define XGMAC_PPS_MAXIDX(x)x) + 1) * 8) - 1) +#define XGMAC_PPS_MINIDX(x)((x) * 8) +#define XGMAC_PPSx_MASK(x) \ + GENMASK(XGMAC_PPS_MAXIDX(x), XGMAC_PPS_MINIDX(x)) +#define XGMAC_TRGTMODSELx(x, val) \ + GENMASK(XGMAC_PPS_MAXIDX(x) - 1, XGMAC_PPS_MAXIDX(x) - 2) & \ + ((val) << (XGMAC_PPS_MAXIDX(x) - 2)) +#define XGMAC_PPSCMDx(x, val) \ + GENMASK(XGMAC_PPS_MINIDX(x) + 3, XGMAC_PPS_MINIDX(x)) & \ + ((val) << XGMAC_PPS_MINIDX(x)) +#define XGMAC_PPSCMD_START 0x2 +#define XGMAC_PPSCMD_STOP 0x5 +#define XGMAC_PPSEN0 BIT(4) +#define XGMAC_PPSx_TARGET_TIME_SEC(x) (0x0d80 + (x) * 0x10) +#define XGMAC_PPSx_TARGET_TIME_NSEC(x) (0x0d84 + (x) * 0x10) +#define XGMAC_TRGTBUSY0BIT(31) +#define XGMAC_PPSx_INTERVAL(x) (0x0d88 + (x) * 0x10) +#define XGMAC_PPSx_WIDTH(x)(0x0d8c + (x) * 0x10) /* MTL Registers */ #define XGMAC_MTL_OPMODE 0x1000 diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index ba5183f38f84..f843e3640f50 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -8,6 +8,7 @@ #include #include #include "stmmac.h" +#include "stmmac_ptp.h" #include "dwxgmac2.h" static void dwxgmac2_core_init(struct mac_device_info *hw, @@ -1011,6 +1012,60 @@ static int dwxgmac2_get_mac_tx_timestamp(struct mac_device_info *hw, u64 *ts) return 0; } +static int dwxgmac2_flex_pps_config(void __iomem *ioaddr, int index, + struct stmmac_pps_cfg *cfg, bool enable, + u32 sub_second_inc, u32 systime_flags) +{ + u32 tnsec = readl(ioaddr + XGMAC_PPSx_TARGET_TIME_NSEC(index)); + u32 val = readl(ioaddr + XGMAC_PPS_CONTROL); + u64 period; + + if (!cfg->available) + return -EINVAL; + if (tnsec & XGMAC_TRGTBUSY0) + return -EBUSY; + if (!sub_second_inc || !systime_flags) + return -EINVAL; + + val &= ~XGMAC_PPSx_MASK(index); + + if (!enable) { + val |= XGMAC_PPSCMDx(index, XGMAC_PPSCMD_STOP); + writel(val, ioaddr + XGMAC_PPS_CONTROL); + return 0; + } + + val |= XGMAC_PPSCMDx(index, XGMAC_PPSCMD_START); + val |= XGMAC_TRGTMODSELx(index, XGMAC_PPSCMD_START); + val |= XGMAC_PPSEN0; + + writel(cfg->start.tv_sec, ioaddr + XGMAC_PPSx_TARGET_TIME_SEC(index)); + + if (!(systime_flags & PTP_TCR_TSCTRLSSR)) + cfg->start.tv_nsec = (cfg->start.tv_nsec * 1000) / 465; + writel(cfg->start.tv_nsec, ioaddr + XGMAC_PPSx_TARGET_TIME_NSEC(index)); + + period = cfg->period.tv_sec * 10; + period += cfg->period.tv_nsec; + + do_div(period, sub_second_inc); + + if (period <= 1) + return -EINVAL; + + writel(period - 1, ioaddr + XGMAC_PPSx_INTERVAL(index)); + + period >>= 1; + if (period <= 1) + return -EINVAL; + + writel(period - 1, ioaddr + XGMAC_PPSx_WIDTH(index)); + + /* Finally, activate it */ + writel(val, ioaddr + XGMAC_PPS_CONTROL); + return 0; +} + const struct stmmac_ops dwxgmac210_ops = { .core_init = dwxgmac2_core_init, .set_mac = dwxgmac2_set_mac, @@ -1048,6 +1103,7 @@ const struct stmmac_ops dwxgmac210_ops = { .update_vlan_hash = dwxgmac2_update_vlan_hash, .rxp_config = dwxgmac3_rxp_config, .get_mac_tx_timestamp = dwxgmac2_get_mac_tx_timestamp, + .flex_pps_config = dwxgmac2_flex_pps_config, }; int dwxgmac2_setup(struct stmmac_priv *priv) -- 2.7.4
[PATCH net-next v3 05/12] net: stmmac: Add a counter for Split Header packets
Add a counter that increments each time a packet with split header is received. Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/common.h | 1 + drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 1 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c| 1 + 3 files changed, 3 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 527f961579f4..1303ec81fd3d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -75,6 +75,7 @@ struct stmmac_extra_stats { unsigned long rx_missed_cntr; unsigned long rx_overflow_cntr; unsigned long rx_vlan; + unsigned long rx_split_hdr_pkt_n; /* Tx/Rx IRQ error info */ unsigned long tx_undeflow_irq; unsigned long tx_process_stopped_irq; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index 2423160ab582..eb784fdb6d32 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -65,6 +65,7 @@ static const struct stmmac_stats stmmac_gstrings_stats[] = { STMMAC_STAT(rx_missed_cntr), STMMAC_STAT(rx_overflow_cntr), STMMAC_STAT(rx_vlan), + STMMAC_STAT(rx_split_hdr_pkt_n), /* Tx/Rx IRQ error info */ STMMAC_STAT(tx_undeflow_irq), STMMAC_STAT(tx_process_stopped_irq), diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 60e5f3584790..f2a198eda20b 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3503,6 +3503,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) len = hlen; prefetch(page_address(buf->sec_page)); + priv->xstats.rx_split_hdr_pkt_n++; } skb = napi_alloc_skb(&ch->rx_napi, len); -- 2.7.4
[PATCH net-next v3 08/12] net: stmmac: Add support for SA Insertion/Replacement in XGMAC cores
Add the support for Source Address Insertion and Replacement in XGMAC cores. Two methods are supported: Descriptor based and register based. Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 ++ drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 11 +++ drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 10 +- drivers/net/ethernet/stmicro/stmmac/hwif.h | 7 +++ drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c| 6 ++ 6 files changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 7fed3d2d4a95..3fb023953023 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -337,6 +337,8 @@ #define XGMAC_TDES3_CPCGENMASK(27, 26) #define XGMAC_TDES3_CPC_SHIFT 26 #define XGMAC_TDES3_TCMSSV BIT(26) +#define XGMAC_TDES3_SAIC GENMASK(25, 23) +#define XGMAC_TDES3_SAIC_SHIFT 23 #define XGMAC_TDES3_THLGENMASK(22, 19) #define XGMAC_TDES3_THL_SHIFT 19 #define XGMAC_TDES3_TSEBIT(18) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index a161285340c6..d0e7b62cc2ae 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -1075,6 +1075,16 @@ static int dwxgmac2_flex_pps_config(void __iomem *ioaddr, int index, return 0; } +static void dwxgmac2_sarc_configure(void __iomem *ioaddr, int val) +{ + u32 value = readl(ioaddr + XGMAC_TX_CONFIG); + + value &= ~XGMAC_CONFIG_SARC; + value |= val << XGMAC_CONFIG_SARC_SHIFT; + + writel(value, ioaddr + XGMAC_TX_CONFIG); +} + const struct stmmac_ops dwxgmac210_ops = { .core_init = dwxgmac2_core_init, .set_mac = dwxgmac2_set_mac, @@ -1113,6 +1123,7 @@ const struct stmmac_ops dwxgmac210_ops = { .rxp_config = dwxgmac3_rxp_config, .get_mac_tx_timestamp = dwxgmac2_get_mac_tx_timestamp, .flex_pps_config = dwxgmac2_flex_pps_config, + .sarc_configure = dwxgmac2_sarc_configure, }; int dwxgmac2_setup(struct stmmac_priv *priv) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c index 41985a2d7380..ab11e73ac6b1 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c @@ -148,7 +148,7 @@ static void dwxgmac2_prepare_tx_desc(struct dma_desc *p, int is_fs, int len, p->des2 |= cpu_to_le32(len & XGMAC_TDES2_B1L); - tdes3 = tot_pkt_len & XGMAC_TDES3_FL; + tdes3 |= tot_pkt_len & XGMAC_TDES3_FL; if (is_fs) tdes3 |= XGMAC_TDES3_FD; else @@ -298,6 +298,13 @@ static void dwxgmac2_set_sec_addr(struct dma_desc *p, dma_addr_t addr) p->des3 = cpu_to_le32(upper_32_bits(addr)); } +static void dwxgmac2_set_sarc(struct dma_desc *p, u32 sarc_type) +{ + sarc_type <<= XGMAC_TDES3_SAIC_SHIFT; + + p->des3 |= cpu_to_le32(sarc_type & XGMAC_TDES3_SAIC); +} + const struct stmmac_desc_ops dwxgmac210_desc_ops = { .tx_status = dwxgmac2_get_tx_status, .rx_status = dwxgmac2_get_rx_status, @@ -324,4 +331,5 @@ const struct stmmac_desc_ops dwxgmac210_desc_ops = { .get_rx_hash = dwxgmac2_get_rx_hash, .get_rx_header_len = dwxgmac2_get_rx_header_len, .set_sec_addr = dwxgmac2_set_sec_addr, + .set_sarc = dwxgmac2_set_sarc, }; diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index ed9fda50ee22..e54864cde01b 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -91,6 +91,7 @@ struct stmmac_desc_ops { enum pkt_hash_types *type); int (*get_rx_header_len)(struct dma_desc *p, unsigned int *len); void (*set_sec_addr)(struct dma_desc *p, dma_addr_t addr); + void (*set_sarc)(struct dma_desc *p, u32 sarc_type); }; #define stmmac_init_rx_desc(__priv, __args...) \ @@ -147,6 +148,8 @@ struct stmmac_desc_ops { stmmac_do_callback(__priv, desc, get_rx_header_len, __args) #define stmmac_set_desc_sec_addr(__priv, __args...) \ stmmac_do_void_callback(__priv, desc, set_sec_addr, __args) +#define stmmac_set_desc_sarc(__priv, __args...) \ + stmmac_do_void_callback(__priv, desc, set_sarc, __args) struct stmmac_dma_cfg;
[PATCH net-next v3 10/12] net: stmmac: xgmac: Add EEE support
Add support for EEE in XGMAC cores by implementing the necessary callbacks. Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 12 .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c| 75 -- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 1 + 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 3fb023953023..79c145ba25a8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -71,6 +71,7 @@ #define XGMAC_PSRQ(x) GENMASK((x) * 8 + 7, (x) * 8) #define XGMAC_PSRQ_SHIFT(x)((x) * 8) #define XGMAC_INT_STATUS 0x00b0 +#define XGMAC_LPIISBIT(5) #define XGMAC_PMTISBIT(4) #define XGMAC_INT_EN 0x00b4 #define XGMAC_TSIE BIT(12) @@ -88,10 +89,21 @@ #define XGMAC_RWKPKTEN BIT(2) #define XGMAC_MGKPKTEN BIT(1) #define XGMAC_PWRDWN BIT(0) +#define XGMAC_LPI_CTRL 0x00d0 +#define XGMAC_TXCGEBIT(21) +#define XGMAC_LPITXA BIT(19) +#define XGMAC_PLS BIT(17) +#define XGMAC_LPITXEN BIT(16) +#define XGMAC_RLPIEX BIT(3) +#define XGMAC_RLPIEN BIT(2) +#define XGMAC_TLPIEX BIT(1) +#define XGMAC_TLPIEN BIT(0) +#define XGMAC_LPI_TIMER_CTRL 0x00d4 #define XGMAC_HW_FEATURE0 0x011c #define XGMAC_HWFEAT_SAVLANINS BIT(27) #define XGMAC_HWFEAT_RXCOESEL BIT(16) #define XGMAC_HWFEAT_TXCOESEL BIT(14) +#define XGMAC_HWFEAT_EEESELBIT(13) #define XGMAC_HWFEAT_TSSEL BIT(12) #define XGMAC_HWFEAT_AVSEL BIT(11) #define XGMAC_HWFEAT_RAVSELBIT(10) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index d0e7b62cc2ae..d8483d088711 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -253,6 +253,7 @@ static int dwxgmac2_host_irq_status(struct mac_device_info *hw, { void __iomem *ioaddr = hw->pcsr; u32 stat, en; + int ret = 0; en = readl(ioaddr + XGMAC_INT_EN); stat = readl(ioaddr + XGMAC_INT_STATUS); @@ -264,7 +265,24 @@ static int dwxgmac2_host_irq_status(struct mac_device_info *hw, readl(ioaddr + XGMAC_PMT); } - return 0; + if (stat & XGMAC_LPIIS) { + u32 lpi = readl(ioaddr + XGMAC_LPI_CTRL); + + if (lpi & XGMAC_TLPIEN) { + ret |= CORE_IRQ_TX_PATH_IN_LPI_MODE; + x->irq_tx_path_in_lpi_mode_n++; + } + if (lpi & XGMAC_TLPIEX) { + ret |= CORE_IRQ_TX_PATH_EXIT_LPI_MODE; + x->irq_tx_path_exit_lpi_mode_n++; + } + if (lpi & XGMAC_RLPIEN) + x->irq_rx_path_in_lpi_mode_n++; + if (lpi & XGMAC_RLPIEX) + x->irq_rx_path_exit_lpi_mode_n++; + } + + return ret; } static int dwxgmac2_host_mtl_irq_status(struct mac_device_info *hw, u32 chan) @@ -357,6 +375,53 @@ static void dwxgmac2_get_umac_addr(struct mac_device_info *hw, addr[5] = (hi_addr >> 8) & 0xff; } +static void dwxgmac2_set_eee_mode(struct mac_device_info *hw, + bool en_tx_lpi_clockgating) +{ + void __iomem *ioaddr = hw->pcsr; + u32 value; + + value = readl(ioaddr + XGMAC_LPI_CTRL); + + value |= XGMAC_LPITXEN | XGMAC_LPITXA; + if (en_tx_lpi_clockgating) + value |= XGMAC_TXCGE; + + writel(value, ioaddr + XGMAC_LPI_CTRL); +} + +static void dwxgmac2_reset_eee_mode(struct mac_device_info *hw) +{ + void __iomem *ioaddr = hw->pcsr; + u32 value; + + value = readl(ioaddr + XGMAC_LPI_CTRL); + value &= ~(XGMAC_LPITXEN | XGMAC_LPITXA | XGMAC_TXCGE); + writel(value, ioaddr + XGMAC_LPI_CTRL); +} + +static void dwxgmac2_set_eee_pls(struct mac_device_info *hw, int link) +{ + void __iomem *ioaddr = hw->pcsr; + u32 value; + + value = readl(ioaddr + XGMAC_LPI_CTRL); + if (link) + value |= XGMAC_PLS; + else + value &= ~XGMAC_PLS; + writel(value, ioaddr + XGMAC_LPI_CTRL); +} + +static void dwxgmac2_set_eee_timer(struct mac_device_info *hw, int
[PATCH net-next v3 12/12] net: stmmac: selftests: Add selftest for VLAN TX Offload
Add 2 new selftests for VLAN Insertion offloading. Tests are for inner and outer VLAN offloading. Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- .../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 96 +- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c index acfab86431b1..ecc8602c6799 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c @@ -296,7 +296,9 @@ static int __stmmac_test_loopback(struct stmmac_priv *priv, tpriv->pt.dev = priv->dev; tpriv->pt.af_packet_priv = tpriv; tpriv->packet = attr; - dev_add_pack(&tpriv->pt); + + if (!attr->dont_wait) + dev_add_pack(&tpriv->pt); skb = stmmac_test_get_udp_skb(priv, attr); if (!skb) { @@ -319,7 +321,8 @@ static int __stmmac_test_loopback(struct stmmac_priv *priv, ret = !tpriv->ok; cleanup: - dev_remove_pack(&tpriv->pt); + if (!attr->dont_wait) + dev_remove_pack(&tpriv->pt); kfree(tpriv); return ret; } @@ -731,6 +734,9 @@ static int stmmac_test_vlan_validate(struct sk_buff *skb, struct ethhdr *ehdr; struct udphdr *uhdr; struct iphdr *ihdr; + u16 proto; + + proto = tpriv->double_vlan ? ETH_P_8021AD : ETH_P_8021Q; skb = skb_unshare(skb, GFP_ATOMIC); if (!skb) @@ -740,6 +746,12 @@ static int stmmac_test_vlan_validate(struct sk_buff *skb, goto out; if (skb_headlen(skb) < (STMMAC_TEST_PKT_SIZE - ETH_HLEN)) goto out; + if (tpriv->vlan_id) { + if (skb->vlan_proto != htons(proto)) + goto out; + if (skb->vlan_tci != tpriv->vlan_id) + goto out; + } ehdr = (struct ethhdr *)skb_mac_header(skb); if (!ether_addr_equal(ehdr->h_dest, tpriv->packet->dst)) @@ -1084,6 +1096,78 @@ static int stmmac_test_reg_sar(struct stmmac_priv *priv) return ret; } +static int stmmac_test_vlanoff_common(struct stmmac_priv *priv, bool svlan) +{ + struct stmmac_packet_attrs attr = { }; + struct stmmac_test_priv *tpriv; + struct sk_buff *skb = NULL; + int ret = 0; + u16 proto; + + if (!priv->dma_cap.vlins) + return -EOPNOTSUPP; + + tpriv = kzalloc(sizeof(*tpriv), GFP_KERNEL); + if (!tpriv) + return -ENOMEM; + + proto = svlan ? ETH_P_8021AD : ETH_P_8021Q; + + tpriv->ok = false; + tpriv->double_vlan = svlan; + init_completion(&tpriv->comp); + + tpriv->pt.type = svlan ? htons(ETH_P_8021Q) : htons(ETH_P_IP); + tpriv->pt.func = stmmac_test_vlan_validate; + tpriv->pt.dev = priv->dev; + tpriv->pt.af_packet_priv = tpriv; + tpriv->packet = &attr; + tpriv->vlan_id = 0x123; + dev_add_pack(&tpriv->pt); + + ret = vlan_vid_add(priv->dev, htons(proto), tpriv->vlan_id); + if (ret) + goto cleanup; + + attr.dst = priv->dev->dev_addr; + + skb = stmmac_test_get_udp_skb(priv, &attr); + if (!skb) { + ret = -ENOMEM; + goto vlan_del; + } + + __vlan_hwaccel_put_tag(skb, htons(proto), tpriv->vlan_id); + skb->protocol = htons(proto); + + skb_set_queue_mapping(skb, 0); + ret = dev_queue_xmit(skb); + if (ret) + goto vlan_del; + + wait_for_completion_timeout(&tpriv->comp, STMMAC_LB_TIMEOUT); + ret = tpriv->ok ? 0 : -ETIMEDOUT; + +vlan_del: + vlan_vid_del(priv->dev, htons(proto), tpriv->vlan_id); +cleanup: + dev_remove_pack(&tpriv->pt); + kfree(tpriv); + return ret; +} + +static int stmmac_test_vlanoff(struct stmmac_priv *priv) +{ + return stmmac_test_vlanoff_common(priv, false); +} + +static int stmmac_test_svlanoff(struct stmmac_priv *priv) +{ + if (!priv->dma_cap.dvlan) + return -EOPNOTSUPP; + return stmmac_test_vlanoff_common(priv, true); +} + #define STMMAC_LOOPBACK_NONE 0 #define STMMAC_LOOPBACK_MAC1 #define STMMAC_LOOPBACK_PHY2 @@ -1161,6 +1245,14 @@ static const struct stmmac_test { .name = "SA Replacement (reg)", .lb = STMMAC_LOOPBACK_PHY, .fn = stmmac_test_reg_sar, + }, { + .name = "VLAN TX Insertion ", + .lb = STMMAC_LOOPBACK_PHY, + .fn = stmmac_test_vlanoff, + }, { + .name = "SVLAN TX Insertion ", + .lb = STMMAC_LOOPBACK_PHY, + .fn = stmmac_test_svlanoff
[PATCH net-next v3 07/12] net: stmmac: Add ethtool register dump for XGMAC cores
Add the ethtool interface to dump the register map in XGMAC cores. Changes from v2: - Remove uneeded memset (Jakub) Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 ++ .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c| 11 ++- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 10 +- .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 23 +++--- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index dbac63972faf..7fed3d2d4a95 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -244,6 +244,7 @@ #define XGMAC_RXOVFIS BIT(16) #define XGMAC_ABPSIS BIT(1) #define XGMAC_TXUNFIS BIT(0) +#define XGMAC_MAC_REGSIZE (XGMAC_MTL_QINT_STATUS(15) / 4) /* DMA Registers */ #define XGMAC_DMA_MODE 0x3000 @@ -321,6 +322,7 @@ #define XGMAC_TBU BIT(2) #define XGMAC_TPS BIT(1) #define XGMAC_TI BIT(0) +#define XGMAC_REGSIZE ((0x317c + (0x80 * 15)) / 4) /* Descriptors */ #define XGMAC_TDES2_IOCBIT(31) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index f843e3640f50..a161285340c6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -239,6 +239,15 @@ static void dwxgmac2_config_cbs(struct mac_device_info *hw, writel(value, ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue)); } +static void dwxgmac2_dump_regs(struct mac_device_info *hw, u32 *reg_space) +{ + void __iomem *ioaddr = hw->pcsr; + int i; + + for (i = 0; i < XGMAC_MAC_REGSIZE; i++) + reg_space[i] = readl(ioaddr + i * 4); +} + static int dwxgmac2_host_irq_status(struct mac_device_info *hw, struct stmmac_extra_stats *x) { @@ -1079,7 +1088,7 @@ const struct stmmac_ops dwxgmac210_ops = { .set_mtl_tx_queue_weight = dwxgmac2_set_mtl_tx_queue_weight, .map_mtl_to_dma = dwxgmac2_map_mtl_to_dma, .config_cbs = dwxgmac2_config_cbs, - .dump_regs = NULL, + .dump_regs = dwxgmac2_dump_regs, .host_irq_status = dwxgmac2_host_irq_status, .host_mtl_irq_status = dwxgmac2_host_mtl_irq_status, .flow_ctrl = dwxgmac2_flow_ctrl, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c index 0f3de4895cf7..42c13d144203 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c @@ -128,6 +128,14 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi) writel(XGMAC_RDPS, ioaddr + XGMAC_RX_EDMA_CTRL); } +static void dwxgmac2_dma_dump_regs(void __iomem *ioaddr, u32 *reg_space) +{ + int i; + + for (i = (XGMAC_DMA_MODE / 4); i < XGMAC_REGSIZE; i++) + reg_space[i] = readl(ioaddr + i * 4); +} + static void dwxgmac2_dma_rx_mode(void __iomem *ioaddr, int mode, u32 channel, int fifosz, u8 qmode) { @@ -496,7 +504,7 @@ const struct stmmac_dma_ops dwxgmac210_dma_ops = { .init_rx_chan = dwxgmac2_dma_init_rx_chan, .init_tx_chan = dwxgmac2_dma_init_tx_chan, .axi = dwxgmac2_dma_axi, - .dump_regs = NULL, + .dump_regs = dwxgmac2_dma_dump_regs, .dma_rx_mode = dwxgmac2_dma_rx_mode, .dma_tx_mode = dwxgmac2_dma_tx_mode, .enable_dma_irq = dwxgmac2_enable_dma_irq, diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index eb784fdb6d32..1c450105e5a6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -18,10 +18,12 @@ #include "stmmac.h" #include "dwmac_dma.h" +#include "dwxgmac2.h" #define REG_SPACE_SIZE 0x1060 #define MAC100_ETHTOOL_NAME"st_mac100" #define GMAC_ETHTOOL_NAME "st_gmac" +#define XGMAC_ETHTOOL_NAME "st_xgmac" #define ETHTOOL_DMA_OFFSET 55 @@ -260,6 +262,8 @@ static void stmmac_ethtool_getdrvinfo(struct net_device *dev, if (priv->plat->has_gmac || priv->plat->has_gmac4) strlcpy(info->driver, GMAC_ETHTOOL_NAME, sizeof(info->driver)); + else if (priv->plat->has_xgmac) + strlcpy(info->driver, XGMAC_ETHTOOL_NAME, sizeof(info->driver));
[PATCH net-next v3 01/12] net: stmmac: Get correct timestamp values from XGMAC
TX Timestamp in XGMAC comes from MAC instead of descriptors. Implement this in a new callback. Also, RX Timestamp in XGMAC must be cheked against corruption and we need a barrier to make sure that descriptor fields are read correctly. Changes from v2: - Rework return code check (Jakub) Changes from v1: - Rework the get timestamp function (David) Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 15 +++ drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 15 +-- drivers/net/ethernet/stmicro/stmmac/hwif.h | 4 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c| 9 ++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index 767f3fe5efaa..ba5183f38f84 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -997,6 +997,20 @@ static int dwxgmac3_rxp_config(void __iomem *ioaddr, return ret; } +static int dwxgmac2_get_mac_tx_timestamp(struct mac_device_info *hw, u64 *ts) +{ + void __iomem *ioaddr = hw->pcsr; + u32 value; + + if (readl_poll_timeout_atomic(ioaddr + XGMAC_TIMESTAMP_STATUS, + value, value & XGMAC_TXTSC, 100, 1)) + return -EBUSY; + + *ts = readl(ioaddr + XGMAC_TXTIMESTAMP_NSEC) & XGMAC_TXTSSTSLO; + *ts += readl(ioaddr + XGMAC_TXTIMESTAMP_SEC) * 10ULL; + return 0; +} + const struct stmmac_ops dwxgmac210_ops = { .core_init = dwxgmac2_core_init, .set_mac = dwxgmac2_set_mac, @@ -1033,6 +1047,7 @@ const struct stmmac_ops dwxgmac210_ops = { .rss_configure = dwxgmac2_rss_configure, .update_vlan_hash = dwxgmac2_update_vlan_hash, .rxp_config = dwxgmac3_rxp_config, + .get_mac_tx_timestamp = dwxgmac2_get_mac_tx_timestamp, }; int dwxgmac2_setup(struct stmmac_priv *priv) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c index 8c5dd6a36157..58b69fa97837 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c @@ -98,11 +98,17 @@ static int dwxgmac2_rx_check_timestamp(void *desc) unsigned int rdes3 = le32_to_cpu(p->des3); bool desc_valid, ts_valid; + dma_rmb(); + desc_valid = !(rdes3 & XGMAC_RDES3_OWN) && (rdes3 & XGMAC_RDES3_CTXT); ts_valid = !(rdes3 & XGMAC_RDES3_TSD) && (rdes3 & XGMAC_RDES3_TSA); - if (likely(desc_valid && ts_valid)) + if (likely(desc_valid && ts_valid)) { + if ((p->des0 == 0x) && (p->des1 == 0x)) + return -EINVAL; return 0; + } + return -EINVAL; } @@ -113,13 +119,10 @@ static int dwxgmac2_get_rx_timestamp_status(void *desc, void *next_desc, unsigned int rdes3 = le32_to_cpu(p->des3); int ret = -EBUSY; - if (likely(rdes3 & XGMAC_RDES3_CDA)) { + if (likely(rdes3 & XGMAC_RDES3_CDA)) ret = dwxgmac2_rx_check_timestamp(next_desc); - if (ret) - return ret; - } - return ret; + return !ret; } static void dwxgmac2_init_rx_desc(struct dma_desc *p, int disable_rx_ic, diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index 52fc2344b066..7e1523c6f456 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -339,6 +339,8 @@ struct stmmac_ops { /* VLAN */ void (*update_vlan_hash)(struct mac_device_info *hw, u32 hash, bool is_double); + /* TX Timestamp */ + int (*get_mac_tx_timestamp)(struct mac_device_info *hw, u64 *ts); }; #define stmmac_core_init(__priv, __args...) \ @@ -413,6 +415,8 @@ struct stmmac_ops { stmmac_do_callback(__priv, mac, rss_configure, __args) #define stmmac_update_vlan_hash(__priv, __args...) \ stmmac_do_void_callback(__priv, mac, update_vlan_hash, __args) +#define stmmac_get_mac_tx_timestamp(__priv, __args...) \ + stmmac_do_callback(__priv, mac, get_mac_tx_timestamp, __args) /* PTP and HW Timer helpers */ struct stmmac_hwtimestamp { diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 06a63df1c2c5..b2e5f4ecd551 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -432,6 +432,7 @@ stat
[PATCH net-next v3 02/12] net: stmmac: Prepare to add Split Header support
In order to add Split Header support, stmmac_rx() needs to take into account that packet may be split accross multiple descriptors. Refactor the logic of this function in order to support this scenario. Changes from v2: - Fixup if condition detection (Jakub) - Don't stop NAPI with unfinished packet (Jakub) - Use napi_alloc_skb() (Jakub) Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 6 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 149 +- 2 files changed, 95 insertions(+), 60 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index 80276587048a..56158e1448ac 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -74,6 +74,12 @@ struct stmmac_rx_queue { u32 rx_zeroc_thresh; dma_addr_t dma_rx_phy; u32 rx_tail_addr; + unsigned int state_saved; + struct { + struct sk_buff *skb; + unsigned int len; + unsigned int error; + } state; }; struct stmmac_channel { diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index b2e5f4ecd551..05f0fa7a6f02 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3353,9 +3353,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) { struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue]; struct stmmac_channel *ch = &priv->channel[queue]; + unsigned int count = 0, error = 0, len = 0; + int status = 0, coe = priv->hw->rx_csum; unsigned int next_entry = rx_q->cur_rx; - int coe = priv->hw->rx_csum; - unsigned int count = 0; + struct sk_buff *skb = NULL; if (netif_msg_rx_status(priv)) { void *rx_head; @@ -3369,10 +3370,28 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) stmmac_display_ring(priv, rx_head, DMA_RX_SIZE, true); } while (count < limit) { + enum pkt_hash_types hash_type; struct stmmac_rx_buffer *buf; + unsigned int prev_len = 0; struct dma_desc *np, *p; - int entry, status; + int entry; + u32 hash; + if (!count && rx_q->state_saved) { + skb = rx_q->state.skb; + error = rx_q->state.error; + len = rx_q->state.len; + } else { + rx_q->state_saved = false; + skb = NULL; + error = 0; + len = 0; + } + + if (count >= limit) + break; + +read_again: entry = next_entry; buf = &rx_q->buf_pool[entry]; @@ -3407,28 +3426,24 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) page_pool_recycle_direct(rx_q->page_pool, buf->page); priv->dev->stats.rx_errors++; buf->page = NULL; + error = 1; + } + + if (unlikely(error && (status & rx_not_ls))) + goto read_again; + if (unlikely(error)) { + if (skb) + dev_kfree_skb(skb); + continue; + } + + /* Buffer is good. Go on. */ + + if (likely(status & rx_not_ls)) { + len += priv->dma_buf_sz; } else { - enum pkt_hash_types hash_type; - struct sk_buff *skb; - unsigned int des; - int frame_len; - u32 hash; - - stmmac_get_desc_addr(priv, p, &des); - frame_len = stmmac_get_rx_frame_len(priv, p, coe); - - /* If frame length is greater than skb buffer size -* (preallocated during init) then the packet is -* ignored -*/ - if (frame_len > priv->dma_buf_sz) { - if (net_ratelimit()) - netdev_err(priv->dev, - "len %d larger than size (%d)\n", - frame_len, priv->dma_buf_sz); - pr
[PATCH net-next v3 09/12] net: stmmac: selftests: Add tests for SA Insertion/Replacement
Add 4 new tests: - SA Insertion (register based) - SA Insertion (descriptor based) - SA Replacament (register based) - SA Replacement (descriptor based) Signed-off-by: Jose Abreu --- Cc: Giuseppe Cavallaro Cc: Alexandre Torgue Cc: Jose Abreu Cc: "David S. Miller" Cc: Maxime Coquelin Cc: net...@vger.kernel.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- .../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 98 +- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c index abab84f2ef8b..acfab86431b1 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c @@ -45,6 +45,7 @@ struct stmmac_packet_attrs { int size; int remove_sa; u8 id; + int sarc; }; static u8 stmmac_test_next_id; @@ -230,7 +231,10 @@ static int stmmac_test_loopback_validate(struct sk_buff *skb, if (!ether_addr_equal(ehdr->h_dest, tpriv->packet->dst)) goto out; } - if (tpriv->packet->src) { + if (tpriv->packet->sarc) { + if (!ether_addr_equal(ehdr->h_source, ehdr->h_dest)) + goto out; + } else if (tpriv->packet->src) { if (!ether_addr_equal(ehdr->h_source, tpriv->packet->src)) goto out; } @@ -1004,6 +1008,82 @@ static int stmmac_test_rxp(struct stmmac_priv *priv) } #endif +static int stmmac_test_desc_sai(struct stmmac_priv *priv) +{ + unsigned char src[ETH_ALEN] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + struct stmmac_packet_attrs attr = { }; + int ret; + + attr.remove_sa = true; + attr.sarc = true; + attr.src = src; + attr.dst = priv->dev->dev_addr; + + priv->sarc_type = 0x1; + + ret = __stmmac_test_loopback(priv, &attr); + + priv->sarc_type = 0x0; + return ret; +} + +static int stmmac_test_desc_sar(struct stmmac_priv *priv) +{ + unsigned char src[ETH_ALEN] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + struct stmmac_packet_attrs attr = { }; + int ret; + + attr.sarc = true; + attr.src = src; + attr.dst = priv->dev->dev_addr; + + priv->sarc_type = 0x2; + + ret = __stmmac_test_loopback(priv, &attr); + + priv->sarc_type = 0x0; + return ret; +} + +static int stmmac_test_reg_sai(struct stmmac_priv *priv) +{ + unsigned char src[ETH_ALEN] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + struct stmmac_packet_attrs attr = { }; + int ret; + + attr.remove_sa = true; + attr.sarc = true; + attr.src = src; + attr.dst = priv->dev->dev_addr; + + if (stmmac_sarc_configure(priv, priv->ioaddr, 0x2)) + return -EOPNOTSUPP; + + ret = __stmmac_test_loopback(priv, &attr); + + stmmac_sarc_configure(priv, priv->ioaddr, 0x0); + return ret; +} + +static int stmmac_test_reg_sar(struct stmmac_priv *priv) +{ + unsigned char src[ETH_ALEN] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + struct stmmac_packet_attrs attr = { }; + int ret; + + attr.sarc = true; + attr.src = src; + attr.dst = priv->dev->dev_addr; + + if (stmmac_sarc_configure(priv, priv->ioaddr, 0x3)) + return -EOPNOTSUPP; + + ret = __stmmac_test_loopback(priv, &attr); + + stmmac_sarc_configure(priv, priv->ioaddr, 0x0); + return ret; +} + #define STMMAC_LOOPBACK_NONE 0 #define STMMAC_LOOPBACK_MAC1 #define STMMAC_LOOPBACK_PHY2 @@ -1065,6 +1145,22 @@ static const struct stmmac_test { .name = "Flexible RX Parser ", .lb = STMMAC_LOOPBACK_PHY, .fn = stmmac_test_rxp, + }, { + .name = "SA Insertion (desc) ", + .lb = STMMAC_LOOPBACK_PHY, + .fn = stmmac_test_desc_sai, + }, { + .name = "SA Replacement (desc)", + .lb = STMMAC_LOOPBACK_PHY, + .fn = stmmac_test_desc_sar, + }, { + .name = "SA Insertion (reg) ", + .lb = STMMAC_LOOPBACK_PHY, + .fn = stmmac_test_reg_sai, + }, { + .name = "SA Replacement (reg)", + .lb = STMMAC_LOOPBACK_PHY, + .fn = stmmac_test_reg_sar, }, }; -- 2.7.4
Re: [PATCH] Partially revert "mm/memcontrol.c: keep local VM counters in sync with the hierarchical ones"
On Sat, Aug 17, 2019 at 11:33:57AM +0800, Yafang Shao wrote: > On Sat, Aug 17, 2019 at 8:47 AM Roman Gushchin wrote: > > > > Commit 766a4c19d880 ("mm/memcontrol.c: keep local VM counters in sync > > with the hierarchical ones") effectively decreased the precision of > > per-memcg vmstats_local and per-memcg-per-node lruvec percpu counters. > > > > That's good for displaying in memory.stat, but brings a serious regression > > into the reclaim process. > > > > One issue I've discovered and debugged is the following: > > lruvec_lru_size() can return 0 instead of the actual number of pages > > in the lru list, preventing the kernel to reclaim last remaining > > pages. Result is yet another dying memory cgroups flooding. > > The opposite is also happening: scanning an empty lru list > > is the waste of cpu time. > > > > Also, inactive_list_is_low() can return incorrect values, preventing > > the active lru from being scanned and freed. It can fail both because > > the size of active and inactive lists are inaccurate, and because > > the number of workingset refaults isn't precise. In other words, > > the result is pretty random. > > > > I'm not sure, if using the approximate number of slab pages in > > count_shadow_number() is acceptable, but issues described above > > are enough to partially revert the patch. > > > > Let's keep per-memcg vmstat_local batched (they are only used for > > displaying stats to the userspace), but keep lruvec stats precise. > > This change fixes the dead memcg flooding on my setup. > > > > That will make some misunderstanding if the local counters are not in > sync with the hierarchical ones > (someone may doubt whether there're something leaked.). Sure, but the actual leakage is a much more serious issue. > If we have to do it like this, I think we should better document this > behavior. Lru size calculations can be done using per-zone counters, which is actually cheaper, because the number of zones is usually smaller than the number of cpus. I'll send a corresponding patch on Monday. Maybe other use cases can also be converted? Thanks! > > > Fixes: 766a4c19d880 ("mm/memcontrol.c: keep local VM counters in sync with > > the hierarchical ones") > > Signed-off-by: Roman Gushchin > > Cc: Yafang Shao > > Cc: Johannes Weiner > > --- > > mm/memcontrol.c | 8 +++- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 249187907339..3429340adb56 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -746,15 +746,13 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum > > node_stat_item idx, > > /* Update memcg */ > > __mod_memcg_state(memcg, idx, val); > > > > + /* Update lruvec */ > > + __this_cpu_add(pn->lruvec_stat_local->count[idx], val); > > + > > x = val + __this_cpu_read(pn->lruvec_stat_cpu->count[idx]); > > if (unlikely(abs(x) > MEMCG_CHARGE_BATCH)) { > > struct mem_cgroup_per_node *pi; > > > > - /* > > -* Batch local counters to keep them in sync with > > -* the hierarchical ones. > > -*/ > > - __this_cpu_add(pn->lruvec_stat_local->count[idx], x); > > for (pi = pn; pi; pi = parent_nodeinfo(pi, pgdat->node_id)) > > atomic_long_add(x, &pi->lruvec_stat[idx]); > > x = 0; > > -- > > 2.21.0 > >
Re: [PATCH] Partially revert "mm/memcontrol.c: keep local VM counters in sync with the hierarchical ones"
On Sat, Aug 17, 2019 at 08:36:16AM +0200, Greg KH wrote: > On Fri, Aug 16, 2019 at 05:47:26PM -0700, Roman Gushchin wrote: > > Commit 766a4c19d880 ("mm/memcontrol.c: keep local VM counters in sync > > with the hierarchical ones") effectively decreased the precision of > > per-memcg vmstats_local and per-memcg-per-node lruvec percpu counters. > > > > That's good for displaying in memory.stat, but brings a serious regression > > into the reclaim process. > > > > One issue I've discovered and debugged is the following: > > lruvec_lru_size() can return 0 instead of the actual number of pages > > in the lru list, preventing the kernel to reclaim last remaining > > pages. Result is yet another dying memory cgroups flooding. > > The opposite is also happening: scanning an empty lru list > > is the waste of cpu time. > > > > Also, inactive_list_is_low() can return incorrect values, preventing > > the active lru from being scanned and freed. It can fail both because > > the size of active and inactive lists are inaccurate, and because > > the number of workingset refaults isn't precise. In other words, > > the result is pretty random. > > > > I'm not sure, if using the approximate number of slab pages in > > count_shadow_number() is acceptable, but issues described above > > are enough to partially revert the patch. > > > > Let's keep per-memcg vmstat_local batched (they are only used for > > displaying stats to the userspace), but keep lruvec stats precise. > > This change fixes the dead memcg flooding on my setup. > > > > Fixes: 766a4c19d880 ("mm/memcontrol.c: keep local VM counters in sync with > > the hierarchical ones") > > Signed-off-by: Roman Gushchin > > Cc: Yafang Shao > > Cc: Johannes Weiner > > --- > > mm/memcontrol.c | 8 +++- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > This is not the correct way to submit patches for inclusion in the > stable kernel tree. Please read: > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > for how to do this properly. Oh, I'm sorry, will read and follow next time. Thanks! > >
Re: [PATCH] FS: timerfd: Fix unexpected return value of timerfd_read function.
Arul, On Sat, 17 Aug 2019, Arul Jeniston wrote: > Do you agree the possibility of returning zero value from timerfd_read()? Obviosuly it happens. > > That has absolutely nothing to do with CLOCK_REALTIME. Your machines > > TSC is either going backwards or not synchronized between cores. > > > > Hint: Dell has a track record of BIOS doing the wrong things to TSC in > > order to hide their 'value add' features stealing CPU time. > > We haven't seen any issue in Dell hardware but we would definitely check > the possibility of hardware bug. BIOS is the more likely candidate. > Let us say, due to some reason the tsc goes backwards. Isn't it handled in > clocksource_delta(). No. clocksource_delta() does damage limitation. It prevents insane large time jumps which would result if the read out TSC value is less than the base value which is used to calculate the delta. It cannot do more than that. > Is timerfd_read expected to return 0 if tsc drifts backwards? If so, why it > is not documented? > Being a system call, we expect all return values of read() on timerfd to be > documented in the man page. We expect TSC not to go backwards. If it does it's broken and not usable as a clocksource for the kernel. The problem is that this is not necessarily easy to detect. Fact is, that your machines TSC goes backwards or is not properly synchronized between the cores. Otherwise the problem would not exist at all. That's the problem which needs to be fixed and not papered over with crude hacks and weird justifications. > > ktime_get() is CLOCK_MONOTONIC and not CLOCK_REALTIME. > > We see the same base used for CLOCK_MONOTONIC, CLOCK_REALTIME timers here. > both MONOTONIC, REALTIME timers hits the following code flow. we confirmed > it through instrumentation. > timerfd_read()-->hrtimer_forward_now()-->ktime_get()-->timekeeping_get_ns()-->timekeeping_get_delta()-->clocksource_delta(). > Do you want me to share any other logs to confirm it? No. That's the case when you use a relative timer with CLOCK_REALTIME because only absolute timers are affected by modifications of CLOCK_REALTIME. So it's NOT an issue of a CLOCK_REALTIME modification via settimeofday() or adjtimex(). > > It's a bug, but either a hardware or a BIOS bug and you are trying to > > paper over it at the place where you observe the symptom, which is > > obviously the wrong place because: > > > > 1) Any other time related function even in timerfd is affected as well > > > > 2) We do not cure symptoms, we cure the root cause. And clearly the root > > cause hase not been explained and addressed. > > if we don't fix this in kernel, can we document this return value in > timerfd read() man page? Again: You cannot fix a hardware problem by hacking around it at exactly one place where you can observe it. If that problem exists on your machine, then any other time related function is affected as well. Are you going to submit patches against _ALL_ time{r} related syscalls to fix^Wpaper over this? Either against the kernel or against the man pages? As this is a 4 core Rangely, it has a properly synchronized TSC on all 4 cores which runs with constant frequency and is not affected by deeper C-States. Here is the flow: timerfd_read() waitfortick() timer interrupt() time = ktime_get() expire timer time >= timer_time tick++; wakeup_reader() hrtimer_forward_now() now = ktime_get() In the failure case now < timer_time i.e. time went backwards since the timer was expired. That's absolutely unexpected behaviour and no, we are not papering over it. Did you ever quantify how much time goes backwards in that case? Is the timer expiry and the timerfd_read() on the same CPU or on different ones? Can you please provide a full dmesg from boot to after the point where this failure happens? Thanks, tglx
Re: [PATCH 07/15] dt: thermal: tsens: Document interrupt support in tsens driver
On Fri, Aug 16, 2019 at 5:02 PM Amit Kucheria wrote: > > On Sat, Aug 17, 2019 at 3:06 AM Rob Herring wrote: > > > > On Fri, Jul 26, 2019 at 03:48:42AM +0530, Amit Kucheria wrote: > > > Define two new required properties to define interrupts and > > > interrupt-names for tsens. > > > > > > Signed-off-by: Amit Kucheria > > > --- > > > Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > > > b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > > > index 673cc1831ee9..3d3dd5dc6d36 100644 > > > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > > > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > > > @@ -22,6 +22,8 @@ Required properties: > > > > > > - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a > > > description. > > > - #qcom,sensors: Number of sensors in tsens block > > > +- interrupts: Interrupts generated from Always-On subsystem (AOSS) > > > +- interrupt-names: The name of the interrupt e.g. "tsens0", "tsens1" > > > > How many interrupts? A name with just indices isn't too useful. > > Depending on the version of the tsens IP, there can be 1 (upper/lower > threshold), 2 (upper/lower + critical threshold) or 3 (upper/lower + > critical + zero degree) interrupts. This patch series only introduces > support for a single interrupt (upper/lower). I would expect a different compatible for each possibility. > I used the names tsens0, tsens1 to encapsulate the controller instance > since some SoCs have 1 controller, others have two. So we'll end up > with something like the following in DT: That's not really how *-names is supposed to work. The name is for identifying what is at each index. Or to put it another way, a driver should be able to use platform_get_irq_by_name(). So 'critical', 'zero' and something for the first one. > tsens0: thermal-sensor@c263000 { > compatible = "qcom,sdm845-tsens", "qcom,tsens-v2"; > reg = <0 0x0c263000 0 0x1ff>, /* TM */ > <0 0x0c222000 0 0x1ff>; /* SROT */ > #qcom,sensors = <13>; > interrupts = , > ; > interrupt-names = "tsens0", "tsens0-critical"; > #thermal-sensor-cells = <1>; > }; > > tsens1: thermal-sensor@c265000 { > compatible = "qcom,sdm845-tsens", "qcom,tsens-v2"; > reg = <0 0x0c265000 0 0x1ff>, /* TM */ > <0 0x0c223000 0 0x1ff>; /* SROT */ > #qcom,sensors = <8>; > interrupts = , > ; > interrupt-names = "tsens1", "tsens1-critical"; > #thermal-sensor-cells = <1>; > } > > Does that work? > > Regards, > Amit > > > > - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how > > > to specify > > > nvmem cells > > > > > > @@ -40,6 +42,9 @@ tsens0: thermal-sensor@c263000 { > > > reg = <0xc263000 0x1ff>, /* TM */ > > > <0xc222000 0x1ff>; /* SROT */ > > > #qcom,sensors = <13>; > > > + interrupts = ; > > > + interrupt-names = "tsens0"; > > > + > > > #thermal-sensor-cells = <1>; > > > }; > > > > > > -- > > > 2.17.1 > > >
Re: [PATCH net-next v3 00/12] net: stmmac: Improvements for -next
From: Jose Abreu Date: Sat, 17 Aug 2019 20:54:39 +0200 > Couple of improvements for -next tree. More info in commit logs. Series applied.
[GIT PULL] Hyper-V fixes for v5.3-rc
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b: Linus 5.3-rc1 (2019-07-21 14:05:38 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed for you to fetch changes up to bafe1e79e05de725e26b3f60c90b49e635b686b9: MAINTAINERS: Fix Hyperv vIOMMU driver file name (2019-08-17 15:29:39 -0400) - - - A few fixes for the userspace hyper-v tools from Adrian Vladu. - - A fix for the hyper-v MAINTAINERs entry from Lan Tianyu. - - Fix for SPDX license identifier in the userspace tools from Nishad Kamdar. - Adrian Vladu (3): tools: hv: fixed Python pep8/flake8 warnings for lsvmbus tools: hv: fix KVP and VSS daemons exit code tools: hv: fix typos in toolchain Lan Tianyu (1): MAINTAINERS: Fix Hyperv vIOMMU driver file name Nishad Kamdar (1): tools: hv: Use the correct style for SPDX License Identifier MAINTAINERS | 2 +- drivers/hv/hv_trace.h| 2 +- tools/hv/hv_get_dhcp_info.sh | 2 +- tools/hv/hv_kvp_daemon.c | 8 +++-- tools/hv/hv_set_ifconfig.sh | 2 +- tools/hv/hv_vss_daemon.c | 4 ++- tools/hv/lsvmbus | 75 +--- 7 files changed, 54 insertions(+), 41 deletions(-) -BEGIN PGP SIGNATURE- iQIzBAEBCgAdFiEE4n5dijQDou9mhzu83qZv95d3LNwFAl1YXDEACgkQ3qZv95d3 LNwzQxAAlJgFE5AQ0etqM4ns/wx05AOePVHR90ua+FCU0W3umL5vGeYxCbl9dJ08 zYUhnoq/y4nbccIH7edJlrxb/J9+Slsp0FxWBbPSGjvbLK0yjaxHup8bcdyq6/pP UM/fvaPzd7NnK/LPehDlV1l2skqbsimm2wbv7P1sXYZ8aQwowXxJkeVeKvfipiCw MAB1KMCZGJg1n9w6xi2j+wnV4cuRgcMX/n+0C5Qc2AfFAVPrPzEJoGiRJBhQgqf0 JDg1+hWKrAyTAJvKHQf8o/8EsvLr/Itm1t9Q+s3eQUFqbvVPilbFR8OltSgPHgUM PJG+Kur49jjOpUR1OJ8MeRQXqblRqSxpe7POsjP1vnGTCvuO/sLSHiCDT7+3YIEN +bXaONOTCQSH5j8KgNE6MN/wfGIpoSEgMJEoN16OyELsQa2zIhRCwHdz72DkojJq QzrGkChLwAz8Dw3Ul/NX36MJMAyT9DTM3GE1IXY4LzOL0381JRbvms1vwP2dIkL4 c3+tGJ7iBi23KEfkBdD3fq2JBs3KVCucIMOZX9RWDqbRhl61015GKPSALAD7r9vR BKYo3hdaDxk7DpSTHTupUds/EmSnkS+7poaXl2iY2jVJTUXXIyg7Ig3IBI7Vqhc9 3SgunAaaKdgrh60JxkVTJ7WQUAPbOf3h/a/P1WAhSrpkxHB0XXI= =BevR -END PGP SIGNATURE-
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
Apologies to Steve for continuing this thread when all he wanted was moving an operation inside a mutex... On 17/08/2019 16:02, Mathieu Desnoyers wrote: [...] > However, if the state of "x" can be any pointer value, or a reference > count value, then not using "WRITE_ONCE()" to store a constant leaves > the compiler free to perform that store in more than one memory access. > Based on [1], section "Store tearing", there are situations where this > happens on x86 in the wild today when storing 64-bit constants: the > compiler is then free to decide to use two 32-bit immediate store > instructions. > That's also how I understand things, and it's also one of the points raised in the compiler barrier section of memory-barriers.txt Taking this store tearing, or the invented stores - e.g. the branch optimization pointed out by Linus: >if (a) > global_var = 1 >else > global_var = 0 > > then the compiler had better not turn that into > > global_var = 0 > if (a) > global_var = 1 AFAICT nothing prevents this from happening inside a critical section (where the locking primitives provide the right barriers, but that's it). That's all fine when data is never accessed locklessly, but in the case of locked writes vs lockless reads, couldn't there be "leaks" of these transient states? In those cases we would want WRITE_ONCE() for the writes. So going back to: > But the reverse is not really true. All a READ_ONCE() says is "I want > either the old or the new value", and it can get that _without_ being > paired with a WRITE_ONCE(). AFAIU it's not always the case, since a lone READ_ONCE() could get transient values. I'll be honest, it's not 100% clear to me when those optimizations can actually be done (maybe the branch thingy but the others are dubious), and it's even less clear when compilers *actually* do it - only that they have been reported to do it (so it's not made up). > Thanks, > > Mathieu > > [1] https://lwn.net/Articles/793253/ >
Re: sched: Unexpected reschedule of offline CPU#2!
On Fri, 16 Aug 2019, Guenter Roeck wrote: > On Fri, Aug 16, 2019 at 12:22:22PM +0200, Thomas Gleixner wrote: > > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c > > index 75fea0d48c0e..625627b1457c 100644 > > --- a/arch/x86/kernel/process.c > > +++ b/arch/x86/kernel/process.c > > @@ -601,6 +601,7 @@ void stop_this_cpu(void *dummy) > > /* > > * Remove this CPU: > > */ > > + set_cpu_active(smp_processor_id(), false); > > set_cpu_online(smp_processor_id(), false); > > disable_local_APIC(); > > mcheck_cpu_clear(this_cpu_ptr(&cpu_info)); > > > No luck. The problem is still seen with this patch applied on top of > the mainline kernel (commit a69e90512d9def6). Yeah, was a bit too naive We actually need to do the full cpuhotplug dance for a regular reboot. In the panic case, there is nothing we can do about. I'll have a look tomorrow. Thanks, tglx
Re: /sys/devices/system/cpu/vulnerabilities/ doesn't show all known CPU vulnerabilities
Thomas Gleixner, Alright. Then I guess I am wasting everyone's time and everything is as it should be according to you. I will unsubscribe from this mailing list because it is flooding my mail box with so many messages and I don't know of any way to subscribe only to this particular thread. Please CC me if this discussion continues.
[PATCH] staging: android: Remove ion device tree bindings from the TODO
This patch removes the todo for the ion chunk and carveout device tree bindings. Signed-off-by: Donald Yandt --- drivers/staging/android/TODO | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO index fbf015cc6..767dd98fd 100644 --- a/drivers/staging/android/TODO +++ b/drivers/staging/android/TODO @@ -6,8 +6,6 @@ TODO: ion/ - - Add dt-bindings for remaining heaps (chunk and carveout heaps). This would - involve putting appropriate bindings in a memory node for Ion to find. - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0) - Better test framework (integration with VGEM was suggested) -- 2.21.0
Re: [PATCH -rcu/dev] Please squash: fixup! rcu/tree: Add basic support for kfree_rcu() batching
On Sat, Aug 17, 2019 at 12:43:08AM -0400, Joel Fernandes wrote: > On Fri, Aug 16, 2019 at 09:38:54PM -0700, Paul Walmsley wrote: > > On Sat, 17 Aug 2019, Joel Fernandes (Google) wrote: > > > > > xchg() on a bool is causing issues on riscv and arm32. > > > > Indeed, it seems best not to use xchg() on any type that's not 32 bits > > long or that's not the CPU's native word size. Probably we should update > > the documentation. > > I would endorse any such documentation effort ;-) > > > > Please squash this into the -rcu dev branch to resolve the issue. > > > > > > Please squash this fix. Done, please see below for updated version. > > > Fixes: -rcu dev commit 3cbd3aa7d9c7bdf ("rcu/tree: Add basic support for > > > kfree_rcu() batching") > > > > > > Signed-off-by: Joel Fernandes (Google) > > > > Link: > > https://lore.kernel.org/lkml/alpine.deb.2.21..1908161931110.32...@viisi.sifive.com/T/#me9956f66cb611b95d26ae92700e1d901f46e8c59 > > Reviewed-by: Paul Walmsley I added the link, thank you Paul! If you meant the Reviewed-by to apply to the entire kfree_rcu() patch, I will of course be very happy to apply that as well. > Thanks Paul! And nice to meet you again after many years ;-) Glad to see you > working on riscv. What Joel said! ;-) Thanx, Paul
Re: PROBLEM: 5.3.0-rc* causes iwlwifi failure
After some private coaching from Serge Belyshev on git-revert I can confirm that reverting that commit atop the current tree resolves the issue (the wifi card scans for and finds networks just fine, no dmesg errors reported, etc.). On Sat, Aug 17, 2019 at 11:59:59AM +0300, Serge Belyshev wrote: > > > I am on an Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz running Linux > > x86_64 (Slackware), with a custom-compiled 5.3.0-rc4 (.config > > attached). > > > > I am using the Intel wifi adapter on this machine: > > > > 02:00.0 Network controller: Intel Corporation Device 24fb (rev 10) > > > > with the iwlwifi driver. I am attaching the output to 'lspci -vv -s > > 02:00.0' as the file device-info. > > > > All 5.3.0-rc* versions I have tried (including rc4) cause multiple > > dmesg iwlwifi-related errors (dmesg attached). Examples: > > > > iwlwifi :02:00.0: Failed to get geographic profile info -5 > > iwlwifi :02:00.0: Microcode SW error detected. Restarting 0x8200 > > iwlwifi :02:00.0: 0x0038 | BAD_COMMAND > > > > I have my logs filled with similar garbage throughout 5.3-rc*. Also > since 5.3-rcsomething not only it WARNS in dmesg about firmware failure, > but completely stops working after suspend/resume cycle. > > It looks like that: > > commit 4fd445a2c855bbcab81fbe06d110e78dbd974a5b > Author: Haim Dreyfuss > Date: Thu May 2 11:45:02 2019 +0300 > > iwlwifi: mvm: Add log information about SAR status > > Inform users when SAR status is changing. > > Signed-off-by: Haim Dreyfuss > Signed-off-by: Luca Coelho > > > is the culprit. (manually) reverting it on top of 5.3-rc4 makes > everything work again.
Re: [PATCH v4 1/2] rcu/tree: Add basic support for kfree_rcu() batching
On Sat, Aug 17, 2019 at 01:53:29AM -0400, Joel Fernandes wrote: > On Fri, Aug 16, 2019 at 10:20:23PM -0700, Paul E. McKenney wrote: > > On Sat, Aug 17, 2019 at 12:30:24AM -0400, Joel Fernandes wrote: > > > On Fri, Aug 16, 2019 at 08:56:37PM -0700, Paul E. McKenney wrote: > > > > On Fri, Aug 16, 2019 at 09:32:23PM -0400, Joel Fernandes wrote: > > > > > Hi Paul, > > > > > > > > > > On Fri, Aug 16, 2019 at 3:16 PM Paul E. McKenney > > > > > wrote: > > > > > > > > Hello, Joel, > > > > > > > > > > > > > > > > I reworked the commit log as follows, but was then unsuccessful > > > > > > > > in > > > > > > > > working out which -rcu commit to apply it to. Could you please > > > > > > > > tell me what commit to apply this to? (Once applied, git > > > > > > > > cherry-pick > > > > > > > > is usually pretty good about handling minor conflicts.) > > > > > > > > > > > > > > It was originally based on v5.3-rc2 > > > > > > > > > > > > > > I was able to apply it just now to the rcu -dev branch and I > > > > > > > pushed it here: > > > > > > > https://github.com/joelagnel/linux-kernel.git (branch paul-dev) > > > > > > > > > > > > > > Let me know if any other issues, thanks for the change log rework! > > > > > > > > > > > > Pulled and cherry-picked, thank you! > > > > > > > > > > > > Just for grins, I also pushed out a from-joel.2019.08.16a showing > > > > > > the > > > > > > results of the pull. If you pull that branch, then run something > > > > > > like > > > > > > "gitk v5.3-rc2..", and then do the same with branch "dev", > > > > > > comparing the > > > > > > two might illustrate some of the reasons for the current > > > > > > restrictions > > > > > > on pull requests and trees subject to rebase. > > > > > > > > > > Right, I did the compare and see what you mean. I guess sending any > > > > > future pull requests against Linux -next would be the best option? > > > > > > > > Hmmm... You really want to send some pull requests, don't you? ;-) > > > > > > I would be lying if I said I don't have the itch to ;-) > > > > > > > Suppose you had sent that pull request against Linux -next or v5.2 > > > > or wherever. What would happen next, given the high probability of a > > > > conflict with someone else's patch? What would the result look like? > > > > > > One hopes that the tools are able to automatically resolve the resolution, > > > however adequate re-inspection of the resulting code and testing it would > > > be > > > needed in either case, to ensure the conflict resolution (whether manual > > > or > > > automatic) happened correctly. > > > > I didn't ask you to hope. I instead asked you what tell me what would > > actually happen. ;-) > > > > You could actually try this by randomly grouping the patches in -rcu > > (say, placing every third patch into one of three groups), generating > > separate pull requests, and then merging the pull requests together. > > Then you wouldn't have to hope. You could instead look at it in (say) > > gitk after the pieces were put together. > > So you take whatever is worked on in 'dev' and create separate branches out > of them, then merge them together later? > > I have seen you doing these tricks and would love to get ideas from your > experiences on these. If the release dates line up, perhaps I can demo it for v5.4 at LPC. > > > IIUC, this usually depends on the maintainer's preference on which branch > > > to > > > send patches against. > > > > > > Are you saying -rcu's dev branch is still the best option to send patches > > > against, even though it is rebased often? > > > > Sounds like we might need to discuss this face to face. > > Yes, let us talk for sure at plumbers, thank you so much! > > (Also I sent a patch just now to fix that xchg() issue). Yes, I just now squashed it in, thank you! Thanx, Paul
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Sat, Aug 17, 2019 at 12:40:40PM -0400, Steven Rostedt wrote: > On Sat, 17 Aug 2019 11:55:17 -0400 (EDT) > Mathieu Desnoyers wrote: > > > - On Aug 17, 2019, at 11:26 AM, rostedt rost...@goodmis.org wrote: > > > > > On Sat, 17 Aug 2019 10:40:31 -0400 (EDT) > > > Mathieu Desnoyers wrote: > > > > > >> > I'm now even more against adding the READ_ONCE() or WRITE_ONCE(). > > >> > > >> I'm not convinced by your arguments. > > > > > > Prove to me that there's an issue here beyond theoretical analysis, > > > then I'll consider that patch. > > > > > > Show me a compiler used to compile the kernel that zeros out the > > > increment. Show me were the race actually occurs. > > > > > > I think the READ/WRITE_ONCE() is more confusing than helpful. And > > > unneeded churn to the code. And really not needed for something that's > > > not critical to execution. > > > > I'll have to let the authors of the LWN article speak up on this, because > > I have limited time to replicate this investigation myself. > > I'll let Paul McKenney convince me then, if he has any spare cycles ;-) You guys do manage to time these things sometimes. ;-) > The one instance in that article is from a 2013 bug, which talks about > storing a 64 bit value on a 32 bit machine. But the ref count is an int > (32 bit), and I highly doubt any compiler will split it into 16 bit > stores for a simple increment. And I don't believe Linux even supports > any architecture that requires 16 bit stores anymore. For a machine-sized and aligned increment, it is indeed hard to imagine, even for me. I would be more worried about stores of constants with lots of zero bits between non-zero bits on systems with small-sized store-immediate instructions. Thanx, Paul