Re: [PATCH v3 0/8] Support ACPI PSP on Hyper-V

[Jeremi Piotrowski]

On 4/3/2023 8:20 AM, Thomas Gleixner wrote:
> On Sun, Apr 02 2023 at 17:44, Borislav Petkov wrote:
>> On Fri, Mar 24, 2023 at 06:10:09PM +0100, Jeremi Piotrowski wrote:
>>> Since the AMD PSP is a privileged device, there is a desire to not have to trust the
>>> ACPI stack,
>>
>> And yet you do:
>>
>> +	err = acpi_parse_aspt(&res[0], &pdata);
>> +	if (err)
>> +		return err;
>>
>> You don't trust the ACPI stack, and yet you're parsing an ACPI table?!?!
>> You have to make up your mind here.
>>
>> Btw, you still haven't answered my question about doing:
>>
>> 	devm_request_irq(dev, 9, ..)
>>
>> where 9 is the default ACPI interrupt.
>>
>> You can have some silly table tell you what to map or you can simply map
>> IRQ 9 and be done with it. In this second case you can *really* not
>> trust ACPI because you know which IRQ it is

Will respond to this mail directly.

> 
> The real problem here is that the information provided about the overall
> design and requirements is close to zero. All we heard so far is hand
> waving about not trusting PCI and ACPI.

That's not a fair characterization Thomas, but I will turn the other cheek.

> 
> Jeremi, can you please describe exactly what the design and constraints
> are in understandable and coherent sentences?
> 

Here goes, I will keep it as simple as I can.

The goal of these patches is to operate all the hardware interfaces required
to run AMD SEV-SNP VMs, but in the context of a Linux VM running on top of
Hyper-V. This Linux VM is called the SNP-host VM. All the patches I submit 
target the SNP-host VM kernel, which uses KVM to bring up SEV-SNP VMs. To get
SEV-SNP working you need to combine this work with AMD's KVM SEV-SNP patches.
I posted two patch sets: one that extends AMD's patches, and one that is
independent of them (this one here) that could be merged sooner.

Here are the design constraints:
1. the interfaces exposed to the SNP-host VM to operate SEV-SNP match real
   hardware interface specifications defined by AMD. This is because we are
   emulating/virtualizing a hardware feature, and not some made up virtual
   thing.

2. the SNP-host VM may run either Windows(Hyper-V) or Linux, so the SEV-SNP
   interfaces need to be supported by both.

3. Hyper-V Generation 2 VMs do not have a PCI bus. The SNP-host VM must be a
   Hyper-V Gen 2 VM.

One of the components needed to operate SEV-SNP is the Platform Security
Processor (PSP), aka AMD Secure Processor (ASP). The PSP is the root-of-trust on
AMD systems. The PSP is specified as being discoverable either on the PCI bus,
or through the presence of an ACPI table with the "ASPT" (AMD Secure Processor
Table) signature.

Here goes the design:
Constraint 1 means that only the two specified ways of discovering and
configuring a PSP inside the SNP-host VM were in the running: PCI or ASPT.
Constraint 3 means that the PCI version of the PSP is not a viable option.
Additionally, the ASPT is used on AMD hardware in Microsoft datacenters, which
means it is supported in Hyper-V (constraint 2). The outcome is that the
SNP-host VM sees an ASPT.

The ASPT provides the following information: memory range of PSP registers and
offsets of individual PSP registers inside that memory range. There are 7
registers:
- 6 are related to the "command submission" portion of the PSP; the ccp module
  knows how to operate those.
- the last one, "ACPI CmdResp" register, is used to configure the PSP interrupt
  to the OS.

The PSP interrupt configuration through the "ACPI CmdResp" register takes the
following information:
- APIC ID
- interrupt vector
- destination mode (physical/logical)
- message type (fixed/lowest priority)

So to hook this up with the Linux device model I wrote patches that do the
following:
Detect the ASPT table, extract information and register a "psp" platform_device
for the "ccp" module to bind to.
Create an irq_domain and encapsulate dealing with the PSP interrupt register
there, so that the "ccp" module has an irq number that it passes to
request_irq().

There is an "if (hypervisor == Hyper-V)" check before the ASPT table detection.
Here is the reasoning behind that:
According to AMD specifications the *same* PSP may be discoverable both through
ASPT and on the PCI bus. In that case, if the ASPT is to be used the OS is supposed
to disable the "PCI interface" through the "ACPI CmdResp" register, which will
result in no PCI-MSI interrupts, BAR writes ignored, BAR reads return all 0xF's.
I can't verify whether that would work correctly, so in the interest of not
breaking other users, the ASPT handling is hidden behind the hypervisor check.
There is nothing Hyper-V specific about any of this code, it supports a hardware
interface present in server grade hardware and would work on physical hardware if
when (not if) someone removes the condition.

That's all there is to it.

All the other information I gave is background information that I hoped would
help better understand the setting. The most relevant piece of information is the
one that I came across last. You asked "what makes this PSP device special". The PSP
is the root-of-trust on the system, it controls memory encryption keys, it can
encrypt/decrypt individual memory pages. SEV-SNP ties together a lot of system components
and requires enabling support for it in the AMD IOMMU too, which is presumably why
the PSP gets the same special treatment (as the AMD IOMMU). The ASPT and AMD PSP interrupt
configuration through the "ACPI CmdResp" register is based on a similar design of the AMD IOMMU.
The AMD IOMMU is:
- discovered through the presence of the IVRS ACPI table
- the MMIO address of the IOMMU is parsed out of the IVRS table
- if x2APIC support is enabled, the IOMMU interrupts are delivered based on
  programming APIC-ID+vector+destination mode into an interrupt control register
  in IOMMU MMIO space. This causes any PCI-MSI configuration present for the
  IOMMU to   be ignored.
- Linux supports and uses that interrupt delivery mechanism. It is implemented
  as an irq_domain.

Do you think it makes sense to include parts of the above description in cover letter
commit message?

Thanks,
Jeremi