[PATCH v3 1/1] hmp: synchronize cpu state for lapic info

[PATCH v3 1/1] hmp: synchronize cpu state for lapic info

[Dongli Zhang]

While the default "info lapic" always synchronizes cpu state ...

mon_get_cpu()
-> mon_get_cpu_sync(mon, true)
   -> cpu_synchronize_state(cpu)
      -> ioctl KVM_GET_LAPIC (taking KVM as example)

... the cpu state is not synchronized when the apic-id is available as
argument.

The cpu state should be synchronized when apic-id is available. Otherwise
the "info lapic <apic-id>" always returns stale data.

Reference:
https://lore.kernel.org/all/20211028155457.967291-19-berrange@redhat.com/

Cc: Joe Jin <joe.jin@oracle.com>
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
Changed since v1:
  - I sent out wrong patch version in v1
Changed since v2:
  - Add the Reviewed-by from Daniel and David

 target/i386/monitor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 6512846327..d727270fd0 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -28,6 +28,7 @@
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
 #include "qapi/qmp/qdict.h"
+#include "sysemu/hw_accel.h"
 #include "sysemu/kvm.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-misc-target.h"
@@ -654,7 +655,11 @@ void hmp_info_local_apic(Monitor *mon, const QDict *qdict)
 
     if (qdict_haskey(qdict, "apic-id")) {
         int id = qdict_get_try_int(qdict, "apic-id", 0);
+
         cs = cpu_by_arch_id(id);
+        if (cs) {
+            cpu_synchronize_state(cs);
+        }
     } else {
         cs = mon_get_cpu(mon);
     }
-- 
2.34.1

Re: [PATCH v3 1/1] hmp: synchronize cpu state for lapic info

[Juan Quintela]

Dongli Zhang <dongli.zhang@oracle.com> wrote:
> While the default "info lapic" always synchronizes cpu state ...
>
> mon_get_cpu()
> -> mon_get_cpu_sync(mon, true)
>    -> cpu_synchronize_state(cpu)
>       -> ioctl KVM_GET_LAPIC (taking KVM as example)
>
> ... the cpu state is not synchronized when the apic-id is available as
> argument.
>
> The cpu state should be synchronized when apic-id is available. Otherwise
> the "info lapic <apic-id>" always returns stale data.
>
> Reference:
> https://lore.kernel.org/all/20211028155457.967291-19-berrange@redhat.com/
>
> Cc: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

Reviewed-by: Juan Quintela <quintela@redhat.com>

But I wonder how I did get CC'd on this patch O:-)

Later, Juan.

Re: [PATCH v3 1/1] hmp: synchronize cpu state for lapic info

[Dongli Zhang]

Hi Juan,

On 10/30/23 09:31, Juan Quintela wrote:
> Dongli Zhang <dongli.zhang@oracle.com> wrote:
>> While the default "info lapic" always synchronizes cpu state ...
>>
>> mon_get_cpu()
>> -> mon_get_cpu_sync(mon, true)
>>    -> cpu_synchronize_state(cpu)
>>       -> ioctl KVM_GET_LAPIC (taking KVM as example)
>>
>> ... the cpu state is not synchronized when the apic-id is available as
>> argument.
>>
>> The cpu state should be synchronized when apic-id is available. Otherwise
>> the "info lapic <apic-id>" always returns stale data.
>>
>> Reference:
>> https://urldefense.com/v3/__https://lore.kernel.org/all/20211028155457.967291-19-berrange@redhat.com/__;!!ACWV5N9M2RV99hQ!KOLfuCesLC4T6ka9bjf4x6ncC34GPK9pVvWwOJhbwSZw2fwp3Mxlakk0fnR-NCoqRPKOX7X4SOAxozQBC7VQ$ 
>>
>> Cc: Joe Jin <joe.jin@oracle.com>
>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> But I wonder how I did get CC'd on this patch O:-)
> 

Thank you very much!

This component does not have a maintainer. I just blindly cc all suggested
reviewers :), in order to get it reviewed and merged.

get_maintainer.pl: No maintainers found, printing recent contributors.
get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.

Thank you very much!

Dongli Zhang

Re: [PATCH v3 1/1] hmp: synchronize cpu state for lapic info

[Markus Armbruster]

Dongli Zhang <dongli.zhang@oracle.com> writes:

> Hi Juan,
>
> On 10/30/23 09:31, Juan Quintela wrote:
>> Dongli Zhang <dongli.zhang@oracle.com> wrote:
>>> While the default "info lapic" always synchronizes cpu state ...
>>>
>>> mon_get_cpu()
>>> -> mon_get_cpu_sync(mon, true)
>>>    -> cpu_synchronize_state(cpu)
>>>       -> ioctl KVM_GET_LAPIC (taking KVM as example)
>>>
>>> ... the cpu state is not synchronized when the apic-id is available as
>>> argument.
>>>
>>> The cpu state should be synchronized when apic-id is available. Otherwise
>>> the "info lapic <apic-id>" always returns stale data.
>>>
>>> Reference:
>>> https://urldefense.com/v3/__https://lore.kernel.org/all/20211028155457.967291-19-berrange@redhat.com/__;!!ACWV5N9M2RV99hQ!KOLfuCesLC4T6ka9bjf4x6ncC34GPK9pVvWwOJhbwSZw2fwp3Mxlakk0fnR-NCoqRPKOX7X4SOAxozQBC7VQ$ 
>>>
>>> Cc: Joe Jin <joe.jin@oracle.com>
>>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
>> 
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> 
>> But I wonder how I did get CC'd on this patch O:-)
>> 
>
> Thank you very much!
>
> This component does not have a maintainer. I just blindly cc all suggested
> reviewers :), in order to get it reviewed and merged.
>
> get_maintainer.pl: No maintainers found, printing recent contributors.
> get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.

I posted a fix the other day:

    Subject: [PATCH] MAINTAINERS: Fully cover target/i386/*
    Message-ID: <20231030085336.2681386-1-armbru@redhat.com>
    https://lore.kernel.org/all/20231030085336.2681386-1-armbru@redhat.com/

> Thank you very much!

Re: [PATCH v3 1/1] hmp: synchronize cpu state for lapic info

[Thomas Huth]

On 02/11/2023 07.00, Markus Armbruster wrote:
> Dongli Zhang <dongli.zhang@oracle.com> writes:
> 
>> Hi Juan,
>>
>> On 10/30/23 09:31, Juan Quintela wrote:
>>> Dongli Zhang <dongli.zhang@oracle.com> wrote:
>>>> While the default "info lapic" always synchronizes cpu state ...
>>>>
>>>> mon_get_cpu()
>>>> -> mon_get_cpu_sync(mon, true)
>>>>     -> cpu_synchronize_state(cpu)
>>>>        -> ioctl KVM_GET_LAPIC (taking KVM as example)
>>>>
>>>> ... the cpu state is not synchronized when the apic-id is available as
>>>> argument.
>>>>
>>>> The cpu state should be synchronized when apic-id is available. Otherwise
>>>> the "info lapic <apic-id>" always returns stale data.
>>>>
>>>> Reference:
>>>> https://urldefense.com/v3/__https://lore.kernel.org/all/20211028155457.967291-19-berrange@redhat.com/__;!!ACWV5N9M2RV99hQ!KOLfuCesLC4T6ka9bjf4x6ncC34GPK9pVvWwOJhbwSZw2fwp3Mxlakk0fnR-NCoqRPKOX7X4SOAxozQBC7VQ$
>>>>
>>>> Cc: Joe Jin <joe.jin@oracle.com>
>>>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
>>>
>>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>>
>>> But I wonder how I did get CC'd on this patch O:-)
>>>
>>
>> Thank you very much!
>>
>> This component does not have a maintainer. I just blindly cc all suggested
>> reviewers :), in order to get it reviewed and merged.
>>
>> get_maintainer.pl: No maintainers found, printing recent contributors.
>> get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.
> 
> I posted a fix the other day:
> 
>      Subject: [PATCH] MAINTAINERS: Fully cover target/i386/*
>      Message-ID: <20231030085336.2681386-1-armbru@redhat.com>
>      https://lore.kernel.org/all/20231030085336.2681386-1-armbru@redhat.com/

I wrote a slightly better patch a month ago already (since these files do 
not only belong to TCG, but also to the other accelerators):

  https://lore.kernel.org/qemu-devel/20230929134551.395438-1-thuth@redhat.com/

Unfortunately, it got ignored so far :-(

  Thomas

Re: [PATCH v3 1/1] hmp: synchronize cpu state for lapic info

[Philippe Mathieu-Daudé]

On 26/10/23 23:19, Dongli Zhang wrote:
> While the default "info lapic" always synchronizes cpu state ...
> 
> mon_get_cpu()
> -> mon_get_cpu_sync(mon, true)
>     -> cpu_synchronize_state(cpu)
>        -> ioctl KVM_GET_LAPIC (taking KVM as example)
> 
> ... the cpu state is not synchronized when the apic-id is available as
> argument.
> 
> The cpu state should be synchronized when apic-id is available. Otherwise
> the "info lapic <apic-id>" always returns stale data.
> 
> Reference:
> https://lore.kernel.org/all/20211028155457.967291-19-berrange@redhat.com/
> 
> Cc: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> Changed since v1:
>    - I sent out wrong patch version in v1
> Changed since v2:
>    - Add the Reviewed-by from Daniel and David
> 
>   target/i386/monitor.c | 5 +++++
>   1 file changed, 5 insertions(+)

Patch queued via my cpu/misc tree, thanks!