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

[Alexey G]

On Mon, 19 Mar 2018 14:45:29 +
Roger Pau Monné  wrote:

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

OK, will change to a switch.

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

AFAIR qemu-traditional knows nothing about Q35 emulation, so we won't
ever encounter a Q35 chipset while using qemu-traditional.

>> +{
>> +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?)

No problem, ok.

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

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

[Roger Pau Monné]

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 
> ---
>  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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel