Re: [PATCH v2 gnumach] ACPI: Support XSDT (ACPI >= v2.0)

[Samuel Thibault]

Damien Zammit, le mar. 30 janv. 2024 08:04:38 +0000, a ecrit:
> This enables gnumach to additionally parse the XSDT table
> if the revision of ACPI is 2.
> 
> NB: I removed a few checksum checks in acpi tables where
> there is no checksum present in the table.
> 
> TESTED: Still works on qemu (ACPI v1.0)
> TESTED: Works on a x86 board with XSDT (ACPI v2.0)
> 
> ---
>  i386/i386at/acpi_parse_apic.c | 243 ++++++++++++++++++++++------------
>  i386/i386at/acpi_parse_apic.h |  18 ++-
>  i386/i386at/model_dep.c       |   8 +-
>  3 files changed, 180 insertions(+), 89 deletions(-)
> 
> diff --git a/i386/i386at/acpi_parse_apic.c b/i386/i386at/acpi_parse_apic.c
> index 1aef53ed..476d846e 100644
> --- a/i386/i386at/acpi_parse_apic.c
> +++ b/i386/i386at/acpi_parse_apic.c
> @@ -43,13 +43,12 @@ unsigned lapic_addr;
>   * and the number of entries stored in RSDT.
>   */
>  void
> -acpi_print_info(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt, int 
> acpi_rsdt_n)
> +acpi_print_info(phys_addr_t rsdp, void *rsdt, int acpi_rsdt_n)
>  {
>  
>      printf("ACPI:\n");
> -    printf(" rsdp = %p; rsdp->rsdt_addr = %x\n", rsdp, rsdp->rsdt_addr);
> -    printf(" rsdt = %p; rsdt->length = %x (n = %x)\n", rsdt, 
> rsdt->header.length,
> -           acpi_rsdt_n);
> +    printf(" rsdp = 0x%lx\n", rsdp);
> +    printf(" rsdt/xsdt = 0x%p (n = %d)\n", rsdt, acpi_rsdt_n);
>  }
>  
>  /*
> @@ -69,6 +68,8 @@ acpi_checksum(void *addr, uint32_t length)
>      for (i = 0; i < length; i++)
>          checksum += bytes[i];
>  
> +    printf("checksum result = 0x%x\n", checksum);

leftover ;)

>      return checksum;
>  }
>  
> -/*
> - * acpi_check_rsdt: check if the RSDT initial address is correct
> - * checking its checksum.
> - *
> - * Receives as input a reference for the RSDT "candidate" table.
> - * Returns 0 if success.
> - *
> - * Preconditions: rsdp must not be NULL.
> - *
> - */
> -static int
> -acpi_check_rsdt(struct acpi_rsdt *rsdt)
> -{
> -    uint8_t checksum;
> -
> -    checksum = acpi_checksum(rsdt, rsdt->header.length);
> -
> -    if (checksum != 0)
> -        return ACPI_BAD_CHECKSUM;
> -
> -    return ACPI_SUCCESS;
> -}

Don't we want to keep checking the rsdt checksum?

> @@ -264,6 +253,40 @@ acpi_get_rsdt(struct acpi_rsdp *rsdp, int* acpi_rsdt_n)
>      return rsdt;
>  }
>  
> +/*
> + * acpi_get_xsdt: Get XSDT table reference from RSDPv2 entries.
> + *
> + * Receives as input a reference for RSDPv2 table
> + * and a reference to store the number of entries of XSDT.
> + *
> + * Returns the reference to XSDT table if success, NULL if error.
> + */
> +static struct acpi_xsdt*
> +acpi_get_xsdt(phys_addr_t rsdp_phys, int* acpi_xsdt_n)
> +{
> +    struct acpi_xsdt *xsdt = NULL;
> +    int signature_check;
> +
> +    xsdt = (struct acpi_xsdt*) kmem_map_aligned_table(rsdp_phys, 
> sizeof(struct acpi_xsdt), VM_PROT_READ);
> +
> +    /* Check if the RSDT mapping is fine. */
> +    if (xsdt == NULL)
> +        return NULL;
> +
> +    /* Check is rsdt signature is equals to ACPI RSDT signature. */
> +    signature_check = acpi_check_signature(xsdt->header.signature, 
> ACPI_XSDT_SIG,
> +                                           4*sizeof(uint8_t));
> +
> +    if (signature_check != ACPI_SUCCESS)
> +        return NULL;

Probably this table has a checksum too?

> +    /* Calculated number of elements stored in rsdt. */
> +    *acpi_xsdt_n = (xsdt->header.length - sizeof(xsdt->header))
> +                   / sizeof(xsdt->entry[0]);
> +
> +    return xsdt;
> +}
> +
>  /*
>   * acpi_get_apic: get MADT/APIC table from RSDT entries.
>   *

> @@ -452,16 +512,9 @@ acpi_apic_parse_table(struct acpi_apic *apic)
>  static int
>  acpi_apic_setup(struct acpi_apic *apic)
>  {
> -    int apic_checksum;
>      ApicLocalUnit* lapic_unit;
>      uint8_t ncpus, nioapics;
>  
> -    /* Check the checksum of the APIC */
> -    apic_checksum = acpi_checksum(apic, apic->header.length);
> -
> -    if(apic_checksum != 0)
> -        return ACPI_BAD_CHECKSUM;
> -

And here as well, we'd want to keep the checksum?

>      /* map common lapic address */
>      lapic_addr = apic->lapic_addr;
>      lapic_unit = kmem_map_aligned_table(apic->lapic_addr, 
> sizeof(ApicLocalUnit),

Samuel