Re: [PATCH 1/1] x2apic interface to lapic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gleb Natapov wrote:
 static unsigned int apic_lvt_mask[APIC_LVT_NUM] = {
 	LVT_MASK | APIC_LVT_TIMER_PERIODIC,	/* LVTT */
 	LVT_MASK | APIC_MODE_MASK,	/* LVTTHMR */
@@ -251,7 +257,12 @@ int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
 int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda)
 {
 	int result = 0;
-	u8 logical_id;
+	u32 logical_id;
+
+	if (apic_x2apic_mode(apic)) {
+		logical_id = apic_get_reg(apic, APIC_LDR);
+		return !!(logical_id & (uint16_t)mda & 0xffff);
+	}

!! unnecessary.  And one of the (cast) and (& 0xffff) is unnecessary.

logical_id = GET_APIC_LOGICAL_ID(apic_get_reg(apic, APIC_LDR)); @@ -440,7 +451,8 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 	irq.level = icr_low & APIC_INT_ASSERT;
 	irq.trig_mode = icr_low & APIC_INT_LEVELTRIG;
 	irq.shorthand = icr_low & APIC_SHORT_MASK;
-	irq.dest_id = GET_APIC_DEST_FIELD(icr_high);
+	irq.dest_id = apic_x2apic_mode(apic) ? icr_high :
+		GET_APIC_DEST_FIELD(icr_high);

Please replace the ?: (here and elsewhere) with explicit if statements. ?: is unreadable when split over two lines like this.

(I find it more readable to do

   blah = predicate
          ? iftrue
          : iffalse;

but that's almost the same as an if)

-static void apic_mmio_read(struct kvm_io_device *this,
-			   gpa_t address, int len, void *data)
+static int apic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
+		void *data)
 {
-	struct kvm_lapic *apic = (struct kvm_lapic *)this->private;
-	unsigned int offset = address - apic->base_address;
 	unsigned char alignment = offset & 0xf;
 	u32 result;
+	static const uint64_t rmask = 0x43ff01ffffffe70c;

Wow.  A comment perhaps?

if ((alignment + len) > 4) {
-		printk(KERN_ERR "KVM_APIC_READ: alignment error %lx %d",
-		       (unsigned long)address, len);
-		return;
+		printk(KERN_ERR "KVM_APIC_READ: alignment error %x %d\n",
+				offset, len);
+		return 1;
 	}
+
+	if (!(rmask & (1ULL << (offset >> 4)))) {
+		printk(KERN_ERR "KVM_APIC_READ: read reserved register %x\n",
+				offset);
+		return 1;
+	}
+

(offset >> 4) can still be 255, yielding unspecified results for the shift.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 98c2434..d3df59a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -800,6 +800,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 	case MSR_IA32_APICBASE:
 		kvm_set_apic_base(vcpu, data);
 		break;
+    case 0x800 ... 0xbff:
+        return kvm_x2apic_msr_write(vcpu, msr, data);
 	case MSR_IA32_MISC_ENABLE:
 		vcpu->arch.ia32_misc_enable_msr = data;
 		break;
@@ -958,6 +960,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_IA32_APICBASE:
 		data = kvm_get_apic_base(vcpu);
 		break;
+    case 0x800 ... 0xbff:
+        return kvm_x2apic_msr_read(vcpu, msr, pdata);
+        break;
 	case MSR_IA32_MISC_ENABLE:
 		data = vcpu->arch.ia32_misc_enable_msr;
 		break;

Whitespace...

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux