This series is inspired and suggested by Daniel:
https://lore.kernel.org/qemu-devel/ZbfoQsEuv6_zwl3b@redhat.com/

Currently, different confidential VMs in different architectures have
their own specific *_kvm_init() (and some have *_kvm_reset()) exposed
for KVM stuff when it's a confidential VM. e.g., sev_kmv_init() for x86
SEV, pef_kvm_init() and pef_kvm_reset() for PPC PEF, and s390_pv_init()
for s390 PV VMs.

Introduce a generic .kvm_init() and .kvm_reset() functions in
ConfidentialGuestSupportClass, so that different cgs technologies in
different architectures can implement their own, while common interface
of cgs can be used.

This RFC implements two helper functions confidential_guest_kvm_init()
and confidential_guest_kvm_reset() in Patch 1. In the following patches,
they are called in arch specific implementation. X86 will benefit more
for the generic implementation when TDX support is added.

There is one step forward possible, that calling
confidential_guest_kvm_init() before kvm_arch_init() in kvm_int() in
accel/kvm/kvm-all.c. This way, each arch doesn't need to call in their
arch specific code.
	
X86 fits it, however I'm not sure if ppc and s390 fit it as well.
Because currently, ppc calls it in machine->init()
and s390 calls in MachineClass->init(). I'm not sure if there is any
order dependency. 

Xiaoyao Li (4):
  confidential guest support: Add kvm_init() and kvm_reset() in class
  i386/sev: Switch to use confidential_guest_kvm_init()
  ppc/pef: switch to use confidential_guest_kvm_init/reset()
  s390: Switch to use confidential_guest_kvm_init()

 hw/ppc/pef.c                              |   9 +-
 hw/ppc/spapr.c                            |   6 +-
 hw/s390x/s390-virtio-ccw.c                |   3 +-
 include/exec/confidential-guest-support.h |  42 +++++++-
 include/hw/ppc/pef.h                      |  17 ---
 target/i386/kvm/kvm.c                     |   2 +-
 target/i386/kvm/meson.build               |   2 -
 target/i386/kvm/sev-stub.c                |   5 -
 target/i386/sev.c                         | 120 +++++++++++-----------
 target/i386/sev.h                         |   2 -
 target/s390x/kvm/pv.c                     |   8 ++
 target/s390x/kvm/pv.h                     |  14 ---
 12 files changed, 123 insertions(+), 107 deletions(-)
 delete mode 100644 include/hw/ppc/pef.h

-- 
2.34.1

On Tue, Feb 06, 2024 at 03:28:48AM -0500, Xiaoyao Li wrote:
> This series is inspired and suggested by Daniel:
> https://lore.kernel.org/qemu-devel/ZbfoQsEuv6_zwl3b@redhat.com/
> 
> Currently, different confidential VMs in different architectures have
> their own specific *_kvm_init() (and some have *_kvm_reset()) exposed
> for KVM stuff when it's a confidential VM. e.g., sev_kmv_init() for x86
> SEV, pef_kvm_init() and pef_kvm_reset() for PPC PEF, and s390_pv_init()
> for s390 PV VMs.
> 
> Introduce a generic .kvm_init() and .kvm_reset() functions in
> ConfidentialGuestSupportClass, so that different cgs technologies in
> different architectures can implement their own, while common interface
> of cgs can be used.
> 
> This RFC implements two helper functions confidential_guest_kvm_init()
> and confidential_guest_kvm_reset() in Patch 1. In the following patches,
> they are called in arch specific implementation. X86 will benefit more
> for the generic implementation when TDX support is added.
> 
> There is one step forward possible, that calling
> confidential_guest_kvm_init() before kvm_arch_init() in kvm_int() in
> accel/kvm/kvm-all.c. This way, each arch doesn't need to call in their
> arch specific code.
> 
> X86 fits it, however I'm not sure if ppc and s390 fit it as well.
> Because currently, ppc calls it in machine->init()
> and s390 calls in MachineClass->init(). I'm not sure if there is any
> order dependency.

IIUC that s390 call is still a machine->init method, rather than
class init.

I think this series is nice, but its up to the KVM maintainers
to decide...


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On 2/6/2024 10:19 PM, Daniel P. Berrangé wrote:
> On Tue, Feb 06, 2024 at 03:28:48AM -0500, Xiaoyao Li wrote:
>> This series is inspired and suggested by Daniel:
>> https://lore.kernel.org/qemu-devel/ZbfoQsEuv6_zwl3b@redhat.com/
>>
>> Currently, different confidential VMs in different architectures have
>> their own specific *_kvm_init() (and some have *_kvm_reset()) exposed
>> for KVM stuff when it's a confidential VM. e.g., sev_kmv_init() for x86
>> SEV, pef_kvm_init() and pef_kvm_reset() for PPC PEF, and s390_pv_init()
>> for s390 PV VMs.
>>
>> Introduce a generic .kvm_init() and .kvm_reset() functions in
>> ConfidentialGuestSupportClass, so that different cgs technologies in
>> different architectures can implement their own, while common interface
>> of cgs can be used.
>>
>> This RFC implements two helper functions confidential_guest_kvm_init()
>> and confidential_guest_kvm_reset() in Patch 1. In the following patches,
>> they are called in arch specific implementation. X86 will benefit more
>> for the generic implementation when TDX support is added.
>>
>> There is one step forward possible, that calling
>> confidential_guest_kvm_init() before kvm_arch_init() in kvm_int() in
>> accel/kvm/kvm-all.c. This way, each arch doesn't need to call in their
>> arch specific code.
>>
>> X86 fits it, however I'm not sure if ppc and s390 fit it as well.
>> Because currently, ppc calls it in machine->init()
>> and s390 calls in MachineClass->init(). I'm not sure if there is any
>> order dependency.
> 
> IIUC that s390 call is still a machine->init method, rather than
> class init.

I double check the code again. Only struct MachineClass has .init() 
function defined. And I find both ppc and s390 calls the 
confidential_guest_kvm_init() (or their specific cgs kvm_init()) inside 
their machine_class->init().

> I think this series is nice, but its up to the KVM maintainers
> to decide...
> 
> 
> With regards,
> Daniel