[Qemu-devel] [PATCH for-2.12 0/7] Test more machines and TCG CPUs automatically

[Qemu-devel] [PATCH for-2.12 0/7] Test more machines and TCG CPUs automatically

[Thomas Huth]

During the 2.11 development cycle, a bunch of ARM boards were broken
completely, and nobody noticed it for a couple of weeks. Also a lot
of TCG CPUs do not get any test coverage at all yet (especially since
the old tests in the tests/tcg/ are still pretty much dead today).
So we really should improve our testing in this area.

This patch series now tries to improve the test coverage at least a
little bit: The second patch adds the possibility to the boot-serial
test to also load home-brew mini-kernels and firmware blobs, and the
following patches then add some tests for various boards / CPUs with
some few lines of self-made assembly.

Thomas Huth (7):
  tests/boot-serial-test: Make sure that we check the timeout regularly
  tests/boot-serial-test: Add code to allow to specify our own kernel or
    bios
  tests/boot-serial-test: Add support for the mcf5208evb board
  tests/boot-serial-test: Add tests for microblaze boards
  hw/moxie/moxiesim: Add support for loading a BIOS on moxiesim
  tests/boot-serial-test: Add a test for the moxiesim machine
  tests/boot-serial-test: Add support for the raspi2 machine

 hw/moxie/moxiesim.c      |  12 +++++-
 tests/Makefile.include   |   7 ++++
 tests/boot-serial-test.c | 106 ++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 109 insertions(+), 16 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.12 1/7] tests/boot-serial-test: Make sure that we check the timeout regularly

[Thomas Huth]

