diff mbox

[for-2.3?,7/7] tests/tcg: Enable runcom test by default

Message ID 1428777292-24628-8-git-send-email-afaerber@suse.de
State New
Headers show

Commit Message

Andreas Färber April 11, 2015, 6:34 p.m. UTC
The comment of it requiring root appears outdated - makes no difference.
pi_10.com terminates reproducibly with "...10559240190274216248".

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 tests/tcg/Makefile | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Peter Maydell April 11, 2015, 8:22 p.m. UTC | #1
On 11 April 2015 at 19:34, Andreas Färber <afaerber@suse.de> wrote:
> The comment of it requiring root appears outdated - makes no difference.

This depends on what your system has set for the tunable
/proc/sys/vm/mmap_min_addr. On my system it is 65536 (I think
Ubuntu and Debian ship like this), and in that case you can't
mmap MAP_FIXED to address zero unless you're root (tested with
a little C program that does the same mmap that runcom does).

thanks
-- PMM
Andreas Färber April 11, 2015, 8:28 p.m. UTC | #2
Am 11.04.2015 um 22:22 schrieb Peter Maydell:
> On 11 April 2015 at 19:34, Andreas Färber <afaerber@suse.de> wrote:
>> The comment of it requiring root appears outdated - makes no difference.
> 
> This depends on what your system has set for the tunable
> /proc/sys/vm/mmap_min_addr. On my system it is 65536 (I think
> Ubuntu and Debian ship like this), and in that case you can't
> mmap MAP_FIXED to address zero unless you're root (tested with
> a little C program that does the same mmap that runcom does).

Hm,

$ cat /proc/sys/vm/mmap_min_addr
65536

Are you sure that QEMU / runcom / pi_10.com still try to map to that
address zero?

Regards,
Andreas
Peter Maydell April 11, 2015, 9:33 p.m. UTC | #3
On 11 April 2015 at 21:28, Andreas Färber <afaerber@suse.de> wrote:
> Are you sure that QEMU / runcom / pi_10.com still try to map to that
> address zero?

It's the first thing runcom.c's main() does after checking you've
passed it enough command line arguments.

-- PMM
Andreas Färber April 11, 2015, 9:46 p.m. UTC | #4
Am 11.04.2015 um 23:33 schrieb Peter Maydell:
> On 11 April 2015 at 21:28, Andreas Färber <afaerber@suse.de> wrote:
>> Are you sure that QEMU / runcom / pi_10.com still try to map to that
>> address zero?
> 
> It's the first thing runcom.c's main() does after checking you've
> passed it enough command line arguments.

This?

    vm86_mem = mmap((void *)0x00000000, 0x110000,
                    PROT_WRITE | PROT_READ | PROT_EXEC,
                    MAP_FIXED | MAP_ANON | MAP_PRIVATE, -1, 0);
    if (vm86_mem == MAP_FAILED) {
        perror("mmap");
        exit(1);
    }

For whatever reason it does not take the MAP_FAILED path here...

But I can certainly live with dropping this patch, it's the last for a
reason. :)

Andreas
Alexander Graf April 12, 2015, 8:34 a.m. UTC | #5
> Am 11.04.2015 um 23:46 schrieb Andreas Färber <afaerber@suse.de>:
> 
>> Am 11.04.2015 um 23:33 schrieb Peter Maydell:
>>> On 11 April 2015 at 21:28, Andreas Färber <afaerber@suse.de> wrote:
>>> Are you sure that QEMU / runcom / pi_10.com still try to map to that
>>> address zero?
>> 
>> It's the first thing runcom.c's main() does after checking you've
>> passed it enough command line arguments.
> 
> This?
> 
>    vm86_mem = mmap((void *)0x00000000, 0x110000,
>                    PROT_WRITE | PROT_READ | PROT_EXEC,
>                    MAP_FIXED | MAP_ANON | MAP_PRIVATE, -1, 0);
>    if (vm86_mem == MAP_FAILED) {
>        perror("mmap");
>        exit(1);
>    }
> 
> For whatever reason it does not take the MAP_FAILED path here...

Wasn't a NULL argument for the address a hint saying "map wherever you please"? I also feel like I'm missing context here - what exactly is broken?

Alex

