Re: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/29/24 6:28 AM, Ard Biesheuvel wrote:
On Thu, 29 Aug 2024 at 15:24, Jonathan McDowell <noodles@xxxxxxxx> wrote:

On Wed, Aug 28, 2024 at 01:19:16PM -0700, ross.philipson@xxxxxxxxxx wrote:
On 8/28/24 10:14 AM, Ard Biesheuvel wrote:
On Wed, 28 Aug 2024 at 19:09, kernel test robot <lkp@xxxxxxxxx> wrote:

Hi Ross,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5]
[cannot apply to herbert-crypto-2.6/master next-20240828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch*_base_tree_information__;Iw!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIxuz-LAC$ ]

url:    https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI7Z6SQKy$
base:   tip/x86/core
patch link:    https://urldefense.com/v3/__https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson*40oracle.com__;JQ!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIzWfs1XZ$
patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch
config: i386-randconfig-062-20240828 (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@xxxxxxxxx/config__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIwkYG0TY$ )


This is a i386 32-bit build, which makes no sense: this stuff should
just declare 'depends on 64BIT'

Our config entry already has 'depends on X86_64' which in turn depends on
64BIT. I would think that would be enough. Do you think it needs an explicit
'depends on 64BIT' in our entry as well?

The error is in x86-stub.c, which is pre-existing and compiled for 32
bit as well, so you need more than a "depends" here.


Ugh, that means this is my fault - apologies. Replacing the #ifdef
with IS_ENABLED() makes the code visible to the 32-bit compiler, even
though the code is disregarded.

I'd still prefer IS_ENABLED(), but this would require the code in
question to live in a separate compilation unit (which depends on
CONFIG_SECURE_LAUNCH). If that is too fiddly, feel free to bring back
the #ifdef CONFIG_SECURE_LAUNCH here (and retain my R-b)

Thanks to both of you for the followup.

If there was a very large amount of new code here, I would consider making a separate compilation unit. I can't really say if this is a wider issue with the the EFI libstub code but if it ware broken up later, it would make sense to move our bits to where 64bit only code lives.

Given that it is a small block of code though, I am inclined to just go back to #ifdef's for now.

Thanks
Ross




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux