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: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Subject: RE: [Xen-ia64-devel] [PATCH] Fix vti guests broken issue.
From: "Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>
Date: Fri, 7 Nov 2008 16:42:52 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-ia64-devel@xxxxxxxxxxxxxxxxxxx" <xen-ia64-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 07 Nov 2008 00:44:04 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20081107080558.GD20847%yamahata@xxxxxxxxxxxxx>
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>
Sender: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AclAr7eZz1mit3/kS1emNcwt9yhajwABPYLA
Thread-topic: [Xen-ia64-devel] [PATCH] Fix vti guests broken issue.
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

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