On Sat, 24 Feb 2024 at 09:31, Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote: > > > On 2/23/24 5:24 AM, Ilias Apalodimas wrote: > > Apologies for the late reply, > > > > > > On Mon, 19 Feb 2024 at 09:34, Kuppuswamy Sathyanarayanan > > <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote: > >> Hi Ilias, > >> > >> On 2/18/24 11:03 PM, Ilias Apalodimas wrote: > >>> On Thu, 15 Feb 2024 at 05:02, Kuppuswamy Sathyanarayanan > >>> <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote: > >>>> To allow event log info access after boot, EFI boot stub extracts > >>>> the event log information and installs it in an EFI configuration > >>>> table. Currently, EFI boot stub only supports installation of event > >>>> log only for TPM 1.2 and TPM 2.0 protocols. Extend the same support > >>>> for CC protocol. Since CC platform also uses TCG2 format, reuse TPM2 > >>>> support code as much as possible. > >>>> > >>>> Link: https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#efi-cc-measurement-protocol [1] > >>>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> > >>> [...] > >>> > >>>> +void efi_retrieve_eventlog(void) > >>>> +{ > >>>> + efi_physical_addr_t log_location = 0, log_last_entry = 0; > >>>> + efi_guid_t cc_guid = EFI_CC_MEASUREMENT_PROTOCOL_GUID; > >>>> + efi_guid_t tpm2_guid = EFI_TCG2_PROTOCOL_GUID; > >>>> + int version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2; > >>>> + efi_tcg2_protocol_t *tpm2 = NULL; > >>>> + efi_cc_protocol_t *cc = NULL; > >>>> + efi_bool_t truncated; > >>>> + efi_status_t status; > >>>> + > >>>> + status = efi_bs_call(locate_protocol, &tpm2_guid, NULL, (void **)&tpm2); > >>>> + if (status == EFI_SUCCESS) { > >>>> + status = efi_call_proto(tpm2, get_event_log, version, &log_location, > >>>> + &log_last_entry, &truncated); > >>>> + > >>>> + if (status != EFI_SUCCESS || !log_location) { > >>>> + version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; > >>>> + status = efi_call_proto(tpm2, get_event_log, version, > >>>> + &log_location, &log_last_entry, > >>>> + &truncated); > >>>> + if (status != EFI_SUCCESS || !log_location) > >>>> + return; > >>>> + } > >>>> + > >>>> + efi_retrieve_tcg2_eventlog(version, log_location, log_last_entry, > >>>> + truncated); > >>>> + return; > >>>> + } > >>>> + > >>>> + status = efi_bs_call(locate_protocol, &cc_guid, NULL, (void **)&cc); > >>>> + if (status == EFI_SUCCESS) { > >>>> + version = EFI_CC_EVENT_LOG_FORMAT_TCG_2; > >>>> + status = efi_call_proto(cc, get_event_log, version, &log_location, > >>>> + &log_last_entry, &truncated); > >>>> + if (status != EFI_SUCCESS || !log_location) > >>>> + return; > >>>> + > >>>> + efi_retrieve_tcg2_eventlog(version, log_location, log_last_entry, > >>>> + truncated); > >>>> + return; > >>>> + } > >>>> +} > >>> [...] > >>> > >>> I haven't looked into CC measurements much, but do we always want to > >>> prioritize the tcg2 protocol? IOW if you have firmware that implements > >>> both, shouldn't we prefer the CC protocol for VMs? > >> According the UEFI specification, sec "Conidential computing", if a firmware implements > >> the TPM, then it should be used and CC interfaces should not be published. So I think > >> we should check for TPM first, if it does not exist then try for CC. > > Ok thanks, that makes sense. That document also says the services > > should be implemented on a virtual firmware. > > I am unsure at the moment though if it's worth checking that and > > reporting an error otherwise. Thoughts? > > IMO, it is not fatal for the firmware to implement both protocols. Although, it > violates the specification, does it makes sense to return error and skip > measurements? I think for such case, we can add a warning and proceed > with TPM if it exists. If you have a TPM, the current code wouldn't even look for CC (which we agreed is correct). The question is, should we care if a firmware exposes the CC protocol, but isn't virtualized Thanks /Ilias > > > > > Thanks > > /Ilias > >> https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#confidential-computing > >> > >>> Thanks > >>> /Ilias > >> -- > >> Sathyanarayanan Kuppuswamy > >> Linux Kernel Developer > >> > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer >
