mbox series

[v1,0/9] KVM selftests for s390x

Message ID 20190523164309.13345-1-thuth@redhat.com (mailing list archive)
Headers show
Series KVM selftests for s390x | expand

Message

Thomas Huth May 23, 2019, 4:43 p.m. UTC
This patch series enables the KVM selftests for s390x. As a first
test, the sync_regs from x86 has been adapted to s390x, and after
a fix for KVM_CAP_MAX_VCPU_ID on s390x, the kvm_create_max_vcpus
is now enabled here, too.

Please note that the ucall() interface is not used yet - since
s390x neither has PIO nor MMIO, this needs some more work first
before it becomes usable (we likely should use a DIAG hypercall
here, which is what the sync_reg test is currently using, too...
I started working on that topic, but did not finish that work
yet, so I decided to not include it yet).

RFC -> v1:
 - Rebase, needed to add the first patch for vcpu_nested_state_get/set
 - Added patch to introduce VM_MODE_DEFAULT macro
 - Improved/cleaned up the code in processor.c
 - Added patch to fix KVM_CAP_MAX_VCPU_ID on s390x
 - Added patch to enable the kvm_create_max_vcpus on s390x and aarch64

Andrew Jones (1):
  kvm: selftests: aarch64: fix default vm mode

Thomas Huth (8):
  KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86
    guard
  KVM: selftests: Guard struct kvm_vcpu_events with
    __KVM_HAVE_VCPU_EVENTS
  KVM: selftests: Introduce a VM_MODE_DEFAULT macro for the default bits
  KVM: selftests: Align memory region addresses to 1M on s390x
  KVM: selftests: Add processor code for s390x
  KVM: selftests: Add the sync_regs test for s390x
  KVM: s390: Do not report unusabled IDs via KVM_CAP_MAX_VCPU_ID
  KVM: selftests: Move kvm_create_max_vcpus test to generic code

 MAINTAINERS                                   |   2 +
 arch/mips/kvm/mips.c                          |   3 +
 arch/powerpc/kvm/powerpc.c                    |   3 +
 arch/s390/kvm/kvm-s390.c                      |   1 +
 arch/x86/kvm/x86.c                            |   3 +
 tools/testing/selftests/kvm/Makefile          |   7 +-
 .../testing/selftests/kvm/include/kvm_util.h  |  10 +
 .../selftests/kvm/include/s390x/processor.h   |  22 ++
 .../kvm/{x86_64 => }/kvm_create_max_vcpus.c   |   3 +-
 .../selftests/kvm/lib/aarch64/processor.c     |   2 +-
 tools/testing/selftests/kvm/lib/kvm_util.c    |  25 +-
 .../selftests/kvm/lib/s390x/processor.c       | 286 ++++++++++++++++++
 .../selftests/kvm/lib/x86_64/processor.c      |   2 +-
 .../selftests/kvm/s390x/sync_regs_test.c      | 151 +++++++++
 virt/kvm/arm/arm.c                            |   3 +
 virt/kvm/kvm_main.c                           |   2 -
 16 files changed, 514 insertions(+), 11 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
 rename tools/testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c (93%)
 create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
 create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c

Comments

Christian Borntraeger May 24, 2019, 11:11 a.m. UTC | #1
I do get

[10400.440298] kvm-s390: failed to commit memory region
[10400.508723] kvm-s390: failed to commit memory region

when running the tests. Will have a look.

