On Tue, 2024-08-27 at 16:08 +0200, Nico Boehr wrote:
> Quoting Nina Schoetterl-Glausch (2024-06-20 16:17:00)
> [...]
> > diff --git a/lib/s390x/asm/facility.h b/lib/s390x/asm/facility.h
> > index a66fe56a..2bad05c5 100644
> > --- a/lib/s390x/asm/facility.h
> > +++ b/lib/s390x/asm/facility.h
> > @@ -27,12 +27,20 @@ static inline void stfl(void)
> > asm volatile(" stfl 0(0)\n" : : : "memory");
> > }
> >
> > -static inline void stfle(uint64_t *fac, unsigned int nb_doublewords)
> > +static inline unsigned int stfle(uint64_t *fac, unsigned int nb_doublewords)
>
> Why unsigned int?
The return value is 1-256, the size of the type is a bit arbitrary I suppose.
>
> [...]
> > diff --git a/s390x/snippets/c/stfle.c b/s390x/snippets/c/stfle.c
> > new file mode 100644
> > index 00000000..eb024a6a
> > --- /dev/null
> > +++ b/s390x/snippets/c/stfle.c
> [...]
> > +int main(void)
> > +{
> > + const unsigned int max_fac_len = 8;
> > + uint64_t res[max_fac_len + 1];
> > +
> > + res[0] = max_fac_len - 1;
> > + asm volatile ( "lg 0,%[len]\n"
> > + " stfle %[fac]\n"
> > + " stg 0,%[len]\n"
> > + : [fac] "=QS"(*(uint64_t(*)[max_fac_len])&res[1]),
>
> Out of curiosity:
>
> Q = Memory reference without index register and with short displacement
> S = Memory reference without index register but with long displacement
>
> Which one is it?
Ups, just short displacement actually.
>
> And: is long displacement even appropriate here?
>
> The cast also is hard to understand. Since this is not super high
> performance code, do we just want to clobber memory so this gets a bit
> easier to understand?
>
> > + [len] "+RT"(res[0])
>
> Same question about RT as above.
Long, but providing a short displacement should be fine too.
Not sure if there is any benefit to letting the compiler choose.
>
> [...]
> > diff --git a/s390x/stfle-sie.c b/s390x/stfle-sie.c
> > new file mode 100644
> > index 00000000..a3e7f1c9
> > --- /dev/null
> > +++ b/s390x/stfle-sie.c
> [...]
> > +static struct guest_stfle_res run_guest(void)
> > +{
> > + struct guest_stfle_res res;
> > + uint64_t guest_stfle_addr;
> > +
> > + sie(&vm);
> > + assert(snippet_is_force_exit_value(&vm));
> > + guest_stfle_addr = snippet_get_force_exit_value(&vm);
> > + res.mem = &vm.guest_mem[guest_stfle_addr];
> > + memcpy(&res.reg, res.mem, sizeof(res.reg));
> > + res.len = (res.reg & 0xff) + 1;
>
> If I'm not mistaken, you subtracted 1 in the guest. Here you add it again.
> Is there a particular reason why?
No, it's the direct result of STFLE on register 0.
