Re: [PATCH 3/6] kvmppc: rewrite guest code - sprg0-3

[Hollis Blanchard <hollisb@xxxxxxxxxx>]

Some initial comments:

On Tue, 2008-08-19 at 12:36 +0200, ehrhardt@xxxxxxxxxxxxxxxxxx wrote:
> +               if (rw == KVM_PPCPV_PVMEM_READ) {
> +                       set_op(32, &newinst); /* lwz */
> +                       set_rt(get_rt(inst), &newinst); /* set original rt */
> +               } else {
> +                       set_op(36, &newinst); /* stw */
> +                       set_rs(get_rs(inst), &newinst); /* set original rs */
> +               }

I think it's time we replaced these magic numbers (32, 36 above, but
also in the opcode switch statements) with named constants.

> +int kvmppc_pvmem_rewrite_instruction(struct kvm_vcpu *vcpu)
> +{
> ...
> +               down_read(&current->mm->mmap_sem);
> +               pcpage = gfn_to_page(vcpu->kvm,
> +                               guest_pcaddr >> KVM_PPCPV_MAGIC_PAGE_SHIFT);
> +               up_read(&current->mm->mmap_sem);
> +               if (pcpage == bad_page)
> +                       return -EFAULT;
> +
> +               pcmem = kmap_atomic(pcpage, KM_USER0);
> +               if (!pcmem)
> +                       return -EFAULT;
> +               pcaddr = ((unsigned long)pcmem
> +                               | (vcpu->arch.pc & KVM_PPCPV_MAGIC_PAGE_MASK));
> +               BUG_ON(inst != *((u32 *)pcaddr));
> +               create_instruction(pcaddr, newinst);
> +               kunmap_atomic(pcpage, KM_USER0);

This suffers from the same kunmap_atomic() problem you pointed out to me
elsewhere in my own code. :) (You're passing pcpage to kunmap_atomic()
instead of pcaddr.)

-- 
Hollis Blanchard
IBM Linux Technology Center

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html