Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751695AbbEDQQm (ORCPT ); Mon, 4 May 2015 12:16:42 -0400 Received: from ns.iliad.fr ([212.27.33.1]:41546 "EHLO ns.iliad.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751153AbbEDQQc (ORCPT ); Mon, 4 May 2015 12:16:32 -0400 Message-ID: <55479B5E.2070901@freebox.fr> Date: Mon, 04 May 2015 18:16:30 +0200 From: Nicolas Schichan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Russell King - ARM Linux CC: "David S. Miller" , Alexei Starovoitov , Daniel Borkmann , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/4] ARM: net fix emit_udiv() for BPF_ALU | BPF_DIV | BPF_K intruction. References: <1430314657-2552-1-git-send-email-nschichan@freebox.fr> <1430314657-2552-5-git-send-email-nschichan@freebox.fr> <20150501173721.GO12732@n2100.arm.linux.org.uk> In-Reply-To: <20150501173721.GO12732@n2100.arm.linux.org.uk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1940 Lines: 62 On 05/01/2015 07:37 PM, Russell King - ARM Linux wrote: > On Wed, Apr 29, 2015 at 03:37:37PM +0200, Nicolas Schichan wrote: [...] >> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c >> index b5f470d..ffaf311 100644 >> --- a/arch/arm/net/bpf_jit_32.c >> +++ b/arch/arm/net/bpf_jit_32.c >> @@ -449,10 +449,10 @@ static inline void emit_udiv(u8 rd, u8 rm, u8 rn, struct jit_ctx *ctx) >> return; >> } >> #endif >> - if (rm != ARM_R0) >> - emit(ARM_MOV_R(ARM_R0, rm), ctx); >> if (rn != ARM_R1) >> emit(ARM_MOV_R(ARM_R1, rn), ctx); >> + if (rm != ARM_R0) >> + emit(ARM_MOV_R(ARM_R0, rm), ctx); > > I don't think you've thought enough about this. What if rm is ARM_R1? > What if rn = ARM_R0 and rm = ARM_R1? > > How about: > > if (rn == ARM_R0 && rm == ARM_R1) { > emit(ARM_MOV_R(ARM_R3, rn), ctx); // r3 <- r0(rn) > emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- r1(rm) > emit(ARM_MOV_R(ARM_R1, ARM_R3), ctx); // r1 <- r3 > } else if (rn == ARM_R0) { > emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn > if (rm != ARM_R0) > emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm > } else { > if (rm != ARM_R0) > emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm > if (rn != ARM_R1) > emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn > } > Hello Russell, In the current JIT, emit_udiv() is only being called with: - rm = ARM_R4 (r_A) and rn = ARM_R0 (r_scrach) for BPF_ALU | BPF_DIV | BPF_K - rm = ARM_R4 (r_A) and rn = ARM_R5 (r_X) for BPF_ALU | BPF_DIV | BPF_X so it should not cause any issue in the current code state. But yes, I'll rework the patch to avoid any other nasty surprises should the code change. Thanks, -- Nicolas Schichan Freebox SAS -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/