All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel]  [PATCH 0/4] Invert Endian bit in SPARCv9 MMU TTE
@ 2019-07-17  5:57 tony.nguyen
  2019-07-17  6:04 ` [Qemu-devel] [PATCH 1/4] tcg: TCGMemOp is now accelerator independent MemOp tony.nguyen
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: tony.nguyen @ 2019-07-17  5:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, mark.cave-ayland, atar4qemu, rth

This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.

It is an attempt of the instructions outlined by Richard Henderson to Mark
Cave-Ayland.

Tested with OpenBSD on sun4u. Solaris 10 is my actual goal, but unfortunately a
separate keyboard issue remains in the way.

On 01/11/17 19:15, Mark Cave-Ayland wrote:

>On 15/08/17 19:10, Richard Henderson wrote:
>
>> [CC Peter re MemTxAttrs below]
>> 
>> On 08/15/2017 09:38 AM, Mark Cave-Ayland wrote:
>>> Working through an incorrect endian issue on qemu-system-sparc64, it has
>>> become apparent that at least one OS makes use of the IE (Invert Endian)
>>> bit in the SPARCv9 MMU TTE to map PCI memory space without the
>>> programmer having to manually endian-swap accesses.
>>>
>>> In other words, to quote the UltraSPARC specification: "if this bit is
>>> set, accesses to the associated page are processed with inverse
>>> endianness from what is specified by the instruction (big-for-little and
>>> little-for-big)".

A good explanation by Mark why the IE bit is required.

>>>
>>> Looking through various bits of code, I'm trying to get a feel for the
>>> best way to implement this in an efficient manner. From what I can see
>>> this could be solved using an additional MMU index, however I'm not
>>> overly familiar with the memory and softmmu subsystems.
>> 
>> No, it can't be solved with an MMU index.
>> 
>>> Can anyone point me in the right direction as to what would be the best
>>> way to implement this feature within QEMU?
>> 
>> It's definitely tricky.
>> 
>> We definitely need some TLB_FLAGS_MASK bit set so that we're forced through 
>> the
>> memory slow path.  There is no other way to bypass the endianness that we've
>> already encoded from the target instruction.
>> 
>> Given the tlb_set_page_with_attrs interface, I would think that we need a new
>> bit in MemTxAttrs, so that the target/sparc tlb_fill (and subroutines) can 
>> pass
>> along the TTE bit for the given page.
>> 
>> We have an existing problem in softmmu_template.h,
>> 
>>     /* ??? Note that the io helpers always read data in the target
>>        byte ordering.  We should push the LE/BE request down into io.  */
>>     res = glue(io_read, SUFFIX)(env, mmu_idx, index, addr, retaddr);
>>     res = TGT_BE(res);
>> 
>> We do not want to add a third(!) byte swap along the i/o path.  We need to
>> collapse the two that we have already before considering this one.
>> 
>> This probably takes the form of:
>> 
>> (1) Replacing the "int size" argument with "TCGMemOp memop" for
>>       a) io_{read,write}x in accel/tcg/cputlb.c,
>>       b) memory_region_dispatch_{read,write} in memory.c,
>>       c) adjust_endianness in memory.c.
>>     This carries size+sign+endianness down to the next level.
>> 
>> (2) In memory.c, adjust_endianness,
>> 
>>      if (memory_region_wrong_endianness(mr)) {
>> -        switch (size) {
>> +        memop ^= MO_BSWAP;
>> +    }
>> +    if (memop & MO_BSWAP) {
>> 
>>     For extra credit, re-arrange memory_region_wrong_endianness
>>     to something more explicit -- "wrong" isn't helpful.
>
>Finally I've had a bit of spare time to experiment with this approach,
>and from what I can see there are currently 2 issues:
>
>
>1) Using TCGMemOp in memory.c means it is no longer accelerator agnostic
>
>For the moment I've defined a separate MemOp in memory.h and provided a
>mapping function in io_{read,write}x to map from TCGMemOp to MemOp and
>then pass that into memory_region_dispatch_{read,write}.
>
>Other than not referencing TCGMemOp in the memory API, another reason
>for doing this was that I wasn't convinced that all the MO_ attributes
>were valid outside of TCG. I do, of course, strongly defer to other
>people's knowledge in this area though.

Patch 1/4 turns TCGMemOp into accelerator agnostic MemOp.

I am mindful it is a far reaching change, and like Mark, will be grateful of
any alternative suggestions =)

>
>
>2) The above changes to adjust_endianness() fail when
>memory_region_dispatch_{read,write} are called recursively
>
>Whilst booting qemu-system-sparc64 I see that
>memory_region_dispatch_{read,write} get called recursively - once via
>io_{read,write}x and then again via flatview_read_continue() in exec.c.
>
>The net effect of this is that we perform the bswap correctly at the
>tail of the recursion, but then as we travel back up the stack we hit
>memory_region_dispatch_{read,write} once again causing a second bswap
>which means the value is returned with the incorrect endian again.
>
>
>My understanding from your softmmu_template.h comment above is that the
>memory API should do the endian swapping internally allowing the removal
>of the final TGT_BE/TGT_LE applied to the result, or did I get this wrong?
>
>> (3) In tlb_set_page_with_attrs, notice attrs.byte_swap and set
>>     a new TLB_FORCE_SLOW bit within TLB_FLAGS_MASK.
>> 
>> (4) In io_{read,write}x, if iotlbentry->attrs.byte_swap is set,
>>     then memop ^= MO_BSWAP.

Tony Nguyen.

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-07-21 19:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-17  5:57 [Qemu-devel] [PATCH 0/4] Invert Endian bit in SPARCv9 MMU TTE tony.nguyen
2019-07-17  6:04 ` [Qemu-devel] [PATCH 1/4] tcg: TCGMemOp is now accelerator independent MemOp tony.nguyen
2019-07-17 14:04   ` Richard Henderson
2019-07-17  6:06 ` [Qemu-devel] [PATCH 2/4] memory: Single byte swap along the I/O path tony.nguyen
2019-07-17 10:08   ` Paolo Bonzini
2019-07-17 14:24   ` Richard Henderson
2019-07-17  6:08 ` [Qemu-devel] [PATCH 3/4] cputlb: Byte swap memory transaction attribute tony.nguyen
2019-07-17 14:29   ` Richard Henderson
2019-07-17  6:10 ` [Qemu-devel] [PATCH 4/4] target/sparc: sun4u Invert Endian TTE bit tony.nguyen
2019-07-21 19:50   ` Mark Cave-Ayland
2019-07-17 10:09 ` [Qemu-devel] [PATCH 0/4] Invert Endian bit in SPARCv9 MMU TTE Paolo Bonzini

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.