diff mbox series

[v3,15/22] microvm: make virtio irq base runtime configurable

Message ID 20200520132003.9492-16-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series microvm: add acpi support | expand

Commit Message

Gerd Hoffmann May 20, 2020, 1:19 p.m. UTC 
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
---
 include/hw/i386/microvm.h |  2 +-
 hw/i386/microvm.c         | 11 +++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé May 20, 2020, 1:29 p.m. UTC | #1 
On 5/20/20 3:19 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Sergio Lopez <slp@redhat.com>
> ---
>   include/hw/i386/microvm.h |  2 +-
>   hw/i386/microvm.c         | 11 +++++++----
>   2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index ba68d1f22bb3..2709306c4016 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -27,7 +27,6 @@
>   
>   /* Platform virtio definitions */
>   #define VIRTIO_MMIO_BASE      0xc0000000
> -#define VIRTIO_IRQ_BASE       5
>   #define VIRTIO_NUM_TRANSPORTS 8
>   #define VIRTIO_CMDLINE_MAXLEN 64
>   
> @@ -57,6 +56,7 @@ typedef struct {
>       bool auto_kernel_cmdline;
>   
>       /* Machine state */
> +    uint32_t virtio_irq_base;
>       bool kernel_cmdline_fixed;
>   } MicrovmMachineState;
>   
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index b8f0d3283758..ea4460714a99 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -121,10 +121,11 @@ static void microvm_devices_init(MicrovmMachineState *mms)
>   
>       kvmclock_create();
>   
> +    mms->virtio_irq_base = 8;

Hmm you made it runtime configurable indeed, but you also changed it 
from 5 to 8. Worth a different patch, or at least mentioning in commit 
description.

