From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a17:504:1e0a:b0:1be9:327d:8ee3 with SMTP id i10csp2987751njk; Tue, 28 Jan 2025 02:01:14 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCUhMk9wJOtTkyF5Be77ku5dfZEeN1AdszPwDMdNbtURxGu69eYyXGFZBfe4F/5Ns5Y5XHVqgi1vKJwNWA==@linaro.org X-Google-Smtp-Source: AGHT+IHyeCsRedLizs6jdmSC9MLdZNiKMQ+LyNPNOlFhksgXj6OKEAWJY+j3pWSPMBk28F4h0ylX X-Received: by 2002:a05:6102:b01:b0:4af:a216:c0d0 with SMTP id ada2fe7eead31-4b6909c1d11mr41935610137.0.1738058474643; Tue, 28 Jan 2025 02:01:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1738058474; cv=none; d=google.com; s=arc-20240605; b=Zhh87zV+RwIxyqU+wobWBSl/7rz1j5QOQfeKRqavRADuPgZmgimo3ie+46Z/NuZXyX YD6TpTTzWbq4xBNggg5OuNuf3dDg8jBVQnQ9wlPekeLDEAHluD7G/dgtRmuUIyG9xxft 9HQDTH9r4O3zBSaEAfZa8i+VRk3xMbQrLwj6/rkZL3T4ZA0shYbqdxc/UcyQ95eXWEJw Mh7eyJwoVW7QYQ3pYa1DUUdcoavc7339O3WNWpwmlT3e7CzW4g5GW75VsG+RNRANGPdJ TdcdNUG7Xt/B4iiE6wEhyurvixOPNv8+/z2nwnhJaIZQCbnxDZ1oVokQnPNGceDT5IVL j2kQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:references:in-reply-to:message-id:subject:cc:to:from :date:dkim-signature; bh=tR3ctpdBtfVDWRGXkkRv+NzMbzIJpRZLUqkczU6g/Vs=; fh=ga30WJz+AM+zGlkQ58aeLKsNtsPzufzpPtZNXhGYMmI=; b=RKmApYh6r6dfYAAXWhu8PYkTiPxInKYxVMEC0aFmaIcTyQ4672aFrQe8h229Wt1uTu chyRwryjQtDTifkK4Mko4LSodByHthwc2EbafBmGtthlBjhqMuZNHVMttRdTiiEee9Vp 4qU778/jdxZWCF+Npvq2CZ6zu9w730NXg6HsmoR6MGAuUJ7d4yAVFlZbgz0jXZvc70Cs 8kjs5V4/m3/qCNqxPQjImRfWzrf5eP+dKpwmlvwD37PyVjEeVXIdMscG9qPXf+agwI2h NJ/p/kTNK9behKg5RJiCe4JdrasQfOnQ8KWe/wVfxBQGJ4W2Fr3QNx6tnmNVRZ/t8POB u/NA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=G6vtGOYg; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=kernel.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id ada2fe7eead31-4b7099f8075si2778320137.505.2025.01.28.02.01.13 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 28 Jan 2025 02:01:13 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=G6vtGOYg; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=kernel.org Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tciOt-0003na-Hx; Tue, 28 Jan 2025 05:00:55 -0500 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 1tciOs-0003nH-34; Tue, 28 Jan 2025 05:00:54 -0500 Received: from dfw.source.kernel.org ([139.178.84.217]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tciOp-0002Wt-DO; Tue, 28 Jan 2025 05:00:53 -0500 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0DC985C58A0; Tue, 28 Jan 2025 10:00:02 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 99BF2C4CEE7; Tue, 28 Jan 2025 10:00:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1738058441; bh=kZ5N8tHCHo1PrNZqE9WlUMEB8RGu5iVv5hQ4/VAaUpc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=G6vtGOYgV8ruY7AF1SWwVYIpaG4+lPlu6Fkv24SvdbRutxSMsJW2GOgelXXeXHj9K XKgnKAamkuOpz34TgqYxcsJpPMfLV5NG62q9KQHMZExlr0iA66S3G35TfiZIWm5Znq JxxmXCl5fBxZsiB3BQXH7iCy4x1Q68HWSU+5tdxum/g3iHnB9J9gWrxU1uhl5yIZ1U +a0gED1Y4Ja2J4wBTqgE6H56TKmSBF7MlKxtsF7o1r/Zo3uRFunaDpxktlHv4oA6fa nEgtP1dxeW1ip/Ey9MYsC8ybPdtbf6f3M+S5/2Vhh3CWhrlzLnv2F6eJdMvFpsbHMw nwG3dIhbGJ8wA== Date: Tue, 28 Jan 2025 11:00:34 +0100 From: Mauro Carvalho Chehab To: Jonathan Cameron Cc: Igor Mammedov , "Michael S . Tsirkin" , Shiju Jose , , , Ani Sinha , Dongjiu Geng , Subject: Re: [PATCH 02/11] acpi/ghes: add a firmware file with HEST address Message-ID: <20250128110034.650445fc@foz.lan> In-Reply-To: <20250123100217.00007373@huawei.com> References: <20250123100217.00007373@huawei.com> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.43; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=139.178.84.217; envelope-from=mchehab+huawei@kernel.org; helo=dfw.source.kernel.org X-Spam_score_int: -83 X-Spam_score: -8.4 X-Spam_bar: -------- X-Spam_report: (-8.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-1.3, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-arm@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org Sender: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org X-TUID: n+1Lt9Q8/iyy Em Thu, 23 Jan 2025 10:02:17 +0000 Jonathan Cameron escreveu: > On Wed, 22 Jan 2025 16:46:19 +0100 > Mauro Carvalho Chehab wrote: > > > Store HEST table address at GPA, placing its content at > > hest_addr_le variable. > > > > Signed-off-by: Mauro Carvalho Chehab > > Reviewed-by: Jonathan Cameron > > > A few trivial things inline. > > Jonathan > > > --- > > > > Change from v8: > > - hest_addr_lr is now pointing to the error source size and data. > > > > Signed-off-by: Mauro Carvalho Chehab > Bonus. I guess you really like this patch :) > > --- > > hw/acpi/ghes.c | 17 ++++++++++++++++- > > include/hw/acpi/ghes.h | 1 + > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c > > index 3f519ccab90d..34e3364d3fd8 100644 > > --- a/hw/acpi/ghes.c > > +++ b/hw/acpi/ghes.c > > @@ -30,6 +30,7 @@ > > > > #define ACPI_HW_ERROR_FW_CFG_FILE "etc/hardware_errors" > > #define ACPI_HW_ERROR_ADDR_FW_CFG_FILE "etc/hardware_errors_addr" > > +#define ACPI_HEST_ADDR_FW_CFG_FILE "etc/acpi_table_hest_addr" > > > > /* The max size in bytes for one error block */ > > #define ACPI_GHES_MAX_RAW_DATA_LENGTH (1 * KiB) > > @@ -261,7 +262,7 @@ static void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker, > > } > > > > /* > > - * tell firmware to write hardware_errors GPA into > > + * Tell firmware to write hardware_errors GPA into > > Sneaky tidy up. No problem with it in general but adding noise here, so if there > are others in the series maybe gather them up in a cleanup patch. There are no other cleanups pending. Besides, as you noticed, this aligns with the comment below. So, I'm opting to add a note at the patch's description. > > > * hardware_errors_addr fw_cfg, once the former has been initialized. > > */ > > bios_linker_loader_write_pointer(linker, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, 0, > > @@ -355,6 +356,8 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > > > acpi_table_begin(&table, table_data); > > > > + int hest_offset = table_data->len; > > Local style looks to be traditional C with definitions at top. Maybe define > hest_offset up a few lines and just set it here? Ok. I'll follow Igor's suggestion of using uint32_t. > > + > > /* Error Source Count */ > > build_append_int_noprefix(table_data, num_sources, 4); > > for (i = 0; i < num_sources; i++) { > > @@ -362,6 +365,15 @@ void acpi_build_hest(GArray *table_data, GArray *hardware_errors, > > } > > > > acpi_table_end(linker, &table); > > + > > + /* > > + * tell firmware to write into GPA the address of HEST via fw_cfg, > > Given the tidy up above, fix this one to have a capital T, or was this > where you meant to change it? OK. > > + * once initialized. > > + */ > > + bios_linker_loader_write_pointer(linker, > > + ACPI_HEST_ADDR_FW_CFG_FILE, 0, > > Could wrap less and stay under 80 chars as both lines above add up to 70 something Why? This follows QEMU coding style and lines aren't longer than 80 columns. Besides, at least for my eyes and some experience doing maintainership on other projects over the years, it is a lot quicker to identify function parameters if they're properly aligned with the parenthesis. Thanks, Mauro