Messages in this thread | | | Date | Thu, 14 Apr 2022 09:00:58 -0400 | Subject | Re: [PATCH v5 11/21] KVM: s390: pci: do initial setup for AEN interpretation | From | Matthew Rosato <> |
| |
On 4/14/22 3:20 AM, Christian Borntraeger wrote: > > > Am 04.04.22 um 19:43 schrieb Matthew Rosato: >> 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> > [...] >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index 156d1c25a3c1..9db6f8080f71 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -47,6 +47,7 @@ >> #include <asm/fpu/api.h> >> #include "kvm-s390.h" >> #include "gaccess.h" >> +#include "pci.h" >> #define CREATE_TRACE_POINTS >> #include "trace.h" >> @@ -502,6 +503,14 @@ int kvm_arch_init(void *opaque) >> goto out; >> } >> + if (kvm_s390_pci_interp_allowed()) { >> + rc = kvm_s390_pci_init(); >> + if (rc) { >> + pr_err("Unable to allocate AIFT for PCI\n"); >> + goto out; >> + } >> + } >> + >> rc = kvm_s390_gib_init(GAL_ISC); >> if (rc) >> goto out; > > We would not free the aift that was allocated by kvm_s390_pci_init > in kvm_arch_exit. > Wouldnt we re-allocate a new aift when we unload/reload kvm forgetting > about the old one?
Oops, yes it looks like that's the case. We must back-pocket a certain subset of firmware-shared structures (e.g. zpci_aipb and zpci_aif_sbv) as these cannot change for the life of the system once registered with firmware; but the aift is a kernel-only structure that should be safe to free until next module load. I think this can be done at the end of kvm_s390_pci_aen_exit (with some caller adjustments re: the aift mutex) > > >> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c > [...] >> +static int zpci_setup_aipb(u8 nisc) > [...] >> + size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES * >> + sizeof(struct zpci_gaite))); > [...] >> + if (zpci_set_irq_ctrl(SIC_SET_AENI_CONTROLS, 0, zpci_aipb)) { >> + rc = -EIO; >> + goto free_gait; >> + } >> + >> + return 0; >> + >> +free_gait: >> + size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES * >> + sizeof(struct zpci_gaite))); > > size should still be valid here?
Good point
> >> + free_pages((unsigned long)aift->gait, size); >> +free_sbv: >> + airq_iv_release(aift->sbv); >> + zpci_aif_sbv = 0; >> +free_aipb: >> + kfree(zpci_aipb); >> + zpci_aipb = 0; >> + >> + return rc; >> +} >> + > > The remaining parts look sane.
| |