Alex, On 5/7/19 9:16 AM, Graf, Alexander wrote: > [CAUTION: External Email] > > On 03.05.19 15:37, Suthikulpanit, Suravee wrote: >> Current logic does not allow VCPU to be loaded onto CPU with >> APIC ID 255. This should be allowed since the host physical APIC ID >> field in the AVIC Physical APIC table entry is an 8-bit value, >> and APIC ID 255 is valid in system with x2APIC enabled. >> >> Instead, do not allow VCPU load if the host APIC ID cannot be >> represented by an 8-bit value. >> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> > > Your comment for AVIC_MAX_PHYSICAL_ID_COUNT says that 0xff (255) is > broadcast hence you disallow that value. In fact, even the comment a few > lines above the patch hunk does say that. Why the change of mind? Actually, I would need to make change to that comment to remove the mentioning of 255 as broadcast. I will send out V2 with proper comment fix. The reason is because on system w/ x2APIC, the APIC ID 255 is actually non-broadcast, and this should be allowed. The code here should not need to check for broadcast. Thanks, Suravee > Alex > >> --- >> arch/x86/kvm/svm.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 294448e..122788f 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -2071,7 +2071,11 @@ static void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu) >> if (!kvm_vcpu_apicv_active(vcpu)) >> return; >> >> - if (WARN_ON(h_physical_id >= AVIC_MAX_PHYSICAL_ID_COUNT)) >> + /* >> + * Since the host physical APIC id is 8 bits, >> + * we can support host APIC ID upto 255. >> + */ >> + if (WARN_ON(h_physical_id > AVIC_MAX_PHYSICAL_ID_COUNT)) >> return; >> >> entry = READ_ONCE(*(svm->avic_physical_id_cache)); >> >