Re: [kvm-unit-tests PATCH v3 5/7] s390x: Add library functions for exiting from snippet

["Nicholas Piggin" <npiggin@gmail.com>]

On Fri Jun 21, 2024 at 3:26 AM AEST, Claudio Imbrenda wrote:
> On Thu, 20 Jun 2024 19:16:05 +0200
> Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:
>
> > On Thu, 2024-06-20 at 18:55 +0200, Claudio Imbrenda wrote:
> > > On Thu, 20 Jun 2024 16:16:58 +0200
> > > Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:
> > >   
> > > > It is useful to be able to force an exit to the host from the snippet,
> > > > as well as do so while returning a value.
> > > > Add this functionality, also add helper functions for the host to check
> > > > for an exit and get or check the value.
> > > > Use diag 0x44 and 0x9c for this.
> > > > Add a guest specific snippet header file and rename snippet.h to reflect
> > > > that it is host specific.
> > > > 
> > > > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>  
> > > 
> > > 
> > > [...]
> > > 
> > >   
> > > > +static inline void diag44(void)
> > > > +{
> > > > +	asm volatile("diag	0,0,0x44\n");
> > > > +}
> > > > +
> > > > +static inline void diag9c(uint64_t val)
> > > > +{
> > > > +	asm volatile("diag	%[val],0,0x9c\n"
> > > > +		:
> > > > +		: [val] "d"(val)
> > > > +	);
> > > > +}
> > > > +
> > > >  #endif  
> > > 
> > > [...]
> > >   
> > > > +static inline void force_exit(void)
> > > > +{
> > > > +	diag44();
> > > > +	mb(); /* allow host to modify guest memory */
> > > > +}
> > > > +
> > > > +static inline void force_exit_value(uint64_t val)
> > > > +{
> > > > +	diag9c(val);
> > > > +	mb(); /* allow host to modify guest memory */
> > > > +}  
> > > 
> > > why not adding "memory" to the clobbers of the inline asm? (not a big
> > > deal, I'm just curious if there is a specific reason for an explicit
> > > mb())  
> > 
> > Mostly a matter of taste I guess.
> > The diag functions are just convenience wrappers, doing nothing but
> > executing the diag.
> > force_exit is a protocol between the host and guest that uses the diags
> > and adds additional semantics on top.
> > In theory you could have other use cases where the diags are just a timeslice yield.
>
> fair enough

Sorry I missed these comments. I still had a question about the
barriers (e.g., one needed before the diag()?)

>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>
> > > 
> > > 
> > > [...]  
> >