Hi Henry,
On 11/05/2024 10:02, Henry Wang wrote:
+
+ rc = guest_physmap_add_pages(d, gfn, mfn, NR_MAGIC_PAGES);
+ if ( rc )
+ {
+ free_domheap_pages(magic_pg,
get_order_from_pages(NR_MAGIC_PAGES));
+ return rc;
+ }
+
+ return 0;
+}
+
static int __init construct_domU(struct domain *d,
const struct dt_device_node *node)
{
@@ -840,6 +868,10 @@ static int __init construct_domU(struct domain *d,
if ( rc < 0 )
return rc;
d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL;
+
+ rc = alloc_magic_pages(d);
+ if ( rc < 0 )
+ return rc;
This will only be allocated xenstore is enabled. But I don't think
some of the magic pages really require xenstore to work. In the future
we may need some more fine graine choice (see my comment in patch #2
as well).
Sorry, but it seems that by the time that I am writing this reply, I
didn't get the email for patch #2 comment. I will reply both together
when I see it.
That was expected, I knew what I wanted to mention about patch #2 but
the e-mail was not fully written. You should have it in your inbox now.
}
return rc;
diff --git a/xen/include/public/arch-arm.h
b/xen/include/public/arch-arm.h
index 289af81bd6..186520d01f 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -476,6 +476,12 @@ typedef uint64_t xen_callback_t;
#define GUEST_MAGIC_BASE xen_mk_ullong(0x39000000)
#define GUEST_MAGIC_SIZE xen_mk_ullong(0x01000000)
+#define NR_MAGIC_PAGES 4
This name and the other below are far too generic to be part of the
shared header. For NR_MAGIC_PAGES, can you explain why
GUEST_MAGIC_SIZE cannot be used? Is it because it is "too" big?
Yes, I don't really want to allocate whole 16MB when we merely need 4
pages.
What about updating GUEST_MAGIC_SIZE to 0x4000? It doesn't make any
sense to have two define which have pretty much the same meaning.
Then on the toolstack side, you can check that NR_MAGIC_PAGES is smaller
or equal to GUEST_MAGIC_SIZE.
For the rest below...
+#define CONSOLE_PFN_OFFSET 0
+#define XENSTORE_PFN_OFFSET 1
+#define MEMACCESS_PFN_OFFSET 2
+#define VUART_PFN_OFFSET 3
... the order we allocate the magic pages is purely an internal
decision of the toolstack. For the rest of the system, they need to
access HVM_PARAM_*. So it doesn't feel like they should be part of the
shared headers.
If there is a strong reason to include them, then I think they all
should be prefixed with GUEST_MAGIC_*.
One of the benefits as Michal pointed out in comments for v1 [1] would
be this would also allow init-dom0less.h not to re-define
XENSTORE_PFN_OFFSET.
At the moment, yes they have the same values. But with series,
XENSTORE_PFN_OFFSET should technically be 0 in init-dom0less.
This is because Xen may only allocate one page (you don't need 4) for
the reserved area. So...
I will rename them as suggested if both of you are
ok with moving them to the arch header.
... I don't think they should be shared.
Cheers,
--
Julien Grall
