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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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