All of lore.kernel.org
 help / color / mirror / Atom feed
* [Devel] AcpiHwRead/AcpiHwWrite regression in 20160318
@ 2016-04-14 14:49 Thomas Faber
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Faber @ 2016-04-14 14:49 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 4869 bytes --]

Hi,

the new AcpiHwRead/AcpiHwWrite implementations have caused a regression
on VirtualBox here. On ReactOS I'm getting an interrupt storm because
interaction with the event status register is no longer working.

The obvious issue that I observe is that the XPm1aStatus register is
now read from/written to in individual 8-bit operations instead of
using the full 16 bit length as before.

VirtualBox does not seem to allow this:
https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk/blob/master/src/VBox/Devices/PC/DevACPI.cpp#L1466

The register description looks like this (which seems as expected):
kd> ?? AcpiGbl_FADT.XPm1aEventBlock
struct acpi_generic_address
+0x000 SpaceId : 0x1 ''
+0x001 BitWidth : 0x20 ' '
+0x002 BitOffset : 0 ''
+0x003 AccessWidth : 0x2 ''
+0x004 Address : 0x4000
kd> ?? AcpiGbl_XPm1aStatus
struct acpi_generic_address
+0x000 SpaceId : 0x1 ''
+0x001 BitWidth : 0x10 ''
+0x002 BitOffset : 0 ''
+0x003 AccessWidth : 0 ''
+0x004 Address : 0x4000

It looks like the AccessWidth needs to be explicitly set to 2 here now
rather than defaulting to 0? Alternatively, zero AccessWidth would have
to lead to a smarter default than just assuming 8-bit reads.

For reference, backtraces for reading this register with versions
20160318 and 20160108 showing the arguments passed to the OSL functions
can be found below.

Assuming my analysis is correct, it's probably faster & more correct for
someone else to fix the issue in the code, but I can provide a patch if
you prefer.

Thanks!
-Thomas



Read New (20160318) -- returns 0xff (and so does the follow-up read on port 0x4001):
kd> kp
ChildEBP RetAddr
f392a228 f8b3eb5a acpi!AcpiOsReadPort(unsigned int64 Address = 0x4000, unsigned int * Value = 0xf392a288, unsigned int Width = 8) [reactos\drivers\bus\acpi\osl.c @ 707]
f392a24c f8b3e26d acpi!AcpiHwReadPort(unsigned int64 Address = 0x4000, unsigned int * Value = 0xf392a288, unsigned int Width = 8)+0x5a [reactos\drivers\bus\acpi\acpica\hardware\hwvalid.c @ 258]
f392a29c f8b3ea43 acpi!AcpiHwRead(unsigned int * Value = 0xf392a2b4, struct acpi_generic_address * Reg = 0xf8b68860)+0x11d [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 231]
f392a2bc f8b3e795 acpi!AcpiHwReadMultiple(unsigned int * Value = 0xf392a2d8, struct acpi_generic_address * RegisterA = 0xf8b68860, struct acpi_generic_address * RegisterB = 0xf8b68690)+0x23 [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 780]
f392a2dc f8b3d17a acpi!AcpiHwRegisterRead(unsigned int RegisterId = 1, unsigned int * ReturnValue = 0xf392a2f0)+0x45 [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 565]
f392a2fc f8b4683a acpi!AcpiEvFixedEventDetect(void)+0x1a [reactos\drivers\bus\acpi\acpica\events\evevent.c @ 248]
f392a30c f8b2f1d2 acpi!AcpiEvSciXruptHandler(void * Context = 0xf50dbff0)+0x1a [reactos\drivers\bus\acpi\acpica\events\evsci.c @ 146]
f392a31c 804f160c acpi!OslIsrStub(struct _KINTERRUPT * Interrupt = 0xf50ddd98, void * ServiceContext = 0x00000000)+0x12 [reactos\drivers\bus\acpi\osl.c @ 541]
f392a350 804f1738 nt!KiChainedDispatch(struct _KTRAP_FRAME * TrapFrame = 0xf392a364, struct _KINTERRUPT * Interrupt = 0xf50ddd98)+0xbc [reactos\ntoskrnl\ke\i386\irqobj.c @ 284]

