Re: [PATCH v2 2/5] x86: don't maintain compat M2P when !PV32

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/08/2020 13:34, Jan Beulich wrote:
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -34,17 +34,31 @@ EMIT_FILE;
>  #include <asm/fixmap.h>
>  #include <asm/hypercall.h>
>  #include <asm/msr.h>
> +#include <asm/pv/domain.h>
>  #include <asm/setup.h>
>  #include <asm/numa.h>
>  #include <asm/mem_paging.h>
>  #include <asm/mem_sharing.h>
>  #include <public/memory.h>
>  
> +#ifdef CONFIG_PV32
> +
>  #define compat_machine_to_phys_mapping ((unsigned int 
> *)RDWR_COMPAT_MPT_VIRT_START)
>  
> -unsigned int __read_mostly m2p_compat_vstart = 
> __HYPERVISOR_COMPAT_VIRT_START;
> +unsigned int __initdata m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START;
> +
> +l2_pgentry_t *__read_mostly compat_idle_pg_table_l2;
> +
> +#else /* !CONFIG_PV32 */
> +
> +/*
> + * Declare the symbol such that (dead) code referencing it can be built
> + * without a lot of #ifdef-ary, but mark it fully const and don't define
> + * this symbol anywhere (relying on DCE by the compiler).
> + */
> +extern const unsigned int *const compat_machine_to_phys_mapping;

This has a different indirection.  I know it is for DCE, but it still
ought to match.

I'm also not convinced that asymmetric const is a good idea.  All it
will do is confuse people, because now the "failed to DCE" error will be
a compiler error for writing to a read-only array, not a link time error
like every other instance of failed DCE.

Therefore, it ought to be:

extern unsigned int compat_machine_to_phys_mapping[];

Otherwise, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I'm disappointed that HYPERVISOR_COMPAT_VIRT_START() hasn't disappeared,
and instead sprouted a nop wrapper, but I guess it can wait for
subsequent cleanup.

~Andrew