Re: [kvm-unit-tests PATCH v4 4/6] s390x: Add library functions for exiting from snippet

Nico Boehr
2024-10-21T08:21:55Z

Quoting Janosch Frank (2024-10-18 14:53:34)
> On 10/18/24 12:56 PM, Nico Boehr wrote:
> > Quoting Janosch Frank (2024-10-18 10:02:37)
> > [...]
> >>> +static inline uint64_t snippet_get_force_exit_value(struct vm *vm)
> >>> +{
> >>> + struct kvm_s390_sie_block *sblk = vm->sblk;
> >>> +
> >>> + assert(snippet_is_force_exit_value(vm));
> >>> +
> >>> + return vm->save_area.guest.grs[sblk_ip_as_diag(sblk).r_1];
> >>> +}
> >>
> >> The cpu address parameter for 9C is 16 bit.
> >> While we could make it 64 bit for snippets I don't see a reason to do
> >> so. The 16 bits are enough to indicate something to the host which can
> >> then go and fetch memory for more data.
> >
> > Mh, how exactly would you "fetch memory"? That requires knowledge on where
> > things are in guest memory which can be painful to figure out from the
> > host.
> >
> > I've found it useful to be able to pass a pointer from guest to host. Maybe
> > a diag500 is the better option? gr2 contains the cookie which is a 64-bit
> > value - see Linux' Documentation/virt/kvm/s390/s390-diag.rst.
> >
> > P.S. Did I miss the part in the docs where the 16-bit restriction of 9c is
> > documented or is it missing?
>
> For ASM snippets addresses 0x2000 to 0x4000 are a free area.
> For C snippets that area is the stack.
> The 16 bits should be good enough to point into that area.

Actually, it's currently just enough to point into the stack (snippet stack
is 64K)... also requires additional fiddling in the host to figure out the
complete address. Probably also in the guest, if you do it r15-relative
(can't think of a different solution right now).

> If the snippet requires a lot of memory then you can use constant
> addresses which are way over the snippet binary or just store a 64 bit
> address in a "free" lowcore location.

Would you prefer any of those over the diag500 solution?

> As you mentioned we also have diag500 which has the drawback of
> requiring to specify a couple more registers but that's not a huge
> hassle.

Since Nina contributed this nice wrapper function, going diag500 seems to
be the best thing to do.