RE: [Xen-devel] [PATCH] HVM vcpu hotplug: Fix acpi method NTFY bug

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Keir Fraser wrote:
> On 01/02/2010 03:31, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> 
>> How about the followed update:
>> 1. keep original method NTFY, keep decision_tree to reduce scan loop;
>> 2. update method PRSC
>>     1). transfer para 'maxvcpus' (comes from config file) from qemu
>>     to mk_dsdt.c through bios_info; 2). at PRSC, only scan
>> 'maxvcpus' vcpus; 
>> because maxvcpus< 128, no risk for NTFY then.
> 
> Well, I'm confused now. #2 is really no more than an optimisation,
> right? And #1 contradicts your original patch, which only affected
> NTFY, and you claimed was a bug fix.
> 
> Is there, or is there not, currently a bug in NTFY? Or some bug in
> the way it is called by PRSC?
> 
> I mean, if there's no bug, let's leave it alone. At least until 4.0.0
> is done. I still haven't been able to understand your original
> complaints about the current NTFY method by the way -- I still firmly
> believe it is behaviourally identical to your patched version, for
> any given pair of arguments passed to it.
> 
> I could be missing something. If so you're going to have spell it out
> very slowly and clearly. :-)


Sorry, I didn't tell the story clear.

Sevreal days ago, when we pull from xen upstream (c/s 20840), we found vcpu 
add work fail.
For example, at config file, we set
    maxvcpus=8
    vcpus=3
When we add a new vcpu through monitor by 'cpu_set 3 online', it cannot add 
new cpu subdir as /sys/devices/system/cpu/cpu3, and the subdir cpu1 & cpu2 
also disappear.

We debug it, and found some issue with method NTFY and PRSC:
1. for method NTFY, dsdt code auto-generated by mk_dsdt.c is
        Method (NTFY, 2, NotSerialized)
        {
            If (And (Arg0, 0x40))
            {
                If (And (Arg0, 0x20))
                {
                    If (And (Arg0, 0x10))
                    {
                        If (And (Arg0, 0x08))
                        {
                            If (And (Arg0, 0x04))
                            {
                                If (And (Arg0, 0x02))
                                {
                                    If (And (Arg0, One))
                                    {
                                        If (LNotEqual (Arg1, ^PR7F.FLG))
                                        {
                                            Store (Arg1, ^PR7F.FLG)
                                            If (LEqual (Arg1, One))
                                            {
                                                Notify (PR7F, One)
                                                Subtract (MSU, One, MSU)
                                            }
                                            Else
                                            {
                                                Notify (PR7F, 0x03)
                                                Add (MSU, One, MSU)
                                            }
                                        }
                                    }
                                    Else
                                ......

                                ......
                                Else
                                {
                                    If (And (Arg0, One))
                                    {
                                        If (LNotEqual (Arg1, ^PR01.FLG))
                                        {
                                            Store (Arg1, ^PR01.FLG)
                                            If (LEqual (Arg1, One))
                                            {
                                                Notify (PR01, One)
                                                Subtract (MSU, One, MSU)
                                            }
                                            Else
                                            {
                                                Notify (PR01, 0x03)
                                                Add (MSU, One, MSU)
                                            }
                                        }
                                    }
                                    Else
                                    {
                                        If (LNotEqual (Arg1, ^PR00.FLG))
                                        {
                                            Store (Arg1, ^PR00.FLG)
                                            If (LEqual (Arg1, One))
                                            {
                                                Notify (PR00, One)
                                                Subtract (MSU, One, MSU)
                                            }
                                            Else
                                            {
                                                Notify (PR00, 0x03)
                                                Add (MSU, One, MSU)
                                            }
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }

            Return (One)
        }
 
2. for method PRSC, dsdt code auto-generated by mk_dsdt.c is
        OperationRegion (PRST, SystemIO, 0xAF00, 0x20)
        Field (PRST, ByteAcc, NoLock, Preserve)
        {
            PRS,    256
        }

        Method (PRSC, 0, NotSerialized)
        {
            Store (PRS, Local3)
            Store (Zero, Local0)
            While (LLess (Local0, 0x20))
            {
                Store (Zero, Local1)
                Store (DerefOf (Index (Local3, Local0)), Local2)
                While (LLess (Local1, 0x08))
                {
                    NTFY (Add (Multiply (Local0, 0x08), Local1), And (Local2, 
                        One))
                    ShiftRight (Local2, One, Local2)
                    Increment (Local1)
                }

                Increment (Local0)
            }

            Return (One)
        }
   
3. so PRSC will scan vcpu from 0 to 255, the problem comes.
    for example, when we add vcpu3, PRSC will scan vcpu3, it will execute
          Notify (PR03, One)
          Subtract (MSU, One, MSU)
    until now, it's OK as expected, will add node /sys/devices/system/cpu3
    however, when PRSC continue scan vcpu 128+3, it will execute
          Notify (PR03, 0x03)
          Add (MSU, One, MSU)
    this is not we expected, will remove node /sys/devices/system/cpu3

4. the key issue comes from the scan loop of NTFY < scan loop of PRSC. That's 
the problem.

5. a simple way to solve the issue is, to make sure scan loop of NTFY = scan 
loop of PRSC, and to make sure NTFY always scan 2^n vcpus. 
    However, NTFY scan loop may change in the future, not necessary equal to 
2^n, and not necessary equal to scan loop of PRSC. That's the reason why I use 
for() loop inside of decision_tree() in method NTFY at the patch I send Jan 27. 
It will work correctly under whatever conditions, and keep mk_dsdt.c easier to 
understand. Decision_tree indeed reduce scan greatly, but it's not in key path.


Thanks,
Jinsong
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel