Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM

From: "Jan Beulich" <JBeulich@suse.com>
To: Wei Liu <wei.liu2@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	Tamas K Lengyel <tamas@tklengyel.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3 07/16] x86/p2m/pod: make it build with !CONFIG_HVM
Date: Thu, 06 Sep 2018 09:05:37 -0600	[thread overview]
Message-ID: <5B91424102000078001E6012@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <20180904161533.11575-8-wei.liu2@citrix.com>

>>> On 04.09.18 at 18:15, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -52,15 +52,20 @@ DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>  /* Init the datastructures for later use by the p2m code */
>  static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>  {
> -    unsigned int i;
>      int ret = 0;
> +#ifdef CONFIG_HVM
> +    unsigned int i;
> +#endif
>  
>      mm_rwlock_init(&p2m->lock);
> -    mm_lock_init(&p2m->pod.lock);
>      INIT_LIST_HEAD(&p2m->np2m_list);
>      INIT_PAGE_LIST_HEAD(&p2m->pages);
> +
> +#ifdef CONFIG_HVM
> +    mm_lock_init(&p2m->pod.lock);
>      INIT_PAGE_LIST_HEAD(&p2m->pod.super);
>      INIT_PAGE_LIST_HEAD(&p2m->pod.single);
> +#endif
>  
>      p2m->domain = d;
>      p2m->default_access = p2m_access_rwx;
> @@ -69,8 +74,10 @@ static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
>      p2m->np2m_base = P2M_BASE_EADDR;
>      p2m->np2m_generation = 0;
>  
> +#ifdef CONFIG_HVM
>      for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
>          p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
> +#endif

I wonder whether all of this wouldn't better go into p2m_pod_init()
(or alike), to limit/avoid the #ifdef-ary here.

> @@ -2560,7 +2573,10 @@ void audit_p2m(struct domain *d,
>      P2M_PRINTK("p2m audit starts\n");
>  
>      p2m_lock(p2m);
> +
> +#ifdef CONFIG_HVM
>      pod_lock(p2m);
> +#endif
>  
>      if (p2m->audit_p2m)
>          pmbad = p2m->audit_p2m(p2m);
> @@ -2621,7 +2637,9 @@ void audit_p2m(struct domain *d,
>      }
>      spin_unlock(&d->page_alloc_lock);
>  
> +#ifdef CONFIG_HVM
>      pod_unlock(p2m);
> +#endif

Perhaps better make them inline stubs? Otoh - isn't the whole
function useful for HVM only?

> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -630,7 +630,9 @@ int vm_event_domctl(struct domain *d, struct 
> xen_domctl_vm_event_op *vec,
>          {
>          case XEN_VM_EVENT_ENABLE:
>          {
> +#ifdef CONFIG_HVM
>              struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +#endif
>  
>              rc = -EOPNOTSUPP;
>              /* hvm fixme: p2m_is_foreign types need addressing */
> @@ -647,10 +649,12 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
>              if ( unlikely(need_iommu(d)) )
>                  break;
>  
> +#ifdef CONFIG_HVM
>              rc = -EXDEV;
>              /* Disallow paging in a PoD guest */
>              if ( p2m->pod.entry_count )
>                  break;
> +#endif

Perhaps simply ditch the local variable?

Jan

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