| Message ID | 20220216103026.11533-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: extended destination ID support | expand |
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. Jan
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. If you wanted to add "… without having to use interrupt remapping in the guest" to the end of the comment then I suppose you could, but I don't think it adds much.
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. 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. Jan
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.
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@suse.com> and I'd be fine making the adjustment while committing, if no other concerns arise. Jan
> /* > * 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. > */ I am not keen on that wording because it doesn't seem to fully reflect the fact that the I/OAPIC is just a device to turn line interrupts into MSIs. The values in bits 55:49 of the RTE *are* what go into bits 11:5 of the resulting MSI address. Perhaps make it more parenthetical to make it clearer that they are not independent... "bits 11:5 of the MSI address (which come from bits 55:49 of the I/OAPIC RTE)..."
On Sat, Feb 19, 2022 at 11:24:25AM -0000, David Woodhouse wrote: > > > > /* > > * 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. > > */ > > I am not keen on that wording because it doesn't seem to fully reflect the > fact that the I/OAPIC is just a device to turn line interrupts into MSIs. But that's an architecture implementation detail, I'm not sure I've seen this written down explicitly in any specification. > The values in bits 55:49 of the RTE *are* what go into bits 11:5 of the > resulting MSI address. Perhaps make it more parenthetical to make it > clearer that they are not independent... "bits 11:5 of the MSI address > (which come from bits 55:49 of the I/OAPIC RTE)..." That could be an option also, as long as it's clear to which bits of the IO-APIC RTE this affects. Thanks, Roger.
diff --git a/xen/include/public/arch-x86/cpuid.h b/xen/include/public/arch-x86/cpuid.h index ce46305bee..49bcc93b6b 100644 --- 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) /* * Leaf 6 (0x40000x05)
Introduce the CPUID flag to be used in order to signal the support for using an extended destination ID in IO-APIC RTEs and MSI address fields. Such format expands the maximum target APIC ID from 255 to 32768 without requiring the usage of interrupt remapping. The design document describing the feature can be found at: http://david.woodhou.se/15-bit-msi.pdf Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - New in this version. --- xen/include/public/arch-x86/cpuid.h | 6 ++++++ 1 file changed, 6 insertions(+)