[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-ia64-devel] [patch 07/14] Kexec: Fix ia64_do_tlb_purge so that it works with XEN



Hi Simon,

I have some comments.

> 1. Make all calls to GET_THIS_PADDR before purging the DTR for PERCPU.
>    This is because the kernel registers are saved in per_cpu data to allow
>    the real kernel registers to be used by domains.

I did not notice that ia64_do_tlb_purge is called from ia64_jump_to_sal.
SET_PER_CPU_DATA() is called in ia64_os_mca_dispatch that calls
ia64_do_tlb_purge, because SET_PER_CPU_DATA() has to be called once.

> 2. Wrap the purging of the DTR for PERCPU in #ifdef XEN as Linux
>    doesn't seem to do this (any more?)

The following lines are found in Linux tree.

arch/ia64/kernel/mca_asm.S:
104             // 2. Purge DTR for PERCPU data.
105             movl r16=PERCPU_ADDR
106             mov r18=PERCPU_PAGE_SHIFT<<2
107             ;;
108             ptr.d r16,r18
109             ;;
110             srlz.d
111             ;;

Isn't it such a meaning?

> 4. Fix VHPT purging code to treat what is at the IA64_DOMAIN_FLAGS_OFFSET
>    offset as a litteral value (flag) rather than a pointer to a literal.

Suppose that,

struct domain *dom              = 0xf000000007ccc080;
IA64_DOMAIN_FLAGS_OFFSET        = 0x688

   And I show how the value of r3 becomes since each instruction is executed.

@@ -289,25 +304,24 @@ ia64_do_tlb_purge:
        ld8 r3=[r3];;      // r3=0xf000000007ccc080 virtual address of dom
        dep r3=0,r3,60,4;; // r3=0x0000000007ccc080 physical address of dom
        add r3=IA64_DOMAIN_FLAGS_OFFSET,r3;; // r3=0x0000000007ccc708 physical 
address of dom->arch.flags
        ld8 r3=[r3];;      // r3=2       load the value of domain->arch.flags

So 'r3=[r3]' is necessary.


Thanks,
KAZ


From: Simon Horman <horms@xxxxxxxxxxxx>
Subject: [Xen-ia64-devel] [patch 07/14] Kexec: Fix ia64_do_tlb_purge so that it 
works with XEN
Date: Wed, 12 Sep 2007 17:08:52 +0900

> Fix ia64_do_tlb_purge, its broken in too many ways
> 
> 1. Make all calls to GET_THIS_PADDR before purging the DTR for PERCPU.
>    This is because the kernel registers are saved in per_cpu data to allow
>    the real kernel registers to be used by domains.
> 
> 2. Wrap the purging of the DTR for PERCPU in #ifdef XEN as Linux
>    doesn't seem to do this (any more?)
> 
> 3. Use the per_cpu variable to derive CURRENT_STACK_OFFSET rather
>    than reading it from a kernel register. See 1) for explanation
>    of why.
> 
> 4. Fix VHPT purging code to treat what is at the IA64_DOMAIN_FLAGS_OFFSET
>    offset as a litteral value (flag) rather than a pointer to a literal.
> 
> 5. In the VHPT pruning code, don't use r25 as ia64_jump_to_sal,
>    which branches to ia64_do_tlb_purge expects r25 to be preserved.
>    There seems no reason not to use r2 as per the other purges
>    done in ia64_do_tlb_purge.  Furthermore use r16 and r18 instead
>    of r20 and r24 for consistency reasons.
> 
> 6. Move __va_ul(vcpu_vhpt_maddr(v)) comment outside of
>    #if VHPT_ENABLED as it also applies to code further down that
>    is outside the #if
> 
> 7. There is no 7th change
> 
> Cc: Tristan Gingold <tgingold@xxxxxxx>,
> Cc: Yutaka Ezaki <yutaka.ezaki@xxxxxxxxxxxxxx>,
> Cc: Masaki Kanno <kanno.masaki@xxxxxxxxxxxxxx>,
> Cc: Kazuhiro Suzuki <kaz@xxxxxxxxxxxxxx>,
> Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
> 
> Index: xen-unstable.hg/xen/arch/ia64/linux-xen/mca_asm.S
> ===================================================================
> --- xen-unstable.hg.orig/xen/arch/ia64/linux-xen/mca_asm.S    2007-07-11 
> 12:05:33.000000000 +0900
> +++ xen-unstable.hg/xen/arch/ia64/linux-xen/mca_asm.S 2007-07-11 
> 13:13:56.000000000 +0900
> @@ -195,6 +195,10 @@
>   */
>  
>  ia64_do_tlb_purge:
> +#ifdef XEN
> +     // This needs to be called in order for GET_THIS_PADDR to work
> +     SET_PER_CPU_DATA();;
> +#endif
>  #define O(member)    IA64_CPUINFO_##member##_OFFSET
>  
>       GET_THIS_PADDR(r2, cpu_info)    // load phys addr of cpu_info into r2
> @@ -244,6 +248,7 @@ ia64_do_tlb_purge:
>       ;;
>       srlz.d
>       ;;
> +#ifdef XEN
>       // 2. Purge DTR for PERCPU data.
>       movl r16=PERCPU_ADDR
>       mov r18=PERCPU_PAGE_SHIFT<<2
> @@ -252,6 +257,7 @@ ia64_do_tlb_purge:
>       ;;
>       srlz.d
>       ;;
> +#endif
>       // 3. Purge ITR for PAL code.
>       GET_THIS_PADDR(r2, ia64_mca_pal_base)
>       ;;
> @@ -263,7 +269,16 @@ ia64_do_tlb_purge:
>       srlz.i
>       ;;
>       // 4. Purge DTR for stack.
> +#ifdef XEN
> +     // Kernel registers are saved in a per_cpu cpu_kr_ia64_t
> +     // to allow the kernel registers themselves to be used by domains.
> +     GET_THIS_PADDR(r2, cpu_kr);;
> +     add r2=IA64_KR_CURRENT_STACK_OFFSET,r2
> +     ;;
> +     ld8 r16=[r2]
> +#else
>       mov r16=IA64_KR(CURRENT_STACK)
> +#endif
>       ;;
>       shl r16=r16,IA64_GRANULE_SHIFT
>       movl r19=PAGE_OFFSET
> @@ -277,8 +292,8 @@ ia64_do_tlb_purge:
>       ;;
>  #ifdef XEN
>       // 5. VHPT
> +     // r2 = __va_ul(vcpu_vhpt_maddr(v));
>  #if VHPT_ENABLED
> -     // r25 = __va_ul(vcpu_vhpt_maddr(v));
>       GET_THIS_PADDR(r2,cpu_kr);;
>       add r2=IA64_KR_CURRENT_OFFSET,r2;;
>       ld8 r2=[r2];;
> @@ -289,25 +304,24 @@ ia64_do_tlb_purge:
>       ld8 r3=[r3];; 
>       dep r3=0,r3,60,4;;                      // virtual to physical
>       add r3=IA64_DOMAIN_FLAGS_OFFSET,r3;;
> -     ld8 r3=[r3];; 
>       and r3=HAS_PERVCPU_VHPT_MASK,r3;;
>       cmp.eq p6,p0=r3,r0;;
>  (p6) br.cond.sptk    .not_pervcpu_vhpt
>       add r2=IA64_VCPU_VHPT_MADDR_OFFSET,r2;;
>       dep r2=0,r2,60,4;;                      // virtual to physical
>       ld8 r2=[r2];; 
> -     dep r25=-1,r2,60,4;;                    // physical to virtual
> +     dep r2=-1,r2,60,4;;                     // physical to virtual
>       br.sptk         .percpu_vhpt_done
>  #endif
>  .not_pervcpu_vhpt:
>       GET_THIS_PADDR(r2, vhpt_paddr);; 
>       ld8 r2=[r2];; 
> -     dep r25=-1,r2,60,4;;                    // physical to virtual
> +     dep r2=-1,r2,60,4;;                     // physical to virtual
>  .percpu_vhpt_done:
> -     dep r20=0,r25,0,IA64_GRANULE_SHIFT
> -     mov r24=IA64_GRANULE_SHIFT<<2
> +     dep r16=0,r2,0,IA64_GRANULE_SHIFT
> +     mov r18=IA64_GRANULE_SHIFT<<2
>       ;;
> -     ptr.d r20,r24
> +     ptr.d r16,r18
>       ;;
>       srlz.d
>       ;;
> 
> -- 
> 
> -- 
> Horms
>   H: http://www.vergenet.net/~horms/
>   W: http://www.valinux.co.jp/en/
> 
> 
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.