On 13 October 2012 10:09, Blue Swirl <[email protected]> wrote:
> On Wed, Oct 10, 2012 at 3:07 PM, Peter Maydell <[email protected]>
> wrote:
>> From: Christoffer Dall <[email protected]>
>>
>> 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