| Message ID | 20210622135517.234801-4-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Test compiling with Clang in the Travis-CI | expand |
On Tue, 22 Jun 2021 15:55:16 +0200 Thomas Huth <thuth@redhat.com> wrote: > According to the Principles of Operation, the epsw instruction > does not touch the second register if it is r0. With GCC we were > lucky so far that it never tried to use r0 here, but when compiling > the kvm-unit-tests with Clang, this indeed happens and leads to > very weird crashes. Thus let's make sure to never use r0 for the > second operand of the epsw instruction. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> maybe also mention in the patch description why you changed + to = > --- > lib/s390x/asm/arch_def.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index 3aa5da9..15cf7d4 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -265,7 +265,7 @@ static inline uint64_t extract_psw_mask(void) > > asm volatile( > " epsw %0,%1\n" > - : "+r" (mask_upper), "+r" (mask_lower) : : ); > + : "=r" (mask_upper), "=a" (mask_lower)); > > return (uint64_t) mask_upper << 32 | mask_lower; > }
On 22/06/2021 16.12, Claudio Imbrenda wrote: > On Tue, 22 Jun 2021 15:55:16 +0200 > Thomas Huth <thuth@redhat.com> wrote: > >> According to the Principles of Operation, the epsw instruction >> does not touch the second register if it is r0. With GCC we were >> lucky so far that it never tried to use r0 here, but when compiling >> the kvm-unit-tests with Clang, this indeed happens and leads to >> very weird crashes. Thus let's make sure to never use r0 for the >> second operand of the epsw instruction. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> > > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > maybe also mention in the patch description why you changed + to = Yes, that makes sense. I'll add something like: While we're at it, also change the constraint modifier from "+" to "=" since these are only output parameters, not input. Thomas
On 6/22/21 3:55 PM, Thomas Huth wrote: > According to the Principles of Operation, the epsw instruction > does not touch the second register if it is r0. With GCC we were > lucky so far that it never tried to use r0 here, but when compiling > the kvm-unit-tests with Clang, this indeed happens and leads to > very weird crashes. Thus let's make sure to never use r0 for the > second operand of the epsw instruction. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > lib/s390x/asm/arch_def.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index 3aa5da9..15cf7d4 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -265,7 +265,7 @@ static inline uint64_t extract_psw_mask(void) > > asm volatile( > " epsw %0,%1\n" > - : "+r" (mask_upper), "+r" (mask_lower) : : ); > + : "=r" (mask_upper), "=a" (mask_lower)); > > return (uint64_t) mask_upper << 32 | mask_lower; > } > With Claudio's nits fixed: Acked-by: Janosch Frank <frankja@linux.ibm.com>
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h index 3aa5da9..15cf7d4 100644 --- a/lib/s390x/asm/arch_def.h +++ b/lib/s390x/asm/arch_def.h @@ -265,7 +265,7 @@ static inline uint64_t extract_psw_mask(void) asm volatile( " epsw %0,%1\n" - : "+r" (mask_upper), "+r" (mask_lower) : : ); + : "=r" (mask_upper), "=a" (mask_lower)); return (uint64_t) mask_upper << 32 | mask_lower; }
According to the Principles of Operation, the epsw instruction does not touch the second register if it is r0. With GCC we were lucky so far that it never tried to use r0 here, but when compiling the kvm-unit-tests with Clang, this indeed happens and leads to very weird crashes. Thus let's make sure to never use r0 for the second operand of the epsw instruction. Signed-off-by: Thomas Huth <thuth@redhat.com> --- lib/s390x/asm/arch_def.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)