From: Andrew Jeffery <andrew@aj.id.au>
To: qemu-devel@nongnu.org
Cc: Andrew Jeffery <andrew@aj.id.au>,
qemu-arm@nongnu.org, joel@jms.id.au, clg@kaod.org,
f4bug@amsat.org, pbonzini@redhat.com
Subject: [Qemu-devel] [RFC PATCH 1/1] memory: Support unaligned accesses on aligned-only models
Date: Fri, 30 Jun 2017 12:30:58 +0930 [thread overview]
Message-ID: <20170630030058.28943-1-andrew@aj.id.au> (raw)
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Hello,
This RFC patch stems from a discussion on a patch for an ADC model[1] where it
was pointed out that I should be able to use the .impl member of
MemoryRegionOps to constrain how my read() and write() callbacks where invoked.
I tried Phil's suggested approach and found I got reads of size 4, but with an
address that was not 4-byte aligned.
Looking at the source for access_with_adjusted_size() lead to the comment
/* FIXME: support unaligned access? */
which at least suggests that the implementation isn't complete.
So, this patch is a quick and incomplete attempt at resolving the issue to see
whether I'm on the right track or way off in the weeds.
I've lightly tested it with the ADC model mentioned above, and it appears to do
the right thing (I changed the values generated by the ADC to distinguish
between the lower and upper 16 bits).
Things the patch is not:
1. Capable of handling unaligned writes
2. Tested on big-endian models
If the general idea of the patch is reasonable I'll look to resolve the above
to points.
Cheers,
Andrew
[1] http://lists.nongnu.org/archive/html/qemu-arm/2017-05/msg00400.html
memory.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 137 insertions(+), 7 deletions(-)
diff --git a/memory.c b/memory.c
index 0ddc4cc28deb..b9fae8d382bc 100644
--- a/memory.c
+++ b/memory.c
@@ -432,6 +432,7 @@ static MemTxResult memory_region_read_accessor(MemoryRegion *mr,
{
uint64_t tmp;
+
tmp = mr->ops->read(mr->opaque, addr, size);
if (mr->subpage) {
trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);
@@ -552,7 +553,7 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
return mr->ops->write_with_attrs(mr->opaque, addr, tmp, size, attrs);
}
-static MemTxResult access_with_adjusted_size(hwaddr addr,
+static MemTxResult access_with_adjusted_size_aligned(hwaddr addr,
uint64_t *value,
unsigned size,
unsigned access_size_min,
@@ -567,10 +568,11 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
MemoryRegion *mr,
MemTxAttrs attrs)
{
+
+ MemTxResult r = MEMTX_OK;
uint64_t access_mask;
unsigned access_size;
unsigned i;
- MemTxResult r = MEMTX_OK;
if (!access_size_min) {
access_size_min = 1;
@@ -579,7 +581,6 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
access_size_max = 4;
}
- /* FIXME: support unaligned access? */
access_size = MAX(MIN(size, access_size_max), access_size_min);
access_mask = -1ULL >> (64 - access_size * 8);
if (memory_region_big_endian(mr)) {
@@ -596,6 +597,133 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
return r;
}
+/* Assume power-of-two size */
+#define align_down(addr, size) ((addr) & ~((size) - 1))
+#define align_up(addr, size) \
+ ({ typeof(size) __size = size; align_down((addr) + (__size) - 1, (__size)); })
+
+static MemTxResult access_with_adjusted_size_unaligned(hwaddr addr,
+ uint64_t *value,
+ unsigned size,
+ unsigned access_size_min,
+ unsigned access_size_max,
+ bool unaligned,
+ MemTxResult (*access)(MemoryRegion *mr,
+ hwaddr addr,
+ uint64_t *value,
+ unsigned size,
+ unsigned shift,
+ uint64_t mask,
+ MemTxAttrs attrs),
+ MemoryRegion *mr,
+ MemTxAttrs attrs)
+{
+ uint64_t access_value = 0;
+ MemTxResult r = MEMTX_OK;
+ hwaddr access_addr[2];
+ uint64_t access_mask;
+ unsigned access_size;
+
+ if (unlikely(!access_size_min)) {
+ access_size_min = 1;
+ }
+ if (unlikely(!access_size_max)) {
+ access_size_max = 4;
+ }
+
+ access_size = MAX(MIN(size, access_size_max), access_size_min);
+ access_addr[0] = align_down(addr, access_size);
+ access_addr[1] = align_up(addr + size, access_size);
+
+ if (memory_region_big_endian(mr)) {
+ hwaddr cur;
+
+ /* XXX: Big-endian path is untested... */
+
+ for (cur = access_addr[0]; cur < access_addr[1]; cur += access_size) {
+ uint64_t mask_bounds[2];
+
+ mask_bounds[0] = MAX(addr, cur) - cur;
+ mask_bounds[1] =
+ MIN(addr + size, align_up(cur + 1, access_size)) - cur;
+
+ access_mask = (-1ULL << mask_bounds[0] * 8) &
+ (-1ULL >> (64 - mask_bounds[1] * 8));
+
+ r |= access(mr, cur, &access_value, access_size,
+ (size - access_size - (MAX(addr, cur) - addr)),
+ access_mask, attrs);
+
+ /* XXX: Can't do this hack for writes */
+ access_value >>= mask_bounds[0] * 8;
+ }
+ } else {
+ hwaddr cur;
+
+ for (cur = access_addr[0]; cur < access_addr[1]; cur += access_size) {
+ uint64_t mask_bounds[2];
+
+ mask_bounds[0] = MAX(addr, cur) - cur;
+ mask_bounds[1] =
+ MIN(addr + size, align_up(cur + 1, access_size)) - cur;
+
+ access_mask = (-1ULL << mask_bounds[0] * 8) &
+ (-1ULL >> (64 - mask_bounds[1] * 8));
+
+ r |= access(mr, cur, &access_value, access_size,
+ (MAX(addr, cur) - addr), access_mask, attrs);
+
+ /* XXX: Can't do this hack for writes */
+ access_value >>= mask_bounds[0] * 8;
+ }
+ }
+
+ *value = access_value;
+
+ return r;
+}
+
+static inline MemTxResult access_with_adjusted_size(hwaddr addr,
+ uint64_t *value,
+ unsigned size,
+ unsigned access_size_min,
+ unsigned access_size_max,
+ bool unaligned,
+ MemTxResult (*access)(MemoryRegion *mr,
+ hwaddr addr,
+ uint64_t *value,
+ unsigned size,
+ unsigned shift,
+ uint64_t mask,
+ MemTxAttrs attrs),
+ MemoryRegion *mr,
+ MemTxAttrs attrs)
+{
+ unsigned access_size;
+
+ if (!access_size_min) {
+ access_size_min = 1;
+ }
+ if (!access_size_max) {
+ access_size_max = 4;
+ }
+
+ access_size = MAX(MIN(size, access_size_max), access_size_min);
+
+ /* Handle unaligned accesses if the model only supports natural alignment */
+ if (unlikely((addr & (access_size - 1)) && !unaligned)) {
+ return access_with_adjusted_size_unaligned(addr, value, size,
+ access_size_min, access_size_max, unaligned, access, mr, attrs);
+ }
+
+ /*
+ * Otherwise, if the access is aligned or the model specifies it can handle
+ * unaligned accesses, use the 'aligned' handler
+ */
+ return access_with_adjusted_size_aligned(addr, value, size,
+ access_size_min, access_size_max, access, mr, attrs);
+}
+
static AddressSpace *memory_region_to_address_space(MemoryRegion *mr)
{
AddressSpace *as;
@@ -1238,16 +1366,18 @@ static MemTxResult memory_region_dispatch_read1(MemoryRegion *mr,
return access_with_adjusted_size(addr, pval, size,
mr->ops->impl.min_access_size,
mr->ops->impl.max_access_size,
+ mr->ops->impl.unaligned,
memory_region_read_accessor,
mr, attrs);
} else if (mr->ops->read_with_attrs) {
return access_with_adjusted_size(addr, pval, size,
mr->ops->impl.min_access_size,
mr->ops->impl.max_access_size,
+ mr->ops->impl.unaligned,
memory_region_read_with_attrs_accessor,
mr, attrs);
} else {
- return access_with_adjusted_size(addr, pval, size, 1, 4,
+ return access_with_adjusted_size(addr, pval, size, 1, 4, true,
memory_region_oldmmio_read_accessor,
mr, attrs);
}
@@ -1316,20 +1446,20 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
}
if (mr->ops->write) {
- return access_with_adjusted_size(addr, &data, size,
+ return access_with_adjusted_size_aligned(addr, &data, size,
mr->ops->impl.min_access_size,
mr->ops->impl.max_access_size,
memory_region_write_accessor, mr,
attrs);
} else if (mr->ops->write_with_attrs) {
return
- access_with_adjusted_size(addr, &data, size,
+ access_with_adjusted_size_aligned(addr, &data, size,
mr->ops->impl.min_access_size,
mr->ops->impl.max_access_size,
memory_region_write_with_attrs_accessor,
mr, attrs);
} else {
- return access_with_adjusted_size(addr, &data, size, 1, 4,
+ return access_with_adjusted_size_aligned(addr, &data, size, 1, 4,
memory_region_oldmmio_write_accessor,
mr, attrs);
}
--
2.11.0
next reply other threads:[~2017-06-30 3:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-30 3:00 Andrew Jeffery [this message]
2017-07-13 12:05 ` [Qemu-devel] [RFC PATCH 1/1] memory: Support unaligned accesses on aligned-only models Paolo Bonzini
2017-07-14 6:20 ` Andrew Jeffery
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=20170630030058.28943-1-andrew@aj.id.au \
--to=andrew@aj.id.au \
--cc=clg@kaod.org \
--cc=f4bug@amsat.org \
--cc=joel@jms.id.au \
--cc=pbonzini@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/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).