Re: [Qemu-devel] [RFC v2 2/6] ARM: KVM: Add support for KVM on ARM architecture

[Peter Maydell]

On 13 October 2012 10:09, Blue Swirl <address@hidden> wrote:
> On Wed, Oct 10, 2012 at 3:07 PM, Peter Maydell <address@hidden> wrote:
>> From: Christoffer Dall <address@hidden>
>>
>> Add basic support for KVM on ARM architecture.
>> +#include "device_tree.h"
>
> Is this used?

Don't think so, will remove.

>> +#include "hw/arm-misc.h"
>> +
>> +const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>
> 'static'. In fact, 'static' not used at all in this file, probably it
> could be used a lot more.

Agreed.

>> +struct reg {
>
> Reg or other CamelCase version and a typedef, please.

OK.

>> +    env->cp15.c2_mask = ~(((uint32_t)0xffffffffu) >> env->cp15.c2_control);
>> +    env->cp15.c2_base_mask = ~((uint32_t)0x3fffu >> env->cp15.c2_control);
>
> The casts don't look useful.

This is just a copy of the equivalent lines in target-arm/helper.c's
vmsa_ttbcr_write() function, which also have the casts... I agree
they don't look like they're actually doing anything useful though.

>> +void kvm_arch_update_guest_debug(CPUARMState *env, struct kvm_guest_debug 
>> *dbg)
>> +{
>> +    fprintf(stderr, "%s: not implemented\n", __func__);
>
> Please use qemu_log_mask(LOG_UNIMP, ...) instead.

Happy to -- hadn't noticed that had made it in. (There are a bunch
of similar printfs in various bits of ARM code I should probably
update to use that...)

-- PMM