If the guest continuesly writes characters to the UART, we never leave
the inner while loop and thus never check whether we've reached the
timeout value. So if we fail to find the expected string in the UART
output, the test just hangs and never finishs. Use a counter to regularly
break out of the while loop to check the timeout.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/boot-serial-test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index c935d69..fa4183d 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -43,12 +43,13 @@ static testdef_t tests[] = {
 static void check_guest_output(const testdef_t *test, int fd)
 {
     bool output_ok = false;
-    int i, nbr, pos = 0;
+    int i, nbr, pos = 0, ccnt;
     char ch;
 
     /* Poll serial output... Wait at most 60 seconds */
     for (i = 0; i < 6000; ++i) {
-        while ((nbr = read(fd, &ch, 1)) == 1) {
+        ccnt = 0;
+        while ((nbr = read(fd, &ch, 1)) == 1 && ccnt++ < 512) {
             if (ch == test->expect[pos]) {
                 pos += 1;
                 if (test->expect[pos] == '\0') {
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.12 2/7] tests/boot-serial-test: Add code to allow to specify our own kernel or bios

[Thomas Huth]

QEMU only ships with some few firmware images, i.e. we can currently run
the boot-serial test only on a very limited set of machines. But writing
some characters to the default UART of a machine can often be done with
some few lines of assembly, so we add the possibility to the boot-serial
tester to use its own mini-kernels or mini-firmwares. We write such images
then into a file that we can load with the "-kernel" or "-bios" parameter
when we launch QEMU.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include   |  2 ++
 tests/boot-serial-test.c | 54 +++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index c002352..41ca44c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -297,6 +297,8 @@ gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y)
 
 check-qtest-alpha-y = tests/boot-serial-test$(EXESUF)
 
+check-qtest-m68k-y = tests/boot-serial-test$(EXESUF)
+
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index fa4183d..d997269 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -7,9 +7,10 @@
  * or later. See the COPYING file in the top-level directory.
  *
  * This test is used to check that the serial output of the firmware
- * (that we provide for some machines) contains an expected string.
- * Thus we check that the firmware still boots at least to a certain
- * point and so we know that the machine is not completely broken.
+ * (that we provide for some machines) or some small mini-kernels that
+ * we provide here contains an expected string. Thus we check that the
+ * firmware/kernel still boots at least to a certain point and so we
+ * know that the machine is not completely broken.
  */
 
 #include "qemu/osdep.h"
@@ -20,6 +21,9 @@ typedef struct testdef {
     const char *machine;    /* Name of the machine */
     const char *extra;      /* Additional parameters */
     const char *expect;     /* Expected string in the serial output */
+    size_t codesize;        /* Size of the kernel or bios data */
+    const uint8_t *kernel;  /* Set in case we use our own mini kernel */
+    const uint8_t *bios;    /* Set in case we use our own mini bios */
 } testdef_t;
 
 static testdef_t tests[] = {
@@ -72,26 +76,52 @@ done:
 static void test_machine(const void *data)
 {
     const testdef_t *test = data;
-    char tmpname[] = "/tmp/qtest-boot-serial-XXXXXX";
-    int fd;
+    char serialtmp[] = "/tmp/qtest-boot-serial-sXXXXXX";
+    char codetmp[] = "/tmp/qtest-boot-serial-cXXXXXX";
+    const char *codeparam = "";
+    const uint8_t *code = NULL;
+    int ser_fd;
 
-    fd = mkstemp(tmpname);
-    g_assert(fd != -1);
+    ser_fd = mkstemp(serialtmp);
+    g_assert(ser_fd != -1);
+
+    if (test->kernel) {
+        code = test->kernel;
+        codeparam = "-kernel";
+    } else if (test->bios) {
+        code = test->bios;
+        codeparam = "-bios";
+    }
+
+    if (code) {
+        ssize_t wlen;
+        int code_fd;
+
+        code_fd = mkstemp(codetmp);
+        g_assert(code_fd != -1);
+        wlen = write(code_fd, code, test->codesize);
+        g_assert(wlen == test->codesize);
+        close(code_fd);
+    }
 
     /*
      * Make sure that this test uses tcg if available: It is used as a
      * fast-enough smoketest for that.
      */
-    global_qtest = qtest_startf("-M %s,accel=tcg:kvm "
+    global_qtest = qtest_startf("%s %s -M %s,accel=tcg:kvm "
                                 "-chardev file,id=serial0,path=%s "
                                 "-no-shutdown -serial chardev:serial0 %s",
-                                test->machine, tmpname, test->extra);
-    unlink(tmpname);
+                                codeparam, code ? codetmp : "",
+                                test->machine, serialtmp, test->extra);
+    unlink(serialtmp);
+    if (code) {
+        unlink(codetmp);
+    }
 
-    check_guest_output(test, fd);
+    check_guest_output(test, ser_fd);
     qtest_quit(global_qtest);
 
-    close(fd);
+    close(ser_fd);
 }
 
 int main(int argc, char *argv[])
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Thomas Huth]

We can output a character quite easily here with some few lines of
assembly that we provide as a mini-kernel for this board.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/boot-serial-test.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index d997269..dd3828c 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -16,6 +16,14 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 
+static const uint8_t kernel_mcf5208[] = {
+    0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00,     /* lea 0xfc060000,%a0 */
+    0x10, 0x3c, 0x00, 0x54,                 /* move.b #'T',%d0 */
+    0x11, 0x7c, 0x00, 0x04, 0x00, 0x08,     /* move.b #4,8(%a0)     Enable TX */
+    0x11, 0x40, 0x00, 0x0c,                 /* move.b %d0,12(%a0)   Print 'T' */
+    0x60, 0xfa                              /* bra.s  loop */
+};
+
 typedef struct testdef {
     const char *arch;       /* Target architecture */
     const char *machine;    /* Name of the machine */
@@ -41,6 +49,8 @@ static testdef_t tests[] = {
     { "x86_64", "q35", "-device sga", "SGABIOS" },
     { "s390x", "s390-ccw-virtio",
       "-nodefaults -device sclpconsole,chardev=serial0", "virtio device" },
+    { "m68k", "mcf5208evb", "", "TT", sizeof(kernel_mcf5208), kernel_mcf5208 },
+
     { NULL }
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Peter Maydell]

On 30 November 2017 at 08:53, Thomas Huth <thuth@redhat.com> wrote:
> We can output a character quite easily here with some few lines of
> assembly that we provide as a mini-kernel for this board.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/boot-serial-test.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> index d997269..dd3828c 100644
> --- a/tests/boot-serial-test.c
> +++ b/tests/boot-serial-test.c
> @@ -16,6 +16,14 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>
> +static const uint8_t kernel_mcf5208[] = {
> +    0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00,     /* lea 0xfc060000,%a0 */
> +    0x10, 0x3c, 0x00, 0x54,                 /* move.b #'T',%d0 */
> +    0x11, 0x7c, 0x00, 0x04, 0x00, 0x08,     /* move.b #4,8(%a0)     Enable TX */
> +    0x11, 0x40, 0x00, 0x0c,                 /* move.b %d0,12(%a0)   Print 'T' */
> +    0x60, 0xfa                              /* bra.s  loop */
> +};

This approach doesn't seem to be scalable to me -- are we
really going to have 50 or more fragments of hand-coded hex in
this file to cover the various board models?

I'd much rather see us have a framework for being able
to build test blobs from source using a cross compiler
setup (and docker or similar so anybody can rebuild
the test blobs). That will be much easier to maintain
and easier to extend to having tests that test other
parts of the board or other aspects of TCG emulation.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Thomas Huth]

On 30.11.2017 13:14, Peter Maydell wrote:
> On 30 November 2017 at 08:53, Thomas Huth <thuth@redhat.com> wrote:
>> We can output a character quite easily here with some few lines of
>> assembly that we provide as a mini-kernel for this board.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  tests/boot-serial-test.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
>> index d997269..dd3828c 100644
>> --- a/tests/boot-serial-test.c
>> +++ b/tests/boot-serial-test.c
>> @@ -16,6 +16,14 @@
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>>
>> +static const uint8_t kernel_mcf5208[] = {
>> +    0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00,     /* lea 0xfc060000,%a0 */
>> +    0x10, 0x3c, 0x00, 0x54,                 /* move.b #'T',%d0 */
>> +    0x11, 0x7c, 0x00, 0x04, 0x00, 0x08,     /* move.b #4,8(%a0)     Enable TX */
>> +    0x11, 0x40, 0x00, 0x0c,                 /* move.b %d0,12(%a0)   Print 'T' */
>> +    0x60, 0xfa                              /* bra.s  loop */
>> +};
> 
> This approach doesn't seem to be scalable to me -- are we
> really going to have 50 or more fragments of hand-coded hex in
> this file to cover the various board models?

No, since this only works for certain few boards with these criteria:

1) There must be a way to load such small blobs either with -kernel or
-bios. Many boards only support loading kernels in the ELF format, and
we certainly don't want to encode the hex-dump of such a kernel here.
Or some boards / CPUs also support -bios, but the entry point must be at
the end of a large image, so that also does not make sense to include
them here.

2) The UART must be usable with some few lines of assembly. That's also
not always the case.

So no, this is really not meant to scale to all boards, it's just meant
to get at least some few more test coverage, especially while we're not
having other test mechanisms in place yet. (e.g. how long are we talking
about reviving tests/tcg/ again already? ... some attempts have been
made to make it compilable again, but non of the patch series has been
included yet so far)

> I'd much rather see us have a framework for being able
> to build test blobs from source using a cross compiler
> setup (and docker or similar so anybody can rebuild
> the test blobs). That will be much easier to maintain
> and easier to extend to having tests that test other
> parts of the board or other aspects of TCG emulation.

I agree that this idea is way more flexible and could cover many more
boards, but there are also things that need to be solved first:
Not everybody has docker / cross-compilers installed or can use them, so
we likely need a set of pre-build binary images somewhere. But we
certainly don't want to include megabytes of blobs in the git repository
... so this would need to go into an external repository instead. Then
you likely can not include this in "make check" so easily anymore
(unless you force everybody to check out the external repository with
the git-submodule.sh script - but I really dislike this idea) ...

So yes, we should have better test coverage for almost all machines if
possible, but I don't see that this is happening soon or could be really
tightly integrated into "make check". I.e. the boot-serial tester could
fill at least parts of this gap, I think.

 Thomas

Re: [Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Paolo Bonzini]

On 30/11/2017 13:14, Peter Maydell wrote:
> On 30 November 2017 at 08:53, Thomas Huth <thuth@redhat.com> wrote:
>> +static const uint8_t kernel_mcf5208[] = {
>> +    0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00,     /* lea 0xfc060000,%a0 */
>> +    0x10, 0x3c, 0x00, 0x54,                 /* move.b #'T',%d0 */
>> +    0x11, 0x7c, 0x00, 0x04, 0x00, 0x08,     /* move.b #4,8(%a0)     Enable TX */
>> +    0x11, 0x40, 0x00, 0x0c,                 /* move.b %d0,12(%a0)   Print 'T' */
>> +    0x60, 0xfa                              /* bra.s  loop */
>> +};
> 
> This approach doesn't seem to be scalable to me -- are we
> really going to have 50 or more fragments of hand-coded hex in
> this file to cover the various board models?
> 
> I'd much rather see us have a framework for being able
> to build test blobs from source using a cross compiler
> setup (and docker or similar so anybody can rebuild
> the test blobs). That will be much easier to maintain
> and easier to extend to having tests that test other
> parts of the board or other aspects of TCG emulation.

It seems a bit overkill, as these snippets are ~16 bytes long.

However, it would be useful to have a basic patching mechanism so that
board descriptions could include a common hand-coded const array and
place an address at a given offset.  So you'd have

    struct HexFirmware {
        int     patch_offset;
        short   patch_size;
        bool    patch_bigendian;
        uint8_t data[32];
    }

and microblaze boards could have:

struct HexFirmware kernel_microblaze = {
    .patch_offset    = 0,
    .patch_size      = 2,
    .patch_bigendian = false,
    .data = {
        0xaa, 0xaa, 0x00, 0xb0,                 /* imm   0x???? */
        0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
        0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
        0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
        0xfc, 0xff, 0x00, 0xb8,                 /* bri   -4  loop */
    }
};

...

    { "microblaze", "petalogix-s3adsp1800", "", "TT",
      kernel_microblaze, 0x8400 },
    { "microblazeel", "petalogix-ml605", "", "TT",
      kernel_microblaze, 0x83a0 },

Likewise, you could have just two copies of the code for all ARM boards
that have a pl011 (or any other UART with a simple byte-long transmit
register), one 32-bit and one 64-bit.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Thomas Huth]

On 30.11.2017 13:40, Paolo Bonzini wrote:
> On 30/11/2017 13:14, Peter Maydell wrote:
>> On 30 November 2017 at 08:53, Thomas Huth <thuth@redhat.com> wrote:
>>> +static const uint8_t kernel_mcf5208[] = {
>>> +    0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00,     /* lea 0xfc060000,%a0 */
>>> +    0x10, 0x3c, 0x00, 0x54,                 /* move.b #'T',%d0 */
>>> +    0x11, 0x7c, 0x00, 0x04, 0x00, 0x08,     /* move.b #4,8(%a0)     Enable TX */
>>> +    0x11, 0x40, 0x00, 0x0c,                 /* move.b %d0,12(%a0)   Print 'T' */
>>> +    0x60, 0xfa                              /* bra.s  loop */
>>> +};
>>
>> This approach doesn't seem to be scalable to me -- are we
>> really going to have 50 or more fragments of hand-coded hex in
>> this file to cover the various board models?
>>
>> I'd much rather see us have a framework for being able
>> to build test blobs from source using a cross compiler
>> setup (and docker or similar so anybody can rebuild
>> the test blobs). That will be much easier to maintain
>> and easier to extend to having tests that test other
>> parts of the board or other aspects of TCG emulation.
> 
> It seems a bit overkill, as these snippets are ~16 bytes long.
> 
> However, it would be useful to have a basic patching mechanism so that
> board descriptions could include a common hand-coded const array and
> place an address at a given offset.  So you'd have
> 
>     struct HexFirmware {
>         int     patch_offset;
>         short   patch_size;
>         bool    patch_bigendian;
>         uint8_t data[32];
>     }
> 
> and microblaze boards could have:
> 
> struct HexFirmware kernel_microblaze = {
>     .patch_offset    = 0,
>     .patch_size      = 2,
>     .patch_bigendian = false,
>     .data = {
>         0xaa, 0xaa, 0x00, 0xb0,                 /* imm   0x???? */
>         0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
>         0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
>         0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
>         0xfc, 0xff, 0x00, 0xb8,                 /* bri   -4  loop */
>     }
> };
> 
> ...
> 
>     { "microblaze", "petalogix-s3adsp1800", "", "TT",
>       kernel_microblaze, 0x8400 },
>     { "microblazeel", "petalogix-ml605", "", "TT",
>       kernel_microblaze, 0x83a0 },

The two micrablaze data arrays are completely different, since one is
big endian, the other is little, so I'd need to byte-swap the whole
array on the fly, too. Not sure whether it makes sense to add such
complex code just to safe 16 bytes of blob data...?

> Likewise, you could have just two copies of the code for all ARM boards
> that have a pl011 (or any other UART with a simple byte-long transmit
> register), one 32-bit and one 64-bit.

OK, having a patching mechanism in place likely makes sense as soon as
we want to include multiple ARM boards here. I can do that, but I'd
rather like to do it as a second step. It was already quite hard work to
come up with all these assembler snippets (from CPUs where I don't have
a clue of) and to determine which machines could be tested this way at
all, so I'd first like to wait and see whether this patch series is
acceptable at all or not, since Peter has objections.

 Thomas

Re: [Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Paolo Bonzini]

On 30/11/2017 13:51, Thomas Huth wrote:
> The two micrablaze data arrays are completely different, since one is
> big endian, the other is little, so I'd need to byte-swap the whole
> array on the fly, too. Not sure whether it makes sense to add such
> complex code just to safe 16 bytes of blob data...?

*facepalm* :)

>> Likewise, you could have just two copies of the code for all ARM boards
>> that have a pl011 (or any other UART with a simple byte-long transmit
>> register), one 32-bit and one 64-bit.
> 
> OK, having a patching mechanism in place likely makes sense as soon as
> we want to include multiple ARM boards here. I can do that, but I'd
> rather like to do it as a second step. It was already quite hard work to
> come up with all these assembler snippets (from CPUs where I don't have
> a clue of) and to determine which machines could be tested this way at
> all, so I'd first like to wait and see whether this patch series is
> acceptable at all or not, since Peter has objections.

Given your remark on endianness then no, having one snippet per binary
is enough.

Paolo

Re: [Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Peter Maydell]

On 30 November 2017 at 12:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 30/11/2017 13:14, Peter Maydell wrote:
>> On 30 November 2017 at 08:53, Thomas Huth <thuth@redhat.com> wrote:
>>> +static const uint8_t kernel_mcf5208[] = {
>>> +    0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00,     /* lea 0xfc060000,%a0 */
>>> +    0x10, 0x3c, 0x00, 0x54,                 /* move.b #'T',%d0 */
>>> +    0x11, 0x7c, 0x00, 0x04, 0x00, 0x08,     /* move.b #4,8(%a0)     Enable TX */
>>> +    0x11, 0x40, 0x00, 0x0c,                 /* move.b %d0,12(%a0)   Print 'T' */
>>> +    0x60, 0xfa                              /* bra.s  loop */
>>> +};
>>
>> This approach doesn't seem to be scalable to me -- are we
>> really going to have 50 or more fragments of hand-coded hex in
>> this file to cover the various board models?
>>
>> I'd much rather see us have a framework for being able
>> to build test blobs from source using a cross compiler
>> setup (and docker or similar so anybody can rebuild
>> the test blobs). That will be much easier to maintain
>> and easier to extend to having tests that test other
>> parts of the board or other aspects of TCG emulation.
>
> It seems a bit overkill, as these snippets are ~16 bytes long.

They're 16 bytes long because that's about the limit of
what you can do with this approach. The consequence is that
they barely test anything at all. A more sensible framework
would allow:
 * better testing of TCG instructions more generally
 * writing your test cases in C
 * more interesting board dependent tests
 * "integration test" setups like 'boot entire kernel'
 * etc

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.12 3/7] tests/boot-serial-test: Add support for the mcf5208evb board

