Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On Tue, 8 Dec 2015, Boris Ostrovsky wrote: > On 12/08/2015 04:02 PM, Thomas Gleixner wrote: > > > > --- a/arch/x86/kernel/rtc.c > > > > +++ b/arch/x86/kernel/rtc.c > > > > @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) > > > > } > > > > #endif > > > > + if (xen_pv_domain()) > > > > + return -ENODEV; > > > > + > > > Note there's a missing include that breaks !XEN builds. > > What's the state of this? > > I think we are waiting for x86 maintainers to express their preference. There > were 3 proposals to add in add_rtc_cmos() > > 1. if (!nr_legacy_irqs()) > return -ENODEV; > > 2. #ifdef XEN > if (xen_pv_domain()) > return -ENODEV; > #endif > > 3. if (paravirt_enabled()) > return -ENODEV; Either #2 (but with the #ifdefs removed, include the proper header instead) or #3. Thanks, tglx ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On 12/08/2015 04:02 PM, Thomas Gleixner wrote: On Fri, 4 Dec 2015, David Vrabel wrote: On 04/12/15 14:06, David Vrabel wrote: On 03/12/15 10:43, David Vrabel wrote: Adding the rtc platform device when there are no legacy irqs (no legacy PIC) causes a conflict with other devices that end up using the same irq number. An alternative is to remove the rtc_cmos platform device in Xen PV guests. Any preference on how this regression should be fixed? David 8<-- x86: Xen PV guests don't have the rtc_cmos platform device [...] --- a/arch/x86/kernel/rtc.c +++ b/arch/x86/kernel/rtc.c @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) } #endif + if (xen_pv_domain()) + return -ENODEV; + Note there's a missing include that breaks !XEN builds. What's the state of this? I think we are waiting for x86 maintainers to express their preference. There were 3 proposals to add in add_rtc_cmos() 1. if (!nr_legacy_irqs()) return -ENODEV; 2. #ifdef XEN if (xen_pv_domain()) return -ENODEV; #endif 3. if (paravirt_enabled()) return -ENODEV; -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On Fri, 4 Dec 2015, David Vrabel wrote: > On 04/12/15 14:06, David Vrabel wrote: > > On 03/12/15 10:43, David Vrabel wrote: > >> Adding the rtc platform device when there are no legacy irqs (no > >> legacy PIC) causes a conflict with other devices that end up using the > >> same irq number. > > > > An alternative is to remove the rtc_cmos platform device in Xen PV > > guests. > > > > Any preference on how this regression should be fixed? > > > > David > > > > 8<-- > > x86: Xen PV guests don't have the rtc_cmos platform device > > > [...] > > --- a/arch/x86/kernel/rtc.c > > +++ b/arch/x86/kernel/rtc.c > > @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) > > } > > #endif > > > > + if (xen_pv_domain()) > > + return -ENODEV; > > + > > Note there's a missing include that breaks !XEN builds. What's the state of this? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On 12/04/2015 10:52 AM, Vitaly Kuznetsov wrote: Boris Ostrovsky writes: On 12/04/2015 10:24 AM, David Vrabel wrote: On 04/12/15 14:06, David Vrabel wrote: On 03/12/15 10:43, David Vrabel wrote: Adding the rtc platform device when there are no legacy irqs (no legacy PIC) causes a conflict with other devices that end up using the same irq number. An alternative is to remove the rtc_cmos platform device in Xen PV guests. Any preference on how this regression should be fixed? David 8<-- x86: Xen PV guests don't have the rtc_cmos platform device [...] --- a/arch/x86/kernel/rtc.c +++ b/arch/x86/kernel/rtc.c @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) } #endif +if (xen_pv_domain()) + return -ENODEV; + Note there's a missing include that breaks !XEN builds. We could also use paravirt_enable() here which will probably cover HVMlite case as well. (Until we start turning on and off various HVMlite features). Would it make sense to create a new abstraction, e.g. 'rtc_available' in struct hypervisor_x86? We could do this but since this fine-grained feature enabling is still way off it may be worth waiting until we actually get to this. Besides, it would probably be something like if (paravirt_enabled() && !rtc_available) so for now having just the first term should suffice. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
Boris Ostrovsky writes: > On 12/04/2015 10:24 AM, David Vrabel wrote: >> On 04/12/15 14:06, David Vrabel wrote: >>> On 03/12/15 10:43, David Vrabel wrote: Adding the rtc platform device when there are no legacy irqs (no legacy PIC) causes a conflict with other devices that end up using the same irq number. >>> An alternative is to remove the rtc_cmos platform device in Xen PV >>> guests. >>> >>> Any preference on how this regression should be fixed? >>> >>> David >>> >>> 8<-- >>> x86: Xen PV guests don't have the rtc_cmos platform device >>> >> [...] >>> --- a/arch/x86/kernel/rtc.c >>> +++ b/arch/x86/kernel/rtc.c >>> @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) >>> } >>> #endif >>> + if (xen_pv_domain()) >>> + return -ENODEV; >>> + >> Note there's a missing include that breaks !XEN builds. > > We could also use paravirt_enable() here which will probably cover > HVMlite case as well. (Until we start turning on and off various > HVMlite features). Would it make sense to create a new abstraction, e.g. 'rtc_available' in struct hypervisor_x86? -- Vitaly ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On 12/04/2015 10:24 AM, David Vrabel wrote: On 04/12/15 14:06, David Vrabel wrote: On 03/12/15 10:43, David Vrabel wrote: Adding the rtc platform device when there are no legacy irqs (no legacy PIC) causes a conflict with other devices that end up using the same irq number. An alternative is to remove the rtc_cmos platform device in Xen PV guests. Any preference on how this regression should be fixed? David 8<-- x86: Xen PV guests don't have the rtc_cmos platform device [...] --- a/arch/x86/kernel/rtc.c +++ b/arch/x86/kernel/rtc.c @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) } #endif + if (xen_pv_domain()) + return -ENODEV; + Note there's a missing include that breaks !XEN builds. We could also use paravirt_enable() here which will probably cover HVMlite case as well. (Until we start turning on and off various HVMlite features). Don't know how it will affect lguest (which so far is the only other paravirt-enabled guest). -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On 04/12/15 14:06, David Vrabel wrote: > On 03/12/15 10:43, David Vrabel wrote: >> Adding the rtc platform device when there are no legacy irqs (no >> legacy PIC) causes a conflict with other devices that end up using the >> same irq number. > > An alternative is to remove the rtc_cmos platform device in Xen PV > guests. > > Any preference on how this regression should be fixed? > > David > > 8<-- > x86: Xen PV guests don't have the rtc_cmos platform device > [...] > --- a/arch/x86/kernel/rtc.c > +++ b/arch/x86/kernel/rtc.c > @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) > } > #endif > > + if (xen_pv_domain()) > + return -ENODEV; > + Note there's a missing include that breaks !XEN builds. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On 03/12/15 10:43, David Vrabel wrote: > Adding the rtc platform device when there are no legacy irqs (no > legacy PIC) causes a conflict with other devices that end up using the > same irq number. An alternative is to remove the rtc_cmos platform device in Xen PV guests. Any preference on how this regression should be fixed? David 8<-- x86: Xen PV guests don't have the rtc_cmos platform device Adding the rtc platform device in a Xen PV guests causes an IRQ conflict because these guests do not have a legacy PIC. In a single VCPU Xen PV guest we should have: /proc/interrupts: CPU0 0: 4934 xen-percpu-virq timer0 1: 0 xen-percpu-ipi spinlock0 2: 0 xen-percpu-ipi resched0 3: 0 xen-percpu-ipi callfunc0 4: 0 xen-percpu-virq debug0 5: 0 xen-percpu-ipi callfuncsingle0 6: 0 xen-percpu-ipi irqwork0 7:321 xen-dyn-event xenbus 8: 90 xen-dyn-event hvc_console ... But hvc_console cannot get its interrupt because it is already in use by rtc0 and the console does not work. genirq: Flags mismatch irq 8. (hvc_console) vs. (rtc0) So don't add the rtc_cmos device in Xen PV guests. Reported-by: Sander Eikelenboom Signed-off-by: David Vrabel Tested-by: Sander Eikelenboom --- arch/x86/kernel/rtc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index cd96852..7b190b8 100644 --- a/arch/x86/kernel/rtc.c +++ b/arch/x86/kernel/rtc.c @@ -200,6 +200,9 @@ static __init int add_rtc_cmos(void) } #endif + if (xen_pv_domain()) + return -ENODEV; + platform_device_register(&rtc_device); dev_info(&rtc_device.dev, "registered platform RTC device (no PNP device found)\n"); -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
On 03/12/15 11:23, Vitaly Kuznetsov wrote: > David Vrabel writes: > >> Adding the rtc platform device when there are no legacy irqs (no >> legacy PIC) > > No PIC != No legacy IRQs, Hyper-V Gen2 represents such a platform (and > it has RTC on irq8). I've tested this patch against it and it appears to > work because the device is present in ACPI and we initialize it in > drivers/acpi/acpi_cmos_rtc.c, add_rtc_cmos() bails out in the very > beginning as we see PNP0b00 device. It's not a legacy IRQ if it isn't going via the legacy PIC, it just happens to have the same number. I think it is safe to assume that any machine with a CMOS RTC but no PNP information is also going to have a legacy PIC. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
David Vrabel writes: > Adding the rtc platform device when there are no legacy irqs (no > legacy PIC) No PIC != No legacy IRQs, Hyper-V Gen2 represents such a platform (and it has RTC on irq8). I've tested this patch against it and it appears to work because the device is present in ACPI and we initialize it in drivers/acpi/acpi_cmos_rtc.c, add_rtc_cmos() bails out in the very beginning as we see PNP0b00 device. > causes a conflict with other devices that end up using the > same irq number. > > In a single VCPU Xen PV guest we should have: > > /proc/interrupts: >CPU0 > 0: 4934 xen-percpu-virq timer0 > 1: 0 xen-percpu-ipi spinlock0 > 2: 0 xen-percpu-ipi resched0 > 3: 0 xen-percpu-ipi callfunc0 > 4: 0 xen-percpu-virq debug0 > 5: 0 xen-percpu-ipi callfuncsingle0 > 6: 0 xen-percpu-ipi irqwork0 > 7:321 xen-dyn-event xenbus > 8: 90 xen-dyn-event hvc_console > ... > > But hvc_console cannot get its interrupt because it is already in use > by rtc0 and the console does not work. > > genirq: Flags mismatch irq 8. (hvc_console) vs. (rtc0) > > The rtc_cmos device requires a particular legacy irq so don't add it > if there are no legacy irqs. > > Reported-by: Sander Eikelenboom > Signed-off-by: David Vrabel > Tested-by: Sander Eikelenboom > --- > arch/x86/kernel/rtc.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c > index cd96852..07c70f1 100644 > --- a/arch/x86/kernel/rtc.c > +++ b/arch/x86/kernel/rtc.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > #ifdef CONFIG_X86_32 > /* > @@ -200,6 +201,10 @@ static __init int add_rtc_cmos(void) > } > #endif > > + /* RTC uses legacy IRQs. */ > + if (!nr_legacy_irqs()) > + return -ENODEV; > + > platform_device_register(&rtc_device); > dev_info(&rtc_device.dev, >"registered platform RTC device (no PNP device found)\n"); -- Vitaly ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs
Adding the rtc platform device when there are no legacy irqs (no legacy PIC) causes a conflict with other devices that end up using the same irq number. In a single VCPU Xen PV guest we should have: /proc/interrupts: CPU0 0: 4934 xen-percpu-virq timer0 1: 0 xen-percpu-ipi spinlock0 2: 0 xen-percpu-ipi resched0 3: 0 xen-percpu-ipi callfunc0 4: 0 xen-percpu-virq debug0 5: 0 xen-percpu-ipi callfuncsingle0 6: 0 xen-percpu-ipi irqwork0 7:321 xen-dyn-event xenbus 8: 90 xen-dyn-event hvc_console ... But hvc_console cannot get its interrupt because it is already in use by rtc0 and the console does not work. genirq: Flags mismatch irq 8. (hvc_console) vs. (rtc0) The rtc_cmos device requires a particular legacy irq so don't add it if there are no legacy irqs. Reported-by: Sander Eikelenboom Signed-off-by: David Vrabel Tested-by: Sander Eikelenboom --- arch/x86/kernel/rtc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index cd96852..07c70f1 100644 --- a/arch/x86/kernel/rtc.c +++ b/arch/x86/kernel/rtc.c @@ -14,6 +14,7 @@ #include #include #include +#include #ifdef CONFIG_X86_32 /* @@ -200,6 +201,10 @@ static __init int add_rtc_cmos(void) } #endif + /* RTC uses legacy IRQs. */ + if (!nr_legacy_irqs()) + return -ENODEV; + platform_device_register(&rtc_device); dev_info(&rtc_device.dev, "registered platform RTC device (no PNP device found)\n"); -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
