> Am 09.03.2021 um 21:04 schrieb Peter Xu <pet...@redhat.com>: > > On Mon, Mar 08, 2021 at 04:05:57PM +0100, David Hildenbrand wrote: >> Let's introduce a new set of flags that abstract mmap logic and replace >> our current set of bools, to prepare for another flag. >> >> Signed-off-by: David Hildenbrand <da...@redhat.com> >> --- >> include/qemu/mmap-alloc.h | 17 +++++++++++------ >> softmmu/physmem.c | 8 +++++--- >> util/mmap-alloc.c | 14 +++++++------- >> util/oslib-posix.c | 3 ++- >> 4 files changed, 25 insertions(+), 17 deletions(-) >> >> diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h >> index 456ff87df1..55664ea9f3 100644 >> --- a/include/qemu/mmap-alloc.h >> +++ b/include/qemu/mmap-alloc.h >> @@ -6,6 +6,15 @@ size_t qemu_fd_getpagesize(int fd); >> >> size_t qemu_mempath_getpagesize(const char *mem_path); >> >> +/* Map PROT_READ instead of PROT_READ|PROT_WRITE. */ >> +#define QEMU_RAM_MMAP_READONLY (1 << 0) >> + >> +/* Map MAP_SHARED instead of MAP_PRIVATE. */ >> +#define QEMU_RAM_MMAP_SHARED (1 << 1) >> + >> +/* Map MAP_SYNC|MAP_SHARED_VALIDATE if possible, fallback and warn >> otherwise. */ >> +#define QEMU_RAM_MMAP_PMEM (1 << 2) > > Sorry to speak late - I just noticed that is_pmem can actually be converted > too > with "MAP_SYNC | MAP_SHARED_VALIDATE". We can even define MAP_PMEM_EXTRA for > use within qemu if we want. Then we can avoid one layer of QEMU_RAM_* by > directly using MAP_*, I think? >
No problem :) I don‘t think passing in random MAP_ flags is a good interface (we would at least need an allow list). I like the abstraction / explicit semenatics of QEMU_RAM_MMAP_PMEM as spelled out in the comment. Doing the fallback when passing in the mmap flags is a little ugly. We could do the fallback in the caller, I think I remember there is only a single call site. PROT_READ won‘t be covered as well, not sure if passing in protections improves the interface. Long story short, I like the abstraction provided by these flags, only exporting what we actually support/abstracting it, and setting some MAP_ flags automatically (MAP_PRIVATE, MAP_ANON) instead of having to spell that put in the caller. > -- > Peter Xu >