* Re: [PATCH 05/11] ftrace: create memcache for hash entries
From: Changbin Du @ 2019-08-26 22:35 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-arch, linux-s390, linux-parisc, Jonathan Corbet, linux-sh,
x86, linux-doc, linux-kernel, Steven Rostedt, linux-mips,
Ingo Molnar, Jessica Yu, sparclinux, linux-kbuild,
Thomas Gleixner, linuxppc-dev, linux-riscv, linux-arm-kernel,
Changbin Du
In-Reply-To: <20190826074437.GM2369@hirez.programming.kicks-ass.net>
On Mon, Aug 26, 2019 at 09:44:37AM +0200, Peter Zijlstra wrote:
> On Sun, Aug 25, 2019 at 09:23:24PM +0800, Changbin Du wrote:
> > When CONFIG_FTRACE_FUNC_PROTOTYPE is enabled, thousands of
> > ftrace_func_entry instances are created. So create a dedicated
> > memcache to enhance performance.
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> > kernel/trace/ftrace.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index a314f0768b2c..cfcb8dad93ea 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -94,6 +94,8 @@ struct ftrace_ops *function_trace_op __read_mostly = &ftrace_list_end;
> > /* What to set function_trace_op to */
> > static struct ftrace_ops *set_function_trace_op;
> >
> > +struct kmem_cache *hash_entry_cache;
> > +
> > static bool ftrace_pids_enabled(struct ftrace_ops *ops)
> > {
> > struct trace_array *tr;
> > @@ -1169,7 +1171,7 @@ static int add_hash_entry(struct ftrace_hash *hash, unsigned long ip,
> > {
> > struct ftrace_func_entry *entry;
> >
> > - entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> > + entry = kmem_cache_alloc(hash_entry_cache, GFP_KERNEL);
> > if (!entry)
> > return -ENOMEM;
> >
> > @@ -6153,6 +6155,15 @@ void __init ftrace_init(void)
> > if (ret)
> > goto failed;
> >
> > + hash_entry_cache = kmem_cache_create("ftrace-hash",
> > + sizeof(struct ftrace_func_entry),
> > + sizeof(struct ftrace_func_entry),
> > + 0, NULL);
> > + if (!hash_entry_cache) {
> > + pr_err("failed to create ftrace hash entry cache\n");
> > + goto failed;
> > + }
>
> Wait what; you already have then in the binary image, now you're
> allocating extra memory for each of them?
>
No, here we only allocate ftrace hash entries. The prototype data is not copied.
The entry->priv points to prototype data in binary.
> Did you look at what ORC does? Is the binary search really not fast
> enough?
For ftrace, binary search is not enough. Just like the hash tables
(ftrace_graph_notrace_hash, ftrace_graph_hash) we already have which is used to
filter traced functions.
--
Cheers,
Changbin Du
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/5] soc: amlogic: Add support for Everything-Else power domains controller
From: Martin Blumenstingl @ 2019-08-26 22:40 UTC (permalink / raw)
To: Neil Armstrong
Cc: ulf.hansson, linux-pm, khilman, linux-kernel, linux-amlogic,
linux-arm-kernel
In-Reply-To: <f6e7e4de-e1b7-f642-07cb-fa029ff2a883@baylibre.com>
Hi Neil,
On Mon, Aug 26, 2019 at 10:10 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> On 25/08/2019 23:10, Martin Blumenstingl wrote:
> > Hi Neil,
> >
> > thank you for this update
> > I haven't tried this on the 32-bit SoCs yet, but I am confident that I
> > can make it work by "just" adding the SoC specific bits!
> >
> > On Fri, Aug 23, 2019 at 11:06 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
> > [...]
> >> +/* AO Offsets */
> >> +
> >> +#define AO_RTI_GEN_PWR_SLEEP0 (0x3a << 2)
> >> +#define AO_RTI_GEN_PWR_ISO0 (0x3b << 2)
> >> +
> >> +/* HHI Offsets */
> >> +
> >> +#define HHI_MEM_PD_REG0 (0x40 << 2)
> >> +#define HHI_VPU_MEM_PD_REG0 (0x41 << 2)
> >> +#define HHI_VPU_MEM_PD_REG1 (0x42 << 2)
> >> +#define HHI_VPU_MEM_PD_REG3 (0x43 << 2)
> >> +#define HHI_VPU_MEM_PD_REG4 (0x44 << 2)
> >> +#define HHI_AUDIO_MEM_PD_REG0 (0x45 << 2)
> >> +#define HHI_NANOQ_MEM_PD_REG0 (0x46 << 2)
> >> +#define HHI_NANOQ_MEM_PD_REG1 (0x47 << 2)
> >> +#define HHI_VPU_MEM_PD_REG2 (0x4d << 2)
> > should we switch to the actual register offsets like we did in the
> > clock drivers?
>
> I find it simpler to refer to the numbers in the documentation...
OK, I have no strong preference here
for the 32-bit SoCs I will need to use the offsets based on the
"amlogic,meson8b-pmu", "syscon" [0], so these will be magic anyways
[...]
> >> +#define VPU_HHI_MEMPD(__reg) \
> >> + { __reg, BIT(8) }, \
> >> + { __reg, BIT(9) }, \
> >> + { __reg, BIT(10) }, \
> >> + { __reg, BIT(11) }, \
> >> + { __reg, BIT(12) }, \
> >> + { __reg, BIT(13) }, \
> >> + { __reg, BIT(14) }, \
> >> + { __reg, BIT(15) }
> > the Amlogic implementation from buildroot-openlinux-A113-201901 (the
> > latest one I have)
> > kernel/aml-4.9/drivers/amlogic/media/vout/hdmitx/hdmi_tx_20/hw/hdmi_tx_hw.c
> > uses:
> > hd_set_reg_bits(P_HHI_MEM_PD_REG0, 0, 8, 8)
> > that basically translates to: GENMASK(15, 8) (which means we could
> > drop this macro)
> >
> > the datasheet also states: 15~8 [...] HDMI memory PD (as a single
> > 8-bit wide register)
>
> Yep, but the actual code setting the VPU power domain is in u-boot :
>
> drivers/vpu/aml_vpu_power_init.c:
> 108 for (i = 8; i < 16; i++) {
> 109 vpu_hiu_setb(HHI_MEM_PD_REG0, 0, i, 1);
> 110 udelay(5);
> 111 }
>
> the linux code is like never used here, my preference goes to the u-boot code
> implementation.
I see, let's keep your implementation then
> >
> > [...]
> >> +static struct meson_ee_pwrc_domain_desc g12a_pwrc_domains[] = {
> >> + [PWRC_G12A_VPU_ID] = VPU_PD("VPU", &g12a_pwrc_vpu, g12a_pwrc_mem_vpu,
> >> + pwrc_ee_get_power, 11, 2),
> >> + [PWRC_G12A_ETH_ID] = MEM_PD("ETH", g12a_pwrc_mem_eth),
> >> +};
> >> +
> >> +static struct meson_ee_pwrc_domain_desc sm1_pwrc_domains[] = {
> >> + [PWRC_SM1_VPU_ID] = VPU_PD("VPU", &sm1_pwrc_vpu, sm1_pwrc_mem_vpu,
> >> + pwrc_ee_get_power, 11, 2),
> >> + [PWRC_SM1_NNA_ID] = TOP_PD("NNA", &sm1_pwrc_nna, sm1_pwrc_mem_nna,
> >> + pwrc_ee_get_power),
> >> + [PWRC_SM1_USB_ID] = TOP_PD("USB", &sm1_pwrc_usb, sm1_pwrc_mem_usb,
> >> + pwrc_ee_get_power),
> >> + [PWRC_SM1_PCIE_ID] = TOP_PD("PCI", &sm1_pwrc_pci, sm1_pwrc_mem_pcie,
> >> + pwrc_ee_get_power),
> >> + [PWRC_SM1_GE2D_ID] = TOP_PD("GE2D", &sm1_pwrc_ge2d, sm1_pwrc_mem_ge2d,
> >> + pwrc_ee_get_power),
> >> + [PWRC_SM1_AUDIO_ID] = MEM_PD("AUDIO", sm1_pwrc_mem_audio),
> >> + [PWRC_SM1_ETH_ID] = MEM_PD("ETH", g12a_pwrc_mem_eth),
> >> +};
> > my impression: I find this hard to read as it merges the TOP and
> > Memory PD domains from above, adding some seemingly random "11, 2" for
> > the VPU PD as well as pwrc_ee_get_power for some of the power domains
> > personally I like the way we describe clk_regmap because it's easy to
> > read (even though it adds a bit of boilerplate). I'm not sure if we
> > can make it work here, but this (not compile tested) is what I have in
> > mind (I chose two random power domains):
> > [PWRC_SM1_VPU_ID] = {
> > .name = "VPU",
> > .top_pd = SM1_EE_PD(8),
> > .mem_pds = {
> > VPU_MEMPD(HHI_VPU_MEM_PD_REG0),
> > VPU_MEMPD(HHI_VPU_MEM_PD_REG1),
> > VPU_MEMPD(HHI_VPU_MEM_PD_REG2),
> > VPU_MEMPD(HHI_VPU_MEM_PD_REG3),
> > { HHI_VPU_MEM_PD_REG4, GENMASK(1, 0) },
> > { HHI_VPU_MEM_PD_REG4, GENMASK(3, 2) },
> > { HHI_VPU_MEM_PD_REG4, GENMASK(5, 4) },
> > { HHI_VPU_MEM_PD_REG4, GENMASK(7, 6) },
> > { HHI_MEM_PD_REG0, GENMASK(15, 8) },
> > },
> > .num_mem_pds = 9,
> > .reset_names_count = 11,
> > .clk_names_count = 2,
> > },
> > [PWRC_SM1_ETH_ID] = {
> > .name = "ETH",
> > .mem_pds = { HHI_MEM_PD_REG0, GENMASK(3, 2) },
> > .num_mem_pds = 1,
> > },
> > ...
> >
> > I'd like to get Kevin's feedback on this
> > what you have right now is probably good enough for the initial
> > version of this driver. I'm bringing this discussion up because we
> > will add support for more SoCs to this driver (we migrate GX over to
> > it and I want to add 32-bit SoC support, which probably means at least
> > Meson8 - assuming they kept the power domains identical between
> > Meson8/8b/8m2).
>
> I find it more compact, but nothing is set in stone, you can refactor this as
> will when adding meson8 support, no problems here.
OK. if Kevin (or someone else) has feedback on this then I don't have
to waste time if it turns out that it's not a great idea ;)
> >
> > [...]
> >> +struct meson_ee_pwrc_domain {
> >> + struct generic_pm_domain base;
> >> + bool enabled;
> >> + struct meson_ee_pwrc *pwrc;
> >> + struct meson_ee_pwrc_domain_desc desc;
> >> + struct clk_bulk_data *clks;
> >> + int num_clks;
> >> + struct reset_control *rstc;
> >> + int num_rstc;
> >> +};
> >> +
> >> +struct meson_ee_pwrc {
> >> + struct regmap *regmap_ao;
> >> + struct regmap *regmap_hhi;
> >> + struct meson_ee_pwrc_domain *domains;
> >> + struct genpd_onecell_data xlate;
> >> +};
> > (my impressions on this: I was surprised to find more structs down
> > here, I expected them to be together with the other structs further
> > up)
>
> These are the "live" structures, opposed to the static structures defining the
> data and these are allocated and filled a probe time.
I see, thanks for the explanation
> I dislike changing static global data at runtime, this is why I clearly separated both.
I didn't mean to make them static - the thing that caught my eye was
that some of the structs are defined at the top of the driver while
these two are define much further down
I am used to having all struct definitions in one place
> >
> >> +static bool pwrc_ee_get_power(struct meson_ee_pwrc_domain *pwrc_domain)
> >> +{
> >> + u32 reg;
> >> +
> >> + regmap_read(pwrc_domain->pwrc->regmap_ao,
> >> + pwrc_domain->desc.top_pd->sleep_reg, ®);
> >> +
> >> + return (reg & pwrc_domain->desc.top_pd->sleep_mask);
> > should this also check for top_pd->iso_* as well as mem_pd->*?
> > if the top_pd part was optional we could even use the get_power
> > callback for *all* power domains in this driver (right now audio and
> > Ethernet don't have any get_power callback)
>
> We could, but how should we handle if one unexpected bit is set ? No idea...
hmm, I see
if we need it for other power domains then we can still implement it,
so it's good for now
[...]
> > bonus question: what about the video decoder power domains?
> > here is an example from vdec_1_start
> > (drivers/staging/media/meson/vdec/vdec_1.c):
> > /* Enable power for VDEC_1 */
> > regmap_update_bits(core->regmap_ao, AO_RTI_GEN_PWR_SLEEP0,
> > GEN_PWR_VDEC_1, 0);
> > usleep_range(10, 20);
> > [...]
> > /* enable VDEC Memories */
> > amvdec_write_dos(core, DOS_MEM_PD_VDEC, 0);
> > /* Remove VDEC1 Isolation */
> > regmap_write(core->regmap_ao, AO_RTI_GEN_PWR_ISO0, 0);
> >
> > (my point here is that it mixes video decoder "DOS" registers with
> > AO_RTI_GEN_PWR registers)
> > do we also want to add support for these "DOS" power domains to the
> > meson-ee-pwrc driver?
> > what about the AO_RTI_GEN_PWR part then - should we keep management
> > for the video decoder power domain bits in AO_RTI_GEN_PWR as part of
> > the video decoder driver?
>
> I left the decoders power domains aside so we can discuss it later on,
> we should expose multiple power domains, but the driver would need to
> be changed to support multiple power domains. But will loose the ability
> to enable/disable each domain at will unless it created a sub-device for
> each decoder and attaches the domain to to each device and use runtime pm.
>
> It's simpler to discuss it later on !
OK - does this mean you and/or Maxime have "discuss decoder power
domains" on your (long) TODO-list or do you want me to open this
discussion after this driver is merged?
Martin
[0] https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/amlogic/pmu.txt
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 01/11] ftrace: move recordmcount tools to scripts/ftrace
From: Steven Rostedt @ 2019-08-26 22:44 UTC (permalink / raw)
To: Changbin Du
Cc: linux-arch, Jonathan Corbet, linux-parisc, linux-doc, linux-sh,
linux-s390, John F . Reiser, x86, linux-kernel, linux-mips,
Ingo Molnar, Jessica Yu, sparclinux, linux-kbuild,
Thomas Gleixner, linuxppc-dev, linux-riscv, linux-arm-kernel,
Matt Helsley
In-Reply-To: <20190825132330.5015-2-changbin.du@gmail.com>
On Sun, 25 Aug 2019 21:23:20 +0800
Changbin Du <changbin.du@gmail.com> wrote:
> Move ftrace tools to its own directory. We will add another tool later.
>
> Cc: John F. Reiser <jreiser@BitWagon.com>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
> scripts/.gitignore | 1 -
> scripts/Makefile | 2 +-
> scripts/Makefile.build | 10 +++++-----
> scripts/ftrace/.gitignore | 4 ++++
> scripts/ftrace/Makefile | 4 ++++
> scripts/{ => ftrace}/recordmcount.c | 0
> scripts/{ => ftrace}/recordmcount.h | 0
> scripts/{ => ftrace}/recordmcount.pl | 0
> 8 files changed, 14 insertions(+), 7 deletions(-)
> create mode 100644 scripts/ftrace/.gitignore
> create mode 100644 scripts/ftrace/Makefile
> rename scripts/{ => ftrace}/recordmcount.c (100%)
> rename scripts/{ => ftrace}/recordmcount.h (100%)
> rename scripts/{ => ftrace}/recordmcount.pl (100%)
> mode change 100755 => 100644
Note, we are in the process of merging recordmcount with objtool. It
would be better to continue from that work.
http://lkml.kernel.org/r/2767f55f4a5fbf30ba0635aed7a9c5ee92ac07dd.1563992889.git.mhelsley@vmware.com
-- Steve
>
> diff --git a/scripts/.gitignore b/scripts/.gitignore
> index 17f8cef88fa8..1b5b5d595d80 100644
> --- a/scripts/.gitignore
> +++ b/scripts/.gitignore
> @@ -6,7 +6,6 @@ conmakehash
> kallsyms
> pnmtologo
> unifdef
> -recordmcount
> sortextable
> asn1_compiler
> extract-cert
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 16bcb8087899..d5992def49a8 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -14,7 +14,6 @@ hostprogs-$(CONFIG_BUILD_BIN2C) += bin2c
> hostprogs-$(CONFIG_KALLSYMS) += kallsyms
> hostprogs-$(CONFIG_LOGO) += pnmtologo
> hostprogs-$(CONFIG_VT) += conmakehash
> -hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
> hostprogs-$(CONFIG_BUILDTIME_EXTABLE_SORT) += sortextable
> hostprogs-$(CONFIG_ASN1) += asn1_compiler
> hostprogs-$(CONFIG_MODULE_SIG) += sign-file
> @@ -34,6 +33,7 @@ hostprogs-y += unifdef
> subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
> subdir-$(CONFIG_MODVERSIONS) += genksyms
> subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> +subdir-$(CONFIG_FTRACE) += ftrace
>
> # Let clean descend into subdirs
> subdir- += basic dtc gdb kconfig mod package
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 2f66ed388d1c..67558983c518 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -188,18 +188,18 @@ endif
> # files, including recordmcount.
> sub_cmd_record_mcount = \
> if [ $(@) != "scripts/mod/empty.o" ]; then \
> - $(objtree)/scripts/recordmcount $(RECORDMCOUNT_FLAGS) "$(@)"; \
> + $(objtree)/scripts/ftrace/recordmcount $(RECORDMCOUNT_FLAGS) "$(@)"; \
> fi;
> -recordmcount_source := $(srctree)/scripts/recordmcount.c \
> - $(srctree)/scripts/recordmcount.h
> +recordmcount_source := $(srctree)/scripts/ftrace/recordmcount.c \
> + $(srctree)/scripts/ftrace/recordmcount.h
> else
> -sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
> +sub_cmd_record_mcount = perl $(srctree)/scripts/ftrace/recordmcount.pl "$(ARCH)" \
> "$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
> "$(if $(CONFIG_64BIT),64,32)" \
> "$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS)" \
> "$(LD) $(KBUILD_LDFLAGS)" "$(NM)" "$(RM)" "$(MV)" \
> "$(if $(part-of-module),1,0)" "$(@)";
> -recordmcount_source := $(srctree)/scripts/recordmcount.pl
> +recordmcount_source := $(srctree)/scripts/ftrace/recordmcount.pl
> endif # BUILD_C_RECORDMCOUNT
> cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)), \
> $(sub_cmd_record_mcount))
> diff --git a/scripts/ftrace/.gitignore b/scripts/ftrace/.gitignore
> new file mode 100644
> index 000000000000..54d582c8faad
> --- /dev/null
> +++ b/scripts/ftrace/.gitignore
> @@ -0,0 +1,4 @@
> +#
> +# Generated files
> +#
> +recordmcount
> diff --git a/scripts/ftrace/Makefile b/scripts/ftrace/Makefile
> new file mode 100644
> index 000000000000..6797e51473e5
> --- /dev/null
> +++ b/scripts/ftrace/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
> +always := $(hostprogs-y)
> diff --git a/scripts/recordmcount.c b/scripts/ftrace/recordmcount.c
> similarity index 100%
> rename from scripts/recordmcount.c
> rename to scripts/ftrace/recordmcount.c
> diff --git a/scripts/recordmcount.h b/scripts/ftrace/recordmcount.h
> similarity index 100%
> rename from scripts/recordmcount.h
> rename to scripts/ftrace/recordmcount.h
> diff --git a/scripts/recordmcount.pl b/scripts/ftrace/recordmcount.pl
> old mode 100755
> new mode 100644
> similarity index 100%
> rename from scripts/recordmcount.pl
> rename to scripts/ftrace/recordmcount.pl
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/8] coresight: etm4x: Add missing API to set EL match on address filters
From: Mathieu Poirier @ 2019-08-26 22:59 UTC (permalink / raw)
To: Mike Leach; +Cc: coresight, linux-arm-kernel, suzuki.poulose
In-Reply-To: <20190819205720.24457-4-mike.leach@linaro.org>
On Mon, Aug 19, 2019 at 09:57:15PM +0100, Mike Leach wrote:
> TRCACATRn registers have match bits for secure and non-secure exception
> levels which are not accessible by the sysfs API.
> This adds a new sysfs parameter to enable this - addr_exlevel_s_ns.
>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
> .../coresight/coresight-etm4x-sysfs.c | 39 +++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index fa1d6a938f6c..7eab5d7d0b62 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -1233,6 +1233,44 @@ static ssize_t addr_context_store(struct device *dev,
> }
> static DEVICE_ATTR_RW(addr_context);
>
> +static ssize_t addr_exlevel_s_ns_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + u8 idx;
> + unsigned long val;
> + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> + struct etmv4_config *config = &drvdata->config;
> +
> + spin_lock(&drvdata->spinlock);
> + idx = config->addr_idx;
> + val = BMVAL(config->addr_acc[idx], 14, 8);
> + spin_unlock(&drvdata->spinlock);
> + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
> +}
> +
> +static ssize_t addr_exlevel_s_ns_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + u8 idx;
> + unsigned long val;
> + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
> + struct etmv4_config *config = &drvdata->config;
> +
> + if (kstrtoul(buf, 16, &val))
> + return -EINVAL;
> +
> + spin_lock(&drvdata->spinlock);
> + idx = config->addr_idx;
> + /* clear Exlevel_ns & Exlevel_s bits[14:12, 11:8] */
> + config->addr_acc[idx] &= ~(GENMASK(14, 8));
> + config->addr_acc[idx] |= (val << 8);
> + spin_unlock(&drvdata->spinlock);
> + return size;
> +}
> +static DEVICE_ATTR_RW(addr_exlevel_s_ns);
> +
> static ssize_t seq_idx_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -2038,6 +2076,7 @@ static struct attribute *coresight_etmv4_attrs[] = {
> &dev_attr_addr_stop.attr,
> &dev_attr_addr_ctxtype.attr,
> &dev_attr_addr_context.attr,
> + &dev_attr_addr_exlevel_s_ns.attr,
> &dev_attr_seq_idx.attr,
> &dev_attr_seq_state.attr,
> &dev_attr_seq_event.attr,
I'm ok with this patch but the new entry needs to be documented in [1]. But
before moving forward with that I'm wondering if this is the way to go. Would
it be better to consolidate type, ctxtype, context and exlevel_s_ns in a single
entry, say addr_acc_type? We'd shed a fair amount of code and make it more
simple for users to configure.
[1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH -next] mmc: aspeed: Fix return value check in aspeed_sdc_probe()
From: Andrew Jeffery @ 2019-08-26 23:37 UTC (permalink / raw)
To: Dan Carpenter, Wei Yongjun
Cc: Ulf Hansson, linux-aspeed, kernel-janitors, linux-mmc,
Adrian Hunter, Joel Stanley, linux-arm-kernel
In-Reply-To: <20190826130343.GA23584@kadam>
On Mon, 26 Aug 2019, at 22:34, Dan Carpenter wrote:
> > Fixes: 09eed7fffd33 ("mmc: Add support for the ASPEED SD controller")
> ^^^^
> When we're adding new files, could we use the prefix for the new driver
> instead of just the subsystem? "mmc: aspeed: Add new driver"?
> Otherwise it's tricky to know what people want for the driver.
I don't have any issue with the request, but I don't understand this last
bit. What do you mean by "it's tricky to know what people want for the
driver"?
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH -next] mmc: aspeed: Fix return value check in aspeed_sdc_probe()
From: Andrew Jeffery @ 2019-08-26 23:40 UTC (permalink / raw)
To: Wei Yongjun, Adrian Hunter, Ulf Hansson, Joel Stanley
Cc: kernel-janitors, linux-mmc, linux-arm-kernel, linux-aspeed
In-Reply-To: <20190826120013.183435-1-weiyongjun1@huawei.com>
On Mon, 26 Aug 2019, at 21:27, Wei Yongjun wrote:
> In case of error, the function of_platform_device_create() returns
> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
>
> Fixes: 09eed7fffd33 ("mmc: Add support for the ASPEED SD controller")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> drivers/mmc/host/sdhci-of-aspeed.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c
> b/drivers/mmc/host/sdhci-of-aspeed.c
> index 8bb095ca2fa9..d5acb5afc50f 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -261,9 +261,9 @@ static int aspeed_sdc_probe(struct platform_device
> *pdev)
> struct platform_device *cpdev;
>
> cpdev = of_platform_device_create(child, NULL, &pdev->dev);
> - if (IS_ERR(cpdev)) {
> + if (!cpdev) {
> of_node_put(child);
> - ret = PTR_ERR(cpdev);
> + ret = -ENODEV;
> goto err_clk;
> }
> }
I ... have no idea why I wrote it that way. I must have just assumed it returned
an ERR_PTR(). Thanks for finding/fixing that.
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
From: Andrew Jeffery @ 2019-08-26 23:57 UTC (permalink / raw)
To: Ivan Mikhaylov, Guenter Roeck, Wim Van Sebroeck
Cc: Mark Rutland, devicetree, linux-watchdog, linux-aspeed, openbmc,
Alexander Amelkin, linux-kernel, Rob Herring, Joel Stanley,
linux-arm-kernel
In-Reply-To: <20190826104636.19324-5-i.mikhaylov@yadro.com>
On Mon, 26 Aug 2019, at 20:17, Ivan Mikhaylov wrote:
> The option for the ast2400/2500 to get access to CS0 at runtime.
>
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
> Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> index c5077a1f5cb3..023a9b578df6 100644
> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> @@ -34,6 +34,13 @@ Optional properties:
> engine is responsible for this.
>
> - aspeed,alt-boot: If property is present then boot from alternate
> block.
> + At alternate side 'access_cs0' sysfs file
> provides:
Why are we talking about sysfs in the devicetree binding? This patch
doesn't seem right to me.
Also if we're not supporting the aspeed,alt-boot property we should
probably document it as deprecated rather than making it disappear,
unless you're going to fix the systems that are using it.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
From: Guenter Roeck @ 2019-08-27 0:08 UTC (permalink / raw)
To: Andrew Jeffery, Ivan Mikhaylov, Wim Van Sebroeck
Cc: Mark Rutland, devicetree, linux-watchdog, linux-aspeed, openbmc,
Alexander Amelkin, linux-kernel, Rob Herring, Joel Stanley,
linux-arm-kernel
In-Reply-To: <f33aa4b8-6d06-4226-8859-ce21a4b8728b@www.fastmail.com>
On 8/26/19 4:57 PM, Andrew Jeffery wrote:
>
>
> On Mon, 26 Aug 2019, at 20:17, Ivan Mikhaylov wrote:
>> The option for the ast2400/2500 to get access to CS0 at runtime.
>>
>> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
>> ---
>> Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> index c5077a1f5cb3..023a9b578df6 100644
>> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
>> @@ -34,6 +34,13 @@ Optional properties:
>> engine is responsible for this.
>>
>> - aspeed,alt-boot: If property is present then boot from alternate
>> block.
>> + At alternate side 'access_cs0' sysfs file
>> provides:
>
> Why are we talking about sysfs in the devicetree binding? This patch
> doesn't seem right to me.
>
Correct; this is the wrong document. The attribute also will need
to be better explained. "At alternate side" does not explain (at
least not at all clearly enough) that the attribute only exists
if the system has booted from the alternate flash / block.
> Also if we're not supporting the aspeed,alt-boot property we should
> probably document it as deprecated rather than making it disappear,
> unless you're going to fix the systems that are using it.
>
Sorry, you lost me here. Where is it made to disappear ?
Guenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 4/4] dt-bindings/watchdog: Add access_cs0 option for alt-boot
From: Andrew Jeffery @ 2019-08-27 0:11 UTC (permalink / raw)
To: Guenter Roeck, Ivan Mikhaylov, Wim Van Sebroeck
Cc: Mark Rutland, devicetree, linux-watchdog, linux-aspeed, openbmc,
Alexander Amelkin, linux-kernel, Rob Herring, Joel Stanley,
linux-arm-kernel
In-Reply-To: <d040fee1-d12d-9ac9-a72a-e76137a4579f@roeck-us.net>
On Tue, 27 Aug 2019, at 09:38, Guenter Roeck wrote:
> On 8/26/19 4:57 PM, Andrew Jeffery wrote:
> >
> >
> > On Mon, 26 Aug 2019, at 20:17, Ivan Mikhaylov wrote:
> >> The option for the ast2400/2500 to get access to CS0 at runtime.
> >>
> >> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> >> ---
> >> Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> index c5077a1f5cb3..023a9b578df6 100644
> >> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
> >> @@ -34,6 +34,13 @@ Optional properties:
> >> engine is responsible for this.
> >>
> >> - aspeed,alt-boot: If property is present then boot from alternate
> >> block.
> >> + At alternate side 'access_cs0' sysfs file
> >> provides:
> >
> > Why are we talking about sysfs in the devicetree binding? This patch
> > doesn't seem right to me.
> >
>
> Correct; this is the wrong document. The attribute also will need
> to be better explained. "At alternate side" does not explain (at
> least not at all clearly enough) that the attribute only exists
> if the system has booted from the alternate flash / block.
>
> > Also if we're not supporting the aspeed,alt-boot property we should
> > probably document it as deprecated rather than making it disappear,
> > unless you're going to fix the systems that are using it.
> >
> Sorry, you lost me here. Where is it made to disappear ?
Oh, hah, I read the bullet '-' as a diff marker. Maybe I should go back to
bed!
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 3/4] watchdog/aspeed: add support for dual boot
From: Guenter Roeck @ 2019-08-27 0:14 UTC (permalink / raw)
To: Ivan Mikhaylov, Wim Van Sebroeck
Cc: Mark Rutland, devicetree, linux-watchdog, linux-aspeed,
Andrew Jeffery, openbmc, Alexander Amelkin, linux-kernel,
Rob Herring, Joel Stanley, linux-arm-kernel
In-Reply-To: <20190826104636.19324-4-i.mikhaylov@yadro.com>
On 8/26/19 3:46 AM, Ivan Mikhaylov wrote:
> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
> to clear out boot code source and re-enable access to the primary SPI flash
> chip while booted via wdt2 from the alternate chip.
>
> AST2400 datasheet says:
> "In the 2nd flash booting mode, all the address mapping to CS0# would be
> re-directed to CS1#. And CS0# is not accessable under this mode. To access
> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
> register WDT30.bit[1]."
>
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
> drivers/watchdog/aspeed_wdt.c | 62 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index cc71861e033a..bbc42847c0e3 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -53,6 +53,8 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
> #define WDT_CTRL_ENABLE BIT(0)
> #define WDT_TIMEOUT_STATUS 0x10
> #define WDT_TIMEOUT_STATUS_BOOT_SECONDARY BIT(1)
> +#define WDT_CLEAR_TIMEOUT_STATUS 0x14
> +#define WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION BIT(0)
>
> /*
> * WDT_RESET_WIDTH controls the characteristics of the external pulse (if
> @@ -165,6 +167,57 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
> return 0;
> }
>
> +/* access_cs0 shows if cs0 is accessible, hence the reverted bit */
> +static ssize_t access_cs0_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
This and other multi-line declarations do not appear to be aligned
with '('.
> +{
> + struct aspeed_wdt *wdt = dev_get_drvdata(dev);
> + uint32_t status = readl(wdt->base + WDT_TIMEOUT_STATUS);
> +
> + return sprintf(buf, "%u\n",
> + !(status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY));
> +}
> +
> +static ssize_t access_cs0_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct aspeed_wdt *wdt = dev_get_drvdata(dev);
> + unsigned long val;
> +
> + if (kstrtoul(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val)
> + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> +
> + return size;
> +}
> +
> +/*
> + * At alternate side the 'access_cs0' sysfs node provides:
> + * ast2400: a way to get access to the primary SPI flash chip at CS0
> + * after booting from the alternate chip at CS1.
> + * ast2500: a way to restore the normal address mapping from
> + * (CS0->CS1, CS1->CS0) to (CS0->CS0, CS1->CS1).
> + *
> + * Clearing the boot code selection and timeout counter also resets to the
> + * initial state the chip select line mapping. When the SoC is in normal
> + * mapping state (i.e. booted from CS0), clearing those bits does nothing for
> + * both versions of the SoC. For alternate boot mode (booted from CS1 due to
> + * wdt2 expiration) the behavior differs as described above.
> + *
The above needs to be in the sysfs attribute documentation as well.
> + * This option can be used with wdt2 (watchdog1) only.
This implies a specific watchdog numbering which is not guaranteed.
Someone might implement a system with some external watchdog.
> + */
> +static DEVICE_ATTR_RW(access_cs0);
> +
> +static struct attribute *bswitch_attrs[] = {
> + &dev_attr_access_cs0.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(bswitch);
> +
> static const struct watchdog_ops aspeed_wdt_ops = {
> .start = aspeed_wdt_start,
> .stop = aspeed_wdt_stop,
> @@ -306,9 +359,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> }
>
> status = readl(wdt->base + WDT_TIMEOUT_STATUS);
> - if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
> + if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY) {
> wdt->wdd.bootstatus = WDIOF_CARDRESET;
>
> + if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
> + of_device_is_compatible(np, "aspeed,ast2500-wdt"))
> + wdt->wdd.groups = bswitch_groups;
Kind of odd that the attribute only exists if the system booted from the
second flash, but if that is what you want I won't object. Just make sure
that this is explained properly.
> + }
> +
> + dev_set_drvdata(dev, wdt);
> +
> return devm_watchdog_register_device(dev, &wdt->wdd);
> }
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: change *FLAGS_<basetarget>.o to take the path relative to $(obj)
From: kbuild test robot @ 2019-08-27 0:29 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-arm-kernel, x86, Michal Marek, linux-kbuild, Marc Zyngier,
Suzuki K Poulose, Russell King, linux-kernel, Masahiro Yamada,
Ingo Molnar, Borislav Petkov, kbuild-all, Andy Lutomirski,
H. Peter Anvin, James Morse, Thomas Gleixner, kvmarm,
Julien Thierry
In-Reply-To: <20190825172833.5708-1-yamada.masahiro@socionext.com>
[-- Attachment #1: Type: text/plain, Size: 3404 bytes --]
Hi Masahiro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc6 next-20190826]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-change-FLAGS_-basetarget-o-to-take-the-path-relative-to-obj/20190827-071627
config: i386-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_fbdev.c:105:16: warning: initialized field overwritten [-Woverride-init]
.fb_set_par = intel_fbdev_set_par,
^~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_fbdev.c:105:16: note: (near initialization for 'intelfb_ops.fb_set_par')
drivers/gpu/drm/i915/display/intel_fbdev.c:109:20: warning: initialized field overwritten [-Woverride-init]
.fb_pan_display = intel_fbdev_pan_display,
^~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_fbdev.c:109:20: note: (near initialization for 'intelfb_ops.fb_pan_display')
drivers/gpu/drm/i915/display/intel_fbdev.c:110:14: warning: initialized field overwritten [-Woverride-init]
.fb_blank = intel_fbdev_blank,
^~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_fbdev.c:110:14: note: (near initialization for 'intelfb_ops.fb_blank')
vim +105 drivers/gpu/drm/i915/display/intel_fbdev.c
d9a946b52350bb drivers/gpu/drm/i915/intel_fbdev.c Rodrigo Vivi 2015-05-28 101
79e539453b34e3 drivers/gpu/drm/i915/intel_fb.c Jesse Barnes 2008-11-07 102 static struct fb_ops intelfb_ops = {
79e539453b34e3 drivers/gpu/drm/i915/intel_fb.c Jesse Barnes 2008-11-07 103 .owner = THIS_MODULE,
a36384dd941b48 drivers/gpu/drm/i915/intel_fbdev.c Stefan Christ 2016-11-14 104 DRM_FB_HELPER_DEFAULT_OPS,
e991077ec67e08 drivers/gpu/drm/i915/intel_fbdev.c Daniel Vetter 2014-06-18 @105 .fb_set_par = intel_fbdev_set_par,
21cff14847421f drivers/gpu/drm/i915/intel_fbdev.c Archit Taneja 2015-07-31 106 .fb_fillrect = drm_fb_helper_cfb_fillrect,
21cff14847421f drivers/gpu/drm/i915/intel_fbdev.c Archit Taneja 2015-07-31 107 .fb_copyarea = drm_fb_helper_cfb_copyarea,
21cff14847421f drivers/gpu/drm/i915/intel_fbdev.c Archit Taneja 2015-07-31 108 .fb_imageblit = drm_fb_helper_cfb_imageblit,
d9a946b52350bb drivers/gpu/drm/i915/intel_fbdev.c Rodrigo Vivi 2015-05-28 109 .fb_pan_display = intel_fbdev_pan_display,
03e515f7f8949c drivers/gpu/drm/i915/intel_fbdev.c Rodrigo Vivi 2015-03-09 110 .fb_blank = intel_fbdev_blank,
785b93ef8c3097 drivers/gpu/drm/i915/intel_fb.c Dave Airlie 2009-08-28 111 };
785b93ef8c3097 drivers/gpu/drm/i915/intel_fb.c Dave Airlie 2009-08-28 112
:::::: The code at line 105 was first introduced by commit
:::::: e991077ec67e08bd345fcee4f810e59740359da5 drm/i915: Properly track domain of the fbcon fb
:::::: TO: Daniel Vetter <daniel.vetter@ffwll.ch>
:::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28074 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH -next] mmc: aspeed: Fix return value check in aspeed_sdc_probe()
From: Julia Lawall @ 2019-08-27 0:47 UTC (permalink / raw)
To: Andrew Jeffery
Cc: linux-arm-kernel, Ulf Hansson, linux-aspeed, kernel-janitors,
linux-mmc, Adrian Hunter, Wei Yongjun, Joel Stanley,
Dan Carpenter
In-Reply-To: <629128e7-cc91-412f-8946-668fac2eb3b9@www.fastmail.com>
On Tue, 27 Aug 2019, Andrew Jeffery wrote:
>
>
> On Mon, 26 Aug 2019, at 22:34, Dan Carpenter wrote:
> > > Fixes: 09eed7fffd33 ("mmc: Add support for the ASPEED SD controller")
> > ^^^^
> > When we're adding new files, could we use the prefix for the new driver
> > instead of just the subsystem? "mmc: aspeed: Add new driver"?
> > Otherwise it's tricky to know what people want for the driver.
>
> I don't have any issue with the request, but I don't understand this last
> bit. What do you mean by "it's tricky to know what people want for the
> driver"?
There is no obvious algorithm that tells how to go from a file name to an
appropriate subject line prefix.
julia
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/3] coresight: tmc-etr: Decouple buffer sync and barrier packet insertion
From: Leo Yan @ 2019-08-27 1:30 UTC (permalink / raw)
To: Mathieu Poirier
Cc: suzuki.poulose, alexander.shishkin, yabinc, linux-kernel,
mike.leach, linux-arm-kernel
In-Reply-To: <20190826194605.3791-3-mathieu.poirier@linaro.org>
On Mon, Aug 26, 2019 at 01:46:04PM -0600, Mathieu Poirier wrote:
> If less space is available in the perf ring buffer than the ETR buffer,
> barrier packets inserted in the trace stream by tmc_sync_etr_buf() are
> skipped over when the head of the buffer is moved forward, resulting in
> traces that can't be decoded.
>
> This patch decouples the process of syncing ETR buffers and the addition
> of barrier packets in order to perform the latter once the offset in the
> trace buffer has been properly computed.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
> .../hwtracing/coresight/coresight-tmc-etr.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 4f000a03152e..bae47272de98 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -946,10 +946,6 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
> WARN_ON(!etr_buf->ops || !etr_buf->ops->sync);
>
> etr_buf->ops->sync(etr_buf, rrp, rwp);
> -
> - /* Insert barrier packets at the beginning, if there was an overflow */
> - if (etr_buf->full)
> - tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
> }
>
> static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
> @@ -1086,6 +1082,13 @@ static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata)
> drvdata->sysfs_buf = NULL;
> } else {
> tmc_sync_etr_buf(drvdata);
> + /*
> + * Insert barrier packets at the beginning, if there was
> + * an overflow.
> + */
> + if (etr_buf->full)
> + tmc_etr_buf_insert_barrier_packet(etr_buf,
> + etr_buf->offset);
> }
> }
>
> @@ -1502,11 +1505,16 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
> CS_LOCK(drvdata->base);
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> + lost = etr_buf->full;
Comparing to the previous version, it drops '|' bitwise operator;
seems to me this is more neat :)
I think Yabin's testing is more convinced, so I skip to test.
FWIW, these three patches look good to me:
Reviewed-by: Leo Yan <leo.yan@linaro.org>
> size = etr_buf->len;
> if (!etr_perf->snapshot && size > handle->size) {
> size = handle->size;
> lost = true;
> }
> +
> + /* Insert barrier packets at the beginning, if there was an overflow */
> + if (lost)
> + tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
> tmc_etr_sync_perf_buffer(etr_perf, size);
>
> /*
> @@ -1517,8 +1525,6 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
> */
> if (etr_perf->snapshot)
> handle->head += size;
> -
> - lost |= etr_buf->full;
> out:
> /*
> * Don't set the TRUNCATED flag in snapshot mode because 1) the
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: change *FLAGS_<basetarget>.o to take the path relative to $(obj)
From: kbuild test robot @ 2019-08-27 1:36 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-arm-kernel, x86, Michal Marek, linux-kbuild, Marc Zyngier,
Suzuki K Poulose, Russell King, linux-kernel, Masahiro Yamada,
Ingo Molnar, Borislav Petkov, kbuild-all, Andy Lutomirski,
H. Peter Anvin, James Morse, Thomas Gleixner, kvmarm,
Julien Thierry
In-Reply-To: <20190825172833.5708-1-yamada.masahiro@socionext.com>
[-- Attachment #1: Type: text/plain, Size: 12184 bytes --]
Hi Masahiro,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc6 next-20190826]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-change-FLAGS_-basetarget-o-to-take-the-path-relative-to-obj/20190827-071627
config: ia64-allnoconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/ia64/kernel/efi.o: In function `find_memmap_space':
efi.c:(.text+0x2402): undefined reference to `__udivdi3'
arch/ia64/kernel/time.o: In function `ia64_init_itm':
time.c:(.text+0xa32): undefined reference to `__udivdi3'
time.c:(.text+0xae2): undefined reference to `__udivdi3'
time.c:(.text+0xb62): undefined reference to `__udivdi3'
time.c:(.text+0xd62): undefined reference to `__udivdi3'
arch/ia64/kernel/time.o:time.c:(.text+0xe12): more undefined references to `__udivdi3' follow
kernel/ptrace.o: In function `ptrace_request':
ptrace.c:(.text+0x3262): undefined reference to `__umoddi3'
kernel/sched/core.o: In function `to_ratio':
core.c:(.text+0x2c32): undefined reference to `__udivdi3'
kernel/sched/cputime.o: In function `cputime_adjust':
cputime.c:(.text+0xd72): undefined reference to `__udivdi3'
kernel/sched/fair.o: In function `__calc_delta':
fair.c:(.text+0x362): undefined reference to `__udivdi3'
kernel/time/timekeeping.o: In function `scale64_check_overflow':
timekeeping.c:(.text+0x42): undefined reference to `__umoddi3'
timekeeping.c:(.text+0x62): undefined reference to `__udivdi3'
timekeeping.c:(.text+0x1b2): undefined reference to `__udivdi3'
kernel/time/timekeeping.o: In function `timekeeping_advance':
timekeeping.c:(.text+0x1552): undefined reference to `__udivdi3'
kernel/time/timekeeping.o: In function `tk_setup_internals.constprop.6':
timekeeping.c:(.text+0x19b2): undefined reference to `__udivdi3'
kernel/time/timekeeping.o: In function `get_device_system_crosststamp':
timekeeping.c:(.text+0x3f52): undefined reference to `__umoddi3'
timekeeping.c:(.text+0x3f72): undefined reference to `__udivdi3'
timekeeping.c:(.text+0x3f92): undefined reference to `__udivdi3'
kernel/time/clocksource.o: In function `clocks_calc_mult_shift':
clocksource.c:(.text+0x4b2): undefined reference to `__udivdi3'
kernel/time/clocksource.o: In function `clocks_calc_max_nsecs':
clocksource.c:(.text+0xaa2): undefined reference to `__udivdi3'
kernel/time/clocksource.o: In function `__clocksource_update_freq_scale':
clocksource.c:(.text+0xb72): undefined reference to `__udivdi3'
kernel/time/clocksource.o:clocksource.c:(.text+0xb82): more undefined references to `__udivdi3' follow
mm/percpu.o: In function `pcpu_setup_first_chunk':
>> percpu.c:(.init.text+0xa02): undefined reference to `__moddi3'
>> percpu.c:(.init.text+0xae2): undefined reference to `__udivdi3'
percpu.c:(.init.text+0xb22): undefined reference to `__moddi3'
percpu.c:(.init.text+0xc32): undefined reference to `__udivdi3'
percpu.c:(.init.text+0xc72): undefined reference to `__moddi3'
percpu.c:(.init.text+0xd52): undefined reference to `__udivdi3'
percpu.c:(.init.text+0xd92): undefined reference to `__moddi3'
percpu.c:(.init.text+0xe72): undefined reference to `__udivdi3'
percpu.c:(.init.text+0xeb2): undefined reference to `__moddi3'
percpu.c:(.init.text+0xf92): undefined reference to `__udivdi3'
percpu.c:(.init.text+0xfd2): undefined reference to `__moddi3'
percpu.c:(.init.text+0x10b2): undefined reference to `__udivdi3'
percpu.c:(.init.text+0x1132): undefined reference to `__moddi3'
percpu.c:(.init.text+0x1242): undefined reference to `__udivdi3'
percpu.c:(.init.text+0x12c2): undefined reference to `__moddi3'
percpu.c:(.init.text+0x1672): undefined reference to `__udivdi3'
percpu.c:(.init.text+0x16e2): undefined reference to `__moddi3'
percpu.c:(.init.text+0x1812): undefined reference to `__udivdi3'
percpu.c:(.init.text+0x1882): undefined reference to `__moddi3'
percpu.c:(.init.text+0x1a72): undefined reference to `__udivdi3'
percpu.c:(.init.text+0x1ae2): undefined reference to `__moddi3'
percpu.c:(.init.text+0x1bc2): undefined reference to `__udivdi3'
percpu.c:(.init.text+0x1c32): undefined reference to `__moddi3'
mm/page_alloc.o: In function `setup_per_zone_lowmem_reserve':
page_alloc.c:(.text+0x572): undefined reference to `__udivdi3'
mm/page_alloc.o: In function `__setup_per_zone_wmarks':
page_alloc.c:(.text+0xb42): undefined reference to `__udivdi3'
mm/page_alloc.o: In function `pageset_set_high_and_batch':
page_alloc.c:(.text+0x15e2): undefined reference to `__udivdi3'
mm/page_alloc.o: In function `find_zone_movable_pfns_for_nodes':
page_alloc.c:(.init.text+0x9f2): undefined reference to `__udivdi3'
page_alloc.c:(.init.text+0xa72): undefined reference to `__udivdi3'
mm/page_alloc.o:page_alloc.c:(.init.text+0x2d82): more undefined references to `__udivdi3' follow
mm/dmapool.o: In function `dma_pool_create':
dmapool.c:(.text+0x3e2): undefined reference to `__umoddi3'
mm/mempolicy.o: In function `offset_il_node':
mempolicy.c:(.text+0x412): undefined reference to `__umoddi3'
mm/slub.o: In function `__kmem_cache_create':
slub.c:(.text+0x6ff2): undefined reference to `__udivdi3'
slub.c:(.text+0x7042): undefined reference to `__udivdi3'
slub.c:(.text+0x7302): undefined reference to `__udivdi3'
slub.c:(.text+0x7392): undefined reference to `__udivdi3'
slub.c:(.text+0x7732): undefined reference to `__udivdi3'
slub.c:(.text+0x7752): undefined reference to `__umoddi3'
slub.c:(.text+0x77b2): undefined reference to `__umoddi3'
slub.c:(.text+0x77d2): undefined reference to `__udivdi3'
slub.c:(.text+0x7932): undefined reference to `__umoddi3'
slub.c:(.text+0x7992): undefined reference to `__umoddi3'
slub.c:(.text+0x7a52): undefined reference to `__umoddi3'
slub.c:(.text+0x7ab2): undefined reference to `__umoddi3'
mm/quicklist.o: In function `quicklist_trim':
quicklist.c:(.text+0x142): undefined reference to `__udivdi3'
fs/super.o: In function `super_cache_scan':
super.c:(.text+0x1ca2): undefined reference to `__udivdi3'
super.c:(.text+0x1cc2): undefined reference to `__umoddi3'
super.c:(.text+0x1cf2): undefined reference to `__udivdi3'
super.c:(.text+0x1d42): undefined reference to `__udivdi3'
super.c:(.text+0x1dc2): undefined reference to `__udivdi3'
fs/inode.o: In function `timespec64_trunc':
inode.c:(.text+0x5172): undefined reference to `__moddi3'
fs/inode.o: In function `current_time':
inode.c:(.text+0x52b2): undefined reference to `__moddi3'
lib/bitmap.o: In function `bitmap_remap':
bitmap.c:(.text+0x24c2): undefined reference to `__umoddi3'
lib/bitmap.o: In function `bitmap_bitremap':
bitmap.c:(.text+0x2682): undefined reference to `__moddi3'
lib/bitmap.o: In function `bitmap_fold':
bitmap.c:(.text+0x2982): undefined reference to `__umoddi3'
lib/kfifo.o: In function `kfifo_copy_from_user.isra.1':
kfifo.c:(.text+0x232): undefined reference to `__udivdi3'
kfifo.c:(.text+0x312): undefined reference to `__udivdi3'
lib/kfifo.o: In function `kfifo_copy_to_user.isra.2':
kfifo.c:(.text+0x582): undefined reference to `__udivdi3'
lib/kfifo.o: In function `__kfifo_init':
kfifo.c:(.text+0x1302): undefined reference to `__udivdi3'
lib/kfifo.o: In function `__kfifo_from_user':
kfifo.c:(.text+0x1672): undefined reference to `__udivdi3'
lib/kfifo.o:kfifo.c:(.text+0x17a2): more undefined references to `__udivdi3' follow
lib/string_helpers.o: In function `string_get_size':
string_helpers.c:(.text+0x282): undefined reference to `__umoddi3'
lib/hexdump.o: In function `hex_dump_to_buffer':
hexdump.c:(.text+0x682): undefined reference to `__umoddi3'
hexdump.c:(.text+0x6a2): undefined reference to `__udivdi3'
lib/kstrtox.o: In function `_parse_integer':
kstrtox.c:(.text+0x2e2): undefined reference to `__udivdi3'
lib/math/lcm.o: In function `lcm':
lcm.c:(.text+0x62): undefined reference to `__udivdi3'
lib/math/lcm.o: In function `lcm_not_zero':
lcm.c:(.text+0x122): undefined reference to `__udivdi3'
lib/math/reciprocal_div.o: In function `reciprocal_value':
reciprocal_div.c:(.text+0xd2): undefined reference to `__udivdi3'
lib/math/reciprocal_div.o:reciprocal_div.c:(.text+0x1e2): more undefined references to `__udivdi3' follow
drivers/pci/pci.o: In function `pci_set_cacheline_size':
pci.c:(.text+0xb7e2): undefined reference to `__umoddi3'
drivers/pci/setup-bus.o: In function `pci_bus_distribute_available_resources':
setup-bus.c:(.text+0x1ec2): undefined reference to `__udivdi3'
setup-bus.c:(.text+0x1f42): undefined reference to `__udivdi3'
setup-bus.c:(.text+0x1fc2): undefined reference to `__udivdi3'
setup-bus.c:(.text+0x21a2): undefined reference to `__udivdi3'
setup-bus.c:(.text+0x2212): undefined reference to `__udivdi3'
drivers/pci/setup-bus.o:setup-bus.c:(.text+0x2282): more undefined references to `__udivdi3' follow
drivers/acpi/acpica/exfldio.o: In function `acpi_ex_insert_into_field':
exfldio.c:(.text+0x812): undefined reference to `__umoddi3'
drivers/acpi/acpica/exfldio.o: In function `acpi_ex_extract_from_field':
exfldio.c:(.text+0x1222): undefined reference to `__udivdi3'
exfldio.c:(.text+0x1332): undefined reference to `__udivdi3'
exfldio.c:(.text+0x1362): undefined reference to `__umoddi3'
drivers/acpi/acpica/tbutils.o: In function `acpi_tb_parse_root_table':
tbutils.c:(.init.text+0x462): undefined reference to `__udivdi3'
drivers/acpi/acpica/utmath.o: In function `acpi_ut_short_divide':
utmath.c:(.text+0x152): undefined reference to `__udivdi3'
utmath.c:(.text+0x192): undefined reference to `__umoddi3'
drivers/acpi/acpica/utmath.o: In function `acpi_ut_divide':
utmath.c:(.text+0x262): undefined reference to `__udivdi3'
utmath.c:(.text+0x2a2): undefined reference to `__umoddi3'
drivers/tty/tty_port.o: In function `tty_port_close_start.part.1':
tty_port.c:(.text+0x5a2): undefined reference to `__udivdi3'
drivers/char/random.o: In function `add_device_randomness':
random.c:(.text+0x39d2): undefined reference to `__umoddi3'
drivers/char/random.o: In function `randomize_page':
random.c:(.text+0x4f82): undefined reference to `__umoddi3'
drivers/base/swnode.o: In function `software_node_read_int_array':
swnode.c:(.text+0x12f2): undefined reference to `__udivdi3'
drivers/firmware/efi/memmap.o: In function `__efi_memmap_init':
>> memmap.c:(.init.text+0x112): undefined reference to `__udivdi3'
arch/ia64/hp/common/sba_iommu.o: In function `sba_init':
sba_iommu.c:(.init.text+0x982): undefined reference to `__udivdi3'
arch/ia64/sn/kernel/bte.o: In function `bte_copy':
bte.c:(.text+0x3b2): undefined reference to `__moddi3'
arch/ia64/sn/pci/tioca_provider.o: In function `tioca_bus_fixup':
tioca_provider.c:(.text+0x662): undefined reference to `__udivdi3'
tioca_provider.c:(.text+0x772): undefined reference to `__udivdi3'
tioca_provider.c:(.text+0xab2): undefined reference to `__udivdi3'
arch/ia64/sn/pci/tioca_provider.o: In function `tioca_dma_map':
tioca_provider.c:(.text+0x1392): undefined reference to `__umoddi3'
lib/nodemask.o: In function `node_random':
nodemask.c:(.text+0x102): undefined reference to `__umoddi3'
lib/vsprintf.o: In function `vsscanf':
vsprintf.c:(.text+0xac62): undefined reference to `__udivdi3'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6360 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 4/5] perf symbols: Use CAP_SYSLOG with kptr_restrict checks
From: Igor Lubashev @ 2019-08-27 1:39 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Suzuki Poulouse, Alexander Shishkin, Alexey Budankov,
Igor Lubashev, James Morris, Linux Kernel Mailing List,
Peter Zijlstra, Namhyung Kim, linux-arm-kernel
In-Reply-To: <1566869956-7154-1-git-send-email-ilubashe@akamai.com>
The kernel is using CAP_SYSLOG capability instead of uid==0 and euid==0
when checking kptr_restrict. Make perf do the same.
Also, the kernel is a more restrictive than "no restrictions" in case of
kptr_restrict==0, so add the same logic to perf.
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/291d2cda6ee75b4cd4c9ce717c177db18bf03a31.1565188228.git.ilubashe@akamai.com
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/symbol.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4efde7879474..035f2e75728c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,6 +4,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
+#include <linux/capability.h>
#include <linux/kernel.h>
#include <linux/mman.h>
#include <linux/time64.h>
@@ -15,8 +16,10 @@
#include <inttypes.h>
#include "annotate.h"
#include "build-id.h"
+#include "cap.h"
#include "util.h"
#include "debug.h"
+#include "event.h"
#include "machine.h"
#include "map.h"
#include "symbol.h"
@@ -2195,13 +2198,19 @@ static bool symbol__read_kptr_restrict(void)
char line[8];
if (fgets(line, sizeof(line), fp) != NULL)
- value = ((geteuid() != 0) || (getuid() != 0)) ?
- (atoi(line) != 0) :
- (atoi(line) == 2);
+ value = perf_cap__capable(CAP_SYSLOG) ?
+ (atoi(line) >= 2) :
+ (atoi(line) != 0);
fclose(fp);
}
+ /* Per kernel/kallsyms.c:
+ * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+ */
+ if (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))
+ value = true;
+
return value;
}
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/5] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
From: Igor Lubashev @ 2019-08-27 1:39 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Suzuki Poulouse, Alexander Shishkin, Alexey Budankov,
Igor Lubashev, James Morris, Linux Kernel Mailing List,
Peter Zijlstra, Namhyung Kim, linux-arm-kernel
In-Reply-To: <1566869956-7154-1-git-send-email-ilubashe@akamai.com>
The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
perf_event_paranoid check. Make perf do the same.
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/ad56df5452eeafb99dda9fc3d30f0f487aace503.1565188228.git.ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/arch/arm/util/cs-etm.c | 3 ++-
tools/perf/arch/arm64/util/arm-spe.c | 3 ++-
tools/perf/arch/x86/util/intel-bts.c | 3 ++-
tools/perf/arch/x86/util/intel-pt.c | 2 +-
tools/perf/util/evsel.c | 2 +-
5 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 5cb07e8cb296..b87a1ca2968f 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -18,6 +18,7 @@
#include "../../perf.h"
#include "../../util/auxtrace.h"
#include "../../util/cpumap.h"
+#include "../../util/event.h"
#include "../../util/evlist.h"
#include "../../util/evsel.h"
#include "../../util/pmu.h"
@@ -254,7 +255,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
struct evsel *evsel, *cs_etm_evsel = NULL;
struct perf_cpu_map *cpus = evlist->core.cpus;
- bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
+ bool privileged = perf_event_paranoid_check(-1);
int err = 0;
ptr->evlist = evlist;
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 00915b8fd05b..29275a0544cd 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -12,6 +12,7 @@
#include <time.h>
#include "../../util/cpumap.h"
+#include "../../util/event.h"
#include "../../util/evsel.h"
#include "../../util/evlist.h"
#include "../../util/session.h"
@@ -66,7 +67,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
container_of(itr, struct arm_spe_recording, itr);
struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
struct evsel *evsel, *arm_spe_evsel = NULL;
- bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+ bool privileged = perf_event_paranoid_check(-1);
struct evsel *tracking_evsel;
int err;
diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
index 7b23318ebd7b..56a76142e9fd 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -12,6 +12,7 @@
#include <linux/zalloc.h>
#include "../../util/cpumap.h"
+#include "../../util/event.h"
#include "../../util/evsel.h"
#include "../../util/evlist.h"
#include "../../util/session.h"
@@ -107,7 +108,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr,
struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu;
struct evsel *evsel, *intel_bts_evsel = NULL;
const struct perf_cpu_map *cpus = evlist->core.cpus;
- bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+ bool privileged = perf_event_paranoid_check(-1);
btsr->evlist = evlist;
btsr->snapshot_mode = opts->auxtrace_snapshot_mode;
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index a8e633aa278a..7abccc0b0dc0 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -578,7 +578,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
bool have_timing_info, need_immediate = false;
struct evsel *evsel, *intel_pt_evsel = NULL;
const struct perf_cpu_map *cpus = evlist->core.cpus;
- bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+ bool privileged = perf_event_paranoid_check(-1);
u64 tsc_bit;
int err;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0a33f7322ecc..0b3b5af33954 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -279,7 +279,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
static bool perf_event_can_profile_kernel(void)
{
- return geteuid() == 0 || perf_event_paranoid() == -1;
+ return perf_event_paranoid_check(-1);
}
struct evsel *perf_evsel__new_cycles(bool precise)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
From: Igor Lubashev @ 2019-08-27 1:39 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Suzuki Poulouse, Alexander Shishkin, Alexey Budankov,
Igor Lubashev, James Morris, Linux Kernel Mailing List,
Peter Zijlstra, Namhyung Kim, linux-arm-kernel
In-Reply-To: <1566869956-7154-1-git-send-email-ilubashe@akamai.com>
Perf was too restrictive about sysctl kernel.perf_event_paranoid. The
kernel only disallows profiling when perf_event_paranoid > 1. Make perf do
the same.
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
tools/perf/util/evsel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0b3b5af33954..bfe6ed2abcc2 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -279,7 +279,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
static bool perf_event_can_profile_kernel(void)
{
- return perf_event_paranoid_check(-1);
+ return perf_event_paranoid_check(1);
}
struct evsel *perf_evsel__new_cycles(bool precise)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/5] perf event: Check ref_reloc_sym before using it
From: Igor Lubashev @ 2019-08-27 1:39 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Suzuki Poulouse, Alexander Shishkin, Alexey Budankov,
Igor Lubashev, James Morris, Linux Kernel Mailing List,
Peter Zijlstra, Namhyung Kim, linux-arm-kernel
In-Reply-To: <1566869956-7154-1-git-send-email-ilubashe@akamai.com>
Check for ref_reloc_sym before using it instead of checking
symbol_conf.kptr_restrict and relying solely on that check.
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Reported-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
tools/perf/util/event.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index f440fdc3e953..b84f5f3c9651 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -913,11 +913,13 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
int err;
union perf_event *event;
- if (symbol_conf.kptr_restrict)
- return -1;
if (map == NULL)
return -1;
+ kmap = map__kmap(map);
+ if (!kmap->ref_reloc_sym)
+ return -1;
+
/*
* We should get this from /sys/kernel/sections/.text, but till that is
* available use this, and after it is use this as a fallback for older
@@ -940,7 +942,6 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
}
- kmap = map__kmap(map);
size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
"%s%s", machine->mmap_name, kmap->ref_reloc_sym->name) + 1;
size = PERF_ALIGN(size, sizeof(u64));
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 5/5] perf: warn that perf_event_paranoid can restrict kernel symbols
From: Igor Lubashev @ 2019-08-27 1:39 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Suzuki Poulouse, Alexander Shishkin, Alexey Budankov,
Igor Lubashev, James Morris, Linux Kernel Mailing List,
Peter Zijlstra, Namhyung Kim, linux-arm-kernel
In-Reply-To: <1566869956-7154-1-git-send-email-ilubashe@akamai.com>
Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
symbols.
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
tools/perf/builtin-record.c | 2 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/builtin-trace.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f71631f2bcb5..18505d92ff69 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2372,7 +2372,7 @@ int cmd_record(int argc, const char **argv)
if (symbol_conf.kptr_restrict && !perf_evlist__exclude_kernel(rec->evlist))
pr_warning(
"WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,\n"
-"check /proc/sys/kernel/kptr_restrict.\n\n"
+"check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
"Samples in kernel functions may not be resolved if a suitable vmlinux\n"
"file is not found in the buildid cache or in the vmlinux path.\n\n"
"Samples in kernel modules won't be resolved at all.\n\n"
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5970723cd55a..29e910fb2d9a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -770,7 +770,7 @@ static void perf_event__process_sample(struct perf_tool *tool,
if (!perf_evlist__exclude_kernel(top->session->evlist)) {
ui__warning(
"Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
-"Check /proc/sys/kernel/kptr_restrict.\n\n"
+"Check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
"Kernel%s samples will not be resolved.\n",
al.map && map__has_symbols(al.map) ?
" modules" : "");
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index bc44ed29e05a..9443b8e05810 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1381,7 +1381,7 @@ static char *trace__machine__resolve_kernel_addr(void *vmachine, unsigned long l
if (symbol_conf.kptr_restrict) {
pr_warning("Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
- "Check /proc/sys/kernel/kptr_restrict.\n\n"
+ "Check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
"Kernel samples will not be resolved.\n");
machine->kptr_restrict_warned = true;
return NULL;
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it
From: Igor Lubashev @ 2019-08-27 1:39 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Suzuki Poulouse, Alexander Shishkin, Alexey Budankov,
Igor Lubashev, James Morris, Linux Kernel Mailing List,
Peter Zijlstra, Namhyung Kim, linux-arm-kernel
This is a follow up series to the ensure perf treats perf_event_paranoid and
kptr_restrict in a way that is similar to the kernel's. That includes use of
capabilities instead of euid==0, when possible, as well as adjusting the logic
and fixing bugs.
Prior discussion: https://lkml.kernel.org/lkml/cover.1565188228.git.ilubashe@akamai.com
=== Testing notes ===
I have tested on x86 with perf binary installed according to
Documentation/admin-guide/perf-security.rst (cap_sys_admin, cap_sys_ptrace,
cap_syslog assigned to the perf executable).
I tested each permutation of:
* 7 commits:
1. HEAD of perf/core
2. patch 01 on top of perf/core
3. patches 01-02 on top of perf/core
4. patches 01-03 on top of perf/core
5. patches 01-04 on top of perf/core
6. patches 01-05 on top of perf/core
7. HEAD of perf/cap (with known bug fixed by patch 01 of this series)
* 2 build environments: with and without libcap-dev
* 3 kernel.kptr_restrict values: 0, 1, 2
* 4 kernel.perf_event_paranoid values: -1, 0, 1, 2
* 2 users: root and non-root
Total: 336 permutations
Each permutation consisted of:
perf test
perf record -e instructions -- sleep 1
All test runs were expected. Also, as expected, the following permutation (just
that permutation) resulted in segmentation failure:
commit: perf/cap
build: no libcap-dev
kernel.kptr_restrict: 0
kernel.perf_event_paranoid: 2
user: non-root
The perf/cap commit was included in the test to ensure that we can reproduce the
crash and hence test that the patch series fixes the crash, while retaining the
desired behavior of perf/cap.
=== Series Contents ===
01: perf event: Check ref_reloc_sym before using it
Fix the pre-existing cause of the crash above: use of ref_reloc_sym without
a check for NULL
02: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
Replace the use of euid==0 with a check for CAP_SYS_ADMIN whenever
perf_event_paranoid level is verified.
* This patch has been reviewed previously and is unchanged.
* I kept Acks and Sign-offs.
03: perf util: kernel profiling is disallowed only when perf_event_paranoid>1
Align perf logic regarding perf_event_paranoid to match kernel's.
This has been reported by Arnaldo.
04: perf symbols: Use CAP_SYSLOG with kptr_restrict checks
Replace the use of uid and euid with a check for CAP_SYSLOG when
kptr_restrict is verified (similar to kernel/kallsyms.c and lib/vsprintf.c).
Consult perf_event_paranoid when kptr_restrict==0 (see kernel/kallsyms.c).
* A previous version of this patch has been reviewed previously, but I
* modified it in a non-trivial way, so I removed Acks.
05: perf: warn perf_event_paranoid restrict kernel symbols
Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
symbols.
Igor Lubashev (5):
perf event: Check ref_reloc_sym before using it
perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
perf symbols: Use CAP_SYSLOG with kptr_restrict checks
perf: warn that perf_event_paranoid can restrict kernel symbols
tools/perf/arch/arm/util/cs-etm.c | 3 ++-
tools/perf/arch/arm64/util/arm-spe.c | 3 ++-
tools/perf/arch/x86/util/intel-bts.c | 3 ++-
tools/perf/arch/x86/util/intel-pt.c | 2 +-
tools/perf/builtin-record.c | 2 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/builtin-trace.c | 2 +-
tools/perf/util/event.c | 7 ++++---
tools/perf/util/evsel.c | 2 +-
tools/perf/util/symbol.c | 15 ++++++++++++---
10 files changed, 27 insertions(+), 14 deletions(-)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: CPUfreq fail on rk3399-firefly
From: Kever Yang @ 2019-08-27 1:54 UTC (permalink / raw)
To: Kevin Hilman, Heiko Stuebner
Cc: kernel-build-reports, linux-rockchip, linux-next,
闫孝军, 张晴, linux-arm-kernel
In-Reply-To: <7h4l23zwej.fsf@baylibre.com>
On 2019/8/27 上午1:09, Kevin Hilman wrote:
> Hi Kever,
>
> Kever Yang <kever.yang@rock-chips.com> writes:
>
>> Hi Kevin,
>>
>> I want to have a test with my board, I can get the Image and dtb
>> from the link for the job,
>>
>> but how can I get the randisk which is named initrd-SDbyy2.cpio.gz?
> The ramdisk images are here:
>
> https://storage.kernelci.org/images/rootfs/buildroot/kci-2019.02/arm64/base/
>
> in the kernelCI logs the ramdisk is slightly modified because the kernel
> modules have been inserted into the cpio archive.
>
> However, for the purposes of this test, you can just test with the
> unmodified rootfs.cpio.gz above.
I try with this ramdisk, and it hangs at fan53555 init, but not get into
cpufreq.
Any suggestion?
My boot log:
https://paste.ubuntu.com/p/WYZKPWp7sk/
Thanks,
- Kever
>
> Kevin
>
>
>> Thanks,
>>
>> - Kever
>>
>> On 2019/8/24 上午1:03, Kevin Hilman wrote:
>>> Kevin Hilman <khilman@baylibre.com> writes:
>>>
>>>> Kever Yang <kever.yang@rock-chips.com> writes:
>>>>
>>>>> Hi Kevin, Heiko,
>>>>>
>>>>> On 2019/8/22 上午2:59, Kevin Hilman wrote:
>>>>>> Hi Heiko,
>>>>>>
>>>>>> Heiko Stuebner <heiko@sntech.de> writes:
>>>>>>
>>>>>>> Am Dienstag, 13. August 2019, 19:35:31 CEST schrieb Kevin Hilman:
>>>>>>>> [ resent with correct addr for linux-rockchip list ]
>>>>>>>>
>>>>>>>> Mark Brown <broonie@kernel.org> writes:
>>>>>>>>
>>>>>>>>> On Thu, Jul 18, 2019 at 04:28:08AM -0700, kernelci.org bot wrote:
>>>>>>>>>
>>>>>>>>> Today's -next started failing to boot defconfig on rk3399-firefly:
>>>>>>>>>
>>>>>>>>>> arm64:
>>>>>>>>>> defconfig:
>>>>>>>>>> gcc-8:
>>>>>>>>>> rk3399-firefly: 1 failed lab
>>>>>>>>> It hits a BUG() trying to set up cpufreq:
>>>>>>>>>
>>>>>>>>> [ 87.381606] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 200000 KHz
>>>>>>>>> [ 87.393244] cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 408000 KHz
>>>>>>>>> [ 87.469777] cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 12000 KHz
>>>>>>>>> [ 87.488595] cpu cpu4: _generic_set_opp_clk_only: failed to set clock rate: -22
>>>>>>>>> [ 87.491881] cpufreq: __target_index: Failed to change cpu frequency: -22
>>>>>>>>> [ 87.495335] ------------[ cut here ]------------
>>>>>>>>> [ 87.496821] kernel BUG at drivers/cpufreq/cpufreq.c:1438!
>>>>>>>>> [ 87.498462] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>>>>>>>>>
>>>>>>>>> I'm struggling to see anything relevant in the diff from yesterday, the
>>>>>>>>> unlisted frequency warnings were there in the logs yesterday but no oops
>>>>>>>>> and I'm not seeing any changes in cpufreq, clk or anything relevant
>>>>>>>>> looking.
>>>>>>>>>
>>>>>>>>> Full bootlog and other info can be found here:
>>>>>>>>>
>>>>>>>>> https://kernelci.org/boot/id/5d302d8359b51498d049e983/
>>>>>>>> I confirm that disabling CPUfreq in the defconfig (CONFIG_CPU_FREQ=n)
>>>>>>>> makes the firefly board start working again.
>>>>>>>>
>>>>>>>> Note that the default defconfig enables the "performance" CPUfreq
>>>>>>>> governor as the default governor, so during kernel boot, it will always
>>>>>>>> switch to the max frequency.
>>>>>>>>
>>>>>>>> For fun, I set the default governor to "userspace" so the kernel
>>>>>>>> wouldn't make any OPP changes, and that leads to a slightly more
>>>>>>>> informative splat[1]
>>>>>>>>
>>>>>>>> There is still an OPP change happening because the detected OPP is not
>>>>>>>> one that's listed in the table, so it tries to change to a listed OPP
>>>>>>>> and fails in the bowels of clk_set_rate()
>>>>>>> Though I think that might only be a symptom as well.
>>>>>>> Both the PLL setting code as well as the actual cpu-clock implementation
>>>>>>> is unchanged since 2017 (and runs just fine on all boards in my farm).
>>>>>>>
>>>>>>> One source for these issues is often the regulator supplying the cpu
>>>>>>> going haywire - aka the voltage not matching the opp.
>>>>>>>
>>>>>>> As in this error-case it's CPU4 being set, this would mean it might
>>>>>>> be the big cluster supplied by the external syr825 (fan5355 clone)
>>>>>>> that might act up. In the Firefly-rk3399 case this is even stranger.
>>>>>>>
>>>>>>> There is a discrepancy between the "fcs,suspend-voltage-selector"
>>>>>>> between different bootloader versions (how the selection-pin is set up),
>>>>>>> so the kernel might actually write his requested voltage to the wrong
>>>>>>> register (not the one for actual voltage, but the second set used for
>>>>>>> the suspend voltage).
>>>>>>>
>>>>>>> Did you by chance swap bootloaders at some point in recent past?
>>>>>> No, haven't touched bootloader since I initially setup the board.
>>>>> The CPU voltage does not affect by bootloader for kernel should have its
>>>>> own opp-table,
>>>>>
>>>>> the bootloader may only affect the center/logic power supply.
>>>>>
>>>>>>> I'd assume [2] might actually be the same issue last year, though
>>>>>>> the CI-logs are not available anymore it seems.
>>>>>>>
>>>>>>> Could you try to set the vdd_cpu_b regulator to disabled, so that
>>>>>>> cpufreq for this cluster defers and see what happens?
>>>>>> Yes, this change[1] definitely makes things boot reliably again, so
>>>>>> there's defintiely something a bit unstable with this regulator, at
>>>>>> least on this firefly.
>>>>> Is it possible to target which patch introduce this bug? This board
>>>>> should have work correctly for a long time with upstream source code.
>>>> Unfortunately, it seems to be a regular, but intermittent failure, so
>>>> bisection is not producing anything reliable.
>>>>
>>>> You can see that both in mainline[1] and in linux-next[2] there are
>>>> periodic failures, but it's hard to see any patterns.
>>> Even worse, I (re)tested mainline for versions that were previously
>>> passing (v5.2, v5.3-rc5) and they are also failing now.
>>>
>>> They work again if I disable that regulator as suggested by Heiko.
>>>
>>> So this is increasingly pointing to failing hardware.
>>>
>>> Kevin
>>>
>>>
>>>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
From: Lubashev, Igor @ 2019-08-27 1:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Suzuki K Poulose, Peter Zijlstra, Alexey Budankov,
Linux Kernel Mailing List, Alexander Shishkin, Ingo Molnar,
Leo Yan, Namhyung Kim, Jiri Olsa, linux-arm-kernel
In-Reply-To: <20190820171342.GD3929@kernel.org>
On Tue, August 20, 2019 at 1:14 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > Arnaldo, once we decide what the right fix is, I am happy to post the
> update (options 1, 1+2) as a patch series.
>
> I think you should get the checks for ref_reloc_sym in place so as to make the
> code overall more robust, and also go on continuing to make the checks in
> tools/perf/ to match what is checked on the other side of the mirror, i.e. by
> the kernel, so from a quick read, please put first the robustness patches
> (check ref_reloc_sym) do your other suggestions and update the warnings,
> then refresh the two patches that still are not in my perf/core branch:
>
> [acme@quaco perf]$ git rebase perf/core
> First, rewinding head to replay your work on top of it...
> Applying: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> Applying: perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> [acme@quaco perf]$
>
> I've pushed out perf/cap, so you can go from there as it is rebased on my
> current perf/core.
>
> Then test all these cases: with/without libcap, with euid==0 and different
> than zero, with capabilities, etc, patch by patch so that we don't break
> bisection nor regress,
All done. I've posted the update as a new follow-up series: https://lkml.kernel.org/lkml/1566869956-7154-1-git-send-email-ilubashe@akamai.com/ rebased on your perf/core.
I've tested 336 permutations (see the new cover). In particular, I was able to reproduce the crash on perf/cap and confirm that no permutation can cause such crashes for any of the patches in the series.
> Thanks and keep up the good work!
>
> - Arnaldo
- Igor
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: swiotlb-xen cleanups
From: Stefano Stabellini @ 2019-08-27 2:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, x86, linux-kernel,
iommu, xen-devel, linux-arm-kernel
In-Reply-To: <20190816130013.31154-1-hch@lst.de>
On Fri, 16 Aug 2019, Christoph Hellwig wrote:
> Hi Xen maintainers and friends,
>
> please take a look at this series that cleans up the parts of swiotlb-xen
> that deal with non-coherent caches.
Hi Christoph,
I just wanted to let you know that your series is on my radar, but I
have been swamped the last few days. I hope to get to it by the end of
the week.
Cheers,
Stefano
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] kvm/arm/vgic: fix potential deadlock when ap_list is long
From: Guoheyi @ 2019-08-27 2:08 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, linux-kernel
Cc: Suzuki K Poulose, Marc Zyngier, James Morse, Zenghui Yu,
wanghaibin.wang, Julien Thierry
In-Reply-To: <1566837552-127854-1-git-send-email-guoheyi@huawei.com>
On 2019/8/27 0:39, Heyi Guo wrote:
> If ap_list is longer than 256, merge_final() in sort_list() will call
> comparison function with the same element just as below:
>
> do {
> /*
> * If the merge is highly unbalanced (e.g. the input is
> * already sorted), this loop may run many iterations.
> * Continue callbacks to the client even though no
> * element comparison is needed, so the client's cmp()
> * routine can invoke cond_resched() periodically.
> */
> if (unlikely(!++count))
> cmp(priv, b, b);
>
> This will definitely cause deadlock in vgic_irq_cmp() and the call trace
> is:
>
> [ 2667.130283] Call trace:
> [ 2667.130284] queued_spin_lock_slowpath+0x64/0x2a8
> [ 2667.130284] vgic_irq_cmp+0xfc/0x130
> [ 2667.130284] list_sort.part.0+0x1c0/0x268
> [ 2667.130285] list_sort+0x18/0x28
> [ 2667.130285] vgic_flush_lr_state+0x158/0x518
> [ 2667.130285] kvm_vgic_flush_hwstate+0x70/0x108
> [ 2667.130286] kvm_arch_vcpu_ioctl_run+0x114/0xa50
> [ 2667.130286] kvm_vcpu_ioctl+0x490/0x8c8
> [ 2667.130286] do_vfs_ioctl+0xc4/0x8c0
> [ 2667.130287] ksys_ioctl+0x8c/0xa0
> [ 2667.130287] __arm64_sys_ioctl+0x28/0x38
> [ 2667.130287] el0_svc_common+0x78/0x130
> [ 2667.130288] el0_svc_handler+0x38/0x78
> [ 2667.130288] el0_svc+0x8/0xc
>
> So return 0 immediately when a==b.
>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: James Morse <james.morse@arm.com>
> Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> virt/kvm/arm/vgic/vgic.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index 13d4b38..64ed0dc 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -254,6 +254,13 @@ static int vgic_irq_cmp(void *priv, struct list_head *a, struct list_head *b)
> bool penda, pendb;
> int ret;
>
> + /*
> + * list_sort may call this function with the same element when the list
> + * is farely long.
Sorry, s/farely/fairly/ :)
HG
> + */
> + if (unlikely(a == b))
> + return 0;
> +
> raw_spin_lock(&irqa->irq_lock);
> raw_spin_lock_nested(&irqb->irq_lock, SINGLE_DEPTH_NESTING);
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: CPUfreq fail on rk3399-firefly
From: Heiko Stuebner @ 2019-08-27 2:14 UTC (permalink / raw)
To: Kever Yang
Cc: kernel-build-reports, Kevin Hilman, linux-rockchip, linux-next,
闫孝军, 张晴, linux-arm-kernel
In-Reply-To: <852dd7d4-520b-7eb0-f307-c3fa9c0ebf2a@rock-chips.com>
Hi Kever,
Am Dienstag, 27. August 2019, 03:54:26 CEST schrieb Kever Yang:
> On 2019/8/27 上午1:09, Kevin Hilman wrote:
> > Kever Yang <kever.yang@rock-chips.com> writes:
> >> I want to have a test with my board, I can get the Image and dtb
> >> from the link for the job,
> >>
> >> but how can I get the randisk which is named initrd-SDbyy2.cpio.gz?
> > The ramdisk images are here:
> >
> > https://storage.kernelci.org/images/rootfs/buildroot/kci-2019.02/arm64/base/
> >
> > in the kernelCI logs the ramdisk is slightly modified because the kernel
> > modules have been inserted into the cpio archive.
> >
> > However, for the purposes of this test, you can just test with the
> > unmodified rootfs.cpio.gz above.
>
>
> I try with this ramdisk, and it hangs at fan53555 init, but not get into
> cpufreq.
>
> Any suggestion?
My guess would be the fcs,suspend-voltage-selector maybe?
I.e. old uboots somehow set the voltage gpio strangely, so you'd need
fcs,suspend-voltage-selector = <0>
while newer uboots I think do configure the gpio, needing a value of <1>;
So try to swap that number in the dts perhaps for a start?
Heiko
> My boot log:
>
> https://paste.ubuntu.com/p/WYZKPWp7sk/
>
> Thanks,
>
> - Kever
>
> >
> > Kevin
> >
> >
> >> Thanks,
> >>
> >> - Kever
> >>
> >> On 2019/8/24 上午1:03, Kevin Hilman wrote:
> >>> Kevin Hilman <khilman@baylibre.com> writes:
> >>>
> >>>> Kever Yang <kever.yang@rock-chips.com> writes:
> >>>>
> >>>>> Hi Kevin, Heiko,
> >>>>>
> >>>>> On 2019/8/22 上午2:59, Kevin Hilman wrote:
> >>>>>> Hi Heiko,
> >>>>>>
> >>>>>> Heiko Stuebner <heiko@sntech.de> writes:
> >>>>>>
> >>>>>>> Am Dienstag, 13. August 2019, 19:35:31 CEST schrieb Kevin Hilman:
> >>>>>>>> [ resent with correct addr for linux-rockchip list ]
> >>>>>>>>
> >>>>>>>> Mark Brown <broonie@kernel.org> writes:
> >>>>>>>>
> >>>>>>>>> On Thu, Jul 18, 2019 at 04:28:08AM -0700, kernelci.org bot wrote:
> >>>>>>>>>
> >>>>>>>>> Today's -next started failing to boot defconfig on rk3399-firefly:
> >>>>>>>>>
> >>>>>>>>>> arm64:
> >>>>>>>>>> defconfig:
> >>>>>>>>>> gcc-8:
> >>>>>>>>>> rk3399-firefly: 1 failed lab
> >>>>>>>>> It hits a BUG() trying to set up cpufreq:
> >>>>>>>>>
> >>>>>>>>> [ 87.381606] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 200000 KHz
> >>>>>>>>> [ 87.393244] cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 408000 KHz
> >>>>>>>>> [ 87.469777] cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 12000 KHz
> >>>>>>>>> [ 87.488595] cpu cpu4: _generic_set_opp_clk_only: failed to set clock rate: -22
> >>>>>>>>> [ 87.491881] cpufreq: __target_index: Failed to change cpu frequency: -22
> >>>>>>>>> [ 87.495335] ------------[ cut here ]------------
> >>>>>>>>> [ 87.496821] kernel BUG at drivers/cpufreq/cpufreq.c:1438!
> >>>>>>>>> [ 87.498462] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> >>>>>>>>>
> >>>>>>>>> I'm struggling to see anything relevant in the diff from yesterday, the
> >>>>>>>>> unlisted frequency warnings were there in the logs yesterday but no oops
> >>>>>>>>> and I'm not seeing any changes in cpufreq, clk or anything relevant
> >>>>>>>>> looking.
> >>>>>>>>>
> >>>>>>>>> Full bootlog and other info can be found here:
> >>>>>>>>>
> >>>>>>>>> https://kernelci.org/boot/id/5d302d8359b51498d049e983/
> >>>>>>>> I confirm that disabling CPUfreq in the defconfig (CONFIG_CPU_FREQ=n)
> >>>>>>>> makes the firefly board start working again.
> >>>>>>>>
> >>>>>>>> Note that the default defconfig enables the "performance" CPUfreq
> >>>>>>>> governor as the default governor, so during kernel boot, it will always
> >>>>>>>> switch to the max frequency.
> >>>>>>>>
> >>>>>>>> For fun, I set the default governor to "userspace" so the kernel
> >>>>>>>> wouldn't make any OPP changes, and that leads to a slightly more
> >>>>>>>> informative splat[1]
> >>>>>>>>
> >>>>>>>> There is still an OPP change happening because the detected OPP is not
> >>>>>>>> one that's listed in the table, so it tries to change to a listed OPP
> >>>>>>>> and fails in the bowels of clk_set_rate()
> >>>>>>> Though I think that might only be a symptom as well.
> >>>>>>> Both the PLL setting code as well as the actual cpu-clock implementation
> >>>>>>> is unchanged since 2017 (and runs just fine on all boards in my farm).
> >>>>>>>
> >>>>>>> One source for these issues is often the regulator supplying the cpu
> >>>>>>> going haywire - aka the voltage not matching the opp.
> >>>>>>>
> >>>>>>> As in this error-case it's CPU4 being set, this would mean it might
> >>>>>>> be the big cluster supplied by the external syr825 (fan5355 clone)
> >>>>>>> that might act up. In the Firefly-rk3399 case this is even stranger.
> >>>>>>>
> >>>>>>> There is a discrepancy between the "fcs,suspend-voltage-selector"
> >>>>>>> between different bootloader versions (how the selection-pin is set up),
> >>>>>>> so the kernel might actually write his requested voltage to the wrong
> >>>>>>> register (not the one for actual voltage, but the second set used for
> >>>>>>> the suspend voltage).
> >>>>>>>
> >>>>>>> Did you by chance swap bootloaders at some point in recent past?
> >>>>>> No, haven't touched bootloader since I initially setup the board.
> >>>>> The CPU voltage does not affect by bootloader for kernel should have its
> >>>>> own opp-table,
> >>>>>
> >>>>> the bootloader may only affect the center/logic power supply.
> >>>>>
> >>>>>>> I'd assume [2] might actually be the same issue last year, though
> >>>>>>> the CI-logs are not available anymore it seems.
> >>>>>>>
> >>>>>>> Could you try to set the vdd_cpu_b regulator to disabled, so that
> >>>>>>> cpufreq for this cluster defers and see what happens?
> >>>>>> Yes, this change[1] definitely makes things boot reliably again, so
> >>>>>> there's defintiely something a bit unstable with this regulator, at
> >>>>>> least on this firefly.
> >>>>> Is it possible to target which patch introduce this bug? This board
> >>>>> should have work correctly for a long time with upstream source code.
> >>>> Unfortunately, it seems to be a regular, but intermittent failure, so
> >>>> bisection is not producing anything reliable.
> >>>>
> >>>> You can see that both in mainline[1] and in linux-next[2] there are
> >>>> periodic failures, but it's hard to see any patterns.
> >>> Even worse, I (re)tested mainline for versions that were previously
> >>> passing (v5.2, v5.3-rc5) and they are also failing now.
> >>>
> >>> They work again if I disable that regulator as suggested by Heiko.
> >>>
> >>> So this is increasingly pointing to failing hardware.
> >>>
> >>> Kevin
> >>>
> >>>
> >>>
> >
> >
>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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