qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: "Eric Auger" <eric.auger@redhat.com>,
	qemu-devel@nongnu.org, "Michael S . Tsirkin" <mst@redhat.com>,
	"Eugenio Pérez" <eperezma@redhat.com>
Subject: Re: [PATCH] x86-iommu: Fail flag registration of DEVIOTLB if DT not supported
Date: Thu, 20 Oct 2022 12:45:29 -0400	[thread overview]
Message-ID: <Y1F7Iujybjr3kVCF@x1n> (raw)
In-Reply-To: <CACGkMEuVOyW6e-U_79UruLotx2AygbjKxeAE16JZaE1uAdSwuw@mail.gmail.com>

On Thu, Oct 20, 2022 at 11:58:34AM +0800, Jason Wang wrote:
> Haven't tried but I guess there would be some issue other than the
> suggested configuration "ats=on, device-iotlb=on"
> 
> So we have:
> 
> 1) ats=on, device-iotlb=on, this is the configuration that libvirt is
> using and it should work
> 2) ats=off, device-iotlb=on, in this case, the DEVICEIOTLB_UNMAP
> notifier will succeed but there won't be a device iotlb invalidation
> sent from guest, so we will meet errors since there's no way to flush
> device IOTLB. According to the PCIe spec, the device should still work
> (using untranslated transactions). In this case we probably need a way
> to detect if device page fault (ats) is enabled and fallback to UNMAP
> if it doesn't.

Yeah, agreed that we should not register to dev-iotlb notifier if ats is
off in the first place.  Maybe worth another patch.

> 3) ats=on, device-iotlb=off, in this case, without your patch, it
> won't work since the DEVICEIOTLB_UNMAP will succeed but guest won't
> enable ATS so there will be no IOTLB invalidation. With your patch, we
> fallback to UNMAP and I think it should then work
> 4) ats=off, device-iotlb=off, similar to 3), it won't work without
> your patch, but with your patch we fallback to UNMAP so it should
> work.

I think the current patch should still be correct, but maybe something else
is missing.  I'll add this one into the todo list but I'd be more than glad
if anyone can look into this too before that..  Or as Eric suggested to
detect such a mismatch before it's fixed, but then if we know they're not
working we should fail hard rather than a warning anymore.

Thanks,

-- 
Peter Xu



      reply	other threads:[~2022-10-20 16:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-18 21:54 [PATCH] x86-iommu: Fail flag registration of DEVIOTLB if DT not supported Peter Xu
2022-10-19  5:45 ` Jason Wang
2022-10-19 11:24 ` Eric Auger
2022-10-19 14:01   ` Peter Xu
2022-10-19 14:12     ` Eric Auger
2022-10-19 14:54       ` Peter Xu
2022-10-20  3:58         ` Jason Wang
2022-10-20 16:45           ` Peter Xu [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=Y1F7Iujybjr3kVCF@x1n \
    --to=peterx@redhat.com \
    --cc=eperezma@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@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 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 NNTP newsgroup(s).