[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH 10/12] libacpi: build ACPI MCFG table if requested



On Tue, Mar 13, 2018 at 04:33:55AM +1000, Alexey Gerasimenko wrote:
> This adds construct_mcfg() function to libacpi which allows to build MCFG
> table for a given mmconfig_addr/mmconfig_len pair if the ACPI_HAS_MCFG
> flag was specified in acpi_config struct.
> 
> The maximum bus number is calculated from mmconfig_len using
> MCFG_SIZE_TO_NUM_BUSES macro (1MByte of MMIO space per bus).
> 
> Signed-off-by: Alexey Gerasimenko <x1917x@xxxxxxxxx>
> ---
>  tools/libacpi/acpi2_0.h | 21 +++++++++++++++++++++
>  tools/libacpi/build.c   | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tools/libacpi/libacpi.h |  4 ++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/tools/libacpi/acpi2_0.h b/tools/libacpi/acpi2_0.h
> index 2619ba32db..209ad1acd3 100644
> --- a/tools/libacpi/acpi2_0.h
> +++ b/tools/libacpi/acpi2_0.h
> @@ -422,6 +422,25 @@ struct acpi_20_slit {
>  };
>  
>  /*
> + * PCI Express Memory Mapped Configuration Description Table
> + */
> +struct mcfg_range_entry {
> +    uint64_t base_address;
> +    uint16_t pci_segment;
> +    uint8_t  start_pci_bus_num;
> +    uint8_t  end_pci_bus_num;
> +    uint32_t reserved;
> +};
> +
> +struct acpi_mcfg {
> +    struct acpi_header header;
> +    uint8_t reserved[8];
> +    struct mcfg_range_entry entries[1];
> +};

I would define this as:

struct acpi_10_mcfg {
    struct acpi_header header;
    uint8_t reserved[8];
    struct acpi_10_mcfg_entry {
        uint64_t base_address;
        uint16_t pci_segment;
        uint8_t  start_pci_bus;
        uint8_t  end_pci_bus;
        uint32_t reserved;
    } entries[1];
};

> +
> +#define MCFG_SIZE_TO_NUM_BUSES(size)  ((size) >> 20)

I'm not sure the following macro belongs here. This is not directly
related to ACPI.

> +
> +/*
>   * Table Signatures.
>   */
>  #define ACPI_2_0_RSDP_SIGNATURE ASCII64('R','S','D',' ','P','T','R',' ')
> @@ -435,6 +454,7 @@ struct acpi_20_slit {
>  #define ACPI_2_0_WAET_SIGNATURE ASCII32('W','A','E','T')
>  #define ACPI_2_0_SRAT_SIGNATURE ASCII32('S','R','A','T')
>  #define ACPI_2_0_SLIT_SIGNATURE ASCII32('S','L','I','T')
> +#define ACPI_MCFG_SIGNATURE     ASCII32('M','C','F','G')
>  
>  /*
>   * Table revision numbers.
> @@ -449,6 +469,7 @@ struct acpi_20_slit {
>  #define ACPI_1_0_FADT_REVISION 0x01
>  #define ACPI_2_0_SRAT_REVISION 0x01
>  #define ACPI_2_0_SLIT_REVISION 0x01
> +#define ACPI_1_0_MCFG_REVISION 0x01
>  
>  #pragma pack ()
>  
> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
> index f9881c9604..5daf1fc5b8 100644
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -303,6 +303,37 @@ static struct acpi_20_slit *construct_slit(struct 
> acpi_ctxt *ctxt,
>      return slit;
>  }
>  
> +static struct acpi_mcfg *construct_mcfg(struct acpi_ctxt *ctxt,
> +                                        const struct acpi_config *config)
> +{
> +    struct acpi_mcfg *mcfg;
> +
> +    /* Warning: this code expects that we have only one PCI segment */
> +    mcfg = ctxt->mem_ops.alloc(ctxt, sizeof(*mcfg), 16);
> +    if (!mcfg)

Coding style.

> +        return NULL;
> +
> +    memset(mcfg, 0, sizeof(*mcfg));
> +    mcfg->header.signature    = ACPI_MCFG_SIGNATURE;
> +    mcfg->header.revision     = ACPI_1_0_MCFG_REVISION;
> +    fixed_strcpy(mcfg->header.oem_id, ACPI_OEM_ID);
> +    fixed_strcpy(mcfg->header.oem_table_id, ACPI_OEM_TABLE_ID);
> +    mcfg->header.oem_revision = ACPI_OEM_REVISION;
> +    mcfg->header.creator_id   = ACPI_CREATOR_ID;
> +    mcfg->header.creator_revision = ACPI_CREATOR_REVISION;
> +    mcfg->header.length = sizeof(*mcfg);

As said before, if you want to align things, please do it for the
whole block.

> +
> +    mcfg->entries[0].base_address = config->mmconfig_addr;
> +    mcfg->entries[0].pci_segment = 0;
> +    mcfg->entries[0].start_pci_bus_num = 0;
> +    mcfg->entries[0].end_pci_bus_num =
> +        MCFG_SIZE_TO_NUM_BUSES(config->mmconfig_len) - 1;

Why not pass the start_bus and end_bus values in acpi_config at least?

> +
> +    set_checksum(mcfg, offsetof(struct acpi_header, checksum), 
> sizeof(*mcfg));
> +
> +    return mcfg;;

Double ;;

> +}
> +
>  static int construct_passthrough_tables(struct acpi_ctxt *ctxt,
>                                          unsigned long *table_ptrs,
>                                          int nr_tables,
> @@ -350,6 +381,7 @@ static int construct_secondary_tables(struct acpi_ctxt 
> *ctxt,
>      struct acpi_20_hpet *hpet;
>      struct acpi_20_waet *waet;
>      struct acpi_20_tcpa *tcpa;
> +    struct acpi_mcfg *mcfg;
>      unsigned char *ssdt;
>      static const uint16_t tis_signature[] = {0x0001, 0x0001, 0x0001};
>      void *lasa;
> @@ -417,6 +449,16 @@ static int construct_secondary_tables(struct acpi_ctxt 
> *ctxt,
>          printf("CONV disabled\n");
>      }
>  
> +    /* MCFG */
> +    if ( config->table_flags & ACPI_HAS_MCFG )
> +    {
> +        mcfg = construct_mcfg(ctxt, config);
> +        if (!mcfg)

Coding style.

> +            return -1;
> +
> +        table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, mcfg);
> +    }
> +
>      /* TPM TCPA and SSDT. */
>      if ( (config->table_flags & ACPI_HAS_TCPA) &&
>           (config->tis_hdr[0] == tis_signature[0]) &&
> diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
> index a2efd23b0b..dd85b928e9 100644
> --- a/tools/libacpi/libacpi.h
> +++ b/tools/libacpi/libacpi.h
> @@ -36,6 +36,7 @@
>  #define ACPI_HAS_8042              (1<<13)
>  #define ACPI_HAS_CMOS_RTC          (1<<14)
>  #define ACPI_HAS_SSDT_LAPTOP_SLATE (1<<15)
> +#define ACPI_HAS_MCFG              (1<<16)
>  
>  struct xen_vmemrange;
>  struct acpi_numa {
> @@ -96,6 +97,9 @@ struct acpi_config {
>      uint32_t ioapic_base_address;
>      uint16_t pci_isa_irq_mask;
>      uint8_t ioapic_id;
> +
> +    uint64_t mmconfig_addr;
> +    uint32_t mmconfig_len;

This interface is quite limited because it only allows us to create a
single MCFG entry, but since this is not a public interface I guess it
doesn't matter that much, it can always be expanded when required.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.