| Message ID | 20250206051856.323651-1-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] hw/i386/amd_iommu: Allow migration | expand |
On Thu, Feb 06, 2025 at 05:18:56AM +0000, Suravee Suthikulpanit wrote: > 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> > --- > Changes from v1: > (https://lore.kernel.org/all/9ecffa7a-f4c6-45a5-a066-84826ccb55dd@amd.com/T/) > * Include ppr_log, pprlog_len, pprlog_head, pprlog_tail per Joao. > > hw/i386/amd_iommu.c | 43 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 42 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c > index 13af7211e1..a1940a0ab3 100644 > --- a/hw/i386/amd_iommu.c > +++ b/hw/i386/amd_iommu.c > @@ -1673,7 +1673,48 @@ static Property amdvi_properties[] = { > > static const VMStateDescription vmstate_amdvi_sysbus = { > .name = "amd-iommu", > - .unmigratable = 1 > + .version_id = 1, IMHO we should not remove the 'unmigratable = 1' setting, until we have fixed the design problem with this device, to split the creation of the 'AMDVI-PCI' device off from the 'amd-iommu' device, so that the former is user creatable. As it stands now, there's no mechanism to guarantee that the internally created 'AMDVI-PCI' device gets the same PCI address properties on every boot. Thus it isn't safe to claim this device is migratable yet IMHO. > + .minimum_version_id = 1, > + .priority = MIG_PRI_IOMMU, > + .fields = (VMStateField[]) { With regards, Daniel
Daniel, On 2/6/2025 4:01 PM, Daniel P. Berrangé wrote: > On Thu, Feb 06, 2025 at 05:18:56AM +0000, Suravee Suthikulpanit wrote: >> 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> >> --- >> Changes from v1: >> (https://lore.kernel.org/all/9ecffa7a-f4c6-45a5- >> a066-84826ccb55dd@amd.com/T/) >> * Include ppr_log, pprlog_len, pprlog_head, pprlog_tail per Joao. >> >> hw/i386/amd_iommu.c | 43 ++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 42 insertions(+), 1 deletion(-) >> >> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c >> index 13af7211e1..a1940a0ab3 100644 >> --- a/hw/i386/amd_iommu.c >> +++ b/hw/i386/amd_iommu.c >> @@ -1673,7 +1673,48 @@ static Property amdvi_properties[] = { >> >> static const VMStateDescription vmstate_amdvi_sysbus = { >> .name = "amd-iommu", >> - .unmigratable = 1 >> + .version_id = 1, > IMHO we should not remove the 'unmigratable = 1' setting, until we > have fixed the design problem with this device, to split the creation > of the 'AMDVI-PCI' device off from the 'amd-iommu' device, so that the > former is user creatable. > > As it stands now, there's no mechanism to guarantee that the internally > created 'AMDVI-PCI' device gets the same PCI address properties on every > boot. Thus it isn't safe to claim this device is migratable yet IMHO. I'll send out v3 including the change you suggested, where we split the creation of AMDVI-PCI device to allow fix enumeration of amd-iommu device. Thanks, Suravee
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 13af7211e1..a1940a0ab3 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -1673,7 +1673,48 @@ static Property amdvi_properties[] = { static const VMStateDescription vmstate_amdvi_sysbus = { .name = "amd-iommu", - .unmigratable = 1 + .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_instance_init(Object *klass)
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> --- Changes from v1: (https://lore.kernel.org/all/9ecffa7a-f4c6-45a5-a066-84826ccb55dd@amd.com/T/) * Include ppr_log, pprlog_len, pprlog_head, pprlog_tail per Joao. hw/i386/amd_iommu.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-)