[Paolo Bonzini]

On 30/11/2017 13:51, Peter Maydell wrote:
> On 30 November 2017 at 12:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 30/11/2017 13:14, Peter Maydell wrote:
>>> On 30 November 2017 at 08:53, Thomas Huth <thuth@redhat.com> wrote:
>>>> +static const uint8_t kernel_mcf5208[] = {
>>>> +    0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00,     /* lea 0xfc060000,%a0 */
>>>> +    0x10, 0x3c, 0x00, 0x54,                 /* move.b #'T',%d0 */
>>>> +    0x11, 0x7c, 0x00, 0x04, 0x00, 0x08,     /* move.b #4,8(%a0)     Enable TX */
>>>> +    0x11, 0x40, 0x00, 0x0c,                 /* move.b %d0,12(%a0)   Print 'T' */
>>>> +    0x60, 0xfa                              /* bra.s  loop */
>>>> +};
>>>
>>> This approach doesn't seem to be scalable to me -- are we
>>> really going to have 50 or more fragments of hand-coded hex in
>>> this file to cover the various board models?
>>>
>>> I'd much rather see us have a framework for being able
>>> to build test blobs from source using a cross compiler
>>> setup (and docker or similar so anybody can rebuild
>>> the test blobs). That will be much easier to maintain
>>> and easier to extend to having tests that test other
>>> parts of the board or other aspects of TCG emulation.
>>
>> It seems a bit overkill, as these snippets are ~16 bytes long.
> 
> They're 16 bytes long because that's about the limit of
> what you can do with this approach. The consequence is that
> they barely test anything at all.

Certainly they are an awful test for boards, but they are a great smoke
test for TCG changes that require modifications in all target/
subdirectories.

The infrastructure you want for integration tests is already provided by
kvm-unit-tests, which satisfies at least bullets 2-4 from your list
below (the first is unclear to me).

Thanks,

Paolo

> A more sensible framework
> would allow:
>  * better testing of TCG instructions more generally
>  * writing your test cases in C
>  * more interesting board dependent tests
>  * "integration test" setups like 'boot entire kernel'
>  * etc
> 
> thanks
> -- PMM
> 

[Qemu-devel] [PATCH for-2.12 4/7] tests/boot-serial-test: Add tests for microblaze boards

[Thomas Huth]

This adds two simple TCG + UART tests for the microblaze boards,
one in big endian mode, and one in little endian mode.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include   |  2 ++
 tests/boot-serial-test.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 41ca44c..cd908da 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -299,6 +299,8 @@ check-qtest-alpha-y = tests/boot-serial-test$(EXESUF)
 
 check-qtest-m68k-y = tests/boot-serial-test$(EXESUF)
 
+check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF)
+
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index dd3828c..a39273a 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -24,6 +24,22 @@ static const uint8_t kernel_mcf5208[] = {
     0x60, 0xfa                              /* bra.s  loop */
 };
 
