All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] hw/arm: use correct base address when setting CBAR
@ 2026-06-18  9:36 Alex Bennée
  2026-06-18 10:07 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Bennée @ 2026-06-18  9:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Arnd Bergmann, Tyrone Ting, Hao Wu,
	Peter Maydell, open list:Nuvoton NPCM7xx

This should point at the base of the Cortex-A9 MPCore private address
space. If it doesn't we will confuse the Linux kernel as it probes the
SCU registers and erroneously assumes the system is a buggy Aegis SOC
and nerf the emission of SEV instructions, deadlocking any WFE's in
the kernel (or QEMU smpboot code).

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Suggested-by: Arnd Bergmann <arnd@linaro.org>
---
 hw/arm/npcm7xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index c2bbcd89dbc..c27f149c04a 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -492,7 +492,7 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
     /* CPUs */
     for (i = 0; i < nc->num_cpus; i++) {
         object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
-                                NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
+                                NPCM7XX_CPUP_BA, &error_abort);
         object_property_set_bool(OBJECT(&s->cpu[i]), "reset-hivecs", true,
                                  &error_abort);
 
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] hw/arm: use correct base address when setting CBAR
  2026-06-18  9:36 [RFC PATCH] hw/arm: use correct base address when setting CBAR Alex Bennée
@ 2026-06-18 10:07 ` Philippe Mathieu-Daudé
  2026-06-18 10:17   ` Peter Maydell
  2026-06-18 10:37   ` Alex Bennée
  0 siblings, 2 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-06-18 10:07 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Arnd Bergmann, Tyrone Ting, Hao Wu, Peter Maydell,
	open list:Nuvoton NPCM7xx

On 18/6/26 11:36, Alex Bennée wrote:
> This should point at the base of the Cortex-A9 MPCore private address
> space. If it doesn't we will confuse the Linux kernel as it probes the
> SCU registers and erroneously assumes the system is a buggy Aegis SOC
> and nerf the emission of SEV instructions, deadlocking any WFE's in
> the kernel (or QEMU smpboot code).

I'm confused...

   The Configuration Base Address Register holds the physical base
   address of the memory-mapped GIC CPU interface registers.

NPCM7XX_GIC_CPU_IF_ADDR is the correct address of "the memory-mapped
GIC CPU interface", NPCM7XX_CPUP_BA is the A9MPCORE SCU MMIO address.
AFAIU the modelling looks correct.

Per your description, the Linux guest tries to access GIC_CPU_IF_ADDR
but ends up accessing SCU_ADDR? Maybe the issue is with the
arm_boot_info::gic_cpu_if_addr address we pass to the guest? Maybe
the secondary reset code is corrupted?

static const ARMInsnFixup smpboot[] = {
     { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
     { 0xe59f0028 }, /* ldr r0, bootreg_addr */
     { 0xe3a01001 }, /* mov r1, #1 */
     { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
     { 0xe3a010ff }, /* mov r1, #0xff */
     { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
     { 0, FIXUP_DSB },   /* dsb */
     { 0xe320f003 }, /* wfi */
     { 0xe5901000 }, /* ldr     r1, [r0] */
     { 0xe1110001 }, /* tst     r1, r1 */
     { 0x0afffffb }, /* beq     <wfi> */
     { 0xe12fff11 }, /* bx      r1 */
     { 0, FIXUP_GIC_CPU_IF }, /* gic_cpu_if: .word 0x.... */

Can you check ^^^ this value?

Maybe this code has to be updated for your WFE use case?

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@linaro.org>
> ---
>   hw/arm/npcm7xx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> index c2bbcd89dbc..c27f149c04a 100644
> --- a/hw/arm/npcm7xx.c
> +++ b/hw/arm/npcm7xx.c
> @@ -492,7 +492,7 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
>       /* CPUs */
>       for (i = 0; i < nc->num_cpus; i++) {
>           object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
> -                                NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
> +                                NPCM7XX_CPUP_BA, &error_abort);
>           object_property_set_bool(OBJECT(&s->cpu[i]), "reset-hivecs", true,
>                                    &error_abort);
>   



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] hw/arm: use correct base address when setting CBAR
  2026-06-18 10:07 ` Philippe Mathieu-Daudé
