Patchwork [v8,18/18] kvm: Activate in-kernel irqchip support

login
register
mail settings
Submitter Jan Kiszka
Date Jan. 19, 2012, 11:25 a.m.
Message ID <6a48ffaaa732b2142c1b5030178f2d4a0fa499fe.1326972302.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/136826/
State New
Headers show

Comments

Jan Kiszka - Jan. 19, 2012, 11:25 a.m.
Make the basic in-kernel irqchip support selectable via
-machine ...,kernel_irqchip=on. Leave it off by default until it can
fully replace user space models.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-config.c   |    4 ++++
 qemu-options.hx |    5 ++++-
 2 files changed, 8 insertions(+), 1 deletions(-)
Marcelo Tosatti - Jan. 19, 2012, 6:08 p.m.
On Thu, Jan 19, 2012 at 12:25:10PM +0100, Jan Kiszka wrote:
> Make the basic in-kernel irqchip support selectable via
> -machine ...,kernel_irqchip=on. Leave it off by default until it can
> fully replace user space models.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  qemu-config.c   |    4 ++++
>  qemu-options.hx |    5 ++++-
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/qemu-config.c b/qemu-config.c
> index ecc88e8..b030205 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -531,6 +531,10 @@ static QemuOptsList qemu_machine_opts = {
>              .name = "accel",
>              .type = QEMU_OPT_STRING,
>              .help = "accelerator list",
> +        }, {
> +            .name = "kernel_irqchip",
> +            .type = QEMU_OPT_BOOL,
> +            .help = "use KVM in-kernel irqchip",
>          },
>          { /* End of list */ }
>      },


Pulled (and pushed), but you forgot to actually hook this option to kvm_irqchip_in_kernel?
Jan Kiszka - Jan. 19, 2012, 6:54 p.m.
On 2012-01-19 19:08, Marcelo Tosatti wrote:
> On Thu, Jan 19, 2012 at 12:25:10PM +0100, Jan Kiszka wrote:
>> Make the basic in-kernel irqchip support selectable via
>> -machine ...,kernel_irqchip=on. Leave it off by default until it can
>> fully replace user space models.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  qemu-config.c   |    4 ++++
>>  qemu-options.hx |    5 ++++-
>>  2 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-config.c b/qemu-config.c
>> index ecc88e8..b030205 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -531,6 +531,10 @@ static QemuOptsList qemu_machine_opts = {
>>              .name = "accel",
>>              .type = QEMU_OPT_STRING,
>>              .help = "accelerator list",
>> +        }, {
>> +            .name = "kernel_irqchip",
>> +            .type = QEMU_OPT_BOOL,
>> +            .help = "use KVM in-kernel irqchip",
>>          },
>>          { /* End of list */ }
>>      },
> 
> 
> Pulled (and pushed),

Thanks. Will refresh my back-merge patch for qemu-kvm now.

> but you forgot to actually hook this option to kvm_irqchip_in_kernel?
> 

Nope, see kvm_irqchip_create, patch 13. You can also check by browsing
the qtree (different device model names).

Jan
Avi Kivity - Jan. 24, 2012, 2:05 p.m.
On 01/19/2012 08:54 PM, Jan Kiszka wrote:
> Nope, see kvm_irqchip_create, patch 13. You can also check by browsing
> the qtree (different device model names).

That was my biggest objection to the previous iterations.  Later
versions changed to use an attribute (selecting the backend).  What
happened now?
Jan Kiszka - Jan. 24, 2012, 2:10 p.m.
On 2012-01-24 15:05, Avi Kivity wrote:
> On 01/19/2012 08:54 PM, Jan Kiszka wrote:
>> Nope, see kvm_irqchip_create, patch 13. You can also check by browsing
>> the qtree (different device model names).
> 
> That was my biggest objection to the previous iterations.  Later
> versions changed to use an attribute (selecting the backend).  What
> happened now?

The other approach was rejected.

