From: Jason Wang <jasowang@redhat.com>
To: Peter Xu <peterx@redhat.com>, qemu-devel@nongnu.org
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
Eric Auger <eric.auger@redhat.com>,
Alex Williamson <alex.williamson@redhat.com>,
Alexander Witte <alexander.witte@baicanada.com>,
Jintack Lim <jintack@cs.columbia.edu>
Subject: Re: [Qemu-devel] [PATCH] intel-iommu: send PSI always when notify_unmap set
Date: Fri, 20 Apr 2018 12:57:13 +0800 [thread overview]
Message-ID: <d550eb50-c9e3-838a-ac9a-4a998375572b@redhat.com> (raw)
In-Reply-To: <20180418045121.14233-1-peterx@redhat.com>
On 2018年04月18日 12:51, Peter Xu wrote:
> During IOVA page table walk, there is a special case when:
>
> - notify_unmap is set, meanwhile
> - entry is invalid
>
> In the past, we skip the entry always. This is not correct. We should
> send UNMAP notification to registered notifiers in this case. Otherwise
> some stall pages will still be mapped in the host even if L1 guest
You mean 'stale' here?
> unmapped them already.
It looks like some IOTLB invalidation were missed? If not, the page
should be unmaped during invalidation.
>
> Without this patch, nested device assignment to L2 guests might dump
> some errors like:
>
> qemu-system-x86_64: VFIO_MAP_DMA: -17
> qemu-system-x86_64: vfio_dma_map(0x557305420c30, 0xad000, 0x1000,
> 0x7f89a920d000) = -17 (File exists)
>
> To fix this, we need to apply this patch to L1 QEMU (L2 QEMU is not
> affected by this problem).
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>
> To test nested assignment, one also needs to apply below patchset:
> https://lkml.org/lkml/2018/4/18/5
Have a quick glance at it, looks like there's no need for this patch is
we had that kernel patch applied.
Thanks
> ---
> hw/i386/intel_iommu.c | 42 ++++++++++++++++++++++++++++++------------
> 1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index fb31de9416..b359efd6f9 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -722,6 +722,15 @@ static int vtd_iova_to_slpte(VTDContextEntry *ce, uint64_t iova, bool is_write,
>
> typedef int (*vtd_page_walk_hook)(IOMMUTLBEntry *entry, void *private);
>
> +static int vtd_page_walk_one(IOMMUTLBEntry *entry, int level,
> + vtd_page_walk_hook hook_fn, void *private)
> +{
> + assert(hook_fn);
> + trace_vtd_page_walk_one(level, entry->iova, entry->translated_addr,
> + entry->addr_mask, entry->perm);
> + return hook_fn(entry, private);
> +}
> +
> /**
> * vtd_page_walk_level - walk over specific level for IOVA range
> *
> @@ -781,28 +790,37 @@ static int vtd_page_walk_level(dma_addr_t addr, uint64_t start,
> */
> entry_valid = read_cur | write_cur;
>
> + entry.target_as = &address_space_memory;
> + entry.iova = iova & subpage_mask;
> + entry.perm = IOMMU_ACCESS_FLAG(read_cur, write_cur);
> + entry.addr_mask = ~subpage_mask;
> +
> if (vtd_is_last_slpte(slpte, level)) {
> - entry.target_as = &address_space_memory;
> - entry.iova = iova & subpage_mask;
> /* NOTE: this is only meaningful if entry_valid == true */
> entry.translated_addr = vtd_get_slpte_addr(slpte, aw);
> - entry.addr_mask = ~subpage_mask;
> - entry.perm = IOMMU_ACCESS_FLAG(read_cur, write_cur);
> if (!entry_valid && !notify_unmap) {
> trace_vtd_page_walk_skip_perm(iova, iova_next);
> goto next;
> }
> - trace_vtd_page_walk_one(level, entry.iova, entry.translated_addr,
> - entry.addr_mask, entry.perm);
> - if (hook_fn) {
> - ret = hook_fn(&entry, private);
> - if (ret < 0) {
> - return ret;
> - }
> + ret = vtd_page_walk_one(&entry, level, hook_fn, private);
> + if (ret < 0) {
> + return ret;
> }
> } else {
> if (!entry_valid) {
> - trace_vtd_page_walk_skip_perm(iova, iova_next);
> + if (notify_unmap) {
> + /*
> + * The whole entry is invalid; unmap it all.
> + * Translated address is meaningless, zero it.
> + */
> + entry.translated_addr = 0x0;
> + ret = vtd_page_walk_one(&entry, level, hook_fn, private);
> + if (ret < 0) {
> + return ret;
> + }
> + } else {
> + trace_vtd_page_walk_skip_perm(iova, iova_next);
> + }
> goto next;
> }
> ret = vtd_page_walk_level(vtd_get_slpte_addr(slpte, aw), iova,
next prev parent reply other threads:[~2018-04-20 4:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-18 4:51 [Qemu-devel] [PATCH] intel-iommu: send PSI always when notify_unmap set Peter Xu
2018-04-18 5:28 ` Peter Xu
2018-04-18 5:29 ` Liu, Yi L
2018-04-18 6:26 ` Peter Xu
2018-04-20 4:57 ` Jason Wang [this message]
2018-04-20 5:11 ` Peter Xu
2018-04-20 8:29 ` Jason Wang
2018-04-20 8:30 ` Jason Wang
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=d550eb50-c9e3-838a-ac9a-4a998375572b@redhat.com \
--to=jasowang@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=alexander.witte@baicanada.com \
--cc=eric.auger@redhat.com \
--cc=jintack@cs.columbia.edu \
--cc=mst@redhat.com \
--cc=peterx@redhat.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.