linux-kernel - Re: [PATCH v7] KVM: LAPIC: Apply change to TDCR right away to the timer

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] [day] [month] [year] [list]

Message-ID: <CANRm+CxKHRyu1Y27a5ieODE6ocTBca3sRxJ7QrOgEV3oRYC4zg@mail.gmail.com>
Date:   Fri, 6 Oct 2017 22:40:12 +0800
From:   Wanpeng Li <kernellwp@...il.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kvm <kvm@...r.kernel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Wanpeng Li <wanpeng.li@...mail.com>
Subject: Re: [PATCH v7] KVM: LAPIC: Apply change to TDCR right away to the timer

2017-10-06 22:21 GMT+08:00 Paolo Bonzini <pbonzini@...hat.com>:
> On 06/10/2017 16:01, Wanpeng Li wrote:
>> +     if (!apic->lapic_timer.period)
>> +             return;
>> +
>> +     now = ktime_get();
>> +     remaining = ktime_sub(apic->lapic_timer.target_expiration, now);
>> +     if (ktime_to_ns(remaining) < 0)
>> +             remaining = 0;
>> +     delta = mod_64(ktime_to_ns(remaining), apic->lapic_timer.period);
>
> I think this shouldn't be happening.  If it does, I'm not sure the mod
> is the right thing to do, so I'd just use ktime_to_ns(remaining).
>
> So perhaps let's simplify all this to:
>
>         ns_remaining_old = ktime_to_ns(remaining);
>         ns_remaining_new = mul_u64_u32_div(ns_remaining_old,
>                                            apic->divide_count, old_divisor);
>
> because below you're calling nsec_to_cycles but remaining is not expressed
> in nanoseconds.
>
>> +     if (!delta)
>> +             return;
>> +
>> +     delta = delta * apic->divide_count / old_divisor;
>> +
>> +     limit_periodic_timer_frequency(apic);
>
> This should be done before all the "if"s (which should not be there in v8,
> but you should still call it before "now = ktime_get();").

Just sent out v8 to handle all the comments.

Regards,
Wanpeng Li

>
> Paolo
>
>> +     apic->lapic_timer.tscdeadline += nsec_to_cycles(apic->vcpu, delta) -
>> +                                    nsec_to_cycles(apic->vcpu, remaining);
>> +     apic->lapic_timer.target_expiration = ktime_add_ns(now, delta);
>> +}
>