Re: [Qemu-devel] [PATCH] target-arm: Split cp helper API to new C file

[Peter Crosthwaite]

On Mon, Jun 15, 2015 at 12:34 AM, Peter Maydell
<address@hidden> wrote:
> On 15 June 2015 at 03:21, Peter Crosthwaite <address@hidden> wrote:
>> Move the ARM coprocessor API to a new C file. helper.c is huge and
>> splitting off this self contained piece increases modularity.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>> I also need this for multi-arch where, this file needs to remain obj-y
>> while the others in target-arm are converted to arch-obj-y. This is
>> because these are the only APIs that are directly callable from system
>> level code.
>> ---
>>  dtc                      |   2 +-
>>  target-arm/Makefile.objs |   1 +
>>  target-arm/cp.c          | 330 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  target-arm/helper.c      | 326 
>> ----------------------------------------------
>>  4 files changed, 332 insertions(+), 327 deletions(-)
>>  create mode 100644 target-arm/cp.c
>
> I've wondered before about splitting this out, but we haven't
> had a good enough reason to justify the inevitable breaking
> of patches in flight before.
>
> I don't understand where your dividing line is between
> code that's been moved and code that hasn't, though: why
> do we only need to move the registration functions, but not
> the code that calls those registration functions, and not the
> actual data we register or the functions those cpreg structures
> define to be called from translated code (except for the
> read-zero/write-ignore functions, which you have moved).
> Can you clarify?
>

My criteria is APIs that are called outside of target-arm/:

address@hidden arm]$ git grep cp_reg .
pxa2xx.c:370:    define_arm_cp_regs_with_opaque(s->cpu, pxa_cp_reginfo, s);
pxa2xx_pic.c:290:    define_arm_cp_regs_with_opaque(cpu, pxa_pic_cp_reginfo, s);

The goal is to split of universally accessible APIs from those private
to target-arm. Note arm_cp_read_zero is refd by PXA cp reg definitions
leading to that anomaly on the RAZWI functions.

After this patch nothing outside target-arm/ calls into helper.c.

Splitting just the registration-core might be a good start, as
in-flight patches to that are rare. With the core migrated the
different cpreg defs can be migrated gradually if we want to do your
longer list of movements.

>> diff --git a/dtc b/dtc
>> index 65cc4d2..bc895d6 160000
>> --- a/dtc
>> +++ b/dtc
>> @@ -1 +1 @@
>> -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
>> +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
>
> Stray submodule change.
>

Doh.

Regards,
Peter

> thanks
> -- PMM
>