Re: [PATCH v1 3/8] x86/psp: Register PSP platform device when ASP table is present

Tom Lendacky
2023-02-01T14:57:47Z

On 2/1/23 08:09, Jeremi Piotrowski wrote:
> On Tue, Jan 31, 2023 at 12:49:54PM -0600, Tom Lendacky wrote:
>> On 1/23/23 09:22, Jeremi Piotrowski wrote:
>>> The ASP table contains the memory location of the register window for
>>> communication with the Platform Security Processor. The device is not
>>> exposed as an acpi node, so it is necessary to probe for the table and
>>> register a platform_device to represent it in the kernel.
>>>
>>> At least conceptually, the same PSP may be exposed on the PCIe bus as
>>> well, in which case it would be necessary to choose whether to use a PCI
>>> BAR or the register window defined in ASPT for communication. There is
>>> no advantage to using the ACPI and there are no known bare-metal systems
>>> that expose the ASP table, so device registration is restricted to the
>>> only systems known to provide an ASPT: Hyper-V VMs. Hyper-V VMs also do
>>> not expose the PSP over PCIe.
>>>
>>> This is a skeleton device at this point, as the ccp driver is not yet
>>> prepared to correctly probe it. Interrupt configuration will come later
>>> on as well.
>>>
>>> Signed-off-by: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
>>> ---
>>> arch/x86/kernel/Makefile | 2 +-
>>> arch/x86/kernel/psp.c | 39 +++++++++++++++++++++++++++++++++++++++
>>
>> Based on comments about other SEV related items, this should
>> probably be moved into the arch/x86/coco/sev/ directory.
>>
>> Thanks,
>> Tom
>
> I'll do that. This will make the code depend on CONFIG_ARCH_HAS_CC_PLATFORM
> and CONFIG_AMD_MEM_ENCRYPT, the latter selects the former. This will work
> as long as CONFIG_AMD_MEM_ENCRYPT continues to be needed for both SNP guest
> and host sides.

CONFIG_AMD_MEM_ENCRYPT is only required on the guest side. It is not
needed to launch an SEV guest of any type. I believe the latest SNP
hypervisor patches are being updated to remove that dependency and replace
it with CONFIG_KVM_AMD_SEV. So you'll have to figure out what you want for
your CONFIG requirement.

Thanks,
Tom

>
> Jeremi
>
>>
>>> 2 files changed, 40 insertions(+), 1 deletion(-)
>>> create mode 100644 arch/x86/kernel/psp.c
>>>
>>> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>>> index f901658d9f7c..e2e19f2d08a7 100644
>>> --- a/arch/x86/kernel/Makefile
>>> +++ b/arch/x86/kernel/Makefile
>>> @@ -139,7 +139,7 @@ obj-$(CONFIG_UNWINDER_ORC) += unwind_orc.o
>>> obj-$(CONFIG_UNWINDER_FRAME_POINTER) += unwind_frame.o
>>> obj-$(CONFIG_UNWINDER_GUESS) += unwind_guess.o
>>> -obj-$(CONFIG_AMD_MEM_ENCRYPT) += sev.o
>>> +obj-$(CONFIG_AMD_MEM_ENCRYPT) += psp.o sev.o
>>> obj-$(CONFIG_CFI_CLANG) += cfi.o
>>> diff --git a/arch/x86/kernel/psp.c b/arch/x86/kernel/psp.c
>>> new file mode 100644
>>> index 000000000000..d404df47cc04
>>> --- /dev/null
>>> +++ b/arch/x86/kernel/psp.c
>>> @@ -0,0 +1,39 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +#include <linux/platform_data/psp.h>
>>> +#include <linux/platform_device.h>
>>> +#include <asm/hypervisor.h>
>>> +
>>> +static struct platform_device psp_device = {
>>> + .name = "psp",
>>> + .id = PLATFORM_DEVID_NONE,
>>> +};
>>> +
>>> +static int __init psp_init_platform_device(void)
>>> +{
>>> + struct psp_platform_data pdata = {};
>>> + struct resource res[1];
>>> + int err;
>>> +
>>> + /*
>>> + * The ACPI PSP interface is mutually exclusive with the PCIe interface,
>>> + * but there is no reason to use the ACPI interface over the PCIe one.
>>> + * Restrict probing ACPI PSP to platforms known to only expose the ACPI
>>> + * interface, which at this time is SNP-host capable Hyper-V VMs.
>>> + */
>>> + if (!hypervisor_is_type(X86_HYPER_MS_HYPERV))
>>> + return -ENODEV;
>>> +
>>> + err = acpi_parse_aspt(res, &pdata);
>>> + if (err)
>>> + return err;
>>> + err = platform_device_add_resources(&psp_device, res, 1);
>>> + if (err)
>>> + return err;
>>> +
>>> + err = platform_device_register(&psp_device);
>>> + if (err)
>>> + return err;
>>> + return 0;
>>> +}
>>> +device_initcall(psp_init_platform_device);