Re: [kvm-unit-tests PATCH v6 3/4] s390x: lib: add SPX and STPX instruction wrapper

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Thu, 9 Jan 2020 17:43:55 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 09/01/2020 17.16, Claudio Imbrenda wrote:
> > Add a wrapper for the SET PREFIX and STORE PREFIX instructions, and
> > use it instead of using inline assembly everywhere.
> > 
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> >  lib/s390x/asm/arch_def.h | 10 ++++++++++
> >  s390x/intercept.c        | 33 +++++++++++++--------------------
> >  2 files changed, 23 insertions(+), 20 deletions(-)
> > 
> > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> > index 1a5e3c6..465fe0f 100644
> > --- a/lib/s390x/asm/arch_def.h
> > +++ b/lib/s390x/asm/arch_def.h
> > @@ -284,4 +284,14 @@ static inline int servc(uint32_t command,
> > unsigned long sccb) return cc;
> >  }
> >  
> > +static inline void spx(uint32_t *new_prefix)  
> 
> Looking at this a second time ... why is new_prefix a pointer? A
> normal value should be sufficient here, shouldn't it?

no. if you look at the code in the same patch, intercept.c at some
points needs to pass "wrong" pointers to spx and stpx in order to test
them, so this needs to be a pointer

the instructions themselves expect pointers (base register + offset)

> > +{
> > +	asm volatile("spx %0" : : "Q" (*new_prefix) : "memory");
> > +}
> > +
> > +static inline void stpx(uint32_t *current_prefix)
> > +{
> > +	asm volatile("stpx %0" : "=Q" (*current_prefix));
> > +}
> > +  
> 
>  Thomas
>