From: Henry Wang <xin.wang2@amd.com>
To: Julien Grall <julien@xen.org>, <xen-devel@lists.xenproject.org>
Cc: Anthony PERARD <anthony@xenproject.org>,
Juergen Gross <jgross@suse.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Bertrand Marquis <bertrand.marquis@arm.com>,
Michal Orzel <michal.orzel@amd.com>,
"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
Alec Kwapis <alec.kwapis@medtronic.com>,
"Daniel P . Smith" <dpsmith@apertussolutions.com>
Subject: Re: [PATCH v2 1/4] xen/arm: Alloc hypervisor reserved pages as magic pages for Dom0less DomUs
Date: Mon, 13 May 2024 11:42:01 +0800 [thread overview]
Message-ID: <fad20165-ad8a-4bc4-be5f-e95061a57a2d@amd.com> (raw)
In-Reply-To: <686ba256-f8bf-47e7-872f-d277bf7df0aa@xen.org>
Hi Julien,
On 5/11/2024 7:03 PM, Julien Grall wrote:
> Hi Henry,
>
> On 11/05/2024 01:56, Henry Wang wrote:
>> +static int __init alloc_magic_pages(struct domain *d)
>> +{
>> + struct page_info *magic_pg;
>> + mfn_t mfn;
>> + gfn_t gfn;
>> + int rc;
>> +
>> + d->max_pages += NR_MAGIC_PAGES;
>> + magic_pg = alloc_domheap_pages(d,
>> get_order_from_pages(NR_MAGIC_PAGES), 0);
>> + if ( magic_pg == NULL )
>> + return -ENOMEM;
>> +
>> + mfn = page_to_mfn(magic_pg);
>> + if ( !is_domain_direct_mapped(d) )
>> + gfn = gaddr_to_gfn(GUEST_MAGIC_BASE);
>> + else
>> + gfn = gaddr_to_gfn(mfn_to_maddr(mfn));
>
> Summarizing the discussion we had on Matrix. Regions like the extend
> area and shared memory may not be direct mapped. So unfortunately, I
> think it is possible that the GFN could clash with one of those.
>
> At least in the shared memory case, the user can provide the address.
> But as you use the domheap allocator, the address returned could
> easily change if you tweak your setup.
>
> I am not entirely sure what's the best solution. We could ask the user
> to provide the information for reserved region. But it feels like we
> are exposing a bit too much to the user.
>
> So possibly we would want to use the same approach as extended
> regions. Once we processed all the mappings, find some space for the
> hypervisor regions.
One thing that I noticed when I re-visit the extended region finding
code from the hypervisor side is:
When the domain is direct-mapped, when we find extended region for the
domain, we either use find_unallocated_memory() or find_memory_holes().
It looks like the removal of shared memory regions in both functions
uses the paddr parsed from the device tree to remove the regions, which
indicates there is an assumption that when a domain is direct-mapped,
the shared memory should also be direct-mapped. I might be wrong, but
otherwise I don't think the extended region finding logic will carve out
the correct shared memory region gpaddr range for guests.
So I think we are missing the documentation (and the corresponding
checking when we parse the device tree) for above assumption for the
static shared memory, i.e., when the domain is direct-mapped, the static
shared memory should also be direct-mapped, and user should make sure
this is satisfied in the device tree otherwise Xen should complain.
If we add this assumption and related checking code, I think your
concern of clashing with static shared memory can be addressed. Do you
agree?
Kind regards,
Henry
>
> Any other suggestions?
>
> Cheers,
>
next prev parent reply other threads:[~2024-05-13 3:42 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-11 0:56 [PATCH v2 0/4] Guest magic region allocation for 11 Dom0less domUs - Take two Henry Wang
2024-05-11 0:56 ` [PATCH v2 1/4] xen/arm: Alloc hypervisor reserved pages as magic pages for Dom0less DomUs Henry Wang
2024-05-11 8:46 ` Julien Grall
2024-05-11 9:02 ` Henry Wang
2024-05-11 9:33 ` Julien Grall
2024-05-11 11:03 ` Julien Grall
2024-05-13 3:42 ` Henry Wang [this message]
2024-05-11 0:56 ` [PATCH v2 2/4] xen/arm: Add new HVM_PARAM_HV_RSRV_{BASE_PFN,SIZE} keys in HVMOP Henry Wang
2024-05-11 9:17 ` Julien Grall
2024-05-11 0:56 ` [PATCH v2 3/4] tools/init-dom0less: Avoid hardcoding GUEST_MAGIC_BASE Henry Wang
2024-05-11 9:46 ` Julien Grall
2024-05-11 0:56 ` [PATCH v2 4/4] docs/features/dom0less: Update the late XenStore init protocol Henry Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fad20165-ad8a-4bc4-be5f-e95061a57a2d@amd.com \
--to=xin.wang2@amd.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=alec.kwapis@medtronic.com \
--cc=anthony@xenproject.org \
--cc=bertrand.marquis@arm.com \
--cc=dpsmith@apertussolutions.com \
--cc=jgross@suse.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).