From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55987) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VhNpF-0007bt-Tu for qemu-devel@nongnu.org; Fri, 15 Nov 2013 13:09:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VhNp3-0003ZE-Un for qemu-devel@nongnu.org; Fri, 15 Nov 2013 13:09:01 -0500 Message-ID: <5286632C.7040900@suse.de> Date: Fri, 15 Nov 2013 19:08:44 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1384492469-11881-1-git-send-email-aik@ozlabs.ru> <52863995.2070007@suse.de> <5286538E.3050807@ozlabs.ru> In-Reply-To: <5286538E.3050807@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4] ppc: introduce CPUPPCState::cpu_dt_id and CPUState::kvm_cpu_id List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: Alexander Graf , Bharat Bhushan , qemu-ppc@nongnu.org, Paolo Bonzini , Scott Wood , Paul Mackerras Am 15.11.2013 18:02, schrieb Alexey Kardashevskiy: > On 16.11.2013 2:11, Andreas F=C3=A4rber wrote: >> Am 15.11.2013 06:14, schrieb Alexey Kardashevskiy: >>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h >>> index 7739e00..52fc76d 100644 >>> --- a/include/qom/cpu.h >>> +++ b/include/qom/cpu.h >>> @@ -197,6 +197,7 @@ struct CPUState { >>> bool kvm_vcpu_dirty; >>> struct KVMState *kvm_state; >>> struct kvm_run *kvm_run; >>> + int kvm_cpu_id; >>> =20 >>> /* TODO Move common fields from CPUArchState here. */ >>> int cpu_index; /* used by alpha TCG */ >> >> Here you are adding a field to CPUState, fine with me. (Please add a >> documentation line above the struct then.) >> >>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h >>> index bb84767..8052f6b 100644 >>> --- a/target-ppc/cpu.h >>> +++ b/target-ppc/cpu.h >>> @@ -1074,6 +1074,9 @@ struct CPUPPCState { >>> */ >>> uint8_t fit_period[4]; >>> uint8_t wdt_period[4]; >>> + >>> + /* The CPU index used in the device tree. KVM uses this index to= o */ >>> + int cpu_dt_id; >> >> But I believe I have requested a number of times not to add random >> fields to CPUPPCState unless they are accessed by TCG. Please place th= e >> new field in PowerPCCPU instead and put the description into the struc= t >> documentation. >=20 >=20 > It is accessed by xics.c which is used in TCG. Or I misinterpret the > rule, do not I? Yes, you do. ;) The question is, is the field accessed using offsetof(CPUPPCState, x) such as in target-ppc/translate.c (search for TCG_AREG0). That's what CPUPPCState is still needed for - mainly GPRs and the TLBs that TCG ops load from / store to using host instructions with immediate offset. Whether it is used in a HELPER() / helper_* function called from TCG (target-ppc/*helper.c) has become less relevant since the optimization of ppc_env_get_cpu() macro. Neither of that seem to be the case here, unless I missed something. I still hope we can one day get rid of CPUPPCState by having only numeric-offset accesses from cpu+sizeof(CPUState) and all fields (including TLB, as Paolo recently pointed out) directly in PowerPCCPU. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg