Change in ...coreboot[master]: vendorcode/google/chromeos: Use explicit zero check in ACPI code
Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32694
Change subject: vendorcode/google/chromeos: Use explicit zero check in ACPI code ......................................................................
vendorcode/google/chromeos: Use explicit zero check in ACPI code
The ASL 2.0 syntax for "!X" resolves to "LNot(X)" which will evaluate the object as an integer and turn into a boolean. This may not do the right thing if the object is actually a string and it can lead to unexpected behavior.
Instead be specific about the object type and check for zero or an empty string depending on what is being returned.
This fixes an issue where some VPD keys were causing the search to stop and miss subsequent entries.
Change-Id: I1688842964f9c2f81ca31073da9c2d71a8c81767 Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/vendorcode/google/chromeos/acpi/amac.asl M src/vendorcode/google/chromeos/acpi/vpd.asl 2 files changed, 7 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/32694/1
diff --git a/src/vendorcode/google/chromeos/acpi/amac.asl b/src/vendorcode/google/chromeos/acpi/amac.asl index 51159f5..c87862f 100644 --- a/src/vendorcode/google/chromeos/acpi/amac.asl +++ b/src/vendorcode/google/chromeos/acpi/amac.asl @@ -42,6 +42,7 @@ /* Get "dock_passthru" value from RW_VPD */ Local0 = \VPD.VPDF ("RW", "dock_passthru")
+ Local1 = Zero Switch (ToString (Local0)) { Case ("ethernet_mac0") { @@ -55,7 +56,7 @@ Local1 = \VPD.VPDF ("RO", "dock_mac") } } - If (!Local1) { + If (Local1 == Zero) { Return (Zero) } Printf ("MAC address returned from VPD: %o", Local1) diff --git a/src/vendorcode/google/chromeos/acpi/vpd.asl b/src/vendorcode/google/chromeos/acpi/vpd.asl index 3b262f7..8f8b0e5 100644 --- a/src/vendorcode/google/chromeos/acpi/vpd.asl +++ b/src/vendorcode/google/chromeos/acpi/vpd.asl @@ -139,7 +139,7 @@ Local1 <<= 7 Local1 |= Local2 & 0x7f } - If (!Local1) { + If (Local1 == Zero) { Return (Zero) }
@@ -162,7 +162,7 @@ */ Method (VPDS, 0, Serialized) { - Name (VPKV, Package () { Zero, Zero }) + Name (VPKV, Package () { "", "" })
/* Read the VPD type and ensure it is a string */ If (^VPRB () != ^VPES) { @@ -193,14 +193,14 @@ /* End address of VPD region */ ^VEND = ^VPTR + DerefOf (Local0[1])
- If (!^VPTR || !^VEND) { + If (^VPTR == Zero || ^VEND == Zero) { Printf ("Unable to find VPD region") Return (Zero) }
/* Verify VPD info header and save size */ Local0 = VVPD (^VPTR) - If (!Local0) { + If (Local0 == Zero) { Printf ("VPD region %o did not verify", Arg0) Return (Zero) } @@ -213,7 +213,7 @@ While (Local1 != ToString (Arg1)) { Local2 = VPDS () Local1 = DerefOf (Local2[0]) - If (!Local1) { + If (Local1 == "") { Printf ("VPD KEY %o not found", Arg1) Return (Zero) }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32694 )
Change subject: vendorcode/google/chromeos: Use explicit zero check in ACPI code ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32694 )
Change subject: vendorcode/google/chromeos: Use explicit zero check in ACPI code ......................................................................
Patch Set 1:
Probably need to do the same for rest of the !X uses:
src/ec/google/wilco/acpi/battery.asl: If (!Local2) { src/ec/google/wilco/acpi/battery.asl: If (!Local0) { src/ec/google/wilco/acpi/ec.asl: If (!EREG) {
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32694 )
Change subject: vendorcode/google/chromeos: Use explicit zero check in ACPI code ......................................................................
Patch Set 1:
Patch Set 1:
Probably need to do the same for rest of the !X uses:
src/ec/google/wilco/acpi/battery.asl: If (!Local2) { src/ec/google/wilco/acpi/battery.asl: If (!Local0) { src/ec/google/wilco/acpi/ec.asl: If (!EREG) {
Yeah I wanted to do some more testing on these as they weren't breaking anything yet like this one.
Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32694 )
Change subject: vendorcode/google/chromeos: Use explicit zero check in ACPI code ......................................................................
vendorcode/google/chromeos: Use explicit zero check in ACPI code
The ASL 2.0 syntax for "!X" resolves to "LNot(X)" which will evaluate the object as an integer and turn into a boolean. This may not do the right thing if the object is actually a string and it can lead to unexpected behavior.
Instead be specific about the object type and check for zero or an empty string depending on what is being returned.
This fixes an issue where some VPD keys were causing the search to stop and miss subsequent entries.
Change-Id: I1688842964f9c2f81ca31073da9c2d71a8c81767 Signed-off-by: Duncan Laurie dlaurie@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32694 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/vendorcode/google/chromeos/acpi/amac.asl M src/vendorcode/google/chromeos/acpi/vpd.asl 2 files changed, 7 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/vendorcode/google/chromeos/acpi/amac.asl b/src/vendorcode/google/chromeos/acpi/amac.asl index 51159f5..c87862f 100644 --- a/src/vendorcode/google/chromeos/acpi/amac.asl +++ b/src/vendorcode/google/chromeos/acpi/amac.asl @@ -42,6 +42,7 @@ /* Get "dock_passthru" value from RW_VPD */ Local0 = \VPD.VPDF ("RW", "dock_passthru")
+ Local1 = Zero Switch (ToString (Local0)) { Case ("ethernet_mac0") { @@ -55,7 +56,7 @@ Local1 = \VPD.VPDF ("RO", "dock_mac") } } - If (!Local1) { + If (Local1 == Zero) { Return (Zero) } Printf ("MAC address returned from VPD: %o", Local1) diff --git a/src/vendorcode/google/chromeos/acpi/vpd.asl b/src/vendorcode/google/chromeos/acpi/vpd.asl index 3b262f7..8f8b0e5 100644 --- a/src/vendorcode/google/chromeos/acpi/vpd.asl +++ b/src/vendorcode/google/chromeos/acpi/vpd.asl @@ -139,7 +139,7 @@ Local1 <<= 7 Local1 |= Local2 & 0x7f } - If (!Local1) { + If (Local1 == Zero) { Return (Zero) }
@@ -162,7 +162,7 @@ */ Method (VPDS, 0, Serialized) { - Name (VPKV, Package () { Zero, Zero }) + Name (VPKV, Package () { "", "" })
/* Read the VPD type and ensure it is a string */ If (^VPRB () != ^VPES) { @@ -193,14 +193,14 @@ /* End address of VPD region */ ^VEND = ^VPTR + DerefOf (Local0[1])
- If (!^VPTR || !^VEND) { + If (^VPTR == Zero || ^VEND == Zero) { Printf ("Unable to find VPD region") Return (Zero) }
/* Verify VPD info header and save size */ Local0 = VVPD (^VPTR) - If (!Local0) { + If (Local0 == Zero) { Printf ("VPD region %o did not verify", Arg0) Return (Zero) } @@ -213,7 +213,7 @@ While (Local1 != ToString (Arg1)) { Local2 = VPDS () Local1 = DerefOf (Local2[0]) - If (!Local1) { + If (Local1 == "") { Printf ("VPD KEY %o not found", Arg1) Return (Zero) }
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)