| Message ID | 20170911171235.29331-19-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | Guest exploitation of the XIVE interrupt controller (POWER9) | expand |
On Mon, Sep 11, 2017 at 07:12:32PM +0200, Cédric Le Goater wrote: > Like for XICS, the XIVE interface for the guest is described in the > device tree under the "interrupt-controller" node. A couple of new > properties are specific to XIVE : > > - "reg" > > contains the base address and size of the thread interrupt > managnement areas (TIMA), also called rings, for the User level and > for the Guest OS level. Only the Guest OS level is taken into > account today. > > - "ibm,xive-eq-sizes" > > the size of the event queues. One cell per size supported, contains > log2 of size, in ascending order. > > - "ibm,xive-lisn-ranges" > > the interrupt numbers ranges assigned to the guest. These are > allocated using a simple bitmap. > > and also under the root node : > > - "ibm,plat-res-int-priorities" > > contains a list of priorities that the hypervisor has reserved for > its own use. Simulate ranges as defined by the PowerVM Hypervisor. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/intc/spapr_xive_hcall.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr_xive.h | 1 + > 2 files changed, 55 insertions(+) > > diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c > index 4c77b65683de..7b19ea6373dd 100644 > --- a/hw/intc/spapr_xive_hcall.c > +++ b/hw/intc/spapr_xive_hcall.c > @@ -874,3 +874,57 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr) > spapr_register_hypercall(H_INT_SYNC, h_int_sync); > spapr_register_hypercall(H_INT_RESET, h_int_reset); > } > + > +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle) > +{ > + int node; > + uint64_t timas[2 * 2]; > + uint32_t lisn_ranges[] = { > + cpu_to_be32(xive->nr_irqs - xive->nr_targets + xive->ics->offset), > + cpu_to_be32(xive->nr_targets), > + }; > + uint32_t eq_sizes[] = { > + cpu_to_be32(12), /* 4K */ > + cpu_to_be32(16), /* 64K */ > + cpu_to_be32(21), /* 2M */ > + cpu_to_be32(24), /* 16M */ > + }; > + > + /* Use some ranges to exercise the Linux driver, which should > + * result in Linux choosing priority 6. This is not strictly > + * necessary > + */ > + uint32_t reserved_priorities[] = { > + cpu_to_be32(1), /* start */ > + cpu_to_be32(2), /* count */ > + cpu_to_be32(7), /* start */ > + cpu_to_be32(0xf8), /* count */ > + }; > + int i; > + > + /* Thread Interrupt Management Areas : User and OS */ > + for (i = 0; i < 2; i++) { > + timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << xive->tm_shift)); > + timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift); > + } > + > + _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller")); > + > + _FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller")); Shouldn't need this - SLOF will figure it out from the node name above. > + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); > + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); > + > + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); > + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, > + sizeof(eq_sizes))); > + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, > + sizeof(lisn_ranges))); I note this doesn't have the interrupt-controller or #interrupt-cells properties. So what acts as the interrupt parent for all the devices in the tree with XIVE? > + /* For SLOF */ > + _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); > + _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); > + > + /* top properties */ > + _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities", > + reserved_priorities, sizeof(reserved_priorities))); > +} > diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h > index ae5ff89533c0..0a156f2d8591 100644 > --- a/include/hw/ppc/spapr_xive.h > +++ b/include/hw/ppc/spapr_xive.h > @@ -69,5 +69,6 @@ struct sPAPRXive { > typedef struct sPAPRMachineState sPAPRMachineState; > > void spapr_xive_hcall_init(sPAPRMachineState *spapr); > +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle); > > #endif /* PPC_SPAPR_XIVE_H */
On 09/19/2017 10:44 AM, David Gibson wrote: > On Mon, Sep 11, 2017 at 07:12:32PM +0200, Cédric Le Goater wrote: >> Like for XICS, the XIVE interface for the guest is described in the >> device tree under the "interrupt-controller" node. A couple of new >> properties are specific to XIVE : >> >> - "reg" >> >> contains the base address and size of the thread interrupt >> managnement areas (TIMA), also called rings, for the User level and >> for the Guest OS level. Only the Guest OS level is taken into >> account today. >> >> - "ibm,xive-eq-sizes" >> >> the size of the event queues. One cell per size supported, contains >> log2 of size, in ascending order. >> >> - "ibm,xive-lisn-ranges" >> >> the interrupt numbers ranges assigned to the guest. These are >> allocated using a simple bitmap. >> >> and also under the root node : >> >> - "ibm,plat-res-int-priorities" >> >> contains a list of priorities that the hypervisor has reserved for >> its own use. Simulate ranges as defined by the PowerVM Hypervisor. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/intc/spapr_xive_hcall.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr_xive.h | 1 + >> 2 files changed, 55 insertions(+) >> >> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c >> index 4c77b65683de..7b19ea6373dd 100644 >> --- a/hw/intc/spapr_xive_hcall.c >> +++ b/hw/intc/spapr_xive_hcall.c >> @@ -874,3 +874,57 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr) >> spapr_register_hypercall(H_INT_SYNC, h_int_sync); >> spapr_register_hypercall(H_INT_RESET, h_int_reset); >> } >> + >> +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle) >> +{ >> + int node; >> + uint64_t timas[2 * 2]; >> + uint32_t lisn_ranges[] = { >> + cpu_to_be32(xive->nr_irqs - xive->nr_targets + xive->ics->offset), >> + cpu_to_be32(xive->nr_targets), >> + }; >> + uint32_t eq_sizes[] = { >> + cpu_to_be32(12), /* 4K */ >> + cpu_to_be32(16), /* 64K */ >> + cpu_to_be32(21), /* 2M */ >> + cpu_to_be32(24), /* 16M */ >> + }; >> + >> + /* Use some ranges to exercise the Linux driver, which should >> + * result in Linux choosing priority 6. This is not strictly >> + * necessary >> + */ >> + uint32_t reserved_priorities[] = { >> + cpu_to_be32(1), /* start */ >> + cpu_to_be32(2), /* count */ >> + cpu_to_be32(7), /* start */ >> + cpu_to_be32(0xf8), /* count */ >> + }; >> + int i; >> + >> + /* Thread Interrupt Management Areas : User and OS */ >> + for (i = 0; i < 2; i++) { >> + timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << xive->tm_shift)); >> + timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift); >> + } >> + >> + _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller")); >> + >> + _FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller")); > > Shouldn't need this - SLOF will figure it out from the node name above. It is in the specs. phyp has it. we might as well keep it. > >> + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); >> + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); >> + >> + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); >> + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, >> + sizeof(eq_sizes))); >> + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, >> + sizeof(lisn_ranges))); > > I note this doesn't have the interrupt-controller or #interrupt-cells > properties. So what acts as the interrupt parent for all the devices > in the tree with XIVE? these properties are not in the specs anymore for the interrupt-controller node and I don't think Linux makes use of them (even for XICS). So it just works fine. C. >> + /* For SLOF */ >> + _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); >> + _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); >> + >> + /* top properties */ >> + _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities", >> + reserved_priorities, sizeof(reserved_priorities))); >> +} >> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h >> index ae5ff89533c0..0a156f2d8591 100644 >> --- a/include/hw/ppc/spapr_xive.h >> +++ b/include/hw/ppc/spapr_xive.h >> @@ -69,5 +69,6 @@ struct sPAPRXive { >> typedef struct sPAPRMachineState sPAPRMachineState; >> >> void spapr_xive_hcall_init(sPAPRMachineState *spapr); >> +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle); >> >> #endif /* PPC_SPAPR_XIVE_H */ >
On Wed, Sep 20, 2017 at 02:26:32PM +0200, Cédric Le Goater wrote: > On 09/19/2017 10:44 AM, David Gibson wrote: > > On Mon, Sep 11, 2017 at 07:12:32PM +0200, Cédric Le Goater wrote: > >> Like for XICS, the XIVE interface for the guest is described in the > >> device tree under the "interrupt-controller" node. A couple of new > >> properties are specific to XIVE : > >> > >> - "reg" > >> > >> contains the base address and size of the thread interrupt > >> managnement areas (TIMA), also called rings, for the User level and > >> for the Guest OS level. Only the Guest OS level is taken into > >> account today. > >> > >> - "ibm,xive-eq-sizes" > >> > >> the size of the event queues. One cell per size supported, contains > >> log2 of size, in ascending order. > >> > >> - "ibm,xive-lisn-ranges" > >> > >> the interrupt numbers ranges assigned to the guest. These are > >> allocated using a simple bitmap. > >> > >> and also under the root node : > >> > >> - "ibm,plat-res-int-priorities" > >> > >> contains a list of priorities that the hypervisor has reserved for > >> its own use. Simulate ranges as defined by the PowerVM Hypervisor. > >> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >> --- > >> hw/intc/spapr_xive_hcall.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ > >> include/hw/ppc/spapr_xive.h | 1 + > >> 2 files changed, 55 insertions(+) > >> > >> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c > >> index 4c77b65683de..7b19ea6373dd 100644 > >> --- a/hw/intc/spapr_xive_hcall.c > >> +++ b/hw/intc/spapr_xive_hcall.c > >> @@ -874,3 +874,57 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr) > >> spapr_register_hypercall(H_INT_SYNC, h_int_sync); > >> spapr_register_hypercall(H_INT_RESET, h_int_reset); > >> } > >> + > >> +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle) > >> +{ > >> + int node; > >> + uint64_t timas[2 * 2]; > >> + uint32_t lisn_ranges[] = { > >> + cpu_to_be32(xive->nr_irqs - xive->nr_targets + xive->ics->offset), > >> + cpu_to_be32(xive->nr_targets), > >> + }; > >> + uint32_t eq_sizes[] = { > >> + cpu_to_be32(12), /* 4K */ > >> + cpu_to_be32(16), /* 64K */ > >> + cpu_to_be32(21), /* 2M */ > >> + cpu_to_be32(24), /* 16M */ > >> + }; > >> + > >> + /* Use some ranges to exercise the Linux driver, which should > >> + * result in Linux choosing priority 6. This is not strictly > >> + * necessary > >> + */ > >> + uint32_t reserved_priorities[] = { > >> + cpu_to_be32(1), /* start */ > >> + cpu_to_be32(2), /* count */ > >> + cpu_to_be32(7), /* start */ > >> + cpu_to_be32(0xf8), /* count */ > >> + }; > >> + int i; > >> + > >> + /* Thread Interrupt Management Areas : User and OS */ > >> + for (i = 0; i < 2; i++) { > >> + timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << xive->tm_shift)); > >> + timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift); > >> + } > >> + > >> + _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller")); > >> + > >> + _FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller")); > > > > Shouldn't need this - SLOF will figure it out from the node name above. > > It is in the specs. phyp has it. we might as well keep it. You misunderstand. SLOF will *create* the name property based on the node name. Adding it here has *no effect*. > >> + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); > >> + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); > >> + > >> + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); > >> + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, > >> + sizeof(eq_sizes))); > >> + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, > >> + sizeof(lisn_ranges))); > > > > I note this doesn't have the interrupt-controller or #interrupt-cells > > properties. So what acts as the interrupt parent for all the devices > > in the tree with XIVE? > > these properties are not in the specs anymore for the interrupt-controller > node and I don't think Linux makes use of them (even for XICS). So > it just works fine. Um.. what!? Are you saying that the PAPR XIVE spec completely broke how interrupt specifiers have worked in the device tree since forever? And I'm pretty sure Linux does make use of them. Without #interrupt-cells, there's no way it can properly interpret the interrupts properties in the device nodes.
On 09/21/2017 03:35 AM, David Gibson wrote: > On Wed, Sep 20, 2017 at 02:26:32PM +0200, Cédric Le Goater wrote: >> On 09/19/2017 10:44 AM, David Gibson wrote: >>> On Mon, Sep 11, 2017 at 07:12:32PM +0200, Cédric Le Goater wrote: >>>> Like for XICS, the XIVE interface for the guest is described in the >>>> device tree under the "interrupt-controller" node. A couple of new >>>> properties are specific to XIVE : >>>> >>>> - "reg" >>>> >>>> contains the base address and size of the thread interrupt >>>> managnement areas (TIMA), also called rings, for the User level and >>>> for the Guest OS level. Only the Guest OS level is taken into >>>> account today. >>>> >>>> - "ibm,xive-eq-sizes" >>>> >>>> the size of the event queues. One cell per size supported, contains >>>> log2 of size, in ascending order. >>>> >>>> - "ibm,xive-lisn-ranges" >>>> >>>> the interrupt numbers ranges assigned to the guest. These are >>>> allocated using a simple bitmap. >>>> >>>> and also under the root node : >>>> >>>> - "ibm,plat-res-int-priorities" >>>> >>>> contains a list of priorities that the hypervisor has reserved for >>>> its own use. Simulate ranges as defined by the PowerVM Hypervisor. >>>> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> hw/intc/spapr_xive_hcall.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ >>>> include/hw/ppc/spapr_xive.h | 1 + >>>> 2 files changed, 55 insertions(+) >>>> >>>> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c >>>> index 4c77b65683de..7b19ea6373dd 100644 >>>> --- a/hw/intc/spapr_xive_hcall.c >>>> +++ b/hw/intc/spapr_xive_hcall.c >>>> @@ -874,3 +874,57 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr) >>>> spapr_register_hypercall(H_INT_SYNC, h_int_sync); >>>> spapr_register_hypercall(H_INT_RESET, h_int_reset); >>>> } >>>> + >>>> +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle) >>>> +{ >>>> + int node; >>>> + uint64_t timas[2 * 2]; >>>> + uint32_t lisn_ranges[] = { >>>> + cpu_to_be32(xive->nr_irqs - xive->nr_targets + xive->ics->offset), >>>> + cpu_to_be32(xive->nr_targets), >>>> + }; >>>> + uint32_t eq_sizes[] = { >>>> + cpu_to_be32(12), /* 4K */ >>>> + cpu_to_be32(16), /* 64K */ >>>> + cpu_to_be32(21), /* 2M */ >>>> + cpu_to_be32(24), /* 16M */ >>>> + }; >>>> + >>>> + /* Use some ranges to exercise the Linux driver, which should >>>> + * result in Linux choosing priority 6. This is not strictly >>>> + * necessary >>>> + */ >>>> + uint32_t reserved_priorities[] = { >>>> + cpu_to_be32(1), /* start */ >>>> + cpu_to_be32(2), /* count */ >>>> + cpu_to_be32(7), /* start */ >>>> + cpu_to_be32(0xf8), /* count */ >>>> + }; >>>> + int i; >>>> + >>>> + /* Thread Interrupt Management Areas : User and OS */ >>>> + for (i = 0; i < 2; i++) { >>>> + timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << xive->tm_shift)); >>>> + timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift); >>>> + } >>>> + >>>> + _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller")); >>>> + >>>> + _FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller")); >>> >>> Shouldn't need this - SLOF will figure it out from the node name above. >> >> It is in the specs. phyp has it. we might as well keep it. > > You misunderstand. SLOF will *create* the name property based on the > node name. Adding it here has *no effect*. ok. I was not ware of that. I will remove it then. >>>> + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); >>>> + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); >>>> + >>>> + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); >>>> + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, >>>> + sizeof(eq_sizes))); >>>> + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, >>>> + sizeof(lisn_ranges))); >>> >>> I note this doesn't have the interrupt-controller or #interrupt-cells >>> properties. So what acts as the interrupt parent for all the devices >>> in the tree with XIVE? >> >> these properties are not in the specs anymore for the interrupt-controller >> node and I don't think Linux makes use of them (even for XICS). So >> it just works fine. > > Um.. what!? Are you saying that the PAPR XIVE spec completely broke > how interrupt specifiers have worked in the device tree since forever? Let me be more precise. I am saying that the interrupt-controller and #interrupt-cells properties are not needed under the main interrupt controller node. They can be removed from the tree and the Linux guest kernel will boot perfectly well. These properties still are needed under the sub nodes like : /proc/device-tree/vdevice/interrupt-controller /proc/device-tree/event-sources/interrupt-controller C. > And I'm pretty sure Linux does make use of them. Without > #interrupt-cells, there's no way it can properly interpret the > interrupts properties in the device nodes. >
On Thu, Sep 21, 2017 at 01:21:10PM +0200, Cédric Le Goater wrote: > On 09/21/2017 03:35 AM, David Gibson wrote: > > On Wed, Sep 20, 2017 at 02:26:32PM +0200, Cédric Le Goater wrote: > >> On 09/19/2017 10:44 AM, David Gibson wrote: > >>> On Mon, Sep 11, 2017 at 07:12:32PM +0200, Cédric Le Goater wrote: > >>>> Like for XICS, the XIVE interface for the guest is described in the > >>>> device tree under the "interrupt-controller" node. A couple of new > >>>> properties are specific to XIVE : > >>>> > >>>> - "reg" > >>>> > >>>> contains the base address and size of the thread interrupt > >>>> managnement areas (TIMA), also called rings, for the User level and > >>>> for the Guest OS level. Only the Guest OS level is taken into > >>>> account today. > >>>> > >>>> - "ibm,xive-eq-sizes" > >>>> > >>>> the size of the event queues. One cell per size supported, contains > >>>> log2 of size, in ascending order. > >>>> > >>>> - "ibm,xive-lisn-ranges" > >>>> > >>>> the interrupt numbers ranges assigned to the guest. These are > >>>> allocated using a simple bitmap. > >>>> > >>>> and also under the root node : > >>>> > >>>> - "ibm,plat-res-int-priorities" > >>>> > >>>> contains a list of priorities that the hypervisor has reserved for > >>>> its own use. Simulate ranges as defined by the PowerVM Hypervisor. > >>>> > >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >>>> --- > >>>> hw/intc/spapr_xive_hcall.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ > >>>> include/hw/ppc/spapr_xive.h | 1 + > >>>> 2 files changed, 55 insertions(+) > >>>> > >>>> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c > >>>> index 4c77b65683de..7b19ea6373dd 100644 > >>>> --- a/hw/intc/spapr_xive_hcall.c > >>>> +++ b/hw/intc/spapr_xive_hcall.c > >>>> @@ -874,3 +874,57 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr) > >>>> spapr_register_hypercall(H_INT_SYNC, h_int_sync); > >>>> spapr_register_hypercall(H_INT_RESET, h_int_reset); > >>>> } > >>>> + > >>>> +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle) > >>>> +{ > >>>> + int node; > >>>> + uint64_t timas[2 * 2]; > >>>> + uint32_t lisn_ranges[] = { > >>>> + cpu_to_be32(xive->nr_irqs - xive->nr_targets + xive->ics->offset), > >>>> + cpu_to_be32(xive->nr_targets), > >>>> + }; > >>>> + uint32_t eq_sizes[] = { > >>>> + cpu_to_be32(12), /* 4K */ > >>>> + cpu_to_be32(16), /* 64K */ > >>>> + cpu_to_be32(21), /* 2M */ > >>>> + cpu_to_be32(24), /* 16M */ > >>>> + }; > >>>> + > >>>> + /* Use some ranges to exercise the Linux driver, which should > >>>> + * result in Linux choosing priority 6. This is not strictly > >>>> + * necessary > >>>> + */ > >>>> + uint32_t reserved_priorities[] = { > >>>> + cpu_to_be32(1), /* start */ > >>>> + cpu_to_be32(2), /* count */ > >>>> + cpu_to_be32(7), /* start */ > >>>> + cpu_to_be32(0xf8), /* count */ > >>>> + }; > >>>> + int i; > >>>> + > >>>> + /* Thread Interrupt Management Areas : User and OS */ > >>>> + for (i = 0; i < 2; i++) { > >>>> + timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << xive->tm_shift)); > >>>> + timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift); > >>>> + } > >>>> + > >>>> + _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller")); > >>>> + > >>>> + _FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller")); > >>> > >>> Shouldn't need this - SLOF will figure it out from the node name above. > >> > >> It is in the specs. phyp has it. we might as well keep it. > > > > You misunderstand. SLOF will *create* the name property based on the > > node name. Adding it here has *no effect*. > > ok. I was not ware of that. I will remove it then. Historical aside: in traditional OF there aren't "node names" as such. Each node has a 'name' and 'reg' property and the "node name" displayed in listings is formed from those as name@unit-address - with the tricky catch being that unit-address is encoded from 'reg' in a bus specific manner (using what's essentially a method attached to the parent node). Obviously that's awkward in the flat tree world, since we can't have methods. So instead nodes have a real string name built into the structure including both the name and unit address components. 'name' properties are generally omitted and derived from that name. 'reg' should match according to the bus's encoding conventions, but the number of things that can actually verify that is relatively small. > >>>> + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); > >>>> + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); > >>>> + > >>>> + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); > >>>> + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, > >>>> + sizeof(eq_sizes))); > >>>> + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, > >>>> + sizeof(lisn_ranges))); > >>> > >>> I note this doesn't have the interrupt-controller or #interrupt-cells > >>> properties. So what acts as the interrupt parent for all the devices > >>> in the tree with XIVE? > >> > >> these properties are not in the specs anymore for the interrupt-controller > >> node and I don't think Linux makes use of them (even for XICS). So > >> it just works fine. > > > > Um.. what!? Are you saying that the PAPR XIVE spec completely broke > > how interrupt specifiers have worked in the device tree since forever? > > Let me be more precise. I am saying that the interrupt-controller > and #interrupt-cells properties are not needed under the main interrupt > controller node. They can be removed from the tree and the Linux guest > kernel will boot perfectly well. > > These properties still are needed under the sub nodes like : > > /proc/device-tree/vdevice/interrupt-controller > /proc/device-tree/event-sources/interrupt-controller Um. This still makes no sense. In order to have a common interrupt space, those nodes must have an interrupt-parent pointing somewhere - the top level interrupt controller, which needs interrupt-controller and #interrupt-cells properties. Note that that will be the "source" side of the intc. There could also be a presentation side of the intc, which wouldn't need those properties.
On Thu, 2017-09-21 at 11:35 +1000, David Gibson wrote: > > >> + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); > > >> + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); > > >> + > > >> + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); > > >> + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, > > >> + sizeof(eq_sizes))); > > >> + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, > > >> + sizeof(lisn_ranges))); > > > > > > I note this doesn't have the interrupt-controller or #interrupt-cells > > > properties. So what acts as the interrupt parent for all the devices > > > in the tree with XIVE? > > > > these properties are not in the specs anymore for the interrupt-controller > > node and I don't think Linux makes use of them (even for XICS). So > > it just works fine. > > Um.. what!? Are you saying that the PAPR XIVE spec completely broke > how interrupt specifiers have worked in the device tree since forever? > > And I'm pretty sure Linux does make use of them. Without > #interrupt-cells, there's no way it can properly interpret the > interrupts properties in the device nodes. Linux does make use of them and they are in the spec, but don't confuse the nodes for the presentation controllers vs the node for the virtual source controller which is the one that is the root of the interrupt tree. Cheers, Ben.
On Thu, 2017-09-21 at 13:21 +0200, Cédric Le Goater wrote: > Let me be more precise. I am saying that the interrupt-controller > and #interrupt-cells properties are not needed under the main interrupt > controller node. They can be removed from the tree and the Linux guest > kernel will boot perfectly well. No they are needed. They are the parents of PCI interrupts for example. There's something fishy here. Do you have a DT snapshot from pHyp for me to look at ? > These properties still are needed under the sub nodes like : > > /proc/device-tree/vdevice/interrupt-controller > /proc/device-tree/event-sources/interrupt-controller
On 09/28/2017 10:43 AM, Benjamin Herrenschmidt wrote: > On Thu, 2017-09-21 at 13:21 +0200, Cédric Le Goater wrote: >> Let me be more precise. I am saying that the interrupt-controller >> and #interrupt-cells properties are not needed under the main interrupt >> controller node. They can be removed from the tree and the Linux guest >> kernel will boot perfectly well. > > No they are needed. They are the parents of PCI interrupts for example. > There's something fishy here. probably, I just removed the properties under QEMU and could boot the guest, with disks and network. > Do you have a DT snapshot from pHyp for me to look at ? # lsprop /proc/device-tree/interrupt-controller\@200010000/ compatible "ibm,power-ivpe" device_type "power-ivpe" ibm,xive-eq-sizes 00000007 00000009 0000000c 0000000e 00000010 00000012 00000015 00000016 00000018 reg 00000002 00010000 00000000 00010000 00000002 00000000 00000000 00010000 linux,phandle 00dce438 (14476344) ibm,xive-lisn-ranges 00094000 00000030 name "interrupt-controller" Cheers, C. > >> These properties still are needed under the sub nodes like : >> >> /proc/device-tree/vdevice/interrupt-controller >> /proc/device-tree/event-sources/interrupt-controller
On Thu, 2017-09-28 at 10:51 +0200, Cédric Le Goater wrote: > probably, I just removed the properties under QEMU and could > boot the guest, with disks and network. As long as you don't use LSIs... > > > Do you have a DT snapshot from pHyp for me to look at ? > > > # lsprop /proc/device-tree/interrupt-controller\@200010000/ > compatible "ibm,power-ivpe" > device_type "power-ivpe" > ibm,xive-eq-sizes > 00000007 00000009 0000000c 0000000e > 00000010 00000012 00000015 00000016 > 00000018 > reg 00000002 00010000 00000000 00010000 > 00000002 00000000 00000000 00010000 > linux,phandle 00dce438 (14476344) > ibm,xive-lisn-ranges > 00094000 00000030 > name "interrupt-controller" > > > Cheers,
On 09/28/2017 12:03 PM, Benjamin Herrenschmidt wrote: > On Thu, 2017-09-28 at 10:51 +0200, Cédric Le Goater wrote: >> probably, I just removed the properties under QEMU and could >> boot the guest, with disks and network. > > As long as you don't use LSIs... That I didn't test much. Which the devices could I use for the guest ? Thanks, C.
diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c index 4c77b65683de..7b19ea6373dd 100644 --- a/hw/intc/spapr_xive_hcall.c +++ b/hw/intc/spapr_xive_hcall.c @@ -874,3 +874,57 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr) spapr_register_hypercall(H_INT_SYNC, h_int_sync); spapr_register_hypercall(H_INT_RESET, h_int_reset); } + +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle) +{ + int node; + uint64_t timas[2 * 2]; + uint32_t lisn_ranges[] = { + cpu_to_be32(xive->nr_irqs - xive->nr_targets + xive->ics->offset), + cpu_to_be32(xive->nr_targets), + }; + uint32_t eq_sizes[] = { + cpu_to_be32(12), /* 4K */ + cpu_to_be32(16), /* 64K */ + cpu_to_be32(21), /* 2M */ + cpu_to_be32(24), /* 16M */ + }; + + /* Use some ranges to exercise the Linux driver, which should + * result in Linux choosing priority 6. This is not strictly + * necessary + */ + uint32_t reserved_priorities[] = { + cpu_to_be32(1), /* start */ + cpu_to_be32(2), /* count */ + cpu_to_be32(7), /* start */ + cpu_to_be32(0xf8), /* count */ + }; + int i; + + /* Thread Interrupt Management Areas : User and OS */ + for (i = 0; i < 2; i++) { + timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << xive->tm_shift)); + timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift); + } + + _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller")); + + _FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller")); + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); + + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, + sizeof(eq_sizes))); + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, + sizeof(lisn_ranges))); + + /* For SLOF */ + _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); + _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); + + /* top properties */ + _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities", + reserved_priorities, sizeof(reserved_priorities))); +} diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h index ae5ff89533c0..0a156f2d8591 100644 --- a/include/hw/ppc/spapr_xive.h +++ b/include/hw/ppc/spapr_xive.h @@ -69,5 +69,6 @@ struct sPAPRXive { typedef struct sPAPRMachineState sPAPRMachineState; void spapr_xive_hcall_init(sPAPRMachineState *spapr); +void spapr_xive_populate(sPAPRXive *xive, void *fdt, uint32_t phandle); #endif /* PPC_SPAPR_XIVE_H */
Like for XICS, the XIVE interface for the guest is described in the device tree under the "interrupt-controller" node. A couple of new properties are specific to XIVE : - "reg" contains the base address and size of the thread interrupt managnement areas (TIMA), also called rings, for the User level and for the Guest OS level. Only the Guest OS level is taken into account today. - "ibm,xive-eq-sizes" the size of the event queues. One cell per size supported, contains log2 of size, in ascending order. - "ibm,xive-lisn-ranges" the interrupt numbers ranges assigned to the guest. These are allocated using a simple bitmap. and also under the root node : - "ibm,plat-res-int-priorities" contains a list of priorities that the hypervisor has reserved for its own use. Simulate ranges as defined by the PowerVM Hypervisor. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/intc/spapr_xive_hcall.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr_xive.h | 1 + 2 files changed, 55 insertions(+)