Re: [PATCH v2 3/3] arm64: implement KPROBES_ON_FTRACE
On Tue, 20 Aug 2019 03:54:20 + Jisheng Zhang wrote:
>
>
> KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
> eliminates the need for a trap, as well as the need to emulate or
> single-step instructions.
>
> This patch implements KPROBES_ON_FTRACE for arm64.
>
> Tested on berlin arm64 platform.
some performance numbers may be interesting.
HW: Berlin arm64 platform, cpufreq is forced to 800MHZ
SW: getppid syscall micro-benchmark, source code is put at the end of this
email.
A. Not probed.
B. Probed at __arm64_sys_getppid w/ non-operation probe functions, w/o
KPROBES_ON_FTRACE
C. Probed at __arm64_sys_getppid w/ non-operation probe functions, w/
KPROBES_ON_FTRACE
A: 1905 ns/call
B: 5833 ns/call
C: 2169 ns/call
The overhead of kprobes is 5833 - 1905 = 3928 ns/call
The overhead of kprobes w/ KPROBES_ON_FTRACE is 2169 - 1905 = 264 ns/call
As can be seen, KPROBES_ON_FTRACE significantly reduce the overhead of kprobes.
Thanks
<---8---
#include
#include
#include
#include
#include
int main (int argc, char *argv[])
{
struct timeval tv;
unsigned long count;
struct rusage usage;
for (count = 0; count < 1000; count++)
getppid();
getrusage(RUSAGE_SELF, &usage);
tv = usage.ru_stime;
tv.tv_sec += usage.ru_utime.tv_sec;
tv.tv_usec += usage.ru_utime.tv_usec;
fprintf(stderr, "getppid was called %u times: %d nsec per call\n",
count, (tv.tv_sec*1000*1000 + tv.tv_usec)/(count/1000));
return 0;
}
>
> ~ # mount -t debugfs debugfs /sys/kernel/debug/
> ~ # cd /sys/kernel/debug/
> /sys/kernel/debug # echo 'p _do_fork' > tracing/kprobe_events
>
> before the patch:
>
> /sys/kernel/debug # cat kprobes/list
> ff801009fe28 k _do_fork+0x0[DISABLED]
>
> after the patch:
>
> /sys/kernel/debug # cat kprobes/list
> ff801009ff54 k _do_fork+0x4[DISABLED][FTRACE]
>
> Signed-off-by: Jisheng Zhang
> ---
> .../debug/kprobes-on-ftrace/arch-support.txt | 2 +-
> arch/arm64/Kconfig| 1 +
> arch/arm64/kernel/probes/Makefile | 1 +
> arch/arm64/kernel/probes/ftrace.c | 60 +++
> 4 files changed, 63 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kernel/probes/ftrace.c
>
> diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> index 68f266944d5f..e8358a38981c 100644
> --- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> +++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
> @@ -9,7 +9,7 @@
> | alpha: | TODO |
> | arc: | TODO |
> | arm: | TODO |
> -| arm64: | TODO |
> +| arm64: | ok |
> | c6x: | TODO |
> |csky: | TODO |
> | h8300: | TODO |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 663392d1eae2..928700f15e23 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -167,6 +167,7 @@ config ARM64
> select HAVE_STACKPROTECTOR
> select HAVE_SYSCALL_TRACEPOINTS
> select HAVE_KPROBES
> + select HAVE_KPROBES_ON_FTRACE
> select HAVE_KRETPROBES
> select HAVE_GENERIC_VDSO
> select IOMMU_DMA if IOMMU_SUPPORT
> diff --git a/arch/arm64/kernel/probes/Makefile
> b/arch/arm64/kernel/probes/Makefile
> index 8e4be92e25b1..4020cfc66564 100644
> --- a/arch/arm64/kernel/probes/Makefile
> +++ b/arch/arm64/kernel/probes/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o
> \
>simulate-insn.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o \
>simulate-insn.o
> +obj-$(CONFIG_KPROBES_ON_FTRACE)+= ftrace.o
> diff --git a/arch/arm64/kernel/probes/ftrace.c
> b/arch/arm64/kernel/probes/ftrace.c
> new file mode 100644
> index ..52901570
> --- /dev/null
> +++ b/arch/arm64/kernel/probes/ftrace.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Dynamic Ftrace based Kprobes Optimization
> + *
> + * Copyright (C) Hitachi Ltd., 2012
> + * Copyright (C) 2019 Jisheng Zhang
> + * Synaptics Incorporated
> + */
> +
> +#include
> +
> +/* Ftrace callback handler for kprobes -- called under preepmt disabed */
> +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *ops, struct pt_regs *regs)
> +{
> + struct kprobe *p;
> + struct kprobe_ctlblk *kcb;
> +
> + /* Preempt is disabled by ftrace */
> + p = get_kprobe((kprobe_opcode_t *)ip);
> + if (unlikely(!p) || kprobe_disabled(p))
> + return;
> +
> + kcb = get_kprobe_ctlblk();
> + if (kprobe_running()) {
> + kprobes_inc_nmissed_count(p);
> +
Re: [PATCH v5 15/17] mfd: ioc3: Add driver for SGI IOC3 chip
On Tue, 20 Aug 2019 08:23:08 +0200 Alexandre Belloni wrote: > On 19/08/2019 18:31:38+0200, Thomas Bogendoerfer wrote: > > diff --git a/drivers/mfd/ioc3.c b/drivers/mfd/ioc3.c > > new file mode 100644 > > index ..5bcb3461a189 > > --- /dev/null > > +++ b/drivers/mfd/ioc3.c > > @@ -0,0 +1,586 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * SGI IOC3 multifunction device driver > > + * > > + * Copyright (C) 2018, 2019 Thomas Bogendoerfer > > + * > > + * Based on work by: > > + * Stanislaw Skowronek > > + * Joshua Kinard > > + * Brent Casavant - IOC4 master driver > > + * Pat Gefre - IOC3 serial port IRQ demuxer > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > I don't think this include is necessary. you are right. I'll move it to the patch where IP30 systemboard gets added. Thanks, Thomas. -- SUSE Linux GmbH GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 21284 (AG Nürnberg)
Re: [PATCH v2 3/3] arm64: implement KPROBES_ON_FTRACE
On Tue, 20 Aug 2019, Jisheng Zhang wrote: > KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it > eliminates the need for a trap, as well as the need to emulate or > single-step instructions. > > This patch implements KPROBES_ON_FTRACE for arm64. git grep 'This patch' Documentation/process/submitting-patches.rst Thanks, tglx
Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
On Tue, 20 Aug 2019, Jisheng Zhang wrote: > This is to make the x86 kprobe_ftrace_handler() more common so that > the code could be reused in future. While I agree with the change in general, I can't find anything which reuses that code. So the change log is pretty useless and I have no idea how this is related to the rest of the series. Thanks, tglx
Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
Hi Thomas, On Tue, 20 Aug 2019 10:53:58 +0200 (CEST) Thomas Gleixner wrote: > > > On Tue, 20 Aug 2019, Jisheng Zhang wrote: > > > This is to make the x86 kprobe_ftrace_handler() more common so that > > the code could be reused in future. > > While I agree with the change in general, I can't find anything which > reuses that code. So the change log is pretty useless and I have no idea > how this is related to the rest of the series. In v1, this code is moved from x86 to common kprobes.c [1] But I agree with Masami, consolidation could be done when arm64 kprobes on ftrace is stable. In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same as x86's, the only difference is comment, e.g /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */ while in arm64 /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */ W/ above, any suggestion about the suitable change log? Thanks [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2019-August/674417.html
Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
On Tue, 20 Aug 2019 09:02:59 + Jisheng Zhang wrote: > > > Hi Thomas, > > On Tue, 20 Aug 2019 10:53:58 +0200 (CEST) Thomas Gleixner wrote: > > > > > > > On Tue, 20 Aug 2019, Jisheng Zhang wrote: > > > > > This is to make the x86 kprobe_ftrace_handler() more common so that > > > the code could be reused in future. > > > > While I agree with the change in general, I can't find anything which > > reuses that code. So the change log is pretty useless and I have no idea > > how this is related to the rest of the series. Indeed, this isn't related to the rest of the series. So will update the change log and resend it alone. > > In v1, this code is moved from x86 to common kprobes.c [1] > But I agree with Masami, consolidation could be done when arm64 kprobes > on ftrace is stable. > > In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same > as x86's, the only difference is comment, e.g > > /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */ > > while in arm64 > > /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */ > > > W/ above, any suggestion about the suitable change log? > > Thanks >
Re: [PATCH v8 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
On Tue, Aug 13, 2019 at 01:52:20PM -0700, Yu-cheng Yu wrote:
> An ELF file's .note.gnu.property indicates features the executable file
> can support. For example, the property GNU_PROPERTY_X86_FEATURE_1_AND
> indicates the file supports GNU_PROPERTY_X86_FEATURE_1_IBT and/or
> GNU_PROPERTY_X86_FEATURE_1_SHSTK.
>
> With this patch, if an arch needs to setup features from ELF properties,
> it needs CONFIG_ARCH_USE_GNU_PROPERTY to be set, and specific
> arch_parse_property() and arch_setup_property().
>
> For example, for X86_64:
>
> int arch_setup_property(void *ehdr, void *phdr, struct file *f, bool inter)
> {
> int r;
> uint32_t property;
>
> r = get_gnu_property(ehdr, phdr, f, GNU_PROPERTY_X86_FEATURE_1_AND,
>&property);
> ...
> }
>
> This patch is derived from code provided by H.J. Lu .
>
> Signed-off-by: Yu-cheng Yu
[...]
For the hell of it, I tried implementing an alternate version [1] that
tries to integrate into the existing ELF loader more directly.
This may or may not be a better approach, but tries to solve some
issues such as not repeatedly reading and parsing the properties.
Cheers
---Dave
[1] [RFC PATCH 0/2] ELF: Alternate program property parser
https://lore.kernel.org/lkml/1566295063-7387-1-git-send-email-dave.mar...@arm.com/
Re: [PATCH v2 2/3] kprobes: adjust kprobe addr for KPROBES_ON_FTRACE
Jisheng Zhang wrote:
For KPROBES_ON_FTRACE case, we need to adjust the kprobe's addr
correspondingly.
Signed-off-by: Jisheng Zhang
---
kernel/kprobes.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 9873fc627d61..3fd2f68644da 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1484,15 +1484,19 @@ static inline int check_kprobe_rereg(struct kprobe *p)
int __weak arch_check_ftrace_location(struct kprobe *p)
{
- unsigned long ftrace_addr;
+ unsigned long ftrace_addr, addr = (unsigned long)p->addr;
- ftrace_addr = ftrace_location((unsigned long)p->addr);
+#ifdef CONFIG_KPROBES_ON_FTRACE
+ addr = ftrace_call_adjust(addr);
+#endif
Looking at the commit message for patch 3/3, it looks like you want the
probe to be placed on ftrace entry by default, and this patch seems to
be aimed at that.
If so, this is not the right approach. As I mentioned previously, you
would want to over-ride kprobe_lookup_name(). This ensures that the
address is changed only if the user provided a symbol, and not if the
user wanted to probe at a very specific address. See commit
24bd909e94776 ("powerpc/kprobes: Prefer ftrace when probing function
entry").
If this patch is for some other purpose, then it isn't clear from the
commit log. Please provide a better explanation.
- Naveen
Re: [PATCH v2 2/3] kprobes: adjust kprobe addr for KPROBES_ON_FTRACE
On Tue, 20 Aug 2019 15:45:24 +0530 "Naveen N. Rao" wrote:
>
>
> Jisheng Zhang wrote:
> > For KPROBES_ON_FTRACE case, we need to adjust the kprobe's addr
> > correspondingly.
> >
> > Signed-off-by: Jisheng Zhang
> > ---
> > kernel/kprobes.c | 10 +++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 9873fc627d61..3fd2f68644da 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1484,15 +1484,19 @@ static inline int check_kprobe_rereg(struct kprobe
> > *p)
> >
> > int __weak arch_check_ftrace_location(struct kprobe *p)
> > {
> > - unsigned long ftrace_addr;
> > + unsigned long ftrace_addr, addr = (unsigned long)p->addr;
> >
> > - ftrace_addr = ftrace_location((unsigned long)p->addr);
> > +#ifdef CONFIG_KPROBES_ON_FTRACE
> > + addr = ftrace_call_adjust(addr);
> > +#endif
>
> Looking at the commit message for patch 3/3, it looks like you want the
> probe to be placed on ftrace entry by default, and this patch seems to
> be aimed at that.
Yeah.
>
> If so, this is not the right approach. As I mentioned previously, you
> would want to over-ride kprobe_lookup_name(). This ensures that the
> address is changed only if the user provided a symbol, and not if the
> user wanted to probe at a very specific address. See commit
Great! Now I understand the reason.
> 24bd909e94776 ("powerpc/kprobes: Prefer ftrace when probing function
> entry").
Now, I got your meaning. You are right. I will update the patch in newer
version.
Thanks a lot!
>
> If this patch is for some other purpose, then it isn't clear from the
> commit log. Please provide a better explanation.
>
>
> - Naveen
>
Re: [PATCH V3 2/3] KVM/Hyper-V: Add new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH
Hi Thomas: Thanks for your review. Will fix your comment in the next version. On Mon, Aug 19, 2019 at 9:27 PM Thomas Gleixner wrote: > > On Mon, 19 Aug 2019, lantianyu1...@gmail.com wrote: > > > From: Tianyu Lan > > > > This patch adds > > Same git grep command as before > > > new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH and let > > baseball cap? Please do not use weird acronyms. This is text and there is > not limitation on characters. > > > user space to enable direct tlb flush function when only Hyper-V > > hypervsior capability is exposed to VM. > > Sorry, but I'm not understanding this sentence. > > > This patch also adds > > Once more > > > enable_direct_tlbflush callback in the struct kvm_x86_ops and > > platforms may use it to implement direct tlb flush support. > > Please tell in the changelog WHY you are doing things not what. The what is > obviously in the patch. > > So you want to explain what you are trying to achieve and why it is > useful. Then you can add a short note about what you are adding, but not at > the level of detail which is available from the diff itself. > > Thanks, > > tglx -- Best regards Tianyu Lan
Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
On Tue, Aug 20, 2019 at 09:02:59AM +, Jisheng Zhang wrote: > In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same > as x86's, the only difference is comment, e.g > > /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */ > > while in arm64 > > /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */ What's weird; I thought ARM has fixed sized instructions and they are all 4 bytes? So how does a single byte offset make sense for ARM?
Re: [PATCH] docs: mtd: Update spi nor reference driver
On 19/08/2019 05:39, Vignesh Raghavendra wrote: Hi, On 16/08/19 3:50 PM, John Garry wrote: On 06/08/2019 17:40, Schrempf Frieder wrote: [...] Hi, Could someone kindly advise on the following: Hi Vignesh, I am looking at ACPI support only for an mtd spi nor driver we're targeting for mainline support. If its a new driver, please add it under drivers/spi implementing SPI MEM framework. There are few drivers under drivers/spi that can be used as example. (Search for "spi_mem_ops" Ok, fine. I note that in doing this I would still be using the spi nor framework indirectly through the m25p80 driver. So for the host, I could use a proprietary HID in the DSDT for matching in the kernel driver. About the child spi flash devices, is the recommendation to just use PRP0001 HID and "jedec,spi-nor" compatible? I am not quite familiar with ACPI systems, but child flash device should use "jedec,spi-nor" as compatible. Right, so to use SPI MEM framework, it looks like I will have to use PRP0001 and "jedec,spi-nor" as compatible. My reluctance in using PRP0001 and compatible "jedec,spi-nor" is how other OS can understand this. All the best, John Regards Vignesh thanks, John
Re: [PATCH v2] Documentation/admin-guide: Embargoed hardware security issues
On Thu, Aug 15, 2019 at 11:25:05PM +0200, Greg Kroah-Hartman wrote: > +Contact > +--- > + > +The Linux kernel hardware security team is separate from the regular Linux > +kernel security team. > + > +The team only handles the coordination of embargoed hardware security > +issues. Reports of pure software security bugs in the Linux kernel are not > +handled by this team and the reporter will be guided to contact the regular > +Linux kernel security team (:ref:`Documentation/admin-guide/ > +`) instead. > + > +The team can be contacted by email at . This > +is a private list of security officers who will help you to coordinate an > +issue according to our documented process. > + > +The list is encrypted and email to the list can be sent by either PGP or > +S/MIME encrypted and must be signed with the reporter's PGP key or S/MIME > +certificate. The list's PGP key and S/MIME certificate are available from > +https://www.kernel.org/ This link needs to be filled in? > +Encrypted mailing-lists > +--- > + > +We use encrypted mailing-lists for communication. The operating principle > +of these lists is that email sent to the list is encrypted either with the > +list's PGP key or with the list's S/MIME certificate. The mailing-list > +software decrypts the email and re-encrypts it individually for each > +subscriber with the subscriber's PGP key or S/MIME certificate. Details > +about the mailing-list software and the setup which is used to ensure the > +security of the lists and protection of the data can be found here: > +https://www.kernel.org/ Ditto? -- Josh
Re: [PATCH v6 2/2] hwmon: pmbus: Add Inspur Power System power supply driver
On Mon, Aug 19, 2019 at 05:15:09PM +0800, John Wang wrote:
> Add the driver to monitor Inspur Power System power supplies
> with hwmon over pmbus.
>
> This driver adds sysfs attributes for additional power supply data,
> including vendor, model, part_number, serial number,
> firmware revision, hardware revision, and psu mode(active/standby).
>
> Signed-off-by: John Wang
> Reviewed-by: Joel Stanley
Applied to hwmon-next.
Thanks,
Guenter
> ---
> v6:
> - Name i2c device as ipsps1
> - Use of_match_ptr to save a few bytes if CONFIG_OF
> is not enabled :)
> v5:
> - Align sysfs attrs description in inspur-ipsps1.rst
> (Use tab instead of space to sperate names and values)
> v4:
> - Remove the additional tabs in the Makefile
> - Rebased on 5.3-rc4, not 5.2
> v3:
> - Sort kconfig/makefile entries alphabetically
> - Remove unnecessary initialization
> - Use ATTRIBUTE_GROUPS instead of expanding directly
> - Use memscan to avoid reimplementation
> v2:
> - Fix typos in commit message
> - Invert Christmas tree
> - Configure device with sysfs attrs, not debugfs entries
> - Fix errno in fw_version_read, ENODATA to EPROTO
> - Change the print format of fw-version
> - Use sysfs_streq instead of strcmp("xxx" "\n", "xxx")
> - Document sysfs attributes
> ---
> Documentation/hwmon/inspur-ipsps1.rst | 79 +
> drivers/hwmon/pmbus/Kconfig | 9 +
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/inspur-ipsps.c| 228 ++
> 4 files changed, 317 insertions(+)
> create mode 100644 Documentation/hwmon/inspur-ipsps1.rst
> create mode 100644 drivers/hwmon/pmbus/inspur-ipsps.c
>
> diff --git a/Documentation/hwmon/inspur-ipsps1.rst
> b/Documentation/hwmon/inspur-ipsps1.rst
> new file mode 100644
> index ..2b871ae3448f
> --- /dev/null
> +++ b/Documentation/hwmon/inspur-ipsps1.rst
> @@ -0,0 +1,79 @@
> +Kernel driver inspur-ipsps1
> +===
> +
> +Supported chips:
> +
> + * Inspur Power System power supply unit
> +
> +Author: John Wang
> +
> +Description
> +---
> +
> +This driver supports Inspur Power System power supplies. This driver
> +is a client to the core PMBus driver.
> +
> +Usage Notes
> +---
> +
> +This driver does not auto-detect devices. You will have to instantiate the
> +devices explicitly. Please see Documentation/i2c/instantiating-devices for
> +details.
> +
> +Sysfs entries
> +-
> +
> +The following attributes are supported:
> +
> +===
> ==
> +curr1_input Measured input current
> +curr1_label "iin"
> +curr1_maxMaximum current
> +curr1_max_alarm Current high alarm
> +curr2_input Measured output current in mA.
> +curr2_label "iout1"
> +curr2_crit Critical maximum current
> +curr2_crit_alarm Current critical high alarm
> +curr2_maxMaximum current
> +curr2_max_alarm Current high alarm
> +
> +fan1_alarm Fan 1 warning.
> +fan1_fault Fan 1 fault.
> +fan1_input Fan 1 speed in RPM.
> +
> +in1_alarmInput voltage under-voltage alarm.
> +in1_inputMeasured input voltage in mV.
> +in1_label"vin"
> +in2_inputMeasured output voltage in mV.
> +in2_label"vout1"
> +in2_lcritCritical minimum output voltage
> +in2_lcrit_alarm Output voltage critical low alarm
> +in2_max Maximum output voltage
> +in2_max_alarmOutput voltage high alarm
> +in2_min Minimum output voltage
> +in2_min_alarmOutput voltage low alarm
> +
> +power1_alarm Input fault or alarm.
> +power1_input Measured input power in uW.
> +power1_label "pin"
> +power1_max Input power limit
> +power2_max_alarm Output power high alarm
> +power2_max Output power limit
> +power2_input Measured output power in uW.
> +power2_label "pout"
> +
> +temp[1-3]_input Measured temperature
> +temp[1-2]_maxMaximum temperature
> +temp[1-3]_max_alarm Temperature high alarm
> +
> +vendor Manufacturer name
> +modelProduct model
> +part_number Product part number
> +serial_numberProduct serial number
> +fw_version Firmware version
> +hw_version Hardware version
> +mode Work mode. Can be set to active or
> + standby, when set to standby, PSU will
> + automatically switch between standby
> + and redundancy mode.
> +===
> ==
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index b6588483fae1..d6
Re: [PATCH v8 18/27] mm: Introduce do_mmap_locked()
On Mon, 2019-08-19 at 18:02 -0700, Sean Christopherson wrote:
> On Tue, Aug 13, 2019 at 01:52:16PM -0700, Yu-cheng Yu wrote:
> > There are a few places that need do_mmap() with mm->mmap_sem held.
> > Create an in-line function for that.
> >
> > Signed-off-by: Yu-cheng Yu
> > ---
> > include/linux/mm.h | 18 ++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index bc58585014c9..275c385f53c6 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2394,6 +2394,24 @@ static inline void mm_populate(unsigned long addr,
> > unsigned long len)
> > static inline void mm_populate(unsigned long addr, unsigned long len) {}
> > #endif
> >
> > +static inline unsigned long do_mmap_locked(struct file *file,
> > + unsigned long addr, unsigned long len, unsigned long prot,
> > + unsigned long flags, vm_flags_t vm_flags, struct list_head *uf)
> > +{
> > + struct mm_struct *mm = current->mm;
> > + unsigned long populate;
> > +
> > + down_write(&mm->mmap_sem);
> > + addr = do_mmap(file, addr, len, prot, flags, vm_flags, 0,
> > + &populate, uf);
> > + up_write(&mm->mmap_sem);
> > +
> > + if (populate)
> > + mm_populate(addr, populate);
> > +
> > + return addr;
> > +}
>
> Any reason not to put this in cet.c, as suggested by PeterZ? All of the
> calls from CET have identical params except for @len, e.g. you can add
> 'static unsigned long cet_mmap(unsigned long len)' and bury most of the
> copy-paste code in there.
>
> https://lkml.kernel.org/r/20190607074707.gd3...@hirez.programming.kicks-ass.ne
> t
Yes, I will do that. I thought this would be useful in other places, but
currently only in mpx.c.
Yu-cheng
Re: [PATCH v2 2/6] arm64: Introduce config for S32
Hello Shawn, On 8/19/2019 11:15 AM, Shawn Guo wrote: > On Fri, Aug 09, 2019 at 11:29:10AM +, Stefan-gabriel Mirea wrote: >> +config ARCH_S32 >> + bool "Freescale S32 SoC Family" > > So you still want to use 'Freescale' than 'NXP' here? > >> + help >> + This enables support for the Freescale S32 family of processors. Thanks; I will replace Freescale with NXP wherever possible in the next version. Regards, Stefan
Re: [PATCH v2 3/6] arm64: dts: fsl: Add device tree for S32V234-EVB
Hello Shawn,
Thank you for your suggestions!
On 8/19/2019 11:58 AM, Shawn Guo wrote:
> On Fri, Aug 09, 2019 at 11:29:12AM +, Stefan-gabriel Mirea wrote:
>> .../boot/dts/freescale/fsl-s32v234-evb.dts| 24
>
> The 'fsl-' prefix can be saved here, so that we can distinguish three
> families by starting string: imx??? for i.MX, fsl-??? for LayerScape,
> and s32??? for S32.
All right, I will remove the prefixes.
>> + model = "Freescale S32V234";
>
> The 'model' is usually used in board level DTS to describe the board.
I will delete the 'model' property from fsl-s32v234.dtsi and add a
suitable one in fsl-s32v234-evb.dts.
>> + };
>
> Please have a newline between nodes.
>
>> + cpu1: cpu@1 {
I've got it.
>> + interrupts = <0 59 1>;
>
> Please use GIC_SPI and IRQ_TYPE_xxx defines to make it more readable.
I will use GIC_SPI/GIC_PPI and IRQ_TYPE_xxx/GIC_CPU_MASK_xxx where
applicable.
>> +
>> + timer {
>> + compatible = "arm,armv8-timer";
>> + interrupts = <1 13 0xf08>,
>> + <1 14 0xf08>,
>> + <1 11 0xf08>,
>> + <1 10 0xf08>;
>> + /* clock-frequency might be modified by u-boot, depending on
>> the
>> + * chip version.
>> + */
>> + clock-frequency = <1000>;
>> + };
>> +
>> + gic: interrupt-controller@7d001000 {
>> + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>> + #interrupt-cells = <3>;
>> + #address-cells = <0>;
>> + interrupt-controller;
>> + reg = <0 0x7d001000 0 0x1000>,
>> + <0 0x7d002000 0 0x2000>,
>> + <0 0x7d004000 0 0x2000>,
>> + <0 0x7d006000 0 0x2000>;
>> + interrupts = <1 9 0xf04>;
>> + };
>
> We usually put these core platform devices prior to 'soc' node.
Sure, I will move the 'timer' and 'interrupt-controller' nodes right
before 'soc'.
Regards,
Stefan
Re: [PATCH v2] Documentation/admin-guide: Embargoed hardware security issues
On Tue, Aug 20, 2019 at 09:58:50AM -0500, Josh Poimboeuf wrote: > On Thu, Aug 15, 2019 at 11:25:05PM +0200, Greg Kroah-Hartman wrote: > > +Contact > > +--- > > + > > +The Linux kernel hardware security team is separate from the regular Linux > > +kernel security team. > > + > > +The team only handles the coordination of embargoed hardware security > > +issues. Reports of pure software security bugs in the Linux kernel are not > > +handled by this team and the reporter will be guided to contact the regular > > +Linux kernel security team (:ref:`Documentation/admin-guide/ > > +`) instead. > > + > > +The team can be contacted by email at . This > > +is a private list of security officers who will help you to coordinate an > > +issue according to our documented process. > > + > > +The list is encrypted and email to the list can be sent by either PGP or > > +S/MIME encrypted and must be signed with the reporter's PGP key or S/MIME > > +certificate. The list's PGP key and S/MIME certificate are available from > > +https://www.kernel.org/ > > This link needs to be filled in? > > > +Encrypted mailing-lists > > +--- > > + > > +We use encrypted mailing-lists for communication. The operating principle > > +of these lists is that email sent to the list is encrypted either with the > > +list's PGP key or with the list's S/MIME certificate. The mailing-list > > +software decrypts the email and re-encrypts it individually for each > > +subscriber with the subscriber's PGP key or S/MIME certificate. Details > > +about the mailing-list software and the setup which is used to ensure the > > +security of the lists and protection of the data can be found here: > > +https://www.kernel.org/ > > Ditto? Yes, they will once the links are up and running :) thanks, greg k-h
Re: [PATCH] docs: mtd: Update spi nor reference driver
On Tue, Aug 20, 2019 at 03:09:15PM +0100, John Garry wrote: > On 19/08/2019 05:39, Vignesh Raghavendra wrote: > > On 16/08/19 3:50 PM, John Garry wrote: > > > About the child spi flash devices, is the recommendation to just use > > > PRP0001 HID and "jedec,spi-nor" compatible? > > I am not quite familiar with ACPI systems, but child flash device should > > use "jedec,spi-nor" as compatible. > Right, so to use SPI MEM framework, it looks like I will have to use PRP0001 > and "jedec,spi-nor" as compatible. > My reluctance in using PRP0001 and compatible "jedec,spi-nor" is how other > OS can understand this. Last I heard Windows wasn't doing anything with PRP0001 but on the other hand the idiomatic way to handle this for ACPI is as far as I can tell to have what is essentially a board file loaded based on DMI information without any real enumerability so there's no real conflict between the two methods. signature.asc Description: PGP signature
Re: [PATCH v7 3/7] of/platform: Add functional dependency link from DT bindings
On Mon, Aug 19, 2019 at 9:26 PM Frank Rowand wrote: > > On 8/19/19 5:09 PM, Saravana Kannan wrote: > > On Mon, Aug 19, 2019 at 2:30 PM Frank Rowand wrote: > >> > >> On 8/19/19 1:49 PM, Saravana Kannan wrote: > >>> On Mon, Aug 19, 2019 at 10:16 AM Frank Rowand > >>> wrote: > > On 8/15/19 6:50 PM, Saravana Kannan wrote: > > On Wed, Aug 7, 2019 at 7:06 PM Frank Rowand > > wrote: > >> > >> On 7/23/19 5:10 PM, Saravana Kannan wrote: > >>> Add device-links after the devices are created (but before they are > >>> probed) by looking at common DT bindings like clocks and > >>> interconnects. > > > < very big snip (lots of comments that deserve answers) > > > > >> > >> /** > >> * of_link_property - TODO: > >> * dev: > >> * con_np: > >> * prop: > >> * > >> * TODO... > >> * > >> * Any failed attempt to create a link will NOT result in an immediate > >> return. > >> * of_link_property() must create all possible links even when one of > >> more > >> * attempts to create a link fail. > >> > >> Why? isn't one failure enough to prevent probing this device? > >> Continuing to scan just results in extra work... which will be > >> repeated every time device_link_check_waiting_consumers() is called > > > > Context: > > As I said in the cover letter, avoiding unnecessary probes is just one > > of the reasons for this patch. The other (arguably more important) > > Agree that it is more important. > > > > reason for this patch is to make sure suppliers know that they have > > consumers that are yet to be probed. That way, suppliers can leave > > their resource on AND in the right state if they were left on by the > > bootloader. For example, if a clock was left on and at 200 MHz, the > > clock provider needs to keep that clock ON and at 200 MHz till all the > > consumers are probed. > > > > Answer: Let's say a consumer device Z has suppliers A, B and C. If the > > linking fails at A and you return immediately, then B and C could > > probe and then figure that they have no more consumers (they don't see > > a link to Z) and turn off their resources. And Z could fail > > catastrophically. > > Then I think that this approach is fatally flawed in the current > implementation. > >>> > >>> I'm waiting to hear how it is fatally flawed. But maybe this is just a > >>> misunderstanding of the problem? > >> > >> Fatally flawed because it does not handle modules that add a consumer > >> device when the module is loaded. > > > > If you are talking about modules adding child devices of the device > > they are managing, then that's handled correctly later in the series. > > They may or they may not. I do not know. I am not going to audit all > current cases of devices being added to check that relationship and I am > not going to monitor all future patches that add devices. Adding devices > is an existing pattern of behavior that the new feature must be able to > handle. > > I have not looked at patch 6 yet (the place where modules adding child > devices is handled). I am guessing that patch 6 could be made more > general to remove the parent child relationship restriction. Please do look into it then. I think it already handles all cases. > > > > If you are talking about modules adding devices that aren't defined in > > DT, then right, I'm not trying to handle that. The module needs to > > make sure it keeps the resources needed for new devices it's adding > > are in the right state or need to add the right device links. > > I am not talking about devices that are not defined in the devicetree. In that case, I'm sure my patch series handle all the scenarios correctly. Here's why: 1. For all the top level devices the patches you've reviewed already show how it's handled correctly. 2. All other devices in the DT are by definition the child devices of the top level devices and patch 6 handles those cases. Hopefully this shows to you that all DT cases are handled correctly. > >>> In the text below, I'm not sure if you mixing up two different things > >>> or just that your wording it a bit ambiguous. So pardon my nitpick to > >>> err on the side of clarity. > >> > >> Please do nitpick. Clarity is good. > >> > >> > >>> > A device can be added by a module that is loaded. > >>> > >>> No, in the example I gave, of_platform_default_populate_init() would > >>> add all 3 of those devices during arch_initcall_sync(). > >> > >> The example you gave does not cover all use cases. > >> > >> There are modules that add devices when the module is loaded. You can not > >> ignore systems using such modules. > > > > I'll have to agree to disagree on that. While I understand that the > > design should be good and I'm happy to work on that, you can't insist > > that a patch series shouldn't be allowed b
Re: [RFC 01/19] kbuild: Fixes to rules for host-cshlib and host-cxxshlib
On Wed, Aug 14, 2019 at 9:53 PM Knut Omang wrote: > > On Wed, 2019-08-14 at 07:52 +0200, Knut Omang wrote: > > On Wed, 2019-08-14 at 11:02 +0900, Masahiro Yamada wrote: > > > Hi Knut, > > > > > > On Wed, Aug 14, 2019 at 1:19 AM Knut Omang wrote: > > > > On Tue, 2019-08-13 at 23:01 +0900, Masahiro Yamada wrote: > > > > > On Tue, Aug 13, 2019 at 3:13 PM Knut Omang > > > > > wrote: > > > > > > C++ libraries interfacing to C APIs might sometimes need some glue > > > > > > logic more easily written in C. > > > > > > Allow a C++ library to also contain 0 or more C objects. > > > > > > > > > > > > Also fix rules for both C and C++ shared libraries: > > > > > > - C++ shared libraries depended on .c instead of .cc files > > > > > > - Rules were referenced as -objs instead of the intended > > > > > > -cobjs and -cxxobjs following the pattern from hostprogs*. > > > > > > > > > > > > Signed-off-by: Knut Omang > > > > > > > > > > How is this patch related to the rest of this series? > > > > > > > > This is just my (likely naive) way I to get what I had working > > > > using autotools in the Github version of KTF) translated into something > > > > comparable using kbuild only. We need to build a shared library > > > > consisting > > > > of a few C++ files and a very simple C file, and a couple of simple > > > > binaries, > > > > and the rule in there does seem to take .c files and subject them to the > > > > C++ compiler, which makes this difficult to achieve? > > > > > > Looking at the diff stat of the cover-letter, > > > the rest of this patch series is touching only > > > Documentation/ and tools/testing/kselftests/. > > > > > > So, this one is unused by the rest of the changes, isn't it? > > > Am I missing something? > > > > > > > > > > > > > > This patch breaks GCC-plugins. > > > > > Did you really compile-test this patch before the submission? > > > > > > > > Sorry for my ignorance here: > > > > I ran through the kernel build and installed the resulting kernel > > > > on a VM that I used to test this, if that's what you are asking > > > > about? > > > > > > > > Do I need some unusual .config options or run a special make target > > > > to trigger the problem you see? > > > > > > > > I used a recent Fedora config with default values for new options, > > > > and ran the normal default make target (also with O=) and make selftests > > > > to test the patch itself. > > > > > > I just built allmodconfig for arm. > > > > > > (The 0-day bot tests allmodconfig for most of architectures, > > > so you may receive error reports anyway.) > > > > > > > > > With your patch, I got the following: > > > > > > > > > masahiro@grover:~/ref/linux$ make ARCH=arm > > > CROSS_COMPILE=- allmodconfig all > > > HOSTCC scripts/basic/fixdep > > > HOSTCC scripts/kconfig/conf.o > > > HOSTCC scripts/kconfig/confdata.o > > > HOSTCC scripts/kconfig/expr.o > > > LEX scripts/kconfig/lexer.lex.c > > > YACCscripts/kconfig/parser.tab.h > > > HOSTCC scripts/kconfig/lexer.lex.o > > > YACCscripts/kconfig/parser.tab.c > > > HOSTCC scripts/kconfig/parser.tab.o > > > HOSTCC scripts/kconfig/preprocess.o > > > HOSTCC scripts/kconfig/symbol.o > > > HOSTLD scripts/kconfig/conf > > > scripts/kconfig/conf --allmodconfig Kconfig > > > # > > > # configuration written to .config > > > # > > > SYSHDR arch/arm/include/generated/uapi/asm/unistd-common.h > > > SYSHDR arch/arm/include/generated/uapi/asm/unistd-oabi.h > > > SYSHDR arch/arm/include/generated/uapi/asm/unistd-eabi.h > > > HOSTCC scripts/dtc/dtc.o > > > HOSTCC scripts/dtc/flattree.o > > > HOSTCC scripts/dtc/fstree.o > > > HOSTCC scripts/dtc/data.o > > > HOSTCC scripts/dtc/livetree.o > > > HOSTCC scripts/dtc/treesource.o > > > HOSTCC scripts/dtc/srcpos.o > > > HOSTCC scripts/dtc/checks.o > > > HOSTCC scripts/dtc/util.o > > > LEX scripts/dtc/dtc-lexer.lex.c > > > YACCscripts/dtc/dtc-parser.tab.h > > > HOSTCC scripts/dtc/dtc-lexer.lex.o > > > YACCscripts/dtc/dtc-parser.tab.c > > > HOSTCC scripts/dtc/dtc-parser.tab.o > > > HOSTCC scripts/dtc/yamltree.o > > > HOSTLD scripts/dtc/dtc > > > CC scripts/gcc-plugins/latent_entropy_plugin.o > > > cc1: error: cannot load plugin > > > ./scripts/gcc-plugins/arm_ssp_per_task_plugin.so > > >./scripts/gcc-plugins/arm_ssp_per_task_plugin.so: cannot open > > > shared object file: No such file or directory > > > cc1: error: cannot load plugin ./scripts/gcc-plugins/structleak_plugin.so > > >./scripts/gcc-plugins/structleak_plugin.so: cannot open shared > > > object file: No such file or directory > > > cc1: error: cannot load plugin > > > ./scripts/gcc-plugins/latent_entropy_plugin.so > > >./scripts/gcc-plugins/latent_entropy_plugin.so: cannot open shared > > > object file: No such file or directory > > > cc1: error: cannot load plugin > > > ./scripts/gcc-plugins/randomize_layout_plugin.so > > >./scripts/gcc-plugins/randomize_layout_pl
Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
Hi Jisheng, On Tue, 20 Aug 2019 09:02:59 + Jisheng Zhang wrote: > Hi Thomas, > > On Tue, 20 Aug 2019 10:53:58 +0200 (CEST) Thomas Gleixner wrote: > > > > > > > On Tue, 20 Aug 2019, Jisheng Zhang wrote: > > > > > This is to make the x86 kprobe_ftrace_handler() more common so that > > > the code could be reused in future. > > > > While I agree with the change in general, I can't find anything which > > reuses that code. So the change log is pretty useless and I have no idea > > how this is related to the rest of the series. > > In v1, this code is moved from x86 to common kprobes.c [1] > But I agree with Masami, consolidation could be done when arm64 kprobes > on ftrace is stable. We'll revisit to consolidate the code after we got 3rd or 4th clones. > > In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same > as x86's, the only difference is comment, e.g > > /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */ > > while in arm64 > > /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */ As Peter pointed, on arm64, is that really 1 or 4 bytes? This part is heavily depends on the processor software-breakpoint implementation. > > > W/ above, any suggestion about the suitable change log? I think you just need to keep the first half of the description. Since this patch itself is not related to the series, could you update the description and resend it as a single cleanup patch out of the series? Thank you! > > Thanks > > [1] > http://lists.infradead.org/pipermail/linux-arm-kernel/2019-August/674417.html -- Masami Hiramatsu
Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
Hi Peter, On Tue, 20 Aug 2019 15:21:10 +0200 Peter Zijlstra wrote: > > > On Tue, Aug 20, 2019 at 09:02:59AM +, Jisheng Zhang wrote: > > In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same > > as x86's, the only difference is comment, e.g > > > > /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */ > > > > while in arm64 > > > > /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */ > > What's weird; I thought ARM has fixed sized instructions and they are > all 4 bytes? So how does a single byte offset make sense for ARM? I believe the "+1" here means + one kprobe_opcode_t. Thanks
Re: [PATCH v2 2/3] kprobes: adjust kprobe addr for KPROBES_ON_FTRACE
Hi Jisheng,
On Tue, 20 Aug 2019 03:53:31 +
Jisheng Zhang wrote:
> For KPROBES_ON_FTRACE case, we need to adjust the kprobe's addr
> correspondingly.
Either KPROBES_ON_FTRACE=y or not, ftrace_location() check must be
done correctly. If it failed, kprobes can modify the instruction
which can be modified by ftrace.
>
> Signed-off-by: Jisheng Zhang
> ---
> kernel/kprobes.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 9873fc627d61..3fd2f68644da 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1484,15 +1484,19 @@ static inline int check_kprobe_rereg(struct kprobe *p)
>
> int __weak arch_check_ftrace_location(struct kprobe *p)
> {
> - unsigned long ftrace_addr;
> + unsigned long ftrace_addr, addr = (unsigned long)p->addr;
>
> - ftrace_addr = ftrace_location((unsigned long)p->addr);
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> + addr = ftrace_call_adjust(addr);
> +#endif
> + ftrace_addr = ftrace_location(addr);
No, this is not right way to do. If we always need to adjust address
before calling ftrace_location(), something wrong with ftrace_location()
interface.
ftrace_location(addr) must check the address is within the range which
can be changed by ftrace. (dyn->ip <= addr <= dyn->ip+MCOUNT_INSN_SIZE)
> if (ftrace_addr) {
> #ifdef CONFIG_KPROBES_ON_FTRACE
> /* Given address is not on the instruction boundary */
> - if ((unsigned long)p->addr != ftrace_addr)
> + if (addr != ftrace_addr)
> return -EILSEQ;
> p->flags |= KPROBE_FLAG_FTRACE;
> + p->addr = (kprobe_opcode_t *)addr;
And again, please don't change the p->addr silently.
Thank you,
> #else/* !CONFIG_KPROBES_ON_FTRACE */
> return -EINVAL;
> #endif
> --
> 2.23.0.rc1
>
--
Masami Hiramatsu
Re: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
Hi, On Wed, 21 Aug 2019 10:52:47 +0900 Masami Hiramatsu wrote: > > > Hi Jisheng, > > On Tue, 20 Aug 2019 09:02:59 + > Jisheng Zhang wrote: > > > Hi Thomas, > > > > On Tue, 20 Aug 2019 10:53:58 +0200 (CEST) Thomas Gleixner wrote: > > > > > > > > > > > On Tue, 20 Aug 2019, Jisheng Zhang wrote: > > > > > > > This is to make the x86 kprobe_ftrace_handler() more common so that > > > > the code could be reused in future. > > > > > > While I agree with the change in general, I can't find anything which > > > reuses that code. So the change log is pretty useless and I have no idea > > > how this is related to the rest of the series. > > > > In v1, this code is moved from x86 to common kprobes.c [1] > > But I agree with Masami, consolidation could be done when arm64 kprobes > > on ftrace is stable. > > We'll revisit to consolidate the code after we got 3rd or 4th clones. > > > > > In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same > > as x86's, the only difference is comment, e.g > > > > /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */ > > > > while in arm64 > > > > /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */ > > As Peter pointed, on arm64, is that really 1 or 4 bytes? > This part is heavily depends on the processor software-breakpoint > implementation. Per my understanding, the "+1" here means "+ one kprobe_opcode_t". > > > > > > > W/ above, any suggestion about the suitable change log? > > I think you just need to keep the first half of the description. > Since this patch itself is not related to the series, could you update > the description and resend it as a single cleanup patch out of the series? > Got it. Will do today. Thanks a lot
Re: [PATCH v2 2/3] kprobes: adjust kprobe addr for KPROBES_ON_FTRACE
Hi,
On Wed, 21 Aug 2019 11:07:39 +0900 Masami Hiramatsu wrote:
>
>
> Hi Jisheng,
>
> On Tue, 20 Aug 2019 03:53:31 +
> Jisheng Zhang wrote:
>
> > For KPROBES_ON_FTRACE case, we need to adjust the kprobe's addr
> > correspondingly.
>
> Either KPROBES_ON_FTRACE=y or not, ftrace_location() check must be
> done correctly. If it failed, kprobes can modify the instruction
> which can be modified by ftrace.
>
> >
> > Signed-off-by: Jisheng Zhang
> > ---
> > kernel/kprobes.c | 10 +++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 9873fc627d61..3fd2f68644da 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1484,15 +1484,19 @@ static inline int check_kprobe_rereg(struct kprobe
> > *p)
> >
> > int __weak arch_check_ftrace_location(struct kprobe *p)
> > {
> > - unsigned long ftrace_addr;
> > + unsigned long ftrace_addr, addr = (unsigned long)p->addr;
> >
> > - ftrace_addr = ftrace_location((unsigned long)p->addr);
> > +#ifdef CONFIG_KPROBES_ON_FTRACE
> > + addr = ftrace_call_adjust(addr);
> > +#endif
> > + ftrace_addr = ftrace_location(addr);
>
> No, this is not right way to do. If we always need to adjust address
> before calling ftrace_location(), something wrong with ftrace_location()
> interface.
> ftrace_location(addr) must check the address is within the range which
> can be changed by ftrace. (dyn->ip <= addr <= dyn->ip+MCOUNT_INSN_SIZE)
yeah! I will try Naveen's suggestion, I.E patch kprobe_lookup_name() instead.
Thanks
>
>
> > if (ftrace_addr) {
> > #ifdef CONFIG_KPROBES_ON_FTRACE
> > /* Given address is not on the instruction boundary */
> > - if ((unsigned long)p->addr != ftrace_addr)
> > + if (addr != ftrace_addr)
> > return -EILSEQ;
> > p->flags |= KPROBE_FLAG_FTRACE;
> > + p->addr = (kprobe_opcode_t *)addr;
>
> And again, please don't change the p->addr silently.
>
> Thank you,
>
> > #else/* !CONFIG_KPROBES_ON_FTRACE */
> > return -EINVAL;
> > #endif
> > --
> > 2.23.0.rc1
> >
>
>
> --
> Masami Hiramatsu
Re: [PATCH v5 1/9] fpga: dfl: make init callback optional
Hi,
On Mon, Aug 12, 2019 at 10:49:56AM +0800, Wu Hao wrote:
> This patch makes init callback of sub features optional. With
> this change, people don't need to prepare any empty init callback.
>
> Signed-off-by: Wu Hao
Acked-by: Moritz Fischer
> ---
> drivers/fpga/dfl.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index c0512af..96a2b82 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -271,11 +271,13 @@ static int dfl_feature_instance_init(struct
> platform_device *pdev,
>struct dfl_feature *feature,
>struct dfl_feature_driver *drv)
> {
> - int ret;
> + int ret = 0;
>
> - ret = drv->ops->init(pdev, feature);
> - if (ret)
> - return ret;
> + if (drv->ops->init) {
> + ret = drv->ops->init(pdev, feature);
> + if (ret)
> + return ret;
> + }
>
> feature->ops = drv->ops;
You could swap it around maybe like so:
int dfl_feature_instance_init() ...
{
feature->ops = drv->ops;
if (drv->ops->init)
return drv->ops->init(pdev, feature);
return 0;
}
With the caveat that feature->ops gets always set ...
Your call.
Thanks,
Moritz
Re: [PATCH v5 2/9] fpga: dfl: fme: convert platform_driver to use dev_groups
Hi Hao,
On Mon, Aug 12, 2019 at 10:49:57AM +0800, Wu Hao wrote:
> This patch takes advantage of driver core which helps to create
> and remove sysfs attribute files, so there is no need to register
> sysfs entries manually in dfl-fme platform river code.
Nit: s/river/driver
>
> Signed-off-by: Wu Hao
Acked-by: Moritz Fischer
> ---
> drivers/fpga/dfl-fme-main.c | 29 ++---
> 1 file changed, 2 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> index f033f1c..bf8114d 100644
> --- a/drivers/fpga/dfl-fme-main.c
> +++ b/drivers/fpga/dfl-fme-main.c
> @@ -129,30 +129,6 @@ static ssize_t socket_id_show(struct device *dev,
> };
> ATTRIBUTE_GROUPS(fme_hdr);
>
> -static int fme_hdr_init(struct platform_device *pdev,
> - struct dfl_feature *feature)
> -{
> - void __iomem *base = feature->ioaddr;
> - int ret;
> -
> - dev_dbg(&pdev->dev, "FME HDR Init.\n");
> - dev_dbg(&pdev->dev, "FME cap %llx.\n",
> - (unsigned long long)readq(base + FME_HDR_CAP));
> -
> - ret = device_add_groups(&pdev->dev, fme_hdr_groups);
> - if (ret)
> - return ret;
> -
> - return 0;
> -}
> -
> -static void fme_hdr_uinit(struct platform_device *pdev,
> - struct dfl_feature *feature)
> -{
> - dev_dbg(&pdev->dev, "FME HDR UInit.\n");
> - device_remove_groups(&pdev->dev, fme_hdr_groups);
> -}
> -
> static long fme_hdr_ioctl_release_port(struct dfl_feature_platform_data
> *pdata,
> unsigned long arg)
> {
> @@ -199,8 +175,6 @@ static long fme_hdr_ioctl(struct platform_device *pdev,
> };
>
> static const struct dfl_feature_ops fme_hdr_ops = {
> - .init = fme_hdr_init,
> - .uinit = fme_hdr_uinit,
> .ioctl = fme_hdr_ioctl,
> };
>
> @@ -361,7 +335,8 @@ static int fme_remove(struct platform_device *pdev)
>
> static struct platform_driver fme_driver = {
> .driver = {
> - .name= DFL_FPGA_FEATURE_DEV_FME,
> + .name = DFL_FPGA_FEATURE_DEV_FME,
> + .dev_groups = fme_hdr_groups,
> },
> .probe = fme_probe,
> .remove = fme_remove,
> --
> 1.8.3.1
>
Thanks,
Moritz
Re: [RFC 01/19] kbuild: Fixes to rules for host-cshlib and host-cxxshlib
On Wed, 2019-08-21 at 10:47 +0900, Masahiro Yamada wrote: > On Wed, Aug 14, 2019 at 9:53 PM Knut Omang wrote: > > > > On Wed, 2019-08-14 at 07:52 +0200, Knut Omang wrote: > > > On Wed, 2019-08-14 at 11:02 +0900, Masahiro Yamada wrote: > > > > Hi Knut, > > > > > > > > On Wed, Aug 14, 2019 at 1:19 AM Knut Omang > > > > wrote: > > > > > On Tue, 2019-08-13 at 23:01 +0900, Masahiro Yamada wrote: > > > > > > On Tue, Aug 13, 2019 at 3:13 PM Knut Omang > > > > > > wrote: > > > > > > > C++ libraries interfacing to C APIs might sometimes need some glue > > > > > > > logic more easily written in C. > > > > > > > Allow a C++ library to also contain 0 or more C objects. > > > > > > > > > > > > > > Also fix rules for both C and C++ shared libraries: > > > > > > > - C++ shared libraries depended on .c instead of .cc files > > > > > > > - Rules were referenced as -objs instead of the intended > > > > > > > -cobjs and -cxxobjs following the pattern from hostprogs*. > > > > > > > > > > > > > > Signed-off-by: Knut Omang > > > > > > > > > > > > How is this patch related to the rest of this series? > > > > > > > > > > This is just my (likely naive) way I to get what I had working > > > > > using autotools in the Github version of KTF) translated into > > > > > something > > > > > comparable using kbuild only. We need to build a shared library > > > > > consisting > > > > > of a few C++ files and a very simple C file, and a couple of simple > > > > > binaries, > > > > > and the rule in there does seem to take .c files and subject them to > > > > > the > > > > > C++ compiler, which makes this difficult to achieve? > > > > > > > > Looking at the diff stat of the cover-letter, > > > > the rest of this patch series is touching only > > > > Documentation/ and tools/testing/kselftests/. > > > > > > > > So, this one is unused by the rest of the changes, isn't it? > > > > Am I missing something? > > > > > > > > > > > > > > > > > > This patch breaks GCC-plugins. > > > > > > Did you really compile-test this patch before the submission? > > > > > > > > > > Sorry for my ignorance here: > > > > > I ran through the kernel build and installed the resulting kernel > > > > > on a VM that I used to test this, if that's what you are asking > > > > > about? > > > > > > > > > > Do I need some unusual .config options or run a special make target > > > > > to trigger the problem you see? > > > > > > > > > > I used a recent Fedora config with default values for new options, > > > > > and ran the normal default make target (also with O=) and make > > > > > selftests > > > > > to test the patch itself. > > > > > > > > I just built allmodconfig for arm. > > > > > > > > (The 0-day bot tests allmodconfig for most of architectures, > > > > so you may receive error reports anyway.) > > > > > > > > > > > > With your patch, I got the following: > > > > > > > > > > > > masahiro@grover:~/ref/linux$ make ARCH=arm > > > > CROSS_COMPILE=- allmodconfig all > > > > HOSTCC scripts/basic/fixdep > > > > HOSTCC scripts/kconfig/conf.o > > > > HOSTCC scripts/kconfig/confdata.o > > > > HOSTCC scripts/kconfig/expr.o > > > > LEX scripts/kconfig/lexer.lex.c > > > > YACCscripts/kconfig/parser.tab.h > > > > HOSTCC scripts/kconfig/lexer.lex.o > > > > YACCscripts/kconfig/parser.tab.c > > > > HOSTCC scripts/kconfig/parser.tab.o > > > > HOSTCC scripts/kconfig/preprocess.o > > > > HOSTCC scripts/kconfig/symbol.o > > > > HOSTLD scripts/kconfig/conf > > > > scripts/kconfig/conf --allmodconfig Kconfig > > > > # > > > > # configuration written to .config > > > > # > > > > SYSHDR arch/arm/include/generated/uapi/asm/unistd-common.h > > > > SYSHDR arch/arm/include/generated/uapi/asm/unistd-oabi.h > > > > SYSHDR arch/arm/include/generated/uapi/asm/unistd-eabi.h > > > > HOSTCC scripts/dtc/dtc.o > > > > HOSTCC scripts/dtc/flattree.o > > > > HOSTCC scripts/dtc/fstree.o > > > > HOSTCC scripts/dtc/data.o > > > > HOSTCC scripts/dtc/livetree.o > > > > HOSTCC scripts/dtc/treesource.o > > > > HOSTCC scripts/dtc/srcpos.o > > > > HOSTCC scripts/dtc/checks.o > > > > HOSTCC scripts/dtc/util.o > > > > LEX scripts/dtc/dtc-lexer.lex.c > > > > YACCscripts/dtc/dtc-parser.tab.h > > > > HOSTCC scripts/dtc/dtc-lexer.lex.o > > > > YACCscripts/dtc/dtc-parser.tab.c > > > > HOSTCC scripts/dtc/dtc-parser.tab.o > > > > HOSTCC scripts/dtc/yamltree.o > > > > HOSTLD scripts/dtc/dtc > > > > CC scripts/gcc-plugins/latent_entropy_plugin.o > > > > cc1: error: cannot load plugin > > > > ./scripts/gcc-plugins/arm_ssp_per_task_plugin.so > > > >./scripts/gcc-plugins/arm_ssp_per_task_plugin.so: cannot open > > > > shared object file: No such file or directory > > > > cc1: error: cannot load plugin > > > > ./scripts/gcc-plugins/structleak_plugin.so > > > >./scripts/gcc-plugins/structleak_plugin.so: cannot open shared > > > > object file: No such file or directory > > > > cc1: error: cannot
Re: [PATCH v5 1/9] fpga: dfl: make init callback optional
On Tue, Aug 20, 2019 at 08:24:06PM -0700, Moritz Fischer wrote:
> Hi,
>
> On Mon, Aug 12, 2019 at 10:49:56AM +0800, Wu Hao wrote:
> > This patch makes init callback of sub features optional. With
> > this change, people don't need to prepare any empty init callback.
> >
> > Signed-off-by: Wu Hao
>
> Acked-by: Moritz Fischer
> > ---
> > drivers/fpga/dfl.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> > index c0512af..96a2b82 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -271,11 +271,13 @@ static int dfl_feature_instance_init(struct
> > platform_device *pdev,
> > struct dfl_feature *feature,
> > struct dfl_feature_driver *drv)
> > {
> > - int ret;
> > + int ret = 0;
> >
> > - ret = drv->ops->init(pdev, feature);
> > - if (ret)
> > - return ret;
> > + if (drv->ops->init) {
> > + ret = drv->ops->init(pdev, feature);
> > + if (ret)
> > + return ret;
> > + }
> >
> > feature->ops = drv->ops;
>
> You could swap it around maybe like so:
>
> int dfl_feature_instance_init() ...
> {
> feature->ops = drv->ops;
> if (drv->ops->init)
> return drv->ops->init(pdev, feature);
>
> return 0;
> }
>
> With the caveat that feature->ops gets always set ...
>
> Your call.
Hi Moritz,
Thanks a lot for the review and comments. It does simplify the code,
will modify it.
Thanks
Hao
>
> Thanks,
> Moritz