On 23.05.19 18:43, Thomas Huth wrote:
> This patch series enables the KVM selftests for s390x. As a first
> test, the sync_regs from x86 has been adapted to s390x, and after
> a fix for KVM_CAP_MAX_VCPU_ID on s390x, the kvm_create_max_vcpus
> is now enabled here, too.
> 
> Please note that the ucall() interface is not used yet - since
> s390x neither has PIO nor MMIO, this needs some more work first
> before it becomes usable (we likely should use a DIAG hypercall
> here, which is what the sync_reg test is currently using, too...
> I started working on that topic, but did not finish that work
> yet, so I decided to not include it yet).
> 
> RFC -> v1:
>  - Rebase, needed to add the first patch for vcpu_nested_state_get/set
>  - Added patch to introduce VM_MODE_DEFAULT macro
>  - Improved/cleaned up the code in processor.c
>  - Added patch to fix KVM_CAP_MAX_VCPU_ID on s390x
>  - Added patch to enable the kvm_create_max_vcpus on s390x and aarch64
> 
> Andrew Jones (1):
>   kvm: selftests: aarch64: fix default vm mode
> 
> Thomas Huth (8):
>   KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86
>     guard
>   KVM: selftests: Guard struct kvm_vcpu_events with
>     __KVM_HAVE_VCPU_EVENTS
>   KVM: selftests: Introduce a VM_MODE_DEFAULT macro for the default bits
>   KVM: selftests: Align memory region addresses to 1M on s390x
>   KVM: selftests: Add processor code for s390x
>   KVM: selftests: Add the sync_regs test for s390x
>   KVM: s390: Do not report unusabled IDs via KVM_CAP_MAX_VCPU_ID
>   KVM: selftests: Move kvm_create_max_vcpus test to generic code
> 
>  MAINTAINERS                                   |   2 +
>  arch/mips/kvm/mips.c                          |   3 +
>  arch/powerpc/kvm/powerpc.c                    |   3 +
>  arch/s390/kvm/kvm-s390.c                      |   1 +
>  arch/x86/kvm/x86.c                            |   3 +
>  tools/testing/selftests/kvm/Makefile          |   7 +-
>  .../testing/selftests/kvm/include/kvm_util.h  |  10 +
>  .../selftests/kvm/include/s390x/processor.h   |  22 ++
>  .../kvm/{x86_64 => }/kvm_create_max_vcpus.c   |   3 +-
>  .../selftests/kvm/lib/aarch64/processor.c     |   2 +-
>  tools/testing/selftests/kvm/lib/kvm_util.c    |  25 +-
>  .../selftests/kvm/lib/s390x/processor.c       | 286 ++++++++++++++++++
>  .../selftests/kvm/lib/x86_64/processor.c      |   2 +-
>  .../selftests/kvm/s390x/sync_regs_test.c      | 151 +++++++++
>  virt/kvm/arm/arm.c                            |   3 +
>  virt/kvm/kvm_main.c                           |   2 -
>  16 files changed, 514 insertions(+), 11 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
>  rename tools/testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c (93%)
>  create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
>  create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c
>
Christian Borntraeger May 24, 2019, 12:17 p.m. UTC | #2
On 24.05.19 13:11, Christian Borntraeger wrote:
> I do get
> 
> [10400.440298] kvm-s390: failed to commit memory region
> [10400.508723] kvm-s390: failed to commit memory region
> 
> when running the tests. Will have a look.

It comes from kvm_vm_free. This calls KVM_SET_USER_MEMORY_REGION with size 0,
which the s390 code does not like.
Christian Borntraeger May 24, 2019, 12:29 p.m. UTC | #3
On 24.05.19 14:17, Christian Borntraeger wrote:
> 
> 
> On 24.05.19 13:11, Christian Borntraeger wrote:
>> I do get
>>
>> [10400.440298] kvm-s390: failed to commit memory region
>> [10400.508723] kvm-s390: failed to commit memory region
>>
>> when running the tests. Will have a look.
> 
> It comes from kvm_vm_free. This calls KVM_SET_USER_MEMORY_REGION with size 0,
> which the s390 code does not like.
> 

The doc says about  KVM_SET_USER_MEMORY_REGION:

This ioctl allows the user to create or modify a guest physical memory
slot.  When changing an existing slot, it may be moved in the guest
physical memory space, or its flags may be modified.  --> It may not be
resized. <----

$ strace -f -e trace=ioctl tools/testing/selftests/kvm/s390x/sync_regs_test 
ioctl(3, KVM_CHECK_EXTENSION, KVM_CAP_SYNC_REGS) = 1
ioctl(4, KVM_CHECK_EXTENSION, KVM_CAP_IMMEDIATE_EXIT) = 1
ioctl(3, KVM_CREATE_VM, 0)              = 4
ioctl(4, KVM_SET_USER_MEMORY_REGION, {slot=0, flags=0, guest_phys_addr=0, memory_size=2097152, userspace_addr=0x3ffac500000}) = 0
ioctl(4, KVM_CREATE_VCPU, 5)            = 7
ioctl(8, KVM_GET_VCPU_MMAP_SIZE, 0)     = 4096
ioctl(8, KVM_GET_VCPU_MMAP_SIZE, 0)     = 4096
ioctl(7, KVM_GET_SREGS, 0x3ffef0fdb90)  = 0
ioctl(7, KVM_SET_SREGS, 0x3ffef0fdb90)  = 0
ioctl(7, KVM_GET_REGS, 0x3ffef0fdcf8)   = 0
ioctl(7, KVM_SET_REGS, 0x3ffef0fdcf8)   = 0
ioctl(7, KVM_GET_SREGS, 0x3ffef0fdd78)  = 0
ioctl(7, KVM_SET_SREGS, 0x3ffef0fdd78)  = 0
ioctl(7, KVM_RUN, 0)                    = 0
ioctl(7, KVM_GET_REGS, 0x3ffef0fdf90)   = 0
ioctl(7, KVM_GET_SREGS, 0x3ffef0fe010)  = 0
ioctl(7, KVM_RUN, 0)                    = 0
ioctl(7, KVM_GET_REGS, 0x3ffef0fdf90)   = 0
ioctl(7, KVM_GET_SREGS, 0x3ffef0fe010)  = 0
ioctl(7, KVM_RUN, 0)                    = 0
ioctl(4, KVM_SET_USER_MEMORY_REGION, {slot=0, flags=0, guest_phys_addr=0, memory_size=0, userspace_addr=0x3ffac500000}) = 0
+++ exited with 0 +++

So the testcase is wrong? (I think the s390 code is also not fully correct will double check)
David Hildenbrand May 24, 2019, 12:36 p.m. UTC | #4
On 24.05.19 14:29, Christian Borntraeger wrote:
> 
> 
> On 24.05.19 14:17, Christian Borntraeger wrote:
>>
>>
>> On 24.05.19 13:11, Christian Borntraeger wrote:
>>> I do get
>>>
>>> [10400.440298] kvm-s390: failed to commit memory region
>>> [10400.508723] kvm-s390: failed to commit memory region
>>>
>>> when running the tests. Will have a look.
>>
>> It comes from kvm_vm_free. This calls KVM_SET_USER_MEMORY_REGION with size 0,
>> which the s390 code does not like.
>>
> 
> The doc says about  KVM_SET_USER_MEMORY_REGION:
> 
> This ioctl allows the user to create or modify a guest physical memory
> slot.  When changing an existing slot, it may be moved in the guest
> physical memory space, or its flags may be modified.  --> It may not be
> resized. <----

Size 0 is deleting, not resizing AFAIK.
Christian Borntraeger May 24, 2019, 12:56 p.m. UTC | #5
On 24.05.19 14:36, David Hildenbrand wrote:
> On 24.05.19 14:29, Christian Borntraeger wrote:
>>
>>
>> On 24.05.19 14:17, Christian Borntraeger wrote:
>>>
>>>
>>> On 24.05.19 13:11, Christian Borntraeger wrote:
>>>> I do get
>>>>
>>>> [10400.440298] kvm-s390: failed to commit memory region
>>>> [10400.508723] kvm-s390: failed to commit memory region
>>>>
>>>> when running the tests. Will have a look.
>>>
>>> It comes from kvm_vm_free. This calls KVM_SET_USER_MEMORY_REGION with size 0,
>>> which the s390 code does not like.
>>>
>>
>> The doc says about  KVM_SET_USER_MEMORY_REGION:
>>
>> This ioctl allows the user to create or modify a guest physical memory
>> slot.  When changing an existing slot, it may be moved in the guest
>> physical memory space, or its flags may be modified.  --> It may not be
>> resized. <----
> 
> Size 0 is deleting, not resizing AFAIK.

Right this seems to translate to KVM_MR_DELETE, which the s390 code does not handle (we
will simply deliver a page fault as we share the last page table level). 
I will have a look at implementing KVM_MR_DELETE and KVM_MR_MOVE. In fact, we should
have a testcase for that as well.
Christian Borntraeger May 24, 2019, 6:33 p.m. UTC | #6
I have now queued every patch in the kselftest branch on kvms390 at kernel.org.
I will push out for next as soon as I have some ack/nacks on the
"KVM: s390: fix memory slot handling for KVM_SET_USER_MEMORY_REGION"
patch.

