"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.
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.
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?
So how about PPC_RAW_FOO() for just the numeric value, without the .long
and stringify.
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.
cheers
