Re: [qemu-s390x] [PATCH v5 15/15] s390-bios: Support booting from real dasd device

[Thomas Huth]

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