stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: jpiotrowski@linux.microsoft.com, Ard Biesheuvel <ardb@kernel.org>
Cc: stable@vger.kernel.org, dpark@linux.microsoft.com,
	t-lo@linux.microsoft.com, Sasha Levin <sashal@kernel.org>
Subject: Re: [PATCH 6.1] arm64: efi: Use SMBIOS processor version to key off Ampere quirk
Date: Wed, 7 Jun 2023 20:36:43 +0200	[thread overview]
Message-ID: <2023060719-uncertain-implant-dede@gregkh> (raw)
In-Reply-To: <20230607122612.GA846@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>

On Wed, Jun 07, 2023 at 05:26:12AM -0700, Ard Biesheuvel wrote:
> [ Upstream commit eb684408f3ea4856639675d6465f0024e498e4b1 ]
> 
> Instead of using the SMBIOS type 1 record 'family' field, which is often
> modified by OEMs, use the type 4 'processor ID' and 'processor version'
> fields, which are set to a small set of probe-able values on all known
> Ampere EFI systems in the field.
> 
> Fixes: 550b33cfd4452968 ("arm64: efi: Force the use of ...")
> Tested-by: Andrea Righi <andrea.righi@canonical.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

Where did Sasha sign off on this?

> Signed-off-by: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
> ---
>  drivers/firmware/efi/libstub/arm64-stub.c | 39 ++++++++++++++++-----
>  drivers/firmware/efi/libstub/efistub.h    | 41 +++++++++++++++++++++--
>  drivers/firmware/efi/libstub/smbios.c     | 13 +++++--
>  3 files changed, 80 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index 42282c5c3fe6..e2f90566b291 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -17,20 +17,43 @@
>  
>  static bool system_needs_vamap(void)
>  {
> -	const u8 *type1_family = efi_get_smbios_string(1, family);
> +	const struct efi_smbios_type4_record *record;
> +	const u32 __aligned(1) *socid;
> +	const u8 *version;
>  
>  	/*
>  	 * Ampere eMAG, Altra, and Altra Max machines crash in SetTime() if
> -	 * SetVirtualAddressMap() has not been called prior.
> +	 * SetVirtualAddressMap() has not been called prior. Most Altra systems
> +	 * can be identified by the SMCCC soc ID, which is conveniently exposed
> +	 * via the type 4 SMBIOS records. Otherwise, test the processor version
> +	 * field. eMAG systems all appear to have the processor version field
> +	 * set to "eMAG".
>  	 */
> -	if (!type1_family || (
> -	    strcmp(type1_family, "eMAG") &&
> -	    strcmp(type1_family, "Altra") &&
> -	    strcmp(type1_family, "Altra Max")))
> +	record = (struct efi_smbios_type4_record *)efi_get_smbios_record(4);
> +	if (!record)
>  		return false;
>  
> -	efi_warn("Working around broken SetVirtualAddressMap()\n");
> -	return true;
> +	socid = (u32 *)record->processor_id;
> +	switch (*socid & 0xffff000f) {
> +		static char const altra[] = "Ampere(TM) Altra(TM) Processor";
> +		static char const emag[] = "eMAG";
> +
> +	default:
> +		version = efi_get_smbios_string(&record->header, 4,
> +						processor_version);
> +		if (!version || (strncmp(version, altra, sizeof(altra) - 1) &&
> +				 strncmp(version, emag, sizeof(emag) - 1)))
> +			break;
> +
> +		fallthrough;
> +
> +	case 0x0a160001:	// Altra
> +	case 0x0a160002:	// Altra Max
> +		efi_warn("Working around broken SetVirtualAddressMap()\n");
> +		return true;
> +	}
> +
> +	return false;
>  }
>  
>  efi_status_t check_platform_features(void)
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 900df67a2078..970e86e3aab0 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -983,6 +983,8 @@ struct efi_smbios_record {
>  	u16	handle;
>  };
>  
> +const struct efi_smbios_record *efi_get_smbios_record(u8 type);
> +
>  struct efi_smbios_type1_record {
>  	struct efi_smbios_record	header;
>  
> @@ -996,13 +998,46 @@ struct efi_smbios_type1_record {
>  	u8				family;
>  };
>  
> -#define efi_get_smbios_string(__type, __name) ({			\
> +struct efi_smbios_type4_record {
> +	struct efi_smbios_record	header;
> +
> +	u8				socket;
> +	u8				processor_type;
> +	u8				processor_family;
> +	u8				processor_manufacturer;
> +	u8				processor_id[8];
> +	u8				processor_version;
> +	u8				voltage;
> +	u16				external_clock;
> +	u16				max_speed;
> +	u16				current_speed;
> +	u8				status;
> +	u8				processor_upgrade;
> +	u16				l1_cache_handle;
> +	u16				l2_cache_handle;
> +	u16				l3_cache_handle;
> +	u8				serial_number;
> +	u8				asset_tag;
> +	u8				part_number;
> +	u8				core_count;
> +	u8				enabled_core_count;
> +	u8				thread_count;
> +	u16				processor_characteristics;
> +	u16				processor_family2;
> +	u16				core_count2;
> +	u16				enabled_core_count2;
> +	u16				thread_count2;
> +	u16				thread_enabled;
> +};
> +
> +#define efi_get_smbios_string(__record, __type, __name) ({		\
>  	int size = sizeof(struct efi_smbios_type ## __type ## _record);	\
>  	int off = offsetof(struct efi_smbios_type ## __type ## _record,	\
>  			   __name);					\
> -	__efi_get_smbios_string(__type, off, size);			\
> +	__efi_get_smbios_string((__record), __type, off, size);		\
>  })
>  
> -const u8 *__efi_get_smbios_string(u8 type, int offset, int recsize);
> +const u8 *__efi_get_smbios_string(const struct efi_smbios_record *record,
> +				  u8 type, int offset, int recsize);
>  
>  #endif
> diff --git a/drivers/firmware/efi/libstub/smbios.c b/drivers/firmware/efi/libstub/smbios.c
> index aadb422b9637..f9c159c28f46 100644
> --- a/drivers/firmware/efi/libstub/smbios.c
> +++ b/drivers/firmware/efi/libstub/smbios.c
> @@ -22,19 +22,28 @@ struct efi_smbios_protocol {
>  	u8 minor_version;
>  };
>  
> -const u8 *__efi_get_smbios_string(u8 type, int offset, int recsize)
> +const struct efi_smbios_record *efi_get_smbios_record(u8 type)
>  {
>  	struct efi_smbios_record *record;
>  	efi_smbios_protocol_t *smbios;
>  	efi_status_t status;
>  	u16 handle = 0xfffe;
> -	const u8 *strtable;
>  
>  	status = efi_bs_call(locate_protocol, &EFI_SMBIOS_PROTOCOL_GUID, NULL,
>  			     (void **)&smbios) ?:
>  		 efi_call_proto(smbios, get_next, &handle, &type, &record, NULL);
>  	if (status != EFI_SUCCESS)
>  		return NULL;
> +	return record;
> +}
> +
> +const u8 *__efi_get_smbios_string(const struct efi_smbios_record *record,
> +				  u8 type, int offset, int recsize)
> +{
> +	const u8 *strtable;
> +
> +	if (!record)
> +		return NULL;
>  
>  	strtable = (u8 *)record + record->length;
>  	for (int i = 1; i < ((u8 *)record)[offset]; i++) {
> -- 
> 2.30.2
> 

  parent reply	other threads:[~2023-06-07 18:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-06 14:47 soft lockup on Ampere Altra on v6.1.30 Jeremi Piotrowski
2023-06-06 17:40 ` Greg KH
2023-06-07 12:26   ` [PATCH 6.1] arm64: efi: Use SMBIOS processor version to key off Ampere quirk Ard Biesheuvel
2023-06-07 12:50     ` Jeremi Piotrowski
2023-06-07 18:36     ` Greg KH [this message]
2023-06-08  7:36       ` Jeremi Piotrowski
2023-06-08  7:44         ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2023060719-uncertain-implant-dede@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=ardb@kernel.org \
    --cc=dpark@linux.microsoft.com \
    --cc=jpiotrowski@linux.microsoft.com \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=t-lo@linux.microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).