[eternalcomet]

When a field targets a PCI_Config operation region, resolving _SEG, _BBN, or _ADR can execute AML and re-enter do_store.

The fix resolves those addresses before taking object_token, caches the result on each region, then reacquires the token for the field access. Preserve read-modify-write and the two-step BankField/IndexField accesses remain serialized. This follows ACPICA's region setup (evregion.c).

The address cache makes later changes to _ADR take effect only for a new region, and a concurrent CopyObject can leave an in-flight store targeting the field it cloned. The cache is a private field on OpRegion, so this is a semver change for 7.0.0-pre1.

The full test suite passes, including test_region_in_pci_device_with_method_adr.

[martin-hughes]

Thank you @eternalcomet! Nice test.

I want to think a bit more about the locking. Clearly keeping an interpreter-wide lock through all of this tree is bad:

do_store                       <- takes object_token (src/aml/mod.rs:2477)
└─ do_field_write              (:2512)
   └─ do_native_region_write   (:2749)
      └─ pci_address_for_device (:2885)
         └─ evaluate_if_present(_ADR)   <- _ADR is a Method
            └─ do_execute_method
               └─ do_binary_maths
                  └─ do_store   <- takes object_token again: same-thread deadlock

But if we release the lock I think that exposes us to the possibility of two threads writing to the same variable at once (or a read + concurrent write), which could be unsound.

[eternalcomet]

The field access still needs the object token. I changed the fix so the token is released only while resolving a PCI device's _SEG, _BBN, and _ADR; it is reacquired before do_field_write. Preserve read-modify-write and the two-step BankField/IndexField accesses therefore stay serialized against other stores: field-store path.

This follows ACPICA's region setup, which exits the interpreter around RegionSetup and marks the region initialized afterward (evregion.c); the PCI setup in evrgnini.c is where _ADR, _SEG, and _BBN get evaluated. The address is cached per region, which also removes the old pci_address_for_device TODO.

The trade-offs are that a concurrent CopyObject can leave this store targeting the field it cloned, and a later change to _ADR is ignored after the region is cached. The private cache field also makes OpRegion struct literals a semver change; the crate is already 7.0.0-pre1.

Field reads don't take the token on main today, and this PR doesn't change that side.

The full test suite passes, including test_region_in_pci_device_with_method_adr.