On Mon, Dec 04, 2017 at 04:30:36PM +0100, Cédric Le Goater wrote:
> On 12/04/2017 08:52 AM, David Gibson wrote:
> > On Thu, Nov 23, 2017 at 02:29:51PM +0100, Cédric Le Goater wrote:
> >> When the XIVE interrupt mode is activated, the machine needs to expose
> >> to the guest the MMIO regions use by the controller :
> >>
> >> - Event State Buffer (ESB)
> >> - Thread Interrupt Management Area (TIMA)
> >>
> >> Migration will also need to reflect the current interrupt mode in use.
> >>
> >> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> >> ---
> >> hw/intc/spapr_xive_hcall.c | 14 ++++++++++++++
> >> hw/ppc/spapr.c | 5 +++++
> >> include/hw/ppc/spapr_xive.h | 1 +
> >> 3 files changed, 20 insertions(+)
> >>
> >> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c
> >> index 60c6c9f4be8f..ba217144878e 100644
> >> --- a/hw/intc/spapr_xive_hcall.c
> >> +++ b/hw/intc/spapr_xive_hcall.c
> >> @@ -933,3 +933,17 @@ void spapr_xive_populate(sPAPRMachineState *spapr,
> >> int nr_servers,
> >> _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
> >> plat_res_int_priorities,
> >> sizeof(plat_res_int_priorities)));
> >> }
> >> +
> >> +void spapr_xive_mmio_map(sPAPRMachineState *spapr)
> >> +{
> >> + sPAPRXive *xive = spapr->xive;
> >> +
> >> + /* ESBs */
> >> + sysbus_mmio_map(SYS_BUS_DEVICE(xive), 0, xive->esb_base);
> >> +
> >> + /* Thread Management Interrupt Areas */
> >> + /* TODO: Only map the OS TIMA for the moment. Mapping the whole
> >> + * region needs some rework in the handlers */
> >> + sysbus_mmio_map(SYS_BUS_DEVICE(xive), 1,
> >> + xive->tm_base + (1 << xive->tm_shift));
> >
> > You probably shouldn't be exposing the user TIMA in the DT if you're
> > only allowing the OS TIME to be mapped.
>
> The specs requires to map both Uset and OS TIMA.
Ok.
> >
> >> +}
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 3a62369883cc..734706c18cb3 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -1132,6 +1132,7 @@ static void *spapr_build_fdt(sPAPRMachineState
> >> *spapr,
> >> } else {
> >> /* Populate device tree for XIVE */
> >> spapr_xive_populate(spapr, xics_max_server_number(), fdt,
> >> PHANDLE_XICP);
> >> + spapr_xive_mmio_map(spapr);
> >
> > This doesn't belong here, spapr_build_fdt() should _just_ build the
> > fdt, not have side effects on the actual device state.
>
> Yes. I will move the rest of the XIVE setup in the reset handler
> before the device tree is built.
Ok.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
