Re: [RFC 0/5] Reuse 32 bit C code more safely

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.09.2024 17:59, Andrew Cooper wrote:
> On 04/09/2024 3:56 pm, Frediano Ziglio wrote:
>> This RFC series attempt to:
>> - use more C code, that is replace some assembly code with C;
>> - avoid some code duplication between C and assembly;
>> - prevent some issues having relocations in C code.
>>
>> The idea is extending the current C to binary code conversion
>> done for 32 bit C code called from head.S making sure relocations
>> are safe and allowing external symbols usage from C code.
>>
>> Note that, as an addition, scripts generating code check for no
>> data to allow code and data separation.
>>
>> More details of the implementation are in commit message 2/5,
>> which is the largest patch.
>> Patch 1/5 is to prepare code and avoid data.
>> Patch 3/5 is an example of code reuse between 32 and 64 bit.
>> Patch 4/5 is also another example of code reuse but is more hacky and
>> dirty due to not being possible include use some headers.
>>
>> Code boot successfully using:
>> - BIOS boot;
>> - EFI boot with Grub2 and ELF file;
>> - direct EFI boot without Grub.
>>
>> Suggestions/opinions are welcome.
>>
>> Code is currently based on "staging" branch, currently commit
>> 6471badeeec92db1cb8155066551f7509cd82efd.
> 
> I fully support taking logic out of asm and writing it in C, as well as
> taking steps to dedup the EFI and non-EFI paths.  A couple of
> observations before diving into the details.
> 
> The visibility pragmas mean that you've lost the `-include xen/config.h`
> from the $(CC) invocation.  We use this to get some Xen-wide settings
> everywhere, which includes handling visibility.
> 
> The symlinks for dual builds are going to cause problems for tarball
> archives.  Instead you can encode this with make rules.  e.g.
> 
>     obj-y += foo32.o foo64.o
> 
>     %32.o: %.c
>         $(CC) -m32 ...
> 
>     %64.o: %.c
>         $(CC) -m64 ...
> 
> will build two different .o's from the same .c.  This is how XTF builds
> different tests from the same source.
> 
> 
> I'm on the fence with the ifdefary and bit suffixes.  I don't think we
> need the error case because x86_128 isn't coming along any time soon.
> 
> For completeness, there's a trick used by the shadow code (see
> SHADOW_INTERNAL_NAME()) which adds a suffix without local ifdefary. 
> It's nicer to read, but breaks grep/cscope/etc.  I'm torn as to which is
> the lesser evil.

While generally I prefer that approach shadow code takes, I think the
#ifdef-ary is acceptably bounded here. What I'm more worried by are the
fair number of #define-s for the 32-bit case of setting up page tables.

Jan