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

[Jason J. Herne]

On 4/5/19 3:52 AM, Thomas Huth wrote:
> On 05/04/2019 08.58, Thomas Huth wrote:
> > On 04/04/2019 16.34, Jason J. Herne wrote:
> > > This is to support booting from vfio-ccw dasd devices. We basically implement
> > > the real hardware ipl procedure. This allows for booting Linux guests on
> > > vfio-ccw devices.
> > >
> > > vfio-ccw's channel program prefetch algorithm complicates ipl because most ipl
> > > channel programs dynamically modify themselves. Details on the ipl process and
> > > how we worked around this issue can be found in docs/devel/s390-dasd-ipl.txt.
> >
> >   Hi Jason,
> >
> > while running my s390-ccw bios tests, I noticed that network booting
> > seems to be broken now. This used to work before:
> >
> > s390x-softmmu/qemu-system-s390x -nographic -accel kvm \
> >   -bios pc-bios/s390-ccw/s390-ccw.img \
> >   -global s390-ipl.netboot_fw=pc-bios/s390-ccw/s390-netboot.img \
> >   -netdev user,id=n1,tftp=/boot,bootfile=vmlinuz-4.18.0 \
> >   -device virtio-net-ccw,netdev=n1,bootindex=1
> >
> > Now it simply fails with "! No IPL device available !".
> >
> > Could you have a look at it, please?
>
> FWIW: The problem seems to be in the last patch: virtio_is_supported()
> is now not called anymore, and so virtio_get_device_type() now returns
> the wrong type.
>
>   Thomas

Good catch. Testing netboot for this iteration did slip my mind. I've now added a basic 
netboot script to my test suite to avoid this type of regression in the future.

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.


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


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