linux-kernel - Re: [RFC: timer passthrough 1/9] KVM: vmx: hook set_next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 05 Feb 2021 19:11:58 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Zhimin Feng <fengzhimin@...edance.com>, x86@...nel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     pbonzini@...hat.com, seanjc@...gle.com, vkuznets@...hat.com,
        wanpengli@...cent.com, jmattson@...gle.com, joro@...tes.org,
        mingo@...hat.com, bp@...en8.de, hpa@...or.com, fweisbec@...il.com,
        zhouyibo@...edance.com, zhanghaozhong@...edance.com,
        Zhimin Feng <fengzhimin@...edance.com>
Subject: Re: [RFC: timer passthrough 1/9] KVM: vmx: hook set_next_event for getting the host tscd

On Fri, Feb 05 2021 at 18:03, Zhimin Feng wrote:
> @@ -520,6 +521,24 @@ struct kvm_vcpu_hv {
>  	cpumask_t tlb_flush;
>  };
>  
> +enum tick_device_mode {
> +	TICKDEV_MODE_PERIODIC,
> +	TICKDEV_MODE_ONESHOT,
> +};
> +
> +struct tick_device {
> +	struct clock_event_device *evtdev;
> +	enum tick_device_mode mode;
> +};

There is a reason why these things are defined in a header file which is
not public. Nothing outside of kernel/time/ has to fiddle with
this. Aside of that how are these things supposed to stay in sync?

> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index 6c9c342dd0e5..bc50f4a1a7c0 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -26,6 +26,7 @@
>   * Tick devices
>   */
>  DEFINE_PER_CPU(struct tick_device, tick_cpu_device);
> +EXPORT_SYMBOL_GPL(tick_cpu_device);

Not going to happen ever.

> +#define TSC_DIVISOR  8
> +static DEFINE_PER_CPU(struct timer_passth_info, passth_info);
> +
> +static int override_lapic_next_event(unsigned long delta,
> +		struct clock_event_device *evt)
> +{
> +	struct timer_passth_info *local_timer_info;
> +	u64 tsc;
> +	u64 tscd;
> +
> +	local_timer_info = &per_cpu(passth_info, smp_processor_id());
> +	tsc = rdtsc();
> +	tscd = tsc + (((u64) delta) * TSC_DIVISOR);
> +	local_timer_info->host_tscd = tscd;
> +	wrmsrl(MSR_IA32_TSCDEADLINE, tscd);
> +	return 0;
> +}
> +
> +static void vmx_host_timer_passth_init(void *junk)
> +{
> +	struct timer_passth_info *local_timer_info;
> +	int cpu = smp_processor_id();
> +
> +	local_timer_info = &per_cpu(passth_info, cpu);
> +	local_timer_info->curr_dev = per_cpu(tick_cpu_device, cpu).evtdev;
> +	local_timer_info->orig_set_next_event =
> +		local_timer_info->curr_dev->set_next_event;
> +	local_timer_info->curr_dev->set_next_event = override_lapic_next_event;

So when loading the KVM module you steal the set_next_event pointer of
the clock event device which is currently active. What guarantees that

    1) The current active device is the tsc deadline timer
    2) The active device does not change

Nothing.

Thanks,

        tglx