From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web09.8295.1607046913797340672 for ; Thu, 03 Dec 2020 17:55:14 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=EQu7XvbI; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607046913; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vbIMnDWbTYEFILPOjI3R6RE5Nw2+o4n6J8u+GsfUmIs=; b=EQu7XvbIlyVU5ZcDVZBiQbJVl0TZ76SUukNLMDlH2LoN1rPG8lERmzUPRIjMdLLzZbjM6G Z4fAvDVyMnxtpn6jWs2qsCa9UVwPYdGc6yB4W/LvTW7DBV+ZbD4Sn3qxDvDPjhN59dEoJS cHeU0IEcx30dW1HzHgI62DiNL/W9yxw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-402-Mw_Owp00MlaHCNPtoNLgaQ-1; Thu, 03 Dec 2020 20:55:09 -0500 X-MC-Unique: Mw_Owp00MlaHCNPtoNLgaQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F35501005513; Fri, 4 Dec 2020 01:55:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-157.ams2.redhat.com [10.36.114.157]) by smtp.corp.redhat.com (Postfix) with ESMTP id E31BD1A8A3; Fri, 4 Dec 2020 01:55:03 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 0/6] SEV Encrypted Boot for Ovmf To: jejb@linux.ibm.com, devel@edk2.groups.io Cc: dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com, ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com, david.kaplan@amd.com, jon.grimm@amd.com, thomas.lendacky@amd.com, frankeh@us.ibm.com, "Dr . David Alan Gilbert" , Jordan Justen , Ard Biesheuvel References: <20201130202819.3910-1-jejb@linux.ibm.com> <0805f171-b5c2-a556-3e64-c700aaf06d85@redhat.com> <762be18c6132f0f55e029879931ba6bca79323cd.camel@linux.ibm.com> From: "Laszlo Ersek" Message-ID: <18bbe7d1-a51a-647e-d05a-73e5465d31cc@redhat.com> Date: Fri, 4 Dec 2020 02:55:02 +0100 MIME-Version: 1.0 In-Reply-To: <762be18c6132f0f55e029879931ba6bca79323cd.camel@linux.ibm.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/04/20 02:05, James Bottomley wrote: > On Fri, 2020-12-04 at 01:46 +0100, Laszlo Ersek wrote: >> On 12/03/20 15:27, James Bottomley wrote: >>> On Thu, 2020-12-03 at 13:26 +0100, Laszlo Ersek wrote: >>>> Hi James, >>>> >>>> On 11/30/20 21:28, James Bottomley wrote: >>>>> v3: >>>>> >>>>> - More grub and boot stripping (I think I got everything out, >>>>> but there may be something that strayed in the boot panic >>>>> resolution). >>>>> - grub.sh tidy up with tabs->spaces. >>>>> - Move the reset vector GUIDisation patch to the front so it >>>>> can be applied independently >>>>> - Update the .dsc and .fdf files for variable policy >>>> >>>> In preparation for submitting the github PR for merging this >>>> series, I first ran PatchCheck.py locally. >>>> >>>> It doesn't like that I converted "OvmfPkg/AmdSev/Grub/grub.cfg" >>>> to LF, in addition to "OvmfPkg/AmdSev/Grub/grub.sh". >>>> >>>> PatchCheck.py recognizes the ".sh" suffix, so it's not >>>> complaining about "OvmfPkg/AmdSev/Grub/grub.sh". But, the config >>>> file is a problem. >>>> >>>> Can you please confirm that grub works fine if the config file >>>> has CRLF line terminators? Because then I'll just convert the >>>> config file back to CRLF, and merge the series. >>> >>> I converted the entire file with unix2dos and grub does seem to be >>> fine with the CRLF line endings (probably because it wants to be a >>> boot loader beyond linux), so I think converting the file to CRLF >>> is the right way to go. >> >> I submitted PR ;, but it >> was rejected due to ECC failures. >> >> I think we all need to have a serious talk about ECC. I'll send a >> separate email. >> >> I'm really sorry. > > Is it just complaining that gGrubFileGuid is the same as the FILE_GUID > in Grub.inf (as the comment in Grub.inf says) but it shouldn't be, so > the fix is to remove the comment and generate a new FILE_GUID for > Grub.inf? > > James > > Log files (same content, modulo timestamps etc): https://dev.azure.com/tianocore/11ea4a10-ac9f-4e5f-8b13-7def1f19d478/_apis/build/builds/16071/logs/115 https://dev.azure.com/tianocore/11ea4a10-ac9f-4e5f-8b13-7def1f19d478/_apis/build/builds/16072/logs/113 Two plugins failed (search either log for the string "Test Failed" -- there's going to be two instances). (1) The GUID check plugin is one of both failures. It's unjustified of course, because you *need* to know the FILE_GUID of Grub.inf -- that's how you can generate boot option to it in the Platform BDS library. Making gGrubFileGuid different from what's in FILE_GUID would *break the code*. One hack for this is to change the FILE_GUID in Grub.inf (regenerate it with uuidgen), but then use a DSC-level FILE_GUID override, to make it match gGrubFileGuid from the DEC file again: > diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc > index bb7697eb324b..1593856faca1 100644 > --- a/OvmfPkg/AmdSev/AmdSevX64.dsc > +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc > @@ -779,7 +779,10 @@ [Components] > } > !endif > OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf > - OvmfPkg/AmdSev/Grub/Grub.inf > + OvmfPkg/AmdSev/Grub/Grub.inf { > + > + FILE_GUID = ... > + } > !if $(BUILD_SHELL) == TRUE > ShellPkg/Application/Shell/Shell.inf { > But it's terrible for code that's actually valid. So instead, we should add an exception to "OvmfPkg/OvmfPkg.ci.yaml", near "GuidCheck". (2) The other failure is from ECC. I've evaluated the errors it has reported now, and all of them are unjustifiedly reported. Again, more exceptions are needed in "OvmfPkg/OvmfPkg.ci.yaml", this time near "EccCheck". I will send a short patch series to add the exceptions, and once that's upstream, we *will* merge this (v3) series. Thanks Laszlo