[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [qemu-s390x] [PATCH v1 4/5] s390-ccw: interactive boot

From: Collin L. Walling
Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH v1 4/5] s390-ccw: interactive boot menu for eckd dasd
Date: Tue, 28 Nov 2017 11:31:23 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 11/28/2017 07:36 AM, Thomas Huth wrote:
On 27.11.2017 21:55, Collin L. Walling wrote:
+static void read_stage2(block_number_t s1b_block_nr, void **stage2_data)
+    block_number_t s2_block_nr;
+    BootEckdStage1b *s1b = (void *)sec;
+    int i;
+    /* Get Stage1b data */
+    memset(sec, FREE_SPACE_FILLER, sizeof(sec));
+    read_block(s1b_block_nr, s1b, "Cannot read stage1b boot loader.");
+    /* Get Stage2 data */
+    *stage2_data = (void *)s2_area;
I think you could also simply "return s2_area" at the end of the
function instead of using a pointer-to-a-pointer as parameter of the

Ideally I'd like to figure out some way to get rid of the s2_area and
just read stage2 block-by-block (that way we're only using sec).

I'll play around with things here after I've taken care of the other


+#define KEYCODE_BACKSP '\177'
+#define KEYCODE_ENTER  '\r'
  long write(int fd, const void *str, size_t len);
static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
@@ -101,3 +106,130 @@ void sclp_get_loadparm_ascii(char *loadparm)
          ebcdic_to_ascii((char *) sccb->loadparm, loadparm, 8);
+static void read(char **str)
Please avoid using names from the standard C-library, unless the
function is supposed to do the same (as the write() function here).

Thinking about it, I'm not sure if it would make sense for us to have a
C-like read() function... perhaps it'd be best to rename it for now.

+    );
+static inline bool check_clock_int(void)
+    uint16_t code;
+    consume_sclp_int();
+    asm volatile(
+        "lh         1, 0x86(0,0)\n"
+        "sth        1, %0"
+        : "=r" (code)
+    );
That way, you need r1 in the clobber list. But why don't you simply use
only "lh %0,0x86(0,0)\n" instead? Or simply do it without assembly:

  code = *(volatile uint16_t *)0x86;

This is new to me... thanks for showing me this :)


Thanks for the review, Thomas.I've noted all other suggestions.

- Collin L Walling

reply via email to

[Prev in Thread] Current Thread [Next in Thread]