Re: [kvm-unit-tests PATCH v1 2/2] s390x: add cmm migration test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 :-(:

/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.



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux