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

[Tom Lendacky <thomas.lendacky@amd.com>]

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);