On Mon Dec 9, 2024 at 7:14 PM CET, Claudio Imbrenda wrote:
> On Mon,  9 Dec 2024 12:07:13 +0100
> Christoph Schlameuss <schlame...@linux.ibm.com> wrote:
>
> > Add some superficial selftests for the floating interrupt controller
> > when using ucontrol VMs. These tests are intended to cover very basic
> > calls only.
> > 
> > Some of the calls may trigger null pointer dereferences on kernels not
> > containing the fixes in this patch series.
> > 
> > Signed-off-by: Christoph Schlameuss <schlame...@linux.ibm.com>
> > ---
> >  .../selftests/kvm/s390x/ucontrol_test.c       | 150 ++++++++++++++++++
> >  1 file changed, 150 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c 
> > b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > index 0c112319dab1..972fac1023b5 100644
> > --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c
> > @@ -635,4 +635,154 @@ TEST_F(uc_kvm, uc_skey)
> >     uc_assert_diag44(self);
> >  }
> >  
> > +static char uc_flic_b[PAGE_SIZE];
> > +static struct kvm_s390_io_adapter uc_flic_ioa = { .id = 0 };
> > +static struct kvm_s390_io_adapter_req uc_flic_ioam = { .id = 0 };
> > +static struct kvm_s390_ais_req uc_flic_asim = { .isc = 0 };
> > +static struct kvm_s390_ais_all uc_flic_asima = { .simm = 0 };
> > +static struct uc_flic_attr_test {
> > +   char *name;
> > +   struct kvm_device_attr a;
> > +   int hasrc;
> > +   u64 getrc;
> > +   int geterrno;
> > +   u64 setrc;
>
> I wonder if you really need getrc and setrc? (see below)
>
> > +   int seterrno;
> > +} uc_flic_attr_tests[] = {
> > +   {
> > +           .name = "KVM_DEV_FLIC_GET_ALL_IRQS",
> > +           .setrc = 1, .seterrno = EINVAL,
>
> please put them on separate lines ^ (if you end up keeping both)
>
> > +           .a = {
> > +                   .group = KVM_DEV_FLIC_GET_ALL_IRQS,
> > +                   .addr = (u64)&uc_flic_b,
> > +                   .attr = PAGE_SIZE,
> > +           },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_ENQUEUE",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .a = { .group = KVM_DEV_FLIC_ENQUEUE, },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_CLEAR_IRQS",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .a = { .group = KVM_DEV_FLIC_CLEAR_IRQS, },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_ADAPTER_REGISTER",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .a = {
> > +                   .group = KVM_DEV_FLIC_ADAPTER_REGISTER,
> > +                   .addr = (u64)&uc_flic_ioa,
> > +           },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_ADAPTER_MODIFY",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .setrc = 1, .seterrno = EINVAL,
> > +           .a = {
> > +                   .group = KVM_DEV_FLIC_ADAPTER_MODIFY,
> > +                   .addr = (u64)&uc_flic_ioam,
> > +                   .attr = sizeof(uc_flic_ioam),
> > +           },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_CLEAR_IO_IRQ",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .setrc = 1, .seterrno = EINVAL,
> > +           .a = {
> > +                   .group = KVM_DEV_FLIC_CLEAR_IO_IRQ,
> > +                   .attr = 32,
> > +           },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_AISM",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .setrc = 1, .seterrno = ENOTSUP,
> > +           .a = {
> > +                   .group = KVM_DEV_FLIC_AISM,
> > +                   .addr = (u64)&uc_flic_asim,
> > +           },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_AIRQ_INJECT",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .a = { .group = KVM_DEV_FLIC_AIRQ_INJECT, },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_AISM_ALL",
> > +           .getrc = 1, .geterrno = ENOTSUP,
> > +           .setrc = 1, .seterrno = ENOTSUP,
> > +           .a = {
> > +                   .group = KVM_DEV_FLIC_AISM_ALL,
> > +                   .addr = (u64)&uc_flic_asima,
> > +                   .attr = sizeof(uc_flic_asima),
> > +           },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_APF_ENABLE",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .setrc = 1, .seterrno = EINVAL,
> > +           .a = { .group = KVM_DEV_FLIC_APF_ENABLE, },
> > +   },
> > +   {
> > +           .name = "KVM_DEV_FLIC_APF_DISABLE_WAIT",
> > +           .getrc = 1, .geterrno = EINVAL,
> > +           .setrc = 1, .seterrno = EINVAL,
> > +           .a = { .group = KVM_DEV_FLIC_APF_DISABLE_WAIT, },
> > +   },
> > +};
> > +
> > +TEST_F(uc_kvm, uc_flic_attrs)
> > +{
> > +   struct kvm_create_device cd = { .type = KVM_DEV_TYPE_FLIC };
> > +   struct kvm_device_attr attr;
> > +   u64 value;
> > +   int rc, i;
> > +
> > +   rc = ioctl(self->vm_fd, KVM_CREATE_DEVICE, &cd);
> > +   ASSERT_EQ(0, rc) TH_LOG("create device failed with err %s (%i)",
> > +                           strerror(errno), errno);
> > +
> > +   for (i = 0; i < ARRAY_SIZE(uc_flic_attr_tests); i++) {
> > +           TH_LOG("test %s", uc_flic_attr_tests[i].name);
> > +           attr = (struct kvm_device_attr) {
> > +                   .group = uc_flic_attr_tests[i].a.group,
> > +                   .attr = uc_flic_attr_tests[i].a.attr,
> > +                   .addr = uc_flic_attr_tests[i].a.addr,
> > +           };
> > +           if (attr.addr == 0)
> > +                   attr.addr = (u64)&value;
> > +
> > +           rc = ioctl(cd.fd, KVM_HAS_DEVICE_ATTR, &attr);
> > +           EXPECT_EQ(uc_flic_attr_tests[i].hasrc, !!rc)
> > +                   TH_LOG("expected dev attr missing %s",
> > +                          uc_flic_attr_tests[i].name);
> > +
> > +           rc = ioctl(cd.fd, KVM_GET_DEVICE_ATTR, &attr);
> > +           EXPECT_EQ(uc_flic_attr_tests[i].getrc, !!rc)
>
> maybe you could just do:
>
>       EXPECT_EQ(!!uc_flic_attr_tests[i].geterrno, !!rc)
>
> (unless I am missing something)
>
> this is not super important, though
>

Yes, that should work. I will do that. Thanks!

> > +                   TH_LOG("get dev attr rc not expected on %s %s (%i)",
> > +                          uc_flic_attr_tests[i].name,
> > +                          strerror(errno), errno);
> > +           if (uc_flic_attr_tests[i].geterrno)
> > +                   EXPECT_EQ(uc_flic_attr_tests[i].geterrno, errno)
> > +                           TH_LOG("get dev attr errno not expected on %s 
> > %s (%i)",
> > +                                  uc_flic_attr_tests[i].name,
> > +                                  strerror(errno), errno);
> > +
> > +           rc = ioctl(cd.fd, KVM_SET_DEVICE_ATTR, &attr);
> > +           EXPECT_EQ(uc_flic_attr_tests[i].setrc, !!rc)
> > +                   TH_LOG("set sev attr rc not expected on %s %s (%i)",
> > +                          uc_flic_attr_tests[i].name,
> > +                          strerror(errno), errno);
> > +           if (uc_flic_attr_tests[i].seterrno)
> > +                   EXPECT_EQ(uc_flic_attr_tests[i].seterrno, errno)
> > +                           TH_LOG("set dev attr errno not expected on %s 
> > %s (%i)",
> > +                                  uc_flic_attr_tests[i].name,
> > +                                  strerror(errno), errno);
> > +   }
> > +
> > +   close(cd.fd);
> > +}
> > +
> >  TEST_HARNESS_MAIN


Reply via email to