On Fri, 2024-06-28 at 12:23 +0100, Daniel P. Berrangé wrote: > On Fri, Jun 28, 2024 at 12:09:59PM +0100, Roy Hopkins wrote: > > On Mon, 2024-06-24 at 15:00 +0100, Daniel P. Berrangé wrote: > > > On Fri, Jun 21, 2024 at 03:29:07PM +0100, Roy Hopkins wrote: > > > > An IGVM file contains configuration of guest state that should be > > > > applied during configuration of the guest, before the guest is started. > > > > > > > > This patch allows the user to add an igvm-cfg object to the machine > > > > configuration that allows an IGVM file to be configured that will be > > > > applied to the guest before it is started. > > > > > > > > If an IGVM configuration is provided then the IGVM file is processed at > > > > the end of the board initialization, before the state transition to > > > > PHASE_MACHINE_INITIALIZED. > > > > > > > > Signed-off-by: Roy Hopkins <roy.hopk...@suse.com> > > > > --- > > > > include/hw/boards.h | 2 ++ > > > > hw/core/machine.c | 20 ++++++++++++++++++++ > > > > qemu-options.hx | 25 +++++++++++++++++++++++++ > > > > 3 files changed, 47 insertions(+) > > snip > > > > This adds igvm-cfg for all machines, regardless of architecture target. > > > > > > Are igvm files fully cross-platform portable, or should we just put > > > this into the TYPE_X86_MACHINE base class to limit it ? > > > > > > It at least reports errors if I try to load an IGVM file with > > > qemu-system-aarch64 + virt type > > > > > > $ ./build/qemu-system-aarch64 -object igvm-cfg,file=../buildigvm/ovmf- > > > sev.igvm,id=igvm -machine virt,igvm-cfg=igvm > > > qemu-system-aarch64: IGVM file does not describe a compatible supported > > > platform > > > > > > so that's good. > > > > The IGVM specification is designed to support non X86 platforms, hence its > > inclusion for all machines. Support for non-X86 is likely to result in > > changes > > to the specification though that will impact the library we depend on. > > > > There would obviously need to be some further implementation to support non- > > X86 > > machines in QEMU, in the same way that further implementation is required to > > support other X86 confidential computing platforms such as TDX. > > > > So, this poses the question: should we move it to TYPE_X86_MACHINE as the > > current supported platforms are all on X86? Or should we leave it where it > > is > > with a view to adding non X86 platform support with less impact later? I'd > > appreciate your views on this. > > The pro of putting it in the base machine class is that we don't need to > repeat the property creation in each machine as we broaden support to other > arches, I presume aarch64 is probably most likely future candidate. > > The downside of putting it in the base machine class is that it limits > mgmt app ability to probe QEMU for support. ie it wouldn't be possible > to probe whether individual machines support it or not, as we broaden > QEMU support. > > Then again, we will already face that limited ability to probe even on > x86, as we won't be able to query whether IGVM is SNP only, or has been > extended to TDX too. > > With my libvirt hat on, probing is still probably the more important > factor, so would push towards putting the property just to individual > machine classes that definitely have been wired up to be able to use > it. > > With regards, > Daniel
Ok, thanks. I'll move the instance to 'X86MachineState' and the call to 'igvm->process()' into 'pc_q35_init()' and 'pc_init1()'. Regards, Roy
