From: David Gibson <david@gibson.dropbear.id.au>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
qemu-devel@nongnu.org, jan.kiszka@gmail.com
Subject: Re: [Qemu-devel] [PATCH 15/30] memory: add address_space_valid
Date: Sun, 26 May 2013 23:02:28 +1000 [thread overview]
Message-ID: <20130526130228.GA4765@boomeroo.fritz.box> (raw)
In-Reply-To: <CAFEAcA-=tmDNEGoLT807fXX3qU+Dqg2mb4WYkDNuwC3-0dj8XA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3130 bytes --]
On Sat, May 25, 2013 at 10:23:32AM +0100, Peter Maydell wrote:
> On 25 May 2013 04:44, David Gibson <david@gibson.dropbear.id.au> wrote:
> > On Fri, May 24, 2013 at 11:52:17AM +0100, Peter Maydell wrote:
> >> So when *is* it a good idea to use this API? In real
> >> hardware you don't usually get a "tell me whether this
> >> access would succeed if I did it" bus operation -- you
> >> just do the operation and the memory transaction either
> >> succeeds or it doesn't. Are we modelling something that
> >> really exists in hardware on spapr here?
> >
> > So, as a general rule, you should just attempt the access and handle
> > failures - this is a bad interface. The reason I added it, however,
> > is that the PAPR specification mandates that the virtual LAN pre-check
> > various buffers when they're registered, and return specific errors if
> > they're not mapped to valid memory. Since we have nothing to read or
> > write at that point, adding this interface was the only way I could
> > see to implement that requirement.
>
> Would it work to just read and throw away the result of the read?
Not when checking for writability.
> > Or... a bit more charitably: You should always handle failures at the
> > point of read or write, but using this interface can give you an
> > earlier, and therefore potentially easier to analyze, error in the
> > more common failure cases, even if there are more complex cases where
> > the pre-check succeeds but the read/write still fails later.
>
> There's also the converse case where the pre-check fails but
> doing the operation at the proper time would succeed, in
> which case where we're modelling real hardware we would
> be doing it wrong. So the set of cases where it's OK to
> pre-check seems a bit limited.
Whether originally real or otherwise, this is a question of faithfully
implementing what the hardware is supposed to do. In the case of the
PAPR llan, the (virtual) hardware specification says that the buffer
accessibility is checked at buffer add time, and the driver won't work
if it only makes the buffers accessible between that point and the
actual buffer access. It would be entirely possible to design real
hardware with similar behaviour (probe buffer accessibility when a
descriptor is added to a pool, say), although it's not the sort of
thing hardware people generally do.
So, certainly qemu should not go using this pre-check when the
hardware it's emulating does not do such a check. Designing hardware
that does do a pre-check is not, IMO, a good idea on balance, but the
point is that there is a reason (albeit, not a great one) you might
want to design the (possibly virtual) hardware that way.
As an aside, when you consider devices with embedded firmware - which
is practically everything these days - the distinction between "real"
hardware and virtual hardware can get kind of blurry.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
next prev parent reply other threads:[~2013-05-26 13:02 UTC|newest]
Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-21 10:57 [Qemu-devel] [PATCH 00/30] Introduction of IOMMUs into the memory API Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 01/30] exec: remove obsolete comment Paolo Bonzini
2013-05-21 11:36 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 02/30] exec: eliminate qemu_put_ram_ptr Paolo Bonzini
2013-05-21 11:38 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 03/30] exec: make qemu_get_ram_ptr private Paolo Bonzini
2013-05-21 11:38 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 04/30] exec: eliminate stq_phys_notdirty Paolo Bonzini
2013-05-23 17:32 ` Peter Maydell
2013-05-23 19:18 ` Anthony Liguori
2013-05-23 19:22 ` Paolo Bonzini
2013-05-23 23:14 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 05/30] memory: assert that PhysPageEntry's ptr does not overflow Paolo Bonzini
2013-05-23 17:36 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 06/30] memory: allow memory_region_find() to run on non-root memory regions Paolo Bonzini
2013-05-23 17:52 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 07/30] memory: Replace open-coded memory_region_is_romd Paolo Bonzini
2013-05-21 11:54 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 08/30] memory: Rename readable flag to romd_mode Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 09/30] memory: do not duplicate memory_region_destructor_none Paolo Bonzini
2013-05-21 11:55 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 10/30] memory: make memory_global_sync_dirty_bitmap take an AddressSpace Paolo Bonzini
2013-05-21 11:56 ` Peter Maydell
2013-05-23 1:05 ` David Gibson
2013-05-21 10:57 ` [Qemu-devel] [PATCH 11/30] memory: fix address space initialization/destruction Paolo Bonzini
2013-05-21 11:58 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 12/30] s390x: reduce TARGET_PHYS_ADDR_SPACE_BITS to 62 Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 13/30] memory: limit sections in the radix tree to the actual address space size Paolo Bonzini
2013-05-21 12:02 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 14/30] memory: create FlatView for new address spaces Paolo Bonzini
2013-05-21 12:03 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 15/30] memory: add address_space_valid Paolo Bonzini
2013-05-23 12:05 ` David Gibson
2013-05-23 14:22 ` Jan Kiszka
2013-05-23 14:43 ` Paolo Bonzini
2013-05-23 18:04 ` Peter Maydell
2013-05-24 6:13 ` Jan Kiszka
2013-05-24 10:28 ` Jan Kiszka
2013-05-24 10:50 ` Peter Maydell
2013-05-24 11:02 ` Jan Kiszka
2013-05-24 8:02 ` Paolo Bonzini
2013-05-24 10:52 ` Peter Maydell
2013-05-24 12:58 ` Paolo Bonzini
2013-05-24 13:27 ` Peter Maydell
2013-05-24 13:33 ` Paolo Bonzini
2013-05-24 13:38 ` Peter Maydell
2013-05-25 3:44 ` David Gibson
2013-05-25 9:23 ` Peter Maydell
2013-05-26 13:02 ` David Gibson [this message]
2013-05-21 10:57 ` [Qemu-devel] [PATCH 16/30] memory: clean up phys_page_find Paolo Bonzini
2013-05-23 18:06 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 17/30] memory: add address_space_translate Paolo Bonzini
2013-05-23 7:09 ` liu ping fan
2013-05-23 9:59 ` Paolo Bonzini
2013-05-23 13:06 ` liu ping fan
2013-05-23 13:17 ` Paolo Bonzini
2013-05-23 18:15 ` Peter Maydell
2013-05-25 6:40 ` Jan Kiszka
2013-05-25 7:47 ` Paolo Bonzini
2013-05-25 10:19 ` Jan Kiszka
2013-05-25 11:20 ` Paolo Bonzini
2013-05-25 11:30 ` Jan Kiszka
2013-05-26 8:56 ` Paolo Bonzini
2013-05-26 9:02 ` Jan Kiszka
2013-05-27 7:20 ` Paolo Bonzini
2013-05-27 7:23 ` Jan Kiszka
2013-05-27 8:19 ` Paolo Bonzini
2013-05-27 8:37 ` Jan Kiszka
2013-05-27 10:33 ` Peter Maydell
2013-05-27 10:45 ` Paolo Bonzini
2013-05-27 11:33 ` Peter Maydell
2013-05-26 9:01 ` Paolo Bonzini
2013-05-26 9:12 ` Jan Kiszka
2013-05-26 18:23 ` Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 18/30] memory: add return value to address_space_rw/read/write Paolo Bonzini
2013-05-23 18:18 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 19/30] memory: Introduce address_space_lookup_region Paolo Bonzini
2013-05-23 18:19 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 20/30] memory: iommu support Paolo Bonzini
2013-05-23 18:24 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 21/30] memory: Add iommu map/unmap notifiers Paolo Bonzini
2013-05-23 18:27 ` Peter Maydell
2013-05-23 19:24 ` Paolo Bonzini
2013-05-29 4:08 ` David Gibson
2013-05-21 10:57 ` [Qemu-devel] [PATCH 22/30] vfio: abort if an emulated iommu is used Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 23/30] spapr: convert TCE API to use an opaque type Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 24/30] spapr: make IOMMU translation go through IOMMUTLBEntry Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 25/30] spapr: use memory core for iommu support Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 26/30] dma: eliminate old-style IOMMU support Paolo Bonzini
2013-05-23 18:31 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 27/30] pci: use memory core for iommu support Paolo Bonzini
2013-05-23 18:36 ` Peter Maydell
2013-05-23 21:22 ` Michael S. Tsirkin
2013-05-21 10:57 ` [Qemu-devel] [PATCH 28/30] spapr_vio: take care of creating our own AddressSpace/DMAContext Paolo Bonzini
2013-05-21 10:57 ` [Qemu-devel] [PATCH 29/30] dma: eliminate DMAContext Paolo Bonzini
2013-05-23 18:40 ` Peter Maydell
2013-05-21 10:57 ` [Qemu-devel] [PATCH 30/30] memory: give name to every AddressSpace Paolo Bonzini
2013-05-23 18:46 ` Peter Maydell
2013-05-22 2:30 ` [Qemu-devel] [PATCH 00/30] Introduction of IOMMUs into the memory API Alexey Kardashevskiy
2013-05-22 9:24 ` Paolo Bonzini
2013-05-23 17:08 ` Paolo Bonzini
2013-05-23 17:25 ` Peter Maydell
2013-05-23 17:30 ` Paolo Bonzini
2013-05-23 18:47 ` Peter Maydell
2013-05-24 14:12 ` Alexey Kardashevskiy
2013-05-24 14:20 ` Paolo Bonzini
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=20130526130228.GA4765@boomeroo.fritz.box \
--to=david@gibson.dropbear.id.au \
--cc=jan.kiszka@gmail.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@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 NNTP newsgroup(s).