From patchwork Wed Jan 16 15:42:02 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 1989411 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 0692C3FC85 for ; Wed, 16 Jan 2013 15:42:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753963Ab3APPmF (ORCPT ); Wed, 16 Jan 2013 10:42:05 -0500 Received: from mail-ie0-f171.google.com ([209.85.223.171]:39531 "EHLO mail-ie0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753420Ab3APPmD (ORCPT ); Wed, 16 Jan 2013 10:42:03 -0500 Received: by mail-ie0-f171.google.com with SMTP id 17so2748185iea.30 for ; Wed, 16 Jan 2013 07:42:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:x-originating-ip:in-reply-to:references :date:message-id:subject:from:to:cc:content-type:x-gm-message-state; bh=6gdr91Pjl6/A4LFOvLytj5VZzP2B2CyoACMwkGY41lc=; b=eDZadsQJ8UdTsa0Lvnb0fb7oCWcEVhZWajs/c6si/Pvq32UHKIYnQ1KFjzFrxc+mfl uxT1nAB4dsNQsRyZZRxBg7NsnJI2rzyl3x45m/xK69QcfSXHmE1jka/r/vaU0FeMKL2G zcxyiDId5SbVmcR9nYv2/u2ricBt2cNSHl8gQr+58JrSBj2CEUAJEplA79B5o+lRvcHu 1BRnHfRau4vL5thsdsV3RpugNRIPV0/gn4l44ArI+yrbGAYJEvZyBIoA5xtXW1JxAB0D rtmFvc86yjCzQiepB3t3pbMew4Mixqe2+ZUfICAxxYqQwaU6RKW8tSPImErSJCkKUKUq 70Ug== MIME-Version: 1.0 X-Received: by 10.50.161.232 with SMTP id xv8mr1081627igb.22.1358350922560; Wed, 16 Jan 2013 07:42:02 -0800 (PST) Received: by 10.64.37.70 with HTTP; Wed, 16 Jan 2013 07:42:02 -0800 (PST) X-Originating-IP: [72.80.83.148] In-Reply-To: <20130116121238.GS11529@redhat.com> References: <20130108183811.46302.58543.stgit@ubuntu> <20130108183924.46302.65998.stgit@ubuntu> <20130115094312.GI11529@redhat.com> <20130116121238.GS11529@redhat.com> Date: Wed, 16 Jan 2013 10:42:02 -0500 Message-ID: Subject: Re: [PATCH v5 07/14] KVM: ARM: World-switch implementation From: Christoffer Dall To: Gleb Natapov Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, Marc Zyngier , Antonios Motakis , Marcelo Tosatti , Rusty Russell , nicolas@viennot.biz X-Gm-Message-State: ALoCoQlCC+vM9J3EZclSd9dV4t6VfcFDE2fMVczcG2Oxj1QvRsb15RvDvQPdzqPofx5tVHMd04hP Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org [...] > >> read side RCU protects against is the memslots data structure as far >> as I can see, so the second patch pasted below fixes this for the code >> that actually accesses this data structure. > Many memory related functions that you call access memslots under the > hood and assume that locking is done by the caller. From the quick look > I found those that you've missed: > kvm_is_visible_gfn() > kvm_read_guest() > gfn_to_hva() > gfn_to_pfn_prot() > kvm_memslots() > > May be there are more. Can you enable RCU debugging in your kernel config > and check? This does not guaranty that it will catch all of the places, > but better than nothing. > yeah, I missed the call to is_visible_gfn and friends, this fixes it: is_iabt = (hsr_ec == HSR_EC_IABT); @@ -608,33 +608,43 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run) return -EFAULT; } + idx = srcu_read_lock(&vcpu->kvm->srcu); + gfn = fault_ipa >> PAGE_SHIFT; if (!kvm_is_visible_gfn(vcpu->kvm, gfn)) { if (is_iabt) { /* Prefetch Abort on I/O address */ kvm_inject_pabt(vcpu, vcpu->arch.hxfar); - return 1; + ret = 1; + goto out_unlock; } if (fault_status != FSC_FAULT) { kvm_err("Unsupported fault status on io memory: %#lx\n", fault_status); - return -EFAULT; + ret = -EFAULT; + goto out_unlock; } /* Adjust page offset */ fault_ipa |= vcpu->arch.hxfar & ~PAGE_MASK; - return io_mem_abort(vcpu, run, fault_ipa); + ret = io_mem_abort(vcpu, run, fault_ipa); + goto out_unlock; } memslot = gfn_to_memslot(vcpu->kvm, gfn); if (!memslot->user_alloc) { kvm_err("non user-alloc memslots not supported\n"); - return -EINVAL; + ret = -EINVAL; + goto out_unlock; } ret = user_mem_abort(vcpu, fault_ipa, gfn, memslot, fault_status); - return ret ? ret : 1; + if (ret == 0) + ret = 1; +out_unlock: + srcu_read_unlock(&vcpu->kvm->srcu, idx); + return ret; } static void handle_hva_to_gpa(struct kvm *kvm, --- Thanks, -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index c806080..f30e131 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -591,7 +591,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run) struct kvm_memory_slot *memslot; bool is_iabt; gfn_t gfn; - int ret; + int ret, idx; hsr_ec = vcpu->arch.hsr >> HSR_EC_SHIFT;