Re: [qemu-s390x] [PATCH 04/15] s390-bios: Extend find_dev() for non-virtio devices

[Jason J. Herne]

On 2/11/19 11:38 AM, Thomas Huth wrote:
> On 2019-01-29 14:29, Jason J. Herne wrote:
> > We need a method for finding the subchannel of a dasd device. Let's
> > modify find_dev to handle this since it mostly does what we need. Up to
> > this point find_dev has been specific to only virtio devices.
> >
> > Signed-off-by: Jason J. Herne <address@hidden>
> > Acked-by: Halil Pasic <address@hidden>
> > ---
> >   pc-bios/s390-ccw/main.c | 16 +++++++++++-----
> >   1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> > index 67df421..7e3f65e 100644
> > --- a/pc-bios/s390-ccw/main.c
> > +++ b/pc-bios/s390-ccw/main.c
> > @@ -49,6 +49,12 @@ unsigned int get_loadparm_index(void)
> >       return atoui(loadparm_str);
> >   }
> >   
> > +/*
> > + * Find the subchannel connected to the given device (dev_no) and fill in the
> > + * subchannel information block (schib) with the connected subchannel's info.
> > + * NOTE: The global variable blk_schid is updated to contain the subchannel
> > + * information.
> > + */
> >   static bool find_dev(Schib *schib, int dev_no)
> >   {
> >       int i, r;
> > @@ -62,15 +68,15 @@ static bool find_dev(Schib *schib, int dev_no)
> >           if (!schib->pmcw.dnv) {
> >               continue;
> >           }
> > -        if (!virtio_is_supported(blk_schid)) {
> > -            continue;
> > -        }
> > +
> >           /* Skip net devices since no IPLB is created and therefore no
> > -         * no network bootloader has been loaded
> > +         * network bootloader has been loaded
> >            */
> > -        if (virtio_get_device_type() == VIRTIO_ID_NET && dev_no < 0) {
> > +        if (virtio_is_supported(blk_schid) &&
> > +            virtio_get_device_type() == VIRTIO_ID_NET && dev_no < 0) {
> >               continue;
> >           }
> > +
> >           if ((dev_no < 0) || (schib->pmcw.dev == dev_no)) {
> >               return true;
> >           }
>
> Not sure whether this really works as expected? If dev_no is -1, this
> used to return the first supported virtio device. Now it returns the
> first device that could be found - but how are we sure that we can boot
> from that device?

How could we ever be sure we could boot from the first virtio device? The dev_no=1 case 
means we don't know which device to boot from so we're guessing.  The only thing that is 
changing here is that we're allowing non-virtio devices as well. We do this because we now 
support booting from a device type that is not virtio.


--
-- Jason J. Herne (address@hidden)