* Re: PROBLEM: Calling ObjectType on buffer field reports type integer [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C595C50@ORSMSX122.amr.corp.intel.com> @ 2019-07-20 20:45 ` Maximilian Luz 2019-07-22 23:01 ` Schmauss, Erik 0 siblings, 1 reply; 14+ messages in thread From: Maximilian Luz @ 2019-07-20 20:45 UTC (permalink / raw) To: Schmauss, Erik, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel On 3/22/19 10:28 PM, Schmauss, Erik wrote: > I've been discussing this with Bob. I've decided to file a bug against Microsoft > internally. I would like to wait and see what they say... I've never done such > things so I don't know how long the process takes to get a response form them. > > I'll post updates when I get them but feel free to ping us again in a few > weeks if you don't hear back. We're still investigating their operation region > behavior as well... > > Erik Hi, I assume there hasn't been any response from Microsoft? Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-07-20 20:45 ` PROBLEM: Calling ObjectType on buffer field reports type integer Maximilian Luz @ 2019-07-22 23:01 ` Schmauss, Erik 2019-07-23 11:38 ` Maximilian Luz 2019-11-10 21:29 ` Maximilian Luz 0 siblings, 2 replies; 14+ messages in thread From: Schmauss, Erik @ 2019-07-22 23:01 UTC (permalink / raw) To: Maximilian Luz, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel > -----Original Message----- > From: Maximilian Luz [mailto:luzmaximilian@gmail.com] > Sent: Saturday, July 20, 2019 1:45 PM > To: Schmauss, Erik <erik.schmauss@intel.com>; Moore, Robert > <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com> > Cc: linux-acpi@vger.kernel.org; devel@acpica.org > Subject: Re: PROBLEM: Calling ObjectType on buffer field reports type integer > > On 3/22/19 10:28 PM, Schmauss, Erik wrote: > > I've been discussing this with Bob. I've decided to file a bug against > > Microsoft internally. I would like to wait and see what they say... > > I've never done such things so I don't know how long the process takes to get > a response form them. > > > > I'll post updates when I get them but feel free to ping us again in a > > few weeks if you don't hear back. We're still investigating their > > operation region behavior as well... > > > > Erik > > Hi, > > I assume there hasn't been any response from Microsoft? Sorry about the late response. This slipped through the cracks. I've sent them an email just now and I'll keep you informed Thanks, Erik > > Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-07-22 23:01 ` Schmauss, Erik @ 2019-07-23 11:38 ` Maximilian Luz 2019-11-10 21:29 ` Maximilian Luz 1 sibling, 0 replies; 14+ messages in thread From: Maximilian Luz @ 2019-07-23 11:38 UTC (permalink / raw) To: Schmauss, Erik, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel On 7/23/19 1:01 AM, Schmauss, Erik wrote: > Sorry about the late response. This slipped through the cracks. > I've sent them an email just now and I'll keep you informed No worries. Thank you. Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-07-22 23:01 ` Schmauss, Erik 2019-07-23 11:38 ` Maximilian Luz @ 2019-11-10 21:29 ` Maximilian Luz 2019-11-22 17:11 ` Moore, Robert 1 sibling, 1 reply; 14+ messages in thread From: Maximilian Luz @ 2019-11-10 21:29 UTC (permalink / raw) To: Schmauss, Erik, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel On 7/23/19 1:01 AM, Schmauss, Erik wrote: > Sorry about the late response. This slipped through the cracks. > I've sent them an email just now and I'll keep you informed Hi again, is there any update on this? Regards, Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-11-10 21:29 ` Maximilian Luz @ 2019-11-22 17:11 ` Moore, Robert 2019-11-22 22:07 ` Moore, Robert 0 siblings, 1 reply; 14+ messages in thread From: Moore, Robert @ 2019-11-22 17:11 UTC (permalink / raw) To: Maximilian Luz, Schmauss, Erik, Wysocki, Rafael J; +Cc: linux-acpi, devel We will probably make this change, depending on what Windows does. Bob -----Original Message----- From: Maximilian Luz <luzmaximilian@gmail.com> Sent: Sunday, November 10, 2019 1:30 PM To: Schmauss, Erik <erik.schmauss@intel.com>; Moore, Robert <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com> Cc: linux-acpi@vger.kernel.org; devel@acpica.org Subject: Re: PROBLEM: Calling ObjectType on buffer field reports type integer On 7/23/19 1:01 AM, Schmauss, Erik wrote: > Sorry about the late response. This slipped through the cracks. > I've sent them an email just now and I'll keep you informed Hi again, is there any update on this? Regards, Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-11-22 17:11 ` Moore, Robert @ 2019-11-22 22:07 ` Moore, Robert 2019-11-22 23:00 ` Maximilian Luz 2019-11-22 23:29 ` Schmauss, Erik 0 siblings, 2 replies; 14+ messages in thread From: Moore, Robert @ 2019-11-22 22:07 UTC (permalink / raw) To: Moore, Robert, Maximilian Luz, Schmauss, Erik, Wysocki, Rafael J Cc: linux-acpi, devel I'm not seeing this problem. For example: Method (DS01) { Name(BUFZ, Buffer(48){}) CreateField(BUFZ, 192, 69, DST0) Store (ObjectType (DST0), Debug) } Acpiexec dsdt.aml Eval DS01 Evaluating \DS01 ACPI Debug: 0x000000000000000E 0x0E is in fact type "buffer field". -----Original Message----- From: Moore, Robert <robert.moore@intel.com> Sent: Friday, November 22, 2019 9:12 AM To: Maximilian Luz <luzmaximilian@gmail.com>; Schmauss, Erik <erik.schmauss@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com> Cc: linux-acpi@vger.kernel.org; devel@acpica.org Subject: [Devel] Re: PROBLEM: Calling ObjectType on buffer field reports type integer We will probably make this change, depending on what Windows does. Bob -----Original Message----- From: Maximilian Luz <luzmaximilian@gmail.com> Sent: Sunday, November 10, 2019 1:30 PM To: Schmauss, Erik <erik.schmauss@intel.com>; Moore, Robert <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com> Cc: linux-acpi@vger.kernel.org; devel@acpica.org Subject: Re: PROBLEM: Calling ObjectType on buffer field reports type integer On 7/23/19 1:01 AM, Schmauss, Erik wrote: > Sorry about the late response. This slipped through the cracks. > I've sent them an email just now and I'll keep you informed Hi again, is there any update on this? Regards, Maximilian _______________________________________________ Devel mailing list -- devel@acpica.org To unsubscribe send an email to devel-leave@acpica.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-11-22 22:07 ` Moore, Robert @ 2019-11-22 23:00 ` Maximilian Luz 2019-11-22 23:29 ` Schmauss, Erik 1 sibling, 0 replies; 14+ messages in thread From: Maximilian Luz @ 2019-11-22 23:00 UTC (permalink / raw) To: Moore, Robert, Schmauss, Erik, Wysocki, Rafael J; +Cc: linux-acpi, devel On 11/22/19 11:07 PM, Moore, Robert wrote: > I'm not seeing this problem. For example: > Method (DS01) > { > Name(BUFZ, Buffer(48){}) > CreateField(BUFZ, 192, 69, DST0) > Store (ObjectType (DST0), Debug) > } > > Acpiexec dsdt.aml > Eval DS01 > Evaluating \DS01 > ACPI Debug: 0x000000000000000E > > 0x0E is in fact type "buffer field". Hi, unfortunately, the bug is a bit more convoluted, here's a minimal example: Method (DS01) { Name(BUFZ, Buffer(48){}) CreateField(BUFZ, 192, 32, DST0) Local0 = DST0 Store (ObjectType (Local0), Debug) } It is based on the combination of CreateField with length smaller or equal to the maximum integer size and the assignment of the field to a local variable. For me, this yields Eval DS01 Evaluating \DS01 ACPI Debug: 0x0000000000000001 As already iterated, this should be the correct behavior according to spec, but causes failures on Microsoft Surface devices. For reference, I've added two links to the relevant parts in the DSDT of the MS Surface Book 2: In short, MS identifies errors by the type of a local variable which is supposed to either contain a payload (buffer) or an error value (integer). This local variable (Local4) is previously assigned from a buffer field [1] containing some payload data. However on Linux, if the payload data is smaller than or equal to 64 bits, the type of the variable is integer and the transaction gets misidentified as having failed [2]. [1]: https://github.com/qzed/surfacebook2-dsdt/blob/fa0ca7c7cba8fb0da1e79c282f9a9f4a12eaaa17/dsdt.dsl#L15396-L15397 [2]: https://github.com/qzed/surfacebook2-dsdt/blob/fa0ca7c7cba8fb0da1e79c282f9a9f4a12eaaa17/dsdt.dsl#L15427 In hindsight, the subject line is not describing the actual problem very well, I apologize for that. Thanks, Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-11-22 22:07 ` Moore, Robert 2019-11-22 23:00 ` Maximilian Luz @ 2019-11-22 23:29 ` Schmauss, Erik 2019-11-23 0:33 ` Maximilian Luz 1 sibling, 1 reply; 14+ messages in thread From: Schmauss, Erik @ 2019-11-22 23:29 UTC (permalink / raw) To: Moore, Robert, Maximilian Luz, Wysocki, Rafael J; +Cc: linux-acpi, devel Bob and I started debugging this and we found the issue: Let's say that we have this code: Name (BUF1, Buffer (0x10) {}) Method (M001) { CreateField (BUF1, 1, 72, FLD0) local0 = FLD0 // BUG: store operator (aka =) converts FLD0 into an integer return (ObjectType (local0)) // Integer is returned } Although FLD0's value is small enough to fit in an integer, the bit length of FLD0 exceeds 64 bits so local0 should actually be a Buffer type. This is likely an issue in the implicit object conversion rules implemented in the store operator. I'll take a look at this next week or the week after... Thanks for your patience, Erik > -----Original Message----- > From: Moore, Robert <robert.moore@intel.com> > Sent: Friday, November 22, 2019 2:07 PM > To: Moore, Robert <robert.moore@intel.com>; Maximilian Luz > <luzmaximilian@gmail.com>; Schmauss, Erik <erik.schmauss@intel.com>; > Wysocki, Rafael J <rafael.j.wysocki@intel.com> > Cc: linux-acpi@vger.kernel.org; devel@acpica.org > Subject: RE: PROBLEM: Calling ObjectType on buffer field reports type integer > > I'm not seeing this problem. For example: > Method (DS01) > { > Name(BUFZ, Buffer(48){}) > CreateField(BUFZ, 192, 69, DST0) > Store (ObjectType (DST0), Debug) > } > > Acpiexec dsdt.aml > Eval DS01 > Evaluating \DS01 > ACPI Debug: 0x000000000000000E > > 0x0E is in fact type "buffer field". > > > -----Original Message----- > From: Moore, Robert <robert.moore@intel.com> > Sent: Friday, November 22, 2019 9:12 AM > To: Maximilian Luz <luzmaximilian@gmail.com>; Schmauss, Erik > <erik.schmauss@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com> > Cc: linux-acpi@vger.kernel.org; devel@acpica.org > Subject: [Devel] Re: PROBLEM: Calling ObjectType on buffer field reports type > integer > > We will probably make this change, depending on what Windows does. > Bob > > > -----Original Message----- > From: Maximilian Luz <luzmaximilian@gmail.com> > Sent: Sunday, November 10, 2019 1:30 PM > To: Schmauss, Erik <erik.schmauss@intel.com>; Moore, Robert > <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com> > Cc: linux-acpi@vger.kernel.org; devel@acpica.org > Subject: Re: PROBLEM: Calling ObjectType on buffer field reports type integer > > > On 7/23/19 1:01 AM, Schmauss, Erik wrote: > > Sorry about the late response. This slipped through the cracks. > > I've sent them an email just now and I'll keep you informed > > Hi again, > > is there any update on this? > > Regards, > > Maximilian > _______________________________________________ > Devel mailing list -- devel@acpica.org > To unsubscribe send an email to devel-leave@acpica.org > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-11-22 23:29 ` Schmauss, Erik @ 2019-11-23 0:33 ` Maximilian Luz 2019-11-27 0:25 ` Schmauss, Erik 0 siblings, 1 reply; 14+ messages in thread From: Maximilian Luz @ 2019-11-23 0:33 UTC (permalink / raw) To: Schmauss, Erik, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel On 11/23/19 12:29 AM, Schmauss, Erik wrote: > Bob and I started debugging this and we found the issue: > > Let's say that we have this code: > > Name (BUF1, Buffer (0x10) {}) > Method (M001) > { > CreateField (BUF1, 1, 72, FLD0) > local0 = FLD0 // BUG: store operator (aka =) converts FLD0 into an integer > return (ObjectType (local0)) // Integer is returned > } > > Although FLD0's value is small enough to fit in an integer, the bit length of FLD0 exceeds 64 bits so local0 should actually be a Buffer type. > > This is likely an issue in the implicit object conversion rules implemented in the store operator. I'll take a look at this next week or the week after... This looks like a separate problem to me. On the SB2 there is this piece of code, simplified: Name(RQBF, Buffer (0xFF) {}) CreateByteField (RQBF, 0x03, ALEN) // ... // GenericSerialBus/AttribRawProcessBytes access to fill RQBF // ... If (/* success and everything is valid */) { Local3 = (ALEN * 0x08) CreateField (RQBF, 0x20, Local3, ARB) Local4 = ARB /* \_SB_._SAN.RQSX.ARB_ */ } Else { Local4 = 0x01 // or some other error code as integer } // ... // some more stuff // ... If ((ObjectType (Local4) == One /* Integer */)) { // notify that an error has occurred } Else { // success and actually use data from Local4 } The code in question basically unpacks a payload from some other management stuff sent over the OperationRegion. Here, ALEN is the length of a dynamically sized payload in bytes, which is obtained from the data returned by the OperationRegion access. This can for example be 4, making the field length 32 bit. So this is not an issue of the field length being larger than intmax bits, it actually is sometimes only 32 bits, or 8 bits, depending on the response of the driver connected to the OperationRegion. Also the DSDT depends on that, see the example below. Just to reiterate, the code works fine for payloads with ALEN > 8 (so more than 8 bytes), but fails for anything less. Also note that this is not something that can be fixed by just telling the GenericSerialBus/OperationRegion driver to just return 9 bytes instead: There are length-checks on Local4 further down the line to validate it actually contains what was requested. An example of how this piece of code is actually used, if that helps (again simplified): Method (RQST, 1) { // pretty much the code above Return (Local4) // either payload or integer error code } Scope (_SB.BAT1) { Method (_STA, 0) { Local0 = RQST(0x01) // request battery status If ((ObjectType (Local0) == 0x03)) // is buffer type { If ((SizeOf (Local0) == 0x04)) // has length 4 { CreateDWordField (Local0, 0, BAST) Return (BAST) } } Return (0x00) // return default value } } Regards, Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-11-23 0:33 ` Maximilian Luz @ 2019-11-27 0:25 ` Schmauss, Erik 2019-12-03 22:21 ` Kaneda, Erik 0 siblings, 1 reply; 14+ messages in thread From: Schmauss, Erik @ 2019-11-27 0:25 UTC (permalink / raw) To: Maximilian Luz, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel > > This looks like a separate problem to me. On the SB2 there is this piece of code, > simplified: > > Name(RQBF, Buffer (0xFF) {}) > CreateByteField (RQBF, 0x03, ALEN) > > // ... > // GenericSerialBus/AttribRawProcessBytes access to fill RQBF > // ... > > If (/* success and everything is valid */) > { > Local3 = (ALEN * 0x08) > CreateField (RQBF, 0x20, Local3, ARB) > Local4 = ARB /* \_SB_._SAN.RQSX.ARB_ */ > } > Else > { > Local4 = 0x01 // or some other error code as integer > } > > // ... > // some more stuff > // ... > > If ((ObjectType (Local4) == One /* Integer */)) > { > // notify that an error has occurred > } > Else > { > // success and actually use data from Local4 > } > > The code in question basically unpacks a payload from some other > management stuff sent over the OperationRegion. > > Here, ALEN is the length of a dynamically sized payload in bytes, which is > obtained from the data returned by the OperationRegion access. This can for > example be 4, making the field length 32 bit. So this is not an issue of the field > length being larger than intmax bits, it actually is sometimes only 32 bits, or 8 > bits, depending on the response of the driver connected to the > OperationRegion. Also the DSDT depends on that, see the example below. > > Just to reiterate, the code works fine for payloads with ALEN > 8 (so more than > 8 bytes), but fails for anything less. Forget what I said in the previous email. You are correct. They treat bufferfields that are small enough to fit inside of an integer as a buffer or bufferfield. I did a bunch of testing on windows and determined that, objects created by Create(Bit|Byte|Word|DWord|Qword)Field work the same way on both interpreters. They get converted to Integers as outlined in the spec. We simply need to stop perform the BufferField->Integer conversion for buffer fields created by the ASL CreateField() operators. I'll get a solution hopefully sometime next week.. Thanks, Erik > > Also note that this is not something that can be fixed by just telling the > GenericSerialBus/OperationRegion driver to just return 9 bytes > instead: There are length-checks on Local4 further down the line to validate it > actually contains what was requested. > > An example of how this piece of code is actually used, if that helps (again > simplified): > > Method (RQST, 1) > { > // pretty much the code above > Return (Local4) // either payload or integer error code > } > > Scope (_SB.BAT1) > { > Method (_STA, 0) > { > Local0 = RQST(0x01) // request battery status > > If ((ObjectType (Local0) == 0x03)) // is buffer type > { > If ((SizeOf (Local0) == 0x04)) // has length 4 > { > CreateDWordField (Local0, 0, BAST) > Return (BAST) > } > } > > Return (0x00) // return default value > } > } > > > Regards, > Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-11-27 0:25 ` Schmauss, Erik @ 2019-12-03 22:21 ` Kaneda, Erik 2019-12-04 19:33 ` Maximilian Luz 0 siblings, 1 reply; 14+ messages in thread From: Kaneda, Erik @ 2019-12-03 22:21 UTC (permalink / raw) To: Kaneda, Erik, Maximilian Luz, Moore, Robert, Wysocki, Rafael J Cc: linux-acpi, devel > -----Original Message----- > From: linux-acpi-owner@vger.kernel.org <linux-acpi- > owner@vger.kernel.org> On Behalf Of Schmauss, Erik > Sent: Tuesday, November 26, 2019 4:26 PM > To: Maximilian Luz <luzmaximilian@gmail.com>; Moore, Robert > <robert.moore@intel.com>; Wysocki, Rafael J > <rafael.j.wysocki@intel.com> > Cc: linux-acpi@vger.kernel.org; devel@acpica.org > Subject: RE: PROBLEM: Calling ObjectType on buffer field reports > type integer > > > > > This looks like a separate problem to me. On the SB2 there is > this > > piece of code, > > simplified: > > > > Name(RQBF, Buffer (0xFF) {}) > > CreateByteField (RQBF, 0x03, ALEN) > > > > // ... > > // GenericSerialBus/AttribRawProcessBytes access to fill RQBF > > // ... > > > > If (/* success and everything is valid */) > > { > > Local3 = (ALEN * 0x08) > > CreateField (RQBF, 0x20, Local3, ARB) > > Local4 = ARB /* \_SB_._SAN.RQSX.ARB_ */ > > } > > Else > > { > > Local4 = 0x01 // or some other error code as integer > > } > > > > // ... > > // some more stuff > > // ... > > > > If ((ObjectType (Local4) == One /* Integer */)) > > { > > // notify that an error has occurred > > } > > Else > > { > > // success and actually use data from Local4 > > } > > > > The code in question basically unpacks a payload from some other > > management stuff sent over the OperationRegion. > > > > Here, ALEN is the length of a dynamically sized payload in bytes, > > which is obtained from the data returned by the > OperationRegion > > access. This can for example be 4, making the field length 32 bit. > So > > this is not an issue of the field length being larger than intmax > > bits, it actually is sometimes only 32 bits, or 8 bits, depending > on > > the response of the driver connected to the OperationRegion. > Also the DSDT depends on that, see the example below. > > > > Just to reiterate, the code works fine for payloads with ALEN > 8 > (so > > more than > > 8 bytes), but fails for anything less. > > Forget what I said in the previous email. You are correct. > > They treat bufferfields that are small enough to fit inside of an > integer as a buffer or bufferfield. > > I did a bunch of testing on windows and determined that, objects > created by Create(Bit|Byte|Word|DWord|Qword)Field > work the same way on both interpreters. They get converted to > Integers as outlined in the spec. > > We simply need to stop perform the BufferField->Integer > conversion for buffer fields created by the ASL CreateField() > operators. > I'll get a solution hopefully sometime next week.. > Hi Maximilian, Please try the patch below: From 9a949c05253e36f69451c0e1af4f862d76820c8a Mon Sep 17 00:00:00 2001 From: Erik Schmauss <erik.schmauss@intel.com> Date: Tue, 3 Dec 2019 12:42:51 -0800 From: Erik Schmauss <erik.schmauss@intel.com> Subject: [PATCH 1] ACPICA: Dispatcher: always generate buffer objects for ASL create_field() operator According to table 19-419 of the ACPI 6.3 specification, buffer_fields created using the ASL create_field() Operator have been treated as integers if the buffer_field is small enough to fit inside of an ASL integer (32-bits or 64-bits depending on the definition block revision). If they are larger, buffer fields are treated as ASL Buffer objects. However, this is not true for other AML interpreter implementations. It has been discovered that other AML interpreters always treat buffer fields created by create_field() as a buffer regardless of the length of the buffer field. This change prohibits the previously mentioned integer conversion to match other AML interpreter implementations. Reported-by: Maximilian Luz <luzmaximilian@gmail.com> Signed-off-by: Erik Schmauss <erik.schmauss@intel.com> --- acobject.h | 3 ++- dsutils.c | 3 +++ exfield.c | 10 ++++++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff -Nurp linux.before_name/drivers/acpi/acpica/acobject.h linux.after_name/drivers/acpi/acpica/acobject.h --- linux.before_name/drivers/acpi/acpica/acobject.h 2019-12-03 13:36:21.802578716 -0800 +++ linux.after_name/drivers/acpi/acpica/acobject.h 2019-12-03 13:36:18.267579061 -0800 @@ -259,7 +259,8 @@ struct acpi_object_index_field { /* The buffer_field is different in that it is part of a Buffer, not an op_region */ struct acpi_object_buffer_field { - ACPI_OBJECT_COMMON_HEADER ACPI_COMMON_FIELD_INFO union acpi_operand_object *buffer_obj; /* Containing Buffer object */ + ACPI_OBJECT_COMMON_HEADER ACPI_COMMON_FIELD_INFO u8 is_create_field; /* Special case for objects created by create_field() */ + union acpi_operand_object *buffer_obj; /* Containing Buffer object */ }; /****************************************************************************** diff -Nurp linux.before_name/drivers/acpi/acpica/dsutils.c linux.after_name/drivers/acpi/acpica/dsutils.c --- linux.before_name/drivers/acpi/acpica/dsutils.c 2019-12-03 13:36:21.782578718 -0800 +++ linux.after_name/drivers/acpi/acpica/dsutils.c 2019-12-03 13:36:17.765579110 -0800 @@ -470,6 +470,9 @@ acpi_ds_create_operand(struct acpi_walk_ ACPI_CAST_PTR(union acpi_operand_object, walk_state->deferred_node); status = AE_OK; + if (walk_state->opcode == AML_CREATE_FIELD_OP) { + obj_desc->buffer_field.is_create_field = TRUE; + } } else { /* All other opcodes */ /* diff -Nurp linux.before_name/drivers/acpi/acpica/exfield.c linux.after_name/drivers/acpi/acpica/exfield.c --- linux.before_name/drivers/acpi/acpica/exfield.c 2019-12-03 13:36:21.791578717 -0800 +++ linux.after_name/drivers/acpi/acpica/exfield.c 2019-12-03 13:36:18.257579062 -0800 @@ -95,7 +95,8 @@ acpi_ex_get_protocol_buffer_length(u32 p * RETURN: Status * * DESCRIPTION: Read from a named field. Returns either an Integer or a - * Buffer, depending on the size of the field. + * Buffer, depending on the size of the field and whether if a + * field is created by the create_field() operator. * ******************************************************************************/ @@ -153,12 +154,17 @@ acpi_ex_read_data_from_field(struct acpi * the use of arithmetic operators on the returned value if the * field size is equal or smaller than an Integer. * + * However, all buffer fields created by create_field operator needs to + * remain as a buffer to match other AML interpreter implementations. + * * Note: Field.length is in bits. */ buffer_length = (acpi_size)ACPI_ROUND_BITS_UP_TO_BYTES(obj_desc->field.bit_length); - if (buffer_length > acpi_gbl_integer_byte_width) { + if (buffer_length > acpi_gbl_integer_byte_width || + (obj_desc->common.type == ACPI_TYPE_BUFFER_FIELD && + obj_desc->buffer_field.is_create_field)) { /* Field is too large for an Integer, create a Buffer instead */ > Thanks, > Erik > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-12-03 22:21 ` Kaneda, Erik @ 2019-12-04 19:33 ` Maximilian Luz 2019-12-05 23:57 ` Kaneda, Erik 0 siblings, 1 reply; 14+ messages in thread From: Maximilian Luz @ 2019-12-04 19:33 UTC (permalink / raw) To: Kaneda, Erik, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel On 12/3/19 11:21 PM, Kaneda, Erik wrote: > Hi Maximilian, > > Please try the patch below: Hi, I've tested this on 5.4.1 and the current upstream master now. Unfortunately it doesn't seem to fix the issue. Specifically, it seems like the flag doesn't get set here (printk-debugging indicates that the code in the if never runs): > /****************************************************************************** > diff -Nurp linux.before_name/drivers/acpi/acpica/dsutils.c linux.after_name/drivers/acpi/acpica/dsutils.c > --- linux.before_name/drivers/acpi/acpica/dsutils.c 2019-12-03 13:36:21.782578718 -0800 > +++ linux.after_name/drivers/acpi/acpica/dsutils.c 2019-12-03 13:36:17.765579110 -0800 > @@ -470,6 +470,9 @@ acpi_ds_create_operand(struct acpi_walk_ > ACPI_CAST_PTR(union acpi_operand_object, > walk_state->deferred_node); > status = AE_OK; > + if (walk_state->opcode == AML_CREATE_FIELD_OP) { > + obj_desc->buffer_field.is_create_field = TRUE; > + } > } else { /* All other opcodes */ > > /* That of course means that the if condition here is still not fulfilled, so the problem is not fixed. > diff -Nurp linux.before_name/drivers/acpi/acpica/exfield.c linux.after_name/drivers/acpi/acpica/exfield.c > --- linux.before_name/drivers/acpi/acpica/exfield.c 2019-12-03 13:36:21.791578717 -0800 > +++ linux.after_name/drivers/acpi/acpica/exfield.c 2019-12-03 13:36:18.257579062 -0800 > @@ -153,12 +154,17 @@ acpi_ex_read_data_from_field(struct acpi > * the use of arithmetic operators on the returned value if the > * field size is equal or smaller than an Integer. > * > + * However, all buffer fields created by create_field operator needs to > + * remain as a buffer to match other AML interpreter implementations. > + * > * Note: Field.length is in bits. > */ > buffer_length = > (acpi_size)ACPI_ROUND_BITS_UP_TO_BYTES(obj_desc->field.bit_length); > > - if (buffer_length > acpi_gbl_integer_byte_width) { > + if (buffer_length > acpi_gbl_integer_byte_width || > + (obj_desc->common.type == ACPI_TYPE_BUFFER_FIELD && > + obj_desc->buffer_field.is_create_field)) { > > /* Field is too large for an Integer, create a Buffer instead */ > If I'm not mistaken I've proposed a patch here that is conceptually quite similar (but may have some flaws or cause other issues, as I'm not really familiar with the ACPICA code-base). My idea was to modify acpi_ds_init_buffer_field (dsopcode.c) instead of acpi_ds_create_operand. As said, I'm not sure however if this may have any other consequences that I'm unaware of. Also I've avoided creating an is_create_field flag by changing the field access from AML_FIELD_ACCESS_BYTE to AML_FIELD_ACCESS_BUFFER (which doesn't seem to be used anywhere else) and checking that, but the general idea of extending the if condition in acpi_ex_read_data_from_field is the same. Link to the patch: https://github.com/qzed/linux-surfacegen5-acpi/blob/2014964bac52f138109443de3e92dc2c6cff5e83/patches/5.3/0001-ACPI-Fix-buffer-integer-type-mismatch.patch Thanks, Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-12-04 19:33 ` Maximilian Luz @ 2019-12-05 23:57 ` Kaneda, Erik 2019-12-06 13:48 ` Maximilian Luz 0 siblings, 1 reply; 14+ messages in thread From: Kaneda, Erik @ 2019-12-05 23:57 UTC (permalink / raw) To: Maximilian Luz, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel > -----Original Message----- > From: Maximilian Luz <luzmaximilian@gmail.com> > Sent: Wednesday, December 4, 2019 11:34 AM > To: Kaneda, Erik <erik.kaneda@intel.com>; Moore, Robert > <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com> > Cc: linux-acpi@vger.kernel.org; devel@acpica.org > Subject: Re: PROBLEM: Calling ObjectType on buffer field reports type integer > > On 12/3/19 11:21 PM, Kaneda, Erik wrote: > > Hi Maximilian, > > > > Please try the patch below: > > Hi, > > I've tested this on 5.4.1 and the current upstream master now. > Unfortunately it doesn't seem to fix the issue. > > Specifically, it seems like the flag doesn't get set here (printk-debugging > indicates that the code in the if never runs): > > > > > > /****************************************************************** > *** > > ********* diff -Nurp linux.before_name/drivers/acpi/acpica/dsutils.c > > linux.after_name/drivers/acpi/acpica/dsutils.c > > --- linux.before_name/drivers/acpi/acpica/dsutils.c 2019-12-03 > 13:36:21.782578718 -0800 > > +++ linux.after_name/drivers/acpi/acpica/dsutils.c 2019-12-03 > 13:36:17.765579110 -0800 > > @@ -470,6 +470,9 @@ acpi_ds_create_operand(struct acpi_walk_ > > ACPI_CAST_PTR(union acpi_operand_object, > > walk_state->deferred_node); > > status = AE_OK; > > + if (walk_state->opcode == AML_CREATE_FIELD_OP) { > > + obj_desc->buffer_field.is_create_field = TRUE; > > + } > > } else { /* All other opcodes */ > > > > /* > > That of course means that the if condition here is still not fulfilled, so the > problem is not fixed. > > > diff -Nurp linux.before_name/drivers/acpi/acpica/exfield.c > linux.after_name/drivers/acpi/acpica/exfield.c > > --- linux.before_name/drivers/acpi/acpica/exfield.c 2019-12-03 > 13:36:21.791578717 -0800 > > +++ linux.after_name/drivers/acpi/acpica/exfield.c 2019-12-03 > 13:36:18.257579062 -0800 > > @@ -153,12 +154,17 @@ acpi_ex_read_data_from_field(struct acpi > > * the use of arithmetic operators on the returned value if the > > * field size is equal or smaller than an Integer. > > * > > + * However, all buffer fields created by create_field operator needs to > > + * remain as a buffer to match other AML interpreter implementations. > > + * > > * Note: Field.length is in bits. > > */ > > buffer_length = > > > > (acpi_size)ACPI_ROUND_BITS_UP_TO_BYTES(obj_desc->field.bit_length); > > > > - if (buffer_length > acpi_gbl_integer_byte_width) { > > + if (buffer_length > acpi_gbl_integer_byte_width || > > + (obj_desc->common.type == ACPI_TYPE_BUFFER_FIELD && > > + obj_desc->buffer_field.is_create_field)) { > > > > /* Field is too large for an Integer, create a Buffer instead */ > > > > If I'm not mistaken I've proposed a patch here that is conceptually quite similar > (but may have some flaws or cause other issues, as I'm not really familiar with > the ACPICA code-base). My idea was to modify acpi_ds_init_buffer_field > (dsopcode.c) instead of acpi_ds_create_operand. As said, I'm not sure however > if this may have any other consequences that I'm unaware of. > > Also I've avoided creating an is_create_field flag by changing the field access > from AML_FIELD_ACCESS_BYTE to AML_FIELD_ACCESS_BUFFER (which doesn't > seem to be used anywhere else) and checking that, but the general idea of > extending the if condition in acpi_ex_read_data_from_field is the same. > > Link to the patch: > https://github.com/qzed/linux-surfacegen5- > acpi/blob/2014964bac52f138109443de3e92dc2c6cff5e83/patches/5.3/0001- > ACPI-Fix-buffer-integer-type-mismatch.patch Your solution looks very similar to my patch. In AcpiDsInitBufferField, I would prefer to use a new field "is_create_field" or something like that. AML_FIELD_ACCESS_BUFFER does get used in other parts of ACPICA so let's create and use a new field in the object union. Go ahead and re-submit the patch with the above changes. I'll edit your explanation with more details and include this in the next ACPICA release. It'll land in linux after the release. Thanks! Erik > > Thanks, > Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: PROBLEM: Calling ObjectType on buffer field reports type integer 2019-12-05 23:57 ` Kaneda, Erik @ 2019-12-06 13:48 ` Maximilian Luz 0 siblings, 0 replies; 14+ messages in thread From: Maximilian Luz @ 2019-12-06 13:48 UTC (permalink / raw) To: Kaneda, Erik, Moore, Robert, Wysocki, Rafael J; +Cc: linux-acpi, devel On 12/6/19 12:57 AM, Kaneda, Erik wrote: > Your solution looks very similar to my patch. > In AcpiDsInitBufferField, I would prefer to use a new field "is_create_field" or something like that. AML_FIELD_ACCESS_BUFFER > does get used in other parts of ACPICA so let's create and use a new field in the object union. > > Go ahead and re-submit the patch with the above changes. I'll edit your explanation with more details and include this in > the next ACPICA release. It'll land in linux after the release. Thank you! I've re-submitted the patch with the requested changes and included your comments. Regards, Maximilian ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-12-06 13:48 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <3ef42aa1-196d-f3db-0e5d-2fd84c198242@gmail.com> [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C592D47@ORSMSX122.amr.corp.intel.com> [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C59405C@ORSMSX122.amr.corp.intel.com> [not found] ` <fe4bcc1c-5c15-caa6-ce01-a5df962ff008@gmail.com> [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C5942CA@ORSMSX122.amr.corp.intel.com> [not found] ` <51e156ec-c2ed-84be-13c0-99a213e1d4b7@gmail.com> [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C595C50@ORSMSX122.amr.corp.intel.com> 2019-07-20 20:45 ` PROBLEM: Calling ObjectType on buffer field reports type integer Maximilian Luz 2019-07-22 23:01 ` Schmauss, Erik 2019-07-23 11:38 ` Maximilian Luz 2019-11-10 21:29 ` Maximilian Luz 2019-11-22 17:11 ` Moore, Robert 2019-11-22 22:07 ` Moore, Robert 2019-11-22 23:00 ` Maximilian Luz 2019-11-22 23:29 ` Schmauss, Erik 2019-11-23 0:33 ` Maximilian Luz 2019-11-27 0:25 ` Schmauss, Erik 2019-12-03 22:21 ` Kaneda, Erik 2019-12-04 19:33 ` Maximilian Luz 2019-12-05 23:57 ` Kaneda, Erik 2019-12-06 13:48 ` Maximilian Luz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).