Re: [PATCH v2] target/s390x/kvm: Enable adapter interruption suppression again

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 20 Jan 2020 10:49:01 +0100
Thomas Huth <thuth@redhat.com> wrote:

> The AIS feature has been disabled late in the v2.10 development cycle since
> there were some issues with migration (see commit 3f2d07b3b01ea61126b -
> "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted
> to enable it again for newer machine types, but apparently we forgot to do
> this so far. Let's do it for the new s390-ccw-virtio-5.0 machine now.
> 
> While at it, also add a more verbose comment why we need the *_allowed()
> wrappers in s390-virtio-ccw.c.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  Matthew, could you please give this another try on your system? Thanks!
> 
>  hw/s390x/s390-virtio-ccw.c         | 20 +++++++++++++++++---
>  include/hw/s390x/s390-virtio-ccw.h |  3 +++
>  target/s390x/kvm.c                 |  9 ++++++---
>  3 files changed, 26 insertions(+), 6 deletions(-)
> 

> @@ -658,6 +669,9 @@ static void ccw_machine_4_2_instance_options(MachineState *machine)
>  
>  static void ccw_machine_4_2_class_options(MachineClass *mc)
>  {
> +    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
> +
> +    s390mc->kvm_ais_allowed = false;
>      ccw_machine_5_0_class_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);

One thing I've noticed: You set the _allowed value to false, and
afterwards apply the options from any 'later' class; this is the same
order as for the other _allowed values. There's also
css_migration_enabled, which is set to false _after_ the class options
from 'later' classes have been applied.

Both variants end up the same, as we only ever set the value to true in
the base class and to false just in a single class option callback; but
I wonder whether it would be cleaner to set it to false after the other
options have been applied. Or am I thinking backwards here?

>  }