Re: [PATCH v2 1/5] x86/cpuid: add CPUID flag for Extended Destination ID support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.02.2022 10:24, Roger Pau Monné wrote:
> On Thu, Feb 17, 2022 at 09:52:51AM +0100, Jan Beulich wrote:
>> On 16.02.2022 17:08, David Woodhouse wrote:
>>> On Wed, 2022-02-16 at 16:43 +0100, Jan Beulich wrote:
>>>> On 16.02.2022 11:30, Roger Pau Monne wrote:
>>>>> --- a/xen/include/public/arch-x86/cpuid.h
>>>>> +++ b/xen/include/public/arch-x86/cpuid.h
>>>>> @@ -102,6 +102,12 @@
>>>>>  #define XEN_HVM_CPUID_IOMMU_MAPPINGS   (1u << 2)
>>>>>  #define XEN_HVM_CPUID_VCPU_ID_PRESENT  (1u << 3) /* vcpu id is present 
>>>>> in EBX */
>>>>>  #define XEN_HVM_CPUID_DOMID_PRESENT    (1u << 4) /* domid is present in 
>>>>> ECX */
>>>>> +/*
>>>>> + * Bits 55:49 from the IO-APIC RTE and bits 11:5 from the MSI address 
>>>>> can be
>>>>> + * used to store high bits for the Destination ID. This expands the 
>>>>> Destination
>>>>> + * ID field from 8 to 15 bits, allowing to target APIC IDs up 32768.
>>>>> + */
>>>>> +#define XEN_HVM_CPUID_EXT_DEST_ID      (1u << 5)
>>>>
>>>> Would the comment perhaps better include "in the absence of (guest
>>>> visible) interrupt remapping", since otherwise the layout / meaning
>>>> changes anyway? Apart from this I'd be fine with this going in
>>>> ahead of the rest of this series.
>>>
>>> No, this still works even if the guest has a vIOMMU with interrupt
>>> remapping. The Compatibility Format and Remappable Format MSI messages
>>> are distinct because the low bit of the Ext Dest ID is used to indicate
>>> Remappable Format.
>>
>> Well, yes, that was my point: With that bit set bits 55:49 / 11:5 change
>> meaning.
> 
> Bits 55:49/11:5 become reserved again with the interrupt format bit
> set to remappable.
> 
>> As an alternative to my initial proposal the comment could also
>> state that bit 48 / 4 needs to be clear for this feature to take effect.
> 
> I've always assumed that setting the IF to remappable invalidates
> extended destination ID, as the format of the interrupt is different
> then and there's no destination ID anymore, just a handle field. Maybe
> I could make it more explicit:
> 
> /*
>  * With interrupt format set to 0 (non-remappable) bits 55:49 from the
>  * IO-APIC RTE and bits 11:5 from the MSI address can be used to store
>  * high bits for the Destination ID. This expands the Destination ID
>  * field from 8 to 15 bits, allowing to target APIC IDs up 32768.
>  */

Yes, this disambiguates things enough to address my concern. Then
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
and I'd be fine making the adjustment while committing, if no other
concerns arise.

Jan