lkml.org 
[lkml]   [2019]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] KVM: selftests: Enable dirty_log_test on s390x
From
Date
On 30/07/2019 12.57, Andrew Jones wrote:
> On Tue, Jul 30, 2019 at 12:01:12PM +0200, Thomas Huth wrote:
>> To run the dirty_log_test on s390x, we have to make sure that we
>> access the dirty log bitmap with little endian byte ordering and
>> we have to properly align the memslot of the guest.
>> Also all dirty bits of a segment are set once on s390x when one
>> of the pages of a segment are written to for the first time, so
>> we have to make sure that we touch all pages during the first
>> iteration to keep the test in sync here.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
[...]
>> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
>> index ceb52b952637..7a1223ad0ff3 100644
>> --- a/tools/testing/selftests/kvm/dirty_log_test.c
>> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
>> @@ -26,9 +26,22 @@
>> /* The memory slot index to track dirty pages */
>> #define TEST_MEM_SLOT_INDEX 1
>>
>> +#ifdef __s390x__
>> +
>> +/*
>> + * On s390x, the ELF program is sometimes linked at 0x80000000, so we can
>> + * not use 0x40000000 here without overlapping into that region. Thus let's
>> + * use 0xc0000000 as base address there instead.
>> + */
>> +#define DEFAULT_GUEST_TEST_MEM 0xc0000000
>
> I think both x86 and aarch64 should be ok with this offset. If testing
> proves it does, then we can just change it for all architecture.

Ok. It seems to work on x86 - could you please check aarch64, since I
don't have such a system available right now?

>> +/* Dirty bitmaps are always little endian, so we need to swap on big endian */
>> +#if defined(__s390x__)
>> +# define BITOP_LE_SWIZZLE ((BITS_PER_LONG-1) & ~0x7)
>> +# define test_bit_le(nr, addr) \
>> + test_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
>> +# define set_bit_le(nr, addr) \
>> + set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
>> +# define clear_bit_le(nr, addr) \
>> + clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
>> +# define test_and_set_bit_le(nr, addr) \
>> + test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
>> +# define test_and_clear_bit_le(nr, addr) \
>> + test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
>> +#else
>> +# define test_bit_le test_bit
>> +# define set_bit_le set_bit
>> +# define clear_bit_le clear_bit
>> +# define test_and_set_bit_le test_and_set_bit
>> +# define test_and_clear_bit_le test_and_clear_bit
>> +#endif
>
> nit: does the formatting above look right after applying the patch?

It looked ok to me, but I can add some more tabs to even make it nicer :)

>> @@ -293,6 +341,10 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>> * case where the size is not aligned to 64 pages.
>> */
>> guest_num_pages = (1ul << (30 - guest_page_shift)) + 16;
>> +#ifdef __s390x__
>> + /* Round up to multiple of 1M (segment size) */
>> + guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL;
>
> We could maybe do this for all architectures as well.

It's really only needed on s390x, so I think we should keep the #ifdef here.

Thomas

\
 
 \ /
  Last update: 2019-07-31 10:20    [W:0.500 / U:0.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site