On Tue, Nov 29, 2016 at 4:36 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > On 29/11/16 03:28, Jintack Lim wrote: >> On Mon, Nov 28, 2016 at 1:39 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: >>> On 28/11/16 17:43, Marc Zyngier wrote: >>>> Hi Jintack, >> >> Hi Marc, >> >>>> >>>> On 28/11/16 16:46, Jintack Lim wrote: >>>>> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit. >>>>> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they >>>>> are 11th and 10th bits respectively when E2H is set. Current code is >>>>> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set, which >>>>> may allow guest OS to access physical timer. So, fix it. >>>>> >>>>> Signed-off-by: Jintack Lim <jintack@xxxxxxxxxxxxxxx> >>>>> --- >>>>> arch/arm/include/asm/kvm_timer.h | 33 +++++++++++++++++++ >>>>> arch/arm64/include/asm/kvm_timer.h | 62 ++++++++++++++++++++++++++++++++++++ >>>>> include/clocksource/arm_arch_timer.h | 6 ++-- >>>>> virt/kvm/arm/hyp/timer-sr.c | 8 ++--- >>>>> 4 files changed, 103 insertions(+), 6 deletions(-) >>>>> create mode 100644 arch/arm/include/asm/kvm_timer.h >>>>> create mode 100644 arch/arm64/include/asm/kvm_timer.h >>>>> >>> >>> [...] >>> >>>> We could make it nicer (read "faster") by introducing a >>>> hyp_alternate_select construct that only returns a value instead >>>> of calling a function. I remember writing something like that >>>> at some point, and dropping it... >>> >>> So here's what this could look like (warning, wacky code ahead, >>> though I fixed a stupid bug that was present in the previous patch). >>> The generated code is quite nice (no branch, only an extra mov >>> instruction on the default path)... Of course, completely untested! >> >> This looks much cleaner than my patch. >> While we are at it, is it worth to consider that we just need to set >> those bits once for VHE case, not for every world switch as an >> optimization? > > Ah! That's a much better idea indeed! And we could stop messing with > cntvoff_el2 as well, as it doesn't need to be restored to zero on exit. > Could you try and respin something along those lines? Yes, I can. Thanks, Jintack > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
