From: Reiji Watanabe <reijiw@google.com> To: Jing Zhang <jingzhangos@google.com> Cc: Marc Zyngier <maz@kernel.org>, KVM <kvm@vger.kernel.org>, KVMARM <kvmarm@lists.linux.dev>, ARMLinux <linux-arm-kernel@lists.infradead.org>, Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, James Morse <james.morse@arm.com>, Alexandru Elisei <alexandru.elisei@arm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Fuad Tabba <tabba@google.com>, Ricardo Koller <ricarkol@google.com>, Raghavendra Rao Ananta <rananta@google.com> Subject: Re: [PATCH v4 2/6] KVM: arm64: Save ID registers' sanitized value per guest Date: Wed, 29 Mar 2023 09:26:13 -0700 [thread overview] Message-ID: <CAAeT=FyJyip9NOhTjdh169+9jE_eP3uEHMTszEQa7VfUY9MS1Q@mail.gmail.com> (raw) In-Reply-To: <CAAdAUtjp1tdyadjtU0RrdsRq-D3518G8eqP_coYNJ1vFEvrz2Q@mail.gmail.com> > > > +/* > > > + * Set the guest's ID registers that are defined in id_reg_descs[] > > > + * with ID_SANITISED() to the host's sanitized value. > > > + */ > > > +void kvm_arm_set_default_id_regs(struct kvm *kvm) > > > +{ > > > + int i; > > > + u32 id; > > > + u64 val; > > > + > > > + for (i = 0; i < ARRAY_SIZE(id_reg_descs); i++) { > > > + id = reg_to_encoding(&id_reg_descs[i]); > > > + if (WARN_ON_ONCE(!is_id_reg(id))) > > > + /* Shouldn't happen */ > > > + continue; > > > + > > > + if (id_reg_descs[i].visibility == raz_visibility) > > > + /* Hidden or reserved ID register */ > > > + continue; > > > > Relying on function pointer comparison is really fragile. If I wrap > > raz_visibility() in another function, this won't catch it. It also > > doesn't bode well with your 'inline' definition of this function. > > > > More importantly, why do we care about checking for visibility at all? > > We can happily populate the array and rely on the runtime visibility. > Right. I'll remove this checking. Without the check, calling read_sanitised_ftr_reg() for some hidden ID registers will show a warning as some of them are not in arm64_ftr_regs[] (e.g. reserved ones). This checking is needed temporarily to avoid the warning (the check is removed in the following patches of this series). It would be much less fragile to call the visibility function instead, but I don't think this is a also good way to check the availability of the sanitized values for ID registers either. I didn't found a good (proper) way to check that without making changes in cpufeature.c, and I'm not sure if it is worth it for this temporary purpose. Thank you, Reiji > > > > > + > > > + val = read_sanitised_ftr_reg(id); > > > + kvm->arch.id_regs[IDREG_IDX(id)] = val; > > > + } > > > +} > > > > Thanks, > > > > M. > > > > -- > > Without deviation from the norm, progress is not possible. > > Thanks, > Jing
WARNING: multiple messages have this Message-ID (diff)
From: Reiji Watanabe <reijiw@google.com> To: Jing Zhang <jingzhangos@google.com> Cc: Marc Zyngier <maz@kernel.org>, KVM <kvm@vger.kernel.org>, KVMARM <kvmarm@lists.linux.dev>, ARMLinux <linux-arm-kernel@lists.infradead.org>, Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, James Morse <james.morse@arm.com>, Alexandru Elisei <alexandru.elisei@arm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Fuad Tabba <tabba@google.com>, Ricardo Koller <ricarkol@google.com>, Raghavendra Rao Ananta <rananta@google.com> Subject: Re: [PATCH v4 2/6] KVM: arm64: Save ID registers' sanitized value per guest Date: Wed, 29 Mar 2023 09:26:13 -0700 [thread overview] Message-ID: <CAAeT=FyJyip9NOhTjdh169+9jE_eP3uEHMTszEQa7VfUY9MS1Q@mail.gmail.com> (raw) In-Reply-To: <CAAdAUtjp1tdyadjtU0RrdsRq-D3518G8eqP_coYNJ1vFEvrz2Q@mail.gmail.com> > > > +/* > > > + * Set the guest's ID registers that are defined in id_reg_descs[] > > > + * with ID_SANITISED() to the host's sanitized value. > > > + */ > > > +void kvm_arm_set_default_id_regs(struct kvm *kvm) > > > +{ > > > + int i; > > > + u32 id; > > > + u64 val; > > > + > > > + for (i = 0; i < ARRAY_SIZE(id_reg_descs); i++) { > > > + id = reg_to_encoding(&id_reg_descs[i]); > > > + if (WARN_ON_ONCE(!is_id_reg(id))) > > > + /* Shouldn't happen */ > > > + continue; > > > + > > > + if (id_reg_descs[i].visibility == raz_visibility) > > > + /* Hidden or reserved ID register */ > > > + continue; > > > > Relying on function pointer comparison is really fragile. If I wrap > > raz_visibility() in another function, this won't catch it. It also > > doesn't bode well with your 'inline' definition of this function. > > > > More importantly, why do we care about checking for visibility at all? > > We can happily populate the array and rely on the runtime visibility. > Right. I'll remove this checking. Without the check, calling read_sanitised_ftr_reg() for some hidden ID registers will show a warning as some of them are not in arm64_ftr_regs[] (e.g. reserved ones). This checking is needed temporarily to avoid the warning (the check is removed in the following patches of this series). It would be much less fragile to call the visibility function instead, but I don't think this is a also good way to check the availability of the sanitized values for ID registers either. I didn't found a good (proper) way to check that without making changes in cpufeature.c, and I'm not sure if it is worth it for this temporary purpose. Thank you, Reiji > > > > > + > > > + val = read_sanitised_ftr_reg(id); > > > + kvm->arch.id_regs[IDREG_IDX(id)] = val; > > > + } > > > +} > > > > Thanks, > > > > M. > > > > -- > > Without deviation from the norm, progress is not possible. > > Thanks, > Jing _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-03-29 16:26 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-17 5:06 [PATCH v4 0/6] Support writable CPU ID registers from userspace Jing Zhang 2023-03-17 5:06 ` Jing Zhang 2023-03-17 5:06 ` [PATCH v4 1/6] KVM: arm64: Move CPU ID feature registers emulation into a separate file Jing Zhang 2023-03-17 5:06 ` Jing Zhang 2023-03-27 10:14 ` Marc Zyngier 2023-03-27 10:14 ` Marc Zyngier 2023-03-28 17:16 ` Jing Zhang 2023-03-28 17:16 ` Jing Zhang 2023-03-17 5:06 ` [PATCH v4 2/6] KVM: arm64: Save ID registers' sanitized value per guest Jing Zhang 2023-03-17 5:06 ` Jing Zhang 2023-03-27 10:15 ` Marc Zyngier 2023-03-27 10:15 ` Marc Zyngier 2023-03-28 17:36 ` Jing Zhang 2023-03-28 17:36 ` Jing Zhang 2023-03-28 19:22 ` Marc Zyngier 2023-03-28 19:22 ` Marc Zyngier 2023-03-28 20:05 ` Jing Zhang 2023-03-28 20:05 ` Jing Zhang 2023-03-29 16:26 ` Reiji Watanabe [this message] 2023-03-29 16:26 ` Reiji Watanabe 2023-03-17 5:06 ` [PATCH v4 3/6] KVM: arm64: Use per guest ID register for ID_AA64PFR0_EL1.[CSV2|CSV3] Jing Zhang 2023-03-17 5:06 ` Jing Zhang 2023-03-27 10:31 ` Marc Zyngier 2023-03-27 10:31 ` Marc Zyngier 2023-03-28 19:54 ` Jing Zhang 2023-03-28 19:54 ` Jing Zhang 2023-03-28 12:39 ` Fuad Tabba 2023-03-28 12:39 ` Fuad Tabba 2023-03-28 20:01 ` Jing Zhang 2023-03-28 20:01 ` Jing Zhang 2023-03-29 8:23 ` Fuad Tabba 2023-03-29 8:23 ` Fuad Tabba 2023-03-17 5:06 ` [PATCH v4 4/6] KVM: arm64: Use per guest ID register for ID_AA64DFR0_EL1.PMUVer Jing Zhang 2023-03-17 5:06 ` Jing Zhang 2023-03-27 10:40 ` Marc Zyngier 2023-03-27 10:40 ` Marc Zyngier 2023-03-28 20:20 ` Jing Zhang 2023-03-28 20:20 ` Jing Zhang 2023-03-17 5:06 ` [PATCH v4 5/6] KVM: arm64: Introduce ID register specific descriptor Jing Zhang 2023-03-17 5:06 ` Jing Zhang 2023-03-27 11:28 ` Marc Zyngier 2023-03-27 11:28 ` Marc Zyngier 2023-03-29 3:46 ` Jing Zhang 2023-03-29 3:46 ` Jing Zhang 2023-03-17 5:06 ` [PATCH v4 6/6] KVM: arm64: Refactor writings for PMUVer/CSV2/CSV3 Jing Zhang 2023-03-17 5:06 ` Jing Zhang 2023-03-27 13:34 ` Marc Zyngier 2023-03-27 13:34 ` Marc Zyngier 2023-03-29 4:29 ` Jing Zhang 2023-03-29 4:29 ` Jing Zhang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAAeT=FyJyip9NOhTjdh169+9jE_eP3uEHMTszEQa7VfUY9MS1Q@mail.gmail.com' \ --to=reijiw@google.com \ --cc=alexandru.elisei@arm.com \ --cc=james.morse@arm.com \ --cc=jingzhangos@google.com \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.linux.dev \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=maz@kernel.org \ --cc=oupton@google.com \ --cc=pbonzini@redhat.com \ --cc=rananta@google.com \ --cc=ricarkol@google.com \ --cc=suzuki.poulose@arm.com \ --cc=tabba@google.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.