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 <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
>
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel
--
yamahata
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home