Hi Xiantao, More comments. Zhang, Xiantao wrote:
From 696b9eea9f5001a7b7a07c0e58514aa10306b91a Mon Sep 17 00:00:00 2001From: Xiantao Zhang <xiantao.zhang@xxxxxxxxx> Date: Fri, 28 Mar 2008 09:51:36 +0800 Subject: [PATCH] KVM:IA64 : Add head files for kvm/ia64 ia64_regs: some defintions for special registers which aren't defined in asm-ia64/ia64regs.
Please put missing definitions of registers into asm-ia64/ia64regs.h if they are official definitions from the spec.
kvm_minstate.h : Marcos about Min save routines. lapic.h: apic structure definition. vcpu.h : routions related to vcpu virtualization. vti.h : Some macros or routines for VT support on Itanium. Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
+/*
+ * Flushrs instruction stream.
+ */
+#define ia64_flushrs() asm volatile ("flushrs;;":::"memory")
+
+#define ia64_loadrs() asm volatile ("loadrs;;":::"memory")
Please put these into include/asm-ia64/gcc_intrin.h
+#define ia64_get_rsc()
\
+({
\
+ unsigned long val;
\
+ asm volatile ("mov %0=ar.rsc;;" : "=r"(val) :: "memory");
\
+ val;
\
+})
+
+#define ia64_set_rsc(val) \
+ asm volatile ("mov ar.rsc=%0;;" :: "r"(val) : "memory")
Please update the ia64_get/set_reg macros to handle the RSC register and use those macros.
+#define ia64_get_bspstore()
\
+({
\
+ unsigned long val;
\
+ asm volatile ("mov %0=ar.bspstore;;" : "=r"(val) :: "memory");
\
+ val;
\
+})
Ditto for for AR.BSPSTORE
+#define ia64_get_rnat()
\
+({
\
+ unsigned long val;
\
+ asm volatile ("mov %0=ar.rnat;" : "=r"(val) :: "memory");
\
+ val;
\
+})
Ditto for AR.RNAT
+static inline unsigned long ia64_get_itc(void)
+{
+ unsigned long result;
+ result = ia64_getreg(_IA64_REG_AR_ITC);
+ return result;
+}
This exists in include/asm-ia64/delay.h
+static inline void ia64_set_dcr(unsigned long dcr)
+{
+ ia64_setreg(_IA64_REG_CR_DCR, dcr);
+}
Please just call ia64_setreg() in your code rather than defining a wrapper for it.
+#define ia64_ttag(addr)
\
+({
\
+ __u64 ia64_intri_res;
\
+ asm volatile ("ttag %0=%1" : "=r"(ia64_intri_res) : "r" (addr));
\
+ ia64_intri_res;
\
+})
Please add to include/asm-ia64/gcc_intrin.h instead.
diff --git a/arch/ia64/kvm/lapic.h b/arch/ia64/kvm/lapic.h new file mode 100644 index 0000000..152cbdc --- /dev/null +++ b/arch/ia64/kvm/lapic.h @@ -0,0 +1,27 @@ +#ifndef __KVM_IA64_LAPIC_H +#define __KVM_IA64_LAPIC_H + +#include "iodev.h"
I don't understand why iodev.h is included here?
--- /dev/null +++ b/arch/ia64/kvm/vcpu.h
The formatting of this file is dodgy, please try and make it comply with the Linux standards in Documentation/CodingStyle
+#define _vmm_raw_spin_lock(x) \
[snip]
+ +#define _vmm_raw_spin_unlock(x) \
Could you explain the reasoning behind these two macros? Whenever I see open coded spin lock modifications like these, I have to admit I get a bit worried.
+typedef struct kvm_vcpu VCPU;
+typedef struct kvm_pt_regs REGS;
+typedef enum { DATA_REF, NA_REF, INST_REF, RSE_REF } vhpt_ref_t;
+typedef enum { INSTRUCTION, DATA, REGISTER } miss_type;
ARGH! Please see previous mail about typedefs! I suspect this is code inherited from Xen ? Xen has a lot of really nasty and pointless typedefs like these :-(
+static inline void vcpu_set_dbr(VCPU *vcpu, u64 reg, u64 val)
+{
+ /* TODO: need to virtualize */
+ __ia64_set_dbr(reg, val);
+}
+
+static inline void vcpu_set_ibr(VCPU *vcpu, u64 reg, u64 val)
+{
+ /* TODO: need to virtualize */
+ ia64_set_ibr(reg, val);
+}
+
+static inline u64 vcpu_get_dbr(VCPU *vcpu, u64 reg)
+{
+ /* TODO: need to virtualize */
+ return ((u64)__ia64_get_dbr(reg));
+}
+
+static inline u64 vcpu_get_ibr(VCPU *vcpu, u64 reg)
+{
+ /* TODO: need to virtualize */
+ return ((u64)ia64_get_ibr(reg));
+}
More wrapper macros that really should just use ia64_get/set_reg() directly in the code.
diff --git a/arch/ia64/kvm/vti.h b/arch/ia64/kvm/vti.h new file mode 100644 index 0000000..591ab22
[ship]
+/* -*- Mode:C; c-basic-offset:4; tab-width:4; indent-tabs-mode:nil -*- */
Evil formatting again! Cheers, Jes -- To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
|