From patchwork Thu Feb 25 14:06:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shannon Zhao X-Patchwork-Id: 8423621 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id C50059F314 for ; Thu, 25 Feb 2016 14:14:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EE02E202E9 for ; Thu, 25 Feb 2016 14:14:11 +0000 (UTC) Received: from lists.xen.org (unknown [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0363C202B8 for ; Thu, 25 Feb 2016 14:14:11 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1aYwdI-0005CV-HG; Thu, 25 Feb 2016 14:11:08 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1aYwdH-0005CP-AF for xen-devel@lists.xen.org; Thu, 25 Feb 2016 14:11:07 +0000 Received: from [85.158.139.211] by server-11.bemta-5.messagelabs.com id 70/E4-02987-A7B0FC65; Thu, 25 Feb 2016 14:11:06 +0000 X-Env-Sender: zhaoshenglong@huawei.com X-Msg-Ref: server-7.tower-206.messagelabs.com!1456409461!24749148!1 X-Originating-IP: [119.145.14.66] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTE5LjE0NS4xNC42NiA9PiA4NTI3\n X-StarScan-Received: X-StarScan-Version: 7.35.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 9729 invoked from network); 25 Feb 2016 14:11:05 -0000 Received: from szxga03-in.huawei.com (HELO szxga03-in.huawei.com) (119.145.14.66) by server-7.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 25 Feb 2016 14:11:05 -0000 Received: from 172.24.1.48 (EHLO szxeml433-hub.china.huawei.com) ([172.24.1.48]) by szxrg03-dlp.huawei.com (MOS 4.4.3-GA FastPath queued) with ESMTP id BWP26867; Thu, 25 Feb 2016 22:06:48 +0800 (CST) Received: from [127.0.0.1] (10.177.16.142) by szxeml433-hub.china.huawei.com (10.82.67.210) with Microsoft SMTP Server id 14.3.235.1; Thu, 25 Feb 2016 22:06:41 +0800 Message-ID: <56CF0A70.8060203@huawei.com> Date: Thu, 25 Feb 2016 22:06:40 +0800 From: Shannon Zhao User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Stefano Stabellini References: <1453540813-15764-1-git-send-email-zhaoshenglong@huawei.com> <1453540813-15764-14-git-send-email-zhaoshenglong@huawei.com> <56CEBC2D.9090501@huawei.com> In-Reply-To: X-Originating-IP: [10.177.16.142] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090204.56CF0A88.0085, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 077bc96585db9882e299548d5a1f59d1 Cc: ian.campbell@citrix.com, peter.huangpeng@huawei.com, xen-devel@lists.xen.org, julien.grall@citrix.com, stefano.stabellini@citrix.com, shannon.zhao@linaro.org Subject: Re: [Xen-devel] [PATCH v4 13/21] arm/gic-v2: Add ACPI boot support for GICv2 X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,RDNS_NONE, UNPARSEABLE_RELAY autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 2016/2/25 18:42, Stefano Stabellini wrote: > On Thu, 25 Feb 2016, Shannon Zhao wrote: >> On 2016/1/28 20:56, Stefano Stabellini wrote: >>> On Sat, 23 Jan 2016, Shannon Zhao wrote: >>>> From: Parth Dixit >>>> >>>> ACPI on Xen hypervisor uses MADT table for proper GIC initialization. >>>> First get the GIC version from GIC Distributor. Then parse GIC related >>>> subtables, collect CPU interface and distributor addresses and call >>>> driver initialization function (which is hardware abstraction agnostic). >>>> In a similar way, FDT initialize GICv2. >>>> >>>> Signed-off-by: Parth Dixit >>>> Signed-off-by: Shannon Zhao >>>> >>>> V4: use container_of and fix coding style >>>> --- >>>> xen/arch/arm/gic-v2.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 119 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c >>>> index 668b863..7e46ee9 100644 >>>> --- a/xen/arch/arm/gic-v2.c >>>> +++ b/xen/arch/arm/gic-v2.c >>>> @@ -29,6 +29,8 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> @@ -36,6 +38,7 @@ >>>> >>>> #include >>>> #include >>>> +#include >>>> >>>> /* >>>> * LR register definitions are GIC v2 specific. >>>> @@ -681,11 +684,111 @@ static void __init gicv2_dt_init(void) >>>> csize, vsize); >>>> } >>>> >>>> +#ifdef CONFIG_ACPI >>>> +static int __init >>>> +gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, >>>> + const unsigned long end) >>>> +{ >>>> + static int cpu_base_assigned = 0; >>>> + struct acpi_madt_generic_interrupt *processor = >>>> + container_of(header, struct acpi_madt_generic_interrupt, header); >>>> + >>>> + if ( BAD_MADT_ENTRY(processor, end) ) >>>> + return -EINVAL; >>>> + >>>> + /* Read from APIC table and fill up the GIC variables */ >>>> + if ( cpu_base_assigned == 0 ) >>>> + { >>>> + cbase = processor->base_address; >>>> + csize = SZ_8K; >>>> + hbase = processor->gich_base_address; >>>> + vbase = processor->gicv_base_address; >>>> + gicv2_info.maintenance_irq = processor->vgic_interrupt; >>>> + >>>> + if( processor->flags & ACPI_MADT_VGIC_IRQ_MODE ) >>> >>> ^ if ( >>> >>> >>>> + irq_set_type(gicv2_info.maintenance_irq, IRQ_TYPE_EDGE_BOTH); >>>> + else >>>> + irq_set_type(gicv2_info.maintenance_irq, IRQ_TYPE_LEVEL_MASK); >>>> + >>>> + cpu_base_assigned = 1; >>>> + } >>>> + else >>>> + { >>>> + if ( cbase != processor->base_address >>>> + || hbase != processor->gich_base_address >>>> + || vbase != processor->gicv_base_address >>>> + || gicv2_info.maintenance_irq != processor->vgic_interrupt ) >>>> + { >>>> + printk("GICv2: GICC entries are not same in MADT table\n"); >>>> + return -EINVAL; >>>> + } >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int __init >>>> +gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, >>>> + const unsigned long end) >>>> +{ >>>> + struct acpi_madt_generic_distributor *dist = >>>> + container_of(header, struct acpi_madt_generic_distributor, header); >>>> + >>>> + if ( BAD_MADT_ENTRY(dist, end) ) >>>> + return -EINVAL; >>>> + >>>> + dbase = dist->base_address; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static void __init gicv2_acpi_init(void) >>>> +{ >>>> + acpi_status status; >>>> + struct acpi_table_header *table; >>>> + int count; >>>> + >>>> + status = acpi_get_table(ACPI_SIG_MADT, 0, &table); >>>> + >>>> + if ( ACPI_FAILURE(status) ) >>>> + { >>>> + const char *msg = acpi_format_exception(status); >>>> + >>>> + panic("GICv2: Failed to get MADT table, %s", msg); >>>> + } >>>> + >>>> + /* Collect CPU base addresses */ >>>> + count = acpi_parse_entries(ACPI_SIG_MADT, sizeof(struct acpi_table_madt), >>>> + gic_acpi_parse_madt_cpu, table, >>>> + ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0); >>>> + if ( count <= 0 ) >>>> + panic("GICv2: No valid GICC entries exists"); >>>> + >>>> + /* >>>> + * Find distributor base address. We expect one distributor entry since >>>> + * ACPI 5.0 spec neither support multi-GIC instances nor GIC cascade. >>>> + */ >>>> + count = acpi_parse_entries(ACPI_SIG_MADT, sizeof(struct acpi_table_madt), >>>> + gic_acpi_parse_madt_distributor, table, >>>> + ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0); >>>> + if ( count <= 0 ) >>>> + panic("GICv2: No valid GICD entries exists"); >>>> +} >>>> +#else >>>> +static void __init gicv2_acpi_init(void) >>>> +{ >>>> +/* Should never reach here */ >>>> +} >>>> +#endif >>> >>> With acpi_disabled becoming an #define constant, this #else should not >>> be needed. >>> >> If I remove #else and define acpi_disabled as a constant as you >> suggested, it has below compiling error on arm32. >> >> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c >> index 9a9fcd5..28ec064 100644 >> --- a/xen/arch/arm/gic-v2.c >> +++ b/xen/arch/arm/gic-v2.c >> @@ -776,11 +776,6 @@ static void __init acpi_gicv2_init(void) >> if ( count <= 0 ) >> panic("GICv2: No valid GICD entries exists"); >> } >> -#else >> -static void __init acpi_gicv2_init(void) >> -{ >> -/* Should never reach here */ >> -} >> #endif >> >> gic-v2.c: In function 'gicv2_init': >> gic-v2.c:793:9: error: implicit declaration of function >> 'acpi_gicv2_init' [-Werror=implicit-function-declaration] >> gic-v2.c:793:9: error: nested extern declaration of 'acpi_gicv2_init' >> [-Werror=nested-externs] >> cc1: all warnings being treated as errors > > Sorry, I should have been clearer. You still need to have a declaration > of the function outside the #ifdef. Something like the following should > work: > > static void acpi_gicv2_init(void); > > [...] > > #ifdef CONFIG_ACPI > static void __init acpi_gicv2_init(void) > { > /* do something */ > } > #endif > > [...] > > if ( acpi_disabled ) > gicv2_dt_init(); > else > gicv2_acpi_init(); > > . > I'm afraid it doesn't work. Still has below errors with the declaration: gic-v2.c:687:13: error: 'gicv2_acpi_init' used but never defined [-Werror] cc1: all warnings being treated as errors diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index 4b6c4c0..7b7e094 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -684,6 +684,8 @@ static void __init gicv2_dt_init(void) csize, vsize); } +static void gicv2_acpi_init(void); + #ifdef CONFIG_ACPI static int __init gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, @@ -774,11 +776,6 @@ static void __init gicv2_acpi_init(void) if ( count <= 0 ) panic("GICv2: No valid GICD entries exists"); } -#else -static void __init gicv2_acpi_init(void) -{ -/* Should never reach here */ -} #endif static int __init gicv2_init(void)