Re: [Qemu-devel] [PATCH v8 10/12] spapr: enable XIVE MMIOs at reset

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3204 bytes --]

On Tue, Dec 11, 2018 at 11:38:21PM +0100, Cédric Le Goater wrote:
> Depending on the interrupt mode of the machine, enable or disable the
> XIVE MMIOs.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v7:
> 
>  - renamed spapr_xive_enable_mmio() to spapr_xive_mmio_set_enabled()
> 
>  include/hw/ppc/spapr_xive.h | 1 +
>  hw/intc/spapr_xive.c        | 9 +++++++++
>  hw/ppc/spapr_irq.c          | 8 ++++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 728735dbcfbe..9b49871bdb1a 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -48,5 +48,6 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr);
>  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>                     uint32_t phandle);
>  void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
> +void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable);
>  
>  #endif /* PPC_SPAPR_XIVE_H */
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index aaa5865c4080..cd1b2c06f88b 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -179,6 +179,15 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
>      sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
>  }
>  
> +void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable)
> +{
> +    memory_region_set_enabled(&xive->source.esb_mmio, enable);
> +    memory_region_set_enabled(&xive->tm_mmio, enable);
> +
> +    /* Disable the END ESBs until a guest OS makes use of them */
> +    memory_region_set_enabled(&xive->end_source.esb_mmio, false);
> +}
> +
>  /*
>   * When a Virtual Processor is scheduled to run on a HW thread, the
>   * hypervisor pushes its identifier in the OS CAM line. Emulate the
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 814500f22d34..b1319905327f 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -216,6 +216,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
>      CPU_FOREACH(cs) {
>          spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
>      }
> +
> +    /* Deactivate the XIVE MMIOs */
> +    if (spapr->xive) {
> +        spapr_xive_mmio_set_enabled(spapr->xive, false);
> +    }

Rather than having the XICS reset reach across and disable the XIVE
MMIO, I think it would be preferable for the common wrapper to disable
both XICS and XIVE, then have the intc specific hook re-enable the
correct one.

>  }
>  
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
> @@ -341,6 +346,9 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>          /* (TCG) Set the OS CAM line of the thread interrupt context. */
>          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
>      }
> +
> +    /* Activate the XIVE MMIOs */
> +    spapr_xive_mmio_set_enabled(spapr->xive, true);
>  }
>  
>  /*

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]