From: "Cédric Le Goater" <clg@redhat.com>
To: Zhenzhong Duan <zhenzhong.duan@intel.com>, qemu-devel@nongnu.org
Cc: alex.williamson@redhat.com, eric.auger@redhat.com,
chao.p.peng@intel.com, Nicholas Piggin <npiggin@gmail.com>,
Daniel Henrique Barboza <danielhb413@gmail.com>,
David Gibson <david@gibson.dropbear.id.au>,
Harsh Prateek Bora <harshpb@linux.ibm.com>,
"open list:sPAPR (pseries)" <qemu-ppc@nongnu.org>
Subject: Re: [PATCH 3/3] vfio: Make VFIOIOMMUClass::add_window() and its wrapper return bool
Date: Mon, 6 May 2024 14:06:39 +0200 [thread overview]
Message-ID: <f470f4cb-285e-43d7-907e-d4e75d776f99@redhat.com> (raw)
In-Reply-To: <20240506083352.4037226-4-zhenzhong.duan@intel.com>
On 5/6/24 10:33, Zhenzhong Duan wrote:
> Make VFIOIOMMUClass::add_window() and its wrapper function
> vfio_container_add_section_window() return bool.
>
> This is to follow the coding standand to return bool if 'Error **'
> is used to pass error.
>
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
One comment below for the spapr maintainers,
> ---
> include/hw/vfio/vfio-container-base.h | 12 ++++++------
> hw/vfio/common.c | 2 +-
> hw/vfio/container-base.c | 8 ++++----
> hw/vfio/spapr.c | 16 ++++++++--------
> 4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index 68539e3bed..e96cda78c8 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -76,9 +76,9 @@ int vfio_container_dma_map(VFIOContainerBase *bcontainer,
> int vfio_container_dma_unmap(VFIOContainerBase *bcontainer,
> hwaddr iova, ram_addr_t size,
> IOMMUTLBEntry *iotlb);
> -int vfio_container_add_section_window(VFIOContainerBase *bcontainer,
> - MemoryRegionSection *section,
> - Error **errp);
> +bool vfio_container_add_section_window(VFIOContainerBase *bcontainer,
> + MemoryRegionSection *section,
> + Error **errp);
> void vfio_container_del_section_window(VFIOContainerBase *bcontainer,
> MemoryRegionSection *section);
> int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
> @@ -131,9 +131,9 @@ struct VFIOIOMMUClass {
> int (*pci_hot_reset)(VFIODevice *vbasedev, bool single);
>
> /* SPAPR specific */
> - int (*add_window)(VFIOContainerBase *bcontainer,
> - MemoryRegionSection *section,
> - Error **errp);
> + bool (*add_window)(VFIOContainerBase *bcontainer,
> + MemoryRegionSection *section,
> + Error **errp);
> void (*del_window)(VFIOContainerBase *bcontainer,
> MemoryRegionSection *section);
> void (*release)(VFIOContainerBase *bcontainer);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 890d30910e..9f1f2e19f7 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -585,7 +585,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
> return;
> }
>
> - if (vfio_container_add_section_window(bcontainer, section, &err)) {
> + if (!vfio_container_add_section_window(bcontainer, section, &err)) {
> goto fail;
> }
>
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index 913ae49077..98d71b3144 100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -31,12 +31,12 @@ int vfio_container_dma_unmap(VFIOContainerBase *bcontainer,
> return bcontainer->ops->dma_unmap(bcontainer, iova, size, iotlb);
> }
>
> -int vfio_container_add_section_window(VFIOContainerBase *bcontainer,
> - MemoryRegionSection *section,
> - Error **errp)
> +bool vfio_container_add_section_window(VFIOContainerBase *bcontainer,
> + MemoryRegionSection *section,
> + Error **errp)
> {
> if (!bcontainer->ops->add_window) {
> - return 0;
> + return true;
> }
>
> return bcontainer->ops->add_window(bcontainer, section, errp);
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 148b257c9c..47b040f1bc 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -323,7 +323,7 @@ static int vfio_spapr_create_window(VFIOContainer *container,
> return 0;
> }
>
> -static int
> +static bool
> vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
> MemoryRegionSection *section,
> Error **errp)
> @@ -351,13 +351,13 @@ vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
> error_setg(errp, "Container %p can't map guest IOVA region"
> " 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx, container,
> iova, end);
> - return -EINVAL;
> + return false;
> }
> - return 0;
> + return true;
> }
>
> if (container->iommu_type != VFIO_SPAPR_TCE_v2_IOMMU) {
> - return 0;
> + return true;
> }
>
> /* For now intersections are not allowed, we may relax this later */
> @@ -373,14 +373,14 @@ vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
> section->offset_within_address_space +
> int128_get64(section->size) - 1,
> hostwin->min_iova, hostwin->max_iova);
> - return -EINVAL;
> + return false;
> }
> }
>
> ret = vfio_spapr_create_window(container, section, &pgsize);
vfio_spapr_create_window() contains several calls to error_report() which
would be good to replace with error_setg().
Thanks,
C.
> if (ret) {
> error_setg_errno(errp, -ret, "Failed to create SPAPR window");
> - return ret;
> + return false;
> }
>
> vfio_host_win_add(scontainer, section->offset_within_address_space,
> @@ -406,14 +406,14 @@ vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
> "vfio: failed GROUP_SET_SPAPR_TCE for "
> "KVM VFIO device %d and group fd %d",
> param.tablefd, param.groupfd);
> - return -errno;
> + return false;
> }
> trace_vfio_spapr_group_attach(param.groupfd, param.tablefd);
> }
> }
> }
> #endif
> - return 0;
> + return true;
> }
>
> static void
prev parent reply other threads:[~2024-05-06 12:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 8:33 [PATCH 0/3] Cleanup VFIOIOMMUClass callback return with bool Zhenzhong Duan
2024-05-06 8:33 ` [PATCH 1/3] vfio: Make VFIOIOMMUClass::attach_device() and its wrapper return bool Zhenzhong Duan
2024-05-06 11:59 ` Cédric Le Goater
2024-05-07 2:09 ` Duan, Zhenzhong
2024-05-07 5:57 ` Cédric Le Goater
2024-05-07 6:09 ` Duan, Zhenzhong
2024-05-06 8:33 ` [PATCH 2/3] vfio: Make VFIOIOMMUClass::setup() " Zhenzhong Duan
2024-05-06 12:02 ` Cédric Le Goater
2024-05-06 8:33 ` [PATCH 3/3] vfio: Make VFIOIOMMUClass::add_window() and its wrapper " Zhenzhong Duan
2024-05-06 12:06 ` Cédric Le Goater [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f470f4cb-285e-43d7-907e-d4e75d776f99@redhat.com \
--to=clg@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=chao.p.peng@intel.com \
--cc=danielhb413@gmail.com \
--cc=david@gibson.dropbear.id.au \
--cc=eric.auger@redhat.com \
--cc=harshpb@linux.ibm.com \
--cc=npiggin@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=zhenzhong.duan@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).