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

[Andrew Jones]

On Fri, Jun 12, 2015 at 06:21:35PM +0200, Igor Mammedov wrote:
> On Fri, 12 Jun 2015 15:55:22 +0100
> Peter Maydell <address@hidden> wrote:
> 
> > On 1 June 2015 at 11:15,  <address@hidden> wrote:
> > > From: Shannon Zhao <address@hidden>
> > >
> > > Add GICv2m description in ACPI MADT table, so guest can use MSI when
> > > booting with ACPI.
> > >
> > > Signed-off-by: Shannon Zhao <address@hidden>
> > > Signed-off-by: Shannon Zhao <address@hidden>
> > > ---
> > >     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? 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.
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.

[*] https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg03899.html

Thanks,
drew