| Message ID | 1244488224-31171-4-git-send-email-glommer@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Glauber Costa wrote: > This patch should be applied to main qemu, but I'll > first post it here for appreciation. In this patch, > we move KVMState definition to kvm.h header. With this > done, we can also use its definition in our files, until > there is no more such thing as "our" files. This is too > selfish anyway. > > Later on, we'll move our internal state inside it. Well, in upstream no one outside kvm-all.c needs to (and likely should be allowed to) access fields from struct KVMState & KVMSlot directly. That avoids misuse outside the KVM layer and enforces KVM arch code to properly call into the generic layer. But I see the problem for qemu-kvm's transition time, so let's try to find an intermediate solution until its code layout is aligned (I don't see any blockers for this). Suggestion: Replicate the relevant structures into a new, temporary header. If upstream may extend its original structures, this should from now on have happened *first* inside qemu-kvm, so no inconsistency can arise unless downstream messed it up already. At some point (hopefully not too far away), no user of that header will remain and we will be able to drop it again. Jan
On Mon, Jun 08, 2009 at 09:54:34PM +0200, Jan Kiszka wrote: > Glauber Costa wrote: > > This patch should be applied to main qemu, but I'll > > first post it here for appreciation. In this patch, > > we move KVMState definition to kvm.h header. With this > > done, we can also use its definition in our files, until > > there is no more such thing as "our" files. This is too > > selfish anyway. > > > > Later on, we'll move our internal state inside it. > > Well, in upstream no one outside kvm-all.c needs to (and likely should > be allowed to) access fields from struct KVMState & KVMSlot directly. > That avoids misuse outside the KVM layer and enforces KVM arch code to > properly call into the generic layer. > > But I see the problem for qemu-kvm's transition time, so let's try to > find an intermediate solution until its code layout is aligned (I don't > see any blockers for this). Suggestion: Replicate the relevant > structures into a new, temporary header. If upstream may extend its > original structures, this should from now on have happened *first* > inside qemu-kvm, so no inconsistency can arise unless downstream messed > it up already. At some point (hopefully not too far away), no user of > that header will remain and we will be able to drop it again. I'm fine with whatever anthony wants. > > Jan > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Glauber Costa wrote: > On Mon, Jun 08, 2009 at 09:54:34PM +0200, Jan Kiszka wrote: > >> Glauber Costa wrote: >> >>> This patch should be applied to main qemu, but I'll >>> first post it here for appreciation. In this patch, >>> we move KVMState definition to kvm.h header. With this >>> done, we can also use its definition in our files, until >>> there is no more such thing as "our" files. This is too >>> selfish anyway. >>> >>> Later on, we'll move our internal state inside it. >>> >> Well, in upstream no one outside kvm-all.c needs to (and likely should >> be allowed to) access fields from struct KVMState & KVMSlot directly. >> That avoids misuse outside the KVM layer and enforces KVM arch code to >> properly call into the generic layer. >> >> But I see the problem for qemu-kvm's transition time, so let's try to >> find an intermediate solution until its code layout is aligned (I don't >> see any blockers for this). Suggestion: Replicate the relevant >> structures into a new, temporary header. If upstream may extend its >> original structures, this should from now on have happened *first* >> inside qemu-kvm, so no inconsistency can arise unless downstream messed >> it up already. At some point (hopefully not too far away), no user of >> that header will remain and we will be able to drop it again. >> > I'm fine with whatever anthony wants. > I'm fine with either solution as they are both temporary measures... Regards, Anthony Liguori -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/kvm-all.c b/kvm-all.c index c89e3b1..d60126c 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -39,32 +39,11 @@ do { } while (0) #endif -typedef struct KVMSlot -{ - target_phys_addr_t start_addr; - ram_addr_t memory_size; - ram_addr_t phys_offset; - int slot; - int flags; -} KVMSlot; typedef struct kvm_dirty_log KVMDirtyLog; int kvm_allowed = 0; -struct KVMState -{ - KVMSlot slots[32]; - int fd; - int vmfd; - int coalesced_mmio; - int broken_set_mem_region; - int migration_log; -#ifdef KVM_CAP_SET_GUEST_DEBUG - struct kvm_sw_breakpoint_head kvm_sw_breakpoints; -#endif -}; - static KVMState *kvm_state; static KVMSlot *kvm_alloc_slot(KVMState *s) diff --git a/kvm.h b/kvm.h index e1c6eba..553c03e 100644 --- a/kvm.h +++ b/kvm.h @@ -18,6 +18,37 @@ #include "sys-queue.h" #include "libkvm-all.h" +typedef struct KVMSlot +{ + target_phys_addr_t start_addr; + ram_addr_t memory_size; + ram_addr_t phys_offset; + int slot; + int flags; +} KVMSlot; + +struct kvm_sw_breakpoint { + target_ulong pc; + target_ulong saved_insn; + int use_count; + TAILQ_ENTRY(kvm_sw_breakpoint) entry; +}; + +TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint); + +struct KVMState +{ + KVMSlot slots[32]; + int fd; + int vmfd; + int coalesced_mmio; + int broken_set_mem_region; + int migration_log; +#ifdef KVM_CAP_SET_GUEST_DEBUG + struct kvm_sw_breakpoint_head kvm_sw_breakpoints; +#endif +}; + #ifdef KVM_UPSTREAM #ifdef CONFIG_KVM @@ -97,15 +128,6 @@ int kvm_arch_init_vcpu(CPUState *env); struct kvm_guest_debug; struct kvm_debug_exit_arch; -struct kvm_sw_breakpoint { - target_ulong pc; - target_ulong saved_insn; - int use_count; - TAILQ_ENTRY(kvm_sw_breakpoint) entry; -}; - -TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint); - int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info); struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env, diff --git a/qemu-kvm.h b/qemu-kvm.h index fa40542..88bbba4 100644 --- a/qemu-kvm.h +++ b/qemu-kvm.h @@ -81,14 +81,6 @@ void kvm_arch_cpu_reset(CPUState *env); struct kvm_guest_debug; struct kvm_debug_exit_arch; -struct kvm_sw_breakpoint { - target_ulong pc; - target_ulong saved_insn; - int use_count; - TAILQ_ENTRY(kvm_sw_breakpoint) entry; -}; -TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint); - extern struct kvm_sw_breakpoint_head kvm_sw_breakpoints; int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info);
This patch should be applied to main qemu, but I'll first post it here for appreciation. In this patch, we move KVMState definition to kvm.h header. With this done, we can also use its definition in our files, until there is no more such thing as "our" files. This is too selfish anyway. Later on, we'll move our internal state inside it. Signed-off-by: Glauber Costa <glommer@redhat.com> --- kvm-all.c | 21 --------------------- kvm.h | 40 +++++++++++++++++++++++++++++++--------- qemu-kvm.h | 8 -------- 3 files changed, 31 insertions(+), 38 deletions(-)