@ 2026-06-18 10:17   ` Peter Maydell
  2026-06-18 10:41     ` Philippe Mathieu-Daudé
  2026-06-18 10:37   ` Alex Bennée
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2026-06-18 10:17 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Alex Bennée, qemu-devel, Arnd Bergmann, Tyrone Ting, Hao Wu,
	open list:Nuvoton NPCM7xx

On Thu, 18 Jun 2026 at 11:07, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> On 18/6/26 11:36, Alex Bennée wrote:
> > This should point at the base of the Cortex-A9 MPCore private address
> > space. If it doesn't we will confuse the Linux kernel as it probes the
> > SCU registers and erroneously assumes the system is a buggy Aegis SOC
> > and nerf the emission of SEV instructions, deadlocking any WFE's in
> > the kernel (or QEMU smpboot code).
>
> I'm confused...
>
>    The Configuration Base Address Register holds the physical base
>    address of the memory-mapped GIC CPU interface registers.

Where did this quote come from? Note that the CBAR is an IMPDEF
sysreg, so its exact behaviour varies depending on the CPU model.

The A9 TRM says:

https://developer.arm.com/documentation/ddi0388/i/system-control/register-descriptions/configuration-base-address-register?lang=en

In Cortex-A9 MPCore implementations, the base address is reset to
PERIPHBASE[31:13] so that software can determine the location of the
private memory region.

and then in the A9 MPCore TRM
https://developer.arm.com/documentation/ddi0407/g/Introduction/Private-Memory-Region

you can see that the private memory region has all the per-core
devices; the SCU happens to be the one at the start.

-- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] hw/arm: use correct base address when setting CBAR
  2026-06-18 10:07 ` Philippe Mathieu-Daudé
  2026-06-18 10:17   ` Peter Maydell
@ 2026-06-18 10:37   ` Alex Bennée
  2026-06-18 10:49     ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Bennée @ 2026-06-18 10:37 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Arnd Bergmann, Tyrone Ting, Hao Wu, Peter Maydell,
	open list:Nuvoton NPCM7xx

Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> writes:

> On 18/6/26 11:36, Alex Bennée wrote:
>> This should point at the base of the Cortex-A9 MPCore private address
>> space. If it doesn't we will confuse the Linux kernel as it probes the
>> SCU registers and erroneously assumes the system is a buggy Aegis SOC
>> and nerf the emission of SEV instructions, deadlocking any WFE's in
>> the kernel (or QEMU smpboot code).
>
> I'm confused...
>
>   The Configuration Base Address Register holds the physical base
>   address of the memory-mapped GIC CPU interface registers.

For which CPU? These are cortex-a9's not a57's and the behaviour is
IMPDEF.

See:

  https://developer.arm.com/documentation/ddi0407/i/introduction/private-memory-region

>
> NPCM7XX_GIC_CPU_IF_ADDR is the correct address of "the memory-mapped
> GIC CPU interface", NPCM7XX_CPUP_BA is the A9MPCORE SCU MMIO address.
> AFAIU the modelling looks correct.
>
> Per your description, the Linux guest tries to access GIC_CPU_IF_ADDR
> but ends up accessing SCU_ADDR? Maybe the issue is with the
> arm_boot_info::gic_cpu_if_addr address we pass to the guest? Maybe
> the secondary reset code is corrupted?
>
> static const ARMInsnFixup smpboot[] = {
>     { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>     { 0xe59f0028 }, /* ldr r0, bootreg_addr */
>     { 0xe3a01001 }, /* mov r1, #1 */
>     { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>     { 0xe3a010ff }, /* mov r1, #0xff */
>     { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>     { 0, FIXUP_DSB },   /* dsb */
>     { 0xe320f003 }, /* wfi */
>     { 0xe5901000 }, /* ldr     r1, [r0] */
>     { 0xe1110001 }, /* tst     r1, r1 */
>     { 0x0afffffb }, /* beq     <wfi> */
>     { 0xe12fff11 }, /* bx      r1 */
>     { 0, FIXUP_GIC_CPU_IF }, /* gic_cpu_if: .word 0x.... */
>
> Can you check ^^^ this value?

I don't think we use this for npcm7xx:

  static void npcm7xx_write_secondary_boot(ARMCPU *cpu,
                                           const struct arm_boot_info *info)
  {
      /*
       * The default smpboot stub halts the secondary CPU with a 'wfi'
       * instruction, but the arch/arm/mach-npcm/platsmp.c in the Linux kernel
       * does not send an IPI to wake it up, so the second CPU fails to boot. So
       * we need to provide our own smpboot stub that can not use 'wfi', it has
       * to spin the secondary CPU until the first CPU writes to the SCRPAD reg.
       */
      uint32_t smpboot[] = {
          0xe59f2018,     /* ldr r2, bootreg_addr */
          0xe3a00000,     /* mov r0, #0 */
          0xe5820000,     /* str r0, [r2] */
          0xe320f002,     /* wfe */
          0xe5921000,     /* ldr r1, [r2] */
          0xe1110001,     /* tst r1, r1 */
          0x0afffffb,     /* beq <wfe> */
          0xe12fff11,     /* bx r1 */
          NPCM7XX_SMP_BOOTREG_ADDR,
      };
      int i;

      for (i = 0; i < ARRAY_SIZE(smpboot); i++) {
          smpboot[i] = tswap32(smpboot[i]);
      }

      rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
                         NPCM7XX_SMP_LOADER_START);
  }


>
> Maybe this code has to be updated for your WFE use case?

In earlier attempts I did drop the wfe in favour of a NOP until I
figured out why the SEV's were not flowing.

>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Suggested-by: Arnd Bergmann <arnd@linaro.org>
>> ---
>>   hw/arm/npcm7xx.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
>> index c2bbcd89dbc..c27f149c04a 100644
>> --- a/hw/arm/npcm7xx.c
>> +++ b/hw/arm/npcm7xx.c
>> @@ -492,7 +492,7 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
>>       /* CPUs */
>>       for (i = 0; i < nc->num_cpus; i++) {
>>           object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
>> -                                NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
>> +                                NPCM7XX_CPUP_BA, &error_abort);
>>           object_property_set_bool(OBJECT(&s->cpu[i]), "reset-hivecs", true,
>>                                    &error_abort);
>>   

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] hw/arm: use correct base address when setting CBAR
  2026-06-18 10:17   ` Peter Maydell
@ 2026-06-18 10:41     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-06-18 10:41 UTC (permalink / raw)
  To: Peter Maydell, Philippe Mathieu-Daudé
  Cc: Alex Bennée, qemu-devel, Arnd Bergmann, Tyrone Ting, Hao Wu,
	open list:Nuvoton NPCM7xx

On 18/6/26 12:17, Peter Maydell wrote:
> On Thu, 18 Jun 2026 at 11:07, Philippe Mathieu-Daudé
> <philmd@oss.qualcomm.com> wrote:
>>
>> On 18/6/26 11:36, Alex Bennée wrote:
>>> This should point at the base of the Cortex-A9 MPCore private address
>>> space. If it doesn't we will confuse the Linux kernel as it probes the
>>> SCU registers and erroneously assumes the system is a buggy Aegis SOC
>>> and nerf the emission of SEV instructions, deadlocking any WFE's in
>>> the kernel (or QEMU smpboot code).
>>
>> I'm confused...
>>
>>     The Configuration Base Address Register holds the physical base
>>     address of the memory-mapped GIC CPU interface registers.
> 
> Where did this quote come from? Note that the CBAR is an IMPDEF
> sysreg, so its exact behaviour varies depending on the CPU model.

Oops I was looking at the A57 TRM, not A9:
https://developer.arm.com/documentation/ddi0488/h/system-control/aarch32-register-descriptions/configuration-base-address-register

> 
> The A9 TRM says:
> 
> https://developer.arm.com/documentation/ddi0388/i/system-control/register-descriptions/configuration-base-address-register?lang=en
> 
> In Cortex-A9 MPCore implementations, the base address is reset to
> PERIPHBASE[31:13] so that software can determine the location of the
> private memory region.
> 
> and then in the A9 MPCore TRM
> https://developer.arm.com/documentation/ddi0407/g/Introduction/Private-Memory-Region
> 
> you can see that the private memory region has all the per-core
> devices; the SCU happens to be the one at the start.
> 
> -- PMM
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] hw/arm: use correct base address when setting CBAR
  2026-06-18 10:37   ` Alex Bennée
@ 2026-06-18 10:49     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-06-18 10:49 UTC (permalink / raw)
  To: Alex Bennée, Philippe Mathieu-Daudé
  Cc: qemu-devel, Arnd Bergmann, Tyrone Ting, Hao Wu, Peter Maydell,
	open list:Nuvoton NPCM7xx

On 18/6/26 12:37, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> writes:
> 
>> On 18/6/26 11:36, Alex Bennée wrote:
>>> This should point at the base of the Cortex-A9 MPCore private address
>>> space. If it doesn't we will confuse the Linux kernel as it probes the
>>> SCU registers and erroneously assumes the system is a buggy Aegis SOC
>>> and nerf the emission of SEV instructions, deadlocking any WFE's in
>>> the kernel (or QEMU smpboot code).
>>
>> I'm confused...
>>
>>    The Configuration Base Address Register holds the physical base
>>    address of the memory-mapped GIC CPU interface registers.
> 
> For which CPU? These are cortex-a9's not a57's and the behaviour is
> IMPDEF.
> 
> See:
> 
>    https://developer.arm.com/documentation/ddi0407/i/introduction/private-memory-region

Indeed. I'll let Nuvoton folks confirm that then.

> 
>>
>> NPCM7XX_GIC_CPU_IF_ADDR is the correct address of "the memory-mapped
>> GIC CPU interface", NPCM7XX_CPUP_BA is the A9MPCORE SCU MMIO address.
>> AFAIU the modelling looks correct.
>>
>> Per your description, the Linux guest tries to access GIC_CPU_IF_ADDR
>> but ends up accessing SCU_ADDR? Maybe the issue is with the
>> arm_boot_info::gic_cpu_if_addr address we pass to the guest? Maybe
>> the secondary reset code is corrupted?
>>
>> static const ARMInsnFixup smpboot[] = {
>>      { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
>>      { 0xe59f0028 }, /* ldr r0, bootreg_addr */
>>      { 0xe3a01001 }, /* mov r1, #1 */
>>      { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
>>      { 0xe3a010ff }, /* mov r1, #0xff */
>>      { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
>>      { 0, FIXUP_DSB },   /* dsb */
>>      { 0xe320f003 }, /* wfi */
>>      { 0xe5901000 }, /* ldr     r1, [r0] */
>>      { 0xe1110001 }, /* tst     r1, r1 */
>>      { 0x0afffffb }, /* beq     <wfi> */
>>      { 0xe12fff11 }, /* bx      r1 */
>>      { 0, FIXUP_GIC_CPU_IF }, /* gic_cpu_if: .word 0x.... */
>>
>> Can you check ^^^ this value?
> 
> I don't think we use this for npcm7xx:

Oh I missed the specific smpboot[].

> 
>    static void npcm7xx_write_secondary_boot(ARMCPU *cpu,
>                                             const struct arm_boot_info *info)
>    {
>        /*
>         * The default smpboot stub halts the secondary CPU with a 'wfi'
>         * instruction, but the arch/arm/mach-npcm/platsmp.c in the Linux kernel
>         * does not send an IPI to wake it up, so the second CPU fails to boot. So
>         * we need to provide our own smpboot stub that can not use 'wfi', it has
>         * to spin the secondary CPU until the first CPU writes to the SCRPAD reg.
>         */
>        uint32_t smpboot[] = {
>            0xe59f2018,     /* ldr r2, bootreg_addr */
>            0xe3a00000,     /* mov r0, #0 */
>            0xe5820000,     /* str r0, [r2] */
>            0xe320f002,     /* wfe */
>            0xe5921000,     /* ldr r1, [r2] */
>            0xe1110001,     /* tst r1, r1 */
>            0x0afffffb,     /* beq <wfe> */
>            0xe12fff11,     /* bx r1 */
>            NPCM7XX_SMP_BOOTREG_ADDR,
>        };
>        int i;
> 
>        for (i = 0; i < ARRAY_SIZE(smpboot); i++) {
>            smpboot[i] = tswap32(smpboot[i]);
>        }
> 
>        rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
>                           NPCM7XX_SMP_LOADER_START);
>    }
> 
> 
>>
>> Maybe this code has to be updated for your WFE use case?
> 
> In earlier attempts I did drop the wfe in favour of a NOP until I
> figured out why the SEV's were not flowing.
> 
>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Suggested-by: Arnd Bergmann <arnd@linaro.org>
>>> ---
>>>    hw/arm/npcm7xx.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
>>> index c2bbcd89dbc..c27f149c04a 100644
>>> --- a/hw/arm/npcm7xx.c
>>> +++ b/hw/arm/npcm7xx.c
>>> @@ -492,7 +492,7 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
>>>        /* CPUs */
>>>        for (i = 0; i < nc->num_cpus; i++) {
>>>            object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
>>> -                                NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
>>> +                                NPCM7XX_CPUP_BA, &error_abort);
>>>            object_property_set_bool(OBJECT(&s->cpu[i]), "reset-hivecs", true,
>>>                                     &error_abort);
>>>    
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-18 10:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18  9:36 [RFC PATCH] hw/arm: use correct base address when setting CBAR Alex Bennée
2026-06-18 10:07 ` Philippe Mathieu-Daudé
2026-06-18 10:17   ` Peter Maydell
2026-06-18 10:41     ` Philippe Mathieu-Daudé
2026-06-18 10:37   ` Alex Bennée
2026-06-18 10:49     ` Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.