[Qemu-devel] [PATCH 0/2] couple ITS support fixups

[Qemu-devel] [PATCH 0/2] couple ITS support fixups

[Andrew Jones]

Andrew Jones (2):
  hw/arm/virt-acpi-build: fix MADT generation
  hw/arm/virt: no ITS on older machine types

 hw/arm/virt-acpi-build.c         | 14 ++++++--------
 hw/arm/virt.c                    | 15 +++++++++++----
 include/hw/arm/virt-acpi-build.h |  1 +
 3 files changed, 18 insertions(+), 12 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/2] hw/arm/virt-acpi-build: fix MADT generation

[Andrew Jones]

We can't return early from build_* functions, as build_header is
only called at the end.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt-acpi-build.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7b39b1d2d676..c31349561c95 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -556,15 +556,13 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
         gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
         gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
 
-        if (!its_class_name()) {
-            return;
+        if (its_class_name()) {
+            gic_its = acpi_data_push(table_data, sizeof *gic_its);
+            gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
+            gic_its->length = sizeof(*gic_its);
+            gic_its->translation_id = 0;
+            gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base);
         }
-
-        gic_its = acpi_data_push(table_data, sizeof *gic_its);
-        gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
-        gic_its->length = sizeof(*gic_its);
-        gic_its->translation_id = 0;
-        gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base);
     } else {
         gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
         gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/2] hw/arm/virt-acpi-build: fix MADT generation

[Auger Eric]

Hi Drew,
On 10/10/2016 18:35, Andrew Jones wrote:
> We can't return early from build_* functions, as build_header is
> only called at the end.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt-acpi-build.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7b39b1d2d676..c31349561c95 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -556,15 +556,13 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>          gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
>          gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
>  
> -        if (!its_class_name()) {
> -            return;
> +        if (its_class_name()) {
> +            gic_its = acpi_data_push(table_data, sizeof *gic_its);
> +            gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
> +            gic_its->length = sizeof(*gic_its);
> +            gic_its->translation_id = 0;
> +            gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base);
>          }
> -
> -        gic_its = acpi_data_push(table_data, sizeof *gic_its);
> -        gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
> -        gic_its->length = sizeof(*gic_its);
> -        gic_its->translation_id = 0;
> -        gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base);
>      } else {
>          gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
>          gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
> 
Thanks for spotting the issue

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

[Qemu-devel] [PATCH 2/2] hw/arm/virt: no ITS on older machine types

[Andrew Jones]

We should avoid exposing new hardware (through DT and ACPI) on older
machine types. This patch keeps 2.7 and older from changing, despite
the introduction of ITS support for 2.8.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---

As Eduardo pointed out long ago for a different reason, we should
probably replace VirtGuestInfo with direct use of VirtMachineClass,
like x86 has done to replace PcGuestInfo with direct use of
PCMachineClass. I'll work on that, but wanted to get this ITS
fixup in sooner than later, so I'm posting this patch now, which
requires 'no_its' to be duplicated.

Also, this patch will have a trivial conflict with Wei's PMU
property series.


 hw/arm/virt-acpi-build.c         |  2 +-
 hw/arm/virt.c                    | 15 +++++++++++----
 include/hw/arm/virt-acpi-build.h |  1 +
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index c31349561c95..48820f372830 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -556,7 +556,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
         gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
         gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
 
-        if (its_class_name()) {
+        if (its_class_name() && !guest_info->no_its) {
             gic_its = acpi_data_push(table_data, sizeof *gic_its);
             gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
             gic_its->length = sizeof(*gic_its);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0f6305d3c7f6..e3734d47df81 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -84,6 +84,7 @@ typedef struct {
     MachineClass parent;
     VirtBoardInfo *daughterboard;
     bool disallow_affinity_adjustment;
+    bool no_its;
 } VirtMachineClass;
 
 typedef struct {
@@ -552,7 +553,8 @@ static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
     fdt_add_v2m_gic_node(vbi);
 }
 
-static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
+static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type,
+                       bool secure, bool no_its)
 {
     /* We create a standalone GIC */
     DeviceState *gicdev;
@@ -616,9 +618,9 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
 
     fdt_add_gic_node(vbi, type);
 
-    if (type == 3) {
+    if (type == 3 && !no_its) {
         create_its(vbi, gicdev);
-    } else {
+    } else if (type == 2) {
         create_v2m(vbi, pic);
     }
 }
@@ -1376,7 +1378,7 @@ static void machvirt_init(MachineState *machine)
 
     create_flash(vbi, sysmem, secure_sysmem ? secure_sysmem : sysmem);
 
-    create_gic(vbi, pic, gic_version, vms->secure);
+    create_gic(vbi, pic, gic_version, vms->secure, vmc->no_its);
 
     fdt_add_pmu_nodes(vbi, gic_version);
 
@@ -1408,6 +1410,7 @@ static void machvirt_init(MachineState *machine)
     guest_info->irqmap = vbi->irqmap;
     guest_info->use_highmem = vms->highmem;
     guest_info->gic_version = gic_version;
+    guest_info->no_its = vmc->no_its;
     guest_info_state->machine_done.notify = virt_guest_info_machine_done;
     qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
 
@@ -1562,8 +1565,12 @@ static void virt_2_7_instance_init(Object *obj)
 
 static void virt_machine_2_7_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
     virt_machine_2_8_options(mc);
     SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_7);
+    /* ITS was introduced with 2.8 */
+    vmc->no_its = true;
 }
 DEFINE_VIRT_MACHINE(2, 7)
 
diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
index e43330ad659b..f5ec749b8fea 100644
--- a/include/hw/arm/virt-acpi-build.h
+++ b/include/hw/arm/virt-acpi-build.h
@@ -33,6 +33,7 @@ typedef struct VirtGuestInfo {
     const int *irqmap;
     bool use_highmem;
     int gic_version;
+    bool no_its;
 } VirtGuestInfo;
 
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/2] hw/arm/virt: no ITS on older machine types

