[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-next 5/7] x86_64: move PV specific code under pv/x86_64
On Fri, Apr 21, 2017 at 03:33:11AM -0600, Jan Beulich wrote: > >>> On 06.04.17 at 19:14, <wei.liu2@xxxxxxxxxx> wrote: > > No functional change. > > > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx> > > --- > > xen/arch/x86/pv/Makefile | 1 + > > xen/arch/x86/pv/x86_64/Makefile | 1 + > > xen/arch/x86/{ => pv}/x86_64/compat/traps.c | 0 > > xen/arch/x86/pv/x86_64/traps.c | 398 > > ++++++++++++++++++++++++++++ > > xen/arch/x86/x86_64/traps.c | 377 > > -------------------------- > > 5 files changed, 400 insertions(+), 377 deletions(-) > > create mode 100644 xen/arch/x86/pv/x86_64/Makefile > > rename xen/arch/x86/{ => pv}/x86_64/compat/traps.c (100%) > > create mode 100644 xen/arch/x86/pv/x86_64/traps.c > > Please don't - we really should try to remove all those x86_64/ > subdirectories (and I must have overlooked this same aspect > earlier on). There's no reason to believe the current leftovers > from the 32-/64-bit split are going to be appropriate for a > hypothetical successor architecture to x86-64. > Fine with me to remove x86_64 directories. > > -void subarch_percpu_traps_init(void) > > -{ > > - unsigned long stack_bottom = get_stack_bottom(); > > - unsigned long stub_va = this_cpu(stubs.addr); > > - unsigned char *stub_page; > > - unsigned int offset; > > - > > - /* IST_MAX IST pages + 1 syscall page + 1 guard page + primary stack. > > */ > > - BUILD_BUG_ON((IST_MAX + 2) * PAGE_SIZE + PRIMARY_STACK_SIZE > > > STACK_SIZE); > > Even if it's only this one line - this isn't PV-specific, and hence shouldn't > be moved. It is also inappropriate for a function with this name to live > in PV-specific code. > I see your point for this BUILD_BUG_ON. But the rest of this function is setting up trampoline stub for syscall and sysenter, which, AFAICT, are only ever going to be used by PV guests. What did I miss? May be I should rename this function? > > -void hypercall_page_initialise(struct domain *d, void *hypercall_page) > > -{ > > - memset(hypercall_page, 0xCC, PAGE_SIZE); > > - if ( is_hvm_domain(d) ) > > - hvm_hypercall_page_initialise(d, hypercall_page); > > - else if ( !is_pv_32bit_domain(d) ) > > - hypercall_page_initialise_ring3_kernel(hypercall_page); > > - else > > - hypercall_page_initialise_ring1_kernel(hypercall_page); > > -} > > Same for this function, the more that it calls a HVM one. > Yes this one needs to stay in commmon code. Wei. > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |