Re: [Xen-devel] [PATCH v5 05/12] x86: provide executable fixmap facility

[Wei Liu <wl@xxxxxxx>]

On Fri, Jan 31, 2020 at 02:19:16PM +0100, Jan Beulich wrote:
> On 31.01.2020 14:12, Wei Liu wrote:
> > On Thu, Jan 30, 2020 at 12:30:47AM +0000, Wei Liu wrote:
> >> On Wed, Jan 29, 2020 at 08:20:27PM +0000, Wei Liu wrote:
> >>>  
> >>> +void __set_fixmap_x(
> >>> +    enum fixed_addresses_x idx, unsigned long mfn, unsigned long flags)
> >>> +{
> >>> +    BUG_ON(idx >= __end_of_fixed_addresses_x || idx <= FIX_X_RESERVED);
> >>> +    map_pages_to_xen(__fix_x_to_virt(idx), _mfn(mfn), 1, flags);
> >>> +
> >>> +    /* Generate a symbol to be used in linker script */
> >>> +    asm ( ".equ FIXADDR_X_SIZE, %c0; .global FIXADDR_X_SIZE"
> >>> +          :: "i" (__end_of_fixed_addresses_x << PAGE_SHIFT) );
> >>
> >> The (__end << SHIFT) part can be replaced with FIXADDR_X_SIZE (the macro
> >> defined in fixmap.h) directly.
> > 
> > I also discover that this snippet to generate symbol should be moved
> > else where because if Hyper-V support is compiled out, this function has
> > no user. That causes it to be DCE'd. FIXADDR_X_SIZE would be gone and
> > linking would fail.
> > 
> > I have moved this to arch_init_memory. Its storage will be reclaimed
> > during runtime, but this symbol is not used anywhere else in code, so we
> > should be fine.
> 
> Storage? This is a constant, i.e. just a symbol without any

"Its" meaning arch_init_memory, but I wasn't clear. Sorry.

> storage. Therefore an __init function is a very good place to
> put it. It could also live outside of any function, if only
> file scope asm()-s finally permitted [certain] inputs.

Tried putting in in file scope, didn't work.

Wei.

> 
> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel