On 07/24/2012 03:04 AM, Bhushan Bharat-R65777 wrote:
-----Original Message-----
From: Alexander Graf [mailto:agraf@xxxxxxx]
Sent: Monday, July 23, 2012 11:12 PM
To: Bhushan Bharat-R65777
Cc: Wood Scott-B07421; kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxx; agraf@xxxxxxx; kvm@xxxxxxxxxxxxxxx;
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.bhushan@xxxxxxxxxxxxx>
---
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 ?
struct kvmppc_booke_debug_reg {
<lots of defines>
};
struct kvmppc_book3s_debug_reg {
<lots of other defines>
};
void booke_foo() {
struct kvmppc_booke_debug_reg r;
r.dbcr0 = 0;
}
vs
struct kvmppc_debug_reg {
#ifdef CONFIG_BOOKE
<lots of defines>
#else
<lots of other defines>
#endif
};
void booke_foo() {
struct kvmppc_debug_reg r;
r.dbcr0 = 0;
}
Which one has less #ifdefs? Keep in mind that the less #ifdefs you have,
the more readable and maintainable your code becomes, because config
options have less effect on your syntax.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
