I my original proposal actually contained getGE()/setGE() that use the guest endianness. I decided to scrap them since we don¹t really have any devices using them. I can add them back into the patch I¹m intending to post tomorrow.
Another option for devices that want to use the same endianness as the guest would be to query the systems endianness and store that in a constant that¹s passed to get(ByteOrder). I would prefer this since it doesn¹t clutter the API. As for deprecating get()/set(), I think we still want to do that in order to catch endianness bugs. Their endianness was always confusing and we should weed out any use of them in our internal code base. Code that isn¹t in the main repo ought to be reviewed as well and if using guest endianness is the desired mode of operation, that should be explicit. //Andreas On 04/07/2015 19:14, "Steve Reinhardt" <[email protected]> wrote: >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 -- 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
