Re: [Xen-devel] [PATCH] AMD IOMMU: only translate remapped IO-APIC RTEs

["Suthikulpanit, Suravee" <Suravee.Suthikulpanit@xxxxxxx>]

On 4/17/15, 10:27, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

>1aeb1156fa ("x86 don't change affinity with interrupt unmasked")
>introducing RTE reads prior to the respective interrupt having got
>enabled for the first time uncovered a bug in 2ca9fbd739 ("AMD IOMMU:
>allocate IRTE entries instead of using a static mapping"): We obviously
>shouldn't be translating RTEs for which remapping didn't get set up
>yet.
>
>Reported-by: Sander Eikelenboom <linux@xxxxxxxxxxxxxx>
>Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
>--- a/xen/drivers/passthrough/amd/iommu_intr.c
>+++ b/xen/drivers/passthrough/amd/iommu_intr.c
>@@ -365,15 +365,17 @@ unsigned int amd_iommu_read_ioapic_from_
>     unsigned int apic, unsigned int reg)
> {
>     unsigned int val = __io_apic_read(apic, reg);
>+    unsigned int pin = (reg - 0x10) / 2;
>+    unsigned int offset = ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx[pin];
> 
>-    if ( !(reg & 1) )
>+    if ( !(reg & 1) && offset < INTREMAP_ENTRIES )
>     {
>-        unsigned int offset = val & (INTREMAP_ENTRIES - 1);
>         u16 bdf = ioapic_sbdf[IO_APIC_ID(apic)].bdf;
>         u16 seg = ioapic_sbdf[IO_APIC_ID(apic)].seg;
>         u16 req_id = get_intremap_requestor_id(seg, bdf);
>         const u32 *entry = get_intremap_entry(seg, req_id, offset);
> 
>+        ASSERT(offset == (val & (INTREMAP_ENTRIES - 1)));

Jan, could you please explain why the ASSERT is needed here?

Thanks,

Suravee 

>         val &= ~(INTREMAP_ENTRIES - 1);
>         val |= get_field_from_reg_u32(*entry,
>                                       INT_REMAP_ENTRY_INTTYPE_MASK,
>
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel