* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Andy Lutomirski @ 2019-06-07 22:27 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu-cheng Yu, Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <f6de9073-9939-a20d-2196-25fa223cf3fc@intel.com>
> On Jun 7, 2019, at 2:09 PM, Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
>>> Huh, how does glibc know about all possible past and future legacy code
>>> in the application?
>> When dlopen() gets a legacy binary and the policy allows that, it will manage
>> the bitmap:
>>
>> If a bitmap has not been created, create one.
>> Set bits for the legacy code being loaded.
>
> I was thinking about code that doesn't go through GLIBC like JITs.
CRIU is another consideration: it would be rather annoying if CET programs can’t migrate between LA57 and normal machines.
^ permalink raw reply
* Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
From: Brendan Higgins @ 2019-06-07 22:22 UTC (permalink / raw)
To: Stephen Boyd
Cc: Iurii Zaikin, Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook,
Kieran Bingham, Luis Chamberlain, Peter Zijlstra, Rob Herring,
shuah, Theodore Ts'o, Masahiro Yamada, devicetree, dri-devel,
kunit-dev, open list:DOCUMENTATION, linux-fsdevel, linux-kbuild,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
linux-nvdimm, linux-um, Sasha Levin, Bird, Timothy,
Amir Goldstein, Dan Carpenter, Daniel Vetter, Jeff Dike,
Joel Stanley, Julia Lawall, Kevin Hilman, Knut Omang,
Logan Gunthorpe, Michael Ellerman, Petr Mladek, Randy Dunlap,
Richard Weinberger, David Rientjes, Steven Rostedt, wfg
In-Reply-To: <20190607190047.C3E7A20868@mail.kernel.org>
On Fri, Jun 7, 2019 at 12:00 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Iurii Zaikin (2019-06-05 18:29:42)
> > On Fri, May 17, 2019 at 11:22 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Brendan Higgins (2019-05-14 15:17:10)
> > > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > > > new file mode 100644
> > > > index 0000000000000..fe0f2bae66085
> > > > --- /dev/null
> > > > +++ b/kernel/sysctl-test.c
> > > > +
> > > > +
> > > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test)
> > > > +{
> > > > + struct ctl_table table = {
> > > > + .procname = "foo",
> > > > + .data = &test_data.int_0001,
> > > > + .maxlen = sizeof(int),
> > > > + .mode = 0644,
> > > > + .proc_handler = proc_dointvec,
> > > > + .extra1 = &i_zero,
> > > > + .extra2 = &i_one_hundred,
> > > > + };
> > > > + char input[] = "-9";
> > > > + size_t len = sizeof(input) - 1;
> > > > + loff_t pos = 0;
> > > > +
> > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos));
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
> > >
> > > Is the casting necessary? Or can the macro do a type coercion of the
> > > second parameter based on the first type?
> > Data field is defined as void* so I believe casting is necessary to
> > dereference it as a pointer to an array of ints. I don't think the
> > macro should do any type coercion that == operator wouldn't do.
> > I did change the cast to make it more clear that it's a pointer to an
> > array of ints being dereferenced.
>
> Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on
> both sides and cause a build warning/error if the types aren't the same.
> This would be similar to our min/max macros that complain about
> mismatched types in the comparisons. Then if a test developer needs to
> convert one type or the other they could do so with a
> KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to
> explicitly.
Good point. I would definitely like to do this, for me it is only a
question of how difficult it would be to make all that happen.
We will investigate and report back on it.
Thanks for the suggestion! It's a really good idea!
Cheers
^ permalink raw reply
* Re: [PATCH v2 12/22] docs: gpio: driver.rst: fix a bad tag
From: Linus Walleij @ 2019-06-07 22:08 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel@vger.kernel.org, Jonathan Corbet,
Bartosz Golaszewski, open list:GPIO SUBSYSTEM
In-Reply-To: <ee8f4f56658247a0ab0d9e2c16a9afafefe38da0.1559656538.git.mchehab+samsung@kernel.org>
On Tue, Jun 4, 2019 at 4:18 PM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
> With ReST, [foo]_ means a reference to foo, causing this warning:
>
> Documentation/driver-api/gpio/driver.rst:419: WARNING: Unknown target name: "devm".
>
> Fix it by using a literal for the name.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Looks identical to the v1 I already applied, so keeping
that one.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v12 05/13] clk: ingenic: Add driver for the TCU clocks
From: Paul Cercueil @ 2019-06-07 21:59 UTC (permalink / raw)
To: Stephen Boyd
Cc: Daniel Lezcano, James Hogan, Jason Cooper, Jonathan Corbet,
Lee Jones, Marc Zyngier, Mark Rutland, Michael Turquette,
Paul Burton, Ralf Baechle, Rob Herring, Thomas Gleixner,
Mathieu Malaterre, linux-kernel, devicetree, linux-mips,
linux-doc, linux-clk, od
In-Reply-To: <20190607212819.A5FAE208C3@mail.kernel.org>
Hi Stephen, thanks for the review.
Le ven. 7 juin 2019 à 23:28, Stephen Boyd <sboyd@kernel.org> a écrit :
> Quoting Paul Cercueil (2019-05-21 07:51:33)
>> diff --git a/drivers/clk/ingenic/Kconfig
>> b/drivers/clk/ingenic/Kconfig
>> index 34dc0da79c39..434893133eb4 100644
>> --- a/drivers/clk/ingenic/Kconfig
>> +++ b/drivers/clk/ingenic/Kconfig
>> @@ -1,4 +1,4 @@
>> -menu "Ingenic JZ47xx CGU drivers"
>> +menu "Ingenic JZ47xx drivers"
>> depends on MIPS
>>
>> config INGENIC_CGU_COMMON
>> @@ -44,4 +44,13 @@ config INGENIC_CGU_JZ4780
>>
>> If building for a JZ4780 SoC, you want to say Y here.
>>
>> +config INGENIC_TCU_CLK
>> + bool "Ingenic JZ47xx TCU clocks driver"
>> + default MACH_INGENIC
>> + depends on COMMON_CLK
>
> Does the INGENIC_TCU_CLK config even exist if COMMON_CLK is disabled?
> I
> suspect it's all part of the menuconfig so this depends is not useful?
Right, it can be dropped.
>> + select INGENIC_TCU
>> + help
>> + Support the clocks of the Timer/Counter Unit (TCU) of the
>> Ingenic
>> + JZ47xx SoCs.
>> +
>> endmenu
>> diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c
>> new file mode 100644
>> index 000000000000..7249225a6994
>> --- /dev/null
>> +++ b/drivers/clk/ingenic/tcu.c
>> @@ -0,0 +1,458 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * JZ47xx SoCs TCU clocks driver
>> + * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/mfd/ingenic-tcu.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <dt-bindings/clock/ingenic,tcu.h>
>> +
>> +/* 8 channels max + watchdog + OST */
>> +#define TCU_CLK_COUNT 10
>> +
>> +#define TCU_ERR(...) pr_crit("ingenic-tcu-clk: " __VA_ARGS__)
>
> Why is it pr_crit instead of pr_err()?
If the TCU timer clocks are not provided for any reason, the system
will have no timer, and the kernel will hang very early in the init
process. That's why I chose pr_crit().
>> +
>> +enum tcu_clk_parent {
>> + TCU_PARENT_PCLK,
>> + TCU_PARENT_RTC,
>> + TCU_PARENT_EXT,
>> +};
>> +
> [...]
>> +
>> +static int __init ingenic_tcu_register_clock(struct ingenic_tcu
>> *tcu,
>> + unsigned int idx, enum tcu_clk_parent
>> parent,
>> + const struct ingenic_tcu_clk_info *info,
>> + struct clk_hw_onecell_data *clocks)
>> +{
>> + struct ingenic_tcu_clk *tcu_clk;
>> + int err;
>> +
>> + tcu_clk = kzalloc(sizeof(*tcu_clk), GFP_KERNEL);
>> + if (!tcu_clk)
>> + return -ENOMEM;
>> +
>> + tcu_clk->hw.init = &info->init_data;
>> + tcu_clk->idx = idx;
>> + tcu_clk->info = info;
>> + tcu_clk->tcu = tcu;
>> +
>> + /* Reset channel and clock divider, set default parent */
>> + ingenic_tcu_enable_regs(&tcu_clk->hw);
>> + regmap_update_bits(tcu->map, info->tcsr_reg, 0xffff,
>> BIT(parent));
>> + ingenic_tcu_disable_regs(&tcu_clk->hw);
>> +
>> + err = clk_hw_register(NULL, &tcu_clk->hw);
>> + if (err)
>> + goto err_free_tcu_clk;
>> +
>> + err = clk_hw_register_clkdev(&tcu_clk->hw,
>> info->init_data.name, NULL);
>
> Do you have a use for clkdev? If DT lookups work just as well it would
> be better to skip clkdev registration.
OK.
>> + if (err)
>> + goto err_clk_unregister;
>> +
>> + clocks->hws[idx] = &tcu_clk->hw;
>> +
>> + return 0;
>> +
>> +err_clk_unregister:
>> + clk_hw_unregister(&tcu_clk->hw);
>> +err_free_tcu_clk:
>> + kfree(tcu_clk);
>> + return err;
>> +}
>> +
>> +static const struct ingenic_soc_info jz4740_soc_info = {
>> + .num_channels = 8,
>> + .has_ost = false,
>> + .has_tcu_clk = true,
>> +};
>> +
>> +static const struct ingenic_soc_info jz4725b_soc_info = {
>> + .num_channels = 6,
>> + .has_ost = true,
>> + .has_tcu_clk = true,
>> +};
>> +
>> +static const struct ingenic_soc_info jz4770_soc_info = {
>> + .num_channels = 8,
>> + .has_ost = true,
>> + .has_tcu_clk = false,
>> +};
>> +
>> +static const struct of_device_id ingenic_tcu_of_match[]
>> __initconst = {
>> + { .compatible = "ingenic,jz4740-tcu", .data =
>> &jz4740_soc_info, },
>> + { .compatible = "ingenic,jz4725b-tcu", .data =
>> &jz4725b_soc_info, },
>> + { .compatible = "ingenic,jz4770-tcu", .data =
>> &jz4770_soc_info, },
>> + { }
>> +};
>> +
>> +static int __init ingenic_tcu_probe(struct device_node *np)
>> +{
>> + const struct of_device_id *id =
>> of_match_node(ingenic_tcu_of_match, np);
>> + struct ingenic_tcu *tcu;
>> + struct regmap *map;
>> + unsigned int i;
>> + int ret;
>> +
>> + map = ingenic_tcu_get_regmap(np);
>> + if (IS_ERR(map))
>> + return PTR_ERR(map);
>> +
>> + tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
>> + if (!tcu)
>> + return -ENOMEM;
>> +
>> + tcu->map = map;
>> + tcu->soc_info = id->data;
>> +
>> + if (tcu->soc_info->has_tcu_clk) {
>> + tcu->clk = of_clk_get_by_name(np, "tcu");
>
> Is this clk necessary to read/write registers in this clk driver? And
> this clk isn't the parent of the clks? Why is it managed by Linux at
> all? Will there be a time when it's turned off?
For the SoCs which have the "tcu" clock, it has to be enabled for the
registers to be accessible, yes. And as you noticed, it is not the
parent of the timer clocks.
The "tcu" clock can be turned off during suspend, for instance.
> I'm asking because it looks like we're calling clk APIs from within
> clk
> provider implementation. That works right now because of our locking
> scheme, but this will put up another roadblock to making the prepare
> and
> enable locks not recursive. I've seen some drivers take an approach of
> enabling the clk when the provider is PM runtime active, and disable
> the
> clk when the provider is runtime PM inactive. This gets it out of the
> provider path and into the runtime PM path. If you take that approach
> then when we move the runtime PM code in clk core outside of the
> prepare
> lock we should be able to avoid any recursive locking scenarios.
Most of the code here works without a struct device, it wouldn't be
easy to
get it to work with runtime PM.
I can enable the "tcu" clock in the probe and just gate/ungate it in the
suspend/resume callbacks, that would work just fine. We don't need
anything
fancy here.
>> + if (IS_ERR(tcu->clk)) {
>> + ret = PTR_ERR(tcu->clk);
>> + TCU_ERR("Cannot get TCU clock\n");
>> + goto err_free_tcu;
>> + }
>> +
^ permalink raw reply
* Re: [PATCH 4/4] Documentation: x86: fix some typos
From: Fenghua Yu @ 2019-06-07 21:29 UTC (permalink / raw)
To: James Morse
Cc: linux-doc, x86, Jonathan Corbet, Reinette Chatre, Babu Moger,
linux-kernel
In-Reply-To: <20190607151409.15476-5-james.morse@arm.com>
On Fri, Jun 07, 2019 at 04:14:09PM +0100, James Morse wrote:
> These are all obvious typos.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Documentation/x86/resctrl_ui.rst | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/x86/resctrl_ui.rst b/Documentation/x86/resctrl_ui.rst
> index 866b66aa289b..5368cedfb530 100644
> --- a/Documentation/x86/resctrl_ui.rst
> +++ b/Documentation/x86/resctrl_ui.rst
> @@ -40,7 +40,7 @@ mount options are:
> Enable the MBA Software Controller(mba_sc) to specify MBA
> bandwidth in MBps
>
> -L2 and L3 CDP are controlled seperately.
> +L2 and L3 CDP are controlled separately.
>
> RDT features are orthogonal. A particular system may support only
> monitoring, only control, or both monitoring and control. Cache
> @@ -118,7 +118,7 @@ related to allocation:
> Corresponding region is pseudo-locked. No
> sharing allowed.
>
> -Memory bandwitdh(MB) subdirectory contains the following files
> +Memory bandwidth(MB) subdirectory contains the following files
> with respect to allocation:
>
> "min_bandwidth":
> @@ -209,7 +209,7 @@ All groups contain the following files:
> CPUs to/from this group. As with the tasks file a hierarchy is
> maintained where MON groups may only include CPUs owned by the
> parent CTRL_MON group.
> - When the resouce group is in pseudo-locked mode this file will
> + When the resource group is in pseudo-locked mode this file will
> only be readable, reflecting the CPUs associated with the
> pseudo-locked region.
>
> @@ -380,7 +380,7 @@ where L2 external is 10GBps (hence aggregate L2 external bandwidth is
> 240GBps) and L3 external bandwidth is 100GBps. Now a workload with '20
> threads, having 50% bandwidth, each consuming 5GBps' consumes the max L3
> bandwidth of 100GBps although the percentage value specified is only 50%
> -<< 100%. Hence increasing the bandwidth percentage will not yeild any
> +<< 100%. Hence increasing the bandwidth percentage will not yield any
> more bandwidth. This is because although the L2 external bandwidth still
> has capacity, the L3 external bandwidth is fully used. Also note that
> this would be dependent on number of cores the benchmark is run on.
> @@ -398,7 +398,7 @@ In order to mitigate this and make the interface more user friendly,
> resctrl added support for specifying the bandwidth in MBps as well. The
> kernel underneath would use a software feedback mechanism or a "Software
> Controller(mba_sc)" which reads the actual bandwidth using MBM counters
> -and adjust the memowy bandwidth percentages to ensure::
> +and adjust the memory bandwidth percentages to ensure::
>
> "actual bandwidth < user specified bandwidth".
>
> --
> 2.20.1
>
Acked-by: Fenghua Yu <fenghua.yu@intel.com>
Thanks.
-Fenghua
^ permalink raw reply
* Re: [PATCH 3/4] Documentation: x86: Clarify MBA takes MB as referring to mba_sc
From: Fenghua Yu @ 2019-06-07 21:28 UTC (permalink / raw)
To: James Morse
Cc: linux-doc, x86, Jonathan Corbet, Reinette Chatre, Babu Moger,
linux-kernel
In-Reply-To: <20190607151409.15476-4-james.morse@arm.com>
On Fri, Jun 07, 2019 at 04:14:08PM +0100, James Morse wrote:
> "If the MBA is specified in MB then user can enter the max b/w in MB"
> is a tautology. How can the user know if the schemata takes a percentage
> or a MB/s value?
>
> This is referring to whether the software controller is interpreting
> the schemata's value. Make this clear.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Documentation/x86/resctrl_ui.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/x86/resctrl_ui.rst b/Documentation/x86/resctrl_ui.rst
> index 638cd987937d..866b66aa289b 100644
> --- a/Documentation/x86/resctrl_ui.rst
> +++ b/Documentation/x86/resctrl_ui.rst
> @@ -677,8 +677,8 @@ allocations can overlap or not. The allocations specifies the maximum
> b/w that the group may be able to use and the system admin can configure
> the b/w accordingly.
>
> -If the MBA is specified in MB(megabytes) then user can enter the max b/w in MB
> -rather than the percentage values.
> +If resctrl is using the software controller (mba_sc) then user can enter the
> +max b/w in MB rather than the percentage values.
> ::
>
> # echo "L3:0=3;1=c\nMB:0=1024;1=500" > /sys/fs/resctrl/p0/schemata
> --
> 2.20.1
>
Acked-by: Fenghua Yu <fenghua.yu@intel.com>
Thanks.
-Fenghua
^ permalink raw reply
* Re: [PATCH 2/4] Documentation: x86: Remove cdpl2 unspported statement and fix capitalisation
From: Fenghua Yu @ 2019-06-07 21:26 UTC (permalink / raw)
To: James Morse
Cc: linux-doc, x86, Jonathan Corbet, Reinette Chatre, Babu Moger,
linux-kernel
In-Reply-To: <20190607151409.15476-3-james.morse@arm.com>
On Fri, Jun 07, 2019 at 04:14:07PM +0100, James Morse wrote:
> "L2 cache does not support code and data prioritization". This isn't
> true, elsewhere the document says it can be enabled with the cdpl2
> mount option.
>
> While we're here, these sample strings have lower-case code/data,
> which isn't how the kernel exports them.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Documentation/x86/resctrl_ui.rst | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/x86/resctrl_ui.rst b/Documentation/x86/resctrl_ui.rst
> index 066f94e53418..638cd987937d 100644
> --- a/Documentation/x86/resctrl_ui.rst
> +++ b/Documentation/x86/resctrl_ui.rst
> @@ -418,16 +418,22 @@ L3 schemata file details (CDP enabled via mount option to resctrl)
> When CDP is enabled L3 control is split into two separate resources
> so you can specify independent masks for code and data like this::
>
> - L3data:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
> - L3code:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
> + L3DATA:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
> + L3CODE:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
>
> L2 schemata file details
> ------------------------
> -L2 cache does not support code and data prioritization, so the
> -schemata format is always::
> +CDP is supported at L2 using the 'cdpl2' mount option. The schemata
> +format is either::
>
> L2:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
>
> +or
> +
> + L2DATA:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
> + L2CODE:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
> +
> +
> Memory bandwidth Allocation (default mode)
> ------------------------------------------
>
> --
> 2.20.1
>
Acked-by: Fenghua Yu <fenghua.yu@intel.com>
Thanks.
-Fenghua
^ permalink raw reply
* Re: [PATCH 1/4] Documentation: x86: Contiguous cbm isn't all X86
From: Fenghua Yu @ 2019-06-07 21:24 UTC (permalink / raw)
To: James Morse
Cc: linux-doc, x86, Jonathan Corbet, Reinette Chatre, Babu Moger,
linux-kernel
In-Reply-To: <20190607151409.15476-2-james.morse@arm.com>
On Fri, Jun 07, 2019 at 04:14:06PM +0100, James Morse wrote:
> Since commit 4d05bf71f157 ("x86/resctrl: Introduce AMD QOS feature")
> resctrl has supported non-contiguous cache bit masks. The interface
> for this is currently try-it-and-see.
>
> Update the documentation to say Intel CPUs have this requirement,
> instead of X86.
>
> Cc: Babu Moger <Babu.Moger@amd.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Documentation/x86/resctrl_ui.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/x86/resctrl_ui.rst b/Documentation/x86/resctrl_ui.rst
> index 225cfd4daaee..066f94e53418 100644
> --- a/Documentation/x86/resctrl_ui.rst
> +++ b/Documentation/x86/resctrl_ui.rst
> @@ -342,7 +342,7 @@ For cache resources we describe the portion of the cache that is available
> for allocation using a bitmask. The maximum value of the mask is defined
> by each cpu model (and may be different for different cache levels). It
> is found using CPUID, but is also provided in the "info" directory of
> -the resctrl file system in "info/{resource}/cbm_mask". X86 hardware
> +the resctrl file system in "info/{resource}/cbm_mask". Intel hardware
> requires that these masks have all the '1' bits in a contiguous block. So
> 0x3, 0x6 and 0xC are legal 4-bit masks with two bits set, but 0x5, 0x9
> and 0xA are not. On a system with a 20-bit mask each bit represents 5%
> --
> 2.20.1
>
Acked-by: Fenghua Yu <fenghua.yu@intel.com>
Thanks.
-Fenghua Yu
^ permalink raw reply
* Re: [PATCH v12 05/13] clk: ingenic: Add driver for the TCU clocks
From: Stephen Boyd @ 2019-06-07 21:28 UTC (permalink / raw)
To: Daniel Lezcano, James Hogan, Jason Cooper, Jonathan Corbet,
Lee Jones, Marc Zyngier, Mark Rutland, Michael Turquette,
Paul Burton, Paul Cercueil, Ralf Baechle, Rob Herring,
Thomas Gleixner
Cc: Mathieu Malaterre, linux-kernel, devicetree, linux-mips,
linux-doc, linux-clk, od, Paul Cercueil
In-Reply-To: <20190521145141.9813-6-paul@crapouillou.net>
Quoting Paul Cercueil (2019-05-21 07:51:33)
> diff --git a/drivers/clk/ingenic/Kconfig b/drivers/clk/ingenic/Kconfig
> index 34dc0da79c39..434893133eb4 100644
> --- a/drivers/clk/ingenic/Kconfig
> +++ b/drivers/clk/ingenic/Kconfig
> @@ -1,4 +1,4 @@
> -menu "Ingenic JZ47xx CGU drivers"
> +menu "Ingenic JZ47xx drivers"
> depends on MIPS
>
> config INGENIC_CGU_COMMON
> @@ -44,4 +44,13 @@ config INGENIC_CGU_JZ4780
>
> If building for a JZ4780 SoC, you want to say Y here.
>
> +config INGENIC_TCU_CLK
> + bool "Ingenic JZ47xx TCU clocks driver"
> + default MACH_INGENIC
> + depends on COMMON_CLK
Does the INGENIC_TCU_CLK config even exist if COMMON_CLK is disabled? I
suspect it's all part of the menuconfig so this depends is not useful?
> + select INGENIC_TCU
> + help
> + Support the clocks of the Timer/Counter Unit (TCU) of the Ingenic
> + JZ47xx SoCs.
> +
> endmenu
> diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c
> new file mode 100644
> index 000000000000..7249225a6994
> --- /dev/null
> +++ b/drivers/clk/ingenic/tcu.c
> @@ -0,0 +1,458 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * JZ47xx SoCs TCU clocks driver
> + * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/clockchips.h>
> +#include <linux/mfd/ingenic-tcu.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/ingenic,tcu.h>
> +
> +/* 8 channels max + watchdog + OST */
> +#define TCU_CLK_COUNT 10
> +
> +#define TCU_ERR(...) pr_crit("ingenic-tcu-clk: " __VA_ARGS__)
Why is it pr_crit instead of pr_err()?
> +
> +enum tcu_clk_parent {
> + TCU_PARENT_PCLK,
> + TCU_PARENT_RTC,
> + TCU_PARENT_EXT,
> +};
> +
[...]
> +
> +static int __init ingenic_tcu_register_clock(struct ingenic_tcu *tcu,
> + unsigned int idx, enum tcu_clk_parent parent,
> + const struct ingenic_tcu_clk_info *info,
> + struct clk_hw_onecell_data *clocks)
> +{
> + struct ingenic_tcu_clk *tcu_clk;
> + int err;
> +
> + tcu_clk = kzalloc(sizeof(*tcu_clk), GFP_KERNEL);
> + if (!tcu_clk)
> + return -ENOMEM;
> +
> + tcu_clk->hw.init = &info->init_data;
> + tcu_clk->idx = idx;
> + tcu_clk->info = info;
> + tcu_clk->tcu = tcu;
> +
> + /* Reset channel and clock divider, set default parent */
> + ingenic_tcu_enable_regs(&tcu_clk->hw);
> + regmap_update_bits(tcu->map, info->tcsr_reg, 0xffff, BIT(parent));
> + ingenic_tcu_disable_regs(&tcu_clk->hw);
> +
> + err = clk_hw_register(NULL, &tcu_clk->hw);
> + if (err)
> + goto err_free_tcu_clk;
> +
> + err = clk_hw_register_clkdev(&tcu_clk->hw, info->init_data.name, NULL);
Do you have a use for clkdev? If DT lookups work just as well it would
be better to skip clkdev registration.
> + if (err)
> + goto err_clk_unregister;
> +
> + clocks->hws[idx] = &tcu_clk->hw;
> +
> + return 0;
> +
> +err_clk_unregister:
> + clk_hw_unregister(&tcu_clk->hw);
> +err_free_tcu_clk:
> + kfree(tcu_clk);
> + return err;
> +}
> +
> +static const struct ingenic_soc_info jz4740_soc_info = {
> + .num_channels = 8,
> + .has_ost = false,
> + .has_tcu_clk = true,
> +};
> +
> +static const struct ingenic_soc_info jz4725b_soc_info = {
> + .num_channels = 6,
> + .has_ost = true,
> + .has_tcu_clk = true,
> +};
> +
> +static const struct ingenic_soc_info jz4770_soc_info = {
> + .num_channels = 8,
> + .has_ost = true,
> + .has_tcu_clk = false,
> +};
> +
> +static const struct of_device_id ingenic_tcu_of_match[] __initconst = {
> + { .compatible = "ingenic,jz4740-tcu", .data = &jz4740_soc_info, },
> + { .compatible = "ingenic,jz4725b-tcu", .data = &jz4725b_soc_info, },
> + { .compatible = "ingenic,jz4770-tcu", .data = &jz4770_soc_info, },
> + { }
> +};
> +
> +static int __init ingenic_tcu_probe(struct device_node *np)
> +{
> + const struct of_device_id *id = of_match_node(ingenic_tcu_of_match, np);
> + struct ingenic_tcu *tcu;
> + struct regmap *map;
> + unsigned int i;
> + int ret;
> +
> + map = ingenic_tcu_get_regmap(np);
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
> + if (!tcu)
> + return -ENOMEM;
> +
> + tcu->map = map;
> + tcu->soc_info = id->data;
> +
> + if (tcu->soc_info->has_tcu_clk) {
> + tcu->clk = of_clk_get_by_name(np, "tcu");
Is this clk necessary to read/write registers in this clk driver? And
this clk isn't the parent of the clks? Why is it managed by Linux at
all? Will there be a time when it's turned off?
I'm asking because it looks like we're calling clk APIs from within clk
provider implementation. That works right now because of our locking
scheme, but this will put up another roadblock to making the prepare and
enable locks not recursive. I've seen some drivers take an approach of
enabling the clk when the provider is PM runtime active, and disable the
clk when the provider is runtime PM inactive. This gets it out of the
provider path and into the runtime PM path. If you take that approach
then when we move the runtime PM code in clk core outside of the prepare
lock we should be able to avoid any recursive locking scenarios.
> + if (IS_ERR(tcu->clk)) {
> + ret = PTR_ERR(tcu->clk);
> + TCU_ERR("Cannot get TCU clock\n");
> + goto err_free_tcu;
> + }
> +
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Dave Hansen @ 2019-06-07 21:09 UTC (permalink / raw)
To: Yu-cheng Yu, Andy Lutomirski
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <0e505563f7dae3849b57fb327f578f41b760b6f7.camel@intel.com>
On 6/7/19 1:06 PM, Yu-cheng Yu wrote:
>> Huh, how does glibc know about all possible past and future legacy code
>> in the application?
> When dlopen() gets a legacy binary and the policy allows that, it will manage
> the bitmap:
>
> If a bitmap has not been created, create one.
> Set bits for the legacy code being loaded.
I was thinking about code that doesn't go through GLIBC like JITs.
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Dave Hansen @ 2019-06-07 21:05 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Peter Zijlstra, Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <D10B5B59-1BE7-44DC-8E91-C8E4292DC6FB@amacapital.net>
On 6/7/19 1:40 PM, Andy Lutomirski wrote:
>>> Hmm. Can we be creative and skip populating it with zeros? The
>>> CPU
>> should only ever touch a page if we miss an ENDBR on it, so, in
>> normal operation, we don’t need anything to be there. We could try
>> to prevent anyone from *reading* it outside of ENDBR tracking if we
>> want to avoid people accidentally wasting lots of memory by forcing
>> it to be fully populated when the read it.
>>
>> Won't reads on a big, contiguous private mapping get the huge zero
>> page anyway?
>
> The zero pages may be free, but the page tables could be decently
large. Does the core mm code use huge, immense, etc huge zero pages?
Or can it synthesize them by reusing page table pages that map zeros?
IIRC, we only ever fill single PMDs, even though we could gang a pmd
page up and do it for 1GB areas too.
I guess the page table consumption could really suck if we had code all
over the 57-bit address space and that code moved around and the process
ran for a long long time. Pathologically, we need a ulong/pmd_t for
each 2MB of address space which is 8*2^56-30=512GB per process. Yikes.
Right now, we'd at least detect the memory consumption and OOM-kill the
process(es) eventually. But, that's not really _this_ patch's problem.
It's a general problem, and doesn't even require the zero page to be
mapped all over.
Longer-term, I'd much rather see us add some page table reclaim
mechanism that new how to go after things like excessive page tables in
MAP_NORESERVE areas.
^ permalink raw reply
* Re: [PATCH 16/22] docs: gpio: driver.rst: fix a bad tag
From: Linus Walleij @ 2019-06-07 20:57 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel@vger.kernel.org, Jonathan Corbet,
Bartosz Golaszewski, open list:GPIO SUBSYSTEM
In-Reply-To: <ab694e03ba6de8908d0a19a58408180eee3f795b.1559171394.git.mchehab+samsung@kernel.org>
On Thu, May 30, 2019 at 1:24 AM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:
> With ReST, [foo]_ means a reference to foo, causing this warning:
>
> Documentation/driver-api/gpio/driver.rst:419: WARNING: Unknown target name: "devm".
>
> Fix it by using a literal for the name.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Patch applied to the GPIO tree.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Andy Lutomirski @ 2019-06-07 20:43 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Dave Hansen, Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <7e0b97bf1fbe6ff20653a8e4e147c6285cc5552d.camel@intel.com>
> On Jun 7, 2019, at 12:49 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Fri, 2019-06-07 at 11:29 -0700, Andy Lutomirski wrote:
>>> On Jun 7, 2019, at 10:59 AM, Dave Hansen <dave.hansen@intel.com> wrote:
>>>
>>>> On 6/7/19 10:43 AM, Peter Zijlstra wrote:
>>>> I've no idea what the kernel should do; since you failed to answer the
>>>> question what happens when you point this to garbage.
>>>>
>>>> Does it then fault or what?
>>>
>>> Yeah, I think you'll fault with a rather mysterious CR2 value since
>>> you'll go look at the instruction that faulted and not see any
>>> references to the CR2 value.
>>>
>>> I think this new MSR probably needs to get included in oops output when
>>> CET is enabled.
>>
>> This shouldn’t be able to OOPS because it only happens at CPL 3, right? We
>> should put it into core dumps, though.
>>
>>>
>>> Why don't we require that a VMA be in place for the entire bitmap?
>>> Don't we need a "get" prctl function too in case something like a JIT is
>>> running and needs to find the location of this bitmap to set bits itself?
>>>
>>> Or, do we just go whole-hog and have the kernel manage the bitmap
>>> itself. Our interface here could be:
>>>
>>> prctl(PR_MARK_CODE_AS_LEGACY, start, size);
>>>
>>> and then have the kernel allocate and set the bitmap for those code
>>> locations.
>>
>> Given that the format depends on the VA size, this might be a good idea. I
>> bet we can reuse the special mapping infrastructure for this — the VMA could
>> be a MAP_PRIVATE special mapping named [cet_legacy_bitmap] or similar, and we
>> can even make special rules to core dump it intelligently if needed. And we
>> can make mremap() on it work correctly if anyone (CRIU?) cares.
>>
>> Hmm. Can we be creative and skip populating it with zeros? The CPU should
>> only ever touch a page if we miss an ENDBR on it, so, in normal operation, we
>> don’t need anything to be there. We could try to prevent anyone from
>> *reading* it outside of ENDBR tracking if we want to avoid people accidentally
>> wasting lots of memory by forcing it to be fully populated when the read it.
>>
>> The one downside is this forces it to be per-mm, but that seems like a
>> generally reasonable model anyway.
>>
>> This also gives us an excellent opportunity to make it read-only as seen from
>> userspace to prevent exploits from just poking it full of ones before
>> redirecting execution.
>
> GLIBC sets bits only for legacy code, and then makes the bitmap read-only. That
> avoids most issues:
How does glibc know the linear address space size? We don’t want LA64 to break old binaries because the address calculation changed.
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Andy Lutomirski @ 2019-06-07 20:40 UTC (permalink / raw)
To: Dave Hansen
Cc: Peter Zijlstra, Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <352e6172-938d-f8e4-c195-9fd1b881bdee@intel.com>
> On Jun 7, 2019, at 11:58 AM, Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 6/7/19 11:29 AM, Andy Lutomirski wrote:
> ...
>>> I think this new MSR probably needs to get included in oops output when
>>> CET is enabled.
>>
>> This shouldn’t be able to OOPS because it only happens at CPL 3,
>> right? We should put it into core dumps, though.
>
> Good point.
>
> Yu-cheng, can you just confirm that the bitmap can't be referenced in
> ring-0, no matter what? We should also make sure that no funny business
> happens if we put an address in the bitmap that faults, or is
> non-canonical. Do we have any self-tests for that?
>
> Let's say userspace gets a fault on this. Do they have the
> introspection capability to figure out why they faulted, say in their
> signal handler?
We need to stick the tracker state in the sigcontext somewhere.
Did we end up defining a signal frame shadow stack token?
>
>>> Why don't we require that a VMA be in place for the entire bitmap?
>>> Don't we need a "get" prctl function too in case something like a JIT is
>>> running and needs to find the location of this bitmap to set bits itself?
>>>
>>> Or, do we just go whole-hog and have the kernel manage the bitmap
>>> itself. Our interface here could be:
>>>
>>> prctl(PR_MARK_CODE_AS_LEGACY, start, size);
>>>
>>> and then have the kernel allocate and set the bitmap for those code
>>> locations.
>>
>> Given that the format depends on the VA size, this might be a good
>> idea.
>
> Yeah, making userspace know how large the address space is or could be
> is rather nasty, especially if we ever get any fancy CPU features that
> eat up address bits (a la ARM top-byte-ignore or SPARC ADI).
That gets extra bad if we ever grow user code that uses it but is unaware. It could poke the wrong part of the bitmap.
>
>> Hmm. Can we be creative and skip populating it with zeros? The CPU
> should only ever touch a page if we miss an ENDBR on it, so, in normal
> operation, we don’t need anything to be there. We could try to prevent
> anyone from *reading* it outside of ENDBR tracking if we want to avoid
> people accidentally wasting lots of memory by forcing it to be fully
> populated when the read it.
>
> Won't reads on a big, contiguous private mapping get the huge zero page
> anyway?
The zero pages may be free, but the page tables could be decently large. Does the core mm code use huge, immense, etc huge zero pages? Or can it synthesize them by reusing page table pages that map zeros?
>
>> The one downside is this forces it to be per-mm, but that seems like
>> a generally reasonable model anyway.
>
> Yeah, practically, you could only make it shared if you shared the
> layout of all code in the address space. I'm sure the big database(s)
> do that cross-process, but I bet nobody else does. User ASLR
> practically guarantees that nobody can do this.
I meant per-mm instead of per-task.
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Yu-cheng Yu @ 2019-06-07 20:06 UTC (permalink / raw)
To: Dave Hansen, Andy Lutomirski
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <4b448cde-ee4e-1c95-0f7f-4fe694be7db6@intel.com>
On Fri, 2019-06-07 at 13:00 -0700, Dave Hansen wrote:
> On 6/7/19 12:49 PM, Yu-cheng Yu wrote:
> > >
> > > This also gives us an excellent opportunity to make it read-only as seen
> > > from
> > > userspace to prevent exploits from just poking it full of ones before
> > > redirecting execution.
> >
> > GLIBC sets bits only for legacy code, and then makes the bitmap read-
> > only. That
> > avoids most issues:
> >
> > To populate bitmap pages, mprotect() is required.
> > Reading zero bitmap pages would not waste more physical memory, right?
>
> Huh, how does glibc know about all possible past and future legacy code
> in the application?
When dlopen() gets a legacy binary and the policy allows that, it will manage
the bitmap:
If a bitmap has not been created, create one.
Set bits for the legacy code being loaded.
Yu-cheng
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Yu-cheng Yu @ 2019-06-07 19:56 UTC (permalink / raw)
To: Dave Hansen, Andy Lutomirski
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <352e6172-938d-f8e4-c195-9fd1b881bdee@intel.com>
On Fri, 2019-06-07 at 11:58 -0700, Dave Hansen wrote:
> On 6/7/19 11:29 AM, Andy Lutomirski wrote:
> ...
> > > I think this new MSR probably needs to get included in oops output when
> > > CET is enabled.
> >
> > This shouldn’t be able to OOPS because it only happens at CPL 3,
> > right? We should put it into core dumps, though.
>
> Good point.
>
> Yu-cheng, can you just confirm that the bitmap can't be referenced in
> ring-0, no matter what? We should also make sure that no funny business
> happens if we put an address in the bitmap that faults, or is
> non-canonical. Do we have any self-tests for that?
Yes, the bitmap is user memory, but the kernel can still get to it (e.g.
copy_from_user()). We can do more check on the address.
>
> Let's say userspace gets a fault on this. Do they have the
> introspection capability to figure out why they faulted, say in their
> signal handler?
The bitmap address is kept by the application; the kernel won't provide it again
to user-space. In the signal handler, the app can find out from its own record.
[...]
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Dave Hansen @ 2019-06-07 20:00 UTC (permalink / raw)
To: Yu-cheng Yu, Andy Lutomirski
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <7e0b97bf1fbe6ff20653a8e4e147c6285cc5552d.camel@intel.com>
On 6/7/19 12:49 PM, Yu-cheng Yu wrote:
>>
>> This also gives us an excellent opportunity to make it read-only as seen from
>> userspace to prevent exploits from just poking it full of ones before
>> redirecting execution.
> GLIBC sets bits only for legacy code, and then makes the bitmap read-only. That
> avoids most issues:
>
> To populate bitmap pages, mprotect() is required.
> Reading zero bitmap pages would not waste more physical memory, right?
Huh, how does glibc know about all possible past and future legacy code
in the application?
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Yu-cheng Yu @ 2019-06-07 19:49 UTC (permalink / raw)
To: Andy Lutomirski, Dave Hansen
Cc: Peter Zijlstra, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <34E0D316-552A-401C-ABAA-5584B5BC98C5@amacapital.net>
On Fri, 2019-06-07 at 11:29 -0700, Andy Lutomirski wrote:
> > On Jun 7, 2019, at 10:59 AM, Dave Hansen <dave.hansen@intel.com> wrote:
> >
> > > On 6/7/19 10:43 AM, Peter Zijlstra wrote:
> > > I've no idea what the kernel should do; since you failed to answer the
> > > question what happens when you point this to garbage.
> > >
> > > Does it then fault or what?
> >
> > Yeah, I think you'll fault with a rather mysterious CR2 value since
> > you'll go look at the instruction that faulted and not see any
> > references to the CR2 value.
> >
> > I think this new MSR probably needs to get included in oops output when
> > CET is enabled.
>
> This shouldn’t be able to OOPS because it only happens at CPL 3, right? We
> should put it into core dumps, though.
>
> >
> > Why don't we require that a VMA be in place for the entire bitmap?
> > Don't we need a "get" prctl function too in case something like a JIT is
> > running and needs to find the location of this bitmap to set bits itself?
> >
> > Or, do we just go whole-hog and have the kernel manage the bitmap
> > itself. Our interface here could be:
> >
> > prctl(PR_MARK_CODE_AS_LEGACY, start, size);
> >
> > and then have the kernel allocate and set the bitmap for those code
> > locations.
>
> Given that the format depends on the VA size, this might be a good idea. I
> bet we can reuse the special mapping infrastructure for this — the VMA could
> be a MAP_PRIVATE special mapping named [cet_legacy_bitmap] or similar, and we
> can even make special rules to core dump it intelligently if needed. And we
> can make mremap() on it work correctly if anyone (CRIU?) cares.
>
> Hmm. Can we be creative and skip populating it with zeros? The CPU should
> only ever touch a page if we miss an ENDBR on it, so, in normal operation, we
> don’t need anything to be there. We could try to prevent anyone from
> *reading* it outside of ENDBR tracking if we want to avoid people accidentally
> wasting lots of memory by forcing it to be fully populated when the read it.
>
> The one downside is this forces it to be per-mm, but that seems like a
> generally reasonable model anyway.
>
> This also gives us an excellent opportunity to make it read-only as seen from
> userspace to prevent exploits from just poking it full of ones before
> redirecting execution.
GLIBC sets bits only for legacy code, and then makes the bitmap read-only. That
avoids most issues:
To populate bitmap pages, mprotect() is required.
Reading zero bitmap pages would not waste more physical memory, right?
Yu-cheng
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Yu-cheng Yu @ 2019-06-07 19:23 UTC (permalink / raw)
To: Dave Hansen, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue,
Dave Martin
In-Reply-To: <c5c21778-f10f-cef8-c937-1e8ad1e2a7cf@intel.com>
On Fri, 2019-06-07 at 12:03 -0700, Dave Hansen wrote:
> On 6/6/19 1:09 PM, Yu-cheng Yu wrote:
> > + modify_fpu_regs_begin();
> > + rdmsrl(MSR_IA32_U_CET, r);
> > + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> > + wrmsrl(MSR_IA32_U_CET, r);
> > + modify_fpu_regs_end();
>
> Isn't there a bunch of other stuff in this MSR? It seems like the
> bitmap value would allow overwriting lots of bits in the MSR that have
> nothing to do with the bitmap... in a prctl() that's supposed to only be
> dealing with the bitmap.
Yes, the bitmap address should have been masked, although it is checked for page
alignment (which has the same effect). I will fix it.
Yu-cheng
^ permalink raw reply
* Re: [PATCH] Documentation: xfs: Fix typo
From: Jonathan Corbet @ 2019-06-07 19:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Shiyang Ruan, linux-doc, linux-kernel, linux-xfs
In-Reply-To: <20190607183410.GF1871505@magnolia>
On Fri, 7 Jun 2019 11:34:10 -0700
"Darrick J. Wong" <darrick.wong@oracle.com> wrote:
> I doubt the value of maintaining duplicate copies of this document in
> the kernel and the xfs documentation repo, and since the xfs docs and
> kernel licences aren't compatible maybe we should withdraw one...
Um .... the in-kernel docs say nothing about an incompatible license. I
see only a GPL license at the repo you pointed to as well. Is there
something I don't know going on here? If the licenses aren't compatible
then those files shouldn't be there.
Thanks,
jon
^ permalink raw reply
* Applied "ASoC: dt-bindings: fix some broken links from txt->yaml conversion" to the asoc tree
From: Mark Brown @ 2019-06-07 19:07 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Alexandre Torgue, alsa-devel, Arnaud Pouliquen, devicetree,
Hugues Fruchet, Jonathan Corbet, Liam Girdwood, linux-arm-kernel,
Linux Doc Mailing List, linux-kernel, linux-stm32, Mark Brown,
Mark Rutland, Mauro Carvalho Chehab, Maxime Coquelin,
Olivier Moysan, Rob Herring
In-Reply-To: <effeafed3023d8dc5f2440c8d5637ea31c02a533.1559933665.git.mchehab+samsung@kernel.org>
The patch
ASoC: dt-bindings: fix some broken links from txt->yaml conversion
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 6c0215f5d9f2a1fa5cab2ca320a41d9f19cfa80c Mon Sep 17 00:00:00 2001
From: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Date: Fri, 7 Jun 2019 15:54:33 -0300
Subject: [PATCH] ASoC: dt-bindings: fix some broken links from txt->yaml
conversion
Some new files got converted to yaml, but references weren't
updated accordingly.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/media/st,stm32-dcmi.txt | 2 +-
Documentation/devicetree/bindings/sound/st,stm32-i2s.txt | 2 +-
Documentation/devicetree/bindings/sound/st,stm32-sai.txt | 2 +-
MAINTAINERS | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.txt b/Documentation/devicetree/bindings/media/st,stm32-dcmi.txt
index 249790a93017..3122ded82eb4 100644
--- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.txt
+++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.txt
@@ -11,7 +11,7 @@ Required properties:
- clock-names: must contain "mclk", which is the DCMI peripherial clock
- pinctrl: the pincontrol settings to configure muxing properly
for pins that connect to DCMI device.
- See Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt.
+ See Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml.
- dmas: phandle to DMA controller node,
see Documentation/devicetree/bindings/dma/stm32-dma.txt
- dma-names: must contain "tx", which is the transmit channel from DCMI to DMA
diff --git a/Documentation/devicetree/bindings/sound/st,stm32-i2s.txt b/Documentation/devicetree/bindings/sound/st,stm32-i2s.txt
index 58c341300552..cbf24bcd1b8d 100644
--- a/Documentation/devicetree/bindings/sound/st,stm32-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/st,stm32-i2s.txt
@@ -18,7 +18,7 @@ Required properties:
See Documentation/devicetree/bindings/dma/stm32-dma.txt.
- dma-names: Identifier for each DMA request line. Must be "tx" and "rx".
- pinctrl-names: should contain only value "default"
- - pinctrl-0: see Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
+ - pinctrl-0: see Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
Optional properties:
- resets: Reference to a reset controller asserting the reset controller
diff --git a/Documentation/devicetree/bindings/sound/st,stm32-sai.txt b/Documentation/devicetree/bindings/sound/st,stm32-sai.txt
index 3f4467ff0aa2..944743dd9212 100644
--- a/Documentation/devicetree/bindings/sound/st,stm32-sai.txt
+++ b/Documentation/devicetree/bindings/sound/st,stm32-sai.txt
@@ -41,7 +41,7 @@ SAI subnodes required properties:
"tx": if sai sub-block is configured as playback DAI
"rx": if sai sub-block is configured as capture DAI
- pinctrl-names: should contain only value "default"
- - pinctrl-0: see Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt
+ - pinctrl-0: see Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
SAI subnodes Optional properties:
- st,sync: specify synchronization mode.
diff --git a/MAINTAINERS b/MAINTAINERS
index 5cfbea4ce575..b3d686fba562 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1288,7 +1288,7 @@ ARM PRIMECELL SSP PL022 SPI DRIVER
M: Linus Walleij <linus.walleij@linaro.org>
L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
S: Maintained
-F: Documentation/devicetree/bindings/spi/spi_pl022.txt
+F: Documentation/devicetree/bindings/spi/spi-pl022.yaml
F: drivers/spi/spi-pl022.c
ARM PRIMECELL UART PL010 AND PL011 DRIVERS
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Dave Hansen @ 2019-06-07 19:03 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue,
Dave Martin
In-Reply-To: <20190606200926.4029-4-yu-cheng.yu@intel.com>
On 6/6/19 1:09 PM, Yu-cheng Yu wrote:
> + modify_fpu_regs_begin();
> + rdmsrl(MSR_IA32_U_CET, r);
> + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> + wrmsrl(MSR_IA32_U_CET, r);
> + modify_fpu_regs_end();
Isn't there a bunch of other stuff in this MSR? It seems like the
bitmap value would allow overwriting lots of bits in the MSR that have
nothing to do with the bitmap... in a prctl() that's supposed to only be
dealing with the bitmap.
^ permalink raw reply
* Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
From: Stephen Boyd @ 2019-06-07 19:00 UTC (permalink / raw)
To: Iurii Zaikin
Cc: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
kieran.bingham, mcgrof, peterz, robh, shuah, tytso,
yamada.masahiro, devicetree, dri-devel, kunit-dev, linux-doc,
linux-fsdevel, linux-kbuild, linux-kernel, linux-kselftest,
linux-nvdimm, linux-um, Alexander.Levin, Tim.Bird, amir73il,
dan.carpenter, daniel, jdike, joel, julia.lawall, khilman,
knut.omang, logang, mpe, pmladek, rdunlap, richard, rientjes,
rostedt, wfg
In-Reply-To: <CAAXuY3p4qhKVsSpQ44_kQeGDMfg7OuFLgFyxhcFWS3yf-5A_7g@mail.gmail.com>
Quoting Iurii Zaikin (2019-06-05 18:29:42)
> On Fri, May 17, 2019 at 11:22 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Brendan Higgins (2019-05-14 15:17:10)
> > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > > new file mode 100644
> > > index 0000000000000..fe0f2bae66085
> > > --- /dev/null
> > > +++ b/kernel/sysctl-test.c
> > > +
> > > +
> > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test)
> > > +{
> > > + struct ctl_table table = {
> > > + .procname = "foo",
> > > + .data = &test_data.int_0001,
> > > + .maxlen = sizeof(int),
> > > + .mode = 0644,
> > > + .proc_handler = proc_dointvec,
> > > + .extra1 = &i_zero,
> > > + .extra2 = &i_one_hundred,
> > > + };
> > > + char input[] = "-9";
> > > + size_t len = sizeof(input) - 1;
> > > + loff_t pos = 0;
> > > +
> > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos));
> > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
> >
> > Is the casting necessary? Or can the macro do a type coercion of the
> > second parameter based on the first type?
> Data field is defined as void* so I believe casting is necessary to
> dereference it as a pointer to an array of ints. I don't think the
> macro should do any type coercion that == operator wouldn't do.
> I did change the cast to make it more clear that it's a pointer to an
> array of ints being dereferenced.
Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on
both sides and cause a build warning/error if the types aren't the same.
This would be similar to our min/max macros that complain about
mismatched types in the comparisons. Then if a test developer needs to
convert one type or the other they could do so with a
KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to
explicitly.
^ permalink raw reply
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Dave Hansen @ 2019-06-07 18:58 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Peter Zijlstra, Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, linux-kernel, linux-doc, linux-mm, linux-arch,
linux-api, Arnd Bergmann, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <34E0D316-552A-401C-ABAA-5584B5BC98C5@amacapital.net>
On 6/7/19 11:29 AM, Andy Lutomirski wrote:
...
>> I think this new MSR probably needs to get included in oops output when
>> CET is enabled.
>
> This shouldn’t be able to OOPS because it only happens at CPL 3,
> right? We should put it into core dumps, though.
Good point.
Yu-cheng, can you just confirm that the bitmap can't be referenced in
ring-0, no matter what? We should also make sure that no funny business
happens if we put an address in the bitmap that faults, or is
non-canonical. Do we have any self-tests for that?
Let's say userspace gets a fault on this. Do they have the
introspection capability to figure out why they faulted, say in their
signal handler?
>> Why don't we require that a VMA be in place for the entire bitmap?
>> Don't we need a "get" prctl function too in case something like a JIT is
>> running and needs to find the location of this bitmap to set bits itself?
>>
>> Or, do we just go whole-hog and have the kernel manage the bitmap
>> itself. Our interface here could be:
>>
>> prctl(PR_MARK_CODE_AS_LEGACY, start, size);
>>
>> and then have the kernel allocate and set the bitmap for those code
>> locations.
>
> Given that the format depends on the VA size, this might be a good
> idea.
Yeah, making userspace know how large the address space is or could be
is rather nasty, especially if we ever get any fancy CPU features that
eat up address bits (a la ARM top-byte-ignore or SPARC ADI).
> Hmm. Can we be creative and skip populating it with zeros? The CPU
should only ever touch a page if we miss an ENDBR on it, so, in normal
operation, we don’t need anything to be there. We could try to prevent
anyone from *reading* it outside of ENDBR tracking if we want to avoid
people accidentally wasting lots of memory by forcing it to be fully
populated when the read it.
Won't reads on a big, contiguous private mapping get the huge zero page
anyway?
> The one downside is this forces it to be per-mm, but that seems like
> a generally reasonable model anyway.
Yeah, practically, you could only make it shared if you shared the
layout of all code in the address space. I'm sure the big database(s)
do that cross-process, but I bet nobody else does. User ASLR
practically guarantees that nobody can do this.
> This also gives us an excellent opportunity to make it read-only as
> seen from userspace to prevent exploits from just poking it full of
> ones before redirecting execution.
That would be fun.
^ permalink raw reply
* Re: [PATCH v3 17/20] dt: bindings: fix some broken links from txt->yaml conversion
From: Mark Brown @ 2019-06-07 18:57 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Hugues Fruchet, Rob Herring, Mark Rutland,
Maxime Coquelin, Alexandre Torgue, Olivier Moysan,
Arnaud Pouliquen, Liam Girdwood, devicetree, linux-stm32,
linux-arm-kernel, alsa-devel
In-Reply-To: <effeafed3023d8dc5f2440c8d5637ea31c02a533.1559933665.git.mchehab+samsung@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 263 bytes --]
On Fri, Jun 07, 2019 at 03:54:33PM -0300, Mauro Carvalho Chehab wrote:
> Some new files got converted to yaml, but references weren't
> updated accordingly.
These should probably just be sent as normal patches rather than tied in
with the rest of this series...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox