On 16/10/26 08:39, Pierrick Bouvier wrote:
On 10/15/26 10:43 PM, Philippe Mathieu-Daudé wrote:
On 15/10/26 23:32, Pierrick Bouvier wrote:
For now, we keep the global cpu_get_dump_info symbol defined.
Signed-off-by: Pierrick Bouvier <[email protected]>
---
target/i386/arch_dump.c | 17 ++++++++++++++---
target/i386/cpu-internal.h | 5 +++++
target/i386/cpu.c | 1 +
3 files changed, 20 insertions(+), 3 deletions(-)
-int cpu_get_dump_info(ArchDumpInfo *info,
+int x86_get_dump_info(const CPUState *cs, ArchDumpInfo *info,
const GuestPhysBlockList *guest_phys_blocks)
{
bool lma = false;
GuestPhysBlock *block;
#ifdef TARGET_X86_64
- X86CPU *first_x86_cpu = X86_CPU(first_cpu);
- lma = first_cpu && (first_x86_cpu->env.hflags & HF_LMA_MASK);
I know this is pre-existing, but I'd like to understand this check, ...
+ const X86CPU *cpu = X86_CPU(cs);
+ lma = !!(cpu->env.hflags & HF_LMA_MASK);
#endif
if (lma) {
@@ -429,6 +430,16 @@ int cpu_get_dump_info(ArchDumpInfo *info,
return 0;
}
+int cpu_get_dump_info(ArchDumpInfo *info,
+ const GuestPhysBlockList *guest_phys_blocks)
+{
+ if (first_cpu == NULL) {
... because you kept it. Why do we need it? QTest something?
It's here to ensure it doesn't segfault when dump is called for none
machine:
https://gitlab.com/qemu-project/qemu/-/commit/
fd5d23babf9838c1b099a9e8020e778aac0ebb4d
It's a corner case, but doesn't hurt to keep it IMHO.
I don't see it as a corner case but a bad API design. Why get called
if a machine doesn't have any CPU? The caller (dump_init in dump/dump.c)
should do the &first_cpu check, and either call with a valid CPU or not
call at all. The callees should abort for NULL vcpu (programming error).
Actually dump_init() already does the check by assigning nr_cpus.
Anyway your changes look correct since you pass a @cpu argument to the
methods added in SysemuCPUOps.
(goal is to understand when this can be called on a non-vCPU thread)
I don't think it matters where this code runs, first_cpu is just the
first cpu available and it can be accessed from any thread, vcpu or not.
Now, the day we'll cleanup this TARGET_X86_64 guard, then it will be
time to revisit this question.
No need to revisit, add the target_heterogenenous() check (this code is
completely broken in this setup) then we are good / safe.
