diff mbox

[RFC] hw/arm/virt-acpi-build: Add GICv2m description in ACPI MADT table

Message ID 1433153700-1336-1-git-send-email-shannon.zhao@linaro.org
State New
Headers show

Commit Message

Shannon Zhao June 1, 2015, 10:15 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Add GICv2m description in ACPI MADT table, so guest can use MSI when
booting with ACPI.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
    This is based on Christoffer's GICv2m patchset.
	Test virtio-net-pci, e1000.
---
 hw/arm/virt-acpi-build.c    | 11 +++++++++++
 include/hw/acpi/acpi-defs.h | 12 ++++++++++++
 2 files changed, 23 insertions(+)

Comments

Peter Maydell June 12, 2015, 2:55 p.m. UTC | #1
On 1 June 2015 at 11:15,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Add GICv2m description in ACPI MADT table, so guest can use MSI when
> booting with ACPI.
>
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>     This is based on Christoffer's GICv2m patchset.
>         Test virtio-net-pci, e1000.
> ---
>  hw/arm/virt-acpi-build.c    | 11 +++++++++++
>  include/hw/acpi/acpi-defs.h | 12 ++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index a9373cc..d8dcf45 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -385,8 +385,10 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
>  {
>      int madt_start = table_data->len;
>      const MemMapEntry *memmap = guest_info->memmap;
> +    const int *irqmap = guest_info->irqmap;
>      AcpiMultipleApicTable *madt;
>      AcpiMadtGenericDistributor *gicd;
> +    AcpiMadtGenericMsiFrame *gic_msi;
>      int i;
>
>      madt = acpi_data_push(table_data, sizeof *madt);
> @@ -410,6 +412,15 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
>      gicd->length = sizeof(*gicd);
>      gicd->base_address = memmap[VIRT_GIC_DIST].base;
>
> +    gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
> +    gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
> +    gic_msi->length = sizeof(*gic_msi);
> +    gic_msi->gic_msi_frame_id = 0;
> +    gic_msi->base_address = memmap[VIRT_GIC_V2M].base;
> +    gic_msi->flags = cpu_to_le32(1);
> +    gic_msi->spi_count = NUM_GICV2M_SPIS;
> +    gic_msi->spi_base = irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;

Why do we need to convert the flags to little-endian,
but not the length, base address or other fields?

thanks
-- PMM
Shannon Zhao June 16, 2015, 1:21 a.m. UTC | #2
Hi Andrew,

On 2015/6/16 2:09, Andrew Jones wrote:
> On Fri, Jun 12, 2015 at 06:21:35PM +0200, Igor Mammedov wrote:
>> On Fri, 12 Jun 2015 15:55:22 +0100
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>>> On 1 June 2015 at 11:15,  <shannon.zhao@linaro.org> wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Add GICv2m description in ACPI MADT table, so guest can use MSI when
>>>> booting with ACPI.
>>>>
>>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> ---
>>>>     This is based on Christoffer's GICv2m patchset.
>>>>         Test virtio-net-pci, e1000.
>>>> ---
>>>>  hw/arm/virt-acpi-build.c    | 11 +++++++++++
>>>>  include/hw/acpi/acpi-defs.h | 12 ++++++++++++
>>>>  2 files changed, 23 insertions(+)
>>>>
>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>>> index a9373cc..d8dcf45 100644
>>>> --- a/hw/arm/virt-acpi-build.c
>>>> +++ b/hw/arm/virt-acpi-build.c
>>>> @@ -385,8 +385,10 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
>>>>  {
>>>>      int madt_start = table_data->len;
>>>>      const MemMapEntry *memmap = guest_info->memmap;
>>>> +    const int *irqmap = guest_info->irqmap;
>>>>      AcpiMultipleApicTable *madt;
>>>>      AcpiMadtGenericDistributor *gicd;
>>>> +    AcpiMadtGenericMsiFrame *gic_msi;
>>>>      int i;
>>>>
>>>>      madt = acpi_data_push(table_data, sizeof *madt);
>>>> @@ -410,6 +412,15 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
>>>>      gicd->length = sizeof(*gicd);
>>>>      gicd->base_address = memmap[VIRT_GIC_DIST].base;
>>>>
>>>> +    gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
>>>> +    gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
>>>> +    gic_msi->length = sizeof(*gic_msi);
>>>> +    gic_msi->gic_msi_frame_id = 0;
>>>> +    gic_msi->base_address = memmap[VIRT_GIC_V2M].base;
>>>> +    gic_msi->flags = cpu_to_le32(1);
>>>> +    gic_msi->spi_count = NUM_GICV2M_SPIS;
>>>> +    gic_msi->spi_base = irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
>>>
>>> Why do we need to convert the flags to little-endian,
>>> but not the length, base address or other fields?
>> +1 all ACPI numbers should be converted to little-endian.
>>
> 
> Hi Shannon,
> 
> Are you going to send another version of this?

Yes, it's on my to-do list.

> I also see that
> build_gtdt and build_madt appear to be missing LE conversions
> (maybe other tables too, I didn't look too closely). So I
> think we need an LE conversion fix patch for all missing places
> in hw/arm/virt-acpi-build.c, as well as new version of this one.

Agree.

> Let me know if I can help out in any way. Maybe we can see what
> mst and Igor say about Peter's acpi_struct_assign macro suggestion[*]
> before fixing everything first.
> 
Thanks for your reminding.

> [*] https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg03899.html
> 
> Thanks,
> drew
>
diff mbox

Patch

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index a9373cc..d8dcf45 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -385,8 +385,10 @@  build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
 {
     int madt_start = table_data->len;
     const MemMapEntry *memmap = guest_info->memmap;
+    const int *irqmap = guest_info->irqmap;
     AcpiMultipleApicTable *madt;
     AcpiMadtGenericDistributor *gicd;
+    AcpiMadtGenericMsiFrame *gic_msi;
     int i;
 
     madt = acpi_data_push(table_data, sizeof *madt);
@@ -410,6 +412,15 @@  build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
     gicd->length = sizeof(*gicd);
     gicd->base_address = memmap[VIRT_GIC_DIST].base;
 
+    gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
+    gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
+    gic_msi->length = sizeof(*gic_msi);
+    gic_msi->gic_msi_frame_id = 0;
+    gic_msi->base_address = memmap[VIRT_GIC_V2M].base;
+    gic_msi->flags = cpu_to_le32(1);
+    gic_msi->spi_count = NUM_GICV2M_SPIS;
+    gic_msi->spi_base = irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE;
+
     build_header(linker, table_data,
                  (void *)(table_data->data + madt_start), "APIC",
                  table_data->len - madt_start, 5);
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index f503ec4..61c4314 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -340,6 +340,18 @@  struct AcpiMadtGenericDistributor {
 
 typedef struct AcpiMadtGenericDistributor AcpiMadtGenericDistributor;
 
+struct AcpiMadtGenericMsiFrame {
+    ACPI_SUB_HEADER_DEF
+    uint16_t reserved;
+    uint32_t gic_msi_frame_id;
+    uint64_t base_address;
+    uint32_t flags;
+    uint16_t spi_count;
+    uint16_t spi_base;
+} QEMU_PACKED;
+
+typedef struct AcpiMadtGenericMsiFrame AcpiMadtGenericMsiFrame;
+
 /*
  * Generic Timer Description Table (GTDT)
  */