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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CFD7BEB64DA for ; Wed, 19 Jul 2023 09:13:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230291AbjGSJNQ (ORCPT ); Wed, 19 Jul 2023 05:13:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59536 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229823AbjGSJNQ (ORCPT ); Wed, 19 Jul 2023 05:13:16 -0400 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BEC0F10B; Wed, 19 Jul 2023 02:13:14 -0700 (PDT) Received: from dggpemm500002.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4R5VT71yWZzrRr5; Wed, 19 Jul 2023 17:12:27 +0800 (CST) Received: from [10.174.178.247] (10.174.178.247) by dggpemm500002.china.huawei.com (7.185.36.229) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Wed, 19 Jul 2023 17:13:11 +0800 Subject: Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info() To: Shameerali Kolothum Thodi , Lorenzo Pieralisi , Guanghui Feng CC: "sudeep.holla@arm.com" , "rafael@kernel.org" , "linux-acpi@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "baolin.wang@linux.alibaba.com" , "alikernel-developer@linux.alibaba.com" , "will@kernel.org" , "catalin.marinas@arm.com" References: <1689593625-45213-1-git-send-email-guanghuifeng@linux.alibaba.com> <597f481b0e5149dabe4821ca618af6b3@huawei.com> From: Hanjun Guo Message-ID: <613da2c3-d515-b49c-4ff3-cf94836b2acf@huawei.com> Date: Wed, 19 Jul 2023 17:13:10 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <597f481b0e5149dabe4821ca618af6b3@huawei.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.178.247] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpemm500002.china.huawei.com (7.185.36.229) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On 2023/7/18 16:56, Shameerali Kolothum Thodi wrote: >> [+Catalin, Will, Shameer] >> >> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote: >>> According to the ARM IORT specifications DEN 0049 issue E, >>> the "Number of IDs" field in the ID mapping format reports >>> the number of IDs in the mapping range minus one. >>> >>> In iort_node_get_rmr_info(), we erroneously skip ID mappings >>> whose "Number of IDs" equal to 0, resulting in valid mapping >>> nodes with a single ID to map being skipped, which is wrong. >>> >>> Fix iort_node_get_rmr_info() by removing the bogus id_count >>> check. >>> >>> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR >> reserved regions") >>> Signed-off-by: Guanghui Feng >>> --- >>> drivers/acpi/arm64/iort.c | 3 --- >>> 1 file changed, 3 deletions(-) >>> >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>> index 3631230..56d8873 100644 >>> --- a/drivers/acpi/arm64/iort.c >>> +++ b/drivers/acpi/arm64/iort.c >>> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct >> acpi_iort_node *node, >>> for (i = 0; i < node->mapping_count; i++, map++) { >>> struct acpi_iort_node *parent; >>> >>> - if (!map->id_count) >>> - continue; >>> - >>> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, >>> map->output_reference); >>> if (parent != iommu) >> Shameer, I know this may look like overkill since the hunk we are >> removing is buggy but can you please test this patch on platforms >> with RMR to make sure we are not triggering regressions by removing >> it (by the specs that's what should be done but current firmware >> is always something to reckon with) ? > Yes, that is a valid fix. Unlikely it will be a problem. Anyway, I have requested > Hanjun to help with the testing as I don't have a test setup with me now. Valid fix for me as well, we had a firmware bug which reported the numbers of ID as 1 when we only have one ID mapping, so remove the check is fine for the old firmware, but to make it sure, we need some test before give it a pass. > > Hanjun, please help. I need some time to get it properly tested on two versions of firmware, and get the test machine properly setup, please allow me give the feedback next week. Thanks Hanjun 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 E90C9EB64DA for ; Wed, 19 Jul 2023 09:13:45 +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:MIME-Version:Date:Message-ID:From: References:CC:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Ia/bRfQ/pgcHzggF9fO1TzEPdjg3BuLzw96stGPgDrE=; b=aD778sd3XH3A56JESlKtjtG5AC GHzEB36qlaPndfB/8Rbr9n3o0fDQj54RRPYXfB6jHFaDspIK/mKHPGLrZ9MWOJfsQePf0xEcskhDo Tza5u3LH4b0KVob6RaQJ4vuIGNbdXCf/rtadrDTcC/dUotQ909BlQAojMAgeUJdNnnGpRX/AaYpEY pD7xPmWgaxaDPTjx6FztcvXwPW9ZKnkULqHJQ6gWZWwg0w1j1SG9CMQYHlSZso1Irmz5mOC8FCpEd YVgNuBncdXd8ueAoiIvzvmVUq8ookZERZ+ot6uDOvKMb9ETJYztHoAhm7W3/mppklwcBsp8ja+24I Miaw9Gwg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qM3FT-006W1J-1Y; Wed, 19 Jul 2023 09:13:31 +0000 Received: from szxga01-in.huawei.com ([45.249.212.187]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qM3FP-006Vys-2j for linux-arm-kernel@lists.infradead.org; Wed, 19 Jul 2023 09:13:30 +0000 Received: from dggpemm500002.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4R5VT71yWZzrRr5; Wed, 19 Jul 2023 17:12:27 +0800 (CST) Received: from [10.174.178.247] (10.174.178.247) by dggpemm500002.china.huawei.com (7.185.36.229) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Wed, 19 Jul 2023 17:13:11 +0800 Subject: Re: [PATCH v3] ACPI/IORT: Remove erroneous id_count check in iort_node_get_rmr_info() To: Shameerali Kolothum Thodi , Lorenzo Pieralisi , Guanghui Feng CC: "sudeep.holla@arm.com" , "rafael@kernel.org" , "linux-acpi@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "baolin.wang@linux.alibaba.com" , "alikernel-developer@linux.alibaba.com" , "will@kernel.org" , "catalin.marinas@arm.com" References: <1689593625-45213-1-git-send-email-guanghuifeng@linux.alibaba.com> <597f481b0e5149dabe4821ca618af6b3@huawei.com> From: Hanjun Guo Message-ID: <613da2c3-d515-b49c-4ff3-cf94836b2acf@huawei.com> Date: Wed, 19 Jul 2023 17:13:10 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <597f481b0e5149dabe4821ca618af6b3@huawei.com> Content-Language: en-GB X-Originating-IP: [10.174.178.247] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpemm500002.china.huawei.com (7.185.36.229) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230719_021328_289861_B529AA9E X-CRM114-Status: GOOD ( 21.26 ) 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 2023/7/18 16:56, Shameerali Kolothum Thodi wrote: >> [+Catalin, Will, Shameer] >> >> On Mon, Jul 17, 2023 at 07:33:45PM +0800, Guanghui Feng wrote: >>> According to the ARM IORT specifications DEN 0049 issue E, >>> the "Number of IDs" field in the ID mapping format reports >>> the number of IDs in the mapping range minus one. >>> >>> In iort_node_get_rmr_info(), we erroneously skip ID mappings >>> whose "Number of IDs" equal to 0, resulting in valid mapping >>> nodes with a single ID to map being skipped, which is wrong. >>> >>> Fix iort_node_get_rmr_info() by removing the bogus id_count >>> check. >>> >>> Fixes: 491cf4a6735a ("ACPI/IORT: Add support to retrieve IORT RMR >> reserved regions") >>> Signed-off-by: Guanghui Feng >>> --- >>> drivers/acpi/arm64/iort.c | 3 --- >>> 1 file changed, 3 deletions(-) >>> >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>> index 3631230..56d8873 100644 >>> --- a/drivers/acpi/arm64/iort.c >>> +++ b/drivers/acpi/arm64/iort.c >>> @@ -1007,9 +1007,6 @@ static void iort_node_get_rmr_info(struct >> acpi_iort_node *node, >>> for (i = 0; i < node->mapping_count; i++, map++) { >>> struct acpi_iort_node *parent; >>> >>> - if (!map->id_count) >>> - continue; >>> - >>> parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, >>> map->output_reference); >>> if (parent != iommu) >> Shameer, I know this may look like overkill since the hunk we are >> removing is buggy but can you please test this patch on platforms >> with RMR to make sure we are not triggering regressions by removing >> it (by the specs that's what should be done but current firmware >> is always something to reckon with) ? > Yes, that is a valid fix. Unlikely it will be a problem. Anyway, I have requested > Hanjun to help with the testing as I don't have a test setup with me now. Valid fix for me as well, we had a firmware bug which reported the numbers of ID as 1 when we only have one ID mapping, so remove the check is fine for the old firmware, but to make it sure, we need some test before give it a pass. > > Hanjun, please help. I need some time to get it properly tested on two versions of firmware, and get the test machine properly setup, please allow me give the feedback next week. Thanks Hanjun _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel