Re: [Xen-ia64-devel] [PATCH] Fix vti guests broken issue.

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

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

-- 
yamahata

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