Re: [Xen-devel] [PATCH RFC 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 06/02/2016 08:54 AM, Jan Beulich wrote:
>>>> On 06.04.16 at 03:25, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> acpi_info can be initialized by hvmloader itself. Now ACPI code
>> doesn't need to use hvmloader-private variables/routines such as
>> uart_exists(), lpt_exists() etc.
> So from a mechanical pov the patch looks fine (one remark below),
> but if you move this out from acpi/, who's going to do the setup of
> that structure outside of hvmloader? And if done by another
> entity, how do you mean things to remain in sync? 

Each caller (currently hvmloader and libxc, possibly the hypervisor in
the future) will initialize it. (I think you saw this in the next patch)

> And btw., you're
> still not fully decoupling things: ACPI_INFO_PHYSICAL_ADDRESS is
> a hvmloader thing, which you just move the reference to inside
> acpi/.

It's not an hvmloader thing, it's the ACPI thing: this is the address
that should match DSDT's description.

Perhaps I should have a script that extracts this address from dsdt.asl
and then make libacpi return it instead of hardcoding it in callers' code.

-boris

>
>> @@ -619,20 +597,12 @@ void acpi_build_tables(struct acpi_config *config, 
>> unsigned int physical)
>>                   offsetof(struct acpi_20_rsdp, extended_checksum),
>>                   sizeof(struct acpi_20_rsdp));
>>  
>> -    if ( !new_vm_gid(acpi_info) )
>> +    if ( !new_vm_gid(&config->acpi_info) )
>>          goto oom;
>>  
>> -    acpi_info->com1_present = uart_exists(0x3f8);
>> -    acpi_info->com2_present = uart_exists(0x2f8);
>> -    acpi_info->lpt1_present = lpt_exists(0x378);
>> -    acpi_info->hpet_present = hpet_exists(ACPI_HPET_ADDRESS);
>> -    acpi_info->pci_min = pci_mem_start;
>> -    acpi_info->pci_len = pci_mem_end - pci_mem_start;
>> -    if ( pci_hi_mem_end > pci_hi_mem_start )
>> -    {
>> -        acpi_info->pci_hi_min = pci_hi_mem_start;
>> -        acpi_info->pci_hi_len = pci_hi_mem_end - pci_hi_mem_start;
>> -    }
>> +    memcpy((struct acpi_info *)ACPI_INFO_PHYSICAL_ADDRESS,
>> +           &config->acpi_info,
>> +           sizeof(config->acpi_info));
> I'd prefer structure assignment to be used in cases like this, as
> that lets the compiler check that the types match.
>
> Jan
>



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