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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.
 */

Thanks, Roger.