Re: [kvm-unit-tests PATCH 1/1] s390x: sclp: consider monoprocessor on read_info error

Pierre Morel
2023-04-25T10:54:06Z

On 4/25/23 10:26, Claudio Imbrenda wrote:
> On Mon, 24 Apr 2023 19:42:18 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> When we can not read SCP information we can not abort during
>> sclp_get_cpu_num() because this function is called during exit
>> and calling it will lead to an infnite loop.
>>
>> The loop is:
>> abort() -> exit() -> smp_teardown() -> smp_query_num_cpus() ->
>> sclp_get_cpu_num() -> assert() -> abort()
>>
>> Since smp_setup() is done after sclp_read_info() inside setup() this
>> loop happens when only the start processor is running.
>> Let sclp_get_cpu_num() return 1 in this case.
> looks good to me, but please add a comment to explain that this is only
> supposed to happen in exceptional circumstances

Is this ok like this:

"
Read SCP information can fails if the SCLP buffer length is too small
for the information to return which happens for example when defining 248 CPUs.

When SCLP read SCP information did fail during setup, we can currently not abort because
the function sclp_get_cpu_num(), called during exit, asserts on the previous success
of SCLP read SCP information.

The loop is:
abort() -> exit() -> smp_teardown() -> smp_query_num_cpus() ->
sclp_get_cpu_num() -> assert() -> abort()

Since smp_setup() is done after sclp_read_info() inside setup() this
loop happens when only the start processor is running.

Since only one processor is running and we know it, we do not
need to make the SCLP call in sclp_get_cpu_num() and can safely return 1.
"

>
>> Fixes: 52076a63d569 ("s390x: Consolidate sclp read info")
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/sclp.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>> index acdc8a9..c09360d 100644
>> --- a/lib/s390x/sclp.c
>> +++ b/lib/s390x/sclp.c
>> @@ -119,8 +119,9 @@ void sclp_read_info(void)
>>
>> int sclp_get_cpu_num(void)
>> {
>> - assert(read_info);
>> - return read_info->entries_cpu;
>> + if (read_info)
>> + return read_info->entries_cpu;
>> + return 1;
>> }
>>
>> CPUEntry *sclp_get_cpu_entries(void)