Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 12 May 2023 12:26:23 +0100
From:   Matthew Garrett <mjg59@srcf.ucam.org>
To:     Ross Philipson <ross.philipson@oracle.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org,
        linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-crypto@vger.kernel.org, iommu@lists.linux-foundation.org,
        kexec@lists.infradead.org, linux-efi@vger.kernel.org,
        dpsmith@apertussolutions.com, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, hpa@zytor.com, ardb@kernel.org,
        James.Bottomley@hansenpartnership.com, luto@amacapital.net,
        nivedita@alum.mit.edu, kanth.ghatraju@oracle.com,
        trenchboot-devel@googlegroups.com
Subject: Re: [PATCH v6 07/14] x86: Secure Launch kernel early boot stub
Message-ID: <20230512112623.GE14461@srcf.ucam.org>
References: <20230504145023.835096-1-ross.philipson@oracle.com>
 <20230504145023.835096-8-ross.philipson@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230504145023.835096-8-ross.philipson@oracle.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Thu, May 04, 2023 at 02:50:16PM +0000, Ross Philipson wrote:

> +static void sl_find_event_log(struct slr_table *slrt)

If this is called after the EFI stub then we're presumably 
post-ExitBootServices and we're copied the TPM event log into a 
configuration table so it's available to the runtime kernel. That also 
means that we should be adding all further measurements to the Final 
Events Table rather than the initial event log. How's that handled here, 
both in terms of ensuring further events (generated by firmware or by 
us) get added to the right place, and in terms of ensuring the event 
logs the kernel has later on were covered appropriately? Or is the SL 
event log an entirely different thing that can be merged in later 
because it only covers the DRTM PCRs?

> +static void sl_extend_setup_data(struct slr_policy_entry *entry)
> +{
> +	struct setup_data *data;
> +
> +	/*
> +	 * Measuring the boot params measured the fixed e820 memory map.
> +	 * Measure any setup_data entries including e820 extended entries.
> +	 */
> +	data = (struct setup_data *)(unsigned long)entry->entity;
> +	while (data)
> +		data = sl_handle_setup_data(data, entry);
> +}

Is e820 sufficient here? There are cases where we use the EFI memory map 
directly (sorry), but I don't know if any of them are relevant to DRTM 
outcomes.