* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.