From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42570) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4XJH-0001t1-6A for qemu-devel@nongnu.org; Mon, 15 Jun 2015 12:32:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4XJD-0001IR-0r for qemu-devel@nongnu.org; Mon, 15 Jun 2015 12:32:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32873) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4XJC-0001I3-Qf for qemu-devel@nongnu.org; Mon, 15 Jun 2015 12:32:26 -0400 Date: Mon, 15 Jun 2015 18:32:21 +0200 From: Andrew Jones Message-ID: <20150615163221.GA30395@hawk.localdomain> References: <1433929959-29530-1-git-send-email-drjones@redhat.com> <1433929959-29530-3-git-send-email-drjones@redhat.com> <20150615180904-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150615180904-mutt-send-email-mst@redhat.com> 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: "Michael S. Tsirkin" Cc: Peter Maydell , Shannon Zhao , QEMU Developers , Igor Mammedov On Mon, Jun 15, 2015 at 06:10:25PM +0200, Michael S. Tsirkin wrote: > 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. > Everything used in the table is either a single byte, or I used le32, Well, I didn't bother for the pci_{device,vendor}_id assignments, as they're 0xffff anyway. I can change those two to make them more explicit, if that's preferred. Thanks, drew