[PATCH] x86: make sure we really have an hpet mapping before using it

[PATCH] x86: make sure we really have an hpet mapping before using it

[Jeremy Fitzhardinge]

When booting in Xen dom0, the hpet isn't really accessible, so make
sure the mapping is non-NULL before use.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/kernel/hpet.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

===================================================================
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -818,9 +818,10 @@
 
 		hpet_address = force_hpet_address;
 		hpet_enable();
-		if (!hpet_virt_address)
-			return -ENODEV;
 	}
+
+	if (!hpet_virt_address)
+		return -ENODEV;
 
 	hpet_reserve_platform_timers(hpet_readl(HPET_ID));
 

Re: [PATCH] x86: make sure we really have an hpet mapping before using it

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> When booting in Xen dom0, the hpet isn't really accessible, so make
> sure the mapping is non-NULL before use.
>
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> ---
> arch/x86/kernel/hpet.c |    5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> ===================================================================
> --- a/arch/x86/kernel/hpet.c
> +++ b/arch/x86/kernel/hpet.c
> @@ -818,9 +818,10 @@
>
> 		hpet_address = force_hpet_address;
> 		hpet_enable();
> -		if (!hpet_virt_address)
> -			return -ENODEV;
> 	}
> +
> +	if (!hpet_virt_address)
> +		return -ENODEV;

hm, how can hpet_virt_address become non-NULL if hpet_address is zero?

	Ingo

Re: [PATCH] x86: make sure we really have an hpet mapping before using it

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>   
>> When booting in Xen dom0, the hpet isn't really accessible, so make
>> sure the mapping is non-NULL before use.
>>
>> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>> ---
>> arch/x86/kernel/hpet.c |    5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> ===================================================================
>> --- a/arch/x86/kernel/hpet.c
>> +++ b/arch/x86/kernel/hpet.c
>> @@ -818,9 +818,10 @@
>>
>> 		hpet_address = force_hpet_address;
>> 		hpet_enable();
>> -		if (!hpet_virt_address)
>> -			return -ENODEV;
>> 	}
>> +
>> +	if (!hpet_virt_address)
>> +		return -ENODEV;
>>     
>
> hm, how can hpet_virt_address become non-NULL if hpet_address is zero?
>   

The mapping is normally created in (paravirt_ops.)time_init -> 
hpet_time_init -> hpet_enable -> hpet_set_mapping.  However, under Xen, 
time_init points to the Xen time init function, and so hpet_time_init is 
never called.

hpet_time_late_init is always called because its an initfunc.  In domU 
it doesn't matter because hpet_address is NULL and so it does nothing.  
In dom0, however, all the ACPI table parsing gets done as normal, so the 
hpet is discovered and its address found.  But it is not available for 
the dom0 kernel's use, and it is never mapped.

    J

Re: [PATCH] x86: make sure we really have an hpet mapping before using it

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Ingo Molnar wrote:
>> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>>
>>   
>>> When booting in Xen dom0, the hpet isn't really accessible, so make
>>> sure the mapping is non-NULL before use.
>>>
>>> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>> ---
>>> arch/x86/kernel/hpet.c |    5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> ===================================================================
>>> --- a/arch/x86/kernel/hpet.c
>>> +++ b/arch/x86/kernel/hpet.c
>>> @@ -818,9 +818,10 @@
>>>
>>> 		hpet_address = force_hpet_address;
>>> 		hpet_enable();
>>> -		if (!hpet_virt_address)
>>> -			return -ENODEV;
>>> 	}
>>> +
>>> +	if (!hpet_virt_address)
>>> +		return -ENODEV;
>>>     
>>
>> hm, how can hpet_virt_address become non-NULL if hpet_address is zero?
>>   
>
> The mapping is normally created in (paravirt_ops.)time_init -> 
> hpet_time_init -> hpet_enable -> hpet_set_mapping.  However, under Xen, 
> time_init points to the Xen time init function, and so hpet_time_init is 
> never called.
>
> hpet_time_late_init is always called because its an initfunc.  In domU 
> it doesn't matter because hpet_address is NULL and so it does nothing.  
> In dom0, however, all the ACPI table parsing gets done as normal, so the 
> hpet is discovered and its address found.  But it is not available for 
> the dom0 kernel's use, and it is never mapped.

ok - applied it to tip/timers/hpet.

	Ingo