Re: [kvm-unit-tests PATCH v8 09/12] s390x: Library resources for CSS tests

Pierre Morel
2020-06-10T15:10:15Z

On 2020-06-10 16:51, Thomas Huth wrote:
> On 09/06/2020 17.01, Pierre Morel wrote:
>>
>>
>> On 2020-06-09 09:09, Thomas Huth wrote:
>>> On 08/06/2020 10.12, Pierre Morel wrote:
>>>> Provide some definitions and library routines that can be used by
>>
>> ...snip...
>>
>>>> +static inline int ssch(unsigned long schid, struct orb *addr)
>>>> +{
>>>> +    register long long reg1 asm("1") = schid;
>>>> +    int cc;
>>>> +
>>>> +    asm volatile(
>>>> +        "    ssch    0(%2)\n"
>>>> +        "    ipm    %0\n"
>>>> +        "    srl    %0,28\n"
>>>> +        : "=d" (cc)
>>>> +        : "d" (reg1), "a" (addr), "m" (*addr)
>>>
>>> Hmm... What's the "m" (*addr) here good for? %3 is not used in the
>>> assembly code?
>>
>> addr is %2
>> "m" (*addr) means memory pointed by addr is read
>>
>>>
>>>> +        : "cc", "memory");
>>>
>>> Why "memory" ? Can this instruction also change the orb?
>>
>> The orb not but this instruction modifies memory as follow:
>> orb -> ccw -> data
>>
>> The CCW can be a READ or a WRITE instruction and the data my be anywhere
>> in memory (<2G)
>>
>> A compiler memory barrier is need to avoid write instructions started
>> before the SSCH instruction to occur after for a write
>> and memory read made after the instruction to be executed before for a
>> read.
>
> Ok, makes sense now, thanks!
>
>>>> +static inline int msch(unsigned long schid, struct schib *addr)
>>>> +{
>>>> +    register unsigned long reg1 asm ("1") = schid;
>>>> +    int cc;
>>>> +
>>>> +    asm volatile(
>>>> +        "    msch    0(%3)\n"
>>>> +        "    ipm    %0\n"
>>>> +        "    srl    %0,28"
>>>> +        : "=d" (cc), "=m" (*addr)
>>>> +        : "d" (reg1), "a" (addr)
>>>
>>> I'm not an expert with these IO instructions, but this looks wrong to me
>>> ... Is MSCH reading or writing the SCHIB data?
>>
>> MSCH is reading the SCHIB data in memory.
>
> So if it is reading, you don't need the "=m" (*addr) in the output
> list, do you? You should rather use "m" (*addr) in the input list instead?

Yes, absolutely, it should be the oposite of stsch(), not the same!

I change it to:

asm volatile(
" msch 0(%3)\n"
" ipm %0\n"
" srl %0,28"
: "=d" (cc)
: "d" (reg1), "m" (*addr), "a" (addr)
: "cc");

Thanks,

Pierre

>
> Thomas
>

--
Pierre Morel
IBM Lab Boeblingen