linux-kernel - Re: [PATCH v2] selftests: KVM: avoid failures due to reserved HyperTransport region

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YbJ5jyCyqZwZU3uH@google.com>
Date:   Thu, 9 Dec 2021 21:47:59 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        vkuznets@...hat.com, mlevitsk@...hat.com,
        joao.m.martins@...cle.com, stable@...r.kernel.org,
        David Matlack <dmatlack@...gle.com>
Subject: Re: [PATCH v2] selftests: KVM: avoid failures due to reserved
 HyperTransport region

On Thu, Dec 09, 2021, Paolo Bonzini wrote:
> +unsigned long vm_compute_max_gfn(struct kvm_vm *vm)
> +{
> +	const unsigned long num_ht_pages = 12 << 18; /* 12 GiB */
> +	unsigned long ht_gfn, max_gfn, max_pfn;
> +	uint32_t eax, ebx, ecx, edx;
> +
> +	max_gfn = (1ULL << (vm->pa_bits - vm->page_shift)) - 1;
> +
> +	/* Avoid reserved HyperTransport region on AMD processors.  */
> +	eax = ecx = 0;
> +	cpuid(&eax, &ebx, &ecx, &edx);
> +	if (ebx != X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx ||
> +	    ecx != X86EMUL_CPUID_VENDOR_AuthenticAMD_ecx ||
> +	    edx != X86EMUL_CPUID_VENDOR_AuthenticAMD_edx)
> +		return max_gfn;
> +
> +	/* On parts with <40 physical address bits, the area is fully hidden */
> +	if (vm->pa_bits < 40)
> +		return max_gfn;
> +
> +	eax = 1;
> +	cpuid(&eax, &ebx, &ecx, &edx);
> +	if (x86_family(eax) < 0x17) {
> +		/* Before family 17h, the HyperTransport area is just below 1T.  */
> +		ht_gfn = (1 << 28) - num_ht_pages;
> +	} else {
> +		/*
> +		 * Otherwise it's at the top of the physical address
> +		 * space, possibly reduced due to SME by bits 11:6 of
> +		 * CPUID[0x8000001f].EBX.
> +		 */
> +		eax = 0x80000008;
> +		cpuid(&eax, &ebx, &ecx, &edx);

Should't this check 0x80000000.eax >= 0x80000008 first?  Or do we just accept
failure if family==0x17 and there's no 0x80000008?  One paranoid option would be
to use the pre-fam17 value, e.g.

        /* Before family 17h, the HyperTransport area is just below 1T. */
        ht_gfn = (1 << 28) - num_ht_pages;
        if (x86_family(eax) < 0x17)
                goto out;

        eax = 0x80000000;
        cpuid(&eax, &ebx, &ecx, &edx);
        max_ext_leaf = eax;

        /* Use the old, conservative value if MAXPHYADDR isn't enumerated. */
        if (max_ext_leaf < 0x80000008)
                goto out;

        /* comment */
        eax = 0x80000008;
        cpuid(&eax, &ebx, &ecx, &edx);
        max_pfn = (1ULL << ((eax & 255) - vm->page_shift)) - 1;
        if (max_ext_leaf >= 0x8000001f) {
                <adjust>
        }
        ht_gfn = max_pfn - num_ht_pages;
out:
        return min(max_gfn, ht_gfn - 1);

> +             max_pfn = (1ULL << ((eax & 255) - vm->page_shift)) - 1;

LOL, "& 255", you just couldn't resist, huh?  My version of Rami Code only goes
up to 15.  :-)