Re: [RFC 05/12] ACPICA: Add TLBI table definition

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] acpi: direct modification of upstream ACPICA headers with draft specs
- [Medium] kvm: arm64: HCRX_EL2.FNB bit is not filtered for nested hypervisors
--

commit 1c073d3327bf45ba56b8ee1d936edd5e15ec68be
Author: Kristina Martšenko <kristina.martsenko@arm.com>

ACPICA: Add TLBI table definition

This commit adds the definition of the new Arm TLBI ACPI table to the global
acpica headers. The table is noted as an initial proposal that is not yet
public and may change.

> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index baef525367b5c..68849c9f7cf69 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -61,6 +61,7 @@
>  #define ACPI_SIG_SVKL           "SVKL"	/* Storage Volume Key Location Table */
>  #define ACPI_SIG_SWFT           "SWFT"	/* SoundWire File Table */
>  #define ACPI_SIG_TDEL           "TDEL"	/* TD Event Log Table */
> +#define ACPI_SIG_TLBI           "TLBI"	/* TLB Invalidate Domain Table */
                                   ^^^^

[Severity: Medium]
Is it appropriate to introduce a draft, non-final table signature directly into
the global upstream-maintained acpica headers?

The commit message explicitly notes this patch should not be merged as is
because the TLBI table description is not yet public and may change. Merging
draft specifications creates a risk of dead uapi and compatibility issues if
the final specification differs.

[ ... ]

> @@ -3701,6 +3702,37 @@ struct acpi_table_tdel {
>  	u64 log_area_start_address;
>  };
>  
> +/*******************************************************************************
> + *
> + * TLBI - TLBI Domain Table
> + *
> + ******************************************************************************/
> +
> +struct acpi_table_tlbi {
> +	struct acpi_table_header header;
> +	u32 domain_count;
> +	u32 domains_offset;
> +};

[Severity: Medium]
Is it acceptable to add a Linux-specific out-of-tree structure definition to an
upstream-maintained acpica file?

The acpica headers are maintained upstream, and Linux kernel policy typically
requires changes to first be accepted by the acpica project before being synced
to the Linux tree.

commit cb4c50a2ac546f3127893846943d61a1ea5cc49e
Author: Kristina Martšenko <kristina.martsenko@arm.com>

KVM: arm64: Hide TLBID from guest sysregs

This commit hides TLBID system registers and fields from the guest to prevent
them from discovering domains. It reports the feature as unsupported and
sanitizes a nested hypervisor's HCRX register bits.

>   * Sanitize a nested hypervisor's HCRX.{VTLBIDEn,VTLBIDOSEn,FNB}.

[Severity: Medium]
The commit message claims to sanitize the guest's HCRX_EL2.FNB bit, but it
appears HCRX_EL2_FNB is omitted from NV_HCRX_GUEST_EXCLUDE:

arch/arm64/kvm/hyp/include/hyp/switch.h:
#define NV_HCRX_GUEST_EXCLUDE	(HCRX_EL2_TMEA	    | HCRX_EL2_PTTWI	| \
				 HCRX_EL2_EnIDCP128 | HCRX_EL2_VTLBIDEn	| \
				 HCRX_EL2_VTLBIDOSEn)

Because HCRX_EL2 is a VNCR-backed register, a nested guest hypervisor (L1) can
write to it directly in memory without trapping. During context switch, KVM
reads the guest's HCRX_EL2 from the VNCR page and masks it with
~NV_HCRX_GUEST_EXCLUDE.

Since FNB is not excluded, will KVM apply the guest's FNB bit to the physical
HCRX_EL2 register, breaking the virtualization boundary?

The impact appears to be limited to the guest's own TLBI coherence.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001110655.461473-1-kristina.martsenko@arm.com?part=5