Re: [Qemu-devel] [PATCH 11/12] kvm: x86: Pass KVMState to kvm_arch_get_supported_cpuid
On Wed, Jun 08, 2011 at 04:11:05PM +0200, Jan Kiszka wrote: > kvm_arch_get_supported_cpuid checks for global cpuid restrictions, it > does not require any CPUState reference. Changing its interface allows > to call it before any VCPU is initialized. I'm wondering if it wouldn't be simpler to keep the existing interface but just initialize CPUState->kvm_state earlier (today it is initialized only on kvm_init_vcpu(), although the kvm_state global is initialized much earlier). Even with this new KVMState-based interface, code that needs to use these functions before kvm_init_vcpu() (e.g. check_features_against_host()) will have to use the 'kvm_state' global (and I would like to avoid that). -- Eduardo
Re: [Qemu-devel] [PATCH 11/12] kvm: x86: Pass KVMState to kvm_arch_get_supported_cpuid
On 2011-06-09 19:33, Eduardo Habkost wrote: > On Wed, Jun 08, 2011 at 04:11:05PM +0200, Jan Kiszka wrote: >> kvm_arch_get_supported_cpuid checks for global cpuid restrictions, it >> does not require any CPUState reference. Changing its interface allows >> to call it before any VCPU is initialized. > > I'm wondering if it wouldn't be simpler to keep the existing interface > but just initialize CPUState->kvm_state earlier (today it is initialized > only on kvm_init_vcpu(), although the kvm_state global is initialized > much earlier). If there was more need for a slit up, I would agree. But I do not see it. > > Even with this new KVMState-based interface, code that needs to use > these functions before kvm_init_vcpu() (e.g. > check_features_against_host()) will have to use the 'kvm_state' global > (and I would like to avoid that). That is a different topic that needs to be resolved for other use cases as well (e.g. for kvm devices). In many cases you should be able to find a way to pass kvm_state from functions that already holds a reference. Is that not true for your use case? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 11/12] kvm: x86: Pass KVMState to kvm_arch_get_supported_cpuid
On Thu, Jun 09, 2011 at 07:41:03PM +0200, Jan Kiszka wrote: > > I'm wondering if it wouldn't be simpler to keep the existing interface > > but just initialize CPUState->kvm_state earlier (today it is initialized > > only on kvm_init_vcpu(), although the kvm_state global is initialized > > much earlier). > > If there was more need for a slit up, I would agree. But I do not see it. I guess it won't hurt to make the functions require only what they really need. > > > > Even with this new KVMState-based interface, code that needs to use > > these functions before kvm_init_vcpu() (e.g. > > check_features_against_host()) will have to use the 'kvm_state' global > > (and I would like to avoid that). > > That is a different topic that needs to be resolved for other use cases > as well (e.g. for kvm devices). In many cases you should be able to find > a way to pass kvm_state from functions that already holds a reference. > Is that not true for your use case? My use case is cpu_x86_register()/check_features_against_host(), that hold only a CPUState reference, and I wouldn't like to add a new reference to the kvm_state global variable there. -- Eduardo
Re: [Qemu-devel] [PATCH 11/12] kvm: x86: Pass KVMState to kvm_arch_get_supported_cpuid
On 2011-06-09 20:08, Eduardo Habkost wrote: > On Thu, Jun 09, 2011 at 07:41:03PM +0200, Jan Kiszka wrote: > >>> I'm wondering if it wouldn't be simpler to keep the existing interface >>> but just initialize CPUState->kvm_state earlier (today it is initialized >>> only on kvm_init_vcpu(), although the kvm_state global is initialized >>> much earlier). >> >> If there was more need for a slit up, I would agree. But I do not see it. > > I guess it won't hurt to make the functions require only what they > really need. Yes, so this patch has an independent value IMO. > >>> >>> Even with this new KVMState-based interface, code that needs to use >>> these functions before kvm_init_vcpu() (e.g. >>> check_features_against_host()) will have to use the 'kvm_state' global >>> (and I would like to avoid that). >> >> That is a different topic that needs to be resolved for other use cases >> as well (e.g. for kvm devices). In many cases you should be able to find >> a way to pass kvm_state from functions that already holds a reference. >> Is that not true for your use case? > > My use case is cpu_x86_register()/check_features_against_host(), that > hold only a CPUState reference, and I wouldn't like to add a new > reference to the kvm_state global variable there. For me, this is like requiring kvm_state for kvm_check_extension, and I wouldn't hesitate using the global state until we solved that in a better way. But if you feel like it's not appropriate here, then let's go for early/late vcpu init. Jan signature.asc Description: OpenPGP digital signature
