On Thu, 2020-04-02 at 12:34 +0530, Naveen N. Rao wrote: > Michael Ellerman wrote: > > "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com> writes: > > > Balamuruhan S wrote: > > > > Few ppc instructions are encoded in test_emulate_step.c, consolidate > > > > them to > > > > ppc-opcode.h, fix redefintion errors in bpf_jit caused due to this > > > > consolidation. > > > > Reuse the macros from ppc-opcode.h > > ... > > > > diff --git a/arch/powerpc/net/bpf_jit32.h > > > > b/arch/powerpc/net/bpf_jit32.h > > > > index 4ec2a9f14f84..8a9f16a7262e 100644 > > > > --- a/arch/powerpc/net/bpf_jit32.h > > > > +++ b/arch/powerpc/net/bpf_jit32.h > > > > @@ -76,13 +76,13 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh); > > > > else { PPC_ADDIS(r, base, IMM_HA(i)); > > > > \ > > > > PPC_LBZ(r, r, IMM_L(i)); } } while(0) > > > > > > > > -#define PPC_LD_OFFS(r, base, i) do { if ((i) < 32768) PPC_LD(r, base, > > > > i); \ > > > > +#define _OFFS(r, base, i) do { if ((i) < 32768) EMIT(PPC_ENCODE_LD(r, > > > > base, i)); \ > > > ^^^^^ > > > Should be PPC_LD_OFFS. For the next version, please also build ppc32 and > > > booke codebase to confirm that your changes in those areas are fine. > > > > > > PPC_ENCODE_* also looks quite verbose, so perhaps PPC_ENC_* might be > > > better. Otherwise, this patchset looks good to me and should help reuse > > > some of those macros, especially from the eBPF codebase. > > > > > > Michael, > > > Can you let us know if this looks ok to you? Based on your feedback, we > > > will also update the eBPF codebase. > > > > I didn't really like the first patch which does the mass renaming. It > > creates a huge amount of churn.
sorry for that. > > > > I think I'd be happier if this series just did what it needs, and then > > maybe at the end there's a patch to update all the existing names, which > > I may or may not take. > > Ok. I will work on it. > > > As far as the naming, currently we have: > > > > PPC_INST_FOO - just the opcode > > > > PPC_FOO(x) - macro to encode the opcode with x and (usually) also emit a > > .long and stringify. > > > > And you need an in-between that gives you the full instruction but > > without the .long and stringify, right? > > Yes. > > > So how about PPC_RAW_FOO() for just the numeric value, without the .long > > and stringify. > > Sure, thanks for the feedback -- that makes sense. Thanks for the feedback. > > > We also seem to have a lot of PPC_INST_FOO's that are only ever used in > > the PPC_INST macro. I'm inclined to fold those into the PPC_INST macro, > > to avoid people accidentally using the PPC_INST version when they don't > > mean to. But that's a separate issue. > > Good point -- I do see many uses of PPC_INST_FOO that can be replaced > with PPC_RAW_FOO once we introduce that. We will take a stab at doing > this cleanup as a separate patch at the end. Will make the changes as suggested. -- Bala > > > Thanks, > Naveen >