Re: [PATCH 0/3] hw/arm/virt: Use generic CPU invalidation

[Gavin Shan]

Hi Marcin,

On 7/13/23 22:44, Marcin Juszkiewicz wrote:
> W dniu 13.07.2023 o 14:34, Gavin Shan pisze:
> > On 7/13/23 21:52, Marcin Juszkiewicz wrote:
> > > W dniu 13.07.2023 o 13:44, Peter Maydell pisze:
> > >
> > > > I see this isn't a change in this patch, but given that what the user specifies is not 
> > > > "cortex-a8-arm-cpu" but "cortex-a8", why
> > > > do we include the "-arm-cpu" suffix in the error messages? It's
> > > > not valid syntax to say "-cpu cortex-a8-arm-cpu", so it's a bit misleading...
>
> > > I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}"
> > > string from names:
>
> > Peter and Marcin, how about to split the CPU types to two fields, as below? In 
> > this way, the complete CPU type will be used for validation
> > and the 'internal' names will be used for the error messages.
>
> Note that it should probably propagate to all architectures handled in QEMU, 
> not only Arm ones. I do not know which machines have list of supported cpu 
> types like arm/virt or arm/sbsa-ref ones.

Right, there are more boards eligible for this generic CPU type invalidation. I 
will
cover all of them in next revision. Currently, the pattern of prefix + suffix, 
used
to split the CPU type name, can work well for all cases.

[gshan@gshan hw]$ git grep strcmp.*cpu_type
arm/bananapi_m2u.c:    if (strcmp(machine->cpu_type, 
ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
arm/cubieboard.c:    if (strcmp(machine->cpu_type, 
ARM_CPU_TYPE_NAME("cortex-a8")) != 0) {
arm/mps2-tz.c:    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
arm/mps2.c:    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
arm/msf2-som.c:    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
arm/musca.c:    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
arm/npcm7xx_boards.c:    if (strcmp(machine->cpu_type, mc->default_cpu_type) != 
0) {
arm/orangepi.c:    if (strcmp(machine->cpu_type, 
ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
avr/boot.c:        if (strcmp(elf_cpu, mcu_cpu_type)) {                                       
<<<<< needsn't CPU type validation
riscv/shakti_c.c:    if (strcmp(mstate->cpu_type, TYPE_RISCV_CPU_SHAKTI_C) != 
0) {


> > > I can change sbsa-ref to follow that change but let it be userfriendly.
>
> > Yes, sbsa-ref needs an extra patch to use the generic invalidation.
> > I can add one patch on the top for that in next revision if you
> > agree, Marcin.
>
> I am fine with it.

Thanks a lot for your confirm, Marcin.

Thanks,
Gavin