[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v5 12/12] pc-bios: s390x: Cleanup jump to ipl code
From: |
Janosch Frank |
Subject: |
Re: [RFC v5 12/12] pc-bios: s390x: Cleanup jump to ipl code |
Date: |
Fri, 26 Jun 2020 10:04:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 |
On 6/25/20 2:58 PM, Thomas Huth wrote:
> On 24/06/2020 09.52, Janosch Frank wrote:
>> jump_to_IPL_code takes a 64 bit address, masks it with the short psw
>> address mask and later branches to it using a full 64 bit register.
>>
>> * As the masking is not necessary, let's remove it
>> * Without the mask we can save the ipl address to a static 64 bit
>> function ptr as we later branch to it
>> * Let's also clean up the variable names and remove the now unneeded
>> ResetInfo
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> pc-bios/s390-ccw/jump2ipl.c | 27 +++++++++++----------------
>> 1 file changed, 11 insertions(+), 16 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>> index 767012bf0c..aef37cea76 100644
>> --- a/pc-bios/s390-ccw/jump2ipl.c
>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>> @@ -13,20 +13,15 @@
>> #define KERN_IMAGE_START 0x010000UL
>> #define RESET_PSW_MASK (PSW_MASK_SHORTPSW | PSW_MASK_64)
>>
>> -typedef struct ResetInfo {
>> - uint64_t ipl_psw;
>> - uint32_t ipl_continue;
>> -} ResetInfo;
>> -
>> -static ResetInfo save;
>> +static void (*ipl_continue)(void);
>> +static uint64_t psw_save;
>
> I wonder whether there was a reason for having ipl_continue in the
> lowcore instead of using a simple static function pointer... Christian,
> do you remember?
>
>> static void jump_to_IPL_2(void)
>> {
>> - ResetInfo *current = 0;
>> + uint64_t *psw_current = 0;
>
> Mhh, what about using uint64_t *psw_current = (uint64_t *)lowcore
> instead, to make it more more explicit?
Sure, that would make it way better to read.
>
>> - void (*ipl)(void) = (void *) (uint64_t) current->ipl_continue;
>> - *current = save;
>> - ipl(); /* should not return */
>> + *psw_current = psw_save;
>> + ipl_continue(); /* should not return */
>> }
>>
>> void jump_to_IPL_code(uint64_t address)
>> @@ -46,15 +41,15 @@ void jump_to_IPL_code(uint64_t address)
>> * content of non-BIOS memory after we loaded the guest, so we
>> * save the original content and restore it in jump_to_IPL_2.
>> */
>> - ResetInfo *current = 0;
>> + uint64_t *psw_current = 0;
>
> dito.
>
>> - save = *current;
>> + psw_save = *psw_current;
>>
>> - current->ipl_psw = (uint64_t) &jump_to_IPL_2;
>> - current->ipl_psw |= RESET_PSW_MASK;
>> - current->ipl_continue = address & PSW_MASK_SHORT_ADDR;
>> + *psw_current = (uint64_t) &jump_to_IPL_2;
>> + *psw_current |= RESET_PSW_MASK;
>> + ipl_continue = (void *)address;
>>
>> - debug_print_int("set IPL addr to", current->ipl_continue);
>> + debug_print_int("set IPL addr to", (uint64_t)ipl_continue);
>>
>> /* Ensure the guest output starts fresh */
>> sclp_print("\n");
>>
>
> The patch sounds like a good idea to me ... but since this code proofed
> to be very fragile in the past, let's wait for Christian to say whether
> there was a good reason for ipl_continue in the lowcore or not.
This is a RFC and will need a lot of testing.
I guess I'll move patch 11 and 12 of this series into a new one and also
fix some more boot related stuff so this becomes less maze like.
>
> Thomas
>
>
signature.asc
Description: OpenPGP digital signature
- [PATCH v5 05/12] pc-bios: s390x: Remove unneeded dasd-ipl.c reset psw mask changes, (continued)
- [PATCH v5 05/12] pc-bios: s390x: Remove unneeded dasd-ipl.c reset psw mask changes, Janosch Frank, 2020/06/24
- [PATCH v5 08/12] pc-bios: s390x: Move panic() into header and add infinite loop, Janosch Frank, 2020/06/24
- [PATCH v5 04/12] pc-bios: s390x: Get rid of magic offsets into the lowcore, Janosch Frank, 2020/06/24
- [PATCH v5 10/12] pc-bios: s390x: Make u32 ptr check explicit, Janosch Frank, 2020/06/24
- [RFC v5 12/12] pc-bios: s390x: Cleanup jump to ipl code, Janosch Frank, 2020/06/24
- [PATCH v5 09/12] pc-bios: s390x: Use ebcdic2ascii table, Janosch Frank, 2020/06/24
- Re: [PATCH v5 00/12] pc-bios: s390x: Cleanup part 1, no-reply, 2020/06/24
- Re: [PATCH v5 00/12] pc-bios: s390x: Cleanup part 1, Cornelia Huck, 2020/06/24