Re: Unexpected error in rme_configure_one() at ../target/arm/kvm-rme.c:159

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Thu, Jun 06, 2024 at 03:05:02PM +1000, Gavin Shan wrote:
> > This commit moves the page tables from .rodata to .data. When linking
> > IdMap.obj into ArmPlatformPrePeiCore.dll, the alignment of the .text
> > section changes from 0x1000 to 0x800. This change comes from the linker
> > script putting .rodata into .text. I don't know why the included .rodata
> > alignment affects the .text alignment, but I don't think it matters here.
> > 
> > In GenFw, ScanSections64() calculates a mCoffAlignment as the max
> > .text/.data/.hii section alignement. Since with this commit, .data
> > alignement (0x1000) becomes larger than .text (0x800), it picks 0x1000 as
> > the output text offset, and then WriteSections64() complains that this
> > offset isn't equal to the input .text alignment modulo 0x1000.
> > 
> > The linker script says:
> > 
> >    /*
> >     * The alignment of the .data section should be less than or equal to the
> >     * alignment of the .text section. This ensures that the relative offset
> >     * between these sections is the same in the ELF and the PE/COFF versions of
> >     * this binary.
> >     */
> > 
> > but that's not what we're getting. I don't have a fix yet, other than
> > forcing the .text and .data alignment to 4k.
> > 
> 
> Jean, thanks for your explanation. Right, the issue is caused by mismatched
> alignments for ELF and PE/COFF sections. I ever dumped the variables at the
> failing point, showing the mismatched alignments (0x800 vs 0x1000). Apart from
> that, the virtual address of 'text' section is aligned to 0x800 instead of
> 0x1000 after ArmPlatformPrePeiCore.dll is dumped by 'readelf'.
> 
> SecHdr->sh_addr:                    0x800              <<< Mismatched alignment between ELF and PE/COFF
> SecOffset:                          0x1000
> SymShdr->sh_addr:                   0x800
> mCoffSectionsOffset[Sym->st_shndx]: 0x1000
> GenFw: ERROR 3000: Invalid
>   WriteSections64(): /home/gavin/sandbox/CCA/edk2-guest/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore/DEBUG/ArmPlatformPrePeiCore.dll AARCH64 small code model requires identical ELF and PE/COFF section offsets modulo 4 KB.
> 
> # readelf -S Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore/DEBUG/ArmPlatformPrePeiCore.dll
> Section Headers:
>   [Nr] Name              Type             Address           Offset
>        Size              EntSize          Flags  Link  Info  Align
>   [ 0]                   NULL             0000000000000000  00000000
>        0000000000000000  0000000000000000           0     0     0
>   [ 1] .text             PROGBITS         0000000000000800  00000800       <<< Aligned to 0x800
>        00000000000051b8  0000000000000000  AX       0     0     2048
> 
> With the following changes, I'm able to build the firmware successfully. I don't
> see how COMMONPAGESIZE is sorted out because I don't find its definition in the
> source code.

It's a ld builtin, set on the command-line with "-z common-page-size=X" by
Conf/tools_def.txt, in this case I believe DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS. 

> 
> diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
> index 9f27e83bb0..5463df47a9 100644
> --- a/BaseTools/Scripts/GccBase.lds
> +++ b/BaseTools/Scripts/GccBase.lds
> @@ -20,7 +20,8 @@ SECTIONS {
>     */
>    . = PECOFF_HEADER_SIZE;
> -  .text : ALIGN(CONSTANT(COMMONPAGESIZE)) {
> +  /* .text : ALIGN(CONSTANT(COMMONPAGESIZE)) { */^M
> +  .text : ALIGN(4096) {^M

Build (after clean) fails for me if I only change the .text 
alignment, I need .data as well. So changing Conf/tools_def.txt is easier.
I'll try to find a proper fix but it will take me some time to understand
GenFw.


> > > > Note that the guest edk2 is optional and experimental, you can use direct
> > > > kernel boot to get a working demo quicker.
> > > > 
> > > 
> > > I never did this before. Could you please provide the detailed steps on this?
> > 
> > Removing the -bios parameter to QEMU should be enough. You can also add
> > 'earlycon' to -append to show early boot errors.
> > 
> 
> I didn't get a chance to try this yet since the host can't be brought up now.
> I will try this later. I originally thought some sort of boot wrapper is needed
> so that the kernel image has the capability to boot itself. For example, Mark
> Rutland's boot wrapper [1] can be leveraged in this case. I don't think Image has
> the capability to boot itself, right?

Yes QEMU can set up everything so that the Image boots on its own. What
the boot-wrapper does is minimal hardware initialization, handling PSCI
calls and passing the DTB pointer in x0. But that's only needed when using
the Arm FastModel (boot-wrapper is a lightweight firmware specifically for
the FastModel). QEMU can do all that itself so you can boot a kernel
without any firmware.

Using edk2 in the Realm guest will be needed for example to boot a distro
image which contains the kernel, but direct kernel boot is useful both for
prototyping and real-life use cases like confidential containers and some
cloud VMs.

Thanks,
Jean

> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git
> 
> Thanks,
> Gavin
>