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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 230D2C77B73 for ; Wed, 19 Apr 2023 17:33:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7032710E60D; Wed, 19 Apr 2023 17:33:40 +0000 (UTC) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8C52910E1D7; Wed, 19 Apr 2023 17:33:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1681925617; x=1713461617; h=message-id:date:subject:to:cc:references:from: in-reply-to:content-transfer-encoding:mime-version; bh=GHermTAwBv9G1+5Xc/wKWo95MYN3bZGctosrN8JetAc=; b=X6X58JKZnAyvcW6fx50wze361sqcbuSQv6qa2tjDIdw93ZdHlYlJ5HiY lUABFO4XYf6p+wMXQvA5s+ZveX+XwpPaBSkGz4zb5HLsCjQ52kzjJl1+j vJJlkMbBMaL1Ep70sGFI0Iwr8zLiHAyUD2ds4DdyUOx06sbOfuXAIwTgd HUwmbx/ppJ0cMeJH/5c0AqTc5HoQYoJJbtgd8vtHtUbX63Qc2GzF/S1nI ZJ9lJD0KDISk35e2eL/cDgNF3oqZ1Js/izCkNKFsEgGDLIzkVWhcMD5pR eKwoc1VdcPDPr9JUM623i+eLIl6GsWXOAPBKIN6AV1hJlQTl2RJXb998I g==; X-IronPort-AV: E=McAfee;i="6600,9927,10685"; a="348265534" X-IronPort-AV: E=Sophos;i="5.99,208,1677571200"; d="scan'208";a="348265534" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Apr 2023 10:33:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10685"; a="722016982" X-IronPort-AV: E=Sophos;i="5.99,208,1677571200"; d="scan'208";a="722016982" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orsmga008.jf.intel.com with ESMTP; 19 Apr 2023 10:33:36 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Wed, 19 Apr 2023 10:33:36 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Wed, 19 Apr 2023 10:33:36 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.45) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.23; Wed, 19 Apr 2023 10:33:36 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NzfSVd2quhTMj9EiymvFPej8UG52oRmtF6HsTX9/FXIFRFnqRwEgFnF/BnGSCRJJ1NVbI0PrBGFyuib+95T/h684885wHHRFBInOsh80aIaLKXngXLCkccrPAbnvSoFS4gcRVVWXW5a6wu3NNXXMxba+mtygQyYtyVKkLlRqH5bPOnkOcxANcVrXQokWVgJJAnnFJLv84CzN0WJ008im2n0ozqD9+hKWIhE6P0lazYO6zxl3Ekhhxxbw2NGVBo+7G4FojwMJJ+Mqn0hv93DNyH6C7Modz/wLYGLQP8bp6ETWHUo46+JJsf+k8P4QOvTQ+fftn073UC4Fa0wCUD4Wmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=hHRJT8TzxXzHcSCsGyGGCo3CTK/47ISz/aXwTOqAilw=; b=Nzd4Yn29KtElflDlOSMabvQ2Jvksfb23osZnv9y1737ATyjRH1PCXxht6DEG580j4daAgJOpUEESdaWAy0bwkpZvgB6To0YOALqizg5K+JIvGP4FIcaA6JfvB6BoNG4ELUl/0Tp3Xl3g42i2fkvBGtI24bfWjcHDgAvFM39hR9dhPaZszg7dmlXuC+X5ZNuL+UxwK3NqjzUvlctCoS33UZFjwbjtaAXo97U6m0Fn66MFc7p5NtYwxGqXSWb33sSKZUdzhqCIPnHKamnelNBxwDCHBPwadRoM4SbNpQdEf3f/3n2WNuNcGvE4IUMIUxsqk5At2w3/yIx0hV8n1y/xwQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DM4PR11MB5488.namprd11.prod.outlook.com (2603:10b6:5:39d::5) by CY8PR11MB7194.namprd11.prod.outlook.com (2603:10b6:930:92::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6298.45; Wed, 19 Apr 2023 17:33:34 +0000 Received: from DM4PR11MB5488.namprd11.prod.outlook.com ([fe80::4ae6:750e:a237:4eb0]) by DM4PR11MB5488.namprd11.prod.outlook.com ([fe80::4ae6:750e:a237:4eb0%7]) with mapi id 15.20.6319.022; Wed, 19 Apr 2023 17:33:34 +0000 Message-ID: Date: Wed, 19 Apr 2023 10:33:31 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 To: John Harrison , References: <20230415005706.4135485-1-John.C.Harrison@Intel.com> <20230415005706.4135485-6-John.C.Harrison@Intel.com> <2cd1381b-10c6-f5ce-e868-39615d24280c@intel.com> Content-Language: en-US From: "Ceraolo Spurio, Daniele" In-Reply-To: <2cd1381b-10c6-f5ce-e868-39615d24280c@intel.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: SJ0PR13CA0199.namprd13.prod.outlook.com (2603:10b6:a03:2c3::24) To DM4PR11MB5488.namprd11.prod.outlook.com (2603:10b6:5:39d::5) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DM4PR11MB5488:EE_|CY8PR11MB7194:EE_ X-MS-Office365-Filtering-Correlation-Id: da78cf8a-e294-467b-dd4c-08db40fc334d X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: gmhMa638tgke1YfiA7hsvVsK5M9EHYjM1R8pJ4RhKD/FiBP/RH6+3UB7uoM1O2SPBC8ihgECpkyPJ7PTIYypSTlDD4QAGX1C7KWho1ZnVQvT3yRxl51c6gAISt98v3kRpl4mKi3Qkeu+jqEbRi0FSiCFN9azMP6AZcwkm2xcjx4v8XdJ8/ohdy0JVKUBG4aPkm9X2jwve08NrWeVmCH7Qo3se5r/FE/mzYqDumW/hudAXj+BWCwfav21FeeY49eanOjELymceOpcae27NBGJxG7VocKsyk6E6h5IwFm02hHDdu9LDi4s5ZUHapHXrZngp9b4YjMTICAKpL4tc95Jtxl7yts/EtG7qkIg0hrczmtrGkcbvphZc82Yhna3hx5iYLLwxlRMpaTKhaTGs0WmeKPGp/Zx8bN/uci9vbMjMRZFBzlOuC3Fcyx12RlCDkXM8LU8FFMi9am4XNsDZJyNsxN31r+5KiBynF5nS9VXK5ha8JULZHpL9R5qU24i4LO8YvnbxLrvaxhFI1Iv+Gy6glZ+iwqu1TR9LitLh17z5tIpaL56S0RafO30LOtbvZ+htnt8NK2LkWqpVZkoymjKMYTZruiKXF/VgnTKqlgCuvm82XppQbBTY/okJloHaZIGnJYooaBYzAU7SX1laFpOlQ== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM4PR11MB5488.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(39860400002)(396003)(136003)(366004)(376002)(346002)(451199021)(36756003)(2906002)(8936002)(5660300002)(8676002)(41300700001)(86362001)(31696002)(38100700002)(31686004)(478600001)(2616005)(316002)(53546011)(26005)(6506007)(6512007)(186003)(6666004)(6486002)(4326008)(66476007)(66556008)(66946007)(450100002)(83380400001)(82960400001)(45980500001)(43740500002); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZjUwcmdRSmlDK0pOU1JBVEtLYkVCS1dVdFZCN1BzUVl3YXR0MS9VeGFDaGhD?= =?utf-8?B?QU55M3FmUzIzWW5xMGJWNW5HNjA2NWhqbjVwQWJlZDA1emF1clpQcEhNOFhS?= =?utf-8?B?b3lnTFA1Vi84UmZXUXc4aWlyQUtRaDVjOTBzRWN0TVpEekJVK015anlDUzB2?= =?utf-8?B?OUIzVUsxOVRSYmR1V2lLTTVFVmlXYjNHM0J2NXdzakducnAyOXlWSG56UDhm?= =?utf-8?B?ejJRdWJSYUIyZ3B1bHRKNXdybmt3SzgyRWgzQmRpZ21NOTk4MEtucHQ2cXpL?= =?utf-8?B?Ni9HOHZHQUtVZUx1dlFyT3p0NitwUHJUM3drc2R0bE40TnJvclplcUh2cTIx?= =?utf-8?B?OVZ5RWZiU1BBaXl0OEVkWE9WWWRIK3JQeDhGWlZ0YVBSYmhRcFZkem5wMnVQ?= =?utf-8?B?czczc1RYZWZQOUxFVkx2L1lrdUZ2REIzajMrREhSRW9wVFlRT2JHVzlXeGRh?= =?utf-8?B?T3hDRGNMUG5vTmlRNjVrbHg0V1diT2JNWFBPc2ZJREY0WnFQbCtoSFRUVjJP?= =?utf-8?B?ejhXUGhxU21WQ0ZaTkxKeDRpa2ZrWjRaeWtsK0NzS2huV211bUYvNW9JRE1V?= =?utf-8?B?QXNFaWcvTUE0V0h5cWVRNllvQ2prSWM3dXNKT1BwNVN4UW1vMGtuaElrdnU0?= =?utf-8?B?Qnl2N3gwL0dUUXBjTDJNS0Z4V3NHdjVOaklkdlU5bWNDSTJGWVhyNnowSVYv?= =?utf-8?B?Nmo3WmNqckNWZ1c0NW0vWUNFN09mb1pON25GNHo5M3ZKTGkzYXFhMFErekt0?= =?utf-8?B?c21CcXFBdFFEU2hCazVCYWJOdlBMeHg1TGM3Rm9lb1RrTjM5alc2UllVRWd0?= =?utf-8?B?N05ZZHh0U2pZL2lCSmJBdklpQTYxd3FBeHhOVlhkeEhKdTRQK0c5cnB6YWZX?= =?utf-8?B?SUNKMTBIcDV5QnQ3MVd5ZVpYS3E1SEJuaWhGdzhwOFRaVnh1a0lYTVEvSVJj?= =?utf-8?B?Y3hLS2Uxb2pVVm5Ialc2N0Y5UWI1SmJRd0wrUUdNS2EyTkhVT0NLV3JXd2tp?= =?utf-8?B?RzNnbndaZ0VNVWZURldDUVVxNzRFTG1FZzBwaCtkQXRiTlNyNkozajB4WUNI?= =?utf-8?B?NFkvVDNrTERJalNlQU1MS2ZUaWZRNDRETksxSkRscWE1K1l1SG9yRkRoWmFG?= =?utf-8?B?WjhwdWwzdmNlam80enIweWE0U002ZE9EdjcybjY1RmFWSVhLcTRmQ2ZOaVZ3?= =?utf-8?B?N2xQOTBua1ZnOHA4eTh2TnpVWDJ0dm5jYmptczcxYkowZU5jL0FqK1BvNmp5?= =?utf-8?B?NGpERTU2TTFmQ25mSGNheDgweFZsMnRPdGtTNkU0NGdrMmxNbDk3MUQ1Y1Yx?= =?utf-8?B?MnV4b2Z4VWNYdXhvQzJVekZyYUtDYkJEMjJEVitueHVONUFXL2JWNlpudjNq?= =?utf-8?B?L1RmUVlsZlU4OWZVQVZWNlVLbkQ2OXBXQllFMTE5SURwTFRJclNMSld1d05n?= =?utf-8?B?eTROY054cHQ4VjlERElwTm5Oc0RueWNZQUcrREI5a0NqS0RNWVNNN2kyU3FY?= =?utf-8?B?NW5ZeVNVZEM2TnFnMFY4Qy9yK0tkQ25HK1YvYWd6Y1owZVVQWDJnNW9KWmIz?= =?utf-8?B?bTkrWmFra2NlR25sdnBwNlFGVVF2cWU4ajNPRXM1aFJpTlBLWU84b3ozY3ZK?= =?utf-8?B?OC84OVpQWmV6RjZydks5dTcrZUdtYklkRTRiSjErMWphMHFHYklnbFh3WUl5?= =?utf-8?B?Y0VxUVhLV1NMQnZhb2xMUFhzN2FnWWZ4aTl3MHBIN05pTWJuVHQ0RFNIZTdG?= =?utf-8?B?WHpUWURXT0xCZVdxRnQ0TGwwZXhQZ3R5TE5GK050NHRrL1NMWVg5SDRnZStn?= =?utf-8?B?cFBpcnpNMGFvUEwrQzNtM29CQmdLNFVjV2JXenVIa2NMN1o0Rmh4NDNqWlIy?= =?utf-8?B?aHRQL3VtNm5xdDJSR202dEQ2RW95RFdmQTZra0R4NG1RQ3ovSHpXN2VERTNT?= =?utf-8?B?SVZzTTV1UHNjanhkTWI1a2ZEb2E4eFVUenI0YXZURDU2cGtkeVpLM3NGQjRm?= =?utf-8?B?QVVnNEFjNEc1WUttRUZJSlZqWm01MFdnZ3ZkYjc1ZmRET2xHZzFWQ3hLMm1M?= =?utf-8?B?NVY2N2hFd01Pa1VHM1M1UG5DQktzR0VEZk0yWWpOMTdOV2d3d2syZmdyT1VK?= =?utf-8?B?WTNiQUlvdG1tWmsvU1JBWFRWYVBtb3Q0RkI0K1ozVkZqVm5Cem90VkJFRzZy?= =?utf-8?B?ekE9PQ==?= X-MS-Exchange-CrossTenant-Network-Message-Id: da78cf8a-e294-467b-dd4c-08db40fc334d X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB5488.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Apr 2023 17:33:34.2051 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: AxXktqlrtfWZ+0i2r85nm8aBBtO3wBv0TpMuLU5+xirP0kOiQ+qhGl5SK8BAfYcXBixuAS9NEc7dsiedBzAgHbgs/RBfQHAL0GKSZC89BoQ= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7194 X-OriginatorOrg: intel.com Subject: Re: [Intel-gfx] [PATCH 5/5] drm/i915/uc: Reject doplicate entries in firmware table X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: DRI-Devel@Lists.FreeDesktop.Org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 4/19/2023 10:12 AM, John Harrison wrote: > On 4/19/2023 10:02, John Harrison wrote: >> On 4/18/2023 16:24, Ceraolo Spurio, Daniele wrote: >>> Typo doplicate in patch title >>> >>> On 4/14/2023 5:57 PM, John.C.Harrison@Intel.com wrote: >>>> From: John Harrison >>>> >>>> It was noticed that duplicte entries in the firmware table could cause >>> >>> typo duplicte >>> >>>> an infinite loop in the firmware loading code if that entry failed to >>>> load. Duplicate entries are a bug anyway and so should never happen. >>>> Ensure they don't by tweaking the table validation code to reject >>>> duplicates. >>> >>> Here you're not really rejecting anything though, just printing an >>> error (and even that only if the SELFTEST kconfig is selected). This >>> would allow our CI to catch issues with patches sent to our ML, but >>> IIRC the reported bug was on a kernel fork. We could disable the FW >>> loading is the table for that particular blob type is in an invalid >>> state, as it wouldn't be safe to attempt a load in that case anyway. >> The validation code is rejecting duplicates. Whether the driver loads >> or not after a failed validation is another matter. >> >> I was basically assuming that CI will fail on the error message and >> thus prevent such code ever being merged. But yeah, I guess we don't >> run CI on backports to stable kernels and such. Although, I would >> hope that anyone pushing patches to a stable kernel would run some >> testing on it first! >> >> Any thoughts on a good way to fail the load? We don't want to just >> pretend that firmware is not wanted/required on the platform and just >> load the i915 module without the firmware. Also, what about the >> longer plan of moving the validation to a selftest. You can't fail >> the load at all then. > Actually, forgot we already have a INTEL_UC_FIRMWARE_ERROR status. > That works fine for aborting the load. So just go with that and drop > the plan to move to a selftest? > > John. I do actually like the idea of moving this code to a mock selftest. Maybe just add a comment above the tables making clear that duplicated entries are not allowed and will break the loading flow? Daniele > > >> >> John. >> >>> >>>> >>>> For full m/m/p files, that can be done by simply tweaking the patch >>>> level check to reject matching values. For reduced version entries, >>>> the filename itself must be compared. >>>> >>>> Signed-off-by: John Harrison >>>> --- >>>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 >>>> +++++++++++++++++++++--- >>>>   1 file changed, 24 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >>>> index c589782467265..44829247ef6bc 100644 >>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >>>> @@ -319,7 +319,7 @@ static void validate_fw_table_type(struct >>>> drm_i915_private *i915, enum intel_uc_ >>>>   { >>>>       const struct uc_fw_platform_requirement *fw_blobs; >>>>       u32 fw_count; >>>> -    int i; >>>> +    int i, j; >>>>         if (type >= ARRAY_SIZE(blobs_all)) { >>>>           drm_err(&i915->drm, "No blob array for %s\n", >>>> intel_uc_fw_type_repr(type)); >>>> @@ -334,6 +334,27 @@ static void validate_fw_table_type(struct >>>> drm_i915_private *i915, enum intel_uc_ >>>>         /* make sure the list is ordered as expected */ >>>>       for (i = 1; i < fw_count; i++) { >>>> +        /* Versionless file names must be unique per platform: */ >>>> +        for (j = i + 1; j < fw_count; j++) { >>>> +            /* Same platform? */ >>>> +            if (fw_blobs[i].p != fw_blobs[j].p) >>>> +                continue; >>>> + >>>> +            if (fw_blobs[i].blob.path != fw_blobs[j].blob.path) >>>> +                continue; >>>> + >>>> +            drm_err(&i915->drm, "Diplicaate %s blobs: %s r%u >>>> %s%d.%d.%d [%s] matches %s r%u %s%d.%d.%d [%s]\n", >>> >>> Typo Diplicaate >>> >>> Daniele >>> >>>> + intel_uc_fw_type_repr(type), >>>> +                intel_platform_name(fw_blobs[j].p), fw_blobs[j].rev, >>>> +                fw_blobs[j].blob.legacy ? "L" : "v", >>>> +                fw_blobs[j].blob.major, fw_blobs[j].blob.minor, >>>> +                fw_blobs[j].blob.patch, fw_blobs[j].blob.path, >>>> +                intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev, >>>> +                fw_blobs[i].blob.legacy ? "L" : "v", >>>> +                fw_blobs[i].blob.major, fw_blobs[i].blob.minor, >>>> +                fw_blobs[i].blob.patch, fw_blobs[i].blob.path); >>>> +        } >>>> + >>>>           /* Next platform is good: */ >>>>           if (fw_blobs[i].p < fw_blobs[i - 1].p) >>>>               continue; >>>> @@ -377,8 +398,8 @@ static void validate_fw_table_type(struct >>>> drm_i915_private *i915, enum intel_uc_ >>>>           if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor) >>>>               goto bad; >>>>   -        /* Patch versions must be in order: */ >>>> -        if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch) >>>> +        /* Patch versions must be in order and unique: */ >>>> +        if (fw_blobs[i].blob.patch < fw_blobs[i - 1].blob.patch) >>>>               continue; >>>>     bad: >>> >> >