Re: [Qemu-devel] [PATCH 15/30] memory: add address_space_valid

[David Gibson]

On Fri, May 24, 2013 at 11:52:17AM +0100, Peter Maydell wrote:
> On 24 May 2013 09:02, Paolo Bonzini <address@hidden> wrote:
> > Il 23/05/2013 20:04, Peter Maydell ha scritto:
> >> Shouldn't we be calling the MemoryRegionOps
> >> accepts() callback here? What about access alignment constraints
> >> and access size restrictions?
> >
> > Yes, we should.
> >
> >> What if the validity of the range
> >> changes between the time you asked and when you actually do the
> >> access?
> >
> > If that's a concern, you shouldn't use this API, you should just do the
> > access and rely on the return value of address_space_rw & friends.
> 
> 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.

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.

-- 
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

signature.asc
Description: Digital signature