Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-16 Thread Laurent Vivier 

Le 16/08/2022 à 10:41, Alex Bennée a écrit :


Laurent Vivier  writes:


Le 11/08/2022 à 17:18, Alex Bennée a écrit :

Laurent Vivier  writes:


Le 11/08/2022 à 13:54, Peter Maydell a écrit :

On Thu, 11 Aug 2022 at 09:29, Laurent Vivier  wrote:


Le 10/08/2022 à 22:47, Richard Henderson a écrit :

On 8/10/22 13:32, Vitaly Buka wrote:

Sorry, I only noticed today that it's not submitted.
Version is not critical for us, as we build from masters anyway.
Richard, do you know a reason to consider this critical?

On Wed, 10 Aug 2022 at 13:04, Peter Maydell mailto:peter.mayd...@linaro.org>> wrote:

   On Wed, 10 Aug 2022 at 21:00, Vitaly Buka mailto:vitalyb...@google.com>> wrote:
>
> How can we land this one?

   Pinging it a week ago rather than now would have been a good start :-(
   I think it got missed because you didn't cc the linux-user maintainer.

   Is this a critical fix for 7.1 or can we let it slip to 7.2 ?


It's unfortunate that it got missed.  It's not critical, but it would be nice, 
because support for
MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).

I'll note there are missing braces for coding style on an IF.

Laurent, do you have an objection to merging this for rc3?



No objection.

Do you want it goes via the arm branch or via the linux-user branch?

If it goes via linux-user I can run the LTP testsuite but it takes 1 day.

I think we should definitely run the LTP testsuite on it, so
taking it via linux-user probably makes more sense.


ok, applied to my linux-user-for-7.1 branch.

Running tests.

Any chance you could pick up:
Subject: [PATCH v2] linux-user: un-parent OBJECT(cpu) when
closing thread
Date: Wed,  3 Aug 2022 14:05:37 +0100
Message-Id: <20220803130537.763666-1-alex.ben...@linaro.org>
before you run the tests?



I've tested it, it works fine.

Do you plan to do a PR including it or do you want I do (there will be
only this one in mine)?


I'm going to a roll a PR today so I can include it. Shall I add a
Tested-by for you?



OK. No need to add the Tested-by, I've run generic tests, not targeted to this 
problem.

Thanks,
Laurent

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-16 Thread Alex Bennée 


Laurent Vivier  writes:

> Le 11/08/2022 à 17:18, Alex Bennée a écrit :
>> Laurent Vivier  writes:
>> 
>>> Le 11/08/2022 à 13:54, Peter Maydell a écrit :
 On Thu, 11 Aug 2022 at 09:29, Laurent Vivier  wrote:
>
> Le 10/08/2022 à 22:47, Richard Henderson a écrit :
>> On 8/10/22 13:32, Vitaly Buka wrote:
>>> Sorry, I only noticed today that it's not submitted.
>>> Version is not critical for us, as we build from masters anyway.
>>> Richard, do you know a reason to consider this critical?
>>>
>>> On Wed, 10 Aug 2022 at 13:04, Peter Maydell >> > wrote:
>>>
>>>   On Wed, 10 Aug 2022 at 21:00, Vitaly Buka >>   > wrote:
>>>>
>>>> How can we land this one?
>>>
>>>   Pinging it a week ago rather than now would have been a good 
>>> start :-(
>>>   I think it got missed because you didn't cc the linux-user 
>>> maintainer.
>>>
>>>   Is this a critical fix for 7.1 or can we let it slip to 7.2 ?
>>
>> It's unfortunate that it got missed.  It's not critical, but it would be 
>> nice, because support for
>> MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).
>>
>> I'll note there are missing braces for coding style on an IF.
>>
>> Laurent, do you have an objection to merging this for rc3?
>>
>
> No objection.
>
> Do you want it goes via the arm branch or via the linux-user branch?
>
> If it goes via linux-user I can run the LTP testsuite but it takes 1 day.
 I think we should definitely run the LTP testsuite on it, so
 taking it via linux-user probably makes more sense.
