Re: [Qemu-devel] [PATCH RFC v2 7/9] s390x/pci: fence off instructions for non-pci

[Cornelia Huck <cohuck@redhat.com>]

On Wed, 19 Jul 2017 10:07:53 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 18.07.2017 16:24, Cornelia Huck wrote:
> > If a guest running on a machine without zpci issues a pci instruction,
> > throw them an exception.
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  target/s390x/kvm.c | 54 +++++++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 41 insertions(+), 13 deletions(-)  
> [...]
> >  static int kvm_mpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
> > @@ -1269,10 +1293,14 @@ static int kvm_mpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
> >      uint64_t fiba;
> >      uint8_t ar;
> >  
> > -    cpu_synchronize_state(CPU(cpu));
> > -    fiba = get_base_disp_rxy(cpu, run, &ar);
> > +    if (s390_has_feat(S390_FEAT_ZPCI)) {
> > +        cpu_synchronize_state(CPU(cpu));
> > +        fiba = get_base_disp_rxy(cpu, run, &ar);
> >  
> > -    return mpcifc_service_call(cpu, r1, fiba, ar);
> > +        return mpcifc_service_call(cpu, r1, fiba, ar);
> > +    } else {
> > +        return -1;
> > +    }
> >  }  
> 
> I think I'd rather write these cases with early exits à la
> 
>     if (!s390_has_feat(S390_FEAT_ZPCI)) {
>         return -1;
>     }
> 
> so you could get along with less change and less indentation 

Yeah, that's just the same as for the other patch, only that the
difference is smaller here. I'll just keep it, unless there are strong
feelings.

> ... but
> since this is rather a matter of taste, anyway:
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Thanks!