Re: [PATCH] physmem: replace assertion with error

[Paolo Bonzini <pbonzini@redhat.com>]

On Mon, Feb 17, 2025 at 1:28 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >      qemu-system-x86_64 -m 512 -nographic -object sev-snp-guest,reduced-phys-bits=48,id=sev0 \
> >         -M q35,kernel-irqchip=split,confidential-guest-support=sev0
> >      qemu-system-x86_64: ../system/physmem.c:1871: ram_block_add: Assertion `kvm_enabled()' failed.
>
> I'd expect sev-snp-guest to bail out early enough.
>
> Is a KVM-enabled check in sev_snp_guest_instance_init() missing?

instance_init cannot do any check (it cannot fail). There is a check for
whether sev_common_kvm_init succeeded:

     if (machine->cgs && !machine->cgs->ready) {
         error_setg(errp, "accelerator does not support confidential guest %s",
                    object_get_typename(OBJECT(machine->cgs)));
         exit(1);
     }

but that doesn't help if the system/physmem.c code is reached before
qemu_machine_creation_done(), for example in qemu_init_board().

Likewise, you cannot fal in host_memory_backend_init() because that's
also an instance_init callback.

> > -        assert(kvm_enabled());
> > +        if (!kvm_enabled()) {
> > +            error_setg(errp, "cannot set up private guest memory for %s: KVM required",
> > +                       object_get_typename(OBJECT(current_machine->cgs)));
>
> Common code should be SW vs HW accel, so IMHO your check
> should go within sev_snp_guest_instance_init(), removing
> the assertion here. That said I have no clue about SEV.

This code is outside accel/kvm or target/*/kvn, but it is not quite
common code. A few lines below you have:

     new_block->guest_memfd =
           kvm_create_guest_memfd(new_block->max_length, 0, errp);

Paolo