[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMC
From: |
Igor Mammedov |
Subject: |
Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region |
Date: |
Tue, 16 Feb 2021 23:04:13 +0100 |
On Tue, 16 Feb 2021 10:13:25 -0800
Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
> On Tue, Feb 16, 2021 at 08:45:48AM -0500,
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Tue, Feb 16, 2021 at 01:43:01AM -0800, Isaku Yamahata wrote:
> > > On Mon, Feb 15, 2021 at 01:48:32PM +0100,
> > > Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > > On Fri, 12 Feb 2021 12:51:57 -0800
> > > > Isaku Yamahata <isaku.yamahata@gmail.com> wrote:
> > > >
> > > > > On Fri, Feb 12, 2021 at 04:40:38PM +0100,
> > > > > Igor Mammedov <imammedo@redhat.com> wrote:
> > > > >
> > > > > > On Wed, 10 Feb 2021 22:46:43 -0800
> > > > > > Isaku Yamahata <isaku.yamahata@intel.com> wrote:
> > > > > >
> > > > > > > + Aml *dev;
> > > > > > > + Aml *rbuf;
> > > > > > > + Aml *resource_template;
> > > > > > > + Aml *rbuf_name;
> > > > > > > + Aml *crs;
> > > > > > > +
> > > > > > > + if (!acpi_get_mcfg(&mcfg)) {
> > > > > > > + return NULL;
> > > > > > > + }
> > > > > > > +
> > > > > > > + /* DRAM controller */
> > > > > > > + dev = aml_device("DRAC");
> > > > > > > + aml_append(dev, aml_name_decl("_HID",
> > > > > > > aml_string("PNP0C01")));
> > > > > > > +
> > > > > > > + resource_template = aml_resource_template();
> > > > > > > + aml_append(resource_template,
> > > > > > > + aml_qword_memory(AML_POS_DECODE,
> > > > > > > + AML_MIN_FIXED,
> > > > > > > + AML_MAX_FIXED,
> > > > > > > + AML_NON_CACHEABLE,
> > > > > > > + AML_READ_WRITE,
> > > > > > > + 0x0000000000000000,
> > > > > > > + mcfg.base,
> > > > > > > + mcfg.base + mcfg.size - 1,
> > > > > > s/mcfg.base + mcfg.size - 1/mcfg.base/
> > > > >
> > > > > AddressMaximum needs to be the highest address of the region.
> > > > > Not base address. By disassemble/assembl it, iasl complains as
> > > > > follows.
> > > > > Also there are similar examples in acpi-build.c.
> > > > I mostly clean up all places to use the same base address for min/max,
> > > > but probably something were left behind.
> > > >
> > > > spec says:
> > > >
> > > > acpi 6.3: 19.6.110 QWordMemory
> > > >
> > > > AddressMaximum evaluates to a 64-bit integer that specifies the highest
> > > > possible base address of the
> > > > Memory range. The value must have ‘0’ in all bits where the
> > > > corresponding bit in AddressGranularity is
> > > > ‘1’. For bridge devices which translate addresses, this is the address
> > > > on the secondary bus. The 64-bit
> > > > field DescriptorName ._MAX is automatically created to refer to this
> > > > portion of the resource descriptor.
> > >
> > > Ok, Linux guest is happy with min=max.
> > > I conlude that it's iasl issue.
> > >
> > > Thanks,
> >
> > OK but what about all the other places in the code that seem to use this
> > field differently?
>
> Igor, what do you think?
> The followings are the summary of the situation.
>
> - acpi 6.4: 19.6.110 QWordMemory
> _MAX: maximum of base address.
>
> - acpi 6.4: 6.4.3.5 Address Space Resource Descriptors
> table 6.44
> If _LEN > 0 and _MIF = 1 and _MAF = 1, then _LEN = _MAX - _MIN + 1
> This doesn't match with the above description
I'd say it 19.6.110 doesn't describes whole possibilities of resource,
but only subset in its current phrasing.
> - iasl
> If _LEN > 0 and _MIF = 1 and _MAF = 1,
> it emits warning on _LEN != _MAX - _MIN + 1
so IASL is right and follows spec.
Anyways, My apologies for misleading and your patch is correct.
I need to add similar checks to QEMU code plus comments to avoid confusion
in future.
> - Linux Guest MMCONFIG check
> check_mcfg_resoure() uses only _MIN. doesn't use _MAX.
> _MAX value doesn't matter
>
> - Linux acpi code
> acpi_decode_space() uses _MAX to calulate range [start, end], not use _LEN.
> i.e. It assumes _LEN = _MAX - _MIN + 1 if _LEN > 0, _MIF = 1, _MAF = 1
>
> - qemu code sets for [qd]word_memory_resource (except this patch)
> If _LEN > 0 and _MIF = 1 and _MAF = 1, then set _LEN = _MAX - _MIN + 1
- [PATCH v3 06/10] acpi: add test case for smm unsupported -machine smm=off, (continued)
- [PATCH v3 06/10] acpi: add test case for smm unsupported -machine smm=off, Isaku Yamahata, 2021/02/11
- [PATCH v3 09/10] acpi: add test case for -no-hpet, Isaku Yamahata, 2021/02/11
- [PATCH v3 08/10] i386: acpi: Don't build HPET ACPI entry if HPET is disabled, Isaku Yamahata, 2021/02/11
- [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/11
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Igor Mammedov, 2021/02/12
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/12
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Igor Mammedov, 2021/02/15
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/16
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Michael S. Tsirkin, 2021/02/16
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region, Isaku Yamahata, 2021/02/16
- Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region,
Igor Mammedov <=
[PATCH v3 10/10] qtest/acpi/bios-tables-test: update acpi tables, Isaku Yamahata, 2021/02/11