Maxim,
On 3/24/22 9:14 PM, Maxim Levitsky wrote:
On Tue, 2022-03-08 at 10:39 -0600, Suravee Suthikulpanit wrote:
This function returns the currently programmed guest physical
APIC ID of a vCPU in both xAPIC and x2APIC modes.
Suggested-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
---
arch/x86/kvm/lapic.c | 23 +++++++++++++++++++++++
arch/x86/kvm/lapic.h | 5 +----
arch/x86/kvm/svm/avic.c | 21 +++++++++++++++++----
3 files changed, 41 insertions(+), 8 deletions(-)
...
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 4d7a8743196e..7e5a39a8e698 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -441,14 +441,21 @@ static void avic_invalidate_logical_id_entry(struct kvm_vcpu *vcpu)
static int avic_handle_ldr_update(struct kvm_vcpu *vcpu)
{
- int ret = 0;
+ int ret;
struct vcpu_svm *svm = to_svm(vcpu);
u32 ldr = kvm_lapic_get_reg(vcpu->arch.apic, APIC_LDR);
- u32 id = kvm_xapic_id(vcpu->arch.apic);
+ u32 id;
+
+ ret = kvm_get_apic_id(vcpu, &id);
+ if (ret)
+ return ret;
What about apic_id == 0?
The "id" is the returned apic ID. The "ret" is to let caller know if
the APIC look up is successful or not. "ret == 0" means success and
the value in "id" is valid.
if (ldr == svm->ldr_reg)
return 0;
+ if (id == X2APIC_BROADCAST)
+ return -EINVAL;
+
Why this is needed? avic_handle_ldr_update is called either
when guest writes to APIC_LDR (should not reach here),
or if LDR got changed while AVIC was inhibited (also
thankfully KVM doesn't allow it to be changed in x2APIC mode,
and it does reset it when enabling x2apic).
At this point, we don't need to handle LDR update in x2APIC case.
I will add apic_x2apic_mode() check here.
avic_invalidate_logical_id_entry(vcpu);
if (ldr)
@@ -464,7 +471,12 @@ static int avic_handle_apic_id_update(struct kvm_vcpu *vcpu)
{
u64 *old, *new;
struct vcpu_svm *svm = to_svm(vcpu);
- u32 id = kvm_xapic_id(vcpu->arch.apic);
+ u32 id;
+ int ret;
+
+ ret = kvm_get_apic_id(vcpu, &id);
+ if (ret)
+ return 1;
Well this function is totally broken anyway and I woudn't even bother touching it,
maximum, just stick 'return 0' in the very start of this function if the apic is
in x2apic mode now.
Oh well...
Sorry, I'm not sure if I understand what you mean by "broken".
This function setup/update the AVIC physical APIC ID table, whenever the APIC ID is
initialize or updated. It is needed in both xAPIC and x2APIC cases.
Regards,
Suravee
