Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To:     Masami Hiramatsu <mhiramat@kernel.org>
CC:     "catalin.marinas@arm.com" <catalin.marinas@arm.com>,
        "will@kernel.org" <will@kernel.org>,
        "naveen.n.rao@linux.vnet.ibm.com" <naveen.n.rao@linux.vnet.ibm.com>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v6] arm64: implement KPROBES_ON_FTRACE
Thread-Topic: [PATCH v6] arm64: implement KPROBES_ON_FTRACE
Thread-Index: AQHVtWtmR90OKS0v9Um1jpVbBvlnOKe/4n0AgAd5EwA=
Date:   Mon, 23 Dec 2019 07:47:24 +0000
Message-ID: <20191223153300.30281a93@xhacker.debian>
References: <20191218140622.57bbaca5@xhacker.debian>
        <20191218222550.51f0b681de7bbab7e49b09a9@kernel.org>
In-Reply-To: <20191218222550.51f0b681de7bbab7e49b09a9@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu)
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: f446b039-bbc4-4ca6-12f0-08d7877c5936
x-ms-traffictypediagnostic: BYAPR03MB4584:
x-microsoft-antispam-prvs: <BYAPR03MB45844924DD7ACA1513C7862FED2E0@BYAPR03MB4584.namprd03.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-forefront-prvs: 0260457E99
x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(39850400004)(396003)(366004)(136003)(376002)(346002)(189003)(199004)(5660300002)(54906003)(86362001)(9686003)(2906002)(1076003)(55016002)(6916009)(52116002)(7696005)(316002)(66476007)(66556008)(66946007)(64756008)(26005)(7416002)(186003)(16526019)(66446008)(8676002)(81156014)(6506007)(956004)(81166006)(4326008)(71200400001)(8936002)(478600001)(39210200001);DIR:OUT;SFP:1101;SCL:1;SRVR:BYAPR03MB4584;H:BYAPR03MB4773.namprd03.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:0;
received-spf: None (protection.outlook.com: synaptics.com does not designate
 permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: hrBZmR7sP67IUr0bsc+LhAKPTjD7woPxbafQ9ce6F3EU3tgndPGXtZBwwb96qMmdkR4d++/EQ6hasJNdLC+Qhb3ZkvgtEoPxrJVzj6q+6LJ7HtBG/S0VTrJGYcMa8nQSRJmmU50CzFCxZ+NES4t/jw9cuF2udFK6+q60ibn+ujgpOAC7MjlRukjd4Lv53Br2KsVS5qvKWNkEaJNZILIxu/G7FCMFXVFNGQF6fcszGjApyq35wQv2npFejnKqmALWTtJvbrm+VSZLbOxLlXIc2lRXNclSJV4FdNn4Y1v5tvMKqM8PjYqnyendlHvMhQjKCrT7i+8vXz95zX/zpe5vP/pukjdx9xCY8itg2NT1mfLKaEbpac1ldnfo0xNpz+niLaepmDHCCJ8eFwhY2kGvAD4T79U5thPzFNHs+QdRJsHY3mIC/NVZEh1qY3zmqKPO4SSQQ7zKMwTBfQWIsO0SnddyHrMUjRcgqaasbqb4i/xTWU6g1uRYjoQ3VO+HvwZg
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3A361DDF74E5F848BA8CA005812464A1@namprd03.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: synaptics.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f446b039-bbc4-4ca6-12f0-08d7877c5936
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Dec 2019 07:47:24.1743
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 335d1fbc-2124-4173-9863-17e7051a2a0e
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: EPI63Rh0MhmDGfTtYWz9i8oIGWabR1NHsL6Ro4AiSLeIKhjMJ3YCBZpWdKIZNkKXnMePbJK8Nr9RXXM5AbucMw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR03MB4584
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Masami,

On Wed, 18 Dec 2019 22:25:50 +0900 Masami Hiramatsu wrote:


>=20
>=20
> On Wed, 18 Dec 2019 06:21:35 +0000
> Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
>=20
> > KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as i=
t
> > eliminates the need for a trap, as well as the need to emulate or
> > single-step instructions.
> >
> > Tested on berlin arm64 platform.
> >
> > ~ # 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
> > ffffff801009fe28  k  _do_fork+0x0    [DISABLED]
> >
> > after the patch:
> >
> > /sys/kernel/debug # cat kprobes/list
> > ffffff801009ff54  k  _do_fork+0x4    [DISABLED][FTRACE] =20
>=20
> BTW, it seems this automatically changes the offset without
> user's intention or any warnings. How would you manage if the user
> pass a new probe on _do_fork+0x4?

In current implementation, two probes at the same address _do_fork+0x4

>=20
> IOW, it is still the question who really wants to probe on
> the _do_fork+"0", if kprobes modifies it automatically,
> no one can do that anymore.
> This can be happen if the user want to record LR or SP value
> at the function call for debug. If kprobe always modifies it,
> we will lose the way to do it.

arm64's DYNAMIC_FTRACE_WITH_REGS implementation makes use of GCC
-fpatchable-function-entry=3D2 option to insert two nops. When the function
is traced, the first nop will be modified to the LR saver, then the
second nop to "bl <ftrace-entry>", commit 3b23e4991fb6("
arm64: implement ftrace with regs") explains the mechanism.

So on arm64(in fact any arch makes use of -fpatchable-function-entry will
behave similarly), when DYNAMIC_FTRACE_WITH_REGS is enabled, the ftrace loc=
ation
is always on the first 4 bytes offset.

I think when users want to register a kprobe on _do_fork+0, what he really =
want
is to kprobe on the patched(by -fpatchable-function-entry)_do_fork+4

PS: per my understanding, powerpc's kprobes_on_ftrace also introduces an
extra automatic offset due to its implementation.

>=20
> Could you remove below function at this moment?
>=20
> > +kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int off=
set)
> > +{
> > +     unsigned long addr =3D kallsyms_lookup_name(name);
> > +
> > +     if (addr && !offset) {
> > +             unsigned long faddr;
> > +             /*
> > +              * with -fpatchable-function-entry=3D2, the first 4 bytes=
 is the
> > +              * LR saver, then the actual call insn. So ftrace locatio=
n is
> > +              * always on the first 4 bytes offset.
> > +              */
> > +             faddr =3D ftrace_location_range(addr,
> > +                                           addr + AARCH64_INSN_SIZE);
> > +             if (faddr)
> > +                     return (kprobe_opcode_t *)faddr;
> > +     }
> > +     return (kprobe_opcode_t *)addr;
> > +}
> > +
> > +bool arch_kprobe_on_func_entry(unsigned long offset)
> > +{
> > +     return offset <=3D AARCH64_INSN_SIZE;
> > +} =20
>=20
>=20
> Without this automatic change, we still can change the offset
> in upper layer.

If remove the two functions, kprobe on  _do_fork can't ride on
ftrace infrastructure, but kprobe on _do_fork+4 can. I'm not sure
whether this is reasonable. Every kprobe users on arm64 will need to
remember to pass an extra offset +4 to make use of kprobe_on_ftrace, could
we hide the "+4"?

Thanks