Enable CPU cluster support on SbsaQemu platform, so that users can
specify a 4-level CPU hierarchy sockets/clusters/cores/threads. And this
topology can be passed to the firmware through DT cpu-map.

Changes in v2:
- put this code before sbsa_fdt_add_gic_node().

xiongyining1480 (2):
  hw/arm/sbsa-ref:Enable CPU cluster on ARM sbsa machine
  hw/arm/sbsa-ref: Add cpu-map to device tree

 hw/arm/sbsa-ref.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

-- 
2.34.1

On Tue, 12 Mar 2024 at 08:32, Xiong Yining
<xiongyining1480@phytium.com.cn> wrote:
>
> Enable CPU cluster support on SbsaQemu platform, so that users can
> specify a 4-level CPU hierarchy sockets/clusters/cores/threads. And this
> topology can be passed to the firmware through DT cpu-map.
>
> Changes in v2:
> - put this code before sbsa_fdt_add_gic_node().
>
> xiongyining1480 (2):
>   hw/arm/sbsa-ref:Enable CPU cluster on ARM sbsa machine
>   hw/arm/sbsa-ref: Add cpu-map to device tree

Thanks for these patches. I think we should squash the two
patches together into one, because the first patch is only
a single line, and also because we shouldn't say that the
machine supports cluster topology until it actually does
by putting the information into the device tree.

There's no rush, because we're  now in softfreeze for 9.0, so these
will have to wait until 9.0 is released (in about a month's time).

I'm also a bit confused by the Reviewed-by: tag from Marcin on patch 2,
because I can't see that in my mail archives of the discussion on version
1 of this patchset, only a Tested-by. Marcin, are you OK with these patches?

Also, is this change to the DTB something that would require an
increase in the sbsa-ref platform version number, or not?
Should we adjust the documentation in docs/system/arm/sbsa.rst to
mention that the DTB might have cluster topology information?

thanks
-- PMM

W dniu 22.03.2024 o 19:51, Peter Maydell pisze:
> On Tue, 12 Mar 2024 at 08:32, Xiong Yining

>> xiongyining1480 (2):
>>    hw/arm/sbsa-ref:Enable CPU cluster on ARM sbsa machine
>>    hw/arm/sbsa-ref: Add cpu-map to device tree
> 
> Thanks for these patches. I think we should squash the two
> patches together into one, because the first patch is only
> a single line, and also because we shouldn't say that the
> machine supports cluster topology until it actually does
> by putting the information into the device tree.
> 
> There's no rush, because we're  now in softfreeze for 9.0, so these
> will have to wait until 9.0 is released (in about a month's time).

> I'm also a bit confused by the Reviewed-by: tag from Marcin on patch 2,
> because I can't see that in my mail archives of the discussion on version
> 1 of this patchset, only a Tested-by.
> Marcin, are you OK with these patches?

I only tested them. They are fine, will check on Monday.

> Also, is this change to the DTB something that would require an
> increase in the sbsa-ref platform version number, or not?

TF-A will check for "/cpus/cpu-map" node and if it is missing then will 
not provide it to EDK2. So far I did not saw patches for firmware side.

I would add bump of platform version to 0.4 one. It is cheap operation 
and so far (from firmware side) we check for >= 0.3 only.

 > Should we adjust the documentation in docs/system/arm/sbsa.rst to
 > mention that the DTB might have cluster topology information?

Yes. I will send an update to mention that NUMA configuration can be 
there too (we already export it from TF-A to EDK2 via SMC calls).

> W dniu 22.03.2024 o 19:51, Peter Maydell pisze:
> > On Tue, 12 Mar 2024 at 08:32, Xiong Yining
> 
> >> xiongyining1480 (2):
> >>    hw/arm/sbsa-ref:Enable CPU cluster on ARM sbsa machine
> >>    hw/arm/sbsa-ref: Add cpu-map to device tree
> > 
> > Thanks for these patches. I think we should squash the two
> > patches together into one, because the first patch is only
> > a single line, and also because we shouldn't say that the
> > machine supports cluster topology until it actually does
> > by putting the information into the device tree.

fully agree

> > There's no rush, because we're  now in softfreeze for 9.0, so these
> > will have to wait until 9.0 is released (in about a month's time).
> 
> > I'm also a bit confused by the Reviewed-by: tag from Marcin on patch 2,
> > because I can't see that in my mail archives of the discussion on version
> > 1 of this patchset, only a Tested-by.
> > Marcin, are you OK with these patches?
> 
> I only tested them. They are fine, will check on Monday.
> 
> > Also, is this change to the DTB something that would require an
> > increase in the sbsa-ref platform version number, or not?
> 
> TF-A will check for "/cpus/cpu-map" node and if it is missing then will 
> not provide it to EDK2. So far I did not saw patches for firmware side.

I send a patch in TF-A to check  "/cpus/cpu-map" node https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/27189/1#message-2c29be6b8b9b4fd3fef23ba7be6fe6fc3a2d0aef. It can be used with this patch in qemu.

> I would add bump of platform version to 0.4 one. It is cheap operation 
> and so far (from firmware side) we check for >= 0.3 only.
> 
>  > Should we adjust the documentation in docs/system/arm/sbsa.rst to
>  > mention that the DTB might have cluster topology information?
> 
> Yes. I will send an update to mention that NUMA configuration can be 
> there too (we already export it from TF-A to EDK2 via SMC calls).


信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。
Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.