* [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support
@ 2025-02-12 5:44 Suravee Suthikulpanit
2025-02-12 5:44 ` [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation Suravee Suthikulpanit
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Suravee Suthikulpanit @ 2025-02-12 5:44 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, mtosatti, mst, marcel.apfelbaum, jon.grimm,
santosh.shukla, vasant.hegde, Wei.Huang2, bsd, berrange,
joao.m.martins, alejandro.j.jimenez, Suravee Suthikulpanit
Currently, amd-iommu device does not support migration. This series addresses
an issue due hidden AMDVI-PCI device enumeration. Then introduces migratable
VMStateDescription, which saves necessary parameters for the device.
Changes from v2:
(https://lore.kernel.org/all/20250206051856.323651-1-suravee.suthikulpanit@amd.com)
* Add patch 1/2
Suravee Suthikulpanit (2):
hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow
full control over the PCI device creation
hw/i386/amd_iommu: Allow migration when explicitly create the
AMDVI-PCI device
hw/i386/acpi-build.c | 8 ++--
hw/i386/amd_iommu.c | 111 +++++++++++++++++++++++++++++++++----------
hw/i386/amd_iommu.h | 3 +-
3 files changed, 91 insertions(+), 31 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation 2025-02-12 5:44 [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support Suravee Suthikulpanit @ 2025-02-12 5:44 ` Suravee Suthikulpanit 2025-02-21 0:38 ` Michael S. Tsirkin 2025-02-12 5:44 ` [PATCH v3 2/2] hw/i386/amd_iommu: Allow migration when explicitly create the AMDVI-PCI device Suravee Suthikulpanit 2025-02-20 23:21 ` [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support Michael S. Tsirkin 2 siblings, 1 reply; 6+ messages in thread From: Suravee Suthikulpanit @ 2025-02-12 5:44 UTC (permalink / raw) To: qemu-devel Cc: pbonzini, mtosatti, mst, marcel.apfelbaum, jon.grimm, santosh.shukla, vasant.hegde, Wei.Huang2, bsd, berrange, joao.m.martins, alejandro.j.jimenez, Suravee Suthikulpanit Current amd-iommu model internally creates an AMDVI-PCI device. Here is a snippet from info qtree: bus: main-system-bus type System dev: amd-iommu, id "" xtsup = false pci-id = "" intremap = "on" device-iotlb = false pt = true ... dev: q35-pcihost, id "" MCFG = -1 (0xffffffffffffffff) pci-hole64-size = 34359738368 (32 GiB) below-4g-mem-size = 134217728 (128 MiB) above-4g-mem-size = 0 (0 B) smm-ranges = true x-pci-hole64-fix = true x-config-reg-migration-enabled = true bypass-iommu = false bus: pcie.0 type PCIE dev: AMDVI-PCI, id "" addr = 01.0 romfile = "" romsize = 4294967295 (0xffffffff) rombar = -1 (0xffffffffffffffff) multifunction = false x-pcie-lnksta-dllla = true x-pcie-extcap-init = true failover_pair_id = "" acpi-index = 0 (0x0) x-pcie-err-unc-mask = true x-pcie-ari-nextfn-1 = false x-max-bounce-buffer-size = 4096 (4 KiB) x-pcie-ext-tag = true busnr = 0 (0x0) class Class 0806, addr 00:01.0, pci id 1022:0000 (sub 1af4:1100) This prohibits users from specifying the PCI topology for the amd-iommu device, which becomes a problem when trying to support VM migration since it does not guarantee the same enumeration of AMD IOMMU device. Therfore, decouple the AMDVI-PCI from amd-iommu device and introduce pci-id parameter to link between the two devices. For example: -device AMDVI-PCI,id=iommupci0,bus=pcie.0,addr=0x05 \ -device amd-iommu,intremap=on,pt=on,xtsup=on,pci-id=iommupci0 \ For backward-compatibility, internally create the AMDVI-PCI device if not specified on the CLI. Co-developed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- hw/i386/acpi-build.c | 8 +++---- hw/i386/amd_iommu.c | 53 +++++++++++++++++++++++++++----------------- hw/i386/amd_iommu.h | 3 ++- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 53b7306b43..e70eeaf577 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2333,10 +2333,10 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id, build_append_int_noprefix(table_data, ivhd_blob->len + 24, 2); /* DeviceID */ build_append_int_noprefix(table_data, - object_property_get_int(OBJECT(&s->pci), "addr", + object_property_get_int(OBJECT(s->pci), "addr", &error_abort), 2); /* Capability offset */ - build_append_int_noprefix(table_data, s->pci.capab_offset, 2); + build_append_int_noprefix(table_data, s->pci->capab_offset, 2); /* IOMMU base address */ build_append_int_noprefix(table_data, s->mr_mmio.addr, 8); /* PCI Segment Group */ @@ -2368,10 +2368,10 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id, build_append_int_noprefix(table_data, ivhd_blob->len + 40, 2); /* DeviceID */ build_append_int_noprefix(table_data, - object_property_get_int(OBJECT(&s->pci), "addr", + object_property_get_int(OBJECT(s->pci), "addr", &error_abort), 2); /* Capability offset */ - build_append_int_noprefix(table_data, s->pci.capab_offset, 2); + build_append_int_noprefix(table_data, s->pci->capab_offset, 2); /* IOMMU base address */ build_append_int_noprefix(table_data, s->mr_mmio.addr, 8); /* PCI Segment Group */ diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 6b13ce894b..0f552bafa0 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -167,11 +167,11 @@ static void amdvi_generate_msi_interrupt(AMDVIState *s) { MSIMessage msg = {}; MemTxAttrs attrs = { - .requester_id = pci_requester_id(&s->pci.dev) + .requester_id = pci_requester_id(&s->pci->dev) }; - if (msi_enabled(&s->pci.dev)) { - msg = msi_get_message(&s->pci.dev, 0); + if (msi_enabled(&s->pci->dev)) { + msg = msi_get_message(&s->pci->dev, 0); address_space_stl_le(&address_space_memory, msg.address, msg.data, attrs, NULL); } @@ -239,7 +239,7 @@ static void amdvi_page_fault(AMDVIState *s, uint16_t devid, info |= AMDVI_EVENT_IOPF_I | AMDVI_EVENT_IOPF; amdvi_encode_event(evt, devid, addr, info); amdvi_log_event(s, evt); - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, PCI_STATUS_SIG_TARGET_ABORT); } /* @@ -256,7 +256,7 @@ static void amdvi_log_devtab_error(AMDVIState *s, uint16_t devid, amdvi_encode_event(evt, devid, devtab, info); amdvi_log_event(s, evt); - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, PCI_STATUS_SIG_TARGET_ABORT); } /* log an event trying to access command buffer @@ -269,7 +269,7 @@ static void amdvi_log_command_error(AMDVIState *s, hwaddr addr) amdvi_encode_event(evt, 0, addr, info); amdvi_log_event(s, evt); - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, PCI_STATUS_SIG_TARGET_ABORT); } /* log an illegal command event @@ -310,7 +310,7 @@ static void amdvi_log_pagetab_error(AMDVIState *s, uint16_t devid, info |= AMDVI_EVENT_PAGE_TAB_HW_ERROR; amdvi_encode_event(evt, devid, addr, info); amdvi_log_event(s, evt); - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, PCI_STATUS_SIG_TARGET_ABORT); } @@ -1607,26 +1607,45 @@ static void amdvi_sysbus_reset(DeviceState *dev) { AMDVIState *s = AMD_IOMMU_DEVICE(dev); - msi_reset(&s->pci.dev); + msi_reset(&s->pci->dev); amdvi_init(s); } static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) { + DeviceClass *dc = (DeviceClass *) object_get_class(OBJECT(dev)); AMDVIState *s = AMD_IOMMU_DEVICE(dev); MachineState *ms = MACHINE(qdev_get_machine()); PCMachineState *pcms = PC_MACHINE(ms); X86MachineState *x86ms = X86_MACHINE(ms); PCIBus *bus = pcms->pcibus; - s->iotlb = g_hash_table_new_full(amdvi_uint64_hash, - amdvi_uint64_equal, g_free, g_free); + if (s->pci_id) { + PCIDevice *pdev = NULL; + int ret = pci_qdev_find_device(s->pci_id, &pdev); - /* This device should take care of IOMMU PCI properties */ - if (!qdev_realize(DEVICE(&s->pci), &bus->qbus, errp)) { - return; + if (ret) { + error_report("Cannot find PCI device '%s'", s->pci_id); + return; + } + + if (!object_dynamic_cast(OBJECT(pdev), TYPE_AMD_IOMMU_PCI)) { + error_report("Device '%s' must be an AMDVI-PCI device type", s->pci_id); + return; + } + + s->pci = AMD_IOMMU_PCI(pdev); + } else { + s->pci = AMD_IOMMU_PCI(object_new(TYPE_AMD_IOMMU_PCI)); + /* This device should take care of IOMMU PCI properties */ + if (!qdev_realize(DEVICE(s->pci), &bus->qbus, errp)) { + return; + } } + s->iotlb = g_hash_table_new_full(amdvi_uint64_hash, + amdvi_uint64_equal, g_free, g_free); + /* Pseudo address space under root PCI bus. */ x86ms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_IOAPIC_SB_DEVID); @@ -1663,6 +1682,7 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) static const Property amdvi_properties[] = { DEFINE_PROP_BOOL("xtsup", AMDVIState, xtsup, false), + DEFINE_PROP_STRING("pci-id", AMDVIState, pci_id), }; static const VMStateDescription vmstate_amdvi_sysbus = { @@ -1670,12 +1690,6 @@ static const VMStateDescription vmstate_amdvi_sysbus = { .unmigratable = 1 }; -static void amdvi_sysbus_instance_init(Object *klass) -{ - AMDVIState *s = AMD_IOMMU_DEVICE(klass); - - object_initialize(&s->pci, sizeof(s->pci), TYPE_AMD_IOMMU_PCI); -} static void amdvi_sysbus_class_init(ObjectClass *klass, void *data) { @@ -1698,7 +1712,6 @@ static const TypeInfo amdvi_sysbus = { .name = TYPE_AMD_IOMMU_DEVICE, .parent = TYPE_X86_IOMMU_DEVICE, .instance_size = sizeof(AMDVIState), - .instance_init = amdvi_sysbus_instance_init, .class_init = amdvi_sysbus_class_init }; diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h index e0dac4d9a9..ece71ff0b6 100644 --- a/hw/i386/amd_iommu.h +++ b/hw/i386/amd_iommu.h @@ -315,7 +315,8 @@ struct AMDVIPCIState { struct AMDVIState { X86IOMMUState iommu; /* IOMMU bus device */ - AMDVIPCIState pci; /* IOMMU PCI device */ + AMDVIPCIState *pci; /* IOMMU PCI device */ + char *pci_id; /* ID of AMDVI-PCI device, if user created */ uint32_t version; -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation 2025-02-12 5:44 ` [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation Suravee Suthikulpanit @ 2025-02-21 0:38 ` Michael S. Tsirkin 2025-02-25 13:39 ` Suthikulpanit, Suravee 0 siblings, 1 reply; 6+ messages in thread From: Michael S. Tsirkin @ 2025-02-21 0:38 UTC (permalink / raw) To: Suravee Suthikulpanit Cc: qemu-devel, pbonzini, mtosatti, marcel.apfelbaum, jon.grimm, santosh.shukla, vasant.hegde, Wei.Huang2, bsd, berrange, joao.m.martins, alejandro.j.jimenez On Wed, Feb 12, 2025 at 05:44:49AM +0000, Suravee Suthikulpanit wrote: > Current amd-iommu model internally creates an AMDVI-PCI device. Here is > a snippet from info qtree: > > bus: main-system-bus > type System > dev: amd-iommu, id "" > xtsup = false > pci-id = "" > intremap = "on" > device-iotlb = false > pt = true > ... > dev: q35-pcihost, id "" > MCFG = -1 (0xffffffffffffffff) > pci-hole64-size = 34359738368 (32 GiB) > below-4g-mem-size = 134217728 (128 MiB) > above-4g-mem-size = 0 (0 B) > smm-ranges = true > x-pci-hole64-fix = true > x-config-reg-migration-enabled = true > bypass-iommu = false > bus: pcie.0 > type PCIE > dev: AMDVI-PCI, id "" > addr = 01.0 > romfile = "" > romsize = 4294967295 (0xffffffff) > rombar = -1 (0xffffffffffffffff) > multifunction = false > x-pcie-lnksta-dllla = true > x-pcie-extcap-init = true > failover_pair_id = "" > acpi-index = 0 (0x0) > x-pcie-err-unc-mask = true > x-pcie-ari-nextfn-1 = false > x-max-bounce-buffer-size = 4096 (4 KiB) > x-pcie-ext-tag = true > busnr = 0 (0x0) > class Class 0806, addr 00:01.0, pci id 1022:0000 (sub 1af4:1100) > > This prohibits users from specifying the PCI topology for the amd-iommu device, > which becomes a problem when trying to support VM migration since it does not > guarantee the same enumeration of AMD IOMMU device. > > Therfore, decouple the AMDVI-PCI from amd-iommu device and introduce pci-id > parameter to link between the two devices. > > For example: > -device AMDVI-PCI,id=iommupci0,bus=pcie.0,addr=0x05 \ > -device amd-iommu,intremap=on,pt=on,xtsup=on,pci-id=iommupci0 \ > > For backward-compatibility, internally create the AMDVI-PCI device if not > specified on the CLI. > > Co-developed-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> breaks build: https://gitlab.com/mstredhat/qemu/-/jobs/9202802751 ./hw/i386/amd_iommu.c: In function ‘amdvi_sysbus_realize’: ../hw/i386/amd_iommu.c:1616:18: error: unused variable ‘dc’ [-Werror=unused-variable] 1616 | DeviceClass *dc = (DeviceClass *) object_get_class(OBJECT(dev)); | ^~ cc1: all warnings being treated as errors > --- > hw/i386/acpi-build.c | 8 +++---- > hw/i386/amd_iommu.c | 53 +++++++++++++++++++++++++++----------------- > hw/i386/amd_iommu.h | 3 ++- > 3 files changed, 39 insertions(+), 25 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 53b7306b43..e70eeaf577 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2333,10 +2333,10 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id, > build_append_int_noprefix(table_data, ivhd_blob->len + 24, 2); > /* DeviceID */ > build_append_int_noprefix(table_data, > - object_property_get_int(OBJECT(&s->pci), "addr", > + object_property_get_int(OBJECT(s->pci), "addr", > &error_abort), 2); > /* Capability offset */ > - build_append_int_noprefix(table_data, s->pci.capab_offset, 2); > + build_append_int_noprefix(table_data, s->pci->capab_offset, 2); > /* IOMMU base address */ > build_append_int_noprefix(table_data, s->mr_mmio.addr, 8); > /* PCI Segment Group */ > @@ -2368,10 +2368,10 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id, > build_append_int_noprefix(table_data, ivhd_blob->len + 40, 2); > /* DeviceID */ > build_append_int_noprefix(table_data, > - object_property_get_int(OBJECT(&s->pci), "addr", > + object_property_get_int(OBJECT(s->pci), "addr", > &error_abort), 2); > /* Capability offset */ > - build_append_int_noprefix(table_data, s->pci.capab_offset, 2); > + build_append_int_noprefix(table_data, s->pci->capab_offset, 2); > /* IOMMU base address */ > build_append_int_noprefix(table_data, s->mr_mmio.addr, 8); > /* PCI Segment Group */ > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c > index 6b13ce894b..0f552bafa0 100644 > --- a/hw/i386/amd_iommu.c > +++ b/hw/i386/amd_iommu.c > @@ -167,11 +167,11 @@ static void amdvi_generate_msi_interrupt(AMDVIState *s) > { > MSIMessage msg = {}; > MemTxAttrs attrs = { > - .requester_id = pci_requester_id(&s->pci.dev) > + .requester_id = pci_requester_id(&s->pci->dev) > }; > > - if (msi_enabled(&s->pci.dev)) { > - msg = msi_get_message(&s->pci.dev, 0); > + if (msi_enabled(&s->pci->dev)) { > + msg = msi_get_message(&s->pci->dev, 0); > address_space_stl_le(&address_space_memory, msg.address, msg.data, > attrs, NULL); > } > @@ -239,7 +239,7 @@ static void amdvi_page_fault(AMDVIState *s, uint16_t devid, > info |= AMDVI_EVENT_IOPF_I | AMDVI_EVENT_IOPF; > amdvi_encode_event(evt, devid, addr, info); > amdvi_log_event(s, evt); > - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, > + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, > PCI_STATUS_SIG_TARGET_ABORT); > } > /* > @@ -256,7 +256,7 @@ static void amdvi_log_devtab_error(AMDVIState *s, uint16_t devid, > > amdvi_encode_event(evt, devid, devtab, info); > amdvi_log_event(s, evt); > - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, > + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, > PCI_STATUS_SIG_TARGET_ABORT); > } > /* log an event trying to access command buffer > @@ -269,7 +269,7 @@ static void amdvi_log_command_error(AMDVIState *s, hwaddr addr) > > amdvi_encode_event(evt, 0, addr, info); > amdvi_log_event(s, evt); > - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, > + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, > PCI_STATUS_SIG_TARGET_ABORT); > } > /* log an illegal command event > @@ -310,7 +310,7 @@ static void amdvi_log_pagetab_error(AMDVIState *s, uint16_t devid, > info |= AMDVI_EVENT_PAGE_TAB_HW_ERROR; > amdvi_encode_event(evt, devid, addr, info); > amdvi_log_event(s, evt); > - pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS, > + pci_word_test_and_set_mask(s->pci->dev.config + PCI_STATUS, > PCI_STATUS_SIG_TARGET_ABORT); > } > > @@ -1607,26 +1607,45 @@ static void amdvi_sysbus_reset(DeviceState *dev) > { > AMDVIState *s = AMD_IOMMU_DEVICE(dev); > > - msi_reset(&s->pci.dev); > + msi_reset(&s->pci->dev); > amdvi_init(s); > } > > static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) > { > + DeviceClass *dc = (DeviceClass *) object_get_class(OBJECT(dev)); > AMDVIState *s = AMD_IOMMU_DEVICE(dev); > MachineState *ms = MACHINE(qdev_get_machine()); > PCMachineState *pcms = PC_MACHINE(ms); > X86MachineState *x86ms = X86_MACHINE(ms); > PCIBus *bus = pcms->pcibus; > > - s->iotlb = g_hash_table_new_full(amdvi_uint64_hash, > - amdvi_uint64_equal, g_free, g_free); > + if (s->pci_id) { > + PCIDevice *pdev = NULL; > + int ret = pci_qdev_find_device(s->pci_id, &pdev); > > - /* This device should take care of IOMMU PCI properties */ > - if (!qdev_realize(DEVICE(&s->pci), &bus->qbus, errp)) { > - return; > + if (ret) { > + error_report("Cannot find PCI device '%s'", s->pci_id); > + return; > + } > + > + if (!object_dynamic_cast(OBJECT(pdev), TYPE_AMD_IOMMU_PCI)) { > + error_report("Device '%s' must be an AMDVI-PCI device type", s->pci_id); > + return; > + } > + > + s->pci = AMD_IOMMU_PCI(pdev); > + } else { > + s->pci = AMD_IOMMU_PCI(object_new(TYPE_AMD_IOMMU_PCI)); > + /* This device should take care of IOMMU PCI properties */ > + if (!qdev_realize(DEVICE(s->pci), &bus->qbus, errp)) { > + return; > + } > } > > + s->iotlb = g_hash_table_new_full(amdvi_uint64_hash, > + amdvi_uint64_equal, g_free, g_free); > + > /* Pseudo address space under root PCI bus. */ > x86ms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_IOAPIC_SB_DEVID); > > @@ -1663,6 +1682,7 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) > > static const Property amdvi_properties[] = { > DEFINE_PROP_BOOL("xtsup", AMDVIState, xtsup, false), > + DEFINE_PROP_STRING("pci-id", AMDVIState, pci_id), > }; > > static const VMStateDescription vmstate_amdvi_sysbus = { > @@ -1670,12 +1690,6 @@ static const VMStateDescription vmstate_amdvi_sysbus = { > .unmigratable = 1 > }; > > -static void amdvi_sysbus_instance_init(Object *klass) > -{ > - AMDVIState *s = AMD_IOMMU_DEVICE(klass); > - > - object_initialize(&s->pci, sizeof(s->pci), TYPE_AMD_IOMMU_PCI); > -} > > static void amdvi_sysbus_class_init(ObjectClass *klass, void *data) > { > @@ -1698,7 +1712,6 @@ static const TypeInfo amdvi_sysbus = { > .name = TYPE_AMD_IOMMU_DEVICE, > .parent = TYPE_X86_IOMMU_DEVICE, > .instance_size = sizeof(AMDVIState), > - .instance_init = amdvi_sysbus_instance_init, > .class_init = amdvi_sysbus_class_init > }; > > diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h > index e0dac4d9a9..ece71ff0b6 100644 > --- a/hw/i386/amd_iommu.h > +++ b/hw/i386/amd_iommu.h > @@ -315,7 +315,8 @@ struct AMDVIPCIState { > > struct AMDVIState { > X86IOMMUState iommu; /* IOMMU bus device */ > - AMDVIPCIState pci; /* IOMMU PCI device */ > + AMDVIPCIState *pci; /* IOMMU PCI device */ > + char *pci_id; /* ID of AMDVI-PCI device, if user created */ > > uint32_t version; > > -- > 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation 2025-02-21 0:38 ` Michael S. Tsirkin @ 2025-02-25 13:39 ` Suthikulpanit, Suravee 0 siblings, 0 replies; 6+ messages in thread From: Suthikulpanit, Suravee @ 2025-02-25 13:39 UTC (permalink / raw) To: Michael S. Tsirkin Cc: qemu-devel, pbonzini, mtosatti, marcel.apfelbaum, jon.grimm, santosh.shukla, vasant.hegde, Wei.Huang2, bsd, berrange, joao.m.martins, alejandro.j.jimenez On 2/21/2025 7:38 AM, Michael S. Tsirkin wrote: > On Wed, Feb 12, 2025 at 05:44:49AM +0000, Suravee Suthikulpanit wrote: >> Current amd-iommu model internally creates an AMDVI-PCI device. Here is >> a snippet from info qtree: >> >> bus: main-system-bus >> type System >> dev: amd-iommu, id "" >> xtsup = false >> pci-id = "" >> intremap = "on" >> device-iotlb = false >> pt = true >> ... >> dev: q35-pcihost, id "" >> MCFG = -1 (0xffffffffffffffff) >> pci-hole64-size = 34359738368 (32 GiB) >> below-4g-mem-size = 134217728 (128 MiB) >> above-4g-mem-size = 0 (0 B) >> smm-ranges = true >> x-pci-hole64-fix = true >> x-config-reg-migration-enabled = true >> bypass-iommu = false >> bus: pcie.0 >> type PCIE >> dev: AMDVI-PCI, id "" >> addr = 01.0 >> romfile = "" >> romsize = 4294967295 (0xffffffff) >> rombar = -1 (0xffffffffffffffff) >> multifunction = false >> x-pcie-lnksta-dllla = true >> x-pcie-extcap-init = true >> failover_pair_id = "" >> acpi-index = 0 (0x0) >> x-pcie-err-unc-mask = true >> x-pcie-ari-nextfn-1 = false >> x-max-bounce-buffer-size = 4096 (4 KiB) >> x-pcie-ext-tag = true >> busnr = 0 (0x0) >> class Class 0806, addr 00:01.0, pci id 1022:0000 (sub 1af4:1100) >> >> This prohibits users from specifying the PCI topology for the amd-iommu device, >> which becomes a problem when trying to support VM migration since it does not >> guarantee the same enumeration of AMD IOMMU device. >> >> Therfore, decouple the AMDVI-PCI from amd-iommu device and introduce pci-id >> parameter to link between the two devices. >> >> For example: >> -device AMDVI-PCI,id=iommupci0,bus=pcie.0,addr=0x05 \ >> -device amd-iommu,intremap=on,pt=on,xtsup=on,pci-id=iommupci0 \ >> >> For backward-compatibility, internally create the AMDVI-PCI device if not >> specified on the CLI. >> >> Co-developed-by: Daniel P. Berrangé<berrange@redhat.com> >> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com> > > breaks build: > > https://gitlab.com/mstredhat/qemu/-/jobs/9202802751 > > ./hw/i386/amd_iommu.c: In function ‘amdvi_sysbus_realize’: > ../hw/i386/amd_iommu.c:1616:18: error: unused variable ‘dc’ [-Werror=unused-variable] > 1616 | DeviceClass *dc = (DeviceClass *) object_get_class(OBJECT(dev)); > | ^~ > cc1: all warnings being treated as errors I'll send out v4 with the fix. Suravee ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] hw/i386/amd_iommu: Allow migration when explicitly create the AMDVI-PCI device 2025-02-12 5:44 [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support Suravee Suthikulpanit 2025-02-12 5:44 ` [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation Suravee Suthikulpanit @ 2025-02-12 5:44 ` Suravee Suthikulpanit 2025-02-20 23:21 ` [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support Michael S. Tsirkin 2 siblings, 0 replies; 6+ messages in thread From: Suravee Suthikulpanit @ 2025-02-12 5:44 UTC (permalink / raw) To: qemu-devel Cc: pbonzini, mtosatti, mst, marcel.apfelbaum, jon.grimm, santosh.shukla, vasant.hegde, Wei.Huang2, bsd, berrange, joao.m.martins, alejandro.j.jimenez, Suravee Suthikulpanit Add migration support for AMD IOMMU model by saving necessary AMDVIState parameters for MMIO registers, device table, command buffer, and event buffers. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- hw/i386/amd_iommu.c | 58 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 0f552bafa0..dda1a5781f 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -1611,6 +1611,57 @@ static void amdvi_sysbus_reset(DeviceState *dev) amdvi_init(s); } +static const VMStateDescription vmstate_amdvi_sysbus = { + .name = "amd-iommu", + .unmigratable = 1 +}; + +static const VMStateDescription vmstate_amdvi_sysbus_migratable = { + .name = "amd-iommu", + .version_id = 1, + .minimum_version_id = 1, + .priority = MIG_PRI_IOMMU, + .fields = (VMStateField[]) { + /* Updated in amdvi_handle_control_write() */ + VMSTATE_BOOL(enabled, AMDVIState), + VMSTATE_BOOL(ga_enabled, AMDVIState), + VMSTATE_BOOL(ats_enabled, AMDVIState), + VMSTATE_BOOL(cmdbuf_enabled, AMDVIState), + VMSTATE_BOOL(completion_wait_intr, AMDVIState), + VMSTATE_BOOL(evtlog_enabled, AMDVIState), + VMSTATE_BOOL(evtlog_intr, AMDVIState), + /* Updated in amdvi_handle_devtab_write() */ + VMSTATE_UINT64(devtab, AMDVIState), + VMSTATE_UINT64(devtab_len, AMDVIState), + /* Updated in amdvi_handle_cmdbase_write() */ + VMSTATE_UINT64(cmdbuf, AMDVIState), + VMSTATE_UINT64(cmdbuf_len, AMDVIState), + /* Updated in amdvi_handle_cmdhead_write() */ + VMSTATE_UINT32(cmdbuf_head, AMDVIState), + /* Updated in amdvi_handle_cmdtail_write() */ + VMSTATE_UINT32(cmdbuf_tail, AMDVIState), + /* Updated in amdvi_handle_evtbase_write() */ + VMSTATE_UINT64(evtlog, AMDVIState), + VMSTATE_UINT32(evtlog_len, AMDVIState), + /* Updated in amdvi_handle_evthead_write() */ + VMSTATE_UINT32(evtlog_head, AMDVIState), + /* Updated in amdvi_handle_evttail_write() */ + VMSTATE_UINT32(evtlog_tail, AMDVIState), + /* Updated in amdvi_handle_pprbase_write() */ + VMSTATE_UINT64(ppr_log, AMDVIState), + VMSTATE_UINT32(pprlog_len, AMDVIState), + /* Updated in amdvi_handle_pprhead_write() */ + VMSTATE_UINT32(pprlog_head, AMDVIState), + /* Updated in amdvi_handle_tailhead_write() */ + VMSTATE_UINT32(pprlog_tail, AMDVIState), + /* MMIO registers */ + VMSTATE_UINT8_ARRAY(mmior, AMDVIState, AMDVI_MMIO_SIZE), + VMSTATE_UINT8_ARRAY(romask, AMDVIState, AMDVI_MMIO_SIZE), + VMSTATE_UINT8_ARRAY(w1cmask, AMDVIState, AMDVI_MMIO_SIZE), + VMSTATE_END_OF_LIST() + } +}; + static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) { DeviceClass *dc = (DeviceClass *) object_get_class(OBJECT(dev)); @@ -1635,6 +1686,7 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) } s->pci = AMD_IOMMU_PCI(pdev); + dc->vmsd = &vmstate_amdvi_sysbus_migratable; } else { s->pci = AMD_IOMMU_PCI(object_new(TYPE_AMD_IOMMU_PCI)); /* This device should take care of IOMMU PCI properties */ @@ -1685,12 +1737,6 @@ static const Property amdvi_properties[] = { DEFINE_PROP_STRING("pci-id", AMDVIState, pci_id), }; -static const VMStateDescription vmstate_amdvi_sysbus = { - .name = "amd-iommu", - .unmigratable = 1 -}; - - static void amdvi_sysbus_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support 2025-02-12 5:44 [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support Suravee Suthikulpanit 2025-02-12 5:44 ` [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation Suravee Suthikulpanit 2025-02-12 5:44 ` [PATCH v3 2/2] hw/i386/amd_iommu: Allow migration when explicitly create the AMDVI-PCI device Suravee Suthikulpanit @ 2025-02-20 23:21 ` Michael S. Tsirkin 2 siblings, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2025-02-20 23:21 UTC (permalink / raw) To: Suravee Suthikulpanit Cc: qemu-devel, pbonzini, mtosatti, marcel.apfelbaum, jon.grimm, santosh.shukla, vasant.hegde, Wei.Huang2, bsd, berrange, joao.m.martins, alejandro.j.jimenez On Wed, Feb 12, 2025 at 05:44:48AM +0000, Suravee Suthikulpanit wrote: > Currently, amd-iommu device does not support migration. This series addresses > an issue due hidden AMDVI-PCI device enumeration. Then introduces migratable > VMStateDescription, which saves necessary parameters for the device. > > Changes from v2: > (https://lore.kernel.org/all/20250206051856.323651-1-suravee.suthikulpanit@amd.com) > * Add patch 1/2 Fails build on 32 bit: https://gitlab.com/mstredhat/qemu/-/jobs/9202574769 In file included from ../include/qemu/osdep.h:53, from ../hw/i386/amd_iommu.c:23: ../include/qemu/compiler.h:70:35: error: invalid operands to binary - (have ‘uint64_t *’ {aka ‘long long unsigned int *’} and ‘size_t *’ {aka ‘unsigned int *’}) 70 | #define type_check(t1,t2) ((t1*)0 - (t2*)0) | ^ ../include/migration/vmstate.h:269:6: note: in expansion of macro ‘type_check’ 269 | type_check(_type, typeof_field(_state, _field))) | ^~~~~~~~~~ ../include/migration/vmstate.h:320:21: note: in expansion of macro ‘vmstate_offset_value’ 320 | .offset = vmstate_offset_value(_state, _field, _type), \ | ^~~~~~~~~~~~~~~~~~~~ ../include/migration/vmstate.h:853:5: note: in expansion of macro ‘VMSTATE_SINGLE_TEST’ 853 | VMSTATE_SINGLE_TEST(_field, _state, NULL, _version, _info, _type) | ^~~~~~~~~~~~~~~~~~~ ../include/migration/vmstate.h:903:5: note: in expansion of macro ‘VMSTATE_SINGLE’ 903 | VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint64, uint64_t) | ^~~~~~~~~~~~~~ ../include/migration/vmstate.h:937:5: note: in expansion of macro ‘VMSTATE_UINT64_V’ 937 | VMSTATE_UINT64_V(_f, _s, 0) | ^~~~~~~~~~~~~~~~ ../hw/i386/amd_iommu.c:1635:7: note: in expansion of macro ‘VMSTATE_UINT64’ 1635 | VMSTATE_UINT64(devtab_len, AMDVIState), | ^~~~~~~~~~~~~~ > Suravee Suthikulpanit (2): > hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow > full control over the PCI device creation > hw/i386/amd_iommu: Allow migration when explicitly create the > AMDVI-PCI device > > hw/i386/acpi-build.c | 8 ++-- > hw/i386/amd_iommu.c | 111 +++++++++++++++++++++++++++++++++---------- > hw/i386/amd_iommu.h | 3 +- > 3 files changed, 91 insertions(+), 31 deletions(-) > > -- > 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-25 13:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-12 5:44 [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support Suravee Suthikulpanit 2025-02-12 5:44 ` [PATCH v3 1/2] hw/i386/amd_iommu: Isolate AMDVI-PCI from amd-iommu device to allow full control over the PCI device creation Suravee Suthikulpanit 2025-02-21 0:38 ` Michael S. Tsirkin 2025-02-25 13:39 ` Suthikulpanit, Suravee 2025-02-12 5:44 ` [PATCH v3 2/2] hw/i386/amd_iommu: Allow migration when explicitly create the AMDVI-PCI device Suravee Suthikulpanit 2025-02-20 23:21 ` [PATCH v3 0/2] hw/i386/amd_iommu: Add migration support Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).