From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8317FCD3425 for ; Mon, 18 Sep 2023 23:44:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=prTccXWjsRZWCKoILbn+zcUoLSjQwW5DK9tQJGdBeA0=; b=ZJcEc4F+n0g8O5 /Y9204r0AkP6yJl3DCb2CviHNlF/kyyTE9LeZPO8cPujMDXoIrSlV1zt11TDYilh7n9/r12rVgIHm ZEBsdcv5pj+dszQMsIBTfNSUggJdVW2SValY4MNTiaax3AbVGGHei/voE68xcJSCCriKPI5bIQu8I vFf2sntDc6PCUX1+4sjjrWDVa64/cB/xw737TJWTnNRqDUziJs2VnO+XchtyfygdlBJT+WWqd1zJL b1OxhqasXyck5VkdMkGHEE70tr4JqG8r6QLPJi35Dxoxmf5n6tw5UKYlgQC2+x+T5D0kbDLFTiW9I mNVD2rF975h/t3eLbVnA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qiNuO-00GXPG-3B; Mon, 18 Sep 2023 23:44:05 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qiNuL-00GXOT-22 for linux-arm-kernel@lists.infradead.org; Mon, 18 Sep 2023 23:44:03 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695080640; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zVlOSfQ+Cs92BylKR+M/JB7hqrVK10vOpKWpaqimizU=; b=K79SaXLiw0fUjayVnixiCSp2Ah2B5WGitaXVoXCytPwTtRM7hfaw/ksUm95qTLSm/GvUnG fzN2P+fm7+zWe8FeG/tnOb0xatCihCx111J/4SIxVYG6/26dNEIazUk+kurdncX3/jBycD /pZBZ8SOQ5v7Ng6pITTVgPVV013eBqk= Received: from mail-pf1-f200.google.com (mail-pf1-f200.google.com [209.85.210.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-688-bIt6yeueMneUjUz5YPXR4w-1; Mon, 18 Sep 2023 19:43:57 -0400 X-MC-Unique: bIt6yeueMneUjUz5YPXR4w-1 Received: by mail-pf1-f200.google.com with SMTP id d2e1a72fcca58-68fbe41b9d3so4683263b3a.2 for ; Mon, 18 Sep 2023 16:43:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695080636; x=1695685436; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zVlOSfQ+Cs92BylKR+M/JB7hqrVK10vOpKWpaqimizU=; b=h5tW+DGHHKM3WJPsXVXr99uq4kK03gv3ft7gghBdGsls+FMgqpTdNcT2diKBM9N2zE ZTW8YjqA1rn2pYBYl7ptmo+FmZcLvDHi+dEQWgpDxoGoIcjrII+ZUgh4AjvrUEmhYo6Y cf3elOYYbifiiXX/iCwERx1VVnQIbSUm5vUEQgSjD0zSpV40t2NkrqjKc0ZZMtQv78ZP J6yO4njzwX5VbpO1x8xXCTpPXxOGHkIZ+7afmSHHHyGwJXDktVOMMJEWIJCg0xjsJYQs lk7obhiuQ1J7GaziLu9eNurqkz5Tq6UuqMtYXlyjr3fV4Dm1g0j6EIJFeUS2bgROHzJa qClQ== X-Gm-Message-State: AOJu0YxaFHaLx7Y2SjCj3uQg5xD4hSVPEhTGUVvfg+cgjKFn9MDNYD8E bKX1CEqtDNiOieTu+aVHkHM5SgSW+xsVANGQ9kCEK0PdVRvnaddlyZK+ljhvU7hfTMPvrJcjszn o10xNd9wbWc5+J8peHRYACX44MHjybJ1c9mg= X-Received: by 2002:a05:6a20:430d:b0:118:e70:6f7d with SMTP id h13-20020a056a20430d00b001180e706f7dmr10495445pzk.10.1695080635941; Mon, 18 Sep 2023 16:43:55 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHFKOy814XSQK8TdPjNzCWuHRcOodlWCLUpNrs1TYBainKT1BB0hWvhf0Gyqm0LB88Jzxx1dQ== X-Received: by 2002:a05:6a20:430d:b0:118:e70:6f7d with SMTP id h13-20020a056a20430d00b001180e706f7dmr10495432pzk.10.1695080635630; Mon, 18 Sep 2023 16:43:55 -0700 (PDT) Received: from ?IPV6:2001:8003:e5b0:9f00:dbbc:1945:6e65:ec5? ([2001:8003:e5b0:9f00:dbbc:1945:6e65:ec5]) by smtp.gmail.com with ESMTPSA id jo13-20020a170903054d00b001bc68602e54sm8823472plb.142.2023.09.18.16.43.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 Sep 2023 16:43:54 -0700 (PDT) Message-ID: Date: Tue, 19 Sep 2023 09:43:46 +1000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [RFC PATCH v2 14/35] ACPI: Only enumerate enabled (or functional) devices To: James Morse , linux-pm@vger.kernel.org, loongarch@lists.linux.dev, linux-acpi@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-riscv@lists.infradead.org, kvmarm@lists.linux.dev Cc: x86@kernel.org, Salil Mehta , Russell King , Jean-Philippe Brucker , jianyong.wu@arm.com, justin.he@arm.com References: <20230913163823.7880-1-james.morse@arm.com> <20230913163823.7880-15-james.morse@arm.com> From: Gavin Shan In-Reply-To: <20230913163823.7880-15-james.morse@arm.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230918_164401_740882_C05A021E X-CRM114-Status: GOOD ( 34.02 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 9/14/23 02:38, James Morse wrote: > Today the ACPI enumeration code 'visits' all devices that are present. > > This is a problem for arm64, where CPUs are always present, but not > always enabled. When a device-check occurs because the firmware-policy > has changed and a CPU is now enabled, the following error occurs: > | acpi ACPI0007:48: Enumeration failure > > This is ultimately because acpi_dev_ready_for_enumeration() returns > true for a device that is not enabled. The ACPI Processor driver > will not register such CPUs as they are not 'decoding their resources'. > > Change acpi_dev_ready_for_enumeration() to also check the enabled bit. > ACPI allows a device to be functional instead of maintaining the > present and enabled bit. Make this behaviour an explicit check with > a reference to the spec, and then check the present and enabled bits. > This is needed to avoid enumerating present && functional devices that > are not enabled. > > Signed-off-by: James Morse > --- > If this change causes problems on deployed hardware, I suggest an > arch opt-in: ACPI_IGNORE_STA_ENABLED, that causes > acpi_dev_ready_for_enumeration() to only check the present bit. > --- > drivers/acpi/device_pm.c | 2 +- > drivers/acpi/device_sysfs.c | 2 +- > drivers/acpi/internal.h | 1 - > drivers/acpi/property.c | 2 +- > drivers/acpi/scan.c | 23 +++++++++++++---------- > 5 files changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c > index f007116a8427..76c38478a502 100644 > --- a/drivers/acpi/device_pm.c > +++ b/drivers/acpi/device_pm.c > @@ -313,7 +313,7 @@ int acpi_bus_init_power(struct acpi_device *device) > return -EINVAL; > > device->power.state = ACPI_STATE_UNKNOWN; > - if (!acpi_device_is_present(device)) { > + if (!acpi_dev_ready_for_enumeration(device)) { > device->flags.initialized = false; > return -ENXIO; > } > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c > index b9bbf0746199..16e586d74aa2 100644 > --- a/drivers/acpi/device_sysfs.c > +++ b/drivers/acpi/device_sysfs.c > @@ -141,7 +141,7 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modalia > struct acpi_hardware_id *id; > > /* Avoid unnecessarily loading modules for non present devices. */ > - if (!acpi_device_is_present(acpi_dev)) > + if (!acpi_dev_ready_for_enumeration(acpi_dev)) > return 0; > > /* > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index 866c7c4ed233..a1b45e345bcc 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -107,7 +107,6 @@ int acpi_device_setup_files(struct acpi_device *dev); > void acpi_device_remove_files(struct acpi_device *dev); > void acpi_device_add_finalize(struct acpi_device *device); > void acpi_free_pnp_ids(struct acpi_device_pnp *pnp); > -bool acpi_device_is_present(const struct acpi_device *adev); > bool acpi_device_is_battery(struct acpi_device *adev); > bool acpi_device_is_first_physical_node(struct acpi_device *adev, > const struct device *dev); > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index 413e4fcadcaf..e03f00b98701 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -1418,7 +1418,7 @@ static bool acpi_fwnode_device_is_available(const struct fwnode_handle *fwnode) > if (!is_acpi_device_node(fwnode)) > return false; > > - return acpi_device_is_present(to_acpi_device_node(fwnode)); > + return acpi_dev_ready_for_enumeration(to_acpi_device_node(fwnode)); > } > > static const void * > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index 17ab875a7d4e..f898591ce05f 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -304,7 +304,7 @@ static int acpi_scan_device_check(struct acpi_device *adev) > int error; > > acpi_bus_get_status(adev); > - if (acpi_device_is_present(adev)) { > + if (acpi_dev_ready_for_enumeration(adev)) { > /* > * This function is only called for device objects for which > * matching scan handlers exist. The only situation in which > @@ -338,7 +338,7 @@ static int acpi_scan_bus_check(struct acpi_device *adev, void *not_used) > int error; > > acpi_bus_get_status(adev); > - if (!acpi_device_is_present(adev)) { > + if (!acpi_dev_ready_for_enumeration(adev)) { > acpi_scan_device_not_enumerated(adev); > return 0; > } > @@ -1908,11 +1908,6 @@ static bool acpi_device_should_be_hidden(acpi_handle handle) > return true; > } > > -bool acpi_device_is_present(const struct acpi_device *adev) > -{ > - return adev->status.present || adev->status.functional; > -} > - > static bool acpi_scan_handler_matching(struct acpi_scan_handler *handler, > const char *idstr, > const struct acpi_device_id **matchid) > @@ -2375,16 +2370,24 @@ EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies); > * acpi_dev_ready_for_enumeration - Check if the ACPI device is ready for enumeration > * @device: Pointer to the &struct acpi_device to check > * > - * Check if the device is present and has no unmet dependencies. > + * Check if the device is functional or enabled and has no unmet dependencies. > * > - * Return true if the device is ready for enumeratino. Otherwise, return false. > + * Return true if the device is ready for enumeration. Otherwise, return false. > */ > bool acpi_dev_ready_for_enumeration(const struct acpi_device *device) > { > if (device->flags.honor_deps && device->dep_unmet) > return false; > > - return acpi_device_is_present(device); > + /* > + * ACPI 6.5's 6.3.7 "_STA (Device Status)" allows firmware to return > + * (!present && functional) for certain types of devices that should be > + * enumerated. > + */ > + if (!device->status.present && !device->status.enabled) > + return device->status.functional; > + > + return device->status.present && device->status.enabled; > } > EXPORT_SYMBOL_GPL(acpi_dev_ready_for_enumeration); > Looking at Salil's latest branch (vcpu-hotplug-RFCv2-rc7), there are 3 possible statuses: 0x0 when CPU isn't present 0xD when CPU is present, but not enabled 0xF when CPU is present and enabled Previously, the ACPI device is enumerated on 0xD and 0xF. We want to avoid the enumeration on 0xD since the processor isn't ready for enumeration in this specific case. The changed check (device->status.present && device->status.enabled) can ensure it. So the addition of checking @device->state.functional seems irrelevant to ARM64 vCPU hot-add? I guess we probably want a relaxation after the condition (device->status.present || device->status.enabled) becomes a more strict one (device->status.present && device->status.enabled) Thanks, Gavin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel