Message ID | 20200129202034.15052-6-liuwe@microsoft.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | More Hyper-V infrastructures | expand |
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. Wei.
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. Wei.
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 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. Jan
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
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S index 1cbf5acdfb..314a32a19f 100644 --- a/xen/arch/x86/boot/x86_64.S +++ b/xen/arch/x86/boot/x86_64.S @@ -81,11 +81,20 @@ GLOBAL(l2_directmap) .size l2_directmap, . - l2_directmap /* - * L2 mapping the Xen text/data/bss region, constructed dynamically. Uses 1x - * 4k page. + * L2 mapping the Xen text/data/bss region, constructed dynamically. + * Executable fixmap is hooked up statically. + * Uses 1x 4k page. */ GLOBAL(l2_xenmap) - .fill L2_PAGETABLE_ENTRIES, 8, 0 + idx = 0 + .rept L2_PAGETABLE_ENTRIES + .if idx == l2_table_offset(FIXADDR_X_TOP - 1) + .quad sym_offs(l1_fixmap_x) + __PAGE_HYPERVISOR + .else + .quad 0 + .endif + idx = idx + 1 + .endr .size l2_xenmap, . - l2_xenmap /* L2 mapping the fixmap. Uses 1x 4k page. */ diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index 2749cbc5cf..513b0f3841 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -12,6 +12,7 @@ #include <xen/livepatch.h> #include <xen/sched.h> +#include <asm/fixmap.h> #include <asm/nmi.h> #include <asm/livepatch.h> @@ -311,7 +312,7 @@ void __init arch_livepatch_init(void) void *start, *end; start = (void *)xen_virt_end; - end = (void *)(XEN_VIRT_END - NR_CPUS * PAGE_SIZE); + end = (void *)(XEN_VIRT_END - FIXADDR_X_SIZE - NR_CPUS * PAGE_SIZE); BUG_ON(end <= start); diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index f50c065af3..44abde24b2 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -157,6 +157,8 @@ /* Mapping of the fixmap space needed early. */ l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) l1_fixmap[L1_PAGETABLE_ENTRIES]; +l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) + l1_fixmap_x[L1_PAGETABLE_ENTRIES]; paddr_t __read_mostly mem_hotplug; @@ -5718,10 +5720,21 @@ int destroy_xen_mappings(unsigned long s, unsigned long e) void __set_fixmap( enum fixed_addresses idx, unsigned long mfn, unsigned long flags) { - BUG_ON(idx >= __end_of_fixed_addresses); + BUG_ON(idx >= __end_of_fixed_addresses || idx <= FIX_RESERVED); map_pages_to_xen(__fix_to_virt(idx), _mfn(mfn), 1, flags); } +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) ); +} + void *__init arch_vmap_virt_end(void) { return fix_to_virt(__end_of_fixed_addresses); diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index 93b86a09e9..e83e4564a4 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -644,7 +644,7 @@ unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn) unmap_domain_page(memset(__map_domain_page(pg), 0xcc, PAGE_SIZE)); } - stub_va = XEN_VIRT_END - (cpu + 1) * PAGE_SIZE; + stub_va = XEN_VIRT_END - FIXADDR_X_SIZE - (cpu + 1) * PAGE_SIZE; if ( map_pages_to_xen(stub_va, page_to_mfn(pg), 1, PAGE_HYPERVISOR_RX | MAP_SMALL_PAGES) ) { diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 07c6448dbb..97f9c07891 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -3,6 +3,8 @@ #include <xen/cache.h> #include <xen/lib.h> + +#include <asm/fixmap.h> #include <asm/page.h> #undef ENTRY #undef ALIGN @@ -353,6 +355,7 @@ SECTIONS } ASSERT(__2M_rwdata_end <= XEN_VIRT_END - XEN_VIRT_START + __XEN_VIRT_START - + FIXADDR_X_SIZE - DIV_ROUND_UP(NR_CPUS, STUBS_PER_PAGE) * PAGE_SIZE, "Xen image overlaps stubs area") diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h index d0cfbb70a8..a34053c4c0 100644 --- a/xen/include/asm-x86/config.h +++ b/xen/include/asm-x86/config.h @@ -218,7 +218,7 @@ extern unsigned char boot_edid_info[128]; /* Slot 261: high read-only compat machine-to-phys conversion table (1GB). */ #define HIRO_COMPAT_MPT_VIRT_START RDWR_COMPAT_MPT_VIRT_END #define HIRO_COMPAT_MPT_VIRT_END (HIRO_COMPAT_MPT_VIRT_START + GB(1)) -/* Slot 261: xen text, static data and bss (1GB). */ +/* Slot 261: xen text, static data, bss, per-cpu stubs and executable fixmap (1GB). */ #define XEN_VIRT_START (HIRO_COMPAT_MPT_VIRT_END) #define XEN_VIRT_END (XEN_VIRT_START + GB(1)) diff --git a/xen/include/asm-x86/fixmap.h b/xen/include/asm-x86/fixmap.h index 9fb2f47946..8094546b75 100644 --- a/xen/include/asm-x86/fixmap.h +++ b/xen/include/asm-x86/fixmap.h @@ -15,6 +15,7 @@ #include <asm/page.h> #define FIXADDR_TOP (VMAP_VIRT_END - PAGE_SIZE) +#define FIXADDR_X_TOP (XEN_VIRT_END - PAGE_SIZE) #ifndef __ASSEMBLY__ @@ -89,6 +90,30 @@ static inline unsigned long virt_to_fix(const unsigned long vaddr) return __virt_to_fix(vaddr); } +enum fixed_addresses_x { + /* Index 0 is reserved since fix_x_to_virt(0) == FIXADDR_X_TOP. */ + FIX_X_RESERVED, +#ifdef CONFIG_HYPERV_GUEST + FIX_X_HYPERV_HCALL, +#endif + __end_of_fixed_addresses_x +}; + +#define FIXADDR_X_SIZE (__end_of_fixed_addresses_x << PAGE_SHIFT) +#define FIXADDR_X_START (FIXADDR_X_TOP - FIXADDR_X_SIZE) + +extern void __set_fixmap_x( + enum fixed_addresses_x idx, unsigned long mfn, unsigned long flags); + +#define set_fixmap_x(idx, phys) \ + __set_fixmap_x(idx, (phys)>>PAGE_SHIFT, PAGE_HYPERVISOR_RX | MAP_SMALL_PAGES) + +#define clear_fixmap_x(idx) __set_fixmap_x(idx, 0, 0) + +#define __fix_x_to_virt(x) (FIXADDR_X_TOP - ((x) << PAGE_SHIFT)) + +#define fix_x_to_virt(x) ((void *)__fix_x_to_virt(x)) + #endif /* __ASSEMBLY__ */ #endif
This allows us to set aside some address space for executable mapping. This fixed map range starts from XEN_VIRT_END so that it is within reach of the .text section. Shift the percpu stub range and shrink livepatch range accordingly. Signed-off-by: Wei Liu <liuwe@microsoft.com> --- v5: 1. drop __virt_to_fix_x 2. also check FIX*_RESERVED in __set_fixmap* 3. generate global symbol to be used in linker script 4. address other misc comments --- xen/arch/x86/boot/x86_64.S | 15 ++++++++++++--- xen/arch/x86/livepatch.c | 3 ++- xen/arch/x86/mm.c | 15 ++++++++++++++- xen/arch/x86/smpboot.c | 2 +- xen/arch/x86/xen.lds.S | 3 +++ xen/include/asm-x86/config.h | 2 +- xen/include/asm-x86/fixmap.h | 25 +++++++++++++++++++++++++ 7 files changed, 58 insertions(+), 7 deletions(-)