On Tue, Jul 09, 2024 at 15:12:37 +0200, Marcin Juszkiewicz wrote: > Dnia wtorek, 9 lipca 2024 15:00:12 CEST Leif Lindholm via groups.io pisze: > > On Tue, Jul 09, 2024 at 12:47:10 +0200, Marcin Juszkiewicz wrote: > > > Function AddPpttTable() adding PPTT got too long. This change moves part > > > of it into helper function AddCoresToPpttTable() which takes care of > > > generating entries for Core and below (Cache, Thread). > > > > > > Signed-off-by: Marcin Juszkiewicz <marcin.juszkiew...@linaro.org> > > > --- > > > > > > .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 237 > > > +++++++++++--------- 1 file changed, 133 insertions(+), 104 deletions(-) > > > > > > diff --git > > > a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > > > b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c index > > > a7a9664abdcb..a4b2ee2fdcb0 100644 > > > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > > > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c > > > @@ -29,6 +29,9 @@ > > > > > > static UINTN GicItsBase; > > > > > > +static UINTN CpuId; > > > +static UINTN CacheId; > > > + > > > > static variables are supposed to have g (global) or m (module) prefix. > > This is local, so m. > > (Yes, that means I missed that when reviewing the GitIts bits.) > > > > Also, why are these in a #pragma pack(1) block? > > Added right after GicItsBase. Moved out of block.
I don't think it makes any sense for GicItsBase either, but that's not part of this review. > > > #pragma pack () > > > STATIC > > > > > +UINT32 > > > +AddCoresToPpttTable ( > > > + UINT8 *New, > > > + UINT32 ClusterIndex, > > > + CpuTopology CpuTopo > > > + ) > > done > > > > - ClusterIndex = CoreIndex; > > > + CoresPartSize = AddCoresToPpttTable (New, ClusterIndex, CpuTopo); > > > + ClusterIndex += CoresPartSize; > > > > This sounds like ClisterIndex is no longer an Index after this patch. > > Should it be renamed? > > It is still an Index. CoresPartSize is a size taken by Core/Cache/Thread part > of this Cluster. But doesn't that make it an offset instead of an index? / Leif > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119844): https://edk2.groups.io/g/devel/message/119844 Mute This Topic: https://groups.io/mt/107120147/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-