All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@web.de>
To: "Liu, Jinsong" <jinsong.liu@intel.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Avi Kivity" <avi@redhat.com>, kvm <kvm@vger.kernel.org>,
	"Alexey Zaytsev" <alexey.zaytsev@gmail.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Date: Fri, 09 Mar 2012 21:52:29 +0100	[thread overview]
Message-ID: <4F5A6D8D.2040104@web.de> (raw)
In-Reply-To: <DE8DF0795D48FD4CA783C40EC82923350D749C@SHSMSX101.ccr.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 4644 bytes --]

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.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kiszka <jan.kiszka@web.de>
To: "Liu, Jinsong" <jinsong.liu@intel.com>
Cc: "Alexey Zaytsev" <alexey.zaytsev@gmail.com>,
	kvm <kvm@vger.kernel.org>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Avi Kivity" <avi@redhat.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Date: Fri, 09 Mar 2012 21:52:29 +0100	[thread overview]
Message-ID: <4F5A6D8D.2040104@web.de> (raw)
In-Reply-To: <DE8DF0795D48FD4CA783C40EC82923350D749C@SHSMSX101.ccr.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 4644 bytes --]

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.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

  reply	other threads:[~2012-03-09 20:52 UTC|newest]

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-28 21:55 [PATCH 2/2] Expose tsc deadline timer cpuid to guest Liu, Jinsong
2011-12-28 21:55 ` [Qemu-devel] " Liu, Jinsong
2012-01-04 16:48 ` Jan Kiszka
2012-01-04 16:48   ` [Qemu-devel] " Jan Kiszka
2012-01-05 20:07   ` Liu, Jinsong
2012-01-05 20:07     ` [Qemu-devel] " Liu, Jinsong
2012-01-05 20:34     ` Jan Kiszka
2012-01-05 20:34       ` [Qemu-devel] " Jan Kiszka
2012-01-07 18:23       ` Liu, Jinsong
2012-01-07 18:23         ` [Qemu-devel] " Liu, Jinsong
2012-01-08 21:24         ` Jan Kiszka
2012-01-08 21:24           ` [Qemu-devel] " Jan Kiszka
2012-02-27 16:05           ` Liu, Jinsong
2012-02-27 16:05             ` [Qemu-devel] " Liu, Jinsong
2012-02-27 17:18             ` Jan Kiszka
2012-02-27 17:18               ` [Qemu-devel] " Jan Kiszka
2012-02-28 10:30               ` Liu, Jinsong
2012-02-28 10:30                 ` [Qemu-devel] " Liu, Jinsong
2012-03-06  7:49                 ` Liu, Jinsong
2012-03-06  7:49                   ` Liu, Jinsong
2012-03-06 10:14                   ` Jan Kiszka
2012-03-06 10:14                     ` Jan Kiszka
2012-03-09 18:27                     ` Liu, Jinsong
2012-03-09 18:27                       ` [Qemu-devel] " Liu, Jinsong
2012-03-09 18:56                       ` Jan Kiszka
2012-03-09 18:56                         ` Jan Kiszka
2012-03-09 19:09                         ` Liu, Jinsong
2012-03-09 19:09                           ` Liu, Jinsong
2012-03-09 20:52                           ` Jan Kiszka [this message]
2012-03-09 20:52                             ` Jan Kiszka
2012-03-10  1:07                             ` Andreas Färber
2012-03-10  1:07                               ` Andreas Färber
2012-03-11 18:54                             ` Liu, Jinsong
2012-03-11 18:54                               ` Liu, Jinsong
2012-03-12 17:21                             ` Eduardo Habkost
2012-03-25  8:51                               ` Liu, Jinsong
2012-03-25  8:51                                 ` [Qemu-devel] " Liu, Jinsong
2012-03-09 19:29                       ` Liu, Jinsong
2012-03-09 19:29                         ` [Qemu-devel] " Liu, Jinsong
2012-03-19 22:35                       ` Rik van Riel
2012-03-20 12:53                         ` Liu, Jinsong
2012-03-20 13:33                           ` Eduardo Habkost
2012-03-20 13:33                             ` Eduardo Habkost
2012-03-23  3:49                             ` Liu, Jinsong
2012-03-23  3:49                               ` Liu, Jinsong
2012-03-23 13:46                               ` Eduardo Habkost
2012-03-23 13:46                                 ` Eduardo Habkost
2012-03-23 14:17                                 ` Liu, Jinsong
2012-03-23 14:17                                   ` Liu, Jinsong
2012-04-19 20:03                                   ` Eduardo Habkost
2012-04-20 10:12                                     ` Jan Kiszka
2012-04-20 15:00                                       ` Eduardo Habkost
2012-04-20 15:19                                         ` [Qemu-devel] " Jan Kiszka
2012-04-20 15:36                                           ` Eduardo Habkost
2012-04-21  7:23                                             ` Jan Kiszka
2012-04-23 14:48                                               ` Eduardo Habkost
2012-04-23 16:31                                                 ` Jan Kiszka
2012-04-23 20:02                                                   ` Eduardo Habkost
2012-04-24 16:06                                                     ` Jan Kiszka
2012-04-24 17:19                                                       ` [Qemu-devel] " Eduardo Habkost
2012-05-07 18:21                                                         ` Semantics of "-cpu host" (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest) Eduardo Habkost
2012-05-07 18:21                                                           ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost
2012-05-08  0:58                                                           ` Alexander Graf
2012-05-08  0:58                                                             ` [Qemu-devel] " Alexander Graf
2012-05-08 20:14                                                             ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Eduardo Habkost
2012-05-08 20:14                                                               ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost
2012-05-08 22:07                                                               ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf
2012-05-08 22:07                                                                 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf
2012-05-09  8:14                                                                 ` Gleb Natapov
2012-05-09  8:14                                                                   ` [Qemu-devel] " Gleb Natapov
2012-05-09  8:42                                                                   ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf
2012-05-09  8:42                                                                     ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf
2012-05-09  8:51                                                                     ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Gleb Natapov
2012-05-09  8:51                                                                       ` [Qemu-devel] Semantics of "-cpu host" (was " Gleb Natapov
2012-05-09  9:05                                                                       ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf
2012-05-09  9:05                                                                         ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf
2012-05-09  9:38                                                                         ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Gleb Natapov
2012-05-09  9:38                                                                           ` [Qemu-devel] Semantics of "-cpu host" (was " Gleb Natapov
2012-05-09  9:54                                                                           ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf
2012-05-09  9:54                                                                             ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf
2012-05-09 19:38                                                                           ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Eduardo Habkost
2012-05-09 19:38                                                                             ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost
2012-05-09 20:30                                                                             ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf
2012-05-09 20:30                                                                               ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf
2012-05-10 12:53                                                                             ` Gleb Natapov
2012-05-10 12:53                                                                               ` [Qemu-devel] " Gleb Natapov
2012-05-10 13:21                                                                               ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf
2012-05-10 13:21                                                                                 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf
2012-05-10 13:39                                                                                 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Gleb Natapov
2012-05-10 13:39                                                                                   ` [Qemu-devel] Semantics of "-cpu host" (was " Gleb Natapov
2012-05-10 14:12                                                                                   ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Eduardo Habkost
2012-05-10 14:12                                                                                     ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost
2012-05-09  7:16                                                           ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Andre Przywara
2012-05-09  7:16                                                             ` [Qemu-devel] Semantics of "-cpu host" (was " Andre Przywara
2012-06-14 19:02                                                         ` [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest Liu, Jinsong
2012-06-14 19:02                                                           ` Liu, Jinsong
2012-06-14 19:12                                                           ` Eduardo Habkost
2012-06-14 19:12                                                             ` Eduardo Habkost
2012-06-14 19:18                                                             ` Liu, Jinsong
2012-06-14 19:18                                                               ` [Qemu-devel] " Liu, Jinsong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F5A6D8D.2040104@web.de \
    --to=jan.kiszka@web.de \
    --cc=afaerber@suse.de \
    --cc=alexey.zaytsev@gmail.com \
    --cc=avi@redhat.com \
    --cc=jinsong.liu@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.