[PATCH] hw/i386/amd_iommu: Allow migration

[PATCH] hw/i386/amd_iommu: Allow migration

[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 | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 13af7211e1..3d2bb9d81e 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1673,7 +1673,41 @@ 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),
+      /* 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)
-- 
2.34.1

Re: [PATCH] hw/i386/amd_iommu: Allow migration

[Joao Martins]

On 20/11/2024 07:31, 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>
> ---
>  hw/i386/amd_iommu.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 13af7211e1..3d2bb9d81e 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1673,7 +1673,41 @@ 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),

no xtsup ?  I guess you are relying on the dest command line having xtsup=on
like intel-iommu

> +      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),

Are we missing:

	ppr_log
	pprlog_len
	pprlog_head
	pprlog_tail

  ?

Although perhaps excluding it was deliberate given that these aren't actually
fed with PPR log entries, only register initialization. Given no PPR entries are
generated it's doing nothing useful.

Out of correctness this is guest initialized data, so perhaps it should be
included such that it doesn't suddenly see different values on destination. In
theory you 'just' need to wire in qemu a VF generating PPR log entries for page
requests, so the log will eventually be what you need to migrate anyhow like the
event log...?

> +      /* 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)

Re: [PATCH] hw/i386/amd_iommu: Allow migration

[Joao Martins]

On 21/11/2024 11:42, Joao Martins wrote:> On 20/11/2024 07:31, 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>
>> ---
>>  hw/i386/amd_iommu.c | 36 +++++++++++++++++++++++++++++++++++-
>>  1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 13af7211e1..3d2bb9d81e 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -1673,7 +1673,41 @@ 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),
>
> no xtsup ?  I guess you are relying on the dest command line having xtsup=on
> like intel-iommu
>

Having said, I think I found a flaw here that sort of "ignores" the default
command line param of 'device-iotlb' (broad x86-iommu param). By default it
seems we enable device-iotlb in amd-iommu regardless, even though it's disabled
by default in qemu command line params.

Should we enable migration I think stuff like that starts to be important to
honor given the compability issues we would have to deal apriori. See below on
how to fix, happy to formally send if what I explained makes sense to all

-------->8---------

Subject: i386/amd-iommu: Set IotblSup based on x86_iommu::dt_supported

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9fcc2897b84a..f30e103d649b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2310,7 +2310,8 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker,
const char *oem_id,
     /* virtualization flags */
     build_append_int_noprefix(table_data,
                              (1UL << 0) | /* HtTunEn      */
-                             (1UL << 4) | /* iotblSup     */
+                             (s->iommu.dt_supported ?
+                              (1UL << 4) : 0) |  /* iotblSup     */
                              (1UL << 6) | /* PrefSup      */
                              (1UL << 7),  /* PPRSup       */
                              1);
@@ -2347,7 +2348,8 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker,
const char *oem_id,
     /* virtualization flags */
     build_append_int_noprefix(table_data,
                              (1UL << 0) | /* HtTunEn      */
-                             (1UL << 4),  /* iotblSup     */
+                             (s->iommu.dt_supported ?
+                              (1UL << 4) : 0),  /* iotblSup     */
                              1);

     /* IVHD length */
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 13af7211e11d..7e6140b9a0a2 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -77,6 +77,18 @@ typedef struct AMDVIIOTLBEntry {
     uint64_t page_mask;         /* physical page size  */
 } AMDVIIOTLBEntry;

