On 24.09.2012, at 14:16, Paul Mackerras wrote:
> 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.
What a shame. Could you please repost a version that only handles 32/64 setting
with the above helper then and leaves 128-bit the way they are implemented now?
> 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.
... that way we can at least make the "common case" of 32-bit and 64-bit
registers error proof and only have to worry more about double-checking the
128-bit ones, which are a lot less.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
