Messages in this thread | | | Date | Wed, 20 Apr 2022 09:43:40 -0400 | Subject | Re: [PATCH v5 11/21] KVM: s390: pci: do initial setup for AEN interpretation | From | Matthew Rosato <> |
| |
On 4/19/22 4:16 AM, Pierre Morel wrote: > > > On 4/4/22 19:43, Matthew Rosato wrote: >> Initial setup for Adapter Event Notification Interpretation for zPCI >> passthrough devices. Specifically, allocate a structure for >> forwarding of >> adapter events and pass the address of this structure to firmware. >> >> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >> ---
...
>> + >> +static int zpci_reset_aipb(u8 nisc) >> +{ >> + /* >> + * AEN registration can only happen once per system boot. If >> + * an aipb already exists then AEN was already registered and >> + * we can re-use the aipb contents. This can only happen if >> + * the KVM module was removed and re-inserted. >> + */ >> + if (zpci_aipb->aipb.faal != ZPCI_NR_DEVICES || >> + zpci_aipb->aipb.afi != nisc) { >> + return -EINVAL; >> + } > > I do not understand how faal cound be different of ZPCI_NR_DEVICES if > aipb has been already initialised. > Same for afi. > Can you please explain?
Well, my concern was along the lines of 'what if we rmmod kvm and then insmod a different version of the kvm module' -- These are really sanity checks.
Now, ZPCI_NR_DEVICES/faal is built in with PCI, so yeah this check is probably unnecessary as we shouldn't be able to change this value without a new kernel.
afi is however derived from nisc, which was passed in all the way from kvm_s390_gib_init during kvm_arch_init. Today, this is hard-coded as GAL_ISC; but the point is that this is hard-coded within the kvm module, so we can't be quite sure that it's the same value every time we insmod kvm. In an (admittedly, far-fetched) scenario where we insmod kvm, initialize AEN with GAL_ISC, rmmod kvm, then insmod a kvm where, for example, GAL_ISC was changed to a different number, we would need to trigger a failure here because we have no way to update the forwarding isc with firmware until the kernel is rebooted.
| |