> On 02-Nov-2023, at 2:13 PM, David Hildenbrand <[email protected]> wrote:
> 
> On 01.11.23 02:29, Ani Sinha wrote:
>>> On 27-Oct-2023, at 4:12 PM, Ani Sinha <[email protected]> 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: [email protected]
>>> CC: David Hildenbrand <[email protected]>
>>> Signed-off-by: Ani Sinha <[email protected]>
>> 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 <[email protected]>
>>> +#
>>> +# 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,

Ah ok I see https://lwn.net/Articles/755423/  :-) 

> 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.

I went with this approach in v4/v5.

> 
>>> +        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


Reply via email to