Applied, thanks. On Fri, Nov 07, 2008 at 04:42:52PM +0800, Zhang, Xiantao wrote: > 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 >
> _______________________________________________ > Xen-ia64-devel mailing list > Xen-ia64-devel@lists.xensource.com > http://lists.xensource.com/xen-ia64-devel -- yamahata _______________________________________________ Xen-ia64-devel mailing list Xen-ia64-devel@lists.xensource.com http://lists.xensource.com/xen-ia64-devel
