Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1542072

Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with kvm_{x,x2}apic_id

Show all headers | View raw

>> think I'd even prefer here a simple
>>
>> aid = kvm_xapic_id(apic);
>> if (apic_x2apic_mode(apic))
>> 	aid = kvm_x2apic_id(apic);
>>
>> that would keep changes minimal and I don't really see any benefit in the
>> code when splitting handling up.
>
> It is neccesassary to write an entry for both IDs and I wanted to split
> it before [4/4], because doing both changes at once seemed hard to
> grasp.
>
> Putting it here didn't work well either ... is a separate patch for the
> hunk below better, or would you prefer to have it in [4/4]?

I actually would prefer to have it in 4/4, but not sure if it is worth 
yet another round. Anyhow,

Reviewed-by: David Hildenbrand <david@redhat.com>

for this patch (with the 256 fixed)

>
>> Patch 4 then simply can fixup setting code
>>
>> if (aid <= new->max_apic_id && !new->phys_map[aid])
>> 	new->phys_map[aid] = apic;
>>
>> (if I am not missing some important corner case here)
>
> The trick is that we want to do the following even in xAPIC mode:
>
>   new->phys_map[kvm_x2apic_id(apic)] = apic;
>
> This is the main idea of the hotplug hack -- to allow unique addressing
> of processors that were reset in xAPIC mode.  (And I add a disgusting
> "x2apic_id > 0xff" condition in [4/4], because we still allow guests to
> change xAPIC IDs, which wouldn't play nice with this.)
>
> Hardware does a superset of this, because it only looks at lower 8 bits
> of the desination ID when delivering to xAPIC.
>
> When kvm_x2apic_id(apic) != kvm_xapic_id(apic), then the APIC is in
> xAPIC mode so we definitely want to keep xAPIC working, hence
>
>   if (!apic_x2apic_mode(apic))
>   	new->phys_map[kvm_xapic_id(apic)] = apic;

Okay, so this is is the case I missed in patch 4 :)

Thanks for the explanation and sorry for the noise.

-- 

David

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v2 0/4] KVM: x86: allow hotplug of VCPU with APIC ID over 0xff Radim Krčmář <rkrcmar@redhat.com> - 2016-12-13 17:40 +0100
  [PATCH v2 3/4] KVM: x86: make interrupt delivery fast and slow path behave the same Radim Krčmář <rkrcmar@redhat.com> - 2016-12-13 17:40 +0100
  [PATCH v2 4/4] KVM: x86: allow hotplug of VCPU with APIC ID over 0xff Radim Krčmář <rkrcmar@redhat.com> - 2016-12-13 17:40 +0100
  [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with kvm_{x,x2}apic_id Radim Krčmář <rkrcmar@redhat.com> - 2016-12-13 17:40 +0100
    Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id David Hildenbrand <david@redhat.com> - 2016-12-14 13:40 +0100
      Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id Radim Krčmář <rkrcmar@redhat.com> - 2016-12-14 14:50 +0100
        Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id Paolo Bonzini <pbonzini@redhat.com> - 2016-12-14 17:00 +0100
    Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id David Hildenbrand <david@redhat.com> - 2016-12-14 17:20 +0100
      Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id Paolo Bonzini <pbonzini@redhat.com> - 2016-12-14 18:00 +0100
        Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id Radim Krčmář <rkrcmar@redhat.com> - 2016-12-14 18:20 +0100
      Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id Radim Krčmář <rkrcmar@redhat.com> - 2016-12-14 18:20 +0100
        Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with  kvm_{x,x2}apic_id David Hildenbrand <david@redhat.com> - 2016-12-14 18:30 +0100

csiph-web