Re: [Qemu-devel] [qemu-s390x] [PATCH v6 00/16] s390: vfio-ccw dasd ipl support

[Cornelia Huck <cohuck@redhat.com>]

On Fri, 5 Apr 2019 15:26:24 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 05/04/2019 15.11, Jason J. Herne wrote:

> > Your analysis of the problem matches what I'm seeing as well. Here is
> > what I'm proposing to fix it. If you like it, let me know if you want me
> > to re-send just the final patch, or the entire series again with a v7 tag.

If you (Jason) resend just that patch, I'm happy to give it a tag :)

> > 
> > 
> > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> > index 03e90e3..3d1e9a4 100644
> > --- a/pc-bios/s390-ccw/main.c
> > +++ b/pc-bios/s390-ccw/main.c
> > @@ -67,6 +67,7 @@ static bool find_subch(int dev_no)
> >  {
> >      Schib schib;
> >      int i, r;
> > +    bool is_virtio;
> > 
> >      for (i = 0; i < 0x10000; i++) {
> >          blk_schid.sch_no = i;
> > @@ -80,12 +81,13 @@ static bool find_subch(int dev_no)
> > 
> >          enable_subchannel(blk_schid);
> >          cutype = cu_type(blk_schid);
> > +        is_virtio = virtio_is_supported(blk_schid);
> > 
> >          /* No specific devno given, just return 1st possibly bootable
> > device */
> >          if (dev_no < 0) {
> >              switch (cutype) {
> >              case CU_TYPE_VIRTIO:
> > -                if (virtio_is_supported(blk_schid)) {
> > +                if (is_virtio) {
> >                      /*
> >                       * Skip net devices since no IPLB is created and
> > therefore
> >                       * no network bootloader has been loaded
> > 
> >   
> 
> Looks fine to me. But maybe we should also add a comment before the new
> "virtio_is_supported" line, saying something like "Note: we always have
> to run virtio_is_supported() here to make sure that the vdev.senseid
> data gets pre-initialized"? Otherwise, we might accidentally "revert"
> this patch when somebody thinks that the code might be optimizable by
> removing the is_virtio variable again...

Agreed.

> 
> If you like, I can squash these changes in when I pick up the patches,
> so you don't have to resend.
> 
>  Thomas
>