Re: [Xen-devel] [RFC PATCH 05/12] hvmloader: add Q35 DSDT table loading

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 13, 2018 at 04:33:50AM +1000, Alexey Gerasimenko wrote:
> Allows to select Q35 DSDT table in hvmloader_acpi_build_tables(). Function
> get_pc_machine_type() is used to select a proper table (i440/q35).
> 
> As we are bound to the qemu-xen device model for Q35, no need
> to initialize config->dsdt_15cpu/config->dsdt_15cpu_len fields.
> 
> Signed-off-by: Alexey Gerasimenko <x1917x@xxxxxxxxx>
> ---
>  tools/firmware/hvmloader/util.c | 13 +++++++++++--
>  tools/firmware/hvmloader/util.h |  2 ++
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
> index 5739a87628..d8db9e3c8e 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -955,8 +955,17 @@ void hvmloader_acpi_build_tables(struct acpi_config 
> *config,
>      }
>      else if ( !strncmp(s, "qemu_xen", 9) )
>      {
> -        config->dsdt_anycpu = dsdt_anycpu_qemu_xen;
> -        config->dsdt_anycpu_len = dsdt_anycpu_qemu_xen_len;
> +        if (get_pc_machine_type() == MACHINE_TYPE_Q35)

Coding style (missing spaces between parentheses), and I would prefer
a switch here.


IMO you should add a BUG_ON(Q35) in the qemu_xen_traditional condition
above this one..

> +        {
> +            config->dsdt_anycpu = dsdt_q35_anycpu_qemu_xen;
> +            config->dsdt_anycpu_len = dsdt_q35_anycpu_qemu_xen_len;
> +        }
> +        else
> +        {
> +            config->dsdt_anycpu = dsdt_anycpu_qemu_xen;
> +            config->dsdt_anycpu_len = dsdt_anycpu_qemu_xen_len;
> +        }
> +
>          config->dsdt_15cpu = NULL;
>          config->dsdt_15cpu_len = 0;
>      }
> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index 7c77bedb00..fd2d885c96 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -288,7 +288,9 @@ bool check_overlap(uint64_t start, uint64_t size,
>                     uint64_t reserved_start, uint64_t reserved_size);
>  
>  extern const unsigned char dsdt_anycpu_qemu_xen[], dsdt_anycpu[], 
> dsdt_15cpu[];
> +extern const unsigned char dsdt_q35_anycpu_qemu_xen[];
>  extern const int dsdt_anycpu_qemu_xen_len, dsdt_anycpu_len, dsdt_15cpu_len;
> +extern const int dsdt_q35_anycpu_qemu_xen_len;

Since you are adding this, maybe unsigned int? (or size_t?)

Thanks, Roger.

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