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
|