On 2017-04-18 14:55, Andy Shevchenko wrote:
> On Tue, Apr 18, 2017 at 3:48 PM, Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx> wrote:
>> On Wed, 05 Apr, at 10:23:17AM, Ard Biesheuvel wrote:
>
>>> The firmware for Quark X102x prepends a security header to the capsule
>>> which is needed to support the mandatory secure boot on this processor.
>>> The header can be detected by checking for the "_CSH" signature and -
>>> to avoid any GUID conflict - validating its size field to contain the
>>> expected value. Then we need to look for the EFI header right after the
>>> security header and pass the real header to __efi_capsule_setup_info.
>>>
>>> To be minimally invasive and maximally safe, the quirk version of
>>> efi_capsule_identify_image is only effective on Quark processors.
>
>>> +#ifdef CONFIG_EFI_CAPSULE_QUIRK_QUARK_CSH
>>> +
>>> +static const struct x86_cpu_id quark_ids[] = {
>>> + { X86_VENDOR_INTEL, 5, 9 }, /* Intel Quark X1000 */
>>> + { }
>>> +};
>>> +
>>> +int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
>>> + size_t hdr_bytes)
>>> +{
>>> + struct quark_security_header *csh = kbuff;
>>> +
>>> + cap_info->total_size = 0;
>>> +
>>> + if (!x86_match_cpu(quark_ids))
>>> + goto fallback;
>>> +
>>
>> I'd prefer to see the quark quirk pulled out into its own function and
>> referenced from the __weak efi_capsule_setup_info() function, which
>> makes it easier to people to read the EFI capsule code flow if they're
>> not interested in the Quark quick.
>>
>> Something like this,
>>
>> int efi_capsule_setup_info(...)
>> {
>> ...
>>
>> if (x86_match_cpu(quark_ids))
>> return efi_capsule_quark_setup_quirk(cap_info, kbuff, hdr_bytes);
>
> Previously I had proposed to use driver_data field
>
> Something like
>
> struct _info {
> int(*setup)(...);
> }; <<< this is optional, might be useful in the future, otherwise just
> simple typedef.
>
> int efi_capsule_quark_setup_quirk(...)
> {
> }
>
> struct _info efi_capsule_quark_info = {
> .setup = ..._setup_quirk,
> };
>
> #define ICPU(family, model, ddata) ...
>
> static const struct x86_cpu_id quark_ids[] = {
> ICPU(5, 9, efi_capsule_quark_info),
> {}
> };
>
> Though didn't hear back on it.
>
I've implemented this, but for the old design, and Ard took over then.
So it never made it to the list.
Whatever layout of these bits is preferred, it can probably be done. I
just need an indication that there is (likely) a consensus.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
