Hi Paul,
On 10/03/2020 17:49, p...@xen.org wrote:
From: Paul Durrant <pdurr...@amazon.com>
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
arch_domain_create().
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.
+ */
+ ASSERT_UNREACHABLE();
+ 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.
Cheers,
--
Julien Grall