qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH for-2.12 v3 08/11] spapr: introduce a XICSFabric i


From: Cédric Le Goater
Subject: Re: [Qemu-ppc] [PATCH for-2.12 v3 08/11] spapr: introduce a XICSFabric irq_is_lsi() operation
Date: Fri, 17 Nov 2017 08:23:00 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 11/17/2017 05:54 AM, David Gibson wrote:
> On Fri, Nov 10, 2017 at 03:20:14PM +0000, Cédric Le Goater wrote:
>> It will be used later on to distinguish the allocation of an LSI
>> interrupt from an MSI and also to reduce the use of the ICSIRQState
>> array of the ICSState object, which is on our way to introduce XIVE.
>>
>> The 'irq' parameter continues to refer to the global IRQ number space.
>>
>> On PowerNV, only the PSI controller interrupts are handled and they
>> are all LSIs.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> !?! AFAICT this is a step backwards.  The users of ics_is_lsi() here
> are in the xics code.  So they already have the right information
> locally in the ICSState object.  Why on earth would you indirect
> through the fabric.

because I am trying to get rid of the fact that the current ICS 
allocator handles two things at the same time : IRQ allocation 
and IRQ type. And that's a problem because the ICSState object 
is just used everywhere in the code. 

OK, So that's is not to your liking, I will come up with some 
other solution. Thanks for looking.

C.   
  
