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

["Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>]

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. 
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

Attachment: fix_mfn_valid.patch
Description: fix_mfn_valid.patch

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