WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-ia64-devel][PATCH]Build new infrastructure for fast fault handl

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