Re: [PATCH 1/2] log: Add separate debug option for logging invalid memory accesses

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 13/2/23 15:58, Philippe Mathieu-Daudé wrote:
> On 13/2/23 15:47, BALATON Zoltan wrote:
>> On Mon, 13 Feb 2023, Peter Xu wrote:
>>> On Mon, Feb 13, 2023 at 12:41:29PM +0100, Thomas Huth wrote:
>>>> On 07/02/2023 17.33, BALATON Zoltan wrote:
>>>>> On Tue, 31 Jan 2023, BALATON Zoltan wrote:
>>>>>> On Thu, 19 Jan 2023, BALATON Zoltan wrote:
>>>>>>> Currently -d guest_errors enables logging of different invalid 
>>>>>>> actions
>>>>>>> by the guest such as misusing hardware, accessing missing 
>>>>>>> features or
>>>>>>> invalid memory areas.

- misusing hardware => LOG_GUEST_ERROR
- accessing missing features => LOG_UNIMP
- invalid memory areas => depending on the bus bridge, can be:

   #define MEMTX_OK              0
   #define MEMTX_ERROR          (1U << 0) /* device returned an error */
   #define MEMTX_DECODE_ERROR   (1U << 1) /* nothing at that address */
   #define MEMTX_ACCESS_ERROR   (1U << 2) /* access denied */

>>>>>>> The memory access logging can be quite verbose
>>>>>>> which obscures the other messages enabled by this debug switch so
>>>>>>> separate it by adding a new -d memaccess option to make it 
>>>>>>> possible to
>>>>>>> control it independently of other guest error logs.
>>>>>>>
>>>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>>>
>>>>>> Ping? Could somebody review and pick it up please?
>>>>>
>>>>> Ping?
>>>>
>>>> Patch makes sense to me and looks fine, so:
>>>>
>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>>
>>>> ... I think this should go via one of the "Memory API" maintainers 
>>>> branches?
>>>> Paolo? Peter? David?
>>>
>>> Paolo normally does the pull, I assume that'll still be the case.  The
>>> patch looks good to me if Phil's comment will be addressed on merging 
>>> with
>>> the old mask, which makes sense to me:
>>
>> Keeping the old mask kind of defies the purpose. I've tried to explain 
>> that in the commit message

It is hard to justify we need a new mask and scripts have to adapt to
your new format. I assume you can not use trace-event because you want
this logging available in a RELEASE build, right?

>> but now that two of you did not get it 
>> maybe that message needs to be clarified instead?
> 
> Is your use case memaccess enabled and guest-errors disabled?