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] Fix vti guests broken issue.

To: "Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>
Subject: Re: [Xen-ia64-devel] [PATCH] Fix vti guests broken issue.
From: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Date: Fri, 7 Nov 2008 19:31:18 +0900
Cc: "xen-ia64-devel@xxxxxxxxxxxxxxxxxxx" <xen-ia64-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 07 Nov 2008 02:32:25 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <706158FABBBA044BAD4FE898A02E4BC21983F8B8@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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: <706158FABBBA044BAD4FE898A02E4BC21983F5F3@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20081107030326.GB20847%yamahata@xxxxxxxxxxxxx> <706158FABBBA044BAD4FE898A02E4BC21983F65E@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <706158FABBBA044BAD4FE898A02E4BC21983F67A@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20081107034634.GC20847%yamahata@xxxxxxxxxxxxx> <706158FABBBA044BAD4FE898A02E4BC21983F83E@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20081107080558.GD20847%yamahata@xxxxxxxxxxxxx> <706158FABBBA044BAD4FE898A02E4BC21983F8B8@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.6i
Applied, thanks.

On Fri, Nov 07, 2008 at 04:42:52PM +0800, Zhang, Xiantao wrote:
> Okay, Updated :)
> Xiantao
> 
> PATCH: Fix frametable_miss handling for HVM guests.
> 
> For hvm guests, hypervisor use mfn_valid to check mfn, but it will incur
> weird faults. It is becasue ipsr is saved in r29, but frametalbe miss assumes
> saved in r21.
> 
> Signed-off-by Xiantao Zhang <xiantao.zhang@xxxxxxxxx> 
> 
> diff -r f6795589ef82 xen/arch/ia64/vmx/vmx_ivt.S
> --- a/xen/arch/ia64/vmx/vmx_ivt.S     Thu Nov 06 12:14:57 2008 +0900
> +++ b/xen/arch/ia64/vmx/vmx_ivt.S     Fri Nov 07 16:35:55 2008 +0800
> @@ -343,7 +343,7 @@ END(vmx_alt_itlb_miss)
>  // 0x1000 Entry 4 (size 64 bundles) Alt DTLB (7,46)
>  ENTRY(vmx_alt_dtlb_miss)
>      VMX_DBG_FAULT(4)
> -    mov r29=cr.ipsr
> +    mov r29=cr.ipsr  //frametable_miss needs ipsr is saved in r29.
>      mov r31=pr
>      adds r22=IA64_VCPU_MMU_MODE_OFFSET, r21
>      ;;
> @@ -356,7 +356,7 @@ vmx_alt_dtlb_miss_vmm:
>      // Test for the address of virtual frame_table
>      shr r22=r16,56;;
>      cmp.eq p8,p0=((VIRT_FRAME_TABLE_ADDR>>56)&0xff)-0x100,r22
> -(p8)br.cond.sptk frametable_miss ;;
> +(p8)br.cond.sptk frametable_miss ;; //Make sure ipsr is saved in r29
>  #endif
>      movl r17=PAGE_KERNEL
>      mov r20=cr.isr
> diff -r f6795589ef82 xen/arch/ia64/xen/ivt.S
> --- a/xen/arch/ia64/xen/ivt.S Thu Nov 06 12:14:57 2008 +0900
> +++ b/xen/arch/ia64/xen/ivt.S Fri Nov 07 16:35:55 2008 +0800
> @@ -184,10 +184,12 @@ late_alt_dtlb_miss:
>  late_alt_dtlb_miss:
>       mov r20=cr.isr
>       movl r17=PAGE_KERNEL
> -     mov r21=cr.ipsr
> +     mov r29=cr.ipsr // frametable_miss is shared by paravirtual and HVM 
> sides
> +                     // and it assumes ipsr is saved in r29. If change the
> +                     // registers usage here, please check both sides!   
>       movl r19=(((1 << IA64_MAX_PHYS_BITS) - 1) & ~0xfff)
>       ;;
> -     extr.u r23=r21,IA64_PSR_CPL0_BIT,2      // extract psr.cpl
> +     extr.u r23=r29,IA64_PSR_CPL0_BIT,2      // extract psr.cpl
>       and r22=IA64_ISR_CODE_MASK,r20          // get the isr.code field
>       tbit.nz p6,p7=r20,IA64_ISR_SP_BIT       // is speculation bit on?
>       extr.u r18=r16,XEN_VIRT_UC_BIT,1        // extract UC bit
> @@ -234,7 +236,7 @@ late_alt_dtlb_miss:
>       br.cond.spnt page_fault
>       ;;
>  alt_dtlb_miss_identity_map:
> -     dep r21=-1,r21,IA64_PSR_ED_BIT,1
> +     dep r29=-1,r29,IA64_PSR_ED_BIT,1
>       or r19=r19,r17          // insert PTE control bits into r19
>       mov cr.itir=r20         // set itir with cleared key
>       ;;
> @@ -243,7 +245,7 @@ alt_dtlb_miss_identity_map:
>       cmp.eq.or p8,p0=0x18,r22        // Region 6 is UC for EFI
>       ;;
>  (p8) dep r19=-1,r19,4,1      // set bit 4 (uncached) if access to UC area
> -(p6) mov cr.ipsr=r21
> +(p6) mov cr.ipsr=r29
>       ;;
>  (p7) itc.d r19               // insert the TLB entry
>       mov pr=r31,-1
> @@ -288,17 +290,17 @@ GLOBAL_ENTRY(frametable_miss)
>       rfi
>  END(frametable_miss)
>  
> -ENTRY(frametable_fault)
> +ENTRY(frametable_fault)              //ipsr saved in r29 before coming here!
>       ssm psr.dt              // switch to using virtual data addressing
>       mov r18=cr.iip
>       movl r19=ia64_frametable_probe
>       ;;
>       cmp.eq p6,p7=r18,r19    // is faulting addrress ia64_frametable_probe?
>       mov r8=0                // assumes that 'probe.r' uses r8
> -     dep r21=-1,r21,IA64_PSR_RI_BIT+1,1 // return to next instruction in
> +     dep r29=-1,r29,IA64_PSR_RI_BIT+1,1 // return to next instruction in
>                                          //   bundle 2
>       ;;
> -(p6) mov cr.ipsr=r21
> +(p6) mov cr.ipsr=r29
>       mov r19=4               // FAULT(4)
>  (p7) br.spnt.few dispatch_to_fault_handler
>       ;;
> Isaku Yamahata wrote:
> > On Fri, Nov 07, 2008 at 03:47:10PM +0800, Zhang, Xiantao wrote:
> >> Hi, Isaku
> >>    Attached patch should fix the issue.  Paravirtualized ivt and HVM
> >> ivt share the code for frametable_miss handling, but they have
> >> different assumptions for registers useage. IPSR is savded in r21 at
> >> paravirtualized side, but r29 is used for HVM side. This patch
> >> uniform them to use r29 for ipsr save.   
> > 
> > Oh great! That explains why mfn_valid() didn't work and
> > the patch looks good.
> > Could you please add the comment above late_alt_dtlb_miss
> > why the r29 is used instead of r21 in ivt.S?
> > 
> > thanks,
> > 
> >> Thanks
> >> Xiantao
> >> 
> >> 
> >> PATCH: Fix frametable_miss handling for HVM guests.
> >> 
> >> For hvm guests, hypervisor use mfn_valid to check mfn, but it will
> >> incur 
> >> weird faults. It is becasue ipsr is saved in r29, but frametalbe
> >> miss assumes 
> >> saved in r21.
> >> 
> >> Signed-off-by Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> >> 
> >> diff -r f6795589ef82 xen/arch/ia64/vmx/vmx_ivt.S
> >> --- a/xen/arch/ia64/vmx/vmx_ivt.S  Thu Nov 06 12:14:57 2008 +0900
> >> +++ b/xen/arch/ia64/vmx/vmx_ivt.S  Fri Nov 07 15:31:26 2008 +0800
> >> @@ -356,7 +356,7 @@ vmx_alt_dtlb_miss_vmm:
> >>      // Test for the address of virtual frame_table      shr
> >>      r22=r16,56;; cmp.eq
> >> p8,p0=((VIRT_FRAME_TABLE_ADDR>>56)&0xff)-0x100,r22 -(p8)br.cond.sptk
> >> frametable_miss ;; +(p8)br.cond.sptk frametable_miss ;; //Make sure
> >>      ipsr is saved in r29  #endif movl r17=PAGE_KERNEL
> >>      mov r20=cr.isr
> >> diff -r f6795589ef82 xen/arch/ia64/xen/ivt.S
> >> --- a/xen/arch/ia64/xen/ivt.S      Thu Nov 06 12:14:57 2008 +0900
> >> +++ b/xen/arch/ia64/xen/ivt.S      Fri Nov 07 15:31:26 2008 +0800
> >> @@ -184,10 +184,10 @@ late_alt_dtlb_miss:
> >>  late_alt_dtlb_miss:
> >>    mov r20=cr.isr
> >>    movl r17=PAGE_KERNEL
> >> -  mov r21=cr.ipsr
> >> +  mov r29=cr.ipsr
> >>    movl r19=(((1 << IA64_MAX_PHYS_BITS) - 1) & ~0xfff)     ;;
> >> -  extr.u r23=r21,IA64_PSR_CPL0_BIT,2      // extract psr.cpl
> >> +  extr.u r23=r29,IA64_PSR_CPL0_BIT,2      // extract psr.cpl
> >>    and r22=IA64_ISR_CODE_MASK,r20          // get the isr.code field
> >>    tbit.nz p6,p7=r20,IA64_ISR_SP_BIT       // is speculation bit on?
> >>    extr.u r18=r16,XEN_VIRT_UC_BIT,1        // extract UC bit
> >> @@ -234,7 +234,7 @@ late_alt_dtlb_miss:
> >>    br.cond.spnt page_fault
> >>    ;;
> >>  alt_dtlb_miss_identity_map:
> >> -  dep r21=-1,r21,IA64_PSR_ED_BIT,1
> >> +  dep r29=-1,r29,IA64_PSR_ED_BIT,1
> >>    or r19=r19,r17          // insert PTE control bits into r19
> >>    mov cr.itir=r20         // set itir with cleared key
> >>    ;;
> >> @@ -243,7 +243,7 @@ alt_dtlb_miss_identity_map:
> >>    cmp.eq.or p8,p0=0x18,r22        // Region 6 is UC for EFI       ;;
> >>  (p8)      dep r19=-1,r19,4,1      // set bit 4 (uncached) if access to UC
> >> area -(p6) mov cr.ipsr=r21 +(p6)   mov cr.ipsr=r29
> >>    ;;
> >>  (p7)      itc.d r19               // insert the TLB entry
> >>    mov pr=r31,-1
> >> @@ -288,17 +288,17 @@ GLOBAL_ENTRY(frametable_miss)        rfi
> >>  END(frametable_miss)
> >> 
> >> -ENTRY(frametable_fault)
> >> +ENTRY(frametable_fault)           //ipsr saved in r29 before coming here!
> >>    ssm psr.dt              // switch to using virtual data addressing      
> >> mov
> >>    r18=cr.iip movl r19=ia64_frametable_probe
> >>    ;;
> >>    cmp.eq p6,p7=r18,r19    // is faulting addrress ia64_frametable_probe?
> >>    mov r8=0                // assumes that 'probe.r' uses r8
> >> -  dep r21=-1,r21,IA64_PSR_RI_BIT+1,1 // return to next instruction in
> >> +  dep r29=-1,r29,IA64_PSR_RI_BIT+1,1 // return to next instruction
> >>    in                                         //   bundle 2 ;;
> >> -(p6)      mov cr.ipsr=r21
> >> +(p6)      mov cr.ipsr=r29
> >>    mov r19=4               // FAULT(4)
> >>  (p7)      br.spnt.few dispatch_to_fault_handler
> >>    ;;
> >> 
> >> Isaku Yamahata wrote:
> >>> On Fri, Nov 07, 2008 at 11:33:43AM +0800, Zhang, Xiantao wrote:
> >>>> But another thing to meation, why mfn_valid with invalid parameter
> >>>> will incur the fault?  Seems mfn_valid has something wrong, I have
> >>>> no enough time to find the cause.  Is it a known issue ? Or
> >>>> mfn_valid has some limitation ?
> >>> 
> >>> mfn_valid() with invalid parameter shouldn't cause panic.
> >>> It may cause tlb miss fault, but the fault should be handled
> >>> specially by frametable_fault in ivt.S and should be recovered
> >>> resulting 
> >>> in mfn_valid() returning false.
> >>> 
> >>> I agree with you that there's something wrong in mfn_valid()
> >>> I haven't aware of the issue.
> >>> 
> >>> thanks,
> >>> 
> >>>> Thanks
> >>>> Xiantao
> >>>> 
> >>>> Zhang, Xiantao wrote:
> >>>>> Yes. Should be addressed.
> >>>>> 
> >>>>> -----Original Message-----
> >>>>> From: Isaku Yamahata [mailto:yamahata@xxxxxxxxxxxxx]
> >>>>> Sent: Friday, November 07, 2008 11:03 AM
> >>>>> To: Zhang, Xiantao
> >>>>> Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> >>>>> Subject: Re: [Xen-ia64-devel] [PATCH] Fix vti guests broken issue.
> >>>>> 
> >>>>> Oh, my bad. Thank you for debugging.
> >>>>> I applied and pushed out.
> >>>>> Does this fixed the issue you repoted?
> >>>>> 
> >>>>> thanks,
> >>>>> 
> >>>>> On Fri, Nov 07, 2008 at 10:42:57AM +0800, Zhang, Xiantao wrote:
> >>>>>> PATCH : Fix vti guests broken issue.
> >>>>>> mfn_valid should use machine physical pfn, not guest physical
> >>>>>> pfn. 
> >>>>>> 
> >>>>>> Sign-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> >>>>>> 
> >>>>>> 
> >>>>>> diff -r f6795589ef82 xen/arch/ia64/vmx/vtlb.c
> >>>>>> --- a/xen/arch/ia64/vmx/vtlb.c Thu Nov 06 12:14:57 2008 +0900
> >>>>>> +++ b/xen/arch/ia64/vmx/vtlb.c Fri Nov 07 10:35:11 2008 +0800
> >>>>>> @@ -522,7 +522,7 @@ static u64 translate_phy_pte(VCPU *v, u6
> >>>>>>       * which is required by vga acceleration since qemu maps
> >>>>>> shared 
> >>>>>>       * vram buffer with WB.
> >>>>>>       */
> >>>>>> -    if (mfn_valid(pte_pfn(__pte(pte))) && phy_pte.ma !=
> >>>>>> VA_MATTR_NATPAGE) +    if (mfn_valid(pte_pfn(__pte(maddr))) &&
> >>>>>>          phy_pte.ma != VA_MATTR_NATPAGE) phy_pte.ma =
> >>>>>> VA_MATTR_WB; 
> >>>>>> 
> >>>>>>      maddr = ((maddr & _PAGE_PPN_MASK) & PAGE_MASK) | (paddr &
> >>>>>> ~PAGE_MASK);
> >>>>> 
> >>>>>> _______________________________________________
> >>>>>> 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
> 


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

-- 
yamahata

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