Re: [PATCH v2 02/10] xen/x86: move reusable EFI stub functions from x86 to common

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.04.2022 11:07, Wei Chen wrote:
> diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub-x86.c
> similarity index 71%
> rename from xen/arch/x86/efi/stub.c
> rename to xen/arch/x86/efi/stub-x86.c
> index 9984932626..2cd5c8d4dc 100644
> --- a/xen/arch/x86/efi/stub.c
> +++ b/xen/arch/x86/efi/stub-x86.c

I'm not happy to see a file named *x86*.[ch] under x86/. I think the
x86 file wants to simply include the common one (and the symlinking
be suppressed when a real file already exists). Naming the common
file stub-common.c wouldn't help, as a similar anomaly would result.

> --- /dev/null
> +++ b/xen/common/efi/stub.c
> @@ -0,0 +1,38 @@
> +#include <xen/efi.h>
> +#include <xen/errno.h>
> +#include <xen/lib.h>
> +
> +bool efi_enabled(unsigned int feature)
> +{
> +    return false;
> +}
> +
> +bool efi_rs_using_pgtables(void)
> +{
> +    return false;
> +}
> +
> +unsigned long efi_get_time(void)
> +{
> +    BUG();
> +    return 0;
> +}
> +
> +void efi_halt_system(void) { }
> +void efi_reset_system(bool warm) { }
> +
> +int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
> +{
> +    return -ENOSYS;
> +}
> +
> +int efi_compat_get_info(uint32_t idx, union compat_pf_efi_info *)
> +    __attribute__((__alias__("efi_get_info")));

I doubt you need this outside of x86.

> +int efi_runtime_call(struct xenpf_efi_runtime_call *op)
> +{
> +    return -ENOSYS;
> +}
> +
> +int efi_compat_runtime_call(struct compat_pf_efi_runtime_call *)
> +    __attribute__((__alias__("efi_runtime_call")));

Same here.

Even for the non-compat variants the need is un-obvious: Are you
intending to wire these up anywhere in Arm or common code? This
of course is once again pointing out that such code movement would
better be done when the new consumers actually appear, such that
it's clear why the movement is done - for every individual item.

Jan