On 07/03/2023 10:22, Joao Martins wrote: > On 07/03/2023 09:47, Cédric Le Goater wrote: >> On 3/7/23 10:13, Avihai Horon wrote: >>> On 07/03/2023 4:02, Joao Martins wrote: >>>> External email: Use caution opening links or attachments >>>> >>>> The checks are replicated against region_add and region_del >>>> and will be soon added in another memory listener dedicated >>>> for dirty tracking. >>>> >>>> Move these into a new helper for avoid duplication. >>>> >>>> Signed-off-by: Joao Martins <joao.m.mart...@oracle.com> >>>> Reviewed-by: Cédric Le Goater <c...@redhat.com> >>>> --- >>>> hw/vfio/common.c | 52 +++++++++++++++++++----------------------------- >>>> 1 file changed, 21 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >>>> index 99acb998eb14..54b4a4fc7daf 100644 >>>> --- a/hw/vfio/common.c >>>> +++ b/hw/vfio/common.c >>>> @@ -933,23 +933,14 @@ static bool >>>> vfio_known_safe_misalignment(MemoryRegionSection *section) >>>> return true; >>>> } >>>> >>>> -static void vfio_listener_region_add(MemoryListener *listener, >>>> - MemoryRegionSection *section) >>>> +static bool vfio_listener_valid_section(MemoryRegionSection *section) >>>> { >>>> - VFIOContainer *container = container_of(listener, VFIOContainer, >>>> listener); >>>> - hwaddr iova, end; >>>> - Int128 llend, llsize; >>>> - void *vaddr; >>>> - int ret; >>>> - VFIOHostDMAWindow *hostwin; >>>> - Error *err = NULL; >>>> - >>>> if (vfio_listener_skipped_section(section)) { >>>> trace_vfio_listener_region_add_skip( >>>> section->offset_within_address_space, >>>> section->offset_within_address_space + >>>> int128_get64(int128_sub(section->size, int128_one()))); >>> >>> The original code uses two different traces depending on add or del -- >>> trace_vfio_listener_region_{add,del}_skip. >>> Should we combine the two traces into a single trace? If the distinction is >>> important then maybe pass a flag or the caller name to indicate whether it's >>> add, del or dirty tracking update? >> >> I think introducing a new trace event 'trace_vfio_listener_region_skip' >> to replace 'trace_vfio_listener_region_add_skip' above should be enough. >> > OK, I'll introduce a predecessor patch to change the name. >
Albeit this trace_vfio_listener_region_skip will have a new argument which the caller passes e.g. region_add, region_skip, tracking_update.