From mboxrd@z Thu Jan 1 00:00:00 1970 From: julien.grall@citrix.com (Julien Grall) Date: Mon, 30 Nov 2015 15:25:34 +0000 Subject: [PATCH v3 49/62] arm/acpi: Map rest tables for Dom0 In-Reply-To: References: <1447753261-7552-1-git-send-email-shannon.zhao@linaro.org> <1447753261-7552-50-git-send-email-shannon.zhao@linaro.org> Message-ID: <565C6A6E.3000102@citrix.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 27/11/15 12:16, Stefano Stabellini wrote: > On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote: >> From: Shannon Zhao >> >> Map other reused tables for Dom0. > > "Map all other tables to Dom0 using 1:1 mappings." > > >> Signed-off-by: Shannon Zhao >> --- >> xen/arch/arm/domain_build.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 6ae5761..da4e271 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -1359,6 +1359,29 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) >> #ifdef CONFIG_ACPI >> #define XEN_HYPERVISOR_ID 0x000058656E564D4D /* "XenVMM" */ >> >> +static void acpi_map_rest_tables(struct domain *d) > > The name doesn't sound nice, "acpi_map_other_tables" would be better. > However this function is not actually mapping the other tables, it is > mapping *all* of them, including the original madt and fadt, right? I > think it would be best to avoid mapping the originals. > > >> +{ >> + int i; >> + unsigned long res; >> + u64 addr, size; > > Add a comment that they are being mapped 1:1 I don't like the fact that we begin to assume domain is mapped 1:1 in so many places. If we decide one day to drop the 1:1 mapping it would be more difficult. Can we try to rationalize the place where the 1:1 mapping is added? Some ASSERT(is_domain_direct_mapped(..)) would be useful too. Regards, -- Julien Grall From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 49/62] arm/acpi: Map rest tables for Dom0 Date: Mon, 30 Nov 2015 15:25:34 +0000 Message-ID: <565C6A6E.3000102@citrix.com> References: <1447753261-7552-1-git-send-email-shannon.zhao@linaro.org> <1447753261-7552-50-git-send-email-shannon.zhao@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini , shannon.zhao@linaro.org Cc: mark.rutland@arm.com, hangaohuai@huawei.com, david.vrabel@citrix.com, keir@xen.org, ian.campbell@citrix.com, ard.biesheuvel@linaro.org, andrew.cooper3@citrix.com, peter.huangpeng@huawei.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, christoffer.dall@linaro.org, jbeulich@suse.com, zhaoshenglong@huawei.com, linux-arm-kernel@lists.infradead.org, roger.pau@citrix.com List-Id: xen-devel@lists.xenproject.org Hi, On 27/11/15 12:16, Stefano Stabellini wrote: > On Tue, 17 Nov 2015, shannon.zhao@linaro.org wrote: >> From: Shannon Zhao >> >> Map other reused tables for Dom0. > > "Map all other tables to Dom0 using 1:1 mappings." > > >> Signed-off-by: Shannon Zhao >> --- >> xen/arch/arm/domain_build.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 6ae5761..da4e271 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -1359,6 +1359,29 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) >> #ifdef CONFIG_ACPI >> #define XEN_HYPERVISOR_ID 0x000058656E564D4D /* "XenVMM" */ >> >> +static void acpi_map_rest_tables(struct domain *d) > > The name doesn't sound nice, "acpi_map_other_tables" would be better. > However this function is not actually mapping the other tables, it is > mapping *all* of them, including the original madt and fadt, right? I > think it would be best to avoid mapping the originals. > > >> +{ >> + int i; >> + unsigned long res; >> + u64 addr, size; > > Add a comment that they are being mapped 1:1 I don't like the fact that we begin to assume domain is mapped 1:1 in so many places. If we decide one day to drop the 1:1 mapping it would be more difficult. Can we try to rationalize the place where the 1:1 mapping is added? Some ASSERT(is_domain_direct_mapped(..)) would be useful too. Regards, -- Julien Grall