Re: [Qemu-devel] [SeaBIOS] [PATCH v5 2/3] target-i386: reserve RCRB mmio space in ACPI DSDT table

[Igor Mammedov]

On Mon, 22 Jun 2015 20:10:28 -0300
Paulo Alcantara <address@hidden> wrote:

> This block is mapped into memory space, using the Root Complex Base
> Address (RCBA) register of the PCI-to-LPC bridge. Accesses in this
> space must be limited to 32-(DW) bit quantities. Burst accesses are
> not allowed.
> 
> All Chipset Configuration Registers are located in this 16KiB space.
Do we really need to describe this block in ASL?
PCI0 bus _CRS doesn't include 0xfed1c000 address and stops at
0xfec00000 (w32.end - end of 32-bit PCI window), so adding a
device that consumes not provided by parent bus resources
probably is not correct.

Taking in account that address is beyond of dynamically used PCI
memory resources, I'd just drop patch.

If the goal of it to make sure that it provides resource conflict
check then Windows wasn't doing a good job with PNP0C02 used,
that's why we ended up with PNP0A06 (between PNP0C02, PNP0A06, ACPI0004)
for this task in cases of absence of a defined device HID.
And it should be moved out of PCI0 bus scope anyway.

> 
> Signed-off-by: Paulo Alcantara <address@hidden>
> ---
> v1 -> v2:
>   * s/PDRC/CCR/ for clarity and match ICH9 spec
>   * remove unnecessary OperationRegion for RCRB
> 
> v2 -> v3: (no changes)
> 
> v3 -> v4:
>   * quote RCRB description from ICH9 spec to commit log
>   * fix indentation issue in _CRS() method declaration
>   * create hw/i386/ich9-cc.h for chipset configuration register values
>     and use them in ASL
> 
> v4 -> v5:
>   * prefix macros in ich9-cc.h with "ICH9_" for better readability and
>     make use of them in CCR device definition
> ---
>  hw/i386/q35-acpi-dsdt.dsl     |  16 ++++++++++++++++
>  include/hw/i386/ich9-cc.h     |  15 +++++++++++++++
>  tests/acpi-test-data/q35/DSDT | Bin 7666 -> 7723 bytes
>  3 files changed, 31 insertions(+)
>  create mode 100644 include/hw/i386/ich9-cc.h
> 
> diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
> index 16eaca3..8f4bb6a 100644
> --- a/hw/i386/q35-acpi-dsdt.dsl
> +++ b/hw/i386/q35-acpi-dsdt.dsl
> @@ -114,6 +114,22 @@ DefinitionBlock (
>          }
>      }
>  
> +#include "hw/i386/ich9-cc.h"
> +
> +/****************************************************************
> + * Chipset Configuration Registers
> + ****************************************************************/
> +Scope(\_SB.PCI0) {
> +    Device (CCR) {
> +        Name (_HID, EISAID("PNP0C02"))
> +        Name (_UID, 1)
> +
> +        Name (_CRS, ResourceTemplate() {
> +            Memory32Fixed(ReadWrite, ICH9_RCBA_BASE_ADDR,
> ICH9_RCRB_SIZE)
> +        })
> +    }
> +}
> +
>  #include "acpi-dsdt-hpet.dsl"
>  
>  
> diff --git a/include/hw/i386/ich9-cc.h b/include/hw/i386/ich9-cc.h
> new file mode 100644
> index 0000000..d4918ff
> --- /dev/null
> +++ b/include/hw/i386/ich9-cc.h
> @@ -0,0 +1,15 @@
> +/*
> + * QEMU ICH9 Chipset Configuration Registers
> + *
> + * Copyright (c) 2015 Paulo Alcantara <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2
> or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#ifndef HW_ICH9_CC_H
> +#define HW_ICH9_CC_H
> +
> +#define ICH9_RCBA_BASE_ADDR    0xfed1c000
> +#define ICH9_RCRB_SIZE         0x00004000
> +
> +#endif /* HW_ICH9_CC_H */
> diff --git a/tests/acpi-test-data/q35/DSDT
> b/tests/acpi-test-data/q35/DSDT index
> 4723e5954dccb00995ccaf521b7daf6bf15cf1d4..f3bda7b54ea6d669b1498d9380e7781207fb6e49
> 100644 GIT binary patch delta 81
> zcmexlz1oJ$CD<iITaJN&F>xbTJfnq$UVN}qe1Nm3L3ERjvvW{9N4$rp3y<Rk9wv_X
> lP)`>|j(F#wU_n7HzBWz<Mur0y|1mf)FjO*Z&S3140RVI`6(s-w
> 
> delta 24
> gcmZ2&^U0daCD<k8lPm)R<address@hidden;`nF?PxT0Bl$YHUIzs
>