>>>
>>> ok, applied to my linux-user-for-7.1 branch.
>>>
>>> Running tests.
>> Any chance you could pick up:
>>Subject: [PATCH v2] linux-user: un-parent OBJECT(cpu) when
>> closing thread
>>Date: Wed,  3 Aug 2022 14:05:37 +0100
>>Message-Id: <20220803130537.763666-1-alex.ben...@linaro.org>
>> before you run the tests?
>> 
>
> I've tested it, it works fine.
>
> Do you plan to do a PR including it or do you want I do (there will be
> only this one in mine)?

I'm going to a roll a PR today so I can include it. Shall I add a
Tested-by for you?

>
> Thanks,
> Laurent


-- 
Alex Bennée

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-13 Thread Laurent Vivier 

Le 11/08/2022 à 17:18, Alex Bennée a écrit :


Laurent Vivier  writes:


Le 11/08/2022 à 13:54, Peter Maydell a écrit :

On Thu, 11 Aug 2022 at 09:29, Laurent Vivier  wrote:


Le 10/08/2022 à 22:47, Richard Henderson a écrit :

On 8/10/22 13:32, Vitaly Buka wrote:

Sorry, I only noticed today that it's not submitted.
Version is not critical for us, as we build from masters anyway.
Richard, do you know a reason to consider this critical?

On Wed, 10 Aug 2022 at 13:04, Peter Maydell mailto:peter.mayd...@linaro.org>> wrote:

  On Wed, 10 Aug 2022 at 21:00, Vitaly Buka mailto:vitalyb...@google.com>> wrote:
   >
   > How can we land this one?

  Pinging it a week ago rather than now would have been a good start :-(
  I think it got missed because you didn't cc the linux-user maintainer.

  Is this a critical fix for 7.1 or can we let it slip to 7.2 ?


It's unfortunate that it got missed.  It's not critical, but it would be nice, 
because support for
MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).

I'll note there are missing braces for coding style on an IF.

Laurent, do you have an objection to merging this for rc3?



No objection.

Do you want it goes via the arm branch or via the linux-user branch?

If it goes via linux-user I can run the LTP testsuite but it takes 1 day.

I think we should definitely run the LTP testsuite on it, so
taking it via linux-user probably makes more sense.


ok, applied to my linux-user-for-7.1 branch.

Running tests.


Any chance you could pick up:

   Subject: [PATCH v2] linux-user: un-parent OBJECT(cpu) when closing thread
   Date: Wed,  3 Aug 2022 14:05:37 +0100
   Message-Id: <20220803130537.763666-1-alex.ben...@linaro.org>

before you run the tests?



I've tested it, it works fine.

Do you plan to do a PR including it or do you want I do (there will be only 
this one in mine)?

Thanks,
Laurent

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-11 Thread Alex Bennée 


Laurent Vivier  writes:

> Le 11/08/2022 à 13:54, Peter Maydell a écrit :
>> On Thu, 11 Aug 2022 at 09:29, Laurent Vivier  wrote:
>>>
>>> Le 10/08/2022 à 22:47, Richard Henderson a écrit :
 On 8/10/22 13:32, Vitaly Buka wrote:
> Sorry, I only noticed today that it's not submitted.
> Version is not critical for us, as we build from masters anyway.
> Richard, do you know a reason to consider this critical?
>
> On Wed, 10 Aug 2022 at 13:04, Peter Maydell  > wrote:
>
>  On Wed, 10 Aug 2022 at 21:00, Vitaly Buka   > wrote:
>   >
>   > How can we land this one?
>
>  Pinging it a week ago rather than now would have been a good start 
> :-(
>  I think it got missed because you didn't cc the linux-user 
> maintainer.
>
>  Is this a critical fix for 7.1 or can we let it slip to 7.2 ?

 It's unfortunate that it got missed.  It's not critical, but it would be 
 nice, because support for
 MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).

 I'll note there are missing braces for coding style on an IF.

 Laurent, do you have an objection to merging this for rc3?

>>>
>>> No objection.
>>>
>>> Do you want it goes via the arm branch or via the linux-user branch?
>>>
>>> If it goes via linux-user I can run the LTP testsuite but it takes 1 day.
>> I think we should definitely run the LTP testsuite on it, so
>> taking it via linux-user probably makes more sense.
>
> ok, applied to my linux-user-for-7.1 branch.
>
> Running tests.

Any chance you could pick up:

  Subject: [PATCH v2] linux-user: un-parent OBJECT(cpu) when closing thread
  Date: Wed,  3 Aug 2022 14:05:37 +0100
  Message-Id: <20220803130537.763666-1-alex.ben...@linaro.org>

before you run the tests?

>
> Thanks,
> Laurent


-- 
Alex Bennée

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-11 Thread Laurent Vivier 

Le 11/08/2022 à 13:54, Peter Maydell a écrit :

On Thu, 11 Aug 2022 at 09:29, Laurent Vivier  wrote:


Le 10/08/2022 à 22:47, Richard Henderson a écrit :

On 8/10/22 13:32, Vitaly Buka wrote:

Sorry, I only noticed today that it's not submitted.
Version is not critical for us, as we build from masters anyway.
Richard, do you know a reason to consider this critical?

On Wed, 10 Aug 2022 at 13:04, Peter Maydell mailto:peter.mayd...@linaro.org>> wrote:

 On Wed, 10 Aug 2022 at 21:00, Vitaly Buka mailto:vitalyb...@google.com>> wrote:
  >
  > How can we land this one?

 Pinging it a week ago rather than now would have been a good start :-(
 I think it got missed because you didn't cc the linux-user maintainer.

 Is this a critical fix for 7.1 or can we let it slip to 7.2 ?


It's unfortunate that it got missed.  It's not critical, but it would be nice, 
because support for
MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).

I'll note there are missing braces for coding style on an IF.

Laurent, do you have an objection to merging this for rc3?



No objection.

Do you want it goes via the arm branch or via the linux-user branch?

If it goes via linux-user I can run the LTP testsuite but it takes 1 day.


I think we should definitely run the LTP testsuite on it, so
taking it via linux-user probably makes more sense.


ok, applied to my linux-user-for-7.1 branch.

Running tests.

Thanks,
Laurent

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-11 Thread Peter Maydell 
On Thu, 11 Aug 2022 at 09:29, Laurent Vivier  wrote:
>
> Le 10/08/2022 à 22:47, Richard Henderson a écrit :
> > On 8/10/22 13:32, Vitaly Buka wrote:
> >> Sorry, I only noticed today that it's not submitted.
> >> Version is not critical for us, as we build from masters anyway.
> >> Richard, do you know a reason to consider this critical?
> >>
> >> On Wed, 10 Aug 2022 at 13:04, Peter Maydell  >> > wrote:
> >>
> >> On Wed, 10 Aug 2022 at 21:00, Vitaly Buka  >> > wrote:
> >>  >
> >>  > How can we land this one?
> >>
> >> Pinging it a week ago rather than now would have been a good start :-(
> >> I think it got missed because you didn't cc the linux-user maintainer.
> >>
> >> Is this a critical fix for 7.1 or can we let it slip to 7.2 ?
> >
> > It's unfortunate that it got missed.  It's not critical, but it would be 
> > nice, because support for
> > MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).
> >
> > I'll note there are missing braces for coding style on an IF.
> >
> > Laurent, do you have an objection to merging this for rc3?
> >
>
> No objection.
>
> Do you want it goes via the arm branch or via the linux-user branch?
>
> If it goes via linux-user I can run the LTP testsuite but it takes 1 day.

