From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Liu, Jinsong" Subject: RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest Date: Sun, 11 Mar 2012 18:54:15 +0000 Message-ID: References: <4F0482D6.8080705@web.de> <4F060961.9050002@web.de> <4F0A099C.5040805@web.de> <4F4BBAEC.2040603@siemens.com> <4F55E39F.6020608@siemens.com> <4F5A5257.9060008@siemens.com> <4F5A6D8D.2040104@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT Cc: "qemu-devel@nongnu.org" , Marcelo Tosatti , Avi Kivity , kvm , Alexey Zaytsev , =?iso-8859-1?Q?Andreas_F=E4rber?= To: Jan Kiszka Return-path: Received: from mga03.intel.com ([143.182.124.21]:5545 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751988Ab2CKSyU convert rfc822-to-8bit (ORCPT ); Sun, 11 Mar 2012 14:54:20 -0400 In-Reply-To: <4F5A6D8D.2040104@web.de> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: Jan Kiszka wrote: > On 2012-03-09 20:09, Liu, Jinsong wrote: >> Jan Kiszka wrote: >>> On 2012-03-09 19:27, Liu, Jinsong wrote: >>>> Jan Kiszka wrote: >>>>> On 2012-03-06 08:49, Liu, Jinsong wrote: >>>>>> Jan, >>>>>> >>>>>> Any comments? I feel some confused about your point 'disable >>>>>> cpuid feature for older machine types by default': are you >>>>>> planning a common approach for this common issue, or, you just >>>>>> ask me a specific solution for the tsc deadline timer case? >>>>> >>>>> I think a generic solution for this can be as simple as passing a >>>>> feature exclusion mask to cpu_init. You could simple append a >>>>> string of "-feature1,-feature2" to the cpu model that is >>>>> specified on creation. And that string could be defined in the >>>>> compat machine descriptions. Does this make sense? >>>>> >>>> >>>> Jan, to prevent misunderstanding, I elaborate my understanding of >>>> your points below (if any misunderstanding please point out to >>>> me): ===================== Your target is, to migrate from A(old >>>> qemu) to B(new qemu) by >>>> 1. at A: qemu-version-A [-cpu whatever] // currently the >>>> default machine type is pc-A >>>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 >>>> -feature2 >>>> >>>> B run new qemu-version-B (w/ new features 'feature1' and >>>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should >>>> not see 'feature1' and 'feature2', so commandline append string to >>>> cpu model '-cpu whatever -feature1 -feature2' to hidden new >>>> feature1 and feature2 to vm, hence vm can see same cpuid features >>>> (at B) as those at A (which means, no feature1, no feature2) >>>> ===================== >>>> >>>> If my understanding of your thoughts is right, I think currently >>>> qemu has satisfied your target, code refer to >>>> pc_cpus_init(cpu_model) ...... cpu_init(cpu_model) >>>> --> cpu_x86_register(*env, cpu_model) >>>> --> cpu_x86_find_by_name(*def, cpu_model) // parse '+/- >>>> >>>> features', generate feature masks plus_features... // and >>>> minus_features...(this is feature exclusion masks you want) >>>> I think your point 'define in the compat machine description' is >>>> unnecessary. >>> >>> The user would have to specify the new feature as exclusions >>> *manually* on the command line if -machine pc-A doesn't inject them >>> *automatically*. So it is necessary to enhance qemu in this regard. >>> >> >> ... You suggest 'append a string of "-feature1,-feature2" to the cpu >> model that is specified on creation' at your last email. Could you >> tell me other way user exclude features? I only know qemu command >> line :-( > > I was thinking of something like > > diff --git a/hw/boards.h b/hw/boards.h > index 667177d..2bae071 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -28,6 +28,7 @@ typedef struct QEMUMachine { > int is_default; > const char *default_machine_opts; > GlobalProperty *compat_props; > + const char *compat_cpu_features; > struct QEMUMachine *next; > } QEMUMachine; > > diff --git a/hw/pc.c b/hw/pc.c > index bb9867b..4d11559 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model) > return env; > } > > -void pc_cpus_init(const char *cpu_model) > +void pc_cpus_init(const char *cpu_model, const char *append_features) > { > + char *model_and_features; > int i; > > /* init CPUs */ > @@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model) > cpu_model = "qemu32"; > #endif > } > + model_and_features = g_strconcat(cpu_model, ",", > append_features, NULL); > > for(i = 0; i < smp_cpus; i++) { > - pc_new_cpu(cpu_model); > + pc_new_cpu(model_and_features); > } > + > + g_free(model_and_features); > } > > void pc_memory_init(MemoryRegion *system_memory, > > > However, getting machine.compat_cpu_features to pc_cpus_init is rather > ugly. And we will have CPU devices with real properties soon. Then the > compat feature string could be passed that way, without changing any > machine init function. > > Andreas, do you expect CPU devices to be ready for qemu 1.1? We would > need them to pass a feature exclusion mask from machine.compat_props > to > the (x86) CPU init code. cpu devices is just another format of current cpu_model. It helps nothing to our problem. Again, the point is, by what method the feature exclusion mask can be generated, if user not give hint manually? Thanks, Jinsong > > Well, given that introducing some intermediate solution for this would > be complex and hacky and that there is a way to configure tsc_deadline > for old machines away, though only an explicit one, I could live with > postponing the feature mask after the CPU device conversion. But the > last word will have the maintainers. > > Jan From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S6nuT-00066Y-HW for qemu-devel@nongnu.org; Sun, 11 Mar 2012 14:54:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S6nuR-0002sR-E2 for qemu-devel@nongnu.org; Sun, 11 Mar 2012 14:54:25 -0400 Received: from mga03.intel.com ([143.182.124.21]:39281) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S6nuR-0002rG-0f for qemu-devel@nongnu.org; Sun, 11 Mar 2012 14:54:23 -0400 From: "Liu, Jinsong" Date: Sun, 11 Mar 2012 18:54:15 +0000 Message-ID: References: <4F0482D6.8080705@web.de> <4F060961.9050002@web.de> <4F0A099C.5040805@web.de> <4F4BBAEC.2040603@siemens.com> <4F55E39F.6020608@siemens.com> <4F5A5257.9060008@siemens.com> <4F5A6D8D.2040104@web.de> In-Reply-To: <4F5A6D8D.2040104@web.de> Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Alexey Zaytsev , kvm , Marcelo Tosatti , "qemu-devel@nongnu.org" , Avi Kivity , =?iso-8859-1?Q?Andreas_F=E4rber?= Jan Kiszka wrote: > On 2012-03-09 20:09, Liu, Jinsong wrote: >> Jan Kiszka wrote: >>> On 2012-03-09 19:27, Liu, Jinsong wrote: >>>> Jan Kiszka wrote: >>>>> On 2012-03-06 08:49, Liu, Jinsong wrote: >>>>>> Jan, >>>>>>=20 >>>>>> Any comments? I feel some confused about your point 'disable >>>>>> cpuid feature for older machine types by default': are you >>>>>> planning a common approach for this common issue, or, you just >>>>>> ask me a specific solution for the tsc deadline timer case? >>>>>=20 >>>>> I think a generic solution for this can be as simple as passing a >>>>> feature exclusion mask to cpu_init. You could simple append a >>>>> string of "-feature1,-feature2" to the cpu model that is >>>>> specified on creation. And that string could be defined in the >>>>> compat machine descriptions. Does this make sense? >>>>>=20 >>>>=20 >>>> Jan, to prevent misunderstanding, I elaborate my understanding of >>>> your points below (if any misunderstanding please point out to >>>> me): =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Y= our target is, to migrate from A(old >>>> qemu) to B(new qemu) by=20 >>>> 1. at A: qemu-version-A [-cpu whatever] // currently the >>>> default machine type is pc-A=20 >>>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1 >>>> -feature2=20 >>>>=20 >>>> B run new qemu-version-B (w/ new features 'feature1' and >>>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should >>>> not see 'feature1' and 'feature2', so commandline append string to >>>> cpu model '-cpu whatever -feature1 -feature2' to hidden new >>>> feature1 and feature2 to vm, hence vm can see same cpuid features >>>> (at B) as those at A (which means, no feature1, no feature2) >>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>=20 >>>> If my understanding of your thoughts is right, I think currently >>>> qemu has satisfied your target, code refer to >>>> pc_cpus_init(cpu_model) ...... cpu_init(cpu_model) >>>> --> cpu_x86_register(*env, cpu_model) >>>> --> cpu_x86_find_by_name(*def, cpu_model) // parse '+/- >>>>=20 >>>> features', generate feature masks plus_features... // and >>>> minus_features...(this is feature exclusion masks you want) >>>> I think your point 'define in the compat machine description' is >>>> unnecessary. >>>=20 >>> The user would have to specify the new feature as exclusions >>> *manually* on the command line if -machine pc-A doesn't inject them >>> *automatically*. So it is necessary to enhance qemu in this regard. >>>=20 >>=20 >> ... You suggest 'append a string of "-feature1,-feature2" to the cpu >> model that is specified on creation' at your last email. Could you >> tell me other way user exclude features? I only know qemu command >> line :-( =20 >=20 > I was thinking of something like >=20 > diff --git a/hw/boards.h b/hw/boards.h > index 667177d..2bae071 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -28,6 +28,7 @@ typedef struct QEMUMachine { > int is_default; > const char *default_machine_opts; > GlobalProperty *compat_props; > + const char *compat_cpu_features; > struct QEMUMachine *next; > } QEMUMachine; >=20 > diff --git a/hw/pc.c b/hw/pc.c > index bb9867b..4d11559 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char *cpu_model) > return env; > } >=20 > -void pc_cpus_init(const char *cpu_model) > +void pc_cpus_init(const char *cpu_model, const char *append_features) > { > + char *model_and_features; > int i; >=20 > /* init CPUs */ > @@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model) > cpu_model =3D "qemu32"; > #endif > } > + model_and_features =3D g_strconcat(cpu_model, ",", > append_features, NULL);=20 >=20 > for(i =3D 0; i < smp_cpus; i++) { > - pc_new_cpu(cpu_model); > + pc_new_cpu(model_and_features); > } > + > + g_free(model_and_features); > } >=20 > void pc_memory_init(MemoryRegion *system_memory, >=20 >=20 > However, getting machine.compat_cpu_features to pc_cpus_init is rather > ugly. And we will have CPU devices with real properties soon. Then the > compat feature string could be passed that way, without changing any > machine init function. >=20 > Andreas, do you expect CPU devices to be ready for qemu 1.1? We would > need them to pass a feature exclusion mask from machine.compat_props > to=20 > the (x86) CPU init code. cpu devices is just another format of current cpu_model. It helps nothing t= o our problem. Again, the point is, by what method the feature exclusion mask can be gener= ated, if user not give hint manually? Thanks, Jinsong >=20 > Well, given that introducing some intermediate solution for this would > be complex and hacky and that there is a way to configure tsc_deadline > for old machines away, though only an explicit one, I could live with > postponing the feature mask after the CPU device conversion. But the > last word will have the maintainers. >=20 > Jan