[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC Patch v4 6/8] hvmload: Add x2apic entry support in the MADT and SRAT build



>>> On 06.12.17 at 08:50, <chao.gao@xxxxxxxxx> wrote:
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -30,6 +30,11 @@
>  
>  #define align16(sz)        (((sz) + 15) & ~15)
>  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
> +#define min(X, Y) ({                             \
> +            const typeof (X) _x = (X);           \
> +            const typeof (Y) _y = (Y);           \
> +            (void) (&_x == &_y);                 \
> +            (_x < _y) ? _x : _y; })

No new name space violations please: Identifiers starting with a single 
underscore
and a lower case letter are reserved for file scope symbols. Use a trailing
underscore instead, for example.

> @@ -79,16 +84,19 @@ static struct acpi_20_madt *construct_madt(struct 
> acpi_ctxt *ctxt,
>      struct acpi_20_madt_intsrcovr *intsrcovr;
>      struct acpi_20_madt_ioapic    *io_apic;
>      struct acpi_20_madt_lapic     *lapic;
> +    struct acpi_20_madt_x2apic    *x2apic;
>      const struct hvm_info_table   *hvminfo = config->hvminfo;
> -    int i, sz;
> +    int i, sz, nr_apic;

Apart from your own addition I think both i and sz also want to be unsigned.
Please make such corrections when you touch a line anyway.

>      if ( config->lapic_id == NULL )
>          return NULL;
>  
> +    nr_apic = min(hvminfo->nr_vcpus, MADT_MAX_LOCAL_APIC);

With this the variable name is not well chosen. nr_xapic perhaps?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.