Messages in this thread | | | Date | Thu, 3 Aug 2023 19:09:34 +0200 | Subject | Re: [PATCH v5 3/3] firmware: Add support for Qualcomm UEFI Secure Application | From | Maximilian Luz <> |
| |
On 8/3/23 17:44, Ard Biesheuvel wrote: > On Sun, 30 Jul 2023 at 18:19, Maximilian Luz <luzmaximilian@gmail.com> wrote:
[...]
>> +/* -- Driver setup. --------------------------------------------------------- */ >> + >> +static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev, >> + const struct auxiliary_device_id *aux_dev_id) >> +{ >> + struct qcuefi_client *qcuefi; >> + int status; >> + >> + qcuefi = devm_kzalloc(&aux_dev->dev, sizeof(*qcuefi), GFP_KERNEL); >> + if (!qcuefi) >> + return -ENOMEM; >> + >> + qcuefi->client = container_of(aux_dev, struct qseecom_client, aux_dev); >> + >> + auxiliary_set_drvdata(aux_dev, qcuefi); >> + status = qcuefi_set_reference(qcuefi); >> + if (status) >> + return status; >> + >> + status = efivars_register(&qcuefi->efivars, &qcom_efivar_ops); > > Will this also work if the EFI runtime services were already > registered by the time we reach this point?
That's actually a good question. In short: No. However, let me explain that a bit:
First, we assume that we're the only other non-generic provider (arguably, multiple non-generic providers don't make much sense on a single platform anyway, so I'd say in that case it's okay to fail here).
Second, we assume that the generic ops are not going to be registered at all on the platforms that this implementation is used. In particular, on the platforms I've tested and heard reports from so far, "standard" efivars either aren't actively advertised as "supported" or they return EFI_UNSUPPORTED for all calls. So we assume that either the check in efisubsys_init() or in generic_ops_supported() prevents registration of the generic ops.
Further, I'd hope that the uefisecapp would not be loaded if generic ops would be supported on such a platform, thus preventing instantiation of the respective client device.
So the only issue that I can see is that if uefisecapp is loaded and generic ops are supported, we would need a way to choose one over the other. But I think that is fairly unlikely to happen and I think it would probably be best to sort that out then (e.g. by refusing to load this new driver with some additional check).
Apart from that case, there should not be any timing issues that could cause registration to fail spuriously.
Regards Max
| |