On Tue, 10 May 2022 15:25:09 +0200
Nico Boehr <nrb@xxxxxxxxxxxxx> wrote:
> On Mon, 2022-05-09 at 15:58 +0200, Claudio Imbrenda wrote:
> > > + for (i = 0; i < NUM_PAGES; i++) {
> > > + switch(i % 4) {
> > > + case 0:
> > > + essa(ESSA_SET_STABLE, (unsigned
> > > long)pagebuf[i]);
> > > + break;
> > > + case 1:
> > > + essa(ESSA_SET_UNUSED, (unsigned
> > > long)pagebuf[i]);
> > > + break;
> > > + case 2:
> > > + essa(ESSA_SET_VOLATILE, (unsigned
> > > long)pagebuf[i]);
> > > + break;
> > > + case 3:
> > > + essa(ESSA_SET_POT_VOLATILE,
> > > (unsigned long)pagebuf[i]);
> > > + break;
> >
> > const int essa_commands[4] = {ESSA_SET_STABLE, ESSA_SET_UNUSED, ...
> >
> > for (i = 0; i < NUM_PAGES; i++)
> > essa(essa_commands[i % 4], ...
> >
> > I think it would look more compact and more readable
>
> I like your idea a lot, but the compiler doesn't :-(:
ouch, I had completely forgotten about that.
>
> /home/nrb/kvm-unit-tests/lib/asm/cmm.h: In function ‘main’:
> /home/nrb/kvm-unit-tests/lib/asm/cmm.h:32:9: error: ‘asm’ operand 2
> probably does not match constraints [-Werror]
> 32 | asm volatile(".insn
> rrf,0xb9ab0000,%[extr_state],%[addr],%[new_state],0" \
> | ^~~
> /home/nrb/kvm-unit-tests/lib/asm/cmm.h:32:9: error: impossible
> constraint in ‘asm’
>
> To satify the "i" constraint, new_state needs to be a compile time
> constant, which it won't be any more with your suggestion
> unfortunately.
>
> We can do crazy stuff like this in cmm.h:
>
> #define __essa(state) \
> asm volatile(".insn
> rrf,0xb9ab0000,%[extr_state],%[addr],%[new_state],0" \
> : [extr_state] "=d" (extr_state) \
> : [addr] "a" (paddr), [new_state] "i"
> (state));
> static unsigned long essa(uint8_t state, unsigned long paddr)
> {
> uint64_t extr_state = 0;
>
> switch(state) {
> case ESSA_SET_STABLE:
> __essa(ESSA_SET_STABLE);
> break;
> case ESSA_SET_UNUSED:
> __essa(ESSA_SET_UNUSED);
> break;
> case ESSA_SET_VOLATILE:
> __essa(ESSA_SET_VOLATILE);
> break;
> case ESSA_SET_POT_VOLATILE:
> __essa(ESSA_SET_POT_VOLATILE);
> break;
> }
>
> return (unsigned long)extr_state;
> }
>
> But that essentially just shifts the readability problem to a different
> file. What do you think?
>
> Or we make essa a marco, which doesn't sound like a particularily
> attractive alternative either.
ok, next less ugly thing: unroll the loop
for (i = 0; i < NUM_PAGES; i += 4) {
essa(ESSA_SET_STABLE, (unsigned long)pagebuf[i]);
essa(ESSA_SET_UNUSED, (unsigned long)pagebuf[i + 1]);
... etc
}
maybe assert(NUM_PAGES % 4 == 0) before that, just for good measure
