Re: [PATCH v3 0/8] PM / ACPI: sleep: Additional changes related to suspend-to-idle
On 20.08.2019 23.38, Rafael J. Wysocki wrote: On Tuesday, August 20, 2019 3:29:48 PM CEST Rafael J. Wysocki wrote: On Tue, Aug 20, 2019 at 3:10 PM Kristian Klausen wrote: On 19.08.2019 22.41, Rafael J. Wysocki wrote: On Mon, Aug 19, 2019 at 5:47 PM Kristian Klausen wrote: On 19.08.2019 11.05, Rafael J. Wysocki wrote: On Monday, August 19, 2019 9:59:02 AM CEST Rafael J. Wysocki wrote: On Fri, Aug 16, 2019 at 10:26 PM Kristian Klausen wrote: On 02.08.2019 12.33, Rafael J. Wysocki wrote: Hi All, On top of the "Simplify the suspend-to-idle control flow" patch series posted previously: https://lore.kernel.org/lkml/71085220.z6FKkvYQPX@kreacher/ sanitize the suspend-to-idle flow even further. First off, decouple EC wakeup from the LPS0 _DSM processing (patch 1). Next, reorder the code to invoke LPS0 _DSM Functions 5 and 6 in the specification-compliant order with respect to suspending and resuming devices (patch 2). Finally, rearrange lps0_device_attach() (patch 3) and add a command line switch to prevent the LPS0 _DSM from being used. The v2 is because I found a (minor) bug in patch 1, decided to use a module parameter instead of a kernel command line option in patch 4. Also, there are 4 new patches: Patch 5: Switch the EC over to polling during "noirq" suspend and back during "noirq" resume. Patch 6: Eliminate acpi_sleep_no_ec_events(). Patch 7: Consolidate some EC code depending on PM_SLEEP. Patch 8: Add EC GPE dispatching debug message. The v3 is just a rearranged v2 so as to move the post sensitive patch (previous patch 2) to the end of the series. [After applying the full series the code is the same as before.] For easier testing, the series (along with some previous patches depended on by it) is available in the pm-s2idle-testing branch of the linux-pm.git tree at kernel.org: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=pm-s2idle-testing It was just testing this patch series(461fc1caed55), to see if it would fix my charging issue (https://bugzilla.kernel.org/show_bug.cgi?id=201307), which it didn't. It is unlikely to help in that case. Do you have any idea what the issue could be? Basically, there are two possibilities: either the OS is expected to handle the AC/battery switching events, or the platform firmware should take care of them. In the former case, the EC should generate events to be handled by the OS and in the latter one there needs to be a way to let the platform firmware that it needs to take care of those events going forward. In either case there may be a platform-specific action to be carried out during suspend and resume to set this up as expected which may be missing. Thanks for the explanation. I don't think I have the expertise to solve the issue, but at least now I'm one step closer. I did however notice that my laptop (ASUS Zenbook UX430UNR/i7-8550U) won't wake when opening the lid or pressing a key, the only way to wake the laptop is pressing the power button. I also tested mainline (5.3.0-rc4 b7e7c85dc7b0) and 5.2.8 and the laptop wakes without issue when the lid is opened or a key is presed. Please refer to the changelogs for details. Thanks for your report. I seem to see a similar issue with respect to the lid on one of my test machines, looking into it right now. Well, my lid issue seems to be unrelated as it doesn't result from any patches in the series in question. First off, please clone 5.3-rc5 from kernel.org and double check if the issue is not present in that one. If that's not the case, merge the pm-s2idle-rework branch from my tree on top of it and retest. If you still see the issue then, apply the appended patch (on top of the pm-s2idle-reqork branch ) and, after starting the kernel, do # echo 1 > /sys/power/pm_debug_messages suspend the system and try to wake it up through all of the ways that stopped working. Then, wake it up with the power button, save the output of dmesg and send it to me. Thanks! With 5.3-rc5 the laptops wakes up without any issue when pressing a key or opening the lid. With v5.3-rc5+pm-s2idle-testing I can only wake the laptop by pressing the power button. OK, thanks for verifying. So it is unclear to me how the series can cause an issue like that to appear. dmesg with pm_debug_messages=1 and your patch: [ 55.646109] PM: suspend entry (s2idle) [ 55.698559] Filesystems sync: 0.052 seconds [ 55.698561] PM: Preparing system for sleep (s2idle) [ 55.700661] Freezing user space processes ... (elapsed 0.210 seconds) done. [ 55.911494] OOM killer disabled. [ 55.911495] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 55.913192] PM: Suspending system (s2idle) [ 55.913195] printk: Suspending console(s) (use no_console_suspend to debug) [ 55.914778] [drm] CT: disabled [ 55.916057] wlan0: deauthenticating from 64:70:02:a5:fd:02 by local choice (Reason: 3=DEAUTH_LEAVING) [ 56.045634] sd 2:0:0:0: [sda] Synchronizing SCSI
Re: linux-next: Signed-off-by missing for commit in the security tree
On Tue, Aug 20, 2019 at 3:32 PM Stephen Rothwell wrote: > > Hi all, > > Commit > > 9d1f8be5cf42 ("bpf: Restrict bpf when kernel lockdown is in confidentiality > mode") > > is missing a Signed-off-by from its author. I'm providing this under category (c) of the DCO.
Re: [PATCH v5 2/3] OPP: Add support for bandwidth OPP tables
On Tue, Aug 20, 2019 at 3:27 PM Saravana Kannan wrote: > > On Mon, Aug 19, 2019 at 11:13 PM Viresh Kumar wrote: > > > > On 07-08-19, 15:31, Saravana Kannan wrote: > > > + ret = of_property_read_u32(np, "opp-peak-kBps", ); > > > + if (ret) > > > + return ret; > > > + new_opp->rate = (unsigned long) bw; > > > + > > > + ret = of_property_read_u32(np, "opp-avg-kBps", ); > > > + if (!ret) > > > + new_opp->avg_bw = (unsigned long) bw; > > > > If none of opp-hz/level/peak-kBps are available, print error message here > > itself.. > > But you don't print any error for opp-level today. Seems like it's optional? > > > > > > + > > > + return 0; > > > > You are returning 0 on failure as well here. > > Thanks. Wait, no. This is not actually a failure. opp-avg-kBps is optional. So returning 0 is the right thing to do. If the mandatory properties aren't present an error is returned before you get to th end. -Saravana
Re: [PATCH 2/6] dt-bindings: net: sun8i-a83t-emac: Add phy-io-supply property
On Tue, Aug 20, 2019 at 11:57:06AM -0500, Rob Herring wrote: > On Tue, Aug 20, 2019 at 11:34 AM Ondřej Jirman wrote: > > > > On Tue, Aug 20, 2019 at 11:20:22AM -0500, Rob Herring wrote: > > > On Tue, Aug 20, 2019 at 9:53 AM wrote: > > > > > > > > From: Ondrej Jirman > > > > > > > > Some PHYs require separate power supply for I/O pins in some modes > > > > of operation. Add phy-io-supply property, to allow enabling this > > > > power supply. > > > > > > Perhaps since this is new, such phys should have *-supply in their nodes. > > > > Yes, I just don't understand, since external ethernet phys are so common, > > and they require power, how there's no fairly generic mechanism for this > > already in the PHY subsystem, or somewhere? > > Because generic mechanisms for this don't work. For example, what > happens when the 2 supplies need to be turned on in a certain order > and with certain timings? And then add in reset or control lines into > the mix... You can see in the bindings we already have some of that. I've looked at the emac bindings that have phy-supply, and don't see reason why this can't be generic for the phy. Just like there's generic reset properties for phys, now. Some bindings, like fsl-fec.txt even list custom reset properties for phy as deprecated, and recommend using generic ones. >From the point of the view of the emac driver, it just wants to power on/power off the phy, and wait until it's ready to be communicated with. It's probably better to have power supplies of the phy covered by generic phy code, because then you don't have to duplicate all this special power up logic in every emac driver, whenever a HW designer decides to combine such emac with external phy that requires some special hadnling on powerup. At the moment, this lack of flexibility is hacked around by adding multiple regulators to the DTS, and making them dependent on each other (even if one doesn't supply the other), just because this makes the regulator core driver enable them all. Power up delays for the PHY are described as enable-ramp-delays on the regulators (actual regulator ramp delay + wait time for PHY to initialize). Basically just hacking the DT so that the Linux kernel in the end does what's necessary, instead of DT describing the actual HW. Adding a single supply property to the phy node, as you suggest will do nothing to help this situation. It will just result in a more complicated dwmac-sun8i driver and will not help anyone in the future. So I think, maybe phy powerup should be moved to generic code, just like the phy reset code was. Generic code can have multiple supplies and some generic way to specify power up order and timings. But I guess, this patch series is a dead end. > > It looks like other ethernet mac drivers also implement supplies on phys > > on the EMAC nodes. Just grep phy-supply through dt-bindings/net. > > > > Historical reasons, or am I missing something? It almost seems like I must > > be missing something, since putting these properties to phy nodes > > seems so obvious. > > Things get added one by one and one new property isn't that > controversial. We've generally learned the lesson and avoid this > pattern now, but ethernet phys are one of the older bindings. Understood. So maybe the solution suggested above would improve the situation eventually? regards, o. > Rob > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v5 2/3] OPP: Add support for bandwidth OPP tables
On Fri, Aug 16, 2019 at 11:21 AM Stephen Boyd wrote: > > Quoting Saravana Kannan (2019-08-07 15:31:10) > > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > > index 1813f5ad5fa2..e1750033fef9 100644 > > --- a/drivers/opp/of.c > > +++ b/drivers/opp/of.c > > @@ -523,6 +523,35 @@ void dev_pm_opp_of_remove_table(struct device *dev) > > } > > EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); > > > > +static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node > > *np) > > +{ > > + int ret; > > + u64 rate; > > + u32 bw; > > + > > + ret = of_property_read_u64(np, "opp-hz", ); > > + if (!ret) { > > + /* > > +* Rate is defined as an unsigned long in clk API, and so > > +* casting explicitly to its type. Must be fixed once rate > > is 64 > > +* bit guaranteed in clk API. > > +*/ > > + new_opp->rate = (unsigned long)rate; > > + return 0; > > + } > > + > > + ret = of_property_read_u32(np, "opp-peak-kBps", ); > > + if (ret) > > + return ret; > > + new_opp->rate = (unsigned long) bw; > > + > > + ret = of_property_read_u32(np, "opp-avg-kBps", ); > > + if (!ret) > > I would write this as > > if (!of_property_read_u32(np, "opp-avg-kBps", )) > new_opp->avg_bw = (unsigned long) bw; > > because you don't care about the return value. Sure, will do. > > > + new_opp->avg_bw = (unsigned long) bw; > > + > > + return 0; > > +} > > + > > /** > > * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) > > * @opp_table: OPP table > > diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h > > index 01a500e2c40a..6bb238af9cac 100644 > > --- a/drivers/opp/opp.h > > +++ b/drivers/opp/opp.h > > @@ -56,7 +56,8 @@ extern struct list_head opp_tables; > > * @turbo: true if turbo (boost) OPP > > * @suspend: true if suspend OPP > > * @pstate: Device's power domain's performance state. > > - * @rate: Frequency in hertz > > + * @rate: Frequency in hertz OR Peak bandwidth in kilobytes per second > > Why is Peak capitalized? Because it's another Key? :) Just kidding. I'll fix it. > > + * @avg_bw:Average bandwidth in kilobytes per second > > * @level: Performance level > > * @supplies: Power supplies voltage/current values > > * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's > > @@ -78,6 +79,7 @@ struct dev_pm_opp { > > bool suspend; > > unsigned int pstate; > > unsigned long rate; > > If you're trying to save space why not make an anonymous union here of > 'rate' and 'bandwidth'? Then the code doesn't read all weird. It's not about saving space. It's about having to rewrite all the helper functions (see subsequent patch in this series) for different "keys" with zero difference in the actual comparisons/logic. I'm proposing I rename this to "key" in another email. -Saravana
linux-next: Signed-off-by missing for commit in the security tree
Hi all, Commit 9d1f8be5cf42 ("bpf: Restrict bpf when kernel lockdown is in confidentiality mode") is missing a Signed-off-by from its author. -- Cheers, Stephen Rothwell pgp4NgOtEgmvz.pgp Description: OpenPGP digital signature
Re: [PATCH v2 07/12] powerpc/mm: move ioremap_prot() into ioremap.c
On Tue, Aug 20, 2019 at 02:07:15PM +, Christophe Leroy wrote: > Both ioremap_prot() are idenfical, move them into ioremap.c s/idenfical/identical/
Re: [PATCH v2 05/12] powerpc/mm: rework io-workaround invocation.
On Tue, Aug 20, 2019 at 02:07:13PM +, Christophe Leroy wrote: > ppc_md.ioremap() is only used for I/O workaround on CELL platform, > so indirect function call can be avoided. > > This patch reworks the io-workaround and ioremap() functions to > use the global 'io_workaround_inited' flag for the activation > of io-workaround. > > When CONFIG_PPC_IO_WORKAROUNDS or CONFIG_PPC_INDIRECT_MMIO are not > selected, the I/O workaround ioremap() voids and the global flag is > not used. Note that CONFIG_PPC_IO_WORKAROUNDS is only selected by a specific cell config, and CONFIG_PPC_INDIRECT_MMIO is always selected by cell, so I think we can make CONFIG_PPC_IO_WORKAROUNDS depend on CONFIG_PPC_INDIRECT_MMIO > #define _IO_WORKAROUNDS_H > > +#ifdef CONFIG_PPC_IO_WORKAROUNDS > #include > #include > > @@ -32,4 +33,23 @@ extern int spiderpci_iowa_init(struct iowa_bus *, void *); > #define SPIDER_PCI_DUMMY_READ0x0810 > #define SPIDER_PCI_DUMMY_READ_BASE 0x0814 > > +#endif > + > +#if defined(CONFIG_PPC_IO_WORKAROUNDS) && defined(CONFIG_PPC_INDIRECT_MMIO) and simplify the ifdefs here a bit. Otherwise this looks fine: Reviewed-by: Christoph Hellwig
Re: [PATCH v5 2/3] OPP: Add support for bandwidth OPP tables
On Mon, Aug 19, 2019 at 11:13 PM Viresh Kumar wrote: > > On 07-08-19, 15:31, Saravana Kannan wrote: > > Not all devices quantify their performance points in terms of frequency. > > Devices like interconnects quantify their performance points in terms of > > bandwidth. We need a way to represent these bandwidth levels in OPP. So, > > add support for parsing bandwidth OPPs from DT. > > > > Signed-off-by: Saravana Kannan > > --- > > drivers/opp/of.c | 41 - > > drivers/opp/opp.h | 4 +++- > > 2 files changed, 35 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > > index 1813f5ad5fa2..e1750033fef9 100644 > > --- a/drivers/opp/of.c > > +++ b/drivers/opp/of.c > > @@ -523,6 +523,35 @@ void dev_pm_opp_of_remove_table(struct device *dev) > > } > > EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); > > > > +static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node > > *np) > > +{ > > + int ret; > > + u64 rate; > > + u32 bw; > > + > > + ret = of_property_read_u64(np, "opp-hz", ); > > + if (!ret) { > > + /* > > + * Rate is defined as an unsigned long in clk API, and so > > + * casting explicitly to its type. Must be fixed once rate is > > 64 > > + * bit guaranteed in clk API. > > + */ > > + new_opp->rate = (unsigned long)rate; > > + return 0; > > + } > > + > > Please read opp-level also here and do error handling. Can you please explain what's the reasoning? opp-level doesn't seem to be a "key" based on looking at the code. > > > + ret = of_property_read_u32(np, "opp-peak-kBps", ); > > + if (ret) > > + return ret; > > + new_opp->rate = (unsigned long) bw; > > + > > + ret = of_property_read_u32(np, "opp-avg-kBps", ); > > + if (!ret) > > + new_opp->avg_bw = (unsigned long) bw; > > If none of opp-hz/level/peak-kBps are available, print error message here > itself.. But you don't print any error for opp-level today. Seems like it's optional? > > > + > > + return 0; > > You are returning 0 on failure as well here. Thanks. > > +} > > + > > /** > > * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) > > * @opp_table: OPP table > > @@ -560,22 +589,16 @@ static struct dev_pm_opp *_opp_add_static_v2(struct > > opp_table *opp_table, > > if (!new_opp) > > return ERR_PTR(-ENOMEM); > > > > - ret = of_property_read_u64(np, "opp-hz", ); > > + ret = _read_opp_key(new_opp, np); > > if (ret < 0) { > > /* "opp-hz" is optional for devices like power domains. */ > > if (!opp_table->is_genpd) { > > - dev_err(dev, "%s: opp-hz not found\n", __func__); > > + dev_err(dev, "%s: opp-hz or opp-peak-kBps not > > found\n", > > + __func__); > > goto free_opp; > > } > > > > rate_not_available = true; > > Move all above as well to read_opp_key(). Ok. I didn't want to print an error at the API level and instead print at the caller level. But if that's what you want, that's fine by me. > > > - } else { > > - /* > > - * Rate is defined as an unsigned long in clk API, and so > > - * casting explicitly to its type. Must be fixed once rate is > > 64 > > - * bit guaranteed in clk API. > > - */ > > - new_opp->rate = (unsigned long)rate; > > } > > > > of_property_read_u32(np, "opp-level", _opp->level); > > diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h > > index 01a500e2c40a..6bb238af9cac 100644 > > --- a/drivers/opp/opp.h > > +++ b/drivers/opp/opp.h > > @@ -56,7 +56,8 @@ extern struct list_head opp_tables; > > * @turbo: true if turbo (boost) OPP > > * @suspend: true if suspend OPP > > * @pstate: Device's power domain's performance state. > > - * @rate:Frequency in hertz > > + * @rate:Frequency in hertz OR Peak bandwidth in kilobytes per second > > + * @avg_bw: Average bandwidth in kilobytes per second > > Please add separate entry for peak_bw here. > > I know you reused rate because you don't want to reimplement the helpers we > have. Maybe we can just update them to return peak_bw when opp-hz isn't > present. How about I just rename this to "key"? That makes a lot more sense than trying to save 3 different keys and going through them one at a time. > > * @level: Performance level > > * @supplies:Power supplies voltage/current values > > * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's > > @@ -78,6 +79,7 @@ struct dev_pm_opp { > > bool suspend; > > unsigned int pstate; > > unsigned long rate; > > + unsigned long avg_bw; > > unsigned int level; > > > > struct dev_pm_opp_supply *supplies; > > -- > >
Re: linux-next: Fixes tag needs some work in the sound-asoc tree
Hi all, These actually relate to the sound-asoc-fixes tree. On Wed, 21 Aug 2019 07:04:52 +1000 Stephen Rothwell wrote: > > Hi all, > > In commit > > 0f6fc97501b7 ("ASoC: mchp-i2s-mcc: Wait for RX/TX RDY only if controller is > running") > > Fixes tag > > Fixes: 7e0cdf545a55 ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel > Controller") > > has these problem(s): > > - Target SHA1 does not exist > > Did you mean > > Fixes: b87d37d0231f ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel > Controller") > > In commit > > 988b59467b2b ("ASoC: mchp-i2s-mcc: Fix unprepare of GCLK") > > Fixes tag > > Fixes: 7e0cdf545a55 ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel > Controller") > > has these problem(s): > > - Target SHA1 does not exist > > Did you mean > > Fixes: b87d37d0231f ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel > Controller") -- Cheers, Stephen Rothwell pgpEjdqmZs0CV.pgp Description: OpenPGP digital signature
RE: [PATCH] net/ncsi: add control packet payload to NC-SI commands from netlink
> Hi Ben, > > > Hi Justin, > > > > > Hi Ben, > > > > > > I have similar fix locally with different approach as the command handler > > > may have some expectation for those byes. > > > We can use NCSI_PKT_CMD_OEM handler as it only copies data based on the > > > payload length. > > > > Great! Yes I was thinking the same, we just need some way to take data > > payload sent from netlink message and sent it over NC-SI. > > > > > > > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c index > > > 5c3fad8..3b01f65 100644 > > > --- a/net/ncsi/ncsi-cmd.c > > > +++ b/net/ncsi/ncsi-cmd.c > > > @@ -309,14 +309,19 @@ static struct ncsi_request > > > *ncsi_alloc_command(struct ncsi_cmd_arg *nca) > > > > > > int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) { > > > + struct ncsi_cmd_handler *nch = NULL; > > > struct ncsi_request *nr; > > > + unsigned char type; > > > struct ethhdr *eh; > > > - struct ncsi_cmd_handler *nch = NULL; > > > int i, ret; > > > > > > + if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) > > > + type = NCSI_PKT_CMD_OEM; > > > + else > > > + type = nca->type; > > > /* Search for the handler */ > > > for (i = 0; i < ARRAY_SIZE(ncsi_cmd_handlers); i++) { > > > - if (ncsi_cmd_handlers[i].type == nca->type) { > > > + if (ncsi_cmd_handlers[i].type == type) { > > > if (ncsi_cmd_handlers[i].handler) > > > nch = _cmd_handlers[i]; > > > else > > > > > > > So in this case NCSI_PKT_CMD_OEM would be the default handler for all NC-SI > > command over netlink (standard and OEM), correct? > Yes, that is correct. The handler for NCSI_PKT_CMD_OEM command is generic. > > > Should we rename this to something like NCSI_PKT_CMD_GENERIC for clarity > > perhaps? Do you plan to upstream this patch? > NCSI_PKT_CMD_OEM is a real command type and it is defined by the NC-SI > specific. > We can add comments to indicate that we use the generic command handler from > NCSI_PKT_CMD_OEM command. > > Does the change work for you? If so, I will prepare the patch. Thanks Justin. I verified this change and it works, thanks! -Ben
linux-next: Fixes tag needs some work in the sound-asoc tree
Hi all, In commit c9cff337eab3 ("ASoC: mchp-i2s-mcc: Fix simultaneous capture and playback in master mode") Fixes tag Fixes: 7e0cdf545a55 ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel Controller") has these problem(s): - Target SHA1 does not exist Did you mean Fixes: b87d37d0231f ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel Controller") -- Cheers, Stephen Rothwell pgpdcGoONufXN.pgp Description: OpenPGP digital signature
Re: [v2 PATCH] RISC-V: Optimize tlb flush path.
On Tue, 2019-08-20 at 15:18 -0700, h...@infradead.org wrote: > CAUTION: This email originated from outside of Western Digital. Do > not click on links or open attachments unless you recognize the > sender and know that the content is safe. > > > On Tue, Aug 20, 2019 at 08:28:36PM +, Atish Patra wrote: > > > http://git.infradead.org/users/hch/riscv.git/commitdiff/ea4067ae61e20fcfcf46a6f6bd1cc25710ce3afe > > > > This does seem a lot cleaner to me. We can reuse some of the code > > for > > this patch as well. Based on NATIVE_CLINT configuration, it will > > send > > an IPI or SBI call. > > > > I can rebase my patch on top of yours and I can send it together or > > you > > can include in your series. > > > > Let me know your preference. > > I think the native clint for S-mode will need more discussion, so you > should not wait for it. Ok sure. I will move the code to tlbflush.c and refactor the tlb flush functions similar to what you have in your patch. -- Regards, Atish
Re: [PATCH v2 04/12] powerpc/mm: drop function __ioremap()
On Tue, Aug 20, 2019 at 02:07:12PM +, Christophe Leroy wrote: > __ioremap() is not used anymore, drop it. > > Suggested-by: Christoph Hellwig > Signed-off-by: Christophe Leroy Looks good, I've already dropped my version of this from the generic ioremap series: Reviewed-by: Christoph Hellwig
Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory.
On Wed, Aug 21, 2019 at 07:06:51AM +0900, Tetsuo Handa wrote: > syzbot found that a thread can stall for minutes inside read_mem() > after that thread was killed by SIGKILL [1]. Reading 2GB at one read() > is legal, but delaying termination of killed thread for minutes is bad. > > [ 1335.912419][T20577] read_mem: sz=4096 count=2134565632 > [ 1335.943194][T20577] read_mem: sz=4096 count=2134561536 > [ 1335.978280][T20577] read_mem: sz=4096 count=2134557440 > [ 1336.011147][T20577] read_mem: sz=4096 count=2134553344 > [ 1336.041897][T20577] read_mem: sz=4096 count=2134549248 > > [1] > https://syzkaller.appspot.com/bug?id=a0e3436829698d5824231251fad9d8e998f94f5e > > Signed-off-by: Tetsuo Handa > Reported-by: syzbot > --- > drivers/char/mem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > index b08dc50..0f7d4c4 100644 > --- a/drivers/char/mem.c > +++ b/drivers/char/mem.c > @@ -135,7 +135,7 @@ static ssize_t read_mem(struct file *file, char __user > *buf, > if (!bounce) > return -ENOMEM; > > - while (count > 0) { > + while (count > 0 && !fatal_signal_pending(current)) { > unsigned long remaining; > int allowed, probe; > > -- > 1.8.3.1 > Oh, nice! This shouldn't break anything that is assuming that the read will complete before a signal is delivered, right? I know userspace handling of "short" reads is almost always not there... thanks, greg k-h
Re: [PATCH v2 02/12] powerpc/ps3: replace __ioremap() by ioremap_prot()
On Tue, Aug 20, 2019 at 02:07:10PM +, Christophe Leroy wrote: > __ioremap() is similar to ioremap_prot() except that ioremap_prot() > does a few sanity changes in addition. > > The flags used by PS3 are not impacted by those changes so for > PS3 both functions are equivalent. > > At the same time, drop parts of the comment that have been invalid > since commit e58e87adc8bf ("powerpc/mm: Update _PAGE_KERNEL_RO") > > Suggested-by: Christoph Hellwig > Signed-off-by: Christophe Leroy Looks good, Reviewed-by: Christoph Hellwig
Re: [RFC] mm: Proactive compaction
On Fri, Aug 16, 2019 at 02:43:30PM -0700, Nitin Gupta wrote: > Testing done (on x86): > - Set /sys/kernel/mm/compaction/order-9/extfrag_{low,high} = {25, 30} > respectively. > - Use a test program to fragment memory: the program allocates all memory > and then for each 2M aligned section, frees 3/4 of base pages using > munmap. > - kcompactd0 detects fragmentation for order-9 > extfrag_high and starts > compaction till extfrag < extfrag_low for order-9. Your test program is a good idea, but I worry it may produce unrealistically optimistic outcomes. Page cache is readily reclaimable, so you're setting up a situation where 2MB pages can once again be produced. How about this: One program which creates a file several times the size of memory (or several files which total the same amount). Then read the file(s). Maybe by mmap(), and just do nice easy sequential accesses. A second program which causes slab allocations. eg for (;;) { for (i = 0; i < n * 1000 * 1000; i++) { char fname[64]; sprintf(fname, "/tmp/missing.%d", i); open(fname, O_RDWR); } } The first program should thrash the pagecache, causing pages to continuously be allocated, reclaimed and freed. The second will create millions of dentries, causing the slab allocator to allocate a lot of order-0 pages which are harder to free. If you really want to make it work hard, mix in opening some files whihc actually exist, preventing the pages which contain those dentries from being evicted. This feels like it's simulating a more normal workload than your test. What do you think?
Re: [v2 PATCH] RISC-V: Optimize tlb flush path.
On Tue, Aug 20, 2019 at 08:28:36PM +, Atish Patra wrote: > > http://git.infradead.org/users/hch/riscv.git/commitdiff/ea4067ae61e20fcfcf46a6f6bd1cc25710ce3afe > > This does seem a lot cleaner to me. We can reuse some of the code for > this patch as well. Based on NATIVE_CLINT configuration, it will send > an IPI or SBI call. > > I can rebase my patch on top of yours and I can send it together or you > can include in your series. > > Let me know your preference. I think the native clint for S-mode will need more discussion, so you should not wait for it.
Re: Linux 3.16.73
Thanks, a bunch Ben. :) On 22:52 Tue 20 Aug 2019, Ben Hutchings wrote: I'm announcing the release of the 3.16.73 kernel. All users of the 3.16 kernel series should upgrade. The updated 3.16.y git tree can be found at: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git linux-3.16.y and can be browsed at the normal kernel.org git web browser: https://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git The diff from 3.16.72 is attached to this message. Ben. Documentation/siphash.txt | 75 +++ Makefile | 2 +- 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, 572 insertions(+), 27 deletions(-) Ben Hutchings (2): tcp: Clear sk_send_head after purging the write queue Linux 3.16.73 Jason A. Donenfeld (1): siphash: implement HalfSipHash1-3 for hash tables zhangyi (F) (2): ext4: brelse all indirect buffer in ext4_ind_remove_space() ext4: cleanup bh release code in ext4_ind_remove_space() -- Ben Hutchings Experience is what causes a person to make new mistakes instead of old ones. diff --git a/Documentation/siphash.txt b/Documentation/siphash.txt index e8e6ddbbaab4..908d348ff777 100644 --- a/Documentation/siphash.txt +++ b/Documentation/siphash.txt @@ -98,3 +98,78 @@ u64 h = siphash(, offsetofend(typeof(combined), dport), ); 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(, 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(>key, sizeof(table->key)); +} + +static inline hlist_head *some_hashtable_bucket(struct some_hashtable *table, struct interesting_input *input) +{ + return >hashtable[hsiphash(input, sizeof(*input), >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. diff --git a/Makefile b/Makefile index e2d6e0b9f22d..935fc9df7b17 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ VERSION = 3 PATCHLEVEL = 16 -SUBLEVEL = 72 +SUBLEVEL = 73 EXTRAVERSION = NAME = Museum of Fishiegoodies diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c index 8df46f49a3d5..475a1d40f23e 100644 --- a/fs/ext4/indirect.c +++ b/fs/ext4/indirect.c @@ -1313,6 +1313,7 @@ int ext4_ind_remove_space(handle_t *handle, struct inode *inode, 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;
Re: [PATCH v7 1/7] driver core: Add support for linking devices during device addition
On Mon, Aug 19, 2019 at 9:25 PM Frank Rowand wrote: > > On 8/19/19 5:00 PM, Saravana Kannan wrote: > > On Sun, Aug 18, 2019 at 8:38 PM Frank Rowand wrote: > >> > >> On 8/15/19 6:50 PM, Saravana Kannan wrote: > >>> On Wed, Aug 7, 2019 at 7:04 PM Frank Rowand > >>> wrote: > > > Date: Tue, 23 Jul 2019 17:10:54 -0700 > > Subject: [PATCH v7 1/7] driver core: Add support for linking devices > > during > > device addition > > From: Saravana Kannan > > > > When devices are added, the bus might want to create device links to > > track > > functional dependencies between supplier and consumer devices. This > > tracking of supplier-consumer relationship allows optimizing device > > probe > > order and tracking whether all consumers of a supplier are active. The > > add_links bus callback is added to support this. > > Change above to: > > When devices are added, the bus may create device links to track which > suppliers a consumer device depends upon. This > tracking of supplier-consumer relationship may be used to defer probing > the driver of a consumer device before the driver(s) for its supplier > device(s) > are probed. It may also be used by a supplier driver to determine if > all of its consumers have been successfully probed. > The add_links bus callback is added to create the supplier device links > > > > > However, when consumer devices are added, they might not have a supplier > > device to link to despite needing mandatory resources/functionality from > > one or more suppliers. A waiting_for_suppliers list is created to track > > such consumers and retry linking them when new devices get added. > > Change above to: > > If a supplier device has not yet been created when the consumer device > attempts > to link it, the consumer device is added to the wait_for_suppliers list. > When supplier devices are created, the supplier device link will be > added to > the relevant consumer devices on the wait_for_suppliers list. > > >>> > >>> I'll take these commit text suggestions if we decide to revert the > >>> entire series at the end of this review. > >>> > > > > Signed-off-by: Saravana Kannan > > --- > > drivers/base/core.c| 83 ++ > > include/linux/device.h | 14 +++ > > 2 files changed, 97 insertions(+) > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index da84a73f2ba6..1b4eb221968f 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -44,6 +44,8 @@ early_param("sysfs.deprecated", > > sysfs_deprecated_setup); > > #endif > > > > /* Device links support. */ > > +static LIST_HEAD(wait_for_suppliers); > > +static DEFINE_MUTEX(wfs_lock); > > > > #ifdef CONFIG_SRCU > > static DEFINE_MUTEX(device_links_lock); > > @@ -401,6 +403,51 @@ struct device_link *device_link_add(struct device > > *consumer, > > } > > EXPORT_SYMBOL_GPL(device_link_add); > > > > +/** > > > + * device_link_wait_for_supplier - Mark device as waiting for supplier > > * device_link_wait_for_supplier - Add device to wait_for_suppliers > list > >>> > >> > >> As a meta-comment, I found this series very hard to understand in the > >> context > >> of reading the new code for the first time. When I read the code again in > >> six months or a year or two years it will not be in near term memory and it > >> will be as if I am reading it for the first time. A lot of my suggestions > >> for changes of names are in that context -- the current names may be fine > >> when one has recently read the code, but not so much when trying to read > >> the whole thing again with a blank mind. > > > > Thanks for the context. > > > >> The code also inherits a good deal of complexity because it does not stand > >> alone in a nice discrete chunk, but instead delicately weaves into a more > >> complex body of code. > > > > I'll take this as a compliment :) > > Please do! > > > > > >> When I was trying to understand the code, I wrote a lot of additional > >> comments within my reply email to provide myself context, information > >> about various things, and questions that I needed to answer (or if I > >> could not answer to then ask you). Then I ended up being able to remove > >> many of those notes before sending the reply. > >> > >> > >>> I intentionally chose "Mark device..." because that's a better > >>> description of the semantics of the function instead of trying to > >>> describe the implementation. Whether I'm using a linked list or some > >>> other data structure should not be the one line documentation of a > >>> function. Unless the function is explicitly about operating on that > >>> specific data structure. > >> > >> I agree with
Re: [PATCH V40 16/29] acpi: Disable ACPI table override if the kernel is locked down
On Tuesday, August 20, 2019 2:17:52 AM CEST Matthew Garrett wrote: > From: Linn Crosetto > > >From the kernel documentation (initrd_table_override.txt): > > If the ACPI_INITRD_TABLE_OVERRIDE compile option is true, it is possible > to override nearly any ACPI table provided by the BIOS with an > instrumented, modified one. > > When lockdown is enabled, the kernel should disallow any unauthenticated > changes to kernel space. ACPI tables contain code invoked by the kernel, > so do not allow ACPI tables to be overridden if the kernel is locked down. > > Signed-off-by: Linn Crosetto > Signed-off-by: David Howells > Signed-off-by: Matthew Garrett > Reviewed-by: Kees Cook > cc: linux-a...@vger.kernel.org > Signed-off-by: James Morris Acked-by: Rafael J. Wysocki > --- > drivers/acpi/tables.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c > index de974322a197..b7c29a11c0c1 100644 > --- a/drivers/acpi/tables.c > +++ b/drivers/acpi/tables.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include "internal.h" > > #ifdef CONFIG_ACPI_CUSTOM_DSDT > @@ -577,6 +578,11 @@ void __init acpi_table_upgrade(void) > if (table_nr == 0) > return; > > + if (security_locked_down(LOCKDOWN_ACPI_TABLES)) { > + pr_notice("kernel is locked down, ignoring table override\n"); > + return; > + } > + > acpi_tables_addr = > memblock_find_in_range(0, ACPI_TABLE_UPGRADE_MAX_PHYS, > all_tables_size, PAGE_SIZE); >
Re: [PATCH V40 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
On Tuesday, August 20, 2019 2:17:51 AM CEST Matthew Garrett wrote: > From: Josh Boyer > > This option allows userspace to pass the RSDP address to the kernel, which > makes it possible for a user to modify the workings of hardware. Reject > the option when the kernel is locked down. This requires some reworking > of the existing RSDP command line logic, since the early boot code also > makes use of a command-line passed RSDP when locating the SRAT table > before the lockdown code has been initialised. This is achieved by > separating the command line RSDP path in the early boot code from the > generic RSDP path, and then copying the command line RSDP into boot > params in the kernel proper if lockdown is not enabled. If lockdown is > enabled and an RSDP is provided on the command line, this will only be > used when parsing SRAT (which shouldn't permit kernel code execution) > and will be ignored in the rest of the kernel. > > (Modified by Matthew Garrett in order to handle the early boot RSDP > environment) > > Signed-off-by: Josh Boyer > Signed-off-by: David Howells > Signed-off-by: Matthew Garrett > Reviewed-by: Kees Cook > cc: Dave Young > cc: linux-a...@vger.kernel.org > Signed-off-by: James Morris Acked-by: Rafael J. Wysocki > --- > arch/x86/boot/compressed/acpi.c | 19 +-- > arch/x86/include/asm/acpi.h | 9 + > arch/x86/include/asm/x86_init.h | 2 ++ > arch/x86/kernel/acpi/boot.c | 5 + > arch/x86/kernel/x86_init.c | 1 + > drivers/acpi/osl.c | 14 +- > include/linux/acpi.h| 6 ++ > 7 files changed, 49 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c > index ad84239e595e..e726e9b44bb1 100644 > --- a/arch/x86/boot/compressed/acpi.c > +++ b/arch/x86/boot/compressed/acpi.c > @@ -26,7 +26,7 @@ struct mem_vector immovable_mem[MAX_NUMNODES*2]; > */ > #define MAX_ADDR_LEN 19 > > -static acpi_physical_address get_acpi_rsdp(void) > +static acpi_physical_address get_cmdline_acpi_rsdp(void) > { > acpi_physical_address addr = 0; > > @@ -215,10 +215,7 @@ acpi_physical_address get_rsdp_addr(void) > { > acpi_physical_address pa; > > - pa = get_acpi_rsdp(); > - > - if (!pa) > - pa = boot_params->acpi_rsdp_addr; > + pa = boot_params->acpi_rsdp_addr; > > if (!pa) > pa = efi_get_rsdp_addr(); > @@ -240,7 +237,17 @@ static unsigned long get_acpi_srat_table(void) > char arg[10]; > u8 *entry; > > - rsdp = (struct acpi_table_rsdp *)(long)boot_params->acpi_rsdp_addr; > + /* > + * Check whether we were given an RSDP on the command line. We don't > + * stash this in boot params because the kernel itself may have > + * different ideas about whether to trust a command-line parameter. > + */ > + rsdp = (struct acpi_table_rsdp *)get_cmdline_acpi_rsdp(); > + > + if (!rsdp) > + rsdp = (struct acpi_table_rsdp *)(long) > + boot_params->acpi_rsdp_addr; > + > if (!rsdp) > return 0; > > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h > index aac686e1e005..bc9693c9107e 100644 > --- a/arch/x86/include/asm/acpi.h > +++ b/arch/x86/include/asm/acpi.h > @@ -117,6 +117,12 @@ static inline bool acpi_has_cpu_in_madt(void) > return !!acpi_lapic; > } > > +#define ACPI_HAVE_ARCH_SET_ROOT_POINTER > +static inline void acpi_arch_set_root_pointer(u64 addr) > +{ > + x86_init.acpi.set_root_pointer(addr); > +} > + > #define ACPI_HAVE_ARCH_GET_ROOT_POINTER > static inline u64 acpi_arch_get_root_pointer(void) > { > @@ -125,6 +131,7 @@ static inline u64 acpi_arch_get_root_pointer(void) > > void acpi_generic_reduced_hw_init(void); > > +void x86_default_set_root_pointer(u64 addr); > u64 x86_default_get_root_pointer(void); > > #else /* !CONFIG_ACPI */ > @@ -138,6 +145,8 @@ static inline void disable_acpi(void) { } > > static inline void acpi_generic_reduced_hw_init(void) { } > > +static inline void x86_default_set_root_pointer(u64 addr) { } > + > static inline u64 x86_default_get_root_pointer(void) > { > return 0; > diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h > index b85a7c54c6a1..d584128435cb 100644 > --- a/arch/x86/include/asm/x86_init.h > +++ b/arch/x86/include/asm/x86_init.h > @@ -134,10 +134,12 @@ struct x86_hyper_init { > > /** > * struct x86_init_acpi - x86 ACPI init functions > + * @set_root_poitner:set RSDP address > * @get_root_pointer:get RSDP address > * @reduced_hw_early_init: hardware reduced platform early init > */ > struct x86_init_acpi { > + void (*set_root_pointer)(u64 addr); > u64 (*get_root_pointer)(void); > void (*reduced_hw_early_init)(void); > }; > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index
[PATCH] /dev/mem: Bail out upon SIGKILL when reading memory.
syzbot found that a thread can stall for minutes inside read_mem() after that thread was killed by SIGKILL [1]. Reading 2GB at one read() is legal, but delaying termination of killed thread for minutes is bad. [ 1335.912419][T20577] read_mem: sz=4096 count=2134565632 [ 1335.943194][T20577] read_mem: sz=4096 count=2134561536 [ 1335.978280][T20577] read_mem: sz=4096 count=2134557440 [ 1336.011147][T20577] read_mem: sz=4096 count=2134553344 [ 1336.041897][T20577] read_mem: sz=4096 count=2134549248 [1] https://syzkaller.appspot.com/bug?id=a0e3436829698d5824231251fad9d8e998f94f5e Signed-off-by: Tetsuo Handa Reported-by: syzbot --- drivers/char/mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/mem.c b/drivers/char/mem.c index b08dc50..0f7d4c4 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -135,7 +135,7 @@ static ssize_t read_mem(struct file *file, char __user *buf, if (!bounce) return -ENOMEM; - while (count > 0) { + while (count > 0 && !fatal_signal_pending(current)) { unsigned long remaining; int allowed, probe; -- 1.8.3.1
Re: [PATCH V40 14/29] ACPI: Limit access to custom_method when the kernel is locked down
On Tuesday, August 20, 2019 2:17:50 AM CEST Matthew Garrett wrote: > From: Matthew Garrett > > custom_method effectively allows arbitrary access to system memory, making > it possible for an attacker to circumvent restrictions on module loading. > Disable it if the kernel is locked down. > > Signed-off-by: Matthew Garrett > Signed-off-by: David Howells > Reviewed-by: Kees Cook > cc: linux-a...@vger.kernel.org > Signed-off-by: James Morris Acked-by: Rafael J. Wysocki > --- > drivers/acpi/custom_method.c | 6 ++ > include/linux/security.h | 1 + > security/lockdown/lockdown.c | 1 + > 3 files changed, 8 insertions(+) > > diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c > index b2ef4c2ec955..7031307becd7 100644 > --- a/drivers/acpi/custom_method.c > +++ b/drivers/acpi/custom_method.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > > #include "internal.h" > > @@ -29,6 +30,11 @@ static ssize_t cm_write(struct file *file, const char > __user * user_buf, > > struct acpi_table_header table; > acpi_status status; > + int ret; > + > + ret = security_locked_down(LOCKDOWN_ACPI_TABLES); > + if (ret) > + return ret; > > if (!(*ppos)) { > /* parse the table header to get the table length */ > diff --git a/include/linux/security.h b/include/linux/security.h > index 010637a79eac..390e39395112 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -110,6 +110,7 @@ enum lockdown_reason { > LOCKDOWN_PCI_ACCESS, > LOCKDOWN_IOPORT, > LOCKDOWN_MSR, > + LOCKDOWN_ACPI_TABLES, > LOCKDOWN_INTEGRITY_MAX, > LOCKDOWN_CONFIDENTIALITY_MAX, > }; > diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c > index b1c1c72440d5..6d44db0ddffa 100644 > --- a/security/lockdown/lockdown.c > +++ b/security/lockdown/lockdown.c > @@ -25,6 +25,7 @@ static char > *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = { > [LOCKDOWN_PCI_ACCESS] = "direct PCI access", > [LOCKDOWN_IOPORT] = "raw io port access", > [LOCKDOWN_MSR] = "raw MSR access", > + [LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables", > [LOCKDOWN_INTEGRITY_MAX] = "integrity", > [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality", > }; >
[PATCH] media: saa7134: keep demod i2c gate closed on Medion 7134
Medion 7134 has two i2c eeproms on the same i2c bus sharing the same bus addresses: the first one for SAA7134 chip config and the second one behind TDA10046 DVB-T demod chip i2c gate storing its firmware. The TV tuner on this board is not behind this i2c gate. Due to the bus conflict described above, the card PCI SVID / SSID sometimes gets garbled after a reboot, which makes it necessary to specify the card model manually as an insmod option in order for it to be detected reliably. To avoid this, let's just leave the gate permanently closed so the eeprom chips won't clash. The demod firmware load is done with its i2c gate closed anyway so it is not affected by this change. Signed-off-by: Maciej S. Szmigiero --- This replaces the "media: saa7134: keep demod i2c gate open on Medion 7134" submission as it used an inverted open / closed terminology by mistake. drivers/media/pci/saa7134/saa7134-dvb.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/media/pci/saa7134/saa7134-dvb.c b/drivers/media/pci/saa7134/saa7134-dvb.c index eb8377a95023..f359cd5c006a 100644 --- a/drivers/media/pci/saa7134/saa7134-dvb.c +++ b/drivers/media/pci/saa7134/saa7134-dvb.c @@ -1264,6 +1264,20 @@ static int dvb_init(struct saa7134_dev *dev) _cardbus, >i2c_adap); if (fe0->dvb.frontend) { + /* +* The TV tuner on this board is actually NOT +* behind the demod i2c gate. +* However, the demod EEPROM is indeed there and it +* conflicts with the SAA7134 chip config EEPROM +* if the i2c gate is open (since they have same +* bus addresses) resulting in card PCI SVID / SSID +* being garbage after a reboot from time to time. +* +* Let's just leave the gate permanently closed - +* saa7134_i2c_eeprom_md7134_gate() will close it for +* us at probe time if it was open for some reason. +*/ + fe0->dvb.frontend->ops.i2c_gate_ctrl = NULL; dvb_attach(simple_tuner_attach, fe0->dvb.frontend, >i2c_adap, medion_cardbus.tuner_address, TUNER_PHILIPS_FMD1216ME_MK3);
[PATCH] media: saa7134: fix terminology around saa7134_i2c_eeprom_md7134_gate()
saa7134_i2c_eeprom_md7134_gate() function and the associated comment uses an inverted i2c gate open / closed terminology. Let's fix this. Signed-off-by: Maciej S. Szmigiero --- drivers/media/pci/saa7134/saa7134-i2c.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/media/pci/saa7134/saa7134-i2c.c b/drivers/media/pci/saa7134/saa7134-i2c.c index 493b1858815f..1414d580c98f 100644 --- a/drivers/media/pci/saa7134/saa7134-i2c.c +++ b/drivers/media/pci/saa7134/saa7134-i2c.c @@ -342,7 +342,11 @@ static const struct i2c_client saa7134_client_template = { /* --- */ -/* On Medion 7134 reading EEPROM needs DVB-T demod i2c gate open */ +/* + * On Medion 7134 reading the SAA7134 chip config EEPROM needs DVB-T + * demod i2c gate closed due to an address clash between this EEPROM + * and the demod one. + */ static void saa7134_i2c_eeprom_md7134_gate(struct saa7134_dev *dev) { u8 subaddr = 0x7, dmdregval; @@ -359,13 +363,13 @@ static void saa7134_i2c_eeprom_md7134_gate(struct saa7134_dev *dev) ret = i2c_transfer(>i2c_adap, i2cgatemsg_r, 2); if ((ret == 2) && (dmdregval & 0x2)) { - pr_debug("%s: DVB-T demod i2c gate was left closed\n", + pr_debug("%s: DVB-T demod i2c gate was left open\n", dev->name); data[0] = subaddr; data[1] = (dmdregval & ~0x2); if (i2c_transfer(>i2c_adap, i2cgatemsg_w, 1) != 1) - pr_err("%s: EEPROM i2c gate open failure\n", + pr_err("%s: EEPROM i2c gate close failure\n", dev->name); } }
Linux 3.16.73
I'm announcing the release of the 3.16.73 kernel. All users of the 3.16 kernel series should upgrade. The updated 3.16.y git tree can be found at: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git linux-3.16.y and can be browsed at the normal kernel.org git web browser: https://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git The diff from 3.16.72 is attached to this message. Ben. Documentation/siphash.txt | 75 +++ Makefile | 2 +- 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, 572 insertions(+), 27 deletions(-) Ben Hutchings (2): tcp: Clear sk_send_head after purging the write queue Linux 3.16.73 Jason A. Donenfeld (1): siphash: implement HalfSipHash1-3 for hash tables zhangyi (F) (2): ext4: brelse all indirect buffer in ext4_ind_remove_space() ext4: cleanup bh release code in ext4_ind_remove_space() -- Ben Hutchings Experience is what causes a person to make new mistakes instead of old ones. diff --git a/Documentation/siphash.txt b/Documentation/siphash.txt index e8e6ddbbaab4..908d348ff777 100644 --- a/Documentation/siphash.txt +++ b/Documentation/siphash.txt @@ -98,3 +98,78 @@ u64 h = siphash(, offsetofend(typeof(combined), dport), ); 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(, 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(>key, sizeof(table->key)); +} + +static inline hlist_head *some_hashtable_bucket(struct some_hashtable *table, struct interesting_input *input) +{ + return >hashtable[hsiphash(input, sizeof(*input), >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. diff --git a/Makefile b/Makefile index e2d6e0b9f22d..935fc9df7b17 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ VERSION = 3 PATCHLEVEL = 16 -SUBLEVEL = 72 +SUBLEVEL = 73 EXTRAVERSION = NAME = Museum of Fishiegoodies diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c index 8df46f49a3d5..475a1d40f23e 100644 --- a/fs/ext4/indirect.c +++ b/fs/ext4/indirect.c @@ -1313,6 +1313,7 @@ int ext4_ind_remove_space(handle_t *handle, struct inode *inode, 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 *handle,
Re: [RFC PATCH 2/2] ELF: Add ELF program property parsing support
On Tue, 2019-08-20 at 10:57 +0100, Dave Martin wrote: > ELF program properties will needed for detecting whether to enable > optional architecture or ABI features for a new ELF process. > > For now, there are no generic properties that we care about, so do > nothing unless CONFIG_ARCH_USE_GNU_PROPERTY=y. > > Otherwise, the presence of properties using the PT_PROGRAM_PROPERTY > phdrs entry (if any), and notify each property to the arch code. > > For now, the added code is not used. > > Signed-off-by: Dave Martin > --- > fs/binfmt_elf.c | 109 > +++ > fs/compat_binfmt_elf.c | 4 ++ > include/linux/elf.h | 21 + > include/uapi/linux/elf.h | 4 ++ > 4 files changed, 138 insertions(+) > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index d4e11b2..52f4b96 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -39,12 +39,18 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > #include > #include > > +#ifndef ELF_COMPAT > +#define ELF_COMPAT 0 > +#endif > + > #ifndef user_long_t > #define user_long_t long > #endif > @@ -690,6 +696,93 @@ static unsigned long randomize_stack_top(unsigned long > stack_top) > #endif > } > > +static int parse_elf_property(const void **prop, size_t *notesz, > + struct arch_elf_state *arch) > +{ > + const struct gnu_property *pr = *prop; > + size_t sz = *notesz; > + int ret; > + size_t step; > + > + BUG_ON(sz < sizeof(*pr)); > + > + if (sizeof(*pr) > sz) > + return -EIO; > + > + if (pr->pr_datasz > sz - sizeof(*pr)) > + return -EIO; > + > + step = round_up(sizeof(*pr) + pr->pr_datasz, elf_gnu_property_align); > + if (step > sz) > + return -EIO; > + > + ret = arch_parse_elf_property(pr->pr_type, *prop + sizeof(*pr), > + pr->pr_datasz, ELF_COMPAT, arch); > + if (ret) > + return ret; > + > + *prop += step; > + *notesz -= step; > + return 0; > +} > + > +#define NOTE_DATA_SZ SZ_1K > +#define GNU_PROPERTY_TYPE_0_NAME "GNU" > +#define NOTE_NAME_SZ (sizeof(GNU_PROPERTY_TYPE_0_NAME)) > + > +static int parse_elf_properties(struct file *f, const struct elf_phdr *phdr, > + struct arch_elf_state *arch) > +{ > + ssize_t n; > + loff_t pos = phdr->p_offset; > + union { > + struct elf_note nhdr; > + char data[NOTE_DATA_SZ]; > + } note; > + size_t off, notesz; > + const void *prop; > + int ret; > + > + if (!IS_ENABLED(ARCH_USE_GNU_PROPERTY)) > + return 0; > + > + BUG_ON(phdr->p_type != PT_GNU_PROPERTY); > + > + /* If the properties are crazy large, that's too bad (for now): */ > + if (phdr->p_filesz > sizeof(note)) > + return -ENOEXEC; > + n = kernel_read(f, , phdr->p_filesz, ); > + > + BUILD_BUG_ON(sizeof(note) < sizeof(note.nhdr) + NOTE_NAME_SZ); > + if (n < 0 || n < sizeof(note.nhdr) + NOTE_NAME_SZ) > + return -EIO; > + > + if (note.nhdr.n_type != NT_GNU_PROPERTY_TYPE_0 || > + note.nhdr.n_namesz != NOTE_NAME_SZ || > + strncmp(note.data + sizeof(note.nhdr), > + GNU_PROPERTY_TYPE_0_NAME, n - sizeof(note.nhdr))) > + return -EIO; > + > + off = round_up(sizeof(note.nhdr) + NOTE_NAME_SZ, > +elf_gnu_property_align); > + if (off > n) > + return -EIO; > + > + prop = (const struct gnu_property *)(note.data + off); > + notesz = n - off; > + if (note.nhdr.n_descsz > notesz) > + return -EIO; > + > + while (notesz) { > + BUG_ON(((char *)prop - note.data) % elf_gnu_property_align); > + ret = parse_elf_property(, , arch); Properties need to be in ascending order. Can we keep track of it from here. Also, can we replace BUG_ON with returning an error. Thanks, Yu-cheng
[PATCH v4 bpf-next 2/4] libbpf: Add helpers to extract perf fd from bpf_link
It is sometimes necessary to perform ioctl's on the underlying perf fd. There is not currently a way to extract the fd given a bpf_link, so add a a pair of casting and getting helpers. The casting and getting helpers are nice because they let us define broad categories of links that makes it clear to users what they can expect to extract from what type of link. Acked-by: Song Liu Acked-by: Andrii Nakryiko Signed-off-by: Daniel Xu --- tools/lib/bpf/libbpf.c | 21 + tools/lib/bpf/libbpf.h | 13 + tools/lib/bpf/libbpf.map | 3 +++ 3 files changed, 37 insertions(+) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 2233f919dd88..41588e13be2b 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -4876,6 +4876,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr, struct bpf_link { int (*destroy)(struct bpf_link *link); + enum bpf_link_type type; }; int bpf_link__destroy(struct bpf_link *link) @@ -4909,6 +4910,24 @@ static int bpf_link__destroy_perf_event(struct bpf_link *link) return err; } +const struct bpf_link_fd *bpf_link__as_fd(const struct bpf_link *link) +{ + if (link->type != LIBBPF_LINK_FD) + return NULL; + + return (struct bpf_link_fd *)link; +} + +enum bpf_link_type bpf_link__type(const struct bpf_link *link) +{ + return link->type; +} + +int bpf_link_fd__fd(const struct bpf_link_fd *link) +{ + return link->fd; +} + struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog, int pfd) { @@ -4932,6 +4951,7 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog, if (!link) return ERR_PTR(-ENOMEM); link->link.destroy = _link__destroy_perf_event; + link->link.type = LIBBPF_LINK_FD; link->fd = pfd; if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) { @@ -5225,6 +5245,7 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog, link = malloc(sizeof(*link)); if (!link) return ERR_PTR(-ENOMEM); + link->link.type = LIBBPF_LINK_FD; link->link.destroy = _link__destroy_fd; pfd = bpf_raw_tracepoint_open(tp_name, prog_fd); diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index e8f70977d137..2ddef5315ff9 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -166,7 +166,20 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path); LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path); LIBBPF_API void bpf_program__unload(struct bpf_program *prog); +enum bpf_link_type { + LIBBPF_LINK_FD, +}; + struct bpf_link; +struct bpf_link_fd; + +/* casting APIs */ +LIBBPF_API const struct bpf_link_fd * +bpf_link__as_fd(const struct bpf_link *link); + +/* getters APIs */ +LIBBPF_API enum bpf_link_type bpf_link__type(const struct bpf_link *link); +LIBBPF_API int bpf_link_fd__fd(const struct bpf_link_fd *link); LIBBPF_API int bpf_link__destroy(struct bpf_link *link); diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index 664ce8e7a60e..ed169579896f 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -188,4 +188,7 @@ LIBBPF_0.0.4 { LIBBPF_0.0.5 { global: bpf_btf_get_next_id; + bpf_link__type; + bpf_link__as_fd; + bpf_link_fd__fd; } LIBBPF_0.0.4; -- 2.21.0
[PATCH v4 bpf-next 4/4] tracing/probe: Add self test for PERF_EVENT_IOC_QUERY_PROBE
Acked-by: Andrii Nakryiko Signed-off-by: Daniel Xu --- .../selftests/bpf/prog_tests/attach_probe.c | 115 ++ 1 file changed, 115 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c index 5ecc267d98b0..414d5f4f7ccd 100644 --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c @@ -27,17 +27,27 @@ void test_attach_probe(void) const char *kretprobe_name = "kretprobe/sys_nanosleep"; const char *uprobe_name = "uprobe/trigger_func"; const char *uretprobe_name = "uretprobe/trigger_func"; + struct perf_event_query_probe kprobe_query = {}; + struct perf_event_query_probe kretprobe_query = {}; + struct perf_event_query_probe uprobe_query = {}; + struct perf_event_query_probe uretprobe_query = {}; const int kprobe_idx = 0, kretprobe_idx = 1; const int uprobe_idx = 2, uretprobe_idx = 3; const char *file = "./test_attach_probe.o"; struct bpf_program *kprobe_prog, *kretprobe_prog; struct bpf_program *uprobe_prog, *uretprobe_prog; struct bpf_object *obj; + const struct bpf_link_fd *kprobe_fd_link; + const struct bpf_link_fd *kretprobe_fd_link; + const struct bpf_link_fd *uprobe_fd_link; + const struct bpf_link_fd *uretprobe_fd_link; int err, prog_fd, duration = 0, res; struct bpf_link *kprobe_link = NULL; struct bpf_link *kretprobe_link = NULL; struct bpf_link *uprobe_link = NULL; struct bpf_link *uretprobe_link = NULL; + int kprobe_fd, kretprobe_fd; + int uprobe_fd, uretprobe_fd; int results_map_fd; size_t uprobe_offset; ssize_t base_addr; @@ -116,6 +126,63 @@ void test_attach_probe(void) /* trigger & validate kprobe && kretprobe */ usleep(1); + kprobe_fd_link = bpf_link__as_fd(kprobe_link); + if (CHECK(!kprobe_fd_link, "kprobe_link_as_fd", + "failed to cast link to fd link\n")) + goto cleanup; + + kprobe_fd = bpf_link_fd__fd(kprobe_fd_link); + if (CHECK(kprobe_fd < 0, "kprobe_get_perf_fd", + "failed to get perf fd from kprobe link\n")) + goto cleanup; + + kretprobe_fd_link = bpf_link__as_fd(kretprobe_link); + if (CHECK(!kretprobe_fd_link, "kretprobe_link_as_fd", + "failed to cast link to fd link\n")) + goto cleanup; + + kretprobe_fd = bpf_link_fd__fd(kretprobe_fd_link); + if (CHECK(kretprobe_fd < 0, "kretprobe_get_perf_fd", + "failed to get perf fd from kretprobe link\n")) + goto cleanup; + + kprobe_query.size = sizeof(kprobe_query); + err = ioctl(kprobe_fd, PERF_EVENT_IOC_QUERY_PROBE, _query); + if (CHECK(err, "get_kprobe_ioctl", + "failed to issue kprobe query ioctl\n")) + goto cleanup; + if (CHECK(kprobe_query.size != sizeof(kprobe_query), "get_kprobe_ioctl", + "read incorrect size from kprobe_ioctl: %llu\n", + kprobe_query.size)) + goto cleanup; + if (CHECK(kprobe_query.nmissed > 0, "get_kprobe_ioctl", + "read incorrect nmissed from kprobe_ioctl: %llu\n", + kprobe_query.nmissed)) + goto cleanup; + if (CHECK(kprobe_query.nhit == 0, "get_kprobe_ioctl", + "read incorrect nhit from kprobe_ioctl: %llu\n", + kprobe_query.nhit)) + goto cleanup; + + kretprobe_query.size = sizeof(kretprobe_query); + err = ioctl(kretprobe_fd, PERF_EVENT_IOC_QUERY_PROBE, _query); + if (CHECK(err, "get_kretprobe_ioctl", + "failed to issue kretprobe query ioctl\n")) + goto cleanup; + if (CHECK(kretprobe_query.size != sizeof(kretprobe_query), + "get_kretprobe_ioctl", + "read incorrect size from kretprobe_ioctl: %llu\n", + kretprobe_query.size)) + goto cleanup; + if (CHECK(kretprobe_query.nmissed > 0, "get_kretprobe_ioctl", + "read incorrect nmissed from kretprobe_ioctl: %llu\n", + kretprobe_query.nmissed)) + goto cleanup; + if (CHECK(kretprobe_query.nhit <= 0, "get_kretprobe_ioctl", + "read incorrect nhit from kretprobe_ioctl: %llu\n", + kretprobe_query.nhit)) + goto cleanup; + err = bpf_map_lookup_elem(results_map_fd, _idx, ); if (CHECK(err, "get_kprobe_res", "failed to get kprobe res: %d\n", err)) @@ -135,6 +202,54 @@ void test_attach_probe(void) /* trigger & validate uprobe & uretprobe */ get_base_addr(); + uprobe_fd_link = bpf_link__as_fd(uprobe_link); + if (CHECK(!uprobe_fd_link, "uprobe_link_as_fd", + "failed to
[PATCH v4 bpf-next 3/4] tracing/probe: Sync perf_event.h to tools
Signed-off-by: Daniel Xu --- tools/include/uapi/linux/perf_event.h | 23 +++ 1 file changed, 23 insertions(+) diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h index 7198ddd0c6b1..8783d29a807a 100644 --- a/tools/include/uapi/linux/perf_event.h +++ b/tools/include/uapi/linux/perf_event.h @@ -447,6 +447,28 @@ struct perf_event_query_bpf { __u32 ids[0]; }; +/* + * Structure used by below PERF_EVENT_IOC_QUERY_PROBE command + * to query information about the probe attached to the perf + * event. Currently only supports [uk]probes. + */ +struct perf_event_query_probe { + /* +* Size of structure for forward/backward compatibility +*/ + __u64 size; + /* +* Set by the kernel to indicate number of times this probe +* was temporarily disabled +*/ + __u64 nmissed; + /* +* Set by the kernel to indicate number of times this probe +* was hit +*/ + __u64 nhit; +}; + /* * Ioctls that can be done on a perf event fd: */ @@ -462,6 +484,7 @@ struct perf_event_query_bpf { #define PERF_EVENT_IOC_PAUSE_OUTPUT_IOW('$', 9, __u32) #define PERF_EVENT_IOC_QUERY_BPF _IOWR('$', 10, struct perf_event_query_bpf *) #define PERF_EVENT_IOC_MODIFY_ATTRIBUTES _IOW('$', 11, struct perf_event_attr *) +#define PERF_EVENT_IOC_QUERY_PROBE _IOR('$', 12, struct perf_event_query_probe *) enum perf_event_ioc_flags { PERF_IOC_FLAG_GROUP = 1U << 0, -- 2.21.0
[PATCH v4 bpf-next 1/4] tracing/probe: Add PERF_EVENT_IOC_QUERY_PROBE ioctl
It's useful to know [uk]probe's nmissed and nhit stats. For example with tracing tools, it's important to know when events may have been lost. debugfs currently exposes a control file to get this information, but it is not compatible with probes registered with the perf API. While bpf programs may be able to manually count nhit, there is no way to gather nmissed. In other words, it is currently not possible to retrieve information about FD-based probes. This patch adds a new ioctl that lets users query nmissed (as well as nhit for completeness). We currently only add support for [uk]probes but leave the possibility open for other probes like tracepoint. Signed-off-by: Daniel Xu --- include/linux/trace_events.h| 2 ++ include/uapi/linux/perf_event.h | 23 +++ kernel/events/core.c| 20 kernel/trace/trace_kprobe.c | 25 + kernel/trace/trace_uprobe.c | 25 + 5 files changed, 95 insertions(+) diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index 5150436783e8..96f3cf2e39b4 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h @@ -586,6 +586,7 @@ extern int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type, const char **symbol, u64 *probe_offset, u64 *probe_addr, bool perf_type_tracepoint); +extern int perf_kprobe_event_query(struct perf_event *event, void __user *info); #endif #ifdef CONFIG_UPROBE_EVENTS extern int perf_uprobe_init(struct perf_event *event, @@ -594,6 +595,7 @@ extern void perf_uprobe_destroy(struct perf_event *event); extern int bpf_get_uprobe_info(const struct perf_event *event, u32 *fd_type, const char **filename, u64 *probe_offset, bool perf_type_tracepoint); +extern int perf_uprobe_event_query(struct perf_event *event, void __user *info); #endif extern int ftrace_profile_set_filter(struct perf_event *event, int event_id, char *filter_str); diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h index 7198ddd0c6b1..8783d29a807a 100644 --- a/include/uapi/linux/perf_event.h +++ b/include/uapi/linux/perf_event.h @@ -447,6 +447,28 @@ struct perf_event_query_bpf { __u32 ids[0]; }; +/* + * Structure used by below PERF_EVENT_IOC_QUERY_PROBE command + * to query information about the probe attached to the perf + * event. Currently only supports [uk]probes. + */ +struct perf_event_query_probe { + /* +* Size of structure for forward/backward compatibility +*/ + __u64 size; + /* +* Set by the kernel to indicate number of times this probe +* was temporarily disabled +*/ + __u64 nmissed; + /* +* Set by the kernel to indicate number of times this probe +* was hit +*/ + __u64 nhit; +}; + /* * Ioctls that can be done on a perf event fd: */ @@ -462,6 +484,7 @@ struct perf_event_query_bpf { #define PERF_EVENT_IOC_PAUSE_OUTPUT_IOW('$', 9, __u32) #define PERF_EVENT_IOC_QUERY_BPF _IOWR('$', 10, struct perf_event_query_bpf *) #define PERF_EVENT_IOC_MODIFY_ATTRIBUTES _IOW('$', 11, struct perf_event_attr *) +#define PERF_EVENT_IOC_QUERY_PROBE _IOR('$', 12, struct perf_event_query_probe *) enum perf_event_ioc_flags { PERF_IOC_FLAG_GROUP = 1U << 0, diff --git a/kernel/events/core.c b/kernel/events/core.c index 0463c1151bae..ed33d50511a3 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5060,6 +5060,8 @@ static int perf_event_set_filter(struct perf_event *event, void __user *arg); static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd); static int perf_copy_attr(struct perf_event_attr __user *uattr, struct perf_event_attr *attr); +static int perf_probe_event_query(struct perf_event *event, + void __user *info); static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg) { @@ -5143,6 +5145,10 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon return perf_event_modify_attr(event, _attr); } +#if defined(CONFIG_KPROBE_EVENTS) || defined(CONFIG_UPROBE_EVENTS) + case PERF_EVENT_IOC_QUERY_PROBE: + return perf_probe_event_query(event, (void __user *)arg); +#endif default: return -ENOTTY; } @@ -8833,6 +8839,20 @@ static inline void perf_tp_register(void) #endif } +static int perf_probe_event_query(struct perf_event *event, + void __user *info) +{ +#ifdef CONFIG_KPROBE_EVENTS + if (event->attr.type == perf_kprobe.type) + return
[PATCH v4 bpf-next 0/4] tracing/probe: Add PERF_EVENT_IOC_QUERY_PROBE
It's useful to know [uk]probe's nmissed and nhit stats. For example with tracing tools, it's important to know when events may have been lost. debugfs currently exposes a control file to get this information, but it is not compatible with probes registered with the perf API. While bpf programs may be able to manually count nhit, there is no way to gather nmissed. In other words, it is currently not possible to retrieve information about FD-based probes. This patch adds a new ioctl that lets users query nmissed (as well as nhit for completeness). We currently only add support for [uk]probes but leave the possibility open for other probes like tracepoint. v3 -> v4: - Make kernel code set size field on ioctl arg - Update selftests to check size field - Remove unnecessary function stubs v2 -> v3: - Introduce bpf_link_type and associated getter to track underlying link types - Add back size field in perf_event_query_probe for forward/backwards compat - Remove NULL checks, fix typos v1 -> v2: - More descriptive cover letter - Make API more generic and support uprobes as well - Use casters/getters for libbpf instead of single getter - Fix typos - Remove size field from ioctl struct - Split out libbpf.h sync to tools dir to separate commit Daniel Xu (4): tracing/probe: Add PERF_EVENT_IOC_QUERY_PROBE ioctl libbpf: Add helpers to extract perf fd from bpf_link tracing/probe: Sync perf_event.h to tools tracing/probe: Add self test for PERF_EVENT_IOC_QUERY_PROBE include/linux/trace_events.h | 2 + include/uapi/linux/perf_event.h | 23 kernel/events/core.c | 20 +++ kernel/trace/trace_kprobe.c | 25 kernel/trace/trace_uprobe.c | 25 tools/include/uapi/linux/perf_event.h | 23 tools/lib/bpf/libbpf.c| 21 tools/lib/bpf/libbpf.h| 13 ++ tools/lib/bpf/libbpf.map | 3 + .../selftests/bpf/prog_tests/attach_probe.c | 115 ++ 10 files changed, 270 insertions(+) -- 2.21.0
Re: [patch 04/44] posix-cpu-timers: Fixup stale comment
On Tue, 20 Aug 2019, Frederic Weisbecker wrote: > On Tue, Aug 20, 2019 at 07:57:37PM +0200, Thomas Gleixner wrote: > > On Tue, 20 Aug 2019, Frederic Weisbecker wrote: > > > On Mon, Aug 19, 2019 at 04:31:45PM +0200, Thomas Gleixner wrote: > > > > /* > > > > - * Clean out CPU timers still ticking when a thread exited. The task > > > > - * pointer is cleared, and the expiry time is replaced with the > > > > residual > > > > - * time for later timer_gettime calls to return. > > > > + * Clean out CPU timers which are still armed when a thread exits. The > > > > + * timers are only removed from the list. No other updates are done. > > > > The > > > > + * corresponding posix timers are still accessible, but cannot be > > > > rearmed. > > > > + * > > > > * This must be called with the siglock held. > > > > */ > > > > static void cleanup_timers(struct list_head *head) > > > > > > Indeed and I believe we could avoid that step. We remove the sighand at > > > the same > > > time so those can't be accessed anymore anyway. > > > > > > exit_itimers() takes care of the last call release and could force remove > > > from > > > the list (although it might be taken care of in your series, haven't > > > checked yet): > > > > No. The posix timer is not necessarily owned by the exiting task or > > process. It can be owned by a different entity which has permissions, > > e.g. parent. > > > > So those are not in the posix timer list of the exiting task, which gets > > cleaned up in exit_itimers(). Those are in the list of the task which armed > > the timer. The timer is merily queued in the 'active timers' list of the > > exiting task and posix_cpu_timers_exit()/posix_cpu_timers_exit_group() > > remove it before the task/signal structs go away. > > Sure, I understand there's two distinct things here: the owner that queues > timers in owner->sig->posix_timers (cleaned in exit_itimers()) and the target > that queues > in target->[signal->]cputime_expires (cleaned in > posix_cpu_timers_exit[_group](). > > So I'm wondering why we bother with posix_cpu_timers_exit[_group]() at > all when exit_itimers() could handle the list deletion from > target->[signal]->cputime_expires throughout posix_cpu_timer_del() as it > already does on targets that still have their sighands. No it can't do that throughout posix_cpu_timer_del() because exit_itimers() can only look at current->signal->posix_timers which does not contain the posix timers owned by a different task/process. We could of course invoke posix_cpu_timers_exit() from exit_itimers() but does that buy anything? > It would make things more simple to delete the timer off the target from > the same caller and place and we could remove posix_cpu_timers_exit*(). We can't. The foreign owned cpu timers are not in cur->signal->posix_timers so how should we invoke posix_cpu_timer_del() on them. Only the owner task can. The only thing the exiting task can do is to remove the foreign timer from it's expiry list which has nothing to do with cur->signal->posix_timers. cur->signal->posix_timers only contains posix timers which are owned by current not those which are owned by a different task and armed on the exiting one. exit_itimers() handles cur->signal->posix_timers, i.e. timers owned by current. posix_cpu_timers_exit() handles timers enqueued on current, which are foreign owned timers because exit_itimers() removed those which were owned by current already. posix_cpu_timers_exit_group() handles timers enqueued on current->signal, which are foreign owned timers because exit_itimers() removed those which were owned by current already. > Or is there something I'm awkwardly missing as usual? :-) I think so :) Thanks, tglx
Re: [PATCH] media: saa7134: keep demod i2c gate open on Medion 7134
Hi Matthias, On 20.08.2019 21:54, Matthias Schwarzott wrote: > Hi Maciej, > > some comment about wording in commit message and code-comment. > > As far as I know the terms are defined like this: > * gate open = i2c-clients behind gate can be reached > * gate closed = i2c-clients behind gate are not reachable I always thought that this terminology is like the one used for a switch: if it is closed then the signal can pass, if open then it blocks the signal but apparently it is literally like a physical gate, so you are obviously right here - thanks for pointing this out. Will respin this patch and also add a second one fixing the terminology already present in saa7134_i2c_eeprom_md7134_gate(). > Regards > Matthias Regards, Maciej
Re: [PATCH V40 10/29] hibernate: Disable when the kernel is locked down
On Tuesday, August 20, 2019 2:17:46 AM CEST Matthew Garrett wrote: > From: Josh Boyer > > There is currently no way to verify the resume image when returning > from hibernate. This might compromise the signed modules trust model, > so until we can work with signed hibernate images we disable it when the > kernel is locked down. > > Signed-off-by: Josh Boyer > Signed-off-by: David Howells > Signed-off-by: Matthew Garrett > Reviewed-by: Kees Cook > Cc: r...@rjwysocki.net > Cc: pa...@ucw.cz > cc: linux...@vger.kernel.org > Signed-off-by: James Morris Acked-by: Rafael J. Wysocki > --- > include/linux/security.h | 1 + > kernel/power/hibernate.c | 3 ++- > security/lockdown/lockdown.c | 1 + > 3 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/linux/security.h b/include/linux/security.h > index b607a8ac97fe..80ac7fb27aa9 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -106,6 +106,7 @@ enum lockdown_reason { > LOCKDOWN_MODULE_SIGNATURE, > LOCKDOWN_DEV_MEM, > LOCKDOWN_KEXEC, > + LOCKDOWN_HIBERNATION, > LOCKDOWN_INTEGRITY_MAX, > LOCKDOWN_CONFIDENTIALITY_MAX, > }; > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index cd7434e6000d..3c0a5a8170b0 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > #include > > #include "power.h" > @@ -68,7 +69,7 @@ static const struct platform_hibernation_ops > *hibernation_ops; > > bool hibernation_available(void) > { > - return (nohibernate == 0); > + return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION); > } > > /** > diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c > index aaf30ad351f9..3462f7edcaac 100644 > --- a/security/lockdown/lockdown.c > +++ b/security/lockdown/lockdown.c > @@ -21,6 +21,7 @@ static char > *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = { > [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading", > [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port", > [LOCKDOWN_KEXEC] = "kexec of unsigned images", > + [LOCKDOWN_HIBERNATION] = "hibernation", > [LOCKDOWN_INTEGRITY_MAX] = "integrity", > [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality", > }; >
RE: [RFC] mm: Proactive compaction
> -Original Message- > From: Vlastimil Babka > Sent: Tuesday, August 20, 2019 1:46 AM > To: Nitin Gupta ; a...@linux-foundation.org; > mgor...@techsingularity.net; mho...@suse.com; > dan.j.willi...@intel.com > Cc: Yu Zhao ; Matthew Wilcox ; > Qian Cai ; Andrey Ryabinin ; Roman > Gushchin ; Greg Kroah-Hartman > ; Kees Cook ; Jann > Horn ; Johannes Weiner ; Arun > KS ; Janne Huttunen > ; Konstantin Khlebnikov > ; linux-kernel@vger.kernel.org; linux- > m...@kvack.org; Khalid Aziz > Subject: Re: [RFC] mm: Proactive compaction > > +CC Khalid Aziz who proposed a different approach: > https://lore.kernel.org/linux-mm/20190813014012.30232-1- > khalid.a...@oracle.com/T/#u > > On 8/16/19 11:43 PM, Nitin Gupta wrote: > > For some applications we need to allocate almost all memory as > > hugepages. However, on a running system, higher order allocations can > > fail if the memory is fragmented. Linux kernel currently does > > on-demand compaction as we request more hugepages but this style of > > compaction incurs very high latency. Experiments with one-time full > > memory compaction (followed by hugepage allocations) shows that kernel > > is able to restore a highly fragmented memory state to a fairly > > compacted memory state within <1 sec for a 32G system. Such data > > suggests that a more proactive compaction can help us allocate a large > > fraction of memory as hugepages keeping allocation latencies low. > > > > For a more proactive compaction, the approach taken here is to define > > per page-order external fragmentation thresholds and let kcompactd > > threads act on these thresholds. > > > > The low and high thresholds are defined per page-order and exposed > > through sysfs: > > > > /sys/kernel/mm/compaction/order-[1..MAX_ORDER]/extfrag_{low,high} > > > > Per-node kcompactd thread is woken up every few seconds to check if > > any zone on its node has extfrag above the extfrag_high threshold for > > any order, in which case the thread starts compaction in the backgrond > > till all zones are below extfrag_low level for all orders. By default > > both these thresolds are set to 100 for all orders which essentially > > disables kcompactd. > > Could you define what exactly extfrag is, in the changelog? > extfrag for order-n = ((total free pages) - (free pages for order >= n)) / (total free pages) * 100; I will add this to v2 changelog. > > To avoid wasting CPU cycles when compaction cannot help, such as when > > memory is full, we check both, extfrag > extfrag_high and > > compaction_suitable(zone). This allows kcomapctd thread to stays > > inactive even if extfrag thresholds are not met. > > How does it translate to e.g. the number of free pages of order? > Watermarks are checked as follows (see: __compaction_suitable) watermark = (order > PAGE_ALLOC_COSTLY_ORDER) ? low_wmark_pages(zone) : min_wmark_pages(zone); If a zone does not satisfy this watermark, we don't start compaction. > > This patch is largely based on ideas from Michal Hocko posted here: > > https://lore.kernel.org/linux- > mm/20161230131412.gi13...@dhcp22.suse.cz > > / > > > > Testing done (on x86): > > - Set /sys/kernel/mm/compaction/order-9/extfrag_{low,high} = {25, 30} > > respectively. > > - Use a test program to fragment memory: the program allocates all > > memory and then for each 2M aligned section, frees 3/4 of base pages > > using munmap. > > - kcompactd0 detects fragmentation for order-9 > extfrag_high and > > starts compaction till extfrag < extfrag_low for order-9. > > > > The patch has plenty of rough edges but posting it early to see if I'm > > going in the right direction and to get some early feedback. > > That's a lot of control knobs - how is an admin supposed to tune them to > their needs? I expect that a workload would typically care for just a particular page order (say, order-9 on x86 for the default hugepage size). An admin can set extfrag_{low,high} for just that order (say, low=25, high=30) and leave these thresholds to their default value (low=100, high=100) for all other orders. Thanks, Nitin > > (keeping the rest for reference) > > > Signed-off-by: Nitin Gupta > > --- > > include/linux/compaction.h | 12 ++ > > mm/compaction.c| 250 ++--- > > mm/vmstat.c| 12 ++ > > 3 files changed, 228 insertions(+), 46 deletions(-) > > > > diff --git a/include/linux/compaction.h b/include/linux/compaction.h > > index 9569e7c786d3..26bfedbbc64b 100644 > > --- a/include/linux/compaction.h > > +++ b/include/linux/compaction.h > > @@ -60,6 +60,17 @@ enum compact_result { > > > > struct alloc_context; /* in mm/internal.h */ > > > > +// "order-%d" > > +#define COMPACTION_ORDER_STATE_NAME_LEN 16 // Per-order > compaction > > +state struct compaction_order_state { > > + unsigned int order; > > + unsigned int extfrag_low; > > + unsigned int extfrag_high; > > + unsigned int extfrag_curr;
Re: [PATCH] ARM: UNWINDER_FRAME_POINTER implementation for Clang
On Tue, Aug 20, 2019 at 12:44 PM Nathan Huckleberry wrote: > > The stackframe setup when compiled with clang is different. > Since the stack unwinder expects the gcc stackframe setup it > fails to print backtraces. This patch adds support for the > clang stackframe setup. > > Link: https://github.com/ClangBuiltLinux/linux/issues/35 > Cc: clang-built-li...@googlegroups.com > Suggested-by: Tri Vo > Signed-off-by: Nathan Huckleberry > --- > Changes from RFC > * Push extra register to satisfy 8 byte alignment requirement > * Add clarifying comments > * Separate code into its own file Thanks for the patch! The added comments and moving the implementation to its own file make it easier to review. > > arch/arm/Kconfig.debug | 4 +- > arch/arm/Makefile | 5 +- > arch/arm/lib/Makefile | 8 +- > arch/arm/lib/backtrace-clang.S | 224 + > 4 files changed, 237 insertions(+), 4 deletions(-) > create mode 100644 arch/arm/lib/backtrace-clang.S > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > index 85710e078afb..92fca7463e21 100644 > --- a/arch/arm/Kconfig.debug > +++ b/arch/arm/Kconfig.debug > @@ -56,7 +56,7 @@ choice > > config UNWINDER_FRAME_POINTER > bool "Frame pointer unwinder" > - depends on !THUMB2_KERNEL && !CC_IS_CLANG > + depends on !THUMB2_KERNEL > select ARCH_WANT_FRAME_POINTERS > select FRAME_POINTER > help > @@ -1872,7 +1872,7 @@ config DEBUG_UNCOMPRESS > When this option is set, the selected DEBUG_LL output method > will be re-used for normal decompressor output on multiplatform > kernels. > - > + Probably can drop the added newline? > > config UNCOMPRESS_INCLUDE > string > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index c3624ca6c0bc..729e223b83fe 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -36,7 +36,10 @@ KBUILD_CFLAGS+= $(call > cc-option,-mno-unaligned-access) > endif > > ifeq ($(CONFIG_FRAME_POINTER),y) > -KBUILD_CFLAGS +=-fno-omit-frame-pointer -mapcs -mno-sched-prolog > +KBUILD_CFLAGS +=-fno-omit-frame-pointer > + ifeq ($(CONFIG_CC_IS_GCC),y) > + KBUILD_CFLAGS += -mapcs -mno-sched-prolog > + endif While I can appreciate the indentation, it's unusual to indent additional depths of kernel Makefiles. At least the rest of this file does not do so. Of course, the other Makefile you touch below does two spaces. At least try to keep the file internally consistent, even if the kernel itself is inconsistent. > endif > > ifeq ($(CONFIG_CPU_BIG_ENDIAN),y) > diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile > index b25c54585048..e10a769c72ec 100644 > --- a/arch/arm/lib/Makefile > +++ b/arch/arm/lib/Makefile > @@ -5,7 +5,7 @@ > # Copyright (C) 1995-2000 Russell King > # > > -lib-y := backtrace.o changebit.o csumipv6.o csumpartial.o \ > +lib-y := changebit.o csumipv6.o csumpartial.o \ >csumpartialcopy.o csumpartialcopyuser.o clearbit.o \ >delay.o delay-loop.o findbit.o memchr.o memcpy.o \ >memmove.o memset.o setbit.o\ > @@ -19,6 +19,12 @@ lib-y:= backtrace.o changebit.o csumipv6.o > csumpartial.o \ > mmu-y := clear_user.o copy_page.o getuser.o putuser.o \ >copy_from_user.o copy_to_user.o > > +ifdef CONFIG_CC_IS_CLANG > + lib-y += backtrace-clang.o > +else > + lib-y += backtrace.o > +endif The indentation should match the above (from this file). Looks like 1 tab after lib-y. See L34(CONFIG_CPU_32v3) for what I would have expected. > + > # using lib_ here won't override already available weak symbols > obj-$(CONFIG_UACCESS_WITH_MEMCPY) += uaccess_with_memcpy.o > > diff --git a/arch/arm/lib/backtrace-clang.S b/arch/arm/lib/backtrace-clang.S > new file mode 100644 > index ..2b02014dbdd1 > --- /dev/null > +++ b/arch/arm/lib/backtrace-clang.S > @@ -0,0 +1,224 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * linux/arch/arm/lib/backtrace-clang.S > + * > + * Copyright (C) 2019 Nathan Huckleberry > + * > + */ > +#include > +#include > +#include > + .text > + > +/* fp is 0 or stack frame */ ah, I see that the reference implementation uses an assembly comment here. Ok (sorry for the noise). > + > +#define frame r4 > +#define sv_fp r5 > +#define sv_pc r6 > +#define mask r7 > +#define sv_lr r8 > + > +ENTRY(c_backtrace) > + > +#if !defined(CONFIG_FRAME_POINTER) || !defined(CONFIG_PRINTK) > + ret lr > +ENDPROC(c_backtrace) > +#else > + > + > +/* > + * Clang does not store pc or sp in function prologues > + * so we don't know exactly where the function > + * starts. > + * We can treat the current frame's lr as the saved pc and the > + * preceding frame's lr as the lr, but we can't preceding frame's lr as
Re: [PATCH v3 0/8] PM / ACPI: sleep: Additional changes related to suspend-to-idle
On Tuesday, August 20, 2019 3:29:48 PM CEST Rafael J. Wysocki wrote: > On Tue, Aug 20, 2019 at 3:10 PM Kristian Klausen wrote: > > > > On 19.08.2019 22.41, Rafael J. Wysocki wrote: > > > On Mon, Aug 19, 2019 at 5:47 PM Kristian Klausen > > > wrote: > > >> On 19.08.2019 11.05, Rafael J. Wysocki wrote: > > >>> On Monday, August 19, 2019 9:59:02 AM CEST Rafael J. Wysocki wrote: > > On Fri, Aug 16, 2019 at 10:26 PM Kristian Klausen > > wrote: > > > On 02.08.2019 12.33, Rafael J. Wysocki wrote: > > >> Hi All, > > >> > > On top of the "Simplify the suspend-to-idle control flow" patch > > series > > posted previously: > > > > https://lore.kernel.org/lkml/71085220.z6FKkvYQPX@kreacher/ > > > > sanitize the suspend-to-idle flow even further. > > > > First off, decouple EC wakeup from the LPS0 _DSM processing (patch > > 1). > > > > Next, reorder the code to invoke LPS0 _DSM Functions 5 and 6 in the > > specification-compliant order with respect to suspending and > > resuming > > devices (patch 2). > > > > Finally, rearrange lps0_device_attach() (patch 3) and add a > > command line > > switch to prevent the LPS0 _DSM from being used. > > >>> The v2 is because I found a (minor) bug in patch 1, decided to use > > >>> a module > > >>> parameter instead of a kernel command line option in patch 4. > > >>> Also, there > > >>> are 4 new patches: > > >>> > > >>> Patch 5: Switch the EC over to polling during "noirq" suspend and > > >>> back > > >>> during "noirq" resume. > > >>> > > >>> Patch 6: Eliminate acpi_sleep_no_ec_events(). > > >>> > > >>> Patch 7: Consolidate some EC code depending on PM_SLEEP. > > >>> > > >>> Patch 8: Add EC GPE dispatching debug message. > > >> The v3 is just a rearranged v2 so as to move the post sensitive > > >> patch (previous patch 2) > > >> to the end of the series. [After applying the full series the code > > >> is the same as before.] > > >> > > >> For easier testing, the series (along with some previous patches > > >> depended on by it) > > >> is available in the pm-s2idle-testing branch of the linux-pm.git > > >> tree at kernel.org: > > >> > > >> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=pm-s2idle-testing > > > It was just testing this patch series(461fc1caed55), to see if it > > > would > > > fix my charging issue > > > (https://bugzilla.kernel.org/show_bug.cgi?id=201307), which it didn't. > > It is unlikely to help in that case. > > >> Do you have any idea what the issue could be? > > > Basically, there are two possibilities: either the OS is expected to > > > handle the AC/battery switching events, or the platform firmware > > > should take care of them. In the former case, the EC should generate > > > events to be handled by the OS and in the latter one there needs to be > > > a way to let the platform firmware that it needs to take care of those > > > events going forward. > > > > > > In either case there may be a platform-specific action to be carried > > > out during suspend and resume to set this up as expected which may be > > > missing. > > Thanks for the explanation. I don't think I have the expertise to solve > > the issue, but at least now I'm one step closer. > > > > > > I did however notice that my laptop (ASUS Zenbook UX430UNR/i7-8550U) > > > won't wake when opening the lid or pressing a key, the only way to > > > wake > > > the laptop is pressing the power button. > > > > > > I also tested mainline (5.3.0-rc4 b7e7c85dc7b0) and 5.2.8 and the > > > laptop > > > wakes without issue when the lid is opened or a key is presed. > > >> Please refer to the changelogs for details. > > Thanks for your report. > > > > I seem to see a similar issue with respect to the lid on one of my > > test machines, looking into it right now. > > >>> Well, my lid issue seems to be unrelated as it doesn't result from any > > >>> patches in the > > >>> series in question. > > >>> > > >>> First off, please clone 5.3-rc5 from kernel.org and double check if the > > >>> issue is not > > >>> present in that one. > > >>> > > >>> If that's not the case, merge the pm-s2idle-rework branch from my tree > > >>> on top of it > > >>> and retest. > > >>> > > >>> If you still see the issue then, apply the appended patch (on top of > > >>> the pm-s2idle-reqork > > >>> branch ) and, after starting the kernel, do > > >>> > > >>> # echo 1 > /sys/power/pm_debug_messages > > >>> > > >>> suspend the system and try to wake it up through all of the ways that > > >>> stopped working. > > >>> > > >>> Then, wake it up with the power button, save the output of dmesg and > > >>> send it to me. > > >>>
Re: [PATCH v3 2/9] soc: samsung: Convert exynos-chipid driver to use the regmap API
On 8/20/19 21:37, Krzysztof Kozlowski wrote: >>> diff --git a/drivers/soc/samsung/exynos-chipid.c >>> b/drivers/soc/samsung/exynos-chipid.c >>> @@ -51,29 +48,24 @@ static const char * __init >>> product_id_to_soc_id(unsigned int product_id) >>> int __init exynos_chipid_early_init(void) >>> { >>>struct soc_device_attribute *soc_dev_attr; >>> - void __iomem *exynos_chipid_base; >>>struct soc_device *soc_dev; >>>struct device_node *root; >>> - struct device_node *np; >>> + struct regmap *regmap; >>>u32 product_id; >>>u32 revision; >>> + int ret; >>> >>> - /* look up for chipid node */ >>> - np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid"); >>> - if (!np) >>> - return -ENODEV; >>> - >>> - exynos_chipid_base = of_iomap(np, 0); >>> - of_node_put(np); >>> - >>> - if (!exynos_chipid_base) { >>> - pr_err("Failed to map SoC chipid\n"); >>> - return -ENXIO; >>> + regmap = >>> syscon_regmap_lookup_by_compatible("samsung,exynos4210-chipid"); >>> + if (IS_ERR(regmap)) { >>> + pr_err("Failed to get CHIPID regmap\n"); >>> + return PTR_ERR(regmap); >>>} >> Following this change, I am now seeing the above error on our Tegra >> boards where this driver is enabled. This is triggering a kernel >> warnings test we have to fail. Hence, I don't think that you can remove >> the compatible node test here, unless you have a better way to determine >> if this is a samsung device. > > Right, this is really wrong... I missed that it is not a probe but > early init. And this init will be called on every board... Probably it > should be converted to a regular driver. I'm also inclined to have it converted to a regular driver. We already have "exynos-asv" driver matching on the chipid node (patch 3/9). The ASV patches will not be merged soon anyway, all this needs some more thought. Krzysztof, can we abandon the chipid patches for now? Your pull request doesn't appear to be merged to arm-soc yet. Sorry about that. -- Regards, Sylwester
[v7 2/2] gpio: aspeed: Add SGPIO driver
Hello Linus, Thanks for your review! I just submitted v8 to the list, please help to review it again. Since you have already merged the dt-binding document [v7 1/2], and I don't have your update to this file, so to avoid confusion, I only include the driver code in v8. Regards, --Hongwei > From: Linus Walleij > Sent: Wednesday, August 14, 2019 4:09 AM > To: Hongwei Zhang > Cc: Andrew Jeffery; open list:GPIO SUBSYSTEM; Joel Stanley; linux-aspeed; > Bartosz Golaszewski; > linux-kernel@vger.kernel.org; Linux ARM > Subject: Re: [v7 2/2] gpio: aspeed: Add SGPIO driver > > Hi Hongwei, > > thanks for your patch! > > I have now merged the bindings so you only need to respin this patch. > > On Wed, Jul 31, 2019 at 10:02 PM Hongwei Zhang wrote: > > > Add SGPIO driver support for Aspeed AST2500 SoC. > > > > Signed-off-by: Hongwei Zhang > > Reviewed-by: Andrew Jeffery > > I guess I need to go with this, there are some minor things I still want to > be fixed: > > > +static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int > > +offset, int val) > > I don't like __underscore_functions because their semantic is ambiguous. > done, please see v8. > Rename this something like aspeed_sgpio_commit() or whatever best fits the > actual use. > > > +static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio, > > + struct platform_device *pdev) { > (...) > > + rc = gpiochip_irqchip_add(>chip, _sgpio_irqchip, > > + 0, handle_bad_irq, IRQ_TYPE_NONE); > (...) > > + gpiochip_set_chained_irqchip(>chip, _sgpio_irqchip, > > +gpio->irq, > > + aspeed_sgpio_irq_handler); > > We do not set up chained irqchips like this anymore, sorry. > > I am currently rewriting all existing chained drivers to pass an initialized > irqchip when registering the > whole gpio chip. > See drivers/gpio/TODO. > > Here are examples: > https://lore.kernel.org/linux-gpio/20190811080539.15647-1-linus.wall...@linaro.org/ > https://lore.kernel.org/linux-gpio/20190812132554.18313-1-linus.wall...@linaro.org/ > done, please see v8. > > + /* set all SGPIO pins as input (1). */ > > + memset(gpio->dir_in, 0xff, sizeof(gpio->dir_in)); > > Do the irqchip set-up here, before adding the gpio_chip. > > > + rc = devm_gpiochip_add_data(>dev, >chip, gpio); > > + if (rc < 0) > > + return rc; > > + > > + return aspeed_sgpio_setup_irqs(gpio, pdev); > > Yours, > Linus Walleij
Re: [PATCH v2 1/4] fs/posix_acl: apply umask if superblock disables ACL support
What happened to these patches? All four make sense to me, for what it's worth; feel free to add a Reviewed-by: J. Bruce Fields --b. On Sat, Jul 13, 2019 at 06:11:57AM +0200, Max Kellermann wrote: > The function posix_acl_create() applies the umask only if the inode > has no ACL (= NULL) or if ACLs are not supported by the filesystem > driver (= -EOPNOTSUPP). > > However, this happens only after after the IS_POSIXACL() check > succeeeded. If the superblock doesn't enable ACL support, umask will > never be applied. A filesystem which has no ACL support will of > course not enable SB_POSIXACL, rendering the umask-applying code path > unreachable. > > This fixes a bug which causes the umask to be ignored with O_TMPFILE > on tmpfs: > > https://github.com/MusicPlayerDaemon/MPD/issues/558 > https://bugs.gentoo.org/show_bug.cgi?id=686142#c3 > https://bugzilla.kernel.org/show_bug.cgi?id=203625 > > Signed-off-by: Max Kellermann > Cc: sta...@vger.kernel.org > --- > fs/posix_acl.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/posix_acl.c b/fs/posix_acl.c > index 84ad1c90d535..4071c66f234a 100644 > --- a/fs/posix_acl.c > +++ b/fs/posix_acl.c > @@ -589,9 +589,14 @@ posix_acl_create(struct inode *dir, umode_t *mode, > *acl = NULL; > *default_acl = NULL; > > - if (S_ISLNK(*mode) || !IS_POSIXACL(dir)) > + if (S_ISLNK(*mode)) > return 0; > > + if (!IS_POSIXACL(dir)) { > + *mode &= ~current_umask(); > + return 0; > + } > + > p = get_acl(dir, ACL_TYPE_DEFAULT); > if (!p || p == ERR_PTR(-EOPNOTSUPP)) { > *mode &= ~current_umask(); > -- > 2.20.1
Re: [PATCH bpf-next] xsk: proper socket state check in xsk_poll
On 8/20/19 5:29 PM, Björn Töpel wrote: On Tue, 20 Aug 2019 at 16:30, Daniel Borkmann wrote: On 8/20/19 12:04 PM, Björn Töpel wrote: From: Björn Töpel The poll() implementation for AF_XDP sockets did not perform the proper state checks, prior accessing the socket umem. This patch fixes that by performing a xsk_is_bound() check. Suggested-by: Hillf Danton Reported-by: syzbot+c82697e3043781e08...@syzkaller.appspotmail.com Fixes: 77cd0d7b3f25 ("xsk: add support for need_wakeup flag in AF_XDP rings") Signed-off-by: Björn Töpel --- net/xdp/xsk.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c index ee4428a892fa..08bed5e92af4 100644 --- a/net/xdp/xsk.c +++ b/net/xdp/xsk.c @@ -356,13 +356,20 @@ static int xsk_generic_xmit(struct sock *sk, struct msghdr *m, return err; } +static bool xsk_is_bound(struct xdp_sock *xs) +{ + struct net_device *dev = READ_ONCE(xs->dev); + + return dev && xs->state == XSK_BOUND; +} + static int xsk_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len) { bool need_wait = !(m->msg_flags & MSG_DONTWAIT); struct sock *sk = sock->sk; struct xdp_sock *xs = xdp_sk(sk); - if (unlikely(!xs->dev)) + if (unlikely(!xsk_is_bound(xs))) return -ENXIO; if (unlikely(!(xs->dev->flags & IFF_UP))) return -ENETDOWN; @@ -383,6 +390,9 @@ static unsigned int xsk_poll(struct file *file, struct socket *sock, struct net_device *dev = xs->dev; struct xdp_umem *umem = xs->umem; + if (unlikely(!xsk_is_bound(xs))) + return mask; + if (umem->need_wakeup) dev->netdev_ops->ndo_xsk_wakeup(dev, xs->queue_id, umem->need_wakeup); @@ -417,7 +427,7 @@ static void xsk_unbind_dev(struct xdp_sock *xs) { struct net_device *dev = xs->dev; - if (!dev || xs->state != XSK_BOUND) + if (!xsk_is_bound(xs)) return; I think I'm a bit confused by your READ_ONCE() usage. ;-/ I can see why you're using it in xsk_is_bound() above, but then at the same time all the other callbacks like xsk_poll() or xsk_unbind_dev() above have a struct net_device *dev = xs->dev right before the test. Could you elaborate? Yes, now I'm confused as well! Digging deeper... I believe there are a couple of places in xsk.c that do not have READ_ONCE/WRITE_ONCE-correctness. Various xdp_sock members are read lock-less outside the control plane mutex (mutex member of struct xdp_sock). This needs some re-work. I'll look into using the newly Right, so even in above two cases, the compiler could have refetched, e.g. dev variable could have first been NULL, but xsk_is_bound() later returns true. introduced state member (with corresponding read/write barriers) for this. I'll cook some patch(es) that address this, but first it sounds like I need to reread [1] two, or three times. At least. ;-) Thanks, Björn [1] https://lwn.net/Articles/793253/ Thanks, Daniel
Re: [PATCH 10/15] riscv: poison SBI calls for M-mode
On Tue, 2019-08-13 at 17:47 +0200, Christoph Hellwig wrote: > There is no SBI when we run in M-mode, so fail the compile for any > code > trying to use SBI calls. > > Signed-off-by: Christoph Hellwig > --- > arch/riscv/include/asm/sbi.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/sbi.h > b/arch/riscv/include/asm/sbi.h > index 21134b3ef404..1e17f07eadaf 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -8,6 +8,7 @@ > > #include > > +#ifndef CONFIG_M_MODE > #define SBI_SET_TIMER 0 > #define SBI_CONSOLE_PUTCHAR 1 > #define SBI_CONSOLE_GETCHAR 2 > @@ -94,4 +95,5 @@ static inline void sbi_remote_sfence_vma_asid(const > unsigned long *hart_mask, > SBI_CALL_4(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, > asid); > } > > -#endif > +#endif /* CONFIG_M_MODE */ > +#endif /* _ASM_RISCV_SBI_H */ Reviewed-by: Atish Patra -- Regards, Atish
Re: [PATCH RFC] dt-bindings: regulator: define a mux regulator
Hello Rob, On Tue, Aug 20, 2019 at 11:39:27AM -0500, Rob Herring wrote: > On Tue, Aug 20, 2019 at 10:25 AM Uwe Kleine-König > wrote: > > > > A mux regulator is used to provide current on one of several outputs. It > > might look as follows: > > > > ,. > > -- > -- > -- > -- > -- > -- > -- > -- > `' > > > > Depending on which address is encoded on the three address inputs A0, A1 > > and A2 the current provided on IN is provided on one of the eight > > outputs. > > > > What is new here is that the binding makes use of a #regulator-cells > > property. This uses the approach known from other bindings (e.g. gpio) > > to allow referencing all eight outputs with phandle arguments. This > > requires an extention in of_get_regulator to use a new variant of > > of_parse_phandle_with_args that has a cell_count_default parameter that > > is used in absence of a $cell_name property. Even if we'd choose to > > update all regulator-bindings to add #regulator-cells = <0>; we still > > needed something to implement compatibility to the currently defined > > bindings. > > > > Signed-off-by: Uwe Kleine-König > > --- > > Hello, > > > > the obvious alternative is to add (here) eight subnodes to represent the > > eight outputs. This is IMHO less pretty, but wouldn't need to introduce > > #regulator-cells. > > I'm okay with #regulator-cells approach. OK, then I will look into that in more detail; unless the regulator guys don't agree with this approach of course. > > Apart from reg = <..> and a phandle there is (I think) nothing that > > needs to be specified in the subnodes because all properties of an > > output (apart from the address) apply to all outputs. > > > > What do you think? > > > > Best regards > > Uwe > > > > .../bindings/regulator/mux-regulator.yaml | 52 +++ > > 1 file changed, 52 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/regulator/mux-regulator.yaml > > > > diff --git a/Documentation/devicetree/bindings/regulator/mux-regulator.yaml > > b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml > > new file mode 100644 > > index ..f06dbb969090 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/regulator/mux-regulator.yaml > > @@ -0,0 +1,52 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > (GPL-2.0-only OR BSD-2-Clause) is preferred. OK. > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/regulator/mux-regulator.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: MUX regulators > > + > > +properties: > > + compatible: > > +const: XXX,adb708 > > ? I assume you will split this into a common and specific schemas. I > suppose there could be differing ways to control the mux just like all > other muxes. Not sure if a specific schema is necessary. I wrote XXX because I was offline while I authored the binding and so couldn't determine the right vendor to use. > > + enable-gpios: > > +maxItems: 1 > > + > > + address-gpios: > > +description: Array of typically three GPIO pins used to select the > > + regulator's output. The least significant address GPIO must be listed > > + first. The others follow in order of significance. > > +minItems: 1 > > + > > + "#regulator-cells": > > How is this not required? It should. For the RFC patch I didn't took the time to iron all the details. My main concern was/is how the binding should look like and if an #regulator-cells with a default would be acceptable. Best regards and thanks for your feedback, Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ |
linux-next: Signed-off-by missing for commit in the pci tree
Hi all, Commit c4a29fbba415 ("PCI: hv: Use bytes 4 and 5 from instance ID as the PCI domain numbers") is missing a Signed-off-by from its committer. Also, all the tags should be kept together, please. -- Cheers, Stephen Rothwell pgp1roVGesKBf.pgp Description: OpenPGP digital signature
[PATCH v3 1/2] media: vimc: Collapse component structure into a single monolithic driver
vimc uses Component API to split the driver into functional components. The real hardware resembles a monolith structure than component and component structure added a level of complexity making it hard to maintain without adding any real benefit. The sensor is one vimc component that would makes sense to be a separate module to closely align with the real hardware. It would be easier to collapse vimc into single monolithic driver first and then split the sensor off as a separate module. Collapse it into a single monolithic driver removing the Component API. This patch removes the component API and makes minimal changes to the code base preserving the functional division of the code structure. Preserving the functional structure allows us to split the sensor off as a separate module in the future. Major design elements in this change are: - Use existing struct vimc_ent_config and struct vimc_pipeline_config to drive the initialization of the functional components. - Make vimc_device and vimc_ent_config global by moving them to vimc-common.h - Add two new hooks add and rm to initialize and register, unregister and free subdevs. - All component API is now gone and bind and unbind hooks are modified to do "add" and "rm" with minimal changes to just add and rm subdevs. - vimc-core's bind and unbind are now register and unregister. - vimc-core invokes "add" hooks from its vimc_register_devices(). The "add" hooks remain the same and register subdevs. They don't create platform devices of their own and use vimc's pdev.dev as their reference device. The "add" hooks save their vimc_ent_device(s) in the corresponding vimc_ent_config. - vimc-core invokes "rm" hooks from its unregister to unregister subdevs and cleanup. - vimc-core invokes "add" and "rm" hooks with pointer to struct vimc_device and the corresponding struct vimc_ent_config pointer. The following configure and stream test works on all devices. media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]' v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440 v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81 v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video1 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video3 Signed-off-by: Shuah Khan --- drivers/media/platform/vimc/Makefile | 7 +- drivers/media/platform/vimc/vimc-capture.c | 79 ++-- drivers/media/platform/vimc/vimc-common.h | 48 + drivers/media/platform/vimc/vimc-core.c| 199 - drivers/media/platform/vimc/vimc-debayer.c | 67 ++- drivers/media/platform/vimc/vimc-scaler.c | 71 ++-- drivers/media/platform/vimc/vimc-sensor.c | 71 ++-- 7 files changed, 176 insertions(+), 366 deletions(-) diff --git a/drivers/media/platform/vimc/Makefile b/drivers/media/platform/vimc/Makefile index 96d06f030c31..a53b2b532e9f 100644 --- a/drivers/media/platform/vimc/Makefile +++ b/drivers/media/platform/vimc/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 -vimc-y := vimc-core.o vimc-common.o vimc-streamer.o +vimc-y := vimc-core.o vimc-common.o vimc-streamer.o vimc-capture.o \ + vimc-debayer.o vimc-scaler.o vimc-sensor.o + +obj-$(CONFIG_VIDEO_VIMC) += vimc.o -obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-capture.o vimc-debayer.o \ -vimc-scaler.o vimc-sensor.o diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c index 1d56b91830ba..e04b60ec0738 100644 --- a/drivers/media/platform/vimc/vimc-capture.c +++ b/drivers/media/platform/vimc/vimc-capture.c @@ -5,10 +5,6 @@ * Copyright (C) 2015-2017 Helen Koike */ -#include -#include -#include -#include #include #include #include @@ -16,8 +12,6 @@ #include "vimc-common.h" #include "vimc-streamer.h" -#define VIMC_CAP_DRV_NAME "vimc-capture" - struct vimc_cap_device { struct vimc_ent_device ved; struct video_device vdev; @@ -340,13 +334,15 @@ static void vimc_cap_release(struct video_device *vdev) kfree(vcap); } -static void vimc_cap_comp_unbind(struct device *comp, struct device *master, -void *master_data) +void vimc_cap_rm(struct vimc_device *vimc, struct vimc_ent_config *vcfg) { - struct vimc_ent_device *ved = dev_get_drvdata(comp); - struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device, - ved); + struct vimc_ent_device *ved = vcfg->ved; + struct vimc_cap_device *vcap; + + if (!ved) + return; + vcap = container_of(ved, struct
[PATCH v3 0/2] Collapse vimc into single monolithic driver
vimc uses Component API to split the driver into functional components. The real hardware resembles a monolith structure than component and component structure added a level of complexity making it hard to maintain without adding any real benefit. The sensor is one vimc component that would makes sense to be a separate module to closely align with the real hardware. It would be easier to collapse vimc into single monolithic driver first and then split the sensor off as a separate module. This patch series removes the component API and makes minimal changes to the code base preserving the functional division of the code structure. Preserving the functional structure allows us to split the sensor off as a separate module in the future. Major design elements in this change are: - Use existing struct vimc_ent_config and struct vimc_pipeline_config to drive the initialization of the functional components. - Make vimc_device and vimc_ent_config global by moving them to vimc-common.h - Add two new hooks add and rm to initialize and register, unregister and free subdevs. - All component API is now gone and bind and unbind hooks are modified to do "add" and "rm" with minimal changes to just add and rm subdevs. - vimc-core's bind and unbind are now register and unregister. - vimc-core invokes "add" hooks from its vimc_register_devices(). The "add" hooks remain the same and register subdevs. They don't create platform devices of their own and use vimc's pdev.dev as their reference device. The "add" hooks save their vimc_ent_device(s) in the corresponding vimc_ent_config. - vimc-core invokes "rm" hooks from its unregister to unregister subdevs and cleanup. - vimc-core invokes "add" and "rm" hooks with pointer to struct vimc_device and the corresponding struct vimc_ent_config pointer. The following configure and stream test works on all devices. media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]' v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440 v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81 v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video1 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video3 The second patch in the series fixes a general protection fault found when rmmod is done while stream is active. - rmmod while streaming returns vimc is in use - rmmod without active stream works correctly Changes since v2: - Rebase to media master on top of vimc reverts. No merge conflicts. - Rename "vent" variable to "vcfg" to reflect config and standout from "ved" - Error handling when adding subdevs fails to remove already added subdevs, do other clean-up and bail out. - No changes to patch 2/2 Changes since v1: Patch 1 & 2: (patch 1 in this series) - Collapsed the two patches into one - Added common defines (vimc_device and vimc_ent_config) to vimc-common.h based on our discussion. - Addressed review comments from Helen and Laurent - Use vimc-common.h instead of creating a new file. - Other minor comments from Helen on int vs. unsigned int and not needing to initialize ret in vimc_add_subdevs() Patch 3 (patch 2 in this series): - The second patch is the fix for gpf. Updated the patch after looking at the test results from Andre and Helen. This problem is in a common code and impacts all subdevs. The fix addresses the core problem and fixes it. Fix removes pads release from v4l2_device_unregister_subdev() and pads are now released from the sd release handler with all other resources. Outstanding: - Update documentation with the correct topology. - There is one outstanding gpf remaining in the unbind path. I will fix that in a separate patch. This is an existing problem and will be easier to fix on top of this patch series. vimc_print_dot (--print-dot) topology after this change: (no change compared to media master) digraph board { rankdir=TB n0001 [label="{{} | Sensor A\n/dev/v4l-subdev0 | { 0}}", shape=Mrecord, style=filled, fillcolor=green] n0001:port0 -> n0005:port0 [style=bold] n0001:port0 -> n000b [style=bold] n0003 [label="{{} | Sensor B\n/dev/v4l-subdev1 | { 0}}", shape=Mrecord, style=filled, fillcolor=green] n0003:port0 -> n0008:port0 [style=bold] n0003:port0 -> n000f [style=bold] n0005 [label="{{ 0} | Debayer A\n/dev/v4l-subdev2 | { 1}}", shape=Mrecord, style=filled, fillcolor=green] n0005:port1 -> n0015:port0 n0008
[PATCH v3 2/2] media: vimc: Fix gpf in rmmod path when stream is active
If vimc module is removed while streaming is in progress, sensor subdev unregister runs into general protection fault when it tries to unregister media entities. This is a common subdev problem related to releasing pads from v4l2_device_unregister_subdev() before calling unregister. Unregister references pads during unregistering subdev. The sd release handler is the right place for releasing all sd resources including pads. The release handlers currently release all resources except the pads. Fix v4l2_device_unregister_subdev() not release pads and release pads from the sd_int_op release handlers. kernel: [ 4136.715839] general protection fault: [#1] SMP PTI kernel: [ 4136.715847] CPU: 2 PID: 1972 Comm: bash Not tainted 5.3.0-rc2+ #4 kernel: [ 4136.715850] Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A18 09/24/2013 kernel: [ 4136.715858] RIP: 0010:media_gobj_destroy.part.16+0x1f/0x60 kernel: [ 4136.715863] Code: ff 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 fe 48 89 e5 53 48 89 fb 48 c7 c7 00 7f cf b0 e8 24 fa ff ff 48 8b 03 <48> 83 80 a0 00 00 00 01 48 8b 43 18 48 8b 53 10 48 89 42 08 48 89 kernel: [ 4136.715866] RSP: 0018:9b2248fe3cb0 EFLAGS: 00010246 kernel: [ 4136.715870] RAX: bcf2bfbfa0d63c2f RBX: 88c3eb37e9c0 RCX: 802a0018 kernel: [ 4136.715873] RDX: 88c3e4f6a078 RSI: 88c3eb37e9c0 RDI: b0cf7f00 kernel: [ 4136.715876] RBP: 9b2248fe3cb8 R08: 0102 R09: b0492b00 kernel: [ 4136.715879] R10: 9b2248fe3c28 R11: 0001 R12: 0038 kernel: [ 4136.715881] R13: c09a1628 R14: 88c3e4f6a028 R15: fff2 kernel: [ 4136.715885] FS: 7f8389647740() GS:88c46550() knlGS: kernel: [ 4136.715888] CS: 0010 DS: ES: CR0: 80050033 kernel: [ 4136.715891] CR2: 55d008f80fd8 CR3: 0001996ec005 CR4: 000606e0 kernel: [ 4136.715894] Call Trace: kernel: [ 4136.715903] media_gobj_destroy+0x14/0x20 kernel: [ 4136.715908] __media_device_unregister_entity+0xb3/0xe0 kernel: [ 4136.715915] media_device_unregister_entity+0x30/0x40 kernel: [ 4136.715920] v4l2_device_unregister_subdev+0xa8/0xe0 kernel: [ 4136.715928] vimc_ent_sd_unregister+0x1e/0x30 [vimc] kernel: [ 4136.715933] vimc_sen_rm+0x16/0x20 [vimc] kernel: [ 4136.715938] vimc_remove+0x3e/0xa0 [vimc] kernel: [ 4136.715945] platform_drv_remove+0x25/0x50 kernel: [ 4136.715951] device_release_driver_internal+0xe0/0x1b0 kernel: [ 4136.715956] device_driver_detach+0x14/0x20 kernel: [ 4136.715960] unbind_store+0xd1/0x130 kernel: [ 4136.715965] drv_attr_store+0x27/0x40 kernel: [ 4136.715971] sysfs_kf_write+0x48/0x60 kernel: [ 4136.715976] kernfs_fop_write+0x128/0x1b0 kernel: [ 4136.715982] __vfs_write+0x1b/0x40 kernel: [ 4136.715987] vfs_write+0xc3/0x1d0 kernel: [ 4136.715993] ksys_write+0xaa/0xe0 kernel: [ 4136.715999] __x64_sys_write+0x1a/0x20 kernel: [ 4136.716005] do_syscall_64+0x5a/0x130 kernel: [ 4136.716010] entry_SYSCALL_64_after_hwframe+0x4 Signed-off-by: Shuah Khan --- drivers/media/platform/vimc/vimc-common.c | 3 +-- drivers/media/platform/vimc/vimc-debayer.c | 1 + drivers/media/platform/vimc/vimc-scaler.c | 1 + drivers/media/platform/vimc/vimc-sensor.c | 1 + 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c index 7e1ae0b12f1e..a3120f4f7a90 100644 --- a/drivers/media/platform/vimc/vimc-common.c +++ b/drivers/media/platform/vimc/vimc-common.c @@ -375,7 +375,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved, { int ret; - /* Allocate the pads */ + /* Allocate the pads. Should be released from the sd_int_op release */ ved->pads = vimc_pads_init(num_pads, pads_flag); if (IS_ERR(ved->pads)) return PTR_ERR(ved->pads); @@ -424,7 +424,6 @@ EXPORT_SYMBOL_GPL(vimc_ent_sd_register); void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev *sd) { media_entity_cleanup(ved->ent); - vimc_pads_cleanup(ved->pads); v4l2_device_unregister_subdev(sd); } EXPORT_SYMBOL_GPL(vimc_ent_sd_unregister); diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c index 73dc17f0d990..6cee911bf149 100644 --- a/drivers/media/platform/vimc/vimc-debayer.c +++ b/drivers/media/platform/vimc/vimc-debayer.c @@ -482,6 +482,7 @@ static void vimc_deb_release(struct v4l2_subdev *sd) struct vimc_deb_device *vdeb = container_of(sd, struct vimc_deb_device, sd); + vimc_pads_cleanup(vdeb->ved.pads); kfree(vdeb); } diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c index 17da47a103ef..04c778f74972 100644 --- a/drivers/media/platform/vimc/vimc-scaler.c +++ b/drivers/media/platform/vimc/vimc-scaler.c @@ -338,6 +338,7 @@ static void
Re: How to add multiple example with conflicting includes
On Fri, Jul 26, 2019 at 1:11 PM Corentin Labbe wrote: > > Hello > > When I try to check the following examples of a devicetree schema: > examples: > - | > #include > #include > #include > > crypto: crypto@1c15000 { > compatible = "allwinner,sun8i-h3-crypto"; > reg = <0x01c15000 0x1000>; > interrupts = ; > clocks = < CLK_BUS_CE>, < CLK_CE>; > clock-names = "ahb", "mod"; > resets = < RST_BUS_CE>; > reset-names = "ahb"; > }; > > - | > #include > #include > #include > > crypto: crypto@1904000 { > compatible = "allwinner,sun50i-h6-crypto"; > reg = <0x01904000 0x1000>; > interrupts = ; > clocks = < CLK_BUS_CE>, < CLK_CE>, < CLK_MBUS_CE>; > clock-names = "ahb", "mod", "mbus"; > resets = < RST_BUS_CE>; > reset-names = "ahb"; > }; > > I get: > In file included from > Documentation/devicetree/bindings/crypto/allwinner,sun8i-ce.example.dts:42: > /linux-next/scripts/dtc/include-prefixes/dt-bindings/clock/sun50i-h6-ccu.h:9: > warning: "CLK_PLL_PERIPH0" redefined > #define CLK_PLL_PERIPH0 3 > [...] > > So how can I add multiple examples which need somes conflicting #include to > be validated. 2 clocks versus 3 clocks hardly seems like a reason for a 2nd example and I would just drop it IMO. You could rename your defines to not collide, but that's kind of painful. The 3rd option is getting dt-extract-example to spit out N example files and then build each one separately. That was more complicated than I cared to figure out with kbuild, so I went the simple route. Rob
Re: [PATCH 15/15] riscv: disable the EFI PECOFF header for M-mode
On Tue, 2019-08-13 at 17:47 +0200, Christoph Hellwig wrote: > No point in bloating the kernel image with a bootloader header if > we run bare metal. > > Signed-off-by: Christoph Hellwig > --- > arch/riscv/kernel/head.S | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S > index 670e5cacb24e..09fcf3d000c0 100644 > --- a/arch/riscv/kernel/head.S > +++ b/arch/riscv/kernel/head.S > @@ -16,6 +16,7 @@ > > __INIT > ENTRY(_start) > +#ifndef CONFIG_M_MODE > /* >* Image header expected by Linux boot-loaders. The image > header data >* structure is described in asm/image.h. > @@ -47,6 +48,7 @@ ENTRY(_start) > > .global _start_kernel > _start_kernel: > +#endif /* CONFIG_M_MODE */ > /* Mask all interrupts */ > csrw CSR_XIE, zero > csrw CSR_XIP, zero Reviewed-by: Atish Patra -- Regards, Atish
[v8] gpio: aspeed: Add SGPIO driver
Hello, This short series introduce dt-binding document and a driver for the Aspeed AST2500 SGPIO controller. Please review. [v8]: Changes between v7 and v8: - v7 updates based on Linus' feedback - since Linus has already merged sgpio-aspeed.txt, I only include the driver here to avoid confusion. [v7]: Changes between v6 and v7: - fix missing variable 'reg' assign issue in aspeed_sgpio_set() - v6 feedback updates [v6]: Changes between v5 and v6: - fix a bug in aspeed_sgpio_dir_out() - v5 feedback updates, some comments cleanup The related SGPM pinmux dt-binding document, dts, and pinctrl driver updates have been accepted and merged: _http://patchwork.ozlabs.org/patch/1110210/ Hongwei Zhang (1): gpio: aspeed: Add SGPIO driver drivers/gpio/sgpio-aspeed.c | 533 1 file changed, 533 insertions(+) create mode 100644 drivers/gpio/sgpio-aspeed.c -- 2.7.4
linux-next: Fixes tag needs some work in the sound-asoc tree
Hi all, In commit 0f6fc97501b7 ("ASoC: mchp-i2s-mcc: Wait for RX/TX RDY only if controller is running") Fixes tag Fixes: 7e0cdf545a55 ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel Controller") has these problem(s): - Target SHA1 does not exist Did you mean Fixes: b87d37d0231f ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel Controller") In commit 988b59467b2b ("ASoC: mchp-i2s-mcc: Fix unprepare of GCLK") Fixes tag Fixes: 7e0cdf545a55 ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel Controller") has these problem(s): - Target SHA1 does not exist Did you mean Fixes: b87d37d0231f ("ASoC: mchp-i2s-mcc: add driver for I2SC Multi-Channel Controller") -- Cheers, Stephen Rothwell pgppbUawPq0Bm.pgp Description: OpenPGP digital signature
[v8 1/1] gpio: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC. Signed-off-by: Hongwei Zhang Reviewed-by: Andrew Jeffery --- drivers/gpio/sgpio-aspeed.c | 533 1 file changed, 533 insertions(+) create mode 100644 drivers/gpio/sgpio-aspeed.c diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c new file mode 100644 index 000..7e99860 --- /dev/null +++ b/drivers/gpio/sgpio-aspeed.c @@ -0,0 +1,533 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright 2019 American Megatrends International LLC. + * + * Author: Karthikeyan Mani + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define MAX_NR_SGPIO 80 + +#define ASPEED_SGPIO_CTRL 0x54 + +#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6) +#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16) +#define ASPEED_SGPIO_ENABLEBIT(0) + +struct aspeed_sgpio { + struct gpio_chip chip; + struct clk *pclk; + spinlock_t lock; + void __iomem *base; + uint32_t dir_in[3]; + int irq; +}; + +struct aspeed_sgpio_bank { + uint16_tval_regs; + uint16_trdata_reg; + uint16_tirq_regs; + const char names[4][3]; +}; + +/* + * Note: The "value" register returns the input value when the GPIO is + * configured as an input. + * + * The "rdata" register returns the output value when the GPIO is + * configured as an output. + */ +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = { + { + .val_regs = 0x, + .rdata_reg = 0x0070, + .irq_regs = 0x0004, + .names = { "A", "B", "C", "D" }, + }, + { + .val_regs = 0x001C, + .rdata_reg = 0x0074, + .irq_regs = 0x0020, + .names = { "E", "F", "G", "H" }, + }, + { + .val_regs = 0x0038, + .rdata_reg = 0x0078, + .irq_regs = 0x003C, + .names = { "I", "J" }, + }, +}; + +enum aspeed_sgpio_reg { + reg_val, + reg_rdata, + reg_irq_enable, + reg_irq_type0, + reg_irq_type1, + reg_irq_type2, + reg_irq_status, +}; + +#define GPIO_VAL_VALUE 0x00 +#define GPIO_IRQ_ENABLE 0x00 +#define GPIO_IRQ_TYPE0 0x04 +#define GPIO_IRQ_TYPE1 0x08 +#define GPIO_IRQ_TYPE2 0x0C +#define GPIO_IRQ_STATUS 0x10 + +static void __iomem *bank_reg(struct aspeed_sgpio *gpio, +const struct aspeed_sgpio_bank *bank, +const enum aspeed_sgpio_reg reg) +{ + switch (reg) { + case reg_val: + return gpio->base + bank->val_regs + GPIO_VAL_VALUE; + case reg_rdata: + return gpio->base + bank->rdata_reg; + case reg_irq_enable: + return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE; + case reg_irq_type0: + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0; + case reg_irq_type1: + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1; + case reg_irq_type2: + return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2; + case reg_irq_status: + return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS; + default: + /* acturally if code runs to here, it's an error case */ + BUG_ON(1); + } +} + +#define GPIO_BANK(x)((x) >> 5) +#define GPIO_OFFSET(x) ((x) & 0x1f) +#define GPIO_BIT(x) BIT(GPIO_OFFSET(x)) + +static const struct aspeed_sgpio_bank *to_bank(unsigned int offset) +{ + unsigned int bank = GPIO_BANK(offset); + + WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks)); + return _sgpio_banks[bank]; +} + +static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset) +{ + struct aspeed_sgpio *gpio = gpiochip_get_data(gc); + const struct aspeed_sgpio_bank *bank = to_bank(offset); + unsigned long flags; + enum aspeed_sgpio_reg reg; + bool is_input; + int rc = 0; + + spin_lock_irqsave(>lock, flags); + + is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset); + reg = is_input ? reg_val : reg_rdata; + rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset)); + + spin_unlock_irqrestore(>lock, flags); + + return rc; +} + +static void sgpio_set_value(struct gpio_chip *gc, unsigned int offset, int val) +{ + struct aspeed_sgpio *gpio = gpiochip_get_data(gc); + const struct aspeed_sgpio_bank *bank = to_bank(offset); + void __iomem *addr; + u32 reg = 0; + + addr = bank_reg(gpio, bank, reg_val); + reg = ioread32(addr); + + if (val) + reg |= GPIO_BIT(offset); + else + reg &= ~GPIO_BIT(offset); + + iowrite32(reg, addr); +} +
Re: [PATCH V40 11/29] PCI: Lock down BAR access when the kernel is locked down
On Tue, Aug 20, 2019 at 12:45 PM Bjorn Helgaas wrote: > Since I've acked this and it's 11/29, I've been assuming you intend > to merge the whole series together. But the fact that it's up to V40 > makes me wonder if you're waiting for me to merge this one. Just let > me know either way. James has merged the series.
Re: [PATCH 09/15] riscv: implement remote sfence.i natively for M-mode
On Tue, 2019-08-13 at 17:47 +0200, Christoph Hellwig wrote: > The RISC-V ISA only supports flushing the instruction cache for the > local > CPU core. For normal S-mode Linux remote flushing is offloaded to > machine mode using ecalls, but for M-mode Linux we'll have to do it > ourselves. Use the same implementation as all the existing open > source > SBI implementations by just doing an IPI to all remote cores to > execute > th sfence.i instruction on every live core. > > Signed-off-by: Christoph Hellwig > --- > arch/riscv/mm/cacheflush.c | 31 +++ > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > index 9ebcff8ba263..10875ea1065e 100644 > --- a/arch/riscv/mm/cacheflush.c > +++ b/arch/riscv/mm/cacheflush.c > @@ -10,10 +10,35 @@ > > #include > > +#ifdef CONFIG_M_MODE > +static void ipi_remote_fence_i(void *info) > +{ > + return local_flush_icache_all(); > +} > + > +void flush_icache_all(void) > +{ > + on_each_cpu(ipi_remote_fence_i, NULL, 1); > +} > + > +static void flush_icache_cpumask(const cpumask_t *mask) > +{ > + on_each_cpu_mask(mask, ipi_remote_fence_i, NULL, 1); > +} > +#else /* CONFIG_M_MODE */ > void flush_icache_all(void) > { > sbi_remote_fence_i(NULL); > } > +static void flush_icache_cpumask(const cpumask_t *mask) > +{ > + cpumask_t hmask; > + > + cpumask_clear(); > + riscv_cpuid_to_hartid_mask(mask, ); > + sbi_remote_fence_i(hmask.bits); > +} > +#endif /* CONFIG_M_MODE */ > > /* > * Performs an icache flush for the given MM context. RISC-V has no > direct > @@ -28,7 +53,7 @@ void flush_icache_all(void) > void flush_icache_mm(struct mm_struct *mm, bool local) > { > unsigned int cpu; > - cpumask_t others, hmask, *mask; > + cpumask_t others, *mask; > > preempt_disable(); > > @@ -47,9 +72,7 @@ void flush_icache_mm(struct mm_struct *mm, bool > local) > cpumask_andnot(, mm_cpumask(mm), cpumask_of(cpu)); > local |= cpumask_empty(); > if (mm != current->active_mm || !local) { > - cpumask_clear(); > - riscv_cpuid_to_hartid_mask(, ); > - sbi_remote_fence_i(hmask.bits); > + flush_icache_cpumask(); > } else { > /* >* It's assumed that at least one strongly ordered > operation is Reviewed-by: Atish Patra -- Regards, Atish
Re: [PATCH] net/mlx5: Fix a memory leak bug
On Tue, 2019-08-13 at 03:21 -0500, Wenwen Wang wrote: > In mlx5_cmd_invoke(), 'ent' is allocated through kzalloc() in > alloc_cmd(). > After the work is queued, wait_func() is invoked to wait the > completion of > the work. If wait_func() returns -ETIMEDOUT, the following execution > will > be terminated. However, the allocated 'ent' is not deallocated on > this > program path, leading to a memory leak bug. > > To fix the above issue, free 'ent' before returning the error. Hi Wenewn, sorry i have to nack this. As Moshe already pointed out, we intentionally don't free ent, since even if the driver decided to timeout, FW might still send a completion, until the FW sends the completion, this entry shouldn't be freed and is not reusable by driver. So this is not a memory leak, it just means that only FW completion is allowed to free this entry or driver shutdown.. otherwise this command entry is just dead until next fw completion. Thanks, Saeed. > > Signed-off-by: Wenwen Wang > --- > drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c > b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c > index 8cdd7e6..90cdb9a 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c > @@ -1036,7 +1036,7 @@ static int mlx5_cmd_invoke(struct mlx5_core_dev > *dev, struct mlx5_cmd_msg *in, > > err = wait_func(dev, ent); > if (err == -ETIMEDOUT) > - goto out; > + goto out_free; > > ds = ent->ts2 - ent->ts1; > op = MLX5_GET(mbox_in, in->first.data, opcode);
Re: [PATCH] net: Fix detection for IPv4 duplicate address.
From: Dongxu Liu Date: Tue, 20 Aug 2019 23:19:05 +0800 > @@ -800,8 +800,11 @@ static int arp_process(struct net *net, struct sock *sk, > struct sk_buff *skb) > iptunnel_metadata_reply(skb_metadata_dst(skb), > GFP_ATOMIC); > > - /* Special case: IPv4 duplicate address detection packet (RFC2131) */ > - if (sip == 0) { > +/* Special case: IPv4 duplicate address detection packet (RFC2131). > + * Linux usually sends zero to detect duplication, and windows may > + * send a same ip (not zero, sip equal to tip) to do this detection. > + */ > + if (sip == 0 || sip == tip) { Regardless of whether this is a valid change or not, you've unindented the comment which is completely inappropriate.
Re: [PATCH 0/6] Add ethernet support for Orange Pi 3
It looks like there will be some updates to this series either involving adding -supply to the property names or adjusting some of the kernel log messages. Seriously, I would prefer less verbiage in the logs rather than more.
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Tue, Aug 20, 2019 at 10:39:39PM +0200, Peter Zijlstra wrote: > On Tue, Aug 20, 2019 at 01:31:35PM -0700, Paul E. McKenney wrote: > > On Tue, Aug 20, 2019 at 04:01:16PM +0200, Peter Zijlstra wrote: > > > > We really should get the compiler folks to give us a > > > -fno-pointer-provenance. Waiting on the standards committee to get their > > > act together seems unlikely, esp. given that some people actually seem > > > to _want_ this nonsense :/ > > > > The reason that they want it is to enable some significant optimizations > > in numerical code on the one hand and in heavily templated C++ code on > > the other. Neither of which has much bearing on kernel code. > > > > Interested in coming to the next C standards committee meeting in October > > to help me push for this? ;-) > > How about we try and get some compiler folks together at plumbers and > bribe them with beer? Once we have our compiler knob, we happy :-) C'mon, Peter! Where is your sense of self-destruction??? ;-) But yes, if nothing else there is a Toolchains MC [1]. Which happens to have a topic entitled "Potential impact/benefit/detriment of recently developed GCC optimizations on the kernel", now that you mention it. Looking forward to seeing you in Lisbon! Thanx, Paul [1] https://linuxplumbersconf.org/event/4/sessions/45/#20190909
Re: [Linux-kernel-mentees][PATCH v6 1/2] sgi-gru: Convert put_page() to put_user_page*()
On 8/20/19 1:18 AM, Michal Hocko wrote: On Mon 19-08-19 12:30:18, John Hubbard wrote: On 8/19/19 12:06 PM, Bharath Vedartham wrote: On Mon, Aug 19, 2019 at 07:56:11AM -0500, Dimitri Sivanich wrote: ... Conversion of gup/put_page sites: Before: get_user_pages(...); ... for each page: put_page(); After: gup_flags |= FOLL_PIN; (maybe FOLL_LONGTERM in some cases) vaddr_pin_user_pages(...gup_flags...) I was hoping that FOLL_PIN would be handled by vaddr_pin_user_pages. Good point: now that we've got the 4 cases summarized, it turns out that either FOLL_PIN is required, or there is no need to call vaddr_pin_user_pages() at all. So we can go back to setting FOLL_PIN inside it, which is of course much better for maintenance. Great! ... vaddr_unpin_user_pages(); /* which invokes put_user_page() */ Fortunately, it's not harmful for the simpler conversion from put_page() to put_user_page() to happen first, and in fact those have usually led to simplifications, paving the way to make it easier to call vaddr_unpin_user_pages(), once it's ready. (And showing exactly what to convert, too.) If that makes the later conversion easier then no real objections from me. Assuming that the current put_user_page conversions are correct of course (I have the mlock one and potentials that falls into the same category in mind). Agreed: only correct conversions should be done. Not the incorrect ones. ahem. :) thanks, -- John Hubbard NVIDIA
Re: [PATCH v2 1/3] dt-bindings: Document JZ47xx VPU auxiliary processor
On Mon, Jul 29, 2019 at 02:31:07PM -0400, Paul Cercueil wrote: > Inside the Video Processing Unit (VPU) of the recent JZ47xx SoCs from > Ingenic is a second Xburst MIPS CPU very similar to the main core. > This document describes the devicetree bindings for this auxiliary > processor. > > Signed-off-by: Paul Cercueil > --- > > Notes: > v2: Update TCSM0 address in example > > .../bindings/remoteproc/ingenic,vpu.txt | 36 +++ > 1 file changed, 36 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/remoteproc/ingenic,vpu.txt > > diff --git a/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.txt > b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.txt > new file mode 100644 > index ..576f9e582780 > --- /dev/null > +++ b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.txt > @@ -0,0 +1,36 @@ > +* Ingenic JZ47xx auxiliary processor > + > +Inside the Video Processing Unit (VPU) of the recent JZ47xx SoCs from Ingenic > +is a second Xburst MIPS CPU very similar to the main core. > +This document describes the devicetree bindings for this auxiliary processor. > + > +Required properties: > +- compatible: Should be "ingenic,jz4770-vpu-rproc" > +- reg: Must contain the registers location and length for: > + * the auxiliary processor, > + * the Tightly Coupled Shared Memory 0 (TCSM0), > + * the Tightly Coupled Shared Memory 1 (TCSM1), > + * the shared SRAM. > +- reg-names: Must contain "aux", "tcsm0", "tcsm1", "sram". > +- clocks: Clock specifier for the AUX and VPU clocks. > +- clock-names: Must contain "aux", "vpu". > +- interrupts: Interrupt specifier for the VPU hardware block. > + > +Example: > + > +vpu: cpu@132a { cpu is reserved for CPUs under /cpus/. Use video-codec or video-decoder or ?? It's not clear what type of video processing this does. > + compatible = "ingenic,jz4770-vpu-rproc"; > + > + reg = <0x132a 0x20 /* AUX */ > +0x132b 0x4000 /* TCSM0 */ > +0x132c 0xc000 /* TCSM1 */ > +0x132f 0x7000 /* SRAM */ > + >; > + reg-names = "aux", "tcsm0", "tcsm1", "sram"; > + > + clocks = < JZ4770_CLK_AUX>, < JZ4770_CLK_VPU>; > + clock-names = "aux", "vpu"; > + > + interrupt-parent = <>; > + interrupts = <3>; > +}; > -- > 2.21.0.593.g511ec345e18 >
Re: [patch 04/44] posix-cpu-timers: Fixup stale comment
On Tue, Aug 20, 2019 at 07:57:37PM +0200, Thomas Gleixner wrote: > On Tue, 20 Aug 2019, Frederic Weisbecker wrote: > > On Mon, Aug 19, 2019 at 04:31:45PM +0200, Thomas Gleixner wrote: > > > /* > > > - * Clean out CPU timers still ticking when a thread exited. The task > > > - * pointer is cleared, and the expiry time is replaced with the residual > > > - * time for later timer_gettime calls to return. > > > + * Clean out CPU timers which are still armed when a thread exits. The > > > + * timers are only removed from the list. No other updates are done. The > > > + * corresponding posix timers are still accessible, but cannot be > > > rearmed. > > > + * > > > * This must be called with the siglock held. > > > */ > > > static void cleanup_timers(struct list_head *head) > > > > Indeed and I believe we could avoid that step. We remove the sighand at the > > same > > time so those can't be accessed anymore anyway. > > > > exit_itimers() takes care of the last call release and could force remove > > from > > the list (although it might be taken care of in your series, haven't > > checked yet): > > No. The posix timer is not necessarily owned by the exiting task or > process. It can be owned by a different entity which has permissions, > e.g. parent. > > So those are not in the posix timer list of the exiting task, which gets > cleaned up in exit_itimers(). Those are in the list of the task which armed > the timer. The timer is merily queued in the 'active timers' list of the > exiting task and posix_cpu_timers_exit()/posix_cpu_timers_exit_group() > remove it before the task/signal structs go away. Sure, I understand there's two distinct things here: the owner that queues timers in owner->sig->posix_timers (cleaned in exit_itimers()) and the target that queues in target->[signal->]cputime_expires (cleaned in posix_cpu_timers_exit[_group](). So I'm wondering why we bother with posix_cpu_timers_exit[_group]() at all when exit_itimers() could handle the list deletion from target->[signal]->cputime_expires throughout posix_cpu_timer_del() as it already does on targets that still have their sighands. It would make things more simple to delete the timer off the target from the same caller and place and we could remove posix_cpu_timers_exit*(). Or is there something I'm awkwardly missing as usual? :-)
Re: [PATCH v2 net-next] netdevsim: Fix build error without CONFIG_INET
From: YueHaibing Date: Tue, 20 Aug 2019 22:14:46 +0800 > If CONFIG_INET is not set, building fails: > > drivers/net/netdevsim/dev.o: In function `nsim_dev_trap_report_work': > dev.c:(.text+0x67b): undefined reference to `ip_send_check' > > Use ip_fast_csum instead of ip_send_check to avoid > dependencies on CONFIG_INET. > > Reported-by: Hulk Robot > Fixes: da58f90f11f5 ("netdevsim: Add devlink-trap support") > Signed-off-by: YueHaibing > --- > v2: use ip_fast_csum instead of ip_send_check Applied, thank you.
Re: [PATCH 10/14] arm64: dts: meson-g12a: fix reset controller compatible
On Wed, Aug 14, 2019 at 4:32 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-g12a-u200.dt.yaml: reset-controller@1004: compatible:0: > 'amlogic,meson-g12a-reset' is not one of ['amlogic,meson8b-reset', > 'amlogic,meson-gxbb-reset', 'amlogic,meson-axg-reset'] > meson-g12a-sei510.dt.yaml: reset-controller@1004: compatible:0: > 'amlogic,meson-g12a-reset' is not one of ['amlogic,meson8b-reset', > 'amlogic,meson-gxbb-reset', 'amlogic,meson-axg-reset'] note to self: reference where the decision against a g12a reset compatible string was made -> [0] > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl [0] https://lkml.org/lkml/2019/2/7/358
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Tue, Aug 20, 2019 at 01:31:35PM -0700, Paul E. McKenney wrote: > On Tue, Aug 20, 2019 at 04:01:16PM +0200, Peter Zijlstra wrote: > > We really should get the compiler folks to give us a > > -fno-pointer-provenance. Waiting on the standards committee to get their > > act together seems unlikely, esp. given that some people actually seem > > to _want_ this nonsense :/ > > The reason that they want it is to enable some significant optimizations > in numerical code on the one hand and in heavily templated C++ code on > the other. Neither of which has much bearing on kernel code. > > Interested in coming to the next C standards committee meeting in October > to help me push for this? ;-) How about we try and get some compiler folks together at plumbers and bribe them with beer? Once we have our compiler knob, we happy :-)
Re: [PATCH 08/14] arm64: dts: meson-gxl: fix internal phy compatible
adding Jerome On Wed, Aug 14, 2019 at 4:31 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxl-s805x-libretech-ac.dt.yaml: ethernet-phy@8: compatible: > ['ethernet-phy-id0181.4400', 'ethernet-phy-ieee802.3-c22'] is not valid under > any of the given schemas > > Signed-off-by: Neil Armstrong > --- > arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi > b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi > index ee1ecdbcc958..43eb158bee24 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi > +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi > @@ -709,7 +709,7 @@ > #size-cells = <0>; > > internal_phy: ethernet-phy@8 { > - compatible = "ethernet-phy-id0181.4400", > "ethernet-phy-ieee802.3-c22"; > + compatible = "ethernet-phy-id0181.4400"; on G12A there was a specific reason (iirc it was because the PHY ID can be any arbitrary value programmed into some register) why we added it with a compatible string Jerome, do we have the same situation on GXL/GXM as well? if not I prefer to drop the compatible string because it's probably from a time where the PHY dt-bindings stated "add the PHY ID compatible string if you know it" while the actual suggestion was "only add it if reading the ID doesn't work for some reason" Martin
Re: [PATCH 14/14] arm64: dts: meson: fix boards regulators states format
On Wed, Aug 14, 2019 at 4:34 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxbb-odroidc2.dt.yaml: gpio-regulator-tf_io: states:0: Additional items > are not allowed (180, 1 were unexpected) > meson-gxbb-odroidc2.dt.yaml: gpio-regulator-tf_io: states:0: [330, 0, > 180, 1] is too long > meson-gxbb-nexbox-a95x.dt.yaml: gpio-regulator: states:0: Additional items > are not allowed (330, 1 were unexpected) > meson-gxbb-nexbox-a95x.dt.yaml: gpio-regulator: states:0: [180, 0, > 330, 1] is too long > meson-gxbb-p200.dt.yaml: gpio-regulator: states:0: Additional items are not > allowed (330, 1 were unexpected) > meson-gxbb-p200.dt.yaml: gpio-regulator: states:0: [180, 0, 330, 1] > is too long > meson-gxl-s905x-hwacom-amazetv.dt.yaml: gpio-regulator: states:0: Additional > items are not allowed (330, 1 were unexpected) > meson-gxl-s905x-hwacom-amazetv.dt.yaml: gpio-regulator: states:0: [180, > 0, 330, 1] is too long > meson-gxbb-p201.dt.yaml: gpio-regulator: states:0: Additional items are not > allowed (330, 1 were unexpected) > meson-gxbb-p201.dt.yaml: gpio-regulator: states:0: [180, 0, 330, 1] > is too long > meson-g12b-odroid-n2.dt.yaml: gpio-regulator-tf_io: states:0: Additional > items are not allowed (180, 1 were unexpected) > meson-g12b-odroid-n2.dt.yaml: gpio-regulator-tf_io: states:0: [330, 0, > 180, 1] is too long > meson-gxl-s905x-nexbox-a95x.dt.yaml: gpio-regulator: states:0: Additional > items are not allowed (330, 1 were unexpected) > meson-gxl-s905x-nexbox-a95x.dt.yaml: gpio-regulator: states:0: [180, 0, > 330, 1] is too long > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl note to self: NanoPi K2 and the Le Potato board .dts are already doing it "right"
Re: [PATCH 6/6] arm64: dts: add support for SM1 based SEI Robotics SEI610
Martin Blumenstingl writes: > Hi Neil, > > On Tue, Aug 20, 2019 at 4:43 PM Neil Armstrong > wrote: >> >> Add support for the Amlogic SM1 Based SEI610 board. >> >> The SM1 SoC is a derivative of the G12A SoC Family with : >> - Cortex-A55 core instead of A53 >> - more power domains, including USB & PCIe >> - a neural network co-processor (NNA) >> - a CSI input and image processor >> - some changes in the audio complex, thus not yet enabled >> >> The SEI610 board is a derivative of the SEI510 board with : >> - removed ADC based touch button, replaced with 3x GPIO buttons >> - physical switch disabling on-board MICs >> - USB-C port for USB 2.0 OTG >> - On-board FTDI USB2SERIAL port for Linux console >> >> Audio, Display and USB support will be added later when support of the >> corresponding power domains will be added, for now they are kept disabled. >> >> Signed-off-by: Neil Armstrong > I don't have any details about this board but overall this looks sane, so: > Acked-by: Martin Blumenstingl > > [...] >> + /* Used by Tuner, RGB Led & IR Emitter LED array */ >> + vddao_3v3_t: regultor-vddao_3v3_t { > that should be regulator-vddao_3v3_t - maybe Kevin can fix this up, if > not then we can still fix it with a follow-up patch I fixed it up while applying, Kevin
Re: [PATCH 11/14] arm64: dts: meson-g12a-x96-max: fix compatible
On Wed, Aug 14, 2019 at 4:33 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-g12a-x96-max.dt.yaml: /: compatible: ['amediatech,x96-max', > 'amlogic,u200', 'amlogic,g12a'] is not valid under any of the given schemas > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl [...] > - compatible = "amediatech,x96-max", "amlogic,u200", "amlogic,g12a"; > + compatible = "amediatech,x96-max", "amlogic,g12a"; only partially related: I wonder if we should add a s905x2 compatible string here and to the .dts filename (just like we separate the GXL variants s905x, s905d, s905w, ...)
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Tue, Aug 20, 2019 at 04:01:16PM +0200, Peter Zijlstra wrote: > On Fri, Aug 16, 2019 at 09:52:17PM -0700, Paul E. McKenney wrote: > > On Fri, Aug 16, 2019 at 03:57:43PM -0700, Linus Torvalds wrote: > > > > [ . . . ] > > > > > We add READ_ONCE and WRITE_ONCE annotations when they make sense. Not > > > because of some theoretical "compiler is free to do garbage" > > > arguments. If such garbage happens, we need to fix the compiler, the > > > same way we already do with > > > > > > -fno-strict-aliasing > > > > Yeah, the compete-with-FORTRAN stuff. :-/ > > > > There is some work going on in the C committee on this, where the > > theorists would like to restrict strict-alias based optimizations to > > enable better analysis tooling. And no, although the theorists are > > pushing in the direction we would like them to, as far as I can see > > they are not pushing as far as we would like. But it might be that > > -fno-strict-aliasing needs some upgrades as well. I expect to learn > > more at the next meeting in a few months. > > > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2364.pdf > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2363.pdf > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2362.pdf > > We really should get the compiler folks to give us a > -fno-pointer-provenance. Waiting on the standards committee to get their > act together seems unlikely, esp. given that some people actually seem > to _want_ this nonsense :/ The reason that they want it is to enable some significant optimizations in numerical code on the one hand and in heavily templated C++ code on the other. Neither of which has much bearing on kernel code. Interested in coming to the next C standards committee meeting in October to help me push for this? ;-) Thanx, Paul
Re: [PATCH 1/1] Fix: trace sched switch start/stop racy updates
On Tue, Aug 20, 2019 at 03:56:12PM +0200, Peter Zijlstra wrote: > On Sat, Aug 17, 2019 at 01:08:02AM -0700, Linus Torvalds wrote: > > > 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. > > Paulmck actually has an example of that somewhere; ISTR that particular > case actually got fixed by GCC, but I'd really _love_ for some compiler > people (both GCC and LLVM) to state that their respective compilers will > not do load/store tearing for machine word sized load/stores. I do very much recall such an example, but I am now unable to either find it or reproduce it. :-/ If I cannot turn it up in a few days, I will ask the LWN editors to make appropriate changes to the "Who is afraid" article. > Without this written guarantee (which supposedly was in older GCC > manuals but has since gone missing), I'm loathe to rely on it. > > Yes, it is very rare, but it is a massive royal pain to debug if/when it > does do happen. But from what I can see, Linus is OK with use of WRITE_ONCE() for data races on any variable for which there is at least one READ_ONCE(). So we can still use WRITE_ONCE() as we would like in our own code. Yes, you or I might be hit by someone else's omission of WRITE_ONCE(), it is better than the proverbial kick in the teeth. Of course, if anyone knows of a compiler/architecture combination that really does tear stores of 32-bit constants, please do not keep it a secret! After all, it would be good to get that addressed easily starting now rather than after a difficult and painful series of debugging sessions. Thanx, Paul
Re: [PATCH 04/14] arm64: dts: meson-gx: fix spifc compatible
On Wed, Aug 14, 2019 at 4:31 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxl-s805x-libretech-ac.dt.yaml: spi@8c80: compatible:0: > 'amlogic,meson-gx-spifc' is not one of ['amlogic,meson6-spifc', > 'amlogic,meson-gxbb-spifc'] > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
Re: [v2 PATCH] RISC-V: Optimize tlb flush path.
On Tue, 2019-08-20 at 14:21 +0530, Anup Patel wrote: > On Tue, Aug 20, 2019 at 6:17 AM Atish Patra > wrote: > > In RISC-V, tlb flush happens via SBI which is expensive. > > If the target cpumask contains a local hartid, some cost > > can be saved by issuing a local tlb flush as we do that > > in OpenSBI anyways. There is also no need of SBI call if > > cpumask is empty. > > > > Do a local flush first if current cpu is present in cpumask. > > Invoke SBI call only if target cpumask contains any cpus > > other than local cpu. > > > > Signed-off-by: Atish Patra > > --- > > arch/riscv/include/asm/tlbflush.h | 37 ++- > > > > 1 file changed, 31 insertions(+), 6 deletions(-) > > > > diff --git a/arch/riscv/include/asm/tlbflush.h > > b/arch/riscv/include/asm/tlbflush.h > > index b5e64dc19b9e..3f9cd17b5402 100644 > > --- a/arch/riscv/include/asm/tlbflush.h > > +++ b/arch/riscv/include/asm/tlbflush.h > > @@ -8,6 +8,7 @@ > > #define _ASM_RISCV_TLBFLUSH_H > > > > #include > > +#include > > #include > > > > /* > > @@ -42,20 +43,44 @@ static inline void flush_tlb_range(struct > > vm_area_struct *vma, > > > > #include > > > > -static inline void remote_sfence_vma(struct cpumask *cmask, > > unsigned long start, > > -unsigned long size) > > +static void __riscv_flush_tlb(struct cpumask *cmask, unsigned long > > start, > > + unsigned long size) > > { > > struct cpumask hmask; > > + unsigned int hartid; > > + unsigned int cpuid; > > > > cpumask_clear(); > > + > > + if (!cmask) { > > + riscv_cpuid_to_hartid_mask(cpu_online_mask, > > ); > > + goto issue_sfence; > > + } > > + > > + cpuid = get_cpu(); > > + if (cpumask_test_cpu(cpuid, cmask)) { > > + /* Save trap cost by issuing a local tlb flush here > > */ > > + if ((start == 0 && size == -1) || (size > > > PAGE_SIZE)) > > + local_flush_tlb_all(); > > + else if (size == PAGE_SIZE) > > + local_flush_tlb_page(start); > > + } > > + if (cpumask_any_but(cmask, cpuid) >= nr_cpu_ids) > > + goto done; > > + > > riscv_cpuid_to_hartid_mask(cmask, ); > > + hartid = cpuid_to_hartid_map(cpuid); > > + cpumask_clear_cpu(hartid, ); > > + > > +issue_sfence: > > sbi_remote_sfence_vma(hmask.bits, start, size); > > +done: > > + put_cpu(); > > } > > > > -#define flush_tlb_all() sbi_remote_sfence_vma(NULL, 0, -1) > > - > > +#define flush_tlb_all() __riscv_flush_tlb(NULL, 0, -1) > > #define flush_tlb_range(vma, start, end) \ > > - remote_sfence_vma(mm_cpumask((vma)->vm_mm), start, (end) - > > (start)) > > + __riscv_flush_tlb(mm_cpumask((vma)->vm_mm), start, (end) - > > (start)) > > > > static inline void flush_tlb_page(struct vm_area_struct *vma, > > unsigned long addr) { > > @@ -63,7 +88,7 @@ static inline void flush_tlb_page(struct > > vm_area_struct *vma, > > } > > > > #define flush_tlb_mm(mm) \ > > - remote_sfence_vma(mm_cpumask(mm), 0, -1) > > + __riscv_flush_tlb(mm_cpumask(mm), 0, -1) > > > > #endif /* CONFIG_SMP */ > > > > -- > > 2.21.0 > > > > I think we should move __riscv_flush_tlb() to mm/tlbflush.c because > it's quite > big now. > > In future, we will also have __riscv_flush_tlb_asid() which will > flush TLB based > on ASID. > Sounds good to me. Christoph has already mm/tlbflush.c in his mmu series. I will rebase on top of it. > Regards, > Anup -- Regards, Atish
Re: [PATCH 02/14] arm64: dts: meson-gx: drop the vpu dmc memory cell
On Wed, Aug 14, 2019 at 4:31 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxl-s805x-libretech-ac.dt.yaml: vpu@d010: reg-names: Additional > items are not allowed ('dmc' was unexpected) > meson-gxl-s805x-libretech-ac.dt.yaml: vpu@d010: reg-names: ['vpu', 'hhi', > 'dmc'] is too long if you have to re-send it for whatever reason I would add: " The 'dmc' register area was replaced by the amlogic,canvas property which was introduced in commit f1726043426c73 ("arm64: dts: meson-gx: add dmcbus and canvas nodes.") and commit cf34287986d0b6 ("arm64: dts: meson-gx: Add canvas provider node to the vpu") " > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
Re: [PATCH] dt-bindings: arm: Add kryo485 compatible
On Tue, Aug 20, 2019 at 10:40:20PM +0530, Vinod Koul wrote: > Kryo485 is found in SM8150, so add it it list of cpu compatibles > > Signed-off-by: Vinod Koul > --- > Documentation/devicetree/bindings/arm/cpus.yaml | 1 + > 1 file changed, 1 insertion(+) Applied, thanks. Rob
Re: [v2 PATCH] RISC-V: Optimize tlb flush path.
On Tue, 2019-08-20 at 02:22 -0700, h...@infradead.org wrote: > On Tue, Aug 20, 2019 at 08:42:19AM +, Atish Patra wrote: > > cmask NULL is pretty common case and we would be unnecessarily > > executing bunch of instructions everytime while not saving much. > > Kernel > > still have to make an SBI call and OpenSBI is doing a local flush > > anyways. > > > > Looking at the code again, I think we can just use cpumask_weight > > and > > do local tlb flush only if local cpu is the only cpu present. > > > > Otherwise, it will just fall through and call > > sbi_remote_sfence_vma(). > > Maybe it is just time to split the different cases at a higher level. > The idea to multiple everything onto a single function always seemed > odd to me. > > FYI, here is what I do for the IPI based tlbflush for the native S- > mode > clint prototype, which seems much easier to understand: > > http://git.infradead.org/users/hch/riscv.git/commitdiff/ea4067ae61e20fcfcf46a6f6bd1cc25710ce3afe This does seem a lot cleaner to me. We can reuse some of the code for this patch as well. Based on NATIVE_CLINT configuration, it will send an IPI or SBI call. I can rebase my patch on top of yours and I can send it together or you can include in your series. Let me know your preference. -- Regards, Atish
Re: [PATCH v6 3/4] dt-bindings: arm: fsl: Add Kontron i.MX6UL N6310 compatibles
On Tue, Aug 20, 2019 at 3:21 PM Krzysztof Kozlowski wrote: > > On Tue, Aug 20, 2019 at 03:04:57PM -0500, Rob Herring wrote: > > On Tue, Aug 20, 2019 at 1:36 PM Krzysztof Kozlowski wrote: > > > > > > On Tue, 20 Aug 2019 at 18:59, Rob Herring wrote: > > > > > > > > On Tue, Aug 20, 2019 at 10:35 AM Krzysztof Kozlowski > > > > wrote: > > > > > > > > > > Add the compatibles for Kontron i.MX6UL N6310 SoM and boards. > > > > > > > > > > Signed-off-by: Krzysztof Kozlowski > > > > > > > > > > --- > > > > > > > > > > Changes since v5: > > > > > New patch > > > > > --- > > > > > Documentation/devicetree/bindings/arm/fsl.yaml | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > index 7294ac36f4c0..d07b3c06d7cf 100644 > > > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > @@ -161,6 +161,9 @@ properties: > > > > > items: > > > > >- enum: > > > > >- fsl,imx6ul-14x14-evk # i.MX6 UltraLite 14x14 > > > > > EVK Board > > > > > + - kontron,imx6ul-n6310-som # Kontron N6310 SOM > > > > > + - kontron,imx6ul-n6310-s# Kontron N6310 S Board > > > > > + - kontron,imx6ul-n6310-s-43 # Kontron N6310 S 43 Board > > > > > > > > This doesn't match what is in your dts files. Run 'make dtbs_check' and > > > > see. > > > > > > You mean the name does not match? I thought that '#' is a comment in > > > YAML... > > > > No, the number of compatible strings is the problem. > > I see. If I understand the schema correctly, this should look like: Looks correct, but a couple of comments. > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > b/Documentation/devicetree/bindings/arm/fsl.yaml > index 7294ac36f4c0..eb263d1ccf13 100644 > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > @@ -161,6 +161,22 @@ properties: > items: >- enum: >- fsl,imx6ul-14x14-evk # i.MX6 UltraLite 14x14 EVK Board > + - kontron,imx6ul-n6310-som # Kontron N6310 SOM Is the SOM ever used alone? If not, then no point in listing this here. > + - const: fsl,imx6ul > + > + - description: Kontron N6310 S Board > +items: > + - enum: > + - kontron,imx6ul-n6310-s This could be a 'const' instead. It depends if you think there will ever be more than one entry. > + - const: kontron,imx6ul-n6310-som > + - const: fsl,imx6ul > + > + - description: Kontron N6310 S 43 Board > +items: > + - enum: > + - kontron,imx6ul-n6310-s-43 > + - const: kontron,imx6ul-n6310-s > + - const: kontron,imx6ul-n6310-som >- const: fsl,imx6ul > >- description: i.MX6ULL based Boards > > > It passes the dtbs_check. Is it correct? > > Best regards, > Krzysztof >
Re: [PATCH 07/14] arm64: dts: meson-gx: fix periphs bus node name
On Wed, Aug 14, 2019 at 4:32 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxbb-nanopi-k2.dt.yaml: periphs@c8834000: $nodename:0: > 'periphs@c8834000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > meson-gxl-s805x-libretech-ac.dt.yaml: periphs@c8834000: $nodename:0: > 'periphs@c8834000' does not match '^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$' > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
[PATCH v8 08/16] crypto: caam - share definition for MAX_SDLEN
Both qi.h and cammalg_qi2.h seem to define identical versions of MAX_SDLEN. Move it to desc_constr.h to avoid duplication. Signed-off-by: Andrey Smirnov Cc: Chris Spencer Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: Horia Geantă Cc: Aymen Sghaier Cc: Leonard Crestez Cc: linux-cry...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/caamalg_qi2.h | 27 --- drivers/crypto/caam/desc_constr.h | 27 +++ drivers/crypto/caam/qi.h | 26 -- 3 files changed, 27 insertions(+), 53 deletions(-) diff --git a/drivers/crypto/caam/caamalg_qi2.h b/drivers/crypto/caam/caamalg_qi2.h index b450e2a25c1f..706736776b47 100644 --- a/drivers/crypto/caam/caamalg_qi2.h +++ b/drivers/crypto/caam/caamalg_qi2.h @@ -92,33 +92,6 @@ struct dpaa2_caam_priv_per_cpu { struct dpaa2_io *dpio; }; -/* - * The CAAM QI hardware constructs a job descriptor which points - * to shared descriptor (as pointed by context_a of FQ to CAAM). - * When the job descriptor is executed by deco, the whole job - * descriptor together with shared descriptor gets loaded in - * deco buffer which is 64 words long (each 32-bit). - * - * The job descriptor constructed by QI hardware has layout: - * - * HEADER (1 word) - * Shdesc ptr (1 or 2 words) - * SEQ_OUT_PTR (1 word) - * Out ptr (1 or 2 words) - * Out length (1 word) - * SEQ_IN_PTR (1 word) - * In ptr (1 or 2 words) - * In length (1 word) - * - * The shdesc ptr is used to fetch shared descriptor contents - * into deco buffer. - * - * Apart from shdesc contents, the total number of words that - * get loaded in deco buffer are '8' or '11'. The remaining words - * in deco buffer can be used for storing shared descriptor. - */ -#define MAX_SDLEN ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ) - /* Length of a single buffer in the QI driver memory cache */ #define CAAM_QI_MEMCACHE_SIZE 512 diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h index 536f360bf131..1fe50a4fefaa 100644 --- a/drivers/crypto/caam/desc_constr.h +++ b/drivers/crypto/caam/desc_constr.h @@ -18,6 +18,33 @@ #define CAAM_DESC_BYTES_MAX (CAAM_CMD_SZ * MAX_CAAM_DESCSIZE) #define DESC_JOB_IO_LEN (CAAM_CMD_SZ * 5 + CAAM_PTR_SZ * 3) +/* + * The CAAM QI hardware constructs a job descriptor which points + * to shared descriptor (as pointed by context_a of FQ to CAAM). + * When the job descriptor is executed by deco, the whole job + * descriptor together with shared descriptor gets loaded in + * deco buffer which is 64 words long (each 32-bit). + * + * The job descriptor constructed by QI hardware has layout: + * + * HEADER (1 word) + * Shdesc ptr (1 or 2 words) + * SEQ_OUT_PTR (1 word) + * Out ptr (1 or 2 words) + * Out length (1 word) + * SEQ_IN_PTR (1 word) + * In ptr (1 or 2 words) + * In length (1 word) + * + * The shdesc ptr is used to fetch shared descriptor contents + * into deco buffer. + * + * Apart from shdesc contents, the total number of words that + * get loaded in deco buffer are '8' or '11'. The remaining words + * in deco buffer can be used for storing shared descriptor. + */ +#define MAX_SDLEN ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ) + #ifdef DEBUG #define PRINT_POS do { printk(KERN_DEBUG "%02d: %s\n", desc_len(desc),\ &__func__[sizeof("append")]); } while (0) diff --git a/drivers/crypto/caam/qi.h b/drivers/crypto/caam/qi.h index f93c9c7ed430..db0549549e3b 100644 --- a/drivers/crypto/caam/qi.h +++ b/drivers/crypto/caam/qi.h @@ -14,32 +14,6 @@ #include "desc.h" #include "desc_constr.h" -/* - * CAAM hardware constructs a job descriptor which points to a shared descriptor - * (as pointed by context_a of to-CAAM FQ). - * When the job descriptor is executed by DECO, the whole job descriptor - * together with shared descriptor gets loaded in DECO buffer, which is - * 64 words (each 32-bit) long. - * - * The job descriptor constructed by CAAM hardware has the following layout: - * - * HEADER (1 word) - * Shdesc ptr (1 or 2 words) - * SEQ_OUT_PTR (1 word) - * Out ptr (1 or 2 words) - * Out length (1 word) - * SEQ_IN_PTR (1 word) - * In ptr (1 or 2 words) - * In length (1 word) - * - * The shdesc ptr is used to fetch shared descriptor contents into DECO buffer. - * - * Apart from shdesc contents, the total number of words that get loaded in DECO - * buffer are '8' or '11'. The remaining words in DECO buffer can be used for - * storing shared descriptor. - */ -#define MAX_SDLEN ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ) - /* Length of a single buffer in the QI driver memory cache */ #define CAAM_QI_MEMCACHE_SIZE 768 -- 2.21.0
[PATCH v8 06/16] crypto: caam - use ioread64*_hi_lo in rd_reg64
Following the same transformation logic as outlined in previous commit converting wr_reg64, convert rd_reg64 to use helpers from first. No functional change intended. Signed-off-by: Andrey Smirnov Reviewed-by: Horia Geantă Cc: Chris Spencer Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: Horia Geantă Cc: Aymen Sghaier Cc: Leonard Crestez Cc: linux-cry...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/regs.h | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h index 6acfef30a90c..4efc10534873 100644 --- a/drivers/crypto/caam/regs.h +++ b/drivers/crypto/caam/regs.h @@ -172,12 +172,20 @@ static inline void wr_reg64(void __iomem *reg, u64 data) static inline u64 rd_reg64(void __iomem *reg) { - if (!caam_imx && caam_little_end) - return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 | - (u64)rd_reg32((u32 __iomem *)(reg))); + if (caam_little_end) { + if (caam_imx) { + u32 low, high; - return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 | - (u64)rd_reg32((u32 __iomem *)(reg) + 1)); + high = ioread32(reg); + low = ioread32(reg + sizeof(u32)); + + return low + ((u64)high << 32); + } else { + return ioread64(reg); + } + } else { + return ioread64be(reg); + } } #endif /* CONFIG_64BIT */ -- 2.21.0
Re: [PATCH 12/14] arm64: dts: meson-gxbb-nanopi-k2: add missing model
On Wed, Aug 14, 2019 at 4:33 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxbb-nanopi-k2.dt.yaml: /: 'model' is a required property > > Signed-off-by: Neil Armstrong > --- > arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts > b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts > index c34c1c90ccb6..1a36d2bd2d21 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts > @@ -10,6 +10,7 @@ > > / { > compatible = "friendlyarm,nanopi-k2", "amlogic,meson-gxbb"; > + model = "Nanopi K2"; this should be "FriendlyARM NanoPi K2" to be consistent with other boards (for example meson-gxbb-odroidc2.dts)
[PATCH v8 05/16] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64
In order to be able to unify 64 and 32 bit implementations of wr_reg64, let's convert it to use helpers from first. Here are the steps of the transformation: 1. Inline wr_reg32 helpers: if (!caam_imx && caam_little_end) { if (caam_little_end) { iowrite32(data >> 32, (u32 __iomem *)(reg) + 1); iowrite32(data, (u32 __iomem *)(reg)); } else { iowrite32be(data >> 32, (u32 __iomem *)(reg) + 1); iowrite32be(data, (u32 __iomem *)(reg)); } } else { if (caam_little_end) { iowrite32(data >> 32, (u32 __iomem *)(reg)); iowrite32(data, (u32 __iomem *)(reg) + 1); } else { iowrite32be(data >> 32, (u32 __iomem *)(reg)); iowrite32be(data, (u32 __iomem *)(reg) + 1); } } 2. Transfrom the conditionals such that the check for 'caam_little_end' is at the top level: if (caam_little_end) { if (!caam_imx) { iowrite32(data >> 32, (u32 __iomem *)(reg) + 1); iowrite32(data, (u32 __iomem *)(reg)); } else { iowrite32(data >> 32, (u32 __iomem *)(reg)); iowrite32(data, (u32 __iomem *)(reg) + 1); } } else { iowrite32be(data >> 32, (u32 __iomem *)(reg)); iowrite32be(data, (u32 __iomem *)(reg) + 1); } 3. Invert the check for !caam_imx: if (caam_little_end) { if (caam_imx) { iowrite32(data >> 32, (u32 __iomem *)(reg)); iowrite32(data, (u32 __iomem *)(reg) + 1); } else { iowrite32(data >> 32, (u32 __iomem *)(reg) + 1); iowrite32(data, (u32 __iomem *)(reg)); } } else { iowrite32be(data >> 32, (u32 __iomem *)(reg)); iowrite32be(data, (u32 __iomem *)(reg) + 1); } 4. Make use of iowrite64* helpers from if (caam_little_end) { if (caam_imx) { iowrite32(data >> 32, (u32 __iomem *)(reg)); iowrite32(data, (u32 __iomem *)(reg) + 1); } else { iowrite64(data, reg); } } else { iowrite64be(data, reg); } No functional change intended. Signed-off-by: Andrey Smirnov Reviewed-by: Horia Geantă Cc: Chris Spencer Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: Horia Geantă Cc: Aymen Sghaier Cc: Leonard Crestez Cc: linux-cry...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/regs.h | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h index 7c7ea8af6a48..6acfef30a90c 100644 --- a/drivers/crypto/caam/regs.h +++ b/drivers/crypto/caam/regs.h @@ -12,6 +12,7 @@ #include #include #include +#include /* * Architecture-specific register access methods @@ -157,12 +158,15 @@ static inline u64 rd_reg64(void __iomem *reg) #else /* CONFIG_64BIT */ static inline void wr_reg64(void __iomem *reg, u64 data) { - if (!caam_imx && caam_little_end) { - wr_reg32((u32 __iomem *)(reg) + 1, data >> 32); - wr_reg32((u32 __iomem *)(reg), data); + if (caam_little_end) { + if (caam_imx) { + iowrite32(data >> 32, (u32 __iomem *)(reg)); + iowrite32(data, (u32 __iomem *)(reg) + 1); + } else { + iowrite64(data, reg); + } } else { - wr_reg32((u32 __iomem *)(reg), data >> 32); - wr_reg32((u32 __iomem *)(reg) + 1, data); + iowrite64be(data, reg); } } -- 2.21.0
Re: [PATCH 13/14] arm64: dts: meson-gxbb-p201: fix snps, reset-delays-us format
On Wed, Aug 14, 2019 at 4:33 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxbb-p201.dt.yaml: ethernet@c941: snps,reset-delays-us: [[0, 1, > 100]] is too short > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
[PATCH v8 09/16] crypto: caam - make CAAM_PTR_SZ dynamic
In order to be able to configure CAAM pointer size at run-time, which needed to support i.MX8MQ, which is 64-bit SoC with 32-bit pointer size, convert CAAM_PTR_SZ to refer to a global variable of the same name ("caam_ptr_sz") and adjust the rest of the code accordingly. No functional change intended. Signed-off-by: Andrey Smirnov Cc: Chris Spencer Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: Horia Geantă Cc: Aymen Sghaier Cc: Leonard Crestez Cc: linux-cry...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/caamalg.c | 2 +- drivers/crypto/caam/caamhash.c| 2 +- drivers/crypto/caam/caamrng.c | 2 +- drivers/crypto/caam/ctrl.c| 1 + drivers/crypto/caam/desc_constr.h | 12 +--- drivers/crypto/caam/error.c | 3 +++ 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c index 947ba8ef487a..2053b3a71b48 100644 --- a/drivers/crypto/caam/caamalg.c +++ b/drivers/crypto/caam/caamalg.c @@ -74,7 +74,7 @@ #define CHACHAPOLY_DESC_JOB_IO_LEN (AEAD_DESC_JOB_IO_LEN + CAAM_CMD_SZ * 6) -#define DESC_MAX_USED_BYTES(CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) +#define DESC_MAX_USED_BYTES(CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN_MIN) #define DESC_MAX_USED_LEN (DESC_MAX_USED_BYTES / CAAM_CMD_SZ) struct caam_alg_entry { diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c index 8a07edb45dad..65399cb2a770 100644 --- a/drivers/crypto/caam/caamhash.c +++ b/drivers/crypto/caam/caamhash.c @@ -560,7 +560,7 @@ struct ahash_edesc { dma_addr_t sec4_sg_dma; int src_nents; int sec4_sg_bytes; - u32 hw_desc[DESC_JOB_IO_LEN / sizeof(u32)] cacheline_aligned; + u32 hw_desc[DESC_JOB_IO_LEN_MAX / sizeof(u32)] cacheline_aligned; struct sec4_sg_entry sec4_sg[0]; }; diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c index 7fbda1b08360..e8baacaabe07 100644 --- a/drivers/crypto/caam/caamrng.c +++ b/drivers/crypto/caam/caamrng.c @@ -53,7 +53,7 @@ L1_CACHE_BYTES) /* length of descriptors */ -#define DESC_JOB_O_LEN (CAAM_CMD_SZ * 2 + CAAM_PTR_SZ * 2) +#define DESC_JOB_O_LEN (CAAM_CMD_SZ * 2 + CAAM_PTR_SZ_MAX * 2) #define DESC_RNG_LEN (3 * CAAM_CMD_SZ) /* Buffer, its dma address and lock */ diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index 0b4007068c31..47b92451756f 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -602,6 +602,7 @@ static int caam_probe(struct platform_device *pdev) caam_imx = (bool)imx_soc_match; comp_params = rd_reg32(>perfmon.comp_parms_ms); + caam_ptr_sz = sizeof(dma_addr_t); caam_dpaa2 = !!(comp_params & CTPR_MS_DPAA2); ctrlpriv->qi_present = !!(comp_params & CTPR_MS_QI_MASK); diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h index 1fe50a4fefaa..89187831d74f 100644 --- a/drivers/crypto/caam/desc_constr.h +++ b/drivers/crypto/caam/desc_constr.h @@ -14,9 +14,14 @@ #define IMMEDIATE (1 << 23) #define CAAM_CMD_SZ sizeof(u32) -#define CAAM_PTR_SZ sizeof(dma_addr_t) +#define CAAM_PTR_SZ caam_ptr_sz +#define CAAM_PTR_SZ_MAX sizeof(dma_addr_t) +#define CAAM_PTR_SZ_MIN sizeof(u32) #define CAAM_DESC_BYTES_MAX (CAAM_CMD_SZ * MAX_CAAM_DESCSIZE) -#define DESC_JOB_IO_LEN (CAAM_CMD_SZ * 5 + CAAM_PTR_SZ * 3) +#define __DESC_JOB_IO_LEN(n) (CAAM_CMD_SZ * 5 + (n) * 3) +#define DESC_JOB_IO_LEN __DESC_JOB_IO_LEN(CAAM_PTR_SZ) +#define DESC_JOB_IO_LEN_MAX __DESC_JOB_IO_LEN(CAAM_PTR_SZ_MAX) +#define DESC_JOB_IO_LEN_MIN __DESC_JOB_IO_LEN(CAAM_PTR_SZ_MIN) /* * The CAAM QI hardware constructs a job descriptor which points @@ -43,7 +48,7 @@ * get loaded in deco buffer are '8' or '11'. The remaining words * in deco buffer can be used for storing shared descriptor. */ -#define MAX_SDLEN ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ) +#define MAX_SDLEN ((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN_MIN) / CAAM_CMD_SZ) #ifdef DEBUG #define PRINT_POS do { printk(KERN_DEBUG "%02d: %s\n", desc_len(desc),\ @@ -64,6 +69,7 @@ (LDOFF_ENABLE_AUTO_NFIFO << LDST_OFFSET_SHIFT)) extern bool caam_little_end; +extern size_t caam_ptr_sz; /* * HW fetches 4 S/G table entries at a time, irrespective of how many entries diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c index b7fbf1be37a4..17c6108b6d41 100644 --- a/drivers/crypto/caam/error.c +++ b/drivers/crypto/caam/error.c @@ -56,6 +56,9 @@ EXPORT_SYMBOL(caam_little_end); bool caam_imx; EXPORT_SYMBOL(caam_imx); +size_t caam_ptr_sz; +EXPORT_SYMBOL(caam_ptr_sz); + static const struct { u8 value; const char *error_text; -- 2.21.0
[PATCH v8 04/16] crypto: caam - request JR IRQ as the last step
In order to avoid any risk of JR IRQ request being handled while some of the resources used for that are not yet allocated move the code requesting said IRQ to the endo of caam_jr_init(). Signed-off-by: Andrey Smirnov Cc: Chris Spencer Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: Horia Geantă Cc: Aymen Sghaier Cc: Leonard Crestez Cc: linux-cry...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/jr.c | 34 +++--- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index ea02f7774f7c..98b308de42c0 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -428,38 +428,26 @@ static int caam_jr_init(struct device *dev) jrp = dev_get_drvdata(dev); - tasklet_init(>irqtask, caam_jr_dequeue, (unsigned long)dev); - - /* Connect job ring interrupt handler. */ - error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, IRQF_SHARED, -dev_name(dev), dev); - if (error) { - dev_err(dev, "can't connect JobR %d interrupt (%d)\n", - jrp->ridx, jrp->irq); - goto out_kill_deq; - } - error = caam_reset_hw_jr(dev); if (error) - goto out_kill_deq; + return error; - error = -ENOMEM; jrp->inpring = dmam_alloc_coherent(dev, sizeof(*jrp->inpring) * JOBR_DEPTH, , GFP_KERNEL); if (!jrp->inpring) - goto out_kill_deq; + return -ENOMEM; jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) * JOBR_DEPTH, , GFP_KERNEL); if (!jrp->outring) - goto out_kill_deq; + return -ENOMEM; jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo), GFP_KERNEL); if (!jrp->entinfo) - goto out_kill_deq; + return -ENOMEM; for (i = 0; i < JOBR_DEPTH; i++) jrp->entinfo[i].desc_addr_dma = !0; @@ -483,9 +471,17 @@ static int caam_jr_init(struct device *dev) (JOBR_INTC_COUNT_THLD << JRCFG_ICDCT_SHIFT) | (JOBR_INTC_TIME_THLD << JRCFG_ICTT_SHIFT)); - return 0; -out_kill_deq: - tasklet_kill(>irqtask); + tasklet_init(>irqtask, caam_jr_dequeue, (unsigned long)dev); + + /* Connect job ring interrupt handler. */ + error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, IRQF_SHARED, +dev_name(dev), dev); + if (error) { + dev_err(dev, "can't connect JobR %d interrupt (%d)\n", + jrp->ridx, jrp->irq); + tasklet_kill(>irqtask); + } + return error; } -- 2.21.0
[PATCH v8 07/16] crypto: caam - drop 64-bit only wr/rd_reg64()
Since 32-bit of both wr_reg64 and rd_reg64 now use 64-bit IO helpers, these functions should no longer be necessary. No functional change intended. Signed-off-by: Andrey Smirnov Reviewed-by: Horia Geantă Cc: Chris Spencer Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: Horia Geantă Cc: Aymen Sghaier Cc: Leonard Crestez Cc: linux-cry...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/regs.h | 19 --- 1 file changed, 19 deletions(-) diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h index 4efc10534873..489d6c1eec7d 100644 --- a/drivers/crypto/caam/regs.h +++ b/drivers/crypto/caam/regs.h @@ -138,24 +138,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set) *base + 0x : least-significant 32 bits *base + 0x0004 : most-significant 32 bits */ -#ifdef CONFIG_64BIT -static inline void wr_reg64(void __iomem *reg, u64 data) -{ - if (caam_little_end) - iowrite64(data, reg); - else - iowrite64be(data, reg); -} - -static inline u64 rd_reg64(void __iomem *reg) -{ - if (caam_little_end) - return ioread64(reg); - else - return ioread64be(reg); -} - -#else /* CONFIG_64BIT */ static inline void wr_reg64(void __iomem *reg, u64 data) { if (caam_little_end) { @@ -187,7 +169,6 @@ static inline u64 rd_reg64(void __iomem *reg) return ioread64be(reg); } } -#endif /* CONFIG_64BIT */ static inline u64 cpu_to_caam_dma64(dma_addr_t value) { -- 2.21.0
[PATCH v2] x86/mm/pti: in pti_clone_pgtable(), increase addr properly
Before 32-bit support, pti_clone_pmds() always adds PMD_SIZE to addr. This behavior changes after the 32-bit support: pti_clone_pgtable() increases addr by PUD_SIZE for pud_none(*pud) case, and increases addr by PMD_SIZE for pmd_none(*pmd) case. However, this is not accurate because addr may not be PUD_SIZE/PMD_SIZE aligned. Fix this issue by properly rounding up addr to next PUD_SIZE/PMD_SIZE in these two cases. The following explains how we debugged this issue: We use huge page for hot text and thus reduces iTLB misses. As we benchmark 5.2 based kernel (vs. 4.16 based), we found ~2.5x more iTLB misses. To figure out the issue, I use a debug patch that dumps page table for a pid. The following are information from the workload pid. For the 4.16 based kernel: host-4.16 # grep "x pmd" /sys/kernel/debug/page_tables/dump_pid 0x0060-0x00e0 8M USR ro PSE x pmd 0x81a0-0x81c0 2M ro PSE x pmd For the 5.2 based kernel before this patch: host-5.2-before # grep "x pmd" /sys/kernel/debug/page_tables/dump_pid 0x0060-0x00e0 8M USR ro PSE x pmd The 8MB text in pmd is from user space. 4.16 kernel has 1 pmd for the irq entry table; while 4.16 kernel doesn't have it. For the 5.2 based kernel after this patch: host-5.2-after # grep "x pmd" /sys/kernel/debug/page_tables/dump_pid 0x0060-0x00e0 8M USR ro PSE x pmd 0x8100-0x81e0 14M ro PSE GLB x pmd So after this patch, the 5.2 based kernel has 7 PMDs instead of 1 PMD in 4.16 kernel. This further reduces iTLB miss rate Cc: sta...@vger.kernel.org # v4.19+ Fixes: 16a3fe634f6a ("x86/mm/pti: Clone kernel-image on PTE level for 32 bit") Reviewed-by: Rik van Riel Signed-off-by: Song Liu Cc: Joerg Roedel Cc: Thomas Gleixner Cc: Dave Hansen Cc: Andy Lutomirski Cc: Peter Zijlstra --- arch/x86/mm/pti.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c index b196524759ec..1337494e22ef 100644 --- a/arch/x86/mm/pti.c +++ b/arch/x86/mm/pti.c @@ -330,13 +330,13 @@ pti_clone_pgtable(unsigned long start, unsigned long end, pud = pud_offset(p4d, addr); if (pud_none(*pud)) { - addr += PUD_SIZE; + addr = round_up(addr + 1, PUD_SIZE); continue; } pmd = pmd_offset(pud, addr); if (pmd_none(*pmd)) { - addr += PMD_SIZE; + addr = round_up(addr + 1, PMD_SIZE); continue; } -- 2.17.1
[PATCH v8 01/16] crypto: caam - move DMA mask selection into a function
Exactly the same code to figure out DMA mask is repeated twice in the driver code. To avoid repetition, move that logic into a standalone subroutine in intern.h. While at it re-shuffle the code to make it more readable with early returns. Signed-off-by: Andrey Smirnov Reviewed-by: Horia Geantă Cc: Chris Spencer Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: Horia Geantă Cc: Aymen Sghaier Cc: Leonard Crestez Cc: linux-cry...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/crypto/caam/ctrl.c | 11 +-- drivers/crypto/caam/intern.h | 20 drivers/crypto/caam/jr.c | 15 +-- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index e0590beae240..50336494f285 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -711,16 +711,7 @@ static int caam_probe(struct platform_device *pdev) JRSTART_JR1_START | JRSTART_JR2_START | JRSTART_JR3_START); - if (sizeof(dma_addr_t) == sizeof(u64)) { - if (caam_dpaa2) - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(49)); - else if (of_device_is_compatible(nprop, "fsl,sec-v5.0")) - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40)); - else - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36)); - } else { - ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); - } + ret = dma_set_mask_and_coherent(dev, caam_get_dma_mask(dev)); if (ret) { dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret); goto disable_caam_emi_slow; diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h index 6af84bbc612c..ec25d260fa40 100644 --- a/drivers/crypto/caam/intern.h +++ b/drivers/crypto/caam/intern.h @@ -10,6 +10,8 @@ #ifndef INTERN_H #define INTERN_H +#include "ctrl.h" + /* Currently comes from Kconfig param as a ^2 (driver-required) */ #define JOBR_DEPTH (1 << CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE) @@ -215,4 +217,22 @@ DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u32_ro, caam_debugfs_u32_get, NULL, "%llu\n"); DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u64_ro, caam_debugfs_u64_get, NULL, "%llu\n"); #endif +static inline u64 caam_get_dma_mask(struct device *dev) +{ + struct device_node *nprop = dev->of_node; + + if (sizeof(dma_addr_t) != sizeof(u64)) + return DMA_BIT_MASK(32); + + if (caam_dpaa2) + return DMA_BIT_MASK(49); + + if (of_device_is_compatible(nprop, "fsl,sec-v5.0-job-ring") || + of_device_is_compatible(nprop, "fsl,sec-v5.0")) + return DMA_BIT_MASK(40); + + return DMA_BIT_MASK(36); +} + + #endif /* INTERN_H */ diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index cea811fed320..4b25b2fa3d02 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -543,20 +543,7 @@ static int caam_jr_probe(struct platform_device *pdev) jrpriv->rregs = (struct caam_job_ring __iomem __force *)ctrl; - if (sizeof(dma_addr_t) == sizeof(u64)) { - if (caam_dpaa2) - error = dma_set_mask_and_coherent(jrdev, - DMA_BIT_MASK(49)); - else if (of_device_is_compatible(nprop, -"fsl,sec-v5.0-job-ring")) - error = dma_set_mask_and_coherent(jrdev, - DMA_BIT_MASK(40)); - else - error = dma_set_mask_and_coherent(jrdev, - DMA_BIT_MASK(36)); - } else { - error = dma_set_mask_and_coherent(jrdev, DMA_BIT_MASK(32)); - } + error = dma_set_mask_and_coherent(jrdev, caam_get_dma_mask(jrdev)); if (error) { dev_err(jrdev, "dma_set_mask_and_coherent failed (%d)\n", error); -- 2.21.0
Re: [PATCH 05/14] arm64: dts: meson-gx: fix watchdog compatible
On Wed, Aug 14, 2019 at 4:30 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxbb-nanopi-k2.dt.yaml: watchdog@98d0: compatible:0: > 'amlogic,meson-gx-wdt' is not one of ['amlogic,meson-gxbb-wdt'] > meson-gxl-s805x-libretech-ac.dt.yaml: watchdog@98d0: compatible:0: > 'amlogic,meson-gx-wdt' is not one of ['amlogic,meson-gxbb-wdt'] > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
Re: [patch 27/44] posix-cpu-timers: Provide array based access to expiry cache
On Mon, 19 Aug 2019, Ingo Molnar wrote: > * Thomas Gleixner wrote: > > struct posix_cputimers { > > - struct task_cputime cputime_expires; > > - struct list_headcpu_timers[CPUCLOCK_MAX]; > > + /* Temporary union until all users are cleaned up */ > > + union { > > + struct task_cputime cputime_expires; > > + u64 expiries[CPUCLOCK_MAX]; > > + }; > > + struct list_headcpu_timers[CPUCLOCK_MAX]; > > }; > > Could we please name this first_expiry[] or such, to make it clear that > this is cached value of the first expiry of all timers of this process, > instead of the rather vague 'expiries[]' naming? > > Also, while at it, after the above temporary transition union, the final > structure becomes: > > struct posix_cputimers { >u64 expiries[CPUCLOCK_MAX]; >struct list_headcpu_timers[CPUCLOCK_MAX]; > }; > > Wouldn't it be more natural and easier to read to have the list head and > the expiry together: > > struct posix_cputimer_list { > u64 first_expiry; > struct list_headlist; > }; > > struct posix_cputimers { > struct posix_cputimer_list timers[CPUCLOCK_MAX]; > }; > > ? > > This makes the array structure rather clear and the first_expiry field > mostly self-documenting. I kept the odd named expiries for the temporary union and then after the patch which removes the abused struct task_cputime, I applied a separate cleanup which looks similar to the above. Just the names are a bit different and more aligned to what we have in hrtimers: struct posix_cputimer_base { u64 nextevt; struct timerqueue_head tqhead; }; and then have struct posix_cputimers { struct posix_cputimer_base bases[CPUCLOCK_MAX]; }; I'll send out a new version after doing some more testing. Thanks, tglx
Re: [PATCH 01/14] arm64: dts: meson: fix ethernet mac reg format
On Wed, Aug 14, 2019 at 4:30 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-axg-s400.dt.yaml: soc: ethernet@ff3f:reg:0: [0, 4282318848, 0, > 65536, 0, 4284695872, 0, 8] is too long > meson-axg-s400.dt.yaml: ethernet@ff3f: reg: [[0, 4282318848, 0, 65536, 0, > 4284695872, 0, 8]] is too short > meson-g12a-u200.dt.yaml: soc: ethernet@ff3f:reg:0: [0, 4282318848, 0, > 65536, 0, 4284695872, 0, 8] is too long > meson-g12a-u200.dt.yaml: ethernet@ff3f: reg: [[0, 4282318848, 0, 65536, > 0, 4284695872, 0, 8]] is too short > meson-gxbb-nanopi-k2.dt.yaml: soc: ethernet@c941:reg:0: [0, 3376480256, > 0, 65536, 0, 3364046144, 0, 4] is too long > meson-gxl-s805x-libretech-ac.dt.yaml: soc: ethernet@c941:reg:0: [0, > 3376480256, 0, 65536, 0, 3364046144, 0, 4] is too lon if you have to re-send it for whatever reason I would appreciate if you could add: "while here, also drop the redundant reg property from meson-gxl.dtsi because it had the same value as meson-gx.dtsi from which it inherits" > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
Re: [PATCH 03/14] arm64: dts: meson-gx: fix reset controller compatible
On Wed, Aug 14, 2019 at 4:29 PM Neil Armstrong wrote: > > This fixes the following DT schemas check errors: > meson-gxbb-nanopi-k2.dt.yaml: reset-controller@4404: compatible:0: > 'amlogic,meson-gx-reset' is not one of ['amlogic,meson8b-reset', > 'amlogic,meson-gxbb-reset', 'amlogic,meson-axg-reset'] > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
Re: [PATCH v6 3/4] dt-bindings: arm: fsl: Add Kontron i.MX6UL N6310 compatibles
On Tue, Aug 20, 2019 at 03:04:57PM -0500, Rob Herring wrote: > On Tue, Aug 20, 2019 at 1:36 PM Krzysztof Kozlowski wrote: > > > > On Tue, 20 Aug 2019 at 18:59, Rob Herring wrote: > > > > > > On Tue, Aug 20, 2019 at 10:35 AM Krzysztof Kozlowski > > > wrote: > > > > > > > > Add the compatibles for Kontron i.MX6UL N6310 SoM and boards. > > > > > > > > Signed-off-by: Krzysztof Kozlowski > > > > > > > > --- > > > > > > > > Changes since v5: > > > > New patch > > > > --- > > > > Documentation/devicetree/bindings/arm/fsl.yaml | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > index 7294ac36f4c0..d07b3c06d7cf 100644 > > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > @@ -161,6 +161,9 @@ properties: > > > > items: > > > >- enum: > > > >- fsl,imx6ul-14x14-evk # i.MX6 UltraLite 14x14 EVK > > > > Board > > > > + - kontron,imx6ul-n6310-som # Kontron N6310 SOM > > > > + - kontron,imx6ul-n6310-s# Kontron N6310 S Board > > > > + - kontron,imx6ul-n6310-s-43 # Kontron N6310 S 43 Board > > > > > > This doesn't match what is in your dts files. Run 'make dtbs_check' and > > > see. > > > > You mean the name does not match? I thought that '#' is a comment in YAML... > > No, the number of compatible strings is the problem. I see. If I understand the schema correctly, this should look like: diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml index 7294ac36f4c0..eb263d1ccf13 100644 --- a/Documentation/devicetree/bindings/arm/fsl.yaml +++ b/Documentation/devicetree/bindings/arm/fsl.yaml @@ -161,6 +161,22 @@ properties: items: - enum: - fsl,imx6ul-14x14-evk # i.MX6 UltraLite 14x14 EVK Board + - kontron,imx6ul-n6310-som # Kontron N6310 SOM + - const: fsl,imx6ul + + - description: Kontron N6310 S Board +items: + - enum: + - kontron,imx6ul-n6310-s + - const: kontron,imx6ul-n6310-som + - const: fsl,imx6ul + + - description: Kontron N6310 S 43 Board +items: + - enum: + - kontron,imx6ul-n6310-s-43 + - const: kontron,imx6ul-n6310-s + - const: kontron,imx6ul-n6310-som - const: fsl,imx6ul - description: i.MX6ULL based Boards It passes the dtbs_check. Is it correct? Best regards, Krzysztof
Re: [PATCH net-next v4 1/2] dt-bindings: net: snps,dwmac: update reg minItems maxItems
On Tue, Aug 20, 2019 at 9:57 AM Neil Armstrong wrote: > > The Amlogic Meson DWMAC glue bindings needs a second reg cells for the > glue registers, thus update the reg minItems/maxItems to allow more > than a single reg cell. > > Also update the allwinner,sun7i-a20-gmac.yaml derivative schema to specify > maxItems to 1. this looks good to me because: - allwinner,sun7i-a20-gmac.yaml now restricts reg to maxItems 1 - allwinner,sun8i-a83t-emac.yaml already restricts reg to maxItems 1 - amlogic,meson-dwmac.yaml (introduced in patch 2 from this series) will need maxItems 2 - (these are all yaml based schemas for DWMAC IP) > Signed-off-by: Neil Armstrong > Acked-by: Rob Herring > Acked-by: Maxime Ripard Reviewed-by: Martin Blumenstingl
WARNING: refcount bug in chrdev_open
Hello, syzbot found the following crash on: HEAD commit:a69e9051 Merge tag 'xfs-5.3-fixes-2' of git://git.kernel.o.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1450b25a60 kernel config: https://syzkaller.appspot.com/x/.config?x=3d7eaed8496da4da dashboard link: https://syzkaller.appspot.com/bug?extid=1c85a21f1c6bc88eb388 compiler: clang version 9.0.0 (/home/glider/llvm/clang 80fee25776c2fb61e74c1ecb1a523375c2500b69) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1088edba60 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=104260 Bisection is inconclusive: the bug happens on the oldest tested release. bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17d7389c60 final crash:https://syzkaller.appspot.com/x/report.txt?x=1437389c60 console output: https://syzkaller.appspot.com/x/log.txt?x=1037389c60 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+1c85a21f1c6bc88eb...@syzkaller.appspotmail.com [ cut here ] refcount_t: increment on 0; use-after-free. WARNING: CPU: 1 PID: 12599 at lib/refcount.c:156 refcount_inc_checked+0x4b/0x50 lib/refcount.c:156 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 12599 Comm: syz-executor314 Not tainted 5.3.0-rc4+ #78 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1d8/0x2f8 lib/dump_stack.c:113 panic+0x25c/0x799 kernel/panic.c:219 __warn+0x22f/0x230 kernel/panic.c:576 report_bug+0x190/0x290 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] do_error_trap+0xd7/0x440 arch/x86/kernel/traps.c:272 do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:291 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1028 RIP: 0010:refcount_inc_checked+0x4b/0x50 lib/refcount.c:156 Code: 3d 2e 8b 68 05 01 75 08 e8 52 53 21 fe 5b 5d c3 e8 4a 53 21 fe c6 05 18 8b 68 05 01 48 c7 c7 34 4b 45 88 31 c0 e8 85 3a f4 fd <0f> 0b eb df 90 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 10 RSP: 0018:88808cb579b0 EFLAGS: 00010246 RAX: cd29ce2624bf4d00 RBX: 8880934dcc00 RCX: 88809eec65c0 RDX: RSI: 0001 RDI: RBP: 88808cb579b8 R08: 815cf524 R09: ed1015d640d2 R10: ed1015d640d2 R11: R12: dc00 R13: 8880934dcbc8 R14: 8880934dcc04 R15: 8880934dcbc8 kref_get include/linux/kref.h:45 [inline] kobject_get+0x91/0xc0 lib/kobject.c:644 cdev_get fs/char_dev.c:355 [inline] chrdev_open+0x17e/0x590 fs/char_dev.c:400 do_dentry_open+0x73b/0xf90 fs/open.c:797 vfs_open+0x73/0x80 fs/open.c:906 do_last fs/namei.c:3416 [inline] path_openat+0x1397/0x4460 fs/namei.c:3533 do_filp_open+0x192/0x3d0 fs/namei.c:3563 do_sys_open+0x29f/0x560 fs/open.c:1089 __do_sys_open fs/open.c:1107 [inline] __se_sys_open fs/open.c:1102 [inline] __x64_sys_open+0x87/0x90 fs/open.c:1102 do_syscall_64+0xfe/0x140 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x406311 Code: 75 14 b8 02 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 a4 18 00 00 c3 48 83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 02 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 RSP: 002b:7fb29f268960 EFLAGS: 0293 ORIG_RAX: 0002 RAX: ffda RBX: RCX: 00406311 RDX: RSI: RDI: 7fb29f268970 RBP: 6667 R08: 000f R09: 7fb29f269700 R10: 7fb29f2699d0 R11: 0293 R12: 006dbc3c R13: R14: R15: 317a7973 Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. For information about bisection process see: https://goo.gl/tpsmEJ#bisection syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
Hi, On 01/08/2019 19:01:20+0800, Ran Bi wrote: > diff --git a/drivers/rtc/rtc-mt2712.c b/drivers/rtc/rtc-mt2712.c > new file mode 100644 > index ..1eb71ca64c2c > --- /dev/null > +++ b/drivers/rtc/rtc-mt2712.c > @@ -0,0 +1,444 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2019 MediaTek Inc. > + * Author: Ran Bi > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MTK_RTC_DEV KBUILD_MODNAME You probably shouldn't do that and have a static string for the driver name. I probably doesn't matter much though because DT is used to probe the driver. > +#define MT2712_WRTGR 0x0078 > + > +/* we map HW YEAR 0 to 2000 because 2000 is the leap year */ > +#define MT2712_MIN_YEAR 2000 > +#define MT2712_BASE_YEAR 1900 > +#define MT2712_MIN_YEAR_OFFSET (MT2712_MIN_YEAR - MT2712_BASE_YEAR) > +#define MT2712_MAX_YEAR_OFFSET (MT2712_MIN_YEAR_OFFSET + 127) > + All those defines are unecessary, see below. > +struct mt2712_rtc { > + struct device *dev; Looking at the code closely, it seems this is only used for debug and error messages. Maybe you could use rtc_dev->dev instead. > + struct rtc_device *rtc_dev; > + void __iomem*base; > + int irq; > + u8 irq_wake_enabled; > +}; > + > +static inline u32 mt2712_readl(struct mt2712_rtc *rtc, u32 reg) > +{ > + return readl(rtc->base + reg); > +} > + > +static inline void mt2712_writel(struct mt2712_rtc *rtc, u32 reg, u32 val) > +{ > + writel(val, rtc->base + reg); > +} > + > +static void mt2712_rtc_write_trigger(struct mt2712_rtc *rtc) > +{ > + unsigned long timeout = jiffies + HZ/10; > + > + mt2712_writel(rtc, MT2712_WRTGR, 1); > + while (1) { > + if (!(mt2712_readl(rtc, MT2712_BBPU) & MT2712_BBPU_CBUSY)) > + break; > + > + if (time_after(jiffies, timeout)) { > + dev_err(rtc->dev, "%s time out!\n", __func__); > + break; > + } > + cpu_relax(); > + } > +} > + > +static void mt2712_rtc_writeif_unlock(struct mt2712_rtc *rtc) > +{ > + mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK1); > + mt2712_rtc_write_trigger(rtc); > + mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK2); > + mt2712_rtc_write_trigger(rtc); > +} > + > +static irqreturn_t rtc_irq_handler_thread(int irq, void *data) > +{ > + struct mt2712_rtc *rtc = data; > + u16 irqsta, irqen; > + > + mutex_lock(>rtc_dev->ops_lock); > + > + irqsta = mt2712_readl(rtc, MT2712_IRQ_STA); Do you have to lock that read? Is the register cleared on read? > + if (irqsta & MT2712_IRQ_STA_AL) { > + rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF); > + irqen = irqsta & ~MT2712_IRQ_EN_AL; > + > + mt2712_writel(rtc, MT2712_IRQ_EN, irqen); > + mt2712_rtc_write_trigger(rtc); > + > + mutex_unlock(>rtc_dev->ops_lock); > + return IRQ_HANDLED; > + } > + > + mutex_unlock(>rtc_dev->ops_lock); > + return IRQ_NONE; > +} > + > +static void __mt2712_rtc_read_time(struct mt2712_rtc *rtc, > +struct rtc_time *tm, int *sec) > +{ > + tm->tm_sec = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK; > + tm->tm_min = mt2712_readl(rtc, MT2712_TC_MIN) & MT2712_MIN_MASK; > + tm->tm_hour = mt2712_readl(rtc, MT2712_TC_HOU) & MT2712_HOU_MASK; > + tm->tm_mday = mt2712_readl(rtc, MT2712_TC_DOM) & MT2712_DOM_MASK; > + tm->tm_mon = mt2712_readl(rtc, MT2712_TC_MTH) & MT2712_MTH_MASK; > + tm->tm_year = mt2712_readl(rtc, MT2712_TC_YEA) & MT2712_YEA_MASK; > + > + *sec = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK; > +} > + > +static int mt2712_rtc_read_time(struct device *dev, struct rtc_time *tm) > +{ > + struct mt2712_rtc *rtc = dev_get_drvdata(dev); > + int sec; > + > + do { > + __mt2712_rtc_read_time(rtc, tm, ); > + } while (sec < tm->tm_sec); /* SEC has carried */ Shouldn't that be while (tm->tm_sec < sec)? > + > + /* HW register use 7 bits to store year data, minus > + * MT2712_MIN_YEAR_OFFSET brfore write year data to register, and plus > + * MT2712_MIN_YEAR_OFFSET back after read year from register > + */ > + tm->tm_year += MT2712_MIN_YEAR_OFFSET; Simply add 100 in __mt2712_rtc_read_time > + > + /* HW register start mon from one, but tm_mon start from zero. */ > + tm->tm_mon--; > + You can also do that in __mt2712_rtc_read_time. > + if (rtc_valid_tm(tm)) { This check is unnecessary, the validity is always checked by the core. > + dev_dbg(rtc->dev, "%s: invalid time %ptR\n", __func__, tm); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int mt2712_rtc_set_time(struct
Re: [PATCH 6/6] arm64: dts: add support for SM1 based SEI Robotics SEI610
Hi Neil, On Tue, Aug 20, 2019 at 4:43 PM Neil Armstrong wrote: > > Add support for the Amlogic SM1 Based SEI610 board. > > The SM1 SoC is a derivative of the G12A SoC Family with : > - Cortex-A55 core instead of A53 > - more power domains, including USB & PCIe > - a neural network co-processor (NNA) > - a CSI input and image processor > - some changes in the audio complex, thus not yet enabled > > The SEI610 board is a derivative of the SEI510 board with : > - removed ADC based touch button, replaced with 3x GPIO buttons > - physical switch disabling on-board MICs > - USB-C port for USB 2.0 OTG > - On-board FTDI USB2SERIAL port for Linux console > > Audio, Display and USB support will be added later when support of the > corresponding power domains will be added, for now they are kept disabled. > > Signed-off-by: Neil Armstrong I don't have any details about this board but overall this looks sane, so: Acked-by: Martin Blumenstingl [...] > + /* Used by Tuner, RGB Led & IR Emitter LED array */ > + vddao_3v3_t: regultor-vddao_3v3_t { that should be regulator-vddao_3v3_t - maybe Kevin can fix this up, if not then we can still fix it with a follow-up patch Martin
Re: [PATCH v6 3/4] dt-bindings: arm: fsl: Add Kontron i.MX6UL N6310 compatibles
On Tue, Aug 20, 2019 at 1:36 PM Krzysztof Kozlowski wrote: > > On Tue, 20 Aug 2019 at 18:59, Rob Herring wrote: > > > > On Tue, Aug 20, 2019 at 10:35 AM Krzysztof Kozlowski > > wrote: > > > > > > Add the compatibles for Kontron i.MX6UL N6310 SoM and boards. > > > > > > Signed-off-by: Krzysztof Kozlowski > > > > > > --- > > > > > > Changes since v5: > > > New patch > > > --- > > > Documentation/devicetree/bindings/arm/fsl.yaml | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > > > b/Documentation/devicetree/bindings/arm/fsl.yaml > > > index 7294ac36f4c0..d07b3c06d7cf 100644 > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > > > @@ -161,6 +161,9 @@ properties: > > > items: > > >- enum: > > >- fsl,imx6ul-14x14-evk # i.MX6 UltraLite 14x14 EVK > > > Board > > > + - kontron,imx6ul-n6310-som # Kontron N6310 SOM > > > + - kontron,imx6ul-n6310-s# Kontron N6310 S Board > > > + - kontron,imx6ul-n6310-s-43 # Kontron N6310 S 43 Board > > > > This doesn't match what is in your dts files. Run 'make dtbs_check' and see. > > You mean the name does not match? I thought that '#' is a comment in YAML... No, the number of compatible strings is the problem. > The dtbs_check fail on missing dt-mk-schema. Any reason why it is not > in the scripts? Because it is not just that script, but the whole project of scripts, schemas and meta-schemas. Read the instructions in Documentation/devicetree/writing-schema.md(.rst in next). Rob
Re: [PATCH AUTOSEL 5.2 09/44] intel_th: Use the correct style for SPDX License Identifier
On Tue, Aug 20, 2019 at 07:27:22AM -0700, Greg Kroah-Hartman wrote: On Tue, Aug 20, 2019 at 09:39:53AM -0400, Sasha Levin wrote: From: Nishad Kamdar [ Upstream commit fac7b714c514fcc41e1d6450c694b0a5f8d3 ] This patch corrects the SPDX License Identifier style in header files related to Drivers for Intel(R) Trace Hub controller. For C header files Documentation/process/license-rules.rst mandates C-like comments (opposed to C source files where C++ style should be used) Changes made by using a script provided by Joe Perches here: https://lkml.org/lkml/2019/2/7/46 Suggested-by: Joe Perches Signed-off-by: Nishad Kamdar Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin --- drivers/hwtracing/intel_th/msu.h | 2 +- drivers/hwtracing/intel_th/pti.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Not really a stable patch at all, unless you want to start backporting all SPDX changes (hint, NO we do not!) :) please drop this from everywhere. And what triggered this? It's just comment changes, shouldn't the autobot know to ignore those? It got a score just above my cutoff, and I missed it during review :( FWIW, there's no explicit rule to ignore documentation, AUTOSEL sort of learned to do it on it's own, but here it seems that the content of the commit message outweighed the code change metrics. -- Thanks, Sasha