From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33107) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9Hlp-0000nS-NM for qemu-devel@nongnu.org; Sat, 04 Jun 2016 16:02:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9Hlm-0000dz-Ge for qemu-devel@nongnu.org; Sat, 04 Jun 2016 16:02:09 -0400 Received: from mail-pa0-x242.google.com ([2607:f8b0:400e:c03::242]:36247) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9Hlm-0000dp-9F for qemu-devel@nongnu.org; Sat, 04 Jun 2016 16:02:06 -0400 Received: by mail-pa0-x242.google.com with SMTP id fg1so8557820pad.3 for ; Sat, 04 Jun 2016 13:02:06 -0700 (PDT) Sender: Richard Henderson References: <1464898022-97990-1-git-send-email-rolnik@amazon.com> <1464898022-97990-2-git-send-email-rolnik@amazon.com> From: Richard Henderson Message-ID: Date: Sat, 4 Jun 2016 13:02:03 -0700 MIME-Version: 1.0 In-Reply-To: <1464898022-97990-2-git-send-email-rolnik@amazon.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 02/10] target-avr: adding AVR CPU features/flavors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Rolnik , qemu-devel@nongnu.org Cc: Michael Rolnik On 06/02/2016 01:06 PM, Michael Rolnik wrote: > Signed-off-by: Michael Rolnik > --- > target-avr/cpu.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > target-avr/cpu.h | 59 ++++++++++ > 2 files changed, 383 insertions(+), 2 deletions(-) > > diff --git a/target-avr/cpu.c b/target-avr/cpu.c > index ff26018..9be0a1d 100644 > --- a/target-avr/cpu.c > +++ b/target-avr/cpu.c > @@ -31,7 +31,7 @@ static void avr_cpu_set_pc( > { > AVRCPU *cpu = AVR_CPU(cs); > > - cpu->env.pc = value / 2; /* internaly PC points to words, not bytes */ > + cpu->env.pc = value / 2; /* internally PC points to words */ > } > > static bool avr_cpu_has_work( > @@ -52,7 +52,7 @@ static void avr_cpu_synchronize_from_tb( > AVRCPU *cpu = AVR_CPU(cs); > CPUAVRState *env = &cpu->env; > > - env->pc = tb->pc / 2; > + env->pc = tb->pc / 2; /* internally PC points to words */ Fold these fixups into the previous patch. > @@ -61,12 +61,14 @@ static void avr_cpu_reset( > AVRCPU *cpu = AVR_CPU(s); > AVRCPUClass *mcc = AVR_CPU_GET_CLASS(cpu); > CPUAVRState *env = &cpu->env; > + uint32_t features = env->features; > > mcc->parent_reset(s); > > memset(env, 0, sizeof(CPUAVRState)); > env->pc = 0; > env->sregI = 1; > + env->features = features; As I said re patch 1, this is fixed by only clearing to before features. > +} > +static void avr_avr6_initfn( Blank line between functions. Many examples. > +static inline int avr_feature( > + CPUAVRState *env, > + int feature) > +{ > + return (env->features & (1UL << feature)) != 0; features is type uint32_t; you don't need UL, just U. > +static inline void avr_del_feature( > + CPUAVRState *env, > + int feature) > +{ > + env->features &= ~(1Ul << feature); > +} When would you ever delete a feature? Seems like this would be forever unused. r~