One minor nit that I mentioned on v1: On 1/29/26 5:28 AM, Sairaj Kodilkar wrote: > Current code uses 32 bit cpu destination irrespective of the fact that
s/"32 bit cpu destination"/"32-bit destination ID" > guest has enabled x2APIC support through control register[XTEn] and > completely depends on command line parameter xtsup=on. This is not a > correct hardware behaviour and can cause problems in the guest which has > not enabled XT mode. > > Introduce new flag "xten", which is enabled when guest writes 1 to the > control register bit 50 (XTEn). Also, add a new subsection in > `VMStateDescription` for backward compatibility during vm migration. > > Signed-off-by: Sairaj Kodilkar <[email protected]> > Reviewed-by: Vasant Hegde <[email protected]> Reviewed-by: Alejandro Jimenez <[email protected]> > --- > hw/i386/amd_iommu.c | 21 +++++++++++++++++++-- > hw/i386/amd_iommu.h | 4 +++- > 2 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c > index 62175cc366ac..850d3920a76d 100644 > --- a/hw/i386/amd_iommu.c > +++ b/hw/i386/amd_iommu.c > @@ -1535,6 +1535,8 @@ static void amdvi_handle_control_write(AMDVIState *s) > s->cmdbuf_enabled = s->enabled && !!(control & > AMDVI_MMIO_CONTROL_CMDBUFLEN); > s->ga_enabled = !!(control & AMDVI_MMIO_CONTROL_GAEN); > + s->xten = !!(control & AMDVI_MMIO_CONTROL_XTEN) && s->xtsup && > + s->ga_enabled; > > /* update the flags depending on the control register */ > if (s->cmdbuf_enabled) { > @@ -2007,7 +2009,7 @@ static int amdvi_int_remap_ga(AMDVIState *iommu, > irq->vector = irte.hi.fields.vector; > irq->dest_mode = irte.lo.fields_remap.dm; > irq->redir_hint = irte.lo.fields_remap.rq_eoi; > - if (iommu->xtsup) { > + if (iommu->xten) { > irq->dest = irte.lo.fields_remap.destination | > (irte.hi.fields.destination_hi << 24); > } else { > @@ -2390,6 +2392,7 @@ static void amdvi_init(AMDVIState *s) > s->mmio_enabled = false; > s->enabled = false; > s->cmdbuf_enabled = false; > + s->xten = false; > > /* reset MMIO */ > memset(s->mmior, 0, AMDVI_MMIO_SIZE); > @@ -2454,6 +2457,16 @@ static void amdvi_sysbus_reset(DeviceState *dev) > amdvi_reset_address_translation_all(s); > } > > +static const VMStateDescription vmstate_xt = { > + .name = "amd-iommu-xt", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_BOOL(xten, AMDVIState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const VMStateDescription vmstate_amdvi_sysbus_migratable = { > .name = "amd-iommu", > .version_id = 1, > @@ -2498,7 +2511,11 @@ static const VMStateDescription > vmstate_amdvi_sysbus_migratable = { > VMSTATE_UINT8_ARRAY(romask, AMDVIState, AMDVI_MMIO_SIZE), > VMSTATE_UINT8_ARRAY(w1cmask, AMDVIState, AMDVI_MMIO_SIZE), > VMSTATE_END_OF_LIST() > - } > + }, > + .subsections = (const VMStateDescription *const []) { > + &vmstate_xt, > + NULL > + } > }; > > static void amdvi_sysbus_realize(DeviceState *dev, Error **errp) > diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h > index 302ccca5121f..e9401f3a5c27 100644 > --- a/hw/i386/amd_iommu.h > +++ b/hw/i386/amd_iommu.h > @@ -106,6 +106,7 @@ > #define AMDVI_MMIO_CONTROL_COMWAITINTEN (1ULL << 4) > #define AMDVI_MMIO_CONTROL_CMDBUFLEN (1ULL << 12) > #define AMDVI_MMIO_CONTROL_GAEN (1ULL << 17) > +#define AMDVI_MMIO_CONTROL_XTEN (1ULL << 50) > > /* MMIO status register bits */ > #define AMDVI_MMIO_STATUS_CMDBUF_RUN (1 << 4) > @@ -418,7 +419,8 @@ struct AMDVIState { > > /* Interrupt remapping */ > bool ga_enabled; > - bool xtsup; > + bool xtsup; /* xtsup=on command line */ > + bool xten; /* guest controlled, x2apic mode enabled */ > > /* DMA address translation */ > bool dma_remap;
