Marcelo Tosatti wrote:

- svm always caches registers, and all registers are dirty, since cache/decache is cheap

Accurate regs_dirty information is useful for converting the emulator,
so that you can do something like:

emul_register_write(ctxt, reg, val)
{
    if (!__test_and_set_bit(reg, &ctxt->vcpu->regs_dirty))
        ctxt->original_regs[reg] = kvm_register_read(ctxt->vcpu, reg);
    ctxt->vcpu->regs[reg] = val;
}

Because restoring the original reg contents on failure is necessary.
Otherwise you need to cache all regs on emulation entry. RIP is always
read anyway, but RSP not so frequently.

Well, might not be worth the complexity for saving just one vmcs_read().
Or it can be changed later during conversion.


That doesn't work, because some of the registers may already be dirty when the emulator is invoked (say, if we're emulating several instructions back-to-back). I think the best way to change the emulator is to let it have its own set of dirty/available bits.

                if (io->in) {
                        r = pio_copy_data(vcpu);
                        if (r) {
-                               kvm_x86_ops->cache_regs(vcpu);
+                               kvm_x86_ops->cache_reg(vcpu, VCPU_REGS_RAX);
                                return r;
                        }

These two don't go well together. Apparently the intent of this
->cache_regs call on failure was to restore the original registers in
case they were modified by pio_copy_data? But pio_copy_data does not
write to any guest register (and even if it did, this ->cache_regs call
assumes what registers are fetched from the guest's originals).

        kvm_x86_ops->cache_regs(vcpu);

        if (!io->string) {
                if (io->in)
                        memcpy(&vcpu->arch.regs[VCPU_REGS_RAX], 
vcpu->arch.pio_data,
                               io->size);
        } else {
                if (io->in) {
                        r = pio_copy_data(vcpu);
                        if (r) {
                                kvm_x86_ops->cache_regs(vcpu);
                                return r;
                        }
                }

Unless I'm mistaken you can just remove it.


Right.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

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

Reply via email to