> -----Original Message-----
> From: linux-acpi-owner@xxxxxxxxxxxxxxx <linux-acpi-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Schmauss, Erik
> Sent: Tuesday, November 26, 2019 4:26 PM
> To: Maximilian Luz <luzmaximilian@xxxxxxxxx>; Moore, Robert
> <robert.moore@xxxxxxxxx>; Wysocki, Rafael J
> <rafael.j.wysocki@xxxxxxxxx>
> Cc: linux-acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx
> 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@xxxxxxxxx>
Date: Tue, 3 Dec 2019 12:42:51 -0800
From: Erik Schmauss <erik.schmauss@xxxxxxxxx>
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@xxxxxxxxx>
Signed-off-by: Erik Schmauss <erik.schmauss@xxxxxxxxx>
---
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
>
