* [kvm-unit-tests RFC 0/2] lib: s390x: Inline asm cleanup @ 2024-02-01 14:23 Janosch Frank 2024-02-01 14:23 ` [kvm-unit-tests RFC 1/2] lib: s390x: sigp: Name inline assembly arguments Janosch Frank 2024-02-01 14:23 ` [kvm-unit-tests RFC 2/2] lib: s390x: css: Name inline assembly arguments and clean them up Janosch Frank 0 siblings, 2 replies; 6+ messages in thread From: Janosch Frank @ 2024-02-01 14:23 UTC (permalink / raw) To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb The library should not have unnamed inline assembly arguments for the sake of readability. This series names the arguments and removes unused ones. Janosch Frank (2): lib: s390x: sigp: Name inline assembly arguments lib: s390x: css: Name inline assembly arguments and clean them up lib/s390x/asm/sigp.h | 10 +++--- lib/s390x/css.h | 76 ++++++++++++++++++++++---------------------- 2 files changed, 43 insertions(+), 43 deletions(-) -- 2.40.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-unit-tests RFC 1/2] lib: s390x: sigp: Name inline assembly arguments 2024-02-01 14:23 [kvm-unit-tests RFC 0/2] lib: s390x: Inline asm cleanup Janosch Frank @ 2024-02-01 14:23 ` Janosch Frank 2024-02-01 17:52 ` Claudio Imbrenda 2024-02-01 14:23 ` [kvm-unit-tests RFC 2/2] lib: s390x: css: Name inline assembly arguments and clean them up Janosch Frank 1 sibling, 1 reply; 6+ messages in thread From: Janosch Frank @ 2024-02-01 14:23 UTC (permalink / raw) To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb Less need to count the operands makes the code easier to read. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/asm/sigp.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/s390x/asm/sigp.h b/lib/s390x/asm/sigp.h index 4eae95d0..c9af2c49 100644 --- a/lib/s390x/asm/sigp.h +++ b/lib/s390x/asm/sigp.h @@ -54,11 +54,11 @@ static inline int sigp(uint16_t addr, uint8_t order, unsigned long parm, asm volatile( " tmll %[bogus_cc],3\n" - " sigp %1,%2,0(%3)\n" - " ipm %0\n" - " srl %0,28\n" - : "=d" (cc), "+d" (reg1) - : "d" (addr), "a" (order), [bogus_cc] "d" (bogus_cc) + " sigp %[reg1],%[addr],0(%[order])\n" + " ipm %[cc]\n" + " srl %[cc],28\n" + : [cc] "=d" (cc), [reg1] "+d" (reg1) + : [addr] "d" (addr), [order] "a" (order), [bogus_cc] "d" (bogus_cc) : "cc"); if (status) *status = reg1; -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests RFC 1/2] lib: s390x: sigp: Name inline assembly arguments 2024-02-01 14:23 ` [kvm-unit-tests RFC 1/2] lib: s390x: sigp: Name inline assembly arguments Janosch Frank @ 2024-02-01 17:52 ` Claudio Imbrenda 0 siblings, 0 replies; 6+ messages in thread From: Claudio Imbrenda @ 2024-02-01 17:52 UTC (permalink / raw) To: Janosch Frank; +Cc: kvm, linux-s390, thuth, david, nsg, nrb On Thu, 1 Feb 2024 14:23:55 +0000 Janosch Frank <frankja@linux.ibm.com> wrote: > Less need to count the operands makes the code easier to read. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > lib/s390x/asm/sigp.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/lib/s390x/asm/sigp.h b/lib/s390x/asm/sigp.h > index 4eae95d0..c9af2c49 100644 > --- a/lib/s390x/asm/sigp.h > +++ b/lib/s390x/asm/sigp.h > @@ -54,11 +54,11 @@ static inline int sigp(uint16_t addr, uint8_t order, unsigned long parm, > > asm volatile( > " tmll %[bogus_cc],3\n" > - " sigp %1,%2,0(%3)\n" > - " ipm %0\n" > - " srl %0,28\n" > - : "=d" (cc), "+d" (reg1) > - : "d" (addr), "a" (order), [bogus_cc] "d" (bogus_cc) > + " sigp %[reg1],%[addr],0(%[order])\n" > + " ipm %[cc]\n" > + " srl %[cc],28\n" > + : [cc] "=d" (cc), [reg1] "+d" (reg1) > + : [addr] "d" (addr), [order] "a" (order), [bogus_cc] "d" (bogus_cc) > : "cc"); > if (status) > *status = reg1; ^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-unit-tests RFC 2/2] lib: s390x: css: Name inline assembly arguments and clean them up 2024-02-01 14:23 [kvm-unit-tests RFC 0/2] lib: s390x: Inline asm cleanup Janosch Frank 2024-02-01 14:23 ` [kvm-unit-tests RFC 1/2] lib: s390x: sigp: Name inline assembly arguments Janosch Frank @ 2024-02-01 14:23 ` Janosch Frank 2024-04-22 7:43 ` Nico Boehr 1 sibling, 1 reply; 6+ messages in thread From: Janosch Frank @ 2024-02-01 14:23 UTC (permalink / raw) To: kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg, nrb Less need to count the operands makes the code easier to read. For ssch and msch the second addr argument which was unused was removed. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/css.h | 76 ++++++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/lib/s390x/css.h b/lib/s390x/css.h index 504b3f14..e4311124 100644 --- a/lib/s390x/css.h +++ b/lib/s390x/css.h @@ -135,11 +135,11 @@ static inline int ssch(unsigned long schid, struct orb *addr) int cc; asm volatile( - " ssch 0(%2)\n" - " ipm %0\n" - " srl %0,28\n" - : "=d" (cc) - : "d" (reg1), "a" (addr), "m" (*addr) + " ssch 0(%[addr])\n" + " ipm %[cc]\n" + " srl %[cc],28\n" + : [cc] "=d" (cc) + : "d" (reg1), [addr] "a" (addr) : "cc", "memory"); return cc; } @@ -152,11 +152,11 @@ static inline int stsch(unsigned long schid, struct schib *addr) asm volatile( " tmll %[bogus_cc],3\n" - " stsch 0(%3)\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc), "=m" (*addr) - : "d" (reg1), "a" (addr), [bogus_cc] "d" (bogus_cc) + " stsch 0(%[addr])\n" + " ipm %[cc]\n" + " srl %[cc],28" + : [cc] "=d" (cc), "=m" (*addr) + : "d" (reg1), [addr] "a" (addr), [bogus_cc] "d" (bogus_cc) : "cc"); return cc; } @@ -167,11 +167,11 @@ static inline int msch(unsigned long schid, struct schib *addr) int cc; asm volatile( - " msch 0(%3)\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc) - : "d" (reg1), "m" (*addr), "a" (addr) + " msch 0(%[addr])\n" + " ipm %[cc]\n" + " srl %[cc],28" + : [cc] "=d" (cc) + : "d" (reg1), [addr] "a" (addr) : "cc"); return cc; } @@ -184,11 +184,11 @@ static inline int tsch(unsigned long schid, struct irb *addr) asm volatile( " tmll %[bogus_cc],3\n" - " tsch 0(%3)\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc), "=m" (*addr) - : "d" (reg1), "a" (addr), [bogus_cc] "d" (bogus_cc) + " tsch 0(%[addr])\n" + " ipm %[cc]\n" + " srl %[cc],28" + : [cc] "=d" (cc), "=m" (*addr) + : "d" (reg1), [addr] "a" (addr), [bogus_cc] "d" (bogus_cc) : "cc"); return cc; } @@ -200,9 +200,9 @@ static inline int hsch(unsigned long schid) asm volatile( " hsch\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc) + " ipm %[cc]\n" + " srl %[cc],28" + : [cc] "=d" (cc) : "d" (reg1) : "cc"); return cc; @@ -215,9 +215,9 @@ static inline int xsch(unsigned long schid) asm volatile( " xsch\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc) + " ipm %[cc]\n" + " srl %cc,28" + : [cc] "=d" (cc) : "d" (reg1) : "cc"); return cc; @@ -230,9 +230,9 @@ static inline int csch(unsigned long schid) asm volatile( " csch\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc) + " ipm %[cc]\n" + " srl %[cc],28" + : [cc] "=d" (cc) : "d" (reg1) : "cc"); return cc; @@ -245,9 +245,9 @@ static inline int rsch(unsigned long schid) asm volatile( " rsch\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc) + " ipm %[cc]\n" + " srl %[cc],28" + : [cc] "=d" (cc) : "d" (reg1) : "cc"); return cc; @@ -262,9 +262,9 @@ static inline int rchp(unsigned long chpid) asm volatile( " tmll %[bogus_cc],3\n" " rchp\n" - " ipm %0\n" - " srl %0,28" - : "=d" (cc) + " ipm %[cc]\n" + " srl %[cc],28" + : [cc] "=d" (cc) : "d" (reg1), [bogus_cc] "d" (bogus_cc) : "cc"); return cc; @@ -369,9 +369,9 @@ static inline int _chsc(void *p) int cc; asm volatile(" .insn rre,0xb25f0000,%2,0\n" - " ipm %0\n" - " srl %0,28\n" - : "=d" (cc), "=m" (p) + " ipm %[cc]\n" + " srl %[cc],28\n" + : [cc] "=d" (cc), "=m" (p) : "d" (p), "m" (p) : "cc"); -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] lib: s390x: css: Name inline assembly arguments and clean them up 2024-02-01 14:23 ` [kvm-unit-tests RFC 2/2] lib: s390x: css: Name inline assembly arguments and clean them up Janosch Frank @ 2024-04-22 7:43 ` Nico Boehr 2024-04-22 10:57 ` Heiko Carstens 0 siblings, 1 reply; 6+ messages in thread From: Nico Boehr @ 2024-04-22 7:43 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, thuth, david, nsg Quoting Janosch Frank (2024-02-01 15:23:56) [...] > diff --git a/lib/s390x/css.h b/lib/s390x/css.h > index 504b3f14..e4311124 100644 > --- a/lib/s390x/css.h > +++ b/lib/s390x/css.h [...] > @@ -167,11 +167,11 @@ static inline int msch(unsigned long schid, struct schib *addr) > int cc; > > asm volatile( > - " msch 0(%3)\n" > - " ipm %0\n" > - " srl %0,28" > - : "=d" (cc) > - : "d" (reg1), "m" (*addr), "a" (addr) > + " msch 0(%[addr])\n" > + " ipm %[cc]\n" > + " srl %[cc],28" > + : [cc] "=d" (cc) > + : "d" (reg1), [addr] "a" (addr) I think there was a reason why the "m"(*addr) was here. Either add it back or add a memory clobber. I will only take the first patch of this series for now. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests RFC 2/2] lib: s390x: css: Name inline assembly arguments and clean them up 2024-04-22 7:43 ` Nico Boehr @ 2024-04-22 10:57 ` Heiko Carstens 0 siblings, 0 replies; 6+ messages in thread From: Heiko Carstens @ 2024-04-22 10:57 UTC (permalink / raw) To: Nico Boehr; +Cc: Janosch Frank, kvm, linux-s390, imbrenda, thuth, david, nsg On Mon, Apr 22, 2024 at 09:43:44AM +0200, Nico Boehr wrote: > Quoting Janosch Frank (2024-02-01 15:23:56) > [...] > > diff --git a/lib/s390x/css.h b/lib/s390x/css.h > > index 504b3f14..e4311124 100644 > > --- a/lib/s390x/css.h > > +++ b/lib/s390x/css.h > [...] > > @@ -167,11 +167,11 @@ static inline int msch(unsigned long schid, struct schib *addr) > > int cc; > > > > asm volatile( > > - " msch 0(%3)\n" > > - " ipm %0\n" > > - " srl %0,28" > > - : "=d" (cc) > > - : "d" (reg1), "m" (*addr), "a" (addr) > > + " msch 0(%[addr])\n" > > + " ipm %[cc]\n" > > + " srl %[cc],28" > > + : [cc] "=d" (cc) > > + : "d" (reg1), [addr] "a" (addr) > > I think there was a reason why the "m"(*addr) was here. Either add it back > or add a memory clobber. It is there to tell the compiler that the memory contents at *addr are used as input. Without that, and only the "a" contraint, the compiler is free to discard any potential previous writes to *addr. The best solution here would be to use the Q constraint (memory reference with short displacement and without index register) for the second operand address of msch. Or simply copy the current implementation from the kernel (drivers/s390/cio/ioasm.c). ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-22 10:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-01 14:23 [kvm-unit-tests RFC 0/2] lib: s390x: Inline asm cleanup Janosch Frank 2024-02-01 14:23 ` [kvm-unit-tests RFC 1/2] lib: s390x: sigp: Name inline assembly arguments Janosch Frank 2024-02-01 17:52 ` Claudio Imbrenda 2024-02-01 14:23 ` [kvm-unit-tests RFC 2/2] lib: s390x: css: Name inline assembly arguments and clean them up Janosch Frank 2024-04-22 7:43 ` Nico Boehr 2024-04-22 10:57 ` Heiko Carstens
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox