WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-ia64-devel] [patch 14/14] ia64: kexec: Map EFI regions into the

To: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Subject: Re: [Xen-ia64-devel] [patch 14/14] ia64: kexec: Map EFI regions into the same place they are maped into in Linux
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Mon, 14 Jul 2008 22:22:19 +1000
Cc: Aron Griffis <aron@xxxxxx>, Alex Williamson <alex.williamson@xxxxxx>, xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 14 Jul 2008 05:22:25 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20080714104411.GH6955%yamahata@xxxxxxxxxxxxx>
List-help: <mailto:xen-ia64-devel-request@lists.xensource.com?subject=help>
List-id: Discussion of the ia64 port of Xen <xen-ia64-devel.lists.xensource.com>
List-post: <mailto:xen-ia64-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20080714092138.629986006@xxxxxxxxxxxx> <20080714093114.002737376@xxxxxxxxxxxx> <20080714104411.GH6955%yamahata@xxxxxxxxxxxxx>
Sender: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
On Mon, Jul 14, 2008 at 07:44:11PM +0900, Isaku Yamahata wrote:
> 
> Hi Simon.
> This patch moves PAL code mapping area from xen identity mapping area
> to EFI mapping area which overlaps with guest virtual address.
> It means that PAL code area can't be pinned down always as 
> xen/ia64 vmm does now.
> 
> So PAL code should be pinned down after RR7 is switched 
> and right before firmware calls.
> Without the followings patches, xen didn't boot up.
> Initial boot. Not kexec boot.
> 
> 
> [IA64] pin down correctly PAL CODE area.
> 
> So far PAL code is always pinned down by itr[IA64_TR_PALCODE].
> However the rule was changed such that the area is pinned
> down only by rid XEN_EFI_RR right before firmware call by set_one_rr_efi().
> So pinning down PAL CODE area by efi_map_pal_code(), ia64_new_rr7(),
> __vmx_switch_rr7() and ia64_reload_tr() is wrong.
> So pin down PAL code area only by ia64_new_rr7_efi() and don't pin it
> down by other functions.

Good point. I have made some comments below.

> Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
> 
> diff -r e8056a7091a7 xen/arch/ia64/linux-xen/efi.c
> --- a/xen/arch/ia64/linux-xen/efi.c   Mon Jul 14 19:31:54 2008 +0900
> +++ b/xen/arch/ia64/linux-xen/efi.c   Mon Jul 14 19:33:20 2008 +0900
> @@ -447,7 +447,11 @@
>  efi_map_pal_code (void)
>  {
>  #ifdef XEN
> -     u64 psr;
> +     /*
> +      * don't map PAL code.
> +      * PAL code is mapped by set_one_rr_efi() right before
> +      * firmware call.
> +      */
>       (void)efi_get_pal_addr();
>  #else
>       void *pal_vaddr = efi_get_pal_addr ();
> @@ -455,7 +459,6 @@
>  
>       if (!pal_vaddr)
>               return;
> -#endif
>  
>       /*
>        * Cannot write to CRx with PSR.ic=1
> @@ -466,6 +469,7 @@
>                IA64_GRANULE_SHIFT);
>       ia64_set_psr(psr);              /* restore psr */
>       ia64_srlz_i();
> +#endif
>  }
>  
>  void __init
> diff -r e8056a7091a7 xen/arch/ia64/linux-xen/mca_asm.S
> --- a/xen/arch/ia64/linux-xen/mca_asm.S       Mon Jul 14 19:31:54 2008 +0900
> +++ b/xen/arch/ia64/linux-xen/mca_asm.S       Mon Jul 14 19:33:20 2008 +0900
> @@ -473,6 +473,7 @@
>       ;;
>       srlz.d
>       ;;
> +#ifndef XEN
>       // 3. Reload ITR for PAL code.
>       GET_THIS_PADDR(r2, ia64_mca_pal_pte)
>       ;;
> @@ -491,6 +492,8 @@
>       ;;
>       srlz.i
>       ;;
> +#endif
> +     
>       // 4. Reload DTR for stack.
>  #ifdef XEN
>       // Kernel registers are saved in a per_cpu cpu_kr_ia64_t
> diff -r e8056a7091a7 xen/arch/ia64/vmx/vmx_entry.S
> --- a/xen/arch/ia64/vmx/vmx_entry.S   Mon Jul 14 19:31:54 2008 +0900
> +++ b/xen/arch/ia64/vmx/vmx_entry.S   Mon Jul 14 19:33:20 2008 +0900
> @@ -598,7 +598,7 @@
>  /*
>   * in0: new rr7
>   * in1: virtual address of guest_vhpt
> - * in2: virtual address of pal code segment
> + * in2: virtual addres of guest shared_info
>   * r8: will contain old rid value
>   */
>  
> @@ -611,7 +611,7 @@
>  GLOBAL_ENTRY(__vmx_switch_rr7)
>         // not sure this unwind statement is correct...
>         .prologue ASM_UNW_PRLG_RP|ASM_UNW_PRLG_PFS, ASM_UNW_PRLG_GRSAVE(1)
> -     alloc loc1 = ar.pfs, 4, 8, 0, 0
> +     alloc loc1 = ar.pfs, 4, 7, 0, 0
>  1:{
>       mov r28  = in0                  // copy procedure index
>       mov r8   = ip                   // save ip to compute branch
> @@ -623,12 +623,9 @@
>       tpa loc2 = loc2                 // get physical address of per cpu date
>       tpa r3 = r8                     // get physical address of ip
>       dep loc5 = 0,in1,60,4           // get physical address of guest_vhpt
> -     dep loc6 = 0,in2,60,4           // get physical address of pal code
> -     dep loc7 = 0,in3,60,4           // get physical address of privregs
> +     dep loc6 = 0,in2,60,4           // get physical address of privregs
>       ;;
>       dep loc6 = 0,loc6,0,IA64_GRANULE_SHIFT
> -                                        // mask granule shift
> -     dep loc7 = 0,loc7,0,IA64_GRANULE_SHIFT
>                                          // mask granule shift
>       mov loc4 = psr                  // save psr
>       ;;
> @@ -725,46 +722,31 @@
>       ;;
>  .vhpt_overlaps:
>  
> -     // re-pin mappings for PAL code section
> -     mov r24=IA64_TR_PALCODE
> -     or loc6 = r25,loc6              // construct PA | page properties
> -     mov r23 = IA64_GRANULE_SHIFT<<2
> -     ;;
> -     ptr.i   in2,r23
> -     ;;
> -     mov cr.itir=r23
> -     mov cr.ifa=in2
> -     ;;
> -     itr.i itr[r24]=loc6             // wire in new mapping...
> -     ;;
> -
>       // r16, r19, r20 are used by
>       //  ia64_switch_mode_phys()/ia64_switch_mode_virt()
>       // re-pin mappings for privregs
>       // r21  = (current physical addr) & (IA64_GRANULE_SIZE - 1)
>       // r17  = (guest_vhpt physical addr) & (IA64_GRANULE_SIZE - 1)
> -     // loc6 = (((pal phys addr) & (IA64_GRANULE_SIZE - 1) << 2)) | 
> PAGE_KERNEL
> -     // loc7 = (privregs physical addr) & (IA64_GRANULE_SIZE - 1)
> -     cmp.ne.unc p7,p0=r21,loc7       // check overlap with current stack
> +     // loc6 = (privregs physical addr) & (IA64_GRANULE_SIZE - 1)
> +     cmp.ne.unc p7,p0=r21,loc6       // check overlap with current stack
>       ;;
> -(p7) cmp.ne.unc p8,p0=r17,loc7       // check overlap with guest_vhpt
> +(p7) cmp.ne.unc p8,p0=r17,loc6       // check overlap with guest_vhpt
>       ;;
> -     // loc7 = (((privregs phys) & (IA64_GRANULE_SIZE - 1)) << 2) | 
> PAGE_KERNEL
> -     or loc7 = r25,loc7          // construct PA | page properties
> +     // loc6 = (((privregs phys) & (IA64_GRANULE_SIZE - 1)) << 2) | 
> PAGE_KERNEL
> +     or loc6 = r25,loc6          // construct PA | page properties
>       ;;
> -     cmp.ne p9,p0=loc6,loc7
>       mov r22=IA64_TR_VPD
>       mov r24=IA64_TR_MAPPED_REGS
>       mov r23=IA64_GRANULE_SHIFT<<2
>       ;;
> -(p9) ptr.i   in3,r23 
> -(p8) ptr.d   in3,r23
> +     ptr.i   in2,r23
> +(p8) ptr.d   in2,r23
>       mov cr.itir=r23
> -     mov cr.ifa=in3
> +     mov cr.ifa=in2
>       ;;
> -(p9) itr.i itr[r22]=loc7         // wire in new mapping...
> +     itr.i itr[r22]=loc6         // wire in new mapping...
>       ;;
> -(p8) itr.d dtr[r24]=loc7         // wire in new mapping...
> +(p8) itr.d dtr[r24]=loc6         // wire in new mapping...
>       ;;
>  
>       // done, switch back to virtual and return
> diff -r e8056a7091a7 xen/arch/ia64/vmx/vmx_phy_mode.c
> --- a/xen/arch/ia64/vmx/vmx_phy_mode.c        Mon Jul 14 19:31:54 2008 +0900
> +++ b/xen/arch/ia64/vmx/vmx_phy_mode.c        Mon Jul 14 19:33:20 2008 +0900
> @@ -172,7 +172,7 @@
>       ia64_dv_serialize_data();
>       vmx_switch_rr7(vrrtomrr(vcpu,VMX(vcpu, vrr[VRN7])),
>                        (void *)vcpu->arch.vhpt.hash,
> -                    pal_vaddr, vcpu->arch.privregs);
> +                    vcpu->arch.privregs);
>       ia64_set_pta(VMX(vcpu, mpta));
>       vmx_ia64_set_dcr(vcpu);
>  
> diff -r e8056a7091a7 xen/arch/ia64/vmx/vmx_vcpu.c
> --- a/xen/arch/ia64/vmx/vmx_vcpu.c    Mon Jul 14 19:31:54 2008 +0900
> +++ b/xen/arch/ia64/vmx/vmx_vcpu.c    Mon Jul 14 19:33:20 2008 +0900
> @@ -197,12 +197,12 @@
>  }
>  
>  void vmx_switch_rr7(unsigned long rid, void *guest_vhpt,
> -                    void *pal_vaddr, void *shared_arch_info)
> +                    void *shared_arch_info)
>  {
>      __get_cpu_var(inserted_vhpt) = guest_vhpt;
>      __get_cpu_var(inserted_vpd) = shared_arch_info;
>      __get_cpu_var(inserted_mapped_regs) = shared_arch_info;
> -    __vmx_switch_rr7(rid, guest_vhpt, pal_vaddr, shared_arch_info);
> +    __vmx_switch_rr7(rid, guest_vhpt, shared_arch_info);
>  }
>  
>  IA64FAULT vmx_vcpu_set_rr(VCPU *vcpu, u64 reg, u64 val)
> @@ -219,7 +219,7 @@
>      case VRN7:
>          if (likely(vcpu == current))
>              vmx_switch_rr7(vrrtomrr(vcpu,val), (void *)vcpu->arch.vhpt.hash,
> -                           pal_vaddr, vcpu->arch.privregs);
> +                           vcpu->arch.privregs);
>         break;
>      case VRN4:
>          rrval = vrrtomrr(vcpu,val);
> diff -r e8056a7091a7 xen/arch/ia64/xen/xenasm.S
> --- a/xen/arch/ia64/xen/xenasm.S      Mon Jul 14 19:31:54 2008 +0900
> +++ b/xen/arch/ia64/xen/xenasm.S      Mon Jul 14 19:33:20 2008 +0900
> @@ -34,12 +34,13 @@
>  //                         unsigned long va_vhpt)     /* in4 */
>  //Local usage:
>  //  loc0=rp, loc1=ar.pfs, loc2=percpu_paddr, loc3=psr, loc4=ar.rse
> -//  loc5=pal_vaddr, loc6=xen_paddr, loc7=shared_archinfo_paddr,
> +//  loc5=shared_archinfo_paddr, loc6=xen_paddr, 
>  //  r16, r19, r20 are used by ia64_switch_mode_{phys, virt}()
> +// loc5 is unused.
>  GLOBAL_ENTRY(ia64_new_rr7)
>       // FIXME? not sure this unwind statement is correct...
>       .prologue ASM_UNW_PRLG_RP|ASM_UNW_PRLG_PFS, ASM_UNW_PRLG_GRSAVE(1)
> -     alloc loc1 = ar.pfs, 5, 8, 0, 0
> +     alloc loc1 = ar.pfs, 5, 7, 0, 0
>       movl loc2=PERCPU_ADDR
>  1:   {
>         mov loc3 = psr                // save psr     
> @@ -51,7 +52,7 @@
>       tpa in1=in1                     // grab shared_info BEFORE changing rr7
>       adds r8 = 1f-1b,r8              // calculate return address for call
>       ;;
> -     tpa loc7=in2                    // grab arch_vcpu_info BEFORE chg rr7
> +     tpa loc5=in2                    // grab arch_vcpu_info BEFORE chg rr7
>       movl r17=PSR_BITS_TO_SET
>       mov loc4=ar.rsc                 // save RSE configuration
>       movl r16=PSR_BITS_TO_CLEAR
> @@ -60,10 +61,7 @@
>       mov ar.rsc=0                    // put RSE in enforced lazy, LE mode
>       or loc3=loc3,r17                // add in psr the bits to set
>       ;;
> -     movl loc5=pal_vaddr             // get pal_vaddr
> -     ;;
> -     ld8 loc5=[loc5]                 // read pal_vaddr
> -     ;;
> +
>       andcm r16=loc3,r16              // removes bits to clear from psr
>       dep loc6=0,r8,0,KERNEL_TR_PAGE_SHIFT // Xen code paddr
>       br.call.sptk.many rp=ia64_switch_mode_phys
> @@ -163,24 +161,12 @@
>       add r22=r22,in3
>       ;;
>       ptr.d   r22,r24
> -     or r23=loc7,r25                 // construct PA | page properties
> +     or r23=loc5,r25                 // construct PA | page properties
>       mov cr.itir=r24
>       mov cr.ifa=r22
>       mov r21=IA64_TR_MAPPED_REGS
>       ;;
>       itr.d dtr[r21]=r23              // wire in new mapping...
> -
> -     // Purge/insert PAL TR
> -     mov r24=IA64_TR_PALCODE
> -     mov r23=IA64_GRANULE_SHIFT<<2
> -     dep r25=0,loc5,60,4             // convert pal vaddr to paddr
> -     ;;
> -     ptr.i   loc5,r23
> -     or r25=r25,r26                  // construct PA | page properties
> -     mov cr.itir=r23
> -     mov cr.ifa=loc5
> -     ;;
> -     itr.i itr[r24]=r25
>  
>       // done, switch back to virtual and return
>       mov r16=loc3                    // r16= original psr
> @@ -361,6 +347,8 @@
>       mov cr.ifa=loc5
>       ;;
>       itr.i itr[r24]=r25
> +     ;;
> +     srlz.i

This srlz.i is probably good to have, but its not really related ?

>       // done, switch back to virtual and return
>       mov r16=loc3                    // r16= original psr
> diff -r e8056a7091a7 xen/include/asm-ia64/linux-xen/linux/efi.h
> --- a/xen/include/asm-ia64/linux-xen/linux/efi.h      Mon Jul 14 19:31:54 
> 2008 +0900
> +++ b/xen/include/asm-ia64/linux-xen/linux/efi.h      Mon Jul 14 19:33:20 
> 2008 +0900
> @@ -25,6 +25,7 @@
>  #include <asm/system.h>
>  
>  #ifdef XEN
> +#include <asm/meminit.h>     /* GRANULEROUNDDOWN */
>  extern void * pal_vaddr;
>  #endif
>  
> @@ -474,6 +475,10 @@
>  } while (0)
>  
>  #define XEN_EFI_RR_RESTORE(rr6, rr7) do {            \
> +     ia64_ptr(0x1 /*I*/,                             \
> +              GRANULEROUNDDOWN(                      \
> +                      (unsigned long)pal_vaddr),     \
> +              IA64_GRANULE_SHIFT);                   \
>       set_one_rr_efi(6UL << 61, rr6);                 \
>       set_one_rr_efi(7UL << 61, rr7);                 \

