AvatarPeripheral with x86 target

[pwissenlit]

Hey,

First, thanks for the really cool tool. :)
I'm currently trying my hand on it but I kind of struggle with few errors that might either be because I don't understand what I'm doing (insert here the dog-in-front-of-a-computer meme) or because of an issue with AvatarPeripheral.

Basically, here is a simplified example of what I'm doing:

from avatar2 import *
from avatar2.archs import X86
from avatar2.peripherals.avatar_peripheral import AvatarPeripheral

class HelloWorldPeripheral(AvatarPeripheral):

    def hw_read(self, offset, size):
        ret = self.hello_world[:size]
        self.hello_world = self.hello_world[size:] + self.hello_world[:size]

        # Convert the return value to an integer (py2/py3-compatible)
        # Python >3.2 could just call int.from_bytes(ret, byteorder='little')
        s2fmt = {1: 'B', 2: 'H', 4: 'I', 8: 'Q'}
        ret = struct.unpack('<' + s2fmt[size], ret)[0]
        return ret

    def nop_write(self, offset, size, value):
        return True

    def __init__(self, name, address, size, **kwargs):
        AvatarPeripheral.__init__(self, name, address, size)

        self.hello_world=b'Hello World'

        self.read_handler[0:size] = self.hw_read
        self.write_handler[0:size] = self.nop_write


avatar = Avatar(arch=X86, output_directory='/tmp/avatar_out')
qemu = avatar.add_target(QemuTarget, executable="/tmp/avatar-qemu/build/i386-softmmu/qemu-system-i386")

hw = avatar.add_memory_range(0x40004c00, 0x100, name='hello_world', emulate=HelloWorldPeripheral, permissions='rw-')

avatar.init_targets()

It does work perfectly fine if my target architecture is ARM but fails with the following error with x86:

Configurable: Adding peripheral[avatar-rmemory] region hello_world at address 0x40004c00
Bail out! ERROR:../qom/object.c:715:object_new_with_type: assertion failed: (type != NULL)

Do you have any idea about why I would obtain such behavior?

[rawsample]

Hey!

Unfortunately, the remote memory in the configurable machine is not supported for the i386 target.
You can partially add the support with the patch below. However, it will still not work, the gdbstub complains he does not find any CPU: qemu-system-i386: -gdb tcp::3333: gdbstub: meaningless to attach gdb to a machine without any CPU. I will check this tomorrow.

diff --git a/hw/avatar/meson.build b/hw/avatar/meson.build
index a42b1429c6..1e59604165 100644
--- a/hw/avatar/meson.build
+++ b/hw/avatar/meson.build
@@ -17,6 +17,10 @@ specific_ss.add(when: ['CONFIG_AVATAR', 'TARGET_MIPS'], if_true: files(
   'avatar_posix.c',
   'remote_memory.c'
 ))
-specific_ss.add(when: ['CONFIG_AVATAR', 'TARGET_I386'], if_true: files('configurable_machine.c'))
+specific_ss.add(when: ['CONFIG_AVATAR', 'TARGET_I386'], if_true: files(
+  'configurable_machine.c',
+  'avatar_posix.c',
+  'remote_memory.c'
+))
 specific_ss.add(when: ['CONFIG_AVATAR', 'TARGET_X86_64'], if_true: files('configurable_machine.c'))
 specific_ss.add(when: ['CONFIG_AVATAR', 'TARGET_PPC'], if_true: files('configurable_machine.c'))
diff --git a/hw/avatar/remote_memory.c b/hw/avatar/remote_memory.c
index ba74a5f5ee..f0581645a8 100644
--- a/hw/avatar/remote_memory.c
+++ b/hw/avatar/remote_memory.c
@@ -14,8 +14,6 @@
 
 #ifdef TARGET_ARM
 #include "target/arm/cpu.h"
-#elif TARGET_MIPS
-    //
 #endif
 
 
@@ -26,8 +24,6 @@ uint64_t get_current_pc(void){
 #ifdef TARGET_ARM
     ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0));
     return cpu->env.regs[15];
-#elif TARGET_MIPS
-    return 0; /*  implement me */
 #endif
     return 0;
 }

[pwissenlit]

Yay!
Many thanks for the quick reply and the patch. :)
Indeed, I stumbled on the "no cpu" error too. I'm currently digging in the source code to try and spot what could be the issue with no luck for now.
I guess you'll be more efficient than me but I'll keep you posted if I find a fix.
Have a good day!

[pwissenlit]

So, for the record, I quick fixed it with the following patch and it seems to work (I haven't tested it extensively though and I'm definitely not sure about the hardcoded apic-id value)...

diff --git a/hw/avatar/configurable_machine.c b/hw/avatar/configurable_machine.c
index a22ec11259..4cc18c1fb6 100644
--- a/hw/avatar/configurable_machine.c
+++ b/hw/avatar/configurable_machine.c
@@ -495,10 +495,7 @@ static THISCPU *create_cpu(MachineState * ms, QDict *conf)
     BusState* sysbus = sysbus_get_default();
     int num_irq = 64;

-#elif defined(TARGET_I386)
-    //
-
-#elif defined(TARGET_MIPS)
+#elif defined(TARGET_I386) ||  defined(TARGET_MIPS)
     Error *err = NULL;
 #endif  /* TARGET_ARM && ! TARGET_AARCH64 */

@@ -562,6 +559,18 @@ static THISCPU *create_cpu(MachineState * ms, QDict *conf)
             device_legacy_reset(cpuu->apic_state);
     }

+    if (!object_property_set_uint(OBJECT(cpuu), "apic-id", 0, &err)) {
+        error_report_err(err);
+        object_unref(OBJECT(cpuu));
+        exit(EXIT_FAILURE);
+    }
+
+    if (!qdev_realize(DEVICE(cpuu), NULL, &err)) {
+        error_report_err(err);
+        object_unref(OBJECT(cpuu));
+        exit(EXIT_FAILURE);
+    }
+
 #elif defined(TARGET_MIPS)
     cpu_oc = cpu_class_by_name(TYPE_MIPS_CPU, cpu_type);
     if (!cpu_oc) {

[rawsample]

This seems correct. I suppose the apic-id would become an issue in case of creating multiple CPU.
That would be nice to create a PR on the avatar-qemu repo, otherwise I can also do it :)
(Also, I updated the tests/test_qemutarget.py for this target, I will push it later)

Thanks for your contribution!

[pwissenlit]

Sure, I'll do it tomorrow. :)
Also, just to let you know, I may have stumbled on two additional issues that could still block the proper functioning of this target: for now the entry_address is not correctly taken into account by the cpu during its initialization and memory regions created with memory_region_init_ram don't seem to be writable. I'm currently investigating the root causes and will do a PR if I manage to fix them.