Re: [Xen-ia64-devel] [patch 03/12] ia64: kexec: Unpin the correct VHPT TR in ia64_do_tlb_purge

[Simon Horman <horms@xxxxxxxxxxxx>]

On Wed, May 07, 2008 at 05:31:21PM +0900, Isaku Yamahata wrote:
> Hi Simon-san.
> I guess that this patch isn't copmiled as inteneded.
> With the VHPT_ENABLED patch which make sure VHPT_ENABLED always
> defined to 1 or 0, I got compilation errors.
> 
> 
> [IA64] compilation fix with VHPT_ENABLED=1
> 
> Enabling VHPT_ENABLED=1 results in compilation error in vmx_phy_mode.c
> and vmx_vcpu.c. This patch fixes them.
> vmx_phy_mode.c: In function 'vmx_load_all_rr':
> vmx_phy_mode.c:175: warning: implicit declaration of function 
> 'vcpu_vhpt_maddr'
> vmx_phy_mode.c:175: error: 'v' undeclared (first use in this function)
> vmx_phy_mode.c:175: error: (Each undeclared identifier is reported only once
> vmx_phy_mode.c:175: error: for each function it appears in.)
> PATCHNAME: compilation_fix_with_vhpt_enabled
> 
> Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>

Thanks Yamahata-san,

that looks correct to me. I have a related fix to the same code which
alters the value that is tracked. Do you think this is correct:


[IA64] Always track vmx vhpt insertions

Argument 2 of vmx_switch_rr7() pins a new value for the vhpt
in the TLB. This argument is supplied as vcpu->arch.vhpt.hash in
the calls to vmx_switch_rr7() in vmx_vcpu_set_rr() and
vmx_load_all_rr(), the only two calls that exist.

This is done independent of whether VHPT_ENABLED is set or not.

Accordingly, update the tracking of the inserted VHPT entry
to use vcpu->arch.vhpt.hash and to do this regardless of
VHPT_ENABLED.

Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>

--- 
This patch applied on top of
"[patch 03/12] ia64: kexec: Unpin the correct VHPT TR in ia64_do_tlb_purge"
which is part of my current EFI memory mapping patch series and
Yamahata-san's fix for that patch.

http://lists.xensource.com/archives/html/xen-ia64-devel/2008-04/msg00121.html
http://lists.xensource.com/archives/html/xen-ia64-devel/2008-05/msg00057.html

Index: xen-unstable.hg/xen/arch/ia64/linux-xen/mca_asm.S
===================================================================
--- xen-unstable.hg.orig/xen/arch/ia64/linux-xen/mca_asm.S      2008-05-08 
08:37:51.000000000 +1000
+++ xen-unstable.hg/xen/arch/ia64/linux-xen/mca_asm.S   2008-05-08 
08:48:16.000000000 +1000
@@ -321,10 +321,13 @@ ia64_do_tlb_purge:
        ;;
 #ifdef XEN
        // 5. VHPT
-#ifdef VHPT_ENABLED
        // GET_VA_VCPU_VHPT_MADDR() may not give the
        // value of the VHPT currently pinned into the TLB
        GET_THIS_PADDR(r2, inserted_vhpt);;
+       ;;
+       cmp.eq p7,p0=r2,r0
+       ;;
+(p7)   br.cond.sptk .vhpt_not_mapped
        dep r16=0,r2,0,IA64_GRANULE_SHIFT
        mov r18=IA64_GRANULE_SHIFT<<2
        ;;
@@ -332,7 +335,7 @@ ia64_do_tlb_purge:
        ;;
        srlz.d
        ;;
-#endif
+.vhpt_not_mapped:
 #endif
        // Now branch away to caller.
        br.sptk.many b1
Index: xen-unstable.hg/xen/arch/ia64/vmx/vmx_phy_mode.c
===================================================================
--- xen-unstable.hg.orig/xen/arch/ia64/vmx/vmx_phy_mode.c       2008-05-08 
08:46:20.000000000 +1000
+++ xen-unstable.hg/xen/arch/ia64/vmx/vmx_phy_mode.c    2008-05-08 
08:52:15.000000000 +1000
@@ -27,7 +27,6 @@
 #include <asm/vmx_phy_mode.h>
 #include <asm/pgtable.h>
 #include <asm/vmmu.h>
-#include <asm/vhpt.h>
 #include <asm/debugger.h>
 
 #define MODE_IND(psr)   \
@@ -172,9 +171,7 @@ vmx_load_all_rr(VCPU *vcpu)
        ia64_dv_serialize_data();
        ia64_set_rr((VRN6 << VRN_SHIFT), vrrtomrr(vcpu, VMX(vcpu, vrr[VRN6])));
        ia64_dv_serialize_data();
-#if VHPT_ENABLED
-        __get_cpu_var(inserted_vhpt) = __va_ul(vcpu_vhpt_maddr(vcpu));
-#endif
+        __get_cpu_var(inserted_vhpt) = vcpu->arch.vhpt.hash;
        vmx_switch_rr7(vrrtomrr(vcpu,VMX(vcpu, vrr[VRN7])),
                       (void *)vcpu->arch.vhpt.hash,
                       pal_vaddr, vcpu->arch.privregs);
Index: xen-unstable.hg/xen/arch/ia64/vmx/vmx_vcpu.c
===================================================================
--- xen-unstable.hg.orig/xen/arch/ia64/vmx/vmx_vcpu.c   2008-05-08 
08:46:20.000000000 +1000
+++ xen-unstable.hg/xen/arch/ia64/vmx/vmx_vcpu.c        2008-05-08 
08:52:30.000000000 +1000
@@ -32,7 +32,6 @@
 #include <asm/delay.h>
 #include <asm/regs.h>
 #include <asm/gcc_intrin.h>
-#include <asm/vhpt.h>
 #include <asm/vmx_mm_def.h>
 #include <asm/vmx.h>
 #include <asm/vmx_phy_mode.h>
@@ -182,9 +181,7 @@ IA64FAULT vmx_vcpu_set_rr(VCPU *vcpu, u6
     switch((u64)(reg>>VRN_SHIFT)) {
     case VRN7:
         if (likely(vcpu == current)) {
-#if VHPT_ENABLED
-            __get_cpu_var(inserted_vhpt) = __va_ul(vcpu_vhpt_maddr(vcpu));
-#endif
+            __get_cpu_var(inserted_vhpt) = vcpu->arch.vhpt.hash;
             vmx_switch_rr7(vrrtomrr(vcpu,val), (void *)vcpu->arch.vhpt.hash,
                            pal_vaddr, vcpu->arch.privregs);
         }
Index: xen-unstable.hg/xen/arch/ia64/xen/regionreg.c
===================================================================
--- xen-unstable.hg.orig/xen/arch/ia64/xen/regionreg.c  2008-05-08 
08:37:51.000000000 +1000
+++ xen-unstable.hg/xen/arch/ia64/xen/regionreg.c       2008-05-08 
08:48:16.000000000 +1000
@@ -48,9 +48,7 @@ extern void ia64_new_rr7(unsigned long r
 static unsigned int domain_rid_bits_default = IA64_MIN_IMPL_RID_BITS;
 integer_param("dom_rid_bits", domain_rid_bits_default); 
 
-#if VHPT_ENABLED
 DEFINE_PER_CPU(unsigned long, inserted_vhpt);
-#endif
 
 #if 0
 // following already defined in include/asm-ia64/gcc_intrin.h

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel