On 11/30/2017 05:55 AM, David Gibson wrote:
> On Thu, Nov 23, 2017 at 02:29:47PM +0100, Cédric Le Goater wrote:
>> The XIVE object is designed to be always available, so it is created
>> unconditionally on newer machines.
>
> There doesn't actually seem to be anything dependent on machine
> version here.
No. I thought that was too early in the patchset. This is handled
in the last patch with a 'xive_exploitation' bool which is set to
false on older machines.
But, nevertheless, the XIVE objects are always created even if not
used. Something to discuss.
>> Depending on the configuration and
>> the guest capabilities, the CAS negotiation process will decide which
>> interrupt model to use, legacy or XIVE.
>>
>> The XIVE model makes use of the full range of the IRQ number space
>> because the IRQ numbers for the CPU IPIs are allocated in the range
>> below XICS_IRQ_BASE, which is unused by XICS.
>
> Ok. And I take it 4096 is enough space for the XIVE IPIs for the
> forseeable future?
The biggest real system I am aware of as 16 sockets, 192 cores, SMT8.
That's 1536 cpus. pseries has a max_cpus of 1024.
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>> ---
>> hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
>> include/hw/ppc/spapr.h | 2 ++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 5d3325ca3c88..0e0107c8272c 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -56,6 +56,7 @@
>> #include "hw/ppc/spapr_vio.h"
>> #include "hw/pci-host/spapr.h"
>> #include "hw/ppc/xics.h"
>> +#include "hw/ppc/spapr_xive.h"
>> #include "hw/pci/msi.h"
>>
>> #include "hw/pci/pci.h"
>> @@ -204,6 +205,29 @@ static void xics_system_init(MachineState *machine, int
>> nr_irqs, Error **errp)
>> }
>> }
>>
>> +static sPAPRXive *spapr_xive_create(sPAPRMachineState *spapr, int nr_irqs,
>> + Error **errp)
>> +{
>> + Error *local_err = NULL;
>> + Object *obj;
>> +
>> + obj = object_new(TYPE_SPAPR_XIVE);
>> + object_property_add_child(OBJECT(spapr), "xive", obj, &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 SPAPR_XIVE(obj);
>> +error:
>> + error_propagate(errp, local_err);
>> + return NULL;
>> +}
>> +
>> static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>> int smt_threads)
>> {
>> @@ -2360,6 +2384,16 @@ static void ppc_spapr_init(MachineState *machine)
>> /* Set up Interrupt Controller before we create the VCPUs */
>> xics_system_init(machine, XICS_IRQS_SPAPR, &error_fatal);
>>
>> + /* We don't have KVM support yet, so check for irqchip=on */
>> + if (kvm_enabled() && machine_kernel_irqchip_required(machine)) {
>> + error_report("kernel_irqchip requested. no XIVE support");
>
> I think you want an actual exit(1) here, no? error_report() will
> print an error but keep going.
yes. Today, it coredumps. I am not sure why. I will add an exit().
>
>> + } else {
>> + /* XIVE uses the full range of IRQ numbers. The CPU IPIs will
>> + * use the range below XICS_IRQ_BASE, which is unused by XICS. */
>> + spapr->xive = spapr_xive_create(spapr, XICS_IRQ_BASE +
>> XICS_IRQS_SPAPR,
>> + &error_fatal);
>
> XICS_IRQ_BASE == 4096, and XICS_IRQS_SPAPR (which we should rename at
> some point) == 1024.
>
> So we have a total irq space of 5k, which is a bit odd. I'd be ok
> with rounding it out to 8k for newer machines if that's useful.
> Sparse allocations in there might make life easier for getting
> consistent irq numbers without an "allocator" per se (because we can
> use different regions for VIO, PCI intx, MSI, etc. etc.).
I will start another thread on that topic.
Thanks,
C.
>
>> + }
>> +
>> /* Set up containers for ibm,client-architecture-support negotiated
>> options
>> */
>> spapr->ov5 = spapr_ovec_new();
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 9a3885593c86..90e2b0f6c678 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -14,6 +14,7 @@ struct sPAPRNVRAM;
>> typedef struct sPAPREventLogEntry sPAPREventLogEntry;
>> typedef struct sPAPREventSource sPAPREventSource;
>> typedef struct sPAPRPendingHPT sPAPRPendingHPT;
>> +typedef struct sPAPRXive sPAPRXive;
>>
>> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
>> #define SPAPR_ENTRY_POINT 0x100
>> @@ -127,6 +128,7 @@ struct sPAPRMachineState {
>> MemoryHotplugState hotplug_memory;
>>
>> const char *icp_type;
>> + sPAPRXive *xive;
>> };
>>
>> #define H_SUCCESS 0
>
