Re: [Qemu-devel] [PATCH v5 06/17] xics: Write source state to KVM at claim time

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

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

On Tue, Feb 19, 2019 at 06:18:03PM +0100, Greg Kurz wrote:
> The pseries machine only uses LSIs to support legacy PCI devices. Every
> PHB claims 4 LSIs at realize time. When using in-kernel XICS (or upcoming
> in-kernel XIVE), QEMU synchronizes the state of all irqs, including these
> LSIs, later on at machine reset.
> 
> In order to support PHB hotplug, we need a way to tell KVM about the LSIs
> that doesn't require a machine reset. An easy way to do that is to always
> inform KVM when an interrupt is claimed, which really isn't a performance
> path.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied, thanks.

> ---
>  hw/intc/xics.c        |    4 +++
>  hw/intc/xics_kvm.c    |   74 ++++++++++++++++++++++++++++---------------------
>  include/hw/ppc/xics.h |    1 +
>  3 files changed, 48 insertions(+), 31 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 767fdeb82900..af7dc709abab 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -758,6 +758,10 @@ void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
>  
>      ics->irqs[srcno].flags |=
>          lsi ? XICS_FLAGS_IRQ_LSI : XICS_FLAGS_IRQ_MSI;
> +
> +    if (kvm_irqchip_in_kernel()) {
> +        ics_set_kvm_state_one(ics, srcno);
> +    }
>  }
>  
>  static void xics_register_types(void)
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index a00d0a7962e1..c6e1b630a404 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -213,45 +213,57 @@ void ics_synchronize_state(ICSState *ics)
>      ics_get_kvm_state(ics);
>  }
>  
> -int ics_set_kvm_state(ICSState *ics)
> +int ics_set_kvm_state_one(ICSState *ics, int srcno)
>  {
>      uint64_t state;
> -    int i;
>      Error *local_err = NULL;
> +    ICSIRQState *irq = &ics->irqs[srcno];
> +    int ret;
>  
> -    for (i = 0; i < ics->nr_irqs; i++) {
> -        ICSIRQState *irq = &ics->irqs[i];
> -        int ret;
> -
> -        state = irq->server;
> -        state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> -            << KVM_XICS_PRIORITY_SHIFT;
> -        if (irq->priority != irq->saved_priority) {
> -            assert(irq->priority == 0xff);
> -            state |= KVM_XICS_MASKED;
> -        }
> +    state = irq->server;
> +    state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> +        << KVM_XICS_PRIORITY_SHIFT;
> +    if (irq->priority != irq->saved_priority) {
> +        assert(irq->priority == 0xff);
> +        state |= KVM_XICS_MASKED;
> +    }
>  
> -        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
> -            state |= KVM_XICS_LEVEL_SENSITIVE;
> -            if (irq->status & XICS_STATUS_ASSERTED) {
> -                state |= KVM_XICS_PENDING;
> -            }
> -        } else {
> -            if (irq->status & XICS_STATUS_MASKED_PENDING) {
> -                state |= KVM_XICS_PENDING;
> -            }
> +    if (irq->flags & XICS_FLAGS_IRQ_LSI) {
> +        state |= KVM_XICS_LEVEL_SENSITIVE;
> +        if (irq->status & XICS_STATUS_ASSERTED) {
> +            state |= KVM_XICS_PENDING;
>          }
> -        if (irq->status & XICS_STATUS_PRESENTED) {
> -                state |= KVM_XICS_PRESENTED;
> -        }
> -        if (irq->status & XICS_STATUS_QUEUED) {
> -                state |= KVM_XICS_QUEUED;
> +    } else {
> +        if (irq->status & XICS_STATUS_MASKED_PENDING) {
> +            state |= KVM_XICS_PENDING;
>          }
> +    }
> +    if (irq->status & XICS_STATUS_PRESENTED) {
> +        state |= KVM_XICS_PRESENTED;
> +    }
> +    if (irq->status & XICS_STATUS_QUEUED) {
> +        state |= KVM_XICS_QUEUED;
> +    }
> +
> +    ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> +                            srcno + ics->offset, &state, true, &local_err);
> +    if (local_err) {
> +        error_report_err(local_err);
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +int ics_set_kvm_state(ICSState *ics)
> +{
> +    int i;
> +
> +    for (i = 0; i < ics->nr_irqs; i++) {
> +        int ret;
>  
> -        ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> -                                i + ics->offset, &state, true, &local_err);
> -        if (local_err) {
> -            error_report_err(local_err);
> +        ret = ics_set_kvm_state_one(ics, i);
> +        if (ret) {
>              return ret;
>          }
>      }
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index d36bbe11ee2e..eb65ad7e43b7 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -195,6 +195,7 @@ void icp_synchronize_state(ICPState *icp);
>  void icp_kvm_realize(DeviceState *dev, Error **errp);
>  
>  void ics_get_kvm_state(ICSState *ics);
> +int ics_set_kvm_state_one(ICSState *ics, int srcno);
>  int ics_set_kvm_state(ICSState *ics);
>  void ics_synchronize_state(ICSState *ics);
>  void ics_kvm_set_irq(ICSState *ics, int srcno, int val);
> 

-- 
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]