+static uint64_t amdvi_capab_feature_register(AMDVIState *s)
+{
+    uint64_t feature = AMDVI_CAPAB_FEATURES;
+    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
+
+    if (x86_iommu->dt_supported) {
+        feature |= AMDVI_CAPAB_FLAG_IOTLBSUP;
+    }
+
+    return feature;
+}
+
 uint64_t amdvi_extended_feature_register(AMDVIState *s)
 {
     uint64_t feature = AMDVI_DEFAULT_EXT_FEATURES;
@@ -1563,6 +1575,7 @@ static void amdvi_init(AMDVIState *s)
 static void amdvi_pci_realize(PCIDevice *pdev, Error **errp)
 {
     AMDVIPCIState *s = AMD_IOMMU_PCI(pdev);
+    AMDVIState *amdvi = container_of(s, AMDVIState, pci);
     int ret;

     ret = pci_add_capability(pdev, AMDVI_CAPAB_ID_SEC, 0,
@@ -1591,7 +1604,7 @@ static void amdvi_pci_realize(PCIDevice *pdev, Error **errp)
     pci_config_set_prog_interface(pdev->config, 0);

     /* reset AMDVI specific capabilities, all r/o */
-    pci_set_long(pdev->config + s->capab_offset, AMDVI_CAPAB_FEATURES);
+    pci_set_long(pdev->config + s->capab_offset,
amdvi_capab_feature_register(amdvi));
     pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_BAR_LOW,
                  AMDVI_BASE_ADDR & ~(0xffff0000));
     pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_BAR_HIGH,
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index e0dac4d9a96c..98e155549532 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -182,7 +182,7 @@

 /* capabilities header */
 #define AMDVI_CAPAB_FEATURES (AMDVI_CAPAB_FLAT_EXT | \
-        AMDVI_CAPAB_FLAG_NPCACHE | AMDVI_CAPAB_FLAG_IOTLBSUP \
+        AMDVI_CAPAB_FLAG_NPCACHE \
         | AMDVI_CAPAB_ID_SEC | AMDVI_CAPAB_INIT_TYPE | \
         AMDVI_CAPAB_FLAG_HTTUNNEL |  AMDVI_CAPAB_EFR_SUP)

Re: [PATCH] hw/i386/amd_iommu: Allow migration

[Vasant Hegde]

Hi Joao,


On 11/28/2024 10:44 PM, Joao Martins wrote:
> On 21/11/2024 11:42, Joao Martins wrote:> On 20/11/2024 07:31, 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>
>>> ---
>>>  hw/i386/amd_iommu.c | 36 +++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>>> index 13af7211e1..3d2bb9d81e 100644
>>> --- a/hw/i386/amd_iommu.c
>>> +++ b/hw/i386/amd_iommu.c
>>> @@ -1673,7 +1673,41 @@ 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),
>>
>> no xtsup ?  I guess you are relying on the dest command line having xtsup=on
>> like intel-iommu
>>
> 
> Having said, I think I found a flaw here that sort of "ignores" the default
> command line param of 'device-iotlb' (broad x86-iommu param). By default it
> seems we enable device-iotlb in amd-iommu regardless, even though it's disabled
> by default in qemu command line params.
> 
> Should we enable migration I think stuff like that starts to be important to
> honor given the compability issues we would have to deal apriori. See below on
> how to fix, happy to formally send if what I explained makes sense to all
> 

Below patch looks good. Can you please send formal patch?

-Vasant

Re: [PATCH] hw/i386/amd_iommu: Allow migration

[Suthikulpanit, Suravee]

On 11/29/2024 12:14 AM, Joao Martins wrote:
> On 21/11/2024 11:42, Joao Martins wrote:> On 20/11/2024 07:31, 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>
>>> ---
>>>   hw/i386/amd_iommu.c | 36 +++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>>> index 13af7211e1..3d2bb9d81e 100644
>>> --- a/hw/i386/amd_iommu.c
>>> +++ b/hw/i386/amd_iommu.c
>>> @@ -1673,7 +1673,41 @@ 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),
>>
>> no xtsup ?  I guess you are relying on the dest command line having xtsup=on
>> like intel-iommu
>>
> 
> Having said, I think I found a flaw here that sort of "ignores" the default
> command line param of 'device-iotlb' (broad x86-iommu param). By default it
> seems we enable device-iotlb in amd-iommu regardless, even though it's disabled
> by default in qemu command line params.
> 
> Should we enable migration I think stuff like that starts to be important to
> honor given the compability issues we would have to deal apriori. See below on
> how to fix, happy to formally send if what I explained makes sense to all

As you mentioned, AMD vIOMMU currently ignore this parameter and QEMU 
default dt_supported to false, and set the IOTLBSup in the IVRS table to 
1. If we start interpret this option, we need to also emulate the 
following cases when dt_supported=0:

* When DTE[I]=1. From AMD IOMMU Spec:

I: IOTLB enable. Controls IOMMU response to address translation requests 
from peripherals. 0=IOMMU returns target abort status when it receives 
an ATS requests from the peripheral. 1=IOMMU responds to ATS requests 
from the peripheral. This bit does not affect interrupts from the 
peripheral. If I=1 when Capability Offset 00h[IotlbSup]=0, the results 
are undefined.

* Handle the INVALIDATE_IOTLB_PAGES command, which also needs 
dt_supported=1.

