Re: [PATCH v6 10/11] virt: arm-cca-guest: TSM_REPORT support for realms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/10/2024 07:06, Gavin Shan wrote:
On 10/12/24 2:22 AM, Suzuki K Poulose wrote:
On 11/10/2024 15:14, Steven Price wrote:
On 08/10/2024 05:12, Gavin Shan wrote:
On 10/5/24 12:43 AM, Steven Price wrote:
From: Sami Mujawar <sami.mujawar@xxxxxxx>

Introduce an arm-cca-guest driver that registers with
the configfs-tsm module to provide user interfaces for
retrieving an attestation token.

When a new report is requested the arm-cca-guest driver
invokes the appropriate RSI interfaces to query an
attestation token.

The steps to retrieve an attestation token are as follows:
    1. Mount the configfs filesystem if not already mounted
       mount -t configfs none /sys/kernel/config
    2. Generate an attestation token
       report=/sys/kernel/config/tsm/report/report0
       mkdir $report
       dd if=/dev/urandom bs=64 count=1 > $report/inblob
       hexdump -C $report/outblob
       rmdir $report

Signed-off-by: Sami Mujawar <sami.mujawar@xxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
Signed-off-by: Steven Price <steven.price@xxxxxxx>
---
v3: Minor improvements to comments and adapt to the renaming of
GRANULE_SIZE to RSI_GRANULE_SIZE.
---
   drivers/virt/coco/Kconfig                     |   2 +
   drivers/virt/coco/Makefile                    |   1 +
   drivers/virt/coco/arm-cca-guest/Kconfig       |  11 +
   drivers/virt/coco/arm-cca-guest/Makefile      |   2 +
   .../virt/coco/arm-cca-guest/arm-cca-guest.c   | 211 ++++++++++++ ++++++
   5 files changed, 227 insertions(+)
   create mode 100644 drivers/virt/coco/arm-cca-guest/Kconfig
   create mode 100644 drivers/virt/coco/arm-cca-guest/Makefile
   create mode 100644 drivers/virt/coco/arm-cca-guest/arm-cca-guest.c

[...]

+/**
+ * arm_cca_report_new - Generate a new attestation token.
+ *
+ * @report: pointer to the TSM report context information.
+ * @data:  pointer to the context specific data for this module.
+ *
+ * Initialise the attestation token generation using the challenge data
+ * passed in the TSM descriptor. Allocate memory for the attestation
token
+ * and schedule calls to retrieve the attestation token on the same CPU
+ * on which the attestation token generation was initialised.
+ *
+ * The challenge data must be at least 32 bytes and no more than 64
bytes. If
+ * less than 64 bytes are provided it will be zero padded to 64 bytes.
+ *
+ * Return:
+ * * %0        - Attestation token generated successfully.
+ * * %-EINVAL  - A parameter was not valid.
+ * * %-ENOMEM  - Out of memory.
+ * * %-EFAULT  - Failed to get IPA for memory page(s).
+ * * A negative status code as returned by smp_call_function_single().
+ */
+static int arm_cca_report_new(struct tsm_report *report, void *data)
+{
+    int ret;
+    int cpu;
+    long max_size;
+    unsigned long token_size;
+    struct arm_cca_token_info info;
+    void *buf;
+    u8 *token __free(kvfree) = NULL;
+    struct tsm_desc *desc = &report->desc;
+
+    if (!report)
+        return -EINVAL;
+

This check seems unnecessary and can be dropped.

Ack

+    if (desc->inblob_len < 32 || desc->inblob_len > 64)
+        return -EINVAL;
+
+    /*
+     * Get a CPU on which the attestation token generation will be
+     * scheduled and initialise the attestation token generation.
+     */
+    cpu = get_cpu();
+    max_size = rsi_attestation_token_init(desc->inblob,
desc->inblob_len);
+    put_cpu();
+

It seems that put_cpu() is called early, meaning the CPU can go away before
the subsequent call to arm_cca_attestation_continue() ?

Indeed, good spot. I'll move it to the end of the function and update
the error paths below.

Actually this was on purpose, not to block the CPU hotplug. The
attestation must be completed on the same CPU.

We can detect the failure from "smp_call" further down and make sure
we can safely complete the operation or restart it.


Yes, It's fine to call put_cpu() early since we're tolerant to error introduced by CPU unplug. It's a bit confused that rsi_attestation_token_init() is called
on the local CPU while arm_cca_attestation_continue() is called on same CPU
with help of smp_call_function_single(). Does it make sense to unify so that
both will be invoked with the help of smp_call_function_single() ?

     int cpu = smp_processor_id();

     /*
     * The calling and target CPU can be different after the calling process      * is migrated to another different CPU. It's guaranteed the attestatation
      * always happen on the target CPU with smp_call_function_single().
      */
    ret = smp_call_function_single(cpu, rsi_attestation_token_init_wrapper,
                                    (void *)&info, true);

Well, we want to allocate sufficient size buffer (size returned from
token_init())  outside an atomic context (thus not in smp_call_function()).

May be we could make this "allocation" restriction in a comment to
make it clear, why we do it this way.

Suzuki




     if (ret) {
         ...
     }

Thanks,
Gavin






[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux