Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Sep 22, 2016 at 11:57:18AM -0400, Boris Ostrovsky wrote:
> On 09/22/2016 06:53 AM, Wei Liu wrote:
> >
> >> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> >> index 90427ff..336358c 100644
> >> --- a/tools/libxl/Makefile
> >> +++ b/tools/libxl/Makefile
> >> @@ -75,7 +75,21 @@ else
> >>  LIBXL_OBJS-y += libxl_no_colo.o
> >>  endif
> >>  
> >> -LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
> >> +ACPI_PATH  = $(XEN_ROOT)/tools/libacpi
> >> +ACPI_FILES = dsdt_pvh.c
> >> +ACPI_OBJS  = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o
> >> +$(ACPI_FILES): acpi
> >> +$(ACPI_OBJS): CFLAGS += -I. 
> >> -DLIBACPI_STDUTILS=\"$(CURDIR)/libxl_x86_acpi.h\"
> >> +vpath build.c $(ACPI_PATH)/
> >> +vpath static_tables.c $(ACPI_PATH)/
> >> +LIBXL_OBJS-$(CONFIG_X86) += $(ACPI_OBJS)
> >> +
> >> +.PHONY: acpi
> >> +acpi:
> >> +  $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR)
> >> +-include acpi
> > Useless include?
> 
> Hmm... I thought there was a reason I put this here but I can't remember
> now.
> 
> >
> >> +
> >> +static uint8_t acpi_lapic_id(unsigned cpu)
> >> +{
> >> +    return cpu * 2;
> > Is this in accordance with hardware spec?
> 
> That's how we (Xen) encode APIC IDs for guests.  For example, see
> vlapic_reset().
> 

OK.

> >> +
> >> +int libxl__dom_load_acpi(libxl__gc *gc,
> >> +                         const libxl_domain_build_info *b_info,
> >> +                         struct xc_dom_image *dom)
> >> +{
> >> +    struct acpi_config config = {0};
> >> +    struct libxl_acpi_ctxt libxl_ctxt;
> >> +    int rc, acpi_pages_num;
> >> +    void *acpi_pages;
> >> +    unsigned long page_mask;
> >> +
> >> +    if ((b_info->type != LIBXL_DOMAIN_TYPE_HVM) ||
> >> +        (b_info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE))
> >> +        return 0;
> > Please don't use mixed-style error handling.
> 
> Not sure I understand what you are asking for. You want 'return rc'? Or
> 'goto out'?
> 

goto out please.

> -boris
> 

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