Quoting Thomas Huth (2022-08-30 14:52:14)
> > diff --git a/s390x/exittime.c b/s390x/exittime.c
> > new file mode 100644
[...]
> > +static void test_nop(long ignore)
> > +{
> > + asm volatile("nop" : : : "memory");
> > +}
>
> What's the purpose of testing "nop"? ... it does not trap to the
> hypervisor... ? Is it just for reference? Then a comment might be helpful here.
Yes, the idea is to have some reference for a non-trapping instruction. Added a comment.
[...]
> > +static void test_diag44(long ignore)
> > +{
> > + asm volatile("diag 0,0,0x44" : : : );
>
> Drop the " : : : " please.
OK
> > +static void test_stnsm(long ignore)
> > +{
> > + int out;
> > +
> > + asm volatile(
> > + "stnsm %[out],0xff"
> > + : [out] "=Q" (out)
> > + :
> > + : "memory"
>
> I don't think you need the "memory" clobber here, do you?
Uhm, yes, right. Good thing we have reviews. :-)
[...]
> > +static void test_stpx(long ignore)
> > +{
> > + unsigned int prefix;
> > +
> > + asm volatile(
> > + "stpx %[prefix]"
> > + : [prefix] "=S" (prefix)
>
> STPX seems to have only a short displacement, so it should have "=Q" instead
> of "=S" ?
Yes, right, fixed. Also the memory clobber is unneeded. I think I will do through all the clobbers and constraint once more, seems like I messed up a bit there.
[...]
> > +struct test {
> > + char name[100];
>
> "const char *name" instead of using an array here, please. Otherwise this
> wastes a lot of space in the binary.
Makes sense, thanks.
