qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Blue Swirl <blauwirbel@gmail.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, dantesu@gmail.com, rth@twiddle.net,
	"Andreas Färber" <afaerber@suse.de>,
	agraf@suse.de
Subject: Re: [Qemu-devel] [PATCH for-1.4] libi2c-omap: Fix endianness dependency
Date: Sat, 2 Feb 2013 20:24:04 +0000	[thread overview]
Message-ID: <CAAu8pHs_tuYD1NpzU=zHeu3-m_Pvqz70RBtmAc6CjzZU3KCaKg@mail.gmail.com> (raw)
In-Reply-To: <CAFEAcA_sfTwsaZVyUsqDN028ngYsi6=nf5gAkBQuv5vj0DEwyg@mail.gmail.com>

On Sat, Feb 2, 2013 at 8:18 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 2 February 2013 18:26, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sat, Feb 2, 2013 at 5:44 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 2 February 2013 17:37, Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 02.02.2013 17:49, schrieb Peter Maydell:
>>>> libqtest.h has no generic endian-aware memread functions unlike Alex,
>>>> you or me expected. It reads a sequence of bytes from guest memory and
>>>> transmits them one-by-one over the text-based qtest protocol.
>>>
>>> OK, so this is just busted for accessing devices. The protocol
>>> has to have some way of letting you do a 32 bit / 16 bit / 8 bit
>>> access (and maybe 64 bit as well while we're here). memread
>>> and memwrite are OK for RAM accesses [ie anything you'd be
>>> happy to have cached or buffered in a real system] but for
>>> memory mapped registers we need to have an equivalent of
>>> inb/inw/inl/outb/outw/outl that guarantee to do exactly one
>>> access of exactly the required width.
>>
>> I was also just making a patch (but not so nice as Andreas'). My
>> analysis was that qtest.c and libqtest.c just pass the result of
>> cpu_physical_memory_rw() as is, with no endian conversion. So the
>> result needs to be converted to host CPU order just like Andreas did.
>> I think the protocol is OK, my initial reaction was to put byte
>> swapping there but that's not right.
>
> No, I think the protocol is broken, even if you ignore the
> question of endianness. Consider that a device's MemoryRegion
> can have different behaviour depending on whether you access it
> as a byte, halfword or word size. cpu_physical_memory_rw() won't
> let you make a word size access to an unaligned address.

But the width is transmitted too, so any size accesses can be performed.

>
> -- PMM

      reply	other threads:[~2013-02-02 20:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-02 16:45 [Qemu-devel] [PATCH for-1.4] libi2c-omap: Fix endianness dependency Andreas Färber
2013-02-02 16:49 ` Peter Maydell
2013-02-02 17:37   ` Andreas Färber
2013-02-02 17:44     ` Peter Maydell
2013-02-02 17:50       ` Andreas Färber
2013-02-02 18:26       ` Blue Swirl
2013-02-02 20:18         ` Peter Maydell
2013-02-02 20:24           ` Blue Swirl [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='CAAu8pHs_tuYD1NpzU=zHeu3-m_Pvqz70RBtmAc6CjzZU3KCaKg@mail.gmail.com' \
    --to=blauwirbel@gmail.com \
    --cc=afaerber@suse.de \
    --cc=agraf@suse.de \
    --cc=dantesu@gmail.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).