RE: [PATCH v1 13/22] vfio: Add base container
>-Original Message- >From: Eric Auger >Sent: Friday, September 22, 2023 1:20 AM >Subject: Re: [PATCH v1 13/22] vfio: Add base container > >Hi Zhenzhong, >On 9/21/23 05:35, Duan, Zhenzhong wrote: >> Hi Eric, >> >>> -Original Message- >>> From: Eric Auger >>> Sent: Thursday, September 21, 2023 1:31 AM >>> Subject: Re: [PATCH v1 13/22] vfio: Add base container >>> >>> Hi Zhenzhong, >>> >>> On 9/19/23 19:23, Cédric Le Goater wrote: >>>> On 8/30/23 12:37, Zhenzhong Duan wrote: >>>>> From: Yi Liu >>>>> >>>>> Abstract the VFIOContainer to be a base object. It is supposed to be >>>>> embedded by legacy VFIO container and later on, into the new iommufd >>>>> based container. >>>>> >>>>> The base container implements generic code such as code related to >>>>> memory_listener and address space management. The VFIOContainerOps >>>>> implements callbacks that depend on the kernel user space being used. >>>>> >>>>> 'common.c' and vfio device code only manipulates the base container with >>>>> wrapper functions that calls the functions defined in >>>>> VFIOContainerOpsClass. >>>>> Existing 'container.c' code is converted to implement the legacy >>>>> container >>>>> ops functions. >>>>> >>>>> Below is the base container. It's named as VFIOContainer, old >>>>> VFIOContainer >>>>> is replaced with VFIOLegacyContainer. >>>> Usualy, we introduce the new interface solely, port the current models >>>> on top of the new interface, wire the new models in the current >>>> implementation and remove the old implementation. Then, we can start >>>> adding extensions to support other implementations. >>>> >>>> spapr should be taken care of separatly following the principle above. >>>> With my PPC hat, I would not even read such a massive change, too risky >>>> for the subsystem. This path will need (much) further splitting to be >>>> understandable and acceptable. >>> We might split this patch by >>> 1) introducing VFIOLegacyContainer encapsulating the base VFIOContainer, >>> without using the ops in a first place: >>> common.c would call vfio_container_* with harcoded legacy >>> implementation, ie. retrieving the legacy container with container_of. >>> 2) we would introduce the BE interface without using it. >>> 3) we would use the new BE interface >>> >>> Obviously this needs to be further tried out. If you wish I can try to >>> split it that way ... Please let me know >> Sure, thanks for your help, glad that I can cooperate with you to move >> this series forward. >> I just updated the branch which rebased to newest upstream for you to pick at >https://github.com/yiliu1765/qemu/tree/zhenzhong/iommufd_cdev_v1_rebased > >I have spent most of my day reshuffling this single patch into numerous >ones (16!). This should help the review. >I was short of time. This compiles, the end code should be identical to >the original one. Besides this deserves some additional review on your >end, commit msg tuning, ... > >But at least it is a move forward. Feel free to incorporate that in your >next respin. > >Please find that work on the following branch > >https://github.com/eauger/qemu/tree/iommufd_cdev_v1_rebased_split Thanks Eric, you have done a so quick and awesome work. Let me learn your change and integrate with my other changes. Will get back to you then. BRs. Zhenzhong
Re: [PATCH v1 13/22] vfio: Add base container
Hi Zhenzhong, On 9/21/23 05:35, Duan, Zhenzhong wrote: > Hi Eric, > >> -Original Message- >> From: Eric Auger >> Sent: Thursday, September 21, 2023 1:31 AM >> Subject: Re: [PATCH v1 13/22] vfio: Add base container >> >> Hi Zhenzhong, >> >> On 9/19/23 19:23, Cédric Le Goater wrote: >>> On 8/30/23 12:37, Zhenzhong Duan wrote: >>>> From: Yi Liu >>>> >>>> Abstract the VFIOContainer to be a base object. It is supposed to be >>>> embedded by legacy VFIO container and later on, into the new iommufd >>>> based container. >>>> >>>> The base container implements generic code such as code related to >>>> memory_listener and address space management. The VFIOContainerOps >>>> implements callbacks that depend on the kernel user space being used. >>>> >>>> 'common.c' and vfio device code only manipulates the base container with >>>> wrapper functions that calls the functions defined in >>>> VFIOContainerOpsClass. >>>> Existing 'container.c' code is converted to implement the legacy >>>> container >>>> ops functions. >>>> >>>> Below is the base container. It's named as VFIOContainer, old >>>> VFIOContainer >>>> is replaced with VFIOLegacyContainer. >>> Usualy, we introduce the new interface solely, port the current models >>> on top of the new interface, wire the new models in the current >>> implementation and remove the old implementation. Then, we can start >>> adding extensions to support other implementations. >>> >>> spapr should be taken care of separatly following the principle above. >>> With my PPC hat, I would not even read such a massive change, too risky >>> for the subsystem. This path will need (much) further splitting to be >>> understandable and acceptable. >> We might split this patch by >> 1) introducing VFIOLegacyContainer encapsulating the base VFIOContainer, >> without using the ops in a first place: >> common.c would call vfio_container_* with harcoded legacy >> implementation, ie. retrieving the legacy container with container_of. >> 2) we would introduce the BE interface without using it. >> 3) we would use the new BE interface >> >> Obviously this needs to be further tried out. If you wish I can try to >> split it that way ... Please let me know > Sure, thanks for your help, glad that I can cooperate with you to move > this series forward. > I just updated the branch which rebased to newest upstream for you to pick at > https://github.com/yiliu1765/qemu/tree/zhenzhong/iommufd_cdev_v1_rebased I have spent most of my day reshuffling this single patch into numerous ones (16!). This should help the review. I was short of time. This compiles, the end code should be identical to the original one. Besides this deserves some additional review on your end, commit msg tuning, ... But at least it is a move forward. Feel free to incorporate that in your next respin. Please find that work on the following branch https://github.com/eauger/qemu/tree/iommufd_cdev_v1_rebased_split Thanks Eric > > Thanks > Zhenzhong
Re: [PATCH v1 13/22] vfio: Add base container
Hi, On 9/21/23 05:35, Duan, Zhenzhong wrote: > Hi Eric, > >> -Original Message- >> From: Eric Auger >> Sent: Thursday, September 21, 2023 1:31 AM >> Subject: Re: [PATCH v1 13/22] vfio: Add base container >> >> Hi Zhenzhong, >> >> On 9/19/23 19:23, Cédric Le Goater wrote: >>> On 8/30/23 12:37, Zhenzhong Duan wrote: >>>> From: Yi Liu >>>> >>>> Abstract the VFIOContainer to be a base object. It is supposed to be >>>> embedded by legacy VFIO container and later on, into the new iommufd >>>> based container. >>>> >>>> The base container implements generic code such as code related to >>>> memory_listener and address space management. The VFIOContainerOps >>>> implements callbacks that depend on the kernel user space being used. >>>> >>>> 'common.c' and vfio device code only manipulates the base container with >>>> wrapper functions that calls the functions defined in >>>> VFIOContainerOpsClass. >>>> Existing 'container.c' code is converted to implement the legacy >>>> container >>>> ops functions. >>>> >>>> Below is the base container. It's named as VFIOContainer, old >>>> VFIOContainer >>>> is replaced with VFIOLegacyContainer. >>> Usualy, we introduce the new interface solely, port the current models >>> on top of the new interface, wire the new models in the current >>> implementation and remove the old implementation. Then, we can start >>> adding extensions to support other implementations. >>> >>> spapr should be taken care of separatly following the principle above. >>> With my PPC hat, I would not even read such a massive change, too risky >>> for the subsystem. This path will need (much) further splitting to be >>> understandable and acceptable. >> We might split this patch by >> 1) introducing VFIOLegacyContainer encapsulating the base VFIOContainer, >> without using the ops in a first place: >> common.c would call vfio_container_* with harcoded legacy >> implementation, ie. retrieving the legacy container with container_of. >> 2) we would introduce the BE interface without using it. >> 3) we would use the new BE interface >> >> Obviously this needs to be further tried out. If you wish I can try to >> split it that way ... Please let me know > Sure, thanks for your help, glad that I can cooperate with you to move > this series forward. > I just updated the branch which rebased to newest upstream for you to pick at > https://github.com/yiliu1765/qemu/tree/zhenzhong/iommufd_cdev_v1_rebased OK thanks. Let me do the exercise. Eric > > Thanks > Zhenzhong
RE: [PATCH v1 13/22] vfio: Add base container
Hi Eric, >-Original Message- >From: Eric Auger >Sent: Thursday, September 21, 2023 1:31 AM >Subject: Re: [PATCH v1 13/22] vfio: Add base container > >Hi Zhenzhong, > >On 9/19/23 19:23, Cédric Le Goater wrote: >> On 8/30/23 12:37, Zhenzhong Duan wrote: >>> From: Yi Liu >>> >>> Abstract the VFIOContainer to be a base object. It is supposed to be >>> embedded by legacy VFIO container and later on, into the new iommufd >>> based container. >>> >>> The base container implements generic code such as code related to >>> memory_listener and address space management. The VFIOContainerOps >>> implements callbacks that depend on the kernel user space being used. >>> >>> 'common.c' and vfio device code only manipulates the base container with >>> wrapper functions that calls the functions defined in >>> VFIOContainerOpsClass. >>> Existing 'container.c' code is converted to implement the legacy >>> container >>> ops functions. >>> >>> Below is the base container. It's named as VFIOContainer, old >>> VFIOContainer >>> is replaced with VFIOLegacyContainer. >> >> Usualy, we introduce the new interface solely, port the current models >> on top of the new interface, wire the new models in the current >> implementation and remove the old implementation. Then, we can start >> adding extensions to support other implementations. >> >> spapr should be taken care of separatly following the principle above. >> With my PPC hat, I would not even read such a massive change, too risky >> for the subsystem. This path will need (much) further splitting to be >> understandable and acceptable. >We might split this patch by >1) introducing VFIOLegacyContainer encapsulating the base VFIOContainer, >without using the ops in a first place: > common.c would call vfio_container_* with harcoded legacy >implementation, ie. retrieving the legacy container with container_of. >2) we would introduce the BE interface without using it. >3) we would use the new BE interface > >Obviously this needs to be further tried out. If you wish I can try to >split it that way ... Please let me know Sure, thanks for your help, glad that I can cooperate with you to move this series forward. I just updated the branch which rebased to newest upstream for you to pick at https://github.com/yiliu1765/qemu/tree/zhenzhong/iommufd_cdev_v1_rebased Thanks Zhenzhong
RE: [PATCH v1 13/22] vfio: Add base container
>-Original Message- >From: Eric Auger >Sent: Wednesday, September 20, 2023 9:54 PM >Subject: Re: [PATCH v1 13/22] vfio: Add base container > >Hi Cedric, > >On 9/19/23 19:23, Cédric Le Goater wrote: >> On 8/30/23 12:37, Zhenzhong Duan wrote: >>> From: Yi Liu >>> >>> Abstract the VFIOContainer to be a base object. It is supposed to be >>> embedded by legacy VFIO container and later on, into the new iommufd >>> based container. >>> >>> The base container implements generic code such as code related to >>> memory_listener and address space management. The VFIOContainerOps >>> implements callbacks that depend on the kernel user space being used. >>> >>> 'common.c' and vfio device code only manipulates the base container with >>> wrapper functions that calls the functions defined in >>> VFIOContainerOpsClass. >>> Existing 'container.c' code is converted to implement the legacy >>> container >>> ops functions. >>> >>> Below is the base container. It's named as VFIOContainer, old >>> VFIOContainer >>> is replaced with VFIOLegacyContainer. >> >> Usualy, we introduce the new interface solely, port the current models >> on top of the new interface, wire the new models in the current >> implementation and remove the old implementation. Then, we can start >> adding extensions to support other implementations. >> spapr should be taken care of separatly following the principle above. >> With my PPC hat, I would not even read such a massive change, too risky >> for the subsystem. This path will need (much) further splitting to be >> understandable and acceptable. >> >> Also, please include the .h file first, it helps in reading. Have you >> considered using an InterfaceClass ? >in the transition from v1 -> v2, I removed the QOMification of the >VFIOContainer, following David Gibson's advice. QOM objects are visible >from the user interface and there was no interest in that. Does it >answer your question? > >- remove the QOMification of the VFIOContainer and simply use standard ops >(David) > >Unfortunately the coverletter log history has disappeared in this new version. >Zhenzhong, I think it is useful to understand how the series moves on. I have archive it with a link https://lists.nongnu.org/archive/html/qemu-devel/2023-07/msg02529.html for cleaner cover letter, looks I'm wrong. I'll restore the whole changelog in v2. Thanks Zhenzhong
RE: [PATCH v1 13/22] vfio: Add base container
>-Original Message-
>From: Cédric Le Goater
>Sent: Wednesday, September 20, 2023 8:58 PM
>Subject: Re: [PATCH v1 13/22] vfio: Add base container
>
>On 9/20/23 10:48, Duan, Zhenzhong wrote:
>>
>>
>>> -Original Message-
>>> From: Cédric Le Goater
>>> Sent: Wednesday, September 20, 2023 1:24 AM
>>> Subject: Re: [PATCH v1 13/22] vfio: Add base container
>>>
>>> On 8/30/23 12:37, Zhenzhong Duan wrote:
>>>> From: Yi Liu
>>>>
>>>> Abstract the VFIOContainer to be a base object. It is supposed to be
>>>> embedded by legacy VFIO container and later on, into the new iommufd
>>>> based container.
>>>>
>>>> The base container implements generic code such as code related to
>>>> memory_listener and address space management. The VFIOContainerOps
>>>> implements callbacks that depend on the kernel user space being used.
>>>>
>>>> 'common.c' and vfio device code only manipulates the base container with
>>>> wrapper functions that calls the functions defined in
>>>> VFIOContainerOpsClass.
>>>> Existing 'container.c' code is converted to implement the legacy container
>>>> ops functions.
>>>>
>>>> Below is the base container. It's named as VFIOContainer, old VFIOContainer
>>>> is replaced with VFIOLegacyContainer.
>>>
>>> Usualy, we introduce the new interface solely, port the current models
>>> on top of the new interface, wire the new models in the current
>>> implementation and remove the old implementation. Then, we can start
>>> adding extensions to support other implementations.
>>
>> Not sure if I understand your point correctly. Do you mean to introduce
>> a new type for the base container as below:
>>
>> static const TypeInfo vfio_container_info = {
>> .parent = TYPE_OBJECT,
>> .name = TYPE_VFIO_CONTAINER,
>> .class_size = sizeof(VFIOContainerClass),
>> .instance_size = sizeof(VFIOContainer),
>> .abstract = true,
>> .interfaces = (InterfaceInfo[]) {
>> { TYPE_VFIO_IOMMU_BACKEND_OPS },
>> { }
>> }
>> };
>>
>> and a new interface as below:
>>
>> static const TypeInfo nvram_info = {
>> .name = TYPE_VFIO_IOMMU_BACKEND_OPS,
>> .parent = TYPE_INTERFACE,
>> .class_size = sizeof(VFIOIOMMUBackendOpsClass),
>> };
>>
>> struct VFIOIOMMUBackendOpsClass {
>> InterfaceClass parent;
>> VFIODevice *(*dev_iter_next)(VFIOContainer *container, VFIODevice
>> *curr);
>> int (*dma_map)(VFIOContainer *container,
>> ..
>> };
>>
>> and legacy container on top of TYPE_VFIO_CONTAINER?
>>
>> static const TypeInfo vfio_legacy_container_info = {
>> .parent = TYPE_VFIO_CONTAINER,
>> .name = TYPE_VFIO_LEGACY_CONTAINER,
>> .class_init = vfio_legacy_container_class_init,
>> };
>>
>> This object style is rejected early in RFCv1.
>> See https://lore.kernel.org/kvm/20220414104710.28534-8-yi.l@intel.com/
>
>ouch. this is long ago and I was not aware :/ Bare with me, I will
>probably ask the same questions. Nevertheless, we could improve the
>cover and the flow of changes in the patchset to help the reader.
Sure.
>
>>> spapr should be taken care of separatly following the principle above.
>>> With my PPC hat, I would not even read such a massive change, too risky
>>> for the subsystem. This path will need (much) further splitting to be
>>> understandable and acceptable.
>>
>> I'll digging into this and try to split it.
>
>I know I am asking for a lot of work. Thanks for that.
Np, all comments, suggestions, etc are appreciated
>
>> Meanwhile, there are many changes
>> just renaming the parameter or function name for code readability.
>> For example:
>>
>> -int vfio_dma_unmap(VFIOContainer *container, hwaddr iova,
>> - ram_addr_t size, IOMMUTLBEntry *iotlb)
>> +static int vfio_legacy_dma_unmap(VFIOContainer *bcontainer, hwaddr iova,
>> + ram_addr_t size, IOMMUTLBEntry *iotlb)
>>
>> -ret = vfio_get_dirty_bitmap(container, iova, size,
>> +ret = vfio_get_dirty_bitmap(bcontainer, iova, size,
>>
>> Let me know if you think such changes are unnecessary which could reduce
>> this patch largely.
>
>Cleanups, renames, some code reshuffling, anything preparing ground for
&
Re: [PATCH v1 13/22] vfio: Add base container
Hi Zhenzhong,
On 9/19/23 19:23, Cédric Le Goater wrote:
> On 8/30/23 12:37, Zhenzhong Duan wrote:
>> From: Yi Liu
>>
>> Abstract the VFIOContainer to be a base object. It is supposed to be
>> embedded by legacy VFIO container and later on, into the new iommufd
>> based container.
>>
>> The base container implements generic code such as code related to
>> memory_listener and address space management. The VFIOContainerOps
>> implements callbacks that depend on the kernel user space being used.
>>
>> 'common.c' and vfio device code only manipulates the base container with
>> wrapper functions that calls the functions defined in
>> VFIOContainerOpsClass.
>> Existing 'container.c' code is converted to implement the legacy
>> container
>> ops functions.
>>
>> Below is the base container. It's named as VFIOContainer, old
>> VFIOContainer
>> is replaced with VFIOLegacyContainer.
>
> Usualy, we introduce the new interface solely, port the current models
> on top of the new interface, wire the new models in the current
> implementation and remove the old implementation. Then, we can start
> adding extensions to support other implementations.
>
> spapr should be taken care of separatly following the principle above.
> With my PPC hat, I would not even read such a massive change, too risky
> for the subsystem. This path will need (much) further splitting to be
> understandable and acceptable.
We might split this patch by
1) introducing VFIOLegacyContainer encapsulating the base VFIOContainer,
without using the ops in a first place:
common.c would call vfio_container_* with harcoded legacy
implementation, ie. retrieving the legacy container with container_of.
2) we would introduce the BE interface without using it.
3) we would use the new BE interface
Obviously this needs to be further tried out. If you wish I can try to
split it that way ... Please let me know
Eric
>
> Also, please include the .h file first, it helps in reading. Have you
> considered using an InterfaceClass ?
>
> Thanks,
>
> C.
>
>>
>> struct VFIOContainer {
>> VFIOIOMMUBackendOpsClass *ops;
>> VFIOAddressSpace *space;
>> MemoryListener listener;
>> Error *error;
>> bool initialized;
>> bool dirty_pages_supported;
>> uint64_t dirty_pgsizes;
>> uint64_t max_dirty_bitmap_size;
>> unsigned long pgsizes;
>> unsigned int dma_max_mappings;
>> QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
>> QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
>> QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
>> QLIST_ENTRY(VFIOContainer) next;
>> };
>>
>> struct VFIOLegacyContainer {
>> VFIOContainer bcontainer;
>> int fd; /* /dev/vfio/vfio, empowered by the attached groups */
>> MemoryListener prereg_listener;
>> unsigned iommu_type;
>> QLIST_HEAD(, VFIOGroup) group_list;
>> };
>>
>> Co-authored-by: Eric Auger
>> Signed-off-by: Eric Auger
>> Signed-off-by: Yi Liu
>> Signed-off-by: Yi Sun
>> Signed-off-by: Zhenzhong Duan
>> ---
>> hw/vfio/common.c | 72 +---
>> hw/vfio/container-base.c | 160 +
>> hw/vfio/container.c | 247 --
>> hw/vfio/meson.build | 1 +
>> hw/vfio/spapr.c | 22 +--
>> hw/vfio/trace-events | 4 +-
>> include/hw/vfio/vfio-common.h | 85 ++---
>> include/hw/vfio/vfio-container-base.h | 155
>> 8 files changed, 540 insertions(+), 206 deletions(-)
>> create mode 100644 hw/vfio/container-base.c
>> create mode 100644 include/hw/vfio/vfio-container-base.h
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 044710fc1f..86b6af5740 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -379,19 +379,20 @@ static void vfio_iommu_map_notify(IOMMUNotifier
>> *n, IOMMUTLBEntry *iotlb)
>> * of vaddr will always be there, even if the memory object is
>> * destroyed and its backing memory munmap-ed.
>> */
>> - ret = vfio_dma_map(container, iova,
>> - iotlb->addr_mask + 1, vaddr,
>> - read_only);
>> + ret = vfio_container_dma_map(container, iova,
>> + iotlb->addr_mask + 1, vaddr,
>> + read_only);
>> if (ret) {
>> - error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
>> + error_report("vfio_container_dma_map(%p,
>> 0x%"HWADDR_PRIx", "
>> "0x%"HWADDR_PRIx", %p) = %d (%s)",
>> container, iova,
>> iotlb->addr_mask + 1, vaddr, ret,
>> strerror(-ret));
>> }
>> } else {
>> - ret = vfio_dma_unmap(container, iova, iotlb->addr_mask + 1,
>> iotlb);
>> + ret = vfio_container_dma_unmap(container, iova,
>> + iotlb->addr_mask +
Re: [PATCH v1 13/22] vfio: Add base container
Hi Cédric,
On 9/20/23 14:57, Cédric Le Goater wrote:
> On 9/20/23 10:48, Duan, Zhenzhong wrote:
>>
>>
>>> -Original Message-
>>> From: Cédric Le Goater
>>> Sent: Wednesday, September 20, 2023 1:24 AM
>>> Subject: Re: [PATCH v1 13/22] vfio: Add base container
>>>
>>> On 8/30/23 12:37, Zhenzhong Duan wrote:
>>>> From: Yi Liu
>>>>
>>>> Abstract the VFIOContainer to be a base object. It is supposed to be
>>>> embedded by legacy VFIO container and later on, into the new iommufd
>>>> based container.
>>>>
>>>> The base container implements generic code such as code related to
>>>> memory_listener and address space management. The VFIOContainerOps
>>>> implements callbacks that depend on the kernel user space being used.
>>>>
>>>> 'common.c' and vfio device code only manipulates the base container
>>>> with
>>>> wrapper functions that calls the functions defined in
>>>> VFIOContainerOpsClass.
>>>> Existing 'container.c' code is converted to implement the legacy
>>>> container
>>>> ops functions.
>>>>
>>>> Below is the base container. It's named as VFIOContainer, old
>>>> VFIOContainer
>>>> is replaced with VFIOLegacyContainer.
>>>
>>> Usualy, we introduce the new interface solely, port the current models
>>> on top of the new interface, wire the new models in the current
>>> implementation and remove the old implementation. Then, we can start
>>> adding extensions to support other implementations.
>>
>> Not sure if I understand your point correctly. Do you mean to introduce
>> a new type for the base container as below:
>>
>> static const TypeInfo vfio_container_info = {
>> .parent = TYPE_OBJECT,
>> .name = TYPE_VFIO_CONTAINER,
>> .class_size = sizeof(VFIOContainerClass),
>> .instance_size = sizeof(VFIOContainer),
>> .abstract = true,
>> .interfaces = (InterfaceInfo[]) {
>> { TYPE_VFIO_IOMMU_BACKEND_OPS },
>> { }
>> }
>> };
>>
>> and a new interface as below:
>>
>> static const TypeInfo nvram_info = {
>> .name = TYPE_VFIO_IOMMU_BACKEND_OPS,
>> .parent = TYPE_INTERFACE,
>> .class_size = sizeof(VFIOIOMMUBackendOpsClass),
>> };
>>
>> struct VFIOIOMMUBackendOpsClass {
>> InterfaceClass parent;
>> VFIODevice *(*dev_iter_next)(VFIOContainer *container,
>> VFIODevice *curr);
>> int (*dma_map)(VFIOContainer *container,
>> ..
>> };
>>
>> and legacy container on top of TYPE_VFIO_CONTAINER?
>>
>> static const TypeInfo vfio_legacy_container_info = {
>> .parent = TYPE_VFIO_CONTAINER,
>> .name = TYPE_VFIO_LEGACY_CONTAINER,
>> .class_init = vfio_legacy_container_class_init,
>> };
>>
>> This object style is rejected early in RFCv1.
>> See
>> https://lore.kernel.org/kvm/20220414104710.28534-8-yi.l@intel.com/
>
> ouch. this is long ago and I was not aware :/ Bare with me, I will
> probably ask the same questions. Nevertheless, we could improve the
> cover and the flow of changes in the patchset to help the reader.
>
>>> spapr should be taken care of separatly following the principle above.
>>> With my PPC hat, I would not even read such a massive change, too risky
>>> for the subsystem. This path will need (much) further splitting to be
>>> understandable and acceptable.
>>
>> I'll digging into this and try to split it.
>
> I know I am asking for a lot of work. Thanks for that.
>
>> Meanwhile, there are many changes
>> just renaming the parameter or function name for code readability.
>> For example:
>>
>> -int vfio_dma_unmap(VFIOContainer *container, hwaddr iova,
>> - ram_addr_t size, IOMMUTLBEntry *iotlb)
>> +static int vfio_legacy_dma_unmap(VFIOContainer *bcontainer, hwaddr
>> iova,
>> + ram_addr_t size, IOMMUTLBEntry *iotlb)
>>
>> - ret = vfio_get_dirty_bitmap(container, iova, size,
>> + ret = vfio_get_dirty_bitmap(bcontainer, iova, size,
>>
>> Let me know if you think such changes are unnecessary which could reduce
>> this patch largely.
>
> Cleanups, renames, some code reshuffling, anything preparing ground for
> the new abstraction is good to have first and can be merged very quickly
> if there are no functional changes. It reduces the overall patchset and
> ease the coming reviews.
>
> You can send such series independently. That's fine.
>
>>
>>>
>>> Also, please include the .h file first, it helps in reading.
>>
>> Do you mean to put struct declaration earlier in patch description?
>
> Just add to your .gitconfig :
>
> [diff]
> orderFile = /path/to/qemu/scripts/git.orderfile
>
> It should be enough
>
>>> Have you considered using an InterfaceClass ?
>>
>> See above, with object style rejected, it looks hard to use
>> InterfaceClass.
>
> I am not convinced by the QOM approach. I will dig in the past arguments
> and let's see what we come with.
Here is the reference:
https://lore.kernel.org/all/YmuFv2s5TPuw7K%2Fu@yekko/
Eric
>
> Thanks,
>
> C.
>
>
>> Thanks
>> Zhenzhong
>>
>
Re: [PATCH v1 13/22] vfio: Add base container
Hi Cedric,
On 9/19/23 19:23, Cédric Le Goater wrote:
> On 8/30/23 12:37, Zhenzhong Duan wrote:
>> From: Yi Liu
>>
>> Abstract the VFIOContainer to be a base object. It is supposed to be
>> embedded by legacy VFIO container and later on, into the new iommufd
>> based container.
>>
>> The base container implements generic code such as code related to
>> memory_listener and address space management. The VFIOContainerOps
>> implements callbacks that depend on the kernel user space being used.
>>
>> 'common.c' and vfio device code only manipulates the base container with
>> wrapper functions that calls the functions defined in
>> VFIOContainerOpsClass.
>> Existing 'container.c' code is converted to implement the legacy
>> container
>> ops functions.
>>
>> Below is the base container. It's named as VFIOContainer, old
>> VFIOContainer
>> is replaced with VFIOLegacyContainer.
>
> Usualy, we introduce the new interface solely, port the current models
> on top of the new interface, wire the new models in the current
> implementation and remove the old implementation. Then, we can start
> adding extensions to support other implementations.
> spapr should be taken care of separatly following the principle above.
> With my PPC hat, I would not even read such a massive change, too risky
> for the subsystem. This path will need (much) further splitting to be
> understandable and acceptable.
>
> Also, please include the .h file first, it helps in reading. Have you
> considered using an InterfaceClass ?
in the transition from v1 -> v2, I removed the QOMification of the
VFIOContainer, following David Gibson's advice. QOM objects are visible
from the user interface and there was no interest in that. Does it
answer your question?
- remove the QOMification of the VFIOContainer and simply use standard ops
(David)
Unfortunately the coverletter log history has disappeared in this new version.
Zhenzhong, I think it is useful to understand how the series moves on.
Thanks
Eric
>
> Thanks,
>
> C.
>
>>
>> struct VFIOContainer {
>> VFIOIOMMUBackendOpsClass *ops;
>> VFIOAddressSpace *space;
>> MemoryListener listener;
>> Error *error;
>> bool initialized;
>> bool dirty_pages_supported;
>> uint64_t dirty_pgsizes;
>> uint64_t max_dirty_bitmap_size;
>> unsigned long pgsizes;
>> unsigned int dma_max_mappings;
>> QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
>> QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
>> QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
>> QLIST_ENTRY(VFIOContainer) next;
>> };
>>
>> struct VFIOLegacyContainer {
>> VFIOContainer bcontainer;
>> int fd; /* /dev/vfio/vfio, empowered by the attached groups */
>> MemoryListener prereg_listener;
>> unsigned iommu_type;
>> QLIST_HEAD(, VFIOGroup) group_list;
>> };
>>
>> Co-authored-by: Eric Auger
>> Signed-off-by: Eric Auger
>> Signed-off-by: Yi Liu
>> Signed-off-by: Yi Sun
>> Signed-off-by: Zhenzhong Duan
>> ---
>> hw/vfio/common.c | 72 +---
>> hw/vfio/container-base.c | 160 +
>> hw/vfio/container.c | 247 --
>> hw/vfio/meson.build | 1 +
>> hw/vfio/spapr.c | 22 +--
>> hw/vfio/trace-events | 4 +-
>> include/hw/vfio/vfio-common.h | 85 ++---
>> include/hw/vfio/vfio-container-base.h | 155
>> 8 files changed, 540 insertions(+), 206 deletions(-)
>> create mode 100644 hw/vfio/container-base.c
>> create mode 100644 include/hw/vfio/vfio-container-base.h
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 044710fc1f..86b6af5740 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -379,19 +379,20 @@ static void vfio_iommu_map_notify(IOMMUNotifier
>> *n, IOMMUTLBEntry *iotlb)
>> * of vaddr will always be there, even if the memory object is
>> * destroyed and its backing memory munmap-ed.
>> */
>> - ret = vfio_dma_map(container, iova,
>> - iotlb->addr_mask + 1, vaddr,
>> - read_only);
>> + ret = vfio_container_dma_map(container, iova,
>> + iotlb->addr_mask + 1, vaddr,
>> + read_only);
>> if (ret) {
>> - error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
>> + error_report("vfio_container_dma_map(%p,
>> 0x%"HWADDR_PRIx", "
>> "0x%"HWADDR_PRIx", %p) = %d (%s)",
>> container, iova,
>> iotlb->addr_mask + 1, vaddr, ret,
>> strerror(-ret));
>> }
>> } else {
>> - ret = vfio_dma_unmap(container, iova, iotlb->addr_mask + 1,
>> iotlb);
>> + ret = vfio_container_dma_unmap(container, iova,
>> + iotlb->addr_mask + 1, iotlb);
Re: [PATCH v1 13/22] vfio: Add base container
On 9/20/23 10:48, Duan, Zhenzhong wrote:
-Original Message-
From: Cédric Le Goater
Sent: Wednesday, September 20, 2023 1:24 AM
Subject: Re: [PATCH v1 13/22] vfio: Add base container
On 8/30/23 12:37, Zhenzhong Duan wrote:
From: Yi Liu
Abstract the VFIOContainer to be a base object. It is supposed to be
embedded by legacy VFIO container and later on, into the new iommufd
based container.
The base container implements generic code such as code related to
memory_listener and address space management. The VFIOContainerOps
implements callbacks that depend on the kernel user space being used.
'common.c' and vfio device code only manipulates the base container with
wrapper functions that calls the functions defined in VFIOContainerOpsClass.
Existing 'container.c' code is converted to implement the legacy container
ops functions.
Below is the base container. It's named as VFIOContainer, old VFIOContainer
is replaced with VFIOLegacyContainer.
Usualy, we introduce the new interface solely, port the current models
on top of the new interface, wire the new models in the current
implementation and remove the old implementation. Then, we can start
adding extensions to support other implementations.
Not sure if I understand your point correctly. Do you mean to introduce
a new type for the base container as below:
static const TypeInfo vfio_container_info = {
.parent = TYPE_OBJECT,
.name = TYPE_VFIO_CONTAINER,
.class_size = sizeof(VFIOContainerClass),
.instance_size = sizeof(VFIOContainer),
.abstract = true,
.interfaces = (InterfaceInfo[]) {
{ TYPE_VFIO_IOMMU_BACKEND_OPS },
{ }
}
};
and a new interface as below:
static const TypeInfo nvram_info = {
.name = TYPE_VFIO_IOMMU_BACKEND_OPS,
.parent = TYPE_INTERFACE,
.class_size = sizeof(VFIOIOMMUBackendOpsClass),
};
struct VFIOIOMMUBackendOpsClass {
InterfaceClass parent;
VFIODevice *(*dev_iter_next)(VFIOContainer *container, VFIODevice *curr);
int (*dma_map)(VFIOContainer *container,
..
};
and legacy container on top of TYPE_VFIO_CONTAINER?
static const TypeInfo vfio_legacy_container_info = {
.parent = TYPE_VFIO_CONTAINER,
.name = TYPE_VFIO_LEGACY_CONTAINER,
.class_init = vfio_legacy_container_class_init,
};
This object style is rejected early in RFCv1.
See https://lore.kernel.org/kvm/20220414104710.28534-8-yi.l@intel.com/
ouch. this is long ago and I was not aware :/ Bare with me, I will
probably ask the same questions. Nevertheless, we could improve the
cover and the flow of changes in the patchset to help the reader.
spapr should be taken care of separatly following the principle above.
With my PPC hat, I would not even read such a massive change, too risky
for the subsystem. This path will need (much) further splitting to be
understandable and acceptable.
I'll digging into this and try to split it.
I know I am asking for a lot of work. Thanks for that.
Meanwhile, there are many changes
just renaming the parameter or function name for code readability.
For example:
-int vfio_dma_unmap(VFIOContainer *container, hwaddr iova,
- ram_addr_t size, IOMMUTLBEntry *iotlb)
+static int vfio_legacy_dma_unmap(VFIOContainer *bcontainer, hwaddr iova,
+ ram_addr_t size, IOMMUTLBEntry *iotlb)
-ret = vfio_get_dirty_bitmap(container, iova, size,
+ret = vfio_get_dirty_bitmap(bcontainer, iova, size,
Let me know if you think such changes are unnecessary which could reduce
this patch largely.
Cleanups, renames, some code reshuffling, anything preparing ground for
the new abstraction is good to have first and can be merged very quickly
if there are no functional changes. It reduces the overall patchset and
ease the coming reviews.
You can send such series independently. That's fine.
Also, please include the .h file first, it helps in reading.
Do you mean to put struct declaration earlier in patch description?
Just add to your .gitconfig :
[diff]
orderFile = /path/to/qemu/scripts/git.orderfile
It should be enough
Have you considered using an InterfaceClass ?
See above, with object style rejected, it looks hard to use InterfaceClass.
I am not convinced by the QOM approach. I will dig in the past arguments
and let's see what we come with.
Thanks,
C.
Thanks
Zhenzhong
RE: [PATCH v1 13/22] vfio: Add base container
>-Original Message-
>From: Cédric Le Goater
>Sent: Wednesday, September 20, 2023 1:24 AM
>Subject: Re: [PATCH v1 13/22] vfio: Add base container
>
>On 8/30/23 12:37, Zhenzhong Duan wrote:
>> From: Yi Liu
>>
>> Abstract the VFIOContainer to be a base object. It is supposed to be
>> embedded by legacy VFIO container and later on, into the new iommufd
>> based container.
>>
>> The base container implements generic code such as code related to
>> memory_listener and address space management. The VFIOContainerOps
>> implements callbacks that depend on the kernel user space being used.
>>
>> 'common.c' and vfio device code only manipulates the base container with
>> wrapper functions that calls the functions defined in VFIOContainerOpsClass.
>> Existing 'container.c' code is converted to implement the legacy container
>> ops functions.
>>
>> Below is the base container. It's named as VFIOContainer, old VFIOContainer
>> is replaced with VFIOLegacyContainer.
>
>Usualy, we introduce the new interface solely, port the current models
>on top of the new interface, wire the new models in the current
>implementation and remove the old implementation. Then, we can start
>adding extensions to support other implementations.
Not sure if I understand your point correctly. Do you mean to introduce
a new type for the base container as below:
static const TypeInfo vfio_container_info = {
.parent = TYPE_OBJECT,
.name = TYPE_VFIO_CONTAINER,
.class_size = sizeof(VFIOContainerClass),
.instance_size = sizeof(VFIOContainer),
.abstract = true,
.interfaces = (InterfaceInfo[]) {
{ TYPE_VFIO_IOMMU_BACKEND_OPS },
{ }
}
};
and a new interface as below:
static const TypeInfo nvram_info = {
.name = TYPE_VFIO_IOMMU_BACKEND_OPS,
.parent = TYPE_INTERFACE,
.class_size = sizeof(VFIOIOMMUBackendOpsClass),
};
struct VFIOIOMMUBackendOpsClass {
InterfaceClass parent;
VFIODevice *(*dev_iter_next)(VFIOContainer *container, VFIODevice *curr);
int (*dma_map)(VFIOContainer *container,
..
};
and legacy container on top of TYPE_VFIO_CONTAINER?
static const TypeInfo vfio_legacy_container_info = {
.parent = TYPE_VFIO_CONTAINER,
.name = TYPE_VFIO_LEGACY_CONTAINER,
.class_init = vfio_legacy_container_class_init,
};
This object style is rejected early in RFCv1.
See https://lore.kernel.org/kvm/20220414104710.28534-8-yi.l@intel.com/
>
>spapr should be taken care of separatly following the principle above.
>With my PPC hat, I would not even read such a massive change, too risky
>for the subsystem. This path will need (much) further splitting to be
>understandable and acceptable.
I'll digging into this and try to split it. Meanwhile, there are many changes
just renaming the parameter or function name for code readability.
For example:
-int vfio_dma_unmap(VFIOContainer *container, hwaddr iova,
- ram_addr_t size, IOMMUTLBEntry *iotlb)
+static int vfio_legacy_dma_unmap(VFIOContainer *bcontainer, hwaddr iova,
+ ram_addr_t size, IOMMUTLBEntry *iotlb)
-ret = vfio_get_dirty_bitmap(container, iova, size,
+ret = vfio_get_dirty_bitmap(bcontainer, iova, size,
Let me know if you think such changes are unnecessary which could reduce
this patch largely.
>
>Also, please include the .h file first, it helps in reading.
Do you mean to put struct declaration earlier in patch description?
> Have you considered using an InterfaceClass ?
See above, with object style rejected, it looks hard to use InterfaceClass.
Thanks
Zhenzhong
Re: [PATCH v1 13/22] vfio: Add base container
On 8/30/23 12:37, Zhenzhong Duan wrote:
From: Yi Liu
Abstract the VFIOContainer to be a base object. It is supposed to be
embedded by legacy VFIO container and later on, into the new iommufd
based container.
The base container implements generic code such as code related to
memory_listener and address space management. The VFIOContainerOps
implements callbacks that depend on the kernel user space being used.
'common.c' and vfio device code only manipulates the base container with
wrapper functions that calls the functions defined in VFIOContainerOpsClass.
Existing 'container.c' code is converted to implement the legacy container
ops functions.
Below is the base container. It's named as VFIOContainer, old VFIOContainer
is replaced with VFIOLegacyContainer.
Usualy, we introduce the new interface solely, port the current models
on top of the new interface, wire the new models in the current
implementation and remove the old implementation. Then, we can start
adding extensions to support other implementations.
spapr should be taken care of separatly following the principle above.
With my PPC hat, I would not even read such a massive change, too risky
for the subsystem. This path will need (much) further splitting to be
understandable and acceptable.
Also, please include the .h file first, it helps in reading. Have you
considered using an InterfaceClass ?
Thanks,
C.
struct VFIOContainer {
VFIOIOMMUBackendOpsClass *ops;
VFIOAddressSpace *space;
MemoryListener listener;
Error *error;
bool initialized;
bool dirty_pages_supported;
uint64_t dirty_pgsizes;
uint64_t max_dirty_bitmap_size;
unsigned long pgsizes;
unsigned int dma_max_mappings;
QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
QLIST_ENTRY(VFIOContainer) next;
};
struct VFIOLegacyContainer {
VFIOContainer bcontainer;
int fd; /* /dev/vfio/vfio, empowered by the attached groups */
MemoryListener prereg_listener;
unsigned iommu_type;
QLIST_HEAD(, VFIOGroup) group_list;
};
Co-authored-by: Eric Auger
Signed-off-by: Eric Auger
Signed-off-by: Yi Liu
Signed-off-by: Yi Sun
Signed-off-by: Zhenzhong Duan
---
hw/vfio/common.c | 72 +---
hw/vfio/container-base.c | 160 +
hw/vfio/container.c | 247 --
hw/vfio/meson.build | 1 +
hw/vfio/spapr.c | 22 +--
hw/vfio/trace-events | 4 +-
include/hw/vfio/vfio-common.h | 85 ++---
include/hw/vfio/vfio-container-base.h | 155
8 files changed, 540 insertions(+), 206 deletions(-)
create mode 100644 hw/vfio/container-base.c
create mode 100644 include/hw/vfio/vfio-container-base.h
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 044710fc1f..86b6af5740 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -379,19 +379,20 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n,
IOMMUTLBEntry *iotlb)
* of vaddr will always be there, even if the memory object is
* destroyed and its backing memory munmap-ed.
*/
-ret = vfio_dma_map(container, iova,
- iotlb->addr_mask + 1, vaddr,
- read_only);
+ret = vfio_container_dma_map(container, iova,
+ iotlb->addr_mask + 1, vaddr,
+ read_only);
if (ret) {
-error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
+error_report("vfio_container_dma_map(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx", %p) = %d (%s)",
container, iova,
iotlb->addr_mask + 1, vaddr, ret, strerror(-ret));
}
} else {
-ret = vfio_dma_unmap(container, iova, iotlb->addr_mask + 1, iotlb);
+ret = vfio_container_dma_unmap(container, iova,
+ iotlb->addr_mask + 1, iotlb);
if (ret) {
-error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
+error_report("vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx") = %d (%s)",
container, iova,
iotlb->addr_mask + 1, ret, strerror(-ret));
@@ -407,14 +408,15 @@ static void
vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
{
VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
listener);
+VFIOContainer *container = vrdl->container;
const hwaddr size = int128_get64(section->size);
const hwaddr iova = section->offset_within_address_space;
int ret;
/* Unmap with a single call. */
-ret =
[PATCH v1 13/22] vfio: Add base container
From: Yi Liu
Abstract the VFIOContainer to be a base object. It is supposed to be
embedded by legacy VFIO container and later on, into the new iommufd
based container.
The base container implements generic code such as code related to
memory_listener and address space management. The VFIOContainerOps
implements callbacks that depend on the kernel user space being used.
'common.c' and vfio device code only manipulates the base container with
wrapper functions that calls the functions defined in VFIOContainerOpsClass.
Existing 'container.c' code is converted to implement the legacy container
ops functions.
Below is the base container. It's named as VFIOContainer, old VFIOContainer
is replaced with VFIOLegacyContainer.
struct VFIOContainer {
VFIOIOMMUBackendOpsClass *ops;
VFIOAddressSpace *space;
MemoryListener listener;
Error *error;
bool initialized;
bool dirty_pages_supported;
uint64_t dirty_pgsizes;
uint64_t max_dirty_bitmap_size;
unsigned long pgsizes;
unsigned int dma_max_mappings;
QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
QLIST_ENTRY(VFIOContainer) next;
};
struct VFIOLegacyContainer {
VFIOContainer bcontainer;
int fd; /* /dev/vfio/vfio, empowered by the attached groups */
MemoryListener prereg_listener;
unsigned iommu_type;
QLIST_HEAD(, VFIOGroup) group_list;
};
Co-authored-by: Eric Auger
Signed-off-by: Eric Auger
Signed-off-by: Yi Liu
Signed-off-by: Yi Sun
Signed-off-by: Zhenzhong Duan
---
hw/vfio/common.c | 72 +---
hw/vfio/container-base.c | 160 +
hw/vfio/container.c | 247 --
hw/vfio/meson.build | 1 +
hw/vfio/spapr.c | 22 +--
hw/vfio/trace-events | 4 +-
include/hw/vfio/vfio-common.h | 85 ++---
include/hw/vfio/vfio-container-base.h | 155
8 files changed, 540 insertions(+), 206 deletions(-)
create mode 100644 hw/vfio/container-base.c
create mode 100644 include/hw/vfio/vfio-container-base.h
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 044710fc1f..86b6af5740 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -379,19 +379,20 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n,
IOMMUTLBEntry *iotlb)
* of vaddr will always be there, even if the memory object is
* destroyed and its backing memory munmap-ed.
*/
-ret = vfio_dma_map(container, iova,
- iotlb->addr_mask + 1, vaddr,
- read_only);
+ret = vfio_container_dma_map(container, iova,
+ iotlb->addr_mask + 1, vaddr,
+ read_only);
if (ret) {
-error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
+error_report("vfio_container_dma_map(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx", %p) = %d (%s)",
container, iova,
iotlb->addr_mask + 1, vaddr, ret, strerror(-ret));
}
} else {
-ret = vfio_dma_unmap(container, iova, iotlb->addr_mask + 1, iotlb);
+ret = vfio_container_dma_unmap(container, iova,
+ iotlb->addr_mask + 1, iotlb);
if (ret) {
-error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
+error_report("vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx") = %d (%s)",
container, iova,
iotlb->addr_mask + 1, ret, strerror(-ret));
@@ -407,14 +408,15 @@ static void
vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
{
VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
listener);
+VFIOContainer *container = vrdl->container;
const hwaddr size = int128_get64(section->size);
const hwaddr iova = section->offset_within_address_space;
int ret;
/* Unmap with a single call. */
-ret = vfio_dma_unmap(vrdl->container, iova, size , NULL);
+ret = vfio_container_dma_unmap(container, iova, size , NULL);
if (ret) {
-error_report("%s: vfio_dma_unmap() failed: %s", __func__,
+error_report("%s: vfio_container_dma_unmap() failed: %s", __func__,
strerror(-ret));
}
}
@@ -424,6 +426,7 @@ static int
vfio_ram_discard_notify_populate(RamDiscardListener *rdl,
{
VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
listener);
+VFIOContainer *container = vrdl->container;
const hwaddr end = section->offset_within_region +
int128_get64(section->size);
hwaddr start, next,
