diff mbox

[v4,2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node

Message ID 033764b9-6098-1949-e4e7-a21cafcc6901@cn.fujitsu.com
State New
Headers show

Commit Message

Dou Liyang Aug. 24, 2017, 1:42 a.m. UTC
Hi Igor,

At 08/24/2017 01:47 AM, Igor Mammedov wrote:
> On Wed, 23 Aug 2017 21:35:29 +0800
> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>
>> Hi Igor,
>>
>> At 08/23/2017 08:45 PM, Igor Mammedov wrote:
>>> On Wed, 23 Aug 2017 20:12:51 +0800
>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>>>
>>>> Hi Igor,
>>>>
>>>> At 08/23/2017 04:40 PM, Igor Mammedov wrote:
>>>>> On Tue, 22 Aug 2017 11:24:10 +0800
>>>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
>>>>>
>>>>>> As QEMU supports the memory-less node, it is possible that there is
>>>>>> no RAM in the first numa node(also be called as node0). eg:
>>>>>>   ... \
>>>>>>   -m 128,slots=3,maxmem=1G \
>>>>>>   -numa node -numa node,mem=128M \
>>>>>>
>>>>>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
>>>>>> table. Only fixing it is not enough.
>>>>>>
>>>>>> Add a testcase for this situation to make sure the ACPI table is
>>>>>> correct for guest.
>>>>>>
>>>>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>>>>>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>>>>>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>>>>>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>>>>>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>>>>>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>>>>>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
>>>>>>  7 files changed, 30 insertions(+)
>>>>>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>>>>>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
>>>>>
>>>>>
>>>>> considering only SRAT table has been changed and the other
>>>>> tables match with default blobs, I'd suggest to keep only
>>>>
>>>>
>>>> Our testcase is:
>>>>
>>>> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
>>>> +                  " -numa node -numa node,mem=128"
>>>> +                  " -numa dist,src=0,dst=1,val=21",
>>>> +                  &data);
>>>>
>>>> The DSDT and SLIT don't match with default blobs.
>>> do you actually need SLIT table /i.e. -numa dist/ for test at all?
>>> it looks not relevant for the test case at the hand,
>>> I'd suggest to drop '-numa dist' option for the test.
>>>
>>
>> OK, Got it, will drop '-numa dist' option in next version.
>>
>>>>
>>>> So, they can't be dropped.
>>>
>>> I wonder what's changed, could you post DSDT diff here?
>>>
>>
>> Just like memory hot-plug cases, when we use the '-m 128
>> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
>> Memory Device in the DSDT table.
> for your case '-numa node -numa node,mem=128', there is no need in enabling memory hotplug.

Thank you very much for your kind explanation.

Yes, I understood.

> If I recall it correctly the default memory for x86 is 128Mb,
> hence removing "-m" would probably make DSDT match default one.

Yes, I removed the "-m":

test_acpi_one(" -numa node -numa node,mem=128", &data);

but, the DSDT didn't match the default one. because, if we support
NUMA, the DSDT will give us "_PXM" to map the CPU to node.


   *     OEM Revision     0x00000001 (1)
@@ -783,6 +783,8 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", 
"BXPCDSDT", 0x00000001)
                  {
                      COST (Zero, Arg0, Arg1, Arg2)
                  }
+
+                Name (_PXM, Zero)  // _PXM: Device Proximity
              }
          }
      }


Thanks,
	dou.
>

> [...]
>
>
>
>

Comments

Igor Mammedov Aug. 24, 2017, 12:33 p.m. UTC | #1
On Thu, 24 Aug 2017 09:42:28 +0800
Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:

