Re: [Xen-devel] arm/vtimer: Physical timer emulation and the physical counter

[Jeff Kubascik <jeff.kubascik@xxxxxxxxxxxxxxx>]

On 11/21/2019 10:06 AM, Julien Grall wrote:> Hi,
> 
> On 21/11/2019 14:31, Jeff Kubascik wrote:
>> On 11/19/2019 7:48 AM, Julien Grall wrote:
>>> Hi,
>>>
>>> On 17/11/2019 22:32, Stewart Hildebrand wrote:
>>>> CC'ing Julien's new email address
>>>
>>> For Xen-devel, I have filter to get in my inbox all e-mails where my
>>> @arm.com is CCed :).
>>>
>>>>
>>>> On Thursday, November 14, 2019 2:33 PM, Jeff Kubascik wrote:
>>>>> Hello,
>>>>>
>>>>> I'm working on a port of a RTOS (RTEMS) to Xen on ARM, and came across an
>>>>> interesting finding in how Xen emulates the physical timer on ARM.
>>>>>
>>>>> In testing different configurations of the port, I have the RTOS 
>>>>> configured to
>>>>> use the ARM generic physical timer. The driver operates the physical 
>>>>> timer in
>>>>> the "CompareView" mode, where the timer condition is met when the physical
>>>>> counter reaches the programmed CompareValue.
>>>>>
>>>>> The driver initializes the physical timer by first reading the physical 
>>>>> counter
>>>>> register CNTPCT, adding the systick interval, and then writing the result 
>>>>> to the
>>>>> CompareValue register CNTP_CVAL. This appears to be valid behavior based 
>>>>> on my
>>>>> understanding of the ARMV8 Architecture Reference Manual, since the 
>>>>> physical
>>>>> timer "offset" is specified to be zero.
>>>>>
>>>>> Xen will trap accesses to the physical timer registers - CNTP_CTL, 
>>>>> CNTP_CVAL,
>>>>> and CNTP_TVAL, which happens in xen/arch/arm/vtimer.c. Xen will add or 
>>>>> remove an
>>>>> offset phys_timer_base.offset when reading or writing to the 
>>>>> CNTP_CVAL/CNTP_TVAL
>>>>> registers. This offset is determined when the vtimer is initialized on 
>>>>> guest
>>>>> creation.
>>>>>
>>>>> However, Xen does not trap access to the physical counter register 
>>>>> CNTPCT. This
>>>>> means the guest has direct access to the register. It also means the 
>>>>> offset is
>>>>> not applied here. I believe this is a problem, because the physical timer 
>>>>> is no
>>>>> longer consistent with the physical counter from the guest's perspective 
>>>>> - there
>>>>> is a non-zero, unknown offset between the two.
>>>>>
>>>>> This was a problem for the RTOS, since it was reading the physical counter
>>>>> register (Xen does not apply an offset), adding some interval, and then 
>>>>> setting
>>>>> the CompareValue register (Xen applies the offset), resulting in a long 
>>>>> delay
>>>>> before the timer expires.
>>>
>>> The description makes sense.
>>>
>>>>>
>>>>> I was able to fix this by adding code in Xen to trap access to CNTPCT and
>>>>> applying the offset - I can submit the patch if there is interest. 
>>>>> However, I
>>>>> was curious if there was an reason for not trapping/ emulating access to 
>>>>> the
>>>>> physical counter register and applying the offset?
>>>
>>> This is definitely a bug in the emulation. But I am not entirely sure we
>>> actually want to trap the physical counter register as this has a cost.
>>>
>>> The only reasons to trap physical timer registers is to ensure the
>>> counter starts at 0 for the guest. I am not entirely convinced this is
>>> useful as we don' t yet support migration. Even with migration, we may
>>> want to not trap the registers until the guest has been migrated to save
>>> cost.
>>
>> Would there be any security concerns in allowing the guest to know the actual
>> run time of the system? Does this leak information in some way? Trying to 
>> think
>> of possible reasons for this design.
> The virtual timer counter is just physical timer counter minus an
> offset. So this is not really to give you more information here.
> 
>>
>>> But, the timer code looks a bit fishy, the hypervisor should use the
>>> hypervisor timer but we seem to have code to handle the interrupt for
>>> the physical timer (see arch/arm/time.c) but not configure it. Looking
>>> at the log, this seems to be a left-over from early Xen that was not
>>> removed by 6c76cb8cb5 "xen/arm: Some clean up in time.c".
>>
>> Make sense.
>>
>>> So I think we can handle the physical timer in similar fashion to the
>>> virtual timer. This should likely improve performance for OS still using
>>> the physcial timer (AFAIK linux arm64 will use the virt timer by default).
>>
>> One caveat is the virtual timer currently masks itself when it fires. This
>> requires the guest to unmask it every time in the interrupt handler, which 
>> is a
>> deviation from normal ARMv8 behavior. Every RTOS port to Xen I have worked 
>> with
>> requires this modification for the system tick to work correctly. We may 
>> need to
>> copy this behavior for the physical timer as well.
> 
> That's a good point. This should be solved by Stewart's series (I
> haven't yet reviewed it).

That would be great if we could fix this deviation in Xen - it would be one less
problem to deal with when porting guests.

> While waiting on the series, we should still fix the problem. But I
> would like to avoid trapping the physical timer counter register. So I
> would suggest to adapt the emulation of the CVAL & co.

I would propose removing phys_timer_base.offset and just make the offset zero
for the physical timer traps. I can put together a patch for this.

>>
>>> @Jeff, would you mind to have a look at it?
>>
>> I can spend some digging into this.
> 
> Thank you!
> 
> Cheers,
> 
> --
> Julien Grall
> 

-Jeff K

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel