Re: [kvm-unit-tests PATCH] lib: s390x: css: Name inline assembly arguments and clean them up

[Janosch Frank <frankja@linux.ibm.com>]

On 2/5/25 11:25 AM, Claudio Imbrenda wrote:
> On Tue,  4 Feb 2025 09:51:33 +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>
>> ---
>>
>> 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...