Re: [Xen-devel] [PATCH v1 19/20] libxl/pvhv2: Include APIC page in MMIO hole for PVHv2 guests

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jul 05, 2016 at 03:05:18PM -0400, Boris Ostrovsky wrote:
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> ---
> 
> New patch
> 
>  tools/libxl/Makefile    |    2 ++
>  tools/libxl/libxl_dom.c |   22 ++++++++++++++++++----
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 9fee752..3a2d64a 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -165,6 +165,8 @@ $(XL_OBJS) $(TEST_PROG_OBJS) _libxl.api-for-check: \
>  $(XL_OBJS): CFLAGS += $(CFLAGS_XL)
>  $(XL_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h # libxl_json.h 
> needs it.
>  
> +libxl_dom.o: CFLAGS += -I$(XEN_ROOT)/tools  # include libacpi/x86.h
> +
>  SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o
>  $(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenevtchn)
>  
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index ccc41b4..ba3472e 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -24,6 +24,9 @@
>  #include <xen/hvm/hvm_info_table.h>
>  #include <xen/hvm/hvm_xs_strings.h>
>  #include <xen/hvm/e820.h>
> +#if defined(__i386__) || defined(__x86_64__)
> +#include <libacpi/x86.h>
> +#endif
>  
>  #include "_paths.h"
>  
> @@ -1006,10 +1009,21 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>          dom->target_pages = mem_size >> XC_PAGE_SHIFT;
>      if (dom->mmio_size == 0 && device_model)
>          dom->mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
> -    else if (dom->mmio_size == 0 && !device_model)
> -        dom->mmio_size = GB(4) -
> -                    ((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
> -                    << XC_PAGE_SHIFT);
> +    else if (dom->mmio_size == 0 && !device_model) {
> +#if defined(__i386__) || defined(__x86_64__)
> +        if (libxl_defbool_val(info->u.hvm.apic)) {
> +            /* Make sure LAPIC_BASE_ADDRESS is below special pages */
> +            assert(((((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
> +                      << XC_PAGE_SHIFT) - LAPIC_BASE_ADDRESS)) >= 
> XC_PAGE_SIZE);
> +            dom->mmio_size = GB(4) - LAPIC_BASE_ADDRESS;
> +        } else
> +            dom->mmio_size = GB(4) -
> +                ((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
> +                 << XC_PAGE_SHIFT);
> +#else
> +        assert(1);

This looks a bit odd. Do you want to avoid "if {}" (nothing in braces)?

If this branch doesn't nothing on ARM, maybe you can just do

    #if defined(x86)
     else if () {
     }
    #endif

?
      
> +#endif
> +    }
>      lowmem_end = mem_size;
>      highmem_end = 0;
>      mmio_start = (1ull << 32) - dom->mmio_size;
> -- 
> 1.7.1
> 

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