|
From: | Cédric Le Goater |
Subject: | Re: [PATCH v4 06/14] vfio/common: Consolidate skip/invalid section into helper |
Date: | Tue, 7 Mar 2023 10:47:28 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 |
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.martins@oracle.com> Reviewed-by: Cédric Le Goater <clg@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. Thanks, C.
But other than that: Reviewed-by: Avihai Horon <avihaih@nvidia.com> Thanks.- return; + return false; } if (unlikely((section->offset_within_address_space & @@ -964,6 +955,24 @@ static void vfio_listener_region_add(MemoryListener *listener, section->offset_within_region, qemu_real_host_page_size()); } + return false; + } + + return true; +} + +static void vfio_listener_region_add(MemoryListener *listener, + 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_valid_section(section)) { return; } @@ -1182,26 +1191,7 @@ static void vfio_listener_region_del(MemoryListener *listener, int ret; bool try_unmap = true; - if (vfio_listener_skipped_section(section)) { - trace_vfio_listener_region_del_skip( - section->offset_within_address_space, - section->offset_within_address_space + - int128_get64(int128_sub(section->size, int128_one()))); - return; - } - - if (unlikely((section->offset_within_address_space & - ~qemu_real_host_page_mask()) != - (section->offset_within_region & ~qemu_real_host_page_mask()))) { - if (!vfio_known_safe_misalignment(section)) { - error_report("%s received unaligned region %s iova=0x%"PRIx64 - " offset_within_region=0x%"PRIx64 - " qemu_real_host_page_size=0x%"PRIxPTR, - __func__, memory_region_name(section->mr), - section->offset_within_address_space, - section->offset_within_region, - qemu_real_host_page_size()); - } + if (!vfio_listener_valid_section(section)) { return; } -- 2.17.2
[Prev in Thread] | Current Thread | [Next in Thread] |