Re: [PATCH 11/13] x86/paravirt: Add paravirt alternatives infrastructure

On Thu, Oct 05, 2017 at 04:35:03PM -0400, Boris Ostrovsky wrote:

quoted

 #ifdef CONFIG_PARAVIRT
+/*
+ * Paravirt alternatives are applied much earlier than normal alternatives.
+ * They are only applied when running on a hypervisor.  They replace some
+ * native instructions with calls to pv ops.
+ */
+void __init apply_pv_alternatives(void)
+{
+	setup_force_cpu_cap(X86_FEATURE_PV_OPS);

Not for Xen HVM guests.

From what I can tell, HVM guests still use pv_time_ops and
pv_mmu_ops.exit_mmap, right?

quoted

+	apply_alternatives(__pv_alt_instructions, __pv_alt_instructions_end);
+}

This is a problem (at least for Xen PV guests):
apply_alternatives()->text_poke_early()->local_irq_save()->...'cli'->death.

Ah, right.

It might be possible not to turn off/on the interrupts in this
particular case since the guest probably won't be able to handle an
interrupt at this point anyway.

Yeah, that should work.  For Xen and for the other hypervisors, this is
called well before irq init, so interrupts can't be handled yet anyway.

quoted

+
 void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
 				     struct paravirt_patch_site *end)
 {

diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 4fa90006ac68..17243fe0f5ce 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c

@@ -71,6 +71,8 @@ void __init init_hypervisor_platform(void)
 	if (!x86_hyper)
 		return;
 
+	apply_pv_alternatives();

Not for Xen PV guests who have already done this.

I think it would be harmless, but yeah, it's probably best to only write
it once.

Thanks for the review!

-- 
Josh