On 08/10/2018 02:46 AM, David Gibson wrote:
> On Mon, Jul 30, 2018 at 04:11:33PM +0200, Cédric Le Goater wrote:
>> This proposal moves all the related IRQ routines of the sPAPR machine
>> behind a sPAPR IRQ backend interface 'spapr_irq' to prepare for future
>> changes. First of which will be to increase the size of the IRQ number
>> space, then, will follow a new backend for the POWER9 XIVE IRQ controller.
>>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>
> Applied to ppc-for-3.1.
I see that you have applied patch 2/4 and not 3/4. I suppose you
are still reviewing 3/4.
Btw, it would be good if we could change the way the interrupt
device objects (ICSState and soon sPAPRXive) are allocated and
use object_initialize() instead of object_new(). That can come
later for XICS but for XIVE, it would be better to get the initial
patch right. This means that the holding C struct is common to
the QEMU and KVM devices.
Thanks,
C.
>> ---
>> include/hw/ppc/spapr.h | 11 +-
>> include/hw/ppc/spapr_irq.h | 22 ++++
>> hw/ppc/spapr.c | 180 +----------------------------
>> hw/ppc/spapr_cpu_core.c | 1 +
>> hw/ppc/spapr_irq.c | 230 +++++++++++++++++++++++++++++++++++++
>> 5 files changed, 259 insertions(+), 185 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 73067f5ee8aa..ad4d7cfd97b0 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -4,7 +4,6 @@
>> #include "qemu/units.h"
>> #include "sysemu/dma.h"
>> #include "hw/boards.h"
>> -#include "hw/ppc/xics.h"
>> #include "hw/ppc/spapr_drc.h"
>> #include "hw/mem/pc-dimm.h"
>> #include "hw/ppc/spapr_ovec.h"
>> @@ -16,6 +15,7 @@ struct sPAPRNVRAM;
>> typedef struct sPAPREventLogEntry sPAPREventLogEntry;
>> typedef struct sPAPREventSource sPAPREventSource;
>> typedef struct sPAPRPendingHPT sPAPRPendingHPT;
>> +typedef struct ICSState ICSState;
>>
>> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
>> #define SPAPR_ENTRY_POINT 0x100
>> @@ -110,6 +110,7 @@ struct sPAPRMachineClass {
>> unsigned n_dma, uint32_t *liobns, Error **errp);
>> sPAPRResizeHPT resize_hpt_default;
>> sPAPRCapabilities default_caps;
>> + sPAPRIrq *irq;
>> };
>>
>> /**
>> @@ -780,14 +781,6 @@ int spapr_get_vcpu_id(PowerPCCPU *cpu);
>> void spapr_set_vcpu_id(PowerPCCPU *cpu, int cpu_index, Error **errp);
>> PowerPCCPU *spapr_find_cpu(int vcpu_id);
>>
>> -int spapr_irq_find(sPAPRMachineState *spapr, int num, bool align,
>> - Error **errp);
>> -#define spapr_irq_findone(spapr, errp) spapr_irq_find(spapr, 1, false, errp)
>> -int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error
>> **errp);
>> -void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
>> -qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
>> -
>> -
>> int spapr_caps_pre_load(void *opaque);
>> int spapr_caps_pre_save(void *opaque);
>>
>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>> index 6f7f50548809..0e98c4474bb2 100644
>> --- a/include/hw/ppc/spapr_irq.h
>> +++ b/include/hw/ppc/spapr_irq.h
>> @@ -29,4 +29,26 @@ int spapr_irq_msi_alloc(sPAPRMachineState *spapr,
>> uint32_t num, bool align,
>> void spapr_irq_msi_free(sPAPRMachineState *spapr, int irq, uint32_t num);
>> void spapr_irq_msi_reset(sPAPRMachineState *spapr);
>>
>> +typedef struct sPAPRIrq {
>> + uint32_t nr_irqs;
>> +
>> + void (*init)(sPAPRMachineState *spapr, Error **errp);
>> + int (*claim)(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp);
>> + void (*free)(sPAPRMachineState *spapr, int irq, int num);
>> + qemu_irq (*qirq)(sPAPRMachineState *spapr, int irq);
>> + void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
>> +} sPAPRIrq;
>> +
>> +extern sPAPRIrq spapr_irq_xics;
>> +
>> +int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error
>> **errp);
>> +void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
>> +qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
>> +
>> +/*
>> + * XICS legacy routines
>> + */
>> +int spapr_irq_find(sPAPRMachineState *spapr, int num, bool align, Error
>> **errp);
>> +#define spapr_irq_findone(spapr, errp) spapr_irq_find(spapr, 1, false, errp)
>> +
>> #endif
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 792e24453d8b..d9f8cca49208 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -54,7 +54,6 @@
>> #include "hw/ppc/spapr.h"
>> #include "hw/ppc/spapr_vio.h"
>> #include "hw/pci-host/spapr.h"
>> -#include "hw/ppc/xics.h"
>> #include "hw/pci/msi.h"
>>
>> #include "hw/pci/pci.h"
>> @@ -117,33 +116,6 @@ static bool spapr_is_thread0_in_vcore(sPAPRMachineState
>> *spapr,
>> return spapr_get_vcpu_id(cpu) % spapr->vsmt == 0;
>> }
>>
>> -static ICSState *spapr_ics_create(sPAPRMachineState *spapr,
>> - const char *type_ics,
>> - int nr_irqs, Error **errp)
>> -{
>> - Error *local_err = NULL;
>> - Object *obj;
>> -
>> - obj = object_new(type_ics);
>> - object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort);
>> - object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr),
>> - &error_abort);
>> - object_property_set_int(obj, nr_irqs, "nr-irqs", &local_err);
>> - if (local_err) {
>> - goto error;
>> - }
>> - object_property_set_bool(obj, true, "realized", &local_err);
>> - if (local_err) {
>> - goto error;
>> - }
>> -
>> - return ICS_BASE(obj);
>> -
>> -error:
>> - error_propagate(errp, local_err);
>> - return NULL;
>> -}
>> -
>> static bool pre_2_10_vmstate_dummy_icp_needed(void *opaque)
>> {
>> /* Dummy entries correspond to unused ICPState objects in older QEMUs,
>> @@ -184,43 +156,6 @@ static int xics_max_server_number(sPAPRMachineState
>> *spapr)
>> return DIV_ROUND_UP(max_cpus * spapr->vsmt, smp_threads);
>> }
>>
>> -static void xics_system_init(MachineState *machine, int nr_irqs, Error
>> **errp)
>> -{
>> - sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
>> - Error *local_err = NULL;
>> -
>> - /* Initialize the MSI IRQ allocator. */
>> - if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
>> - spapr_irq_msi_init(spapr, XICS_IRQ_BASE + nr_irqs - SPAPR_IRQ_MSI);
>> - }
>> -
>> - if (kvm_enabled()) {
>> - if (machine_kernel_irqchip_allowed(machine) &&
>> - !xics_kvm_init(spapr, &local_err)) {
>> - spapr->icp_type = TYPE_KVM_ICP;
>> - spapr->ics = spapr_ics_create(spapr, TYPE_ICS_KVM, nr_irqs,
>> - &local_err);
>> - }
>> - if (machine_kernel_irqchip_required(machine) && !spapr->ics) {
>> - error_prepend(&local_err,
>> - "kernel_irqchip requested but unavailable: ");
>> - goto error;
>> - }
>> - error_free(local_err);
>> - local_err = NULL;
>> - }
>> -
>> - if (!spapr->ics) {
>> - xics_spapr_init(spapr);
>> - spapr->icp_type = TYPE_ICP;
>> - spapr->ics = spapr_ics_create(spapr, TYPE_ICS_SIMPLE, nr_irqs,
>> - &local_err);
>> - }
>> -
>> -error:
>> - error_propagate(errp, local_err);
>> -}
>> -
>> static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>> int smt_threads)
>> {
>> @@ -2618,7 +2553,7 @@ static void spapr_machine_init(MachineState *machine)
>> load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
>>
>> /* Set up Interrupt Controller before we create the VCPUs */
>> - xics_system_init(machine, XICS_IRQS_SPAPR, &error_fatal);
>> + smc->irq->init(spapr, &error_fatal);
>>
>> /* Set up containers for ibm,client-architecture-support negotiated
>> options
>> */
>> @@ -3810,121 +3745,13 @@ static ICPState *spapr_icp_get(XICSFabric *xi, int
>> vcpu_id)
>> return cpu ? ICP(cpu->intc) : NULL;
>> }
>>
>> -#define ICS_IRQ_FREE(ics, srcno) \
>> - (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
>> -
>> -static int ics_find_free_block(ICSState *ics, int num, int alignnum)
>> -{
>> - int first, i;
>> -
>> - for (first = 0; first < ics->nr_irqs; first += alignnum) {
>> - if (num > (ics->nr_irqs - first)) {
>> - return -1;
>> - }
>> - for (i = first; i < first + num; ++i) {
>> - if (!ICS_IRQ_FREE(ics, i)) {
>> - break;
>> - }
>> - }
>> - if (i == (first + num)) {
>> - return first;
>> - }
>> - }
>> -
>> - return -1;
>> -}
>> -
>> -int spapr_irq_find(sPAPRMachineState *spapr, int num, bool align, Error
>> **errp)
>> -{
>> - ICSState *ics = spapr->ics;
>> - int first = -1;
>> -
>> - assert(ics);
>> -
>> - /*
>> - * MSIMesage::data is used for storing VIRQ so
>> - * it has to be aligned to num to support multiple
>> - * MSI vectors. MSI-X is not affected by this.
>> - * The hint is used for the first IRQ, the rest should
>> - * be allocated continuously.
>> - */
>> - if (align) {
>> - assert((num == 1) || (num == 2) || (num == 4) ||
>> - (num == 8) || (num == 16) || (num == 32));
>> - first = ics_find_free_block(ics, num, num);
>> - } else {
>> - first = ics_find_free_block(ics, num, 1);
>> - }
>> -
>> - if (first < 0) {
>> - error_setg(errp, "can't find a free %d-IRQ block", num);
>> - return -1;
>> - }
>> -
>> - return first + ics->offset;
>> -}
>> -
>> -int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error
>> **errp)
>> -{
>> - ICSState *ics = spapr->ics;
>> -
>> - assert(ics);
>> -
>> - if (!ics_valid_irq(ics, irq)) {
>> - error_setg(errp, "IRQ %d is invalid", irq);
>> - return -1;
>> - }
>> -
>> - if (!ICS_IRQ_FREE(ics, irq - ics->offset)) {
>> - error_setg(errp, "IRQ %d is not free", irq);
>> - return -1;
>> - }
>> -
>> - ics_set_irq_type(ics, irq - ics->offset, lsi);
>> - return 0;
>> -}
>> -
>> -void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num)
>> -{
>> - ICSState *ics = spapr->ics;
>> - int srcno = irq - ics->offset;
>> - int i;
>> -
>> - if (ics_valid_irq(ics, irq)) {
>> - trace_spapr_irq_free(0, irq, num);
>> - for (i = srcno; i < srcno + num; ++i) {
>> - if (ICS_IRQ_FREE(ics, i)) {
>> - trace_spapr_irq_free_warn(0, i + ics->offset);
>> - }
>> - memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
>> - }
>> - }
>> -}
>> -
>> -qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq)
>> -{
>> - ICSState *ics = spapr->ics;
>> -
>> - if (ics_valid_irq(ics, irq)) {
>> - return ics->qirqs[irq - ics->offset];
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> static void spapr_pic_print_info(InterruptStatsProvider *obj,
>> Monitor *mon)
>> {
>> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>> - CPUState *cs;
>> -
>> - CPU_FOREACH(cs) {
>> - PowerPCCPU *cpu = POWERPC_CPU(cs);
>> -
>> - icp_pic_print_info(ICP(cpu->intc), mon);
>> - }
>> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>>
>> - ics_pic_print_info(spapr->ics, mon);
>> + smc->irq->print_info(spapr, mon);
>> }
>>
>> int spapr_get_vcpu_id(PowerPCCPU *cpu)
>> @@ -4036,6 +3863,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
>> void *data)
>> smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
>> smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = 16; /* 64kiB */
>> spapr_caps_add_properties(smc, &error_abort);
>> + smc->irq = &spapr_irq_xics;
>> }
>>
>> static const TypeInfo spapr_machine_info = {
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index 993759db47fa..fb091995a11f 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -11,6 +11,7 @@
>> #include "hw/ppc/spapr_cpu_core.h"
>> #include "target/ppc/cpu.h"
>> #include "hw/ppc/spapr.h"
>> +#include "hw/ppc/xics.h" /* for icp_create() - to be removed */
>> #include "hw/boards.h"
>> #include "qapi/error.h"
>> #include "sysemu/cpus.h"
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index 24e9c1d4433c..0cbb5dd39368 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -13,6 +13,9 @@
>> #include "qapi/error.h"
>> #include "hw/ppc/spapr.h"
>> #include "hw/ppc/xics.h"
>> +#include "sysemu/kvm.h"
>> +
>> +#include "trace.h"
>>
>> void spapr_irq_msi_init(sPAPRMachineState *spapr, uint32_t nr_msis)
>> {
>> @@ -54,3 +57,230 @@ void spapr_irq_msi_reset(sPAPRMachineState *spapr)
>> {
>> bitmap_clear(spapr->irq_map, 0, spapr->irq_map_nr);
>> }
>> +
>> +
>> +/*
>> + * XICS IRQ backend.
>> + */
>> +
>> +static ICSState *spapr_ics_create(sPAPRMachineState *spapr,
>> + const char *type_ics,
>> + int nr_irqs, Error **errp)
>> +{
>> + Error *local_err = NULL;
>> + Object *obj;
>> +
>> + obj = object_new(type_ics);
>> + object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort);
>> + object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr),
>> + &error_abort);
>> + object_property_set_int(obj, nr_irqs, "nr-irqs", &local_err);
>> + if (local_err) {
>> + goto error;
>> + }
>> + object_property_set_bool(obj, true, "realized", &local_err);
>> + if (local_err) {
>> + goto error;
>> + }
>> +
>> + return ICS_BASE(obj);
>> +
>> +error:
>> + error_propagate(errp, local_err);
>> + return NULL;
>> +}
>> +
>> +static void spapr_irq_init_xics(sPAPRMachineState *spapr, Error **errp)
>> +{
>> + MachineState *machine = MACHINE(spapr);
>> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> + int nr_irqs = smc->irq->nr_irqs;
>> + Error *local_err = NULL;
>> +
>> + /* Initialize the MSI IRQ allocator. */
>> + if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
>> + spapr_irq_msi_init(spapr, XICS_IRQ_BASE + nr_irqs - SPAPR_IRQ_MSI);
>> + }
>> +
>> + if (kvm_enabled()) {
>> + if (machine_kernel_irqchip_allowed(machine) &&
>> + !xics_kvm_init(spapr, &local_err)) {
>> + spapr->icp_type = TYPE_KVM_ICP;
>> + spapr->ics = spapr_ics_create(spapr, TYPE_ICS_KVM, nr_irqs,
>> + &local_err);
>> + }
>> + if (machine_kernel_irqchip_required(machine) && !spapr->ics) {
>> + error_prepend(&local_err,
>> + "kernel_irqchip requested but unavailable: ");
>> + goto error;
>> + }
>> + error_free(local_err);
>> + local_err = NULL;
>> + }
>> +
>> + if (!spapr->ics) {
>> + xics_spapr_init(spapr);
>> + spapr->icp_type = TYPE_ICP;
>> + spapr->ics = spapr_ics_create(spapr, TYPE_ICS_SIMPLE, nr_irqs,
>> + &local_err);
>> + }
>> +
>> +error:
>> + error_propagate(errp, local_err);
>> +}
>> +
>> +#define ICS_IRQ_FREE(ics, srcno) \
>> + (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
>> +
>> +static int spapr_irq_claim_xics(sPAPRMachineState *spapr, int irq, bool lsi,
>> + Error **errp)
>> +{
>> + ICSState *ics = spapr->ics;
>> +
>> + assert(ics);
>> +
>> + if (!ics_valid_irq(ics, irq)) {
>> + error_setg(errp, "IRQ %d is invalid", irq);
>> + return -1;
>> + }
>> +
>> + if (!ICS_IRQ_FREE(ics, irq - ics->offset)) {
>> + error_setg(errp, "IRQ %d is not free", irq);
>> + return -1;
>> + }
>> +
>> + ics_set_irq_type(ics, irq - ics->offset, lsi);
>> + return 0;
>> +}
>> +
>> +static void spapr_irq_free_xics(sPAPRMachineState *spapr, int irq, int num)
>> +{
>> + ICSState *ics = spapr->ics;
>> + uint32_t srcno = irq - ics->offset;
>> + int i;
>> +
>> + if (ics_valid_irq(ics, irq)) {
>> + trace_spapr_irq_free(0, irq, num);
>> + for (i = srcno; i < srcno + num; ++i) {
>> + if (ICS_IRQ_FREE(ics, i)) {
>> + trace_spapr_irq_free_warn(0, i);
>> + }
>> + memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
>> + }
>> + }
>> +}
>> +
>> +static qemu_irq spapr_qirq_xics(sPAPRMachineState *spapr, int irq)
>> +{
>> + ICSState *ics = spapr->ics;
>> + uint32_t srcno = irq - ics->offset;
>> +
>> + if (ics_valid_irq(ics, irq)) {
>> + return ics->qirqs[srcno];
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void spapr_irq_print_info_xics(sPAPRMachineState *spapr, Monitor
>> *mon)
>> +{
>> + CPUState *cs;
>> +
>> + CPU_FOREACH(cs) {
>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +
>> + icp_pic_print_info(ICP(cpu->intc), mon);
>> + }
>> +
>> + ics_pic_print_info(spapr->ics, mon);
>> +}
>> +
>> +sPAPRIrq spapr_irq_xics = {
>> + .nr_irqs = XICS_IRQS_SPAPR,
>> +
>> + .init = spapr_irq_init_xics,
>> + .claim = spapr_irq_claim_xics,
>> + .free = spapr_irq_free_xics,
>> + .qirq = spapr_qirq_xics,
>> + .print_info = spapr_irq_print_info_xics,
>> +};
>> +
>> +/*
>> + * sPAPR IRQ frontend routines for devices
>> + */
>> +
>> +int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error
>> **errp)
>> +{
>> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> +
>> + return smc->irq->claim(spapr, irq, lsi, errp);
>> +}
>> +
>> +void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num)
>> +{
>> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> +
>> + smc->irq->free(spapr, irq, num);
>> +}
>> +
>> +qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq)
>> +{
>> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> +
>> + return smc->irq->qirq(spapr, irq);
>> +}
>> +
>> +/*
>> + * XICS legacy routines - to deprecate one day
>> + */
>> +
>> +static int ics_find_free_block(ICSState *ics, int num, int alignnum)
>> +{
>> + int first, i;
>> +
>> + for (first = 0; first < ics->nr_irqs; first += alignnum) {
>> + if (num > (ics->nr_irqs - first)) {
>> + return -1;
>> + }
>> + for (i = first; i < first + num; ++i) {
>> + if (!ICS_IRQ_FREE(ics, i)) {
>> + break;
>> + }
>> + }
>> + if (i == (first + num)) {
>> + return first;
>> + }
>> + }
>> +
>> + return -1;
>> +}
>> +
>> +int spapr_irq_find(sPAPRMachineState *spapr, int num, bool align, Error
>> **errp)
>> +{
>> + ICSState *ics = spapr->ics;
>> + int first = -1;
>> +
>> + assert(ics);
>> +
>> + /*
>> + * MSIMesage::data is used for storing VIRQ so
>> + * it has to be aligned to num to support multiple
>> + * MSI vectors. MSI-X is not affected by this.
>> + * The hint is used for the first IRQ, the rest should
>> + * be allocated continuously.
>> + */
>> + if (align) {
>> + assert((num == 1) || (num == 2) || (num == 4) ||
>> + (num == 8) || (num == 16) || (num == 32));
>> + first = ics_find_free_block(ics, num, num);
>> + } else {
>> + first = ics_find_free_block(ics, num, 1);
>> + }
>> +
>> + if (first < 0) {
>> + error_setg(errp, "can't find a free %d-IRQ block", num);
>> + return -1;
>> + }
>> +
>> + return first + ics->offset;
>> +}
>
