Re: [Qemu-devel] [PATCH] intel_iommu: a fix to vtd_dev_get_trans_type()

[Liu, Yi L]

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