Skip to contentSkip to conversation

X86: hyperv: Enable MSR based APIC access

PATCH In review K. Y. Srinivasan 2 participants 3 messages Mar 18, 2015 – Mar 25, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 X86: hyperv: Enable MSR based APIC access

If the hypervisor supports MSR based access to the APIC registers (EOI, TPR and ICR), implement the MSR based access.

---
Changes from V1: Addressed comments from Ingo Molnar <[email protected]>

 arch/x86/kernel/cpu/mshyperv.c |   62 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/mshyperv.c +62 -0
index 939155f..76833c6 100644
@@ -110,6 +110,57 @@ static struct clocksource hyperv_cs = {
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
+static u64 ms_hv_apic_icr_read(void)
+{
+	u64 reg_val;
+
+	rdmsrl(HV_X64_MSR_ICR, reg_val);
+	return reg_val;
+}
+
+static void ms_hv_apic_icr_write(u32 low, u32 id)
+{
+	u64 reg_val;
+
+	reg_val = SET_APIC_DEST_FIELD(id);
+	reg_val = reg_val << 32;
+	reg_val |= low;
+
+	wrmsrl(HV_X64_MSR_ICR, reg_val);
+}
+
+static u32 ms_hv_apic_read(u32 reg)
+{
+	u32 reg_val, hi;
+
+	switch (reg) {
+	case APIC_EOI:
+		rdmsr(HV_X64_MSR_EOI, reg_val, hi);
+		return reg_val;
+	case APIC_TASKPRI:
+		rdmsr(HV_X64_MSR_TPR, reg_val, hi);
+		return reg_val;
+
+	default:
+		return native_apic_mem_read(reg);
+	}
+}
+
+static void ms_hv_apic_write(u32 reg, u32 val)
+{
+	switch (reg) {
+	case APIC_EOI:
+		wrmsr(HV_X64_MSR_EOI, val, 0);
+		break;
+	case APIC_TASKPRI:
+		wrmsr(HV_X64_MSR_TPR, val, 0);
+		break;
+	default:
+		native_apic_mem_write(reg, val);
+	}
+}
+
+
 static void __init ms_hyperv_init_platform(void)
 {
 	/*
@@ -143,6 +194,17 @@ static void __init ms_hyperv_init_platform(void)
 	no_timer_check = 1;
 #endif
 
+	if (ms_hyperv.features & HV_X64_MSR_APIC_ACCESS_AVAILABLE) {
+		/*
+		 * Setup the hooks for optimized APIC read/write.
+		 */
+		apic->read	= ms_hv_apic_read;
+		apic->write	= ms_hv_apic_write;
+		apic->icr_write	= ms_hv_apic_icr_write;
+		apic->icr_read	= ms_hv_apic_icr_read;
+		apic->eoi_write	= ms_hv_apic_write;
+	}
+
 }
 
 const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
Signature · 1 line
1.7.4.1
#1 lore raw
Quoted 6 lines · K. Y. Srinivasan

If the hypervisor supports MSR based access to the APIC registers (EOI, TPR and ICR), implement the MSR based access.

---
Changes from V1: Addressed comments from Ingo Molnar <[email protected]>

You didn't address my primary feedback:

Quoted 6 lines
Quoted 6 lines

So since we are trying to explain things, wouldn't this comment be more informative if it explained why we are trying to use the x2APIC facilities of Hyper-V?

I.e. what are the benefits of using the x2apic API towards the hypervisor?

please explain the purpose, context and benefits, not just what the patch does - even if the comment from your first patch is now gone.

Thanks,

Ingo

#2 lore raw
Quoted 19 lines

-----Original Message-----
From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo Molnar
Sent: Wednesday, March 25, 2015 5:15 AM
To: KY Srinivasan
Cc: [email protected]; [email protected]; linux- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V2 1/1] X86: hyperv: Enable MSR based APIC access

Quoted 3 lines · K. Y. Srinivasan

If the hypervisor supports MSR based access to the APIC registers (EOI, TPR and ICR), implement the MSR based access.

Quoted 10 lines
Quoted 3 lines

---
Changes from V1: Addressed comments from Ingo Molnar

<[email protected]>

You didn't address my primary feedback:

Quoted 3 lines
Quoted 3 lines

So since we are trying to explain things, wouldn't this comment be more informative if it explained why we are trying to use the x2APIC facilities of Hyper-V?

Ingo,

I added the code for advertising support for x2APIC since I saw the feature flag in the public documentation of Hyper-V. Subsequently, when I checked with the Hyper-V developers, I learned that Hyper-V does not support x2APIC for guests. So, I got rid of the code that checked for this flag as that would be dead code.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

I.e. what are the benefits of using the x2apic API towards the hypervisor?

please explain the purpose, context and benefits, not just what the patch does - even if the comment from your first patch is now gone.

The MSR based access can be a little more efficient than the mmio based access to the select APIC registers. That is the intent of this patch. Do you want me to resubmit the patch with these additional details.

Regards,

K. Y