qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Peter Crosthwaite" <peter.crosthwaite@xilinx.com>,
	"Patch Tracking" <patches@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Greg Bellows" <greg.bellows@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Richard Henderson" <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 01/14] memory: Define API for MemoryRegionOps to take attrs and return status
Date: Sat, 11 Apr 2015 20:27:10 +1000	[thread overview]
Message-ID: <20150411102710.GS30629@toto> (raw)
In-Reply-To: <CAFEAcA_kjX4NhWzpeboNTGSa2cONw7cSD2E8XfWN7sdHvG+pfw@mail.gmail.com>

On Fri, Apr 10, 2015 at 03:51:07PM +0100, Peter Maydell wrote:
> On 10 April 2015 at 03:07, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Thu, Apr 09, 2015 at 11:21:26AM +0200, Paolo Bonzini wrote:
> >> On 09/04/2015 11:04, Peter Maydell wrote:
> >> > We discussed this last time round, I think. Whether structs get
> >> > passed in registers depends on the host CPU ABI/calling convention.
> >>
> >> Because of C++, structs up to pointer size are in practice always passed
> >> in registers.  64-bit structs may or may not.
> >>
> >> The main advantage of structs is that it's impossible to mismatch the
> >> parameter order.  That even trumps readability in my opinion.
> >>
> >> I'm ambivalent, but I wouldn't mind at all using structs.
> >
> > Thanks for clarifying that Paolo.
> >
> > Yes, the manual bit masking and shifting is easier to get wrong.
> > The struct also has stronger type checking in a way, as you cant OR in literals
> > that are out of bounds. (IIRC GCC will even warn you for free).
> > The struct is also easy to extend if we ever run out of bits in the uint64_t.
> >
> > Peter, would you consider switching to struct or are you still convinced
> > of the pure uint64_t approach?
> 
> OK, having thought about this I'm willing to take the struct-and-bitfields
> approach. My preferences are somewhat based on habit and also on some
> of Linus' rants about bitfields for kernel use [eg
> http://yarchive.net/comp/linux/bitfields.html], but I think we are
> not going to hit any of the problem cases (notably, we don't care
> about the arrangement of the bitfields within the word, we aren't
> trying to have bitfields and locks/volatile/atomic info shared in
> one struct, and we don't have a particular need to test multiple
> bits at once).
> 
> I'll change over to structs for v2.

Awesome, thanks!

  reply	other threads:[~2015-04-11 10:27 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-07 20:09 [Qemu-devel] [PATCH 00/14] Add memory attributes and use them in ARM Peter Maydell
2015-04-07 20:09 ` [Qemu-devel] [PATCH 01/14] memory: Define API for MemoryRegionOps to take attrs and return status Peter Maydell
2015-04-08 10:49   ` Paolo Bonzini
2015-04-09  8:55   ` Edgar E. Iglesias
2015-04-09  9:04     ` Peter Maydell
2015-04-09  9:21       ` Paolo Bonzini
2015-04-10  2:07         ` Edgar E. Iglesias
2015-04-10 14:51           ` Peter Maydell
2015-04-11 10:27             ` Edgar E. Iglesias [this message]
2015-04-09  9:32       ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 02/14] memory: Add MemTxAttrs, MemTxResult to io_mem_read and io_mem_write Peter Maydell
2015-04-08 10:51   ` Paolo Bonzini
2015-04-08 10:59     ` Peter Maydell
2015-04-08 11:13       ` Paolo Bonzini
2015-04-09  8:59   ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 03/14] Make CPU iotlb a structure rather than a plain hwaddr Peter Maydell
2015-04-08 10:52   ` Paolo Bonzini
2015-04-09  9:02   ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 04/14] Add MemTxAttrs to the IOTLB Peter Maydell
2015-04-08 10:53   ` Paolo Bonzini
2015-04-09  9:04   ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 05/14] exec.c: Convert subpage memory ops to _with_attrs Peter Maydell
2015-04-08 10:54   ` Paolo Bonzini
2015-04-09  9:07   ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 06/14] exec.c: Make address_space_rw take transaction attributes Peter Maydell
2015-04-08 12:55   ` Paolo Bonzini
2015-04-09  9:59   ` Edgar E. Iglesias
2015-04-09 10:14     ` Peter Maydell
2015-04-09 10:21       ` Paolo Bonzini
2015-04-09 10:43         ` Peter Maydell
2015-04-09 11:40           ` Paolo Bonzini
2015-04-09 11:43             ` Peter Maydell
2015-04-07 20:09 ` [Qemu-devel] [PATCH 07/14] exec.c: Add new address_space_ld*/st* functions Peter Maydell
2015-04-08 11:03   ` Paolo Bonzini
2015-04-09 11:49     ` Peter Maydell
2015-04-09 12:00       ` Paolo Bonzini
2015-04-09 12:38         ` Peter Maydell
2015-04-09 12:42           ` Paolo Bonzini
2015-04-09 10:34   ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 08/14] Switch non-CPU callers from ld/st*_phys to address_space_ld/st* Peter Maydell
2015-04-09 10:44   ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 09/14] exec.c: Capture the memory attributes for a watchpoint hit Peter Maydell
2015-04-08 11:04   ` Paolo Bonzini
2015-04-08 11:14     ` Peter Maydell
2015-04-07 20:09 ` [Qemu-devel] [PATCH 10/14] target-arm: Honour NS bits in page tables Peter Maydell
2015-04-09 11:23   ` Edgar E. Iglesias
2015-04-09 14:14     ` Peter Maydell
2015-04-09 14:23       ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 11/14] target-arm: Use correct memory attributes for page table walks Peter Maydell
2015-04-09 11:34   ` Edgar E. Iglesias
2015-04-07 20:09 ` [Qemu-devel] [PATCH 12/14] target-arm: Add user-mode transaction attribute Peter Maydell
2015-04-07 20:09 ` [Qemu-devel] [PATCH 13/14] target-arm: Use attribute info to handle user-only watchpoints Peter Maydell
2015-04-09 11:37   ` Edgar E. Iglesias
2015-04-07 20:10 ` [Qemu-devel] [PATCH 14/14] target-arm: Check watchpoints against CPU security state Peter Maydell
2015-04-09 11:38   ` Edgar E. Iglesias
2015-04-09  9:37 ` [Qemu-devel] [PATCH 00/14] Add memory attributes and use them in ARM Edgar E. Iglesias

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=20150411102710.GS30629@toto \
    --to=edgar.iglesias@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=greg.bellows@linaro.org \
    --cc=patches@linaro.org \
    --cc=pbonzini@redhat.com \
    --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 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).