paravirt_ops: refactor struct paravirt_ops into smaller pv_*_ops

This patch refactors the paravirt_ops structure into groups of functionally related ops:

pv_info - random info, rather than function entrypoints pv_init_ops - functions used at boot time (some for module_init too) pv_misc_ops - lazy mode, which didn't fit well anywhere else pv_time_ops - time-related functions
pv_cpu_ops - various privileged instruction ops pv_irq_ops - operations for managing interrupt state pv_apic_ops - APIC operations
pv_mmu_ops - operations for managing pagetables

There are several motivations for this:

1. Some of these ops will be general to all x86, and some will be i386/x86-64 specific. This makes it easier to share common stuff while allowing separate implementations where needed.

2. At the moment we must export all of paravirt_ops, but modules only need selected parts of it. This allows us to export on a case by case basis (and also choose which export license we want to apply).

This still needs to be reconciled with the x86 merge and glommer's paravirt_ops unification work.

Quoted 18 lines · Jeremy Fitzhardinge

This patch refactors the paravirt_ops structure into groups of functionally related ops:

pv_info - random info, rather than function entrypoints pv_init_ops - functions used at boot time (some for module_init too) pv_misc_ops - lazy mode, which didn't fit well anywhere else pv_time_ops - time-related functions
pv_cpu_ops - various privileged instruction ops pv_irq_ops - operations for managing interrupt state pv_apic_ops - APIC operations
pv_mmu_ops - operations for managing pagetables

There are several motivations for this:

1. Some of these ops will be general to all x86, and some will be i386/x86-64 specific. This makes it easier to share common stuff while allowing separate implementations where needed.

Quoted 4 lines

2. At the moment we must export all of paravirt_ops, but modules only need selected parts of it. This allows us to export on a case by case basis (and also choose which export license we want to apply).

We shouldn't need to export pv_init_ops.

It is debatable whether
CR2/CR3 should be part of CPU or MMU ops.

Also, can we drop write_cr2?
It isn't used anywhere, so the only reason to keep it is symmetry. Which was a fine argument when it was an inline, but now it just adds unused junk to the code.

Quoted 1 line

We shouldn't need to export pv_init_ops.

No. The only ones I export are:

EXPORT_SYMBOL_GPL(pv_time_ops);
EXPORT_SYMBOL_GPL(pv_cpu_ops);
EXPORT_SYMBOL_GPL(pv_mmu_ops);
EXPORT_SYMBOL_GPL(pv_apic_ops);
EXPORT_SYMBOL (pv_irq_ops);

Quoted 4 lines

It is debatable whether
CR2/CR3 should be part of CPU or MMU ops.

Yeah, I was in two minds. CR3, at least, should be grouped with the other tlb operations, wherever they go. And while they're privileged CPU instructions (cpu_ops), they're more logically related to the rest of the mmu state. On the other hand, we could have an ops structure specifically dedicated to pagetable manipulations, and put the cr3/tlb ops elsewhere.

Quoted 5 lines

Also, can we drop write_cr2?
It isn't used anywhere, so the only reason to keep it is symmetry. Which was a fine argument when it was an inline, but now it just adds unused junk to the code.

I think its used in some cpu state save/restore code, but its not relevant to pv-ops.

This patch refactors the paravirt_ops structure into groups of functionally related ops:

pv_info - random info, rather than function entrypoints pv_init_ops - functions used at boot time (some for module_init too) pv_misc_ops - lazy mode, which didn't fit well anywhere else pv_time_ops - time-related functions
pv_cpu_ops - various privileged instruction ops pv_irq_ops - operations for managing interrupt state pv_apic_ops - APIC operations
pv_mmu_ops - operations for managing pagetables

Quoted 13 lines · Jeremy Fitzhardinge

This patch refactors the paravirt_ops structure into groups of functionally related ops:

pv_info - random info, rather than function entrypoints pv_init_ops - functions used at boot time (some for module_init too) pv_misc_ops - lazy mode, which didn't fit well anywhere else pv_time_ops - time-related functions
pv_cpu_ops - various privileged instruction ops pv_irq_ops - operations for managing interrupt state pv_apic_ops - APIC operations
pv_mmu_ops - operations for managing pagetables

