Am 12. Januar 2026 13:22:40 UTC schrieb Ani Sinha <[email protected]>:
>For confidential guests during the reset process, the old KVM VM file
>descriptor is closed and a new one is created. When a new file descriptor is
>created, a new openpic device needs to be created against this new KVM VM file
>descriptor as well. Additionally, existing memory region needs to be reattached
>to this new openpic device and proper CPU attributes set associating new file
>descriptor. This change makes this happen with the help of a callback handler
>that gets called when the KVM VM file descriptor changes as a part of the
>confidential guest reset process.
>
>Signed-off-by: Ani Sinha <[email protected]>
>---
> hw/intc/openpic_kvm.c | 108 ++++++++++++++++++++++++++++++++----------
> 1 file changed, 83 insertions(+), 25 deletions(-)
>
>diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
>index 9aafef5d9e..4fd70d4b32 100644
>--- a/hw/intc/openpic_kvm.c
>+++ b/hw/intc/openpic_kvm.c
>@@ -49,6 +49,7 @@ struct KVMOpenPICState {
> uint32_t fd;
> uint32_t model;
> hwaddr mapped;
>+ NotifierWithReturn open_pic_vmfd_change_notifier;
I'd drop the "open_pic_" prefix here since the attribute resides inside a
struct where the context is clear.
> };
>
> static void kvm_openpic_set_irq(void *opaque, int n_IRQ, int level)
>@@ -114,6 +115,83 @@ static const MemoryRegionOps kvm_openpic_mem_ops = {
> },
> };
>
>+static int create_open_pic_device(KVMOpenPICState *opp, Error **errp)
Here in turn I'd stick to existing conventions and use an "kvm_openpic_"
prefix. What about naming this function kvm_openpic_setup_vmfd() (or reversed:
kvm_openpic_vmfd_setup())?
>+{
>+ int kvm_openpic_model;
>+ struct kvm_create_device cd = {0};
>+ KVMState *s = kvm_state;
>+ int ret;
>+
>+ switch (opp->model) {
>+ case OPENPIC_MODEL_FSL_MPIC_20:
>+ kvm_openpic_model = KVM_DEV_TYPE_FSL_MPIC_20;
>+ break;
>+
>+ case OPENPIC_MODEL_FSL_MPIC_42:
>+ kvm_openpic_model = KVM_DEV_TYPE_FSL_MPIC_42;
>+ break;
>+
>+ default:
>+ error_setg(errp, "Unsupported OpenPIC model %" PRIu32, opp->model);
>+ return -1;
>+ }
>+
>+ cd.type = kvm_openpic_model;
>+ ret = kvm_vm_ioctl(s, KVM_CREATE_DEVICE, &cd);
>+ if (ret < 0) {
>+ error_setg(errp, "Can't create device %d: %s",
>+ cd.type, strerror(errno));
>+ return -1;
>+ }
>+ opp->fd = cd.fd;
>+
>+ return 0;
>+}
>+
>+static int open_pic_vmfd_handle_vmfd_change(NotifierWithReturn *notifier,
>+ void *data, Error **errp)
kvm_openpic_handle_vmfd_change() or similar?
>+{
>+ KVMOpenPICState *opp = container_of(notifier, KVMOpenPICState,
>+ open_pic_vmfd_change_notifier);
>+ uint64_t reg_base;
>+ struct kvm_device_attr attr;
>+ CPUState *cs;
>+ int ret;
>+
>+ /* close the old descriptor */
>+ close(opp->fd);
>+
>+ if (create_open_pic_device(opp, errp) < 0) {
>+ return -1;
>+ }
>+
>+ if (!opp->mapped) {
>+ return 0;
>+ }
>+
>+ reg_base = opp->mapped;
>+ attr.group = KVM_DEV_MPIC_GRP_MISC;
>+ attr.attr = KVM_DEV_MPIC_BASE_ADDR;
>+ attr.addr = (uint64_t)(unsigned long)®_base;
>+
>+ ret = ioctl(opp->fd, KVM_SET_DEVICE_ATTR, &attr);
>+ if (ret < 0) {
>+ fprintf(stderr, "%s: %s %" PRIx64 "\n", __func__,
>+ strerror(errno), reg_base);
Why not use error_set*()?
Best regards,
Bernhard
>+ return -1;
>+ }
>+
>+ CPU_FOREACH(cs) {
>+ ret = kvm_vcpu_enable_cap(cs, KVM_CAP_IRQ_MPIC, 0, opp->fd,
>+ kvm_arch_vcpu_id(cs));
>+ if (ret < 0) {
>+ return ret;
>+ }
>+ }
>+
>+ return 0;
>+}
>+
> static void kvm_openpic_region_add(MemoryListener *listener,
> MemoryRegionSection *section)
> {
>@@ -197,37 +275,14 @@ static void kvm_openpic_realize(DeviceState *dev, Error
>**errp)
> SysBusDevice *d = SYS_BUS_DEVICE(dev);
> KVMOpenPICState *opp = KVM_OPENPIC(dev);
> KVMState *s = kvm_state;
>- int kvm_openpic_model;
>- struct kvm_create_device cd = {0};
>- int ret, i;
>+ int i;
>
> if (!kvm_check_extension(s, KVM_CAP_DEVICE_CTRL)) {
> error_setg(errp, "Kernel is lacking Device Control API");
> return;
> }
>
>- switch (opp->model) {
>- case OPENPIC_MODEL_FSL_MPIC_20:
>- kvm_openpic_model = KVM_DEV_TYPE_FSL_MPIC_20;
>- break;
>-
>- case OPENPIC_MODEL_FSL_MPIC_42:
>- kvm_openpic_model = KVM_DEV_TYPE_FSL_MPIC_42;
>- break;
>-
>- default:
>- error_setg(errp, "Unsupported OpenPIC model %" PRIu32, opp->model);
>- return;
>- }
>-
>- cd.type = kvm_openpic_model;
>- ret = kvm_vm_ioctl(s, KVM_CREATE_DEVICE, &cd);
>- if (ret < 0) {
>- error_setg(errp, "Can't create device %d: %s",
>- cd.type, strerror(errno));
>- return;
>- }
>- opp->fd = cd.fd;
>+ create_open_pic_device(opp, errp);
>
> sysbus_init_mmio(d, &opp->mem);
> qdev_init_gpio_in(dev, kvm_openpic_set_irq, OPENPIC_MAX_IRQ);
>@@ -236,6 +291,9 @@ static void kvm_openpic_realize(DeviceState *dev, Error
>**errp)
> opp->mem_listener.region_del = kvm_openpic_region_del;
> opp->mem_listener.name = "openpic-kvm";
> memory_listener_register(&opp->mem_listener, &address_space_memory);
>+ opp->open_pic_vmfd_change_notifier.notify =
>+ open_pic_vmfd_handle_vmfd_change;
>+ kvm_vmfd_add_change_notifier(&opp->open_pic_vmfd_change_notifier);
>
> /* indicate pic capabilities */
> msi_nonbroken = true;
