| Message ID | 159679992680.876294.7520540158586170894.stgit@bahia.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr: Cleanups for XIVE | expand |
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 <clg@kaod.org> > Signed-off-by: Greg Kurz <groug@kaod.org> This is much better Reviewed-by: Cédric Le Goater <clg@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) > { > >
On Fri, Aug 07, 2020 at 02:36:01PM +0200, Cédric Le Goater wrote: > 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 <clg@kaod.org> > > Signed-off-by: Greg Kurz <groug@kaod.org> > > This is much better > > Reviewed-by: Cédric Le Goater <clg@kaod.org> Appled to ppc-for-5.2. > > 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) > > { > > > > >
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) {
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 <clg@kaod.org> Signed-off-by: Greg Kurz <groug@kaod.org> --- 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(-)