Read Old (20160108) -- returns 0x0000:
kd> kp
ChildEBP RetAddr
f392a350 f8b3f57a acpi!AcpiOsReadPort(unsigned int64 Address = 0x4000, unsigned int * Value = 0xf392a3bc, unsigned int Width = 0x10) [reactos\drivers\bus\acpi\osl.c @ 707]
f392a374 f8b3efbd acpi!AcpiHwReadPort(unsigned int64 Address = 0x4000, unsigned int * Value = 0xf392a3bc, unsigned int Width = 0x10)+0x5a [reactos\drivers\bus\acpi\acpica\hardware\hwvalid.c @ 258]
f392a3a4 f8b3f463 acpi!AcpiHwRead(unsigned int * Value = 0xf392a3bc, struct acpi_generic_address * Reg = 0xf8b68800)+0x7d [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 207]
f392a3c4 f8b3f1b5 acpi!AcpiHwReadMultiple(unsigned int * Value = 0xf392a3e0, struct acpi_generic_address * RegisterA = 0xf8b68800, struct acpi_generic_address * RegisterB = 0xf8b68630)+0x23 [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 646]
f392a3e4 f8b3d72a acpi!AcpiHwRegisterRead(unsigned int RegisterId = 1, unsigned int * ReturnValue = 0xf392a3f8)+0x45 [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 431]
f392a404 f8b46d6a acpi!AcpiEvFixedEventDetect(void)+0x1a [reactos\drivers\bus\acpi\acpica\events\evevent.c @ 248]
f392a414 f8b2f1c2 acpi!AcpiEvSciXruptHandler(void * Context = 0xf4717ff0)+0x1a [reactos\drivers\bus\acpi\acpica\events\evsci.c @ 146]
f392a424 804f160c acpi!OslIsrStub(struct _KINTERRUPT * Interrupt = 0xf4719d98, void * ServiceContext = 0x00000000)+0x12 [reactos\drivers\bus\acpi\osl.c @ 541]
f392a458 804f1738 nt!KiChainedDispatch(struct _KTRAP_FRAME * TrapFrame = 0xf392a46c, struct _KINTERRUPT * Interrupt = 0xf4719d98)+0xbc [reactos\ntoskrnl\ke\i386\irqobj.c @ 284]

^ permalink raw reply	[flat|nested] 2+ messages in thread
* Re: [Devel] AcpiHwRead/AcpiHwWrite regression in 20160318
@ 2016-04-14 15:48 Moore, Robert
  0 siblings, 0 replies; 2+ messages in thread
From: Moore, Robert @ 2016-04-14 15:48 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 5721 bytes --]

Yes, we know about it.

It will be reverted in the next ACPICA release for now, since we are still working on it.

We want to support arbitrary bit lengths on arbitrary bit boundaries, it is a pain.
Bob


