Re: [PATCH v5 3/4] xen/ppc: Implement early serial printk on pseries

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.07.2023 17:42, Shawn Anastasio wrote:
> On 7/26/23 10:32 AM, Jan Beulich wrote:
>> On 24.07.2023 17:06, Shawn Anastasio wrote:
>>> On 7/24/23 7:40 AM, Jan Beulich wrote:
>>>> On 21.07.2023 19:02, Shawn Anastasio wrote:
>>>>> On typical Power VMs (e.g. QEMU's -M pseries), a variety of services
>>>>> including an early serial console are provided by Open Firmware.
>>>>> Implement the required interfaces to call into Open Firmware and write
>>>>> to the serial console.
>>>>>
>>>>> Since Open Firmware runs in 32-bit Big Endian mode and Xen runs in
>>>>> 64-bit Little Endian mode, a thunk is required to save/restore
>>>>> any potentially-clobbered registers as well as to perform the
>>>>> required endianness switch. Thankfully, linux already has such
>>>>> a routine, which was imported into ppc64/of-call.S.
>>>>>
>>>>> Support for bare metal (PowerNV) will be implemented in a future
>>>>> patch.
>>>>>
>>>>> Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
>>>>
>>>> While I've committed the earlier two patches, I had to back out this
>>>> one. In my environment (gcc13) the build fails due an unresolved
>>>> reference to memset() out of boot-of.c (supposedly from of_call()).
>>>
>>> Does removing the `{ 0 }` initializer to `struct of_service s` on line
>>> 43 resolve this?
>>
>> Yes, that's what's causing the call (and removing, whether or not correct,
>> helps).
> 
> Thanks for confirming. Removing it should be fine since the code
> manually initializes all of the other fields of the struct. The only
> behavioral difference is that the members of `ofs_args` at indices >=
> nargs would be left uninitialized. This shouldn't be an issue though
> since we're guarding reads of the array on `nargs` and `nrets` and thus
> only read explicitly initialized values (and of course, firmware would
> do the same).
> 
> Naturally we can't avoid memset calls forever. I have lib/ building
> locally, but if we could get this series in without having to make those
> changes here that'd be great.

Are you suggesting I should put in this patch almost as is, with just
that initializer dropped?

Jan