* [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
* [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 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
* 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