Re: [PATCH v3 07/10] selftests: kvm: s390: Add uc_map_unmap VM test case

Christoph Schlameuss
2024-08-01T15:06:35Z

On Thu, 1 Aug 2024 11:08:30 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 7/30/24 9:24 AM, Christoph Schlameuss wrote:
> > Add a test case verifying basic running and interaction of ucontrol VMs.
> > Fill the segment and page tables for allocated memory and map memory on
> > first access.
> >
> > * uc_map_unmap
> > Store and load data to mapped and unmapped memory and use pic segment
> > translation handling to map memory on access.
> >
> > Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> > ---
> > .../selftests/kvm/s390x/ucontrol_test.c | 165 +++++++++++++++++-
> > 1 file changed, 164 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > index 817b1e08559c..b7f760f980fd 100644
> > --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > @@ -16,7 +16,13 @@
> > #include <linux/capability.h>
> > #include <linux/sizes.h>
> >
> > +#define UC_PIC_SEGMENT_TRANSLATION 0x10
>
> That's a bit clearer and used by KVM:
> #define PGM_SEGMENT_TRANSLATION 0x10
>

I will rename the constant here. (The original constant is defined in
kvm_host.h which is not pulled into the userspace selftests.)
Also since this is only used here so far and does not really fit into
processor.h or sie.h, I would leave that here for now.

[...]

> > @@ -245,7 +338,11 @@ static bool uc_handle_sieic(FIXTURE_DATA(uc_kvm) * self)
> > break;
> > case ICPT_INST:
> > /* end execution in caller on intercepted instruction */
> > + pr_info("sie instruction interception\n");
>
> That should have been part of an earlier patch?
>

Yes, on closer observation this is actually already needed in patch 6:
"selftests: kvm: s390: Add VM run test case".

I will also make sure all patches do run on its own again before
sending the next version.

Good catch, thank you.

[...]

I will also fix up the comments as advised.

Christoph