Re: [Xen-devel] [PATCH v4 3/7] x86/mm: put nested p2m code under CONFIG_HVM

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.18 at 18:38, <wei.liu2@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1691,7 +1691,9 @@ void context_switch(struct vcpu *prev, struct vcpu 
> *next)
>      {
>          _update_runstate_area(prev);
>          vpmu_switch_from(prev);
> +#ifdef CONFIG_HVM
>          np2m_schedule(NP2M_SCHEDLE_OUT);
> +#endif
>      }
>  
>      if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
> @@ -1758,7 +1760,9 @@ void context_switch(struct vcpu *prev, struct vcpu 
> *next)
>  
>          /* Must be done with interrupts enabled */
>          vpmu_switch_to(next);
> +#ifdef CONFIG_HVM
>          np2m_schedule(NP2M_SCHEDLE_IN);
> +#endif
>      }

Instead of these two #ifdef-s a single stub would have been neater imo.

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -63,23 +63,30 @@ static void p2m_pod_init(struct p2m_domain *p2m)
>  #endif
>  }
>  
> +static void p2m_nestedp2m_init(struct p2m_domain *p2m)
> +{
> +#ifdef CONFIG_HVM
> +    INIT_LIST_HEAD(&p2m->np2m_list);
> +
> +    p2m->np2m_base = P2M_BASE_EADDR;
> +    p2m->np2m_generation = 0;
> +#endif
> +}

Why don't you follow this placement model ...

> @@ -151,6 +158,7 @@ static void p2m_teardown_hostp2m(struct domain *d)
>      }
>  }
>  
> +#ifdef CONFIG_HVM
>  static void p2m_teardown_nestedp2m(struct domain *d)
>  {
>      unsigned int i;
> @@ -188,6 +196,7 @@ static int p2m_init_nestedp2m(struct domain *d)
>  
>      return 0;
>  }
> +#endif

... here avoiding ...

> @@ -244,12 +254,15 @@ int p2m_init(struct domain *d)
>          p2m_teardown_hostp2m(d);
>          return rc;
>      }
> +#endif
>  
>      rc = p2m_init_altp2m(d);
>      if ( rc )
>      {
>          p2m_teardown_hostp2m(d);
> +#ifdef CONFIG_HVM
>          p2m_teardown_nestedp2m(d);
> +#endif
>      }
>  
>      return rc;
> @@ -701,7 +714,9 @@ void p2m_final_teardown(struct domain *d)
>       * we initialise them unconditionally.
>       */
>      p2m_teardown_altp2m(d);
> +#ifdef CONFIG_HVM
>      p2m_teardown_nestedp2m(d);
> +#endif
>  
>      /* Iterate over all p2m tables per domain */
>      p2m_teardown_hostp2m(d);

... two other #ifdef-s?

Anyway, in the interest of forward progress, preferably with the
adjustments made
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel