Re: [Qemu-devel] [qemu-s390x] [PATCH v5 15/15] s390-bios: Support booting from real dasd device
On 01/04/2019 17.35, Jason J. Herne wrote:
> On 3/29/19 4:33 AM, Thomas Huth wrote:
>> On 13/03/2019 17.31, Jason J. Herne wrote:
[...]
>>> +static void ipl1_fixup(void)
>>> +{
>>> + Ccw0 *ccwSeek = (Ccw0 *) 0x08;
>>> + Ccw0 *ccwSearchID = (Ccw0 *) 0x10;
>>> + Ccw0 *ccwSearchTic = (Ccw0 *) 0x18;
>>> + Ccw0 *ccwRead = (Ccw0 *) 0x20;
>>> + CcwSeekData *seekData = (CcwSeekData *) 0x30;
>>> + CcwSearchIdData *searchData = (CcwSearchIdData *) 0x38;
>>> +
>>> + /* move IPL1 CCWs to make room for CCWs needed to locate record
>>> 2 */
>>> + memcpy(ccwRead, (void *)0x08, 16);
>>
>> lowcore->ccw1 ?
>>
>
> All the other CCWs in this section still need to use the numeric memory
> references. I think it makes it slightly more confusing to switch just
> the one to using the struct. For this reason I prefer it the way it is,
> if you're okay with that.
Fine for me.
> ...
>>> +static void lpsw(void *psw_addr)
>>> +{
>>> + PSWLegacy *pswl = (PSWLegacy *) psw_addr;
>>> +
>>> + pswl->mask |= PSW_MASK_EAMODE; /* Force z-mode */
>>> + pswl->addr |= PSW_MASK_BAMODE;
>>> + asm volatile(" llgtr 0,0\n llgtr 1,1\n" /* Some OS's expect
>>> to be */
>>> + " llgtr 2,2\n llgtr 3,3\n" /* in 32-bit mode.
>>> Clear */
>>> + " llgtr 4,4\n llgtr 5,5\n" /* high part of
>>> regs to */
>>> + " llgtr 6,6\n llgtr 7,7\n" /* avoid messing
>>> up */
>>> + " llgtr 8,8\n llgtr 9,9\n" /* instructions
>>> that work */
>>> + " llgtr 10,10\n llgtr 11,11\n" /* in both
>>> addressing */
>>> + " llgtr 12,12\n llgtr 13,13\n" /* modes, like
>>> servc. */
>>> + " llgtr 14,14\n llgtr 15,15\n"
>>> + " lpsw %0\n"
>>> + : : "Q" (*pswl) : "cc");
>>> +}
>>
>> Have you tried to use jump_to_low_kernel() already? ... it might be
>> cleaner to do the diag 0x308 reset here, too, to avoid that some part of
>> the machine is in an unexpected state...
>
> I had not tried jump_to_low_kernel() until just now. It *does* seem to
> work... and eliminates the need for the manual register clearing. I
> assume the diag 308 reset baked into the jump_to_low_kernel() is
> responsible for that? I guess switching to jump_to_low_kernel() would be
> a good thing ... I'll admit I'm slightly uneasy about it since I've been
> testing my way for so long :) But jump_to_low_kernel() works for all of
> my manual test cases.
I originally used lpsw in the netboot code, too, but there was the risk
that some device activity in the background could confuse the OS kernel
later ... so Christian pointed me to the diag diag 308 reset code, which
is certainly the better way to hand over the control to the OS. Thus if
it passes your test cases, I'd say let's try to do it the better way
here right from the start and switch to jump_to_low_kernel() instead.
Thomas
Re: [Qemu-devel] [qemu-s390x] [PATCH v5 15/15] s390-bios: Support booting from real dasd device
On 3/29/19 4:33 AM, Thomas Huth wrote:
On 13/03/2019 17.31, Jason J. Herne wrote:
s/tansfers/transfers/
Will fix this, and the other typos you've pointed out.
...
+static char prefix_page[PAGE_SIZE * 2]
+__attribute__((__aligned__(PAGE_SIZE * 2)));
+
+static void enable_prefixing(void)
+{
+memcpy(_page, (void *)0, 4096);
You could use the "lowcore" variable from s390-arch.h here instead of
"(void *)0", I guess.
Agreed.
+set_prefix(ptr2u32(_page));
+}
+
+static void disable_prefixing(void)
+{
+set_prefix(0);
+/* Copy io interrupt info back to low core */
+memcpy((void *)0xB8, prefix_page + 0xB8, 12);
Maybe use >subchannel_id instead of 0xB8 ? ... not sure whether
that's nicer here, though...
I think it is nicer using the named field. I Will make that change.
Note: We need to keep the (void*) cast to prevent a compiler warning about discarding
const qualifier.
+static void ipl1_fixup(void)
+{
+Ccw0 *ccwSeek = (Ccw0 *) 0x08;
+Ccw0 *ccwSearchID = (Ccw0 *) 0x10;
+Ccw0 *ccwSearchTic = (Ccw0 *) 0x18;
+Ccw0 *ccwRead = (Ccw0 *) 0x20;
+CcwSeekData *seekData = (CcwSeekData *) 0x30;
+CcwSearchIdData *searchData = (CcwSearchIdData *) 0x38;
+
+/* move IPL1 CCWs to make room for CCWs needed to locate record 2 */
+memcpy(ccwRead, (void *)0x08, 16);
lowcore->ccw1 ?
All the other CCWs in this section still need to use the numeric memory references. I
think it makes it slightly more confusing to switch just the one to using the struct. For
this reason I prefer it the way it is, if you're okay with that.
...
+static void lpsw(void *psw_addr)
+{
+PSWLegacy *pswl = (PSWLegacy *) psw_addr;
+
+pswl->mask |= PSW_MASK_EAMODE; /* Force z-mode */
+pswl->addr |= PSW_MASK_BAMODE;
+asm volatile(" llgtr 0,0\n llgtr 1,1\n" /* Some OS's expect to be */
+ " llgtr 2,2\n llgtr 3,3\n" /* in 32-bit mode. Clear */
+ " llgtr 4,4\n llgtr 5,5\n" /* high part of regs to */
+ " llgtr 6,6\n llgtr 7,7\n" /* avoid messing up */
+ " llgtr 8,8\n llgtr 9,9\n" /* instructions that work */
+ " llgtr 10,10\n llgtr 11,11\n" /* in both addressing */
+ " llgtr 12,12\n llgtr 13,13\n" /* modes, like servc. */
+ " llgtr 14,14\n llgtr 15,15\n"
+ " lpsw %0\n"
+ : : "Q" (*pswl) : "cc");
+}
Have you tried to use jump_to_low_kernel() already? ... it might be
cleaner to do the diag 0x308 reset here, too, to avoid that some part of
the machine is in an unexpected state...
Thomas
I had not tried jump_to_low_kernel() until just now. It *does* seem to work... and
eliminates the need for the manual register clearing. I assume the diag 308 reset baked
into the jump_to_low_kernel() is responsible for that? I guess switching to
jump_to_low_kernel() would be a good thing ... I'll admit I'm slightly uneasy about it
since I've been testing my way for so long :) But jump_to_low_kernel() works for all of my
manual test cases.
--
-- Jason J. Herne (jjhe...@linux.ibm.com)
Re: [Qemu-devel] [qemu-s390x] [PATCH v5 15/15] s390-bios: Support booting from real dasd device
On 13/03/2019 17.31, Jason J. Herne wrote:
> Allows guest to boot from a vfio configured real dasd device.
>
> Signed-off-by: Jason J. Herne
> Reviewed-by: Cornelia Huck
> ---
[...]
> diff --git a/docs/devel/s390-dasd-ipl.txt b/docs/devel/s390-dasd-ipl.txt
> new file mode 100644
> index 000..236428a
> --- /dev/null
> +++ b/docs/devel/s390-dasd-ipl.txt
> @@ -0,0 +1,133 @@
> +*
> +* s390 hardware IPL *
> +*
> +
> +The s390 hardware IPL process consists of the following steps.
> +
> +1. A READ IPL ccw is constructed in memory location 0x0.
> +This ccw, by definition, reads the IPL1 record which is located on the
> disk
> +at cylinder 0 track 0 record 1. Note that the chain flag is on in this
> ccw
> +so when it is complete another ccw will be fetched and executed from
> memory
> +location 0x08.
> +
> +2. Execute the Read IPL ccw at 0x00, thereby reading IPL1 data into 0x00.
> +IPL1 data is 24 bytes in length and consists of the following pieces of
> +information: [psw][read ccw][tic ccw]. When the machine executes the Read
> +IPL ccw it read the 24-bytes of IPL1 to be read into memory starting at
> +location 0x0. Then the ccw program at 0x08 which consists of a read
> +ccw and a tic ccw is automatically executed because of the chain flag
> from
> +the original READ IPL ccw. The read ccw will read the IPL2 data into
> memory
> +and the TIC (Tranfer In Channel) will transfer control to the channel
s/Tranfer/Transfer/ ?
[...]
> +**
> +* How this all pertains to QEMU (and the kernel) *
> +**
> +
> +In theory we should merely have to do the following to IPL/boot a guest
> +operating system from a DASD device:
> +
> +1. Place a "Read IPL" ccw into memory location 0x0 with chaining bit on.
> +2. Execute channel program at 0x0.
> +3. LPSW 0x0.
> +
> +However, our emulation of the machine's channel program logic within the
> kernel
> +is missing one key feature that is required for this process to work:
> +non-prefetch of ccw data.
> +
> +When we start a channel program we pass the channel subsystem parameters via
> an
> +ORB (Operation Request Block). One of those parameters is a prefetch bit. If
> the
> +bit is on then the vfio-ccw kernel driver is allowed to read the entire
> channel
> +program from guest memory before it starts executing it. This means that any
> +channel commands that read additional channel commands will not work as
> expected
> +because the newly read commands will only exist in guest memory and NOT
> within
> +the kernel's channel subsystem memory. The kernel vfio-ccw driver currently
> +requires this bit to be on for all channel programs. This is a problem
> because
> +the IPL process consists of transferring control from the "Read IPL" ccw
> +immediately to the IPL1 channel program that was read by "Read IPL".
> +
> +Not being able to turn off prefetch will also prevent the TIC at the end of
> the
> +IPL1 channel program from transferring control to the IPL2 channel program.
> +
> +Lastly, in some cases (the zipl bootloader for example) the IPL2 program also
> +tansfers control to another channel program segment immediately after
> reading it
s/tansfers/transfers/
> +from the disk. So we need to be able to handle this case.
> +
> +**
> +* What QEMU does *
> +**
> +
> +Since we are forced to live with prefetch we cannot use the very simple IPL
> +procedure we defined in the preceding section. So we compensate by doing the
> +following.
> +
> +1. Place "Read IPL" ccw into memory location 0x0, but turn off chaining bit.
> +2. Execute "Read IPL" at 0x0.
> +
> + So now IPL1's psw is at 0x0 and IPL1's channel program is at 0x08.
> +
> +4. Write a custom channel program that will seek to the IPL2 record and then
> + execute the READ and TIC ccws from IPL1. Normamly the seek is not
> required
s/Normamly/Normally/
[...]
> diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c
> new file mode 100644
> index 000..1a44469
> --- /dev/null
> +++ b/pc-bios/s390-ccw/dasd-ipl.c
> @@ -0,0 +1,249 @@
> +/*
> + * S390 IPL (boot) from a real DASD device via vfio framework.
> + *
> + * Copyright (c) 2019 Jason J. Herne
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include "libc.h"
> +#include "s390-ccw.h"
> +#include "s390-arch.h"
> +#include "dasd-ipl.h"
> +#include "helper.h"
> +
> +static char prefix_page[PAGE_SIZE * 2]
> +__attribute__((__aligned__(PAGE_SIZE * 2)));
> +
> +static void enable_prefixing(void)
> +{
> +memcpy(_page, (void *)0, 4096);
You could use the "lowcore" variable from s390-arch.h here instead of
