On 05.12.18 20:27, David Hildenbrand wrote:
> On 05.12.18 16:39, Janosch Frank wrote:
>> We need to properly implement interrupt handling for SCLP, because on
>> z/VM and LPAR SCLP calls are not synchronous!
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
[]
>> -
>> static void sclp_set_write_mask(void)
>> {
>> WriteEventMask *sccb = (void *)_sccb;
>>
>> + while (sclp_busy)
>> + /* Wait for SCLP request to complete */;
>
> should we use barrier(); here to make this clearer?
I could factor that out in sclp_wait_busy(), to make it one place to
change. I thought about using spinlocks, but whatever we do, we have to
be careful to not print when holding the lock.
Maybe we need a sclp spinlock/busy bust error path for exit with > 0,
like we do in the kernel on a panic.
>
>> + sclp_busy = true;
>> sccb->h.length = sizeof(WriteEventMask);
>> sccb->mask_length = sizeof(unsigned int);
>> sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
>> @@ -57,6 +40,9 @@ void sclp_print(const char *str)
>> int len = strlen(str);
>> WriteEventData *sccb = (void *)_sccb;
>>
>> + while (sclp_busy)
>> + /* Wait for SCLP request to complete */;
>
> dito
>
> (I guess do not we need barriers between setting sclp_busy and testing
> it, because we're using volatile and we don't have multi-threading
> messing with the value)
>
>> + sclp_busy = true;
>> sccb->h.length = sizeof(WriteEventData) + len;
>> sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
>> sccb->ebh.length = sizeof(EventBufferHeader) + len;
>> @@ -65,4 +51,7 @@ void sclp_print(const char *str)
>> memcpy(sccb->data, str, len);
>>
>> sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
>> + while (sclp_busy)
>> + /* Wait for SCLP request to complete */;
>
> dito
>
>> +
>> }
>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>> index 1d4a010..cd0e5e5 100644
>> --- a/lib/s390x/sclp.c
>> +++ b/lib/s390x/sclp.c
>> @@ -23,6 +23,9 @@ static uint64_t storage_increment_size;
>> static uint64_t max_ram_size;
>> static uint64_t ram_size;
>>
>> +char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
>> +volatile bool sclp_busy = false;
>
> false should be the default value already.
Yes, that's a leftover from the bss clearing problem, where it sometimes
was > 0...
>
>> +
>> static void mem_init(phys_addr_t mem_end)
>> {
>> phys_addr_t freemem_start = (phys_addr_t)&stacktop;
>> @@ -30,6 +33,42 @@ static void mem_init(phys_addr_t mem_end)
>> phys_alloc_init(freemem_start, mem_end - freemem_start);
>> }
>>
>> +static void sclp_setup_int(void)
>> +{
>> + uint64_t mask;
>> +
>> + ctl_set_bit(0, 9);
>> +
>> + mask = extract_psw_mask();
>> + mask |= PSW_MASK_EXT;
>> + load_psw_mask(mask);
>> +}
>> +
>> +void sclp_handle_ext(void)
>> +{
>
> Error out if !sclp_busy but we get an interrupt?
Yes, but then I need to reset sclp_busy in the progress and we need to
hope that we get sclp output.
>
>> + ctl_clear_bit(0, 9);
>> + sclp_busy = false;
>> +}
>> +
>> +/* Perform service call. Return 0 on success, non-zero otherwise. */
>> +int sclp_service_call(unsigned int command, void *sccb)
>> +{
>> + int cc;
>> +
>> + sclp_setup_int();
>> + asm volatile(
>> + " .insn rre,0xb2200000,%1,%2\n" /* servc %1,%2 */
>> + " ipm %0\n"
>> + " srl %0,28"
>> + : "=&d" (cc) : "d" (command), "a" (__pa(sccb))
>> + : "cc", "memory");
>> + if (cc == 3)
>> + return -1;
>> + if (cc == 2)
>> + return -1;
>> + return 0;
>> +}
>
> Guess moving that (including _sccb) could also be a separate patch ;)
Will do
>
>> +
>> void sclp_memory_setup(void)
>> {
>> ReadInfo *ri = (void *)_sccb;
>> @@ -37,7 +76,10 @@ void sclp_memory_setup(void)
>> int cc;
>>
>> ri->h.length = SCCB_SIZE;
>> + sclp_busy = true;
>> sclp_service_call(SCLP_CMDW_READ_SCP_INFO_FORCED, ri);
>> + while (sclp_busy)
>> + /* Wait for SCLP request to complete */;
>
> Should we factor that out into something like
As written above, I'm all for the second one, but I dislike the start
one liner.
>
> sclp_start_request()
> {
> sclp_busy = true;
> }
>
> sclp_wait_response()
> {
> while (sclp_busy)
> barrier();
> }
>
>>
>> /* calculate the storage increment size */
>> rnsize = ri->rnsize;
>> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
>> index e008932..a91aad1 100644
>> --- a/lib/s390x/sclp.h
>> +++ b/lib/s390x/sclp.h
>> @@ -207,9 +207,11 @@ typedef struct ReadEventData {
>> uint32_t mask;
>> } __attribute__((packed)) ReadEventData;
>>
>> +extern char _sccb[];
>> +volatile bool sclp_busy;
>> +void sclp_handle_ext(void);
>> void sclp_console_setup(void);
>> void sclp_print(const char *str);
>> -extern char _sccb[];
>> int sclp_service_call(unsigned int command, void *sccb);
>> void sclp_memory_setup(void);
>>
>>
>
>
Attachment:
signature.asc
Description: OpenPGP digital signature
