Re: [Xen-devel] [PATCH for-next 5/7] x86_64: move PV specific code under pv/x86_64

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> -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.

> -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.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel