Re: [Qemu-devel] [Xen-devel] [RFC PATCH V2 1/2] xen-pt: bind/unbind interrupt remapping format MSI

2017-05-23 Thread Lan Tianyu 
On 2017年05月24日 01:06, Anthony PERARD wrote:
> On Tue, May 23, 2017 at 08:16:25PM +0800, Lan Tianyu wrote:
>> On 2017年05月19日 20:04, Jan Beulich wrote:
>> On 19.05.17 at 13:16,  wrote:
 On Thu, May 18, 2017 at 01:32:59AM -0400, Lan Tianyu wrote:
> --- a/include/hw/i386/apic-msidef.h
> +++ b/include/hw/i386/apic-msidef.h
> @@ -26,6 +26,7 @@
>  
>  #define MSI_ADDR_DEST_ID_SHIFT  12
>  #define MSI_ADDR_DEST_IDX_SHIFT 4
> -#define  MSI_ADDR_DEST_ID_MASK  0x000
> +#define  MSI_ADDR_DEST_ID_MASK  0x000fff00
 The value of MSI_ADDR_DEST_ID_MASK is changed here. I think the patch
 should be:
 +#define  MSI_ADDR_DEST_ID_MASK  0x0000
>>> Judging from other sources, rather the other way around - the
>>> mask needs to have further bits removed (should be 0x000ff000
>>> afaict). Xen sources confirm this, and while Linux has the value
>>> you suggest, that contradicts
>> Agree. Defining the mask as "0x000ff000" makes more sense.
>> Just check Qemu source code. Only apic_send_msi() and msi_dest_id() use
>> the mask
>> to get dest apic id. They mask MSI address field with 
>> MSI_ADDR_DEST_ID_MASK and
>> then right-shift 12bit. The low 12bit won't be used.
>>
>> Anthony, does this make sense?
> Yes, it does.
> The change to MSI_ADDR_DEST_ID_MASK should probably go in its own patch.
>
OK. Will update.


-- 
Best regards
Tianyu Lan

Re: [Qemu-devel] [Xen-devel] [RFC PATCH V2 1/2] xen-pt: bind/unbind interrupt remapping format MSI

2017-05-23 Thread Anthony PERARD 
On Tue, May 23, 2017 at 08:16:25PM +0800, Lan Tianyu wrote:
> On 2017年05月19日 20:04, Jan Beulich wrote:
>  On 19.05.17 at 13:16,  wrote:
> >> On Thu, May 18, 2017 at 01:32:59AM -0400, Lan Tianyu wrote:
> >>> --- a/include/hw/i386/apic-msidef.h
> >>> +++ b/include/hw/i386/apic-msidef.h
> >>> @@ -26,6 +26,7 @@
> >>>  
> >>>  #define MSI_ADDR_DEST_ID_SHIFT  12
> >>>  #define MSI_ADDR_DEST_IDX_SHIFT 4
> >>> -#define  MSI_ADDR_DEST_ID_MASK  0x000
> >>> +#define  MSI_ADDR_DEST_ID_MASK  0x000fff00
> >> The value of MSI_ADDR_DEST_ID_MASK is changed here. I think the patch
> >> should be:
> >> +#define  MSI_ADDR_DEST_ID_MASK  0x0000
> > Judging from other sources, rather the other way around - the
> > mask needs to have further bits removed (should be 0x000ff000
> > afaict). Xen sources confirm this, and while Linux has the value
> > you suggest, that contradicts
> Agree. Defining the mask as "0x000ff000" makes more sense.
> Just check Qemu source code. Only apic_send_msi() and msi_dest_id() use
> the mask
> to get dest apic id. They mask MSI address field with 
> MSI_ADDR_DEST_ID_MASK and
> then right-shift 12bit. The low 12bit won't be used.
> 
> Anthony, does this make sense?

Yes, it does.
The change to MSI_ADDR_DEST_ID_MASK should probably go in its own patch.

-- 
Anthony PERARD

Re: [Qemu-devel] [Xen-devel] [RFC PATCH V2 1/2] xen-pt: bind/unbind interrupt remapping format MSI

2017-05-23 Thread Lan Tianyu 
On 2017年05月19日 20:04, Jan Beulich wrote:
 On 19.05.17 at 13:16,  wrote:
>> On Thu, May 18, 2017 at 01:32:59AM -0400, Lan Tianyu wrote:
>>> --- a/include/hw/i386/apic-msidef.h
>>> +++ b/include/hw/i386/apic-msidef.h
>>> @@ -26,6 +26,7 @@
>>>  
>>>  #define MSI_ADDR_DEST_ID_SHIFT  12
>>>  #define MSI_ADDR_DEST_IDX_SHIFT 4
>>> -#define  MSI_ADDR_DEST_ID_MASK  0x000
>>> +#define  MSI_ADDR_DEST_ID_MASK  0x000fff00
>> The value of MSI_ADDR_DEST_ID_MASK is changed here. I think the patch
>> should be:
>> +#define  MSI_ADDR_DEST_ID_MASK  0x0000
> Judging from other sources, rather the other way around - the
> mask needs to have further bits removed (should be 0x000ff000
> afaict). Xen sources confirm this, and while Linux has the value
> you suggest, that contradicts
Agree. Defining the mask as "0x000ff000" makes more sense.
Just check Qemu source code. Only apic_send_msi() and msi_dest_id() use
the mask
to get dest apic id. They mask MSI address field with 
MSI_ADDR_DEST_ID_MASK and
then right-shift 12bit. The low 12bit won't be used.

Anthony, does this make sense?

-- 
Best regards
Tianyu Lan

Re: [Qemu-devel] [Xen-devel] [RFC PATCH V2 1/2] xen-pt: bind/unbind interrupt remapping format MSI

2017-05-19 Thread Jan Beulich 
>>> On 19.05.17 at 13:16,  wrote:
> On Thu, May 18, 2017 at 01:32:59AM -0400, Lan Tianyu wrote:
>> --- a/include/hw/i386/apic-msidef.h
>> +++ b/include/hw/i386/apic-msidef.h
>> @@ -26,6 +26,7 @@
>>  
>>  #define MSI_ADDR_DEST_ID_SHIFT  12
>>  #define MSI_ADDR_DEST_IDX_SHIFT 4
>> -#define  MSI_ADDR_DEST_ID_MASK  0x000
>> +#define  MSI_ADDR_DEST_ID_MASK  0x000fff00
> 
> The value of MSI_ADDR_DEST_ID_MASK is changed here. I think the patch
> should be:
> +#define  MSI_ADDR_DEST_ID_MASK  0x0000

Judging from other sources, rather the other way around - the
mask needs to have further bits removed (should be 0x000ff000
afaict). Xen sources confirm this, and while Linux has the value
you suggest, that contradicts

#define MSI_ADDR_DEST_ID_SHIFT  12
#define  MSI_ADDR_DEST_ID(dest) (((dest) << MSI_ADDR_DEST_ID_SHIFT) & \
 MSI_ADDR_DEST_ID_MASK)

as well as

#define MSI_ADDR_EXT_DEST_ID(dest)  ((dest) & 0xff00)

chopping off just the low 8 bits.

Jan