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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel