All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Peter Crosthwaite" <peter.crosthwaite@xilinx.com>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Greg Bellows" <greg.bellows@linaro.org>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	"Andreas Färber" <afaerber@suse.de>,
	"Richard Henderson" <rth@twiddle.net>
Subject: Re: [Qemu-devel] RFC: memory API changes
Date: Tue, 24 Mar 2015 21:00:17 +0100	[thread overview]
Message-ID: <5511C251.8030907@redhat.com> (raw)
In-Reply-To: <CAFEAcA86KdfyzP3Jgj85=njFp8meAkyOf5qPL6miMy0MFnkxgQ@mail.gmail.com>



On 24/03/2015 19:06, Peter Maydell wrote:
> On 24 March 2015 at 17:51, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 24/03/2015 17:35, Peter Maydell wrote:
>>> On 24 March 2015 at 16:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>> On 24 March 2015 at 15:08, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> , for those callers
>>>> of ld/st*_phys that use cs->as as the first argument.
>>>
>>> ...but I don't understand this caveat. I want to add arguments
>>> and rename the functions for *all* callers of ld/st*_phys.
>>> I don't want to specialcase the ones which happen to be
>>> operating on cs->as.
>>
>> The ones that operate on cs->as could become (for some CPUs at least)
>> special-cased accessors like the bus ones; for example building the
>> MemTxAttrs according to internal CPU state.
> 
> Sure, individual targets could do something like this if they
> wanted (compare the arm_ldl_code functions), once these renames
> have gone in.
> 
>> ld/st*_phys actually started as CPU-specific accessors, and most uses
>> are still of that kind, so it makes sense to me that we special-case
>> them.  Maybe it limits churn, maybe it doesn't.  But if it doesn't, it's
>> not like anything is lost.
> 
> I think this is where we disagree. I see ld/st*_phys as being
> really generic -- they take an AddressSpace, after all, and
> part of the same family with address_space_read &c. If you
> don't leave them as generic, then you end up having to use
> the really awkward _read/_write for simple accesses and
> then manage the byteswapping yourself. That's why I want
> to rename them into address_space_* : to indicate that they
> are all part of the same family, and you can use
> address_space_read if you want to read an arbitrary byte
> buffer, or address_space_ldl_be if you want to read a
> big endian 32 bit word, and so on.

I agree with that.  I just want to keep ld/st*_phys _in addition_ as the
short forms of address_space_ld/st*, and keep ld/st*_phys instead of
address_space_ld/st* for those uses that have cs->as as the first argument.

The rationale is to evolve ld/st*_phys into CPU-specific accessors
paralleling the bus-specific accessors.

Paolo

> (The only reason they started out CPU specific is because
> we didn't have any concept at all of having more than
> one address space, so there wasn't any need to say which
> one you meant when you were doing a load.)
> 
> To me it makes much more sense that if a DMA controller
> like pl080 wants to do an LE word read from the AS which
> its bus master is connected to, that it can just do
>  word = ldl_le_phys(my_as, addr, ...);
> 
> I'd expect pretty much any bus master to want to do this
> kind of thing, in fact. It just happens that most of the
> bus masters we have in QEMU right now are CPUs...
> 
> -- PMM
> 
> 

  reply	other threads:[~2015-03-24 20:00 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-23 12:24 [Qemu-devel] RFC: memory API changes Peter Maydell
2015-03-23 12:30 ` Andreas Färber
2015-03-23 12:33   ` Peter Maydell
2015-03-23 14:39 ` Paolo Bonzini
2015-03-23 15:11   ` Peter Maydell
2015-03-23 15:18     ` Paolo Bonzini
2015-03-23 15:26       ` Peter Maydell
2015-03-23 15:27         ` Paolo Bonzini
2015-03-23 15:39           ` Peter Maydell
2015-03-23 15:47             ` Paolo Bonzini
2015-03-23 16:00               ` Peter Maydell
2015-03-23 16:30                 ` Paolo Bonzini
2015-03-23 16:43                   ` Peter Maydell
2015-03-23 16:32     ` Andreas Färber
2015-03-25 10:56       ` Igor Mammedov
2015-03-23 17:51   ` Andreas Färber
2015-03-23 17:59     ` Peter Maydell
2015-03-24 13:47 ` Peter Maydell
2015-03-24 14:45   ` Paolo Bonzini
2015-03-24 14:53     ` Peter Maydell
2015-03-24 15:08       ` Paolo Bonzini
2015-03-24 15:12         ` Peter Maydell
2015-03-24 16:23           ` Paolo Bonzini
2015-03-24 16:35             ` Peter Maydell
2015-03-24 17:51               ` Paolo Bonzini
2015-03-24 18:06                 ` Peter Maydell
2015-03-24 20:00                   ` Paolo Bonzini [this message]
2015-03-24 23:41                     ` Peter Maydell
2015-03-25 11:34                       ` Paolo Bonzini
2015-03-25 11:43                         ` Peter Maydell
2015-03-25 11:50                           ` 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=5511C251.8030907@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=afaerber@suse.de \
    --cc=edgar.iglesias@gmail.com \
    --cc=greg.bellows@linaro.org \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /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.