All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: CJ Chen <cjchen@igel.co.jp>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	qemu-riscv@nongnu.org, qemu-arm@nongnu.org,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Keith Busch" <kbusch@kernel.org>,
	"Klaus Jensen" <its@irrelevant.dk>,
	"Jesper Devantier" <foss@defmacro.it>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Weiwei Li" <liwei1518@gmail.com>,
	"Daniel Henrique Barboza" <dbarboza@ventanamicro.com>,
	"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
	"Tyrone Ting" <kfting@nuvoton.com>,
	"Hao Wu" <wuhaotsh@google.com>,
	"Max Filippov" <jcmvbkbc@gmail.com>,
	"David Hildenbrand" <david@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Fabiano Rosas" <farosas@suse.de>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Tomoyuki Hirose" <hrstmyk811m@gmail.com>,
	"Peter Maydell" <peter.maydell@linaro.org>
Subject: Re: [RFC PATCH v2 1/9] doc/devel/memory.rst: additional explanation for unaligned access
Date: Tue, 2 Sep 2025 12:09:33 -0400	[thread overview]
Message-ID: <aLcWvR5Snm1RXEcY@x1.local> (raw)
In-Reply-To: <20250822092410.25833-2-cjchen@igel.co.jp>

On Fri, Aug 22, 2025 at 06:24:02PM +0900, CJ Chen wrote:
> Add documentation to clarify that if `.valid.unaligned = true` but
> `.impl.unaligned = false`, QEMU’s memory core will automatically split
> unaligned guest accesses into multiple aligned accesses. This helps
> devices avoid implementing their own unaligned logic, but can be
> problematic for devices with side-effect-heavy registers. Also note
> that setting `.valid.unaligned = false` together with
> `.impl.unaligned = true` is invalid, as it contradicts itself and
> will trigger an assertion.
> 
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/devel/memory.rst | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
> index 57fb2aec76..71d7de7ae5 100644
> --- a/docs/devel/memory.rst
> +++ b/docs/devel/memory.rst
> @@ -365,6 +365,24 @@ callbacks are called:
>  - .impl.unaligned specifies that the *implementation* supports unaligned
>    accesses; if false, unaligned accesses will be emulated by two aligned
>    accesses.
> +- Additionally, if .valid.unaligned = true but .impl.unaligned = false, the
> +  memory core will emulate each unaligned guest access by splitting it into
> +  multiple aligned sub-accesses. This ensures that devices which only handle
> +  aligned requests do not need to implement unaligned logic themselves. For
> +  example, see xhci_cap_ops in hw/usb/hcd-xhci.c: it sets  .valid.unaligned
> +  = true so guests can do unaligned reads on the xHCI Capability Registers,
> +  while keeping .impl.unaligned = false to rely on the core splitting logic.
> +  However, if a device’s registers have side effects on read or write, this
> +  extra splitting can introduce undesired behavior. Specifically, for devices
> +  whose registers trigger state changes on each read/write, splitting an access
> +  can lead to reading or writing bytes beyond the originally requested subrange
> +  thereby triggering repeated or otherwise unintended register side effects.
> +  In such cases, one should set .valid.unaligned = false to reject unaligned
> +  accesses entirely.
> +- Conversely, if .valid.unaligned = false but .impl.unaligned = true,
> +  that setting is considered invalid; it claims unaligned access is allowed
> +  by the implementation yet disallowed for the device. QEMU enforces this with
> +  an assertion to prevent contradictory usage.

Some cosmetic comments only..

This shouldn't be another bullet, but rather a separate sub-section,
because it describes the relationship of above two entries.

IMO it can be better like this:

MMIO Operations
---------------

...

- .valid.min_access_size, .valid.max_access_size...

- .valid.unaligned...  See :ref:`unaligned-mmio-accesses` for details.

- .impl.min_access_size, .impl.max_access_size...

- .impl.unaligned...  See :ref:`unaligned-mmio-accesses` for details.

.. _unaligned-mmio-accesses:

Unaligned MMIO Accesses
~~~~~~~~~~~~~~~~~~~~~~~

...

-- 
Peter Xu



  parent reply	other threads:[~2025-09-02 16:10 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22  9:24 [RFC PATCH v2 0/9] support unaligned access to xHCI Capability CJ Chen
2025-08-22  9:24 ` [RFC PATCH v2 1/9] doc/devel/memory.rst: additional explanation for unaligned access CJ Chen
2025-09-01 17:09   ` Peter Maydell
2025-09-02 16:09   ` Peter Xu [this message]
2025-08-22  9:24 ` [RFC PATCH v2 2/9] hw/riscv: iommu-trap: remove .impl.unaligned = true CJ Chen
2025-08-24  9:22   ` Daniel Henrique Barboza
2025-08-22  9:24 ` [RFC PATCH v2 3/9] hw: npcm7xx_fiu and mx_pic change " CJ Chen
2025-08-25 11:00   ` Philippe Mathieu-Daudé
2025-09-02 19:09     ` Peter Xu
2026-04-24  9:09       ` Peter Maydell
2026-04-24  9:37         ` Max Filippov
2025-08-22  9:24 ` [RFC PATCH v2 4/9] hw/nvme/ctrl: specify the 'valid' field in MemoryRegionOps CJ Chen
2025-08-22  9:24 ` [RFC PATCH v2 5/9] system/memory: support unaligned access CJ Chen
2025-09-01 17:21   ` Peter Maydell
2025-08-22  9:24 ` [RFC PATCH v2 6/9] hw/usb/hcd-xhci: allow unaligned access to Capability Registers CJ Chen
2025-08-22  9:24 ` [RFC PATCH v2 7/9] system/memory: assert on invalid unaligned combo CJ Chen
2025-08-25 11:06   ` Philippe Mathieu-Daudé
2025-08-22  9:24 ` [RFC PATCH v2 8/9] hw/misc: add test device for memory access CJ Chen
2025-09-01 17:03   ` Peter Maydell
2025-09-04 14:01     ` Peter Xu
2025-08-22  9:24 ` [PATCH RFC v2 9/9] tests/qtest: add test for memory region access CJ Chen
2025-08-25 11:16   ` Philippe Mathieu-Daudé
2025-08-26  2:04     ` chen CJ
2025-09-01 16:57   ` Peter Maydell
2025-09-05 14:21     ` Peter Xu
2025-09-01 17:22 ` [RFC PATCH v2 0/9] support unaligned access to xHCI Capability Peter Maydell
2025-09-03  5:03 ` [Withdrawn] " chen CJ
2025-09-03  9:47   ` Peter Maydell
2025-09-05 14:32     ` Peter Xu
2026-04-24 18:01     ` Peter Maydell
2026-04-28  9:07       ` Peter Maydell

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=aLcWvR5Snm1RXEcY@x1.local \
    --to=peterx@redhat.com \
    --cc=alistair.francis@wdc.com \
    --cc=cjchen@igel.co.jp \
    --cc=david@redhat.com \
    --cc=dbarboza@ventanamicro.com \
    --cc=farosas@suse.de \
    --cc=foss@defmacro.it \
    --cc=hrstmyk811m@gmail.com \
    --cc=its@irrelevant.dk \
    --cc=jcmvbkbc@gmail.com \
    --cc=kbusch@kernel.org \
    --cc=kfting@nuvoton.com \
    --cc=liwei1518@gmail.com \
    --cc=lvivier@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=wuhaotsh@google.com \
    --cc=zhiwei_liu@linux.alibaba.com \
    /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.