Re: [Xen-ia64-devel] [PATCH] Enable hash vtlb on dom0&domU

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

Hi Anthony.
Here are some comments.


- You replaced tlb related functions completely.
  priv_ptc_l(), priv_ptc_e(), priv_ptc_g(), priv_ptc_ga()...
  vcpu_tpa(), vcpu_itc_d(), vcpu_itc_i()...
  Simple '#ifdef HASH_VHPT' makes sources very hard to read.
  Could you go a cleaner way? For example, define a switch.

- if (mfn < max_page)
        set_gpfn_from_mfn(mfn, gpfn)

  What's is the purpose if (mfn < max_page)?
  If you want to avoid to write beyond arrays size,
  set_gpfn_from_mfn() should be changed instead of sprinkling
  if (mfn < max_pages).

--- a/xen/arch/ia64/xen/hyperprivop.S   Mon Mar 27 22:36:47 2006
+++ b/xen/arch/ia64/xen/hyperprivop.S   Tue Mar 28 22:50:16 2006
@@ -736,6 +738,7 @@
        adds r21=XSI_RR0_OFS-XSI_PSR_IC_OFS,r18 ;;
        shladd r22=r22,3,r21;;
        ld8 r22=[r22];;
+    and r22= -2,r22;;
        st8 [r23]=r22;;
        br.cond.sptk.many fast_reflect;;

Is this a bug fix?
It seems unrelated to HASH_VHPT. Could you explain?

--- a/xen/arch/ia64/xen/vcpu.c  Mon Mar 27 22:36:47 2006
+++ b/xen/arch/ia64/xen/vcpu.c  Tue Mar 28 22:50:16 2006
@@ -172,10 +176,10 @@
 {
        /* only do something if mode changes */
        if (!!newmode ^ !!PSCB(vcpu,metaphysical_mode)) {
+               PSCB(vcpu,metaphysical_mode) = newmode;
                if (newmode) set_metaphysical_rr0();
                else if (PSCB(vcpu,rrs[0]) != -1)
                        set_one_rr(0, PSCB(vcpu,rrs[0]));
-               PSCB(vcpu,metaphysical_mode) = newmode;
        }
 }
 
Is this a bug fix independent of HASH_VHPT?


- diff -r 7e3cbc409676 xen/include/asm-ia64/vhpt.h
Could you align the backslashes of the end of line?
It's ugly.

--- a/xen/arch/ia64/xen/vcpu.c  Mon Mar 27 22:36:47 2006
+++ b/xen/arch/ia64/xen/vcpu.c  Tue Mar 28 22:50:16 2006
 IA64FAULT vcpu_tpa(VCPU *vcpu, UINT64 vadr, UINT64 *padr)
 {
+#ifdef  HASH_VHPT
+    thash_data_t *data;
+    thash_cb_t  *hcb;
+    UINT64 mask;
+    hcb = vmx_vcpu_get_vtlb(vcpu);
+    data = vtlb_lookup(hcb, vadr, DSIDE_TLB);
+    if(data){
+        mask = itir_mask(data->itir);
+        *padr = (data->page_flags & _PAGE_PPN_MASK & mask) |
+            (vadr & ~mask);
+        return (IA64_NO_FAULT);
+    }        
+    data = vhpt_lookup(vadr);
+    if(data){
+        if(vcpu->domain == dom0){
+            *padr =  (arch_to_xen_ppn(data->ppn)<<PAGE_SHIFT) |
+                (vadr&(PAGE_SIZE-1));
+        }else{
+            *padr = ((*(mpt_table+arch_to_xen_ppn(data->ppn)))<<PAGE_SHIFT) | 
+                (vadr&(PAGE_SIZE-1));
+        }
+        return (IA64_NO_FAULT);
+    }    
+#else
+
        UINT64 pteval, itir, mask, iha;
        IA64FAULT fault;
 
I thinks get_gmfn_from_mfn() is more preferable than direct
accessing mpt_table.


Thanks.

On Tue, Mar 28, 2006 at 11:42:14PM +0800, Xu, Anthony wrote:
> This patch is intended to enable hash vtlb on dom0&domU.
> It's an option, and is turned off by default.
> If you want to turn on it, uncomment below line in file 
> xen/arch/ia64/Rules.mk.
> #CFLAGS  += -DHASH_VHPT
> 
> Signed-off-by: Anthony Xu <anthony.xu@xxxxxxxxx>
> 
> Thanks,
> -Anthony 


> _______________________________________________
> 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