From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Maydell Subject: Re: [Qemu-devel] [RFC v2 2/6] ARM: KVM: Add support for KVM on ARM architecture Date: Sat, 13 Oct 2012 20:19:53 +0100 Message-ID: References: <1349881659-8403-1-git-send-email-peter.maydell@linaro.org> <1349881659-8403-3-git-send-email-peter.maydell@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: qemu-devel@nongnu.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, patches@linaro.org To: Blue Swirl Return-path: Received: from mail-ie0-f174.google.com ([209.85.223.174]:54708 "EHLO mail-ie0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751426Ab2JMTTx (ORCPT ); Sat, 13 Oct 2012 15:19:53 -0400 Received: by mail-ie0-f174.google.com with SMTP id k13so6229295iea.19 for ; Sat, 13 Oct 2012 12:19:53 -0700 (PDT) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 13 October 2012 10:09, Blue Swirl wrote: > On Wed, Oct 10, 2012 at 3:07 PM, Peter Maydell wrote: >> From: Christoffer Dall >> >> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44934) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TN7Fb-00033N-JI for qemu-devel@nongnu.org; Sat, 13 Oct 2012 15:19:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TN7Fa-0007u5-DJ for qemu-devel@nongnu.org; Sat, 13 Oct 2012 15:19:55 -0400 Received: from mail-ia0-f173.google.com ([209.85.210.173]:62112) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TN7Fa-0007tF-8v for qemu-devel@nongnu.org; Sat, 13 Oct 2012 15:19:54 -0400 Received: by mail-ia0-f173.google.com with SMTP id m10so2779498iam.4 for ; Sat, 13 Oct 2012 12:19:53 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1349881659-8403-1-git-send-email-peter.maydell@linaro.org> <1349881659-8403-3-git-send-email-peter.maydell@linaro.org> Date: Sat, 13 Oct 2012 20:19:53 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [RFC v2 2/6] ARM: KVM: Add support for KVM on ARM architecture List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: patches@linaro.org, qemu-devel@nongnu.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu On 13 October 2012 10:09, Blue Swirl wrote: > On Wed, Oct 10, 2012 at 3:07 PM, Peter Maydell wrote: >> From: Christoffer Dall >> >> 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