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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id C11C3C47077 for ; Tue, 16 Jan 2024 16:21:14 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rPmB5-0007xT-0x; Tue, 16 Jan 2024 11:20:39 -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 1rPmAo-0007v5-4w for qemu-devel@nongnu.org; Tue, 16 Jan 2024 11:20:22 -0500 Received: from mail-il1-x12c.google.com ([2607:f8b0:4864:20::12c]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rPmAm-0001Tx-78 for qemu-devel@nongnu.org; Tue, 16 Jan 2024 11:20:21 -0500 Received: by mail-il1-x12c.google.com with SMTP id e9e14a558f8ab-3606dd96868so54117775ab.2 for ; Tue, 16 Jan 2024 08:20:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1705422018; x=1706026818; darn=nongnu.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=yMx2L1+o6Lxao8lN2+Iy4S7RzUqY4Zi08OgW0aT2u7o=; b=zQQElSSC44sn70nZo9jswMuMz17YZ2RXiekV7AjMjLVviMlGRXghQ/KtZyltWtxgee 9rVH3ECi1qFLcP0d3MZFl7uTqzQmALQYmXCUzh2AO4p1Oo9/R+TOjudrpwqCUtjrkJbq 5WAgTfKtp2bOFnVuLWV9KV26bHd6AJ0iKrLv9sfmO082SlFJgZPj7MFAIiQtxfZ2YIxK P8HUY4DgUYwTzgv4ht3JUD0YoGk1DAtDD+gyqEE93THl2nIvRRr2ckAZ43xZihhm5KbV lshr9g8k1aNc2gL0HY8UdfizYPJioaa80YEwgfpfexLJA0TdE8gbSUlsXQ+DieCh5fJI V/Wg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705422018; x=1706026818; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=yMx2L1+o6Lxao8lN2+Iy4S7RzUqY4Zi08OgW0aT2u7o=; b=sYBYHK8zv3Atxr5gNB3hkrMt7dKxVfAuX9rWoD9oaoYajDZMrW3hcHbR+RuJ5ChICB aEhlQz7bsJoutTVBLD93mqOkfGg4Yr0VJoCntx7dlyearLmVogENw/8t22XD5A+R/rli PiyVFRR+zIvj9ze6lWwgE/6f8JHoulidIRucXelKFeCEqkJoHjEuv5irkbi6EQdx0CQV d++s1US+fXpwoLLe6MdiDRb/UZUa0hpuv24uqj+6zeon0owf8T70oSBJmhqw90LAsyeJ oUWcCqyfbJc+4bRPWC0NEDkIjjldwqoPpbLr3hiHxd8lNTCKiCuuIxwp8fKaif3e2iIF 7Ccg== X-Gm-Message-State: AOJu0YzddViAD76lLM/nsR9/l50SfHbBPUnpJxkW94psBX4aDHbbx39h xI59mATw5v+dzqmTVlpIrZ/wrx+u0qfUeS20qzZUoNyC4yo= X-Google-Smtp-Source: AGHT+IEQ5OV5BIC2q15KsebeioDvRsWyXEhHD2P4ji6M5ruw5D6eMht9npmzQDIi+yPPSXRTNWXcAQ== X-Received: by 2002:a05:6e02:510:b0:361:923b:d79e with SMTP id d16-20020a056e02051000b00361923bd79emr510337ils.85.1705422018354; Tue, 16 Jan 2024 08:20:18 -0800 (PST) Received: from [192.168.1.102] ([176.176.156.199]) by smtp.gmail.com with ESMTPSA id y1-20020a056e02128100b0035fea0268bcsm3607268ilq.28.2024.01.16.08.20.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jan 2024 08:20:18 -0800 (PST) Message-ID: <7a73231a-d7ae-4abe-9bca-e8e295a5f70d@linaro.org> Date: Tue, 16 Jan 2024 17:20:13 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 00/14] hw/arm: Prefer arm_feature() over object_property_find() Content-Language: en-US To: Peter Maydell Cc: qemu-devel@nongnu.org, Marcin Juszkiewicz , qemu-arm@nongnu.org, Kevin Wolf , Igor Mitsyanko , =?UTF-8?Q?Alex_Benn=C3=A9e?= , Radoslaw Biernacki , "Edgar E. Iglesias" , Leif Lindholm , Rob Herring , Markus Armbruster , Alistair Francis References: <20240110195329.3995-1-philmd@linaro.org> From: =?UTF-8?Q?Philippe_Mathieu-Daud=C3=A9?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2607:f8b0:4864:20::12c; envelope-from=philmd@linaro.org; helo=mail-il1-x12c.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, 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-devel@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-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On 13/1/24 14:36, Peter Maydell wrote: > On Wed, 10 Jan 2024 at 19:53, Philippe Mathieu-Daudé wrote: >> >> Since v2 [2]: >> - Dropped "Simplify checking A64_MTE bit in FEATURE_ID register" >> - Correct object_property_get_bool() uses >> - Update ARM_FEATURE_AARCH64 && aa64_mte >> >> Since RFC [1]: >> - Split one patch per feature >> - Addressed Peter's review comments >> >> [1] https://lore.kernel.org/qemu-devel/20231214171447.44025-1-philmd@linaro.org/ >> [2] https://lore.kernel.org/qemu-devel/20240109180930.90793-1-philmd@linaro.org/ >> >> Based-on: <20231123143813.42632-1-philmd@linaro.org> >> "hw: Simplify accesses to CPUState::'start-powered-off' property" >> >> Philippe Mathieu-Daudé (14): >> hw/arm/armv7m: Introduce cpudev variable in armv7m_realize() >> hw/arm/armv7m: Ensure requested CPU type implements ARM_FEATURE_M >> hw/arm/armv7m: Move code setting 'start-powered-off' property around >> hw/arm/armv7m: Always set 'init-nsvtor' property for Cortex-M CPUs > The first part of this is fine and reasonable cleanup, but I > continue to disagree about the later parts. What we want to do is > "if this property is present, then set it", and that's what we do. > Conversion to "if decide whether to define the property> then set it" is duplicating > the condition logic in two places and opening the door for bugs > where we change the condition in one place and not in the other. > Where the is a simple "feature X is set" it doesn't > look too bad, but where it gets more complex it makes it IMHO more > obvious that this is a bad idea, for example with: > > - if (object_property_find(cpuobj, "reset-cbar")) { > + if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) || > + arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) { For that we could add helpers such static inline bool arm_feature_cbar(CPUState *cpu) { return arm_feature(&cpu->env, ARM_FEATURE_CBAR) || arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO); } and use it in the target/ code where we register the property and in the hw/ code where we set it. Anyway I'll wait to see how Kevin's effort is going before insisting with this series. Do you mind taking the cleanup patches (1-4) meanwhile? Thanks, Phil.