Hi all, the last update on the PV on HVM Xen series contains the following changes: - some variables and functions have been renamed following Jeremy''s suggestions, in particular: s/init_shared_info/xen_hvm_init_shared_info s/xen_platform_pci/xen_platform_pci_enabled s/UNPLUG_/XEN_UNPLUG_ - the two platform_pci.h header files have been merged and the useless intro has been removed; - the xen platform pci product number and driver versions have been made static; - the description on the VIRQ_TIMER patch has been improved; - a new patch to fix hpet behaviour has been introduced: hpet_disable is called unconditionally on machine shutdown, and doesn''t check whether hpet has been actually enabled, causing trouble if it hasn''t; - the vector callback patch has been improved: we don''t use the ipi vector anymore but we allocate our own so that we can avoid useless and expensive vlapic acks. Meanwhile the vector callback patch for xen has been checked into xen-unstable, so you don''t need a separate patch for xen anymore to take full advantage of this patch series. A git tree is available here: git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git branch name 2.6.34-pvhvm-v3. Cheers, Stefano _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 06/03/2010 06:07 AM, Stefano Stabellini wrote:> Hi all, > the last update on the PV on HVM Xen series contains the following > changes: > > - some variables and functions have been renamed following Jeremy''s > suggestions, in particular: > s/init_shared_info/xen_hvm_init_shared_info > s/xen_platform_pci/xen_platform_pci_enabled > s/UNPLUG_/XEN_UNPLUG_ > > - the two platform_pci.h header files have been merged and the useless > intro has been removed; > > - the xen platform pci product number and driver versions have been made > static; > > - the description on the VIRQ_TIMER patch has been improved; > > - a new patch to fix hpet behaviour has been introduced: hpet_disable is > called unconditionally on machine shutdown, and doesn''t check whether > hpet has been actually enabled, causing trouble if it hasn''t; > > - the vector callback patch has been improved: we don''t use the ipi > vector anymore but we allocate our own so that we can avoid useless and > expensive vlapic acks. >I think they''re worse than that; I think they could end up potentially acking a pending interrupt which hasn''t been delivered yet.> > Meanwhile the vector callback patch for xen has been checked into > xen-unstable, so you don''t need a separate patch for xen anymore to take > full advantage of this patch series. > > A git tree is available here: > > git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git > > branch name 2.6.34-pvhvm-v3. >Thanks, J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi all, this is another update on the PV on HVM Xen series; a list of changes compared to the previous version follows: - the bug caused by modprobing xen frontend modules when xenbus is not initialized has been fixed: now xenbus is always initialized during postcore_initcall, before device drivers are initialized, so that we can be sure that loading xen pv frontends will always happen afterwards, no matter how the xen platform pci driver is built. In order to do that xenbus_probe has been moved out of the xenbus initialization and called it later on at device_initcall. - a new HVMOP_pagetable_dying hypercall has been added to notify Xen that a pagetable is going to be destroyed: this improves performances significantly when running on shadow pagetables. A patch is currently need on the Xen side for this to work. Jeremy''s comments have been addressed: - xen_guest_init has been renamed xen_hvm_guest_init; - init_hvm_time has been moved to arch/x86/xen/time.c; Konrad''s comments have been addressed: - gnttab_max_nr_grant_frames has been renamed gnttab_max_grant_frames; - few inaccurate comments have been rewritten; - the preprocessor checks in platform-pci-unplug.c have been moved to platform_pci.h; - few other code style improvements, like using dev_err instead of printk and strncmp instead of strcmp. A git tree is available here: git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git branch name 2.6.34-pvhvm-v4. Cheers, Stefano _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
As usual I mistyped the topic, the series is made up of 13 patches now :) On Mon, 21 Jun 2010, Stefano Stabellini wrote:> Hi all, > this is another update on the PV on HVM Xen series; a list of changes > compared to the previous version follows: > > - the bug caused by modprobing xen frontend modules when xenbus is not > initialized has been fixed: now xenbus is always initialized during > postcore_initcall, before device drivers are initialized, so that we > can be sure that loading xen pv frontends will always happen afterwards, > no matter how the xen platform pci driver is built. > In order to do that xenbus_probe has been moved out of the xenbus > initialization and called it later on at device_initcall. > > - a new HVMOP_pagetable_dying hypercall has been added to notify Xen > that a pagetable is going to be destroyed: this improves performances > significantly when running on shadow pagetables. > A patch is currently need on the Xen side for this to work. > > Jeremy''s comments have been addressed: > > - xen_guest_init has been renamed xen_hvm_guest_init; > > - init_hvm_time has been moved to arch/x86/xen/time.c; > > Konrad''s comments have been addressed: > > - gnttab_max_nr_grant_frames has been renamed gnttab_max_grant_frames; > > - few inaccurate comments have been rewritten; > > - the preprocessor checks in platform-pci-unplug.c have been moved to > platform_pci.h; > > - few other code style improvements, like using dev_err instead of > printk and strncmp instead of strcmp. > > > > A git tree is available here: > > git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git > > branch name 2.6.34-pvhvm-v4. > > Cheers, > > Stefano >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2010-Jun-30 17:53 UTC
[Xen-devel] Re: [PATCH 10/13] Do not try to disable hpet if it hasn''t been initialized before
On Mon, Jun 21, 2010 at 05:14:04PM +0100, stefano@stabellini.net wrote:> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > hpet_disable is called unconditionally on machine reboot if hpet support > is compiled in the kernel. > hpet_disable only checks if the machine is hpet capable but doesn''t make > sure that hpet has been initialized.CC-ing John Stultz who has had his head wrapped around this time-thingy. Hi John! What are your thoughts about this patch?> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > arch/x86/kernel/hpet.c | 18 ++++++++++-------- > 1 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c > index 23b4ecd..2b299da 100644 > --- a/arch/x86/kernel/hpet.c > +++ b/arch/x86/kernel/hpet.c > @@ -959,16 +959,18 @@ fs_initcall(hpet_late_init); > > void hpet_disable(void) > { > - if (is_hpet_capable()) { > - unsigned int cfg = hpet_readl(HPET_CFG); > + unsigned int cfg; > > - if (hpet_legacy_int_enabled) { > - cfg &= ~HPET_CFG_LEGACY; > - hpet_legacy_int_enabled = 0; > - } > - cfg &= ~HPET_CFG_ENABLE; > - hpet_writel(cfg, HPET_CFG); > + if (!is_hpet_capable() || !hpet_address || !hpet_virt_address) > + return; > + > + cfg = hpet_readl(HPET_CFG); > + if (hpet_legacy_int_enabled) { > + cfg &= ~HPET_CFG_LEGACY; > + hpet_legacy_int_enabled = 0; > } > + cfg &= ~HPET_CFG_ENABLE; > + hpet_writel(cfg, HPET_CFG); > } > > #ifdef CONFIG_HPET_EMULATE_RTC > -- > 1.7.0.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2010-Jun-30 17:55 UTC
[Xen-devel] Re: [PATCH 04/13] Xen PCI platform device driver
> @@ -476,9 +503,28 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx) > > int gnttab_resume(void) > { > - if (max_nr_grant_frames() < nr_grant_frames) > + unsigned int max_nr_gframes; > + > + max_nr_gframes = max_nr_grant_frames(); > + if (max_nr_gframes < nr_grant_frames) > return -ENOSYS; > - return gnttab_map(0, nr_grant_frames - 1); > + > + if (xen_pv_domain()) > + return gnttab_map(0, nr_grant_frames - 1); > + > + if (!hvm_pv_resume_frames) { > + hvm_pv_resume_frames = alloc_xen_mmio(PAGE_SIZE * max_nr_gframes); > + shared = ioremap(hvm_pv_resume_frames, PAGE_SIZE * max_nr_gframes); > + if (shared == NULL) { > + printk(KERN_WARNING > + "Fail to ioremap gnttab share frames\n");I would say: "Failed to ioremap gnttab share frames!\n". .. snip ..> + if (request_region(ioaddr, iolen, DRV_NAME) == NULL) { > + dev_err(&pdev->dev, "I/O resource 0x%lx @ 0x%lx busy\n", > + iolen, ioaddr); > + ret = -EBUSY; > + goto out; > + } > + > + platform_mmio = mmio_addr; > + platform_mmiolen = mmio_len; > + > + if (!xen_have_vector_callback) { > + ret = xen_allocate_irq(pdev); > + if (ret) { > + printk(KERN_WARNING "request_irq failed err=%d\n", ret);Use dev_warn here.> + goto out; > + } > + callback_via = get_callback_via(pdev); > + ret = xen_set_callback_via(callback_via); > + if (ret) { > + printk(KERN_WARNING > + "Unable to set the evtchn callback err=%d\n", ret);ditto.> + goto out; > + } > + } > + > + ret = gnttab_init(); > + if (ret) > + goto out; > + xenbus_probe(NULL); > + > +out: > + if (ret) { > + release_mem_region(mmio_addr, mmio_len); > + release_region(ioaddr, iolen); > + pci_disable_device(pdev); > + } > + > + return ret; > +} > + > +static struct pci_device_id platform_pci_tbl[] __devinitdata = { > + {PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM, > + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > + {0,} > +}; > + > +MODULE_DEVICE_TABLE(pci, platform_pci_tbl); > + > +static struct pci_driver platform_driver = { > + .name = DRV_NAME, > + .probe = platform_pci_init, > + .id_table = platform_pci_tbl, > +}; > + > +static int __init platform_pci_module_init(void) > +{ > + int rc; > + > + rc = pci_register_driver(&platform_driver); > + if (rc) { > + printk(KERN_INFO DRV_NAME > + ": No platform pci device model found\n");No need for that. The pci_register_driver call to ->probe and its subsequent setup, if it failed, should have displayed the appropiate error+explanation? How about just making it: return pci_register_driver(&platform_driver); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Mon, Jun 21, 2010 at 05:12:20PM +0100, Stefano Stabellini wrote:> Hi all, > this is another update on the PV on HVM Xen series; a list of changes > compared to the previous version follows:Can you use the [v4] in the main patch (this is the fourth version of these patches, right?)? I am getting confused as to which version I am reviewing and if I''ve missed one or what not. And in this first intro email state: Since v7 the following things have changed: - Konrad made some comments. - etc. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Venkatesh Pallipadi
2010-Jun-30 21:24 UTC
[Xen-devel] Re: [PATCH 10/13] Do not try to disable hpet if it hasn''t been initialized before
Looks Good. Acked-by: Venkatesh Pallipadi <venki@google.com> Copying Thomas/Ingo/Peter. On Mon, Jun 21, 2010 at 9:14 AM, <stefano@stabellini.net> wrote:> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > hpet_disable is called unconditionally on machine reboot if hpet support > is compiled in the kernel. > hpet_disable only checks if the machine is hpet capable but doesn''t make > sure that hpet has been initialized. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > arch/x86/kernel/hpet.c | 18 ++++++++++-------- > 1 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c > index 23b4ecd..2b299da 100644 > --- a/arch/x86/kernel/hpet.c > +++ b/arch/x86/kernel/hpet.c > @@ -959,16 +959,18 @@ fs_initcall(hpet_late_init); > > void hpet_disable(void) > { > - if (is_hpet_capable()) { > - unsigned int cfg = hpet_readl(HPET_CFG); > + unsigned int cfg; > > - if (hpet_legacy_int_enabled) { > - cfg &= ~HPET_CFG_LEGACY; > - hpet_legacy_int_enabled = 0; > - } > - cfg &= ~HPET_CFG_ENABLE; > - hpet_writel(cfg, HPET_CFG); > + if (!is_hpet_capable() || !hpet_address || !hpet_virt_address) > + return; > + > + cfg = hpet_readl(HPET_CFG); > + if (hpet_legacy_int_enabled) { > + cfg &= ~HPET_CFG_LEGACY; > + hpet_legacy_int_enabled = 0; > } > + cfg &= ~HPET_CFG_ENABLE; > + hpet_writel(cfg, HPET_CFG); > } > > #ifdef CONFIG_HPET_EMULATE_RTC > -- > 1.7.0.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Wed, 30 Jun 2010, Konrad Rzeszutek Wilk wrote:> On Mon, Jun 21, 2010 at 05:12:20PM +0100, Stefano Stabellini wrote: > > Hi all, > > this is another update on the PV on HVM Xen series; a list of changes > > compared to the previous version follows: > > Can you use the [v4] in the main patch (this is the fourth version of > these patches, right?)? I am getting confused as to which version I am > reviewing and if I''ve missed one or what not. > > And in this first intro email state: > > Since v7 the following things have changed: > - Konrad made some comments. > - etc. >Good point, I''ll do that from now on. I am pretty sure that you are reviewing the latest version I sent. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-01 11:38 UTC
[Xen-devel] Re: [PATCH 04/13] Xen PCI platform device driver
On Wed, 30 Jun 2010, Konrad Rzeszutek Wilk wrote:> > @@ -476,9 +503,28 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx) > > > > int gnttab_resume(void) > > { > > - if (max_nr_grant_frames() < nr_grant_frames) > > + unsigned int max_nr_gframes; > > + > > + max_nr_gframes = max_nr_grant_frames(); > > + if (max_nr_gframes < nr_grant_frames) > > return -ENOSYS; > > - return gnttab_map(0, nr_grant_frames - 1); > > + > > + if (xen_pv_domain()) > > + return gnttab_map(0, nr_grant_frames - 1); > > + > > + if (!hvm_pv_resume_frames) { > > + hvm_pv_resume_frames = alloc_xen_mmio(PAGE_SIZE * max_nr_gframes); > > + shared = ioremap(hvm_pv_resume_frames, PAGE_SIZE * max_nr_gframes); > > + if (shared == NULL) { > > + printk(KERN_WARNING > > + "Fail to ioremap gnttab share frames\n"); > > I would say: "Failed to ioremap gnttab share frames!\n". > > .. snip .. > > > + if (request_region(ioaddr, iolen, DRV_NAME) == NULL) { > > + dev_err(&pdev->dev, "I/O resource 0x%lx @ 0x%lx busy\n", > > + iolen, ioaddr); > > + ret = -EBUSY; > > + goto out; > > + } > > + > > + platform_mmio = mmio_addr; > > + platform_mmiolen = mmio_len; > > + > > + if (!xen_have_vector_callback) { > > + ret = xen_allocate_irq(pdev); > > + if (ret) { > > + printk(KERN_WARNING "request_irq failed err=%d\n", ret); > Use dev_warn here. > > + goto out; > > + } > > + callback_via = get_callback_via(pdev); > > + ret = xen_set_callback_via(callback_via); > > + if (ret) { > > + printk(KERN_WARNING > > + "Unable to set the evtchn callback err=%d\n", ret); > ditto. > > + goto out; > > + } > > + } > > + > > + ret = gnttab_init(); > > + if (ret) > > + goto out; > > + xenbus_probe(NULL); > > + > > +out: > > + if (ret) { > > + release_mem_region(mmio_addr, mmio_len); > > + release_region(ioaddr, iolen); > > + pci_disable_device(pdev); > > + } > > + > > + return ret; > > +} > > + > > +static struct pci_device_id platform_pci_tbl[] __devinitdata = { > > + {PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM, > > + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, > > + {0,} > > +}; > > + > > +MODULE_DEVICE_TABLE(pci, platform_pci_tbl); > > + > > +static struct pci_driver platform_driver = { > > + .name = DRV_NAME, > > + .probe = platform_pci_init, > > + .id_table = platform_pci_tbl, > > +}; > > + > > +static int __init platform_pci_module_init(void) > > +{ > > + int rc; > > + > > + rc = pci_register_driver(&platform_driver); > > + if (rc) { > > + printk(KERN_INFO DRV_NAME > > + ": No platform pci device model found\n"); > > No need for that. The pci_register_driver call to ->probe and > its subsequent setup, if it failed, should have displayed the > appropiate error+explanation? > > How about just making it: > > return pci_register_driver(&platform_driver); > >Thanks for all the comments and suggestions, I''ll make the changes. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Don Dutile
2010-Jul-01 19:41 UTC
[Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
stefano@stabellini.net wrote:> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Add a xen_emul_unplug command line option to the kernel to unplug > xen emulated disks and nics. > > Set the default value of xen_emul_unplug depending on whether or > not the Xen PV frontends and the Xen platform PCI driver have > been compiled for this kernel (modules or built-in are both OK). > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > Documentation/kernel-parameters.txt | 11 +++ > arch/x86/xen/Makefile | 2 +- > arch/x86/xen/enlighten.c | 1 + > arch/x86/xen/platform-pci-unplug.c | 130 +++++++++++++++++++++++++++++++++++ > arch/x86/xen/xen-ops.h | 1 + > drivers/xen/platform-pci.c | 4 + > drivers/xen/xenbus/xenbus_probe.c | 4 + > include/xen/platform_pci.h | 49 +++++++++++++ > 8 files changed, 201 insertions(+), 1 deletions(-) > create mode 100644 arch/x86/xen/platform-pci-unplug.c > create mode 100644 include/xen/platform_pci.h > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > index 839b21b..716eea8 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -113,6 +113,7 @@ parameter is applicable: > More X86-64 boot options can be found in > Documentation/x86/x86_64/boot-options.txt . > X86 Either 32bit or 64bit x86 (same as X86-32+X86-64) > + XEN Xen support is enabled > > In addition, the following text indicates that the option: > > @@ -2834,6 +2835,16 @@ and is between 256 and 4096 characters. It is defined in the file > xd= [HW,XT] Original XT pre-IDE (RLL encoded) disks. > xd_geo= See header of drivers/block/xd.c. > > + xen_emul_unplug= [HW,X86,XEN] > + Unplug Xen emulated devices > + Format: [unplug0,][unplug1] > + ide-disks -- unplug primary master IDE devices > + aux-ide-disks -- unplug non-primary-master IDE devices > + nics -- unplug network devices > + all -- unplug all emulated devices (NICs and IDE disks) > + ignore -- continue loading the Xen platform PCI driver even > + if the version check failed > + > xirc2ps_cs= [NET,PCMCIA] > Format: > <irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]] > diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile > index 3bb4fc2..9309546 100644 > --- a/arch/x86/xen/Makefile > +++ b/arch/x86/xen/Makefile > @@ -12,7 +12,7 @@ CFLAGS_mmu.o := $(nostackp) > > obj-y := enlighten.o setup.o multicalls.o mmu.o irq.o \ > time.o xen-asm.o xen-asm_$(BITS).o \ > - grant-table.o suspend.o > + grant-table.o suspend.o platform-pci-unplug.o > > obj-$(CONFIG_SMP) += smp.o > obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= spinlock.o > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index bcd98a9..5126b53 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -1317,6 +1317,7 @@ void __init xen_hvm_guest_init(void) > if (xen_feature(XENFEAT_hvm_callback_vector)) > xen_have_vector_callback = 1; > register_cpu_notifier(&xen_hvm_cpu_notifier); > + xen_unplug_emulated_devices(); > have_vcpu_info_placement = 0; > x86_init.irqs.intr_init = xen_init_IRQ; > xen_hvm_init_time_ops(); > diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c > new file mode 100644 > index 0000000..d686440 > --- /dev/null > +++ b/arch/x86/xen/platform-pci-unplug.c > @@ -0,0 +1,130 @@ > +/****************************************************************************** > + * platform-pci-unplug.c > + * > + * Xen platform PCI device driver > + * Copyright (c) 2010, Citrix > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple > + * Place - Suite 330, Boston, MA 02111-1307 USA. > + * > + */ > + > +#include <asm/io.h> > + > +#include <linux/init.h> > +#include <linux/module.h> > + > +#include <xen/platform_pci.h> > + > +/* boolean to signal that the platform pci device can be used */ > +bool xen_platform_pci_enabled; > +EXPORT_SYMBOL_GPL(xen_platform_pci_enabled); > +static int xen_emul_unplug; > + > +static int __init check_platform_magic(void) > +{ > + short magic; > + char protocol; > + > + magic = inw(XEN_IOPORT_MAGIC); > + if (magic != XEN_IOPORT_MAGIC_VAL) { > + printk(KERN_ERR "Xen Platform PCI: unrecognised magic value\n"); > + return -1; > + } > + > + protocol = inb(XEN_IOPORT_PROTOVER); > + > + printk(KERN_DEBUG "Xen Platform PCI: I/O protocol version %d\n", > + protocol); > + > + switch (protocol) { > + case 1: > + outw(XEN_IOPORT_LINUX_PRODNUM, XEN_IOPORT_PRODNUM); > + outl(XEN_IOPORT_LINUX_DRVVER, XEN_IOPORT_DRVVER); > + if (inw(XEN_IOPORT_MAGIC) != XEN_IOPORT_MAGIC_VAL) { > + printk(KERN_ERR "Xen Platform: blacklisted by host\n"); > + return -3; > + } > + break; > + default: > + printk(KERN_WARNING "Xen Platform PCI: unknown I/O protocol version"); > + return -2; > + } > + > + return 0; > +} > + > +void __init xen_unplug_emulated_devices(void) > +{ > + int r; > + > + /* check the version of the xen platform PCI device */ > + r = check_platform_magic(); > + /* If the version matches enable the Xen platform PCI driver. > + * Also enable the Xen platform PCI driver if the version is really old > + * and the user told us to ignore it. */ > + if (!r || (r == -1 && (xen_emul_unplug & XEN_UNPLUG_IGNORE))) > + xen_platform_pci_enabled = 1; > + /* Set the default value of xen_emul_unplug depending on whether or > + * not the Xen PV frontends and the Xen platform PCI driver have > + * been compiled for this kernel (modules or built-in are both OK). */ > + if (xen_platform_pci_enabled && !xen_emul_unplug) { > + if (xen_must_unplug_nics()) { > + printk(KERN_INFO "Netfront and the Xen platform PCI driver have " > + "been compiled for this kernel: unplug emulated NICs.\n"); > + xen_emul_unplug |= XEN_UNPLUG_ALL_NICS; > + } > + if (xen_must_unplug_disks()) { > + printk(KERN_INFO "Blkfront and the Xen platform PCI driver have " > + "been compiled for this kernel: unplug emulated disks.\n" > + "You might have to change the root device\n" > + "from /dev/hd[a-d] to /dev/xvd[a-d]\n" > + "in your root= kernel command line option\n"); > + xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS; > + } > + } > + /* Now unplug the emulated devices */ > + if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE)) > + outw(xen_emul_unplug, XEN_IOPORT_UNPLUG); > +} > + > +static int __init parse_xen_emul_unplug(char *arg) > +{ > + char *p, *q; > + int l; > + > + for (p = arg; p; p = q) { > + q = strchr(p, '',''); > + if (q) { > + l = q - p; > + q++; > + } else { > + l = strlen(p); > + } > + if (!strncmp(p, "all", l)) > + xen_emul_unplug |= XEN_UNPLUG_ALL; > + else if (!strncmp(p, "ide-disks", l)) > + xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS; > + else if (!strncmp(p, "aux-ide-disks", l)) > + xen_emul_unplug |= XEN_UNPLUG_AUX_IDE_DISKS; > + else if (!strncmp(p, "nics", l)) > + xen_emul_unplug |= XEN_UNPLUG_ALL_NICS; > + else if (!strncmp(p, "ignore", l)) > + xen_emul_unplug |= XEN_UNPLUG_IGNORE; > + else > + printk(KERN_WARNING "unrecognised option ''%s'' " > + "in module parameter ''dev_unplug''\n", p); > + } > + return 0; > +} > +early_param("xen_emul_unplug", parse_xen_emul_unplug); > diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h > index 089d189..ed77694 100644 > --- a/arch/x86/xen/xen-ops.h > +++ b/arch/x86/xen/xen-ops.h > @@ -40,6 +40,7 @@ void xen_vcpu_restore(void); > > void xen_callback_vector(void); > void xen_hvm_init_shared_info(void); > +void __init xen_unplug_emulated_devices(void); > > void __init xen_build_dynamic_phys_to_machine(void); > > diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c > index 10b92ec..35f162d 100644 > --- a/drivers/xen/platform-pci.c > +++ b/drivers/xen/platform-pci.c > @@ -27,6 +27,7 @@ > #include <linux/module.h> > #include <linux/pci.h> > > +#include <xen/platform_pci.h> > #include <xen/grant_table.h> > #include <xen/xenbus.h> > #include <xen/events.h> > @@ -195,6 +196,9 @@ static int __init platform_pci_module_init(void) > { > int rc; > > + if (!xen_platform_pci_enabled) > + return -ENODEV; > +The problem with this check/enable is that if you run this on an older qemu-xen that doesn''t have unplug support, it fails pv-hvm configuration. But, all that means is that you can''t use an xvd as the boot device, and you have to use an emulated IDE device as boot device. There are a couple ways to configure the vnif correctly (in guest or in xen guest config file). So, on an older (say, rhel5) xen, I don''t have this check; the boot device is required to be spec''d as hda, not vda, and xen-blkfront is not allowed to configure blk major nums for IDE (& SCSI) (to avoid 2 drivers twiddling w/same phys backend... not good!). When that older qemu is updated, this restriction will be lifted (in the guest kernel & related xen guest config file spec).> rc = pci_register_driver(&platform_driver); > if (rc) { > printk(KERN_INFO DRV_NAME > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > index 3d941ec..243279a 100644 > --- a/drivers/xen/xenbus/xenbus_probe.c > +++ b/drivers/xen/xenbus/xenbus_probe.c > @@ -56,6 +56,7 @@ > #include <xen/events.h> > #include <xen/page.h> > > +#include <xen/platform_pci.h> > #include <xen/hvm.h> > > #include "xenbus_comms.h" > @@ -977,6 +978,9 @@ static void wait_for_devices(struct xenbus_driver *xendrv) > #ifndef MODULE > static int __init boot_wait_for_devices(void) > { > + if (xen_hvm_domain() && !xen_platform_pci_enabled) > + return -ENODEV; > +This check also fails on qemu-xen that doesn''t support ide-unplug but pv-hvm is configured in (aka, on rhel5 today). The flag should be something different.> ready_to_wait_for_devices = 1; > wait_for_devices(NULL); > return 0; > diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h > new file mode 100644 > index 0000000..afa8855 > --- /dev/null > +++ b/include/xen/platform_pci.h > @@ -0,0 +1,49 @@ > +#ifndef _XEN_PLATFORM_PCI_H > +#define _XEN_PLATFORM_PCI_H > + > +#define XEN_IOPORT_MAGIC_VAL 0x49d2 > +#define XEN_IOPORT_LINUX_PRODNUM 0x0003 > +#define XEN_IOPORT_LINUX_DRVVER 0x0001 > + > +#define XEN_IOPORT_BASE 0x10 > + > +#define XEN_IOPORT_PLATFLAGS (XEN_IOPORT_BASE + 0) /* 1 byte access (R/W) */ > +#define XEN_IOPORT_MAGIC (XEN_IOPORT_BASE + 0) /* 2 byte access (R) */ > +#define XEN_IOPORT_UNPLUG (XEN_IOPORT_BASE + 0) /* 2 byte access (W) */ > +#define XEN_IOPORT_DRVVER (XEN_IOPORT_BASE + 0) /* 4 byte access (W) */ > + > +#define XEN_IOPORT_SYSLOG (XEN_IOPORT_BASE + 2) /* 1 byte access (W) */ > +#define XEN_IOPORT_PROTOVER (XEN_IOPORT_BASE + 2) /* 1 byte access (R) */ > +#define XEN_IOPORT_PRODNUM (XEN_IOPORT_BASE + 2) /* 2 byte access (W) */ > + > +#define XEN_UNPLUG_ALL_IDE_DISKS 1 > +#define XEN_UNPLUG_ALL_NICS 2 > +#define XEN_UNPLUG_AUX_IDE_DISKS 4 > +#define XEN_UNPLUG_ALL 7 > +#define XEN_UNPLUG_IGNORE 8 > + > +static inline int xen_must_unplug_nics(void) { > +#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \ > + defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \ > + (defined(CONFIG_XEN_PLATFORM_PCI) || \ > + defined(CONFIG_XEN_PLATFORM_PCI_MODULE)) > + return 1; > +#else > + return 0; > +#endif > +} > + > +static inline int xen_must_unplug_disks(void) { > +#if (defined(CONFIG_XEN_BLKDEV_FRONTEND) || \ > + defined(CONFIG_XEN_BLKDEV_FRONTEND_MODULE)) && \ > + (defined(CONFIG_XEN_PLATFORM_PCI) || \ > + defined(CONFIG_XEN_PLATFORM_PCI_MODULE)) > + return 1; > +#else > + return 0; > +#endif > +} > + > +extern bool xen_platform_pci_enabled; > + > +#endif /* _XEN_PLATFORM_PCI_H */_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Don Dutile
2010-Jul-01 19:41 UTC
[Xen-devel] Re: [PATCH 11/13] Use xen_vcpuop_clockevent, xen_clocksource and xen wallclock.
stefano@stabellini.net wrote:> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Use xen_vcpuop_clockevent instead of hpet and APIC timers as main > clockevent device on all vcpus, use the xen wallclock time as wallclock > instead of rtc and use xen_clocksource as clocksource. > The pv clock algorithm needs to work correctly for the xen_clocksource > and xen wallclock to be usable, only modern Xen versions offer a > reliable pv clock in HVM guests (XENFEAT_hvm_safe_pvclock). > > Using the hpet as clocksource means a VMEXIT every time we read/write to > the hpet mmio addresses, pvclock give us a better rating without > VMEXITs. Same goes for the xen wallclock and xen_vcpuop_clockevent > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > arch/x86/xen/enlighten.c | 14 +-------- > arch/x86/xen/suspend.c | 4 ++ > arch/x86/xen/time.c | 59 ++++++++++++++++++++++++++++++++++--- > arch/x86/xen/xen-ops.h | 7 +--- > include/xen/interface/features.h | 3 ++ > 5 files changed, 65 insertions(+), 22 deletions(-) > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index 90bac21..bcd98a9 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -933,10 +933,6 @@ static const struct pv_init_ops xen_init_ops __initdata = { > .patch = xen_patch, > }; > > -static const struct pv_time_ops xen_time_ops __initdata = { > - .sched_clock = xen_sched_clock, > -}; > - > static const struct pv_cpu_ops xen_cpu_ops __initdata = { > .cpuid = xen_cpuid, > > @@ -1074,7 +1070,6 @@ asmlinkage void __init xen_start_kernel(void) > /* Install Xen paravirt ops */ > pv_info = xen_info; > pv_init_ops = xen_init_ops; > - pv_time_ops = xen_time_ops; > pv_cpu_ops = xen_cpu_ops; > pv_apic_ops = xen_apic_ops; > > @@ -1082,13 +1077,7 @@ asmlinkage void __init xen_start_kernel(void) > x86_init.oem.arch_setup = xen_arch_setup; > x86_init.oem.banner = xen_banner; > > - x86_init.timers.timer_init = xen_time_init; > - x86_init.timers.setup_percpu_clockev = x86_init_noop; > - x86_cpuinit.setup_percpu_clockev = x86_init_noop; > - > - x86_platform.calibrate_tsc = xen_tsc_khz; > - x86_platform.get_wallclock = xen_get_wallclock; > - x86_platform.set_wallclock = xen_set_wallclock; > + xen_init_time_ops(); > > /* > * Set up some pagetable state before starting to set any ptes. > @@ -1330,4 +1319,5 @@ void __init xen_hvm_guest_init(void) > register_cpu_notifier(&xen_hvm_cpu_notifier); > have_vcpu_info_placement = 0; > x86_init.irqs.intr_init = xen_init_IRQ; > + xen_hvm_init_time_ops(); > } > diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c > index 6ff9665..0774c67 100644 > --- a/arch/x86/xen/suspend.c > +++ b/arch/x86/xen/suspend.c > @@ -28,8 +28,12 @@ void xen_pre_suspend(void) > > void xen_hvm_post_suspend(int suspend_cancelled) > { > + int cpu; > xen_hvm_init_shared_info(); > xen_callback_vector(); > + for_each_online_cpu(cpu) { > + xen_setup_runstate_info(cpu); > + } > } >As I found out on older xen (non ''modern Xen version'' ;-) ) w/o XENFEAT_hvm_safe_pvclock, the above patch should look like: if (xen_feature(XENFEAT_hvm_safe_pvclock)) { int cpu; for_each_online_cpu(cpu) { xen_setup_runstate_info(cpu); } } - Don> void xen_post_suspend(int suspend_cancelled) > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c > index 32764b8..2d76c28 100644 > --- a/arch/x86/xen/time.c > +++ b/arch/x86/xen/time.c > @@ -20,6 +20,7 @@ > #include <asm/xen/hypercall.h> > > #include <xen/events.h> > +#include <xen/features.h> > #include <xen/interface/xen.h> > #include <xen/interface/vcpu.h> > > @@ -160,7 +161,7 @@ static void do_stolen_accounting(void) > * nanoseconds, which is nanoseconds the VCPU spent in RUNNING+BLOCKED > * states. > */ > -unsigned long long xen_sched_clock(void) > +static unsigned long long xen_sched_clock(void) > { > struct vcpu_runstate_info state; > cycle_t now; > @@ -195,7 +196,7 @@ unsigned long long xen_sched_clock(void) > > > /* Get the TSC speed from Xen */ > -unsigned long xen_tsc_khz(void) > +static unsigned long xen_tsc_khz(void) > { > struct pvclock_vcpu_time_info *info > &HYPERVISOR_shared_info->vcpu_info[0].time; > @@ -230,7 +231,7 @@ static void xen_read_wallclock(struct timespec *ts) > put_cpu_var(xen_vcpu); > } > > -unsigned long xen_get_wallclock(void) > +static unsigned long xen_get_wallclock(void) > { > struct timespec ts; > > @@ -238,7 +239,7 @@ unsigned long xen_get_wallclock(void) > return ts.tv_sec; > } > > -int xen_set_wallclock(unsigned long now) > +static int xen_set_wallclock(unsigned long now) > { > /* do nothing for domU */ > return -1; > @@ -473,7 +474,11 @@ void xen_timer_resume(void) > } > } > > -__init void xen_time_init(void) > +static const struct pv_time_ops xen_time_ops __initdata = { > + .sched_clock = xen_sched_clock, > +}; > + > +static __init void xen_time_init(void) > { > int cpu = smp_processor_id(); > > @@ -497,3 +502,47 @@ __init void xen_time_init(void) > xen_setup_timer(cpu); > xen_setup_cpu_clockevents(); > } > + > +__init void xen_init_time_ops(void) > +{ > + pv_time_ops = xen_time_ops; > + > + x86_init.timers.timer_init = xen_time_init; > + x86_init.timers.setup_percpu_clockev = x86_init_noop; > + x86_cpuinit.setup_percpu_clockev = x86_init_noop; > + > + x86_platform.calibrate_tsc = xen_tsc_khz; > + x86_platform.get_wallclock = xen_get_wallclock; > + x86_platform.set_wallclock = xen_set_wallclock; > +} > + > +static void xen_hvm_setup_cpu_clockevents(void) > +{ > + int cpu = smp_processor_id(); > + xen_setup_runstate_info(cpu); > + xen_setup_timer(cpu); > + xen_setup_cpu_clockevents(); > +} > + > +__init void xen_hvm_init_time_ops(void) > +{ > + /* vector callback is needed otherwise we cannot receive interrupts > + * on cpu > 0 */ > + if (!xen_have_vector_callback && num_present_cpus() > 1) > + return; > + if (!xen_feature(XENFEAT_hvm_safe_pvclock)) { > + printk(KERN_INFO "Xen doesn''t support pvclock on HVM," > + "disable pv timer\n"); > + return; > + } > + > + pv_time_ops = xen_time_ops; > + x86_init.timers.timer_init = xen_time_init; > + x86_init.timers.setup_percpu_clockev = x86_init_noop; > + x86_cpuinit.setup_percpu_clockev = xen_hvm_setup_cpu_clockevents; > + > + x86_platform.calibrate_tsc = xen_tsc_khz; > + x86_platform.get_wallclock = xen_get_wallclock; > + x86_platform.set_wallclock = xen_set_wallclock; > +} > + > diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h > index 01c9dd3..089d189 100644 > --- a/arch/x86/xen/xen-ops.h > +++ b/arch/x86/xen/xen-ops.h > @@ -49,11 +49,8 @@ void xen_setup_runstate_info(int cpu); > void xen_teardown_timer(int cpu); > cycle_t xen_clocksource_read(void); > void xen_setup_cpu_clockevents(void); > -unsigned long xen_tsc_khz(void); > -void __init xen_time_init(void); > -unsigned long xen_get_wallclock(void); > -int xen_set_wallclock(unsigned long time); > -unsigned long long xen_sched_clock(void); > +void __init xen_init_time_ops(void); > +void __init xen_hvm_init_time_ops(void); > > irqreturn_t xen_debug_interrupt(int irq, void *dev_id); > > diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h > index 8ab08b9..70d2563 100644 > --- a/include/xen/interface/features.h > +++ b/include/xen/interface/features.h > @@ -44,6 +44,9 @@ > /* x86: Does this Xen host support the HVM callback vector type? */ > #define XENFEAT_hvm_callback_vector 8 > > +/* x86: pvclock algorithm is safe to use on HVM */ > +#define XENFEAT_hvm_safe_pvclock 9 > + > #define XENFEAT_NR_SUBMAPS 1 > > #endif /* __XEN_PUBLIC_FEATURES_H__ */_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hi Stefano -- What versions of Xen are compatible with this series? I.e. is 4.0 required? Are all Xen-side changes in official 4.0.0? Or only in 4.0-testing (and therefore will be in 4.0.1)? Or only in xen-unstable? If post-4.0.0 xen changes are necessary, could you point out the changesets? Thanks, Dan P.S. Sorry if I missed earlier discussion or documentation on this... but if not otherwise documented, maybe the info should be in the patch comment?> -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Thursday, July 01, 2010 5:39 AM > To: Konrad Rzeszutek Wilk > Cc: Fitzhardinge; xen-devel@lists.xensource.com; > Jeremy@acsinet12.oracle.com; Stefano Stabellini; linux- > kernel@vger.kernel.org; Don Dutile; Sheng Yang > Subject: [Xen-devel] Re: [PATCH 0/12] PV on HVM Xen > > On Wed, 30 Jun 2010, Konrad Rzeszutek Wilk wrote: > > On Mon, Jun 21, 2010 at 05:12:20PM +0100, Stefano Stabellini wrote: > > > Hi all, > > > this is another update on the PV on HVM Xen series; a list of > changes > > > compared to the previous version follows: > > > > Can you use the [v4] in the main patch (this is the fourth version of > > these patches, right?)? I am getting confused as to which version I > am > > reviewing and if I''ve missed one or what not. > > > > And in this first intro email state: > > > > Since v7 the following things have changed: > > - Konrad made some comments. > > - etc. > > > > Good point, I''ll do that from now on. > I am pretty sure that you are reviewing the latest version I sent. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Thu, 1 Jul 2010, Dan Magenheimer wrote:> Hi Stefano -- > > What versions of Xen are compatible with this series? > I.e. is 4.0 required? Are all Xen-side changes in > official 4.0.0? Or only in 4.0-testing (and therefore > will be in 4.0.1)? Or only in xen-unstable? > > If post-4.0.0 xen changes are necessary, could you point > out the changesets? >Any Xen version is going to work, but you need unstable to get all the features. In particular you need these patches: - 21647: implement HVMOP_pagetable_dying - 21493: update_runstate_area for 32 bit PV on HVM guests - 21449: implement vector callback for evtchn delivery - 21445: TSC handling cleanups (version 2) - 21341: Export timer hypercalls to HVM guests too - 21339: TSC handling cleanups _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Paolo Bonzini
2010-Jul-02 10:44 UTC
[Xen-devel] Re: [PATCH 10/13] Do not try to disable hpet if it hasn''t been initialized before
On 06/21/2010 06:14 PM, stefano@stabellini.net wrote:> - if (is_hpet_capable()) { > - unsigned int cfg = hpet_readl(HPET_CFG); > + unsigned int cfg;You changed this earlier from unsigned long to unsigned int, but it is wrong (though it works because bits 32-63 of the HPET_CFG register are unused). Can you please make a note to change it back sometime? Thanks, Paolo _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > On Thu, 1 Jul 2010, Dan Magenheimer wrote: > > Hi Stefano -- > > > > What versions of Xen are compatible with this series? > > I.e. is 4.0 required? Are all Xen-side changes in > > official 4.0.0? Or only in 4.0-testing (and therefore > > will be in 4.0.1)? Or only in xen-unstable? > > > > If post-4.0.0 xen changes are necessary, could you point > > out the changesets? > > Any Xen version is going to work, but you need unstable to get all the > features. > > In particular you need these patches: > > - 21647: implement HVMOP_pagetable_dying > > - 21493: update_runstate_area for 32 bit PV on HVM guests > > - 21449: implement vector callback for evtchn delivery > > - 21445: TSC handling cleanups (version 2) > > - 21341: Export timer hypercalls to HVM guests too > > - 21339: TSC handling cleanupsThanks for the thorough reply! Any reason that any of these cannot or should not be back-ported to go into 4.0.1? Thanks, Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2 Jul 2010, Dan Magenheimer wrote:> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > > On Thu, 1 Jul 2010, Dan Magenheimer wrote: > > > Hi Stefano -- > > > > > > What versions of Xen are compatible with this series? > > > I.e. is 4.0 required? Are all Xen-side changes in > > > official 4.0.0? Or only in 4.0-testing (and therefore > > > will be in 4.0.1)? Or only in xen-unstable? > > > > > > If post-4.0.0 xen changes are necessary, could you point > > > out the changesets? > > > > Any Xen version is going to work, but you need unstable to get all the > > features. > > > > In particular you need these patches: > > > > - 21647: implement HVMOP_pagetable_dying > > > > - 21493: update_runstate_area for 32 bit PV on HVM guests > > > > - 21449: implement vector callback for evtchn delivery > > > > - 21445: TSC handling cleanups (version 2) > > > > - 21341: Export timer hypercalls to HVM guests too > > > > - 21339: TSC handling cleanups > > Thanks for the thorough reply! Any reason that any of these > cannot or should not be back-ported to go into 4.0.1? >They should be safe to backport, the only ones we should be extra careful are the TSC related patches, and you were the one to raise that complain :) If you have some spare time to test 21445 and 21339 on 4.0.1 to make sure that everything keeps working as expected, then we can safely apply them to xen 4.0 testing. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-02 17:17 UTC
[Xen-devel] Re: [PATCH 11/13] Use xen_vcpuop_clockevent, xen_clocksource and xen wallclock.
On Thu, Jul 1, 2010 at 8:41 PM, Don Dutile <ddutile@redhat.com> wrote:> stefano@stabellini.net wrote: >> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> Use xen_vcpuop_clockevent instead of hpet and APIC timers as main >> clockevent device on all vcpus, use the xen wallclock time as wallclock >> instead of rtc and use xen_clocksource as clocksource. >> The pv clock algorithm needs to work correctly for the xen_clocksource >> and xen wallclock to be usable, only modern Xen versions offer a >> reliable pv clock in HVM guests (XENFEAT_hvm_safe_pvclock). >> >> Using the hpet as clocksource means a VMEXIT every time we read/write to >> the hpet mmio addresses, pvclock give us a better rating without >> VMEXITs. Same goes for the xen wallclock and xen_vcpuop_clockevent >> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> --- >> arch/x86/xen/enlighten.c | 14 +-------- >> arch/x86/xen/suspend.c | 4 ++ >> arch/x86/xen/time.c | 59 ++++++++++++++++++++++++++++++++++--- >> arch/x86/xen/xen-ops.h | 7 +--- >> include/xen/interface/features.h | 3 ++ >> 5 files changed, 65 insertions(+), 22 deletions(-) >> >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> index 90bac21..bcd98a9 100644 >> --- a/arch/x86/xen/enlighten.c >> +++ b/arch/x86/xen/enlighten.c >> @@ -933,10 +933,6 @@ static const struct pv_init_ops xen_init_ops __initdata = { >> .patch = xen_patch, >> }; >> >> -static const struct pv_time_ops xen_time_ops __initdata = { >> - .sched_clock = xen_sched_clock, >> -}; >> - >> static const struct pv_cpu_ops xen_cpu_ops __initdata = { >> .cpuid = xen_cpuid, >> >> @@ -1074,7 +1070,6 @@ asmlinkage void __init xen_start_kernel(void) >> /* Install Xen paravirt ops */ >> pv_info = xen_info; >> pv_init_ops = xen_init_ops; >> - pv_time_ops = xen_time_ops; >> pv_cpu_ops = xen_cpu_ops; >> pv_apic_ops = xen_apic_ops; >> >> @@ -1082,13 +1077,7 @@ asmlinkage void __init xen_start_kernel(void) >> x86_init.oem.arch_setup = xen_arch_setup; >> x86_init.oem.banner = xen_banner; >> >> - x86_init.timers.timer_init = xen_time_init; >> - x86_init.timers.setup_percpu_clockev = x86_init_noop; >> - x86_cpuinit.setup_percpu_clockev = x86_init_noop; >> - >> - x86_platform.calibrate_tsc = xen_tsc_khz; >> - x86_platform.get_wallclock = xen_get_wallclock; >> - x86_platform.set_wallclock = xen_set_wallclock; >> + xen_init_time_ops(); >> >> /* >> * Set up some pagetable state before starting to set any ptes. >> @@ -1330,4 +1319,5 @@ void __init xen_hvm_guest_init(void) >> register_cpu_notifier(&xen_hvm_cpu_notifier); >> have_vcpu_info_placement = 0; >> x86_init.irqs.intr_init = xen_init_IRQ; >> + xen_hvm_init_time_ops(); >> } >> diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c >> index 6ff9665..0774c67 100644 >> --- a/arch/x86/xen/suspend.c >> +++ b/arch/x86/xen/suspend.c >> @@ -28,8 +28,12 @@ void xen_pre_suspend(void) >> >> void xen_hvm_post_suspend(int suspend_cancelled) >> { >> + int cpu; >> xen_hvm_init_shared_info(); >> xen_callback_vector(); >> + for_each_online_cpu(cpu) { >> + xen_setup_runstate_info(cpu); >> + } >> } >> > > As I found out on older xen (non ''modern Xen version'' ;-) ) > w/o XENFEAT_hvm_safe_pvclock, the above patch should look like: > > if (xen_feature(XENFEAT_hvm_safe_pvclock)) { > int cpu; > for_each_online_cpu(cpu) { > xen_setup_runstate_info(cpu); > } > } >Thank you for testing my series and for the patch, I''ll include it in the next version. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > > Thanks for the thorough reply! Any reason that any of these > > cannot or should not be back-ported to go into 4.0.1? > > They should be safe to backport, the only ones we should be extra > careful are the TSC related patches, and you were the one to raise that > complain :) > If you have some spare time to test 21445 and 21339 on 4.0.1 to make > sure that everything keeps working as expected, then we can safely > apply them to xen 4.0 testing.Thanks, yes, I was putting that task on my list and wasn''t sure what the full set of required changes were and if there were interdependencies so that I could test on 4.0-testing instead of on xen-unstable. (Depending on Keir''s 4.0.1 schedule, I might not get it done in time for 4.0.1 though.) Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-05 11:58 UTC
[Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
On Thu, 1 Jul 2010, Don Dutile wrote:> The problem with this check/enable is that if you run > this on an older qemu-xen that doesn''t have unplug support, > it fails pv-hvm configuration. > > But, all that means is that you can''t use an xvd as the boot device, > and you have to use an emulated IDE device as boot device. > There are a couple ways to configure the vnif correctly (in guest > or in xen guest config file). > > So, on an older (say, rhel5) xen, I don''t have this check; > the boot device is required to be spec''d as hda, not vda, and > xen-blkfront is not allowed to configure blk major nums > for IDE (& SCSI) (to avoid 2 drivers twiddling w/same phys backend... not good!). >Who is requiring that the boot device is spec''d as hda and not xvda? I don''t think there is such limitation in xend or libxl at the moment. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Don Dutile
2010-Jul-07 20:01 UTC
[Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
Stefano Stabellini wrote:> On Thu, 1 Jul 2010, Don Dutile wrote: >> The problem with this check/enable is that if you run >> this on an older qemu-xen that doesn''t have unplug support, >> it fails pv-hvm configuration. >> >> But, all that means is that you can''t use an xvd as the boot device, >> and you have to use an emulated IDE device as boot device. >> There are a couple ways to configure the vnif correctly (in guest >> or in xen guest config file). >> >> So, on an older (say, rhel5) xen, I don''t have this check; >> the boot device is required to be spec''d as hda, not vda, and >> xen-blkfront is not allowed to configure blk major nums >> for IDE (& SCSI) (to avoid 2 drivers twiddling w/same phys backend... not good!). >> > > Who is requiring that the boot device is spec''d as hda and not xvda? > I don''t think there is such limitation in xend or libxl at the moment. >If you take a previous xen HVM guest spec & just run it on a guest that has pv-hvm added to it, then one has hda spec''d as boot device (by default, by not editing the guest config spec/file). Ideally, both config specs should/would work. - Don _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-08 13:13 UTC
[Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
On Wed, 7 Jul 2010, Don Dutile wrote:> Stefano Stabellini wrote: > > On Thu, 1 Jul 2010, Don Dutile wrote: > >> The problem with this check/enable is that if you run > >> this on an older qemu-xen that doesn''t have unplug support, > >> it fails pv-hvm configuration. > >> > >> But, all that means is that you can''t use an xvd as the boot device, > >> and you have to use an emulated IDE device as boot device. > >> There are a couple ways to configure the vnif correctly (in guest > >> or in xen guest config file). > >> > >> So, on an older (say, rhel5) xen, I don''t have this check; > >> the boot device is required to be spec''d as hda, not vda, and > >> xen-blkfront is not allowed to configure blk major nums > >> for IDE (& SCSI) (to avoid 2 drivers twiddling w/same phys backend... not good!). > >> > > > > Who is requiring that the boot device is spec''d as hda and not xvda? > > I don''t think there is such limitation in xend or libxl at the moment. > > > > If you take a previous xen HVM guest spec & just run it on a guest > that has pv-hvm added to it, then one has hda spec''d as boot device (by default, > by not editing the guest config spec/file). > > Ideally, both config specs should/would work.I wouldn''t want to cause data corruptions by default to people that specified xvda in their HVM config files by mistake (for example because they copied and pasted from a PV guest config file). But I agree that we should be able to do the right thing in your case scenario too. I propose the appended patch (to be merge with "Unplug emulated disks and nics"): if the user specifies xen_emul_unplug=ignore and the unplug protocol is not supported (old xen installations like rhel5), we continue with the PV on HVM initialization and we make sure that blkfront doesn''t hook any IDE or SCSI device. Don, Jeremy, what do you think about it? --- diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c index 72a3da6..2f7f3fb 100644 --- a/arch/x86/xen/platform-pci-unplug.c +++ b/arch/x86/xen/platform-pci-unplug.c @@ -29,9 +29,9 @@ #define XEN_PLATFORM_ERR_PROTOCOL -2 #define XEN_PLATFORM_ERR_BLACKLIST -3 -/* boolean to signal that the platform pci device can be used */ -bool xen_platform_pci_enabled; -EXPORT_SYMBOL_GPL(xen_platform_pci_enabled); +/* store the value of xen_emul_unplug after the unplug is done */ +int xen_platform_pci_unplug; +EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); static int xen_emul_unplug; static int __init check_platform_magic(void) @@ -76,13 +76,13 @@ void __init xen_unplug_emulated_devices(void) /* If the version matches enable the Xen platform PCI driver. * Also enable the Xen platform PCI driver if the version is really old * and the user told us to ignore it. */ - if (!r || (r == XEN_PLATFORM_ERR_MAGIC && - (xen_emul_unplug & XEN_UNPLUG_IGNORE))) - xen_platform_pci_enabled = 1; + if (r && !(r == XEN_PLATFORM_ERR_MAGIC && + (xen_emul_unplug & XEN_UNPLUG_IGNORE))) + return; /* Set the default value of xen_emul_unplug depending on whether or * not the Xen PV frontends and the Xen platform PCI driver have * been compiled for this kernel (modules or built-in are both OK). */ - if (xen_platform_pci_enabled && !xen_emul_unplug) { + if (!xen_emul_unplug) { if (xen_must_unplug_nics()) { printk(KERN_INFO "Netfront and the Xen platform PCI driver have " "been compiled for this kernel: unplug emulated NICs.\n"); @@ -98,8 +98,9 @@ void __init xen_unplug_emulated_devices(void) } } /* Now unplug the emulated devices */ - if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE)) + if (!(xen_emul_unplug & XEN_UNPLUG_IGNORE)) outw(xen_emul_unplug, XEN_IOPORT_UNPLUG); + xen_platform_pci_unplug = xen_emul_unplug; } static int __init parse_xen_emul_unplug(char *arg) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 82ed403..6eb2989 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -48,6 +48,7 @@ #include <xen/grant_table.h> #include <xen/events.h> #include <xen/page.h> +#include <xen/platform_pci.h> #include <xen/interface/grant_table.h> #include <xen/interface/io/blkif.h> @@ -737,6 +738,22 @@ static int blkfront_probe(struct xenbus_device *dev, } } + /* no unplug has been done: do not hook devices != xen vbds */ + if (xen_hvm_domain() && (xen_platform_pci_unplug & XEN_UNPLUG_IGNORE)) { + int major; + + if (!VDEV_IS_EXTENDED(vdevice)) + major = BLKIF_MAJOR(vdevice); + else + major = XENVBD_MAJOR; + + if (major != XENVBD_MAJOR) { + printk(KERN_INFO + "%s: HVM does not support vbd %d as xen block device\n", + __FUNCTION__, vdevice); + return -ENODEV; + } + } info = kzalloc(sizeof(*info), GFP_KERNEL); if (!info) { xenbus_dev_fatal(dev, -ENOMEM, "allocating info structure"); diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c index be8b4f3..c01b5dd 100644 --- a/drivers/xen/platform-pci.c +++ b/drivers/xen/platform-pci.c @@ -196,7 +196,9 @@ static struct pci_driver platform_driver = { static int __init platform_pci_module_init(void) { - if (!xen_platform_pci_enabled) + /* no unplug has been done, IGNORE hasn''t been specified: just + * return now */ + if (!xen_platform_pci_unplug) return -ENODEV; return pci_register_driver(&platform_driver); diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c index 243279a..34287da 100644 --- a/drivers/xen/xenbus/xenbus_probe.c +++ b/drivers/xen/xenbus/xenbus_probe.c @@ -978,7 +978,7 @@ static void wait_for_devices(struct xenbus_driver *xendrv) #ifndef MODULE static int __init boot_wait_for_devices(void) { - if (xen_hvm_domain() && !xen_platform_pci_enabled) + if (xen_hvm_domain() && !xen_platform_pci_unplug) return -ENODEV; ready_to_wait_for_devices = 1; diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h index afa8855..ce9d671 100644 --- a/include/xen/platform_pci.h +++ b/include/xen/platform_pci.h @@ -44,6 +44,6 @@ static inline int xen_must_unplug_disks(void) { #endif } -extern bool xen_platform_pci_enabled; +extern int xen_platform_pci_unplug; #endif /* _XEN_PLATFORM_PCI_H */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Don Dutile
2010-Jul-08 19:57 UTC
Re: [Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
Stefano Stabellini wrote:> On Wed, 7 Jul 2010, Don Dutile wrote: >> Stefano Stabellini wrote: >>> On Thu, 1 Jul 2010, Don Dutile wrote: >>>> The problem with this check/enable is that if you run >>>> this on an older qemu-xen that doesn''t have unplug support, >>>> it fails pv-hvm configuration. >>>> >>>> But, all that means is that you can''t use an xvd as the boot device, >>>> and you have to use an emulated IDE device as boot device. >>>> There are a couple ways to configure the vnif correctly (in guest >>>> or in xen guest config file). >>>> >>>> So, on an older (say, rhel5) xen, I don''t have this check; >>>> the boot device is required to be spec''d as hda, not vda, and >>>> xen-blkfront is not allowed to configure blk major nums >>>> for IDE (& SCSI) (to avoid 2 drivers twiddling w/same phys backend... not good!). >>>> >>> Who is requiring that the boot device is spec''d as hda and not xvda? >>> I don''t think there is such limitation in xend or libxl at the moment. >>> >> If you take a previous xen HVM guest spec & just run it on a guest >> that has pv-hvm added to it, then one has hda spec''d as boot device (by default, >> by not editing the guest config spec/file). >> >> Ideally, both config specs should/would work. > > I wouldn''t want to cause data corruptions by default to people that > specified xvda in their HVM config files by mistake (for example because > they copied and pasted from a PV guest config file). > But I agree that we should be able to do the right thing in your case > scenario too. > > I propose the appended patch (to be merge with "Unplug emulated disks > and nics"): if the user specifies xen_emul_unplug=ignore and the unplug > protocol is not supported (old xen installations like rhel5), we > continue with the PV on HVM initialization and we make sure that > blkfront doesn''t hook any IDE or SCSI device. > > Don, Jeremy, what do you think about it? > >I guess what I''m wondering is why not set xen_emul_unplug to ignore by default (static int xen_emul_unplug=XEN_UNPLUG_IGNORE), which handles the case I mentioned (just take existing guest config file as is, no edits, pre-pv-hvm added to guest kernel), and if person edits config file to change boot device to xvda, they would then edit the config to add -x xen_emul_unplug=[all|ide-disks|...] as well. overall, i like the direction of the change to the unplug patch; there''d be minor changes if you adopted the above default, but effectively what''s below would work well on older (rhel5) dom0''s/tools. - Don> --- > > > diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c > index 72a3da6..2f7f3fb 100644 > --- a/arch/x86/xen/platform-pci-unplug.c > +++ b/arch/x86/xen/platform-pci-unplug.c > @@ -29,9 +29,9 @@ > #define XEN_PLATFORM_ERR_PROTOCOL -2 > #define XEN_PLATFORM_ERR_BLACKLIST -3 > > -/* boolean to signal that the platform pci device can be used */ > -bool xen_platform_pci_enabled; > -EXPORT_SYMBOL_GPL(xen_platform_pci_enabled); > +/* store the value of xen_emul_unplug after the unplug is done */ > +int xen_platform_pci_unplug; > +EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); > static int xen_emul_unplug; > > static int __init check_platform_magic(void) > @@ -76,13 +76,13 @@ void __init xen_unplug_emulated_devices(void) > /* If the version matches enable the Xen platform PCI driver. > * Also enable the Xen platform PCI driver if the version is really old > * and the user told us to ignore it. */ > - if (!r || (r == XEN_PLATFORM_ERR_MAGIC && > - (xen_emul_unplug & XEN_UNPLUG_IGNORE))) > - xen_platform_pci_enabled = 1; > + if (r && !(r == XEN_PLATFORM_ERR_MAGIC && > + (xen_emul_unplug & XEN_UNPLUG_IGNORE))) > + return;and it should return ??? ^^^^> /* Set the default value of xen_emul_unplug depending on whether or > * not the Xen PV frontends and the Xen platform PCI driver have > * been compiled for this kernel (modules or built-in are both OK). */ > - if (xen_platform_pci_enabled && !xen_emul_unplug) { > + if (!xen_emul_unplug) { > if (xen_must_unplug_nics()) { > printk(KERN_INFO "Netfront and the Xen platform PCI driver have " > "been compiled for this kernel: unplug emulated NICs.\n"); > @@ -98,8 +98,9 @@ void __init xen_unplug_emulated_devices(void) > } > } > /* Now unplug the emulated devices */ > - if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE)) > + if (!(xen_emul_unplug & XEN_UNPLUG_IGNORE)) > outw(xen_emul_unplug, XEN_IOPORT_UNPLUG); > + xen_platform_pci_unplug = xen_emul_unplug; > } > > static int __init parse_xen_emul_unplug(char *arg) > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index 82ed403..6eb2989 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -48,6 +48,7 @@ > #include <xen/grant_table.h> > #include <xen/events.h> > #include <xen/page.h> > +#include <xen/platform_pci.h> > > #include <xen/interface/grant_table.h> > #include <xen/interface/io/blkif.h> > @@ -737,6 +738,22 @@ static int blkfront_probe(struct xenbus_device *dev, > } > } > > + /* no unplug has been done: do not hook devices != xen vbds */ > + if (xen_hvm_domain() && (xen_platform_pci_unplug & XEN_UNPLUG_IGNORE)) { > + int major; > + > + if (!VDEV_IS_EXTENDED(vdevice)) > + major = BLKIF_MAJOR(vdevice); > + else > + major = XENVBD_MAJOR; > + > + if (major != XENVBD_MAJOR) { > + printk(KERN_INFO > + "%s: HVM does not support vbd %d as xen block device\n", > + __FUNCTION__, vdevice); > + return -ENODEV; > + } > + }Close to what I have for rhel5 (except I specifically scanned for ide & scsi blk major); I like the above better -- xen-blkfront only supports.... xvd''s ! :)> info = kzalloc(sizeof(*info), GFP_KERNEL); > if (!info) { > xenbus_dev_fatal(dev, -ENOMEM, "allocating info structure"); > diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c > index be8b4f3..c01b5dd 100644 > --- a/drivers/xen/platform-pci.c > +++ b/drivers/xen/platform-pci.c > @@ -196,7 +196,9 @@ static struct pci_driver platform_driver = { > > static int __init platform_pci_module_init(void) > { > - if (!xen_platform_pci_enabled) > + /* no unplug has been done, IGNORE hasn''t been specified: just > + * return now */ > + if (!xen_platform_pci_unplug) > return -ENODEV; > > return pci_register_driver(&platform_driver); > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > index 243279a..34287da 100644 > --- a/drivers/xen/xenbus/xenbus_probe.c > +++ b/drivers/xen/xenbus/xenbus_probe.c > @@ -978,7 +978,7 @@ static void wait_for_devices(struct xenbus_driver *xendrv) > #ifndef MODULE > static int __init boot_wait_for_devices(void) > { > - if (xen_hvm_domain() && !xen_platform_pci_enabled) > + if (xen_hvm_domain() && !xen_platform_pci_unplug) > return -ENODEV; > > ready_to_wait_for_devices = 1; > diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h > index afa8855..ce9d671 100644 > --- a/include/xen/platform_pci.h > +++ b/include/xen/platform_pci.h > @@ -44,6 +44,6 @@ static inline int xen_must_unplug_disks(void) { > #endif > } > > -extern bool xen_platform_pci_enabled; > +extern int xen_platform_pci_unplug; > > #endif /* _XEN_PLATFORM_PCI_H */ > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-08 21:29 UTC
Re: [Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
On Thu, 2010-07-08 at 20:57 +0100, Don Dutile wrote:> I guess what I''m wondering is why not set xen_emul_unplug to ignore by > default (static int xen_emul_unplug=XEN_UNPLUG_IGNORE), which handles > the case I mentioned (just take existing guest config file as is, no edits, > pre-pv-hvm added to guest kernel), and if person edits config file to > change boot device to xvda, they would then edit the config to add > -x xen_emul_unplug=[all|ide-disks|...] as well.Can you guarantee that nobody is running an HVM guest today with a configuration file that specifies xvda (I believe it would work)? In other words can you be sure that defaulting to XEN_UNPLUG_IGNORE is _always_ going to be safe? Not just on RHEL hosts and with configurations generated by the RH tools or according to the RH docs but on any host with any (possibly hand-crafted) configuration? Any guest which uses xvda in its configuration file today will be using emulated devices but I think that with Stefano''s patch and your proposed change in default on a Xen system without support for unplug will start using PV devices without unplugging the emulated ones first. I don''t think there is any way for a guest running on a platform which does not support the unplug protocol to know automatically if it is safe to use the PV devices or not, therefore we have to err on the side of caution and ask users with such systems who know that their configuration is safe to explicitly request PV devices by using the command line option. Doing anything else is taking risks with people''s data. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Don Dutile
2010-Jul-08 21:59 UTC
Re: [Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
Ian Campbell wrote:> On Thu, 2010-07-08 at 20:57 +0100, Don Dutile wrote: >> I guess what I''m wondering is why not set xen_emul_unplug to ignore by >> default (static int xen_emul_unplug=XEN_UNPLUG_IGNORE), which handles >> the case I mentioned (just take existing guest config file as is, no edits, >> pre-pv-hvm added to guest kernel), and if person edits config file to >> change boot device to xvda, they would then edit the config to add >> -x xen_emul_unplug=[all|ide-disks|...] as well. > > Can you guarantee that nobody is running an HVM guest today with a > configuration file that specifies xvda (I believe it would work)? In > other words can you be sure that defaulting to XEN_UNPLUG_IGNORE is > _always_ going to be safe? Not just on RHEL hosts and with > configurations generated by the RH tools or according to the RH docs but > on any host with any (possibly hand-crafted) configuration? >No, you have a valid point. We have pv-on-hvm support for rhel3->rhel5 HVM guests, and they support xvda on boot devices (once initrd is rebuilt), so it''s possible to have that config (spec''d) as well, and someone to copy & edit it for use on a more current disk image w/relatively current kernel. But I''m considering 2.6.32+ HVM guests that didn''t have xvda spec''d in the boot path ever, and are upgraded to a post-2.6.32 kernel that has pv-on-hvm added to it.> Any guest which uses xvda in its configuration file today will be using > emulated devices but I think that with Stefano''s patch and your proposed > change in default on a Xen system without support for unplug will start > using PV devices without unplugging the emulated ones first. >Well, Stefano requires the admin to add unplug switch to kernel cmd line, so I don''t see the harm in defaulting to unplug...> I don''t think there is any way for a guest running on a platform which > does not support the unplug protocol to know automatically if it is safe > to use the PV devices or not, therefore we have to err on the side of > caution and ask users with such systems who know that their > configuration is safe to explicitly request PV devices by using the > command line option. Doing anything else is taking risks with people''s > data. > > Ian. > >Either way, the user/admin has to add cmdline to unplug to be safe. I don''t see how defaulting to UNPLUG_IGNORE changes that requirement. ... or am I missing a case? -- ah, if IGNORE isn''t spec''d, pvhvm just won''t be configured in, and blkfront wont run, and cant have blkfront & ide accessing the same device.... is that the case I''m missing ? - Don _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
john stultz
2010-Jul-09 01:05 UTC
[Xen-devel] Re: [PATCH 10/13] Do not try to disable hpet if it hasn''t been initialized before
On Wed, 2010-06-30 at 13:53 -0400, Konrad Rzeszutek Wilk wrote:> On Mon, Jun 21, 2010 at 05:14:04PM +0100, stefano@stabellini.net wrote: > > From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > hpet_disable is called unconditionally on machine reboot if hpet support > > is compiled in the kernel. > > hpet_disable only checks if the machine is hpet capable but doesn''t make > > sure that hpet has been initialized. > > CC-ing John Stultz who has had his head wrapped around this time-thingy. > > Hi John! > > What are your thoughts about this patch?Sorry for the slow reply. The patch looks good to me. It makes the code easier to read as well! However, I''ve not had my head in the hpet details for awhile, so Venkatesh''s ack (already provided) is the better endorsement. thanks -john> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > arch/x86/kernel/hpet.c | 18 ++++++++++-------- > > 1 files changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c > > index 23b4ecd..2b299da 100644 > > --- a/arch/x86/kernel/hpet.c > > +++ b/arch/x86/kernel/hpet.c > > @@ -959,16 +959,18 @@ fs_initcall(hpet_late_init); > > > > void hpet_disable(void) > > { > > - if (is_hpet_capable()) { > > - unsigned int cfg = hpet_readl(HPET_CFG); > > + unsigned int cfg; > > > > - if (hpet_legacy_int_enabled) { > > - cfg &= ~HPET_CFG_LEGACY; > > - hpet_legacy_int_enabled = 0; > > - } > > - cfg &= ~HPET_CFG_ENABLE; > > - hpet_writel(cfg, HPET_CFG); > > + if (!is_hpet_capable() || !hpet_address || !hpet_virt_address) > > + return; > > + > > + cfg = hpet_readl(HPET_CFG); > > + if (hpet_legacy_int_enabled) { > > + cfg &= ~HPET_CFG_LEGACY; > > + hpet_legacy_int_enabled = 0; > > } > > + cfg &= ~HPET_CFG_ENABLE; > > + hpet_writel(cfg, HPET_CFG); > > } > > > > #ifdef CONFIG_HPET_EMULATE_RTC > > -- > > 1.7.0.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-09 08:02 UTC
Re: [Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
On Thu, 2010-07-08 at 22:59 +0100, Don Dutile wrote:> Ian Campbell wrote: > > On Thu, 2010-07-08 at 20:57 +0100, Don Dutile wrote: > >> I guess what I''m wondering is why not set xen_emul_unplug to ignore by > >> default (static int xen_emul_unplug=XEN_UNPLUG_IGNORE), which handles > >> the case I mentioned (just take existing guest config file as is, no edits, > >> pre-pv-hvm added to guest kernel), and if person edits config file to > >> change boot device to xvda, they would then edit the config to add > >> -x xen_emul_unplug=[all|ide-disks|...] as well. > > > > Can you guarantee that nobody is running an HVM guest today with a > > configuration file that specifies xvda (I believe it would work)? In > > other words can you be sure that defaulting to XEN_UNPLUG_IGNORE is > > _always_ going to be safe? Not just on RHEL hosts and with > > configurations generated by the RH tools or according to the RH docs but > > on any host with any (possibly hand-crafted) configuration? > > > No, you have a valid point. We have pv-on-hvm support for rhel3->rhel5 > HVM guests, and they support xvda on boot devices (once initrd is rebuilt), > so it''s possible to have that config (spec''d) as well, and someone > to copy & edit it for use on a more current disk image w/relatively current > kernel. > But I''m considering 2.6.32+ HVM guests that didn''t have xvda spec''d in > the boot path ever, and are upgraded to a post-2.6.32 kernel that has > pv-on-hvm added to it.An HVM guest could have any kernel version, even one prior to 2.6.32, and be updated to a post-2.6.32 with PV-on-HVM, we cannot restrict ourselves to just 2.6.32+ kernels (not that I think it makes a difference anyhow).> > > Any guest which uses xvda in its configuration file today will be using > > emulated devices but I think that with Stefano''s patch and your proposed > > change in default on a Xen system without support for unplug will start > > using PV devices without unplugging the emulated ones first. > > > Well, Stefano requires the admin to add unplug switch to kernel cmd line,In the case where the host platform does not support the unplug protocol this is correct and requiring explicit admin action to allow the PV frontends to activate is the only safe option WRT the users data. However if the host platform does support the unplug protocol then this is incorrect. In that case the default (in the absence of the command line option) is to automatically unplug any device for which a PV driver is available and so no command line option will be required in the common case. (see xen_unplug_emulated_devices() under the comment "Set the default value of xen_emul_unplug depending on...")> so I don''t see the harm in defaulting to unplug...As I described in my previous mail this is unsafe on host platforms which do not support unplug. As I describe above it is unnecessary on host platforms which do support unplug> Either way, the user/admin has to add cmdline to unplug to be safe.Not true, on a recent Xen system which supports the unplug protocol then devices will be unplugged automatically without an additional command line option. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-09 10:54 UTC
Re: [Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
On Fri, 9 Jul 2010, Ian Campbell wrote:> > > > > Any guest which uses xvda in its configuration file today will be using > > > emulated devices but I think that with Stefano''s patch and your proposed > > > change in default on a Xen system without support for unplug will start > > > using PV devices without unplugging the emulated ones first. > > > > > Well, Stefano requires the admin to add unplug switch to kernel cmd line, > > In the case where the host platform does not support the unplug protocol > this is correct and requiring explicit admin action to allow the PV > frontends to activate is the only safe option WRT the users data. > > However if the host platform does support the unplug protocol then this > is incorrect. In that case the default (in the absence of the command > line option) is to automatically unplug any device for which a PV driver > is available and so no command line option will be required in the > common case. (see xen_unplug_emulated_devices() under the comment "Set > the default value of xen_emul_unplug depending on...") >that''s right: on host platforms supporting unplug no command line options are required; on the other hand if the host platform does not support unplug then xen_emul_unplug=ignore is required to use PV drivers anyway.> > so I don''t see the harm in defaulting to unplug... > > As I described in my previous mail this is unsafe on host platforms > which do not support unplug. As I describe above it is unnecessary on > host platforms which do support unplug >yep, keep in mind the copy and paste example. If you are happy about the patch, I''ll include it in my next version. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Don Dutile
2010-Jul-09 13:42 UTC
Re: [Xen-devel] Re: [PATCH 12/13] Unplug emulated disks and nics
Stefano Stabellini wrote:> On Fri, 9 Jul 2010, Ian Campbell wrote: >>>> Any guest which uses xvda in its configuration file today will be using >>>> emulated devices but I think that with Stefano''s patch and your proposed >>>> change in default on a Xen system without support for unplug will start >>>> using PV devices without unplugging the emulated ones first. >>>> >>> Well, Stefano requires the admin to add unplug switch to kernel cmd line, >> In the case where the host platform does not support the unplug protocol >> this is correct and requiring explicit admin action to allow the PV >> frontends to activate is the only safe option WRT the users data. >> >> However if the host platform does support the unplug protocol then this >> is incorrect. In that case the default (in the absence of the command >> line option) is to automatically unplug any device for which a PV driver >> is available and so no command line option will be required in the >> common case. (see xen_unplug_emulated_devices() under the comment "Set >> the default value of xen_emul_unplug depending on...") >> > > that''s right: on host platforms supporting unplug no command line options > are required; on the other hand if the host platform does not support > unplug then xen_emul_unplug=ignore is required to use PV drivers anyway. > >I see by looking at xen_unplug_emulated_devices() closer, that the default unplugs emulated ide & emulated nic: if (xen_platform_pci_enabled && !xen_emul_unplug) I had to work around the fact that xen_platform_pci_enabled would not be set on rhel5-dom0 (no emul-unplug), so I hadn''t looked at this default so closely.>>> so I don''t see the harm in defaulting to unplug... >> As I described in my previous mail this is unsafe on host platforms >> which do not support unplug. As I describe above it is unnecessary on >> host platforms which do support unplug >> > > yep, keep in mind the copy and paste example. > > > > If you are happy about the patch, I''ll include it in my next version. >Yes, the patch seems like a reasonable solution for running on older qemu-dm''s. Thanks for the clarification(s). - Don _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel