Re: [PATCH] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 22, 2019 at 07:07:30PM +0100, Ard Biesheuvel wrote:
> On Tue, 22 Jan 2019 at 18:32, Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> wrote:
> > Side note (not related to this patch):
> >
> > I do not understand this check:
> >
> > if (bgrt->status & 0xfe) {
> >         pr_notice("Ignoring BGRT: reserved status bits are non-zero %u\n",
> >                bgrt->status);
> >         goto out;
> > }
> >
> > Should not we check against 0xf8 (reserved bits are [7:3]) ? And what
> > about status[0] (valid), should we check that as well ?
> 
> I will let Peter take that question ...

I think this was originally coded from a draft spec, but also in ACPI
6.1A bits [2:1] got defined without updating the "version" field :/, so
if we're going to check them, checking against 0xf8 would be correct -
but it's probably saner just to not check them?  Nothing that's been
defined so far is actually a reason not to expose the data, and it's not
clear that anything in the future would either.  Also, the "valid" bit
is meant only to reflect if the image is on the screen or not.  This was
clarified in one of the ACPI 5.0A spec - the field has been renamed to
"displayed", so as not to confuse people.  Here's what the current ACPI
spec says:

Name    Size    Offset  Description
Status  1       38      1-byte status field indicating current status
                        about the table.
                        Bits [7:3] = Reserved (must be zero)
                        Bits [2:1] = Orientation Offset. These bits
                                     describe the clockwise degree
                                     offset from the image’s default
                                     orientation.
                                     [00] = 0, no offset
                                     [01] = 90
                                     [10] = 180
                                     [11] = 270

And I think 6.1 or 6.1A has accidentally dropped the text:

                        Bit [0] = Displayed. A one indicates the boot
                                  image graphic is displayed.

Further down it says:

        Bit 0 - Displayed
        The status field contains information about the current status
        of the table.  The Valid bit is bit 0 of the lowest byte.  It
        should be set to 1 while the image resource is displayed on the
        screen, and set to 0 while it is not displayed.

Clearly there has been some editing error going on here, and I'll send
in an ECR to fix that up.  It might be worth exposing "orientation" and
"displayed" directly instead of just exposing "status".

Aside from that an Lorenzo's other observations, the patch looks
reasonable to me.

-- 
  Peter



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux