On 14 Dec 2011, at 17:13, Sasha Levin <levinsasha928@xxxxxxxxx> wrote:
> On Wed, 2011-12-14 at 13:06 +1100, Matt Evans wrote:
>> Hi Sasha,
>>
>> On 12/12/11 06:50, Sasha Levin wrote:
>>> Just set delivery mode directly without going through ugly casting.
>>>
>>> This cleans up and simplifies the code.
>>>
>>> Signed-off-by: Sasha Levin <levinsasha928@xxxxxxxxx>
>>> ---
>>> tools/kvm/x86/kvm-cpu.c | 10 ++--------
>>> 1 files changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tools/kvm/x86/kvm-cpu.c b/tools/kvm/x86/kvm-cpu.c
>>> index 27b7a8f..cc1f560 100644
>>> --- a/tools/kvm/x86/kvm-cpu.c
>>> +++ b/tools/kvm/x86/kvm-cpu.c
>>> @@ -81,18 +81,12 @@ static int kvm_cpu__set_lint(struct kvm_cpu *vcpu)
>>> {
>>> struct kvm_lapic_state klapic;
>>> struct local_apic *lapic = (void *)&klapic;
>>> - u32 lvt;
>>>
>>> if (ioctl(vcpu->vcpu_fd, KVM_GET_LAPIC, &klapic))
>>> return -1;
>>>
>>> - lvt = *(u32 *)&lapic->lvt_lint0;
>>> - lvt = SET_APIC_DELIVERY_MODE(lvt, APIC_MODE_EXTINT);
>>> - *(u32 *)&lapic->lvt_lint0 = lvt;
>>> -
>>> - lvt = *(u32 *)&lapic->lvt_lint1;
>>> - lvt = SET_APIC_DELIVERY_MODE(lvt, APIC_MODE_NMI);
>>> - *(u32 *)&lapic->lvt_lint1 = lvt;
>>> + lapic->lvt_lint0.delivery_mode = APIC_MODE_EXTINT;
>>> + lapic->lvt_lint1.delivery_mode = APIC_MODE_NMI;
>>>
>>> return ioctl(vcpu->vcpu_fd, KVM_SET_LAPIC, &klapic);
>>> }
>>
>> I'm getting this on x86-32, gcc 4.4.3:
>>
>> CC x86/kvm-cpu.o
>> cc1: warnings being treated as errors
>> x86/kvm-cpu.c: In function ‘kvm_cpu__set_lint’:
>> x86/kvm-cpu.c:89: error: dereferencing pointer ‘lapic’ does break strict-aliasing rules
>> x86/kvm-cpu.c:88: error: dereferencing pointer ‘lapic’ does break strict-aliasing rules
>> x86/kvm-cpu.c:83: note: initialized from here
>> make: *** [x86/kvm-cpu.o] Error 1
>>
>> Removing the nasty aliasing (patch below) seems to be a good way to go. What do
>> you think?
>>
>>
>> Cheers,
>>
>>
>> Matt
>>
>> ---
>>
>> diff --git a/tools/kvm/x86/kvm-cpu.c b/tools/kvm/x86/kvm-cpu.c
>> index cc1f560..30f1ad6 100644
>> --- a/tools/kvm/x86/kvm-cpu.c
>> +++ b/tools/kvm/x86/kvm-cpu.c
>> @@ -79,16 +79,15 @@ void kvm_cpu__delete(struct kvm_cpu *vcpu)
>>
>> static int kvm_cpu__set_lint(struct kvm_cpu *vcpu)
>> {
>> - struct kvm_lapic_state klapic;
>> - struct local_apic *lapic = (void *)&klapic;
>> + struct local_apic lapic;
>>
>> - if (ioctl(vcpu->vcpu_fd, KVM_GET_LAPIC, &klapic))
>> + if (ioctl(vcpu->vcpu_fd, KVM_GET_LAPIC, &lapic))
>> return -1;
>
> It felt a bit wrong to me getting the ioctl to assign it directly to
> local_apic since api.txt says it should be returning a kvm_lapic_state.
> But on the other hand, api.txt also says "The data format and layout are
> the same as documented in the architecture manual." so I guess we can go
> with that.
>
> Acked-by: Sasha Levin <levinsasha928@xxxxxxxxx>
Pekka, as I forgot the SOB:
Signed-off-by: Matt Evans <matt@xxxxxxxxxx>
(Or would you prefer a repost so it's all in one?)
Matt--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
