ref: e77fa31516bc5dc834f1caf1f4ed3d888a9aa810
parent: 472958e3e7313cbe0cdb8d482045c2be87a23659
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Mon Feb 22 14:27:49 EST 2021
libaml: fix IndexField and BankField implementations (thanks Michael Forney)
IndexField is supposed to increment the index value when an
access is done with a bigger size than the data field.
The index value is always a byte offset.
Now that we always calculate the offset for each field unit
access for IndexField, rename the indexv to bank (the bank
value), as it is only used for that. Also, do not compare
it with nil, as it is a integer constant which can be
encoded as nil to mean zero.
For BankField, the banking field was written using store(),
which does nothing when the destination is a Field*.
Use rwfield() to fix it in the new rwfieldunit().
Resolve all the Name*'s when IndexField, BankField and
Field are created. Now, Field.reg points to eigther
Buffer object, Region or Field (data Field of an IndexField).
PS: initial bug report by Michael Forney follows below:
In /dev/kmesg on my T14, I saw a message
amlmapio: [0xffffff18-0x100000018] overlaps usable memory
amlmapio: mapping \_SB.FRTP failed
Here is the relevant snippet from my DSDT:
Scope (_SB)
{
...
OperationRegion (ECMC, SystemIO, 0x72, 0x02)
Field (ECMC, AnyAcc, NoLock, Preserve)
{
ECMI, 8,
ECMD, 8
}
IndexField (ECMI, ECMD, ByteAcc, NoLock, Preserve)
{
Offset (0x08),
FRTB, 32
}
OperationRegion (FRTP, SystemMemory, FRTB, 0x0100)
Field (FRTP, AnyAcc, NoLock, Preserve)
{
...
}
}
With some debugging output:
amlmapio(\_SB.ECMC): Io 72 - 74
rwreg(\_SB.ECMC): Io [72+0]/1 <- 8
rwreg(\_SB.ECMC): Io [72+1]/1 -> 18
amlmapio(\_SB.FRTP): Mem ffffff18 - 100000018
amlmapio: [0xffffff18-0x100000018) overlaps usable memory
amlmapio: mapping \_SB.FRTP failed
It seems that libaml does not handle IndexField correctly and just did
a single read from ECMD after setting ECMI to 8, causing the FRTP
region to be evaluated as 0xffffff18-0x100000018. Instead, it should
be reading 4 bytes [18 c0 22 cc], evaluating it as
0xcc22c018-0xcc22118:
amlmapio(\_SB.ECMC): Io 72 - 74
rwreg(\_SB.ECMC): Io [72+0]/1 <- 8
rwreg(\_SB.ECMC): Io [72+1]/1 -> 18
rwreg(\_SB.ECMC): Io [72+0]/1 <- 9
rwreg(\_SB.ECMC): Io [72+1]/1 -> c0
rwreg(\_SB.ECMC): Io [72+0]/1 <- a
rwreg(\_SB.ECMC): Io [72+1]/1 -> 22
rwreg(\_SB.ECMC): Io [72+0]/1 <- b
rwreg(\_SB.ECMC): Io [72+1]/1 -> cc
amlmapio(\_SB.FRTP): Mem cc22c018 - cc22c118
I wrote a patch (attached) to fix this, and it seems to work. Though,
it's not clear to me when things should be dereferenced. Previously,
the data field was dereferenced at evalfield, but the region and index
field were not until rwfield. After the patch, the index field is
also dereferenced in evalfield.
For BankField, the index *is* dereferenced in evalfield. I'm pretty
sure that this means that BankField does not work currently, since
store() just returns nil for 'f' objects. The bank selector will
never get set.
Anyway, I don't know if this solves any real problems; it's just
something I noticed and thought I'd try to fix.
--- a/sys/src/libaml/aml.c
+++ b/sys/src/libaml/aml.c
@@ -70,9 +70,9 @@
};
struct Field {- void *reg; /* Buffer or Region */
- Field *index;
- void *indexv;
+ void *reg; /* Buffer or Region or data Field */
+ void *bank; /* bank value */
+ Field *index; /* bank or index Field */
int flags;
int bitoff;
int bitlen;
@@ -217,8 +217,8 @@
case 'u':
f = p;
gcmark(f->reg);
+ gcmark(f->bank);
gcmark(f->index);
- gcmark(f->indexv);
break;
}
}
@@ -608,18 +608,46 @@
}
}
+static void *rwfield(Field *f, void *v, int write);
+
+static uvlong
+rwfieldunit(Field *f, int off, int len, uvlong v, int write)
+{+ if(f->index){+ if(TAG(f->reg) == 'f'){+ void *b;
+
+ /* set index field */
+ rwfield(f->index, mki(off), 1);
+
+ /* set data field */
+ f = f->reg;
+ if(write){+ b = mk('b', len);+ putle(b, len, v);
+ rwfield(f, b, 1);
+ }else{+ b = rwfield(f, nil, 0);
+ v = getle(b, len);
+ }
+ return v;
+ }
+
+ /* set bank field */
+ rwfield(f->index, f->bank, 1);
+ }
+ return rwreg(f->reg, off, len, v, write);
+}
+
static void*
rwfield(Field *f, void *v, int write)
{int boff, blen, wo, ws, wl, wa, wd, i;
uvlong w, m;
- void *reg;
uchar *b;
- if(f == nil || (reg = deref(f->reg)) == nil)
+ if(f == nil)
return nil;
- if(f->index)
- store(f->indexv, f->index);
blen = f->bitlen;
if(write){ if(v && TAG(v) == 'b'){@@ -649,11 +677,11 @@
w <<= ws;
if(wl != wd){m = ((1ULL<<wl)-1) << ws;
- w |= rwreg(reg, wo*wa, wa, 0, 0) & ~m;
+ w |= rwfieldunit(f, wo*wa, wa, 0, 0) & ~m;
}
- rwreg(reg, wo*wa, wa, w, 1);
+ rwfieldunit(f, wo*wa, wa, w, 1);
} else {- w = rwreg(reg, wo*wa, wa, 0, 0) >> ws;
+ w = rwfieldunit(f, wo*wa, wa, 0, 0) >> ws;
for(i = 0; i < wl; i++, boff++){b[boff/8] |= (w&1)<<(boff%8);
w >>= 1;
@@ -937,9 +965,14 @@
/* no break */
case 'f':
l = p;
- if(l->index)
- return fmtprint(f, "IndexField(%x, %x, %x) @ %V[%V]",
- l->flags, l->bitoff, l->bitlen, l->index, l->indexv);
+ if(l->index){+ if(TAG(l->reg) == 'f')
+ return fmtprint(f, "IndexField(%x, %x, %x) @ %V[%V]",
+ l->flags, l->bitoff, l->bitlen, l->reg, l->index);
+ else
+ return fmtprint(f, "BankField(%x, %x, %x, %V=%V) @ %V",
+ l->flags, l->bitoff, l->bitlen, l->index, l->bank, l->reg);
+ }
return fmtprint(f, "Field(%x, %x, %x) @ %V",
l->flags, l->bitoff, l->bitlen, l->reg);
default:
@@ -1374,28 +1407,36 @@
static void*
evalfield(void)
{- int flags, bitoff, wa, n;
- Field *f, *df;
+ int flags, bitoff, n;
+ Field *f, *index;
+ void *reg, *bank;
Name *d;
uchar *p;
- df = nil;
- flags = 0;
+ bank = nil;
+ index = nil;
bitoff = 0;
switch(FP->op - optab){+ default:
+ goto Out;
case Ofld:
+ if((reg = deref(FP->arg[0])) == nil || TAG(reg) != 'r')
+ goto Out;
flags = ival(FP->arg[1]);
break;
case Oxfld:
- df = deref(FP->arg[1]);
- if(df == nil || TAG(df) != 'f')
+ if((index = deref(FP->arg[0])) == nil || TAG(index) != 'f')
goto Out;
+ if((reg = deref(FP->arg[1])) == nil || TAG(reg) != 'f') /* data field */
+ goto Out;
flags = ival(FP->arg[2]);
break;
case Obfld:
- df = deref(FP->arg[1]);
- if(df == nil || TAG(df) != 'f')
+ if((reg = deref(FP->arg[0])) == nil || TAG(reg) != 'r')
goto Out;
+ if((index = deref(FP->arg[1])) == nil || TAG(index) != 'f')
+ goto Out;
+ bank = FP->arg[2];
flags = ival(FP->arg[3]);
break;
}
@@ -1425,25 +1466,10 @@
f = mk('f', sizeof(Field));f->flags = flags;
f->bitlen = n;
- switch(FP->op - optab){- case Ofld:
- f->reg = FP->arg[0];
- f->bitoff = bitoff;
- break;
- case Oxfld:
- wa = fieldalign(df->flags);
- f->reg = df->reg;
- f->bitoff = df->bitoff + (bitoff % (wa*8));
- f->indexv = mki((bitoff/(wa*8))*wa);
- f->index = FP->arg[0];
- break;
- case Obfld:
- f->reg = FP->arg[0];
- f->bitoff = bitoff;
- f->indexv = FP->arg[2];
- f->index = df;
- break;
- }
+ f->bitoff = bitoff;
+ f->reg = reg;
+ f->bank = bank;
+ f->index = index;
bitoff += n;
d->v = f;
}