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
| ||
|
Date: Mon, 30 Jan 2017 14:51:51 +0000 From: Marc Zyngier <marc.zyngier@....com> To: Christoffer Dall <christoffer.dall@...aro.org> Cc: Jintack Lim <jintack@...columbia.edu>, pbonzini@...hat.com, rkrcmar@...hat.com, linux@...linux.org.uk, catalin.marinas@....com, will.deacon@....com, andre.przywara@....com, kvm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu, linux-kernel@...r.kernel.org Subject: Re: [RFC v2 02/10] KVM: arm/arm64: Move cntvoff to each timer context On 30/01/17 14:45, Christoffer Dall wrote: > On Sun, Jan 29, 2017 at 11:54:05AM +0000, Marc Zyngier wrote: >> On Fri, Jan 27 2017 at 01:04:52 AM, Jintack Lim <jintack@...columbia.edu> wrote: >>> Make cntvoff per each timer context. This is helpful to abstract kvm >>> timer functions to work with timer context without considering timer >>> types (e.g. physical timer or virtual timer). >>> >>> This also would pave the way for ever doing adjustments of the cntvoff >>> on a per-CPU basis if that should ever make sense. >>> >>> Signed-off-by: Jintack Lim <jintack@...columbia.edu> >>> --- >>> arch/arm/include/asm/kvm_host.h | 6 +++--- >>> arch/arm64/include/asm/kvm_host.h | 4 ++-- >>> include/kvm/arm_arch_timer.h | 8 +++----- >>> virt/kvm/arm/arch_timer.c | 26 ++++++++++++++++++++------ >>> virt/kvm/arm/hyp/timer-sr.c | 3 +-- >>> 5 files changed, 29 insertions(+), 18 deletions(-) >>> >>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h >>> index d5423ab..f5456a9 100644 >>> --- a/arch/arm/include/asm/kvm_host.h >>> +++ b/arch/arm/include/asm/kvm_host.h >>> @@ -60,9 +60,6 @@ struct kvm_arch { >>> /* The last vcpu id that ran on each physical CPU */ >>> int __percpu *last_vcpu_ran; >>> >>> - /* Timer */ >>> - struct arch_timer_kvm timer; >>> - >>> /* >>> * Anything that is not used directly from assembly code goes >>> * here. >>> @@ -75,6 +72,9 @@ struct kvm_arch { >>> /* Stage-2 page table */ >>> pgd_t *pgd; >>> >>> + /* A lock to synchronize cntvoff among all vtimer context of vcpus */ >>> + spinlock_t cntvoff_lock; >> >> Is there any condition where we need this to be a spinlock? I would have >> thought that a mutex should have been enough, as this should only be >> updated on migration or initialization. Not that it matters much in this >> case, but I wondered if there is something I'm missing. >> > > I would think the critical section is small enough that a spinlock makes > sense, but what I don't think we need is to add the additional lock. > > I think just taking the kvm->lock should be sufficient, which happens to > be a mutex, and while that may be a bit slower to take than the > spinlock, it's not in the critical path so let's just keep things > simple. > > Perhaps this what Marc also meant. That would be the logical conclusion, assuming that we can sleep on this path. Thanks, M. -- Jazz is not dead. It just smells funny...
Powered by blists - more mailing lists