+static const uint8_t kernel_pls3adsp1800[] = {
+    0xb0, 0x00, 0x84, 0x00,                 /* imm   0x8400 */
+    0x30, 0x60, 0x00, 0x04,                 /* addik r3,r0,4 */
+    0x30, 0x80, 0x00, 0x54,                 /* addik r4,r0,'T' */
+    0xf0, 0x83, 0x00, 0x00,                 /* sbi   r4,r3,0 */
+    0xb8, 0x00, 0xff, 0xfc                  /* bri   -4  loop */
+};
+
+static const uint8_t kernel_plml605[] = {
+    0xe0, 0x83, 0x00, 0xb0,                 /* imm   0x83e0 */
+    0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
+    0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
+    0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
+    0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
+};
+
 typedef struct testdef {
     const char *arch;       /* Target architecture */
     const char *machine;    /* Name of the machine */
@@ -50,6 +66,10 @@ static testdef_t tests[] = {
     { "s390x", "s390-ccw-virtio",
       "-nodefaults -device sclpconsole,chardev=serial0", "virtio device" },
     { "m68k", "mcf5208evb", "", "TT", sizeof(kernel_mcf5208), kernel_mcf5208 },
+    { "microblaze", "petalogix-s3adsp1800", "", "TT",
+      sizeof(kernel_pls3adsp1800), kernel_pls3adsp1800 },
+    { "microblazeel", "petalogix-ml605", "", "TT",
+      sizeof(kernel_plml605), kernel_plml605 },
 
     { NULL }
 };
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.12 5/7] hw/moxie/moxiesim: Add support for loading a BIOS on moxiesim

