Re: [PATCH v3] tests/avocado: add test to exercize processor address space memory bound checks

[David Hildenbrand <david@redhat.com>]

On 01.11.23 02:29, Ani Sinha wrote:
> 
> 
>> On 27-Oct-2023, at 4:12 PM, Ani Sinha <anisinha@redhat.com> wrote:
>>
>> QEMU has validations to make sure that a VM is not started with more memory
>> (static and hotpluggable memory) than what the guest processor can address
>> directly with its addressing bits. This change adds a test to make sure QEMU
>> fails to start with a specific error message when an attempt is made to
>> start a VM with more memory than what the processor can directly address.
>> The test also checks for passing cases when the address space of the processor
>> is capable of addressing all memory. Boundary cases are tested.
>>
>> CC: imammedo@redhat.com
>> CC: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> 
> Gentle ping on this. David does these tests look good and cover all cases more or less?

Hi,

sorry, for some reason the patches never made it to my inbox.

> 
>> ---
>> tests/avocado/mem-addr-space-check.py | 331 ++++++++++++++++++++++++++
>> 1 file changed, 331 insertions(+)
>> create mode 100644 tests/avocado/mem-addr-space-check.py
>>
>> Changelog:
>> v3: added pae tests as well.
>> v2: added 64-bit tests. Added cxl tests.
>>
>> Sample run:
>> $ ./pyvenv/bin/avocado run tests/avocado/mem-addr-space-check.py --tap -
>> 1..14
>> ok 1 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_low_pse36
>> ok 2 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_low_pae
>> ok 3 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_ok_pentium_pse36
>> ok 4 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_ok_pentium_pae
>> ok 5 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_ok_pentium2
>> ok 6 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_low_nonpse36
>> ok 7 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_low_tcg_q35_70_amd
>> ok 8 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_low_tcg_q35_71_amd
>> ok 9 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_ok_tcg_q35_70_amd
>> ok 10 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_ok_tcg_q35_71_amd
>> ok 11 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_ok_tcg_q35_71_intel
>> ok 12 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_low_tcg_q35_71_amd_41bits
>> ok 13 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_low_tcg_q35_intel_cxl
>> ok 14 tests/avocado/mem-addr-space-check.py:MemAddrCheck.test_phybits_ok_tcg_q35_intel_cxl
>>
>> diff --git a/tests/avocado/mem-addr-space-check.py b/tests/avocado/mem-addr-space-check.py
>> new file mode 100644
>> index 0000000000..6ded11d658
>> --- /dev/null
>> +++ b/tests/avocado/mem-addr-space-check.py
>> @@ -0,0 +1,331 @@
>> +# Check for crash when using memory beyond the available guest processor
>> +# address space.
>> +#
>> +# Copyright (c) 2023 Red Hat, Inc.
>> +#
>> +# Author:
>> +#  Ani Sinha <anisinha@redhat.com>
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +from avocado_qemu import QemuSystemTest
>> +import signal
>> +
>> +class MemAddrCheck(QemuSystemTest):
>> +    # first, lets test some 32-bit processors.
>> +    # for all 32-bit cases, pci64_hole_size is 0.
>> +    def test_phybits_low_pse36(self):
>> +        """
>> +        :avocado: tags=machine:q35
>> +        :avocado: tags=arch:x86_64
>> +
>> +        With pse36 feature ON, a processor has 36 bits of addressing. So it can
>> +        access up to a maximum of 64GiB of memory. Memory hotplug region begins
>> +        at 4 GiB boundary when "above_4g_mem_size" is 0 (this would be true when
>> +        we have 0.5 GiB of VM memory, see pc_q35_init()). This means total
>> +        hotpluggable memory size is 60 GiB. Per slot, we reserve 1 GiB of memory
>> +        for dimm alignment for all newer machines (see enforce_aligned_dimm
>> +        property for pc machines and pc_get_device_memory_range()). That leaves
>> +        total hotpluggable actual memory size of 59 GiB. If the VM is started
>> +        with 0.5 GiB of memory, maxmem should be set to a maximum value of
>> +        59.5 GiB to ensure that the processor can address all memory directly.
>> +        Note that 64-bit pci hole size is 0 in this case. If maxmem is set to
>> +        59.6G, QEMU should fail to start with a message "phy-bits are too low".
>> +        If maxmem is set to 59.5G with all other QEMU parameters identical, QEMU
>> +        should start fine.
>> +        """
>> +        self.vm.add_args('-S', '-machine', 'q35', '-m',
>> +                         '512,slots=1,maxmem=59.6G',
>> +                         '-cpu', 'pentium,pse36=on', '-display', 'none',
>> +                         '-object', 'memory-backend-ram,id=mem1,size=1G',
>> +                         '-device', 'virtio-mem-pci,id=vm0,memdev=mem1')

Just a note that for virtio-mem, you don't even have to reserve an (ACPI NVDIMM/DIMM) slot.
So you might get away just setting "slots=0" and using maxmem=60.6G,
making it possible to simplify the comment(s) a bit (and all the other test cases).

Otherwise, maybe clearer use a DIMM instead of a virtio-mem device.

>> +        self.vm.set_qmp_monitor(enabled=False)
>> +        self.vm.launch()
>> +        self.vm.wait()
>> +        self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
>> +        self.assertRegex(self.vm.get_log(), r'phys-bits too low')
>> +
>> +    def test_phybits_low_pae(self):
>> +        """
>> +        :avocado: tags=machine:q35
>> +        :avocado: tags=arch:x86_64
>> +
>> +        With pae feature ON, a processor has 36 bits of addressing. So it can
>> +        access up to a maximum of 64GiB of memory. Rest is the same as the case
>> +        with pse36 above.
>> +        """
>> +        self.vm.add_args('-S', '-machine', 'q35', '-m',
>> +                         '512,slots=1,maxmem=59.6G',
>> +                         '-cpu', 'pentium,pae=on', '-display', 'none',
>> +                         '-object', 'memory-backend-ram,id=mem1,size=1G',
>> +                         '-device', 'virtio-mem-pci,id=vm0,memdev=mem1')

Same considerations.

>> +        self.vm.set_qmp_monitor(enabled=False)
>> +        self.vm.launch()
>> +        self.vm.wait()
>> +        self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
>> +        self.assertRegex(self.vm.get_log(), r'phys-bits too low')
>> +
>> +    def test_phybits_ok_pentium_pse36(self):
>> +        """
>> +        :avocado: tags=machine:q35
>> +        :avocado: tags=arch:x86_64
>> +
>> +        Setting maxmem to 59.5G and making sure that QEMU can start with the
>> +        same options as the failing case above with pse36 cpu feature.
>> +        """
>> +        self.vm.add_args('-machine', 'q35', '-m',
>> +                         '512,slots=1,maxmem=59.5G',
>> +                         '-cpu', 'pentium,pse36=on', '-display', 'none',
>> +                         '-object', 'memory-backend-ram,id=mem1,size=1G',
>> +                         '-device', 'virtio-mem-pci,id=vm0,memdev=mem1')
>> +        self.vm.set_qmp_monitor(enabled=False)
>> +        self.vm.launch()
>> +        self.vm.shutdown()
>> +        self.assertEquals(self.vm.exitcode(), -signal.SIGTERM,
>> +                          "QEMU did not terminate gracefully upon SIGTERM")
>> +
>> +    def test_phybits_ok_pentium_pae(self):
>> +        """
>> +        :avocado: tags=machine:q35
>> +        :avocado: tags=arch:x86_64
>> +
>> +        Test is same as above but now with pae CPUID turned on. Setting

s/CPUID/cpu feture/ ?

>> +        maxmem to 59.5G and making sure that QEMU can start fine with the
>> +        same options as the case above.
>> +        """
>> +        self.vm.add_args('-machine', 'q35', '-m',
>> +                         '512,slots=1,maxmem=59.5G',
>> +                         '-cpu', 'pentium,pae=on', '-display', 'none',
>> +                         '-object', 'memory-backend-ram,id=mem1,size=1G',
>> +                         '-device', 'virtio-mem-pci,id=vm0,memdev=mem1')
>> +        self.vm.set_qmp_monitor(enabled=False)
>> +        self.vm.launch()
>> +        self.vm.shutdown()
>> +        self.assertEquals(self.vm.exitcode(), -signal.SIGTERM,
>> +                          "QEMU did not terminate gracefully upon SIGTERM")
>> +
>> +    def test_phybits_ok_pentium2(self):
>> +        """
>> +        :avocado: tags=machine:q35
>> +        :avocado: tags=arch:x86_64
>> +
>> +        Pentium2 has 36 bits of addressing, so its same as pentium
>> +        with pse36 ON.
>> +        """
>> +        self.vm.add_args('-machine', 'q35', '-m',
>> +                         '512,slots=1,maxmem=59.5G',
>> +                         '-cpu', 'pentium2', '-display', 'none',
>> +                         '-object', 'memory-backend-ram,id=mem1,size=1G',
>> +                         '-device', 'virtio-mem-pci,id=vm0,memdev=mem1')
>> +        self.vm.set_qmp_monitor(enabled=False)
>> +        self.vm.launch()
>> +        self.vm.shutdown()
>> +        self.assertEquals(self.vm.exitcode(), -signal.SIGTERM,
>> +                          "QEMU did not terminate gracefully upon SIGTERM")
>> +
>> +    def test_phybits_low_nonpse36(self):
>> +        """
>> +        :avocado: tags=machine:q35
>> +        :avocado: tags=arch:x86_64
>> +
>> +        Pentium processor has 32 bits of addressing without pse36 or pae
>> +        so it can access up to 4 GiB of memory. Setting maxmem to 4GiB

"access physical addresses up to 4 GiB"

>> +        should make QEMU fail to start with "phys-bits too low" message.

"because the region for memory hotplug is always placed above 4 GiB due to the
PCI hole and simplicity."

>> +        """
>> +        self.vm.add_args('-S', '-machine', 'q35', '-m',
>> +                         '512,slots=1,maxmem=4G',
>> +                         '-cpu', 'pentium', '-display', 'none',
>> +                         '-object', 'memory-backend-ram,id=mem1,size=1G',
>> +                         '-device', 'virtio-mem-pci,id=vm0,memdev=mem1')
>> +        self.vm.set_qmp_monitor(enabled=False)
>> +        self.vm.launch()
>> +        self.vm.wait()
>> +        self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
>> +        self.assertRegex(self.vm.get_log(), r'phys-bits too low')
>> +
>> +    # now lets test some 64-bit CPU cases.
>> +    def test_phybits_low_tcg_q35_70_amd(self):
>> +        """
>> +        :avocado: tags=machine:q35
>> +        :avocado: tags=arch:x86_64
>> +
>> +        For q35 7.1 machines and above, "above_4G" memory starts at 1 TiB
>> +        boundary for AMD cpus (default). Lets test without that case.

^ first time I hear about that. Weird AMD-specific stuff, really.

     /*
      * The HyperTransport range close to the 1T boundary is unique to AMD
      * hosts with IOMMUs enabled. Restrict the ram-above-4g relocation
      * to above 1T to AMD vCPUs only. @enforce_amd_1tb_hole is only false in
      * older machine types (<= 7.0) for compatibility purposes.
      */
     if (IS_AMD_CPU(&cpu->env) && pcmc->enforce_amd_1tb_hole) {
         /* Bail out if max possible address does not cross HT range */
         if (pc_max_used_gpa(pcms, pci_hole64_size) >= AMD_HT_START) {
             x86ms->above_4g_mem_start = AMD_ABOVE_1TB_START;

So there is a fixed memory hole that starts at 0xfd00000000UL. If our max GPA would
overlap that region, we'll place everything above that memory hole.


         }

         /*
          * Advertise the HT region if address space covers the reserved
          * region or if we relocate.
          */
         if (cpu->phys_bits >= 40) {
             e820_add_entry(AMD_HT_START, AMD_HT_SIZE, E820_RESERVED);
         }
     }

I don't think your desciption is quite correct for that case. "above_4G" only starts above
1 TiB if it would overlap that special memory hole starting at 0xfd00000000UL.


-- 
Cheers,

David / dhildenb