>       for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
>           sysbus_create_simple("virtio-mmio",
>                                VIRTIO_MMIO_BASE + i * 512,
> -                             x86ms->gsi[VIRTIO_IRQ_BASE + i]);
> +                             x86ms->gsi[mms->virtio_irq_base + i]);
>       }
>   
>       /* Optional and legacy devices */
> @@ -246,7 +247,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
>       x86ms->ioapic_as = &address_space_memory;
>   }
>   
> -static gchar *microvm_get_mmio_cmdline(gchar *name)
> +static gchar *microvm_get_mmio_cmdline(gchar *name, uint32_t virtio_irq_base)
>   {
>       gchar *cmdline;
>       gchar *separator;
> @@ -266,7 +267,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
>       ret = g_snprintf(cmdline, VIRTIO_CMDLINE_MAXLEN,
>                        " virtio_mmio.device=512@0x%lx:%ld",
>                        VIRTIO_MMIO_BASE + index * 512,
> -                     VIRTIO_IRQ_BASE + index);
> +                     virtio_irq_base + index);
>       if (ret < 0 || ret >= VIRTIO_CMDLINE_MAXLEN) {
>           g_free(cmdline);
>           return NULL;
> @@ -278,6 +279,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
>   static void microvm_fix_kernel_cmdline(MachineState *machine)
>   {
>       X86MachineState *x86ms = X86_MACHINE(machine);
> +    MicrovmMachineState *mms = MICROVM_MACHINE(machine);
>       BusState *bus;
>       BusChild *kid;
>       char *cmdline;
> @@ -301,7 +303,8 @@ static void microvm_fix_kernel_cmdline(MachineState *machine)
>               BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
>   
>               if (!QTAILQ_EMPTY(&mmio_bus->children)) {
> -                gchar *mmio_cmdline = microvm_get_mmio_cmdline(mmio_bus->name);
> +                gchar *mmio_cmdline = microvm_get_mmio_cmdline
> +                    (mmio_bus->name, mms->virtio_irq_base);
>                   if (mmio_cmdline) {
>                       char *newcmd = g_strjoin(NULL, cmdline, mmio_cmdline, NULL);
>                       g_free(mmio_cmdline);
>
Gerd Hoffmann May 25, 2020, 11:49 a.m. UTC | #2 
Hi,

> > -#define VIRTIO_IRQ_BASE       5

> > +    mms->virtio_irq_base = 8;
> 
> Hmm you made it runtime configurable indeed, but you also changed it from 5
> to 8. Worth a different patch, or at least mentioning in commit description.

Oops, that wasn't intentional.  I'll change it back to 5.

take care,
  Gerd
Philippe Mathieu-Daudé May 25, 2020, 12:27 p.m. UTC | #3 
On 5/25/20 1:49 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> -#define VIRTIO_IRQ_BASE       5
> 
>>> +    mms->virtio_irq_base = 8;
>>
>> Hmm you made it runtime configurable indeed, but you also changed it from 5
>> to 8. Worth a different patch, or at least mentioning in commit description.
> 
> Oops, that wasn't intentional.  I'll change it back to 5.

OK. Using 5:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff mbox series

Patch

diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index ba68d1f22bb3..2709306c4016 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -27,7 +27,6 @@ 
 
 /* Platform virtio definitions */
 #define VIRTIO_MMIO_BASE      0xc0000000
-#define VIRTIO_IRQ_BASE       5
 #define VIRTIO_NUM_TRANSPORTS 8
 #define VIRTIO_CMDLINE_MAXLEN 64
 
@@ -57,6 +56,7 @@  typedef struct {
     bool auto_kernel_cmdline;
 
     /* Machine state */
+    uint32_t virtio_irq_base;
     bool kernel_cmdline_fixed;
 } MicrovmMachineState;
 
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index b8f0d3283758..ea4460714a99 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -121,10 +121,11 @@  static void microvm_devices_init(MicrovmMachineState *mms)
 
     kvmclock_create();
 
+    mms->virtio_irq_base = 8;
     for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
         sysbus_create_simple("virtio-mmio",
                              VIRTIO_MMIO_BASE + i * 512,
-                             x86ms->gsi[VIRTIO_IRQ_BASE + i]);
+                             x86ms->gsi[mms->virtio_irq_base + i]);
     }
 
     /* Optional and legacy devices */
@@ -246,7 +247,7 @@  static void microvm_memory_init(MicrovmMachineState *mms)
     x86ms->ioapic_as = &address_space_memory;
 }
 
-static gchar *microvm_get_mmio_cmdline(gchar *name)
+static gchar *microvm_get_mmio_cmdline(gchar *name, uint32_t virtio_irq_base)
 {
     gchar *cmdline;
     gchar *separator;
@@ -266,7 +267,7 @@  static gchar *microvm_get_mmio_cmdline(gchar *name)
     ret = g_snprintf(cmdline, VIRTIO_CMDLINE_MAXLEN,
                      " virtio_mmio.device=512@0x%lx:%ld",
                      VIRTIO_MMIO_BASE + index * 512,
-                     VIRTIO_IRQ_BASE + index);
+                     virtio_irq_base + index);
     if (ret < 0 || ret >= VIRTIO_CMDLINE_MAXLEN) {
         g_free(cmdline);
         return NULL;
@@ -278,6 +279,7 @@  static gchar *microvm_get_mmio_cmdline(gchar *name)
 static void microvm_fix_kernel_cmdline(MachineState *machine)
 {
     X86MachineState *x86ms = X86_MACHINE(machine);
+    MicrovmMachineState *mms = MICROVM_MACHINE(machine);
     BusState *bus;
     BusChild *kid;
     char *cmdline;
@@ -301,7 +303,8 @@  static void microvm_fix_kernel_cmdline(MachineState *machine)
             BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
 
             if (!QTAILQ_EMPTY(&mmio_bus->children)) {
-                gchar *mmio_cmdline = microvm_get_mmio_cmdline(mmio_bus->name);
+                gchar *mmio_cmdline = microvm_get_mmio_cmdline
+                    (mmio_bus->name, mms->virtio_irq_base);
                 if (mmio_cmdline) {
                     char *newcmd = g_strjoin(NULL, cmdline, mmio_cmdline, NULL);
                     g_free(mmio_cmdline);