[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-ia64-devel][PATCH]Build new infrastructure for fast fault handling path.
On Fri, May 09, 2008 at 11:23:25AM +0800, Xu, Anthony wrote: > This is updated one according to your comments except for belows. Great. I found some xenoprof issue. Please see comments below.. I remembered that someone asked xenoprof doesn't work for VTi domain. I suspect that similar issue existed. > >> diff -r f2457c7aff8d xen/arch/ia64/vmx/vmx_phy_mode.c > >> --- a/xen/arch/ia64/vmx/vmx_phy_mode.c Fri Apr 25 20:13:52 2008 > +0900 > >> +++ b/xen/arch/ia64/vmx/vmx_phy_mode.c Thu May 08 16:23:42 2008 > +0800 > >> @@ -252,8 +252,8 @@ switch_mm_mode(VCPU *vcpu, IA64_PSR old_ > >> switch_to_virtual_rid(vcpu); > >> break; > >> case SW_SELF: > >> - printk("Switch to self-0x%lx!!! MM mode doesn't > >> change...\n", > >> - old_psr.val); > >> +// printk("Switch to self-0x%lx!!! MM mode doesn't > >> change...\n", +// old_psr.val); break; > >> case SW_NOP: > >> // printk("No action required for mode transition: (0x%lx -> > >> 0x%lx)\n", > > > > What's the purpose here. > > Anyway if you want this part, please create another patch. > > > > > Switch_mm_mode is called in fast path, since printk accesses PIO, it may > trigger TLB fault, which can't be handled in fast path, due to psr.ic=0. I see. I understand that with this patch, SW_SELF and SW_NOP case can occur frequenstly. > diff -r f2457c7aff8d xen/arch/ia64/vmx/vmx_vcpu.c > --- a/xen/arch/ia64/vmx/vmx_vcpu.c Fri Apr 25 20:13:52 2008 +0900 > +++ b/xen/arch/ia64/vmx/vmx_vcpu.c Fri May 09 10:58:37 2008 +0800 > @@ -172,11 +172,6 @@ IA64FAULT vmx_vcpu_set_rr(VCPU *vcpu, u6 > { > u64 rrval; > > - if (unlikely(is_reserved_rr_rid(vcpu, val))) { > - gdprintk(XENLOG_DEBUG, "use of invalid rrval %lx\n", val); > - return IA64_RSVDREG_FAULT; > - } > - > VMX(vcpu,vrr[reg>>VRN_SHIFT]) = val; > switch((u64)(reg>>VRN_SHIFT)) { > case VRN7: Without the check, VTi domain guest may access other domain's page or xen's page. So it can't be dropped. Do you find that calling is_reserved_rr_rid() is heavy? If so, please make it an inline function. > +void vmx_vcpu_mov_to_psr_fast(VCPU *vcpu, u64 value) > +{ > + // TODO: Only allowed for current vcpu > + u64 old_vpsr, new_vpsr, mipsr; > + old_vpsr = VCPU(vcpu,vpsr); > > + new_vpsr = (old_vpsr & 0xffffffff00000000) > + | ( value & 0xffffffff); > + VCPU(vcpu, vpsr) = new_vpsr; > > + mipsr = ia64_getreg( _IA64_REG_CR_IPSR); > + mipsr = (mipsr & 0xffffffff00000000) | ( value & 0xffffffff); > + /* xenoprof: > + * don't change psr.pp. > + * It is manipulated by xenoprof. > + */ > + mipsr |= IA64_PSR_IC | IA64_PSR_I | IA64_PSR_DT > + | IA64_PSR_PP | IA64_PSR_SI | IA64_PSR_RT; > + > + if (FP_PSR(vcpu) & IA64_PSR_DFH) > + mipsr |= IA64_PSR_DFH; > + > + ia64_setreg(_IA64_REG_CR_IPSR, mipsr); > + > + switch_mm_mode(vcpu,(IA64_PSR)old_vpsr, (IA64_PSR)new_vpsr); > + > +} For xenoprof, ipsr.pp shouldn't be changed. This code always sets ispsr.pp. > + > + > + > +extern void vmx_asm_bsw0(void); > +extern void vmx_asm_bsw1(void); Please move these declarations into header file. > +#define IA64_PSR_MMU_VIRT (IA64_PSR_DT | IA64_PSR_RT | IA64_PSR_IT) > +void vmx_vcpu_rfi_fast(VCPU *vcpu) > +{ > + // TODO: Only allowed for current vcpu > + u64 vifs, vipsr, vpsr, mipsr, mask; > + vipsr = VCPU(vcpu,ipsr); > + vpsr = VCPU(vcpu,vpsr); > + vifs = VCPU(vcpu,ifs); > + if (vipsr&IA64_PSR_BN){ > + if(!(vpsr&IA64_PSR_BN)) > + vmx_asm_bsw1(); > + } > + else if (vpsr&IA64_PSR_BN) > + vmx_asm_bsw0(); > + > + /* > + * For those IA64_PSR bits: id/da/dd/ss/ed/ia > + * Since these bits will become 0, after success execution of each > + * instruction, we will change set them to mIA64_PSR > + */ > + VCPU(vcpu, vpsr) = vipsr & (~ (IA64_PSR_ID |IA64_PSR_DA > + | IA64_PSR_DD | IA64_PSR_ED | IA64_PSR_IA)); > + > + /* > + * All vIA64_PSR bits shall go to mPSR (v->tf->tf_special.psr) > + * , except for the following bits: > + * ic/i/dt/si/rt/mc/it/bn/vm > + */ > + mask = (IA64_PSR_IC | IA64_PSR_I | IA64_PSR_DT | IA64_PSR_SI | > + IA64_PSR_RT | IA64_PSR_MC | IA64_PSR_IT | IA64_PSR_BN | > + IA64_PSR_VM); > + mipsr = ia64_getreg( _IA64_REG_CR_IPSR); > + mipsr = (mipsr & mask ) | ( vipsr & (~mask) ); > + /* xenoprof */ > + mipsr |= IA64_PSR_PP; > + > + if (FP_PSR(vcpu) & IA64_PSR_DFH) > + mipsr |= IA64_PSR_DFH; > + > + ia64_setreg(_IA64_REG_CR_IPSR, mipsr); > + vmx_ia64_set_dcr(vcpu); > + > + if(vifs>>63) > + ia64_setreg(_IA64_REG_CR_IFS, vifs); > + > + ia64_setreg(_IA64_REG_CR_IIP, VCPU(vcpu,iip)); > + > + switch_mm_mode(vcpu,(IA64_PSR)vpsr, (IA64_PSR)vipsr); > +} Ditto. For xenoprof, ipsr.pp shouldn't be changed. This code always sets ipsr.pp. add IA64_PSR_PP to mask. not to mipsr. > + > + > +void vmx_vcpu_ssm_fast(VCPU *vcpu, u64 imm24) > +{ > + u64 old_vpsr, new_vpsr, mipsr; > + > + old_vpsr = VCPU(vcpu,vpsr); > + new_vpsr = old_vpsr | imm24; > + > + VCPU(vcpu, vpsr) = new_vpsr; > + > + mipsr = ia64_getreg( _IA64_REG_CR_IPSR); > + mipsr |= imm24; > + ia64_setreg(_IA64_REG_CR_IPSR, mipsr); > + > + switch_mm_mode(vcpu,(IA64_PSR)old_vpsr, (IA64_PSR)new_vpsr); > +} Ditto. For xenoprof, ipsr.pp shouldn't be changed. Mask it out. > + > +void vmx_vcpu_rsm_fast(VCPU *vcpu, u64 imm24) > +{ > + u64 old_vpsr, new_vpsr, mipsr; > + > + old_vpsr = VCPU(vcpu,vpsr); > + new_vpsr = old_vpsr & ~imm24; > + > + VCPU(vcpu, vpsr) = new_vpsr; > + > + mipsr = ia64_getreg( _IA64_REG_CR_IPSR); > + mipsr &= ~imm24; > + /* xenoprof: > + * don't change psr.pp. > + * It is manipulated by xenoprof. > + */ > + mipsr |= IA64_PSR_IC | IA64_PSR_I | IA64_PSR_DT > + | IA64_PSR_PP | IA64_PSR_SI; > + > + if (FP_PSR(vcpu) & IA64_PSR_DFH) > + mipsr |= IA64_PSR_DFH; > + > + ia64_setreg(_IA64_REG_CR_IPSR, mipsr); > + > + switch_mm_mode(vcpu,(IA64_PSR)old_vpsr, (IA64_PSR)new_vpsr); > +} Ditto. For xenoprof, ipsr.pp shouldn't be changed. This code always sets ipsr.pp. -- yamahata _______________________________________________ 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 |