Re: [Qemu-devel] [PATCH v3] s390x/ccw: create s390 phb for compat reasons as well

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 18 Sep 2017 14:03:20 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 18.09.2017 10:55, Cornelia Huck wrote:
> > d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made
> > registering the s390 pci host bridge conditional on presense
> > of the zpci facility bit. Sadly, that breaks migration from
> > machines that did not use the cpu model (2.7 and previous).
> > 
> > Create the s390 phb for pre-cpu model machines as well: We can
> > tweak s390_has_feat() to always indicate the zpci facility bit
> > when no cpu model is available (on 2.7 and previous compat machines).
> > 
> > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally")
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> > 
> > v2->v3:
> > - no longer RFC (I tested a bit more)
> > - removed unrelated hunk
> > - more verbose patch description
> > 
> > I'll wait a bit for more acks/reviews and will probably send a pull
> > request for s390x tomorrow or so before the amount of queued patches
> > gets out of hand...
> > 
> > ---
> >  target/s390x/cpu_models.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> > index c295e641e6..5169379db5 100644
> > --- a/target/s390x/cpu_models.c
> > +++ b/target/s390x/cpu_models.c
> > @@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat)
> >              }
> >          }
> >  #endif
> > +        if (feat == S390_FEAT_ZPCI) {
> > +            return true;
> > +        }
> >          return 0;
> >      }
> >      return test_bit(feat, cpu->model->features);
> >   
> 
> 1. cpu->model will always be set for QEMU, so you can move that into the
> ifdef, next do the other checks. You can even send a cleanup to remove
> the if (kvm_enabled()) check.

I prefer it the way it is now. There's nothing kvm specific about that
bit, and cpu->model always being set is not really obvious.

> 
> 2. "return pci_available" ?

I thought about that as well. I think, however, that a better way is
just to disable compat machines that rely on pci being available.

As you can turn off pci only manually, I'd like to defer this and first
get this out of the door, as it fixes an existing problem.