| Message ID | a968c446bde75bf019580366854349bf94e6c961.1675897882.git.darren@os.amperecomputing.com |
|---|---|
| State | Accepted |
| Commit | 190233164cd77115f8dea718cbac561f557092c6 |
| Headers | show |
| Series | arm64: efi: Force the use of SetVirtualAddressMap() on Altra Max machines | expand |
Hello Darren, On Thu, 9 Feb 2023 at 00:14, Darren Hart <darren@os.amperecomputing.com> wrote: > > Commit 550b33cfd445 ("arm64: efi: Force the use of SetVirtualAddressMap() > on Altra machines") identifies the Altra family via the family field in > the type#1 SMBIOS record. Altra Max machines are similarly affected but > not detected with the strict strcmp test. > > Rather than risk greedy matching with strncmp, add a second test for > Altra Max. Do not refactor to handle multiple tests as these should be > the only two needed. > Famous last words ... Unfortunately, I just had a report the other day that 'eMAG' and 'Server' (!) are also being used. https://lore.kernel.org/all/20230131040355.3116-1-justin.he@arm.com/ > Fixes: 550b33cfd445 ("arm64: efi: Force the use of SetVirtualAddressMap() on Altra machines") > Cc: <stable@vger.kernel.org> # 6.1.x > Cc: <linux-efi@vger.kernel.org> > Cc: Alexandru Elisei <alexandru.elisei@gmail.com> > Cc: Ard Biesheuvel <ardb@kernel.org> > Signed-off-by: Darren Hart <darren@os.amperecomputing.com> > --- > drivers/firmware/efi/libstub/arm64.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/firmware/efi/libstub/arm64.c b/drivers/firmware/efi/libstub/arm64.c > index ff2d18c42ee7..97f4423059c7 100644 > --- a/drivers/firmware/efi/libstub/arm64.c > +++ b/drivers/firmware/efi/libstub/arm64.c > @@ -19,10 +19,10 @@ static bool system_needs_vamap(void) > const u8 *type1_family = efi_get_smbios_string(1, family); > > /* > - * Ampere Altra machines crash in SetTime() if SetVirtualAddressMap() > - * has not been called prior. > + * Ampere Altra and Altra Max machines crash in SetTime() if > + * SetVirtualAddressMap() has not been called prior. > */ > - if (!type1_family || strcmp(type1_family, "Altra")) > + if (!type1_family || (strcmp(type1_family, "Altra") && strcmp(type1_family, "Altra Max"))) > return false; > > efi_warn("Working around broken SetVirtualAddressMap()\n"); > -- > 2.34.3 >
On Thu, 9 Feb 2023 at 00:43, Darren Hart <darren@os.amperecomputing.com> wrote: > > On Thu, Feb 09, 2023 at 12:16:46AM +0100, Ard Biesheuvel wrote: > > Hello Darren, > > > > On Thu, 9 Feb 2023 at 00:14, Darren Hart <darren@os.amperecomputing.com> wrote: > > > > > > Commit 550b33cfd445 ("arm64: efi: Force the use of SetVirtualAddressMap() > > > on Altra machines") identifies the Altra family via the family field in > > > the type#1 SMBIOS record. Altra Max machines are similarly affected but > > > not detected with the strict strcmp test. > > > > > > Rather than risk greedy matching with strncmp, add a second test for > > > Altra Max. Do not refactor to handle multiple tests as these should be > > > the only two needed. > > > > > > > Famous last words ... > > Indeed, I nearly included that myself... > > > > > Unfortunately, I just had a report the other day that 'eMAG' and > > 'Server' (!) are also being used. > > > > https://lore.kernel.org/all/20230131040355.3116-1-justin.he@arm.com/ > > > > OK, so in order to workaround this in the kernel, we need a better way to match. > Unfortunately, this is specific to the oem platform, and the oem controls those > strings. > > Thanks for the pointer, will go mull this over and see if I can come up with > something better. > > In the meantime, would you consider matching on: > Altra > Altra Max > eMAG > > to capture the bulk of the systems until we have a better solution? > Yeah that's fine.
diff --git a/drivers/firmware/efi/libstub/arm64.c b/drivers/firmware/efi/libstub/arm64.c index ff2d18c42ee7..97f4423059c7 100644 --- a/drivers/firmware/efi/libstub/arm64.c +++ b/drivers/firmware/efi/libstub/arm64.c @@ -19,10 +19,10 @@ static bool system_needs_vamap(void) const u8 *type1_family = efi_get_smbios_string(1, family); /* - * Ampere Altra machines crash in SetTime() if SetVirtualAddressMap() - * has not been called prior. + * Ampere Altra and Altra Max machines crash in SetTime() if + * SetVirtualAddressMap() has not been called prior. */ - if (!type1_family || strcmp(type1_family, "Altra")) + if (!type1_family || (strcmp(type1_family, "Altra") && strcmp(type1_family, "Altra Max"))) return false; efi_warn("Working around broken SetVirtualAddressMap()\n");
Commit 550b33cfd445 ("arm64: efi: Force the use of SetVirtualAddressMap() on Altra machines") identifies the Altra family via the family field in the type#1 SMBIOS record. Altra Max machines are similarly affected but not detected with the strict strcmp test. Rather than risk greedy matching with strncmp, add a second test for Altra Max. Do not refactor to handle multiple tests as these should be the only two needed. Fixes: 550b33cfd445 ("arm64: efi: Force the use of SetVirtualAddressMap() on Altra machines") Cc: <stable@vger.kernel.org> # 6.1.x Cc: <linux-efi@vger.kernel.org> Cc: Alexandru Elisei <alexandru.elisei@gmail.com> Cc: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Darren Hart <darren@os.amperecomputing.com> --- drivers/firmware/efi/libstub/arm64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)