Re: [PATCH v3 1/2] KVM: PPC: Book3S: Get/set guest SPRs using the GET/SET_ONE_REG interface

[Paul Mackerras <paulus@samba.org>]

On Fri, Sep 21, 2012 at 11:15:51AM +0200, Alexander Graf wrote:

> So how about something like
> 
> #define kvmppc_set_reg(id, val, reg) { \
>   switch (one_reg_size(id)) { \
>   case 4: val.wval = reg; break; \
>   case 8: val.dval = reg; break; \
>   default: BUG(); \
>   } \
> }
> 
> case KVM_REG_PPC_DAR:
>   kvmppc_set_reg(reg->id, &val, vcpu->arch.shared->dar);
>   break;

I tried that and it was fine for the wval vs. dval thing, and gcc even
compiled it to the same number of instructions as what I had before.
But it breaks down when you get to VMX and VSX -- firstly because
there are two different types that are both 16 bytes, and secondly
because when you do this:

#define kvmppc_get_reg(id, val, reg) { \
  switch (one_reg_size(id)) { \
  case 4: val.wval = reg; break; \
  case 8: val.dval = reg; break; \
  case 16: val.vval = reg; break; \
  default: BUG(); \
  } \
}

you get compile errors on things like:

  kvmppc_set_reg(reg->id, val, vcpu->arch.shared->dar);

because val.vval is a vector128, and vcpu->arch.shared->dar is a u64,
and you can't assign a u64 to a vector128 since vector128 is a struct.
In other words, all of the cases of the switch have to satisfy the
type rules even though only one of them will ever be used.  Basically
that trick will only work for integer types, and we don't have 128 bit
integer support in gcc.

Given all that, I would like to see my patches go in while we continue
to search for a way to avoid the potential mistakes you're talking
about.

Paul.