Re: [RFC PATCH 3/4] powerpc ppc-opcode: move ppc instuction encoding from test_emulate_step

["Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>]

Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@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.

Ok.

> 
> 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.

> 
> 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.


Thanks,
Naveen