[Xen-devel] Re: xen: use paging_mode_hap()

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 16:51 +0100 on 30 Mar (1269967863), Christoph Egger wrote:
> 
> Hi!
> 
> Attached patch makes xen consequently use paging_mode_hap().
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>
> 
> P.S.: This is a resend. Needs ack from Tim.

Nack, at least in its current form - paging_mode_hap(d) is not the same
as d->arch.hvm_domain.hap_enabled.  Especially in paging.c, where we
test hap_enabled to gate whether we should call the function that makes
paging_mode_hap(d) true!

I think the p2m.c changes are wrong too, since the p2m init code is
called from hap_enable before it writes PG_HAP_enable to the paging
mode.  (That's probably a bug in its own right; it should set the mode
and then call the p2m code, and shadow should do likewise, and then
these changes would be correct and desirable).

The other substitutions are probably OK. 

Cheers,

Tim.

Content-Description: xen_pagingmode.diff
> diff -r af15aa70b6ff xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c      Tue Mar 09 14:01:51 2010 +0100
> +++ b/xen/arch/x86/hvm/vmx/vmx.c      Tue Mar 09 14:34:24 2010 +0100
> @@ -92,7 +92,7 @@ static int vmx_domain_initialise(struct 
>  
>  static void vmx_domain_destroy(struct domain *d)
>  {
> -    if ( d->arch.hvm_domain.hap_enabled )
> +    if ( paging_mode_hap(d) )
>          on_each_cpu(__ept_sync_domain, d, 1);
>      vmx_free_vlapic_mapping(d);
>  }
> @@ -678,7 +678,7 @@ static void vmx_ctxt_switch_to(struct vc
>      if ( old_cr4 != new_cr4 )
>          write_cr4(new_cr4);
>  
> -    if ( d->arch.hvm_domain.hap_enabled )
> +    if ( paging_mode_hap(d) )
>      {
>          unsigned int cpu = smp_processor_id();
>          /* Test-and-test-and-set this CPU in the EPT-is-synced mask. */
> @@ -1222,7 +1222,7 @@ static void __ept_sync_domain(void *info
>  void ept_sync_domain(struct domain *d)
>  {
>      /* Only if using EPT and this domain has some VCPUs to dirty. */
> -    if ( !d->arch.hvm_domain.hap_enabled || !d->vcpu || !d->vcpu[0] )
> +    if ( !paging_mode_hap(d) || !d->vcpu || !d->vcpu[0] )
>          return;
>  
>      ASSERT(local_irq_is_enabled());
> diff -r af15aa70b6ff xen/arch/x86/mm/mem_sharing.c
> --- a/xen/arch/x86/mm/mem_sharing.c   Tue Mar 09 14:01:51 2010 +0100
> +++ b/xen/arch/x86/mm/mem_sharing.c   Tue Mar 09 14:34:24 2010 +0100
> @@ -44,7 +44,7 @@ static void mem_sharing_audit(void);
>  
>  
>  #define hap_enabled(d) \
> -    (is_hvm_domain(d) && (d)->arch.hvm_domain.hap_enabled)
> +    (is_hvm_domain(d) && paging_mode_hap(d))
>  #define mem_sharing_enabled(d) \
>      (is_hvm_domain(d) && (d)->arch.hvm_domain.mem_sharing_enabled)
>   
> diff -r af15aa70b6ff xen/arch/x86/mm/p2m.c
> --- a/xen/arch/x86/mm/p2m.c   Tue Mar 09 14:01:51 2010 +0100
> +++ b/xen/arch/x86/mm/p2m.c   Tue Mar 09 14:34:24 2010 +0100
> @@ -1231,7 +1231,7 @@ p2m_set_entry(struct domain *d, unsigned
>      if ( !p2m_next_level(d, &table_mfn, &table, &gfn_remainder, gfn,
>                           L3_PAGETABLE_SHIFT - PAGE_SHIFT,
>                           ((CONFIG_PAGING_LEVELS == 3)
> -                          ? (d->arch.hvm_domain.hap_enabled ? 4 : 8)
> +                          ? (paging_mode_hap(d) ? 4 : 8)
>                            : L3_PAGETABLE_ENTRIES),
>                           PGT_l2_page_table) )
>          goto out;
> @@ -1568,7 +1568,7 @@ int p2m_init(struct domain *d)
>      p2m->get_entry_current = p2m_gfn_to_mfn_current;
>      p2m->change_entry_type_global = p2m_change_type_global;
>  
> -    if ( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled &&
> +    if ( is_hvm_domain(d) && paging_mode_hap(d) &&
>           (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) )
>          ept_p2m_init(d);
>  
> @@ -1595,7 +1595,7 @@ int set_p2m_entry(struct domain *d, unsi
>  
>      while ( todo )
>      {
> -        if ( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled )
> +        if ( is_hvm_domain(d) && paging_mode_hap(d) )
>              order = (((gfn | mfn_x(mfn) | todo) & (SUPERPAGE_PAGES - 1)) == 
> 0) ?
>                  9 : 0;
>          else
> diff -r af15aa70b6ff xen/arch/x86/mm/paging.c
> --- a/xen/arch/x86/mm/paging.c        Tue Mar 09 14:01:51 2010 +0100
> +++ b/xen/arch/x86/mm/paging.c        Tue Mar 09 14:34:24 2010 +0100
> @@ -29,7 +29,7 @@
>  #include <xen/numa.h>
>  #include <xsm/xsm.h>
>  
> -#define hap_enabled(d) (is_hvm_domain(d) && (d)->arch.hvm_domain.hap_enabled)
> +#define hap_enabled(d) (is_hvm_domain(d) && paging_mode_hap(d))
>  
>  /* Printouts */
>  #define PAGING_PRINTK(_f, _a...)                                     \


-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel