From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by mx.groups.io with SMTP id smtpd.web10.1259.1639588735751875616 for ; Wed, 15 Dec 2021 09:18:55 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20210112.gappssmtp.com header.s=20210112 header.b=2HgN4G72; spf=pass (domain: nuviainc.com, ip: 209.85.216.43, mailfrom: rebecca@nuviainc.com) Received: by mail-pj1-f43.google.com with SMTP id c22-20020a17090a109600b001b101f02dd1so3677918pja.5 for ; Wed, 15 Dec 2021 09:18:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20210112.gappssmtp.com; s=20210112; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language; bh=Ci9MCO31XCXltiXrvEhJFHvm9chIYpWeVdYj7wPuUp8=; b=2HgN4G72ZeQSml0XRnzoHG/KA+HbpSys0S3FQt7kJHKKDbISYJlHRz52bnLpIM+fz9 X5kz1fy3BZM1QXzw1wPrerJc++zJkqGgfObc5fuqsdayMAhYA1dR7cbnGOT/WWY3xnpi YVF9cjYRpGC2c+bqZKgrxB9EDM0AbcS3YgdXtLLRMzGKX9A0tUZbrFymVL2/cQEyHvhh d1GNoAY0lFW2TudVLZdPeFTSC73UTcBGLpXYvXIl4opwtVBIg2KBCVUXOXVO0ylJufzQ WGNUS4vYfye6wXc1M+My4W9SelUT+cxAVy8yVQNw5Xw6SkoPgFFR3NyJ9Qg5jU2N4+8q PMtQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=Ci9MCO31XCXltiXrvEhJFHvm9chIYpWeVdYj7wPuUp8=; b=tokX9gpMsM/g0/VJsaKZrQ9z0iAczffmGl3BUTEY2kYM//KrprOC5dGPfZ0DWXpkXZ 2x5wVbl7q/L6p2++wwH/77Praq+GYRWAzJl6PCN+WtD1iThs8al74qKCWm96taBPhZjR CoRT3R0UnP5auy3VSf4aD9xh9zsZ/+6Q0k8ZJYj3DJP3ofQxJCWKN/u00tr9FsZtVr0G kL3hmq8YE83FXk/UNhBrE8cAnn0t3jhngQmuC4pEQNN6LU9ZjF6q/2ki5TduhgPUldiZ 5DvjBG4dT3XTrorg6tU87aQegaXxLi/TtgG7QkdVVFLt4zBHX8fXuMEHhXzDv5VQV7W3 Kvlw== X-Gm-Message-State: AOAM531o1ia+eEr34wwKOPptDa+gRogESIHAvQrR7JqSTWFNgDd72rFQ OO8nDHfxnHWXb7ieU5Vr2V7WlA== X-Google-Smtp-Source: ABdhPJzhJxUWJeS6B6ciayUoJjTp8nV/kBnQoCyvbCJrvXuTTfly1fFvFPravN96l85Fx2MWJxzbrA== X-Received: by 2002:a17:903:1210:b0:143:a088:7932 with SMTP id l16-20020a170903121000b00143a0887932mr12528271plh.11.1639588735287; Wed, 15 Dec 2021 09:18:55 -0800 (PST) Return-Path: Received: from ?IPv6:2601:681:4300:69e:9e7b:efff:fe2b:884c? ([2601:681:4300:69e:9e7b:efff:fe2b:884c]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-148bb55d3acsm421025ad.229.2021.12.15.09.18.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 15 Dec 2021 09:18:54 -0800 (PST) Subject: Re: [PATCH 2/2] ArmPkg: Add Library/MpInitLib to support EFI_MP_SERVICES_PROTOCOL To: Sami Mujawar , Ard Biesheuvel , Gerd Hoffmann , Samer El-Haj-Mahmoud , Leif Lindholm , devel@edk2.groups.io, nd References: <20211018153945.1690-1-rebecca@nuviainc.com> <20211018153945.1690-3-rebecca@nuviainc.com> From: "Rebecca Cran" Message-ID: Date: Wed, 15 Dec 2021 10:18:53 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/alternative; boundary="------------32796AD9858C7F786E013974" Content-Language: en-US --------------32796AD9858C7F786E013974 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi Sami, I've been looking at this again to make sure I've addressed all the issues before sending out a v2 patch series. I'm stuck on one point though: I'm not sure how it can be split into 2-3 patches without breaking bisecting. The CpuDxe and MpInitLib changes introduce a dependency on MpInitLib, so I don't see how I can put the ArmVirtPkg change into a separate commit without breaking it? -- Rebecca Cran On 10/22/21 11:51 AM, Sami Mujawar wrote: > > Hi Rebecca, > > Thanks you for this patch. > > I think this patch could be split in 2 - 3 patches (CpuDxe & MpInitLib > and the ArmVirtPkg change should be a separate patch). > > Please find my feedback inline marked [SAMI]. > > I think the usage of MPIDR and CpuInfo->ProcessorId needs to be > revisited in this patch series. I have kept my feedback on usage of > MPIDR and the MT bit [24] to give the context. But, it will be good to > only retain the affinity bits and mask all other bits of the MPIDR. > > Please let me know if you have any queries. > > Regards, > > Sami Mujawar > > > On 18/10/2021 04:39 PM, Rebecca Cran wrote: >> Add support for EFI_MP_SERVICES_PROTOCOL during the DXE phase under >> AArch64. >> >> PSCI_CPU_ON is called to power on the core, the supplied procedure is >> executed and PSCI_CPU_OFF is called to power off the core. >> >> Minimal setup is done before calling the supplied procedure: for example >> the MMU and caches are not enabled. >> >> Signed-off-by: Rebecca Cran >> --- >> ArmPkg/ArmPkg.dec | 4 + >> ArmPkg/ArmPkg.dsc | 4 + >> ArmPkg/Drivers/CpuDxe/AArch64/Arch.c | 22 + >> ArmPkg/Drivers/CpuDxe/Arm/Arch.c | 22 + >> ArmPkg/Drivers/CpuDxe/CpuDxe.c | 2 + >> ArmPkg/Drivers/CpuDxe/CpuDxe.h | 10 + >> ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 6 + >> ArmPkg/Drivers/CpuDxe/CpuMpInit.c | 604 ++++++++ >> ArmPkg/Include/Library/MpInitLib.h | 366 +++++ >> ArmPkg/Library/MpInitLib/AArch64/MpFuncs.S | 61 + >> ArmPkg/Library/MpInitLib/DxeMpInitLib.inf | 53 + >> ArmPkg/Library/MpInitLib/DxeMpLib.c | 1498 ++++++++++++++++++++ >> ArmPkg/Library/MpInitLib/InternalMpInitLib.h | 358 +++++ >> ArmVirtPkg/ArmVirt.dsc.inc | 3 + >> 14 files changed, 3013 insertions(+) --------------32796AD9858C7F786E013974 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit

