Jeremy Fitzhardinge wrote: > On 11/18/09 08:13, Alexander Graf wrote: > >> Currently when using paravirt ops it's an all-or-nothing option. We can either >> use pv-ops for CPU, MMU, timing, etc. or not at all. >> >> Now there are some use cases where we don't need the full feature set, but only >> a small chunk of it. KVM is a pretty prominent example for this. >> >> So let's make everything a bit more fine-grained. We already have a splitting >> by function groups, namely "cpu", "mmu", "time", "irq", "apic" and "spinlock". >> >> Taking that existing splitting and extending it to only compile in the PV >> capable bits sounded like a natural fit. That way we don't get performance hits >> in MMU code from using the KVM PV clock which only needs the TIME parts of >> pv-ops. >> >> We define a new CONFIG_PARAVIRT_ALL option that basically does the same thing >> the CONFIG_PARAVIRT did before this splitting. We move all users of >> CONFIG_PARAVIRT to CONFIG_PARAVIRT_ALL, so they behave the same way they did >> before. >> >> So here it is - the splitting! I would have made the patch smaller, but this >> was the closest I could get to atomic (for bisect) while staying sane. >> >> > > The basic idea seems pretty sane. I'm wondering how much compile test > coverage you've given all these extra config options; there are now a > lot more combinations, and your use of select is particularly worrying > because they don't propagate dependencies properly. > Uh - I don't see where there should be any dependencies. > For example, does this actually work? > >> config XEN >> bool "Xen guest support" >> - select PARAVIRT >> + select PARAVIRT_ALL >> select PARAVIRT_CLOCK >> depends on X86_64 || (X86_32 && X86_PAE && !X86_VISWS) >> depends on X86_CMPXCHG && X86_TSC >> >> > Does selecting PARAVIRT_ALL end up selecting all the other PARAVIRT_*? > > Can you reassure me? > > +config PARAVIRT_ALL > + bool > + select PARAVIRT_CPU > + select PARAVIRT_TIME > + select PARAVIRT_IRQ > + select PARAVIRT_APIC > + select PARAVIRT_MMU > + default n > + So selecting PARAVIRT_ALL selects all the other split PARAVIRT parts that in turn select PARAVIRT. I tested a compile on x86_64 with Xen DomU support enabled and it compiled fine :-). > Also, I think VMI is the only serious user of PARAVIRT_APIC, so we can > mark that to go when VMI does. > Sounds good :-). So the patch even serves as a helper for anyone who'll remove that support later. > What ends up using plain CONFIG_PARAVIRT? Do we still need it? > It's used for an info field that says which hypervisor we're running on and as config option to know if we need to include the binary patch magic. As soon as a single sub-paravirt option is selected, we need that to make sure we have the framework in place. Alex -- 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
