Re: [PATCH v2 0/4] apic: Fix migration breakage of >255 vcpus

[Peter Xu <peterx@redhat.com>]

On Sat, Oct 19, 2019 at 11:41:53AM +0800, Peter Xu wrote:
> On Wed, Oct 16, 2019 at 11:40:01AM -0300, Eduardo Habkost wrote:
> > On Wed, Oct 16, 2019 at 10:29:29AM +0800, Peter Xu wrote:
> > > v2:
> > > - use uint32_t rather than int64_t [Juan]
> > > - one more patch (patch 4) to check dup SaveStateEntry [Dave]
> > > - one more patch to define a macro (patch 1) to simplify patch 2
> > > 
> > > Please review, thanks.
> > 
> > I wonder how hard it is to write a simple test case to reproduce
> > the original bug.  We can extend tests/migration-test.c or
> > tests/acceptance/migration.py.  If using -device with explicit
> > apic-id, we probably don't even need to create >255 VCPUs.
> 
> I can give it a shot next week. :)

When trying this, I probably noticed a block layer issue: q35 seems to
have problem on booting from a very small block device (like 512B,
which is the image size that currently used for migration-test.c).
For example, this cmdline can boot successfully into the test image:

$qemu -M pc -m 200m -accel kvm -nographic \
      -drive file=$image,id=drive0,index=0,format=raw \
      -device ide-hd,drive=drive0

While this cannot:

$qemu -M q35 -m 200m -accel kvm -nographic \
      -drive file=$image,id=drive0,index=0,format=raw \
      -device ide-hd,drive=drive0

With error (BIOS debug messages on):

Booting from Hard Disk..invalid basic_access:143:
   a=00000201  b=00000000  c=00000001  d=00000080 ds=0000 es=07c0 ss=d980
  si=00000000 di=00000000 bp=00000000 sp=0000fd8e cs=f000 ip=cb81  f=0202
invalid basic_access:144:
   a=00000201  b=00000000  c=00000001  d=00000080 ds=0000 es=07c0 ss=d980
  si=00000000 di=00000000 bp=00000000 sp=0000fd8e cs=f000 ip=cb81  f=0202
.
Boot failed: could not read the boot disenter handle_18:
  NULL
k

This corresponds to this SeaBIOS check error:

static void noinline
basic_access(struct bregs *regs, struct drive_s *drive_fl, u16 command)
{
    ...
    // sanity check on cyl heads, sec
    if (cylinder >= nlc || head >= nlh || sector > nls) {
        warn_invalid(regs);
        disk_ret(regs, DISK_RET_EPARAM);
        return;
    }
    ...
}

And... below cmdline will work even for q35 (as suggested by Fam when
we talked offline):

$qemu -M q35 -m 200m -accel kvm -nographic \
      -drive file=$image,id=drive0,index=0,format=raw \
      -device ide-hd,drive=drive0,secs=1,cyls=1,heads=1

I think for migration test we can workaround like above, but I'm also
curious whether this is a real bug somewhere because I don't see a
reason for q35 to refuse to boot on a one-sector image.

Thanks,

-- 
Peter Xu