From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.261.1672941611877779746 for ; Thu, 05 Jan 2023 10:00:12 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=clUuKBto; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D17EB61BD5 for ; Thu, 5 Jan 2023 18:00:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B20B7C43392 for ; Thu, 5 Jan 2023 18:00:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1672941609; bh=quIx4wtKy9Q9MRQd5KtbvQt+e7aWzmhxEkDdXK5lUpU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=clUuKBtotJvPk1jRP84EWemkM0swey0F1cjo9CoK61aoVPN1xGQ2VL40J98BbO5hg 24PjGfHdVXh4N2IAwR8KFZFb232V+XjRiUDaoti2OLKmb/SXL9QjpgxliMqUWIMeRq eRpT7WAR1I8b/yy0jy+WYs4yfbiDjo7in403mbNjRy8GpNFEHDcOuReR5Hel1QhS/H Xc7kMjx5sywPZsmOJy9xSo+OqTEgEMMHNBojzCj+YIio8si8mob2TordHiRaLlBhSo OuSkPUjp81VjIJLh6cJYABS+II2NRv5EIV3ac2xATO8RjH4GhtLvdPNJWMZTDH3la2 zx+Jt09KGCWPQ== Received: by mail-lf1-f54.google.com with SMTP id z26so56082323lfu.8 for ; Thu, 05 Jan 2023 10:00:09 -0800 (PST) X-Gm-Message-State: AFqh2kp4imaN5I9PaB8M0QbCq+Y6/SChZPBCsFvHCLVhoHFETji3QAzm vvsTH/O9B3V+oZslPCtxzpPGgC2H7bmmV4HtkTU= X-Google-Smtp-Source: AMrXdXu2lyj/Ef9t7KLv53AjmM5h4xBVWsD3vVmOTkaQEnW+pTQb0q0o2gHZDzmm4K4x60zTFgyPdDJEwXtt5hb4e9s= X-Received: by 2002:ac2:5d4e:0:b0:4b5:964d:49a4 with SMTP id w14-20020ac25d4e000000b004b5964d49a4mr4487465lfd.637.1672941607632; Thu, 05 Jan 2023 10:00:07 -0800 (PST) MIME-Version: 1.0 References: <20230104153727.345236-1-rebecca@quicinc.com> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 5 Jan 2023 18:59:56 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 0/3] ArmPkg,MdeModulePkg: Implement EFI_MP_SERVICES_PROTOCOL for AArch64 and add an MpServicesTest application to exercise it To: Rebecca Cran Cc: devel@edk2.groups.io, Sami Mujawar , Ard Biesheuvel , Leif Lindholm , Jian J Wang , Liming Gao , Tiger Liu Content-Type: text/plain; charset="UTF-8" On Thu, 5 Jan 2023 at 18:39, Ard Biesheuvel wrote: > > On Wed, 4 Jan 2023 at 16:37, Rebecca Cran wrote: > > > > Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, and > > add an MpServicesTest application to exercise it. > > > > Changes from v3: > > > > Removed Ard's 'Reviewed-by' line from the commits since the code has changed > > sufficiently that it's no longer valid. > > > > Rebecca Cran (3): > > ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to ArmLib.h > > ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls > > MdeModulePkg: Add new Application/MpServicesTest application > > > > ArmPkg/ArmPkg.dsc | 1 + > > MdeModulePkg/MdeModulePkg.dsc | 2 + > > ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf | 56 + > > MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf | 40 + > > ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h | 344 ++++ > > ArmPkg/Include/Library/ArmLib.h | 16 +- > > MdeModulePkg/Application/MpServicesTest/Options.h | 39 + > > ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c | 1847 ++++++++++++++++++++ > > MdeModulePkg/Application/MpServicesTest/MpServicesTest.c | 560 ++++++ > > MdeModulePkg/Application/MpServicesTest/Options.c | 164 ++ > > ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S | 57 + > > 11 files changed, 3119 insertions(+), 7 deletions(-) > > Hello Rebecca, > > Thanks for reposting this. > > Running the secondaries with MMU and caches on is a huge improvement. > I would suggest, though, that we enable the MMU first thing out of > reset, and do so from asm code so we don't have to reason about the > stack (pushing something with the MMU off and popping it with the MMU > on requires cache maintenance as well, and there is no way this can be > done from the code itself without help from the compiler) > > So I propose adding the diff below - note that only the variables > holding TCR, MAIR and TTBR0 need cache maintenance now (in addition to > the executable image) - everything else will be accessed by the > secondaries with the MMU enabled. > > https://paste.debian.net/1266242/ > > WIth a tweak to the RPI4 platform that I sent out separately, this all > works fine for me (both with and without the diff above applied) > Actually, maybe better to retain the hunk below. I saw some weird hangs without them diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c index ab439bffd722..eb634a25b33a 100644 --- a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c +++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c @@ -1345,18 +1345,6 @@ MpServicesInitialize ( gProcessorIDs[Index] = MAX_UINT64; - // - // The global pointer variables as well as the gProcessorIDs array contents - // are accessed by the other cores so we must clean them to the PoC - // - WriteBackDataCacheRange (&gProcessorIDs, sizeof (UINT64 *)); - WriteBackDataCacheRange (&gApStacksBase, sizeof (UINT64 *)); - - WriteBackDataCacheRange ( - gProcessorIDs, - (mCpuMpData.NumberOfProcessors + 1) * sizeof (UINT64) - ); - mNonBlockingModeAllowed = TRUE; Status = EfiCreateEventReadyToBootEx (