| Message ID | 20220311173822.1234617-4-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x SIGP fixes | expand |
On Fri, 2022-03-11 at 18:38 +0100, Eric Farman wrote: > In the routine test_stop_store_status(), the "running" part of > the test checks a few of the fields in lowcore (to verify the > "STORE STATUS" part of the SIGP order), and then ensures that > the CPU has stopped. But this is backwards, according to the > Principles of Operation: > The addressed CPU performs the stop function, fol- > lowed by the store-status operation (see “Store Sta- > tus” on page 4-82). > > If the CPU were not yet stopped, the contents of the lowcore > fields would be unpredictable. It works today because the > library functions wait on the stop function, so the CPU is > stopped by the time it comes back. Let's first check that the > CPU is stopped first, just to be clear. > > While here, add the same check to the second part of the test, > even though the CPU is explicitly stopped prior to the SIGP. > > Fixes: fc67b07a4 ("s390x: smp: Test stop and store status on a > running and stopped cpu") > Signed-off-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
On Fri, 11 Mar 2022 18:38:19 +0100 Eric Farman <farman@linux.ibm.com> wrote: > In the routine test_stop_store_status(), the "running" part of > the test checks a few of the fields in lowcore (to verify the > "STORE STATUS" part of the SIGP order), and then ensures that > the CPU has stopped. But this is backwards, according to the > Principles of Operation: > The addressed CPU performs the stop function, fol- > lowed by the store-status operation (see “Store Sta- > tus” on page 4-82). > > If the CPU were not yet stopped, the contents of the lowcore > fields would be unpredictable. It works today because the > library functions wait on the stop function, so the CPU is > stopped by the time it comes back. Let's first check that the > CPU is stopped first, just to be clear. > > While here, add the same check to the second part of the test, > even though the CPU is explicitly stopped prior to the SIGP. > > Fixes: fc67b07a4 ("s390x: smp: Test stop and store status on a running and stopped cpu") > Signed-off-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > s390x/smp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/s390x/smp.c b/s390x/smp.c > index 2f4af820..50811bd0 100644 > --- a/s390x/smp.c > +++ b/s390x/smp.c > @@ -98,9 +98,9 @@ static void test_stop_store_status(void) > lc->grs_sa[15] = 0; > smp_cpu_stop_store_status(1); > mb(); > + report(smp_cpu_stopped(1), "cpu stopped"); > report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix"); > report(lc->grs_sa[15], "stack"); > - report(smp_cpu_stopped(1), "cpu stopped"); > report_prefix_pop(); > > report_prefix_push("stopped"); > @@ -108,6 +108,7 @@ static void test_stop_store_status(void) > lc->grs_sa[15] = 0; > smp_cpu_stop_store_status(1); > mb(); > + report(smp_cpu_stopped(1), "cpu stopped"); > report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix"); > report(lc->grs_sa[15], "stack"); > report_prefix_pop();
diff --git a/s390x/smp.c b/s390x/smp.c index 2f4af820..50811bd0 100644 --- a/s390x/smp.c +++ b/s390x/smp.c @@ -98,9 +98,9 @@ static void test_stop_store_status(void) lc->grs_sa[15] = 0; smp_cpu_stop_store_status(1); mb(); + report(smp_cpu_stopped(1), "cpu stopped"); report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix"); report(lc->grs_sa[15], "stack"); - report(smp_cpu_stopped(1), "cpu stopped"); report_prefix_pop(); report_prefix_push("stopped"); @@ -108,6 +108,7 @@ static void test_stop_store_status(void) lc->grs_sa[15] = 0; smp_cpu_stop_store_status(1); mb(); + report(smp_cpu_stopped(1), "cpu stopped"); report(lc->prefix_sa == (uint32_t)(uintptr_t)cpu->lowcore, "prefix"); report(lc->grs_sa[15], "stack"); report_prefix_pop();
In the routine test_stop_store_status(), the "running" part of the test checks a few of the fields in lowcore (to verify the "STORE STATUS" part of the SIGP order), and then ensures that the CPU has stopped. But this is backwards, according to the Principles of Operation: The addressed CPU performs the stop function, fol- lowed by the store-status operation (see “Store Sta- tus” on page 4-82). If the CPU were not yet stopped, the contents of the lowcore fields would be unpredictable. It works today because the library functions wait on the stop function, so the CPU is stopped by the time it comes back. Let's first check that the CPU is stopped first, just to be clear. While here, add the same check to the second part of the test, even though the CPU is explicitly stopped prior to the SIGP. Fixes: fc67b07a4 ("s390x: smp: Test stop and store status on a running and stopped cpu") Signed-off-by: Eric Farman <farman@linux.ibm.com> --- s390x/smp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)