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: 64775 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp104051lbc; Thu, 31 Mar 2016 04:44:12 -0700 (PDT) X-Received: by 10.98.15.135 with SMTP id 7mr21186479pfp.142.1459424652199; Thu, 31 Mar 2016 04:44:12 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id y15si450246pfi.161.2016.03.31.04.44.11; Thu, 31 Mar 2016 04:44:12 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of devicetree-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of devicetree-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=devicetree-owner@vger.kernel.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752206AbcCaLoK (ORCPT + 7 others); Thu, 31 Mar 2016 07:44:10 -0400 Received: from mail-ig0-f171.google.com ([209.85.213.171]:32947 "EHLO mail-ig0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751771AbcCaLoJ (ORCPT ); Thu, 31 Mar 2016 07:44:09 -0400 Received: by mail-ig0-f171.google.com with SMTP id ma7so91178631igc.0 for ; Thu, 31 Mar 2016 04:44:08 -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=fVyB6yo88EDJCOJb4E3N3B6wXbNtnTYM6ykrqcioC3mbXEjg7+/669pxWWt1hBVFkJ JhfjvelwbzImsJrkgEq70CM5U2uxrUhMeLZBgi2qLNFVezR/dKTWesUzkTcq/n9ivAxO +nGOWJNc8f9L4LbDaFpqgwgK3ZP/z5fAmTeFsZ0p6Z9ZPnA5kvWYL7YQfUdri1N+mQgn 5giO78l3RWeceEvNdssUmDBAmD900MPugerg0kRHTxGtmMbrPeAbnCqlhQxeaivkJPlf albqMZsuQLrYwjdrDodI/j1yDj/LrrfznH6a5SOBBvXtHUnB9pPOHm9SwjYDRDIAzr83 g9JA== X-Gm-Message-State: AD7BkJKGHhsypCY2WCRd7kQzk4dZJ3/XiV3luNHu7lQyUrU2whJ/sNK7mEW1ncZqLLlFe6QKFJyroIyZRg+Tkc5k 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: Subject: Re: [PATCH v7 12/17] ARM64: ACPI: Check if it runs on Xen to enable or disable ACPI From: Ard Biesheuvel To: Stefano Stabellini Cc: Shannon Zhao , 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 , "xen-devel@lists.xen.org" , "linux-arm-kernel@lists.infradead.org" , David Vrabel Sender: devicetree-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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; }