Re: [PATCH v5 00/12] pc-bios: s390x: Cleanup part 1

[Janosch Frank]

On 6/24/20 12:46 PM, Thomas Huth wrote:
> On 24/06/2020 12.44, Cornelia Huck wrote:
>> On Wed, 24 Jun 2020 03:52:14 -0400
>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>
>>> The bios is in dire need for a cleanup as there are still a lot of
>>> magic constants being used throughout as well as duplicated code.
>>>
>>> In the first part of this series we consolidate constants and
>>> functions, as well as doing some minor cleanups and fixes.
>>>
>>> The patches are available here:
>>> https://github.com/frankjaa/qemu/pull/new/cleanup_bios
>>>
>>> v5:
>>>     * Fixed whitespace damage
>>>     * Removed reset PSW mask changes in dasd-ipl.c
>>>     * Added jump2ipl.c cleanup patches
>>>
>>> v4:
>>>     * Renamed time.h to s390-time.h
>>>     * Fixed function names in sleep()
>>>     * Changed order of sense_id_ccw initialization
>>>     * Added missing include before sleep()
>>>
>>> v3:
>>>     * Dropped 0x00 to 0x0/0 patch
>>>     * Moved some timing functions into helper.h instead of time.h
>>>     * Fixed IPL psw manipulation in dasd-ipl.c
>>>     * Minor cosmetic fixes found by review
>>>
>>> v2:
>>>     * Included cio fixup to get rid of compile errors...
>>>     * Minor cosmetic fixes found by review
>>>
>>>
>>> Janosch Frank (12):
>>>    pc-bios: s390x: cio.c cleanup and compile fix
>>>    pc-bios: s390x: Consolidate timing functions into time.h
>>>    pc-bios: s390x: Move sleep and yield to helper.h
>>>    pc-bios: s390x: Get rid of magic offsets into the lowcore
>>>    pc-bios: s390x: Remove unneeded dasd-ipl.c reset psw mask changes
>>>    pc-bios: s390x: Rename PSW_MASK_ZMODE to PSW_MASK_64
>>>    pc-bios: s390x: Use PSW masks where possible and introduce
>>>      PSW_MASK_SHORT_ADDR
>>>    pc-bios: s390x: Move panic() into header and add infinite loop
>>>    pc-bios: s390x: Use ebcdic2ascii table
>>>    pc-bios: s390x: Make u32 ptr check explicit
>>>    pc-bios: s390x: Fix bootmap.c passing PSWs as addresses
>>>    pc-bios: s390x: Cleanup jump to ipl code
>>>
>>>   pc-bios/s390-ccw/bootmap.c     |  9 ++++----
>>>   pc-bios/s390-ccw/bootmap.h     |  2 +-
>>>   pc-bios/s390-ccw/cio.c         | 40 +++++++++++++++++++---------------
>>>   pc-bios/s390-ccw/cio.h         | 17 ++++++++++-----
>>>   pc-bios/s390-ccw/dasd-ipl.c    |  3 ---
>>>   pc-bios/s390-ccw/helper.h      | 19 +++++++++++++++-
>>>   pc-bios/s390-ccw/jump2ipl.c    | 35 ++++++++++++-----------------
>>>   pc-bios/s390-ccw/main.c        | 15 +++----------
>>>   pc-bios/s390-ccw/menu.c        |  1 +
>>>   pc-bios/s390-ccw/netmain.c     | 23 +++----------------
>>>   pc-bios/s390-ccw/s390-arch.h   |  4 +++-
>>>   pc-bios/s390-ccw/s390-ccw.h    | 27 ++++++-----------------
>>>   pc-bios/s390-ccw/s390-time.h   | 23 +++++++++++++++++++
>>>   pc-bios/s390-ccw/start.S       |  5 +++--
>>>   pc-bios/s390-ccw/virtio-net.c  |  2 ++
>>>   pc-bios/s390-ccw/virtio-scsi.c |  2 ++
>>>   pc-bios/s390-ccw/virtio.c      | 18 +++------------
>>>   17 files changed, 120 insertions(+), 125 deletions(-)
>>>   create mode 100644 pc-bios/s390-ccw/s390-time.h
>>>
>>
>> Hm... what's the general status of this? Most of the patches have at
>> least one R-b/A-b already, I see.
>>
>> Do the s390-ccw boot maintainers want to pick this (once the rest has
>> been looked at) and then send me a pull req, or should I pick it when
>> it is good to go? Softfreeze is less than two weeks away :)
> 
> I'd like to review the missing parts and run my tests with the patches 
> applied ... I'm just a little bit swamped right now, so please give me 
> some more time...
> 
>   Thomas

I'd suggest we hold off on the RFC patch since it makes testing all of
the boot methods necessary and I was only able to test direct, dasd
passthrough and virtio-ccw up to now. Time to build a testing environment...

I can move that patch into the next series for further discussion. It
was mostly added to prove the 64 bit PSW mask removal in patch #5.

signature.asc
Description: OpenPGP digital signature