| Message ID | 1499864265-144136-9-git-send-email-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 12 Jul 2017 14:57:42 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > From: Fei Li <sherrylf@linux.vnet.ibm.com> > > In order to emulate the adapter interruption suppression (AIS) > facility properly, the guest needs to be able to modify the AIS mask. > Interrupt suppression will be handled via the flic (for kvm, via a > recently introduced kernel backend; for !kvm, in the flic code), so > let's introduce a method to change the mode via the flic interface. > > Co-authored-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com> > Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com> > Signed-off-by: Fei Li <sherrylf@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/intc/s390_flic.c | 37 ++++++++++++++++++++++++++++++++++++- > hw/intc/s390_flic_kvm.c | 36 ++++++++++++++++++++++++++++++++++++ > include/hw/s390x/s390_flic.h | 9 ++++++++- > 3 files changed, 80 insertions(+), 2 deletions(-) > > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c > index d3938b3..ce7f355 100644 > --- a/hw/intc/s390_flic.c > +++ b/hw/intc/s390_flic.c > @@ -79,15 +79,47 @@ static int qemu_s390_clear_io_flic(S390FLICState *fs, uint16_t subchannel_id, > return -ENOSYS; > } > > +static int qemu_s390_modify_ais_mode(S390FLICState *fs, uint8_t isc, > + uint16_t mode) > +{ > + QEMUS390FLICState *flic = QEMU_S390_FLIC(fs); > + > + switch (mode) { > + case SIC_IRQ_MODE_ALL: > + flic->simm &= ~AIS_MODE_MASK(isc); > + flic->nimm &= ~AIS_MODE_MASK(isc); > + break; > + case SIC_IRQ_MODE_SINGLE: > + flic->simm |= AIS_MODE_MASK(isc); > + flic->nimm &= ~AIS_MODE_MASK(isc); Can we have some documentation about the meaning of simm/nimm in qemu? The one from the patch description of the respective kernel patch would be fine. > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > (...) > @@ -406,6 +427,7 @@ typedef struct KVMS390FLICStateClass { > > static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) > { > + S390FLICState *fs = S390_FLIC_COMMON(dev); > KVMS390FLICState *flic_state = KVM_S390_FLIC(dev); > struct kvm_create_device cd = {0}; > struct kvm_device_attr test_attr = {0}; > @@ -438,6 +460,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) > flic_state->clear_io_supported = !ioctl(flic_state->fd, > KVM_HAS_DEVICE_ATTR, test_attr); > > + fs->ais_supported = false; The patch actually enabling this for kvm is in the next batch of patches, I assume? > return; > fail: > error_propagate(errp, errp_local);
On 07/12/2017 05:07 PM, Cornelia Huck wrote: >> + >> + switch (mode) { >> + case SIC_IRQ_MODE_ALL: >> + flic->simm &= ~AIS_MODE_MASK(isc); >> + flic->nimm &= ~AIS_MODE_MASK(isc); >> + break; >> + case SIC_IRQ_MODE_SINGLE: >> + flic->simm |= AIS_MODE_MASK(isc); >> + flic->nimm &= ~AIS_MODE_MASK(isc); > > Can we have some documentation about the meaning of simm/nimm in qemu? > The one from the patch description of the respective kernel patch would > be fine. Do you want to have that description as part of the patch description or somewhere in the docs folder or somewhere in the code? > >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> > > (...) > >> @@ -406,6 +427,7 @@ typedef struct KVMS390FLICStateClass { >> >> static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) >> { >> + S390FLICState *fs = S390_FLIC_COMMON(dev); >> KVMS390FLICState *flic_state = KVM_S390_FLIC(dev); >> struct kvm_create_device cd = {0}; >> struct kvm_device_attr test_attr = {0}; >> @@ -438,6 +460,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) >> flic_state->clear_io_supported = !ioctl(flic_state->fd, >> KVM_HAS_DEVICE_ATTR, test_attr); >> >> + fs->ais_supported = false; > > The patch actually enabling this for kvm is in the next batch of > patches, I assume? yes, will hopefully come today.
On Thu, 13 Jul 2017 09:12:50 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 07/12/2017 05:07 PM, Cornelia Huck wrote: > >> + > >> + switch (mode) { > >> + case SIC_IRQ_MODE_ALL: > >> + flic->simm &= ~AIS_MODE_MASK(isc); > >> + flic->nimm &= ~AIS_MODE_MASK(isc); > >> + break; > >> + case SIC_IRQ_MODE_SINGLE: > >> + flic->simm |= AIS_MODE_MASK(isc); > >> + flic->nimm &= ~AIS_MODE_MASK(isc); > > > > Can we have some documentation about the meaning of simm/nimm in qemu? > > The one from the patch description of the respective kernel patch would > > be fine. > > Do you want to have that description as part of the patch description or > somewhere in the docs folder or somewhere in the code? A comment near the definition of simm/nimm would be nice. Or in the commit message, as for the kernel.
On 07/13/2017 09:27 AM, Cornelia Huck wrote: > On Thu, 13 Jul 2017 09:12:50 +0200 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> On 07/12/2017 05:07 PM, Cornelia Huck wrote: >>>> + >>>> + switch (mode) { >>>> + case SIC_IRQ_MODE_ALL: >>>> + flic->simm &= ~AIS_MODE_MASK(isc); >>>> + flic->nimm &= ~AIS_MODE_MASK(isc); >>>> + break; >>>> + case SIC_IRQ_MODE_SINGLE: >>>> + flic->simm |= AIS_MODE_MASK(isc); >>>> + flic->nimm &= ~AIS_MODE_MASK(isc); >>> >>> Can we have some documentation about the meaning of simm/nimm in qemu? >>> The one from the patch description of the respective kernel patch would >>> be fine. >> >> Do you want to have that description as part of the patch description or >> somewhere in the docs folder or somewhere in the code? > > A comment near the definition of simm/nimm would be nice. Or in the > commit message, as for the kernel. > I think I will just add this to the commit message: We introduce the 'simm' and 'nimm' fields to QEMUS390FLICState to store interruption modes for each ISC. Each bit in 'simm' and 'nimm' targets to one ISC, and collaboratively indicate three modes: ALL-Interruptions, SINGLE-Interruption and NO-Interruptions. This interface can initiate most transitions between the states; transition from SINGLE-Interruption to NO-Interruptions via adapter interrupt injection will be introduced in a following patch. The meaningful combinations are as follows: interruption mode | simm bit | nimm bit ------------------|----------|---------- ALL | 0 | 0 SINGLE | 1 | 0 NO | 1 | 1
On Thu, 13 Jul 2017 09:35:16 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 07/13/2017 09:27 AM, Cornelia Huck wrote: > > On Thu, 13 Jul 2017 09:12:50 +0200 > > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > >> On 07/12/2017 05:07 PM, Cornelia Huck wrote: > >>>> + > >>>> + switch (mode) { > >>>> + case SIC_IRQ_MODE_ALL: > >>>> + flic->simm &= ~AIS_MODE_MASK(isc); > >>>> + flic->nimm &= ~AIS_MODE_MASK(isc); > >>>> + break; > >>>> + case SIC_IRQ_MODE_SINGLE: > >>>> + flic->simm |= AIS_MODE_MASK(isc); > >>>> + flic->nimm &= ~AIS_MODE_MASK(isc); > >>> > >>> Can we have some documentation about the meaning of simm/nimm in qemu? > >>> The one from the patch description of the respective kernel patch would > >>> be fine. > >> > >> Do you want to have that description as part of the patch description or > >> somewhere in the docs folder or somewhere in the code? > > > > A comment near the definition of simm/nimm would be nice. Or in the > > commit message, as for the kernel. > > > > I think I will just add this to the commit message: > > We introduce the 'simm' and 'nimm' fields to QEMUS390FLICState > to store interruption modes for each ISC. Each bit in 'simm' and > 'nimm' targets to one ISC, and collaboratively indicate three modes: s/targets to one/targets one/ > ALL-Interruptions, SINGLE-Interruption and NO-Interruptions. This > interface can initiate most transitions between the states; transition > from SINGLE-Interruption to NO-Interruptions via adapter interrupt > injection will be introduced in a following patch. The meaningful > combinations are as follows: > > interruption mode | simm bit | nimm bit > ------------------|----------|---------- > ALL | 0 | 0 > SINGLE | 1 | 0 > NO | 1 | 1 > > > Sounds good.
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c index d3938b3..ce7f355 100644 --- a/hw/intc/s390_flic.c +++ b/hw/intc/s390_flic.c @@ -79,15 +79,47 @@ static int qemu_s390_clear_io_flic(S390FLICState *fs, uint16_t subchannel_id, return -ENOSYS; } +static int qemu_s390_modify_ais_mode(S390FLICState *fs, uint8_t isc, + uint16_t mode) +{ + QEMUS390FLICState *flic = QEMU_S390_FLIC(fs); + + switch (mode) { + case SIC_IRQ_MODE_ALL: + flic->simm &= ~AIS_MODE_MASK(isc); + flic->nimm &= ~AIS_MODE_MASK(isc); + break; + case SIC_IRQ_MODE_SINGLE: + flic->simm |= AIS_MODE_MASK(isc); + flic->nimm &= ~AIS_MODE_MASK(isc); + break; + default: + return -EINVAL; + } + + return 0; +} + +static void qemu_s390_flic_reset(DeviceState *dev) +{ + QEMUS390FLICState *flic = QEMU_S390_FLIC(dev); + + flic->simm = 0; + flic->nimm = 0; +} + static void qemu_s390_flic_class_init(ObjectClass *oc, void *data) { + DeviceClass *dc = DEVICE_CLASS(oc); S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc); + dc->reset = qemu_s390_flic_reset; fsc->register_io_adapter = qemu_s390_register_io_adapter; fsc->io_adapter_map = qemu_s390_io_adapter_map; fsc->add_adapter_routes = qemu_s390_add_adapter_routes; fsc->release_adapter_routes = qemu_s390_release_adapter_routes; fsc->clear_io_irq = qemu_s390_clear_io_flic; + fsc->modify_ais_mode = qemu_s390_modify_ais_mode; } static Property s390_flic_common_properties[] = { @@ -98,12 +130,15 @@ static Property s390_flic_common_properties[] = { static void s390_flic_common_realize(DeviceState *dev, Error **errp) { - uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch; + S390FLICState *fs = S390_FLIC_COMMON(dev); + uint32_t max_batch = fs->adapter_routes_max_batch; if (max_batch > ADAPTER_ROUTES_MAX_GSI) { error_setg(errp, "flic property adapter_routes_max_batch too big" " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI); } + + fs->ais_supported = true; } static void s390_flic_class_init(ObjectClass *oc, void *data) diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c index 15ff534..55aa35f 100644 --- a/hw/intc/s390_flic_kvm.c +++ b/hw/intc/s390_flic_kvm.c @@ -20,6 +20,7 @@ #include "sysemu/kvm.h" #include "hw/s390x/s390_flic.h" #include "hw/s390x/adapter.h" +#include "hw/s390x/css.h" #include "trace.h" #define FLIC_SAVE_INITIAL_SIZE getpagesize() @@ -149,6 +150,26 @@ static int kvm_s390_clear_io_flic(S390FLICState *fs, uint16_t subchannel_id, return rc ? -errno : 0; } +static int kvm_s390_modify_ais_mode(S390FLICState *fs, uint8_t isc, + uint16_t mode) +{ + KVMS390FLICState *flic = KVM_S390_FLIC(fs); + struct kvm_s390_ais_req req = { + .isc = isc, + .mode = mode, + }; + struct kvm_device_attr attr = { + .group = KVM_DEV_FLIC_AISM, + .addr = (uint64_t)&req, + }; + + if (!fs->ais_supported) { + return -ENOSYS; + } + + return ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr) ? -errno : 0; +} + /** * __get_all_irqs - store all pending irqs in buffer * @flic: pointer to flic device state @@ -406,6 +427,7 @@ typedef struct KVMS390FLICStateClass { static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) { + S390FLICState *fs = S390_FLIC_COMMON(dev); KVMS390FLICState *flic_state = KVM_S390_FLIC(dev); struct kvm_create_device cd = {0}; struct kvm_device_attr test_attr = {0}; @@ -438,6 +460,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) flic_state->clear_io_supported = !ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr); + fs->ais_supported = false; return; fail: error_propagate(errp, errp_local); @@ -446,10 +469,12 @@ fail: static void kvm_s390_flic_reset(DeviceState *dev) { KVMS390FLICState *flic = KVM_S390_FLIC(dev); + S390FLICState *fs = S390_FLIC_COMMON(dev); struct kvm_device_attr attr = { .group = KVM_DEV_FLIC_CLEAR_IRQS, }; int rc = 0; + uint8_t isc; if (flic->fd == -1) { return; @@ -457,6 +482,16 @@ static void kvm_s390_flic_reset(DeviceState *dev) flic_disable_wait_pfault(flic); + if (fs->ais_supported) { + for (isc = 0; isc <= MAX_ISC; isc++) { + rc = kvm_s390_modify_ais_mode(fs, isc, SIC_IRQ_MODE_ALL); + if (rc) { + error_report("Failed to reset ais mode for isc %d: %s", + isc, strerror(-rc)); + } + } + } + rc = ioctl(flic->fd, KVM_SET_DEVICE_ATTR, &attr); if (rc) { trace_flic_reset_failed(errno); @@ -479,6 +514,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void *data) fsc->add_adapter_routes = kvm_s390_add_adapter_routes; fsc->release_adapter_routes = kvm_s390_release_adapter_routes; fsc->clear_io_irq = kvm_s390_clear_io_flic; + fsc->modify_ais_mode = kvm_s390_modify_ais_mode; } static const TypeInfo kvm_s390_flic_info = { diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h index d4145ad..f500184 100644 --- a/include/hw/s390x/s390_flic.h +++ b/include/hw/s390x/s390_flic.h @@ -44,7 +44,7 @@ typedef struct S390FLICState { SysBusDevice parent_obj; /* to limit AdapterRoutes.num_routes for compat */ uint32_t adapter_routes_max_batch; - + bool ais_supported; } S390FLICState; #define S390_FLIC_COMMON_CLASS(klass) \ @@ -63,6 +63,7 @@ typedef struct S390FLICStateClass { void (*release_adapter_routes)(S390FLICState *fs, AdapterRoutes *routes); int (*clear_io_irq)(S390FLICState *fs, uint16_t subchannel_id, uint16_t subchannel_nr); + int (*modify_ais_mode)(S390FLICState *fs, uint8_t isc, uint16_t mode); } S390FLICStateClass; #define TYPE_KVM_S390_FLIC "s390-flic-kvm" @@ -73,8 +74,14 @@ typedef struct S390FLICStateClass { #define QEMU_S390_FLIC(obj) \ OBJECT_CHECK(QEMUS390FLICState, (obj), TYPE_QEMU_S390_FLIC) +#define SIC_IRQ_MODE_ALL 0 +#define SIC_IRQ_MODE_SINGLE 1 +#define AIS_MODE_MASK(isc) (0x80 >> isc) + typedef struct QEMUS390FLICState { S390FLICState parent_obj; + uint8_t simm; + uint8_t nimm; } QEMUS390FLICState; void s390_flic_init(void);