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. 00000000 (hvc_console) vs. 00000000 (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 <[email protected]>
Signed-off-by: David Vrabel <[email protected]>
Tested-by: Sander Eikelenboom <[email protected]>
---
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 <asm/time.h>
#include <asm/intel-mid.h>
#include <asm/rtc.h>
+#include <asm/i8259.h>
#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
David Vrabel <[email protected]> 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. 00000000 (hvc_console) vs. 00000000 (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 <[email protected]>
> Signed-off-by: David Vrabel <[email protected]>
> Tested-by: Sander Eikelenboom <[email protected]>
> ---
> 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 <asm/time.h>
> #include <asm/intel-mid.h>
> #include <asm/rtc.h>
> +#include <asm/i8259.h>
>
> #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
On 03/12/15 11:23, Vitaly Kuznetsov wrote:
> David Vrabel <[email protected]> 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
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. 00000000 (hvc_console) vs. 00000000 (rtc0)
So don't add the rtc_cmos device in Xen PV guests.
Reported-by: Sander Eikelenboom <[email protected]>
Signed-off-by: David Vrabel <[email protected]>
Tested-by: Sander Eikelenboom <[email protected]>
---
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
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
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
Boris Ostrovsky <[email protected]> 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
On 12/04/2015 10:52 AM, Vitaly Kuznetsov wrote:
> Boris Ostrovsky <[email protected]> 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
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?
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
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