diff mbox series

[2/3] KVM: x86: Simplify APIC ID selection in kvm_recalculate_phys_map()

Message ID 20230525183347.2562472-3-mhal@rbox.co (mailing list archive)
State New, archived
Headers show
Series Out-of-bounds access in kvm_recalculate_phys_map() | expand

Commit Message

Michal Luczaj May 25, 2023, 6:33 p.m. UTC
Move the comments and condense the code.
No functional change intended.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
Perhaps, as it was suggested by Maxim in [1], it would be a good moment to
change kvm_recalculate_phys_map() to return bool?

https://lore.kernel.org/kvm/e90e4c4bd558b9e41acea9f8ce84783e7341c9b4.camel@redhat.com/

 arch/x86/kvm/lapic.c | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 39b9a318d04c..7db1c698f6da 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -223,10 +223,10 @@  static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
 				    struct kvm_vcpu *vcpu,
 				    bool *xapic_id_mismatch)
 {
+	bool x2format = vcpu->kvm->arch.x2apic_format;
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	u32 x2apic_id = kvm_x2apic_id(apic);
 	u32 xapic_id = kvm_xapic_id(apic);
-	u32 physical_id;
 
 	/*
 	 * Deliberately truncate the vCPU ID when detecting a mismatched APIC
@@ -250,34 +250,24 @@  static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
 	 * effective APIC ID, e.g. due to the x2APIC wrap or because the guest
 	 * manually modified its xAPIC IDs, events targeting that ID are
 	 * supposed to be recognized by all vCPUs with said ID.
+	 *
+	 * For !x2apic_format disable the optimized map if the physical APIC ID
+	 * is already mapped, i.e. is aliased to multiple vCPUs.  The optimized
+	 * map requires a strict 1:1 mapping between IDs and vCPUs.
+	 *
+	 * See also kvm_apic_match_physical_addr().
 	 */
-	if (vcpu->kvm->arch.x2apic_format) {
-		/* See also kvm_apic_match_physical_addr(). */
-		if ((apic_x2apic_mode(apic) || x2apic_id > 0xff) &&
-			x2apic_id <= new->max_apic_id)
-			new->phys_map[x2apic_id] = apic;
-
-		if (!apic_x2apic_mode(apic) && !new->phys_map[xapic_id])
-			new->phys_map[xapic_id] = apic;
-	} else {
-		/*
-		 * Disable the optimized map if the physical APIC ID is already
-		 * mapped, i.e. is aliased to multiple vCPUs.  The optimized
-		 * map requires a strict 1:1 mapping between IDs and vCPUs.
-		 */
-		if (apic_x2apic_mode(apic)) {
-			if (x2apic_id > new->max_apic_id)
-				return -EINVAL;
+	if (apic_x2apic_mode(apic) || (x2format && x2apic_id > 0xff)) {
+		if (x2apic_id > new->max_apic_id ||
+		    (!x2format && new->phys_map[x2apic_id]))
+			return !x2format;
 
-			physical_id = x2apic_id;
-		} else {
-			physical_id = xapic_id;
-		}
-
-		if (new->phys_map[physical_id])
-			return -EINVAL;
+		new->phys_map[x2apic_id] = apic;
+	} else {
+		if (new->phys_map[xapic_id])
+			return !x2format;
 
-		new->phys_map[physical_id] = apic;
+		new->phys_map[xapic_id] = apic;
 	}
 
 	return 0;