Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: Fix HP Omnibook X 14 boot in ACPI mode
To:
Mark Kettenis <mark.kettenis@xs4all.nl>
Cc:
landry@openbsd.org, marcus@nazgul.ch, tech@openbsd.org
Date:
Thu, 23 Jan 2025 12:04:36 +0100

Download raw body.

Thread
> Date: Thu, 26 Dec 2024 19:03:10 +0100
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> 
> > Date: Thu, 26 Dec 2024 14:13:42 +0100
> > From: Mark Kettenis <mark.kettenis@xs4all.nl>
> > 
> > > Date: Thu, 26 Dec 2024 13:21:47 +0100
> > > From: Marcus Glocker <marcus@nazgul.ch>
> > > 
> > > On Thu, Dec 26, 2024 at 10:37:25AM GMT, Mark Kettenis wrote:
> > > 
> > > > > Date: Mon, 23 Dec 2024 19:53:48 +0100
> > > > > From: Marcus Glocker <marcus@nazgul.ch>
> > > > > 
> > > > > landry@ noticed a crash when trying to boot his HP Omnibook X 14 in
> > > > > ACPI mode:
> > > > > 
> > > > > 	acpitz24 at acpi0unsupported access type 0xf05
> > > > > 	02ce Called: \_SB_.IC10.RDEC
> > > > > 	  arg0: 0xffffff8023691c08 cnt:01 stk:00 integer: a9
> > > > > 	53a97 Called: \_SB_.TZ39._TMP
> > > > > 	panic: aml_die aml_rwgsb:2581
> > > > > 
> > > > > 	aml_die at aml_rwgsb+0x348
> > > > > 	aml_rwinde3xfield at aml_rwfield+0x29c
> > > > > 	aml_rwfield at aml_store+0x1cc
> > > > > 	aml_store at aml_parse+0x15b0
> > > > > 
> > > > > Apparently we are not supporting the 'AttribRawProcessBytes' attribute
> > > > > in our DSDT code, which leads to the "unsupported access type" message
> > > > > followed by the panic.
> > > > > 
> > > > > By adding the 'AttribRawProcessBytes' attribute, and handling it the
> > > > > same way as the 'AttribRawBytes' attribute, landry@ can boot the HP
> > > > > Omnibook X 14 also in ACPI mode.
> > > > > 
> > > > > Feedback, OKs?
> > > > 
> > > > I see that you've already committed the diff, but I don't think this
> > > > is right.  The AttribRawProcessBytes call is supposed to be a write
> > > > followed by a read.  But I suspect that AttribRawBytes isn't
> > > > implemented correctly either.
> > > > 
> > > > The documentation of these calls is annoyingly ambigious.  At least I
> > > > can't figure out what the language in the spec means.  I'll have
> > > > another look at this; cross-referencing the SMBus spec might help me
> > > > figuring this out.
> > > 
> > > All right.  Should I already revert this commit then?
> > 
> > Probably best if we leave it in but annotate the implementations with
> > an XXX comment for now.  I'll commit something later today.
> 
> Looking at things again, I think AttribRawBytes is implemented
> correctly.  So only AttribRawProcessBytes needs something.
> 
> landry, can you check that this diff doesn't break things on the
> Omnibook X 14 in ACPI mode?

Still have this diff in one of my trees.  Since landry@ confirmed it
worked just as well, can someone give me an ok?

> Index: dev/acpi/dsdt.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> diff -u -p -r1.272 dsdt.c
> --- dev/acpi/dsdt.c	24 Dec 2024 12:06:34 -0000	1.272
> +++ dev/acpi/dsdt.c	26 Dec 2024 17:59:52 -0000
> @@ -2574,7 +2574,16 @@ aml_rwgsb(struct aml_value *conn, int le
>  			buflen = len;
>  			break;
>  		case 0x0e:	/* AttribRawBytes */
> +			cmdlen = 0;
> +			buflen = len;
> +			break;
>  		case 0x0f:	/* AttribRawProcessBytes */
> +			/*
> +			 * XXX Not implemented yet but used by various
> +			 * WoA laptops.  Force an error status instead
> +			 * of a panic for now.
> +			 */
> +			node = NULL;
>  			cmdlen = 0;
>  			buflen = len;
>  			break;
> 
>