diff mbox series

svm/avic: Fix invalidate logical APIC id entry

Message ID 20190322101706.9377-1-suravee.suthikulpanit@amd.com (mailing list archive)
State New, archived
Headers show
Series svm/avic: Fix invalidate logical APIC id entry | expand

Commit Message

Suthikulpanit, Suravee March 22, 2019, 10:17 a.m. UTC
Only clear the valid bit when invalidate logical APIC id entry.
The current logic clear the valid bit, but also set the rest of
the bits (including reserved bits) to 1.

Fixes: 98d90582be2e ('svm: Fix AVIC DFR and LDR handling')
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/kvm/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vitaly Kuznetsov March 22, 2019, 1:04 p.m. UTC | #1
"Suthikulpanit, Suravee" <Suravee.Suthikulpanit@amd.com> writes:

> Only clear the valid bit when invalidate logical APIC id entry.
> The current logic clear the valid bit, but also set the rest of
> the bits (including reserved bits) to 1.
>
> Fixes: 98d90582be2e ('svm: Fix AVIC DFR and LDR handling')
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/kvm/svm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 7a4ce1a22ca0..f4fb766e474c 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4640,7 +4640,7 @@ static void avic_invalidate_logical_id_entry(struct kvm_vcpu *vcpu)
>  	u32 *entry = avic_get_logical_id_entry(vcpu, svm->ldr_reg, flat);
>  
>  	if (entry)
> -		WRITE_ONCE(*entry, (u32) ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK);
> +		WRITE_ONCE(*entry, (u32)(*entry & ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK));

I'm not sure how important this is, but this change creates a tiny time
window between reading from *entry and writing there. Should we use
atomic bitops instead?

>  }
>  
>  static int avic_handle_ldr_update(struct kvm_vcpu *vcpu)
Suthikulpanit, Suravee March 26, 2019, 3:47 a.m. UTC | #2
On 3/22/19 8:04 PM, Vitaly Kuznetsov wrote:
> "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@amd.com> writes:
> 
>> Only clear the valid bit when invalidate logical APIC id entry.
>> The current logic clear the valid bit, but also set the rest of
>> the bits (including reserved bits) to 1.
>>
>> Fixes: 98d90582be2e ('svm: Fix AVIC DFR and LDR handling')
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> ---
>>   arch/x86/kvm/svm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 7a4ce1a22ca0..f4fb766e474c 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -4640,7 +4640,7 @@ static void avic_invalidate_logical_id_entry(struct kvm_vcpu *vcpu)
>>   	u32 *entry = avic_get_logical_id_entry(vcpu, svm->ldr_reg, flat);
>>   
>>   	if (entry)
>> -		WRITE_ONCE(*entry, (u32) ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK);
>> +		WRITE_ONCE(*entry, (u32)(*entry & ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK));
> 
> I'm not sure how important this is, but this change creates a tiny time
> window between reading from *entry and writing there. Should we use
> atomic bitops instead?

Sure. That is also simpler. I'll send out V2 w/ clear_bit() instead.

Suravee

> 
>>   }
>>   
>>   static int avic_handle_ldr_update(struct kvm_vcpu *vcpu)
>
diff mbox series

Patch

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 7a4ce1a22ca0..f4fb766e474c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4640,7 +4640,7 @@  static void avic_invalidate_logical_id_entry(struct kvm_vcpu *vcpu)
 	u32 *entry = avic_get_logical_id_entry(vcpu, svm->ldr_reg, flat);
 
 	if (entry)
-		WRITE_ONCE(*entry, (u32) ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK);
+		WRITE_ONCE(*entry, (u32)(*entry & ~AVIC_LOGICAL_ID_ENTRY_VALID_MASK));
 }
 
 static int avic_handle_ldr_update(struct kvm_vcpu *vcpu)