Re: [qemu-s390x] [PATCH v6 10/12] s390-ccw: read user input for boot index via the SCLP console

[David Hildenbrand]

On 15.02.2018 23:54, 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 <address@hidden>
> Reviewed-by: Thomas Huth <address@hidden>
> ---
>  pc-bios/s390-ccw/menu.c     | 146 
> +++++++++++++++++++++++++++++++++++++++++++-
>  pc-bios/s390-ccw/s390-ccw.h |   2 +
>  pc-bios/s390-ccw/sclp.c     |  19 ++++++
>  pc-bios/s390-ccw/virtio.c   |   2 +-
>  4 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
> index 9631ac0..9601043 100644
> --- a/pc-bios/s390-ccw/menu.c
> +++ b/pc-bios/s390-ccw/menu.c
> @@ -12,16 +12,160 @@
>  #include "menu.h"
>  #include "s390-ccw.h"
>  
> +#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_MILLISECOND   0x3e8000
> +
>  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_MILLISECOND;
> +        set_clock_comparator(time);
> +        enable_clock_int();
> +        timeout = 0;
> +    }
> +
> +    while (!check_clock_int()) {
> +
> +        sclp_read(inp, 1); /* Process only one character at a time */
> +
> +        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;
> +        default:
> +            /* Echo input and add to buffer */
> +            if (idx < len) {
> +                buf[idx++] = inp[0];
> +                sclp_print(inp);
> +            }
> +        }
> +    }
> +
> +    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 atoui(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(uitoa(timeout / 1000, 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);
> +

Can you add a comment like

/*
 * We might still have pending SERVICE interrupts. Print something to
 * definitely clear it.
 */

> +    sclp_print("\nBooting entry #");
> +    sclp_print(uitoa(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..a7e6253 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);
> +int sclp_read(char *str, size_t count);
>  
>  /* 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 90d1bc3..abce139 100644
> --- a/pc-bios/s390-ccw/sclp.c
> +++ b/pc-bios/s390-ccw/sclp.c
> @@ -127,3 +127,22 @@ void sclp_get_loadparm_ascii(char *loadparm)
>          ebcdic_to_ascii((char *) sccb->loadparm, loadparm, 8);
>      }
>  }
> +
> +int sclp_read(char *str, size_t count)
> +{
> +    ReadEventData *sccb = (void *)_sccb;
> +    char *buf = (char *)(&sccb->ebh) + 7;
> +
> +    /* If count exceeds max buffer size, then restrict it to the max size */
> +    if (count > SCCB_SIZE - 8) {
> +        count = SCCB_SIZE - 8;
> +    }
> +
> +    sccb->h.length = SCCB_SIZE;
> +    sccb->h.function_code = SCLP_UNCONDITIONAL_READ;
> +
> +    sclp_service_call(SCLP_CMD_READ_EVENT_DATA, sccb);
> +    memcpy(str, buf, count);
> +
> +    return sccb->ebh.length - 7;
> +}
> 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;
>  
> 


-- 

Thanks,

David / dhildenb