From: Richard Henderson <richard.henderson@linaro.org>
To: Akihiko Odaki <akihiko.odaki@daynix.com>
Cc: qemu-devel@nongnu.org, joel@jms.id.au, laurent@vivier.eu, deller@gmx.de
Subject: Re: [PATCH v2] linux-user: Undo incomplete mmap
Date: Tue, 3 Oct 2023 13:42:24 -0700 [thread overview]
Message-ID: <fea1c085-8b92-7d01-bedb-35cefb357df5@linaro.org> (raw)
In-Reply-To: <619a188e-aaae-4d45-9aa5-bd9c4eac1a4d@daynix.com>
On 9/21/23 00:09, Akihiko Odaki wrote:
> On 2023/09/03 14:39, Akihiko Odaki wrote:
>> When the host page size is greater than the target page size and
>> MAP_FIXED or MAP_FIXED_NOREPLACE is requested, mmap will be done for
>> three parts: start, middle, and end. If a later part of mmap fail,
>> mmap done in the earlier parts must be reverted.
>>
>> Fixes: 54936004fd ("mmap emulation")
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>> V1 -> V2: Rebased.
>>
>> linux-user/mmap.c | 65 +++++++++++++++++++++++++++++------------------
>> 1 file changed, 40 insertions(+), 25 deletions(-)
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 9aab48d4a3..72521f8d27 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -224,13 +224,15 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
>> /* map an incomplete host page */
>> static bool mmap_frag(abi_ulong real_start, abi_ulong start, abi_ulong last,
>> - int prot, int flags, int fd, off_t offset)
>> + int prot, int flags, int fd, off_t offset, bool *mapped)
>> {
>> abi_ulong real_last;
>> void *host_start;
>> int prot_old, prot_new;
>> int host_prot_old, host_prot_new;
>> + *mapped = false;
>> +
>> if (!(flags & MAP_ANONYMOUS)
>> && (flags & MAP_TYPE) == MAP_SHARED
>> && (prot & PROT_WRITE)) {
>> @@ -271,6 +273,7 @@ static bool mmap_frag(abi_ulong real_start, abi_ulong start,
>> abi_ulong last,
>> return false;
>> }
>> prot_old = prot;
>> + *mapped = true;
>> }
>> prot_new = prot | prot_old;
>> @@ -448,7 +451,7 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size, abi_ulong
>> align)
>> abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
>> int flags, int fd, off_t offset)
>> {
>> - abi_ulong ret, last, real_start, real_last, retaddr, host_len;
>> + abi_ulong ret, last, real_start, retaddr, host_len;
>> abi_ulong passthrough_start = -1, passthrough_last = 0;
>> int page_flags;
>> off_t host_offset;
>> @@ -577,12 +580,16 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
>> passthrough_start = start;
>> passthrough_last = last;
>> } else {
>> + abi_ulong middle_start = HOST_PAGE_ALIGN(start);
>> + abi_ulong middle_last = ((start + len) & qemu_host_page_mask) - 1;
>> + abi_ulong mapped_len = 0;
>> + bool mapped;
>> +
>> if (start & ~TARGET_PAGE_MASK) {
>> errno = EINVAL;
>> goto fail;
>> }
>> last = start + len - 1;
>> - real_last = HOST_PAGE_ALIGN(last) - 1;
>> /*
>> * Test if requested memory area fits target address space
>> @@ -649,35 +656,26 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
>> }
>> /* handle the start of the mapping */
>> - if (start > real_start) {
>> - if (real_last == real_start + qemu_host_page_size - 1) {
>> + if (start < middle_start) {
>> + if (last < middle_start) {
>> /* one single host page */
>> if (!mmap_frag(real_start, start, last,
>> - target_prot, flags, fd, offset)) {
>> + target_prot, flags, fd, offset, &mapped)) {
>> goto fail;
>> }
>> goto the_end1;
>> }
>> - if (!mmap_frag(real_start, start,
>> - real_start + qemu_host_page_size - 1,
>> - target_prot, flags, fd, offset)) {
>> + if (!mmap_frag(real_start, start, middle_start - 1,
>> + target_prot, flags, fd, offset, &mapped)) {
>> goto fail;
>> }
>> - real_start += qemu_host_page_size;
>> - }
>> - /* handle the end of the mapping */
>> - if (last < real_last) {
>> - abi_ulong real_page = real_last - qemu_host_page_size + 1;
>> - if (!mmap_frag(real_page, real_page, last,
>> - target_prot, flags, fd,
>> - offset + real_page - start)) {
>> - goto fail;
>> + if (mapped) {
>> + mapped_len = qemu_host_page_size;
>> }
>> - real_last -= qemu_host_page_size;
>> }
>> /* map the middle (easier) */
>> - if (real_start < real_last) {
>> + if (middle_start < middle_last) {
>> void *p, *want_p;
>> off_t offset1;
>> size_t len1;
>> @@ -685,10 +683,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
>> if (flags & MAP_ANONYMOUS) {
>> offset1 = 0;
>> } else {
>> - offset1 = offset + real_start - start;
>> + offset1 = offset + middle_start - start;
>> }
>> - len1 = real_last - real_start + 1;
>> - want_p = g2h_untagged(real_start);
>> + len1 = middle_last - middle_start + 1;
>> + want_p = g2h_untagged(middle_start);
>> p = mmap(want_p, len1, target_to_host_prot(target_prot),
>> flags, fd, offset1);
>> @@ -697,10 +695,27 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
>> munmap(p, len1);
>> errno = EEXIST;
>> }
>> + if (mapped_len) {
>> + munmap(g2h_untagged(middle_start - mapped_len), mapped_len);
>> + }
>> + goto fail;
>> + }
>> + mapped_len += len1;
>> + passthrough_start = middle_start;
>> + passthrough_last = middle_last;
>> + }
>> +
>> + /* handle the end of the mapping */
>> + if (last > middle_last) {
>> + abi_ulong real_page = middle_last + 1;
>> + if (!mmap_frag(real_page, real_page, last,
>> + target_prot, flags, fd,
>> + offset + real_page - start, &mapped)) {
>> + if (mapped_len) {
>> + munmap(g2h_untagged(real_page - mapped_len), mapped_len);
>> + }
>> goto fail;
>> }
>> - passthrough_start = real_start;
>> - passthrough_last = real_last;
>> }
>> }
>> the_end1:
>
> Hi Richard,
>
> Can you have a look at this patch?
munmap isn't always correct -- one has to keep the virtual address space allocated for
reserved_va. So mmap_reserve_or_unmap.
That said, I've got one reorg of mmap.c outstanding already, and I'm planning another to
allow use of softmmu in user-only mode. The latter would eliminate mmap_frag entirely.
r~
next prev parent reply other threads:[~2023-10-03 20:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-03 5:39 [PATCH v2] linux-user: Undo incomplete mmap Akihiko Odaki
2023-09-21 7:09 ` Akihiko Odaki
2023-10-03 20:42 ` Richard Henderson [this message]
2023-10-05 5:17 ` Akihiko Odaki
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=fea1c085-8b92-7d01-bedb-35cefb357df5@linaro.org \
--to=richard.henderson@linaro.org \
--cc=akihiko.odaki@daynix.com \
--cc=deller@gmx.de \
--cc=joel@jms.id.au \
--cc=laurent@vivier.eu \
--cc=qemu-devel@nongnu.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 NNTP newsgroup(s).