Re: [PATCH 2/2] ACPICA: support Generic Address Structure bit_offset in acpi_read/write

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Wednesday, November 16, 2011, Bjorn Helgaas wrote:
> On Tue, Nov 15, 2011 at 9:49 AM, Thomas Renninger <trenn@suse.de> wrote:
> > On Tuesday, November 15, 2011 04:13:15 PM Bjorn Helgaas wrote:
> >> On Fri, Nov 11, 2011 at 4:05 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> > acpi_read(), acpi_write(), acpi_hw_read(), and acpi_hw_write() currently
> >> > ignore the GAS bit_offset field (but they do warn if it is non-zero).
> >> >
> >> > APEI tables are starting to use non-zero bit_offsets.  APEI uses
> >> > special-purpose apei_exec_read_register() and apei_exec_write_register()
> >> > interfaces that apply the bit_offset.
> >> >
> >> > This patch adds bit_offset support to the generic interfaces, which is
> >> > one small step toward using them instead of the special-purpose APEI ones.
> >>
> >> Eww, brown paper bag time.  Just pretend you never saw this lame
> >> implementation attempt.
> >>
> >> I do think we need to make acpi_read() smart enough to extract a bit
> >> field, but this try doesn't work.
> >
> > As a first step it would be great if Ying's and Myron's patches which
> > afaik conflict get serialized and pushed into an "acpi branch".
> > What the status there?
> 
> Ying's patch ("Add RAM mapping support") fixes a real issue with using
> EINJ, so we need to do something with it.

I kind of agree, but I wonder if page_is_ram() is the right check?

> Myron's patches are a nice
> refactoring, but as far as I know, they don't fix any current issues,
> and there's still a lot of work to hash out how to handle bit_offset,
> bit_width, and access_width.
> 
> I think we should regard Ying's patches as being first in line, and
> Myron's as work in progress.  So I don't think we're ready to try to
> combine them and resolve conflicts yet -- Myron just has to update his
> work to follow whatever Ying does.

Agreed.

> > I'd like to add access width support to the APEI parts on top then.
> 
> We should try very hard to treat APEI generic address structures the
> same way as all others.  If that means some machines need firmware
> upgrades or some sort of quirks to work around BIOS bugs, we might
> have to accept that.  I think a single set of GAS accessors plus a few
> quirks that fix the GAS structures is far better than having
> APEI-specific GAS accessors that are basically tailored to a few
> broken machines.

We can use APEI-specific wrappers around generic GAS accessors, though.

> To make acpi_read/acpi_write truly generic, we really need to nail
> down the semantics of GAS bit_offset, bit_width, and access_width.
> 
> It would be useful to know how Windows deals with those.  I don't
> think we have convincing information about it (all I remember is
> "here's a GAS that looks wrong, but Windows still works," but I don't
> think we know exactly *what* Windows is doing, or even whether it
> looks at the broken GAS).  If we could figure out a way to feed a
> variety of structures to Windows and observe what happens with
> something like qemu, I think we'd learn a lot.

Perhaps.

> Or maybe we could learn enough from a conversation with BIOS writers
> about how they interpret the spec and what they expect to happen with
> non-zero bit_offset and bit_width.

I'm a bit skeptic about that, so to speak. ;-)

Thanks,
Rafael