| Message ID | 20231010073855.26319-2-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Improve console handling | expand |
On Tue, 10 Oct 2023 07:38:53 +0000 Janosch Frank <frankja@linux.ibm.com> wrote: > For early sclp printing it's necessary to know if we're under LPAR or > not so we can apply compat SCLP ASCII transformations. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/hardware.c | 8 ++++++++ > lib/s390x/hardware.h | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/lib/s390x/hardware.c b/lib/s390x/hardware.c > index 2bcf9c4c..d5a752c0 100644 > --- a/lib/s390x/hardware.c > +++ b/lib/s390x/hardware.c > @@ -52,6 +52,14 @@ static enum s390_host do_detect_host(void *buf) > return HOST_IS_UNKNOWN; > } > > +enum s390_host detect_host_early(void) > +{ > + if (stsi_get_fc() == 2) > + return HOST_IS_LPAR; > + > + return HOST_IS_UNKNOWN; > +} > + > enum s390_host detect_host(void) > { > static enum s390_host host = HOST_IS_UNKNOWN; > diff --git a/lib/s390x/hardware.h b/lib/s390x/hardware.h > index 86fe873c..5e5a9d90 100644 > --- a/lib/s390x/hardware.h > +++ b/lib/s390x/hardware.h > @@ -24,6 +24,7 @@ enum s390_host { > }; > > enum s390_host detect_host(void); > +enum s390_host detect_host_early(void); I wonder if it weren't easier to fix detect_host so it can be used early.... > > static inline uint16_t get_machine_id(void) > {
On 10/10/23 12:40, Claudio Imbrenda wrote: > On Tue, 10 Oct 2023 07:38:53 +0000 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> For early sclp printing it's necessary to know if we're under LPAR or >> not so we can apply compat SCLP ASCII transformations. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> lib/s390x/hardware.c | 8 ++++++++ >> lib/s390x/hardware.h | 1 + >> 2 files changed, 9 insertions(+) >> >> diff --git a/lib/s390x/hardware.c b/lib/s390x/hardware.c >> index 2bcf9c4c..d5a752c0 100644 >> --- a/lib/s390x/hardware.c >> +++ b/lib/s390x/hardware.c >> @@ -52,6 +52,14 @@ static enum s390_host do_detect_host(void *buf) >> return HOST_IS_UNKNOWN; >> } >> >> +enum s390_host detect_host_early(void) >> +{ >> + if (stsi_get_fc() == 2) >> + return HOST_IS_LPAR; >> + >> + return HOST_IS_UNKNOWN; >> +} >> + >> enum s390_host detect_host(void) >> { >> static enum s390_host host = HOST_IS_UNKNOWN; >> diff --git a/lib/s390x/hardware.h b/lib/s390x/hardware.h >> index 86fe873c..5e5a9d90 100644 >> --- a/lib/s390x/hardware.h >> +++ b/lib/s390x/hardware.h >> @@ -24,6 +24,7 @@ enum s390_host { >> }; >> >> enum s390_host detect_host(void); >> +enum s390_host detect_host_early(void); > > I wonder if it weren't easier to fix detect_host so it can be used > early.... > Problem is, where do you start and where do you end? We could statically allocate a page but why did we add the current version then? (The old version did that if I remember correctly).
On Tue, 10 Oct 2023 13:03:08 +0200 Janosch Frank <frankja@linux.ibm.com> wrote: > On 10/10/23 12:40, Claudio Imbrenda wrote: > > On Tue, 10 Oct 2023 07:38:53 +0000 > > Janosch Frank <frankja@linux.ibm.com> wrote: > > > >> For early sclp printing it's necessary to know if we're under LPAR or > >> not so we can apply compat SCLP ASCII transformations. > >> > >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > >> --- > >> lib/s390x/hardware.c | 8 ++++++++ > >> lib/s390x/hardware.h | 1 + > >> 2 files changed, 9 insertions(+) > >> > >> diff --git a/lib/s390x/hardware.c b/lib/s390x/hardware.c > >> index 2bcf9c4c..d5a752c0 100644 > >> --- a/lib/s390x/hardware.c > >> +++ b/lib/s390x/hardware.c > >> @@ -52,6 +52,14 @@ static enum s390_host do_detect_host(void *buf) > >> return HOST_IS_UNKNOWN; > >> } > >> > >> +enum s390_host detect_host_early(void) > >> +{ > >> + if (stsi_get_fc() == 2) > >> + return HOST_IS_LPAR; > >> + > >> + return HOST_IS_UNKNOWN; > >> +} > >> + > >> enum s390_host detect_host(void) > >> { > >> static enum s390_host host = HOST_IS_UNKNOWN; > >> diff --git a/lib/s390x/hardware.h b/lib/s390x/hardware.h > >> index 86fe873c..5e5a9d90 100644 > >> --- a/lib/s390x/hardware.h > >> +++ b/lib/s390x/hardware.h > >> @@ -24,6 +24,7 @@ enum s390_host { > >> }; > >> > >> enum s390_host detect_host(void); > >> +enum s390_host detect_host_early(void); > > > > I wonder if it weren't easier to fix detect_host so it can be used > > early.... > > > > Problem is, where do you start and where do you end? > > We could statically allocate a page but why did we add the current > version then? (The old version did that if I remember correctly). the old version also allocated pages, and was a hodgepodge of various functions replicating the same behaviour, many calling the others. the main goal of the current version was to make it more readable and maintainable. a possible fix could also involve allocating the buffer on the stack of do_detect_host(), so it's not taking up memory permanently. that said, I don't have a strong opinion about this, but maybe it's a good idea to not replicate the same behaviour again :) if you don't want to fix detect_host(), then maybe something like this instead? diff --git a/lib/s390x/hardware.h b/lib/s390x/hardware.h index 86fe873c..5e5a9d90 100644 --- a/lib/s390x/hardware.h +++ b/lib/s390x/hardware.h @@ -24,6 +24,7 @@ enum s390_host { }; enum s390_host detect_host(void); +enum s390_host detect_host_early(void); static inline uint16_t get_machine_id(void) { diff --git a/lib/s390x/hardware.c b/lib/s390x/hardware.c index 2bcf9c4c..b281cf10 100644 --- a/lib/s390x/hardware.c +++ b/lib/s390x/hardware.c @@ -28,7 +28,7 @@ static enum s390_host do_detect_host(void *buf) if (stsi_get_fc() == 2) return HOST_IS_LPAR; - if (stsi_get_fc() != 3) + if (!buf || stsi_get_fc() != 3) return HOST_IS_UNKNOWN; if (!stsi(buf, 1, 1, 1)) { @@ -67,3 +67,8 @@ enum s390_host detect_host(void) initialized = true; return host; } + +enum s390_host detect_host_early(void) +{ + return do_detect_host(NULL); +}
On 10/10/23 13:42, Claudio Imbrenda wrote: > On Tue, 10 Oct 2023 13:03:08 +0200 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> On 10/10/23 12:40, Claudio Imbrenda wrote: >>> On Tue, 10 Oct 2023 07:38:53 +0000 >>> Janosch Frank <frankja@linux.ibm.com> wrote: >>> >>>> For early sclp printing it's necessary to know if we're under LPAR or >>>> not so we can apply compat SCLP ASCII transformations. >>>> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>>> --- >>>> lib/s390x/hardware.c | 8 ++++++++ >>>> lib/s390x/hardware.h | 1 + >>>> 2 files changed, 9 insertions(+) >>>> >>>> diff --git a/lib/s390x/hardware.c b/lib/s390x/hardware.c >>>> index 2bcf9c4c..d5a752c0 100644 >>>> --- a/lib/s390x/hardware.c >>>> +++ b/lib/s390x/hardware.c >>>> @@ -52,6 +52,14 @@ static enum s390_host do_detect_host(void *buf) >>>> return HOST_IS_UNKNOWN; >>>> } >>>> >>>> +enum s390_host detect_host_early(void) >>>> +{ >>>> + if (stsi_get_fc() == 2) >>>> + return HOST_IS_LPAR; >>>> + >>>> + return HOST_IS_UNKNOWN; >>>> +} >>>> + >>>> enum s390_host detect_host(void) >>>> { >>>> static enum s390_host host = HOST_IS_UNKNOWN; >>>> diff --git a/lib/s390x/hardware.h b/lib/s390x/hardware.h >>>> index 86fe873c..5e5a9d90 100644 >>>> --- a/lib/s390x/hardware.h >>>> +++ b/lib/s390x/hardware.h >>>> @@ -24,6 +24,7 @@ enum s390_host { >>>> }; >>>> >>>> enum s390_host detect_host(void); >>>> +enum s390_host detect_host_early(void); >>> >>> I wonder if it weren't easier to fix detect_host so it can be used >>> early.... >>> >> >> Problem is, where do you start and where do you end? >> >> We could statically allocate a page but why did we add the current >> version then? (The old version did that if I remember correctly). > > the old version also allocated pages, and was a hodgepodge of various > functions replicating the same behaviour, many calling the others. the > main goal of the current version was to make it more readable and > maintainable. > > a possible fix could also involve allocating the buffer on the stack of > do_detect_host(), so it's not taking up memory permanently. > We have 64k of stack and since we're calling this from the setup code a huge part should be free. I'll have a look at this.
diff --git a/lib/s390x/hardware.c b/lib/s390x/hardware.c index 2bcf9c4c..d5a752c0 100644 --- a/lib/s390x/hardware.c +++ b/lib/s390x/hardware.c @@ -52,6 +52,14 @@ static enum s390_host do_detect_host(void *buf) return HOST_IS_UNKNOWN; } +enum s390_host detect_host_early(void) +{ + if (stsi_get_fc() == 2) + return HOST_IS_LPAR; + + return HOST_IS_UNKNOWN; +} + enum s390_host detect_host(void) { static enum s390_host host = HOST_IS_UNKNOWN; diff --git a/lib/s390x/hardware.h b/lib/s390x/hardware.h index 86fe873c..5e5a9d90 100644 --- a/lib/s390x/hardware.h +++ b/lib/s390x/hardware.h @@ -24,6 +24,7 @@ enum s390_host { }; enum s390_host detect_host(void); +enum s390_host detect_host_early(void); static inline uint16_t get_machine_id(void) {
For early sclp printing it's necessary to know if we're under LPAR or not so we can apply compat SCLP ASCII transformations. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/hardware.c | 8 ++++++++ lib/s390x/hardware.h | 1 + 2 files changed, 9 insertions(+)