On 23.05.19 18:43, Thomas Huth wrote:
> This patch series enables the KVM selftests for s390x. As a first
> test, the sync_regs from x86 has been adapted to s390x, and after
> a fix for KVM_CAP_MAX_VCPU_ID on s390x, the kvm_create_max_vcpus
> is now enabled here, too.
> 
> Please note that the ucall() interface is not used yet - since
> s390x neither has PIO nor MMIO, this needs some more work first
> before it becomes usable (we likely should use a DIAG hypercall
> here, which is what the sync_reg test is currently using, too...
> I started working on that topic, but did not finish that work
> yet, so I decided to not include it yet).
> 
> RFC -> v1:
>  - Rebase, needed to add the first patch for vcpu_nested_state_get/set
>  - Added patch to introduce VM_MODE_DEFAULT macro
>  - Improved/cleaned up the code in processor.c
>  - Added patch to fix KVM_CAP_MAX_VCPU_ID on s390x
>  - Added patch to enable the kvm_create_max_vcpus on s390x and aarch64
> 
> Andrew Jones (1):
>   kvm: selftests: aarch64: fix default vm mode
> 
> Thomas Huth (8):
>   KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86
>     guard
>   KVM: selftests: Guard struct kvm_vcpu_events with
>     __KVM_HAVE_VCPU_EVENTS
>   KVM: selftests: Introduce a VM_MODE_DEFAULT macro for the default bits
>   KVM: selftests: Align memory region addresses to 1M on s390x
>   KVM: selftests: Add processor code for s390x
>   KVM: selftests: Add the sync_regs test for s390x
>   KVM: s390: Do not report unusabled IDs via KVM_CAP_MAX_VCPU_ID
>   KVM: selftests: Move kvm_create_max_vcpus test to generic code
> 
>  MAINTAINERS                                   |   2 +
>  arch/mips/kvm/mips.c                          |   3 +
>  arch/powerpc/kvm/powerpc.c                    |   3 +
>  arch/s390/kvm/kvm-s390.c                      |   1 +
>  arch/x86/kvm/x86.c                            |   3 +
>  tools/testing/selftests/kvm/Makefile          |   7 +-
>  .../testing/selftests/kvm/include/kvm_util.h  |  10 +
>  .../selftests/kvm/include/s390x/processor.h   |  22 ++
>  .../kvm/{x86_64 => }/kvm_create_max_vcpus.c   |   3 +-
>  .../selftests/kvm/lib/aarch64/processor.c     |   2 +-
>  tools/testing/selftests/kvm/lib/kvm_util.c    |  25 +-
>  .../selftests/kvm/lib/s390x/processor.c       | 286 ++++++++++++++++++
>  .../selftests/kvm/lib/x86_64/processor.c      |   2 +-
>  .../selftests/kvm/s390x/sync_regs_test.c      | 151 +++++++++
>  virt/kvm/arm/arm.c                            |   3 +
>  virt/kvm/kvm_main.c                           |   2 -
>  16 files changed, 514 insertions(+), 11 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
>  rename tools/testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c (93%)
>  create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
>  create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c
>
Paolo Bonzini June 4, 2019, 5:19 p.m. UTC | #7
On 23/05/19 18:43, Thomas Huth wrote:
> This patch series enables the KVM selftests for s390x. As a first
> test, the sync_regs from x86 has been adapted to s390x, and after
> a fix for KVM_CAP_MAX_VCPU_ID on s390x, the kvm_create_max_vcpus
> is now enabled here, too.
> 
> Please note that the ucall() interface is not used yet - since
> s390x neither has PIO nor MMIO, this needs some more work first
> before it becomes usable (we likely should use a DIAG hypercall
> here, which is what the sync_reg test is currently using, too...
> I started working on that topic, but did not finish that work
> yet, so I decided to not include it yet).

Christian, please include this in your tree (rebasing on top of kvm/next
as soon as I push it).  Note that Thomas is away for about a month.

Paolo