[Thomas Huth]

The moxiesim machine already defines a memory region for a firmware,
but does not provide the possibility to load an image via "-bios" yet.
This will be needed for the boot-serial tester, so let's add support
for "-bios" here now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/moxie/moxiesim.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/moxie/moxiesim.c b/hw/moxie/moxiesim.c
index 3ba5848..00f56df 100644
--- a/hw/moxie/moxiesim.c
+++ b/hw/moxie/moxiesim.c
@@ -25,6 +25,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
@@ -41,6 +42,8 @@
 #include "elf.h"
 
 #define PHYS_MEM_BASE 0x80000000
+#define FIRMWARE_BASE 0x1000
+#define FIRMWARE_SIZE (128 * 0x1000)
 
 typedef struct {
     uint64_t ram_size;
@@ -123,8 +126,8 @@ static void moxiesim_init(MachineState *machine)
     memory_region_init_ram(ram, NULL, "moxiesim.ram", ram_size, &error_fatal);
     memory_region_add_subregion(address_space_mem, ram_base, ram);
 
-    memory_region_init_ram(rom, NULL, "moxie.rom", 128 * 0x1000, &error_fatal);
-    memory_region_add_subregion(get_system_memory(), 0x1000, rom);
+    memory_region_init_ram(rom, NULL, "moxie.rom", FIRMWARE_SIZE, &error_fatal);
+    memory_region_add_subregion(get_system_memory(), FIRMWARE_BASE, rom);
 
     if (kernel_filename) {
         loader_params.ram_size = ram_size;
@@ -133,6 +136,11 @@ static void moxiesim_init(MachineState *machine)
         loader_params.initrd_filename = initrd_filename;
         load_kernel(cpu, &loader_params);
     }
+    if (bios_name) {
+        if (load_image_targphys(bios_name, FIRMWARE_BASE, FIRMWARE_SIZE) < 0) {
+            error_report("Failed to load firmware '%s'", bios_name);
+        }
+    }
 
     /* A single 16450 sits at offset 0x3f8.  */
     if (serial_hds[0]) {
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.12 6/7] tests/boot-serial-test: Add a test for the moxiesim machine

[Thomas Huth]

Now that moxiesim supports the -bios parameter, we can check this machine
in the boot-serial tester, too, by supplying a mini bios that only writes
'T' characters to the UART.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include   | 2 ++
 tests/boot-serial-test.c | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index cd908da..00c1e11 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -307,6 +307,8 @@ check-qtest-mips64-y = tests/endianness-test$(EXESUF)
 
 check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
 
+check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
+
 check-qtest-ppc-y = tests/endianness-test$(EXESUF)
 check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc-y += tests/prom-env-test$(EXESUF)
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index a39273a..1deddb8 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -40,6 +40,13 @@ static const uint8_t kernel_plml605[] = {
     0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
 };
 
+static const uint8_t bios_moxiesim[] = {
+    0x20, 0x10, 0x00, 0x00, 0x03, 0xf8,     /* ldi.s r1,0x3f8 */
+    0x1b, 0x20, 0x00, 0x00, 0x00, 0x54,     /* ldi.b r2,'T' */
+    0x1e, 0x12,                             /* st.b  r1,r2 */
+    0x1a, 0x00, 0x00, 0x00, 0x10, 0x00      /* jmpa  0x1000 */
+};
+
 typedef struct testdef {
     const char *arch;       /* Target architecture */
     const char *machine;    /* Name of the machine */
@@ -70,6 +77,7 @@ static testdef_t tests[] = {
       sizeof(kernel_pls3adsp1800), kernel_pls3adsp1800 },
     { "microblazeel", "petalogix-ml605", "", "TT",
       sizeof(kernel_plml605), kernel_plml605 },
+    { "moxie", "moxiesim", "", "TT", sizeof(bios_moxiesim), 0, bios_moxiesim },
 
     { NULL }
 };
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.12 7/7] tests/boot-serial-test: Add support for the raspi2 machine

