| Message ID | 20220114100245.8643-5-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Allocation and hosting environment detection fixes | expand |
On Fri, 14 Jan 2022 10:02:44 +0000 Janosch Frank <frankja@linux.ibm.com> wrote: > The store status at address order works with 31 bit addresses so let's > use them. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > s390x/smp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/s390x/smp.c b/s390x/smp.c > index 32f128b3..c91f170b 100644 > --- a/s390x/smp.c > +++ b/s390x/smp.c > @@ -124,7 +124,7 @@ static void test_stop_store_status(void) > > static void test_store_status(void) > { > - struct cpu_status *status = alloc_pages(1); > + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > uint32_t r; > > report_prefix_push("store status at address"); > @@ -244,7 +244,7 @@ static void test_func_initial(void) > > static void test_reset_initial(void) > { > - struct cpu_status *status = alloc_pages(0); > + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > struct psw psw; > int i; >
On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: > The store status at address order works with 31 bit addresses so > let's > use them. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/smp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/s390x/smp.c b/s390x/smp.c > index 32f128b3..c91f170b 100644 > --- a/s390x/smp.c > +++ b/s390x/smp.c [...] > @@ -244,7 +244,7 @@ static void test_func_initial(void) > > static void test_reset_initial(void) > { > - struct cpu_status *status = alloc_pages(0); > + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); Why do we need two pages now?
On Fri, 14 Jan 2022 13:50:52 +0100 Nico Boehr <nrb@linux.ibm.com> wrote: > On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: > > The store status at address order works with 31 bit addresses so > > let's > > use them. > > > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > > --- > > s390x/smp.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/s390x/smp.c b/s390x/smp.c > > index 32f128b3..c91f170b 100644 > > --- a/s390x/smp.c > > +++ b/s390x/smp.c > > [...] > > > @@ -244,7 +244,7 @@ static void test_func_initial(void) > > > > static void test_reset_initial(void) > > { > > - struct cpu_status *status = alloc_pages(0); > > + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > > Why do we need two pages now? oh, good catch the next patch has the same issue I can fix them up when I queue them
On Fri, 14 Jan 2022 13:50:52 +0100 Nico Boehr <nrb@linux.ibm.com> wrote: > On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: > > The store status at address order works with 31 bit addresses so > > let's > > use them. > > > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > > --- > > s390x/smp.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/s390x/smp.c b/s390x/smp.c > > index 32f128b3..c91f170b 100644 > > --- a/s390x/smp.c > > +++ b/s390x/smp.c > > [...] > > > @@ -244,7 +244,7 @@ static void test_func_initial(void) > > > > static void test_reset_initial(void) > > { > > - struct cpu_status *status = alloc_pages(0); > > + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > > Why do we need two pages now? actually, wait..... struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); uint32_t r; report_prefix_push("store status at address"); memset(status, 0, PAGE_SIZE * 2); we were allocating one page, and using 2! @Janosch do we need 1 or 2 pages?
On 1/14/22 13:57, Claudio Imbrenda wrote: > On Fri, 14 Jan 2022 13:50:52 +0100 > Nico Boehr <nrb@linux.ibm.com> wrote: > >> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: >>> The store status at address order works with 31 bit addresses so >>> let's >>> use them. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> s390x/smp.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/s390x/smp.c b/s390x/smp.c >>> index 32f128b3..c91f170b 100644 >>> --- a/s390x/smp.c >>> +++ b/s390x/smp.c >> >> [...] >> >>> @@ -244,7 +244,7 @@ static void test_func_initial(void) >>> >>> static void test_reset_initial(void) >>> { >>> - struct cpu_status *status = alloc_pages(0); >>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); >> >> Why do we need two pages now? > > oh, good catch Indeed > > the next patch has the same issue > > I can fix them up when I queue them > Probably because I copied that line from somewhere :-) Yeah, you can fix that up if you want.
On 1/14/22 14:01, Claudio Imbrenda wrote: > On Fri, 14 Jan 2022 13:50:52 +0100 > Nico Boehr <nrb@linux.ibm.com> wrote: > >> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: >>> The store status at address order works with 31 bit addresses so >>> let's >>> use them. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> s390x/smp.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/s390x/smp.c b/s390x/smp.c >>> index 32f128b3..c91f170b 100644 >>> --- a/s390x/smp.c >>> +++ b/s390x/smp.c >> >> [...] >> >>> @@ -244,7 +244,7 @@ static void test_func_initial(void) >>> >>> static void test_reset_initial(void) >>> { >>> - struct cpu_status *status = alloc_pages(0); >>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); >> >> Why do we need two pages now? > > actually, wait..... > > struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > uint32_t r; > > report_prefix_push("store status at address"); > memset(status, 0, PAGE_SIZE * 2); > > we were allocating one page, and using 2! > > @Janosch do we need 1 or 2 pages? > Have a look at the memcmp() below those lines. I test if the status page has changed by doing a memcmp against the second page.
On Fri, 14 Jan 2022 14:13:01 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 1/14/22 14:01, Claudio Imbrenda wrote: > > On Fri, 14 Jan 2022 13:50:52 +0100 > > Nico Boehr <nrb@linux.ibm.com> wrote: > > > >> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: > >>> The store status at address order works with 31 bit addresses so > >>> let's > >>> use them. > >>> > >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > >>> --- > >>> s390x/smp.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/s390x/smp.c b/s390x/smp.c > >>> index 32f128b3..c91f170b 100644 > >>> --- a/s390x/smp.c > >>> +++ b/s390x/smp.c > >> > >> [...] > >> > >>> @@ -244,7 +244,7 @@ static void test_func_initial(void) > >>> > >>> static void test_reset_initial(void) > >>> { > >>> - struct cpu_status *status = alloc_pages(0); > >>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > >> > >> Why do we need two pages now? > > > > actually, wait..... > > > > struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > > uint32_t r; > > > > report_prefix_push("store status at address"); > > memset(status, 0, PAGE_SIZE * 2); > > > > we were allocating one page, and using 2! > > > > @Janosch do we need 1 or 2 pages? > > > > Have a look at the memcmp() below those lines. > > I test if the status page has changed by doing a memcmp against the > second page. so we do need 2 pages, and using 1 was a bug
On 1/14/22 14:16, Claudio Imbrenda wrote: > On Fri, 14 Jan 2022 14:13:01 +0100 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> On 1/14/22 14:01, Claudio Imbrenda wrote: >>> On Fri, 14 Jan 2022 13:50:52 +0100 >>> Nico Boehr <nrb@linux.ibm.com> wrote: >>> >>>> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: >>>>> The store status at address order works with 31 bit addresses so >>>>> let's >>>>> use them. >>>>> >>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>>>> --- >>>>> s390x/smp.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/s390x/smp.c b/s390x/smp.c >>>>> index 32f128b3..c91f170b 100644 >>>>> --- a/s390x/smp.c >>>>> +++ b/s390x/smp.c >>>> >>>> [...] >>>> >>>>> @@ -244,7 +244,7 @@ static void test_func_initial(void) >>>>> >>>>> static void test_reset_initial(void) >>>>> { >>>>> - struct cpu_status *status = alloc_pages(0); >>>>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); >>>> >>>> Why do we need two pages now? >>> >>> actually, wait..... >>> >>> struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); >>> uint32_t r; >>> >>> report_prefix_push("store status at address"); >>> memset(status, 0, PAGE_SIZE * 2); >>> >>> we were allocating one page, and using 2! >>> >>> @Janosch do we need 1 or 2 pages? >>> >> >> Have a look at the memcmp() below those lines. >> >> I test if the status page has changed by doing a memcmp against the >> second page. > > so we do need 2 pages, and using 1 was a bug > We need two for the store status at address tests but only one for the initial reset test Nico is pointing out here.
On Fri, 14 Jan 2022 14:13:01 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 1/14/22 14:01, Claudio Imbrenda wrote: > > On Fri, 14 Jan 2022 13:50:52 +0100 > > Nico Boehr <nrb@linux.ibm.com> wrote: > > > >> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: > >>> The store status at address order works with 31 bit addresses so > >>> let's > >>> use them. > >>> > >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > >>> --- > >>> s390x/smp.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/s390x/smp.c b/s390x/smp.c > >>> index 32f128b3..c91f170b 100644 > >>> --- a/s390x/smp.c > >>> +++ b/s390x/smp.c > >> > >> [...] > >> > >>> @@ -244,7 +244,7 @@ static void test_func_initial(void) > >>> > >>> static void test_reset_initial(void) > >>> { > >>> - struct cpu_status *status = alloc_pages(0); > >>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > >> > >> Why do we need two pages now? > > > > actually, wait..... > > > > struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); > > uint32_t r; > > > > report_prefix_push("store status at address"); > > memset(status, 0, PAGE_SIZE * 2); > > > > we were allocating one page, and using 2! > > > > @Janosch do we need 1 or 2 pages? > > > > Have a look at the memcmp() below those lines. > > I test if the status page has changed by doing a memcmp against the > second page. ENOCOFEE sorry I mixed things up we were using 2, it's the second part of the patch that only needs one
diff --git a/s390x/smp.c b/s390x/smp.c index 32f128b3..c91f170b 100644 --- a/s390x/smp.c +++ b/s390x/smp.c @@ -124,7 +124,7 @@ static void test_stop_store_status(void) static void test_store_status(void) { - struct cpu_status *status = alloc_pages(1); + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); uint32_t r; report_prefix_push("store status at address"); @@ -244,7 +244,7 @@ static void test_func_initial(void) static void test_reset_initial(void) { - struct cpu_status *status = alloc_pages(0); + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31); struct psw psw; int i;
The store status at address order works with 31 bit addresses so let's use them. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/smp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)