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