From: "Cédric Le Goater" <clg@redhat.com>
To: Avihai Horon <avihaih@nvidia.com>, qemu-devel@nongnu.org
Cc: "Peter Xu" <peterx@redhat.com>, "Fabiano Rosas" <farosas@suse.de>,
"Alex Williamson" <alex.williamson@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Markus Armbruster" <armbru@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"David Hildenbrand" <david@redhat.com>
Subject: Re: [PATCH v5 07/10] memory: Add Error** argument to memory_get_xlat_addr()
Date: Tue, 14 May 2024 10:57:11 +0200 [thread overview]
Message-ID: <57318f0e-2446-4843-9c99-cc6e74916165@redhat.com> (raw)
In-Reply-To: <608bf953-3a75-4edf-8b82-3fa53f7f2dbb@nvidia.com>
On 5/13/24 15:44, Avihai Horon wrote:
>
> On 06/05/2024 12:20, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Let the callers do the reporting. This will be useful in
>> vfio_iommu_map_dirty_notify().
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Reviewed-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>> include/exec/memory.h | 15 ++++++++++++++-
>> hw/vfio/common.c | 13 +++++++++----
>> hw/virtio/vhost-vdpa.c | 5 ++++-
>> system/memory.c | 10 +++++-----
>> 4 files changed, 32 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index dadb5cd65ab58b4868fcae06b4e301f0ecb0c1d2..2c45051b7b419c48b4e14c25f4d16a99ccd23996 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -774,9 +774,22 @@ void ram_discard_manager_register_listener(RamDiscardManager *rdm,
>> void ram_discard_manager_unregister_listener(RamDiscardManager *rdm,
>> RamDiscardListener *rdl);
>>
>> +/**
>> + * memory_get_xlat_addr: Extract addresses from a TLB entry
>> + *
>> + * @iotlb: pointer to an #IOMMUTLBEntry
>> + * @vaddr: virtual addressf
>
> Nit: s/addressf/address
Fixed.
Thanks,
C.
>
> Thanks.
>
>> + * @ram_addr: RAM address
>> + * @read_only: indicates if writes are allowed
>> + * @mr_has_discard_manager: indicates memory is controlled by a
>> + * RamDiscardManager
>> + * @errp: pointer to Error*, to store an error if it happens.
>> + *
>> + * Return: true on success, else false setting @errp with error.
>> + */
>> bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>> ram_addr_t *ram_addr, bool *read_only,
>> - bool *mr_has_discard_manager);
>> + bool *mr_has_discard_manager, Error **errp);
>>
>> typedef struct CoalescedMemoryRange CoalescedMemoryRange;
>> typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index b929bb0b7ac60dcef34c0d5a098d5d91f75501dd..da748563eb33843e93631a5240759964f33162f2 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -253,12 +253,13 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>>
>> /* Called with rcu_read_lock held. */
>> static bool vfio_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>> - ram_addr_t *ram_addr, bool *read_only)
>> + ram_addr_t *ram_addr, bool *read_only,
>> + Error **errp)
>> {
>> bool ret, mr_has_discard_manager;
>>
>> ret = memory_get_xlat_addr(iotlb, vaddr, ram_addr, read_only,
>> - &mr_has_discard_manager);
>> + &mr_has_discard_manager, errp);
>> if (ret && mr_has_discard_manager) {
>> /*
>> * Malicious VMs might trigger discarding of IOMMU-mapped memory. The
>> @@ -288,6 +289,7 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>> hwaddr iova = iotlb->iova + giommu->iommu_offset;
>> void *vaddr;
>> int ret;
>> + Error *local_err = NULL;
>>
>> trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
>> iova, iova + iotlb->addr_mask);
>> @@ -304,7 +306,8 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>> if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
>> bool read_only;
>>
>> - if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only)) {
>> + if (!vfio_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, &local_err)) {
>> + error_report_err(local_err);
>> goto out;
>> }
>> /*
>> @@ -1213,6 +1216,7 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>> VFIOContainerBase *bcontainer = giommu->bcontainer;
>> hwaddr iova = iotlb->iova + giommu->iommu_offset;
>> ram_addr_t translated_addr;
>> + Error *local_err = NULL;
>> int ret = -EINVAL;
>>
>> trace_vfio_iommu_map_dirty_notify(iova, iova + iotlb->addr_mask);
>> @@ -1224,7 +1228,8 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>> }
>>
>> rcu_read_lock();
>> - if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL)) {
>> + if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL, &local_err)) {
>> + error_report_err(local_err);
>> goto out_lock;
>> }
>>
>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> index e827b9175fc61f1ef419e48d90a440b00449312a..ed99ab87457d8f31b98ace960713f48d47b27102 100644
>> --- a/hw/virtio/vhost-vdpa.c
>> +++ b/hw/virtio/vhost-vdpa.c
>> @@ -208,6 +208,7 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>> void *vaddr;
>> int ret;
>> Int128 llend;
>> + Error *local_err = NULL;
>>
>> if (iotlb->target_as != &address_space_memory) {
>> error_report("Wrong target AS \"%s\", only system memory is allowed",
>> @@ -227,7 +228,9 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>> if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
>> bool read_only;
>>
>> - if (!memory_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, NULL)) {
>> + if (!memory_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, NULL,
>> + &local_err)) {
>> + error_report_err(local_err);
>> return;
>> }
>> ret = vhost_vdpa_dma_map(s, VHOST_VDPA_GUEST_PA_ASID, iova,
>> diff --git a/system/memory.c b/system/memory.c
>> index 49f1cb2c3835f78f5f948531cbbf13adc706f1ad..e2b9f0aa9ea7326a5bedde28b8c777cf86bc253e 100644
>> --- a/system/memory.c
>> +++ b/system/memory.c
>> @@ -2179,7 +2179,7 @@ void ram_discard_manager_unregister_listener(RamDiscardManager *rdm,
>> /* Called with rcu_read_lock held. */
>> bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>> ram_addr_t *ram_addr, bool *read_only,
>> - bool *mr_has_discard_manager)
>> + bool *mr_has_discard_manager, Error **errp)
>> {
>> MemoryRegion *mr;
>> hwaddr xlat;
>> @@ -2197,7 +2197,7 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>> mr = address_space_translate(&address_space_memory, iotlb->translated_addr,
>> &xlat, &len, writable, MEMTXATTRS_UNSPECIFIED);
>> if (!memory_region_is_ram(mr)) {
>> - error_report("iommu map to non memory area %" HWADDR_PRIx "", xlat);
>> + error_setg(errp, "iommu map to non memory area %" HWADDR_PRIx "", xlat);
>> return false;
>> } else if (memory_region_has_ram_discard_manager(mr)) {
>> RamDiscardManager *rdm = memory_region_get_ram_discard_manager(mr);
>> @@ -2216,8 +2216,8 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>> * were already restored before IOMMUs are restored.
>> */
>> if (!ram_discard_manager_is_populated(rdm, &tmp)) {
>> - error_report("iommu map to discarded memory (e.g., unplugged via"
>> - " virtio-mem): %" HWADDR_PRIx "",
>> + error_setg(errp, "iommu map to discarded memory (e.g., unplugged"
>> + " via virtio-mem): %" HWADDR_PRIx "",
>> iotlb->translated_addr);
>> return false;
>> }
>> @@ -2228,7 +2228,7 @@ bool memory_get_xlat_addr(IOMMUTLBEntry *iotlb, void **vaddr,
>> * check that it did not truncate too much.
>> */
>> if (len & iotlb->addr_mask) {
>> - error_report("iommu has granularity incompatible with target AS");
>> + error_setg(errp, "iommu has granularity incompatible with target AS");
>> return false;
>> }
>>
>> --
>> 2.45.0
>>
>
next prev parent reply other threads:[~2024-05-14 8:58 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 9:20 [PATCH v5 00/10] vfio: Improve error reporting (part 2) Cédric Le Goater
2024-05-06 9:20 ` [PATCH v5 01/10] vfio: Add Error** argument to .set_dirty_page_tracking() handler Cédric Le Goater
2024-05-13 13:03 ` Avihai Horon
2024-05-13 16:27 ` Cédric Le Goater
2024-05-15 12:17 ` Avihai Horon
2024-05-15 12:25 ` Cédric Le Goater
2024-05-15 12:29 ` Avihai Horon
2024-05-15 12:36 ` Cédric Le Goater
2024-05-15 12:49 ` Avihai Horon
2024-05-06 9:20 ` [PATCH v5 02/10] vfio: Add Error** argument to vfio_devices_dma_logging_start() Cédric Le Goater
2024-05-13 13:08 ` Avihai Horon
2024-05-13 16:49 ` Cédric Le Goater
2024-05-06 9:20 ` [PATCH v5 03/10] vfio: Extend migration_file_set_error() with Error** argument Cédric Le Goater
2024-05-13 13:14 ` Avihai Horon
2024-05-13 16:51 ` Cédric Le Goater
2024-05-13 22:19 ` Fabiano Rosas
2024-05-06 9:20 ` [PATCH v5 04/10] vfio: Use new Error** argument in vfio_save_setup() Cédric Le Goater
2024-05-13 13:21 ` Avihai Horon
2024-05-14 7:57 ` Cédric Le Goater
2024-05-06 9:20 ` [PATCH v5 05/10] vfio: Add Error** argument to .vfio_save_config() handler Cédric Le Goater
2024-05-13 13:30 ` Avihai Horon
2024-05-14 8:50 ` Cédric Le Goater
2024-05-06 9:20 ` [PATCH v5 06/10] vfio: Reverse test on vfio_get_dirty_bitmap() Cédric Le Goater
2024-05-13 13:42 ` Avihai Horon
2024-05-14 8:55 ` Cédric Le Goater
2024-05-06 9:20 ` [PATCH v5 07/10] memory: Add Error** argument to memory_get_xlat_addr() Cédric Le Goater
2024-05-13 13:44 ` Avihai Horon
2024-05-14 8:57 ` Cédric Le Goater [this message]
2024-05-06 9:20 ` [PATCH v5 08/10] vfio: Add Error** argument to .get_dirty_bitmap() handler Cédric Le Goater
2024-05-13 13:51 ` Avihai Horon
2024-05-14 9:05 ` Cédric Le Goater
2024-05-06 9:20 ` [PATCH v5 09/10] vfio: Also trace event failures in vfio_save_complete_precopy() Cédric Le Goater
2024-05-13 13:54 ` Avihai Horon
2024-05-06 9:20 ` [PATCH v5 10/10] vfio: Extend vfio_set_migration_error() with Error* argument Cédric Le Goater
2024-05-13 14:26 ` Avihai Horon
2024-05-14 11:20 ` Cédric Le Goater
2024-05-14 14:35 ` Cédric Le Goater
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=57318f0e-2446-4843-9c99-cc6e74916165@redhat.com \
--to=clg@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=armbru@redhat.com \
--cc=avihaih@nvidia.com \
--cc=david@redhat.com \
--cc=farosas@suse.de \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
/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).