[Auger Eric]

Hi Drew,

On 10/10/2016 18:35, Andrew Jones wrote:
> We should avoid exposing new hardware (through DT and ACPI) on older
> machine types. This patch keeps 2.7 and older from changing, despite
> the introduction of ITS support for 2.8.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> 
> As Eduardo pointed out long ago for a different reason, we should
> probably replace VirtGuestInfo with direct use of VirtMachineClass,
> like x86 has done to replace PcGuestInfo with direct use of
> PCMachineClass. I'll work on that, but wanted to get this ITS
> fixup in sooner than later, so I'm posting this patch now, which
> requires 'no_its' to be duplicated.
> 
> Also, this patch will have a trivial conflict with Wei's PMU
> property series.
> 
> 
>  hw/arm/virt-acpi-build.c         |  2 +-
>  hw/arm/virt.c                    | 15 +++++++++++----
>  include/hw/arm/virt-acpi-build.h |  1 +
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index c31349561c95..48820f372830 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -556,7 +556,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
>          gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
>          gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
>  
> -        if (its_class_name()) {
> +        if (its_class_name() && !guest_info->no_its) {
>              gic_its = acpi_data_push(table_data, sizeof *gic_its);
>              gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
>              gic_its->length = sizeof(*gic_its);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 0f6305d3c7f6..e3734d47df81 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -84,6 +84,7 @@ typedef struct {
>      MachineClass parent;
>      VirtBoardInfo *daughterboard;
>      bool disallow_affinity_adjustment;
> +    bool no_its;
>  } VirtMachineClass;
>  
>  typedef struct {
> @@ -552,7 +553,8 @@ static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
>      fdt_add_v2m_gic_node(vbi);
>  }
>  
> -static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
> +static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type,
> +                       bool secure, bool no_its)
>  {
>      /* We create a standalone GIC */
>      DeviceState *gicdev;
> @@ -616,9 +618,9 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
>  
>      fdt_add_gic_node(vbi, type);
>  
> -    if (type == 3) {
> +    if (type == 3 && !no_its) {
>          create_its(vbi, gicdev);
> -    } else {
> +    } else if (type == 2) {
>          create_v2m(vbi, pic);
>      }
>  }
> @@ -1376,7 +1378,7 @@ static void machvirt_init(MachineState *machine)
>  
>      create_flash(vbi, sysmem, secure_sysmem ? secure_sysmem : sysmem);
>  
> -    create_gic(vbi, pic, gic_version, vms->secure);
> +    create_gic(vbi, pic, gic_version, vms->secure, vmc->no_its);
>  
>      fdt_add_pmu_nodes(vbi, gic_version);
>  
> @@ -1408,6 +1410,7 @@ static void machvirt_init(MachineState *machine)
>      guest_info->irqmap = vbi->irqmap;
>      guest_info->use_highmem = vms->highmem;
>      guest_info->gic_version = gic_version;
> +    guest_info->no_its = vmc->no_its;
>      guest_info_state->machine_done.notify = virt_guest_info_machine_done;
>      qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
>  
> @@ -1562,8 +1565,12 @@ static void virt_2_7_instance_init(Object *obj)
>  
>  static void virt_machine_2_7_options(MachineClass *mc)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>      virt_machine_2_8_options(mc);
>      SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_7);
> +    /* ITS was introduced with 2.8 */
> +    vmc->no_its = true;
>  }
>  DEFINE_VIRT_MACHINE(2, 7)
>  
> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
> index e43330ad659b..f5ec749b8fea 100644
> --- a/include/hw/arm/virt-acpi-build.h
> +++ b/include/hw/arm/virt-acpi-build.h
> @@ -33,6 +33,7 @@ typedef struct VirtGuestInfo {
>      const int *irqmap;
>      bool use_highmem;
>      int gic_version;
> +    bool no_its;
>  } VirtGuestInfo;
>  
>  
> 
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

Re: [Qemu-devel] [PATCH 2/2] hw/arm/virt: no ITS on older machine types

[Peter Maydell]