Good. These make sense to me.

arch/i386/kernel/paravirt.c int paravirt_disable_iospace(void) · line 299

Quoted 10 lines

arch/i386/kernel/paravirt.c +9 -0

+	.pv_irq_ops = {
+		 .init_IRQ = native_init_IRQ,
+		 .save_fl = native_save_fl,
+		 .restore_fl = native_restore_fl,
+		 .irq_disable = native_irq_disable,
+		 .irq_enable = native_irq_enable,
+		 .safe_halt = native_safe_halt,
+		 .halt = native_halt,
+	 },

I think the halt stuff should be moved to pv_cpu_ops?

arch/i386/kernel/paravirt.c int paravirt_disable_iospace(void) · line 349

Quoted 4 lines

arch/i386/kernel/paravirt.c +3 -0

+	.pv_misc_ops = {
+		 .set_lazy_mode = paravirt_nop,
+	 },

Or you can split it to pv_cpu_ops and pv_mmu_ops, assuming that they don't need to interact with each other in terms of the lazy handling.

Quoted 16 lines · Jun

arch/i386/kernel/paravirt.c int paravirt_disable_iospace(void) · line 299

Quoted 10 lines · Jeremy Fitzhardinge

arch/i386/kernel/paravirt.c +9 -0

+	.pv_irq_ops = {
+		 .init_IRQ = native_init_IRQ,
+		 .save_fl = native_save_fl,
+		 .restore_fl = native_restore_fl,
+		 .irq_disable = native_irq_disable,
+		 .irq_enable = native_irq_enable,
+		 .safe_halt = native_safe_halt,
+		 .halt = native_halt,
+	 },

I think the halt stuff should be moved to pv_cpu_ops?

You mean halt's alternate "shutdown vcpu" meaning if you call it with interrupts disabled? Yeah, I'd be happy to have an explicit op for that, rather than making it a secondary overloaded meaning. And use "safe_halt" for all uses of "wait for next interrupt".

Quoted 8 lines

arch/i386/kernel/paravirt.c int paravirt_disable_iospace(void) · line 349

Quoted 5 lines

arch/i386/kernel/paravirt.c +3 -0

+	.pv_misc_ops = {
+		 .set_lazy_mode = paravirt_nop,
+	 },

Or you can split it to pv_cpu_ops and pv_mmu_ops, assuming that they don't need to interact with each other in terms of the lazy

Quoted 1 line

You mean have separate lazy_mmu and lazy_cpu (lazy_context_switch)

Possible, but they're still exclusive. (I think VMI, at least,

that you can't have lazy_mmu and lazy_cpu active at the same time, and its nice to enforce this in the interface.)

But having a whole misc structure for this interface is pretty warty,

admit.

J

Yes. For the native, "safe_halt" is "sti; hlt". The "native_halt" is just "hlt". So the para_virt part of "hlt" could be moved to pv_cpu_ops, and the "sti" part stays in pv_irq_ops.

Actually my concern was that such misc ops might grow to include the things don't fit well anywhere else. To me, then pv_lazy_ops (with just .set_mode) might be better.

Quoted 2 lines · Jun

Yes. For the native, "safe_halt" is "sti; hlt". The "native_halt" is just "hlt". So the para_virt part of "hlt" could be moved to

Quoted 2 lines

and the "sti" part stays in pv_irq_ops.

By "sti part", you mean the full "sti; hlt" sequence of safe_halt, right? Since it needs to be an atomic sequence to avoid race conditions, so the native sequence has to be precisely "sti; hlt" to take advantage of the sti shadow, and other pv-backends will need

own way to guarantee this atomicity.

To me such atomicity is provided by the "sti" instruction (i.e. the processor begins responding to external, maskable interrupts _after_ the next instruction is executed), and there is nothing special with that combination "sti; hlt" (you can also have like "sti; ret", for example).

So if you define a PV ops like STI(next_instruction), "safe_halt" for the native should be defined as STI("hlt"), and inlined as "sti; hlt".

If it's hard or we don't need to expose the semantics of "sti" other than that, I think it's okay to have a PV operation for safe_halt.