On Thu, 6 Feb 2025 11:36:11 +0100
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
> On 2/5/25 11:25 AM, Claudio Imbrenda wrote:
> > On Tue, 4 Feb 2025 09:51:33 +0000
> > Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
> >
> >> Less need to count the operands makes the code easier to read.
> >>
> >> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> >> ---
> >>
> >> This one has been gathering dust for a while.
> >> rfc->v1: Moved to Q constraint (thanks Heiko)
> >>
> >> ---
> >
> > [...]
> >
> >> 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)
> >
> > this bit (which you did not touch) is actually the most confusing to me.
> > what's the point of separately specifying both "d" and "m" constraints
> > for (p) ? (and it also has a "=m" in the output clobberlist)
>
> I consulted the kernel code as well as Heiko and the architecture.
>
> CHSC is one of those request/response do everything instructions and is
> similar to sclp. A header is read from memory and a response is written
> below the header. The addressed memory needs to be page aligned and can
> be up to a page in size.
>
> Which means:
> - We need the address in R1
> - CPU reads from the memory area designated by R1
> - CPU writes to the memory area designated by R1
>
> My guess is that nobody bothered defining all of the structs and that's
> how we ended up here. If you look at the kernel assembly you'll notice a
> page size typedef for the +m clobber of the memory area pointer.
>
> Heiko suggested to drop the two "m" clobbers and just add a generic
> memory clobber. If we even want to touch this at all...
tbh, yes, I would like to have it as simple as possible.
this is not super performance-sensitive, IMHO it's more important that
the code is readable and understandable.
