Re: [PATCHv4 10/30] x86/tdx: Handle CPUID via #VE

[Dave Hansen <dave.hansen@intel.com>]

On 2/26/22 17:07, Kirill A. Shutemov wrote:
> On Thu, Feb 24, 2022 at 11:04:04AM -0800, Dave Hansen wrote:
>> On 2/24/22 07:56, Kirill A. Shutemov wrote:
>>>  static bool virt_exception_user(struct pt_regs *regs, struct ve_info *ve)
>>>  {
>>> -	pr_warn("Unexpected #VE: %lld\n", ve->exit_reason);
>>> -	return false;
>>> +	switch (ve->exit_reason) {
>>> +	case EXIT_REASON_CPUID:
>>> +		return handle_cpuid(regs);
>>> +	default:
>>> +		pr_warn("Unexpected #VE: %lld\n", ve->exit_reason);
>>> +		return false;
>>> +	}
>>>  }
>>
>> What does this mean for userspace?  What kinds of things are we ceding
>> to the (untrusted) VMM to supply to userspace?
> 
> Here's what I see called from userspace.
> CPUID(AX=0x2)
> CPUID(AX=0xb, CX=0x0)
> CPUID(AX=0xb, CX=0x1)
> CPUID(AX=0x40000000, CX=0xfffaba17)
> CPUID(AX=0x80000007, CX=0x121)

Hi Kirill,

I'm not quite sure what to make of this.  Is this an *exhaustive* list
of CPUID values?  Or is this an example of what you see on one system
and one boot of userspace?

What I really want to get at is what this *means*.

For instance, maybe all of these are in the hypervisor CPUID space.
Those basically *must* be supplied by the hypervisor.

>>>  /* Handle the kernel #VE */
>>> @@ -200,6 +235,8 @@ static bool virt_exception_kernel(struct pt_regs *regs, struct ve_info *ve)
>>>  		return read_msr(regs);
>>>  	case EXIT_REASON_MSR_WRITE:
>>>  		return write_msr(regs);
>>> +	case EXIT_REASON_CPUID:
>>> +		return handle_cpuid(regs);
>>>  	default:
>>>  		pr_warn("Unexpected #VE: %lld\n", ve->exit_reason);
>>>  		return false;
>> What kinds of random CPUID uses in the kernel at runtime need this
>> handling?
> 
> CPUID(AX=0x2)
> CPUID(AX=0x6, CX=0x0)
> CPUID(AX=0xb, CX=0x0)
> CPUID(AX=0xb, CX=0x1)
> CPUID(AX=0xb, CX=0x2)
> CPUID(AX=0xf, CX=0x0)
> CPUID(AX=0xf, CX=0x1)
> CPUID(AX=0x10, CX=0x0)
> CPUID(AX=0x10, CX=0x1)
> CPUID(AX=0x10, CX=0x2)
> CPUID(AX=0x10, CX=0x3)
> CPUID(AX=0x16, CX=0x0)
> CPUID(AX=0x1f, CX=0x0)
> CPUID(AX=0x40000000, CX=0x0)
> CPUID(AX=0x40000000, CX=0xfffaba17)
> CPUID(AX=0x40000001, CX=0x0)
> CPUID(AX=0x80000002, CX=0x0)
> CPUID(AX=0x80000003, CX=0x0)
> CPUID(AX=0x80000004, CX=0x0)
> CPUID(AX=0x80000007, CX=0x0)
> CPUID(AX=0x80000007, CX=0x121)

OK, that's a good list.  I guess I need to decode those and make sense
of them.  Any help would be appreciated and would speed along this
review process.

>> Is it really OK that we let the VMM inject arbitrary CPUID
>> values into random CPUID uses in the kernel... silently?
> 
> We realise that this is possible vector of attack and plan to implement
> proper filtering. But it is beyon core enabling.
> 
>> Is this better than just returning 0's, for instance?
> 
> Plain 0 injection breaks the boot. More complicated solution is need.

OK, so we're leaving the kernel open to something that might be an
attack vector: we know that we don't know how this might be bad.  It's a
"known unknown"[1].

That doesn't seem *horrible*.  But, it also doesn't seem great.  There
are a lot of ways to address the situation.  But, simply not mentioning
it in the changelog or cover letter isn't a great way to handle it.

Where do we *want* this to be?  I'll take a stab at it:

In a perfect world, we'd simply keep a list of things that come from the
hypervisor and others where the kernel wants to provide the CPUID data.
 But, there are always going to be new uses of CPUID.  There's no way we
can keep a 100% complete list.

That means that the kernel needs to handle unknown CPUID use.  There are
currently no known stupidly simple solutions like "return all 0's".

The other simplest solution is to just call into the hypervisor no
matter what the CPUID use is.  This puts the kernel at the mercy of the
hypervisor to some unknown degree.  But, it is OK given a benign hypervisor.

The kernel can WARN() or taint on the situation for now until you
develop a a more robust list of items that can be deferred to the
hypervisor.

1. https://en.wikipedia.org/wiki/There_are_known_knowns