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.
