On Tue, 2024-02-20 at 14:34 +0100, Nina Schoetterl-Glausch wrote:
> On Thu, 2024-02-15 at 21:53 +0100, Eric Farman wrote:
> > There is a selftest that checks for an (expected) error when an
> > invalid AR is specified, but not one that exercises the AR path.
> >
> > Add a simple test that mirrors the vanilla write/read test while
> > providing an AR. An AR that contains zero will direct the CPU to
> > use the primary address space normally used anyway. AR[1] is
> > selected for this test because the host AR[1] is usually non-zero,
> > and KVM needs to correctly swap those values.
> >
> > Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
>
> Reviewed-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
>
> > ---
> > tools/testing/selftests/kvm/s390x/memop.c | 28
> > +++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/tools/testing/selftests/kvm/s390x/memop.c
> > b/tools/testing/selftests/kvm/s390x/memop.c
> > index bb3ca9a5d731..be20c26ee545 100644
> > --- a/tools/testing/selftests/kvm/s390x/memop.c
> > +++ b/tools/testing/selftests/kvm/s390x/memop.c
> > @@ -375,6 +375,29 @@ static void test_copy(void)
> > kvm_vm_free(t.kvm_vm);
> > }
> >
> > +static void test_copy_access_register(void)
> > +{
> > + struct test_default t = test_default_init(guest_copy);
> > +
> > + HOST_SYNC(t.vcpu, STAGE_INITED);
> > +
> > + prepare_mem12();
> > + t.run->psw_mask &= ~(3UL << (63 - 17));
> > + t.run->psw_mask |= 1UL << (63 - 17); /* Enable AR mode */
> > +
>
> I feel like part of the commit message should be a comment here
>
> /*
> * Guest AR[1] should be zero, in which case the primary address
> space is used.
> * The host makes use of AR[1], its value must not be used for the
> memop.
> */
> > + CHECK_N_DO(MOP, t.vcpu, LOGICAL, WRITE, mem1, t.size,
> > + GADDR_V(mem1), AR(1));
> > + HOST_SYNC(t.vcpu, STAGE_COPIED);
> > +
> > + CHECK_N_DO(MOP, t.vcpu, LOGICAL, READ, mem2, t.size,
> > + GADDR_V(mem2), AR(1));
> > + ASSERT_MEM_EQ(mem1, mem2, t.size);
> > +
> > + t.run->psw_mask &= ~(3UL << (63 - 17)); /* Disable AR
> > mode */
>
> Any reason for this? It's not necessary since the vm is going to be
> destroyed.
Nah. I think I was just putting it back the way I was, in case I did
slap another test on there, but never did.
> > +
> > + kvm_vm_free(t.kvm_vm);
> > +}
> > +
> > static void set_storage_key_range(void *addr, size_t len, uint8_t
> > key)
> > {
> > uintptr_t _addr, abs, i;
> > @@ -1101,6 +1124,11 @@ int main(int argc, char *argv[])
> > .test = test_copy_key_fetch_prot_override,
> > .requirements_met = extension_cap > 0,
> > },
> > + {
> > + .name = "copy with access register mode",
> > + .test = test_copy_access_register,
> > + .requirements_met = true,
> > + },
> > {
> > .name = "error checks with key",
> > .test = test_errors_key,
>