[Thomas Huth]

The raspi2 machine supports loading firmware images, so we can easily
load a small test sequence as raw binary blob here to test the UART.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include   | 1 +
 tests/boot-serial-test.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 00c1e11..09025dc 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -361,6 +361,7 @@ check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
 gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
 check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
 gcov-files-arm-y += hw/timer/arm_mptimer.c
+check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 1deddb8..663b78b 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -47,6 +47,14 @@ static const uint8_t bios_moxiesim[] = {
     0x1a, 0x00, 0x00, 0x00, 0x10, 0x00      /* jmpa  0x1000 */
 };
 
+static const uint8_t bios_raspi2[] = {
+    0x08, 0x30, 0x9f, 0xe5,                 /* ldr   r3,[pc,#8]    Get base */
+    0x54, 0x20, 0xa0, 0xe3,                 /* mov     r2,#'T' */
+    0x00, 0x20, 0xc3, 0xe5,                 /* strb    r2,[r3] */
+    0xfb, 0xff, 0xff, 0xea,                 /* b       loop */
+    0x00, 0x10, 0x20, 0x3f,                 /* 0x3f201000 = UART0 base addr */
+};
+
 typedef struct testdef {
     const char *arch;       /* Target architecture */
     const char *machine;    /* Name of the machine */
@@ -78,6 +86,7 @@ static testdef_t tests[] = {
     { "microblazeel", "petalogix-ml605", "", "TT",
       sizeof(kernel_plml605), kernel_plml605 },
     { "moxie", "moxiesim", "", "TT", sizeof(bios_moxiesim), 0, bios_moxiesim },
+    { "arm", "raspi2", "", "TT", sizeof(bios_raspi2), 0, bios_raspi2 },
 
     { NULL }
 };
-- 
1.8.3.1