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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5B8EDC7EE26 for ; Tue, 16 May 2023 11:43:49 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9B6688663C; Tue, 16 May 2023 13:43:47 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=amd.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=amd.com header.i=@amd.com header.b="l9d7Uzg0"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9F00A86640; Tue, 16 May 2023 13:43:45 +0200 (CEST) Received: from NAM10-MW2-obe.outbound.protection.outlook.com (mail-mw2nam10on20602.outbound.protection.outlook.com [IPv6:2a01:111:f400:7e89::602]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 19F5E86139 for ; Tue, 16 May 2023 13:43:43 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=amd.com Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=michal.simek@amd.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kkeIXO5Iq8qGQf1lVFSbpRTK1MMQZl75R4lD/glh7TM+Mr5OiwMG4hI9LOc7sZaq/k74Sr/MpmcSuO7QOBuFPMhV4zIme2Q/gg6c6CBWqWJOgVqWnqDZ+yDZvJuz7+1YKHAKozQ2BvD4jwhxBL5YJL5hd0uNKD4cEhqwZEdXtoei0sYZ6ogSAdss5fMMJIAbVKEtggVB9dgZfhPc5g051EKA0jmWTNNZrNlj/71oCdHt2l0oTTvoM3Fo0WBi2Z7iTshQyePwyAJciQasJEqMVulZoP5neJ4bDa6gvbsrTWee8XFs3BZQfMlDWVm9JTn4oijwhNsEW4DQboM8IfQMuw== 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=T22sW/7XrnPtf9zo9e7LfRaQv2RZJcKyOiYft+iJpB8=; b=KFwrH+UMGX5pnJr0g3VBuByi4x4sZkznJpzijuix3QyprGk+faqGBuG7SER6iq72MATVhggEj1RpUqa8PTK465VYt6KWWyA/CZM1UcG1Vsu6cEg2/KrUibP3L5RsgjBk4V8AaJZIT2+s+lP3PDmJLoMfF3r1pQn1S8LEaO2iZJaBS4pHMN3qMDztNWffiOFQihDkReyrflUC0Q8nTqNwK0kFgV2kIy5Q7UPyWN2DaUgtFsGOxHtexBXvpR9QLelxo00uIYnvBXB8OUaEbvYJTo8pcJxj8EVnXbCDhuL3ZH5OHmkMb8q8zHdNdInBPpmgTbJLn9s3Y8XzfyS9p5fCyA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=T22sW/7XrnPtf9zo9e7LfRaQv2RZJcKyOiYft+iJpB8=; b=l9d7Uzg0MHBJ4AaCf1DYyri2bTptw8dCdwlHVBV04P5uXY5Atyf6MwMFgw0kfEqiF1DC6wfiQR1RY2d7rxMd4rMNmufcP0CECxAi8RNHCpX8Xf8KLrfFJm0SOOAGm5A0Sq4jW8uz1dZ8o9Xo9cl3dp5MC0aXV6Ptwt5WFgESIbQ= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from BYAPR12MB4758.namprd12.prod.outlook.com (2603:10b6:a03:a5::28) by SJ1PR12MB6292.namprd12.prod.outlook.com (2603:10b6:a03:455::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6387.30; Tue, 16 May 2023 11:43:39 +0000 Received: from BYAPR12MB4758.namprd12.prod.outlook.com ([fe80::e78e:b7da:7b9a:a578]) by BYAPR12MB4758.namprd12.prod.outlook.com ([fe80::e78e:b7da:7b9a:a578%4]) with mapi id 15.20.6387.032; Tue, 16 May 2023 11:43:38 +0000 Message-ID: <03e36898-6144-9552-cce0-e5c4e9629adf@amd.com> Date: Tue, 16 May 2023 13:43:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Content-Language: en-US To: Stefan Herbrechtsmeier , u-boot@lists.denx.de Cc: Stefan Herbrechtsmeier , Ashok Reddy Soma , Jay Buddhabhatti References: <20230427103125.26719-1-stefan.herbrechtsmeier-oss@weidmueller.com> <20230427103125.26719-2-stefan.herbrechtsmeier-oss@weidmueller.com> <8bdcb8b1-64e6-5b72-26f5-b7c35dced700@amd.com> <46205a5c-f7dd-21c2-c5b4-246da76aeed5@weidmueller.com> From: Michal Simek Subject: Re: [PATCH v3 2/2] firmware: zynqmp: Move config object permission check In-Reply-To: <46205a5c-f7dd-21c2-c5b4-246da76aeed5@weidmueller.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: FR0P281CA0116.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a8::18) To BYAPR12MB4758.namprd12.prod.outlook.com (2603:10b6:a03:a5::28) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BYAPR12MB4758:EE_|SJ1PR12MB6292:EE_ X-MS-Office365-Filtering-Correlation-Id: dcb16a4e-70bb-4c8d-1ba4-08db5602c9f3 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: J12+TYZvvVO5kDX/C6RPVPDatN3X0UPdmjCDNhvaAfBcm7P32VCYF0WTaUk1mvqOf3cEOacU2RdZx3CZlOLDbigoV40u/WbnvbHVy1XI/Vurlhzmu8AHBSWlpZuhzRmckxAvIS/yQYvyxOVYKT7hjWniSZsj1VJcA9/iDBAWAiGUys9FgjtRUOjF7FHIetrIM/D7jMH5oEcNklfRPlKm0AhsbYvGl/I3dOoJ/bTCh1VynB3QzatunXP2xBsBySegjed2cFcTDE6B8KII2sStQSltNN4Rl1l9t1hv2R1sPSvrSMiGowW8C6JFmnI4QalZ7izqol3tDDQlDvR5AD1ljN6d2ZSYIX3/lMqfa7Yc88J4kdWTl1gMDDw0VGjeoKR4pH7BCDpanrvv5F6Q19KJKmx8SN1KquA6afk4TDM58FCPv3m0S6Z7cIioOY6BozsywIVQ5V0Xog3P1ASL3seASoNn6z7A58H+lD/UOd8qER2L1TMTH+5RrtwlHr3wRLl9S9WnFncQnXZcpfFtAIFeYN0Bz/WbfTfl6yFUFV1ny14T6bChglK643iEqxZ8W0kHCOTDvQ9WyoqJNuubgJs/P2825PA5ODQtaX8yRa/zhGQPUfnr9u2FedQVFo73nBC0WaF1OPHlG8hl3S5yq67f6g== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR12MB4758.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(4636009)(376002)(366004)(396003)(346002)(136003)(39860400002)(451199021)(36756003)(478600001)(54906003)(66946007)(66476007)(66556008)(6486002)(6506007)(6512007)(6666004)(53546011)(41300700001)(26005)(8936002)(4326008)(8676002)(316002)(44832011)(2906002)(5660300002)(86362001)(31696002)(38100700002)(186003)(2616005)(83380400001)(31686004)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RVBwekgzMGpXc25lMUpQTTkwdWsvUWNtdk5XNFVZWmI3SFQyNFBqc3NZMUdD?= =?utf-8?B?ODRMckl2bEtDNmczWEFZaG5kaFVNVEZmVHdJRlNlUHhHZnRHbGQreUJGWC95?= =?utf-8?B?enA3YXRxSnhCR3J6dDlwUmFndURzK00xUHNCd21URUNiclV1azRjdWFmd293?= =?utf-8?B?R2NYSHJCNDcyRGxKbUZpbVBQaExJd3E2amg3TXlud1V6WTNZSkczdnRIREQ4?= =?utf-8?B?R1VCQ0RPZDhmSUlNMjVBRHVwWlREaUJncEpUV0VodStYaU9iSGd0N0VsaEJH?= =?utf-8?B?T0x1cmdWa0NOM1FrYmt0V2FBSmhLdGtJeFdxS3QwYW1pekhsMEpnMjBhYlJm?= =?utf-8?B?SHFLcE52bE9za1A5QXBoK1pCOE52NlBOQ2dLWVF6cHJQelJjS3NBcnI3bmxh?= =?utf-8?B?eFErTEV3cHlMb09VaVZXUGh6NEx0cTd2OFR3Qm1HaXZHcEhrb0VCWExMQXRO?= =?utf-8?B?ZmtXSDZMRE5xcTlHYm1EaEJ0YzNiNEZRa0IzcFNZN053cGZqcTVKYVNHRGl4?= =?utf-8?B?aHdFNEhQdkVqYVNDb3dKTDZPVHF2eFhrMCtHWVpXa2NLSnFXZHMwa3ZZb3Jx?= =?utf-8?B?NVpNNWRCdytqd1FJRFJCTDI2alJ6K2pzcWxEM3NMeHAzbnBXUWJYWUx6K1Q0?= =?utf-8?B?alYzMUJkQ2RWdTQyWm1CZmJVYWZhM1dNcHZXNCtCVEhFU2tIZXcvNmp4YlhS?= =?utf-8?B?K2k5N1NydXFmV0ZpbWJoQTNWSXpHR2hESDhaVE1OSUg2WnlqVDcrMmFJWS9r?= =?utf-8?B?eUFEUkczaDNrTUpzcTVzOWFzV2wwbXJ3aEhjbThSZVA5OUF0RWgvUDYxMTdF?= =?utf-8?B?a3BwS2NvOGNuQ3ZKQ1lxMnpycUdwUXo4NzIvVzU2bXlXdmErNktVYnBIMlRp?= =?utf-8?B?SE1SNll0NU9Ibko4ZVA0TDloZU13bTlJVU5QVk1hbHM5cjRQRGo2U216RDhB?= =?utf-8?B?UnZjM0p6dVBDT0FqT2tnUjlvNkZJVGsyYktCODRRbWZDMUZTc0c0Ti9nczNm?= =?utf-8?B?VlJuYkFjMldCN3NsUUpHWmx6R0FEWXFaNHpNdWh0djVZanZlRE5yZUZTYy9z?= =?utf-8?B?ZDd4OVVuTWx3M0hIZklCSW1TNEtQS0FJZXJWQkhJSGNuT3o1ZlJaWlU3Y1Z3?= =?utf-8?B?OWFkNC9rL0l5dmtDdWtmN09XbG9NVXFDNUQyaW9Lb3pTZEVnZVNueHoyd1NP?= =?utf-8?B?a2JoQTZmWlFiSVBoWElhY0xkSDUwaEFpTE1Ma3REcUlmMkVWayticE9VNC96?= =?utf-8?B?ZE1BYmtiZktJRGpGTk9MVVJBVE9Mb21XVHQ1RFB1K2g4d3lObWRUYVhGaTJj?= =?utf-8?B?aktGV3BYVS9wcEVUOFdRYVFIbU9UVGFhNTBzcE1ZZDNYRmZHM3RzQXJHYWtK?= =?utf-8?B?c1cxb0ZPVUFCWlVxaUhjcDVlVy9qK2RjTHFrbVJJUmRrK0NTK3Y5N3o5UlRw?= =?utf-8?B?UlJLZzZKUTUvZE1kTlZLQUplUEc5K1FBcE4wUWhsM21lMVVOc3ZoWWRoSXhL?= =?utf-8?B?cUJhQVZIRUlReUpXVmpsSEVNVExzSGUrVms4cEQ4eDRacVcvSFRpNXF2Smk0?= =?utf-8?B?bmtuTkRNb205U2FSOE9EcS81K3Z1dVRQbjlrc3BCYytMdEc4L2I3WVlPY2xC?= =?utf-8?B?WlhLWE5CYXVaWEl2Y285TUtqcEVyenlWNmZJU29iQVF3Y0NvdHpaUWtiWGh3?= =?utf-8?B?Vm9wNkhiUXV0dEgwb2dkZ0ZWSCsyRTdsWEdkM1VVbHRYTHpCYTYvNE5EM08z?= =?utf-8?B?eDhMOVF2enJ6T0VjVmcvN2twOC9OT3pRV0pRYmtUUFZnbW1kRUxMZ2RTNktH?= =?utf-8?B?dHY0MnpGaTFOdnRzMnhqQ213YmVqelJaTEorallPSnpRaGxRd2tSZCtIRW5H?= =?utf-8?B?VVJJdXltdC9pV2hZdFVHRGtYMHlBVFVmTzBOM0JUVHdTZVcvUW1ZNTdVeVRW?= =?utf-8?B?eml3Y0hUbFFOYVBFQlNQaGoralJWb0FheHdFc0NDT094ZWlQVUlFRWlBUXRw?= =?utf-8?B?aDByN05taUxTQ2xKY1ZCTkNiMHJIMWpzLy83TGlVV05XdUpjTEVNRk0rVGxh?= =?utf-8?B?ckhtdWY4eUdZa0xLdmkzSXNOcUljUDV0UTFTb1lCT0JzcGtkT3UwejJpdTkv?= =?utf-8?Q?xG0i+fZU/d9E/jJmDXZwnNp2y?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: dcb16a4e-70bb-4c8d-1ba4-08db5602c9f3 X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB4758.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2023 11:43:38.4685 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 4Tb/d1JUzh+zmw9De/Kmsyr5GbyqP2q/3vPmZlDVS6ojjInYEFyHjKiggZ5ht8Pc X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR12MB6292 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On 5/16/23 13:23, Stefan Herbrechtsmeier wrote: > Hi, > > Am 16.05.2023 um 10:26 schrieb Michal Simek: > >> Hi, >> >> first of all sorry for delay. >> >> On 4/27/23 12:31, Stefan Herbrechtsmeier wrote: >>> From: Stefan Herbrechtsmeier >>> >>> Move the check of the permission to change a config object from >>> zynqmp_pmufw_node function to zynqmp_pmufw_load_config_object function >>> to simplify the code and check the permission only if required. >>> >>> Signed-off-by: Stefan Herbrechtsmeier >>> >>> --- >>> >>> Changes in v3: >>> - Added >>> >>>   drivers/firmware/firmware-zynqmp.c | 32 +++++++++++++++--------------- >>>   1 file changed, 16 insertions(+), 16 deletions(-) >>> >>> diff --git a/drivers/firmware/firmware-zynqmp.c >>> b/drivers/firmware/firmware-zynqmp.c >>> index 2b1ad5d2c3..d12159fa78 100644 >>> --- a/drivers/firmware/firmware-zynqmp.c >>> +++ b/drivers/firmware/firmware-zynqmp.c >>> @@ -70,20 +70,11 @@ int zynqmp_pmufw_config_close(void) >>>     int zynqmp_pmufw_node(u32 id) >>>   { >>> -    static bool skip_config; >>> -    int ret; >>> - >>> -    if (skip_config) >>> -        return 0; >>> - >>>       /* Record power domain id */ >>>       xpm_configobject[NODE_ID_LOCATION] = id; >>>   -    ret = zynqmp_pmufw_load_config_object(xpm_configobject, >>> -                          sizeof(xpm_configobject)); >>> - >>> -    if (ret == -EACCES && id == NODE_OCM_BANK_0) >>> -        skip_config = true; >>> +    zynqmp_pmufw_load_config_object(xpm_configobject, >>> +                    sizeof(xpm_configobject)); >> >> This is not right. >> It should be >> return zynqmp_pmufw_load... for error propagation. > > At the moment the zynqmp_pmufw_node and zynqmp_pmufw_config_close doesn't return > an error. Should the zynqmp_pmufw_load_config_object return 0 or -EACCES if it > is skipped? In context zynqmp_pmufw_node and it's dependency on returning EACESS for failure case which your code depends on here + if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) { + printf("PMUFW: No permission to change config object\n"); + skip = true; + } And for second part around skip and return code. I would say what you have is fine. It means returning -EACCES is appropriate here. And as I see do_zynqmp_pmufw should also return that value. That command will simply fail if there is no permission. And for close part I would say the same. Error should be propagated. I expect current command behavior when you call "pmufw node close" on regular system will just pass but it should just fail because command wasn't successful. Thanks, Michal