Hi Jan,
On 21/06/2019 09:58, Jan Beulich wrote:
On 20.06.19 at 19:24, <[email protected]> wrote:
Actually I may have found the error. I feel quite ashamed I didn't spot
this during review and when the bisector fingered it.
staging-4.11 and staging.4.12 didn't have get_cycles implemented (i.e it
returned 0). During the backport, get_cycles() got suddenly implemented
(aside the isb()) so it now returns the number of cycles.
Stefano, how can this be a valid backport under the given title? The
(imo) only correct way of backporting that hunk would have been to
simply drop it, adding isb() instead of the switch to call the function
in the two other places.
To both of you: How certain are you that the subsequent type
change is really all that's needed, and that the sudden change in
behavior of get_cycles() won't have other undue side effects?
Here the current callers:
42sh> ack get_cycles
include/asm-arm/time.h
12:static inline cycles_t get_cycles (void)
include/asm-x86/time.h
29:static inline cycles_t get_cycles(void)
arch/arm/time.c
152: boot_count = get_cycles();
193: uint64_t ticks = get_cycles() - boot_count;
arch/x86/irq.c
898: tsc_in = tb_init_done ? get_cycles() : 0;
900: TRACE_3D(TRC_HW_IRQ_HANDLED, irq, tsc_in, get_cycles());
921: tsc_in = tb_init_done ? get_cycles() : 0;
923: TRACE_3D(TRC_HW_IRQ_HANDLED, irq, tsc_in, get_cycles());
common/tmem.c
275: pgp->timestamp = get_cycles();
389: life = get_cycles() - pgp->timestamp;
common/keyhandler.c
389: per_cpu(read_cycles_time, cpu) = get_cycles();
412: per_cpu(read_cycles_time, cpu) = get_cycles();
common/trace.c
595: u64 tsc = (u64)get_cycles();
778: this_cpu(lost_records_first_tsc)=(u64)get_cycles();
The two callers we care the most are in arch/arm/time.c. They were added with
the patch backported and I know this is fine because the value before was 64-bit
as well. So there are no change except for the isb() here.
I am not sure anyone ever tested/used tmem.c on Arm (and it is dropped on recent
release). So that's not going to make much difference.
common/keyhandler.c, this is mostly for debugging.
common/trace.c does not matter as tracing does not work on Arm before 4.12.
So I am pretty confident that this change will not provoke more side effects
than an isb() directly in the code.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
