KVM create a win7 guest with Qemu, it boots up fail

[Robert Hu]

Environment:
------------
Host OS (ia32/ia32e/IA64):ia32e
Guest OS (ia32/ia32e/IA64):ia32e
Guest OS Type (Linux/Windows):Windows
kvm.git Commit:94b3ffcd41a90d2cb0b32ca23aa58a01111d5dc0
qemu-kvm Commit:839a5547574e57cce62f49bfc50fe1f04b00589a
Host Kernel Version:3.14.0-rc3
Hardware:Romley_EP, Ivytown_EP, HSW_EP

Bug detailed description:
--------------------------
when create a win7 guest, the guest boot up fail.

note:
1. when create win2000, winxp, win2k3, win2k8, guest, the guest boot up fail.
2. when create win8, win8.1, win2012 guest, the guest boot up fine.

Reproduce steps:
----------------
1.create guest
qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -net none -hda /root/win7.qcow

Current result:
----------------
win7 guest boot up fail

Expected result:
----------------
win7 guest boot up fine

Basic root-causing log:
----------------------

This should be a qemu bug
kvm + qemu = result
94b3ffcd + 839a5547 = bad
94b3ffcd + 3a87f8b6 = good

the first bad commit is:
commit 9bcc80cd71892df42605e0c097d85c0237ff45d1
Author: Laszlo Ersek <email address hidden>
Date: Mon Mar 17 17:05:16 2014 +0100

    i386/acpi-build: allow more than 255 elements in CPON

    The build_ssdt() function builds a number of AML objects that are related
    to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs.
    (APIC IDs are in fact discontiguous, but this is the traditional
    interface: build a contiguous sequence from zero up that covers all
    possible APIC IDs.) These objects are:

    - a Processor() object for each VCPU,
    - a NTFY method, with one branch for each VCPU,
    - a CPON package with one element (hotplug status byte) for each VCPU.

    The build_ssdt() function currently limits the *count* of processor
    objects, and NTFY branches, and CPON elements, in 0xFF (see the assignment
    to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
    This is incorrect, because the highest APIC ID that we otherwise allow a
    VCPU to take is 255.

    In order to extend the maximum count to 256, and the traversed APIC ID
    range correspondingly to [0..255]:
    - the Processor() objects need no change,
    - the NTFY method also needs no change,
    - the CPON package must be updated, because it is defined with a
      DefPackage, and the number of elements in such a package can be at most
      255. We pick a DefVarPackage instead.

    We replace the Op byte, and the encoding of the number of elements.
    Compare:

    DefPackage := PackageOp PkgLength NumElements PackageElementList
    DefVarPackage := VarPackageOp PkgLength VarNumElements PackageElementList

    PackageOp := 0x12
    VarPackageOp := 0x13

    NumElements := ByteData
    VarNumElements := TermArg => Integer

    The build_append_int() function implements precisely the following TermArg
    encodings (a subset of what the ACPI spec describes):

      TermArg := DataObject
      DataObject := ComputationalData
      ComputationalData := ConstObj | ByteConst | WordConst | DWordConst
      directly encoded in the function, with build_append_byte():
        ConstObj := ZeroOp | OneOp
          ZeroOp := 0x00
          OneOp := 0x01

      call to build_append_value(..., 1):
        ByteConst := BytePrefix ByteData
          BytePrefix := 0x0A
          ByteData := 0x00 - 0xFF

      call to build_append_value(..., 2):
        WordConst := WordPrefix WordData
          WordPrefix := 0x0B
          WordData := ByteData[0:7] ByteData[8:15]

      call to build_append_value(..., 4):
        DWordConst := DWordPrefix DWordData
          DWordPrefix := 0x0C
          DWordData := WordData[0:15] WordData[16:31]

    Signed-off-by: Laszlo Ersek <email address hidden>
    Reviewed-by: Michael S. Tsirkin <email address hidden>
    Signed-off-by: Michael S. Tsirkin <email address hidden>

[Gonglei (arei-gonglei)]

Hi,

I also encounter the same problem. When I use the Qemu mainline and with
-machine pc-i440fx-2.0, the win7 guest will show blue screen, and give me
"The BIOS in this system is not fully ACPI compliant. Please contact your system
 Vendor for an updated BIOS.
Technical information:
*** STOP: 0x000000A5 (...)
"

But when I change the machine property to pc-i440fx-1.5, the guest boots up fine.

Best regards,
-Gonglei

> Title:
> KVM create a win7 guest with Qemu, it boots up fail
>
> Status in QEMU:
> New
>
> Bug description:
> Environment:
> ------------
> Host OS (ia32/ia32e/IA64):ia32e
> Guest OS (ia32/ia32e/IA64):ia32e
> Guest OS Type (Linux/Windows):Windows
> kvm.git Commit:94b3ffcd41a90d2cb0b32ca23aa58a01111d5dc0
> qemu-kvm Commit:839a5547574e57cce62f49bfc50fe1f04b00589a
> Host Kernel Version:3.14.0-rc3
> Hardware:Romley_EP, Ivytown_EP, HSW_EP
>
>
> Bug detailed description:
> --------------------------
> when create a win7 guest, the guest boot up fail.
>
> note:
> 1. when create win2000, winxp, win2k3, win2k8, guest, the guest boot up
> fail.
> 2. when create win8, win8.1, win2012 guest, the guest boot up fine.
>
>
> Reproduce steps:
> ----------------
> 1.create guest
> qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -net none -hda
> /root/win7.qcow
>
>
> Current result:
> ----------------
> win7 guest boot up fail
>
> Expected result:
> ----------------
> win7 guest boot up fine
>
> Basic root-causing log:
> ----------------------
>
> This should be a qemu bug
> kvm + qemu = result
> 94b3ffcd + 839a5547 = bad
> 94b3ffcd + 3a87f8b6 = good
>
> the first bad commit is:
> commit 9bcc80cd71892df42605e0c097d85c0237ff45d1
> Author: Laszlo Ersek <email address hidden>
> Date: Mon Mar 17 17:05:16 2014 +0100
>
> i386/acpi-build: allow more than 255 elements in CPON
>
> The build_ssdt() function builds a number of AML objects that are
> related
> to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs.
> (APIC IDs are in fact discontiguous, but this is the traditional
> interface: build a contiguous sequence from zero up that covers all
> possible APIC IDs.) These objects are:
>
> - a Processor() object for each VCPU,
> - a NTFY method, with one branch for each VCPU,
> - a CPON package with one element (hotplug status byte) for each
> VCPU.
>
> The build_ssdt() function currently limits the *count* of processor
> objects, and NTFY branches, and CPON elements, in 0xFF (see the
> assignment
> to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
> This is incorrect, because the highest APIC ID that we otherwise allow a
> VCPU to take is 255.
>
> In order to extend the maximum count to 256, and the traversed APIC
> ID
> range correspondingly to [0..255]:
> - the Processor() objects need no change,
> - the NTFY method also needs no change,
> - the CPON package must be updated, because it is defined with a
> ...

[Stefan Hajnoczi (stefanha)]

On Wed, Mar 26, 2014 at 06:45:10AM -0000, Robert Hu wrote:

CCing Laszlo, Michael, and Marcel for ACPI

> Public bug reported:
>
> Environment:
> ------------
> Host OS (ia32/ia32e/IA64):ia32e
> Guest OS (ia32/ia32e/IA64):ia32e
> Guest OS Type (Linux/Windows):Windows
> kvm.git Commit:94b3ffcd41a90d2cb0b32ca23aa58a01111d5dc0
> qemu-kvm Commit:839a5547574e57cce62f49bfc50fe1f04b00589a
> Host Kernel Version:3.14.0-rc3
> Hardware:Romley_EP, Ivytown_EP, HSW_EP
>
>
> Bug detailed description:
> --------------------------
> when create a win7 guest, the guest boot up fail.
>
> note:
> 1. when create win2000, winxp, win2k3, win2k8, guest, the guest boot up fail.
> 2. when create win8, win8.1, win2012 guest, the guest boot up fine.
>
>
> Reproduce steps:
> ----------------
> 1.create guest
> qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -net none -hda /root/win7.qcow
>
>
> Current result:
> ----------------
> win7 guest boot up fail
>
> Expected result:
> ----------------
> win7 guest boot up fine
>
> Basic root-causing log:
> ----------------------
>
> This should be a qemu bug
> kvm + qemu = result
> 94b3ffcd + 839a5547 = bad
> 94b3ffcd + 3a87f8b6 = good
>
> the first bad commit is:
> commit 9bcc80cd71892df42605e0c097d85c0237ff45d1
> Author: Laszlo Ersek <email address hidden>
> Date: Mon Mar 17 17:05:16 2014 +0100
>
> i386/acpi-build: allow more than 255 elements in CPON
>
> The build_ssdt() function builds a number of AML objects that are related
> to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs.
> (APIC IDs are in fact discontiguous, but this is the traditional
> interface: build a contiguous sequence from zero up that covers all
> possible APIC IDs.) These objects are:
>
> - a Processor() object for each VCPU,
> - a NTFY method, with one branch for each VCPU,
> - a CPON package with one element (hotplug status byte) for each VCPU.
>
> The build_ssdt() function currently limits the *count* of processor
> objects, and NTFY branches, and CPON elements, in 0xFF (see the assignment
> to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
> This is incorrect, because the highest APIC ID that we otherwise allow a
> VCPU to take is 255.
>
> In order to extend the maximum count to 256, and the traversed APIC ID
> range correspondingly to [0..255]:
> - the Processor() objects need no change,
> - the NTFY method also needs no change,
> - the CPON package must be updated, because it is defined with a
> DefPackage, and the number of elements in such a package can be at most
> 255. We pick a DefVarPackage instead.
>
> We replace the Op byte, and the encoding of the number of elements.
> Compare:
>
> DefPackage := PackageOp PkgLength NumElements PackageElementList
> DefVarPackage := VarPackageOp PkgLength VarNumElements PackageElementList
>
> PackageOp := 0x12
> VarPackageOp := 0x13
>
> NumElements := ByteData
> VarNumElements := TermArg => Integer
>
> The build_append_int() function implements precisely the following TermArg
> encodings (a subs...

[Laszlo Ersek (Red Hat) (lersek)]

On 03/26/14 11:31, Michael S. Tsirkin wrote:

> On Wed, Mar 26, 2014 at 06:45:10AM -0000, Robert Hu wrote:

>> Date: Mon Mar 17 17:05:16 2014 +0100
>>
>> i386/acpi-build: allow more than 255 elements in CPON
>>
>> The build_ssdt() function builds a number of AML objects that are related
>> to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs.
>> (APIC IDs are in fact discontiguous, but this is the traditional
>> interface: build a contiguous sequence from zero up that covers all
>> possible APIC IDs.) These objects are:
>>
>> - a Processor() object for each VCPU,
>> - a NTFY method, with one branch for each VCPU,
>> - a CPON package with one element (hotplug status byte) for each VCPU.
>>
>> The build_ssdt() function currently limits the *count* of processor
>> objects, and NTFY branches, and CPON elements, in 0xFF (see the assignment
>> to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
>> This is incorrect, because the highest APIC ID that we otherwise allow a
>> VCPU to take is 255.
>>
>> In order to extend the maximum count to 256, and the traversed APIC ID
>> range correspondingly to [0..255]:
>> - the Processor() objects need no change,
>> - the NTFY method also needs no change,
>> - the CPON package must be updated, because it is defined with a
>> DefPackage, and the number of elements in such a package can be at most
>> 255. We pick a DefVarPackage instead.
>>
>> We replace the Op byte, and the encoding of the number of elements.
>> Compare:
>>
>> DefPackage := PackageOp PkgLength NumElements PackageElementList
>> DefVarPackage := VarPackageOp PkgLength VarNumElements PackageElementList
>>
>> PackageOp := 0x12
>> VarPackageOp := 0x13
>
>
> I think I know what's going on here: the specification says:
>
> The ASL compiler can emit two different AML opcodes for a Package
> declaration, either PackageOp or VarPackageOp. For small, fixed-length
> packages, the PackageOp is used and this
>
> opcode is compatible with ACPI 1.0. A VarPackageOp will be emitted if
> any of the following conditions are true:
> •
> The NumElements argument is a TermArg that can only be resolved at
> runtime.
> •
> At compile time, NumElements resolves to a constant that is larger than
> 255.
> •
> The PackageList contains more than 255 initializer elements.
>
>
> So we clearly violate this rule.

I did see this passage of the spec, but it is not relevant. It is about what the ASL compiler does. It comes from:

19 ACPI Source Language (ASL)Reference
19.5 ASL Operator Reference
19.5.98 Package (Declare Package Object)

We do not have an ASL compiler at hand. The specification nowhere restricts VarPackageOp to > 255.

However, what I *did* mess up is compatibility with ACPI 1.0. Just below the quoted part, there's also this:

  Note: The ability to create variable-sized packages was first
        introduced in ACPI 2.0. ACPI 1.0 only allowed fixed-size
        packages with up to 255 elements.

I forgot that the header of the containing table stated the ACPI version:

    /* Copy he...

[Laszlo Ersek (Red Hat) (lersek)]

On 03/26/14 13:58, Michael S. Tsirkin wrote:
> On Wed, Mar 26, 2014 at 01:28:02PM +0100, Laszlo Ersek wrote:
>> On 03/26/14 11:31, Michael S. Tsirkin wrote:
>>
>>> On Wed, Mar 26, 2014 at 06:45:10AM -0000, Robert Hu wrote:
>>
>>>> Date: Mon Mar 17 17:05:16 2014 +0100
>>>>
>>>> i386/acpi-build: allow more than 255 elements in CPON
>>>>
>>>> The build_ssdt() function builds a number of AML objects that are related
>>>> to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs.
>>>> (APIC IDs are in fact discontiguous, but this is the traditional
>>>> interface: build a contiguous sequence from zero up that covers all
>>>> possible APIC IDs.) These objects are:
>>>>
>>>> - a Processor() object for each VCPU,
>>>> - a NTFY method, with one branch for each VCPU,
>>>> - a CPON package with one element (hotplug status byte) for each VCPU.
>>>>
>>>> The build_ssdt() function currently limits the *count* of processor
>>>> objects, and NTFY branches, and CPON elements, in 0xFF (see the assignment
>>>> to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
>>>> This is incorrect, because the highest APIC ID that we otherwise allow a
>>>> VCPU to take is 255.
>>>>
>>>> In order to extend the maximum count to 256, and the traversed APIC ID
>>>> range correspondingly to [0..255]:
>>>> - the Processor() objects need no change,
>>>> - the NTFY method also needs no change,
>>>> - the CPON package must be updated, because it is defined with a
>>>> DefPackage, and the number of elements in such a package can be at most
>>>> 255. We pick a DefVarPackage instead.
>>>>
>>>> We replace the Op byte, and the encoding of the number of elements.
>>>> Compare:
>>>>
>>>> DefPackage := PackageOp PkgLength NumElements PackageElementList
>>>> DefVarPackage := VarPackageOp PkgLength VarNumElements PackageElementList
>>>>
>>>> PackageOp := 0x12
>>>> VarPackageOp := 0x13
>>>
>>>
>>> I think I know what's going on here: the specification says:
>>>
>>> The ASL compiler can emit two different AML opcodes for a Package
>>> declaration, either PackageOp or VarPackageOp. For small,
>>> fixed-length packages, the PackageOp is used and this
>>>
>>> opcode is compatible with ACPI 1.0. A VarPackageOp will be emitted
>>> if any of the following conditions are true:
>>> *
>>> The NumElements argument is a TermArg that can only be resolved at
>>> runtime.
>>> *
>>> At compile time, NumElements resolves to a constant that is larger
>>> than 55.
>>> *
>>> The PackageList contains more than 255 initializer elements.
>>>
>>>
>>> So we clearly violate this rule.
>>
>> I did see this passage of the spec, but it is not relevant. It is
>> about what the ASL compiler does. It comes from:
>>
>> 19 ACPI Source Language (ASL)Reference
>> 19.5 ASL Operator Reference
>> 19.5.98 Package (Declare Package Object)
>>
>> We do not have an ASL compiler at hand.
>
> True. But I think the spec and guests simply didn't envision writing
> AML by hand :)

I sort of disagree. The spec has an entire chapter dedicated to AML. If
the restriction we...

[Eduardo Habkost (ehabkost)]

On Wed, Mar 26, 2014 at 04:54:31PM +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 26, 2014 at 02:48:29PM +0100, Igor Mammedov wrote:
[...]
> > more clean would be to abort if CPON index (i.e. APIC ID)
> > is more than 255. That would affect small number of weird
> > topologies but sould be fine for most usecases.
>
> Any idea how to trigger this behaviour?

Set sockets and/or cores to a 2^n+1 value. It will use n+1 bits for the
socket ID and core ID, making APIC ID larger than CPU index. You can get
a very large APIC ID if you use, for example:

  -smp 1,cores=17,threads=17,maxcpus=200

There's code to limit the maximum APIC ID and abort if it is too large,
already (so the above command-line will make QEMU abort). If we want to
limit APIC IDs to <255 instead of <256, we just need to change
ACPI_CPU_HOTPLUG_ID_LIMIT.

--
Eduardo

[Eduardo Habkost (ehabkost)]

On Wed, Mar 26, 2014 at 05:09:29PM +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 26, 2014 at 12:06:38PM -0300, Eduardo Habkost wrote:
> > On Wed, Mar 26, 2014 at 04:54:31PM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Mar 26, 2014 at 02:48:29PM +0100, Igor Mammedov wrote:
> > [...]
> > > > more clean would be to abort if CPON index (i.e. APIC ID)
> > > > is more than 255. That would affect small number of weird
> > > > topologies but sould be fine for most usecases.
> > >
> > > Any idea how to trigger this behaviour?
> >
> > Set sockets and/or cores to a 2^n+1 value. It will use n+1 bits for the
> > socket ID and core ID, making APIC ID larger than CPU index. You can get
> > a very large APIC ID if you use, for example:
> >
> > -smp 1,cores=17,threads=17,maxcpus=200
> >
> > There's code to limit the maximum APIC ID and abort if it is too large,
> > already (so the above command-line will make QEMU abort).
>
> Okay so how do you make this package size exactly 256?

I didn't look for a proof yet, but it looks like having apic_id_limit
exactly 256 while max_cpus <= 255 is impossible.

Laszlo, did you find a case where this was possible?

--
Eduardo

[Laszlo Ersek (Red Hat) (lersek)]

On 03/26/14 14:48, Igor Mammedov wrote:
> On Wed, 26 Mar 2014 14:58:28 +0200
> "Michael S. Tsirkin" <email address hidden> wrote:

>> If we want to change ACPI rev, I think we should do this
>> conditionally when max_cpus > 255.
>> Would be worth it if this fixes some guests.
>>
>> As for reverting, I think it's a problem that we seem to
>> allow max_cpus = 256 but then it doesn't really work.

> more clean would be to abort if CPON index (i.e. APIC ID)
> is more than 255. That would affect small number of weird
> topologies but sould be fine for most usecases.

The question is not about a CPON index / APIC ID that *exceeds* 255.

Eduardo's patches already make sure that the APIC ID *width* is at most
8 bits, so the highest APIC ID that any VCPU can take is already at most
255. (IOW the exclusive limit for APIC IDs is already 256.) In other
words, the CPON index already can't exceed 255.

The question is about the CPON index / APIC ID that is *precisely* 255.
Eduardo's patches allow this (correctly), but the SSDT generator used
*not* to create a CPON array element for the index 255. The generator
limited the CPON element *count* in 255, hence the maximum CPON index
(== APIC ID) that was available for hotplugging used to be 254.

My patch wanted to bump this CPON size one higher (to 256), so that the
VCPU with APIC ID 255 (== CPON array index 255) becomes hotpluggable.

However.

Given that
(a) for PC, we limit the *number* of VCPUs in 255, inclusive (ie. max
vcpu *index* is 254), and
(b) Eduardo's patches (correctly) restrict the accepted topologies so
that any APIC ID fits into 8 bits,

it turns out that there simply isn't a (VCPU count, topology) pair,
accepted by (a) and (b) simultaneously, that would enable a VCPU APIC ID
of 255. The attached program prints nothing.

(Note that as soon as you break (a), ie. increase MAX_CPUS to 256 in the
attached program, you immediately get a bunch of topologies where APIC
ID (CPON index) 255 becomes possible, while keeping (b) intact.)

Hence my patches fix a case that is purely academical (never happens in
practice) as long as (a) and (b) are guaranteed *together*.

I should have done more research before posting my patches.

Thanks (and sorry about the churn),
Laszlo

[Laszlo Ersek (Red Hat) (lersek)]

On 03/26/14 16:23, Eduardo Habkost wrote:
> On Wed, Mar 26, 2014 at 05:09:29PM +0200, Michael S. Tsirkin wrote:
>> On Wed, Mar 26, 2014 at 12:06:38PM -0300, Eduardo Habkost wrote:
>>> On Wed, Mar 26, 2014 at 04:54:31PM +0200, Michael S. Tsirkin wrote:
>>>> On Wed, Mar 26, 2014 at 02:48:29PM +0100, Igor Mammedov wrote:
>>> [...]
>>>>> more clean would be to abort if CPON index (i.e. APIC ID)
>>>>> is more than 255. That would affect small number of weird
>>>>> topologies but sould be fine for most usecases.
>>>>
>>>> Any idea how to trigger this behaviour?
>>>
>>> Set sockets and/or cores to a 2^n+1 value. It will use n+1 bits for the
>>> socket ID and core ID, making APIC ID larger than CPU index. You can get
>>> a very large APIC ID if you use, for example:
>>>
>>> -smp 1,cores=17,threads=17,maxcpus=200
>>>
>>> There's code to limit the maximum APIC ID and abort if it is too large,
>>> already (so the above command-line will make QEMU abort).
>>
>> Okay so how do you make this package size exactly 256?
>
> I didn't look for a proof yet, but it looks like having apic_id_limit
> exactly 256 while max_cpus <= 255 is impossible.
>
> Laszlo, did you find a case where this was possible?

No, it's impossible, like you say; see my other email with the small
program doing an exhaustive search. Any topology that simultaneously
results in:
- a maximal VCPU count <= 255, and
- an APIC ID width <= 8
will prevent the exact APIC ID value 255.

Thanks
Laszlo

[Robert Hu (robert-hu)]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:<email address hidden>]
> Sent: Wednesday, March 26, 2014 6:31 PM
> To: Bug 1297651
> Cc: <email address hidden>; <email address hidden>; <email address hidden>; Hu,
> Robert
> Subject: Re: [Bug 1297651] [NEW] KVM create a win7 guest with Qemu, it boots
> up fail
>
> On Wed, Mar 26, 2014 at 06:45:10AM -0000, Robert Hu wrote:
> > Public bug reported:
> >
> > Environment:
> > ------------
> > Host OS (ia32/ia32e/IA64):ia32e
> > Guest OS (ia32/ia32e/IA64):ia32e
> > Guest OS Type (Linux/Windows):Windows
> > kvm.git Commit:94b3ffcd41a90d2cb0b32ca23aa58a01111d5dc0
> > qemu-kvm Commit:839a5547574e57cce62f49bfc50fe1f04b00589a
> > Host Kernel Version:3.14.0-rc3
> > Hardware:Romley_EP, Ivytown_EP, HSW_EP
> >
> >
> > Bug detailed description:
> > --------------------------
> > when create a win7 guest, the guest boot up fail.
> >
> > note:
> > 1. when create win2000, winxp, win2k3, win2k8, guest, the guest boot up fail.
> > 2. when create win8, win8.1, win2012 guest, the guest boot up fine.
> >
> >
> > Reproduce steps:
> > ----------------
> > 1.create guest
> > qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -net none -hda
> /root/win7.qcow
> >
> >
> > Current result:
> > ----------------
> > win7 guest boot up fail
> >
> > Expected result:
> > ----------------
> > win7 guest boot up fine
> >
> > Basic root-causing log:
> > ----------------------
> >
> > This should be a qemu bug
> > kvm + qemu = result
> > 94b3ffcd + 839a5547 = bad
> > 94b3ffcd + 3a87f8b6 = good
> >
> > the first bad commit is:
> > commit 9bcc80cd71892df42605e0c097d85c0237ff45d1
> > Author: Laszlo Ersek <email address hidden>
>
> Thanks for the excellent bug report!
>
>
>
> > Date: Mon Mar 17 17:05:16 2014 +0100
> >
> > i386/acpi-build: allow more than 255 elements in CPON
> >
> > The build_ssdt() function builds a number of AML objects that are related
> > to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs.
> > (APIC IDs are in fact discontiguous, but this is the traditional
> > interface: build a contiguous sequence from zero up that covers all
> > possible APIC IDs.) These objects are:
> >
> > - a Processor() object for each VCPU,
> > - a NTFY method, with one branch for each VCPU,
> > - a CPON package with one element (hotplug status byte) for each VCPU.
> >
> > The build_ssdt() function currently limits the *count* of processor
> > objects, and NTFY branches, and CPON elements, in 0xFF (see the
> assignment
> > to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
> > This is incorrect, because the highest APIC ID that we otherwise allow a
> > VCPU to take is 255.
> >
> > In order to extend the maximum count to 256, and the traversed APIC ID
> > range correspondingly to [0..255]:
> > - the Processor() objects need no change,
> > - the NTFY method also needs no change,
> > - the CPON package must be updated, because it is defined with a
> > DefPackage, and the number of elements in such a package can be at
> most
>...

[Robert Hu (robert-hu)]

Best Regards,
Robert Ho

> -----Original Message-----
> From: Laszlo Ersek [mailto:<email address hidden>]
> Sent: Wednesday, March 26, 2014 9:57 PM
> To: Michael S. Tsirkin
> Cc: Bug 1297651; <email address hidden>; <email address hidden>; Hu, Robert
> Subject: Re: [Bug 1297651] [NEW] KVM create a win7 guest with Qemu, it boots
> up fail
>
> On 03/26/14 13:58, Michael S. Tsirkin wrote:
> > On Wed, Mar 26, 2014 at 01:28:02PM +0100, Laszlo Ersek wrote:
> >> On 03/26/14 11:31, Michael S. Tsirkin wrote:
> >>
> >>> On Wed, Mar 26, 2014 at 06:45:10AM -0000, Robert Hu wrote:
> >>
> >>>> Date: Mon Mar 17 17:05:16 2014 +0100
> >>>>
> >>>> i386/acpi-build: allow more than 255 elements in CPON
> >>>>
> >>>> The build_ssdt() function builds a number of AML objects that are
> related
> >>>> to CPU hotplug, and whose IDs form a contiguous sequence of APIC
> IDs.
> >>>> (APIC IDs are in fact discontiguous, but this is the traditional
> >>>> interface: build a contiguous sequence from zero up that covers all
> >>>> possible APIC IDs.) These objects are:
> >>>>
> >>>> - a Processor() object for each VCPU,
> >>>> - a NTFY method, with one branch for each VCPU,
> >>>> - a CPON package with one element (hotplug status byte) for each
> VCPU.
> >>>>
> >>>> The build_ssdt() function currently limits the *count* of processor
> >>>> objects, and NTFY branches, and CPON elements, in 0xFF (see the
> assignment
> >>>> to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254].
> >>>> This is incorrect, because the highest APIC ID that we otherwise allow
> a
> >>>> VCPU to take is 255.
> >>>>
> >>>> In order to extend the maximum count to 256, and the traversed APIC
> ID
> >>>> range correspondingly to [0..255]:
> >>>> - the Processor() objects need no change,
> >>>> - the NTFY method also needs no change,
> >>>> - the CPON package must be updated, because it is defined with a
> >>>> DefPackage, and the number of elements in such a package can be
> at most
> >>>> 255. We pick a DefVarPackage instead.
> >>>>
> >>>> We replace the Op byte, and the encoding of the number of
> elements.
> >>>> Compare:
> >>>>
> >>>> DefPackage := PackageOp PkgLength NumElements
> PackageElementList
> >>>> DefVarPackage := VarPackageOp PkgLength VarNumElements
> PackageElementList
> >>>>
> >>>> PackageOp := 0x12
> >>>> VarPackageOp := 0x13
> >>>
> >>>
> >>> I think I know what's going on here: the specification says:
> >>>
> >>> The ASL compiler can emit two different AML opcodes for a Package
> >>> declaration, either PackageOp or VarPackageOp. For small,
> >>> fixed-length packages, the PackageOp is used and this
> >>>
> >>> opcode is compatible with ACPI 1.0. A VarPackageOp will be emitted
> >>> if any of the following conditions are true:
> >>> *
> >>> The NumElements argument is a TermArg that can only be resolved at
> >>> runtime.
> >>> *
> >>> At compile time, NumElements resolves to a constant that is larger
> >>> than 55.
> >>> *
> >>> The PackageList contains more...

[Robert Hu (robert-hu)]

on latest commit (db237e33), this bug doesn't exit.

[Serge Hallyn (serge-hallyn)]

Quoting Robert Hu (<email address hidden>):
> on latest commit (db237e33), this bug doesn't exit.

Sorry, I don't see this commit in qemu.git?

[Robert Hu (robert-hu)]

[root@localhost qemu]# git branch -a
* master
  remotes/origin/HEAD -> origin/master
  remotes/origin/master
[root@localhost qemu]# git log db237e33
commit db237e33c08a279f0179f8f5128a6d10d9adc38a
Merge: 61898bc ad1c7e0
Author: Peter Maydell <email address hidden>
Date: Wed Mar 26 17:10:15 2014 +0000

    Merge remote-tracking branch 'remotes/riku/for-2.0' into staging

    * remotes/riku/for-2.0:
      linux-user: Correct DLINFO_ITEMS

    Signed-off-by: Peter Maydell <email address hidden>