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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.04.15 at 15:31, <Suravee.Suthikulpanit@xxxxxxx> wrote:

> 
> 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?

The previous value "offset" got assigned was calculated using
the right side expression. I.e. the assert makes sure that what
we used before and what we use now is the same.

Jan


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