[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 5/5] domain: use PGC_extra domheap page for shared_info

Hi Paul,

On 10/03/2020 17:49, paul@xxxxxxx wrote:
From: Paul Durrant <pdurrant@xxxxxxxxxx>

Currently shared_info is a shared xenheap page but shared xenheap pages
complicate future plans for live-update of Xen so it is desirable to,
where possible, not use them [1]. This patch therefore converts shared_info
into a PGC_extra domheap page. This does entail freeing shared_info during
domain_relinquish_resources() rather than domain_destroy() so care is
needed to avoid de-referencing a NULL shared_info pointer hence some
extra checks of 'is_dying' are needed.

NOTE: For Arm, the call to free_shared_info() in arch_domain_destroy() is
       left in place since it is idempotent and called in the error path for

The approach looks good to me. I have one comment below.


diff --git a/xen/common/domain.c b/xen/common/domain.c
index 4ef0d3b21e..4f3266454f 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1651,24 +1651,44 @@ int continue_hypercall_on_cpu(
int alloc_shared_info(struct domain *d, unsigned int memflags)
-    if ( (d->shared_info.virt = alloc_xenheap_pages(0, memflags)) == NULL )
+    struct page_info *pg;
+    pg = alloc_domheap_page(d, MEMF_no_refcount | memflags);
+    if ( !pg )
          return -ENOMEM;
- d->shared_info.mfn = virt_to_mfn(d->shared_info.virt);
+    if ( !get_page_and_type(pg, d, PGT_writable_page) )
+    {
+        /*
+         * The domain should not be running at this point so there is
+         * no way we should reach this error path.
+         */
+        return -ENODATA;
+    }
+    d->shared_info.mfn = page_to_mfn(pg);
+    d->shared_info.virt = __map_domain_page_global(pg);

__map_domain_page_global() is not guaranteed to succeed. For instance, on Arm32 this will be a call to vmap().

So you want to check the return.


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.