Re: [Qemu-devel] [PULL 0/7] fw_cfg: add dma interface, add strings via cmdline.

[Laszlo Ersek <lersek@redhat.com>]

On 10/19/15 13:12, Peter Maydell wrote:
> On 19 October 2015 at 10:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>   Hi,
>>
>> As promised last week I've picked up the fw_cfg patches which are ready
>> to go, so here comes the pull request for them.  Headline feature is
>> the fw_cfg dam support fo course, enabled for arm and x86.  Also small
>> fixes and an option to add string fw_cfg entries via command line.
>>
>> please pull,
>>   Gerd
>>
>> The following changes since commit 40fe17bea478793fc9106a630fa3610dad51f939:
>>
>>   hw/ide/ahci.c: Fix shift left into sign bit (2015-10-18 11:00:40 +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.kraxel.org/qemu tags/pull-fw_cfg-20151019-1
>>
>> for you to fetch changes up to 7b0eec285d447057405df73beae78c8e8aeb9793:
>>
>>   fw_cfg: Define a static signature to be returned on DMA port reads (2015-10-19 11:00:50 +0200)
>>
>> ----------------------------------------------------------------
>> fw_cfg: add dma interface, add strings via cmdline.
>>
>> ----------------------------------------------------------------
> 
> Hi. I'm afraid this fails 'make check':
> 
> TEST: tests/fw_cfg-test... (pid=17533)
>   /i386/fw_cfg/signature:                                              OK
>   /i386/fw_cfg/id:                                                     **
> ERROR:/home/petmay01/qemu/tests/fw_cfg-test.c:40:test_fw_cfg_id:
> assertion failed (qfw_cfg_get_u32(fw_cfg, FW_CFG_ID) == 1): (3 == 1)
> FAIL
> 
> (same failure on 64-bit ARM, ppc64be, OSX, 32-bit ARM).

Heh, tests. We have tests to keep fixing them. :)

Marc, can you add a hunk to patch "Implement fw_cfg DMA interface" that
changes the above test to mask out everything but bit 0? I don't think
we need to test DMA in the test suite right now (can be done in another
series I guess), but DMA's presence shouldn't break the current test. I
guess something like this would suffice:

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 9be78e9..27f115d 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -37,7 +37,7 @@ static void test_fw_cfg_signature(void)

 static void test_fw_cfg_id(void)
 {
-    g_assert_cmpint(qfw_cfg_get_u32(fw_cfg, FW_CFG_ID), ==, 1);
+    g_assert_cmpint(qfw_cfg_get_u32(fw_cfg, FW_CFG_ID) & 1, ==, 1);
 }

 static void test_fw_cfg_uuid(void)

(Sorry if this has been suggested already, trying to catch up with my
mailbox...)

Thanks
Laszlo

> 
> Windows fails to compile:
> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c: In function
> ‘fw_cfg_dma_mem_read’:
> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:406: warning:
> integer constant is too large for ‘long’ type
> 
> thanks
> -- PMM
>