* [PATCH] svm/avic: Allow avic_vcpu_load logic to support host APIC ID 255
@ 2019-05-03 13:37 Suthikulpanit, Suravee
2019-05-07 14:16 ` Graf, Alexander
0 siblings, 1 reply; 4+ messages in thread
From: Suthikulpanit, Suravee @ 2019-05-03 13:37 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro, Suthikulpanit, Suravee
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@amd.com>
---
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));
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] svm/avic: Allow avic_vcpu_load logic to support host APIC ID 255
2019-05-03 13:37 [PATCH] svm/avic: Allow avic_vcpu_load logic to support host APIC ID 255 Suthikulpanit, Suravee
@ 2019-05-07 14:16 ` Graf, Alexander
2019-05-09 17:39 ` Suthikulpanit, Suravee
0 siblings, 1 reply; 4+ messages in thread
From: Graf, Alexander @ 2019-05-07 14:16 UTC (permalink / raw)
To: Suthikulpanit, Suravee, linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro
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@amd.com>
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?
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));
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] svm/avic: Allow avic_vcpu_load logic to support host APIC ID 255
2019-05-07 14:16 ` Graf, Alexander
@ 2019-05-09 17:39 ` Suthikulpanit, Suravee
2019-05-09 21:03 ` Suthikulpanit, Suravee
0 siblings, 1 reply; 4+ messages in thread
From: Suthikulpanit, Suravee @ 2019-05-09 17:39 UTC (permalink / raw)
To: Graf, Alexander, linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro
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@amd.com>
>
> 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));
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] svm/avic: Allow avic_vcpu_load logic to support host APIC ID 255
2019-05-09 17:39 ` Suthikulpanit, Suravee
@ 2019-05-09 21:03 ` Suthikulpanit, Suravee
0 siblings, 0 replies; 4+ messages in thread
From: Suthikulpanit, Suravee @ 2019-05-09 21:03 UTC (permalink / raw)
To: Graf, Alexander, linux-kernel, kvm; +Cc: pbonzini, rkrcmar, joro
Alex,
Actually, a second thought on this patch, I should have been using
AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK --->
On 5/9/19 12:39 PM, Suthikulpanit, Suravee wrote:
> [CAUTION: External Email]
>
> 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@amd.com>
>>
>> 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))
^ HERE
It does the same thing, but would be easier to understand.
The AVIC_MAX_PHYSICAL_ID_COUNT is meant for representing the max number
of entry allowed for AVIC physical APIC ID table, which is a different thing.
I'll send out V2.
Suravee
>>> return;
>>>
>>> entry = READ_ONCE(*(svm->avic_physical_id_cache));
>>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-09 21:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-03 13:37 [PATCH] svm/avic: Allow avic_vcpu_load logic to support host APIC ID 255 Suthikulpanit, Suravee
2019-05-07 14:16 ` Graf, Alexander
2019-05-09 17:39 ` Suthikulpanit, Suravee
2019-05-09 21:03 ` Suthikulpanit, Suravee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).