From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH RFC 30/35] arm : acpi map XSDT table to dom0 Date: Thu, 05 Feb 2015 04:46:50 +0000 Message-ID: <54D2F5BA.4010507@linaro.org> References: <1423058539-26403-1-git-send-email-parth.dixit@linaro.org> <1423058539-26403-31-git-send-email-parth.dixit@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1423058539-26403-31-git-send-email-parth.dixit@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: parth.dixit@linaro.org, xen-devel@lists.xen.org Cc: christoffer.dall@linaro.org, tim@xen.org, stefano.stabellini@citrix.com, ian.campbell@citrix.com, jbeulich@suse.com List-Id: xen-devel@lists.xenproject.org Hi Parth, On 04/02/2015 14:02, parth.dixit@linaro.org wrote: > From: Parth Dixit > > XSDT table cannot be passed as is to dom0 because new tables specific to xen > need to be added to its table entries > > Signed-off-by: Parth Dixit > --- > xen/arch/arm/arm64/acpi/arm-core.c | 65 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 63 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/arm64/acpi/arm-core.c b/xen/arch/arm/arm64/acpi/arm-core.c > index 9a26202..a210596 100644 > --- a/xen/arch/arm/arm64/acpi/arm-core.c > +++ b/xen/arch/arm/arm64/acpi/arm-core.c > @@ -256,13 +256,74 @@ static void set_psci_fadt(void) > fadt->header.checksum = (u8)( fadt->header.checksum-checksum ); > } > > +#define NR_NEW_XEN_TABLES 2 Please add a comment to specify what is the 2 new tables. > + > +static int map_xsdt_table(struct domain *d, u64 *xsdt) > +{ > + int res; > + u64 size; > + u64 addr = *xsdt; > + u64 *new_xsdt; > + struct acpi_table_header *table; > + u64 *table_entry; > + u8 checksum; > + > + /* map xsdt table */ > + table = acpi_os_map_memory(addr, sizeof(struct acpi_table_header) ); No space before the last ) Also acpi_os_map_memory can fail. > + size = table->length ; No space before the ; > + acpi_os_unmap_memory(table, sizeof(struct acpi_table_header) ); Ditto > + > + table = acpi_os_map_memory(addr, size); acpi_os_map_memory can fail > + size += ( NR_NEW_XEN_TABLES*sizeof(acpi_native_uint) ); (NR_NEW_XEN_TABLELS * sizeof(...)); > + new_xsdt = ACPI_ALLOCATE_ZEROED(size); > + if( new_xsdt == NULL) if ( ... ) > + return -ENOMEM; > + ACPI_MEMCPY(new_xsdt, table,table->length); Missing space after the comma. > + acpi_os_unmap_memory(table, table->length); > + > + table = (struct acpi_table_header *) new_xsdt; > + table->length = size; > + *xsdt = addr = virt_to_maddr(new_xsdt); > + > + /* map xsdt region */ > + res = map_ram_regions(d, > + paddr_to_pfn(addr & PAGE_MASK), > + DIV_ROUND_UP(size, PAGE_SIZE), > + paddr_to_pfn(addr & PAGE_MASK)); > + if ( res ) > + { > + printk(XENLOG_ERR "Unable to map 0x%"PRIx64 > + " - 0x%"PRIx64" in domain \n", > + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1); > + return res; > + } > + > + table_entry = ACPI_CAST_PTR(u64, > + ( ACPI_CAST_PTR(u8, new_xsdt) + sizeof(struct acpi_table_header) ) ); Too much space in general. > + table_entry += > + ( ( (size - sizeof(struct acpi_table_header) ) / > + sizeof(acpi_native_uint) ) ); Ditto > + > + checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, table), table->length); > + table->checksum = (u8)( table->checksum - checksum ); (table->checksum - checksum); Also I'm not sure the the u8 cast is useful. > + return 0; > +} > + > int acpi_map_tables(struct domain *d) > { > int i,res; > - u64 addr, size; > + u64 addr, size, rsdp; > + struct acpi_table_rsdp *rsdt; > + > + addr = rsdp = acpi_os_get_root_pointer(); Why do you set addr, but only use below rather than rsdp? > + rsdt = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp) ); No space before ) > + addr = rsdt->xsdt_physical_address; > + map_xsdt_table(d, &addr); > + rsdt->xsdt_physical_address = addr; > + acpi_os_unmap_memory(rsdt, sizeof(struct acpi_table_rsdp) ); No space before ) Regards, -- Julien Grall