> -----Original Message-----
> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Thomas Faber
> Sent: Thursday, April 14, 2016 7:50 AM
> To: devel(a)acpica.org
> Subject: [Devel] AcpiHwRead/AcpiHwWrite regression in 20160318
> 
> Hi,
> 
> the new AcpiHwRead/AcpiHwWrite implementations have caused a regression on
> VirtualBox here. On ReactOS I'm getting an interrupt storm because
> interaction with the event status register is no longer working.
> 
> The obvious issue that I observe is that the XPm1aStatus register is now
> read from/written to in individual 8-bit operations instead of using the
> full 16 bit length as before.
> 
> VirtualBox does not seem to allow this:
> https://github.com/mdaniel/virtualbox-org-svn-vbox-
> trunk/blob/master/src/VBox/Devices/PC/DevACPI.cpp#L1466
> 
> The register description looks like this (which seems as expected):
> kd> ?? AcpiGbl_FADT.XPm1aEventBlock
> struct acpi_generic_address
> +0x000 SpaceId : 0x1 ''
> +0x001 BitWidth : 0x20 ' '
> +0x002 BitOffset : 0 ''
> +0x003 AccessWidth : 0x2 ''
> +0x004 Address : 0x4000
> kd> ?? AcpiGbl_XPm1aStatus
> struct acpi_generic_address
> +0x000 SpaceId : 0x1 ''
> +0x001 BitWidth : 0x10 ''
> +0x002 BitOffset : 0 ''
> +0x003 AccessWidth : 0 ''
> +0x004 Address : 0x4000
> 
> It looks like the AccessWidth needs to be explicitly set to 2 here now
> rather than defaulting to 0? Alternatively, zero AccessWidth would have to
> lead to a smarter default than just assuming 8-bit reads.
> 
> For reference, backtraces for reading this register with versions
> 20160318 and 20160108 showing the arguments passed to the OSL functions
> can be found below.
> 
> Assuming my analysis is correct, it's probably faster & more correct for
> someone else to fix the issue in the code, but I can provide a patch if
> you prefer.
> 
> Thanks!
> -Thomas
> 
> 
> 
> Read New (20160318) -- returns 0xff (and so does the follow-up read on
> port 0x4001):
> kd> kp
> ChildEBP RetAddr
> f392a228 f8b3eb5a acpi!AcpiOsReadPort(unsigned int64 Address = 0x4000,
> unsigned int * Value = 0xf392a288, unsigned int Width = 8)
> [reactos\drivers\bus\acpi\osl.c @ 707] f392a24c f8b3e26d
> acpi!AcpiHwReadPort(unsigned int64 Address = 0x4000, unsigned int * Value
> = 0xf392a288, unsigned int Width = 8)+0x5a
> [reactos\drivers\bus\acpi\acpica\hardware\hwvalid.c @ 258] f392a29c
> f8b3ea43 acpi!AcpiHwRead(unsigned int * Value = 0xf392a2b4, struct
> acpi_generic_address * Reg = 0xf8b68860)+0x11d
> [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 231] f392a2bc
> f8b3e795 acpi!AcpiHwReadMultiple(unsigned int * Value = 0xf392a2d8, struct
> acpi_generic_address * RegisterA = 0xf8b68860, struct acpi_generic_address
> * RegisterB = 0xf8b68690)+0x23
> [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 780] f392a2dc
> f8b3d17a acpi!AcpiHwRegisterRead(unsigned int RegisterId = 1, unsigned int
> * ReturnValue = 0xf392a2f0)+0x45
> [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 565] f392a2fc
> f8b4683a acpi!AcpiEvFixedEventDetect(void)+0x1a
> [reactos\drivers\bus\acpi\acpica\events\evevent.c @ 248] f392a30c f8b2f1d2
> acpi!AcpiEvSciXruptHandler(void * Context = 0xf50dbff0)+0x1a
> [reactos\drivers\bus\acpi\acpica\events\evsci.c @ 146] f392a31c 804f160c
> acpi!OslIsrStub(struct _KINTERRUPT * Interrupt = 0xf50ddd98, void *
> ServiceContext = 0x00000000)+0x12 [reactos\drivers\bus\acpi\osl.c @ 541]
> f392a350 804f1738 nt!KiChainedDispatch(struct _KTRAP_FRAME * TrapFrame =
> 0xf392a364, struct _KINTERRUPT * Interrupt = 0xf50ddd98)+0xbc
> [reactos\ntoskrnl\ke\i386\irqobj.c @ 284]
> 
> Read Old (20160108) -- returns 0x0000:
> kd> kp
> ChildEBP RetAddr
> f392a350 f8b3f57a acpi!AcpiOsReadPort(unsigned int64 Address = 0x4000,
> unsigned int * Value = 0xf392a3bc, unsigned int Width = 0x10)
> [reactos\drivers\bus\acpi\osl.c @ 707]
> f392a374 f8b3efbd acpi!AcpiHwReadPort(unsigned int64 Address = 0x4000,
> unsigned int * Value = 0xf392a3bc, unsigned int Width = 0x10)+0x5a
> [reactos\drivers\bus\acpi\acpica\hardware\hwvalid.c @ 258]
> f392a3a4 f8b3f463 acpi!AcpiHwRead(unsigned int * Value = 0xf392a3bc,
> struct acpi_generic_address * Reg = 0xf8b68800)+0x7d
> [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 207]
> f392a3c4 f8b3f1b5 acpi!AcpiHwReadMultiple(unsigned int * Value =
> 0xf392a3e0, struct acpi_generic_address * RegisterA = 0xf8b68800, struct
> acpi_generic_address * RegisterB = 0xf8b68630)+0x23
> [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 646]
> f392a3e4 f8b3d72a acpi!AcpiHwRegisterRead(unsigned int RegisterId = 1,
> unsigned int * ReturnValue = 0xf392a3f8)+0x45
> [reactos\drivers\bus\acpi\acpica\hardware\hwregs.c @ 431]
> f392a404 f8b46d6a acpi!AcpiEvFixedEventDetect(void)+0x1a
> [reactos\drivers\bus\acpi\acpica\events\evevent.c @ 248]
> f392a414 f8b2f1c2 acpi!AcpiEvSciXruptHandler(void * Context =
> 0xf4717ff0)+0x1a [reactos\drivers\bus\acpi\acpica\events\evsci.c @ 146]
> f392a424 804f160c acpi!OslIsrStub(struct _KINTERRUPT * Interrupt =
> 0xf4719d98, void * ServiceContext = 0x00000000)+0x12
> [reactos\drivers\bus\acpi\osl.c @ 541]
> f392a458 804f1738 nt!KiChainedDispatch(struct _KTRAP_FRAME * TrapFrame =
> 0xf392a46c, struct _KINTERRUPT * Interrupt = 0xf4719d98)+0xbc
> [reactos\ntoskrnl\ke\i386\irqobj.c @ 284]
> _______________________________________________
> Devel mailing list
> Devel(a)acpica.org
> https://lists.acpica.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-04-14 15:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-14 14:49 [Devel] AcpiHwRead/AcpiHwWrite regression in 20160318 Thomas Faber
  -- strict thread matches above, loose matches on Subject: below --
2016-04-14 15:48 Moore, Robert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.