I think we should definitely run the LTP testsuite on it, so
taking it via linux-user probably makes more sense.

thanks
-- PMM

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-11 Thread Laurent Vivier 

Le 10/08/2022 à 22:47, Richard Henderson a écrit :

On 8/10/22 13:32, Vitaly Buka wrote:

Sorry, I only noticed today that it's not submitted.
Version is not critical for us, as we build from masters anyway.
Richard, do you know a reason to consider this critical?

On Wed, 10 Aug 2022 at 13:04, Peter Maydell > wrote:


    On Wed, 10 Aug 2022 at 21:00, Vitaly Buka mailto:vitalyb...@google.com>> wrote:
 >
 > How can we land this one?

    Pinging it a week ago rather than now would have been a good start :-(
    I think it got missed because you didn't cc the linux-user maintainer.

    Is this a critical fix for 7.1 or can we let it slip to 7.2 ?


It's unfortunate that it got missed.  It's not critical, but it would be nice, because support for 
MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).


I'll note there are missing braces for coding style on an IF.

Laurent, do you have an objection to merging this for rc3?



No objection.

Do you want it goes via the arm branch or via the linux-user branch?

If it goes via linux-user I can run the LTP testsuite but it takes 1 day.

Thanks,
Laurent

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-10 Thread Richard Henderson 

On 8/10/22 13:32, Vitaly Buka wrote:

Sorry, I only noticed today that it's not submitted.
Version is not critical for us, as we build from masters anyway.
Richard, do you know a reason to consider this critical?

On Wed, 10 Aug 2022 at 13:04, Peter Maydell > wrote:


On Wed, 10 Aug 2022 at 21:00, Vitaly Buka mailto:vitalyb...@google.com>> wrote:
 >
 > How can we land this one?

