[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/3] x86/vmsi: add support for extended destination ID in address field
On 20.01.2022 16:23, Roger Pau Monne wrote: > Both QEMU/KVM and HyperV support using bits 11:5 from the MSI address > field in order to store the high part of the target APIC ID. This > allows expanding the maximum APID ID usable without interrupt > remapping support from 255 to 32768. > > Note the interface used by QEMU for emulated devices (via the > XEN_DMOP_inject_msi hypercall) already passes both the address and > data fields into Xen for processing, so there's no need for any change > to QEMU there. > > However for PCI passthrough devices QEMU uses the > XEN_DOMCTL_bind_pt_irq hypercall which does need an addition to the > gflags field in order to pass the high bits of the APIC destination > ID. > > Introduce a new CPUID flag to signal the support for the feature. The > introduced flag covers both the support for extended ID for the > IO-APIC RTE and the MSI address registers. Such flag is currently only > exposed when the domain is using vPCI (ie: a PVH dom0). Because of also covering the IO-APIC side, I think the CPUID aspect of this really wants splitting into a 3rd patch. That way the MSI and IO-APIC parts could in principle go in independently, and only the CPUID one needs to remain at the tail. > --- a/xen/arch/x86/hvm/vmsi.c > +++ b/xen/arch/x86/hvm/vmsi.c > @@ -66,7 +66,7 @@ static void vmsi_inj_irq( > > int vmsi_deliver( > struct domain *d, int vector, > - uint8_t dest, uint8_t dest_mode, > + unsigned int dest, unsigned int dest_mode, If you change the type of dest_mode, then to "bool" please - see its only call site. > @@ -123,7 +125,8 @@ void vmsi_deliver_pirq(struct domain *d, const struct > hvm_pirq_dpci *pirq_dpci) > } > > /* Return value, -1 : multi-dests, non-negative value: dest_vcpu_id */ > -int hvm_girq_dest_2_vcpu_id(struct domain *d, uint8_t dest, uint8_t > dest_mode) > +int hvm_girq_dest_2_vcpu_id(struct domain *d, unsigned int dest, > + unsigned int dest_mode) Same here then. > --- a/xen/arch/x86/include/asm/msi.h > +++ b/xen/arch/x86/include/asm/msi.h > @@ -54,6 +54,7 @@ > #define MSI_ADDR_DEST_ID_SHIFT 12 > #define MSI_ADDR_DEST_ID_MASK 0x00ff000 > #define MSI_ADDR_DEST_ID(dest) (((dest) << > MSI_ADDR_DEST_ID_SHIFT) & MSI_ADDR_DEST_ID_MASK) > +#define MSI_ADDR_EXT_DEST_ID_MASK 0x0000fe0 Especially the immediately preceding macro now becomes kind of stale. > --- a/xen/drivers/passthrough/x86/hvm.c > +++ b/xen/drivers/passthrough/x86/hvm.c > @@ -269,7 +269,7 @@ int pt_irq_create_bind( > { > case PT_IRQ_TYPE_MSI: > { > - uint8_t dest, delivery_mode; > + unsigned int dest, delivery_mode; > bool dest_mode; If you touch delivery_mode's type, wouldn't that better become bool? > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -588,6 +588,7 @@ struct xen_domctl_bind_pt_irq { > #define XEN_DOMCTL_VMSI_X86_DELIV_MASK 0x007000 > #define XEN_DOMCTL_VMSI_X86_TRIG_MASK 0x008000 > #define XEN_DOMCTL_VMSI_X86_UNMASKED 0x010000 > +#define XEN_DOMCTL_VMSI_X86_EXT_DEST_ID_MASK 0xfe0000 I'm not convinced it is a good idea to limit the overall destination ID width to 15 bits here - at the interface level we could as well permit more bits right away; the implementation would reject too high a value, of course. Not only with this I further wonder whether the field shouldn't be unsplit while extending it. You won't get away without bumping XEN_DOMCTL_INTERFACE_VERSION anyway (unless it was bumped already for 4.17) since afaics the unused bits of this field previously weren't checked for being zero. We could easily have 8 bits vector, 16 bits flags, and 32 bits destination ID in the struct. And there would then still be 8 unused bits (which from now on we ought to check for being zero). Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |