Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: YkmvJBfVQsvNz20PVO73+wOnctsN7Ft0FDPGHOeLrHxKqpP9TUaYoV8lHNiviuscxd7E3q0eI2
 xKfB67zb+pxseQGQjzHGFqvrqfn9NAxBhp30MT5Iuh8pa5q4fnRQF+hZHDl6uYEhhIzz90BKAA
 L0RWjFCSyd64ETFJZDSOzKCDpM+5XkmUamE7zacFXmn2OuOioyPVOqPJ7LV9or4fAIRzUB9IZx
 Sliv144dRRaGaPl0nMuNFEw7tM1Ng49HXIUHYiyv2lUuy47GVMSdcg2k0un5yY0jWF+FxyJU08
 CPY=
From:   Anup Patel <Anup.Patel@wdc.com>
To:     Steven Rostedt <rostedt@goodmis.org>,
        Changbin Du <changbin.du@gmail.com>
CC:     Palmer Dabbelt <palmer@dabbelt.com>,
        "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        "aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "jpoimboe@redhat.com" <jpoimboe@redhat.com>,
        "jbaron@akamai.com" <jbaron@akamai.com>,
        "ardb@kernel.org" <ardb@kernel.org>,
        Atish Patra <Atish.Patra@wdc.com>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "rppt@kernel.org" <rppt@kernel.org>,
        "mhiramat@kernel.org" <mhiramat@kernel.org>,
        "zong.li@sifive.com" <zong.li@sifive.com>,
        "guoren@linux.alibaba.com" <guoren@linux.alibaba.com>,
        "wangkefeng.wang@huawei.com" <wangkefeng.wang@huawei.com>,
        "0x7f454c46@gmail.com" <0x7f454c46@gmail.com>,
        "chenhuang5@huawei.com" <chenhuang5@huawei.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kernel-team@android.com" <kernel-team@android.com>,
        Palmer Dabbelt <palmerdabbelt@google.com>
Subject: RE: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POKE*
Thread-Topic: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POKE*
Thread-Index: AQHXPL+/nC0TgFS8Ak2mo06nM8KQNarLsFWAgABauoCAAFHTAIAAFTAg
Date:   Fri, 30 Apr 2021 04:06:35 +0000
Message-ID: <MN2PR04MB6207E102997326E6513416AB8D5E9@MN2PR04MB6207.namprd04.prod.outlook.com>
References: <20210429061713.783628-1-palmer@dabbelt.com>
        <20210429123007.5144fc0d@gandalf.local.home>
        <20210429215451.yuey5gzmfh2dkzp5@mail.google.com>
 <20210429224742.391154ae@oasis.local.home>
In-Reply-To: <20210429224742.391154ae@oasis.local.home>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 6b47abb9-a9b2-4302-25ae-08d90b8d58e1
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Apr 2021 04:06:35.6261
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: i3c+Zpe0JPZe8yurn3Xw4gvi0K8RHDKY1ULupxzl3G5fFL1OEuyOnc1uxQVff6b3P7z0HcLRmgxyn3GNX4pBgA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR04MB6029
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Steven Rostedt <rostedt@goodmis.org>
> Sent: 30 April 2021 08:18
> To: Changbin Du <changbin.du@gmail.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>; linux-riscv@lists.infradead.org;
> Paul Walmsley <paul.walmsley@sifive.com>; aou@eecs.berkeley.edu;
> peterz@infradead.org; jpoimboe@redhat.com; jbaron@akamai.com;
> ardb@kernel.org; Atish Patra <Atish.Patra@wdc.com>; Anup Patel
> <Anup.Patel@wdc.com>; akpm@linux-foundation.org; rppt@kernel.org;
> mhiramat@kernel.org; zong.li@sifive.com; guoren@linux.alibaba.com;
> wangkefeng.wang@huawei.com; 0x7f454c46@gmail.com;
> chenhuang5@huawei.com; linux-kernel@vger.kernel.org; kernel-
> team@android.com; Palmer Dabbelt <palmerdabbelt@google.com>
> Subject: Re: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POK=
E*
>=20
> On Fri, 30 Apr 2021 05:54:51 +0800
> Changbin Du <changbin.du@gmail.com> wrote:
>=20
> > The problem is that lockdep cannot handle locks across tasks since we
> > use stopmachine to patch code for risc-v. So there's a false positive r=
eport.
> > See privious disscussion here
>=20
> > https://lkml.org/lkml/2021/4/29/63
>=20
> Please use lore.kernel.org, lkml.org is highly unreliable, and is conside=
red
> deprecated for use of referencing linux kernel archives.
>=20
> Would the following patch work?

This patch only takes care of ftrace path.

The RISC-V instruction patching is used by RISC-V jump label implementation
as well and will called from various critical parts of core kernel.

The RAW spinlock approach allows same instruction patching to be used
for kprobes, ftrace, and jump label.

Regards,
Anup

>=20
> (note, I did not even compile test it)
>=20
> -- Steve
>=20
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftr=
ace.h
> index 845002cc2e57..19acbb4aaeff 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -25,6 +25,8 @@ struct dyn_arch_ftrace {  };  #endif
>=20
> +extern int running_ftrace;
> +
>  #ifdef CONFIG_DYNAMIC_FTRACE
>  /*
>   * A general call in RISC-V is a pair of insts:
> diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c inde=
x
> 7f1e5203de88..834ab4fad637 100644
> --- a/arch/riscv/kernel/ftrace.c
> +++ b/arch/riscv/kernel/ftrace.c
> @@ -11,15 +11,19 @@
>  #include <asm/cacheflush.h>
>  #include <asm/patch.h>
>=20
> +int running_ftrace;
> +
>  #ifdef CONFIG_DYNAMIC_FTRACE
>  int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)  {
>  	mutex_lock(&text_mutex);
> +	running_ftrace =3D 1;
>  	return 0;
>  }
>=20
>  int ftrace_arch_code_modify_post_process(void) __releases(&text_mutex)
> {
> +	running_ftrace =3D 0;
>  	mutex_unlock(&text_mutex);
>  	return 0;
>  }
> diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c index
> 0b552873a577..4cd1c79a9689 100644
> --- a/arch/riscv/kernel/patch.c
> +++ b/arch/riscv/kernel/patch.c
> @@ -12,6 +12,7 @@
>  #include <asm/cacheflush.h>
>  #include <asm/fixmap.h>
>  #include <asm/patch.h>
> +#include <asm/ftrace.h>
>=20
>  struct patch_insn {
>  	void *addr;
> @@ -59,8 +60,13 @@ static int patch_insn_write(void *addr, const void
> *insn, size_t len)
>  	 * Before reaching here, it was expected to lock the text_mutex
>  	 * already, so we don't need to give another lock here and could
>  	 * ensure that it was safe between each cores.
> +	 *
> +	 * ftrace uses stop machine, and even though the text_mutex is
> +	 * held, the stop machine task that calls this function will not
> +	 * be the owner.
>  	 */
> -	lockdep_assert_held(&text_mutex);
> +	if (!running_ftrace)
> +		lockdep_assert_held(&text_mutex);
>=20
>  	if (across_pages)
>  		patch_map(addr + len, FIX_TEXT_POKE1);