Claude review: platform/x86/intel/vsec: Plumb ACPI PMT discovery tables through vsec

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Overall: Core feature patch. Has a resource leak issue and a minor doc typo.**

This adds the ACPI discovery infrastructure: `enum intel_vsec_disc_source`, `acpi_disc` arrays in both `intel_vsec_platform_info` and `intel_vsec_device`, and the `src` field.

**Issue 1 — Potential resource leak when `kmemdup` succeeds but device registration fails:**

In `intel_vsec_add_dev()`:
```c
intel_vsec_dev->acpi_disc = kmemdup(info->acpi_disc, bytes, GFP_KERNEL);
if (!intel_vsec_dev->acpi_disc)
    return -ENOMEM;
```

The `intel_vsec_dev` is declared with `__free(kfree)` cleanup, but `acpi_disc` is only freed in `intel_vsec_dev_release()` (the device release callback). If `intel_vsec_add_aux()` fails later in this function *before* the device is fully registered (before `device_add` succeeds), the `__free(kfree)` cleanup will free `intel_vsec_dev` but not `acpi_disc`, since `intel_vsec_dev_release` won't be called. This mirrors the existing pattern for `resource` though — `resource` has the same issue with `no_free_ptr(res)` — so this may be acceptable if the existing pattern is considered safe. But it's worth verifying.

Actually, looking more carefully: `intel_vsec_dev` uses `__free(kfree)` and is later passed via `no_free_ptr()` to `intel_vsec_add_aux()`. If `no_free_ptr()` has already been called, ownership has been transferred. But the `acpi_disc` allocation happens *before* `no_free_ptr()`, so if the function returns early between the `kmemdup` and `no_free_ptr`, `acpi_disc` will leak since `kfree(intel_vsec_dev)` doesn't also free `intel_vsec_dev->acpi_disc`.

**Issue 2 — `check_mul_overflow` alignment:**

```c
if (check_mul_overflow(intel_vsec_dev->num_resources,
    sizeof(*info->acpi_disc), &bytes))
```

The `sizeof(*info->acpi_disc)` evaluates to `sizeof(u32[4])` = 16 bytes, and `num_resources` is `int`. The `check_mul_overflow` macro should handle the type promotion correctly, but `bytes` is `size_t` while `num_resources` is `int` — if `num_resources` were somehow negative, this could behave unexpectedly. It won't happen in practice since `num_resources` is validated earlier, but using `(size_t)intel_vsec_dev->num_resources` would be more defensive.

**Issue 3 — Comment typo in the header:**

```c
 * @acpi_disc: ACPI discovery tables, each entry is two QWORDs
 *             in little-endian format as defined by the PMT ACPI spec.
 *             Valid only when @provider == INTEL_VSEC_DISC_ACPI.
```

`@provider` should be `@src` to match the actual field name.

**Issue 4 — Breaking the resource loop early:**

```c
for (i = 0, tmp = res; i < header->num_entries; i++, tmp++) {
    if (info->src == INTEL_VSEC_DISC_ACPI)
        break;
```

This `break` on ACPI effectively leaves `res` allocated but empty (all zeroed). The `num_resources` is still set to `header->num_entries`, so downstream code might expect valid resources. It might be cleaner to skip the allocation and loop entirely for ACPI, or set `num_resources` to 0 for ACPI devices.

---
Generated by Claude Code Patch Reviewer