Re: [Qemu-arm] [Qemu-devel] [PATCH v2 15/15] arm/xlnx-zynqmp: put APUs and RPUs in separate GDB groups

[Peter Maydell]

On 2 October 2018 at 12:33, Philippe Mathieu-Daudé <address@hidden> wrote:
> Cc'ing more QOM involved people.
>
> On 01/10/2018 13:57, Luc Michel wrote:
>> Create two separate QOM containers for APUs and RPUs to indicate to the
>> GDB stub that those CPUs should be put in different processes.
>>
>> Signed-off-by: Luc Michel <address@hidden>
>> ---
>>  hw/arm/xlnx-zynqmp.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index c195040350..5e92adbc71 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -22,10 +22,11 @@
>>  #include "hw/arm/xlnx-zynqmp.h"
>>  #include "hw/intc/arm_gic_common.h"
>>  #include "exec/address-spaces.h"
>>  #include "sysemu/kvm.h"
>>  #include "kvm_arm.h"
>> +#include "exec/gdbstub.h"
>>
>>  #define GIC_NUM_SPI_INTR 160
>>
>>  #define ARM_PHYS_TIMER_PPI  30
>>  #define ARM_VIRT_TIMER_PPI  27
>> @@ -175,17 +176,18 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, 
>> const char *boot_cpu,
>>                                     Error **errp)
>>  {
>>      Error *err = NULL;
>>      int i;
>>      int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, 
>> XLNX_ZYNQMP_NUM_RPU_CPUS);
>> +    Object *rpu_group = gdb_cpu_group_container_get(OBJECT(s));
>
> I'd rather keep this generic: not involve 'gdb' container name.

Yes, I agree. We should structure how we construct our
model to follow what the hardware has (two CPU clusters
with 4 cores each), and then the gdb code should introspect
the system later to decide how it exposes things to the gdb
user. GDB specifics should (as far as possible) be kept out
of the board code.

The fact that there are two clusters here also
affects other things, like whether they have the
same view of memory, whether they can share translated
code (they shouldn't but do at the moment), and so on --
it's not just a GDB-relevant distinction. So we should
be modelling it somehow, definitely. I don't have a clear
view how just yet.

This probably ties into the stuff I have somewhere on
my todo list about supporting multiple heterogenous
systems. The problem with this xilinx board is that it
is trying to model that kind of system but we don't actually
properly support that in QEMU yet.

thanks
-- PMM