Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS

[Le Tan]

Hi Markus,

2014-10-20 19:41 GMT+08:00 Markus Armbruster <address@hidden>:
> "Michael S. Tsirkin" <address@hidden> writes:
>
>> (((sid) >> 8) && 0xff)  makes no sense
>> (((sid) >> 8) & 0xff) seems to be what was meant.
>>
>> Suggested-by: Markus Armbruster <address@hidden>
>
> Actually by the reporter of https://bugs.launchpad.net/bugs/1382477
>
>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>> ---
>>
>> Compile-tested only.
>>
>>  include/hw/i386/intel_iommu.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
>> index f4701e1..e321ee4 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -37,7 +37,7 @@
>>  #define VTD_PCI_DEVFN_MAX           256
>>  #define VTD_PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
>>  #define VTD_PCI_FUNC(devfn)         ((devfn) & 0x07)
>> -#define VTD_SID_TO_BUS(sid)         (((sid) >> 8) && 0xff)
>> +#define VTD_SID_TO_BUS(sid)         (((sid) >> 8) & 0xff)
>>  #define VTD_SID_TO_DEVFN(sid)       ((sid) & 0xff)
>>
>>  #define DMAR_REG_SIZE               0x230
>
> Can't find the spec right now, but it looks plausible enough.

Yes, this is a typo. I am sorry that I introduced such a mistake.
The spec is here in Section 3.4 :
http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/vt-directed-io-spec.html

VTD_SID_TO_BUS(sid) is intended to be used to get the bus id from the
source identifier.

Thanks very much!

Regards,
Le

> Only use is in vtd_context_device_invalidate().  Bug's impact isn't
> obvious to me.
>
> Reviewed-by: Markus Armbruster <address@hidden>