| Message ID | 1516732013-18272-10-git-send-email-walling@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23.01.2018 19:26, Collin L. Walling wrote: > Implements an sclp_read function to capture input from the > console and a wrapper function that handles parsing certain > characters and adding input to a buffer. The input is checked > for any erroneous values and is handled appropriately. > > A prompt will persist until input is entered or the timeout > expires (if one was set). Example: > > Please choose (default will boot in 10 seconds): > > Correct input will boot the respective boot index. If the > user's input is empty, 0, or if the timeout expires, then > the default zipl entry will be chosen. If the input is > within the range of available boot entries, then the > selection will be booted. Any erroneous input will cancel > the timeout and re-prompt the user. > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > --- > pc-bios/s390-ccw/menu.c | 152 +++++++++++++++++++++++++++++++++++++++++++- > pc-bios/s390-ccw/s390-ccw.h | 2 + > pc-bios/s390-ccw/sclp.c | 20 ++++++ > pc-bios/s390-ccw/virtio.c | 2 +- > 4 files changed, 172 insertions(+), 4 deletions(-) [...] > +static int read_prompt(char *buf, size_t len) > +{ > + char inp[2] = {}; > + uint8_t idx = 0; > + uint64_t time; > + > + if (timeout) { > + time = get_clock() + (timeout * TOD_CLOCK_SECOND); Nit: Parentheses around "timeout * TOD_CLOCK_SECOND" are not required. Apart from that, patch looks fine to me now. Reviewed-by: Thomas Huth <thuth@redhat.com>
On 23.01.2018 19:26, Collin L. Walling wrote: > Implements an sclp_read function to capture input from the > console and a wrapper function that handles parsing certain > characters and adding input to a buffer. The input is checked > for any erroneous values and is handled appropriately. > > A prompt will persist until input is entered or the timeout > expires (if one was set). Example: > > Please choose (default will boot in 10 seconds): Wondering if it would be possible to print the list of boot options (just like zipl, if that is possible). > > Correct input will boot the respective boot index. If the > user's input is empty, 0, or if the timeout expires, then > the default zipl entry will be chosen. If the input is > within the range of available boot entries, then the > selection will be booted. Any erroneous input will cancel > the timeout and re-prompt the user. > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > --- > pc-bios/s390-ccw/menu.c | 152 +++++++++++++++++++++++++++++++++++++++++++- > pc-bios/s390-ccw/s390-ccw.h | 2 + > pc-bios/s390-ccw/sclp.c | 20 ++++++ > pc-bios/s390-ccw/virtio.c | 2 +- > 4 files changed, 172 insertions(+), 4 deletions(-) > > diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c > index 174285e..24d4bba 100644 > --- a/pc-bios/s390-ccw/menu.c > +++ b/pc-bios/s390-ccw/menu.c > @@ -12,16 +12,162 @@ > #include "menu.h" > #include "s390-ccw.h" > > -static uint8_t flags; > -static uint64_t timeout; > +#define KEYCODE_NO_INP '\0' > +#define KEYCODE_ESCAPE '\033' > +#define KEYCODE_BACKSP '\177' > +#define KEYCODE_ENTER '\r' > > /* Offsets from zipl fields to zipl banner start */ > #define ZIPL_TIMEOUT_OFFSET 138 > #define ZIPL_FLAG_OFFSET 140 > > +#define TOD_CLOCK_SECOND 0xf4240000 > + > +static uint8_t flags; > +static uint64_t timeout; > + > +static inline void enable_clock_int(void) > +{ > + uint64_t tmp = 0; Initialization is not necessary. (output only register.) > + > + asm volatile( > + "stctg 0,0,%0\n" > + "oi 6+%0, 0x8\n" Isn't the commonly used syntax 6(%0)? (e.g. see consume_sclp_int ) (but then I think you would have to move tmp into an "r" instead). Definitely not an expert :) > + "lctlg 0,0,%0" > + : : "Q" (tmp) : "memory" > + ); > +} > + > +static inline void disable_clock_int(void) > +{ > + uint64_t tmp = 0; dito. Wonder if some stctg/lctlg helpers would be better, than the anding/oring can be done in c code. > + > + asm volatile( > + "stctg 0,0,%0\n" > + "ni 6+%0, 0xf7\n" > + "lctlg 0,0,%0" > + : : "Q" (tmp) : "memory" > + ); > +} > + > +static inline void set_clock_comparator(uint64_t time) > +{ > + asm volatile("sckc %0" : : "Q" (time)); > +} > + > +static inline bool check_clock_int(void) > +{ > + uint16_t *code = (uint16_t *)0x86; /* low-core external interrupt code */ > + > + consume_sclp_int(); Can you add a comment like /* * We either receive an sclp interrupt or a timer interrupt. */ However, I think this would be much cleaner if refactored into: consume_sclp_int() -> consume_ext_int(). And move - the "enable service interrupts in cr0" into a C function enable_sclp_int() - the "disable service interrupts in cr0" into a C function disable_sclp_int() void consume_sclp_int(void) { enable_sclp_int(); consume_ext_int(); disable_sclp_int(); } For existing code and for your function here e.g. enable_sclp_int(); enable_clock_comparator_int(); consume_ext_int(); disable_clck_comparator_int(); disable_sclp_int(); return *code == 0x1004; > + > + return *code == 0x1004; > +} > + > +static int read_prompt(char *buf, size_t len) > +{ > + char inp[2] = {}; > + uint8_t idx = 0; > + uint64_t time; > + > + if (timeout) { > + time = get_clock() + (timeout * TOD_CLOCK_SECOND); > + set_clock_comparator(time); > + enable_clock_int(); > + timeout = 0; > + } > + > + while (!check_clock_int()) { > + > + /* Process only one character at a time */ > + sclp_read(inp, 1); > + > + switch (inp[0]) { > + case KEYCODE_NO_INP: > + case KEYCODE_ESCAPE: > + continue; > + case KEYCODE_BACKSP: > + if (idx > 0) { > + buf[--idx] = 0; > + sclp_print("\b \b"); > + } > + continue; > + case KEYCODE_ENTER: > + disable_clock_int(); > + return idx; > + } > + > + /* Echo input and add to buffer */ > + if (idx < len) { > + buf[idx] = inp[0]; > + sclp_print(inp); > + idx++; > + } > + } > + > + disable_clock_int(); > + *buf = 0; > + > + return 0; > +} > + > +static int get_index(void) > +{ > + char buf[10]; > + int len; > + int i; > + > + memset(buf, 0, sizeof(buf)); > + > + len = read_prompt(buf, sizeof(buf)); > + > + /* If no input, boot default */ > + if (len == 0) { > + return 0; > + } > + > + /* Check for erroneous input */ > + for (i = 0; i < len; i++) { > + if (!isdigit(buf[i])) { > + return -1; > + } > + } > + > + return atoi(buf); > +} > + > +static void boot_menu_prompt(bool retry) > +{ > + char tmp[6]; > + > + if (retry) { > + sclp_print("\nError: undefined configuration" > + "\nPlease choose:\n"); > + } else if (timeout > 0) { > + sclp_print("Please choose (default will boot in "); > + sclp_print(itostr(timeout, tmp, sizeof(tmp))); > + sclp_print(" seconds):\n"); > + } else { > + sclp_print("Please choose:\n"); > + } > +} > + > static int get_boot_index(int entries) > { > - return 0; /* Implemented next patch */ > + int boot_index; > + bool retry = false; > + char tmp[5]; > + > + do { > + boot_menu_prompt(retry); > + boot_index = get_index(); > + retry = true; > + } while (boot_index < 0 || boot_index >= entries); > + > + sclp_print("\nBooting entry #"); > + sclp_print(itostr(boot_index, tmp, sizeof(tmp))); > + > + return boot_index; > } > > static void zipl_println(const char *data, size_t len) > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h > index 25d4d21..df4bc88 100644 > --- a/pc-bios/s390-ccw/s390-ccw.h > +++ b/pc-bios/s390-ccw/s390-ccw.h > @@ -71,6 +71,7 @@ unsigned int get_loadparm_index(void); > void sclp_print(const char *string); > void sclp_setup(void); > void sclp_get_loadparm_ascii(char *loadparm); > +void sclp_read(char *str, size_t len); > > /* virtio.c */ > unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2, > @@ -79,6 +80,7 @@ bool virtio_is_supported(SubChannelId schid); > void virtio_blk_setup_device(SubChannelId schid); > int virtio_read(ulong sector, void *load_addr); > int enable_mss_facility(void); > +u64 get_clock(void); > ulong get_second(void); > > /* bootmap.c */ > diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c > index 486fce1..5e4a78b 100644 > --- a/pc-bios/s390-ccw/sclp.c > +++ b/pc-bios/s390-ccw/sclp.c > @@ -101,3 +101,23 @@ void sclp_get_loadparm_ascii(char *loadparm) > ebcdic_to_ascii((char *) sccb->loadparm, loadparm, 8); > } > } > + > +void sclp_read(char *str, size_t len) > +{ > + ReadEventData *sccb = (void *)_sccb; > + char *buf = (char *)(&sccb->ebh) + 7; > + > + /* Len should not exceed the maximum size of the event buffer */ > + if (len > SCCB_SIZE - 8) { > + len = SCCB_SIZE - 8; > + } > + > + sccb->h.length = SCCB_SIZE; > + sccb->h.function_code = SCLP_UNCONDITIONAL_READ; > + sccb->ebh.length = sizeof(EventBufferHeader); > + sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA; > + sccb->ebh.flags = 0; > + > + sclp_service_call(SCLP_CMD_READ_EVENT_DATA, sccb); > + memcpy(str, buf, len); > +} > diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c > index c890a03..817e7f5 100644 > --- a/pc-bios/s390-ccw/virtio.c > +++ b/pc-bios/s390-ccw/virtio.c > @@ -176,7 +176,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags) > } > } > > -static u64 get_clock(void) > +u64 get_clock(void) > { > u64 r; > >
On 29.01.2018 11:07, David Hildenbrand wrote: > On 23.01.2018 19:26, Collin L. Walling wrote: >> Implements an sclp_read function to capture input from the >> console and a wrapper function that handles parsing certain >> characters and adding input to a buffer. The input is checked >> for any erroneous values and is handled appropriately. >> >> A prompt will persist until input is entered or the timeout >> expires (if one was set). Example: >> >> Please choose (default will boot in 10 seconds): > > Wondering if it would be possible to print the list of boot options > (just like zipl, if that is possible). Just answered my question with patch 8 :)
On 23.01.2018 19:26, Collin L. Walling wrote: > Implements an sclp_read function to capture input from the > console and a wrapper function that handles parsing certain > characters and adding input to a buffer. The input is checked > for any erroneous values and is handled appropriately. > > A prompt will persist until input is entered or the timeout > expires (if one was set). Example: > > Please choose (default will boot in 10 seconds): > > Correct input will boot the respective boot index. If the > user's input is empty, 0, or if the timeout expires, then > the default zipl entry will be chosen. If the input is > within the range of available boot entries, then the > selection will be booted. Any erroneous input will cancel > the timeout and re-prompt the user. > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > --- Also, a very nasty thing to take care of is the following: SCLP and ckc interrupt at the same time pending. -> We only dequeue one, the other remains pending and is presented to the guest
On 01/29/2018 08:08 AM, David Hildenbrand wrote: > On 23.01.2018 19:26, Collin L. Walling wrote: >> Implements an sclp_read function to capture input from the >> console and a wrapper function that handles parsing certain >> characters and adding input to a buffer. The input is checked >> for any erroneous values and is handled appropriately. >> >> A prompt will persist until input is entered or the timeout >> expires (if one was set). Example: >> >> Please choose (default will boot in 10 seconds): >> >> Correct input will boot the respective boot index. If the >> user's input is empty, 0, or if the timeout expires, then >> the default zipl entry will be chosen. If the input is >> within the range of available boot entries, then the >> selection will be booted. Any erroneous input will cancel >> the timeout and re-prompt the user. >> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >> --- > Also, a very nasty thing to take care of is the following: > > SCLP and ckc interrupt at the same time pending. > > -> We only dequeue one, the other remains pending and is presented to > the guest > If I understand the assembler correctly, consume_sclp_int() takes care of enabling / disabling the service interrupts for us. However, I /do/like the refactoring suggestion you made in a previous reply. It makes things easier to read. If it makes sense to do so (such that the refactoring doesn't end up taking me down a rabbit hole) I'll spin up another patch that refactors the enabling / disabling of the service interrupt.
On 01/29/2018 05:07 AM, David Hildenbrand wrote: > On 23.01.2018 19:26, Collin L. Walling wrote: >> Implements an sclp_read function to capture input from the >> console and a wrapper function that handles parsing certain >> characters and adding input to a buffer. The input is checked >> for any erroneous values and is handled appropriately. >> >> A prompt will persist until input is entered or the timeout >> expires (if one was set). Example: >> >> Please choose (default will boot in 10 seconds): > Wondering if it would be possible to print the list of boot options > (just like zipl, if that is possible). > >> Correct input will boot the respective boot index. If the >> user's input is empty, 0, or if the timeout expires, then >> the default zipl entry will be chosen. If the input is >> within the range of available boot entries, then the >> selection will be booted. Any erroneous input will cancel >> the timeout and re-prompt the user. >> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >> --- >> pc-bios/s390-ccw/menu.c | 152 +++++++++++++++++++++++++++++++++++++++++++- >> pc-bios/s390-ccw/s390-ccw.h | 2 + >> pc-bios/s390-ccw/sclp.c | 20 ++++++ >> pc-bios/s390-ccw/virtio.c | 2 +- >> 4 files changed, 172 insertions(+), 4 deletions(-) >> >> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c >> index 174285e..24d4bba 100644 >> --- a/pc-bios/s390-ccw/menu.c >> +++ b/pc-bios/s390-ccw/menu.c >> @@ -12,16 +12,162 @@ >> #include "menu.h" >> #include "s390-ccw.h" >> >> -static uint8_t flags; >> -static uint64_t timeout; >> +#define KEYCODE_NO_INP '\0' >> +#define KEYCODE_ESCAPE '\033' >> +#define KEYCODE_BACKSP '\177' >> +#define KEYCODE_ENTER '\r' >> >> /* Offsets from zipl fields to zipl banner start */ >> #define ZIPL_TIMEOUT_OFFSET 138 >> #define ZIPL_FLAG_OFFSET 140 >> >> +#define TOD_CLOCK_SECOND 0xf4240000 >> + >> +static uint8_t flags; >> +static uint64_t timeout; >> + >> +static inline void enable_clock_int(void) >> +{ >> + uint64_t tmp = 0; > Initialization is not necessary. (output only register.) > >> + >> + asm volatile( >> + "stctg 0,0,%0\n" >> + "oi 6+%0, 0x8\n" > Isn't the commonly used syntax 6(%0)? (e.g. see consume_sclp_int ) > > (but then I think you would have to move tmp into an "r" instead). > > Definitely not an expert :) Neither am I... I'll play around with it and see what happens (worst case: we learn something new here :) ) > >> + "lctlg 0,0,%0" >> + : : "Q" (tmp) : "memory" >> + ); >> +} >> + >> +static inline void disable_clock_int(void) >> +{ >> + uint64_t tmp = 0; > dito. > > Wonder if some stctg/lctlg helpers would be better, than the > anding/oring can be done in c code. > >> + >> + asm volatile( >> + "stctg 0,0,%0\n" >> + "ni 6+%0, 0xf7\n" >> + "lctlg 0,0,%0" >> + : : "Q" (tmp) : "memory" >> + ); >> +} >> + >> +static inline void set_clock_comparator(uint64_t time) >> +{ >> + asm volatile("sckc %0" : : "Q" (time)); >> +} >> + >> +static inline bool check_clock_int(void) >> +{ >> + uint16_t *code = (uint16_t *)0x86; /* low-core external interrupt code */ >> + >> + consume_sclp_int(); > Can you add a comment like > > /* > * We either receive an sclp interrupt or a timer interrupt. > */ > > However, I think this would be much cleaner if refactored into: > > consume_sclp_int() -> consume_ext_int(). > > And move > - the "enable service interrupts in cr0" into a C function > enable_sclp_int() > - the "disable service interrupts in cr0" into a C function > disable_sclp_int() > > void consume_sclp_int(void) > { > enable_sclp_int(); > consume_ext_int(); > disable_sclp_int(); > } > > For existing code and for your function here e.g. > > enable_sclp_int(); > enable_clock_comparator_int(); > consume_ext_int(); > disable_clck_comparator_int(); > disable_sclp_int(); > > return *code == 0x1004; Seems sane to me. >> + >> + return *code == 0x1004; >> +} >> + >> +static int read_prompt(char *buf, size_t len) >> +{ >> + char inp[2] = {}; >> + uint8_t idx = 0; >> + uint64_t time; >> + >> + if (timeout) { >> + time = get_clock() + (timeout * TOD_CLOCK_SECOND); >> + set_clock_comparator(time); >> + enable_clock_int(); >> + timeout = 0; >> + } >> + >> + while (!check_clock_int()) { >> + >> + /* Process only one character at a time */ >> + sclp_read(inp, 1); >> + >> + switch (inp[0]) { >> + case KEYCODE_NO_INP: >> + case KEYCODE_ESCAPE: >> + continue; >> + case KEYCODE_BACKSP: >> + if (idx > 0) { >> + buf[--idx] = 0; >> + sclp_print("\b \b"); >> + } >> + continue; >> + case KEYCODE_ENTER: >> + disable_clock_int(); >> + return idx; >> + } >> + >> + /* Echo input and add to buffer */ >> + if (idx < len) { >> + buf[idx] = inp[0]; >> + sclp_print(inp); >> + idx++; >> + } >> + } >> + >> + disable_clock_int(); >> + *buf = 0; >> + >> + return 0; >> +} >> + >> +static int get_index(void) >> +{ >> + char buf[10]; >> + int len; >> + int i; >> + >> + memset(buf, 0, sizeof(buf)); >> + >> + len = read_prompt(buf, sizeof(buf)); >> + >> + /* If no input, boot default */ >> + if (len == 0) { >> + return 0; >> + } >> + >> + /* Check for erroneous input */ >> + for (i = 0; i < len; i++) { >> + if (!isdigit(buf[i])) { >> + return -1; >> + } >> + } >> + >> + return atoi(buf); >> +} >> + >> +static void boot_menu_prompt(bool retry) >> +{ >> + char tmp[6]; >> + >> + if (retry) { >> + sclp_print("\nError: undefined configuration" >> + "\nPlease choose:\n"); >> + } else if (timeout > 0) { >> + sclp_print("Please choose (default will boot in "); >> + sclp_print(itostr(timeout, tmp, sizeof(tmp))); >> + sclp_print(" seconds):\n"); >> + } else { >> + sclp_print("Please choose:\n"); >> + } >> +} >> + >> static int get_boot_index(int entries) >> { >> - return 0; /* Implemented next patch */ >> + int boot_index; >> + bool retry = false; >> + char tmp[5]; >> + >> + do { >> + boot_menu_prompt(retry); >> + boot_index = get_index(); >> + retry = true; >> + } while (boot_index < 0 || boot_index >= entries); >> + >> + sclp_print("\nBooting entry #"); >> + sclp_print(itostr(boot_index, tmp, sizeof(tmp))); >> + >> + return boot_index; >> } >> >> static void zipl_println(const char *data, size_t len) >> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h >> index 25d4d21..df4bc88 100644 >> --- a/pc-bios/s390-ccw/s390-ccw.h >> +++ b/pc-bios/s390-ccw/s390-ccw.h >> @@ -71,6 +71,7 @@ unsigned int get_loadparm_index(void); >> void sclp_print(const char *string); >> void sclp_setup(void); >> void sclp_get_loadparm_ascii(char *loadparm); >> +void sclp_read(char *str, size_t len); >> >> /* virtio.c */ >> unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2, >> @@ -79,6 +80,7 @@ bool virtio_is_supported(SubChannelId schid); >> void virtio_blk_setup_device(SubChannelId schid); >> int virtio_read(ulong sector, void *load_addr); >> int enable_mss_facility(void); >> +u64 get_clock(void); >> ulong get_second(void); >> >> /* bootmap.c */ >> diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c >> index 486fce1..5e4a78b 100644 >> --- a/pc-bios/s390-ccw/sclp.c >> +++ b/pc-bios/s390-ccw/sclp.c >> @@ -101,3 +101,23 @@ void sclp_get_loadparm_ascii(char *loadparm) >> ebcdic_to_ascii((char *) sccb->loadparm, loadparm, 8); >> } >> } >> + >> +void sclp_read(char *str, size_t len) >> +{ >> + ReadEventData *sccb = (void *)_sccb; >> + char *buf = (char *)(&sccb->ebh) + 7; >> + >> + /* Len should not exceed the maximum size of the event buffer */ >> + if (len > SCCB_SIZE - 8) { >> + len = SCCB_SIZE - 8; >> + } >> + >> + sccb->h.length = SCCB_SIZE; >> + sccb->h.function_code = SCLP_UNCONDITIONAL_READ; >> + sccb->ebh.length = sizeof(EventBufferHeader); >> + sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA; >> + sccb->ebh.flags = 0; >> + >> + sclp_service_call(SCLP_CMD_READ_EVENT_DATA, sccb); >> + memcpy(str, buf, len); >> +} >> diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c >> index c890a03..817e7f5 100644 >> --- a/pc-bios/s390-ccw/virtio.c >> +++ b/pc-bios/s390-ccw/virtio.c >> @@ -176,7 +176,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags) >> } >> } >> >> -static u64 get_clock(void) >> +u64 get_clock(void) >> { >> u64 r; >> >> >
On 29.01.2018 15:38, Collin L. Walling wrote: > On 01/29/2018 08:08 AM, David Hildenbrand wrote: >> On 23.01.2018 19:26, Collin L. Walling wrote: >>> Implements an sclp_read function to capture input from the >>> console and a wrapper function that handles parsing certain >>> characters and adding input to a buffer. The input is checked >>> for any erroneous values and is handled appropriately. >>> >>> A prompt will persist until input is entered or the timeout >>> expires (if one was set). Example: >>> >>> Please choose (default will boot in 10 seconds): >>> >>> Correct input will boot the respective boot index. If the >>> user's input is empty, 0, or if the timeout expires, then >>> the default zipl entry will be chosen. If the input is >>> within the range of available boot entries, then the >>> selection will be booted. Any erroneous input will cancel >>> the timeout and re-prompt the user. >>> >>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >>> --- >> Also, a very nasty thing to take care of is the following: >> >> SCLP and ckc interrupt at the same time pending. >> >> -> We only dequeue one, the other remains pending and is presented to >> the guest >> > > If I understand the assembler correctly, consume_sclp_int() takes care > of enabling / > disabling the service interrupts for us. > > However, I /do/like the refactoring suggestion you made in a previous > reply. It makes > things easier to read. > > If it makes sense to do so (such that the refactoring doesn't end up > taking me down a > rabbit hole) I'll spin up another patch that refactors the enabling / > disabling of > the service interrupt. > The problem I am mentioning is unrelated to the current code / refactoring. If we have - 1 service IRQ - 1 ckc IRQ pending at the same time (which is now possible), consume_sclp_int() will only dequeue exactly __one__ of them. And as the CKC IRQ have higher priority, they will get dequeued, leaving the (bad) service IRQ queued. This is something you will have to take care of - unrelated to the the requested refactoring.
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c index 174285e..24d4bba 100644 --- a/pc-bios/s390-ccw/menu.c +++ b/pc-bios/s390-ccw/menu.c @@ -12,16 +12,162 @@ #include "menu.h" #include "s390-ccw.h" -static uint8_t flags; -static uint64_t timeout; +#define KEYCODE_NO_INP '\0' +#define KEYCODE_ESCAPE '\033' +#define KEYCODE_BACKSP '\177' +#define KEYCODE_ENTER '\r' /* Offsets from zipl fields to zipl banner start */ #define ZIPL_TIMEOUT_OFFSET 138 #define ZIPL_FLAG_OFFSET 140 +#define TOD_CLOCK_SECOND 0xf4240000 + +static uint8_t flags; +static uint64_t timeout; + +static inline void enable_clock_int(void) +{ + uint64_t tmp = 0; + + asm volatile( + "stctg 0,0,%0\n" + "oi 6+%0, 0x8\n" + "lctlg 0,0,%0" + : : "Q" (tmp) : "memory" + ); +} + +static inline void disable_clock_int(void) +{ + uint64_t tmp = 0; + + asm volatile( + "stctg 0,0,%0\n" + "ni 6+%0, 0xf7\n" + "lctlg 0,0,%0" + : : "Q" (tmp) : "memory" + ); +} + +static inline void set_clock_comparator(uint64_t time) +{ + asm volatile("sckc %0" : : "Q" (time)); +} + +static inline bool check_clock_int(void) +{ + uint16_t *code = (uint16_t *)0x86; /* low-core external interrupt code */ + + consume_sclp_int(); + + return *code == 0x1004; +} + +static int read_prompt(char *buf, size_t len) +{ + char inp[2] = {}; + uint8_t idx = 0; + uint64_t time; + + if (timeout) { + time = get_clock() + (timeout * TOD_CLOCK_SECOND); + set_clock_comparator(time); + enable_clock_int(); + timeout = 0; + } + + while (!check_clock_int()) { + + /* Process only one character at a time */ + sclp_read(inp, 1); + + switch (inp[0]) { + case KEYCODE_NO_INP: + case KEYCODE_ESCAPE: + continue; + case KEYCODE_BACKSP: + if (idx > 0) { + buf[--idx] = 0; + sclp_print("\b \b"); + } + continue; + case KEYCODE_ENTER: + disable_clock_int(); + return idx; + } + + /* Echo input and add to buffer */ + if (idx < len) { + buf[idx] = inp[0]; + sclp_print(inp); + idx++; + } + } + + disable_clock_int(); + *buf = 0; + + return 0; +} + +static int get_index(void) +{ + char buf[10]; + int len; + int i; + + memset(buf, 0, sizeof(buf)); + + len = read_prompt(buf, sizeof(buf)); + + /* If no input, boot default */ + if (len == 0) { + return 0; + } + + /* Check for erroneous input */ + for (i = 0; i < len; i++) { + if (!isdigit(buf[i])) { + return -1; + } + } + + return atoi(buf); +} + +static void boot_menu_prompt(bool retry) +{ + char tmp[6]; + + if (retry) { + sclp_print("\nError: undefined configuration" + "\nPlease choose:\n"); + } else if (timeout > 0) { + sclp_print("Please choose (default will boot in "); + sclp_print(itostr(timeout, tmp, sizeof(tmp))); + sclp_print(" seconds):\n"); + } else { + sclp_print("Please choose:\n"); + } +} + static int get_boot_index(int entries) { - return 0; /* Implemented next patch */ + int boot_index; + bool retry = false; + char tmp[5]; + + do { + boot_menu_prompt(retry); + boot_index = get_index(); + retry = true; + } while (boot_index < 0 || boot_index >= entries); + + sclp_print("\nBooting entry #"); + sclp_print(itostr(boot_index, tmp, sizeof(tmp))); + + return boot_index; } static void zipl_println(const char *data, size_t len) diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index 25d4d21..df4bc88 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -71,6 +71,7 @@ unsigned int get_loadparm_index(void); void sclp_print(const char *string); void sclp_setup(void); void sclp_get_loadparm_ascii(char *loadparm); +void sclp_read(char *str, size_t len); /* virtio.c */ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2, @@ -79,6 +80,7 @@ bool virtio_is_supported(SubChannelId schid); void virtio_blk_setup_device(SubChannelId schid); int virtio_read(ulong sector, void *load_addr); int enable_mss_facility(void); +u64 get_clock(void); ulong get_second(void); /* bootmap.c */ diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c index 486fce1..5e4a78b 100644 --- a/pc-bios/s390-ccw/sclp.c +++ b/pc-bios/s390-ccw/sclp.c @@ -101,3 +101,23 @@ void sclp_get_loadparm_ascii(char *loadparm) ebcdic_to_ascii((char *) sccb->loadparm, loadparm, 8); } } + +void sclp_read(char *str, size_t len) +{ + ReadEventData *sccb = (void *)_sccb; + char *buf = (char *)(&sccb->ebh) + 7; + + /* Len should not exceed the maximum size of the event buffer */ + if (len > SCCB_SIZE - 8) { + len = SCCB_SIZE - 8; + } + + sccb->h.length = SCCB_SIZE; + sccb->h.function_code = SCLP_UNCONDITIONAL_READ; + sccb->ebh.length = sizeof(EventBufferHeader); + sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA; + sccb->ebh.flags = 0; + + sclp_service_call(SCLP_CMD_READ_EVENT_DATA, sccb); + memcpy(str, buf, len); +} diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c index c890a03..817e7f5 100644 --- a/pc-bios/s390-ccw/virtio.c +++ b/pc-bios/s390-ccw/virtio.c @@ -176,7 +176,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags) } } -static u64 get_clock(void) +u64 get_clock(void) { u64 r;
Implements an sclp_read function to capture input from the console and a wrapper function that handles parsing certain characters and adding input to a buffer. The input is checked for any erroneous values and is handled appropriately. A prompt will persist until input is entered or the timeout expires (if one was set). Example: Please choose (default will boot in 10 seconds): Correct input will boot the respective boot index. If the user's input is empty, 0, or if the timeout expires, then the default zipl entry will be chosen. If the input is within the range of available boot entries, then the selection will be booted. Any erroneous input will cancel the timeout and re-prompt the user. Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> --- pc-bios/s390-ccw/menu.c | 152 +++++++++++++++++++++++++++++++++++++++++++- pc-bios/s390-ccw/s390-ccw.h | 2 + pc-bios/s390-ccw/sclp.c | 20 ++++++ pc-bios/s390-ccw/virtio.c | 2 +- 4 files changed, 172 insertions(+), 4 deletions(-)