Re: [PATCH v2 1/2] treewide: Add a flag to detect the Apple T2 chip

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Build failure on non-x86 architectures due to an undeclared identifier in the fallback macro.
- [Low] The `apple_t2_devices` DMI match table wastes memory because it lacks the `__initconst` attribute.
- [Low] Leftover `UEFI_QUIRK_SKIP_CERT` macro is completely unused after removing the DMI table in `load_uefi.c`.
--

--- Patch [1]: [PATCH v2 1/2] treewide: Add a flag to detect the Apple T2 chip ---
commit 7dbd2538a2e2da67cae6a20bc7aa3350603d9016
Author: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

treewide: Add a flag to detect the Apple T2 chip

This commit adds a flag to detect Apple T2 chips on Intel Macs and caches
the result to avoid repeated checks. It introduces a DMI match table in x86
quirks and updates UEFI certificate loading code to use this new cached flag.

> diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
> index a92f18db96100..74b29738d4046 100644
> --- a/arch/x86/kernel/quirks.c
> +++ b/arch/x86/kernel/quirks.c
[ ... ]
> @@ -664,8 +664,113 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2083, quirk_intel_purley_xeon_ras
>  bool x86_apple_machine;
>  EXPORT_SYMBOL(x86_apple_machine);
>  
> +bool has_apple_t2_chip;
> +EXPORT_SYMBOL(has_apple_t2_chip);
> +
> +static const struct dmi_system_id apple_t2_devices[] = {

[Severity: Low]
Since apple_t2_devices is only used by early_platform_quirks() which is
marked __init, should this array be marked with __initconst so its memory
can be freed after system initialization?

> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro15,1"),
[ ... ]
> diff --git a/include/linux/platform_data/x86/apple.h b/include/linux/platform_data/x86/apple.h
> index 079e816c3c219..50bbcc5134fcb 100644
> --- a/include/linux/platform_data/x86/apple.h
> +++ b/include/linux/platform_data/x86/apple.h
> @@ -6,8 +6,13 @@
>   * x86_apple_machine - whether the machine is an x86 Apple Macintosh
>   */
>  extern bool x86_apple_machine;
> +/**
> + * has_apple_t2_chip - whether the machine has the Apple T2 chip
> + */
> +extern bool has_apple_t2_chip;
>  #else
>  #define x86_apple_machine false
> +#define has_t2_chip false

[Severity: Low]
Does this fallback macro name cause a build regression on non-x86 architectures?

The macro is named has_t2_chip here, but the generic UEFI code in
load_uefi_certs() expects has_apple_t2_chip. When CONFIG_LOAD_UEFI_KEYS is
enabled on other architectures, could this result in an undeclared identifier
error since has_apple_t2_chip would not be defined?

>  #endif
>  
>  #endif
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index c0d6948446c30..b4096d4c828d4 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
[ ... ]
> -/*
> - * On T2 Macs reading the db and dbx efi variables to load UEFI Secure Boot
> - * certificates causes occurrence of a page fault in Apple's firmware and
> - * a crash disabling EFI runtime services. The following quirk skips reading
> - * these variables.
> - */
> -static const struct dmi_system_id uefi_skip_cert[] = {
> -	{ UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,1") },

[Severity: Low]
With the removal of the uefi_skip_cert array, the UEFI_QUIRK_SKIP_CERT
macro defined in security/integrity/platform_certs/keyring_handler.h
appears to be completely unused.

Should that macro definition be cleaned up as well to prevent leftover code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721054506.11871-1-atharvatiwarilinuxdev@gmail.com?part=1