Re: [Qemu-devel] [PATCH 2/2] hw/pci-host/x86: extend the 64-bit PCI hole relative to the fw-assigned base

[Laszlo Ersek]

(+Eric)

On 09/26/18 14:10, Igor Mammedov wrote:
> On Wed, 26 Sep 2018 13:35:14 +0200
> Laszlo Ersek <address@hidden> wrote:
> 
>> On 09/26/18 13:12, Laszlo Ersek wrote:
>>
>>> (4) append a new patch, for "bios-tables-test", so that the ACPI gen
>>> change is validated as part of the test suite, on SeaBIOS/q35.
>>>
>>> Regarding (4):
>>>
>>> - is it OK if I add the test only for Q35?
>>>
>>> - what guest RAM size am I allowed to use in the test suite? In my own
>>> SeaBIOS/Q35 reproducer I currently use 5GB, but I'm not sure if it's
>>> acceptable for the test suite.  
>>
>> And, even if the patch's effect can be shown with little guest DRAM, the
>> test case still requires a multi-gig ivshmem-plain device. In
>> "tests/ivshmem-test.c", I see how it is set up -- the backend is set up
>> with shm_open(). The file created under /dev/shm (on Linux) might
>> require host RAM just the same as normal guest DRAM (especially with
>> memory overcommit disabled on the host), correct?
> with over commit disable or cgroups limits enforced (I'd expect that
> in automated testing env i.e. travis or something else)
> allocating such amount of RAM probably would fail like crazy.
> 
> Maybe using memdev file backend with manually created sparse file
> might actually work (with preallocate disabled)

Thanks, this sounds like a good idea.

I see shm_open() is used heavily in ivshmem-related tests. I haven't
looked much at shm_open() before. (I've always known it existed in
POSIX, but I've never cared.)

So now I first checked what shm_open() would give me over a regular
temporary file created with open(); after all, the file descriptor
returned by either would have to be mmap()'d. From the rationale in POSIX:

<http://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html#tag_22_02_08_14>,

it seems like the {shm_open(), mmap()} combo has two significant
guarantees over {open(), mmap()}:

- the namespace may be distinct (there need not be a writeable
  filesystem at all),

- the shared object will *always* be locked in RAM ("Shared memory is
  not just simply providing common access to data, it is providing the
  fastest possible communication between the processes").

The rationale seems to permit, on purpose, an shm_open() implementation
that is actually based on open(), using a special file system -- and
AIUI, /dev/shm is just that, on Linux.

Eric, does the above sound more or less correct?

If it is correct, then I think shm_open() is exactly what I *don't* want
for this use case. Because, while I do need a pathname for an
mmap()-able object (regular file, or otherwise), just so I can do:

  -object memory-backend-file,id=mem-obj,...,mem-path=... \
  -device ivshmem-plain,memdev=mem-obj,...

, I want the underlying object to put as little pressure on the system
that runs the test suite as possible.

This means I should specifically ask for a regular file, to be mmap()'d
(with MAP_SHARED). Then the kernel knows in advance that it can always
page out the dirty stuff, and the mapping shouldn't clash with cgroups,
or disabled memory overcommit.

Now, in order to make that actually safe, I should in theory ask for
preallocation on the filesystem (otherwise, if the filesystem runs out
of space, while the kernel is allocating fs extents in order to flush
the dirty pages to them, the process gets a SIGBUS, IIRC). However,
because I know that nothing will be in fact dirtied, I can minimize the
footprint on the filesystem as well, and forego preallocation too.

This suggests that, in my test case,
- I call g_file_open_tmp() for creating the temporary file,
- pass the returned fd to ftruncate() for resizing the temporary file,
- pass the returned pathname to the "memory-backend-file" objects, in
  the "mem-path" property,
- set "share=on",
- set "prealloc=off",
- "discard-data" is irrelevant (there won't be any dirty pages).

Thanks
Laszlo