On Sun, 14 Jun 2026 at 20:09, Michael S. Tsirkin <[email protected]> wrote:
>
> From: Sairaj Kodilkar <[email protected]>
>
> When MMIO 0x18[IntCapXTEn]=1, interrupts originating from the IOMMU itself are
> sent based on the programming in XT IOMMU Interrupt Control Registers in MMIO
> 0x170-0x180 instead of the programming in the IOMMU's MSI capability
> registers.
> The guest programs these registers with appropriate vector and destination
> ID instead of writing to PCI MSI capability.
>
> Current AMD vIOMMU is capable of generating interrupts only through PCI
> MSI capability and does not care about xt mode. Because of this AMD
> vIOMMU cannot generate event log interrupts when the guest has enabled
> xt mode.
>
> Introduce a new flag "intcapxten" which is set when guest writes control
> register [IntCapXTEn] (bit 51) and use vector and destination field in
> the XT MMIO register (0x170) to support XT mode.
Hi; Coverity points out a use of an uninitialized struct field here
(CID 1660056):
> +static void amdvi_build_xt_msi_msg(AMDVIState *s, MSIMessage *msg)
> +{
> + union mmio_xt_intr xt_reg;
> + struct X86IOMMUIrq irq;
Here we declare irq, without a struct initializer...
> +
> + xt_reg.val = amdvi_readq(s, AMDVI_MMIO_XT_GEN_INTR);
> +
> + irq.vector = xt_reg.vector;
> + irq.delivery_mode = xt_reg.delivery_mode;
> + irq.dest_mode = xt_reg.destination_mode;
> + irq.dest = (xt_reg.destination_hi << 24) | xt_reg.destination_lo;
> + irq.trigger_mode = 0;
> + irq.redir_hint = 0;
...and here we fill in some but not all of its fields...
> +
> + x86_iommu_irq_to_msi_message(&irq, msg);
...and here we call a function which (among other things) does this:
msg.__not_used = irq->msi_addr_last_bits;
which will read the uninitialized msi_addr_last_bits field.
Bonus extra bug report:
+union mmio_xt_intr {
+ uint64_t val;
+ struct {
+ uint64_t rsvd_1:2,
+ destination_mode:1,
+ rsvd_2:5,
+ destination_lo:24,
+ vector:8,
+ delivery_mode:1,
+ rsvd_3:15,
+ destination_hi:8;
+ };
+};
Please don't use bitfields like this -- this is not portable to big
endian hosts. Use the extract64() function instead, or the FIELD macros.
I suggest something like (untested):
(at top level in a source file or header; the numbers are
start-bit and length, check these as I may have made typos)
FIELD(XT_GEN_INTR, DEST_MODE, 2, 1)
FIELD(XT_GEN_INTR, DEST_LO, 24, 8)
FIELD(XT_GEN_INTR, VECTOR, 32, 8)
FIELD(XT_GEN_INTR, DELIVERY_MODE, 33, 1)
FIELD(XT_GEN_INTR, DEST_HI, 49, 8)
and then in the function you can do:
uint64_t xtreg = amdvi_readq(s, AMDVI_MMIO_XT_GEN_INTR);
X86IOMMUIrq irq = {
.vector = FIELD_EX64(xtreg, XT_GEN_INTR, VECTOR),
.delivery_mode = FIELD_EX64(xtreg, XT_GEN_INTR, DELIVERY_MODE),
.dest_mode = FIELD_EX64(xtreg, XT_GEN_INTR, DEST_MODE),
.dest = (FIELD_X64(xtreg, XT_GEN_INTR, DEST_HI) << 24) |
FIELD_EX64(xtreg, XT_GEN_INTR, DEST_LO),
};
which gives you a struct initializer and you can rely on the
whole struct being initialized.
> +}
thanks
-- PMM