> 
> But I can certainly live with dropping this patch, it's the last for a
> reason. :)
> 
> Andreas
> 
> -- 
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
> Graham Norton; HRB 21284 (AG Nürnberg)
Andreas Färber April 12, 2015, 8:39 a.m. UTC | #6
Am 12.04.2015 um 10:34 schrieb Alexander Graf:
>> Am 11.04.2015 um 23:46 schrieb Andreas Färber <afaerber@suse.de>:
>>
>>> Am 11.04.2015 um 23:33 schrieb Peter Maydell:
>>>> On 11 April 2015 at 21:28, Andreas Färber <afaerber@suse.de> wrote:
>>>> Are you sure that QEMU / runcom / pi_10.com still try to map to that
>>>> address zero?
>>>
>>> It's the first thing runcom.c's main() does after checking you've
>>> passed it enough command line arguments.
>>
>> This?
>>
>>    vm86_mem = mmap((void *)0x00000000, 0x110000,
>>                    PROT_WRITE | PROT_READ | PROT_EXEC,
>>                    MAP_FIXED | MAP_ANON | MAP_PRIVATE, -1, 0);
>>    if (vm86_mem == MAP_FAILED) {
>>        perror("mmap");
>>        exit(1);
>>    }
>>
>> For whatever reason it does not take the MAP_FAILED path here...
> 
> Wasn't a NULL argument for the address a hint saying "map wherever you please"? I also feel like I'm missing context here - what exactly is broken?

Sorry for CC'ing you so late, I believe you and rth had been fiddling
with guest bases and that stuff back in the day... Please just take a
look at the whole series, in particular patches 3 and 7.

Thanks,
Andreas
Peter Maydell April 12, 2015, 11:45 a.m. UTC | #7
On 12 April 2015 at 09:34, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 11.04.2015 um 23:46 schrieb Andreas Färber <afaerber@suse.de>:
>>
>>> Am 11.04.2015 um 23:33 schrieb Peter Maydell:
>>>> On 11 April 2015 at 21:28, Andreas Färber <afaerber@suse.de> wrote:
>>>> Are you sure that QEMU / runcom / pi_10.com still try to map to that
>>>> address zero?
>>>
>>> It's the first thing runcom.c's main() does after checking you've
>>> passed it enough command line arguments.
>>
>> This?
>>
>>    vm86_mem = mmap((void *)0x00000000, 0x110000,
>>                    PROT_WRITE | PROT_READ | PROT_EXEC,
>>                    MAP_FIXED | MAP_ANON | MAP_PRIVATE, -1, 0);
>>    if (vm86_mem == MAP_FAILED) {
>>        perror("mmap");
>>        exit(1);
>>    }
>>
>> For whatever reason it does not take the MAP_FAILED path here...
>
> Wasn't a NULL argument for the address a hint saying "map wherever
> you please"?

Only if you don't also say MAP_FIXED.

That aside, I've now looked more carefully at the makefile,
and we're not running 'runcom' directly, we're running it
within QEMU's user-mode. So a guest allocation at vaddr 0
isn't going to turn into a host mmap at vaddr 0 (we actually
query the host mmap_min_addr parameter in linux-user/main.c).

So I think the issue noted in the comment on the makefile
(back in 2010) would have been a bug which we've presumably
fixed in the meantime by adding support for guest_base and
making sure it works correctly. (Looking at the revision
log suggests that guest_base should in theory have already
supported adjustment based on mmap_min_addr at the point
when runcom was taken out of the makefile, but presumably
there was a bug.)

-- PMM
diff mbox

Patch

diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
index f853d54..1ba4264 100644
--- a/tests/tcg/Makefile
+++ b/tests/tcg/Makefile
@@ -15,9 +15,6 @@  LDFLAGS=
 
 # TODO: automatically detect ARM and MIPS compilers, and run those too
 
-# runcom maps page 0, so it requires root privileges
-# also, pi_10.com runs indefinitely
-
 I386_TESTS=hello-i386 \
 	   linux-test \
 	   testthread \
@@ -25,7 +22,7 @@  I386_TESTS=hello-i386 \
 	   test-i386 \
 	   test-i386-fprem \
 	   test-mmap \
-	   # runcom
+	   runcom
 
 # native i386 compilers sometimes are not biarch.  assume cross-compilers are
 ifneq ($(ARCH),i386)