On Mon, Jul 11, 2016 at 10:27 PM, David Gibson <da...@gibson.dropbear.id.au> wrote: > On Mon, Jul 11, 2016 at 05:27:50PM +0100, Peter Maydell wrote: >> On 11 July 2016 at 03:24, David Gibson <da...@gibson.dropbear.id.au> wrote: >> > On Sun, Jul 10, 2016 at 08:32:32PM +0100, Peter Maydell wrote: >> >> On 8 July 2016 at 04:42, David Gibson <da...@gibson.dropbear.id.au> wrote: >> >> > My only concern here is that the constants are named >> >> > *MMU*_DATA_... whereas these are physical memory accesses not >> >> > involving the MMU. I can't actually see any current users of >> >> > MMUAccessType which makes me a bit confused as to what it's intended >> >> > meaning was >> >> >> >> If you grep for MMU_DATA_LOAD/MMU_DATA_STORE/MMU_INST_FETCH >> >> you'll see the uses. A lot of the softmmu code uses the >> >> convention of 0=read,1=write,2=insn (which developed I >> >> think historically from a bool "is_write", which you'll >> >> still see in some function argument names, that was >> >> augmented to handle insn-fetch separately). The enum >> >> gives us some symbolic names for the constant values. >> >> (There's a proposed patch somewhere to change the >> >> 'int is_write' arguments to actually use the enum type.) >> > >> > Ah, yes, I see. Still surprisingly few, actually. >> >> Yeah, we didn't go through (yet) and update the legacy code, >> just provided the common type so new code could use it. > > Ah, yes, I see. > >> > My concern about the potentially misleading name still stands. >> >> I don't mind if we want to rename it, but I don't think we want >> to have two types. This is all in the softmmu code, whether it's >> in the physical-address parts or the virtual-address parts. > > Right, I agree we shouldn't have two types. > > I think we should rename the existing constants, though, since it > doesn't really have anything to do with the MMU. >
I agree with your concern about confusing naming, David, and think those should be renamed as well. In the original version of this patch I introduced my own enumeration which was duplication MMUAccessType, so in his feedback Peter asked me to use MMUAccessType instead. Please let me know how I should proceed with this series. Thanks, Andrey
