Re: [kvm-unit-tests PATCH v1 0/5] s390x: smp: avoid hardcoded CPU addresses

From: Janosch Frank <frankja@linux.ibm.com>
To: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: kvm@vger.kernel.org, thuth@redhat.com, david@redhat.com,
	nrb@linux.ibm.com, scgl@linux.ibm.com, seiden@linux.ibm.com
Subject: Re: [kvm-unit-tests PATCH v1 0/5] s390x: smp: avoid hardcoded CPU addresses
Date: Fri, 4 Feb 2022 16:37:50 +0100	[thread overview]
Message-ID: <7c734d96-a4ec-b158-526e-b0bd2290e8af@linux.ibm.com> (raw)
In-Reply-To: <20220204161430.27d1da36@p-imbrenda>

On 2/4/22 16:14, Claudio Imbrenda wrote:
> On Fri, 4 Feb 2022 16:01:54 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 1/28/22 19:54, Claudio Imbrenda wrote:
>>> On s390x there are no guarantees about the CPU addresses, except that
>>> they shall be unique. This means that in some environments, it's
>>> possible that there is no match between the CPU address and its
>>> position (index) in the list of available CPUs returned by the system.
>>>
>>> This series fixes a small bug in the SMP initialization code, adds a
>>> guarantee that the boot CPU will always have index 0, and introduces
>>> some functions to allow tests to use CPU indexes instead of using
>>> hardcoded CPU addresses. This will allow the tests to run successfully
>>> in more environments (e.g. z/VM, LPAR).
>>>
>>> Some existing tests are adapted to take advantage of the new
>>> functionalities.
>>>
>>> Claudio Imbrenda (5):
>>>     lib: s390x: smp: add functions to work with CPU indexes
>>>     lib: s390x: smp: guarantee that boot CPU has index 0
>>>     s390x: smp: avoid hardcoded CPU addresses
>>>     s390x: firq: avoid hardcoded CPU addresses
>>>     s390x: skrf: avoid hardcoded CPU addresses
>>>
>>>    lib/s390x/smp.h |  2 ++
>>>    lib/s390x/smp.c | 28 ++++++++++++-----
>>>    s390x/firq.c    | 17 +++++-----
>>>    s390x/skrf.c    |  8 +++--
>>>    s390x/smp.c     | 83 ++++++++++++++++++++++++++-----------------------
>>
>> We use smp/sigp in uv-host.c and one of those uses looks a bit strange
>> to me anyway.
> 
> I had noticed that, it's fixed in the v2 (and that test will almost
> surely be rewritten anyway)
> 
>>
>> I think we also need to fix the sigp in cstart.S to only stop itself and
>> not the cpu with the addr 0.
> 
> not sure if that is needed right now. that is only used for snippets,
> right?

Yes

> 
> or did you mean cstart64.S?
> but there, sigp is only used for SET_ARCHITECTURE, so it doesn't really
> matter I guess?

My bad, seems like it's Friday

> 
>>
>> Up to now we very much assumed that cpu 0 is always our boot cpu so if
>> you start running the test with cpu addr 1 and 2 and leave out 0 you
>> might find more problematic code.
>>
>>
>>>    5 files changed, 79 insertions(+), 59 deletions(-)
>>>    
>>
>