From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a19:6911:0:0:0:0:0 with SMTP id e17csp2917061lfc; Mon, 25 Nov 2019 01:49:15 -0800 (PST) X-Google-Smtp-Source: APXvYqzxswg04BRnUU1zZK4MUAsWRMbWVN4JoRFf/olB888VFJgwPc186h27EADct5xmQBUGUn/c X-Received: by 2002:a50:ce47:: with SMTP id k7mr17258901edj.150.1574675355623; Mon, 25 Nov 2019 01:49:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1574675355; cv=none; d=google.com; s=arc-20160816; b=wEmnqcycO7mm21mOzw+ZQ5LSBVnMlvibbTGwm/B+th7Revo/NYsSgGhZMmQpq2u4Bp gB8HPDy4lpAS4tgp28MhKL1qYJHpsnJv1FDP4PGItlsEfR+dosSbrEEBtuHchDvJ9WZi ESh8NsnP//8JYG08v5QaBQQB4MohrLzRIzgOd+oboKiNgHda4PVMUKMSKLkckmEZC+7v 36v+h3cAbclcaIXkJp14F4iCUYeRJWjSBUltTM08wLHEjv6xrkq02H+E8lG6M0P5VMqa YtG2Os6xxvAeFc664yU5tkqhHyKpdF5xIH6LO6E1Oi9mNK4FwpdcDnUwiboCMEWvZ2/O C08A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :dkim-signature; bh=LI90vjXh2HXUGcnDPIxy5xrhkGpcg6ttLKuY2YJfARU=; b=namVbN7aF+RkYuqMy9VNhzFeO8eIu58ryiCJ6wk6XGfWZ71nxT0SOh79zyRIndF/cU wo3j4zvcOlWY/9YuXxS2wlzK21U0IBlalN5HnTpwY6/1paC+/7ghJgT02rXPFQf1iszG MXgCMQfOl8N4ZXNmOWck0e5DHv0talynd30T1wZddBieJtqHVKOdyLhJPtu0INVvH+M2 s2DH/IdrQRQ+Pja07Q3klzB3O2Oorl2CshgojlHy5GvMQGitdw5+XhmPReS6DsPWor0U u33zguAix8eauLz1pLuHsfc9xu3UEi5LTDeoQP3D9nsCeJX1290Brg8G3QDWIyO9YEAt yezg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=aIJofAWW; spf=pass (google.com: best guess record for domain of kvm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=kvm-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id bm18si4639424edb.74.2019.11.25.01.49.15; Mon, 25 Nov 2019 01:49:15 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of kvm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=aIJofAWW; spf=pass (google.com: best guess record for domain of kvm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=kvm-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727332AbfKYJtM (ORCPT + 4 others); Mon, 25 Nov 2019 04:49:12 -0500 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:44101 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727052AbfKYJtL (ORCPT ); Mon, 25 Nov 2019 04:49:11 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574675350; 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=LI90vjXh2HXUGcnDPIxy5xrhkGpcg6ttLKuY2YJfARU=; b=aIJofAWWfB45O1ifaaxgp7F/Mjy1pA1nLVDUAcyWtMsyiRe3ITdUx6z74ODSBP6PynCYGg eM1z+ZGaFwr3N4hMhV50tdhiULR7DbGujZAbjcREIuq90IJNcRakCl2cop7aIl7OPRYzM6 ZrxQdxOGrQZuhqC8koLF60rIleQGw6o= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-20-8zfc_lBjPRynSK0RUXnuaA-1; Mon, 25 Nov 2019 04:49:09 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E1D0E107ACE5; Mon, 25 Nov 2019 09:49:07 +0000 (UTC) Received: from localhost (unknown [10.43.2.114]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8DDD510016DA; Mon, 25 Nov 2019 09:49:00 +0000 (UTC) Date: Mon, 25 Nov 2019 10:48:59 +0100 From: Igor Mammedov To: "Michael S. Tsirkin" Cc: gengdongjiu , peter.maydell@linaro.org, ehabkost@redhat.com, kvm@vger.kernel.org, wanghaibin.wang@huawei.com, mtosatti@redhat.com, qemu-devel@nongnu.org, linuxarm@huawei.com, shannon.zhaosl@gmail.com, Xiang Zheng , qemu-arm@nongnu.org, james.morse@arm.com, jonathan.cameron@huawei.com, pbonzini@redhat.com, xuwei5@huawei.com, lersek@redhat.com, rth@twiddle.net Subject: Re: [RESEND PATCH v21 3/6] ACPI: Add APEI GHES table generation support Message-ID: <20191125104859.70047602@redhat.com> In-Reply-To: <20191118082036-mutt-send-email-mst@kernel.org> References: <20191111014048.21296-1-zhengxiang9@huawei.com> <20191111014048.21296-4-zhengxiang9@huawei.com> <20191115103801.547fc84d@redhat.com> <87758ec2-c242-71c3-51f8-a5d348f8e7fd@huawei.com> <20191118082036-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-MC-Unique: 8zfc_lBjPRynSK0RUXnuaA-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-TUID: Yj3/2UeBRSNN On Mon, 18 Nov 2019 08:21:18 -0500 "Michael S. Tsirkin" wrote: > On Mon, Nov 18, 2019 at 09:18:01PM +0800, gengdongjiu wrote: > > On 2019/11/18 20:49, gengdongjiu wrote: > > >>> + */ > > >>> + build_append_int_noprefix(table_data, source_id, 2); > > >>> + /* Related Source Id */ > > >>> + build_append_int_noprefix(table_data, 0xffff, 2); > > >>> + /* Flags */ > > >>> + build_append_int_noprefix(table_data, 0, 1); > > >>> + /* Enabled */ > > >>> + build_append_int_noprefix(table_data, 1, 1); > > >>> + > > >>> + /* Number of Records To Pre-allocate */ > > >>> + build_append_int_noprefix(table_data, 1, 4); > > >>> + /* Max Sections Per Record */ > > >>> + build_append_int_noprefix(table_data, 1, 4); > > >>> + /* Max Raw Data Length */ > > >>> + build_append_int_noprefix(table_data, ACPI_GHES_MAX_RAW_DATA_LENGTH, 4); > > >>> + > > >>> + /* Error Status Address */ > > >>> + build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0, > > >>> + 4 /* QWord access */, 0); > > >>> + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > > >>> + ACPI_GHES_ERROR_STATUS_ADDRESS_OFFSET(hest_start, source_id), > > >> it's fine only if GHESv2 is the only entries in HEST, but once > > >> other types are added this macro will silently fall apart and > > >> cause table corruption. > > why silently fall? > > I think the acpi_ghes.c only support GHESv2 type, not support other type. > > > > >> > > >> Instead of offset from hest_start, I suggest to use offset relative > > >> to GAS structure, here is an idea>> > > >> #define GAS_ADDR_OFFSET 4 > > >> > > >> off = table->len > > >> build_append_gas() > > >> bios_linker_loader_add_pointer(..., > > >> off + GAS_ADDR_OFFSET, ... > > > > If use offset relative to GAS structure, the code does not easily extend to support more Generic Hardware Error Source. > > if use offset relative to hest_start, just use a loop, the code can support more error source, for example: > > for (source_id = 0; i > { > > ...... > > bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > > ACPI_GHES_ERROR_STATUS_ADDRESS_OFFSET(hest_start, source_id), > > sizeof(uint64_t), ACPI_GHES_ERRORS_FW_CFG_FILE, > > source_id * sizeof(uint64_t)); > > ....... > > } > > > > My previous series patch support 2 error sources, but now only enable 'SEA' type Error Source > > I'd try to merge this, worry about extending things later. > This is at v21 and the simpler you can keep things, > the faster it'll go in. I don't think the series is ready for merging yet. It has a number of issues (not stylistic ones) that need to be fixed first. As for extending, I think I've suggested to simplify series to account for single error source only in some places so it would be easier on author and reviewers and worry about extending it later. 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 X-Spam-Level: X-Spam-Status: No, score=-0.6 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 862FDC432C0 for ; Mon, 25 Nov 2019 09:49:54 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 49F74207FD for ; Mon, 25 Nov 2019 09:49:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="aIJofAWW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 49F74207FD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:41798 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iZB0D-0003PQ-55 for qemu-devel@archiver.kernel.org; Mon, 25 Nov 2019 04:49:53 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:32960) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iZAzZ-00030Z-Tl for qemu-devel@nongnu.org; Mon, 25 Nov 2019 04:49:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iZAzY-0007cQ-B9 for qemu-devel@nongnu.org; Mon, 25 Nov 2019 04:49:13 -0500 Received: from us-smtp-delivery-1.mimecast.com ([205.139.110.120]:46209 helo=us-smtp-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iZAzX-0007bV-Sd for qemu-devel@nongnu.org; Mon, 25 Nov 2019 04:49:12 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574675350; 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=LI90vjXh2HXUGcnDPIxy5xrhkGpcg6ttLKuY2YJfARU=; b=aIJofAWWfB45O1ifaaxgp7F/Mjy1pA1nLVDUAcyWtMsyiRe3ITdUx6z74ODSBP6PynCYGg eM1z+ZGaFwr3N4hMhV50tdhiULR7DbGujZAbjcREIuq90IJNcRakCl2cop7aIl7OPRYzM6 ZrxQdxOGrQZuhqC8koLF60rIleQGw6o= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-20-8zfc_lBjPRynSK0RUXnuaA-1; Mon, 25 Nov 2019 04:49:09 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E1D0E107ACE5; Mon, 25 Nov 2019 09:49:07 +0000 (UTC) Received: from localhost (unknown [10.43.2.114]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8DDD510016DA; Mon, 25 Nov 2019 09:49:00 +0000 (UTC) Date: Mon, 25 Nov 2019 10:48:59 +0100 From: Igor Mammedov To: "Michael S. Tsirkin" Subject: Re: [RESEND PATCH v21 3/6] ACPI: Add APEI GHES table generation support Message-ID: <20191125104859.70047602@redhat.com> In-Reply-To: <20191118082036-mutt-send-email-mst@kernel.org> References: <20191111014048.21296-1-zhengxiang9@huawei.com> <20191111014048.21296-4-zhengxiang9@huawei.com> <20191115103801.547fc84d@redhat.com> <87758ec2-c242-71c3-51f8-a5d348f8e7fd@huawei.com> <20191118082036-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-MC-Unique: 8zfc_lBjPRynSK0RUXnuaA-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 205.139.110.120 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.maydell@linaro.org, ehabkost@redhat.com, kvm@vger.kernel.org, pbonzini@redhat.com, mtosatti@redhat.com, linuxarm@huawei.com, qemu-devel@nongnu.org, gengdongjiu , shannon.zhaosl@gmail.com, Xiang Zheng , qemu-arm@nongnu.org, james.morse@arm.com, xuwei5@huawei.com, jonathan.cameron@huawei.com, wanghaibin.wang@huawei.com, lersek@redhat.com, rth@twiddle.net Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Mon, 18 Nov 2019 08:21:18 -0500 "Michael S. Tsirkin" wrote: > On Mon, Nov 18, 2019 at 09:18:01PM +0800, gengdongjiu wrote: > > On 2019/11/18 20:49, gengdongjiu wrote: > > >>> + */ > > >>> + build_append_int_noprefix(table_data, source_id, 2); > > >>> + /* Related Source Id */ > > >>> + build_append_int_noprefix(table_data, 0xffff, 2); > > >>> + /* Flags */ > > >>> + build_append_int_noprefix(table_data, 0, 1); > > >>> + /* Enabled */ > > >>> + build_append_int_noprefix(table_data, 1, 1); > > >>> + > > >>> + /* Number of Records To Pre-allocate */ > > >>> + build_append_int_noprefix(table_data, 1, 4); > > >>> + /* Max Sections Per Record */ > > >>> + build_append_int_noprefix(table_data, 1, 4); > > >>> + /* Max Raw Data Length */ > > >>> + build_append_int_noprefix(table_data, ACPI_GHES_MAX_RAW_DATA_LENGTH, 4); > > >>> + > > >>> + /* Error Status Address */ > > >>> + build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0, > > >>> + 4 /* QWord access */, 0); > > >>> + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > > >>> + ACPI_GHES_ERROR_STATUS_ADDRESS_OFFSET(hest_start, source_id), > > >> it's fine only if GHESv2 is the only entries in HEST, but once > > >> other types are added this macro will silently fall apart and > > >> cause table corruption. > > why silently fall? > > I think the acpi_ghes.c only support GHESv2 type, not support other type. > > > > >> > > >> Instead of offset from hest_start, I suggest to use offset relative > > >> to GAS structure, here is an idea>> > > >> #define GAS_ADDR_OFFSET 4 > > >> > > >> off = table->len > > >> build_append_gas() > > >> bios_linker_loader_add_pointer(..., > > >> off + GAS_ADDR_OFFSET, ... > > > > If use offset relative to GAS structure, the code does not easily extend to support more Generic Hardware Error Source. > > if use offset relative to hest_start, just use a loop, the code can support more error source, for example: > > for (source_id = 0; i > { > > ...... > > bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > > ACPI_GHES_ERROR_STATUS_ADDRESS_OFFSET(hest_start, source_id), > > sizeof(uint64_t), ACPI_GHES_ERRORS_FW_CFG_FILE, > > source_id * sizeof(uint64_t)); > > ....... > > } > > > > My previous series patch support 2 error sources, but now only enable 'SEA' type Error Source > > I'd try to merge this, worry about extending things later. > This is at v21 and the simpler you can keep things, > the faster it'll go in. I don't think the series is ready for merging yet. It has a number of issues (not stylistic ones) that need to be fixed first. As for extending, I think I've suggested to simplify series to account for single error source only in some places so it would be easier on author and reviewers and worry about extending it later.