diff mbox

linux-user: Add TARGET_MAP_STACK and TARGET_MAP_HUGETLB for all remaining architectures

Message ID 20170311183016.GA20514@ls3530.fritz.box
State New
Headers show

Commit Message

Helge Deller March 11, 2017, 6:30 p.m. UTC 
Add the missing defines and for TARGET_MAP_STACK and TARGET_MAP_HUGETLB
for alpha, mips, ppc and x86, and fix the mmap_flags translation table
to translate those flags between host and target architecture.

Signed-off-by: Helge Deller <deller@gmx.de>

Comments

no-reply@patchew.org March 11, 2017, 6:33 p.m. UTC | #1 
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH] linux-user: Add TARGET_MAP_STACK and TARGET_MAP_HUGETLB for all remaining architectures
Message-id: 20170311183016.GA20514@ls3530.fritz.box

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20170311034232.14213-1-rth@twiddle.net -> patchew/20170311034232.14213-1-rth@twiddle.net
 * [new tag]         patchew/20170311183016.GA20514@ls3530.fritz.box -> patchew/20170311183016.GA20514@ls3530.fritz.box
Switched to a new branch 'test'
8c465a2 linux-user: Add TARGET_MAP_STACK and TARGET_MAP_HUGETLB for all remaining architectures

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user: Add TARGET_MAP_STACK and TARGET_MAP_HUGETLB for all remaining architectures...
ERROR: code indent should never use tabs
#23: FILE: linux-user/syscall.c:5878:
+^I{ TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK },$

ERROR: code indent should never use tabs
#24: FILE: linux-user/syscall.c:5879:
+^I{ TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB },$

ERROR: line over 90 characters
#36: FILE: linux-user/syscall_defs.h:1324:
+#define TARGET_MAP_STACK	0x40000		/* give out an address that is best suited for process/thread stacks */

ERROR: code indent should never use tabs
#36: FILE: linux-user/syscall_defs.h:1324:
+#define TARGET_MAP_STACK^I0x40000^I^I/* give out an address that is best suited for process/thread stacks */$

ERROR: code indent should never use tabs
#37: FILE: linux-user/syscall_defs.h:1325:
+#define TARGET_MAP_HUGETLB^I0x80000^I^I/* create a huge page mapping */$

ERROR: line over 90 characters
#45: FILE: linux-user/syscall_defs.h:1336:
+#define TARGET_MAP_STACK	0x20000		/* give out an address that is best suited for process/thread stacks */

ERROR: code indent should never use tabs
#45: FILE: linux-user/syscall_defs.h:1336:
+#define TARGET_MAP_STACK^I0x20000^I^I/* give out an address that is best suited for process/thread stacks */$

ERROR: code indent should never use tabs
#46: FILE: linux-user/syscall_defs.h:1337:
+#define TARGET_MAP_HUGETLB^I0x40000^I^I/* create a huge page mapping */$

ERROR: line over 90 characters
#54: FILE: linux-user/syscall_defs.h:1348:
+#define TARGET_MAP_STACK	0x80000		/* give out an address that is best suited for process/thread stacks */

ERROR: code indent should never use tabs
#54: FILE: linux-user/syscall_defs.h:1348:
+#define TARGET_MAP_STACK^I0x80000^I^I/* give out an address that is best suited for process/thread stacks */$

ERROR: code indent should never use tabs
#55: FILE: linux-user/syscall_defs.h:1349:
+#define TARGET_MAP_HUGETLB^I0x100000^I/* create a huge page mapping */$

ERROR: line over 90 characters
#63: FILE: linux-user/syscall_defs.h:1370:
+#define TARGET_MAP_STACK	0x20000		/* give out an address that is best suited for process/thread stacks */

ERROR: code indent should never use tabs
#63: FILE: linux-user/syscall_defs.h:1370:
+#define TARGET_MAP_STACK^I0x20000^I^I/* give out an address that is best suited for process/thread stacks */$

ERROR: code indent should never use tabs
#64: FILE: linux-user/syscall_defs.h:1371:
+#define TARGET_MAP_HUGETLB^I0x40000^I^I/* create a huge page mapping */$

