[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 08/20] microvm/acpi: add minimal acpi support
From: |
Igor Mammedov |
Subject: |
Re: [PATCH v5 08/20] microvm/acpi: add minimal acpi support |
Date: |
Fri, 10 Jul 2020 21:30:27 +0200 |
On Thu, 9 Jul 2020 14:33:32 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
> > > + scope = aml_scope("\\");
> > > + pkg = aml_package(4);
> > > + aml_append(pkg, aml_int(5)); /* SLEEP_CONTROL_REG.SLP_TYP */
> >
> > I'm not sure what does the comment refer to here.
>
> It's the register field the value gets written to.
> With full acpi this is PM1a_CNT.SLP_TYP, hw-reduced uses
> SLEEP_CONTROL_REG.SLP_TYP instead. This is cut&paste from pc/q35
> version + adapted for hw-reduced.
>
> > Does this 5 match
> > the value IO handler tests against?
>
> Yes. "5" for S5 state (aka poweroff). Can add a #define.
>
> > Below is from "7.3.4 System \_Sx states" right?
>
> "7.4.2 \_Sx (System States)" here (ACPI 6.3), guess that is the same.
>
> > > + AcpiFadtData pmfadt = {
> > > + /*
> > > + * minimum version for ACPI_FADT_F_HW_REDUCED_ACPI,
> > > + * see acpi spec "4.1 Hardware-Reduced ACPI"
> >
> > Spec version - I'm guessing ACPI spec 5.0.
>
> 6.3
>
> > And I think here is where you refer to
> > Table 5-34 Fixed ACPI Description Table (FADT) Format
>
> Table 5-33 FADT Format
>
> > > + */
> > > + .rev = 5,
> > > + .minor_ver = 1,
> >
> > So 5.1 I am guessing just copied from virt/arm?
>
> Yes.
>
> > > + .flags = ((1 << ACPI_FADT_F_HW_REDUCED_ACPI) |
> > > + (1 << ACPI_FADT_F_RESET_REG_SUP)),
> > > +
> > > + /* Table 5-33 FADT Format -- SLEEP_CONTROL_REG */
> >
> > You need to use the earliest spec version that includes
> > a specific feature - and document which one it is.
>
> Phew. Isn't it easier to just use table and field name then, so it is
> easy to find in whatever version of the spec you have at hand? Also how
> can I figure the earliest spec version easily?
>
> Sometimes the 6.3 spec documents which table version added specific
> fields, sometimes not ...
>
> Is the table version synced with the acpi spec version? Does DSDT v2
> mean the DSDT format was updated for ACPI 2.0 and hasn't changed since?
not necessarily, even if it's pain to check for earliest spec where
feature appeared, it's the best and the least ambiguos comment format to
refference spec, i.e. (do not count on chapter/table numbering/naming being
stable accross versions)
/* Spec name + ver: chapter/table where the feature is described */
something like:
/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefObjectType */
Aml *aml_object_type(Aml *object)
>
> > But the main poit is AcpiFadtData actually has nothing to do with
> > FADT format. It's an abstracted API
>
> The FADT is generated from AcpiFadtData. There is a 1:1 relationship
> between most AcpiFadtData fields and FADT table entries. This isn't
> what I would call "has nothing to do with" ...
>
> > > + xsdt = tables_blob->len;
> > > + build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> > > +
> > > + /* RSDP is in FSEG memory, so allocate it separately */
> > > + {
> > > + AcpiRsdpData rsdp_data = {
> > > + /* Table 5-27 RSDP Structure */
> >
> > RSDP is since ACPI 2.0, table number there is different.
>
> References to ACPI 2.0 are almost useless. ACPI 5.0 is the oldest
> version uefi.org offers for download.
all versions are at (starting from 1.0)
https://uefi.org/acpi/specs
> Guess that underlines the point I made above that referencing specific
> versions of the spec doesn't work very well ...
so far that worked well, effect of requiring spec+version was less
questions during review as reviewer doesn't have to guess where
it's documented.
>
> take care,
> Gerd
>
- [PATCH v5 00/20] microvm: add acpi support, Gerd Hoffmann, 2020/07/07
- [PATCH v5 01/20] microvm: name qboot binary qboot.rom, Gerd Hoffmann, 2020/07/07
- [PATCH v5 05/20] acpi: ged: add x86 device variant., Gerd Hoffmann, 2020/07/07
- [PATCH v5 06/20] acpi: move acpi_dsdt_add_power_button() to ged, Gerd Hoffmann, 2020/07/07
- [PATCH v5 04/20] acpi: ged: add control regs, Gerd Hoffmann, 2020/07/07
- [PATCH v5 02/20] seabios: add microvm config, update build rules, Gerd Hoffmann, 2020/07/07
- [PATCH v5 08/20] microvm/acpi: add minimal acpi support, Gerd Hoffmann, 2020/07/07
[PATCH v5 03/20] seabios: add bios-microvm.bin binary, Gerd Hoffmann, 2020/07/07
[PATCH v5 14/20] x86: move acpi_dev from pc/microvm, Gerd Hoffmann, 2020/07/07
[PATCH v5 20/20] tests/acpi: update expected data files for microvm, Gerd Hoffmann, 2020/07/07
[PATCH v5 07/20] microvm: make virtio irq base runtime configurable, Gerd Hoffmann, 2020/07/07
[PATCH v5 17/20] tests/acpi: allow microvm test data updates., Gerd Hoffmann, 2020/07/07
[PATCH v5 13/20] x86: constify x86_machine_is_*_enabled, Gerd Hoffmann, 2020/07/07
[PATCH v5 12/20] microvm/acpi: disable virtio-mmio cmdline hack, Gerd Hoffmann, 2020/07/07
[PATCH v5 19/20] tests/acpi: add microvm test, Gerd Hoffmann, 2020/07/07