On 26/06/2024 5:28 pm, Alejandro Vallejo wrote:
> hvmloader's cpuid() implementation deviates from Xen's in that the value
> passed
> on ecx is unspecified. This means that when used on leaves that implement
> subleaves it's unspecified which one you get; though it's more than likely an
> invalid one.
>
> Import Xen's implementation so there are no surprises.
Fixes: 318ac791f9f9 ("Add utilities needed for SMBIOS generation to
hvmloader")
> Signed-off-by: Alejandro Vallejo <alejandro.vall...@cloud.com>
>
>
> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index deb823a892ef..3ad7c4f6d6a2 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -184,9 +184,30 @@ int uart_exists(uint16_t uart_base);
> int lpt_exists(uint16_t lpt_base);
> int hpet_exists(unsigned long hpet_base);
>
> -/* Do cpuid instruction, with operation 'idx' */
> -void cpuid(uint32_t idx, uint32_t *eax, uint32_t *ebx,
> - uint32_t *ecx, uint32_t *edx);
> +/* Some CPUID calls want 'count' to be placed in ecx */
> +static inline void cpuid_count(
> + uint32_t op,
> + uint32_t count,
> + uint32_t *eax,
> + uint32_t *ebx,
> + uint32_t *ecx,
> + uint32_t *edx)
> +{
> + asm volatile ( "cpuid"
> + : "=a" (*eax), "=b" (*ebx), "=c" (*ecx), "=d" (*edx)
> + : "0" (op), "c" (count) );
"a" to be consistent with "c".
Also it would be better to name the parameters as leaf and subleaf.
Both can be fixed on commit. However, there's no use in HVMLoader
tickling this bug right now, so I'm not sure we want to rush this into
4.19 at this point.
~Andrew