On 10 October 2016 at 17:35, Andrew Jones <drjones@redhat.com> wrote:
> We should avoid exposing new hardware (through DT and ACPI) on older
> machine types. This patch keeps 2.7 and older from changing, despite
> the introduction of ITS support for 2.8.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>
> As Eduardo pointed out long ago for a different reason, we should
> probably replace VirtGuestInfo with direct use of VirtMachineClass,
> like x86 has done to replace PcGuestInfo with direct use of
> PCMachineClass. I'll work on that, but wanted to get this ITS
> fixup in sooner than later, so I'm posting this patch now, which
> requires 'no_its' to be duplicated.

So this patch added a no_its flag which gets set for virt-2.7
and earlier, but there's no user-facing way to say "I'd like
a virt-2.8 board with no ITS", right? That's a bit unfortunate
because the ITS can't be migrated, which means there's no way
to ask for a post-2.7 virt board which can be migrated...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] hw/arm/virt: no ITS on older machine types

[Auger Eric]

Hi Peter,

On 20/01/2017 16:52, Peter Maydell wrote:
> On 10 October 2016 at 17:35, Andrew Jones <drjones@redhat.com> wrote:
>> We should avoid exposing new hardware (through DT and ACPI) on older
>> machine types. This patch keeps 2.7 and older from changing, despite
>> the introduction of ITS support for 2.8.
>>
>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>> ---
>>
>> As Eduardo pointed out long ago for a different reason, we should
>> probably replace VirtGuestInfo with direct use of VirtMachineClass,
>> like x86 has done to replace PcGuestInfo with direct use of
>> PCMachineClass. I'll work on that, but wanted to get this ITS
>> fixup in sooner than later, so I'm posting this patch now, which
>> requires 'no_its' to be duplicated.
> 
> So this patch added a no_its flag which gets set for virt-2.7
> and earlier, but there's no user-facing way to say "I'd like
> a virt-2.8 board with no ITS", right? That's a bit unfortunate
> because the ITS can't be migrated, which means there's no way
> to ask for a post-2.7 virt board which can be migrated...

Sorry for the delay.

For 2.9 machine I can add a new property that would allow the user to
deselect the ITS. Would that make sense?

Thanks

Eric


> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH 2/2] hw/arm/virt: no ITS on older machine types

[Peter Maydell]

On 26 January 2017 at 07:58, Auger Eric <eric.auger@redhat.com> wrote:
> On 20/01/2017 16:52, Peter Maydell wrote:
>> So this patch added a no_its flag which gets set for virt-2.7
>> and earlier, but there's no user-facing way to say "I'd like
>> a virt-2.8 board with no ITS", right? That's a bit unfortunate
>> because the ITS can't be migrated, which means there's no way
>> to ask for a post-2.7 virt board which can be migrated...
>
> Sorry for the delay.
>
> For 2.9 machine I can add a new property that would allow the user to
> deselect the ITS. Would that make sense?

Maybe, but Christoffer pointed out to me that we can't do
migration of the GICv3 yet anyway, so I was getting a bit
ahead of myself -- non-migratable ITS doesn't break something
that worked previously. So perhaps we're OK the way we are?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] hw/arm/virt: no ITS on older machine types

[Auger Eric]

Hi Peter,

On 26/01/2017 11:41, Peter Maydell wrote:
> On 26 January 2017 at 07:58, Auger Eric <eric.auger@redhat.com> wrote:
>> On 20/01/2017 16:52, Peter Maydell wrote:
>>> So this patch added a no_its flag which gets set for virt-2.7
>>> and earlier, but there's no user-facing way to say "I'd like
>>> a virt-2.8 board with no ITS", right? That's a bit unfortunate
>>> because the ITS can't be migrated, which means there's no way
>>> to ask for a post-2.7 virt board which can be migrated...
>>
>> Sorry for the delay.
>>
>> For 2.9 machine I can add a new property that would allow the user to
>> deselect the ITS. Would that make sense?
> 
> Maybe, but Christoffer pointed out to me that we can't do
> migration of the GICv3 yet anyway, so I was getting a bit
> ahead of myself -- non-migratable ITS doesn't break something
> that worked previously. So perhaps we're OK the way we are?
Yes indeed GICv3 migration is not yet upstreamed although under way. For
2.9 your concern still is relevant if ITS migration functionality does
not get supported in 2.9 along with GICv3 one. I will pay attention to this.

Thanks

Eric
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH 0/2] couple ITS support fixups

[Peter Maydell]

On 10 October 2016 at 17:35, Andrew Jones <drjones@redhat.com> wrote:
>
> Andrew Jones (2):
>   hw/arm/virt-acpi-build: fix MADT generation
>   hw/arm/virt: no ITS on older machine types
>
>  hw/arm/virt-acpi-build.c         | 14 ++++++--------
>  hw/arm/virt.c                    | 15 +++++++++++----
>  include/hw/arm/virt-acpi-build.h |  1 +
>  3 files changed, 18 insertions(+), 12 deletions(-)



Applied to target-arm.next, thanks.

-- PMM