Re: [SeaBIOS] [PATCH v2 6/6] acpi: build PCI hotplug devices from a single template

[Michael S. Tsirkin]

On Thu, Aug 02, 2012 at 03:07:26PM +0200, Paolo Bonzini wrote:
> More than 1kb of data is taken by the 32 copies of the PCI hotplug SSDT
> methods.  We can build them from a single template like we do for CPUs
> (wrapped in a Scope(_SB.PCI0) block).
>
> Three items differ for each slot: the device name, bits 16-23 of _ADR,
> the _SUN value.  On top of this we have to rename the eject method for
> non-removable slots, like we already do in build_pcihp.
>
> There is a small change in the ASL: instead of including the number of
> the slot in the implementation of _EJ0, we just call _SUN.  This is also
> similar to what we do for CPU hotplug.
>
> Once we do this, there is no need to keep a separate SSDT for PCI hotplug.
> Everything can reside in the same table.

Size was never a reason to keep it separate. This was suggested by
Kevin, the idea was to make it easier for people to override the hotplug
logic. Does this still apply? Kevin?
> Signed-off-by: Paolo Bonzini
>
> src/acpi.c         |   83 ++++++++++++++++++++++++----------------------------
>  src/ssdt-pcihp.dsl |   51 ++++++++------------------------
>  2 files changed, 50 insertions(+), 84 deletions(-)
>
> diff --git a/src/acpi.c b/src/acpi.c
> index 38b72fb..31b4086 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -406,12 +406,22 @@ encodeLen(u8 *ssdt_ptr, int length, int bytes)
>  #define PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
>  #define PROC_AML (ssdp_proc_aml + *ssdt_proc_start)
>
> +/* 0x5B 0x82 DeviceOp PkgLength NameString */
> +#define PCIHP_OFFSET_HEX (*ssdt_pcihp_name - *ssdt_pcihp_start + 1)
> +#define PCIHP_OFFSET_ID (*ssdt_pcihp_id - *ssdt_pcihp_start)
> +#define PCIHP_OFFSET_ADR (*ssdt_pcihp_adr - *ssdt_pcihp_start)
> +#define PCIHP_OFFSET_EJ0 (*ssdt_pcihp_ej0 - *ssdt_pcihp_start)
> +#define PCIHP_SIZEOF (*ssdt_pcihp_end - *ssdt_pcihp_start)
> +#define PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start)
>  #define PCI_SLOTS 32
>
> #define SSDT_SIGNATURE 0x54445353 // SSDT
>  #define SSDT_HEADER_LENGTH 36
>
> #include "ssdt-susp.hex"
> +#include "ssdt-pcihp.hex"
>
> +#define PCI_RMV_BASE 0xae0c
>
> static u8*
>  build_notify(u8 *ssdt_ptr, const char *name, int skip, int count,
> @@ -443,6 +453,24 @@ build_notify(u8 *ssdt_ptr, const char *name, int skip, int count,
>      return ssdt_ptr;
>  }
>
> +static void patch_pcihp(int slot, u8 *ssdt_ptr, u32 eject)
> +{
> ssdt_ptr[PCIHP_OFFSET_HEX] = getHex(slot >> 4);
> ssdt_ptr[PCIHP_OFFSET_HEX+1] = getHex(slot);
> ssdt_ptr[PCIHP_OFFSET_ID] = slot;
> ssdt_ptr[PCIHP_OFFSET_ADR + 2] = slot;
> /* Runtime patching of EJ0: to disable hotplug for a slot,
> * replace the method name: _EJ0 by EJ0_. */
> /* Sanity check */
> if (memcmp(ssdt_ptr + PCIHP_OFFSET_EJ0, "_EJ0", 4)) {
>    warn_internalerror();
> }
> if (!eject) {
>    memcpy(ssdt_ptr + PCIHP_OFFSET_EJ0, "EJ0_", 4);
> }
>
> +}
>
> static void*
>  build_ssdt(void)
>  {
> @@ -454,6 +482,7 @@ build_ssdt(void)
>                    + (6+2+1+(1*acpi_cpus))                   // CPON
>                    + 17                                      // BDAT
>                    + (1+3+4)                                 // Scope(PCI0)
>              + ((PCI_SLOTS - 1) * PCIHP_SIZEOF)        // slots
>              + (1+2+5+(12*(PCI_SLOTS - 1))));          // PCNT
> u8 *ssdt = malloc_high(length);
>    if (! ssdt) {
>
> @@ -545,6 +574,15 @@ build_ssdt(void)
>      *(ssdt_ptr++) = 'I';
>      *(ssdt_ptr++) = '0';
>
> // build Device object for each slot
>
> u32 rmvc_pcrm = inl(PCI_RMV_BASE);
>
> for (i=1; i<PCI_SLOTS; i++) {
>    u32 eject = rmvc_pcrm & (0x1 << i);
>    memcpy(ssdt_ptr, PCIHP_AML, PCIHP_SIZEOF);
>    patch_pcihp(i, ssdt_ptr, eject != 0);
>    ssdt_ptr += PCIHP_SIZEOF;
>
> }
>
> ssdt_ptr = build_notify(ssdt_ptr, "PCNT", 1, PCI_SLOTS, "S00_", 1);
>
> build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
>
> @@ -554,50 +592,6 @@ build_ssdt(void)
>      return ssdt;
>  }
>
> -#include "ssdt-pcihp.hex"
>
> -#define PCI_RMV_BASE 0xae0c
>
> -extern void link_time_assertion(void);
>
> -static void* build_pcihp(void)
> -{
> u32 rmvc_pcrm;
> int i;
> u8 *ssdt = malloc_high(sizeof ssdp_pcihp_aml);
> if (!ssdt) {
>    warn_noalloc();
>    return NULL;
> }
> memcpy(ssdt, ssdp_pcihp_aml, sizeof ssdp_pcihp_aml);
> /* Runtime patching of EJ0: to disable hotplug for a slot,
> * replace the method name: _EJ0 by EJ0_. */
> if (ARRAY_SIZE(aml_ej0_name) != ARRAY_SIZE(aml_adr_dword)) {
>    link_time_assertion();
> }
> rmvc_pcrm = inl(PCI_RMV_BASE);
> for (i = 0; i < ARRAY_SIZE(aml_ej0_name); ++i) {
>    /* Slot is in byte 2 in _ADR */
>    u8 slot = ssdp_pcihp_aml[aml_adr_dword[i] + 2] & 0x1F;
>    /* Sanity check */
>    if (memcmp(ssdp_pcihp_aml + aml_ej0_name[i], "_EJ0", 4)) {
>        warn_internalerror();
>        free(ssdt);
>        return NULL;
>    }
>    if (!(rmvc_pcrm & (0x1 << slot))) {
>        memcpy(ssdt + aml_ej0_name[i], "EJ0_", 4);
>    }
> }
> ((struct acpi_table_header*)ssdt)->checksum = 0;
> ((struct acpi_table_header*)ssdt)->checksum -= checksum(ssdt, sizeof(ssdp_pcihp_aml));
> return ssdt;
>
> -}
>
> #define HPET_SIGNATURE 0x54455048 // HPET
>  static void*
>  build_hpet(void)
> @@ -779,7 +773,6 @@ acpi_bios_init(void)
>      ACPI_INIT_TABLE(build_madt());
>      ACPI_INIT_TABLE(build_hpet());
>      ACPI_INIT_TABLE(build_srat());
>
> ACPI_INIT_TABLE(build_pcihp());
>
> u16 i, external_tables = qemu_cfg_acpi_additional_tables();
>
> diff --git a/src/ssdt-pcihp.dsl b/src/ssdt-pcihp.dsl
> index fd9c0bb..cd66b83 100644
> --- a/src/ssdt-pcihp.dsl
> +++ b/src/ssdt-pcihp.dsl
> @@ -12,50 +12,23 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
>      External (_SB.PCI0.PCEJ, MethodObj)
>  Scope(\_SB.PCI0) {
>    /* Bulk generated PCI hotplug devices */
>    ACPI_EXTRACT_DEVICE_START ssdt_pcihp_start
>    ACPI_EXTRACT_DEVICE_END ssdt_pcihp_end
>    ACPI_EXTRACT_DEVICE_STRING ssdt_pcihp_name
>    // Method _EJ0 can be patched by BIOS to EJ0_
>    // at runtime, if the slot is detected to not support hotplug.
>    // Extract the offset of the address dword and the
>    // _EJ0 name to allow this patching.
>
> -#define hotplug_slot(slot)                              \
>    Device (S##slot) {                              \
>       ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword  \
>       Name (_ADR, 0x##slot##0000)                  \
>       ACPI_EXTRACT_METHOD_STRING aml_ej0_name      \
>       Method (_EJ0, 1) { Return(PCEJ(0x##slot)) }  \
>       Name (_SUN, 0x##slot)                        \
>    Device (SAA) {
>       ACPI_EXTRACT_NAME_BYTE_CONST ssdt_pcihp_id
>       Name (_SUN, 0xAA)
>       ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcihp_adr
>       Name (_ADR, 0xAA0000)
>       ACPI_EXTRACT_METHOD_STRING ssdt_pcihp_ej0
>       Method (_EJ0, 1) { Return(PCEJ(_SUN)) }
>    }
>    hotplug_slot(01)
>    hotplug_slot(02)
>    hotplug_slot(03)
>    hotplug_slot(04)
>    hotplug_slot(05)
>    hotplug_slot(06)
>    hotplug_slot(07)
>    hotplug_slot(08)
>    hotplug_slot(09)
>    hotplug_slot(0a)
>    hotplug_slot(0b)
>    hotplug_slot(0c)
>    hotplug_slot(0d)
>    hotplug_slot(0e)
>    hotplug_slot(0f)
>    hotplug_slot(10)
>    hotplug_slot(11)
>    hotplug_slot(12)
>    hotplug_slot(13)
>    hotplug_slot(14)
>    hotplug_slot(15)
>    hotplug_slot(16)
>    hotplug_slot(17)
>    hotplug_slot(18)
>    hotplug_slot(19)
>    hotplug_slot(1a)
>    hotplug_slot(1b)
>    hotplug_slot(1c)
>    hotplug_slot(1d)
>    hotplug_slot(1e)
>    hotplug_slot(1f)
> }
> }
>
> 1.7.10.4