Re: [Qemu-devel] [RFC PATCH v3 18/24] xics_kvm: Don't enable KVM_CAP_IRQ_XICS if already enabled

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

On Fri, Apr 24, 2015 at 12:17:40PM +0530, Bharata B Rao wrote:
> When supporting CPU hot removal by parking the vCPU fd and reusing
> it during hotplug again, there can be cases where we try to reenable
> KVM_CAP_IRQ_XICS CAP for the vCPU for which it was already enabled.
> Introduce a boolean member in ICPState to track this and don't
> reenable the CAP if it was already enabled earlier.
> 
> This change allows CPU hot removal to work for sPAPR.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Do you actually need this?  Is there any harm in setting the
capability multiple times, or could you just ignore the "already set"
error?

> ---
>  hw/intc/xics_kvm.c    | 10 ++++++++++
>  include/hw/ppc/xics.h |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index c15453f..5b27bf8 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -331,6 +331,15 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>          abort();
>      }
>  
> +    /*
> +     * If we are reusing a parked vCPU fd corresponding to the CPU
> +     * which was hot-removed earlier we don't have to renable
> +     * KVM_CAP_IRQ_XICS capability again.
> +     */
> +    if (ss->cap_irq_xics_enabled) {
> +        return;
> +    }
> +
>      if (icpkvm->kernel_xics_fd != -1) {
>          int ret;
>  
> @@ -343,6 +352,7 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>                      kvm_arch_vcpu_id(cs), strerror(errno));
>              exit(1);
>          }
> +        ss->cap_irq_xics_enabled = true;
>      }
>  }
>  
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index a214dd7..355a966 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -109,6 +109,7 @@ struct ICPState {
>      uint8_t pending_priority;
>      uint8_t mfrr;
>      qemu_irq output;
> +    bool cap_irq_xics_enabled;
>  };
>  
>  #define TYPE_ICS "ics"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]