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 lists.gnu.org (lists.gnu.org [209.51.188.17]) (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 DD704C71155 for ; Fri, 20 Jun 2025 09:20:52 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1uSXux-0004Us-DU; Fri, 20 Jun 2025 05:20:15 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uSXui-0004Re-Gi; Fri, 20 Jun 2025 05:20:07 -0400 Received: from [185.176.79.56] (helo=frasgout.his.huawei.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uSXuZ-0006ik-Pp; Fri, 20 Jun 2025 05:19:59 -0400 Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4bNsHw1dMjz6K5mL; Fri, 20 Jun 2025 17:14:52 +0800 (CST) Received: from frapeml500008.china.huawei.com (unknown [7.182.85.71]) by mail.maildlp.com (Postfix) with ESMTPS id 42246140446; Fri, 20 Jun 2025 17:19:38 +0800 (CST) Received: from localhost (10.203.177.66) by frapeml500008.china.huawei.com (7.182.85.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Fri, 20 Jun 2025 11:19:37 +0200 Date: Fri, 20 Jun 2025 10:19:36 +0100 To: Eric Auger CC: , , , , , , , , , , , Subject: Re: [PATCH v3 16/29] hw/i386/acpi-build: Move aml_pci_edsm to a generic place Message-ID: <20250620101936.00005f96@huawei.com> In-Reply-To: <20250616094903.885753-17-eric.auger@redhat.com> References: <20250616094903.885753-1-eric.auger@redhat.com> <20250616094903.885753-17-eric.auger@redhat.com> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.42; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.177.66] X-ClientProxiedBy: lhrpeml100009.china.huawei.com (7.191.174.83) To frapeml500008.china.huawei.com (7.182.85.71) X-Host-Lookup-Failed: Reverse DNS lookup failed for 185.176.79.56 (deferred) Received-SPF: pass client-ip=185.176.79.56; envelope-from=jonathan.cameron@huawei.com; helo=frasgout.his.huawei.com X-Spam_score_int: -33 X-Spam_score: -3.4 X-Spam_bar: --- X-Spam_report: (-3.4 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, RDNS_NONE=0.793, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-to: Jonathan Cameron From: Jonathan Cameron via Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On Mon, 16 Jun 2025 11:46:45 +0200 Eric Auger wrote: > Move aml_pci_edsm to pci-bridge.c since we want to reuse that for > ARM and acpi-index support. > > Signed-off-by: Eric Auger A request for a bit of documentation inline. aml_pci_edsm() sounds like we should be able to grep the spec for edsm and find it but that's just internal method naming in qemu. More interesting is I don't think this will ever be called as the kernel has no idea how to call it and unlike on x86 the blobs don't show wrapping the call in a _DSM() (see aml_pci_static_endpoint_dsm()) Did EDSM usage get dropped as this set evolved leaving this behind? > > --- > > v2 -> v3: > - move to pci-bridge.c instead of pcihp.c (Igor) > --- > include/hw/acpi/pci.h | 1 + > hw/acpi/pci-bridge.c | 54 +++++++++++++++++++++++++++++++++++++++++++ > hw/i386/acpi-build.c | 53 ------------------------------------------ > 3 files changed, 55 insertions(+), 53 deletions(-) > > diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h > index 69bae95eac..05e72815c8 100644 > --- a/include/hw/acpi/pci.h > +++ b/include/hw/acpi/pci.h > @@ -42,5 +42,6 @@ void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope); > void build_srat_generic_affinity_structures(GArray *table_data); > > Aml *build_pci_host_bridge_osc_method(bool enable_native_pcie_hotplug); > +Aml *aml_pci_edsm(void); > > #endif > diff --git a/hw/acpi/pci-bridge.c b/hw/acpi/pci-bridge.c > index 7baa7034a1..be68a98c34 100644 > --- a/hw/acpi/pci-bridge.c > +++ b/hw/acpi/pci-bridge.c > @@ -35,3 +35,57 @@ void build_pci_bridge_aml(AcpiDevAmlIf *adev, Aml *scope) > } > } > } > + > +Aml *aml_pci_edsm(void) Can we have some comments, or a more descriptive name than the resulting method name? There is stuff in the function obviously that associates it with the naming DSM but given this is moving to generic code maybe it needs a brief intro comment? > +{ > + Aml *method, *ifctx; > + Aml *zero = aml_int(0); > + Aml *func = aml_arg(2); > + Aml *ret = aml_local(0); > + Aml *aidx = aml_local(1); > + Aml *params = aml_arg(4); > + > + method = aml_method("EDSM", 5, AML_SERIALIZED); > + > + /* get supported functions */ > + ifctx = aml_if(aml_equal(func, zero)); > + { > + /* 1: have supported functions */ > + /* 7: support for function 7 */ > + const uint8_t caps = 1 | BIT(7); > + build_append_pci_dsm_func0_common(ifctx, ret); > + aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret, zero))); > + aml_append(ifctx, aml_return(ret)); > + } > + aml_append(method, ifctx); > + > + /* handle specific functions requests */ > + /* > + * PCI Firmware Specification 3.1 > + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under > + * Operating Systems > + */ > + ifctx = aml_if(aml_equal(func, aml_int(7))); > + { > + Aml *pkg = aml_package(2); > + aml_append(pkg, zero); > + /* optional, if not impl. should return null string */ > + aml_append(pkg, aml_string("%s", "")); > + aml_append(ifctx, aml_store(pkg, ret)); > + > + /* > + * IASL is fine when initializing Package with computational data, > + * however it makes guest unhappy /it fails to process such AML/. > + * So use runtime assignment to set acpi-index after initializer > + * to make OSPM happy. > + */ > + aml_append(ifctx, > + aml_store(aml_derefof(aml_index(params, aml_int(0))), aidx)); > + aml_append(ifctx, aml_store(aidx, aml_index(ret, zero))); > + aml_append(ifctx, aml_return(ret)); > + } > + aml_append(method, ifctx); > + > + return method; > +} > + > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index fe8bc62c03..6cf623392e 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -338,59 +338,6 @@ build_facs(GArray *table_data) > g_array_append_vals(table_data, reserved, 40); /* Reserved */ > } > > -static Aml *aml_pci_edsm(void) > -{ > - Aml *method, *ifctx; > - Aml *zero = aml_int(0); > - Aml *func = aml_arg(2); > - Aml *ret = aml_local(0); > - Aml *aidx = aml_local(1); > - Aml *params = aml_arg(4); > - > - method = aml_method("EDSM", 5, AML_SERIALIZED); > - > - /* get supported functions */ > - ifctx = aml_if(aml_equal(func, zero)); > - { > - /* 1: have supported functions */ > - /* 7: support for function 7 */ > - const uint8_t caps = 1 | BIT(7); > - build_append_pci_dsm_func0_common(ifctx, ret); > - aml_append(ifctx, aml_store(aml_int(caps), aml_index(ret, zero))); > - aml_append(ifctx, aml_return(ret)); > - } > - aml_append(method, ifctx); > - > - /* handle specific functions requests */ > - /* > - * PCI Firmware Specification 3.1 > - * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under > - * Operating Systems > - */ > - ifctx = aml_if(aml_equal(func, aml_int(7))); > - { > - Aml *pkg = aml_package(2); > - aml_append(pkg, zero); > - /* optional, if not impl. should return null string */ > - aml_append(pkg, aml_string("%s", "")); > - aml_append(ifctx, aml_store(pkg, ret)); > - > - /* > - * IASL is fine when initializing Package with computational data, > - * however it makes guest unhappy /it fails to process such AML/. > - * So use runtime assignment to set acpi-index after initializer > - * to make OSPM happy. > - */ > - aml_append(ifctx, > - aml_store(aml_derefof(aml_index(params, aml_int(0))), aidx)); > - aml_append(ifctx, aml_store(aidx, aml_index(ret, zero))); > - aml_append(ifctx, aml_return(ret)); > - } > - aml_append(method, ifctx); > - > - return method; > -} > - > /* > * build_prt - Define interrupt routing rules > *