> -----Original Message----- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Monday, July 23, 2012 11:12 PM > To: Bhushan Bharat-R65777 > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org > Subject: Re: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC debug > registers > > On 07/23/2012 05:48 PM, Bhushan Bharat-R65777 wrote: > > > >> -----Original Message----- > >> From: Wood Scott-B07421 > >> Sent: Monday, July 23, 2012 9:12 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; ag...@suse.de; k...@vger.kernel.org; > >> Bhushan Bharat- > >> R65777 > >> Subject: Re: [PATCH v2] booke: Added ONE_REG interface for IAC/DAC > >> debug registers > >> > >> On 07/23/2012 06:19 AM, Bharat Bhushan wrote: > >>> IAC/DAC are defined as 32 bit while they are 64 bit wide. So ONE_REG > >>> interface is added to set/get them. > >>> > >>> Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com> > >>> --- > >>> v2: > >>> - Using copy_to/from_user() apis. > >>> > >>> arch/powerpc/include/asm/kvm.h | 12 ++++++ > >>> arch/powerpc/include/asm/kvm_host.h | 28 ++++++++++++++- > >>> arch/powerpc/kvm/booke.c | 64 > +++++++++++++++++++++++++++++++++- > >>> arch/powerpc/kvm/booke_emulate.c | 8 ++-- > >>> 4 files changed, 104 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/arch/powerpc/include/asm/kvm.h > >>> b/arch/powerpc/include/asm/kvm.h index 1bea4d8..3c14202 100644 > >>> --- a/arch/powerpc/include/asm/kvm.h > >>> +++ b/arch/powerpc/include/asm/kvm.h > >>> @@ -221,6 +221,12 @@ struct kvm_sregs { > >>> > >>> __u32 dbsr; /* KVM_SREGS_E_UPDATE_DBSR */ > >>> __u32 dbcr[3]; > >>> + /* > >>> + * iac/dac registers are 64bit wide, while this API > >>> + * interface provides only lower 32 bits on 64 bit > >>> + * processors. ONE_REG interface is added for 64bit > >>> + * iac/dac registers. > >>> + */ > >>> __u32 iac[4]; > >>> __u32 dac[2]; > >>> __u32 dvc[2]; > >>> @@ -326,5 +332,11 @@ struct kvm_book3e_206_tlb_params { }; > >>> > >>> #define KVM_REG_PPC_HIOR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x1) > >>> +#define KVM_REG_PPC_IAC1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x2) > >>> +#define KVM_REG_PPC_IAC2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x3) > >>> +#define KVM_REG_PPC_IAC3 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x4) > >>> +#define KVM_REG_PPC_IAC4 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x5) > >>> +#define KVM_REG_PPC_DAC1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x6) > >>> +#define KVM_REG_PPC_DAC2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x7) > >>> > >>> #endif /* __LINUX_KVM_POWERPC_H */ diff --git > >>> a/arch/powerpc/include/asm/kvm_host.h > >>> b/arch/powerpc/include/asm/kvm_host.h > >>> index 43cac48..2c0f015 100644 > >>> --- a/arch/powerpc/include/asm/kvm_host.h > >>> +++ b/arch/powerpc/include/asm/kvm_host.h > >>> @@ -342,6 +342,31 @@ struct kvmppc_slb { > >>> bool class : 1; > >>> }; > >>> > >>> +#ifdef CONFIG_BOOKE > >>> +# ifdef CONFIG_PPC_FSL_BOOK3E > >>> +#define KVMPPC_IAC_NUM 2 > >>> +#define KVMPPC_DAC_NUM 2 > >>> +# else > >>> +#define KVMPPC_IAC_NUM 4 > >>> +#define KVMPPC_DAC_NUM 2 > >>> +# endif > >>> +#define KVMPPC_MAX_IAC 4 > >>> +#define KVMPPC_MAX_DAC 2 > >>> +#endif > >>> + > >>> +struct kvmppc_debug_reg { > >>> +#ifdef CONFIG_BOOKE > >>> + u32 dbcr0; > >>> + u32 dbcr1; > >>> + u32 dbcr2; > >>> +#ifdef CONFIG_KVM_E500MC > >>> + u32 dbcr4; > >>> +#endif > >>> + u64 iac[KVMPPC_MAX_IAC]; > >>> + u64 dac[KVMPPC_MAX_DAC]; > >>> +#endif > >>> +}; > >> Is there any reason for this to be a separate struct? > > No specific reason, The rest of debug ( which will follow sometime soon) > > uses > this structure and looks to make code look easy. > > So why not use an fsl / booke specific struct for the debug patches then? > Debug > registers are really nothing common between book3s and booke, so we shouldn't > treat them as such by using the same struct definition. >
All elements of struct are under #ifdef CONFIG_BOOKE? So for book3s it is as good as void, only struct type if declared. Do you want the struct type also under config_booke ? Thanks -Bharat
