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

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jun 23, 2015 at 04:29:40PM +0200, Igor Mammedov wrote:
> On Mon, 22 Jun 2015 20:10:28 -0300
> Paulo Alcantara <pcacjr@gmail.com> 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.

Paulo, does some guest need this patch (2/3)?

> > 
> > Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> > ---
> > 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 <pcacjr@zytor.com>
> > + *
> > + * 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<DrdQ@r;`nF?PxT0Bl$YHUIzs
> >