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

[Jeremi Piotrowski]

On 4/5/2023 9:56 AM, Jeremi Piotrowski wrote:
> 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

Hi Thomas,

Have you had a chance to review this?

Thanks,
Jeremi