accel/tcg/translate-all.c | 65 --------------------------------------- 1 file changed, 65 deletions(-)
Make bsd-user match linux-user in not marking host pages
as reserved. This isn't especially effective anyway, as
it doesn't take into account any heap memory that qemu
may allocate after startup.
Cc: Warner Losh <imp@bsdimp.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
I started to simply fix up this code to match my user-only interval-tree
patch set, as L1_MAP_ADDR_SPACE_BITS gets removed from translate-all.c,
but then I decided to remove it all.
r~
---
accel/tcg/translate-all.c | 65 ---------------------------------------
1 file changed, 65 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index b964ea44d7..48e9d70b4e 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -354,71 +354,6 @@ void page_init(void)
{
page_size_init();
page_table_config_init();
-
-#if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
- {
-#ifdef HAVE_KINFO_GETVMMAP
- struct kinfo_vmentry *freep;
- int i, cnt;
-
- freep = kinfo_getvmmap(getpid(), &cnt);
- if (freep) {
- mmap_lock();
- for (i = 0; i < cnt; i++) {
- unsigned long startaddr, endaddr;
-
- startaddr = freep[i].kve_start;
- endaddr = freep[i].kve_end;
- if (h2g_valid(startaddr)) {
- startaddr = h2g(startaddr) & TARGET_PAGE_MASK;
-
- if (h2g_valid(endaddr)) {
- endaddr = h2g(endaddr);
- page_set_flags(startaddr, endaddr, PAGE_RESERVED);
- } else {
-#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
- endaddr = ~0ul;
- page_set_flags(startaddr, endaddr, PAGE_RESERVED);
-#endif
- }
- }
- }
- free(freep);
- mmap_unlock();
- }
-#else
- FILE *f;
-
- last_brk = (unsigned long)sbrk(0);
-
- f = fopen("/compat/linux/proc/self/maps", "r");
- if (f) {
- mmap_lock();
-
- do {
- unsigned long startaddr, endaddr;
- int n;
-
- n = fscanf(f, "%lx-%lx %*[^\n]\n", &startaddr, &endaddr);
-
- if (n == 2 && h2g_valid(startaddr)) {
- startaddr = h2g(startaddr) & TARGET_PAGE_MASK;
-
- if (h2g_valid(endaddr)) {
- endaddr = h2g(endaddr);
- } else {
- endaddr = ~0ul;
- }
- page_set_flags(startaddr, endaddr, PAGE_RESERVED);
- }
- } while (!feof(f));
-
- fclose(f);
- mmap_unlock();
- }
-#endif
- }
-#endif
}
PageDesc *page_find_alloc(tb_page_addr_t index, bool alloc)
--
2.34.1
Richard Henderson <richard.henderson@linaro.org> writes: > Make bsd-user match linux-user in not marking host pages > as reserved. This isn't especially effective anyway, as > it doesn't take into account any heap memory that qemu > may allocate after startup. > > Cc: Warner Losh <imp@bsdimp.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> -- Alex Bennée Virtualisation Tech Lead @ Linaro
On Sat, Dec 17, 2022 at 11:48 AM Richard Henderson < richard.henderson@linaro.org> wrote: > Make bsd-user match linux-user in not marking host pages > as reserved. This isn't especially effective anyway, as > it doesn't take into account any heap memory that qemu > may allocate after startup. > > Cc: Warner Losh <imp@bsdimp.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > > I started to simply fix up this code to match my user-only interval-tree > patch set, as L1_MAP_ADDR_SPACE_BITS gets removed from translate-all.c, > but then I decided to remove it all. > I think this is fine. We already do a translation for addresses so marking this as 'reserved' doesn't help that much. We need to map memory into a contiguous guess-address-space, but the underlying host memory needn't be contiguous at all. I've not yet tested this, but would like to. What's your timeline on getting this done? Warner > r~ > > --- > accel/tcg/translate-all.c | 65 --------------------------------------- > 1 file changed, 65 deletions(-) > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index b964ea44d7..48e9d70b4e 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -354,71 +354,6 @@ void page_init(void) > { > page_size_init(); > page_table_config_init(); > - > -#if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY) > - { > -#ifdef HAVE_KINFO_GETVMMAP > - struct kinfo_vmentry *freep; > - int i, cnt; > - > - freep = kinfo_getvmmap(getpid(), &cnt); > - if (freep) { > - mmap_lock(); > - for (i = 0; i < cnt; i++) { > - unsigned long startaddr, endaddr; > - > - startaddr = freep[i].kve_start; > - endaddr = freep[i].kve_end; > - if (h2g_valid(startaddr)) { > - startaddr = h2g(startaddr) & TARGET_PAGE_MASK; > - > - if (h2g_valid(endaddr)) { > - endaddr = h2g(endaddr); > - page_set_flags(startaddr, endaddr, PAGE_RESERVED); > - } else { > -#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS > - endaddr = ~0ul; > - page_set_flags(startaddr, endaddr, PAGE_RESERVED); > -#endif > - } > - } > - } > - free(freep); > - mmap_unlock(); > - } > -#else > - FILE *f; > - > - last_brk = (unsigned long)sbrk(0); > - > - f = fopen("/compat/linux/proc/self/maps", "r"); > - if (f) { > - mmap_lock(); > - > - do { > - unsigned long startaddr, endaddr; > - int n; > - > - n = fscanf(f, "%lx-%lx %*[^\n]\n", &startaddr, &endaddr); > - > - if (n == 2 && h2g_valid(startaddr)) { > - startaddr = h2g(startaddr) & TARGET_PAGE_MASK; > - > - if (h2g_valid(endaddr)) { > - endaddr = h2g(endaddr); > - } else { > - endaddr = ~0ul; > - } > - page_set_flags(startaddr, endaddr, PAGE_RESERVED); > - } > - } while (!feof(f)); > - > - fclose(f); > - mmap_unlock(); > - } > -#endif > - } > -#endif > } > > PageDesc *page_find_alloc(tb_page_addr_t index, bool alloc) > -- > 2.34.1 > >
On 12/20/22 14:09, Warner Losh wrote: > > > On Sat, Dec 17, 2022 at 11:48 AM Richard Henderson <richard.henderson@linaro.org > <mailto:richard.henderson@linaro.org>> wrote: > > Make bsd-user match linux-user in not marking host pages > as reserved. This isn't especially effective anyway, as > it doesn't take into account any heap memory that qemu > may allocate after startup. > > Cc: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org > <mailto:richard.henderson@linaro.org>> > --- > > I started to simply fix up this code to match my user-only interval-tree > patch set, as L1_MAP_ADDR_SPACE_BITS gets removed from translate-all.c, > but then I decided to remove it all. > > > I think this is fine. We already do a translation for addresses so marking this as 'reserved' > doesn't help that much. We need to map memory into a contiguous guess-address-space, > but the underlying host memory needn't be contiguous at all. > > I've not yet tested this, but would like to. What's your timeline on getting this done? ASAP. I want to remove... > - if (h2g_valid(endaddr)) { > - endaddr = h2g(endaddr); > - page_set_flags(startaddr, endaddr, PAGE_RESERVED); > - } else { > -#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS L1_MAP_ADDR_SPACE_BITS. r~
On Tue, Dec 20, 2022 at 4:22 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 12/20/22 14:09, Warner Losh wrote: > > > > > > On Sat, Dec 17, 2022 at 11:48 AM Richard Henderson < > richard.henderson@linaro.org > > <mailto:richard.henderson@linaro.org>> wrote: > > > > Make bsd-user match linux-user in not marking host pages > > as reserved. This isn't especially effective anyway, as > > it doesn't take into account any heap memory that qemu > > may allocate after startup. > > > > Cc: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org > > <mailto:richard.henderson@linaro.org>> > > --- > > > > I started to simply fix up this code to match my user-only > interval-tree > > patch set, as L1_MAP_ADDR_SPACE_BITS gets removed from > translate-all.c, > > but then I decided to remove it all. > > > > > > I think this is fine. We already do a translation for addresses so > marking this as 'reserved' > > doesn't help that much. We need to map memory into a contiguous > guess-address-space, > > but the underlying host memory needn't be contiguous at all. > > > > I've not yet tested this, but would like to. What's your timeline on > getting this done? > > ASAP. I want to remove... > > > - if (h2g_valid(endaddr)) { > > - endaddr = h2g(endaddr); > > - page_set_flags(startaddr, endaddr, > PAGE_RESERVED); > > - } else { > > -#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS > > L1_MAP_ADDR_SPACE_BITS. > OK. I've tested this with both 32-bit and 64-bit binaries on a 64-bit host. It works both with the incomplete upstream as well as our 'blitz' branch which is basically complete. I've not run our full regression tests, though, but I suspect they will produce similar results before/after. My test machine is missing a few things due to an incomplete package upgrade that I don't have the time to sort out this evening. And looking at things, I agree with the analysis: It's a pesky nop. At worst, if it does change something, it's likely to change it for the better. And if not, I'll deal with that when I do my next round of upstreaming after the first of the year. So: Reviewed-by: Warner Losh <imp@bsdimp.com> Tested-by: Warner Losh <imp@bsdimp.com> Warner
On 12/17/22 10:48, Richard Henderson wrote: > Make bsd-user match linux-user in not marking host pages > as reserved. This isn't especially effective anyway, as > it doesn't take into account any heap memory that qemu > may allocate after startup. > > Cc: Warner Losh <imp@bsdimp.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > > I started to simply fix up this code to match my user-only interval-tree > patch set, as L1_MAP_ADDR_SPACE_BITS gets removed from translate-all.c, > but then I decided to remove it all. A further justification for this: The actual PAGE_RESERVED bit is write-only; there is no other reference to this bit elsewhere. r~ > > > r~ > > --- > accel/tcg/translate-all.c | 65 --------------------------------------- > 1 file changed, 65 deletions(-) > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index b964ea44d7..48e9d70b4e 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -354,71 +354,6 @@ void page_init(void) > { > page_size_init(); > page_table_config_init(); > - > -#if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY) > - { > -#ifdef HAVE_KINFO_GETVMMAP > - struct kinfo_vmentry *freep; > - int i, cnt; > - > - freep = kinfo_getvmmap(getpid(), &cnt); > - if (freep) { > - mmap_lock(); > - for (i = 0; i < cnt; i++) { > - unsigned long startaddr, endaddr; > - > - startaddr = freep[i].kve_start; > - endaddr = freep[i].kve_end; > - if (h2g_valid(startaddr)) { > - startaddr = h2g(startaddr) & TARGET_PAGE_MASK; > - > - if (h2g_valid(endaddr)) { > - endaddr = h2g(endaddr); > - page_set_flags(startaddr, endaddr, PAGE_RESERVED); > - } else { > -#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS > - endaddr = ~0ul; > - page_set_flags(startaddr, endaddr, PAGE_RESERVED); > -#endif > - } > - } > - } > - free(freep); > - mmap_unlock(); > - } > -#else > - FILE *f; > - > - last_brk = (unsigned long)sbrk(0); > - > - f = fopen("/compat/linux/proc/self/maps", "r"); > - if (f) { > - mmap_lock(); > - > - do { > - unsigned long startaddr, endaddr; > - int n; > - > - n = fscanf(f, "%lx-%lx %*[^\n]\n", &startaddr, &endaddr); > - > - if (n == 2 && h2g_valid(startaddr)) { > - startaddr = h2g(startaddr) & TARGET_PAGE_MASK; > - > - if (h2g_valid(endaddr)) { > - endaddr = h2g(endaddr); > - } else { > - endaddr = ~0ul; > - } > - page_set_flags(startaddr, endaddr, PAGE_RESERVED); > - } > - } while (!feof(f)); > - > - fclose(f); > - mmap_unlock(); > - } > -#endif > - } > -#endif > } > > PageDesc *page_find_alloc(tb_page_addr_t index, bool alloc)
On 20/12/22 20:31, Richard Henderson wrote: > On 12/17/22 10:48, Richard Henderson wrote: >> Make bsd-user match linux-user in not marking host pages >> as reserved. This isn't especially effective anyway, as >> it doesn't take into account any heap memory that qemu >> may allocate after startup. >> >> Cc: Warner Losh <imp@bsdimp.com> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> >> I started to simply fix up this code to match my user-only interval-tree >> patch set, as L1_MAP_ADDR_SPACE_BITS gets removed from translate-all.c, >> but then I decided to remove it all. > > A further justification for this: The actual PAGE_RESERVED bit is > write-only; there is no other reference to this bit elsewhere. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
© 2016 - 2024 Red Hat, Inc.