On 15.01.24 11:35, Jan Beulich wrote:
> On 14.01.2024 11:01, Mykyta Poturai wrote:
>> --- a/xen/include/public/hvm/dm_op.h
>> +++ b/xen/include/public/hvm/dm_op.h
>> @@ -444,6 +444,17 @@ struct xen_dm_op_nr_vcpus {
>> };
>> typedef struct xen_dm_op_nr_vcpus xen_dm_op_nr_vcpus_t;
>>
>> +#define XEN_DMOP_inject_msi2 21
>> +#define XEN_DMOP_MSI_SOURCE_ID_VALID (1u << 0)
>> +
>> +struct xen_dm_op_inject_msi2 {
>> + uint64_aligned_t addr;
>> + uint32_t data;
>> + uint32_t source_id; /* PCI SBDF */
>
> Since the comment says SBDF (not BDF), how are multiple segments handled
> here? At least on x86 (VT-d and V-i) source IDs are only 16 bits iirc,
> and are local to the respective segment. It would feel wrong to use a
> 32-bit quantity there; IOW I'd rather see this as being two 16-bit fields
> (segment and source_id).
>
Hi Jan,
I'm planning on resuming this series in the near future and want to
clarify the DM op interface.
Wouldn't it be better to keep things consistent and use
XEN_DMOP_PCI_SBDF as it's done in xen_dm_op_ioreq_server_range? Also
with this, the value can be easily casted to pci_sbdf_t later and split
to segment and BDF if needed.
>> + uint32_t flags;
>> +};
>
> Just like in struct xen_dm_op_inject_msi padding wants making explicit,
> and then wants checking for being zero.
>
> Jan
Will do.
--
Mykyta
