On 22/07/2019 09:43, Jan Beulich wrote:
> On 19.07.2019 19:31, Andrew Cooper wrote:
>> On 16/07/2019 17:39, Jan Beulich wrote:
>>> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
>>> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
>>> @@ -416,6 +416,25 @@ union amd_iommu_ext_features {
>>> } flds;
>>> };
>>>
>>> +/* x2APIC Control Registers */
>>> +#define IOMMU_XT_INT_CTRL_MMIO_OFFSET 0x0170
>>> +#define IOMMU_XT_PPR_INT_CTRL_MMIO_OFFSET 0x0178
>>> +#define IOMMU_XT_GA_INT_CTRL_MMIO_OFFSET 0x0180
>>> +
>>> +union amd_iommu_x2apic_control {
>>> + uint64_t raw;
>>> + struct {
>>> + unsigned int :2;
>>> + unsigned int dest_mode:1;
>>> + unsigned int :5;
>>> + unsigned int dest_lo:24;
>>> + unsigned int vector:8;
>>> + unsigned int int_type:1; /* DM in IOMMU spec 3.04 */
>>> + unsigned int :15;
>>> + unsigned int dest_hi:8;
>> Bool bitfields like you've done elsewhere in v3?
> I'd been considering this, but decided against because of ...
>
> +static void set_x2apic_affinity(struct irq_desc *desc, const cpumask_t *mask)
> +{
> + struct amd_iommu *iommu = desc->action->dev_id;
> + unsigned int dest = set_desc_affinity(desc, mask);
> + union amd_iommu_x2apic_control ctrl = {};
> + unsigned long flags;
> +
> + if ( dest == BAD_APICID )
> + return;
> +
> + msi_compose_msg(desc->arch.vector, NULL, &iommu->msi.msg);
> + iommu->msi.msg.dest32 = dest;
> +
> + ctrl.dest_mode = MASK_EXTR(iommu->msi.msg.address_lo,
> + MSI_ADDR_DESTMODE_MASK);
> + ctrl.int_type = MASK_EXTR(iommu->msi.msg.data,
> + MSI_DATA_DELIVERY_MODE_MASK);
>
> ... this: We really mean a value copy here, not an "is zero" or
> "is non-zero" one. I also think that both fields are not suitably
> named for being boolean. In the recent re-work of struct
> IO_APIC_route_entry (ca9310b24e) similar fields similarly were
> left as "unsigned int". MSI's struct msg_data also falls into the
> same category. I think if we wanted to switch to bool here, we
> should do so everywhere at the same time (along with suitably
> renaming fields).
Architecturally, both of these are single-bit fields, no?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
