On Thu, 2021-12-09 at 15:20 -0500, Matthew Rosato wrote:
> On 12/9/21 2:54 PM, Christian Borntraeger wrote:
> > Am 07.12.21 um 21:57 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@xxxxxxxxxxxxx>
> > > ---
> > > arch/s390/include/asm/pci_insn.h | 12 ++++
> > > arch/s390/kvm/interrupt.c | 17 +++++
> > > arch/s390/kvm/kvm-s390.c | 3 +
> > > arch/s390/kvm/pci.c | 113 +++++++++++++++++++++++++++++++
> > > arch/s390/kvm/pci.h | 42 ++++++++++++
> > > 5 files changed, 187 insertions(+)
> > > create mode 100644 arch/s390/kvm/pci.h
> > >
---8<---
> > > int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
> > > {
> > > @@ -55,3 +162,9 @@ int kvm_s390_pci_attach_kvm(struct zpci_dev *zdev,
> > > struct kvm *kvm)
> > > return 0;
> > > }
> > > EXPORT_SYMBOL_GPL(kvm_s390_pci_attach_kvm);
> > > +
> > > +void kvm_s390_pci_init(void)
> > > +{
> > > + spin_lock_init(&aift.gait_lock);
> > > + mutex_init(&aift.lock);
> > > +}
> >
> > Can we maybe use designated initializer for the static definition of
> > aift, e.g. something
> > like
> > static struct zpci_aift aift = {
> > .gait_lock = __SPIN_LOCK_UNLOCKED(aift.gait_lock),
> > .lock = __MUTEX_INITIALIZER(aift.lock),
> > }
> > and get rid of the init function? >
>
> Maybe -- I can certainly do the above, but I do add a call to
> zpci_get_mdd() in the init function (patch 23), so if I want to in patch
> 23 instead add .mdd = zpci_get_mdd() to this designated initializer I'd
> have to re-work zpci_get_mdd (patch 12) to return the mdd rather than
> the CLP LIST PCI return code. We want at least a warning if we're
> setting a 0 for mdd because the CLP failed for some bizarre reason.
>
> I guess one option would be to move the WARN_ON into the zpci_get_mdd()
> function itself and then now we can do
Hmm, if we do change zpci_get_mdd() which I'm generally fine with I
feel like the initializer would be weird mix of truly static lock
initialization and a function that actually does a CLP.
I'm also a little worried about initialization order if kvm is built-
in. The CLP should work even with PCI not initialized but what if for
example the facility isn't even there?
Also if you do change zpci_get-mdd() I'd prefer a pr_err() instead of a
WARN_ON(), no reason to crash the system for this if it runs with
panic-on-warn. So I think overall keeping it as is makes more sense.
