Re: [PATCH 15/16] kvm: introduce kvm_kernel_irqchip_* functions

[Thomas Huth]

On 13/11/2019 15.39, Paolo Bonzini wrote:
> The KVMState struct is opaque, so provide accessors for the fields
> that will be moved from current_machine to the accelerator.  For now
> they just forward to the machine object, but this will change.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  accel/kvm/kvm-all.c  | 23 +++++++++++++++++++----
>  hw/ppc/e500.c        |  4 ++--
>  hw/ppc/spapr_irq.c   | 10 +++++-----
>  include/sysemu/kvm.h |  7 +++++--
>  target/arm/kvm.c     |  8 ++++----
>  target/i386/kvm.c    |  4 ++--
>  target/mips/kvm.c    |  2 +-
>  target/ppc/kvm.c     |  2 +-
>  target/s390x/kvm.c   |  2 +-
>  9 files changed, 40 insertions(+), 22 deletions(-)
[...]
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index b941608..8b66eb9 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -77,9 +77,9 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController 
> *, Error **),
>      MachineState *machine = MACHINE(qdev_get_machine());
>      Error *local_err = NULL;
>  
> -    if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> +    if (kvm_enabled() && kvm_kernel_irqchip_allowed()) {
>          if (fn(intc, &local_err) < 0) {
> -            if (machine_kernel_irqchip_required(machine)) {
> +            if (kvm_kernel_irqchip_required()) {
>                  error_prepend(&local_err,
>                                "kernel_irqchip requested but unavailable: ");
>                  error_propagate(errp, local_err);
> @@ -184,7 +184,7 @@ static int spapr_irq_check(SpaprMachineState *spapr, 
> Error **errp)
>       */
>      if (kvm_enabled() &&
>          spapr->irq == &spapr_irq_dual &&
> -        machine_kernel_irqchip_required(machine) &&
> +        kvm_kernel_irqchip_required() &&
>          xics_kvm_has_broken_disconnect(spapr)) {
>          error_setg(errp, "KVM is too old to support 
> ic-mode=dual,kernel-irqchip=on");
>          return -1;
> @@ -276,12 +276,12 @@ void spapr_irq_init(SpaprMachineState *spapr, Error 
> **errp)
>      MachineState *machine = MACHINE(spapr);
>      SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>  
> -    if (machine_kernel_irqchip_split(machine)) {
> +    if (kvm_enabled() && kvm_kernel_irqchip_split()) {
>          error_setg(errp, "kernel_irqchip split mode not supported on 
> pseries");
>          return;
>      }

Any reason for the additional kvm_enabled() here? I think it should also
be ok without that?

Apart from that question, patch looks fine to me.

 Thomas