[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [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 _______________________________________________ Xen-ia64-devel mailing list Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-ia64-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |