From patchwork Thu Mar 31 11:44:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 8711871 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C8EE3C0553 for ; Thu, 31 Mar 2016 11:46:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C17D7201FA for ; Thu, 31 Mar 2016 11:46:36 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [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 BD6872026F for ; Thu, 31 Mar 2016 11:46:35 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1alb1H-00022f-Om; Thu, 31 Mar 2016 11:44:11 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1alb1G-00022Q-L4 for xen-devel@lists.xen.org; Thu, 31 Mar 2016 11:44:10 +0000 Received: from [85.158.139.211] by server-7.bemta-5.messagelabs.com id 54/DC-22167-98D0DF65; Thu, 31 Mar 2016 11:44:09 +0000 X-Env-Sender: ard.biesheuvel@linaro.org X-Msg-Ref: server-2.tower-206.messagelabs.com!1459424648!16189472!1 X-Originating-IP: [209.85.213.172] X-SpamReason: No, hits=0.3 required=7.0 tests=RCVD_BY_IP X-StarScan-Received: X-StarScan-Version: 8.11; banners=-,-,- X-VirusChecked: Checked Received: (qmail 50756 invoked from network); 31 Mar 2016 11:44:09 -0000 Received: from mail-ig0-f172.google.com (HELO mail-ig0-f172.google.com) (209.85.213.172) by server-2.tower-206.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 31 Mar 2016 11:44:09 -0000 Received: by mail-ig0-f172.google.com with SMTP id sy18so59568759igc.1 for ; Thu, 31 Mar 2016 04:44:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=Ur55xJg/fA3gKI3feeZmBv+1iu5D9ntrxy1p48EccyA=; b=PMORZmp8fCrFuf98TuUxrJiZQoZ+NpErM2pKU8h0QzOcsmwLf21rSkXwhOIXZup5bo QcqyIU9m7IqHxGB0pxDHngSaJ21wANxUIYrnZucZooViMQQKK9o/3BPxb/WHytokEb+0 k63yL6xpb2D8sxv9A8CKl4vRezrQoce0pv/tA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=Ur55xJg/fA3gKI3feeZmBv+1iu5D9ntrxy1p48EccyA=; b=PFAyzbLljVzVi567GakSaoFcul+BjLdQRN/cUcxBgyJNIWMfDt/Ddzvl60kbcab9PX h45A7cln5dPCFPMes/2L7nUf2FL198IAOZuP2xSDlo/E5vsRY8epgLb+iSyFgGTVfRwc dTNcafdC4XNKz8yLlOWVxNMtF257mzyJeFya6giFW0C2sYWEG39qkGjz1YZnfp8NYXhg 7zDvaKeICNXicDzFATmBQvWdVRf0xhRhTsywZfoAajl9vVmcUAFwiLOcDT56IASKkAyw r6K8LisCU7urgNsc0gR+/cJasObW8pDzj1goxPobzbsNoGiWT/fYw+d7tffp1M65E//h 0fHw== X-Gm-Message-State: AD7BkJJG9RRxY5jHxpVi70REDvH1ggZcYbE48c+RN+vuvjAbhN3MgIDYRyi3kMwMOUCaG0ex79AAGs3EH89l7IU1 MIME-Version: 1.0 X-Received: by 10.50.79.200 with SMTP id l8mr1497528igx.75.1459424648238; Thu, 31 Mar 2016 04:44:08 -0700 (PDT) Received: by 10.36.29.6 with HTTP; Thu, 31 Mar 2016 04:44:08 -0700 (PDT) In-Reply-To: References: <1458830676-27075-1-git-send-email-shannon.zhao@linaro.org> <1458830676-27075-13-git-send-email-shannon.zhao@linaro.org> <20160329161837.GH6745@arm.com> <20160329163147.GB27223@leverpostej> <56FB7E00.7030400@huawei.com> Date: Thu, 31 Mar 2016 13:44:08 +0200 Message-ID: From: Ard Biesheuvel To: Stefano Stabellini Cc: Mark Rutland , "devicetree@vger.kernel.org" , "linux-efi@vger.kernel.org" , Catalin Marinas , Will Deacon , "linux-kernel@vger.kernel.org" , "Huangpeng \(Peter\)" , julien.grall@arm.com, Stefano Stabellini , Shannon Zhao , Shannon Zhao , "xen-devel@lists.xen.org" , "linux-arm-kernel@lists.infradead.org" , David Vrabel Subject: Re: [Xen-devel] [PATCH v7 12/17] ARM64: ACPI: Check if it runs on Xen to enable or disable ACPI 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=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable 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 31 March 2016 at 13:04, Stefano Stabellini wrote: > On Wed, 30 Mar 2016, Shannon Zhao wrote: >> Hi Will, Mark, >> >> On 2016/3/30 0:31, Mark Rutland wrote: >> > On Tue, Mar 29, 2016 at 05:18:38PM +0100, Will Deacon wrote: >> >> > On Thu, Mar 24, 2016 at 10:44:31PM +0800, Shannon Zhao wrote: >> >>> > > When it's a Xen domain0 booting with ACPI, it will supply a /chosen and >> >>> > > a /hypervisor node in DT. So check if it needs to enable ACPI. >> >>> > > >> >>> > > Signed-off-by: Shannon Zhao >> >>> > > Reviewed-by: Stefano Stabellini >> >>> > > Acked-by: Hanjun Guo >> >>> > > --- >> >>> > > arch/arm64/kernel/acpi.c | 12 ++++++++---- >> >>> > > 1 file changed, 8 insertions(+), 4 deletions(-) >> >>> > > >> >>> > > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c >> >>> > > index d1ce8e2..4e92be0 100644 >> >>> > > --- a/arch/arm64/kernel/acpi.c >> >>> > > +++ b/arch/arm64/kernel/acpi.c >> >>> > > @@ -67,10 +67,13 @@ static int __init dt_scan_depth1_nodes(unsigned long node, >> >>> > > { >> >>> > > /* >> >>> > > * Return 1 as soon as we encounter a node at depth 1 that is >> >>> > > - * not the /chosen node. >> >>> > > + * not the /chosen node, or /hypervisor node when running on Xen. >> >>> > > */ >> >>> > > - if (depth == 1 && (strcmp(uname, "chosen") != 0)) >> >>> > > - return 1; >> >>> > > + if (depth == 1 && (strcmp(uname, "chosen") != 0)) { >> >>> > > + if (!xen_initial_domain() || (strcmp(uname, "hypervisor") != 0)) >> >>> > > + return 1; >> >>> > > + } >> >> > >> >> > Hmm, but xen_initial_domain() is false when xen isn't being used at all, >> >> > so it feels to me like this is a bit too far-reaching and is basically >> >> > claiming the "/hypervisor" namespace for Xen. Couldn't it be renamed to >> >> > "xen,hypervisor" or something? >> >> > >> >> > Mark, got any thoughts on this? >> > The node has a compatible string, "xen,xen" per [1], which would tell us >> > absolutely that xen is present. I'd be happy checking for that >> > explicitly. >> > >> I think actually the xen_initial_domain is the result of the >> fdt_find_hyper_node. If the compatible string "xen,xen" doesn't exist, >> the xen_initial_domain() will return false and whatever the current node >> is the above check will return 1 since the device tree is not empty. > > Right. > > xen_initial_domain implies both "xen,xen" and XENFEAT_dom0 (which is a > feature retrieved by making a XENVER_get_features hypercall, see > drivers/xen/features.c:xen_setup_features). > > So the following check: > > + if (!xen_initial_domain() || (strcmp(uname, "hypervisor") != 0)) > + return 1; > > means that even if it's xen_initial_domain(), return error unless the > node found is "hypervisor". In other words, even if > xen_initial_domain(), no other nodes are allowed except /chosen and > /hypervisor. > > This doesn't look far reaching to me, but yes, we could check explicitly > for the node to be compatible "xen,xen", in addition to be named > "hypervisor", even though the check is already done elsewhere > (arch/arm/xen/enlighten.c). > > But I would keep it as it is. > The heuristic is there to decide whether some DTB image contains a complete description of the platform, or only some data handed over by the bootloader. Arguably, a DT containing both /chosen and /hypervisor but nothing else can still not describe an actual platform, and whether we execute under Xen or not is completely irrelevant. So this should be sufficient imo >> > In patch 11 fdt_find_hyper_node checks the compatible string. We could >> > factor that out into a helper like is_xen_node(node) and use it here >> > too. >> > >> I don't think so because we already check the compatible string before >> and we could get the result simply via xen_initial_domain(). > > We could add a comment saying xen_initial_domain() implies "xen,xen" or > something. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index d1ce8e2f98b9..d6d61e2e4d49 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -67,9 +67,10 @@ static int __init dt_scan_depth1_nodes(unsigned long node, { /* * Return 1 as soon as we encounter a node at depth 1 that is - * not the /chosen node. + * not the /chosen node or the /hypervisor node. */ - if (depth == 1 && (strcmp(uname, "chosen") != 0)) + if (depth == 1 && strcmp(uname, "chosen") != 0 && + strcmp(uname, "hypervisor") != 0) return 1; return 0; }