linux-mips
[Top] [All Lists]

Re: [PATCH v2 07/18] KVM/MIPS32: MMU/TLB operations for the Guest.

To: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH v2 07/18] KVM/MIPS32: MMU/TLB operations for the Guest.
From: Sanjay Lal <sanjayl@kymasys.com>
Date: Fri, 15 Feb 2013 13:19:29 -0500
Cc: kvm@vger.kernel.org, linux-mips@linux-mips.org
In-reply-to: <20130206120820.GN23213@redhat.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
References: <1353551656-23579-1-git-send-email-sanjayl@kymasys.com> <1353551656-23579-8-git-send-email-sanjayl@kymasys.com> <20130206120820.GN23213@redhat.com>
Sender: linux-mips-bounce@linux-mips.org
On Feb 6, 2013, at 7:08 AM, Gleb Natapov wrote:

>> 
>> +static void kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
>> +{
>> +    pfn_t pfn;
>> +
>> +    if (kvm->arch.guest_pmap[gfn] != KVM_INVALID_PAGE)
>> +            return;
>> +
>> +    pfn =kvm_mips_gfn_to_pfn(kvm, gfn);
> This call should be in srcu read section since it access memory slots which
> are srcu protected. You should test with RCU debug enabled.

kvm_mips_gfn_to_pfn just maps to gfn_to_pfn. I don't see an instance where 
gfn_to_pfn is in a scru read section?

> 
>> 
>> +
>> +uint32_t kvm_get_inst(uint32_t *opc, struct kvm_vcpu *vcpu)
>> +{
>> +    uint32_t inst;
>> +    struct mips_coproc *cop0 __unused = vcpu->arch.cop0;
>> +    int index;
>> +    ulong paddr, flags;
>> +
>> +    if (KVM_GUEST_KSEGX((ulong) opc) < KVM_GUEST_KSEG0 ||
>> +        KVM_GUEST_KSEGX((ulong) opc) == KVM_GUEST_KSEG23) {
>> +            local_irq_save(flags);
>> +            index = kvm_mips_host_tlb_lookup(vcpu, (ulong) opc);
>> +            if (index >= 0) {
>> +                    inst = *(opc);
> Here and in some more places below you access __user memory. Shouldn't you
> use get_user() to access it? What prevents the kernel crash by access fault 
> here
> if userspace remaps the memory to be non-readable? Hmm, may be it uses
> guest translation here so it cannot happen, but still, sparse will not
> be happy and kvm_mips_translate_guest_kseg0_to_hpa() case below uses
> host translation anyway.
> 
Actually, I don't need the __user declaration in most cases, since KVM/MIPS 
handles mapping the page (if needed) and does not rely on the usual kernel 
mechanisms.


<Prev in Thread] Current Thread [Next in Thread>