[PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

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 <linux@eikelenboom.it>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Vitaly Kuznetsov]

David Vrabel <david.vrabel@citrix.com> 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 <linux@eikelenboom.it>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
> ---
>  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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Vitaly Kuznetsov]

David Vrabel <david.vrabel@citrix.com> 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 <linux@eikelenboom.it>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
> ---
>  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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

On 03/12/15 11:23, Vitaly Kuznetsov wrote:
> David Vrabel <david.vrabel@citrix.com> 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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

On 03/12/15 11:23, Vitaly Kuznetsov wrote:
> David Vrabel <david.vrabel@citrix.com> 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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

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 <linux@eikelenboom.it>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

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 <linux@eikelenboom.it>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Boris Ostrovsky]

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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Boris Ostrovsky]

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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Vitaly Kuznetsov]

Boris Ostrovsky <boris.ostrovsky@oracle.com> 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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Vitaly Kuznetsov]

Boris Ostrovsky <boris.ostrovsky@oracle.com> 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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Boris Ostrovsky]

On 12/04/2015 10:52 AM, Vitaly Kuznetsov wrote:
> Boris Ostrovsky <boris.ostrovsky@oracle.com> 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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Boris Ostrovsky]

On 12/04/2015 10:52 AM, Vitaly Kuznetsov wrote:
> Boris Ostrovsky <boris.ostrovsky@oracle.com> 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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Thomas Gleixner]

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?

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Thomas Gleixner]

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?

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Boris Ostrovsky]

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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Boris Ostrovsky]

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

Re: [Xen-devel] [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Thomas Gleixner]

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

Re: [PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[Thomas Gleixner]

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

[PATCHv1] x86: rtc_cmos platform device requires legacy irqs

[David Vrabel]

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 <linux@eikelenboom.it>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 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