diff for duplicates of <1564038073754.91133@bt.com> diff --git a/a/content_digest b/N1/content_digest index 9965110..684bf43 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,33 +1,33 @@ "ref\0e9c6e5310b1a4863be45d45bf087fc3d@tpw09926dag18e.domain1.systemhost.net\0" "ref\01563810716254.18886@bt.com\0" "From\0<tony.nguyen@bt.com>\0" - "Subject\0[Qemu-riscv] [Qemu-devel] [PATCH v3 00/15] Invert Endian bit in SPARCv9 MMU TTE\0" + "Subject\0[Qemu-arm] [Qemu-devel] [PATCH v3 00/15] Invert Endian bit in SPARCv9 MMU TTE\0" "Date\0Thu, 25 Jul 2019 07:01:14 +0000\0" "To\0<qemu-devel@nongnu.org>\0" - "Cc\0<peter.maydell@linaro.org>" - <walling@linux.ibm.com> - <david@redhat.com> - <palmer@sifive.com> - <mark.cave-ayland@ilande.co.uk> - <Alistair.Francis@wdc.com> - <arikalo@wavecomp.com> - <mst@redhat.com> - <pasic@linux.ibm.com> - <borntraeger@de.ibm.com> - <rth@twiddle.net> - <atar4qemu@gmail.com> - <ehabkost@redhat.com> - <sw@weilnetz.de> - <alex.williamson@redhat.com> - <qemu-arm@nongnu.org> - <david@gibson.dropbear.id.au> - <qemu-riscv@nongnu.org> - <cohuck@redhat.com> - <qemu-s390x@nongnu.org> - <qemu-ppc@nongnu.org> - <amarkovic@wavecomp.com> - <pbonzini@redhat.com> - " <aurelien@aurel32.net>\0" + "Cc\0peter.maydell@linaro.org" + walling@linux.ibm.com + mst@redhat.com + palmer@sifive.com + mark.cave-ayland@ilande.co.uk + Alistair.Francis@wdc.com + arikalo@wavecomp.com + david@redhat.com + pasic@linux.ibm.com + borntraeger@de.ibm.com + rth@twiddle.net + atar4qemu@gmail.com + ehabkost@redhat.com + sw@weilnetz.de + qemu-s390x@nongnu.org + qemu-arm@nongnu.org + david@gibson.dropbear.id.au + qemu-riscv@nongnu.org + cohuck@redhat.com + alex.williamson@redhat.com + qemu-ppc@nongnu.org + amarkovic@wavecomp.com + pbonzini@redhat.com + " aurelien@aurel32.net\0" "\01:1\0" "b\0" "This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.\n" @@ -483,4 +483,4 @@ "</body>\r\n" "</html>\r\n" -31a10d1c5c4f6762c7d074fd45f10379cccf0eeddda8796d58c772607ccf1fe4 +b939afe4916321b8764469cffd8fa1ed6f5f62a6f0096a28512c3a204864e042
diff --git a/a/2.bin b/a/2.bin deleted file mode 100644 index e19c044..0000000 --- a/a/2.bin +++ /dev/null @@ -1,239 +0,0 @@ -<html> -<head> -<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> -<style type="text/css" style="display:none"><!-- P { margin-top: 0px; margin-bottom: 0px; } p { margin-top: 0px; margin-bottom: 0px; } .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left-width: 2px; border-left-style: solid; border-left-color: rgb(128, 0, 0); }--></style> -</head> -<body dir="ltr" style="font-size:12pt;color:#000000;background-color:#FFFFFF;font-family:Calibri,Arial,Helvetica,sans-serif;"> -<p></p> -<div><span style="font-size: 12pt;">This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.</span><br> -</div> -<div><br> -</div> -<div>It is an attempt of the instructions outlined by Richard Henderson to Mark</div> -<div>Cave-Ayland.</div> -<div><br> -</div> -<div>Tested with OpenBSD on sun4u. Solaris 10 is my actual goal, but unfortunately a</div> -<div>separate keyboard issue remains in the way.</div> -<div><br> -</div> -<div>On 01/11/17 19:15, Mark Cave-Ayland wrote:</div> -<div><br> -</div> -<div>>On 15/08/17 19:10, Richard Henderson wrote:</div> -<div>></div> -<div>>> [CC Peter re MemTxAttrs below]</div> -<div>>></div> -<div>>> On 08/15/2017 09:38 AM, Mark Cave-Ayland wrote:</div> -<div>>>> Working through an incorrect endian issue on qemu-system-sparc64, it has</div> -<div>>>> become apparent that at least one OS makes use of the IE (Invert Endian)</div> -<div>>>> bit in the SPARCv9 MMU TTE to map PCI memory space without the</div> -<div>>>> programmer having to manually endian-swap accesses.</div> -<div>>>></div> -<div>>>> In other words, to quote the UltraSPARC specification: "if this bit is</div> -<div>>>> set, accesses to the associated page are processed with inverse</div> -<div>>>> endianness from what is specified by the instruction (big-for-little and</div> -<div>>>> little-for-big)".</div> -<div><br> -</div> -<div>A good explanation by Mark why the IE bit is required.</div> -<div><br> -</div> -<div>>>></div> -<div>>>> Looking through various bits of code, I'm trying to get a feel for the</div> -<div>>>> best way to implement this in an efficient manner. From what I can see</div> -<div>>>> this could be solved using an additional MMU index, however I'm not</div> -<div>>>> overly familiar with the memory and softmmu subsystems.</div> -<div>>></div> -<div>>> No, it can't be solved with an MMU index.</div> -<div>>></div> -<div>>>> Can anyone point me in the right direction as to what would be the best</div> -<div>>>> way to implement this feature within QEMU?</div> -<div>>></div> -<div>>> It's definitely tricky.</div> -<div>>></div> -<div>>> We definitely need some TLB_FLAGS_MASK bit set so that we're forced through</div> -<div>>> the</div> -<div>>> memory slow path. There is no other way to bypass the endianness that we've</div> -<div>>> already encoded from the target instruction.</div> -<div>>></div> -<div>>> Given the tlb_set_page_with_attrs interface, I would think that we need a new</div> -<div>>> bit in MemTxAttrs, so that the target/sparc tlb_fill (and subroutines) can</div> -<div>>> pass</div> -<div>>> along the TTE bit for the given page.</div> -<div>>></div> -<div>>> We have an existing problem in softmmu_template.h,</div> -<div>>></div> -<div>>> /* ??? Note that the io helpers always read data in the target</div> -<div>>> byte ordering. We should push the LE/BE request down into io. */</div> -<div>>> res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr);</div> -<div>>> res = TGT_BE(res);</div> -<div>>></div> -<div>>> We do not want to add a third(!) byte swap along the i/o path. We need to</div> -<div>>> collapse the two that we have already before considering this one.</div> -<div>>></div> -<div>>> This probably takes the form of:</div> -<div>>></div> -<div>>> (1) Replacing the "int size" argument with "TCGMemOp memop" for</div> -<div>>> a) io_{read,write}x in accel/tcg/cputlb.c,</div> -<div>>> b) memory_region_dispatch_{read,write} in memory.c,</div> -<div>>> c) adjust_endianness in memory.c.</div> -<div>>> This carries size+sign+endianness down to the next level.</div> -<div>>></div> -<div>>> (2) In memory.c, adjust_endianness,</div> -<div>>></div> -<div>>> if (memory_region_wrong_endianness(mr)) {</div> -<div>>> - switch (size) {</div> -<div>>> + memop ^= MO_BSWAP;</div> -<div>>> + }</div> -<div>>> + if (memop & MO_BSWAP) {</div> -<div>>></div> -<div>>> For extra credit, re-arrange memory_region_wrong_endianness</div> -<div>>> to something more explicit -- "wrong" isn't helpful.</div> -<div>></div> -<div>>Finally I've had a bit of spare time to experiment with this approach,</div> -<div>>and from what I can see there are currently 2 issues:</div> -<div>></div> -<div>></div> -<div>>1) Using TCGMemOp in memory.c means it is no longer accelerator agnostic</div> -<div>></div> -<div>>For the moment I've defined a separate MemOp in memory.h and provided a</div> -<div>>mapping function in io_{read,write}x to map from TCGMemOp to MemOp and</div> -<div>>then pass that into memory_region_dispatch_{read,write}.</div> -<div>></div> -<div>>Other than not referencing TCGMemOp in the memory API, another reason</div> -<div>>for doing this was that I wasn't convinced that all the MO_ attributes</div> -<div>>were valid outside of TCG. I do, of course, strongly defer to other</div> -<div>>people's knowledge in this area though.</div> -<div>></div> -<div>></div> -<div>>2) The above changes to adjust_endianness() fail when</div> -<div>>memory_region_dispatch_{read,write} are called recursively</div> -<div>></div> -<div>>Whilst booting qemu-system-sparc64 I see that</div> -<div>>memory_region_dispatch_{read,write} get called recursively - once via</div> -<div>>io_{read,write}x and then again via flatview_read_continue() in exec.c.</div> -<div>></div> -<div>>The net effect of this is that we perform the bswap correctly at the</div> -<div>>tail of the recursion, but then as we travel back up the stack we hit</div> -<div>>memory_region_dispatch_{read,write} once again causing a second bswap</div> -<div>>which means the value is returned with the incorrect endian again.</div> -<div>></div> -<div>></div> -<div>>My understanding from your softmmu_template.h comment above is that the</div> -<div>>memory API should do the endian swapping internally allowing the removal</div> -<div>>of the final TGT_BE/TGT_LE applied to the result, or did I get this wrong?</div> -<div>></div> -<div>>> (3) In tlb_set_page_with_attrs, notice attrs.byte_swap and set</div> -<div>>> a new TLB_FORCE_SLOW bit within TLB_FLAGS_MASK.</div> -<div>>></div> -<div>>> (4) In io_{read,write}x, if iotlbentry->attrs.byte_swap is set,</div> -<div>>> then memop ^= MO_BSWAP.</div> -<div><br> -</div> -<div>Thanks all for the v1 and v2 feedback.</div> -<div><br> -</div> -<div>v2:</div> -<div>- Moved size+sign+endianness attributes from TCGMemOp into MemOp.</div> -<div> In v1 TCGMemOp was re-purposed entirely into MemOp.</div> -<div>- Replaced MemOp MO_{8|16|32|64} with TCGMemOp MO_{UB|UW|UL|UQ} alias.</div> -<div> This is to avoid warnings on comparing and coercing different enums.</div> -<div>- Renamed get_memop to get_tcgmemop for clarity.</div> -<div>- MEMOP is now SIZE_MEMOP, which is just ctzl(size).</div> -<div>- Split patch 3/4 so one memory_region_dispatch_{read|write} interface</div> -<div> is converted per patch.</div> -<div>- Do not reuse TLB_RECHECK, use new TLB_FORCE_SLOW instead.</div> -<div>- Split patch 4/4 so adding the MemTxAddrs parameters and converting</div> -<div> tlb_set_page() to tlb_set_page_with_attrs() is separate from usage.</div> -<div>- CC'd maintainers.</div> -<div><br> -</div> -<div>v3:</div> -<div>- Like v1, the entire TCGMemOp enum is now MemOp.</div> -<div>- MemOp target dependant attributes are conditional upon NEED_CPU_H <br> -</div> -<div><br> -</div> -<div>Tony Nguyen (15):</div> -<div> tcg: TCGMemOp is now accelerator independent MemOp</div> -<div> memory: Access MemoryRegion with MemOp</div> -<div> target/mips: Access MemoryRegion with MemOp</div> -<div> hw/s390x: Access MemoryRegion with MemOp</div> -<div> hw/intc/armv7m_nic: Access MemoryRegion with MemOp</div> -<div> hw/virtio: Access MemoryRegion with MemOp</div> -<div> hw/vfio: Access MemoryRegion with MemOp</div> -<div> exec: Access MemoryRegion with MemOp</div> -<div> cputlb: Access MemoryRegion with MemOp</div> -<div> memory: Access MemoryRegion with MemOp semantics</div> -<div> memory: Single byte swap along the I/O path</div> -<div> cpu: TLB_FLAGS_MASK bit to force memory slow path</div> -<div> cputlb: Byte swap memory transaction attribute</div> -<div> target/sparc: Add TLB entry with attributes</div> -<div> target/sparc: sun4u Invert Endian TTE bit</div> -<div><br> -</div> -<div> accel/tcg/cputlb.c | 71 +++++++++--------</div> -<div> exec.c | 6 +-</div> -<div> hw/intc/armv7m_nvic.c | 12 ++-</div> -<div> hw/s390x/s390-pci-inst.c | 8 +-</div> -<div> hw/vfio/pci-quirks.c | 5 +-</div> -<div> hw/virtio/virtio-pci.c | 7 +-</div> -<div> include/exec/cpu-all.h | 10 ++-</div> -<div> include/exec/memattrs.h | 2 +</div> -<div> include/exec/memop.h | 112 +++++++++++++++++++++++++++</div> -<div> include/exec/memory.h | 9 ++-</div> -<div> memory.c | 37 +++++----</div> -<div> memory_ldst.inc.c | 18 ++---</div> -<div> target/alpha/translate.c | 2 +-</div> -<div> target/arm/translate-a64.c | 48 ++++++------</div> -<div> target/arm/translate-a64.h | 2 +-</div> -<div> target/arm/translate-sve.c | 2 +-</div> -<div> target/arm/translate.c | 32 ++++----</div> -<div> target/arm/translate.h | 2 +-</div> -<div> target/hppa/translate.c | 14 ++--</div> -<div> target/i386/translate.c | 132 ++++++++++++++++----------------</div> -<div> target/m68k/translate.c | 2 +-</div> -<div> target/microblaze/translate.c | 4 +-</div> -<div> target/mips/op_helper.c | 5 +-</div> -<div> target/mips/translate.c | 8 +-</div> -<div> target/openrisc/translate.c | 4 +-</div> -<div> target/ppc/translate.c | 12 +--</div> -<div> target/riscv/insn_trans/trans_rva.inc.c | 8 +-</div> -<div> target/riscv/insn_trans/trans_rvi.inc.c | 4 +-</div> -<div> target/s390x/translate.c | 6 +-</div> -<div> target/s390x/translate_vx.inc.c | 10 +--</div> -<div> target/sparc/cpu.h | 2 +</div> -<div> target/sparc/mmu_helper.c | 40 ++++++----</div> -<div> target/sparc/translate.c | 14 ++--</div> -<div> target/tilegx/translate.c | 10 +--</div> -<div> target/tricore/translate.c | 8 +-</div> -<div> tcg/README | 2 +-</div> -<div> tcg/aarch64/tcg-target.inc.c | 26 +++----</div> -<div> tcg/arm/tcg-target.inc.c | 26 +++----</div> -<div> tcg/i386/tcg-target.inc.c | 24 +++---</div> -<div> tcg/mips/tcg-target.inc.c | 16 ++--</div> -<div> tcg/optimize.c | 2 +-</div> -<div> tcg/ppc/tcg-target.inc.c | 12 +--</div> -<div> tcg/riscv/tcg-target.inc.c | 20 ++---</div> -<div> tcg/s390/tcg-target.inc.c | 14 ++--</div> -<div> tcg/sparc/tcg-target.inc.c | 6 +-</div> -<div> tcg/tcg-op.c | 38 ++++-----</div> -<div> tcg/tcg-op.h | 86 ++++++++++-----------</div> -<div> tcg/tcg.c | 2 +-</div> -<div> tcg/tcg.h | 99 ++----------------------</div> -<div> trace/mem-internal.h | 4 +-</div> -<div> trace/mem.h | 4 +-</div> -<div> 51 files changed, 561 insertions(+), 488 deletions(-)</div> -<div> create mode 100644 include/exec/memop.h</div> -<div><br> -</div> -<div>-- </div> -<div>1.8.3.1</div> -<div><br> -<br> -</div> -<p><br> -</p> -</body> -</html> diff --git a/a/2.hdr b/a/2.hdr deleted file mode 100644 index e54d0ae..0000000 --- a/a/2.hdr +++ /dev/null @@ -1,2 +0,0 @@ -Content-Type: text/html; charset="iso-8859-1" -Content-Transfer-Encoding: quoted-printable diff --git a/a/content_digest b/N2/content_digest index 9965110..341a385 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -1,34 +1,34 @@ "ref\0e9c6e5310b1a4863be45d45bf087fc3d@tpw09926dag18e.domain1.systemhost.net\0" "ref\01563810716254.18886@bt.com\0" "From\0<tony.nguyen@bt.com>\0" - "Subject\0[Qemu-riscv] [Qemu-devel] [PATCH v3 00/15] Invert Endian bit in SPARCv9 MMU TTE\0" + "Subject\0[Qemu-devel] [PATCH v3 00/15] Invert Endian bit in SPARCv9 MMU TTE\0" "Date\0Thu, 25 Jul 2019 07:01:14 +0000\0" "To\0<qemu-devel@nongnu.org>\0" - "Cc\0<peter.maydell@linaro.org>" - <walling@linux.ibm.com> - <david@redhat.com> - <palmer@sifive.com> - <mark.cave-ayland@ilande.co.uk> - <Alistair.Francis@wdc.com> - <arikalo@wavecomp.com> - <mst@redhat.com> - <pasic@linux.ibm.com> - <borntraeger@de.ibm.com> - <rth@twiddle.net> - <atar4qemu@gmail.com> - <ehabkost@redhat.com> - <sw@weilnetz.de> - <alex.williamson@redhat.com> - <qemu-arm@nongnu.org> - <david@gibson.dropbear.id.au> - <qemu-riscv@nongnu.org> - <cohuck@redhat.com> - <qemu-s390x@nongnu.org> - <qemu-ppc@nongnu.org> - <amarkovic@wavecomp.com> - <pbonzini@redhat.com> - " <aurelien@aurel32.net>\0" - "\01:1\0" + "Cc\0peter.maydell@linaro.org" + walling@linux.ibm.com + mst@redhat.com + palmer@sifive.com + mark.cave-ayland@ilande.co.uk + Alistair.Francis@wdc.com + arikalo@wavecomp.com + david@redhat.com + pasic@linux.ibm.com + borntraeger@de.ibm.com + rth@twiddle.net + atar4qemu@gmail.com + ehabkost@redhat.com + sw@weilnetz.de + qemu-s390x@nongnu.org + qemu-arm@nongnu.org + david@gibson.dropbear.id.au + qemu-riscv@nongnu.org + cohuck@redhat.com + alex.williamson@redhat.com + qemu-ppc@nongnu.org + amarkovic@wavecomp.com + pbonzini@redhat.com + " aurelien@aurel32.net\0" + "\00:1\0" "b\0" "This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.\n" "\n" @@ -241,246 +241,5 @@ "\n" "--\n" 1.8.3.1 - "\01:2\0" - "b\0" - "<html>\r\n" - "<head>\r\n" - "<meta http-equiv=\"Content-Type\" content=\"text/html; charset=iso-8859-1\">\r\n" - "<style type=\"text/css\" style=\"display:none\"><!-- P { margin-top: 0px; margin-bottom: 0px; } p { margin-top: 0px; margin-bottom: 0px; } .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left-width: 2px; border-left-style: solid; border-left-color: rgb(128, 0, 0); }--></style>\r\n" - "</head>\r\n" - "<body dir=\"ltr\" style=\"font-size:12pt;color:#000000;background-color:#FFFFFF;font-family:Calibri,Arial,Helvetica,sans-serif;\">\r\n" - "<p></p>\r\n" - "<div><span style=\"font-size: 12pt;\">This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.</span><br>\r\n" - "</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>It is an attempt of the instructions outlined by Richard Henderson to Mark</div>\r\n" - "<div>Cave-Ayland.</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>Tested with OpenBSD on sun4u. Solaris 10 is my actual goal, but unfortunately a</div>\r\n" - "<div>separate keyboard issue remains in the way.</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>On 01/11/17 19:15, Mark Cave-Ayland wrote:</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>>On 15/08/17 19:10, Richard Henderson wrote:</div>\r\n" - "<div>></div>\r\n" - "<div>>> [CC Peter re MemTxAttrs below]</div>\r\n" - "<div>>></div>\r\n" - "<div>>> On 08/15/2017 09:38 AM, Mark Cave-Ayland wrote:</div>\r\n" - "<div>>>> Working through an incorrect endian issue on qemu-system-sparc64, it has</div>\r\n" - "<div>>>> become apparent that at least one OS makes use of the IE (Invert Endian)</div>\r\n" - "<div>>>> bit in the SPARCv9 MMU TTE to map PCI memory space without the</div>\r\n" - "<div>>>> programmer having to manually endian-swap accesses.</div>\r\n" - "<div>>>></div>\r\n" - "<div>>>> In other words, to quote the UltraSPARC specification: "if this bit is</div>\r\n" - "<div>>>> set, accesses to the associated page are processed with inverse</div>\r\n" - "<div>>>> endianness from what is specified by the instruction (big-for-little and</div>\r\n" - "<div>>>> little-for-big)".</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>A good explanation by Mark why the IE bit is required.</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>>>></div>\r\n" - "<div>>>> Looking through various bits of code, I'm trying to get a feel for the</div>\r\n" - "<div>>>> best way to implement this in an efficient manner. From what I can see</div>\r\n" - "<div>>>> this could be solved using an additional MMU index, however I'm not</div>\r\n" - "<div>>>> overly familiar with the memory and softmmu subsystems.</div>\r\n" - "<div>>></div>\r\n" - "<div>>> No, it can't be solved with an MMU index.</div>\r\n" - "<div>>></div>\r\n" - "<div>>>> Can anyone point me in the right direction as to what would be the best</div>\r\n" - "<div>>>> way to implement this feature within QEMU?</div>\r\n" - "<div>>></div>\r\n" - "<div>>> It's definitely tricky.</div>\r\n" - "<div>>></div>\r\n" - "<div>>> We definitely need some TLB_FLAGS_MASK bit set so that we're forced through</div>\r\n" - "<div>>> the</div>\r\n" - "<div>>> memory slow path. There is no other way to bypass the endianness that we've</div>\r\n" - "<div>>> already encoded from the target instruction.</div>\r\n" - "<div>>></div>\r\n" - "<div>>> Given the tlb_set_page_with_attrs interface, I would think that we need a new</div>\r\n" - "<div>>> bit in MemTxAttrs, so that the target/sparc tlb_fill (and subroutines) can</div>\r\n" - "<div>>> pass</div>\r\n" - "<div>>> along the TTE bit for the given page.</div>\r\n" - "<div>>></div>\r\n" - "<div>>> We have an existing problem in softmmu_template.h,</div>\r\n" - "<div>>></div>\r\n" - "<div>>> /* ??? Note that the io helpers always read data in the target</div>\r\n" - "<div>>> byte ordering. We should push the LE/BE request down into io. */</div>\r\n" - "<div>>> res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr);</div>\r\n" - "<div>>> res = TGT_BE(res);</div>\r\n" - "<div>>></div>\r\n" - "<div>>> We do not want to add a third(!) byte swap along the i/o path. We need to</div>\r\n" - "<div>>> collapse the two that we have already before considering this one.</div>\r\n" - "<div>>></div>\r\n" - "<div>>> This probably takes the form of:</div>\r\n" - "<div>>></div>\r\n" - "<div>>> (1) Replacing the "int size" argument with "TCGMemOp memop" for</div>\r\n" - "<div>>> a) io_{read,write}x in accel/tcg/cputlb.c,</div>\r\n" - "<div>>> b) memory_region_dispatch_{read,write} in memory.c,</div>\r\n" - "<div>>> c) adjust_endianness in memory.c.</div>\r\n" - "<div>>> This carries size+sign+endianness down to the next level.</div>\r\n" - "<div>>></div>\r\n" - "<div>>> (2) In memory.c, adjust_endianness,</div>\r\n" - "<div>>></div>\r\n" - "<div>>> if (memory_region_wrong_endianness(mr)) {</div>\r\n" - "<div>>> - switch (size) {</div>\r\n" - "<div>>> + memop ^= MO_BSWAP;</div>\r\n" - "<div>>> + }</div>\r\n" - "<div>>> + if (memop & MO_BSWAP) {</div>\r\n" - "<div>>></div>\r\n" - "<div>>> For extra credit, re-arrange memory_region_wrong_endianness</div>\r\n" - "<div>>> to something more explicit -- "wrong" isn't helpful.</div>\r\n" - "<div>></div>\r\n" - "<div>>Finally I've had a bit of spare time to experiment with this approach,</div>\r\n" - "<div>>and from what I can see there are currently 2 issues:</div>\r\n" - "<div>></div>\r\n" - "<div>></div>\r\n" - "<div>>1) Using TCGMemOp in memory.c means it is no longer accelerator agnostic</div>\r\n" - "<div>></div>\r\n" - "<div>>For the moment I've defined a separate MemOp in memory.h and provided a</div>\r\n" - "<div>>mapping function in io_{read,write}x to map from TCGMemOp to MemOp and</div>\r\n" - "<div>>then pass that into memory_region_dispatch_{read,write}.</div>\r\n" - "<div>></div>\r\n" - "<div>>Other than not referencing TCGMemOp in the memory API, another reason</div>\r\n" - "<div>>for doing this was that I wasn't convinced that all the MO_ attributes</div>\r\n" - "<div>>were valid outside of TCG. I do, of course, strongly defer to other</div>\r\n" - "<div>>people's knowledge in this area though.</div>\r\n" - "<div>></div>\r\n" - "<div>></div>\r\n" - "<div>>2) The above changes to adjust_endianness() fail when</div>\r\n" - "<div>>memory_region_dispatch_{read,write} are called recursively</div>\r\n" - "<div>></div>\r\n" - "<div>>Whilst booting qemu-system-sparc64 I see that</div>\r\n" - "<div>>memory_region_dispatch_{read,write} get called recursively - once via</div>\r\n" - "<div>>io_{read,write}x and then again via flatview_read_continue() in exec.c.</div>\r\n" - "<div>></div>\r\n" - "<div>>The net effect of this is that we perform the bswap correctly at the</div>\r\n" - "<div>>tail of the recursion, but then as we travel back up the stack we hit</div>\r\n" - "<div>>memory_region_dispatch_{read,write} once again causing a second bswap</div>\r\n" - "<div>>which means the value is returned with the incorrect endian again.</div>\r\n" - "<div>></div>\r\n" - "<div>></div>\r\n" - "<div>>My understanding from your softmmu_template.h comment above is that the</div>\r\n" - "<div>>memory API should do the endian swapping internally allowing the removal</div>\r\n" - "<div>>of the final TGT_BE/TGT_LE applied to the result, or did I get this wrong?</div>\r\n" - "<div>></div>\r\n" - "<div>>> (3) In tlb_set_page_with_attrs, notice attrs.byte_swap and set</div>\r\n" - "<div>>> a new TLB_FORCE_SLOW bit within TLB_FLAGS_MASK.</div>\r\n" - "<div>>></div>\r\n" - "<div>>> (4) In io_{read,write}x, if iotlbentry->attrs.byte_swap is set,</div>\r\n" - "<div>>> then memop ^= MO_BSWAP.</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>Thanks all for the v1 and v2 feedback.</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>v2:</div>\r\n" - "<div>- Moved size+sign+endianness attributes from TCGMemOp into MemOp.</div>\r\n" - "<div> In v1 TCGMemOp was re-purposed entirely into MemOp.</div>\r\n" - "<div>- Replaced MemOp MO_{8|16|32|64} with TCGMemOp MO_{UB|UW|UL|UQ} alias.</div>\r\n" - "<div> This is to avoid warnings on comparing and coercing different enums.</div>\r\n" - "<div>- Renamed get_memop to get_tcgmemop for clarity.</div>\r\n" - "<div>- MEMOP is now SIZE_MEMOP, which is just ctzl(size).</div>\r\n" - "<div>- Split patch 3/4 so one memory_region_dispatch_{read|write} interface</div>\r\n" - "<div> is converted per patch.</div>\r\n" - "<div>- Do not reuse TLB_RECHECK, use new TLB_FORCE_SLOW instead.</div>\r\n" - "<div>- Split patch 4/4 so adding the MemTxAddrs parameters and converting</div>\r\n" - "<div> tlb_set_page() to tlb_set_page_with_attrs() is separate from usage.</div>\r\n" - "<div>- CC'd maintainers.</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>v3:</div>\r\n" - "<div>- Like v1, the entire TCGMemOp enum is now MemOp.</div>\r\n" - "<div>- MemOp target dependant attributes are conditional upon NEED_CPU_H <br>\r\n" - "</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>Tony Nguyen (15):</div>\r\n" - "<div> tcg: TCGMemOp is now accelerator independent MemOp</div>\r\n" - "<div> memory: Access MemoryRegion with MemOp</div>\r\n" - "<div> target/mips: Access MemoryRegion with MemOp</div>\r\n" - "<div> hw/s390x: Access MemoryRegion with MemOp</div>\r\n" - "<div> hw/intc/armv7m_nic: Access MemoryRegion with MemOp</div>\r\n" - "<div> hw/virtio: Access MemoryRegion with MemOp</div>\r\n" - "<div> hw/vfio: Access MemoryRegion with MemOp</div>\r\n" - "<div> exec: Access MemoryRegion with MemOp</div>\r\n" - "<div> cputlb: Access MemoryRegion with MemOp</div>\r\n" - "<div> memory: Access MemoryRegion with MemOp semantics</div>\r\n" - "<div> memory: Single byte swap along the I/O path</div>\r\n" - "<div> cpu: TLB_FLAGS_MASK bit to force memory slow path</div>\r\n" - "<div> cputlb: Byte swap memory transaction attribute</div>\r\n" - "<div> target/sparc: Add TLB entry with attributes</div>\r\n" - "<div> target/sparc: sun4u Invert Endian TTE bit</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div> accel/tcg/cputlb.c | 71 +++++++++--------</div>\r\n" - "<div> exec.c | 6 +-</div>\r\n" - "<div> hw/intc/armv7m_nvic.c | 12 ++-</div>\r\n" - "<div> hw/s390x/s390-pci-inst.c | 8 +-</div>\r\n" - "<div> hw/vfio/pci-quirks.c | 5 +-</div>\r\n" - "<div> hw/virtio/virtio-pci.c | 7 +-</div>\r\n" - "<div> include/exec/cpu-all.h | 10 ++-</div>\r\n" - "<div> include/exec/memattrs.h | 2 +</div>\r\n" - "<div> include/exec/memop.h | 112 +++++++++++++++++++++++++++</div>\r\n" - "<div> include/exec/memory.h | 9 ++-</div>\r\n" - "<div> memory.c | 37 +++++----</div>\r\n" - "<div> memory_ldst.inc.c | 18 ++---</div>\r\n" - "<div> target/alpha/translate.c | 2 +-</div>\r\n" - "<div> target/arm/translate-a64.c | 48 ++++++------</div>\r\n" - "<div> target/arm/translate-a64.h | 2 +-</div>\r\n" - "<div> target/arm/translate-sve.c | 2 +-</div>\r\n" - "<div> target/arm/translate.c | 32 ++++----</div>\r\n" - "<div> target/arm/translate.h | 2 +-</div>\r\n" - "<div> target/hppa/translate.c | 14 ++--</div>\r\n" - "<div> target/i386/translate.c | 132 ++++++++++++++++----------------</div>\r\n" - "<div> target/m68k/translate.c | 2 +-</div>\r\n" - "<div> target/microblaze/translate.c | 4 +-</div>\r\n" - "<div> target/mips/op_helper.c | 5 +-</div>\r\n" - "<div> target/mips/translate.c | 8 +-</div>\r\n" - "<div> target/openrisc/translate.c | 4 +-</div>\r\n" - "<div> target/ppc/translate.c | 12 +--</div>\r\n" - "<div> target/riscv/insn_trans/trans_rva.inc.c | 8 +-</div>\r\n" - "<div> target/riscv/insn_trans/trans_rvi.inc.c | 4 +-</div>\r\n" - "<div> target/s390x/translate.c | 6 +-</div>\r\n" - "<div> target/s390x/translate_vx.inc.c | 10 +--</div>\r\n" - "<div> target/sparc/cpu.h | 2 +</div>\r\n" - "<div> target/sparc/mmu_helper.c | 40 ++++++----</div>\r\n" - "<div> target/sparc/translate.c | 14 ++--</div>\r\n" - "<div> target/tilegx/translate.c | 10 +--</div>\r\n" - "<div> target/tricore/translate.c | 8 +-</div>\r\n" - "<div> tcg/README | 2 +-</div>\r\n" - "<div> tcg/aarch64/tcg-target.inc.c | 26 +++----</div>\r\n" - "<div> tcg/arm/tcg-target.inc.c | 26 +++----</div>\r\n" - "<div> tcg/i386/tcg-target.inc.c | 24 +++---</div>\r\n" - "<div> tcg/mips/tcg-target.inc.c | 16 ++--</div>\r\n" - "<div> tcg/optimize.c | 2 +-</div>\r\n" - "<div> tcg/ppc/tcg-target.inc.c | 12 +--</div>\r\n" - "<div> tcg/riscv/tcg-target.inc.c | 20 ++---</div>\r\n" - "<div> tcg/s390/tcg-target.inc.c | 14 ++--</div>\r\n" - "<div> tcg/sparc/tcg-target.inc.c | 6 +-</div>\r\n" - "<div> tcg/tcg-op.c | 38 ++++-----</div>\r\n" - "<div> tcg/tcg-op.h | 86 ++++++++++-----------</div>\r\n" - "<div> tcg/tcg.c | 2 +-</div>\r\n" - "<div> tcg/tcg.h | 99 ++----------------------</div>\r\n" - "<div> trace/mem-internal.h | 4 +-</div>\r\n" - "<div> trace/mem.h | 4 +-</div>\r\n" - "<div> 51 files changed, 561 insertions(+), 488 deletions(-)</div>\r\n" - "<div> create mode 100644 include/exec/memop.h</div>\r\n" - "<div><br>\r\n" - "</div>\r\n" - "<div>-- </div>\r\n" - "<div>1.8.3.1</div>\r\n" - "<div><br>\r\n" - "<br>\r\n" - "</div>\r\n" - "<p><br>\r\n" - "</p>\r\n" - "</body>\r\n" - "</html>\r\n" -31a10d1c5c4f6762c7d074fd45f10379cccf0eeddda8796d58c772607ccf1fe4 +5f3d4d32b287e4b591ff8a1e19b3f72aae71777bc9b91b732e9b8b4902806875
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.