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

[Igor Mammedov]

On 07/31/2012 11:52 AM, 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.
> Signed-off-by: Paolo Bonzini
>      On top of this: do we actually need the S01/S02 devices?  Their
>      address is a duplicate of \_SB.PCI0.ISA and \_SB.PCI0.VGA's; and
>      they are never removable so _EJ0 is always patched out.  The only
>      addition is the _SUN, which can be added in the DSDT.
>
> src/acpi.c         |   97 +++++++++++++++++++++++++++-------------------------
>   src/ssdt-pcihp.dsl |   51 +++++++--------------------
>   2 files changed, 63 insertions(+), 85 deletions(-)
>
> diff --git a/src/acpi.c b/src/acpi.c
> index 906237c..2d4513b 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -405,11 +405,42 @@ 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 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(ssdp_pcihp_aml + PCIHP_OFFSET_EJ0, "_EJ0", 4)) {
>    warn_internalerror();
> }
> if (!eject) {
>    memcpy(ssdt_ptr + PCIHP_OFFSET_EJ0, "EJ0_", 4);
> }
>
> +}
> static void*
> build_ssdt(void)
> {
>
> @@ -420,7 +451,9 @@ build_ssdt(void)
>                     + (acpi_cpus * PROC_SIZEOF)
>                     + (1+2+5+(12*acpi_cpus))
>                     + (6+2+1+(1*acpi_cpus))
>              + 17);
>              + 17
>              + (1+3+4)

perhaps macros or const here would be better
>              + (PCI_SLOTS * PCIHP_SIZEOF));
> u8 *ssdt = malloc_high(length);
> if (! ssdt) {
>     warn_noalloc();
>
> @@ -522,6 +555,23 @@ build_ssdt(void)
>       *(u32*)ssdt_ptr = sizeof(struct bfld);
>       ssdt_ptr += 4;
> // build Scope(PCI0) opcode
> *(ssdt_ptr++) = 0x10; // ScopeOp
> ssdt_ptr = encodeLen(ssdt_ptr, length - (ssdt_ptr - ssdt), 3);

in case we later decide to add something to _SB scope, could it set len 
to not remaining buffer length but rather actual size of the built PCI0 
scope.

it also will help to get scope len right if we omit _EJ0 for non 
eject-able devices. we could just not copy method and fixup device len.
> *(ssdt_ptr++) = 'P';
>
> *(ssdt_ptr++) = 'C';
>
> *(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;
>
> }
> build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
>
> //hexdump(ssdt, ssdt_ptr - ssdt);
>
> @@ -529,50 +579,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)
>
> @@ -754,7 +760,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)
> }
> }