pagetable cleanups
PATCH 1/1 pagetable cleanups
Hi,
Next version of the pagetable cleanup patch. Builds and boots domain 0 on x86_32. Changes from the last version:
* macro names are changed.
* adapted to the new shadow code checked in last week.
* new macro: l1e_has_changed() to compare page table entries.
Open issues:
* I'm not sure how to handle the debug printk's best. These use the l1e_get_value() macro at the moment to get the raw bits and print them as unsigned long hex value. I'd like to get rid of the l1e_get_value() macro altogether though ...
* x86_64 build needs fixing, will look into this tomorrow.
Enjoy,
Gerd
arch/x86/shadow.c +190 -161
@@ -307,7 +307,7 @@ free_shadow_l1_table(struct domain *d, ufor ( i = min; i <= max; i++ ) {put_page_from_l1e (pl1e[i], d); - pl1e[i] =mk_l1_pgentry (0 ); +l1e_clear (pl1e[i]); }unmap_domain_mem (pl1e);
@@ -333,9 +333,8 @@ free_shadow_hl2_table(struct domain *d,for ( i =0 ; i < limit; i++ ) { -unsigned long hl2e =l1_pgentry_val (hl2[i]); -if ( hl2e & _PAGE_PRESENT ) -put_page (pfn_to_page (hl2e >>PAGE_SHIFT )); +if (l1e_get_flags (*hl2) & _PAGE_PRESENT ) +put_page (pfn_to_page (l1e_get_pfn (*hl2))); }unmap_domain_mem (hl2);
@@ -614,13 +613,14 @@ static void alloc_monitor_pagetable(stru HYPERVISOR_ENTRIES_PER_L2_PAGETABLE * sizeof(l2_pgentry_t));#endif - mpl2e[l2_table_offset (PERDOMAIN_VIRT_START )] = -mk_l2_pgentry ((__pa (d->arch.mm_perdomain_pt) &PAGE_MASK ) - | __PAGE_HYPERVISOR); +l2e_init_phys (mpl2e[l2_table_offset (PERDOMAIN_VIRT_START )], +__pa (d->arch.mm_perdomain_pt), + __PAGE_HYPERVISOR);// map the phys_to_machine map into the Read-Only MPT space for this domain - mpl2e[l2_table_offset (RO_MPT_VIRT_START )] = -mk_l2_pgentry (pagetable_val (d->arch.phys_table) | __PAGE_HYPERVISOR); +l2e_init_phys (mpl2e[l2_table_offset (RO_MPT_VIRT_START )], +pagetable_val (d->arch.phys_table), + __PAGE_HYPERVISOR); ed->arch.monitor_table =mk_pagetable (mmfn <<PAGE_SHIFT ); ed->arch.monitor_vtable = mpl2e;
@@ -644,7 +644,7 @@ void free_monitor_pagetable(struct exec_*/ hl2e = mpl2e[LINEAR_PT_VIRT_START >>L2_PAGETABLE_SHIFT ];ASSERT (l2_pgentry_val (hl2e) & _PAGE_PRESENT); - mfn =l2_pgentry_val (hl2e) >>PAGE_SHIFT ; + mfn =l2e_get_pfn (hl2e);ASSERT (mfn);put_shadow_ref (mfn);
@@ -672,7 +672,8 @@ set_p2m_entry(struct domain *d, unsignedASSERT ( phystab ); l2 =map_domain_mem (phystab); -if ( !l2_pgentry_val (l2e = l2[l2_table_offset (va)]) ) + l2e = l2[l2_table_offset (va)]; +if ( !l2e_get_value (l2e) )/* FIXME: check present bit? */ { l1page =alloc_domheap_page (NULL );if ( !l1page )
@@ -682,15 +683,14 @@ set_p2m_entry(struct domain *d, unsignedmemset (l1,0 ,PAGE_SIZE );unmap_domain_mem (l1); - l2e = l2[l2_table_offset (va)] = -mk_l2_pgentry ((page_to_pfn (l1page) <<PAGE_SHIFT ) | - __PAGE_HYPERVISOR); +l2e_init_pfn (l2e,page_to_pfn (l1page), __PAGE_HYPERVISOR); + l2[l2_table_offset (va)] = l2e; }unmap_domain_mem (l2); - l1 =map_domain_mem (l2_pgentry_val (l2e) &PAGE_MASK ); - l1[l1_table_offset (va)] =mk_l1_pgentry ((mfn <<PAGE_SHIFT ) | - __PAGE_HYPERVISOR); + l1 =map_domain_mem (l2e_get_phys (l2e)); +l1e_init_pfn (l1[l1_table_offset (va)], + mfn, __PAGE_HYPERVISOR);unmap_domain_mem (l1);return 1 ;
@@ -966,13 +966,12 @@ translate_l1pgtable(struct domain *d, l1for (i =0 ; i <L1_PAGETABLE_ENTRIES ; i++) {if (is_guest_l1_slot (i) && - (l1_pgentry_val (l1[i]) & _PAGE_PRESENT) ) + (l1e_get_flags (l1[i]) & _PAGE_PRESENT) ) { -unsigned long mfn =l1_pgentry_val (l1[i]) >>PAGE_SHIFT ; +unsigned long mfn =l1e_get_pfn (l1[i]);unsigned long gpfn =__mfn_to_gpfn (d, mfn); -ASSERT ((l1_pgentry_val (p2m[gpfn]) >>PAGE_SHIFT ) == mfn); - l1[i] =mk_l1_pgentry ((gpfn <<PAGE_SHIFT ) | - (l1_pgentry_val (l1[i]) & ~PAGE_MASK )); +ASSERT (l1e_get_pfn (p2m[gpfn]) == mfn); +l1e_init_pfn (l1[i], gpfn,l1e_get_flags (l1[i])); } }unmap_domain_mem (l1);
@@ -994,13 +993,12 @@ translate_l2pgtable(struct domain *d, l1for (i =0 ; i <L2_PAGETABLE_ENTRIES ; i++) {if (is_guest_l2_slot (i) && - (l2_pgentry_val (l2[i]) & _PAGE_PRESENT) ) + (l2e_get_flags (l2[i]) & _PAGE_PRESENT) ) { -unsigned long mfn =l2_pgentry_val (l2[i]) >>PAGE_SHIFT ; +unsigned long mfn =l2e_get_pfn (l2[i]);unsigned long gpfn =__mfn_to_gpfn (d, mfn);ASSERT ((l1_pgentry_val (p2m[gpfn]) >>PAGE_SHIFT ) == mfn); - l2[i] =mk_l2_pgentry ((gpfn <<PAGE_SHIFT ) | - (l2_pgentry_val (l2[i]) & ~PAGE_MASK )); +l2e_init_pfn (l2[i], gpfn,l2e_get_flags (l2[i]));translate_l1pgtable (d, p2m, mfn); } }
@@ -1266,13 +1264,13 @@ gpfn_to_mfn_foreign(struct domain *d, unl2_pgentry_t *l2 =map_domain_mem (phystab);l2_pgentry_t l2e = l2[l2_table_offset (va)];unmap_domain_mem (l2); -if ( !(l2_pgentry_val (l2e) & _PAGE_PRESENT) ) +if ( !(l2e_get_flags (l2e) & _PAGE_PRESENT) ) {printk ("gpfn_to_mfn_foreign(d->id=%d, gpfn=%p) => 0 l2e=%p\n" , - d->id, gpfn,l2_pgentry_val (l2e)); + d->id, gpfn,l2e_get_value (l2e));return INVALID_MFN ; } -unsigned long l1tab =l2_pgentry_val (l2e) &PAGE_MASK ; +unsigned long l1tab =l2e_get_phys (l2e);l1_pgentry_t *l1 =map_domain_mem (l1tab);l1_pgentry_t l1e = l1[l1_table_offset (va)];unmap_domain_mem (l1);
@@ -1282,14 +1280,14 @@ gpfn_to_mfn_foreign(struct domain *d, un d->id, gpfn, l1_pgentry_val(l1e) >> PAGE_SHIFT, phystab, l2e, l1tab, l1e);#endif -if ( !(l1_pgentry_val (l1e) & _PAGE_PRESENT) ) +if ( !(l1e_get_flags (l1e) & _PAGE_PRESENT) ) {printk ("gpfn_to_mfn_foreign(d->id=%d, gpfn=%p) => 0 l1e=%p\n" , - d->id, gpfn,l1_pgentry_val (l1e)); + d->id, gpfn,l1e_get_value (l1e));return INVALID_MFN ; } -return l1_pgentry_val (l1e) >>PAGE_SHIFT ; +return l1e_get_pfn (l1e); } static unsigned long
@@ -1330,12 +1328,12 @@ shadow_hl2_table(struct domain *d, unsig// Setup easy access to the GL2, SL2, and HL2 frames. // - hl2[l2_table_offset (LINEAR_PT_VIRT_START )] = -mk_l1_pgentry ((gmfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); - hl2[l2_table_offset (SH_LINEAR_PT_VIRT_START )] = -mk_l1_pgentry ((smfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); - hl2[l2_table_offset (PERDOMAIN_VIRT_START )] = -mk_l1_pgentry ((hl2mfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); +l1e_init_pfn (hl2[l2_table_offset (LINEAR_PT_VIRT_START )], + gmfn, __PAGE_HYPERVISOR); +l1e_init_pfn (hl2[l2_table_offset (SH_LINEAR_PT_VIRT_START )], + smfn, __PAGE_HYPERVISOR); +l1e_init_pfn (hl2[l2_table_offset (PERDOMAIN_VIRT_START )], + hl2mfn, __PAGE_HYPERVISOR); }unmap_domain_mem (hl2);
@@ -1383,21 +1381,20 @@ static unsigned long shadow_l2_table( &idle_pg_table[DOMAIN_ENTRIES_PER_L2_PAGETABLE ], HYPERVISOR_ENTRIES_PER_L2_PAGETABLE * sizeof(l2_pgentry_t)); - spl2e[l2_table_offset (SH_LINEAR_PT_VIRT_START )] = -mk_l2_pgentry ((smfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); +l2e_init_pfn (spl2e[l2_table_offset (SH_LINEAR_PT_VIRT_START )], + smfn, __PAGE_HYPERVISOR); - spl2e[l2_table_offset (PERDOMAIN_VIRT_START )] = -mk_l2_pgentry (__pa (page_get_owner ( - &frame_table[gmfn])->arch.mm_perdomain_pt) | - __PAGE_HYPERVISOR); +l2e_init_phys (spl2e[l2_table_offset (PERDOMAIN_VIRT_START )], +__pa (page_get_owner (&frame_table[gmfn])->arch.mm_perdomain_pt), + __PAGE_HYPERVISOR);if (shadow_mode_translate (d) )// NB: not external {unsigned long hl2mfn; - spl2e[l2_table_offset (RO_MPT_VIRT_START )] = -mk_l2_pgentry (pagetable_val (d->arch.phys_table) | - __PAGE_HYPERVISOR); +l2e_init_phys (spl2e[l2_table_offset (RO_MPT_VIRT_START )], +pagetable_val (d->arch.phys_table), + __PAGE_HYPERVISOR);if (unlikely (!(hl2mfn =__shadow_status (d, gpfn, PGT_hl2_shadow))) ) hl2mfn =shadow_hl2_table (d, gpfn, gmfn, smfn);
@@ -1408,12 +1405,12 @@ static unsigned long shadow_l2_table(if ( !get_shadow_ref (hl2mfn) )BUG (); - spl2e[l2_table_offset (LINEAR_PT_VIRT_START )] = -mk_l2_pgentry ((hl2mfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); +l2e_init_pfn (spl2e[l2_table_offset (LINEAR_PT_VIRT_START )], + hl2mfn, __PAGE_HYPERVISOR); }else - spl2e[l2_table_offset (LINEAR_PT_VIRT_START )] = -mk_l2_pgentry ((gmfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); +l2e_init_pfn (spl2e[l2_table_offset (LINEAR_PT_VIRT_START )], + gmfn, __PAGE_HYPERVISOR); }else {
@@ -1430,12 +1427,14 @@ void shadow_map_l1_into_current_l2(unsig {struct exec_domain *ed = current;struct domain *d = ed->domain; -unsigned long *gpl1e, *spl1e, gl2e, sl2e, gl1pfn, gl1mfn, sl1mfn; +l1_pgentry_t *gpl1e, *spl1e; +l2_pgentry_t gl2e, sl2e; +unsigned long gl1pfn, gl1mfn, sl1mfn;int i, init_table =0 ;__guest_get_l2e (ed, va, &gl2e);ASSERT (gl2e & _PAGE_PRESENT); - gl1pfn = gl2e >>PAGE_SHIFT ; + gl1pfn =l2e_get_pfn (gl2e);if ( !(sl1mfn =__shadow_status (d, gl1pfn, PGT_l1_shadow)) ) {
@@ -1468,9 +1467,9 @@ void shadow_map_l1_into_current_l2(unsig }#ifndef NDEBUG -unsigned long old_sl2e; +l2e_pgentry_t old_sl2e;__shadow_get_l2e (ed, va, &old_sl2e); -ASSERT ( !(old_sl2e & _PAGE_PRESENT) ); +ASSERT ( !(l2e_get_flags (old_sl2e) & _PAGE_PRESENT) );#endif if ( !get_shadow_ref (sl1mfn) )
@@ -1481,25 +1480,23 @@ void shadow_map_l1_into_current_l2(unsigif ( init_table ) { - gpl1e = (unsigned long *) - &(linear_pg_table[l1_linear_offset (va) & + gpl1e = &(linear_pg_table[l1_linear_offset (va) & ~(L1_PAGETABLE_ENTRIES -1 )]); - spl1e = (unsigned long *) - &(shadow_linear_pg_table[l1_linear_offset (va) & + spl1e = &(shadow_linear_pg_table[l1_linear_offset (va) & ~(L1_PAGETABLE_ENTRIES -1 )]); -unsigned long sl1e; +l1_pgentry_t sl1e;int index =l1_table_offset (va);int min =1 , max =0 ;for ( i =0 ; i <L1_PAGETABLE_ENTRIES ; i++ ) {l1pte_propagate_from_guest (d, gpl1e[i], &sl1e); -if ( (sl1e & _PAGE_PRESENT) && - !shadow_get_page_from_l1e (mk_l1_pgentry (sl1e), d) ) - sl1e =0 ; -if ( sl1e ==0 ) +if ( (l1e_get_flags (sl1e) & _PAGE_PRESENT) && + !shadow_get_page_from_l1e (sl1e, d) ) +l1e_clear (sl1e); +if (l1e_get_value (sl1e) ==0 )/* FIXME: check flags? */ {// First copy entries from 0 until first invalid. // Then copy entries from index until first invalid.
@@ -1525,7 +1522,7 @@ void shadow_map_l1_into_current_l2(unsigvoid shadow_invlpg (struct exec_domain *ed,unsigned long va) {struct domain *d = ed->domain; -unsigned long gpte, spte; +l1_pgentry_t gpte, spte;ASSERT (shadow_mode_enabled (d));
@@ -1538,8 +1535,8 @@ void shadow_invlpg(struct exec_domain *e// It's not strictly necessary to update the shadow here, // but it might save a fault later. // -if (__get_user (gpte, (unsigned long *) - &linear_pg_table[va >>PAGE_SHIFT ])) { +if (__copy_from_user (&gpte, &linear_pg_table[va >>PAGE_SHIFT ], +sizeof (gpte))) {perfc_incrc (shadow_invlpg_faults);return ; }
@@ -1707,31 +1704,30 @@ void shadow_mark_va_out_of_sync( {struct out_of_sync_entry *entry =shadow_mark_mfn_out_of_sync (ed, gpfn, mfn); -unsigned long sl2e; +l2_pgentry_t sl2e;// We need the address of shadow PTE that maps @va. // It might not exist yet. Make sure it's there. // __shadow_get_l2e (ed, va, &sl2e); -if ( !(sl2e & _PAGE_PRESENT) ) +if ( !(l2e_get_flags (sl2e) & _PAGE_PRESENT) ) {// either this L1 isn't shadowed yet, or the shadow isn't linked into // the current L2. shadow_map_l1_into_current_l2 (va);__shadow_get_l2e (ed, va, &sl2e); } -ASSERT (sl2e & _PAGE_PRESENT); +ASSERT (l2e_get_flags (sl2e) & _PAGE_PRESENT);// NB: this is stored as a machine address. entry->writable_pl1e = - ((sl2e &PAGE_MASK ) | - (sizeof (l1_pgentry_t) *l1_table_offset (va))); +l2e_get_phys (sl2e) | (sizeof (l1_pgentry_t) *l1_table_offset (va));ASSERT ( !(entry->writable_pl1e & (sizeof (l1_pgentry_t)-1 )) );// Increment shadow's page count to represent the reference // inherent in entry->writable_pl1e // -if ( !get_shadow_ref (sl2e >>PAGE_SHIFT ) ) +if ( !get_shadow_ref (l2e_get_pfn (sl2e)) )BUG (); FSH_LOG("mark_out_of_sync(va=%p -> writable_pl1e=%p)",
@@ -1784,7 +1780,7 @@ int __shadow_out_of_sync(struct exec_dom {struct domain *d = ed->domain;unsigned long l2mfn =pagetable_val (ed->arch.guest_table) >>PAGE_SHIFT ; -unsigned long l2e; +l2_pgentry_t l2e;unsigned long l1mfn;ASSERT (spin_is_locked (&d->arch.shadow_lock));
@@ -1796,10 +1792,10 @@ int __shadow_out_of_sync(struct exec_domreturn 1 ;__guest_get_l2e (ed, va, &l2e); -if ( !(l2e & _PAGE_PRESENT) ) +if ( !(l2e_get_flags (l2e) & _PAGE_PRESENT) )return 0 ; - l1mfn =__gpfn_to_mfn (d, l2e >>PAGE_SHIFT ); + l1mfn =__gpfn_to_mfn (d,l2e_get_pfn (l2e));// If the l1 pfn is invalid, it can't be out of sync... if ( !VALID_MFN (l1mfn) )
@@ -1866,31 +1862,31 @@ static u32 remove_all_write_access_in_pt unsigned long readonly_gpfn, unsigned long readonly_gmfn, u32 max_refs_to_find, unsigned long prediction) { - unsigned long *pt = map_domain_mem(pt_mfn << PAGE_SHIFT); -unsigned long match = - (readonly_gmfn <<PAGE_SHIFT ) | _PAGE_RW | _PAGE_PRESENT; -unsigned long mask =PAGE_MASK | _PAGE_RW | _PAGE_PRESENT; + l1_pgentry_t *pt = map_domain_mem(pt_mfn << PAGE_SHIFT); +l1_pgentry_t match; +unsigned long flags = _PAGE_RW | _PAGE_PRESENT;int i;u32 found =0 ;int is_l1_shadow = ((frame_table[pt_mfn].u.inuse.type_info & PGT_type_mask) == PGT_l1_shadow); -#define MATCH_ENTRY (_i) (((pt[_i] ^ match) & mask) == 0) +l1e_init_pfn (match, readonly_gmfn, flags);// returns true if all refs have been found and fixed. // int fix_entry (int i) { -unsigned long old = pt[i]; -unsigned long new = old & ~_PAGE_RW; +l1_pgentry_t old = pt[i]; +l1_pgentry_t new = old; -if ( is_l1_shadow && !shadow_get_page_from_l1e (mk_l1_pgentry (new), d) ) +l1e_remove_flags (new,_PAGE_RW); +if ( is_l1_shadow && !shadow_get_page_from_l1e (new, d) )BUG (); found++; pt[i] = new;if ( is_l1_shadow ) -put_page_from_l1e (mk_l1_pgentry (old), d); +put_page_from_l1e (old, d);#if 0 printk("removed write access to pfn=%p mfn=%p in smfn=%p entry %x "
@@ -1901,8 +1897,8 @@ static u32 remove_all_write_access_in_ptreturn (found == max_refs_to_find); } -if (MATCH_ENTRY (readonly_gpfn & (L1_PAGETABLE_ENTRIES -1 )) && -fix_entry (readonly_gpfn & (L1_PAGETABLE_ENTRIES -1 )) ) + i = readonly_gpfn & (L1_PAGETABLE_ENTRIES -1 ); +if ( !l1e_has_changed (pt[i], match, flags) &&fix_entry (i) ) {perfc_incrc (remove_write_fast_exit);increase_writable_pte_prediction (d, readonly_gpfn, prediction);
@@ -1912,7 +1908,7 @@ static u32 remove_all_write_access_in_ptfor (i =0 ; i <L1_PAGETABLE_ENTRIES ; i++) { -if (unlikely (MATCH_ENTRY (i)) &&fix_entry (i) ) +if (unlikely (!l1e_has_changed (pt[i], match, flags)) &&fix_entry (i) )break ; }
@@ -2005,25 +2001,27 @@ int shadow_remove_all_write_access(static u32 remove_all_access_in_page (struct domain *d,unsigned long l1mfn,unsigned long forbidden_gmfn) { -unsigned long *pl1e =map_domain_mem (l1mfn <<PAGE_SHIFT ); -unsigned long match = (forbidden_gmfn <<PAGE_SHIFT ) | _PAGE_PRESENT; -unsigned long mask =PAGE_MASK | _PAGE_PRESENT; +l1_pgentry_t *pl1e =map_domain_mem (l1mfn <<PAGE_SHIFT ); +l1_pgentry_t match; +unsigned long flags = _PAGE_PRESENT;int i;u32 count =0 ;int is_l1_shadow = ((frame_table[l1mfn].u.inuse.type_info & PGT_type_mask) == PGT_l1_shadow); +l1e_init_pfn (match, forbidden_gmfn, flags); +for (i =0 ; i <L1_PAGETABLE_ENTRIES ; i++) { -if (unlikely (((pl1e[i] ^ match) & mask) ==0 ) ) +if (unlikely (!l1e_has_changed (pl1e[i], match, flags) ==0 ) ) { -unsigned long ol2e = pl1e[i]; - pl1e[i] =0 ; +l1_pgentry_t ol2e = pl1e[i]; +l1e_clear (pl1e[i]); count++;if ( is_l1_shadow ) -put_page_from_l1e (mk_l1_pgentry (ol2e), d); +put_page_from_l1e (ol2e, d);else /* must be an hl2 page */ put_page (&frame_table[forbidden_gmfn]); }
@@ -2076,7 +2074,7 @@ static int resync_all(struct domain *d,struct out_of_sync_entry *entry;unsigned i;unsigned long smfn; -unsigned long *guest, *shadow, *snapshot; +void *guest, *shadow, *snapshot;int need_flush =0 , external =shadow_mode_external (d);int unshadow;int changed;
@@ -2114,14 +2112,18 @@ static int resync_all(struct domain *d,int min_snapshot =SHADOW_MIN (min_max_snapshot);int max_snapshot =SHADOW_MAX (min_max_snapshot); +l1_pgentry_t *guest1 = guest; +l1_pgentry_t *shadow1 = shadow; +l1_pgentry_t *snapshot1 = snapshot; + changed =0 ;for ( i = min_shadow; i <= max_shadow; i++ ) {if ( (i < min_snapshot) || (i > max_snapshot) || - (guest[i] != snapshot[i]) ) +l1e_has_changed (guest1[i], snapshot1[i],PAGE_FLAG_MASK ) ) { - need_flush |=validate_pte_change (d, guest[i], &shadow[i]); + need_flush |=validate_pte_change (d, guest1[i], &shadow1[i]);// can't update snapshots of linear page tables -- they // are used multiple times...
@@ -2140,16 +2142,20 @@ static int resync_all(struct domain *d, {int max =-1 ; +l2_pgentry_t *guest2 = guest; +l2_pgentry_t *shadow2 = shadow; +l2_pgentry_t *snapshot2 = snapshot; + changed =0 ;for ( i =0 ; i <L2_PAGETABLE_ENTRIES ; i++ ) {if ( !is_guest_l2_slot (i) && !external )continue ; -unsigned long new_pde = guest[i]; -if ( new_pde != snapshot[i] ) +l2_pgentry_t new_pde = guest2[i]; +if (l2e_has_changed (new_pde, snapshot2[i],PAGE_FLAG_MASK )) { - need_flush |=validate_pde_change (d, new_pde, &shadow[i]); + need_flush |=validate_pde_change (d, new_pde, &shadow2[i]);// can't update snapshots of linear page tables -- they // are used multiple times...
@@ -2158,12 +2164,13 @@ static int resync_all(struct domain *d, changed++; } -if ( new_pde !=0 ) +if (l2e_get_value (new_pde) !=0 )/* FIXME: check flags? */ max = i;// XXX - This hack works for linux guests. // Need a better solution long term. -if ( !(new_pde & _PAGE_PRESENT) &&unlikely (new_pde !=0 ) && +if ( !(l2e_get_flags (new_pde) & _PAGE_PRESENT) && +unlikely (l2e_get_value (new_pde) !=0 ) && !unshadow && (frame_table[smfn].u.inuse.type_info & PGT_pinned) ) unshadow =1 ;
@@ -2175,16 +2182,21 @@ static int resync_all(struct domain *d,break ; }case PGT_hl2_shadow: + { +l2_pgentry_t *guest2 = guest; +l2_pgentry_t *shadow2 = shadow; +l2_pgentry_t *snapshot2 = snapshot; + changed =0 ;for ( i =0 ; i <L2_PAGETABLE_ENTRIES ; i++ ) {if ( !is_guest_l2_slot (i) && !external )continue ; -unsigned long new_pde = guest[i]; -if ( new_pde != snapshot[i] ) +l2_pgentry_t new_pde = guest2[i]; +if (l2e_has_changed (new_pde, snapshot2[i],PAGE_FLAG_MASK ) ) { - need_flush |=validate_hl2e_change (d, new_pde, &shadow[i]); + need_flush |=validate_hl2e_change (d, new_pde, &shadow2[i]);// can't update snapshots of linear page tables -- they // are used multiple times...
@@ -2197,6 +2209,7 @@ static int resync_all(struct domain *d,perfc_incrc (resync_hl2);perfc_incr_histo (shm_hl2_updates, changed,PT_UPDATES );break ; + }default :BUG (); }
@@ -2234,15 +2247,16 @@ void __shadow_sync_all(struct domain *d)if ( entry->writable_pl1e & (sizeof (l1_pgentry_t)-1 ) )continue ; -unsigned long *ppte =map_domain_mem (entry->writable_pl1e); -unsigned long opte = *ppte; -unsigned long npte = opte & ~_PAGE_RW; +l1_pgentry_t *ppte =map_domain_mem (entry->writable_pl1e); +l1_pgentry_t opte = *ppte; +l1_pgentry_t npte = opte; +l1e_remove_flags (opte, _PAGE_RW); -if ( (npte & _PAGE_PRESENT) && - !shadow_get_page_from_l1e (mk_l1_pgentry (npte), d) ) +if ( (l1e_get_flags (npte) & _PAGE_PRESENT) && + !shadow_get_page_from_l1e (npte, d) )BUG (); *ppte = npte; -put_page_from_l1e (mk_l1_pgentry (opte), d); +put_page_from_l1e (opte, d);unmap_domain_mem (ppte); }
@@ -2277,10 +2291,12 @@ void __shadow_sync_all(struct domain *d)int shadow_fault (unsigned long va,struct xen_regs *regs) { -unsigned long gpte, spte =0 , orig_gpte; +l1_pgentry_t gpte, spte, orig_gpte;struct exec_domain *ed = current;struct domain *d = ed->domain; -unsigned long gpde; +l2_pgentry_t gpde; + +l1e_clear (spte);SH_VVLOG ("shadow_fault( va=%p, code=%lu )" , va, regs->error_code );perfc_incrc (shadow_fault_calls);
@@ -2303,7 +2319,7 @@ int shadow_fault(unsigned long va, struc* STEP 2. Check the guest PTE. */ __guest_get_l2e (ed, va, &gpde); -if (unlikely (!(gpde & _PAGE_PRESENT)) ) +if (unlikely (!(l1e_get_flags (gpte) & _PAGE_PRESENT)) ) {SH_VVLOG ("shadow_fault - EXIT: L1 not present" );perfc_incrc (shadow_fault_bail_pde_not_present);
@@ -2314,8 +2330,8 @@ int shadow_fault(unsigned long va, struc// the mapping is in-sync, so the check of the PDE's present bit, above, // covers this access. // - orig_gpte = gpte =l1_pgentry_val (linear_pg_table[l1_linear_offset (va)]); -if (unlikely (!(gpte & _PAGE_PRESENT)) ) + orig_gpte = gpte = linear_pg_table[l1_linear_offset (va)]; +if (unlikely (!(l1e_get_flags (gpte) & _PAGE_PRESENT)) ) {SH_VVLOG ("shadow_fault - EXIT: gpte not present (%lx)" ,gpte );perfc_incrc (shadow_fault_bail_pte_not_present);
@@ -2325,7 +2341,7 @@ int shadow_fault(unsigned long va, struc/* Write fault? */ if ( regs->error_code &2 ) { -if (unlikely (!(gpte & _PAGE_RW)) ) +if (unlikely (!(l1e_get_flags (gpte) & _PAGE_RW)) ) {/* Write fault on a read-only mapping. */ SH_VVLOG ("shadow_fault - EXIT: wr fault on RO page (%lx)" , gpte);
@@ -2357,8 +2373,8 @@ int shadow_fault(unsigned long va, struc*/ /* XXX Watch out for read-only L2 entries! (not used in Linux). */ -if (unlikely (__put_user (gpte, (unsigned long *) - &linear_pg_table[l1_linear_offset (va)])) ) +if (unlikely (__copy_to_user (&linear_pg_table[l1_linear_offset (va)], + &gpte,sizeof (gpte))) ) { printk("shadow_fault() failed, crashing domain %d " "due to a read-only L2 page table (gpde=%p), va=%p\n",
@@ -2367,8 +2383,9 @@ int shadow_fault(unsigned long va, struc }// if necessary, record the page table page as dirty -if (unlikely (shadow_mode_log_dirty (d)) && (orig_gpte != gpte) ) -mark_dirty (d,__gpfn_to_mfn (d, gpde >>PAGE_SHIFT )); +if (unlikely (shadow_mode_log_dirty (d)) && +l1e_has_changed (orig_gpte, gpte,PAGE_FLAG_MASK )) +mark_dirty (d,__gpfn_to_mfn (d,l2e_get_pfn (gpde)));shadow_set_l1e (va, spte,1 );
@@ -2490,11 +2507,11 @@ void __update_pagetables(struct exec_domBUG ();// ref counts for hl2mfn and smfn need to be maintained! - mpl2e[l2_table_offset (LINEAR_PT_VIRT_START )] = -mk_l2_pgentry ((hl2mfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); +l2e_init_pfn (mpl2e[l2_table_offset (LINEAR_PT_VIRT_START )], + hl2mfn, __PAGE_HYPERVISOR); - mpl2e[l2_table_offset (SH_LINEAR_PT_VIRT_START )] = -mk_l2_pgentry ((smfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); +l2e_init_pfn (mpl2e[l2_table_offset (SH_LINEAR_PT_VIRT_START )], + smfn, __PAGE_HYPERVISOR);// XXX - maybe this can be optimized somewhat?? local_flush_tlb ();
@@ -2515,10 +2532,9 @@ char * sh_check_name;int shadow_status_noswap;#define v2m (adr) ({ \ - unsigned long _a = (unsigned long)(adr); \ - unsigned long _pte = l1_pgentry_val( \ - shadow_linear_pg_table[_a >> PAGE_SHIFT]); \ - unsigned long _pa = _pte & PAGE_MASK; \ + unsigned long _a = (unsigned long)(adr); \ + l1_pgentry_t _pte = shadow_linear_pg_table[_a >> PAGE_SHIFT]; \ + unsigned long _pa = l1e_get_phys(_pte); \ _pa | (_a & ~PAGE_MASK); \ })
@@ -2536,49 +2552,55 @@ int shadow_status_noswap; }while (0 ) static int check_pte( -struct domain *d,unsigned long *pgpte,unsigned long *pspte, +struct domain *d,l1_pgentry_t *pgpte,l1_pgentry_t *pspte,int level,int l2_idx,int l1_idx,int oos_ptes) { -unsigned gpte = *pgpte; -unsigned spte = *pspte; +l1_pgentry_t gpte = *pgpte; +l1_pgentry_t spte = *pspte;unsigned long mask, gpfn, smfn, gmfn;int errors =0 ;int page_table_page; -if ( (spte ==0 ) || (spte ==0xdeadface ) || (spte ==0x00000E00 ) ) +if ( (l1e_get_value (spte) ==0 ) || + (l1e_get_value (spte) ==0xdeadface ) || + (l1e_get_value (spte) ==0x00000E00 ) )return errors;/* always safe */ -if ( !(spte & _PAGE_PRESENT) ) +if ( !(l1e_get_flags (spte) & _PAGE_PRESENT) )FAIL ("Non zero not present spte" );if ( level ==2 ) sh_l2_present++;if ( level ==1 ) sh_l1_present++; -if ( !(gpte & _PAGE_PRESENT) ) +if ( !(l1e_get_flags (gpte) & _PAGE_PRESENT) )FAIL ("Guest not present yet shadow is" ); - mask = ~(_PAGE_DIRTY|_PAGE_ACCESSED|_PAGE_RW|PAGE_MASK ); + mask = ~(_PAGE_DIRTY|_PAGE_ACCESSED|_PAGE_RW); -if ( (spte & mask) != (gpte & mask) ) +if (l1e_has_changed (spte, gpte, mask) )FAIL ("Corrupt?" );if ( (level ==1 ) && - (spte & _PAGE_DIRTY ) && !(gpte & _PAGE_DIRTY) && !oos_ptes ) + (l1e_get_flags (spte) & _PAGE_DIRTY ) && + !(l1e_get_flags (gpte) & _PAGE_DIRTY) && !oos_ptes )FAIL ("Dirty coherence" ); -if ( (spte & _PAGE_ACCESSED ) && !(gpte & _PAGE_ACCESSED) && !oos_ptes ) +if ( (l1e_get_flags (spte) & _PAGE_ACCESSED ) && + !(l1e_get_flags (gpte) & _PAGE_ACCESSED) && !oos_ptes )FAIL ("Accessed coherence" ); - smfn = spte >>PAGE_SHIFT ; - gpfn = gpte >>PAGE_SHIFT ; + smfn =l1e_get_pfn (spte); + gpfn =l1e_get_pfn (gpte); gmfn =__gpfn_to_mfn (d, gpfn);if ( !VALID_MFN (gmfn) ) -FAIL ("invalid gpfn=%p gpte=%p\n" , __func__, gpfn, gpte); +FAIL ("invalid gpfn=%p gpte=%p\n" , __func__, gpfn, +l1e_get_value (gpte)); page_table_page =mfn_is_page_table (gmfn); -if ( (spte & _PAGE_RW ) && !(gpte & _PAGE_RW) && !oos_ptes ) +if ( (l1e_get_flags (spte) & _PAGE_RW ) && + !(l1e_get_flags (gpte) & _PAGE_RW) && !oos_ptes ) { printk("gpfn=%p gmfn=%p smfn=%p t=0x%08x page_table_page=%d " "oos_ptes=%d\n",
@@ -2589,8 +2611,9 @@ static int check_pte( }if ( (level ==1 ) && - (spte & _PAGE_RW ) && - !((gpte & _PAGE_RW) && (gpte & _PAGE_DIRTY)) && + (l1e_get_flags (spte) & _PAGE_RW ) && + !((l1e_get_flags (gpte) & _PAGE_RW) && + (l1e_get_flags (gpte) & _PAGE_DIRTY)) && !oos_ptes ) { printk("gpfn=%p gmfn=%p smfn=%p t=0x%08x page_table_page=%d "
@@ -2629,7 +2652,7 @@ static int check_l1_table( unsigned long gmfn, unsigned long smfn, unsigned l2_idx) { int i; -unsigned long *gpl1e, *spl1e; +l1_pgentry_t *gpl1e, *spl1e;int errors =0 , oos_ptes =0 ;// First check to see if this guest page is currently the active
@@ -2670,6 +2693,7 @@ int check_l2_table( {l2_pgentry_t *gpl2e = (l2_pgentry_t *)map_domain_mem (gmfn <<PAGE_SHIFT );l2_pgentry_t *spl2e = (l2_pgentry_t *)map_domain_mem (smfn <<PAGE_SHIFT ); +l2_pgentry_t match;int i;int errors =0 ;int limit;
@@ -2701,25 +2725,26 @@ int check_l2_table(FAILPT ("hypervisor linear map inconsistent" );#endif +l2e_init_pfn (match, smfn, __PAGE_HYPERVISOR);if ( !shadow_mode_external (d) && - (l2_pgentry_val (spl2e[SH_LINEAR_PT_VIRT_START >> -L2_PAGETABLE_SHIFT ]) != - ((smfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR)) ) +l2e_has_changed (spl2e[SH_LINEAR_PT_VIRT_START >>L2_PAGETABLE_SHIFT ], + match,PAGE_FLAG_MASK )) {FAILPT ("hypervisor shadow linear map inconsistent %p %p" , -l2_pgentry_val (spl2e[SH_LINEAR_PT_VIRT_START >> -L2_PAGETABLE_SHIFT ]), - (smfn <<PAGE_SHIFT ) | __PAGE_HYPERVISOR); +l2e_get_value (spl2e[SH_LINEAR_PT_VIRT_START >> +L2_PAGETABLE_SHIFT ]), +l2e_get_value (match)); } +l2e_init_phys (match,__pa (d->arch.mm_perdomain_pt), __PAGE_HYPERVISOR);if ( !shadow_mode_external (d) && - (l2_pgentry_val (spl2e[PERDOMAIN_VIRT_START >>L2_PAGETABLE_SHIFT ]) != - ((__pa (d->arch.mm_perdomain_pt) | __PAGE_HYPERVISOR))) ) +l2e_has_changed (spl2e[PERDOMAIN_VIRT_START >>L2_PAGETABLE_SHIFT ], + match,PAGE_FLAG_MASK )) {FAILPT ("hypervisor per-domain map inconsistent saw %p, expected (va=%p) %p" , -l2_pgentry_val (spl2e[PERDOMAIN_VIRT_START >>L2_PAGETABLE_SHIFT ]), +l2e_get_value (spl2e[PERDOMAIN_VIRT_START >>L2_PAGETABLE_SHIFT ]), d->arch.mm_perdomain_pt, - (__pa (d->arch.mm_perdomain_pt) | __PAGE_HYPERVISOR)); +l2e_get_value (match)); }#ifdef __i386__
@@ -2733,7 +2758,10 @@ int check_l2_table(/* Check the whole L2. */ for ( i =0 ; i < limit; i++ ) - errors +=check_pte (d, &l2_pgentry_val (gpl2e[i]), &l2_pgentry_val (spl2e[i]),2 , i,0 ,0 ); + errors +=check_pte (d, + (l1_pgentry_t *)(&gpl2e[i]),/* Hmm, dirty ... */ + (l1_pgentry_t *)(&spl2e[i]), +2 , i,0 ,0 );unmap_domain_mem (spl2e);unmap_domain_mem (gpl2e);
@@ -2798,11 +2826,11 @@ int _check_pagetable(struct exec_domainfor ( i =0 ; i < limit; i++ ) { -unsigned long gl1pfn =l2_pgentry_val (gpl2e[i]) >>PAGE_SHIFT ; +unsigned long gl1pfn =l2e_get_pfn (gpl2e[i]);unsigned long gl1mfn =__gpfn_to_mfn (d, gl1pfn); -unsigned long sl1mfn =l2_pgentry_val (spl2e[i]) >>PAGE_SHIFT ; +unsigned long sl1mfn =l2e_get_pfn (spl2e[i]); -if (l2_pgentry_val (spl2e[i]) !=0 ) +if (l2e_get_value (spl2e[i]) !=0 )/* FIXME: check flags? */ { errors +=check_l1_table (d, gl1pfn, gl1mfn, sl1mfn, i); }
@@ -2897,3 +2925,4 @@ int _check_all_pagetables(struct exec_do* indent-tabs-mode: nil * End: */ +
include/asm-x86/shadow.h +148 -136
558 lines not on this page · raw message
arch/x86/vmx.c +4 -3
@@ -109,7 +109,8 @@ static int vmx_do_page_fault(unsigned lo {struct exec_domain *ed = current;unsigned long eip; -unsigned long gpte, gpa; +l1_pgentry_t gpte; +unsigned long gpa;/* FIXME: PAE */ int result;#if VMX_DEBUG
@@ -132,9 +133,9 @@ static int vmx_do_page_fault(unsigned lo } gpte =gva_to_gpte (va); -if (!(gpte & _PAGE_PRESENT) ) +if (!(l1e_get_flags (gpte) & _PAGE_PRESENT) )return 0 ; - gpa = (gpte &PAGE_MASK ) + (va & ~PAGE_MASK ); + gpa =l1e_get_phys (gpte) + (va & ~PAGE_MASK );/* Use 1:1 page table to identify MMIO address space */ if (mmio_space (gpa))
arch/x86/mm.c +112 -113
590 lines not on this page · raw message
arch/x86/vmx_platform.c +2 -2
@@ -408,7 +408,7 @@ static int vmx_decode(const unsigned chastatic int inst_copy_from_guest (unsigned char *buf,unsigned long guest_eip,int inst_len) { -unsigned long gpte; +l1_pgentry_t gpte;unsigned long mfn;unsigned long ma;unsigned char * inst_start;
@@ -419,7 +419,7 @@ static int inst_copy_from_guest(unsignedif ((guest_eip &PAGE_MASK ) == ((guest_eip + inst_len) &PAGE_MASK )) { gpte =gva_to_gpte (guest_eip); - mfn =phys_to_machine_mapping (gpte >>PAGE_SHIFT ); + mfn =phys_to_machine_mapping (l1e_get_pfn (gpte)); ma = (mfn <<PAGE_SHIFT ) | (guest_eip & (PAGE_SIZE -1 )); inst_start = (unsigned char *)map_domain_mem (ma);
include/asm-x86/x86_32/page.h +35 -20
75 lines not on this page · raw message
include/asm-x86/mm.h +9 -8
@@ -263,13 +263,14 @@ static inline unsigned long phys_to_machunsigned long mfn;l1_pgentry_t pte; -if ( !__get_user (l1_pgentry_val (pte), (__phys_to_machine_mapping + pfn)) && - (l1_pgentry_val (pte) & _PAGE_PRESENT) ) - mfn =l1_pgentry_to_phys (pte) >>PAGE_SHIFT ; -else - mfn =INVALID_MFN ; - -return mfn; +if (!__copy_from_user (&pte, (__phys_to_machine_mapping + pfn), +sizeof (pte)) + && (l1e_get_flags (pte) & _PAGE_PRESENT) ) + mfn =l1e_get_pfn (pte); +else + mfn =INVALID_MFN ; + +return mfn; }#define set_machinetophys (_mfn, _pfn) machine_to_phys_mapping[(_mfn)] = (_pfn)
@@ -352,7 +353,7 @@ void propagate_page_fault(unsigned long* hold a reference to the page. */ int update_grant_va_mapping (unsigned long va, -unsigned long val, +l1_pgentry_t _nl1e,struct domain *d,struct exec_domain *ed);#endif /* __ASM_X86_MM_H__ */
arch/x86/domain_build.c +24 -13
@@ -243,10 +243,10 @@ int construct_dom0(struct domain *d,/* WARNING: The new domain must have its 'processor' field filled in! */ l2start = l2tab = (l2_pgentry_t *)mpt_alloc; mpt_alloc +=PAGE_SIZE ;memcpy (l2tab, &idle_pg_table[0 ],PAGE_SIZE ); - l2tab[LINEAR_PT_VIRT_START >>L2_PAGETABLE_SHIFT ] = -mk_l2_pgentry ((unsigned long )l2start | __PAGE_HYPERVISOR); - l2tab[PERDOMAIN_VIRT_START >>L2_PAGETABLE_SHIFT ] = -mk_l2_pgentry (__pa (d->arch.mm_perdomain_pt) | __PAGE_HYPERVISOR); +l2e_init_phys (l2tab[LINEAR_PT_VIRT_START >>L2_PAGETABLE_SHIFT ], + (unsigned long )l2start, __PAGE_HYPERVISOR); +l2e_init_phys (l2tab[PERDOMAIN_VIRT_START >>L2_PAGETABLE_SHIFT ], +__pa (d->arch.mm_perdomain_pt), __PAGE_HYPERVISOR); ed->arch.guest_table =mk_pagetable ((unsigned long )l2start); l2tab +=l2_table_offset (dsi.v_start);
@@ -257,12 +257,14 @@ int construct_dom0(struct domain *d, { l1start = l1tab = (l1_pgentry_t *)mpt_alloc; mpt_alloc +=PAGE_SIZE ; - *l2tab++ =mk_l2_pgentry ((unsigned long )l1start |L2_PROT ); +l2e_init_phys (*l2tab, (unsigned long )l1start,L2_PROT ); + l2tab++;clear_page (l1tab);if ( count ==0 ) l1tab +=l1_table_offset (dsi.v_start); } - *l1tab++ =mk_l1_pgentry ((mfn <<PAGE_SHIFT ) |L1_PROT ); +l1e_init_pfn (*l1tab, mfn,L1_PROT ); + l1tab++; page = &frame_table[mfn];if ( !get_page_and_type (page, d, PGT_writable_page) )
@@ -273,13 +275,13 @@ int construct_dom0(struct domain *d,/* Pages that are part of page tables must be read only. */ l2tab = l2start +l2_table_offset (vpt_start); - l1start = l1tab = (l1_pgentry_t *)l2_pgentry_to_phys (*l2tab); + l1start = l1tab = (l1_pgentry_t *)l2e_get_phys (*l2tab); l1tab +=l1_table_offset (vpt_start);for ( count =0 ; count < nr_pt_pages; count++ ) { - page = &frame_table[l1_pgentry_to_pfn (*l1tab)]; + page = &frame_table[l1e_get_pfn (*l1tab)];if ( !opt_dom0_shadow ) - *l1tab =mk_l1_pgentry (l1_pgentry_val (*l1tab) & ~_PAGE_RW); +l1e_remove_flags (*l1tab, _PAGE_RW);else if ( !get_page_type (page, PGT_writable_page) )BUG ();
@@ -317,7 +319,7 @@ int construct_dom0(struct domain *d, get_page(page, d); /* an extra ref because of readable mapping */ }if ( !((unsigned long )++l1tab & (PAGE_SIZE -1 )) ) - l1start = l1tab = (l1_pgentry_t *)l2_pgentry_to_phys (*++l2tab); + l1start = l1tab = (l1_pgentry_t *)l2e_get_phys (*++l2tab); }#elif defined(__x86_64__)
@@ -525,8 +527,8 @@ int construct_dom0(struct domain *d,#if defined(__i386__)/* Destroy low mappings - they were only for our convenience. */ for ( i =0 ; i <DOMAIN_ENTRIES_PER_L2_PAGETABLE ; i++ ) -if (l2_pgentry_val (l2start[i]) & _PAGE_PSE ) - l2start[i] =mk_l2_pgentry (0 ); +if (l2e_get_flags (l2start[i]) & _PAGE_PSE ) +l2e_clear (l2start[i]);zap_low_mappings ();/* Do the same for the idle page tables. */ #endif
@@ -544,10 +546,18 @@ int construct_dom0(struct domain *d, : SHM_enable));if ( opt_dom0_translate ) { +#if 1 +printk ("FIXME: %s:%d\n" ,__FUNCTION__,__LINE__); +for ( ; ; ) + __asm__ __volatile__ ("hlt" ); +#else +/* Hmm, what does this? +Looks like isn't portable across 32/64 bit and pae/non-pae ... +-- kraxel */ // map this domain's p2m table into current page table, // so that we can easily access it. // -ASSERT (root_pgentry_val (idle_pg_table[1 ]) ==0 ); +ASSERT (root_get_value (idle_pg_table[1 ]) ==0 );ASSERT (pagetable_val (d->arch.phys_table) ); idle_pg_table[1 ] =mk_root_pgentry (pagetable_val (d->arch.phys_table) | __PAGE_HYPERVISOR);
@@ -556,6 +566,7 @@ int construct_dom0(struct domain *d, >> PAGE_SHIFT); idle_pg_table[1 ] =mk_root_pgentry (0 );local_flush_tlb (); +#endif }update_pagetables (ed);/* XXX SMP */
arch/x86/dom0_ops.c +1 -1
@@ -425,7 +425,7 @@ void arch_getdomaininfo_ctxt( {for ( i =0 ; i <16 ; i++ ) c->gdt_frames[i] = -l1_pgentry_to_pfn (ed->arch.perdomain_ptes[i]); +l1e_get_pfn (ed->arch.perdomain_ptes[i]); c->gdt_ents =GET_GDT_ENTRIES (ed); } c->kernel_ss = ed->arch.kernel_ss;
include/asm-x86/page.h +1 -1
@@ -77,7 +77,7 @@ typedef struct { unsigned long pt_lo; }#define linear_l4_table (_ed) ((_ed)->arch.guest_vl4table)#define va_to_l1mfn (_ed, _va) \ - (l2_pgentry_val(linear_l2_table(_ed)[_va>>L2_PAGETABLE_SHIFT]) >> PAGE_SHIFT) + (l2e_get_pfn(linear_l2_table(_ed)[_va>>L2_PAGETABLE_SHIFT]))extern root_pgentry_t idle_pg_table[ROOT_PAGETABLE_ENTRIES ];
arch/x86/x86_32/traps.c +1 -1
@@ -163,7 +163,7 @@ void show_page_walk(unsigned long addr)printk ("Pagetable walk from %p:\n" , addr); - page =l2_pgentry_val (idle_pg_table[l2_table_offset (addr)]); + page =l2e_get_value (idle_pg_table[l2_table_offset (addr)]);printk (" L2 = %p %s\n" , page, (page & _PAGE_PSE) ?"(4MB)" :"" );if ( !(page & _PAGE_PRESENT) || (page & _PAGE_PSE) )return ;
arch/x86/x86_32/mm.c +34 -36
146 lines not on this page · raw message
common/grant_table.c +5 -5
@@ -253,12 +253,12 @@ __gnttab_activate_grant_ref( {/* Write update into the pagetable */ +l1_pgentry_t pte; - rc =update_grant_va_mapping ( host_virt_addr, - (frame <<PAGE_SHIFT ) | _PAGE_PRESENT | - _PAGE_ACCESSED | - _PAGE_DIRTY | - ((dev_hst_ro_flags & GNTMAP_readonly) ?0 : _PAGE_RW), +l1e_init_pfn (pte, frame, _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_DIRTY); +if (!(dev_hst_ro_flags & GNTMAP_readonly)) +l1e_add_flags (pte,_PAGE_RW); + rc =update_grant_va_mapping ( host_virt_addr, pte, mapping_d, mapping_ed );/* IMPORTANT: (rc == 0) => must flush / invalidate entry in TLB.
include/asm-x86/domain.h +1 -1
@@ -130,7 +130,7 @@ struct arch_exec_domainl2_pgentry_t *guest_vtable;/* virtual address of pagetable */ l2_pgentry_t *shadow_vtable;/* virtual address of shadow_table */ l2_pgentry_t *monitor_vtable;/* virtual address of monitor_table */ -l1_pgentry_t *hl2_vtable;/* virtual address of hl2_table */ +l2_pgentry_t *hl2_vtable;/* virtual address of hl2_table */ #ifdef __x86_64__l3_pgentry_t *guest_vl3table;
Overall, I think the patch looks pretty good...
A couple of comments:
1) There's no Signed-Off-By comment attached to this. Could you please provide one?
2) About your question at the bottom of construct_dom0:
The current code there is intended to allow booting of dom0's with
translate mode
enabled. As such, it probably won't stay in the code base forever, but
it was and
is a useful hack. The bringup process for this new shadow code was to
get dom0's
working first, and we're now working on (cleaner) domU support. I'm not
too
worried about x86_64 or pae support for this dom0 translate mode support.
I'd
like to just remove the halt you added there, OK?
3) HL2 tables are not tables of L2 entries. They contain L1 entries. They are essentially shadows of guest L2 pages, which will be used by Xen
to get
a linear-pagetable-like mapping of the guest's L1 pages.
A normal guest L2 has guest-physical pages in it.
A normal shadow L2 points at shadows of the guest's L1s.
An HL2 has machine addresses of the guest's L1 pages in it, and is *used*
as an
L1 table by Xen. So the things like "l1_pgentry_t *hl2_vtable" indomain.h were
not typos, and should stay the way they were...
The HL2 is a concept that works well for the simple 2 level page tables,
and is a
clever solution to avoid doing lots of extra map_domain_mem() calls to
access the
guest's L1 tables, but it both falls apart and is fortunately unnecessary
for
64-bit mode. I haven't thought much about PAE yet, but HL2 are probably still useful there because of the cost of map_domain_mem()...
4) There was probably a bunch of debate about this somewhere before, but I missed it.
The macros which set/clear page table types don't obey C's pass-by-value
calling
semantics. That means that they can't be replaced with simple functions,
if
desired -- there would always have to be a macro layer. There's also no
macros
for creating L1E or L2E as expressions -- only statements which assign
them.
Perhaps this was intentional? It means that you end up declaring extra variables to hold essentially temporary values in a few places...
Comments?
5) I found a couple compilation problems when by compiling with debug=y...
I've dealt with issues #2, 3, and 5 in a slightly modified version of your patch,
available at http://www.cl.cam.ac.uk/~maf46/kraxel.patch.v2
Take a look, and let me know what you think.
Michael
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Gerd Knorr
Sent: Tuesday, April 12, 2005 7:59 PM
To: [email protected]
Subject: [Xen-devel] [patch] pagetable cleanups
Hi,
Next version of the pagetable cleanup patch. Builds and boots domain 0 on x86_32. Changes from the last version:
* macro names are changed.
* adapted to the new shadow code checked in last week.
* new macro: l1e_has_changed() to compare page table entries.
Open issues:
* I'm not sure how to handle the debug printk's best. These use the l1e_get_value() macro at the moment to get the raw bits and print them as unsigned long hex value. I'd like to get rid of the l1e_get_value() macro altogether though ...
* x86_64 build needs fixing, will look into this tomorrow.
Enjoy,
Gerd
Gerd Knorr
AppliedQuoted 6 lines · Michael A Fetterman
Overall, I think the patch looks pretty good...
A couple of comments:
1) There's no Signed-Off-By comment attached to this. Could you please provide one?
Yes, thats easy ;)
Quoted 4 lines
2) About your question at the bottom of
construct_dom0: The current code there is intended to allow booting of dom0's with translate mode enabled. As such, it probably won't stay in the code base forever, but it was and is a useful hack.
Ah, ok.
Quoted 1 line
I'd like to just remove the halt you added there, OK?
Fine with me, I've added your changes instead.
Quoted 3 lines
3) HL2 tables are not tables of L2 entries. They contain L1 entries. They are essentially shadows of guest L2 pages, which will be used by Xen to get a linear-pagetable-like mapping of the guest's L1 pages.
Ah, *thats* the point of these beasts. The page manipulations done on them look like l2 operations (well, they actually are as they really point to l1 pages), that confused me ;)
Quoted 5 lines
4) There was probably a bunch of debate about this somewhere before, but I missed it. The macros which set/clear page table types don't obey C's pass-by-value calling semantics. That means that they can't be replaced with simple functions, if desired -- there would always have to be a macro layer.
Yep, I noticed that as well as the PAE versions became a bit more complex and I tried to make them inline functions instead which didn't work ...
I can change them to pass-by-reference instead, it's probably a good idea. Hope gcc is clever enougth to see that it's the same after all and doesn't generate extra code then.
Quoted 4 lines
There's also no macros for creating L1E or L2E as expressions -- only statements which assign them. Perhaps this was intentional? It means that you end up declaring extra variables to hold essentially temporary values in a few places...
Yes, was intentionally. I think that isn't bad, it makes the code more readable. And I think it actually is impossible to return structs in C, you can only return a pointer to a struct, which would't help for the "building entries as expressions" case.
Quoted 1 line
5) I found a couple compilation problems when by compiling with debug=y...
Merged, thanks.
Current patch set is at http://dl.bytesex.org/patches/xen-2/ now (issue #4 isn't adressed yet).
Gerd
Quoted 4 lines
Yes, was intentionally. I think that isn't bad, it makes the code more readable. And I think it actually is impossible to return structs in C, you can only return a pointer to a struct, which would't help for the "building entries as expressions" case.
Returning structs by values works with GCC. That said, I've never checked whether that's standard or a GCC extension. For big structs I think it transparently generates a memcpy, don't know what it generates for little ones.
Cheers,
Mark
Quoted 16 lines
Quoted 3 lines
5) I found a couple compilation problems when by compiling with debug=y...
Merged, thanks.
Current patch set is at http://dl.bytesex.org/patches/xen-2/ now (issue #4 isn't adressed yet).
Gerd
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 9 lines · Mark Williamson
Quoted 4 lines
Yes, was intentionally. I think that isn't bad, it makes the code more readable. And I think it actually is impossible to return structs in C, you can only return a pointer to a struct, which would't help for the "building entries as expressions" case.
Returning structs by values works with GCC. That said, I've never checked whether that's standard or a GCC extension. For big structs I think it transparently generates a memcpy, don't know what it generates for little ones.
Depends on the ABI.
For ppc64 Linux:
Aggregates or unions of any length, and character strings of length longer than 8 bytes, will be returned in a storage buffer allocated by the caller. The caller will pass the address of this buffer as a hidden first argument in r3, causing the first explicit argument to be passed in r4.
For ppc32 (System V R4 ABI) it's a little different for small structures:
A structure or union whose size is less than or equal to 8 bytes shall be returned in r3 and r4, as if it were first stored in an 8-byte aligned memory area and then the low-addressed word were loaded into r3 and the high-addressed word into r4.
Values of type long double and structures or unions that do not meet the requirements for being returned in registers are returned in a storage buffer allocated by the caller. The address of this buffer is passed as a hidden argument in r3 as if it were the first argument, causing gr in the argument passing algorithm above to be initialized to 4 instead of 3.
x86 (again from SVR4):
If a function returns a structure or union, then the caller provides space for the return value and places its address on the stack as argument word zero. In effect, this address becomes a ``hidden'' first argument.
So no memcpys at all; the caller fills in the values directly into the callee's struct.
Signature · 2 lines
Hollis Blanchard IBM Linux Technology Center
Quoted 2 lines · Hollis Blanchard
So no memcpys at all; the caller fills in the values directly into the callee's struct.
Unless you're dealing with struct assignments. You basically get a memcpy() when you assign structs. The standard makes it pretty clear that the returning a struct is not an lvalue (it's a copy). Unoptimized C will have an inlined memcpy() because of the assignment. Otherwise, the following:
foo(b, c).a = 3;
Would be valid and this is expressly forbidden by the standard (see section 6.5.2.3).
A smart compiler will propagate the copy but it has it's limitations. Once you have sufficiently complex pointer operations in a function there's no guarantee copies will be propagated correctly.
Because you cannot rely on the optimization, structs are not typically returned by value. Since this is so rarely done, I'd argue it's more readable to pass a struct as an argument (the fact that some people don't even realize it's possible should support that).
Regards,
Signature · 4 lines
Anthony Liguori Linux Technology Center (LTC) - IBM Austin E-mail: [email protected] Phone: (512) 838-1208
Quoted 6 lines · Anthony Liguori
Quoted 2 lines · Hollis Blanchard
So no memcpys at all; the caller fills in the values directly into the callee's struct.
Unless you're dealing with struct assignments. You basically get a
memcpy()when you assign structs.
I have attached my testcase. Compile with -c and disassemble the object file.
It is certainly possible that the callee could create a temporary struct on its stack and memcpy into the address provided by the ABI. However, even at -O0 the address of the struct is popped off the stack and used directly when assigning to structure fields. I'm using gcc 3.3.5.
Signature · 2 lines
Hollis Blanchard IBM Linux Technology Center
struct foo {
int i;
};extern int e;
struct foo bar(void)
{
struct foo ret;
ret.i = e + 0x5a;
return ret;
}int main(void)
{
struct foo f;
f = bar();
return f.i;
}_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 4 lines
It is certainly possible that the callee could create a temporary struct on its stack and memcpy into the address provided by the ABI. However, even at -O0 the address of the struct is popped off the stack and used directly when assigning to structure fields. I'm using gcc 3.3.5.
Same here. And with optimization turned on gcc simply inlines the functions, which makes the whole discussion kida useless ;)
So I think we don't have do worry about gcc doing stupid things and can simply return structs in the C code.
Gerd
Quoted 6 lines · Gerd Knorr
Index:
xen/arch/x86/shadow.c
===================================================================arch/x86/shadow.c 2005-04-12 17:33:01.000000000 +0200 renamed from arch/x86/shadow.c 2005-04-12 17:01:41.000000000 +0200 +0
-0@@ -333,9 +333,8 @@ free_shadow_hl2_table(struct domain *d,
arch/x86/shadow.c 2005-04-12 17:33:01.000000000 +0200 free_shadow_hl2_table(struct domain *d, · line 340
Quoted 10 lines
arch/x86/shadow.c 2005-04-12 17:33:01.000000000 +0200 renamed from arch/x86/shadow.c 2005-04-12 17:01:41.000000000 +0200 +2
-3for ( i = 0; i < limit; i++ ) { - unsigned long hl2e = l1_pgentry_val(hl2[i]); - if ( hl2e & _PAGE_PRESENT ) - put_page(pfn_to_page(hl2e >> PAGE_SHIFT)); + if ( l1e_get_flags(*hl2) & _PAGE_PRESENT ) + put_page(pfn_to_page(l1e_get_pfn(*hl2))); } unmap_domain_mem(hl2);
This hunk seems incorrect, you need to use hl2[i] instead of *hl2. We need to be careful not to introduce bugs when making cleanups like this one... It would be good if a few more people looked through the patch before we commit it, thanks!
christian