total: 14 errors, 0 warnings, 40 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Richard Henderson March 11, 2017, 9:53 p.m. UTC | #2 
On 03/12/2017 04:30 AM, Helge Deller wrote:
> Add the missing defines and for TARGET_MAP_STACK and TARGET_MAP_HUGETLB
> for alpha, mips, ppc and x86, and fix the mmap_flags translation table
> to translate those flags between host and target architecture.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index cec8428..03ed370 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5875,6 +5875,8 @@ static bitmask_transtbl mmap_flags_tbl[] = {
>  	{ TARGET_MAP_LOCKED, TARGET_MAP_LOCKED, MAP_LOCKED, MAP_LOCKED },
>          { TARGET_MAP_NORESERVE, TARGET_MAP_NORESERVE, MAP_NORESERVE,
>            MAP_NORESERVE },
> +	{ TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK },
> +	{ TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB },

I don't see any point in this.  First, MAP_STACK is ignored by the kernel, and 
has been for some time.  Second, the size of huge pages varies widely between 
different targets, and we're not really able to map the sizes between guest and 
host.

I suppose we could pass it through and get lucky when the sizes do match.  But 
if they don't, is it better to succeed with small tlb entries or fail with -EINVAL?


r~
Helge Deller March 13, 2017, 2:09 p.m. UTC | #3 
On 11.03.2017 22:53, Richard Henderson wrote:
> On 03/12/2017 04:30 AM, Helge Deller wrote:
>> Add the missing defines and for TARGET_MAP_STACK and TARGET_MAP_HUGETLB
>> for alpha, mips, ppc and x86, and fix the mmap_flags translation table
>> to translate those flags between host and target architecture.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index cec8428..03ed370 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -5875,6 +5875,8 @@ static bitmask_transtbl mmap_flags_tbl[] = {
>>      { TARGET_MAP_LOCKED, TARGET_MAP_LOCKED, MAP_LOCKED, MAP_LOCKED },
>>          { TARGET_MAP_NORESERVE, TARGET_MAP_NORESERVE, MAP_NORESERVE,
>>            MAP_NORESERVE },
>> +    { TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK },
>> +    { TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB },
> 
> I don't see any point in this. First, MAP_STACK is ignored by the
> kernel, and has been for some time. 

Yes, the kernel ignores the native MAP_STACK define.

> Second, the size of huge pages varies widely between different targets,
> and we're not really able to map the sizes between guest and host.

This might probably not be necessary, if the userspace app simply asks
which huge page sizes are supported (e.g. by procfs entries or via 
"hugeadm --page-sizes-all").

But the main intention of my patch was not to fix MAP_STACK or MAP_HUGETLB,
but more to ensure that those flags are avoided to be handed over to the host.
For example, TARGET_MAP_STACK is 0x40000 on hppa, and if it's not filtered out
in e.g. a mmap() call, then this becomes to MAP_HUGETLB on x86_64 (which is 
0x40000 on x86_64).  

Helge
Richard Henderson March 13, 2017, 8:52 p.m. UTC | #4 
On 03/14/2017 12:09 AM, Helge Deller wrote:
> But the main intention of my patch was not to fix MAP_STACK or MAP_HUGETLB,
> but more to ensure that those flags are avoided to be handed over to the host.
> For example, TARGET_MAP_STACK is 0x40000 on hppa, and if it's not filtered out
> in e.g. a mmap() call, then this becomes to MAP_HUGETLB on x86_64 (which is
> 0x40000 on x86_64).

Ah... yes.  That is a much better point.


r~
diff mbox

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index cec8428..03ed370 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5875,6 +5875,8 @@  static bitmask_transtbl mmap_flags_tbl[] = {
 	{ TARGET_MAP_LOCKED, TARGET_MAP_LOCKED, MAP_LOCKED, MAP_LOCKED },
         { TARGET_MAP_NORESERVE, TARGET_MAP_NORESERVE, MAP_NORESERVE,
           MAP_NORESERVE },
+	{ TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK },
+	{ TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB },
 	{ 0, 0, 0, 0 }
 };
 
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index f356189..a516d77 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1346,6 +1346,8 @@  struct target_winsize {
 #define TARGET_MAP_NORESERVE	0x0400		/* don't check for reservations */
 #define TARGET_MAP_POPULATE	0x10000		/* populate (prefault) pagetables */
 #define TARGET_MAP_NONBLOCK	0x20000		/* do not block on IO */
+#define TARGET_MAP_STACK	0x40000		/* give out an address that is best suited for process/thread stacks */
+#define TARGET_MAP_HUGETLB	0x80000		/* create a huge page mapping */
 #elif defined(TARGET_PPC)
 #define TARGET_MAP_FIXED	0x10		/* Interpret addr exactly */
 #define TARGET_MAP_ANONYMOUS	0x20		/* don't use a file */
@@ -1356,6 +1358,8 @@  struct target_winsize {
 #define TARGET_MAP_NORESERVE	0x0040		/* don't check for reservations */
 #define TARGET_MAP_POPULATE	0x8000		/* populate (prefault) pagetables */
 #define TARGET_MAP_NONBLOCK	0x10000		/* do not block on IO */
+#define TARGET_MAP_STACK	0x20000		/* give out an address that is best suited for process/thread stacks */
+#define TARGET_MAP_HUGETLB	0x40000		/* create a huge page mapping */
 #elif defined(TARGET_ALPHA)
 #define TARGET_MAP_ANONYMOUS	0x10		/* don't use a file */
 #define TARGET_MAP_FIXED	0x100		/* Interpret addr exactly */
@@ -1366,6 +1370,8 @@  struct target_winsize {
 #define TARGET_MAP_NORESERVE	0x10000		/* no check for reservations */
 #define TARGET_MAP_POPULATE	0x20000		/* pop (prefault) pagetables */
 #define TARGET_MAP_NONBLOCK	0x40000		/* do not block on IO */
+#define TARGET_MAP_STACK	0x80000		/* give out an address that is best suited for process/thread stacks */
+#define TARGET_MAP_HUGETLB	0x100000	/* create a huge page mapping */
 #elif defined(TARGET_HPPA)
 #define TARGET_MAP_ANONYMOUS	0x10		/* don't use a file */
 #define TARGET_MAP_FIXED	0x04		/* Interpret addr exactly */
@@ -1388,6 +1394,8 @@  struct target_winsize {
 #define TARGET_MAP_NORESERVE	0x4000		/* don't check for reservations */
 #define TARGET_MAP_POPULATE	0x8000		/* populate (prefault) pagetables */
 #define TARGET_MAP_NONBLOCK	0x10000		/* do not block on IO */
+#define TARGET_MAP_STACK	0x20000		/* give out an address that is best suited for process/thread stacks */
+#define TARGET_MAP_HUGETLB	0x40000		/* create a huge page mapping */
 #define TARGET_MAP_UNINITIALIZED 0x4000000	/* for anonymous mmap, memory could be uninitialized */
 #endif