I think there's a distinction here between device models that are trying to emulate real-world devices and experimental device models. In the latter case, it makes sense to use get() and set() as the are written, because you just want the simulated system to send you e.g. a 32-bit value that you can stick in a uint32_t and not have to worry about it. Chances are you're only using this with an ISA of one endianness, but if the device magically switches its apparent endianness when you go to use an ISA of the opposite endianness, then that's actually a good thing, because it keeps your software simpler.
I can see where the last three categories would be valuable if you're modeling a specific real-world device that has a specific endianness, so I'm fine with adding them. I'm not sure what the use case is for getRaw() and setRaw() though. At least with the current get() and set() you're converting values into a known endianness, but these methods (as I understand them) would leave the device needing to know what the guest endianness is (which I argue it shouldn't), or else explicitly use gtoh/htog, in which case why not just keep what we already have? If you're just looking to do a byte-level transfer, you should use getPtr() and do a memcpy instead. So in summary my counter-proposal is: - Leave get()/set() alone, don't deprecate them - Add getLE()/setLE(), getBE()/setBE(), and get(ByteOrder)/set(ByteOrder) as proposed - Forget getRaw()/setRaw() unless there's a use case I'm not thinking of Steve On Fri, Jul 3, 2015 at 7:08 AM Andreas Sandberg <[email protected]> wrote: > Fellow Developers, > > There have been more than a few cases when I have been working on memory > mapped devices that I have been a bit confused/annoyed by the semantics > of Packet::get() and Packet::set(). Currently, these two methods (which > are used pretty much everywhere we implement devices) assume that > devices are always of the same endianness as the host, i.e., they > perform a gtoh or htog endian conversion. This makes it very hard to > work with devices that are require a specific endianness. > > I suspect the only reason things work at the moment is that we don't > really support anything other than little endian on little endian in > full system mode at the moment. > > I'd like to change the get/set API to make endianness explicit. > Specifically, I'd like to add the following methods to Packet: > * getRaw() / setRaw() - Get data without conversion. > * getLE() / setLE() - Get data stored as little endian. > * getBE() / setBE() - Get data stored as big endian. > * get(ByteOrder) / set(value, ByteOrder) - For devices with > configurable byte order > > The plan is to keep the old get & set methods but to flag them as > deprecated once all devices have been transitioned to the new interface. > Flagging the old interface as deprecated means that we'll be able to > slowly transition old code without breaking anything. However, old code > would still trigger non-fatal compiler warnings. > > Any thoughts on this? > > Thanks, > Andreas > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2557590 > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2548782 > > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
