Re: [Qemu-ppc] [PATCH 1/2] tests: make pc_alloc_init/init_flags/uninit generic

[Laurent Vivier]

On 05/09/2016 18:00, Thomas Huth wrote:
> On 05.09.2016 16:38, Laurent Vivier wrote:
>> And add support for ppc64.
>>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>>  tests/Makefile.include      |  3 ++-
>>  tests/libqos/libqos.h       |  2 +-
>>  tests/libqos/malloc-ppc64.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  tests/libqos/malloc-ppc64.h | 17 +++++++++++++++++
>>  tests/libqos/malloc.c       | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  tests/libqos/malloc.h       |  3 +++
>>  6 files changed, 103 insertions(+), 2 deletions(-)
>>  create mode 100644 tests/libqos/malloc-ppc64.c
>>  create mode 100644 tests/libqos/malloc-ppc64.h
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 14be491..a286848 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -557,8 +557,9 @@ tests/test-crypto-block$(EXESUF): 
>> tests/test-crypto-block.o $(test-crypto-obj-y)
>>  
>>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o 
>> tests/libqos/malloc.o
>>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
>> +libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
> 
> Maybe this could rather be put into a libqos-malloc-obj-y variable which
> is then used to link these only to the tests that really need them?

Good idea.

> 
>>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
>> -libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
>> +libqos-pc-obj-y += tests/libqos/libqos-pc.o
>>  libqos-pc-obj-y += tests/libqos/ahci.o
>>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
>> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
>> index 604980d..7b71607 100644
>> --- a/tests/libqos/libqos.h
>> +++ b/tests/libqos/libqos.h
>> @@ -3,7 +3,7 @@
>>  
>>  #include "libqtest.h"
>>  #include "libqos/pci.h"
>> -#include "libqos/malloc-pc.h"
>> +#include "libqos/malloc.h"
>>  
>>  typedef struct QOSOps {
>>      QGuestAllocator *(*init_allocator)(QAllocOpts);
>> diff --git a/tests/libqos/malloc-ppc64.c b/tests/libqos/malloc-ppc64.c
>> new file mode 100644
>> index 0000000..33fd33f
>> --- /dev/null
>> +++ b/tests/libqos/malloc-ppc64.c
>> @@ -0,0 +1,38 @@
>> +/*
>> + * libqos malloc support for PPC64
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqos/malloc-ppc64.h"
>> +
>> +#include "qemu-common.h"
>> +
>> +#define PAGE_SIZE (4096)
> 
> No need for parenthesis here.

You really don't like parenthesis...

> 
>> +/* Memory must be a multiple of 256 MB,
>> + * so we have at least 256MB
>> + */
>> +#define PPC64_MIN_SIZE 0x10000000
>> +
>> +void ppc64_alloc_uninit(QGuestAllocator *allocator)
>> +{
>> +    alloc_uninit(allocator);
>> +}
>> +
>> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags)
>> +{
>> +    QGuestAllocator *s;
>> +
>> +    s = alloc_init_flags(flags, 1 << 20, PPC64_MIN_SIZE);
>> +    alloc_set_page_size(s, PAGE_SIZE);
>> +
>> +    return s;
>> +}
>> +
>> +inline QGuestAllocator *ppc64_alloc_init(void)
>> +{
>> +    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
>> +}
> 
> Why is this function marked as inline? ... it is the last function in
> the source file...

stupidly copied from tests/libqos/malloc-generic.c

> 
> Apart from that, the patch looks fine to me.


Thanks,
Laurent