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 <[EMAIL PROTECTED]> 

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 <[EMAIL PROTECTED]>
>> 
>> 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:[EMAIL PROTECTED]
>>>>> Sent: Friday, November 07, 2008 11:03 AM
>>>>> To: Zhang, Xiantao
>>>>> Cc: xen-ia64-devel@lists.xensource.com
>>>>> 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 <[EMAIL PROTECTED]>
>>>>>> 
>>>>>> 
>>>>>> 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@lists.xensource.com
>>>>>> http://lists.xensource.com/xen-ia64-devel
>> 
> 
> 
>> _______________________________________________
>> Xen-ia64-devel mailing list
>> Xen-ia64-devel@lists.xensource.com
>> 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@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel

Reply via email to