From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58342) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4Wy0-0002DC-TC for qemu-devel@nongnu.org; Mon, 15 Jun 2015 12:10:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4Wxw-0005vo-Bi for qemu-devel@nongnu.org; Mon, 15 Jun 2015 12:10:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45927) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4Wxw-0005vg-5w for qemu-devel@nongnu.org; Mon, 15 Jun 2015 12:10:28 -0400 Date: Mon, 15 Jun 2015 18:10:25 +0200 From: "Michael S. Tsirkin" Message-ID: <20150615180904-mutt-send-email-mst@redhat.com> References: <1433929959-29530-1-git-send-email-drjones@redhat.com> <1433929959-29530-3-git-send-email-drjones@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v3 2/2] hw/arm/virt-acpi-build: Add SPCR table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Igor Mammedov , Andrew Jones , QEMU Developers , Shannon Zhao On Mon, Jun 15, 2015 at 04:45:58PM +0100, Peter Maydell wrote: > On 10 June 2015 at 10:52, Andrew Jones wrote: > > Signed-off-by: Andrew Jones > > Tested-by: Shannon Zhao > > --- > > hw/arm/virt-acpi-build.c | 43 ++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 42 insertions(+), 1 deletion(-) > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > index a9373ccaca6cb..d5a8b9c0178ea 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -84,6 +84,12 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, > > aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > > AML_EXCLUSIVE, uart_irq)); > > aml_append(dev, aml_name_decl("_CRS", crs)); > > + > > + /* The _ADR entry is used to link this device to the UART described > > + * in the SPCR table, i.e. SPCR.base_address.address == _ADR. > > + */ > > + aml_append(dev, aml_name_decl("_ADR", aml_int(uart_memmap->base))); > > + > > aml_append(scope, dev); > > } > > > > @@ -334,6 +340,38 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt) > > } > > > > static void > > +build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info) > > +{ > > + AcpiSerialPortConsoleRedirection *spcr; > > + const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART]; > > + int irq = guest_info->irqmap[VIRT_UART] + ARM_SPI_BASE; > > + > > + spcr = acpi_data_push(table_data, sizeof(*spcr)); > > + > > + spcr->interface_type = 0x3; /* ARM PL011 UART */ > > + > > + spcr->base_address.space_id = AML_SYSTEM_MEMORY; > > + spcr->base_address.bit_width = 8; > > + spcr->base_address.bit_offset = 0; > > + spcr->base_address.access_width = 1; > > + spcr->base_address.address = cpu_to_le64(uart_memmap->base); > > + > > + spcr->interrupt_types = (1 << 3); /* Bit[3] ARMH GIC interrupt */ > > + spcr->gsi = cpu_to_le32(irq); /* Global System Interrupt */ > > I'm still confused about when fields in these ACPI structs > need to be converted to little-endian, and when they don't. > Is there a rule-of-thumb I can use when I'm looking at patches? > > thanks > -- PMM Normally it's all LE unless it's a single byte value. Did not check this specific table. We really need to add sparse support to check endian-ness matches, or re-write it all using byte_add so there's no duplication of info. -- MST