LAI misunderstanding of the Preserve field property(?)

[d-tatianin]

As indicated by LAI's own field test here:

        OperationRegion (IDXF, SystemIO, 0xC00, 0x2)
        Field (IDXF, ByteAcc, NoLock, Preserve) {
            IDX,   8,
            DAT,   8
        }

        IndexField (IDX, DAT, ByteAcc, NoLock, Preserve) {
            REGA,   8,
            REGB,   8,
            REGC,   8,
            REGD,   8,
            REGE,  16,
        }

        Method(_INI)
        {
            //! io-read: pio 8b 0xC00 = 0x0
            //! io-write: pio 8b 0xC00 = 0x2
            //! io-read: pio 8b 0xC01 = 0x0

            //! io-read: pio 8b 0xC00 = 0x0
            //! io-write: pio 8b 0xC00 = 0x2
            //! io-read: pio 8b 0xC01 = 0x0
            //! io-write: pio 8b 0xC01 = 0xAA
            REGC = 0xAA
            
            ...
        }

Preserve means that all bits outside of a field where field size is less than access size must be preserved. This implies that for field size == access size where field bit offset within byte is 0 no preservation of anything is needed. In the latter case, however, LAI still tries to read data only to discard it later.

I'm also not sure what's up with this top part here:

            //! io-read: pio 8b 0xC00 = 0x0
            //! io-write: pio 8b 0xC00 = 0x2
            //! io-read: pio 8b 0xC01 = 0x0

All in all write to REGC here should cause exactly 2 write accesses and 0 read accesses:

write 2 to IDX
write 0xAA to DAT

ACPICA seems to agree on this at least

[qookei]

This is caused by the following code unconditionally doing a read to get the original value.

lai/core/opregion.c

Lines 484 to 491 in a228465

	if (write_flag == FIELD_PRESERVE) {
	if (field->type == LAI_NAMESPACE_FIELD || field->type == LAI_NAMESPACE_BANKFIELD) {
	value = lai_perform_read(field->fld_region_node, access_size, offset);
	} else if (field->type == LAI_NAMESPACE_INDEXFIELD) {
	value = lai_perform_indexfield_read(field, access_size, offset);
	} else {
	lai_panic("Unknown field type in lai_write_field_internal %d", field->type);
	}

One solution would be checking that the write only affects a part of the word, and only doing the read if that is the case.

I'm unsure whether the extra unused read is actually a problem? I suppose it could potentially confuse some hardware that do things on register reads, and it's probably better to stick to what ACPICA does if we want to work on real hardware.

[d-tatianin]

I'm unsure whether the extra unused read is actually a problem? I suppose it could potentially confuse some hardware that do things on register reads, and it's probably better to stick to what ACPICA does if we want to work on real hardware.

Yeah it's pretty much impossible to say, it might trip up some hardware, might not. There are probably some registers cleared by read? Another thing I thought of is if this was some other address space (aka not SystemMemory or SystemIO) but say SMBus. There a spurious read could cause more problems probably. At the very least it's extra wasted cycles or stalls for no reason.