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

[Christoph Schlameuss <schlameuss@linux.ibm.com>]

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