Re: [PATCH] spapr/xive: Add source status helpers

[Greg Kurz <groug@kaod.org>]

On Mon, 4 Oct 2021 23:21:41 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> and use them to set and test the ASSERTED bit of LSI sources.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  include/hw/ppc/xive.h    | 24 ++++++++++++++++++++++++
>  hw/intc/spapr_xive.c     |  2 +-
>  hw/intc/spapr_xive_kvm.c | 10 +++-------
>  hw/intc/xive.c           |  8 ++++----
>  4 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 252c58a1d691..b8ab0bf7490f 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -286,6 +286,30 @@ uint8_t xive_esb_set(uint8_t *pq, uint8_t value);
>  uint8_t xive_source_esb_get(XiveSource *xsrc, uint32_t srcno);
>  uint8_t xive_source_esb_set(XiveSource *xsrc, uint32_t srcno, uint8_t pq);
>  
> +/*
> + * Source status helpers
> + */
> +static inline void xive_source_set_status(XiveSource *xsrc, uint32_t srcno,
> +                                          uint8_t status, bool enable)
> +{
> +    if (enable) {
> +        xsrc->status[srcno] |= status;
> +    } else {
> +        xsrc->status[srcno] &= ~status;
> +    }
> +}
> +
> +static inline void xive_source_set_asserted(XiveSource *xsrc, uint32_t srcno,
> +                                            bool enable)
> +{
> +    xive_source_set_status(xsrc, srcno, XIVE_STATUS_ASSERTED, enable);
> +}
> +
> +static inline bool xive_source_is_asserted(XiveSource *xsrc, uint32_t srcno)
> +{
> +    return xsrc->status[srcno] & XIVE_STATUS_ASSERTED;
> +}
> +
>  void xive_source_pic_print_info(XiveSource *xsrc, uint32_t offset,
>                                  Monitor *mon);
>  
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 89cfa018f598..4ec659b93e13 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -185,7 +185,7 @@ static void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon)
>                         xive_source_irq_is_lsi(xsrc, i) ? "LSI" : "MSI",
>                         pq & XIVE_ESB_VAL_P ? 'P' : '-',
>                         pq & XIVE_ESB_VAL_Q ? 'Q' : '-',
> -                       xsrc->status[i] & XIVE_STATUS_ASSERTED ? 'A' : ' ',
> +                       xive_source_is_asserted(xsrc, i) ? 'A' : ' ',
>                         xive_eas_is_masked(eas) ? "M" : " ",
>                         (int) xive_get_field64(EAS_END_DATA, eas->w));
>  
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 6d4909d0a856..be94cff14837 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -242,7 +242,7 @@ int kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp)
>  
>      if (xive_source_irq_is_lsi(xsrc, srcno)) {
>          state |= KVM_XIVE_LEVEL_SENSITIVE;
> -        if (xsrc->status[srcno] & XIVE_STATUS_ASSERTED) {
> +        if (xive_source_is_asserted(xsrc, srcno)) {
>              state |= KVM_XIVE_LEVEL_ASSERTED;
>          }
>      }
> @@ -321,7 +321,7 @@ uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset,
>      if (xive_source_irq_is_lsi(xsrc, srcno) &&
>          offset == XIVE_ESB_LOAD_EOI) {
>          xive_esb_read(xsrc, srcno, XIVE_ESB_SET_PQ_00);
> -        if (xsrc->status[srcno] & XIVE_STATUS_ASSERTED) {
> +        if (xive_source_is_asserted(xsrc, srcno)) {
>              kvmppc_xive_esb_trigger(xsrc, srcno);
>          }
>          return 0;
> @@ -359,11 +359,7 @@ void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val)
>              return;
>          }
>      } else {
> -        if (val) {
> -            xsrc->status[srcno] |= XIVE_STATUS_ASSERTED;
> -        } else {
> -            xsrc->status[srcno] &= ~XIVE_STATUS_ASSERTED;
> -        }
> +        xive_source_set_asserted(xsrc, srcno, val);
>      }
>  
>      kvmppc_xive_esb_trigger(xsrc, srcno);
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 6c82326ec768..190194d27f84 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -875,7 +875,7 @@ static bool xive_source_lsi_trigger(XiveSource *xsrc, uint32_t srcno)
>  {
>      uint8_t old_pq = xive_source_esb_get(xsrc, srcno);
>  
> -    xsrc->status[srcno] |= XIVE_STATUS_ASSERTED;
> +    xive_source_set_asserted(xsrc, srcno, true);
>  
>      switch (old_pq) {
>      case XIVE_ESB_RESET:
> @@ -923,7 +923,7 @@ static bool xive_source_esb_eoi(XiveSource *xsrc, uint32_t srcno)
>       * notification
>       */
>      if (xive_source_irq_is_lsi(xsrc, srcno) &&
> -        xsrc->status[srcno] & XIVE_STATUS_ASSERTED) {
> +        xive_source_is_asserted(xsrc, srcno)) {
>          ret = xive_source_lsi_trigger(xsrc, srcno);
>      }
>  
> @@ -1104,7 +1104,7 @@ void xive_source_set_irq(void *opaque, int srcno, int val)
>          if (val) {
>              notify = xive_source_lsi_trigger(xsrc, srcno);
>          } else {
> -            xsrc->status[srcno] &= ~XIVE_STATUS_ASSERTED;
> +            xive_source_set_asserted(xsrc, srcno, false);
>          }
>      } else {
>          if (val) {
> @@ -1133,7 +1133,7 @@ void xive_source_pic_print_info(XiveSource *xsrc, uint32_t offset, Monitor *mon)
>                         xive_source_irq_is_lsi(xsrc, i) ? "LSI" : "MSI",
>                         pq & XIVE_ESB_VAL_P ? 'P' : '-',
>                         pq & XIVE_ESB_VAL_Q ? 'Q' : '-',
> -                       xsrc->status[i] & XIVE_STATUS_ASSERTED ? 'A' : ' ');
> +                       xive_source_is_asserted(xsrc, i) ? 'A' : ' ');
>      }
>  }
>