On Fri, Feb 03 2023, Richard Henderson <richard.henderson@xxxxxxxxxx> wrote:
> On 2/3/23 03:44, Cornelia Huck wrote:
>> +static inline bool arm_machine_has_tag_memory(void)
>> +{
>> +#ifndef CONFIG_USER_ONLY
>> + Object *obj = object_dynamic_cast(qdev_get_machine(), TYPE_VIRT_MACHINE);
>> +
>> + /* so far, only the virt machine has support for tag memory */
>> + if (obj) {
>> + VirtMachineState *vms = VIRT_MACHINE(obj);
>
> VIRT_MACHINE() does object_dynamic_cast_assert, and we've just done that.
>
> As this is startup, it's not the speed that matters. But it does look unfortunate. Not
> for this patch set, but perhaps we ought to add TRY_OBJ_NAME to DECLARE_INSTANCE_CHECKER?
Instead of the pattern above, we could also do
VirtMachineState *vms = (VirtMachineState *) object_dynamic_cast(...);
if (vms) {
(...)
>
>> +void arm_cpu_mte_finalize(ARMCPU *cpu, Error **errp)
>> +{
>> + bool enable_mte;
>> +
>> + switch (cpu->prop_mte) {
>> + case ON_OFF_AUTO_OFF:
>> + enable_mte = false;
>> + break;
>> + case ON_OFF_AUTO_ON:
>> + if (tcg_enabled()) {
>> + if (cpu_isar_feature(aa64_mte, cpu)) {
>> + if (!arm_machine_has_tag_memory()) {
>> + error_setg(errp, "mte=on requires tag memory");
>> + return;
>> + }
>> + } else {
>> + error_setg(errp, "mte not supported by this CPU type");
>> + return;
>> + }
>> + }
>> + if (kvm_enabled() && !kvm_arm_mte_supported()) {
>> + error_setg(errp, "mte not supported by kvm");
>> + return;
>> + }
>> + enable_mte = true;
>> + break;
>
> What's here is not wrong, but maybe better structured as
>
> enable_mte = true;
> if (qtest_enabled()) {
> break;
> }
> if (tcg_enabled()) {
> if (arm_machine_tag_mem) {
> break;
> }
> error;
> return;
> }
> if (kvm_enabled() && kvm_arm_mte_supported) {
> break;
> }
> error("mte not supported by %s", current_accel_type());
> return;
That's indeed better, as we also see what's going on for the different
accelarators.
> We only add the property for tcg via -cpu max, so the isar check is redundant.
