| Message ID | 20170922083855.102341-3-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | ais fixups for 2.11 | expand |
On 22/09/2017 10:38, Christian Borntraeger wrote: > Instead of unconditionally enabling the KVM AIS capability > in the kvm arch init function, do this in the flic realize function > when we know if migration is available. This requires to initialize > flic before the CPUs. I am not sure to agree. AIS facility is used for PCI (currently only PCI) We want to support PCI emulation and PCI VFIO Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation. Neither virtio-pci nor TCG. The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices. Wouldn't it be possible to just refuse to realize these devices when we have no kernel support for AISM? And accept to migrate without kernel support if they are not used? Pierre > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/intc/s390_flic.c | 11 +++++++++-- > hw/intc/s390_flic_kvm.c | 8 +++++++- > hw/s390x/s390-virtio-ccw.c | 8 ++++++-- > include/hw/s390x/s390_flic.h | 1 + > target/s390x/cpu_models.c | 6 ++++++ > target/s390x/kvm.c | 8 +------- > 6 files changed, 30 insertions(+), 12 deletions(-) > > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c > index 6eaf178..08040fe 100644 > --- a/hw/intc/s390_flic.c > +++ b/hw/intc/s390_flic.c > @@ -1,7 +1,7 @@ > /* > * QEMU S390x floating interrupt controller (flic) > * > - * Copyright 2014 IBM Corp. > + * Copyright 2014,2017 IBM Corp. > * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> > * Cornelia Huck <cornelia.huck@de.ibm.com> > * > @@ -49,6 +49,13 @@ void s390_flic_init(void) > qdev_init_nofail(dev); > } > > +void s390_flic_enable_ais(void) > +{ > + S390FLICState *fs = s390_get_flic(); > + > + fs->ais_supported = true; > +} > + > static int qemu_s390_register_io_adapter(S390FLICState *fs, uint32_t id, > uint8_t isc, bool swap, > bool is_maskable, uint8_t flags) > @@ -186,7 +193,7 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp) > return; > } > > - fs->ais_supported = s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION); > + fs->ais_supported = false; > } > > 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 7ead17a..a655567 100644 > --- a/hw/intc/s390_flic_kvm.c > +++ b/hw/intc/s390_flic_kvm.c > @@ -1,7 +1,7 @@ > /* > * QEMU S390x KVM floating interrupt controller (flic) > * > - * Copyright 2014 IBM Corp. > + * Copyright 2014,2017 IBM Corp. > * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> > * Cornelia Huck <cornelia.huck@de.ibm.com> > * > @@ -557,6 +557,12 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) > test_attr.group = KVM_DEV_FLIC_CLEAR_IO_IRQ; > flic_state->clear_io_supported = !ioctl(flic_state->fd, > KVM_HAS_DEVICE_ATTR, test_attr); > + /* try enable the AIS facility */ > + test_attr.group = KVM_DEV_FLIC_AISM_ALL; > + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { > + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); > + } > + > return; > fail: > error_propagate(errp, errp_local); > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index fafbc6d..11d4dc4 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -279,11 +279,15 @@ static void ccw_init(MachineState *machine) > s390_sclp_init(); > s390_memory_init(machine->ram_size); > > + /* > + * This might also enable some KVM features like AIS, so it must > + * be called before the CPU model > + */ > + s390_flic_init(); > + > /* init CPUs (incl. CPU model) early so s390_has_feature() works */ > s390_init_cpus(machine); > > - s390_flic_init(); > - > /* get a BUS */ > css_bus = virtual_css_bus_init(); > s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline, > diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h > index 7aab6ef..ac4e170 100644 > --- a/include/hw/s390x/s390_flic.h > +++ b/include/hw/s390x/s390_flic.h > @@ -90,6 +90,7 @@ void s390_flic_init(void); > > S390FLICState *s390_get_flic(void); > bool ais_needed(void *opaque); > +void s390_flic_enable_ais(void); > > #ifdef CONFIG_KVM > DeviceState *s390_flic_kvm_create(void); > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > index 5169379..03ff583 100644 > --- a/target/s390x/cpu_models.c > +++ b/target/s390x/cpu_models.c > @@ -23,6 +23,7 @@ > #include "qapi/qobject-input-visitor.h" > #include "qapi/qmp/qbool.h" > #ifndef CONFIG_USER_ONLY > +#include "hw/s390x/s390_flic.h" > #include "sysemu/arch_init.h" > #endif > > @@ -901,6 +902,11 @@ static inline void apply_cpu_model(const S390CPUModel *model, Error **errp) > applied_model = *model; > } > } > + > + if (model && > + test_bit(S390_FEAT_ADAPTER_INT_SUPPRESSION, model->features)) { > + s390_flic_enable_ais(); > + } > #endif > } > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index ebb75ca..6c5c57e 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -311,13 +311,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > } > } > > - /* > - * The migration interface for ais was introduced with kernel 4.13 > - * but the capability itself had been active since 4.12. As migration > - * support is considered necessary let's disable ais in the 2.10 > - * machine. > - */ > - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ > + /* The AIS enablement happens in the flic realize */ > > qemu_mutex_init(&qemu_sigp_mutex); >
On 09/22/2017 02:13 PM, Pierre Morel wrote: > On 22/09/2017 10:38, Christian Borntraeger wrote: >> Instead of unconditionally enabling the KVM AIS capability >> in the kvm arch init function, do this in the flic realize function >> when we know if migration is available. This requires to initialize >> flic before the CPUs. > > I am not sure to agree. > > AIS facility is used for PCI (currently only PCI) > We want to support PCI emulation and PCI VFIO > > > Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation. > Neither virtio-pci nor TCG. > The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices. This patch enable the conditional enablement facility for the KVM host mask. The cpu model enablement is done differently for KVM and TCG anyway. Right now AIS is only enabled for KVM. For TCG AIS is not implemented at all and disabled. So for whenever this is fixed in TCG it can be handled then. And for emulated devices under KVM you still need the kernel support - otherwise migration is really broken for the nimm/dimm values.
On Fri, 22 Sep 2017 14:40:31 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 09/22/2017 02:13 PM, Pierre Morel wrote: > > On 22/09/2017 10:38, Christian Borntraeger wrote: > >> Instead of unconditionally enabling the KVM AIS capability > >> in the kvm arch init function, do this in the flic realize function > >> when we know if migration is available. This requires to initialize > >> flic before the CPUs. > > > > I am not sure to agree. > > > > AIS facility is used for PCI (currently only PCI) > > We want to support PCI emulation and PCI VFIO > > > > > > Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation. > > Neither virtio-pci nor TCG. > > The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices. > > This patch enable the conditional enablement facility for the KVM host mask. The cpu model enablement is done > differently for KVM and TCG anyway. > Right now AIS is only enabled for KVM. For TCG AIS is not implemented at all and disabled. So for whenever this > is fixed in TCG it can be handled then. Yes, that will probably still need quite a bit of work before we are there. > > And for emulated devices under KVM you still need the kernel support - otherwise migration is really broken for > the nimm/dimm values. Agreed, you need this for any pci device. I have not yet reviewed this (or tested under tcg). I'd like to hear David's opinion, though.
On 22/09/2017 14:40, Christian Borntraeger wrote: > > > On 09/22/2017 02:13 PM, Pierre Morel wrote: >> On 22/09/2017 10:38, Christian Borntraeger wrote: >>> Instead of unconditionally enabling the KVM AIS capability >>> in the kvm arch init function, do this in the flic realize function >>> when we know if migration is available. This requires to initialize >>> flic before the CPUs. >> >> I am not sure to agree. >> >> AIS facility is used for PCI (currently only PCI) >> We want to support PCI emulation and PCI VFIO >> >> >> Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation. >> Neither virtio-pci nor TCG. >> The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices. > > This patch enable the conditional enablement facility for the KVM host mask. The cpu model enablement is done > differently for KVM and TCG anyway. > Right now AIS is only enabled for KVM. For TCG AIS is not implemented at all and disabled. So for whenever this > is fixed in TCG it can be handled then. > > And for emulated devices under KVM you still need the kernel support - otherwise migration is really broken for > the nimm/dimm values. > Yes, that is right. I do not pretend that it is working directly. To support AIS emulation we have some work to do there (the migration), in kvm_s390_inject_airq() and other functions using ais_supported, to use emulated values. Don't you think that we could use emulation instead of faulting when the kernel can not handle values needed for pass-through? We can fault at the moment a VFIO device is realized. Migration will be stopped on realizing VFIO PCI devices instead of flic. I can miss something, is there a reason not to do so?
On 09/22/2017 04:02 PM, Pierre Morel wrote: > On 22/09/2017 14:40, Christian Borntraeger wrote: >> >> >> On 09/22/2017 02:13 PM, Pierre Morel wrote: >>> On 22/09/2017 10:38, Christian Borntraeger wrote: >>>> Instead of unconditionally enabling the KVM AIS capability >>>> in the kvm arch init function, do this in the flic realize function >>>> when we know if migration is available. This requires to initialize >>>> flic before the CPUs. >>> >>> I am not sure to agree. >>> >>> AIS facility is used for PCI (currently only PCI) >>> We want to support PCI emulation and PCI VFIO >>> >>> >>> Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation. >>> Neither virtio-pci nor TCG. >>> The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices. >> >> This patch enable the conditional enablement facility for the KVM host mask. The cpu model enablement is done >> differently for KVM and TCG anyway. >> Right now AIS is only enabled for KVM. For TCG AIS is not implemented at all and disabled. So for whenever this >> is fixed in TCG it can be handled then. >> >> And for emulated devices under KVM you still need the kernel support - otherwise migration is really broken for >> the nimm/dimm values. >> > > Yes, that is right. > I do not pretend that it is working directly. > > To support AIS emulation we have some work to do there (the migration), in kvm_s390_inject_airq() and other functions using ais_supported, to use emulated values. > > Don't you think that we could use emulation instead of faulting when the kernel can not handle values needed for pass-through? > > We can fault at the moment a VFIO device is realized. > Migration will be stopped on realizing VFIO PCI devices instead of flic. > > I can miss something, is there a reason not to do so? The problem is not vfio vs emulation. The problem is that for KVM the kernel is handling the interrupts. and it holds the state of the interrupt controller (nimm/dimm) and without an interface to read/write these, the PCI core will be in a wrong state after migration regarding AIS. Really, this patch just moves the supported host kernel from >=4.12 to >=4.13. It makes absolutely no sense to start having a mixed interrupt stack (qemu + kernel) just to make this work on 4.12.
On 09/22/2017 04:07 PM, Christian Borntraeger wrote: > > > On 09/22/2017 04:02 PM, Pierre Morel wrote: >> On 22/09/2017 14:40, Christian Borntraeger wrote: >>> >>> >>> On 09/22/2017 02:13 PM, Pierre Morel wrote: >>>> On 22/09/2017 10:38, Christian Borntraeger wrote: >>>>> Instead of unconditionally enabling the KVM AIS capability >>>>> in the kvm arch init function, do this in the flic realize function >>>>> when we know if migration is available. This requires to initialize >>>>> flic before the CPUs. >>>> >>>> I am not sure to agree. >>>> >>>> AIS facility is used for PCI (currently only PCI) >>>> We want to support PCI emulation and PCI VFIO >>>> >>>> >>>> Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation. >>>> Neither virtio-pci nor TCG. >>>> The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices. >>> >>> This patch enable the conditional enablement facility for the KVM host mask. The cpu model enablement is done >>> differently for KVM and TCG anyway. >>> Right now AIS is only enabled for KVM. For TCG AIS is not implemented at all and disabled. So for whenever this >>> is fixed in TCG it can be handled then. >>> >>> And for emulated devices under KVM you still need the kernel support - otherwise migration is really broken for >>> the nimm/dimm values. >>> >> >> Yes, that is right. >> I do not pretend that it is working directly. >> >> To support AIS emulation we have some work to do there (the migration), in kvm_s390_inject_airq() and other functions using ais_supported, to use emulated values. >> >> Don't you think that we could use emulation instead of faulting when the kernel can not handle values needed for pass-through? >> >> We can fault at the moment a VFIO device is realized. >> Migration will be stopped on realizing VFIO PCI devices instead of flic. >> >> I can miss something, is there a reason not to do so? > > The problem is not vfio vs emulation. The problem is that for KVM the kernel is handling the interrupts. > and it holds the state of the interrupt controller (nimm/dimm) and without an interface to read/write these, > the PCI core will be in a wrong state after migration regarding AIS. > > Really, this patch just moves the supported host kernel from >=4.12 to >=4.13. It makes absolutely no > sense to start having a mixed interrupt stack (qemu + kernel) just to make this work on 4.12. > I tend to agree with Christian. I don't really understand Pierre's proposal. I'm sure a patch would help clarify things. If it's too complicated for writing up a patch, I think it's too complicated for discussing too. About the wrong state. Back then I did some reasoning about how bad this bad state actually is. In case of AIS starting with the initial state after the migration might not be fatal (all cases considered). I was asking myself is there any problem beyond getting more irqs delivered than necessary, and does that only affect the performance of the guest, or could it lead to correctness issues? I found the later question difficult to answer. The whole AIS stuff was a while ago. From what I see what Christian is trying to do is both sane and conservative. Would need to revisit everything to say more. One thing I would find very helpful is what do we expect to work and not work for which version. Kind of a matrix. For instance should vfio pci work for versions prior 2.11. I think in the not so distant past we changed how SIC works (so it complains when we don't have ais). Btw nimm/dimm is simm/nimm. And I would really like to hear something form the original author(s) too. Regards, Halil
On 22/09/2017 16:07, Christian Borntraeger wrote: > > > On 09/22/2017 04:02 PM, Pierre Morel wrote: >> On 22/09/2017 14:40, Christian Borntraeger wrote: >>> >>> >>> On 09/22/2017 02:13 PM, Pierre Morel wrote: >>>> On 22/09/2017 10:38, Christian Borntraeger wrote: >>>>> Instead of unconditionally enabling the KVM AIS capability >>>>> in the kvm arch init function, do this in the flic realize function >>>>> when we know if migration is available. This requires to initialize >>>>> flic before the CPUs. >>>> >>>> I am not sure to agree. >>>> >>>> AIS facility is used for PCI (currently only PCI) >>>> We want to support PCI emulation and PCI VFIO >>>> >>>> >>>> Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation. >>>> Neither virtio-pci nor TCG. >>>> The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices. >>> >>> This patch enable the conditional enablement facility for the KVM host mask. The cpu model enablement is done >>> differently for KVM and TCG anyway. >>> Right now AIS is only enabled for KVM. For TCG AIS is not implemented at all and disabled. So for whenever this >>> is fixed in TCG it can be handled then. >>> >>> And for emulated devices under KVM you still need the kernel support - otherwise migration is really broken for >>> the nimm/dimm values. >>> >> >> Yes, that is right. >> I do not pretend that it is working directly. >> >> To support AIS emulation we have some work to do there (the migration), in kvm_s390_inject_airq() and other functions using ais_supported, to use emulated values. >> >> Don't you think that we could use emulation instead of faulting when the kernel can not handle values needed for pass-through? >> >> We can fault at the moment a VFIO device is realized. >> Migration will be stopped on realizing VFIO PCI devices instead of flic. >> >> I can miss something, is there a reason not to do so? > > The problem is not vfio vs emulation. The problem is that for KVM the kernel is handling the interrupts. > and it holds the state of the interrupt controller (nimm/dimm) and without an interface to read/write these, > the PCI core will be in a wrong state after migration regarding AIS. I am aware of this. > > Really, this patch just moves the supported host kernel from >=4.12 to >=4.13. It makes absolutely no > sense to start having a mixed interrupt stack (qemu + kernel) just to make this work on 4.12. > > In fact it would have been for much more than just the 4.12 But OK, I can understand that it makes no sense to discuss this now. The urgency is to fence 4.12. Eventually, we can discuss this later.
On Fri, 22 Sep 2017 16:27:00 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > One thing I would find very helpful is what do we expect to work and not > work for which version. Kind of a matrix. For instance should vfio pci > work for versions prior 2.11. I think in the not so distant past we > changed how SIC works (so it complains when we don't have ais). A matrix sounds like a good idea. I don't think we really ever had a setup that worked out of the box for a Linux guest prior to 2.10 anyway. For added fun, it also depends on the host kernel. And tcg won't work at all due to the pci instruction not yet being wired up. > Btw nimm/dimm is simm/nimm. And I would really like to hear something > form the original author(s) too. Seconded.
On 09/25/2017 12:07 PM, Cornelia Huck wrote: > On Fri, 22 Sep 2017 16:27:00 +0200 > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > >> One thing I would find very helpful is what do we expect to work and not >> work for which version. Kind of a matrix. For instance should vfio pci >> work for versions prior 2.11. I think in the not so distant past we >> changed how SIC works (so it complains when we don't have ais). > > A matrix sounds like a good idea. I think we do not even need a matrix, a minimum level will suffice because... > > I don't think we really ever had a setup that worked out of the box exactly: ...it never worked until 2.10 and we do not have libvirt support yet. Now with the fix 2.10 will also not work, so I think its fair to say PCI passthrough via VFIO will be supported for - KVM: host kernel >= 4.13 - TCG: TBD - QEMU >= 2.11 - libvirt TBD > for a Linux guest prior to 2.10 anyway. For added fun, it also depends > on the host kernel. And tcg won't work at all due to the pci > instruction not yet being wired up. > >> Btw nimm/dimm is simm/nimm. And I would really like to hear something yes, I mixed that up. Too many letters :-) >> form the original author(s) too.
On Mon, 25 Sep 2017 12:12:49 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 09/25/2017 12:07 PM, Cornelia Huck wrote: > > On Fri, 22 Sep 2017 16:27:00 +0200 > > Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > > > >> One thing I would find very helpful is what do we expect to work and not > >> work for which version. Kind of a matrix. For instance should vfio pci > >> work for versions prior 2.11. I think in the not so distant past we > >> changed how SIC works (so it complains when we don't have ais). > > > > A matrix sounds like a good idea. > > I think we do not even need a matrix, a minimum level will suffice because... > > > > I don't think we really ever had a setup that worked out of the box > > exactly: ...it never worked until 2.10 and we do not have libvirt support yet. > Now with the fix 2.10 will also not work, so I think its fair to say > > PCI passthrough via VFIO will be supported for > - KVM: host kernel >= 4.13 > - TCG: TBD > - QEMU >= 2.11 > - libvirt TBD Make that zpci-per-se, no? with KVM: host kernel >= 4.13 && QEMU >= 2.11 with TCG: tbd, I don't think anybody has time to wire this up for 2.11 Apropos libvirt: How will it determine whether zpci should be supported? There are some old QEMU + KVM combinations out there that will have a phb (but not be usable by stock Linux guests as the feature bits are missing). Version fence? Check for cpu feature support?
On 09/25/2017 01:45 PM, Cornelia Huck wrote: > On Mon, 25 Sep 2017 12:12:49 +0200 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> On 09/25/2017 12:07 PM, Cornelia Huck wrote: >>> On Fri, 22 Sep 2017 16:27:00 +0200 >>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >>> >>>> One thing I would find very helpful is what do we expect to work and not >>>> work for which version. Kind of a matrix. For instance should vfio pci >>>> work for versions prior 2.11. I think in the not so distant past we >>>> changed how SIC works (so it complains when we don't have ais). >>> >>> A matrix sounds like a good idea. >> >> I think we do not even need a matrix, a minimum level will suffice because... >>> >>> I don't think we really ever had a setup that worked out of the box >> >> exactly: ...it never worked until 2.10 and we do not have libvirt support yet. >> Now with the fix 2.10 will also not work, so I think its fair to say >> >> PCI passthrough via VFIO will be supported for >> - KVM: host kernel >= 4.13 >> - TCG: TBD >> - QEMU >= 2.11 >> - libvirt TBD > > Make that zpci-per-se, no? > > with KVM: host kernel >= 4.13 && QEMU >= 2.11 > with TCG: tbd, I don't think anybody has time to wire this up for 2.11 > > Apropos libvirt: How will it determine whether zpci should be > supported? There are some old QEMU + KVM combinations out there that > will have a phb (but not be usable by stock Linux guests as the feature > bits are missing). Version fence? Check for cpu feature support? I think for multibus or something like that Boris wanted to check for a version anyway. So maybe 2.11 (now that 2.10 is broken regarding ais) as a minimum QEMU level would make sense.
在 2017/9/25 下午7:47, Christian Borntraeger 写道: > On 09/25/2017 01:45 PM, Cornelia Huck wrote: >> On Mon, 25 Sep 2017 12:12:49 +0200 >> Christian Borntraeger <borntraeger@de.ibm.com> wrote: >> >>> On 09/25/2017 12:07 PM, Cornelia Huck wrote: >>>> On Fri, 22 Sep 2017 16:27:00 +0200 >>>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >>>> >>>>> One thing I would find very helpful is what do we expect to work and not >>>>> work for which version. Kind of a matrix. For instance should vfio pci >>>>> work for versions prior 2.11. I think in the not so distant past we >>>>> changed how SIC works (so it complains when we don't have ais). >>>> A matrix sounds like a good idea. >>> I think we do not even need a matrix, a minimum level will suffice because... >>>> I don't think we really ever had a setup that worked out of the box >>> exactly: ...it never worked until 2.10 and we do not have libvirt support yet. >>> Now with the fix 2.10 will also not work, so I think its fair to say >>> >>> PCI passthrough via VFIO will be supported for >>> - KVM: host kernel >= 4.13 >>> - TCG: TBD >>> - QEMU >= 2.11 >>> - libvirt TBD >> Make that zpci-per-se, no? >> >> with KVM: host kernel >= 4.13 && QEMU >= 2.11 >> with TCG: tbd, I don't think anybody has time to wire this up for 2.11 >> >> Apropos libvirt: How will it determine whether zpci should be >> supported? There are some old QEMU + KVM combinations out there that >> will have a phb (but not be usable by stock Linux guests as the feature >> bits are missing). Version fence? Check for cpu feature support? > I think for multibus or something like that Boris wanted to check for a version > anyway. So maybe 2.11 (now that 2.10 is broken regarding ais) as a minimum QEMU > level would make sense. > > I think this makes sense. But I think I have to discuss this with Boris.
On 22.09.2017 10:38, Christian Borntraeger wrote: > Instead of unconditionally enabling the KVM AIS capability > in the kvm arch init function, do this in the flic realize function > when we know if migration is available. This requires to initialize > flic before the CPUs. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/intc/s390_flic.c | 11 +++++++++-- > hw/intc/s390_flic_kvm.c | 8 +++++++- > hw/s390x/s390-virtio-ccw.c | 8 ++++++-- > include/hw/s390x/s390_flic.h | 1 + > target/s390x/cpu_models.c | 6 ++++++ > target/s390x/kvm.c | 8 +------- > 6 files changed, 30 insertions(+), 12 deletions(-) > > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c > index 6eaf178..08040fe 100644 > --- a/hw/intc/s390_flic.c > +++ b/hw/intc/s390_flic.c > @@ -1,7 +1,7 @@ > /* > * QEMU S390x floating interrupt controller (flic) > * > - * Copyright 2014 IBM Corp. > + * Copyright 2014,2017 IBM Corp. > * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> > * Cornelia Huck <cornelia.huck@de.ibm.com> > * > @@ -49,6 +49,13 @@ void s390_flic_init(void) > qdev_init_nofail(dev); > } > > +void s390_flic_enable_ais(void) > +{ > + S390FLICState *fs = s390_get_flic(); > + > + fs->ais_supported = true; Can we simply replace all ais_supported checks by s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION) and drop ais_supported? Then, s390_flic_enable_ais() should not be needed. As far as I understand, we will unlock kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); only if we have migration support. Therefore, CPU model init should see S390_FEAT_ADAPTER_INT_SUPPRESSION only if migration support is available.
On 09/26/2017 02:23 PM, David Hildenbrand wrote: > On 22.09.2017 10:38, Christian Borntraeger wrote: >> Instead of unconditionally enabling the KVM AIS capability >> in the kvm arch init function, do this in the flic realize function >> when we know if migration is available. This requires to initialize >> flic before the CPUs. >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> hw/intc/s390_flic.c | 11 +++++++++-- >> hw/intc/s390_flic_kvm.c | 8 +++++++- >> hw/s390x/s390-virtio-ccw.c | 8 ++++++-- >> include/hw/s390x/s390_flic.h | 1 + >> target/s390x/cpu_models.c | 6 ++++++ >> target/s390x/kvm.c | 8 +------- >> 6 files changed, 30 insertions(+), 12 deletions(-) >> >> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c >> index 6eaf178..08040fe 100644 >> --- a/hw/intc/s390_flic.c >> +++ b/hw/intc/s390_flic.c >> @@ -1,7 +1,7 @@ >> /* >> * QEMU S390x floating interrupt controller (flic) >> * >> - * Copyright 2014 IBM Corp. >> + * Copyright 2014,2017 IBM Corp. >> * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> >> * Cornelia Huck <cornelia.huck@de.ibm.com> >> * >> @@ -49,6 +49,13 @@ void s390_flic_init(void) >> qdev_init_nofail(dev); >> } >> >> +void s390_flic_enable_ais(void) >> +{ >> + S390FLICState *fs = s390_get_flic(); >> + >> + fs->ais_supported = true; > > Can we simply replace all ais_supported checks by > s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION) and drop ais_supported? No, at flic realize, the CPU model is not available yet. And if we move cpumodel before flic then we cannot check the flic attributes so the CPU will not know if ais is available or not.
On 26.09.2017 14:29, Christian Borntraeger wrote: > > > On 09/26/2017 02:23 PM, David Hildenbrand wrote: >> On 22.09.2017 10:38, Christian Borntraeger wrote: >>> Instead of unconditionally enabling the KVM AIS capability >>> in the kvm arch init function, do this in the flic realize function >>> when we know if migration is available. This requires to initialize >>> flic before the CPUs. >>> >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> --- >>> hw/intc/s390_flic.c | 11 +++++++++-- >>> hw/intc/s390_flic_kvm.c | 8 +++++++- >>> hw/s390x/s390-virtio-ccw.c | 8 ++++++-- >>> include/hw/s390x/s390_flic.h | 1 + >>> target/s390x/cpu_models.c | 6 ++++++ >>> target/s390x/kvm.c | 8 +------- >>> 6 files changed, 30 insertions(+), 12 deletions(-) >>> >>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c >>> index 6eaf178..08040fe 100644 >>> --- a/hw/intc/s390_flic.c >>> +++ b/hw/intc/s390_flic.c >>> @@ -1,7 +1,7 @@ >>> /* >>> * QEMU S390x floating interrupt controller (flic) >>> * >>> - * Copyright 2014 IBM Corp. >>> + * Copyright 2014,2017 IBM Corp. >>> * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> >>> * Cornelia Huck <cornelia.huck@de.ibm.com> >>> * >>> @@ -49,6 +49,13 @@ void s390_flic_init(void) >>> qdev_init_nofail(dev); >>> } >>> >>> +void s390_flic_enable_ais(void) >>> +{ >>> + S390FLICState *fs = s390_get_flic(); >>> + >>> + fs->ais_supported = true; >> >> Can we simply replace all ais_supported checks by >> s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION) and drop ais_supported? > > No, at flic realize, the CPU model is not available yet. And if we move cpumodel > before flic then we cannot check the flic attributes so the CPU will not know if > ais is available or not. > We should we need that at realize time? realize() simply inits flic and unlocks the CPU feature if KVM_HAS_DEVICE_ATTR works. What am I missing?
On 09/26/2017 02:29 PM, Christian Borntraeger wrote: > > > On 09/26/2017 02:23 PM, David Hildenbrand wrote: >> On 22.09.2017 10:38, Christian Borntraeger wrote: >>> Instead of unconditionally enabling the KVM AIS capability >>> in the kvm arch init function, do this in the flic realize function >>> when we know if migration is available. This requires to initialize >>> flic before the CPUs. >>> >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> --- >>> hw/intc/s390_flic.c | 11 +++++++++-- >>> hw/intc/s390_flic_kvm.c | 8 +++++++- >>> hw/s390x/s390-virtio-ccw.c | 8 ++++++-- >>> include/hw/s390x/s390_flic.h | 1 + >>> target/s390x/cpu_models.c | 6 ++++++ >>> target/s390x/kvm.c | 8 +------- >>> 6 files changed, 30 insertions(+), 12 deletions(-) >>> >>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c >>> index 6eaf178..08040fe 100644 >>> --- a/hw/intc/s390_flic.c >>> +++ b/hw/intc/s390_flic.c >>> @@ -1,7 +1,7 @@ >>> /* >>> * QEMU S390x floating interrupt controller (flic) >>> * >>> - * Copyright 2014 IBM Corp. >>> + * Copyright 2014,2017 IBM Corp. >>> * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> >>> * Cornelia Huck <cornelia.huck@de.ibm.com> >>> * >>> @@ -49,6 +49,13 @@ void s390_flic_init(void) >>> qdev_init_nofail(dev); >>> } >>> >>> +void s390_flic_enable_ais(void) >>> +{ >>> + S390FLICState *fs = s390_get_flic(); >>> + >>> + fs->ais_supported = true; >> >> Can we simply replace all ais_supported checks by >> s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION) and drop ais_supported? > > No, at flic realize, the CPU model is not available yet. And if we move cpumodel > before flic then we cannot check the flic attributes so the CPU will not know if > ais is available or not. Hmm, let me check again. maybe we do not need that information at realize time. So yes, it could work out.
On 09/26/2017 11:14 AM, Yi Min Zhao wrote: > > > 在 2017/9/25 下午7:47, Christian Borntraeger 写道: >> On 09/25/2017 01:45 PM, Cornelia Huck wrote: >>> On Mon, 25 Sep 2017 12:12:49 +0200 >>> Christian Borntraeger <borntraeger@de.ibm.com> wrote: >>> >>>> On 09/25/2017 12:07 PM, Cornelia Huck wrote: >>>>> On Fri, 22 Sep 2017 16:27:00 +0200 >>>>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote: >>>>>> One thing I would find very helpful is what do we expect to work >>>>>> and not >>>>>> work for which version. Kind of a matrix. For instance should vfio >>>>>> pci >>>>>> work for versions prior 2.11. I think in the not so distant past we >>>>>> changed how SIC works (so it complains when we don't have ais). >>>>> A matrix sounds like a good idea. >>>> I think we do not even need a matrix, a minimum level will suffice >>>> because... >>>>> I don't think we really ever had a setup that worked out of the box >>>> exactly: ...it never worked until 2.10 and we do not have libvirt >>>> support yet. >>>> Now with the fix 2.10 will also not work, so I think its fair to say >>>> >>>> PCI passthrough via VFIO will be supported for >>>> - KVM: host kernel >= 4.13 >>>> - TCG: TBD >>>> - QEMU >= 2.11 >>>> - libvirt TBD >>> Make that zpci-per-se, no? >>> >>> with KVM: host kernel >= 4.13 && QEMU >= 2.11 >>> with TCG: tbd, I don't think anybody has time to wire this up for 2.11 >>> >>> Apropos libvirt: How will it determine whether zpci should be >>> supported? There are some old QEMU + KVM combinations out there that >>> will have a phb (but not be usable by stock Linux guests as the feature >>> bits are missing). Version fence? Check for cpu feature support? >> I think for multibus or something like that Boris wanted to check for >> a version >> anyway. So maybe 2.11 (now that 2.10 is broken regarding ais) as a >> minimum QEMU >> level would make sense. >> >> > I think this makes sense. But I think I have to discuss this with Boris. In libvirt multibus is one of the few (very old) supports that would allow checking against a qemu version. Detection of zpci support is another matter since that capability is tied to the existence of the zpci object in qom. Tying the zpci capability to the multibus support has its pro and cons. I have not made up my mind yet.
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c index 6eaf178..08040fe 100644 --- a/hw/intc/s390_flic.c +++ b/hw/intc/s390_flic.c @@ -1,7 +1,7 @@ /* * QEMU S390x floating interrupt controller (flic) * - * Copyright 2014 IBM Corp. + * Copyright 2014,2017 IBM Corp. * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> * Cornelia Huck <cornelia.huck@de.ibm.com> * @@ -49,6 +49,13 @@ void s390_flic_init(void) qdev_init_nofail(dev); } +void s390_flic_enable_ais(void) +{ + S390FLICState *fs = s390_get_flic(); + + fs->ais_supported = true; +} + static int qemu_s390_register_io_adapter(S390FLICState *fs, uint32_t id, uint8_t isc, bool swap, bool is_maskable, uint8_t flags) @@ -186,7 +193,7 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp) return; } - fs->ais_supported = s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION); + fs->ais_supported = false; } 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 7ead17a..a655567 100644 --- a/hw/intc/s390_flic_kvm.c +++ b/hw/intc/s390_flic_kvm.c @@ -1,7 +1,7 @@ /* * QEMU S390x KVM floating interrupt controller (flic) * - * Copyright 2014 IBM Corp. + * Copyright 2014,2017 IBM Corp. * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> * Cornelia Huck <cornelia.huck@de.ibm.com> * @@ -557,6 +557,12 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) test_attr.group = KVM_DEV_FLIC_CLEAR_IO_IRQ; flic_state->clear_io_supported = !ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr); + /* try enable the AIS facility */ + test_attr.group = KVM_DEV_FLIC_AISM_ALL; + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); + } + return; fail: error_propagate(errp, errp_local); diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index fafbc6d..11d4dc4 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -279,11 +279,15 @@ static void ccw_init(MachineState *machine) s390_sclp_init(); s390_memory_init(machine->ram_size); + /* + * This might also enable some KVM features like AIS, so it must + * be called before the CPU model + */ + s390_flic_init(); + /* init CPUs (incl. CPU model) early so s390_has_feature() works */ s390_init_cpus(machine); - s390_flic_init(); - /* get a BUS */ css_bus = virtual_css_bus_init(); s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline, diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h index 7aab6ef..ac4e170 100644 --- a/include/hw/s390x/s390_flic.h +++ b/include/hw/s390x/s390_flic.h @@ -90,6 +90,7 @@ void s390_flic_init(void); S390FLICState *s390_get_flic(void); bool ais_needed(void *opaque); +void s390_flic_enable_ais(void); #ifdef CONFIG_KVM DeviceState *s390_flic_kvm_create(void); diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 5169379..03ff583 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -23,6 +23,7 @@ #include "qapi/qobject-input-visitor.h" #include "qapi/qmp/qbool.h" #ifndef CONFIG_USER_ONLY +#include "hw/s390x/s390_flic.h" #include "sysemu/arch_init.h" #endif @@ -901,6 +902,11 @@ static inline void apply_cpu_model(const S390CPUModel *model, Error **errp) applied_model = *model; } } + + if (model && + test_bit(S390_FEAT_ADAPTER_INT_SUPPRESSION, model->features)) { + s390_flic_enable_ais(); + } #endif } diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index ebb75ca..6c5c57e 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -311,13 +311,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) } } - /* - * The migration interface for ais was introduced with kernel 4.13 - * but the capability itself had been active since 4.12. As migration - * support is considered necessary let's disable ais in the 2.10 - * machine. - */ - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ + /* The AIS enablement happens in the flic realize */ qemu_mutex_init(&qemu_sigp_mutex);
Instead of unconditionally enabling the KVM AIS capability in the kvm arch init function, do this in the flic realize function when we know if migration is available. This requires to initialize flic before the CPUs. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- hw/intc/s390_flic.c | 11 +++++++++-- hw/intc/s390_flic_kvm.c | 8 +++++++- hw/s390x/s390-virtio-ccw.c | 8 ++++++-- include/hw/s390x/s390_flic.h | 1 + target/s390x/cpu_models.c | 6 ++++++ target/s390x/kvm.c | 8 +------- 6 files changed, 30 insertions(+), 12 deletions(-)