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

[Alexey G <x1917x@xxxxxxxxx>]

On Mon, 19 Mar 2018 14:45:29 +0000
Roger Pau Monné <roger.pau@xxxxxxxxxx> 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 <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.

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