[PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Boris Ostrovsky]

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. (In fact, don't add
it to any paravirt_enabled() guest since lguest is also unable to probe
this device)

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
Cc: stable@vger.kernel.org
---
 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..dd4d6d9 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 (paravirt_enabled())
+		return -ENODEV;
+
 	platform_device_register(&rtc_device);
 	dev_info(&rtc_device.dev,
 		 "registered platform RTC device (no PNP device found)\n");
-- 
1.8.1.4

Re: [Xen-devel] [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Jan Beulich]

>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
> +		return -ENODEV;

What about Xen Dom0?

Jan

Re: [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Sander Eikelenboom]

On 2015-12-09 15:42, Jan Beulich wrote:
>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>> +		return -ENODEV;
> 
> What about Xen Dom0?
> 
> Jan

Checked that in my testing and that still worked:
[   16.733837] rtc_cmos 00:02: RTC can wake from S4
[   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
[   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes 
nvram
[   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09 
08:43:48 UTC (1449650628)

and /dev/rtc and /dev/rtc0 both exist.

But i don't know the nitty gritty details about why ...
--
Sander

Re: [Xen-devel] [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Sander Eikelenboom]

On 2015-12-09 15:42, Jan Beulich wrote:
>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>> +		return -ENODEV;
> 
> What about Xen Dom0?
> 
> Jan

Checked that in my testing and that still worked:
[   16.733837] rtc_cmos 00:02: RTC can wake from S4
[   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
[   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes 
nvram
[   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09 
08:43:48 UTC (1449650628)

and /dev/rtc and /dev/rtc0 both exist.

But i don't know the nitty gritty details about why ...
--
Sander

Re: [Xen-devel] [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Boris Ostrovsky]

On 12/09/2015 10:00 AM, Sander Eikelenboom wrote:
> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>> +        return -ENODEV;
>>
>> What about Xen Dom0?
>>
>> Jan
>
> Checked that in my testing and that still worked:
> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes 
> nvram
> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09 
> 08:43:48 UTC (1449650628)
>
> and /dev/rtc and /dev/rtc0 both exist.
>
> But i don't know the nitty gritty details about why ...


That's because it is discovered by ACPI earlier. I don't know though 
whether we can always assume this will be the case.

-boris

Re: [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Jan Beulich]

>>> On 09.12.15 at 16:15, <boris.ostrovsky@oracle.com> wrote:
> On 12/09/2015 10:00 AM, Sander Eikelenboom wrote:
>> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>>> +        return -ENODEV;
>>>
>>> What about Xen Dom0?
>>>
>>> Jan
>>
>> Checked that in my testing and that still worked:
>> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
>> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
>> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes 
>> nvram
>> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09 
>> 08:43:48 UTC (1449650628)
>>
>> and /dev/rtc and /dev/rtc0 both exist.
>>
>> But i don't know the nitty gritty details about why ...
> 
> 
> That's because it is discovered by ACPI earlier. I don't know though 
> whether we can always assume this will be the case.

I don't think we should - Dom0 should (device-wise) behave just
like a native kernel.

Jan

Re: [Xen-devel] [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Jan Beulich]

>>> On 09.12.15 at 16:15, <boris.ostrovsky@oracle.com> wrote:
> On 12/09/2015 10:00 AM, Sander Eikelenboom wrote:
>> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>>> +        return -ENODEV;
>>>
>>> What about Xen Dom0?
>>>
>>> Jan
>>
>> Checked that in my testing and that still worked:
>> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
>> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
>> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes 
>> nvram
>> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09 
>> 08:43:48 UTC (1449650628)
>>
>> and /dev/rtc and /dev/rtc0 both exist.
>>
>> But i don't know the nitty gritty details about why ...
> 
> 
> That's because it is discovered by ACPI earlier. I don't know though 
> whether we can always assume this will be the case.

I don't think we should - Dom0 should (device-wise) behave just
like a native kernel.

Jan

Re: [Xen-devel] [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Boris Ostrovsky]

On 12/09/2015 10:27 AM, Jan Beulich wrote:
>>>> On 09.12.15 at 16:15, <boris.ostrovsky@oracle.com> wrote:
>> On 12/09/2015 10:00 AM, Sander Eikelenboom wrote:
>>> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>>>> +        return -ENODEV;
>>>> What about Xen Dom0?
>>>>
>>>> Jan
>>> Checked that in my testing and that still worked:
>>> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
>>> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
>>> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes
>>> nvram
>>> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09
>>> 08:43:48 UTC (1449650628)
>>>
>>> and /dev/rtc and /dev/rtc0 both exist.
>>>
>>> But i don't know the nitty gritty details about why ...
>>
>> That's because it is discovered by ACPI earlier. I don't know though
>> whether we can always assume this will be the case.
> I don't think we should - Dom0 should (device-wise) behave just
> like a native kernel.

So maybe then this is the case for having a feature flag (probably in 
pv_info) that marks which features are paravirtualized.

Vitaly suggested it earlier but I thought we won't have use for it until 
we get to HVMlite with variable set of fetures.

-boris

Re: [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Boris Ostrovsky]

On 12/09/2015 10:27 AM, Jan Beulich wrote:
>>>> On 09.12.15 at 16:15, <boris.ostrovsky@oracle.com> wrote:
>> On 12/09/2015 10:00 AM, Sander Eikelenboom wrote:
>>> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>>>> +        return -ENODEV;
>>>> What about Xen Dom0?
>>>>
>>>> Jan
>>> Checked that in my testing and that still worked:
>>> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
>>> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
>>> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes
>>> nvram
>>> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09
>>> 08:43:48 UTC (1449650628)
>>>
>>> and /dev/rtc and /dev/rtc0 both exist.
>>>
>>> But i don't know the nitty gritty details about why ...
>>
>> That's because it is discovered by ACPI earlier. I don't know though
>> whether we can always assume this will be the case.
> I don't think we should - Dom0 should (device-wise) behave just
> like a native kernel.

So maybe then this is the case for having a feature flag (probably in 
pv_info) that marks which features are paravirtualized.

Vitaly suggested it earlier but I thought we won't have use for it until 
we get to HVMlite with variable set of fetures.

-boris

Re: [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Boris Ostrovsky]

On 12/09/2015 10:00 AM, Sander Eikelenboom wrote:
> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>> +        return -ENODEV;
>>
>> What about Xen Dom0?
>>
>> Jan
>
> Checked that in my testing and that still worked:
> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes 
> nvram
> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09 
> 08:43:48 UTC (1449650628)
>
> and /dev/rtc and /dev/rtc0 both exist.
>
> But i don't know the nitty gritty details about why ...


That's because it is discovered by ACPI earlier. I don't know though 
whether we can always assume this will be the case.

-boris

Re: [Xen-devel] [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Vitaly Kuznetsov]

Sander Eikelenboom <linux@eikelenboom.it> writes:

> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>> +		return -ENODEV;
>>
>> What about Xen Dom0?
>>
>> Jan
>
> Checked that in my testing and that still worked:
> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes
> nvram
> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09
> 08:43:48 UTC (1449650628)
>
> and /dev/rtc and /dev/rtc0 both exist.
>
> But i don't know the nitty gritty details about why ...

As far as I can see in Dom0 the device is present in ACPI as PNP0B00 so
a different path is probably in use.

-- 
  Vitaly

Re: [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Vitaly Kuznetsov]

Sander Eikelenboom <linux@eikelenboom.it> writes:

> On 2015-12-09 15:42, Jan Beulich wrote:
>>>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
>>> +		return -ENODEV;
>>
>> What about Xen Dom0?
>>
>> Jan
>
> Checked that in my testing and that still worked:
> [   16.733837] rtc_cmos 00:02: RTC can wake from S4
> [   16.734030] rtc_cmos 00:02: rtc core: registered rtc_cmos as rtc0
> [   16.734087] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes
> nvram
> [   17.760329] rtc_cmos 00:02: setting system clock to 2015-12-09
> 08:43:48 UTC (1449650628)
>
> and /dev/rtc and /dev/rtc0 both exist.
>
> But i don't know the nitty gritty details about why ...

As far as I can see in Dom0 the device is present in ACPI as PNP0B00 so
a different path is probably in use.

-- 
  Vitaly

Re: [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Jan Beulich]

>>> On 09.12.15 at 15:32, <boris.ostrovsky@oracle.com> 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 (paravirt_enabled())
> +		return -ENODEV;

What about Xen Dom0?

Jan

Re: [PATCH] x86: Xen PV guests don't have the rtc_cmos platform device

[Konrad Rzeszutek Wilk]

On Wed, Dec 09, 2015 at 09:32:49AM -0500, Boris Ostrovsky wrote:
> 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. (In fact, don't add
> it to any paravirt_enabled() guest since lguest is also unable to probe
> this device)
> 
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
> Cc: stable@vger.kernel.org

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

You may want to also add a note saying:

[stable-tree maintainers. Please double-check that you also have
29fa6825463c97e5157284db80107d1bfac5d77b x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
which sets pv_info.paravirt_enabled to zero.]

> ---
>  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..dd4d6d9 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 (paravirt_enabled())
> +		return -ENODEV;
> +
>  	platform_device_register(&rtc_device);
>  	dev_info(&rtc_device.dev,
>  		 "registered platform RTC device (no PNP device found)\n");
> -- 
> 1.8.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel