From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH RFC 32/35] arm : acpi map xen environment table to dom0 Date: Thu, 05 Feb 2015 05:29:55 +0000 Message-ID: <54D2FFD3.10302@linaro.org> References: <1423058539-26403-1-git-send-email-parth.dixit@linaro.org> <1423058539-26403-33-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-33-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, As for the STAO table, this is not only mapping but also creating the table. On 04/02/2015 14:02, parth.dixit@linaro.org wrote: > From: Parth Dixit > > xen environment table contains the grant table address,size and event , size > channel interrupt information required by dom0. > > Signed-off-by: Parth Dixit > --- > xen/arch/arm/arm64/acpi/arm-core.c | 52 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 51 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/arm64/acpi/arm-core.c b/xen/arch/arm/arm64/acpi/arm-core.c > index 9fd02f9..9e9285c 100644 > --- a/xen/arch/arm/arm64/acpi/arm-core.c > +++ b/xen/arch/arm/arm64/acpi/arm-core.c > @@ -33,7 +33,6 @@ > #include > #include > #include > - Spurious change. > /* > * We never plan to use RSDT on arm/arm64 as its deprecated in spec but this > * variable is still required by the ACPI core > @@ -297,6 +296,49 @@ static int map_stao_table(struct domain *d, u64 *mstao) > return 0; > } > > +static int map_xenv_table(struct domain *d, u64 *mxenv) > +{ > + u64 addr; > + u64 size; > + int res; > + u8 checksum; > + struct acpi_table_xenv *xenv=NULL; *xenv = NULL > + > + xenv = ACPI_ALLOCATE_ZEROED( sizeof(struct acpi_table_xenv) ); No space necessary after the first ( and before the last ) > + if( xenv == NULL ) > + return -ENOMEM; > + > + ACPI_MEMCPY(xenv->header.signature, ACPI_SIG_XENV, 4); > + xenv->header.length = sizeof(struct acpi_table_header)+12; Where does the 12 comes from? > + xenv->header.checksum = 0; > + ACPI_MEMCPY(xenv->header.oem_id, "XenVMM", 6); > + ACPI_MEMCPY(xenv->header.oem_table_id, "RTSMVEV8", 8); RTSMVEV8? Why? > + xenv->header.revision = 1; > + ACPI_MEMCPY(xenv->header.asl_compiler_id, "INTL", 4); > + xenv->header.asl_compiler_revision = 0x20140828; Why this compiler revision? > + xenv->gnt_start = 0x00000010000000; > + xenv->gnt_size = 0x20000; This is hardcoded. Even if you precise it in the cover letter, you should make clear in the patch that we have hardcoded. > + xenv->evt_intr = 31; > + xenv->evt_intr_flag =3; Ditto Also intr_flag = 3; and please use define rather than a value. > + size = sizeof(struct acpi_table_xenv); > + checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, xenv), size); > + xenv->header.checksum = (u8)( xenv->header.checksum - checksum ); No space after the ( and before ) > + *mxenv = addr = virt_to_maddr(xenv); > + > + res = map_ram_regions(d, > + paddr_to_pfn(addr & PAGE_MASK), > + DIV_ROUND_UP(size, PAGE_SIZE), > + paddr_to_pfn(addr & PAGE_MASK)); Same remark as for the Xen Environment table (see patch #31). > + 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; > + } > + > + return 0; > +} > > #define NR_NEW_XEN_TABLES 2 > > @@ -346,6 +388,14 @@ static int map_xsdt_table(struct domain *d, u64 *xsdt) > ( ( (size - sizeof(struct acpi_table_header) ) / > sizeof(acpi_native_uint) ) ); > > + /* map xen env table */ > + res = map_xenv_table(d, &addr); > + if(res) > + return res; > + > + table_entry--; > + *table_entry = addr ; > + No space before ; > /* map stao table */ > map_stao_table(d, &addr); > if(res) > Regards, -- Julien Grall