> Hi Igor,
> 
> At 08/24/2017 01:47 AM, Igor Mammedov wrote:
> > On Wed, 23 Aug 2017 21:35:29 +0800
> > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >  
> >> Hi Igor,
> >>
> >> At 08/23/2017 08:45 PM, Igor Mammedov wrote:  
> >>> On Wed, 23 Aug 2017 20:12:51 +0800
> >>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >>>  
> >>>> Hi Igor,
> >>>>
> >>>> At 08/23/2017 04:40 PM, Igor Mammedov wrote:  
> >>>>> On Tue, 22 Aug 2017 11:24:10 +0800
> >>>>> Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> >>>>>  
> >>>>>> As QEMU supports the memory-less node, it is possible that there is
> >>>>>> no RAM in the first numa node(also be called as node0). eg:
> >>>>>>   ... \
> >>>>>>   -m 128,slots=3,maxmem=1G \
> >>>>>>   -numa node -numa node,mem=128M \
> >>>>>>
> >>>>>> But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> >>>>>> table. Only fixing it is not enough.
> >>>>>>
> >>>>>> Add a testcase for this situation to make sure the ACPI table is
> >>>>>> correct for guest.
> >>>>>>
> >>>>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> >>>>>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> >>>>>> ---
> >>>>>>  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
> >>>>>>  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
> >>>>>>  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
> >>>>>>  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
> >>>>>>  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
> >>>>>>  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
> >>>>>>  tests/bios-tables-test.c              |  30 ++++++++++++++++++++++++++++++
> >>>>>>  7 files changed, 30 insertions(+)
> >>>>>>  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
> >>>>>>  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem  
> >>>>>
> >>>>>
> >>>>> considering only SRAT table has been changed and the other
> >>>>> tables match with default blobs, I'd suggest to keep only  
> >>>>
> >>>>
> >>>> Our testcase is:
> >>>>
> >>>> +    test_acpi_one(" -m 128,slots=3,maxmem=1G"
> >>>> +                  " -numa node -numa node,mem=128"
> >>>> +                  " -numa dist,src=0,dst=1,val=21",
> >>>> +                  &data);
> >>>>
> >>>> The DSDT and SLIT don't match with default blobs.  
> >>> do you actually need SLIT table /i.e. -numa dist/ for test at all?
> >>> it looks not relevant for the test case at the hand,
> >>> I'd suggest to drop '-numa dist' option for the test.
> >>>  
> >>
> >> OK, Got it, will drop '-numa dist' option in next version.
> >>  
> >>>>
> >>>> So, they can't be dropped.  
> >>>
> >>> I wonder what's changed, could you post DSDT diff here?
> >>>  
> >>
> >> Just like memory hot-plug cases, when we use the '-m 128
> >> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
> >> Memory Device in the DSDT table.  
> > for your case '-numa node -numa node,mem=128', there is no need in enabling memory hotplug.  
> 
> Thank you very much for your kind explanation.
> 
> Yes, I understood.
> 
> > If I recall it correctly the default memory for x86 is 128Mb,
> > hence removing "-m" would probably make DSDT match default one.  
> 
> Yes, I removed the "-m":
> 
> test_acpi_one(" -numa node -numa node,mem=128", &data);
> 
> but, the DSDT didn't match the default one. because, if we support
> NUMA, the DSDT will give us "_PXM" to map the CPU to node.

Ok, looks like you'll have to include your variant of DSDT along with SRAT
Dou Liyang Aug. 24, 2017, 1:20 p.m. UTC | #2
Hi Igor,

At 08/24/2017 08:33 PM, Igor Mammedov wrote:
>> > test_acpi_one(" -numa node -numa node,mem=128", &data);
>> >
>> > but, the DSDT didn't match the default one. because, if we support
>> > NUMA, the DSDT will give us "_PXM" to map the CPU to node.
> Ok, looks like you'll have to include your variant of DSDT along with SRAT
>

Yes, got it.

I will modify and post this patch first.

Thanks,
	dou.

>


>
diff mbox

Patch

--- a/tmp/asl-7QO54Y.dsl
+++ b/tmp/asl-EWM54Y.dsl
@@ -5,13 +5,13 @@ 
   *
   * Disassembling to symbolic ASL+ operators
   *
- * Disassembly of tests/acpi-test-data/pc/DSDT, Thu Aug 24 09:20:29 2017
+ * Disassembly of /tmp/aml-1YM54Y, Thu Aug 24 09:20:29 2017
   *
   * Original Table Header:
   *     Signature        "DSDT"
- *     Length           0x000013EA (5098)
+ *     Length           0x000013F0 (5104)
   *     Revision         0x01 **** 32-bit table (V1), no 64-bit math 
support
- *     Checksum         0x78
+ *     Checksum         0x13
   *     OEM ID           "BOCHS "
   *     OEM Table ID     "BXPCDSDT"