[PATCH v2 32/35] x86: Allow devices to write to DSDT

[Wolfgang Wallner]

Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH v2 32/35] x86: Allow devices to write to DSDT
> 
> Call the new core function to inject ASL programmatically into the DSDT.
> This is made up of fragments generated by devices that have the
> inject_dsdt() method. The normal, compiled ASL file is added after this.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2: None
> Changes in v1: None
> 
>  arch/x86/lib/acpi_table.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 4658d88351..a7ec6d2b15 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -411,7 +411,20 @@ ulong write_acpi_tables(ulong start_addr)
>  	memcpy(ctx->current,
>  	       (char *)&AmlCode + sizeof(struct acpi_table_header),
>  	       dsdt->length - sizeof(struct acpi_table_header));
> -	acpi_inc_align(ctx, dsdt->length - sizeof(struct acpi_table_header));
> +
> +	if (dsdt->length >= sizeof(struct acpi_table_header)) {
> +		acpi_inject_dsdt(ctx);
> +		memcpy(ctx->current,
> +		       (char *)AmlCode + sizeof(struct acpi_table_header),
> +		       dsdt->length - sizeof(struct acpi_table_header));

A similar memcpy is already executed a few lines above before acpi_inject_dsdt()
is called. Would it be possible to unify the two code paths, e.g. only call
that memcpy once with the correct adress? In the case of the if() the initial
memcpy would have no effect because that memory will be overwritten again
if I understand correctly.

> +		acpi_inc(ctx, dsdt->length - sizeof(struct acpi_table_header));
> +
> +		/* (Re)calculate length and checksum. */
> +		dsdt->length = ctx->current - (void *)dsdt;
> +		dsdt->checksum = 0;
> +		dsdt->checksum = table_compute_checksum(dsdt, dsdt->length);

1) dsdt->checksum is set anyway a few line below (after the GVNS update). Is it
also required to set it here?

2) Why is set to 0 just before recomputation? (This is also done in the
existing code a few lines below when it is set again.)

> +	}
> +	acpi_align(ctx);
>  
>  	/* Pack GNVS into the ACPI table area */
>  	for (i = 0; i < dsdt->length; i++) {
> -- 
> 2.26.2.645.ge9eca65c58-goog

regards, Wolfgang