* [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD
@ 2022-12-17 18:48 Richard Henderson
2022-12-20 19:31 ` Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Richard Henderson @ 2022-12-17 18:48 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Warner Losh
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD
2022-12-17 18:48 [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD Richard Henderson
@ 2022-12-20 19:31 ` Richard Henderson
2022-12-20 22:16 ` Philippe Mathieu-Daudé
2022-12-20 22:09 ` Warner Losh
2022-12-20 23:09 ` Alex Bennée
2 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2022-12-20 19:31 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Warner Losh
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)
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD
2022-12-20 19:31 ` Richard Henderson
@ 2022-12-20 22:16 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-20 22:16 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: peter.maydell, Warner Losh
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD
2022-12-17 18:48 [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD Richard Henderson
2022-12-20 19:31 ` Richard Henderson
@ 2022-12-20 22:09 ` Warner Losh
2022-12-20 23:22 ` Richard Henderson
2022-12-20 23:09 ` Alex Bennée
2 siblings, 1 reply; 7+ messages in thread
From: Warner Losh @ 2022-12-20 22:09 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, peter.maydell
[-- Attachment #1: Type: text/plain, Size: 3484 bytes --]
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
>
>
[-- Attachment #2: Type: text/html, Size: 4929 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD
2022-12-20 22:09 ` Warner Losh
@ 2022-12-20 23:22 ` Richard Henderson
2022-12-21 1:04 ` Warner Losh
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2022-12-20 23:22 UTC (permalink / raw)
To: Warner Losh; +Cc: qemu-devel, peter.maydell
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~
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD
2022-12-20 23:22 ` Richard Henderson
@ 2022-12-21 1:04 ` Warner Losh
0 siblings, 0 replies; 7+ messages in thread
From: Warner Losh @ 2022-12-21 1:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, peter.maydell
[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 3746 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD
2022-12-17 18:48 [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD Richard Henderson
2022-12-20 19:31 ` Richard Henderson
2022-12-20 22:09 ` Warner Losh
@ 2022-12-20 23:09 ` Alex Bennée
2 siblings, 0 replies; 7+ messages in thread
From: Alex Bennée @ 2022-12-20 23:09 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, Warner Losh, qemu-devel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-21 1:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-17 18:48 [PATCH] accel/tcg: Drop PAGE_RESERVED for CONFIG_BSD Richard Henderson
2022-12-20 19:31 ` Richard Henderson
2022-12-20 22:16 ` Philippe Mathieu-Daudé
2022-12-20 22:09 ` Warner Losh
2022-12-20 23:22 ` Richard Henderson
2022-12-21 1:04 ` Warner Losh
2022-12-20 23:09 ` Alex Bennée
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).