Re: [PATCH] AMD IOMMU: also allocate IRTEs for HPET MSI

[Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>]

Jan,

I have been trying to test out this patch.  Somehow, I could not get Xen 
boot option to exercise the code path.  What boot configuration are you 
using to test this?

I'm currently using option

"loglvl=all loglvl_guest=all cpuidle cpufreq=xen debug lapic=debug 
iommu=on,debug,verbose,amd-iommu-debug apic_verbosity=debug apic=debug"

I already double check the IVRS table on my system and it looks 
correct.  Am I missing something here?

Thanks,

Suravee.

On 8/26/2013 9:20 AM, Jan Beulich wrote:
> Omitting this was a blatant oversight of mine in commit 2ca9fbd7 ("AMD
> IOMMU: allocate IRTE entries instead of using a static mapping").
>
> This also changes a bogus inequality check into a sensible one, even
> though it is already known that this will make HPET MSI unusable on
> certain systems (having respective broken firmware). This, however,
> seems better than failing on systems with consistent ACPI tables.
>
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -595,14 +595,31 @@ void* __init amd_iommu_alloc_intremap_ta
>   
>   int __init amd_setup_hpet_msi(struct msi_desc *msi_desc)
>   {
> -    if ( (!msi_desc->hpet_id != hpet_sbdf.id) ||
> -         (hpet_sbdf.iommu == NULL) )
> +    spinlock_t *lock;
> +    unsigned long flags;
> +    int rc = 0;
> +
> +    if ( msi_desc->hpet_id != hpet_sbdf.id || !hpet_sbdf.iommu )
>       {
> -        AMD_IOMMU_DEBUG("Fail to setup HPET MSI remapping\n");
> -        return 1;
> +        AMD_IOMMU_DEBUG("Failed to setup HPET MSI remapping: %s\n",
> +                        hpet_sbdf.iommu ? "Wrong HPET" : "No IOMMU");
> +        return -ENODEV;
>       }
>   
> -    return 0;
> +    lock = get_intremap_lock(hpet_sbdf.seg, hpet_sbdf.bdf);
> +    spin_lock_irqsave(lock, flags);
> +
> +    msi_desc->remap_index = alloc_intremap_entry(hpet_sbdf.seg,
> +                                                 hpet_sbdf.bdf, 1);
> +    if ( msi_desc->remap_index >= INTREMAP_ENTRIES )
> +    {
> +        msi_desc->remap_index = -1;
> +        rc = -ENXIO;
> +    }
> +
> +    spin_unlock_irqrestore(lock, flags);
> +
> +    return rc;
>   }
>   
>   static void dump_intremap_table(const u32 *table)
>
>
>