On Mon, Dec 25, 2017 at 01:45:22PM +0800, Peter Xu wrote: > On Thu, Dec 21, 2017 at 02:15:19PM +0800, Liu, Yi L wrote: > > vtd_ce_get_type() returns uin32_t and vtd_dev_get_trans_type() returns > > the value from vtd_ce_get_type(). However, vtd_dev_get_trans_type() > > returns int. This patch switchs to return the translation type by > > parameter. It avoids unsigned to int transfer and also avoid potential > > reading confusion. > > Frankly speaking I would still prefer the old way to do it: return > type when >=0 and error when <0. After all we have a comment for > vtd_dev_get_trans_type() already: > > /* > * Fetch translation type for specific device. Returns <0 if error > * happens, otherwise return the shifted type to check against > * VTD_CONTEXT_TT_*. > */
Peter, I knew your point. It's not a bug. However, it depends on vtd_ce_get_type(), if it returns a value with the most significant bit=1, then it may be wrongly treated as a negative value. It is not possible so far, but it may be an issue if future spec place the translation type in bit 31:29, and type 3'b100 is valid. It's an assumption. Then the return value of vtd_ce_get_type() is 0x80000000, and it would be treated as -2147483648. This should be a mistake. That's why I want to separate the return value and the translation type. Thanks, Yi L