On Wed, 15 Nov 2017 16:08:32 +0000
Cédric Le Goater <c...@kaod.org> wrote:
> On 11/15/2017 03:52 PM, Greg Kurz wrote:
> > On Fri, 10 Nov 2017 15:20:15 +0000
> > Cédric Le Goater <c...@kaod.org> wrote:
> >
> >> The type of an interrupt, MSI or LSI, is stored under the flag
> >> attribute of the ICSIRQState array. To reduce the use of this array
> >> and consequently of the ICSState object (This is needed to introduce
> >> the new XIVE model), we choose to split the IRQ number space of the
> >> machine in two: first the LSIs and then the MSIs.
> >>
> >> This also has the benefit to keep the LSI IRQ numbers in a well known
> >> range which will be useful for PHB hotplug.
> >>
> >
> > Well... LSIs indeed land in a well known range, but it isn't enough for PHB
> > hotplug. Each PHB is uniquely identified by its 'index' property, and we
> > want each PHB to have fixed LSIs, so that they are invariant across
> > migration.
>
> ok.
>
> So, as said in another email, we should think about segmenting
> the allocation per device. At least for PHBs. This is specified in
> the PAPR specs, each device has one or more Bus Unit IDentifier (BUID)
BTW, the code currently uses "BUID" to refer to the PHB Unit ID which
is a different concept in the PAPR spec. Maybe this should be fixed
for the sake of clarity ?
> acting as a prefix for the IRQ number.
>
Since the user can instantiate multiple VIO devices, should we also
have an irq segment for the VIO bus as well ?
> We could model that by using a specific range for each PHB in the
> overall IRQ number space, depending on some index. LSIs would be
Some index should be the "index" property I was mentioning before.
> allocated at the beginning of this range, when the device is realized
> and MSIs later on when the guest starts.
>
> Identifying a LSI could be done using a mask on the IRQ number range
> of each PHB. It should be fast enough. I don't see other devices
> using LSIs under the sPAPR platform.
>
There aren't any other AFAICT.
>
> C.
>
>
>
> >> This change only applies to the latest pseries machines. Older
> >> machines still use the ICSIRQState array to define the IRQ type.
> >>
> >> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> >> ---
> >>
> >> Changes since v2 :
> >>
> >> - introduced a second set of XICSFabric IRQ operations for older
> >> pseries machines
> >>
> >> hw/intc/xics_spapr.c | 6 +++---
> >> hw/ppc/spapr.c | 33 +++++++++++++++++++++++++++++----
> >> include/hw/ppc/xics.h | 2 +-
> >> 3 files changed, 33 insertions(+), 8 deletions(-)pe-total-#msi
> >>
> >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> >> index de9e65d35247..b8e91aaf52bd 100644
> >> --- a/hw/intc/xics_spapr.c
> >> +++ b/hw/intc/xics_spapr.c
> >> @@ -260,7 +260,7 @@ int spapr_ics_alloc(ICSState *ics, int irq_hint, bool
> >> lsi, Error **errp)
> >> }
> >> irq = irq_hint;
> >> } else {
> >> - irq = xic->irq_alloc_block(ics->xics, 1, 1);
> >> + irq = xic->irq_alloc_block(ics->xics, 1, 1, lsi);
> >> if (irq < 0) {
> >> error_setg(errp, "can't allocate IRQ: no IRQ left");
> >> return -1;
> >> @@ -297,9 +297,9 @@ int spapr_ics_alloc_block(ICSState *ics, int num, bool
> >> lsi,
> >> if (align) {
> >> assert((num == 1) || (num == 2) || (num == 4) ||
> >> (num == 8) || (num == 16) || (num == 32));
> >> - first = xic->irq_alloc_block(ics->xics, num, num);
> >> + first = xic->irq_alloc_block(ics->xics, num, num, lsi);
> >> } else {
> >> - first = xic->irq_alloc_block(ics->xics, num, 1);
> >> + first = xic->irq_alloc_block(ics->xics, num, 1, lsi);
> >> }
> >> if (first < 0) {
> >> error_setg(errp, "can't find a free %d-IRQ block", num);
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index ce314fcf38db..f14eae6196cd 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -3596,7 +3596,8 @@ static bool spapr_irq_test_2_11(XICSFabric *xi, int
> >> irq)
> >> return !ICS_IRQ_FREE(ics, srcno);
> >> }
> >>
> >> -static int spapr_irq_alloc_block_2_11(XICSFabric *xi, int count, int
> >> align)
> >> +static int spapr_irq_alloc_block_2_11(XICSFabric *xi, int count, int
> >> align,
> >> + bool lsi)
> >> {
> >> sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> >> ICSState *ics = spapr->ics;
> >> @@ -3628,7 +3629,7 @@ static void spapr_irq_free_block_2_11(XICSFabric
> >> *xi, int irq, int num)
> >> }
> >> }
> >>
> >> -static bool spapr_irq_is_lsi(XICSFabric *xi, int irq)
> >> +static bool spapr_irq_is_lsi_2_11(XICSFabric *xi, int irq)
> >> {
> >> sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> >> int srcno = irq - spapr->ics->offset;
> >> @@ -3644,10 +3645,21 @@ static bool spapr_irq_test(XICSFabric *xi, int irq)
> >> return test_bit(srcno, spapr->irq_map);
> >> }
> >>
> >> -static int spapr_irq_alloc_block(XICSFabric *xi, int count, int align)
> >> +
> >> +/*
> >> + * Let's provision 4 LSIs per PHBs
> >> + */
> >> +#define SPAPR_MAX_LSI (SPAPR_MAX_PHBS * 4)
> >> +
> >> +/*
> >> + * Split the IRQ number space of the machine in two: first the LSIs
> >> + * and then the MSIs. This allows us to keep the LSI IRQ numbers in a
> >> + * well known range which is useful for PHB hotplug.
> >> + */
> >> +static int spapr_irq_alloc_block(XICSFabric *xi, int count, int align,
> >> bool lsi)
> >> {
> >> sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> >> - int start = 0;
> >> + int start = lsi ? 0 : SPAPR_MAX_LSI;
> >> int srcno;
> >>
> >> /*
> >> @@ -3664,6 +3676,10 @@ static int spapr_irq_alloc_block(XICSFabric *xi,
> >> int count, int align)
> >> return -1;
> >> }
> >>
> >> + if (lsi && srcno >= SPAPR_MAX_LSI) {
> >> + return -1;
> >> + }
> >> +
> >> bitmap_set(spapr->irq_map, srcno, count);
> >> return srcno + spapr->irq_base;
> >> }
> >> @@ -3676,6 +3692,14 @@ static void spapr_irq_free_block(XICSFabric *xi,
> >> int irq, int num)
> >> bitmap_clear(spapr->irq_map, srcno, num);
> >> }
> >>
> >> +static bool spapr_irq_is_lsi(XICSFabric *xi, int irq)
> >> +{
> >> + sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> >> + int srcno = irq - spapr->irq_base;
> >> +
> >> + return (srcno >= 0) && (srcno < SPAPR_MAX_LSI);
> >> +}
> >> +
> >> static void spapr_pic_print_info(InterruptStatsProvider *obj,
> >> Monitor *mon)
> >> {
> >> @@ -3860,6 +3884,7 @@ static void
> >> spapr_machine_2_11_class_options(MachineClass *mc)
> >> xic->irq_test = spapr_irq_test_2_11;
> >> xic->irq_alloc_block = spapr_irq_alloc_block_2_11;
> >> xic->irq_free_block = spapr_irq_free_block_2_11;
> >> + xic->irq_is_lsi = spapr_irq_is_lsi_2_11;
> >> }
> >>
> >> DEFINE_SPAPR_MACHINE(2_11, "2.11", false);
> >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> >> index 478f8e510179..292b929e88eb 100644
> >> --- a/include/hw/ppc/xics.h
> >> +++ b/include/hw/ppc/xics.h
> >> @@ -177,7 +177,7 @@ typedef struct XICSFabricClass {
> >> ICPState *(*icp_get)(XICSFabric *xi, int server);
> >> /* IRQ allocator helpers */
> >> bool (*irq_test)(XICSFabric *xi, int irq);
> >> - int (*irq_alloc_block)(XICSFabric *xi, int count, int align);
> >> + int (*irq_alloc_block)(XICSFabric *xi, int count, int align, bool
> >> lsi);
> >> void (*irq_free_block)(XICSFabric *xi, int irq, int num);
> >> bool (*irq_is_lsi)(XICSFabric *xi, int irq);
> >> } XICSFabricClass;
> >
>