> RFC -> v1:
>  - Rebase, needed to add the first patch for vcpu_nested_state_get/set
>  - Added patch to introduce VM_MODE_DEFAULT macro
>  - Improved/cleaned up the code in processor.c
>  - Added patch to fix KVM_CAP_MAX_VCPU_ID on s390x
>  - Added patch to enable the kvm_create_max_vcpus on s390x and aarch64
> 
> Andrew Jones (1):
>   kvm: selftests: aarch64: fix default vm mode
> 
> Thomas Huth (8):
>   KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86
>     guard
>   KVM: selftests: Guard struct kvm_vcpu_events with
>     __KVM_HAVE_VCPU_EVENTS
>   KVM: selftests: Introduce a VM_MODE_DEFAULT macro for the default bits
>   KVM: selftests: Align memory region addresses to 1M on s390x
>   KVM: selftests: Add processor code for s390x
>   KVM: selftests: Add the sync_regs test for s390x
>   KVM: s390: Do not report unusabled IDs via KVM_CAP_MAX_VCPU_ID
>   KVM: selftests: Move kvm_create_max_vcpus test to generic code
> 
>  MAINTAINERS                                   |   2 +
>  arch/mips/kvm/mips.c                          |   3 +
>  arch/powerpc/kvm/powerpc.c                    |   3 +
>  arch/s390/kvm/kvm-s390.c                      |   1 +
>  arch/x86/kvm/x86.c                            |   3 +
>  tools/testing/selftests/kvm/Makefile          |   7 +-
>  .../testing/selftests/kvm/include/kvm_util.h  |  10 +
>  .../selftests/kvm/include/s390x/processor.h   |  22 ++
>  .../kvm/{x86_64 => }/kvm_create_max_vcpus.c   |   3 +-
>  .../selftests/kvm/lib/aarch64/processor.c     |   2 +-
>  tools/testing/selftests/kvm/lib/kvm_util.c    |  25 +-
>  .../selftests/kvm/lib/s390x/processor.c       | 286 ++++++++++++++++++
>  .../selftests/kvm/lib/x86_64/processor.c      |   2 +-
>  .../selftests/kvm/s390x/sync_regs_test.c      | 151 +++++++++
>  virt/kvm/arm/arm.c                            |   3 +
>  virt/kvm/kvm_main.c                           |   2 -
>  16 files changed, 514 insertions(+), 11 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
>  rename tools/testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c (93%)
>  create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
>  create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c
>
Christian Borntraeger June 4, 2019, 5:37 p.m. UTC | #8
On 04.06.19 19:19, Paolo Bonzini wrote:
> On 23/05/19 18:43, Thomas Huth wrote:
>> This patch series enables the KVM selftests for s390x. As a first
>> test, the sync_regs from x86 has been adapted to s390x, and after
>> a fix for KVM_CAP_MAX_VCPU_ID on s390x, the kvm_create_max_vcpus
>> is now enabled here, too.
>>
>> Please note that the ucall() interface is not used yet - since
>> s390x neither has PIO nor MMIO, this needs some more work first
>> before it becomes usable (we likely should use a DIAG hypercall
>> here, which is what the sync_reg test is currently using, too...
>> I started working on that topic, but did not finish that work
>> yet, so I decided to not include it yet).
> 
> Christian, please include this in your tree (rebasing on top of kvm/next
> as soon as I push it).  Note that Thomas is away for about a month.

Will do. Right now it is part of my next tree on top of rc3.
> 
> Paolo
> 
>> RFC -> v1:
>>  - Rebase, needed to add the first patch for vcpu_nested_state_get/set
>>  - Added patch to introduce VM_MODE_DEFAULT macro
>>  - Improved/cleaned up the code in processor.c
>>  - Added patch to fix KVM_CAP_MAX_VCPU_ID on s390x
>>  - Added patch to enable the kvm_create_max_vcpus on s390x and aarch64
>>
>> Andrew Jones (1):
>>   kvm: selftests: aarch64: fix default vm mode
>>
>> Thomas Huth (8):
>>   KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86
>>     guard
>>   KVM: selftests: Guard struct kvm_vcpu_events with
>>     __KVM_HAVE_VCPU_EVENTS
>>   KVM: selftests: Introduce a VM_MODE_DEFAULT macro for the default bits
>>   KVM: selftests: Align memory region addresses to 1M on s390x
>>   KVM: selftests: Add processor code for s390x
>>   KVM: selftests: Add the sync_regs test for s390x
>>   KVM: s390: Do not report unusabled IDs via KVM_CAP_MAX_VCPU_ID
>>   KVM: selftests: Move kvm_create_max_vcpus test to generic code
>>
>>  MAINTAINERS                                   |   2 +
>>  arch/mips/kvm/mips.c                          |   3 +
>>  arch/powerpc/kvm/powerpc.c                    |   3 +
>>  arch/s390/kvm/kvm-s390.c                      |   1 +
>>  arch/x86/kvm/x86.c                            |   3 +
>>  tools/testing/selftests/kvm/Makefile          |   7 +-
>>  .../testing/selftests/kvm/include/kvm_util.h  |  10 +
>>  .../selftests/kvm/include/s390x/processor.h   |  22 ++
>>  .../kvm/{x86_64 => }/kvm_create_max_vcpus.c   |   3 +-
>>  .../selftests/kvm/lib/aarch64/processor.c     |   2 +-
>>  tools/testing/selftests/kvm/lib/kvm_util.c    |  25 +-
>>  .../selftests/kvm/lib/s390x/processor.c       | 286 ++++++++++++++++++
>>  .../selftests/kvm/lib/x86_64/processor.c      |   2 +-
>>  .../selftests/kvm/s390x/sync_regs_test.c      | 151 +++++++++
>>  virt/kvm/arm/arm.c                            |   3 +
>>  virt/kvm/kvm_main.c                           |   2 -
>>  16 files changed, 514 insertions(+), 11 deletions(-)
>>  create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
>>  rename tools/testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c (93%)
>>  create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
>>  create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c
>>
>