Re: [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions

[Cornelia Huck]

On Thu, 24 Aug 2017 11:29:47 +0200
Halil Pasic <address@hidden> wrote:

> On 08/23/2017 05:54 PM, Cornelia Huck wrote:
> > If we do not provide zpci, pci reconfiguration via sclp is not available
> > either. I/O adapter configuration, however, should always be present.
> > 
> > Rename the values that refer to I/O adapter configuration (instead of only
> > pci) to make things clearer.
> > 
> > Move length checking of the sccb for I/O adapter configuration into the
> > common sclp code (out of the pci code). This also fixes an issue that
> > the pci code would refer to a field in the sccb before checking whether
> > it was actually long enough.
> > 
> > Check for the adapter type in the sccb and return unrecognized adapter
> > type if the guest tries to issue I/O adapter configure/deconfigure for
> > a type other than pci or for pci if the zpci facility is not provided.
> > 
> > Signed-off-by: Cornelia Huck <address@hidden>
> > ---
> >  hw/s390x/s390-pci-bus.c  | 14 ++------------
> >  hw/s390x/s390-pci-bus.h  |  8 --------
> >  hw/s390x/s390-pci-stub.c |  2 ++
> >  hw/s390x/sclp.c          | 39 ++++++++++++++++++++++++++++++++++-----
> >  include/hw/s390x/sclp.h  | 17 +++++++++++++----
> >  5 files changed, 51 insertions(+), 29 deletions(-)

> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index 9253dbbc64..7ae6a0e37a 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -80,7 +80,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
> >      prepare_cpu_entries(sclp, read_info->entries, cpu_count);
> > 
> >      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
> > -                                        SCLP_HAS_PCI_RECONFIG);
> > +                                        SCLP_HAS_IOA_RECONFIG);
> > 
> >      /* Memory Hotplug is only supported for the ccw machine type */
> >      if (mhd) {
> > @@ -354,6 +354,35 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB 
> > *sccb)
> >      sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
> >  }
> > 
> > +static void sclp_configure_io_adapter(SCLPDevice *sclp, SCCB *sccb,
> > +                                      bool configure)  
> 
> Just an idea. This could be called reconfigure instead
> of configure (that's sclp_reconfigure_io_adapter) as the
> facility providing both conf and deconf is called reconf.
> 
> It ain't important. Patch looks good!

So I'll just keep it as-is :)

> 
> Reviewed-by: Halil Pasic <address@hidden>

Thanks!