On 01/07/2011 03:03 AM, Jan Kiszka wrote:
Am 06.01.2011 20:24, Anthony Liguori wrote:
On 01/06/2011 11:56 AM, Marcelo Tosatti wrote:
From: Jan Kiszka<jan.kiszka@xxxxxxxxxxx>
QEMU supports only one VM, so there is only one kvm_state per process,
and we gain nothing passing a reference to it around. Eliminate any need
to refer to it outside of kvm-all.c.
Signed-off-by: Jan Kiszka<jan.kiszka@xxxxxxxxxxx>
CC: Alexander Graf<agraf@xxxxxxx>
Signed-off-by: Marcelo Tosatti<mtosatti@xxxxxxxxxx>
I think this is a big mistake.
Obviously, I don't share your concerns. :)
Having to manage kvm_state keeps the abstraction lines well defined.
How does it help?
Otherwise, it's far too easy for portions of code to call into KVM
functions that really shouldn't.
I can't imagine we gain anything from requiring kvm_check_extension
callers to hold a kvm_state "capability". Yes, it's now much easier to
call kvm_[vm_]ioctl, but that's the key point of this change:
So far we primarily complicated the internal interface between generic
and arch-dependent kvm parts by requiring kvm_state joggling. But
external users already find interfaces without this restriction
(kvm_log_*, kvm_ioeventfd_*, ...). That's because it's at least
complicated to _cleanly_ pass kvm_state references to all users that
need it - e.g. sysbus devices like kvmclock or upcoming in-kernel irqchips.
I think you're basically making my point for me.
ioeventfd is a broken interface. It shouldn't be a VM ioctl but rather
a VCPU ioctl because PIO events are dispatched on a per-VCPU basis.
kvm_state is available as part of CPU state so it's quite easy to get at
if these interfaces just took a CPUState argument (and they should).
Let's just stop this artificial abstraction that has no practical use
and focus on detecting layering violations via code review. That's more
reliable IMHO.
Documenting relationships between devices and the CPU is a very
important task. Being able to grep for cpu_single_env to see what
devices models are interacting with the CPU is a very good thing.
When you've got these interactions hidden in a spaghetti of function
calls, things become impossible to understand.
Regards,
Anthony Liguori
Jan
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