> 
>> ---
>>  hw/intc/xics.c        | 26 +++++++++++++++++---------
>>  hw/intc/xics_kvm.c    |  4 ++--
>>  hw/ppc/pnv.c          | 16 ++++++++++++++++
>>  hw/ppc/spapr.c        |  9 +++++++++
>>  include/hw/ppc/xics.h |  2 ++
>>  5 files changed, 46 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index 2c4899f278e2..42880e736697 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -33,6 +33,7 @@
>>  #include "trace.h"
>>  #include "qemu/timer.h"
>>  #include "hw/ppc/xics.h"
>> +#include "hw/ppc/spapr.h"
>>  #include "qemu/error-report.h"
>>  #include "qapi/visitor.h"
>>  #include "monitor/monitor.h"
>> @@ -70,8 +71,7 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon)
>>          }
>>          monitor_printf(mon, "  %4x %s %02x %02x\n",
>>                         ics->offset + i,
>> -                       (irq->flags & XICS_FLAGS_IRQ_LSI) ?
>> -                       "LSI" : "MSI",
>> +                       ics_is_lsi(ics, i) ? "LSI" : "MSI",
> 
> !?! 
> 
>>                         irq->priority, irq->status);
>>      }
>>  }
>> @@ -377,6 +377,14 @@ static const TypeInfo icp_info = {
>>  /*
>>   * ICS: Source layer
>>   */
>> +bool ics_is_lsi(ICSState *ics, int srcno)
>> +{
>> +    XICSFabric *xi = ics->xics;
>> +    XICSFabricClass *xic = XICS_FABRIC_GET_CLASS(xi);
>> +
>> +    return xic->irq_is_lsi(xi, srcno + ics->offset);
>> +}
>> +
>>  static void ics_simple_resend_msi(ICSState *ics, int srcno)
>>  {
>>      ICSIRQState *irq = ics->irqs + srcno;
>> @@ -435,7 +443,7 @@ static void ics_simple_set_irq(void *opaque, int srcno, 
>> int val)
>>  {
>>      ICSState *ics = (ICSState *)opaque;
>>  
>> -    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
>> +    if (ics_is_lsi(ics, srcno)) {
>>          ics_simple_set_irq_lsi(ics, srcno, val);
>>      } else {
>>          ics_simple_set_irq_msi(ics, srcno, val);
>> @@ -472,7 +480,7 @@ void ics_simple_write_xive(ICSState *ics, int srcno, int 
>> server,
>>      trace_xics_ics_simple_write_xive(ics->offset + srcno, srcno, server,
>>                                       priority);
>>  
>> -    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
>> +    if (ics_is_lsi(ics, srcno)) {
>>          ics_simple_write_xive_lsi(ics, srcno);
>>      } else {
>>          ics_simple_write_xive_msi(ics, srcno);
>> @@ -484,10 +492,10 @@ static void ics_simple_reject(ICSState *ics, uint32_t 
>> nr)
>>      ICSIRQState *irq = ics->irqs + nr - ics->offset;
>>  
>>      trace_xics_ics_simple_reject(nr, nr - ics->offset);
>> -    if (irq->flags & XICS_FLAGS_IRQ_MSI) {
>> -        irq->status |= XICS_STATUS_REJECTED;
>> -    } else if (irq->flags & XICS_FLAGS_IRQ_LSI) {
>> +    if (ics_is_lsi(ics, nr - ics->offset)) {
>>          irq->status &= ~XICS_STATUS_SENT;
>> +    } else {
>> +        irq->status |= XICS_STATUS_REJECTED;
>>      }
>>  }
>>  
>> @@ -497,7 +505,7 @@ static void ics_simple_resend(ICSState *ics)
>>  
>>      for (i = 0; i < ics->nr_irqs; i++) {
>>          /* FIXME: filter by server#? */
>> -        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
>> +        if (ics_is_lsi(ics, i)) {
>>              ics_simple_resend_lsi(ics, i);
>>          } else {
>>              ics_simple_resend_msi(ics, i);
>> @@ -512,7 +520,7 @@ static void ics_simple_eoi(ICSState *ics, uint32_t nr)
>>  
>>      trace_xics_ics_simple_eoi(nr);
>>  
>> -    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
>> +    if (ics_is_lsi(ics, srcno)) {
>>          irq->status &= ~XICS_STATUS_SENT;
>>      }
>>  }
>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
>> index 3091ad3ac2c8..2f10637c9f7c 100644
>> --- a/hw/intc/xics_kvm.c
>> +++ b/hw/intc/xics_kvm.c
>> @@ -258,7 +258,7 @@ static int ics_set_kvm_state(ICSState *ics, int 
>> version_id)
>>              state |= KVM_XICS_MASKED;
>>          }
>>  
>> -        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
>> +        if (ics_is_lsi(ics, i)) {
>>              state |= KVM_XICS_LEVEL_SENSITIVE;
>>              if (irq->status & XICS_STATUS_ASSERTED) {
>>                  state |= KVM_XICS_PENDING;
>> @@ -293,7 +293,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int 
>> val)
>>      int rc;
>>  
>>      args.irq = srcno + ics->offset;
>> -    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_MSI) {
>> +    if (!ics_is_lsi(ics, srcno)) {
>>          if (!val) {
>>              return;
>>          }
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 8288940ef9d7..958223376b4c 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -1035,6 +1035,21 @@ static bool pnv_irq_test(XICSFabric *xi, int irq)
>>      return false;
>>  }
>>  
>> +static bool pnv_irq_is_lsi(XICSFabric *xi, int irq)
>> +{
>> +    PnvMachineState *pnv = POWERNV_MACHINE(xi);
>> +    int i;
>> +
>> +    /* PowerNV machine only has PSI interrupts which are all LSIs */
>> +    for (i = 0; i < pnv->num_chips; i++) {
>> +        ICSState *ics = &pnv->chips[i]->psi.ics;
>> +        if (ics_valid_irq(ics, irq)) {
>> +            return true;
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>>  static void pnv_pic_print_info(InterruptStatsProvider *obj,
>>                                 Monitor *mon)
>>  {
>> @@ -1120,6 +1135,7 @@ static void powernv_machine_class_init(ObjectClass 
>> *oc, void *data)
>>      xic->ics_get = pnv_ics_get;
>>      xic->ics_resend = pnv_ics_resend;
>>      xic->irq_test = pnv_irq_test;
>> +    xic->irq_is_lsi = pnv_irq_is_lsi;
>>      ispc->print_info = pnv_pic_print_info;
>>  
>>      powernv_machine_class_props_init(oc);
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 1cbbd7715a85..ce314fcf38db 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3628,6 +3628,14 @@ static void spapr_irq_free_block_2_11(XICSFabric *xi, 
>> int irq, int num)
>>      }
>>  }
>>  
>> +static bool spapr_irq_is_lsi(XICSFabric *xi, int irq)
>> +{
>> +    sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
>> +    int srcno = irq - spapr->ics->offset;
>> +
>> +    return spapr->ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI;
>> +}
>> +
>>  static bool spapr_irq_test(XICSFabric *xi, int irq)
>>  {
>>      sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
>> @@ -3765,6 +3773,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
>> void *data)
>>      xic->irq_test = spapr_irq_test;
>>      xic->irq_alloc_block = spapr_irq_alloc_block;
>>      xic->irq_free_block = spapr_irq_free_block;
>> +    xic->irq_is_lsi = spapr_irq_is_lsi;
>>  
>>      ispc->print_info = spapr_pic_print_info;
>>      /* Force NUMA node memory size to be a multiple of
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 30e7f2e0a7dd..478f8e510179 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -179,6 +179,7 @@ typedef struct XICSFabricClass {
>>      bool (*irq_test)(XICSFabric *xi, int irq);
>>      int (*irq_alloc_block)(XICSFabric *xi, int count, int align);
>>      void (*irq_free_block)(XICSFabric *xi, int irq, int num);
>> +    bool (*irq_is_lsi)(XICSFabric *xi, int irq);
>>  } XICSFabricClass;
>>  
>>  #define XICS_IRQS_SPAPR               1024
>> @@ -205,6 +206,7 @@ void ics_simple_write_xive(ICSState *ics, int nr, int 
>> server,
>>  void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>>  void icp_pic_print_info(ICPState *icp, Monitor *mon);
>>  void ics_pic_print_info(ICSState *ics, Monitor *mon);
>> +bool ics_is_lsi(ICSState *ics, int srno);
>>  
>>  void ics_resend(ICSState *ics);
>>  void icp_resend(ICPState *ss);
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]