Hi Sami,


I've been looking at this again to make sure I've addressed all the issues before sending out a v2 patch series. I'm stuck on one point though: I'm not sure how it can be split into 2-3 patches without breaking bisecting. The CpuDxe and MpInitLib changes introduce a dependency on MpInitLib, so I don't see how I can put the ArmVirtPkg change into a separate commit without breaking it?


--
Rebecca Cran


On 10/22/21 11:51 AM, Sami Mujawar wrote:

Hi Rebecca,

Thanks you for this patch.

I think this patch could be split in 2 - 3 patches (CpuDxe  & MpInitLib and the ArmVirtPkg change should be a separate patch).

Please find my feedback inline marked [SAMI].

I think the usage of MPIDR and CpuInfo->ProcessorId needs to be revisited in this patch series. I have kept my feedback on usage of MPIDR and the MT bit [24] to give the context. But, it will be good to only retain the affinity bits and mask all other bits of the MPIDR.

Please let me know if you have any queries.

Regards,

Sami Mujawar


On 18/10/2021 04:39 PM, Rebecca Cran wrote:
Add support for EFI_MP_SERVICES_PROTOCOL during the DXE phase under
AArch64.

PSCI_CPU_ON is called to power on the core, the supplied procedure is
executed and PSCI_CPU_OFF is called to power off the core.

Minimal setup is done before calling the supplied procedure: for example
the MMU and caches are not enabled.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 ArmPkg/ArmPkg.dec                            |    4 +
 ArmPkg/ArmPkg.dsc                            |    4 +
 ArmPkg/Drivers/CpuDxe/AArch64/Arch.c         |   22 +
 ArmPkg/Drivers/CpuDxe/Arm/Arch.c             |   22 +
 ArmPkg/Drivers/CpuDxe/CpuDxe.c               |    2 +
 ArmPkg/Drivers/CpuDxe/CpuDxe.h               |   10 +
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf             |    6 +
 ArmPkg/Drivers/CpuDxe/CpuMpInit.c            |  604 ++++++++
 ArmPkg/Include/Library/MpInitLib.h           |  366 +++++
 ArmPkg/Library/MpInitLib/AArch64/MpFuncs.S   |   61 +
 ArmPkg/Library/MpInitLib/DxeMpInitLib.inf    |   53 +
 ArmPkg/Library/MpInitLib/DxeMpLib.c          | 1498 ++++++++++++++++++++
 ArmPkg/Library/MpInitLib/InternalMpInitLib.h |  358 +++++
 ArmVirtPkg/ArmVirt.dsc.inc                   |    3 +
 14 files changed, 3013 insertions(+)
--------------32796AD9858C7F786E013974--