I don't think this is quite right because ia64_new_rr7_efi
(via set_one_rr_efi()) will just reinsert the pal_vaddr.

I think it might be better to make things a bit more symmetrical.
Pin pal_vaddr in XEN_EFI_RR_SAVE after calling set_one_rr_efi(),
unpin it in XEN_EFI_RR_RESTORE (as above) and not touch it at all in
set_one_rr_efi().

It would probably also be good to give XEN_EFI_RR_RESTORE and
XEN_EFI_RR_SAVE different names. Perhaps XEN_EFI_ENTER and
XEN_EFI_LEAVE.

I will play with your patch and see if my ideas work.

>  } while (0)
> diff -r e8056a7091a7 xen/include/asm-ia64/vmx_vcpu.h
> --- a/xen/include/asm-ia64/vmx_vcpu.h Mon Jul 14 19:31:54 2008 +0900
> +++ b/xen/include/asm-ia64/vmx_vcpu.h Mon Jul 14 19:33:20 2008 +0900
> @@ -104,9 +104,9 @@
>  extern int vmx_vcpu_pend_interrupt(VCPU * vcpu, uint8_t vector);
>  extern void vcpu_load_kernel_regs(VCPU * vcpu);
>  extern void __vmx_switch_rr7(unsigned long rid, void *guest_vhpt,
> -                             void *pal_vaddr, void *shared_arch_info);
> +                             void *shared_arch_info);
>  extern void vmx_switch_rr7(unsigned long rid, void *guest_vhpt,
> -                           void *pal_vaddr, void *shared_arch_info);
> +                           void *shared_arch_info);
>  extern void vmx_ia64_set_dcr(VCPU * v);
>  extern void inject_guest_interruption(struct vcpu *vcpu, u64 vec);
>  extern void vmx_asm_bsw0(void);

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

<Prev in Thread] Current Thread [Next in Thread>