[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC v1 1/8] paravirt: rename paravirt_enabled to paravirt_legacy
On Tue, Dec 15, 2015 at 02:16:30PM -0800, Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx> Hey Luis, Sorry for the long time to respond.. > > paravirt_enabled conveys the idea that if this is set or if > paravirt_enabled() returns true you are in a paravirtualized > environment. This is not true, this is really only useful to > determine if you have a paravirtualization hypervisor that uses s/users// > supports legacy paravirtualized guests. At run time, this tells > us if we've booted into a Linux guest with support for legacy > paravirtualized guests. I know I suggested the name but when I look at the words 'paravirtualized legacy' immediately I thought about legacy PV drivers.. which we do not have in the code. The description: > + * paravirtualized guests. Examples of features that > + * characterize legacy paravirtualized guests are > + * things such as the need for APM, PNP BIOS. Explains it very nicely. Perhaps (And sorry for that) it should be just called legacy_platform() ? Or ancient_platform() ? Since that would lead folks to immediately think of things that existed long time ago - such as APM or PNP BIOS. Other suggestions would be to piggyback on Microsoft pushing the option in the BIOS to have an "Legacy Free" option (PS/2, PnP, serial port, parallel port - all are disabled): https://en.wikipedia.org/wiki/Legacy-free_PC Perhaps: legacy_free() ? Thank you! > > Make emphasis about this by renaming the member and helper. > While at it, make the type bool, and document it therefore > avoiding special cased comments trying to explain it. > > The rename is done using the following Coccinelle SmPL patch: > > @ rename_paravirt_enabled @ > @@ > > -paravirt_enabled() > +paravirt_legacy() > > @ rename_pv_info_pv_enabled @ > @@ > -pv_info.paravirt_enabled > +pv_info.paravirt_legacy > > @ is_pv @ > identifier pv; > @@ > struct pv_info pv = { > }; > > @ rename_struct_pv_enabled depends on is_pv @ > identifier is_pv.pv; > expression val; > @@ > > struct pv_info pv = { > - .paravirt_enabled > + .paravirt_legacy > = val, > }; > > Generated-by: Coccinelle SmPL > Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > Cc: cocci@xxxxxxxxxxxxxxx > cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxx> > --- > arch/x86/include/asm/paravirt.h | 4 ++-- > arch/x86/include/asm/paravirt_types.h | 11 ++++++++++- > arch/x86/include/asm/processor.h | 2 +- > arch/x86/kernel/apm_32.c | 2 +- > arch/x86/kernel/asm-offsets.c | 2 +- > arch/x86/kernel/cpu/intel.c | 2 +- > arch/x86/kernel/cpu/microcode/core.c | 2 +- > arch/x86/kernel/head.c | 2 +- > arch/x86/kernel/kvm.c | 9 +-------- > arch/x86/kernel/paravirt.c | 2 +- > arch/x86/kernel/tboot.c | 2 +- > arch/x86/lguest/boot.c | 2 +- > arch/x86/mm/dump_pagetables.c | 2 +- > arch/x86/xen/enlighten.c | 2 +- > drivers/pnp/pnpbios/core.c | 2 +- > 15 files changed, 25 insertions(+), 23 deletions(-) > > diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h > index 10d0596433f8..38e698ff8cae 100644 > --- a/arch/x86/include/asm/paravirt.h > +++ b/arch/x86/include/asm/paravirt.h > @@ -14,9 +14,9 @@ > #include <linux/types.h> > #include <linux/cpumask.h> > > -static inline int paravirt_enabled(void) > +static inline bool paravirt_legacy(void) > { > - return pv_info.paravirt_enabled; > + return pv_info.paravirt_legacy; > } > > static inline void load_sp0(struct tss_struct *tss, > diff --git a/arch/x86/include/asm/paravirt_types.h > b/arch/x86/include/asm/paravirt_types.h > index 31247b5bff7c..3d89e8e878bc 100644 > --- a/arch/x86/include/asm/paravirt_types.h > +++ b/arch/x86/include/asm/paravirt_types.h > @@ -61,6 +61,15 @@ struct paravirt_callee_save { > }; > > /* general info */ > + > +/** > + * struct pv_info - hypervisor information > + * > + * @paravirt_legacy: true if this hypervisor supports legacy > + * paravirtualized guests. Examples of features that > + * characterize legacy paravirtualized guests are > + * things such as the need for APM, PNP BIOS. > + */ > struct pv_info { > unsigned int kernel_rpl; > int shared_kernel_pmd; > @@ -69,7 +78,7 @@ struct pv_info { > u16 extra_user_64bit_cs; /* __USER_CS if none */ > #endif > > - int paravirt_enabled; > + bool paravirt_legacy; > const char *name; > }; > > diff --git a/arch/x86/include/asm/processor.h > b/arch/x86/include/asm/processor.h > index 67522256c7ff..354a0d010c59 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -471,7 +471,7 @@ static inline unsigned long current_top_of_stack(void) > #include <asm/paravirt.h> > #else > #define __cpuid native_cpuid > -#define paravirt_enabled() 0 > +#define paravirt_legacy() false > > static inline void load_sp0(struct tss_struct *tss, > struct thread_struct *thread) > diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c > index 052c9c3026cc..74a3e8ca2c6d 100644 > --- a/arch/x86/kernel/apm_32.c > +++ b/arch/x86/kernel/apm_32.c > @@ -2267,7 +2267,7 @@ static int __init apm_init(void) > > dmi_check_system(apm_dmi_table); > > - if (apm_info.bios.version == 0 || paravirt_enabled() || > machine_is_olpc()) { > + if (apm_info.bios.version == 0 || paravirt_legacy() || > machine_is_olpc()) { > printk(KERN_INFO "apm: BIOS not found.\n"); > return -ENODEV; > } > diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c > index 439df975bc7a..20a7a0df2c65 100644 > --- a/arch/x86/kernel/asm-offsets.c > +++ b/arch/x86/kernel/asm-offsets.c > @@ -59,7 +59,7 @@ void common(void) { > > #ifdef CONFIG_PARAVIRT > BLANK(); > - OFFSET(PARAVIRT_enabled, pv_info, paravirt_enabled); > + OFFSET(PARAVIRT_legacy, pv_info, paravirt_legacy); > OFFSET(PARAVIRT_PATCH_pv_cpu_ops, paravirt_patch_template, pv_cpu_ops); > OFFSET(PARAVIRT_PATCH_pv_irq_ops, paravirt_patch_template, pv_irq_ops); > OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable); > diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c > index 209ac1e7d1f0..134b5d5d9c74 100644 > --- a/arch/x86/kernel/cpu/intel.c > +++ b/arch/x86/kernel/cpu/intel.c > @@ -220,7 +220,7 @@ static void intel_workarounds(struct cpuinfo_x86 *c) > * The Quark is also family 5, but does not have the same bug. > */ > clear_cpu_bug(c, X86_BUG_F00F); > - if (!paravirt_enabled() && c->x86 == 5 && c->x86_model < 9) { > + if (!paravirt_legacy() && c->x86 == 5 && c->x86_model < 9) { > static int f00f_workaround_enabled; > > set_cpu_bug(c, X86_BUG_F00F); > diff --git a/arch/x86/kernel/cpu/microcode/core.c > b/arch/x86/kernel/cpu/microcode/core.c > index b3e94ef461fd..022cc8dad97f 100644 > --- a/arch/x86/kernel/cpu/microcode/core.c > +++ b/arch/x86/kernel/cpu/microcode/core.c > @@ -630,7 +630,7 @@ int __init microcode_init(void) > struct cpuinfo_x86 *c = &boot_cpu_data; > int error; > > - if (paravirt_enabled() || dis_ucode_ldr) > + if (paravirt_legacy() || dis_ucode_ldr) > return -EINVAL; > > if (c->x86_vendor == X86_VENDOR_INTEL) > diff --git a/arch/x86/kernel/head.c b/arch/x86/kernel/head.c > index 992f442ca155..279fad7288f8 100644 > --- a/arch/x86/kernel/head.c > +++ b/arch/x86/kernel/head.c > @@ -38,7 +38,7 @@ void __init reserve_ebda_region(void) > * that the paravirt case can handle memory setup > * correctly, without our help. > */ > - if (paravirt_enabled()) > + if (paravirt_legacy()) > return; > > /* end of low (conventional) memory */ > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > index 47190bd399e7..e2368f73bc14 100644 > --- a/arch/x86/kernel/kvm.c > +++ b/arch/x86/kernel/kvm.c > @@ -283,14 +283,7 @@ NOKPROBE_SYMBOL(do_async_page_fault); > static void __init paravirt_ops_setup(void) > { > pv_info.name = "KVM"; > - > - /* > - * KVM isn't paravirt in the sense of paravirt_enabled. A KVM > - * guest kernel works like a bare metal kernel with additional > - * features, and paravirt_enabled is about features that are > - * missing. > - */ > - pv_info.paravirt_enabled = 0; > + pv_info.paravirt_legacy = false; > > if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY)) > pv_cpu_ops.io_delay = kvm_io_delay; > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c > index c2130aef3f9d..326df1ce2a11 100644 > --- a/arch/x86/kernel/paravirt.c > +++ b/arch/x86/kernel/paravirt.c > @@ -312,7 +312,7 @@ enum paravirt_lazy_mode paravirt_get_lazy_mode(void) > > struct pv_info pv_info = { > .name = "bare hardware", > - .paravirt_enabled = 0, > + .paravirt_legacy = false, > .kernel_rpl = 0, > .shared_kernel_pmd = 1, /* Only used when CONFIG_X86_PAE is set */ > > diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c > index 91a4496db434..87a5027ff242 100644 > --- a/arch/x86/kernel/tboot.c > +++ b/arch/x86/kernel/tboot.c > @@ -75,7 +75,7 @@ void __init tboot_probe(void) > } > > /* only a natively booted kernel should be using TXT */ > - if (paravirt_enabled()) { > + if (paravirt_legacy()) { > pr_warning("non-0 tboot_addr but pv_ops is enabled\n"); > return; > } > diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c > index a0d09f6c6533..eb0a42bbf41b 100644 > --- a/arch/x86/lguest/boot.c > +++ b/arch/x86/lguest/boot.c > @@ -1409,7 +1409,7 @@ __init void lguest_init(void) > /* We're under lguest. */ > pv_info.name = "lguest"; > /* Paravirt is enabled. */ > - pv_info.paravirt_enabled = 1; > + pv_info.paravirt_legacy = true; > /* We're running at privilege level 1, not 0 as normal. */ > pv_info.kernel_rpl = 1; > /* Everyone except Xen runs with this set. */ > diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c > index a035c2aa7801..daf87f12a0af 100644 > --- a/arch/x86/mm/dump_pagetables.c > +++ b/arch/x86/mm/dump_pagetables.c > @@ -365,7 +365,7 @@ static inline bool is_hypervisor_range(int idx) > * ffff800000000000 - ffff87ffffffffff is reserved for > * the hypervisor. > */ > - return paravirt_enabled() && > + return paravirt_legacy() && > (idx >= pgd_index(__PAGE_OFFSET) - 16) && > (idx < pgd_index(__PAGE_OFFSET)); > } > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index 5774800ff583..95060b31e49c 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -1186,7 +1186,7 @@ static unsigned xen_patch(u8 type, u16 clobbers, void > *insnbuf, > } > > static const struct pv_info xen_info __initconst = { > - .paravirt_enabled = 1, > + .paravirt_legacy = true, > .shared_kernel_pmd = 0, > > #ifdef CONFIG_X86_64 > diff --git a/drivers/pnp/pnpbios/core.c b/drivers/pnp/pnpbios/core.c > index facd43b8516c..40557bc16005 100644 > --- a/drivers/pnp/pnpbios/core.c > +++ b/drivers/pnp/pnpbios/core.c > @@ -521,7 +521,7 @@ static int __init pnpbios_init(void) > int ret; > > if (pnpbios_disabled || dmi_check_system(pnpbios_dmi_table) || > - paravirt_enabled()) { > + paravirt_legacy()) { > printk(KERN_INFO "PnPBIOS: Disabled\n"); > return -ENODEV; > } > -- > 2.6.2 > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |