On Mon, 16 Sep 2019 10:44:32 +1000 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Wed, Sep 11, 2019 at 03:39:37PM +0200, Cédric Le Goater wrote:
> > A typical pseries VM with 16 vCPUs, one disk, one network adapater
> > uses less than 100 interrupts but the whole IRQ number space of the
> > QEMU machine is allocated at reset time and it is 8K wide. This is
> > wasting a considerable amount of interrupt numbers in the global IRQ
> > space which has 1M interrupts per socket on a POWER9.
> >
> > To optimise the HW resources, only request at the KVM level interrupts
> > which have been claimed by the guest. This will help to increase the
> > maximum number of VMs per system and also help supporting nested guests
> > using the XIVE interrupt mode.
> >
> > Signed-off-by: Cédric Le Goater <c...@kaod.org>
>
> Applied to ppc-for-4.2, thanks.
>
While experimenting 4.1->4.2 migration with your irq cleanup series,
I've hit this:
qemu-system-ppc64: KVM_SET_DEVICE_ATTR failed: Group 3 attr 0x0000000000001300:
Invalid argument
qemu-system-ppc64: error while loading state for instance 0x0 of device 'spapr'
qemu-system-ppc64: load of migration failed: Operation not permitted
Failing to restore the source config with EINVAL seems to come from the
following check in kvmppc_xive_native_set_source_config():
if (!state->valid)
return -EINVAL;
which makes sense since we haven't requested any interrupt yet.
We should hence do it at post load before restoring the source
config.
I'll send a patch ASAP.
> > ---
> > hw/intc/spapr_xive_kvm.c | 29 ++++++++++++++++++++++++++---
> > hw/intc/xics_kvm.c | 8 ++++++++
> > 2 files changed, 34 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> > index 17af4d19f54e..71b88d7797bc 100644
> > --- a/hw/intc/spapr_xive_kvm.c
> > +++ b/hw/intc/spapr_xive_kvm.c
> > @@ -255,11 +255,16 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc,
> > int srcno, Error **errp)
> >
> > static void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp)
> > {
> > + SpaprXive *xive = SPAPR_XIVE(xsrc->xive);
> > int i;
> >
> > for (i = 0; i < xsrc->nr_irqs; i++) {
> > Error *local_err = NULL;
> >
> > + if (!xive_eas_is_valid(&xive->eat[i])) {
> > + continue;
> > + }
> > +
> > kvmppc_xive_source_reset_one(xsrc, i, &local_err);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > @@ -328,11 +333,18 @@ uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int
> > srcno, uint32_t offset,
> >
> > static void kvmppc_xive_source_get_state(XiveSource *xsrc)
> > {
> > + SpaprXive *xive = SPAPR_XIVE(xsrc->xive);
> > int i;
> >
> > for (i = 0; i < xsrc->nr_irqs; i++) {
> > + uint8_t pq;
> > +
> > + if (!xive_eas_is_valid(&xive->eat[i])) {
> > + continue;
> > + }
> > +
> > /* Perform a load without side effect to retrieve the PQ bits */
> > - uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
> > + pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
> >
> > /* and save PQ locally */
> > xive_source_esb_set(xsrc, i, pq);
> > @@ -521,9 +533,14 @@ static void kvmppc_xive_change_state_handler(void
> > *opaque, int running,
> > */
> > if (running) {
> > for (i = 0; i < xsrc->nr_irqs; i++) {
> > - uint8_t pq = xive_source_esb_get(xsrc, i);
> > + uint8_t pq;
> > uint8_t old_pq;
> >
> > + if (!xive_eas_is_valid(&xive->eat[i])) {
> > + continue;
> > + }
> > +
> > + pq = xive_source_esb_get(xsrc, i);
> > old_pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq <<
> > 8));
> >
> > /*
> > @@ -545,7 +562,13 @@ static void kvmppc_xive_change_state_handler(void
> > *opaque, int running,
> > * migration is in progress.
> > */
> > for (i = 0; i < xsrc->nr_irqs; i++) {
> > - uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
> > + uint8_t pq;
> > +
> > + if (!xive_eas_is_valid(&xive->eat[i])) {
> > + continue;
> > + }
> > +
> > + pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
> >
> > /*
> > * PQ is set to PENDING to possibly catch a triggered
> > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> > index a4d2e876cc5f..ba90d6dc966c 100644
> > --- a/hw/intc/xics_kvm.c
> > +++ b/hw/intc/xics_kvm.c
> > @@ -190,6 +190,10 @@ void ics_get_kvm_state(ICSState *ics)
> > for (i = 0; i < ics->nr_irqs; i++) {
> > ICSIRQState *irq = &ics->irqs[i];
> >
> > + if (ics_irq_free(ics, i)) {
> > + continue;
> > + }
> > +
> > kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> > i + ics->offset, &state, false, &error_fatal);
> >
> > @@ -301,6 +305,10 @@ int ics_set_kvm_state(ICSState *ics, Error **errp)
> > Error *local_err = NULL;
> > int ret;
> >
> > + if (ics_irq_free(ics, i)) {
> > + continue;
> > + }
> > +
> > ret = ics_set_kvm_state_one(ics, i, &local_err);
> > if (ret < 0) {
> > error_propagate(errp, local_err);
>
pgpmP4crbHLGs.pgp
Description: OpenPGP digital signature
