[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] memory: do not do out of bound notification
From: |
Auger Eric |
Subject: |
Re: [Qemu-devel] [PATCH] memory: do not do out of bound notification |
Date: |
Wed, 19 Jun 2019 15:17:41 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Yan,
[+ Peter]
On 6/19/19 10:49 AM, Yan Zhao wrote:
> even if an entry overlaps with notifier's range, should not map/unmap
> out of bound part in the entry.
I don't think the patch was based on the master as the trace at the very
end if not part of the upstream code.
>
> This would cause problem in below case:
> 1. initially there are two notifiers with ranges
> 0-0xfedfffff, 0xfef00000-0xffffffffffffffff,
> IOVAs from 0x3c000000 - 0x3c1fffff is in shadow page table.
>
> 2. in vfio, memory_region_register_iommu_notifier() is followed by
> memory_region_iommu_replay(), which will first call address space unmap,
> and walk and add back all entries in vtd shadow page table. e.g.
> (1) for notifier 0-0xfedfffff,
> IOVAs from 0 - 0xffffffff get unmapped,
> and IOVAs from 0x3c000000 - 0x3c1fffff get mapped
While the patch looks sensible, the issue is the notifier scope used in
vtd_address_space_unmap is not a valid mask (ctpop64(size) != 1). Then
the size is recomputed (either using n = 64 - clz64(size) for the 1st
notifier or n = s->aw_bits for the 2d) and also the entry (especially
for the 2d notifier where it becomes 0) to get a proper alignment.
vtd_page_walk sends notifications per block or page (with valid
addr_mask) so stays within the notifier.
Modifying the entry->iova/addr_mask again in memory_region_notify_one
leads to unaligned start address / addr_mask. I don't think we want that.
Can't we modity the vtd_address_space_unmap() implementation to split
the invalidation in smaller chunks instead?
Thanks
Eric
> (2) for notifier 0xfef00000-0xffffffffffffffff
> IOVAs from 0 - 0x7fffffffff get unmapped,> but IOVAs from 0x3c000000
> - 0x3c1fffff cannot get mapped back.
>
> Signed-off-by: Yan Zhao <address@hidden>
> ---
> memory.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/memory.c b/memory.c
> index 07c8315..a6b9da6 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1948,6 +1948,14 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
> return;
> }
>
> + if (entry->iova < notifier->start) {
> + entry->iova = notifier->start;
> + }
> +
> + if (entry->iova + entry->addr_mask > notifier->end) {
> + entry->addr_mask = notifier->end - entry->iova;> + }
> +
> if (entry->perm & IOMMU_RW) {
> printf("map %lx %lx\n", entry->iova, entry->iova + entry->addr_mask);
> request_flags = IOMMU_NOTIFIER_MAP;
>