On 8/7/20 1:32 PM, Greg Kurz wrote:
> Depending on whether XIVE is emultated or backed with a KVM XIVE device,
> the ESB MMIOs of a XIVE source point to an I/O memory region or a mapped
> memory region.
>
> This is currently handled by checking kvm_irqchip_in_kernel() returns
> false in xive_source_realize(). This is a bit awkward as we usually
> need to do extra things when we're using the in-kernel backend, not
> less. But most important, we can do better: turn the existing "xive.esb"
> memory region into a plain container, introduce an "xive.esb-emulated"
> I/O subregion and rename the existing "xive.esb" subregion in the KVM
> code to "xive.esb-kvm". Since "xive.esb-kvm" is added with overlap
> and a higher priority, it prevails over "xive.esb-emulated" (ie.
> a guest using KVM XIVE will interact with "xive.esb-kvm" instead of
> the default "xive.esb-emulated" region.
>
> While here, consolidate the computation of the MMIO region size in
> a common helper.
>
> Suggested-by: Cédric Le Goater <c...@kaod.org>
> Signed-off-by: Greg Kurz <gr...@kaod.org>
This is much better
Reviewed-by: Cédric Le Goater <c...@kaod.org>
Thanks,
C.
> ---
> hw/intc/spapr_xive_kvm.c | 4 ++--
> hw/intc/xive.c | 11 ++++++-----
> include/hw/ppc/xive.h | 6 ++++++
> 3 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 893a1ee77e70..6130882be678 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -742,7 +742,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc,
> uint32_t nr_servers,
> SpaprXive *xive = SPAPR_XIVE(intc);
> XiveSource *xsrc = &xive->source;
> Error *local_err = NULL;
> - size_t esb_len = (1ull << xsrc->esb_shift) * xsrc->nr_irqs;
> + size_t esb_len = xive_source_esb_len(xsrc);
> size_t tima_len = 4ull << TM_SHIFT;
> CPUState *cs;
> int fd;
> @@ -788,7 +788,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc,
> uint32_t nr_servers,
> }
>
> memory_region_init_ram_device_ptr(&xsrc->esb_mmio_kvm, OBJECT(xsrc),
> - "xive.esb", esb_len, xsrc->esb_mmap);
> + "xive.esb-kvm", esb_len,
> xsrc->esb_mmap);
> memory_region_add_subregion_overlap(&xsrc->esb_mmio, 0,
> &xsrc->esb_mmio_kvm, 1);
>
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 9b55e0356c62..561d746cd1da 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1128,6 +1128,7 @@ static void xive_source_reset(void *dev)
> static void xive_source_realize(DeviceState *dev, Error **errp)
> {
> XiveSource *xsrc = XIVE_SOURCE(dev);
> + size_t esb_len = xive_source_esb_len(xsrc);
>
> assert(xsrc->xive);
>
> @@ -1147,11 +1148,11 @@ static void xive_source_realize(DeviceState *dev,
> Error **errp)
> xsrc->status = g_malloc0(xsrc->nr_irqs);
> xsrc->lsi_map = bitmap_new(xsrc->nr_irqs);
>
> - if (!kvm_irqchip_in_kernel()) {
> - memory_region_init_io(&xsrc->esb_mmio, OBJECT(xsrc),
> - &xive_source_esb_ops, xsrc, "xive.esb",
> - (1ull << xsrc->esb_shift) * xsrc->nr_irqs);
> - }
> + memory_region_init(&xsrc->esb_mmio, OBJECT(xsrc), "xive.esb", esb_len);
> + memory_region_init_io(&xsrc->esb_mmio_emulated, OBJECT(xsrc),
> + &xive_source_esb_ops, xsrc, "xive.esb-emulated",
> + esb_len);
> + memory_region_add_subregion(&xsrc->esb_mmio, 0,
> &xsrc->esb_mmio_emulated);
>
> qemu_register_reset(xive_source_reset, dev);
> }
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 705cf48176fc..82a61eaca74f 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -191,6 +191,7 @@ typedef struct XiveSource {
> uint64_t esb_flags;
> uint32_t esb_shift;
> MemoryRegion esb_mmio;
> + MemoryRegion esb_mmio_emulated;
>
> /* KVM support */
> void *esb_mmap;
> @@ -215,6 +216,11 @@ static inline bool xive_source_esb_has_2page(XiveSource
> *xsrc)
> xsrc->esb_shift == XIVE_ESB_4K_2PAGE;
> }
>
> +static inline size_t xive_source_esb_len(XiveSource *xsrc)
> +{
> + return (1ull << xsrc->esb_shift) * xsrc->nr_irqs;
> +}
> +
> /* The trigger page is always the first/even page */
> static inline hwaddr xive_source_esb_page(XiveSource *xsrc, uint32_t srcno)
> {
>
>
