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