Moreover, see http://thread.gmane.org/gmane.comp.emulators.qemu/129659,
QOM will allow addressing with identical paths.

Jan
Avi Kivity - Jan. 24, 2012, 2:13 p.m.
On 01/24/2012 04:10 PM, Jan Kiszka wrote:
> On 2012-01-24 15:05, Avi Kivity wrote:
> > On 01/19/2012 08:54 PM, Jan Kiszka wrote:
> >> Nope, see kvm_irqchip_create, patch 13. You can also check by browsing
> >> the qtree (different device model names).
> > 
> > That was my biggest objection to the previous iterations.  Later
> > versions changed to use an attribute (selecting the backend).  What
> > happened now?
>
> The other approach was rejected.

Sigh.  We'll regret this.

> Moreover, see http://thread.gmane.org/gmane.comp.emulators.qemu/129659,
> QOM will allow addressing with identical paths.

At least that.
Anthony Liguori - Jan. 24, 2012, 3:33 p.m.
On 01/24/2012 08:05 AM, Avi Kivity wrote:
> On 01/19/2012 08:54 PM, Jan Kiszka wrote:
>> Nope, see kvm_irqchip_create, patch 13. You can also check by browsing
>> the qtree (different device model names).
>
> That was my biggest objection to the previous iterations.  Later
> versions changed to use an attribute (selecting the backend).  What
> happened now?

They have different *type* names.  They can have the same *object* name.

All user interaction with a device should be through object name.  The type name 
is merely an implementation detail.

Regards,

Anthony Liguori

>
Anthony Liguori - Jan. 24, 2012, 3:34 p.m.
On 01/24/2012 08:13 AM, Avi Kivity wrote:
> On 01/24/2012 04:10 PM, Jan Kiszka wrote:
>> On 2012-01-24 15:05, Avi Kivity wrote:
>>> On 01/19/2012 08:54 PM, Jan Kiszka wrote:
>>>> Nope, see kvm_irqchip_create, patch 13. You can also check by browsing
>>>> the qtree (different device model names).
>>>
>>> That was my biggest objection to the previous iterations.  Later
>>> versions changed to use an attribute (selecting the backend).  What
>>> happened now?
>>
>> The other approach was rejected.
>
> Sigh.  We'll regret this.

No we won't.  The other approach was creating a mini-qdev as a mechanism to hide 
the fact that we have two different implementations of the same device.

We need to expose this information to the user in some fashion.  The type is the 
natural way to do it.

Regards,

Anthony Liguori

>
>> Moreover, see http://thread.gmane.org/gmane.comp.emulators.qemu/129659,
>> QOM will allow addressing with identical paths.
>
> At least that.

Patch

diff --git a/qemu-config.c b/qemu-config.c
index ecc88e8..b030205 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -531,6 +531,10 @@  static QemuOptsList qemu_machine_opts = {
             .name = "accel",
             .type = QEMU_OPT_STRING,
             .help = "accelerator list",
+        }, {
+            .name = "kernel_irqchip",
+            .type = QEMU_OPT_BOOL,
+            .help = "use KVM in-kernel irqchip",
         },
         { /* End of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index 6295cde..3a07ae8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -31,7 +31,8 @@  DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "-machine [type=]name[,prop[=value][,...]]\n"
     "                selects emulated machine (-machine ? for list)\n"
     "                property accel=accel1[:accel2[:...]] selects accelerator\n"
-    "                supported accelerators are kvm, xen, tcg (default: tcg)\n",
+    "                supported accelerators are kvm, xen, tcg (default: tcg)\n"
+    "                kernel_irqchip=on|off controls accelerated irqchip support\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
@@ -44,6 +45,8 @@  This is used to enable an accelerator. Depending on the target architecture,
 kvm, xen, or tcg can be available. By default, tcg is used. If there is more
 than one accelerator specified, the next one is used if the previous one fails
 to initialize.
+@item kernel_irqchip=on|off
+Enables in-kernel irqchip support for the chosen accelerator when available.
 @end table
 ETEXI