Pinging it a week ago rather than now would have been a good start :-(
I think it got missed because you didn't cc the linux-user maintainer.

Is this a critical fix for 7.1 or can we let it slip to 7.2 ?


It's unfortunate that it got missed.  It's not critical, but it would be nice, because 
support for MADV_DONTNEED is new in 7.1 (previously, we ignored all madvise).


I'll note there are missing braces for coding style on an IF.

Laurent, do you have an objection to merging this for rc3?


r~

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-10 Thread Vitaly Buka 
Sorry, I only noticed today that it's not submitted.
Version is not critical for us, as we build from masters anyway.
Richard, do you know a reason to consider this critical?

On Wed, 10 Aug 2022 at 13:04, Peter Maydell 
wrote:

> On Wed, 10 Aug 2022 at 21:00, Vitaly Buka  wrote:
> >
> > How can we land this one?
>
> Pinging it a week ago rather than now would have been a good start :-(
> I think it got missed because you didn't cc the linux-user maintainer.
>
> Is this a critical fix for 7.1 or can we let it slip to 7.2 ?
>
> thanks
> -- PMM
>

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-10 Thread Peter Maydell 
On Wed, 10 Aug 2022 at 21:00, Vitaly Buka  wrote:
>
> How can we land this one?

Pinging it a week ago rather than now would have been a good start :-(
I think it got missed because you didn't cc the linux-user maintainer.

Is this a critical fix for 7.1 or can we let it slip to 7.2 ?

thanks
-- PMM

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-08-10 Thread Vitaly Buka 
How can we land this one?

Re: [PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-07-11 Thread Richard Henderson 

On 7/12/22 03:30, Vitaly Buka wrote:

aarch64 stores MTE tags in target_date, and they should be reset by
MADV_DONTNEED.

Signed-off-by: Vitaly Buka 
---
  accel/tcg/translate-all.c | 24 
  include/exec/cpu-all.h|  1 +
  linux-user/mmap.c |  2 ++
  3 files changed, 27 insertions(+)


Reviewed-by: Richard Henderson 

r~



diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ef62a199c7..d6f2f1a40a 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2314,6 +2314,30 @@ void page_set_flags(target_ulong start, target_ulong 
end, int flags)
  }
  }
  
+void page_reset_target_data(target_ulong start, target_ulong end)

+{
+target_ulong addr, len;
+
+/* This function should never be called with addresses outside the
+   guest address space.  If this assert fires, it probably indicates
+   a missing call to h2g_valid.  */
+assert(end - 1 <= GUEST_ADDR_MAX);
+assert(start < end);
+assert_memory_lock();
+
+start = start & TARGET_PAGE_MASK;
+end = TARGET_PAGE_ALIGN(end);
+
+for (addr = start, len = end - start;
+ len != 0;
+ len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
+PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
+
+g_free(p->target_data);
+p->target_data = NULL;
+}
+}
+
  void *page_get_target_data(target_ulong address)
  {
  PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f5bda2c3ca..491629b9ba 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -271,6 +271,7 @@ int walk_memory_regions(void *, walk_memory_regions_fn);
  
  int page_get_flags(target_ulong address);

  void page_set_flags(target_ulong start, target_ulong end, int flags);
+void page_reset_target_data(target_ulong start, target_ulong end);
  int page_check_range(target_ulong start, target_ulong len, int flags);
  
  /**

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 4e7a6be6ee..c535dfdc7c 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -894,6 +894,8 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, 
int advice)
  if ((advice & MADV_DONTNEED) &&
  can_passthrough_madv_dontneed(start, end)) {
  ret = get_errno(madvise(g2h_untagged(start), len, MADV_DONTNEED));
+if (ret == 0)
+page_reset_target_data(start, start + len);
  }
  mmap_unlock();

[PATCH] [PATCH] linux-user/aarch64: Reset target data on MADV_DONTNEED

2022-07-11 Thread Vitaly Buka 
aarch64 stores MTE tags in target_date, and they should be reset by
MADV_DONTNEED.

Signed-off-by: Vitaly Buka 
---
 accel/tcg/translate-all.c | 24 
 include/exec/cpu-all.h|  1 +
 linux-user/mmap.c |  2 ++
 3 files changed, 27 insertions(+)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ef62a199c7..d6f2f1a40a 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2314,6 +2314,30 @@ void page_set_flags(target_ulong start, target_ulong 
end, int flags)
 }
 }
 
+void page_reset_target_data(target_ulong start, target_ulong end)
+{
+target_ulong addr, len;
+
+/* This function should never be called with addresses outside the
+   guest address space.  If this assert fires, it probably indicates
+   a missing call to h2g_valid.  */
+assert(end - 1 <= GUEST_ADDR_MAX);
+assert(start < end);
+assert_memory_lock();
+
+start = start & TARGET_PAGE_MASK;
+end = TARGET_PAGE_ALIGN(end);
+
+for (addr = start, len = end - start;
+ len != 0;
+ len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
+PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
+
+g_free(p->target_data);
+p->target_data = NULL;
+}
+}
+
 void *page_get_target_data(target_ulong address)
 {
 PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f5bda2c3ca..491629b9ba 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -271,6 +271,7 @@ int walk_memory_regions(void *, walk_memory_regions_fn);
 
 int page_get_flags(target_ulong address);
 void page_set_flags(target_ulong start, target_ulong end, int flags);
+void page_reset_target_data(target_ulong start, target_ulong end);
 int page_check_range(target_ulong start, target_ulong len, int flags);
 
 /**
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 4e7a6be6ee..c535dfdc7c 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -894,6 +894,8 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, 
int advice)
 if ((advice & MADV_DONTNEED) &&
 can_passthrough_madv_dontneed(start, end)) {
 ret = get_errno(madvise(g2h_untagged(start), len, MADV_DONTNEED));
+if (ret == 0)
+page_reset_target_data(start, start + len);
 }
 mmap_unlock();
 
-- 
2.37.0.144.g8ac04bfd2-goog