Apart from trying to be compatible w/ the x86-iommu dt_supported param 
(which is mostly for Intel vIOMMU), I am not sure if handling the 
dt_supported is necessary for AMD vIOMMU. Do we have a scenario where we 
need to disable ATS support for vIOMMU?

Thanks,
Suravee

Re: [PATCH] hw/i386/amd_iommu: Allow migration

[Joao Martins]

On 05/02/2025 02:45, Suthikulpanit, Suravee wrote:
> 
> 
> On 11/29/2024 12:14 AM, Joao Martins wrote:
>> On 21/11/2024 11:42, Joao Martins wrote:> On 20/11/2024 07:31, 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>
>>>> ---
>>>>   hw/i386/amd_iommu.c | 36 +++++++++++++++++++++++++++++++++++-
>>>>   1 file changed, 35 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>>>> index 13af7211e1..3d2bb9d81e 100644
>>>> --- a/hw/i386/amd_iommu.c
>>>> +++ b/hw/i386/amd_iommu.c
>>>> @@ -1673,7 +1673,41 @@ 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),
>>>
>>> no xtsup ?  I guess you are relying on the dest command line having xtsup=on
>>> like intel-iommu
>>>
>>
>> Having said, I think I found a flaw here that sort of "ignores" the default
>> command line param of 'device-iotlb' (broad x86-iommu param). By default it
>> seems we enable device-iotlb in amd-iommu regardless, even though it's disabled
>> by default in qemu command line params.
>>
>> Should we enable migration I think stuff like that starts to be important to
>> honor given the compability issues we would have to deal apriori. See below on
>> how to fix, happy to formally send if what I explained makes sense to all
> 
> As you mentioned, AMD vIOMMU currently ignore this parameter and QEMU default
> dt_supported to false, and set the IOTLBSup in the IVRS table to 1. If we start
> interpret this option, we need to also emulate the following cases when
> dt_supported=0:
> 
> * When DTE[I]=1. From AMD IOMMU Spec:
> 
> I: IOTLB enable. Controls IOMMU response to address translation requests from
> peripherals. 0=IOMMU returns target abort status when it receives an ATS
> requests from the peripheral. 1=IOMMU responds to ATS requests from the
> peripheral. This bit does not affect interrupts from the peripheral. If I=1 when
> Capability Offset 00h[IotlbSup]=0, the results are undefined.
> 
> * Handle the INVALIDATE_IOTLB_PAGES command, which also needs dt_supported=1.
> 
> Apart from trying to be compatible w/ the x86-iommu dt_supported param (which is
> mostly for Intel vIOMMU), I am not sure if handling the dt_supported is
> necessary for AMD vIOMMU.

I agree; I certainly not suggesting going out of our way to support
device-iotlb=1 with my command. We could throw an error and fail the amd-iommu
device realize if we wanna explicit that amd vIOMMU doesn't support device_iotlb=1.

> Do we have a scenario where we need to disable ATS
> support for vIOMMU?
> It's closer to reality that ATS isn't quite fully exerciable in Qemu (until this
series lands IIUC:
https://lore.kernel.org/qemu-devel/20250120174033.308518-20-clement.mathieu--drif@eviden.com/).


My whole point was to honor the default, which is disabled and not mis-advertise
it as being supported. Specially considering the cases abov you point out look
to be be obvious gaps that aren't implemented, all the more reason to not be
misadvertising it.

Re: [PATCH] hw/i386/amd_iommu: Allow migration

[Suthikulpanit, Suravee]

On 11/21/2024 6:42 PM, Joao Martins wrote:
> On 20/11/2024 07:31, 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>
>> ---
>>   hw/i386/amd_iommu.c | 36 +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 13af7211e1..3d2bb9d81e 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -1673,7 +1673,41 @@ 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),
> no xtsup ?  I guess you are relying on the dest command line having xtsup=on
> like intel-iommu
> 
>> +      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),
> Are we missing:
> 
> 	ppr_log
> 	pprlog_len
> 	pprlog_head
> 	pprlog_tail
> 
>    ?
> 
> Although perhaps excluding it was deliberate given that these aren't actually
> fed with PPR log entries, only register initialization. Given no PPR entries are
> generated it's doing nothing useful.
> 
> Out of correctness this is guest initialized data, so perhaps it should be
> included such that it doesn't suddenly see different values on destination. In
> theory you 'just' need to wire in qemu a VF generating PPR log entries for page
> requests, so the log will eventually be what you need to migrate anyhow like the
> event log...?
> 

Thanks for bringing up this point. I'll send out v2 with ppr stuff.

Suravee.