Re: [Qemu-devel] [PATCH v2 3/7] AARCH64: Add aarch64 CPU initialization, get and put registers support

[Peter Maydell]

On 9 August 2013 20:03, Mian M. Hamayun
<address@hidden> wrote:
> On 09/08/2013 15:24, Peter Maydell wrote:
>> This set of #ifdefs is pretty messy. I suggest splitting kvm.c
>> into three:
>>    target-arm/kvm.c   -- anything common to both 32 and 64 bit
>>    target-arm/kvm32.c -- non-TARGET_AARCH64 functions
>>    target-arm/kvm64.c -- TARGET_AARCH64 functions
>>
>> and have target-arm/Makefile.objs build only one of kvm32.c
>> or kvm64.c depending on whether TARGET_AARCH64 is set.
>
> My initial intuition was to separate 32 and 64 bit versions, but as
> many functions are common, so I decided to use #ifdefs instead.

That's why I propose having a common-functions file.
Big "disable whole set of functions" ifdefs make it
harder to read a source file.

> btw, I have a similar situation in hw/arm/boot.c as well.

Yeah, I'm not entirely happy with the boot code either, but I didn't
have any concrete suggestions for improvement there.

>> We definitely don't want to do this -- see my notes on
>> '-cpu host' in another email thread.  (We should make sure
>> we coordinate who of you or Linaro is going to do the
>> -cpu host support, incidentally.)
>
> I tend to agree with this proposition as well. But the point that I don't
> understand is how something is acceptable in kvmtool and not in the
> qemu.

Different projects, different rules. In particular, kvmtool aims
to be a good medium for quick development and testing of new
features, whereas QEMU is trying to be the production-ready
backwards-compatible stable solution. That means that it's
often going to be possible to put code into kvmtool as an
interim "this works and we'll revisit it later" approach but
not really be able to do the same in QEMU.

> If this implementation lets us use the 64-bit architecture in the
> current state of affairs, then why not use it. We can always replace
> this particular part, when the -cpu host support becomes available,
> right ?

Also, the way that '-cpu host' support becomes available
is that we insist that it gets written in order for 64-bit
support to land. It's not really all that complicated.

-- PMM