On 23.01.20 18:02, Thomas Huth wrote:
> While working on the "Enable adapter interruption suppression again"
> recently, I had to discover that the meaning of get_machine_class()
> and the related *_allowed() wrappers is not very obvious. Add a more
> verbose comment here to clarify how these should be used.
>
> Signed-off-by: Thomas Huth <th...@redhat.com>
> ---
> hw/s390x/s390-virtio-ccw.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index e0e28139a2..7fb389f0e5 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -505,6 +505,18 @@ static inline void machine_set_dea_key_wrap(Object *obj,
> bool value,
>
> static S390CcwMachineClass *current_mc;
>
> +/*
> + * Get the class of the s390-ccw-virtio machine that is currently in use.
> + * Note: libvirt is using the "none" machine to probe for the features of the
> + * host CPU, so in case this is called with the "none" machine, the function
> + * returns the TYPE_S390_CCW_MACHINE base class. In this base class, all the
> + * various "*_allowed" variables are enabled, so that the *_allowed()
> wrappers
> + * below return the correct default value for the "none" machine.
> + * Attention! Do *not* add additional new wrappers for CPU features (e.g.
> like
> + * the ri_allowed() wrapper) via this mechanism anymore. CPU features should
> + * be handled via the CPU models, i.e. checking with cpu_model_allowed()
> during
> + * CPU initialization and s390_has_feat() later should be sufficient.
> + */
> static S390CcwMachineClass *get_machine_class(void)
> {
> if (unlikely(!current_mc)) {
> @@ -521,19 +533,16 @@ static S390CcwMachineClass *get_machine_class(void)
>
> bool ri_allowed(void)
> {
> - /* for "none" machine this results in true */
> return get_machine_class()->ri_allowed;
> }
>
> bool cpu_model_allowed(void)
> {
> - /* for "none" machine this results in true */
> return get_machine_class()->cpu_model_allowed;
> }
>
> bool hpage_1m_allowed(void)
> {
> - /* for "none" machine this results in true */
> return get_machine_class()->hpage_1m_allowed;
> }
>
>
Very helpful, thanks!
Reviewed-by: David Hildenbrand <da...@redhat.com>
--
Thanks,
David / dhildenb
