Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp3311863pxj; Mon, 14 Jun 2021 20:47:21 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxi9mcV5ayAcoasHGW7hCX2X21QeEIHO0W2H0RZKG6NFC+y0Z7gdsY60UGKivNIVR+Be+bR X-Received: by 2002:a17:906:7946:: with SMTP id l6mr7459816ejo.50.1623728841062; Mon, 14 Jun 2021 20:47:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1623728841; cv=none; d=google.com; s=arc-20160816; b=RIcKAU5+UYlyAuhV1xtgfyBnvLpQTv5sDYZO31fTRuOq7j8psEDDJvD2SsIe3t5MxJ 3VfNBqWENMiwOxl/1WWRA11zSDHDjYcHl21J0wU3FzDHqTZOiOo3fA+pfDGl/IiHC2gN zXx9ImxM3wo85ppeKXrZEv8etdH5FgZ5zmGhWznxTsby2vzeCvX2nkBjulP67cHvbd5Q XbOucvB1S1WxZq2sy79cnw+9tqwRKq5GZexhrRfY9ilDMUCcgIWHNmhCi6cA9eyS/LPU dcOvdocFB50tPopONkVMVa3s8t3mzs0CIdARCln2INz+pf89D23eJC8SfuNTepn80j6p Q3pA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=HWe2X0y0YUq0l4++qPnBz+nFUQIa1PU/QJq3rGd+Lnw=; b=VAj9kQppUipWs28wlRPp5bZhb3z+cXgq/jPmQwTFEreRgeclSxyKUg9KYgrGguCicU CEBcbYtmnE/KKNsWS2FhulRyFLdC/0HRIwonFBtLFe9THXqBQjALURvRZ/KACmVXW3x4 im0VBq/nVuxMmxdjW6M9VRJKnlTde/ObiixDLyaRO9+ZoTyUNVTkIZ8KVWeq9InbEypj SlOtgiwnCj9CFlJpl67vfyd7aFRh3MVFnCWLLore3J/9QMAo2DeMBcTjw1L/Gjaux3N0 /MuhrW73/zE0j9HixhKqCylAwXU0oJ0s3o1VxSq2sZ4pVIZr96t05Jh0mhdX1eviqpGY LUWw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=m0x9I947; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id eb13si3923523edb.245.2021.06.14.20.46.58; Mon, 14 Jun 2021 20:47:21 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=m0x9I947; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230179AbhFODpN (ORCPT + 99 others); Mon, 14 Jun 2021 23:45:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33554 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229659AbhFODpM (ORCPT ); Mon, 14 Jun 2021 23:45:12 -0400 Received: from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com [IPv6:2607:f8b0:4864:20::b29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A4F8C061574 for ; Mon, 14 Jun 2021 20:43:08 -0700 (PDT) Received: by mail-yb1-xb29.google.com with SMTP id p15so3303205ybe.6 for ; Mon, 14 Jun 2021 20:43:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=HWe2X0y0YUq0l4++qPnBz+nFUQIa1PU/QJq3rGd+Lnw=; b=m0x9I947UdRTN7gNTRJs0p0OmI0GR2AxRLR8J5RUVvjDtCzhqjsT9QpJIzvbRLH7Rl hG7XtWL2iyGBG/Ichpqx/05zZE4WmzNsh3iciij+L4w/w3j0yzqBmHmFhzOIoPmmDD7l ObgTfU/Q5MUP5woqy8TA47hpVNdNkY5eb7efUOjUIpQq/4g9tk7i69kiCDW3B3e7rdL8 jhRYGy9zFuw7EzyLuxrNsAoW0FMIe7cT4anWCn+sa08tKYZe+J0XtlI94VefbyqXeG72 PZF5Wi4Jw6DoMemUTKF96zyeXDoPnBpBkvpfyif/aLO8r3JBz7PQ6ES4qT/wk2ClQMsz NWVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=HWe2X0y0YUq0l4++qPnBz+nFUQIa1PU/QJq3rGd+Lnw=; b=GpGJaSJIVBs6GtfdvEaFKjMRNcK3kyKFSLZLcNtzJ5aoFEbl6uWIRWrJS6tz5478KS AhK13dudycHt2BBsNiJLrSzg3mpX8tQTMKqbb09gHhOYrU1wuYUxawbEKlfyT4d18RCL 3ouKMah2DssG3/iQBvvhvIxZhiGCJbBF7SPWMzeOA7RBLMALUqD8huQvKdfqadRw9hbQ tg0eMMU6H3KvfYxUQVUxrJItp+rq6Jyva2UjDN2+0kkff9w9ff+Lo6ZBu+7io/jXrxyy HT9poqYb5EX9YZct9kfxXPK9gWoVxfNze8pUN3gR74+j947rvUWifgcOsKLjD0dI5Ffu aiJA== X-Gm-Message-State: AOAM530FzSOJKolbER8ebJmfG3OLJlPW8nlZXQrmg0EwLowU0inSMT4O P255D2hqCPopDqNVbEsl7UqV/GXtIK0DI8VlIE4= X-Received: by 2002:a25:3103:: with SMTP id x3mr29191194ybx.8.1623728587324; Mon, 14 Jun 2021 20:43:07 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Jordan Niethe Date: Tue, 15 Jun 2021 13:42:56 +1000 Message-ID: Subject: Re: [PATCH v2 07/12] powerpc/lib/code-patching: Don't use struct 'ppc_inst' for runnable code in tests. To: Christophe Leroy Cc: Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , "Naveen N. Rao" , linux-kernel@vger.kernel.org, linuxppc-dev Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 20, 2021 at 11:50 PM Christophe Leroy wrote: > > 'struct ppc_inst' is meant to represent an instruction internally, it > is not meant to dereference code in memory. > > For testing code patching, use patch_instruction() to properly > write into memory the code to be tested. > > Signed-off-by: Christophe Leroy > --- > arch/powerpc/lib/code-patching.c | 95 ++++++++++++++++++-------------- > 1 file changed, 53 insertions(+), 42 deletions(-) > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c > index 82f2c1edb498..508e9511ca96 100644 > --- a/arch/powerpc/lib/code-patching.c > +++ b/arch/powerpc/lib/code-patching.c > @@ -422,9 +422,9 @@ static void __init test_branch_iform(void) > { > int err; > struct ppc_inst instr; > - unsigned long addr; > - > - addr = (unsigned long)&instr; > + unsigned int tmp[2]; > + struct ppc_inst *iptr = (struct ppc_inst *)tmp; > + unsigned long addr = (unsigned long)tmp; > > /* The simplest case, branch to self, no flags */ > check(instr_is_branch_iform(ppc_inst(0x48000000))); > @@ -445,52 +445,57 @@ static void __init test_branch_iform(void) > check(!instr_is_branch_iform(ppc_inst(0x7bfffffd))); > > /* Absolute branch to 0x100 */ > - instr = ppc_inst(0x48000103); > - check(instr_is_branch_to_addr(&instr, 0x100)); > + patch_instruction(iptr, ppc_inst(0x48000103)); > + check(instr_is_branch_to_addr(iptr, 0x100)); > /* Absolute branch to 0x420fc */ > - instr = ppc_inst(0x480420ff); > - check(instr_is_branch_to_addr(&instr, 0x420fc)); > + patch_instruction(iptr, ppc_inst(0x480420ff)); > + check(instr_is_branch_to_addr(iptr, 0x420fc)); > /* Maximum positive relative branch, + 20MB - 4B */ > - instr = ppc_inst(0x49fffffc); > - check(instr_is_branch_to_addr(&instr, addr + 0x1FFFFFC)); > + patch_instruction(iptr, ppc_inst(0x49fffffc)); > + check(instr_is_branch_to_addr(iptr, addr + 0x1FFFFFC)); > /* Smallest negative relative branch, - 4B */ > - instr = ppc_inst(0x4bfffffc); > - check(instr_is_branch_to_addr(&instr, addr - 4)); > + patch_instruction(iptr, ppc_inst(0x4bfffffc)); > + check(instr_is_branch_to_addr(iptr, addr - 4)); > /* Largest negative relative branch, - 32 MB */ > - instr = ppc_inst(0x4a000000); > - check(instr_is_branch_to_addr(&instr, addr - 0x2000000)); > + patch_instruction(iptr, ppc_inst(0x4a000000)); > + check(instr_is_branch_to_addr(iptr, addr - 0x2000000)); > > /* Branch to self, with link */ > - err = create_branch(&instr, &instr, addr, BRANCH_SET_LINK); > - check(instr_is_branch_to_addr(&instr, addr)); > + err = create_branch(&instr, iptr, addr, BRANCH_SET_LINK); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr)); > > /* Branch to self - 0x100, with link */ > - err = create_branch(&instr, &instr, addr - 0x100, BRANCH_SET_LINK); > - check(instr_is_branch_to_addr(&instr, addr - 0x100)); > + err = create_branch(&instr, iptr, addr - 0x100, BRANCH_SET_LINK); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr - 0x100)); > > /* Branch to self + 0x100, no link */ > - err = create_branch(&instr, &instr, addr + 0x100, 0); > - check(instr_is_branch_to_addr(&instr, addr + 0x100)); > + err = create_branch(&instr, iptr, addr + 0x100, 0); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr + 0x100)); > > /* Maximum relative negative offset, - 32 MB */ > - err = create_branch(&instr, &instr, addr - 0x2000000, BRANCH_SET_LINK); > - check(instr_is_branch_to_addr(&instr, addr - 0x2000000)); > + err = create_branch(&instr, iptr, addr - 0x2000000, BRANCH_SET_LINK); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr - 0x2000000)); > > /* Out of range relative negative offset, - 32 MB + 4*/ > - err = create_branch(&instr, &instr, addr - 0x2000004, BRANCH_SET_LINK); > + err = create_branch(&instr, iptr, addr - 0x2000004, BRANCH_SET_LINK); > check(err); > > /* Out of range relative positive offset, + 32 MB */ > - err = create_branch(&instr, &instr, addr + 0x2000000, BRANCH_SET_LINK); > + err = create_branch(&instr, iptr, addr + 0x2000000, BRANCH_SET_LINK); > check(err); > > /* Unaligned target */ > - err = create_branch(&instr, &instr, addr + 3, BRANCH_SET_LINK); > + err = create_branch(&instr, iptr, addr + 3, BRANCH_SET_LINK); > check(err); > > /* Check flags are masked correctly */ > - err = create_branch(&instr, &instr, addr, 0xFFFFFFFC); > - check(instr_is_branch_to_addr(&instr, addr)); > + err = create_branch(&instr, iptr, addr, 0xFFFFFFFC); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr)); > check(ppc_inst_equal(instr, ppc_inst(0x48000000))); > } > > @@ -513,9 +518,10 @@ static void __init test_branch_bform(void) > int err; > unsigned long addr; > struct ppc_inst *iptr, instr; > + unsigned int tmp[2]; > unsigned int flags; > > - iptr = &instr; > + iptr = (struct ppc_inst *)tmp; > addr = (unsigned long)iptr; > > /* The simplest case, branch to self, no flags */ > @@ -528,39 +534,43 @@ static void __init test_branch_bform(void) > check(!instr_is_branch_bform(ppc_inst(0x7bffffff))); > > /* Absolute conditional branch to 0x100 */ > - instr = ppc_inst(0x43ff0103); > - check(instr_is_branch_to_addr(&instr, 0x100)); > + patch_instruction(iptr, ppc_inst(0x43ff0103)); > + check(instr_is_branch_to_addr(iptr, 0x100)); > /* Absolute conditional branch to 0x20fc */ > - instr = ppc_inst(0x43ff20ff); > - check(instr_is_branch_to_addr(&instr, 0x20fc)); > + patch_instruction(iptr, ppc_inst(0x43ff20ff)); > + check(instr_is_branch_to_addr(iptr, 0x20fc)); > /* Maximum positive relative conditional branch, + 32 KB - 4B */ > - instr = ppc_inst(0x43ff7ffc); > - check(instr_is_branch_to_addr(&instr, addr + 0x7FFC)); > + patch_instruction(iptr, ppc_inst(0x43ff7ffc)); > + check(instr_is_branch_to_addr(iptr, addr + 0x7FFC)); > /* Smallest negative relative conditional branch, - 4B */ > - instr = ppc_inst(0x43fffffc); > - check(instr_is_branch_to_addr(&instr, addr - 4)); > + patch_instruction(iptr, ppc_inst(0x43fffffc)); > + check(instr_is_branch_to_addr(iptr, addr - 4)); > /* Largest negative relative conditional branch, - 32 KB */ > - instr = ppc_inst(0x43ff8000); > - check(instr_is_branch_to_addr(&instr, addr - 0x8000)); > + patch_instruction(iptr, ppc_inst(0x43ff8000)); > + check(instr_is_branch_to_addr(iptr, addr - 0x8000)); > > /* All condition code bits set & link */ > flags = 0x3ff000 | BRANCH_SET_LINK; > > /* Branch to self */ > err = create_cond_branch(&instr, iptr, addr, flags); > - check(instr_is_branch_to_addr(&instr, addr)); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr)); > > /* Branch to self - 0x100 */ > err = create_cond_branch(&instr, iptr, addr - 0x100, flags); > - check(instr_is_branch_to_addr(&instr, addr - 0x100)); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr - 0x100)); > > /* Branch to self + 0x100 */ > err = create_cond_branch(&instr, iptr, addr + 0x100, flags); > - check(instr_is_branch_to_addr(&instr, addr + 0x100)); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr + 0x100)); > > /* Maximum relative negative offset, - 32 KB */ > err = create_cond_branch(&instr, iptr, addr - 0x8000, flags); > - check(instr_is_branch_to_addr(&instr, addr - 0x8000)); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr - 0x8000)); > > /* Out of range relative negative offset, - 32 KB + 4*/ > err = create_cond_branch(&instr, iptr, addr - 0x8004, flags); > @@ -576,7 +586,8 @@ static void __init test_branch_bform(void) > > /* Check flags are masked correctly */ > err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC); > - check(instr_is_branch_to_addr(&instr, addr)); > + patch_instruction(iptr, instr); > + check(instr_is_branch_to_addr(iptr, addr)); > check(ppc_inst_equal(instr, ppc_inst(0x43FF0000))); > } > > -- > 2.25.0 > Reviewed by: Jordan Niethe